SONARPY-2913 Rule S7483: Asynchronous functions should not accept timeout parameters (#258)

GitOrigin-RevId: 0823c2ee0f8d8319d7d225def5f507249cc054d9

Author: guillaume-dequenne

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

@@ -0,0 +1,67 @@
+/*
+ * SonarQube Python Plugin
+ * Copyright (C) 2011-2025 SonarSource SA
+ * mailto:info AT sonarsource DOT com
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the Sonar Source-Available License Version 1, as published by SonarSource SA.
+ *
+ * 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.FunctionDef;
+import org.sonar.plugins.python.api.tree.Name;
+import org.sonar.plugins.python.api.tree.Parameter;
+import org.sonar.plugins.python.api.tree.Token;
+import org.sonar.plugins.python.api.tree.Tree;
+import org.sonar.plugins.python.api.types.v2.ClassType;
+import org.sonar.plugins.python.api.types.v2.FunctionType;
+import org.sonar.python.tree.TreeUtils;
+
+@Rule(key = "S7483")
+public class AsyncFunctionWithTimeoutCheck extends PythonSubscriptionCheck {
+
+  private static final String MESSAGE = "Remove this \"timeout\" parameter and use a timeout context manager instead.";
+
+  @Override
+  public void initialize(Context context) {
+    context.registerSyntaxNodeConsumer(Tree.Kind.FUNCDEF, AsyncFunctionWithTimeoutCheck::checkFunctionDef);
+  }
+
+  private static void checkFunctionDef(SubscriptionContext ctx) {
+    FunctionDef functionDef = (FunctionDef) ctx.syntaxNode();
+
+    // Check if the function is async using its token
+    Token asyncKeyword = functionDef.asyncKeyword();
+    if (asyncKeyword == null) {
+      return;
+    }
+
+    FunctionType functionType = (FunctionType) functionDef.name().typeV2();
+    if (mightBeOverridingMethod(functionType)) {
+      return;
+    }
+
+    for (Parameter parameter : TreeUtils.nonTupleParameters(functionDef)) {
+      Name parameterName = parameter.name();
+      if (parameterName != null && "timeout".equals(parameterName.name())) {
+        ctx.addIssue(parameter, MESSAGE).secondary(asyncKeyword, "This function is async.");
+        return;
+      }
+    }
+  }
+
+  private static boolean mightBeOverridingMethod(FunctionType functionType) {
+    return functionType.owner() instanceof ClassType classType && (classType.hasUnresolvedHierarchy() || classType.inheritedMember(functionType.name()).isPresent());
+  }
+}

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

@@ -121,6 +121,7 @@ public Stream<Class<?>> getChecks() {
       AssertOnDissimilarTypesCheck.class,
       AssertAfterRaiseCheck.class,
       AssertOnTupleLiteralCheck.class,
+      AsyncFunctionWithTimeoutCheck.class,
       BackslashInStringCheck.class,
       BackticksUsageCheck.class,
       BareRaiseInFinallyCheck.class,

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

@@ -0,0 +1,102 @@
+<p>This rule raises an issue when an asynchronous function accepts a <code>timeout</code> parameter instead of using the timeout context managers.</p>
+<h2>Why is this an issue?</h2>
+<p>Modern asynchronous Python libraries like <code>asyncio</code>, <code>anyio</code>, and <code>trio</code> promote a principle called
+<strong>structured concurrency</strong>. A key aspect of this is that the caller of an asynchronous function should be responsible for managing
+timeouts and cancellation, not the callee.</p>
+<p>When an <code>async</code> function accepts a <code>timeout</code> parameter, it violates this principle:</p>
+<ol>
+  <li> <strong>Coupling between logic and timeout handling</strong>: The function dictates how the timeout is handled internally, rather than letting
+  the caller decide. </li>
+  <li> <strong>Preempting caller control</strong>: The caller might want to enforce a different timeout duration or coordinate timeouts across several
+  concurrent operations. An internal timeout parameter makes this difficult or impossible. </li>
+  <li> <strong>Reducing composability</strong>: Combining functions that manage their own timeouts can lead to complex and unpredictable behavior,
+  especially when nesting calls or running tasks concurrently under a shared deadline. </li>
+</ol>
+<p>Instead, the caller should use the timeout features provided by the concurrency library (e.g., <code>async with asyncio.timeout()</code> or
+<code>with trio.move_on_after()</code>). This separates the concern of what the function does from how long the caller is willing to wait for it.</p>
+<h2>How to fix it in Asyncio</h2>
+<h3>Code examples</h3>
+<h4>Noncompliant code example</h4>
+<pre data-diff-id="1" data-diff-type="noncompliant">
+import asyncio
+
+async def example_function(timeout): # Noncompliant
+    await asyncio.sleep(timeout)
+
+async def main():
+    await example_function(5)
+</pre>
+<h4>Compliant solution</h4>
+<pre data-diff-id="1" data-diff-type="compliant">
+import asyncio
+
+async def example_function():
+    await asyncio.sleep(5)
+
+async def main():
+    async with asyncio.timeout(5): # Compliant
+        await example_function()
+</pre>
+<h2>How to fix it in Trio</h2>
+<h3>Code examples</h3>
+<h4>Noncompliant code example</h4>
+<pre data-diff-id="2" data-diff-type="noncompliant">
+import trio
+
+async def example_function(timeout): # Noncompliant
+    await trio.sleep(timeout)
+
+async def main():
+    await example_function(5)
+
+trio.run(main)
+</pre>
+<h4>Compliant solution</h4>
+<pre data-diff-id="2" data-diff-type="compliant">
+import trio
+
+async def example_function():
+    await trio.sleep(5)
+
+async def main():
+    with trio.move_on_after(5): # Compliant
+        await example_function()
+
+trio.run(main)
+</pre>
+<h2>How to fix it in AnyIO</h2>
+<h3>Code examples</h3>
+<h4>Noncompliant code example</h4>
+<pre data-diff-id="3" data-diff-type="noncompliant">
+import anyio
+
+async def example_function(timeout): # Noncompliant
+    await anyio.sleep(timeout)
+
+async def main():
+    await example_function(5)
+
+anyio.run(main)
+</pre>
+<h4>Compliant solution</h4>
+<pre data-diff-id="3" data-diff-type="compliant">
+import anyio
+
+async def example_function():
+    await anyio.sleep(5)
+
+async def main():
+    with anyio.move_on_after(5): # Compliant
+        await example_function()
+
+anyio.run(main)
+</pre>
+<h2>Resources</h2>
+<h3>Documentation</h3>
+<ul>
+  <li> Asyncio documentation - <a href="https://docs.python.org/3/library/asyncio-task.html#asyncio.timeout">Timeouts</a> </li>
+  <li> Trio documentation - <a href="https://trio.readthedocs.io/en/stable/reference-core.html#cancellation-and-timeouts">Cancellation and
+  timeouts</a> </li>
+  <li> AnyIO documentation - <a href="https://anyio.readthedocs.io/en/stable/cancellation.html#timeouts">Timeouts</a> </li>
+</ul>
+

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

@@ -0,0 +1,26 @@
+{
+  "title": "Asynchronous functions should not accept timeout parameters",
+  "type": "CODE_SMELL",
+  "status": "ready",
+  "remediation": {
+    "func": "Constant\/Issue",
+    "constantCost": "5min"
+  },
+  "tags": [
+    "async",
+    "asyncio",
+    "anyio",
+    "trio"
+  ],
+  "defaultSeverity": "Major",
+  "ruleSpecification": "RSPEC-7483",
+  "sqKey": "S7483",
+  "scope": "All",
+  "quickfix": "infeasible",
+  "code": {
+    "impacts": {
+      "MAINTAINABILITY": "MEDIUM"
+    },
+    "attribute": "FOCUSED"
+  }
+}

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

@@ -255,6 +255,8 @@
     "S6984",
     "S6985",
     "S7488",
-    "S7491"
+    "S7491",
+    "S7483",
+    "S7488"
   ]
 }

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

@@ -0,0 +1,29 @@
+/*
+ * SonarQube Python Plugin
+ * Copyright (C) 2011-2025 SonarSource SA
+ * mailto:info AT sonarsource DOT com
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the Sonar Source-Available License Version 1, as published by SonarSource SA.
+ *
+ * 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 AsyncFunctionWithTimeoutCheckTest {
+
+  @Test
+  void test() {
+    PythonCheckVerifier.verify("src/test/resources/checks/asyncFunctionWithTimeout.py", new AsyncFunctionWithTimeoutCheck());
+  }
+
+}

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

@@ -0,0 +1,96 @@
+import asyncio
+import trio
+import anyio
+
+
+class AsyncioClass:
+    async def method_noncompliant(self, timeout): # Noncompliant {{Remove this "timeout" parameter and use a timeout context manager instead.}}
+#                                       ^^^^^^^
+#   ^^^^^@-1< {{This function is async.}}
+        await asyncio.sleep(1)
+
+    async def method_compliant(self):
+        await asyncio.sleep(1)
+
+    def sync_method_with_timeout(self, timeout): # Compliant
+        pass
+
+# Test cases for asyncio
+async def noncompliant_simple(timeout): # Noncompliant
+    await asyncio.sleep(1)
+
+async def noncompliant_with_typehint(timeout: int): # Noncompliant
+    await asyncio.sleep(1)
+
+async def noncompliant_with_default(timeout=5): # Noncompliant
+    await asyncio.sleep(1)
+
+async def noncompliant_keyword_only(param, *, timeout): # Noncompliant
+    await asyncio.sleep(1)
+
+async def noncompliant_positional_only(timeout, /, param): # Noncompliant
+    await asyncio.sleep(1)
+
+async def timeout_different_name(custom_timeout): # OK
+    await asyncio.sleep(1)
+
+async def noncompliant_multiple_params(arg1, timeout, arg2): # Noncompliant
+    await asyncio.sleep(1)
+
+async def noncompliant_star_args(arg1, *args, timeout): # Noncompliant
+    await asyncio.sleep(1)
+
+async def noncompliant_star_kwargs(arg1, timeout, **kwargs): # Noncompliant
+    await asyncio.sleep(1)
+
+async def compliant_no_timeout_param():
+    await asyncio.sleep(1)
+
+async def compliant_timeout_handled_by_caller():
+    await asyncio.sleep(1)
+
+async def main_compliant():
+    async with asyncio.timeout(5):
+        await compliant_timeout_handled_by_caller()
+
+def sync_function_with_timeout_param(timeout): # Compliant
+    pass
+
+async def timeout_in_kwargs(**kwargs): # FN
+    duration = kwargs.get("timeout", 1)
+    await asyncio.sleep(duration)
+
+# Nested functions
+
+def outer_function():
+    async def nested_function_noncompliant(timeout): # Noncompliant
+        await asyncio.sleep(timeout)
+    await nested_function_noncompliant(5)
+
+async def outer_function():
+    def nested_function_noncompliant(timeout): # OK
+        foo()
+    await something()
+
+
+# Inheritance
+
+class ClassWithUnknownParent(Unknown):
+    async def method_with_timeout(self, timeout): # OK, may be inherited
+        await something()
+
+class KnownClass:
+    ...
+
+class ClassWithKnownParent(KnownClass):
+    async def method_with_timeout(self, timeout): # Noncompliant
+        await something()
+
+class KnownClassWithTimeout:
+    async def method_with_timeout(self, timeout): # Noncompliant
+        await something()
+
+class ClassWithKnownParent(KnownClassWithTimeout):
+    async def method_with_timeout(self, timeout): # OK, inherited
+        await something_else()
+

python-frontend/src/main/java/org/sonar/plugins/python/api/types/v2/ClassType.java

@@ -144,6 +144,10 @@ private Optional<PythonType> localMember(String memberName) {
       .findFirst();
   }
 
+  public Optional<PythonType> inheritedMember(String memberName) {
+    return inheritedMember(memberName, new HashSet<>());
+  }
+
   private Optional<PythonType> inheritedMember(String memberName, Set<PythonType> visited) {
     return superClasses().stream()
       .map(TypeWrapper::type)

python-frontend/src/test/java/org/sonar/plugins/python/api/types/v2/ClassTypeTest.java

@@ -465,10 +465,14 @@ void class_members_with_inheritance() {
     assertThat(classB.members()).hasSize(1);
     ClassType classA = (ClassType) classB.superClasses().get(0).type();
     assertThat(classA.members()).hasSize(1);
+    PythonType meth = classA.resolveMember("meth").get();
+    assertThat(meth).isInstanceOf(FunctionType.class);
 
     assertThat(classB.resolveMember("foo")).isPresent();
     assertThat(classB.resolveMember("meth")).isPresent();
-    assertThat(classB.resolveMember("unkown")).isNotPresent();
+    assertThat(classB.resolveMember("unknown")).isNotPresent();
+    assertThat(classB.inheritedMember("foo")).isEmpty();
+    assertThat(classB.inheritedMember("meth")).contains(meth);
   }
 
   @Test