Skip to content

Commit 4cd3fdc

Browse files
sonar-nigel[bot]sonartech
authored andcommitted
SONARPY-3965 Rule S8495 Functions should return tuples of consistent length (#1004)
GitOrigin-RevId: 115e703b35e94fc608f157b4cb91395814afab84
1 parent 07fe8ea commit 4cd3fdc

File tree

6 files changed

+554
-0
lines changed

6 files changed

+554
-0
lines changed
Lines changed: 114 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,114 @@
1+
/*
2+
* SonarQube Python Plugin
3+
* Copyright (C) 2011-2025 SonarSource Sàrl
4+
* mailto:info AT sonarsource DOT com
5+
*
6+
* This program is free software; you can redistribute it and/or
7+
* modify it under the terms of the Sonar Source-Available License Version 1, as published by SonarSource SA.
8+
*
9+
* This program is distributed in the hope that it will be useful,
10+
* but WITHOUT ANY WARRANTY; without even the implied warranty of
11+
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
12+
* See the Sonar Source-Available License for more details.
13+
*
14+
* You should have received a copy of the Sonar Source-Available License
15+
* along with this program; if not, see https://sonarsource.com/license/ssal/
16+
*/
17+
package org.sonar.python.checks;
18+
19+
import java.util.ArrayList;
20+
import java.util.List;
21+
import java.util.OptionalInt;
22+
import org.sonar.check.Rule;
23+
import org.sonar.plugins.python.api.PythonSubscriptionCheck;
24+
import org.sonar.plugins.python.api.SubscriptionContext;
25+
import org.sonar.plugins.python.api.tree.Expression;
26+
import org.sonar.plugins.python.api.tree.FunctionDef;
27+
import org.sonar.plugins.python.api.tree.ReturnStatement;
28+
import org.sonar.plugins.python.api.tree.Tree;
29+
import org.sonar.plugins.python.api.tree.Tuple;
30+
31+
@Rule(key = "S8495")
32+
public class InconsistentTupleReturnCheck extends PythonSubscriptionCheck {
33+
34+
private static final String MESSAGE = "Refactor this function to always return tuples of the same length.";
35+
private static final String SECONDARY_MESSAGE = "Returns a %d-tuple.";
36+
37+
@Override
38+
public void initialize(Context context) {
39+
context.registerSyntaxNodeConsumer(Tree.Kind.FUNCDEF, InconsistentTupleReturnCheck::checkFunction);
40+
}
41+
42+
private static void checkFunction(SubscriptionContext ctx) {
43+
FunctionDef functionDef = (FunctionDef) ctx.syntaxNode();
44+
45+
ReturnCheckUtils.ReturnStmtCollector collector = ReturnCheckUtils.ReturnStmtCollector.collect(functionDef);
46+
47+
if (collector.containsYield()) {
48+
return;
49+
}
50+
51+
List<ReturnStatement> returnStmts = collector.getReturnStmts();
52+
53+
List<ReturnWithLength> tupleReturns = new ArrayList<>();
54+
for (ReturnStatement returnStmt : returnStmts) {
55+
OptionalInt length = getTupleLengthIfTupleReturn(returnStmt);
56+
if (length.isPresent()) {
57+
tupleReturns.add(new ReturnWithLength(returnStmt, length.getAsInt()));
58+
}
59+
}
60+
61+
if (tupleReturns.size() < 2) {
62+
return;
63+
}
64+
65+
int firstLength = tupleReturns.get(0).length;
66+
boolean allSame = tupleReturns.stream().allMatch(r -> r.length == firstLength);
67+
if (allSame) {
68+
return;
69+
}
70+
71+
PreciseIssue issue = ctx.addIssue(functionDef.name(), MESSAGE);
72+
for (ReturnWithLength tupleReturn : tupleReturns) {
73+
issue.secondary(tupleReturn.returnStmt, String.format(SECONDARY_MESSAGE, tupleReturn.length));
74+
}
75+
}
76+
77+
private static boolean containsUnpacking(List<Expression> exprs) {
78+
return exprs.stream().anyMatch(e -> e.is(Tree.Kind.UNPACKING_EXPR));
79+
}
80+
81+
private static OptionalInt getTupleLengthIfTupleReturn(ReturnStatement returnStmt) {
82+
List<Expression> expressions = returnStmt.expressions();
83+
if (expressions.isEmpty()) {
84+
return OptionalInt.empty();
85+
}
86+
if (expressions.size() > 1) {
87+
// Implicit tuple: return a, b — skip if any element is a star expression
88+
if (containsUnpacking(expressions)) {
89+
return OptionalInt.empty();
90+
}
91+
return OptionalInt.of(expressions.size());
92+
}
93+
// Single expression - check if it's an explicit tuple literal
94+
Expression expr = expressions.get(0);
95+
if (expr.is(Tree.Kind.TUPLE)) {
96+
Tuple tuple = (Tuple) expr;
97+
if (containsUnpacking(tuple.elements())) {
98+
return OptionalInt.empty();
99+
}
100+
return OptionalInt.of(tuple.elements().size());
101+
}
102+
return OptionalInt.empty();
103+
}
104+
105+
private static class ReturnWithLength {
106+
final ReturnStatement returnStmt;
107+
final int length;
108+
109+
ReturnWithLength(ReturnStatement returnStmt, int length) {
110+
this.returnStmt = returnStmt;
111+
this.length = length;
112+
}
113+
}
114+
}

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -280,6 +280,7 @@ public Stream<Class<?>> getChecks() {
280280
ImpossibleBackReferenceCheck.class,
281281
ImpossibleBoundariesCheck.class,
282282
IncompatibleOperandsCheck.class,
283+
InconsistentTupleReturnCheck.class,
283284
InconsistentTypeHintCheck.class,
284285
IncorrectExceptionTypeCheck.class,
285286
IncorrectParameterDatetimeConstructorsCheck.class,
Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
<p>This rule raises an issue when a function returns tuples of different lengths in different code paths.</p>
2+
<h2>Why is this an issue?</h2>
3+
<p>When a function returns tuples of varying lengths, it creates an inconsistent API that is difficult to use correctly. Callers cannot reliably
4+
unpack the return value without checking its length first, which leads to fragile code.</p>
5+
<p>Consider this example:</p>
6+
<pre>
7+
def process_data(value):
8+
if value &gt; 0:
9+
return (value,) # Returns a 1-tuple
10+
else:
11+
return (value, value * 2) # Returns a 2-tuple
12+
13+
result = process_data(10)
14+
x, y = result # ValueError if value &gt; 0!
15+
</pre>
16+
<p>This forces callers to add defensive length-checks before unpacking, making the code fragile and harder to maintain. The function signature
17+
provides no indication that the return type varies, so developers must read the implementation or discover the behavior through runtime errors.</p>
18+
<p>Consistent return types make code more predictable, easier to test, and less likely to cause unexpected failures in production.</p>
19+
<h3>What is the potential impact?</h3>
20+
<p>Runtime errors from tuple unpacking may not surface during initial testing if certain code paths are not fully exercised, leading to production
21+
failures. The need for defensive length-checking also increases the maintenance burden and makes the API harder to document for new team members.</p>
22+
<h2>How to fix it</h2>
23+
<p>Make all return statements return tuples of the same length. Add placeholder values (like <code>None</code> or <code>0</code>) to shorter tuples
24+
to maintain consistency.</p>
25+
<h3>Code examples</h3>
26+
<h4>Noncompliant code example</h4>
27+
<pre data-diff-id="1" data-diff-type="noncompliant">
28+
def calculate_stats(numbers):
29+
if not numbers:
30+
return (0,) # Noncompliant
31+
total = sum(numbers)
32+
average = total / len(numbers)
33+
return (total, average)
34+
</pre>
35+
<h4>Compliant solution</h4>
36+
<pre data-diff-id="1" data-diff-type="compliant">
37+
def calculate_stats(numbers):
38+
if not numbers:
39+
return (0, 0) # Consistent 2-tuple
40+
total = sum(numbers)
41+
average = total / len(numbers)
42+
return (total, average)
43+
</pre>
44+
<h2>Resources</h2>
45+
<h3>Documentation</h3>
46+
<ul>
47+
<li>Python Documentation - <a href="https://docs.python.org/3/tutorial/controlflow.html#more-on-defining-functions">More on Defining Functions</a></li>
48+
<li>Python Documentation - <a href="https://docs.python.org/3/tutorial/datastructures.html#tuples-and-sequences">Tuples and Sequences</a></li>
49+
</ul>
Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
{
2+
"title": "Functions should return tuples of consistent length",
3+
"type": "CODE_SMELL",
4+
"status": "ready",
5+
"remediation": {
6+
"func": "Constant\/Issue",
7+
"constantCost": "5min"
8+
},
9+
"tags": [],
10+
"defaultSeverity": "Major",
11+
"ruleSpecification": "RSPEC-8495",
12+
"sqKey": "S8495",
13+
"scope": "Main",
14+
"quickfix": "unknown",
15+
"code": {
16+
"impacts": {
17+
"MAINTAINABILITY": "HIGH"
18+
},
19+
"attribute": "CONVENTIONAL"
20+
}
21+
}
Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
/*
2+
* SonarQube Python Plugin
3+
* Copyright (C) 2011-2025 SonarSource Sàrl
4+
* mailto:info AT sonarsource DOT com
5+
*
6+
* This program is free software; you can redistribute it and/or
7+
* modify it under the terms of the Sonar Source-Available License Version 1, as published by SonarSource SA.
8+
*
9+
* This program is distributed in the hope that it will be useful,
10+
* but WITHOUT ANY WARRANTY; without even the implied warranty of
11+
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
12+
* See the Sonar Source-Available License for more details.
13+
*
14+
* You should have received a copy of the Sonar Source-Available License
15+
* along with this program; if not, see https://sonarsource.com/license/ssal/
16+
*/
17+
package org.sonar.python.checks;
18+
19+
import org.junit.jupiter.api.Test;
20+
import org.sonar.python.checks.utils.PythonCheckVerifier;
21+
22+
class InconsistentTupleReturnCheckTest {
23+
24+
@Test
25+
void test() {
26+
PythonCheckVerifier.verify(
27+
"src/test/resources/checks/inconsistentTupleReturn.py",
28+
new InconsistentTupleReturnCheck());
29+
}
30+
}

0 commit comments

Comments
 (0)