diff --git a/src/NSubstitute.Analyzers.CSharp/CodeRefactoringProviders/IntroduceSubstituteCodeRefactoringProvider.cs b/src/NSubstitute.Analyzers.CSharp/CodeRefactoringProviders/IntroduceSubstituteCodeRefactoringProvider.cs index 9b61b56b..df9cda44 100644 --- a/src/NSubstitute.Analyzers.CSharp/CodeRefactoringProviders/IntroduceSubstituteCodeRefactoringProvider.cs +++ b/src/NSubstitute.Analyzers.CSharp/CodeRefactoringProviders/IntroduceSubstituteCodeRefactoringProvider.cs @@ -13,10 +13,6 @@ namespace NSubstitute.Analyzers.CSharp.CodeRefactoringProviders [ExportCodeRefactoringProvider(LanguageNames.CSharp)] internal sealed class IntroduceSubstituteCodeRefactoringProvider : AbstractIntroduceSubstituteCodeRefactoringProvider { - protected override int ReadonlySubstituteDeclarationContainerRawKind { get; } = (int)SyntaxKind.ClassDeclaration; - - protected override int LocalSubstituteDeclarationContainerRawKind { get; } = (int)SyntaxKind.Block; - protected override IReadOnlyList GetArgumentSyntaxNodes(ArgumentListSyntax argumentListSyntax, TextSpan span) { return argumentListSyntax.Arguments; @@ -37,6 +33,23 @@ protected override ObjectCreationExpressionSyntax UpdateObjectCreationExpression return objectCreationExpressionSyntax.WithArgumentList(updatedArgumentList); } + protected override SyntaxNode FindSiblingNodeForLocalSubstitute(ObjectCreationExpressionSyntax creationExpression) + { + var container = creationExpression.Ancestors() + .FirstOrDefault(ancestor => ancestor.Kind() == SyntaxKind.Block); + + return container?.ChildNodes().FirstOrDefault(); + } + + protected override SyntaxNode FindSiblingNodeForReadonlySubstitute(SyntaxNode creationExpression) + { + var typeDeclarationSyntax = creationExpression.Ancestors() + .OfType() + .FirstOrDefault(); + + return typeDeclarationSyntax?.Members.FirstOrDefault(); + } + private static ArgumentListSyntax UpdateArgumentListTrivia( ArgumentListSyntax originalArgumentList, ArgumentListSyntax updatedArgumentList) diff --git a/src/NSubstitute.Analyzers.Shared/CodeRefactoringProviders/AbstractIntroduceSubstituteCodeRefactoringProvider.cs b/src/NSubstitute.Analyzers.Shared/CodeRefactoringProviders/AbstractIntroduceSubstituteCodeRefactoringProvider.cs index a726e1be..77cb1900 100644 --- a/src/NSubstitute.Analyzers.Shared/CodeRefactoringProviders/AbstractIntroduceSubstituteCodeRefactoringProvider.cs +++ b/src/NSubstitute.Analyzers.Shared/CodeRefactoringProviders/AbstractIntroduceSubstituteCodeRefactoringProvider.cs @@ -16,10 +16,6 @@ internal abstract class AbstractIntroduceSubstituteCodeRefactoringProvider argumentSyntax.IsMissing; - protected virtual SyntaxNode FindContainerForLocalSubstitute(TObjectCreationExpressionSyntax creationExpression) - { - var container = creationExpression.Ancestors() - .FirstOrDefault(ancestor => ancestor.RawKind == LocalSubstituteDeclarationContainerRawKind); - - return container?.ChildNodes().FirstOrDefault(); - } - - protected virtual SyntaxNode FindContainerForReadonlySubstitute(SyntaxNode creationExpression) - { - var container = creationExpression.Ancestors().FirstOrDefault(ancestor => ancestor.RawKind == ReadonlySubstituteDeclarationContainerRawKind); + protected abstract SyntaxNode FindSiblingNodeForLocalSubstitute(TObjectCreationExpressionSyntax creationExpression); - return container?.ChildNodes().FirstOrDefault(); - } + protected abstract SyntaxNode FindSiblingNodeForReadonlySubstitute(SyntaxNode creationExpression); - private IMethodSymbol GetConstructorSymbolFromExpression(SemanticModel semanticModel, TObjectCreationExpressionSyntax objectCreationExpressionSyntax) + private IEnumerable CreateRefactoringActions( + CodeRefactoringContext context, + SemanticModel semanticModel, + TObjectCreationExpressionSyntax objectCreationExpressionSyntax, + TArgumentListSyntax argumentListSyntax) { - var symbol = semanticModel.GetSymbolInfo(objectCreationExpressionSyntax); + var constructorSymbol = GetKnownConstructorSymbol(semanticModel, objectCreationExpressionSyntax); - if (symbol.Symbol is IMethodSymbol methodSymbol) + if (constructorSymbol == null || constructorSymbol.Parameters.Length == 0) { - return methodSymbol; + yield break; } - var candidateMethodSymbols = symbol.CandidateSymbols.OfType().ToList(); + var existingArguments = GetArgumentSyntaxNodes(argumentListSyntax, context.Span); + var constructorParameters = constructorSymbol.Parameters.OrderBy(parameter => parameter.Ordinal).ToList(); + var argumentIndexAtSpan = FindArgumentIndexAtSpan(existingArguments, context.Span); - if (candidateMethodSymbols.Count == 0 || candidateMethodSymbols.Count > 1) + var localSubstituteSiblingNode = FindSiblingNodeForLocalSubstitute(objectCreationExpressionSyntax); + var readonlySubstituteSiblingNode = FindSiblingNodeForReadonlySubstitute(objectCreationExpressionSyntax); + + if (HasMissingArgumentAtPosition(constructorParameters, existingArguments, argumentIndexAtSpan)) { - return null; - } + var substituteName = constructorParameters[argumentIndexAtSpan].ToMinimalSymbolString(semanticModel); - return candidateMethodSymbols.Single(); - } + if (localSubstituteSiblingNode != null) + { + yield return CodeAction.Create( + $"Introduce local substitute for {substituteName}", + token => IntroduceLocalSubstitute( + context, + objectCreationExpressionSyntax, + existingArguments, + constructorParameters, + new[] { argumentIndexAtSpan }, + localSubstituteSiblingNode)); + } - private IReadOnlyList GetMissingParametersPositions( - IReadOnlyList parameterSymbols, - IReadOnlyList argumentSyntaxNodes) - { - if (parameterSymbols.Count == 0) - { - return Array.Empty(); + if (readonlySubstituteSiblingNode != null) + { + yield return CodeAction.Create( + $"Introduce readonly substitute for {substituteName}", + token => IntroduceReadonlySubstitute( + context, + objectCreationExpressionSyntax, + existingArguments, + constructorParameters, + new[] { argumentIndexAtSpan }, + readonlySubstituteSiblingNode)); + } } - if (argumentSyntaxNodes.Count == 0) + var missingArgumentsPositions = GetMissingArgumentsPositions(existingArguments, constructorParameters); + if (missingArgumentsPositions.Count == 0) { - return Enumerable.Range(0, parameterSymbols.Count).ToList(); + yield break; } - var result = new List(); - for (var symbolPosition = 0; symbolPosition < parameterSymbols.Count; symbolPosition++) + if (localSubstituteSiblingNode != null) { - if (argumentSyntaxNodes.TryGetElementAt(symbolPosition, out var argumentNode) == false || IsMissing(argumentNode)) - { - result.Add(symbolPosition); - } + yield return CodeAction.Create( + "Introduce local substitutes for missing arguments", + token => IntroduceLocalSubstitute( + context, + objectCreationExpressionSyntax, + existingArguments, + constructorParameters, + missingArgumentsPositions, + localSubstituteSiblingNode)); } - return result; - } - - private int FindArgumentIndexAtSpan(IReadOnlyList argumentSyntaxNodes, TextSpan span) - { - var findArgumentIndexAtSpan = argumentSyntaxNodes.IndexOf(node => node.FullSpan.IntersectsWith(span)); - return findArgumentIndexAtSpan >= 0 ? findArgumentIndexAtSpan : 0; + if (readonlySubstituteSiblingNode != null) + { + yield return CodeAction.Create( + "Introduce readonly substitutes for missing arguments", + token => IntroduceReadonlySubstitute( + context, + objectCreationExpressionSyntax, + existingArguments, + constructorParameters, + missingArgumentsPositions, + readonlySubstituteSiblingNode)); + } } private async Task IntroduceReadonlySubstitute( CodeRefactoringContext context, TObjectCreationExpressionSyntax objectCreationExpressionSyntax, - SyntaxNode container, IReadOnlyList existingArguments, - IReadOnlyList positions, - IMethodSymbol methodSymbol) + IReadOnlyList constructorParameters, + IReadOnlyList missingArgumentsPositions, + SyntaxNode siblingNode) { SyntaxNode CreateFieldDeclaration(SyntaxGenerator syntaxGenerator, IParameterSymbol parameterSymbol, SyntaxNode invocationExpression) { @@ -140,19 +158,19 @@ SyntaxNode CreateFieldDeclaration(SyntaxGenerator syntaxGenerator, IParameterSym context, objectCreationExpressionSyntax, existingArguments, - positions, - methodSymbol, - CreateFieldDeclaration, - container); + constructorParameters, + missingArgumentsPositions, + siblingNode, + CreateFieldDeclaration); } private async Task IntroduceLocalSubstitute( CodeRefactoringContext context, TObjectCreationExpressionSyntax objectCreationExpressionSyntax, - SyntaxNode container, IReadOnlyList existingArguments, - IReadOnlyList positions, - IMethodSymbol methodSymbol) + IReadOnlyList constructorParameters, + IReadOnlyList missingArgumentsPositions, + SyntaxNode siblingNode) { SyntaxNode CreateLocalDeclaration(SyntaxGenerator syntaxGenerator, IParameterSymbol parameterSymbol, SyntaxNode invocationExpression) { @@ -163,40 +181,39 @@ SyntaxNode CreateLocalDeclaration(SyntaxGenerator syntaxGenerator, IParameterSym context, objectCreationExpressionSyntax, existingArguments, - positions, - methodSymbol, - CreateLocalDeclaration, - container); + constructorParameters, + missingArgumentsPositions, + siblingNode, + CreateLocalDeclaration); } private async Task IntroduceSubstitute( CodeRefactoringContext context, TObjectCreationExpressionSyntax objectCreationExpressionSyntax, IReadOnlyList existingArguments, - IReadOnlyList positions, - IMethodSymbol methodSymbol, - Func declarationFactory, - SyntaxNode container) + IReadOnlyList constructorParameters, + IReadOnlyList missingArgumentsPositions, + SyntaxNode siblingNode, + Func declarationFactory) { var documentEditor = await DocumentEditor.CreateAsync(context.Document); var syntaxGenerator = documentEditor.Generator; - var declarations = new List(positions.Count); - var newArgumentList = new List(methodSymbol.Parameters.Length); + var declarations = new List(); + var newArgumentList = new List(); - var orderedParameters = methodSymbol.Parameters.OrderBy(parameter => parameter.Ordinal).ToList(); - for (var parameterPosition = 0; parameterPosition < orderedParameters.Count; parameterPosition++) + for (var parameterPosition = 0; parameterPosition < constructorParameters.Count; parameterPosition++) { - var parameterSymbol = orderedParameters[parameterPosition]; - if (positions.Contains(parameterPosition)) + var parameterSymbol = constructorParameters[parameterPosition]; + if (missingArgumentsPositions.Contains(parameterPosition)) { var invocationExpression = syntaxGenerator.SubstituteForInvocationExpression(parameterSymbol); - var withAdditionalAnnotations = declarationFactory( + var declaration = declarationFactory( syntaxGenerator, parameterSymbol, invocationExpression); - declarations.Add(withAdditionalAnnotations); + declarations.Add(declaration); newArgumentList.Add((TArgumentSyntax)syntaxGenerator.Argument(syntaxGenerator.IdentifierName(parameterSymbol.Name))); } @@ -206,75 +223,77 @@ private async Task IntroduceSubstitute( } } - documentEditor.InsertBefore(container, declarations); + documentEditor.InsertBefore(siblingNode, declarations); - var updatedObjectCreationExpression = - UpdateObjectCreationExpression(objectCreationExpressionSyntax, newArgumentList); - documentEditor.ReplaceNode(objectCreationExpressionSyntax, updatedObjectCreationExpression); + documentEditor.ReplaceNode( + objectCreationExpressionSyntax, + UpdateObjectCreationExpression(objectCreationExpressionSyntax, newArgumentList)); return documentEditor.GetChangedDocument(); } - private IEnumerable CreateRefactoringActions( - CodeRefactoringContext context, - SemanticModel semanticModel, - TObjectCreationExpressionSyntax objectCreationExpressionSyntax, - TArgumentListSyntax argumentListSyntax) + private bool HasMissingArgumentAtPosition( + IReadOnlyList orderedParameters, + IReadOnlyList arguments, + int argumentIndexAtSpan) { - var constructorSymbol = GetConstructorSymbolFromExpression(semanticModel, objectCreationExpressionSyntax); - - if (constructorSymbol == null || constructorSymbol.Parameters.Length == 0) + if (argumentIndexAtSpan >= orderedParameters.Count) { - yield break; + return false; } - var arguments = GetArgumentSyntaxNodes(argumentListSyntax, context.Span); + return argumentIndexAtSpan >= arguments.Count || IsMissing(arguments[argumentIndexAtSpan]); + } - var orderedParameters = constructorSymbol.Parameters.OrderBy(parameter => parameter.Ordinal).ToList(); - var argumentIndexAtSpan = FindArgumentIndexAtSpan(arguments, context.Span); - var localSubstituteContainer = FindContainerForLocalSubstitute(objectCreationExpressionSyntax); - var readonlySubstituteContainer = FindContainerForReadonlySubstitute(objectCreationExpressionSyntax); - if (argumentIndexAtSpan < orderedParameters.Count && - (arguments.TryGetElementAt(argumentIndexAtSpan, out var argument) == false || IsMissing(argument))) - { - var substituteType = orderedParameters[argumentIndexAtSpan]; + private IMethodSymbol GetKnownConstructorSymbol(SemanticModel semanticModel, TObjectCreationExpressionSyntax objectCreationExpressionSyntax) + { + var symbol = semanticModel.GetSymbolInfo(objectCreationExpressionSyntax); - var typeName = substituteType.ToMinimalSymbolString(semanticModel); + if (symbol.Symbol is IMethodSymbol methodSymbol) + { + return methodSymbol; + } - if (localSubstituteContainer != null) - { - yield return CodeAction.Create( - $"Introduce local substitute for {typeName}", - token => IntroduceLocalSubstitute(context, objectCreationExpressionSyntax, localSubstituteContainer, arguments, new[] { argumentIndexAtSpan }, constructorSymbol)); - } + var candidateMethodSymbols = symbol.CandidateSymbols.OfType().ToList(); - if (readonlySubstituteContainer != null) - { - yield return CodeAction.Create( - $"Introduce readonly substitute for {typeName}", - token => IntroduceReadonlySubstitute(context, objectCreationExpressionSyntax, readonlySubstituteContainer, arguments, new[] { argumentIndexAtSpan }, constructorSymbol)); - } + if (candidateMethodSymbols.Count == 0 || candidateMethodSymbols.Count > 1) + { + return null; } - var missingParameters = GetMissingParametersPositions(orderedParameters, arguments); - if (missingParameters.Count == 0) + return candidateMethodSymbols.Single(); + } + + private IReadOnlyList GetMissingArgumentsPositions( + IReadOnlyList argumentSyntaxNodes, + IReadOnlyList parameterSymbols) + { + if (parameterSymbols.Count == 0) { - yield break; + return Array.Empty(); } - if (localSubstituteContainer != null) + if (argumentSyntaxNodes.Count == 0) { - yield return CodeAction.Create( - "Introduce local substitutes for missing arguments", - token => IntroduceLocalSubstitute(context, objectCreationExpressionSyntax, localSubstituteContainer, arguments, missingParameters, constructorSymbol)); + return Enumerable.Range(0, parameterSymbols.Count).ToList(); } - if (readonlySubstituteContainer != null) + var result = new List(); + for (var symbolPosition = 0; symbolPosition < parameterSymbols.Count; symbolPosition++) { - yield return CodeAction.Create( - "Introduce readonly substitutes for missing arguments", - token => IntroduceReadonlySubstitute(context, objectCreationExpressionSyntax, readonlySubstituteContainer, arguments, missingParameters, constructorSymbol)); + if (symbolPosition >= argumentSyntaxNodes.Count || IsMissing(argumentSyntaxNodes[symbolPosition])) + { + result.Add(symbolPosition); + } } + + return result; + } + + private int FindArgumentIndexAtSpan(IReadOnlyList argumentSyntaxNodes, TextSpan span) + { + var findArgumentIndexAtSpan = argumentSyntaxNodes.IndexOf(node => node.FullSpan.IntersectsWith(span)); + return findArgumentIndexAtSpan >= 0 ? findArgumentIndexAtSpan : 0; } } } diff --git a/src/NSubstitute.Analyzers.Shared/Extensions/IListExtensions.cs b/src/NSubstitute.Analyzers.Shared/Extensions/IListExtensions.cs deleted file mode 100644 index 4d820586..00000000 --- a/src/NSubstitute.Analyzers.Shared/Extensions/IListExtensions.cs +++ /dev/null @@ -1,19 +0,0 @@ -using System.Collections.Generic; - -namespace NSubstitute.Analyzers.Shared.Extensions -{ - internal static class IListExtensions - { - public static bool TryGetElementAt(this IReadOnlyList list, int index, out T element) - { - if (index < list.Count) - { - element = list[index]; - return true; - } - - element = default; - return false; - } - } -} \ No newline at end of file diff --git a/src/NSubstitute.Analyzers.Shared/Extensions/SyntaxGeneratorExtension.cs b/src/NSubstitute.Analyzers.Shared/Extensions/SyntaxGeneratorExtension.cs index c0417c3c..b0360fdf 100644 --- a/src/NSubstitute.Analyzers.Shared/Extensions/SyntaxGeneratorExtension.cs +++ b/src/NSubstitute.Analyzers.Shared/Extensions/SyntaxGeneratorExtension.cs @@ -3,7 +3,7 @@ namespace NSubstitute.Analyzers.Shared.Extensions { - public static class SyntaxGeneratorExtension + internal static class SyntaxGeneratorExtension { public static SyntaxNode SubstituteForInvocationExpression( this SyntaxGenerator syntaxGenerator, diff --git a/src/NSubstitute.Analyzers.VisualBasic/CodeRefactoringProviders/IntroduceSubstituteCodeRefactoringProvider.cs b/src/NSubstitute.Analyzers.VisualBasic/CodeRefactoringProviders/IntroduceSubstituteCodeRefactoringProvider.cs index 32cff0fa..8545b052 100644 --- a/src/NSubstitute.Analyzers.VisualBasic/CodeRefactoringProviders/IntroduceSubstituteCodeRefactoringProvider.cs +++ b/src/NSubstitute.Analyzers.VisualBasic/CodeRefactoringProviders/IntroduceSubstituteCodeRefactoringProvider.cs @@ -13,10 +13,6 @@ namespace NSubstitute.Analyzers.VisualBasic.CodeRefactoringProviders [ExportCodeRefactoringProvider(LanguageNames.VisualBasic)] internal sealed class IntroduceSubstituteCodeRefactoringProvider : AbstractIntroduceSubstituteCodeRefactoringProvider { - protected override int ReadonlySubstituteDeclarationContainerRawKind { get; } = (int)SyntaxKind.ClassBlock; - - protected override int LocalSubstituteDeclarationContainerRawKind { get; } = (int)SyntaxKind.SubBlock; - protected override IReadOnlyList GetArgumentSyntaxNodes(ArgumentListSyntax argumentListSyntax, TextSpan span) { return argumentListSyntax.Arguments; @@ -35,7 +31,7 @@ protected override bool IsMissing(ArgumentSyntax argumentSyntax) return base.IsMissing(argumentSyntax) || argumentSyntax.IsOmitted; } - protected override SyntaxNode FindContainerForLocalSubstitute(ObjectCreationExpressionSyntax creationExpression) + protected override SyntaxNode FindSiblingNodeForLocalSubstitute(ObjectCreationExpressionSyntax creationExpression) { var container = creationExpression.Ancestors() .FirstOrDefault(ancestor => ancestor.Kind() == SyntaxKind.SubBlock); @@ -48,7 +44,7 @@ protected override SyntaxNode FindContainerForLocalSubstitute(ObjectCreationExpr return null; } - protected override SyntaxNode FindContainerForReadonlySubstitute(SyntaxNode creationExpression) + protected override SyntaxNode FindSiblingNodeForReadonlySubstitute(SyntaxNode creationExpression) { var typeBlockSyntax = creationExpression.Ancestors() .OfType()