Discrepancy in annotation processing, between full and incremental builds eclipse-jdt#4617 (eclipse-jdt#4639)

Store the unsorted list in BinaryTypeBinding when annotation processing is enabled and use that list in annotation models.

Author: jarthana

org.eclipse.jdt.compiler.apt.tests/lib/apttestprocessors8.jar

@@ -58,7 +58,7 @@
 	                       "org.eclipse.jdt.compiler.apt.tests.annotations.IFoo", "org.eclipse.jdt.compiler.apt.tests.annotations.IFooContainer",
 	                       "org.eclipse.jdt.compiler.apt.tests.annotations.Goo", "org.eclipse.jdt.compiler.apt.tests.annotations.GooNonContainer",
 	                       "org.eclipse.jdt.compiler.apt.tests.annotations.FooNonContainer", "targets.filer8.PackageAnnot",
-	                       "java.lang.Deprecated"})
+	                       "java.lang.Deprecated", "org.eclipse.jdt.compiler.apt.tests.annotations.GenClass"})
 
 @SupportedSourceVersion(SourceVersion.RELEASE_8)
 public class Java8ElementProcessor extends BaseProcessor {
@@ -161,6 +161,7 @@ public void testAll() throws AssertionFailedError {
 		testBug526288();
 		testEnumConstArguments();
 		testBug544288();
+		testGH4617();
 	}
 
 	public void testLambdaSpecifics() {
@@ -1070,6 +1071,17 @@ public void testBug544288() {
 		Element asElement = _typeUtils.asElement(asType);
 		verifyAnnotations(asElement, new String[]{"@Deprecated()"});
 	}
+	public void testGH4617() {
+		TypeElement type = _elementUtils.getTypeElement("targets.GH4617.ParentClass");
+		List<? extends Element> enclosedElements = type.getEnclosedElements();
+		List<ExecutableElement> methodsIn = ElementFilter.methodsIn(enclosedElements);
+		assertEquals("Incorrect number of methods", 3, methodsIn.size());
+		StringBuilder builder = new StringBuilder();
+		for (ExecutableElement executableElement : methodsIn) {
+			builder.append(executableElement.getSimpleName().toString()).append(',');
+		}
+		assertEquals("Incorrect order received", "gamma,alpha,beta,", builder.toString());
+	}
 	private void createPackageBinary() throws IOException {
 		String path = packageName.replace('.', '/');
 		ClassLoader loader = getClass().getClassLoader();

org.eclipse.jdt.compiler.apt.tests/processors8/org/eclipse/jdt/compiler/apt/tests/processors/elements/Java8ElementProcessor.java

@@ -0,0 +1,7 @@
+package targets.GH4617;
+@org.eclipse.jdt.compiler.apt.tests.annotations.GenClass(clazz="p.q.R", method="foo")
+public abstract class ParentClass {
+  public abstract int gamma();
+  public abstract byte alpha();
+  public abstract int beta();
+}

org.eclipse.jdt.compiler.apt.tests/resources/targets/GH4617/ParentClass.java

@@ -413,6 +413,12 @@ public void testBug544288() throws Exception {
 		JavaCompiler compiler = BatchTestUtils.getEclipseCompiler();
 		internalTestWithBinary(compiler, JAVA8_ANNOTATION_PROC, "testBug544288", null, "bug544288", "9");
 	}
+	public void testGH4617() throws Exception {
+		if (!canRunJava9())
+			return;
+		JavaCompiler compiler = BatchTestUtils.getEclipseCompiler();
+		internalTestWithBinary(compiler, JAVA8_ANNOTATION_PROC, "testGH4617", null, "GH4617", "9");
+	}
 	private void internalTest(JavaCompiler compiler, String processor, String testMethod) throws IOException {
 		internalTest(compiler, processor, testMethod, null);
 	}

org.eclipse.jdt.compiler.apt.tests/src/org/eclipse/jdt/compiler/apt/tests/Java8ElementsTests.java

@@ -129,8 +129,9 @@ protected AnnotationBinding[] getAnnotationBindings()
 	@Override
 	public List<? extends Element> getEnclosedElements() {
 		ReferenceBinding binding = (ReferenceBinding)this._binding;
-		List<Element> enclosed = new ArrayList<>(binding.fieldCount() + binding.methods().length + binding.memberTypes().length);
-		for (MethodBinding method : binding.methods()) {
+		MethodBinding[] methods = (binding instanceof BinaryTypeBinding btb) ? btb.methodsInOriginalOrder() : binding.methods();
+		List<Element> enclosed = new ArrayList<>(binding.fieldCount() + methods.length + binding.memberTypes().length);
+		for (MethodBinding method : methods) {
 			ExecutableElement executable = new ExecutableElementImpl(this._env, method);
 			enclosed.add(executable);
 		}
@@ -152,7 +153,9 @@ public List<? extends Element> getEnclosedElements() {
 			TypeElement type = new TypeElementImpl(this._env, memberType, null);
 			enclosed.add(type);
 		}
-		Collections.sort(enclosed, new SourceLocationComparator());
+		// SourceLocationComparator is of no use for binaries; so skip it.
+		if (!binding.isBinaryBinding())
+			Collections.sort(enclosed, new SourceLocationComparator());
 		return Collections.unmodifiableList(enclosed);
 	}
 

org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/apt/model/TypeElementImpl.java

@@ -51,6 +51,7 @@
 
 import java.net.URI;
 import java.util.ArrayList;
+import java.util.Arrays;
 import java.util.HashMap;
 import java.util.Map;
 import org.eclipse.jdt.core.compiler.CharOperation;
@@ -96,6 +97,7 @@ public class BinaryTypeBinding extends ReferenceBinding {
 	protected FieldBinding[] fields;
 	protected RecordComponentBinding[] components;
 	protected MethodBinding[] methods;
+	protected MethodBinding[] methodsInOriginalOrder;
 	protected ReferenceBinding[] memberTypes;
 	protected TypeVariableBinding[] typeVariables;
 	protected ModuleBinding module;
@@ -1760,6 +1762,19 @@ private ReferenceBinding[] maybeSortedMemberTypes() {
 	return this.memberTypes;
 }
 
+/**
+ * Returns the methods in the order they appear in the class file if available. In some case,
+ * for e.g., when annotation processing is enabled, the original order is preserved and available
+ * for clients. If the original order is not available, the regular sorted array is returned.
+ *
+ * @return the methods in the original order
+ */
+public MethodBinding[] methodsInOriginalOrder() {
+	if (this.methodsInOriginalOrder != null) {
+		return this.methodsInOriginalOrder;
+	}
+	return this.methods;
+}
 // NOTE: the return type, arg & exception types of each method of a binary type are resolved when needed
 @Override
 public MethodBinding[] methods() {
@@ -1774,8 +1789,12 @@ public MethodBinding[] methods() {
 	// lazily sort methods
 	if ((this.tagBits & TagBits.AreMethodsSorted) == 0) {
 		int length = this.methods.length;
-		if (length > 1)
+		if (length > 1) {
+			if (this.environment.globalOptions.processAnnotations) {
+				this.methodsInOriginalOrder = Arrays.copyOf(this.methods, this.methods.length);
+			}
 			ReferenceBinding.sortMethods(this.methods, 0, length);
+		}
 		this.tagBits |= TagBits.AreMethodsSorted;
 	}
 	for (int i = this.methods.length; --i >= 0;)