Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

false positive RCN_REDUNDANT_NULLCHECK_OF_NONNULL_VALUE on try-with-resources #259

Closed
jhonnen opened this issue Jul 14, 2017 · 17 comments · Fixed by #1248
Closed

false positive RCN_REDUNDANT_NULLCHECK_OF_NONNULL_VALUE on try-with-resources #259

jhonnen opened this issue Jul 14, 2017 · 17 comments · Fixed by #1248

Comments

@jhonnen
Copy link

jhonnen commented Jul 14, 2017

FindBugs (eclipse plugin v3.0.1) reports a redundant null check on the closing brace of the following try-with-resources statement:

public void falsePositive() {
  try (X x = new X(); X y = new X()) {
    throw new IllegalStateException();
  } // Redundant nullcheck of x, which is known to be non-null in test.Bug1169.falsePositive()
}

private static class X implements AutoCloseable {
  @Override
  public void close() {
  }
}

The finding seems to be reported when more than one AutoClosable is used and the try block ends with a thrown exception.

@KengoTODA
Copy link
Member

Please have a try with SpotBugs, it should be already solved by SpotBugs.

@KengoTODA KengoTODA self-assigned this Jul 24, 2017
@jhonnen
Copy link
Author

jhonnen commented Jul 25, 2017

@KengoTODA I've reproduced the same false positive with the eclipse spotbugs plugin 3.1.0.20170610-1497060633070:

Bug: Redundant nullcheck of x, which is known to be non-null in test.Issue259.falsePositive()

This method contains a redundant check of a known non-null value against the constant null.

Rank: Of Concern (20), confidence: Low
Pattern: RCN_REDUNDANT_NULLCHECK_OF_NONNULL_VALUE 
Type: RCN, Category: STYLE (Dodgy code)

Note that the finding is filtered with initial settings since it's rank 20 and low confidence.

@aburmeis
Copy link

still occuring using 3.1.7:

        try (Connection con = dataSource.getConnection()) {
            DatabaseMetaData meta = con.getMetaData();
            return ...
        } catch (SQLException e) {
            return false;
        }

Nullcheck of con at line ... of value previously dereferenced in ...[line <meta =>]Redundant null check at ...:[line ] RCN_REDUNDANT_NULLCHECK_WOULD_HAVE_BEEN_A_NPE

@robinjhector
Copy link
Contributor

Can confirm this is still an issue. Ping pong

@pbludov
Copy link

pbludov commented Dec 8, 2018

This bug affects checkstyle openjdk 11 build:
https://ci.appveyor.com/project/Checkstyle/checkstyle/builds/20856366/job/7h2g68dviuollcuc

[ERROR] Redundant nullcheck of stream, which is known to be non-null
in com.puppycrawl.tools.checkstyle.Main.loadProperties(File)
[com.puppycrawl.tools.checkstyle.Main, com.puppycrawl.tools.checkstyle.Main]
Redundant null check at Main.java:[line 411]Another occurrence at Main.java:[line 413]
RCN_REDUNDANT_NULLCHECK_OF_NONNULL_VALUE

The lines 411-413 of main.java are

    try (InputStream stream = Files.newInputStream(file.toPath())) {
        properties.load(stream);
    }

@bazi
Copy link

bazi commented Dec 31, 2018

Having the same false positive with v3.1.10. Using maven plugin.

Let me elaborate a little about the cause of this issue.
Try-with-resources statements are translated to simple try statements with local variable definitions. Translation can be found in Java Language Specification, section 14.20.3.1.

There we can clearly see how resources are declared as local variables before try statement and thereafter closed in finally block first being checked for nullity. And here we get redundant null check warning because that variable was used in our try block.

I don't know the code base of Spotbugs and quick overview didn't revealed details of its working, so I didn't try to make a fix. I hope that will be fixed soon or, if not, I may spend some time on my spare time.

@bbzg
Copy link

bbzg commented Jan 3, 2019

Happened to me too, SpotBugs 3.1.8, try-with-resources, Java 11

@Flugtiger
Copy link

+1 Eclipse Plugin 3.1.10r201812180244

@KengoTODA
Copy link
Member

#284 has proved that the reported problem has been solved. So I think yours is different and new problem. Please consider to make an unit test to reproduce, and submit PR to merge it to release-3.1 branch. 🙇

@stevenschlansker
Copy link

Can confirm this still affects Spotbugs 3.1.10, hopefully #259 will show that not all cases are fixed yet.

@marcelmore
Copy link

Is this related to #756?

@romani
Copy link
Contributor

romani commented Sep 28, 2019

still problem on 3.1.12.2

cpoenisch added a commit to jenkinsci/ecutest-plugin that referenced this issue Mar 9, 2020
cpoenisch added a commit to jenkinsci/ecutest-plugin that referenced this issue Mar 13, 2020
* Bump maven-checkstyle-plugin from 3.1.0 to 3.1.1

- closes #150

* Update xds references in configuration files

- update changed checkstyle modules

* Convert internal pipeline to declarative form

* Remove copyright header

* Remove copyright header

* Revise POM configuration regarding updated static code analysis

- update rules of static code analysis configurations
- use properties for CheckStyle configuration
- fail the build for all violations in analysis results
- use prepared dependencies for PMD check
- add contributor, issue and CI management section
- add fast profile without validation steps

* Sync IDEA settings with static code analysis configurations

- extend editorconfig with all IDEA specific settings

