diff --git a/src/nunit.analyzers.tests/ClassicModelAssertUsage/AreEqualClassicModelAssertUsageCodeFixTests.cs b/src/nunit.analyzers.tests/ClassicModelAssertUsage/AreEqualClassicModelAssertUsageCodeFixTests.cs index b3fb803d..eea3bf9c 100644 --- a/src/nunit.analyzers.tests/ClassicModelAssertUsage/AreEqualClassicModelAssertUsageCodeFixTests.cs +++ b/src/nunit.analyzers.tests/ClassicModelAssertUsage/AreEqualClassicModelAssertUsageCodeFixTests.cs @@ -252,6 +252,48 @@ public void TestMethod() RoslynAssert.CodeFix(analyzer, fix, expectedDiagnostic, code, fixedCode, fixTitle: ClassicModelAssertUsageCodeFix.TransformToConstraintModelDescription); } + [Test] + public void VerifyAreEqualFixWhenWithMessageAndArgsArray() + { + var code = TestUtility.WrapMethodInClassNamespaceAndAddUsings(@" + [TestCase(3.0, 3)] + public void Test(object actual, object expected) + { + object[] args = { expected, actual }; + ↓ClassicAssert.AreEqual(expected, actual, ""Expected: {0} Got: {1}"", args); + }"); + var fixedCode = TestUtility.WrapMethodInClassNamespaceAndAddUsings(@" + [TestCase(3.0, 3)] + public void Test(object actual, object expected) + { + object[] args = { expected, actual }; + Assert.That(actual, Is.EqualTo(expected), () => string.Format(""Expected: {0} Got: {1}"", args)); + }"); + RoslynAssert.CodeFix(analyzer, fix, expectedDiagnostic, code, fixedCode, fixTitle: ClassicModelAssertUsageCodeFix.TransformToConstraintModelDescription); + } + + [Test] + public void VerifyAreEqualFixWhenWithMessageVariable() + { + var code = TestUtility.WrapMethodInClassNamespaceAndAddUsings(@" + [TestCase(3.0, 3)] + public void Test(object actual, object expected) + { + ↓ClassicAssert.AreEqual(expected, actual, GetLocalizedFormatSpecification(), expected, actual); + } + private static string GetLocalizedFormatSpecification() => ""Expected: {0} Got: {1}""; + "); + var fixedCode = TestUtility.WrapMethodInClassNamespaceAndAddUsings(@" + [TestCase(3.0, 3)] + public void Test(object actual, object expected) + { + Assert.That(actual, Is.EqualTo(expected), () => string.Format(GetLocalizedFormatSpecification(), expected, actual)); + } + private static string GetLocalizedFormatSpecification() => ""Expected: {0} Got: {1}""; + "); + RoslynAssert.CodeFix(analyzer, fix, expectedDiagnostic, code, fixedCode, fixTitle: ClassicModelAssertUsageCodeFix.TransformToConstraintModelDescription); + } + [Test] public void CodeFixPreservesLineBreakBeforeMessage() { diff --git a/src/nunit.analyzers.tests/CollectionAssertUsage/CollectionAssertUsageCodeFixTests.cs b/src/nunit.analyzers.tests/CollectionAssertUsage/CollectionAssertUsageCodeFixTests.cs index 847992bd..665d555e 100644 --- a/src/nunit.analyzers.tests/CollectionAssertUsage/CollectionAssertUsageCodeFixTests.cs +++ b/src/nunit.analyzers.tests/CollectionAssertUsage/CollectionAssertUsageCodeFixTests.cs @@ -171,6 +171,42 @@ public void AnalyzeTwoCollectionWhenFormatAndTwoParamsArgumentsAreUsed(string me RoslynAssert.CodeFix(analyzer, fix, expectedDiagnostic, code, fixedCode); } + [TestCaseSource(nameof(TwoCollectionParameterAsserts))] + public void AnalyzeTwoCollectionWhenFormatAndArgsArrayAreUsed(string method) + { + var code = TestUtility.WrapInTestMethod(@$" + var collection1 = new[] {{ 1, 2, 3 }}; + var collection2 = new[] {{ 2, 4, 6 }}; + var args = new[] {{ ""first"", ""second"" }}; + ↓CollectionAssert.{method}(collection1, collection2, ""{{0}}, {{1}}"", args); + "); + var fixedCode = TestUtility.WrapInTestMethod(@$" + var collection1 = new[] {{ 1, 2, 3 }}; + var collection2 = new[] {{ 2, 4, 6 }}; + var args = new[] {{ ""first"", ""second"" }}; + Assert.That({GetAdjustedTwoCollectionConstraint(method)}, () => string.Format(""{{0}}, {{1}}"", args)); + "); + RoslynAssert.CodeFix(analyzer, fix, expectedDiagnostic, code, fixedCode); + } + + [TestCaseSource(nameof(TwoCollectionParameterAsserts))] + public void AnalyzeTwoCollectionWhenFormatVariableAndTwoParamsArgumentsAreUsed(string method) + { + var code = TestUtility.WrapInTestMethod(@$" + var collection1 = new[] {{ 1, 2, 3 }}; + var collection2 = new[] {{ 2, 4, 6 }}; + const string formatSpecification = ""{{0}}, {{1}}""; + ↓CollectionAssert.{method}(collection1, collection2, formatSpecification, ""first"", ""second""); + "); + var fixedCode = TestUtility.WrapInTestMethod(@$" + var collection1 = new[] {{ 1, 2, 3 }}; + var collection2 = new[] {{ 2, 4, 6 }}; + const string formatSpecification = ""{{0}}, {{1}}""; + Assert.That({GetAdjustedTwoCollectionConstraint(method)}, () => string.Format(formatSpecification, ""first"", ""second"")); + "); + RoslynAssert.CodeFix(analyzer, fix, expectedDiagnostic, code, fixedCode); + } + [TestCaseSource(nameof(TwoCollectionParameterAsserts))] public void AnalyzeTwoCollectionWhenFormatAndParamsArgumentsAreUsedOutOfOrder(string method) { diff --git a/src/nunit.analyzers.tests/StringAssertUsage/StringAssertUsageCodeFixTests.cs b/src/nunit.analyzers.tests/StringAssertUsage/StringAssertUsageCodeFixTests.cs index 6e65380d..c5d0867f 100644 --- a/src/nunit.analyzers.tests/StringAssertUsage/StringAssertUsageCodeFixTests.cs +++ b/src/nunit.analyzers.tests/StringAssertUsage/StringAssertUsageCodeFixTests.cs @@ -53,6 +53,34 @@ public void AnalyzeWhenFormatAndArgumentsAreUsed(string method) RoslynAssert.CodeFix(analyzer, fix, expectedDiagnostic, code, fixedCode); } + [TestCaseSource(nameof(StringAsserts))] + public void AnalyzeWhenFormatAndArgsArrayAreUsed(string method) + { + var code = TestUtility.WrapInTestMethod(@$" + object[] args = {{ ""message"" }}; + ↓StringAssert.{method}(""expected"", ""actual"", ""Because of {{0}}"", args); + "); + var fixedCode = TestUtility.WrapInTestMethod(@$" + object[] args = {{ ""message"" }}; + Assert.That(""actual"", {GetAdjustedConstraint(method)}, () => string.Format(""Because of {{0}}"", args)); + "); + RoslynAssert.CodeFix(analyzer, fix, expectedDiagnostic, code, fixedCode); + } + + [TestCaseSource(nameof(StringAsserts))] + public void AnalyzeWhenFormatVariableAndArgumentsAreUsed(string method) + { + var code = TestUtility.WrapInTestMethod(@$" + ↓StringAssert.{method}(""expected"", ""actual"", GetLocalizedFormatSpecification(), ""message""); + static string GetLocalizedFormatSpecification() => ""Because of {{0}}""; + "); + var fixedCode = TestUtility.WrapInTestMethod(@$" + Assert.That(""actual"", {GetAdjustedConstraint(method)}, () => string.Format(GetLocalizedFormatSpecification(), ""message"")); + static string GetLocalizedFormatSpecification() => ""Because of {{0}}""; + "); + RoslynAssert.CodeFix(analyzer, fix, expectedDiagnostic, code, fixedCode); + } + [TestCaseSource(nameof(StringAsserts))] public void AnalyzeWhenFormatAndArgumentsAreUsedOutOfOrder(string method) { diff --git a/src/nunit.analyzers.tests/UpdateStringFormatToInterpolatableString/UpdateStringFormatToInterpolatableStringCodeFixTests.cs b/src/nunit.analyzers.tests/UpdateStringFormatToInterpolatableString/UpdateStringFormatToInterpolatableStringCodeFixTests.cs index 5ebfc285..6edbbaff 100644 --- a/src/nunit.analyzers.tests/UpdateStringFormatToInterpolatableString/UpdateStringFormatToInterpolatableStringCodeFixTests.cs +++ b/src/nunit.analyzers.tests/UpdateStringFormatToInterpolatableString/UpdateStringFormatToInterpolatableStringCodeFixTests.cs @@ -47,6 +47,26 @@ public void AccidentallyUseFormatSpecification(string assertOrAssume) }}"); RoslynAssert.CodeFix(analyzer, fix, expectedDiagnostic, code, fixedCode); } + + [TestCaseSource(nameof(AssertAndAssume))] + public void AccidentallyUseFormatSpecificationWithVariable(string assertOrAssume) + { + var code = TestUtility.WrapMethodInClassNamespaceAndAddUsings(@$" + [TestCase(""NUnit 4.0"", ""NUnit 3.14"")] + public void {assertOrAssume}Something(string actual, string expected) + {{ + const string formatSpecification = ""Expected '{{0}}', but got: {{1}}""; + ↓{assertOrAssume}.That(actual, Is.EqualTo(expected).IgnoreCase, formatSpecification, expected, actual); + }}"); + var fixedCode = TestUtility.WrapMethodInClassNamespaceAndAddUsings(@$" + [TestCase(""NUnit 4.0"", ""NUnit 3.14"")] + public void {assertOrAssume}Something(string actual, string expected) + {{ + const string formatSpecification = ""Expected '{{0}}', but got: {{1}}""; + {assertOrAssume}.That(actual, Is.EqualTo(expected).IgnoreCase, () => string.Format(formatSpecification, expected, actual)); + }}"); + RoslynAssert.CodeFix(analyzer, fix, expectedDiagnostic, code, fixedCode); + } #else [TestCase(NUnitFrameworkConstants.NameOfAssertIgnore)] public void ConvertWhenNoMinimumParameters(string method) @@ -60,6 +80,34 @@ public void ConvertWhenNoMinimumParameters(string method) RoslynAssert.CodeFix(analyzer, fix, expectedDiagnostic, code, fixedCode); } + [TestCase(NUnitFrameworkConstants.NameOfAssertIgnore)] + public void ConvertWhenNoMinimumParametersAndArgsArray(string method) + { + var code = TestUtility.WrapInTestMethod(@$" + object[] args = {{ ""{method}"" }}; + ↓Assert.{method}(""Method: {{0}}"", args); + "); + var fixedCode = TestUtility.WrapInTestMethod($@" + object[] args = {{ ""{method}"" }}; + Assert.{method}(string.Format(""Method: {{0}}"", args)); + "); + RoslynAssert.CodeFix(analyzer, fix, expectedDiagnostic, code, fixedCode); + } + + [TestCase(NUnitFrameworkConstants.NameOfAssertIgnore)] + public void ConvertWhenNoMinimumParametersVariableFormatSpecification(string method) + { + var code = TestUtility.WrapInTestMethod(@$" + const string formatSpecification = ""Method: {{0}}""; + ↓Assert.{method}(formatSpecification, ""{method}""); + "); + var fixedCode = TestUtility.WrapInTestMethod($@" + const string formatSpecification = ""Method: {{0}}""; + Assert.{method}(string.Format(formatSpecification, ""{method}"")); + "); + RoslynAssert.CodeFix(analyzer, fix, expectedDiagnostic, code, fixedCode); + } + [TestCaseSource(nameof(AssertAndAssume))] public void ConvertWhenMinimumParametersIsOne(string assertOrAssume) { @@ -74,6 +122,38 @@ public void ConvertWhenMinimumParametersIsOne(string assertOrAssume) RoslynAssert.CodeFix(analyzer, fix, expectedDiagnostic, code, fixedCode); } + [TestCaseSource(nameof(AssertAndAssume))] + public void ConvertWhenMinimumParametersIsOneAndArgsArray(string assertOrAssume) + { + var code = TestUtility.WrapInTestMethod(@$" + const bool actual = false; + object[] args = {{ actual }}; + ↓{assertOrAssume}.That(actual, ""Expected actual value to be true, but was: {{0}}"", args); + "); + var fixedCode = TestUtility.WrapInTestMethod(@$" + const bool actual = false; + object[] args = {{ actual }}; + {assertOrAssume}.That(actual, () => string.Format(""Expected actual value to be true, but was: {{0}}"", args)); + "); + RoslynAssert.CodeFix(analyzer, fix, expectedDiagnostic, code, fixedCode); + } + + [TestCaseSource(nameof(AssertAndAssume))] + public void ConvertWhenMinimumParametersIsOneAndVariableFormatSpecification(string assertOrAssume) + { + var code = TestUtility.WrapInTestMethod(@$" + const bool actual = false; + const string formatSpecification = ""Expected actual value to be true, but was: {{0}}""; + ↓{assertOrAssume}.That(actual, formatSpecification, actual); + "); + var fixedCode = TestUtility.WrapInTestMethod(@$" + const bool actual = false; + const string formatSpecification = ""Expected actual value to be true, but was: {{0}}""; + {assertOrAssume}.That(actual, () => string.Format(formatSpecification, actual)); + "); + RoslynAssert.CodeFix(analyzer, fix, expectedDiagnostic, code, fixedCode); + } + [TestCaseSource(nameof(AssertAndAssume))] public void ConvertWhenMinimumParametersIsTwo(string assertOrAssume) { @@ -88,6 +168,38 @@ public void ConvertWhenMinimumParametersIsTwo(string assertOrAssume) RoslynAssert.CodeFix(analyzer, fix, expectedDiagnostic, code, fixedCode); } + [TestCaseSource(nameof(AssertAndAssume))] + public void ConvertWhenMinimumParametersIsTwoAndArgsArray(string assertOrAssume) + { + var code = TestUtility.WrapInTestMethod(@$" + const int actual = 42; + object[] args = {{ actual, DateTime.Now }}; + ↓{assertOrAssume}.That(actual, Is.EqualTo(42), ""Expected actual value to be 42, but was: {{0}} at time {{1:HH:mm:ss}}"", args); + "); + var fixedCode = TestUtility.WrapInTestMethod(@$" + const int actual = 42; + object[] args = {{ actual, DateTime.Now }}; + {assertOrAssume}.That(actual, Is.EqualTo(42), () => string.Format(""Expected actual value to be 42, but was: {{0}} at time {{1:HH:mm:ss}}"", args)); + "); + RoslynAssert.CodeFix(analyzer, fix, expectedDiagnostic, code, fixedCode); + } + + [TestCaseSource(nameof(AssertAndAssume))] + public void ConvertWhenMinimumParametersIsTwoAndVariableFormatSpecification(string assertOrAssume) + { + var code = TestUtility.WrapInTestMethod(@$" + const int actual = 42; + const string formatSpecification = ""Expected actual value to be 42, but was: {{0}} at time {{1:HH:mm:ss}}""; + ↓{assertOrAssume}.That(actual, Is.EqualTo(42), formatSpecification, actual, DateTime.Now); + "); + var fixedCode = TestUtility.WrapInTestMethod(@$" + const int actual = 42; + const string formatSpecification = ""Expected actual value to be 42, but was: {{0}} at time {{1:HH:mm:ss}}""; + {assertOrAssume}.That(actual, Is.EqualTo(42), () => string.Format(formatSpecification, actual, DateTime.Now)); + "); + RoslynAssert.CodeFix(analyzer, fix, expectedDiagnostic, code, fixedCode); + } + // We need to double the backslashes as the text is formatted and we want the code to contain \n and not LF [TestCase("Passed: {0}")] [TestCase("Passed: {0} {{")] diff --git a/src/nunit.analyzers/ClassicModelAssertUsage/ClassicModelAssertUsageAnalyzer.cs b/src/nunit.analyzers/ClassicModelAssertUsage/ClassicModelAssertUsageAnalyzer.cs index c2d479b3..875c2d75 100644 --- a/src/nunit.analyzers/ClassicModelAssertUsage/ClassicModelAssertUsageAnalyzer.cs +++ b/src/nunit.analyzers/ClassicModelAssertUsage/ClassicModelAssertUsageAnalyzer.cs @@ -4,6 +4,7 @@ using Microsoft.CodeAnalysis.Diagnostics; using Microsoft.CodeAnalysis.Operations; using NUnit.Analyzers.Constants; +using NUnit.Analyzers.Helpers; using static NUnit.Analyzers.Constants.NUnitFrameworkConstants; namespace NUnit.Analyzers.ClassicModelAssertUsage @@ -236,23 +237,15 @@ public sealed class ClassicModelAssertUsageAnalyzer : ClassicAssertionAnalyzer protected override void AnalyzeAssertInvocation(OperationAnalysisContext context, IInvocationOperation assertOperation) { - var methodSymbol = assertOperation.TargetMethod; + string methodName = assertOperation.TargetMethod.Name; - if (ClassicModelAssertUsageAnalyzer.NameToDescriptor.TryGetValue(methodSymbol.Name, out DiagnosticDescriptor? descriptor)) + if (ClassicModelAssertUsageAnalyzer.NameToDescriptor.TryGetValue(methodName, out DiagnosticDescriptor? descriptor)) { context.ReportDiagnostic(Diagnostic.Create( descriptor, assertOperation.Syntax.GetLocation(), - ClassicModelAssertUsageAnalyzer.GetProperties(methodSymbol))); + DiagnosticsHelper.GetProperties(methodName, assertOperation.Arguments))); } } - - private static ImmutableDictionary GetProperties(IMethodSymbol invocationSymbol) - { - return new Dictionary - { - [AnalyzerPropertyKeys.ModelName] = invocationSymbol.Name, - }.ToImmutableDictionary(); - } } } diff --git a/src/nunit.analyzers/ClassicModelAssertUsage/ClassicModelAssertUsageCodeFix.cs b/src/nunit.analyzers/ClassicModelAssertUsage/ClassicModelAssertUsageCodeFix.cs index 5eb68206..58b12827 100644 --- a/src/nunit.analyzers/ClassicModelAssertUsage/ClassicModelAssertUsageCodeFix.cs +++ b/src/nunit.analyzers/ClassicModelAssertUsage/ClassicModelAssertUsageCodeFix.cs @@ -71,8 +71,10 @@ public sealed override async Task RegisterCodeFixesAsync(CodeFixContext context) newArguments.Add(constraintArgument); // Do the format spec, params to formattable string conversion + bool argsIsArray = !string.IsNullOrEmpty(diagnostic.Properties[AnalyzerPropertyKeys.ArgsIsArray]); + argumentNamesToArguments.TryGetValue(NUnitFrameworkConstants.NameOfMessageParameter, out ArgumentSyntax? messageArgument); - if (CodeFixHelper.GetInterpolatedMessageArgumentOrDefault(messageArgument, args) is ArgumentSyntax interpolatedMessageArgument) + if (CodeFixHelper.GetInterpolatedMessageArgumentOrDefault(messageArgument, args, unconditional: false, argsIsArray) is ArgumentSyntax interpolatedMessageArgument) newArguments.Add(interpolatedMessageArgument); var newArgumentsList = invocationNode.ArgumentList.WithArguments(newArguments); diff --git a/src/nunit.analyzers/CollectionAssertUsage/CollectionAssertUsageAnalyzer.cs b/src/nunit.analyzers/CollectionAssertUsage/CollectionAssertUsageAnalyzer.cs index b4504399..22232e03 100644 --- a/src/nunit.analyzers/CollectionAssertUsage/CollectionAssertUsageAnalyzer.cs +++ b/src/nunit.analyzers/CollectionAssertUsage/CollectionAssertUsageAnalyzer.cs @@ -5,6 +5,7 @@ using Microsoft.CodeAnalysis.Operations; using NUnit.Analyzers.Constants; using NUnit.Analyzers.Extensions; +using NUnit.Analyzers.Helpers; using static NUnit.Analyzers.Constants.NUnitFrameworkConstants; namespace NUnit.Analyzers.CollectionAssertUsage @@ -61,21 +62,18 @@ protected override bool IsAssert(bool hasClassicAssert, IInvocationOperation inv protected override void AnalyzeAssertInvocation(OperationAnalysisContext context, IInvocationOperation assertOperation) { - var methodSymbol = assertOperation.TargetMethod; + string methodName = assertOperation.TargetMethod.Name; - if (OneCollectionParameterAsserts.TryGetValue(methodSymbol.Name, out string? constraint) || - TwoCollectionParameterAsserts.TryGetValue(methodSymbol.Name, out constraint) || - CollectionAndItemParameterAsserts.TryGetValue(methodSymbol.Name, out constraint)) + if (OneCollectionParameterAsserts.TryGetValue(methodName, out string? constraint) || + TwoCollectionParameterAsserts.TryGetValue(methodName, out constraint) || + CollectionAndItemParameterAsserts.TryGetValue(methodName, out constraint)) { context.ReportDiagnostic(Diagnostic.Create( collectionAssertDescriptor, assertOperation.Syntax.GetLocation(), - new Dictionary - { - [AnalyzerPropertyKeys.ModelName] = methodSymbol.Name, - }.ToImmutableDictionary(), + DiagnosticsHelper.GetProperties(methodName, assertOperation.Arguments), constraint, - methodSymbol.Name)); + methodName)); } } } diff --git a/src/nunit.analyzers/Constants/AnalyzerPropertyKeys.cs b/src/nunit.analyzers/Constants/AnalyzerPropertyKeys.cs index 10b5bdc0..d6537d83 100644 --- a/src/nunit.analyzers/Constants/AnalyzerPropertyKeys.cs +++ b/src/nunit.analyzers/Constants/AnalyzerPropertyKeys.cs @@ -3,6 +3,7 @@ namespace NUnit.Analyzers.Constants internal static class AnalyzerPropertyKeys { internal const string ModelName = nameof(AnalyzerPropertyKeys.ModelName); + internal const string ArgsIsArray = nameof(AnalyzerPropertyKeys.ArgsIsArray); internal const string MinimumNumberOfArguments = nameof(AnalyzerPropertyKeys.MinimumNumberOfArguments); } } diff --git a/src/nunit.analyzers/Helpers/CodeFixHelper.cs b/src/nunit.analyzers/Helpers/CodeFixHelper.cs index 61ccd3ba..ad3f71c8 100644 --- a/src/nunit.analyzers/Helpers/CodeFixHelper.cs +++ b/src/nunit.analyzers/Helpers/CodeFixHelper.cs @@ -38,27 +38,58 @@ internal static class CodeFixHelper /// /// 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}"). + /// Unless we cannot, in which case we create '() => string.Format(specification, args)'. /// /// The argument that corresponds to the composite format string. /// The list of arguments that correspond to format items. - public static ArgumentSyntax? GetInterpolatedMessageArgumentOrDefault(ArgumentSyntax? messageArgument, List args) + /// If the message is not conditional on the test outcome. + /// The params args is passed as an array instead of individual parameters. + public static ArgumentSyntax? GetInterpolatedMessageArgumentOrDefault(ArgumentSyntax? messageArgument, List args, bool unconditional, bool argsIsArray) { if (messageArgument is null) return null; + messageArgument = messageArgument.WithNameColon(null); + var formatSpecificationArgument = messageArgument.Expression; if (formatSpecificationArgument.IsKind(SyntaxKind.NullLiteralExpression)) return null; - // We only support converting if the format specification is a constant string. - if (args.Count == 0 || formatSpecificationArgument is not LiteralExpressionSyntax literalExpression) + // No arguments, nothing to convert. Just a message. + if (args.Count == 0) return messageArgument; + IEnumerable? formatArgumentExpressions; + if (args.Count == 1 && args[0].Expression is ImplicitArrayCreationExpressionSyntax implicitArrayCreation) + { + // Even though the argument is an array, it is an inline array creation we can work with. + formatArgumentExpressions = implicitArrayCreation.Initializer.Expressions; + argsIsArray = false; + } + else + { + formatArgumentExpressions = args.Select(x => x.Expression); + } + + // 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 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)))); + + return SyntaxFactory.Argument(unconditional ? stringFormat : SyntaxFactory.ParenthesizedLambdaExpression(stringFormat)); + } + var formatSpecification = literalExpression.Token.ValueText; - var formatArgumentExpressions = - (args.Count == 1 && args[0].Expression is ImplicitArrayCreationExpressionSyntax argsArrayExpression) - ? argsArrayExpression.Initializer.Expressions - : args.Select(x => x.Expression); var interpolatedStringContent = UpdateStringFormatToFormattableString( formatSpecification, @@ -75,42 +106,26 @@ internal static class CodeFixHelper /// /// The arguments passed to the 'Assert' method. /// The argument needed for the actual method, any more are assumed messages. - public static void UpdateStringFormatToFormattableString(List arguments, int minimumNumberOfArguments = 2) + public static void UpdateStringFormatToFormattableString(List arguments, int minimumNumberOfArguments, bool argsIsArray) { - 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; - if (formatSpecificationArgument is not LiteralExpressionSyntax literalExpression) - { - // We only support converting if the format specification is a constant string. - return; - } + ArgumentSyntax? message = GetInterpolatedMessageArgumentOrDefault( + arguments[minimumNumberOfArguments], + arguments.Skip(minimumNumberOfArguments + 1).ToList(), + unconditional: minimumNumberOfArguments == 0, + argsIsArray); - string formatSpecification = literalExpression.Token.ValueText; - - int formatArgumentCount = arguments.Count - firstParamsArgument; - ExpressionSyntax[] formatArguments = new ExpressionSyntax[formatArgumentCount]; - for (int i = 0; i < formatArgumentCount; i++) + var nextArgument = minimumNumberOfArguments; + if (message is not null) { - formatArguments[i] = arguments[firstParamsArgument + i].Expression; + arguments[minimumNumberOfArguments] = message; + nextArgument++; } - IEnumerable 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); - - // Delete params arguments. - var nextArgument = minimumNumberOfArguments + 1; + // Delete remaining arguments. arguments.RemoveRange(nextArgument, arguments.Count - nextArgument); } diff --git a/src/nunit.analyzers/Helpers/DiagnosticsHelper.cs b/src/nunit.analyzers/Helpers/DiagnosticsHelper.cs new file mode 100644 index 00000000..93f105a9 --- /dev/null +++ b/src/nunit.analyzers/Helpers/DiagnosticsHelper.cs @@ -0,0 +1,26 @@ +using System.Collections.Generic; +using System.Collections.Immutable; +using Microsoft.CodeAnalysis.Operations; +using NUnit.Analyzers.Constants; + +namespace NUnit.Analyzers.Helpers +{ + internal static class DiagnosticsHelper + { + public static bool LastArgumentIsNonParamsArray(ImmutableArray arguments) + { + // Find out if the 'params' argument is an existing array and not one created from a params creation. + return arguments[arguments.Length - 1].ArgumentKind != ArgumentKind.ParamArray; + } + + public static ImmutableDictionary GetProperties(string methodName, ImmutableArray arguments) + { + bool argsIsArray = LastArgumentIsNonParamsArray(arguments); + return new Dictionary + { + [AnalyzerPropertyKeys.ModelName] = methodName, + [AnalyzerPropertyKeys.ArgsIsArray] = argsIsArray ? "ArgsIsArray" : string.Empty, + }.ToImmutableDictionary(); + } + } +} diff --git a/src/nunit.analyzers/StringAssertUsage/StringAssertUsageAnalyzer.cs b/src/nunit.analyzers/StringAssertUsage/StringAssertUsageAnalyzer.cs index 9a064588..2e090beb 100644 --- a/src/nunit.analyzers/StringAssertUsage/StringAssertUsageAnalyzer.cs +++ b/src/nunit.analyzers/StringAssertUsage/StringAssertUsageAnalyzer.cs @@ -5,6 +5,7 @@ using Microsoft.CodeAnalysis.Operations; using NUnit.Analyzers.Constants; using NUnit.Analyzers.Extensions; +using NUnit.Analyzers.Helpers; using static NUnit.Analyzers.Constants.NUnitFrameworkConstants; namespace NUnit.Analyzers.StringAssertUsage @@ -45,19 +46,16 @@ protected override bool IsAssert(bool hasClassicAssert, IInvocationOperation inv protected override void AnalyzeAssertInvocation(OperationAnalysisContext context, IInvocationOperation assertOperation) { - var methodSymbol = assertOperation.TargetMethod; + string methodName = assertOperation.TargetMethod.Name; - if (StringAssertToConstraint.TryGetValue(methodSymbol.Name, out string? constraint)) + if (StringAssertToConstraint.TryGetValue(methodName, out string? constraint)) { context.ReportDiagnostic(Diagnostic.Create( stringAssertDescriptor, assertOperation.Syntax.GetLocation(), - new Dictionary - { - [AnalyzerPropertyKeys.ModelName] = methodSymbol.Name, - }.ToImmutableDictionary(), + DiagnosticsHelper.GetProperties(methodName, assertOperation.Arguments), constraint, - methodSymbol.Name)); + methodName)); } } } diff --git a/src/nunit.analyzers/UpdateStringFormatToInterpolatableString/UpdateStringFormatToInterpolatableStringAnalyzer.cs b/src/nunit.analyzers/UpdateStringFormatToInterpolatableString/UpdateStringFormatToInterpolatableStringAnalyzer.cs index 093ce1a1..1bdff41b 100644 --- a/src/nunit.analyzers/UpdateStringFormatToInterpolatableString/UpdateStringFormatToInterpolatableStringAnalyzer.cs +++ b/src/nunit.analyzers/UpdateStringFormatToInterpolatableString/UpdateStringFormatToInterpolatableStringAnalyzer.cs @@ -8,6 +8,7 @@ using Microsoft.CodeAnalysis.Operations; using NUnit.Analyzers.Constants; using NUnit.Analyzers.Extensions; +using NUnit.Analyzers.Helpers; namespace NUnit.Analyzers.UpdateStringFormatToInterpolatableString { @@ -68,7 +69,8 @@ private static void AnalyzeNUnit3AssertInvocation(OperationAnalysisContext conte if (ObsoleteParamsMethods.Contains(methodName)) { - ReportDiagnostic(context, assertOperation, methodName, lastParameterIndex - 1); + bool argsIsArray = DiagnosticsHelper.LastArgumentIsNonParamsArray(assertOperation.Arguments); + ReportDiagnostic(context, assertOperation, methodName, lastParameterIndex - 1, argsIsArray); } } } @@ -121,7 +123,7 @@ private static void AnalyzeNUnit4AssertInvocation(OperationAnalysisContext conte // The argument after the message is explicitly specified // Most likely the user thought it was using a format specification with a parameter. // Or it copied code from some NUnit 3.x source into an NUNit 4 project. - ReportDiagnostic(context, assertOperation, methodName, formatParameterIndex); + ReportDiagnostic(context, assertOperation, methodName, formatParameterIndex, argsIsArray: false); } } @@ -129,7 +131,8 @@ private static void ReportDiagnostic( OperationAnalysisContext context, IInvocationOperation assertOperation, string methodName, - int minimumNumberOfArguments) + int minimumNumberOfArguments, + bool argsIsArray) { context.ReportDiagnostic(Diagnostic.Create( updateStringFormatToInterpolatableString, @@ -138,6 +141,7 @@ private static void ReportDiagnostic( { [AnalyzerPropertyKeys.ModelName] = methodName, [AnalyzerPropertyKeys.MinimumNumberOfArguments] = minimumNumberOfArguments.ToString(CultureInfo.InvariantCulture), + [AnalyzerPropertyKeys.ArgsIsArray] = argsIsArray ? "ArgsIsArray" : string.Empty, }.ToImmutableDictionary())); } } diff --git a/src/nunit.analyzers/UpdateStringFormatToInterpolatableString/UpdateStringFormatToInterpolatableStringCodeFix.cs b/src/nunit.analyzers/UpdateStringFormatToInterpolatableString/UpdateStringFormatToInterpolatableStringCodeFix.cs index 07438730..19cab7da 100644 --- a/src/nunit.analyzers/UpdateStringFormatToInterpolatableString/UpdateStringFormatToInterpolatableStringCodeFix.cs +++ b/src/nunit.analyzers/UpdateStringFormatToInterpolatableString/UpdateStringFormatToInterpolatableStringCodeFix.cs @@ -51,8 +51,9 @@ public override async Task RegisterCodeFixesAsync(CodeFixContext context) List arguments = invocationNode.ArgumentList.Arguments.ToList(); var minimumNumberOfArguments = int.Parse(diagnostic.Properties[AnalyzerPropertyKeys.MinimumNumberOfArguments]!, CultureInfo.InvariantCulture); + bool argsIsArray = !string.IsNullOrEmpty(diagnostic.Properties[AnalyzerPropertyKeys.ArgsIsArray]); - CodeFixHelper.UpdateStringFormatToFormattableString(arguments, minimumNumberOfArguments); + CodeFixHelper.UpdateStringFormatToFormattableString(arguments, minimumNumberOfArguments, argsIsArray); var newArgumentsList = invocationNode.ArgumentList.WithArguments(arguments); var newInvocationNode = invocationNode.WithArgumentList(newArgumentsList);