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

Problem found when integrating with SonarQube #136

Closed
MichaelHai opened this issue Feb 22, 2017 · 7 comments
Closed

Problem found when integrating with SonarQube #136

MichaelHai opened this issue Feb 22, 2017 · 7 comments
Assignees

Comments

@MichaelHai
Copy link

I tried to integrate SpotBugs with SonarQube by simply change the dependence in sonar-findbugs-plugin from FindBugs to SpotBugs (and change the fb-contrib version for compatibiliy).
The plugin with the modification works well when I use it to analysis a project and several new issues related to lambda expression were found.
However, during the build process, this assertion failed so I have to comment it out. It checks the xml report generated by FindBugs(SpotBugs).

This is what I did: spotbugs/sonar-findbugs@master...MichaelHai:change-to-spotbugs

@jsotuyod
Copy link
Member

@MichaelHai firstly, let me thank you in name of the SpotBugs team for helping us test our release candidate for 3.1.0.

@h3xstream this seems like something you should handle, being the maintainer of that plugin. At first sight, I'm not sure what synthetic you are looking for on that report that we may be missing in SpotBugs.

@jsotuyod jsotuyod added this to the SpotBugs 3.1.0 milestone Feb 22, 2017
@h3xstream
Copy link
Member

This is an old test case.. I am not sure what synthetic bug instances are.

@KengoTODA
Copy link
Member

I've investigated about this, it seems that we can handle it as expected behaviour change.

According to git bisect, 6c91c0c is the commit which broke this test case.

FindBugs 3.0.1 reported name of anonymous class (e.g. Hello$1) which is invisible for Java coders. SpotBugs 3.1.0-RC1 reports not only this information but also name of field or local variable, to make it more intuitive. For detail, please refer related bug report.

In sonar-findbugs's case, it has no field nor local variable, so it reported methodWithViolations and which line the bug exists. So report becomes more detailed and intuitive, but now it has no <SourceLine> which tells that Hello$1 is synthetic class.

3.0.1

  <BugInstance type="SIC_INNER_SHOULD_BE_STATIC_ANON" priority="3" rank="20" abbrev="SIC" category="PERFORMANCE" instanceHash="48c8b5875e5641892e6dfa53c4ed3003" instanceOccurrenceNum="0" instanceOccurrenceMax="0">
    <ShortMessage>Could be refactored into a named static inner class</ShortMessage>
    <LongMessage>The class Hello$1 could be refactored into a named _static_ inner class</LongMessage>
    <Class classname="Hello$1" primary="true">
      <SourceLine classname="Hello$1" start="11" end="13" sourcefile="Hello.java" sourcepath="Hello.java" relSourcepath="Hello.java">
        <Message>At Hello.java:[lines 11-13]</Message>
      </SourceLine>
      <Message>In class Hello$1</Message>
    </Class>
    <SourceLine classname="Hello$1" start="11" end="13" sourcefile="Hello.java" sourcepath="Hello.java" relSourcepath="Hello.java" synthetic="true">
      <Message>At Hello.java:[lines 11-13]</Message>
    </SourceLine>
  </BugInstance>

3.1.0-RC1

  <BugInstance type="SIC_INNER_SHOULD_BE_STATIC_ANON" priority="3" rank="20" abbrev="SIC" category="PERFORMANCE" instanceHash="b3e965ceb7ff56eeaa07f1e7244d5a20" instanceOccurrenceNum="0" instanceOccurrenceMax="0">
    <ShortMessage>Could be refactored into a named static inner class</ShortMessage>
    <LongMessage>The class Hello$1 could be refactored into a named _static_ inner class</LongMessage>
    <Class classname="Hello$1" role="CLASS_ANONYMOUS">
      <SourceLine classname="Hello$1" start="11" end="13" sourcefile="Hello.java" sourcepath="Hello.java" relSourcepath="Hello.java">
        <Message>At Hello.java:[lines 11-13]</Message>
      </SourceLine>
      <Message>Anonymous class Hello$1</Message>
    </Class>
    <Class classname="Hello" primary="true">
      <SourceLine classname="Hello" start="5" end="20" sourcefile="Hello.java" sourcepath="Hello.java" relSourcepath="Hello.java">
        <Message>At Hello.java:[lines 5-20]</Message>
      </SourceLine>
      <Message>In class Hello</Message>
    </Class>
    <Method classname="Hello" name="methodWithViolations" signature="(Ljava/lang/String;)V" isStatic="false" primary="true">
      <SourceLine classname="Hello" start="10" end="16" startBytecode="0" endBytecode="75" sourcefile="Hello.java" sourcepath="Hello.java" relSourcepath="Hello.java"/>
      <Message>In method Hello.methodWithViolations(String)</Message>
    </Method>
    <SourceLine classname="Hello" primary="true" start="11" end="11" startBytecode="37" endBytecode="37" sourcefile="Hello.java" sourcepath="Hello.java" relSourcepath="Hello.java">
      <Message>At Hello.java:[line 11]</Message>
    </SourceLine>
  </BugInstance>

KengoTODA pushed a commit to KengoTODA/sonar-findbugs that referenced this issue Apr 16, 2017
This change makes sonar-findbugs depending on SpotBugs 3.1.0-RC1
which does not support Java7. See changelog for detailed difference:
https://github.com/spotbugs/spotbugs/blob/master/CHANGELOG.md#310-rc1-2017feb21

Note that this change is based on MichaelHai's change which is proposed at:
spotbugs/spotbugs#136
@KengoTODA
Copy link
Member

I've proposed a PR for SonarQubeCommunity, to share how to replace FindBugs with SpotBugs.

@jsotuyod
Copy link
Member

@KengoTODA amazing work. Good to see it wasn't a change we introduced, I couldn't recall anything that could have produced such a change. I agree the new way to report it is much more intuitive, I'm ok with just changing the test on Sonar's side.

@h3xstream
Copy link
Member

h3xstream commented Apr 21, 2017

Did any new rules were added between FindBugs 3.0.1 and SpotBugs 3.1.0-RC1 ?

I ran a diff and their is only description changes.

@KengoTODA
Copy link
Member

Did any new rules were added between FindBugs 3.0.1 and SpotBugs 3.1.0-RC1 ?

No rules were added. Please refer https://github.com/spotbugs/spotbugs/blob/master/CHANGELOG.md#added

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants