search

dotnet / roslyn

The Roslyn .NET compiler provides C# and Visual Basic languages with rich code analysis APIs.
https://docs.microsoft.com/dotnet/csharp/roslyn-sdk/
MIT License
19.04k stars 4.03k forks source link

The way analyzers interact with source packages is bad #51988

Open jnm2 opened 3 years ago

jnm2 commented 3 years ago

Version Used: 16.9.2, 16.10.P1

This is a full minimal repro. Open this project in VS, and you will see tons of suggestions in the error list. In the real-world version of this, it's full of warnings too because other analyzers are present. Both warnings and suggestions coming from source included in NuGet compile content files are a bad thing. Warnings break builds, and suggestions and warnings at this scale wash away all useful entries in the error list for the consuming project.

This problem is exacerbated by the fact that the consuming project's .editorconfig has no ability to silence these diagnostics as a workaround for this challenge. This in itself should be another sign that showing these diagnostics in the consuming project does not make sense.

<Project Sdk="Microsoft.NET.Sdk">

  <PropertyGroup>
    <TargetFramework>net5.0</TargetFramework>
    <Nullable>enable</Nullable>
  </PropertyGroup>

  <ItemGroup>
    <PackageReference Include="Gu.Roslyn.Extensions.Source" Version="0.15.7" />
  </ItemGroup>

</Project>

image

Roslyn clearly knows these are content files external to the solution, so it should treat them like it would treat generated code.

image

"Just add // <auto-generated/>"

No, that would make the source package incredibly painful to develop. It would silence warnings that are really needed while editing these files. Adding a packaging step to insert this line in the NuGet package without having it present in the edited source files would also add major maintenance overhead to producing a source package.

Takeaway

The out-of-the-box defaults on both sides, both for producers and consumers of source packages (and analyzers), should only result in sensible interactions in the consuming projects.

jmarolf commented 3 years ago

Roslyn clearly knows these are content files external to the solution, so it should treat them like it would treat generated code.

In Visual Studio we definitely know this but the compiler is just passed files, it can't differentiate their source at that level.

I agree with the thesis presented though: If a file is effectively readonly then I should not see diagnostics for it. My initial thoughts are that we would capture some MSBuild metadata about the files and pass them to the compiler via the CompilerVisibleProperty mechanism. Then the analysis engine could filter from there