diff --git a/CHANGELOG.md b/CHANGELOG.md
index 5de9e9ea76a..e0e1ab2f96d 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -34,6 +34,7 @@ Currently the versioning policy of this project follows [Semantic Versioning v2.
- Make TypeQualifierResolver recognize org.apache.avro.reflect.Nullable ([#2066](https://github.com/spotbugs/spotbugs/pull/2066))
- New detector `FindArgumentAssertions` detecting bug `ASSERTION_OF_ARGUMENTS` in case of validation of arguments of public functions using assertions (See [MET01-J. Never use assertions to validate method arguments](https://wiki.sei.cmu.edu/confluence/display/java/MET01-J.+Never+use+assertions+to+validate+method+arguments))
- Add new detector `CT_CONSTRUCTOR_THROW` for detecting constructors that throw exceptions.
+- New detector `FindIncreasedAccessibilityOfMethods` for new bug type `IAOM_DO_NOT_INCREASE_METHOD_ACCESSIBILITY`. This detector reports a bug if a class increases the accessibility of overridden or hidden methods. (See [SEI CERT rule MET04-J](https://wiki.sei.cmu.edu/confluence/display/java/MET04-J.+Do+not+increase+the+accessibility+of+overridden+or+hidden+methods))
### Security
- Disable access to external entities when processing XML ([#2217](https://github.com/spotbugs/spotbugs/pull/2217))
diff --git a/spotbugs-tests/src/test/java/edu/umd/cs/findbugs/detect/FindIncreasedAccessibilityOfMethodsTest.java b/spotbugs-tests/src/test/java/edu/umd/cs/findbugs/detect/FindIncreasedAccessibilityOfMethodsTest.java
new file mode 100644
index 00000000000..34cf92ffcc3
--- /dev/null
+++ b/spotbugs-tests/src/test/java/edu/umd/cs/findbugs/detect/FindIncreasedAccessibilityOfMethodsTest.java
@@ -0,0 +1,97 @@
+package edu.umd.cs.findbugs.detect;
+
+import edu.umd.cs.findbugs.AbstractIntegrationTest;
+import edu.umd.cs.findbugs.test.matcher.BugInstanceMatcher;
+import edu.umd.cs.findbugs.test.matcher.BugInstanceMatcherBuilder;
+import org.junit.Test;
+
+import static edu.umd.cs.findbugs.test.CountMatcher.containsExactly;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.hasItem;
+
+public class FindIncreasedAccessibilityOfMethodsTest extends AbstractIntegrationTest {
+
+ @Test
+ public void findIAOMBugInClass_IncreasedAccessibilityOfMethods_SubClassFromSamePackage() {
+ performAnalysis(
+ "increasedAccessibilityOfMethods/SuperClassOfSuperClass.class",
+ "increasedAccessibilityOfMethods/SuperClass.class",
+ "increasedAccessibilityOfMethods/SubClassFromSamePackage.class");
+
+ assertNumberOfIAOMBugs(6);
+
+ assertIAOMBug("SubClassFromSamePackage", "protectedMethodToPublic");
+ assertIAOMBug("SubClassFromSamePackage", "packagePrivateMethodToPublic");
+ assertIAOMBug("SubClassFromSamePackage", "packagePrivateMethodToProtected");
+ assertIAOMBug("SubClassFromSamePackage", "superProtectedMethodToPublic");
+ assertIAOMBug("SubClassFromSamePackage", "superPackagePrivateMethodToPublic");
+ assertIAOMBug("SubClassFromSamePackage", "superPackagePrivateMethodToProtected");
+ }
+
+ @Test
+ public void findIAOMBugInClass_IncreasedAccessibilityOfMethods_SubClassFromAnotherPackage() {
+ performAnalysis(
+ "increasedAccessibilityOfMethods/SuperClassOfSuperClass.class",
+ "increasedAccessibilityOfMethods/SuperClass.class",
+ "increasedAccessibilityOfMethods/anotherPackage/SubClassFromAnotherPackage.class");
+
+ assertNumberOfIAOMBugs(1);
+
+ assertIAOMBug("SubClassFromAnotherPackage", "protectedMethodToPublic");
+ }
+
+ @Test
+ public void findNoIAOMBugInClass_IncreasedAccessibilityOfMethods_SubClassFromAnotherPackage() {
+ performAnalysis(
+ "increasedAccessibilityOfMethods/SuperClassOfSuperClass.class",
+ "increasedAccessibilityOfMethods/SuperClass.class",
+ "increasedAccessibilityOfMethods/anotherPackage/CorrectSubClass.class");
+
+ assertNumberOfIAOMBugs(0);
+ }
+
+ @Test
+ public void findNoIAOMBugInClass_IncreasedAccessibilityOfMethods_CloneableImplementation() {
+ performAnalysis(
+ "increasedAccessibilityOfMethods/CloneableImplementation.class");
+
+ assertNumberOfIAOMBugs(0);
+ }
+
+ @Test
+ public void findNoIAOMBugInClass_IncreasedAccessibilityOfMethods_InterfaceImplementation() {
+ performAnalysis(
+ "increasedAccessibilityOfMethods/InterfaceImplementation.class");
+
+ assertNumberOfIAOMBugs(0);
+ }
+
+ @Test
+ public void findNoIAOMBugInClass_IncreasedAccessibilityOfMethods_GenericImplementation() {
+ performAnalysis(
+ "increasedAccessibilityOfMethods/GenericSuperClass.class",
+ "increasedAccessibilityOfMethods/GenericSubClass.class");
+
+ assertNumberOfIAOMBugs(3);
+
+ assertIAOMBug("GenericSubClass", "protectedMethodToPublicWithGenericParameter");
+ assertIAOMBug("GenericSubClass", "packagePrivateMethodToPublicWithGenericParameter");
+ assertIAOMBug("GenericSubClass", "packagePrivateMethodToProtectedWithGenericParameter");
+ }
+
+ private void assertNumberOfIAOMBugs(int numberOfBugs) {
+ BugInstanceMatcher bugTypeMatcher = new BugInstanceMatcherBuilder()
+ .bugType("IAOM_DO_NOT_INCREASE_METHOD_ACCESSIBILITY").build();
+ assertThat(getBugCollection(), containsExactly(numberOfBugs, bugTypeMatcher));
+ }
+
+ private void assertIAOMBug(String className, String methodName) {
+ BugInstanceMatcher bugInstanceMatcher = new BugInstanceMatcherBuilder()
+ .bugType("IAOM_DO_NOT_INCREASE_METHOD_ACCESSIBILITY")
+ .inClass(className)
+ .inMethod(methodName)
+ .build();
+ assertThat(getBugCollection(), hasItem(bugInstanceMatcher));
+ }
+
+}
diff --git a/spotbugs/etc/findbugs.xml b/spotbugs/etc/findbugs.xml
index ea5c8ed694a..d3df38e09ea 100644
--- a/spotbugs/etc/findbugs.xml
+++ b/spotbugs/etc/findbugs.xml
@@ -679,8 +679,11 @@
reports="VSC_VULNERABLE_SECURITY_CHECK_METHODS"/>
-
-
+
+
+
+
@@ -1312,4 +1315,5 @@
+
diff --git a/spotbugs/etc/messages.xml b/spotbugs/etc/messages.xml
index ed9fcb9e88c..54c2b73901f 100644
--- a/spotbugs/etc/messages.xml
+++ b/spotbugs/etc/messages.xml
@@ -1811,6 +1811,14 @@ factory pattern to create these objects.
]]>
+
+
+ Finds classes where the accessibility of the overridden or hidden method was increased.
+ ]]>
+
+
+
+
Format string problem
Analysis skipped
Infinite Loop
@@ -9149,4 +9174,5 @@ Using floating-point variables should not be used as loop counters, as they are
Public Attribute
Vulnerable security check performing methods
Misuse of assertions for checking arguments of public methods
+ Do not increase the accessibility of overridden or hidden methods
diff --git a/spotbugs/src/main/java/edu/umd/cs/findbugs/detect/FindIncreasedAccessibilityOfMethods.java b/spotbugs/src/main/java/edu/umd/cs/findbugs/detect/FindIncreasedAccessibilityOfMethods.java
new file mode 100644
index 00000000000..7699f21c543
--- /dev/null
+++ b/spotbugs/src/main/java/edu/umd/cs/findbugs/detect/FindIncreasedAccessibilityOfMethods.java
@@ -0,0 +1,123 @@
+/*
+ * SpotBugs - Find bugs in Java programs
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
+ */
+
+package edu.umd.cs.findbugs.detect;
+
+import edu.umd.cs.findbugs.BugInstance;
+import edu.umd.cs.findbugs.BugReporter;
+import edu.umd.cs.findbugs.Detector;
+import edu.umd.cs.findbugs.ba.AnalysisContext;
+import edu.umd.cs.findbugs.ba.ClassContext;
+import org.apache.bcel.Repository;
+import org.apache.bcel.classfile.JavaClass;
+import org.apache.bcel.classfile.Method;
+
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Objects;
+
+public class FindIncreasedAccessibilityOfMethods implements Detector {
+
+ private final BugReporter bugReporter;
+
+ public FindIncreasedAccessibilityOfMethods(BugReporter bugReporter) {
+ this.bugReporter = bugReporter;
+ }
+
+ @Override
+ public void visitClassContext(ClassContext classContext) {
+ JavaClass[] superClasses;
+ JavaClass directSuperClass;
+ JavaClass subClass = classContext.getJavaClass();
+ try {
+ directSuperClass = subClass.getSuperClass();
+ superClasses = subClass.getSuperClasses();
+ } catch (ClassNotFoundException e) {
+ AnalysisContext.reportMissingClass(e);
+ return;
+ }
+
+ Method[] subClassMethods = subClass.getMethods();
+
+ for (JavaClass javaClass : superClasses) {
+ for (Method superClassMethod : getMethodsExceptConstructors(javaClass)) {
+ if (isAccessibleFromSubClass(superClassMethod, subClass, directSuperClass)) {
+ for (Method subClassMethod : subClassMethods) {
+ if (isOverridable(superClassMethod, subClassMethod) && !isCloneMethodFromCloneable(subClass, superClassMethod)) {
+ if (superClassMethod.isProtected() && subClassMethod.isPublic()) {
+ bugReporter.reportBug(new BugInstance(this, "IAOM_DO_NOT_INCREASE_METHOD_ACCESSIBILITY", NORMAL_PRIORITY)
+ .addClassAndMethod(subClass, subClassMethod)
+ .addString("protected")
+ .addString("public"));
+ }
+ if (isPackagePrivate(superClassMethod)) {
+ if (subClassMethod.isPublic()) {
+ bugReporter.reportBug(new BugInstance(this, "IAOM_DO_NOT_INCREASE_METHOD_ACCESSIBILITY", NORMAL_PRIORITY)
+ .addClassAndMethod(subClass, subClassMethod)
+ .addString("package private")
+ .addString("public"));
+ } else if (subClassMethod.isProtected()) {
+ bugReporter.reportBug(new BugInstance(this, "IAOM_DO_NOT_INCREASE_METHOD_ACCESSIBILITY", NORMAL_PRIORITY)
+ .addClassAndMethod(subClass, subClassMethod)
+ .addString("package private")
+ .addString("protected"));
+ }
+ }
+ }
+ }
+ }
+ }
+ }
+ }
+
+ @Override
+ public void report() {
+ }
+
+ private boolean isPackagePrivate(Method method) {
+ return !(method.isPublic() || method.isProtected() || method.isPrivate());
+ }
+
+ private boolean isAccessibleFromSubClass(Method method, JavaClass subClass, JavaClass directSuperClass) {
+ return method.isProtected() || (subClass.getPackageName().equals(directSuperClass.getPackageName()) && isPackagePrivate(method));
+ }
+
+ private boolean isOverridable(Method original, Method overrider) {
+ return Objects.equals(original, overrider) && !original.isFinal();
+ }
+
+ private List getMethodsExceptConstructors(JavaClass javaClass) {
+ ArrayList methods = new ArrayList<>();
+ for (Method method : javaClass.getMethods()) {
+ if (!method.getName().equals("")) {
+ methods.add(method);
+ }
+ }
+ return methods;
+ }
+
+ private boolean isCloneMethodFromCloneable(JavaClass javaClass, Method method) {
+ try {
+ return javaClass.implementationOf(Repository.lookupClass(Cloneable.class)) && method.getName().equals("clone");
+ } catch (ClassNotFoundException e) {
+ AnalysisContext.reportMissingClass(e);
+ return false;
+ }
+ }
+
+}
diff --git a/spotbugsTestCases/src/java/increasedAccessibilityOfMethods/CloneableImplementation.java b/spotbugsTestCases/src/java/increasedAccessibilityOfMethods/CloneableImplementation.java
new file mode 100644
index 00000000000..a9cb0391791
--- /dev/null
+++ b/spotbugsTestCases/src/java/increasedAccessibilityOfMethods/CloneableImplementation.java
@@ -0,0 +1,14 @@
+package increasedAccessibilityOfMethods;
+
+public class CloneableImplementation implements Cloneable {
+
+ @Override
+ public CloneableImplementation clone() {
+ try {
+ return (CloneableImplementation) super.clone();
+ } catch (CloneNotSupportedException e) {
+ throw new AssertionError();
+ }
+ }
+
+}
diff --git a/spotbugsTestCases/src/java/increasedAccessibilityOfMethods/GenericSubClass.java b/spotbugsTestCases/src/java/increasedAccessibilityOfMethods/GenericSubClass.java
new file mode 100644
index 00000000000..cc267284901
--- /dev/null
+++ b/spotbugsTestCases/src/java/increasedAccessibilityOfMethods/GenericSubClass.java
@@ -0,0 +1,35 @@
+package increasedAccessibilityOfMethods;
+
+public class GenericSubClass extends GenericSuperClass {
+
+ @Override
+ public String publicMethodWithGenericParameter(K parameter) {
+ return "GenericSubClass.publicMethodWithGenericParameter";
+ }
+
+ @Override
+ public String protectedMethodToPublicWithGenericParameter(K parameter) {
+ return "GenericSubClass.protectedMethodToPublicWithGenericParameter";
+ }
+
+ @Override
+ protected String protectedMethodWithGenericParameter(K parameter) {
+ return "GenericSubClass.protectedMethodWithGenericParameter";
+ }
+
+ @Override
+ public String packagePrivateMethodToPublicWithGenericParameter(K parameter) {
+ return "GenericSubClass.packagePrivateMethodToPublicWithGenericParameter";
+ }
+
+ @Override
+ protected String packagePrivateMethodToProtectedWithGenericParameter(K parameter) {
+ return "GenericSubClass.packagePrivateMethodToProtectedWithGenericParameter";
+ }
+
+ @Override
+ String packagePrivateWithGenericParameter(K parameter) {
+ return "GenericSubClass.packagePrivateWithGenericParameter";
+ }
+
+}
diff --git a/spotbugsTestCases/src/java/increasedAccessibilityOfMethods/GenericSuperClass.java b/spotbugsTestCases/src/java/increasedAccessibilityOfMethods/GenericSuperClass.java
new file mode 100644
index 00000000000..fc30b0a0778
--- /dev/null
+++ b/spotbugsTestCases/src/java/increasedAccessibilityOfMethods/GenericSuperClass.java
@@ -0,0 +1,36 @@
+package increasedAccessibilityOfMethods;
+
+import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
+
+public class GenericSuperClass {
+
+ public String publicMethodWithGenericParameter(T parameter) {
+ return "GenericSuperClass.publicMethodWithGenericParameter";
+ }
+
+ protected String protectedMethodToPublicWithGenericParameter(T parameter) {
+ return "GenericSuperClass.protectedMethodToPublicWithGenericParameter";
+ }
+
+ protected String protectedMethodWithGenericParameter(T parameter) {
+ return "GenericSuperClass.protectedMethodWithGenericParameter";
+ }
+
+ String packagePrivateMethodToPublicWithGenericParameter(T parameter) {
+ return "GenericSuperClass.packagePrivateMethodToPublicWithGenericParameter";
+ }
+
+ String packagePrivateMethodToProtectedWithGenericParameter(T parameter) {
+ return "GenericSuperClass.packagePrivateMethodToProtectedWithGenericParameter";
+ }
+
+ String packagePrivateWithGenericParameter(T parameter) {
+ return "GenericSuperClass.packagePrivateWithGenericParameter";
+ }
+
+ @SuppressFBWarnings("UPM")
+ private String privateMethodWithGenericParameter(T parameter) {
+ return "GenericSuperClass.privateMethodWithGenericParameter";
+ }
+
+}
diff --git a/spotbugsTestCases/src/java/increasedAccessibilityOfMethods/Interface.java b/spotbugsTestCases/src/java/increasedAccessibilityOfMethods/Interface.java
new file mode 100644
index 00000000000..baea09fbd0c
--- /dev/null
+++ b/spotbugsTestCases/src/java/increasedAccessibilityOfMethods/Interface.java
@@ -0,0 +1,7 @@
+package increasedAccessibilityOfMethods;
+
+public interface Interface {
+
+ String packagePrivateInterfaceMethod();
+
+}
diff --git a/spotbugsTestCases/src/java/increasedAccessibilityOfMethods/InterfaceImplementation.java b/spotbugsTestCases/src/java/increasedAccessibilityOfMethods/InterfaceImplementation.java
new file mode 100644
index 00000000000..1cb23177594
--- /dev/null
+++ b/spotbugsTestCases/src/java/increasedAccessibilityOfMethods/InterfaceImplementation.java
@@ -0,0 +1,10 @@
+package increasedAccessibilityOfMethods;
+
+public class InterfaceImplementation implements Interface {
+
+ @Override
+ public String packagePrivateInterfaceMethod() {
+ return "InterfaceImplementation.packagePrivateInterfaceMethod";
+ }
+
+}
diff --git a/spotbugsTestCases/src/java/increasedAccessibilityOfMethods/SubClassFromSamePackage.java b/spotbugsTestCases/src/java/increasedAccessibilityOfMethods/SubClassFromSamePackage.java
new file mode 100644
index 00000000000..114c1b05453
--- /dev/null
+++ b/spotbugsTestCases/src/java/increasedAccessibilityOfMethods/SubClassFromSamePackage.java
@@ -0,0 +1,39 @@
+package increasedAccessibilityOfMethods;
+
+public class SubClassFromSamePackage extends SuperClass {
+
+ public SubClassFromSamePackage() {
+ super();
+ }
+
+ @Override
+ public String protectedMethodToPublic() {
+ return "SuperClass.protectedMethodToPublic";
+ }
+
+ @Override
+ public String packagePrivateMethodToPublic() {
+ return "SuperClass.packagePrivateMethodToPublic";
+ }
+
+ @Override
+ protected String packagePrivateMethodToProtected() {
+ return "SuperClass.packagePrivateMethodToProtected";
+ }
+
+ @Override
+ public String superProtectedMethodToPublic() {
+ return "SuperClassOfSuperClass.superProtectedMethodToPublic";
+ }
+
+ @Override
+ public String superPackagePrivateMethodToPublic() {
+ return "SuperClassOfSuperClass.superPackagePrivateMethodToPublic";
+ }
+
+ @Override
+ protected String superPackagePrivateMethodToProtected() {
+ return "SuperClassOfSuperClass.superPackagePrivateMethodToProtected";
+ }
+
+}
diff --git a/spotbugsTestCases/src/java/increasedAccessibilityOfMethods/SuperClass.java b/spotbugsTestCases/src/java/increasedAccessibilityOfMethods/SuperClass.java
new file mode 100644
index 00000000000..7ce0466afe1
--- /dev/null
+++ b/spotbugsTestCases/src/java/increasedAccessibilityOfMethods/SuperClass.java
@@ -0,0 +1,40 @@
+package increasedAccessibilityOfMethods;
+
+import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
+
+public class SuperClass extends SuperClassOfSuperClass {
+
+ protected SuperClass() {
+ super();
+ }
+
+ public String publicMethod() {
+ return "SuperClass.publicMethod";
+ }
+
+ protected String protectedMethodToPublic() {
+ return "SuperClass.protectedMethodToPublic";
+ }
+
+ protected String protectedMethod() {
+ return "SuperClass.protectedMethod";
+ }
+
+ String packagePrivateMethodToPublic() {
+ return "SuperClass.packagePrivateMethodToPublic";
+ }
+
+ String packagePrivateMethodToProtected() {
+ return "SuperClass.packagePrivateMethodToProtected";
+ }
+
+ String packagePrivate() {
+ return "SuperClass.packagePrivate";
+ }
+
+ @SuppressFBWarnings("UPM")
+ private String privateMethod() {
+ return "SuperClass.privateMethod";
+ }
+
+}
diff --git a/spotbugsTestCases/src/java/increasedAccessibilityOfMethods/SuperClassOfSuperClass.java b/spotbugsTestCases/src/java/increasedAccessibilityOfMethods/SuperClassOfSuperClass.java
new file mode 100644
index 00000000000..76ae9844662
--- /dev/null
+++ b/spotbugsTestCases/src/java/increasedAccessibilityOfMethods/SuperClassOfSuperClass.java
@@ -0,0 +1,29 @@
+package increasedAccessibilityOfMethods;
+
+public class SuperClassOfSuperClass {
+
+ public String superPublicMethod() {
+ return "SuperClassOfSuperClass.superPublicMethod";
+ }
+
+ protected String superProtectedMethodToPublic() {
+ return "SuperClassOfSuperClass.superProtectedMethodToPublic";
+ }
+
+ protected String superProtectedMethod() {
+ return "SuperClassOfSuperClass.superProtectedMethod";
+ }
+
+ String superPackagePrivateMethodToPublic() {
+ return "SuperClassOfSuperClass.superPackagePrivateMethodToPublic";
+ }
+
+ String superPackagePrivateMethodToProtected() {
+ return "SuperClassOfSuperClass.superPackagePrivateMethodToProtected";
+ }
+
+ String superPackagePrivate() {
+ return "SuperClassOfSuperClass.superPackagePrivate";
+ }
+
+}
diff --git a/spotbugsTestCases/src/java/increasedAccessibilityOfMethods/anotherPackage/CorrectSubClass.java b/spotbugsTestCases/src/java/increasedAccessibilityOfMethods/anotherPackage/CorrectSubClass.java
new file mode 100644
index 00000000000..207970c9f2f
--- /dev/null
+++ b/spotbugsTestCases/src/java/increasedAccessibilityOfMethods/anotherPackage/CorrectSubClass.java
@@ -0,0 +1,15 @@
+package increasedAccessibilityOfMethods.anotherPackage;
+
+import increasedAccessibilityOfMethods.SuperClass;
+
+public class CorrectSubClass extends SuperClass {
+
+ public String packagePrivate() {
+ return "CorrectSubClass.packagePrivate";
+ }
+
+ public String privateMethod() {
+ return "CorrectSubClass.privateMethod";
+ }
+
+}
diff --git a/spotbugsTestCases/src/java/increasedAccessibilityOfMethods/anotherPackage/SubClassFromAnotherPackage.java b/spotbugsTestCases/src/java/increasedAccessibilityOfMethods/anotherPackage/SubClassFromAnotherPackage.java
new file mode 100644
index 00000000000..edc7b185bac
--- /dev/null
+++ b/spotbugsTestCases/src/java/increasedAccessibilityOfMethods/anotherPackage/SubClassFromAnotherPackage.java
@@ -0,0 +1,16 @@
+package increasedAccessibilityOfMethods.anotherPackage;
+
+import increasedAccessibilityOfMethods.SuperClass;
+
+public class SubClassFromAnotherPackage extends SuperClass {
+
+ public SubClassFromAnotherPackage() {
+ super();
+ }
+
+ @Override
+ public String protectedMethodToPublic() {
+ return "SuperClass.protectedMethodToPublic";
+ }
+
+}