Skip to content

Commit 16f0a5c

Browse files
guillaume-dequenneguillaume-dequenne
authored
SONARPY-733 Exclude metaclasses for S3862 & S5644 (SonarSource#794)
* SONARPY-733 Exclude metaclasses for S3862 * S5644 Avoid FPs on decorated classes
1 parent 81f6737 commit 16f0a5c

10 files changed

Lines changed: 128 additions & 36 deletions

File tree

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -134,7 +134,8 @@ private static boolean canHaveMethod(Symbol symbol, String requiredMethod, @Null
134134
}
135135
ClassSymbol classSymbol = (ClassSymbol) symbol;
136136
return classSymbol.canHaveMember(requiredMethod)
137-
|| (classRequiredMethod != null && classSymbol.canHaveMember(classRequiredMethod));
137+
|| (classRequiredMethod != null && classSymbol.canHaveMember(classRequiredMethod))
138+
|| classSymbol.hasDecorators();
138139
}
139140

140141
private static void reportIssue(SubscriptionExpression subscriptionExpression, Expression subscriptionObject,

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -159,7 +159,8 @@ private static boolean isValidIterable(Expression expression, List<LocationInFil
159159
}
160160
if (symbol.is(Symbol.Kind.CLASS)) {
161161
secondaries.add(((ClassSymbol) symbol).definitionLocation());
162-
return false;
162+
// Metaclasses might add the method by default
163+
return ((ClassSymbol) symbol).hasMetaClass();
163164
}
164165
}
165166
}

‎python-checks/src/test/resources/checks/itemOperationsTypeCheck/itemOperations_delitem.py‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -146,8 +146,8 @@ def __delitem__(cls, key):
146146
class MetaclassedWithDelete(metaclass=MyMetaClassWithDelete):
147147
pass
148148

149-
del MetaclassedWithDelete[0] # Ok
150-
del MetaclassedWithDelete()[0] # Ok. Pylint False Positive
149+
del MetaclassedWithDelete[0] # OK
150+
del MetaclassedWithDelete()[0] # OK
151151

152152

153153
class MyMetaClassWithoutDelete(type):
@@ -156,5 +156,5 @@ class MyMetaClassWithoutDelete(type):
156156
class MetaclassedWithoutDelete(metaclass=MyMetaClassWithoutDelete):
157157
pass
158158

159-
del MetaclassedWithoutDelete[0] # Noncompliant
160-
del MetaclassedWithoutDelete()[0] # Noncompliant
159+
del MetaclassedWithoutDelete[0] # FN
160+
del MetaclassedWithoutDelete()[0] # FN

‎python-checks/src/test/resources/checks/itemOperationsTypeCheck/itemOperations_getitem.py‎

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -148,19 +148,29 @@ def __getitem__(cls, key):
148148

149149
class MetaclassedWithGet(metaclass=MyMetaClassWithGet): ...
150150

151-
MetaclassedWithGet[0] # Ok
152-
MetaclassedWithGet()[0] # Ok. Pylint False Positive
151+
MetaclassedWithGet[0] # OK
152+
MetaclassedWithGet()[0] # OK
153153

154154

155155
class MyMetaClassWithoutGet(type): ...
156156
class MetaclassedWithoutGet(metaclass=MyMetaClassWithoutGet): ...
157157

158-
MetaclassedWithoutGet[0] # Noncompliant
159-
MetaclassedWithoutGet()[0] # Noncompliant
158+
MetaclassedWithoutGet[0] # FN
159+
MetaclassedWithoutGet()[0] # FN
160160

161161
def type_annotations():
162162
"""No issue as type annotations do no call item methods"""
163163
from typing import Awaitable
164164
def my_func() -> Awaitable[bool]: ... # OK
165165
def my_other_func(arg: Awaitable[bool]): ... # OK
166166
x: Awaitable[bool] # OK
167+
168+
169+
def decorated_classes():
170+
import enum
171+
@enum.unique
172+
class MyEnum(enum.Enum):
173+
first = 0
174+
second = 1
175+
176+
print(MyEnum["first"]) # OK

‎python-checks/src/test/resources/checks/itemOperationsTypeCheck/itemOperations_setitem.py‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -147,13 +147,13 @@ def __setitem__(cls, key, value):
147147
class MetaclassedWithSet(metaclass=MyMetaClassWithSet):
148148
pass
149149

150-
MetaclassedWithSet[0] = 42 # Ok
151-
MetaclassedWithSet()[0] = 42 # Ok. Pylint False Positive
150+
MetaclassedWithSet[0] = 42 # OK
151+
MetaclassedWithSet()[0] = 42 # OK
152152

153153

154154
class MyMetaClassWithoutSet(type): ...
155155

156156
class MetaclassedWithoutSet(metaclass=MyMetaClassWithoutSet): ...
157157

158-
MetaclassedWithoutSet[0] = 42 # Noncompliant
159-
MetaclassedWithoutSet()[0] = 42 # Noncompliant
158+
MetaclassedWithoutSet[0] = 42 # FN
159+
MetaclassedWithoutSet()[0] = 42 # FN

