Suppressing Analyzer code IDE1003 (Dependency check)

[juliosaenz]

Version Used: VisualStudio 14.0.25431.01 Update 3 Steps to Reproduce:

1. Create a analyzer with assembly dependencies.
2. make a vsix package witch excludes these dependencies (the dependencies excluded should not be need at runtime)
3. load the analyzer in VS.

Expected Behavior: since the analyzer dependencies that were ecluded are not needed at runtime the analyzer and VisualStudio should work without a problem.

Actual Behavior: Visual studio show error "IDE1003". Also these errors do not go away meaning they are not cleared or re-calc.

the request is to either make the diagnostic a info level, or to remove the check, or to add support to suppress the it, or add an exclusion list. the reason is

why is there such a check? thinning out the bulk of a package and selectively excluding assemblies is a common practice, in fact seems like you all are doing it too https://github.com/dotnet/roslyn/issues/7049 The fact that a assembly reference is missing is not an end all conclusive fact that the analyzer will fail. It may be an indicator but it is not a rule.

(Now if the analyzer runs and throws an exception then that’s another thing)

[jinujoseph]

Changing to Info is pretty trivial but @srivatsn/ @tmeschter any context on why this was set to Error?

[tmeschter]

@jinujoseph @srivatsn This is an error because in many cases it will represent an actual problem. It may be a common practice to drop assembly dependencies that you know aren't going to be needed at runtime, but that certainly wasn't a practice we anticipated when we designed analyzers.

@juliosaenz This is fundamentally different from #7049. We have to exclude Microsoft.CodeAnalysis.* assemblies from the dependency check because those are expected to be provided by the host running the analyzer; they should never be included with the analyzer.

[sharwell]

I tend to agree with this rule. The main problem I have with it is it fails to detect the one case of this which caused the most pain in the past - Microsoft.CodeAnalysis.Workspaces cannot be referenced from a build-time analyzer assembly which exports DiagnosticAnalyzer instances, but that assembly is incorrectly excluded from this analysis.

[tmeschter]

@sharwell Good catch. Could you file a separate issue about that?

[tmeschter]

Also, regarding making this an Info-level diagnostic rather than Warning or Error: that's effectively equivalent to removing the check entirely, given the low visibility of Info messages.

There are two things we should do to fix this:

1. Update the IDE to properly honor the suppression.
2. Provide a mechanism for analyzers to exclude certain dependencies from the analysis, that is, to say "yes, I know this assembly is missing, but it's OK because I know I don't need it at runtime".

The latter will, of course, require some design work.

[juliosaenz]

@tmeschter Great! this is exactly the ask :) ether one will work for me since the analyzer gets added by some custom targets in which I can suppress the error. Or if I can add the analyzer to a list I can do so there as well.

[sharwell]

@tmeschter Filed #20618

[sharwell]

@juliosaenz Note that if your analyzers are distributed publicly, it would be poor form to automatically disable IDE0003 when the analyzers are installed since it would remove this check from all analyzers and not just yours.

[juliosaenz]

@sharwell I see, it is not going to be publicly available, but I do agree that we should not throw the baby with the bath water. Can we go for the exception list option then? meaning a list of Analyzers that get except form this check. and of course a way for me to get into this list

[sharwell]

@jasonmalinowski If .editorconfig is visible at the point IDE0003 executes, we could have a configurable property of excluded assemblies.

[jasonmalinowski]

@sharwell I would assume we'd put metadata in the analyzer assembly itself or something else like that. I imagine .editorconfig as user configuration, not analyzer author configuration. Given our numerous forms of analyzer deployment, carrying it in the DLL itself seems reasonable to me.

[mavasani]

Update the IDE to properly honor the suppression.

IDE already honors the suppression for this diagnostic. However, we execute analyzer dependency checker only during project load, not for every compilation change, hence adding a suppression doesn't immediately take effect - you need to reload the project. Changing this behavior to run the dependency checker on every compilation is likely going to be a big perf hit.

Provide a mechanism for analyzers to exclude certain dependencies from the analysis, that is, to say "yes, I know this assembly is missing, but it's OK because I know I don't need it at runtime".

This is a new feature request. We are going to consider it for 15.5, but it purely depends on relative priority of this work versus other existing 15.5 feature work.

As Tom mentions, the diagnostic is actually very useful when analyzer authors actually miss out on packaging real analyzer dependencies, so making it an info is taking away its usefulness.

@juliosaenz Can you give details on your exact scenario that needs you to reference an assembly in your analyzer project that you don't need at runtime? If your scenario is very common and mainline, it will bump up the priority of these requests.

