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..49c1d947d2 100644 --- a/src/Microsoft.CodeQuality.Analyzers/Core/QualityGuidelines/MarkMembersAsStatic.Fixer.cs +++ b/src/Microsoft.CodeQuality.Analyzers/Core/QualityGuidelines/MarkMembersAsStatic.Fixer.cs @@ -1,9 +1,20 @@ // 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 Analyzer.Utilities.Extensions; +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 +23,176 @@ 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; + } + + // 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); + + // 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 = 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) + { + 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. + 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/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) diff --git a/src/Microsoft.CodeQuality.Analyzers/Core/QualityGuidelines/MicrosoftQualityGuidelinesAnalyzersResources.resx b/src/Microsoft.CodeQuality.Analyzers/Core/QualityGuidelines/MicrosoftQualityGuidelinesAnalyzersResources.resx index 0e73772bb4..c2c8a4656f 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 + \ 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..4f97e328fe 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 + Make static + + 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..a6f4eb9ad6 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 + Make static + + 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..b526f6f4b2 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 + Make static + + 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..fd2407f33d 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 + Make static + + 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..d0941135d2 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 + Make static + + 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..66dd06ac75 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 + Make static + + 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..1af62b3cff 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 + Make static + + 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..86d6f7b575 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 + Make static + + 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..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 @@ -2,6 +2,11 @@ + + Make static + Make static + + 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..7b4ef69362 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 + Make static + + 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..879d2c02c0 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 + Make static + + 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..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 @@ -2,6 +2,11 @@ + + Make static + Make static + + 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..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 @@ -2,6 +2,11 @@ + + Make static + Make static + + 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 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; + } + } +}