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

References to this package should not carry forward as transitive dependencies #152

Closed
julealgon opened this issue Dec 17, 2020 · 4 comments · Fixed by #155
Closed

References to this package should not carry forward as transitive dependencies #152

julealgon opened this issue Dec 17, 2020 · 4 comments · Fixed by #155
Milestone

Comments

@julealgon
Copy link

When creating a NuGet package based on a project that contains a reference to NSubstitute.Analyzers, the package ends up carrying it as a dependency:

Section extracted from NuGet Package Manager:
image

This is undesirable since the analyzers are not part of the actual code being developed and instead, is used at development time only.

I propose this package is configured as a development dependency so that it does not carry over as a transitive dependency when other packages are generated based on it.

My specific use case for this is that I'm creating some reusable test classes in a shared package and those classes rely on NSubstitute internally (thus why I'm using the analyzer with it).

@tpodolak
Copy link
Member

Hi @julealgon
I am not sure if development dependency is a way to go here. I could imagine that some people would like to carry this dependency to the package (especially if this is internal testing package shared across different teams in organization).
When it comes to your particular problem, how about marking NSubstitute.Analyzers reference as private asset?

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

    <PropertyGroup>
        <OutputType>Exe</OutputType>
        <TargetFramework>netcoreapp3.1</TargetFramework>
    </PropertyGroup>

    <ItemGroup>
        <PackageReference Include="NSubstitute" Version="4.2.2" />
        <PackageReference Include="NSubstitute.Analyzers.CSharp" Version="1.0.14" PrivateAssets="all" />
    </ItemGroup>
</Project>

Now when you do dotnet pack it wont endp up as dependency

 <?xml version="1.0" encoding="utf-8"?>
 <package xmlns="http://schemas.microsoft.com/packaging/2013/05/nuspec.xsd">
   <metadata>
     <id>ConsoleApp1</id>
     <version>1.0.0</version>
     <authors>ConsoleApp1</authors>
     <requireLicenseAcceptance>false</requireLicenseAcceptance>
     <description>Package Description</description>
     <dependencies>
       <group targetFramework=".NETCoreApp3.1">
         <dependency id="NSubstitute" version="4.2.2" exclude="Build,Analyzers" />
       </group>
     </dependencies>
   </metadata>
 </package>

@julealgon
Copy link
Author

I could imagine that some people would like to carry this dependency to the package (especially if this is internal testing package shared across different teams in organization).

Sure, some people would probably want that. My point is that doing it out of the gate is not a sensible default for analyzer packages. The code in the main package does not in any way depend on the analyzer to execute, since the analyzer only has impact at design time. Additionally, forcing the analyzer on a consumer would potentially result in several compilation warnings (or worse, errors if warnings are treated as errors) in the consumer after the package is consumed.

Note that this is how most all other analyzer packages work today. See Microsoft standard analyzers, StyleCop, etc.

This is also the case for packages that modify the build in any way, as that is not a runtime concern and so marked as development dependencies.

For those who would want to force the analyzer to go along with their package, they could configure the csproj accordingly to change the standard behavior.

When it comes to your particular problem, how about marking NSubstitute.Analyzers reference as private asset?

I'm aware of that option, i just think it is not a good default for analyzers as mentioned above.

I kindly ask you to reconsider the approach here.

@tpodolak
Copy link
Member

Sure, some people would probably want that. My point is that doing it out of the gate is not a sensible default for analyzer packages. The code in the main package does not in any way depend on the analyzer to execute, since the analyzer only has impact at design time. Additionally, forcing the analyzer on a consumer would potentially result in several compilation warnings (or worse, errors if warnings are treated as errors) in the consumer after the package is consumed.
Note that this is how most all other analyzer packages work today. See Microsoft standard analyzers, StyleCop, etc.

@julealgon thanks for further explanation. It seems reasonable to make NSubstitute.Analyzers dev dependency by default. @dtchepak are you ok with that?

@dtchepak
Copy link
Member

@tpodolak Yes it sounds good to me 👍

tpodolak added a commit that referenced this issue Jan 12, 2021
…rs.VisualBasic as development dependencies by default
tpodolak added a commit that referenced this issue Jan 20, 2021
…rs.VisualBasic as development dependencies by default (#155)
@tpodolak tpodolak added this to the 1.0.15 milestone Nov 12, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants