Skip to content

Commit 5dc9d7b

Browse files
andrey-tyukin-sonarsourceandrey-tyukin-sonarsource
authored
SONARPY-666 Rule 3984: instantiation of exceptions should be completed (SonarSource#721)
* SONARPY-666 Rule S3984: instantiation of exceptions should be completed, and instantiated exceptions should be thrown
1 parent 87d8871 commit 5dc9d7b

7 files changed

Lines changed: 242 additions & 0 deletions

File tree

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -99,6 +99,7 @@ public static Iterable<Class> getChecks() {
9999
EmptyFunctionCheck.class,
100100
EmptyNestedBlockCheck.class,
101101
ExceptionCauseTypeCheck.class,
102+
ExceptionNotThrownCheck.class,
102103
ExceptionSuperClassDeclarationCheck.class,
103104
ExceptRethrowingCheck.class,
104105
ExecStatementUsageCheck.class,

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

Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,83 @@
1+
/*
2+
* SonarQube Python Plugin
3+
* Copyright (C) 2011-2020 SonarSource SA
4+
* mailto:info AT sonarsource DOT com
5+
*
6+
* This program is free software; you can redistribute it and/or
7+
* modify it under the terms of the GNU Lesser General Public
8+
* License as published by the Free Software Foundation; either
9+
* version 3 of the License, or (at your option) any later version.
10+
*
11+
* This program is distributed in the hope that it will be useful,
12+
* but WITHOUT ANY WARRANTY; without even the implied warranty of
13+
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
14+
* Lesser General Public License for more details.
15+
*
16+
* You should have received a copy of the GNU Lesser General Public License
17+
* along with this program; if not, write to the Free Software Foundation,
18+
* Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
19+
*/
20+
21+
package org.sonar.python.checks;
22+
23+
import java.util.Set;
24+
import java.util.function.Consumer;
25+
import java.util.function.Function;
26+
import java.util.stream.Collectors;
27+
import java.util.stream.Stream;
28+
import javax.annotation.Nullable;
29+
import org.sonar.check.Rule;
30+
import org.sonar.plugins.python.api.PythonSubscriptionCheck;
31+
import org.sonar.plugins.python.api.SubscriptionContext;
32+
import org.sonar.plugins.python.api.symbols.ClassSymbol;
33+
import org.sonar.plugins.python.api.symbols.Symbol;
34+
import org.sonar.plugins.python.api.tree.CallExpression;
35+
import org.sonar.plugins.python.api.tree.Name;
36+
import org.sonar.plugins.python.api.tree.Tree;
37+
import org.sonar.python.semantic.BuiltinSymbols;
38+
39+
// https://jira.sonarsource.com/browse/RSPEC-3984
40+
// https://jira.sonarsource.com/browse/SONARPY-666
41+
@Rule(key = "S3984")
42+
public class ExceptionNotThrownCheck extends PythonSubscriptionCheck {
43+
private static final String MESSAGE = "Throw this exception or remove this useless statement.";
44+
45+
@Override
46+
public void initialize(Context context) {
47+
context.registerSyntaxNodeConsumer(Tree.Kind.CALL_EXPR, check(ExceptionNotThrownCheck::symbolFromInvocation));
48+
context.registerSyntaxNodeConsumer(Tree.Kind.NAME, check(ExceptionNotThrownCheck::symbolFromName));
49+
}
50+
51+
private static Consumer<SubscriptionContext> check(Function<Tree, Symbol> extractClassSymbol) {
52+
return subscriptionContext -> {
53+
Tree t = subscriptionContext.syntaxNode();
54+
Symbol symb = extractClassSymbol.apply(t);
55+
if (symb != null && symb.is(Symbol.Kind.CLASS) && isThrowable((ClassSymbol) symb)) {
56+
Tree parent = t.parent();
57+
if (parent.is(Tree.Kind.EXPRESSION_STMT)) {
58+
subscriptionContext.addIssue(t, MESSAGE);
59+
}
60+
}
61+
};
62+
}
63+
64+
private static final Set<String> BUILTIN_EXCEPTIONS_FQNS = Stream.of(
65+
BuiltinSymbols.EXCEPTIONS,
66+
BuiltinSymbols.EXCEPTIONS_PYTHON2).flatMap(Set::stream).collect(Collectors.toSet());
67+
68+
private static boolean isThrowable(ClassSymbol cs) {
69+
return BUILTIN_EXCEPTIONS_FQNS.contains(cs.fullyQualifiedName()) ||
70+
cs.superClasses().stream().map(Symbol::fullyQualifiedName).anyMatch(BUILTIN_EXCEPTIONS_FQNS::contains);
71+
}
72+
73+
@Nullable
74+
private static Symbol symbolFromInvocation(Tree t) {
75+
return ((CallExpression) t).calleeSymbol();
76+
}
77+
78+
@Nullable
79+
private static Symbol symbolFromName(Tree t) {
80+
return ((Name) t).symbol();
81+
}
82+
83+
}

‎python-checks/src/main/resources/org/sonar/l10n/py/rules/python/S3984.html‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
<p>Creating a new <code>Exception</code> without actually raising it has no effect and is probably due to a mistake.</p>
2+
<h2>Noncompliant Code Example</h2>
3+
<pre>
4+
def func(x):
5+
if not isinstance(x, int):
6+
TypeError("Wrong type for parameter 'x'. func expects an integer") # Noncompliant
7+
if x &lt; 0:
8+
ValueError
9+
return x + 42
10+
</pre>
11+
<h2>Compliant Solution</h2>
12+
<pre>
13+
def func(x):
14+
if not isinstance(x, int):
15+
raise TypeError("Wrong type for parameter 'x'. func expects an integer")
16+
if x &lt; 0:
17+
raise ValueError
18+
return x + 42
19+
</pre>
20+
<h2>See</h2>
21+
<ul>
22+
<li> <a href="https://docs.python.org/3/tutorial/errors.html#raising-exceptions">Python documentation - Raising Exceptions</a> </li>
23+
</ul>
24+

‎python-checks/src/main/resources/org/sonar/l10n/py/rules/python/S3984.json‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
{
2+
"title": "Exceptions should not be created without being raised",
3+
"type": "BUG",
4+
"status": "ready",
5+
"remediation": {
6+
"func": "Constant\/Issue",
7+
"constantCost": "2min"
8+
},
9+
"tags": [
10+
"error-handling"
11+
],
12+
"defaultSeverity": "Major",
13+
"ruleSpecification": "RSPEC-3984",
14+
"sqKey": "S3984",
15+
"scope": "All"
16+
}

‎python-checks/src/main/resources/org/sonar/l10n/py/rules/python/Sonar_way_profile.json‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,7 @@
6565
"S3827",
6666
"S3923",
6767
"S3981",
68+
"S3984",
6869
"S3985",
6970
"S4143",
7071
"S4144",

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

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
/*
2+
* SonarQube Python Plugin
3+
* Copyright (C) 2011-2020 SonarSource SA
4+
* mailto:info AT sonarsource DOT com
5+
*
6+
* This program is free software; you can redistribute it and/or
7+
* modify it under the terms of the GNU Lesser General Public
8+
* License as published by the Free Software Foundation; either
9+
* version 3 of the License, or (at your option) any later version.
10+
*
11+
* This program is distributed in the hope that it will be useful,
12+
* but WITHOUT ANY WARRANTY; without even the implied warranty of
13+
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
14+
* Lesser General Public License for more details.
15+
*
16+
* You should have received a copy of the GNU Lesser General Public License
17+
* along with this program; if not, write to the Free Software Foundation,
18+
* Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
19+
*/
20+
package org.sonar.python.checks;
21+
22+
import org.junit.Test;
23+
import org.sonar.python.checks.utils.PythonCheckVerifier;
24+
25+
public class ExceptionNotThrownCheckTest {
26+
27+
@Test
28+
public void test() {
29+
PythonCheckVerifier.verify(
30+
"src/test/resources/checks/exceptionNotThrownCheck.py",
31+
new ExceptionNotThrownCheck());
32+
}
33+
34+
}
35+

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

Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,82 @@
1+
def notBasicBuiltinExceptions():
2+
TypeError() # Noncompliant
3+
# ^^^^^^^^^^^
4+
Exception("") # Noncompliant {{Throw this exception or remove this useless statement.}}
5+
# ^^^^^^^^^^^^^
6+
7+
# custom class inside same function
8+
def notThrowingCustomException():
9+
class Custom(TypeError):
10+
pass
11+
12+
Custom() # Noncompliant {{Throw this exception or remove this useless statement.}}
13+
# ^^^^^^^^
14+
15+
# custom class outside of any function
16+
class C1(TypeError):
17+
pass
18+
19+
def customException():
20+
C1() # Noncompliant {{Throw this exception or remove this useless statement.}}
21+
# ^^^^
22+
23+
def coverage():
24+
SomethingUnknown()
25+
SomethingUnknown
26+
27+
class C2(C3):
28+
pass
29+
30+
C2()
31+
32+
def falseNegatives():
33+
34+
# Deeper inheritance hierarchies currently don't work.
35+
class C1(TypeError):
36+
pass
37+
38+
class C2(C1):
39+
pass
40+
41+
C2() # FN. Doesn't report anything, because the inheritance hierarchy of exceptions is currently a linear list,
42+
# and the isOrExtends is not used.
43+
44+
# Binders don't bind.
45+
e = TypeError() # FN. The invocation of the `TypeError` constructor is not a statement, it's an expression.
46+
47+
48+
# rest mutably borrowed from `expected-issues`
49+
class CustomException(TypeError):
50+
pass
51+
52+
def InstantiatedBuiltinExceptions():
53+
BaseException() # Noncompliant {{Throw this exception or remove this useless statement.}}
54+
# ^^^^^^^^^^^^^^^
55+
Exception() # Noncompliant {{Throw this exception or remove this useless statement.}}
56+
# ^^^^^^^^^^^
57+
ValueError() # Noncompliant {{Throw this exception or remove this useless statement.}}
58+
# ^^^^^^^^^^^^
59+
CustomException() # Noncompliant {{Throw this exception or remove this useless statement.}}
60+
# ^^^^^^^^^^^^^^^^^
61+
62+
BaseException # Noncompliant {{Throw this exception or remove this useless statement.}}
63+
# ^^^^^^^^^^^^^
64+
Exception # Noncompliant {{Throw this exception or remove this useless statement.}}
65+
# ^^^^^^^^^
66+
ValueError # Noncompliant {{Throw this exception or remove this useless statement.}}
67+
# ^^^^^^^^^^
68+
CustomException # Noncompliant {{Throw this exception or remove this useless statement.}}
69+
# ^^^^^^^^^^^^^^^
70+
71+
72+
def compliant(param, func):
73+
lambda: ValueError() if param else None
74+
func(ValueError())
75+
if param == 1:
76+
raise ValueError() # added constructor invocation (previously no round parens)
77+
elif param == 2:
78+
raise ValueError()
79+
return ValueError()
80+
81+
def gen():
82+
yield ValueError()

0 commit comments

Comments
 (0)