Skip to content

Commit

Permalink
Issue checkstyle#6466: do not complain in same-mode about constructs …
Browse files Browse the repository at this point in the history 
…that are not multi-part
  • Loading branch information
@Vampire
Vampire committed Mar 22, 2019
1 parent eb55ccf commit f278b75
Show file tree
Hide file tree
Showing 5 changed files with 136 additions and 45 deletions.
53 changes: 30 additions & 23 deletions src/main/java/com/puppycrawl/tools/checkstyle/checks/blocks/RightCurlyCheck.java
View file
Expand Up @@ -222,10 +222,9 @@ private static boolean shouldBeAloneOnLine(RightCurlyOption bracePolicy,
String targetSrcLine) {
return bracePolicy == RightCurlyOption.ALONE
&& shouldBeAloneOnLineWithAloneOption(details, targetSrcLine)
|| bracePolicy == RightCurlyOption.ALONE_OR_SINGLELINE
&& shouldBeAloneOnLineWithAloneOrSinglelineOption(details, targetSrcLine)
|| details.shouldCheckLastRcurly
&& details.rcurly.getLineNo() == details.nextToken.getLineNo();
|| (bracePolicy == RightCurlyOption.ALONE_OR_SINGLELINE
|| details.shouldCheckLastRcurly)
&& shouldBeAloneOnLineWithNotAloneOption(details, targetSrcLine);
}

/**
Expand All @@ -237,21 +236,20 @@ && shouldBeAloneOnLineWithAloneOrSinglelineOption(details, targetSrcLine)
private static boolean shouldBeAloneOnLineWithAloneOption(Details details,
String targetSrcLine) {
return !isAloneOnLine(details, targetSrcLine)
&& !isEmptyBody(details.lcurly);
&& (hasLineBreakBefore(details.rcurly) || !isEmptyBody(details.lcurly));
}

/**
* Whether right curly should be alone on line when ALONE_OR_SINGLELINE option is used.
* Whether right curly should be alone on line when ALONE_OR_SINGLELINE or SAME option is used.
* @param details details for validation.
* @param targetSrcLine A string with contents of rcurly's line
* @return true, if right curly should be alone on line
* when ALONE_OR_SINGLELINE option is used.
* when ALONE_OR_SINGLELINE or SAME option is used.
*/
private static boolean shouldBeAloneOnLineWithAloneOrSinglelineOption(Details details,
String targetSrcLine) {
return !isAloneOnLine(details, targetSrcLine)
&& !isSingleLineBlock(details)
&& !isEmptyBody(details.lcurly);
private static boolean shouldBeAloneOnLineWithNotAloneOption(Details details,
String targetSrcLine) {
return shouldBeAloneOnLineWithAloneOption(details, targetSrcLine)
&& !isSingleLineBlock(details);
}

/**
Expand Down Expand Up @@ -339,9 +337,11 @@ else if (lcurly.getFirstChild().getType() == TokenTypes.RCURLY) {
* @return true, if right curly has line break before.
*/
private static boolean hasLineBreakBefore(DetailAST rightCurly) {
final DetailAST previousToken = rightCurly.getPreviousSibling();
return previousToken == null
|| rightCurly.getLineNo() != previousToken.getLineNo();
DetailAST previousToken = rightCurly.getPreviousSibling();
if (previousToken == null) {
previousToken = rightCurly.getParent();
}
return rightCurly.getLineNo() != previousToken.getLineNo();
}

/**
Expand Down Expand Up @@ -408,8 +408,7 @@ private static Details getDetails(DetailAST ast) {
* @return object containing all details to make a validation
*/
private static Details getDetailsForTryCatchFinally(DetailAST ast) {
boolean shouldCheckLastRcurly = false;
final DetailAST rcurly;
final boolean shouldCheckLastRcurly;
final DetailAST lcurly;
DetailAST nextToken;
final int tokenType = ast.getType();
Expand All @@ -421,28 +420,32 @@ private static Details getDetailsForTryCatchFinally(DetailAST ast) {
lcurly = ast.getFirstChild();
}
nextToken = lcurly.getNextSibling();
rcurly = lcurly.getLastChild();

if (nextToken == null) {
shouldCheckLastRcurly = true;
nextToken = getNextToken(ast);
}
else {
shouldCheckLastRcurly = false;
}
}
else if (tokenType == TokenTypes.LITERAL_CATCH) {
nextToken = ast.getNextSibling();
lcurly = ast.getLastChild();
rcurly = lcurly.getLastChild();
if (nextToken == null) {
shouldCheckLastRcurly = true;
nextToken = getNextToken(ast);
}
else {
shouldCheckLastRcurly = false;
}
}
else {
shouldCheckLastRcurly = true;
nextToken = getNextToken(ast);
lcurly = ast.getFirstChild();
rcurly = lcurly.getLastChild();
}
final DetailAST rcurly = lcurly.getLastChild();
return new Details(lcurly, rcurly, nextToken, shouldCheckLastRcurly);
}

