From b1eced12b2be801d2f68de4e1f404e1c04b10ae8 Mon Sep 17 00:00:00 2001 From: rnveach Date: Mon, 1 Aug 2016 16:43:50 -0400 Subject: [PATCH] Issue #3322: added RedundantModifiers for final in abstract methods --- .../modifier/RedundantModifierCheck.java | 95 +++++++++---------- .../modifier/RedundantModifierCheckTest.java | 13 +++ .../modifier/InputFinalInAbstractMethods.java | 26 +++++ 3 files changed, 85 insertions(+), 49 deletions(-) create mode 100644 src/test/resources/com/puppycrawl/tools/checkstyle/checks/modifier/InputFinalInAbstractMethods.java diff --git a/src/main/java/com/puppycrawl/tools/checkstyle/checks/modifier/RedundantModifierCheck.java b/src/main/java/com/puppycrawl/tools/checkstyle/checks/modifier/RedundantModifierCheck.java index 22081a833..b3283deda 100644 --- a/src/main/java/com/puppycrawl/tools/checkstyle/checks/modifier/RedundantModifierCheck.java +++ b/src/main/java/com/puppycrawl/tools/checkstyle/checks/modifier/RedundantModifierCheck.java @@ -141,25 +141,28 @@ public class RedundantModifierCheck if (ast.getType() == TokenTypes.INTERFACE_DEF) { checkInterfaceModifiers(ast); } - else if (ast.getType() == TokenTypes.CTOR_DEF) { - if (isEnumMember(ast)) { - checkEnumConstructorModifiers(ast); - } - else { - checkClassConstructorModifiers(ast); - } - } else if (ast.getType() == TokenTypes.ENUM_DEF) { checkEnumDef(ast); } - else if (isInterfaceOrAnnotationMember(ast)) { - processInterfaceOrAnnotation(ast); - } - else if (ast.getType() == TokenTypes.METHOD_DEF) { - processMethods(ast); - } - else if (ast.getType() == TokenTypes.RESOURCE) { - processResources(ast); + else { + if (ast.getType() == TokenTypes.CTOR_DEF) { + if (isEnumMember(ast)) { + checkEnumConstructorModifiers(ast); + } + else { + checkClassConstructorModifiers(ast); + } + } + else if (ast.getType() == TokenTypes.METHOD_DEF) { + processMethods(ast); + } + else if (ast.getType() == TokenTypes.RESOURCE) { + processResources(ast); + } + + if (isInterfaceOrAnnotationMember(ast)) { + processInterfaceOrAnnotation(ast); + } } } @@ -203,12 +206,7 @@ public class RedundantModifierCheck processInterfaceOrAnnotation(ast); } else if (ast.getParent() != null) { - final DetailAST modifiers = ast.findFirstToken(TokenTypes.MODIFIERS); - final DetailAST staticModifier = modifiers.findFirstToken(TokenTypes.LITERAL_STATIC); - if (staticModifier != null) { - log(staticModifier.getLineNo(), staticModifier.getColumnNo(), - MSG_KEY, staticModifier.getText()); - } + checkForRedundantModifier(ast, TokenTypes.LITERAL_STATIC); } } @@ -243,7 +241,7 @@ public class RedundantModifierCheck } /** - * Process validation ofMethods. + * Process validation of Methods. * @param ast method AST */ private void processMethods(DetailAST ast) { @@ -270,15 +268,25 @@ public class RedundantModifierCheck } } if (checkFinal && !isAnnotatedWithSafeVarargs(ast)) { - DetailAST modifier = modifiers.getFirstChild(); - while (modifier != null) { - final int type = modifier.getType(); - if (type == TokenTypes.FINAL) { - log(modifier.getLineNo(), modifier.getColumnNo(), - MSG_KEY, modifier.getText()); - break; - } - modifier = modifier.getNextSibling(); + checkForRedundantModifier(ast, TokenTypes.FINAL); + } + + if (!ast.branchContains(TokenTypes.SLIST)) { + processAbstractMethodParameters(ast); + } + } + + /** + * Process validation of parameters for Methods with no definition. + * @param ast method AST + */ + private void processAbstractMethodParameters(DetailAST ast) { + final DetailAST parameters = ast.findFirstToken(TokenTypes.PARAMETERS); + + for (DetailAST child = parameters.getFirstChild(); child != null; child = child + .getNextSibling()) { + if (child.getType() == TokenTypes.PARAMETER_DEF) { + checkForRedundantModifier(child, TokenTypes.FINAL); } } } @@ -290,7 +298,7 @@ public class RedundantModifierCheck private void checkClassConstructorModifiers(DetailAST classCtorAst) { final DetailAST classDef = classCtorAst.getParent().getParent(); if (!isClassPublic(classDef) && !isClassProtected(classDef)) { - checkForRedundantPublicModifier(classCtorAst); + checkForRedundantModifier(classCtorAst, TokenTypes.LITERAL_PUBLIC); } } @@ -299,30 +307,19 @@ public class RedundantModifierCheck * @param ast ast */ private void processResources(DetailAST ast) { - final DetailAST modifiers = ast.findFirstToken(TokenTypes.MODIFIERS); - DetailAST modifier = modifiers.getFirstChild(); - - while (modifier != null) { - final int type = modifier.getType(); - - if (type == TokenTypes.FINAL) { - log(modifier.getLineNo(), modifier.getColumnNo(), MSG_KEY, modifier.getText()); - break; - } - - modifier = modifier.getNextSibling(); - } + checkForRedundantModifier(ast, TokenTypes.FINAL); } /** - * Checks if given ast has redundant public modifier. + * Checks if given ast has a redundant modifier. * @param ast ast + * @param modifierType The modifier to check for. */ - private void checkForRedundantPublicModifier(DetailAST ast) { + private void checkForRedundantModifier(DetailAST ast, int modifierType) { final DetailAST astModifiers = ast.findFirstToken(TokenTypes.MODIFIERS); DetailAST astModifier = astModifiers.getFirstChild(); while (astModifier != null) { - if (astModifier.getType() == TokenTypes.LITERAL_PUBLIC) { + if (astModifier.getType() == modifierType) { log(astModifier.getLineNo(), astModifier.getColumnNo(), MSG_KEY, astModifier.getText()); } diff --git a/src/test/java/com/puppycrawl/tools/checkstyle/checks/modifier/RedundantModifierCheckTest.java b/src/test/java/com/puppycrawl/tools/checkstyle/checks/modifier/RedundantModifierCheckTest.java index 35ccec712..8528623c7 100644 --- a/src/test/java/com/puppycrawl/tools/checkstyle/checks/modifier/RedundantModifierCheckTest.java +++ b/src/test/java/com/puppycrawl/tools/checkstyle/checks/modifier/RedundantModifierCheckTest.java @@ -210,4 +210,17 @@ public class RedundantModifierCheckTest }; verify(checkConfig, getPath("InputFinalInTryWithResource.java"), expected); } + + @Test + public void testFinalInAbstractMethods() throws Exception { + final DefaultConfiguration checkConfig = createCheckConfig(RedundantModifierCheck.class); + final String[] expected = { + "4:33: " + getCheckMessage(MSG_KEY, "final"), + "8:49: " + getCheckMessage(MSG_KEY, "final"), + "11:17: " + getCheckMessage(MSG_KEY, "final"), + "16:24: " + getCheckMessage(MSG_KEY, "final"), + "25:33: " + getCheckMessage(MSG_KEY, "final"), + }; + verify(checkConfig, getPath("InputFinalInAbstractMethods.java"), expected); + } } diff --git a/src/test/resources/com/puppycrawl/tools/checkstyle/checks/modifier/InputFinalInAbstractMethods.java b/src/test/resources/com/puppycrawl/tools/checkstyle/checks/modifier/InputFinalInAbstractMethods.java new file mode 100644 index 000000000..00c1018cb --- /dev/null +++ b/src/test/resources/com/puppycrawl/tools/checkstyle/checks/modifier/InputFinalInAbstractMethods.java @@ -0,0 +1,26 @@ +package com.puppycrawl.tools.checkstyle.checks.modifier; + +public abstract class InputFinalInAbstractMethods { + public abstract void method(final String param); // violation + + public abstract void method2(String param); + + public abstract void method3(String param1, final String param2); // violation +} +interface IWhatever { + void method(final String param); // violation + + void method2(String param); +} +class CWhatever { + native void method(final String param); // violation + + native void method2(String param); +} +enum EWhatever { + TEST() { + public void method(String s) {}; + }; + + public abstract void method(final String s); // violation +}