Hey Guys,
I've been testing out the new Checkstyle 4.1 release with Hackystat.
Here are some interesting findings. Note that all this was done
locally. I didn't commit any thing.
1) The new checkstyle version found MANY errors that the old version
we were using didn't find.. Note that I had to turn failOnViolations
off to be able to see all the errors at once without having to fix
them. I also ran ant -q all.checkstyle on all of the modules in Subversion.
Here is just a sample of the roughly 100 errors that were found.
c:\java\svn\hackyCore_Build>ant -q all.checkstyle
[checkstyle]
C:\java\svn\hackyCore_Common\src\org\hackystat\core\common\selector
\combo\ComboSelector.java:40:42: Expected @param tag for 'user'.
[checkstyle]
C:\java\svn\hackyCore_Common\src\org\hackystat\core\common\selector
\combo\ComboSelector.java:40:53: Expected @param tag for 'page'.
...
[checkstyle]
C:\java\svn\hackySdt_ReviewIssue\src\org\hackystat\sdt\reviewissue\
dailyanalysis\ReviewIssueDailyAnalysis.java:110: Expected an @return tag.
...
[checkstyle]
C:\java\svn\hackyCore_Installer\src\org\hackystat\core\installer\vi
ew\swing\main\common\CommonPropertiesButtonPanel.java:73:44: Expected
@param tag
for 'e'.
[checkstyle]
C:\java\svn\hackyCore_Installer\src\org\hackystat\core\installer\vi
ew\swing\main\proxy\ProxyPropertiesButtonPanel.java:73:43: Expected
@param tag f
or 'e'.
....
[checkstyle]
C:\java\svn\hackyApp_Cgqm\src\org\hackystat\app\cgqm\plugin\issue\I
ssueResult.java:68:36: Expected @param tag for 'user'.
[checkstyle]
C:\java\svn\hackyApp_Cgqm\src\org\hackystat\app\cgqm\plugin\issue\I
ssueResult.java:68:49: Expected @throws tag for 'CgqmException'.
...
[checkstyle]
C:\java\svn\hackyApp_Review\src\org\hackystat\app\review\analysis\c
ache\ReviewActivityUserCache.java:103:38: Expected @param tag for 'user'.
[checkstyle]
C:\java\svn\hackyApp_Review\src\org\hackystat\app\review\analysis\c
ache\ReviewActivityUserCache.java:103:48: Expected @param tag for
'workspaceRoot
Set'.
...
[checkstyle]
C:\java\svn\hackySensor_Bcml\src\org\hackystat\sensor\bcml\BcmlSens
or.java:103:32: Expected @throws tag for 'BuildException'.
[checkstyle]
C:\java\svn\hackySensor_Jupiter\src\org\hackystat\sensor\jupiter\Ju
piterSensor.java:280:63: Expected @param tag for 'event'.
[checkstyle]
C:\java\svn\hackySensor_Jupiter\src\org\hackystat\sensor\jupiter\Ju
piterSensor.java:301:43: Expected @param tag for 'event'.
[checkstyle]
C:\java\svn\hackySensor_Sclc\src\org\hackystat\sensor\sclc\SclcSens
or.java:116:33: Expected @throws tag for 'BuildException'.
[checkstyle]
C:\java\svn\hackySensor_Locc\src\org\hackystat\sensor\locc\LoccSens
or.java:108:32: Expected @throws tag for 'BuildException'.
[echo] (13:33:35) Completed all.checkstyle
BUILD SUCCESSFUL
Total time: 2 minutes 3 seconds
Sending build result to Hackystat server... Done!
c:\java\svn\hackyCore_Build>
It is interesting to point out that the majority of the errors are in
hackyApp_Review, hackySdt_Review, and hackyApp_Cgqm
---------------------------------
2) I added a couple more Checks that we should be following.
<module name="LocalVariableName"/>
<module name="MemberName"/>
<module name="PackageName"/>
<module name="TabCharacter"/>
<module name="LeftCurly">
<property name="maxLineLength " value="100"/>
</module>
<module name="NeedBraces"/>
<module name="RightCurly">
<property name="option " value="alone"/>
</module>
This generated about 500-600 more errors. Many of which were either
TabCharacter or RightCurly violations. I suppose the TabCharacter
issue isn't as bad as the RightCurly violations. In addition, there
must be around 40-50 violations of naming conventions (ie. instance
variables or local variables with capital names), for example:
[checkstyle]
C:\java\svn\hackyCore_Kernel\src\org\hackystat\core\kernel\sdt\SdtM
anager.java:117:14: Name 'EclipseSensorPluginObject' must match
pattern '^[a-z][
a-zA-Z0-9]*$'.
---------------------------------
Ok.. What should we do? I suggest that we upgrade to Checkstyle 4.1
and fix the errors that the old version wasn't finding. Then I
suggest we add at least the following checks and fix those.
<module name="LocalVariableName"/>
<module name="MemberName"/>
<module name="PackageName"/>
<module name="LeftCurly">
<property name="maxLineLength " value="100"/>
</module>
<module name="NeedBraces"/>
Once we eliminate all those errors then we can start to think about
adding the TabCharacter and RightCurly checks. But that is going to
be a big task to remove all those.
<module name="TabCharacter"/>
<module name="RightCurly">
<property name="option " value="alone"/>
</module>
thanks, aaaron