[juliosaenz]

Sure the scenario is that the analyzer uses a common library, this library has multiple dependencies. where the analyzer only uses certain functionality of said library. The dependencies of the dependencies of this library can get pretty bulky. And because I know this library and its dependencies, I have decided not to include the whole dependency graph in the analyzer package. And include only the few I know and have tested.

I want to do this for 3 reasons. 1.- The size of the package is greatly reduced. from about 100mb to about 50mb 2.- I don't want VS to load all of these extra dependencies that are just going to take up memory and will never be used (creating more useless memory usage). 3.- There may be reasons in the future to push this outside the company (other than just internal to us), in which case I definitely don't want to export code outside that is intended to be internal (meaning there may be dependencies that need to be kept internal while other may be ok to export).

@mavasani you said that the IDE already honors these suppressions. I have tried a couple of ways to suppress this and it has not worked. would you mind posting here a sample way to suppress this? would this go on a suppression file? while it shows on the IDE it certainty does not let me suppress it. it is ok to require a reload that is something I can live with and will be happy. if I can add a suppression on a suppression file, can add it to the template for out project, and the ones who have already created can add it themselves.

[sharwell]

@juliosaenz This suppression would be in a rule set file. Here's an example where I needed to disable one of the other IDE analyzers:

https://github.com/DotNetAnalyzers/StyleCopAnalyzers/blob/master/StyleCop.Analyzers/StyleCop.Analyzers.ruleset#L71

[PathogenDavid]

EDIT: It's come to my attention that my main issue (polluting the Visual Studio analyzers listing) might've been a bug in Visual Studio. (I deleted my .vs folder and it stopped happening. Tom brought it to my attention that I actually just switched between csproj formats, and it's a bug with the new format.) So my question is less important now, but I'd still like to know since it still feels like I'm adding a bunch of unnecessary <Analyzer> items when I'm not relying on a NuGet package to do it for me. Tom pointed out why this is important below, leaving this comment for posterity.

---

Is there a reason this analyzer can't ignore assemblies that are co-located with the analyzer? My understanding is that the binder will automatically load them anyway, so adding them as analyzer references is redundant.

---

For instance, say I have my file system structured like the following:

• <Solution root>
  • MySolution.sln
  • Analyzers
  • Monotone.dll
  • Monotone.Analyzer.dll
  • MyProject
  • MyProject.csproj

Monotone.Analyzer.dll references Monotone.dll

In my csproj, I reference the analyzer like so:

<ItemGroup>
  <Analyzer Include="$(SolutionDir)Analyzers\Monotone.Analyzer.dll" />
</ItemGroup>

This should be all that is necessary because the binder will pick up Monotone.dll automatically since Monotone.Analyzer.dll requests it.

I don't want to add these references because they aren't analyzers (they're just used by my analyzer) and doing so adds unnecessary clutter to the analyzers listing in the solution explorer.

[tmeschter]

@PathogenDavid What you're describing is really a different issue. Hiding analyzer assemblies that do not contain any actual analyzers is tracked by https://github.com/dotnet/project-system/issues/2724.

As for this:

This should be all that is necessary because the binder will pick up Monotone.dll automatically since Monotone.Analyzer.dll requests it.

The behavior of the binder is an implementation detail, and not something you should depend on. It would be perfectly valid for the compiler to fail if you did not specify Monotone.dll as an analyzer, regardless of where it is located in relation to Monotone.Analyzer.dll.

The reason is correctness. Say one day someone accidentally deletes Monotone.dll from next to Monotone.Analyzer.dll, but some other analyzer--from a NuGet package, for example--just happens to include a copy. Now whether your build passes or fails depends on the order in which the analyzers are run and the assemblies are loaded. So your build runs fine for a long time until one day it just breaks for no obvious reason.

Or perhaps worse, one day you add another analyzer that includes it's own copy of Monotone.dll with the same identity but different contents. Monotone.Analyzer.dll will now always run... but perhaps produces different errors from one run to the next depending on what copy of Monotone.dll is loaded. csc.exe, vbcscompiler.exe, and VS try to provide consistent behavior when these issues occur (e.g. make sure the build passes consistently with consistent errors, or fails consistently), but that can only work when you tell the compiler about all the inputs--including your analyzer's dependencies.

[PathogenDavid]

Thanks for the clarification, I definitely got a little too far into my own bubble there. Letting the binder pick up dependencies definitely isn't an good move here.

Also thanks for the issue link. I didn't notice until now that the dependencies going away was actually the result of me looking at an old csproj-style project instead of a new one.