Skip to content

Commit

Permalink
Issue checkstyle#3546: add lambda in a rightcurlycheck
Browse files Browse the repository at this point in the history
  • Loading branch information
romani authored and Vladlis committed Mar 9, 2017
1 parent 55041a3 commit 1925eb2
Show file tree
Hide file tree
Showing 7 changed files with 157 additions and 56 deletions.
Expand Up @@ -121,4 +121,19 @@ public void testRightCurlyAloneSameAndLiteralDo() throws Exception {
final Integer[] warnList = getLinesWithWarn(filePath);
verify(checkConfig, filePath, expected, warnList);
}

@Test
public void testRightCurlyAloneInLambda() throws Exception {
final String[] expected = {
"12:70: " + getCheckMessage(RightCurlyCheck.class, MSG_KEY_LINE_ALONE, "}", 70),
"15:46: " + getCheckMessage(RightCurlyCheck.class, MSG_KEY_LINE_ALONE, "}", 46),
"22:33: " + getCheckMessage(RightCurlyCheck.class, MSG_KEY_LINE_ALONE, "}", 33),
};

final Configuration checkConfig = getCheckConfig("RightCurly", "RightCurlyAlone");
final String filePath = getPath("InputRightCurlyAloneLambda.java");

final Integer[] warnList = {12, 15, 22};
verify(checkConfig, filePath, expected, warnList);
}
}
@@ -0,0 +1,30 @@
package com.puppycrawl.tools.checkstyle.checks.blocks;

