dotnet / sdk

Core functionality needed to create .NET Core projects, that is shared between Visual Studio and CLI
https://dot.net/core
MIT License
2.68k stars 1.06k forks source link

Package analyzers always included, ignoring exclude settings #1212

Open natemcmaster opened 7 years ago

natemcmaster commented 7 years ago

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 commented 7 years ago

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

cc @rrelyea

natemcmaster commented 7 years ago

cc @mikeharder

Cryowatt commented 7 years ago

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 commented 6 years ago

Blocked by https://github.com/NuGet/Home/issues/6279

aidapsibr commented 4 years ago

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 commented 4 years ago

This is hit/referred to in these other issues:

https://github.com/aspnet/AspNetCore/issues/11935 https://github.com/aspnet/EntityFrameworkCore/issues/18618 https://github.com/dotnet/roslyn/issues/26222 https://github.com/Particular/NServiceBus.AmazonSQS/pull/185

nguerrera commented 4 years ago

Still blocked by NuGet/Home#6279

EdLichtman commented 1 year ago

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 commented 1 year ago

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

EdLichtman commented 1 year ago

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 commented 1 year ago

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:

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 commented 1 year ago

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

MeikelLP commented 2 months ago

I really need a fix for this. refit is breaking my code because of this https://github.com/reactiveui/refit/discussions/1764