Skip to content

Commit 1a67d6e

Browse files
Seppli11sonartech
Seppli11
authored and
sonartech
committed
SONARPY-1619 S117: should not raise an issue on variable of type dango.db.models.Model. (#461)
GitOrigin-RevId: 41e302fa7fcd41f7268a82e31f60e38ddd97504e
1 parent 6f3ec9c commit 1a67d6e

19 files changed

Lines changed: 398 additions & 63 deletions

File tree

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

Lines changed: 69 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,8 @@
2121
import java.util.List;
2222
import java.util.Set;
2323
import java.util.regex.Pattern;
24+
import java.util.stream.Stream;
25+
import javax.annotation.Nullable;
2426
import org.sonar.check.Rule;
2527
import org.sonar.check.RuleProperty;
2628
import org.sonar.plugins.python.api.PythonSubscriptionCheck;
@@ -33,14 +35,20 @@
3335
import org.sonar.plugins.python.api.tree.Name;
3436
import org.sonar.plugins.python.api.tree.SubscriptionExpression;
3537
import org.sonar.plugins.python.api.tree.Tree;
38+
import org.sonar.plugins.python.api.types.v2.PythonType;
39+
import org.sonar.python.semantic.SymbolUtils;
40+
import org.sonar.python.semantic.v2.SymbolV2;
41+
import org.sonar.python.semantic.v2.UsageV2;
42+
import org.sonar.python.tree.TreeUtils;
43+
import org.sonar.python.types.v2.TypeCheckBuilder;
3644

3745
@Rule(key = "S117")
3846
public class LocalVariableAndParameterNameConventionCheck extends PythonSubscriptionCheck {
3947

4048
private static final String MESSAGE = "Rename this %s \"%s\" to match the regular expression %s.";
4149
private static final String PARAMETER = "parameter";
4250
private static final String LOCAL_VAR = "local variable";
43-
private static final EnumSet<Usage.Kind> USAGES = EnumSet.of(Usage.Kind.PARAMETER, Usage.Kind.LOOP_DECLARATION, Usage.Kind.ASSIGNMENT_LHS);
51+
private static final EnumSet<UsageV2.Kind> USAGES = EnumSet.of(UsageV2.Kind.PARAMETER, UsageV2.Kind.LOOP_DECLARATION, UsageV2.Kind.ASSIGNMENT_LHS);
4452

4553
private static final String CONSTANT_PATTERN = "^[_A-Z][A-Z0-9_]*$";
4654

@@ -54,17 +62,28 @@ public class LocalVariableAndParameterNameConventionCheck extends PythonSubscrip
5462
private Pattern pattern;
5563
private static final Set<String> ML_VARIABLE_NAMES = Set.of("X_train", "X_test", "Y_train", "Y_test", "X", "Y");
5664

65+
private TypeCheckBuilder isDjangoModelTypeCheck;
66+
5767
@Override
5868
public void initialize(Context context) {
5969
pattern = Pattern.compile(format);
6070
constantPattern = Pattern.compile(CONSTANT_PATTERN);
61-
context.registerSyntaxNodeConsumer(Tree.Kind.FUNCDEF, ctx -> {
62-
FunctionDef funcDef = (FunctionDef) ctx.syntaxNode();
63-
funcDef.localVariables().stream().sorted(Comparator.comparing(Symbol::name)).forEach(s -> checkName(s, ctx));
64-
});
71+
context.registerSyntaxNodeConsumer(Tree.Kind.FILE_INPUT, this::initializeTypeChecker);
72+
context.registerSyntaxNodeConsumer(Tree.Kind.FUNCDEF, this::checkFunctionDef);
73+
}
74+
75+
private void initializeTypeChecker(SubscriptionContext ctx) {
76+
isDjangoModelTypeCheck = ctx.typeChecker().typeCheckBuilder().isTypeOrInstanceWithName("type");
6577
}
6678

67-
private void checkName(Symbol symbol, SubscriptionContext ctx) {
79+
private void checkFunctionDef(SubscriptionContext ctx) {
80+
FunctionDef funcDef = (FunctionDef) ctx.syntaxNode();
81+
TreeUtils.getLocalVariableSymbols(funcDef).stream()
82+
.sorted(Comparator.comparing(SymbolV2::name))
83+
.forEach(s -> checkName(s, ctx));
84+
}
85+
86+
private void checkName(SymbolV2 symbol, SubscriptionContext ctx) {
6887
String name = symbol.name();
6988
if (ML_VARIABLE_NAMES.contains(name)) {
7089
return;
@@ -82,54 +101,69 @@ private void checkName(Symbol symbol, SubscriptionContext ctx) {
82101
}
83102
}
84103

85-
private static boolean isType(Symbol symbol) {
86-
return isExtendingType(symbol) || isAssignedFromTyping(symbol);
104+
private boolean isType(SymbolV2 symbolV2) {
105+
// TypeV1 and TypeV2 can detect different cases and work complementary to find more issues
106+
Symbol symbolV1 = SymbolUtils.symbolV2ToSymbolV1(symbolV2).orElse(null);
107+
return symbolV1 != null && (isExtendingType(symbolV1) || isAssignedFromTyping(symbolV2) || isPythonTypeAClassType(symbolV2));
87108
}
88109

89110
private static boolean isExtendingType(Symbol symbol) {
90-
return symbol.usages().stream().map(Usage::tree).filter(Expression.class::isInstance).map(Expression.class::cast).anyMatch(e -> e.type().mustBeOrExtend("type")) ||
91-
(symbol.annotatedTypeName() != null && symbol.annotatedTypeName().startsWith("typing."));
111+
boolean isInferredTypeExtendingType = symbol.usages().stream()
112+
.map(Usage::tree)
113+
.filter(Expression.class::isInstance)
114+
.map(Expression.class::cast)
115+
.anyMatch(e -> e.type().mustBeOrExtend("type"));
116+
117+
boolean isAnnotatedTypeStartingWithTyping = symbol.annotatedTypeName() != null && symbol.annotatedTypeName().startsWith("typing.");
118+
return isInferredTypeExtendingType || isAnnotatedTypeStartingWithTyping;
92119
}
93120

94-
private static boolean isAssignedFromTyping(Symbol symbol) {
95-
List<Tree> assignmentNames = symbol.usages().stream().filter(u -> u.kind() == Usage.Kind.ASSIGNMENT_LHS).map(Usage::tree).toList();
96-
for (Tree assignmentName : assignmentNames) {
97-
Expression assignedValue = getAssignedValue(assignmentName);
98-
if (assignedValue == null) {
99-
continue;
100-
}
101-
if (assignedValue.is(Tree.Kind.SUBSCRIPTION)) {
102-
SubscriptionExpression subscriptionExpression = (SubscriptionExpression) assignedValue;
103-
if (subscriptionExpression.object().is(Tree.Kind.NAME)) {
104-
Symbol assignedSymbol = ((Name) subscriptionExpression.object()).symbol();
105-
if (assignedSymbol != null && isExtendingType(assignedSymbol)) {
106-
return true;
107-
}
108-
}
121+
private static boolean isAssignedFromTyping(SymbolV2 symbol) {
122+
List<Expression> assignedValues = symbol.usages().stream()
123+
.filter(u -> u.kind() == UsageV2.Kind.ASSIGNMENT_LHS)
124+
.flatMap(usage -> getAssignedValue(usage.tree()))
125+
.toList();
126+
127+
for (Expression assignedValue : assignedValues) {
128+
Symbol assignedSymbol = getTypingSymbol(assignedValue);
129+
if (assignedSymbol != null && isExtendingType(assignedSymbol)) {
130+
return true;
109131
}
110132
}
111133
return false;
112134
}
113135

114-
private static Expression getAssignedValue(Tree assignmentName) {
115-
while (assignmentName != null && !assignmentName.is(Tree.Kind.ASSIGNMENT_STMT)) {
116-
assignmentName = assignmentName.parent();
136+
private boolean isPythonTypeAClassType(SymbolV2 symbol) {
137+
PythonType type = SymbolUtils.getPythonType(symbol);
138+
return isDjangoModelTypeCheck.check(type).isTrue();
139+
}
140+
141+
private static Stream<Expression> getAssignedValue(Tree assignmentName) {
142+
var assignmentStmt = TreeUtils.firstAncestorOfClass(assignmentName, AssignmentStatement.class);
143+
if (assignmentStmt != null) {
144+
return Stream.of(assignmentStmt.assignedValue());
145+
} else {
146+
return Stream.empty();
117147
}
118-
if (assignmentName == null) {
119-
return null;
148+
}
149+
150+
private static @Nullable Symbol getTypingSymbol(Expression expr) {
151+
if (expr instanceof SubscriptionExpression subscriptionExpression
152+
&& subscriptionExpression.object() instanceof Name name) {
153+
return name.symbol();
120154
}
121-
return ((AssignmentStatement) assignmentName).assignedValue();
155+
return null;
122156
}
123157

124-
private void raiseIssueForNameAndUsage(SubscriptionContext ctx, String name, Usage usage) {
158+
private void raiseIssueForNameAndUsage(SubscriptionContext ctx, String name, UsageV2 usage) {
125159
String type = PARAMETER;
126-
Usage.Kind kind = usage.kind();
127-
if (kind == Usage.Kind.ASSIGNMENT_LHS) {
160+
UsageV2.Kind kind = usage.kind();
161+
if (kind == UsageV2.Kind.ASSIGNMENT_LHS) {
128162
type = LOCAL_VAR;
129163
if (constantPattern.matcher(name).matches()) {
130164
return;
131165
}
132-
} else if (kind == Usage.Kind.LOOP_DECLARATION) {
166+
} else if (kind == UsageV2.Kind.LOOP_DECLARATION) {
133167
type = LOCAL_VAR;
134168
if (name.length() <= 1) {
135169
return;

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

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -100,3 +100,16 @@ def ml_names():
100100
Y_test = Y
101101

102102
return X_train, Y_train, X_test, Y_test
103+
104+
def django_models():
105+
from django.apps import AppConfig
106+
107+
class RockNRollConfig(AppConfig):
108+
def ready(self):
109+
# FP due to self not being resolved; will be fixed by SONARPY-1865
110+
MyModel = self.get_model("MyModel") # Noncompliant
111+
112+
MySecondModel = RockNRollConfig().get_model("MySecondModel")
113+
114+
from django.apps import apps
115+
Product = apps.get_model('shop', 'Product')

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

Lines changed: 26 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,9 @@
1616
*/
1717
package org.sonar.python.semantic;
1818

19+
import static org.sonar.plugins.python.api.symbols.Symbol.Kind.CLASS;
20+
import static org.sonar.python.tree.TreeUtils.getSymbolFromTree;
21+
1922
import java.io.File;
2023
import java.net.URI;
2124
import java.nio.file.InvalidPathException;
@@ -56,12 +59,13 @@
5659
import org.sonar.plugins.python.api.tree.Tree.Kind;
5760
import org.sonar.plugins.python.api.tree.Tuple;
5861
import org.sonar.plugins.python.api.tree.UnpackingExpression;
62+
import org.sonar.plugins.python.api.types.v2.PythonType;
63+
import org.sonar.python.semantic.v2.SymbolV2;
64+
import org.sonar.python.semantic.v2.UsageV2;
5965
import org.sonar.python.tree.TreeUtils;
6066
import org.sonar.python.types.InferredTypes;
6167
import org.sonar.python.types.TypeShed;
62-
63-
import static org.sonar.plugins.python.api.symbols.Symbol.Kind.CLASS;
64-
import static org.sonar.python.tree.TreeUtils.getSymbolFromTree;
68+
import org.sonar.python.types.v2.TypeUtils;
6569

6670
public class SymbolUtils {
6771

@@ -357,4 +361,23 @@ public static String qualifiedNameOrEmpty(CallExpression callExpression) {
357361
Symbol symbol = callExpression.calleeSymbol();
358362
return symbol != null && symbol.fullyQualifiedName() != null ? symbol.fullyQualifiedName() : "";
359363
}
364+
365+
public static PythonType getPythonType(SymbolV2 symbol) {
366+
return symbol.usages().stream()
367+
.filter(UsageV2::isBindingUsage)
368+
.map(UsageV2::tree)
369+
.flatMap(TreeUtils.toStreamInstanceOfMapper(Expression.class))
370+
.map(Expression::typeV2)
371+
.collect(TypeUtils.toUnionType());
372+
}
373+
374+
public static Optional<Symbol> symbolV2ToSymbolV1(SymbolV2 symbolV2) {
375+
return symbolV2.usages().stream()
376+
.filter(UsageV2::isBindingUsage)
377+
.map(UsageV2::tree)
378+
.filter(HasSymbol.class::isInstance)
379+
.map(tree -> ((HasSymbol) tree).symbol())
380+
.filter(Objects::nonNull)
381+
.findFirst();
382+
}
360383
}

‎python-frontend/src/main/java/org/sonar/python/tree/TreeUtils.java‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@
3838
import org.sonar.plugins.python.api.symbols.ClassSymbol;
3939
import org.sonar.plugins.python.api.symbols.FunctionSymbol;
4040
import org.sonar.plugins.python.api.symbols.Symbol;
41+
import org.sonar.plugins.python.api.symbols.Usage;
4142
import org.sonar.plugins.python.api.tree.AnyParameter;
4243
import org.sonar.plugins.python.api.tree.Argument;
4344
import org.sonar.plugins.python.api.tree.BaseTreeVisitor;
@@ -612,4 +613,14 @@ private static PythonType inferSingleAssignedNameType(Name name) {
612613
.map(Name::typeV2)
613614
.orElse(PythonType.UNKNOWN);
614615
}
616+
617+
public static Set<SymbolV2> getLocalVariableSymbols(FunctionDef functionDef) {
618+
return functionDef.localVariables().stream()
619+
.flatMap(symbol -> symbol.usages().stream())
620+
.map(Usage::tree)
621+
.flatMap(TreeUtils.toStreamInstanceOfMapper(Name.class))
622+
.map(Name::symbolV2)
623+
.filter(Objects::nonNull)
624+
.collect(Collectors.toSet());
625+
}
615626
}

‎python-frontend/src/main/resources/org/sonar/python/types/custom_protobuf/django.apps.config.protobuf‎

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
2+
django.apps.config�
3+
AppConfigdjango.apps.config.AppConfig"builtins.object*�
4+
__init__%django.apps.config.AppConfig.__init__"
5+
None*F
6+
self<
7+
django.apps.config.AppConfig"django.apps.config.AppConfig*
8+
args
9+
Any*
10+
kwargs
11+
Any*�
12+
get_model&django.apps.config.AppConfig.get_model"
13+
Type[Any]
14+
Any"type*F
15+
self<
16+
django.apps.config.AppConfig"django.apps.config.AppConfig*,
17+
18+
model_name
19+
builtins.str" builtins.str*3
20+
require_ready
21+
builtins.bool"builtins.bool *�
22+
23+
get_models'django.apps.config.AppConfig.get_models"K
24+
typing.Iterator[Type[Any]]
25+
Type[Any]
26+
Any"type"typing.Iterator*F
27+
self<
28+
django.apps.config.AppConfig"django.apps.config.AppConfig*:
29+
include_auto_created
30+
builtins.bool"builtins.bool *5
31+
include_swapped
32+
builtins.bool"builtins.bool *�
33+
__annotations__"django.apps.config.__annotations__W
34+
builtins.dict[builtins.str,Any]
35+
builtins.str" builtins.str
36+
Any"builtins.dict**
37+
Modeldjango.apps.config.Model
38+
Any

‎python-frontend/src/main/resources/org/sonar/python/types/custom_protobuf/django.apps.protobuf‎

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
2+
django.apps�
3+
AppConfigdjango.apps.config.AppConfig"builtins.object*�
4+
__init__%django.apps.config.AppConfig.__init__"
5+
None*F
6+
self<
7+
django.apps.config.AppConfig"django.apps.config.AppConfig*
8+
args
9+
Any*
10+
kwargs
11+
Any*�
12+
get_model&django.apps.config.AppConfig.get_model"
13+
Type[Any]
14+
Any"type*F
15+
self<
16+
django.apps.config.AppConfig"django.apps.config.AppConfig*,
17+
18+
model_name
19+
builtins.str" builtins.str*3
20+
require_ready
21+
builtins.bool"builtins.bool *�
22+
23+
get_models'django.apps.config.AppConfig.get_models"K
24+
typing.Iterator[Type[Any]]
25+
Type[Any]
26+
Any"type"typing.Iterator*F
27+
self<
28+
django.apps.config.AppConfig"django.apps.config.AppConfig*:
29+
include_auto_created
30+
builtins.bool"builtins.bool *5
31+
include_swapped
32+
builtins.bool"builtins.bool *l
33+
__path__django.apps.__path__J
34+
builtins.list[builtins.str]
35+
builtins.str" builtins.str"builtins.list*�
36+
__annotations__django.apps.__annotations__W
37+
builtins.dict[builtins.str,Any]
38+
builtins.str" builtins.str
39+
Any"builtins.dict*Y
40+
appsdjango.apps.registry.apps6
41+
django.apps.registry.Apps"django.apps.registry.Apps

‎python-frontend/src/main/resources/org/sonar/python/types/custom_protobuf/django.apps.registry.protobuf‎

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
2+
django.apps.registry�
3+
Appsdjango.apps.registry.Apps"builtins.object*�
4+
__init__"django.apps.registry.Apps.__init__"
5+
None*@
6+
self6
7+
django.apps.registry.Apps"django.apps.registry.Apps*
8+
args
9+
Any*
10+
kwargs
11+
Any*�
12+
13+
get_models$django.apps.registry.Apps.get_models"G
14+
builtins.list[Type[Any]]
15+
Type[Any]
16+
Any"type"builtins.list*@
17+
self6
18+
django.apps.registry.Apps"django.apps.registry.Apps*:
19+
include_auto_created
20+
builtins.bool"builtins.bool *5
21+
include_swapped
22+
builtins.bool"builtins.bool *�
23+
get_model#django.apps.registry.Apps.get_model"
24+
Type[Any]
25+
Any"type*@
26+
self6
27+
django.apps.registry.Apps"django.apps.registry.Apps*+
28+
app_label
29+
builtins.str" builtins.str*V
30+
31+
model_nameD
32+
Union[builtins.str,None]
33+
builtins.str" builtins.str
34+
None *3
35+
require_ready
36+
builtins.bool"builtins.bool *�
37+
__annotations__$django.apps.registry.__annotations__W
38+
builtins.dict[builtins.str,Any]
39+
builtins.str" builtins.str
40+
Any"builtins.dict*,
41+
Modeldjango.apps.registry.Model
42+
Any*Y
43+
appsdjango.apps.registry.apps6
44+
django.apps.registry.Apps"django.apps.registry.Apps

‎python-frontend/src/main/resources/org/sonar/python/types/custom_protobuf/django.db.models.protobuf‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,4 +18,6 @@
1818
builtins.dict[builtins.str,Any]
1919
builtins.str" builtins.str
2020
Any"builtins.dict*%
21-
managerdjango.db.models.manager 
21+
managerdjango.db.models.manager *(
22+
Modeldjango.db.models.Model
23+
Any

0 commit comments

Comments
 (0)