Filter projects based on language name #19450
Conversation
Avoids [VSO bug 433873](https://devdiv.visualstudio.com/DefaultCollection/DevDiv/_workitems/edit/433873). When mapping an `IVsHierarchy` to the related Roslyn project we end up asking each project for its associated `IVsHierarchy` and checking if various properties match up with those on the "origin" hierarchy. The properties supported and the interpretation of their values varies from one project system to another. The `HierarchyItemToProjectIdMap` class was implemented when Roslyn only supported C# and VB projects, and was designed with those in mind. Now that TypeScript can also have Roslyn projects we need to filter out everything that isn't C# or VB or we risk asking for properties that aren't supported or possibly misinterpret the values.
|
@dotnet/project-system Please review. |
|
@dotnet/roslyn-ide Please review. |
| @@ -47,6 +46,16 @@ public bool TryGetProjectId(IVsHierarchyItem hierarchyItem, string targetFramewo | |||
| var project = _workspace.DeferredState.ProjectTracker.ImmutableProjects | |||
| .Where(p => | |||
| { | |||
| // We're about to access various properties of the IVsHierarchy associated with the project. | |||
There was a problem hiding this comment.
who ends up using this API? i.e. what features need this, and how do we know they're only for C# or VB?
There was a problem hiding this comment.
This is needed for matching nodes in Solution Explorer to their corresponding Roslyn project. This is used to determine if we should show the Analyzers folder under the References node, and also in associating nodes for analyzers to the proper AnalyzerReference.
To my knowledge those are only meaningful in C# and VB.
There was a problem hiding this comment.
Ah ok. What about F#? Do they not have analyzers?
There was a problem hiding this comment.
They do, but with a different implementation, and project system support is a WIP for them. Right now they aren't using this code path and this change won't affect them. I talked with @KevinRansom and we'll make changes here to explicitly support F# if and when necessary.
|
@dotnet-bot test windows_release_vs-integration_prtest please |
|
Adding @MattGertz for approval. |
When mapping an
IVsHierarchyto the related Roslyn project we end up asking each project for its associatedIVsHierarchyand checking if various properties match up with those on the "origin" hierarchy. The properties supported and the interpretation of their values varies from one project system to another. TheHierarchyItemToProjectIdMapclass was implemented when Roslyn only supported C# and VB projects, and was designed with those in mind. Now that TypeScript can also have Roslyn projects we need to filter out everything that isn't C# or VB or we risk asking for properties that aren't supported or possibly misinterpreting the values. In bug bug 433873, for example, it appears that TypeScript returnsnullfor one of the properties, and we just aren't expecting that.Note that in theory this change shouldn't be necessary--the properties we check are common and normally I would say their interpretation is unambiguous. But rather than play whack-a-mole with bugs I'd rather just limit us to the scenarios we need to support.
Customer scenario
When working in a solution containing a TypeScript project Visual Studio just up and crashes. Based on the crashes I've seen this most likely happens when the user is traversing nodes in Solution Explorer, but there could be other scenarios as well.
Bugs this fixes:
Works around VSO bug 433873.
Workarounds, if any
None.
Risk
Low. This code path is part of our support for showing analyzer nodes in C# and VB projects in Solution Explorer; the change just makes it explicit that only C# and VB are supported.
Performance impact
Low.
Is this a regression from a previous update?
Yes.
Root cause analysis:
When this code path was first written C# and VB were the only languages we needed to consider, and so it basically assumes it will only encounter C# and VB projects. When TypeScript started to utilize Roslyn we basically got lucky that this didn't break. For Dev15 this code has been refactored to support both the old and new project systems, and in doing so several bugs were exposed.
How was the bug found?
Dogfooding