SONARPY-3710 S8515: use call graph

GitOrigin-RevId: bf2c593caee0a4ade07614a85ea7c9a9e2600f30

Author: Seppli11

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

@@ -38,12 +38,13 @@
 import org.sonar.plugins.python.api.tree.NumericLiteral;
 import org.sonar.plugins.python.api.tree.RaiseStatement;
 import org.sonar.plugins.python.api.tree.RegularArgument;
-import org.sonar.plugins.python.api.tree.StatementList;
 import org.sonar.plugins.python.api.tree.StringLiteral;
 import org.sonar.plugins.python.api.tree.Tree;
+import org.sonar.plugins.python.api.types.v2.FullyQualifiedNameHelper;
 import org.sonar.plugins.python.api.types.v2.matchers.TypeMatcher;
 import org.sonar.plugins.python.api.types.v2.matchers.TypeMatchers;
 import org.sonar.python.checks.utils.Expressions;
+import org.sonar.python.semantic.v2.callgraph.CallGraph;
 import org.sonar.python.tree.TreeUtils;
 
 @Rule(key = "S8415")
@@ -65,14 +66,21 @@ public class FastAPIHTTPExceptionDocumentedCheck extends PythonSubscriptionCheck
     TypeMatchers.isType("fastapi.exceptions.HTTPException"),
     TypeMatchers.isType("fastapi.HTTPException"));
 
-  private static final int MAX_RECURSION_DEPTH = 5;
+  private static final int MAX_FUNCTION_CALLS = 100;
+
+  private final Set<Expression> reportedHttpExceptionCalls = new HashSet<>();
 
   @Override
   public void initialize(Context context) {
-    context.registerSyntaxNodeConsumer(Tree.Kind.FUNCDEF, FastAPIHTTPExceptionDocumentedCheck::checkFunctionDef);
+    context.registerSyntaxNodeConsumer(Tree.Kind.FILE_INPUT, this::init);
+    context.registerSyntaxNodeConsumer(Tree.Kind.FUNCDEF, this::checkFunctionDef);
+  }
+
+  private void init(SubscriptionContext ctx) {
+    reportedHttpExceptionCalls.clear();
   }
 
-  private static void checkFunctionDef(SubscriptionContext ctx) {
+  private void checkFunctionDef(SubscriptionContext ctx) {
     FunctionDef functionDef = (FunctionDef) ctx.syntaxNode();
 
     DecoratorAnalysisResult analysisResult = new DecoratorAnalysis(ctx, functionDef).analyze();
@@ -146,16 +154,16 @@ private static Set<Integer> extractDocumentedStatusCodes(Expression responsesExp
 
       return statusCodes;
     }
-
   }
 
-  private static void reportUndocumentedExceptions(
+  private void reportUndocumentedExceptions(
     SubscriptionContext ctx,
     List<RaiseInfo> httpExceptions,
     Set<Integer> documentedStatusCodes) {
     for (RaiseInfo raiseInfo : httpExceptions) {
-      if (!documentedStatusCodes.contains(raiseInfo.statusCode)) {
+      if (!documentedStatusCodes.contains(raiseInfo.statusCode) && !reportedHttpExceptionCalls.contains(raiseInfo.httpExceptionExpression)) {
         ctx.addIssue(raiseInfo.httpExceptionExpression, String.format(MESSAGE, raiseInfo.statusCode));
+        reportedHttpExceptionCalls.add(raiseInfo.httpExceptionExpression);
       }
     }
   }
@@ -169,33 +177,27 @@ private record DecoratorAnalysisResult(
   private static class RaiseInfoCollector {
     private final SubscriptionContext ctx;
     private final FunctionDef functionDef;
-    private final Set<FunctionDef> visited = new HashSet<>();
 
     RaiseInfoCollector(SubscriptionContext ctx, FunctionDef functionDef) {
       this.ctx = ctx;
       this.functionDef = functionDef;
     }
 
     public List<RaiseInfo> collect() {
-      return collect(functionDef, 0);
-    }
-
-    private List<RaiseInfo> collect(FunctionDef functionDef, int depth) {
-      List<RaiseInfo> result = new ArrayList<>();
+      List<RaiseInfo> result = new ArrayList<>(HTTPExceptionVisitor.collect(ctx, functionDef));
 
-      if (visited.contains(functionDef) || depth > MAX_RECURSION_DEPTH) {
+      String fqn = FullyQualifiedNameHelper.getFullyQualifiedName(functionDef.name().typeV2()).orElse(null);
+      if (fqn == null) {
         return result;
       }
-      visited.add(functionDef);
 
-      StatementList body = functionDef.body();
-      if (body == null) {
-        return result;
-      }
+      CallGraph callGraph = ctx.callGraph();
 
-      HTTPExceptionVisitor visitor = new HTTPExceptionVisitor(ctx);
-      body.accept(visitor);
-      result.addAll(visitor.httpExceptions);
+      callGraph.forwardStream(fqn)
+        .limit(MAX_FUNCTION_CALLS)
+        .forEach(node -> node.tree()
+          .flatMap(TreeUtils.toOptionalInstanceOfMapper(FunctionDef.class))
+          .ifPresent(calledFunction -> result.addAll(HTTPExceptionVisitor.collect(ctx, calledFunction))));
 
       return result;
     }
@@ -244,6 +246,12 @@ public void visitFunctionDef(FunctionDef pyFunctionDefTree) {
     public void visitLambda(LambdaExpression pyLambdaExpressionTree) {
       // don't decend into nested lambdas
     }
+
+    public static List<RaiseInfo> collect(SubscriptionContext ctx, FunctionDef tree) {
+      HTTPExceptionVisitor visitor = new HTTPExceptionVisitor(ctx);
+      tree.body().accept(visitor);
+      return visitor.httpExceptions;
+    }
   }
 
   private static Optional<Integer> extractStatusCode(Expression statusCodeExpr) {

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

@@ -93,7 +93,7 @@ def api_router_instead_of_app():
 # Testing following calls
 def helper_with_exception(item_id: int):
     if ...:
-        raise HTTPException(status_code=400, detail="Invalid ID")  # FN
+        raise HTTPException(status_code=400, detail="Invalid ID")  # Noncompliant
 
 
 @app.get("/items/{item_id}")
@@ -102,12 +102,12 @@ def endpoint_calling_helper(item_id: int):
 
 
 def nested_helper_level_2():
-    raise HTTPException(status_code=403, detail="Forbidden")  # FN
+    raise HTTPException(status_code=403, detail="Forbidden")  # Noncompliant
 
 
 def nested_helper_level_1(id: int):
     if ...:
-        raise HTTPException(status_code=400, detail="Bad ID")  # FN
+        raise HTTPException(status_code=400, detail="Bad ID")  # Noncompliant
     nested_helper_level_2()
 
 
@@ -268,16 +268,28 @@ def exception_without_status_code():
     raise HTTPException(detail="Missing status code")  # Compliant (no status_code to analyze)
 
 @app.get("/no-status-code")
-def exception_without_status_code():
+def exception_with_inner_function():
     def inner_function():
         raise HTTPException(detail="Missing status code")  # Compliant
 
     def inner_function2():
-        raise HTTPException(status_code=500, detail="Missing status code")  # FN
+        raise HTTPException(status_code=500, detail="Missing status code")  # Noncompliant
 
     inner_function2()
 
 
+@app.get("/no-status-code")
+def call_same_function_twice_first_call():
+    called_function()
+
+@app.get("/no-status-code")
+def call_same_function_twice_second_call():
+    called_function()
+
+
+def called_function():
+    raise HTTPException(status_code=500, detail="Missing status code")  # Noncompliant
+
 
 base_responses = {500: {"description": "Server error"}}
 @app.get("/dict-unpacking", responses={404: {"description": "Not found"}, **base_responses})
@@ -287,3 +299,4 @@ def responses_with_dict_unpacking():
 # Dummy decorator for test
 def some_other_decorator(func):
     return func
+