From a7398376429a775c17dc6cb501b064f47702a48e Mon Sep 17 00:00:00 2001 From: Patrick Ziegler Date: Thu, 25 Sep 2025 21:34:53 +0200 Subject: [PATCH] Don't traverse lambda expressions unless necessary We already skip anonymous classes outside of a few exceptions (e.g. EventQueue.invokeLater(Runnable)). Given that Java allows the Runnable to be implemented as both an anonymous class and a lambda expression, WindowBuilder needs to be extended to support both variants. --- .../wb/core/eval/ExecutionFlowUtils.java | 33 +++++------- .../core/eval/ExecutionFlowProvider.java | 38 ++++++++++++- .../rcp/parser/RcpExecutionFlowProvider.java | 17 +++--- .../parser/SwingExecutionFlowProvider.java | 16 +++--- org.eclipse.wb.tests/META-INF/MANIFEST.MF | 1 + .../core/eval/ExecutionFlowUtilsTest.java | 53 ++++++++++++------- 6 files changed, 100 insertions(+), 58 deletions(-) diff --git a/org.eclipse.wb.core.java/src/org/eclipse/wb/core/eval/ExecutionFlowUtils.java b/org.eclipse.wb.core.java/src/org/eclipse/wb/core/eval/ExecutionFlowUtils.java index ff73f4549..697340f89 100644 --- a/org.eclipse.wb.core.java/src/org/eclipse/wb/core/eval/ExecutionFlowUtils.java +++ b/org.eclipse.wb.core.java/src/org/eclipse/wb/core/eval/ExecutionFlowUtils.java @@ -1,5 +1,5 @@ /******************************************************************************* - * Copyright (c) 2011, 2024 Google, Inc. and others. + * Copyright (c) 2011, 2025 Google, Inc. and others. * * This program and the accompanying materials are made available under the * terms of the Eclipse Public License 2.0 which is available at @@ -35,7 +35,6 @@ import org.eclipse.wb.internal.core.utils.exception.DesignerException; import org.eclipse.wb.internal.core.utils.exception.ICoreExceptionConstants; import org.eclipse.wb.internal.core.utils.exception.MultipleConstructorsError; -import org.eclipse.wb.internal.core.utils.execution.ExecutionUtils; import org.eclipse.wb.internal.core.utils.external.ExternalFactoriesHelper; import org.eclipse.jdt.core.dom.ASTNode; @@ -55,6 +54,7 @@ import org.eclipse.jdt.core.dom.IMethodBinding; import org.eclipse.jdt.core.dom.IfStatement; import org.eclipse.jdt.core.dom.Initializer; +import org.eclipse.jdt.core.dom.LambdaExpression; import org.eclipse.jdt.core.dom.MethodDeclaration; import org.eclipse.jdt.core.dom.MethodInvocation; import org.eclipse.jdt.core.dom.PrefixExpression; @@ -132,6 +132,11 @@ public boolean visit(AnonymousClassDeclaration node) { return shouldVisitAnonymousClassDeclaration(node); } + @Override + public boolean visit(LambdaExpression node) { + return shouldVisitAnonymousClassDeclaration(node); + } + //////////////////////////////////////////////////////////////////////////// // // Frames @@ -393,15 +398,13 @@ private static ASTVisitor getInterceptingVisitor(final VisitingContext context, * In general we should not visit anonymous classes, they are usually event handlers. However * there are special cases, such as EventQueue.invokeLater(Runnable) in Swing. */ - private static boolean shouldVisitAnonymousClassDeclaration(final AnonymousClassDeclaration anonymous) { - return ExecutionUtils.runObjectLog(() -> { - for (ExecutionFlowProvider provider : getExecutionFlowProviders()) { - if (provider.shouldVisit(anonymous)) { - return true; - } + private static boolean shouldVisitAnonymousClassDeclaration(final ASTNode anonymous) { + for (ExecutionFlowProvider provider : getExecutionFlowProviders()) { + if (provider.shouldVisit(anonymous)) { + return true; } - return false; - }, false); + } + return false; } /** @@ -1267,11 +1270,7 @@ public Object invoke(Object obj, Method method, Object[] args) throws Throwable Class[] parameterTypes = method.getParameterTypes(); if (parameterTypes.length == 1) { Class parameterType = parameterTypes[0]; - if (method.getName().equals("visit")) { - if (parameterType == AnonymousClassDeclaration.class) { - visit(stub, (AnonymousClassDeclaration) args[0]); - } - } else if (method.getName().equals("endVisit")) { + if (method.getName().equals("endVisit")) { if (parameterType == ClassInstanceCreation.class) { endVisit(stub, (ClassInstanceCreation) args[0]); } else if (parameterType == MethodInvocation.class) { @@ -1289,10 +1288,6 @@ public Object invoke(Object obj, Method method, Object[] args) throws Throwable } } - private boolean visit(VisitorStub stub, AnonymousClassDeclaration node) { - return shouldVisitAnonymousClassDeclaration(node); - } - private void endVisit(VisitorStub stub, ClassInstanceCreation node) { // quick check { diff --git a/org.eclipse.wb.core.java/src/org/eclipse/wb/internal/core/eval/ExecutionFlowProvider.java b/org.eclipse.wb.core.java/src/org/eclipse/wb/internal/core/eval/ExecutionFlowProvider.java index 0e26c56bf..fe1844ee0 100644 --- a/org.eclipse.wb.core.java/src/org/eclipse/wb/internal/core/eval/ExecutionFlowProvider.java +++ b/org.eclipse.wb.core.java/src/org/eclipse/wb/internal/core/eval/ExecutionFlowProvider.java @@ -1,5 +1,5 @@ /******************************************************************************* - * Copyright (c) 2011 Google, Inc. + * Copyright (c) 2011, 2025 Google, Inc. and others. * * This program and the accompanying materials are made available under the * terms of the Eclipse Public License 2.0 which is available at @@ -12,8 +12,15 @@ *******************************************************************************/ package org.eclipse.wb.internal.core.eval; +import org.eclipse.wb.internal.core.utils.ast.AstNodeUtils; + +import org.eclipse.jdt.core.dom.ASTNode; import org.eclipse.jdt.core.dom.AnonymousClassDeclaration; +import org.eclipse.jdt.core.dom.ClassInstanceCreation; +import org.eclipse.jdt.core.dom.ITypeBinding; +import org.eclipse.jdt.core.dom.LambdaExpression; import org.eclipse.jdt.core.dom.MethodDeclaration; +import org.eclipse.jdt.core.dom.MethodInvocation; import org.eclipse.jdt.core.dom.TypeDeclaration; import java.awt.EventQueue; @@ -36,7 +43,34 @@ public MethodDeclaration getDefaultConstructor(TypeDeclaration typeDeclaration) return null; } - public boolean shouldVisit(AnonymousClassDeclaration anonymous) throws Exception { + public boolean shouldVisit(ASTNode node) { return false; } + + protected MethodInvocation getMethodInvocation(ASTNode node) { + return switch (node) { + case AnonymousClassDeclaration declaration -> { + ClassInstanceCreation creation = (ClassInstanceCreation) declaration.getParent(); + if (creation.getLocationInParent() == MethodInvocation.ARGUMENTS_PROPERTY) { + yield (MethodInvocation) creation.getParent(); + } + yield null; + } + case LambdaExpression expression -> { + if (expression.getLocationInParent() == MethodInvocation.ARGUMENTS_PROPERTY) { + yield (MethodInvocation) expression.getParent(); + } + yield null; + } + default -> null; + }; + } + + protected ITypeBinding getTypeBinding(ASTNode node) { + return switch (node) { + case AnonymousClassDeclaration declaration -> AstNodeUtils.getTypeBinding(declaration); + case LambdaExpression expression -> AstNodeUtils.getTypeBinding(expression); + default -> null; + }; + } } diff --git a/org.eclipse.wb.rcp/src/org/eclipse/wb/internal/rcp/parser/RcpExecutionFlowProvider.java b/org.eclipse.wb.rcp/src/org/eclipse/wb/internal/rcp/parser/RcpExecutionFlowProvider.java index dfc48e7fc..5d09936b7 100644 --- a/org.eclipse.wb.rcp/src/org/eclipse/wb/internal/rcp/parser/RcpExecutionFlowProvider.java +++ b/org.eclipse.wb.rcp/src/org/eclipse/wb/internal/rcp/parser/RcpExecutionFlowProvider.java @@ -1,5 +1,5 @@ /******************************************************************************* - * Copyright (c) 2011 Google, Inc. + * Copyright (c) 2011, 2025 Google, Inc. and others. * * This program and the accompanying materials are made available under the * terms of the Eclipse Public License 2.0 which is available at @@ -15,8 +15,7 @@ import org.eclipse.wb.internal.core.eval.ExecutionFlowProvider; import org.eclipse.wb.internal.core.utils.ast.AstNodeUtils; -import org.eclipse.jdt.core.dom.AnonymousClassDeclaration; -import org.eclipse.jdt.core.dom.ClassInstanceCreation; +import org.eclipse.jdt.core.dom.ASTNode; import org.eclipse.jdt.core.dom.ITypeBinding; import org.eclipse.jdt.core.dom.MethodDeclaration; import org.eclipse.jdt.core.dom.MethodInvocation; @@ -75,12 +74,10 @@ public MethodDeclaration getDefaultConstructor(TypeDeclaration typeDeclaration) } @Override - public boolean shouldVisit(AnonymousClassDeclaration anonymous) throws Exception { - // Realm.runWithDefault() - if (AstNodeUtils.isSuccessorOf(anonymous, "java.lang.Runnable")) { - ClassInstanceCreation creation = (ClassInstanceCreation) anonymous.getParent(); - if (creation.getLocationInParent() == MethodInvocation.ARGUMENTS_PROPERTY) { - MethodInvocation invocation = (MethodInvocation) creation.getParent(); + public boolean shouldVisit(ASTNode node) { + if (AstNodeUtils.isSuccessorOf(getTypeBinding(node), "java.lang.Runnable")) { + MethodInvocation invocation = getMethodInvocation(node); + if (invocation != null) { return AstNodeUtils.isMethodInvocation( invocation, "org.eclipse.core.databinding.observable.Realm", @@ -88,6 +85,6 @@ public boolean shouldVisit(AnonymousClassDeclaration anonymous) throws Exception } } // unknown pattern - return false; + return super.shouldVisit(node); } } diff --git a/org.eclipse.wb.swing/src/org/eclipse/wb/internal/swing/parser/SwingExecutionFlowProvider.java b/org.eclipse.wb.swing/src/org/eclipse/wb/internal/swing/parser/SwingExecutionFlowProvider.java index 4d3d316c3..40ac274a6 100644 --- a/org.eclipse.wb.swing/src/org/eclipse/wb/internal/swing/parser/SwingExecutionFlowProvider.java +++ b/org.eclipse.wb.swing/src/org/eclipse/wb/internal/swing/parser/SwingExecutionFlowProvider.java @@ -1,5 +1,5 @@ /******************************************************************************* - * Copyright (c) 2011 Google, Inc. + * Copyright (c) 2011, 2025 Google, Inc. and others. * * This program and the accompanying materials are made available under the * terms of the Eclipse Public License 2.0 which is available at @@ -15,8 +15,7 @@ import org.eclipse.wb.internal.core.eval.ExecutionFlowProvider; import org.eclipse.wb.internal.core.utils.ast.AstNodeUtils; -import org.eclipse.jdt.core.dom.AnonymousClassDeclaration; -import org.eclipse.jdt.core.dom.ClassInstanceCreation; +import org.eclipse.jdt.core.dom.ASTNode; import org.eclipse.jdt.core.dom.ITypeBinding; import org.eclipse.jdt.core.dom.MethodDeclaration; import org.eclipse.jdt.core.dom.MethodInvocation; @@ -45,11 +44,10 @@ public MethodDeclaration getDefaultConstructor(TypeDeclaration typeDeclaration) } @Override - public boolean shouldVisit(AnonymousClassDeclaration anonymous) throws Exception { - if (AstNodeUtils.isSuccessorOf(anonymous.resolveBinding(), "java.lang.Runnable")) { - ClassInstanceCreation creation = (ClassInstanceCreation) anonymous.getParent(); - if (creation.getLocationInParent() == MethodInvocation.ARGUMENTS_PROPERTY) { - MethodInvocation invocation = (MethodInvocation) creation.getParent(); + public boolean shouldVisit(ASTNode node) { + if (AstNodeUtils.isSuccessorOf(getTypeBinding(node), "java.lang.Runnable")) { + MethodInvocation invocation = getMethodInvocation(node); + if (invocation != null) { return AstNodeUtils.isMethodInvocation( invocation, "java.awt.EventQueue", @@ -68,6 +66,6 @@ public boolean shouldVisit(AnonymousClassDeclaration anonymous) throws Exception "invokeAndWait(java.lang.Runnable)"); } } - return super.shouldVisit(anonymous); + return super.shouldVisit(node); } } diff --git a/org.eclipse.wb.tests/META-INF/MANIFEST.MF b/org.eclipse.wb.tests/META-INF/MANIFEST.MF index 436683aeb..ca813c471 100644 --- a/org.eclipse.wb.tests/META-INF/MANIFEST.MF +++ b/org.eclipse.wb.tests/META-INF/MANIFEST.MF @@ -50,6 +50,7 @@ Require-Bundle: org.eclipse.core.runtime;bundle-version="[3.31.100,4.0.0)", org.objenesis;bundle-version="[3.4.0,4.0.0)", net.bytebuddy.byte-buddy;bundle-version="[1.14.16,2.0.0)", junit-jupiter-api;bundle-version="[5.10.2,6.0.0)", + junit-jupiter-params;bundle-version="[5.10.2,6.0.0)", junit-platform-suite-api;bundle-version="[1.10.2,2.0.0)", org.opentest4j;bundle-version="[1.3.0,2.0.0)" Bundle-ActivationPolicy: lazy diff --git a/org.eclipse.wb.tests/src/org/eclipse/wb/tests/designer/core/eval/ExecutionFlowUtilsTest.java b/org.eclipse.wb.tests/src/org/eclipse/wb/tests/designer/core/eval/ExecutionFlowUtilsTest.java index 1f066316c..83bb2ce1b 100644 --- a/org.eclipse.wb.tests/src/org/eclipse/wb/tests/designer/core/eval/ExecutionFlowUtilsTest.java +++ b/org.eclipse.wb.tests/src/org/eclipse/wb/tests/designer/core/eval/ExecutionFlowUtilsTest.java @@ -1,5 +1,5 @@ /******************************************************************************* - * Copyright (c) 2011, 2024 Google, Inc. and others. + * Copyright (c) 2011, 2025 Google, Inc. and others. * * This program and the accompanying materials are made available under the * terms of the Eclipse Public License 2.0 which is available at @@ -43,6 +43,8 @@ import org.eclipse.jdt.core.dom.ExpressionStatement; import org.eclipse.jdt.core.dom.FieldAccess; import org.eclipse.jdt.core.dom.FieldDeclaration; +import org.eclipse.jdt.core.dom.ITypeBinding; +import org.eclipse.jdt.core.dom.LambdaExpression; import org.eclipse.jdt.core.dom.MethodDeclaration; import org.eclipse.jdt.core.dom.MethodInvocation; import org.eclipse.jdt.core.dom.SimpleName; @@ -58,6 +60,8 @@ import org.assertj.core.api.Assertions; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; import java.lang.reflect.InvocationTargetException; import java.lang.reflect.Method; @@ -830,7 +834,7 @@ public void endVisit(SimpleName node) { //////////////////////////////////////////////////////////////////////////// // - // Visiting and AnonymousClassDeclaration + // Visiting and AnonymousClassDeclaration/LambdaExpressios // //////////////////////////////////////////////////////////////////////////// /** @@ -838,9 +842,10 @@ public void endVisit(SimpleName node) { *

* {@link Runnable} in {@link Thread} should not be visited. */ - @Test - public void test_visit_AnonymousClassDeclaration_noVisit() throws Exception { - String threadCode = "new Thread(new Runnable() {public void run() {int a;}});"; + @ParameterizedTest + @ValueSource(strings = { "new Runnable() {public void run() {int a;}}", "() -> {int a;}" }) + public void test_visit_AnonymousClassDeclaration_noVisit(String runnableCode) throws Exception { + String threadCode = "new Thread(" + runnableCode + ");"; String code = "void root() {" + threadCode + "}"; String expectedNodes[] = {threadCode}; check_visitNodes(code, expectedNodes, "root()"); @@ -852,9 +857,10 @@ public void test_visit_AnonymousClassDeclaration_noVisit() throws Exception { * {@link Runnable} in {@link Thread} should not be visited, and invocations also should not be * followed. */ - @Test - public void test_visit_AnonymousClassDeclaration_withInvocation() throws Exception { - String threadCode = "new Thread(new Runnable() {public void run() {foo();}});"; + @ParameterizedTest + @ValueSource(strings = { "new Runnable() {public void run() {foo();}}", "() -> {foo();}" }) + public void test_visit_AnonymousClassDeclaration_withInvocation(String runnableCode) throws Exception { + String threadCode = "new Thread(" + runnableCode + ");"; String code = "void root() {" + threadCode + "} void foo() {int b;}"; String expectedNodes[] = {threadCode}; check_visitNodes(code, expectedNodes, "root()"); @@ -865,13 +871,23 @@ public void test_visit_AnonymousClassDeclaration_withInvocation() throws Excepti */ public static class ExecutionFlowProvider_RunnableInThread extends ExecutionFlowProvider { @Override - public boolean shouldVisit(AnonymousClassDeclaration anonymous) throws Exception { - if (AstNodeUtils.isSuccessorOf(anonymous.resolveBinding(), "java.lang.Runnable")) { - ASTNode anonymousCreation = anonymous.getParent(); - if (anonymousCreation.getLocationInParent() == ClassInstanceCreation.ARGUMENTS_PROPERTY) { - Expression enclosingCreation = (ClassInstanceCreation) anonymousCreation.getParent(); - if (AstNodeUtils.isSuccessorOf(enclosingCreation, "java.lang.Thread")) { - return true; + public boolean shouldVisit(ASTNode node) { + ITypeBinding typeBinding = getTypeBinding(node); + if (AstNodeUtils.isSuccessorOf(typeBinding, "java.lang.Runnable")) { + if (node instanceof AnonymousClassDeclaration anonymous) { + ASTNode anonymousCreation = anonymous.getParent(); + if (anonymousCreation.getLocationInParent() == ClassInstanceCreation.ARGUMENTS_PROPERTY) { + Expression enclosingCreation = (ClassInstanceCreation) anonymousCreation.getParent(); + if (AstNodeUtils.isSuccessorOf(enclosingCreation, "java.lang.Thread")) { + return true; + } + } + } else if (node instanceof LambdaExpression lambda) { + if (lambda.getLocationInParent() == ClassInstanceCreation.ARGUMENTS_PROPERTY) { + Expression enclosingCreation = (ClassInstanceCreation) lambda.getParent(); + if (AstNodeUtils.isSuccessorOf(enclosingCreation, "java.lang.Thread")) { + return true; + } } } } @@ -886,8 +902,9 @@ public boolean shouldVisit(AnonymousClassDeclaration anonymous) throws Exception * should not be followed. However we install special {@link ExecutionFlowProvider} that allows * such visiting. */ - @Test - public void test_visit_AnonymousClassDeclaration_withInvocation_doVisit() throws Exception { + @ParameterizedTest + @ValueSource(strings = { "new Runnable() {public void run() {foo();}}", "() -> {foo();}" }) + public void test_visit_AnonymousClassDeclaration_withInvocation_doVisit(String runnableCode) throws Exception { TestBundle testBundle = new TestBundle(); try { Class providerClass = ExecutionFlowProvider_RunnableInThread.class; @@ -897,7 +914,7 @@ public void test_visit_AnonymousClassDeclaration_withInvocation_doVisit() throws ""); testBundle.install(); try { - String threadCode = "new Thread(new Runnable() {public void run() {foo();}});"; + String threadCode = "new Thread(" + runnableCode + ");"; String code = "void root() {" + threadCode + "} void foo() {int b;}"; String expectedNodes[] = {"int b;", "foo();", threadCode}; check_visitNodes(code, expectedNodes, "root()");