Skip to content

Commit

Permalink
New rule S2970: Assertions should be complete (#6757)
Browse files Browse the repository at this point in the history
  • Loading branch information
martin-strecker-sonarsource committed Feb 27, 2023
1 parent 55e769c commit a0336ce
Show file tree
Hide file tree
Showing 18 changed files with 840 additions and 1 deletion.
29 changes: 29 additions & 0 deletions analyzers/rspec/cs/S2970_c#.html
@@ -0,0 +1,29 @@
<p>It is very easy to write incomplete assertions when using some test frameworks. This rule enforces complete assertions in the following cases:</p>
<ul>
<li> Fluent Assertions: <a href="https://fluentassertions.com/introduction"><code>Should()</code></a> is not followed by an assertion invocation.
</li>
<li> NFluent: <a href="https://www.n-fluent.net"><code>Check.That()</code></a> is not followed by an assertion invocation. </li>
<li> NSubstitute: <a href="https://nsubstitute.github.io/help/received-calls"><code>Received()</code></a> is not followed by an invocation. </li>
</ul>
<p>In such cases, what is intended to be a test doesn’t actually verify anything.</p>
<h2>Noncompliant Code Example</h2>
<pre>
string actual = "Hello World!";
// Fluent Assertions
actual.Should(); // Noncompliant
// NFluent
Check.That(actual); // Noncompliant
// NSubstitute
command.Received(); // Noncompliant
</pre>
<h2>Compliant Solution</h2>
<pre>
string actual = "Hello World!";
// Fluent Assertions
actual.Should().Contain("Hello");
// NFluent
Check.That(actual).Contains("Hello");
// NSubstitute
command.Received().Execute();
</pre>

17 changes: 17 additions & 0 deletions analyzers/rspec/cs/S2970_c#.json
@@ -0,0 +1,17 @@
{
"title": "Assertions should be complete",
"type": "CODE_SMELL",
"status": "ready",
"remediation": {
"func": "Constant\/Issue",
"constantCost": "5min"
},
"tags": [
"tests"
],
"defaultSeverity": "Blocker",
"ruleSpecification": "RSPEC-2970",
"sqKey": "S2970",
"scope": "Tests",
"quickfix": "unknown"
}
1 change: 1 addition & 0 deletions analyzers/rspec/cs/Sonar_way_profile.json
Expand Up @@ -120,6 +120,7 @@
"S2933",
"S2934",
"S2953",
"S2970",
"S2971",
"S2995",
"S2996",
Expand Down
Expand Up @@ -280,6 +280,11 @@ node switch
public static bool NameIs(this SyntaxNode node, string name) =>
node.GetName().Equals(name, StringComparison.InvariantCulture);

public static bool NameIs(this SyntaxNode node, string name, params string[] orNames) =>
node.GetName() is { } nodeName
&& (nodeName.Equals(name, StringComparison.InvariantCulture)
|| orNames.Any(x => nodeName.Equals(x, StringComparison.InvariantCulture)));

public static bool HasConstantValue(this ExpressionSyntax expression, SemanticModel semanticModel) =>
expression.RemoveParentheses().IsAnyKind(LiteralSyntaxKinds) || expression.FindConstantValue(semanticModel) != null;

Expand Down
131 changes: 131 additions & 0 deletions analyzers/src/SonarAnalyzer.CSharp/Rules/AssertionsShouldBeComplete.cs
@@ -0,0 +1,131 @@
/*
* SonarAnalyzer for .NET
* Copyright (C) 2015-2023 SonarSource SA
* mailto: contact AT sonarsource DOT com
*
* This program is free software; you can redistribute it and/or
* modify it under the terms of the GNU Lesser General Public
* License as published by the Free Software Foundation; either
* version 3 of the License, or (at your option) any later version.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
* Lesser General Public License for more details.
*
* You should have received a copy of the GNU Lesser General Public License
* along with this program; if not, write to the Free Software Foundation,
* Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
*/

namespace SonarAnalyzer.Rules.CSharp;

[DiagnosticAnalyzer(LanguageNames.CSharp)]
public sealed class AssertionsShouldBeComplete : SonarDiagnosticAnalyzer
{
private const string DiagnosticId = "S2970";
private const string MessageFormat = "Complete the assertion";

private static readonly DiagnosticDescriptor Rule = DescriptorFactory.Create(DiagnosticId, MessageFormat);

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

protected override void Initialize(SonarAnalysisContext context) =>
context.RegisterCompilationStartAction(start =>
{
if (start.Compilation.References(KnownAssembly.FluentAssertions))
{
start.RegisterNodeAction(c =>
CheckInvocation(c, invocation =>
invocation.NameIs("Should")
&& c.SemanticModel.GetSymbolInfo(invocation).AllSymbols().Any(x =>
x is IMethodSymbol
{
IsExtensionMethod: true,
ReturnsVoid: false,
ContainingType: { } container,
ReturnType: { } returnType,
}
&& (container.Is(KnownType.FluentAssertions_AssertionExtensions)
// ⬆️ Built in assertions. ⬇️ Custom assertions (the majority at least).
|| returnType.DerivesFrom(KnownType.FluentAssertions_Primitives_ReferenceTypeAssertions)))),
SyntaxKind.InvocationExpression);
}
if (start.Compilation.References(KnownAssembly.NFluent))
{
start.RegisterNodeAction(c =>
CheckInvocation(c, invocation =>
invocation.NameIs("That", "ThatEnum", "ThatCode", "ThatAsyncCode", "ThatDynamic")
&& c.SemanticModel.GetSymbolInfo(invocation) is
{
Symbol: IMethodSymbol
{
IsStatic: true,
ReturnsVoid: false,
ContainingType: { IsStatic: true } container
}
}
&& container.Is(KnownType.NFluent_Check)),
SyntaxKind.InvocationExpression);
}
if (start.Compilation.References(KnownAssembly.NSubstitute))
{
start.RegisterNodeAction(c =>
CheckInvocation(c, invocation =>
invocation.NameIs("Received", "DidNotReceive", "ReceivedWithAnyArgs", "DidNotReceiveWithAnyArgs", "ReceivedCalls")
&& c.SemanticModel.GetSymbolInfo(invocation) is
{
Symbol: IMethodSymbol
{
IsExtensionMethod: true,
ReturnsVoid: false,
ContainingType: { } container,
}
}
&& container.Is(KnownType.NSubstitute_SubstituteExtensions)),
SyntaxKind.InvocationExpression);
}
});

private static void CheckInvocation(SonarSyntaxNodeReportingContext c, Func<InvocationExpressionSyntax, bool> isAssertionMethod)
{
if (c.Node is InvocationExpressionSyntax invocation
&& isAssertionMethod(invocation)
&& !HasContinuation(invocation))
{
c.ReportIssue(Diagnostic.Create(Rule, invocation.GetIdentifier()?.GetLocation()));
}
}

private static bool HasContinuation(InvocationExpressionSyntax invocation)
{
var closeParen = invocation.ArgumentList.CloseParenToken;
if (!closeParen.IsKind(SyntaxKind.CloseParenToken) || closeParen.IsMissing || !invocation.GetLastToken().Equals(closeParen))
{
// Any invocation should end with ")". We are in unknown territory here.
return true;
}
if (closeParen.GetNextToken() is var nextToken
&& !nextToken.IsKind(SyntaxKind.SemicolonToken))
{
// There is something right to the invocation that is not a semicolon.
return true;
}
// We are in some kind of statement context "??? Should();"
// The result might be stored in a variable or returned from the method/property
return nextToken.Parent switch
{
MethodDeclarationSyntax { ReturnType: { } returnType } => !IsVoid(returnType),
{ } parent when LocalFunctionStatementSyntaxWrapper.IsInstance(parent) => !IsVoid(((LocalFunctionStatementSyntaxWrapper)parent).ReturnType),
PropertyDeclarationSyntax => true,
AccessorDeclarationSyntax { Keyword.RawKind: (int)SyntaxKind.GetKeyword } => true,
ReturnStatementSyntax => true,
LocalDeclarationStatementSyntax => true,
ExpressionStatementSyntax { Expression: AssignmentExpressionSyntax } => true,
_ => false,
};
}

private static bool IsVoid(TypeSyntax type) =>
type is PredefinedTypeSyntax { Keyword.RawKind: (int)SyntaxKind.VoidKeyword };
}
@@ -0,0 +1,33 @@
/*
* SonarAnalyzer for .NET
* Copyright (C) 2015-2023 SonarSource SA
* mailto: contact AT sonarsource DOT com
*
* This program is free software; you can redistribute it and/or
* modify it under the terms of the GNU Lesser General Public
* License as published by the Free Software Foundation; either
* version 3 of the License, or (at your option) any later version.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
* Lesser General Public License for more details.
*
* You should have received a copy of the GNU Lesser General Public License
* along with this program; if not, write to the Free Software Foundation,
* Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
*/

namespace SonarAnalyzer.Extensions
{
public static class SymbolInfoExtensions
{
/// <summary>
/// Returns the <see cref="SymbolInfo.Symbol"/> or if no symbol could be found the <see cref="SymbolInfo.CandidateSymbols"/>.
/// </summary>
public static IEnumerable<ISymbol> AllSymbols(this SymbolInfo symbolInfo) =>
symbolInfo.Symbol == null
? symbolInfo.CandidateSymbols
: new[] { symbolInfo.Symbol };
}
}
13 changes: 13 additions & 0 deletions analyzers/src/SonarAnalyzer.Common/Helpers/KnownAssembly.cs
Expand Up @@ -30,6 +30,19 @@ public sealed partial class KnownAssembly
And(NameIs("xunit.assert").Or(NameIs("xunit").And(VersionLowerThen("2.0"))),
PublicKeyTokenIs("8d05b1bb7a6fdb6c")));

