Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add auto-formatting support for clang-tidy fixes #9322

Closed
sean-mcmanus opened this issue May 17, 2022 · 6 comments · Fixed by #9661
Closed

Add auto-formatting support for clang-tidy fixes #9322

sean-mcmanus opened this issue May 17, 2022 · 6 comments · Fixed by #9661
Assignees
Labels
enhancement Improvement to an existing feature Feature: Code Analysis Related to integration with clang-tidy, cppcheck, cl.exe /analyze, etc. Feature: Code Formatting fixed Check the Milestone for the release in which the fix is or will be available. Language Service
Milestone

Comments

@sean-mcmanus
Copy link
Collaborator

sean-mcmanus commented May 17, 2022

Some clang-tidy fix types such as for readability-braces-around-statements generates fixes that need to be formatted in order to give acceptable results (otherwise, } will have no indentation). Potentially, we just run the currently configured formatting on the lines that are changed.

One workaround is to add --format-style= to C_Cpp.codeAnalysis.clangTidy.args if clang-format style formatting is desired. Or add FormatStyle to a .clang-tidy file. UPDATE: The workaround doesn't work.

UPDATE: I noticed with readability-else-after-return it is desirable to format the entire range, but the fix only applies it to the start/end lines...not sure if there's a way to improve that.

@sean-mcmanus sean-mcmanus added Language Service enhancement Improvement to an existing feature Feature: Code Analysis Related to integration with clang-tidy, cppcheck, cl.exe /analyze, etc. labels May 17, 2022
@sean-mcmanus sean-mcmanus added this to the 1.11 milestone May 17, 2022
@sean-mcmanus sean-mcmanus self-assigned this May 17, 2022
@sean-mcmanus
Copy link
Collaborator Author

I'm seeing FormatStyle: file not working in .clang-tidy with readability-braces-around-statements. I'll file a clang-tidy bug later.

@sean-mcmanus
Copy link
Collaborator Author

sean-mcmanus commented May 18, 2022

I've filed bug llvm/llvm-project#55569 ...it seems like we could work around that via doing a separate format range though.

@sean-mcmanus
Copy link
Collaborator Author

They said it's by design for --export-fixes to not format.

@H-G-Hristov
Copy link

H-G-Hristov commented Jul 22, 2022

This should be optional. Maybe even popup a dialog to ask for reformatting, so that the user can review the clang-tidy changes before applying the formatting. In any case I tried today the auto-fix feature and I guess Clang-Tidy generates invalid code by adding \ to "%s" such as: \"%s\".

@sean-mcmanus
Copy link
Collaborator Author

@H-G-Hristov Yeah, a setting to disable it sounds good. Do you have repro steps for the invalid `"%s" code? It may be a bug with our processing.

@sean-mcmanus sean-mcmanus added the fixed Check the Milestone for the release in which the fix is or will be available. label Aug 3, 2022
@sean-mcmanus sean-mcmanus modified the milestones: 1.12, 1.12.1 Aug 3, 2022
@Zingam
Copy link

Zingam commented Aug 4, 2022

@H-G-Hristov Yeah, a setting to disable it sounds good. Do you have repro steps for the invalid `"%s" code? It may be a bug with our processing.

@sean-mcmanus Sorry for the delay. I have filed some details here: llvm/llvm-project#56669

@github-actions github-actions bot locked and limited conversation to collaborators Sep 17, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement Improvement to an existing feature Feature: Code Analysis Related to integration with clang-tidy, cppcheck, cl.exe /analyze, etc. Feature: Code Formatting fixed Check the Milestone for the release in which the fix is or will be available. Language Service
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants