Merge pull request #3800 from amishra-u/ast-pos

fix: Don't alter the AST node positions of original code.

AUTHORS

@@ -4,6 +4,7 @@ Adam Juraszek <juriad@gmail.com>
 Aleksandr Zhelezniak <lekan1992@gmail.com>
 Amine Touzani <ttzn.dev@gmail.com>
 Andre Brait <andrebrait@gmail.com>
+Anshuman Mishra <a.mishra@uber.com>
 Bulgakov Alexander <buls@yandex.ru>
 Caleb Brinkman <floralvikings@gmail.com>
 Christian Nüssgens <christian@nuessgens.com>

src/core/lombok/javac/handlers/HandleConstructor.java

@@ -364,7 +364,7 @@ public class HandleConstructor {
 		if (addConstructorProperties && !isLocalType(typeNode) && LombokOptionsFactory.getDelombokOptions(typeNode.getContext()).getFormatPreferences().generateConstructorProperties()) {
 			addConstructorProperties(mods, typeNode, fieldsToParam);
 		}
-		if (onConstructor != null) mods.annotations = mods.annotations.appendList(copyAnnotations(onConstructor));
+		if (onConstructor != null) mods.annotations = mods.annotations.appendList(copyAnnotations(onConstructor, maker));
 		return recursiveSetGeneratedBy(maker.MethodDef(mods, typeNode.toName("<init>"),
 			null, List.<JCTypeParameter>nil(), params.toList(), List.<JCExpression>nil(),
 			maker.Block(0L, nullChecks.appendList(assigns).toList()), null), source);

src/core/lombok/javac/handlers/HandleEqualsAndHashCode.java

@@ -190,7 +190,7 @@ public class HandleEqualsAndHashCode extends JavacAnnotationHandler<EqualsAndHas
 		injectMethod(typeNode, equalsMethod);
 		
 		if (needsCanEqual && canEqualExists == MemberExistsResult.NOT_EXISTS) {
-			JCMethodDecl canEqualMethod = createCanEqual(typeNode, source, copyAnnotations(onParam));
+			JCMethodDecl canEqualMethod = createCanEqual(typeNode, source, copyAnnotations(onParam, typeNode.getTreeMaker()));
 			injectMethod(typeNode, canEqualMethod);
 		}
 		

src/core/lombok/javac/handlers/HandleGetter.java

@@ -250,7 +250,7 @@ public class HandleGetter extends JavacAnnotationHandler<Getter> {
 		
 		List<JCAnnotation> copyableAnnotations = findCopyableAnnotations(field);
 		List<JCAnnotation> delegates = findDelegatesAndRemoveFromField(field);
-		List<JCAnnotation> annsOnMethod = copyAnnotations(onMethod).appendList(copyableAnnotations);
+		List<JCAnnotation> annsOnMethod = copyAnnotations(onMethod, treeMaker).appendList(copyableAnnotations);
 		if (field.isFinal()) {
 			if (getCheckerFrameworkVersion(field).generatePure()) annsOnMethod = annsOnMethod.prepend(treeMaker.Annotation(genTypeRef(field, CheckerFrameworkVersion.NAME__PURE), List.<JCExpression>nil()));
 		} else {

src/core/lombok/javac/handlers/HandleJacksonized.java

@@ -136,7 +136,7 @@ public class HandleJacksonized extends JavacAnnotationHandler<Jacksonized> {
 		
 		// Copy annotations from the class to the builder class.
 		List<JCAnnotation> copyableAnnotations = findJacksonAnnotationsOnClass(tdNode);
-		List<JCAnnotation> copiedAnnotations = copyAnnotations(copyableAnnotations);
+		List<JCAnnotation> copiedAnnotations = copyAnnotations(copyableAnnotations, maker);
 		for (JCAnnotation anno : copiedAnnotations) {
 			recursiveSetGeneratedBy(anno, annotationNode);
 		}

src/core/lombok/javac/handlers/HandleSetter.java

@@ -241,7 +241,7 @@ public class HandleSetter extends JavacAnnotationHandler<Setter> {
 		List<JCAnnotation> copyableAnnotations = findCopyableAnnotations(field);
 		
 		Name methodName = field.toName(setterName);
-		List<JCAnnotation> annsOnParam = copyAnnotations(onParam).appendList(copyableAnnotations);
+		List<JCAnnotation> annsOnParam = copyAnnotations(onParam, treeMaker).appendList(copyableAnnotations);
 		
 		long flags = JavacHandlerUtil.addFinalIfNeeded(Flags.PARAMETER, field.getContext());
 		JCExpression pType = cloneType(treeMaker, fieldDecl.vartype, source);
@@ -274,7 +274,7 @@ public class HandleSetter extends JavacAnnotationHandler<Setter> {
 		List<JCExpression> throwsClauses = List.nil();
 		JCExpression annotationMethodDefaultValue = null;
 		
-		List<JCAnnotation> annsOnMethod = mergeAnnotations(copyAnnotations(onMethod), findCopyableToSetterAnnotations(field));
+		List<JCAnnotation> annsOnMethod = mergeAnnotations(copyAnnotations(onMethod, treeMaker), findCopyableToSetterAnnotations(field));
 		if (isFieldDeprecated(field) || deprecate) {
 			annsOnMethod = annsOnMethod.prepend(treeMaker.Annotation(genJavaLangTypeRef(field, "Deprecated"), List.<JCExpression>nil()));
 		}

src/core/lombok/javac/handlers/HandleWith.java

@@ -229,7 +229,7 @@ public class HandleWith extends JavacAnnotationHandler<With> {
 		
 		JCBlock methodBody = null;
 		long flags = JavacHandlerUtil.addFinalIfNeeded(Flags.PARAMETER, field.getContext());
-		List<JCAnnotation> annsOnParam = copyAnnotations(onParam).appendList(copyableAnnotations);
+		List<JCAnnotation> annsOnParam = copyAnnotations(onParam, maker).appendList(copyableAnnotations);
 		
 		JCExpression pType = cloneType(maker, fieldDecl.vartype, source);
 		JCVariableDecl param = maker.VarDef(maker.Modifiers(flags, annsOnParam), fieldDecl.name, pType, null);
@@ -278,7 +278,7 @@ public class HandleWith extends JavacAnnotationHandler<With> {
 		List<JCExpression> throwsClauses = List.nil();
 		JCExpression annotationMethodDefaultValue = null;
 		
-		List<JCAnnotation> annsOnMethod = copyAnnotations(onMethod);
+		List<JCAnnotation> annsOnMethod = copyAnnotations(onMethod, maker);
 		CheckerFrameworkVersion checkerFramework = getCheckerFrameworkVersion(source);
 		if (checkerFramework.generateSideEffectFree()) annsOnMethod = annsOnMethod.prepend(maker.Annotation(genTypeRef(source, CheckerFrameworkVersion.NAME__SIDE_EFFECT_FREE), List.<JCExpression>nil()));
 		

src/core/lombok/javac/handlers/HandleWithBy.java

@@ -321,7 +321,7 @@ public class HandleWithBy extends JavacAnnotationHandler<WithBy> {
 		List<JCExpression> throwsClauses = List.nil();
 		JCExpression annotationMethodDefaultValue = null;
 		
-		List<JCAnnotation> annsOnMethod = copyAnnotations(onMethod);
+		List<JCAnnotation> annsOnMethod = copyAnnotations(onMethod, maker);
 		CheckerFrameworkVersion checkerFramework = getCheckerFrameworkVersion(source);
 		if (checkerFramework.generateSideEffectFree()) annsOnMethod = annsOnMethod.prepend(maker.Annotation(genTypeRef(source, CheckerFrameworkVersion.NAME__SIDE_EFFECT_FREE), List.<JCExpression>nil()));
 		

src/core/lombok/javac/handlers/JavacHandlerUtil.java

@@ -38,6 +38,7 @@ import java.util.HashMap;
 import java.util.Map;
 import java.util.regex.Pattern;
 
+import com.sun.source.tree.TreeVisitor;
 import com.sun.tools.javac.code.Attribute;
 import com.sun.tools.javac.code.BoundKind;
 import com.sun.tools.javac.code.Flags;
@@ -75,6 +76,7 @@ import com.sun.tools.javac.tree.JCTree.JCTypeParameter;
 import com.sun.tools.javac.tree.JCTree.JCVariableDecl;
 import com.sun.tools.javac.tree.JCTree.JCWildcard;
 import com.sun.tools.javac.tree.JCTree.TypeBoundKind;
+import com.sun.tools.javac.tree.TreeCopier;
 import com.sun.tools.javac.tree.TreeMaker;
 import com.sun.tools.javac.tree.TreeScanner;
 import com.sun.tools.javac.util.Context;
@@ -1746,7 +1748,8 @@ public class JavacHandlerUtil {
 				}
 			}
 		}
-		return copyAnnotations(result.toList());
+
+		return copyAnnotations(result.toList(), node.getTreeMaker());
 	}
 	
 	/**
@@ -1784,7 +1787,7 @@ public class JavacHandlerUtil {
 		if (annoName == null) return List.nil();
 		
 		if (!annoName.isEmpty()) {
-			for (String bn : annotationsToFind) if (typeMatches(bn, node, annoName)) return List.of(anno);
+			for (String bn : annotationsToFind) if (typeMatches(bn, node, annoName)) return copyAnnotations(List.of(anno), node.getTreeMaker());
 		}
 		
 		ListBuffer<JCAnnotation> result = new ListBuffer<JCAnnotation>();
@@ -1799,7 +1802,7 @@ public class JavacHandlerUtil {
 				}
 			}
 		}
-		return copyAnnotations(result.toList());
+		return copyAnnotations(result.toList(), node.getTreeMaker());
 	}
 	
 	/**
@@ -2108,15 +2111,20 @@ public class JavacHandlerUtil {
 		errorNode.addError(out.append(" are not allowed on builder classes.").toString());
 	}
 	
-	static List<JCAnnotation> copyAnnotations(List<? extends JCExpression> in) {
+	static List<JCAnnotation> copyAnnotations(List<? extends JCExpression> in, JavacTreeMaker maker) {
 		ListBuffer<JCAnnotation> out = new ListBuffer<JCAnnotation>();
 		for (JCExpression expr : in) {
 			if (!(expr instanceof JCAnnotation)) continue;
-			out.append((JCAnnotation) expr.clone());
+			out.append(copyAnnotation((JCAnnotation) expr, maker));
 		}
 		return out.toList();
 	}
 	
+	static JCAnnotation copyAnnotation(JCAnnotation annotation, JavacTreeMaker maker) {
+		TreeVisitor<JCTree, Void> visitor = new TreeCopier<Void>(maker.getUnderlyingTreeMaker());
+		return (JCAnnotation) visitor.visitAnnotation(annotation, null);
+	}
+	
 	static List<JCAnnotation> mergeAnnotations(List<JCAnnotation> a, List<JCAnnotation> b) {
 		if (a == null || a.isEmpty()) return b;
 		if (b == null || b.isEmpty()) return a;
@@ -2266,7 +2274,7 @@ public class JavacHandlerUtil {
 		
 		if (JCAnnotatedTypeReflect.is(in)) {
 			JCExpression underlyingType = cloneType0(maker, JCAnnotatedTypeReflect.getUnderlyingType(in));
-			List<JCAnnotation> anns = copyAnnotations(JCAnnotatedTypeReflect.getAnnotations(in));
+			List<JCAnnotation> anns = copyAnnotations(JCAnnotatedTypeReflect.getAnnotations(in), maker);
 			return JCAnnotatedTypeReflect.create(anns, underlyingType);
 		}
 		

src/core/lombok/javac/handlers/JavacSingularsRecipes.java

@@ -315,7 +315,7 @@ public class JavacSingularsRecipes {
 			if (!setterPrefix.isEmpty()) name = builderType.toName(HandlerUtil.buildAccessorName(source, setterPrefix, name.toString()));
 			
 			statements.prepend(createConstructBuilderVarIfNeeded(maker, data, builderType, source));
-			List<JCAnnotation> methodAnnotations = copyAnnotations(findCopyableToBuilderSingularSetterAnnotations(data.annotation.up()));
+			List<JCAnnotation> methodAnnotations = copyAnnotations(findCopyableToBuilderSingularSetterAnnotations(data.annotation.up()), maker);
 			finishAndInjectMethod(cfv, maker, returnType, returnStatement, data, builderType, source, deprecate, statements, name, params, methodAnnotations, access, null);
 		}
 		
@@ -362,7 +362,7 @@ public class JavacSingularsRecipes {
 				statements.prepend(JavacHandlerUtil.generateNullCheck(maker, null, data.getPluralName(), builderType, "%s cannot be null"));
 			}
 
-			List<JCAnnotation> methodAnnotations = copyAnnotations(findCopyableToSetterAnnotations(data.annotation.up()));
+			List<JCAnnotation> methodAnnotations = copyAnnotations(findCopyableToSetterAnnotations(data.annotation.up()), maker);
 			
 			finishAndInjectMethod(cfv, maker, returnType, returnStatement, data, builderType, source, deprecate, statements, name, List.of(param), methodAnnotations, access, ignoreNullCollections);
 		}

src/delombok/lombok/delombok/Delombok.java

@@ -103,6 +103,7 @@ public class Delombok {
 	private LinkedHashMap<File, File> fileToBase = new LinkedHashMap<File, File>();
 	private List<File> filesToParse = new ArrayList<File>();
 	private Map<String, String> formatPrefs = new HashMap<String, String>();
+	private List<AbstractProcessor> preLombokProcessors = new ArrayList<AbstractProcessor>();
 	private List<AbstractProcessor> additionalAnnotationProcessors = new ArrayList<AbstractProcessor>();
 	
 	/** If null, output to standard out. */
@@ -653,6 +654,10 @@ public class Delombok {
 		fileToBase.put(f, base);
 	}
 	
+	public void addPreLombokProcessors(AbstractProcessor processor) {
+		preLombokProcessors.add(processor);
+	}
+
 	public void addAdditionalAnnotationProcessor(AbstractProcessor processor) {
 		additionalAnnotationProcessors.add(processor);
 	}
@@ -735,6 +740,7 @@ public class Delombok {
 		Map<JCCompilationUnit, File> baseMap = new IdentityHashMap<JCCompilationUnit, File>();
 		
 		Set<AbstractProcessor> processors = new LinkedHashSet<AbstractProcessor>();
+		processors.addAll(preLombokProcessors);
 		processors.add(new lombok.javac.apt.LombokProcessor());
 		processors.addAll(additionalAnnotationProcessors);
 		

test/core/src/lombok/RunTestsViaDelombok.java

@@ -33,6 +33,7 @@ import java.util.ArrayList;
 import java.util.Collection;
 import java.util.Collections;
 import java.util.Deque;
+import java.util.HashMap;
 import java.util.IdentityHashMap;
 import java.util.List;
 import java.util.Locale;
@@ -83,7 +84,11 @@ public class RunTestsViaDelombok extends AbstractRunTests {
 		
 		delombok.setDiagnosticsListener(new CapturingDiagnosticListener(file, result.getMessages()));
 		
-		if (parameters.isCheckPositions()) delombok.addAdditionalAnnotationProcessor(new ValidatePositionProcessor(parameters.getMinVersion()));
+		if (parameters.isCheckPositions()) {
+			NodePositionMapper nodePositionMapper = new NodePositionMapper();
+			delombok.addPreLombokProcessors(nodePositionMapper);
+			delombok.addAdditionalAnnotationProcessor(new ValidatePositionProcessor(parameters.getMinVersion(), nodePositionMapper));
+		}
 		delombok.addAdditionalAnnotationProcessor(new ValidateTypesProcessor());
 		delombok.addAdditionalAnnotationProcessor(new ValidateNoDuplicateTreeNodeProcessor());
 		
@@ -105,15 +110,35 @@ public class RunTestsViaDelombok extends AbstractRunTests {
 		}
 	}
 	
+	public static class NodePositionMapper extends TreeProcessor {
+		Map<JCTree, Integer> nodePositions = new HashMap<JCTree, Integer>();
+		
+		@Override void processCompilationUnit(final JCCompilationUnit unit) {
+			unit.accept(new TreeScanner() {
+				@Override public void scan(JCTree tree) {
+					if (tree == null) return;
+					if (tree.pos >= 0) {
+						nodePositions.put(tree, tree.pos);
+					}
+					super.scan(tree);
+					
+				}
+			});
+		}
+		
+	}
+	
 	public static class ValidatePositionProcessor extends TreeProcessor {
 		private final int version;
+		private final NodePositionMapper nodePositionMapper;
 		
-		public ValidatePositionProcessor(int version) {
+		public ValidatePositionProcessor(int version, NodePositionMapper nodePositionMapper) {
 			this.version = version;
+			this.nodePositionMapper = nodePositionMapper;
 		}
 		
 		private String craftFailMsg(String problematicNode, Deque<JCTree> astContext) {
-			StringBuilder msg = new StringBuilder(problematicNode).append(" position of node not set: ");
+			StringBuilder msg = new StringBuilder(problematicNode);
 			for (JCTree t : astContext) {
 				msg.append("\n  ").append(t.getClass().getSimpleName());
 				String asStr = t.toString();
@@ -125,12 +150,30 @@ public class RunTestsViaDelombok extends AbstractRunTests {
 			return msg.append("\n-------").toString();
 		}
 		
+		private boolean isLombokGenerated(JCTree tree) {
+			List<JCAnnotation> annotations = com.sun.tools.javac.util.List.nil();
+			if (tree instanceof JCMethodDecl) {
+				annotations = ((JCMethodDecl) tree).mods.annotations;
+			}
+			if (tree instanceof JCVariableDecl) {
+				annotations = ((JCVariableDecl) tree).mods.annotations;
+			}
+			for (JCAnnotation annotation: annotations) {
+				if ("lombok.Generated".equals(annotation.getAnnotationType().toString())) {
+					return true;
+				}
+			}
+			return false;
+		}
+		
 		@Override void processCompilationUnit(final JCCompilationUnit unit) {
 			final Deque<JCTree> astContext = new ArrayDeque<JCTree>();
+			final Deque<JCTree> lombokGeneratedNodes = new ArrayDeque<JCTree>();
 			unit.accept(new TreeScanner() {
 				@Override public void scan(JCTree tree) {
 					if (tree == null) return;
 					if (tree instanceof JCMethodDecl && (((JCMethodDecl) tree).mods.flags & Flags.GENERATEDCONSTR) != 0) return;
+					if (isLombokGenerated(tree)) lombokGeneratedNodes.add(tree);
 					astContext.push(tree);
 					try {
 						if (tree instanceof JCModifiers) return;
@@ -153,16 +196,26 @@ public class RunTestsViaDelombok extends AbstractRunTests {
 						
 						if (tree instanceof JCVariableDecl && (((JCVariableDecl) tree).mods.flags & Javac.GENERATED_MEMBER) != 0) return;
 						
-						if (check && tree.pos == -1) fail(craftFailMsg("Start", astContext));
+						if (check && tree.pos == -1) fail(craftFailMsg("Start position of node not set: ", astContext));
 						
+						// Ignore ast position validation on lombok generated nodes.
+						if (lombokGeneratedNodes.isEmpty()) {
+							Integer expectedPos = nodePositionMapper.nodePositions.get(tree);
+							if (expectedPos != null && !expectedPos.equals(tree.pos)) {
+								fail(craftFailMsg(String.format("Expected node position %d, actual node position %d: ", expectedPos, tree.pos), astContext));
+							}
+						}
 						if (check && Javac.getEndPosition(tree, unit) == -1) {
-							fail(craftFailMsg("End", astContext));
+							fail(craftFailMsg("End position of node not set: ", astContext));
 						}
 					} finally {
 						try {
 							super.scan(tree);
 						} finally {
-							astContext.pop();
+							JCTree _tree = astContext.pop();
+							if (!lombokGeneratedNodes.isEmpty() && lombokGeneratedNodes.peek().equals(_tree)) {
+								lombokGeneratedNodes.pop();
+							}
 						}
 					}
 				}
@@ -289,14 +342,6 @@ public class RunTestsViaDelombok extends AbstractRunTests {
 					super.scan(tree);
 					parents.pop();
 				}
-				
-				/**
-				 * We always generate shallow copies for annotations
-				 */
-				@Override
-				public void visitAnnotation(JCAnnotation tree) {
-					return;
-				}
 			});
 		}