From 464e318b9ee12e2e793df05b00c883c658705e0c Mon Sep 17 00:00:00 2001 From: tpodolak Date: Wed, 31 Aug 2022 19:14:35 +0200 Subject: [PATCH] GH-153 - simplifying ConstructorArgumentsForInterfaceCodeFixProvider with operations API --- ...torArgumentsForInterfaceCodeFixProvider.cs | 28 +++++++---- ...torArgumentsForInterfaceCodeFixProvider.cs | 48 ++++++++++++------- ...torArgumentsForInterfaceCodeFixProvider.cs | 31 ++++++++---- ...gumentsForInterfaceCodeFixProviderTests.cs | 8 ++-- 4 files changed, 78 insertions(+), 37 deletions(-) diff --git a/src/NSubstitute.Analyzers.CSharp/CodeFixProviders/ConstructorArgumentsForInterfaceCodeFixProvider.cs b/src/NSubstitute.Analyzers.CSharp/CodeFixProviders/ConstructorArgumentsForInterfaceCodeFixProvider.cs index 3f117a64..46c4d3f6 100644 --- a/src/NSubstitute.Analyzers.CSharp/CodeFixProviders/ConstructorArgumentsForInterfaceCodeFixProvider.cs +++ b/src/NSubstitute.Analyzers.CSharp/CodeFixProviders/ConstructorArgumentsForInterfaceCodeFixProvider.cs @@ -3,24 +3,36 @@ using Microsoft.CodeAnalysis.CodeFixes; using Microsoft.CodeAnalysis.CSharp; using Microsoft.CodeAnalysis.CSharp.Syntax; +using Microsoft.CodeAnalysis.Operations; using NSubstitute.Analyzers.Shared.CodeFixProviders; using static Microsoft.CodeAnalysis.CSharp.SyntaxFactory; namespace NSubstitute.Analyzers.CSharp.CodeFixProviders; [ExportCodeFixProvider(LanguageNames.CSharp)] -internal sealed class ConstructorArgumentsForInterfaceCodeFixProvider : AbstractConstructorArgumentsForInterfaceCodeFixProvider +internal sealed class ConstructorArgumentsForInterfaceCodeFixProvider : AbstractConstructorArgumentsForInterfaceCodeFixProvider { - protected override InvocationExpressionSyntax GetInvocationExpressionSyntaxWithEmptyArgumentList(InvocationExpressionSyntax invocationExpressionSyntax) + protected override SyntaxNode GetInvocationExpressionSyntaxWithEmptyArgumentList(IInvocationOperation invocationOperation) { - return invocationExpressionSyntax.WithArgumentList(ArgumentList()); + var invocationExpression = (InvocationExpressionSyntax)invocationOperation.Syntax; + + return invocationExpression.WithArgumentList(ArgumentList()); } - protected override InvocationExpressionSyntax GetInvocationExpressionSyntaxWithNullConstructorArgument(InvocationExpressionSyntax invocationExpressionSyntax) + protected override SyntaxNode GetInvocationExpressionSyntaxWithNullConstructorArgument(IInvocationOperation invocationOperation) { - var nullSyntax = Argument(LiteralExpression(SyntaxKind.NullLiteralExpression)); - var secondArgument = invocationExpressionSyntax.ArgumentList.Arguments.Skip(1).First(); - var argumentListSyntax = invocationExpressionSyntax.ArgumentList.ReplaceNode(secondArgument, nullSyntax); - return invocationExpressionSyntax.WithArgumentList(argumentListSyntax); + var invocationExpressionSyntax = (InvocationExpressionSyntax)invocationOperation.Syntax; + var arguments = invocationOperation.Arguments.Select(argumentOperation => + { + var argumentSyntax = (ArgumentSyntax)argumentOperation.Syntax; + if (argumentOperation.Parameter.Ordinal > 0) + { + argumentSyntax = argumentSyntax.WithExpression(LiteralExpression(SyntaxKind.NullLiteralExpression)); + } + + return argumentSyntax; + }); + + return invocationExpressionSyntax.WithArgumentList(ArgumentList(SeparatedList(arguments))); } } \ No newline at end of file diff --git a/src/NSubstitute.Analyzers.Shared/CodeFixProviders/AbstractConstructorArgumentsForInterfaceCodeFixProvider.cs b/src/NSubstitute.Analyzers.Shared/CodeFixProviders/AbstractConstructorArgumentsForInterfaceCodeFixProvider.cs index 4d073042..ae4ad0c1 100644 --- a/src/NSubstitute.Analyzers.Shared/CodeFixProviders/AbstractConstructorArgumentsForInterfaceCodeFixProvider.cs +++ b/src/NSubstitute.Analyzers.Shared/CodeFixProviders/AbstractConstructorArgumentsForInterfaceCodeFixProvider.cs @@ -5,46 +5,60 @@ using Microsoft.CodeAnalysis; using Microsoft.CodeAnalysis.CodeActions; using Microsoft.CodeAnalysis.CodeFixes; +using Microsoft.CodeAnalysis.Editing; +using Microsoft.CodeAnalysis.Operations; using Document = Microsoft.CodeAnalysis.Document; namespace NSubstitute.Analyzers.Shared.CodeFixProviders; -internal abstract class AbstractConstructorArgumentsForInterfaceCodeFixProvider - : CodeFixProvider - where TInvocationExpressionSyntax : SyntaxNode +internal abstract class AbstractConstructorArgumentsForInterfaceCodeFixProvider : CodeFixProvider { public override FixAllProvider GetFixAllProvider() => WellKnownFixAllProviders.BatchFixer; - public override ImmutableArray FixableDiagnosticIds { get; } = ImmutableArray.Create(DiagnosticIdentifiers.SubstituteConstructorArgumentsForInterface); + public override ImmutableArray FixableDiagnosticIds { get; } = + ImmutableArray.Create(DiagnosticIdentifiers.SubstituteConstructorArgumentsForInterface); public override Task RegisterCodeFixesAsync(CodeFixContext context) { - var diagnostic = context.Diagnostics.FirstOrDefault(diag => diag.Descriptor.Id == DiagnosticIdentifiers.SubstituteConstructorArgumentsForInterface); - if (diagnostic != null) + var diagnostic = context.Diagnostics.FirstOrDefault(diag => + diag.Descriptor.Id == DiagnosticIdentifiers.SubstituteConstructorArgumentsForInterface); + if (diagnostic == null) { - var codeAction = CodeAction.Create("Remove constructor arguments", ct => CreateChangedDocument(ct, context, diagnostic), nameof(AbstractConstructorArgumentsForInterfaceCodeFixProvider)); - context.RegisterCodeFix(codeAction, diagnostic); + return Task.CompletedTask; } + var codeAction = CodeAction.Create( + "Remove constructor arguments", + ct => CreateChangedDocument(ct, context, diagnostic), + nameof(AbstractConstructorArgumentsForInterfaceCodeFixProvider)); + context.RegisterCodeFix(codeAction, diagnostic); + return Task.CompletedTask; } - protected abstract TInvocationExpressionSyntax GetInvocationExpressionSyntaxWithEmptyArgumentList(TInvocationExpressionSyntax invocationExpressionSyntax); + protected abstract SyntaxNode GetInvocationExpressionSyntaxWithEmptyArgumentList(IInvocationOperation invocationOperation); - protected abstract TInvocationExpressionSyntax GetInvocationExpressionSyntaxWithNullConstructorArgument(TInvocationExpressionSyntax invocationExpressionSyntax); + protected abstract SyntaxNode GetInvocationExpressionSyntaxWithNullConstructorArgument(IInvocationOperation invocationOperation); private async Task CreateChangedDocument(CancellationToken cancellationToken, CodeFixContext context, Diagnostic diagnostic) { + var documentEditor = await DocumentEditor.CreateAsync(context.Document, cancellationToken); + var root = await context.Document.GetSyntaxRootAsync(cancellationToken).ConfigureAwait(false); - var invocation = (TInvocationExpressionSyntax)root.FindNode(diagnostic.Location.SourceSpan, getInnermostNodeForTie: true); + var invocation = root.FindNode(diagnostic.Location.SourceSpan, getInnermostNodeForTie: true); var semanticModel = await context.Document.GetSemanticModelAsync(cancellationToken); - var updatedInvocation = semanticModel.GetSymbolInfo(invocation).Symbol is IMethodSymbol methodSymbol && - methodSymbol.IsGenericMethod - ? GetInvocationExpressionSyntaxWithEmptyArgumentList(invocation) - : GetInvocationExpressionSyntaxWithNullConstructorArgument(invocation); + if (semanticModel.GetOperation(invocation) is not IInvocationOperation invocationOperation) + { + return context.Document; + } + + var updatedInvocation = invocationOperation.TargetMethod.IsGenericMethod + ? GetInvocationExpressionSyntaxWithEmptyArgumentList(invocationOperation) + : GetInvocationExpressionSyntaxWithNullConstructorArgument(invocationOperation); + + documentEditor.ReplaceNode(invocation, updatedInvocation); - var replacedRoot = root.ReplaceNode(invocation, updatedInvocation); - return context.Document.WithSyntaxRoot(replacedRoot); + return documentEditor.GetChangedDocument(); } } \ No newline at end of file diff --git a/src/NSubstitute.Analyzers.VisualBasic/CodeFixProviders/ConstructorArgumentsForInterfaceCodeFixProvider.cs b/src/NSubstitute.Analyzers.VisualBasic/CodeFixProviders/ConstructorArgumentsForInterfaceCodeFixProvider.cs index bb93d6d4..915c80ae 100644 --- a/src/NSubstitute.Analyzers.VisualBasic/CodeFixProviders/ConstructorArgumentsForInterfaceCodeFixProvider.cs +++ b/src/NSubstitute.Analyzers.VisualBasic/CodeFixProviders/ConstructorArgumentsForInterfaceCodeFixProvider.cs @@ -1,6 +1,7 @@ using System.Linq; using Microsoft.CodeAnalysis; using Microsoft.CodeAnalysis.CodeFixes; +using Microsoft.CodeAnalysis.Operations; using Microsoft.CodeAnalysis.VisualBasic; using Microsoft.CodeAnalysis.VisualBasic.Syntax; using NSubstitute.Analyzers.Shared.CodeFixProviders; @@ -9,18 +10,32 @@ namespace NSubstitute.Analyzers.VisualBasic.CodeFixProviders; [ExportCodeFixProvider(LanguageNames.VisualBasic)] -internal sealed class ConstructorArgumentsForInterfaceCodeFixProvider : AbstractConstructorArgumentsForInterfaceCodeFixProvider +internal sealed class ConstructorArgumentsForInterfaceCodeFixProvider : AbstractConstructorArgumentsForInterfaceCodeFixProvider { - protected override InvocationExpressionSyntax GetInvocationExpressionSyntaxWithEmptyArgumentList(InvocationExpressionSyntax invocationExpressionSyntax) + protected override SyntaxNode GetInvocationExpressionSyntaxWithEmptyArgumentList(IInvocationOperation invocationOperation) { - return invocationExpressionSyntax.WithArgumentList(ArgumentList()); + var invocationExpression = (InvocationExpressionSyntax)invocationOperation.Syntax; + + return invocationExpression.WithArgumentList(ArgumentList()); } - protected override InvocationExpressionSyntax GetInvocationExpressionSyntaxWithNullConstructorArgument(InvocationExpressionSyntax invocationExpressionSyntax) + protected override SyntaxNode GetInvocationExpressionSyntaxWithNullConstructorArgument(IInvocationOperation invocationOperation) { - var nullSyntax = SimpleArgument(LiteralExpression(SyntaxKind.NothingLiteralExpression, Token(SyntaxKind.NothingKeyword))); - var seconArgument = invocationExpressionSyntax.ArgumentList.Arguments.Skip(1).First(); - var argumentListSyntax = invocationExpressionSyntax.ArgumentList.ReplaceNode(seconArgument, nullSyntax); - return invocationExpressionSyntax.WithArgumentList(argumentListSyntax); + var invocationExpressionSyntax = (InvocationExpressionSyntax)invocationOperation.Syntax; + var arguments = invocationOperation.Arguments + .OrderBy(x => x.Syntax.GetLocation().GetLineSpan().StartLinePosition.Character) + .Select(argumentOperation => + { + var argumentSyntax = (SimpleArgumentSyntax)argumentOperation.Syntax; + if (argumentOperation.Parameter.Ordinal > 0) + { + argumentSyntax = argumentSyntax.WithExpression( + LiteralExpression(SyntaxKind.NothingLiteralExpression, Token(SyntaxKind.NothingKeyword))); + } + + return argumentSyntax; + }); + + return invocationExpressionSyntax.WithArgumentList(ArgumentList(SeparatedList(arguments))); } } \ No newline at end of file diff --git a/tests/NSubstitute.Analyzers.Tests.VisualBasic/CodeFixProvidersTests/ConstructorArgumentsForInterfaceCodeFixProviderTests/ConstructorArgumentsForInterfaceCodeFixProviderTests.cs b/tests/NSubstitute.Analyzers.Tests.VisualBasic/CodeFixProvidersTests/ConstructorArgumentsForInterfaceCodeFixProviderTests/ConstructorArgumentsForInterfaceCodeFixProviderTests.cs index f04f2170..236a0a48 100644 --- a/tests/NSubstitute.Analyzers.Tests.VisualBasic/CodeFixProvidersTests/ConstructorArgumentsForInterfaceCodeFixProviderTests/ConstructorArgumentsForInterfaceCodeFixProviderTests.cs +++ b/tests/NSubstitute.Analyzers.Tests.VisualBasic/CodeFixProvidersTests/ConstructorArgumentsForInterfaceCodeFixProviderTests/ConstructorArgumentsForInterfaceCodeFixProviderTests.cs @@ -76,8 +76,8 @@ End Interface Public Class FooTests Public Sub Test() Dim substitute = NSubstitute.Substitute.[For]({GetType(IFoo)}, Nothing) - Dim otherSubstitute = NSubstitute.Substitute.[For](typesToProxy:= {GetType(IFoo)}, constructorArguments:= Nothing) - Dim yetAnotherSubstitute = NSubstitute.Substitute.[For](constructorArguments:= Nothing, typesToProxy:= {GetType(IFoo)}) + Dim otherSubstitute = NSubstitute.Substitute.[For](typesToProxy:= {GetType(IFoo)}, constructorArguments:=Nothing) + Dim yetAnotherSubstitute = NSubstitute.Substitute.[For](constructorArguments:=Nothing, typesToProxy:= {GetType(IFoo)}) End Sub End Class End Namespace @@ -114,8 +114,8 @@ End Interface Public Class FooTests Public Sub Test() Dim substitute = SubstitutionContext.Current.SubstituteFactory.Create({GetType(IFoo)}, Nothing) - Dim otherSubstitute = SubstitutionContext.Current.SubstituteFactory.Create(typesToProxy:= {GetType(IFoo)}, constructorArguments:= Nothing) - Dim yetAnotherSubstitute = SubstitutionContext.Current.SubstituteFactory.Create(constructorArguments:= Nothing, typesToProxy:= {GetType(IFoo)}) + Dim otherSubstitute = SubstitutionContext.Current.SubstituteFactory.Create(typesToProxy:= {GetType(IFoo)}, constructorArguments:=Nothing) + Dim yetAnotherSubstitute = SubstitutionContext.Current.SubstituteFactory.Create(constructorArguments:=Nothing, typesToProxy:= {GetType(IFoo)}) End Sub End Class End Namespace