microsoft / vscode-cpptools

Official repository for the Microsoft C/C++ extension for VS Code.
Other
5.46k stars 1.53k forks source link

Implement enhanced integration with clang-tidy #2908

Closed ci70 closed 2 years ago

ci70 commented 5 years ago

Implement support for clang extra tools.

EDIT: This issue will just cover clang-tidy. The remaining tools are covered by issue #7276

sean-mcmanus commented 5 years ago

We support clang-format already. Other VS Code extensions might exist that use the other clang tools.

ci70 commented 5 years ago

Ugh. They don't.

lawsonfulton commented 5 years ago

Here's another vote for clang-tidy

skomik commented 5 years ago

And another vote for clang-tidy

tehKaiN commented 4 years ago

There's an extension for clang-tidy (https://marketplace.visualstudio.com/items?itemName=alesiong.clang-tidy-linter) but apparently it's incomplete (some TODOs) and discontinued (last update about year ago).

So, what's the decision for supporting linting in vscode-cpptools? Is it on the roadmap or should we rely on other extensions?

sean-mcmanus commented 4 years ago

@tehKaiN It's not on our extension's roadmap currently.

sean-mcmanus commented 4 years ago

It looks like Visual Studio has added support for clang-tidy: https://devblogs.microsoft.com/cppblog/code-analysis-with-clang-tidy-in-visual-studio/ ...but VS doesn't run on Mac/Linux.

Adobe-Android commented 4 years ago

I still think that clang-tidy support should be added as a first-party feature, but it looks like progress has been made in this space. I'm happily using the following extension: https://marketplace.visualstudio.com/items?itemName=notskm.clang-tidy

Shatur commented 4 years ago

