Skip to content

Commit 8531d74

Browse files
thomas-serre-sonarsourcesonartech
authored andcommitted
SONARPY-2950 Rule S7507 Incorrect default_factory keyword argument for defaultdict (#276)
GitOrigin-RevId: 67d157016615cfcd59e3b9995114a78097e44dbd
1 parent 5f89230 commit 8531d74

7 files changed

Lines changed: 146 additions & 0 deletions

File tree

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
/*
2+
* SonarQube Python Plugin
3+
* Copyright (C) 2011-2025 SonarSource SA
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.sonar.check.Rule;
20+
import org.sonar.plugins.python.api.PythonSubscriptionCheck;
21+
import org.sonar.plugins.python.api.tree.CallExpression;
22+
import org.sonar.plugins.python.api.tree.RegularArgument;
23+
import org.sonar.plugins.python.api.tree.Tree;
24+
import org.sonar.plugins.python.api.types.v2.TriBool;
25+
import org.sonar.python.tree.TreeUtils;
26+
import org.sonar.python.types.v2.TypeCheckBuilder;
27+
28+
@Rule(key = "S7507")
29+
public class DefaultFactoryArgumentCheck extends PythonSubscriptionCheck {
30+
31+
private TypeCheckBuilder defaultDictTypeChecker = null;
32+
33+
@Override
34+
public void initialize(Context context) {
35+
context.registerSyntaxNodeConsumer(Tree.Kind.FILE_INPUT, ctx ->
36+
defaultDictTypeChecker = ctx.typeChecker().typeCheckBuilder().isTypeWithFqn("collections.defaultdict")
37+
);
38+
context.registerSyntaxNodeConsumer(Tree.Kind.CALL_EXPR, ctx -> {
39+
CallExpression callExpression = (CallExpression) ctx.syntaxNode();
40+
if (defaultDictTypeChecker.check(callExpression.callee().typeV2()) != TriBool.TRUE) {
41+
return;
42+
}
43+
RegularArgument defaultFactoryKeywordArgument = TreeUtils.argumentByKeyword("default_factory", callExpression.arguments());
44+
if (defaultFactoryKeywordArgument != null) {
45+
ctx.addIssue(defaultFactoryKeywordArgument, "Replace this keyword argument with a positional argument at the first place.");
46+
}
47+
});
48+
}
49+
}

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
@@ -159,6 +159,7 @@ public Stream<Class<?>> getChecks() {
159159
DeadStoreCheck.class,
160160
DebugModeCheck.class,
161161
DedicatedAssertionCheck.class,
162+
DefaultFactoryArgumentCheck.class,
162163
DeprecatedNumpyTypesCheck.class,
163164
DictionaryDuplicateKeyCheck.class,
164165
DictionaryStaticKeyCheck.class,
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 <code>default_factory</code> is incorrectly supplied as a keyword argument during the initialization of
2+
<code>collections.defaultdict</code>.</p>
3+
<h2>Why is this an issue?</h2>
4+
<p>The <code>collections.defaultdict</code> class provides a dictionary-like structure that calls a factory function to supply missing values. This
5+
factory function (like <code>list</code>, <code>int</code>, or a <code>lambda</code>) is specified during initialization.</p>
6+
<p>Crucially, the <code>defaultdict</code> constructor signature requires the <code>default_factory</code> as its <strong>first positional
7+
argument</strong>. Any subsequent positional or keyword arguments are used to initialize the contents of the dictionary. This mirrors the behavior of
8+
the standard <code>dict</code> constructor.</p>
9+
<p>Providing the factory using the keyword <code>default_factory=…​</code>, as in <code>defaultdict(default_factory=list)</code>, is therefore
10+
incorrect and leads to unexpected behavior:</p>
11+
<ul>
12+
<li> It does <strong>not</strong> set the default factory for missing keys. The <code>defaultdict</code> behaves like a regular <code>dict</code> in
13+
this regard and will raise a <code>KeyError</code> when a missing key is accessed. </li>
14+
<li> It <strong>does</strong> initialize the dictionary with a single key-value pair: <code>{'default_factory': list}</code>. </li>
15+
</ul>
16+
<h2>How to fix it</h2>
17+
<p>To fix this issue correctly initialize the <code>defaultdict</code> with a default factory by providing the factory callable as the first
18+
positional argument, not as a keyword argument.</p>
19+
<pre data-diff-id="1" data-diff-type="noncompliant">
20+
from collections import defaultdict
21+
22+
d1 = defaultdict(default_factory=int) # Noncompliant: this creates a dictionary with a single key-value pair.
23+
</pre>
24+
<pre data-diff-id="1" data-diff-type="noncompliant">
25+
from collections import defaultdict
26+
27+
d1 = defaultdict(int) # Compliant
28+
</pre>
29+
<h2>Resources</h2>
30+
<h3>Documentation</h3>
31+
<ul>
32+
<li> Python Documentation - <a href="https://docs.python.org/3/library/collections.html#collections.defaultdict">collections.defaultdict</a> </li>
33+
</ul>
34+
Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
{
2+
"title": "\"defaultdict\" should not be initialized with \"default_factory\" as a keyword argument",
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-7507",
12+
"sqKey": "S7507",
13+
"scope": "All",
14+
"quickfix": "targeted",
15+
"code": {
16+
"impacts": {
17+
"RELIABILITY": "MEDIUM"
18+
},
19+
"attribute": "CONVENTIONAL"
20+
}
21+
}

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
@@ -266,6 +266,7 @@
266266
"S7502",
267267
"S7503",
268268
"S7506",
269+
"S7507",
269270
"S7510",
270271
"S7511"
271272
]
Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
/*
2+
* SonarQube Python Plugin
3+
* Copyright (C) 2011-2025 SonarSource SA
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 DefaultFactoryArgumentCheckTest {
23+
24+
@Test
25+
void test() {
26+
PythonCheckVerifier.verify("src/test/resources/checks/defaultFactoryArgument.py", new DefaultFactoryArgumentCheck());
27+
}
28+
29+
}
Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
from collections import defaultdict, namedtuple
2+
3+
d1 = defaultdict(default_factory=int) # Noncompliant {{Replace this keyword argument with a positional argument at the first place.}}
4+
# ^^^^^^^^^^^^^^^^^^^
5+
6+
d2 = defaultdict(int, default_factory=list) # Noncompliant {{Replace this keyword argument with a positional argument at the first place.}}
7+
# ^^^^^^^^^^^^^^^^^^^^
8+
9+
d1 = defaultdict(int) # Compliant
10+
11+
named_tuple = namedtuple('Point', ['x', 'y']) # Compliant, for coverage

0 commit comments

Comments
 (0)