diff --git a/src/nunit.analyzers.tests/ClassicModelAssertUsage/AreEqualClassicModelAssertUsageCodeFixTests.cs b/src/nunit.analyzers.tests/ClassicModelAssertUsage/AreEqualClassicModelAssertUsageCodeFixTests.cs index 8c4c79d1..c224c837 100644 --- a/src/nunit.analyzers.tests/ClassicModelAssertUsage/AreEqualClassicModelAssertUsageCodeFixTests.cs +++ b/src/nunit.analyzers.tests/ClassicModelAssertUsage/AreEqualClassicModelAssertUsageCodeFixTests.cs @@ -43,11 +43,11 @@ public void TestMethod() [Test] public void VerifyAreEqualFixWhenToleranceExistsWithNamedArgumentButInNonstandardOrder() { - var code = TestUtility.WrapMethodInClassNamespaceAndAddUsings($@" + var code = TestUtility.WrapMethodInClassNamespaceAndAddUsings(@" public void TestMethod() - {{ + { ↓ClassicAssert.AreEqual(delta: 0.0000001d, actual: 3d, expected: 2d); - }}"); + }"); var fixedCode = TestUtility.WrapMethodInClassNamespaceAndAddUsings(@" public void TestMethod() { @@ -59,11 +59,11 @@ public void TestMethod() [Test] public void VerifyAreEqualFixWithNamedParametersInNonstandardOrder() { - var code = TestUtility.WrapMethodInClassNamespaceAndAddUsings($@" + var code = TestUtility.WrapMethodInClassNamespaceAndAddUsings(@" public void TestMethod() - {{ + { ↓ClassicAssert.AreEqual(actual: 3d, expected: 2d); - }}"); + }"); var fixedCode = TestUtility.WrapMethodInClassNamespaceAndAddUsings(@" public void TestMethod() { @@ -120,6 +120,24 @@ public void TestMethod() RoslynAssert.CodeFix(analyzer, fix, expectedDiagnostic, code, fixedCode, fixTitle: ClassicModelAssertUsageCodeFix.TransformToConstraintModelDescription); } + [Test] + public void VerifyAreEqualFixWithMessageAnd2ParamsInNonstandardOrder() + { + var code = TestUtility.WrapMethodInClassNamespaceAndAddUsings(@" + public void TestMethod() + { + var actual = 3d; + ↓ClassicAssert.AreEqual(delta: 0.0000001d, expected: 2d, actual: actual, args: new[] { ""Guid.NewGuid()"", Guid.NewGuid().ToString() }, message: ""message-id: {0}, {1}""); + }"); + var fixedCode = TestUtility.WrapMethodInClassNamespaceAndAddUsings(@" + public void TestMethod() + { + var actual = 3d; + Assert.That(actual: actual, Is.EqualTo(expected: 2d).Within(0.0000001d), $""message-id: {""Guid.NewGuid()""}, {Guid.NewGuid().ToString() }""); + }"); // TODO: Fix whitespace + RoslynAssert.CodeFix(analyzer, fix, expectedDiagnostic, code, fixedCode, fixTitle: ClassicModelAssertUsageCodeFix.TransformToConstraintModelDescription); + } + [Test] public void VerifyAreEqualFixWhenToleranceExists() { @@ -200,6 +218,22 @@ public void TestMethod() RoslynAssert.CodeFix(analyzer, fix, expectedDiagnostic, code, fixedCode, fixTitle: ClassicModelAssertUsageCodeFix.TransformToConstraintModelDescription); } + [Test] + public void VerifyAreEqualFixWhenToleranceExistsWithMessageAndParamsLiteralExpressionSyntax() + { + var code = TestUtility.WrapMethodInClassNamespaceAndAddUsings(@" + public void TestMethod() + { + ↓ClassicAssert.AreEqual(2d, 3d, 0.0000001d, ""message-id: {0}"", ""mystring""); + }"); + var fixedCode = TestUtility.WrapMethodInClassNamespaceAndAddUsings(@" + public void TestMethod() + { + Assert.That(3d, Is.EqualTo(2d).Within(0.0000001d), $""message-id: {""mystring""}""); + }"); + RoslynAssert.CodeFix(analyzer, fix, expectedDiagnostic, code, fixedCode, fixTitle: ClassicModelAssertUsageCodeFix.TransformToConstraintModelDescription); + } + [Test] public void CodeFixPreservesLineBreakBeforeMessage() { diff --git a/src/nunit.analyzers/ClassicModelAssertUsage/AreEqualClassicModelAssertUsageCodeFix.cs b/src/nunit.analyzers/ClassicModelAssertUsage/AreEqualClassicModelAssertUsageCodeFix.cs index 37401efe..656ca8a2 100644 --- a/src/nunit.analyzers/ClassicModelAssertUsage/AreEqualClassicModelAssertUsageCodeFix.cs +++ b/src/nunit.analyzers/ClassicModelAssertUsage/AreEqualClassicModelAssertUsageCodeFix.cs @@ -1,7 +1,6 @@ using System.Collections.Generic; using System.Collections.Immutable; using System.Composition; -using System.Linq; using Microsoft.CodeAnalysis; using Microsoft.CodeAnalysis.CodeFixes; using Microsoft.CodeAnalysis.CSharp; @@ -17,7 +16,9 @@ public sealed class AreEqualClassicModelAssertUsageCodeFix { public override ImmutableArray FixableDiagnosticIds { get; } = ImmutableArray.Create(AnalyzerIdentifiers.AreEqualUsage); - protected override List UpdateArguments(Diagnostic diagnostic, IReadOnlyDictionary argumentNamesToArguments) + protected override (ArgumentSyntax ActualArgument, ArgumentSyntax ConstraintArgument) UpdateArguments( + Diagnostic diagnostic, + IReadOnlyDictionary argumentNamesToArguments) { var expectedArgument = argumentNamesToArguments[NUnitFrameworkConstants.NameOfExpectedParameter]; var actualArgument = argumentNamesToArguments[NUnitFrameworkConstants.NameOfActualParameter]; @@ -50,18 +51,7 @@ protected override List UpdateArguments(Diagnostic diagnostic, I SyntaxFactory.SingletonSeparatedList(toleranceArgumentNoColon))); } - var arguments = new List() { actualArgument, SyntaxFactory.Argument(equalToInvocationNode) }; - var handledParameterNames = new[] - { - NUnitFrameworkConstants.NameOfExpectedParameter, - NUnitFrameworkConstants.NameOfActualParameter, - NameOfDeltaParameter, - }; - return arguments - .Concat(argumentNamesToArguments - .Where(kvp => !handledParameterNames.Contains(kvp.Key)) - .Select(kvp => kvp.Value)) - .ToList(); + return (actualArgument, SyntaxFactory.Argument(equalToInvocationNode)); } } } diff --git a/src/nunit.analyzers/ClassicModelAssertUsage/ClassicModelAssertUsageCodeFix.cs b/src/nunit.analyzers/ClassicModelAssertUsage/ClassicModelAssertUsageCodeFix.cs index 591d5813..fcf3ec36 100644 --- a/src/nunit.analyzers/ClassicModelAssertUsage/ClassicModelAssertUsageCodeFix.cs +++ b/src/nunit.analyzers/ClassicModelAssertUsage/ClassicModelAssertUsageCodeFix.cs @@ -1,12 +1,14 @@ using System; using System.Collections.Generic; using System.Linq; +using System.Reflection; using System.Threading.Tasks; using Microsoft.CodeAnalysis; using Microsoft.CodeAnalysis.CodeActions; using Microsoft.CodeAnalysis.CodeFixes; using Microsoft.CodeAnalysis.CSharp; using Microsoft.CodeAnalysis.CSharp.Syntax; +using NUnit.Analyzers.Constants; using NUnit.Analyzers.Extensions; using NUnit.Analyzers.Helpers; @@ -52,34 +54,65 @@ public sealed override async Task RegisterCodeFixesAsync(CodeFixContext context) if (newInvocationNode is null) return; - // Now, replace the arguments. - List arguments = invocationNode.ArgumentList.Arguments.ToList(); + var methodSymbol = (IMethodSymbol)semanticModel.GetSymbolInfo(invocationNode).Symbol!; - // See if we need to cast the arguments when they were using a specific classic overload. - arguments[0] = CastIfNecessary(arguments[0]); - if (arguments.Count > 1) - arguments[1] = CastIfNecessary(arguments[1]); + Dictionary argumentNamesToArguments = new(); + const string NameOfArgsParameter = "args"; - var argumentNamesToArguments = arguments.ToDictionary( - argument => GetParameterName(argument), - argument => argument); + // There can be 0 to any number of message and parameters. + List messageAndParams = new(); + var arguments = invocationNode.ArgumentList.Arguments.ToList(); + for (var i = 0; i < arguments.Count; ++i) + { + var argument = arguments[i]; + if (i < methodSymbol.Parameters.Length + && (argument.NameColon?.Name.Identifier.Text ?? methodSymbol.Parameters[i].Name) is { } argumentName + && argumentName is not (NUnitFrameworkConstants.NameOfMessageParameter or NameOfArgsParameter)) + { + // See if we need to cast the arguments when they were using a specific classic overload. + argumentNamesToArguments[argumentName] = + argumentName is NUnitFrameworkConstants.NameOfExpectedParameter or NUnitFrameworkConstants.NameOfActualParameter + ? CastIfNecessary(argument) + : argument; + } + else + { + messageAndParams.Add(argument); + } + } + + // Remove null message to avoid ambiguous calls. + if (argumentNamesToArguments.TryGetValue(NUnitFrameworkConstants.NameOfMessageParameter, out var messageArgument) + && messageArgument.Expression.IsKind(SyntaxKind.NullLiteralExpression)) + { + argumentNamesToArguments.Remove(NUnitFrameworkConstants.NameOfMessageParameter); + } + + // Now, replace the arguments. + List newArguments = new(); + ArgumentSyntax actualArgument, constraintArgument; // Do the rule specific conversion if (typeArguments is null) - arguments = this.UpdateArguments(diagnostic, argumentNamesToArguments); + { + (actualArgument, constraintArgument) = this.UpdateArguments(diagnostic, argumentNamesToArguments); + newArguments.Add(actualArgument); + newArguments.Add(constraintArgument); + if (CodeFixHelper.GetInterpolatedMessageArgumentOrDefault(messageAndParams) is { } interpolatedMessageArgument) + { + newArguments.Add(interpolatedMessageArgument); + } + } else + { this.UpdateArguments(diagnostic, arguments, typeArguments); - // Remove null message to avoid ambiguous calls. - if (arguments.Count == 3 && arguments[2].Expression.IsKind(SyntaxKind.NullLiteralExpression)) - { - arguments.RemoveAt(2); + // Do the format spec, params to formattable string conversion + CodeFixHelper.UpdateStringFormatToFormattableString(arguments, this.MinimumNumberOfParameters); + newArguments = arguments; } - // Do the format spec, params to formattable string conversion - CodeFixHelper.UpdateStringFormatToFormattableString(arguments, this.MinimumNumberOfParameters); - - var newArgumentsList = invocationNode.ArgumentList.WithArguments(arguments); + var newArgumentsList = invocationNode.ArgumentList.WithArguments(newArguments); newInvocationNode = newInvocationNode.WithArgumentList(newArgumentsList); context.CancellationToken.ThrowIfCancellationRequested(); @@ -105,16 +138,6 @@ ArgumentSyntax CastIfNecessary(ArgumentSyntax argument) argument.Expression)); } - string GetParameterName(ArgumentSyntax argument) - { - if (argument.NameColon?.Name.Identifier.Text is { } argumentName) - return argumentName; - - var methodSymbol = (IMethodSymbol)semanticModel.GetSymbolInfo(invocationNode).Symbol!; - var index = invocationNode.ArgumentList.Arguments.IndexOf(argument); - return methodSymbol!.Parameters[index].Name; - } - string? GetUserDefinedImplicitTypeConversion(ExpressionSyntax expression) { var typeInfo = semanticModel.GetTypeInfo(expression, context.CancellationToken); @@ -135,7 +158,7 @@ string GetParameterName(ArgumentSyntax argument) } } - protected virtual List UpdateArguments( + protected virtual (ArgumentSyntax ActualArgument, ArgumentSyntax ConstraintArgument) UpdateArguments( Diagnostic diagnostic, IReadOnlyDictionary argumentNamesToArguments) { diff --git a/src/nunit.analyzers/Constants/NUnitFrameworkConstants.cs b/src/nunit.analyzers/Constants/NUnitFrameworkConstants.cs index 067c147b..57dfa23c 100644 --- a/src/nunit.analyzers/Constants/NUnitFrameworkConstants.cs +++ b/src/nunit.analyzers/Constants/NUnitFrameworkConstants.cs @@ -192,6 +192,7 @@ public static class NUnitFrameworkConstants public const string NameOfConditionParameter = "condition"; public const string NameOfExpectedParameter = "expected"; public const string NameOfExpressionParameter = "expression"; + public const string NameOfMessageParameter = "message"; public const string NameOfConstraintExpressionAnd = "And"; public const string NameOfConstraintExpressionOr = "Or"; diff --git a/src/nunit.analyzers/Helpers/CodeFixHelper.cs b/src/nunit.analyzers/Helpers/CodeFixHelper.cs index f1ac40a5..c20b2af2 100644 --- a/src/nunit.analyzers/Helpers/CodeFixHelper.cs +++ b/src/nunit.analyzers/Helpers/CodeFixHelper.cs @@ -35,6 +35,42 @@ internal static class CodeFixHelper return invocationExpression.ReplaceNode(invocationTargetNode, newInvocationTargetNode); } + /// + /// This is assumed to be arguments for an 'Assert.That(actual, constraint, "...: {0} - {1}", param0, param1)` + /// which needs converting into 'Assert.That(actual, constraint, $"...: {param0} - {param1}"). + /// + /// The arguments passed to the 'Assert' method. + /// The argument needed for the actual method, any more are assumed messages. + public static ArgumentSyntax? GetInterpolatedMessageArgumentOrDefault(List messageAndParams) + { + if (messageAndParams.Count == 0) + return null; + + var messageArgument = messageAndParams.SingleOrDefault(a => a.NameColon?.Name.Identifier.Text == NUnitFrameworkConstants.NameOfMessageParameter) + ?? messageAndParams.First(); + var formatSpecificationArgument = messageArgument.Expression; + if (formatSpecificationArgument.IsKind(SyntaxKind.NullLiteralExpression)) + return null; + + // We only support converting if the format specification is a constant string. + if (messageAndParams.Count == 1 || formatSpecificationArgument is not LiteralExpressionSyntax literalExpression) + return messageAndParams[0]; + + var formatSpecification = literalExpression.Token.ValueText; + + // var formatArgumentExpressions = new List(capacity: messageAndParams.Count - 1); + var argsExpression = messageAndParams.Single(a => a != messageArgument).Expression; + var formatArgumentExpressions = argsExpression is ImplicitArrayCreationExpressionSyntax args + ? args.Initializer.Expressions.ToArray() + : new[] { argsExpression }; + + var interpolatedStringContent = UpdateStringFormatToFormattableString(formatSpecification, formatArgumentExpressions); + var interpolatedString = SyntaxFactory.InterpolatedStringExpression( + SyntaxFactory.Token(SyntaxKind.InterpolatedStringStartToken), + SyntaxFactory.List(interpolatedStringContent)); + return SyntaxFactory.Argument(interpolatedString); + } + /// /// This is assumed to be arguments for an 'Assert.That(actual, constraint, "...: {0} - {1}", param0, param1)` /// which needs converting into 'Assert.That(actual, constraint, $"...: {param0} - {param1}"). diff --git a/src/nunit.analyzers/UpdateStringFormatToInterpolatableString/UpdateStringFormatToInterpolatableStringAnalyzer.cs b/src/nunit.analyzers/UpdateStringFormatToInterpolatableString/UpdateStringFormatToInterpolatableStringAnalyzer.cs index 980e6e1d..c6a00a55 100644 --- a/src/nunit.analyzers/UpdateStringFormatToInterpolatableString/UpdateStringFormatToInterpolatableStringAnalyzer.cs +++ b/src/nunit.analyzers/UpdateStringFormatToInterpolatableString/UpdateStringFormatToInterpolatableStringAnalyzer.cs @@ -106,7 +106,7 @@ private static void AnalyzeNUnit4AssertInvocation(OperationAnalysisContext conte IParameterSymbol parameter = parameters[formatParameterIndex]; if (parameter.IsOptional) { - if (parameter.Name == "message") + if (parameter.Name == NUnitFrameworkConstants.NameOfMessageParameter) break; // Overload with FormattableString or Func overload