Merge pull request #2843 from dpoeschl/RenameConflictsInNameOf

Improve rename conflicts in nameof

src/EditorFeatures/Test2/Rename/RenameEngineTests.CSharpConflicts.vb

@@ -3197,5 +3197,114 @@ class C
                 result.AssertLabeledSpansAre("conflict", "a", RelatedLocationType.UnresolvedConflict)
             End Using
         End Sub
+
+        <WorkItem(446, "https://github.com/dotnet/roslyn/issues/446")>
+        <Fact>
+        <Trait(Traits.Feature, Traits.Features.Rename)>
+        Public Sub NoCrashOrConflictOnRenameWithNameOfInAttribute()
+            Using result = RenameEngineResult.Create(
+                   <Workspace>
+                       <Project Language="C#" CommonReferences="true">
+                           <Document>
+class C
+{
+    static void [|T|]$$(int x) { }
+
+    [System.Obsolete(nameof(Test))]
+    static void Test() { }
+}
+                            </Document>
+                       </Project>
+                   </Workspace>, renameTo:="Test")
+            End Using
+        End Sub
+
+        <WorkItem(1195, "https://github.com/dotnet/roslyn/issues/1195")>
+        <Fact>
+        <Trait(Traits.Feature, Traits.Features.Rename)>
+        Public Sub ConflictWhenNameOfReferenceDoesNotBindToAnyOriginalSymbols()
+            Using result = RenameEngineResult.Create(
+                   <Workspace>
+                       <Project Language="C#" CommonReferences="true">
+                           <Document>
+class C
+{
+    void Test()
+    {
+        int [|T$$|];
+        var x = nameof({|conflict:Test|});
+    }
+}
+                            </Document>
+                       </Project>
+                   </Workspace>, renameTo:="Test")
+
+                result.AssertLabeledSpansAre("conflict", "Test", RelatedLocationType.UnresolvedConflict)
+            End Using
+        End Sub
+
+        <WorkItem(1195, "https://github.com/dotnet/roslyn/issues/1195")>
+        <Fact>
+        <Trait(Traits.Feature, Traits.Features.Rename)>
+        Public Sub NoConflictWhenNameOfReferenceDoesNotBindToSomeOriginalSymbols()
+            Using result = RenameEngineResult.Create(
+                   <Workspace>
+                       <Project Language="C#" CommonReferences="true">
+                           <Document>
+class Program
+{
+    void [|$$M|](int x) { }
+    void M() { var x = nameof(M); }
+}
+                            </Document>
+                       </Project>
+                   </Workspace>, renameTo:="X")
+            End Using
+        End Sub
+
+        <WorkItem(1195, "https://github.com/dotnet/roslyn/issues/1195")>
+        <Fact>
+        <Trait(Traits.Feature, Traits.Features.Rename)>
+        Public Sub NoConflictWhenNameOfReferenceBindsToSymbolForFirstTime()
+            Using result = RenameEngineResult.Create(
+                   <Workspace>
+                       <Project Language="C#" CommonReferences="true">
+                           <Document>
+class Program
+{
+    void [|X$$|]() { }
+    void M() { var x = nameof(T); }
+}
+                            </Document>
+                       </Project>
+                   </Workspace>, renameTo:="T")
+            End Using
+        End Sub
+
+        <WorkItem(1195, "https://github.com/dotnet/roslyn/issues/1195")>
+        <Fact>
+        <Trait(Traits.Feature, Traits.Features.Rename)>
+        Public Sub ConflictWhenNameOfReferenceChangesBindingFromMetadataToSource()
+            Using result = RenameEngineResult.Create(
+                   <Workspace>
+                       <Project Language="C#" CommonReferences="true">
+                           <Document>
+using System;
+
+class Program
+{
+    static void M()
+    {
+        var [|Consol$$|] = 7;
+        var x = nameof({|conflict:Console|});
+    }
+}
+                            </Document>
+                       </Project>
+                   </Workspace>, renameTo:="Console")
+
+                result.AssertLabeledSpansAre("conflict", "Console", RelatedLocationType.UnresolvedConflict)
+            End Using
+        End Sub
     End Class
 End Namespace

