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

NoWhitespaceBefore and empty for loop conditions #5058

Closed
cushon opened this Issue Sep 2, 2017 · 4 comments

Comments

Projects
None yet
2 participants
@cushon
Contributor

cushon commented Sep 2, 2017

The Google Java style guide requires horizontal whitespace after ;: https://google.github.io/styleguide/javaguide.html#s4.6.2-horizontal-whitespace. In for loops with an empty condition, this means that there should be whitespace after the ; that terminates the initializer and before the ; that terminates the condition. NoWhitespaceBefore currently emits an unwanted error in this case.

class EmptyForLoop {
  public static void f() {
    for (; ; ) {} // these spaces are required by Google Style
  }
}
<?xml version="1.0" encoding="UTF-8"?>
<!DOCTYPE module PUBLIC "-//Puppy Crawl//DTD Check Configuration 1.3//EN" "http://www.puppycrawl.com/dtds/configuration_1_3.dtd">
<module name="Checker">
  <module name="TreeWalker">
    <module name="NoWhitespaceBefore">
      <property name="tokens" value="SEMI"/>
      <property name="allowLineBreaks" value="true"/>
    </module>
  </module>
</module>
$ java -jar checkstyle-8.2-all.jar -c checks.xml Test.java
...
[ERROR] Test.java:3:11: ';' is preceded with whitespace. [NoWhitespaceBefore]
@romani

This comment has been minimized.

Show comment
Hide comment
@romani

romani Sep 2, 2017

Member

looks like you already created such issue - #4294
please close this issue and lets continue previous issue, we are waiting for you in discussion

Member

romani commented Sep 2, 2017

looks like you already created such issue - #4294
please close this issue and lets continue previous issue, we are waiting for you in discussion

@cushon

This comment has been minimized.

Show comment
Hide comment
@cushon

cushon Sep 2, 2017

Contributor

That's about ParenPad, this is about NoWhitespaceBefore. The other bug uses for (; ; ) as an example to point out that ParenPad doesn't warn about ) being preceeded by whitespace in that case.

Contributor

cushon commented Sep 2, 2017

That's about ParenPad, this is about NoWhitespaceBefore. The other bug uses for (; ; ) as an example to point out that ParenPad doesn't warn about ) being preceeded by whitespace in that case.

@romani

This comment has been minimized.

Show comment
Hide comment
@romani

romani Sep 6, 2017

Member

special processing for FOR was added a 14 years ago - da2328d , so it is ok to extend check to cover condition are of FOR.
unfortunately web documentation does not tell anything about this, this should be fixed.

Member

romani commented Sep 6, 2017

special processing for FOR was added a 14 years ago - da2328d , so it is ok to extend check to cover condition are of FOR.
unfortunately web documentation does not tell anything about this, this should be fixed.

cushon added a commit to cushon/checkstyle that referenced this issue Sep 6, 2017

cushon added a commit to cushon/checkstyle that referenced this issue Sep 7, 2017

cushon added a commit to cushon/checkstyle that referenced this issue Sep 7, 2017

cushon added a commit to cushon/checkstyle that referenced this issue Sep 7, 2017

cushon added a commit to cushon/checkstyle that referenced this issue Sep 7, 2017

romani added a commit that referenced this issue Sep 8, 2017

@romani romani added this to the 8.3 milestone Sep 8, 2017

@romani

This comment has been minimized.

Show comment
Hide comment
@romani

romani Sep 8, 2017

Member

Fix is merged.

@cushon , thanks a lot for fix.

Member

romani commented Sep 8, 2017

Fix is merged.

@cushon , thanks a lot for fix.

@romani romani closed this Sep 8, 2017

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