Add an analyzer rule against unused using directives #19747
Conversation
|
Did you try defining the full <Rules AnalyzerId="Microsoft.CodeAnalysis.CSharp.Features" RuleNamespace="Microsoft.CodeAnalysis.CSharp.Features">
<Rule Id="IDE0005" Action="Warning" />
</Rules>? IDE0005 isn't part of the |
|
Tested both namespaces ( |
|
So it looks like IDE0005 specifically is borked. IDE0005 docs have a note which states If you follow the advice, you can indeed get it to show up at build time. Add this to the <PropertyGroup>
<GenerateDocumentationFile>true</GenerateDocumentationFile>
</PropertyGroup>You'll get some additional spam but it does actually work! Therefore your changes are fine, it's just a bug in the infrastructure. I wouldn't bother with the workaround since it adds other spam - I think adding the rules will do for now and at some point in future I guess it will magically work in the build again. Other IDEXXX rules are not affected and appear to work without any workarounds. That said I agree with pchote it would be best to put it into the correct analyser group. Apparently you can choose between <Rules AnalyzerId="Microsoft.CodeAnalysis.CSharp.CodeStyle" RuleNamespace="Microsoft.CodeAnalysis.CSharp.CodeStyle">
<Rule Id="IDE0005" Action="Warning" />
</Rules>Changes looks fine otherwise. |
penev92
force-pushed
the
usingDirectives
branch
from
77c864b
to
ef15bb2
Compare
|
Thanks to @RoosterDragon we now know we need a combination of |
|
We could avoid the VS problems by specifying I'm pretty sure the CS rules also don't live in StyleCop's namespace? |
|
Can confirm the above would work. We still get the warning in the IDE, but |
|
Yeah, that seems like the best compromise to me - they just need to go in the right namespace 😁 |
|
This is the namespace you'll want for compiler rules. <Rules AnalyzerId="Microsoft.CodeAnalysis.CSharp" RuleNamespace="Microsoft.CodeAnalysis.CSharp">
<Rule Id="CS1570" Action="None" />
<Rule Id="CS1573" Action="None" />
<Rule Id="CS1591" Action="None" />
</Rules> |
penev92
force-pushed
the
usingDirectives
branch
from
ef15bb2
to
d7f0d6f
Compare
|
Added a separate "group" for the CSxxxx (XML documentation generation) rules with the proper namespace as provided above. |
penev92
force-pushed
the
usingDirectives
branch
3 times, most recently
from
3adfbe9
to
42eb603
Compare
|
*Also made the XML generation (and code style enforcement) only run for the |
Unfortunately due to bugs in the analyzers or something else, the IDE0005 doesn't work as expected. The "officially suggested" workaround is to enable XML documentation generation to trigger IDE0005 during compiling. Then we need to add three more rules to silence the warnings that come from the XML documentation generation. We also need to enable code style enforcing on build for all of this to work. Known issue is that all of this produces a bunch (tens to hundreds) of obscure analyzer warnings on older versions of Visual Studio, but those seem to not be causing issues.
penev92
force-pushed
the
usingDirectives
branch
from
42eb603
to
610cf93
Compare
abcdefg30
added
PR: Unaddressed comment(s)
and removed
PR: Unaddressed comment(s)
labels
…ngs) on build Due to dotnet#41640, enabling IDE0005 on build requires users to enable generation of XML documentation comments. Even though this is documented with a note on IDE0005's [doc page](https://docs.microsoft.com/en-us/dotnet/fundamentals/code-analysis/style-rules/ide0005), we have had numerous reports of users not figuring this out and spending lot of cycles fighting with this, especially given other IDE diagnostics work just fine on build. We have had many user reports of the same: dotnet#58103, dotnet#53720, dotnet#57539, OpenRA/OpenRA#19747 and numerous other offline queries. This change enhances the IDE0005 analyzer to now detect the case when IDE0005 is being reported as a warning or an error in the IDE, but `GenerateDocumentationFile` is `false` for the project, which would mean IDE0005 wouldn't be reported on build. The analyzer reports a special helper diagnostic for this case, which recommends the user to set this property to `true` in their project file to enable IDE0005 on build. This should reduce the pain for customers who try to enforce IDE0005 on build and get hit by this issue.
What started as simple "spring" cleaning turned out bigger than expected, so splitting it into its own PR.
Unfortunately none of the options I tried with the suggested analyzer rules
IDE0005andCS8019could makemake checkfail in any way, but I still think having the rule is an improvement. Arguably it could be even made to show as an error inside an IDE (both VS and VSCode do it), but it still won't stop the build.