google_checks.xml : NO space is allowed method method name and its arguments #2809

Closed
romani opened this Issue Jan 8, 2016 · 3 comments

Comments

Projects
None yet
2 participants
@romani
Member

romani commented Jan 8, 2016

http://checkstyle.sourceforge.net/reports/google-java-style.html#s4.6.2-horizontal-whitespace

Beyond where required by the language or other style rules, and apart from literals, comments and Javadoc, a single ASCII space also appears in the following places only.

main point is "a single ASCII space also appears in the following places only"

Nothing is mentioned about method names and arguments in followed list, so NO space is allowed.
We had a Check to validate this - http://checkstyle.sourceforge.net/config_whitespace.html#MethodParamPad
It should be used to cover "4.6.2" point

MethodParamPad is not used at all in google style.

it should prevent code like :
foo ( param1, param2 )

@romani romani added the approved label Jan 8, 2016

@romani romani changed the title from google_style: NO space is allowed method method name and its arguments to google_checks.xml : NO space is allowed method method name and its arguments Jan 8, 2016

@rnveach

This comment has been minimized.

Show comment
Hide comment
@rnveach

rnveach Dec 10, 2016

Member

I was only able to validate space before and after (, and before ) with these 2 checks combined.
MethodParamPad wasn't enough.

$ cat TestClass.java
public class TestClass {
    void method ( String s , Integer i ) {
method ( s , i ) ;
    }
}

$ cat TestConfig.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"/>

    <module name="TreeWalker">
<module name="MethodParamPad">
    <property name="option" value="nospace"/>
    <property name="allowLineBreaks" value="true"/>
</module>
<module name="ParenPad">
    <property name="option" value="nospace"/>
</module>
    </module>
</module>

$ java -jar checkstyle-7.3-all.jar -c TestConfig.xml TestClass.java
Starting audit...
[ERROR] TestClass.java:2:17: '(' is preceded with whitespace. [MethodParamPad]
[ERROR] TestClass.java:2:18: '(' is followed by whitespace. [ParenPad]
[ERROR] TestClass.java:2:39: ')' is preceded with whitespace. [ParenPad]
[ERROR] TestClass.java:3:8: '(' is preceded with whitespace. [MethodParamPad]
[ERROR] TestClass.java:3:9: '(' is followed by whitespace. [ParenPad]
[ERROR] TestClass.java:3:15: ')' is preceded with whitespace. [ParenPad]
Audit done.
Checkstyle ends with 6 errors.
Member

rnveach commented Dec 10, 2016

I was only able to validate space before and after (, and before ) with these 2 checks combined.
MethodParamPad wasn't enough.

$ cat TestClass.java
public class TestClass {
    void method ( String s , Integer i ) {
method ( s , i ) ;
    }
}

$ cat TestConfig.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"/>

    <module name="TreeWalker">
<module name="MethodParamPad">
    <property name="option" value="nospace"/>
    <property name="allowLineBreaks" value="true"/>
</module>
<module name="ParenPad">
    <property name="option" value="nospace"/>
</module>
    </module>
</module>

$ java -jar checkstyle-7.3-all.jar -c TestConfig.xml TestClass.java
Starting audit...
[ERROR] TestClass.java:2:17: '(' is preceded with whitespace. [MethodParamPad]
[ERROR] TestClass.java:2:18: '(' is followed by whitespace. [ParenPad]
[ERROR] TestClass.java:2:39: ')' is preceded with whitespace. [ParenPad]
[ERROR] TestClass.java:3:8: '(' is preceded with whitespace. [MethodParamPad]
[ERROR] TestClass.java:3:9: '(' is followed by whitespace. [ParenPad]
[ERROR] TestClass.java:3:15: ')' is preceded with whitespace. [ParenPad]
Audit done.
Checkstyle ends with 6 errors.
@romani

This comment has been minimized.

Show comment
Hide comment
@romani

romani Dec 10, 2016

Member

I think we need only that 3 violations

Member

romani commented Dec 10, 2016

I think we need only that 3 violations

@rnveach rnveach self-assigned this Dec 10, 2016

rnveach added a commit to rnveach/checkstyle that referenced this issue Dec 11, 2016

romani added a commit that referenced this issue Dec 11, 2016

@romani romani added the new feature label Dec 11, 2016

@romani romani added this to the 7.4 milestone Dec 11, 2016

@romani

This comment has been minimized.

Show comment
Hide comment
@romani

romani Dec 11, 2016

Member

Fix is merged

Member

romani commented Dec 11, 2016

Fix is merged

@romani romani closed this Dec 11, 2016

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