/// <summary>
/// Any MSTest framework either referenced via
/// <see href="https://www.nuget.org/packages/MicrosoftVisualStudioQualityToolsUnitTestFramework">nuget.org/MicrosoftVisualStudioQualityToolsUnitTestFramework</see> (MSTest V1)
/// or <see href="https://www.nuget.org/packages/MSTest.TestFramework">nuget.org/MSTest.TestFramework</see> (MSTest V2).
/// </summary>
public static KnownAssembly MSTest { get; } =
new(And(NameIs("Microsoft.VisualStudio.QualityTools.UnitTestFramework").Or(NameIs("Microsoft.VisualStudio.TestPlatform.TestFramework")),
PublicKeyTokenIs("b03f5f7f11d50a3a")));

public static KnownAssembly FluentAssertions { get; } = new(NameIs("FluentAssertions").And(PublicKeyTokenIs("33f2691a05b67b6a")));
public static KnownAssembly NFluent { get; } = new(NameIs("NFluent").And(OptionalPublicKeyTokenIs("18828b37b84b1437")));
public static KnownAssembly NSubstitute { get; } = new(NameIs("NSubstitute").And(PublicKeyTokenIs("92dd2e9066daa5ca")));

internal KnownAssembly(Func<AssemblyIdentity, bool> predicate, params Func<AssemblyIdentity, bool>[] or)
: this(predicate is null || or.Any(x => x is null)
? throw new ArgumentNullException(nameof(predicate), "All predicates must be non-null.")
Expand Down
2 changes: 2 additions & 0 deletions analyzers/src/SonarAnalyzer.Common/Helpers/KnownType.cs
Expand Up @@ -37,7 +37,9 @@ public sealed partial class KnownType
public static readonly KnownType Azure_ResourceManager_ArmClient = new("Azure.ResourceManager.ArmClient");
public static readonly KnownType Dapper_SqlMapper = new("Dapper.SqlMapper");
public static readonly KnownType Dapper_CommandDefinition = new("Dapper.CommandDefinition");
public static readonly KnownType FluentAssertions_AssertionExtensions = new("FluentAssertions.AssertionExtensions");
public static readonly KnownType FluentAssertions_Execution_AssertionScope = new("FluentAssertions.Execution.AssertionScope");
public static readonly KnownType FluentAssertions_Primitives_ReferenceTypeAssertions = new("FluentAssertions.Primitives.ReferenceTypeAssertions", "TSubject", "TAssertions");
public static readonly KnownType JWT_Builder_JwtBuilder = new("JWT.Builder.JwtBuilder");
public static readonly KnownType JWT_IJwtDecoder = new("JWT.IJwtDecoder");
public static readonly KnownType JWT_JwtDecoderExtensions = new("JWT.JwtDecoderExtensions");
Expand Down
Expand Up @@ -275,6 +275,84 @@ public void XUnitAssert_NoReference()
compilation.References(XUnit_Assert).Should().BeFalse();
}

