Feedback

eng/pipelines/test-integration-job.yml

@@ -3,8 +3,9 @@ parameters:
     type: string
     default: 'Debug'
   - name: oop64bit
-    # Why string?  Parameters are evaluated at compile time, but all variables are strings at compile time.
-    # So in order to pass a variable in as a parameter here, it must be string.
+    # This is actually a boolean but must be defined as string.
+    # Parameters are evaluated at compile time, but all variables are strings at compile time.
+    # So in order to pass a parameter that comes from a variable these must be typed as string.
     type: string
     default: true
   - name: lspEditor

src/EditorFeatures/Core/Implementation/Classification/SemanticClassificationBufferTaggerProvider.Tagger.cs

@@ -11,7 +11,6 @@
 using System.Threading;
 using System.Threading.Tasks;
 using Microsoft.CodeAnalysis.Classification;
-using Microsoft.CodeAnalysis.Editor.Shared.Extensions;
 using Microsoft.CodeAnalysis.Editor.Shared.Tagging;
 using Microsoft.CodeAnalysis.Editor.Shared.Utilities;
 using Microsoft.CodeAnalysis.Editor.Tagging;
@@ -140,13 +139,6 @@ public IEnumerable<ITagSpan<IClassificationTag>> GetAllTags(NormalizedSnapshotSp
                 var snapshot = firstSpan.Snapshot;
                 Debug.Assert(snapshot.TextBuffer == _subjectBuffer);
 
-                // The LSP client will handle producing tags when running under the LSP editor.
-                // Our tagger implementation should return nothing to prevent conflicts.
-                if (snapshot.TextBuffer.IsInLspEditorContext())
-                {
-                    return Array.Empty<ITagSpan<IClassificationTag>>();
-                }
-
                 // We want to classify from the start of the first requested span to the end of the 
                 // last requested span.
                 var spanToTag = new SnapshotSpan(snapshot,

src/EditorFeatures/Core/Implementation/Classification/SemanticClassificationBufferTaggerProvider.cs

@@ -2,10 +2,9 @@
 // The .NET Foundation licenses this file to you under the MIT license.
 // See the LICENSE file in the project root for more information.
 
-#nullable disable
-
 using System.ComponentModel.Composition;
 using System.Diagnostics.CodeAnalysis;
+using Microsoft.CodeAnalysis.Editor.Shared.Extensions;
 using Microsoft.CodeAnalysis.Editor.Shared.Utilities;
 using Microsoft.CodeAnalysis.Notification;
 using Microsoft.CodeAnalysis.Shared.TestHooks;
@@ -47,13 +46,21 @@ public SemanticClassificationBufferTaggerProvider(
             _asyncListener = listenerProvider.GetListener(FeatureAttribute.Classification);
         }
 
-        public IAccurateTagger<T> CreateTagger<T>(ITextBuffer buffer) where T : ITag
+        public IAccurateTagger<T>? CreateTagger<T>(ITextBuffer buffer) where T : ITag
         {
             this.AssertIsForeground();
+
+            // The LSP client will handle producing tags when running under the LSP editor.
+            // Our tagger implementation should return nothing to prevent conflicts.
+            if (buffer.IsInLspEditorContext())
+            {
+                return null;
+            }
+
             return new Tagger(this, buffer, _asyncListener) as IAccurateTagger<T>;
         }
 
-        ITagger<T> ITaggerProvider.CreateTagger<T>(ITextBuffer buffer)
+        ITagger<T>? ITaggerProvider.CreateTagger<T>(ITextBuffer buffer)
             => CreateTagger<T>(buffer);
     }
 }

src/EditorFeatures/Core/Implementation/InlineRename/CommandHandlers/AbstractRenameCommandHandler_RenameHandler.cs

@@ -106,7 +106,7 @@ private static bool CanRename(RenameCommandArgs args)
         {
             return args.SubjectBuffer.TryGetWorkspace(out var workspace) &&
                 workspace.CanApplyChange(ApplyChangesKind.ChangeDocument) &&
-                args.SubjectBuffer.SupportsRename();
+                args.SubjectBuffer.SupportsRename() && !args.SubjectBuffer.IsInLspEditorContext();
         }
 
         private static void ShowErrorDialog(Workspace workspace, string message)

