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

project.assets.json should indicate "analyzer" assets like "compile" and "runtime" #6279

Open
Tracked by #10846
nguerrera opened this issue Dec 8, 2017 · 41 comments
Labels
Area:Analyzers Category:SeasonOfGiving https://devblogs.microsoft.com/nuget/nuget-season-of-giving/#season-of-giving Functionality:Restore Partner:DotNet Priority:2 Issues for the current backlog. Style:PackageReference Type:Feature

Comments

@nguerrera
Copy link

nguerrera commented Dec 8, 2017

project.assets.json doesn't actually indicate which analyzer references a project should use.

Unlike "compile" and "runtime", the SDK is just pattern matching on all files in the package to find analyzers: https://github.com/dotnet/sdk/blob/634680ab0ae045ba45977b6293b46f95a0385aac/src/Tasks/Microsoft.NET.Build.Tasks/ResolvePackageDependencies.cs#L251

"analyzer" assets should work like "compile" and "runtime" and the SDK should resolve them the same way. The current mechanism blocks respecting PrivateAssets and ExcludeAssets=analyzers

cc @emgarten

@emgarten
Copy link
Member

Analyzers were added without work being done on the NuGet side originally. We should add it to project.assets.json so that the SDK can switch over to using that instead of reading packages directly to find the analyzers folder.

@nguerrera
Copy link
Author

nguerrera commented Dec 23, 2017

Related to this, is there a spec for how analyzer assets should be selected based on project language?

Current implementations are here:
https://github.com/NuGet/NuGet.BuildTasks/blob/1d8af3499f94a32c1d128a42faceade39c1f4337/src/Microsoft.NuGet.Build.Tasks/ResolveNuGetPackageAssets.cs#L444-L470
https://github.com/dotnet/sdk/blob/634680ab0ae045ba45977b6293b46f95a0385aac/src/Tasks/Microsoft.NET.Build.Tasks/ResolvePackageDependencies.cs#L251-L272

I believe they are semantically equivalent, but both have some strange behavior if I'm reading correctly.

  • Leading "analyzers" is matched without separator
    • So "analyzersBanana/myanalyzer.dll" would match
  • Leading "analyzers" is case-sensitive.
    • Is that how the others like "lib" and "ref" are matched too?
  • C# projects matches ("*/cs/*" or not "*/vb/*") and VB matches ("*/vb/*" or not "*/cs/*")?
    • This seems to ignore the possibility of other languages gaining analyzers. e.g. If there were F# analyzers, they would match VB and C# projects.
  • Looking for language in any path component seems like a hammer vs how other nuget assets are matched.

cc @jinujoseph @natidea @jasonmalinowski

@jasonmalinowski
Copy link
Member

Adding @jmarolf too.

This seems to ignores the possibility of other languages gaining analyzers. e.g. If there were F# analyzers, they would match VB and C# projects.

I think that was just the code trying to be tricky and deal with the "no language either" thing at the same time.

@nguerrera
Copy link
Author

cc @davidfowl

@davidfowl
Copy link
Member

This seems pretty broken. It means that analyzers don't respect the PrivateAssets or flags specified on the PacakgeReference (I see a few people suffering from this already today).

@nguerrera
Copy link
Author

Yes, it is pretty broken. See two linked sdk bugs that are blocked on this.

@roryprimrose
Copy link

I have to run this powershell on my builds as a workaround using the parameters -path $(Build.SourcesDirectory) -packageName $(Build.DefinitionName)

Param(
   [string]$path,
   [string]$packageName
)

Add-Type -Assembly System.IO.Compression.FileSystem

