New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Wrong order of "default" method modifier #3471

Closed
agcuda opened this Issue Sep 28, 2016 · 3 comments

Comments

Projects
None yet
3 participants
@agcuda
Contributor

agcuda commented Sep 28, 2016

/var/tmp $ javac Test.java
/var/tmp $ cat Test.java
public interface Test {
  default strictfp void abc() { }
}
/var/tmp $ cat config.xml
<?xml version="1.0"?>
<!DOCTYPE module PUBLIC
          "-//Puppy Crawl//DTD Check Configuration 1.3//EN"
          "http://www.puppycrawl.com/dtds/configuration_1_3.dtd">
<module name = "Checker">
    <property name="charset" value="UTF-8"/>
    <property name="severity" value="warning"/>
    <property name="fileExtensions" value="java, properties, xml"/>
    <module name="TreeWalker">
        <module name="ModifierOrder"/>
    </module>
</module>
/var/tmp $ java -jar checkstyle-7.2-SNAPSHOT-all.jar -c config.xml YOUR_FILE.java
Starting audit...
[WARN] /var/tmp/Test.java:2:11: 'strictfp' modifier out of order with the JLS suggestions. [ModifierOrder]
Audit done.

According to the Java Language Specification, the recommended order of class and member modifiers is:
public protected private abstract default static final transient volatile synchronized native strictfp
The audit should complete without warnings.

@rnveach

This comment has been minimized.

Show comment
Hide comment
@rnveach

rnveach Sep 28, 2016

Member

In JLS8 almost all modifiers state: If two or more (distinct) ... modifiers appear in a ... declaration, then it is customary, though not required, that they appear in the order consistent with that shown above in the production for ... (meaning order should be the same as the JLS defines them)

Section 9.4 states the method modifiers for interfaces as: Annotation public abstract default static strictfp

So it does seem default is misplaced in the check.

Member

rnveach commented Sep 28, 2016

In JLS8 almost all modifiers state: If two or more (distinct) ... modifiers appear in a ... declaration, then it is customary, though not required, that they appear in the order consistent with that shown above in the production for ... (meaning order should be the same as the JLS defines them)

Section 9.4 states the method modifiers for interfaces as: Annotation public abstract default static strictfp

So it does seem default is misplaced in the check.

@romani

This comment has been minimized.

Show comment
Hide comment
@romani

romani Sep 28, 2016

Member

According to the Java Language Specification, the recommended order of class and member modifiers is:
public protected private abstract default static final transient volatile synchronized native strictfp

so ...... default ... strictfp in specification.
in input file order is the same, so there should be no violation.

So it does seem default is misplaced in the check.

Check is not about a reason/meaning of that modifiers . It is about predictable order of them if user want.

Issue is approved. Documentation also need to be updated http://checkstyle.sourceforge.net/config_modifier.html#ModifierOrder , as default modifier is missed.

Member

romani commented Sep 28, 2016

According to the Java Language Specification, the recommended order of class and member modifiers is:
public protected private abstract default static final transient volatile synchronized native strictfp

so ...... default ... strictfp in specification.
in input file order is the same, so there should be no violation.

So it does seem default is misplaced in the check.

Check is not about a reason/meaning of that modifiers . It is about predictable order of them if user want.

Issue is approved. Documentation also need to be updated http://checkstyle.sourceforge.net/config_modifier.html#ModifierOrder , as default modifier is missed.

@romani romani added the approved label Sep 28, 2016

agcuda added a commit to agcuda/checkstyle that referenced this issue Sep 29, 2016

agcuda added a commit to agcuda/checkstyle that referenced this issue Sep 29, 2016

agcuda added a commit to agcuda/checkstyle that referenced this issue Sep 29, 2016

romani added a commit that referenced this issue Sep 29, 2016

@romani romani added the bug label Sep 29, 2016

@romani romani added this to the 7.2 milestone Sep 29, 2016

@romani

This comment has been minimized.

Show comment
Hide comment
@romani

romani Sep 30, 2016

Member

fix is merge.

Member

romani commented Sep 30, 2016

fix is merge.

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