Compiler does not report unused imports when XML doc comments are disabled

[mavasani]

#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:

roslyn/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/CSharp/Helpers/RemoveUnnecessaryImports/CSharpUnnecessaryImportsProvider.cs

Lines 23 to 48 in 2b12b9f

	protected override ImmutableArray<SyntaxNode> GetUnnecessaryImports(
	SemanticModel model, SyntaxNode root,
	Func<SyntaxNode, bool> predicate, CancellationToken cancellationToken)
	{
	predicate ??= Functions<SyntaxNode>.True;
	var diagnostics = model.GetDiagnostics(cancellationToken: cancellationToken);
	if (diagnostics.IsEmpty)
	{
	return ImmutableArray<SyntaxNode>.Empty;
	}
	var unnecessaryImports = new HashSet<SyntaxNode>();
	foreach (var diagnostic in diagnostics)
	{
	if (diagnostic.Id == "CS8019")
	{
	if (root.FindNode(diagnostic.Location.SourceSpan) is UsingDirectiveSyntax node && predicate(node))
	{
	unnecessaryImports.Add(node);
	}
	}
	}
	return unnecessaryImports.ToImmutableArray();
	}

.

However, the logic in compiler layer that reports these unused usings is guarded on XML doc comments being enabled:

roslyn/src/Compilers/CSharp/Portable/Compilation/CSharpCompilation.cs

Lines 2116 to 2140 in 41bc895

	internal override void ReportUnusedImports(SyntaxTree? filterTree, DiagnosticBag diagnostics, CancellationToken cancellationToken)
	{
	if (_lazyImportInfos != null && filterTree?.Options.DocumentationMode != DocumentationMode.None)
	{
	foreach (ImportInfo info in _lazyImportInfos)
	{
	cancellationToken.ThrowIfCancellationRequested();
	SyntaxTree infoTree = info.Tree;
	if ((filterTree == null || filterTree == infoTree) && infoTree.Options.DocumentationMode != DocumentationMode.None)
	{
	TextSpan infoSpan = info.Span;
	if (!this.IsImportDirectiveUsed(infoTree, infoSpan.Start))
	{
	ErrorCode code = info.Kind == SyntaxKind.ExternAliasDirective
	? ErrorCode.HDN_UnusedExternAlias
	: ErrorCode.HDN_UnusedUsingDirective;
	diagnostics.Add(code, infoTree.GetLocation(infoSpan));
	}
	}
	}
	}
	CompleteTrees(filterTree);
	}

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.

[mavasani]

Tagging @jcouv @sharwell as they seem to have good context on this IDE analyzer by looking at past issues.

[CyrusNajmabadi]

I thought we had a flag here to at least tell the compiler to bind the docs, even if we don't want errors or xml generation for them. That way our symbolic information is all correct, regardless of what emit behavior the user specifies.

[sharwell]

@mavasani The underlying compiler diagnostic (CS8019) is not accurate if XML documentation parsing is disabled. Currently the project can only specify DocumentationMode.None or DocumentationMode.Diagnose. To enable IDE0005 in command line scenarios without enabling XML documentation file output, we need a way to set the documentation mode to DocumentationMode.Parse.

[mavasani]

Ah, maybe in that case we can enhance the unused usings analyzer to report a separate diagnostic when IDE0005 is bumped to warning/error but documentation mode is None, which informs the user that XML documentation comments need to be enabled for the project to get accurate IDE0005 on CI. Does that sound reasonable? If so, we can move this issue to Area-IDE.

[jnm2]

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>

[justinmchase]

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.

[jnm2]

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

[justinmchase]

<EnforceCodeStyleInBuild>true</EnforceCodeStyleInBuild>

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

[VictorioBerra]

I am struggling a bit. When I set to <GenerateDocumentationFile>true</GenerateDocumentationFile> I still get CS1573,CS1591,CS1712 errors even if I do:

<NoWarn Condition="'$(Language)' == 'C#'">$(NoWarn),1573,1591,1712</NoWarn>.

The only thing that fixes it for me without toggling GenerateDocumentationFile is just <NoWarn>IDE0005</NoWarn>.

