Skip to content

fix: redundant RCN false positive to build SpotBugs with Java 11#1248

Merged
KengoTODA merged 6 commits into
masterfrom
build-with-java-11-2
Aug 27, 2020
Merged

fix: redundant RCN false positive to build SpotBugs with Java 11#1248
KengoTODA merged 6 commits into
masterfrom
build-with-java-11-2

Conversation

@KengoTODA

@KengoTODA KengoTODA commented Aug 4, 2020

Copy link
Copy Markdown
Member

To reproduce bugs with recent Java and its compilers, it's nice to make this project ready to build with them.
Java 14 is the latest version, but 11 is the latest LTS so this PR uses Java 11 to build SpotBugs. refs #1244.
Gradle uses --release option to make sure the version of .class files are Java 8, so this change won't affect users.

At this moment we won't introduce module-info.java because some of our submodules provide the same package and it's not supported by JPMS.

Fix for RCN false positive

To pass the build with Java 11, this PR fixes #259 and #756 by ignoring RCN_REDUNDANT_NULLCHECK_OF_NONNULL_VALUE next to the end of try-block which catches java/lang/Throwable. See 7700cfc for detail.
This will fix reported false positives, but introduce false negatives in the following case, because we have no way to judge the bytecode to handle exception is generated by javac or not.

@NonNull
SomethingClosable s = ...;
try {
  ...
} catch (Throwable t) { // when the catch clause catches Throwable instance,
  // and when we have null check at the beginning of catch block,
  if (s != null) { // this is redundant null check but cannot be detected by SpotBugs any more
    ...
  }
}

I hope this false negative is acceptable to us. The redundant null check itself has less impact on runtime, I believe.

@KengoTODA KengoTODA added the build label Aug 4, 2020
@KengoTODA KengoTODA self-assigned this Aug 4, 2020
As a workaround for #996, skip the compilation for Bug1169.java in
soitbugsTestCases project.
Comment thread spotbugsTestCases/build.gradle Outdated
@KengoTODA

Copy link
Copy Markdown
Member Author

Found that not #996 but #259 is the root cause.

Problem is that the java compiler adds redundant null-check in the generated code:
https://gist.github.com/KengoTODA/8011f3860ee3d66397b9b0e247ba82c4

@KengoTODA KengoTODA marked this pull request as ready for review August 20, 2020 23:58
@KengoTODA KengoTODA added the bug label Aug 20, 2020
@sonarqubecloud

Copy link
Copy Markdown

Kudos, SonarCloud Quality Gate passed!

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities (and Security Hotspot 0 Security Hotspots to review)
Code Smell A 2 Code Smells

92.9% 92.9% Coverage
0.0% 0.0% Duplication

@KengoTODA KengoTODA changed the title Build with Java 11 fix: redundant RCN false positive to build SpotBugs with Java 11 Aug 21, 2020
@mattnelson

Copy link
Copy Markdown

Will this fix the false positives with RCN_REDUNDANT_NULLCHECK_OF_NONNULL_VALUE and RCN_REDUNDANT_NULLCHECK_WOULD_HAVE_BEEN_A_NPE?

https://github.com/spotbugs/spotbugs/issues?q=RCN_REDUNDANT_NULLCHECK_WOULD_HAVE_BEEN_A_NPE

@KengoTODA

Copy link
Copy Markdown
Member Author

@mattnelson
possible, but not sure because we have no test case nor PR to reproduce it.

@KengoTODA

Copy link
Copy Markdown
Member Author

@spotbugs/everyone please review this PR when you have time. Thanks in advance!

@henrik242 henrik242 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm.

@jscancella

Copy link
Copy Markdown
Contributor

LGTM

@KengoTODA KengoTODA merged commit f7c8c22 into master Aug 27, 2020
@KengoTODA KengoTODA deleted the build-with-java-11-2 branch August 27, 2020 01:17
baingshot added a commit to NoXml/EDATECH that referenced this pull request Oct 14, 2020
* #62 Добавил зависимости, пропертиз и файлы для базы

* #62 Добавил схему и данные для БД

* #62 Исправил схему БД

* #62 .yml вместо .properties

* #62 Добавил Jpa репозиторий для базы + Dao объект для слоя репозитория

* #62 Исправил тест для EntityController с учётом новой реализации репозитория + добавил тестовый класс для записи в базу значений

* #62 Поднял версию SpotBugs, тем самым исправил баг для DbAwareTest (spotbugs/spotbugs#1248)

* #62 Понизил покрытие jacoco

* #62 Исправил замечания

* #62 Добавил везде где нужно @nonnull

* #62 Исправил сообщение в DbAwareTest

* #62 Добавил везде где нужно @nonnull*

* #62 Добавил везде где нужно @nonnull**
asfgit pushed a commit to apache/accumulo that referenced this pull request Apr 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

false positive RCN_REDUNDANT_NULLCHECK_OF_NONNULL_VALUE on try-with-resources

6 participants