Skip to content

Commit 5390bf6

Browse files
authored
SONARPY-2370 Fix FP on S5644 for generic types defined through type parameter syntax (#2209)
1 parent f251712 commit 5390bf6

File tree

2 files changed

+51
-15
lines changed

2 files changed

+51
-15
lines changed

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

Lines changed: 46 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -16,18 +16,24 @@
1616
*/
1717
package org.sonar.python.checks;
1818

19+
import java.util.Collection;
1920
import java.util.Map;
21+
import java.util.Optional;
2022
import javax.annotation.Nullable;
2123
import org.sonar.check.Rule;
2224
import org.sonar.plugins.python.api.LocationInFile;
2325
import org.sonar.plugins.python.api.symbols.ClassSymbol;
2426
import org.sonar.plugins.python.api.symbols.FunctionSymbol;
2527
import org.sonar.plugins.python.api.symbols.Symbol;
28+
import org.sonar.plugins.python.api.tree.ArgList;
2629
import org.sonar.plugins.python.api.tree.CallExpression;
30+
import org.sonar.plugins.python.api.tree.ClassDef;
2731
import org.sonar.plugins.python.api.tree.Expression;
28-
import org.sonar.plugins.python.api.tree.HasSymbol;
32+
import org.sonar.plugins.python.api.tree.RegularArgument;
33+
import org.sonar.plugins.python.api.tree.SubscriptionExpression;
2934
import org.sonar.plugins.python.api.tree.Tree;
3035
import org.sonar.plugins.python.api.types.InferredType;
36+
import org.sonar.python.tree.TreeUtils;
3137
import org.sonar.python.types.InferredTypes;
3238

3339
import static org.sonar.plugins.python.api.symbols.Symbol.Kind.CLASS;
@@ -39,37 +45,62 @@ public class ItemOperationsTypeCheck extends ItemOperationsType {
3945

4046
@Override
4147
public boolean isValidSubscription(Expression subscriptionObject, String requiredMethod, @Nullable String classRequiredMethod,
42-
Map<LocationInFile, String> secondaries) {
48+
Map<LocationInFile, String> secondaries) {
4349

4450
if (subscriptionObject.is(Tree.Kind.GENERATOR_EXPR)) {
4551
return false;
4652
}
47-
if (subscriptionObject.is(Tree.Kind.CALL_EXPR)) {
48-
Symbol subscriptionCalleeSymbol = ((CallExpression) subscriptionObject).calleeSymbol();
49-
if (subscriptionCalleeSymbol != null && subscriptionCalleeSymbol.is(FUNCTION) && ((FunctionSymbol) subscriptionCalleeSymbol).isAsynchronous()) {
50-
FunctionSymbol functionSymbol = (FunctionSymbol) subscriptionCalleeSymbol;
51-
secondaries.put(functionSymbol.definitionLocation(), String.format(SECONDARY_MESSAGE, functionSymbol.name()));
52-
return false;
53-
}
53+
if (isInvalidSubscriptionCallExpr(subscriptionObject, secondaries)) {
54+
return false;
5455
}
55-
if (subscriptionObject instanceof HasSymbol hasSymbol) {
56-
Symbol symbol = hasSymbol.symbol();
57-
if (symbol == null || isTypingOrCollectionsSymbol(symbol)) {
56+
57+
var symbolOptional = TreeUtils.getSymbolFromTree(subscriptionObject);
58+
59+
if (symbolOptional.isPresent()) {
60+
var symbol = symbolOptional.get();
61+
if (isTypingOrCollectionsSymbol(symbol)) {
5862
return true;
5963
}
6064
if (symbol.is(FUNCTION, CLASS)) {
61-
secondaries.put(symbol.is(FUNCTION) ?
62-
((FunctionSymbol) symbol).definitionLocation() : ((ClassSymbol) symbol).definitionLocation(), String.format(SECONDARY_MESSAGE, symbol.name()));
63-
return canHaveMethod(symbol, requiredMethod, classRequiredMethod);
65+
return isValidSubscriptionSymbol(symbol, subscriptionObject, secondaries, requiredMethod, classRequiredMethod);
6466
}
6567
}
68+
6669
InferredType type = subscriptionObject.type();
6770
String typeName = InferredTypes.typeName(type);
6871
String secondaryMessage = typeName != null ? String.format(SECONDARY_MESSAGE, typeName) : DEFAULT_SECONDARY_MESSAGE;
6972
secondaries.put(typeClassLocation(type), secondaryMessage);
7073
return type.canHaveMember(requiredMethod);
7174
}
7275

76+
private static boolean isValidSubscriptionSymbol(Symbol symbol, Expression subscriptionObject, Map<LocationInFile, String> secondaries, String requiredMethod,
77+
@Nullable String classRequiredMethod) {
78+
LocationInFile locationInFile = symbol.is(FUNCTION) ? ((FunctionSymbol) symbol).definitionLocation() : ((ClassSymbol) symbol).definitionLocation();
79+
secondaries.put(locationInFile, SECONDARY_MESSAGE.formatted(symbol.name()));
80+
return isSubscriptionInClassArg(subscriptionObject) || canHaveMethod(symbol, requiredMethod, classRequiredMethod);
81+
}
82+
83+
private static boolean isInvalidSubscriptionCallExpr(Expression expression, Map<LocationInFile, String> secondaries) {
84+
if (expression instanceof CallExpression callExpression
85+
&& callExpression.calleeSymbol() instanceof FunctionSymbol functionSymbol
86+
&& functionSymbol.isAsynchronous()) {
87+
secondaries.put(functionSymbol.definitionLocation(), SECONDARY_MESSAGE.formatted(functionSymbol.name()));
88+
return true;
89+
}
90+
return false;
91+
}
92+
93+
private static boolean isSubscriptionInClassArg(Expression subscriptionObject) {
94+
return Optional.ofNullable(((ClassDef) TreeUtils.firstAncestorOfKind(subscriptionObject, Tree.Kind.CLASSDEF))).map(ClassDef::args).map(ArgList::arguments)
95+
.stream()
96+
.flatMap(Collection::stream)
97+
.flatMap(TreeUtils.toStreamInstanceOfMapper(RegularArgument.class))
98+
.map(RegularArgument::expression)
99+
.flatMap(TreeUtils.toStreamInstanceOfMapper(SubscriptionExpression.class))
100+
.map(SubscriptionExpression::object)
101+
.anyMatch(subscriptionObject::equals);
102+
}
103+
73104
@Override
74105
public String message(@Nullable String name, String missingMethod) {
75106
if (name != null) {

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

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -216,3 +216,8 @@ class ExtendedMock(Mock):
216216

217217
def custom_mock():
218218
a = ExtendedMock()[42]
219+
220+
221+
class MyGenericClass[T]: ...
222+
223+
class MyGenericSubType(MyGenericClass[str]): ...

0 commit comments

Comments
 (0)