Skip to content

Commit 3825a61

Browse files
guillaume-dequennesonartech
guillaume-dequenne
authored and
sonartech
committed
SONARPY-4109 Implement S8685 Function calls should not be used as default values in dataclass attributes (#1071)
GitOrigin-RevId: c50f02aec7cc528ce74f47465ab571ef44bb8b02
1 parent 45b742e commit 3825a61

7 files changed

Lines changed: 643 additions & 1 deletion

File tree

‎python-checks/src/main/java/org/sonar/python/checks/DataClassFunctionCallDefaultCheck.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.Set;
20+
import java.util.stream.Stream;
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.AnnotatedAssignment;
25+
import org.sonar.plugins.python.api.tree.CallExpression;
26+
import org.sonar.plugins.python.api.tree.ClassDef;
27+
import org.sonar.plugins.python.api.tree.Decorator;
28+
import org.sonar.plugins.python.api.tree.Expression;
29+
import org.sonar.plugins.python.api.tree.QualifiedExpression;
30+
import org.sonar.plugins.python.api.tree.RegularArgument;
31+
import org.sonar.plugins.python.api.tree.SubscriptionExpression;
32+
import org.sonar.plugins.python.api.tree.Tree;
33+
import org.sonar.plugins.python.api.tree.TypeAnnotation;
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.tree.TreeUtils;
37+
38+
@Rule(key = "S8685")
39+
public class DataClassFunctionCallDefaultCheck extends PythonSubscriptionCheck {
40+
41+
private static final String MESSAGE = "Use \"field(default_factory=...)\" instead of a function call as a default value.";
42+
private static final String DEFAULT_KEYWORD = "default";
43+
44+
private static final TypeMatcher IS_DATACLASS_DECORATOR = TypeMatchers.isType("dataclasses.dataclass");
45+
private static final TypeMatcher IS_DATACLASSES_FIELD = TypeMatchers.isType("dataclasses.field");
46+
private static final TypeMatcher IS_CLASS_VAR = TypeMatchers.isType("typing.ClassVar");
47+
48+
// Allowlist of callables whose result should be re-evaluated per dataclass instance.
49+
// Calls to anything outside this list are assumed safe (e.g. user-defined helpers, frozen-dataclass
50+
// constructors, wrappers around dataclasses.field(default_factory=...)) — we prefer FNs over FPs here.
51+
//
52+
// NOTE: random.* top-level functions are intentionally absent. In typeshed they are aliased from a
53+
// module-level Random() instance (`randint = _inst.randint`, ...) and the V2 typeshed serializer
54+
// flattens the bound-method type to CallableType[builtins.function], so isType("random.randint")
55+
// resolves through Unknown. They are matched separately below via the random module qualifier.
56+
private static final TypeMatcher IS_PROBLEMATIC_FACTORY = TypeMatchers.any(Stream.of(
57+
// Current time / clock readings
58+
"datetime.datetime.now",
59+
"datetime.datetime.utcnow",
60+
"datetime.datetime.today",
61+
"datetime.datetime.fromtimestamp",
62+
"datetime.datetime.utcfromtimestamp",
63+
"datetime.date.today",
64+
"datetime.date.fromtimestamp",
65+
"time.time",
66+
"time.time_ns",
67+
"time.monotonic",
68+
"time.monotonic_ns",
69+
"time.perf_counter",
70+
"time.perf_counter_ns",
71+
"time.process_time",
72+
"time.process_time_ns",
73+
"time.localtime",
74+
"time.gmtime",
75+
// UUIDs
76+
"uuid.uuid1",
77+
"uuid.uuid3",
78+
"uuid.uuid4",
79+
"uuid.uuid5",
80+
// Secrets
81+
"secrets.token_hex",
82+
"secrets.token_bytes",
83+
"secrets.token_urlsafe",
84+
"secrets.choice",
85+
"secrets.randbelow",
86+
"secrets.randbits",
87+
// OS randomness
88+
"os.urandom",
89+
// Mutable container constructors — shared across all instances otherwise
90+
"builtins.list",
91+
"builtins.dict",
92+
"builtins.set",
93+
"builtins.bytearray",
94+
"collections.defaultdict",
95+
"collections.OrderedDict",
96+
"collections.Counter",
97+
"collections.deque").map(TypeMatchers::isType));
98+
99+
// The random module type itself is known, even though its top-level function members all resolve
100+
// to Unknown (see NOTE on IS_PROBLEMATIC_FACTORY). We match `<random>.<name>` syntactically.
101+
private static final TypeMatcher IS_RANDOM_MODULE = TypeMatchers.isType("random");
102+
103+
private static final Set<String> RANDOM_PROBLEMATIC_FUNCTION_NAMES = Set.of(
104+
"random",
105+
"randint",
106+
"randrange",
107+
"choice",
108+
"choices",
109+
"sample",
110+
"uniform",
111+
"gauss",
112+
"normalvariate",
113+
"triangular",
114+
"betavariate",
115+
"expovariate",
116+
"gammavariate",
117+
"lognormvariate",
118+
"paretovariate",
119+
"vonmisesvariate",
120+
"weibullvariate",
121+
"getrandbits",
122+
"randbytes");
123+
124+
@Override
125+
public void initialize(Context context) {
126+
context.registerSyntaxNodeConsumer(Tree.Kind.CLASSDEF, DataClassFunctionCallDefaultCheck::checkClassDef);
127+
}
128+
129+
private static void checkClassDef(SubscriptionContext ctx) {
130+
ClassDef classDef = (ClassDef) ctx.syntaxNode();
131+
if (!isDataclass(classDef, ctx)) {
132+
return;
133+
}
134+
classDef.body().statements().stream()
135+
.flatMap(TreeUtils.toStreamInstanceOfMapper(AnnotatedAssignment.class))
136+
.forEach(annotatedAssignment -> checkField(ctx, annotatedAssignment));
137+
}
138+
139+
private static boolean isDataclass(ClassDef classDef, SubscriptionContext ctx) {
140+
for (Decorator decorator : classDef.decorators()) {
141+
Expression decoratorExpr = getDecoratorFunctionExpression(decorator);
142+
if (IS_DATACLASS_DECORATOR.isTrueFor(decoratorExpr, ctx)) {
143+
return true;
144+
}
145+
}
146+
return false;
147+
}
148+
149+
private static Expression getDecoratorFunctionExpression(Decorator decorator) {
150+
Expression expr = decorator.expression();
151+
if (expr instanceof CallExpression callExpr) {
152+
return callExpr.callee();
153+
}
154+
return expr;
155+
}
156+
157+
private static void checkField(SubscriptionContext ctx, AnnotatedAssignment annotatedAssignment) {
158+
Expression assignedValue = annotatedAssignment.assignedValue();
159+
if (assignedValue == null || isClassVar(annotatedAssignment.annotation(), ctx)) {
160+
return;
161+
}
162+
if (isMutableLiteral(assignedValue)) {
163+
ctx.addIssue(assignedValue, MESSAGE);
164+
return;
165+
}
166+
if (assignedValue instanceof CallExpression callExpression) {
167+
Tree problematic = problematicCall(callExpression, ctx);
168+
if (problematic != null) {
169+
ctx.addIssue(problematic, MESSAGE);
170+
}
171+
}
172+
}
173+
174+
private static boolean isMutableLiteral(Expression expression) {
175+
return expression.is(Tree.Kind.LIST_LITERAL, Tree.Kind.DICTIONARY_LITERAL, Tree.Kind.SET_LITERAL);
176+
}
177+
178+
// Returns the expression to flag, or null. Handles all of:
179+
// x: T = datetime.now() -> the outer call
180+
// x: T = field(default=datetime.now()) -> the inner default argument
181+
// x: T = field(default=[]) -> the inner mutable literal
182+
private static Tree problematicCall(CallExpression callExpression, SubscriptionContext ctx) {
183+
if (isProblematicFactoryCall(callExpression, ctx)) {
184+
return callExpression;
185+
}
186+
if (IS_DATACLASSES_FIELD.isTrueFor(callExpression.callee(), ctx)) {
187+
RegularArgument defaultArg = TreeUtils.argumentByKeyword(DEFAULT_KEYWORD, callExpression.arguments());
188+
if (defaultArg != null) {
189+
Expression defaultExpr = defaultArg.expression();
190+
if (isMutableLiteral(defaultExpr)) {
191+
return defaultExpr;
192+
}
193+
if (defaultExpr instanceof CallExpression innerCall && isProblematicFactoryCall(innerCall, ctx)) {
194+
return innerCall;
195+
}
196+
}
197+
}
198+
return null;
199+
}
200+
201+
private static boolean isProblematicFactoryCall(CallExpression callExpression, SubscriptionContext ctx) {
202+
Expression callee = callExpression.callee();
203+
return IS_PROBLEMATIC_FACTORY.isTrueFor(callee, ctx) || isRandomModuleFunctionCall(callee, ctx);
204+
}
205+
206+
// Workaround for the random.* alias gap in the V2 typeshed serializer (see NOTE on
207+
// IS_PROBLEMATIC_FACTORY): the random module type itself is known, so we recognise
208+
// a problematic call by checking the qualifier's type and the syntactic name.
209+
private static boolean isRandomModuleFunctionCall(Expression callee, SubscriptionContext ctx) {
210+
if (!(callee instanceof QualifiedExpression qualifiedExpression)) {
211+
return false;
212+
}
213+
return RANDOM_PROBLEMATIC_FUNCTION_NAMES.contains(qualifiedExpression.name().name())
214+
&& IS_RANDOM_MODULE.isTrueFor(qualifiedExpression.qualifier(), ctx);
215+
}
216+
217+
private static boolean isClassVar(TypeAnnotation annotation, SubscriptionContext ctx) {
218+
Expression annotationExpr = annotation.expression();
219+
if (annotationExpr instanceof SubscriptionExpression subscriptionExpr) {
220+
return IS_CLASS_VAR.isTrueFor(subscriptionExpr.object(), ctx);
221+
}
222+
return IS_CLASS_VAR.isTrueFor(annotationExpr, ctx);
223+
}
224+
}

‎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
@@ -181,6 +181,7 @@ public Stream<Class<?>> getChecks() {
181181
CorsCheck.class,
182182
CorsMiddlewareOrderingCheck.class,
183183
CsrfDisabledCheck.class,
184+
DataClassFunctionCallDefaultCheck.class,
184185
DataClassOnEnumCheck.class,
185186
DataclassFieldDefinitionCheck.class,
186187
DataEncryptionCheck.class,

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

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,72 @@
1+
<p>Using a mutable or stateful value as a default for a dataclass attribute is rarely what developers intend. The value is evaluated only once, when
2+
the class is defined, so every instance shares the same object.</p>
3+
<h2>Why is this an issue?</h2>
4+
<p>Default values for dataclass attributes are evaluated once when the class is defined, not each time an instance is created. This is a fundamental
5+
aspect of how Python evaluates default values.</p>
6+
<p>When the default is produced by calling a function such as <code>datetime.now()</code> or <code>uuid.uuid4()</code>, or by a mutable container like
7+
<code>[]</code> or <code>list()</code>, the value is computed once and reused across every instance of the dataclass. Every instance therefore ends up
8+
with the same object.</p>
9+
<pre>
10+
@dataclass
11+
class Event:
12+
timestamp: datetime = datetime.now() # Noncompliant
13+
tags: list = [] # Noncompliant
14+
15+
event1 = Event()
16+
event2 = Event()
17+
event1.tags.append("x")
18+
# event2.tags is also ["x"] — tags is the same list object.
19+
# event1.timestamp and event2.timestamp are both the class-definition time.
20+
</pre>
21+
<p>In this example, <code>event1</code> and <code>event2</code> share the same <code>tags</code> list and the same timestamp.</p>
22+
<h3>What is the potential impact?</h3>
23+
<p>Using such defaults can cause reliability issues:</p>
24+
<ul>
25+
<li><strong>Broken uniqueness</strong>: attributes meant to be unique per instance, such as UUIDs or timestamps, become identical across all
26+
instances.</li>
27+
<li><strong>State sharing bugs</strong>: mutating the default through one instance silently affects every other instance and every instance created
28+
in the future.</li>
29+
<li><strong>Incorrect business logic</strong>: time-sensitive operations use a stale timestamp captured at class definition, which leads to
30+
incorrect ordering, expiration, or logging.</li>
31+
<li><strong>Hard-to-diagnose bugs</strong>: the issue may not be visible in short-lived tests, but appears in production where instances are created
32+
over a longer period of time or mutate their state.</li>
33+
</ul>
34+
<h2>How to fix it</h2>
35+
<p>Use <code>field(default_factory=…​)</code> instead. The <code>default_factory</code> parameter accepts a callable, without parentheses, that is
36+
invoked each time a new instance is created, so each instance receives a fresh value.</p>
37+
<p>The <code>default_factory</code> expects a callable with no arguments. If the function requires arguments, wrap it in a <code>lambda</code> or use
38+
<code>functools.partial</code>.</p>
39+
<h3>Code examples</h3>
40+
<h4>Noncompliant code example</h4>
41+
<pre data-diff-id="1" data-diff-type="noncompliant">
42+
@dataclass
43+
class Event:
44+
timestamp: datetime = datetime.now() # Noncompliant
45+
event_id: uuid.UUID = uuid.uuid4() # Noncompliant
46+
tags: list = [] # Noncompliant
47+
</pre>
48+
<h4>Compliant solution</h4>
49+
<pre data-diff-id="1" data-diff-type="compliant">
50+
from dataclasses import dataclass, field
51+
52+
@dataclass
53+
class Event:
54+
timestamp: datetime = field(default_factory=datetime.now)
55+
event_id: uuid.UUID = field(default_factory=uuid.uuid4)
56+
tags: list = field(default_factory=list)
57+
</pre>
58+
<h2>Resources</h2>
59+
<h3>Documentation</h3>
60+
<ul>
61+
<li>Python dataclasses documentation - <a href="https://docs.python.org/3/library/dataclasses.html">Official Python documentation for the
62+
dataclasses module</a></li>
63+
<li>Python dataclasses - default_factory - <a href="https://docs.python.org/3/library/dataclasses.html#dataclasses.field">Documentation on the
64+
field() function and default_factory parameter</a></li>
65+
<li>Ruff RUF009 - <a href="https://docs.astral.sh/ruff/rules/function-call-in-dataclass-default-argument/">Ruff’s documentation for rule
66+
RUF009</a></li>
67+
</ul>
68+
<h3>Related rules</h3>
69+
<ul>
70+
<li>{rule:python:S6891} - Mutable objects should not be used as default parameter values</li>
71+
</ul>
72+

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

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
{
2+
"title": "Mutable or stateful values should not be used as default values in dataclass attributes",
3+
"type": "BUG",
4+
"status": "ready",
5+
"remediation": {
6+
"func": "Constant\/Issue",
7+
"constantCost": "5min"
8+
},
9+
"tags": [
10+
"dataclass",
11+
"pitfall"
12+
],
13+
"defaultSeverity": "Major",
14+
"ruleSpecification": "RSPEC-8685",
15+
"sqKey": "S8685",
16+
"scope": "Main",
17+
"quickfix": "unknown",
18+
"code": {
19+
"impacts": {
20+
"RELIABILITY": "MEDIUM"
21+
},
22+
"attribute": "LOGICAL"
23+
}
24+
}

‎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
@@ -346,6 +346,7 @@
346346
"S8520",
347347
"S8521",
348348
"S8554",
349-
"S8572"
349+
"S8572",
350+
"S8685"
350351
]
351352
}

‎python-checks/src/test/java/org/sonar/python/checks/DataClassFunctionCallDefaultCheckTest.java‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
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.junit.jupiter.api.Test;
20+
import org.sonar.python.checks.utils.PythonCheckVerifier;
21+
22+
class DataClassFunctionCallDefaultCheckTest {
23+
24+
@Test
25+
void test() {
26+
PythonCheckVerifier.verify("src/test/resources/checks/dataClassFunctionCallDefault.py", new DataClassFunctionCallDefaultCheck());
27+
}
28+
}

0 commit comments

Comments
 (0)