Merge pull request #3786 from Rawi01/eclipse-extract-variable

Return generated node for side effect check

src/eclipseAgent/lombok/eclipse/agent/EclipsePatcher.java

@@ -96,6 +96,7 @@ public class EclipsePatcher implements AgentLauncher.AgentLaunchable {
 		patchSyntaxAndOccurrencesHighlighting(sm);
 		patchSortMembersOperation(sm);
 		patchExtractInterfaceAndPullUp(sm);
+		patchExtractVariable(sm);
 		patchAboutDialog(sm);
 		patchEclipseDebugPatches(sm);
 		patchJavadoc(sm);
@@ -225,6 +226,17 @@ public class EclipsePatcher implements AgentLauncher.AgentLaunchable {
 				.build());
 	}
 	
+	private static void patchExtractVariable(ScriptManager sm) {
+		/* Fix sourceEnding for generated nodes to avoid null pointer */
+		sm.addScriptIfWitness(OSGI_TYPES, ScriptBuilder.replaceMethodCall()
+				.target(new MethodTarget("org.eclipse.jdt.internal.corext.refactoring.util.SideEffectChecker", "findFunctionDefinition", "org.eclipse.jdt.core.dom.MethodDeclaration", "org.eclipse.jdt.core.dom.ITypeBinding", "org.eclipse.jdt.core.dom.IMethodBinding"))
+				.methodToReplace(new Hook("org.eclipse.jdt.core.dom.NodeFinder", "perform", "org.eclipse.jdt.core.dom.ASTNode", "org.eclipse.jdt.core.dom.ASTNode", "org.eclipse.jdt.core.ISourceRange"))
+				.replacementMethod(new Hook("lombok.launch.PatchFixesHider$PatchFixes", "findGeneratedNode", "org.eclipse.jdt.core.dom.ASTNode", "org.eclipse.jdt.core.dom.ASTNode", "org.eclipse.jdt.core.ISourceRange", "org.eclipse.jdt.core.dom.IMethodBinding"))
+				.requestExtra(StackRequest.PARAM2)
+				.transplant()
+				.build());
+	}
+	
 	private static void patchInline(ScriptManager sm) {
 		sm.addScriptIfWitness(OSGI_TYPES, ScriptBuilder.wrapReturnValue()
 				.target(new MethodTarget("org.eclipse.jdt.internal.corext.refactoring.code.SourceProvider", "getCodeBlocks", "java.lang.String[]", "org.eclipse.jdt.internal.corext.refactoring.code.CallContext", "org.eclipse.jdt.core.dom.rewrite.ImportRewrite"))

src/eclipseAgent/lombok/launch/PatchFixesHider.java

@@ -41,12 +41,16 @@ import org.eclipse.jdt.core.IAnnotatable;
 import org.eclipse.jdt.core.IAnnotation;
 import org.eclipse.jdt.core.IJavaElement;
 import org.eclipse.jdt.core.IMethod;
+import org.eclipse.jdt.core.ISourceRange;
 import org.eclipse.jdt.core.IType;
 import org.eclipse.jdt.core.JavaModelException;
 import org.eclipse.jdt.core.dom.AST;
 import org.eclipse.jdt.core.dom.ASTNode;
+import org.eclipse.jdt.core.dom.CompilationUnit;
+import org.eclipse.jdt.core.dom.IMethodBinding;
 import org.eclipse.jdt.core.dom.MethodDeclaration;
 import org.eclipse.jdt.core.dom.Name;
+import org.eclipse.jdt.core.dom.NodeFinder;
 import org.eclipse.jdt.core.dom.ReturnStatement;
 import org.eclipse.jdt.core.dom.SimpleName;
 import org.eclipse.jdt.core.dom.SingleVariableDeclaration;
@@ -951,6 +955,16 @@ final class PatchFixesHider {
 				return blocks;
 			}
 		}
+		
+		public static ASTNode findGeneratedNode(ASTNode root, ISourceRange sourceRange, IMethodBinding methodBinding) {
+			ASTNode result = NodeFinder.perform(root, sourceRange);
+			if (result instanceof MethodDeclaration){
+				return result;
+			}
+			
+			CompilationUnit cu = (CompilationUnit) root;
+			return cu.findDeclaringNode(methodBinding.getKey());
+		}
 	}
 	
 	public static class FieldInitializer {

test/eclipse/resource/extractvariable/multiple/after/A.java

@@ -0,0 +1,15 @@
+package pkg;
+
+import java.util.Arrays;
+
+import lombok.Getter;
+
+@Getter
+public class A {
+	private String string;
+	
+	public List<String> test() {
+		String temp = getString();
+		return Arrays.asList(temp, temp, temp);
+	}
+}

test/eclipse/resource/extractvariable/multiple/before/A.java

@@ -0,0 +1,14 @@
+package pkg;
+
+import java.util.Arrays;
+
+import lombok.Getter;
+
+@Getter
+public class A {
+	private String string;
+	
+	public List<String> test() {
+		return Arrays.asList(getString(), getString(), getString());
+	}
+}

test/eclipse/resource/extractvariable/single/after/A.java

@@ -0,0 +1,13 @@
+package pkg;
+
+import lombok.Getter;
+
+@Getter
+public class A {
+	private String string;
+	
+	public String test() {
+		String temp = getString();
+		return temp;
+	}
+}

test/eclipse/resource/extractvariable/single/before/A.java

@@ -0,0 +1,12 @@
+package pkg;
+
+import lombok.Getter;
+
+@Getter
+public class A {
+	private String string;
+	
+	public String test() {
+		return getString();
+	}
+}

test/eclipse/src/lombok/eclipse/EclipseTests.java

@@ -31,12 +31,13 @@ import lombok.eclipse.edit.SelectTest;
 import lombok.eclipse.misc.DelegateTest;
 import lombok.eclipse.misc.JavadocTest;
 import lombok.eclipse.refactoring.ExtractInterfaceTest;
+import lombok.eclipse.refactoring.ExtractVariableTest;
 import lombok.eclipse.refactoring.InlineTest;
 import lombok.eclipse.refactoring.RenameTest;
 import lombok.eclipse.references.FindReferencesTest;
 
 @RunWith(Suite.class)
-@SuiteClasses({ExtractInterfaceTest.class, RenameTest.class, SelectTest.class, CleanupTest.class, FindReferencesTest.class, InlineTest.class, NoErrorsTest.class, JavadocTest.class, DelegateTest.class})
+@SuiteClasses({ExtractInterfaceTest.class, RenameTest.class, SelectTest.class, CleanupTest.class, FindReferencesTest.class, InlineTest.class, NoErrorsTest.class, JavadocTest.class, DelegateTest.class, ExtractVariableTest.class})
 public class EclipseTests {
 	
 }

test/eclipse/src/lombok/eclipse/RefactoringUtils.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2022 The Project Lombok Authors.
+ * Copyright (C) 2022-2024 The Project Lombok Authors.
  *
  * Permission is hereby granted, free of charge, to any person obtaining a copy
  * of this software and associated documentation files (the "Software"), to deal
@@ -21,7 +21,7 @@
  */
 package lombok.eclipse;
 
-import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.*;
 
 import org.eclipse.core.resources.ResourcesPlugin;
 import org.eclipse.core.runtime.CoreException;
@@ -45,7 +45,7 @@ public class RefactoringUtils {
 		
 		ResourcesPlugin.getWorkspace().run(perform, null);
 		
-		assertTrue("Condition failed", change.getConditionCheckingStatus().isOK());
+		assertEquals("Condition failed", "<OK\n>", change.getConditionCheckingStatus().toString());
 		assertTrue("Perform failed", perform.changeExecuted());
 	}
 }