* Share IDEA run configurations

* CheckStyle: Fix JavadocParagraph violations

- remove @author tag to prevent parse error
- details: no viable alternative at input '<christian.poenisch@' while parsing HTML_ELEMENT

* CheckStyle: Fix FinalParameters

* CheckStyle: Fix SummaryJavadoc

* CheckStyle: Fix FinalLocalVariable

* CheckStyle: Fix JavaDocMethod and MissingJavadocMethod

* CheckStyle: Remove HiddenField module

* CheckStyle: Remove ReturnCount module

* CheckStyle: Fix JavadocParagraph

* CheckStyle: Fix SeparatorWrap

* CheckStyle: Fix several minor violations

* CheckStyle: Fix OperatorWrap, DeclarationOrder, OverloadMethodsDeclarationOrder

- enable SuppressWarningsFilter

* PMD: Fix violations

* CheckStyle: Fix Indentation

* CheckStyle: Fix OperatorWrap

* CheckStyle: Remove NoClone module

* CheckStyle: Fix NoFinalizer, ParameterAssignment, IllegalCatch

* Enable to scan resources for CheckStyle violations

- set UTF-8 encoding for the whole project

* CheckStyle: Fix properties files

* Fix CasC IT for TEST-GUIDE installation

* Fix several found issues of IDEA analysis inspection results

- unnecessary throws, suppress warnings
- public inner static classes
- generic types
- unused return types

* SpotBugs: Fix or suppress all warnings

- use prepared statements for SQL queries
- fix inner serializable classes
- suppress transient field warnings

* Enable CheckStyle and PMD by default

- ignore all generated and test sources for CheckStyle

* Cache Maven dependencies for GitHub CI workflow

- change to verify goal to include static code analysis
- limit to PRs to master

* Fix synchronizing ATX default settings

* Fix overriding ATX settings from pipeline

* Fix showing ATX settings groups

- revise tests

* Fix redundant NP checks

* Avoid NP check for applicable ATX custom settings

* Ignore NP checks for all Launcher.getChannel() calls

* Ignore false positive NP checks in try-with-resources

- see: spotbugs/spotbugs#259

* Move ToolVersion to util package

* Set default value for usePersistedSettings checkbox
@astappiev
Copy link

Still persists on 4.0.0

Delawen added a commit to syndesisio/syndesis that referenced this issue Jul 2, 2020
Delawen added a commit to syndesisio/syndesis that referenced this issue Jul 3, 2020
Delawen added a commit to syndesisio/syndesis that referenced this issue Jul 3, 2020
Delawen added a commit to syndesisio/syndesis that referenced this issue Jul 6, 2020
Delawen added a commit to syndesisio/syndesis that referenced this issue Jul 6, 2020
lars-sh added a commit to lars-sh/parent that referenced this issue Aug 21, 2020
…er as they might fail when using try-with-resource statements as of spotbugs/spotbugs#259 and spotbugs/spotbugs#756
KengoTODA added a commit that referenced this issue Aug 27, 2020
* ci: build with Java 11

* build: configure Gradle to use --release option

As a workaround for #996, skip the compilation for Bug1169.java in
soitbugsTestCases project.

* build: fix a typo in the build script

* fix: ignore RCN_REDUNDANT_NULLCHECK_OF_NONNULL_VALUE right after try-block

close #259

* docs: add CHANGELOG entry to describe the fix
@Zlika
Copy link

Zlika commented Sep 25, 2020

Hi @KengoTODA .
I tested Spotbugs 4.1.3 with Java 11 on my project. This new version resolved all the 7 RCN_REDUNDANT_NULLCHECK_WOULD_HAVE_BEEN_A_NPE false positives I had (#878). However, it did not solve any of the 12 RCN_REDUNDANT_NULLCHECK_OF_NONNULL_VALUE false positives I have, all located on a line of a "try-with-resource" statement.

@Zlika
Copy link

Zlika commented Sep 25, 2020

I've made a very small sample project to reproduce #259 (and also #811) with Spotbugs 4.1.3: https://github.com/Zlika/spotbugs-bug259

@Zlika
Copy link

Zlika commented Oct 2, 2020

The problem seems to be fixed by upgrading the spotbugs-maven-plugin to version 4.1.3. Previously I used spotbugs-maven-plugin 4.0.4 with a forced dependency to spotbugs 4.1.3 (as in https://maven.apache.org/plugins/maven-checkstyle-plugin/examples/upgrading-checkstyle.html). This seems to have some side effects.

@95Kevid
Copy link

95Kevid commented Mar 25, 2021

We still have the issue with 4.2.2 :-(. (false positive inside try with resources block)

elruwen added a commit to elruwen/spotbugs that referenced this issue Jun 10, 2021
…LL_VALUE in Java 11

This should fix issue spotbugs#600 and spotbugs#1338. I also fixed the broken tests for spotbugs#259.
elruwen added a commit to elruwen/spotbugs that referenced this issue Jun 10, 2021
…LL_VALUE in Java 11

This should fix issue spotbugs#600 and spotbugs#1338. I also fixed the broken tests for spotbugs#259.
KengoTODA pushed a commit that referenced this issue Jul 7, 2021
#1575)

* #600: Fix false positves for RCN_REDUNDANT_NULLCHECK_OF_NONNULL_VALUE in Java 11

This should fix issue #600 and #1338. I also fixed the broken tests for #259.

* #600: Factor out method
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.