Roslyn: Surface unused referenced assemblies to user as diagnostics

[gafter]

Someone wrote:

Over-specifying dependencies can lead to bottlenecks within our build schedule, reduced parallelism, and longer tool runtimes.

The native linker has the /VERBOSE:UNUSEDLIBS option which will flag superfluous libs giving us the necessary clue to have us remove the unneeded dependency from the build spec. The managed compilers lack this similar ability and so we can’t readily track extraneous dependencies. I’ve tried in the past to write an FxCop rule to detect mismatches between what the C# compiler’s /r options and what the final IL references, but alas this broke around the use of constants (which are inlined from an input assembly and thus the IL doesn’t hold the reference to the assembly where the constants came from).

Within another project, we extended the C# compiler to report a warning for unused assembly references. It took one of our devs only a few hours to put this feature in. We found literally thousands of superfluous assembly references within our tree as a result and then proceeded to clean things up.

Would it be possible to squeeze this feature into the C# compiler? I think this could help perf and could help bring architectural clarity.

[gafter]

Someone else wrote:

The method used in our version of the C# compiler was pretty straight forward. It simply hooks into type name binding and member import. Those are good choke point for “used” types and at those times we just mark the referenced assembly as used. Anything not used at the end of compliation is unused and we issue a warning.

Note that this is not a fool proof method and in certain cases it will believe assemblies are used when in fact they are not. This is due in part to how features like extension methods work. The cases that come up are fairly esoteric though and as Martin said even with the under reporting we still found, and continue to find, many cases of extraneous references in the system.

[gafter]

@MattGertz You were on the original thread for this issue.
/cc @AnthonyDGreen

[tmat]

I don't think this should be implemented in the compiler for the following reason.

The compiler can't avoid reporting warnings that are not actionable for modern apps (CoreClr, Windows Store, Universal). Such apps reference large sets of contract assemblies (e.g. portable profile, nuget sets, etc.) and virtually all apps use only a small subset of them. But the references are bundled together as a Framework in the project system (.csproj/.vbproj). The user can't easily choose a subset unless they want to manually manage these references, which is a hassle and nobody wants to (should) do that.

To accomplish this task a tool can be written that reads AssemblyRef table from the resulting binary and compares it with references of the project. Such tool can understand various Framework contracts and bundles and not report issues for those references.

[gafter]

@tmat read the original issue for a description of why your suggested approach does not work. Constants are inlined by the compiler and the IL contains no hint of the compile-time dependency.

[tmat]

In that case we should provide API that enables to retrieve that information from the compilation. But we should not issue warnings.

[AnthonyDGreen]

To be consistent with the approach we took for unused Imports/using directives we should surface this as hidden "info" diagnostics rather than warnings. Then the consumer/IDE can make a decision to take action based on other context and would address Tomas's concerns, I think. Maybe we could fade those references in the Solution Explorer.

VB actually has a command in the project properties to prune unused References and this facility would probably help lighting that up. Adding @DustinCampbell and @Pilchie for IDE feedback. Dustin/Kevin is the "Remove Unused References" command implemented already?

[DustinCampbell]

Actually, we are planning on removing this command from the VB References property page. We haven't implemented it yet (I don't know that we actually could without information from the compiler), and we also haven't heard a single report about it not working in VS 2015.

[jaredpar]

I don't think this makes sense as a feature inside the compiler for a couple of reasons:

1. It's easy to get 95% of the way done but that last 5% requires a lot of work. We probably don't have enough time remaining in VS 2015 to get the full 100% and I don't think this is good as a partially done feature. It will end up giving false positives in the most complex projects.
2. There are actually legitimate cases where projects intentionally reference projects they don't actually use in code. One example is to ensure the referenced project is put into the output directory of the build. Roslyn itself does this in a couple of places.

I do think this makes a ton of sense as an analyzer though. This is a diagnostic that doesn't promote type safety across assemblies, it simply protects me from me within an assembly. Perfect scenario for an analyzer.

If the analyzer does prove to be very popular it's something we could consider moving into the core compiler in a future version.

[Pilchie]

You could argue the same thing about the diagnostic for unnecessary usings, but it is in the compiler, simply because it is almost impossible to implement outside the compiler. I think you would find the same thing here. There would be no way for an analyzer to implement the diagnostic correctly.

[jaredpar]

@Pilchie I agree that the analyzer does need the same hook as the unused using feature does. I don't think it needs any additional hooks though.

Do we not expose that hook yet?

[DustinCampbell]

@Pilchie's right. That's what I meant when I mentioned that we probably couldn't implement the legacy VB Remove Unused Usings without an additional Compiler API. The information about which references are used is just not exposed and we wouldn't be able to do it correctly at a higher layer.

@jaredpar There isn't a hook for unused usings. That is implemented as a diagnostic in the compiler as Anthony mentioned. We had an API at one point but it was weird because it served a single feature.

[Pilchie]

We don't expose a hook, the compiler reports the diagnostic directly. We tried a hook approach, but it wasn't anything general purpose, there was just a SemanticModel.GetUnusedUsings() method.

What would you propose the hook be?

[gafter]

@Jaredpar the hook for unused usings is a diagnostic, which is what I would recommend for this.

This in not proposed for VS2015, but for some later update or release.

[jaredpar]

@Pilchie I'm not familiar enough with the particulars of the unused using rule so I can't detail the specifics. But given that we keep trying to shove these features into the compiler it smells of a missing API here.

