diff --git a/src/Compilers/CSharp/Test/Emit/Emit/EditAndContinue/EditAndContinueTests.cs b/src/Compilers/CSharp/Test/Emit/Emit/EditAndContinue/EditAndContinueTests.cs index ab9ca90c11e97..015034f5ef769 100644 --- a/src/Compilers/CSharp/Test/Emit/Emit/EditAndContinue/EditAndContinueTests.cs +++ b/src/Compilers/CSharp/Test/Emit/Emit/EditAndContinue/EditAndContinueTests.cs @@ -10500,6 +10500,82 @@ .locals init ([int] V_0, IL_0023: ldloc.s V_8 IL_0025: ret } +"); + } + + [Fact] + public void AddUsing_AmbiguousCode() + { + var source0 = MarkedSource(@" +using System.Threading; + +class C +{ + static void E() + { + var t = new Timer(s => System.Console.WriteLine(s)); + } +}"); + var source1 = MarkedSource(@" +using System.Threading; +using System.Timers; + +class C +{ + static void E() + { + var t = new Timer(s => System.Console.WriteLine(s)); + } + + static void G() + { + System.Console.WriteLine(new TimersDescriptionAttribute("""")); + } +}"); + + var compilation0 = CreateCompilation(source0.Tree, targetFramework: TargetFramework.NetStandard20, options: ComSafeDebugDll); + var compilation1 = compilation0.WithSource(source1.Tree); + + var e0 = compilation0.GetMember("C.E"); + var e1 = compilation1.GetMember("C.E"); + var g1 = compilation1.GetMember("C.G"); + + var v0 = CompileAndVerify(compilation0); + var md0 = ModuleMetadata.CreateFromImage(v0.EmittedAssemblyData); + var generation0 = EmitBaseline.CreateInitialBaseline(md0, v0.CreateSymReader().GetEncMethodDebugInfo); + + // Pretend there was an update to C.E to ensure we haven't invalidated the test + + var diffError = compilation1.EmitDifference( + generation0, + ImmutableArray.Create( + SemanticEdit.Create(SemanticEditKind.Update, e0, e1, GetSyntaxMapFromMarkers(source0, source1), preserveLocalVariables: true))); + + diffError.EmitResult.Diagnostics.Verify( + // (9,21): error CS0104: 'Timer' is an ambiguous reference between 'System.Threading.Timer' and 'System.Timers.Timer' + // var t = new Timer(s => System.Console.WriteLine(s)); + Diagnostic(ErrorCode.ERR_AmbigContext, "Timer").WithArguments("Timer", "System.Threading.Timer", "System.Timers.Timer").WithLocation(9, 21)); + + // Semantic errors are reported only for the bodies of members being emitted so we shouldn't see any + + var diff = compilation1.EmitDifference( + generation0, + ImmutableArray.Create( + SemanticEdit.Create(SemanticEditKind.Insert, null, g1))); + + diff.EmitResult.Diagnostics.Verify(); + + diff.VerifyIL(@"C.G", @" +{ + // Code size 18 (0x12) + .maxstack 1 + IL_0000: nop + IL_0001: ldstr """" + IL_0006: newobj ""System.Timers.TimersDescriptionAttribute..ctor(string)"" + IL_000b: call ""void System.Console.WriteLine(object)"" + IL_0010: nop + IL_0011: ret +} "); } } diff --git a/src/Compilers/VisualBasic/Test/Emit/Emit/EditAndContinue/EditAndContinueTests.vb b/src/Compilers/VisualBasic/Test/Emit/Emit/EditAndContinue/EditAndContinueTests.vb index a704b9b179454..f13031ee6ca7a 100644 --- a/src/Compilers/VisualBasic/Test/Emit/Emit/EditAndContinue/EditAndContinueTests.vb +++ b/src/Compilers/VisualBasic/Test/Emit/Emit/EditAndContinue/EditAndContinueTests.vb @@ -5299,7 +5299,70 @@ End Class") ") End Sub + + Public Sub AddImports_AmbiguousCode() + + Dim source0 = MarkedSource(" +Imports System.Threading + +Class C + Shared Sub E() + Dim t = New Timer(Sub(s) System.Console.WriteLine(s)) + End Sub +End Class +") + Dim source1 = MarkedSource(" +Imports System.Threading +Imports System.Timers + +Class C + Shared Sub E() + Dim t = New Timer(Sub(s) System.Console.WriteLine(s)) + End Sub + Shared Sub G() + System.Console.WriteLine(new TimersDescriptionAttribute("""")) + End Sub +End Class +") + Dim compilation0 = CreateCompilation(source0.Tree, targetFramework:=TargetFramework.NetStandard20, options:=ComSafeDebugDll) + Dim compilation1 = compilation0.WithSource(source1.Tree) + + Dim e0 = compilation0.GetMember(Of MethodSymbol)("C.E") + Dim e1 = compilation1.GetMember(Of MethodSymbol)("C.E") + Dim g1 = compilation1.GetMember(Of MethodSymbol)("C.G") + + Dim v0 = CompileAndVerify(compilation0) + Dim md0 = ModuleMetadata.CreateFromImage(v0.EmittedAssemblyData) + Dim generation0 = EmitBaseline.CreateInitialBaseline(md0, AddressOf v0.CreateSymReader().GetEncMethodDebugInfo) + + ' Pretend there was an update to C.E to ensure we haven't invalidated the test + + Dim diffError = compilation1.EmitDifference( + generation0, + ImmutableArray.Create(New SemanticEdit(SemanticEditKind.Update, e0, e1, GetSyntaxMapFromMarkers(source0, source1), preserveLocalVariables:=True))) + + diffError.EmitResult.Diagnostics.Verify( + Diagnostic(ERRID.ERR_AmbiguousInImports2, "Timer").WithArguments("Timer", "System.Threading, System.Timers").WithLocation(7, 21)) + + Dim diff = compilation1.EmitDifference( + generation0, + ImmutableArray.Create(New SemanticEdit(SemanticEditKind.Insert, Nothing, g1))) + + diff.EmitResult.Diagnostics.Verify() + + diff.VerifyIL("C.G", " +{ + // Code size 18 (0x12) + .maxstack 1 + IL_0000: nop + IL_0001: ldstr """" + IL_0006: newobj ""Sub System.Timers.TimersDescriptionAttribute..ctor(String)"" + IL_000b: call ""Sub System.Console.WriteLine(Object)"" + IL_0010: nop + IL_0011: ret +}") + End Sub End Class End Namespace diff --git a/src/EditorFeatures/CSharpTest/EditAndContinue/TopLevelEditingTests.cs b/src/EditorFeatures/CSharpTest/EditAndContinue/TopLevelEditingTests.cs index 511f62238ce5b..4091cf658fe0a 100644 --- a/src/EditorFeatures/CSharpTest/EditAndContinue/TopLevelEditingTests.cs +++ b/src/EditorFeatures/CSharpTest/EditAndContinue/TopLevelEditingTests.cs @@ -53,9 +53,7 @@ public void UsingDelete2() "Delete [using D = System.Diagnostics;]@2", "Delete [using System.Collections;]@33"); - edits.VerifyRudeDiagnostics( - Diagnostic(RudeEditKind.Delete, null, CSharpFeaturesResources.using_directive), - Diagnostic(RudeEditKind.Delete, null, CSharpFeaturesResources.using_directive)); + edits.VerifyRudeDiagnostics(); } [Fact] @@ -75,9 +73,7 @@ public void UsingInsert() "Insert [using D = System.Diagnostics;]@2", "Insert [using System.Collections;]@33"); - edits.VerifyRudeDiagnostics( - Diagnostic(RudeEditKind.Insert, "using D = System.Diagnostics;", CSharpFeaturesResources.using_directive), - Diagnostic(RudeEditKind.Insert, "using System.Collections;", CSharpFeaturesResources.using_directive)); + edits.VerifyRudeDiagnostics(); } [Fact] @@ -98,8 +94,7 @@ public void UsingUpdate1() edits.VerifyEdits( "Update [using System.Collections;]@29 -> [using X = System.Collections;]@29"); - edits.VerifyRudeDiagnostics( - Diagnostic(RudeEditKind.Update, "using X = System.Collections;", CSharpFeaturesResources.using_directive)); + edits.VerifyRudeDiagnostics(); } [Fact] @@ -120,8 +115,7 @@ public void UsingUpdate2() edits.VerifyEdits( "Update [using X1 = System.Collections;]@29 -> [using X2 = System.Collections;]@29"); - edits.VerifyRudeDiagnostics( - Diagnostic(RudeEditKind.Update, "using X2 = System.Collections;", CSharpFeaturesResources.using_directive)); + edits.VerifyRudeDiagnostics(); } [Fact] @@ -142,8 +136,7 @@ public void UsingUpdate3() edits.VerifyEdits( "Update [using System.Diagnostics;]@2 -> [using System;]@2"); - edits.VerifyRudeDiagnostics( - Diagnostic(RudeEditKind.Update, "using System;", CSharpFeaturesResources.using_directive)); + edits.VerifyRudeDiagnostics(); } [Fact] @@ -218,6 +211,172 @@ namespace N "Delete [using System.Collections;]@22"); } + [Fact] + public void Using_Delete_ChangesCodeMeaning() + { + // This test specifically validates the scenario we _don't_ support, namely when inserting or deleting + // a using directive, if existing code changes in meaning as a result, we don't issue edits for that code. + // If this ever regresses then please buy a lottery ticket because the feature has magically fixed itself. + var src1 = @" +using System.IO; +using DirectoryInfo = N.C; + +namespace N +{ + public class C + { + public C(string a) { } + public FileAttributes Attributes { get; set; } + } + + public class D + { + public void M() + { + var d = new DirectoryInfo(""aa""); + var x = directoryInfo.Attributes; + } + } +}"; + var src2 = @" +using System.IO; + +namespace N +{ + public class C + { + public C(string a) { } + public FileAttributes Attributes { get; set; } + } + + public class D + { + public void M() + { + var d = new DirectoryInfo(""aa""); + var x = directoryInfo.Attributes; + } + } +}"; + + var edits = GetTopEdits(src1, src2); + + edits.VerifyEdits("Delete [using DirectoryInfo = N.C;]@20"); + + edits.VerifySemantics(); + } + + [Fact] + public void Using_Insert_ForNewCode() + { + // As distinct from the above, this test validates a real world scenario of inserting a using directive + // and changing code that utilizes the new directive to some effect. + var src1 = @" +namespace N +{ + class Program + { + static void Main(string[] args) + { + } + } +}"; + var src2 = @" +using System; + +namespace N +{ + class Program + { + static void Main(string[] args) + { + Console.WriteLine(""Hello World!""); + } + } +}"; + + var edits = GetTopEdits(src1, src2); + + edits.VerifySemantics(SemanticEdit(SemanticEditKind.Update, c => c.GetMember("N.Program.Main"))); + } + + [Fact] + public void Using_Delete_ForOldCode() + { + var src1 = @" +using System; + +namespace N +{ + class Program + { + static void Main(string[] args) + { + Console.WriteLine(""Hello World!""); + } + } +}"; + var src2 = @" +namespace N +{ + class Program + { + static void Main(string[] args) + { + } + } +}"; + + var edits = GetTopEdits(src1, src2); + + edits.VerifySemantics(SemanticEdit(SemanticEditKind.Update, c => c.GetMember("N.Program.Main"))); + } + + [Fact] + public void Using_Insert_CreatesAmbiguousCode() + { + // This test validates that we still issue edits for changed valid code, even when unchanged + // code has ambiguities after adding a using. + var src1 = @" +using System.Threading; + +namespace N +{ + class C + { + void M() + { + // Timer exists in System.Threading and System.Timers + var t = new Timer(s => System.Console.WriteLine(s)); + } + } +}"; + var src2 = @" +using System.Threading; +using System.Timers; + +namespace N +{ + class C + { + void M() + { + // Timer exists in System.Threading and System.Timers + var t = new Timer(s => System.Console.WriteLine(s)); + } + + void M2() + { + // TimersDescriptionAttribute only exists in System.Timers + System.Console.WriteLine(new TimersDescriptionAttribute("""")); + } + } +}"; + var edits = GetTopEdits(src1, src2); + + edits.VerifySemantics(SemanticEdit(SemanticEditKind.Insert, c => c.GetMember("N.C.M2"))); + } + #endregion #region Extern Alias diff --git a/src/EditorFeatures/Core/Implementation/IntelliSense/AsyncCompletion/CompletionSource.cs b/src/EditorFeatures/Core/Implementation/IntelliSense/AsyncCompletion/CompletionSource.cs index 28fc50af52c43..c2a51cad09794 100644 --- a/src/EditorFeatures/Core/Implementation/IntelliSense/AsyncCompletion/CompletionSource.cs +++ b/src/EditorFeatures/Core/Implementation/IntelliSense/AsyncCompletion/CompletionSource.cs @@ -267,9 +267,6 @@ private bool TryInvokeSnippetCompletion( roslynTrigger = new CompletionTrigger(CompletionTriggerKind.Snippets); } - var disallowAddingImports = _isDebuggerTextView || - document.Project.Solution.Workspace.Services.GetRequiredService().CurrentDebuggingState != DebuggingState.Design; - var documentOptions = await document.GetOptionsAsync(cancellationToken).ConfigureAwait(false); var options = documentOptions .WithChangedOption(CompletionServiceOptions.IsExpandedCompletion, isExpanded); @@ -278,12 +275,7 @@ private bool TryInvokeSnippetCompletion( { options = options .WithChangedOption(CompletionControllerOptions.FilterOutOfScopeLocals, false) - .WithChangedOption(CompletionControllerOptions.ShowXmlDocCommentCompletion, false); - } - - if (disallowAddingImports) - { - options = options + .WithChangedOption(CompletionControllerOptions.ShowXmlDocCommentCompletion, false) .WithChangedOption(CompletionServiceOptions.DisallowAddingImports, true); } @@ -329,7 +321,7 @@ private bool TryInvokeSnippetCompletion( // It's OK to overwrite this value when expanded items are requested. session.Properties[CompletionListSpan] = completionList.Span; - if (disallowAddingImports) + if (_isDebuggerTextView) { session.Properties[DisallowAddingImports] = true; } diff --git a/src/EditorFeatures/VisualBasicTest/EditAndContinue/TopLevelEditingTests.vb b/src/EditorFeatures/VisualBasicTest/EditAndContinue/TopLevelEditingTests.vb index d4970c6df6076..5ec44d611ab73 100644 --- a/src/EditorFeatures/VisualBasicTest/EditAndContinue/TopLevelEditingTests.vb +++ b/src/EditorFeatures/VisualBasicTest/EditAndContinue/TopLevelEditingTests.vb @@ -46,10 +46,7 @@ Imports System.Collections.Generic "Delete [Imports ]@34", "Delete [Imports System.Collections]@76") - edits.VerifyRudeDiagnostics( - Diagnostic(RudeEditKind.Delete, Nothing, VBFeaturesResources.import), - Diagnostic(RudeEditKind.Delete, Nothing, VBFeaturesResources.import), - Diagnostic(RudeEditKind.Delete, Nothing, VBFeaturesResources.import)) + edits.VerifyRudeDiagnostics() End Sub @@ -71,10 +68,7 @@ Imports System.Collections.Generic "Insert [Imports ]@34", "Insert [Imports System.Collections]@76") - edits.VerifyRudeDiagnostics( - Diagnostic(RudeEditKind.Insert, "Imports D = System.Diagnostics", VBFeaturesResources.import), - Diagnostic(RudeEditKind.Insert, "Imports ", "import"), - Diagnostic(RudeEditKind.Insert, "Imports System.Collections", VBFeaturesResources.import)) + edits.VerifyRudeDiagnostics() End Sub @@ -95,8 +89,7 @@ Imports System.Collections.Generic edits.VerifyEdits( "Update [Imports System.Collections]@30 -> [Imports X = System.Collections]@30") - edits.VerifyRudeDiagnostics( - Diagnostic(RudeEditKind.Update, "Imports X = System.Collections", VBFeaturesResources.import)) + edits.VerifyRudeDiagnostics() End Sub @@ -130,8 +123,7 @@ Imports System.Collections.Generic edits.VerifyEdits( "Update [Imports X1 = System.Collections]@30 -> [Imports X2 = System.Collections]@30") - edits.VerifyRudeDiagnostics( - Diagnostic(RudeEditKind.Update, "Imports X2 = System.Collections", VBFeaturesResources.import)) + edits.VerifyRudeDiagnostics() End Sub @@ -153,8 +145,7 @@ Imports System.Collections.Generic edits.VerifyEdits( "Update [Imports System.Diagnostics]@2 -> [Imports System]@2") - edits.VerifyRudeDiagnostics( - Diagnostic(RudeEditKind.Update, "Imports System", VBFeaturesResources.import)) + edits.VerifyRudeDiagnostics() End Sub @@ -176,6 +167,60 @@ Imports System.Diagnostics edits.VerifyEdits( "Reorder [Imports System.Diagnostics]@2 -> @66") End Sub + + + Public Sub ImportInsert_WithNewCode() + Dim src1 = " +Class C + Sub M() + End Sub +End Class +" + + Dim src2 = " +Imports System + +Class C + Sub M() + Console.WriteLine(1) + End Sub +End Class +" + + Dim edits = GetTopEdits(src1, src2) + + edits.VerifySemantics(semanticEdits:= + { + SemanticEdit(SemanticEditKind.Update, Function(c) c.GetMember("C.M")) + }) + End Sub + + + Public Sub ImportDelete_WithOldCode() + Dim src1 = " +Imports System + +Class C + Sub M() + Console.WriteLine(1) + End Sub +End Class +" + + Dim src2 = " +Class C + Sub M() + End Sub +End Class +" + + Dim edits = GetTopEdits(src1, src2) + + edits.VerifySemantics(semanticEdits:= + { + SemanticEdit(SemanticEditKind.Update, Function(c) c.GetMember("C.M")) + }) + End Sub #End Region #Region "Option" diff --git a/src/Features/CSharp/Portable/EditAndContinue/CSharpEditAndContinueAnalyzer.cs b/src/Features/CSharp/Portable/EditAndContinue/CSharpEditAndContinueAnalyzer.cs index 4529db903e000..ace0587368a01 100644 --- a/src/Features/CSharp/Portable/EditAndContinue/CSharpEditAndContinueAnalyzer.cs +++ b/src/Features/CSharp/Portable/EditAndContinue/CSharpEditAndContinueAnalyzer.cs @@ -2080,11 +2080,17 @@ private void ClassifyInsert(SyntaxNode node) return; case SyntaxKind.ExternAliasDirective: - case SyntaxKind.UsingDirective: case SyntaxKind.NamespaceDeclaration: ReportError(RudeEditKind.Insert); return; + case SyntaxKind.UsingDirective: + // We don't report rude edits for using directives though we also don't do any + // special processing, so inserting a using directive that changes semantics in + // unedited code will not issue edits for that code. Inserting a using directive + // and then consunming it will result in edits for the changes code as per usual. + return; + case SyntaxKind.ClassDeclaration: case SyntaxKind.StructDeclaration: case SyntaxKind.InterfaceDeclaration: @@ -2149,13 +2155,20 @@ private void ClassifyDelete(SyntaxNode oldNode) return; case SyntaxKind.ExternAliasDirective: - case SyntaxKind.UsingDirective: case SyntaxKind.NamespaceDeclaration: // To allow removal of declarations we would need to update method bodies that // were previously binding to them but now are binding to another symbol that was previously hidden. ReportError(RudeEditKind.Delete); return; + case SyntaxKind.UsingDirective: + // We don't report rude edits for using directives though we also don't do any + // special processing, so deleting a using directive that changes semantics in + // unedited code will not issue edits for that code. Deleting code and then doing + // something like Remove Unused Usings will report edits for the changes code as + // normal. + return; + case SyntaxKind.DestructorDeclaration: case SyntaxKind.ConversionOperatorDeclaration: case SyntaxKind.OperatorDeclaration: @@ -2231,15 +2244,14 @@ private void ClassifyUpdate(SyntaxNode oldNode, SyntaxNode newNode) return; case SyntaxKind.UsingDirective: - // Dev12 distinguishes using alias from using namespace and reports different errors for removing alias. - // None of these changes are allowed anyways, so let's keep it simple. - ReportError(RudeEditKind.Update); + // We don't report rude edits for using directives though we also don't do any + // special processing, so updating a using directive that changes semantics in + // unedited code will not issue edits for that code. return; case SyntaxKind.NamespaceDeclaration: ClassifyUpdate((NamespaceDeclarationSyntax)oldNode, (NamespaceDeclarationSyntax)newNode); return; - case SyntaxKind.ClassDeclaration: case SyntaxKind.StructDeclaration: case SyntaxKind.InterfaceDeclaration: diff --git a/src/Features/VisualBasic/Portable/EditAndContinue/VisualBasicEditAndContinueAnalyzer.vb b/src/Features/VisualBasic/Portable/EditAndContinue/VisualBasicEditAndContinueAnalyzer.vb index 2c8211f98cf71..265bcbeb60281 100644 --- a/src/Features/VisualBasic/Portable/EditAndContinue/VisualBasicEditAndContinueAnalyzer.vb +++ b/src/Features/VisualBasic/Portable/EditAndContinue/VisualBasicEditAndContinueAnalyzer.vb @@ -2081,11 +2081,15 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.EditAndContinue Private Sub ClassifyInsert(node As SyntaxNode) Select Case node.Kind Case SyntaxKind.OptionStatement, - SyntaxKind.ImportsStatement, SyntaxKind.NamespaceBlock ReportError(RudeEditKind.Insert) Return + Case SyntaxKind.ImportsStatement + ' We don't report rude edits for new imports, though note we don't currently report edits + ' for semantic changes they cause either + Return + Case SyntaxKind.ClassBlock, SyntaxKind.StructureBlock, SyntaxKind.InterfaceBlock, @@ -2160,12 +2164,16 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.EditAndContinue Private Sub ClassifyDelete(oldNode As SyntaxNode) Select Case oldNode.Kind Case SyntaxKind.OptionStatement, - SyntaxKind.ImportsStatement, SyntaxKind.NamespaceBlock, SyntaxKind.AttributesStatement ReportError(RudeEditKind.Delete) Return + Case SyntaxKind.ImportsStatement + ' We don't report rude edits for deleting imports, though note we don't currently report edits + ' for semantic changes they cause either + Return + Case SyntaxKind.ClassBlock, SyntaxKind.StructureBlock, SyntaxKind.InterfaceBlock, @@ -2251,7 +2259,8 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.EditAndContinue Return Case SyntaxKind.ImportsStatement - ReportError(RudeEditKind.Update) + ' We don't report rude edits for updating imports, though note we don't currently report edits + ' for semantic changes they cause either Return Case SyntaxKind.NamespaceBlock