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

Package analyzers always included, ignoring exclude settings #1212

Open
natemcmaster opened this issue May 15, 2017 · 13 comments
Open

Package analyzers always included, ignoring exclude settings #1212

natemcmaster opened this issue May 15, 2017 · 13 comments
Milestone

Comments

@natemcmaster
Copy link
Contributor

NuGet provides the "ExcludeAssets" setting to allow excluding analyzers from packages. However, it appears the SDK still includes analyzers in the build even if excluded.

(cref https://docs.microsoft.com/en-us/nuget/consume-packages/package-references-in-project-files#controlling-dependency-assets)

Repro

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

  <PropertyGroup>
    <TargetFramework>netcoreapp2.0</TargetFramework>
  </PropertyGroup>

  <ItemGroup>
    <PackageReference Include="Microsoft.NET.Test.Sdk" Version="15.0.0" />
    <PackageReference Include="xunit" Version="2.2.0" />
    <PackageReference Include="xunit.runner.visualstudio" Version="2.2.0" />

    <PackageReference Include="xunit.analyzers" Version="0.1.0" ExcludeAssets="analyzers" />

  </ItemGroup>

</Project>
using System;                              
using Xunit;                               
                                           
namespace test1                            
{                                          
    public class UnitTest1                 
    {                                      
        [Fact]                             
        public void Test1(string data) // this line intentionally trips an error in xunit.analyzers...just to prove the analyzer is running
        {                                  
        }                                  
    }                                      
}                                          
dotnet restore
dotnet build

Expected
The analyzer in xunit.analzyers/0.1.0/analyzers/dotnet/cs/xunit.analyzers.dll should not have been passed to CSC, and build succeeds.

Result

Build FAILED.

UnitTest1.cs(9,21): error xUnit1001: Fact methods cannot have parameters [C:\tmp\test1\test1.csproj]
    0 Warning(s)
    1 Error(s)

Details
Using dotnet.exe 2.0.0-preview2-006067

project.assets.json file contains:

{
  // ...
  "libraries": {
    "xunit.analyzers/0.1.0": {
      "sha512": "wPHthUmM0vdhL3lrTlyxJFfTgGh+2bkOn5CuxgZaPEMxLJArMCvUx8WmsVFeIATLVulVGvwNnZFkuwc0lsun7g==",
      "type": "package",
      "path": "xunit.analyzers/0.1.0",
      "files": [
        "analyzers/dotnet/cs/xunit.analyzers.dll",
        "tools/install.ps1",
        "tools/uninstall.ps1",
        "xunit.analyzers.0.1.0.nupkg.sha512",
        "xunit.analyzers.nuspec"
      ]
    }
  },
  // ...
  "project": {
    "frameworks": {
      "netcoreapp2.0": {
        "dependencies": {
          // ...
          "xunit.runner.visualstudio": {
            "target": "Package",
            "version": "2.2.0"
          },
          "xunit.analyzers": {
            "include": "Runtime, Compile, Build, Native, ContentFiles",
            "target": "Package",
            "version": "0.1.0"
          },
          // ...
        }
      }
    }
  }
}
@natemcmaster
Copy link
Contributor Author

Ping @nguerrera. Still happening in 2.0.0-preview3-006609

cc @rrelyea

@natemcmaster
Copy link
Contributor Author

cc @mikeharder

@Cryowatt
Copy link

Cryowatt commented Oct 5, 2017

This issue also occurs when a project includes a package that excludes analyzes from it's .nuspec. The solution ends up including the nested dependency analyzers and I get IDE1002 errors during the build.

@nguerrera
Copy link
Contributor

Blocked by NuGet/Home#6279

@aidapsibr
Copy link

I'm encountering this in a conversion to the new SDK style projects. The change in behavior of transitive dependencies means that a single package with an analyzer is now breaking our builds as that analyzer is enforcing itself virally. When coupled with warn as error, this is a difficult problem to work around.

@davkean
Copy link
Member

davkean commented Dec 20, 2019

This is hit/referred to in these other issues:

dotnet/aspnetcore#11935
dotnet/efcore#18618
dotnet/roslyn#26222
Particular/NServiceBus.AmazonSQS#185

@nguerrera
Copy link
Contributor

Still blocked by NuGet/Home#6279

mmitche pushed a commit to mmitche/sdk that referenced this issue Jun 5, 2020
…113.2 (dotnet#1212)

- Microsoft.DotNet.Arcade.Sdk - 5.0.0-beta.20063.2
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.
@EdLichtman
Copy link

I'm curious about what's going on with this. Right now I'm building analyzers and generators for our company and even when I exclude them they throw an error because the project that is consuming them isn't configured to use the analyzers

@kzu
Copy link
Contributor

kzu commented Aug 28, 2023

@EdLichtman you can sort of work around it by using buildTransitive MSBuild targets that provide the "configuration" your analyzers need.

@EdLichtman
Copy link

That is indeed a workaround but it hides the dependency from the target who references it. It's an example of a side effect and a very bad side effect because what if the analyzer it downloads needs to go through legal? In my case I'm writing the analyzer but I'm curious what is the status on when this will be fixed?

@EdLichtman
Copy link

Also btw, @kzu I was wondering what best practices for adding buildTransitive are...

Let's say I have a library: My.Library
Now, I have:

  • build/My.Library.props
  • buildTransitive/My.Library.props

When I have both of those, only the buildTransitive seems to get referenced. Therefore what I've resorted to doing is:

Adding the following to buildTransitive:

Is there a better way of doing it?

@kzu
Copy link
Contributor

kzu commented Aug 30, 2023

If analyzers need anything from MSBuild, you must use buildTransitive. And from there you can import a non-transitive shared targets, if needed.

@MeikelLP
Copy link

I really need a fix for this. refit is breaking my code because of this
reactiveui/refit#1764

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

No branches or pull requests

10 participants