#1543: First primitives, then primitive wrappers, then other references

doc/changelog.markdown

@@ -2,7 +2,7 @@ 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)
+* PERFORMANCE: The generated equals method will first compare primitives, then primitive wrappers and then reference fields. 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).

src/core/lombok/EqualsAndHashCode.java

@@ -126,7 +126,7 @@ 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.
+		 * If not explicitly set, the {@code default} rank for primitives is 1000, and for primitive wrappers 800.
 		 * 
 		 * @return ordering within the generating {@code equals} and {@code hashCode} methods; higher numbers are considered first.
 		 */

src/core/lombok/core/LombokNode.java

@@ -292,6 +292,19 @@ public abstract class LombokNode<A extends AST<A, L, N>, L extends LombokNode<A,
 	public abstract boolean isEnumMember();
 	public abstract boolean isEnumType();
 	
+	/**
+	 * The 'type' of the field or method, or {@code null} if this node is neither.
+	 * 
+	 * The type is as it is written in the code (no resolution), includes array dimensions, 
+	 * but not necessarily generics.
+	 * 
+	 * The main purpose of this method is to verify this type against a list of known types,
+	 * like primitives or primitive wrappers.
+	 * 
+	 * @return The 'type' of the field or method, or {@code null} if this node is neither.
+	 */
+	public abstract String fieldOrMethodBaseType();
+	
 	public abstract int countMethodParameters();
 	
 	public abstract int getStartPos();

src/core/lombok/core/handlers/HandlerUtil.java

@@ -27,6 +27,7 @@ import java.util.Collections;
 import java.util.HashSet;
 import java.util.List;
 import java.util.Set;
+import java.util.regex.Pattern;
 
 import lombok.AllArgsConstructor;
 import lombok.ConfigurationKeys;
@@ -365,6 +366,7 @@ public class HandlerUtil {
 	public static String autoSingularize(String plural) {
 		return Singulars.autoSingularize(plural);
 	}
+	
 	public static void handleFlagUsage(LombokNode<?, ?, ?> node, ConfigurationKey<FlagUsageType> key, String featureName) {
 		FlagUsageType fut = node.getAst().readConfiguration(key);
 		
@@ -742,4 +744,23 @@ public class HandlerUtil {
 		}
 		return b.toString();
 	}
+	
+	/** Matches any of the 8 primitive names, such as {@code boolean}. */
+	private static final Pattern PRIMITIVE_TYPE_NAME_PATTERN = Pattern.compile("^(?:boolean|byte|short|int|long|float|double|char)$");
+
+	public static boolean isPrimitive(String typeName) {
+		return PRIMITIVE_TYPE_NAME_PATTERN.matcher(typeName).matches();
+	}
+
+	/** Matches any of the 8 primitive wrapper names, such as {@code Boolean}. */
+	private static final Pattern PRIMITIVE_WRAPPER_TYPE_NAME_PATTERN = Pattern.compile("^(?:java\\.lang\\.)?(?:Boolean|Byte|Short|Integer|Long|Float|Double|Character)$");
+
+	public static int defaultEqualsAndHashcodeIncludeRank(String typeName) {
+		// Modification in this code should be documented
+		// 1. In the changelog this should be marked as an INPROBABLE BREAKING CHANGE, since the hashcode will change
+		// 2. In the javadoc of EqualsAndHashcode.Include#rank
+		if (isPrimitive(typeName)) return 1000;
+		if (PRIMITIVE_WRAPPER_TYPE_NAME_PATTERN.matcher(typeName).matches()) return 800;
+		return 0;
+	}
 }

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

@@ -225,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.hasExplicitRank() ? a.getInc().rank() : a.node.isPrimitive() ? 1000 : 0;
-				int rb = b.hasExplicitRank() ? b.getInc().rank() : b.node.isPrimitive() ? 1000 : 0;
+				int ra = a.hasExplicitRank() ? a.getInc().rank() : HandlerUtil.defaultEqualsAndHashcodeIncludeRank(a.node.fieldOrMethodBaseType());
+				int rb = b.hasExplicitRank() ? b.getInc().rank() : HandlerUtil.defaultEqualsAndHashcodeIncludeRank(b.node.fieldOrMethodBaseType());
 
 				return compareRankOrPosition(ra, rb, a.getNode(), b.getNode());
 			}

