Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support classic StringAssert in ConstActualValueUsage analyzer and code fix #453

Merged
merged 4 commits into from
May 12, 2022
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -311,5 +311,162 @@ public void Test()

RoslynAssert.Valid(analyzer, testCode);
}

[Test]
public void AnalyzeWhenLiteralArgumentIsProvidedForStringAssertContains()
{
var testCode = TestUtility.WrapMethodInClassNamespaceAndAddUsings(@"
public void Test()
{
string expected = ""exp"";
StringAssert.Contains(expected, ↓""act"");
}");

RoslynAssert.Diagnostics(analyzer, expectedDiagnostic, testCode);
}

[Test]
public void AnalyzeWhenLiteralNamedArgumentIsProvidedForStringAssertContains()
{
var testCode = TestUtility.WrapMethodInClassNamespaceAndAddUsings(@"
public void Test()
{
string expected = ""exp"";
StringAssert.Contains(actual: ↓""act"", expected: expected);
}");

RoslynAssert.Diagnostics(analyzer, expectedDiagnostic, testCode);
}

[Test]
public void AnalyzeWhenLocalConstArgumentIsProvidedForStringAssertContains()
{
var testCode = TestUtility.WrapMethodInClassNamespaceAndAddUsings(@"
public void Test()
{
const string actual = ""act"";
string expected = ""exp"";
StringAssert.Contains(expected, ↓actual);
}");

RoslynAssert.Diagnostics(analyzer, expectedDiagnostic, testCode);
}

[Test]
public void AnalyzeWhenLocalConstNamedArgumentIsProvidedForStringAssertContains()
{
var testCode = TestUtility.WrapMethodInClassNamespaceAndAddUsings(@"
public void Test()
{
const string actual = ""act"";
string expected = ""exp"";
StringAssert.Contains(actual: ↓actual, expected: expected);
}");

RoslynAssert.Diagnostics(analyzer, expectedDiagnostic, testCode);
}

[Test]
public void AnalyzeWhenConstFieldArgumentIsProvidedForStringAssertContains()
{
var testCode = TestUtility.WrapClassInNamespaceAndAddUsing(@"
public class TestFixture
{
private const string actual = ""act"";

public void Test()
{
string expected = ""exp"";
StringAssert.Contains(expected, ↓actual);
}
}");

RoslynAssert.Diagnostics(analyzer, expectedDiagnostic, testCode);
}

[Test]
public void AnalyzeWhenStringEmptyArgumentIsProvidedForStringAssertContains()
{
var testCode = TestUtility.WrapMethodInClassNamespaceAndAddUsings(@"
public void Test()
{
string actual = ""act"";
StringAssert.Contains(actual, string.Empty);
mikkelbu marked this conversation as resolved.
Show resolved Hide resolved
}");

RoslynAssert.Diagnostics(analyzer, expectedDiagnostic, testCode);
}

[Test]
public void ValidWhenNonConstValueIsProvidedAsActualArgumentForStringAssertContains()
{
var testCode = TestUtility.WrapClassInNamespaceAndAddUsing(@"
public class TestFixture
{
private const string expected = ""exp"";

public void Test()
{
string actual = ""act"";
StringAssert.Contains(expected, actual);
}
}");

RoslynAssert.Valid(analyzer, testCode);
}

[Test]
public void ValidWhenConstValueIsProvidedAsActualAndExpectedArgumentForStringAssertContains()
{
var testCode = TestUtility.WrapClassInNamespaceAndAddUsing(@"
public class TestFixture
{
private const string expected = ""exp"";

public void Test()
{
const string actual = ""act"";
StringAssert.Contains(expected, actual);
}
}");

RoslynAssert.Valid(analyzer, testCode);
}

[Test]
public void ValidWhenNonConstValueIsProvidedAsActualNamedArgumentForStringAssertContains()
{
var testCode = TestUtility.WrapClassInNamespaceAndAddUsing(@"
public class TestFixture
{
private const string expected = ""exp"";

public void Test()
{
string actual = ""act"";
StringAssert.Contains(actual: actual, expected: expected);
}
}");

RoslynAssert.Valid(analyzer, testCode);
}

[Test]
public void ValidWhenConstValueIsProvidedAsActualAndExpectedNamedArgumentForStringAssertContains()
{
var testCode = TestUtility.WrapClassInNamespaceAndAddUsing(@"
public class TestFixture
{
private const string expected = ""exp"";

public void Test()
{
const string actual = ""act"";
StringAssert.Contains(actual: actual, expected: expected);
}
}");

RoslynAssert.Valid(analyzer, testCode);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -112,5 +112,56 @@ public void Test()

RoslynAssert.NoFix(analyzer, fix, expectedDiagnostic, code);
}

[TestCase(nameof(StringAssert.AreEqualIgnoringCase))]
[TestCase(nameof(StringAssert.AreNotEqualIgnoringCase))]
[TestCase(nameof(StringAssert.Contains))]
[TestCase(nameof(StringAssert.EndsWith))]
[TestCase(nameof(StringAssert.IsMatch))]
[TestCase(nameof(StringAssert.StartsWith))]
[TestCase(nameof(StringAssert.DoesNotContain))]
[TestCase(nameof(StringAssert.DoesNotMatch))]
[TestCase(nameof(StringAssert.DoesNotEndWith))]
[TestCase(nameof(StringAssert.DoesNotStartWith))]
public void LiteralArgumentIsProvidedForClassicStringAssertCodeFix(string classicAssertMethod)
{
var code = TestUtility.WrapMethodInClassNamespaceAndAddUsings($@"
public void Test()
{{
string actual = ""act"";
StringAssert.{classicAssertMethod}(actual, ↓""exp"");
}}");

var fixedCode = TestUtility.WrapMethodInClassNamespaceAndAddUsings($@"
public void Test()
{{
string actual = ""act"";
StringAssert.{classicAssertMethod}(""exp"", actual);
}}");

RoslynAssert.CodeFix(analyzer, fix, expectedDiagnostic, code, fixedCode,
fixTitle: ConstActualValueUsageCodeFix.SwapArgumentsDescription);
}

[Test]
public void LiteralNamedArgumentIsProvidedForStringAssertContainsCodeFix()
{
var code = TestUtility.WrapMethodInClassNamespaceAndAddUsings(@"
public void Test()
{
string actual = ""act"";
StringAssert.Contains(actual: ↓""exp"", expected: actual);
}");

var fixedCode = TestUtility.WrapMethodInClassNamespaceAndAddUsings(@"
public void Test()
{
string actual = ""act"";
StringAssert.Contains(actual: actual, expected: ""exp"");
}");

RoslynAssert.CodeFix(analyzer, fix, expectedDiagnostic, code, fixedCode,
fixTitle: ConstActualValueUsageCodeFix.SwapArgumentsDescription);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,18 @@ public sealed class NUnitFrameworkConstantsTests
(nameof(NUnitFrameworkConstants.NameOfAssertThrows), nameof(Assert.Throws)),
(nameof(NUnitFrameworkConstants.NameOfAssertThrowsAsync), nameof(Assert.ThrowsAsync)),

(nameof(NUnitFrameworkConstants.NameOfStringAssert), nameof(StringAssert)),
(nameof(NUnitFrameworkConstants.NameOfStringAssertAreEqualIgnoringCase), nameof(StringAssert.AreEqualIgnoringCase)),
(nameof(NUnitFrameworkConstants.NameOfStringAssertAreNotEqualIgnoringCase), nameof(StringAssert.AreNotEqualIgnoringCase)),
(nameof(NUnitFrameworkConstants.NameOfStringAssertContains), nameof(StringAssert.Contains)),
(nameof(NUnitFrameworkConstants.NameOfStringAssertDoesNotContain), nameof(StringAssert.DoesNotContain)),
(nameof(NUnitFrameworkConstants.NameOfStringAssertDoesNotEndWith), nameof(StringAssert.DoesNotEndWith)),
(nameof(NUnitFrameworkConstants.NameOfStringAssertDoesNotMatch), nameof(StringAssert.DoesNotMatch)),
(nameof(NUnitFrameworkConstants.NameOfStringAssertDoesNotStartWith), nameof(StringAssert.DoesNotStartWith)),
(nameof(NUnitFrameworkConstants.NameOfStringAssertEndsWith), nameof(StringAssert.EndsWith)),
(nameof(NUnitFrameworkConstants.NameOfStringAssertIsMatch), nameof(StringAssert.IsMatch)),
(nameof(NUnitFrameworkConstants.NameOfStringAssertStartsWith), nameof(StringAssert.StartsWith)),

(nameof(NUnitFrameworkConstants.NameOfConstraint), nameof(Constraint)),

(nameof(NUnitFrameworkConstants.NameOfTestCaseAttribute), nameof(TestCaseAttribute)),
Expand Down
18 changes: 18 additions & 0 deletions src/nunit.analyzers.tests/Extensions/ITypeSymbolExtensionsTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -200,6 +200,24 @@ public sealed class IsAssertWhenSymbolIsAssertType
Assert.That(typeSymbol.IsAssert(), Is.True);
}

[TestCase("Assert")]
[TestCase("StringAssert")]
public async Task IsAnyAssertWhenSymbolIsAnyAssertType(string assertType)
{
var testCode = $@"
using NUnit.Framework;

namespace NUnit.Analyzers.Tests.Targets.Extensions
{{
public sealed class IsAnyAssertWhenSymbolIsAnyAssertType
{{
public {assertType} x;
}}
}}";
var typeSymbol = await GetTypeSymbolFromFieldAsync(testCode, "IsAnyAssertWhenSymbolIsAnyAssertType").ConfigureAwait(false);
Assert.That(typeSymbol.IsAnyAssert(), Is.True);
}

private static async Task<ImmutableArray<ITypeSymbol>> GetTypeSymbolAsync(string code, string[] typeNames)
{
var rootAndModel = await TestHelpers.GetRootAndModel(code).ConfigureAwait(false);
Expand Down
11 changes: 2 additions & 9 deletions src/nunit.analyzers/BaseAssertionAnalyzer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -16,14 +16,7 @@ public override void Initialize(AnalysisContext context)

protected abstract void AnalyzeAssertInvocation(OperationAnalysisContext context, IInvocationOperation assertOperation);

protected static bool IsAssert(IOperation operation)
{
return operation is IExpressionStatementOperation expressionOperation &&
expressionOperation.Operation is IInvocationOperation invocationOperation &&
IsAssert(invocationOperation);
}

protected static bool IsAssert(IInvocationOperation invocationOperation)
protected virtual bool IsAssert(IInvocationOperation invocationOperation)
{
return invocationOperation.TargetMethod.ContainingType.IsAssert();
}
Expand All @@ -33,7 +26,7 @@ private void AnalyzeInvocation(OperationAnalysisContext context)
if (context.Operation is not IInvocationOperation invocationOperation)
return;

if (!IsAssert(invocationOperation))
if (!this.IsAssert(invocationOperation))
return;

context.CancellationToken.ThrowIfCancellationRequested();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,11 @@ public class ConstActualValueUsageAnalyzer : BaseAssertionAnalyzer

public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics => ImmutableArray.Create(descriptor);

protected override bool IsAssert(IInvocationOperation invocationOperation)
{
return invocationOperation.TargetMethod.ContainingType.IsAnyAssert();
}

protected override void AnalyzeAssertInvocation(OperationAnalysisContext context, IInvocationOperation assertOperation)
{
static bool IsStringEmpty(IOperation operation)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,21 @@ public class ConstActualValueUsageCodeFix : CodeFixProvider
NUnitFrameworkConstants.NameOfAssertAreEqual,
NUnitFrameworkConstants.NameOfAssertAreNotEqual,
NUnitFrameworkConstants.NameOfAssertAreSame,
NUnitFrameworkConstants.NameOfAssertAreNotSame
NUnitFrameworkConstants.NameOfAssertAreNotSame,
};

private static readonly string[] SupportedStringAsserts = new[]
{
NUnitFrameworkConstants.NameOfStringAssertAreEqualIgnoringCase,
NUnitFrameworkConstants.NameOfStringAssertAreNotEqualIgnoringCase,
NUnitFrameworkConstants.NameOfStringAssertContains,
NUnitFrameworkConstants.NameOfStringAssertDoesNotContain,
NUnitFrameworkConstants.NameOfStringAssertDoesNotEndWith,
NUnitFrameworkConstants.NameOfStringAssertDoesNotMatch,
NUnitFrameworkConstants.NameOfStringAssertDoesNotStartWith,
NUnitFrameworkConstants.NameOfStringAssertEndsWith,
NUnitFrameworkConstants.NameOfStringAssertIsMatch,
NUnitFrameworkConstants.NameOfStringAssertStartsWith,
};

private static readonly string[] SupportedIsConstraints = new[]
Expand Down Expand Up @@ -88,11 +102,12 @@ private static bool TryFindArguments(SemanticModel semanticModel, InvocationExpr

var methodSymbol = semanticModel.GetSymbolInfo(invocationSyntax).Symbol as IMethodSymbol;

if (methodSymbol is null || !methodSymbol.ContainingType.IsAssert())
if (methodSymbol is null || !methodSymbol.ContainingType.IsAnyAssert())
return false;

// option 1: Classic assert (e.g. Assert.AreEqual(expected, actual) )
if (SupportedClassicAsserts.Contains(methodSymbol.Name) && methodSymbol.Parameters.Length >= 2)
if ((IsSupportedAssert(methodSymbol) || IsSupportedStringAssert(methodSymbol))
&& methodSymbol.Parameters.Length >= 2)
{
expectedArgument = invocationSyntax.ArgumentList.Arguments[0].Expression;
actualArgument = invocationSyntax.ArgumentList.Arguments[1].Expression;
Expand Down Expand Up @@ -135,5 +150,15 @@ private static bool TryFindArguments(SemanticModel semanticModel, InvocationExpr

return false;
}

private static bool IsSupportedAssert(IMethodSymbol methodSymbol)
{
return methodSymbol.ContainingType.Name == NUnitFrameworkConstants.NameOfAssert && SupportedClassicAsserts.Contains(methodSymbol.Name);
}

private static bool IsSupportedStringAssert(IMethodSymbol methodSymbol)
{
return methodSymbol.ContainingType.Name == NUnitFrameworkConstants.NameOfStringAssert && SupportedStringAsserts.Contains(methodSymbol.Name);
}
}
}
17 changes: 17 additions & 0 deletions src/nunit.analyzers/Constants/NUnitFrameworkConstants.cs
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,18 @@ public static class NUnitFrameworkConstants
public const string NameOfAssertThrows = "Throws";
public const string NameOfAssertThrowsAsync = "ThrowsAsync";

public const string NameOfStringAssert = "StringAssert";
public const string NameOfStringAssertAreEqualIgnoringCase = "AreEqualIgnoringCase";
public const string NameOfStringAssertAreNotEqualIgnoringCase = "AreNotEqualIgnoringCase";
public const string NameOfStringAssertContains = "Contains";
public const string NameOfStringAssertDoesNotContain = "DoesNotContain";
public const string NameOfStringAssertDoesNotEndWith = "DoesNotEndWith";
public const string NameOfStringAssertDoesNotMatch = "DoesNotMatch";
public const string NameOfStringAssertDoesNotStartWith = "DoesNotStartWith";
public const string NameOfStringAssertEndsWith = "EndsWith";
public const string NameOfStringAssertIsMatch = "IsMatch";
public const string NameOfStringAssertStartsWith = "StartsWith";

public const string FullNameOfTypeIs = "NUnit.Framework.Is";
public const string FullNameOfTypeTestCaseAttribute = "NUnit.Framework.TestCaseAttribute";
public const string FullNameOfTypeTestCaseSourceAttribute = "NUnit.Framework.TestCaseSourceAttribute";
Expand Down Expand Up @@ -136,5 +148,10 @@ public static class NUnitFrameworkConstants
public const string NameOfUsing = "Using";

public const string NUnitFrameworkAssemblyName = "nunit.framework";

public static readonly string[] AllAsserts =
{
NameOfAssert, NameOfStringAssert
};
}
}
7 changes: 7 additions & 0 deletions src/nunit.analyzers/Extensions/ITypeSymbolExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,13 @@ internal static bool IsAssert(this ITypeSymbol? @this)
@this.Name == NUnitFrameworkConstants.NameOfAssert;
}

internal static bool IsAnyAssert(this ITypeSymbol? @this)
{
return @this is not null &&
@this.ContainingAssembly.Name == NUnitFrameworkConstants.NUnitFrameworkAssemblyName &&
NUnitFrameworkConstants.AllAsserts.Contains(@this.Name);
}

internal static bool IsConstraint(this ITypeSymbol? @this)
{
return @this is not null && @this.GetAllBaseTypes()
Expand Down
Loading