Skip to content
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

Google Style Should Enforce Spaces after Commas #5624

Closed
GuyPaddock opened this issue Mar 20, 2018 · 3 comments · Fixed by #7988
Closed

Google Style Should Enforce Spaces after Commas #5624

GuyPaddock opened this issue Mar 20, 2018 · 3 comments · Fixed by #7988

Comments

@GuyPaddock
Copy link

GuyPaddock commented Mar 20, 2018

/var/tmp $ javac Test.java:

$ javac Test.java

$

/var/tmp $ cat Test.java:

$ cat Test.java
import java.util.function.BiFunction;

/**
 * A test class to highlight a violation issue.
 */
public class Test {
  /**
   * A lambda, for the purpose of illustrating the issue.
   */
  private static final BiFunction<String,String,String> FUNCTION = (a,b) -> { // Violation -- no space after comma
    return "Hello World, " + a + " and " + b;
  };

  /**
   * Entry point of the application.
   */
  public static void main(String [] args) {
    final Test test = new Test();

    test.doSomething("Lucy","Ricky");
  }

  /**
   * Runs the lambda.
   */
  public void doSomething(final String arg1,final String arg2) { // Violation -- no space after comma
    System.out.println(FUNCTION.apply(arg1,arg2)); // Violation -- no space after comma
  }
}

/var/tmp $ cat config.xml:

(We're using Google Style at this commit: https://github.com/checkstyle/checkstyle/blob/60f41e3c16e6c94b0bf8c2e5e4b4accf4ad394ab/src/main/resources/google_checks.xml).

/var/tmp $ java -Duser.language=en -Duser.country=US -jar checkstyle-8.8-all.jar -c config.xml Test.java:

Starting audit...
Audit done.

According to section 4.6.2 of the official Google Style Guide, item 5 indicates that "a single ASCII space" should appear after commas, colons, semicolons, "or the closing parenthesis ()) of a cast".

It does not appear that CheckStyle is enforcing this requirement; therefore, there is no warning that all of the commas are missing spaces.


@romani
Copy link
Member

romani commented Mar 20, 2018

@GuyPaddock , thanks a lot for detailed report.

http://checkstyle.sourceforge.net/google_style.html#a4.6.2, looks like non of mentioned Checks cover this.
http://checkstyle.sourceforge.net/config_whitespace.html#WhitespaceAfter should be used ... but WhitespaceAfter is not used at all.

@GuyPaddock , if you have time and you are affected by this and want to resolve it. I could guide you on development process(what and where to change in project to resolve this issue). Contributions are very appreciated.

@shashwatj07
Copy link
Contributor

I'm on it!

shashwatj07 added a commit to shashwatj07/checkstyle that referenced this issue Mar 29, 2020
shashwatj07 added a commit to shashwatj07/checkstyle that referenced this issue Mar 29, 2020
shashwatj07 added a commit to shashwatj07/checkstyle that referenced this issue Mar 29, 2020
shashwatj07 added a commit to shashwatj07/checkstyle that referenced this issue Apr 4, 2020
shashwatj07 added a commit to shashwatj07/checkstyle that referenced this issue Apr 13, 2020
shashwatj07 added a commit to shashwatj07/checkstyle that referenced this issue Apr 14, 2020
shashwatj07 added a commit to shashwatj07/checkstyle that referenced this issue Apr 20, 2020
shashwatj07 added a commit to shashwatj07/checkstyle that referenced this issue Apr 20, 2020
shashwatj07 added a commit to shashwatj07/checkstyle that referenced this issue Apr 20, 2020
@romani romani added this to the 8.32 milestone Apr 21, 2020
@romani
Copy link
Member

romani commented Apr 21, 2020

Fix is merged

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

Successfully merging a pull request may close this issue.

3 participants