diff --git a/its/ruling/src/test/resources/commons-beanutils/java-S9357.json b/its/ruling/src/test/resources/commons-beanutils/java-S9357.json new file mode 100644 index 00000000000..8dec2c78df4 --- /dev/null +++ b/its/ruling/src/test/resources/commons-beanutils/java-S9357.json @@ -0,0 +1,5 @@ +{ +"commons-beanutils:commons-beanutils:src/test/java/org/apache/commons/beanutils2/bugs/Jira509TestCase.java": [ +60 +] +} diff --git a/its/ruling/src/test/resources/eclipse-jetty/java-S9357.json b/its/ruling/src/test/resources/eclipse-jetty/java-S9357.json new file mode 100644 index 00000000000..a234f34288b --- /dev/null +++ b/its/ruling/src/test/resources/eclipse-jetty/java-S9357.json @@ -0,0 +1,47 @@ +{ +"org.eclipse.jetty:jetty-project:jetty-server/src/test/java/org/eclipse/jetty/server/RequestTest.java": [ +150, +203, +238, +264, +295, +331, +526, +562, +707, +764, +1600, +1702, +1748, +1998 +], +"org.eclipse.jetty:jetty-project:jetty-server/src/test/java/org/eclipse/jetty/server/ssl/SSLEngineTest.java": [ +285 +], +"org.eclipse.jetty:jetty-project:jetty-util/src/test/java/org/eclipse/jetty/util/IteratingCallbackTest.java": [ +219, +293, +301 +], +"org.eclipse.jetty:jetty-project:jetty-util/src/test/java/org/eclipse/jetty/util/SharedBlockingCallbackTest.java": [ +77, +139, +177 +], +"org.eclipse.jetty:jetty-project:jetty-util/src/test/java/org/eclipse/jetty/util/thread/SchedulerTest.java": [ +85, +107, +123, +144, +165, +188, +202 +], +"org.eclipse.jetty:jetty-project:jetty-util/src/test/java/org/eclipse/jetty/util/thread/SweeperTest.java": [ +77, +113 +], +"org.eclipse.jetty:jetty-project:jetty-util/src/test/java/org/eclipse/jetty/util/thread/strategy/ExecutionStrategyTest.java": [ +188 +] +} diff --git a/its/ruling/src/test/resources/sonar-server/java-S9357.json b/its/ruling/src/test/resources/sonar-server/java-S9357.json new file mode 100644 index 00000000000..ab90c0f4e03 --- /dev/null +++ b/its/ruling/src/test/resources/sonar-server/java-S9357.json @@ -0,0 +1,79 @@ +{ +"org.sonarsource.sonarqube:sonar-server:src/test/java/org/sonar/server/computation/task/container/TaskContainerImplTest.java": [ +64 +], +"org.sonarsource.sonarqube:sonar-server:src/test/java/org/sonar/server/computation/task/projectanalysis/component/CrawlerDepthLimitTest.java": [ +46, +52 +], +"org.sonarsource.sonarqube:sonar-server:src/test/java/org/sonar/server/computation/task/projectanalysis/container/ProjectAnalysisTaskContainerPopulatorTest.java": [ +184 +], +"org.sonarsource.sonarqube:sonar-server:src/test/java/org/sonar/server/computation/task/projectanalysis/measure/MapBasedRawMeasureRepositoryTest.java": [ +86, +210 +], +"org.sonarsource.sonarqube:sonar-server:src/test/java/org/sonar/server/computation/task/projectanalysis/measure/MeasureRepositoryImplTest.java": [ +267 +], +"org.sonarsource.sonarqube:sonar-server:src/test/java/org/sonar/server/computation/task/projectanalysis/measure/MeasureRepositoryRule.java": [ +65 +], +"org.sonarsource.sonarqube:sonar-server:src/test/java/org/sonar/server/computation/task/projectanalysis/measure/MeasureTest.java": [ +98 +], +"org.sonarsource.sonarqube:sonar-server:src/test/java/org/sonar/server/computation/task/projectanalysis/source/SourceHashRepositoryImplTest.java": [ +83 +], +"org.sonarsource.sonarqube:sonar-server:src/test/java/org/sonar/server/computation/task/projectanalysis/step/PurgeDatastoresStepTest.java": [ +93, +111 +], +"org.sonarsource.sonarqube:sonar-server:src/test/java/org/sonar/server/computation/task/projectanalysis/step/ReportComputationStepsTest.java": [ +42, +63 +], +"org.sonarsource.sonarqube:sonar-server:src/test/java/org/sonar/server/duplication/ws/DuplicationsParserTest.java": [ +235 +], +"org.sonarsource.sonarqube:sonar-server:src/test/java/org/sonar/server/es/EsTester.java": [ +168 +], +"org.sonarsource.sonarqube:sonar-server:src/test/java/org/sonar/server/issue/workflow/IssueWorkflowTest.java": [ +259 +], +"org.sonarsource.sonarqube:sonar-server:src/test/java/org/sonar/server/notification/NotificationDaemonTest.java": [ +252 +], +"org.sonarsource.sonarqube:sonar-server:src/test/java/org/sonar/server/platform/db/migration/DatabaseMigrationImplConcurrentAccessTest.java": [ +55 +], +"org.sonarsource.sonarqube:sonar-server:src/test/java/org/sonar/server/qualityprofile/ws/CompareActionMediumTest.java": [ +54 +], +"org.sonarsource.sonarqube:sonar-server:src/test/java/org/sonar/server/rule/RegisterRulesMediumTest.java": [ +118, +135, +149, +183, +205, +230, +265, +290, +320, +356, +380, +403 +], +"org.sonarsource.sonarqube:sonar-server:src/test/java/org/sonar/server/rule/RegisterRulesTest.java": [ +252, +268, +287, +299, +322, +334 +], +"org.sonarsource.sonarqube:sonar-server:src/test/java/org/sonar/server/util/AbstractStoppableExecutorServiceTest.java": [ +44 +] +} diff --git a/java-checks-test-sources/default/src/test/java/checks/tests/AnonymousClassOnFunctionalInterfaceCheckSample.java b/java-checks-test-sources/default/src/test/java/checks/tests/AnonymousClassOnFunctionalInterfaceCheckSample.java new file mode 100644 index 00000000000..274f73e1eb8 --- /dev/null +++ b/java-checks-test-sources/default/src/test/java/checks/tests/AnonymousClassOnFunctionalInterfaceCheckSample.java @@ -0,0 +1,310 @@ +package checks.tests; + +class AnonymousClassOnFunctionalInterfaceCheckSample { + + enum Foo { + FOO { + @Override + public String method() { + return "foo"; + } + }, + BAR { + @Override + public String method() { + return "bar"; + } + }; + + public String method() { + return ""; + } + } + + interface Handler { + String handle(); + } + + interface MyInterface { + enum InnerEnum { + A, B, C; + } + } + + void toto() { + new MyInterface() {}; // Compliant + + new Handler(){ // Noncompliant {{Make this anonymous inner class a lambda}} + @Override + public String handle() { + return "handled"; + } + }.handle(); + + new Handler(){ + private String myMethod(){ + return "plop"; + } + + @Override + public String handle() { + return myMethod(); + } + }.handle(); + + new Handler(){ + @Override + public String handle() { + return this.toString(); + } + }.handle(); + + new Handler(){ // Noncompliant {{Make this anonymous inner class a lambda}} + @Override + public String handle() { + class C{ + String meth(){ + return ""; + } + String fun(){ + return this.meth(); + } + } + return new C().fun(); + } + }; + + new Handler(){ // Compliant + int myVar; + + @Override + public String handle() { + return ""; + } + }; + + new Handler(){ // Noncompliant {{Make this anonymous inner class a lambda}} + @Override + public String handle() { + return ""; + }; // this empty statement should not be counted! + }; + + new + Handler // Noncompliant {{Make this anonymous inner class a lambda}} + (){ + @Override + public String handle() { + return AnonymousClassOnFunctionalInterfaceCheckSample.this.toString(); + } + }; + + new Handler(){ // Compliant, annotation prevents transform to a lambda + @Override + @SuppressWarnings("something") + public String handle() { + return "handled"; + } + }.handle(); + + new Handler(){ // Compliant, annotation prevents transform to a lambda + @SuppressWarnings("something") + @Override + public String handle() { + return "handled"; + } + }.handle(); + } + + String toStr(){ + return ""; + } + + abstract static class AbstractClass { + public abstract void foo(); + + static void bar() { + AbstractClass ac1 = new AbstractClass() { // Compliant: not a SAM + @Override + public void foo() { + } + }; + } + } + + interface MyHandler extends Handler{} + + public abstract static class Main { + + public abstract void myMethod(); + + public static void main(String[] args) { + Main main = new Main() { + @Override + public void myMethod() { + } + }; + main.myMethod(); + Object o1 = new Object() { + @Override + public String toString(){ + return null; + } + }; + Object o12 = new MyHandler() { // Noncompliant + @Override + public String handle() { + return null; + } + }; + } + } +} + +class SamWithExceptionS9357 { + + class MyCheckedException extends Exception {} + interface I { + void apply(String s) throws MyCheckedException; + } + void foo(I i) { + foo(new I() { // Compliant: cannot refactor as lambda because of checked exception + @Override + public void apply(String s) throws MyCheckedException { + } + }); + } +} + +abstract class WithinLambdaS9357 { + + @FunctionalInterface + interface Action { + T run(); + } + + abstract T doSomething(Action action); + + private void bar(WithinLambdaS9357 a) { + a.doSomething( + (Action) () -> { + new Thread( + new Runnable() { // Noncompliant + @Override + public void run() { + } + }); + return null; + }); + } +} + +interface ABS9357 { + default void foo() { + } + + default void bar() { + } + + static void main() { + ABS9357 a = new ABS9357() { // Compliant + @Override + public void foo() { + } + }; + } +} + +interface BAS9357 { + default void foo() { + } + + void bar(); + + static void main() { + BAS9357 a = new BAS9357() { // Noncompliant + @Override + public void bar() { + } + }; + } +} + +class AlphaS9357 { + + interface Lvl1 { + void foo(); + } + + interface Lvl2 extends Lvl1 { + @Override + void foo(); + } + + Lvl2 level = new Lvl2() { // Noncompliant + @Override + public void foo() { + } + }; + Lvl2 level2 = () -> {}; +} + +class ThisInstanceTestS9357 { + + interface WithDefault { + default String defaultMethod() { return "defaultMethod"; } + String funcMethod(); + } + void testDefault() { + WithDefault f = new WithDefault() { // Compliant, invoke a default method + @Override + public String funcMethod() { + return defaultMethod(); + } + }; + } + + interface Math { + int powerOfTwo(int n); + } + + void testRecursion() { + Math f = new Math() { // Compliant, recursion + @Override + public int powerOfTwo(int n) { + return n == 0 ? 1 : 2 * powerOfTwo(n -1); + } + }; + } + + int globalPowerOfTwo(int n) { + return n == 0 ? 1 : 2 * globalPowerOfTwo(n -1); + } + + void testNotThisInstanceMethod() { + Math f = new Math() { // Noncompliant + @Override + public int powerOfTwo(int n) { + return globalPowerOfTwo(n); + } + }; + } +} + +abstract class GenericTypeS9357 { + + void foo(GenericTypeS9357 something) { + bar(something, new MyComparable() { // Compliant - compare is a generic method + @Override + public > int compare(T obj1, T obj2) { + return 0; + } + }); + } + + abstract > void bar(GenericTypeS9357 object, MyComparable comp); + + interface MyComparable { + > int compare(T obj1, T obj2); + } +} diff --git a/java-checks-test-sources/default/src/test/java/checks/tests/AnonymousClassOnFunctionalInterfaceCheckSampleWithoutSemantic.java b/java-checks-test-sources/default/src/test/java/checks/tests/AnonymousClassOnFunctionalInterfaceCheckSampleWithoutSemantic.java new file mode 100644 index 00000000000..441b561eb73 --- /dev/null +++ b/java-checks-test-sources/default/src/test/java/checks/tests/AnonymousClassOnFunctionalInterfaceCheckSampleWithoutSemantic.java @@ -0,0 +1,310 @@ +package checks.tests; + +class AnonymousClassOnFunctionalInterfaceCheckSampleWithoutSemantic { + + enum Foo { + FOO { + @Override + public String method() { + return "foo"; + } + }, + BAR { + @Override + public String method() { + return "bar"; + } + }; + + public String method() { + return ""; + } + } + + interface Handler { + String handle(); + } + + interface MyInterface { + enum InnerEnum { + A, B, C; + } + } + + void toto() { + new MyInterface() {}; // Compliant + + new Handler(){ // Noncompliant + @Override + public String handle() { + return "handled"; + } + }.handle(); + + new Handler(){ + private String myMethod(){ + return "plop"; + } + + @Override + public String handle() { + return myMethod(); + } + }.handle(); + + new Handler(){ + @Override + public String handle() { + return this.toString(); + } + }.handle(); + + new Handler(){ // Noncompliant + @Override + public String handle() { + class C{ + String meth(){ + return ""; + } + String fun(){ + return this.meth(); + } + } + return new C().fun(); + } + }; + + new Handler(){ // Compliant + int myVar; + + @Override + public String handle() { + return ""; + } + }; + + new Handler(){ // Noncompliant + @Override + public String handle() { + return ""; + }; // this empty statement should not be counted! + }; + + new + Handler // Noncompliant + (){ + @Override + public String handle() { + return AnonymousClassOnFunctionalInterfaceCheckSampleWithoutSemantic.this.toString(); + } + }; + + new Handler(){ // Compliant, annotation prevents transform to a lambda + @Override + @SuppressWarnings("something") + public String handle() { + return "handled"; + } + }.handle(); + + new Handler(){ // Compliant, annotation prevents transform to a lambda + @SuppressWarnings("something") + @Override + public String handle() { + return "handled"; + } + }.handle(); + } + + String toStr(){ + return ""; + } + + abstract static class AbstractClass { + public abstract void foo(); + + static void bar() { + AbstractClass ac1 = new AbstractClass() { // Compliant: not a SAM + @Override + public void foo() { + } + }; + } + } + + interface MyHandler extends Handler{} + + public abstract static class Main { + + public abstract void myMethod(); + + public static void main(String[] args) { + Main main = new Main() { + @Override + public void myMethod() { + } + }; + main.myMethod(); + Object o1 = new Object() { + @Override + public String toString(){ + return null; + } + }; + Object o12 = new MyHandler() { // Noncompliant + @Override + public String handle() { + return null; + } + }; + } + } +} + +class SamWithExceptionS9357WS { + + class MyCheckedException extends Exception {} + interface I { + void apply(String s) throws MyCheckedException; + } + void foo(I i) { + foo(new I() { // Compliant: checked exception + @Override + public void apply(String s) throws MyCheckedException { + } + }); + } +} + +abstract class WithinLambdaS9357WS { + + @FunctionalInterface + interface Action { + T run(); + } + + abstract T doSomething(Action action); + + private void bar(WithinLambdaS9357WS a) { + a.doSomething( + (Action) () -> { + new Thread( + new Runnable() { // Noncompliant + @Override + public void run() { + } + }); + return null; + }); + } +} + +interface ABS9357WS { + default void foo() { + } + + default void bar() { + } + + static void main() { + ABS9357WS a = new ABS9357WS() { // Compliant + @Override + public void foo() { + } + }; + } +} + +interface BAS9357WS { + default void foo() { + } + + void bar(); + + static void main() { + BAS9357WS a = new BAS9357WS() { // Noncompliant + @Override + public void bar() { + } + }; + } +} + +class AlphaS9357WS { + + interface Lvl1 { + void foo(); + } + + interface Lvl2 extends Lvl1 { + @Override + void foo(); + } + + Lvl2 level = new Lvl2() { // Noncompliant + @Override + public void foo() { + } + }; + Lvl2 level2 = () -> {}; +} + +class ThisInstanceTestS9357WS { + + interface WithDefault { + default String defaultMethod() { return "defaultMethod"; } + String funcMethod(); + } + void testDefault() { + WithDefault f = new WithDefault() { // Compliant, invoke a default method + @Override + public String funcMethod() { + return defaultMethod(); + } + }; + } + + interface Math { + int powerOfTwo(int n); + } + + void testRecursion() { + Math f = new Math() { // Compliant, recursion + @Override + public int powerOfTwo(int n) { + return n == 0 ? 1 : 2 * powerOfTwo(n -1); + } + }; + } + + int globalPowerOfTwo(int n) { + return n == 0 ? 1 : 2 * globalPowerOfTwo(n -1); + } + + void testNotThisInstanceMethod() { + Math f = new Math() { // Noncompliant + @Override + public int powerOfTwo(int n) { + return globalPowerOfTwo(n); + } + }; + } +} + +abstract class GenericTypeS9357WS { + + void foo(GenericTypeS9357WS something) { + bar(something, new MyComparable() { // Compliant - compare is a generic method + @Override + public > int compare(T obj1, T obj2) { + return 0; + } + }); + } + + abstract > void bar(GenericTypeS9357WS object, MyComparable comp); + + interface MyComparable { + > int compare(T obj1, T obj2); + } +} diff --git a/java-checks/src/main/java/org/sonar/java/checks/AnonymousClassOnFunctionalInterfaceCheck.java b/java-checks/src/main/java/org/sonar/java/checks/AnonymousClassOnFunctionalInterfaceCheck.java new file mode 100644 index 00000000000..bbb6c9222f8 --- /dev/null +++ b/java-checks/src/main/java/org/sonar/java/checks/AnonymousClassOnFunctionalInterfaceCheck.java @@ -0,0 +1,39 @@ +/* + * 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.sonar.check.Rule; +import org.sonar.java.checks.helpers.AnonymousClassToLambdaUtils; +import org.sonar.plugins.java.api.JavaFileScanner; +import org.sonar.plugins.java.api.JavaFileScannerContext; +import org.sonar.plugins.java.api.JavaVersion; +import org.sonar.plugins.java.api.JavaVersionAwareVisitor; + +@Rule(key = "S9357") +public class AnonymousClassOnFunctionalInterfaceCheck implements JavaFileScanner, JavaVersionAwareVisitor { + + @Override + public boolean isCompatibleWithJavaVersion(JavaVersion version) { + return version.isJava8Compatible(); + } + + @Override + public void scanFile(JavaFileScannerContext context) { + AnonymousClassToLambdaUtils.scanForAnonymousClassConvertibleToLambda(this, context); + } + +} diff --git a/java-checks/src/main/java/org/sonar/java/checks/AnonymousClassShouldBeLambdaCheck.java b/java-checks/src/main/java/org/sonar/java/checks/AnonymousClassShouldBeLambdaCheck.java index 3881452ba3d..17d09285295 100644 --- a/java-checks/src/main/java/org/sonar/java/checks/AnonymousClassShouldBeLambdaCheck.java +++ b/java-checks/src/main/java/org/sonar/java/checks/AnonymousClassShouldBeLambdaCheck.java @@ -16,35 +16,15 @@ */ package org.sonar.java.checks; -import java.util.HashSet; -import java.util.List; -import java.util.Set; -import java.util.stream.Collectors; import org.sonar.check.Rule; +import org.sonar.java.checks.helpers.AnonymousClassToLambdaUtils; import org.sonar.plugins.java.api.JavaFileScanner; import org.sonar.plugins.java.api.JavaFileScannerContext; import org.sonar.plugins.java.api.JavaVersion; import org.sonar.plugins.java.api.JavaVersionAwareVisitor; -import org.sonar.plugins.java.api.semantic.Symbol; -import org.sonar.plugins.java.api.semantic.Symbol.MethodSymbol; -import org.sonar.plugins.java.api.semantic.Type; -import org.sonar.plugins.java.api.tree.BaseTreeVisitor; -import org.sonar.plugins.java.api.tree.ClassTree; -import org.sonar.plugins.java.api.tree.EnumConstantTree; -import org.sonar.plugins.java.api.tree.IdentifierTree; -import org.sonar.plugins.java.api.tree.MemberSelectExpressionTree; -import org.sonar.plugins.java.api.tree.MethodInvocationTree; -import org.sonar.plugins.java.api.tree.MethodTree; -import org.sonar.plugins.java.api.tree.NewClassTree; -import org.sonar.plugins.java.api.tree.Tree; -import org.sonar.plugins.java.api.tree.TypeTree; @Rule(key = "S1604") -public class AnonymousClassShouldBeLambdaCheck extends BaseTreeVisitor implements JavaFileScanner, JavaVersionAwareVisitor { - - private static final String JAVA_LANG_OBJECT = "java.lang.Object"; - private JavaFileScannerContext context; - private final Set enumConstants = new HashSet<>(); +public class AnonymousClassShouldBeLambdaCheck implements JavaFileScanner, JavaVersionAwareVisitor { @Override public boolean isCompatibleWithJavaVersion(JavaVersion version) { @@ -53,149 +33,7 @@ public boolean isCompatibleWithJavaVersion(JavaVersion version) { @Override public void scanFile(JavaFileScannerContext context) { - this.context = context; - enumConstants.clear(); - scan(context.getTree()); - } - - @Override - public void visitEnumConstant(EnumConstantTree tree) { - enumConstants.add(tree.simpleName()); - super.visitEnumConstant(tree); - enumConstants.remove(tree.simpleName()); - } - - @Override - public void visitNewClass(NewClassTree tree) { - super.visitNewClass(tree); - ClassTree classBody = tree.classBody(); - if (classBody != null) { - TypeTree identifier = tree.identifier(); - if (!useThisInstance(classBody) && !enumConstants.contains(identifier) && isSAM(classBody)) { - context.reportIssue(this, identifier, "Make this anonymous inner class a lambda" + context.getJavaVersion().java8CompatibilityMessage()); - } - } - } - - private static boolean isSAM(ClassTree classBody) { - if (hasOnlyOneMethod(classBody.members())) { - // When overriding only one method of a functional interface, it can only be the single abstract method - // and not one of the default methods. No need to check that the method signature matches. - Symbol.TypeSymbol symbol = classBody.symbol(); - // should be anonymous class of interface and not abstract class - return symbol.interfaces().size() == 1 - && symbol.superClass().is(JAVA_LANG_OBJECT) - && hasSingleAbstractMethodInHierarchy(symbol.superTypes()); - } - return false; - } - - private static boolean hasSingleAbstractMethodInHierarchy(Set superTypes) { - return superTypes.stream() - .filter(type -> !type.is(JAVA_LANG_OBJECT)) - .map(Type::symbol) - // collect all the methods declared in hierarchy - .flatMap(superType -> superType.memberSymbols().stream().filter(Symbol::isMethodSymbol).filter(Symbol::isAbstract)) - .map(Symbol.MethodSymbol.class::cast) - // remove objects methods redefined in interfaces - .filter(symbol -> !isObjectMethod(symbol)) - // remove generic methods, which can not be written as lambda (JLS-11 §15.27) - .filter(symbol -> !symbol.isParametrizedMethod()) - // always take same symbol if method is redeclared over and over in hierarchy - .map(AnonymousClassShouldBeLambdaCheck::overriddenSymbolIfAny) - .collect(Collectors.toSet()) - .size() == 1; - } - - private static Symbol.MethodSymbol overriddenSymbolIfAny(MethodSymbol symbol) { - return symbol.overriddenSymbols().stream() - .findFirst() - .orElse(symbol); - } - - private static boolean isObjectMethod(Symbol.MethodSymbol methodSymbol) { - return methodSymbol.overriddenSymbols().stream() - .map(Symbol::owner) - .map(Symbol::type) - .anyMatch(t -> t.is(JAVA_LANG_OBJECT)); - } - - private static boolean hasOnlyOneMethod(List members) { - MethodTree methodTree = null; - for (Tree tree : members) { - if (!tree.is(Tree.Kind.EMPTY_STATEMENT, Tree.Kind.METHOD)) { - return false; - } - if (tree.is(Tree.Kind.METHOD)) { - if (methodTree != null) { - return false; - } - methodTree = (MethodTree) tree; - - } - } - - return methodTree != null && canRefactorMethod(methodTree); - } - - private static boolean canRefactorMethod(MethodTree methodTree) { - // if overridden method declares to throw an exception, refactoring to a lambda might prove tricky - // if it is annotated with something else than @Override, it is not possible to refactor the code - return methodTree.throwsClauses().isEmpty() - && methodTree.symbol().metadata().annotations().stream() - .allMatch(annotation -> annotation.symbol().type().is("java.lang.Override")); - } - - private static boolean useThisInstance(ClassTree body) { - UsesThisInstanceVisitor visitor = new UsesThisInstanceVisitor(body.symbol().type()); - body.accept(visitor); - return visitor.usesThisInstance; - } - - private static class UsesThisInstanceVisitor extends BaseTreeVisitor { - private final Type instanceType; - boolean usesThisInstance = false; - boolean visitedClassTree = false; - - public UsesThisInstanceVisitor(Type instanceType) { - this.instanceType = instanceType; - } - - @Override - public void visitClass(ClassTree tree) { - // visit the class body but ignore inner classes - if (!visitedClassTree) { - visitedClassTree = true; - super.visitClass(tree); - } - } - - @Override - public void visitNewClass(NewClassTree tree) { - // ignore anonymous classes - } - - @Override - public void visitMemberSelectExpression(MemberSelectExpressionTree tree) { - scan(tree.expression()); - // ignore identifier, because if it is this, it is a qualified this. - } - - @Override - public void visitMethodInvocation(MethodInvocationTree tree) { - if (tree.methodSelect().is(Tree.Kind.IDENTIFIER)) { - Symbol symbol = ((IdentifierTree) tree.methodSelect()).symbol(); - usesThisInstance |= symbol.isMethodSymbol() && - !symbol.isStatic() && - instanceType.isSubtypeOf(symbol.owner().type()); - } - super.visitMethodInvocation(tree); - } - - @Override - public void visitIdentifier(IdentifierTree tree) { - usesThisInstance |= "this".equals(tree.name()); - } + AnonymousClassToLambdaUtils.scanForAnonymousClassConvertibleToLambda(this, context); } } diff --git a/java-checks/src/main/java/org/sonar/java/checks/helpers/AnonymousClassToLambdaUtils.java b/java-checks/src/main/java/org/sonar/java/checks/helpers/AnonymousClassToLambdaUtils.java new file mode 100644 index 00000000000..2e656c5f912 --- /dev/null +++ b/java-checks/src/main/java/org/sonar/java/checks/helpers/AnonymousClassToLambdaUtils.java @@ -0,0 +1,190 @@ +/* + * 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.helpers; + +import java.util.HashSet; +import java.util.List; +import java.util.Set; +import java.util.stream.Collectors; +import org.sonar.plugins.java.api.JavaFileScanner; +import org.sonar.plugins.java.api.JavaFileScannerContext; +import org.sonar.plugins.java.api.semantic.Symbol; +import org.sonar.plugins.java.api.semantic.Symbol.MethodSymbol; +import org.sonar.plugins.java.api.semantic.Type; +import org.sonar.plugins.java.api.tree.BaseTreeVisitor; +import org.sonar.plugins.java.api.tree.ClassTree; +import org.sonar.plugins.java.api.tree.EnumConstantTree; +import org.sonar.plugins.java.api.tree.IdentifierTree; +import org.sonar.plugins.java.api.tree.MemberSelectExpressionTree; +import org.sonar.plugins.java.api.tree.MethodInvocationTree; +import org.sonar.plugins.java.api.tree.MethodTree; +import org.sonar.plugins.java.api.tree.NewClassTree; +import org.sonar.plugins.java.api.tree.Tree; + +public final class AnonymousClassToLambdaUtils { + + private static final String JAVA_LANG_OBJECT = "java.lang.Object"; + + private AnonymousClassToLambdaUtils() { + } + + public static void scanForAnonymousClassConvertibleToLambda(JavaFileScanner check, JavaFileScannerContext context) { + var visitor = new AnonymousClassToLambdaVisitor(check, context); + context.getTree().accept(visitor); + } + + public static boolean canBeConvertedToLambda(ClassTree classBody, Set enumConstants) { + var identifier = ((NewClassTree) classBody.parent()).identifier(); + return !useThisInstance(classBody) && !enumConstants.contains(identifier) && isSAM(classBody); + } + + private static boolean isSAM(ClassTree classBody) { + if (hasOnlyOneMethod(classBody.members())) { + Symbol.TypeSymbol symbol = classBody.symbol(); + return symbol.interfaces().size() == 1 + && symbol.superClass().is(JAVA_LANG_OBJECT) + && hasSingleAbstractMethodInHierarchy(symbol.superTypes()); + } + return false; + } + + private static boolean hasSingleAbstractMethodInHierarchy(Set superTypes) { + return superTypes.stream() + .filter(type -> !type.is(JAVA_LANG_OBJECT)) + .map(Type::symbol) + .flatMap(superType -> superType.memberSymbols().stream().filter(Symbol::isMethodSymbol).filter(Symbol::isAbstract)) + .map(Symbol.MethodSymbol.class::cast) + .filter(symbol -> !isObjectMethod(symbol)) + .filter(symbol -> !symbol.isParametrizedMethod()) + .map(AnonymousClassToLambdaUtils::overriddenSymbolIfAny) + .collect(Collectors.toSet()) + .size() == 1; + } + + private static Symbol.MethodSymbol overriddenSymbolIfAny(MethodSymbol symbol) { + return symbol.overriddenSymbols().stream() + .findFirst() + .orElse(symbol); + } + + private static boolean isObjectMethod(Symbol.MethodSymbol methodSymbol) { + return methodSymbol.overriddenSymbols().stream() + .map(Symbol::owner) + .map(Symbol::type) + .anyMatch(t -> t.is(JAVA_LANG_OBJECT)); + } + + private static boolean hasOnlyOneMethod(List members) { + MethodTree methodTree = null; + for (Tree tree : members) { + if (!tree.is(Tree.Kind.EMPTY_STATEMENT, Tree.Kind.METHOD)) { + return false; + } + if (tree.is(Tree.Kind.METHOD)) { + if (methodTree != null) { + return false; + } + methodTree = (MethodTree) tree; + } + } + return methodTree != null && canRefactorMethod(methodTree); + } + + private static boolean canRefactorMethod(MethodTree methodTree) { + return methodTree.throwsClauses().isEmpty() + && methodTree.symbol().metadata().annotations().stream() + .allMatch(annotation -> annotation.symbol().type().is("java.lang.Override")); + } + + private static boolean useThisInstance(ClassTree body) { + UsesThisInstanceVisitor visitor = new UsesThisInstanceVisitor(body.symbol().type()); + body.accept(visitor); + return visitor.usesThisInstance; + } + + private static class AnonymousClassToLambdaVisitor extends BaseTreeVisitor { + private final JavaFileScanner check; + private final JavaFileScannerContext context; + private final Set enumConstants = new HashSet<>(); + + AnonymousClassToLambdaVisitor(JavaFileScanner check, JavaFileScannerContext context) { + this.check = check; + this.context = context; + } + + @Override + public void visitEnumConstant(EnumConstantTree tree) { + enumConstants.add(tree.simpleName()); + super.visitEnumConstant(tree); + enumConstants.remove(tree.simpleName()); + } + + @Override + public void visitNewClass(NewClassTree tree) { + super.visitNewClass(tree); + ClassTree classBody = tree.classBody(); + if (classBody != null && canBeConvertedToLambda(classBody, enumConstants)) { + context.reportIssue(check, tree.identifier(), "Make this anonymous inner class a lambda" + context.getJavaVersion().java8CompatibilityMessage()); + } + } + } + + private static class UsesThisInstanceVisitor extends BaseTreeVisitor { + private final Type instanceType; + boolean usesThisInstance = false; + boolean visitedClassTree = false; + + public UsesThisInstanceVisitor(Type instanceType) { + this.instanceType = instanceType; + } + + @Override + public void visitClass(ClassTree tree) { + if (!visitedClassTree) { + visitedClassTree = true; + super.visitClass(tree); + } + } + + @Override + public void visitNewClass(NewClassTree tree) { + // ignore anonymous classes + } + + @Override + public void visitMemberSelectExpression(MemberSelectExpressionTree tree) { + scan(tree.expression()); + } + + @Override + public void visitMethodInvocation(MethodInvocationTree tree) { + if (tree.methodSelect().is(Tree.Kind.IDENTIFIER)) { + Symbol symbol = ((IdentifierTree) tree.methodSelect()).symbol(); + usesThisInstance |= symbol.isMethodSymbol() && + !symbol.isStatic() && + instanceType.isSubtypeOf(symbol.owner().type()); + } + super.visitMethodInvocation(tree); + } + + @Override + public void visitIdentifier(IdentifierTree tree) { + usesThisInstance |= "this".equals(tree.name()); + } + } + +} diff --git a/java-checks/src/test/java/org/sonar/java/checks/AnonymousClassOnFunctionalInterfaceCheckTest.java b/java-checks/src/test/java/org/sonar/java/checks/AnonymousClassOnFunctionalInterfaceCheckTest.java new file mode 100644 index 00000000000..f575c3a9bd1 --- /dev/null +++ b/java-checks/src/test/java/org/sonar/java/checks/AnonymousClassOnFunctionalInterfaceCheckTest.java @@ -0,0 +1,44 @@ +/* + * 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.testCodeSourcesPath; + +class AnonymousClassOnFunctionalInterfaceCheckTest { + + @Test + void test() { + CheckVerifier.newVerifier() + .onFile(testCodeSourcesPath("checks/tests/AnonymousClassOnFunctionalInterfaceCheckSample.java")) + .withCheck(new AnonymousClassOnFunctionalInterfaceCheck()) + .withJavaVersion(8) + .verifyIssues(); + } + + @Test + void test_without_semantic() { + CheckVerifier.newVerifier() + .onFile(testCodeSourcesPath("checks/tests/AnonymousClassOnFunctionalInterfaceCheckSampleWithoutSemantic.java")) + .withCheck(new AnonymousClassOnFunctionalInterfaceCheck()) + .withJavaVersion(8) + .withoutSemantic() + .verifyIssues(); + } +} diff --git a/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S9357.html b/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S9357.html new file mode 100644 index 00000000000..b070e3d3ba4 --- /dev/null +++ b/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S9357.html @@ -0,0 +1,72 @@ +

This rule raises an issue when you create an anonymous inner class that implements a functional interface (an interface with exactly one abstract +method). In Java, these anonymous inner classes should be replaced with lambda expressions, which provide a cleaner and more concise syntax for the +same functionality.

+

Why is this an issue?

+

A single-method interface is a type that defines exactly one abstract behavior contract. Common examples include interfaces for tasks, +computations, comparisons, and various transformation operations.

+

Before modern versions of the language introduced simplified syntax, the only way to implement a single-method interface was through verbose inline +type implementations. This approach requires significant boilerplate code that obscures the actual logic:

+
+taskVariable = new InterfaceImplementation() {
+    @Override
+    public void methodName() {
+        processData();
+    }
+};
+
+

Modern language versions introduced inline function syntax specifically to simplify this pattern. An inline function literal is a concise way to +represent a single-method interface using an expression. The same functionality can be written as:

+
+taskVariable = () -> processData();
+
+

Using inline function syntax instead of verbose type implementations offers several benefits:

+
    +
  • Reduced boilerplate: Inline functions eliminate the need to repeat the interface name, method name, and method override + annotations.
    • +
  • Improved readability: The code focuses on what the method does rather than the mechanics of creating an inline + implementation.
    • +
  • Better maintenance: Less code means fewer opportunities for errors and easier refactoring.
    • +
  • Modern style: Inline function syntax is the idiomatic way to work with single-method interfaces in modern versions of the + language.
    • +
+

The conversion from verbose inline implementation to concise function syntax is straightforward because the compiler can infer the interface type +from the context. This type inference is what makes inline functions so concise.

+

In Java, these single-method interfaces are called functional interfaces. Common examples include Runnable, +Callable, Comparator, and the many interfaces in the java.util.function package. Lambda expressions were +introduced in Java 8 as the standard syntax for implementing functional interfaces concisely.

+

When you use lambda expressions instead of anonymous inner classes:

+
    +
  • Your code becomes more readable and easier to maintain
    • +
  • You reduce boilerplate code significantly
    • +
  • You make your intent clearer to other developers
    • +
  • You follow modern Java conventions and best practices
    • +
+

What is the potential impact?

+

Using older verbose syntax for defining inline behavior instead of modern concise functional syntax makes the code more verbose and harder to read. +While this doesn’t affect the runtime behavior or security of the application, it impacts code maintainability. Developers spend more time reading and +understanding unnecessarily complex code, which slows down development and increases the likelihood of introducing errors during modifications.

+

How to fix it

+

Replace the anonymous inner class with a lambda expression. The lambda syntax uses parameters in parentheses, an arrow →, and the +method body. For single-expression bodies, you can omit the braces and return keyword.

+

Code examples

+

Noncompliant code example

+
+Runnable task = new Runnable() {
+    @Override
+    public void run() {
+        System.out.println("Processing...");
+    }
+}; // Noncompliant
+
+

Compliant solution

+
+Runnable task = () -> System.out.println("Processing...");
+
+

Resources

+

Documentation

+ + diff --git a/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S9357.json b/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S9357.json new file mode 100644 index 00000000000..f22599703fa --- /dev/null +++ b/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S9357.json @@ -0,0 +1,26 @@ +{ + "title": "Anonymous classes on functional interfaces should be lambdas", + "type": "CODE_SMELL", + "status": "ready", + "remediation": { + "func": "Constant\/Issue", + "constantCost": "5 min" + }, + "tags": [ + "lambda", + "java8", + "convention", + "tests" + ], + "defaultSeverity": "Major", + "ruleSpecification": "RSPEC-9357", + "sqKey": "S9357", + "scope": "Tests", + "quickfix": "unknown", + "code": { + "impacts": { + "MAINTAINABILITY": "MEDIUM" + }, + "attribute": "CLEAR" + } +} diff --git a/sonar-java-plugin/src/main/resources/profiles/Sonar_way/S9357 b/sonar-java-plugin/src/main/resources/profiles/Sonar_way/S9357 new file mode 100644 index 00000000000..e69de29bb2d