@gafter I agree that it didn't make sense to have a general purpose API when there was a single consumer of the information that unused usings consumed. This feature though needs roughly the same information for correctness. Hence at this point I think it would make sense to expose as an API vs. another use specific hook in the compiler.

edit clarified wording

[DustinCampbell]

Hey guys -- this is not something we can ever get right. References are not only present for compilation, but also for runtime (for example, dependency injection). In the world of NuGet packages there are potentially _many_ references added to a project that are not used for compilation, but removing them would break the user's program. I can't see us adding this feature in either the compiler or the IDE without a lot of additional tooling.

[tmat]

+1 for what @DustinCampbell says.

If there is a need to capture the references that the compiler embeds metadata from (constants, NoPIA types) we could add that information to PDBs. PDBs in general contain source information useful for tools (not just debuggers) that is otherwise lost during compilation.

[gafter]

@DustinCampbell There is no IDE request here. This request is from someone who wants to do the tooling in a build system. But he can't do that without the compiler feature. We don't need to do both the compiler feature and the tooling. We can do the compiler feature and let others do the tooling if that is what they want. Reopening.

[gafter]

I don't feel strongly whether this needs to be an API, a diagnostic, extra info in PDBs, whatever. But only the compiler can do it and get it right.

[gafter]

@tmat do we generate PDBs when not compiling for debug? If not I don't think that is the right place to put the data.

[DustinCampbell]

@gafter I was not referring to the IDE request. This feature can't be implemented correctly by the compiler either. Such a feature would still report lots of false positives for binaries that are intended to be used at runtime but not compile time. I can't see this being a viable feature.

[DustinCampbell]

@gafter Consider the case where a contract assembly is used for compilation and an implementation assembly is referenced simply to deploy it next to an app. This feature would report the implementation as unused, giving a false positive.

[gafter]

@DustinCampbell I agree it would be possible to use this feature in an incorrect way. One correct way to use the information would be to limit the set of referenced assemblies given to the compiler at build time to improve compile time and to remove false compile-time dependencies. Removing them from the command line has no effect on the behavior of the compiler (except to make it faster). That is the use for which the original requester intends to apply this feature.

[tmat]

@gafter Yes we do generate PDB for both Debug and Release if we are asked for it.

[gafter]

@tmat I think it would be a misuse of PDBs to require their generation to track the referenced assemblies.

[eatdrinksleepcode]

@DustinCampbell I am not sure why you think this can't be "done right". The compiler CAN accurately report whether or not a particular reference was used during compilation. Now, acting naively on that information by removing the reference would often be the wrong thing to do; but no one is suggesting that. Instead, tools in the chain that depend on the compiler can consume this information and take intelligent action.

All of the potential "false positives" brought up so far can be detected as such by an intelligent tool (outside of the compiler):

1. A build tool could detect when an unused assembly is part of a Nuget package (or other bundle) that contains a used assembly.
2. A build tool that has something like Gradle's "runtime" configuration could detect when an unused assembly will be used at runtime instead of compile time (even better, such a tool could prevent the reference from being passed to the compiler in the first place).

I think this is a good idea and should be implemented, if not by a warning than by some appropriate mechanism (I don't think PDB is appropriate). I have encountered the situation described the OP many times and would like a way to alleviate it.

[DustinCampbell]

@eatdrinksleepcode: To be clear, I am _not_ disputing that the compiler can't accurately report whether or not a particular reference is used during compilation. It clearly can do that

You're absolutely right that it's possible to have tools ahead of the compiler in the build chain to help remove runtime dependencies. However, if those tools rely on static analysis, there will always be false positives. Automatic detection of whether a reference gets used at runtime can't ever be perfect, but we can probably get 95% of the way there.

FWIW, @gafter and I chatted about this in person today and we're in agreement on this thread.

[eatdrinksleepcode]

However, if those tools rely on static analysis, there will always be false positives.

I was not suggesting static analysis or automatic detection of runtime-only dependencies. References that are only needed at runtime can be marked as such in the build configuration. This is how both Maven and Gradle handle this situation.

[DustinCampbell]

@eatdrinksleepcode Sorry, I misunderstood your text "a build tool could detect". Sounds like we're in agreement then. :smile:

[ghost]

Is it planned for next major version? FWIW, community interest can be found here: https://stackoverflow.com/questions/3157066/removing-all-unused-references-from-a-project-in-visual-studio-projects.

[dfederm]

I've been working on related tooling (https://github.com/dfederm/ReferenceTrimmer) and my approach is to inspect the produced dll to see what assembly references actually made it into the dll. This feature would cut down on false positives a ton though.

Note that as discussed by others already, tooling such as this will need to be smarter than just blindly removing all refs unused during compilation. Overly simplified, you actually can do that for dlls, but for exes it'd be much safer to collect all the used references transitively for all your dependencies, as those may be needed at runtime (ie, indirect dependencies). NuGet also complicates things, but again, tooling can be smart about it while the compiler would just provide extra data to make the tooling more smart.

[MisinformedDNA]

If the results can't be completely correct for compiler warnings, we could all still benefit from a Roslyn analyzer or a separate VS tool that reports what it can (i.e. "These references _may not_ be in use"). ReSharper has a feature to remove unused resources, and while not perfect, still provides value.

NOTE: I inherited a .NET Framework project with 300+ references (due to many legacy changes and NuGet recursion). _Anything_ that helps me decrease that number is welcome. :)