Change progression providers to use capabilities instead of IntelliSe…nse type. This enables them to work on both .Net core projects and legacy projects. #15935

RaulPerez1 · 2016-12-15T20:07:08Z

Customer scenario

.Net core projects don't implement IntelliSense type so progression features don't work in those (i.e. seeing the types in a file in solution explorer)

Bugs this fixes:

dotnet/project-system#592

Workarounds, if any

No workarounds

Risk

Low. No actual changes to the code here other than move the provider to use capabilities instead of IntelliSense type on the MEF export.

Performance impact

Low - See above for details.

Is this a regression from a previous update?

Not a regression.

Root cause analysis:

Not a bug in Roslyn per-se. The change to support capabilities was added to progression in Dev15.

How was the bug found?

Dogfooding

…nse type. This enables them to work on both .Net core projects and legacy projects.

RaulPerez1 · 2016-12-15T20:08:08Z

@Pilchie @jasonmalinowski - This are the changes I was asking about. I'm not sure that I moved them to the right .Next assembly so please let me know what the right place would be if this isn't it.

Pilchie · 2016-12-15T21:09:46Z

src/VisualStudio/Core/Next/Progression/CSharpGraphProvider.cs

@@ -12,7 +11,7 @@

 namespace Microsoft.VisualStudio.LanguageServices.CSharp.Progression
 {
-    [GraphProvider(Name = "CSharpRoslynProvider", IntellisenseType = Guids.CSharpProjectRootIdString)]
+    [GraphProvider(Name = "CSharpRoslynProvider", ProjectCapability = "CSharp")]


Should be LanguageNames.CSharp.

That constant resolves to C# not CSharp which is the project capability. Those constants are defined in CPS but I'm presuming that we don't really want to take a CPS dependency here just for these strings

Pilchie · 2016-12-15T21:10:48Z

src/VisualStudio/Core/Next/project.json

+        "Microsoft.VisualStudio.Shell.Immutable.10.0": "15.0.25413-Preview5",
+        "RoslynDependencies.Microsoft.VisualStudio.GraphModel": {
+            "version": "15.0.25726-Preview5",
+            "suppressParent": "all"


I don't think you should need suppressParent here.

Pilchie · 2016-12-15T21:13:05Z

src/VisualStudio/Core/Next/Progression/VisualBasicGraphProvider.cs

+
+namespace Microsoft.VisualStudio.LanguageServices.CSharp.Progression
+{
+    [GraphProvider(Name = "VisualBasicRoslynProvider", ProjectCapability = "VisualBasic")]


Unfortunately, I'm not sure that we have a constant that has "VisualBasic" with no space :(

Pilchie · 2016-12-15T21:15:10Z

src/VisualStudio/Core/Next/Progression/VisualBasicGraphProvider.cs

+    internal sealed class VisualBasicGraphProvider : AbstractGraphProvider
+    {
+        [ImportingConstructor]
+        public VisualBasicGraphProvider(


Given that this is identical to the C# one, is it possible to have a single provider instead that is tagged with both Capabilities? Doing that would make it more understandable why these are in the language neutral assembly.

If not, can you add a comment explaining why they are where they are?

I'll try it out, originally there were two (on different assemblies) so I'm presuming they were both needed.

Single provider worked well with the or on the capability.

Pilchie · 2016-12-15T21:16:18Z

Also tagging the rest of @dotnet/roslyn-ide

RaulPerez1 · 2016-12-19T22:52:02Z

@dotnet/roslyn-ide - Can someone shed some light on how this part is supposed to work:

14:25:23 Verifying project.json contents
14:25:25 Error! Package RoslynDependencies.Microsoft.VisualStudio.GraphModel has different versions:
14:25:25 src\VisualStudio\Core\Def\project.json at 14.3.25407
14:25:25 src\VisualStudio\Core\Next\project.json at 15.0.25726-Preview5
14:25:25 The versions must be the same or one must be explicitly listed as fixed in RepoData.json

I didn't see any RepoData.json in the enlistment so I'm guessing this is a build machine thing?

Pilchie

Thanks!

Pilchie · 2016-12-21T19:42:57Z

src/VisualStudio/Core/Next/project.json

+    "Microsoft.VisualStudio.Shell.Immutable.10.0": "15.0.25413-Preview5",
+    "RoslynDependencies.Microsoft.VisualStudio.GraphModel": {
+      "version": "15.0.25726-Preview5",
+      "suppressParent": "all"


I still don't think you want suppressParent here.

@Pilchie is right, suppress parent isn't needed.

Why isn't this being delivered via NuGet? Looks like it's on MyGet instead.

I'll remove it, I added it back seeing if that would fix the other issue.

Pilchie · 2016-12-21T19:47:11Z

The RepoUtil config is at build/config/RepoUtilData.json.

You'll need to add this to the fixedPackages list.

@jaredpar any chance you could include the config path in the errors?

@RaulPerez1 any ideas how to make the error message more actionable?

RaulPerez1 · 2016-12-29T23:19:52Z

@Pilchie - This is what I would write in that error message:

The versions must be the same or one must be explicitly listed as fixed under fixedPackages in build\config\RepoUtilData.json

You may not even need the full path there, if it had mentioned RepoUtilData.json I could have found it and figured it out from there.

@jaredpar - If the question was for me, no idea. I just added the dependency as it was from the other project but with the Preview3 version.

Pilchie · 2016-12-29T23:24:02Z

Note that @jaredpar is changing a bunch of project.json stuff in #16142. Might want to make sure this aligns with that.

jaredpar · 2016-12-29T23:39:46Z

build/config/RepoUtilData.json

@@ -7,6 +7,7 @@
    "System.Collections.Immutable": "1.1.36",
    "Microsoft.VisualStudio.CoreUtility": [ "14.3.25407", "15.0.25726-Preview5" ],
    "Microsoft.VisualStudio.Editor": [ "14.3.25407", "15.0.25726-Preview5" ],
+    "Microsoft.VisualStudio.GraphModel": [ "14.3.25407", "15.0.25726-Preview5" ],


Where is this package uploaded to? I didn't see it on NuGet.org.

I'll follow up with the owners and get back to you on that.

@RaulPerez1 if you rebase off of this PR you shouldn't need any changes here #16305

jaredpar · 2016-12-29T23:40:18Z

src/VisualStudio/Core/Next/project.json

-      "version": "15.0.25726-Preview5",
-      "suppressParent": "all"
-    }
+    "RoslynDependencies.Microsoft.VisualStudio.GraphModel": "15.0.25726-Preview5"


This package name doesn't line up with the entries in the config file. Typo?

Change progression providers to use capabilities instead of IntelliSe…

6473e15

…nse type. This enables them to work on both .Net core projects and legacy projects.

dnfclas added the cla-already-signed label Dec 15, 2016

Pilchie approved these changes Dec 15, 2016

View reviewed changes

Pilchie added the Area-IDE label Dec 15, 2016

Merge providers

f6963e5

jmarolf approved these changes Dec 19, 2016

View reviewed changes

Raul Perez added 2 commits December 19, 2016 13:51

Merge latest

17f5f9f

Fix bad merge

3c2b3a1

Pilchie added this to the 2.0 (RC.3) milestone Dec 21, 2016

Pilchie approved these changes Dec 21, 2016

View reviewed changes

Raul Perez added 2 commits December 29, 2016 15:20

Remove supressParent tag

54e1e32

Add Microsoft.VisualStudio.GraphModel to the list

fbaed63

jaredpar reviewed Dec 29, 2016

View reviewed changes

MattGertz modified the milestones: 2.0 (RTM), 2.0 (RC.3) Jan 14, 2017

MattGertz added the _Product-level triaged label Jan 15, 2017

RaulPerez1 closed this Jan 18, 2017

RaulPerez1 deleted the ProgressionCapabilities branch January 18, 2017 00:35

RaulPerez1 mentioned this pull request Jan 18, 2017

Progression capabilities #16572

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Change progression providers to use capabilities instead of IntelliSe…nse type. This enables them to work on both .Net core projects and legacy projects. #15935

Change progression providers to use capabilities instead of IntelliSe…nse type. This enables them to work on both .Net core projects and legacy projects. #15935

RaulPerez1 commented Dec 15, 2016

RaulPerez1 commented Dec 15, 2016

Pilchie Dec 15, 2016

RaulPerez1 Dec 15, 2016

Pilchie Dec 15, 2016

Pilchie Dec 15, 2016

Pilchie Dec 15, 2016

RaulPerez1 Dec 15, 2016

RaulPerez1 Dec 15, 2016

Pilchie commented Dec 15, 2016

RaulPerez1 commented Dec 19, 2016

Pilchie left a comment

Pilchie Dec 21, 2016

jaredpar Dec 21, 2016

RaulPerez1 Dec 29, 2016

Pilchie commented Dec 21, 2016

RaulPerez1 commented Dec 29, 2016

Pilchie commented Dec 29, 2016

jaredpar Dec 29, 2016

RaulPerez1 Jan 9, 2017

jaredpar Jan 9, 2017

jaredpar Dec 29, 2016

Change progression providers to use capabilities instead of IntelliSe…nse type. This enables them to work on both .Net core projects and legacy projects. #15935

Change progression providers to use capabilities instead of IntelliSe…nse type. This enables them to work on both .Net core projects and legacy projects. #15935

Conversation

RaulPerez1 commented Dec 15, 2016

RaulPerez1 commented Dec 15, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Pilchie commented Dec 15, 2016

RaulPerez1 commented Dec 19, 2016

Pilchie left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Pilchie commented Dec 21, 2016

RaulPerez1 commented Dec 29, 2016

Pilchie commented Dec 29, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment