Skip to content

Commit 30c1ce4

Browse files
maksim-grebeniuk-sonarsourceguillaume-dequenne
maksim-grebeniuk-sonarsource
authored and
guillaume-dequenne
committed
SONARPY-1936 Migrate rule S5864 NonCallableCalled to typeV2 (#1867)
1 parent 899af47 commit 30c1ce4

File tree

10 files changed

+399
-72
lines changed

10 files changed

+399
-72
lines changed

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

Lines changed: 16 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,8 @@
3939
import org.sonar.python.tree.TreeUtils;
4040
import org.sonar.python.types.InferredTypes;
4141
import org.sonar.python.types.TypeShed;
42+
import org.sonar.python.types.v2.PythonType;
43+
import org.sonar.python.types.v2.TriBool;
4244

4345
import static org.sonar.python.tree.TreeUtils.nameFromExpression;
4446
import static org.sonar.python.types.InferredTypes.containsDeclaredType;
@@ -59,22 +61,6 @@ public void initialize(Context context) {
5961
context.registerSyntaxNodeConsumer(Tree.Kind.IS, ConfusingTypeCheckingCheck::checkSillyIdentity);
6062
}
6163

62-
private static class NonCallableCalledCheck extends NonCallableCalled {
63-
64-
@Override
65-
public boolean isNonCallableType(InferredType type) {
66-
return containsDeclaredType(type) && !type.declaresMember("__call__") && !type.mustBeOrExtend("typing.Coroutine");
67-
}
68-
69-
@Override
70-
public String message(InferredType calleeType, @Nullable String name) {
71-
if (name != null) {
72-
return String.format("Fix this call; Previous type checks suggest that \"%s\"%s is not callable.", name, addTypeName(calleeType));
73-
}
74-
return String.format("Fix this call; Previous type checks suggest that this expression%s is not callable.", addTypeName(calleeType));
75-
}
76-
}
77-
7864
private static class IncompatibleOperandsCheck extends IncompatibleOperands {
7965
@Override
8066
public SpecialMethod resolveMethod(InferredType type, String method) {
@@ -218,6 +204,20 @@ String message(String result) {
218204
}
219205
}
220206

207+
private static class NonCallableCalledCheck extends NonCallableCalled {
208+
209+
@Override
210+
protected boolean isExpectedTypeSource(SubscriptionContext ctx, PythonType calleeType) {
211+
return ctx.typeChecker().typeCheckBuilder().isTypeHintTypeSource().check(calleeType) == TriBool.TRUE;
212+
}
213+
214+
@Override
215+
protected boolean isException(SubscriptionContext ctx, PythonType calleeType) {
216+
var isCoroutine = ctx.typeChecker().typeCheckBuilder().isInstanceOf("typing.Coroutine").check(calleeType) == TriBool.TRUE;
217+
return super.isException(ctx, calleeType) || isCoroutine;
218+
}
219+
}
220+
221221
private static boolean areIdentityComparableOrNone(InferredType leftType, InferredType rightType) {
222222
if (!containsDeclaredType(leftType) && !containsDeclaredType(rightType)) {
223223
return true;

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

Lines changed: 29 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -20,45 +20,52 @@
2020
package org.sonar.python.checks;
2121

2222
import javax.annotation.Nullable;
23-
import org.sonar.plugins.python.api.LocationInFile;
2423
import org.sonar.plugins.python.api.PythonSubscriptionCheck;
24+
import org.sonar.plugins.python.api.SubscriptionContext;
2525
import org.sonar.plugins.python.api.tree.CallExpression;
26-
import org.sonar.plugins.python.api.tree.Expression;
2726
import org.sonar.plugins.python.api.tree.Tree;
28-
import org.sonar.plugins.python.api.types.InferredType;
29-
import org.sonar.python.types.InferredTypes;
27+
import org.sonar.python.types.v2.PythonType;
28+
import org.sonar.python.types.v2.TriBool;
3029

3130
import static org.sonar.python.tree.TreeUtils.nameFromExpression;
32-
import static org.sonar.python.types.InferredTypes.typeClassLocation;
3331

3432
public abstract class NonCallableCalled extends PythonSubscriptionCheck {
3533
@Override
3634
public void initialize(Context context) {
3735
context.registerSyntaxNodeConsumer(Tree.Kind.CALL_EXPR, ctx -> {
38-
CallExpression callExpression = (CallExpression) ctx.syntaxNode();
39-
Expression callee = callExpression.callee();
40-
InferredType calleeType = callee.type();
41-
if (isNonCallableType(calleeType)) {
36+
var callExpression = (CallExpression) ctx.syntaxNode();
37+
var callee = callExpression.callee();
38+
var calleeType = callee.typeV2();
39+
if (!isException(ctx, calleeType) && isCallMemberMissing(ctx, calleeType)) {
4240
String name = nameFromExpression(callee);
43-
PreciseIssue preciseIssue = ctx.addIssue(callee, message(calleeType, name));
44-
LocationInFile location = typeClassLocation(calleeType);
45-
if (location != null) {
46-
preciseIssue.secondary(location, "Definition.");
47-
}
41+
var preciseIssue = ctx.addIssue(callee, message(calleeType, name));
42+
calleeType.definitionLocation()
43+
.ifPresent(location -> preciseIssue.secondary(location, "Definition."));
4844
}
4945
});
5046
}
5147

52-
protected static String addTypeName(InferredType type) {
53-
String typeName = InferredTypes.typeName(type);
54-
if (typeName != null) {
55-
return " has type " + typeName + " and it";
56-
}
57-
return "";
48+
protected boolean isCallMemberMissing(SubscriptionContext ctx, PythonType calleeType) {
49+
return ctx.typeChecker().typeCheckBuilder().hasMember("__call__").check(calleeType) == TriBool.FALSE;
50+
}
51+
52+
protected boolean isException(SubscriptionContext ctx, PythonType calleeType) {
53+
return !isExpectedTypeSource(ctx, calleeType);
5854
}
5955

60-
public abstract boolean isNonCallableType(InferredType type);
56+
protected abstract boolean isExpectedTypeSource(SubscriptionContext ctx, PythonType calleeType);
6157

62-
public abstract String message(InferredType calleeType, @Nullable String name);
58+
protected static String addTypeName(PythonType type) {
59+
return type.displayName()
60+
.map(d -> " has type " + d + " and it")
61+
.orElse("");
62+
}
63+
64+
protected String message(PythonType calleeType, @Nullable String name) {
65+
if (name != null) {
66+
return String.format("Fix this call; Previous type checks suggest that \"%s\"%s is not callable.", name, addTypeName(calleeType));
67+
}
68+
return String.format("Fix this call; Previous type checks suggest that this expression%s is not callable.", addTypeName(calleeType));
69+
}
6370

6471
}

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

Lines changed: 6 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -21,45 +21,20 @@
2121

2222
import javax.annotation.Nullable;
2323
import org.sonar.check.Rule;
24-
import org.sonar.plugins.python.api.PythonSubscriptionCheck;
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.Tree;
28-
import org.sonar.python.types.v2.TypeChecker;
24+
import org.sonar.plugins.python.api.SubscriptionContext;
2925
import org.sonar.python.types.v2.PythonType;
3026
import org.sonar.python.types.v2.TriBool;
3127

32-
import static org.sonar.python.tree.TreeUtils.nameFromExpression;
33-
3428
@Rule(key = "S5756")
35-
public class NonCallableCalledCheck extends PythonSubscriptionCheck {
29+
public class NonCallableCalledCheck extends NonCallableCalled {
3630

3731
@Override
38-
public void initialize(Context context) {
39-
context.registerSyntaxNodeConsumer(Tree.Kind.CALL_EXPR, ctx -> {
40-
CallExpression callExpression = (CallExpression) ctx.syntaxNode();
41-
Expression callee = callExpression.callee();
42-
PythonType type = callee.typeV2();
43-
if (isNonCallableType(type, ctx.typeChecker())) {
44-
String name = nameFromExpression(callee);
45-
PreciseIssue preciseIssue = ctx.addIssue(callee, message(type, name));
46-
type.definitionLocation()
47-
.ifPresent(location -> preciseIssue.secondary(location, "Definition."));
48-
}
49-
});
50-
}
51-
52-
protected static String addTypeName(PythonType type) {
53-
return type.displayName()
54-
.map(d -> " has type " + d + " and it")
55-
.orElse("");
32+
protected boolean isExpectedTypeSource(SubscriptionContext ctx, PythonType calleeType) {
33+
return ctx.typeChecker().typeCheckBuilder().isExactTypeSource().check(calleeType) == TriBool.TRUE;
5634
}
5735

58-
public boolean isNonCallableType(PythonType type, TypeChecker typeChecker) {
59-
return typeChecker.typeCheckBuilder().hasMember("__call__").check(type) == TriBool.FALSE;
60-
}
61-
62-
public String message(PythonType typeV2, @Nullable String name) {
36+
@Override
37+
protected String message(PythonType typeV2, @Nullable String name) {
6338
if (name != null) {
6439
return "Fix this call; \"%s\"%s is not callable.".formatted(name, addTypeName(typeV2));
6540
}

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

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,4 +35,10 @@ void test_multiple_files() {
3535
PythonCheckVerifier.verify(List.of("src/test/resources/checks/nonCallableCalledImporter.py", "src/test/resources/checks/nonCallableCalledImported.py"), new NonCallableCalledCheck());
3636
}
3737

38+
@Test
39+
void testNoIssues() {
40+
PythonCheckVerifier.verifyNoIssue("src/test/resources/checks/nonCallableCalledNoIssues.py", new NonCallableCalledCheck());
41+
}
42+
43+
3844
}

‎python-checks/src/test/resources/checks/confusingTypeChecking/nonCallableCalled.py

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
def empty_union(x: Union['A', 'B']):
55
x()
66

7-
def foo(param1: int, param2: Set[int], param3: FrozenSet[int]):
7+
def foo(param1: int, param2: Set[int], param3: FrozenSet[int], param4: list[str], param5: set[int]):
88
param1() # Noncompliant {{Fix this call; Previous type checks suggest that "param1" has type int and it is not callable.}}
99
x = 42
1010
x() # OK, raised by S5756
@@ -17,8 +17,13 @@ def foo(param1: int, param2: Set[int], param3: FrozenSet[int]):
1717
fs = frozenset()
1818
fs() # OK, raised by S5756
1919

20+
for item in param4:
21+
item() # Noncompliant
22+
23+
param5() # Noncompliant
24+
2025
def derived(x: int):
21-
x.conjugate()() # NonCompliant
26+
x.conjugate()() # Noncompliant
2227
y = x or 'str'
2328
y() # Noncompliant
2429
z = x + 42
@@ -38,7 +43,6 @@ async def bar(func: Coroutine):
3843
func()
3944
)
4045

41-
4246
def decorators(decorator: Callable, non_callable: str):
4347

4448
@non_callable() # Noncompliant

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

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
2+
def param_with_type_hint(a: int):
3+
a() # OK

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

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
package org.sonar.python.semantic.v2;
2121

2222
import java.util.ArrayList;
23+
import java.util.Arrays;
2324
import java.util.HashSet;
2425
import java.util.List;
2526
import java.util.Set;
@@ -65,6 +66,11 @@ public List<PythonType> superClasses() {
6566
return superClasses;
6667
}
6768

69+
public ClassTypeBuilder withSuperClasses(PythonType... types) {
70+
superClasses.addAll(Arrays.asList(types));
71+
return this;
72+
}
73+
6874
public List<PythonType> metaClasses() {
6975
return metaClasses;
7076
}

‎python-frontend/src/main/java/org/sonar/python/types/v2/TypeCheckBuilder.java

Lines changed: 103 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,8 +19,11 @@
1919
*/
2020
package org.sonar.python.types.v2;
2121

22+
import java.util.ArrayDeque;
2223
import java.util.ArrayList;
24+
import java.util.HashSet;
2325
import java.util.List;
26+
import java.util.Set;
2427
import org.sonar.python.semantic.v2.ProjectLevelTypeTable;
2528

2629
public class TypeCheckBuilder {
@@ -47,6 +50,11 @@ public TypeCheckBuilder isTypeHintTypeSource() {
4750
return this;
4851
}
4952

53+
public TypeCheckBuilder isExactTypeSource() {
54+
predicates.add(new TypeSourceMatcherTypePredicate(TypeSource.EXACT));
55+
return this;
56+
}
57+
5058
public TypeCheckBuilder isBuiltinWithName(String name) {
5159
PythonType builtinType = projectLevelTypeTable.getBuiltinsModule().resolveMember(name).orElse(PythonType.UNKNOWN);
5260
predicates.add(new IsSameAsTypePredicate(builtinType));
@@ -65,6 +73,12 @@ public TriBool check(PythonType pythonType) {
6573
return result;
6674
}
6775

76+
public TypeCheckBuilder isInstanceOf(String fqn) {
77+
var expected = projectLevelTypeTable.getType(fqn);
78+
predicates.add(new IsInstanceOfPredicate(expected));
79+
return this;
80+
}
81+
6882
interface TypePredicate {
6983
TriBool test(PythonType pythonType);
7084
}
@@ -125,4 +139,93 @@ public TriBool test(PythonType pythonType) {
125139
return pythonType.equals(expectedType) ? TriBool.TRUE : TriBool.FALSE;
126140
}
127141
}
142+
143+
record IsInstanceOfPredicate(PythonType expectedType) implements TypePredicate {
144+
145+
@Override
146+
public TriBool test(PythonType pythonType) {
147+
if (expectedType instanceof ClassType expectedClassType) {
148+
if (pythonType instanceof ObjectType objectType) {
149+
if (objectType.type() instanceof ClassType classType) {
150+
// when the checking type is an ObjectType of a ClassType
151+
return isClassInheritedFrom(classType, expectedClassType);
152+
} else if (objectType.type() instanceof UnionType unionType) {
153+
// when the checking type is an ObjectType of a UnionType
154+
return isObjectOfUnionTypeInstanceOf(expectedClassType, unionType);
155+
}
156+
} else if (pythonType instanceof UnionType unionType) {
157+
// when the checking type is a UnionType
158+
return isUnionTypeInstanceOf(unionType);
159+
}
160+
}
161+
return TriBool.UNKNOWN;
162+
}
163+
164+
private static TriBool isObjectOfUnionTypeInstanceOf(ClassType expectedClassType, UnionType unionType) {
165+
var results = unionType.candidates()
166+
.stream()
167+
.map(classType -> isClassInheritedFrom(classType, expectedClassType))
168+
.distinct()
169+
.toList();
170+
171+
if (results.size() > 1) {
172+
return TriBool.UNKNOWN;
173+
} else {
174+
return results.get(0);
175+
}
176+
}
177+
178+
private TriBool isUnionTypeInstanceOf(UnionType unionType) {
179+
var candidatesResults = unionType.candidates()
180+
.stream()
181+
.map(this::test)
182+
.distinct()
183+
.toList();
184+
185+
if (candidatesResults.size() != 1) {
186+
return TriBool.UNKNOWN;
187+
} else {
188+
return candidatesResults.get(0);
189+
}
190+
}
191+
192+
private static TriBool isClassInheritedFrom(PythonType classType, ClassType expectedClassType) {
193+
if (classType == expectedClassType) {
194+
return TriBool.TRUE;
195+
}
196+
197+
var types = collectTypes(classType);
198+
199+
if (types.contains(expectedClassType)) {
200+
return TriBool.TRUE;
201+
} else if (types.contains(PythonType.UNKNOWN)) {
202+
return TriBool.UNKNOWN;
203+
} else {
204+
return TriBool.FALSE;
205+
}
206+
}
207+
208+
private static Set<PythonType> collectTypes(PythonType type) {
209+
var result = new HashSet<PythonType>();
210+
var queue = new ArrayDeque<PythonType>();
211+
queue.add(type);
212+
while (!queue.isEmpty()) {
213+
var currentType = queue.pop();
214+
if (result.contains(currentType)) {
215+
continue;
216+
}
217+
result.add(currentType);
218+
if (currentType instanceof UnionType) {
219+
result.clear();
220+
result.add(PythonType.UNKNOWN);
221+
queue.clear();
222+
} else if (currentType instanceof ClassType classType) {
223+
queue.addAll(classType.superClasses());
224+
}
225+
}
226+
return result;
227+
}
228+
}
229+
230+
128231
}

0 commit comments

Comments
 (0)