diff --git a/its/ruling/src/test/resources/commons-beanutils/java-S9343.json b/its/ruling/src/test/resources/commons-beanutils/java-S9343.json new file mode 100644 index 00000000000..2410b1a10a6 --- /dev/null +++ b/its/ruling/src/test/resources/commons-beanutils/java-S9343.json @@ -0,0 +1,157 @@ +{ +"commons-beanutils:commons-beanutils:src/main/java/org/apache/commons/beanutils2/BeanMap.java": [ +209 +], +"commons-beanutils:commons-beanutils:src/test/java/org/apache/commons/beanutils2/BasicDynaBeanTestCase.java": [ +102, +161 +], +"commons-beanutils:commons-beanutils:src/test/java/org/apache/commons/beanutils2/BeanComparatorTestCase.java": [ +63, +83 +], +"commons-beanutils:commons-beanutils:src/test/java/org/apache/commons/beanutils2/BeanUtilsBenchCase.java": [ +84, +159 +], +"commons-beanutils:commons-beanutils:src/test/java/org/apache/commons/beanutils2/BeanUtilsTestCase.java": [ +128, +168 +], +"commons-beanutils:commons-beanutils:src/test/java/org/apache/commons/beanutils2/BeanificationTestCase.java": [ +68, +87 +], +"commons-beanutils:commons-beanutils:src/test/java/org/apache/commons/beanutils2/ConstructorUtilsTestCase.java": [ +55, +71 +], +"commons-beanutils:commons-beanutils:src/test/java/org/apache/commons/beanutils2/ConvertUtilsTestCase.java": [ +67, +86 +], +"commons-beanutils:commons-beanutils:src/test/java/org/apache/commons/beanutils2/DynaBeanMapDecoratorTestCase.java": [ +91, +113 +], +"commons-beanutils:commons-beanutils:src/test/java/org/apache/commons/beanutils2/DynaBeanUtilsTestCase.java": [ +106, +176 +], +"commons-beanutils:commons-beanutils:src/test/java/org/apache/commons/beanutils2/DynaPropertyUtilsTestCase.java": [ +104, +173 +], +"commons-beanutils:commons-beanutils:src/test/java/org/apache/commons/beanutils2/DynaResultSetTestCase.java": [ +83, +104 +], +"commons-beanutils:commons-beanutils:src/test/java/org/apache/commons/beanutils2/DynaRowSetTestCase.java": [ +86, +107 +], +"commons-beanutils:commons-beanutils:src/test/java/org/apache/commons/beanutils2/LazyDynaBeanTestCase.java": [ +77, +87 +], +"commons-beanutils:commons-beanutils:src/test/java/org/apache/commons/beanutils2/LazyDynaClassTestCase.java": [ +56, +71 +], +"commons-beanutils:commons-beanutils:src/test/java/org/apache/commons/beanutils2/LazyDynaListTestCase.java": [ +82, +89 +], +"commons-beanutils:commons-beanutils:src/test/java/org/apache/commons/beanutils2/LazyDynaMapTestCase.java": [ +78, +87 +], +"commons-beanutils:commons-beanutils:src/test/java/org/apache/commons/beanutils2/MappedPropertyTestCase.java": [ +53, +67 +], +"commons-beanutils:commons-beanutils:src/test/java/org/apache/commons/beanutils2/MethodUtilsTestCase.java": [ +56, +71 +], +"commons-beanutils:commons-beanutils:src/test/java/org/apache/commons/beanutils2/PropertyUtilsBenchCase.java": [ +82, +148 +], +"commons-beanutils:commons-beanutils:src/test/java/org/apache/commons/beanutils2/PropertyUtilsTestCase.java": [ +204, +239 +], +"commons-beanutils:commons-beanutils:src/test/java/org/apache/commons/beanutils2/converters/ArrayConverterTestCase.java": [ +52, +57 +], +"commons-beanutils:commons-beanutils:src/test/java/org/apache/commons/beanutils2/converters/BigDecimalConverterTestCase.java": [ +45, +58 +], +"commons-beanutils:commons-beanutils:src/test/java/org/apache/commons/beanutils2/converters/BigIntegerConverterTestCase.java": [ +44, +57 +], +"commons-beanutils:commons-beanutils:src/test/java/org/apache/commons/beanutils2/converters/ByteConverterTestCase.java": [ +43, +56 +], +"commons-beanutils:commons-beanutils:src/test/java/org/apache/commons/beanutils2/converters/CharacterConverterTestCase.java": [ +51, +56 +], +"commons-beanutils:commons-beanutils:src/test/java/org/apache/commons/beanutils2/converters/ClassConverterTestCase.java": [ +51, +56 +], +"commons-beanutils:commons-beanutils:src/test/java/org/apache/commons/beanutils2/converters/DateConverterTestCase.java": [ +50, +55 +], +"commons-beanutils:commons-beanutils:src/test/java/org/apache/commons/beanutils2/converters/DoubleConverterTestCase.java": [ +43, +56 +], +"commons-beanutils:commons-beanutils:src/test/java/org/apache/commons/beanutils2/converters/FileConverterTestCase.java": [ +47, +56 +], +"commons-beanutils:commons-beanutils:src/test/java/org/apache/commons/beanutils2/converters/FloatConverterTestCase.java": [ +43, +56 +], +"commons-beanutils:commons-beanutils:src/test/java/org/apache/commons/beanutils2/converters/IntegerConverterTestCase.java": [ +44, +57 +], +"commons-beanutils:commons-beanutils:src/test/java/org/apache/commons/beanutils2/converters/LongConverterTestCase.java": [ +43, +56 +], +"commons-beanutils:commons-beanutils:src/test/java/org/apache/commons/beanutils2/converters/ShortConverterTestCase.java": [ +43, +56 +], +"commons-beanutils:commons-beanutils:src/test/java/org/apache/commons/beanutils2/converters/URLConverterTestCase.java": [ +47, +56 +], +"commons-beanutils:commons-beanutils:src/test/java/org/apache/commons/beanutils2/locale/LocaleBeanUtilsTestCase.java": [ +52, +69 +], +"commons-beanutils:commons-beanutils:src/test/java/org/apache/commons/beanutils2/locale/LocaleBeanificationTestCase.java": [ +77, +96 +], +"commons-beanutils:commons-beanutils:src/test/java/org/apache/commons/beanutils2/locale/LocaleConvertUtilsTestCase.java": [ +68, +94 +], +"commons-beanutils:commons-beanutils:src/test/java/org/apache/commons/beanutils2/locale/converters/BaseLocaleConverterTestCase.java": [ +77, +120 +] +} diff --git a/its/ruling/src/test/resources/eclipse-jetty-similar-to-main/java-S9343.json b/its/ruling/src/test/resources/eclipse-jetty-similar-to-main/java-S9343.json new file mode 100644 index 00000000000..53df8f25d93 --- /dev/null +++ b/its/ruling/src/test/resources/eclipse-jetty-similar-to-main/java-S9343.json @@ -0,0 +1,51 @@ +{ +"org.eclipse.jetty:jetty-project:jetty-io/src/main/java/org/eclipse/jetty/io/ByteArrayEndPoint.java": [ +127, +137, +448 +], +"org.eclipse.jetty:jetty-project:jetty-io/src/main/java/org/eclipse/jetty/io/SocketChannelEndPoint.java": [ +200 +], +"org.eclipse.jetty:jetty-project:jetty-io/src/main/java/org/eclipse/jetty/io/ssl/SslConnection.java": [ +384, +1240, +1347 +], +"org.eclipse.jetty:jetty-project:jetty-io/src/test/java/org/eclipse/jetty/io/SocketChannelEndPointTest.java": [ +711 +], +"org.eclipse.jetty:jetty-project:jetty-jmx/src/main/java/org/eclipse/jetty/jmx/ConnectorServer.java": [ +115, +164 +], +"org.eclipse.jetty:jetty-project:jetty-server/src/main/java/org/eclipse/jetty/server/HttpConnection.java": [ +757, +921 +], +"org.eclipse.jetty:jetty-project:jetty-server/src/main/java/org/eclipse/jetty/server/HttpOutput.java": [ +1516 +], +"org.eclipse.jetty:jetty-project:jetty-server/src/main/java/org/eclipse/jetty/server/ServerConnector.java": [ +380 +], +"org.eclipse.jetty:jetty-project:jetty-server/src/main/java/org/eclipse/jetty/server/handler/ResourceHandler.java": [ +98 +], +"org.eclipse.jetty:jetty-project:jetty-server/src/main/java/org/eclipse/jetty/server/handler/gzip/GzipHttpInputInterceptor.java": [ +82 +], +"org.eclipse.jetty:jetty-project:jetty-server/src/main/java/org/eclipse/jetty/server/session/DefaultSessionCache.java": [ +91, +102, +192 +], +"org.eclipse.jetty:jetty-project:jetty-server/src/main/java/org/eclipse/jetty/server/session/NullSessionCache.java": [ +66, +73, +80 +], +"org.eclipse.jetty:jetty-project:jetty-server/src/test/java/org/eclipse/jetty/server/HttpInputAsyncStateTest.java": [ +138 +] +} diff --git a/its/ruling/src/test/resources/eclipse-jetty/java-S9343.json b/its/ruling/src/test/resources/eclipse-jetty/java-S9343.json new file mode 100644 index 00000000000..6b7a3625deb --- /dev/null +++ b/its/ruling/src/test/resources/eclipse-jetty/java-S9343.json @@ -0,0 +1,81 @@ +{ +"org.eclipse.jetty:jetty-project:jetty-io/src/main/java/org/eclipse/jetty/io/ByteArrayEndPoint.java": [ +127, +137, +448 +], +"org.eclipse.jetty:jetty-project:jetty-io/src/main/java/org/eclipse/jetty/io/SocketChannelEndPoint.java": [ +200 +], +"org.eclipse.jetty:jetty-project:jetty-io/src/main/java/org/eclipse/jetty/io/ssl/SslConnection.java": [ +384, +1240, +1347 +], +"org.eclipse.jetty:jetty-project:jetty-io/src/test/java/org/eclipse/jetty/io/SocketChannelEndPointTest.java": [ +711 +], +"org.eclipse.jetty:jetty-project:jetty-jmx/src/main/java/org/eclipse/jetty/jmx/ConnectorServer.java": [ +115, +164 +], +"org.eclipse.jetty:jetty-project:jetty-server/src/main/java/org/eclipse/jetty/server/HttpConnection.java": [ +757, +921 +], +"org.eclipse.jetty:jetty-project:jetty-server/src/main/java/org/eclipse/jetty/server/HttpOutput.java": [ +1516 +], +"org.eclipse.jetty:jetty-project:jetty-server/src/main/java/org/eclipse/jetty/server/ServerConnector.java": [ +380 +], +"org.eclipse.jetty:jetty-project:jetty-server/src/main/java/org/eclipse/jetty/server/handler/ResourceHandler.java": [ +98 +], +"org.eclipse.jetty:jetty-project:jetty-server/src/main/java/org/eclipse/jetty/server/handler/gzip/GzipHttpInputInterceptor.java": [ +82 +], +"org.eclipse.jetty:jetty-project:jetty-server/src/main/java/org/eclipse/jetty/server/session/DefaultSessionCache.java": [ +91, +102, +192 +], +"org.eclipse.jetty:jetty-project:jetty-server/src/main/java/org/eclipse/jetty/server/session/NullSessionCache.java": [ +66, +73, +80 +], +"org.eclipse.jetty:jetty-project:jetty-server/src/test/java/org/eclipse/jetty/server/HttpInputAsyncStateTest.java": [ +138 +], +"org.eclipse.jetty:jetty-project:jetty-server/src/test/java/org/eclipse/jetty/server/ProxyConnectionTest.java": [ +376, +395 +], +"org.eclipse.jetty:jetty-project:jetty-util/src/main/java/org/eclipse/jetty/util/ClassLoadingObjectInputStream.java": [ +73 +], +"org.eclipse.jetty:jetty-project:jetty-util/src/main/java/org/eclipse/jetty/util/Scanner.java": [ +545, +611 +], +"org.eclipse.jetty:jetty-project:jetty-util/src/main/java/org/eclipse/jetty/util/Utf8StringBuffer.java": [ +56 +], +"org.eclipse.jetty:jetty-project:jetty-util/src/main/java/org/eclipse/jetty/util/Utf8StringBuilder.java": [ +56 +], +"org.eclipse.jetty:jetty-project:jetty-util/src/main/java/org/eclipse/jetty/util/compression/CompressionPool.java": [ +118 +], +"org.eclipse.jetty:jetty-project:jetty-util/src/main/java/org/eclipse/jetty/util/security/Constraint.java": [ +106 +], +"org.eclipse.jetty:jetty-project:jetty-util/src/main/java/org/eclipse/jetty/util/thread/ReservedThreadExecutor.java": [ +155, +163 +], +"org.eclipse.jetty:jetty-project:jetty-util/src/test/java/org/eclipse/jetty/util/IteratingCallbackTest.java": [ +319 +] +} diff --git a/its/ruling/src/test/resources/guava/java-S9343.json b/its/ruling/src/test/resources/guava/java-S9343.json new file mode 100644 index 00000000000..ff009fa1532 --- /dev/null +++ b/its/ruling/src/test/resources/guava/java-S9343.json @@ -0,0 +1,77 @@ +{ +"com.google.guava:guava:src/com/google/common/base/Splitter.java": [ +184, +200, +236, +241, +299, +305 +], +"com.google.guava:guava:src/com/google/common/collect/AbstractMapBasedMultimap.java": [ +1270 +], +"com.google.guava:guava:src/com/google/common/collect/ConcurrentHashMultiset.java": [ +491 +], +"com.google.guava:guava:src/com/google/common/collect/ConsumingQueueIterator.java": [ +43 +], +"com.google.guava:guava:src/com/google/common/collect/ForwardingNavigableMap.java": [ +284 +], +"com.google.guava:guava:src/com/google/common/collect/Maps.java": [ +776, +823, +2724, +2756 +], +"com.google.guava:guava:src/com/google/common/collect/Multimaps.java": [ +128, +210, +288, +368, +599, +620, +1687 +], +"com.google.guava:guava:src/com/google/common/collect/Sets.java": [ +723 +], +"com.google.guava:guava:src/com/google/common/collect/SortedLists.java": [ +115, +126, +156, +174 +], +"com.google.guava:guava:src/com/google/common/collect/StandardTable.java": [ +714, +779 +], +"com.google.guava:guava:src/com/google/common/hash/Crc32cHashFunction.java": [ +112 +], +"com.google.guava:guava:src/com/google/common/hash/Murmur3_128HashFunction.java": [ +163 +], +"com.google.guava:guava:src/com/google/common/hash/Murmur3_32HashFunction.java": [ +184 +], +"com.google.guava:guava:src/com/google/common/hash/SipHashFunction.java": [ +146 +], +"com.google.guava:guava:src/com/google/common/reflect/TypeResolver.java": [ +241 +], +"com.google.guava:guava:src/com/google/common/util/concurrent/AbstractScheduledService.java": [ +128, +149 +], +"com.google.guava:guava:src/com/google/common/util/concurrent/Futures.java": [ +2085 +], +"com.google.guava:guava:src/com/google/common/util/concurrent/SettableFuture.java": [ +48, +52, +58 +] +} diff --git a/its/ruling/src/test/resources/sonar-server/java-S9343.json b/its/ruling/src/test/resources/sonar-server/java-S9343.json new file mode 100644 index 00000000000..129656c98e6 --- /dev/null +++ b/its/ruling/src/test/resources/sonar-server/java-S9343.json @@ -0,0 +1,21 @@ +{ +"org.sonarsource.sonarqube:sonar-server:src/main/java/org/sonar/server/computation/task/projectanalysis/formula/coverage/LinesAndConditionsWithUncoveredVariationCounter.java": [ +34 +], +"org.sonarsource.sonarqube:sonar-server:src/main/java/org/sonar/server/platform/Platform.java": [ +115 +], +"org.sonarsource.sonarqube:sonar-server:src/main/java/org/sonar/server/platform/platformlevel/PlatformLevel1.java": [ +73 +], +"org.sonarsource.sonarqube:sonar-server:src/main/java/org/sonar/server/plugins/StaticResourcesServlet.java": [ +50 +], +"org.sonarsource.sonarqube:sonar-server:src/main/java/org/sonar/server/ws/ServletRequest.java": [ +91 +], +"org.sonarsource.sonarqube:sonar-server:src/test/java/org/sonar/server/platform/ServerTesterPlatform.java": [ +33, +41 +] +} diff --git a/java-checks-test-sources/default/src/main/files/non-compiling/checks/MethodOverrideAccessibilityCheckSample.java b/java-checks-test-sources/default/src/main/files/non-compiling/checks/MethodOverrideAccessibilityCheckSample.java new file mode 100644 index 00000000000..3dbd0d1503b --- /dev/null +++ b/java-checks-test-sources/default/src/main/files/non-compiling/checks/MethodOverrideAccessibilityCheckSample.java @@ -0,0 +1,49 @@ +package checks; + +class MethodOverrideAccessibilityCheckSample { + + // --- Compliant: unknown parameter types should not trigger hiding detection --- + + static class ParentWithUnknownParam { + static void process(Unknown param) {} + } + + static class ChildWithUnknownParam extends ParentWithUnknownParam { + public static void process(Unknown param) {} // Compliant - parameter type is unknown + } + + // --- Noncompliant: hiding still detected when class has partial semantics --- + + static class ParentWithKnownStatic { + protected static void compute(int x) {} + } + + static class ChildHidingWithUnknownField extends ParentWithKnownStatic { + Unknown field; + public static void compute(int x) {} // Noncompliant {{Increase of accessibility from "protected" to "public" when hiding method.}} + } + + // --- Compliant: override from unknown parent type --- + + static class ChildOfUnknownParent extends UnknownParent { + @Override + public void doSomething() {} // Compliant - parent is unknown, no overridden symbols resolved + } + + // --- Compliant: static method with same name as instance method in superclass (compile error in Java) --- + + static class ParentWithInstanceMethod { + protected void process() {} + } + + static class ChildStaticSameAsInstance extends ParentWithInstanceMethod { + public static void process() {} // Compliant - parent method is not static + } + + // --- Compliant: static method in class extending unknown parent --- + + static class StaticChildOfUnknownParent extends UnknownParent { + public static void staticMethod() {} // Compliant - unknown superclass, no hiding detected + } + +} diff --git a/java-checks-test-sources/default/src/main/java/checks/MethodOverrideAccessibilityCheckSample.java b/java-checks-test-sources/default/src/main/java/checks/MethodOverrideAccessibilityCheckSample.java new file mode 100644 index 00000000000..69583507a20 --- /dev/null +++ b/java-checks-test-sources/default/src/main/java/checks/MethodOverrideAccessibilityCheckSample.java @@ -0,0 +1,371 @@ +package checks; + +class MethodOverrideAccessibilityCheckSample { + + // --- Instance override: protected -> public --- + + static class Parent1 { + protected void process() {} +// ^^^^^^^> + } + + static class ChildProtectedToPublic extends Parent1 { + @Override + public void process() {} // Noncompliant {{Increase of accessibility from "protected" to "public" when overriding method.}} +// ^^^^^^^ + } + + // --- Instance override: package-private -> public --- + + static class Parent2 { + void handle() {} +// ^^^^^^> + } + + static class ChildPackageToPublic extends Parent2 { + @Override + public void handle() {} // Noncompliant {{Increase of accessibility from "package-private" to "public" when overriding method.}} +// ^^^^^^ + } + + // --- Instance override: package-private -> protected --- + + static class Parent3 { + void doWork() {} +// ^^^^^^> + } + + static class ChildPackageToProtected extends Parent3 { + @Override + protected void doWork() {} // Noncompliant {{Increase of accessibility from "package-private" to "protected" when overriding method.}} +// ^^^^^^ + } + + // --- Static hiding: protected -> public --- + + static class Parent4 { + protected static void compute(int x) {} +// ^^^^^^^> + } + + static class ChildStaticProtectedToPublic extends Parent4 { + public static void compute(int x) {} // Noncompliant {{Increase of accessibility from "protected" to "public" when hiding method.}} +// ^^^^^^^ + } + + // --- Static hiding: package-private -> protected --- + + static class Parent5 { + static void resolve(String s) {} +// ^^^^^^^> + } + + static class ChildStaticPackageToProtected extends Parent5 { + protected static void resolve(String s) {} // Noncompliant {{Increase of accessibility from "package-private" to "protected" when hiding method.}} +// ^^^^^^^ + } + + // --- Static hiding: package-private -> public --- + + static class Parent6 { + static void transform(int y) {} +// ^^^^^^^^^> + } + + static class ChildStaticPackageToPublic extends Parent6 { + public static void transform(int y) {} // Noncompliant {{Increase of accessibility from "package-private" to "public" when hiding method.}} +// ^^^^^^^^^ + } + + // --- Abstract method implementation with increased access --- + + static abstract class AbstractParent1 { + abstract void doTask(); +// ^^^^^^> + } + + static class ConcretePackageToProtected extends AbstractParent1 { + @Override + protected void doTask() {} // Noncompliant {{Increase of accessibility from "package-private" to "protected" when overriding method.}} +// ^^^^^^ + } + + static abstract class AbstractParent2 { + abstract void perform(); +// ^^^^^^^> + } + + static class ConcretePackageToPublic extends AbstractParent2 { + @Override + public void perform() {} // Noncompliant {{Increase of accessibility from "package-private" to "public" when overriding method.}} +// ^^^^^^^ + } + + static abstract class AbstractParent3 { + protected abstract void execute(); +// ^^^^^^^> + } + + static class ConcreteProtectedToPublic extends AbstractParent3 { + @Override + public void execute() {} // Noncompliant {{Increase of accessibility from "protected" to "public" when overriding method.}} +// ^^^^^^^ + } + + // --- Multi-level hierarchy --- + + static class GrandParent { + protected void action() {} +// ^^^^^^> + } + + static class MiddleClass extends GrandParent { + @Override + protected void action() {} // Compliant - same access level + } + + static class GrandChild extends GrandParent { + @Override + public void action() {} // Noncompliant {{Increase of accessibility from "protected" to "public" when overriding method.}} +// ^^^^^^ + } + + // GrandChild via already-widened parent: compliant since direct parent is public + static class AlreadyPublicParent { + public void method() {} + } + + static class GrandChildCompliant extends AlreadyPublicParent { + @Override + public void method() {} // Compliant - same as direct parent + } + + // --- Compliant: same access level maintained --- + + static class Parent7 { + protected void keep() {} + } + + static class SameProtected extends Parent7 { + @Override + protected void keep() {} // Compliant + } + + static class Parent8 { + void maintain() {} + } + + static class SamePackagePrivate extends Parent8 { + @Override + void maintain() {} // Compliant + } + + static class Parent9 { + public void stay() {} + } + + static class SamePublic extends Parent9 { + @Override + public void stay() {} // Compliant + } + + // --- Compliant: same access level for static hiding --- + + static class Parent10 { + protected static void staticKeep(int x) {} + } + + static class SameStaticProtected extends Parent10 { + protected static void staticKeep(int x) {} // Compliant + } + + // --- Compliant: interface implementation --- + + interface Processor { + void run(); + } + + static class ProcessorImpl implements Processor { + @Override + public void run() {} // Compliant - interface methods are implicitly public + } + + // --- Compliant: constructors --- + + static class ParentWithConstructor { + protected ParentWithConstructor() {} + } + + static class ChildWithConstructor extends ParentWithConstructor { + public ChildWithConstructor() {} // Compliant - constructors don't override + } + + // --- Compliant: method not overriding anything --- + + static class Parent11 { + protected void existing() {} + } + + static class NewMethod extends Parent11 { + public void newMethod() {} // Compliant - not overriding + } + + // --- Compliant: private methods cannot be overridden --- + + static class BaseWithPrivate { + private void secret() {} + } + + static class ChildOfPrivate extends BaseWithPrivate { + public void secret() {} // Compliant - not overriding, private methods are not visible + } + + // --- Superclass override with protected -> public --- + + static class SuperWithProtected { + protected void execute2() {} +// ^^^^^^^^> + } + + static class ChildImplementsAndOverrides extends SuperWithProtected { + @Override + public void execute2() {} // Noncompliant {{Increase of accessibility from "protected" to "public" when overriding method.}} +// ^^^^^^^^ + } + + // --- Compliant: static method with different parameter types (no hiding match) --- + + static class ParentStaticDiffParams { + static void process(int x) {} + } + + static class ChildStaticDiffParams extends ParentStaticDiffParams { + public static void process(String x) {} // Compliant - different parameter types, not hiding + } + + // --- Compliant: static method with private parent (not inherited) --- + + static class ParentStaticPrivate { + private static void hidden() {} + } + + static class ChildStaticPrivate extends ParentStaticPrivate { + public static void hidden() {} // Compliant - private parent method not inherited + } + + // --- Static hiding: protected -> public in multi-level hierarchy --- + + static class StaticGrandParent { + protected static void calc(int x) {} +// ^^^^> + } + + static class StaticMiddle extends StaticGrandParent { + protected static void calc(int x) {} // Compliant - same access + } + + static class StaticGrandChild extends StaticGrandParent { + public static void calc(int x) {} // Noncompliant {{Increase of accessibility from "protected" to "public" when hiding method.}} +// ^^^^ + } + + // --- Noncompliant: overriding bytecode parent (no source declaration available) --- + + static class CustomClassLoader extends ClassLoader { + @Override + public Class findClass(String name) throws ClassNotFoundException { // Noncompliant {{Increase of accessibility from "protected" to "public" when overriding method.}} +// ^^^^^^^^^ + return super.findClass(name); + } + } + + // --- Compliant: overriding bytecode parent with same access --- + + static class CompliantClassLoader extends ClassLoader { + @Override + protected Class findClass(String name) throws ClassNotFoundException { // Compliant - same access + return super.findClass(name); + } + } + + // --- Compliant: static method with same name as a field in superclass --- + + static class ParentWithField { + protected int compute; + } + + static class ChildStaticSameAsField extends ParentWithField { + public static void compute() {} // Compliant - parent has a field, not a method + } + + // --- Compliant: static method with no-arg hiding parent with params --- + + static class ParentStaticWithParams { + protected static void action(int x, String y) {} + } + + static class ChildStaticNoParams extends ParentStaticWithParams { + public static void action() {} // Compliant - different parameter count + } + + // --- Instance override: protected -> public through interface + class hierarchy --- + + interface Doer { + void doIt(); + } + + static class AbstractDoer { + protected void doIt() {} +// ^^^^> + } + + static class ConcreteDoer extends AbstractDoer implements Doer { + @Override + public void doIt() {} // Noncompliant {{Increase of accessibility from "protected" to "public" when overriding method.}} +// ^^^^ + } + + // --- Compliant: static method hiding same access level (protected -> protected) --- + + static class StaticParentSameAccess { + protected static void safeMethod(int x) {} + } + + static class StaticChildSameAccess extends StaticParentSameAccess { + protected static void safeMethod(int x) {} // Compliant - same access level + } + + // --- Compliant: static method hiding with reduced access (public -> protected, compiler error normally) --- + // Note: this case would normally be a compiler error, but covered as non-compiling test + + // --- Noncompliant: static hiding package-private -> public through multiple levels --- + + static class DeepStaticBase { + static void deepMethod() {} +// ^^^^^^^^^^> + } + + static class DeepStaticMiddle extends DeepStaticBase { + // does not redefine deepMethod + } + + static class DeepStaticChild extends DeepStaticMiddle { + public static void deepMethod() {} // Noncompliant {{Increase of accessibility from "package-private" to "public" when hiding method.}} +// ^^^^^^^^^^ + } + + // --- Compliant: instance method override with same access through multiple interfaces --- + + interface InterfaceA { + void doAction(); + } + + interface InterfaceB { + void doAction(); + } + + static class ImplBothInterfaces implements InterfaceA, InterfaceB { + @Override + public void doAction() {} // Compliant - implementing interface methods + } +} diff --git a/java-checks/src/main/java/org/sonar/java/checks/MethodOverrideAccessibilityCheck.java b/java-checks/src/main/java/org/sonar/java/checks/MethodOverrideAccessibilityCheck.java new file mode 100644 index 00000000000..9a025c0415a --- /dev/null +++ b/java-checks/src/main/java/org/sonar/java/checks/MethodOverrideAccessibilityCheck.java @@ -0,0 +1,106 @@ +/* + * SonarQube Java + * Copyright (C) SonarSource Sàrl + * mailto:info AT sonarsource DOT com + * + * You can redistribute and/or modify this program under the terms of + * the Sonar Source-Available License Version 1, as published by SonarSource Sàrl. + * + * This program 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 Sonar Source-Available License for more details. + * + * You should have received a copy of the Sonar Source-Available License + * along with this program; if not, see https://sonarsource.com/license/ssal/ + */ +package org.sonar.java.checks; + +import java.util.Collections; +import java.util.List; +import org.sonar.check.Rule; +import org.sonar.java.checks.helpers.MethodTreeUtils; +import org.sonar.java.model.JUtils; +import org.sonar.plugins.java.api.IssuableSubscriptionVisitor; +import org.sonar.plugins.java.api.JavaFileScannerContext; +import org.sonar.plugins.java.api.semantic.Symbol; +import org.sonar.plugins.java.api.tree.MethodTree; +import org.sonar.plugins.java.api.tree.Tree; + +@Rule(key = "S9343") +public class MethodOverrideAccessibilityCheck extends IssuableSubscriptionVisitor { + + @Override + public List nodesToVisit() { + return Collections.singletonList(Tree.Kind.METHOD); + } + + @Override + public void visitNode(Tree tree) { + if (context.getSemanticModel() == null) { + return; + } + MethodTree methodTree = (MethodTree) tree; + Symbol.MethodSymbol methodSymbol = methodTree.symbol(); + if (methodSymbol.isStatic()) { + checkStaticMethodHiding(methodTree, methodSymbol); + } else { + checkInstanceMethodOverride(methodTree, methodSymbol); + } + } + + private void checkInstanceMethodOverride(MethodTree methodTree, Symbol.MethodSymbol methodSymbol) { + List overriddenSymbols = methodSymbol.overriddenSymbols(); + overriddenSymbols.stream() + .filter(s -> !s.owner().isInterface()) + .findFirst() + .ifPresent(overriddenSymbol -> reportIfAccessIncreased(methodTree, methodSymbol, overriddenSymbol, "overriding")); + } + + private void checkStaticMethodHiding(MethodTree methodTree, Symbol.MethodSymbol methodSymbol) { + MethodTreeUtils.findHiddenStaticMethod(methodSymbol, + symbol -> !(symbol.isPackageVisibility() && !samePackage(methodSymbol, symbol))) + .ifPresent(hidden -> reportIfAccessIncreased(methodTree, methodSymbol, hidden, "hiding")); + } + + private static boolean samePackage(Symbol s1, Symbol s2) { + return JUtils.getPackage(s1).equals(JUtils.getPackage(s2)); + } + + private void reportIfAccessIncreased(MethodTree methodTree, Symbol childMethod, Symbol.MethodSymbol parentMethod, String verb) { + int childLevel = accessLevel(childMethod); + int parentLevel = accessLevel(parentMethod); + if (parentLevel < 0 || childLevel < 0 || childLevel <= parentLevel) { + return; + } + String message = String.format("Increase of accessibility from \"%s\" to \"%s\" when %s method.", + accessLevelName(parentLevel), accessLevelName(childLevel), verb); + MethodTree declaration = parentMethod.declaration(); + if (declaration != null) { + reportIssue(methodTree.simpleName(), message, + Collections.singletonList(new JavaFileScannerContext.Location("Parent method", declaration.simpleName())), + null); + } else { + reportIssue(methodTree.simpleName(), message); + } + } + + private static int accessLevel(Symbol symbol) { + if (symbol.isPrivate()) return 0; + if (symbol.isPackageVisibility()) return 1; + if (symbol.isProtected()) return 2; + if (symbol.isPublic()) return 3; + return -1; + } + + private static String accessLevelName(int level) { + return switch (level) { + case 0 -> "private"; + case 1 -> "package-private"; + case 2 -> "protected"; + case 3 -> "public"; + default -> "unknown"; + }; + } + +} diff --git a/java-checks/src/main/java/org/sonar/java/checks/StaticMethodHidingCheck.java b/java-checks/src/main/java/org/sonar/java/checks/StaticMethodHidingCheck.java index 858900a4ce6..74abc32bb80 100644 --- a/java-checks/src/main/java/org/sonar/java/checks/StaticMethodHidingCheck.java +++ b/java-checks/src/main/java/org/sonar/java/checks/StaticMethodHidingCheck.java @@ -19,10 +19,10 @@ import java.util.Collections; import java.util.List; import org.sonar.check.Rule; +import org.sonar.java.checks.helpers.MethodTreeUtils; import org.sonar.plugins.java.api.IssuableSubscriptionVisitor; import org.sonar.plugins.java.api.JavaFileScannerContext; import org.sonar.plugins.java.api.semantic.Symbol; -import org.sonar.plugins.java.api.semantic.Type; import org.sonar.plugins.java.api.tree.MethodTree; import org.sonar.plugins.java.api.tree.Tree; @@ -44,25 +44,8 @@ public void visitNode(Tree tree) { if (!methodSymbol.isStatic() || isIntentionalHiding(methodSymbol)) { return; } - Symbol.TypeSymbol owner = (Symbol.TypeSymbol) methodSymbol.owner(); - Type superClass = owner.superClass(); - while (superClass != null) { - if (checkHiding(methodTree, methodSymbol, superClass)) { - return; - } - superClass = superClass.symbol().superClass(); - } - } - - private boolean checkHiding(MethodTree methodTree, Symbol.MethodSymbol methodSymbol, Type superClass) { - for (Symbol symbol : superClass.symbol().lookupSymbols(methodSymbol.name())) { - if (symbol.isMethodSymbol() && symbol.isStatic() && !symbol.isPrivate() - && hasSameParameterTypes(methodSymbol, (Symbol.MethodSymbol) symbol)) { - reportHidingIssue(methodTree, methodSymbol, (Symbol.MethodSymbol) symbol); - return true; - } - } - return false; + MethodTreeUtils.findHiddenStaticMethod(methodSymbol, symbol -> true) + .ifPresent(hidden -> reportHidingIssue(methodTree, methodSymbol, hidden)); } private void reportHidingIssue(MethodTree methodTree, Symbol.MethodSymbol methodSymbol, Symbol.MethodSymbol hiddenMethod) { @@ -88,23 +71,4 @@ private static boolean isIntentionalHiding(Symbol.MethodSymbol methodSymbol) { || methodSymbol.metadata().isAnnotatedWith("org.jetbrains.annotations.ApiStatus$ScheduledForRemoval"); } - private static boolean hasSameParameterTypes(Symbol.MethodSymbol method, Symbol.MethodSymbol candidate) { - List methodParams = method.parameterTypes(); - List candidateParams = candidate.parameterTypes(); - if (methodParams.size() != candidateParams.size()) { - return false; - } - for (int i = 0; i < methodParams.size(); i++) { - Type methodParam = methodParams.get(i); - Type candidateParam = candidateParams.get(i); - if (methodParam.isUnknown() || candidateParam.isUnknown()) { - return false; - } - if (!methodParam.erasure().equals(candidateParam.erasure())) { - return false; - } - } - return true; - } - } diff --git a/java-checks/src/main/java/org/sonar/java/checks/helpers/MethodTreeUtils.java b/java-checks/src/main/java/org/sonar/java/checks/helpers/MethodTreeUtils.java index 2837674eadb..ab0b0786bd9 100644 --- a/java-checks/src/main/java/org/sonar/java/checks/helpers/MethodTreeUtils.java +++ b/java-checks/src/main/java/org/sonar/java/checks/helpers/MethodTreeUtils.java @@ -27,6 +27,7 @@ import org.sonar.plugins.java.api.JavaVersion; import org.sonar.plugins.java.api.semantic.MethodMatchers; import org.sonar.plugins.java.api.semantic.Symbol; +import org.sonar.plugins.java.api.semantic.Type; import org.sonar.plugins.java.api.tree.Arguments; import org.sonar.plugins.java.api.tree.ArrayTypeTree; import org.sonar.plugins.java.api.tree.BaseTreeVisitor; @@ -259,6 +260,42 @@ public void visitLambdaExpression(LambdaExpressionTree lambdaExpressionTree) { } } + public static Optional findHiddenStaticMethod(Symbol.MethodSymbol methodSymbol, + Predicate additionalFilter) { + Symbol.TypeSymbol owner = (Symbol.TypeSymbol) methodSymbol.owner(); + Type superClass = owner.superClass(); + while (superClass != null) { + for (Symbol symbol : superClass.symbol().lookupSymbols(methodSymbol.name())) { + if (symbol.isMethodSymbol() && symbol.isStatic() && !symbol.isPrivate() + && additionalFilter.test(symbol) + && hasSameParameterTypes(methodSymbol, (Symbol.MethodSymbol) symbol)) { + return Optional.of((Symbol.MethodSymbol) symbol); + } + } + superClass = superClass.symbol().superClass(); + } + return Optional.empty(); + } + + public static boolean hasSameParameterTypes(Symbol.MethodSymbol method, Symbol.MethodSymbol candidate) { + List methodParams = method.parameterTypes(); + List candidateParams = candidate.parameterTypes(); + if (methodParams.size() != candidateParams.size()) { + return false; + } + for (int i = 0; i < methodParams.size(); i++) { + Type methodParam = methodParams.get(i); + Type candidateParam = candidateParams.get(i); + if (methodParam.isUnknown() || candidateParam.isUnknown()) { + return false; + } + if (!methodParam.erasure().equals(candidateParam.erasure())) { + return false; + } + } + return true; + } + public static Optional isGetterLike(Symbol.MethodSymbol methodSymbol) { if (!methodSymbol.parameterTypes().isEmpty() || isPrivateStaticOrAbstract(methodSymbol)) { return Optional.empty(); diff --git a/java-checks/src/test/java/org/sonar/java/checks/MethodOverrideAccessibilityCheckTest.java b/java-checks/src/test/java/org/sonar/java/checks/MethodOverrideAccessibilityCheckTest.java new file mode 100644 index 00000000000..fabe4d23b58 --- /dev/null +++ b/java-checks/src/test/java/org/sonar/java/checks/MethodOverrideAccessibilityCheckTest.java @@ -0,0 +1,52 @@ +/* + * SonarQube Java + * Copyright (C) SonarSource Sàrl + * mailto:info AT sonarsource DOT com + * + * You can redistribute and/or modify this program under the terms of + * the Sonar Source-Available License Version 1, as published by SonarSource Sàrl. + * + * This program 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 Sonar Source-Available License for more details. + * + * You should have received a copy of the Sonar Source-Available License + * along with this program; if not, see https://sonarsource.com/license/ssal/ + */ +package org.sonar.java.checks; + +import org.junit.jupiter.api.Test; +import org.sonar.java.checks.verifier.CheckVerifier; + +import static org.sonar.java.checks.verifier.TestUtils.mainCodeSourcesPath; +import static org.sonar.java.checks.verifier.TestUtils.nonCompilingTestSourcesPath; + +class MethodOverrideAccessibilityCheckTest { + + @Test + void test() { + CheckVerifier.newVerifier() + .onFile(mainCodeSourcesPath("checks/MethodOverrideAccessibilityCheckSample.java")) + .withCheck(new MethodOverrideAccessibilityCheck()) + .verifyIssues(); + } + + @Test + void withoutSemantic() { + CheckVerifier.newVerifier() + .onFile(mainCodeSourcesPath("checks/MethodOverrideAccessibilityCheckSample.java")) + .withCheck(new MethodOverrideAccessibilityCheck()) + .withoutSemantic() + .verifyNoIssues(); + } + + @Test + void test_non_compiling() { + CheckVerifier.newVerifier() + .onFile(nonCompilingTestSourcesPath("checks/MethodOverrideAccessibilityCheckSample.java")) + .withCheck(new MethodOverrideAccessibilityCheck()) + .verifyIssues(); + } + +} diff --git a/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S9343.html b/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S9343.html new file mode 100644 index 00000000000..4761e132f60 --- /dev/null +++ b/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S9343.html @@ -0,0 +1,59 @@ +

This rule raises an issue when a method in a subclass has broader accessibility than the method it overrides in the parent class.

+

Why is this an issue?

+

In object-oriented languages, access modifiers control the visibility of methods, typically ranging from most restrictive (accessible only within the +defining class) through intermediate levels (accessible within packages or to subclasses) to most accessible (publicly available to all code).

+

When a subclass overrides or redefines a method from a parent class, it should not make the method more accessible than it was in the parent class. +Doing so breaks the encapsulation principle and can lead to several problems:

+
    +
  • Exposes internal implementation: A method that was intentionally kept at a restricted visibility level in the parent class may + have been designed for internal use only. Making it more broadly accessible in a subclass exposes internal details that clients shouldn't depend + on.
    • +
  • Violates the Liskov Substitution Principle: This principle states that objects of a subclass should be substitutable for objects + of the parent class without breaking the application. When you increase accessibility, you're adding new capabilities to the subclass's public API + that weren't part of the parent class's contract.
    • +
  • Creates maintenance issues: If the parent class's method accessibility was intentionally limited, increasing it in a subclass + makes it harder to maintain and evolve the class hierarchy.
    • +
+

In Java, this applies both to instance methods (which are overridden) and static methods (which are hidden).

+

How to fix it

+

Keep the same access modifier as the parent class method. If the method needs to be more accessible, reconsider whether the parent class design +should be changed instead.

+

Code examples

+

Noncompliant code example

+
+class Parent {
+    protected void process() {
+        // Internal processing logic
+    }
+}
+
+class Child extends Parent {
+    @Override
+    public void process() { // Noncompliant - increased from protected to public
+        super.process();
+    }
+}
+
+

Compliant solution

+
+class Parent {
+    protected void process() {
+        // Internal processing logic
+    }
+}
+
+class Child extends Parent {
+    @Override
+    protected void process() { // Compliant - same access level
+        super.process();
+    }
+}
+
+

Resources

+

Documentation

+ diff --git a/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S9343.json b/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S9343.json new file mode 100644 index 00000000000..8b947703b87 --- /dev/null +++ b/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S9343.json @@ -0,0 +1,24 @@ +{ + "title": "Methods should not increase accessibility when overriding or hiding", + "type": "CODE_SMELL", + "status": "ready", + "remediation": { + "func": "Constant\/Issue", + "constantCost": "5min" + }, + "tags": [ + "design", + "inheritance" + ], + "defaultSeverity": "Major", + "ruleSpecification": "RSPEC-9343", + "sqKey": "S9343", + "scope": "All", + "quickfix": "unknown", + "code": { + "impacts": { + "MAINTAINABILITY": "MEDIUM" + }, + "attribute": "CLEAR" + } +} diff --git a/sonar-java-plugin/src/main/resources/profiles/Sonar_way/S9343 b/sonar-java-plugin/src/main/resources/profiles/Sonar_way/S9343 new file mode 100644 index 00000000000..e69de29bb2d