src/core/lombok/eclipse/EclipseNode.java

@@ -275,6 +275,29 @@ public class EclipseNode extends lombok.core.LombokNode<EclipseAST, EclipseNode,
 		return false;
 	}
 	
+	/**
+	 * {@inheritDoc}
+	 */
+	@Override public String fieldOrMethodBaseType() {
+		TypeReference typeReference = null;
+		if (node instanceof FieldDeclaration && !isEnumMember()) {
+			typeReference = ((FieldDeclaration) node).type;
+		}
+		if (node instanceof MethodDeclaration) {
+			typeReference = ((MethodDeclaration) node).returnType;
+		}
+		if (typeReference == null) return null;
+		
+		String fqn = Eclipse.toQualifiedName(typeReference.getTypeName());
+		if (typeReference.dimensions() == 0) return fqn;
+		StringBuilder result = new StringBuilder(fqn.length() + 2 * typeReference.dimensions());
+		result.append(fqn);
+		for (int i = 0; i < typeReference.dimensions(); i++) {
+			result.append("[]");
+		}
+		return result.toString();
+	}
+	
 	@Override public boolean isTransient() {
 		if (getKind() != Kind.FIELD) return false;
 		Integer i = getModifiers();

src/core/lombok/javac/JavacNode.java

@@ -355,6 +355,19 @@ public class JavacNode extends lombok.core.LombokNode<JavacAST, JavacNode, JCTre
 		return false;
 	}
 	
