Fix RCN_REDUNDANT_NULLCHECK_OF_NONNULL_VALUE in try-with-resources in…

… presence of debug symbols (#1932)

* Fix RCN_REDUNDANT_NULLCHECK_OF_NONNULL_VALUE in try-with-resources in presence of debug symbols

Resolves #1931 by improving on #1575
Probably also #868, #1338, #1694

Not entirely sure about what exactly is happening but since 4.4.0 on javac11+ (openjdk)
findbugs was reporting redundant null check if all of following holds
- code in try block always dereferences the object
- try-with-resources is applied to an object via interface reference
- debug symbols are present (LocalVariableTable, via javac -g)

javac11+ generates bytecode along the lines of
```
try-body
if (obj != null) obj.close()
on_any_exceptions_in_try_body: if (obj != null) obj.close()
```
This does always close obj if it is not null
Javac could've deduced that if try-body always throws on `obj==null` then
null check isn't needed but isn't obliged to do so, for simplicity for example.

Fix by treating invokeinterface same way as invokevirtual in FindNullDeref
Adding a minimal test for interface case.

Note that test suite is compiled with default gradle settings that have debug
symbols enabled.

* Add changelog entry

* Update Issue600Test assertions to catch the issue with interfaces

* fix indentation

CHANGELOG.md

@@ -5,6 +5,8 @@ This is the changelog for SpotBugs. This follows [Keep a Changelog v1.0.0](http:
 Currently the versioning policy of this project follows [Semantic Versioning v2.0.0](http://semver.org/spec/v2.0.0.html).
 
 ## Unreleased - 2022-??-??
+### Fixed
+- Fixed False positives for `RCN_REDUNDANT_NULLCHECK_OF_NONNULL_VALUE` on try-with-resources with interface references ([#1931](https://github.com/spotbugs/spotbugs/issues/1931))
 
 ## 4.7.0 - 2022-04-14
 ### Changed

spotbugs-tests/src/test/java/edu/umd/cs/findbugs/nullness/Issue600Test.java

@@ -37,14 +37,20 @@ public void testExplicitNulls() {
 
     private void assertBugCount(String clazz, int count) {
         BugCollection bugCollection = analyse(clazz);
+
         final BugInstanceMatcher bugTypeMatcher = new BugInstanceMatcherBuilder()
                 .bugType("RCN_REDUNDANT_NULLCHECK_OF_NONNULL_VALUE").build();
         assertThat(bugCollection, containsExactly(count, bugTypeMatcher));
+
+        final BugInstanceMatcher bugTypeMatcher2 = new BugInstanceMatcherBuilder()
+                .bugType("RCN_REDUNDANT_NULLCHECK_WOULD_HAVE_BEEN_A_NPE").build();
+        assertThat(bugCollection, containsExactly(0, bugTypeMatcher2));
     }
 
     private BugCollection analyse(String clazz) {
         return spotbugs.performAnalysis(
                 Paths.get("../spotbugsTestCases/build/classes/java/main/ghIssues/issue600/" + clazz + ".class"),
-                Paths.get("../spotbugsTestCases/build/classes/java/main/ghIssues/issue600/MyAutocloseable.class"));
+                Paths.get("../spotbugsTestCases/build/classes/java/main/ghIssues/issue600/MyAutocloseable.class"),
+                Paths.get("../spotbugsTestCases/build/classes/java/main/ghIssues/issue600/IMyAutocloseable.class"));
     }
 }

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

@@ -49,6 +49,7 @@
 import org.apache.bcel.generic.IFNONNULL;
 import org.apache.bcel.generic.IFNULL;
 import org.apache.bcel.generic.INVOKEDYNAMIC;
+import org.apache.bcel.generic.INVOKEINTERFACE;
 import org.apache.bcel.generic.INVOKEVIRTUAL;
 import org.apache.bcel.generic.Instruction;
 import org.apache.bcel.generic.InstructionHandle;
@@ -1919,6 +1920,21 @@ private boolean isGeneratedCodeInCatchBlockViaLineNumber(@NonNull ConstantPool c
                     }
                 }
                 break;
+            case Const.INVOKEINTERFACE:
+                if (handle.getInstruction() instanceof INVOKEINTERFACE) {
+                    String methodName = ((INVOKEINTERFACE) handle.getInstruction()).getMethodName(cpg);
+                    if ("close".equals(methodName)) {
+                        // the close methods get the line number of the end of the try block assigned
+                        if (throwables.stream().anyMatch(x -> lineNumberTable.getSourceLine(x.getEndPC()) == currentLine)) {
+                            closeCounter++;
+                        }
+                    } else if ("addSuppressed".equals(methodName)) {
+                        if (relevantLineNumbers.contains(currentLine)) {
+                            addSuppressedPresent = true;
+                        }
+                    }
+                }
+                break;
             case Const.ATHROW:
                 if (relevantLineNumbers.contains(currentLine) && handle.getInstruction() instanceof ATHROW) {
                     Class<?>[] exceptions = ((ATHROW) handle.getInstruction()).getExceptions();

spotbugsTestCases/src/java/ghIssues/issue600/IMyAutocloseable.java

@@ -0,0 +1,4 @@
+package ghIssues.issue600;
+
+public interface IMyAutocloseable extends AutoCloseable {
+}

spotbugsTestCases/src/java/ghIssues/issue600/TryWithResourcesSimple.java

@@ -1,6 +1,12 @@
 package ghIssues.issue600;
 
 public class TryWithResourcesSimple {
+    public int nocatchInterface(IMyAutocloseable x) throws Exception {
+        try (IMyAutocloseable closeable = x) {
+            return closeable.hashCode();
+        }
+    }
+
     public void nocatch() throws Exception {
         try (MyAutocloseable closeable = MyAutocloseable.create()) {
             closeable.exampleMethod();