Skip to content

Commit 5c10cd4

Browse files
sonar-nigel[bot]Sonar Vibe Botguillaume-dequenne
authored andcommitted
SONARPY-3946 Rule S8508 Mutable default values should not be used with "dict.fromkeys()" or "ContextVar()" (#992)
Co-authored-by: Sonar Vibe Bot <vibe-bot@sonarsource.com> Co-authored-by: Guillaume Dequenne <guillaume.dequenne@sonarsource.com> GitOrigin-RevId: eb0532027b854abc510c3de930187e950d00f720
1 parent 08c2eb0 commit 5c10cd4

File tree

6 files changed

+550
-0
lines changed

6 files changed

+550
-0
lines changed
Lines changed: 111 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,111 @@
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.List;
20+
import java.util.Set;
21+
import org.sonar.check.Rule;
22+
import org.sonar.plugins.python.api.PythonSubscriptionCheck;
23+
import org.sonar.plugins.python.api.SubscriptionContext;
24+
import org.sonar.plugins.python.api.tree.Argument;
25+
import org.sonar.plugins.python.api.tree.CallExpression;
26+
import org.sonar.plugins.python.api.tree.Expression;
27+
import org.sonar.plugins.python.api.tree.Name;
28+
import org.sonar.plugins.python.api.tree.QualifiedExpression;
29+
import org.sonar.plugins.python.api.tree.RegularArgument;
30+
import org.sonar.plugins.python.api.tree.Tree;
31+
import org.sonar.plugins.python.api.types.v2.matchers.TypeMatcher;
32+
import org.sonar.plugins.python.api.types.v2.matchers.TypeMatchers;
33+
import org.sonar.python.tree.TreeUtils;
34+
35+
@Rule(key = "S8508")
36+
public class MutableDefaultValueCheck extends PythonSubscriptionCheck {
37+
38+
private static final String MESSAGE = "Replace this mutable value with an immutable default to avoid shared state.";
39+
40+
private static final TypeMatcher IS_DICT_TYPE = TypeMatchers.isType("builtins.dict");
41+
private static final TypeMatcher IS_CONTEXT_VAR = TypeMatchers.isType("contextvars.ContextVar");
42+
private static final TypeMatcher IS_MUTABLE_CONSTRUCTOR = TypeMatchers.any(
43+
TypeMatchers.isType("builtins.list"),
44+
TypeMatchers.isType("builtins.dict"),
45+
TypeMatchers.isType("builtins.set"),
46+
TypeMatchers.isType("builtins.bytearray")
47+
);
48+
49+
@Override
50+
public void initialize(Context context) {
51+
context.registerSyntaxNodeConsumer(Tree.Kind.CALL_EXPR, MutableDefaultValueCheck::checkCall);
52+
}
53+
54+
private static void checkCall(SubscriptionContext ctx) {
55+
CallExpression callExpression = (CallExpression) ctx.syntaxNode();
56+
Expression callee = callExpression.callee();
57+
if (isDictFromkeys(callee, ctx)) {
58+
checkDictFromkeys(ctx, callExpression);
59+
} else if (IS_CONTEXT_VAR.isTrueFor(callee, ctx)) {
60+
checkContextVar(ctx, callExpression);
61+
}
62+
}
63+
64+
private static boolean isDictFromkeys(Expression callee, SubscriptionContext ctx) {
65+
if (!(callee instanceof QualifiedExpression qualifiedExpr)) {
66+
return false;
67+
}
68+
return "fromkeys".equals(qualifiedExpr.name().name())
69+
&& IS_DICT_TYPE.isTrueFor(qualifiedExpr.qualifier(), ctx);
70+
}
71+
72+
private static void checkDictFromkeys(SubscriptionContext ctx, CallExpression callExpression) {
73+
List<Argument> arguments = callExpression.arguments();
74+
if (arguments.size() < 2) {
75+
return;
76+
}
77+
Argument secondArg = arguments.get(1);
78+
if (secondArg instanceof RegularArgument regularArg && regularArg.keywordArgument() == null) {
79+
checkAndReportIfMutable(regularArg.expression(), ctx);
80+
}
81+
}
82+
83+
private static void checkContextVar(SubscriptionContext ctx, CallExpression callExpression) {
84+
RegularArgument defaultArg = TreeUtils.argumentByKeyword("default", callExpression.arguments());
85+
if (defaultArg != null) {
86+
checkAndReportIfMutable(defaultArg.expression(), ctx);
87+
}
88+
}
89+
90+
private static void checkAndReportIfMutable(Expression value, SubscriptionContext ctx) {
91+
if (isMutableValue(value, ctx)) {
92+
ctx.addIssue(value, MESSAGE);
93+
}
94+
}
95+
96+
private static boolean isMutableValue(Expression expression, SubscriptionContext ctx) {
97+
if (expression.is(Tree.Kind.LIST_LITERAL)
98+
|| expression.is(Tree.Kind.DICTIONARY_LITERAL)
99+
|| expression.is(Tree.Kind.SET_LITERAL)) {
100+
return true;
101+
}
102+
if (expression instanceof CallExpression callExpr) {
103+
return IS_MUTABLE_CONSTRUCTOR.isTrueFor(callExpr.callee(), ctx);
104+
}
105+
if (expression instanceof Name name) {
106+
Set<Expression> values = ctx.valuesAtLocation(name);
107+
return !values.isEmpty() && values.stream().allMatch(v -> isMutableValue(v, ctx));
108+
}
109+
return false;
110+
}
111+
}

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
@@ -316,6 +316,7 @@ public Stream<Class<?>> getChecks() {
316316
ModifiedParameterValueCheck.class,
317317
ModuleNameCheck.class,
318318
MultipleWhitespaceCheck.class,
319+
MutableDefaultValueCheck.class,
319320
NeedlessPassCheck.class,
320321
NestedCollectionsCreationCheck.class,
321322
NestedConditionalExpressionCheck.class,
Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,60 @@
1+
<p>This rule raises an issue when a mutable object is used as the default value in <code>dict.fromkeys()</code> or <code>contextvars.ContextVar()</code>.</p>
2+
<h2>Why is this an issue?</h2>
3+
<p>In Python, when you pass a mutable object as a default value to certain constructs, all instances share the same object reference rather than getting
4+
independent copies. This creates unintended shared state that leads to subtle mutation bugs.</p>
5+
<p>Python evaluates the default value expression once when the code is defined, not each time it is used. The same object instance is then reused
6+
across all keys or contexts.</p>
7+
<h3>The problem with <code>dict.fromkeys()</code></h3>
8+
<p>The <code>dict.fromkeys(keys, value)</code> method creates a dictionary with all specified keys pointing to the <em>same</em> <code>value</code>
9+
object. When this value is mutable (like a list or dictionary), all keys share the same object reference:</p>
10+
<pre>
11+
keys = ['a', 'b', 'c']
12+
my_dict = dict.fromkeys(keys, [])
13+
my_dict['a'].append(1)
14+
print(my_dict) # {'a': [1], 'b': [1], 'c': [1]}
15+
</pre>
16+
<h3>The problem with <code>ContextVar()</code></h3>
17+
<p>When you create a <code>ContextVar</code> with a mutable default value, that same object is shared across all contexts, which is particularly
18+
problematic in concurrent code.</p>
19+
<h3>What is the potential impact?</h3>
20+
<ul>
21+
<li><strong>Data corruption</strong>: changes intended for one key or context accidentally affect others</li>
22+
<li><strong>Concurrency issues</strong>: in the case of <code>ContextVar</code>, shared mutable defaults can cause race conditions</li>
23+
<li><strong>Test flakiness</strong>: tests may pass or fail inconsistently depending on execution order</li>
24+
</ul>
25+
<h2>How to fix it</h2>
26+
<p>For <code>dict.fromkeys()</code>, replace it with a dictionary comprehension that creates a new instance for each key. For
27+
<code>ContextVar()</code>, use a factory pattern or omit the mutable default.</p>
28+
<h3>Code examples</h3>
29+
<h4>Noncompliant code example</h4>
30+
<pre data-diff-id="1" data-diff-type="noncompliant">
31+
keys = ['a', 'b', 'c']
32+
my_dict = dict.fromkeys(keys, []) # Noncompliant
33+
</pre>
34+
<h4>Compliant solution</h4>
35+
<pre data-diff-id="1" data-diff-type="compliant">
36+
keys = ['a', 'b', 'c']
37+
my_dict = {key: [] for key in keys}
38+
</pre>
39+
<h4>Noncompliant code example</h4>
40+
<pre data-diff-id="2" data-diff-type="noncompliant">
41+
from contextvars import ContextVar
42+
43+
my_var = ContextVar('my_var', default=[]) # Noncompliant
44+
</pre>
45+
<h4>Compliant solution</h4>
46+
<pre data-diff-id="2" data-diff-type="compliant">
47+
from contextvars import ContextVar
48+
49+
my_var: ContextVar[list] = ContextVar('my_var')
50+
</pre>
51+
<h2>Resources</h2>
52+
<h3>Documentation</h3>
53+
<ul>
54+
<li>Python Documentation - <a href="https://docs.python.org/3/library/stdtypes.html#dict.fromkeys"><code>dict.fromkeys()</code> method</a></li>
55+
<li>Python Documentation - <a href="https://docs.python.org/3/library/contextvars.html#contextvars.ContextVar"><code>contextvars.ContextVar</code></a></li>
56+
</ul>
57+
<h3>Related rules</h3>
58+
<ul>
59+
<li>S2824 - Function parameters should not have mutable default values</li>
60+
</ul>
Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
{
2+
"title": "Mutable default values should not be used with \"dict.fromkeys()\" or \"ContextVar()\"",
3+
"type": "BUG",
4+
"status": "ready",
5+
"remediation": {
6+
"func": "Constant\/Issue",
7+
"constantCost": "5 min"
8+
},
9+
"tags": [],
10+
"defaultSeverity": "Blocker",
11+
"ruleSpecification": "RSPEC-8508",
12+
"sqKey": "S8508",
13+
"scope": "All",
14+
"quickfix": "unknown",
15+
"code": {
16+
"impacts": {
17+
"RELIABILITY": "BLOCKER"
18+
},
19+
"attribute": "LOGICAL"
20+
}
21+
}
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 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 MutableDefaultValueCheckTest {
23+
24+
@Test
25+
void test() {
26+
PythonCheckVerifier.verify("src/test/resources/checks/mutableDefaultValueCheck.py", new MutableDefaultValueCheck());
27+
}
28+
29+
}

0 commit comments

Comments
 (0)