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 af06aa17e..1307197ef 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 @@ -29,15 +29,40 @@ import com.puppycrawl.tools.checkstyle.api.DetailAST; import com.puppycrawl.tools.checkstyle.api.TokenTypes; /** - * Checks for redundant modifiers in interface and annotation definitions. - * Checks for non public class constructor and enum constructor redundant modifier. - * Checks for redundant final modifiers on methods of final classes. - * Checks for redundant static modifiers on nested enums. + * Checks for redundant modifiers in interface and annotation definitions, + * final modifier on methods of final classes, inner interface + * declarations that are declared as static, non public class + * constructors and enum constructors, nested enum definitions that are declared + * as static. * - *

Examples:

+ *

Interfaces by definition are abstract so the abstract + * modifier on the interface is redundant. + * + *

Classes inside of interfaces by definition are public and static, + * so the public and static modifiers + * on the inner classes are redundant. On the other hand, classes + * inside of interfaces can be abstract or non abstract. + * So, abstract modifier is allowed. + * + *

Fields in interfaces and annotations are automatically + * public, static and final, so these modifiers are redundant as + * well.

+ * + *

As annotations are a form of interface, their fields are also + * automatically public, static and final just as their + * annotation fields are automatically public and abstract.

+ * + *

Enums by definition are static implicit subclasses of java.lang.Enum<E>. + * So, the static modifier on the enums is redundant. In addition, + * if enum is inside of interface, public modifier is also redundant. + * + *

Final classes by definition cannot be extended so the final + * modifier on the method of a final class is redundant. + * + *

Public modifier for constructors in non-public non-protected classes + * is always obsolete:

* * 
- * {@code
  * public class PublicClass {
  *     public PublicClass() {} // OK
  * }
@@ -45,36 +70,30 @@ import com.puppycrawl.tools.checkstyle.api.TokenTypes;
  * class PackagePrivateClass {
  *     public PackagePrivateClass() {} // violation expected
  * }
- * }
  *
* + *

There is no violation in the following example, + * because removing public modifier from ProtectedInnerClass + * constructor will make this code not compiling:

+ * * 
- * {@code
  * package a;
- * public class ClassWithProtectedInnerClass {
- *     protected class ProtectedClass {
- *         public ProtectedClass () {} // OK
+ * public class ClassExample {
+ *     protected class ProtectedInnerClass {
+ *         public ProtectedInnerClass () {}
  *     }
  * }
- * }
- *
- *

- * in this example is no violation because removing public from - * ProtectedClass constructor modifier will make this example - * not compiling: - *

- * 
- * {@code
+ *
  * package b;
- * import a.ClassWithProtectedInnerClass;
- * public class ClassExtending extends ClassWithProtectedInnerClass {
- *     ProtectedClass pc = new ProtectedClass();
- * }
+ * import a.ClassExample;
+ * public class ClassExtending extends ClassExample {
+ *     ProtectedInnerClass pc = new ProtectedInnerClass();
  * }
  *
* * @author lkuehne * @author liscju + * @author Andrei Selkin * @author Vladislav Lisetskiy */ public class RedundantModifierCheck @@ -208,6 +227,7 @@ public class RedundantModifierCheck || type == TokenTypes.LITERAL_STATIC && ast.getType() != TokenTypes.METHOD_DEF || type == TokenTypes.ABSTRACT + && ast.getType() != TokenTypes.CLASS_DEF || type == TokenTypes.FINAL) { log(modifier.getLineNo(), modifier.getColumnNo(), MSG_KEY, modifier.getText()); diff --git a/src/test/java/com/puppycrawl/tools/checkstyle/checks/modifier/RedundantModifierTest.java b/src/test/java/com/puppycrawl/tools/checkstyle/checks/modifier/RedundantModifierTest.java index 00031f38e..45c579ea0 100644 --- a/src/test/java/com/puppycrawl/tools/checkstyle/checks/modifier/RedundantModifierTest.java +++ b/src/test/java/com/puppycrawl/tools/checkstyle/checks/modifier/RedundantModifierTest.java @@ -33,6 +33,20 @@ import com.puppycrawl.tools.checkstyle.api.TokenTypes; public class RedundantModifierTest extends BaseCheckTestSupport { + + @Test + public void testClassesInsideOfInterfaces() throws Exception { + final DefaultConfiguration checkConfig = + createCheckConfig(RedundantModifierCheck.class); + final String[] expected = { + "11:5: " + getCheckMessage(MSG_KEY, "static"), + "17:5: " + getCheckMessage(MSG_KEY, "public"), + "20:5: " + getCheckMessage(MSG_KEY, "public"), + "26:5: " + getCheckMessage(MSG_KEY, "static"), + }; + verify(checkConfig, getPath("InputModifierClassesInsideOfInterfaces.java"), expected); + } + @Test public void testIt() throws Exception { final DefaultConfiguration checkConfig = diff --git a/src/test/resources-noncompilable/com/puppycrawl/tools/checkstyle/InputFinalInDefaultMethods.java b/src/test/resources-noncompilable/com/puppycrawl/tools/checkstyle/InputFinalInDefaultMethods.java index 20d9733c3..b6ddca54a 100644 --- a/src/test/resources-noncompilable/com/puppycrawl/tools/checkstyle/InputFinalInDefaultMethods.java +++ b/src/test/resources-noncompilable/com/puppycrawl/tools/checkstyle/InputFinalInDefaultMethods.java @@ -1,6 +1,6 @@ //Compilable with Java8 public interface MyInterface { - final int k = 5; //WARNING + final int k = 5; // violation default int defaultMethod(final int x) { if (k == 5) { diff --git a/src/test/resources/com/puppycrawl/tools/checkstyle/InputModifier.java b/src/test/resources/com/puppycrawl/tools/checkstyle/InputModifier.java index a0eaa3119..52f7ab115 100644 --- a/src/test/resources/com/puppycrawl/tools/checkstyle/InputModifier.java +++ b/src/test/resources/com/puppycrawl/tools/checkstyle/InputModifier.java @@ -51,32 +51,32 @@ strictfp final class InputModifier // illegal order of modifiers for class } /** holder for redundant 'public' modifier check. */ - public static interface InputRedundantPublicModifier + public static interface InputRedundantPublicModifier // violation { /** redundant 'public' modifier */ - public void a(); + public void a(); // violation /** all OK */ void b(); /** redundant abstract modifier */ - abstract void c(); + abstract void c(); // violation /** redundant 'public' modifier */ - public float PI_PUBLIC = (float) 3.14; + public float PI_PUBLIC = (float) 3.14; // violation /** redundant 'abstract' modifier (field can not be abstract) */ // abstract float PI_ABSTRACT = (float) 3.14; /** redundant 'final' modifier */ - final float PI_FINAL = (float) 3.14; + final float PI_FINAL = (float) 3.14; // violation /** all OK */ float PI_OK = (float) 3.14; } /** redundant 'final' modifier */ - private final void method() + private final void method() // violation { } } @@ -85,7 +85,7 @@ strictfp final class InputModifier // illegal order of modifiers for class final class RedundantFinalClass { /** redundant 'final' modifier */ - public final void finalMethod() + public final void finalMethod() // violation { } @@ -96,7 +96,7 @@ final class RedundantFinalClass } /** Holder for redundant modifiers of inner implementation */ -abstract interface InnerImplementation +abstract interface InnerImplementation // violation { InnerImplementation inner = new InnerImplementation() @@ -113,12 +113,12 @@ abstract interface InnerImplementation /** Holder for redundant modifiers of annotation fields/variables */ @interface Annotation { - public String s1 = ""; - final String s2 = ""; - static String s3 = ""; + public String s1 = ""; // violation + final String s2 = ""; // violation + static String s3 = ""; // violation String s4 = ""; - public String blah(); - abstract String blah2(); + public String blah(); // violation + abstract String blah2(); // violation } @interface MyAnnotation2 { diff --git a/src/test/resources/com/puppycrawl/tools/checkstyle/InputModifierClassesInsideOfInterfaces.java b/src/test/resources/com/puppycrawl/tools/checkstyle/InputModifierClassesInsideOfInterfaces.java new file mode 100644 index 000000000..e7042cdde --- /dev/null +++ b/src/test/resources/com/puppycrawl/tools/checkstyle/InputModifierClassesInsideOfInterfaces.java @@ -0,0 +1,31 @@ +package com.puppycrawl.tools.checkstyle; + +public interface InputModifierClassesInsideOfInterfaces { + + // Class inside of interface can be abstract and non abstract, but always public static. + abstract class A {} + + class B {} + + // All classes inside interfaces are public static classes, and static modifier is redundant. + static class C { // violation + public static boolean verifyState( A a ) { + return true; + } + } + + public class E {} // violation + + // Enums are static implicit subclasses of Enum class. + public enum Role1 { // violation + ADMIN, + EDITOR, + VANILLA; + } + + static enum Role2 { // violation + ADMIN, + EDITOR, + VANILLA; + } +} diff --git a/src/test/resources/com/puppycrawl/tools/checkstyle/InputRedundantConstructorModifier.java b/src/test/resources/com/puppycrawl/tools/checkstyle/InputRedundantConstructorModifier.java index 29b16d3cf..66e76728a 100644 --- a/src/test/resources/com/puppycrawl/tools/checkstyle/InputRedundantConstructorModifier.java +++ b/src/test/resources/com/puppycrawl/tools/checkstyle/InputRedundantConstructorModifier.java @@ -7,7 +7,7 @@ package com.puppycrawl.tools.checkstyle; public enum InputRedundantConstructorModifier { VAL1, VAL2; - private InputRedundantConstructorModifier() { } + private InputRedundantConstructorModifier() { } // violation InputRedundantConstructorModifier(int i) { } diff --git a/src/test/resources/com/puppycrawl/tools/checkstyle/InputRedundantStaticModifierInInnerTypeOfInterface.java b/src/test/resources/com/puppycrawl/tools/checkstyle/InputRedundantStaticModifierInInnerTypeOfInterface.java index eab97cabe..8e7a8c6cb 100644 --- a/src/test/resources/com/puppycrawl/tools/checkstyle/InputRedundantStaticModifierInInnerTypeOfInterface.java +++ b/src/test/resources/com/puppycrawl/tools/checkstyle/InputRedundantStaticModifierInInnerTypeOfInterface.java @@ -5,11 +5,11 @@ package com.puppycrawl.tools.checkstyle; public interface InputRedundantStaticModifierInInnerTypeOfInterface { - static class MyInnerClass { } + static class MyInnerClass { } // violation class MyInnerClass2 { } - static enum MyInnerEnum { } + static enum MyInnerEnum { } // violation enum MyInnerEnum2 { } } diff --git a/src/xdocs/checks.xml b/src/xdocs/checks.xml index dc8d1bd7f..413bd4572 100644 --- a/src/xdocs/checks.xml +++ b/src/xdocs/checks.xml @@ -617,7 +617,11 @@ RedundantModifier - Checks for redundant modifiers in interface and annotation definitions. + Checks for redundant modifiers in interface and annotation definitions, + final modifier on methods of final classes, inner interface + declarations that are declared as static, non public class + constructors and enum constructors, nested enum definitions that are declared + as static. Regexp diff --git a/src/xdocs/config_modifier.xml b/src/xdocs/config_modifier.xml index 9327043d7..437d8dc96 100644 --- a/src/xdocs/config_modifier.xml +++ b/src/xdocs/config_modifier.xml @@ -122,7 +122,15 @@

- Variables in interfaces and annotations are automatically + Classes inside of interfaces by definition are public and static, + so the public and static modifiers + on the inner classes are redundant. On the other hand, classes + inside of interfaces can be abstract or non abstract. + So, abstract modifier is allowed. +

+ +

+ Fields in interfaces and annotations are automatically public, static and final, so these modifiers are redundant as well.

@@ -134,14 +142,20 @@

- Final classes by definition cannot be extended so the final - modifier on the method of a final class is redundant. + Enums by definition are static implicit subclasses of java.lang.Enum<E>. + So, the static modifier on the enums is redundant. In addition, + if enum is inside of interface, public modifier is also redundant.

Nested enum types are always static by default.

+

+ Final classes by definition cannot be extended so the final + modifier on the method of a final class is redundant. +

+

Public modifier for constructors in non-public non-protected classes is always obsolete: @@ -157,22 +171,23 @@ } -

For non-public protected classes public modifier is not obsolete, - removing public from this example will make code not compiling: +

There is no violation in the following example, + because removing public modifier from ProtectedInnerClass + constructor will make this code not compiling:

package a; - public class ClassWithProtectedInnerClass { - protected class ProtectedClass { - public ProtectedClass () {} + public class ClassExample { + protected class ProtectedInnerClass { + public ProtectedInnerClass () {} } } package b; - import a.ClassWithProtectedInnerClass; - public class ClassExtending extends ClassWithProtectedInnerClass { - ProtectedClass pc = new ProtectedClass(); + import a.ClassExample; + public class ClassExtending extends ClassExample { + ProtectedInnerClass pc = new ProtectedInnerClass(); }