Skip to content

Commit

Permalink
implemented first pass at a PipeTo(Sender) CodeFixer (#15)
Browse files Browse the repository at this point in the history
* implemented first pass at a `PipeTo(Sender)` CodeFixer

* renaming existing Analyzer specs

so there won't be naming conflicts or confusion with Fixer specs that target the same ruleset

* refactored `Verify` to include possibility of CodeFix tests

* rename analyzer test files

* trying to debug

* restore proper test behavior

* fixed spec

Roslyn is still not detecting the fixer correctly though

* working on fixing "Fixer" discovery

* removed duplicate project reference

* fixed discovery

* have fixer working, now fighting over whitespace

* fixed whitespace mangling

* removed offending lines from formatter

* verified FixIt cases for PipeTo
  • Loading branch information
Aaronontheweb authored Dec 29, 2023
1 parent 8850fa9 commit 5482a51
Show file tree
Hide file tree
Showing 13 changed files with 335 additions and 10 deletions.
6 changes: 6 additions & 0 deletions akka.analyzers.sln
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@ Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "build", "build", "{B67B6FBE
EndProject
Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "Akka.Analyzers.Tests", "src\Akka.Analyzers.Tests\Akka.Analyzers.Tests.csproj", "{21D3D1DE-CC6D-435C-B025-FB232D3D1A6D}"
EndProject
Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "Akka.Analyzers.Fixes", "src\Akka.Analyzers.Fixes\Akka.Analyzers.Fixes.csproj", "{295556B8-6201-46C5-A866-71F2B0693254}"
EndProject
Global
GlobalSection(SolutionConfigurationPlatforms) = preSolution
Debug|Any CPU = Debug|Any CPU
Expand All @@ -32,5 +34,9 @@ Global
{21D3D1DE-CC6D-435C-B025-FB232D3D1A6D}.Debug|Any CPU.Build.0 = Debug|Any CPU
{21D3D1DE-CC6D-435C-B025-FB232D3D1A6D}.Release|Any CPU.ActiveCfg = Release|Any CPU
{21D3D1DE-CC6D-435C-B025-FB232D3D1A6D}.Release|Any CPU.Build.0 = Release|Any CPU
{295556B8-6201-46C5-A866-71F2B0693254}.Debug|Any CPU.ActiveCfg = Debug|Any CPU
{295556B8-6201-46C5-A866-71F2B0693254}.Debug|Any CPU.Build.0 = Debug|Any CPU
{295556B8-6201-46C5-A866-71F2B0693254}.Release|Any CPU.ActiveCfg = Release|Any CPU
{295556B8-6201-46C5-A866-71F2B0693254}.Release|Any CPU.Build.0 = Release|Any CPU
EndGlobalSection
EndGlobal
Original file line number Diff line number Diff line change
@@ -0,0 +1,84 @@
using System.Composition;
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.CodeActions;
using Microsoft.CodeAnalysis.CodeFixes;
using Microsoft.CodeAnalysis.CSharp;
using Microsoft.CodeAnalysis.CSharp.Syntax;
using Microsoft.CodeAnalysis.Editing;
using Microsoft.CodeAnalysis.Formatting;
using static Akka.Analyzers.Fixes.CodeGeneratorUtilities;

namespace Akka.Analyzers.Fixes.AK1000;

[ExportCodeFixProvider(LanguageNames.CSharp), Shared]
public sealed class MustCloseOverSenderWhenUsingPipeToFixer()
: BatchedCodeFixProvider(RuleDescriptors.Ak1001CloseOverSenderUsingPipeTo.Id)
{
public const string Key_FixPipeToSender = "AK1000_FixPipeToSender";

public override async Task RegisterCodeFixesAsync(CodeFixContext context)
{
var root = await context.Document.GetSyntaxRootAsync(context.CancellationToken).ConfigureAwait(false);
if (root is null)
return;

var diagnostic = context.Diagnostics.FirstOrDefault();
if (diagnostic is null)
return;
var diagnosticSpan = diagnostic.Location.SourceSpan;

// Find the PipeTo invocation expression
var invocationExpr = root.FindToken(diagnosticSpan.Start).Parent?.AncestorsAndSelf()
.OfType<InvocationExpressionSyntax>().First();
if (invocationExpr is null)
return;

context.RegisterCodeFix(
CodeAction.Create(
title: "Use local variable for Sender",
createChangedDocument: c => UseLocalVariableForSenderAsync(context.Document, invocationExpr, c),
equivalenceKey: Key_FixPipeToSender),
context.Diagnostics);
}

private static async Task<Document> UseLocalVariableForSenderAsync(Document document,
InvocationExpressionSyntax invocationExpr, CancellationToken cancellationToken)
{
var editor = await DocumentEditor.CreateAsync(document, cancellationToken).ConfigureAwait(false);

// Generate a local variable to hold 'Sender'
var senderVariable = IntroduceLocalVariableStatement("sender", SyntaxFactory.MemberAccessExpression(
SyntaxKind.SimpleMemberAccessExpression,
SyntaxFactory.ThisExpression(),
SyntaxFactory.IdentifierName("Sender")))
.WithAdditionalAnnotations(Formatter.Annotation); // need this line to get indentation right

// Find an appropriate insertion point for the local variable
var insertionPoint = invocationExpr.FirstAncestorOrSelf<StatementSyntax>();

if (insertionPoint == null)
{
// Unable to find a valid insertion point
return document;
}

// Insert the local variable declaration at the found insertion point
editor.InsertBefore(insertionPoint, senderVariable);

// in the invocationExpr, replace the old argument list with a new one that uses the new local variable
// Replace the invocation arguments with the new local variable
var newArgumentList = SyntaxFactory.ArgumentList(
SyntaxFactory.SingletonSeparatedList(
SyntaxFactory.Argument(
SyntaxFactory.IdentifierName("sender"))));

var newInvocationExpr = invocationExpr.WithArgumentList(newArgumentList);

// Make sure to replace the old invocation with the new one
editor.ReplaceNode(invocationExpr, newInvocationExpr);

var newDocument = editor.GetChangedDocument(); // error happens here

return newDocument;
}
}
13 changes: 13 additions & 0 deletions src/Akka.Analyzers.Fixes/Akka.Analyzers.Fixes.csproj
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
<Project Sdk="Microsoft.NET.Sdk">

<PropertyGroup>
<TargetFramework>netstandard2.0</TargetFramework>
<ImplicitUsings>enable</ImplicitUsings>
<Nullable>enable</Nullable>
</PropertyGroup>

<ItemGroup>
<ProjectReference Include="..\Akka.Analyzers\Akka.Analyzers.csproj" />
</ItemGroup>

</Project>
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
<wpf:ResourceDictionary xml:space="preserve" xmlns:x="http://schemas.microsoft.com/winfx/2006/xaml" xmlns:s="clr-namespace:System;assembly=mscorlib" xmlns:ss="urn:shemas-jetbrains-com:settings-storage-xaml" xmlns:wpf="http://schemas.microsoft.com/winfx/2006/xaml/presentation">
<s:Boolean x:Key="/Default/CodeInspection/NamespaceProvider/NamespaceFoldersToSkip/=utility/@EntryIndexedValue">True</s:Boolean></wpf:ResourceDictionary>
12 changes: 12 additions & 0 deletions src/Akka.Analyzers.Fixes/Utility/BatchedCodeFixProvider.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
using System.Collections.Immutable;
using Microsoft.CodeAnalysis.CodeFixes;

namespace Akka.Analyzers.Fixes;

public abstract class BatchedCodeFixProvider(params string[] diagnostics) : CodeFixProvider
{
public sealed override ImmutableArray<string> FixableDiagnosticIds { get; } = diagnostics.ToImmutableArray();

public sealed override FixAllProvider GetFixAllProvider() =>
WellKnownFixAllProviders.BatchFixer;
}
31 changes: 31 additions & 0 deletions src/Akka.Analyzers.Fixes/Utility/CodeGeneratorUtilities.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
// -----------------------------------------------------------------------
// <copyright file="CodeGeneratorUtilities.cs" company="Akka.NET Project">
// Copyright (C) 2015-2023 .NET Petabridge, LLC
// </copyright>
// -----------------------------------------------------------------------

using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.CSharp.Syntax;
using static Microsoft.CodeAnalysis.CSharp.SyntaxFactory;

namespace Akka.Analyzers.Fixes;

internal static class CodeGeneratorUtilities
{
public static LocalDeclarationStatementSyntax IntroduceLocalVariableStatement(
string parameterName,
ExpressionSyntax replacementNode)
{
var equalsToReplacementNode = EqualsValueClause(replacementNode);

var oneItemVariableDeclaration = VariableDeclaration(
ParseTypeName("var"),
SingletonSeparatedList(
VariableDeclarator(Identifier(parameterName))
.WithInitializer(equalsToReplacementNode)
)
);

return LocalDeclarationStatement(oneItemVariableDeclaration);
}
}
5 changes: 1 addition & 4 deletions src/Akka.Analyzers.Tests/Akka.Analyzers.Tests.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -31,11 +31,8 @@
</ItemGroup>

<ItemGroup>
<ProjectReference Include="..\Akka.Analyzers.Fixes\Akka.Analyzers.Fixes.csproj" OutputItemType="Analyzer" ReferenceOutputAssembly="true"/>
<ProjectReference Include="..\Akka.Analyzers\Akka.Analyzers.csproj" OutputItemType="Analyzer" ReferenceOutputAssembly="true" />
</ItemGroup>

<ItemGroup>
<ProjectReference Include="..\Akka.Analyzers\Akka.Analyzers.csproj" />
</ItemGroup>

</Project>
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@

namespace Akka.Analyzers.Tests.Analyzers.AK1000;

public class MustCloseOverSenderWhenUsingPipeToSpecs
public class MustCloseOverSenderWhenUsingPipeToAnalyzerSpecs
{
public static readonly TheoryData<string> SuccessCases = new()
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@

namespace Akka.Analyzers.Tests.Analyzers.AK1000;

public class AkkaActorInstantiationAnalyzerTests
public class MustNotUseNewKeywordOnActorsAnalyzerSpecs
{
public static readonly TheoryData<string> SuccessCases = new()
{
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,107 @@
using Akka.Analyzers.Fixes.AK1000;
using Verify = Akka.Analyzers.Tests.Utility.AkkaVerifier<Akka.Analyzers.MustCloseOverSenderWhenUsingPipeToAnalyzer>;

namespace Akka.Analyzers.Tests.Fixes.AK1000;

public class MustCloseOverSenderWhenUsingPipeToFixerSpecs
{
[Fact]
public Task AddClosureInsideReceiveActor()
{
var before =
@"using Akka.Actor;
using System.Threading.Tasks;
public sealed class MyActor : ReceiveActor{
public MyActor(){
Receive<string>(str => {
async Task<int> LocalFunction(){
await Task.Delay(10);
return str.Length;
}
// incorrect use of closure
LocalFunction().PipeTo(Sender);
});
}
}";

var after =
@"using Akka.Actor;
using System.Threading.Tasks;
public sealed class MyActor : ReceiveActor{
public MyActor(){
Receive<string>(str => {
async Task<int> LocalFunction(){
await Task.Delay(10);
return str.Length;
}
var sender = this.Sender;
// incorrect use of closure
LocalFunction().PipeTo(sender);
});
}
}";

var expectedDiagnostic = Verify.Diagnostic()
.WithSpan(14, 29, 14, 35)
.WithArguments("Sender");

return Verify.VerifyCodeFix(before, after, MustCloseOverSenderWhenUsingPipeToFixer.Key_FixPipeToSender,
expectedDiagnostic);
}

[Fact]
public Task AddClosureInsideUntypedActor()
{
var before =
@"using Akka.Actor;
using System.Threading.Tasks;
using System;
public sealed class MyActor : UntypedActor{
protected override void OnReceive(object message){
async Task<int> LocalFunction(){
await Task.Delay(10);
return message.ToString().Length;
}
Console.WriteLine(Sender);
// incorrect use of closure
LocalFunction().PipeTo(Sender);
}
}";

var after =
@"using Akka.Actor;
using System.Threading.Tasks;
using System;
public sealed class MyActor : UntypedActor{
protected override void OnReceive(object message){
async Task<int> LocalFunction(){
await Task.Delay(10);
return message.ToString().Length;
}
Console.WriteLine(Sender);
var sender = this.Sender;
// incorrect use of closure
LocalFunction().PipeTo(sender);
}
}";

var expectedDiagnostic = Verify.Diagnostic()
.WithSpan(15, 25, 15, 31)
.WithArguments("Sender");

return Verify.VerifyCodeFix(before, after, MustCloseOverSenderWhenUsingPipeToFixer.Key_FixPipeToSender,
expectedDiagnostic);
}
}
31 changes: 28 additions & 3 deletions src/Akka.Analyzers.Tests/Utility/AkkaVerifier.cs
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,10 @@

using System.Diagnostics.CodeAnalysis;
using System.Globalization;
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.CodeFixes;
using Microsoft.CodeAnalysis.CSharp;
using Microsoft.CodeAnalysis.CSharp.Testing;
using Microsoft.CodeAnalysis.Diagnostics;
using Microsoft.CodeAnalysis.Formatting;
using Microsoft.CodeAnalysis.Testing;

namespace Akka.Analyzers.Tests.Utility;
Expand Down Expand Up @@ -43,9 +42,25 @@ public static Task VerifyAnalyzer(string[] sources, params DiagnosticResult[] di
return test.RunAsync();
}

public static Task VerifyCodeFix(string before, string after, string fixerActionKey,
params DiagnosticResult[] diagnostics)
{
Guard.AssertIsNotNull(before);
Guard.AssertIsNotNull(after);

var test = new AkkaTest()
{
TestCode = before,
FixedCode = after,
CodeActionEquivalenceKey = fixerActionKey
};
test.TestState.ExpectedDiagnostics.AddRange(diagnostics);
return test.RunAsync();
}

private sealed class AkkaTest() : TestBase(ReferenceAssembliesHelper.CurrentAkka);

private class TestBase : CSharpAnalyzerTest<TAnalyzer, DefaultVerifier>
private class TestBase : CSharpCodeFixTest<TAnalyzer, EmptyCodeFixProvider, DefaultVerifier>
{
protected TestBase(ReferenceAssemblies referenceAssemblies)
{
Expand All @@ -57,5 +72,15 @@ protected TestBase(ReferenceAssemblies referenceAssemblies)
// Tests that check for messages should run independent of current system culture.
CultureInfo.DefaultThreadCurrentUICulture = CultureInfo.InvariantCulture;
}

protected override IEnumerable<CodeFixProvider> GetCodeFixProviders()
{
var analyzer = new TAnalyzer();

foreach (var provider in CodeFixProviderDiscovery.GetCodeFixProviders(Language))
if (analyzer.SupportedDiagnostics.Any(diagnostic =>
provider.FixableDiagnosticIds.Contains(diagnostic.Id)))
yield return provider;
}
}
}
Loading

0 comments on commit 5482a51

Please sign in to comment.