From 3cbce45767551034c40391c17a7be407a9d8fde5 Mon Sep 17 00:00:00 2001 From: Martin Strecker Date: Tue, 14 Feb 2023 16:34:24 +0100 Subject: [PATCH 01/35] Add KnownAssembly MSTest and MSTestAssert --- .../Helpers/KnownAssembly.cs | 14 +++++ .../Helpers/KnownAssemblyTest.cs | 58 +++++++++++++++++++ 2 files changed, 72 insertions(+) diff --git a/analyzers/src/SonarAnalyzer.Common/Helpers/KnownAssembly.cs b/analyzers/src/SonarAnalyzer.Common/Helpers/KnownAssembly.cs index d7f085bcefd..d80d66b3d0c 100644 --- a/analyzers/src/SonarAnalyzer.Common/Helpers/KnownAssembly.cs +++ b/analyzers/src/SonarAnalyzer.Common/Helpers/KnownAssembly.cs @@ -30,6 +30,20 @@ public sealed partial class KnownAssembly And(NameIs("xunit.assert").Or(NameIs("xunit").And(VersionLowerThen("2.0"))), PublicKeyTokenIs("8d05b1bb7a6fdb6c"))); + /// + /// Any MSTest framework either referenced via nuget.org/MicrosoftVisualStudioQualityToolsUnitTestFramework (MSTest V1) + /// or nuget.org/MSTest.TestFramework (MSTest V2). + /// + public static KnownAssembly MSTest { get; } = new( + And(NameIs("Microsoft.VisualStudio.QualityTools.UnitTestFramework").Or(NameIs("Microsoft.VisualStudio.TestPlatform.TestFramework")), + PublicKeyTokenIs("b03f5f7f11d50a3a"))); + + /// + /// Assemblies that contain Assert.That. + /// + public static KnownAssembly MSTest_Assert_That { get; } = new( + NameIs("Microsoft.VisualStudio.TestPlatform.TestFramework").And(PublicKeyTokenIs("b03f5f7f11d50a3a"))); + internal KnownAssembly(Func predicate, params Func[] or) : this(predicate is null || or.Any(x => x is null) ? throw new ArgumentNullException(nameof(predicate), "All predicates must be non-null.") diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/Helpers/KnownAssemblyTest.cs b/analyzers/tests/SonarAnalyzer.UnitTest/Helpers/KnownAssemblyTest.cs index 9170933ef03..6e7929bd351 100644 --- a/analyzers/tests/SonarAnalyzer.UnitTest/Helpers/KnownAssemblyTest.cs +++ b/analyzers/tests/SonarAnalyzer.UnitTest/Helpers/KnownAssemblyTest.cs @@ -275,6 +275,64 @@ 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 MSTest_Assert_That_V1_FalsePositive() + { + var compilation = TestHelper.CompileCS("// Empty file", NuGetMetadataReference.MSTestTestFrameworkV1.ToArray()).Model.Compilation; + // FP: Assert.That was introduced in Version 1.1.14 but the AssemblyIdentity version (14.0.0.0) does not align with the Nuget version. + compilation.References(MSTest_Assert_That).Should().BeTrue(); + } + + [TestMethod] + public void MSTest_Assert_That_V2_1_1_13_FalsePostive() + { + var compilation = TestHelper.CompileCS("// Empty file", NuGetMetadataReference.MSTestTestFramework("1.1.13").ToArray()).Model.Compilation; + // FP: Assert.That was introduced in Version 1.1.14 but the AssemblyIdentity version (14.0.0.0) does not align with the Nuget version. + compilation.References(MSTest_Assert_That).Should().BeTrue(); + } + + [TestMethod] + public void MSTest_Assert_That_V2_1_1_14() + { + var compilation = TestHelper.CompileCS("// Empty file", NuGetMetadataReference.MSTestTestFramework("1.1.14").ToArray()).Model.Compilation; + compilation.References(MSTest_Assert_That).Should().BeTrue(); // Assert.That was introduced in Version 1.1.14 + } + + [TestMethod] + public void MSTest_Assert_That_MicrosoftVisualStudioQualityToolsUnitTestFramework() + { + var compilation = TestHelper.CompileCS("// Empty file", NuGetMetadataReference.MicrosoftVisualStudioQualityToolsUnitTestFramework.ToArray()).Model.Compilation; + compilation.References(MSTest_Assert_That).Should().BeFalse(); + } + private static Mock CompilationWithReferenceTo(AssemblyIdentity identity) { var compilation = new Mock("compilationName", ImmutableArray.Empty, new Dictionary(), false, null, null); From cd724cb2c18aef03d72a1ab8f5fcecf3c23a8526 Mon Sep 17 00:00:00 2001 From: Martin Strecker Date: Tue, 14 Feb 2023 16:38:38 +0100 Subject: [PATCH 02/35] Add remark for compatibility --- analyzers/src/SonarAnalyzer.Common/Helpers/KnownAssembly.cs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/analyzers/src/SonarAnalyzer.Common/Helpers/KnownAssembly.cs b/analyzers/src/SonarAnalyzer.Common/Helpers/KnownAssembly.cs index d80d66b3d0c..b304821c548 100644 --- a/analyzers/src/SonarAnalyzer.Common/Helpers/KnownAssembly.cs +++ b/analyzers/src/SonarAnalyzer.Common/Helpers/KnownAssembly.cs @@ -41,6 +41,10 @@ public sealed partial class KnownAssembly /// /// Assemblies that contain Assert.That. /// + /// + /// Assert.That was introduced in version 1.1.14 but + /// the NuGet versioning is not in line with the assembly versioning which is 14.0.0.0 for all versions. + /// public static KnownAssembly MSTest_Assert_That { get; } = new( NameIs("Microsoft.VisualStudio.TestPlatform.TestFramework").And(PublicKeyTokenIs("b03f5f7f11d50a3a"))); From cfa25e74251c243f30de7a0960c46b303ae06cec Mon Sep 17 00:00:00 2001 From: Martin Strecker Date: Tue, 14 Feb 2023 16:43:14 +0100 Subject: [PATCH 03/35] Scaffold rule --- analyzers/rspec/cs/S2970_c#.html | 13 ++++++ analyzers/rspec/cs/S2970_c#.json | 17 ++++++++ analyzers/rspec/cs/Sonar_way_profile.json | 1 + .../Rules/AssertionsShouldBeComplete.cs | 43 +++++++++++++++++++ .../PackagingTests/RuleTypeMappingCS.cs | 2 +- .../Rules/AssertionsShouldBeCompleteTest.cs | 33 ++++++++++++++ .../TestCases/AssertionsShouldBeComplete.cs | 5 +++ 7 files changed, 113 insertions(+), 1 deletion(-) create mode 100644 analyzers/rspec/cs/S2970_c#.html create mode 100644 analyzers/rspec/cs/S2970_c#.json create mode 100644 analyzers/src/SonarAnalyzer.CSharp/Rules/AssertionsShouldBeComplete.cs create mode 100644 analyzers/tests/SonarAnalyzer.UnitTest/Rules/AssertionsShouldBeCompleteTest.cs create mode 100644 analyzers/tests/SonarAnalyzer.UnitTest/TestCases/AssertionsShouldBeComplete.cs diff --git a/analyzers/rspec/cs/S2970_c#.html b/analyzers/rspec/cs/S2970_c#.html new file mode 100644 index 00000000000..39873623766 --- /dev/null +++ b/analyzers/rspec/cs/S2970_c#.html @@ -0,0 +1,13 @@ +

It is very easy to write incomplete assertions when using some test frameworks. This rule raises an issue when an assertion doesn’t actually test +anything.

+

Noncompliant Code Example

+
+string actual = "Hello World!";
+actual.Should();  // Noncompliant
+
+

Compliant Solution

+
+string actual = "Hello World!";
+actual.Should().Contain("Hello");
+
+ diff --git a/analyzers/rspec/cs/S2970_c#.json b/analyzers/rspec/cs/S2970_c#.json new file mode 100644 index 00000000000..9cd57fbfeb4 --- /dev/null +++ b/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" +} diff --git a/analyzers/rspec/cs/Sonar_way_profile.json b/analyzers/rspec/cs/Sonar_way_profile.json index 2b7c15c5eb5..76e9af503d7 100644 --- a/analyzers/rspec/cs/Sonar_way_profile.json +++ b/analyzers/rspec/cs/Sonar_way_profile.json @@ -119,6 +119,7 @@ "S2933", "S2934", "S2953", + "S2970", "S2971", "S2995", "S2996", diff --git a/analyzers/src/SonarAnalyzer.CSharp/Rules/AssertionsShouldBeComplete.cs b/analyzers/src/SonarAnalyzer.CSharp/Rules/AssertionsShouldBeComplete.cs new file mode 100644 index 00000000000..cf3bfce600a --- /dev/null +++ b/analyzers/src/SonarAnalyzer.CSharp/Rules/AssertionsShouldBeComplete.cs @@ -0,0 +1,43 @@ +/* + * 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 = "FIXME"; + + private static readonly DiagnosticDescriptor Rule = DescriptorFactory.Create(DiagnosticId, MessageFormat); + + public override ImmutableArray SupportedDiagnostics => ImmutableArray.Create(Rule); + + protected override void Initialize(SonarAnalysisContext context) => + context.RegisterNodeAction(c => + { + var node = c.Node; + if (true) + { + c.ReportIssue(Diagnostic.Create(Rule, node.GetLocation())); + } + }, + SyntaxKind.InvocationExpression); +} diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/PackagingTests/RuleTypeMappingCS.cs b/analyzers/tests/SonarAnalyzer.UnitTest/PackagingTests/RuleTypeMappingCS.cs index 4dc49032b0f..03ae2298a3f 100644 --- a/analyzers/tests/SonarAnalyzer.UnitTest/PackagingTests/RuleTypeMappingCS.cs +++ b/analyzers/tests/SonarAnalyzer.UnitTest/PackagingTests/RuleTypeMappingCS.cs @@ -2894,7 +2894,7 @@ internal static class RuleTypeMappingCS // ["S2967"], // ["S2968"], // ["S2969"], - // ["S2970"], + ["S2970"] = "CODE_SMELL", ["S2971"] = "CODE_SMELL", // ["S2972"], // ["S2973"], diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/Rules/AssertionsShouldBeCompleteTest.cs b/analyzers/tests/SonarAnalyzer.UnitTest/Rules/AssertionsShouldBeCompleteTest.cs new file mode 100644 index 00000000000..363b02538b3 --- /dev/null +++ b/analyzers/tests/SonarAnalyzer.UnitTest/Rules/AssertionsShouldBeCompleteTest.cs @@ -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. + */ + +using SonarAnalyzer.Rules.CSharp; + +namespace SonarAnalyzer.UnitTest.Rules; + +[TestClass] +public class AssertionsShouldBeCompleteTest +{ + private readonly VerifierBuilder builder = new VerifierBuilder(); + + [TestMethod] + public void AssertionsShouldBeComplete_CS() => + builder.AddPaths("AssertionsShouldBeComplete.cs").Verify(); +} diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/AssertionsShouldBeComplete.cs b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/AssertionsShouldBeComplete.cs new file mode 100644 index 00000000000..a790d6d1f92 --- /dev/null +++ b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/AssertionsShouldBeComplete.cs @@ -0,0 +1,5 @@ +using System; + +public class Program +{ +} From 409332108ff3fa1bc8c318c60a0487d29e028769 Mon Sep 17 00:00:00 2001 From: Martin Strecker Date: Wed, 15 Feb 2023 13:46:03 +0100 Subject: [PATCH 04/35] WIP: Register MSTest --- .../Rules/AssertionsShouldBeComplete.cs | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/analyzers/src/SonarAnalyzer.CSharp/Rules/AssertionsShouldBeComplete.cs b/analyzers/src/SonarAnalyzer.CSharp/Rules/AssertionsShouldBeComplete.cs index cf3bfce600a..067eb52afab 100644 --- a/analyzers/src/SonarAnalyzer.CSharp/Rules/AssertionsShouldBeComplete.cs +++ b/analyzers/src/SonarAnalyzer.CSharp/Rules/AssertionsShouldBeComplete.cs @@ -24,20 +24,24 @@ namespace SonarAnalyzer.Rules.CSharp; public sealed class AssertionsShouldBeComplete : SonarDiagnosticAnalyzer { private const string DiagnosticId = "S2970"; - private const string MessageFormat = "FIXME"; + private const string MessageFormat = "Complete the assertion"; private static readonly DiagnosticDescriptor Rule = DescriptorFactory.Create(DiagnosticId, MessageFormat); public override ImmutableArray SupportedDiagnostics => ImmutableArray.Create(Rule); protected override void Initialize(SonarAnalysisContext context) => - context.RegisterNodeAction(c => + context.RegisterCompilationStartAction(c => { - var node = c.Node; - if (true) + if (c.Compilation.References(KnownAssembly.MSTest_Assert_That) + && c.Compilation.GetTypeByMetadataName(KnownType.Microsoft_VisualStudio_TestTools_UnitTesting_Assert) is { } assertType + && assertType.GetMembers("That") is { Length: 1 } thatMembers + && thatMembers[0] is IPropertySymbol { IsStatic: true }) { - c.ReportIssue(Diagnostic.Create(Rule, node.GetLocation())); + c.RegisterNodeAction(c => + { + + }, SyntaxKind.SimpleMemberAccessExpression); } - }, - SyntaxKind.InvocationExpression); + }); } From 58978716b7dcf63ec0439b6aa97ac8b6c7335a4b Mon Sep 17 00:00:00 2001 From: Martin Strecker Date: Thu, 16 Feb 2023 15:09:20 +0100 Subject: [PATCH 05/35] Remove Assert_That from KnownAssemblies as it does not bring much to the table --- .../Rules/AssertionsShouldBeComplete.cs | 4 ++- .../Helpers/KnownAssembly.cs | 10 ------- .../Helpers/KnownAssemblyTest.cs | 30 ------------------- 3 files changed, 3 insertions(+), 41 deletions(-) diff --git a/analyzers/src/SonarAnalyzer.CSharp/Rules/AssertionsShouldBeComplete.cs b/analyzers/src/SonarAnalyzer.CSharp/Rules/AssertionsShouldBeComplete.cs index 067eb52afab..8db98ceeead 100644 --- a/analyzers/src/SonarAnalyzer.CSharp/Rules/AssertionsShouldBeComplete.cs +++ b/analyzers/src/SonarAnalyzer.CSharp/Rules/AssertionsShouldBeComplete.cs @@ -33,7 +33,9 @@ public sealed class AssertionsShouldBeComplete : SonarDiagnosticAnalyzer protected override void Initialize(SonarAnalysisContext context) => context.RegisterCompilationStartAction(c => { - if (c.Compilation.References(KnownAssembly.MSTest_Assert_That) + if (c.Compilation.References(KnownAssembly.MSTest) + // Assert.That was introduced in Version 1.1.14 but the AssemblyIdentity version (14.0.0.0) does not align with the Nuget version so we need + // to check at runtime for the presence of "Assert.That" && c.Compilation.GetTypeByMetadataName(KnownType.Microsoft_VisualStudio_TestTools_UnitTesting_Assert) is { } assertType && assertType.GetMembers("That") is { Length: 1 } thatMembers && thatMembers[0] is IPropertySymbol { IsStatic: true }) diff --git a/analyzers/src/SonarAnalyzer.Common/Helpers/KnownAssembly.cs b/analyzers/src/SonarAnalyzer.Common/Helpers/KnownAssembly.cs index b304821c548..7382154b946 100644 --- a/analyzers/src/SonarAnalyzer.Common/Helpers/KnownAssembly.cs +++ b/analyzers/src/SonarAnalyzer.Common/Helpers/KnownAssembly.cs @@ -38,16 +38,6 @@ public sealed partial class KnownAssembly And(NameIs("Microsoft.VisualStudio.QualityTools.UnitTestFramework").Or(NameIs("Microsoft.VisualStudio.TestPlatform.TestFramework")), PublicKeyTokenIs("b03f5f7f11d50a3a"))); - /// - /// Assemblies that contain Assert.That. - /// - /// - /// Assert.That was introduced in version 1.1.14 but - /// the NuGet versioning is not in line with the assembly versioning which is 14.0.0.0 for all versions. - /// - public static KnownAssembly MSTest_Assert_That { get; } = new( - NameIs("Microsoft.VisualStudio.TestPlatform.TestFramework").And(PublicKeyTokenIs("b03f5f7f11d50a3a"))); - internal KnownAssembly(Func predicate, params Func[] or) : this(predicate is null || or.Any(x => x is null) ? throw new ArgumentNullException(nameof(predicate), "All predicates must be non-null.") diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/Helpers/KnownAssemblyTest.cs b/analyzers/tests/SonarAnalyzer.UnitTest/Helpers/KnownAssemblyTest.cs index 6e7929bd351..63a1d4b365f 100644 --- a/analyzers/tests/SonarAnalyzer.UnitTest/Helpers/KnownAssemblyTest.cs +++ b/analyzers/tests/SonarAnalyzer.UnitTest/Helpers/KnownAssemblyTest.cs @@ -303,36 +303,6 @@ public void MSTest_NoReference() compilation.References(MSTest).Should().BeFalse(); } - [TestMethod] - public void MSTest_Assert_That_V1_FalsePositive() - { - var compilation = TestHelper.CompileCS("// Empty file", NuGetMetadataReference.MSTestTestFrameworkV1.ToArray()).Model.Compilation; - // FP: Assert.That was introduced in Version 1.1.14 but the AssemblyIdentity version (14.0.0.0) does not align with the Nuget version. - compilation.References(MSTest_Assert_That).Should().BeTrue(); - } - - [TestMethod] - public void MSTest_Assert_That_V2_1_1_13_FalsePostive() - { - var compilation = TestHelper.CompileCS("// Empty file", NuGetMetadataReference.MSTestTestFramework("1.1.13").ToArray()).Model.Compilation; - // FP: Assert.That was introduced in Version 1.1.14 but the AssemblyIdentity version (14.0.0.0) does not align with the Nuget version. - compilation.References(MSTest_Assert_That).Should().BeTrue(); - } - - [TestMethod] - public void MSTest_Assert_That_V2_1_1_14() - { - var compilation = TestHelper.CompileCS("// Empty file", NuGetMetadataReference.MSTestTestFramework("1.1.14").ToArray()).Model.Compilation; - compilation.References(MSTest_Assert_That).Should().BeTrue(); // Assert.That was introduced in Version 1.1.14 - } - - [TestMethod] - public void MSTest_Assert_That_MicrosoftVisualStudioQualityToolsUnitTestFramework() - { - var compilation = TestHelper.CompileCS("// Empty file", NuGetMetadataReference.MicrosoftVisualStudioQualityToolsUnitTestFramework.ToArray()).Model.Compilation; - compilation.References(MSTest_Assert_That).Should().BeFalse(); - } - private static Mock CompilationWithReferenceTo(AssemblyIdentity identity) { var compilation = new Mock("compilationName", ImmutableArray.Empty, new Dictionary(), false, null, null); From 16a04a341eccf83c59931f83f5008f715ff3c2b8 Mon Sep 17 00:00:00 2001 From: Martin Strecker Date: Thu, 16 Feb 2023 16:49:14 +0100 Subject: [PATCH 06/35] Add support for NFluent, NSubsutiture and FluentAssertion --- .../Rules/AssertionsShouldBeComplete.cs | 21 ++++++++ .../Helpers/KnownAssembly.cs | 9 ++++ .../Helpers/KnownAssemblyTest.cs | 50 +++++++++++++++++++ 3 files changed, 80 insertions(+) diff --git a/analyzers/src/SonarAnalyzer.CSharp/Rules/AssertionsShouldBeComplete.cs b/analyzers/src/SonarAnalyzer.CSharp/Rules/AssertionsShouldBeComplete.cs index 8db98ceeead..10a40fe3c7a 100644 --- a/analyzers/src/SonarAnalyzer.CSharp/Rules/AssertionsShouldBeComplete.cs +++ b/analyzers/src/SonarAnalyzer.CSharp/Rules/AssertionsShouldBeComplete.cs @@ -45,5 +45,26 @@ public sealed class AssertionsShouldBeComplete : SonarDiagnosticAnalyzer }, SyntaxKind.SimpleMemberAccessExpression); } + if (c.Compilation.References(KnownAssembly.FluentAssertions)) + { + c.RegisterNodeAction(c => + { + + }, SyntaxKind.InvocationExpression); + } + if (c.Compilation.References(KnownAssembly.NFluent)) + { + c.RegisterNodeAction(c => + { + + }, SyntaxKind.InvocationExpression); + } + if (c.Compilation.References(KnownAssembly.NSubstitute)) + { + c.RegisterNodeAction(c => + { + + }, SyntaxKind.InvocationExpression); + } }); } diff --git a/analyzers/src/SonarAnalyzer.Common/Helpers/KnownAssembly.cs b/analyzers/src/SonarAnalyzer.Common/Helpers/KnownAssembly.cs index 7382154b946..5b73c8cc352 100644 --- a/analyzers/src/SonarAnalyzer.Common/Helpers/KnownAssembly.cs +++ b/analyzers/src/SonarAnalyzer.Common/Helpers/KnownAssembly.cs @@ -38,6 +38,15 @@ public sealed partial class KnownAssembly 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 predicate, params Func[] or) : this(predicate is null || or.Any(x => x is null) ? throw new ArgumentNullException(nameof(predicate), "All predicates must be non-null.") diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/Helpers/KnownAssemblyTest.cs b/analyzers/tests/SonarAnalyzer.UnitTest/Helpers/KnownAssemblyTest.cs index 63a1d4b365f..c128ff60764 100644 --- a/analyzers/tests/SonarAnalyzer.UnitTest/Helpers/KnownAssemblyTest.cs +++ b/analyzers/tests/SonarAnalyzer.UnitTest/Helpers/KnownAssemblyTest.cs @@ -303,6 +303,56 @@ public void MSTest_NoReference() 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 CompilationWithReferenceTo(AssemblyIdentity identity) { var compilation = new Mock("compilationName", ImmutableArray.Empty, new Dictionary(), false, null, null); From 3067aea09cad13f8d15f1763ece09c2010ac6e48 Mon Sep 17 00:00:00 2001 From: Martin Strecker Date: Thu, 16 Feb 2023 16:59:06 +0100 Subject: [PATCH 07/35] wip: add checks --- .../Rules/AssertionsShouldBeComplete.cs | 27 +++++++++++-------- 1 file changed, 16 insertions(+), 11 deletions(-) diff --git a/analyzers/src/SonarAnalyzer.CSharp/Rules/AssertionsShouldBeComplete.cs b/analyzers/src/SonarAnalyzer.CSharp/Rules/AssertionsShouldBeComplete.cs index 10a40fe3c7a..49bafff2aa0 100644 --- a/analyzers/src/SonarAnalyzer.CSharp/Rules/AssertionsShouldBeComplete.cs +++ b/analyzers/src/SonarAnalyzer.CSharp/Rules/AssertionsShouldBeComplete.cs @@ -18,6 +18,8 @@ * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. */ +using SonarAnalyzer.Helpers; + namespace SonarAnalyzer.Rules.CSharp; [DiagnosticAnalyzer(LanguageNames.CSharp)] @@ -31,40 +33,43 @@ public sealed class AssertionsShouldBeComplete : SonarDiagnosticAnalyzer public override ImmutableArray SupportedDiagnostics => ImmutableArray.Create(Rule); protected override void Initialize(SonarAnalysisContext context) => - context.RegisterCompilationStartAction(c => + context.RegisterCompilationStartAction(start => { - if (c.Compilation.References(KnownAssembly.MSTest) + if (start.Compilation.References(KnownAssembly.MSTest) // Assert.That was introduced in Version 1.1.14 but the AssemblyIdentity version (14.0.0.0) does not align with the Nuget version so we need // to check at runtime for the presence of "Assert.That" - && c.Compilation.GetTypeByMetadataName(KnownType.Microsoft_VisualStudio_TestTools_UnitTesting_Assert) is { } assertType + && start.Compilation.GetTypeByMetadataName(KnownType.Microsoft_VisualStudio_TestTools_UnitTesting_Assert) is { } assertType && assertType.GetMembers("That") is { Length: 1 } thatMembers && thatMembers[0] is IPropertySymbol { IsStatic: true }) { - c.RegisterNodeAction(c => + start.RegisterNodeAction(c => { }, SyntaxKind.SimpleMemberAccessExpression); } - if (c.Compilation.References(KnownAssembly.FluentAssertions)) + if (start.Compilation.References(KnownAssembly.FluentAssertions)) { - c.RegisterNodeAction(c => + start.RegisterNodeAction(c => { - + CheckInvocation(c, invoke => invoke.NameIs("Should") + && c.SemanticModel.GetSymbolInfo(invoke) is { Symbol: IMethodSymbol method } + && method.ContainingType.Is(KnownType.FluentAssertions_Execution_AssertionScope)); }, SyntaxKind.InvocationExpression); } - if (c.Compilation.References(KnownAssembly.NFluent)) + if (start.Compilation.References(KnownAssembly.NFluent)) { - c.RegisterNodeAction(c => + start.RegisterNodeAction(c => { }, SyntaxKind.InvocationExpression); } - if (c.Compilation.References(KnownAssembly.NSubstitute)) + if (start.Compilation.References(KnownAssembly.NSubstitute)) { - c.RegisterNodeAction(c => + start.RegisterNodeAction(c => { }, SyntaxKind.InvocationExpression); } }); + private void CheckInvocation(SonarSyntaxNodeReportingContext c, Func value) => throw new NotImplementedException(); } From 7450e81ffb050ac3e7bfb21c604f4f70a54df3d4 Mon Sep 17 00:00:00 2001 From: Martin Strecker Date: Mon, 20 Feb 2023 14:19:31 +0100 Subject: [PATCH 08/35] WIP: Add implementation and test for FluentAssertion --- .../Helpers/CSharpSyntaxHelper.cs | 5 ++ .../Rules/AssertionsShouldBeComplete.cs | 65 +++++++++++++++---- .../Extensions/SymbolInfoExtensions.cs | 34 ++++++++++ .../Rules/AssertionsShouldBeCompleteTest.cs | 9 ++- ...rtionsShouldBeComplete.FluentAssertions.cs | 12 ++++ .../TestCases/AssertionsShouldBeComplete.cs | 5 -- 6 files changed, 110 insertions(+), 20 deletions(-) create mode 100644 analyzers/src/SonarAnalyzer.Common/Extensions/SymbolInfoExtensions.cs create mode 100644 analyzers/tests/SonarAnalyzer.UnitTest/TestCases/AssertionsShouldBeComplete.FluentAssertions.cs delete mode 100644 analyzers/tests/SonarAnalyzer.UnitTest/TestCases/AssertionsShouldBeComplete.cs diff --git a/analyzers/src/SonarAnalyzer.CSharp/Helpers/CSharpSyntaxHelper.cs b/analyzers/src/SonarAnalyzer.CSharp/Helpers/CSharpSyntaxHelper.cs index 8b30ece4ad5..4dd747bae43 100644 --- a/analyzers/src/SonarAnalyzer.CSharp/Helpers/CSharpSyntaxHelper.cs +++ b/analyzers/src/SonarAnalyzer.CSharp/Helpers/CSharpSyntaxHelper.cs @@ -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; diff --git a/analyzers/src/SonarAnalyzer.CSharp/Rules/AssertionsShouldBeComplete.cs b/analyzers/src/SonarAnalyzer.CSharp/Rules/AssertionsShouldBeComplete.cs index 49bafff2aa0..41b66fd76a9 100644 --- a/analyzers/src/SonarAnalyzer.CSharp/Rules/AssertionsShouldBeComplete.cs +++ b/analyzers/src/SonarAnalyzer.CSharp/Rules/AssertionsShouldBeComplete.cs @@ -18,8 +18,6 @@ * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. */ -using SonarAnalyzer.Helpers; - namespace SonarAnalyzer.Rules.CSharp; [DiagnosticAnalyzer(LanguageNames.CSharp)] @@ -50,26 +48,65 @@ public sealed class AssertionsShouldBeComplete : SonarDiagnosticAnalyzer if (start.Compilation.References(KnownAssembly.FluentAssertions)) { start.RegisterNodeAction(c => - { - CheckInvocation(c, invoke => invoke.NameIs("Should") - && c.SemanticModel.GetSymbolInfo(invoke) is { Symbol: IMethodSymbol method } - && method.ContainingType.Is(KnownType.FluentAssertions_Execution_AssertionScope)); - }, SyntaxKind.InvocationExpression); + CheckInvocation(c, invocation => + invocation.NameIs("Should") + && c.SemanticModel.GetSymbolInfo(invocation).AllSymbols().Any(x => + x is IMethodSymbol { IsExtensionMethod: true } method + && method.ContainingType?.IsStatic is true + && method.ContainingType.Is(KnownType.FluentAssertions_AssertionExtensions))), + SyntaxKind.InvocationExpression); } if (start.Compilation.References(KnownAssembly.NFluent)) { start.RegisterNodeAction(c => - { - - }, SyntaxKind.InvocationExpression); + CheckInvocation(c, invocation => + invocation.NameIs("That", "ThatCode", "ThatAsyncCode", "ThatDynamic") + && c.SemanticModel.GetSymbolInfo(invocation) is { Symbol: IMethodSymbol { IsStatic: true } method } + && method.ContainingType?.IsStatic is true + && method.ContainingType.Is(KnownType.NFluent_Check)), SyntaxKind.InvocationExpression); } if (start.Compilation.References(KnownAssembly.NSubstitute)) { start.RegisterNodeAction(c => - { - - }, SyntaxKind.InvocationExpression); + CheckInvocation(c, invocation => + invocation.NameIs("DidNotReceive", "DidNotReceiveWithAnyArgs", "Received", "ReceivedCalls", "ReceivedWithAnyArgs") + && c.SemanticModel.GetSymbolInfo(invocation) is { Symbol: IMethodSymbol { IsStatic: true } method } + && method.ContainingType?.IsStatic is true + && method.ContainingType.Is(KnownType.NSubstitute_SubstituteExtensions)), SyntaxKind.InvocationExpression); } }); - private void CheckInvocation(SonarSyntaxNodeReportingContext c, Func value) => throw new NotImplementedException(); + + private void CheckInvocation(SonarSyntaxNodeReportingContext c, Func isAssertionMethod) + { + if (c.Node is InvocationExpressionSyntax invocation + && isAssertionMethod(invocation) + && !HasContinuation(invocation)) + { + c.ReportIssue(Diagnostic.Create(Rule, invocation.GetLocation())); + } + } + + private static bool HasContinuation(InvocationExpressionSyntax invocation) + { + var closeParen = invocation.ArgumentList.CloseParenToken; + if (!closeParen.IsKind(SyntaxKind.CloseParenToken)) + { + // 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 + if (invocation.Ancestors().TakeWhile(x => !(x.IsAnyKind(SyntaxKind.Block, SyntaxKindEx.LocalFunctionStatement) || x is BaseMethodDeclarationSyntax or AnonymousFunctionExpressionSyntax)). + Any(x => x.IsAnyKind(SyntaxKind.ReturnStatement, SyntaxKind.ArrowExpressionClause) || x is AssignmentExpressionSyntax or LocalDeclarationStatementSyntax)) + { + return true; + } + return false; + } } diff --git a/analyzers/src/SonarAnalyzer.Common/Extensions/SymbolInfoExtensions.cs b/analyzers/src/SonarAnalyzer.Common/Extensions/SymbolInfoExtensions.cs new file mode 100644 index 00000000000..a4bc0116da3 --- /dev/null +++ b/analyzers/src/SonarAnalyzer.Common/Extensions/SymbolInfoExtensions.cs @@ -0,0 +1,34 @@ +/* + * 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 + { + /// + /// Returns the or if no symbol could be found the . + /// + public static IEnumerable AllSymbols(this SymbolInfo symbolInfo) => + symbolInfo.Symbol == null + ? symbolInfo.CandidateSymbols + : new[] { symbolInfo.Symbol }; + } +} diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/Rules/AssertionsShouldBeCompleteTest.cs b/analyzers/tests/SonarAnalyzer.UnitTest/Rules/AssertionsShouldBeCompleteTest.cs index 363b02538b3..31aed59835c 100644 --- a/analyzers/tests/SonarAnalyzer.UnitTest/Rules/AssertionsShouldBeCompleteTest.cs +++ b/analyzers/tests/SonarAnalyzer.UnitTest/Rules/AssertionsShouldBeCompleteTest.cs @@ -19,6 +19,7 @@ */ using SonarAnalyzer.Rules.CSharp; +using SonarAnalyzer.UnitTest.MetadataReferences; namespace SonarAnalyzer.UnitTest.Rules; @@ -29,5 +30,11 @@ public class AssertionsShouldBeCompleteTest [TestMethod] public void AssertionsShouldBeComplete_CS() => - builder.AddPaths("AssertionsShouldBeComplete.cs").Verify(); + builder + .AddPaths("AssertionsShouldBeComplete.FluentAssertions.cs") + .AddReferences(NuGetMetadataReference.FluentAssertions("6.10")) + .AddReferences(MetadataReferenceFacade.SystemXml) + .AddReferences(MetadataReferenceFacade.SystemNetHttp) + .AddReferences(MetadataReferenceFacade.SystemData) + .Verify(); } diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/AssertionsShouldBeComplete.FluentAssertions.cs b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/AssertionsShouldBeComplete.FluentAssertions.cs new file mode 100644 index 00000000000..a76a282ebf1 --- /dev/null +++ b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/AssertionsShouldBeComplete.FluentAssertions.cs @@ -0,0 +1,12 @@ +using System; +using FluentAssertions; + +public class Program +{ + public void Test() + { + var s = "Test"; + s.Should(); // Noncompliant {{Complete the assertion}} + s.Should().Be("Test"); // Compliant + } +} diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/AssertionsShouldBeComplete.cs b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/AssertionsShouldBeComplete.cs deleted file mode 100644 index a790d6d1f92..00000000000 --- a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/AssertionsShouldBeComplete.cs +++ /dev/null @@ -1,5 +0,0 @@ -using System; - -public class Program -{ -} From a6c9549ae7df48b967f56ea7a8d3f603503db4a3 Mon Sep 17 00:00:00 2001 From: Martin Strecker Date: Mon, 20 Feb 2023 16:00:18 +0100 Subject: [PATCH 09/35] Add support for custom assertions --- .../Rules/AssertionsShouldBeComplete.cs | 4 +- ...rtionsShouldBeComplete.FluentAssertions.cs | 104 +++++++++++++++++- 2 files changed, 105 insertions(+), 3 deletions(-) diff --git a/analyzers/src/SonarAnalyzer.CSharp/Rules/AssertionsShouldBeComplete.cs b/analyzers/src/SonarAnalyzer.CSharp/Rules/AssertionsShouldBeComplete.cs index 41b66fd76a9..920492aee03 100644 --- a/analyzers/src/SonarAnalyzer.CSharp/Rules/AssertionsShouldBeComplete.cs +++ b/analyzers/src/SonarAnalyzer.CSharp/Rules/AssertionsShouldBeComplete.cs @@ -52,8 +52,8 @@ public sealed class AssertionsShouldBeComplete : SonarDiagnosticAnalyzer invocation.NameIs("Should") && c.SemanticModel.GetSymbolInfo(invocation).AllSymbols().Any(x => x is IMethodSymbol { IsExtensionMethod: true } method - && method.ContainingType?.IsStatic is true - && method.ContainingType.Is(KnownType.FluentAssertions_AssertionExtensions))), + && (method.ContainingType.Is(KnownType.FluentAssertions_AssertionExtensions) + || method.ReturnType.DerivesFrom(KnownType.FluentAssertions_Primitives_ReferenceTypeAssertions)))), SyntaxKind.InvocationExpression); } if (start.Compilation.References(KnownAssembly.NFluent)) diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/AssertionsShouldBeComplete.FluentAssertions.cs b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/AssertionsShouldBeComplete.FluentAssertions.cs index a76a282ebf1..715b09eb91f 100644 --- a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/AssertionsShouldBeComplete.FluentAssertions.cs +++ b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/AssertionsShouldBeComplete.FluentAssertions.cs @@ -1,12 +1,114 @@ using System; +using System.Collections.Generic; using FluentAssertions; +using FluentAssertions.Primitives; public class Program { - public void Test() + public void StringAssertions() { var s = "Test"; s.Should(); // Noncompliant {{Complete the assertion}} +// ^^^^^^^^^^ + s?.Should(); // Noncompliant + s[0].Should(); // Error [CS0121] ambiguous calls + // Noncompliant@-1 + s?[0].Should(); // Error [CS0121] ambiguous calls + // Noncompliant@-1 s.Should().Be("Test"); // Compliant } + + public void DictAssertions() + { + var dict = new Dictionary(); + dict["A"].Should(); // Noncompliant + dict["A"]?.Should(); // Noncompliant + dict?["A"].Should(); // Noncompliant + dict?["A"]?.Should(); // Noncompliant + } + + public object ReturnedByReturn() + { + var s = "Test"; + return s.Should(); // Compliant + } + + public object ReturnedByArrow(string s) => + s.Should(); // Compliant + + public void Assigned() + { + var s = "Test"; + var assertion = s.Should(); // Compliant + assertion = s.Should(); // Compliant + } + + public void PassedAsArgument() + { + var s = "Test"; + ValidateString(s.Should()); // Compliant + } + private void ValidateString(StringAssertions assertion) { } + + public void UnreducedCall() + { + var s = "Test"; + FluentAssertions.AssertionExtensions.Should(s); // Noncompliant + } + + public void CustomAssertions() + { + var custom = new Custom(); + custom.Should(); // Noncompliant The custom assertion derives from ReferenceTypeAssertions + custom.Should().BeCustomAsserted(); // Compliant + } + + public void CustomStructAssertions() + { + var custom = new CustomStruct(); + custom.Should(); // Compliant Potential FN. CustomStructAssertion does not derive from ReferenceTypeAssertions and is not considered a custom validation. + custom.Should().BeCustomAsserted(); // Compliant + } +} + +public static class CustomAssertionExtension +{ + public static CustomAssertion Should(this Custom instance) + => new CustomAssertion(instance); +} + +public class CustomAssertion : ReferenceTypeAssertions +{ + public CustomAssertion(Custom instance) + : base(instance) + { + } + + protected override string Identifier => "custom"; + + public void BeCustomAsserted() + { + // Not implemented + } } + +public class Custom { } + +public static class CustomStructAssertionExtension +{ + public static CustomStructAssertion Should(this CustomStruct instance) + => new CustomStructAssertion(instance); +} + +public class CustomStructAssertion // Does not derive from ReferenceTypeAssertions +{ + public CustomStructAssertion(CustomStruct instance) { } + + public void BeCustomAsserted() + { + // Not implemented + } +} + +public struct CustomStruct { } + From 4991d86bac54099355d633441bc87d67b5ef526c Mon Sep 17 00:00:00 2001 From: Martin Strecker Date: Mon, 20 Feb 2023 17:21:25 +0100 Subject: [PATCH 10/35] Move nullable tests in own test file and fix Netframework dependencies --- .../Rules/AssertionsShouldBeComplete.cs | 2 +- .../Rules/AssertionsShouldBeCompleteTest.cs | 24 ++++++++++----- ...uldBeComplete.FluentAssertions.CSharp7.cs} | 20 +++++-------- ...ouldBeComplete.FluentAssertions.CSharp8.cs | 30 +++++++++++++++++++ 4 files changed, 55 insertions(+), 21 deletions(-) rename analyzers/tests/SonarAnalyzer.UnitTest/TestCases/{AssertionsShouldBeComplete.FluentAssertions.cs => AssertionsShouldBeComplete.FluentAssertions.CSharp7.cs} (79%) create mode 100644 analyzers/tests/SonarAnalyzer.UnitTest/TestCases/AssertionsShouldBeComplete.FluentAssertions.CSharp8.cs diff --git a/analyzers/src/SonarAnalyzer.CSharp/Rules/AssertionsShouldBeComplete.cs b/analyzers/src/SonarAnalyzer.CSharp/Rules/AssertionsShouldBeComplete.cs index 920492aee03..bee9035511e 100644 --- a/analyzers/src/SonarAnalyzer.CSharp/Rules/AssertionsShouldBeComplete.cs +++ b/analyzers/src/SonarAnalyzer.CSharp/Rules/AssertionsShouldBeComplete.cs @@ -82,7 +82,7 @@ private void CheckInvocation(SonarSyntaxNodeReportingContext c, Func(); - - [TestMethod] - public void AssertionsShouldBeComplete_CS() => - builder - .AddPaths("AssertionsShouldBeComplete.FluentAssertions.cs") + private readonly VerifierBuilder fluentAssertions = new VerifierBuilder() .AddReferences(NuGetMetadataReference.FluentAssertions("6.10")) .AddReferences(MetadataReferenceFacade.SystemXml) + .AddReferences(MetadataReferenceFacade.SystemXmlLinq) .AddReferences(MetadataReferenceFacade.SystemNetHttp) - .AddReferences(MetadataReferenceFacade.SystemData) + .AddReferences(MetadataReferenceFacade.SystemData); + + [TestMethod] + public void AssertionsShouldBeComplete_CSharp7() => + fluentAssertions + .WithOptions(ParseOptionsHelper.OnlyCSharp7) + .AddPaths("AssertionsShouldBeComplete.FluentAssertions.CSharp7.cs") + .Verify(); + + [TestMethod] + public void AssertionsShouldBeComplete_CSharp8() => + fluentAssertions + .WithOptions(ParseOptionsHelper.FromCSharp8) + .AddPaths("AssertionsShouldBeComplete.FluentAssertions.CSharp8.cs") + .WithConcurrentAnalysis(false) .Verify(); } diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/AssertionsShouldBeComplete.FluentAssertions.cs b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/AssertionsShouldBeComplete.FluentAssertions.CSharp7.cs similarity index 79% rename from analyzers/tests/SonarAnalyzer.UnitTest/TestCases/AssertionsShouldBeComplete.FluentAssertions.cs rename to analyzers/tests/SonarAnalyzer.UnitTest/TestCases/AssertionsShouldBeComplete.FluentAssertions.CSharp7.cs index 715b09eb91f..15204c5cfc0 100644 --- a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/AssertionsShouldBeComplete.FluentAssertions.cs +++ b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/AssertionsShouldBeComplete.FluentAssertions.CSharp7.cs @@ -8,32 +8,26 @@ public class Program public void StringAssertions() { var s = "Test"; - s.Should(); // Noncompliant {{Complete the assertion}} -// ^^^^^^^^^^ - s?.Should(); // Noncompliant - s[0].Should(); // Error [CS0121] ambiguous calls - // Noncompliant@-1 - s?[0].Should(); // Error [CS0121] ambiguous calls - // Noncompliant@-1 - s.Should().Be("Test"); // Compliant + s.Should(); // Noncompliant {{Complete the assertion}} +// ^^^^^^ + s[0].Should(); // Error [CS0121] ambiguous calls + // Noncompliant@-1 + s.Should().Be("Test"); // Compliant } public void DictAssertions() { var dict = new Dictionary(); dict["A"].Should(); // Noncompliant - dict["A"]?.Should(); // Noncompliant - dict?["A"].Should(); // Noncompliant - dict?["A"]?.Should(); // Noncompliant } - public object ReturnedByReturn() + public StringAssertions ReturnedByReturn() { var s = "Test"; return s.Should(); // Compliant } - public object ReturnedByArrow(string s) => + public StringAssertions ReturnedByArrow(string s) => s.Should(); // Compliant public void Assigned() diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/AssertionsShouldBeComplete.FluentAssertions.CSharp8.cs b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/AssertionsShouldBeComplete.FluentAssertions.CSharp8.cs new file mode 100644 index 00000000000..09ba874e12c --- /dev/null +++ b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/AssertionsShouldBeComplete.FluentAssertions.CSharp8.cs @@ -0,0 +1,30 @@ +#nullable enable + +using System; +using System.Collections.Generic; +using FluentAssertions; +using FluentAssertions.Primitives; + +public class Program +{ + public void StringAssertions() + { + var s = "Test"; + s?.Should(); // Noncompliant + s?[0].Should(); // Noncompliant + s.Should().Be("Test"); // Compliant + s.Should()?.Be("Test"); // Compliant + s.Should()!?.Be("Test"); // Compliant + } + + public void DictAssertions() + { + var dict = new Dictionary(); + dict["A"]?.Should(); // Noncompliant + // ^^^^^^ + dict?["A"]?.Should(); // Noncompliant + // ^^^^^^ + dict?["A"]!.Should(); // Noncompliant + // ^^^^^^ + } +} From e6a630ca3f2cfbc2b3ae3a5ce5aab0df8c4e4bd9 Mon Sep 17 00:00:00 2001 From: Martin Strecker Date: Mon, 20 Feb 2023 17:43:08 +0100 Subject: [PATCH 11/35] Add type argument test --- .../Rules/AssertionsShouldBeComplete.cs | 2 +- ...ouldBeComplete.FluentAssertions.CSharp7.cs | 21 +++++++++++++------ 2 files changed, 16 insertions(+), 7 deletions(-) diff --git a/analyzers/src/SonarAnalyzer.CSharp/Rules/AssertionsShouldBeComplete.cs b/analyzers/src/SonarAnalyzer.CSharp/Rules/AssertionsShouldBeComplete.cs index bee9035511e..6fcc08badf3 100644 --- a/analyzers/src/SonarAnalyzer.CSharp/Rules/AssertionsShouldBeComplete.cs +++ b/analyzers/src/SonarAnalyzer.CSharp/Rules/AssertionsShouldBeComplete.cs @@ -89,7 +89,7 @@ private void CheckInvocation(SonarSyntaxNodeReportingContext c, Func(); // Noncompliant + collection.Should().Equal("Test", "Test"); // Compliant + } + + public void DictionaryAssertions() { var dict = new Dictionary(); dict["A"].Should(); // Noncompliant From 286b13b3a3e4210e5a092c9d56fef58460a6f59f Mon Sep 17 00:00:00 2001 From: Martin Strecker Date: Mon, 20 Feb 2023 17:46:11 +0100 Subject: [PATCH 12/35] Demonstrate better overload resolution inC#8 vs C#7 --- .../AssertionsShouldBeComplete.FluentAssertions.CSharp8.cs | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/AssertionsShouldBeComplete.FluentAssertions.CSharp8.cs b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/AssertionsShouldBeComplete.FluentAssertions.CSharp8.cs index 09ba874e12c..9ce5a982177 100644 --- a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/AssertionsShouldBeComplete.FluentAssertions.CSharp8.cs +++ b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/AssertionsShouldBeComplete.FluentAssertions.CSharp8.cs @@ -10,6 +10,7 @@ public class Program public void StringAssertions() { var s = "Test"; + s.Should(); // Noncompliant (no "ambiguous calls" compiler error as in C# 7) s?.Should(); // Noncompliant s?[0].Should(); // Noncompliant s.Should().Be("Test"); // Compliant @@ -17,6 +18,12 @@ public void StringAssertions() s.Should()!?.Be("Test"); // Compliant } + public void CollectionAssertions() + { + var collection = new[] { "Test", "Test" }; + collection.Should(); // Noncompliant (no "ambiguous calls" compiler error as in C# 7) + } + public void DictAssertions() { var dict = new Dictionary(); From d7be4d94c09545824ed8f4005175e44b699c4fb2 Mon Sep 17 00:00:00 2001 From: Martin Strecker Date: Mon, 20 Feb 2023 17:47:46 +0100 Subject: [PATCH 13/35] Fix wrong annotated compiler error --- .../AssertionsShouldBeComplete.FluentAssertions.CSharp8.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/AssertionsShouldBeComplete.FluentAssertions.CSharp8.cs b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/AssertionsShouldBeComplete.FluentAssertions.CSharp8.cs index 9ce5a982177..dad424a6a48 100644 --- a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/AssertionsShouldBeComplete.FluentAssertions.CSharp8.cs +++ b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/AssertionsShouldBeComplete.FluentAssertions.CSharp8.cs @@ -10,8 +10,9 @@ public class Program public void StringAssertions() { var s = "Test"; - s.Should(); // Noncompliant (no "ambiguous calls" compiler error as in C# 7) + s.Should(); // Noncompliant s?.Should(); // Noncompliant + s[0].Should(); // Noncompliant (no "ambiguous calls" compiler error as in C# 7) s?[0].Should(); // Noncompliant s.Should().Be("Test"); // Compliant s.Should()?.Be("Test"); // Compliant From c4340a8c5ef3e7920b600143efeb7bc1db0fac89 Mon Sep 17 00:00:00 2001 From: Martin Strecker Date: Mon, 20 Feb 2023 17:51:04 +0100 Subject: [PATCH 14/35] Add KnownTypes after rebase --- analyzers/src/SonarAnalyzer.Common/Helpers/KnownType.cs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/analyzers/src/SonarAnalyzer.Common/Helpers/KnownType.cs b/analyzers/src/SonarAnalyzer.Common/Helpers/KnownType.cs index 7b0c7b39c21..9eaa2c8dccb 100644 --- a/analyzers/src/SonarAnalyzer.Common/Helpers/KnownType.cs +++ b/analyzers/src/SonarAnalyzer.Common/Helpers/KnownType.cs @@ -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"); From 8531f37751b49b3d3fafedb1c82e662154826bd7 Mon Sep 17 00:00:00 2001 From: Martin Strecker Date: Mon, 20 Feb 2023 18:44:12 +0100 Subject: [PATCH 15/35] Add comment --- .../src/SonarAnalyzer.CSharp/Rules/AssertionsShouldBeComplete.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/analyzers/src/SonarAnalyzer.CSharp/Rules/AssertionsShouldBeComplete.cs b/analyzers/src/SonarAnalyzer.CSharp/Rules/AssertionsShouldBeComplete.cs index 6fcc08badf3..02203182ddc 100644 --- a/analyzers/src/SonarAnalyzer.CSharp/Rules/AssertionsShouldBeComplete.cs +++ b/analyzers/src/SonarAnalyzer.CSharp/Rules/AssertionsShouldBeComplete.cs @@ -53,6 +53,7 @@ public sealed class AssertionsShouldBeComplete : SonarDiagnosticAnalyzer && c.SemanticModel.GetSymbolInfo(invocation).AllSymbols().Any(x => x is IMethodSymbol { IsExtensionMethod: true } method && (method.ContainingType.Is(KnownType.FluentAssertions_AssertionExtensions) + // ⬆️ Built in assertions. ⬇️ Custom assertions (the majority at least). || method.ReturnType.DerivesFrom(KnownType.FluentAssertions_Primitives_ReferenceTypeAssertions)))), SyntaxKind.InvocationExpression); } From 0f957904c868e4a7a3dfa41b9332e14f45bdec8c Mon Sep 17 00:00:00 2001 From: Martin Strecker Date: Tue, 21 Feb 2023 17:02:43 +0100 Subject: [PATCH 16/35] NFluent: Add tests --- .../Rules/AssertionsShouldBeCompleteTest.cs | 16 +++++++++++++--- .../AssertionsShouldBeComplete.NFluent.cs | 11 +++++++++++ 2 files changed, 24 insertions(+), 3 deletions(-) create mode 100644 analyzers/tests/SonarAnalyzer.UnitTest/TestCases/AssertionsShouldBeComplete.NFluent.cs diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/Rules/AssertionsShouldBeCompleteTest.cs b/analyzers/tests/SonarAnalyzer.UnitTest/Rules/AssertionsShouldBeCompleteTest.cs index 78dfb129ad9..295b04ebe31 100644 --- a/analyzers/tests/SonarAnalyzer.UnitTest/Rules/AssertionsShouldBeCompleteTest.cs +++ b/analyzers/tests/SonarAnalyzer.UnitTest/Rules/AssertionsShouldBeCompleteTest.cs @@ -27,24 +27,34 @@ namespace SonarAnalyzer.UnitTest.Rules; public class AssertionsShouldBeCompleteTest { private readonly VerifierBuilder fluentAssertions = new VerifierBuilder() - .AddReferences(NuGetMetadataReference.FluentAssertions("6.10")) + .AddReferences(NuGetMetadataReference.FluentAssertions("6.10.0")) .AddReferences(MetadataReferenceFacade.SystemXml) .AddReferences(MetadataReferenceFacade.SystemXmlLinq) .AddReferences(MetadataReferenceFacade.SystemNetHttp) .AddReferences(MetadataReferenceFacade.SystemData); + private readonly VerifierBuilder nfluent = new VerifierBuilder() + .AddReferences(NuGetMetadataReference.NFluent("2.8.0")); + [TestMethod] - public void AssertionsShouldBeComplete_CSharp7() => + public void AssertionsShouldBeComplete_FluentAssertions_CSharp7() => fluentAssertions .WithOptions(ParseOptionsHelper.OnlyCSharp7) .AddPaths("AssertionsShouldBeComplete.FluentAssertions.CSharp7.cs") .Verify(); [TestMethod] - public void AssertionsShouldBeComplete_CSharp8() => + public void AssertionsShouldBeComplete_FluentAssertions_CSharp8() => fluentAssertions .WithOptions(ParseOptionsHelper.FromCSharp8) .AddPaths("AssertionsShouldBeComplete.FluentAssertions.CSharp8.cs") .WithConcurrentAnalysis(false) .Verify(); + + [TestMethod] + public void AssertionsShouldBeComplete_NFluent_CSharp() => + nfluent + .AddTestReference() + .AddPaths("AssertionsShouldBeComplete.NFluent.cs") + .Verify(); } diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/AssertionsShouldBeComplete.NFluent.cs b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/AssertionsShouldBeComplete.NFluent.cs new file mode 100644 index 00000000000..4d99c435f71 --- /dev/null +++ b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/AssertionsShouldBeComplete.NFluent.cs @@ -0,0 +1,11 @@ +using NFluent; + +public class Program +{ + public void Invocations() + { + Check.That(0); // Noncompliant {{Complete the assertion}} + // ^^^^ + Check.That(); // Noncompliant + } +} From d2751723a577c1f032a9607daedcdbab91385085 Mon Sep 17 00:00:00 2001 From: Martin Strecker Date: Tue, 21 Feb 2023 17:29:19 +0100 Subject: [PATCH 17/35] Add tests for NFluent --- .../Rules/AssertionsShouldBeComplete.cs | 2 +- .../Rules/AssertionsShouldBeCompleteTest.cs | 8 +++ ...rtionsShouldBeComplete.NFluent.CSharp11.cs | 16 +++++ .../AssertionsShouldBeComplete.NFluent.cs | 58 +++++++++++++++++-- 4 files changed, 79 insertions(+), 5 deletions(-) create mode 100644 analyzers/tests/SonarAnalyzer.UnitTest/TestCases/AssertionsShouldBeComplete.NFluent.CSharp11.cs diff --git a/analyzers/src/SonarAnalyzer.CSharp/Rules/AssertionsShouldBeComplete.cs b/analyzers/src/SonarAnalyzer.CSharp/Rules/AssertionsShouldBeComplete.cs index 02203182ddc..1db494e9759 100644 --- a/analyzers/src/SonarAnalyzer.CSharp/Rules/AssertionsShouldBeComplete.cs +++ b/analyzers/src/SonarAnalyzer.CSharp/Rules/AssertionsShouldBeComplete.cs @@ -61,7 +61,7 @@ public sealed class AssertionsShouldBeComplete : SonarDiagnosticAnalyzer { start.RegisterNodeAction(c => CheckInvocation(c, invocation => - invocation.NameIs("That", "ThatCode", "ThatAsyncCode", "ThatDynamic") + invocation.NameIs("That", "ThatEnum", "ThatCode", "ThatAsyncCode", "ThatDynamic") && c.SemanticModel.GetSymbolInfo(invocation) is { Symbol: IMethodSymbol { IsStatic: true } method } && method.ContainingType?.IsStatic is true && method.ContainingType.Is(KnownType.NFluent_Check)), SyntaxKind.InvocationExpression); diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/Rules/AssertionsShouldBeCompleteTest.cs b/analyzers/tests/SonarAnalyzer.UnitTest/Rules/AssertionsShouldBeCompleteTest.cs index 295b04ebe31..4daffd06a2d 100644 --- a/analyzers/tests/SonarAnalyzer.UnitTest/Rules/AssertionsShouldBeCompleteTest.cs +++ b/analyzers/tests/SonarAnalyzer.UnitTest/Rules/AssertionsShouldBeCompleteTest.cs @@ -57,4 +57,12 @@ public class AssertionsShouldBeCompleteTest .AddTestReference() .AddPaths("AssertionsShouldBeComplete.NFluent.cs") .Verify(); + + [TestMethod] + public void AssertionsShouldBeComplete_NFluent_CSharp11() => + nfluent + .WithOptions(ParseOptionsHelper.FromCSharp11) + .AddTestReference() + .AddPaths("AssertionsShouldBeComplete.NFluent.CSharp11.cs") + .Verify(); } diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/AssertionsShouldBeComplete.NFluent.CSharp11.cs b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/AssertionsShouldBeComplete.NFluent.CSharp11.cs new file mode 100644 index 00000000000..6ffb58c0166 --- /dev/null +++ b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/AssertionsShouldBeComplete.NFluent.CSharp11.cs @@ -0,0 +1,16 @@ +using System; +using System.Threading.Tasks; +using NFluent; +using static NFluent.Check; + +public class Program +{ + public void CheckThat() + { + That(0); // Noncompliant {{Complete the assertion}} + // ^^^^ + That(); // Noncompliant + + That(1).IsEqualTo(1); + } +} diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/AssertionsShouldBeComplete.NFluent.cs b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/AssertionsShouldBeComplete.NFluent.cs index 4d99c435f71..e5aef5cc084 100644 --- a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/AssertionsShouldBeComplete.NFluent.cs +++ b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/AssertionsShouldBeComplete.NFluent.cs @@ -1,11 +1,61 @@ -using NFluent; +using System; +using System.Threading.Tasks; +using NFluent; public class Program { - public void Invocations() + public void CheckThat() { - Check.That(0); // Noncompliant {{Complete the assertion}} + Check.That(0); // Noncompliant {{Complete the assertion}} // ^^^^ - Check.That(); // Noncompliant + Check.That(); // Noncompliant + + Check.That(1).IsEqualTo(1); + } + + public void CheckThatEnum() + { + Check.ThatEnum(AttributeTargets.All); // Noncompliant {{Complete the assertion}} + // ^^^^^^^^ + Check.ThatEnum(AttributeTargets.All); // Noncompliant + + Check.ThatEnum(AttributeTargets.All).IsEqualTo(AttributeTargets.All); } + + public void CheckThatCode() + { + Check.ThatCode(() => { }); // Noncompliant {{Complete the assertion}} + // ^^^^^^^^ + Check.ThatCode(() => 1); // Noncompliant + Check.ThatCode(CheckThatCode); // Noncompliant + + Check.ThatCode(() => { }).DoesNotThrow(); + } + + public async Task CheckThatCodeAsync() + { + Check.ThatAsyncCode(async () => await Task.CompletedTask); // Noncompliant {{Complete the assertion}} + // ^^^^^^^^^^^^^ + Check.ThatAsyncCode(async () => await Task.FromResult(1)); // Noncompliant + Check.ThatAsyncCode(CheckThatCodeAsync); // Noncompliant + + Check.ThatAsyncCode(async () => await Task.CompletedTask).DoesNotThrow(); + } + + public async Task CheckThatCodeDynamic(dynamic expando) + { + Check.ThatDynamic(1); // Noncompliant {{Complete the assertion}} + // ^^^^^^^^^^^ + Check.ThatDynamic(expando); // Noncompliant + + Check.ThatDynamic(1).IsNotNull(); + } + + public ICheck CheckReturnedByReturn() + { + return Check.That(1); + } + + public ICheck CheckReturnedByExpressionBody() => + Check.That(1); } From 53b00f62eaea41c7b08c63b19427c81f9dfc11e0 Mon Sep 17 00:00:00 2001 From: Martin Strecker Date: Tue, 21 Feb 2023 17:39:22 +0100 Subject: [PATCH 18/35] Add test for other compliant Check class --- .../AssertionsShouldBeComplete.NFluent.cs | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/AssertionsShouldBeComplete.NFluent.cs b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/AssertionsShouldBeComplete.NFluent.cs index e5aef5cc084..47b3e25da90 100644 --- a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/AssertionsShouldBeComplete.NFluent.cs +++ b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/AssertionsShouldBeComplete.NFluent.cs @@ -59,3 +59,19 @@ public ICheck CheckReturnedByReturn() public ICheck CheckReturnedByExpressionBody() => Check.That(1); } + +namespace OtherCheck +{ + public static class Check + { + public static void That(int i) { } + } + + public class Test + { + public void CheckThat() + { + Check.That(1); // Compliant. Not NFluent + } + } +} From a5bd203b2ad2910d845b138d9229b018cfdf2698 Mon Sep 17 00:00:00 2001 From: Martin Strecker Date: Tue, 21 Feb 2023 17:41:57 +0100 Subject: [PATCH 19/35] formatting --- .../SonarAnalyzer.CSharp/Rules/AssertionsShouldBeComplete.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/analyzers/src/SonarAnalyzer.CSharp/Rules/AssertionsShouldBeComplete.cs b/analyzers/src/SonarAnalyzer.CSharp/Rules/AssertionsShouldBeComplete.cs index 1db494e9759..62d93235087 100644 --- a/analyzers/src/SonarAnalyzer.CSharp/Rules/AssertionsShouldBeComplete.cs +++ b/analyzers/src/SonarAnalyzer.CSharp/Rules/AssertionsShouldBeComplete.cs @@ -64,7 +64,8 @@ public sealed class AssertionsShouldBeComplete : SonarDiagnosticAnalyzer invocation.NameIs("That", "ThatEnum", "ThatCode", "ThatAsyncCode", "ThatDynamic") && c.SemanticModel.GetSymbolInfo(invocation) is { Symbol: IMethodSymbol { IsStatic: true } method } && method.ContainingType?.IsStatic is true - && method.ContainingType.Is(KnownType.NFluent_Check)), SyntaxKind.InvocationExpression); + && method.ContainingType.Is(KnownType.NFluent_Check)), + SyntaxKind.InvocationExpression); } if (start.Compilation.References(KnownAssembly.NSubstitute)) { From 1baf1fffb75d9a5fa6e1502b6fc61f680cdcc559 Mon Sep 17 00:00:00 2001 From: Martin Strecker Date: Tue, 21 Feb 2023 17:42:11 +0100 Subject: [PATCH 20/35] formatting --- .../SonarAnalyzer.CSharp/Rules/AssertionsShouldBeComplete.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/analyzers/src/SonarAnalyzer.CSharp/Rules/AssertionsShouldBeComplete.cs b/analyzers/src/SonarAnalyzer.CSharp/Rules/AssertionsShouldBeComplete.cs index 62d93235087..9a3d7da528a 100644 --- a/analyzers/src/SonarAnalyzer.CSharp/Rules/AssertionsShouldBeComplete.cs +++ b/analyzers/src/SonarAnalyzer.CSharp/Rules/AssertionsShouldBeComplete.cs @@ -74,7 +74,8 @@ public sealed class AssertionsShouldBeComplete : SonarDiagnosticAnalyzer invocation.NameIs("DidNotReceive", "DidNotReceiveWithAnyArgs", "Received", "ReceivedCalls", "ReceivedWithAnyArgs") && c.SemanticModel.GetSymbolInfo(invocation) is { Symbol: IMethodSymbol { IsStatic: true } method } && method.ContainingType?.IsStatic is true - && method.ContainingType.Is(KnownType.NSubstitute_SubstituteExtensions)), SyntaxKind.InvocationExpression); + && method.ContainingType.Is(KnownType.NSubstitute_SubstituteExtensions)), + SyntaxKind.InvocationExpression); } }); From c399c2f6c92b835c062f79d4f7abe1acbaeb8716 Mon Sep 17 00:00:00 2001 From: Martin Strecker Date: Wed, 22 Feb 2023 09:15:22 +0100 Subject: [PATCH 21/35] Add tests for NSubstitute --- .../Rules/AssertionsShouldBeComplete.cs | 16 +--- .../Rules/AssertionsShouldBeCompleteTest.cs | 9 ++ .../AssertionsShouldBeComplete.NSubstitute.cs | 82 +++++++++++++++++++ 3 files changed, 93 insertions(+), 14 deletions(-) create mode 100644 analyzers/tests/SonarAnalyzer.UnitTest/TestCases/AssertionsShouldBeComplete.NSubstitute.cs diff --git a/analyzers/src/SonarAnalyzer.CSharp/Rules/AssertionsShouldBeComplete.cs b/analyzers/src/SonarAnalyzer.CSharp/Rules/AssertionsShouldBeComplete.cs index 9a3d7da528a..8e13c02ae22 100644 --- a/analyzers/src/SonarAnalyzer.CSharp/Rules/AssertionsShouldBeComplete.cs +++ b/analyzers/src/SonarAnalyzer.CSharp/Rules/AssertionsShouldBeComplete.cs @@ -33,18 +33,6 @@ public sealed class AssertionsShouldBeComplete : SonarDiagnosticAnalyzer protected override void Initialize(SonarAnalysisContext context) => context.RegisterCompilationStartAction(start => { - if (start.Compilation.References(KnownAssembly.MSTest) - // Assert.That was introduced in Version 1.1.14 but the AssemblyIdentity version (14.0.0.0) does not align with the Nuget version so we need - // to check at runtime for the presence of "Assert.That" - && start.Compilation.GetTypeByMetadataName(KnownType.Microsoft_VisualStudio_TestTools_UnitTesting_Assert) is { } assertType - && assertType.GetMembers("That") is { Length: 1 } thatMembers - && thatMembers[0] is IPropertySymbol { IsStatic: true }) - { - start.RegisterNodeAction(c => - { - - }, SyntaxKind.SimpleMemberAccessExpression); - } if (start.Compilation.References(KnownAssembly.FluentAssertions)) { start.RegisterNodeAction(c => @@ -71,8 +59,8 @@ public sealed class AssertionsShouldBeComplete : SonarDiagnosticAnalyzer { start.RegisterNodeAction(c => CheckInvocation(c, invocation => - invocation.NameIs("DidNotReceive", "DidNotReceiveWithAnyArgs", "Received", "ReceivedCalls", "ReceivedWithAnyArgs") - && c.SemanticModel.GetSymbolInfo(invocation) is { Symbol: IMethodSymbol { IsStatic: true } method } + invocation.NameIs("Received", "DidNotReceive", "ReceivedWithAnyArgs", "DidNotReceiveWithAnyArgs", "ReceivedCalls") + && c.SemanticModel.GetSymbolInfo(invocation) is { Symbol: IMethodSymbol { IsExtensionMethod: true } method } && method.ContainingType?.IsStatic is true && method.ContainingType.Is(KnownType.NSubstitute_SubstituteExtensions)), SyntaxKind.InvocationExpression); diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/Rules/AssertionsShouldBeCompleteTest.cs b/analyzers/tests/SonarAnalyzer.UnitTest/Rules/AssertionsShouldBeCompleteTest.cs index 4daffd06a2d..69e6fe0b5ec 100644 --- a/analyzers/tests/SonarAnalyzer.UnitTest/Rules/AssertionsShouldBeCompleteTest.cs +++ b/analyzers/tests/SonarAnalyzer.UnitTest/Rules/AssertionsShouldBeCompleteTest.cs @@ -36,6 +36,9 @@ public class AssertionsShouldBeCompleteTest private readonly VerifierBuilder nfluent = new VerifierBuilder() .AddReferences(NuGetMetadataReference.NFluent("2.8.0")); + private readonly VerifierBuilder nsubstitute = new VerifierBuilder() + .AddReferences(NuGetMetadataReference.NSubstitute("5.0.0")); + [TestMethod] public void AssertionsShouldBeComplete_FluentAssertions_CSharp7() => fluentAssertions @@ -65,4 +68,10 @@ public class AssertionsShouldBeCompleteTest .AddTestReference() .AddPaths("AssertionsShouldBeComplete.NFluent.CSharp11.cs") .Verify(); + + [TestMethod] + public void AssertionsShouldBeComplete_NSubstitute_CS() => + nsubstitute + .AddPaths("AssertionsShouldBeComplete.NSubstitute.cs") + .Verify(); } diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/AssertionsShouldBeComplete.NSubstitute.cs b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/AssertionsShouldBeComplete.NSubstitute.cs new file mode 100644 index 00000000000..f9a4893c404 --- /dev/null +++ b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/AssertionsShouldBeComplete.NSubstitute.cs @@ -0,0 +1,82 @@ +using NSubstitute; +using System.Net; + +public interface ICommand +{ + void Execute(); +} + +namespace NSubstituteTests +{ + internal class Tests + { + public void Received() + { + var command = Substitute.For(); + command.Received(); // Noncompliant {{Complete the assertion}} + // ^^^^^^^^ + command.Received(requiredNumberOfCalls: 2); // Noncompliant + command.Received(); // Noncompliant + SubstituteExtensions.Received(command); // Noncompliant + + command.Received().Execute(); + } + + public void DidNotReceive() + { + var command = Substitute.For(); + command.DidNotReceive(); // Noncompliant {{Complete the assertion}} + // ^^^^^^^^^^^^^ + command.DidNotReceive(); // Noncompliant + + command.DidNotReceive().Execute(); + } + + public void ReceivedWithAnyArgs() + { + var command = Substitute.For(); + command.ReceivedWithAnyArgs(); // Noncompliant {{Complete the assertion}} + // ^^^^^^^^^^^^^^^^^^^ + command.ReceivedWithAnyArgs(requiredNumberOfCalls: 2); // Noncompliant + command.ReceivedWithAnyArgs(); // Noncompliant + + command.ReceivedWithAnyArgs().Execute(); + } + + public void DidNotReceiveWithAnyArgs() + { + var command = Substitute.For(); + command.DidNotReceiveWithAnyArgs(); // Noncompliant {{Complete the assertion}} + // ^^^^^^^^^^^^^^^^^^^^^^^^ + command.DidNotReceiveWithAnyArgs(); // Noncompliant + + command.DidNotReceiveWithAnyArgs().Execute(); + } + + public void ReceivedCalls() + { + var command = Substitute.For(); + command.ReceivedCalls(); // Noncompliant {{Complete the assertion}} + // ^^^^^^^^^^^^^ + command.ReceivedCalls(); // Noncompliant + + var calls = command.ReceivedCalls(); + } + } +} + +namespace OtherReceived +{ + public static class OtherExtensions + { + public static void Received(this T something) { } + } + + public class Test + { + public void Received(ICommand command) + { + command.Received(); // Compliant. Other Received call + } + } +} From 76fb2ad8e350f8751acecda9f32a52e3298ccab4 Mon Sep 17 00:00:00 2001 From: Martin Strecker Date: Wed, 22 Feb 2023 09:32:00 +0100 Subject: [PATCH 22/35] Refactorings --- .../Rules/AssertionsShouldBeComplete.cs | 39 ++++++++++++++----- 1 file changed, 29 insertions(+), 10 deletions(-) diff --git a/analyzers/src/SonarAnalyzer.CSharp/Rules/AssertionsShouldBeComplete.cs b/analyzers/src/SonarAnalyzer.CSharp/Rules/AssertionsShouldBeComplete.cs index 8e13c02ae22..16af5a90e75 100644 --- a/analyzers/src/SonarAnalyzer.CSharp/Rules/AssertionsShouldBeComplete.cs +++ b/analyzers/src/SonarAnalyzer.CSharp/Rules/AssertionsShouldBeComplete.cs @@ -38,11 +38,16 @@ public sealed class AssertionsShouldBeComplete : SonarDiagnosticAnalyzer start.RegisterNodeAction(c => CheckInvocation(c, invocation => invocation.NameIs("Should") - && c.SemanticModel.GetSymbolInfo(invocation).AllSymbols().Any(x => - x is IMethodSymbol { IsExtensionMethod: true } method - && (method.ContainingType.Is(KnownType.FluentAssertions_AssertionExtensions) + && 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). - || method.ReturnType.DerivesFrom(KnownType.FluentAssertions_Primitives_ReferenceTypeAssertions)))), + || returnType.DerivesFrom(KnownType.FluentAssertions_Primitives_ReferenceTypeAssertions)))), SyntaxKind.InvocationExpression); } if (start.Compilation.References(KnownAssembly.NFluent)) @@ -50,9 +55,16 @@ public sealed class AssertionsShouldBeComplete : SonarDiagnosticAnalyzer start.RegisterNodeAction(c => CheckInvocation(c, invocation => invocation.NameIs("That", "ThatEnum", "ThatCode", "ThatAsyncCode", "ThatDynamic") - && c.SemanticModel.GetSymbolInfo(invocation) is { Symbol: IMethodSymbol { IsStatic: true } method } - && method.ContainingType?.IsStatic is true - && method.ContainingType.Is(KnownType.NFluent_Check)), + && 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)) @@ -60,9 +72,16 @@ public sealed class AssertionsShouldBeComplete : SonarDiagnosticAnalyzer start.RegisterNodeAction(c => CheckInvocation(c, invocation => invocation.NameIs("Received", "DidNotReceive", "ReceivedWithAnyArgs", "DidNotReceiveWithAnyArgs", "ReceivedCalls") - && c.SemanticModel.GetSymbolInfo(invocation) is { Symbol: IMethodSymbol { IsExtensionMethod: true } method } - && method.ContainingType?.IsStatic is true - && method.ContainingType.Is(KnownType.NSubstitute_SubstituteExtensions)), + && c.SemanticModel.GetSymbolInfo(invocation) is + { + Symbol: IMethodSymbol + { + IsExtensionMethod: true, + ReturnsVoid: false, + ContainingType: { } container, + } + } + && container.Is(KnownType.NSubstitute_SubstituteExtensions)), SyntaxKind.InvocationExpression); } }); From 69d142cbbb18497551d3f7d31b701b8434708df5 Mon Sep 17 00:00:00 2001 From: Martin Strecker Date: Wed, 22 Feb 2023 11:03:23 +0100 Subject: [PATCH 23/35] Add tests for lambdas and locla functions --- .../Rules/AssertionsShouldBeComplete.cs | 8 ++++++-- ...ionsShouldBeComplete.FluentAssertions.CSharp7.cs | 5 +++++ ...ionsShouldBeComplete.FluentAssertions.CSharp8.cs | 13 +++++++++++++ 3 files changed, 24 insertions(+), 2 deletions(-) diff --git a/analyzers/src/SonarAnalyzer.CSharp/Rules/AssertionsShouldBeComplete.cs b/analyzers/src/SonarAnalyzer.CSharp/Rules/AssertionsShouldBeComplete.cs index 16af5a90e75..58357705210 100644 --- a/analyzers/src/SonarAnalyzer.CSharp/Rules/AssertionsShouldBeComplete.cs +++ b/analyzers/src/SonarAnalyzer.CSharp/Rules/AssertionsShouldBeComplete.cs @@ -112,8 +112,12 @@ private static bool HasContinuation(InvocationExpressionSyntax invocation) } // We are in some kind of statement context "??? Should();" // The result might be stored in a variable or returned from the method - if (invocation.Ancestors().TakeWhile(x => !(x.IsAnyKind(SyntaxKind.Block, SyntaxKindEx.LocalFunctionStatement) || x is BaseMethodDeclarationSyntax or AnonymousFunctionExpressionSyntax)). - Any(x => x.IsAnyKind(SyntaxKind.ReturnStatement, SyntaxKind.ArrowExpressionClause) || x is AssignmentExpressionSyntax or LocalDeclarationStatementSyntax)) + if (nextToken.Parent is + BaseMethodDeclarationSyntax + or ReturnStatementSyntax + or LocalDeclarationStatementSyntax + or ExpressionStatementSyntax { Expression: AssignmentExpressionSyntax } + || LocalFunctionStatementSyntaxWrapper.IsInstance(nextToken.Parent)) { return true; } diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/AssertionsShouldBeComplete.FluentAssertions.CSharp7.cs b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/AssertionsShouldBeComplete.FluentAssertions.CSharp7.cs index 11de1223e63..62d8f425ade 100644 --- a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/AssertionsShouldBeComplete.FluentAssertions.CSharp7.cs +++ b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/AssertionsShouldBeComplete.FluentAssertions.CSharp7.cs @@ -59,6 +59,11 @@ public void UnreducedCall() FluentAssertions.AssertionExtensions.Should(s); // Noncompliant } + public void ReturnedInLambda() + { + Func a = () => "Test".Should(); + } + public void CustomAssertions() { var custom = new Custom(); diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/AssertionsShouldBeComplete.FluentAssertions.CSharp8.cs b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/AssertionsShouldBeComplete.FluentAssertions.CSharp8.cs index dad424a6a48..c56bb335637 100644 --- a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/AssertionsShouldBeComplete.FluentAssertions.CSharp8.cs +++ b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/AssertionsShouldBeComplete.FluentAssertions.CSharp8.cs @@ -35,4 +35,17 @@ public void DictAssertions() dict?["A"]!.Should(); // Noncompliant // ^^^^^^ } + + public void LocalFunction() + { + var s = "Test"; + + StringAssertions ExpressionBodyLocalFunction() => + s.Should(); + + StringAssertions ReturnLocalFunction() + { + return s.Should(); + } + } } From d960499d3e658a312cfaced47ad7374f6a04d07a Mon Sep 17 00:00:00 2001 From: Martin Strecker Date: Wed, 22 Feb 2023 11:22:10 +0100 Subject: [PATCH 24/35] Add more fine grained tests for valid continuations and fix some bugs. --- .../Rules/AssertionsShouldBeComplete.cs | 6 ++++-- ...sShouldBeComplete.FluentAssertions.CSharp7.cs | 6 ++++++ ...sShouldBeComplete.FluentAssertions.CSharp8.cs | 16 ++++++++++++++++ 3 files changed, 26 insertions(+), 2 deletions(-) diff --git a/analyzers/src/SonarAnalyzer.CSharp/Rules/AssertionsShouldBeComplete.cs b/analyzers/src/SonarAnalyzer.CSharp/Rules/AssertionsShouldBeComplete.cs index 58357705210..6e73c5ec433 100644 --- a/analyzers/src/SonarAnalyzer.CSharp/Rules/AssertionsShouldBeComplete.cs +++ b/analyzers/src/SonarAnalyzer.CSharp/Rules/AssertionsShouldBeComplete.cs @@ -111,9 +111,11 @@ private static bool HasContinuation(InvocationExpressionSyntax invocation) return true; } // We are in some kind of statement context "??? Should();" - // The result might be stored in a variable or returned from the method + // The result might be stored in a variable or returned from the method/property if (nextToken.Parent is - BaseMethodDeclarationSyntax + MethodDeclarationSyntax { ReturnType: not PredefinedTypeSyntax { Keyword.RawKind: (int)SyntaxKind.VoidKeyword } } + or PropertyDeclarationSyntax + or AccessorDeclarationSyntax { Keyword.RawKind: (int)SyntaxKind.GetKeyword } or ReturnStatementSyntax or LocalDeclarationStatementSyntax or ExpressionStatementSyntax { Expression: AssignmentExpressionSyntax } diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/AssertionsShouldBeComplete.FluentAssertions.CSharp7.cs b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/AssertionsShouldBeComplete.FluentAssertions.CSharp7.cs index 62d8f425ade..fec41567151 100644 --- a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/AssertionsShouldBeComplete.FluentAssertions.CSharp7.cs +++ b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/AssertionsShouldBeComplete.FluentAssertions.CSharp7.cs @@ -39,6 +39,12 @@ public StringAssertions ReturnedByReturn() public StringAssertions ReturnedByArrow(string s) => s.Should(); // Compliant + public object ReturnedByArrowWithConversion(string s) => + (object)s.Should(); // Compliant + + public void CalledByArrow(string s) => + s.Should(); // Noncompliant + public void Assigned() { var s = "Test"; diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/AssertionsShouldBeComplete.FluentAssertions.CSharp8.cs b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/AssertionsShouldBeComplete.FluentAssertions.CSharp8.cs index c56bb335637..bc68ad51c5d 100644 --- a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/AssertionsShouldBeComplete.FluentAssertions.CSharp8.cs +++ b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/AssertionsShouldBeComplete.FluentAssertions.CSharp8.cs @@ -48,4 +48,20 @@ StringAssertions ReturnLocalFunction() return s.Should(); } } + + public StringAssertions PropertyArrow => "Test".Should(); + + public StringAssertions PropertyGetterArrow + { + get => "Test".Should(); + set => "Test".Should(); // Noncompliant + } + + public StringAssertions PropertyGetterReturn + { + get + { + return "Test".Should(); + } + } } From d67b3ab51ea195947057d9243b86a9b86473a1f4 Mon Sep 17 00:00:00 2001 From: Martin Strecker Date: Wed, 22 Feb 2023 11:42:32 +0100 Subject: [PATCH 25/35] Fix code smells --- .../Rules/AssertionsShouldBeComplete.cs | 2 +- .../Extensions/SymbolInfoExtensions.cs | 1 - .../Helpers/KnownAssembly.cs | 3 +- ...ssertionsShouldBeComplete.AllFrameworks.cs | 82 +++++++++++++++++++ 4 files changed, 85 insertions(+), 3 deletions(-) create mode 100644 analyzers/tests/SonarAnalyzer.UnitTest/TestCases/AssertionsShouldBeComplete.AllFrameworks.cs diff --git a/analyzers/src/SonarAnalyzer.CSharp/Rules/AssertionsShouldBeComplete.cs b/analyzers/src/SonarAnalyzer.CSharp/Rules/AssertionsShouldBeComplete.cs index 6e73c5ec433..bbc6d19ea29 100644 --- a/analyzers/src/SonarAnalyzer.CSharp/Rules/AssertionsShouldBeComplete.cs +++ b/analyzers/src/SonarAnalyzer.CSharp/Rules/AssertionsShouldBeComplete.cs @@ -86,7 +86,7 @@ public sealed class AssertionsShouldBeComplete : SonarDiagnosticAnalyzer } }); - private void CheckInvocation(SonarSyntaxNodeReportingContext c, Func isAssertionMethod) + private static void CheckInvocation(SonarSyntaxNodeReportingContext c, Func isAssertionMethod) { if (c.Node is InvocationExpressionSyntax invocation && isAssertionMethod(invocation) diff --git a/analyzers/src/SonarAnalyzer.Common/Extensions/SymbolInfoExtensions.cs b/analyzers/src/SonarAnalyzer.Common/Extensions/SymbolInfoExtensions.cs index a4bc0116da3..f667d11e8ec 100644 --- a/analyzers/src/SonarAnalyzer.Common/Extensions/SymbolInfoExtensions.cs +++ b/analyzers/src/SonarAnalyzer.Common/Extensions/SymbolInfoExtensions.cs @@ -18,7 +18,6 @@ * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. */ - namespace SonarAnalyzer.Extensions { public static class SymbolInfoExtensions diff --git a/analyzers/src/SonarAnalyzer.Common/Helpers/KnownAssembly.cs b/analyzers/src/SonarAnalyzer.Common/Helpers/KnownAssembly.cs index 5b73c8cc352..9fdea5cd0d3 100644 --- a/analyzers/src/SonarAnalyzer.Common/Helpers/KnownAssembly.cs +++ b/analyzers/src/SonarAnalyzer.Common/Helpers/KnownAssembly.cs @@ -31,7 +31,8 @@ public sealed partial class KnownAssembly PublicKeyTokenIs("8d05b1bb7a6fdb6c"))); /// - /// Any MSTest framework either referenced via nuget.org/MicrosoftVisualStudioQualityToolsUnitTestFramework (MSTest V1) + /// Any MSTest framework either referenced via + /// nuget.org/MicrosoftVisualStudioQualityToolsUnitTestFramework (MSTest V1) /// or nuget.org/MSTest.TestFramework (MSTest V2). /// public static KnownAssembly MSTest { get; } = new( diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/AssertionsShouldBeComplete.AllFrameworks.cs b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/AssertionsShouldBeComplete.AllFrameworks.cs new file mode 100644 index 00000000000..f9a4893c404 --- /dev/null +++ b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/AssertionsShouldBeComplete.AllFrameworks.cs @@ -0,0 +1,82 @@ +using NSubstitute; +using System.Net; + +public interface ICommand +{ + void Execute(); +} + +namespace NSubstituteTests +{ + internal class Tests + { + public void Received() + { + var command = Substitute.For(); + command.Received(); // Noncompliant {{Complete the assertion}} + // ^^^^^^^^ + command.Received(requiredNumberOfCalls: 2); // Noncompliant + command.Received(); // Noncompliant + SubstituteExtensions.Received(command); // Noncompliant + + command.Received().Execute(); + } + + public void DidNotReceive() + { + var command = Substitute.For(); + command.DidNotReceive(); // Noncompliant {{Complete the assertion}} + // ^^^^^^^^^^^^^ + command.DidNotReceive(); // Noncompliant + + command.DidNotReceive().Execute(); + } + + public void ReceivedWithAnyArgs() + { + var command = Substitute.For(); + command.ReceivedWithAnyArgs(); // Noncompliant {{Complete the assertion}} + // ^^^^^^^^^^^^^^^^^^^ + command.ReceivedWithAnyArgs(requiredNumberOfCalls: 2); // Noncompliant + command.ReceivedWithAnyArgs(); // Noncompliant + + command.ReceivedWithAnyArgs().Execute(); + } + + public void DidNotReceiveWithAnyArgs() + { + var command = Substitute.For(); + command.DidNotReceiveWithAnyArgs(); // Noncompliant {{Complete the assertion}} + // ^^^^^^^^^^^^^^^^^^^^^^^^ + command.DidNotReceiveWithAnyArgs(); // Noncompliant + + command.DidNotReceiveWithAnyArgs().Execute(); + } + + public void ReceivedCalls() + { + var command = Substitute.For(); + command.ReceivedCalls(); // Noncompliant {{Complete the assertion}} + // ^^^^^^^^^^^^^ + command.ReceivedCalls(); // Noncompliant + + var calls = command.ReceivedCalls(); + } + } +} + +namespace OtherReceived +{ + public static class OtherExtensions + { + public static void Received(this T something) { } + } + + public class Test + { + public void Received(ICommand command) + { + command.Received(); // Compliant. Other Received call + } + } +} From 3b99c79b92c4a7301b9bb5596a464ebcb5054ba5 Mon Sep 17 00:00:00 2001 From: Martin Strecker Date: Wed, 22 Feb 2023 12:01:47 +0100 Subject: [PATCH 26/35] Refactor HasContinuation --- .../Rules/AssertionsShouldBeComplete.cs | 25 +++++++++++-------- ...ouldBeComplete.FluentAssertions.CSharp8.cs | 3 +++ 2 files changed, 17 insertions(+), 11 deletions(-) diff --git a/analyzers/src/SonarAnalyzer.CSharp/Rules/AssertionsShouldBeComplete.cs b/analyzers/src/SonarAnalyzer.CSharp/Rules/AssertionsShouldBeComplete.cs index bbc6d19ea29..68326d56de6 100644 --- a/analyzers/src/SonarAnalyzer.CSharp/Rules/AssertionsShouldBeComplete.cs +++ b/analyzers/src/SonarAnalyzer.CSharp/Rules/AssertionsShouldBeComplete.cs @@ -112,17 +112,20 @@ private static bool HasContinuation(InvocationExpressionSyntax invocation) } // We are in some kind of statement context "??? Should();" // The result might be stored in a variable or returned from the method/property - if (nextToken.Parent is - MethodDeclarationSyntax { ReturnType: not PredefinedTypeSyntax { Keyword.RawKind: (int)SyntaxKind.VoidKeyword } } - or PropertyDeclarationSyntax - or AccessorDeclarationSyntax { Keyword.RawKind: (int)SyntaxKind.GetKeyword } - or ReturnStatementSyntax - or LocalDeclarationStatementSyntax - or ExpressionStatementSyntax { Expression: AssignmentExpressionSyntax } - || LocalFunctionStatementSyntaxWrapper.IsInstance(nextToken.Parent)) + return nextToken.Parent switch { - return true; - } - return false; + + MethodDeclarationSyntax { ReturnType: { } returnType } when !IsVoid(returnType) => true, + { } parent when LocalFunctionStatementSyntaxWrapper.IsInstance(parent) && !IsVoid(((LocalFunctionStatementSyntaxWrapper)parent).ReturnType) => true, + 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 }; } diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/AssertionsShouldBeComplete.FluentAssertions.CSharp8.cs b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/AssertionsShouldBeComplete.FluentAssertions.CSharp8.cs index bc68ad51c5d..fda6f1dafab 100644 --- a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/AssertionsShouldBeComplete.FluentAssertions.CSharp8.cs +++ b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/AssertionsShouldBeComplete.FluentAssertions.CSharp8.cs @@ -43,6 +43,9 @@ public void LocalFunction() StringAssertions ExpressionBodyLocalFunction() => s.Should(); + void VoidReturningExpressionBodyLocalFunction() => + s.Should(); // Noncompliant + StringAssertions ReturnLocalFunction() { return s.Should(); From 5028c0497e8d2269c45177d8e235bd4c3950b725 Mon Sep 17 00:00:00 2001 From: Martin Strecker Date: Wed, 22 Feb 2023 14:15:01 +0100 Subject: [PATCH 27/35] Move IsVoid in the switch arm expression --- .../SonarAnalyzer.CSharp/Rules/AssertionsShouldBeComplete.cs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/analyzers/src/SonarAnalyzer.CSharp/Rules/AssertionsShouldBeComplete.cs b/analyzers/src/SonarAnalyzer.CSharp/Rules/AssertionsShouldBeComplete.cs index 68326d56de6..1ee2f13cf35 100644 --- a/analyzers/src/SonarAnalyzer.CSharp/Rules/AssertionsShouldBeComplete.cs +++ b/analyzers/src/SonarAnalyzer.CSharp/Rules/AssertionsShouldBeComplete.cs @@ -114,9 +114,8 @@ private static bool HasContinuation(InvocationExpressionSyntax invocation) // The result might be stored in a variable or returned from the method/property return nextToken.Parent switch { - - MethodDeclarationSyntax { ReturnType: { } returnType } when !IsVoid(returnType) => true, - { } parent when LocalFunctionStatementSyntaxWrapper.IsInstance(parent) && !IsVoid(((LocalFunctionStatementSyntaxWrapper)parent).ReturnType) => true, + 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, From 5f1eeaeae991fd0c4311ab138986d937ea8921c6 Mon Sep 17 00:00:00 2001 From: Martin Strecker Date: Wed, 22 Feb 2023 14:34:44 +0100 Subject: [PATCH 28/35] Add test for missing close paran --- .../Rules/AssertionsShouldBeComplete.cs | 14 +++++++------- .../Rules/AssertionsShouldBeCompleteTest.cs | 16 ++++++++++++++++ ...sShouldBeComplete.FluentAssertions.CSharp7.cs | 2 ++ 3 files changed, 25 insertions(+), 7 deletions(-) diff --git a/analyzers/src/SonarAnalyzer.CSharp/Rules/AssertionsShouldBeComplete.cs b/analyzers/src/SonarAnalyzer.CSharp/Rules/AssertionsShouldBeComplete.cs index 1ee2f13cf35..1d664d3b15e 100644 --- a/analyzers/src/SonarAnalyzer.CSharp/Rules/AssertionsShouldBeComplete.cs +++ b/analyzers/src/SonarAnalyzer.CSharp/Rules/AssertionsShouldBeComplete.cs @@ -39,12 +39,12 @@ public sealed class AssertionsShouldBeComplete : SonarDiagnosticAnalyzer CheckInvocation(c, invocation => invocation.NameIs("Should") && c.SemanticModel.GetSymbolInfo(invocation).AllSymbols().Any(x => x is IMethodSymbol - { - IsExtensionMethod: true, - ReturnsVoid: false, - ContainingType: { } container, - ReturnType: { } returnType, - } + { + 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)))), @@ -99,7 +99,7 @@ private static void CheckInvocation(SonarSyntaxNodeReportingContext c, Func + fluentAssertions + .AddSnippet(""" + using FluentAssertions; + public class Test + { + public void MissingParen() + { + var s = "Test"; + s.Should(; // Error + } + } + """) + .Verify(); + [TestMethod] public void AssertionsShouldBeComplete_FluentAssertions_CSharp8() => fluentAssertions diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/AssertionsShouldBeComplete.FluentAssertions.CSharp7.cs b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/AssertionsShouldBeComplete.FluentAssertions.CSharp7.cs index fec41567151..2d1c422f4a9 100644 --- a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/AssertionsShouldBeComplete.FluentAssertions.CSharp7.cs +++ b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/AssertionsShouldBeComplete.FluentAssertions.CSharp7.cs @@ -12,6 +12,7 @@ public void StringAssertions() //^^^^^^ s[0].Should(); // Error [CS0121] ambiguous calls // Noncompliant@-1 + s.Should().Be("Test"); // Compliant } @@ -21,6 +22,7 @@ public void CollectionAssertions() collection.Should(); // Error [CS0121] ambiguous calls // Noncompliant@-1 collection.Should(); // Noncompliant + collection.Should().Equal("Test", "Test"); // Compliant } From 490e1fe796468d05f84ae734b8eb3b9134cc10ea Mon Sep 17 00:00:00 2001 From: Martin Strecker Date: Wed, 22 Feb 2023 14:55:22 +0100 Subject: [PATCH 29/35] Add tests for IsName overload --- .../Helpers/SyntaxHelperTest.cs | 20 +++++++++++++++++++ .../AssertionsShouldBeComplete.NFluent.cs | 3 +++ 2 files changed, 23 insertions(+) diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/Helpers/SyntaxHelperTest.cs b/analyzers/tests/SonarAnalyzer.UnitTest/Helpers/SyntaxHelperTest.cs index 6bd0e120890..627ee118ba1 100644 --- a/analyzers/tests/SonarAnalyzer.UnitTest/Helpers/SyntaxHelperTest.cs +++ b/analyzers/tests/SonarAnalyzer.UnitTest/Helpers/SyntaxHelperTest.cs @@ -18,6 +18,7 @@ * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. */ +using Microsoft.CodeAnalysis.CSharp; using Microsoft.CodeAnalysis.Text; using SonarAnalyzer.Extensions; using CS = Microsoft.CodeAnalysis.CSharp.Syntax; @@ -137,6 +138,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 = SyntaxFactory.IdentifierName("Test"); + identifier.NameIs("other", orNames).Should().Be(expected); + } + + [TestMethod] + public void NameIsOrNamesNodeWithoutName_CS() + { + var returnStatement = SyntaxFactory.ReturnStatement(); + returnStatement.NameIs("A", "B", "C").Should().BeFalse(); + } + [TestMethod] public void NameIs_VB() { diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/AssertionsShouldBeComplete.NFluent.cs b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/AssertionsShouldBeComplete.NFluent.cs index 47b3e25da90..46acfb97447 100644 --- a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/AssertionsShouldBeComplete.NFluent.cs +++ b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/AssertionsShouldBeComplete.NFluent.cs @@ -58,6 +58,9 @@ public ICheck CheckReturnedByReturn() public ICheck CheckReturnedByExpressionBody() => Check.That(1); + + public void AnonymousInvocation(Func a) => + a()(); } namespace OtherCheck From 7fcea44681302c484bf85c99ec4ef8d5529d6458 Mon Sep 17 00:00:00 2001 From: Martin Strecker Date: Wed, 22 Feb 2023 15:30:03 +0100 Subject: [PATCH 30/35] Add tests for all framework usages in one file --- .../Rules/AssertionsShouldBeCompleteTest.cs | 16 ++++ ...ssertionsShouldBeComplete.AllFrameworks.cs | 79 +++---------------- .../AssertionsShouldBeComplete.NSubstitute.cs | 1 - 3 files changed, 27 insertions(+), 69 deletions(-) diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/Rules/AssertionsShouldBeCompleteTest.cs b/analyzers/tests/SonarAnalyzer.UnitTest/Rules/AssertionsShouldBeCompleteTest.cs index 1387b83f207..3230246c6a2 100644 --- a/analyzers/tests/SonarAnalyzer.UnitTest/Rules/AssertionsShouldBeCompleteTest.cs +++ b/analyzers/tests/SonarAnalyzer.UnitTest/Rules/AssertionsShouldBeCompleteTest.cs @@ -39,6 +39,16 @@ public class AssertionsShouldBeCompleteTest private readonly VerifierBuilder nsubstitute = new VerifierBuilder() .AddReferences(NuGetMetadataReference.NSubstitute("5.0.0")); + private readonly VerifierBuilder allFrameworks; + + public AssertionsShouldBeCompleteTest() + { + allFrameworks = new VerifierBuilder() + .AddReferences(fluentAssertions.References) + .AddReferences(nfluent.References) + .AddReferences(nsubstitute.References); + } + [TestMethod] public void AssertionsShouldBeComplete_FluentAssertions_CSharp7() => fluentAssertions @@ -90,4 +100,10 @@ public void MissingParen() nsubstitute .AddPaths("AssertionsShouldBeComplete.NSubstitute.cs") .Verify(); + + [TestMethod] + public void AssertionsShouldBeComplete_AllFrameworks_CS() => + allFrameworks + .AddPaths("AssertionsShouldBeComplete.AllFrameworks.cs") + .Verify(); } diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/AssertionsShouldBeComplete.AllFrameworks.cs b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/AssertionsShouldBeComplete.AllFrameworks.cs index f9a4893c404..a213fa5c305 100644 --- a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/AssertionsShouldBeComplete.AllFrameworks.cs +++ b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/AssertionsShouldBeComplete.AllFrameworks.cs @@ -1,82 +1,25 @@ -using NSubstitute; -using System.Net; +using FluentAssertions; +using NFluent; +using NSubstitute; +using System; -public interface ICommand -{ - void Execute(); -} - -namespace NSubstituteTests +namespace AllFrameworksTests { internal class Tests { - public void Received() - { - var command = Substitute.For(); - command.Received(); // Noncompliant {{Complete the assertion}} - // ^^^^^^^^ - command.Received(requiredNumberOfCalls: 2); // Noncompliant - command.Received(); // Noncompliant - SubstituteExtensions.Received(command); // Noncompliant - - command.Received().Execute(); - } - - public void DidNotReceive() - { - var command = Substitute.For(); - command.DidNotReceive(); // Noncompliant {{Complete the assertion}} - // ^^^^^^^^^^^^^ - command.DidNotReceive(); // Noncompliant - - command.DidNotReceive().Execute(); - } - - public void ReceivedWithAnyArgs() - { - var command = Substitute.For(); - command.ReceivedWithAnyArgs(); // Noncompliant {{Complete the assertion}} - // ^^^^^^^^^^^^^^^^^^^ - command.ReceivedWithAnyArgs(requiredNumberOfCalls: 2); // Noncompliant - command.ReceivedWithAnyArgs(); // Noncompliant - - command.ReceivedWithAnyArgs().Execute(); - } - - public void DidNotReceiveWithAnyArgs() + public void FluentAssertions(string s) { - var command = Substitute.For(); - command.DidNotReceiveWithAnyArgs(); // Noncompliant {{Complete the assertion}} - // ^^^^^^^^^^^^^^^^^^^^^^^^ - command.DidNotReceiveWithAnyArgs(); // Noncompliant - - command.DidNotReceiveWithAnyArgs().Execute(); + s.Should(); // Noncompliant } - public void ReceivedCalls() + public void NFluent() { - var command = Substitute.For(); - command.ReceivedCalls(); // Noncompliant {{Complete the assertion}} - // ^^^^^^^^^^^^^ - command.ReceivedCalls(); // Noncompliant - - var calls = command.ReceivedCalls(); + Check.That(0); // Noncompliant } - } -} - -namespace OtherReceived -{ - public static class OtherExtensions - { - public static void Received(this T something) { } - } - public class Test - { - public void Received(ICommand command) + public void NSubstitute(IComparable comparable) { - command.Received(); // Compliant. Other Received call + comparable.Received(); // Noncompliant } } } diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/AssertionsShouldBeComplete.NSubstitute.cs b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/AssertionsShouldBeComplete.NSubstitute.cs index f9a4893c404..8952f033795 100644 --- a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/AssertionsShouldBeComplete.NSubstitute.cs +++ b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/AssertionsShouldBeComplete.NSubstitute.cs @@ -1,5 +1,4 @@ using NSubstitute; -using System.Net; public interface ICommand { From dde49fbd640386045ef9bd9e19f2842ccb1945d9 Mon Sep 17 00:00:00 2001 From: Martin Strecker Date: Thu, 23 Feb 2023 13:21:32 +0100 Subject: [PATCH 31/35] Fix test method name --- .../TestCases/AssertionsShouldBeComplete.NFluent.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/AssertionsShouldBeComplete.NFluent.cs b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/AssertionsShouldBeComplete.NFluent.cs index 46acfb97447..2e57f5d147b 100644 --- a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/AssertionsShouldBeComplete.NFluent.cs +++ b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/AssertionsShouldBeComplete.NFluent.cs @@ -32,12 +32,12 @@ public void CheckThatCode() Check.ThatCode(() => { }).DoesNotThrow(); } - public async Task CheckThatCodeAsync() + public async Task CheckThatAsyncCode() { Check.ThatAsyncCode(async () => await Task.CompletedTask); // Noncompliant {{Complete the assertion}} // ^^^^^^^^^^^^^ Check.ThatAsyncCode(async () => await Task.FromResult(1)); // Noncompliant - Check.ThatAsyncCode(CheckThatCodeAsync); // Noncompliant + Check.ThatAsyncCode(CheckThatAsyncCode); // Noncompliant Check.ThatAsyncCode(async () => await Task.CompletedTask).DoesNotThrow(); } From d087c8c5d1d9ed0df7e52ebe197c6d0eb2bad9b0 Mon Sep 17 00:00:00 2001 From: Martin Strecker Date: Thu, 23 Feb 2023 13:58:07 +0100 Subject: [PATCH 32/35] Remove CSharp using from test file --- .../SonarAnalyzer.CSharp/Rules/AssertionsShouldBeComplete.cs | 3 ++- .../tests/SonarAnalyzer.UnitTest/Helpers/SyntaxHelperTest.cs | 5 ++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/analyzers/src/SonarAnalyzer.CSharp/Rules/AssertionsShouldBeComplete.cs b/analyzers/src/SonarAnalyzer.CSharp/Rules/AssertionsShouldBeComplete.cs index 1d664d3b15e..df4bcc22984 100644 --- a/analyzers/src/SonarAnalyzer.CSharp/Rules/AssertionsShouldBeComplete.cs +++ b/analyzers/src/SonarAnalyzer.CSharp/Rules/AssertionsShouldBeComplete.cs @@ -38,7 +38,8 @@ public sealed class AssertionsShouldBeComplete : SonarDiagnosticAnalyzer start.RegisterNodeAction(c => CheckInvocation(c, invocation => invocation.NameIs("Should") - && c.SemanticModel.GetSymbolInfo(invocation).AllSymbols().Any(x => x is IMethodSymbol + && c.SemanticModel.GetSymbolInfo(invocation).AllSymbols().Any(x => + x is IMethodSymbol { IsExtensionMethod: true, ReturnsVoid: false, diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/Helpers/SyntaxHelperTest.cs b/analyzers/tests/SonarAnalyzer.UnitTest/Helpers/SyntaxHelperTest.cs index 627ee118ba1..d61c17da915 100644 --- a/analyzers/tests/SonarAnalyzer.UnitTest/Helpers/SyntaxHelperTest.cs +++ b/analyzers/tests/SonarAnalyzer.UnitTest/Helpers/SyntaxHelperTest.cs @@ -18,7 +18,6 @@ * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. */ -using Microsoft.CodeAnalysis.CSharp; using Microsoft.CodeAnalysis.Text; using SonarAnalyzer.Extensions; using CS = Microsoft.CodeAnalysis.CSharp.Syntax; @@ -146,14 +145,14 @@ public void NameIs_CS() [DataRow(false, "TEST")] public void NameIsOrNames_CS(bool expected, params string[] orNames) { - var identifier = SyntaxFactory.IdentifierName("Test"); + var identifier = Microsoft.CodeAnalysis.CSharp.SyntaxFactory.IdentifierName("Test"); identifier.NameIs("other", orNames).Should().Be(expected); } [TestMethod] public void NameIsOrNamesNodeWithoutName_CS() { - var returnStatement = SyntaxFactory.ReturnStatement(); + var returnStatement = Microsoft.CodeAnalysis.CSharp.SyntaxFactory.ReturnStatement(); returnStatement.NameIs("A", "B", "C").Should().BeFalse(); } From 923500c66a37f05f9c8dfa55e3439bcdf69abee9 Mon Sep 17 00:00:00 2001 From: Martin Strecker Date: Thu, 23 Feb 2023 15:54:51 +0100 Subject: [PATCH 33/35] Formatting --- .../Helpers/KnownAssembly.cs | 17 ++++++----------- 1 file changed, 6 insertions(+), 11 deletions(-) diff --git a/analyzers/src/SonarAnalyzer.Common/Helpers/KnownAssembly.cs b/analyzers/src/SonarAnalyzer.Common/Helpers/KnownAssembly.cs index 9fdea5cd0d3..79b689de1c3 100644 --- a/analyzers/src/SonarAnalyzer.Common/Helpers/KnownAssembly.cs +++ b/analyzers/src/SonarAnalyzer.Common/Helpers/KnownAssembly.cs @@ -35,18 +35,13 @@ public sealed partial class KnownAssembly /// nuget.org/MicrosoftVisualStudioQualityToolsUnitTestFramework (MSTest V1) /// or nuget.org/MSTest.TestFramework (MSTest V2). /// - public static KnownAssembly MSTest { get; } = new( - And(NameIs("Microsoft.VisualStudio.QualityTools.UnitTestFramework").Or(NameIs("Microsoft.VisualStudio.TestPlatform.TestFramework")), - PublicKeyTokenIs("b03f5f7f11d50a3a"))); + 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"))); + 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 predicate, params Func[] or) : this(predicate is null || or.Any(x => x is null) From 264f68802fd8dfc06259795f4232cbf00f57f0e7 Mon Sep 17 00:00:00 2001 From: Martin Strecker <103252490+martin-strecker-sonarsource@users.noreply.github.com> Date: Thu, 23 Feb 2023 17:31:43 +0100 Subject: [PATCH 34/35] Update analyzers/tests/SonarAnalyzer.UnitTest/TestCases/AssertionsShouldBeComplete.NFluent.cs Co-authored-by: Tim Pohlmann --- .../TestCases/AssertionsShouldBeComplete.NFluent.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/AssertionsShouldBeComplete.NFluent.cs b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/AssertionsShouldBeComplete.NFluent.cs index 2e57f5d147b..2d2a904e6d4 100644 --- a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/AssertionsShouldBeComplete.NFluent.cs +++ b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/AssertionsShouldBeComplete.NFluent.cs @@ -42,7 +42,7 @@ public async Task CheckThatAsyncCode() Check.ThatAsyncCode(async () => await Task.CompletedTask).DoesNotThrow(); } - public async Task CheckThatCodeDynamic(dynamic expando) + public async Task CheckThatDynamic(dynamic expando) { Check.ThatDynamic(1); // Noncompliant {{Complete the assertion}} // ^^^^^^^^^^^ From b25227533fa1a7820902f485e1ce27fa4e9e5900 Mon Sep 17 00:00:00 2001 From: Martin Strecker Date: Mon, 27 Feb 2023 13:03:25 +0100 Subject: [PATCH 35/35] Update RSpec --- analyzers/rspec/cs/S2970_c#.html | 22 +++++++++++++++++++--- 1 file changed, 19 insertions(+), 3 deletions(-) diff --git a/analyzers/rspec/cs/S2970_c#.html b/analyzers/rspec/cs/S2970_c#.html index 39873623766..7a5d5177ce6 100644 --- a/analyzers/rspec/cs/S2970_c#.html +++ b/analyzers/rspec/cs/S2970_c#.html @@ -1,13 +1,29 @@ -

It is very easy to write incomplete assertions when using some test frameworks. This rule raises an issue when an assertion doesn’t actually test -anything.

+

It is very easy to write incomplete assertions when using some test frameworks. This rule enforces complete assertions in the following cases:

+
    +
  • Fluent Assertions: Should() is not followed by an assertion invocation. +
    • +
  • NFluent: Check.That() is not followed by an assertion invocation.
    • +
  • NSubstitute: Received() is not followed by an invocation.
    • +
+

In such cases, what is intended to be a test doesn’t actually verify anything.

Noncompliant Code Example

 string actual = "Hello World!";
-actual.Should();  // Noncompliant
+// Fluent Assertions
+actual.Should();     // Noncompliant
+// NFluent
+Check.That(actual);  // Noncompliant
+// NSubstitute
+command.Received();  // Noncompliant

Compliant Solution

 string actual = "Hello World!";
+// Fluent Assertions
 actual.Should().Contain("Hello");
+// NFluent
+Check.That(actual).Contains("Hello");
+// NSubstitute
+command.Received().Execute();