Skip to content

Commit 2caa59c

Browse files
joke1196sonartech
joke1196
authored and
sonartech
committed
SONARPY-4938 Migrate RaiseOutsideExceptCheck to TypeV2
GitOrigin-RevId: a6eda1e881ad5e6cf5c0952ac97bf5bfc1f026ac
1 parent 6be5c33 commit 2caa59c

2 files changed

Lines changed: 198 additions & 5 deletions

File tree

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

Lines changed: 100 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -16,12 +16,21 @@
1616
*/
1717
package org.sonar.python.checks;
1818

19+
import java.util.IdentityHashMap;
20+
import java.util.Map;
1921
import org.sonar.check.Rule;
2022
import org.sonar.plugins.python.api.PythonSubscriptionCheck;
21-
import org.sonar.plugins.python.api.symbols.Symbol;
23+
import org.sonar.plugins.python.api.symbols.v2.SymbolV2;
24+
import org.sonar.plugins.python.api.tree.BaseTreeVisitor;
25+
import org.sonar.plugins.python.api.tree.CallExpression;
26+
import org.sonar.plugins.python.api.tree.ClassDef;
2227
import org.sonar.plugins.python.api.tree.FunctionDef;
28+
import org.sonar.plugins.python.api.tree.Name;
29+
import org.sonar.plugins.python.api.tree.Parameter;
30+
import org.sonar.plugins.python.api.tree.QualifiedExpression;
2331
import org.sonar.plugins.python.api.tree.RaiseStatement;
2432
import org.sonar.plugins.python.api.tree.Tree;
33+
import org.sonar.python.checks.utils.CheckUtils;
2534
import org.sonar.python.tree.TreeUtils;
2635

2736
import static org.sonar.plugins.python.api.tree.Tree.Kind.EXCEPT_CLAUSE;
@@ -32,6 +41,8 @@
3241

3342
@Rule(key = "S5747")
3443
public class RaiseOutsideExceptCheck extends PythonSubscriptionCheck {
44+
private final Map<FunctionDef, Boolean> methodCallsInsideExceptBlock = new IdentityHashMap<>();
45+
3546
@Override
3647
public void initialize(Context context) {
3748
context.registerSyntaxNodeConsumer(RAISE_STMT, ctx -> {
@@ -46,7 +57,12 @@ public void initialize(Context context) {
4657
});
4758
}
4859

49-
private static boolean isInsideExceptOrFinally(Tree tree) {
60+
@Override
61+
public void leaveFile() {
62+
methodCallsInsideExceptBlock.clear();
63+
}
64+
65+
private boolean isInsideExceptOrFinally(Tree tree) {
5066
Tree parent = tree.parent();
5167
while (parent != null) {
5268
if ((parent.is(FUNCDEF))) {
@@ -61,10 +77,89 @@ private static boolean isInsideExceptOrFinally(Tree tree) {
6177
return false;
6278
}
6379

64-
private static boolean isFunctionCalledInsideExceptBlock(FunctionDef functionDef) {
65-
Symbol symbol = functionDef.name().symbol();
66-
return symbol != null && symbol.usages().stream()
80+
private boolean isFunctionCalledInsideExceptBlock(FunctionDef functionDef) {
81+
SymbolV2 symbol = functionDef.name().symbolV2();
82+
boolean hasUsageInsideExceptBlock = symbol != null && symbol.usages().stream()
6783
.filter(usage -> usage.tree() != functionDef.name())
6884
.anyMatch(usage -> TreeUtils.firstAncestorOfKind(usage.tree(), EXCEPT_CLAUSE, EXCEPT_GROUP_CLAUSE, FINALLY_CLAUSE) != null);
85+
return hasUsageInsideExceptBlock || methodCallsInsideExceptBlock.computeIfAbsent(functionDef, RaiseOutsideExceptCheck::isMethodCalledInsideExceptBlock);
86+
}
87+
88+
private static boolean isMethodCalledInsideExceptBlock(FunctionDef functionDef) {
89+
ClassDef classDef = CheckUtils.getParentClassDef(functionDef);
90+
if (classDef == null || !functionDef.isMethodDefinition()) {
91+
return false;
92+
}
93+
Tree compilationUnit = classDef;
94+
while (compilationUnit.parent() != null) {
95+
compilationUnit = compilationUnit.parent();
96+
}
97+
var visitor = new MethodCallInsideExceptVisitor(functionDef.name().name(), classDef);
98+
return visitor.isCalledInsideExceptBlock(compilationUnit);
99+
}
100+
101+
private static class MethodCallInsideExceptVisitor extends BaseTreeVisitor {
102+
private final String methodName;
103+
private final ClassDef classDef;
104+
private final SymbolV2 classSymbol;
105+
private boolean calledInsideExceptBlock;
106+
private SymbolV2 receiverSymbol;
107+
108+
MethodCallInsideExceptVisitor(String methodName, ClassDef classDef) {
109+
this.methodName = methodName;
110+
this.classDef = classDef;
111+
this.classSymbol = classDef.name().symbolV2();
112+
}
113+
114+
boolean isCalledInsideExceptBlock(Tree tree) {
115+
scan(tree);
116+
return calledInsideExceptBlock;
117+
}
118+
119+
@Override
120+
public void visitCallExpression(CallExpression callExpression) {
121+
if (callExpression.callee().is(Tree.Kind.QUALIFIED_EXPR)) {
122+
QualifiedExpression callee = (QualifiedExpression) callExpression.callee();
123+
SymbolV2 qualifierSymbol = callee.qualifier() instanceof Name name ? name.symbolV2() : null;
124+
if (qualifierSymbol != null && methodName.equals(callee.name().name())
125+
&& (qualifierSymbol == receiverSymbol || qualifierSymbol == classSymbol)
126+
&& TreeUtils.firstAncestorOfKind(callExpression, EXCEPT_CLAUSE, EXCEPT_GROUP_CLAUSE, FINALLY_CLAUSE) != null) {
127+
calledInsideExceptBlock = true;
128+
}
129+
}
130+
super.visitCallExpression(callExpression);
131+
}
132+
133+
@Override
134+
public void visitFunctionDef(FunctionDef functionDef) {
135+
SymbolV2 previousReceiverSymbol = receiverSymbol;
136+
if (functionDef.isMethodDefinition()) {
137+
receiverSymbol = isMethodOfClass(functionDef) ? receiverSymbol(functionDef) : null;
138+
}
139+
super.visitFunctionDef(functionDef);
140+
receiverSymbol = previousReceiverSymbol;
141+
}
142+
143+
@Override
144+
public void visitClassDef(ClassDef nestedClassDef) {
145+
SymbolV2 previousReceiverSymbol = receiverSymbol;
146+
receiverSymbol = null;
147+
super.visitClassDef(nestedClassDef);
148+
receiverSymbol = previousReceiverSymbol;
149+
}
150+
151+
private boolean isMethodOfClass(FunctionDef functionDef) {
152+
return functionDef.isMethodDefinition()
153+
&& CheckUtils.getParentClassDef(functionDef) == classDef
154+
&& functionDef.decorators().stream().noneMatch(decorator -> "staticmethod".equals(TreeUtils.decoratorNameFromExpression(decorator.expression())));
155+
}
156+
157+
private static SymbolV2 receiverSymbol(FunctionDef functionDef) {
158+
if (functionDef.parameters().all().isEmpty() || !functionDef.parameters().all().get(0).is(Tree.Kind.PARAMETER)) {
159+
return null;
160+
}
161+
Name receiver = ((Parameter) functionDef.parameters().all().get(0)).name();
162+
return receiver != null ? receiver.symbolV2() : null;
163+
}
69164
}
70165
}

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

Lines changed: 98 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,4 +37,102 @@ def inner():
3737
raise # Noncompliant
3838
inner()
3939

40+
class ClassWithMethodCalledFromNestedFunction:
41+
def inner(self):
42+
raise
43+
44+
def call_inner_inside_nested_function(self):
45+
def callback():
46+
try:
47+
...
48+
except:
49+
self.inner()
50+
51+
callback()
52+
53+
class ClassWithMethodCalledInsideExcept:
54+
def inner(self):
55+
raise
56+
57+
def call_inner_inside_except(self):
58+
self.inner()
59+
inner()
60+
other.inner()
61+
try:
62+
...
63+
except:
64+
self.other()
65+
self.inner()
66+
get_other().inner()
67+
68+
class ClassWithUnconventionalReceiver:
69+
def inner(this):
70+
raise
71+
72+
def call_inner_inside_except(this):
73+
try:
74+
...
75+
except:
76+
this.inner()
77+
78+
class ClassWithClassmethodReceiver:
79+
def inner(self):
80+
raise
81+
82+
@classmethod
83+
def call_inner_inside_except(cls):
84+
try:
85+
...
86+
except:
87+
cls.inner()
88+
89+
class ClassWithClassQualifiedCall:
90+
def inner(self):
91+
raise
92+
93+
try:
94+
...
95+
except:
96+
ClassWithClassQualifiedCall.inner()
97+
98+
class ClassWithStaticmethodCaller:
99+
def inner(self):
100+
raise # Noncompliant
101+
102+
@staticmethod
103+
def call_inner_inside_except(other):
104+
try:
105+
...
106+
except:
107+
other.inner()
108+
109+
class ClassWithUnresolvedQualifier:
110+
def inner(self):
111+
raise # Noncompliant
112+
113+
try:
114+
...
115+
except:
116+
undefined_obj.inner()
117+
118+
class ClassWithLocalFunction:
119+
def method(self):
120+
def inner():
121+
raise # Noncompliant
122+
try:
123+
...
124+
except:
125+
self.inner()
126+
127+
class OuterClass:
128+
def inner(self):
129+
raise # Noncompliant
130+
131+
class NestedClass:
132+
def call_inner_inside_except(self):
133+
try:
134+
...
135+
except:
136+
self.inner()
137+
40138
raise # Noncompliant

0 commit comments

Comments
 (0)