What am I doing wrong?

[VictorioBerra]

So this works:

<NoWarn>$(NoWarn),CS1591</NoWarn>

But this doesnt:

<NoWarn Condition="'$(Language)' == 'C#'">$(NoWarn),1573,1591,1712</NoWarn>

@sharwell what am I doing wrong?

[sharwell]

@VictorioBerra I'm guessing you have that line earlier in the build than wherever $(Language) is defined.

[smkanadl]

@VictorioBerra $(Language) is not supported in a Directory.Build.props but only in a Directory.Build.targets file.

[jnm2]

If I remember right, I've used $(MSBuildProjectExtension) for this purpose.

[AraHaan]

I don't like this one but because I do not want things like documentation files to be output when:

I build an winforms project where everything is "internal" visibility

• has dlls with apis that can be used to load a "plugin interface".
• said plugin interface is then used to extend the winforms project to add new features to it (although limited) on what can be added to it.
  • plugins themselves are forced to not generate documentation files as well by an msbuild sdk they are then "required" to bring in for application plugin development.

And yes I have an application of this type and the csc warning from doing this annoys me a lot.

As such I feel that forcing a warning for people who WANT to enforce no documentation file to be generated would be a must, or at least a switch to skip copy on it from the intermediate -> output directory.

Edit:

• It seems .editorconfig cannot be used to silence this warning (dotnet_diagnostic.EnableGenerateDocumentationFile.severity = silent). This is unacceptable.

[sharwell]

It seems .editorconfig cannot be used to silence this warning. This is unacceptable.

This is intentional, similar to what would happen if you add the following to your project:

<ItemGroup>
  <Compile Include="ThisFileDoesNotExist.cs"/>
</ItemGroup>

By explicitly providing the compiler with an instruction to enforce IDE0005 at warning (or greater) severity, and then removing its ability to do so by disabling the documentation file output, the project is in an invalid state. Multiple solutions are provided above to return the project to a valid state (either disable the IDE0005 analysis by setting the severity to suggestion or lower, or enable the documentation output).

[AraHaan]

It seems .editorconfig cannot be used to silence this warning. This is unacceptable.

This is intentional, similar to what would happen if you add the following to your project:

<ItemGroup>
  <Compile Include="ThisFileDoesNotExist.cs"/>
</ItemGroup>

By explicitly providing the compiler with an instruction to enforce IDE0005 at warning (or greater) severity, and then removing its ability to do so by disabling the documentation file output, the project is in an invalid state. Multiple solutions are provided above to return the project to a valid state (either disable the IDE0005 analysis by setting the severity to suggestion or lower, or enable the documentation output).

Which is funny because I never really enabled that analysis on it specifically for outside of Visual Studio 😄.

[sharwell]

Which is funny because I never really enabled that analysis on it specifically for outside of Visual Studio 😄.

IDE0005 is not enabled at warning severity or greater by default. There are many ways a project could be modified to increase its severity, so without a binlog its difficult to be more specific.

[mwasson74]

VS 17.9.2 17.9.3 and .NET 8 project and with all of the extra things added, the build is still not failing. Am I missing something?

Edit: Upgraded VS to 17.9.3 with the same results

[RenderMichael]

Hi, I'm being affected by this:

More specifically, I am generating the documentation file on every project except the testing projects. In my Directory.Build.props file I have

<GenerateDocumentationFile>true</GenerateDocumentationFile>

Directly in the tests project I then set

<GenerateDocumentationFile>false</GenerateDocumentationFile>

Currently I can get around this by NoWarning on EnableGenerateDocumentationFile, but would it be possible to ignore this for projects where GenerateDocumentationFile is explicitly set to false?

[sharwell]

@RenderMichael instead of disabling EnableGenerateDocumentationFile, you should either enable documentation output in your test projects or disable IDE0005 in your test projects. As it stands, your test projects have directly instructed the compiler to perform an operation which is not supported, and it is resulting in a warning.

[RenderMichael]

Thanks for the response.

It's not clear from the user side that IDE0005 requires documentation files to be generated. Can the error message include something along the lines of IDE0005 requires documentation files to be generated. Please enable or disable both in this project?