clang-power-tools icon indicating copy to clipboard operation
clang-power-tools copied to clipboard

Make Clang-tidy in power tools Visual Studio error list window friendly

Open mydeveloperday opened this issue 7 years ago • 8 comments

If the error messages from clang-tidy were rearranged before being sent to the output window from <file>(<line>): warning: description [clang-tidy-checker]

into

<file>(<line>): warning [clang-tidy-checker]: description

so that issues like this C:\Program Files (x86)\Windows Kits\10\include\10.0.16299.0\um\winnt.h(19370): warning: redundant '__fastfail' declaration [readability-redundant-declaration]

would be

C:\Program Files (x86)\Windows Kits\10\include\10.0.16299.0\um\winnt.h(19370): warning [readability-redundant-declaration]: redundant '__fastfail' declaration

And then that would automatically put the [clang-tidy-checker] into the "Code" section of the ErrorList

And then that would allow us to sort/filter by clang-tidy-checker post running

image

mydeveloperday avatar Feb 28 '18 13:02 mydeveloperday

Hi,

Interesting suggestion. Thanks for the feedback 👍

cpp-red-lion avatar Feb 28 '18 13:02 cpp-red-lion

Hi,

Thank you for your interesting suggestion. Currently we are manually adding the errors messages in the output window and error list. To make an action over the error list we are using an ErrorListProvider object.

We rearranged the error messages using the format that you suggested <file>(<line>): warning [clang-tidy-checker]: description. After that we tried to sync the output window with the error list and we found 3 methods to do this. The scope is to write just in output window and the errors to be automatically transferred to the error list.

The methods are:

  1. The errors are written in the output window using the OutputWindowPane.OutputTaskItemString() method and to be sure that the new task items are added in the error list we called the OutputWindowPane.ForceItemsToTaskList() method.

  2. The errors are written in the output window using the IVsOutputWindowPane.OutputTaskItemString() method and to be sure that the new task items are added in the error list we called the IVsOutputWindowPane.FlushToTaskList() method.

  3. We have tried the IVsOutputWindowPane.OutputTaskItemStringEx() and IVsOutputWindowPane2.OutputTaskItemStringEx2() methods, these are just extensions of the IVsOutputWindowPane.OutputTaskItemString() method, but we tried them anyway.

All the above approaches have the same result. Unfortunately these are the main problems:

  1. The navigation for some items, from both output window and error list, does not work anymore.

  2. The Code field from error list is still empty and none of the above methods do not expose the possibility to set it through a parameter.

  3. Sometimes when a command (compile/tidy) was given on a file which contains many errors, over one thousand from our experience, at the next command the error list still contains the old records and not the new ones. The old errors appear in the error list but none of these are functional when we double clicking.

Do you know a better way to do this? Any help would be appreciated😃

Enache-Ionut avatar Mar 15 '18 08:03 Enache-Ionut

Well I'm not sure about the C# calls from the extension, for me I have an inhouse C# tool which runs as part of the BuildEvent (Pre/Post build), think of it like a linter written in C# to Lint C++ code performing some of what clang-tidy does

This C# application does nothing but write to the Console with WriteLine

1>C:\MyFile.cxx(30): warning CC0143: (PossibleBug) wcsncmp call with a literal size, too easy for a bug to occur use size of string
1>C:\MyOtherFile.cxx(396): warning CC0264: (PossibleBug) static_cast<void>(x) of variable which is potentially used later
1>C:\MyOtherFile.cxx(509): warning CC0264: (PossibleBug) static_cast<void>(x) of variable which is potentially used later
1>C:\MyOtherFile.cxx(532): warning CC0129: (PossibleBug) could be not(!) .empty()

I know that during the development of this tool, if I made my output message match what the Visual C++ compiler would generate i.e.

<filepath>(line)(colon) warning <CODE>(colon) <my description>

Then the error list would pick up the code

image

This then gives you access to filter out the messages based on their code-type

image

For clang tidy, if your not having it automatically make the change, then reviewing all the different messages can be hard, but focusing on one or two clang-tidy checks at a time makes reviewing them much easier

I realize your logging your output into the "Clang Power Tools" "Output Windows" and so it might be harder.

mydeveloperday avatar Mar 15 '18 12:03 mydeveloperday

maybe check out https://github.com/madskristensen/WebAnalyzer

from the sceenshots it looks like they are doing it..

to be honest you should reach out to Mads Kristensen (@mkristensen ) this guy IS Mr Visual Studio extension, if he doesn't know how to do it, then it probably can't be done!

mydeveloperday avatar Mar 15 '18 12:03 mydeveloperday

This is a great piece of work, but this is the single thing that is holding this back from being a useful tool (on exiting projects). Getting thousands of diagnostics in a linear list is not very workable.

Similarly, the options relating to which checks should be performed are fairly painful to work through. I know that you can add a .tidycode file or use the custom options, but it would be better to be able to select from the smaller number of top-level test categories, and suppress tests that are not of interest. Alternatively, having some way of manipulating the seemingly infinite list of true/false switches would help.

Some of this is achieved by ReShaper ++ (which also provides a 1-click way of applying a fix once you have selected the issue).

jrp2014 avatar Mar 29 '18 19:03 jrp2014

Hi @jrp2014 ,

My buddy @Enache-Ionut is hard-at-work on this issue, but it turns out it's not as easy to implement as it seems.
He made significant progress but hit some blockers and reached-out for help in the VS community.
We'll update this thread when we have it completely functional.

Regarding the clang-tidy checks toggle switches in Settings we're aware of the limitation of not being able to do bulk toggle. This is a limitation of the stock UI controller we use for Settings. We have current plans to update that UI. In the meantime, you can use our Cutom checks section where you can specify a list of checks using wildcards, eg. modernize-*,readability-*

Thanks for your feedback.

cpp-red-lion avatar Mar 30 '18 08:03 cpp-red-lion

Thanks.

jrp2014 avatar Mar 30 '18 09:03 jrp2014

An alternative (to using the name of the Clang Tidy Check) would be to assign arbitrary code values (warning numbers) to each Clang Tidy check, then place those warning numbers into the Code column in the Visual Studio Errors window.

The SonarLint for Visual Studio 2022 extension does exactly this. The code for this extension is open source here in GitHub so it should be straightforward to see how they pulled this off.

As for the name of the Clang Tidy Check, it already shows up in the Description (though it might be best if the name of the check came first).

The arbitrary warning numbers associated with the Clang Tidy checks could have a numbering scheme. For example, warnings with available fixes could be numbered 1000 to 4999, while warnings from checks which lack automatic fixes could be in the range 5000 to 9999. Within these ranges, there could be further sub-divisions, based on the group of checks (e.g. readability, bug-prone, modernize, etc.), the severity of the warning, or other criteria.

SonarLint uses the code prefix cpp: to distinguish their warning numbers from the Visual C++ compiler (which prefixes warning numbers with C). Perhaps cpt: or ct: would be a suitable prefix for each warning number.

Also, the Code values (warning numbers shown in the Visual Studio Errors window) can be hyperlinks. Microsoft VC++ compiler warnings link to the corresponding Microsoft Learn page. SonarLint warnings link to their reference pages showing non-compliant and compliant code examples. The code values for Clang Tidy warnings could link to the Clang Tidy page for the corresponding check.

Mike4Online avatar Feb 08 '23 23:02 Mike4Online