[fixes projectlombok#1758] Remove generated nodes in search results

Lombok can now handle generated methods in nested classes.
janrieke · Dec 6, 2023 · 664a229 · 664a229
1 parent d6ee06c
commit 664a229
Show file tree

Hide file tree

Showing 8 changed files with 86 additions and 30 deletions.
diff --git a/src/core/lombok/eclipse/TransformEclipseAST.java b/src/core/lombok/eclipse/TransformEclipseAST.java
@@ -170,6 +170,9 @@ public static void transform(Parser parser, CompilationUnitDeclaration ast) {
 		if (fileName != null && String.valueOf(fileName).endsWith("module-info.java")) return;
 
 		if (Symbols.hasSymbol("lombok.disable")) return;
+		// The IndexingParser only supports a single import statement, restricting lombok annotations to either fully qualified ones or
+		// those specified in the last import statement. To avoid handling hard to reproduce edge cases, we opt to ignore the entire parser.
+		if ("org.eclipse.jdt.internal.core.search.indexing.IndexingParser".equals(parser.getClass().getName())) return;
 		if (alreadyTransformed(ast)) return;
 
 		// Do NOT abort if (ast.bits & ASTNode.HasAllMethodBodies) != 0 - that doesn't work.

diff --git a/src/eclipseAgent/lombok/eclipse/agent/EclipsePatcher.java b/src/eclipseAgent/lombok/eclipse/agent/EclipsePatcher.java
@@ -132,14 +132,16 @@ private static void patchRenameField(ScriptManager sm) {
 				.methodToWrap(new Hook("org.eclipse.jdt.internal.corext.refactoring.RefactoringSearchEngine", "search", "org.eclipse.jdt.internal.corext.refactoring.SearchResultGroup[]", "org.eclipse.jdt.core.search.SearchPattern","org.eclipse.jdt.core.search.IJavaSearchScope","org.eclipse.core.runtime.IProgressMonitor","org.eclipse.ltk.core.refactoring.RefactoringStatus"))
 				.wrapMethod(new Hook("lombok.launch.PatchFixesHider$PatchFixes", "createFakeSearchResult", "org.eclipse.jdt.internal.corext.refactoring.SearchResultGroup[]", "org.eclipse.jdt.internal.corext.refactoring.SearchResultGroup[]", "java.lang.Object"))
 				.requestExtra(StackRequest.THIS)
-				.transplant().build());
+				.transplant()
+				.build());
 
-		/* Filter search results which are Generated and based on Fields, e.g. Generated getters/setters */
-		sm.addScript(ScriptBuilder.wrapMethodCall()
-				.target(new MethodTarget("org.eclipse.jdt.internal.corext.refactoring.rename.RenameFieldProcessor", "addAccessorOccurrences", "void", "org.eclipse.core.runtime.IProgressMonitor", "org.eclipse.jdt.core.IMethod", "java.lang.String","java.lang.String","org.eclipse.ltk.core.refactoring.RefactoringStatus"))
-				.methodToWrap(new Hook("org.eclipse.jdt.internal.corext.refactoring.SearchResultGroup", "getSearchResults", "org.eclipse.jdt.core.search.SearchMatch[]"))
-				.wrapMethod(new Hook("lombok.launch.PatchFixesHider$PatchFixes", "removeGenerated", "org.eclipse.jdt.core.search.SearchMatch[]", "org.eclipse.jdt.core.search.SearchMatch[]"))
-				.transplant().build());
+		/* Prevent adding generated nodes as possible search matches */
+		sm.addScript(ScriptBuilder.exitEarly()
+				.target(new MethodTarget("org.eclipse.jdt.internal.core.search.matching.MatchingNodeSet", "addPossibleMatch", "void", "org.eclipse.jdt.internal.compiler.ast.ASTNode"))
+				.decisionMethod(new Hook("lombok.launch.PatchFixesHider$PatchFixes", "isGenerated", "boolean", "org.eclipse.jdt.internal.compiler.ast.ASTNode"))
+				.request(StackRequest.PARAM1)
+				.transplant()
+				.build());
 	}
 
 	private static void patchExtractInterfaceAndPullUp(ScriptManager sm) {

diff --git a/src/eclipseAgent/lombok/launch/PatchFixesHider.java b/src/eclipseAgent/lombok/launch/PatchFixesHider.java
@@ -38,7 +38,6 @@
 import org.eclipse.core.runtime.CoreException;
 import org.eclipse.jdt.core.IAnnotatable;
 import org.eclipse.jdt.core.IAnnotation;
-import org.eclipse.jdt.core.IField;
 import org.eclipse.jdt.core.IJavaElement;
 import org.eclipse.jdt.core.IMethod;
 import org.eclipse.jdt.core.IType;
@@ -782,27 +781,6 @@ public static IMethod[] removeGeneratedMethods(IMethod[] methods) throws Excepti
 			return result.size() == methods.length ? methods : result.toArray(new IMethod[0]);
 		}
 
