From cfb8e110ee374a8a31593aaf7d82933250a01b30 Mon Sep 17 00:00:00 2001 From: lingenj Date: Tue, 24 Dec 2024 17:39:27 +0100 Subject: [PATCH 1/3] `UseLambdaForFunctionalInterface` recipe should not convert when code uses a static field from enum constructor --- .../UseLambdaForFunctionalInterface.java | 35 +++++ .../UseLambdaForFunctionalInterfaceTest.java | 140 ++++++++++++++++++ 2 files changed, 175 insertions(+) diff --git a/src/main/java/org/openrewrite/staticanalysis/UseLambdaForFunctionalInterface.java b/src/main/java/org/openrewrite/staticanalysis/UseLambdaForFunctionalInterface.java index d5213d062..61822fc0f 100644 --- a/src/main/java/org/openrewrite/staticanalysis/UseLambdaForFunctionalInterface.java +++ b/src/main/java/org/openrewrite/staticanalysis/UseLambdaForFunctionalInterface.java @@ -96,6 +96,10 @@ public J visitNewClass(J.NewClass newClass, ExecutionContext ctx) { StringBuilder templateBuilder = new StringBuilder(); J.MethodDeclaration methodDeclaration = (J.MethodDeclaration) n.getBody().getStatements().get(0); + if (isLambdaInEnumConstructorWithStaticField(getCursor(), methodDeclaration)) { + return n; + } + // If the functional interface method has type parameters, we can't replace it with a lambda. if (methodDeclaration.getTypeParameters() != null && !methodDeclaration.getTypeParameters().isEmpty()) { return n; @@ -137,6 +141,37 @@ public J visitNewClass(J.NewClass newClass, ExecutionContext ctx) { return n; } + private boolean isLambdaInEnumConstructorWithStaticField(Cursor cursor, J.MethodDeclaration md) { + if (md.getBody() == null) { + return false; + } + + for (Statement s : md.getBody().getStatements()) { + J.MethodInvocation e = null; + if (s instanceof J.VariableDeclarations && ((J.VariableDeclarations) s).getVariables().size() == 1 && ((J.VariableDeclarations) s).getVariables().get(0).getInitializer() instanceof J.MethodInvocation) { + e = (J.MethodInvocation) ((J.VariableDeclarations) s).getVariables().get(0).getInitializer(); + } else if (s instanceof J.MethodInvocation) { + e = (J.MethodInvocation) s; + } + + if (e != null && e.getSelect() instanceof J.Identifier) { + JavaType.Variable v = ((J.Identifier) e.getSelect()).getFieldType(); + if (v != null && v.hasFlags(Flag.Static)) { + JavaType owner = v.getOwner(); + if (owner instanceof JavaType.Class && ((JavaType.Class) owner).getKind() == JavaType.Class.Kind.Enum) { + J.MethodDeclaration parentMethodDeclaration = cursor.dropParentUntil(p -> p instanceof J.MethodDeclaration).getValue(); + J.ClassDeclaration parentClass = cursor.dropParentUntil(p -> p instanceof J.ClassDeclaration).getValue(); + if (parentMethodDeclaration.isConstructor() && owner.equals(parentClass.getType())) { + return true; + } + } + } + } + } + + return false; + } + private J maybeAddCast(J.Lambda lambda, J.NewClass original) { J parent = getCursor().getParentTreeCursor().getValue(); diff --git a/src/test/java/org/openrewrite/staticanalysis/UseLambdaForFunctionalInterfaceTest.java b/src/test/java/org/openrewrite/staticanalysis/UseLambdaForFunctionalInterfaceTest.java index 13b9d3007..12c49d554 100644 --- a/src/test/java/org/openrewrite/staticanalysis/UseLambdaForFunctionalInterfaceTest.java +++ b/src/test/java/org/openrewrite/staticanalysis/UseLambdaForFunctionalInterfaceTest.java @@ -774,4 +774,144 @@ public List call() { ) ); } + + @Test + @Issue("https://github.com/openrewrite/rewrite-static-analysis/issues/413") + void dontUseLambdaWhenEnumAccessesStaticFieldFromConstructor() { + rewriteRun( + //language=java + java( + """ + import java.time.LocalDate; + import java.time.format.DateTimeFormatter; + enum Test { + A, B; + + private static final DateTimeFormatter DATE_FORMAT = DateTimeFormatter.ofPattern("yyyy-MM-dd"); + + Test() { + Runnable r = new Runnable() { + @Override + public void run() { + DATE_FORMAT.format(LocalDate.now()); + } + }; + } + } + """ + ) + ); + } + + @Test + @Issue("https://github.com/openrewrite/rewrite-static-analysis/issues/413") + void dontUseLambdaWhenEnumAccessesStaticFieldFromConstructorWithAssignment() { + rewriteRun( + //language=java + java( + """ + import java.time.LocalDate; + import java.time.format.DateTimeFormatter; + enum Test { + A, B; + + private static final DateTimeFormatter DATE_FORMAT = DateTimeFormatter.ofPattern("yyyy-MM-dd"); + + Test() { + Runnable r = new Runnable() { + @Override + public void run() { + String s = DATE_FORMAT.format(LocalDate.now()); + } + }; + } + } + """ + ) + ); + } + + @Test + @Issue("https://github.com/openrewrite/rewrite-static-analysis/issues/413") + void dontUseLambdaWhenEnumAccessesNonStaticFieldFromConstructor() { + rewriteRun( + //language=java + java( + """ + import java.time.LocalDate; + import java.time.format.DateTimeFormatter; + enum Test { + A, B; + + private final DateTimeFormatter DATE_FORMAT = DateTimeFormatter.ofPattern("yyyy-MM-dd"); + + Test() { + Runnable r = new Runnable() { + @Override + public void run() { + DATE_FORMAT.format(LocalDate.now()); + } + }; + } + } + """, + """ + import java.time.LocalDate; + import java.time.format.DateTimeFormatter; + enum Test { + A, B; + + private final DateTimeFormatter DATE_FORMAT = DateTimeFormatter.ofPattern("yyyy-MM-dd"); + + Test() { + Runnable r = () -> + DATE_FORMAT.format(LocalDate.now()); + } + } + """ + ) + ); + } + + @Test + @Issue("https://github.com/openrewrite/rewrite-static-analysis/issues/413") + void doUseLambdaWhenEnumAccessesStaticFieldFromMethod() { + rewriteRun( + //language=java + java( + """ + import java.time.LocalDate; + import java.time.format.DateTimeFormatter; + enum Test { + A, B; + + private static final DateTimeFormatter DATE_FORMAT = DateTimeFormatter.ofPattern("yyyy-MM-dd"); + + void test() { + Runnable r = new Runnable() { + @Override + public void run() { + DATE_FORMAT.format(LocalDate.now()); + } + }; + } + } + """, + """ + import java.time.LocalDate; + import java.time.format.DateTimeFormatter; + enum Test { + A, B; + + private static final DateTimeFormatter DATE_FORMAT = DateTimeFormatter.ofPattern("yyyy-MM-dd"); + + void test() { + Runnable r = () -> + DATE_FORMAT.format(LocalDate.now()); + } + } + """ + ) + ); + } } From d3d137c01bdd5508cc89e50cbee60961c4834fcd Mon Sep 17 00:00:00 2001 From: lingenj Date: Tue, 24 Dec 2024 17:42:51 +0100 Subject: [PATCH 2/3] `UseLambdaForFunctionalInterface` recipe should not convert when code uses a static field from enum constructor --- .../staticanalysis/UseLambdaForFunctionalInterfaceTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/java/org/openrewrite/staticanalysis/UseLambdaForFunctionalInterfaceTest.java b/src/test/java/org/openrewrite/staticanalysis/UseLambdaForFunctionalInterfaceTest.java index 12c49d554..8540944f0 100644 --- a/src/test/java/org/openrewrite/staticanalysis/UseLambdaForFunctionalInterfaceTest.java +++ b/src/test/java/org/openrewrite/staticanalysis/UseLambdaForFunctionalInterfaceTest.java @@ -833,7 +833,7 @@ public void run() { @Test @Issue("https://github.com/openrewrite/rewrite-static-analysis/issues/413") - void dontUseLambdaWhenEnumAccessesNonStaticFieldFromConstructor() { + void doUseLambdaWhenEnumAccessesNonStaticFieldFromConstructor() { rewriteRun( //language=java java( From f274ef1c2cf7c6f1dbbd46dd79c79fb277137910 Mon Sep 17 00:00:00 2001 From: lingenj Date: Fri, 27 Dec 2024 08:55:37 +0100 Subject: [PATCH 3/3] Simplify --- .../UseLambdaForFunctionalInterface.java | 44 ++-------- .../UseLambdaForFunctionalInterfaceTest.java | 86 +------------------ 2 files changed, 10 insertions(+), 120 deletions(-) diff --git a/src/main/java/org/openrewrite/staticanalysis/UseLambdaForFunctionalInterface.java b/src/main/java/org/openrewrite/staticanalysis/UseLambdaForFunctionalInterface.java index 759150477..a0f874070 100644 --- a/src/main/java/org/openrewrite/staticanalysis/UseLambdaForFunctionalInterface.java +++ b/src/main/java/org/openrewrite/staticanalysis/UseLambdaForFunctionalInterface.java @@ -61,6 +61,15 @@ public Duration getEstimatedEffortPerOccurrence() { @Override public TreeVisitor getVisitor() { return Repeat.repeatUntilStable(new JavaVisitor() { + @Override + public J visitClassDeclaration(J.ClassDeclaration classDecl, ExecutionContext ctx) { + // Don't convert anonymous classes to lambdas when located in an enum class, to avoid `Accessing static field from enum constructor is not allowed` errors. + if (classDecl.getKind() == J.ClassDeclaration.Kind.Type.Enum) { + return classDecl; + } + return super.visitClassDeclaration(classDecl, ctx); + } + @Override public J visitNewClass(J.NewClass newClass, ExecutionContext ctx) { J.NewClass n = (J.NewClass) super.visitNewClass(newClass, ctx); @@ -96,10 +105,6 @@ public J visitNewClass(J.NewClass newClass, ExecutionContext ctx) { StringBuilder templateBuilder = new StringBuilder(); J.MethodDeclaration methodDeclaration = (J.MethodDeclaration) n.getBody().getStatements().get(0); - if (isLambdaInEnumConstructorWithStaticField(getCursor(), methodDeclaration)) { - return n; - } - // If the functional interface method has type parameters, we can't replace it with a lambda. if (methodDeclaration.getTypeParameters() != null && !methodDeclaration.getTypeParameters().isEmpty()) { return n; @@ -141,37 +146,6 @@ public J visitNewClass(J.NewClass newClass, ExecutionContext ctx) { return n; } - private boolean isLambdaInEnumConstructorWithStaticField(Cursor cursor, J.MethodDeclaration md) { - if (md.getBody() == null) { - return false; - } - - for (Statement s : md.getBody().getStatements()) { - J.MethodInvocation e = null; - if (s instanceof J.VariableDeclarations && ((J.VariableDeclarations) s).getVariables().size() == 1 && ((J.VariableDeclarations) s).getVariables().get(0).getInitializer() instanceof J.MethodInvocation) { - e = (J.MethodInvocation) ((J.VariableDeclarations) s).getVariables().get(0).getInitializer(); - } else if (s instanceof J.MethodInvocation) { - e = (J.MethodInvocation) s; - } - - if (e != null && e.getSelect() instanceof J.Identifier) { - JavaType.Variable v = ((J.Identifier) e.getSelect()).getFieldType(); - if (v != null && v.hasFlags(Flag.Static)) { - JavaType owner = v.getOwner(); - if (owner instanceof JavaType.Class && ((JavaType.Class) owner).getKind() == JavaType.Class.Kind.Enum) { - J.MethodDeclaration parentMethodDeclaration = cursor.dropParentUntil(p -> p instanceof J.MethodDeclaration).getValue(); - J.ClassDeclaration parentClass = cursor.dropParentUntil(p -> p instanceof J.ClassDeclaration).getValue(); - if (parentMethodDeclaration.isConstructor() && owner.equals(parentClass.getType())) { - return true; - } - } - } - } - } - - return false; - } - private J maybeAddCast(J.Lambda lambda, J.NewClass original) { J parent = getCursor().getParentTreeCursor().getValue(); diff --git a/src/test/java/org/openrewrite/staticanalysis/UseLambdaForFunctionalInterfaceTest.java b/src/test/java/org/openrewrite/staticanalysis/UseLambdaForFunctionalInterfaceTest.java index 8540944f0..94fa0cebe 100644 --- a/src/test/java/org/openrewrite/staticanalysis/UseLambdaForFunctionalInterfaceTest.java +++ b/src/test/java/org/openrewrite/staticanalysis/UseLambdaForFunctionalInterfaceTest.java @@ -805,77 +805,7 @@ public void run() { @Test @Issue("https://github.com/openrewrite/rewrite-static-analysis/issues/413") - void dontUseLambdaWhenEnumAccessesStaticFieldFromConstructorWithAssignment() { - rewriteRun( - //language=java - java( - """ - import java.time.LocalDate; - import java.time.format.DateTimeFormatter; - enum Test { - A, B; - - private static final DateTimeFormatter DATE_FORMAT = DateTimeFormatter.ofPattern("yyyy-MM-dd"); - - Test() { - Runnable r = new Runnable() { - @Override - public void run() { - String s = DATE_FORMAT.format(LocalDate.now()); - } - }; - } - } - """ - ) - ); - } - - @Test - @Issue("https://github.com/openrewrite/rewrite-static-analysis/issues/413") - void doUseLambdaWhenEnumAccessesNonStaticFieldFromConstructor() { - rewriteRun( - //language=java - java( - """ - import java.time.LocalDate; - import java.time.format.DateTimeFormatter; - enum Test { - A, B; - - private final DateTimeFormatter DATE_FORMAT = DateTimeFormatter.ofPattern("yyyy-MM-dd"); - - Test() { - Runnable r = new Runnable() { - @Override - public void run() { - DATE_FORMAT.format(LocalDate.now()); - } - }; - } - } - """, - """ - import java.time.LocalDate; - import java.time.format.DateTimeFormatter; - enum Test { - A, B; - - private final DateTimeFormatter DATE_FORMAT = DateTimeFormatter.ofPattern("yyyy-MM-dd"); - - Test() { - Runnable r = () -> - DATE_FORMAT.format(LocalDate.now()); - } - } - """ - ) - ); - } - - @Test - @Issue("https://github.com/openrewrite/rewrite-static-analysis/issues/413") - void doUseLambdaWhenEnumAccessesStaticFieldFromMethod() { + void dontUseLambdaWhenEnumAccessesStaticFieldFromFromMethod() { rewriteRun( //language=java java( @@ -896,20 +826,6 @@ public void run() { }; } } - """, - """ - import java.time.LocalDate; - import java.time.format.DateTimeFormatter; - enum Test { - A, B; - - private static final DateTimeFormatter DATE_FORMAT = DateTimeFormatter.ofPattern("yyyy-MM-dd"); - - void test() { - Runnable r = () -> - DATE_FORMAT.format(LocalDate.now()); - } - } """ ) );