SONARPY-3424 S112: Raise when passing a newly constructed Exception or BaseException to function (#935)

GitOrigin-RevId: 41f1ed016cc89b82303afa1f2ef50dca4645e55e

Author: thomas-serre-sonarsource

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

@@ -19,11 +19,15 @@
 import java.util.List;
 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.v2.SymbolV2;
 import org.sonar.plugins.python.api.symbols.v2.UsageV2;
+import org.sonar.plugins.python.api.tree.Argument;
+import org.sonar.plugins.python.api.tree.CallExpression;
 import org.sonar.plugins.python.api.tree.Expression;
 import org.sonar.plugins.python.api.tree.Name;
 import org.sonar.plugins.python.api.tree.RaiseStatement;
+import org.sonar.plugins.python.api.tree.RegularArgument;
 import org.sonar.plugins.python.api.tree.Tree;
 import org.sonar.plugins.python.api.tree.Tree.Kind;
 import org.sonar.plugins.python.api.types.v2.matchers.TypeMatcher;
@@ -36,42 +40,65 @@
 @Rule(key = "S112")
 public class GenericExceptionRaisedCheck extends PythonSubscriptionCheck {
 
+  private static final String MESSAGE = "Replace this generic exception class with a more specific one.";
+
   private final TypeMatcher isExceptionOrBaseExceptionMatcher = TypeMatchers.any(
     TypeMatchers.isObjectOfType(EXCEPTION),
     TypeMatchers.isObjectOfType(BASE_EXCEPTION),
     TypeMatchers.isType(EXCEPTION),
     TypeMatchers.isType(BASE_EXCEPTION)
   );
 
+  private final TypeMatcher isObjectOfTypeExceptionOrBaseExceptionMatcher = TypeMatchers.any(
+    TypeMatchers.isObjectOfType(EXCEPTION),
+    TypeMatchers.isObjectOfType(BASE_EXCEPTION)
+  );
+
   @Override
   public void initialize(Context context) {
-    context.registerSyntaxNodeConsumer(Kind.RAISE_STMT, ctx -> {
-      RaiseStatement raise = (RaiseStatement) ctx.syntaxNode();
-      List<Expression> expressions = raise.expressions();
-      if (expressions.isEmpty()) {
-        return;
-      }
+    context.registerSyntaxNodeConsumer(Kind.RAISE_STMT, this::checkRaise);
+    context.registerSyntaxNodeConsumer(Kind.CALL_EXPR, this::checkFunctionCall);
+  }
+
+  private void checkRaise(SubscriptionContext ctx) {
+    RaiseStatement raise = (RaiseStatement) ctx.syntaxNode();
+    List<Expression> expressions = raise.expressions();
+    if (expressions.isEmpty()) {
+      return;
+    }
 
-      Expression expression = expressions.get(0);
-      if (!isExceptionOrBaseExceptionMatcher.isTrueFor(expression, ctx)) {
-        return;
+    Expression expression = expressions.get(0);
+    if (!isExceptionOrBaseExceptionMatcher.isTrueFor(expression, ctx)) {
+      return;
+    }
+    if (!isExceptionFunctionLocal(expression, raise)) {
+      return;
+    }
+    ctx.addIssue(expression, MESSAGE);
+  }
+
+  private void checkFunctionCall(SubscriptionContext ctx) {
+    CallExpression call = (CallExpression) ctx.syntaxNode();
+    List<Argument> arguments = call.arguments();
+    for (Argument arg : arguments) {
+      if (!(arg instanceof RegularArgument regArg) || regArg.keywordArgument() != null) {
+        continue;
       }
-      if (!isExceptionFunctionLocal(expression, raise)) {
-        return;
+      Expression argExpr = regArg.expression();
+      if (isObjectOfTypeExceptionOrBaseExceptionMatcher.isTrueFor(argExpr, ctx) && isExceptionFunctionLocal(argExpr, call)) {
+        ctx.addIssue(argExpr, MESSAGE);
       }
-
-      ctx.addIssue(expression, "Replace this generic exception class with a more specific one.");
-    });
+    }
   }
 
-  private static boolean isExceptionFunctionLocal(Expression expression, RaiseStatement raise) {
+  private static boolean isExceptionFunctionLocal(Expression expression, Tree contextTree) {
     if (!(expression instanceof Name name)) return true;
     SymbolV2 symbolV2 = name.symbolV2();
-    return symbolV2 == null || isLocalVariable(symbolV2, raise);
+    return symbolV2 == null || isLocalVariable(symbolV2, contextTree);
   }
 
-  private static boolean isLocalVariable(SymbolV2 symbol, Tree raiseStatement) {
-    Tree function = TreeUtils.firstAncestorOfKind(raiseStatement, Kind.FUNCDEF);
+  private static boolean isLocalVariable(SymbolV2 symbol, Tree contextTree) {
+    Tree function = TreeUtils.firstAncestorOfKind(contextTree, Kind.FUNCDEF);
     if (function == null) {
       return false;
     }

python-checks/src/test/resources/checks/genericException/genericExceptionRaised.py

@@ -55,3 +55,16 @@ def raised_exception_is_the_parameter(exception: BaseException):
 global_exception = BaseException()
 def raise_global_exception():
     raise global_exception
+
+def handle_exception(e: Exception):
+    raise e
+
+def intermediate_handle_exception(e: Exception):
+    handle_exception(e)
+
+def constructed_exceptions_passed_to_handle_exception_raise():
+    handle_exception(Exception()) # Noncompliant
+    handle_exception(BaseException()) # Noncompliant
+
+def is_instance_do_not_raise(e):
+    isInstance(e, Exception)