src/VisualStudio/Core/Def/Implementation/LanguageClient/LiveShareInProcLanguageClient.cs

@@ -46,6 +46,7 @@ protected internal override VSServerCapabilities GetCapabilities()
         {
             var experimentationService = Workspace.Services.GetRequiredService<IExperimentationService>();
             var isLspEditorEnabled = experimentationService.IsExperimentEnabled(VisualStudioWorkspaceContextService.LspEditorFeatureFlagName);
+
             // If the preview feature flag to turn on the LSP editor in local scenarios is on, advertise no capabilities for this Live Share
             // LSP server as LSP requests will be serviced by the AlwaysActiveInProcLanguageClient in both local and remote scenarios.
             if (isLspEditorEnabled)

src/VisualStudio/Core/Def/Implementation/VisualStudioSupportsFeatureService.cs

@@ -44,13 +44,6 @@ public bool SupportsRefactorings(ITextBuffer textBuffer)
 
             public bool SupportsRename(ITextBuffer textBuffer)
             {
-                // Disable rename entry points when running under the LSP editor.
-                // The LSP client will handle the rename request.
-                if (textBuffer.IsInLspEditorContext())
-                {
-                    return false;
-                }
-
                 var sourceTextContainer = textBuffer.AsTextContainer();
                 if (Workspace.TryGetWorkspace(sourceTextContainer, out var workspace))
                 {

src/VisualStudio/Core/Def/PackageRegistration.pkgdef

@@ -14,7 +14,7 @@
 "VB"="EEC3DF0D-6D3F-4544-ABF9-8E26E6A90275"
 
 [$RootKey$\FeatureFlags\Roslyn\LSP\Editor]
-"Description"="Enables the LSP powered C#/VB editing experience outside of CodeSpaces."
+"Description"="Enables the LSP-powered C#/VB editing experience outside of CodeSpaces."
 "Value"=dword:00000000
 "Title"="Enable experimental C#/VB editor (requires restart)"
 "PreviewPaneChannels"="IntPreview,int.main"

src/VisualStudio/IntegrationTest/IntegrationTests/CSharp/CSharpGoToDefinition.cs

@@ -26,7 +26,7 @@ public CSharpGoToDefinition(VisualStudioInstanceFactory instanceFactory)
         {
         }
 
-        [WpfFact, Trait(Traits.Feature, Traits.Features.GoToDefinition), Trait(Traits.Editor, Traits.Editors.LanguageServerProtocol)]
+        [WpfFact, Trait(Traits.Feature, Traits.Features.GoToDefinition)]
         public void GoToClassDeclaration()
         {
             var project = new ProjectUtils.Project(ProjectName);
@@ -49,7 +49,7 @@ public void GoToClassDeclaration()
             Assert.False(VisualStudio.Shell.IsActiveTabProvisional());
         }
 
-        [WpfFact, Trait(Traits.Feature, Traits.Features.GoToDefinition), Trait(Traits.Editor, Traits.Editors.LanguageServerProtocol)]
+        [WpfFact, Trait(Traits.Feature, Traits.Features.GoToDefinition)]
         public void GoToDefinitionOpensProvisionalTabIfDocumentNotAlreadyOpen()
         {
             var project = new ProjectUtils.Project(ProjectName);
@@ -74,21 +74,25 @@ public void GoToDefinitionOpensProvisionalTabIfDocumentNotAlreadyOpen()
             Assert.True(VisualStudio.Shell.IsActiveTabProvisional());
         }
 
-        [WpfFact, Trait(Traits.Feature, Traits.Features.GoToDefinition)]
-        public void GoToDefinitionWithMultipleResults()
+        [Fact, Trait(Traits.Feature, Traits.Features.GoToDefinition)]
+        public virtual void GoToDefinitionWithMultipleResults()
+        {
+            TestGoToDefinitionWithMultipleResults(declarationWindowName: "'PartialClass' declarations");
+        }
+
+        protected void TestGoToDefinitionWithMultipleResults(string declarationWindowName)
         {
             SetUpEditor(
 @"partial class /*Marker*/ $$PartialClass { }
 
 partial class PartialClass { int i = 0; }");
 
-            VisualStudio.Editor.GoToDefinition("Class1.cs");
+            VisualStudio.Editor.GoToDefinition(declarationWindowName);
 
-            const string programReferencesCaption = "'PartialClass' declarations";
-            var results = VisualStudio.FindReferencesWindow.GetContents(programReferencesCaption);
+            var results = VisualStudio.FindReferencesWindow.GetContents(declarationWindowName);
 
             var activeWindowCaption = VisualStudio.Shell.GetActiveWindowCaption();
-            Assert.Equal(expected: programReferencesCaption, actual: activeWindowCaption);
+            Assert.Equal(expected: declarationWindowName, actual: activeWindowCaption);
 
             Assert.Collection(
                 results,

src/VisualStudio/IntegrationTest/IntegrationTests/CSharp/CSharpGoToImplementation.cs

@@ -88,7 +88,7 @@ public void SomeMethod()
     }
 }");
             VisualStudio.Editor.PlaceCaret("IDisposable d", charsOffset: -1);
-            VisualStudio.Editor.GoToDefinition("IDisposable.cs");
+            VisualStudio.Editor.GoToDefinition("IDisposable [from metadata]");
             VisualStudio.Editor.GoToImplementation("FileImplementation.cs");
             VisualStudio.Editor.Verify.TextContains(@"class Implementation$$ : IDisposable", assertCaretPosition: true);
         }

src/VisualStudio/IntegrationTest/IntegrationTests/LanguageServerProtocol/LspGoToDefinition.cs

@@ -18,54 +18,25 @@ namespace Roslyn.VisualStudio.IntegrationTests.LanguageServerProtocol
     /// These tests test behavior that only applies to the LSP version of goto definition.
     /// </summary>
     [Collection(nameof(SharedIntegrationHostFixture))]
-    public class LspGoToDefinition : AbstractEditorTest
+    [Trait(Traits.Editor, Traits.Editors.LanguageServerProtocol)]
+    public class LspGoToDefinition : CSharpGoToDefinition
     {
         protected override string LanguageName => LanguageNames.CSharp;
 
         public LspGoToDefinition(VisualStudioInstanceFactory instanceFactory)
-            : base(instanceFactory, nameof(LspGoToDefinition))
+            : base(instanceFactory)
         {
         }
 
         /// <summary>
-        /// This test corresponds to the <see cref="CSharpGoToDefinition.GoToDefinitionWithMultipleResults"/> test.
-        /// The non-LSP version will not work due to feature parity issues with the multiple results window, mainly
-        /// different naming and grouping functionality.
+        /// We need to pass in a different window name to look for declarations in as the LSP client
+        /// uses a different name to create the references window.
+        /// https://devdiv.visualstudio.com/DevDiv/_workitems/edit/1286575
         /// </summary>
-        [WpfFact, Trait(Traits.Feature, Traits.Features.GoToDefinition), Trait(Traits.Editor, Traits.Editors.LanguageServerProtocol)]
-        public void GoToDefinitionWithMultipleLSP()
+        [Fact, Trait(Traits.Feature, Traits.Features.GoToDefinition), Trait(Traits.Editor, Traits.Editors.LanguageServerProtocol)]
+        public override void GoToDefinitionWithMultipleResults()
         {
-            SetUpEditor(
-@"partial class /*Marker*/ $$PartialClass { }
-
-partial class PartialClass { int i = 0; }");
-
-            VisualStudio.Editor.GoToDefinition("Class1.cs");
-
-            const string programReferencesCaption = "'PartialClass' references";
-            VisualStudio.Editor.WaitForActiveWindow(programReferencesCaption);
-            var results = VisualStudio.FindReferencesWindow.GetContents(programReferencesCaption);
-
-            var activeWindowCaption = VisualStudio.Shell.GetActiveWindowCaption();
-            Assert.Equal(expected: programReferencesCaption, actual: activeWindowCaption);
-
-            Assert.Collection(
-                results,
-                new Action<Reference>[]
-                {
-                    reference =>
-                    {
-                        Assert.Equal(expected: "partial class /*Marker*/ PartialClass { }", actual: reference.Code);
-                        Assert.Equal(expected: 0, actual: reference.Line);
-                        Assert.Equal(expected: 25, actual: reference.Column);
-                    },
-                    reference =>
-                    {
-                        Assert.Equal(expected: "partial class PartialClass { int i = 0; }", actual: reference.Code);
-                        Assert.Equal(expected: 2, actual: reference.Line);
-                        Assert.Equal(expected: 14, actual: reference.Column);
-                    }
-                });
+            TestGoToDefinitionWithMultipleResults(declarationWindowName: "'PartialClass' references");
         }
     }
 }

src/VisualStudio/IntegrationTest/TestUtilities/OutOfProcess/Editor_OutOfProc.cs

@@ -365,16 +365,16 @@ private TextSpan[] Deserialize(string[] v)
             }).ToArray();
         }
 