+	/**
+	 * {@inheritDoc}
+	 */
+	@Override public String fieldOrMethodBaseType() {
+		if (node instanceof JCVariableDecl && !isEnumMember()) {
+			return (((JCVariableDecl) node).vartype).toString();
+		}
+		if (node instanceof JCMethodDecl) {
+			return (((JCMethodDecl) node).restype).toString();
+		}
+		return null;
+	}
+	
 	@Override public boolean isTransient() {
 		if (getKind() != Kind.FIELD) return false;
 		JCModifiers mods = getModifiers();

src/utils/lombok/eclipse/Eclipse.java

@@ -29,6 +29,7 @@ import java.util.regex.Pattern;
 
 import lombok.core.ClassLiteral;
 import lombok.core.FieldSelect;
+import lombok.core.handlers.HandlerUtil;
 import lombok.permit.Permit;
 
 import org.eclipse.jdt.internal.compiler.ast.ASTNode;
@@ -158,16 +159,12 @@ public class Eclipse {
 		return result.toArray(EMPTY_ANNOTATIONS_ARRAY);
 	}
 	
-	/** Matches any of the 8 primitive names, such as {@code boolean}. */
-	private static final Pattern PRIMITIVE_TYPE_NAME_PATTERN = Pattern.compile(
-			"^(boolean|byte|short|int|long|float|double|char)$");
-	
 	/**
 	 * Checks if the given type reference represents a primitive type.
 	 */
 	public static boolean isPrimitive(TypeReference ref) {
 		if (ref.dimensions() > 0) return false;
-		return PRIMITIVE_TYPE_NAME_PATTERN.matcher(toQualifiedName(ref.getTypeName())).matches();
+		return HandlerUtil.isPrimitive(toQualifiedName(ref.getTypeName()));
 	}
 	
 	/**

src/utils/lombok/javac/Javac.java

@@ -37,12 +37,6 @@ import javax.lang.model.type.NoType;
 import javax.lang.model.type.TypeKind;
 import javax.lang.model.type.TypeVisitor;
 
-import lombok.core.ClassLiteral;
-import lombok.core.FieldSelect;
-import lombok.javac.JavacTreeMaker.TreeTag;
-import lombok.javac.JavacTreeMaker.TypeTag;
-import lombok.permit.Permit;
-
 import com.sun.tools.javac.code.Flags;
 import com.sun.tools.javac.code.Source;
 import com.sun.tools.javac.code.Symtab;
@@ -61,6 +55,13 @@ import com.sun.tools.javac.tree.JCTree.JCLiteral;
 import com.sun.tools.javac.tree.JCTree.JCVariableDecl;
 import com.sun.tools.javac.util.JCDiagnostic.DiagnosticPosition;
 
+import lombok.core.ClassLiteral;
+import lombok.core.FieldSelect;
+import lombok.core.handlers.HandlerUtil;
+import lombok.javac.JavacTreeMaker.TreeTag;
+import lombok.javac.JavacTreeMaker.TypeTag;
+import lombok.permit.Permit;
+
 /**
  * Container for static utility methods relevant to lombok's operation on javac.
  */
@@ -69,9 +70,6 @@ public class Javac {
 		// prevent instantiation
 	}
 	
-	/** Matches any of the 8 primitive names, such as {@code boolean}. */
-	private static final Pattern PRIMITIVE_TYPE_NAME_PATTERN = Pattern.compile("^(boolean|byte|short|int|long|float|double|char)$");
-	
 	private static final Pattern VERSION_PARSER = Pattern.compile("^(\\d{1,6})\\.?(\\d{1,6})?.*$");
 	private static final Pattern SOURCE_PARSER = Pattern.compile("^JDK(\\d{1,6})_?(\\d{1,6})?.*$");
 	
@@ -135,8 +133,7 @@ public class Javac {
 	 * expression) represents a primitive type.
 	 */
 	public static boolean isPrimitive(JCExpression ref) {
-		String typeName = ref.toString();
-		return PRIMITIVE_TYPE_NAME_PATTERN.matcher(typeName).matches();
+		return HandlerUtil.isPrimitive(ref.toString());
 	}
 	
 	/**

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

@@ -23,12 +23,12 @@ public final class ValueStaticConstructorOf {
 		if (o == this) return true;
 		if (!(o instanceof ValueStaticConstructorOf)) return false;
 		final ValueStaticConstructorOf other = (ValueStaticConstructorOf) o;
-		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;
 		final java.lang.Object this$price = this.getPrice();
 		final java.lang.Object other$price = other.getPrice();
 		if (this$price == null ? other$price != null : !this$price.equals(other$price)) 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;
 		return true;
 	}
 	@java.lang.Override
@@ -36,10 +36,10 @@ public final class ValueStaticConstructorOf {
 	public int hashCode() {
 		final int PRIME = 59;
 		int result = 1;
-		final java.lang.Object $name = this.getName();
-		result = result * PRIME + ($name == null ? 43 : $name.hashCode());
 		final java.lang.Object $price = this.getPrice();
 		result = result * PRIME + ($price == null ? 43 : $price.hashCode());
+		final java.lang.Object $name = this.getName();
+		result = result * PRIME + ($name == null ? 43 : $name.hashCode());
 		return result;
 	}
 	@java.lang.Override

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

@@ -19,23 +19,23 @@ public final @Value(staticConstructor = "of") class ValueStaticConstructorOf {
     if ((! (o instanceof ValueStaticConstructorOf)))
         return false;
     final ValueStaticConstructorOf other = (ValueStaticConstructorOf) o;
-    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;
     final java.lang.Object this$price = this.getPrice();
     final java.lang.Object other$price = other.getPrice();
     if (((this$price == null) ? (other$price != null) : (! this$price.equals(other$price))))
         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;
     return true;
   }
   public @java.lang.Override @java.lang.SuppressWarnings("all") int hashCode() {
     final int PRIME = 59;
     int result = 1;
-    final java.lang.Object $name = this.getName();
-    result = ((result * PRIME) + (($name == null) ? 43 : $name.hashCode()));
     final java.lang.Object $price = this.getPrice();
     result = ((result * PRIME) + (($price == null) ? 43 : $price.hashCode()));
+    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() {
@@ -44,4 +44,4 @@ public final @Value(staticConstructor = "of") class ValueStaticConstructorOf {
   public static @java.lang.SuppressWarnings("all") ValueStaticConstructorOf of(final String name, final Double price) {
     return new ValueStaticConstructorOf(name, price);
   }
-}
+}