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

New rule S2970: Assertions should be complete #6757

Merged
merged 35 commits into from Feb 27, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
35 commits
Select commit Hold shift + click to select a range
3cbce45
Add KnownAssembly MSTest and MSTestAssert
martin-strecker-sonarsource Feb 14, 2023
cd724cb
Add remark for compatibility
martin-strecker-sonarsource Feb 14, 2023
cfa25e7
Scaffold rule
martin-strecker-sonarsource Feb 14, 2023
4093321
WIP: Register MSTest
martin-strecker-sonarsource Feb 15, 2023
5897871
Remove Assert_That from KnownAssemblies as it does not bring much to …
martin-strecker-sonarsource Feb 16, 2023
16a04a3
Add support for NFluent, NSubsutiture and FluentAssertion
martin-strecker-sonarsource Feb 16, 2023
3067aea
wip: add checks
martin-strecker-sonarsource Feb 16, 2023
7450e81
WIP: Add implementation and test for FluentAssertion
martin-strecker-sonarsource Feb 20, 2023
a6c9549
Add support for custom assertions
martin-strecker-sonarsource Feb 20, 2023
4991d86
Move nullable tests in own test file and fix Netframework dependencies
martin-strecker-sonarsource Feb 20, 2023
e6a630c
Add type argument test
martin-strecker-sonarsource Feb 20, 2023
286b13b
Demonstrate better overload resolution inC#8 vs C#7
martin-strecker-sonarsource Feb 20, 2023
d7be4d9
Fix wrong annotated compiler error
martin-strecker-sonarsource Feb 20, 2023
c4340a8
Add KnownTypes after rebase
martin-strecker-sonarsource Feb 20, 2023
8531f37
Add comment
martin-strecker-sonarsource Feb 20, 2023
0f95790
NFluent: Add tests
martin-strecker-sonarsource Feb 21, 2023
d275172
Add tests for NFluent
martin-strecker-sonarsource Feb 21, 2023
53b00f6
Add test for other compliant Check class
martin-strecker-sonarsource Feb 21, 2023
a5bd203
formatting
martin-strecker-sonarsource Feb 21, 2023
1baf1ff
formatting
martin-strecker-sonarsource Feb 21, 2023
c399c2f
Add tests for NSubstitute
martin-strecker-sonarsource Feb 22, 2023
76fb2ad
Refactorings
martin-strecker-sonarsource Feb 22, 2023
69d142c
Add tests for lambdas and locla functions
martin-strecker-sonarsource Feb 22, 2023
d960499
Add more fine grained tests for valid continuations and fix some bugs.
martin-strecker-sonarsource Feb 22, 2023
d67b3ab
Fix code smells
martin-strecker-sonarsource Feb 22, 2023
3b99c79
Refactor HasContinuation
martin-strecker-sonarsource Feb 22, 2023
5028c04
Move IsVoid in the switch arm expression
martin-strecker-sonarsource Feb 22, 2023
5f1eeae
Add test for missing close paran
martin-strecker-sonarsource Feb 22, 2023
490e1fe
Add tests for IsName overload
martin-strecker-sonarsource Feb 22, 2023
7fcea44
Add tests for all framework usages in one file
martin-strecker-sonarsource Feb 22, 2023
dde49fb
Fix test method name
martin-strecker-sonarsource Feb 23, 2023
d087c8c
Remove CSharp using from test file
martin-strecker-sonarsource Feb 23, 2023
923500c
Formatting
martin-strecker-sonarsource Feb 23, 2023
264f688
Update analyzers/tests/SonarAnalyzer.UnitTest/TestCases/AssertionsSho…
martin-strecker-sonarsource Feb 23, 2023
b252275
Update RSpec
martin-strecker-sonarsource Feb 27, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
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 @@ -119,6 +119,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)));
Tim-Pohlmann marked this conversation as resolved.
Show resolved Hide resolved

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)))),
Tim-Pohlmann marked this conversation as resolved.
Show resolved Hide resolved
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