SONARPY-3833 Fix S8411 false positive when path parameter is used in Depends() (#986)

Co-authored-by: Vibe Bot <vibe-bot@sonarsource.com>
GitOrigin-RevId: b2dd5520b1acce2f16f12f7814e0ed777fb3129a

Author: 2 people

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

@@ -18,6 +18,7 @@
 
 import java.util.HashSet;
 import java.util.List;
+import java.util.Objects;
 import java.util.Optional;
 import java.util.Set;
 import java.util.regex.Matcher;
@@ -26,11 +27,22 @@
 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.CallExpression;
 import org.sonar.plugins.python.api.tree.Decorator;
 import org.sonar.plugins.python.api.tree.Expression;
 import org.sonar.plugins.python.api.tree.FunctionDef;
+import org.sonar.plugins.python.api.tree.Name;
+import org.sonar.plugins.python.api.tree.Parameter;
+import org.sonar.plugins.python.api.tree.ParameterList;
+import org.sonar.plugins.python.api.tree.QualifiedExpression;
+import org.sonar.plugins.python.api.tree.RegularArgument;
+import org.sonar.plugins.python.api.tree.SubscriptionExpression;
 import org.sonar.plugins.python.api.tree.Tree;
+import org.sonar.plugins.python.api.tree.TypeAnnotation;
+import org.sonar.plugins.python.api.types.v2.FunctionType;
+import org.sonar.plugins.python.api.types.v2.ParameterV2;
 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;
@@ -56,6 +68,9 @@ public class FastAPIPathParametersCheck extends PythonSubscriptionCheck {
         TypeMatchers.isType("fastapi.APIRouter." + method)))
   );
 
+  private static final TypeMatcher FASTAPI_DEPENDS_MATCHER = TypeMatchers.isType("fastapi.param_functions.Depends");
+  private static final TypeMatcher TYPING_ANNOTATED_MATCHER = TypeMatchers.isType("typing.Annotated");
+
   @Override
   public void initialize(Context context) {
     context.registerSyntaxNodeConsumer(Tree.Kind.FUNCDEF, FastAPIPathParametersCheck::checkFunction);
@@ -84,9 +99,89 @@ private static void checkDecorator(SubscriptionContext ctx, Decorator decorator,
     }
 
     FunctionParameterInfo paramInfo = FunctionParameterUtils.extractFunctionParameters(functionDef);
+    pathParams.removeAll(extractDependencyParameters(functionDef, ctx));
     reportIssues(ctx, functionDef, pathParams, paramInfo);
   }
 
