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

NoLineWrap should validate lambdas #4006

Open
MEZk opened this Issue Mar 14, 2017 · 2 comments

Comments

Projects
None yet
2 participants
@MEZk
Contributor

MEZk commented Mar 14, 2017

Based on the discussion at #3755 (comment) and according to Google Java Style Guide (3 November 2016 version) to cover the rule from the guide NoLineWrap should validate lambdas. It means that LAMBDA should be included in a set of acceptable tokens and the check needs to treat LAMBDAS accordingly to Google Guide by default (as we cannot predict how users will wrap lambdas lets follow Google Guide by default).

Examples of expected behavior:

(String label, Long value, Object obj) -> { // OK
    ...
}

Predicate<String> predicate = str -> // OK
    isReversed(str);

(String label) -> // violation (should not have new line next to the arrow in a lambda)
{
    ...
}
@romani

This comment has been minimized.

Show comment
Hide comment
@romani

romani Mar 15, 2017

Member

http://checkstyle.sourceforge.net/config_whitespace.html#NoLineWrap
is for simple line wrapping for whole structure like package declaration and import declaration.

Here we have request to certain rules of wrapping in certain place. Even current OperatorWrap is not enough.

This guide point should be covered by combination of Checks (I am not 100% sure that will satisfy) :

  • NeedBrace (allowSingleLine), beware of #3837 .
  • RightCurly

@MEZk , please confirm that we are on the same position for this issue.

Member

romani commented Mar 15, 2017

http://checkstyle.sourceforge.net/config_whitespace.html#NoLineWrap
is for simple line wrapping for whole structure like package declaration and import declaration.

Here we have request to certain rules of wrapping in certain place. Even current OperatorWrap is not enough.

This guide point should be covered by combination of Checks (I am not 100% sure that will satisfy) :

  • NeedBrace (allowSingleLine), beware of #3837 .
  • RightCurly

@MEZk , please confirm that we are on the same position for this issue.

@MEZk

This comment has been minimized.

Show comment
Hide comment
@MEZk

MEZk Mar 15, 2017

Contributor

@romani
In the current implementation of the check it is possible to use it for lambdas. For example:

public class InputNoLineWrap1 {
    Predicate<String> isPolindrone = x -> // violation
    {
        return foo1(1);
    };

    boolean foo1(int a) {
        return true;
    }
}

getLastChild will return "{" (SLIST token) and we will be able to validate whether it is on the same line as lambda or not.

NeedBrace (allowSingleLine), beware of #3837 .
RightCurly

I do not understand how NeedBrace and RightCurly can help us to follow the rule.

Contributor

MEZk commented Mar 15, 2017

@romani
In the current implementation of the check it is possible to use it for lambdas. For example:

public class InputNoLineWrap1 {
    Predicate<String> isPolindrone = x -> // violation
    {
        return foo1(1);
    };

    boolean foo1(int a) {
        return true;
    }
}

getLastChild will return "{" (SLIST token) and we will be able to validate whether it is on the same line as lambda or not.

NeedBrace (allowSingleLine), beware of #3837 .
RightCurly

I do not understand how NeedBrace and RightCurly can help us to follow the rule.

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