From 753114b1a1a6b9be0814915c557e065d9c3d0df3 Mon Sep 17 00:00:00 2001 From: Manish Vasani Date: Mon, 7 Jan 2019 10:16:06 -0800 Subject: [PATCH 1/3] Add code fix for MarkMembersStatic (CA1822) I borrowed the code from Fred's original PR #1116, and extended it to fix the references along with the declaration. We conservatively bail out for cases where the replacement can potentially lead to semantic changes - these will be very rare cases and still cause explicit build break, so I think we should be fine. Fixes #471 Fixes #1929 --- .../CSharpMarkMembersAsStatic.Fixer.cs | 7 + .../MarkMembersAsStatic.Fixer.cs | 184 ++- ...ftQualityGuidelinesAnalyzersResources.resx | 3 + ...QualityGuidelinesAnalyzersResources.cs.xlf | 5 + ...QualityGuidelinesAnalyzersResources.de.xlf | 5 + ...QualityGuidelinesAnalyzersResources.es.xlf | 5 + ...QualityGuidelinesAnalyzersResources.fr.xlf | 5 + ...QualityGuidelinesAnalyzersResources.it.xlf | 5 + ...QualityGuidelinesAnalyzersResources.ja.xlf | 5 + ...QualityGuidelinesAnalyzersResources.ko.xlf | 5 + ...QualityGuidelinesAnalyzersResources.pl.xlf | 5 + ...lityGuidelinesAnalyzersResources.pt-BR.xlf | 5 + ...QualityGuidelinesAnalyzersResources.ru.xlf | 5 + ...QualityGuidelinesAnalyzersResources.tr.xlf | 5 + ...tyGuidelinesAnalyzersResources.zh-Hans.xlf | 5 + ...tyGuidelinesAnalyzersResources.zh-Hant.xlf | 5 + .../MarkMembersAsStaticTests.Fixer.cs | 1140 +++++++++++++++++ .../BasicMarkMembersAsStatic.Fixer.vb | 17 + 18 files changed, 1412 insertions(+), 4 deletions(-) diff --git a/src/Microsoft.CodeQuality.Analyzers/CSharp/QualityGuidelines/CSharpMarkMembersAsStatic.Fixer.cs b/src/Microsoft.CodeQuality.Analyzers/CSharp/QualityGuidelines/CSharpMarkMembersAsStatic.Fixer.cs index dab09c021f..a03a91e3a4 100644 --- a/src/Microsoft.CodeQuality.Analyzers/CSharp/QualityGuidelines/CSharpMarkMembersAsStatic.Fixer.cs +++ b/src/Microsoft.CodeQuality.Analyzers/CSharp/QualityGuidelines/CSharpMarkMembersAsStatic.Fixer.cs @@ -1,8 +1,10 @@ // Copyright (c) Microsoft. All Rights Reserved. Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. +using System.Collections.Generic; using System.Composition; using Microsoft.CodeAnalysis; using Microsoft.CodeAnalysis.CodeFixes; +using Microsoft.CodeAnalysis.CSharp.Syntax; using Microsoft.CodeQuality.Analyzers.QualityGuidelines; namespace Microsoft.CodeQuality.CSharp.Analyzers.QualityGuidelines @@ -13,5 +15,10 @@ namespace Microsoft.CodeQuality.CSharp.Analyzers.QualityGuidelines [ExportCodeFixProvider(LanguageNames.CSharp), Shared] public sealed class CSharpMarkMembersAsStaticFixer : MarkMembersAsStaticFixer { + protected override IEnumerable GetTypeArguments(SyntaxNode node) + => (node as GenericNameSyntax)?.TypeArgumentList.Arguments; + + protected override SyntaxNode GetExpressionOfInvocation(SyntaxNode invocation) + => (invocation as InvocationExpressionSyntax)?.Expression; } } \ No newline at end of file diff --git a/src/Microsoft.CodeQuality.Analyzers/Core/QualityGuidelines/MarkMembersAsStatic.Fixer.cs b/src/Microsoft.CodeQuality.Analyzers/Core/QualityGuidelines/MarkMembersAsStatic.Fixer.cs index da57bb7d26..1a5e3174d6 100644 --- a/src/Microsoft.CodeQuality.Analyzers/Core/QualityGuidelines/MarkMembersAsStatic.Fixer.cs +++ b/src/Microsoft.CodeQuality.Analyzers/Core/QualityGuidelines/MarkMembersAsStatic.Fixer.cs @@ -1,9 +1,19 @@ // Copyright (c) Microsoft. All Rights Reserved. Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. 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 Microsoft.CodeAnalysis; using Microsoft.CodeAnalysis.CodeFixes; +using Microsoft.CodeAnalysis.Editing; +using Microsoft.CodeAnalysis.FindSymbols; +using Microsoft.CodeAnalysis.Formatting; +using Microsoft.CodeAnalysis.Operations; namespace Microsoft.CodeQuality.Analyzers.QualityGuidelines { @@ -12,19 +22,185 @@ namespace Microsoft.CodeQuality.Analyzers.QualityGuidelines /// public abstract class MarkMembersAsStaticFixer : CodeFixProvider { + protected abstract IEnumerable GetTypeArguments(SyntaxNode node); + protected abstract SyntaxNode GetExpressionOfInvocation(SyntaxNode invocation); + protected virtual SyntaxNode GetSyntaxNodeToReplace(IMemberReferenceOperation memberReference) + => memberReference.Syntax; + public sealed override ImmutableArray FixableDiagnosticIds => ImmutableArray.Create(MarkMembersAsStaticAnalyzer.RuleId); public sealed override FixAllProvider GetFixAllProvider() { - // See https://github.com/dotnet/roslyn/blob/master/docs/analyzers/FixAllProvider.md for more information on Fix All Providers return WellKnownFixAllProviders.BatchFixer; } - public sealed override Task RegisterCodeFixesAsync(CodeFixContext context) + public sealed override async Task RegisterCodeFixesAsync(CodeFixContext context) + { + var root = await context.Document.GetSyntaxRootAsync(context.CancellationToken).ConfigureAwait(false); + var node = root.FindNode(context.Span); + if (node == null) + { + return; + } + + context.RegisterCodeFix( + new MarkMembersAsStaticAction( + MicrosoftQualityGuidelinesAnalyzersResources.MarkMembersAsStaticCodeFix, + ct => MakeStaticAsync(context.Document, root, node, ct)), + context.Diagnostics); + } + + private async Task MakeStaticAsync(Document document, SyntaxNode root, SyntaxNode node, CancellationToken cancellationToken) + { + cancellationToken.ThrowIfCancellationRequested(); + + // Update definition to add static modifier. + var syntaxGenerator = SyntaxGenerator.GetGenerator(document); + var madeStatic = syntaxGenerator.WithModifiers(node, DeclarationModifiers.Static); + document = document.WithSyntaxRoot(root.ReplaceNode(node, madeStatic)); + var solution = document.Project.Solution; + + // Update references, if any. + root = await document.GetSyntaxRootAsync(cancellationToken).ConfigureAwait(false); + node = root.DescendantNodes().Single(n => n.SpanStart == node.SpanStart && n.Span.Length == madeStatic.Span.Length); + var semanticModel = await document.GetSemanticModelAsync(cancellationToken).ConfigureAwait(false); + var symbol = semanticModel.GetDeclaredSymbol(node, cancellationToken); + if (symbol != null) + { + solution = await UpdateReferencesAsync(symbol, solution, cancellationToken).ConfigureAwait(false); + } + + return solution; + } + + private async Task UpdateReferencesAsync(ISymbol symbol, Solution solution, CancellationToken cancellationToken) { - // Fixer not yet implemented. - return Task.CompletedTask; + cancellationToken.ThrowIfCancellationRequested(); + + var references = await SymbolFinder.FindReferencesAsync(symbol, solution, cancellationToken).ConfigureAwait(false); + if (references?.Count() != 1) + { + return solution; + } + + foreach (var referenceLocationGroup in references.Single().Locations.GroupBy(r => r.Document)) + { + // Get document in current solution + var document = solution.GetDocument(referenceLocationGroup.Key.Id); + var root = await document.GetSyntaxRootAsync(cancellationToken).ConfigureAwait(false); + var semanticModel = await document.GetSemanticModelAsync(cancellationToken).ConfigureAwait(false); + + // Compute replacements + var editor = new SyntaxEditor(root, solution.Workspace); + foreach (var referenceLocation in referenceLocationGroup) + { + cancellationToken.ThrowIfCancellationRequested(); + + var referenceNode = root.FindNode(referenceLocation.Location.SourceSpan, getInnermostNodeForTie: true); + if (referenceNode != null) + { + var operation = GetOperationWalkingUpParentChain(referenceNode, semanticModel); + 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) + { + continue; + } + + 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); + } + } + } + + document = document.WithSyntaxRoot(editor.GetChangedRoot()); + solution = document.Project.Solution; + } + + return solution; + + // Local functions. + IOperation GetOperationWalkingUpParentChain(SyntaxNode node, SemanticModel semanticModel) + { + // Walk up the parent chain to fetch the first non-null operation. + do + { + var operation = semanticModel.GetOperation(node, cancellationToken); + if (operation != null) + { + return operation; + } + + node = node.Parent; + } + while (node != null); + + return null; + } + + bool IsReplacableOperation(IOperation operation) + { + // We only replace reference operations whose removal cannot change semantics. + if (operation != null) + { + switch (operation.Kind) + { + case OperationKind.InstanceReference: + case OperationKind.ParameterReference: + case OperationKind.LocalReference: + return true; + + case OperationKind.FieldReference: + case OperationKind.PropertyReference: + return IsReplacableOperation(((IMemberReferenceOperation)operation).Instance); + } + } + + return false; + } + } + + private class MarkMembersAsStaticAction : SolutionChangeAction + { + public MarkMembersAsStaticAction(string title, Func> createChangedSolution) + : base(title, createChangedSolution, equivalenceKey: title) + { + } } } } \ No newline at end of file diff --git a/src/Microsoft.CodeQuality.Analyzers/Core/QualityGuidelines/MicrosoftQualityGuidelinesAnalyzersResources.resx b/src/Microsoft.CodeQuality.Analyzers/Core/QualityGuidelines/MicrosoftQualityGuidelinesAnalyzersResources.resx index 0e73772bb4..6d6b5a2fba 100644 --- a/src/Microsoft.CodeQuality.Analyzers/Core/QualityGuidelines/MicrosoftQualityGuidelinesAnalyzersResources.resx +++ b/src/Microsoft.CodeQuality.Analyzers/Core/QualityGuidelines/MicrosoftQualityGuidelinesAnalyzersResources.resx @@ -231,4 +231,7 @@ Do not duplicate indexed element initializations + + Make static (Shared in VB) + \ No newline at end of file diff --git a/src/Microsoft.CodeQuality.Analyzers/Core/QualityGuidelines/xlf/MicrosoftQualityGuidelinesAnalyzersResources.cs.xlf b/src/Microsoft.CodeQuality.Analyzers/Core/QualityGuidelines/xlf/MicrosoftQualityGuidelinesAnalyzersResources.cs.xlf index c147053c77..fcafb0e434 100644 --- a/src/Microsoft.CodeQuality.Analyzers/Core/QualityGuidelines/xlf/MicrosoftQualityGuidelinesAnalyzersResources.cs.xlf +++ b/src/Microsoft.CodeQuality.Analyzers/Core/QualityGuidelines/xlf/MicrosoftQualityGuidelinesAnalyzersResources.cs.xlf @@ -2,6 +2,11 @@ + + Make static (Shared in VB) + Make static (Shared in VB) + + Use literals where appropriate Tam, kde je to vhodné, použijte literály diff --git a/src/Microsoft.CodeQuality.Analyzers/Core/QualityGuidelines/xlf/MicrosoftQualityGuidelinesAnalyzersResources.de.xlf b/src/Microsoft.CodeQuality.Analyzers/Core/QualityGuidelines/xlf/MicrosoftQualityGuidelinesAnalyzersResources.de.xlf index f966c17e3d..ae2f400cc8 100644 --- a/src/Microsoft.CodeQuality.Analyzers/Core/QualityGuidelines/xlf/MicrosoftQualityGuidelinesAnalyzersResources.de.xlf +++ b/src/Microsoft.CodeQuality.Analyzers/Core/QualityGuidelines/xlf/MicrosoftQualityGuidelinesAnalyzersResources.de.xlf @@ -2,6 +2,11 @@ + + Make static (Shared in VB) + Make static (Shared in VB) + + Use literals where appropriate Nach Möglichkeit Literale verwenden diff --git a/src/Microsoft.CodeQuality.Analyzers/Core/QualityGuidelines/xlf/MicrosoftQualityGuidelinesAnalyzersResources.es.xlf b/src/Microsoft.CodeQuality.Analyzers/Core/QualityGuidelines/xlf/MicrosoftQualityGuidelinesAnalyzersResources.es.xlf index 9e24bceaf2..9596c54f97 100644 --- a/src/Microsoft.CodeQuality.Analyzers/Core/QualityGuidelines/xlf/MicrosoftQualityGuidelinesAnalyzersResources.es.xlf +++ b/src/Microsoft.CodeQuality.Analyzers/Core/QualityGuidelines/xlf/MicrosoftQualityGuidelinesAnalyzersResources.es.xlf @@ -2,6 +2,11 @@ + + Make static (Shared in VB) + Make static (Shared in VB) + + Use literals where appropriate Usar literales cuando resulte apropiado diff --git a/src/Microsoft.CodeQuality.Analyzers/Core/QualityGuidelines/xlf/MicrosoftQualityGuidelinesAnalyzersResources.fr.xlf b/src/Microsoft.CodeQuality.Analyzers/Core/QualityGuidelines/xlf/MicrosoftQualityGuidelinesAnalyzersResources.fr.xlf index f232eb781f..15c6ca59a4 100644 --- a/src/Microsoft.CodeQuality.Analyzers/Core/QualityGuidelines/xlf/MicrosoftQualityGuidelinesAnalyzersResources.fr.xlf +++ b/src/Microsoft.CodeQuality.Analyzers/Core/QualityGuidelines/xlf/MicrosoftQualityGuidelinesAnalyzersResources.fr.xlf @@ -2,6 +2,11 @@ + + Make static (Shared in VB) + Make static (Shared in VB) + + Use literals where appropriate Utiliser des littéraux quand cela est approprié diff --git a/src/Microsoft.CodeQuality.Analyzers/Core/QualityGuidelines/xlf/MicrosoftQualityGuidelinesAnalyzersResources.it.xlf b/src/Microsoft.CodeQuality.Analyzers/Core/QualityGuidelines/xlf/MicrosoftQualityGuidelinesAnalyzersResources.it.xlf index ab5cf5e1a7..269b530a91 100644 --- a/src/Microsoft.CodeQuality.Analyzers/Core/QualityGuidelines/xlf/MicrosoftQualityGuidelinesAnalyzersResources.it.xlf +++ b/src/Microsoft.CodeQuality.Analyzers/Core/QualityGuidelines/xlf/MicrosoftQualityGuidelinesAnalyzersResources.it.xlf @@ -2,6 +2,11 @@ + + Make static (Shared in VB) + Make static (Shared in VB) + + Use literals where appropriate Usa valori letterali dove appropriato diff --git a/src/Microsoft.CodeQuality.Analyzers/Core/QualityGuidelines/xlf/MicrosoftQualityGuidelinesAnalyzersResources.ja.xlf b/src/Microsoft.CodeQuality.Analyzers/Core/QualityGuidelines/xlf/MicrosoftQualityGuidelinesAnalyzersResources.ja.xlf index 6a8e445fde..eea650bd6a 100644 --- a/src/Microsoft.CodeQuality.Analyzers/Core/QualityGuidelines/xlf/MicrosoftQualityGuidelinesAnalyzersResources.ja.xlf +++ b/src/Microsoft.CodeQuality.Analyzers/Core/QualityGuidelines/xlf/MicrosoftQualityGuidelinesAnalyzersResources.ja.xlf @@ -2,6 +2,11 @@ + + Make static (Shared in VB) + Make static (Shared in VB) + + Use literals where appropriate 適切な場所にリテラルを使用します diff --git a/src/Microsoft.CodeQuality.Analyzers/Core/QualityGuidelines/xlf/MicrosoftQualityGuidelinesAnalyzersResources.ko.xlf b/src/Microsoft.CodeQuality.Analyzers/Core/QualityGuidelines/xlf/MicrosoftQualityGuidelinesAnalyzersResources.ko.xlf index 9d7b6dee3b..f1e7c5cf35 100644 --- a/src/Microsoft.CodeQuality.Analyzers/Core/QualityGuidelines/xlf/MicrosoftQualityGuidelinesAnalyzersResources.ko.xlf +++ b/src/Microsoft.CodeQuality.Analyzers/Core/QualityGuidelines/xlf/MicrosoftQualityGuidelinesAnalyzersResources.ko.xlf @@ -2,6 +2,11 @@ + + Make static (Shared in VB) + Make static (Shared in VB) + + Use literals where appropriate 적합한 리터럴을 사용하세요. diff --git a/src/Microsoft.CodeQuality.Analyzers/Core/QualityGuidelines/xlf/MicrosoftQualityGuidelinesAnalyzersResources.pl.xlf b/src/Microsoft.CodeQuality.Analyzers/Core/QualityGuidelines/xlf/MicrosoftQualityGuidelinesAnalyzersResources.pl.xlf index 9aca720a06..8f477242d9 100644 --- a/src/Microsoft.CodeQuality.Analyzers/Core/QualityGuidelines/xlf/MicrosoftQualityGuidelinesAnalyzersResources.pl.xlf +++ b/src/Microsoft.CodeQuality.Analyzers/Core/QualityGuidelines/xlf/MicrosoftQualityGuidelinesAnalyzersResources.pl.xlf @@ -2,6 +2,11 @@ + + Make static (Shared in VB) + Make static (Shared in VB) + + Use literals where appropriate Używaj literałów w odpowiednich miejscach diff --git a/src/Microsoft.CodeQuality.Analyzers/Core/QualityGuidelines/xlf/MicrosoftQualityGuidelinesAnalyzersResources.pt-BR.xlf b/src/Microsoft.CodeQuality.Analyzers/Core/QualityGuidelines/xlf/MicrosoftQualityGuidelinesAnalyzersResources.pt-BR.xlf index b944a1a345..43c20fa0fa 100644 --- a/src/Microsoft.CodeQuality.Analyzers/Core/QualityGuidelines/xlf/MicrosoftQualityGuidelinesAnalyzersResources.pt-BR.xlf +++ b/src/Microsoft.CodeQuality.Analyzers/Core/QualityGuidelines/xlf/MicrosoftQualityGuidelinesAnalyzersResources.pt-BR.xlf @@ -2,6 +2,11 @@ + + Make static (Shared in VB) + Make static (Shared in VB) + + Use literals where appropriate Usar literais sempre que apropriado diff --git a/src/Microsoft.CodeQuality.Analyzers/Core/QualityGuidelines/xlf/MicrosoftQualityGuidelinesAnalyzersResources.ru.xlf b/src/Microsoft.CodeQuality.Analyzers/Core/QualityGuidelines/xlf/MicrosoftQualityGuidelinesAnalyzersResources.ru.xlf index 497a2f8215..e0e185768e 100644 --- a/src/Microsoft.CodeQuality.Analyzers/Core/QualityGuidelines/xlf/MicrosoftQualityGuidelinesAnalyzersResources.ru.xlf +++ b/src/Microsoft.CodeQuality.Analyzers/Core/QualityGuidelines/xlf/MicrosoftQualityGuidelinesAnalyzersResources.ru.xlf @@ -2,6 +2,11 @@ + + Make static (Shared in VB) + Make static (Shared in VB) + + Use literals where appropriate Используйте литералы, когда это уместно diff --git a/src/Microsoft.CodeQuality.Analyzers/Core/QualityGuidelines/xlf/MicrosoftQualityGuidelinesAnalyzersResources.tr.xlf b/src/Microsoft.CodeQuality.Analyzers/Core/QualityGuidelines/xlf/MicrosoftQualityGuidelinesAnalyzersResources.tr.xlf index c4f577bcd4..33fbdbf070 100644 --- a/src/Microsoft.CodeQuality.Analyzers/Core/QualityGuidelines/xlf/MicrosoftQualityGuidelinesAnalyzersResources.tr.xlf +++ b/src/Microsoft.CodeQuality.Analyzers/Core/QualityGuidelines/xlf/MicrosoftQualityGuidelinesAnalyzersResources.tr.xlf @@ -2,6 +2,11 @@ + + Make static (Shared in VB) + Make static (Shared in VB) + + Use literals where appropriate Uygun durumlarda sabit değerler kullanın diff --git a/src/Microsoft.CodeQuality.Analyzers/Core/QualityGuidelines/xlf/MicrosoftQualityGuidelinesAnalyzersResources.zh-Hans.xlf b/src/Microsoft.CodeQuality.Analyzers/Core/QualityGuidelines/xlf/MicrosoftQualityGuidelinesAnalyzersResources.zh-Hans.xlf index c1b0983122..1a811e1f00 100644 --- a/src/Microsoft.CodeQuality.Analyzers/Core/QualityGuidelines/xlf/MicrosoftQualityGuidelinesAnalyzersResources.zh-Hans.xlf +++ b/src/Microsoft.CodeQuality.Analyzers/Core/QualityGuidelines/xlf/MicrosoftQualityGuidelinesAnalyzersResources.zh-Hans.xlf @@ -2,6 +2,11 @@ + + Make static (Shared in VB) + Make static (Shared in VB) + + Use literals where appropriate 在合适的位置使用文本 diff --git a/src/Microsoft.CodeQuality.Analyzers/Core/QualityGuidelines/xlf/MicrosoftQualityGuidelinesAnalyzersResources.zh-Hant.xlf b/src/Microsoft.CodeQuality.Analyzers/Core/QualityGuidelines/xlf/MicrosoftQualityGuidelinesAnalyzersResources.zh-Hant.xlf index 7db25d63c6..40ecf709d8 100644 --- a/src/Microsoft.CodeQuality.Analyzers/Core/QualityGuidelines/xlf/MicrosoftQualityGuidelinesAnalyzersResources.zh-Hant.xlf +++ b/src/Microsoft.CodeQuality.Analyzers/Core/QualityGuidelines/xlf/MicrosoftQualityGuidelinesAnalyzersResources.zh-Hant.xlf @@ -2,6 +2,11 @@ + + Make static (Shared in VB) + Make static (Shared in VB) + + Use literals where appropriate 適當時機使用常值 diff --git a/src/Microsoft.CodeQuality.Analyzers/UnitTests/QualityGuidelines/MarkMembersAsStaticTests.Fixer.cs b/src/Microsoft.CodeQuality.Analyzers/UnitTests/QualityGuidelines/MarkMembersAsStaticTests.Fixer.cs index 3ad5df6003..f9c7971d89 100644 --- a/src/Microsoft.CodeQuality.Analyzers/UnitTests/QualityGuidelines/MarkMembersAsStaticTests.Fixer.cs +++ b/src/Microsoft.CodeQuality.Analyzers/UnitTests/QualityGuidelines/MarkMembersAsStaticTests.Fixer.cs @@ -5,6 +5,7 @@ using Microsoft.CodeQuality.CSharp.Analyzers.QualityGuidelines; using Microsoft.CodeQuality.VisualBasic.Analyzers.QualityGuidelines; using Test.Utilities; +using Xunit; namespace Microsoft.CodeQuality.Analyzers.QualityGuidelines.UnitTests { @@ -29,5 +30,1144 @@ protected override CodeFixProvider GetCSharpCodeFixProvider() { return new CSharpMarkMembersAsStaticFixer(); } + + [Fact] + public void TestCSharp_SimpleMembers_NoReferences() + { + VerifyCSharpFix(@" +public class MembersTests +{ + internal static int s_field; + public const int Zero = 0; + + public int Method1(string name) + { + return name.Length; + } + + public void Method2() { } + + public void Method3() + { + s_field = 4; + } + + public int Method4() + { + return Zero; + } + + public int Property + { + get { return 5; } + } + + public int Property2 + { + set { s_field = value; } + } + + public int MyProperty + { + get { return 10; } + set { System.Console.WriteLine(value); } + } + + public event System.EventHandler CustomEvent { add {} remove {} } +}", +@" +public class MembersTests +{ + internal static int s_field; + public const int Zero = 0; + + public static int Method1(string name) + { + return name.Length; + } + + public static void Method2() { } + + public static void Method3() + { + s_field = 4; + } + + public static int Method4() + { + return Zero; + } + + public static int Property + { + get { return 5; } + } + + public static int Property2 + { + set { s_field = value; } + } + + public static int MyProperty + { + get { return 10; } + set { System.Console.WriteLine(value); } + } + + public static event System.EventHandler CustomEvent { add {} remove {} } +}"); + } + + [Fact] + public void TestBasic_SimpleMembers_NoReferences() + { + VerifyBasicFix(@" +Imports System +Public Class MembersTests + Shared s_field As Integer + Public Const Zero As Integer = 0 + + Public Function Method1(name As String) As Integer + Return name.Length + End Function + + Public Sub Method2() + End Sub + + Public Sub Method3() + s_field = 4 + End Sub + + Public Function Method4() As Integer + Return Zero + End Function + + Public Property MyProperty As Integer + Get + Return 10 + End Get + Set + System.Console.WriteLine(Value) + End Set + End Property + + Public Custom Event CustomEvent As EventHandler(Of EventArgs) + AddHandler(value As EventHandler(Of EventArgs)) + End AddHandler + RemoveHandler(value As EventHandler(Of EventArgs)) + End RemoveHandler + RaiseEvent(sender As Object, e As EventArgs) + End RaiseEvent + End Event +End Class", +@" +Imports System +Public Class MembersTests + Shared s_field As Integer + Public Const Zero As Integer = 0 + + Public Shared Function Method1(name As String) As Integer + Return name.Length + End Function + + Public Shared Sub Method2() + End Sub + + Public Shared Sub Method3() + s_field = 4 + End Sub + + Public Shared Function Method4() As Integer + Return Zero + End Function + + Public Shared Property MyProperty As Integer + Get + Return 10 + End Get + Set + System.Console.WriteLine(Value) + End Set + End Property + + Public Shared Custom Event CustomEvent As EventHandler(Of EventArgs) + AddHandler(value As EventHandler(Of EventArgs)) + End AddHandler + RemoveHandler(value As EventHandler(Of EventArgs)) + End RemoveHandler + RaiseEvent(sender As Object, e As EventArgs) + End RaiseEvent + End Event +End Class"); + } + + [Fact] + public void TestCSharp_ReferencesInSameType_MemberReferences() + { + VerifyCSharpFix(@" +using System; + +public class C +{ + private C fieldC; + private C PropertyC { get; set; } + + public int M1() + { + return 0; + } + + public void M2(C paramC) + { + var localC = fieldC; + Func m1 = M1, + m2 = paramC.M1, + m3 = localC.M1, + m4 = fieldC.M1, + m5 = PropertyC.M1, + m6 = fieldC.PropertyC.M1, + m7 = this.M1; + } +}", +@" +using System; + +public class C +{ + private C fieldC; + private C PropertyC { get; set; } + + public static int M1() + { + return 0; + } + + public void M2(C paramC) + { + var localC = fieldC; + Func m1 = M1, + m2 = M1, + m3 = M1, + m4 = M1, + m5 = M1, + m6 = M1, + m7 = M1; + } +}"); + } + + [Fact] + public void TestBasic_ReferencesInSameType_MemberReferences() + { + VerifyBasicFix(@" +Imports System + +Public Class C + Private fieldC As C + Private Property PropertyC As C + + Public Function M1() As Integer + Return 0 + End Function + + Public Sub M2(paramC As C) + Dim localC = fieldC + Dim m As Func(Of Integer) = AddressOf M1, + m2 As Func(Of Integer) = AddressOf paramC.M1, + m3 As Func(Of Integer) = AddressOf localC.M1, + m4 As Func(Of Integer) = AddressOf fieldC.M1, + m5 As Func(Of Integer) = AddressOf PropertyC.M1, + m6 As Func(Of Integer) = AddressOf fieldC.PropertyC.M1, + m7 As Func(Of Integer) = AddressOf Me.M1 + End Sub +End Class", +@" +Imports System + +Public Class C + Private fieldC As C + Private Property PropertyC As C + + Public Shared Function M1() As Integer + Return 0 + End Function + + Public Sub M2(paramC As C) + Dim localC = fieldC + Dim m As Func(Of Integer) = AddressOf M1, + m2 As Func(Of Integer) = AddressOf M1, + m3 As Func(Of Integer) = AddressOf M1, + m4 As Func(Of Integer) = AddressOf M1, + m5 As Func(Of Integer) = AddressOf M1, + m6 As Func(Of Integer) = AddressOf M1, + m7 As Func(Of Integer) = AddressOf M1 + End Sub +End Class"); + } + + [Fact] + public void TestCSharp_ReferencesInSameType_Invocations() + { + VerifyCSharpFix(@" +public class C +{ + private int x; + private C fieldC; + private C PropertyC { get; set; } + + public int M1() + { + return 0; + } + + public void M2(C paramC) + { + var localC = fieldC; + x = M1() + paramC.M1() + localC.M1() + fieldC.M1() + PropertyC.M1() + fieldC.PropertyC.M1() + this.M1(); + } +}", +@" +public class C +{ + private int x; + private C fieldC; + private C PropertyC { get; set; } + + public static int M1() + { + return 0; + } + + public void M2(C paramC) + { + var localC = fieldC; + x = M1() + M1() + M1() + M1() + M1() + M1() + M1(); + } +}"); + } + + [Fact] + public void TestBasic_ReferencesInSameType_Invocations() + { + VerifyBasicFix(@" +Public Class C + Private x As Integer + Private fieldC As C + Private Property PropertyC As C + + Public Function M1() As Integer + Return 0 + End Function + + Public Sub M2(paramC As C) + Dim localC = fieldC + x = M1() + paramC.M1() + localC.M1() + fieldC.M1() + PropertyC.M1() + fieldC.PropertyC.M1() + Me.M1() + End Sub +End Class", +@" +Public Class C + Private x As Integer + Private fieldC As C + Private Property PropertyC As C + + Public Shared Function M1() As Integer + Return 0 + End Function + + Public Sub M2(paramC As C) + Dim localC = fieldC + x = M1() + M1() + M1() + M1() + M1() + M1() + M1() + End Sub +End Class"); + } + + [Fact] + public void TestCSharp_ReferencesInSameFile_MemberReferences() + { + VerifyCSharpFix(@" +using System; + +public class C +{ + public C PropertyC { get; set; } + + public int M1() + { + return 0; + } +} + +class C2 +{ + private C fieldC; + private C PropertyC { get; set; } + + public void M2(C paramC) + { + var localC = fieldC; + Func m1 = paramC.M1, + m2 = localC.M1, + m3 = fieldC.M1, + m4 = PropertyC.M1, + m5 = fieldC.PropertyC.M1; + } +}", +@" +using System; + +public class C +{ + public C PropertyC { get; set; } + + public static int M1() + { + return 0; + } +} + +class C2 +{ + private C fieldC; + private C PropertyC { get; set; } + + public void M2(C paramC) + { + var localC = fieldC; + Func m1 = C.M1, + m2 = C.M1, + m3 = C.M1, + m4 = C.M1, + m5 = C.M1; + } +}"); + } + + [Fact] + public void TestCSharp_ReferencesInSameFile_Invocations() + { + VerifyCSharpFix(@" +using System; + +public class C +{ + public C PropertyC { get; set; } + + public int M1() + { + return 0; + } +} + +class C2 +{ + private int x; + private C fieldC; + private C PropertyC { get; set; } + + public void M2(C paramC) + { + var localC = fieldC; + x = paramC.M1() + localC.M1() + fieldC.M1() + PropertyC.M1() + fieldC.PropertyC.M1(); + } +}", +@" +using System; + +public class C +{ + public C PropertyC { get; set; } + + public static int M1() + { + return 0; + } +} + +class C2 +{ + private int x; + private C fieldC; + private C PropertyC { get; set; } + + public void M2(C paramC) + { + var localC = fieldC; + x = C.M1() + C.M1() + C.M1() + C.M1() + C.M1(); + } +}"); + } + + [Fact] + public void TestCSharp_ReferencesInMultipleFiles_MemberReferences() + { + VerifyCSharpFix(new[] { @" +using System; + +public class C +{ + public C PropertyC { get; set; } + + public int M1() + { + return 0; + } +} + +class C2 +{ + private C fieldC; + private C PropertyC { get; set; } + + public void M2(C paramC) + { + var localC = fieldC; + Func m1 = paramC.M1, + m2 = localC.M1, + m3 = fieldC.M1, + m4 = PropertyC.M1, + m5 = fieldC.PropertyC.M1; + } +}", +@" +using System; + +class C3 +{ + private C fieldC; + private C PropertyC { get; set; } + + public void M3(C paramC) + { + var localC = fieldC; + Func m1 = paramC.M1, + m2 = localC.M1, + m3 = fieldC.M1, + m4 = PropertyC.M1, + m5 = fieldC.PropertyC.M1; + } +}" + }, + new[] {@" +using System; + +public class C +{ + public C PropertyC { get; set; } + + public static int M1() + { + return 0; + } +} + +class C2 +{ + private C fieldC; + private C PropertyC { get; set; } + + public void M2(C paramC) + { + var localC = fieldC; + Func m1 = C.M1, + m2 = C.M1, + m3 = C.M1, + m4 = C.M1, + m5 = C.M1; + } +}", +@" +using System; + +class C3 +{ + private C fieldC; + private C PropertyC { get; set; } + + public void M3(C paramC) + { + var localC = fieldC; + Func m1 = C.M1, + m2 = C.M1, + m3 = C.M1, + m4 = C.M1, + m5 = C.M1; + } +}" }); + } + + [Fact] + public void TestCSharp_ReferencesInMultipleFiles_Invocations() + { + VerifyCSharpFix(new[] { @" +using System; + +public class C +{ + public C PropertyC { get; set; } + + public int M1() + { + return 0; + } +} + +class C2 +{ + private int x; + private C fieldC; + private C PropertyC { get; set; } + + public void M2(C paramC) + { + var localC = fieldC; + x = paramC.M1() + localC.M1() + fieldC.M1() + PropertyC.M1() + fieldC.PropertyC.M1(); + } +}", +@" +using System; + +class C3 +{ + private int x; + private C fieldC; + private C PropertyC { get; set; } + + public void M3(C paramC) + { + var localC = fieldC; + x = paramC.M1() + localC.M1() + fieldC.M1() + PropertyC.M1() + fieldC.PropertyC.M1(); + } +}" }, + new[] { @" +using System; + +public class C +{ + public C PropertyC { get; set; } + + public static int M1() + { + return 0; + } +} + +class C2 +{ + private int x; + private C fieldC; + private C PropertyC { get; set; } + + public void M2(C paramC) + { + var localC = fieldC; + x = C.M1() + C.M1() + C.M1() + C.M1() + C.M1(); + } +}", +@" +using System; + +class C3 +{ + private int x; + private C fieldC; + private C PropertyC { get; set; } + + public void M3(C paramC) + { + var localC = fieldC; + x = C.M1() + C.M1() + C.M1() + C.M1() + C.M1(); + } +}" }); + } + + [Fact] + public void TestCSharp_ReferenceInArgument() + { + VerifyCSharpFix(@" +public class C +{ + private C fieldC; + public C M1(C c) + { + return c; + } + + public C M2(C paramC) + { + var localC = fieldC; + return this.M1(paramC.M1(localC)); + } +}", +@" +public class C +{ + private C fieldC; + public static C M1(C c) + { + return c; + } + + public C M2(C paramC) + { + var localC = fieldC; + return M1(M1(localC)); + } +}"); + } + + [Fact] + public void TestBasic_ReferenceInArgument() + { + VerifyBasicFix(@" +Public Class C + Private fieldC As C + + Public Function M1(c As C) As C + Return c + End Function + + Public Function M2(paramC As C) As C + Dim localC = fieldC + Return Me.M1(paramC.M1(localC)) + End Function +End Class", +@" +Public Class C + Private fieldC As C + + Public Shared Function M1(c As C) As C + Return c + End Function + + Public Function M2(paramC As C) As C + Dim localC = fieldC + Return M1(M1(localC)) + End Function +End Class"); + } + + [Fact] + public void TestCSharp_GenericMethod() + { + VerifyCSharpFix(@" +public class C +{ + private C fieldC; + public C M1(C c, T t) + { + return c; + } + + public C M1(T t, int i) + { + return fieldC; + } +} + +public class C2 +{ + private C fieldC; + public void M2(C paramC) + { + // Explicit type argument + paramC.M1(fieldC, 0); + + // Implicit type argument + paramC.M1(fieldC, this); + } +}", +@" +public class C +{ + private C fieldC; + public static C M1(C c, T t) + { + return c; + } + + public C M1(T t, int i) + { + return fieldC; + } +} + +public class C2 +{ + private C fieldC; + public void M2(C paramC) + { + // Explicit type argument + C.M1(fieldC, 0); + + // Implicit type argument + C.M1(fieldC, this); + } +}"); + } + + [Fact] + public void TestBasic_GenericMethod() + { + VerifyBasicFix(@" +Public Class C + Private fieldC As C + + Public Function M1(Of T)(c As C, t1 As T) As C + Return c + End Function + + Public Function M1(Of T)(t1 As T, i As Integer) As C + Return fieldC + End Function +End Class + +Public Class C2(Of T2) + Private fieldC As C + + Public Sub M2(paramC As C) + ' Explicit type argument + paramC.M1(Of Integer)(fieldC, 0) + + ' Implicit type argument + paramC.M1(fieldC, Me) + End Sub +End Class", +@" +Public Class C + Private fieldC As C + + Public Shared Function M1(Of T)(c As C, t1 As T) As C + Return c + End Function + + Public Function M1(Of T)(t1 As T, i As Integer) As C + Return fieldC + End Function +End Class + +Public Class C2(Of T2) + Private fieldC As C + + Public Sub M2(paramC As C) + ' Explicit type argument + C.M1(Of Integer)(fieldC, 0) + + ' Implicit type argument + C.M1(fieldC, Me) + End Sub +End Class"); + } + + [Fact] + public void TestCSharp_InvocationInInstance() + { + // We don't make the replacement if instance has an invocation. + VerifyCSharpFix(@" +public class C +{ + private C fieldC; + public C M1(C c) + { + return c; + } + + public C M2(C paramC) + { + var localC = fieldC; + return localC.M1(paramC).M1(paramC.M1(localC)); + } +}", +@" +public class C +{ + private C fieldC; + public static C M1(C c) + { + return c; + } + + public C M2(C paramC) + { + var localC = fieldC; + return M1(paramC).M1(M1(localC)); + } +}", allowNewCompilerDiagnostics: true, validationMode: TestValidationMode.AllowCompileErrors); + } + + [Fact] + public void TestBasic_InvocationInInstance() + { + // We don't make the replacement if instance has an invocation. + VerifyBasicFix(@" +Public Class C + Private fieldC As C + + Public Function M1(c As C) As C + Return c + End Function + + Public Function M2(paramC As C) As C + Dim localC = fieldC + Return localC.M1(paramC).M1(paramC.M1(localC)) + End Function +End Class", +@" +Public Class C + Private fieldC As C + + Public Shared Function M1(c As C) As C + Return c + End Function + + Public Function M2(paramC As C) As C + Dim localC = fieldC + Return M1(paramC).M1(M1(localC)) + End Function +End Class", allowNewCompilerDiagnostics: true, validationMode: TestValidationMode.AllowCompileErrors); + } + + [Fact] + public void TestCSharp_ConversionInInstance() + { + // We don't make the replacement if instance has a conversion. + VerifyCSharpFix(@" +public class C +{ + private C fieldC; + public object M1(C c) + { + return c; + } + + public C M2(C paramC) + { + var localC = fieldC; + return ((C)paramC).M1(localC); + } +}", +@" +public class C +{ + private C fieldC; + public static object M1(C c) + { + return c; + } + + public C M2(C paramC) + { + var localC = fieldC; + return ((C)paramC).M1(localC); + } +}", allowNewCompilerDiagnostics: true, validationMode: TestValidationMode.AllowCompileErrors); + } + + [Fact] + public void TestBasic_ConversionInInstance() + { + // We don't make the replacement if instance has a conversion. + VerifyBasicFix(@" +Public Class C + Private fieldC As C + + Public Function M1(c As C) As Object + Return c + End Function + + Public Function M2(paramC As C) As C + Dim localC = fieldC + Return (CType(paramC, C)).M1(localC) + End Function +End Class", +@" +Public Class C + Private fieldC As C + + Public Shared Function M1(c As C) As Object + Return c + End Function + + Public Function M2(paramC As C) As C + Dim localC = fieldC + Return (CType(paramC, C)).M1(localC) + End Function +End Class", allowNewCompilerDiagnostics: true, validationMode: TestValidationMode.AllowCompileErrors); + } + + [Fact] + public void TestCSharp_FixAll() + { + VerifyCSharpFix(new[] { @" +using System; + +public class C +{ + public C PropertyC { get; set; } + + public int M1() + { + return 0; + } + + public int M2() + { + return 0; + } +} + +class C2 +{ + private int x; + private C fieldC; + private C PropertyC { get; set; } + + public void M2(C paramC) + { + var localC = fieldC; + x = paramC.M1() + localC.M2() + fieldC.M1() + PropertyC.M2() + fieldC.PropertyC.M1(); + } +}", +@" +using System; + +class C3 +{ + private int x; + private C fieldC; + private C PropertyC { get; set; } + + public void M3(C paramC) + { + var localC = fieldC; + x = paramC.M2() + localC.M1() + fieldC.M2() + PropertyC.M1() + fieldC.PropertyC.M2(); + } +}" }, + new[] { @" +using System; + +public class C +{ + public C PropertyC { get; set; } + + public static int M1() + { + return 0; + } + + public static int M2() + { + return 0; + } +} + +class C2 +{ + private int x; + private C fieldC; + private C PropertyC { get; set; } + + public void M2(C paramC) + { + var localC = fieldC; + x = C.M1() + C.M2() + C.M1() + C.M2() + C.M1(); + } +}", +@" +using System; + +class C3 +{ + private int x; + private C fieldC; + private C PropertyC { get; set; } + + public void M3(C paramC) + { + var localC = fieldC; + x = C.M2() + C.M1() + C.M2() + C.M1() + C.M2(); + } +}" }); + } + + [Fact] + public void TestBasic_FixAll() + { + VerifyBasicFix(new[] { @" +Imports System + +Public Class C + Public Property PropertyC As C + + Public Function M1() As Integer + Return 0 + End Function + + Public Function M2() As Integer + Return 0 + End Function +End Class + +Class C2 + Private x As Integer + Private fieldC As C + Private Property PropertyC As C + + Public Sub M2(paramC As C) + Dim localC = fieldC + x = paramC.M1() + localC.M2() + fieldC.M1() + PropertyC.M2() + fieldC.PropertyC.M1() + End Sub +End Class", +@" +Imports System + +Class C3 + Private x As Integer + Private fieldC As C + Private Property PropertyC As C + + Public Sub M3(paramC As C) + Dim localC = fieldC + x = paramC.M2() + localC.M1() + fieldC.M2() + PropertyC.M1() + fieldC.PropertyC.M2() + End Sub +End Class" }, + new[] { @" +Imports System + +Public Class C + Public Property PropertyC As C + + Public Shared Function M1() As Integer + Return 0 + End Function + + Public Shared Function M2() As Integer + Return 0 + End Function +End Class + +Class C2 + Private x As Integer + Private fieldC As C + Private Property PropertyC As C + + Public Sub M2(paramC As C) + Dim localC = fieldC + x = C.M1() + C.M2() + C.M1() + C.M2() + C.M1() + End Sub +End Class", +@" +Imports System + +Class C3 + Private x As Integer + Private fieldC As C + Private Property PropertyC As C + + Public Sub M3(paramC As C) + Dim localC = fieldC + x = C.M2() + C.M1() + C.M2() + C.M1() + C.M2() + End Sub +End Class" }); + } } } \ No newline at end of file diff --git a/src/Microsoft.CodeQuality.Analyzers/VisualBasic/QualityGuidelines/BasicMarkMembersAsStatic.Fixer.vb b/src/Microsoft.CodeQuality.Analyzers/VisualBasic/QualityGuidelines/BasicMarkMembersAsStatic.Fixer.vb index 6f97561acc..3dc2cf0dbb 100644 --- a/src/Microsoft.CodeQuality.Analyzers/VisualBasic/QualityGuidelines/BasicMarkMembersAsStatic.Fixer.vb +++ b/src/Microsoft.CodeQuality.Analyzers/VisualBasic/QualityGuidelines/BasicMarkMembersAsStatic.Fixer.vb @@ -3,6 +3,9 @@ Imports System.Composition Imports Microsoft.CodeAnalysis Imports Microsoft.CodeAnalysis.CodeFixes +Imports Microsoft.CodeAnalysis.Operations +Imports Microsoft.CodeAnalysis.VisualBasic +Imports Microsoft.CodeAnalysis.VisualBasic.Syntax Imports Microsoft.CodeQuality.Analyzers.QualityGuidelines Namespace Microsoft.CodeQuality.VisualBasic.Analyzers.QualityGuidelines @@ -13,5 +16,19 @@ Namespace Microsoft.CodeQuality.VisualBasic.Analyzers.QualityGuidelines Public NotInheritable Class BasicMarkMembersAsStaticFixer Inherits MarkMembersAsStaticFixer + Protected Overrides Function GetTypeArguments(node As SyntaxNode) As IEnumerable(Of SyntaxNode) + Return TryCast(node, GenericNameSyntax)?.TypeArgumentList.Arguments + End Function + + Protected Overrides Function GetExpressionOfInvocation(invocation As SyntaxNode) As SyntaxNode + Return TryCast(invocation, InvocationExpressionSyntax)?.Expression + End Function + + Protected Overrides Function GetSyntaxNodeToReplace(memberReference As IMemberReferenceOperation) As SyntaxNode + Dim syntax = MyBase.GetSyntaxNodeToReplace(memberReference) + + ' VB operation tree seems to have an incorrect syntax node association. + Return If(syntax.IsKind(SyntaxKind.AddressOfExpression), DirectCast(syntax, UnaryExpressionSyntax).Operand, syntax) + End Function End Class End Namespace From f0a1a6d633b44c846a1695d77af813ef90ac3a06 Mon Sep 17 00:00:00 2001 From: Manish Vasani Date: Tue, 8 Jan 2019 12:12:53 -0800 Subject: [PATCH 2/3] Change MakeStaticAnalyzer to register the invocation operation action inside operation block start. Without this change, the operation block start and nested actions never seem to execute within live analysis in IDE (I will file a separate Roslyn bug for it) --- .../QualityGuidelines/MarkMembersAsStatic.cs | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/src/Microsoft.CodeQuality.Analyzers/Core/QualityGuidelines/MarkMembersAsStatic.cs b/src/Microsoft.CodeQuality.Analyzers/Core/QualityGuidelines/MarkMembersAsStatic.cs index a47e7b7535..ad3e947a04 100644 --- a/src/Microsoft.CodeQuality.Analyzers/Core/QualityGuidelines/MarkMembersAsStatic.cs +++ b/src/Microsoft.CodeQuality.Analyzers/Core/QualityGuidelines/MarkMembersAsStatic.cs @@ -78,6 +78,15 @@ public override void Initialize(AnalysisContext analysisContext) isInstanceReferenced = true; }, OperationKind.InstanceReference); + blockStartContext.RegisterOperationAction(operationContext => + { + var invocation = (IInvocationOperation)operationContext.Operation; + if (!invocation.TargetMethod.IsExternallyVisible()) + { + invokedInternalMethods.Add(invocation.TargetMethod); + } + }, OperationKind.Invocation); + blockStartContext.RegisterOperationBlockEndAction(blockEndContext => { // Methods referenced by other non static methods @@ -101,15 +110,6 @@ public override void Initialize(AnalysisContext analysisContext) }); }); - compilationContext.RegisterOperationAction(operationContext => - { - var invocation = (IInvocationOperation)operationContext.Operation; - if (!invocation.TargetMethod.IsExternallyVisible()) - { - invokedInternalMethods.Add(invocation.TargetMethod); - } - }, OperationKind.Invocation); - compilationContext.RegisterCompilationEndAction(compilationEndContext => { foreach (var candidate in internalCandidates) From 5bb5a4bf3d0bdcd4e854fd6ba173da34ccb33e48 Mon Sep 17 00:00:00 2001 From: Manish Vasani Date: Tue, 8 Jan 2019 13:04:28 -0800 Subject: [PATCH 3/3] Address feedback --- .../MarkMembersAsStatic.Fixer.cs | 32 +++++++------------ ...ftQualityGuidelinesAnalyzersResources.resx | 2 +- ...QualityGuidelinesAnalyzersResources.cs.xlf | 4 +-- ...QualityGuidelinesAnalyzersResources.de.xlf | 4 +-- ...QualityGuidelinesAnalyzersResources.es.xlf | 4 +-- ...QualityGuidelinesAnalyzersResources.fr.xlf | 4 +-- ...QualityGuidelinesAnalyzersResources.it.xlf | 4 +-- ...QualityGuidelinesAnalyzersResources.ja.xlf | 4 +-- ...QualityGuidelinesAnalyzersResources.ko.xlf | 4 +-- ...QualityGuidelinesAnalyzersResources.pl.xlf | 4 +-- ...lityGuidelinesAnalyzersResources.pt-BR.xlf | 4 +-- ...QualityGuidelinesAnalyzersResources.ru.xlf | 4 +-- ...QualityGuidelinesAnalyzersResources.tr.xlf | 4 +-- ...tyGuidelinesAnalyzersResources.zh-Hans.xlf | 4 +-- ...tyGuidelinesAnalyzersResources.zh-Hant.xlf | 4 +-- src/Utilities/Analyzer.Utilities.projitems | 1 + .../Extensions/SemanticModelExtensions.cs | 28 ++++++++++++++++ 17 files changed, 68 insertions(+), 47 deletions(-) create mode 100644 src/Utilities/Extensions/SemanticModelExtensions.cs diff --git a/src/Microsoft.CodeQuality.Analyzers/Core/QualityGuidelines/MarkMembersAsStatic.Fixer.cs b/src/Microsoft.CodeQuality.Analyzers/Core/QualityGuidelines/MarkMembersAsStatic.Fixer.cs index 1a5e3174d6..49c1d947d2 100644 --- a/src/Microsoft.CodeQuality.Analyzers/Core/QualityGuidelines/MarkMembersAsStatic.Fixer.cs +++ b/src/Microsoft.CodeQuality.Analyzers/Core/QualityGuidelines/MarkMembersAsStatic.Fixer.cs @@ -8,6 +8,7 @@ using System.Threading; using System.Threading.Tasks; using Analyzer.Utilities; +using Analyzer.Utilities.Extensions; using Microsoft.CodeAnalysis; using Microsoft.CodeAnalysis.CodeFixes; using Microsoft.CodeAnalysis.Editing; @@ -78,15 +79,24 @@ private async Task UpdateReferencesAsync(ISymbol symbol, Solution solu cancellationToken.ThrowIfCancellationRequested(); var references = await SymbolFinder.FindReferencesAsync(symbol, solution, cancellationToken).ConfigureAwait(false); - if (references?.Count() != 1) + if (references.Count() != 1) { return solution; } + // Group references by document and fix references in each document. foreach (var referenceLocationGroup in references.Single().Locations.GroupBy(r => r.Document)) { // Get document in current solution var document = solution.GetDocument(referenceLocationGroup.Key.Id); + + // Skip references in projects with different language. + // https://github.com/dotnet/roslyn-analyzers/issues/1986 tracks handling them. + if (!document.Project.Language.Equals(symbol.Language, StringComparison.Ordinal)) + { + continue; + } + var root = await document.GetSyntaxRootAsync(cancellationToken).ConfigureAwait(false); var semanticModel = await document.GetSemanticModelAsync(cancellationToken).ConfigureAwait(false); @@ -99,7 +109,7 @@ private async Task UpdateReferencesAsync(ISymbol symbol, Solution solu var referenceNode = root.FindNode(referenceLocation.Location.SourceSpan, getInnermostNodeForTie: true); if (referenceNode != null) { - var operation = GetOperationWalkingUpParentChain(referenceNode, semanticModel); + var operation = semanticModel.GetOperationWalkingUpParentChain(referenceNode, cancellationToken); SyntaxNode nodeToReplaceOpt = null; switch (operation) { @@ -155,24 +165,6 @@ private async Task UpdateReferencesAsync(ISymbol symbol, Solution solu return solution; // Local functions. - IOperation GetOperationWalkingUpParentChain(SyntaxNode node, SemanticModel semanticModel) - { - // Walk up the parent chain to fetch the first non-null operation. - do - { - var operation = semanticModel.GetOperation(node, cancellationToken); - if (operation != null) - { - return operation; - } - - node = node.Parent; - } - while (node != null); - - return null; - } - bool IsReplacableOperation(IOperation operation) { // We only replace reference operations whose removal cannot change semantics. diff --git a/src/Microsoft.CodeQuality.Analyzers/Core/QualityGuidelines/MicrosoftQualityGuidelinesAnalyzersResources.resx b/src/Microsoft.CodeQuality.Analyzers/Core/QualityGuidelines/MicrosoftQualityGuidelinesAnalyzersResources.resx index 6d6b5a2fba..c2c8a4656f 100644 --- a/src/Microsoft.CodeQuality.Analyzers/Core/QualityGuidelines/MicrosoftQualityGuidelinesAnalyzersResources.resx +++ b/src/Microsoft.CodeQuality.Analyzers/Core/QualityGuidelines/MicrosoftQualityGuidelinesAnalyzersResources.resx @@ -232,6 +232,6 @@ Do not duplicate indexed element initializations - Make static (Shared in VB) + Make static \ No newline at end of file diff --git a/src/Microsoft.CodeQuality.Analyzers/Core/QualityGuidelines/xlf/MicrosoftQualityGuidelinesAnalyzersResources.cs.xlf b/src/Microsoft.CodeQuality.Analyzers/Core/QualityGuidelines/xlf/MicrosoftQualityGuidelinesAnalyzersResources.cs.xlf index fcafb0e434..4f97e328fe 100644 --- a/src/Microsoft.CodeQuality.Analyzers/Core/QualityGuidelines/xlf/MicrosoftQualityGuidelinesAnalyzersResources.cs.xlf +++ b/src/Microsoft.CodeQuality.Analyzers/Core/QualityGuidelines/xlf/MicrosoftQualityGuidelinesAnalyzersResources.cs.xlf @@ -3,8 +3,8 @@ - Make static (Shared in VB) - Make static (Shared in VB) + Make static + Make static diff --git a/src/Microsoft.CodeQuality.Analyzers/Core/QualityGuidelines/xlf/MicrosoftQualityGuidelinesAnalyzersResources.de.xlf b/src/Microsoft.CodeQuality.Analyzers/Core/QualityGuidelines/xlf/MicrosoftQualityGuidelinesAnalyzersResources.de.xlf index ae2f400cc8..a6f4eb9ad6 100644 --- a/src/Microsoft.CodeQuality.Analyzers/Core/QualityGuidelines/xlf/MicrosoftQualityGuidelinesAnalyzersResources.de.xlf +++ b/src/Microsoft.CodeQuality.Analyzers/Core/QualityGuidelines/xlf/MicrosoftQualityGuidelinesAnalyzersResources.de.xlf @@ -3,8 +3,8 @@ - Make static (Shared in VB) - Make static (Shared in VB) + Make static + Make static diff --git a/src/Microsoft.CodeQuality.Analyzers/Core/QualityGuidelines/xlf/MicrosoftQualityGuidelinesAnalyzersResources.es.xlf b/src/Microsoft.CodeQuality.Analyzers/Core/QualityGuidelines/xlf/MicrosoftQualityGuidelinesAnalyzersResources.es.xlf index 9596c54f97..b526f6f4b2 100644 --- a/src/Microsoft.CodeQuality.Analyzers/Core/QualityGuidelines/xlf/MicrosoftQualityGuidelinesAnalyzersResources.es.xlf +++ b/src/Microsoft.CodeQuality.Analyzers/Core/QualityGuidelines/xlf/MicrosoftQualityGuidelinesAnalyzersResources.es.xlf @@ -3,8 +3,8 @@ - Make static (Shared in VB) - Make static (Shared in VB) + Make static + Make static diff --git a/src/Microsoft.CodeQuality.Analyzers/Core/QualityGuidelines/xlf/MicrosoftQualityGuidelinesAnalyzersResources.fr.xlf b/src/Microsoft.CodeQuality.Analyzers/Core/QualityGuidelines/xlf/MicrosoftQualityGuidelinesAnalyzersResources.fr.xlf index 15c6ca59a4..fd2407f33d 100644 --- a/src/Microsoft.CodeQuality.Analyzers/Core/QualityGuidelines/xlf/MicrosoftQualityGuidelinesAnalyzersResources.fr.xlf +++ b/src/Microsoft.CodeQuality.Analyzers/Core/QualityGuidelines/xlf/MicrosoftQualityGuidelinesAnalyzersResources.fr.xlf @@ -3,8 +3,8 @@ - Make static (Shared in VB) - Make static (Shared in VB) + Make static + Make static diff --git a/src/Microsoft.CodeQuality.Analyzers/Core/QualityGuidelines/xlf/MicrosoftQualityGuidelinesAnalyzersResources.it.xlf b/src/Microsoft.CodeQuality.Analyzers/Core/QualityGuidelines/xlf/MicrosoftQualityGuidelinesAnalyzersResources.it.xlf index 269b530a91..d0941135d2 100644 --- a/src/Microsoft.CodeQuality.Analyzers/Core/QualityGuidelines/xlf/MicrosoftQualityGuidelinesAnalyzersResources.it.xlf +++ b/src/Microsoft.CodeQuality.Analyzers/Core/QualityGuidelines/xlf/MicrosoftQualityGuidelinesAnalyzersResources.it.xlf @@ -3,8 +3,8 @@ - Make static (Shared in VB) - Make static (Shared in VB) + Make static + Make static diff --git a/src/Microsoft.CodeQuality.Analyzers/Core/QualityGuidelines/xlf/MicrosoftQualityGuidelinesAnalyzersResources.ja.xlf b/src/Microsoft.CodeQuality.Analyzers/Core/QualityGuidelines/xlf/MicrosoftQualityGuidelinesAnalyzersResources.ja.xlf index eea650bd6a..66dd06ac75 100644 --- a/src/Microsoft.CodeQuality.Analyzers/Core/QualityGuidelines/xlf/MicrosoftQualityGuidelinesAnalyzersResources.ja.xlf +++ b/src/Microsoft.CodeQuality.Analyzers/Core/QualityGuidelines/xlf/MicrosoftQualityGuidelinesAnalyzersResources.ja.xlf @@ -3,8 +3,8 @@ - Make static (Shared in VB) - Make static (Shared in VB) + Make static + Make static diff --git a/src/Microsoft.CodeQuality.Analyzers/Core/QualityGuidelines/xlf/MicrosoftQualityGuidelinesAnalyzersResources.ko.xlf b/src/Microsoft.CodeQuality.Analyzers/Core/QualityGuidelines/xlf/MicrosoftQualityGuidelinesAnalyzersResources.ko.xlf index f1e7c5cf35..1af62b3cff 100644 --- a/src/Microsoft.CodeQuality.Analyzers/Core/QualityGuidelines/xlf/MicrosoftQualityGuidelinesAnalyzersResources.ko.xlf +++ b/src/Microsoft.CodeQuality.Analyzers/Core/QualityGuidelines/xlf/MicrosoftQualityGuidelinesAnalyzersResources.ko.xlf @@ -3,8 +3,8 @@ - Make static (Shared in VB) - Make static (Shared in VB) + Make static + Make static diff --git a/src/Microsoft.CodeQuality.Analyzers/Core/QualityGuidelines/xlf/MicrosoftQualityGuidelinesAnalyzersResources.pl.xlf b/src/Microsoft.CodeQuality.Analyzers/Core/QualityGuidelines/xlf/MicrosoftQualityGuidelinesAnalyzersResources.pl.xlf index 8f477242d9..86d6f7b575 100644 --- a/src/Microsoft.CodeQuality.Analyzers/Core/QualityGuidelines/xlf/MicrosoftQualityGuidelinesAnalyzersResources.pl.xlf +++ b/src/Microsoft.CodeQuality.Analyzers/Core/QualityGuidelines/xlf/MicrosoftQualityGuidelinesAnalyzersResources.pl.xlf @@ -3,8 +3,8 @@ - Make static (Shared in VB) - Make static (Shared in VB) + Make static + Make static diff --git a/src/Microsoft.CodeQuality.Analyzers/Core/QualityGuidelines/xlf/MicrosoftQualityGuidelinesAnalyzersResources.pt-BR.xlf b/src/Microsoft.CodeQuality.Analyzers/Core/QualityGuidelines/xlf/MicrosoftQualityGuidelinesAnalyzersResources.pt-BR.xlf index 43c20fa0fa..22c1d6d574 100644 --- a/src/Microsoft.CodeQuality.Analyzers/Core/QualityGuidelines/xlf/MicrosoftQualityGuidelinesAnalyzersResources.pt-BR.xlf +++ b/src/Microsoft.CodeQuality.Analyzers/Core/QualityGuidelines/xlf/MicrosoftQualityGuidelinesAnalyzersResources.pt-BR.xlf @@ -3,8 +3,8 @@ - Make static (Shared in VB) - Make static (Shared in VB) + Make static + Make static diff --git a/src/Microsoft.CodeQuality.Analyzers/Core/QualityGuidelines/xlf/MicrosoftQualityGuidelinesAnalyzersResources.ru.xlf b/src/Microsoft.CodeQuality.Analyzers/Core/QualityGuidelines/xlf/MicrosoftQualityGuidelinesAnalyzersResources.ru.xlf index e0e185768e..7b4ef69362 100644 --- a/src/Microsoft.CodeQuality.Analyzers/Core/QualityGuidelines/xlf/MicrosoftQualityGuidelinesAnalyzersResources.ru.xlf +++ b/src/Microsoft.CodeQuality.Analyzers/Core/QualityGuidelines/xlf/MicrosoftQualityGuidelinesAnalyzersResources.ru.xlf @@ -3,8 +3,8 @@ - Make static (Shared in VB) - Make static (Shared in VB) + Make static + Make static diff --git a/src/Microsoft.CodeQuality.Analyzers/Core/QualityGuidelines/xlf/MicrosoftQualityGuidelinesAnalyzersResources.tr.xlf b/src/Microsoft.CodeQuality.Analyzers/Core/QualityGuidelines/xlf/MicrosoftQualityGuidelinesAnalyzersResources.tr.xlf index 33fbdbf070..879d2c02c0 100644 --- a/src/Microsoft.CodeQuality.Analyzers/Core/QualityGuidelines/xlf/MicrosoftQualityGuidelinesAnalyzersResources.tr.xlf +++ b/src/Microsoft.CodeQuality.Analyzers/Core/QualityGuidelines/xlf/MicrosoftQualityGuidelinesAnalyzersResources.tr.xlf @@ -3,8 +3,8 @@ - Make static (Shared in VB) - Make static (Shared in VB) + Make static + Make static diff --git a/src/Microsoft.CodeQuality.Analyzers/Core/QualityGuidelines/xlf/MicrosoftQualityGuidelinesAnalyzersResources.zh-Hans.xlf b/src/Microsoft.CodeQuality.Analyzers/Core/QualityGuidelines/xlf/MicrosoftQualityGuidelinesAnalyzersResources.zh-Hans.xlf index 1a811e1f00..a1d3d140f6 100644 --- a/src/Microsoft.CodeQuality.Analyzers/Core/QualityGuidelines/xlf/MicrosoftQualityGuidelinesAnalyzersResources.zh-Hans.xlf +++ b/src/Microsoft.CodeQuality.Analyzers/Core/QualityGuidelines/xlf/MicrosoftQualityGuidelinesAnalyzersResources.zh-Hans.xlf @@ -3,8 +3,8 @@ - Make static (Shared in VB) - Make static (Shared in VB) + Make static + Make static diff --git a/src/Microsoft.CodeQuality.Analyzers/Core/QualityGuidelines/xlf/MicrosoftQualityGuidelinesAnalyzersResources.zh-Hant.xlf b/src/Microsoft.CodeQuality.Analyzers/Core/QualityGuidelines/xlf/MicrosoftQualityGuidelinesAnalyzersResources.zh-Hant.xlf index 40ecf709d8..ab1fa4def3 100644 --- a/src/Microsoft.CodeQuality.Analyzers/Core/QualityGuidelines/xlf/MicrosoftQualityGuidelinesAnalyzersResources.zh-Hant.xlf +++ b/src/Microsoft.CodeQuality.Analyzers/Core/QualityGuidelines/xlf/MicrosoftQualityGuidelinesAnalyzersResources.zh-Hant.xlf @@ -3,8 +3,8 @@ - Make static (Shared in VB) - Make static (Shared in VB) + Make static + Make static diff --git a/src/Utilities/Analyzer.Utilities.projitems b/src/Utilities/Analyzer.Utilities.projitems index d7173ab71c..376ad51b80 100644 --- a/src/Utilities/Analyzer.Utilities.projitems +++ b/src/Utilities/Analyzer.Utilities.projitems @@ -23,6 +23,7 @@ + diff --git a/src/Utilities/Extensions/SemanticModelExtensions.cs b/src/Utilities/Extensions/SemanticModelExtensions.cs new file mode 100644 index 0000000000..ef832373cd --- /dev/null +++ b/src/Utilities/Extensions/SemanticModelExtensions.cs @@ -0,0 +1,28 @@ +// Copyright (c) Microsoft. All Rights Reserved. Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using System.Threading; +using Microsoft.CodeAnalysis; + +namespace Analyzer.Utilities.Extensions +{ + internal static class SemanticModelExtensions + { + public static IOperation GetOperationWalkingUpParentChain(this SemanticModel semanticModel, SyntaxNode node, CancellationToken cancellationToken) + { + // Walk up the parent chain to fetch the first non-null operation. + do + { + var operation = semanticModel.GetOperation(node, cancellationToken); + if (operation != null) + { + return operation; + } + + node = node.Parent; + } + while (node != null); + + return null; + } + } +}