Expand All @@ -452,7 +455,7 @@ else if (tokenType == TokenTypes.LITERAL_CATCH) {
* @return object containing all details to make a validation
*/
private static Details getDetailsForIfElse(DetailAST ast) {
boolean shouldCheckLastRcurly = false;
final boolean shouldCheckLastRcurly;
final DetailAST lcurly;
DetailAST nextToken;
final int tokenType = ast.getType();
Expand All @@ -464,6 +467,7 @@ private static Details getDetailsForIfElse(DetailAST ast) {
lcurly = ast.getLastChild();
}
else {
shouldCheckLastRcurly = false;
lcurly = nextToken.getPreviousSibling();
}
}
Expand Down Expand Up @@ -501,7 +505,7 @@ private static Details getDetailsForOthers(DetailAST ast) {
rcurly = lcurly.getLastChild();
}
}
return new Details(lcurly, rcurly, getNextToken(ast), false);
return new Details(lcurly, rcurly, getNextToken(ast), true);
}

/**
Expand All @@ -514,22 +518,25 @@ private static Details getDetailsForLoops(DetailAST ast) {
final DetailAST lcurly;
final DetailAST nextToken;
final int tokenType = ast.getType();
final boolean shouldCheckLastRcurly;
if (tokenType == TokenTypes.LITERAL_DO) {
shouldCheckLastRcurly = false;
nextToken = ast.findFirstToken(TokenTypes.DO_WHILE);
lcurly = ast.findFirstToken(TokenTypes.SLIST);
if (lcurly != null) {
rcurly = lcurly.getLastChild();
}
}
else {
shouldCheckLastRcurly = true;
lcurly = ast.findFirstToken(TokenTypes.SLIST);
if (lcurly != null) {
// SLIST could be absent in code like "while(true);"
rcurly = lcurly.getLastChild();
}
nextToken = getNextToken(ast);
}
return new Details(lcurly, rcurly, nextToken, false);
return new Details(lcurly, rcurly, nextToken, shouldCheckLastRcurly);
}

/**
Expand Down
53 changes: 31 additions & 22 deletions src/test/java/com/puppycrawl/tools/checkstyle/checks/blocks/RightCurlyCheckTest.java
View file
Expand Up @@ -66,12 +66,16 @@ public void testDefault() throws Exception {
public void testSame() throws Exception {
final DefaultConfiguration checkConfig = createModuleConfig(RightCurlyCheck.class);
checkConfig.addAttribute("option", RightCurlyOption.SAME.toString());
checkConfig.addAttribute("tokens", "LITERAL_TRY, LITERAL_CATCH, LITERAL_FINALLY, "
+ "LITERAL_IF, LITERAL_ELSE, LITERAL_FOR, LITERAL_WHILE, LITERAL_DO");
final String[] expected = {
"25:17: " + getCheckMessage(MSG_KEY_LINE_SAME, "}", 17),
"28:17: " + getCheckMessage(MSG_KEY_LINE_SAME, "}", 17),
"40:13: " + getCheckMessage(MSG_KEY_LINE_SAME, "}", 13),
"44:13: " + getCheckMessage(MSG_KEY_LINE_SAME, "}", 13),
"93:27: " + getCheckMessage(MSG_KEY_LINE_BREAK_BEFORE, "}", 27),
"188:9: " + getCheckMessage(MSG_KEY_LINE_ALONE, "}", 9),
"189:53: " + getCheckMessage(MSG_KEY_LINE_ALONE, "}", 53),
};
verify(checkConfig, getPath("InputRightCurlyLeft.java"), expected);
}
Expand All @@ -84,13 +88,33 @@ public void testSameOmitOneLiners() throws Exception {
verify(checkConfig, getPath("InputRightCurlyNameForOneLiners.java"), expected);
}

@Test
public void testSameDoesNotComplainForNonMultilineConstructs() throws Exception {
final DefaultConfiguration checkConfig = createModuleConfig(RightCurlyCheck.class);
checkConfig.addAttribute("option", RightCurlyOption.SAME.toString());
checkConfig.addAttribute("tokens", "LITERAL_DO, LITERAL_FOR, LITERAL_WHILE, STATIC_INIT,"
+ "INSTANCE_INIT, CLASS_DEF, METHOD_DEF, CTOR_DEF");
final String[] expected = CommonUtil.EMPTY_STRING_ARRAY;
verify(checkConfig, getPath("InputRightCurlySame.java"), expected);
}

@Test
public void testAlone() throws Exception {
final DefaultConfiguration checkConfig = createModuleConfig(RightCurlyCheck.class);
checkConfig.addAttribute("option", RightCurlyOption.ALONE.toString());
checkConfig.addAttribute("tokens", "LITERAL_TRY, LITERAL_CATCH, LITERAL_FINALLY, "
+ "LITERAL_IF, LITERAL_ELSE, LITERAL_FOR, LITERAL_WHILE, LITERAL_DO");
final String[] expected = {
"56:13: " + getCheckMessage(MSG_KEY_LINE_ALONE, "}", 13),
"93:27: " + getCheckMessage(MSG_KEY_LINE_ALONE, "}", 27),
"97:72: " + getCheckMessage(MSG_KEY_LINE_ALONE, "}", 72),
"173:9: " + getCheckMessage(MSG_KEY_LINE_ALONE, "}", 9),
"175:9: " + getCheckMessage(MSG_KEY_LINE_ALONE, "}", 9),
"177:9: " + getCheckMessage(MSG_KEY_LINE_ALONE, "}", 9),
"178:9: " + getCheckMessage(MSG_KEY_LINE_ALONE, "}", 9),
"183:9: " + getCheckMessage(MSG_KEY_LINE_ALONE, "}", 9),
"188:9: " + getCheckMessage(MSG_KEY_LINE_ALONE, "}", 9),
"189:53: " + getCheckMessage(MSG_KEY_LINE_ALONE, "}", 53),
};
verify(checkConfig, getPath("InputRightCurlyLeft.java"), expected);
}
Expand All @@ -111,17 +135,6 @@ public void testNewLine() throws Exception {
verify(checkConfig, getPath("InputRightCurlyLeft.java"), expected);
}

@Test
public void testShouldStartLine() throws Exception {
final DefaultConfiguration checkConfig = createModuleConfig(RightCurlyCheck.class);
checkConfig.addAttribute("option", RightCurlyOption.ALONE.toString());
final String[] expected = {
"93:27: " + getCheckMessage(MSG_KEY_LINE_ALONE, "}", 27),
"97:72: " + getCheckMessage(MSG_KEY_LINE_ALONE, "}", 72),
};
verify(checkConfig, getPath("InputRightCurlyLeft.java"), expected);
}

@Test
public void testShouldStartLine2() throws Exception {
final DefaultConfiguration checkConfig = createModuleConfig(RightCurlyCheck.class);
Expand All @@ -137,17 +150,6 @@ public void testShouldStartLine2() throws Exception {
verify(checkConfig, getPath("InputRightCurlyLeft.java"), expected);
}

@Test
public void testMethodCtorNamedClassClosingBrace() throws Exception {
final DefaultConfiguration checkConfig = createModuleConfig(RightCurlyCheck.class);
checkConfig.addAttribute("option", RightCurlyOption.ALONE.toString());
final String[] expected = {
"93:27: " + getCheckMessage(MSG_KEY_LINE_ALONE, "}", 27),
"97:72: " + getCheckMessage(MSG_KEY_LINE_ALONE, "}", 72),
};
verify(checkConfig, getPath("InputRightCurlyLeft.java"), expected);
}

@Test
public void testForceLineBreakBefore() throws Exception {
final DefaultConfiguration checkConfig = createModuleConfig(RightCurlyCheck.class);
Expand Down Expand Up @@ -280,6 +282,13 @@ public void testAloneOrSingleLine() throws Exception {
"182:24: " + getCheckMessage(MSG_KEY_LINE_ALONE, "}", 24),
"185:24: " + getCheckMessage(MSG_KEY_LINE_ALONE, "}", 24),
"188:24: " + getCheckMessage(MSG_KEY_LINE_ALONE, "}", 24),
"194:9: " + getCheckMessage(MSG_KEY_LINE_ALONE, "}", 9),
"196:9: " + getCheckMessage(MSG_KEY_LINE_ALONE, "}", 9),
"198:9: " + getCheckMessage(MSG_KEY_LINE_ALONE, "}", 9),
"199:9: " + getCheckMessage(MSG_KEY_LINE_ALONE, "}", 9),
"204:9: " + getCheckMessage(MSG_KEY_LINE_ALONE, "}", 9),
"209:9: " + getCheckMessage(MSG_KEY_LINE_ALONE, "}", 9),
"210:53: " + getCheckMessage(MSG_KEY_LINE_ALONE, "}", 53),
};
verify(checkConfig, getPath("InputRightCurlyAloneOrSingleline.java"), expected);
}
Expand Down
22 changes: 22 additions & 0 deletions ...uppycrawl/tools/checkstyle/checks/blocks/rightcurly/InputRightCurlyAloneOrSingleline.java
View file
Expand Up @@ -187,4 +187,26 @@ void foo30() {
while (true) {
getClass();} // violation
}

public void emptyBlocks() {
try {
// ignore
} catch (RuntimeException e) { // violation
new Object();
} catch (Exception e) { // violation
// ignore
} catch (Throwable e) { // violation
} finally { // violation
// ignore
}

do {
} while (true); // violation
}

public void codeAfterLastRightCurly() {
while (new Object().equals(new Object())) {
}; // violation
for (int i = 0; i < 1; i++) { new Object(); }; // violation
}
}
21 changes: 21 additions & 0 deletions ...sources/com/puppycrawl/tools/checkstyle/checks/blocks/rightcurly/InputRightCurlyLeft.java
View file
Expand Up @@ -167,4 +167,25 @@ static class Inner
}
}

public void emptyBlocks() {
try {
// ignore
} catch (RuntimeException e) { // violation except for SAME
new Object();
} catch (Exception e) { // violation except for SAME
// ignore
} catch (Throwable e) { // violation except for SAME
} finally { // violation except for SAME
// ignore
}

do {
} while (true); // violation except for SAME
}

public void codeAfterLastRightCurly() {
while (new Object().equals(new Object())) {
}; // violation
for (int i = 0; i < 1; i++) { new Object(); }; // violation
}
}
32 changes: 32 additions & 0 deletions ...sources/com/puppycrawl/tools/checkstyle/checks/blocks/rightcurly/InputRightCurlySame.java
View file
@@ -0,0 +1,32 @@
package com.puppycrawl.tools.checkstyle.checks.blocks.rightcurly;

public class InputRightCurlySame {
static {
}

public InputRightCurlySame() {
Thread t = new Thread(new Runnable() {
{
}

@Override
public void run() {
}
});
}

public void doLoop() {
do {
} while (true);
}

public void whileLoop() {
while (true) {
}
}

public void forLoop() {
for (; ; ) {
}
}
}

0 comments on commit f278b75

Please sign in to comment.