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 9, 2017
1 parent 460c185 commit b8dadd6
Show file tree
Hide file tree
Showing 14 changed files with 365 additions and 12 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,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 +76,59 @@ 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()) {
log(ast, MSG_KEY);
if (skipIfLastAndSharedWithCase) {
if (findNextSibling(ast, TokenTypes.LITERAL_CASE) != -1) {
log(ast, MSG_KEY_SKIP_IF_LAST_AND_SHARED_WITH_CASE);
}
else {
if (ast.getPreviousSibling() == null
&& findNextSibling(ast.getParent(), TokenTypes.CASE_GROUP) != -1) {
log(ast, MSG_KEY);
}
}
}
else {
if (findNextSibling(ast.getParent(), TokenTypes.CASE_GROUP) != -1) {
log(ast, MSG_KEY);
}
}
}
}

/**
* Determines if token type is CASE_GROUP, default is in last case group or
* if token type is LITERAL_CASE, default is the last in that group.
*
* @param ast root node.
* @param tokenType tokentype to be processed.
* @return true if condition is satisfied or false
*/
private static int findNextSibling(DetailAST ast, int tokenType) {
int token = -1;
DetailAST node = ast.getNextSibling();
while (node != null) {
if (node.getType() == tokenType) {
token = node.getType();
break;
}
node = node.getNextSibling();
}
return token;
}
}
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Loading

0 comments on commit b8dadd6

Please sign in to comment.