Skip to content

Commit 93508ba

Browse files
guillaume-dequennesonartech
authored andcommitted
CLP-1018 Represent class decorators with semantic type metadata
Preserve ordered decorator expression types and optional factory callee types through inference, lazy resolution, descriptors, and serialization. Derive the conservative V1 identity projection from this metadata and classify known runtime effects centrally. Cover aliases, factories, unions, unknown decorators, enclosing scopes, cache round trips, and affected checks. Extend the existing arithmetic and subscription fixtures with class-preserving decorator scenarios. GitOrigin-RevId: ce41b040fa717025dd84a8cf4bc25e20d6218cdb
1 parent 541f3ee commit 93508ba

52 files changed

Lines changed: 2517 additions & 34 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,7 @@ private static class ItemOperationsTypeCheck extends ItemOperationsType {
8787

8888
@Override
8989
public boolean isValidSubscription(Expression subscriptionObject, String requiredMethod, @Nullable String classRequiredMethod,
90-
Map<LocationInFile, String> secondaries) {
90+
Map<LocationInFile, String> secondaries, SubscriptionContext context) {
9191

9292
InferredType type = subscriptionObject.type();
9393
String typeName = InferredTypes.typeName(type);

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

Lines changed: 22 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -29,18 +29,24 @@
2929
import org.sonar.plugins.python.api.symbols.Symbol;
3030
import org.sonar.plugins.python.api.tree.BaseTreeVisitor;
3131
import org.sonar.plugins.python.api.tree.BinaryExpression;
32+
import org.sonar.plugins.python.api.tree.Expression;
3233
import org.sonar.plugins.python.api.tree.Token;
3334
import org.sonar.plugins.python.api.tree.Tree;
3435
import org.sonar.plugins.python.api.tree.TypeAnnotation;
3536
import org.sonar.plugins.python.api.tree.UnaryExpression;
3637
import org.sonar.plugins.python.api.types.InferredType;
38+
import org.sonar.plugins.python.api.types.v2.UnknownType;
39+
import org.sonar.plugins.python.api.types.v2.matchers.TypeMatcher;
40+
import org.sonar.python.types.v2.matchers.InternalTypeMatchers;
3741

3842
import static org.sonar.plugins.python.api.symbols.Symbol.Kind.AMBIGUOUS;
3943
import static org.sonar.plugins.python.api.symbols.Symbol.Kind.FUNCTION;
4044
import static org.sonar.python.types.InferredTypes.typeSymbols;
4145

4246
public abstract class IncompatibleOperands extends PythonSubscriptionCheck {
4347

48+
private static final TypeMatcher KNOWN_DECORATOR_EFFECTS = InternalTypeMatchers.hasKnownClassDecoratorEffectsInHierarchy();
49+
4450
private static final Map<String, String> UNARY_SPECIAL_METHODS_BY_OPERATOR = new HashMap<>();
4551
static {
4652
UNARY_SPECIAL_METHODS_BY_OPERATOR.put("+", "__pos__");
@@ -105,25 +111,36 @@ public void visitBinaryExpression(BinaryExpression binaryExpression) {
105111
// SONARPY-1666 We should only exclude types that represent ctypes
106112
return;
107113
}
108-
checkOperands(binaryExpression.operator(), leftType, rightType);
114+
if (canCheckBinaryOperand(binaryExpression.leftOperand(), leftType)
115+
&& canCheckBinaryOperand(binaryExpression.rightOperand(), rightType)) {
116+
checkOperands(binaryExpression.operator(), leftType, rightType);
117+
}
109118
super.visitBinaryExpression(binaryExpression);
110119
}
111120

121+
private boolean canCheckBinaryOperand(Expression operand, InferredType type) {
122+
if (operand.typeV2() instanceof UnknownType) {
123+
// Preserve the V1 check, including its decorator guard, when V2 has no type information.
124+
return typeSymbols(type).stream().noneMatch(ClassSymbol::hasDecorators);
125+
}
126+
return KNOWN_DECORATOR_EFFECTS.isTrueFor(operand, context);
127+
}
128+
112129
@Override
113130
public void visitUnaryExpression(UnaryExpression unaryExpression) {
114131
InferredType type = unaryExpression.expression().type();
115132
Token operator = unaryExpression.operator();
116133
String memberName = UNARY_SPECIAL_METHODS_BY_OPERATOR.get(operator.value());
117-
if (memberName != null && resolveMethod(type, memberName).isAbsent()) {
134+
if (memberName != null
135+
&& (unaryExpression.expression().typeV2() instanceof UnknownType
136+
|| KNOWN_DECORATOR_EFFECTS.isTrueFor(unaryExpression.expression(), context))
137+
&& resolveMethod(type, memberName).isAbsent()) {
118138
context.addIssue(operator, message(operator));
119139
}
120140
super.visitUnaryExpression(unaryExpression);
121141
}
122142

123143
private void checkOperands(Token operator, InferredType left, InferredType right) {
124-
if (typeSymbols(left).stream().anyMatch(ClassSymbol::hasDecorators) || typeSymbols(right).stream().anyMatch(ClassSymbol::hasDecorators)) {
125-
return;
126-
}
127144
SpecialMethodNames specialMethodNames = BINARY_SPECIAL_METHODS_BY_OPERATOR.get(operator.value());
128145
if (specialMethodNames == null) {
129146
return;

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

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -51,18 +51,18 @@ private void checkSubscription(SubscriptionContext ctx) {
5151
Map<LocationInFile, String> secondaries = new HashMap<>();
5252
Expression subscriptionObject = subscriptionExpression.object();
5353
if (isWithinDelStatement(subscriptionExpression)) {
54-
if (!isValidSubscription(subscriptionObject, "__delitem__", null, secondaries)) {
54+
if (!isValidSubscription(subscriptionObject, "__delitem__", null, secondaries, ctx)) {
5555
reportIssue(subscriptionExpression, subscriptionObject, "__delitem__", ctx, secondaries);
5656
}
5757
return;
5858
}
5959
if (isWithinAssignment(subscriptionExpression)) {
60-
if (!isValidSubscription(subscriptionObject, "__setitem__", null, secondaries)) {
60+
if (!isValidSubscription(subscriptionObject, "__setitem__", null, secondaries, ctx)) {
6161
reportIssue(subscriptionExpression, subscriptionObject, "__setitem__", ctx, secondaries);
6262
}
6363
return;
6464
}
65-
if (!isValidSubscription(subscriptionObject, "__getitem__", "__class_getitem__", secondaries)) {
65+
if (!isValidSubscription(subscriptionObject, "__getitem__", "__class_getitem__", secondaries, ctx)) {
6666
reportIssue(subscriptionExpression, subscriptionObject, "__getitem__", ctx, secondaries);
6767
}
6868
}
@@ -97,7 +97,8 @@ private void reportIssue(SubscriptionExpression subscriptionExpression, Expressi
9797
secondaries.keySet().stream().filter(Objects::nonNull).forEach(location -> preciseIssue.secondary(location, secondaries.get(location)));
9898
}
9999

100-
public abstract boolean isValidSubscription(Expression subscriptionObject, String requiredMethod, @Nullable String classRequiredMethod, Map<LocationInFile, String> secondaries);
100+
public abstract boolean isValidSubscription(Expression subscriptionObject, String requiredMethod, @Nullable String classRequiredMethod,
101+
Map<LocationInFile, String> secondaries, SubscriptionContext context);
101102

102103
public abstract String message(@Nullable String name, String missingMethod);
103104
}

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

Lines changed: 17 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
import javax.annotation.Nullable;
2424
import org.sonar.check.Rule;
2525
import org.sonar.plugins.python.api.LocationInFile;
26+
import org.sonar.plugins.python.api.SubscriptionContext;
2627
import org.sonar.plugins.python.api.symbols.ClassSymbol;
2728
import org.sonar.plugins.python.api.symbols.FunctionSymbol;
2829
import org.sonar.plugins.python.api.symbols.Symbol;
@@ -43,8 +44,11 @@
4344
import org.sonar.plugins.python.api.types.v2.FullyQualifiedNameHelper;
4445
import org.sonar.plugins.python.api.types.InferredType;
4546
import org.sonar.plugins.python.api.types.v2.ClassType;
47+
import org.sonar.plugins.python.api.types.v2.UnknownType;
48+
import org.sonar.plugins.python.api.types.v2.matchers.TypeMatcher;
4649
import org.sonar.python.tree.TreeUtils;
4750
import org.sonar.python.types.InferredTypes;
51+
import org.sonar.python.types.v2.matchers.InternalTypeMatchers;
4852

4953
import static org.sonar.plugins.python.api.symbols.Symbol.Kind.CLASS;
5054
import static org.sonar.plugins.python.api.symbols.Symbol.Kind.FUNCTION;
@@ -53,12 +57,14 @@
5357
@Rule(key = "S5644")
5458
public class ItemOperationsTypeCheck extends ItemOperationsType {
5559

60+
private static final TypeMatcher KNOWN_DECORATOR_EFFECTS = InternalTypeMatchers.hasKnownClassDecoratorEffectsInHierarchy();
61+
5662
private static final Set<String> TYPING_LITERAL = Set.of("typing.Literal", "typing_extensions.Literal");
5763
private static final Set<String> TYPING_ANNOTATED = Set.of("typing.Annotated", "typing_extensions.Annotated");
5864

5965
@Override
6066
public boolean isValidSubscription(Expression subscriptionObject, String requiredMethod, @Nullable String classRequiredMethod,
61-
Map<LocationInFile, String> secondaries) {
67+
Map<LocationInFile, String> secondaries, SubscriptionContext context) {
6268

6369
if (subscriptionObject.is(Tree.Kind.GENERATOR_EXPR)) {
6470
return false;
@@ -75,10 +81,16 @@ public boolean isValidSubscription(Expression subscriptionObject, String require
7581
return true;
7682
}
7783
if (symbol.is(FUNCTION, CLASS)) {
78-
return isValidSubscriptionSymbol(symbol, subscriptionObject, secondaries, requiredMethod, classRequiredMethod);
84+
return isValidSubscriptionSymbol(symbol, subscriptionObject, secondaries, requiredMethod, classRequiredMethod, context);
7985
}
8086
}
8187

88+
// Preserve the existing V1 check when V2 has no type information at all.
89+
if (!(subscriptionObject.typeV2() instanceof UnknownType)
90+
&& !KNOWN_DECORATOR_EFFECTS.isTrueFor(subscriptionObject, context)) {
91+
return true;
92+
}
93+
8294
InferredType type = subscriptionObject.type();
8395
String typeName = InferredTypes.typeName(type);
8496
String secondaryMessage = typeName != null ? String.format(SECONDARY_MESSAGE, typeName) : DEFAULT_SECONDARY_MESSAGE;
@@ -87,10 +99,11 @@ public boolean isValidSubscription(Expression subscriptionObject, String require
8799
}
88100

89101
private static boolean isValidSubscriptionSymbol(Symbol symbol, Expression subscriptionObject, Map<LocationInFile, String> secondaries, String requiredMethod,
90-
@Nullable String classRequiredMethod) {
102+
@Nullable String classRequiredMethod, SubscriptionContext context) {
91103
LocationInFile locationInFile = symbol.is(FUNCTION) ? ((FunctionSymbol) symbol).definitionLocation() : ((ClassSymbol) symbol).definitionLocation();
92104
secondaries.put(locationInFile, SECONDARY_MESSAGE.formatted(symbol.name()));
93105
return isSubscriptionInClassArg(subscriptionObject)
106+
|| (symbol.is(CLASS) && !KNOWN_DECORATOR_EFFECTS.isTrueFor(subscriptionObject, context))
94107
|| canHaveMethod(symbol, requiredMethod, classRequiredMethod)
95108
|| isValidGenericUsage(symbol, subscriptionObject, requiredMethod);
96109
}
@@ -207,7 +220,6 @@ private static boolean canHaveMethod(Symbol symbol, String requiredMethod, @Null
207220
}
208221
ClassSymbol classSymbol = (ClassSymbol) symbol;
209222
return classSymbol.canHaveMember(requiredMethod)
210-
|| (classRequiredMethod != null && classSymbol.canHaveMember(classRequiredMethod))
211-
|| classSymbol.hasDecorators();
223+
|| (classRequiredMethod != null && classSymbol.canHaveMember(classRequiredMethod));
212224
}
213225
}

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

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
*/
1717
package org.sonar.python.checks;
1818

19+
import java.util.List;
1920
import org.junit.jupiter.api.Test;
2021
import org.sonar.python.checks.utils.PythonCheckVerifier;
2122

@@ -28,7 +29,22 @@ void test_comparison() {
2829

2930
@Test
3031
void test_arithmetic() {
31-
PythonCheckVerifier.verify("src/test/resources/checks/incompatibleOperands/arithmetic.py", new IncompatibleOperandsCheck());
32+
PythonCheckVerifier.verify(List.of(
33+
"src/test/resources/checks/incompatibleOperands/arithmetic.py",
34+
"src/test/resources/checks/classDecorators/importedDecorators.py"), new IncompatibleOperandsCheck());
3235
}
3336

37+
@Test
38+
void instance_decorator_effects_on_unary_operations() {
39+
PythonCheckVerifier.verify(List.of(
40+
"src/test/resources/checks/classDecorators/instanceUnary.py",
41+
"src/test/resources/checks/classDecorators/importedDecorators.py"), new IncompatibleOperandsCheck());
42+
}
43+
44+
@Test
45+
void project_function_marker_remains_conservative() {
46+
PythonCheckVerifier.verifyNoIssue(List.of(
47+
"src/test/resources/checks/classDecoratorFunctionImpostor/usage.py",
48+
"src/test/resources/checks/classDecoratorFunctionImpostor/typing.py"), new IncompatibleOperandsCheck());
49+
}
3450
}

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

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,12 +31,26 @@ void test_delitem() {
3131
void test_getitem() {
3232
PythonCheckVerifier.verify(List.of(
3333
"src/test/resources/checks/itemOperationsTypeCheck/itemOperations_getitem.py",
34-
"src/test/resources/checks/itemOperationsTypeCheck/importedGeneric.py"), new ItemOperationsTypeCheck());
34+
"src/test/resources/checks/itemOperationsTypeCheck/importedGeneric.py",
35+
"src/test/resources/checks/classDecorators/importedDecorators.py"), new ItemOperationsTypeCheck());
3536
}
3637

3738
@Test
3839
void test_setitem() {
3940
PythonCheckVerifier.verify("src/test/resources/checks/itemOperationsTypeCheck/itemOperations_setitem.py", new ItemOperationsTypeCheck());
4041
}
4142

43+
@Test
44+
void instance_decorator_effects_on_item_operations() {
45+
PythonCheckVerifier.verify(List.of(
46+
"src/test/resources/checks/classDecorators/instanceItems.py",
47+
"src/test/resources/checks/classDecorators/importedDecorators.py"), new ItemOperationsTypeCheck());
48+
}
49+
50+
@Test
51+
void project_function_marker_remains_conservative() {
52+
PythonCheckVerifier.verifyNoIssue(List.of(
53+
"src/test/resources/checks/classDecoratorFunctionImpostor/usage.py",
54+
"src/test/resources/checks/classDecoratorFunctionImpostor/typing.py"), new ItemOperationsTypeCheck());
55+
}
4256
}
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
# Project-local module shadowing typing: this function changes the class at runtime.
2+
def final(cls):
3+
cls.__add__ = lambda self, other: 42
4+
cls.__class_getitem__ = classmethod(lambda cls, key: 42)
5+
return cls
Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
from typing import final
2+
3+
4+
@final
5+
class C:
6+
pass
7+
8+
9+
# The project decorator installs both methods; its runtime effects remain unknown.
10+
assert C[0] == 42
11+
assert C() + C() == 42
Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
def add_protocol(cls):
2+
cls.__add__ = lambda self, other: 42
3+
cls.__class_getitem__ = classmethod(lambda cls, index: 42)
4+
cls.__getitem__ = lambda self, index: 42
5+
cls.__setitem__ = lambda self, index, value: None
6+
cls.__delitem__ = lambda self, index: None
7+
cls.__neg__ = lambda self: 42
8+
cls.__pos__ = lambda self: 42
9+
cls.__invert__ = lambda self: 42
10+
return cls
11+
12+
13+
@add_protocol
14+
class ImportedBase:
15+
pass
Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,66 @@
1+
from typing import final
2+
from importedDecorators import add_protocol, ImportedBase
3+
4+
5+
@add_protocol
6+
class Direct:
7+
pass
8+
9+
10+
class Child(ImportedBase):
11+
pass
12+
13+
14+
class Plain:
15+
pass
16+
17+
18+
@final
19+
class Final:
20+
pass
21+
22+
23+
@unknown_factory()
24+
class Unknown:
25+
pass
26+
27+
28+
def item_operations(condition):
29+
direct = Direct()
30+
direct[0]
31+
direct[0] = 42
32+
del direct[0]
33+
34+
child = Child()
35+
child[0]
36+
child[0] = 42
37+
del child[0]
38+
39+
plain = Plain()
40+
plain[0] # Noncompliant
41+
plain[0] = 42 # Noncompliant
42+
del plain[0] # Noncompliant
43+
44+
marked = Final()
45+
marked[0] # Noncompliant
46+
marked[0] = 42 # Noncompliant
47+
del marked[0] # Noncompliant
48+
49+
unknown = Unknown()
50+
unknown[0]
51+
unknown[0] = 42
52+
del unknown[0]
53+
54+
mixed = Final() if condition else Child()
55+
mixed[0]
56+
mixed[0] = 42
57+
del mixed[0]
58+
59+
# An unknown outer subscription must not hide an invalid inner operation.
60+
direct[42[0]] # Noncompliant
61+
62+
63+
# Constructor expressions take the same path as instance variables.
64+
Direct()[0]
65+
Child()[0]
66+
Final()[0] # Noncompliant

0 commit comments

Comments
 (0)