ParenPad: add LAMBDA token support #3329

Closed
eric-milles opened this Issue Jun 28, 2016 · 5 comments

Comments

Projects
None yet
4 participants
@eric-milles

eric-milles commented Jun 28, 2016

$ cat LambdaParams.java

class LambdaParams
{
    {
        java.util.function.Consumer c = ( o ) -> { o.toString() }; // 2 VIOLATIONS -- want no space after opening '(' and no space before closing ')' in lambda expression

        java.util.stream.Stream.of().forEach(( o ) -> o.toString()); // 2 VIOLATIONS -- want no space after opening '(' and no space before closing ')' in lambda expression

        java.util.stream.Stream.of().forEach(( Object o ) -> o.toString()); // 2 VIOLATIONS -- want no space after opening '(' and no space before closing ')' in lambda expression
    }

    void someMethod( String param ) // 2 VIOLATIONS -- want no space after opening '(' and no space before closing ')' in method def
    {
    }
}

$ cat config.xml

<!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="ParenPad">
            <message key="ws.followed" value="Whitespace after opening ''{0}'' should be removed" />
            <message key="ws.preceded" value="Whitespace before closing ''{0}'' should be removed" />
        </module>
    </module>
</module>

$ java -jar checkstyle-7.0-all.jar -c config.xml LambdaParams.java
Starting audit...
[ERROR] C:\Users\U0076777\LambdaParams.java:6:47: Whitespace after opening '(' should be removed [ParenPad]
[ERROR] C:\Users\U0076777\LambdaParams.java:6:49: Whitespace before closing ')' should be removed [ParenPad]
[ERROR] C:\Users\U0076777\LambdaParams.java:8:47: Whitespace after opening '(' should be removed [ParenPad]
[ERROR] C:\Users\U0076777\LambdaParams.java:8:56: Whitespace before closing ')' should be removed [ParenPad]
[ERROR] C:\Users\U0076777\LambdaParams.java:11:21: Whitespace after opening '(' should be removed [ParenPad]
[ERROR] C:\Users\U0076777\LambdaParams.java:11:34: Whitespace before closing ')' should be removed [ParenPad]
Audit done.
Checkstyle ends with 6 errors.


I'm not sure what is different about the assignment statement, but that lambda is not producing violations for the ParenPad check. There should be a total of 8 violations in the sample source.
So line 4 should produce 2 violations.

@romani romani added the approved label Sep 1, 2016

@rnveach rnveach added the easy label Mar 2, 2017

@soon

This comment has been minimized.

Show comment
Hide comment
@soon

soon Mar 2, 2017

Contributor

Working on this issue.

On the line java.util.function.Consumer c = ( o ) -> { o.toString() }; lambda is parsed as a LAMBDA token, but ParenPadCheck module does not accept lambdas and the checker skips the lambda.

On the line java.util.stream.Stream.of().forEach(( o ) -> o.toString()); lambda is a part of ELIST of METHOD_CALL and therefore processed as an expression.

My first impression was to add a TokenType.LAMBDA into a list of acceptable tokens, but I'm not sure if this is a correct solution.

Contributor

soon commented Mar 2, 2017

Working on this issue.

On the line java.util.function.Consumer c = ( o ) -> { o.toString() }; lambda is parsed as a LAMBDA token, but ParenPadCheck module does not accept lambdas and the checker skips the lambda.

On the line java.util.stream.Stream.of().forEach(( o ) -> o.toString()); lambda is a part of ELIST of METHOD_CALL and therefore processed as an expression.

My first impression was to add a TokenType.LAMBDA into a list of acceptable tokens, but I'm not sure if this is a correct solution.

@rnveach

This comment has been minimized.

Show comment
Hide comment
@rnveach

rnveach Mar 2, 2017

Member

My first impression was to add a TokenType.LAMBDA into a list of acceptable tokens

This was mine as well. There is no other way for the check to examine this token unless we tell it to visit it.
The method visitToken might have to change depending on if lambda needs special handling than the default.

Member

rnveach commented Mar 2, 2017

My first impression was to add a TokenType.LAMBDA into a list of acceptable tokens

This was mine as well. There is no other way for the check to examine this token unless we tell it to visit it.
The method visitToken might have to change depending on if lambda needs special handling than the default.

@soon

This comment has been minimized.

Show comment
Hide comment
@soon

soon Mar 2, 2017

Contributor

@rnveach Suppose we're excluding LAMBDA from the tokens list via config. Should java.util.stream.Stream.of().forEach(( o ) -> o.toString()); raise a violation it that case?

Contributor

soon commented Mar 2, 2017

@rnveach Suppose we're excluding LAMBDA from the tokens list via config. Should java.util.stream.Stream.of().forEach(( o ) -> o.toString()); raise a violation it that case?

@romani

This comment has been minimized.

Show comment
Hide comment
@romani

romani Mar 2, 2017

Member

raise a violation it that case?

no.
http://checkstyle.sourceforge.net/config_whitespace.html#ParenPad
See example. The same as if we remove "CTOR_CALL" from config, violations is disappear.

It would be super great if you put few examples of code+config to xdoc file (html site is generated from xdoc files) to show user how exactly config force code to looks like. Users like examples.

Member

romani commented Mar 2, 2017

raise a violation it that case?

no.
http://checkstyle.sourceforge.net/config_whitespace.html#ParenPad
See example. The same as if we remove "CTOR_CALL" from config, violations is disappear.

It would be super great if you put few examples of code+config to xdoc file (html site is generated from xdoc files) to show user how exactly config force code to looks like. Users like examples.

soon added a commit to soon/checkstyle that referenced this issue Mar 4, 2017

soon added a commit to soon/checkstyle that referenced this issue Mar 4, 2017

soon added a commit to soon/checkstyle that referenced this issue Mar 4, 2017

soon added a commit to soon/checkstyle that referenced this issue Mar 4, 2017

soon added a commit to soon/checkstyle that referenced this issue Mar 4, 2017

soon added a commit to soon/checkstyle that referenced this issue Mar 6, 2017

soon added a commit to soon/checkstyle that referenced this issue Mar 6, 2017

soon added a commit to soon/checkstyle that referenced this issue Mar 6, 2017

romani added a commit that referenced this issue Mar 7, 2017

@romani romani added this to the 7.7 milestone Mar 7, 2017

@romani

This comment has been minimized.

Show comment
Hide comment
@romani

romani Mar 7, 2017

Member

fix is merged.

Member

romani commented Mar 7, 2017

fix is merged.

@romani romani closed this Mar 7, 2017

sagar-shah94 pushed a commit to sagar-shah94/checkstyle that referenced this issue Mar 18, 2017

@romani romani referenced this issue in checkstyle/contribution Mar 28, 2017

Closed

releasenotes-builder: missed new feature #206

@romani romani changed the title from ParenPad: bug in lambda support to ParenPad: add lambda support Mar 28, 2017

@romani romani changed the title from ParenPad: add lambda support to ParenPad: add LAMBDA token support Mar 28, 2017

sergileon added a commit to sergileon/checkstyle that referenced this issue Sep 5, 2017

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