Skip to content

Commit 45b742e

Browse files
sonar-nigel[bot]sonartech
authored andcommitted
SONARPY-3991 Rule S8514 Dataclass attributes should use type annotations (#1017)
GitOrigin-RevId: 3535711f601fecbb4368b28f8f07e1632c74d2a5
1 parent 9a90495 commit 45b742e

7 files changed

Lines changed: 363 additions & 0 deletions

File tree

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,70 @@
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.tree.AssignmentStatement;
23+
import org.sonar.plugins.python.api.tree.CallExpression;
24+
import org.sonar.plugins.python.api.tree.ClassDef;
25+
import org.sonar.plugins.python.api.tree.Decorator;
26+
import org.sonar.plugins.python.api.tree.Expression;
27+
import org.sonar.plugins.python.api.tree.Tree;
28+
import org.sonar.plugins.python.api.types.v2.matchers.TypeMatcher;
29+
import org.sonar.plugins.python.api.types.v2.matchers.TypeMatchers;
30+
31+
@Rule(key = "S8514")
32+
public class DataclassFieldDefinitionCheck extends PythonSubscriptionCheck {
33+
34+
private static final String MESSAGE_MISSING_ANNOTATION = "Add a type annotation to this dataclass attribute.";
35+
36+
private static final TypeMatcher IS_DATACLASS = TypeMatchers.isType("dataclasses.dataclass");
37+
38+
@Override
39+
public void initialize(Context context) {
40+
context.registerSyntaxNodeConsumer(Tree.Kind.CLASSDEF, DataclassFieldDefinitionCheck::checkClassDef);
41+
}
42+
43+
private static void checkClassDef(SubscriptionContext ctx) {
44+
ClassDef classDef = (ClassDef) ctx.syntaxNode();
45+
46+
if (!isDataclass(classDef, ctx)) {
47+
return;
48+
}
49+
50+
classDef.body().statements().forEach(statement -> checkStatement(ctx, statement));
51+
}
52+
53+
private static boolean isDataclass(ClassDef classDef, SubscriptionContext ctx) {
54+
return classDef.decorators().stream().anyMatch(decorator -> isDataclassDecorator(decorator, ctx));
55+
}
56+
57+
private static boolean isDataclassDecorator(Decorator decorator, SubscriptionContext ctx) {
58+
Expression expression = decorator.expression();
59+
if (expression instanceof CallExpression callExpr) {
60+
return IS_DATACLASS.isTrueFor(callExpr.callee(), ctx);
61+
}
62+
return IS_DATACLASS.isTrueFor(expression, ctx);
63+
}
64+
65+
private static void checkStatement(SubscriptionContext ctx, Tree statement) {
66+
if (statement instanceof AssignmentStatement assignmentStatement) {
67+
ctx.addIssue(assignmentStatement, MESSAGE_MISSING_ANNOTATION);
68+
}
69+
}
70+
}

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
@@ -182,6 +182,7 @@ public Stream<Class<?>> getChecks() {
182182
CorsMiddlewareOrderingCheck.class,
183183
CsrfDisabledCheck.class,
184184
DataClassOnEnumCheck.class,
185+
DataclassFieldDefinitionCheck.class,
185186
DataEncryptionCheck.class,
186187
DbNoPasswordCheck.class,
187188
DeadStoreCheck.class,
Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
<p>This rule raises an issue when a dataclass contains attributes without type annotations.</p>
2+
<h2>Why is this an issue?</h2>
3+
<p>The <code>@dataclass</code> decorator only processes attributes that have type annotations. Attributes without annotations are silently excluded
4+
from dataclass processing and become regular class variables instead of instance attributes. This means:</p>
5+
<ul>
6+
<li>they are not included in the generated <code>\__init__</code> method</li>
7+
<li>they are not part of <code>\__repr__</code> or <code>\__eq__</code></li>
8+
<li>they behave as class variables, not instance variables</li>
9+
</ul>
10+
<h3>What is the potential impact?</h3>
11+
<p>Unannotated attributes are silently excluded from initialization, <code>\__repr__</code>, and <code>\__eq__</code>, making them invisible to the
12+
dataclass contract. These bugs are difficult to trace because the class appears to work normally, but the affected attributes do not participate in
13+
any of the generated methods.</p>
14+
<h2>How to fix it</h2>
15+
<p>Add type annotations to all attributes that should be dataclass fields. If you intend an attribute to be a class variable shared across instances,
16+
move it outside the dataclass or document this intention clearly.</p>
17+
<h3>Code examples</h3>
18+
<h4>Noncompliant code example</h4>
19+
<pre data-diff-id="1" data-diff-type="noncompliant">
20+
from dataclasses import dataclass
21+
22+
@dataclass
23+
class Config:
24+
timeout = 30 # Noncompliant: missing type annotation
25+
retries = 3 # Noncompliant: missing type annotation
26+
</pre>
27+
<h4>Compliant solution</h4>
28+
<pre data-diff-id="1" data-diff-type="compliant">
29+
from dataclasses import dataclass
30+
31+
@dataclass
32+
class Config:
33+
timeout: int = 30
34+
retries: int = 3
35+
</pre>
36+
<h2>Resources</h2>
37+
<h3>Documentation</h3>
38+
<ul>
39+
<li>Python Documentation - <a href="https://docs.python.org/3/library/dataclasses.html">dataclasses — Data Classes</a></li>
40+
<li>PEP 557 - <a href="https://peps.python.org/pep-0557/">Data Classes</a></li>
41+
</ul>
42+
Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
{
2+
"title": "Dataclass attributes should use type annotations",
3+
"type": "CODE_SMELL",
4+
"status": "ready",
5+
"remediation": {
6+
"func": "Constant\/Issue",
7+
"constantCost": "5min"
8+
},
9+
"tags": [
10+
"pitfall",
11+
"confusing"
12+
],
13+
"defaultSeverity": "Major",
14+
"ruleSpecification": "RSPEC-8514",
15+
"sqKey": "S8514",
16+
"scope": "All",
17+
"quickfix": "targeted",
18+
"code": {
19+
"impacts": {
20+
"RELIABILITY": "HIGH",
21+
"MAINTAINABILITY": "MEDIUM"
22+
},
23+
"attribute": "CONVENTIONAL"
24+
}
25+
}

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
@@ -339,6 +339,7 @@
339339
"S8505",
340340
"S8507",
341341
"S8513",
342+
"S8514",
342343
"S8516",
343344
"S8517",
344345
"S8519",
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) 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 DataclassFieldDefinitionCheckTest {
23+
24+
@Test
25+
void test() {
26+
PythonCheckVerifier.verify("src/test/resources/checks/dataclassFieldDefinition.py", new DataclassFieldDefinitionCheck());
27+
}
28+
29+
}
Lines changed: 195 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,195 @@
1+
from dataclasses import dataclass, field, InitVar
2+
from datetime import datetime
3+
from typing import ClassVar, Optional
4+
5+
6+
# =======================
7+
# Issues
8+
# =======================
9+
10+
@dataclass
11+
class SingleUnannotatedAttribute:
12+
timeout = 30 # Noncompliant {{Add a type annotation to this dataclass attribute.}}
13+
# ^^^^^^^^^^^^
14+
15+
16+
@dataclass
17+
class MultipleUnannotatedAttributes:
18+
timeout = 30 # Noncompliant {{Add a type annotation to this dataclass attribute.}}
19+
# ^^^^^^^^^^^^
20+
retries = 3 # Noncompliant {{Add a type annotation to this dataclass attribute.}}
21+
# ^^^^^^^^^^^
22+
23+
24+
@dataclass
25+
class MixedAnnotatedAndUnannotated:
26+
name: str = "default"
27+
timeout = 30 # Noncompliant
28+
# ^^^^^^^^^^^^
29+
30+
31+
@dataclass(frozen=True)
32+
class FrozenDataclassUnannotated:
33+
timeout = 30 # Noncompliant
34+
35+
36+
@dataclass
37+
class MultipleViolations:
38+
unannotated = 42 # Noncompliant
39+
other = "value" # Noncompliant
40+
41+
42+
import dataclasses
43+
44+
@dataclasses.dataclass
45+
class QualifiedDecoratorNoncompliant:
46+
unannotated = 42 # Noncompliant
47+
48+
49+
@dataclass
50+
class ChildDataclassWithViolation:
51+
unannotated = 99 # Noncompliant
52+
extra: int = 0
53+
54+
55+
def outer_function():
56+
# FN: type inference resolves `@dataclass` to Unknown when the decorated class is nested in a function
57+
@dataclass
58+
class InnerDataclass:
59+
value = 10
60+
name: str = ""
61+
62+
63+
# =======================
64+
# Compliant
65+
# =======================
66+
67+
class RegularClassWithUnannotatedAttrs:
68+
timeout = 30
69+
retries = 3
70+
71+
72+
@dataclass
73+
class ImmutableScalarDefaults:
74+
count: int = 0
75+
name: str = ""
76+
flag: bool = False
77+
ratio: float = 1.0
78+
79+
80+
@dataclass
81+
class NoneDefault:
82+
opt: Optional[str] = None
83+
84+
85+
@dataclass
86+
class MutableDefaultsAreNotFlagged:
87+
members: list = []
88+
config: dict = {}
89+
tags: set = {1, 2}
90+
91+
92+
@dataclass
93+
class CallDefaultsAreNotFlagged:
94+
items: list = list()
95+
config: dict = dict()
96+
buffer: bytearray = bytearray()
97+
created_at: datetime = datetime.now()
98+
allowed: frozenset = frozenset([1, 2, 3])
99+
tags: set = set()
100+
101+
102+
@dataclass
103+
class FieldDefaultFactoryCompliant:
104+
members: list = field(default_factory=list)
105+
config: dict = field(default_factory=dict)
106+
created_at: datetime = field(default_factory=datetime.now)
107+
108+
109+
@dataclass
110+
class FieldDefaultCompliant:
111+
count: int = field(default=0)
112+
name: str = field(default="hello")
113+
derived: int = field(init=False, default=0)
114+
115+
116+
@dataclass
117+
class ClassVarAnnotations:
118+
count: ClassVar[int] = 5
119+
items: ClassVar[list] = []
120+
metadata: ClassVar[dict] = {}
121+
122+
123+
@dataclass
124+
class ClassVarBareForm:
125+
count: ClassVar = 0
126+
127+
128+
@dataclass
129+
class InitVarAnnotations:
130+
init_value: InitVar[int] = 1
131+
init_param: InitVar[str] = "default"
132+
133+
134+
@dataclass
135+
class NoDefaultAnnotatedFields:
136+
x: float
137+
y: int
138+
name: str
139+
140+
141+
@dataclass
142+
class OptionalWithNoneDefault:
143+
label: Optional[str] = None
144+
145+
146+
@dataclass(frozen=True)
147+
class FrozenDataclassCompliant:
148+
name: str = ""
149+
items: list = field(default_factory=list)
150+
151+
152+
@dataclasses.dataclass
153+
class QualifiedDecoratorCompliant:
154+
name: str = ""
155+
count: int = 0
156+
157+
158+
def make_default_list():
159+
return []
160+
161+
@dataclass
162+
class CustomFactoryCompliant:
163+
items: list = field(default_factory=make_default_list)
164+
165+
166+
@dataclass
167+
class TupleLiteralDefault:
168+
coords: tuple = (0, 0)
169+
170+
171+
@dataclass
172+
class NestedCallDefault:
173+
value: str = str(object())
174+
175+
176+
@dataclass
177+
class EmptyDataclass:
178+
pass
179+
180+
181+
@dataclass
182+
class DataclassWithOnlyMethods:
183+
name: str
184+
185+
def get_name(self) -> str:
186+
return self.name
187+
188+
189+
@dataclass
190+
class ParentDataclass:
191+
name: str = ""
192+
193+
194+
class NonDataclassChild(ParentDataclass):
195+
unannotated = 99

0 commit comments

Comments
 (0)