Sonar analysis of OpenJDK 7 available

[Kelly O'Hair]

On Nov 24, 2011, at 12:36 AM, Roman Kennke wrote:

> Hi Kelly,
> 
>> Who gets to decide what the definition of "quality" here, or the configuration of what things to look for?
>> I see 1,285 "violations" for using extra parens, Really?  Things like  return (true);    are "violations"?
> 
> return (true); is certainly correct code, but it's not good good style.
> Code quality is not only about correctness, but also (or most
> importantly) about maintainability. Things that makes difficult to read
> are violations.

People find "return (true);" to be difficult to read? Really?  I myself have no issue with extra parens,
doesn't bother me in the least. But that's just my opinion.
In a perfect world, and one where everyone agrees on what "good good style" is, you might, just
might, consider changing hundreds or thousands of "return (true);" statements to "return true;", but
how important is that in the face of everything else? In this case, you would think the risk would be low.

But I would argue that there are many many more important quality issues than extra parens in code,
and we should focus on the more important issues.
When you include ALL quality issues like this one in a report, it creates a HUGE pile of issues that
I consider an unfair representation of the code, and a daunting situation to deal with.

My issue is one of priorities, we should focus on the more dangerous quality issues here,
and not many style issues are high priority in my opinion.

> 
>> It seems like a very nice tool, we just need to be careful what we change and why.
>> I've trusted findbugs to do no harm when fixing what it reports, but I haven't found any other tool
>> I would trust.
>> 
>> The tool PMD would tell you a variable was not used, but fail to detect that it's assignment used
>> a method call that had critical side-effects. This tool seems to suffer from the same problem.
>> So people need to be very very careful here.
> 
> Critical side effects are bad bad quality IMO.

Unnecessary side effects, I agree. But there are many APIs that rely on it, like buffers and I/O.
So it has to be something a tool understands so that recommendations are not blindly followed.

Please don't get me wrong, I think this Sonar tool is great, I just want to make sure we focus on
the right things and don't create regressions by being too quick to change code that has been around
for a  long time.

-kto

> 
> Cheers, Roman
> 
>> -kto
>> 
>> On Nov 22, 2011, at 1:24 AM, Evgeny Mandrikov wrote:
>> 
>>> Hi,
>>> 
>>> As per request of Dalibor Topic [1] I'm announcing that static analysis of
>>> OpenJDK 7 [2] by Sonar [3] available at our public instance called Nemo [4].
>>> Analysis is scheduled on a periodic basis once in a week.
>>> Dedicated quality profile was not used, so there might be some
>>> false-positive violations (like rule "Dont Import Sun"). However we are
>>> open for collaborations and ready to create a dedicated quality profile and
>>> I suppose that "Code Conventions" [5] might be used as a starting point.
>>> 
>>> [1] https://twitter.com/#!/robilad/status/138707382363635712
>>> [2] http://hg.openjdk.java.net/jdk7u/jdk7u/jdk/
>>> [3] http://www.sonarsource.org/
>>> [4] http://nemo.sonarsource.org/dashboard/index/net.java.openjdk:jdk7
>>> [5] http://openjdk.java.net/guide/codeConventions.html
>>> 
>>> -- 
>>> Best regards,
>>> Evgeny Mandrikov aka Godin <http://godin.net.ru> | SonarSource
>>> http://twitter.com/_godin_
>>> http://sonarsource.com
>> 
>> 
> 
>