[TestMethod]
public void MSTest_V1()
{
var compilation = TestHelper.CompileCS("// Empty file", NuGetMetadataReference.MSTestTestFrameworkV1.ToArray()).Model.Compilation;
compilation.References(MSTest).Should().BeTrue();
}

[TestMethod]
public void MSTest_V2()
{
var compilation = TestHelper.CompileCS("// Empty file", NuGetMetadataReference.MSTestTestFramework("3.0.2").ToArray()).Model.Compilation;
compilation.References(MSTest).Should().BeTrue();
}

[TestMethod]
public void MSTest_MicrosoftVisualStudioQualityToolsUnitTestFramework()
{
var compilation = TestHelper.CompileCS("// Empty file", NuGetMetadataReference.MicrosoftVisualStudioQualityToolsUnitTestFramework.ToArray()).Model.Compilation;
compilation.References(MSTest).Should().BeTrue();
}

[TestMethod]
public void MSTest_NoReference()
{
var compilation = TestHelper.CompileCS("// Empty file").Model.Compilation;
compilation.References(MSTest).Should().BeFalse();
}

[TestMethod]
public void FluentAssertions_6_10()
{
var compilation = TestHelper.CompileCS("// Empty file", NuGetMetadataReference.FluentAssertions("6.10.0").ToArray()).Model.Compilation;
compilation.References(KnownAssembly.FluentAssertions).Should().BeTrue();
}

