roslyn: Compiler does not report unused imports when XML doc comments are disabled
https://github.com/dotnet/roslyn/pull/41363 moves the “Remove unnecessary usings” IDE analyzer into the CodeStyle NuGet package, so this analyzer can execute on CI. This analyzer depends on the compiler layer reporting hidden diagnostics (CS8019
) for unused usings: https://github.com/dotnet/roslyn/blob/2b12b9f3b8fe8895694280648b523bc4fd95211f/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/CSharp/Helpers/RemoveUnnecessaryImports/CSharpUnnecessaryImportsProvider.cs#L23-L48.
However, the logic in compiler layer that reports these unused usings is guarded on XML doc comments being enabled: https://github.com/dotnet/roslyn/blob/41bc8954aa449ee96a0858de5f614f8a61fa2cad/src/Compilers/CSharp/Portable/Compilation/CSharpCompilation.cs#L2116-L2140
This logic seems to work fine for IDE-only purpose as XML doc comments are explicitly force enabled in the IDE. However, it breaks down for this same analyzer from command line when XML doc comments are disabled.
I presume this was done as a performance optimization to prevent this code from executing on default command line builds, when XML doc comments are disabled. I see 2 options to retain this performance optimization while fixing the command line case:
- Expose a new public API on Compilation or SemanticModel to for unused usings. The existing compiler code can invoke the same functionality wrapped around the XML documentation mode check, and we can move the IDE’s unnecessary usings analyzer to use this new API instead of
CS8019
diagnostics. - Loosen the compiler layer’s guard around the above code to also execute this code when the effective severity of IDE0005 (unnecessary usings IDE diagnostic) is warning or above. This seems more simpler, but also more fragile change.
IMO, option 1. is a much better approach.
About this issue
- Original URL
- State: open
- Created 4 years ago
- Reactions: 14
- Comments: 31 (14 by maintainers)
Links to this issue
Commits related to this issue
- Add a helper diagnostic for enforcing IDE0005 (remove unnecessary usings) on build Due to #41640, enabling IDE0005 on build requires users to enable generation of XML documentation comments. Even tho... — committed to mavasani/roslyn by mavasani 2 years ago
- Build-break for unused usings Bubble up all unused usings https://github.com/dotnet/roslyn/issues/41640#issuecomment-985780130 — committed to stan-sz/bicep by stan-sz 2 years ago
- Enforce no unused using with build break (#6978) * Build-break for unused usings Bubble up all unused usings https://github.com/dotnet/roslyn/issues/41640#issuecomment-985780130 * Fix code *... — committed to Azure/bicep by stan-sz 2 years ago
- Fix all unused usings Bubble up all unused usings https://github.com/dotnet/roslyn/issues/41640#issuecomment-985780130 — committed to stan-sz/csharp by stan-sz 2 years ago
- Enable generation of XML documentation files Although I don't intend to use XML documentation tags in this project, at least one [1] of the recommended code style analysis rules I want to enforce cur... — committed to smfeest/buttercup-api-server-ef by smfeest 2 years ago
- Enable generation of XML documentation files Although I don't intend to use XML documentation tags in this project, at least one [1] of the recommended code style analysis rules I want to enforce cur... — committed to smfeest/buttercup-api-server-ef by smfeest 2 years ago
- Enable generation of XML documentation files Although I don't intend to use XML documentation tags in this project, at least one [1] of the recommended code style analysis rules I want to enforce cur... — committed to smfeest/buttercup-api-server-ef by smfeest 2 years ago
- Enable generation of XML documentation files Although I don't intend to use XML documentation tags in this project, at least one [1] of the recommended code style analysis rules I want to enforce cur... — committed to smfeest/buttercup-api-server-ef by smfeest 2 years ago
- Enable generation of XML documentation files Although I don't intend to use XML documentation tags in this project, at least one [1] of the recommended code style analysis rules I want to enforce cur... — committed to smfeest/buttercup-api-server-ef by smfeest 2 years ago
- Enable generation of XML documentation files Although I don't intend to use XML documentation tags in this project, at least one [1] of the recommended code style analysis rules I want to enforce cur... — committed to smfeest/buttercup-api-server-ef by smfeest 2 years ago
- Cleanup unused usings Apply the suggestion from https://github.com/dotnet/roslyn/issues/41640#issuecomment-985780130 — committed to stan-sz/vstest by stan-sz 2 years ago
- Enable generation of XML documentation files Although I don't intend to use XML documentation tags in this project, at least one [1] of the recommended code style analysis rules I want to enforce cur... — committed to smfeest/buttercup-api-server-ef by smfeest 2 years ago
- Enable generation of XML documentation files Although I don't intend to use XML documentation tags in this project, at least one [1] of the recommended code style analysis rules I want to enforce cur... — committed to smfeest/buttercup-api-server-ef by smfeest 2 years ago
- Enable generation of XML documentation files At least one [1] of the recommended code style analysis rules I want to enforce currently does nothing [2] unless XML documentation is enabled. [1] https... — committed to smfeest/buttercup-api-server-ef by smfeest 2 years ago
- Workaround for: https://github.com/dotnet/roslyn/issues/41640 — committed to vezel-dev/celerity by alexrp a year ago
- Move GenerateDocumentationFile to Directory.Build.props. This resolves a new compiler warning that the unused imports diagnostic can't be reported unless this setting is enabled: https://github.com/d... — committed to Faithlife/FaithlifeBuild by bgrainger a year ago
- Workaround for: https://github.com/dotnet/roslyn/issues/41640 — committed to vezel-dev/cathode by alexrp a year ago
- Workaround for: https://github.com/dotnet/roslyn/issues/41640 — committed to vezel-dev/zig-sdk by alexrp a year ago
- Workaround for: https://github.com/dotnet/roslyn/issues/41640 — committed to vezel-dev/novadrop by alexrp a year ago
- Workaround for: https://github.com/dotnet/roslyn/issues/41640 — committed to vezel-dev/ruptura by alexrp a year ago
@mavasani
This is now a warning in
7.0.400
:This issue is now 3 and a half years old – when will it be resolved?
For people wondering why command-line/CI builds are letting unused usings through, this is what you need to paste into your csproj or Directory.Build.props after adding
<EnforceCodeStyleInBuild>true</EnforceCodeStyleInBuild>
and editorconfigdotnet_diagnostic.IDE0005.severity = warning
/error
:The core compiler behavior did not change. The only change is that a warning is now alerting you to the fact that the compiler was (always was, and still is) ignoring your requested configuration.
The description of the new diagnostic contains information about how to eliminate the noise. It’s generally as simple as this: https://github.com/dotnet/roslyn-analyzers/blob/8281feab05b88e0fe99e0b9614b39a3ac79eeb58/Directory.Build.props#L34-L46
@sharwell Thanks for the explanation. From a developer experience point of view, I now have a broken build where previously it was working. It’d also be nicer if we didn’t have to disable IDE0005 in our .editorconfig files. If I now do so and in the future Roslyn decides to fix this issue, I will have IDE0005 disabled with no way to know that this bug has been fixed.
@RehanSaeed @lonix1 This is already described in detail in my comment above, including all the options to proceed: https://github.com/dotnet/roslyn/issues/41640#issuecomment-1699278994
The key detail here is the diagnostics described in this issue only appear in cases where a user has explicitly configured their project with a non-default configuration which the compiler is incapable of honoring. There is no actual change in behavior; the only difference is you are now being alerted to the fact that the compiler was and is unable to perform the action your project has explicitly requested.
Please let me know if any part of that is unclear. 😄
I tried to understand this issue but it goes back years and there seems to be workarounds on top of workarounds. Like RehanSaeed noted, it’s all very counter intuitive. And my builds are noisy where before they were not.
I want
GenerateDocumentationFile
disabled for test projects. But I wantIDE0005
(“Remove unnecessary import”) enabled, including for test projects.Is that no longer possible?
This is all very counter-intuitive. For test projects, it doesn’t make sense to enable
GenerateDocumentationFile
since documentation is not needed and slowing down the build for no benefit. Enabling it just to get IDE0005 to work correctly seems strange to me. Is there a way to disable this?TL;DR for the lazy people:
@jnm2’s workaround triggered me to write my own (different) solution as a nuget package that enables IDE0005 warnings in the CLI build. It does not cause a useless XML doc file in the output directory. It also works as expected with both documentation generation enabled and disabled. No manual intervention necessary. See my repo for more information: https://github.com/shuebner/CheckUnnecessaryUsings
Bug reports are of course welcome 😃
@ismaelhamed CS8019 is what makes IDE0005 work. The idea is that it’s harder to implement “unused usings” analyzer outside of the compiler. So, this is implemented as a hidden diagnostic (CS8019) in the compiler, and IDE0005 just bridges CS8019 to the user and provides fading in IDE, etc…
https://github.com/dotnet/roslyn/blob/28754bb176aa35823cb1cca60b259448c856d0a8/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/CSharp/Helpers/RemoveUnnecessaryImports/CSharpUnnecessaryImportsProvider.cs#L23-L44
https://github.com/dotnet/roslyn/blob/28754bb176aa35823cb1cca60b259448c856d0a8/src/Analyzers/Core/Analyzers/RemoveUnnecessaryImports/AbstractRemoveUnnecessaryImportsDiagnosticAnalyzer.cs#L65-L86
In short, the implementation of IDE0005 relies on the existence of CS8019
For my projects that do not need XML doc comments I prefer to disable this new warning:
Very annoying to have to modify all my project files as a workaround to a workaround warning!
Why is it so important to have IDE0005 (Remove unnecessary usings/imports) enabled? Does not removing unnecessary usings/imports cause a runtime / build penalty? Because enabling XML comments like the warning says will make the build slower… is that worth it?
Does not cause build errors when the above rule is set to
severity = error
Shows up as an error in the IDE but is not reported as an error during the build, even with
GenerateDocumentationFile
set to true.@VincentH-Net Disabling
EnableGenerateDocumentationFile
will not work around this issue. In your project, IDE0005 has been explicitly set to either warning or error, yet the build is unable to satisfy the request. This is not a default configuration, but rather one that has been customized as part of your build. If you do not wish to produce documentation files, the solution to this issue is to reduce the severity of IDE0005.Your build has been customized to a non-default configuration where IDE0005 has been explicitly set to enabled with a severity of warning or error. Performing the IDE0005 analysis is a required part of providing IDE0005 diagnostics that have been requested.
Not really. Enabling IDE0005 is a code style decision which has been made as part of your solution.
As with enabling IDE0005, that is a decision for your team. If you want to enforce IDE0005, you will need to enable documentation comments. If you don’t want to enable documentation comments, you can remove the customization where IDE0005 was set to warning or error (reducing it to suggestion or lower severity will remove the warnings and will stop attempting to enforce IDE0005 during builds).
The IDE also uses CS8019 to fade out unnecessary usings in generated code files, where it would never actually report an IDE0005 diagnostic. CS8019 reports all unnecessary usings, while IDE0005 filters those down to only report actionable ones.
@Youssef1313 Got it now, thanks! Key point is
CS8019
being “a hidden diagnostic”.@justinmchase I wouldn’t expect GenerateDocumentationFile to have anything to do with that diagnostic. On a side note, does setting https://docs.microsoft.com/en-us/dotnet/core/project-sdk/msbuild-props#enforcecodestyleinbuild in your csproj help?