-        public void GoToDefinition(string viewName)
+        public void GoToDefinition(string expectedWindowName)
         {
             _editorInProc.GoToDefinition();
-            _editorInProc.WaitForActiveView(viewName);
+            _editorInProc.WaitForActiveWindow(expectedWindowName);
         }
 
-        public void GoToImplementation(string viewName)
+        public void GoToImplementation(string expectedWindowName)
         {
             _editorInProc.GoToImplementation();
-            _editorInProc.WaitForActiveView(viewName);
+            _editorInProc.WaitForActiveWindow(expectedWindowName);
         }
 
         public void SendExplicitFocus()

src/VisualStudio/IntegrationTest/TestUtilities/VisualStudioInstanceFactory.cs

@@ -14,6 +14,7 @@
 using System.Runtime.ExceptionServices;
 using System.Threading.Tasks;
 using EnvDTE;
+using Microsoft.VisualStudio.LanguageServices.Implementation;
 using Microsoft.VisualStudio.Setup.Configuration;
 using Microsoft.Win32;
 using Roslyn.Utilities;
@@ -335,7 +336,8 @@ private static Process StartNewVisualStudioProcess(string installationPath, int
                 Process.Start(CreateSilentStartInfo(vsRegEditExeFile, $"set \"{installationPath}\" {Settings.Default.VsRootSuffix} HKCU \"FeatureFlags\\Setup\\BackgroundDownload\" Value dword 0")).WaitForExit();
 
                 var lspRegistryValue = usingLspEditor ? "1" : "0";
-                Process.Start(CreateSilentStartInfo(vsRegEditExeFile, $"set \"{installationPath}\" {Settings.Default.VsRootSuffix} HKCU \"FeatureFlags\\Roslyn\\LSP\\Editor\" Value dword {lspRegistryValue}")).WaitForExit();
+                var lspFeatureFlagName = VisualStudioWorkspaceContextService.LspEditorFeatureFlagName.Replace(".", "\\");
+                Process.Start(CreateSilentStartInfo(vsRegEditExeFile, $"set \"{installationPath}\" {Settings.Default.VsRootSuffix} HKCU \"FeatureFlags\\{lspFeatureFlagName}\" Value dword {lspRegistryValue}")).WaitForExit();
                 Registry.SetValue(@"HKEY_CURRENT_USER\Software\Microsoft\VisualStudio\Telemetry\Channels", "fileLogger", 1, RegistryValueKind.DWord);
 
                 // Remove legacy experiment setting for controlling async completion to ensure it does not interfere.

src/VisualStudio/Setup.Dependencies/Roslyn.VisualStudio.Setup.Dependencies.csproj

@@ -35,7 +35,6 @@
     <NuGetPackageToIncludeInVsix Include="System.Runtime.CompilerServices.Unsafe" PkgDefEntry="BindingRedirect" />
     <NuGetPackageToIncludeInVsix Include="System.Text.Encoding.CodePages" PkgDefEntry="BindingRedirect" />
     <NuGetPackageToIncludeInVsix Include="System.Threading.Tasks.Extensions" PkgDefEntry="BindingRedirect" />
-    <NuGetPackageToIncludeInVsix Include="System.IO.Pipelines" PkgDefEntry="BindingRedirect" />
   </ItemGroup>
   <ItemGroup>
     <None Include="source.extension.vsixmanifest">