[TestMethod]
public void FluentAssertions_NoReference()
{
var compilation = TestHelper.CompileCS("// Empty file").Model.Compilation;
compilation.References(KnownAssembly.FluentAssertions).Should().BeFalse();
}

[TestMethod]
public void NFluent_2_8()
{
var compilation = TestHelper.CompileCS("// Empty file", NuGetMetadataReference.NFluent("2.8.0").ToArray()).Model.Compilation;
compilation.References(NFluent).Should().BeTrue();
}

[TestMethod]
public void NFluent_1_0()
{
// 1.0.0 has no publicKeyToken
var compilation = TestHelper.CompileCS("// Empty file", NuGetMetadataReference.NFluent("1.0.0").ToArray()).Model.Compilation;
compilation.References(NFluent).Should().BeTrue();
}

[TestMethod]
public void NFluent_NoReference()
{
var compilation = TestHelper.CompileCS("// Empty file").Model.Compilation;
compilation.References(NFluent).Should().BeFalse();
}

[TestMethod]
public void NSubstitute_5_0()
{
var compilation = TestHelper.CompileCS("// Empty file", NuGetMetadataReference.NSubstitute("5.0.0").ToArray()).Model.Compilation;
compilation.References(NSubstitute).Should().BeTrue();
}

[TestMethod]
public void NSubstitute_NoReference()
{
var compilation = TestHelper.CompileCS("// Empty file").Model.Compilation;
compilation.References(NSubstitute).Should().BeFalse();
}

private static Mock<Compilation> CompilationWithReferenceTo(AssemblyIdentity identity)
{
var compilation = new Mock<Compilation>("compilationName", ImmutableArray<MetadataReference>.Empty, new Dictionary<string, string>(), false, null, null);
Expand Down
19 changes: 19 additions & 0 deletions analyzers/tests/SonarAnalyzer.UnitTest/Helpers/SyntaxHelperTest.cs
Expand Up @@ -137,6 +137,25 @@ public void NameIs_CS()
toString.NameIs(null).Should().BeFalse();
}

[DataTestMethod]
[DataRow(true, "Test")]
[DataRow(true, "Test", "Test")]
[DataRow(true, "Other", "Test")]
[DataRow(false)]
[DataRow(false, "TEST")]
public void NameIsOrNames_CS(bool expected, params string[] orNames)
{
var identifier = Microsoft.CodeAnalysis.CSharp.SyntaxFactory.IdentifierName("Test");
identifier.NameIs("other", orNames).Should().Be(expected);
}

[TestMethod]
public void NameIsOrNamesNodeWithoutName_CS()
{
var returnStatement = Microsoft.CodeAnalysis.CSharp.SyntaxFactory.ReturnStatement();
returnStatement.NameIs("A", "B", "C").Should().BeFalse();
}

[TestMethod]
public void NameIs_VB()
{
Expand Down
Expand Up @@ -2894,7 +2894,7 @@ internal static class RuleTypeMappingCS
// ["S2967"],
// ["S2968"],
// ["S2969"],
// ["S2970"],
["S2970"] = "CODE_SMELL",
["S2971"] = "CODE_SMELL",
// ["S2972"],
// ["S2973"],
Expand Down

0 comments on commit a0336ce

Please sign in to comment.