Merge pull request #1989 from mavasani/WarningAnnotation

Show warning annotation in MakeMemberStatic code fix preview if all r…

src/Microsoft.CodeQuality.Analyzers/Core/QualityGuidelines/MarkMembersAsStatic.Fixer.cs

@@ -3,13 +3,13 @@
 using System;
 using System.Collections.Generic;
 using System.Collections.Immutable;
-using System.Composition;
 using System.Linq;
 using System.Threading;
 using System.Threading.Tasks;
 using Analyzer.Utilities;
 using Analyzer.Utilities.Extensions;
 using Microsoft.CodeAnalysis;
+using Microsoft.CodeAnalysis.CodeActions;
 using Microsoft.CodeAnalysis.CodeFixes;
 using Microsoft.CodeAnalysis.Editing;
 using Microsoft.CodeAnalysis.FindSymbols;
@@ -23,6 +23,8 @@ namespace Microsoft.CodeQuality.Analyzers.QualityGuidelines
     /// </summary>
     public abstract class MarkMembersAsStaticFixer : CodeFixProvider
     {
+        private static readonly SyntaxAnnotation s_annotationForFixedDeclaration = new SyntaxAnnotation();
+
         protected abstract IEnumerable<SyntaxNode> GetTypeArguments(SyntaxNode node);
         protected abstract SyntaxNode GetExpressionOfInvocation(SyntaxNode invocation);
         protected virtual SyntaxNode GetSyntaxNodeToReplace(IMemberReferenceOperation memberReference)
@@ -57,7 +59,7 @@ private async Task<Solution> MakeStaticAsync(Document document, SyntaxNode root,
 
             // Update definition to add static modifier.
             var syntaxGenerator = SyntaxGenerator.GetGenerator(document);
-            var madeStatic = syntaxGenerator.WithModifiers(node, DeclarationModifiers.Static);
+            var madeStatic = syntaxGenerator.WithModifiers(node, DeclarationModifiers.Static).WithAdditionalAnnotations(s_annotationForFixedDeclaration);
             document = document.WithSyntaxRoot(root.ReplaceNode(node, madeStatic));
             var solution = document.Project.Solution;
 
@@ -68,22 +70,39 @@ private async Task<Solution> MakeStaticAsync(Document document, SyntaxNode root,
             var symbol = semanticModel.GetDeclaredSymbol(node, cancellationToken);
             if (symbol != null)
             {
-                solution = await UpdateReferencesAsync(symbol, solution, cancellationToken).ConfigureAwait(false);
+                var (newSolution, allReferencesFixed) = await UpdateReferencesAsync(symbol, solution, cancellationToken).ConfigureAwait(false);
+                solution = newSolution;
+
+                if (!allReferencesFixed)
+                {
+                    // We could not fix all references, so add a warning annotation that users need to manually fix these.
+                    document = await AddWarningAnnotation(solution.GetDocument(document.Id), symbol, cancellationToken).ConfigureAwait(false);
+                    solution = document.Project.Solution;
+                }
             }
 
             return solution;
         }
 
-        private async Task<Solution> UpdateReferencesAsync(ISymbol symbol, Solution solution, CancellationToken cancellationToken)
+        /// <summary>
+        /// Returns the updated solution and a flag indicating if all references were fixed or not.
+        /// </summary>
+        private async Task<(Solution newSolution, bool allReferencesFixed)> UpdateReferencesAsync(ISymbol symbol, Solution solution, CancellationToken cancellationToken)
         {
             cancellationToken.ThrowIfCancellationRequested();
 
             var references = await SymbolFinder.FindReferencesAsync(symbol, solution, cancellationToken).ConfigureAwait(false);
+
+            // Filter out cascaded symbol references. For example, accessor references for property symbol.
+            references = references.Where(r => symbol.Equals(r.Definition));
+
             if (references.Count() != 1)
             {
-                return solution;
+                return (newSolution: solution, allReferencesFixed: references.Count() == 0);
             }
 
+            var allReferencesFixed = true;
+
             // Group references by document and fix references in each document.
             foreach (var referenceLocationGroup in references.Single().Locations.GroupBy(r => r.Document))
             {
@@ -94,6 +113,7 @@ private async Task<Solution> UpdateReferencesAsync(ISymbol symbol, Solution solu
                 // https://github.com/dotnet/roslyn-analyzers/issues/1986 tracks handling them.
                 if (!document.Project.Language.Equals(symbol.Language, StringComparison.Ordinal))
                 {
+                    allReferencesFixed = false;
                     continue;
                 }
 
@@ -107,62 +127,69 @@ private async Task<Solution> UpdateReferencesAsync(ISymbol symbol, Solution solu
                     cancellationToken.ThrowIfCancellationRequested();
 
                     var referenceNode = root.FindNode(referenceLocation.Location.SourceSpan, getInnermostNodeForTie: true);
-                    if (referenceNode != null)
+                    if (referenceNode == null)
                     {
-                        var operation = semanticModel.GetOperationWalkingUpParentChain(referenceNode, cancellationToken);
-                        SyntaxNode nodeToReplaceOpt = null;
-                        switch (operation)
-                        {
-                            case IMemberReferenceOperation memberReference:
-                                if (IsReplacableOperation(memberReference.Instance))
-                                {
-                                    nodeToReplaceOpt = GetSyntaxNodeToReplace(memberReference);
-                                }
-
-                                break;
-
-                            case IInvocationOperation invocation:
-                                if (IsReplacableOperation(invocation.Instance))
-                                {
-                                    nodeToReplaceOpt = GetExpressionOfInvocation(invocation.Syntax);
-                                }
-
-                                break;
-                        }
-
-                        if (nodeToReplaceOpt != null)
-                        {
-                            // Fetch the symbol for the node to replace - note that this might be
-                            // different from the original symbol due to generic type arguments.
-                            var symbolInfo = semanticModel.GetSymbolInfo(nodeToReplaceOpt, cancellationToken);
-                            if (symbolInfo.Symbol == null)
+                        allReferencesFixed = false;
+                        continue;
+                    }
+
+                    var operation = semanticModel.GetOperationWalkingUpParentChain(referenceNode, cancellationToken);
+                    SyntaxNode nodeToReplaceOpt = null;
+                    switch (operation)
+                    {
+                        case IMemberReferenceOperation memberReference:
+                            if (IsReplacableOperation(memberReference.Instance))
                             {
-                                continue;
+                                nodeToReplaceOpt = GetSyntaxNodeToReplace(memberReference);
                             }
 
-                            SyntaxNode memberName;
-                            var typeArgumentsOpt = GetTypeArguments(referenceNode);
-                            memberName = typeArgumentsOpt != null ?
-                                editor.Generator.GenericName(symbolInfo.Symbol.Name, typeArgumentsOpt) :
-                                editor.Generator.IdentifierName(symbolInfo.Symbol.Name);
-
-                            var newNode = editor.Generator.MemberAccessExpression(
-                                    expression: editor.Generator.TypeExpression(symbolInfo.Symbol.ContainingType),
-                                    memberName: memberName)
-                                .WithLeadingTrivia(nodeToReplaceOpt.GetLeadingTrivia())
-                                .WithTrailingTrivia(nodeToReplaceOpt.GetTrailingTrivia())
-                                .WithAdditionalAnnotations(Formatter.Annotation);
-
-                            editor.ReplaceNode(nodeToReplaceOpt, newNode);
-                        }
+                            break;
+
+                        case IInvocationOperation invocation:
+                            if (IsReplacableOperation(invocation.Instance))
+                            {
+                                nodeToReplaceOpt = GetExpressionOfInvocation(invocation.Syntax);
+                            }
+
+                            break;
+                    }
+
+                    if (nodeToReplaceOpt == null)
+                    {
+                        allReferencesFixed = false;
+                        continue;
                     }
+
+                    // Fetch the symbol for the node to replace - note that this might be
+                    // different from the original symbol due to generic type arguments.
+                    var symbolForNodeToReplace = GetSymbolForNodeToReplace(nodeToReplaceOpt, semanticModel);
+                    if (symbolForNodeToReplace == null)
+                    {
+                        allReferencesFixed = false;
+                        continue;
+                    }
+
+                    SyntaxNode memberName;
+                    var typeArgumentsOpt = GetTypeArguments(referenceNode);
+                    memberName = typeArgumentsOpt != null ?
+                        editor.Generator.GenericName(symbolForNodeToReplace.Name, typeArgumentsOpt) :
+                        editor.Generator.IdentifierName(symbolForNodeToReplace.Name);
+
+                    var newNode = editor.Generator.MemberAccessExpression(
+                            expression: editor.Generator.TypeExpression(symbolForNodeToReplace.ContainingType),
+                            memberName: memberName)
+                        .WithLeadingTrivia(nodeToReplaceOpt.GetLeadingTrivia())
+                        .WithTrailingTrivia(nodeToReplaceOpt.GetTrailingTrivia())
+                        .WithAdditionalAnnotations(Formatter.Annotation);
+
+                    editor.ReplaceNode(nodeToReplaceOpt, newNode);
                 }
 
                 document = document.WithSyntaxRoot(editor.GetChangedRoot());
                 solution = document.Project.Solution;
             }
 
-            return solution;
+            return (solution, allReferencesFixed);
 
             // Local functions.
             bool IsReplacableOperation(IOperation operation)
@@ -185,6 +212,30 @@ bool IsReplacableOperation(IOperation operation)
 
                 return false;
             }
+
+            ISymbol GetSymbolForNodeToReplace(SyntaxNode nodeToReplace, SemanticModel semanticModel)
+            {
+                var symbolInfo = semanticModel.GetSymbolInfo(nodeToReplace, cancellationToken);
+                var symbolForNodeToReplace = symbolInfo.Symbol;
+
+                if (symbolForNodeToReplace == null &&
+                    symbolInfo.CandidateReason == CandidateReason.StaticInstanceMismatch &&
+                    symbolInfo.CandidateSymbols.Length == 1)
+                {
+                    return symbolInfo.CandidateSymbols[0];
+                }
+
+                return symbolForNodeToReplace;
+            }
+        }
+
+        private static async Task<Document> AddWarningAnnotation(Document document, ISymbol symbolFromEarlierSnapshot, CancellationToken cancellationToken)
+        {
+            var semanticModel = await document.GetSemanticModelAsync(cancellationToken).ConfigureAwait(false);
+            var root = await document.GetSyntaxRootAsync(cancellationToken).ConfigureAwait(false);
+            var fixedDeclaration = root.DescendantNodes().Single(n => n.HasAnnotation(s_annotationForFixedDeclaration));
+            var annotation = WarningAnnotation.Create(string.Format(MicrosoftQualityGuidelinesAnalyzersResources.MarkMembersAsStaticCodeFix_WarningAnnotation, symbolFromEarlierSnapshot.Name));
+            return document.WithSyntaxRoot(root.ReplaceNode(fixedDeclaration, fixedDeclaration.WithAdditionalAnnotations(annotation)));
         }
 
         private class MarkMembersAsStaticAction : SolutionChangeAction

src/Microsoft.CodeQuality.Analyzers/Core/QualityGuidelines/MicrosoftQualityGuidelinesAnalyzersResources.resx

@@ -234,4 +234,7 @@
   <data name="MarkMembersAsStaticCodeFix" xml:space="preserve">
     <value>Make static</value>
   </data>
+  <data name="MarkMembersAsStaticCodeFix_WarningAnnotation" xml:space="preserve">
+    <value>Some references to '{0}' could not be fixed, they should be fixed manually.</value>
+  </data>
 </root>

src/Microsoft.CodeQuality.Analyzers/Core/QualityGuidelines/xlf/MicrosoftQualityGuidelinesAnalyzersResources.cs.xlf

@@ -7,6 +7,11 @@
         <target state="new">Make static</target>
         <note />
       </trans-unit>
+      <trans-unit id="MarkMembersAsStaticCodeFix_WarningAnnotation">
+        <source>Some references to '{0}' could not be fixed, they should be fixed manually.</source>
+        <target state="new">Some references to '{0}' could not be fixed, they should be fixed manually.</target>
+        <note />
+      </trans-unit>
       <trans-unit id="UseLiteralsWhereAppropriateTitle">
         <source>Use literals where appropriate</source>
         <target state="translated">Tam, kde je to vhodné, použijte literály</target>

src/Microsoft.CodeQuality.Analyzers/Core/QualityGuidelines/xlf/MicrosoftQualityGuidelinesAnalyzersResources.de.xlf

@@ -7,6 +7,11 @@
         <target state="new">Make static</target>
         <note />
       </trans-unit>
+      <trans-unit id="MarkMembersAsStaticCodeFix_WarningAnnotation">
+        <source>Some references to '{0}' could not be fixed, they should be fixed manually.</source>
+        <target state="new">Some references to '{0}' could not be fixed, they should be fixed manually.</target>
+        <note />
+      </trans-unit>
       <trans-unit id="UseLiteralsWhereAppropriateTitle">
         <source>Use literals where appropriate</source>
         <target state="translated">Nach Möglichkeit Literale verwenden</target>

src/Microsoft.CodeQuality.Analyzers/Core/QualityGuidelines/xlf/MicrosoftQualityGuidelinesAnalyzersResources.es.xlf

@@ -7,6 +7,11 @@
         <target state="new">Make static</target>
         <note />
       </trans-unit>
+      <trans-unit id="MarkMembersAsStaticCodeFix_WarningAnnotation">
+        <source>Some references to '{0}' could not be fixed, they should be fixed manually.</source>
+        <target state="new">Some references to '{0}' could not be fixed, they should be fixed manually.</target>
+        <note />
+      </trans-unit>
       <trans-unit id="UseLiteralsWhereAppropriateTitle">
         <source>Use literals where appropriate</source>
         <target state="translated">Usar literales cuando resulte apropiado</target>

src/Microsoft.CodeQuality.Analyzers/Core/QualityGuidelines/xlf/MicrosoftQualityGuidelinesAnalyzersResources.fr.xlf

@@ -7,6 +7,11 @@
         <target state="new">Make static</target>
         <note />
       </trans-unit>
+      <trans-unit id="MarkMembersAsStaticCodeFix_WarningAnnotation">
+        <source>Some references to '{0}' could not be fixed, they should be fixed manually.</source>
+        <target state="new">Some references to '{0}' could not be fixed, they should be fixed manually.</target>
+        <note />
+      </trans-unit>
       <trans-unit id="UseLiteralsWhereAppropriateTitle">
         <source>Use literals where appropriate</source>
         <target state="translated">Utiliser des littéraux quand cela est approprié</target>

src/Microsoft.CodeQuality.Analyzers/Core/QualityGuidelines/xlf/MicrosoftQualityGuidelinesAnalyzersResources.it.xlf

@@ -7,6 +7,11 @@
         <target state="new">Make static</target>
         <note />
       </trans-unit>
+      <trans-unit id="MarkMembersAsStaticCodeFix_WarningAnnotation">
+        <source>Some references to '{0}' could not be fixed, they should be fixed manually.</source>
+        <target state="new">Some references to '{0}' could not be fixed, they should be fixed manually.</target>
+        <note />
+      </trans-unit>
       <trans-unit id="UseLiteralsWhereAppropriateTitle">
         <source>Use literals where appropriate</source>
         <target state="translated">Usa valori letterali dove appropriato</target>

src/Microsoft.CodeQuality.Analyzers/Core/QualityGuidelines/xlf/MicrosoftQualityGuidelinesAnalyzersResources.ja.xlf

@@ -7,6 +7,11 @@
         <target state="new">Make static</target>
         <note />
       </trans-unit>
+      <trans-unit id="MarkMembersAsStaticCodeFix_WarningAnnotation">
+        <source>Some references to '{0}' could not be fixed, they should be fixed manually.</source>
+        <target state="new">Some references to '{0}' could not be fixed, they should be fixed manually.</target>
+        <note />
+      </trans-unit>
       <trans-unit id="UseLiteralsWhereAppropriateTitle">
         <source>Use literals where appropriate</source>
         <target state="translated">適切な場所にリテラルを使用します</target>

src/Microsoft.CodeQuality.Analyzers/Core/QualityGuidelines/xlf/MicrosoftQualityGuidelinesAnalyzersResources.ko.xlf

@@ -7,6 +7,11 @@
         <target state="new">Make static</target>
         <note />
       </trans-unit>
+      <trans-unit id="MarkMembersAsStaticCodeFix_WarningAnnotation">
+        <source>Some references to '{0}' could not be fixed, they should be fixed manually.</source>
+        <target state="new">Some references to '{0}' could not be fixed, they should be fixed manually.</target>
+        <note />
+      </trans-unit>
       <trans-unit id="UseLiteralsWhereAppropriateTitle">
         <source>Use literals where appropriate</source>
         <target state="translated">적합한 리터럴을 사용하세요.</target>

src/Microsoft.CodeQuality.Analyzers/Core/QualityGuidelines/xlf/MicrosoftQualityGuidelinesAnalyzersResources.pl.xlf

@@ -7,6 +7,11 @@
         <target state="new">Make static</target>
         <note />
       </trans-unit>
+      <trans-unit id="MarkMembersAsStaticCodeFix_WarningAnnotation">
+        <source>Some references to '{0}' could not be fixed, they should be fixed manually.</source>
+        <target state="new">Some references to '{0}' could not be fixed, they should be fixed manually.</target>
+        <note />
+      </trans-unit>
       <trans-unit id="UseLiteralsWhereAppropriateTitle">
         <source>Use literals where appropriate</source>
         <target state="translated">Używaj literałów w odpowiednich miejscach</target>

src/Microsoft.CodeQuality.Analyzers/Core/QualityGuidelines/xlf/MicrosoftQualityGuidelinesAnalyzersResources.pt-BR.xlf

@@ -7,6 +7,11 @@
         <target state="new">Make static</target>
         <note />
       </trans-unit>
+      <trans-unit id="MarkMembersAsStaticCodeFix_WarningAnnotation">
+        <source>Some references to '{0}' could not be fixed, they should be fixed manually.</source>
+        <target state="new">Some references to '{0}' could not be fixed, they should be fixed manually.</target>
+        <note />
+      </trans-unit>
       <trans-unit id="UseLiteralsWhereAppropriateTitle">
         <source>Use literals where appropriate</source>
         <target state="translated">Usar literais sempre que apropriado</target>

src/Microsoft.CodeQuality.Analyzers/Core/QualityGuidelines/xlf/MicrosoftQualityGuidelinesAnalyzersResources.ru.xlf

@@ -7,6 +7,11 @@
         <target state="new">Make static</target>
         <note />
       </trans-unit>
+      <trans-unit id="MarkMembersAsStaticCodeFix_WarningAnnotation">
+        <source>Some references to '{0}' could not be fixed, they should be fixed manually.</source>
+        <target state="new">Some references to '{0}' could not be fixed, they should be fixed manually.</target>
+        <note />
+      </trans-unit>
       <trans-unit id="UseLiteralsWhereAppropriateTitle">
         <source>Use literals where appropriate</source>
         <target state="translated">Используйте литералы, когда это уместно</target>

src/Microsoft.CodeQuality.Analyzers/Core/QualityGuidelines/xlf/MicrosoftQualityGuidelinesAnalyzersResources.tr.xlf

@@ -7,6 +7,11 @@
         <target state="new">Make static</target>
         <note />
       </trans-unit>
+      <trans-unit id="MarkMembersAsStaticCodeFix_WarningAnnotation">
+        <source>Some references to '{0}' could not be fixed, they should be fixed manually.</source>
+        <target state="new">Some references to '{0}' could not be fixed, they should be fixed manually.</target>
+        <note />
+      </trans-unit>
       <trans-unit id="UseLiteralsWhereAppropriateTitle">
         <source>Use literals where appropriate</source>
         <target state="translated">Uygun durumlarda sabit değerler kullanın</target>

src/Microsoft.CodeQuality.Analyzers/Core/QualityGuidelines/xlf/MicrosoftQualityGuidelinesAnalyzersResources.zh-Hans.xlf

@@ -7,6 +7,11 @@
         <target state="new">Make static</target>
         <note />
       </trans-unit>
+      <trans-unit id="MarkMembersAsStaticCodeFix_WarningAnnotation">
+        <source>Some references to '{0}' could not be fixed, they should be fixed manually.</source>
+        <target state="new">Some references to '{0}' could not be fixed, they should be fixed manually.</target>
+        <note />
+      </trans-unit>
       <trans-unit id="UseLiteralsWhereAppropriateTitle">
         <source>Use literals where appropriate</source>
         <target state="translated">在合适的位置使用文本</target>