+  private static Set<String> extractDependencyParameters(FunctionDef functionDef, SubscriptionContext ctx) {
+    return extractDependencyParameters(functionDef, ctx, new HashSet<>());
+  }
+
+  private static Set<String> extractDependencyParameters(FunctionDef functionDef, SubscriptionContext ctx, Set<FunctionDef> visited) {
+    if (!visited.add(functionDef)) {
+      return Set.of();
+    }
+    ParameterList parameterList = functionDef.parameters();
+    if (parameterList == null) {
+      return Set.of();
+    }
+    Set<String> dependencyParams = new HashSet<>();
+    parameterList.nonTuple().stream()
+      .map(param -> getDependsCall(param, ctx))
+      .filter(Optional::isPresent)
+      .map(Optional::get)
+      .forEach(dependsCall -> collectDependencyParams(dependsCall, dependencyParams, visited, ctx));
+    return dependencyParams;
+  }
+
+  private static Optional<CallExpression> getDependsCall(Parameter param, SubscriptionContext ctx) {
+    Expression defaultValue = param.defaultValue();
+    if (defaultValue instanceof CallExpression callExpr && FASTAPI_DEPENDS_MATCHER.isTrueFor(callExpr.callee(), ctx)) {
+      return Optional.of(callExpr);
+    }
+    TypeAnnotation typeAnnotation = param.typeAnnotation();
+    if (typeAnnotation != null && typeAnnotation.expression() instanceof SubscriptionExpression subscriptionExpr
+      && TYPING_ANNOTATED_MATCHER.isTrueFor(subscriptionExpr.object(), ctx)) {
+      return subscriptionExpr.subscripts().expressions().stream()
+        .filter(e -> e instanceof CallExpression ce && FASTAPI_DEPENDS_MATCHER.isTrueFor(ce.callee(), ctx))
+        .map(e -> (CallExpression) e)
+        .findFirst();
+    }
+    return Optional.empty();
+  }
+
+  private static void collectDependencyParams(CallExpression dependsCall, Set<String> dependencyParams, Set<FunctionDef> visited, SubscriptionContext ctx) {
+    TreeUtils.nthArgumentOrKeywordOptional(0, "dependency", dependsCall.arguments())
+      .map(RegularArgument::expression)
+      .ifPresent(argExpr -> {
+        if (argExpr.typeV2() instanceof FunctionType funcType) {
+          funcType.parameters().stream()
+            .filter(param -> !param.isVariadic())
+            .map(ParameterV2::name)
+            .filter(Objects::nonNull)
+            .forEach(dependencyParams::add);
+        }
+        getFunctionDef(argExpr).ifPresent(depFuncDef ->
+          dependencyParams.addAll(extractDependencyParameters(depFuncDef, ctx, visited))
+        );
+      });
+  }
+
+  private static Optional<FunctionDef> getFunctionDef(Expression expression) {
+    Name name;
+    if (expression instanceof Name n) {
+      name = n;
+    } else if (expression instanceof QualifiedExpression qe) {
+      name = qe.name();
+    } else {
+      name = null;
+    }
+    if (name == null) {
+      return Optional.empty();
+    }
+    SymbolV2 symbol = name.symbolV2();
+    if (symbol == null) {
+      return Optional.empty();
+    }
+    return symbol.usages().stream()
+      .filter(u -> u.kind() == UsageV2.Kind.FUNC_DECLARATION)
+      .map(UsageV2::tree)
+      .map(tree -> TreeUtils.firstAncestorOfKind(tree, Tree.Kind.FUNCDEF))
+      .filter(Objects::nonNull)
+      .map(FunctionDef.class::cast)
+      .findFirst();
+  }
+
   private static Set<String> extractPathParameters(CallExpression callExpr) {
     Set<String> pathParams = new HashSet<>();
     String pathString = getPathArgument(callExpr).orElse("");

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

@@ -1,4 +1,6 @@
-from fastapi import FastAPI, APIRouter
+import typing
+from fastapi import FastAPI, APIRouter, Depends
+from typing import Annotated
 
 app = FastAPI()
 router = APIRouter()
@@ -167,3 +169,91 @@ def compliant_empty_path():
 @app.get(True)
 def compliant_path_is_not_a_string():
     pass
+
+# --- Depends() path parameter delegation ---
+
+def get_item(item_id: int):
+    return {"item_id": item_id}
+
+@router.get("/items/{item_id}")
+def compliant_depends_default_value(item=Depends(get_item)):
+    return item
+
+@app.get("/items/{item_id}")
+def compliant_depends_annotated(item: Annotated[dict, Depends(get_item)]):
+    return item
+
+@app.get("/users/{user_id}/items/{item_id}")
+def compliant_depends_partial(user_id: int, item=Depends(get_item)):
+    return {"user_id": user_id, "item": item}
+
+async def get_item_async(item_id: int):
+    return {"item_id": item_id}
+
+@router.get("/items/{item_id}")
+def compliant_depends_async_dependency(item=Depends(get_item_async)):
+    return item
+
+def dep_without_item_id(name: str):
+    return name
+
+@app.get("/items/{item_id}")
+def noncompliant_depends_param_not_in_dep(item=Depends(dep_without_item_id)):  # Noncompliant {{Add path parameter "item_id" to the function signature.}}
+    return item
+
+@app.get("/items/{item_id}")
+def compliant_depends_annotated_qualified(item: typing.Annotated[dict, Depends(get_item)]):
+    return item
+
+# --- Nested/recursive Depends() ---
+
+def get_item_wrapper(item=Depends(get_item)):
+    return item
+
+@router.get("/items/{item_id}")
+def compliant_nested_depends(item=Depends(get_item_wrapper)):
+    return item
+
+def get_item_annotated_wrapper(item: Annotated[dict, Depends(get_item)]):
+    return item
+
+@app.get("/items/{item_id}")
+def compliant_nested_depends_annotated(item=Depends(get_item_annotated_wrapper)):
+    return item
+
+def dep_level2_no_id(name: str):
+    return name
+
+def dep_level1_no_id(x=Depends(dep_level2_no_id)):
+    return x
+
+@app.get("/items/{item_id}")
+def noncompliant_nested_depends_not_covering(item=Depends(dep_level1_no_id)):  # Noncompliant {{Add path parameter "item_id" to the function signature.}}
+    return item
+
+# --- Circular dependency detection (cycle in Depends chain) ---
+# dep_b_circular references dep_a_circular (defined below) - tests cycle detection at line 112
+def dep_b_circular(dep=Depends(dep_a_circular)):
+    return dep
+
+def dep_a_circular(item_id: int, dep=Depends(dep_b_circular)):
+    return item_id
+
+@app.get("/items/{item_id}")
+def compliant_circular_depends(dep=Depends(dep_a_circular)):
+    return dep
+
+# --- QualifiedExpression as Depends argument (tests lines 164-165 and 174) ---
+class ItemDepsHolder:
+    @staticmethod
+    def get_item(item_id: int):
+        return item_id
+
+@app.get("/items/{item_id}")
+def compliant_qualified_expr_depends(dep=Depends(ItemDepsHolder.get_item)):
+    return dep
+
+# --- Non-Name/non-QualifiedExpression as Depends argument (tests lines 167 and 170) ---
+@app.get("/items/{item_id}")
+def noncompliant_lambda_depends(dep=Depends(lambda: None)):  # Noncompliant {{Add path parameter "item_id" to the function signature.}}
+    return dep