Skip to content

Commit

Permalink
SONARJAVA-1528 do not raise issue if raw exceptions are thrown by met…
Browse files Browse the repository at this point in the history
…hod invocation + rely on semantic
  • Loading branch information
Wohops committed Mar 16, 2016
1 parent e3a3e02 commit a5684b9
Show file tree
Hide file tree
Showing 6 changed files with 54 additions and 41 deletions.
8 changes: 0 additions & 8 deletions its/ruling/src/test/resources/fluent-http/squid-S00112.json
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,6 @@
'net.code-story:http:src/main/java/net/codestory/http/filters/PayloadSupplier.java':[
24,
],
'net.code-story:http:src/main/java/net/codestory/http/filters/auth/CookieAuthFilter.java':[
81,
96,
],
'net.code-story:http:src/main/java/net/codestory/http/misc/PreCompile.java':[
67,
],
Expand All @@ -31,12 +27,8 @@
22,
],
'net.code-story:http:src/main/java/net/codestory/http/routes/Route.java':[
24,
33,
],
'net.code-story:http:src/main/java/net/codestory/http/routes/RouteCollection.java':[
519,
],
'net.code-story:http:src/main/java/net/codestory/http/routes/StaticRoute.java':[
79,
],
Expand Down
1 change: 0 additions & 1 deletion its/ruling/src/test/resources/guava/squid-S00112.json
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
],
'com.google.guava:guava:src/com/google/common/cache/CacheLoader.java':[
69,
93,
121,
],
'com.google.guava:guava:src/com/google/common/cache/Striped64.java':[
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,6 @@
],
'jboss-ejb3-tutorial:extended_pc/src/org/jboss/tutorial/extended/client/Client.java':[
41,
46,
66,
],
'jboss-ejb3-tutorial:interceptor/src/org/jboss/tutorial/interceptor/bean/AccountsCancelInterceptor.java':[
63,
Expand Down
18 changes: 0 additions & 18 deletions its/ruling/src/test/resources/jdk6/squid-S00112.json
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,6 @@
'jdk6:java/awt/im/spi/InputMethodDescriptor.java':[
105,
],
'jdk6:java/beans/DefaultPersistenceDelegate.java':[
203,
],
'jdk6:java/beans/Encoder.java':[
89,
],
Expand All @@ -34,17 +31,13 @@
459,
466,
],
'jdk6:java/beans/Expression.java':[
96,
],
'jdk6:java/beans/MetaData.java':[
685,
],
'jdk6:java/beans/PropertyDescriptor.java':[
399,
],
'jdk6:java/beans/Statement.java':[
127,
131,
151,
246,
Expand All @@ -62,14 +55,6 @@
55,
80,
],
'jdk6:java/lang/ClassLoader.java':[
1356,
],
'jdk6:java/lang/StringCoding.java':[
89,
149,
249,
],
'jdk6:java/lang/ref/Finalizer.java':[
21,
],
Expand Down Expand Up @@ -119,9 +104,6 @@
'jdk6:java/rmi/server/RemoteCall.java':[
104,
],
'jdk6:java/rmi/server/RemoteObjectInvocationHandler.java':[
171,
],
'jdk6:java/rmi/server/RemoteRef.java':[
61,
114,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
package org.sonar.java.checks;

import com.google.common.collect.ImmutableSet;

import org.apache.commons.lang.BooleanUtils;
import org.sonar.api.server.rule.RulesDefinition;
import org.sonar.check.Priority;
Expand All @@ -28,8 +29,10 @@
import org.sonar.java.tag.Tag;
import org.sonar.plugins.java.api.JavaFileScanner;
import org.sonar.plugins.java.api.JavaFileScannerContext;
import org.sonar.plugins.java.api.semantic.Symbol;
import org.sonar.plugins.java.api.semantic.Type;
import org.sonar.plugins.java.api.tree.BaseTreeVisitor;
import org.sonar.plugins.java.api.tree.IdentifierTree;
import org.sonar.plugins.java.api.tree.MethodInvocationTree;
import org.sonar.plugins.java.api.tree.MethodTree;
import org.sonar.plugins.java.api.tree.NewClassTree;
import org.sonar.plugins.java.api.tree.ThrowStatementTree;
Expand All @@ -39,6 +42,7 @@
import org.sonar.squidbridge.annotations.SqaleConstantRemediation;
import org.sonar.squidbridge.annotations.SqaleSubCharacteristic;

import java.util.HashSet;
import java.util.Set;

@Rule(
Expand All @@ -51,9 +55,10 @@
@SqaleConstantRemediation("20min")
public class RawExceptionCheck extends BaseTreeVisitor implements JavaFileScanner {

private static final Set<String> RAW_EXCEPTIONS = ImmutableSet.of("Throwable", "Error", "Exception", "RuntimeException");
private static final Set<String> RAW_EXCEPTIONS = ImmutableSet.of("java.lang.Throwable", "java.lang.Error", "java.lang.Exception", "java.lang.RuntimeException");

private JavaFileScannerContext context;
private Set<Type> exceptionsThrownByMethodInvocations = new HashSet<>();

@Override
public void scanFile(JavaFileScannerContext context) {
Expand All @@ -63,34 +68,58 @@ public void scanFile(JavaFileScannerContext context) {

@Override
public void visitMethod(MethodTree tree) {
if ((tree.is(Tree.Kind.CONSTRUCTOR) || isNotOverriden(tree)) && !((MethodTreeImpl) tree).isMainMethod()) {
super.visitMethod(tree);
if ((tree.is(Tree.Kind.CONSTRUCTOR) || isNotOverriden(tree)) && isNotMainMethod(tree)) {
for (TypeTree throwClause : tree.throwsClauses()) {
checkExceptionAndRaiseIssue(throwClause);
Type exceptionType = throwClause.symbolType();
if (isRawException(exceptionType) && !exceptionsThrownByMethodInvocations.contains(exceptionType)) {
reportIssue(throwClause);
}
}
}
super.visitMethod(tree);
exceptionsThrownByMethodInvocations.clear();
}

@Override
public void visitThrowStatement(ThrowStatementTree tree) {
if (tree.expression().is(Tree.Kind.NEW_CLASS)) {
checkExceptionAndRaiseIssue(((NewClassTree) tree.expression()).identifier());
TypeTree exception = ((NewClassTree) tree.expression()).identifier();
if (isRawException(exception.symbolType())) {
reportIssue(exception);
}
}
super.visitThrowStatement(tree);
}

private void checkExceptionAndRaiseIssue(Tree tree) {
if (isRawException(tree)) {
context.reportIssue(this, tree, "Define and throw a dedicated exception instead of using a generic one.");
private void reportIssue(Tree tree) {
context.reportIssue(this, tree, "Define and throw a dedicated exception instead of using a generic one.");
}

@Override
public void visitMethodInvocation(MethodInvocationTree tree) {
if (tree.symbol().isMethodSymbol()) {
for (Type thrownType : ((Symbol.MethodSymbol) tree.symbol()).thrownTypes()) {
exceptionsThrownByMethodInvocations.add(thrownType);
}
}
super.visitMethodInvocation(tree);
}

private static boolean isRawException(Tree tree) {
return tree.is(Tree.Kind.IDENTIFIER) && RAW_EXCEPTIONS.contains(((IdentifierTree) tree).name());
private static boolean isRawException(Type type) {
for (String rawException : RAW_EXCEPTIONS) {
if (type.is(rawException)) {
return true;
}
}
return false;
}

private static boolean isNotOverriden(MethodTree tree) {
return BooleanUtils.isFalse(((MethodTreeImpl) tree).isOverriding());
}

private static boolean isNotMainMethod(MethodTree tree) {
return !((MethodTreeImpl) tree).isMainMethod();
}

}
15 changes: 14 additions & 1 deletion java-checks/src/test/files/checks/RawException.java
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,16 @@ public class Example {
throw new Throwable(); // Noncompliant
}

public void method_throwing_throwable() throws Throwable { // Compliant
throwingExceptionMethod1();
}

private void throwingExceptionMethod1() throws Throwable { // Noncompliant [[sc=50;ec=59]]
}

private void throwingExceptionMethod2() throws java.lang.Throwable { // Noncompliant [[sc=50;ec=69]]
}

public void throws_Error() {
throw new Error(); // Noncompliant [[sc=15;ec=20]]
}
Expand All @@ -16,13 +26,16 @@ public void throws_RuntimeException() {
throw new RuntimeException(); // Noncompliant
}

public void throws_custom() {
public void throws_custom() throws MyOtherException { // Compliant
throw new MyException(); // OK
}

class MyException extends RuntimeException { // Compliant
}

class MyOtherException extends Exception { // Compliant
}

public void throws_value() {
throw create(); // OK
}
Expand Down

0 comments on commit a5684b9

Please sign in to comment.