RC-9 : Port S8714 to python: Dedicated exception assertions should be used instead of "try-catch" with "fail()"

python-checks/src/main/java/org/sonar/python/checks/OpenSourceCheckList.java

@@ -95,6 +95,7 @@
 import org.sonar.python.checks.tests.AssertOnDissimilarTypesCheck;
 import org.sonar.python.checks.tests.AssertOnTupleLiteralCheck;
 import org.sonar.python.checks.tests.DedicatedAssertionCheck;
+import org.sonar.python.checks.tests.DedicatedExceptionAssertionCheck;
 import org.sonar.python.checks.tests.ImplicitlySkippedTestCheck;
 import org.sonar.python.checks.tests.NotDiscoverableTestMethodCheck;
 import org.sonar.python.checks.tests.SkippedTestNoReasonCheck;
@@ -180,6 +181,7 @@ public Stream<Class<?>> getChecks() {
       DeadStoreCheck.class,
       DebugModeCheck.class,
       DedicatedAssertionCheck.class,
+      DedicatedExceptionAssertionCheck.class,
       DefaultFactoryArgumentCheck.class,
       DeprecatedNumpyTypesCheck.class,
       DictKeysMembershipTestCheck.class,

python-checks/src/main/java/org/sonar/python/checks/tests/DedicatedExceptionAssertionCheck.java

@@ -0,0 +1,152 @@
+/*
+ * SonarQube Python Plugin
+ * Copyright (C) SonarSource Sàrl
+ * mailto:info AT sonarsource DOT com
+ *
+ * You can redistribute and/or modify this program under the terms of
+ * the Sonar Source-Available License Version 1, as published by SonarSource Sàrl.
+ *
+ * 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 Sonar Source-Available License for more details.
+ *
+ * You should have received a copy of the Sonar Source-Available License
+ * along with this program; if not, see https://sonarsource.com/license/ssal/
+ */
+package org.sonar.python.checks.tests;
+
+import java.util.List;
+import javax.annotation.Nullable;
+import org.sonar.check.Rule;
+import org.sonar.plugins.python.api.PythonSubscriptionCheck;
+import org.sonar.plugins.python.api.SubscriptionContext;
+import org.sonar.plugins.python.api.symbols.Symbol;
+import org.sonar.plugins.python.api.tree.CallExpression;
+import org.sonar.plugins.python.api.tree.ElseClause;
+import org.sonar.plugins.python.api.tree.Expression;
+import org.sonar.plugins.python.api.tree.ExpressionStatement;
+import org.sonar.plugins.python.api.tree.Name;
+import org.sonar.plugins.python.api.tree.QualifiedExpression;
+import org.sonar.plugins.python.api.tree.Statement;
+import org.sonar.plugins.python.api.tree.Tree;
+import org.sonar.plugins.python.api.tree.TryStatement;
+import org.sonar.python.tests.UnittestUtils;
+
+@Rule(key = "S8714")
+public class DedicatedExceptionAssertionCheck extends PythonSubscriptionCheck {
+  private static final String PYTEST_FAIL_FQN = "pytest.fail";
+  private static final String PYTEST_MESSAGE = "Replace this try/except block with a \"pytest.raises\" context manager.";
+  private static final String UNITTEST_MESSAGE = "Replace this try/except block with \"self.assertRaises()\".";
+  private static final String NO_EXCEPTION_MESSAGE = "Remove this try/except block and let the test fail naturally if an exception is raised.";
+
+  @Override
+  public void initialize(Context context) {
+    context.registerSyntaxNodeConsumer(Tree.Kind.TRY_STMT, ctx -> checkTryStatement(ctx, (TryStatement) ctx.syntaxNode()));
+  }
+
+  @Override
+  public CheckScope scope() {
+    return CheckScope.ALL;
+  }
+
+  private static void checkTryStatement(SubscriptionContext ctx, TryStatement tryStatement) {
+    if (!hasSupportedExceptClauses(tryStatement) || tryStatement.finallyClause() != null) {
+      return;
+    }
+
+    if (tryStatement.exceptClauses().size() == 1) {
+      tryStatement.exceptClauses().stream()
+        .map(exceptClause -> failCallFromSingleStatementBody(exceptClause.body().statements()))
+        .filter(callExpression -> callExpression != null)
+        .forEach(callExpression -> ctx.addIssue(callExpression, NO_EXCEPTION_MESSAGE));
+    }
+
+    CallExpression failCall = failCallFromElseClause(tryStatement.elseClause());
+    if (failCall == null) {
+      failCall = failCallFromTryBody(tryStatement.body().statements());
+    }
+    if (failCall == null) {
+      return;
+    }
+
+    String message = issueMessage(failCall);
+    if (message != null) {
+      ctx.addIssue(failCall, message);
+    }
+  }
+
+  private static boolean hasSupportedExceptClauses(TryStatement tryStatement) {
+    return !tryStatement.exceptClauses().isEmpty() && tryStatement.exceptClauses().stream().allMatch(exceptClause ->
+      exceptClause.starToken() == null && exceptClause.exception() != null);
+  }
+
+  @Nullable
+  private static CallExpression failCallFromElseClause(@Nullable ElseClause elseClause) {
+    if (elseClause == null) {
+      return null;
+    }
+    return failCallFromSingleStatementBody(elseClause.body().statements());
+  }
+
+  @Nullable
+  private static CallExpression failCallFromTryBody(List<Statement> statements) {
+    if (statements.size() < 2) {
+      return null;
+    }
+    return failCallFromStatement(statements.get(statements.size() - 1));
+  }
+
+  @Nullable
+  private static CallExpression failCallFromSingleStatementBody(List<Statement> statements) {
+    if (statements.size() != 1) {
+      return null;
+    }
+    return failCallFromStatement(statements.get(0));
+  }
+
+  @Nullable
+  private static CallExpression failCallFromStatement(Statement statement) {
+    if (!(statement instanceof ExpressionStatement expressionStatement) || expressionStatement.expressions().size() != 1) {
+      return null;
+    }
+    Expression expression = expressionStatement.expressions().get(0);
+    if (!(expression instanceof CallExpression callExpression)) {
+      return null;
+    }
+    if (!(isPytestFail(callExpression) || isUnittestFail(callExpression))) {
+      return null;
+    }
+    return callExpression;
+  }
+
+  private static boolean isPytestFail(CallExpression callExpression) {
+    Symbol calleeSymbol = callExpression.calleeSymbol();
+    return calleeSymbol != null && PYTEST_FAIL_FQN.equals(calleeSymbol.fullyQualifiedName());
+  }
+
+  private static boolean isUnittestFail(CallExpression callExpression) {
+    if (!UnittestUtils.isWithinUnittestTestCase(callExpression)) {
+      return false;
+    }
+    Expression callee = callExpression.callee();
+    if (!(callee instanceof QualifiedExpression qualifiedExpression)) {
+      return false;
+    }
+    if (!(qualifiedExpression.qualifier() instanceof Name qualifier) || !"self".equals(qualifier.name())) {
+      return false;
+    }
+    return "fail".equals(qualifiedExpression.name().name());
+  }
+
+  @Nullable
+  private static String issueMessage(CallExpression failCall) {
+    if (isPytestFail(failCall)) {
+      return PYTEST_MESSAGE;
+    }
+    if (isUnittestFail(failCall)) {
+      return UNITTEST_MESSAGE;
+    }
+    return null;
+  }
+}

python-checks/src/main/resources/org/sonar/l10n/py/rules/python/S8714.html

@@ -0,0 +1,145 @@
+<p>This rule raises an issue when a test uses a <code>try-except</code> block combined with <code>fail()</code> to verify whether an exception is
+raised or not raised.</p>
+<h2>Why is this an issue?</h2>
+<p>Using <code>try-except</code> blocks combined with <code>fail()</code> to test for the presence or absence of exceptions is an anti-pattern.</p>
+<p>Modern testing frameworks like <code>pytest</code> and Python’s built-in <code>unittest</code> provide dedicated assertion methods that make
+exception testing cleaner and more explicit. These methods eliminate boilerplate code, clearly communicate test intent, and make it easier to assert
+on exception details.</p>
+<p>The <code>try-except-fail()</code> pattern has several drawbacks:</p>
+<ul>
+  <li>It adds unnecessary boilerplate that obscures the test’s purpose</li>
+  <li>It requires manual exception handling instead of using framework features</li>
+  <li>It makes the test harder to maintain and read</li>
+  <li>It provides no straightforward way to assert on exception attributes when an exception is expected</li>
+</ul>
+<p>Dedicated exception assertions solve these problems by providing a declarative way to express exception expectations.</p>
+<h3>What is the potential impact?</h3>
+<p>Using <code>try-except</code> with <code>fail()</code> instead of dedicated exception assertions reduces test code quality and maintainability:</p>
+<ul>
+  <li><strong>Readability</strong>: Tests become harder to understand at a glance, especially for developers unfamiliar with the codebase</li>
+  <li><strong>Maintainability</strong>: More verbose code means more places for bugs to hide and more effort required to update tests</li>
+  <li><strong>Intent clarity</strong>: The test’s purpose (verifying exception behavior) is buried in control flow rather than being explicit</li>
+  <li><strong>Lost functionality</strong>: It’s harder to assert on exception messages, types, or other attributes when using the
+  <code>try-except</code> pattern</li>
+</ul>
+<h2>How to fix it in unittest</h2>
+<h3>Code examples</h3>
+<h4>Noncompliant code example</h4>
+<p>Replace <code>try-except</code> blocks that use <code>fail()</code> with <code>assertRaises</code> used as a context manager. This built-in method
+from Python’s <code>unittest</code> framework provides a clean way to verify exception behavior.</p>
+<p>When you expect no exception, simply call the code directly without wrapping it in a <code>try-except</code> block. If an unexpected exception is
+raised, the test framework will catch it and fail the test automatically.</p>
+<p>When you expect an exception, use <code>assertRaises</code> as a context manager. This allows you to capture the exception and make additional
+assertions on its properties.</p>
+<pre data-diff-id="1" data-diff-type="noncompliant">
+import unittest
+
+class UserServiceTest(unittest.TestCase):
+    def test_no_exception_thrown(self):
+        try:
+            self.user_service.register_user(self.valid_user)
+        except ValidationError:
+            self.fail("Should not have thrown any exception")  # Noncompliant
+</pre>
+<p>When verifying that an exception is raised, use <code>assertRaises</code> as a context manager. This provides access to the exception object for
+further assertions.</p>
+<pre data-diff-id="2" data-diff-type="noncompliant">
+import unittest
+
+class UserServiceTest(unittest.TestCase):
+    def test_exception_is_thrown(self):
+        try:
+            self.user_service.register_user(self.invalid_user)
+            self.fail("Expected ValidationError to be thrown")  # Noncompliant
+        except ValidationError as e:
+            self.assertEqual("Invalid email", str(e))
+</pre>
+<h4>Compliant solution</h4>
+<p>Replace <code>try-except</code> blocks that use <code>fail()</code> with <code>assertRaises</code> used as a context manager. This built-in method
+from Python’s <code>unittest</code> framework provides a clean way to verify exception behavior.</p>
+<p>When you expect no exception, simply call the code directly without wrapping it in a <code>try-except</code> block. If an unexpected exception is
+raised, the test framework will catch it and fail the test automatically.</p>
+<p>When you expect an exception, use <code>assertRaises</code> as a context manager. This allows you to capture the exception and make additional
+assertions on its properties.</p>
+<pre data-diff-id="1" data-diff-type="compliant">
+import unittest
+
+class UserServiceTest(unittest.TestCase):
+    def test_no_exception_thrown(self):
+        # Simply call the method - unittest will fail if an exception is raised
+        self.user_service.register_user(self.valid_user)
+</pre>
+<p>When verifying that an exception is raised, use <code>assertRaises</code> as a context manager. This provides access to the exception object for
+further assertions.</p>
+<pre data-diff-id="2" data-diff-type="compliant">
+import unittest
+
+class UserServiceTest(unittest.TestCase):
+    def test_exception_is_thrown(self):
+        with self.assertRaises(ValidationError) as context:
+            self.user_service.register_user(self.invalid_user)
+
+        self.assertEqual("Invalid email", str(context.exception))
+</pre>
+<h2>How to fix it in pytest</h2>
+<h3>Code examples</h3>
+<h4>Noncompliant code example</h4>
+<p>Replace <code>try-except</code> blocks with <code>pytest.raises</code> used as a context manager. Pytest is the most popular Python testing
+framework and provides excellent exception testing support.</p>
+<p>When you expect no exception, simply call the code directly. Pytest will fail the test if an unexpected exception occurs.</p>
+<p>When you expect an exception, use <code>pytest.raises</code> as a context manager to verify the exception type and optionally inspect its
+properties.</p>
+<pre data-diff-id="3" data-diff-type="noncompliant">
+import pytest
+
+def test_no_exception_thrown(user_service, valid_user):
+    try:
+        user_service.register_user(valid_user)
+    except ValidationError:
+        pytest.fail("Should not have thrown any exception")  # Noncompliant
+</pre>
+<p>When verifying that an exception is raised with pytest, use <code>pytest.raises</code> as a context manager. This gives you access to the exception
+object for additional assertions using the <code>match</code> parameter or the <code>value</code> attribute.</p>
+<pre data-diff-id="4" data-diff-type="noncompliant">
+import pytest
+
+def test_exception_is_thrown(user_service, invalid_user):
+    try:
+        user_service.register_user(invalid_user)
+        pytest.fail("Expected ValidationError to be thrown")  # Noncompliant
+    except ValidationError as e:
+        assert str(e) == "Invalid email"
+</pre>
+<h4>Compliant solution</h4>
+<p>Replace <code>try-except</code> blocks with <code>pytest.raises</code> used as a context manager. Pytest is the most popular Python testing
+framework and provides excellent exception testing support.</p>
+<p>When you expect no exception, simply call the code directly. Pytest will fail the test if an unexpected exception occurs.</p>
+<p>When you expect an exception, use <code>pytest.raises</code> as a context manager to verify the exception type and optionally inspect its
+properties.</p>
+<pre data-diff-id="3" data-diff-type="compliant">
+def test_no_exception_thrown(user_service, valid_user):
+    # Simply call the method - pytest will fail if an exception is raised
+    user_service.register_user(valid_user)
+</pre>
+<p>When verifying that an exception is raised with pytest, use <code>pytest.raises</code> as a context manager. This gives you access to the exception
+object for additional assertions using the <code>match</code> parameter or the <code>value</code> attribute.</p>
+<pre data-diff-id="4" data-diff-type="compliant">
+import pytest
+
+def test_exception_is_thrown(user_service, invalid_user):
+    with pytest.raises(ValidationError) as exc_info:
+        user_service.register_user(invalid_user)
+
+    assert str(exc_info.value) == "Invalid email"
+</pre>
+<h2>Resources</h2>
+<h3>Documentation</h3>
+<ul>
+  <li>unittest documentation - assertRaises - <a href="https://docs.python.org/3/library/unittest.html#unittest.TestCase.assertRaises">Official Python
+  documentation for unittest’s assertRaises method</a></li>
+  <li>pytest documentation - raises - <a href="https://docs.pytest.org/en/stable/reference/reference.html#pytest.raises">Official pytest documentation
+  for the raises context manager</a></li>
+  <li>pytest - Testing exceptions - <a href="https://docs.pytest.org/en/stable/how-to/assert.html#assertions-about-expected-exceptions">Pytest guide
+  on asserting exceptions in tests</a></li>
+</ul>
+

python-checks/src/main/resources/org/sonar/l10n/py/rules/python/S8714.json

@@ -0,0 +1,23 @@
+{
+  "title": "Dedicated exception assertions should be used instead of \"try-except\" with \"fail()\"",
+  "type": "CODE_SMELL",
+  "status": "ready",
+  "remediation": {
+    "func": "Constant\/Issue",
+    "constantCost": "5min"
+  },
+  "tags": [
+    "tests"
+  ],
+  "defaultSeverity": "Minor",
+  "ruleSpecification": "RSPEC-8714",
+  "sqKey": "S8714",
+  "scope": "Tests",
+  "quickfix": "unknown",
+  "code": {
+    "impacts": {
+      "MAINTAINABILITY": "LOW"
+    },
+    "attribute": "CONVENTIONAL"
+  }
+}

python-checks/src/main/resources/org/sonar/l10n/py/rules/python/Sonar_agentic_AI_profile.json

@@ -306,6 +306,7 @@
     "S8413",
     "S8414",
     "S8415",
+    "S8714",
     "S8786"
   ]
 }

