Roslyn: Disable analyzers (and other IDE features?) on .cshtml and .aspx files

[dpoeschl]

As of VS 2017 RC 2, you can get (for example) Naming Style suggestions in the codebehind of .cshtml pages that are unfixable. Since we don't show lightbulbs on .cshtml and .aspx pages, it may be okay to just disable analyzers on those files completely.

1. When installing VS, pick the ".NET desktop development" and ".NET Core and Docker (Preview)" workloads
2. File | New | Project | Templates | Visual C# | Web | ASP.NET Core Web Application (.NET Core)
3. Choose "Web Application"
4. Open _ViewImports in the Views folder

[dpoeschl]

To fix this for analyzers, look here: http://source.roslyn.io/#Microsoft.CodeAnalysis/DiagnosticAnalyzer/AnalyzerDriver.GeneratedCodeUtilities.cs,352ce1a834ab33b9,references

To be consistent in other IDE features, consider also updating http://source.roslyn.io/#Microsoft.CodeAnalysis.Workspaces/GeneratedCodeRecognition/GeneratedCodeRecognitionServiceFactory.cs,21 after investigating what all depends on that.

[CyrusNajmabadi]

I'm not sure this is what we want to do. Shouldn't you just check if the location of the declaration is visible or not?

[Eilon]

I don't think it makes sense to disable analyzers completely in CSHTML files. CSHTML files can contain user-written methods, classes, properties, etc. and it could be valuable to have style analyzer rules run on them.

However, because CSHTML files go through their own parser and contain a mix of C# and HTML, the rules of how to work within them are a bit trickier.

Somehow checking if the code is "visible" makes a lot more sense.

[DamianEdwards]

Yep, we really want analyzers to just consider projected code from ASPX and CSHTML files, so that the user's own code is included in what's analyzed. Do these operate in a context such that knowing what code is projected into a visible buffer is determinable?

[CyrusNajmabadi]

@mavasani @heejaechang @srivatsn

To me, this sounds like something we'd want in the core code analysis system (similar to how we have knobs to control if we analyze generated code). It seems like we'd like to opt out of analyzing 'hidden' code entirely. We have the "GetLineVisibility" helper on syntax trees to help the individual analzyer (or driver) to know if the code is visible.

[CyrusNajmabadi]

What do you guys think? If we don't push this to the driver, then everyone will need to manually opt out. Given that even reporting issues about hidden code is suspect (since hte user can't navigate to it to fix the issue), it feels like we should act as if this code simply doesn't exist, and not analyze it.

[heejaechang]

@mavasani shouldn't those files part of generated files thing?

[CyrusNajmabadi]

IMO, that's likely how it should work. However, it def adds complexity. The current solution just needs to look at the extension, or the first bit of trivia in the file. Now we'll need to look at every node to see if it's in a hidden region or not.

[heejaechang]

@mavasani engine ignoring diagnostics reported from hiddne region seems good idea. engine probably still need to run analyzers to gather information, but just filter out diagnostics reported for the region. and same hidden region information re-used from compilation end analyzers.

[mavasani]

@CyrusNajmabadi I am not worried about the implementation - we can just invoke this API before calling into analyzers for syntax nodes/operations. I am only concerned about performance aspect as we are doing this additional work for every single node being analyzed. @srivatsn does this seem fine to you? I can implement the visible region check in the driver.

[srivatsn]

Ya perf is the concern - can we do some measurements to validate?

[Pilchie]

Moving, since we think the right place to fix this is in the driver.

[heejaechang]

what milestone are we shooting for?

[mavasani]

We discussed this issue offline and making a change to the analyzer driver here seems risky, both functionality and performance wise. We will instead change the IDE analyzers to explicitly opt out of the specific file types for RTW. @CyrusNajmabadi. I am moving this bug to you for that change. Please re-assign back to me as active so that post RTW, I can revert that change and add hidden region checks in the analyzer driver.

[jrmoreno1]

Does anyone remember what happen with this? I was having an analyzer I wrote showing warnings (correctly) for the aspx page, but was unable to apply the fix.

[Eilon]

@jrmoreno1 this is a pretty old issue by now so I recommend opening a new issue with details of the behavior you're seeing, what you expect should have happened, and link back to this bug. That should help get more traction.