SONARPY-3832 Fix S905 false positive for bare Airflow operator names at module level (#985)

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

Author: 2 people

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

@@ -17,24 +17,23 @@
 package org.sonar.python.checks;
 
 import java.util.Arrays;
-import java.util.HashSet;
 import java.util.List;
-import java.util.Objects;
 import java.util.Optional;
-import java.util.Set;
 import java.util.stream.Stream;
 import org.sonar.check.Rule;
 import org.sonar.check.RuleProperty;
 import org.sonar.plugins.python.api.PythonSubscriptionCheck;
 import org.sonar.plugins.python.api.SubscriptionContext;
-import org.sonar.plugins.python.api.symbols.ClassSymbol;
-import org.sonar.plugins.python.api.symbols.FunctionSymbol;
-import org.sonar.plugins.python.api.symbols.Symbol;
-import org.sonar.plugins.python.api.symbols.Usage;
+import org.sonar.plugins.python.api.symbols.v2.SymbolV2;
+import org.sonar.plugins.python.api.symbols.v2.UsageV2;
+import org.sonar.plugins.python.api.types.v2.FunctionType;
+import org.sonar.plugins.python.api.types.v2.matchers.TypeMatcher;
+import org.sonar.plugins.python.api.types.v2.matchers.TypeMatchers;
 import org.sonar.plugins.python.api.tree.BinaryExpression;
 import org.sonar.plugins.python.api.tree.CallExpression;
 import org.sonar.plugins.python.api.tree.ClassDef;
 import org.sonar.plugins.python.api.tree.ConditionalExpression;
+import org.sonar.plugins.python.api.tree.Decorator;
 import org.sonar.plugins.python.api.tree.Expression;
 import org.sonar.plugins.python.api.tree.FileInput;
 import org.sonar.plugins.python.api.tree.FunctionDef;
@@ -87,10 +86,22 @@ private List<String> ignoredOperators() {
 
   private static final List<Kind> unaryExpressionKinds = Arrays.asList(Kind.UNARY_PLUS, Kind.UNARY_MINUS, Kind.BITWISE_COMPLEMENT, Kind.NOT);
 
-  private static final Set<String> ignoredContexts = new HashSet<>(List.of("contextlib.suppress"));
-
   private static final String MESSAGE = "Remove or refactor this statement; it has no side effects.";
 
+  private static final TypeMatcher EXCEPTION_CLASS_TYPE_MATCHER = TypeMatchers.isOrExtendsType("builtins.BaseException");
+
+  private static final TypeMatcher CONTEXTLIB_SUPPRESS_TYPE_MATCHER = TypeMatchers.isType("contextlib.suppress");
+
+  private static final TypeMatcher AIRFLOW_TYPE_MATCHER = TypeMatchers.any(
+    TypeMatchers.isObjectInstanceOf("airflow.models.baseoperator.BaseOperator"),
+    TypeMatchers.isObjectInstanceOf("airflow.models.dag.DAG"),
+    TypeMatchers.isObjectInstanceOf("airflow.models.taskmixin.DependencyMixin")
+  );
+
+  private static final TypeMatcher DAG_CLASS_TYPE_MATCHER = TypeMatchers.isType("airflow.models.dag.DAG");
+
+  private static final TypeMatcher DAG_DECORATOR_TYPE_MATCHER = TypeMatchers.isType("airflow.decorators.dag");
+
   @Override
   public void initialize(Context context) {
     context.registerSyntaxNodeConsumer(Kind.STRING_LITERAL, this::checkStringLiteral);
@@ -118,56 +129,55 @@ private static void checkNode(SubscriptionContext ctx) {
     if (parent == null || !parent.is(Kind.EXPRESSION_STMT)) {
       return;
     }
-    if (isWithinIgnoredContext(tree)) {
+    if (isWithinIgnoredContext(tree, ctx)) {
       return;
     }
     // Safe cast because the rule only subscribes to expressions
-    if (isAnAirflowException((Expression) tree)) {
+    if (isAnAirflowException((Expression) tree, ctx)) {
       return;
     }
     ctx.addIssue(tree, MESSAGE);
   }
 
-  private static boolean isAnAirflowException(Expression expression) {
-    if (isWithinAirflowContext(expression)) {
+  private static boolean isAnAirflowException(Expression expression, SubscriptionContext ctx) {
+    if (isWithinAirflowContext(expression, ctx)) {
       StatementList statementList = (StatementList) TreeUtils.firstAncestorOfKind(expression, Kind.STATEMENT_LIST);
       return Optional.ofNullable(statementList).map(StatementList::statements).map(statements -> statements.get(statements.size() - 1))
         .filter(lastStatement -> lastStatement.equals(TreeUtils.firstAncestorOfKind(expression, Kind.EXPRESSION_STMT))).isPresent();
     }
     return false;
   }
 
-  private static boolean isWithinIgnoredContext(Tree tree) {
+  private static boolean isWithinIgnoredContext(Tree tree, SubscriptionContext ctx) {
     Tree withParent = TreeUtils.firstAncestorOfKind(tree, Kind.WITH_STMT);
     if (withParent != null) {
       WithStatement withStatement = (WithStatement) withParent;
       return withStatement.withItems().stream()
         .map(WithItem::test)
         .filter(item -> item.is(Kind.CALL_EXPR))
-        .map(item -> ((CallExpression) item).calleeSymbol())
-        .filter(Objects::nonNull)
-        .anyMatch(s -> ignoredContexts.contains(s.fullyQualifiedName()));
+        .map(item -> ((CallExpression) item).callee())
+        .anyMatch(callee -> CONTEXTLIB_SUPPRESS_TYPE_MATCHER.isTrueFor(callee, ctx));
     }
     return false;
   }
 
-  private static boolean isWithinAirflowContext(Tree tree) {
+  private static boolean isWithinAirflowContext(Tree tree, SubscriptionContext ctx) {
     Tree withParent = TreeUtils.firstAncestorOfKind(tree, Kind.WITH_STMT);
     while (withParent != null) {
       WithStatement withStatement = (WithStatement) withParent;
       if (withStatement.withItems().stream()
         .map(WithItem::test)
         .filter(item -> item.is(Kind.CALL_EXPR))
-        .map(item -> ((CallExpression) item).calleeSymbol())
-        .filter(Objects::nonNull)
-        .anyMatch(s -> "airflow.DAG".equals(s.fullyQualifiedName()))) {
+        .map(item -> ((CallExpression) item).callee())
+        .anyMatch(callee -> DAG_CLASS_TYPE_MATCHER.isTrueFor(callee, ctx))) {
         return true;
       }
       withParent = TreeUtils.firstAncestorOfKind(withParent, Kind.WITH_STMT);
     }
     FunctionDef funcParent = (FunctionDef) TreeUtils.firstAncestorOfKind(tree, Kind.FUNCDEF);
-    return funcParent != null && funcParent.decorators().stream().map(deco -> TreeUtils.getSymbolFromTree(deco.expression())).filter(Optional::isPresent)
-      .anyMatch(symbol -> "airflow.decorators.dag".equals(symbol.get().fullyQualifiedName()));
+    return funcParent != null && funcParent.decorators().stream()
+      .map(Decorator::expression)
+      .anyMatch(expr -> DAG_DECORATOR_TYPE_MATCHER.isTrueFor(expr, ctx));
   }
 
   private static boolean isBooleanExpressionWithCalls(Tree tree) {
@@ -192,25 +202,25 @@ private void checkStringLiteral(SubscriptionContext ctx) {
 
   private static void checkName(SubscriptionContext ctx) {
     Name name = (Name) ctx.syntaxNode();
-    Symbol symbol = name.symbol();
-    if (symbol != null && symbol.is(Symbol.Kind.CLASS)) {
-      ClassSymbol classSymbol = (ClassSymbol) symbol;
-      // Creating an exception without raising it is covered by S3984
-      if (classSymbol.canBeOrExtend("BaseException")) {
-        return;
-      }
+    // Creating an exception without raising it is covered by S3984
+    if (EXCEPTION_CLASS_TYPE_MATCHER.isTrueFor(name, ctx)) {
+      return;
+    }
+    // Avoid raising on useless statements made to suppress issues due to "unused" import
+    SymbolV2 symbolV2 = name.symbolV2();
+    if (symbolV2 != null && symbolV2.usages().stream().anyMatch(u -> u.kind() == UsageV2.Kind.IMPORT) && symbolV2.usages().size() == 2) {
+      return;
     }
-    if (symbol != null && symbol.usages().stream().anyMatch(u -> u.kind().equals(Usage.Kind.IMPORT)) && symbol.usages().size() == 2) {
-      // Avoid raising on useless statements made to suppress issues due to "unused" import
+    if (AIRFLOW_TYPE_MATCHER.isTrueFor(name, ctx)) {
       return;
     }
     checkNode(ctx);
   }
 
   private static void checkQualifiedExpression(SubscriptionContext ctx) {
     QualifiedExpression qualifiedExpression = (QualifiedExpression) ctx.syntaxNode();
-    Symbol symbol = qualifiedExpression.symbol();
-    if (symbol != null && symbol.is(Symbol.Kind.FUNCTION) && ((FunctionSymbol) symbol).decorators().stream().noneMatch(d -> d.matches("property"))) {
+    // Only raise on functions; properties are already resolved to their return type by the type inference engine
+    if (qualifiedExpression.typeV2() instanceof FunctionType) {
       checkNode(ctx);
     }
   }

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

@@ -6,6 +6,8 @@ class MyClass:
 
     def method(self): ...
 
+    def __eq__(self, other): ...
+
     @classmethod
     def class_method(cls): ...
 
@@ -149,7 +151,7 @@ def airflow_ignore_context_false_negative():
         ping = HttpOperator(endpoint="http://example.com/update/")
         download = HttpOperator(endpoint="http://example.com/download/")
         upload = HttpOperator(endpoint="http://example.com/upload/")
-        ping # Noncompliant
+        ping
         download >> upload
 
 from airflow.decorators import dag
@@ -165,7 +167,7 @@ def airflow_decorator_multiple_statements():
     download = HttpOperator(endpoint="http://example.com/download/")
     upload = HttpOperator(endpoint="http://example.com/upload/")
     download >> upload
-    ping # Noncompliant
+    ping
     ping >> upload
 
 def airflow_nested_with():
@@ -174,7 +176,7 @@ def airflow_nested_with():
             ping = HttpOperator(endpoint="http://example.com/update/")
             download = HttpOperator(endpoint="http://example.com/download/")
             upload = HttpOperator(endpoint="http://example.com/upload/")
-            ping # Noncompliant
+            ping
             download >> upload
 
 @dag
@@ -189,3 +191,19 @@ def airflow_ignore_context_not_operator():
         some_other_var = True
         some_var # Noncompliant
         some_other_var
+
+from airflow.providers.amazon.aws.operators.ecs import EcsRunTaskOperator
+from airflow.models.baseoperator import BaseOperator as AirflowBaseOperator
+
+_module_dag = DAG(dag_id="example_dag", schedule_interval=None)
+_ecs_task = EcsRunTaskOperator(task_id="generate_yaml", dag=_module_dag)
+_ecs_task
+
+_http_task = HttpOperator(task_id="http", endpoint="/", dag=_module_dag)
+_http_task
+
+class _CustomOperator(AirflowBaseOperator):
+    def execute(self, context): pass
+
+_custom_task = _CustomOperator(task_id="custom", dag=_module_dag)
+_custom_task

python-frontend/src/main/resources/org/sonar/python/types/custom_protobuf/airflow.decorators.protobuf

@@ -0,0 +1,16 @@
+
+airflow.decorators�

+dagairflow.decorators.dag"K
+CallableType[builtins.function]&
+builtins.function"builtins.function*
+args
+Any*
+kwargs
+Any*s
+__path__airflow.decorators.__path__J
+builtins.list[builtins.str]
+builtins.str"builtins.str"builtins.list*�

+__annotations__"airflow.decorators.__annotations__W
+builtins.dict[builtins.str,Any]
+builtins.str"builtins.str
+Any"builtins.dict

python-frontend/src/main/resources/org/sonar/python/types/custom_protobuf/airflow.models.baseoperator.protobuf

@@ -0,0 +1,15 @@
+
+airflow.models.baseoperator�
+BaseOperator(airflow.models.baseoperator.BaseOperator"*SonarPythonAnalyzerFakeStub.CustomStubBase*�

+__init__1airflow.models.baseoperator.BaseOperator.__init__"
+None*^
+self
T
+(airflow.models.baseoperator.BaseOperator"(airflow.models.baseoperator.BaseOperator*
+args
+Any*
+kwargs
+Any*�

+__annotations__+airflow.models.baseoperator.__annotations__W
+builtins.dict[builtins.str,Any]
+builtins.str"builtins.str
+Any"builtins.dict

python-frontend/src/main/resources/org/sonar/python/types/custom_protobuf/airflow.models.dag.protobuf

@@ -0,0 +1,15 @@
+
+airflow.models.dag�

+DAGairflow.models.dag.DAG"*SonarPythonAnalyzerFakeStub.CustomStubBase*�

+__init__airflow.models.dag.DAG.__init__"
+None*:
+self
0
+airflow.models.dag.DAG"airflow.models.dag.DAG*
+args
+Any*
+kwargs
+Any*�

+__annotations__"airflow.models.dag.__annotations__W
+builtins.dict[builtins.str,Any]
+builtins.str"builtins.str
+Any"builtins.dict

python-frontend/src/main/resources/org/sonar/python/types/custom_protobuf/airflow.models.protobuf

@@ -0,0 +1,36 @@
+
+airflow.models�
+BaseOperator(airflow.models.baseoperator.BaseOperator"*SonarPythonAnalyzerFakeStub.CustomStubBase*�

+__init__1airflow.models.baseoperator.BaseOperator.__init__"
+None*^
+self
T
+(airflow.models.baseoperator.BaseOperator"(airflow.models.baseoperator.BaseOperator*
+args
+Any*
+kwargs
+Any�

+DAGairflow.models.dag.DAG"*SonarPythonAnalyzerFakeStub.CustomStubBase*�

+__init__airflow.models.dag.DAG.__init__"
+None*:
+self
0
+airflow.models.dag.DAG"airflow.models.dag.DAG*
+args
+Any*
+kwargs
+Any�
+DependencyMixin(airflow.models.taskmixin.DependencyMixin"*SonarPythonAnalyzerFakeStub.CustomStubBase*�

+__init__1airflow.models.taskmixin.DependencyMixin.__init__"
+None*^
+self
T
+(airflow.models.taskmixin.DependencyMixin"(airflow.models.taskmixin.DependencyMixin*
+args
+Any*
+kwargs
+Any*o
+__path__airflow.models.__path__J
+builtins.list[builtins.str]
+builtins.str"builtins.str"builtins.list*�

+__annotations__airflow.models.__annotations__W
+builtins.dict[builtins.str,Any]
+builtins.str"builtins.str
+Any"builtins.dict

python-frontend/src/main/resources/org/sonar/python/types/custom_protobuf/airflow.models.taskmixin.protobuf

@@ -0,0 +1,15 @@
+
+airflow.models.taskmixin�
+DependencyMixin(airflow.models.taskmixin.DependencyMixin"*SonarPythonAnalyzerFakeStub.CustomStubBase*�

+__init__1airflow.models.taskmixin.DependencyMixin.__init__"
+None*^
+self
T
+(airflow.models.taskmixin.DependencyMixin"(airflow.models.taskmixin.DependencyMixin*
+args
+Any*
+kwargs
+Any*�

+__annotations__(airflow.models.taskmixin.__annotations__W
+builtins.dict[builtins.str,Any]
+builtins.str"builtins.str
+Any"builtins.dict

python-frontend/src/main/resources/org/sonar/python/types/custom_protobuf/airflow.protobuf

@@ -0,0 +1,27 @@
+
+airflow�

+DAGairflow.models.dag.DAG"*SonarPythonAnalyzerFakeStub.CustomStubBase*�

+__init__airflow.models.dag.DAG.__init__"
+None*:
+self
0
+airflow.models.dag.DAG"airflow.models.dag.DAG*
+args
+Any*
+kwargs
+Any�
+BaseOperator(airflow.models.baseoperator.BaseOperator"*SonarPythonAnalyzerFakeStub.CustomStubBase*�

+__init__1airflow.models.baseoperator.BaseOperator.__init__"
+None*^
+self
T
+(airflow.models.baseoperator.BaseOperator"(airflow.models.baseoperator.BaseOperator*
+args
+Any*
+kwargs
+Any*h
+__path__airflow.__path__J
+builtins.list[builtins.str]
+builtins.str"builtins.str"builtins.list*�

+__annotations__airflow.__annotations__W
+builtins.dict[builtins.str,Any]
+builtins.str"builtins.str
+Any"builtins.dict

python-frontend/src/main/resources/org/sonar/python/types/custom_protobuf/airflow.providers.amazon.aws.operators.ecs.protobuf

@@ -0,0 +1,15 @@
+
+*airflow.providers.amazon.aws.operators.ecs�
+EcsRunTaskOperator=airflow.providers.amazon.aws.operators.ecs.EcsRunTaskOperator"(airflow.models.baseoperator.BaseOperator*�
+__init__Fairflow.providers.amazon.aws.operators.ecs.EcsRunTaskOperator.__init__"
+None*�

+self
~
+=airflow.providers.amazon.aws.operators.ecs.EcsRunTaskOperator"=airflow.providers.amazon.aws.operators.ecs.EcsRunTaskOperator*
+args
+Any*
+kwargs
+Any*�

+__annotations__:airflow.providers.amazon.aws.operators.ecs.__annotations__W
+builtins.dict[builtins.str,Any]
+builtins.str"builtins.str
+Any"builtins.dict

python-frontend/src/main/resources/org/sonar/python/types/custom_protobuf/airflow.providers.amazon.aws.operators.protobuf

@@ -0,0 +1,18 @@
+
+&airflow.providers.amazon.aws.operators�
+EcsRunTaskOperator=airflow.providers.amazon.aws.operators.ecs.EcsRunTaskOperator"(airflow.models.baseoperator.BaseOperator*�
+__init__Fairflow.providers.amazon.aws.operators.ecs.EcsRunTaskOperator.__init__"
+None*�

+self
~
+=airflow.providers.amazon.aws.operators.ecs.EcsRunTaskOperator"=airflow.providers.amazon.aws.operators.ecs.EcsRunTaskOperator*
+args
+Any*
+kwargs
+Any*�

+__path__/airflow.providers.amazon.aws.operators.__path__J
+builtins.list[builtins.str]
+builtins.str"builtins.str"builtins.list*�

+__annotations__6airflow.providers.amazon.aws.operators.__annotations__W
+builtins.dict[builtins.str,Any]
+builtins.str"builtins.str
+Any"builtins.dict