test/eclipse/src/lombok/eclipse/refactoring/ExtractVariableTest.java

@@ -0,0 +1,65 @@
+/*
+ * Copyright (C) 2024 The Project Lombok Authors.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+ * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+package lombok.eclipse.refactoring;
+
+import static lombok.eclipse.RefactoringUtils.performRefactoring;
+
+import org.eclipse.jdt.core.ICompilationUnit;
+import org.eclipse.jdt.core.dom.CompilationUnit;
+import org.eclipse.jdt.internal.corext.refactoring.code.ExtractTempRefactoring;
+import org.eclipse.jdt.internal.corext.refactoring.util.RefactoringASTParser;
+import org.eclipse.jdt.internal.ui.javaeditor.ASTProvider;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+
+import lombok.eclipse.EclipseRunner;
+import lombok.eclipse.SetupBeforeAfterTest;
+
+@RunWith(EclipseRunner.class)
+public class ExtractVariableTest {
+	
+	@Rule
+	public SetupBeforeAfterTest setup = new SetupBeforeAfterTest();
+
+	@Test
+	public void single() throws Exception {
+		ICompilationUnit cu = setup.getPackageFragment().getCompilationUnit("A.java");
+		
+		CompilationUnit compilationUnit = new RefactoringASTParser(ASTProvider.SHARED_AST_LEVEL).parse(cu, true);
+		
+		ExtractTempRefactoring extractTempRefactoring = new ExtractTempRefactoring(compilationUnit, 121, 11);
+		extractTempRefactoring.setTempName("temp");
+		performRefactoring(extractTempRefactoring);
+	}
+
+	@Test
+	public void multiple() throws Exception {
+		ICompilationUnit cu = setup.getPackageFragment().getCompilationUnit("A.java");
+		
+		CompilationUnit compilationUnit = new RefactoringASTParser(ASTProvider.SHARED_AST_LEVEL).parse(cu, true);
+		
+		ExtractTempRefactoring extractTempRefactoring = new ExtractTempRefactoring(compilationUnit, 167, 11);
+		extractTempRefactoring.setTempName("temp");
+		performRefactoring(extractTempRefactoring);
+	}
+}