src/EditorFeatures/Test2/Rename/RenameEngineTests.VisualBasicConflicts.vb

@@ -2969,6 +2969,50 @@ End Namespace
                     </Workspace>, renameTo:="N2")
                 End Using
             End Sub
+
+            <WorkItem(1195, "https://github.com/dotnet/roslyn/issues/1195")>
+            <Fact, Trait(Traits.Feature, Traits.Features.Rename)>
+            Public Sub NameOfReferenceNoConflict()
+                Using result = RenameEngineResult.Create(
+                    <Workspace>
+                        <Project Language="Visual Basic" CommonReferences="true">
+                            <Document FilePath="Test.cs"><![CDATA[
+Class C
+    Sub [|T$$|](x As Integer)
+    End Sub
+
+    Sub Test()
+        Dim x = NameOf(Test)
+    End Sub
+End Class
+]]>
+                            </Document>
+                        </Project>
+                    </Workspace>, renameTo:="Test")
+                End Using
+            End Sub
+
+            <WorkItem(1195, "https://github.com/dotnet/roslyn/issues/1195")>
+            <Fact, Trait(Traits.Feature, Traits.Features.Rename)>
+            Public Sub NameOfReferenceWithConflict()
+                Using result = RenameEngineResult.Create(
+                    <Workspace>
+                        <Project Language="Visual Basic" CommonReferences="true">
+                            <Document FilePath="Test.cs"><![CDATA[
+Class C
+    Sub Test()
+        Dim [|T$$|] As Integer
+        Dim x = NameOf({|conflict:Test|})
+    End Sub
+End Class
+]]>
+                            </Document>
+                        </Project>
+                    </Workspace>, renameTo:="Test")
+
+                    result.AssertLabeledSpansAre("conflict", "Test", RelatedLocationType.UnresolvedConflict)
+                End Using
+            End Sub
         End Class
     End Class
 End Namespace

src/EditorFeatures/Test2/Rename/RenameEngineTests.vb

@@ -6768,29 +6768,6 @@ class C
 
             End Using
         End Sub
-
-        <WorkItem(446, "https://github.com/dotnet/roslyn/issues/446")>
-        <Fact>
-        <Trait(Traits.Feature, Traits.Features.Rename)>
-        Public Sub RenameWithNameOfInAttribute()
-            Using result = RenameEngineResult.Create(
-                   <Workspace>
-                       <Project Language="C#" CommonReferences="true">
-                           <Document>
-class C
-{
-    // Rename F to Fail
-    static void [|F|]$$(int x) { }
-
-    [System.Obsolete(nameof({|conflict:Fail|}))]
-    static void Fail() { }
-}
-                            </Document>
-                       </Project>
-                   </Workspace>, renameTo:="Fail")
-                result.AssertLabeledSpansAre("conflict", type:=RelatedLocationType.UnresolvedConflict)
-            End Using
-        End Sub
 #End Region
 
     End Class

src/Workspaces/CSharp/Portable/Rename/CSharpRenameRewriterLanguageService.cs

@@ -63,6 +63,7 @@ private class RenameRewriter : CSharpSyntaxRewriter
             private readonly bool _isRenamingInComments;
             private readonly ISet<TextSpan> _stringAndCommentTextSpans;
             private readonly ISimplificationService _simplificationService;
+            private readonly ISemanticFactsService _semanticFactsService;
             private readonly HashSet<SyntaxToken> _annotatedIdentifierTokens = new HashSet<SyntaxToken>();
             private readonly HashSet<InvocationExpressionSyntax> _invocationExpressionsNeedingConflictChecks = new HashSet<InvocationExpressionSyntax>();
 
@@ -122,6 +123,7 @@ public RenameRewriter(RenameRewriterParameters parameters)
                 _isVerbatim = _replacementText.StartsWith("@", StringComparison.Ordinal);
 
                 _simplificationService = parameters.Document.Project.LanguageServices.GetService<ISimplificationService>();
+                _semanticFactsService = parameters.Document.Project.LanguageServices.GetService<ISemanticFactsService>();
             }
 
             public override SyntaxNode Visit(SyntaxNode node)
@@ -376,6 +378,8 @@ private async Task<SyntaxToken> RenameAndAnnotateAsync(SyntaxToken token, Syntax
                         isNamespaceDeclarationReference = true;
                     }
 
+                    var isMemberGroupReference = _semanticFactsService.IsNameOfContext(_semanticModel, token.Span.Start, _cancellationToken);
+
                     var renameAnnotation =
                             new RenameActionAnnotation(
                                 token.Span,
@@ -385,7 +389,8 @@ private async Task<SyntaxToken> RenameAndAnnotateAsync(SyntaxToken token, Syntax
                                 renameDeclarationLocations: renameDeclarationLocations,
                                 isOriginalTextLocation: isOldText,
                                 isNamespaceDeclarationReference: isNamespaceDeclarationReference,
-                                isInvocationExpression: false);
+                                isInvocationExpression: false,
+                                isMemberGroupReference: isMemberGroupReference);
 
                     newToken = _renameAnnotations.WithAdditionalAnnotations(newToken, renameAnnotation, new RenameTokenSimplificationAnnotation() { OriginalTextSpan = token.Span });
 
@@ -465,7 +470,8 @@ private RenameActionAnnotation GetAnnotationForInvocationExpression(InvocationEx
                                                 renameDeclarationLocations: renameDeclarationLocations,
                                                 isOriginalTextLocation: false,
                                                 isNamespaceDeclarationReference: false,
-                                                isInvocationExpression: true);
+                                                isInvocationExpression: true,
+                                                isMemberGroupReference: false);
 
                     return renameAnnotation;
                 }

src/Workspaces/Core/Portable/Rename/ConflictEngine/Annotations/RenameActionAnnotation.cs

@@ -50,6 +50,11 @@ internal class RenameActionAnnotation : RenameAnnotation
         /// </summary>
         public readonly bool IsNamespaceDeclarationReference;
 
+        /// <summary>
+        /// States if this token is a member group reference, typically found in NameOf expressions
+        /// </summary>
+        public readonly bool IsMemberGroupReference;
+
         /// <summary>
         /// States if this token is annotated as a part of the Invocation Expression that needs to be checked for the Conflicts
         /// </summary>
@@ -63,7 +68,8 @@ internal class RenameActionAnnotation : RenameAnnotation
             bool isOriginalTextLocation,
             RenameDeclarationLocationReference[] renameDeclarationLocations,
             bool isNamespaceDeclarationReference,
-            bool isInvocationExpression)
+            bool isInvocationExpression,
+            bool isMemberGroupReference)
         {
             this.OriginalSpan = originalSpan;
             this.IsRenameLocation = isRenameLocation;
@@ -73,6 +79,7 @@ internal class RenameActionAnnotation : RenameAnnotation
             this.IsOriginalTextLocation = isOriginalTextLocation;
             this.IsNamespaceDeclarationReference = isNamespaceDeclarationReference;
             this.IsInvocationExpression = isInvocationExpression;
+            this.IsMemberGroupReference = isMemberGroupReference;
         }
     }
 }

src/Workspaces/Core/Portable/Rename/ConflictEngine/ConflictResolver.Session.cs

