diff --git a/code/src/java/pcgen/cdom/enumeration/DisplayLocation.java b/code/src/java/pcgen/cdom/enumeration/DisplayLocation.java index eebd4d886d3..ea15ed10e19 100644 --- a/code/src/java/pcgen/cdom/enumeration/DisplayLocation.java +++ b/code/src/java/pcgen/cdom/enumeration/DisplayLocation.java @@ -21,6 +21,7 @@ import java.util.Collections; import java.util.Objects; +import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; import pcgen.base.enumeration.TypeSafeConstant; import pcgen.base.util.CaseInsensitiveMap; @@ -161,15 +162,11 @@ public static void clearConstants() } @Override + @SuppressFBWarnings(value = "EQ_COMPARETO_USE_OBJECT_EQUALS", + justification = "Singleton-by-construction via getConstant intern map; " + + "no .equals() callers, not used as a collection key. Identity equality is correct here.") public int compareTo(DisplayLocation type) { - /* - * Note: Some tools will report a problem here because Type implements - * compareTo, but does not implement custom implementations of hashCode - * or equals(). Because this is intended as a TypeSafeConstant, and Type - * has a private constructor, it is unnecessary to implement a custom - * hashCode or equals. - */ return fieldName.compareTo(type.fieldName); } diff --git a/code/src/java/pcgen/cdom/enumeration/Region.java b/code/src/java/pcgen/cdom/enumeration/Region.java index 8995713ea64..9385b60eb9c 100644 --- a/code/src/java/pcgen/cdom/enumeration/Region.java +++ b/code/src/java/pcgen/cdom/enumeration/Region.java @@ -20,6 +20,7 @@ import java.lang.reflect.Field; import java.util.Collection; import java.util.Collections; +import java.util.Locale; import java.util.Objects; import pcgen.base.enumeration.TypeSafeConstant; @@ -28,13 +29,23 @@ import pcgen.cdom.base.Constants; /** - * * This Class is a Type Safe Constant. It is designed to hold Regions in a * type-safe fashion, so that they can be quickly compared and use less memory * when identical Regions exist in two CDOMObjects. + * + *

{@code equals} and {@code hashCode} are case-insensitive on + * {@code fieldName} to match {@link #compareTo} (Comparable contract) and to + * support use as a {@code HashMap} key — see {@code BioSet}'s + * {@code DoubleKeyMap, …>} and the {@code .equals()} calls + * in {@code RegionFacet.matchesRegion}.

*/ public final class Region implements TypeSafeConstant, Comparable { + /** + * This is used to provide a unique ordinal to each constant in this class + */ + private static int ordinalCount = 0; + /** * A "None" region for universal use. */ @@ -45,11 +56,6 @@ public final class Region implements TypeSafeConstant, Comparable */ private static CaseInsensitiveMap typeMap; - /** - * This is used to provide a unique ordinal to each constant in this class - */ - private static int ordinalCount = 0; - /** * The name of this Constant */ @@ -202,14 +208,20 @@ public static void clearConstants() @Override public int compareTo(Region region) { - /* - * Note: Some tools will report a problem here because Region implements - * compareTo, but does not implement custom implementations of hashCode - * or equals(). Because this is intended as a TypeSafeConstant, and Type - * has a private constructor, it is unnecessary to implement a custom - * hashCode or equals. - */ return fieldName.compareToIgnoreCase(region.fieldName); } + @Override + public boolean equals(Object o) + { + return o instanceof Region other + && String.CASE_INSENSITIVE_ORDER.compare(fieldName, other.fieldName) == 0; + } + + @Override + public int hashCode() + { + return fieldName.toLowerCase(Locale.ROOT).hashCode(); + } + } diff --git a/code/src/java/pcgen/cdom/enumeration/Type.java b/code/src/java/pcgen/cdom/enumeration/Type.java index 31c50e48a08..9f2dddfd2c4 100644 --- a/code/src/java/pcgen/cdom/enumeration/Type.java +++ b/code/src/java/pcgen/cdom/enumeration/Type.java @@ -28,10 +28,15 @@ import pcgen.cdom.base.Constants; /** - * * This Class is a Type Safe Constant. It is designed to hold Types in a * type-safe fashion, so that they can be quickly compared and use less memory * when identical Types exist in two CDOMObjects. + * + *

{@code equals} and {@code hashCode} are value-based on {@code fieldName} + * (case-sensitive, matching {@link #compareTo}). Type is used as a + * {@code HashSet}/{@code HashMap} key across the codebase (DataSet, GameMode, + * BodyStructure, spell/skill lists, …); the value-equals contract holds even + * when an instance is constructed outside the {@link #getConstant} intern map.

*/ public final class Type implements TypeSafeConstant, Comparable { @@ -219,16 +224,21 @@ public static Collection getAllConstants() @Override public int compareTo(Type type) { - /* - * Note: Some tools will report a problem here because Type implements - * compareTo, but does not implement custom implementations of hashCode - * or equals(). Because this is intended as a TypeSafeConstant, and Type - * has a private constructor, it is unnecessary to implement a custom - * hashCode or equals. - */ return fieldName.compareTo(type.fieldName); } + @Override + public boolean equals(Object o) + { + return o instanceof Type other && fieldName.equals(other.fieldName); + } + + @Override + public int hashCode() + { + return fieldName.hashCode(); + } + public static void buildMap() { TYPE_MAP.clear();