Function Remove-RoslynAnalyzers([string] $filePath, [string] $projectName)
{
    if ((Test-Path -Path $filePath) -eq $false)
    {
        Write-Host "$filePath was not found"

        return
    }

    Write-Host "Scanning $filePath"

    $ZipStream = [System.IO.Compression.ZipFile]::Open("$filePath", 'Update')
    $ZipItem = $ZipStream.GetEntry("$projectName.nuspec")

    if ($ZipItem -eq $null)
    {
        Write-Host "$projectName.nuspec was not found in the zip"

        return
    }

    [xml]$nuspec = $null
    $reader = $null
    
    $reader = New-Object System.IO.StreamReader($ZipItem.Open())

    try
    {
        [xml]$nuspec = $reader.ReadToEnd()
    }
    finally
    {
        if ($null -ne $reader) 
        { 
            $reader.Close() 
            $reader.Dispose() 
        }
    }

    Clean-NuSpec $nuspec
    
    # Re-open the file this time with streamwriter
    $writer = [System.IO.StreamWriter]($ZipItem).Open()
    
    try
    {
        # If needed, zero out the file -- in case the new file is shorter than the old one
        $writer.BaseStream.SetLength(0)

        # Insert the $text to the file and close
        $writer.Write($nuspec.OuterXml)
    }
    finally
    {
        $writer.Flush()
        $writer.Close()
    }

    # Write the changes and close the zip file
    $ZipStream.Dispose()
}

Function Clean-NuSpec([xml]$xml)
{
    $namespace = "http://schemas.microsoft.com/packaging/2013/05/nuspec.xsd"
        
    [System.Xml.XmlNamespaceManager] $nsMgr = New-Object System.Xml.XmlNamespaceManager($nuspec.NameTable)
    $nsMgr.AddNamespace("ns", "http://schemas.microsoft.com/packaging/2013/05/nuspec.xsd");
    
    Remove-Package $nuspec "CodeCracker.CSharp" $nsMgr
        
    [string]$analyzersPath = "//ns:package/ns:metadata/ns:dependencies/ns:group/ns:dependency[contains(@id,'.Analyzers')]"
        
    $nodes = $nuspec.SelectNodes($analyzersPath, $nsMgr)
        
    foreach ($node in $nodes)
    {
        [string]$id = $node.Attributes["id"].Value

        Write-Host "Removing $id"

        $node.ParentNode.RemoveChild($node)
    }
}

Function Remove-Package([xml]$xml, [string] $name, [System.Xml.XmlNamespaceManager] $nsMgr)
{
    [string]$path = "//ns:package/ns:metadata/ns:dependencies/ns:group/ns:dependency[@id='$name']"

    $nodes = $xml.SelectNodes($path, $nsMgr)
    
    foreach ($node in $nodes)
    {
        [string]$id = $node.Attributes["id"].Value

        Write-Host "Removing $id"

        $node.ParentNode.RemoveChild($node)
    }
}

$items = Get-ChildItem $path -Recurse -Filter "$packageName.*.nupkg"

foreach ($item in $items)
{
     Remove-RoslynAnalyzers $item.FullName $packageName | out-null
}

@davidfowl
Copy link
Member

@rrelyea I see this is both a pri 0 and on the backlog. Any chance we can re-triage this?

@nguerrera
Copy link
Author

See also dotnet/sdk#968 (comment) that notes that PrivateAssets="all" can prevent analyzers from flowing, if you can afford to prevent all other package assets from flowing.

@AArnott
Copy link
Contributor

AArnott commented Oct 23, 2018

Is there a workaround for the problem we're seeing in #7426? Specifically, we want to suppress chaining in analyzers as a transitive dependency through our package.

@AArnott
Copy link
Contributor

AArnott commented Oct 23, 2018

I just tried adding ExcludeAssets="analyzers" to my PackageReference and it still didn't exclude the analyzers. Is there no way to turn off analyzers brought in by a transitive dependency?

@levhaikin
Copy link

levhaikin commented Jan 15, 2019

have you tried <PrivateAssets>all</PrivateAssets>?

e.g.:
<ItemGroup> <PackageReference Include="Microsoft.CodeAnalysis.FxCopAnalyzers" Version="2.6.3" > <PrivateAssets>all</PrivateAssets> </PackageReference> <PackageReference Include="StyleCop.Analyzers" Version="1.1.1-beta.61" > <PrivateAssets>all</PrivateAssets> </PackageReference> </ItemGroup>

@AArnott
Copy link
Contributor

AArnott commented Jan 15, 2019

@levhaikin PrivateAssets=all only applies if I am the one with a direct dependency on an analyzer package. And yes, that works.

But if I'm referencing a package that should propagate to my consumers, but that package itself has an analyzer dependency, I should be able to suppress that using ExcludeAssets="analyzers", but it doesn't work.

@ViktorHofer
Copy link

ViktorHofer commented Sep 27, 2022

@nkolev92 what does Category:SeasonOfGiving mean? Is that some sort of backlog? Note that this became much more important with the addition of source generators that often ship via nuget packages.

@nkolev92
Copy link
Member

Hey all,
if you are curious about the Category:SeasonOfGiving effort, refer to our blog, https://devblogs.microsoft.com/nuget/nuget-season-of-giving/#season-of-giving.

@erdembayar erdembayar self-assigned this Oct 1, 2022
@nkolev92 nkolev92 removed the Category:Quality Week Issues that should be considered for quality week label Oct 3, 2022
@erdembayar erdembayar removed their assignment Nov 8, 2022
@erdembayar
Copy link
Contributor

Unassigned myself for now. If no one pickup for Nov then I'll pick up again in December.

@kartheekp-ms kartheekp-ms self-assigned this Nov 8, 2022
@kartheekp-ms kartheekp-ms removed their assignment Feb 6, 2023
kzu added a commit to devlooped/SponsorLinkCore that referenced this issue Mar 15, 2023
Each consuming package will use SL as a private development dependency exclusively, but gets aids via its (authoring) targets to properly pack and ship the dependencies and run-time targets.

The targets need to be included from buildTransitive targets since analyzers are always included (see dotnet/sdk#1212) and transitive (see NuGet/Home#6279), so the build properties need to be present always.
kzu added a commit to devlooped/SponsorLinkCore that referenced this issue Mar 15, 2023
Each consuming package will use SL as a private development dependency exclusively, but gets aids via its (authoring) targets to properly pack and ship the dependencies and run-time targets.

The targets need to be included from buildTransitive targets since analyzers are always included (see dotnet/sdk#1212) and transitive (see NuGet/Home#6279), so the build properties need to be present always.
kzu added a commit to devlooped/SponsorLinkCore that referenced this issue Mar 15, 2023
Up to now, we were running the SL check even if the project did not have a direct dependency with SL or the sponsorable package. This is because analyzers are (by design, for now?) transitive and this can't even be stopped via NuGet (see dotnet/sdk#1212 and NuGet/Home#6279).

We now introduce a `transitive` setting (defaulting to false) that is combined with the package dependencies to detect the indirect-ness and if so, skip the check.

Note that unless all the targets are in place to surface this indirect-ness to the analyzer, the check *will* be performed. This shields against the scenario where users might tweak targets to try to avoid getting the check to run at all.
kzu added a commit to devlooped/SponsorLinkCore that referenced this issue Mar 15, 2023
Up to now, we were running the SL check even if the project did not have a direct dependency with SL or the sponsorable package. This is because analyzers are (by design, for now?) transitive and this can't even be stopped via NuGet (see dotnet/sdk#1212 and NuGet/Home#6279).

We now introduce a `transitive` setting (defaulting to false) that is combined with the package dependencies to detect the indirect-ness and if so, skip the check.

Note that unless all the targets are in place to surface this indirect-ness to the analyzer, the check *will* be performed. This shields against the scenario where users might tweak targets to try to avoid getting the check to run at all.
kzu added a commit to devlooped/SponsorLinkCore that referenced this issue Aug 10, 2023
Each consuming package will use SL as a private development dependency exclusively, but gets aids via its (authoring) targets to properly pack and ship the dependencies and run-time targets.

The targets need to be included from buildTransitive targets since analyzers are always included (see dotnet/sdk#1212) and transitive (see NuGet/Home#6279), so the build properties need to be present always.
kzu added a commit to devlooped/SponsorLinkCore that referenced this issue Aug 10, 2023
Up to now, we were running the SL check even if the project did not have a direct dependency with SL or the sponsorable package. This is because analyzers are (by design, for now?) transitive and this can't even be stopped via NuGet (see dotnet/sdk#1212 and NuGet/Home#6279).

We now introduce a `transitive` setting (defaulting to false) that is combined with the package dependencies to detect the indirect-ness and if so, skip the check.

Note that unless all the targets are in place to surface this indirect-ness to the analyzer, the check *will* be performed. This shields against the scenario where users might tweak targets to try to avoid getting the check to run at all.
kzu added a commit to devlooped/SponsorLink that referenced this issue Aug 10, 2023
Each consuming package will use SL as a private development dependency exclusively, but gets aids via its (authoring) targets to properly pack and ship the dependencies and run-time targets.

The targets need to be included from buildTransitive targets since analyzers are always included (see dotnet/sdk#1212) and transitive (see NuGet/Home#6279), so the build properties need to be present always.
kzu added a commit to devlooped/SponsorLink that referenced this issue Aug 10, 2023
Up to now, we were running the SL check even if the project did not have a direct dependency with SL or the sponsorable package. This is because analyzers are (by design, for now?) transitive and this can't even be stopped via NuGet (see dotnet/sdk#1212 and NuGet/Home#6279).

We now introduce a `transitive` setting (defaulting to false) that is combined with the package dependencies to detect the indirect-ness and if so, skip the check.

Note that unless all the targets are in place to surface this indirect-ness to the analyzer, the check *will* be performed. This shields against the scenario where users might tweak targets to try to avoid getting the check to run at all.
@MeikelLP
Copy link

Still unable to exclude analyzers after 7 years...

@Drmorph
Copy link

Drmorph commented Aug 8, 2024

if someone search a hack to be able to do something like ExcludeAssets=analyzers (in my case StyleCop.Analyzers) , i found an old tweet https://twitter.com/Nick_Craver/status/1173996405276467202?s=09 who help me a lot to do it , added this on my .csproj

  <Target Name="DisableStaticAnalysis" BeforeTargets="CoreCompile" Condition="$(RunStaticAnalysis) == '' OR $(RunStaticAnalysis) == 'false'">
    <ItemGroup>
      <Analyzer Remove="@(Analyzer)" Condition="%(Filename) == 'StyleCop.Analyzers'" />
    </ItemGroup>
  </Target>

@dominikjeske
Copy link

Any updates on this?

@davidebbo
Copy link

That seems like a really bad issue, because those analyzers end up being viral. In my scenario:

  • I have a Project Foo, which uses package UFX.Relay, which uses Package Nerdbank.Streams
  • Nerdbank.Streams depends on several Microsoft.VisualStudio.* analyzer packages
  • There are dozens of other projects in my solution that depend on Project Foo.

And the end result is that every single project in my solution gets those unwanted analyzers.

Is there a way to disable them globally in the solution? Doing it at project level is quite painful...

@AArnott
Copy link
Contributor

AArnott commented Oct 31, 2024

@davidebbo Yes, just add an .editorconfig file in the root of your solution and set the severity for each analyzer to None that you don't want to see.

@davidebbo
Copy link

davidebbo commented Nov 1, 2024

@AArnott ah yes, that's a much better solution, thanks!

e.g. (but you don't need to manually edit it if you do it through the designer):

dotnet_diagnostic.VSTHRD003.severity = none
dotnet_diagnostic.VSTHRD002.severity = none
dotnet_diagnostic.VSTHRD103.severity = none
dotnet_diagnostic.VSTHRD110.severity = none
dotnet_diagnostic.VSTHRD011.severity = none

@dominikjeske
Copy link

yes but this is not fixing the problem but rather covering up the problem - when someone is developing nuget package there should be readme with info - just add .editorconfig to hide warnings from analyzers you are not expecting :)

@davidebbo
Copy link

yes but this is not fixing the problem but rather covering up the problem - when someone is developing nuget package there should be readme with info - just add .editorconfig to hide warnings from analyzers you are not expecting :)

Of course, it's just a workaround. Fixing the underlying issue is still very much needed. This is just to unblock...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area:Analyzers Category:SeasonOfGiving https://devblogs.microsoft.com/nuget/nuget-season-of-giving/#season-of-giving Functionality:Restore Partner:DotNet Priority:2 Issues for the current backlog. Style:PackageReference Type:Feature
Projects
None yet
Development

No branches or pull requests