Package Analysis Warnings: Packages' assembly level dependencies should match the declared dependencies

[nkolev92]

Today it's possible that a package unwittingly takes an API dependency on transitive package and doesn't account for that when expressing it's dependencies.

dotnet/core#2571 is such an example.

@ericstj commented

So the problem here is that the ClassLibrary1 is relying on the transitive dependency from ClassLibrary2 to deliver SQL client.

ClassLibrary2 is actually malformed to remove compile-time dependencies from a compatible target framework. A library need to consider both the API it exposes, and any packages it depends on as part of its public contract. This is the reason why we are very careful in CoreFx to choose which packages we expose to the compiler vs just reference for runtime consumption.

We had a similar problem in CoreFx years back and specifically made our packaging infra catch this and promote those dependencies. Here's that commit: dotnet/buildtools@98119a0. Now the code lives in arcade and has gone through a few iterations to handle other cases but it still addresses this type of issue.

NuGet could do a couple things here:

1. Add validation whenever you create a dependency group that is compatible with some other dependency group and isn't a super set of that compatible depedendency group (perhaps only considering dependencies that are not Exclude="Compile". This would fix ClassLibrary2 by making the author change their PakcageReference conditions.
2. Add validation for a package that directly references some library: it's resultant assembly has an assembly reference to library X that came from package X but it didn't directly reference package X. This would fix ClassLibrary1 by making the author change the project to directy reference all packages he directly uses in his assembly, effectively insulating him from cases like ClassLibrary2,

@nkolev92 commented

So the problem here is that the ClassLibrary1 is relying on the transitive dependency from ClassLibrary2 to deliver SQL client.

ClassLibrary2 is actually malformed to remove compile-time dependencies from a compatible target framework. A library need to consider both the API it exposes, and any packages it depends on as part of its public contract. This is the reason why we are very careful in CoreFx to choose which packages we expose to the compiler vs just reference for runtime consumption.

We had a similar problem in CoreFx years back and specifically made our packaging infra catch this and promote those dependencies. Here's that commit: dotnet/buildtools@98119a0. Now the code lives in arcade and has gone through a few iterations to handle other cases but it still addresses this type of issue.

NuGet could do a couple things here:

1. Add validation whenever you create a dependency group that is compatible with some other dependency group and isn't a super set of that compatible depedendency group (perhaps only considering dependencies that are not Exclude="Compile". This would fix ClassLibrary2 by making the author change their PakcageReference conditions.
2. Add validation for a package that directly references some library: it's resultant assembly has an assembly reference to library X that came from package X but it didn't directly reference package X. This would fix ClassLibrary1 by making the author change the project to directy reference all packages he directly uses in his assembly, effectively insulating him from cases like ClassLibrary2,

A library need to consider both the API it exposes, and any packages it depends on as part of its public contract

I've seen issues like this quite often, and there are some related asks from customers wanting to avoid taking compile time dependencies on transitively delivered libraries.

Related issue: https://github.com/NuGet/Home/issues/6720|

1. Add validation whenever you create a dependency group that is compatible with some other dependency group and isn't a super set of that compatible depedendency group (perhaps only considering dependencies that are not Exclude="Compile". This would fix ClassLibrary2 by making the author change their PakcageReference conditions.

I like this but it might raise too many warnings. Especially in the .NET Framework/.NET Standard2.0 multi targeting worlds.
See https://www.nuget.org/packages/NuGet.Protocol/5.3.0 for example.

1. Add validation for a package that directly references some library: it's resultant assembly has an assembly reference to library X that came from package X but it didn't directly reference package X. This would fix ClassLibrary1 by making the author change the project to directy reference all packages he directly uses in his assembly, effectively insulating him from cases like ClassLibrary2,

NuGet doesn't spy on the assemblies today, but I'd be excited to explore what we can do here.
It'll frustrate devs initially but it can do wonders for the eco-system.

I'll move some of this ideas to NuGet/Home and we can continue the discussion there.

[nkolev92]

We also need to consider that some library authors avoid declaring all their dependencies in all the projects (especially services, that basically package for internal purposes), and rely on transitivity to get needed assembly.

We could argue this is not the best practice, but I'd imagine it's super common.