python-checks/src/main/resources/org/sonar/l10n/py/rules/python/Sonar_way_profile.json

@@ -353,6 +353,7 @@
     "S8521",
     "S8572",
     "S8685",
+    "S8714",
     "S8786"
   ]
 }

python-checks/src/test/java/org/sonar/python/checks/tests/DedicatedExceptionAssertionCheckTest.java

@@ -0,0 +1,36 @@
+/*
+ * SonarQube Python Plugin
+ * Copyright (C) SonarSource Sàrl
+ * mailto:info AT sonarsource DOT com
+ *
+ * You can redistribute and/or modify this program under the terms of
+ * the Sonar Source-Available License Version 1, as published by SonarSource Sàrl.
+ *
+ * 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 Sonar Source-Available License for more details.
+ *
+ * You should have received a copy of the Sonar Source-Available License
+ * along with this program; if not, see https://sonarsource.com/license/ssal/
+ */
+package org.sonar.python.checks.tests;
+
+import org.junit.jupiter.api.Test;
+import org.sonar.plugins.python.api.PythonCheck;
+import org.sonar.python.checks.utils.PythonCheckVerifier;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+class DedicatedExceptionAssertionCheckTest {
+
+  @Test
+  void sample() {
+    PythonCheckVerifier.verify("src/test/resources/checks/tests/dedicatedExceptionAssertion.py", new DedicatedExceptionAssertionCheck());
+  }
+
+  @Test
+  void test_scope() {
+    assertThat(new DedicatedExceptionAssertionCheck().scope()).isEqualTo(PythonCheck.CheckScope.ALL);
+  }
+}