SONARJAVA-1528 do not raise issue if raw exceptions are thrown by method invocation + rely on semantic

Author: Wohops

its/ruling/src/test/resources/fluent-http/squid-S00112.json

@@ -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,
 ],
@@ -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,
 ],

its/ruling/src/test/resources/guava/squid-S00112.json

@@ -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':[

its/ruling/src/test/resources/jboss-ejb3-tutorial/squid-S00112.json

@@ -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,

its/ruling/src/test/resources/jdk6/squid-S00112.json

@@ -22,9 +22,6 @@
 'jdk6:java/awt/im/spi/InputMethodDescriptor.java':[
 105,
 ],
-'jdk6:java/beans/DefaultPersistenceDelegate.java':[
-203,
-],
 'jdk6:java/beans/Encoder.java':[
 89,
 ],
@@ -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,
@@ -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,
 ],
@@ -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,

java-checks/src/main/java/org/sonar/java/checks/RawExceptionCheck.java

@@ -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;
@@ -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;
@@ -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(
@@ -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) {
@@ -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();
+  }
+
 }

java-checks/src/test/files/checks/RawException.java

@@ -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]]
   }
@@ -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
   }