Skip to content

Commit 1e4c9bd

Browse files
sonar-nigel[bot]marc-jasper-sonarsource
authored and
sonartech
committed
SONARPY-3992 Rule S8513 "startswith" and "endswith" calls should use tuples instead of multiple "or" operators (#1016)
Co-authored-by: Marc Jasper <marc.jasper@sonarsource.com> GitOrigin-RevId: ccff1f73850e2d46e33a778430deca03a4117cf4
1 parent 1a25443 commit 1e4c9bd

File tree

7 files changed

+552
-0
lines changed

7 files changed

+552
-0
lines changed

‎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
@@ -415,6 +415,7 @@ public Stream<Class<?>> getChecks() {
415415
SpecialMethodReturnTypeCheck.class,
416416
SQLQueriesCheck.class,
417417
StandardInputCheck.class,
418+
StartsWithEndsWithTupleCheck.class,
418419
StopIterationInGeneratorCheck.class,
419420
StrftimeConfusingHourSystemCheck.class,
420421
StringFormatCorrectnessCheck.class,

‎python-checks/src/main/java/org/sonar/python/checks/StartsWithEndsWithTupleCheck.java‎

Lines changed: 224 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,224 @@
1+
/*
2+
* SonarQube Python Plugin
3+
* Copyright (C) SonarSource Sàrl
4+
* mailto:info AT sonarsource DOT com
5+
*
6+
* You can redistribute and/or modify this program under the terms of
7+
* the Sonar Source-Available License Version 1, as published by SonarSource Sàrl.
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.Optional;
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.TriBool;
26+
import org.sonar.plugins.python.api.quickfix.PythonQuickFix;
27+
import org.sonar.plugins.python.api.tree.Argument;
28+
import org.sonar.plugins.python.api.tree.BinaryExpression;
29+
import org.sonar.plugins.python.api.tree.CallExpression;
30+
import org.sonar.plugins.python.api.tree.Expression;
31+
import org.sonar.plugins.python.api.tree.QualifiedExpression;
32+
import org.sonar.plugins.python.api.tree.RegularArgument;
33+
import org.sonar.plugins.python.api.tree.Tree;
34+
import org.sonar.plugins.python.api.types.v2.matchers.TypeMatcher;
35+
import org.sonar.plugins.python.api.types.v2.matchers.TypeMatchers;
36+
import org.sonar.python.checks.utils.CheckUtils;
37+
import org.sonar.python.checks.utils.Expressions;
38+
import org.sonar.python.quickfix.TextEditUtils;
39+
import org.sonar.python.tree.TreeUtils;
40+
41+
@Rule(key = "S8513")
42+
public class StartsWithEndsWithTupleCheck extends PythonSubscriptionCheck {
43+
44+
private static final String STARTSWITH = "startswith";
45+
private static final String ENDSWITH = "endswith";
46+
47+
private static final TypeMatcher STR_BYTES_OR_BYTEARRAY_METHOD_MATCHER = TypeMatchers.isFunctionOwnerSatisfying(
48+
TypeMatchers.any(
49+
TypeMatchers.isOrExtendsType("builtins.str"),
50+
TypeMatchers.isOrExtendsType("builtins.bytes"),
51+
TypeMatchers.isOrExtendsType("builtins.bytearray")));
52+
53+
@Override
54+
public void initialize(Context context) {
55+
context.registerSyntaxNodeConsumer(Tree.Kind.OR, StartsWithEndsWithTupleCheck::checkOrExpression);
56+
}
57+
58+
private static void checkOrExpression(SubscriptionContext ctx) {
59+
BinaryExpression orExpr = (BinaryExpression) ctx.syntaxNode();
60+
Tree ancestor = orExpr.parent();
61+
while (ancestor instanceof Expression expr && Expressions.removeParentheses(expr) != expr) {
62+
ancestor = ancestor.parent();
63+
}
64+
if (ancestor != null && ancestor.is(Tree.Kind.OR)) {
65+
return;
66+
}
67+
68+
List<Expression> operands = flattenOrChain(orExpr);
69+
if (operands.size() < 2) {
70+
return;
71+
}
72+
73+
Optional<List<CallExpression>> maybeCalls = extractCalls(operands, ctx);
74+
if (maybeCalls.isEmpty()) {
75+
return;
76+
}
77+
List<CallExpression> calls = maybeCalls.get();
78+
79+
String methodName = extractUniformMethodName(calls);
80+
if (methodName == null) {
81+
return;
82+
}
83+
84+
if (!allSameReceiver(calls)) {
85+
return;
86+
}
87+
88+
if (anyReceiverHasNestedCall(calls)) {
89+
return;
90+
}
91+
92+
Optional<List<String>> maybeArgTexts = extractStringLiteralArgs(calls);
93+
if (maybeArgTexts.isEmpty()) {
94+
return;
95+
}
96+
List<String> argTexts = maybeArgTexts.get();
97+
98+
String message = String.format("Replace chained \"%s\" calls with a single call using a tuple argument.", methodName);
99+
String receiverText = receiverSourceText(calls.get(0));
100+
if (receiverText == null) {
101+
return;
102+
}
103+
String replacement = receiverText + "." + methodName + "((" + String.join(", ", argTexts) + "))";
104+
PythonQuickFix quickFix = PythonQuickFix.newQuickFix("Replace with a single call using a tuple argument",
105+
TextEditUtils.replace(orExpr, replacement));
106+
ctx.addIssue(orExpr, message).addQuickFix(quickFix);
107+
}
108+
109+
private static List<Expression> flattenOrChain(BinaryExpression orExpr) {
110+
List<Expression> result = new ArrayList<>();
111+
flattenOrChainInto(orExpr, result);
112+
return result;
113+
}
114+
115+
private static void flattenOrChainInto(Expression expr, List<Expression> result) {
116+
Expression stripped = Expressions.removeParentheses(expr);
117+
if (stripped.is(Tree.Kind.OR)) {
118+
BinaryExpression binExpr = (BinaryExpression) stripped;
119+
flattenOrChainInto(binExpr.leftOperand(), result);
120+
flattenOrChainInto(binExpr.rightOperand(), result);
121+
} else {
122+
result.add(stripped);
123+
}
124+
}
125+
126+
private static Optional<List<CallExpression>> extractCalls(List<Expression> operands, SubscriptionContext ctx) {
127+
List<CallExpression> calls = new ArrayList<>();
128+
for (Expression operand : operands) {
129+
Expression stripped = Expressions.removeParentheses(operand);
130+
if (!stripped.is(Tree.Kind.CALL_EXPR)) {
131+
return Optional.empty();
132+
}
133+
CallExpression call = (CallExpression) stripped;
134+
if (!isStartsWithOrEndsWith(call, ctx)) {
135+
return Optional.empty();
136+
}
137+
calls.add(call);
138+
}
139+
return Optional.of(calls);
140+
}
141+
142+
private static boolean isStartsWithOrEndsWith(CallExpression call, SubscriptionContext ctx) {
143+
if (!call.callee().is(Tree.Kind.QUALIFIED_EXPR)) {
144+
return false;
145+
}
146+
QualifiedExpression callee = (QualifiedExpression) call.callee();
147+
String name = callee.name().name();
148+
if (!STARTSWITH.equals(name) && !ENDSWITH.equals(name)) {
149+
return false;
150+
}
151+
return STR_BYTES_OR_BYTEARRAY_METHOD_MATCHER.evaluateFor(callee, ctx) != TriBool.FALSE;
152+
}
153+
154+
private static String extractUniformMethodName(List<CallExpression> calls) {
155+
String methodName = null;
156+
for (CallExpression call : calls) {
157+
QualifiedExpression callee = (QualifiedExpression) call.callee();
158+
String name = callee.name().name();
159+
if (methodName == null) {
160+
methodName = name;
161+
} else if (!methodName.equals(name)) {
162+
return null;
163+
}
164+
}
165+
return methodName;
166+
}
167+
168+
private static boolean allSameReceiver(List<CallExpression> calls) {
169+
Expression firstReceiver = getReceiver(calls.get(0));
170+
for (int i = 1; i < calls.size(); i++) {
171+
Expression receiver = getReceiver(calls.get(i));
172+
if (!CheckUtils.areEquivalent(firstReceiver, receiver)) {
173+
return false;
174+
}
175+
}
176+
return true;
177+
}
178+
179+
private static Expression getReceiver(CallExpression call) {
180+
return ((QualifiedExpression) call.callee()).qualifier();
181+
}
182+
183+
private static boolean anyReceiverHasNestedCall(List<CallExpression> calls) {
184+
for (CallExpression call : calls) {
185+
Expression receiver = getReceiver(call);
186+
if (receiver.is(Tree.Kind.CALL_EXPR) || TreeUtils.hasDescendant(receiver, t -> t.is(Tree.Kind.CALL_EXPR))) {
187+
return true;
188+
}
189+
}
190+
return false;
191+
}
192+
193+
private static Optional<List<String>> extractStringLiteralArgs(List<CallExpression> calls) {
194+
List<String> argTexts = new ArrayList<>();
195+
for (CallExpression call : calls) {
196+
List<Argument> args = call.arguments();
197+
if (args.size() != 1) {
198+
return Optional.empty();
199+
}
200+
Argument arg = args.get(0);
201+
if (!(arg instanceof RegularArgument regularArg)) {
202+
return Optional.empty();
203+
}
204+
if (regularArg.keywordArgument() != null) {
205+
return Optional.empty();
206+
}
207+
Expression argExpr = regularArg.expression();
208+
if (!argExpr.is(Tree.Kind.STRING_LITERAL)) {
209+
return Optional.empty();
210+
}
211+
String argSource = TreeUtils.treeToString(argExpr, false);
212+
if (argSource == null) {
213+
return Optional.empty();
214+
}
215+
argTexts.add(argSource);
216+
}
217+
return Optional.of(argTexts);
218+
}
219+
220+
private static String receiverSourceText(CallExpression call) {
221+
Expression receiver = getReceiver(call);
222+
return TreeUtils.treeToString(receiver, false);
223+
}
224+
}

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

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
<p>This rule raises an issue when multiple <code>startswith()</code> or <code>endswith()</code> calls on the same string are chained with the
2+
<code>or</code> operator to check for different prefixes or suffixes.</p>
3+
<h2>Why is this an issue?</h2>
4+
<p>When you need to check if a string starts or ends with one of several possible values, chaining multiple <code>startswith()</code> or
5+
<code>endswith()</code> calls with the <code>or</code> operator creates unnecessary verbosity. Both methods accept a tuple of strings as their first
6+
argument, so a single call with a tuple is both more concise and more readable.</p>
7+
<h2>How to fix it</h2>
8+
<p>Replace multiple <code>startswith()</code> or <code>endswith()</code> calls connected by <code>or</code> operators with a single call that uses a
9+
tuple containing all the prefixes or suffixes.</p>
10+
<h3>Code examples</h3>
11+
<h4>Noncompliant code example</h4>
12+
<pre data-diff-id="1" data-diff-type="noncompliant">
13+
if s.startswith("http://") or s.startswith("https://"): # Noncompliant
14+
process_url(s)
15+
</pre>
16+
<h4>Compliant solution</h4>
17+
<pre data-diff-id="1" data-diff-type="compliant">
18+
if s.startswith(("http://", "https://")):
19+
process_url(s)
20+
</pre>
21+
<h4>Noncompliant code example</h4>
22+
<pre data-diff-id="2" data-diff-type="noncompliant">
23+
if filename.endswith(".jpg") or filename.endswith(".png") or filename.endswith(".gif"): # Noncompliant
24+
display_image(filename)
25+
</pre>
26+
<h4>Compliant solution</h4>
27+
<pre data-diff-id="2" data-diff-type="compliant">
28+
if filename.endswith((".jpg", ".png", ".gif")):
29+
display_image(filename)
30+
</pre>
31+
<h2>Resources</h2>
32+
<h3>Documentation</h3>
33+
<ul>
34+
<li>Python Documentation - <a href="https://docs.python.org/3/library/stdtypes.html#str.startswith">str.startswith()</a></li>
35+
<li>Python Documentation - <a href="https://docs.python.org/3/library/stdtypes.html#str.endswith">str.endswith()</a></li>
36+
</ul>
37+
<h3>Related rules</h3>
38+
<ul>
39+
<li>{rule:python:S6659} - <code>startswith</code> or <code>endswith</code> methods should be used instead of string slicing in condition
40+
expressions</li>
41+
</ul>

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

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
{
2+
"title": "\"startswith\" and \"endswith\" calls should use tuples instead of multiple \"or\" operators",
3+
"type": "CODE_SMELL",
4+
"status": "ready",
5+
"remediation": {
6+
"func": "Constant\/Issue",
7+
"constantCost": "5min"
8+
},
9+
"tags": [
10+
"convention"
11+
],
12+
"defaultSeverity": "Minor",
13+
"ruleSpecification": "RSPEC-8513",
14+
"sqKey": "S8513",
15+
"scope": "All",
16+
"quickfix": "covered"
17+
}

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -333,6 +333,7 @@
333333
"S8495",
334334
"S8504",
335335
"S8507",
336+
"S8513",
336337
"S8517",
337338
"S8519",
338339
"S8520",

0 commit comments

Comments
 (0)