Skip to content

Commit

Permalink
[fixes projectlombok#3783] Return generated node for side effect check
Browse files Browse the repository at this point in the history
  • Loading branch information
Rawi01 committed Nov 23, 2024
1 parent 8ae385c commit 4f29214
Show file tree
Hide file tree
Showing 9 changed files with 150 additions and 4 deletions.
12 changes: 12 additions & 0 deletions src/eclipseAgent/lombok/eclipse/agent/EclipsePatcher.java
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,7 @@ public String mapResourceName(int classFileFormatVersion, String resourceName) {
patchSyntaxAndOccurrencesHighlighting(sm);
patchSortMembersOperation(sm);
patchExtractInterfaceAndPullUp(sm);
patchExtractVariable(sm);
patchAboutDialog(sm);
patchEclipseDebugPatches(sm);
patchJavadoc(sm);
Expand Down Expand Up @@ -225,6 +226,17 @@ private static void patchExtractInterfaceAndPullUp(ScriptManager sm) {
.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"))
Expand Down
14 changes: 14 additions & 0 deletions src/eclipseAgent/lombok/launch/PatchFixesHider.java
Original file line number Diff line number Diff line change
Expand Up @@ -41,12 +41,16 @@
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;
Expand Down Expand Up @@ -951,6 +955,16 @@ public static String[] getRealCodeBlocks(String[] blocks, SourceProvider sourceP
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 {
Expand Down
15 changes: 15 additions & 0 deletions test/eclipse/resource/extractvariable/multiple/after/A.java
Original file line number Diff line number Diff line change
@@ -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);
}
}
14 changes: 14 additions & 0 deletions test/eclipse/resource/extractvariable/multiple/before/A.java
Original file line number Diff line number Diff line change
@@ -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());
}
}
13 changes: 13 additions & 0 deletions test/eclipse/resource/extractvariable/single/after/A.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
package pkg;

import lombok.Getter;

@Getter
public class A {
private String string;

public String test() {
String temp = getString();
return temp;
}
}
12 changes: 12 additions & 0 deletions test/eclipse/resource/extractvariable/single/before/A.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
package pkg;

import lombok.Getter;

@Getter
public class A {
private String string;

public String test() {
return getString();
}
}
3 changes: 2 additions & 1 deletion test/eclipse/src/lombok/eclipse/EclipseTests.java
Original file line number Diff line number Diff line change
Expand Up @@ -31,12 +31,13 @@
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 {

}
6 changes: 3 additions & 3 deletions test/eclipse/src/lombok/eclipse/RefactoringUtils.java
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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;
Expand All @@ -45,7 +45,7 @@ public static void performRefactoring(Refactoring refactoring) throws CoreExcept

ResourcesPlugin.getWorkspace().run(perform, null);

assertTrue("Condition failed", change.getConditionCheckingStatus().isOK());
assertEquals("Condition failed", "<OK\n>", change.getConditionCheckingStatus().toString());
assertTrue("Perform failed", perform.changeExecuted());
}
}
Original file line number Diff line number Diff line change
@@ -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);
}
}

0 comments on commit 4f29214

Please sign in to comment.