Ability to differentiate annotation placement in for each loop from variable declaration. #3117

Closed
vrozov opened this Issue Apr 18, 2016 · 5 comments

Comments

Projects
None yet
3 participants
@vrozov

vrozov commented Apr 18, 2016

Apache Apex would prefer to adopt code style where annotations are placed on a separate line from variable declaration except for parameters and variable definitions in for each loop (or other blocks that are single line).

For example, the following code should be valid with allowSamelineSingleParameterlessAnnotation set to false for VARIABLE_DEF token.

@Deprecated // <--class, separate line
public class Annotation
{
  @Deprecated // <--method, separate line
  public void test(@Nullable String s) { // <--parameter, same line
    @Rule // <--variable, separate line
    Integer i;
    for (@Nullable Char c : s.getChars()) { // <--variable in for each, same line // line #8
    }
  }
}

expected output:

no violations, as forcing FOR to be multi-line is weird and ugly.

current output:

java -jar checkstyle-6.17-all.jar -c 3.xml Annotation.java 
Starting audit...
[ERROR] Annotation.java:8: Annotation 'Nullable' should be alone on line. [AnnotationLocation]
Audit done.
Checkstyle ends with 1 errors.

cat -n 3.xml 
     1  <?xml version="1.0"?>
     2  <!DOCTYPE module PUBLIC "-//Puppy Crawl//DTD Check Configuration 1.3//EN" "http://www.puppycrawl.com/dtds/configuration_1_3.dtd">
     3         
     4  <module name="Checker">
     5    <property name="charset" value="UTF-8"/>
     6    <module name="TreeWalker">
     7      <module name="AnnotationLocation">
     8        <property name="allowSamelineMultipleAnnotations" value="false"/>
     9        <property name="allowSamelineSingleParameterlessAnnotation" value="false"/>
    10        <property name="allowSamelineParameterizedAnnotation" value="false"/>
    11        <!--property name="tokens" value = "CLASS_DEF, INTERFACE_DEF, ENUM_DEF, METHOD_DEF, CTOR_DEF, VARIABLE_DEF, PARAMETER_DEF, ANNOTATION_DEF, TYPECAST, LITERAL_THROWS, IMPLEMENTS_CLAUSE, TYPE_ARGUMENT, LITERAL_NEW, DOT, ANNOTATION_FIELD_DEF" /i-->
    12      </module>
    13    </module>
    14  </module>

@romani romani added the approved label May 3, 2016

@romani

This comment has been minimized.

Show comment
Hide comment
@romani

romani May 3, 2016

Member

The same might be a case for FOR, CATCH, ...... other places where variable declaration is possible.

documentation need to be updated to mention this nuance with FOR, CATCH ,.....

Member

romani commented May 3, 2016

The same might be a case for FOR, CATCH, ...... other places where variable declaration is possible.

documentation need to be updated to mention this nuance with FOR, CATCH ,.....

MEZk pushed a commit to MEZk/checkstyle that referenced this issue Jun 6, 2016

Andrei Selkin
Issue #3117: Add ability to differentiate annotation placement in for…
… / for each loop, parameter definition from variable declaration

MEZk added a commit to MEZk/checkstyle that referenced this issue Jun 10, 2016

@romani romani added the bug label Jun 18, 2016

MEZk added a commit to MEZk/checkstyle that referenced this issue Jun 18, 2016

MEZk added a commit to MEZk/checkstyle that referenced this issue Jun 19, 2016

MEZk added a commit to MEZk/checkstyle that referenced this issue Jun 19, 2016

romani added a commit that referenced this issue Jun 19, 2016

Issue #3117: Add ability to differentiate annotation placement in for…
…each, for loops, parameter definition (#3269)

@romani romani added this to the 7.0 milestone Jun 19, 2016

@romani

This comment has been minimized.

Show comment
Hide comment
@romani

romani Jun 19, 2016

Member

Fix is merged.

@vrozov , please recheck on your projects that all violations disappear. You can use snapshot versions https://github.com/checkstyle/checkstyle/wiki/How-to-use-Checkstyle-snapshot-artifacts

Member

romani commented Jun 19, 2016

Fix is merged.

@vrozov , please recheck on your projects that all violations disappear. You can use snapshot versions https://github.com/checkstyle/checkstyle/wiki/How-to-use-Checkstyle-snapshot-artifacts

@romani romani closed this Jun 19, 2016

@vrozov

This comment has been minimized.

Show comment
Hide comment
@vrozov

vrozov Jun 20, 2016

@romani Does 7.0 require Java 1.8?

vrozov commented Jun 20, 2016

@romani Does 7.0 require Java 1.8?

@romani

This comment has been minimized.

Show comment
Hide comment
@romani

romani Jun 20, 2016

Member

Yes, 1.8 is required for runtime

Member

romani commented Jun 20, 2016

Yes, 1.8 is required for runtime

@vrozov

This comment has been minimized.

Show comment
Hide comment
@vrozov

vrozov Jun 20, 2016

It will be a blocker for Apache Apex to upgrade to 7.x checkstyle.

vrozov commented Jun 20, 2016

It will be a blocker for Apache Apex to upgrade to 7.x checkstyle.

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