Skip to content

Commit

Permalink
Remove duplication between 2 ToFormattableString methods
Browse files Browse the repository at this point in the history
  • Loading branch information
manfred-brands committed Apr 10, 2024
1 parent 1982668 commit b84d8e0
Show file tree
Hide file tree
Showing 4 changed files with 33 additions and 67 deletions.
2 changes: 1 addition & 1 deletion src/nunit.analyzers.tests/nunit.analyzers.tests.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
<PropertyGroup>
<RootNamespace>NUnit.Analyzers.Tests</RootNamespace>
<TargetFrameworks>net6.0;net462</TargetFrameworks>
<NUnitVersion Condition="'$(NUnitVersion)'==''">4</NUnitVersion>
<NUnitVersion Condition="'$(NUnitVersion)'==''">3</NUnitVersion>
</PropertyGroup>

<PropertyGroup Condition="'$(NUnitVersion)' == '4'">
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ public sealed override async Task RegisterCodeFixesAsync(CodeFixContext context)
bool argsIsArray = !string.IsNullOrEmpty(diagnostic.Properties[AnalyzerPropertyKeys.ArgsIsArray]);

argumentNamesToArguments.TryGetValue(NUnitFrameworkConstants.NameOfMessageParameter, out ArgumentSyntax? messageArgument);
if (CodeFixHelper.GetInterpolatedMessageArgumentOrDefault(argsIsArray, messageArgument, args) is ArgumentSyntax interpolatedMessageArgument)
if (CodeFixHelper.GetInterpolatedMessageArgumentOrDefault(false, argsIsArray, messageArgument, args) is ArgumentSyntax interpolatedMessageArgument)
newArguments.Add(interpolatedMessageArgument);

var newArgumentsList = invocationNode.ArgumentList.WithArguments(newArguments);
Expand Down
94 changes: 30 additions & 64 deletions src/nunit.analyzers/Helpers/CodeFixHelper.cs
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ internal static class CodeFixHelper
/// </summary>
/// <param name="arguments">The arguments passed to the 'Assert' method. </param>
/// <param name="minimumNumberOfArguments">The argument needed for the actual method, any more are assumed messages.</param>
public static ArgumentSyntax? GetInterpolatedMessageArgumentOrDefault(bool argsIsArray, ArgumentSyntax? messageArgument, List<ArgumentSyntax> args)
public static ArgumentSyntax? GetInterpolatedMessageArgumentOrDefault(bool unconditional, bool argsIsArray, ArgumentSyntax? messageArgument, List<ArgumentSyntax> args)
{
if (messageArgument is null)
return null;
Expand Down Expand Up @@ -74,16 +74,24 @@ internal static class CodeFixHelper
// If this is the case convert code to use () => string.Format(format, args)
if (formatSpecificationArgument is not LiteralExpressionSyntax literalExpression || argsIsArray)
{
var stringFormatLambda = SyntaxFactory.ParenthesizedLambdaExpression(
SyntaxFactory.InvocationExpression(
SyntaxFactory.MemberAccessExpression(
SyntaxKind.SimpleMemberAccessExpression,
SyntaxFactory.PredefinedType(
SyntaxFactory.Token(SyntaxKind.StringKeyword)),
SyntaxFactory.IdentifierName(nameof(string.Format))))
.WithArgumentList(SyntaxFactory.ArgumentList(SyntaxFactory.SeparatedList(
args.Select(x => x.WithNameColon(null)).Prepend(messageArgument)))));
return SyntaxFactory.Argument(stringFormatLambda);
var stringFormat = SyntaxFactory.InvocationExpression(
SyntaxFactory.MemberAccessExpression(
SyntaxKind.SimpleMemberAccessExpression,
SyntaxFactory.PredefinedType(
SyntaxFactory.Token(SyntaxKind.StringKeyword)),
SyntaxFactory.IdentifierName(nameof(string.Format))))
.WithArgumentList(SyntaxFactory.ArgumentList(SyntaxFactory.SeparatedList(
args.Select(x => x.WithNameColon(null)).Prepend(messageArgument))));

if (unconditional)
{
return SyntaxFactory.Argument(stringFormat);
}
else
{
var stringFormatLambda = SyntaxFactory.ParenthesizedLambdaExpression(stringFormat);
return SyntaxFactory.Argument(stringFormatLambda);
}
}

var formatSpecification = literalExpression.Token.ValueText;
Expand All @@ -103,68 +111,26 @@ internal static class CodeFixHelper
/// </summary>
/// <param name="arguments">The arguments passed to the 'Assert' method. </param>
/// <param name="minimumNumberOfArguments">The argument needed for the actual method, any more are assumed messages.</param>
public static void UpdateStringFormatToFormattableString(List<ArgumentSyntax> arguments, int minimumNumberOfArguments, bool argsIsArray)
public static void UpdateStringFormatToFormattableString(bool argsIsArray, List<ArgumentSyntax> arguments, int minimumNumberOfArguments)
{
int firstParamsArgument = minimumNumberOfArguments + 1;

// If only 1 extra argument is passed, it must be a non-formattable message.
if (arguments.Count <= firstParamsArgument)
if (arguments.Count <= minimumNumberOfArguments + 1)
return;

ExpressionSyntax formatSpecificationArgument = arguments[minimumNumberOfArguments].Expression;
ArgumentSyntax? message = GetInterpolatedMessageArgumentOrDefault(
minimumNumberOfArguments == 0,
argsIsArray,
arguments[minimumNumberOfArguments],
arguments.Skip(minimumNumberOfArguments + 1).ToList());

// We cannot convert to an interpolated string if:
// - The format specification is not a literal we can analyze.
// - The 'args' argument is a real array instead of a list of params array.
// If this is the case convert code to use () => string.Format(format, args)
if (formatSpecificationArgument is not LiteralExpressionSyntax literalExpression || argsIsArray)
var nextArgument = minimumNumberOfArguments;
if (message is not null)
{
var stringFormat =
SyntaxFactory.InvocationExpression(
SyntaxFactory.MemberAccessExpression(
SyntaxKind.SimpleMemberAccessExpression,
SyntaxFactory.PredefinedType(
SyntaxFactory.Token(SyntaxKind.StringKeyword)),
SyntaxFactory.IdentifierName(nameof(string.Format))))
.WithArgumentList(SyntaxFactory.ArgumentList(SyntaxFactory.SeparatedList(
arguments.Skip(minimumNumberOfArguments).Select(x => x.WithNameColon(null)))));

if (minimumNumberOfArguments == 0)
{
// Replace format specification argument with unconditional string.Format call.
arguments[0] = SyntaxFactory.Argument(stringFormat);
}
else
{
// Replace format specification argument with () => string.Format lambda.
var stringFormatLambda = SyntaxFactory.ParenthesizedLambdaExpression(stringFormat);
arguments[minimumNumberOfArguments] = SyntaxFactory.Argument(stringFormatLambda);
}
}
else
{
string formatSpecification = literalExpression.Token.ValueText;

int formatArgumentCount = arguments.Count - firstParamsArgument;
ExpressionSyntax[] formatArguments = new ExpressionSyntax[formatArgumentCount];
for (int i = 0; i < formatArgumentCount; i++)
{
formatArguments[i] = arguments[firstParamsArgument + i].Expression;
}

IEnumerable<InterpolatedStringContentSyntax> interpolatedStringContent =
UpdateStringFormatToFormattableString(formatSpecification, formatArguments);

InterpolatedStringExpressionSyntax interpolatedString = SyntaxFactory.InterpolatedStringExpression(
SyntaxFactory.Token(SyntaxKind.InterpolatedStringStartToken),
SyntaxFactory.List(interpolatedStringContent));

// Replace format specification argument with interpolated string.
arguments[minimumNumberOfArguments] = SyntaxFactory.Argument(interpolatedString);
arguments[minimumNumberOfArguments] = message;
nextArgument++;
}

// Delete remaining arguments.
var nextArgument = minimumNumberOfArguments + 1;
arguments.RemoveRange(nextArgument, arguments.Count - nextArgument);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ public override async Task RegisterCodeFixesAsync(CodeFixContext context)
var minimumNumberOfArguments = int.Parse(diagnostic.Properties[AnalyzerPropertyKeys.MinimumNumberOfArguments]!, CultureInfo.InvariantCulture);
bool argsIsArray = !string.IsNullOrEmpty(diagnostic.Properties[AnalyzerPropertyKeys.ArgsIsArray]);

CodeFixHelper.UpdateStringFormatToFormattableString(arguments, minimumNumberOfArguments, argsIsArray);
CodeFixHelper.UpdateStringFormatToFormattableString(argsIsArray, arguments, minimumNumberOfArguments);

var newArgumentsList = invocationNode.ArgumentList.WithArguments(arguments);
var newInvocationNode = invocationNode.WithArgumentList(newArgumentsList);
Expand Down

0 comments on commit b84d8e0

Please sign in to comment.