‎python-checks/src/test/resources/checks/iterationOnNonIterable.py‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -177,8 +177,8 @@ class MyMetaClassWithoutIter(type): ...
177177
class MetaclassedNonIterable(metaclass=MyMetaClassWithoutIter): ...
178178

179179
# Accepted FNs
180-
a, *rest = MetaclassedNonIterable # Noncompliant
181-
a, *rest = MetaclassedNonIterable() # Noncompliant
180+
a, *rest = MetaclassedNonIterable # FN
181+
a, *rest = MetaclassedNonIterable() # FN
182182

183183
def myiter(self):
184184
return iter(range(10))
@@ -191,8 +191,7 @@ def __new__(cls, name, bases, dct):
191191

192192
class MetaclassedIterable(metaclass=MyMetaClassWithIter): ...
193193

194-
# FP (SONARPY-733)
195-
a, *rest = MetaclassedIterable() # Noncompliant
194+
a, *rest = MetaclassedIterable() # OK
196195

197196
def attributes_and_properties():
198197
"""Out of scope: Detecting when a non-iterable class and instance attribute is iterated over."""

‎python-frontend/src/main/java/org/sonar/plugins/python/api/symbols/ClassSymbol.java‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,9 @@ public interface ClassSymbol extends Symbol {
3939
@Beta
4040
Optional<Symbol> resolveMember(String memberName);
4141

42+
@Beta
43+
boolean hasMetaClass();
44+
4245
@Beta
4346
boolean canHaveMember(String memberName);
4447

‎python-frontend/src/main/java/org/sonar/python/semantic/ClassSymbolImpl.java‎

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,7 @@ public class ClassSymbolImpl extends SymbolImpl implements ClassSymbol {
5454
private boolean hasAlreadyReadSuperClasses = false;
5555
private boolean hasAlreadyReadMembers = false;
5656
private boolean hasDecorators = false;
57+
private boolean hasMetaClass = false;
5758
private final LocationInFile classDefinitionLocation;
5859

5960
public ClassSymbolImpl(ClassDef classDef, @Nullable String fullyQualifiedName, PythonFile pythonFile) {
@@ -150,9 +151,14 @@ public Optional<Symbol> resolveMember(String memberName) {
150151
return Optional.empty();
151152
}
152153

154+
@Override
155+
public boolean hasMetaClass() {
156+
return hasMetaClass || membersByName().get("__metaclass__") != null;
157+
}
158+
153159
@Override
154160
public boolean canHaveMember(String memberName) {
155-
if (hasUnresolvedTypeHierarchy()) {
161+
if (hasUnresolvedTypeHierarchy() || hasMetaClass()) {
156162
return true;
157163
}
158164
for (Symbol symbol : allSuperClasses(true)) {
@@ -215,6 +221,10 @@ public void setHasSuperClassWithoutSymbol() {
215221
this.hasSuperClassWithoutSymbol = true;
216222
}
217223

224+
public void setHasMetaClass() {
225+
this.hasMetaClass = true;
226+
}
227+
218228
private Set<Symbol> allSuperClasses(boolean includeAmbiguousSymbols) {
219229
if (!includeAmbiguousSymbols) {
220230
if (allSuperClasses == null) {

‎python-frontend/src/main/java/org/sonar/python/semantic/SymbolUtils.java‎

Lines changed: 30 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -81,7 +81,7 @@ public static String fullyQualifiedModuleName(String packageName, String fileNam
8181
}
8282

8383
static void resolveTypeHierarchy(ClassDef classDef, @Nullable Symbol symbol, PythonFile pythonFile, Map<String, Symbol> symbolsByName) {
84-
if (symbol == null || !Symbol.Kind.CLASS.equals(symbol.kind())) {
84+
if (symbol == null || !CLASS.equals(symbol.kind())) {
8585
return;
8686
}
8787
ClassSymbolImpl classSymbol = (ClassSymbolImpl) symbol;
@@ -95,14 +95,29 @@ static void resolveTypeHierarchy(ClassDef classDef, @Nullable Symbol symbol, Pyt
9595
return;
9696
}
9797
for (Argument argument : argList.arguments()) {
98-
Symbol argumentSymbol = getSymbolFromArgument(argument);
99-
if (argumentSymbol == null) {
98+
if (!argument.is(Kind.REGULAR_ARGUMENT)) {
10099
classSymbol.setHasSuperClassWithoutSymbol();
101100
} else {
102-
Symbol normalizedArgumentSymbol = normalizeSymbol(argumentSymbol, pythonFile, symbolsByName);
103-
if (normalizedArgumentSymbol != null) {
104-
classSymbol.addSuperClass(normalizedArgumentSymbol);
105-
}
101+
addParentClass(pythonFile, symbolsByName, classSymbol, (RegularArgument) argument);
102+
}
103+
}
104+
}
105+
106+
private static void addParentClass(PythonFile pythonFile, Map<String, Symbol> symbolsByName, ClassSymbolImpl classSymbol, RegularArgument regularArgument) {
107+
Name keyword = regularArgument.keywordArgument();
108+
if (keyword != null) {
109+
if (keyword.name().equals("metaclass")) {
110+
classSymbol.setHasMetaClass();
111+
}
112+
return;
113+
}
114+
Symbol argumentSymbol = getSymbolFromArgument(regularArgument);
115+
if (argumentSymbol == null) {
116+
classSymbol.setHasSuperClassWithoutSymbol();
117+
} else {
118+
Symbol normalizedArgumentSymbol = normalizeSymbol(argumentSymbol, pythonFile, symbolsByName);
119+
if (normalizedArgumentSymbol != null) {
120+
classSymbol.addSuperClass(normalizedArgumentSymbol);
106121
}
107122
}
108123
}
@@ -131,16 +146,14 @@ private static boolean isTypingFile(PythonFile pythonFile) {
131146
}
132147

133148
@CheckForNull
134-
private static Symbol getSymbolFromArgument(Argument argument) {
135-
if (argument.is(Kind.REGULAR_ARGUMENT)) {
136-
Expression expression = ((RegularArgument) argument).expression();
137-
while (expression.is(Kind.SUBSCRIPTION)) {
138-
// to support using 'typing' symbols like 'List[str]'
139-
expression = ((SubscriptionExpression) expression).object();
140-
}
141-
if (expression instanceof HasSymbol) {
142-
return ((HasSymbol) expression).symbol();
143-
}
149+
private static Symbol getSymbolFromArgument(RegularArgument regularArgument) {
150+
Expression expression = regularArgument.expression();
151+
while (expression.is(Kind.SUBSCRIPTION)) {
152+
// to support using 'typing' symbols like 'List[str]'
153+
expression = ((SubscriptionExpression) expression).object();
154+
}
155+
if (expression instanceof HasSymbol) {
156+
return ((HasSymbol) expression).symbol();
144157
}
145158
return null;
146159
}

‎python-frontend/src/test/java/org/sonar/python/semantic/ClassSymbolTest.java‎

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -230,6 +230,61 @@ public void parent_has_multiple_bindings() {
230230
assertThat(classSymbol.hasUnresolvedTypeHierarchy()).isTrue();
231231
}
232232

233+
@Test
234+
public void defines_metaclass() {
235+
FileInput fileInput = parse(
236+
"class A: ",
237+
" pass",
238+
"class B(metaclass=A): ",
239+
" pass");
240+
ClassDef classDef = (ClassDef) fileInput.statements().statements().get(1);
241+
Symbol symbol = classDef.name().symbol();
242+
assertThat(symbol).isInstanceOf(ClassSymbol.class);
243+
assertThat(symbol.kind()).isEqualTo(Symbol.Kind.CLASS);
244+
ClassSymbol classSymbol = (ClassSymbol) symbol;
245+
assertThat(classSymbol.hasUnresolvedTypeHierarchy()).isFalse();
246+
assertThat(classSymbol.superClasses()).isEmpty();
247+
assertThat(classSymbol.hasMetaClass()).isTrue();
248+
assertThat(classSymbol.canHaveMember("foo")).isTrue();
249+
}
250+
251+
@Test
252+
public void defines_metaclass_python_2() {
253+
FileInput fileInput = parse(
254+
"class A: ",
255+
" pass",
256+
"class B(): ",
257+
" __metaclass__ = A");
258+
ClassDef classDef = (ClassDef) fileInput.statements().statements().get(1);
259+
Symbol symbol = classDef.name().symbol();
260+
assertThat(symbol).isInstanceOf(ClassSymbol.class);
261+
assertThat(symbol.kind()).isEqualTo(Symbol.Kind.CLASS);
262+
ClassSymbol classSymbol = (ClassSymbol) symbol;
263+
assertThat(classSymbol.hasUnresolvedTypeHierarchy()).isFalse();
264+
assertThat(classSymbol.superClasses()).isEmpty();
265+
assertThat(classSymbol.hasMetaClass()).isTrue();
266+
assertThat(classSymbol.canHaveMember("foo")).isTrue();
267+
}
268+
269+
@Test
270+
public void defines_attrs() {
271+
FileInput fileInput = parse(
272+
"class A: ",
273+
" pass",
274+
"class B(A, attrs=...): ",
275+
" pass");
276+
ClassDef classDef = (ClassDef) fileInput.statements().statements().get(1);
277+
Symbol symbol = classDef.name().symbol();
278+
assertThat(symbol).isInstanceOf(ClassSymbol.class);
279+
assertThat(symbol.kind()).isEqualTo(Symbol.Kind.CLASS);
280+
ClassSymbol classSymbol = (ClassSymbol) symbol;
281+
assertThat(classSymbol.hasUnresolvedTypeHierarchy()).isFalse();
282+
assertThat(classSymbol.superClasses()).hasSize(1);
283+
assertThat(classSymbol.superClasses()).extracting(Symbol::name).containsExactly("A");
284+
assertThat(classSymbol.hasMetaClass()).isFalse();
285+
assertThat(classSymbol.canHaveMember("foo")).isFalse();
286+
}
287+
233288
@Test
234289
public void class_with_global_statement() {
235290
FileInput fileInput = parse(

0 commit comments

Comments
 (0)