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:

  1. 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.
  2. 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)

Commits related to this issue

Most upvoted comments

@mavasani

This is now a warning in 7.0.400:

CSC : warning EnableGenerateDocumentationFile: Set MSBuild property ‘GenerateDocumentationFile’ to ‘true’ in project file to enable IDE0005 (Remove unnecessary usings/imports) on build (https://github.com/dotnet/roslyn/issues/41640)

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 editorconfig dotnet_diagnostic.IDE0005.severity = warning/error:

    <!-- Workaround for https://github.com/dotnet/roslyn/issues/41640 -->
    <PropertyGroup>
      <GenerateDocumentationFile>true</GenerateDocumentationFile>
      <NoWarn>$(NoWarn);CS1591;CS1573</NoWarn>
    </PropertyGroup>

Assuming I understand, I can no longer … but the result is it’s different to how it was before

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.

But my builds are definitely noisy now.

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 want IDE0005 (“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:

<PackageReference Include="SvSoft.MSBuild.CheckUnnecessaryUsings" Version="1.0.0" PrivateAssets="all" />

@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

Just FYI: This will produce warnings in builds that previously didn’t have warnings. If your CI treats warning as errors, you will now have a build that mysteriously fails when it used to build without error.

It’s unavoidable, but I’m just leaving this explanation for future readers who also pushed changes and wondered how those changes could possibly have led to a compile error of this kind.

For my projects that do not need XML doc comments I prefer to disable this new warning:

<NoWarn>EnableGenerateDocumentationFile</NoWarn> <!-- Remove this to get a warning how to enable IDE0005 (Remove unnecessary usings/imports) on build; that warning describes a workaround for https://github.com/dotnet/roslyn/issues/41640 -->

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?

<EnforceCodeStyleInBuild>true</EnforceCodeStyleInBuild>

Does not cause build errors when the above rule is set to severity = error

dotnet_diagnostic.CS8619.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.

Screen Shot 2022-04-07 at 10 03 39 PM

@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.

Why is it so important to have IDE0005 (Remove unnecessary usings/imports) enabled?

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.

Does not removing unnecessary usings/imports cause a runtime / build penalty?

Not really. Enabling IDE0005 is a code style decision which has been made as part of your solution.

Because enabling XML comments like the warning says will make the build slower… is that worth it?

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?