which version of checkstyle are you using? there are two errors in parsing
the proposed checkstyle file with 5.1;
<!-- <property name="ignoreEnhancedForColon" value="false"/> -->
<!-- <module name="OneStatementPerLine"/> -->
once i fixed the checkstyle file to work with 5.1, i see that 4935 and
31935 new warnings/errors are introduced in trunk and in my i18n branches,
respectively; clearly, this is going to require a large amount of editing
to allow use of the proposed rules;
many of the new errors I notice (in both trunk and my i18n branches) have
to do with whitespace before or after '(', ')', and cast operations; i do
not agree with enforcing the presence or absence of whitespace around these
constructs; i happen to always use whitespace before and after parens,
e.g., the following should produce no checkstyle warning:
public int foo ( int a, int b, int c ) {
return bar ( a, b, c );
};
i would like whitespace after '{' and before '}' in an array
initialization, e.g., both of the following should be permitted:
int[] a = new int[] { 1, 2, 3 };
int[] a = new int[] {1, 2, 3};
i would like SimplifyBooleanReturn to be removed;
i would like whitespace after BNOT produce a warning, e.g. both ! foo and
!foo should be accepted without warning;
i would like whitespace after DOT operator to be permissible, e.g., both
x.y and x . y should be permitted;
i would like empty blocks to be permissible, e.g., the following should be
permitted:
if ( test ) {
/* TBD - handle test is true */
} else {
/* TBD - handle test is false */
}
i would like the arbitrary line length rule to be removed; i do not agree
to 110 line length; or if you insist, i could accept 150;
i do not agree with including MultipleVariableDeclarations rule; i
routinely define multiple local variables in one statement, e.g., int x, y;
i do not agree with requiring LocalFinalVariableName to match
'^[A-Z][A-Z0-9]*(_[A-Z0-9]+)*$';
instead, it should continue to match the currently used
pattern "^[a-z][a-zA-Z0-9]*$";
why are there two NoWhitespaceAfter rules?
<module name="NoWhitespaceAfter">
<property name="tokens" value="ARRAY_INIT"/>
</module>
<module name="NoWhitespaceAfter">
<property name="allowLineBreaks" value="false"/>
<property name="tokens"
value="BNOT,DEC,DOT,INC,LNOT,UNARY_MINUS,UNARY_PLUS"/>
</module>
if you fix the above problems, then i will re-run on trunk and my i18n
branch to check if there are any other issues that need to be resolved;
On Fri, Feb 3, 2012 at 10:45 AM, Vincent Hennebert <[email protected]>wrote:
> Hi All,
>
> it is well-known that people are not happy with the Checkstyle file we
> have in FOP. And there’s no point enforcing the application of
> Checkstyle rules if we don’t agree with them in the first place.
>
> I’ve finally taken on me to create a new Checkstyle file that follows
> modern development practices. I’ve been testing it on my own projects
> for a few months now and I’m happy with it, so I’d like to share it with
> the community. The idea is that once we’ve reached consensus on the
> Checkstyle rules we want to apply, we could set up a no warning policy
> and enforce it by running Checkstyle in CI.
>
> I’m also taking this as an opportunity to propose that we adopt a common
> Checkstyle policy to all the sub-projects of XML Graphics. So once we’ve
> agreed on a set of rules we would apply them to FOP and XGC immediately,
> and eventually also to Batik, and keep them in sync.
>
> We would also apply the rules to the test files as well as the main
> code. Tests are as important as the actual code and there is no reason
> why they shouldn’t be checked.
>
> It is likely that the current code will not be compliant with the new
> rules. However, most of them are really just about the syntax, so
> I believe it should be fairly straightforward to make the code at least
> 90% compliant just by applying Eclipse’s command-line code formatter.
>
> Please find the Checkstyle file attached. It is based on Checkstyle 5.5
> and basically follows Sun’s recommendations for Java styling with a few
> adaptations. What’s noteworthy is the following:
>
> • Removed checks for Javadoc. What we want is quality Javadoc, and that
> is not something that Checkstyle can check. Having Javadoc checks is
> counter-productive as it forces us to put {@inheritDoc} everywhere, or
> to create truly useless doc like the following:
> /**
> * Returns the thing.
> * @return the thing
> */
> public Thing getThing() {
> return thing;
> }
> This is just clutter really. I think it should be left to peer review
> to check whether a Javadoc comment is properly written, or whether the
> lack thereof is justified. There’s an excellent blog entry from
> Torsten Curdt about this:
> http://vafer.org/blog/20050323095453/
> • Removed check for file and method lengths. I don’t think it makes
> sense to define a maximum size for files and methods. Sometimes
> a 10-line method is way too big, sometimes it makes sense to have it
> reach 20 lines. Same for files: it’s ok to reach 1000 lines if the
> class contains several inner classes. If it doesn’t, then it’s
> probably too big. I don’t think there is any definite figure we can
> agree on and blindly follow, so I think sizes should be left to peer
> review.
> • However, I left the check for maximum line length because unreasonably
> long lines make the code hard to follow. I increased it to 110
> though to follow the evolution of monitor sizes. But as Peter
> suggested me, we probably want to keep it low in order to make
> side-by-side comparison easy.
> • I added a check for the order of imports; this is to reduce noise in
> diffs when committing. I think most of us have configured their IDE to
> automatically organise imports when saving changes to a file. This is
> a great feature because it allows to keep the list of imports
> up-to-date. But in order to avoid constant back and forth changes when
> different committers change the same file, I think it makes sense that
> we all have the same configuration. I modeled this list after
> Jeremias’ one, that I progressively inferred from his commits.
>
> Please let me know what you think. I’m inclined to follow lazy consensus
> on this, and apply the proposed changes if nobody has objected within
> 2 weeks. If anybody feels that a formal vote is necessary, feel free to
> say so.
>
> Thanks,
> Vincent
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: [email protected]
> For additional commands, e-mail: [email protected]
>
