Issue #7040: spotbugs is not executed in build process

[krichter722]

#7040

Add exclusions to spotbugs-exclude.xml for Files.newInputStream which can be ignored

I don't know why this doesn't fail on CI, but it blocks usage of mvn clean install locally. The messages don't reveal any issues and can be ignored.

[rnveach]

I assume this is another JDK12 issue.
We skip and a few things in the JDK12 run: https://github.com/checkstyle/checkstyle/blob/master/.ci/appveyor.bat#L29

[krichter722]

@rnveach Does that mean that you're in favour or opposed to this change? The issues occur on all of OpenJDK 8, 11 and 12 when I run mvn install locally. Sooner or later someone will stumbled over them again and spend time on them.

[rnveach]

If we make this change, you have to remove the exclusion for spotbugs to show the change fixes it in the CI.
I do question making an exclusion for an entire method when a JDK version far beyond what we currently use requires it for the whole project. I was waiting for @romani to give his opinion.

[krichter722]

Judging from the CI output of master and latest commit I pushed here spotbugs was never running for OpenJDK < 12. I suggest adding it for OpenJDK 8 and 12 which are GA and OpenJDK 11 which has only recently be deprecated.

[rnveach]

spotbugs was never running for OpenJDK < 12. I suggest adding it for OpenJDK 8 and 12 which are GA and OpenJDK 11 which has only recently be deprecated.

We do run it for openjdk 8.
See https://github.com/checkstyle/checkstyle/blob/master/.travis.yml#L55-L58

[romani]

@krichter722 , please define a problem and what problem you experience.

You can create issue on it or you can treat this PR as issue and reference in commit it's number "Pull #7034: ...".
Minor prefix is for stuff that is not a problem and just polishing of something.

[romani]

but it blocks usage of mvn clean install locally.

Is it resolved ? if yes, lets make CI to control it.

spotbugs:check -Dspotbugs.skip=false

please explain this hack

[romani]

items to improve:

[rnveach]

@krichter722 ping

[krichter722]

This patch allows to run mvn clean install locally. Excluding the hole method from the check is fine afaik since it's a weakness of JDK design. I excluded all occurances since I don't see a way around it.

I updated the commit message to include the PR number.

[romani]

Item to improve:

[romani]

I can not validate this PR, because I got output of spotbugs from jdk11 and placed it to issue description, but line numbers are pointing to comments in code.
It is not that easy for me to get to non jdk8 environment, if somebody have it, please share spotbugs output again.

[romani]

clarification is required

[romani]

https://github.com/checkstyle/checkstyle/blob/master/src/main/java/com/puppycrawl/tools/checkstyle/Main.java#L423-L425

Why it is JDK weakness ?
it is problem of spotbugs tool that it goes too deep in bytecode, without consideration of syntax of language, it should not report issues on this such code.
We need to report issue on spotbugs and suppress until issue on their side is resolved.

comment should be till LINK_TO_ISSUE

[romani]

we already experienced this problem, here is our team member post - spotbugs/spotbugs#259 (comment)

[romani]

comment should be till https://github.com/spotbugs/spotbugs/issues/259

[romani]

done.

[romani]

@rnveach , please finalize.

code is rebased on latest master.

[rnveach]

It seems to me
https://github.com/checkstyle/checkstyle/blob/master/.ci/appveyor.bat#L26
https://github.com/checkstyle/checkstyle/blob/master/.ci/appveyor.bat#L43
these overrides can be removed

[krichter722]

@rnveach good finding. This doesn't seem to be fixed by spotbugs so soon since it's open since 2017. What do you think about adding the spotbugs issue URL to the comment explaining the exclusion and enabling spotbugs for OpenJDK 11 and 12? The issue that final fields can't be mock with OpenJDK 12 is addressed in #7035.

[rnveach]

@romani https://teamcity.jetbrains.com/viewLog.html?buildId=2558933&buildTypeId=Checkstyle_IdeaInspectionsPullRequest&tab=buildLog&_focus=3132

The build Checkstyle::IDEA Inspections Pull Request #12381 {buildId=2558933} has been running for more than 50 minutes. Terminating...

[romani]

Relaunched,

[romani]

https://github.com/checkstyle/checkstyle/blob/master/.ci/appveyor.bat#L29
Need to be updated to remove Dspotbugs.skip=true

[romani]

appveyor is updated, @rnveach , please review.