Re: [platform-releng-dev] Progress with Platform Build and Sonar

[John Arthorne <John_Arthorne@xxxxxxxxxx>]

I have to say I generally I agree with
Stephan here. Sonar is a great tool but the particular analysis being done
here is shaky. I spent some time combing through the results and found
very few real problems. Certainly everything of "Major" severity
and lower is purely subjective stylistic suggestions. These are better
enforced with a code formatter, with profile included in project and performed
automatically on save. Post-build analysis is the wrong time to enforce
styling. Now the user has to create a new commit, run a new build, etc,
rather than just cleaning this up automatically at development time. 

Most of the Critical bugs are false
positives as Stephan mentioned. The most obvious example, an else block
with a comment is often much better than removing the else block entirely
- this way there is an explanation in the source code of why the developer
believes that branch requires no instructions. This has no performance
penalty. There are also clear bugs in the null reference analysis. For
example it considers this a null reference:

public void setFoo(Foo foo) {
  if (foo == null && this.foo
!= null) ...

The analysis believes the field and
the variable are the same and considers this.foo to be a null reference.
Once I lose trust in the analysis I will just turn it off because it's
wasting my time. In most cases where we believe a certain code pattern
is really problematic, we add this kind of analysis in JDT. As Stephan
said if we could plug JDT warnings into Sonar this would be useful and
trustworthy information.

John




From:      
 Mickael Istria <mistria@xxxxxxxxxx>
To:      
 Common-build Developers
discussion <cbi-dev@xxxxxxxxxxx>, platform-dev@xxxxxxxxxxx, "Eclipse
platform release engineering list." <platform-releng-dev@xxxxxxxxxxx>,

Date:      
 07/11/2013 08:32 AM
Subject:    
   [platform-releng-dev]
Progress with Platform Build and Sonar
Sent by:    
   platform-releng-dev-bounces@xxxxxxxxxxx

---

Hi all,

Build (without tests) of Platform on Hudson is now successful [1]. It's
using a fork of the aggregator to use latest fix for bug 412664 but it
is meant to use the latest aggregator (I-Build) when available.
This did run default Sonar analysis (PMD + Checkstyle), and reports can
be found at [2]. On this reports you can see some metrics. So far, the
most interesting one is the Rules Compliance. Sonar configuration makes
that generally BLOCKER and CRITICAL issues are worth a fix as they're having
a bad effect at runtime. If we look at the currently broken CRITICAL rules
[3], we can see some of them (equals/hashCode or useless control) can have
bad effect on performance, some others such as "Useless operations
on immutable" are most likely bugs introduced by a wrong API usage.
Then if we look at the MAJOR rules [4], we can see many advices of how
to make code cleaner, then more maintainable. Those probably don't deserve
immediate actions, but might be worth a shot while modifying a source file.
I'd like to run FindBugs analysis on Platform as well, but I'm currently
facing an issue with FindBugs not able to find class files: https://hudson.eclipse.org/hudson/job/platform-sonar/13/console.

Note that all those warning we see in Sonar can be shown directly in Workspace
using a more aggressive warning check in JDT preferences, and installing
FindBugs plugin and enabling it on your project. I personally find that
an aggressive warning config + FindBugs really helps in avoiding some bugs
with very low effort (just fix what has a bug icon on the left). Of course,
you'll see a lot of warnings in your IDE, and not all of them are important,
but in the end, you learn how to care about the most important, and time
after time, you start adopting the conventions of checkstyle and FindBugs
which are actually quite good for maintenance.

Now that we know some places where to improve code, I'll try to work on
running performance tests. When I can have them to work, I'll try to fix
some issues spotted by Sonar, and check whether this has an effect on performances.

Here is a simple process to take advantage of Sonar: before a milestone
build, let's have a look at Sonar and find whether some issues would deserve
an effort; if yes, fix them.

I really hope that on mid-term, Sonar will be used occasionally by the
existing and new contributors, and that the project as it will help to
remove issues and make code more accessible.

Cheers,

[1] https://hudson.eclipse.org/hudson/job/platform-sonar/12/
[2] https://dev.eclipse.org/sonar/dashboard/index/25482?did=4
[3] https://dev.eclipse.org/sonar/drilldown/violations/org.eclipse.platform:platform-aggregator?severity=CRITICAL
[4] https://dev.eclipse.org/sonar/drilldown/violations/25482?severity=MAJOR
-- 
Mickael Istria
Eclipse developer at JBoss,
by Red Hat
My
blog - My
Tweets_______________________________________________
platform-releng-dev mailing list
platform-releng-dev@xxxxxxxxxxx
https://dev.eclipse.org/mailman/listinfo/platform-releng-dev