Yes, it would be nice to have because using it as a separate extension (https://github.com/notskm/vscode-clang-tidy for example) is incredibly slow.

https://github.com/clangd/vscode-clangd have support for clang-tidy, but it not as good as this plugin.

Adobe-Android commented 4 years ago

I believe it makes sense for Microsoft to start looking at this. The only real alternative will no longer be maintained. https://github.com/notskm/vscode-clang-tidy/issues/45 Is there anywhere else where we can vote for this feature that would have more visibility?

github-actions[bot] commented 3 years ago

This feature request has received enough votes to be added to our backlog.

rahatchd commented 3 years ago

is the thumbs up reaction enough? bc i'd like to cast a vote for this feature. specifically clang-tidy.

clang-tidy and cppcheck are the only reasons i still use clion, but i would much rather use vscode

Kri-Ol commented 3 years ago

PLease!

benvanik commented 3 years ago

I believe it makes sense for Microsoft to start looking at this. The only real alternative will no longer be maintained. notskm/vscode-clang-tidy#45

Yeah, and it also has some issues with remote workspaces and is lacking features that would make vscode shine like using CodeActionKind so clang tidy fixes can be applied nicely in the UI. I think clang-tidy fits in line with the cpptools including clang-format support and I'm hopeful a cpptools implementation would use all some of the cool vscode features :)

mh0s41n commented 3 years ago

I simply don't understand why they don't understand the importance of Clang-tidy. It is as important as C/C++ for Visual Studio Code or Clangd to me. Clang-tidy is the only reason I'm not switching to VS Code.

sean-mcmanus commented 3 years ago

@mh0s41n This is currently the 5th highest voted feature request, so it seems like it's important to me. Our current focus for 1.2 is on improving stability, testing, and C++20 IntelliSense issues. I'm not sure yet what the feature roadmap is after that.

ci70 commented 3 years ago

Didn't even know this was upvoted.

bobbrow commented 3 years ago

Moving this to "On Deck." This feature may need to be split into smaller ones as we might not be able to do all the tools in a single release. clang-tidy will be the highest priority.

MrSparc commented 3 years ago

Great news, clang-tidy will be priority. ETA? Thanks

sean-mcmanus commented 3 years ago

I don't have an ETA yet, but if you see this issue moved to a particular milestone then that milestone will usually have a target release date. We're about to release 1.2.0 in a few days and 1.3.0 is expected to last 6 weeks or so, but I don't think this issue will make that milestone, so it would be 1.4.0 or later, which could be 3 months.

KaungZawHtet commented 3 years ago

Isnt clangd the official extension from LLVM team ? If that so, I wish cooperation between Microsoft and LLVM for compatibility between the two extensions. Reinventing the wheel may not be a good option in every case.

bobbrow commented 3 years ago

This issue is going to be repurposed to cover the implementation of just clang-tidy. We believe that's what the majority of folks who upvoted this issue would like to see. In the event that this is incorrect, we copied out the remaining clang tools mentioned in this issue to a new issue #7276 and if you have interest in those, please go and add your 👍 to that issue.

sean-mcmanus commented 3 years ago

To use clang-tidy in VS Code right now (without installing a clang-tidy extension), you can add the following to the "tasks" array in tasks.json:

        {
            "type": "shell",
            "label": "clang-tidy",
            "command": "<pathToClangTidy>",
            "args": [
                "${file}",
// any other clang-tidy arguments (https://releases.llvm.org/12.0.1/tools/clang/tools/extra/docs/clang-tidy/index.html)
                // "-p",
                // "<pathToCompileCommands.json>",
// any other arguments to your compiler (if you're not using -p)
                "--",
                "-std=c++17" 
            ],
            "options": {
                "cwd": "${fileDirname}"
            },
            "problemMatcher": [
                {
                    "base": "$gcc",
                    "source": "clang-tidy"
                }
            ],
            "group": "none"
        }

and then do Run Task and select "clang-tidy" (or see https://code.visualstudio.com/docs/getstarted/tips-and-tricks#_define-keyboard-shortcuts-for-tasks for setting up a keyboard shortcut).

image

You can get the clang-tidy 12.0.0 binaries from https://github.com/llvm/llvm-project/releases/tag/llvmorg-12.0.0 (160-400 MB download) or the 12.0.1 binaries we've built below (~15 MB download):

Does anyone have complaints with this existing tasks.json approach (or other clang-tidy extensions) or suggestions on how you'd like our C/C++ extension to improve upon that experience via "integrating" clang-tidy for our next release? I see one user suggest having code actions for fixes. UPDATE: We've identified various "bugs" that could be potentially be fixed via "integration".

One thing to note is that clang-tidy itself can run really slowly for non-trivial cases. For example, it can take over 3 minutes (with default checks) for a file that takes our IntelliSense process 7 seconds, so our integration wouldn't make it any faster.

ben-edna commented 3 years ago

Is it somehow possible to use fields from the c_cpp_properties.json as variables in the task definition?

Such as:

Or alternatively could your C/C++ extension generate a compile_commands.txt or compile_flags.txt

sean-mcmanus commented 3 years ago

@ben-edna c_cpp_properties.json can't be referenced in the task definition (same issue exists with build tasks). You could use CMake or another tool to generate the compile_commands.json, which could be used by IntelliSense and clang-tidy.

When we add clang-tidy integration, we may be able to fallback to the configurations in c_cpp_properties.json if no overriding .clang-tidy files or compile commands exist.

Zingam commented 2 years ago

It's great that it's happening. But what about CMake support? Even about VS I could only find how to enable it via CMakeSettings.json and nothing about configuration via presets.

sean-mcmanus commented 2 years ago

@Zingam It works with CMake Tools as the configuration provider, i.e. it just hooks into whatever configuration is being used for IntelliSense. We won't have "run on build" for V1, but "Run Code Analysis on All Files" should be exactly the same if you have the configuration provider or compile commands set. Was there additional CMake support you wanted?

Zingam commented 2 years ago

@sean-mcmanus I was thinking in the context of CMakePresets something like the settings in CMakeSettings.json https://docs.microsoft.com/en-us/cpp/code-quality/clang-tidy?view=msvc-170 It maybe more of a case for the CMake extension. Let me see how the integration will work when you release it. It got merged right on time. I hope it won't be too long before release so I have the time to gain experience first hand myself. I plan to start integrating clang-tidy and the other tools in our project and I want to "enforce" them on the other developers by some global setting, so I don't have to explain myself over and over to each one of them how to setup and use these tools.

Thank you for your work! I really appreciate it.

sean-mcmanus commented 2 years ago

@Zingam You should be able to enforce the clang-tidy rules with a checked in .clang-tidy file. But if you want to exclude certain folders from being processed (i.e. due to the code not yet conforming to the .clang-tidy rules or owned by other teams), then you'll also need to check in a .vscode/settings.json with C_Cpp.codeAnalysis.exclude set as well.

rickstaa commented 2 years ago

@sean-mcmanus I tested the feature in the insider version, and it works seamlessly. Thanks a lot to you and the team for implementing this!

heartacker commented 2 years ago

yeah, thank you @sean-mcmanus and cpptools team for fantastic works . I had been waiting for a long long time.

sean-mcmanus commented 2 years ago

This is now available for "preview" in 1.8.0-insiders: https://github.com/microsoft/vscode-cpptools/releases/tag/1.8.0-insiders .

There are some known bugs we're working on fixing still for 1.8.0 (e.g. saving of headers may fail if clang-tidy is processing the header as part of a TU that is different from the "default" TU chosen by IntelliSense). And the clang-tidy quick fixes are postponed till the next release.

Zingam commented 2 years ago

+1 for "And the clang-tidy quick fixes are postponed till the next release." - for the quick fixes, not the postponing.