Skip to content

Commit

Permalink
Issue checkstyle#4078: DefaultComesLast only if doesnt share case
Browse files Browse the repository at this point in the history
  • Loading branch information
sagarshah94 committed Apr 12, 2017
1 parent 460c185 commit 3644fe6
Show file tree
Hide file tree
Showing 14 changed files with 363 additions and 11 deletions.
Expand Up @@ -19,6 +19,8 @@

package com.puppycrawl.tools.checkstyle.checks.coding;

import java.util.Objects;

import com.puppycrawl.tools.checkstyle.api.AbstractCheck;
import com.puppycrawl.tools.checkstyle.api.DetailAST;
import com.puppycrawl.tools.checkstyle.api.TokenTypes;
Expand Down Expand Up @@ -49,6 +51,16 @@ public class DefaultComesLastCheck extends AbstractCheck {
*/
public static final String MSG_KEY = "default.comes.last";

/**
* A key is pointing to the warning message text in "messages.properties"
* file.
*/
public static final String MSG_KEY_SKIP_IF_LAST_AND_SHARED_WITH_CASE =
"default.comes.last.in.casegroup";

/** Whether to process skipIfLastAndSharedWithCaseInSwitch() invocations. */
private boolean skipIfLastAndSharedWithCase;

@Override
public int[] getAcceptableTokens() {
return new int[] {
Expand All @@ -66,22 +78,56 @@ public int[] getRequiredTokens() {
return getAcceptableTokens();
}

/**
* Whether to allow default keyword not in last but surrounded with case.
* @param newValue whether to ignore checking.
*/
public void setSkipIfLastAndSharedWithCase(boolean newValue) {
skipIfLastAndSharedWithCase = newValue;
}

@Override
public void visitToken(DetailAST ast) {
final DetailAST defaultGroupAST = ast.getParent();
//default keywords used in annotations too - not what we're
//interested in
if (defaultGroupAST.getType() != TokenTypes.ANNOTATION_FIELD_DEF
&& defaultGroupAST.getType() != TokenTypes.MODIFIERS) {
final DetailAST switchAST = defaultGroupAST.getParent();
final DetailAST lastGroupAST =
switchAST.getLastChild().getPreviousSibling();

if (defaultGroupAST.getLineNo() != lastGroupAST.getLineNo()
|| defaultGroupAST.getColumnNo()
!= lastGroupAST.getColumnNo()) {
if (skipIfLastAndSharedWithCase) {
if (Objects.nonNull(findNextSibling(ast, TokenTypes.LITERAL_CASE))) {
log(ast, MSG_KEY_SKIP_IF_LAST_AND_SHARED_WITH_CASE);
}
else if (ast.getPreviousSibling() == null
&& Objects.nonNull(findNextSibling(defaultGroupAST,
TokenTypes.CASE_GROUP))) {
log(ast, MSG_KEY);
}
}
else if (Objects.nonNull(findNextSibling(defaultGroupAST,
TokenTypes.CASE_GROUP))) {
log(ast, MSG_KEY);
}
}
}

/**
* Return token type only if passed tokenType in argument is found or returns -1.
*
* @param ast root node.
* @param tokenType tokentype to be processed.
* @return token id if desired token is not found then -1.
*/
private static Object findNextSibling(DetailAST ast, int tokenType) {
Object token = null;
DetailAST node = ast.getNextSibling();
while (node != null) {
if (node.getType() == tokenType) {
token = node;
break;
}
node = node.getNextSibling();
}
return token;
}
}
Expand Up @@ -8,6 +8,7 @@ declaration.order.static=Static variable definition in wrong order.
declaration.order.instance=Instance variable definition in wrong order.
declaration.order.access=Variable access definition in wrong order.
default.comes.last=Default should be last label in the switch.
default.comes.last.in.casegroup=Default should be last label in the case group.
empty.statement=Empty statement.
equals.avoid.null=String literal expressions should be on the left side of an equals comparison.
equalsIgnoreCase.avoid.null=String literal expressions should be on the left side of an equalsIgnoreCase comparison.
Expand Down
Expand Up @@ -8,6 +8,7 @@ declaration.order.static=Die statische Variable wird an der falschen Stelle dekl
declaration.order.instance=Die Instanz-Variable wird an der falschen Stelle deklariert.
declaration.order.access=Fehlerhafte Deklarationsreihenfolge für diesen Scope.
default.comes.last=Der Default-Fall sollte der letzte Fall der switch-Anweisung sein.
default.comes.last.in.casegroup=Default sollte letztes Label in der Fallgruppe sein.
empty.statement=Leere Anweisung.
equals.avoid.null=Bei einem equals()-Vergleich sollten String-Literale auf der linken Seite stehen.
equalsIgnoreCase.avoid.null=Bei einem equalsIgnoreCase()-Vergleich sollten String-Literale auf der linken Seite stehen.
Expand Down
Expand Up @@ -55,6 +55,7 @@ modified.control.variable=Se modifica la variable de control ''{0}''.

explicit.init=La variable ''{0}'' se inicializa explicitamente a ''{1}'' (valor por defecto para su tipo).
default.comes.last=La etiqueta default debe ser la última etiqueta en el switch.
default.comes.last.in.casegroup=El valor predeterminado debe ser la última etiqueta en el grupo de casos.
missing.ctor=La clase debería definir un constructor.
fall.through=Caída desde la etiqueta anterior en la sentencia switch.
require.this.variable=La referencia a la variable de instancia ''{0}'' necesita \"{1}this.\".
Expand Down
Expand Up @@ -28,6 +28,7 @@ declaration.order.static = Staattinen muuttuja määritelmä väärässä järje
declaration.order.instance = Esimerkiksi muuttuja määritelmä väärässä järjestyksessä.
declaration.order.access = Muuttuja pääsy määritelmä väärässä järjestyksessä.
default.comes.last = Oletus pitäisi olla viimeinen tarra kytkin.
default.comes.last.in.casegroup=Oletus on oltava viimeinen tarra tapauksessa ryhmässä.
empty.statement = Tyhjä lausunto.
equals.avoid.null = String kirjaimellinen ilmaisuja pitäisi olla vasemmalla puolella vastaa vertailun.
equalsIgnoreCase.avoid.null = String kirjaimellinen ilmaisuja pitäisi olla vasemmalla puolella equalsIgnoreCase vertailun.
Expand Down
Expand Up @@ -56,6 +56,7 @@ modified.control.variable=La variable de contrôle ''{0}'' est modifiée.
explicit.init=L''initialisation explicite de la variable ''{0}'' à la valeur ''{1}'' est inutile, c''est la valeur par défaut pour ce type.
default.comes.last=Le cas \"default\" devrait apparaitre en dernier dans le bloc \"switch\".
default.comes.last.in.casegroup=La valeur par défaut doit être la dernière étiquette dans le groupe de cas.
missing.ctor=Il manque un constructeur à la classe.
fall.through=Le cas précédent du \"switch\" ne contient pas de break, return, throw ou continue.
fall.through.last=Le dernier cas du \"switch\" ne contient pas de break, return, throw ou continue.
Expand Down
Expand Up @@ -52,6 +52,7 @@ explicit.init=変数 ''{0}'' が明示的に ''{1}'' (この型のデフォル
avoid.finalizer.method = ファイナライザメソッドを使用しないでください。
avoid.clone.method = cloneメソッドを使用しないでください。
default.comes.last = default は switch 文の最後のラベルにしてください。
default.comes.last.in.casegroup=デフォルトは、ケースグループの最後のラベルにする必要があります。
equals.avoid.null = equals による比較では、文字列リテラル式を左側にしてください。
equalsIgnoreCase.avoid.null = equals による比較では、文字列リテラル式を左側にしてください。
fall.through = switch 文の前の分岐からフォールスルーしています。
Expand Down
Expand Up @@ -32,6 +32,7 @@ string.literal.equality=\"Strings\" literais devem ser comparadas com equals(),
avoid.finalizer.method = Evite o uso de método finalizador.
avoid.clone.method = Evite o uso de método clone.
default.comes.last = Padrão deve ser a última etiqueta no interruptor.
default.comes.last.in.casegroup=O padrão deve ser o último marcador no grupo de casos.
equals.avoid.null = Corda expressões literais deve estar no lado esquerdo de um é igual comparação.
equalsIgnoreCase.avoid.null = Corda expressões literais deve estar no lado esquerdo de uma comparação equalsIgnoreCase.
fall.through = Queda através do ramo anterior da instrução switch.
Expand Down
Expand Up @@ -15,6 +15,7 @@ declaration.order.instance = Değişken tanımı yanlış sırada yapılmış
declaration.order.static = ''static'' değişken tanımı yanlış sırada yapılmış.

default.comes.last = ''switch'' içerisindeki ''default'' ifadesi son durum olarak yer almalıdır.
default.comes.last.in.casegroup=Varsayılan değer, vaka grubundaki son etiket olmalıdır.

empty.statement = Boş ifade.

Expand Down
Expand Up @@ -8,6 +8,7 @@ declaration.order.static=静态属性定义顺序错误。
declaration.order.instance=实例属性定义顺序错误。
declaration.order.access=属性访问器定义顺序错误。
default.comes.last=default 应为 switch 块最后一个元素。
default.comes.last.in.casegroup=默认值应为案例组中的最后一个标签。
empty.statement=避免空行。
equals.avoid.null=字符串常量应出现在 equals 比较的左侧。
equalsIgnoreCase.avoid.null=字符串常量应出现在 equalsIgnoreCase 比较的左侧。
Expand Down
Expand Up @@ -20,6 +20,7 @@
package com.puppycrawl.tools.checkstyle.checks.coding;

import static com.puppycrawl.tools.checkstyle.checks.coding.DefaultComesLastCheck.MSG_KEY;
import static com.puppycrawl.tools.checkstyle.checks.coding.DefaultComesLastCheck.MSG_KEY_SKIP_IF_LAST_AND_SHARED_WITH_CASE;

import java.io.File;
import java.io.IOException;
Expand All @@ -32,6 +33,9 @@
import com.puppycrawl.tools.checkstyle.utils.CommonUtils;

public class DefaultComesLastCheckTest extends BaseCheckTestSupport {

private DefaultConfiguration checkConfig;

@Override
protected String getPath(String filename) throws IOException {
return super.getPath("checks" + File.separator
Expand All @@ -44,13 +48,42 @@ protected String getNonCompilablePath(String filename) throws IOException {
+ "coding" + File.separator + filename);
}

@Test
public void testSkipIfLastAndSharedWithCase() throws Exception {
checkConfig = createCheckConfig(DefaultComesLastCheck.class);
checkConfig.addAttribute("skipIfLastAndSharedWithCase", "true");
final String[] expected = {
"17:13: " + getCheckMessage(MSG_KEY_SKIP_IF_LAST_AND_SHARED_WITH_CASE),
"25:13: " + getCheckMessage(MSG_KEY_SKIP_IF_LAST_AND_SHARED_WITH_CASE),
"33:21: " + getCheckMessage(MSG_KEY_SKIP_IF_LAST_AND_SHARED_WITH_CASE),
"37:13: " + getCheckMessage(MSG_KEY_SKIP_IF_LAST_AND_SHARED_WITH_CASE),
"57:13: " + getCheckMessage(MSG_KEY_SKIP_IF_LAST_AND_SHARED_WITH_CASE),
"77:13: " + getCheckMessage(MSG_KEY_SKIP_IF_LAST_AND_SHARED_WITH_CASE),
"89:13: " + getCheckMessage(MSG_KEY_SKIP_IF_LAST_AND_SHARED_WITH_CASE),
"98:13: " + getCheckMessage(MSG_KEY),
};

verify(checkConfig, getPath("InputSkipIfLastAndSharedWithCase.java"), expected);
}

@Test
public void testIt() throws Exception {
final DefaultConfiguration checkConfig =
createCheckConfig(DefaultComesLastCheck.class);
checkConfig = createCheckConfig(DefaultComesLastCheck.class);
final String[] expected = {
"25:9: " + getCheckMessage(MSG_KEY),
"32:24: " + getCheckMessage(MSG_KEY),
"37:13: " + getCheckMessage(MSG_KEY),
"45:13: " + getCheckMessage(MSG_KEY),
"53:13: " + getCheckMessage(MSG_KEY),
"61:21: " + getCheckMessage(MSG_KEY),
"65:13: " + getCheckMessage(MSG_KEY),
"69:21: " + getCheckMessage(MSG_KEY),
"74:13: " + getCheckMessage(MSG_KEY),
"85:13: " + getCheckMessage(MSG_KEY),
"96:13: " + getCheckMessage(MSG_KEY),
"106:13: " + getCheckMessage(MSG_KEY),
"114:13: " + getCheckMessage(MSG_KEY),
"125:13: " + getCheckMessage(MSG_KEY),
};
verify(checkConfig,
getPath("InputDefaultComesLast.java"),
Expand All @@ -60,8 +93,7 @@ public void testIt() throws Exception {
@Test
public void testDefaultMethodsInJava8()
throws Exception {
final DefaultConfiguration checkConfig =
createCheckConfig(DefaultComesLastCheck.class);
checkConfig = createCheckConfig(DefaultComesLastCheck.class);
final String[] expected = CommonUtils.EMPTY_STRING_ARRAY;
verify(checkConfig,
getPath("InputDefaultComesLast2.java"),
Expand Down
Expand Up @@ -31,6 +31,104 @@ void method(int i) {
switch (i) {
case 1: break; default: break; case 2: break;
}

switch (i) {
case 1:
default: //violation
break;
case 2:
break;
}

switch (i) {
case 1:
default: //violation
case 2:
break;
case 3:
break;
}

switch (i) {
default: //violation
case 1:
break;
case 2:
break;
}

switch (i) {
case 0: default: case 1: break; case 2: break; //violation
}

switch (i) {
default: case 1: break; case 2: break; //violation
}

switch (i) {
case 1: default: break; case 2: break; //violation
}

switch (i) {
case 1:
default: //violation
break;
case 2:
break;
case 3:
break;
}

switch (i) {
case 1:
break;
default: //violation
case 2:
break;
case 3:
break;
}

switch (i) {
case 1:
break;
case 2:
default: //violation
break;
case 3:
break;
}

switch (i) {
case 1:
break;
case 2:
default: //violation
case 3:
break;
case 4:
break;
}

switch (i) {
default: //violation
break;
case 1:
break;
}

switch (i) {
case 1:
break;
case 2:
break;
default: //violation
case 5:
case 6:
break;
case 7:
break;
}
}
}

Expand Down

0 comments on commit 3644fe6

Please sign in to comment.