Skip to content

Commit 2f71dcc

Browse files
andrea-guarino-sonarsourceandrea-guarino-sonarsource
authored
SONARPY-744 Avoid FP on S5864 when 'not isinstance' is used (SonarSource#843)
1 parent ea6bb39 commit 2f71dcc

3 files changed

Lines changed: 38 additions & 21 deletions

File tree

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

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,3 +15,8 @@ def builtin(param1: memoryview, param2: frozenset, param3: List[int]):
1515
def derived(param1: int, param2: int, *param3: int):
1616
(param1 + param2)[0] # Noncompliant {{Fix this "__getitem__" operation; Previous type checks suggest that this expression does not have this method.}}
1717
param3[42] # OK
18+
19+
def f(val: object):
20+
if not isinstance(val, tuple):
21+
...
22+
val[0]

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

Lines changed: 26 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,6 @@
4343
import org.sonar.python.types.TypeInference.MemberAccess;
4444

4545
import static org.sonar.plugins.python.api.tree.Tree.Kind.ASSIGNMENT_STMT;
46-
import static org.sonar.plugins.python.api.tree.Tree.Kind.CALL_EXPR;
4746
import static org.sonar.plugins.python.api.tree.Tree.Kind.NAME;
4847
import static org.sonar.plugins.python.api.tree.Tree.Kind.REGULAR_ARGUMENT;
4948

@@ -81,36 +80,42 @@ public void updateProgramState(Tree element, ProgramState programState) {
8180
handleAssignment(assignment, state);
8281
// update lhs
8382
assignment.lhsExpressions().forEach(lhs -> updateTree(lhs, state));
84-
} else if (isIsInstanceCall(element)) {
85-
Symbol firstArgumentSymbol = getFirstArgumentSymbol(((CallExpression) element), state);
86-
if (firstArgumentSymbol != null) {
87-
state.setTypes(firstArgumentSymbol, Collections.singleton(InferredTypes.anyType()));
88-
}
89-
updateTree(element, state);
9083
} else {
84+
element.accept(new IsInstanceVisitor(state));
9185
updateTree(element, state);
9286
}
9387
}
9488

95-
private static boolean isIsInstanceCall(Tree tree) {
96-
if (tree.is(CALL_EXPR)) {
97-
CallExpression callExpression = (CallExpression) tree;
89+
private static class IsInstanceVisitor extends BaseTreeVisitor {
90+
private final TypeInferenceProgramState state;
91+
92+
public IsInstanceVisitor(TypeInferenceProgramState state) {
93+
this.state = state;
94+
}
95+
96+
@Override
97+
public void visitCallExpression(CallExpression callExpression) {
9898
Symbol calleeSymbol = callExpression.calleeSymbol();
99-
return calleeSymbol != null && "isinstance".equals(calleeSymbol.fullyQualifiedName()) && callExpression.arguments().size() == 2;
99+
if (calleeSymbol != null && "isinstance".equals(calleeSymbol.fullyQualifiedName()) && callExpression.arguments().size() == 2) {
100+
Symbol firstArgumentSymbol = getFirstArgumentSymbol(callExpression);
101+
if (firstArgumentSymbol != null) {
102+
state.setTypes(firstArgumentSymbol, Collections.singleton(InferredTypes.anyType()));
103+
}
104+
}
105+
super.visitCallExpression(callExpression);
100106
}
101-
return false;
102-
}
103107

104-
@CheckForNull
105-
private static Symbol getFirstArgumentSymbol(CallExpression callExpression, TypeInferenceProgramState state) {
106-
Argument argument = callExpression.arguments().get(0);
107-
if (argument.is(REGULAR_ARGUMENT) && ((RegularArgument) argument).expression().is(NAME)) {
108-
Name variableName = (Name) ((RegularArgument) argument).expression();
109-
if (state.getTypes(variableName.symbol()).stream().anyMatch(InferredTypes::containsDeclaredType)) {
110-
return variableName.symbol();
108+
@CheckForNull
109+
private Symbol getFirstArgumentSymbol(CallExpression callExpression) {
110+
Argument argument = callExpression.arguments().get(0);
111+
if (argument.is(REGULAR_ARGUMENT) && ((RegularArgument) argument).expression().is(NAME)) {
112+
Name variableName = (Name) ((RegularArgument) argument).expression();
113+
if (state.getTypes(variableName.symbol()).stream().anyMatch(InferredTypes::containsDeclaredType)) {
114+
return variableName.symbol();
115+
}
111116
}
117+
return null;
112118
}
113-
return null;
114119
}
115120

116121
private void updateTree(Tree tree, TypeInferenceProgramState state) {

‎python-frontend/src/test/java/org/sonar/python/types/TypeInferenceTest.java‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -538,6 +538,13 @@ public void isinstance_flow_sensitive() {
538538
" x"
539539
).type()).isEqualTo(anyType());
540540

541+
assertThat(lastExpression(
542+
"def f(x: int):",
543+
" if not isinstance(x, Foo):",
544+
" ...",
545+
" x"
546+
).type()).isEqualTo(anyType());
547+
541548
FileInput fileInput = parse(
542549
"def f(x: int):",
543550
" if isinstance(x, Foo):",

0 commit comments

Comments
 (0)