Fixes #1543: in equals, by default first compare the primitives

doc/changelog.markdown

@@ -2,6 +2,8 @@ Lombok Changelog
 ----------------
 
 ### v1.18.13 "Edgy Guinea Pig"
+* PERFORMANCE: The generated equals method will first compare primitives. Manual re-ordering is possible using `@Include(rank=n)`. [Pull Request #2485](https://github.com/rzwitserloot/lombok/pull/2485), [Issue #1543](https://github.com/rzwitserloot/lombok/issues/1543)
+* IMPROBABLE BREAKING CHANGE: The generated hashcode has changed for classes that include both primitive fields and reference fields.
 * PLATFORM: Added support for compiling projects with OpenJ9 [Pull Request #2437](https://github.com/rzwitserloot/lombok/pull/2437)
 * BREAKING CHANGE: mapstruct users should now add a dependency to lombok-mapstruct-binding. This solves compiling modules with lombok (and mapstruct).
 * FEATURE: Similar to `@Builder`, you can now configure a `@SuperBuilder`'s 'setter' prefixes via `@SuperBuilder(setterPrefix = "set")` for example. We still discourage doing this. [Pull Request #2357](https://github.com/rzwitserloot/lombok/pull/2357).
@@ -14,6 +16,7 @@ Lombok Changelog
 * BUGFIX: Javac sets incorrect annotated type on with methods. [Issue #2463](https://github.com/rzwitserloot/lombok/issues/2463)
 * FEATURE: `@Jacksonized` on a `@Builder` or `@SuperBuilder` will configure [Jackson](https://github.com/FasterXML/jackson) to use this builder when deserializing. [Pull Request #2387](https://github.com/rzwitserloot/lombok/pull/2387) thanks to __@JanRieke__. [@Jacksonized documentation](https://projectlombok.org/features/experimental/Jacksonized).
 
+
 ### v1.18.12 (February 1st, 2020)
 * PLATFORM: Support for JDK13 (including `yield` in switch expressions, as well as delombok having a nicer style for arrow-style switch blocks, and text blocks).
 * PLATFORM: Support for JDK14 (including `pattern match` instanceof expressions).

src/core/lombok/EqualsAndHashCode.java

@@ -125,7 +125,9 @@ public @interface EqualsAndHashCode {
 
 		/**
 		 * Higher ranks are considered first. Members of the same rank are considered in the order they appear in the source file.
-		 *
+		 * 
+		 * If not explicitly set, the {@code default} rank for primitives is 1000.
+		 * 
 		 * @return ordering within the generating {@code equals} and {@code hashCode} methods; higher numbers are considered first.
 		 */
 		int rank() default 0;

src/core/lombok/core/LombokNode.java

@@ -288,6 +288,7 @@ public abstract class LombokNode<A extends AST<A, L, N>, L extends LombokNode<A,
 	public abstract boolean isStatic();
 	public abstract boolean isFinal();
 	public abstract boolean isTransient();
+	public abstract boolean isPrimitive();
 	public abstract boolean isEnumMember();
 	public abstract boolean isEnumType();
 	

src/core/lombok/core/handlers/InclusionExclusionUtils.java

@@ -76,11 +76,13 @@ public class InclusionExclusionUtils {
 		private final L node;
 		private final I inc;
 		private final boolean defaultInclude;
+		private final boolean explicitRank;
 		
-		public Included(L node, I inc, boolean defaultInclude) {
+		public Included(L node, I inc, boolean defaultInclude, boolean explicitRank) {
 			this.node = node;
 			this.inc = inc;
 			this.defaultInclude = defaultInclude;
+			this.explicitRank = explicitRank;
 		}
 		
 		public L getNode() {
@@ -94,6 +96,10 @@ public class InclusionExclusionUtils {
 		public boolean isDefaultInclude() {
 			return defaultInclude;
 		}
+		
+		public boolean hasExplicitRank() {
+			return explicitRank;
+		}
 	}
 	
 	private static String innerAnnName(Class<? extends Annotation> type) {
@@ -164,13 +170,13 @@ public class InclusionExclusionUtils {
 					if (n.isEmpty()) n = name;
 					namesToAutoExclude.add(n);
 				}
-				members.add(new Included<L, I>(child, inc, false));
+				members.add(new Included<L, I>(child, inc, false, markInclude.isExplicit("rank")));
 				continue;
 			}
 			
 			if (onlyExplicitlyIncluded) continue;
 			if (oldIncludes != null) {
-				if (child.getKind() == Kind.FIELD && oldIncludes.contains(name)) members.add(new Included<L, I>(child, null, false));
+				if (child.getKind() == Kind.FIELD && oldIncludes.contains(name)) members.add(new Included<L, I>(child, null, false, false));
 				continue;
 			}
 			if (child.getKind() != Kind.FIELD) continue;
@@ -178,7 +184,7 @@ public class InclusionExclusionUtils {
 			if (child.isTransient() && !includeTransient) continue;
 			if (name.startsWith("$")) continue;
 			if (child.isEnumMember()) continue;
-			members.add(new Included<L, I>(child, null, true));
+			members.add(new Included<L, I>(child, null, true, false));
 		}
 		
 		/* delete default-included fields with the same name as an explicit inclusion */ {
@@ -219,8 +225,8 @@ public class InclusionExclusionUtils {
 
 		Collections.sort(members, new Comparator<Included<L, EqualsAndHashCode.Include>>() {
 			@Override public int compare(Included<L, EqualsAndHashCode.Include> a, Included<L, EqualsAndHashCode.Include> b) {
-				int ra = a.getInc() == null ? 0 : a.getInc().rank();
-				int rb = b.getInc() == null ? 0 : b.getInc().rank();
+				int ra = a.hasExplicitRank() ? a.getInc().rank() : a.node.isPrimitive() ? 1000 : 0;
+				int rb = b.hasExplicitRank() ? b.getInc().rank() : b.node.isPrimitive() ? 1000 : 0;
 
 				return compareRankOrPosition(ra, rb, a.getNode(), b.getNode());
 			}

src/core/lombok/eclipse/EclipseNode.java

@@ -23,10 +23,6 @@ package lombok.eclipse;
 
 import java.util.List;
 
-import lombok.core.AnnotationValues;
-import lombok.core.AST.Kind;
-import lombok.eclipse.handlers.EclipseHandlerUtil;
-
 import org.eclipse.jdt.internal.compiler.ast.ASTNode;
 import org.eclipse.jdt.internal.compiler.ast.AbstractMethodDeclaration;
 import org.eclipse.jdt.internal.compiler.ast.Annotation;
@@ -36,11 +32,16 @@ import org.eclipse.jdt.internal.compiler.ast.CompilationUnitDeclaration;
 import org.eclipse.jdt.internal.compiler.ast.FieldDeclaration;
 import org.eclipse.jdt.internal.compiler.ast.Initializer;
 import org.eclipse.jdt.internal.compiler.ast.LocalDeclaration;
+import org.eclipse.jdt.internal.compiler.ast.MethodDeclaration;
 import org.eclipse.jdt.internal.compiler.ast.Statement;
 import org.eclipse.jdt.internal.compiler.ast.TypeDeclaration;
 import org.eclipse.jdt.internal.compiler.ast.TypeReference;
 import org.eclipse.jdt.internal.compiler.classfmt.ClassFileConstants;
 
+import lombok.core.AST.Kind;
+import lombok.core.AnnotationValues;
+import lombok.eclipse.handlers.EclipseHandlerUtil;
+
 /**
  * Eclipse specific version of the LombokNode class.
  */
@@ -264,6 +265,16 @@ public class EclipseNode extends lombok.core.LombokNode<EclipseAST, EclipseNode,
 		return (ClassFileConstants.AccFinal & f) != 0;
 	}
 	
+	@Override public boolean isPrimitive() {
+		if (node instanceof FieldDeclaration && !isEnumMember()) {
+			return Eclipse.isPrimitive(((FieldDeclaration) node).type);
+		}
+		if (node instanceof MethodDeclaration) {
+			return Eclipse.isPrimitive(((MethodDeclaration) node).returnType);
+		}
+		return false;
+	}
+	
 	@Override public boolean isTransient() {
 		if (getKind() != Kind.FIELD) return false;
 		Integer i = getModifiers();

src/core/lombok/eclipse/handlers/HandleBuilder.java

@@ -485,7 +485,7 @@ public class HandleBuilder extends EclipseAnnotationHandler<Builder> {
 			List<Included<EclipseNode, ToString.Include>> fieldNodes = new ArrayList<Included<EclipseNode, ToString.Include>>();
 			for (BuilderFieldData bfd : builderFields) {
 				for (EclipseNode f : bfd.createdFields) {
-					fieldNodes.add(new Included<EclipseNode, ToString.Include>(f, null, true));
+					fieldNodes.add(new Included<EclipseNode, ToString.Include>(f, null, true, false));
 				}
 			}
 			MethodDeclaration md = HandleToString.createToString(builderType, fieldNodes, true, false, ast, FieldAccess.ALWAYS_FIELD);

src/core/lombok/eclipse/handlers/HandleSuperBuilder.java

@@ -354,7 +354,7 @@ public class HandleSuperBuilder extends EclipseAnnotationHandler<SuperBuilder> {
 			List<Included<EclipseNode, ToString.Include>> fieldNodes = new ArrayList<Included<EclipseNode, ToString.Include>>();
 			for (BuilderFieldData bfd : builderFields) {
 				for (EclipseNode f : bfd.createdFields) {
-					fieldNodes.add(new Included<EclipseNode, ToString.Include>(f, null, true));
+					fieldNodes.add(new Included<EclipseNode, ToString.Include>(f, null, true, false));
 				}
 			}
 			// Let toString() call super.toString() if there is a superclass, so that it also shows fields from the superclass' builder.

src/core/lombok/javac/JavacNode.java

@@ -27,10 +27,6 @@ import java.util.List;
 import javax.lang.model.element.Element;
 import javax.tools.Diagnostic;
 
-import lombok.core.AnnotationValues;
-import lombok.core.AST.Kind;
-import lombok.javac.handlers.JavacHandlerUtil;
-
 import com.sun.tools.javac.code.Flags;
 import com.sun.tools.javac.code.Symtab;
 import com.sun.tools.javac.model.JavacTypes;
@@ -43,8 +39,12 @@ import com.sun.tools.javac.tree.JCTree.JCMethodDecl;
 import com.sun.tools.javac.tree.JCTree.JCModifiers;
 import com.sun.tools.javac.tree.JCTree.JCVariableDecl;
 import com.sun.tools.javac.util.Context;
-import com.sun.tools.javac.util.Name;
 import com.sun.tools.javac.util.JCDiagnostic.DiagnosticPosition;
+import com.sun.tools.javac.util.Name;
+
+import lombok.core.AST.Kind;
+import lombok.core.AnnotationValues;
+import lombok.javac.handlers.JavacHandlerUtil;
 
 /**
  * Javac specific version of the LombokNode class.
@@ -345,6 +345,16 @@ public class JavacNode extends lombok.core.LombokNode<JavacAST, JavacNode, JCTre
 		return mods != null && (Flags.ENUM & mods.flags) != 0;
 	}
 	
+	@Override public boolean isPrimitive() {
+		if (node instanceof JCVariableDecl && !isEnumMember()) {
+			return Javac.isPrimitive(((JCVariableDecl) node).vartype);
+		}
+		if (node instanceof JCMethodDecl) {
+			return Javac.isPrimitive(((JCMethodDecl) node).restype);
+		}
+		return false;
+	}
+	
 	@Override public boolean isTransient() {
 		if (getKind() != Kind.FIELD) return false;
 		JCModifiers mods = getModifiers();

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

@@ -435,7 +435,7 @@ public class HandleBuilder extends JavacAnnotationHandler<Builder> {
 			java.util.List<Included<JavacNode, ToString.Include>> fieldNodes = new ArrayList<Included<JavacNode, ToString.Include>>();
 			for (BuilderFieldData bfd : builderFields) {
 				for (JavacNode f : bfd.createdFields) {
-					fieldNodes.add(new Included<JavacNode, ToString.Include>(f, null, true));
+					fieldNodes.add(new Included<JavacNode, ToString.Include>(f, null, true, false));
 				}
 			}
 			

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

@@ -304,7 +304,7 @@ public class HandleSuperBuilder extends JavacAnnotationHandler<SuperBuilder> {
 		java.util.List<Included<JavacNode, ToString.Include>> fieldNodes = new ArrayList<Included<JavacNode, ToString.Include>>();
 		for (BuilderFieldData bfd : builderFields) {
 			for (JavacNode f : bfd.createdFields) {
-				fieldNodes.add(new Included<JavacNode, ToString.Include>(f, null, true));
+				fieldNodes.add(new Included<JavacNode, ToString.Include>(f, null, true, false));
 			}
 		}
 		

test/transform/resource/after-delombok/BuilderDefaults.java

@@ -85,10 +85,10 @@ public final class BuilderDefaults {
 		if (!(o instanceof BuilderDefaults)) return false;
 		final BuilderDefaults other = (BuilderDefaults) o;
 		if (this.getX() != other.getX()) return false;
+		if (this.getZ() != other.getZ()) return false;
 		final java.lang.Object this$name = this.getName();
 		final java.lang.Object other$name = other.getName();
 		if (this$name == null ? other$name != null : !this$name.equals(other$name)) return false;
-		if (this.getZ() != other.getZ()) return false;
 		return true;
 	}
 	@java.lang.Override
@@ -97,10 +97,10 @@ public final class BuilderDefaults {
 		final int PRIME = 59;
 		int result = 1;
 		result = result * PRIME + this.getX();
-		final java.lang.Object $name = this.getName();
-		result = result * PRIME + ($name == null ? 43 : $name.hashCode());
 		final long $z = this.getZ();
 		result = result * PRIME + (int) ($z >>> 32 ^ $z);
+		final java.lang.Object $name = this.getName();
+		result = result * PRIME + ($name == null ? 43 : $name.hashCode());
 		return result;
 	}
 	@java.lang.Override
@@ -108,4 +108,4 @@ public final class BuilderDefaults {
 	public java.lang.String toString() {
 		return "BuilderDefaults(x=" + this.getX() + ", name=" + this.getName() + ", z=" + this.getZ() + ")";
 	}
-}
+}

test/transform/resource/after-delombok/EqualsAndHashCodeRank.java

@@ -9,9 +9,9 @@ public class EqualsAndHashCodeRank {
 		if (!(o instanceof EqualsAndHashCodeRank)) return false;
 		final EqualsAndHashCodeRank other = (EqualsAndHashCodeRank) o;
 		if (!other.canEqual((java.lang.Object) this)) return false;
-		if (this.b != other.b) return false;
 		if (this.a != other.a) return false;
 		if (this.c != other.c) return false;
+		if (this.b != other.b) return false;
 		return true;
 	}
 	@java.lang.SuppressWarnings("all")
@@ -23,9 +23,9 @@ public class EqualsAndHashCodeRank {
 	public int hashCode() {
 		final int PRIME = 59;
 		int result = 1;
-		result = result * PRIME + this.b;
 		result = result * PRIME + this.a;
 		result = result * PRIME + this.c;
+		result = result * PRIME + this.b;
 		return result;
 	}
-}
+}

test/transform/resource/after-ecj/BuilderDefaults.java

@@ -72,25 +72,25 @@ public final @Value @Builder class BuilderDefaults {
     final BuilderDefaults other = (BuilderDefaults) o;
     if ((this.getX() != other.getX()))
         return false;
+    if ((this.getZ() != other.getZ()))
+        return false;
     final java.lang.Object this$name = this.getName();
     final java.lang.Object other$name = other.getName();
     if (((this$name == null) ? (other$name != null) : (! this$name.equals(other$name))))
         return false;
-    if ((this.getZ() != other.getZ()))
-        return false;
     return true;
   }
   public @java.lang.Override @java.lang.SuppressWarnings("all") int hashCode() {
     final int PRIME = 59;
     int result = 1;
     result = ((result * PRIME) + this.getX());
-    final java.lang.Object $name = this.getName();
-    result = ((result * PRIME) + (($name == null) ? 43 : $name.hashCode()));
     final long $z = this.getZ();
     result = ((result * PRIME) + (int) ($z ^ ($z >>> 32)));
+    final java.lang.Object $name = this.getName();
+    result = ((result * PRIME) + (($name == null) ? 43 : $name.hashCode()));
     return result;
   }
   public @java.lang.Override @java.lang.SuppressWarnings("all") java.lang.String toString() {
     return (((((("BuilderDefaults(x=" + this.getX()) + ", name=") + this.getName()) + ", z=") + this.getZ()) + ")");
   }
-}
+}

test/transform/resource/after-ecj/EqualsAndHashCodeRank.java

@@ -7,18 +7,18 @@ public @EqualsAndHashCode class EqualsAndHashCodeRank {
     super();
   }
   public @java.lang.Override @java.lang.SuppressWarnings("all") boolean equals(final java.lang.Object o) {
-    if (o == this)
+    if ((o == this))
         return true;
-    if (!(o instanceof EqualsAndHashCodeRank))
+    if ((! (o instanceof EqualsAndHashCodeRank)))
         return false;
     final EqualsAndHashCodeRank other = (EqualsAndHashCodeRank) o;
-    if (!other.canEqual((java.lang.Object) this))
+    if ((! other.canEqual((java.lang.Object) this)))
         return false;
-    if (this.b != other.b)
+    if ((this.a != other.a))
         return false;
-    if (this.a != other.a)
+    if ((this.c != other.c))
         return false;
-    if (this.c != other.c)
+    if ((this.b != other.b))
         return false;
     return true;
   }
@@ -28,9 +28,9 @@ public @EqualsAndHashCode class EqualsAndHashCodeRank {
   public @java.lang.Override @java.lang.SuppressWarnings("all") int hashCode() {
     final int PRIME = 59;
     int result = 1;
-    result = result * PRIME + this.b;
-    result = result * PRIME + this.a;
-    result = result * PRIME + this.c;
+    result = ((result * PRIME) + this.a);
+    result = ((result * PRIME) + this.c);
+    result = ((result * PRIME) + this.b);
     return result;
   }
 }