From: Robert Varga <robert.varga@pantheon.tech>
Date: Sat, 16 Dec 2023 13:59:55 +0000 (+0100)
Subject: Clean up AbstractCompositeRuntimeType
X-Git-Tag: v13.0.0~76
X-Git-Url: https://git.opendaylight.org/gerrit/gitweb?p=mdsal.git;a=commitdiff_plain;h=edc1fe4610c43c79a039ce8156c143a5ee43df89

Clean up AbstractCompositeRuntimeType

SpotBugs is no longer complaining about the Comparator, hence we can
completely remove the dependency of spotbugs-annotations.

While we are here, also clean up the Arrays.binarySearch() assumption we
make, so we tolerate if the key is reported as the first argument.

Change-Id: I97b9683b64e49617ca529826e37392face70739a
Signed-off-by: Robert Varga <robert.varga@pantheon.tech>
---

diff --git a/binding/mdsal-binding-generator/pom.xml b/binding/mdsal-binding-generator/pom.xml
index dfdb448b6e..a44f167917 100644
--- a/binding/mdsal-binding-generator/pom.xml
+++ b/binding/mdsal-binding-generator/pom.xml
@@ -20,11 +20,6 @@
     <packaging>bundle</packaging>
 
     <dependencies>
-        <dependency>
-            <groupId>com.github.spotbugs</groupId>
-            <artifactId>spotbugs-annotations</artifactId>
-            <optional>true</optional>
-        </dependency>
         <dependency>
             <groupId>com.google.guava</groupId>
             <artifactId>guava</artifactId>
diff --git a/binding/mdsal-binding-generator/src/main/java/module-info.java b/binding/mdsal-binding-generator/src/main/java/module-info.java
index 88dd6f8bd1..83b182b150 100644
--- a/binding/mdsal-binding-generator/src/main/java/module-info.java
+++ b/binding/mdsal-binding-generator/src/main/java/module-info.java
@@ -34,7 +34,6 @@ module org.opendaylight.mdsal.binding.generator {
 
     // Annotations
     requires static transitive org.eclipse.jdt.annotation;
-    requires static com.github.spotbugs.annotations;
     requires static javax.inject;
     requires static org.kohsuke.metainf_services;
     requires static org.osgi.service.component.annotations;
diff --git a/binding/mdsal-binding-generator/src/main/java/org/opendaylight/mdsal/binding/generator/impl/rt/AbstractCompositeRuntimeType.java b/binding/mdsal-binding-generator/src/main/java/org/opendaylight/mdsal/binding/generator/impl/rt/AbstractCompositeRuntimeType.java
index a806f99a48..d144120299 100644
--- a/binding/mdsal-binding-generator/src/main/java/org/opendaylight/mdsal/binding/generator/impl/rt/AbstractCompositeRuntimeType.java
+++ b/binding/mdsal-binding-generator/src/main/java/org/opendaylight/mdsal/binding/generator/impl/rt/AbstractCompositeRuntimeType.java
@@ -7,13 +7,12 @@
  */
 package org.opendaylight.mdsal.binding.generator.impl.rt;
 
-import static com.google.common.base.Verify.verify;
 import static java.util.Objects.requireNonNull;
+import static java.util.Objects.requireNonNullElse;
 
 import com.google.common.base.Functions;
 import com.google.common.base.VerifyException;
 import com.google.common.collect.ImmutableMap;
-import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
 import java.util.Arrays;
 import java.util.Collections;
 import java.util.List;
@@ -34,8 +33,6 @@ abstract class AbstractCompositeRuntimeType<S extends EffectiveStatement<?, ?>>
     private final ImmutableMap<JavaTypeName, GeneratedRuntimeType> byClass;
     private final Object bySchemaTree;
 
-    @SuppressFBWarnings(value = "SE_COMPARATOR_SHOULD_BE_SERIALIZABLE",
-        justification = "https://github.com/spotbugs/spotbugs/issues/1985")
     AbstractCompositeRuntimeType(final GeneratedType bindingType, final S statement, final List<RuntimeType> children) {
         super(bindingType, statement);
 
@@ -53,7 +50,9 @@ abstract class AbstractCompositeRuntimeType<S extends EffectiveStatement<?, ?>>
             default -> {
                 Arrays.sort(tmp, (o1, o2) -> {
                     final int cmp = extractQName(o1).compareTo(extractQName(o2));
-                    verify(cmp != 0, "Type %s conflicts with %s on schema tree", o1, o2);
+                    if (cmp == 0) {
+                        throw new VerifyException("Type " + o1 + " conflicts with " + o2 + " on schema tree");
+                    }
                     return cmp;
                 });
                 yield tmp;
@@ -68,15 +67,13 @@ abstract class AbstractCompositeRuntimeType<S extends EffectiveStatement<?, ?>>
         }
 
         final var tmp = (RuntimeType[]) bySchemaTree;
-        @SuppressFBWarnings(value = "SE_COMPARATOR_SHOULD_BE_SERIALIZABLE",
-            justification = "https://github.com/spotbugs/spotbugs/issues/1985")
-        final int offset = Arrays.binarySearch(tmp, null, (o1, o2) -> {
-            // We make assumptions about how Arrays.binarySearch() is implemented: o2 is expected to be the provided
-            // key -- which is null. This helps CHA by not introducing a fake RuntimeType class and the
-            // corresponding instanceof checks.
-            verify(o2 == null, "Unexpected key %s", o2);
-            return extractQName(o1).compareTo(qname);
-        });
+        // Here we are assuming that Arrays.binarySearch() accepts a null object, so as to help CHA by not introducing
+        // a fake RuntimeType class and the corresponding instanceof checks to side-step the statement lookup (which
+        // would need more faking).
+        // We make a slight assumption that o2 is the what we specify as a key, but can recover if it is the other way
+        // around.
+        final int offset = Arrays.binarySearch(tmp, null,
+            (o1, o2) -> extractQName(requireNonNullElse(o1, o2)).compareTo(qname));
         return offset < 0 ? null : tmp[offset];
     }
 
@@ -94,7 +91,9 @@ abstract class AbstractCompositeRuntimeType<S extends EffectiveStatement<?, ?>>
 
         final var tmp = (RuntimeType[]) bySchemaTree;
         for (var item : tmp) {
-            verify(expectedType.isInstance(item), "Unexpected schema tree child %s", item);
+            if (!expectedType.isInstance(item)) {
+                throw new VerifyException("Unexpected schema tree child " + item);
+            }
         }
         return (List<T>) Collections.unmodifiableList(Arrays.asList(tmp));
     }