Fix for Issue 2040

CHANGELOG.md

@@ -17,6 +17,7 @@ Currently the versioning policy of this project follows [Semantic Versioning v2.
 - Fixed false positive RV_EXCEPTION_NOT_THROWN when asserting to exception throws ([[#2628](https://github.com/spotbugs/spotbugs/issues/2628)])
 - Fix false positive CT_CONSTRUCTOR_THROW when supertype has final finalize ([[#2665](https://github.com/spotbugs/spotbugs/issues/2665)])
 - Lowered the priority of `PA_PUBLIC_MUTABLE_OBJECT_ATTRIBUTE` bug ([[#2652](https://github.com/spotbugs/spotbugs/issues/2652)])
+- Fixed detector `ThrowingExceptions` by removing false positive reports, such as synthetic methods (lambdas), methods which inherited their exception specifications and methods which call throwing methods ([#2040](https://github.com/spotbugs/spotbugs/issues/2040))
 
 ### Build
 - Fix deprecated GHA on '::set-output' by using GITHUB_OUTPUT ([[#2651](https://github.com/spotbugs/spotbugs/pull/2651)])

spotbugs-tests/src/test/java/edu/umd/cs/findbugs/detect/Issue2040Test.java

@@ -0,0 +1,66 @@
+package edu.umd.cs.findbugs.detect;
+
+import edu.umd.cs.findbugs.AbstractIntegrationTest;
+import edu.umd.cs.findbugs.test.matcher.BugInstanceMatcher;
+import edu.umd.cs.findbugs.test.matcher.BugInstanceMatcherBuilder;
+import org.junit.jupiter.api.Test;
+
+import static edu.umd.cs.findbugs.test.CountMatcher.containsExactly;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.hasItem;
+
+class Issue2040Test extends AbstractIntegrationTest {
+    @Test
+    void test() {
+        performAnalysis("ghIssues/issue2040/Base.class",
+                "ghIssues/issue2040/Derived.class",
+                "ghIssues/issue2040/GenericBase.class",
+                "ghIssues/issue2040/GenericDerived.class",
+                "ghIssues/issue2040/Interface.class",
+                "ghIssues/issue2040/Generic.class",
+                "ghIssues/issue2040/Caller.class");
+
+        assertNumOfBugs("THROWS_METHOD_THROWS_RUNTIMEEXCEPTION", 3);
+        assertREBugInMethod("Derived", "lambda$lambda2$1", 36);
+        assertREBugInMethod("Derived", "runtimeExThrownFromCatch", 97);
+        assertREBugInMethod("Derived", "runtimeExceptionThrowingMethod", 45);
+
+        assertNumOfBugs("THROWS_METHOD_THROWS_CLAUSE_THROWABLE", 4);
+        assertBugInMethod("THROWS_METHOD_THROWS_CLAUSE_THROWABLE", "Base", "method");
+        assertBugInMethod("THROWS_METHOD_THROWS_CLAUSE_THROWABLE", "Derived", "throwableThrowingMethod");
+        assertBugInMethod("THROWS_METHOD_THROWS_CLAUSE_THROWABLE", "GenericBase", "method1");
+        assertBugInMethod("THROWS_METHOD_THROWS_CLAUSE_THROWABLE", "GenericBase", "method2");
+
+        assertNumOfBugs("THROWS_METHOD_THROWS_CLAUSE_BASIC_EXCEPTION", 5);
+        assertBugInMethod("THROWS_METHOD_THROWS_CLAUSE_BASIC_EXCEPTION", "Derived", "exceptionThrowingMethod");
+        assertBugInMethod("THROWS_METHOD_THROWS_CLAUSE_BASIC_EXCEPTION", "Derived", "exThrownFromCatch");
+        assertBugInMethod("THROWS_METHOD_THROWS_CLAUSE_BASIC_EXCEPTION", "GenericBase", "method");
+        assertBugInMethod("THROWS_METHOD_THROWS_CLAUSE_BASIC_EXCEPTION", "GenericBase", "method2");
+        assertBugInMethod("THROWS_METHOD_THROWS_CLAUSE_BASIC_EXCEPTION", "Interface", "iMethod");
+    }
+
+    private void assertNumOfBugs(String bugtype, int num) {
+        final BugInstanceMatcher bugTypeMatcher = new BugInstanceMatcherBuilder()
+                .bugType(bugtype).build();
+        assertThat(getBugCollection(), containsExactly(num, bugTypeMatcher));
+    }
+
+    private void assertBugInMethod(String bugType, String className, String method) {
+        final BugInstanceMatcher bugInstanceMatcher = new BugInstanceMatcherBuilder()
+                .bugType(bugType)
+                .inClass(className)
+                .inMethod(method)
+                .build();
+        assertThat(getBugCollection(), hasItem(bugInstanceMatcher));
+    }
+
+    private void assertREBugInMethod(String className, String method, int line) {
+        final BugInstanceMatcher bugInstanceMatcher = new BugInstanceMatcherBuilder()
+                .bugType("THROWS_METHOD_THROWS_RUNTIMEEXCEPTION")
+                .inClass(className)
+                .inMethod(method)
+                .atLine(line)
+                .build();
+        assertThat(getBugCollection(), hasItem(bugInstanceMatcher));
+    }
+}

spotbugs/etc/findbugs.xml

@@ -665,7 +665,7 @@
                     reports="SSD_DO_NOT_USE_INSTANCE_LOCK_ON_SHARED_STATIC_DATA"/>
           <Detector class="edu.umd.cs.findbugs.detect.DontUseFloatsAsLoopCounters" speed="fast"
                     reports="FL_FLOATS_AS_LOOP_COUNTERS"/>
-          <Detector class="edu.umd.cs.findbugs.detect.ThrowingExceptions" speed="fast" disabled="true"
+          <Detector class="edu.umd.cs.findbugs.detect.ThrowingExceptions" speed="fast"
                     reports="THROWS_METHOD_THROWS_RUNTIMEEXCEPTION,THROWS_METHOD_THROWS_CLAUSE_BASIC_EXCEPTION,THROWS_METHOD_THROWS_CLAUSE_THROWABLE" />
           <Detector class="edu.umd.cs.findbugs.detect.PermissionsSuper"
                     reports="PERM_SUPER_NOT_CALLED_IN_GETPERMISSIONS"/>

spotbugs/etc/messages.xml

@@ -8817,8 +8817,8 @@ Using floating-point variables should not be used as loop counters, as they are
     </Details>
   </BugPattern>
   <BugPattern type="THROWS_METHOD_THROWS_CLAUSE_BASIC_EXCEPTION">
-    <ShortDescription>Method lists Exception in its throws clause.</ShortDescription>
-    <LongDescription>Method lists Exception in its throws clause.</LongDescription>
+    <ShortDescription>Method lists Exception in its throws clause, but it could be more specific.</ShortDescription>
+    <LongDescription>Method lists Exception in its throws clause, but it could be more specific.</LongDescription>
     <Details>
       <![CDATA[
         <p>
@@ -8834,8 +8834,8 @@ Using floating-point variables should not be used as loop counters, as they are
     </Details>
   </BugPattern>
   <BugPattern type="THROWS_METHOD_THROWS_CLAUSE_THROWABLE">
-    <ShortDescription>Method lists Throwable in its throws clause.</ShortDescription>
-    <LongDescription>Method lists Throwable in its throws clause.</LongDescription>
+    <ShortDescription>Method lists Throwable in its throws clause, but it could be more specific.</ShortDescription>
+    <LongDescription>Method lists Throwable in its throws clause, but it could be more specific.</LongDescription>
     <Details>
       <![CDATA[
         <p>

spotbugs/src/main/java/edu/umd/cs/findbugs/ba/SignatureParser.java

@@ -113,6 +113,7 @@ public String next() {
                     break;
 
                 case 'L':
+                case 'T':
                     int semi = signature.indexOf(';', index + 1);
                     if (semi < 0) {
                         throw new IllegalStateException("Invalid method signature: " + signature);

spotbugs/src/main/java/edu/umd/cs/findbugs/detect/ThrowingExceptions.java

@@ -3,12 +3,24 @@
 import edu.umd.cs.findbugs.BugInstance;
 import edu.umd.cs.findbugs.BugReporter;
 import edu.umd.cs.findbugs.OpcodeStack;
+import edu.umd.cs.findbugs.annotations.NonNull;
+import edu.umd.cs.findbugs.ba.AnalysisContext;
+import edu.umd.cs.findbugs.ba.SignatureParser;
 import edu.umd.cs.findbugs.ba.XMethod;
 import edu.umd.cs.findbugs.bcel.OpcodeStackDetector;
+import edu.umd.cs.findbugs.internalAnnotations.DottedClassName;
 
+import java.util.Arrays;
+import java.util.Optional;
+import java.util.stream.Stream;
+
+import edu.umd.cs.findbugs.util.ClassName;
 import org.apache.bcel.Const;
+import org.apache.bcel.classfile.Code;
 import org.apache.bcel.classfile.ExceptionTable;
+import org.apache.bcel.classfile.JavaClass;
 import org.apache.bcel.classfile.Method;
+import org.apache.commons.lang3.StringUtils;
 
 
 public class ThrowingExceptions extends OpcodeStackDetector {
@@ -18,25 +30,71 @@ public ThrowingExceptions(BugReporter bugReporter) {
         this.bugReporter = bugReporter;
     }
 
+    private JavaClass clazz;
+    private String exceptionThrown = null;
+
+    @Override
+    public void visit(JavaClass obj) {
+        exceptionThrown = null;
+        clazz = obj;
+    }
+
     @Override
     public void visit(Method obj) {
+        exceptionThrown = null;
+
         if (obj.isSynthetic()) {
             return;
         }
 
-        ExceptionTable exceptionTable = obj.getExceptionTable();
-        if (exceptionTable == null) {
-            return;
+        Stream<String> exceptionStream = null;
+
+        // If the method is generic or is a method of a generic class, then first check the generic signature to avoid detection
+        // of generic descendants of Exception or Throwable as Exception or Throwable itself.
+        String signature = obj.getGenericSignature();
+        String[] exceptions = null;
+        if (signature != null) {
+            exceptions = StringUtils.substringsBetween(signature, "^", ";");
+            if (exceptions != null) {
+                exceptionStream = Arrays.stream(exceptions)
+                        .filter(s -> s.charAt(0) == 'L')
+                        .map(s -> ClassName.toDottedClassName(s.substring(1)));
+            }
+        }
+
+        // If the method is not generic or it does not throw a generic exception then it has no exception specification in its generic
+        // signature.
+        if (signature == null || exceptions == null) {
+            ExceptionTable exceptionTable = obj.getExceptionTable();
+            if (exceptionTable != null) {
+                exceptionStream = Arrays.stream(exceptionTable.getExceptionNames());
+            }
         }
 
-        String[] exceptionNames = exceptionTable.getExceptionNames();
-        for (String exception : exceptionNames) {
-            if ("java.lang.Exception".equals(exception)) {
-                reportBug("THROWS_METHOD_THROWS_CLAUSE_BASIC_EXCEPTION", getXMethod());
-            } else if ("java.lang.Throwable".equals(exception)) {
-                reportBug("THROWS_METHOD_THROWS_CLAUSE_THROWABLE", getXMethod());
+        // If the method throws Throwable or Exception because its ancestor throws the same exception then ignore it.
+        if (exceptionStream != null) {
+            Optional<String> exception = exceptionStream
+                    .filter(s -> "java.lang.Exception".equals(s) || "java.lang.Throwable".equals(s))
+                    .findAny();
+            if (exception.isPresent() && !parentThrows(obj, exception.get())) {
+                exceptionThrown = exception.get();
             }
         }
+
+        // If the method's body is empty then we report the bug immediately.
+        if (obj.getCode() == null && exceptionThrown != null) {
+            reportBug("java.lang.Exception".equals(exceptionThrown) ? "THROWS_METHOD_THROWS_CLAUSE_BASIC_EXCEPTION"
+                    : "THROWS_METHOD_THROWS_CLAUSE_THROWABLE", getXMethod());
+        }
+    }
+
+    @Override
+    public void visitAfter(Code obj) {
+        // For methods with bodies we report the exception after checking their body.
+        if (exceptionThrown != null) {
+            reportBug("java.lang.Exception".equals(exceptionThrown) ? "THROWS_METHOD_THROWS_CLAUSE_BASIC_EXCEPTION"
+                    : "THROWS_METHOD_THROWS_CLAUSE_THROWABLE", getXMethod());
+        }
     }
 
     @Override
@@ -45,13 +103,111 @@ public void sawOpcode(int seen) {
             OpcodeStack.Item item = stack.getStackItem(0);
             if (item != null) {
                 if ("Ljava/lang/RuntimeException;".equals(item.getSignature())) {
-                    reportBug("THROWS_METHOD_THROWS_RUNTIMEEXCEPTION", getXMethod());
+                    bugReporter.reportBug(
+                            new BugInstance(this, "THROWS_METHOD_THROWS_RUNTIMEEXCEPTION", LOW_PRIORITY)
+                                    .addClass(this)
+                                    .addMethod(getXMethod())
+                                    .addSourceLine(this));
                 }
             }
+        } else if (exceptionThrown != null &&
+                (seen == Const.INVOKEVIRTUAL ||
+                        seen == Const.INVOKEINTERFACE ||
+                        seen == Const.INVOKESTATIC)) {
+
+            // If the method throws Throwable or Exception because it invokes another method throwing such
+            // exceptions then ignore this bug by resetting exceptionThrown to null.
+            XMethod calledMethod = getXMethodOperand();
+            if (calledMethod == null) {
+                return;
+            }
+
+            String[] thrownExceptions = calledMethod.getThrownExceptions();
+            if (thrownExceptions != null && Arrays.stream(thrownExceptions)
+                    .map(ClassName::toDottedClassName)
+                    .anyMatch(exceptionThrown::equals)) {
+                exceptionThrown = null;
+            }
+
         }
     }
 
     private void reportBug(String bugName, XMethod method) {
-        bugReporter.reportBug(new BugInstance(this, bugName, NORMAL_PRIORITY).addClass(this).addMethod(method));
+        bugReporter.reportBug(new BugInstance(this, bugName, LOW_PRIORITY).addClass(this).addMethod(method));
+    }
+
+    private boolean parentThrows(@NonNull Method method, @DottedClassName String exception) {
+        return parentThrows(clazz, method, exception);
+    }
+
+    private boolean parentThrows(@NonNull JavaClass clazz, @NonNull Method method, @DottedClassName String exception) {
+        JavaClass ancestor;
+        boolean throwsEx = false;
+        try {
+            ancestor = clazz.getSuperClass();
+            if (ancestor != null) {
+                Optional<Method> superMethod = Arrays.stream(ancestor.getMethods())
+                        .filter(m -> method.getName().equals(m.getName()) && signatureMatches(method, m))
+                        .findAny();
+                if (superMethod.isPresent()) {
+                    throwsEx = Arrays.stream(superMethod.get().getExceptionTable().getExceptionNames())
+                            .anyMatch(exception::equals);
+                } else {
+                    throwsEx = parentThrows(ancestor, method, exception);
+                }
+            }
+
+            for (JavaClass intf : clazz.getInterfaces()) {
+                Optional<Method> superMethod = Arrays.stream(intf.getMethods())
+                        .filter(m -> method.getName().equals(m.getName()) && signatureMatches(method, m))
+                        .findAny();
+                if (superMethod.isPresent()) {
+                    throwsEx |= Arrays.stream(superMethod.get().getExceptionTable().getExceptionNames())
+                            .anyMatch(exception::equals);
+                } else {
+                    throwsEx |= parentThrows(intf, method, exception);
+                }
+            }
+        } catch (ClassNotFoundException e) {
+            AnalysisContext.reportMissingClass(e);
+        }
+        return throwsEx;
+    }
+
+    private boolean signatureMatches(Method child, Method parent) {
+        String genSig = parent.getGenericSignature();
+        if (genSig == null) {
+            return child.getSignature().equals(parent.getSignature());
+        }
+
+        String sig = child.getSignature();
+
+        SignatureParser genSP = new SignatureParser(genSig);
+        SignatureParser sp = new SignatureParser(sig);
+
+        if (genSP.getNumParameters() != sp.getNumParameters()) {
+            return false;
+        }
+
+        String[] gArgs = genSP.getArguments();
+        String[] args = sp.getArguments();
+
+        for (int i = 0; i < sp.getNumParameters(); ++i) {
+            if (gArgs[i].charAt(0) == 'T') {
+                if (args[i].charAt(0) != 'L') {
+                    return false;
+                }
+            } else {
+                if (!gArgs[i].equals(args[i])) {
+                    return false;
+                }
+            }
+        }
+
+        String gRet = genSP.getReturnTypeSignature();
+        String ret = sp.getReturnTypeSignature();
+
+        return (gRet.charAt(0) == 'T' && ret.charAt(0) == 'L') ||
+                gRet.equals(ret);
     }
 }

spotbugsTestCases/src/java/ghIssues/issue2040/Base.java

@@ -0,0 +1,7 @@
+package ghIssues.issue2040;
+
+public class Base {
+    public void method() throws Throwable {
+        throw new Throwable();
+    }
+}

spotbugsTestCases/src/java/ghIssues/issue2040/Caller.java

@@ -0,0 +1,8 @@
+package ghIssues.issue2040;
+
+public class Caller {
+    public void method() throws Throwable {
+        Base b = new Base();
+        b.method();
+    }
+}