Skip to content

Commit

Permalink
Fix S3431 FP: Don't raise if assertions are done in catch or finally (#…
Browse files Browse the repository at this point in the history 
…9615)
  • Loading branch information
@sebastien-marichal
sebastien-marichal authored Aug 9, 2024
1 parent 3191807 commit 4bf4902
Show file tree
Hide file tree
Showing 8 changed files with 429 additions and 63 deletions.
38 changes: 36 additions & 2 deletions analyzers/src/SonarAnalyzer.CSharp/Rules/ExpectedExceptionAttributeShouldNotBeUsed.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,18 +18,52 @@
* Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
*/

using SonarAnalyzer.Common.Walkers;

namespace SonarAnalyzer.Rules.CSharp
{
[DiagnosticAnalyzer(LanguageNames.CSharp)]
public sealed class ExpectedExceptionAttributeShouldNotBeUsed : ExpectedExceptionAttributeShouldNotBeUsedBase<SyntaxKind>
{
protected override ILanguageFacade<SyntaxKind> Language => CSharpFacade.Instance;

protected override bool HasMultiLineBody(SyntaxNode syntax)
protected override bool HasMultiLineBody(SyntaxNode node)
{
var declaration = (MethodDeclarationSyntax)syntax;
var declaration = (MethodDeclarationSyntax)node;
return declaration.ExpressionBody is null
&& declaration.Body?.Statements.Count > 1;
}

protected override bool AssertInCatchFinallyBlock(SyntaxNode node)
{
var walker = new CatchFinallyAssertion();
foreach (var x in node.DescendantNodes().Where(x => x.Kind() is SyntaxKind.CatchClause or SyntaxKind.FinallyClause))
{
if (!walker.HasAssertion)
{
walker.SafeVisit(x);
}
}
return walker.HasAssertion;
}

private sealed class CatchFinallyAssertion : SafeCSharpSyntaxWalker
{
public bool HasAssertion { get; set; }

public override void VisitInvocationExpression(InvocationExpressionSyntax node)
{
if (HasAssertion)
{
return;
}

HasAssertion = node.Expression
.ToString()
.SplitCamelCaseToWords()
.Intersect(UnitTestHelper.KnownAssertionMethodParts)
.Any();
}
}
}
}
4 changes: 3 additions & 1 deletion analyzers/src/SonarAnalyzer.Common/Rules/ExpectedExceptionAttributeShouldNotBeUsedBase.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,8 @@ public abstract class ExpectedExceptionAttributeShouldNotBeUsedBase<TSyntaxKind>
{
internal const string DiagnosticId = "S3431";

protected abstract bool HasMultiLineBody(SyntaxNode syntax);
protected abstract bool HasMultiLineBody(SyntaxNode node);
protected abstract bool AssertInCatchFinallyBlock(SyntaxNode node);

protected override string MessageFormat => "Replace the 'ExpectedException' attribute with a throw assertion or a try/catch block.";

Expand All @@ -35,6 +36,7 @@ protected override void Initialize(SonarAnalysisContext context) =>
context.RegisterNodeAction(Language.GeneratedCodeRecognizer, c =>
{
if (HasMultiLineBody(c.Node)
&& !AssertInCatchFinallyBlock(c.Node)
&& c.SemanticModel.GetDeclaredSymbol(c.Node) is { } methodSymbol
&& methodSymbol.GetAttributes(UnitTestHelper.KnownExpectedExceptionAttributes).FirstOrDefault() is { } attribute)
{
Expand Down
44 changes: 37 additions & 7 deletions analyzers/src/SonarAnalyzer.VisualBasic/Rules/ExpectedExceptionAttributeShouldNotBeUsed.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,15 +18,45 @@
* Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
*/

namespace SonarAnalyzer.Rules.VisualBasic
namespace SonarAnalyzer.Rules.VisualBasic;

[DiagnosticAnalyzer(LanguageNames.VisualBasic)]
public sealed class ExpectedExceptionAttributeShouldNotBeUsed : ExpectedExceptionAttributeShouldNotBeUsedBase<SyntaxKind>
{
[DiagnosticAnalyzer(LanguageNames.VisualBasic)]
public sealed class ExpectedExceptionAttributeShouldNotBeUsed : ExpectedExceptionAttributeShouldNotBeUsedBase<SyntaxKind>
protected override ILanguageFacade<SyntaxKind> Language => VisualBasicFacade.Instance;

protected override bool HasMultiLineBody(SyntaxNode node) =>
node.Parent is MethodBlockSyntax { Statements.Count: > 1 };

protected override bool AssertInCatchFinallyBlock(SyntaxNode node)
{
protected override ILanguageFacade<SyntaxKind> Language => VisualBasicFacade.Instance;
var walker = new CatchFinallyAssertion();
foreach (var x in node.Parent.DescendantNodes().Where(x => x.Kind() is SyntaxKind.CatchBlock or SyntaxKind.FinallyBlock))
{
if (!walker.HasAssertion)
{
walker.SafeVisit(x);
}
}
return walker.HasAssertion;
}

private sealed class CatchFinallyAssertion : SafeVisualBasicSyntaxWalker
{
public bool HasAssertion { get; set; }

public override void VisitInvocationExpression(InvocationExpressionSyntax node)
{
if (HasAssertion)
{
return;
}

protected override bool HasMultiLineBody(SyntaxNode syntax) =>
syntax.Parent is MethodBlockSyntax methodBlockSyntax
&& methodBlockSyntax.Statements.Count > 1;
HasAssertion = node.Expression
.ToString()
.SplitCamelCaseToWords()
.Intersect(UnitTestHelper.KnownAssertionMethodParts)
.Any();
}
}
}
105 changes: 52 additions & 53 deletions analyzers/tests/SonarAnalyzer.Test/Rules/ExpectedExceptionAttributeShouldNotBeUsedTest.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,64 +21,63 @@
using CS = SonarAnalyzer.Rules.CSharp;
using VB = SonarAnalyzer.Rules.VisualBasic;

namespace SonarAnalyzer.Test.Rules
namespace SonarAnalyzer.Test.Rules;

[TestClass]
public class ExpectedExceptionAttributeShouldNotBeUsedTest
{
[TestClass]
public class ExpectedExceptionAttributeShouldNotBeUsedTest
{
private readonly VerifierBuilder builderCS = new VerifierBuilder<CS.ExpectedExceptionAttributeShouldNotBeUsed>();
private readonly VerifierBuilder builderVB = new VerifierBuilder<VB.ExpectedExceptionAttributeShouldNotBeUsed>();
private readonly VerifierBuilder builderCS = new VerifierBuilder<CS.ExpectedExceptionAttributeShouldNotBeUsed>();
private readonly VerifierBuilder builderVB = new VerifierBuilder<VB.ExpectedExceptionAttributeShouldNotBeUsed>();

[DataTestMethod]
[DataRow("1.1.11")]
[DataRow(Constants.NuGetLatestVersion)]
public void ExpectedExceptionAttributeShouldNotBeUsed_MsTest_CS(string testFwkVersion) =>
builderCS.AddPaths("ExpectedExceptionAttributeShouldNotBeUsed.MsTest.cs")
.AddReferences(NuGetMetadataReference.MSTestTestFramework(testFwkVersion))
.Verify();
[DataTestMethod]
[DataRow("1.1.11")]
[DataRow(Constants.NuGetLatestVersion)]
public void ExpectedExceptionAttributeShouldNotBeUsed_MsTest_CS(string testFwkVersion) =>
builderCS.AddPaths("ExpectedExceptionAttributeShouldNotBeUsed.MsTest.cs")
.AddReferences(NuGetMetadataReference.MSTestTestFramework(testFwkVersion))
.Verify();

[DataTestMethod]
[DataRow("2.5.7.10213")]
[DataRow("2.6.7")]
public void ExpectedExceptionAttributeShouldNotBeUsed_NUnit_CS(string testFwkVersion) =>
builderCS.AddPaths("ExpectedExceptionAttributeShouldNotBeUsed.NUnit.cs")
.AddReferences(NuGetMetadataReference.NUnit(testFwkVersion))
.Verify();
[DataTestMethod]
[DataRow("2.5.7.10213")] // Lowest NUnit version available
[DataRow("2.7.1")] // Latest version of NUnit that contains the attribute
public void ExpectedExceptionAttributeShouldNotBeUsed_NUnit_CS(string testFwkVersion) =>
builderCS.AddPaths("ExpectedExceptionAttributeShouldNotBeUsed.NUnit.cs")
.AddReferences(NuGetMetadataReference.NUnit(testFwkVersion))
.Verify();

[DataTestMethod]
[DataRow("3.0.0")]
[DataRow(Constants.NuGetLatestVersion)]
[Description("Starting with version 3.0.0 the attribute was removed.")]
public void ExpectedExceptionAttributeShouldNotBeUsed_NUnit_NoIssue_CS(string testFwkVersion) =>
builderCS.AddPaths("ExpectedExceptionAttributeShouldNotBeUsed.NUnit.cs")
.AddReferences(NuGetMetadataReference.NUnit(testFwkVersion))
.WithErrorBehavior(CompilationErrorBehavior.Ignore)
.VerifyNoIssues();
[DataTestMethod]
[DataRow("3.0.0")]
[DataRow(Constants.NuGetLatestVersion)]
[Description("Starting with version 3.0.0 the attribute was removed.")]
public void ExpectedExceptionAttributeShouldNotBeUsed_NUnit_NoIssue_CS(string testFwkVersion) =>
builderCS.AddPaths("ExpectedExceptionAttributeShouldNotBeUsed.NUnit.cs")
.AddReferences(NuGetMetadataReference.NUnit(testFwkVersion))
.WithErrorBehavior(CompilationErrorBehavior.Ignore)
.VerifyNoIssues();

[DataTestMethod]
[DataRow("1.1.11")]
[DataRow(Constants.NuGetLatestVersion)]
public void ExpectedExceptionAttributeShouldNotBeUsed_MsTest_VB(string testFwkVersion) =>
builderVB.AddPaths("ExpectedExceptionAttributeShouldNotBeUsed.MsTest.vb")
.AddReferences(NuGetMetadataReference.MSTestTestFramework(testFwkVersion))
.Verify();
[DataTestMethod]
[DataRow("1.1.11")]
[DataRow(Constants.NuGetLatestVersion)]
public void ExpectedExceptionAttributeShouldNotBeUsed_MsTest_VB(string testFwkVersion) =>
builderVB.AddPaths("ExpectedExceptionAttributeShouldNotBeUsed.MsTest.vb")
.AddReferences(NuGetMetadataReference.MSTestTestFramework(testFwkVersion))
.Verify();

[DataTestMethod]
[DataRow("2.5.7.10213")]
[DataRow("2.6.7")]
public void ExpectedExceptionAttributeShouldNotBeUsed_NUnit_VB(string testFwkVersion) =>
builderVB.AddPaths("ExpectedExceptionAttributeShouldNotBeUsed.NUnit.vb")
.AddReferences(NuGetMetadataReference.NUnit(testFwkVersion))
.Verify();
[DataTestMethod]
[DataRow("2.5.7.10213")]
[DataRow("2.6.7")]
public void ExpectedExceptionAttributeShouldNotBeUsed_NUnit_VB(string testFwkVersion) =>
builderVB.AddPaths("ExpectedExceptionAttributeShouldNotBeUsed.NUnit.vb")
.AddReferences(NuGetMetadataReference.NUnit(testFwkVersion))
.Verify();

[DataTestMethod]
[DataRow("3.0.0")]
[DataRow(Constants.NuGetLatestVersion)]
[Description("Starting with version 3.0.0 the attribute was removed.")]
public void ExpectedExceptionAttributeShouldNotBeUsed_NUnit_NoIssue_VB(string testFwkVersion) =>
builderVB.AddPaths("ExpectedExceptionAttributeShouldNotBeUsed.NUnit.vb")
.AddReferences(NuGetMetadataReference.NUnit(testFwkVersion))
.WithErrorBehavior(CompilationErrorBehavior.Ignore)
.VerifyNoIssues();
}
[DataTestMethod]
[DataRow("3.0.0")]
[DataRow(Constants.NuGetLatestVersion)]
[Description("Starting with version 3.0.0 the attribute was removed.")]
public void ExpectedExceptionAttributeShouldNotBeUsed_NUnit_NoIssue_VB(string testFwkVersion) =>
builderVB.AddPaths("ExpectedExceptionAttributeShouldNotBeUsed.NUnit.vb")
.AddReferences(NuGetMetadataReference.NUnit(testFwkVersion))
.WithErrorBehavior(CompilationErrorBehavior.Ignore)
.VerifyNoIssues();
}
Loading

0 comments on commit 4bf4902

Please sign in to comment.