SONARPY-3912 Rule S8493 "StopIteration" should not be raised inside generators (#957)

GitOrigin-RevId: d2513cf9afb9167f8a7d2431ed871efa9e137ca0

Author: sonar-nigel[bot]

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

@@ -414,6 +414,7 @@ public Stream<Class<?>> getChecks() {
       SpecialMethodReturnTypeCheck.class,
       SQLQueriesCheck.class,
       StandardInputCheck.class,
+      StopIterationInGeneratorCheck.class,
       StrftimeConfusingHourSystemCheck.class,
       StringFormatCorrectnessCheck.class,
       StringFormatMisuseCheck.class,

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

@@ -0,0 +1,63 @@
+/*
+ * 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;
+
+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.tree.Expression;
+import org.sonar.plugins.python.api.tree.FunctionDef;
+import org.sonar.plugins.python.api.tree.RaiseStatement;
+import org.sonar.plugins.python.api.tree.Tree;
+import org.sonar.plugins.python.api.types.v2.matchers.TypeMatcher;
+import org.sonar.plugins.python.api.types.v2.matchers.TypeMatchers;
+import org.sonar.python.tree.TreeUtils;
+
+@Rule(key = "S8493")
+public class StopIterationInGeneratorCheck extends PythonSubscriptionCheck {
+
+  private static final String MESSAGE = "Replace this \"raise StopIteration\" with a \"return\" statement.";
+
+  private final TypeMatcher stopIterationMatcher = TypeMatchers.any(
+    TypeMatchers.isObjectOfType("StopIteration"),
+    TypeMatchers.isType("StopIteration")
+  );
+
+  @Override
+  public void initialize(Context context) {
+    context.registerSyntaxNodeConsumer(Tree.Kind.RAISE_STMT, this::checkRaise);
+  }
+
+  private void checkRaise(SubscriptionContext ctx) {
+    RaiseStatement raiseStatement = (RaiseStatement) ctx.syntaxNode();
+    if (raiseStatement.expressions().isEmpty()) {
+      return;
+    }
+    Expression expression = raiseStatement.expressions().get(0);
+    if (!stopIterationMatcher.isTrueFor(expression, ctx)) {
+      return;
+    }
+    FunctionDef enclosingFunction = (FunctionDef) TreeUtils.firstAncestorOfKind(raiseStatement, Tree.Kind.FUNCDEF);
+    if (enclosingFunction == null) {
+      return;
+    }
+    if (!ReturnCheckUtils.ReturnStmtCollector.collect(enclosingFunction).containsYield()) {
+      return;
+    }
+    ctx.addIssue(raiseStatement, MESSAGE);
+  }
+}

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

@@ -0,0 +1,52 @@
+<p>This rule raises an issue when <code>StopIteration</code> is explicitly raised within a generator function, that is, a function containing
+<code>yield</code>.</p>
+<h2>Why is this an issue?</h2>
+<p>In Python, generators are special functions that use the <code>yield</code> keyword to produce a sequence of values lazily. They implement the
+iterator protocol and are used to create iterators in a concise way.</p>
+<p>Traditionally, raising <code>StopIteration</code> was the standard way to signal that an iterator had no more values to produce. However, this
+could lead to subtle bugs when <code>StopIteration</code> exceptions from nested iterators or function calls accidentally propagated up and terminated
+the generator unexpectedly.</p>
+<p>To address this issue, <a href="https://peps.python.org/pep-0479/">PEP 479</a> changed the behavior in Python 3.7+. Now, when
+<code>StopIteration</code> is raised inside a generator function, Python automatically converts it to a <code>RuntimeError</code>. This conversion
+happens at the point where the exception would leave the generator.</p>
+<p>This means code that explicitly raises <code>StopIteration</code> in a generator will fail with a <code>RuntimeError</code> at runtime in Python
+3.7 and later versions. The error message will indicate that the <code>StopIteration</code> exception was raised inside a generator.</p>
+<p>The proper way to terminate a generator is to use a <code>return</code> statement, optionally with a value. When a generator executes a
+<code>return</code>, Python automatically raises <code>StopIteration</code> behind the scenes, but does so in a way that is compatible with PEP
+479.</p>
+<h3>What is the potential impact?</h3>
+<p>When <code>StopIteration</code> is raised inside a generator in Python 3.7+, it causes a <code>RuntimeError</code> at runtime. This can lead
+to:</p>
+<ul>
+  <li>application crashes or unexpected failures</li>
+  <li>difficult-to-debug errors, especially in production environments</li>
+  <li>code that works in older Python versions but breaks when upgraded</li>
+  <li>inconsistent behavior across different Python versions</li>
+</ul>
+<h2>How to fix it</h2>
+<p>Replace the <code>raise StopIteration</code> statement with a <code>return</code> statement. In generators, <code>return</code> is the proper way
+to signal completion. If you need to return a value when the generator completes, you can use <code>return value</code>, though note that this value
+is not yielded but becomes the exception value of the <code>StopIteration</code> that is automatically raised.</p>
+<h3>Code examples</h3>
+<h4>Noncompliant code example</h4>
+<pre data-diff-id="1" data-diff-type="noncompliant">
+def my_generator():
+    yield 1
+    yield 2
+    raise StopIteration  # Noncompliant
+</pre>
+<h4>Compliant solution</h4>
+<pre data-diff-id="1" data-diff-type="compliant">
+def my_generator():
+    yield 1
+    yield 2
+    return  # Properly terminates the generator
+</pre>
+<h2>Resources</h2>
+<h3>Documentation</h3>
+<ul>
+  <li>PEP 479 - <a href="https://peps.python.org/pep-0479/">Change StopIteration handling inside generators</a></li>
+  <li>Python Documentation - <a href="https://docs.python.org/3/howto/functional.html#generators">Generators</a></li>
+  <li>Python Documentation - <a href="https://docs.python.org/3/library/stdtypes.html#iterator-types">Iterator Types</a></li>
+</ul>
+

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

@@ -0,0 +1,25 @@
+{
+  "title": "\"StopIteration\" should not be raised inside generators",
+  "type": "BUG",
+  "status": "ready",
+  "remediation": {
+    "func": "Constant\/Issue",
+    "constantCost": "5 min"
+  },
+  "tags": [
+    "pitfall",
+    "python3"
+  ],
+  "defaultSeverity": "Blocker",
+  "ruleSpecification": "RSPEC-8493",
+  "sqKey": "S8493",
+  "scope": "Main",
+  "quickfix": "unknown",
+  "code": {
+    "impacts": {
+      "RELIABILITY": "BLOCKER",
+      "MAINTAINABILITY": "HIGH"
+    },
+    "attribute": "LOGICAL"
+  }
+}

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

@@ -334,6 +334,7 @@
     "S8504",
     "S8517",
     "S8520",
-    "S8521"
+    "S8521",
+    "S8493"
   ]
 }

python-checks/src/test/java/org/sonar/python/checks/StopIterationInGeneratorCheckTest.java

@@ -0,0 +1,29 @@
+/*
+ * 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;
+
+import org.junit.jupiter.api.Test;
+import org.sonar.python.checks.utils.PythonCheckVerifier;
+
+class StopIterationInGeneratorCheckTest {
+
+  @Test
+  void test() {
+    PythonCheckVerifier.verify("src/test/resources/checks/stopIterationInGenerator.py", new StopIterationInGeneratorCheck());
+  }
+
+}

python-checks/src/test/resources/checks/stopIterationInGenerator.py

@@ -0,0 +1,203 @@
+def generator_basic():
+    yield 1
+    yield 2
+    raise StopIteration  # Noncompliant {{Replace this "raise StopIteration" with a "return" statement.}}
+
+def generator_with_int_arg():
+    yield 1
+    raise StopIteration(42)  # Noncompliant {{Replace this "raise StopIteration" with a "return" statement.}}
+
+def generator_with_str_arg():
+    yield 1
+    raise StopIteration("message")  # Noncompliant {{Replace this "raise StopIteration" with a "return" statement.}}
+
+def generator_raise_in_if(condition):
+    yield 1
+    if condition:
+        raise StopIteration  # Noncompliant
+
+def generator_raise_in_else(condition):
+    yield 1
+    if condition:
+        pass
+    else:
+        raise StopIteration  # Noncompliant
+
+def generator_raise_in_for(items):
+    for item in items:
+        if item is None:
+            raise StopIteration  # Noncompliant
+        yield item
+
+def generator_raise_in_while():
+    count = 0
+    while count < 10:
+        if count == 5:
+            raise StopIteration  # Noncompliant
+        yield count
+        count += 1
+
+def generator_raise_in_try():
+    yield 1
+    try:
+        raise StopIteration  # Noncompliant
+    except ValueError:
+        pass
+
+def generator_raise_in_except():
+    yield 1
+    try:
+        pass
+    except StopIteration:
+        raise StopIteration  # Noncompliant
+
+def generator_raise_in_finally():
+    yield 1
+    try:
+        pass
+    finally:
+        raise StopIteration  # Noncompliant
+
+def generator_raise_chained():
+    yield 1
+    raise StopIteration from ValueError("cause")  # Noncompliant
+
+async def async_generator_raise():
+    yield 1
+    raise StopIteration  # Noncompliant
+
+def generator_after_yield_from():
+    yield from range(3)
+    raise StopIteration  # Noncompliant
+
+def generator_nested_control_flow(data):
+    for item in data:
+        yield item
+        if item > 100:
+            if item > 1000:
+                raise StopIteration  # Noncompliant
+
+
+def generator_return():
+    yield 1
+    yield 2
+    return
+
+def generator_return_value():
+    yield 1
+    return 42
+
+def generator_natural_exhaustion():
+    yield 1
+    yield 2
+
+def regular_function():
+    raise StopIteration
+
+class ManualIterator:
+    def __init__(self):
+        self.count = 0
+
+    def __next__(self):
+        self.count += 1
+        if self.count > 3:
+            raise StopIteration
+        return self.count
+
+def helper_raises():
+    raise StopIteration
+
+def generator_calls_helper():
+    yield 1
+    helper_raises()
+
+def outer_generator_nested_helper():
+    def inner_helper():
+        raise StopIteration
+    yield 1
+    inner_helper()
+
+def generator_catches_stop_iteration():
+    yield 1
+    try:
+        pass
+    except StopIteration:
+        return
+
+def generator_raises_value_error():
+    yield 1
+    raise ValueError("not a stop iteration")
+
+def generator_bare_raise():
+    yield 1
+    try:
+        pass
+    except Exception:
+        raise
+
+raise StopIteration
+
+def generator_deeply_nested_helper():
+    def inner_non_generator():
+        def innermost():
+            raise StopIteration
+        innermost()
+    yield 1
+    inner_non_generator()
+
+async def async_generator_catches():
+    yield 1
+    try:
+        pass
+    except StopIteration:
+        return
+
+
+class IteratorClass:
+    def generator_method(self):
+        yield 1
+        raise StopIteration  # Noncompliant
+
+
+def nested_generators():
+    def inner_generator():
+        yield 10
+        raise StopIteration  # Noncompliant
+    yield 1
+    raise StopIteration  # Noncompliant
+
+
+class CustomStopIteration(StopIteration):
+    pass
+
+def generator_raises_stop_iteration_subclass():
+    yield 1
+    raise CustomStopIteration
+
+
+def generator_raises_stop_iteration_subclass_instance():
+    yield 1
+    raise CustomStopIteration("done")
+
+
+def generator_raise_assigned_variable():
+    yield 1
+    exc = StopIteration("error")
+    raise exc  # Noncompliant
+
+
+async def async_generator_yield_from():
+    yield from range(3)
+    raise StopIteration  # Noncompliant
+
+
+def generator_yield_in_nested_loop(items, stop_early):
+    # Real-world pattern: yield inside nested for loop, raise StopIteration in elif branch
+    for item in items:
+        if isinstance(item, list):
+            for x in item:
+                yield x
+        elif stop_early:
+            raise StopIteration  # Noncompliant
+        else:
+            yield item