From 361a97d4ac7cf24124c5c091b37a63c0070e27e1 Mon Sep 17 00:00:00 2001 From: Giovanni Bassi Date: Wed, 10 Jun 2015 23:14:15 -0300 Subject: [PATCH] Make method static closes #364 --- src/CSharp/CodeCracker/CodeCracker.csproj | 4 +- .../Design/MakeMethodStaticAnalyzer.cs | 63 ++++++ .../Design/MakeMethodStaticCodeFixProvider.cs | 97 +++++++++ .../Extensions/CSharpAnalyzerExtensions.cs | 11 + src/Common/CodeCracker.Common/DiagnosticId.cs | 1 + .../CodeCracker.Test/CodeCracker.Test.csproj | 3 +- .../Design/MakeMethodStaticTests.cs | 201 ++++++++++++++++++ 7 files changed, 378 insertions(+), 2 deletions(-) create mode 100644 src/CSharp/CodeCracker/Design/MakeMethodStaticAnalyzer.cs create mode 100644 src/CSharp/CodeCracker/Design/MakeMethodStaticCodeFixProvider.cs create mode 100644 test/CSharp/CodeCracker.Test/Design/MakeMethodStaticTests.cs diff --git a/src/CSharp/CodeCracker/CodeCracker.csproj b/src/CSharp/CodeCracker/CodeCracker.csproj index cc8a2ce6c..f2174d53a 100644 --- a/src/CSharp/CodeCracker/CodeCracker.csproj +++ b/src/CSharp/CodeCracker/CodeCracker.csproj @@ -39,6 +39,8 @@ + + @@ -271,4 +273,4 @@ --> - \ No newline at end of file + diff --git a/src/CSharp/CodeCracker/Design/MakeMethodStaticAnalyzer.cs b/src/CSharp/CodeCracker/Design/MakeMethodStaticAnalyzer.cs new file mode 100644 index 000000000..4d78bbf4f --- /dev/null +++ b/src/CSharp/CodeCracker/Design/MakeMethodStaticAnalyzer.cs @@ -0,0 +1,63 @@ +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.CSharp; +using Microsoft.CodeAnalysis.CSharp.Syntax; +using Microsoft.CodeAnalysis.Diagnostics; +using System.Linq; +using System.Collections.Immutable; + +namespace CodeCracker.CSharp.Design +{ + [DiagnosticAnalyzer(LanguageNames.CSharp)] + public class MakeMethodStaticAnalyzer : DiagnosticAnalyzer + { + internal const string Title = "Use static method"; + internal const string MessageFormat = "Make '{0}' method static."; + internal const string Category = SupportedCategories.Design; + const string Description = "If the method is not referencing any instance variable and if you are " + + "not creating a virtual, abstract, new or partial method, and if it is not a method override, " + + "your instance method may be changed to a static method."; + + internal static DiagnosticDescriptor Rule = new DiagnosticDescriptor( + DiagnosticId.MakeMethodStatic.ToDiagnosticId(), + Title, + MessageFormat, + Category, + DiagnosticSeverity.Warning, + true, + description: Description, + helpLinkUri: HelpLink.ForDiagnostic(DiagnosticId.MakeMethodStatic)); + + public override ImmutableArray SupportedDiagnostics => ImmutableArray.Create(Rule); + + public override void Initialize(AnalysisContext context) => + context.RegisterSyntaxNodeAction(LanguageVersion.CSharp6, AnalyzeMethod, SyntaxKind.MethodDeclaration); + + private void AnalyzeMethod(SyntaxNodeAnalysisContext context) + { + if (context.IsGenerated()) return; + var method = (MethodDeclarationSyntax)context.Node; + if (method.Modifiers.Any( + SyntaxKind.StaticKeyword, + SyntaxKind.PartialKeyword, + SyntaxKind.VirtualKeyword, + SyntaxKind.NewKeyword, + SyntaxKind.AbstractKeyword, + SyntaxKind.OverrideKeyword)) return; + if (method.Body == null) + { + if (method.ExpressionBody?.Expression == null) return; + var dataFlowAnalysis = context.SemanticModel.AnalyzeDataFlow(method.ExpressionBody.Expression); + if (!dataFlowAnalysis.Succeeded) return; + if (dataFlowAnalysis.DataFlowsIn.Any(inSymbol => inSymbol.Name == "this")) return; + } + else if (method.Body.Statements.Count > 0) + { + var dataFlowAnalysis = context.SemanticModel.AnalyzeDataFlow(method.Body); + if (!dataFlowAnalysis.Succeeded) return; + if (dataFlowAnalysis.DataFlowsIn.Any(inSymbol => inSymbol.Name == "this")) return; + } + var diagnostic = Diagnostic.Create(Rule, method.Identifier.GetLocation(), method.Identifier.ValueText); + context.ReportDiagnostic(diagnostic); + } + } +} \ No newline at end of file diff --git a/src/CSharp/CodeCracker/Design/MakeMethodStaticCodeFixProvider.cs b/src/CSharp/CodeCracker/Design/MakeMethodStaticCodeFixProvider.cs new file mode 100644 index 000000000..e14eac769 --- /dev/null +++ b/src/CSharp/CodeCracker/Design/MakeMethodStaticCodeFixProvider.cs @@ -0,0 +1,97 @@ +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.CodeActions; +using Microsoft.CodeAnalysis.CodeFixes; +using Microsoft.CodeAnalysis.CSharp; +using Microsoft.CodeAnalysis.CSharp.Syntax; +using Microsoft.CodeAnalysis.FindSymbols; +using Microsoft.CodeAnalysis.Formatting; +using System.Collections.Generic; +using System.Collections.Immutable; +using System.Composition; +using System.Linq; +using System.Threading; +using System.Threading.Tasks; + +namespace CodeCracker.CSharp.Design +{ + [ExportCodeFixProvider(LanguageNames.CSharp, Name = nameof(MakeMethodStaticCodeFixProvider)), Shared] + public class MakeMethodStaticCodeFixProvider : CodeFixProvider + { + public override ImmutableArray FixableDiagnosticIds => ImmutableArray.Create(DiagnosticId.MakeMethodStatic.ToDiagnosticId()); + + public override FixAllProvider GetFixAllProvider() => WellKnownFixAllProviders.BatchFixer; + + public sealed override Task RegisterCodeFixesAsync(CodeFixContext context) + { + var diagnostic = context.Diagnostics.First(); + context.RegisterCodeFix(CodeAction.Create("Make static", c => MakeNameOfAsync(context.Document, diagnostic, c)), diagnostic); + return Task.FromResult(0); + } + + private static readonly SyntaxToken staticToken = SyntaxFactory.Token(SyntaxKind.StaticKeyword); + + private static async Task MakeNameOfAsync(Document document, Diagnostic diagnostic, CancellationToken cancellationToken) + { + var root = await document.GetSyntaxRootAsync(cancellationToken).ConfigureAwait(false); + var diagnosticSpan = diagnostic.Location.SourceSpan; + var method = (MethodDeclarationSyntax)root.FindNode(diagnosticSpan); + var semanticModel = await document.GetSemanticModelAsync(cancellationToken).ConfigureAwait(false); + var methodSymbol = semanticModel.GetDeclaredSymbol(method); + var methodClassName = methodSymbol.ContainingType.Name; + var references = await SymbolFinder.FindReferencesAsync(methodSymbol, document.Project.Solution, cancellationToken).ConfigureAwait(false); + var documentGroups = references.SelectMany(r => r.Locations).GroupBy(loc => loc.Document); + var newSolution = UpdateMainDocument(document, root, method, documentGroups); + newSolution = await UpdateReferencingDocuments(document, methodClassName, documentGroups, newSolution, cancellationToken); + return newSolution; + } + + private static Solution UpdateMainDocument(Document document, SyntaxNode root, MethodDeclarationSyntax method, IEnumerable> documentGroups) + { + var mainDocGroup = documentGroups.FirstOrDefault(dg => dg.Key.Equals(document)); + SyntaxNode newRoot; + if (mainDocGroup == null) + { + newRoot = root.ReplaceNode(method, method.AddModifiers(staticToken)); + } + else + { + var diagnosticNodes = mainDocGroup.Select(referenceLocation => root.FindNode(referenceLocation.Location.SourceSpan)).ToList(); + newRoot = root.TrackNodes(diagnosticNodes.Union(new[] { method })); + newRoot = newRoot.ReplaceNode(newRoot.GetCurrentNode(method), method.AddModifiers(staticToken)); + foreach (var diagnosticNode in diagnosticNodes) + { + var invocationExpression = newRoot.GetCurrentNode(diagnosticNode).FirstAncestorOrSelfOfType().Expression; + if (invocationExpression.IsKind(SyntaxKind.IdentifierName)) continue; + var memberAccess = (MemberAccessExpressionSyntax)invocationExpression; + var newMemberAccessParent = memberAccess.Parent.ReplaceNode(memberAccess, memberAccess.Name) + .WithAdditionalAnnotations(Formatter.Annotation); + newRoot = newRoot.ReplaceNode(memberAccess.Parent, newMemberAccessParent); + } + } + var newSolution = document.Project.Solution.WithDocumentSyntaxRoot(document.Id, newRoot); + return newSolution; + } + + private static async Task UpdateReferencingDocuments(Document document, string methodClassName, IEnumerable> documentGroups, Solution newSolution, CancellationToken cancellationToken) + { + var methodIdentifier = SyntaxFactory.IdentifierName(methodClassName); + foreach (var documentGroup in documentGroups) + { + var referencingDocument = documentGroup.Key; + if (referencingDocument.Equals(document)) continue; + var newReferencingRoot = await referencingDocument.GetSyntaxRootAsync(cancellationToken).ConfigureAwait(false); + var diagnosticNodes = documentGroup.Select(referenceLocation => newReferencingRoot.FindNode(referenceLocation.Location.SourceSpan)).ToList(); + newReferencingRoot = newReferencingRoot.TrackNodes(diagnosticNodes); + foreach (var diagnosticNode in diagnosticNodes) + { + var memberAccess = (MemberAccessExpressionSyntax)newReferencingRoot.GetCurrentNode(diagnosticNode).FirstAncestorOrSelfOfType().Expression; + var newMemberAccess = memberAccess.ReplaceNode(memberAccess.Expression, methodIdentifier) + .WithAdditionalAnnotations(Formatter.Annotation); + newReferencingRoot = newReferencingRoot.ReplaceNode(memberAccess, newMemberAccess); + } + newSolution = newSolution.WithDocumentSyntaxRoot(referencingDocument.Id, newReferencingRoot); + } + return newSolution; + } + } +} \ No newline at end of file diff --git a/src/CSharp/CodeCracker/Extensions/CSharpAnalyzerExtensions.cs b/src/CSharp/CodeCracker/Extensions/CSharpAnalyzerExtensions.cs index b17950b03..0f06e0f65 100644 --- a/src/CSharp/CodeCracker/Extensions/CSharpAnalyzerExtensions.cs +++ b/src/CSharp/CodeCracker/Extensions/CSharpAnalyzerExtensions.cs @@ -125,5 +125,16 @@ public static bool IsKind(this SyntaxNodeOrToken nodeOrToken, params SyntaxKind[ } public static bool IsNotKind(this SyntaxNode node, params SyntaxKind[] kinds) => !node.IsKind(kinds); + + public static bool Any(this SyntaxTokenList list, SyntaxKind kind1, SyntaxKind kind2) => + list.IndexOf(kind1) >= 0 || list.IndexOf(kind2) >= 0; + + public static bool Any(this SyntaxTokenList list, SyntaxKind kind1, SyntaxKind kind2, params SyntaxKind[] kinds) + { + if (list.Any(kind1, kind2)) return true; + for (int i = 0; i < kinds.Length; i++) + if (list.IndexOf(kinds[i]) >= 0) return true; + return false; + } } } \ No newline at end of file diff --git a/src/Common/CodeCracker.Common/DiagnosticId.cs b/src/Common/CodeCracker.Common/DiagnosticId.cs index ec6eda1e4..4ee7cb8c2 100644 --- a/src/Common/CodeCracker.Common/DiagnosticId.cs +++ b/src/Common/CodeCracker.Common/DiagnosticId.cs @@ -73,6 +73,7 @@ public enum DiagnosticId UseStringEmpty = 84, UseEmptyString = 88, RemoveRedundantElseClause = 89, + MakeMethodStatic = 91, ChangeAllToAny = 92, } } diff --git a/test/CSharp/CodeCracker.Test/CodeCracker.Test.csproj b/test/CSharp/CodeCracker.Test/CodeCracker.Test.csproj index c3d046469..390851886 100644 --- a/test/CSharp/CodeCracker.Test/CodeCracker.Test.csproj +++ b/test/CSharp/CodeCracker.Test/CodeCracker.Test.csproj @@ -137,6 +137,7 @@ + @@ -261,4 +262,4 @@ --> - \ No newline at end of file + diff --git a/test/CSharp/CodeCracker.Test/Design/MakeMethodStaticTests.cs b/test/CSharp/CodeCracker.Test/Design/MakeMethodStaticTests.cs new file mode 100644 index 000000000..7d26448c4 --- /dev/null +++ b/test/CSharp/CodeCracker.Test/Design/MakeMethodStaticTests.cs @@ -0,0 +1,201 @@ +using CodeCracker.CSharp.Design; +using Microsoft.CodeAnalysis; +using System.Threading.Tasks; +using Xunit; +using System; + +namespace CodeCracker.Test.CSharp.Design +{ + public class MakeMethodStaticTests : CodeFixVerifier + { + [Theory] + [InlineData(@"static void Foo() { }")] + [InlineData(@"public virtual void Foo() { }")] + [InlineData(@"string i; void Foo() { Console.WriteLine(i); }")] + [InlineData(@"string i; void Foo() { i = """"; }")] + [InlineData(@"string i; string Foo() { return i; }")] + [InlineData(@"string i; +void Foo() +{ + if (System.DateTime.Now.Seconds > 5) + { + Console.WriteLine(i); + } +}")] + public async Task NoDiagnostic(string code) + { + var source = code.WrapInCSharpClass(); + await VerifyCSharpHasNoDiagnosticsAsync(source); + } + + [Fact] + public async Task NoDiagnosticOnNew() + { + var source = @" + class B + { + private int i = 1; + public int Foo() => i; + } + class C : B + { + public new int Foo() => 1; + }"; + await VerifyCSharpHasNoDiagnosticsAsync(source); + } + + [Fact] + public async Task NoDiagnosticOnOverrideAndAbstract() + { + var source = @" + abstract class B + { + public abstract void Foo(); + } + class C : B + { + public override void Foo() { } + }"; + await VerifyCSharpHasNoDiagnosticsAsync(source); + } + + [Fact] + public async Task NoDiagnosticOnPartial() + { + var source = @" + partial class C + { + partial void Foo() { } + partial void Foo(); + }"; + await VerifyCSharpHasNoDiagnosticsAsync(source); + } + + [Theory] + [InlineData("void Foo() { }")] + [InlineData(@"void Foo() +{ + Console.WriteLine(1); +}")] + [InlineData(@"void Foo() +{ + Console.WriteLine(i); +} +static string i;")] + [InlineData("void Foo() => Console.WriteLine(1);")] + public async Task WithDiagnostic(string code) + { + var source = code.WrapInCSharpClass(); + var expected = new DiagnosticResult + { + Id = DiagnosticId.MakeMethodStatic.ToDiagnosticId(), + Message = string.Format(MakeMethodStaticAnalyzer.MessageFormat, "Foo"), + Severity = DiagnosticSeverity.Warning, + Locations = new[] { new DiagnosticResultLocation("Test0.cs", 8, 18) } + }; + await VerifyCSharpDiagnosticAsync(source, expected); + } + + [Theory] + [InlineData("void Foo() { }", "static void Foo() { }")] + [InlineData("int Foo() => 1;", "static int Foo() => 1;")] + public async Task FixMakeMethodStaticWithoutReference(string code, string fix) + { + var source = code.WrapInCSharpClass(); + var fixtest = fix.WrapInCSharpClass(); + await VerifyCSharpFixAsync(source, fixtest); + } + + [Fact] + public async Task MakeMethodStaticWithReference() + { + var source = @"void Foo() { } +void Bar() +{ + Foo(); +}".WrapInCSharpClass(); + var fixtest = @"static void Foo() { } +void Bar() +{ + Foo(); +}".WrapInCSharpClass(); + await VerifyCSharpFixAsync(source, fixtest); + } + + [Fact] + public async Task MakeMethodStaticWithReferenceWithThis() + { + var source = @"void Foo() { } +void Bar() +{ + this.Foo(); +}".WrapInCSharpClass(); + var fixtest = @"static void Foo() { } +void Bar() +{ + Foo(); +}".WrapInCSharpClass(); + await VerifyCSharpFixAsync(source, fixtest); + } + + [Fact] + public async Task MakeMethodStaticWithReferenceAndLongNames() + { + var source = @"int Foo() => 1; +void Bar() +{ + var result = this.Foo() + this.Foo(); +}".WrapInCSharpClass(); + var fixtest = @"static int Foo() => 1; +void Bar() +{ + var result = Foo() + Foo(); +}".WrapInCSharpClass(); + await VerifyCSharpFixAsync(source, fixtest); + } + + [Fact] + public async Task MakeMethodStaticWithReferenceInDifferentDocs() + { + var source1 = @"private int i; +void Bar() +{ + i = 1; + var t = new Type2(); + t.Foo(); +}".WrapInCSharpClass("Type1"); + var source2 = @"public void Foo() { }".WrapInCSharpClass("Type2"); + var fixtest1 = @"private int i; +void Bar() +{ + i = 1; + var t = new Type2(); + Type2.Foo(); +}".WrapInCSharpClass("Type1"); + var fixtest2 = @"public static void Foo() { }".WrapInCSharpClass("Type2"); + await VerifyCSharpFixAllAsync(new[] { source1, source2 }, new[] { fixtest1, fixtest2 }); + } + + [Fact] + public async Task MakeMethodStaticWithReferenceInDifferentDocsWithCallsOnTheSameLine() + { + var source1 = @"private int i; +void Bar() +{ + i = 1; + var t = new LargeTypeName(); + var result = t.Foo() + t.Foo(); +}".WrapInCSharpClass("Type1"); + var source2 = @"public int Foo() { return 1; }".WrapInCSharpClass("LargeTypeName"); + var fixtest1 = @"private int i; +void Bar() +{ + i = 1; + var t = new LargeTypeName(); + var result = LargeTypeName.Foo() + LargeTypeName.Foo(); +}".WrapInCSharpClass("Type1"); + var fixtest2 = @"public static int Foo() { return 1; }".WrapInCSharpClass("LargeTypeName"); + await VerifyCSharpFixAllAsync(new[] { source1, source2 }, new[] { fixtest1, fixtest2 }); + } + } +} \ No newline at end of file