Skip to content

Conditionalise loading of NetAnalyzers #234

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.

Sign up for GitHub

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conditionalise loading of NetAnalyzers #234

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 7 additions & 2 deletions src/WTG.Analyzers/WTG.Analyzers.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,16 @@
<Nullable>enable</Nullable>
<LangVersion>10</LangVersion>
</PropertyGroup>
<ItemGroup>
<PackageReference Include="Microsoft.CodeAnalysis.NetAnalyzers" Version="8.0.0">
<ItemGroup Condition="'$(TargetFramework)' != ''">
<!-- Disable for SDK 9 and above -->
<PackageReference Include="Microsoft.CodeAnalysis.NetAnalyzers"
Copy link
Contributor

@brian-reichle brian-reichle Oct 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

globals.json currently pegs the SDK to 8.0.300. Visual Studio seems to ignore this though and use whatever SDK it comes with, but that only affects when WTG.Analyzers is built directly in VS and not when built with dotnet. None of this will have any effect on the consumers of WTG.Analyzers.

I assume you are trying to address the warning produced when the SDK provides a later version of the analyzer package. This approach means the analyzer package is silently dropped when the SDK version is updated past that point. I think a better solution would be to replace this package reference with EnableNETAnalyzers, then it will use whatever version the SDK provides without having to be separately updated.

Copy link
Member

@yaakov-h yaakov-h Oct 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

EnableNETAnalyzers is enabled by default in all supported SDKs by now, so we can probably just drop this entirely.

Copy link
Contributor

@brian-reichle brian-reichle Oct 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That doesn't sound like the way they operate, are you sure it isn't set based on target framework?

Copy link
Member

@yaakov-h yaakov-h Oct 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah you're right, it's net5.0 or higher.

Version="8.0.0"
Condition="!$([MSBuild]::VersionGreaterThanOrEquals('$(NETCoreSdkVersion)', '9.0.100'))" >
<PrivateAssets>all</PrivateAssets>
<IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets>
</PackageReference>
</ItemGroup>
<ItemGroup>
<PackageReference Include="Nullable" Version="1.3.1">
<PrivateAssets>all</PrivateAssets>
<IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets>
Expand Down