/*Test lambda*/
public class InputRightCurlyAloneLambda {

static Runnable k1 = () -> {
String.valueOf("Test rightCurly 1!");
};

static Runnable k2 = () -> String.valueOf("Test rightCurly 2!");

static Runnable k3 = () -> {String.valueOf("Test rightCurly 3!");}; // violation

static Runnable k4 = () -> {
String.valueOf("Test rightCurly 4!");}; // violation

static Runnable k5 = () ->
{
String.valueOf("Test rightCurly 5!");
};

static Runnable k6 = () -> {}; // violation

static Runnable k7 = () -> {
};

static Runnable k8 = () ->
{
};
}
Expand Up @@ -40,7 +40,8 @@
* {@link TokenTypes#LITERAL_CATCH LITERAL_CATCH},
* {@link TokenTypes#LITERAL_FINALLY LITERAL_FINALLY},
* {@link TokenTypes#LITERAL_IF LITERAL_IF},
* {@link TokenTypes#LITERAL_ELSE LITERAL_ELSE}.
* {@link TokenTypes#LITERAL_ELSE LITERAL_ELSE},
* {@link TokenTypes#LAMBDA LAMBDA}.
* Other acceptable tokens are:
* {@link TokenTypes#CLASS_DEF CLASS_DEF},
* {@link TokenTypes#METHOD_DEF METHOD_DEF},
Expand All @@ -50,6 +51,7 @@
* {@link TokenTypes#LITERAL_DO LITERAL_DO}.
* {@link TokenTypes#STATIC_INIT STATIC_INIT}.
* {@link TokenTypes#INSTANCE_INIT INSTANCE_INIT}.
* {@link TokenTypes#LAMBDA LAMBDA}.
* </p>
* <p>
* <b>shouldStartLine</b> - does the check need to check
Expand Down Expand Up @@ -141,6 +143,7 @@ public int[] getDefaultTokens() {
TokenTypes.LITERAL_FINALLY,
TokenTypes.LITERAL_IF,
TokenTypes.LITERAL_ELSE,
TokenTypes.LAMBDA,
};
}

Expand All @@ -160,6 +163,7 @@ public int[] getAcceptableTokens() {
TokenTypes.LITERAL_DO,
TokenTypes.STATIC_INIT,
TokenTypes.INSTANCE_INIT,
TokenTypes.LAMBDA,
};
}

Expand Down Expand Up @@ -201,14 +205,11 @@ public void visitToken(DetailAST ast) {
private static String validate(Details details, RightCurlyOption bracePolicy,
boolean shouldStartLine, String targetSourceLine) {
final DetailAST rcurly = details.rcurly;
final DetailAST lcurly = details.lcurly;
final DetailAST nextToken = details.nextToken;
final boolean shouldCheckLastRcurly = details.shouldCheckLastRcurly;
String violation = "";

if (bracePolicy == RightCurlyOption.SAME
&& !hasLineBreakBefore(rcurly)
&& lcurly.getLineNo() != rcurly.getLineNo()) {
if (shouldHaveBeLineBreakBefore(bracePolicy, details)) {
violation = MSG_KEY_LINE_BREAK_BEFORE;
}
else if (shouldCheckLastRcurly) {
Expand Down Expand Up @@ -245,15 +246,22 @@ private static boolean shouldBeOnSameLine(RightCurlyOption bracePolicy, Details
* @param details Details for validation
* @return true if a right curly should be alone on a line.
*/
// -@cs[CyclomaticComplexity] keeping the logic in one place increases readability
// -@cs[BooleanExpressionComplexity] keeping the logic in one place increases readability
private static boolean shouldBeAloneOnLine(RightCurlyOption bracePolicy, Details details) {
final int tokenLambda = details.lcurly.getParent().getType();
return bracePolicy == RightCurlyOption.ALONE
&& !isAloneOnLine(details)
&& !isEmptyBody(details.lcurly)
&& tokenLambda != TokenTypes.LAMBDA
&& !isAloneOnLine(details)
&& !isEmptyBody(details.lcurly)
|| bracePolicy == RightCurlyOption.ALONE_OR_SINGLELINE
&& !isAloneOnLine(details)
&& !isSingleLineBlock(details)
&& !isAnonInnerClassInit(details.lcurly)
&& !isEmptyBody(details.lcurly);
&& !isAloneOnLine(details)
&& !isSingleLineBlock(details)
&& !isAnonInnerClassInit(details.lcurly)
&& !isEmptyBody(details.lcurly)
|| bracePolicy == RightCurlyOption.ALONE
&& tokenLambda == TokenTypes.LAMBDA
&& !isAloneLambda(details);
}

/**
Expand Down Expand Up @@ -293,6 +301,24 @@ private static boolean isSingleLineBlock(Details details) {
&& rcurly.getLineNo() != nextToken.getLineNo();
}

/**
* Checks if right curly is alone on line in token the lambda.
* @param details for validation.
* @return true, if right curly is alone on line.
*/
private static boolean isAloneLambda(Details details) {
final DetailAST lcurly = details.lcurly;
final DetailAST rcurly = details.rcurly;
final int lcurlyNo = lcurly.getLineNo();
final int rcurlyNo = rcurly.getLineNo();
boolean result = false;
if (lcurlyNo != rcurlyNo) {
result = rcurly.getPreviousSibling() == null
|| rcurlyNo != rcurly.getPreviousSibling().getLineNo();
}
return result;
}

/**
* Checks whether lcurly is in anonymous inner class initialization.
* @param lcurly left curly token.
Expand All @@ -310,7 +336,6 @@ private static boolean isAnonInnerClassInit(DetailAST lcurly) {
*/
// -@cs[JavaNCSS] getDetails() method is a huge SWITCH, it has to be monolithic
// -@cs[ExecutableStatementCount] getDetails() method is a huge SWITCH, it has to be monolithic
// -@cs[NPathComplexity] getDetails() method is a huge SWITCH, it has to be monolithic
private static Details getDetails(DetailAST ast) {
// Attempt to locate the tokens to do the check
boolean shouldCheckLastRcurly = false;
Expand All @@ -320,19 +345,9 @@ private static Details getDetails(DetailAST ast) {

switch (ast.getType()) {
case TokenTypes.LITERAL_TRY:
if (ast.getFirstChild().getType() == TokenTypes.RESOURCE_SPECIFICATION) {
lcurly = ast.getFirstChild().getNextSibling();
}
else {
lcurly = ast.getFirstChild();
}
lcurly = ast.getFirstChild();
nextToken = lcurly.getNextSibling();
rcurly = lcurly.getLastChild();

if (nextToken == null) {
shouldCheckLastRcurly = true;
nextToken = getNextToken(ast);
}
break;
case TokenTypes.LITERAL_CATCH:
nextToken = ast.getNextSibling();
Expand Down Expand Up @@ -386,9 +401,16 @@ private static Details getDetails(DetailAST ast) {
rcurly = lcurly.getLastChild();
}
break;
case TokenTypes.LAMBDA:
lcurly = ast.findFirstToken(TokenTypes.SLIST);
nextToken = getNextToken(ast);
if (lcurly != null) {
rcurly = lcurly.getLastChild();
}
break;
default:
// ATTENTION! We have default here, but we expect case TokenTypes.METHOD_DEF,
// TokenTypes.LITERAL_FOR, TokenTypes.LITERAL_WHILE, only.
// TokenTypes.LITERAL_FOR, TokenTypes.LITERAL_WHILE only.
// It has been done to improve coverage to 100%. I couldn't replace it with
// if-else-if block because code was ugly and didn't pass pmd check.

Expand Down Expand Up @@ -444,6 +466,21 @@ private static DetailAST getNextToken(DetailAST ast) {
return CheckUtils.getFirstNode(next);
}

/**
* Checks that before a right curly should be a linebreak.
* @param bracePolicy options for placing the right curly brace.
* @param details Details for validation
* @return true if before a right curly should be a linebreak.
*/
private static boolean shouldHaveBeLineBreakBefore(RightCurlyOption bracePolicy,
Details details) {
final DetailAST rcurly = details.rcurly;
final DetailAST lcurly = details.lcurly;
return bracePolicy == RightCurlyOption.SAME
&& !hasLineBreakBefore(rcurly)
&& lcurly.getLineNo() != rcurly.getLineNo();
}

/**
* Checks if right curly has line break before.
* @param rightCurly right curly token.
Expand Down
2 changes: 1 addition & 1 deletion src/main/resources/google_checks.xml
Expand Up @@ -61,7 +61,7 @@
<module name="RightCurly">
<property name="id" value="RightCurlyAlone"/>
<property name="option" value="alone"/>
<property name="tokens" value="CLASS_DEF, METHOD_DEF, CTOR_DEF, LITERAL_FOR, LITERAL_WHILE, STATIC_INIT, INSTANCE_INIT"/>
<property name="tokens" value="CLASS_DEF, METHOD_DEF, CTOR_DEF, LITERAL_FOR, LITERAL_WHILE, STATIC_INIT, INSTANCE_INIT, LAMBDA"/>
</module>
<module name="WhitespaceAround">
<property name="allowEmptyConstructors" value="true"/>
Expand Down
Expand Up @@ -259,6 +259,18 @@ public void testAloneOrSingleLine() throws Exception {
verify(checkConfig, getPath("InputRightCurlyAloneOrSingleline.java"), expected);
}

@Test
public void testRightCurlyIsAloneInLambda() throws Exception {
checkConfig.addAttribute("option", RightCurlyOption.ALONE.toString());
checkConfig.addAttribute("tokens", "LAMBDA");
final String[] expected = {
"14:74: " + getCheckMessage(MSG_KEY_LINE_ALONE, "}", 74),
"17:49: " + getCheckMessage(MSG_KEY_LINE_ALONE, "}", 49),
"24:33: " + getCheckMessage(MSG_KEY_LINE_ALONE, "}", 33),
};
verify(checkConfig, getPath("InputRightCurlyAloneInLambda.java"), expected);
}

@Test
public void testCatchWithoutFinally() throws Exception {
final String[] expected = {
Expand Down Expand Up @@ -305,33 +317,4 @@ public void testRightCurlySameAndLiteralDo() throws Exception {
};
verify(checkConfig, getPath("InputRightCurlyDoWhile.java"), expected);
}

@Test
public void testTryWithResourceSame() throws Exception {
checkConfig.addAttribute("option", RightCurlyOption.SAME.toString());
final String[] expected = {
"11:9: " + getCheckMessage(MSG_KEY_LINE_SAME, "}", 9),
"24:67: " + getCheckMessage(MSG_KEY_LINE_SAME, "}", 67),
};
verify(checkConfig, getPath("InputRightCurlyTryResource.java"), expected);
}

@Test
public void testTryWithResourceAlone() throws Exception {
checkConfig.addAttribute("option", RightCurlyOption.ALONE.toString());
final String[] expected = {
"19:9: " + getCheckMessage(MSG_KEY_LINE_ALONE, "}", 9),
"24:67: " + getCheckMessage(MSG_KEY_LINE_ALONE, "}", 67),
};
verify(checkConfig, getPath("InputRightCurlyTryResource.java"), expected);
}

@Test
public void testTryWithResourceAloneSingle() throws Exception {
checkConfig.addAttribute("option", RightCurlyOption.ALONE_OR_SINGLELINE.toString());
final String[] expected = {
"19:9: " + getCheckMessage(MSG_KEY_LINE_ALONE, "}", 9),
};
verify(checkConfig, getPath("InputRightCurlyTryResource.java"), expected);
}
}
@@ -0,0 +1,32 @@
package com.puppycrawl.tools.checkstyle.checks.blocks;

/*
This is test class for token LAMBDA.
*/
public class InputRightCurlyAloneInLambda {

static Runnable r1 = () -> {
String.valueOf("Test rightCurly one!");
};

static Runnable r2 = () -> String.valueOf("Test rightCurly two!");

static Runnable r3 = () -> {String.valueOf("Test rightCurly three!");}; //violation

static Runnable r4 = () -> {
String.valueOf("Test rightCurly four!");}; //violation

static Runnable r5 = () ->
{
String.valueOf("Test rightCurly five!");
};

static Runnable r6 = () -> {}; //violation

static Runnable r7 = () -> {
};

static Runnable r8 = () ->
{
};
}
8 changes: 6 additions & 2 deletions src/xdocs/config_blocks.xml
Expand Up @@ -799,7 +799,9 @@ for(int i = 0; i &lt; 10; value.incrementValue()); // OK
<a
href="apidocs/com/puppycrawl/tools/checkstyle/api/TokenTypes.html#STATIC_INIT">STATIC_INIT</a>,
<a
href="apidocs/com/puppycrawl/tools/checkstyle/api/TokenTypes.html#INSTANCE_INIT">INSTANCE_INIT</a>.</td>
href="apidocs/com/puppycrawl/tools/checkstyle/api/TokenTypes.html#INSTANCE_INIT">INSTANCE_INIT</a>,
<a
href="apidocs/com/puppycrawl/tools/checkstyle/api/TokenTypes.html#LAMBDA">LAMBDA</a>.</td>

<td><a
href="apidocs/com/puppycrawl/tools/checkstyle/api/TokenTypes.html#LITERAL_TRY">LITERAL_TRY</a>,
Expand All @@ -810,7 +812,9 @@ for(int i = 0; i &lt; 10; value.incrementValue()); // OK
<a
href="apidocs/com/puppycrawl/tools/checkstyle/api/TokenTypes.html#LITERAL_IF">LITERAL_IF</a>,
<a
href="apidocs/com/puppycrawl/tools/checkstyle/api/TokenTypes.html#LITERAL_ELSE">LITERAL_ELSE</a>.</td>
href="apidocs/com/puppycrawl/tools/checkstyle/api/TokenTypes.html#LITERAL_ELSE">LITERAL_ELSE</a>,
<a
href="apidocs/com/puppycrawl/tools/checkstyle/api/TokenTypes.html#LAMBDA">LAMBDA</a>.</td>
</tr>
</table>
</subsection>
Expand Down

0 comments on commit 1925eb2

Please sign in to comment.