-		public static SearchMatch[] removeGenerated(SearchMatch[] returnValue) {
-			List<SearchMatch> result = new ArrayList<SearchMatch>();
-			for (int j = 0; j < returnValue.length; j++) {
-				SearchMatch searchResult = returnValue[j];
-				if (searchResult.getElement() instanceof IField) {
-					IField field = (IField) searchResult.getElement();
-
-					// can not check for value=lombok because annotation is
-					// not fully resolved
-					IAnnotation annotation = field.getAnnotation("Generated");
-					if (annotation != null) {
-						// Method generated at field location, skip
-						continue;
-					}
-
-				}
-				result.add(searchResult);
-			}
-			return result.toArray(new SearchMatch[0]);
-		}
-
 		public static SearchResultGroup[] createFakeSearchResult(SearchResultGroup[] returnValue,
 				Object/*
 						 * org.eclipse.jdt.internal.corext.refactoring.rename.

diff --git a/test/eclipse/resource/rename/data/after/A.java b/test/eclipse/resource/rename/data/after/A.java
@@ -0,0 +1,12 @@
+package pkg;
+
+import lombok.Data;
+
+@Data
+public class A {
+	private String newString;
+
+	public String test() {
+		return getNewString();
+	}
+}
diff --git a/test/eclipse/resource/rename/data/before/A.java b/test/eclipse/resource/rename/data/before/A.java
@@ -0,0 +1,12 @@
+package pkg;
+
+import lombok.Data;
+
+@Data
+public class A {
+	private String string;
+
+	public String test() {
+		return getString();
+	}
+}
diff --git a/test/eclipse/resource/rename/nestedClass/after/A.java b/test/eclipse/resource/rename/nestedClass/after/A.java
@@ -0,0 +1,9 @@
+package pkg;
+
+public class A {
+	@lombok.Getter
+	@lombok.Setter
+	public static class Nested {
+		private String newString;
+	}
+}
diff --git a/test/eclipse/resource/rename/nestedClass/before/A.java b/test/eclipse/resource/rename/nestedClass/before/A.java
@@ -0,0 +1,9 @@
+package pkg;
+
+public class A {
+	@lombok.Getter
+	@lombok.Setter
+	public static class Nested {
+		private String string;
+	}
+}
diff --git a/test/eclipse/src/lombok/eclipse/refactoring/RenameTest.java b/test/eclipse/src/lombok/eclipse/refactoring/RenameTest.java
@@ -21,7 +21,7 @@ public class RenameTest {
 
 	@Rule
 	public SetupBeforeAfterTest setup = new SetupBeforeAfterTest();
-
+//	
 	@Test
 	public void simple() throws Exception {
 		ICompilationUnit cu = setup.getPackageFragment().getCompilationUnit("A.java");
@@ -30,6 +30,8 @@ public void simple() throws Exception {
 
 		RenameFieldProcessor renameFieldProcessor = new RenameFieldProcessor(field);
 		renameFieldProcessor.setNewElementName("newString");
+		renameFieldProcessor.setRenameGetter(true);
+		renameFieldProcessor.setRenameSetter(true);
 
 		performRefactoring(renameFieldProcessor);
 	}
@@ -83,4 +85,33 @@ public void extensionMethod() throws Exception {
 
 		performRefactoring(renameMethodProcessor);
 	}
+
+	@Test
+	public void data() throws Exception {
+		ICompilationUnit cu = setup.getPackageFragment().getCompilationUnit("A.java");
+		IType type = cu.findPrimaryType();
+		IField field = type.getField("string");
+
+		RenameFieldProcessor renameFieldProcessor = new RenameFieldProcessor(field);
+		renameFieldProcessor.setNewElementName("newString");
+		renameFieldProcessor.setRenameGetter(true);
+		renameFieldProcessor.setRenameSetter(true);
+
+		performRefactoring(renameFieldProcessor);
+	}
+
+	@Test
+	public void nestedClass() throws Exception {
+		ICompilationUnit cu = setup.getPackageFragment().getCompilationUnit("A.java");
+		IType type = cu.findPrimaryType();
+		IType nestedType = type.getType("Nested");
+		IField field = nestedType.getField("string");
+
+		RenameFieldProcessor renameFieldProcessor = new RenameFieldProcessor(field);
+		renameFieldProcessor.setNewElementName("newString");
+		renameFieldProcessor.setRenameGetter(true);
+		renameFieldProcessor.setRenameSetter(true);
+
+		performRefactoring(renameFieldProcessor);
+	}
 }