@@ -413,6 +413,32 @@ private void DebugVerifyNoErrors(ConflictResolution conflictResolution, IEnumera
                 {
                     hasConflict = false;
                 }
+                else if (conflictAnnotation.IsMemberGroupReference)
+                {
+                    if (!conflictAnnotation.RenameDeclarationLocationReferences.Any())
+                    {
+                        hasConflict = false;
+                    }
+                    else
+                    {
+                        // Ensure newReferencedSymbols contains at least one of the original referenced
+                        // symbols, and allow any new symbols to be added to the set of references.
+
+                        hasConflict = true;
+
+                        var newLocationTasks = newReferencedSymbols.Select(async symbol => await GetSymbolLocationAsync(solution, symbol, _cancellationToken).ConfigureAwait(false));
+                        var newLocations = (await Task.WhenAll(newLocationTasks).ConfigureAwait(false)).Where(loc => loc != null && loc.IsInSource);
+                        foreach (var originalReference in conflictAnnotation.RenameDeclarationLocationReferences.Where(loc => loc.IsSourceLocation))
+                        {
+                            var adjustedStartPosition = conflictResolution.GetAdjustedTokenStartingPosition(originalReference.TextSpan.Start, originalReference.DocumentId);
+                            if (newLocations.Any(loc => loc.SourceSpan.Start == adjustedStartPosition))
+                            {
+                                hasConflict = false;
+                                break;
+                            }
+                        }
+                    }
+                }
                 else if (!conflictAnnotation.IsRenameLocation && conflictAnnotation.IsOriginalTextLocation && conflictAnnotation.RenameDeclarationLocationReferences.Length > 1 && newReferencedSymbols.Count() == 1)
                 {
                     // an ambiguous situation was resolved through rename in non reference locations

src/Workspaces/VisualBasic/Portable/Rename/VisualBasicRenameRewriterLanguageService.vb

@@ -58,6 +58,7 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.Rename
             Private ReadOnly _annotatedIdentifierTokens As New HashSet(Of SyntaxToken)
             Private ReadOnly _invocationExpressionsNeedingConflictChecks As New HashSet(Of InvocationExpressionSyntax)
             Private ReadOnly _syntaxFactsService As ISyntaxFactsService
+            Private ReadOnly _semanticFactsService As ISemanticFactsService
             Private ReadOnly _renameAnnotations As AnnotationTable(Of RenameAnnotation)
 
             Private ReadOnly Property AnnotateForComplexification As Boolean
@@ -104,6 +105,7 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.Rename
                 Me._renamableDeclarationLocation = Me._renamedSymbol.Locations.Where(Function(loc) loc.IsInSource AndAlso loc.SourceTree Is _semanticModel.SyntaxTree).FirstOrDefault()
                 Me._simplificationService = parameters.Document.Project.LanguageServices.GetService(Of ISimplificationService)()
                 Me._syntaxFactsService = parameters.Document.Project.LanguageServices.GetService(Of ISyntaxFactsService)()
+                Me._semanticFactsService = parameters.Document.Project.LanguageServices.GetService(Of ISemanticFactsService)()
                 Me._isVerbatim = Me._syntaxFactsService.IsVerbatimIdentifier(_replacementText)
                 Me._renameAnnotations = parameters.RenameAnnotations
             End Sub
@@ -318,6 +320,8 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.Rename
                     isNamespaceDeclarationReference = True
                 End If
 
+                Dim isMemberGroupReference = _semanticFactsService.IsNameOfContext(_semanticModel, token.Span.Start, _cancellationToken)
+
                 Dim renameAnnotation = New RenameActionAnnotation(
                                     token.Span,
                                     isRenameLocation,
@@ -326,7 +330,8 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.Rename
                                     isOldText,
                                     renameDeclarationLocations,
                                     isNamespaceDeclarationReference,
-                                    isInvocationExpression:=False)
+                                    isInvocationExpression:=False,
+                                    isMemberGroupReference:=isMemberGroupReference)
 
                 _annotatedIdentifierTokens.Add(token)
                 newToken = Me._renameAnnotations.WithAdditionalAnnotations(newToken, renameAnnotation, New RenameTokenSimplificationAnnotation() With {.OriginalTextSpan = token.Span})
@@ -453,7 +458,8 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.Rename
                                             renameDeclarationLocations:=renameDeclarationLocations,
                                             isOriginalTextLocation:=False,
                                             isNamespaceDeclarationReference:=False,
-                                            isInvocationExpression:=True)
+                                            isInvocationExpression:=True,
+                                            isMemberGroupReference:=False)
 
                     Return renameAnnotation
                 End If