BCELComparator now uses generics

Avoid NPEs in BCELComparator implementations
apache · Jan 7, 2024 · f230b00 · f230b00
1 parent add006f
commit f230b00
Show file tree

Hide file tree

Showing 13 changed files with 250 additions and 103 deletions.
diff --git a/src/changes/changes.xml b/src/changes/changes.xml
@@ -68,6 +68,14 @@ The <action> type attribute can be add,update,fix,remove.
       <!-- FIX  -->
       <action                  type="fix" dev="ggregory" due-to="Gary Gregory">Replace internal use of StringBuffer with StringBuilder.</action>
       <action issue="BCEL-370" type="fix" dev="ggregory" due-to="Gary Gregory">CONSTANT_Dynamic is not handled in LDC #254.</action>
+      <action                  type="fix" dev="ggregory" due-to="Gary Gregory">BCELComparator now uses generics.</action>
+      <action                  type="fix" dev="ggregory" due-to="Gary Gregory">Avoid NullPointerException in ClassGen.BCELComparator#equals() and ClassGen.BCELComparator#hashCode().</action>
+      <action                  type="fix" dev="ggregory" due-to="Gary Gregory">Avoid NullPointerException in Constant.BCELComparator#equals() and Constant.BCELComparator#hashCode().</action>
+      <action                  type="fix" dev="ggregory" due-to="Gary Gregory">Avoid NullPointerException in Field.BCELComparator#equals() and Field.BCELComparator#hashCode().</action>
+      <action                  type="fix" dev="ggregory" due-to="Gary Gregory">Avoid NullPointerException in FieldGen.BCELComparator#equals() and FieldGen.BCELComparator#hashCode().</action>
+      <action                  type="fix" dev="ggregory" due-to="Gary Gregory">Avoid NullPointerException in JavaClass.BCELComparator#equals() and JavaClass.BCELComparator#hashCode().</action>
+      <action                  type="fix" dev="ggregory" due-to="Gary Gregory">Avoid NullPointerException in Method.BCELComparator#equals() and Method.BCELComparator#hashCode().</action>
+      <action                  type="fix" dev="ggregory" due-to="Gary Gregory">Avoid NullPointerException in MethodGen.BCELComparator#equals() and MethodGen.BCELComparator#hashCode().</action>
       <!-- UPDATE -->
       <action                  type="update" dev="ggregory" due-to="Dependabot">Bump GitHub various actions for CI builds.</action>
       <action                  type="update" dev="ggregory" due-to="Dependabot">Bump jna.version from 5.13.0 to 5.14.0 #250.</action>

