-
Notifications
You must be signed in to change notification settings - Fork 510
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
Violations of SA1113 are not reported in the MSBuild output #1659
Comments
@sharwell Any chance the analyzer is failing to load with an AD0001 diagnostic? You might have to bump up the MSBuild logging verbosity and then search the build log to find it. |
|
@sharwell That would be a definitive "yes". Your analyzer appears to have a dependency on Microsoft.CodeAnalysis.Workspaces, which isn't part of the compilers. It is part of VS and is almost certainly being copied over for your unit test runs, which is why the analyzers load and run there. Unfortunately a quick perusal of the code doesn't reveal any obvious dependencies. |
@tmeschter The analyzer in question referenced a static member of a type which was derived from 📝 This design is a disaster for the way analyzers and code fixes are currently structured in samples and now in various projects. The only way to be certain at compile time that analyzers will work is to place all analyzers in one assembly (which does not reference Microsoft.CodeAnalysis.Workspaces), and all code fixes for those analyzers in a different assembly. |
Hi Sam, Has this been fixed? I'm still seeing AD0001 errors when compiling using "dotnet build" but not seeing those errors in Visual Studio. Some of the analysers must be loading OK and appear to be working because I am seeing violations of SA1505 which is good, but I am seeing AD0001 for the SpecialRules and DocumentationRules analyzers.
|
@mattfrear That's a different error, related to the use of StyleCop Analyzers with .NET Core. We've had several challenges with stabilizing behavior for users working with that toolchain. |
How crazy would I be to try to bundle Microsoft.CodeAnalysis.Workspaces.dll in the nupkg for CSC? My analyzer needs to run logic in Workspaces in order to determine whether to report a diagnostic, so it's either that or copy and paste the logic. |
@jnm2 There are both design an implementation issue with that idea. From a design point of view analyzers are meant to sit on top of the Compiler layer, not the Workspaces layer. By taking a dependency on MS.CA.Workspaces.dll you're working against the intended layering. As for the implementation issues, a lot of the hosts (VS, csc, vbc, vbcscompiler, etc.) are going to have binding redirects to force their version of MS.CA.Workspaces.dll, so the one you include might never be loaded. Or worse, yours gets loaded instead of the one provided by the host and you've potentially broken other things. Or maybe both get loaded, and someone MEF imports the "same" type from both assemblies and, well, who knows. Also, including MS.CS.Workspace.dll probably requires you also include a matching copy of Microsoft.CodeAnalysis.dll, and then maybe also System.Collections.Immutable.dll, and perhaps others, all of which will be subject to similar problems. So instead of bundling MS.CA.Workspaces.dll, I suggest attacking the root issue: what functionality do you need from it, and why? |
The alternative is being blocked on dotnet/roslyn#23842, if you move that logic at all, or copying and pasting that logic into my analyzers. |
I think your best bet in the short term is to copy that code to your own project. |
Okay. |
See #1658 for a clear example. The pull request introduces two violations of SA1113, in addition to violations of SA1116 and SA1117. When you open the solution in Visual Studio, all of these warnings are reported in the Error List window. When you build the project, only SA1116 and SA1117 are included in the build output (see AppVeyor for example).
📝 SA1113 is implemented in SA1113CommaMustBeOnSameLineAsPreviousParameter.
The text was updated successfully, but these errors were encountered: