Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Microsoft.VisualStudio.Threading analyzers leaking into consuming project #101

Closed
joelverhagen opened this issue Nov 17, 2024 · 2 comments · Fixed by #103
Closed

Microsoft.VisualStudio.Threading analyzers leaking into consuming project #101

joelverhagen opened this issue Nov 17, 2024 · 2 comments · Fixed by #103

Comments

@joelverhagen
Copy link

A bunch of VSTHRD warnings and errors appear in my project when I reference Nerdbank.MessagePack 0.2.34-alpha. I believe this is due to VS threading analyzers you use in the project.

I was able to suppress them by doing <ExcludeAssets>analyzers</ExcludeAssets> on the Nerdbank.MessagePack <ProjectReference> but this forced me to lift the dependency to all of my projects that transitively reference Nerdbank.MessagePack.

I saw this approach in your code but this seems like a pretty extreme measure since I just want to suppress the VS Threading analyzer coming from Nerdbank.MessagePack.

<ItemDefinitionGroup>
<!-- We need this, even with SuppressDependenciesWhenPacking=true because the
dependencies will otherwise show up as dependencies in packages built by projects
that reference this one. Probably due to transitive pinning. -->
<PackageReference PrivateAssets="all" />
<ProjectReference PrivateAssets="all" />
</ItemDefinitionGroup>

Is this expected? Personally, I would expect MessagePack-related analyzers to run but not Visual Studio ones.

Right now, I preferred working around the problem by simply suppressing the message codes I didn't want, e.g.

dotnet_diagnostic.VSTHRD002.severity = none
dotnet_diagnostic.VSTHRD003.severity = none
dotnet_diagnostic.VSTHRD011.severity = none
dotnet_diagnostic.VSTHRD103.severity = none
dotnet_diagnostic.VSTHRD200.severity = none
@AArnott AArnott changed the title Microsoft.VisualStudio.Validation analyzers leaking into consuming project Microsoft.VisualStudio.Threading analyzers leaking into consuming project Nov 17, 2024
@AArnott
Copy link
Owner

AArnott commented Nov 17, 2024

I saw this approach in your code but this seems like a pretty extreme measure

That doesn't address the same problem you're seeing and isn't extreme. It's necessary to prevent any dependencies in the analyzers from being exposed as nuget dependencies. Analyzers must be entirely self-contained, so any dependencies they have that aren't provided by the compiler must be bundled in the nuget package itself, and thus there is no need or desire to have the package itself express dependencies to support analyzers. The xml snippet you found prevents these dependencies from showing up in the package.

It's entirely separate the problem you're facing, which is totally a legit complaint. Sadly, nuget doesn't support PrivateAssets="analyzers" or I would very happily do that, which would prevent analyzers associated with upstream runtime dependency packages from activating for your project.
This has been a long outstanding feature request against nuget itself, as you can see at microsoft/vs-streamjsonrpc#195, which is the same as this one except it was filed against the StreamJsonRpc library.
The request on nuget is here: NuGet/Home#6279 (please 👍🏻 vote it up!)

In the meantime, there are only two options:

  1. Suppress the analyzers on your consuming side (as you've already done). This is the approach I recommend.
  2. Nerdbank.MessagePack drop its dependencies on other packages that bring in analyzers. This would require that we drop Nerdbank.Streams as a dependency, which is conceivable but would be a loss in some ways.

@AArnott
Copy link
Owner

AArnott commented Nov 17, 2024

I would expect MessagePack-related analyzers to run but not Visual Studio ones

FYI these "Visual Studio ones" are not specific to Visual Studio, and in fact many of them may be useful for most apps to have. In fact some of them are so generally interesting that the .NET SDK started porting them from the vs-threading analyzer library to the SDK a while back. That work was never finished, unfortunately.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants