Skip to content

Commit fbe63c5

Browse files
joke1196sonartech
authored andcommitted
SONARPY-5082: Implement rule S9460 - Sole starred generator arguments should be parenthesized
GitOrigin-RevId: 596802420b12a43260ecba56181834ecefb82d85
1 parent 74214ee commit fbe63c5

7 files changed

Lines changed: 210 additions & 21 deletions

File tree

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

Lines changed: 21 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -240,6 +240,9 @@ public Stream<Class<?>> getChecks() {
240240
DisabledHtmlAutoEscapeLegacyCheck.class,
241241
DisabledRDSEncryptionCheck.class,
242242
DisabledSNSTopicEncryptionCheck.class,
243+
DjangoModelFormFieldsCheck.class,
244+
DjangoModelStringFieldCheck.class,
245+
DjangoModelStrMethodCheck.class,
243246
DjangoNonDictSerializationCheck.class,
244247
DjangoRenderContextCheck.class,
245248
DoublePrefixOperatorCheck.class,
@@ -323,6 +326,7 @@ public Stream<Class<?>> getChecks() {
323326
IdentityComparisonWithCachedTypesCheck.class,
324327
IdentityComparisonWithNewObjectCheck.class,
325328
IgnoredPureOperationsCheck.class,
329+
ImplicitlySkippedTestCheck.class,
326330
ImplicitStringConcatenationCheck.class,
327331
ImpossibleBackReferenceCheck.class,
328332
ImpossibleBoundariesCheck.class,
@@ -340,6 +344,7 @@ public Stream<Class<?>> getChecks() {
340344
InstanceAndClassMethodsAtLeastOnePositionalCheck.class,
341345
InstanceMethodSelfAsFirstCheck.class,
342346
InputInAsyncCheck.class,
347+
IntentionalTestFailureMessageCheck.class,
343348
InvalidOpenModeCheck.class,
344349
InvalidRegexCheck.class,
345350
InvariantReturnCheck.class,
@@ -368,6 +373,7 @@ public Stream<Class<?>> getChecks() {
368373
MethodNameCheck.class,
369374
MethodShouldBeStaticCheck.class,
370375
MissingDocstringCheck.class,
376+
MissingHyperParameterCheck.class,
371377
MissingNewlineAtEndOfFileCheck.class,
372378
MisspelledMockAssertionCheck.class,
373379
MockPatchLambdaReturnValueCheck.class,
@@ -383,6 +389,7 @@ public Stream<Class<?>> getChecks() {
383389
NestedControlFlowDepthCheck.class,
384390
NestedLoopVariableReuseCheck.class,
385391
NestedWithStatementsCheck.class,
392+
NetworkCallsWithoutTimeoutsInLambdaCheck.class,
386393
NewStyleClassCheck.class,
387394
NonCallableCalledCheck.class,
388395
NonContextManagerCheck.class,
@@ -395,7 +402,6 @@ public Stream<Class<?>> getChecks() {
395402
NoReRaiseInExitCheck.class,
396403
NoSonarCommentCheck.class,
397404
NoSonarCommentFormatCheck.class,
398-
TemplateAndStrConcatenationCheck.class,
399405
NotDiscoverableTestMethodCheck.class,
400406
NotImplementedBooleanContextCheck.class,
401407
NotImplementedErrorInOperatorMethodsCheck.class,
@@ -415,14 +421,14 @@ public Stream<Class<?>> getChecks() {
415421
PandasReadNoDataTypeCheck.class,
416422
PandasToDatetimeFormatCheck.class,
417423
ParsingErrorCheck.class,
424+
PossessiveQuantifierContinuationCheck.class,
418425
PredictableSaltCheck.class,
419426
PreIncrementDecrementCheck.class,
420427
PrintStatementUsageCheck.class,
421428
PrivilegePolicyCheck.class,
422429
ProcessSignallingCheck.class,
423430
PropertyAccessorParameterCountCheck.class,
424431
PropertyMethodWithoutReturnCheck.class,
425-
PytzUsageCheck.class,
426432
PseudoRandomCheck.class,
427433
PublicApiIsSecuritySensitiveCheck.class,
428434
PubliclyWritableDirectoriesCheck.class,
@@ -449,17 +455,21 @@ public Stream<Class<?>> getChecks() {
449455
PytestUselessYieldFixtureCheck.class,
450456
PyTorchDataLoaderNumWorkersCheck.class,
451457
PytzTimeZoneInDatetimeConstructorCheck.class,
458+
PytzUsageCheck.class,
452459
RaiseOutsideExceptCheck.class,
453460
RandomSeedCheck.class,
454461
RedosCheck.class,
455462
RedundantJumpCheck.class,
456-
PossessiveQuantifierContinuationCheck.class,
457463
RedundantRegexAlternativesCheck.class,
464+
ReferencedBeforeAssignmentCheck.class,
465+
RegexComplexityCheck.class,
466+
RegexLookaheadCheck.class,
458467
ReluctantQuantifierCheck.class,
459468
ReluctantQuantifierWithEmptyContinuationCheck.class,
460469
ResourceAccessPolicyCheck.class,
461470
ReturnAndYieldInOneFunctionCheck.class,
462471
ReturnYieldOutsideFunctionCheck.class,
472+
RewriteCollectionConstructorAsComprehensionCheck.class,
463473
RobustCipherAlgorithmCheck.class,
464474
RouterPrefixInIncludeRouterCheck.class,
465475
S3BucketBlockPublicAccessCheck.class,
@@ -485,15 +495,18 @@ public Stream<Class<?>> getChecks() {
485495
SingleInvocationRuntimeExceptionCheck.class,
486496
SingleInvocationWarningCheck.class,
487497
SkippedTestNoReasonCheck.class,
498+
SklearnCachedPipelineDontAccessTransformersCheck.class,
488499
SkLearnEstimatorDontInitializeEstimatedValuesCheck.class,
500+
SklearnPipelineSpecifyMemoryArgumentCheck.class,
501+
SklearnPipelineParameterAreCorrectCheck.class,
489502
SleepZeroInAsyncCheck.class,
490-
SortedMinMaxCheck.class,
491503
SlotsAssignmentCheck.class,
504+
SortedMinMaxCheck.class,
505+
SoleStarredGeneratorArgumentCheck.class,
492506
SpecialMethodParamListCheck.class,
493507
SpecialMethodReturnTypeCheck.class,
494508
SpecificExceptionAssertionCheck.class,
495509
SpecificWarningAssertionCheck.class,
496-
TestCasesShouldContainTestsCheck.class,
497510
SQLQueriesCheck.class,
498511
StartsWithEndsWithTupleCheck.class,
499512
StopIterationInGeneratorCheck.class,
@@ -504,22 +517,17 @@ public Stream<Class<?>> getChecks() {
504517
StringReplaceCheck.class,
505518
StrongCryptographicKeysCheck.class,
506519
SumListConcatenationCheck.class,
507-
SklearnCachedPipelineDontAccessTransformersCheck.class,
508-
MissingHyperParameterCheck.class,
509-
NetworkCallsWithoutTimeoutsInLambdaCheck.class,
510-
SklearnPipelineSpecifyMemoryArgumentCheck.class,
511-
SklearnPipelineParameterAreCorrectCheck.class,
512520
SuperfluousCurlyBraceCheck.class,
513521
SuperLinearRegexCheck.class,
514522
SynchronousFileOperationsInAsyncCheck.class,
515523
SynchronousHttpOperationsInAsyncCheck.class,
516524
SynchronousSubprocessOperationsInAsyncCheck.class,
517525
SynchronousOsCallsInAsyncCheck.class,
518-
TempFileCreationCheck.class,
519-
ImplicitlySkippedTestCheck.class,
520-
IntentionalTestFailureMessageCheck.class,
521526
TaskGroupNurseryUsedOnlyOnceCheck.class,
527+
TempFileCreationCheck.class,
528+
TemplateAndStrConcatenationCheck.class,
522529
TemplateStringStructuralPatternMatchingCheck.class,
530+
TestCasesShouldContainTestsCheck.class,
523531
TestClientContentParameterCheck.class,
524532
TimeSleepInAsyncCheck.class,
525533
ToDoCommentCheck.class,
@@ -541,10 +549,6 @@ public Stream<Class<?>> getChecks() {
541549
TfInputShapeOnModelSubclassCheck.class,
542550
TfGatherDeprecatedValidateIndicesCheck.class,
543551
TfPyTorchSpecifyReductionAxisCheck.class,
544-
ReferencedBeforeAssignmentCheck.class,
545-
RegexComplexityCheck.class,
546-
RegexLookaheadCheck.class,
547-
RewriteCollectionConstructorAsComprehensionCheck.class,
548552
UnconditionalAssertionCheck.class,
549553
UseCachedPropertyCheck.class,
550554
UseMonkeypatchFixtureCheck.class,
@@ -593,10 +597,7 @@ public Stream<Class<?>> getChecks() {
593597
XMLParserXXEVulnerableCheck.class,
594598
XMLSignatureValidationCheck.class,
595599
YieldFromDelegationCheck.class,
596-
DjangoModelFormFieldsCheck.class,
597600
WebEntryPointDecoratorCheck.class,
598-
DjangoModelStringFieldCheck.class,
599-
DjangoModelStrMethodCheck.class,
600601
HardcodedCredentialsCallCheck.class);
601602
}
602603
}
Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
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 org.sonar.check.Rule;
20+
import org.sonar.plugins.python.api.PythonSubscriptionCheck;
21+
import org.sonar.plugins.python.api.SubscriptionContext;
22+
import org.sonar.plugins.python.api.quickfix.PythonQuickFix;
23+
import org.sonar.plugins.python.api.tree.ComprehensionExpression;
24+
import org.sonar.plugins.python.api.tree.RegularArgument;
25+
import org.sonar.plugins.python.api.tree.Tree;
26+
import org.sonar.python.quickfix.TextEditUtils;
27+
28+
@Rule(key = "S9460")
29+
public class SoleStarredGeneratorArgumentCheck extends PythonSubscriptionCheck {
30+
31+
private static final String MESSAGE = "Add parentheses around this starred generator argument.";
32+
33+
@Override
34+
public void initialize(Context context) {
35+
context.registerSyntaxNodeConsumer(Tree.Kind.GENERATOR_EXPR, SoleStarredGeneratorArgumentCheck::checkGeneratorExpression);
36+
}
37+
38+
private static void checkGeneratorExpression(SubscriptionContext ctx) {
39+
ComprehensionExpression generatorExpression = (ComprehensionExpression) ctx.syntaxNode();
40+
if (generatorExpression.parent() instanceof RegularArgument
41+
&& generatorExpression.resultExpression().is(Tree.Kind.UNPACKING_EXPR)
42+
&& !"(".equals(generatorExpression.firstToken().value())) {
43+
var issue = ctx.addIssue(generatorExpression, MESSAGE);
44+
var quickFix = PythonQuickFix.newQuickFix(MESSAGE)
45+
.addTextEdit(TextEditUtils.insertBefore(generatorExpression.firstToken(), "("))
46+
.addTextEdit(TextEditUtils.insertAfter(generatorExpression.lastToken(), ")"))
47+
.build();
48+
issue.addQuickFix(quickFix);
49+
}
50+
}
51+
}
Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
<p>This rule raises an issue when a call has one unparenthesized flattened generator argument, such as a call that passes generated values without
2+
enclosing the generator expression in parentheses.</p>
3+
<p>This syntax represents one generator argument. It can be mistaken for positional argument unpacking.</p>
4+
<h2>Why is this an issue?</h2>
5+
<p>Recent versions of the language allow an iterable-expansion expression as the element of a generator expression. In a call written with a leading
6+
expansion marker immediately before a generator expression, the call receives one generator that produces the flattened elements. It does not expand
7+
that generator into several positional arguments.</p>
8+
<p>Without parentheses, this meaning is difficult to see. A reader may interpret the leading expansion marker as call-site argument unpacking,
9+
especially because expanding an iterable directly into a function call is a common pattern.</p>
10+
<p>Add parentheses to show the intended operation. Parenthesize the generator expression to pass one flattened generator argument. Use an additional
11+
expansion marker outside those parentheses when the generator itself must be unpacked into positional arguments.</p>
12+
<h3>What is the potential impact?</h3>
13+
<p>Ambiguous call syntax can lead to incorrect reviews, maintenance mistakes, or changes that alter how arguments are passed. Explicit parentheses
14+
make the call’s argument-passing behavior easier to understand.</p>
15+
<h2>How to fix it</h2>
16+
<p>Review the intended argument-passing behavior. If the function should receive one flattened generator as its only argument, add parentheses around
17+
the generator. A report is a false positive only if the analyzer incorrectly identified the source as this sole, unparenthesized starred-generator
18+
form, or if the configured Python version does not support this syntax. If the unparenthesized form is intentional and has been reviewed, keeping it
19+
is an acceptable won’t-fix decision, although parentheses make the intent clearer.</p>
20+
<h3>Code examples</h3>
21+
<h4>Noncompliant code example</h4>
22+
<pre data-diff-id="1" data-diff-type="noncompliant">
23+
result = f(*x for x in groups) # Noncompliant
24+
</pre>
25+
<h4>Compliant solution</h4>
26+
<pre data-diff-id="1" data-diff-type="compliant">
27+
result = f((*x for x in groups))
28+
</pre>
29+
<h2>Resources</h2>
30+
<h3>Documentation</h3>
31+
<ul>
32+
<li>PEP - <a href="https://peps.python.org/pep-0798/">PEP 798 – Unpacking in Comprehensions</a></li>
33+
</ul>
34+
Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
{
2+
"title": "Sole starred generator arguments should be parenthesized",
3+
"type": "CODE_SMELL",
4+
"status": "ready",
5+
"remediation": {
6+
"func": "Constant\/Issue",
7+
"constantCost": "5 min"
8+
},
9+
"tags": [
10+
"python3.15"
11+
],
12+
"defaultSeverity": "Major",
13+
"ruleSpecification": "RSPEC-9460",
14+
"sqKey": "S9460",
15+
"scope": "All",
16+
"quickfix": "unknown",
17+
"code": {
18+
"impacts": {
19+
"MAINTAINABILITY": "MEDIUM"
20+
},
21+
"attribute": "CLEAR"
22+
}
23+
}

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -406,6 +406,7 @@
406406
"S9409",
407407
"S9453",
408408
"S9454",
409-
"S9455"
409+
"S9455",
410+
"S9460"
410411
]
411412
}
Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
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+
20+
import org.junit.jupiter.api.Test;
21+
import org.sonar.python.checks.quickfix.PythonQuickFixVerifier;
22+
import org.sonar.python.checks.utils.PythonCheckVerifier;
23+
24+
class SoleStarredGeneratorArgumentCheckTest {
25+
26+
private static final SoleStarredGeneratorArgumentCheck check = new SoleStarredGeneratorArgumentCheck();
27+
28+
@Test
29+
void test() {
30+
PythonCheckVerifier.verify("src/test/resources/checks/soleStarredGeneratorArgument.py", check);
31+
}
32+
33+
@Test
34+
void quickFix() {
35+
PythonQuickFixVerifier.verify(check, "f(*x for x in groups)", "f((*x for x in groups))");
36+
}
37+
38+
@Test
39+
void multilineQuickFix() {
40+
PythonQuickFixVerifier.verify(check, """
41+
f(
42+
*x for x in groups
43+
)""", """
44+
f(
45+
(*x for x in groups)
46+
)""");
47+
}
48+
}
Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
def f(*args):
2+
pass
3+
4+
5+
def noncompliant(groups):
6+
f(*x for x in groups) # Noncompliant {{Add parentheses around this starred generator argument.}}
7+
# ^^^^^^^^^^^^^^^^^^
8+
obj.m(*x for x in groups) # Noncompliant {{Add parentheses around this starred generator argument.}}
9+
# ^^^^^^^^^^^^^^^^^^
10+
11+
12+
async def noncompliant_async(groups):
13+
f(*x async for x in groups) # Noncompliant {{Add parentheses around this starred generator argument.}}
14+
# ^^^^^^^^^^^^^^^^^^^^^^^^
15+
16+
17+
@f(*x for x in groups) # Noncompliant {{Add parentheses around this starred generator argument.}}
18+
# ^^^^^^^^^^^^^^^^^^
19+
def decorated():
20+
pass
21+
22+
23+
def compliant(groups, values):
24+
f((*x for x in groups))
25+
f(*values)
26+
f(*(x for x in groups))
27+
f(*(x async for x in groups))
28+
f(x for x in groups)
29+
f(*(*x for x in groups))
30+
[*x for x in groups]
31+
a = (*x for x in groups)

0 commit comments

Comments
 (0)