diff --git a/src/main/java/org/apache/bcel/classfile/Constant.java b/src/main/java/org/apache/bcel/classfile/Constant.java
@@ -30,26 +30,23 @@
  */
 public abstract class Constant implements Cloneable, Node {
 
-    private static BCELComparator bcelComparator = new BCELComparator() {
+    private static BCELComparator<Constant> bcelComparator = new BCELComparator<Constant>() {
 
         @Override
-        public boolean equals(final Object o1, final Object o2) {
-            final Constant THIS = (Constant) o1;
-            final Constant THAT = (Constant) o2;
-            return Objects.equals(THIS.toString(), THAT.toString());
+        public boolean equals(final Constant a, final Constant b) {
+            return a == b || a != null && b != null && Objects.equals(a.toString(), b.toString());
         }
 
         @Override
-        public int hashCode(final Object o) {
-            final Constant THIS = (Constant) o;
-            return THIS.toString().hashCode();
+        public int hashCode(final Constant o) {
+            return o != null ? Objects.hashCode(o.toString()) : 0;
         }
     };
 
     /**
-     * @return Comparison strategy object
+     * @return Comparison strategy object.
      */
-    public static BCELComparator getComparator() {
+    public static BCELComparator<Constant> getComparator() {
         return bcelComparator;
     }
 
@@ -107,7 +104,7 @@ public static Constant readConstant(final DataInput dataInput) throws IOExceptio
     /**
      * @param comparator Comparison strategy object
      */
-    public static void setComparator(final BCELComparator comparator) {
+    public static void setComparator(final BCELComparator<Constant> comparator) {
         bcelComparator = comparator;
     }
 
@@ -168,7 +165,7 @@ public Constant copy() {
      */
     @Override
     public boolean equals(final Object obj) {
-        return bcelComparator.equals(this, obj);
+        return obj instanceof Constant && bcelComparator.equals(this, (Constant) obj);
     }
 
     /**

diff --git a/src/main/java/org/apache/bcel/classfile/Field.java b/src/main/java/org/apache/bcel/classfile/Field.java
@@ -37,19 +37,16 @@ public final class Field extends FieldOrMethod {
      */
     public static final Field[] EMPTY_ARRAY = {};
 
-    private static BCELComparator bcelComparator = new BCELComparator() {
+    private static BCELComparator<Field> bcelComparator = new BCELComparator<Field>() {
 
         @Override
-        public boolean equals(final Object o1, final Object o2) {
-            final Field THIS = (Field) o1;
-            final Field THAT = (Field) o2;
-            return Objects.equals(THIS.getName(), THAT.getName()) && Objects.equals(THIS.getSignature(), THAT.getSignature());
+        public boolean equals(final Field a, final Field b) {
+            return a == b || a != null && b != null && Objects.equals(a.getName(), b.getName()) && Objects.equals(a.getSignature(), b.getSignature());
         }
 
         @Override
-        public int hashCode(final Object o) {
-            final Field THIS = (Field) o;
-            return THIS.getSignature().hashCode() ^ THIS.getName().hashCode();
+        public int hashCode(final Field o) {
+            return o != null ? Objects.hash(o.getSignature(), o.getName()) : 0;
         }
     };
 
@@ -59,23 +56,23 @@ public int hashCode(final Object o) {
     static final Field[] EMPTY_FIELD_ARRAY = {};
 
     /**
-     * @return Comparison strategy object
+     * @return Comparison strategy object.
      */
-    public static BCELComparator getComparator() {
+    public static BCELComparator<Field> getComparator() {
         return bcelComparator;
     }
 
     /**
-     * @param comparator Comparison strategy object
+     * @param comparator Comparison strategy object.
      */
-    public static void setComparator(final BCELComparator comparator) {
+    public static void setComparator(final BCELComparator<Field> comparator) {
         bcelComparator = comparator;
     }
 
     /**
      * Constructs object from file stream.
      *
-     * @param file Input stream
+     * @param file Input stream.
      */
     Field(final DataInput file, final ConstantPool constantPool) throws IOException, ClassFormatException {
         super(file, constantPool);
@@ -128,7 +125,7 @@ public Field copy(final ConstantPool constantPool) {
      */
     @Override
     public boolean equals(final Object obj) {
-        return bcelComparator.equals(this, obj);
+        return obj instanceof Field && bcelComparator.equals(this, (Field) obj);
     }
 
     /**

diff --git a/src/main/java/org/apache/bcel/classfile/JavaClass.java b/src/main/java/org/apache/bcel/classfile/JavaClass.java
@@ -65,19 +65,17 @@ public class JavaClass extends AccessFlags implements Cloneable, Node, Comparabl
     public static final byte FILE = 2;
     public static final byte ZIP = 3;
     private static final boolean debug = Boolean.getBoolean("JavaClass.debug"); // Debugging on/off
-    private static BCELComparator bcelComparator = new BCELComparator() {
+
+    private static BCELComparator<JavaClass> bcelComparator = new BCELComparator<JavaClass>() {
 
         @Override
-        public boolean equals(final Object o1, final Object o2) {
-            final JavaClass THIS = (JavaClass) o1;
-            final JavaClass THAT = (JavaClass) o2;
-            return Objects.equals(THIS.getClassName(), THAT.getClassName());
+        public boolean equals(final JavaClass a, final JavaClass b) {
+            return a == b || a != null && b != null && Objects.equals(a.getClassName(), b.getClassName());
         }
 
         @Override
-        public int hashCode(final Object o) {
-            final JavaClass THIS = (JavaClass) o;
-            return THIS.getClassName().hashCode();
+        public int hashCode(final JavaClass o) {
+            return o != null ? Objects.hashCode(o.getClassName()) : 0;
         }
     };
 
@@ -91,9 +89,9 @@ static void Debug(final String str) {
     }
 
     /**
-     * @return Comparison strategy object
+     * @return Comparison strategy object.
      */
-    public static BCELComparator getComparator() {
+    public static BCELComparator<JavaClass> getComparator() {
         return bcelComparator;
     }
 
@@ -107,9 +105,9 @@ private static String indent(final Object obj) {
     }
 
     /**
-     * @param comparator Comparison strategy object
+     * @param comparator Comparison strategy object.
      */
-    public static void setComparator(final BCELComparator comparator) {
+    public static void setComparator(final BCELComparator<JavaClass> comparator) {
         bcelComparator = comparator;
     }
 
@@ -391,7 +389,7 @@ public void dump(final String fileName) throws IOException {
      */
     @Override
     public boolean equals(final Object obj) {
-        return bcelComparator.equals(this, obj);
+        return obj instanceof JavaClass && bcelComparator.equals(this, (JavaClass) obj);
     }
 
     /**

diff --git a/src/main/java/org/apache/bcel/classfile/Method.java b/src/main/java/org/apache/bcel/classfile/Method.java
@@ -36,19 +36,16 @@ public final class Method extends FieldOrMethod {
      */
     public static final Method[] EMPTY_ARRAY = {};
 
-    private static BCELComparator bcelComparator = new BCELComparator() {
+    private static BCELComparator<Method> bcelComparator = new BCELComparator<Method>() {
 
         @Override
-        public boolean equals(final Object o1, final Object o2) {
-            final Method THIS = (Method) o1;
-            final Method THAT = (Method) o2;
-            return Objects.equals(THIS.getName(), THAT.getName()) && Objects.equals(THIS.getSignature(), THAT.getSignature());
+        public boolean equals(final Method a, final Method b) {
+            return a == b || a != null && b != null && Objects.equals(a.getName(), b.getName()) && Objects.equals(a.getSignature(), b.getSignature());
         }
 
         @Override
-        public int hashCode(final Object o) {
-            final Method THIS = (Method) o;
-            return THIS.getSignature().hashCode() ^ THIS.getName().hashCode();
+        public int hashCode(final Method o) {
+            return o != null ? Objects.hash(o.getSignature(), o.getName()) : 0;
         }
     };
 
@@ -58,20 +55,20 @@ public int hashCode(final Object o) {
     static final Method[] EMPTY_METHOD_ARRAY = {};
 
     /**
-     * @return Comparison strategy object
+     * @return Comparison strategy object.
      */
-    public static BCELComparator getComparator() {
+    public static BCELComparator<Method> getComparator() {
         return bcelComparator;
     }
 
     /**
-     * @param comparator Comparison strategy object
+     * @param comparator Comparison strategy object.
      */
-    public static void setComparator(final BCELComparator comparator) {
+    public static void setComparator(final BCELComparator<Method> comparator) {
         bcelComparator = comparator;
     }
 
-    // annotations defined on the parameters of a method
+    /** Annotations defined on the parameters of a method. */
     private ParameterAnnotationEntry[] parameterAnnotationEntries;
 
     /**
@@ -138,7 +135,7 @@ public Method copy(final ConstantPool constantPool) {
      */
     @Override
     public boolean equals(final Object obj) {
-        return bcelComparator.equals(this, obj);
+        return obj instanceof Method && bcelComparator.equals(this, (Method) obj);
     }
 
     /**

diff --git a/src/main/java/org/apache/bcel/generic/ClassGen.java b/src/main/java/org/apache/bcel/generic/ClassGen.java
@@ -44,33 +44,30 @@
  */
 public class ClassGen extends AccessFlags implements Cloneable {
 
-    private static BCELComparator bcelComparator = new BCELComparator() {
+    private static BCELComparator<ClassGen> bcelComparator = new BCELComparator<ClassGen>() {
 
         @Override
-        public boolean equals(final Object o1, final Object o2) {
-            final ClassGen THIS = (ClassGen) o1;
-            final ClassGen THAT = (ClassGen) o2;
-            return Objects.equals(THIS.getClassName(), THAT.getClassName());
+        public boolean equals(final ClassGen a, final ClassGen b) {
+            return a == b || a != null && b != null && Objects.equals(a.getClassName(), b.getClassName());
         }
 
         @Override
-        public int hashCode(final Object o) {
-            final ClassGen THIS = (ClassGen) o;
-            return THIS.getClassName().hashCode();
+        public int hashCode(final ClassGen o) {
+            return o != null ? Objects.hashCode(o.getClassName()) : 0;
         }
     };
 
     /**
      * @return Comparison strategy object
      */
-    public static BCELComparator getComparator() {
+    public static BCELComparator<ClassGen> getComparator() {
         return bcelComparator;
     }
 
     /**
      * @param comparator Comparison strategy object
      */
-    public static void setComparator(final BCELComparator comparator) {
+    public static void setComparator(final BCELComparator<ClassGen> comparator) {
         bcelComparator = comparator;
     }
 
@@ -279,7 +276,7 @@ public Method containsMethod(final String name, final String signature) {
      */
     @Override
     public boolean equals(final Object obj) {
-        return bcelComparator.equals(this, obj);
+        return obj instanceof ClassGen && bcelComparator.equals(this, (ClassGen) obj);
     }
 
     // J5TODO: Should we make calling unpackAnnotations() lazy and put it in here?

diff --git a/src/main/java/org/apache/bcel/generic/FieldGen.java b/src/main/java/org/apache/bcel/generic/FieldGen.java
@@ -40,33 +40,30 @@
  */
 public class FieldGen extends FieldGenOrMethodGen {
 
-    private static BCELComparator bcelComparator = new BCELComparator() {
+    private static BCELComparator<FieldGen> bcelComparator = new BCELComparator<FieldGen>() {
 
         @Override
-        public boolean equals(final Object o1, final Object o2) {
-            final FieldGen THIS = (FieldGen) o1;
-            final FieldGen THAT = (FieldGen) o2;
-            return Objects.equals(THIS.getName(), THAT.getName()) && Objects.equals(THIS.getSignature(), THAT.getSignature());
+        public boolean equals(final FieldGen a, final FieldGen b) {
+            return a == b || a != null && b != null && Objects.equals(a.getName(), b.getName()) && Objects.equals(a.getSignature(), b.getSignature());
         }
 
         @Override
-        public int hashCode(final Object o) {
-            final FieldGen THIS = (FieldGen) o;
-            return THIS.getSignature().hashCode() ^ THIS.getName().hashCode();
+        public int hashCode(final FieldGen o) {
+            return o != null ? Objects.hash(o.getSignature(), o.getName()) : 0;
         }
     };
 
     /**
-     * @return Comparison strategy object
+     * @return Comparison strategy object.
      */
-    public static BCELComparator getComparator() {
+    public static BCELComparator<FieldGen> getComparator() {
         return bcelComparator;
     }
 
     /**
-     * @param comparator Comparison strategy object
+     * @param comparator Comparison strategy object.
      */
-    public static void setComparator(final BCELComparator comparator) {
+    public static void setComparator(final BCELComparator<FieldGen> comparator) {
         bcelComparator = comparator;
     }
 
@@ -77,8 +74,8 @@ public static void setComparator(final BCELComparator comparator) {
     /**
      * Instantiate from existing field.
      *
-     * @param field Field object
-     * @param cp constant pool (must contain the same entries as the field's constant pool)
+     * @param field Field object.
+     * @param cp constant pool (must contain the same entries as the field's constant pool).
      */
     public FieldGen(final Field field, final ConstantPoolGen cp) {
         this(field.getAccessFlags(), Type.getType(field.getSignature()), field.getName(), cp);
@@ -183,7 +180,7 @@ public FieldGen copy(final ConstantPoolGen cp) {
      */
     @Override
     public boolean equals(final Object obj) {
-        return bcelComparator.equals(this, obj);
+        return obj instanceof FieldGen && bcelComparator.equals(this, (FieldGen) obj);
     }
 
     /**