WhitespaceAround, false positive for right curly brace in Annotation #14

Closed
romani opened this Issue Oct 5, 2013 · 4 comments

Comments

Projects
None yet
2 participants
@romani
Owner

romani commented Oct 5, 2013

Created: 2013-02-27
Creator: Florian Reiser

http://checkstyle.sourceforge.net/config_whitespace.html#WhitespaceAround
When having the WhitespaceAround test configured in the default configuration, it reports an error for the right curly brace in the following snippet, even though the formatting of the annotation is completely fine. The error message is "There is no whitespace before '}'".
Code:

@Local({Interface1.class, Interface2.class})
public class Implementation implements Interface1, Interface2 {

In my opinion there should be no error in this case, as it is in an annotation (javax.ejb.Local specifically)

More examples:

    @Target({TYPE, FIELD, METHOD, LOCAL_VARIABLE}) //warning
    @SuppressWarnings({"unused", "all"}) //warning
    @SuppressWarnings({ "unused", "all" }) // ok and Eclipse do format like this
    @SuppressWarnings( { "unused", "all" } ) //ok with Check but ugly

It might be "religious" question how to format Annotation declarations, Sun/Oracle and others do that in bit different way, But I think it is reasonable to reconcile that in Checkstyle by option "skipAnnotationBraces" or smth similar

case from Created: 2009-10-07 Creator: Mark Hobson :

@MyAnnotation(myArrayElement = {"a", "b", "c"})

With: "'}' is not preceded with whitespace."

Whereas the same formatting used with array initialisation passes:

String myArrayElement = new String[] {"a", "b", "c"};
@romani

This comment has been minimized.

Show comment Hide comment
@romani

romani Feb 21, 2014

Owner

this feature is present in Google Code Style
http://google-styleguide.googlecode.com/svn/trunk/javaguide.html#s4.6.2-horizontal-whitespace
Search for "Before any open curly brace ({), with two exceptions:"

Owner

romani commented Feb 21, 2014

this feature is present in Google Code Style
http://google-styleguide.googlecode.com/svn/trunk/javaguide.html#s4.6.2-horizontal-whitespace
Search for "Before any open curly brace ({), with two exceptions:"

@flatiron32

This comment has been minimized.

Show comment Hide comment
@flatiron32

flatiron32 Feb 21, 2014

Is it acceptable to enhance the check to skip RCURLY and LCURLY for
TokenTypes.ANNOTATION_ARRAY_INIT and there is for TokenTypes.ARRAY_INIT? It seem like this is just an oversight to allow no space if the parent is Array Init and not Annotation Array Init.

If this is an acceptable solution, I have made the changes locally with tests. I can submit a pull request.

Is it acceptable to enhance the check to skip RCURLY and LCURLY for
TokenTypes.ANNOTATION_ARRAY_INIT and there is for TokenTypes.ARRAY_INIT? It seem like this is just an oversight to allow no space if the parent is Array Init and not Annotation Array Init.

If this is an acceptable solution, I have made the changes locally with tests. I can submit a pull request.

@romani

This comment has been minimized.

Show comment Hide comment
@romani

romani Feb 21, 2014

Owner

As you already did a change - please provide link to changes or do pull request to show diff. I will take a look

Owner

romani commented Feb 21, 2014

As you already did a change - please provide link to changes or do pull request to show diff. I will take a look

@flatiron32

This comment has been minimized.

Show comment Hide comment
@flatiron32

flatiron32 Feb 21, 2014

@romani romani closed this Jul 22, 2014

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment