Remove JSR 305 annotations

The findbugs jsr305 annotations jar uses annotations in the javax.annotation namespace, which causes problems with JPMS. Remove the annotations from the code base. The Jdbi code itself transitions to error prone and spotbugs. For code that uses Jdbi (and all tests), the Jakarta annotation-api `jakarta.annotation.Nullable` is used as other candidates either do not have Runtime retention (spotbugs, jetbrains), have problems with JPMS (jsr305), or are very specialized (spring) or obscure (checker framework).
jdbi · Jul 1, 2023 · f8ce7f3 · f8ce7f3
1 parent 8458281
commit f8ce7f3
Show file tree

Hide file tree

Showing 42 changed files with 213 additions and 205 deletions.
diff --git a/RELEASE_NOTES.md b/RELEASE_NOTES.md
@@ -4,6 +4,7 @@
   - Slight memory optimization on RowMappers and ColumnMappers findFor
   - JsonMapper implementations now bind the Type earlier, which saves work in Jackson and Gson (#2420)
   - performance improvements for all ExtensionConfigurer implementations (#2416, #2424)
+  - Remove JSR 305 annotations from the Jdbi codebase, one small step closer to JPMS.
 
 # 3.39.1
 

diff --git a/core/pom.xml b/core/pom.xml
@@ -55,9 +55,10 @@
             <groupId>com.github.spotbugs</groupId>
             <artifactId>spotbugs-annotations</artifactId>
         </dependency>
+
         <dependency>
-            <groupId>com.google.code.findbugs</groupId>
-            <artifactId>jsr305</artifactId>
+            <groupId>com.google.errorprone</groupId>
+            <artifactId>error_prone_annotations</artifactId>
         </dependency>
 
         <dependency>
@@ -91,6 +92,12 @@
             <scope>test</scope>
         </dependency>
 
+        <dependency>
+            <groupId>jakarta.annotation</groupId>
+            <artifactId>jakarta.annotation-api</artifactId>
+            <scope>test</scope>
+        </dependency>
+
         <dependency>
             <groupId>com.google.guava</groupId>
             <artifactId>guava</artifactId>

diff --git a/core/src/main/java/org/jdbi/v3/core/Handle.java b/core/src/main/java/org/jdbi/v3/core/Handle.java
@@ -24,8 +24,7 @@
 import java.util.Set;
 import java.util.concurrent.atomic.AtomicBoolean;
 
-import javax.annotation.concurrent.GuardedBy;
-
+import com.google.errorprone.annotations.concurrent.GuardedBy;
 import org.jdbi.v3.core.config.ConfigRegistry;
 import org.jdbi.v3.core.config.Configurable;
 import org.jdbi.v3.core.extension.ExtensionContext;

diff --git a/core/src/main/java/org/jdbi/v3/core/Sql.java b/core/src/main/java/org/jdbi/v3/core/Sql.java
@@ -18,8 +18,7 @@
 import java.util.stream.Collectors;
 import java.util.stream.StreamSupport;
 
-import javax.annotation.Nonnull;
-
+import edu.umd.cs.findbugs.annotations.NonNull;
 import org.jdbi.v3.meta.Beta;
 
 /**
@@ -104,7 +103,7 @@ public char charAt(int index) {
         return str.charAt(index);
     }
 
-    @Nonnull
+    @NonNull
     @Override
     public CharSequence subSequence(int start, int end) {
         return str.subSequence(start, end);

diff --git a/core/src/main/java/org/jdbi/v3/core/cache/internal/DefaultJdbiCache.java b/core/src/main/java/org/jdbi/v3/core/cache/internal/DefaultJdbiCache.java
@@ -18,8 +18,7 @@
 import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.ConcurrentMap;
 
-import javax.annotation.concurrent.GuardedBy;
-
+import com.google.errorprone.annotations.concurrent.GuardedBy;
 import org.jdbi.v3.core.cache.JdbiCache;
 import org.jdbi.v3.core.cache.JdbiCacheLoader;
 

diff --git a/core/src/main/java/org/jdbi/v3/core/codec/CodecFactory.java b/core/src/main/java/org/jdbi/v3/core/codec/CodecFactory.java
@@ -23,9 +23,7 @@
 import java.util.concurrent.ConcurrentMap;
 import java.util.function.Function;
 
-import javax.annotation.concurrent.NotThreadSafe;
-import javax.annotation.concurrent.ThreadSafe;
-
+import com.google.errorprone.annotations.ThreadSafe;
 import org.jdbi.v3.core.argument.Argument;
 import org.jdbi.v3.core.argument.QualifiedArgumentFactory;
 import org.jdbi.v3.core.config.ConfigRegistry;
@@ -113,7 +111,6 @@ protected Codec<?> resolveType(QualifiedType<?> qualifiedType) {
     /**
      * Fluent Builder for {@link CodecFactory}.
      */
-    @NotThreadSafe
     @Beta
     public static final class Builder {
 

diff --git a/core/src/main/java/org/jdbi/v3/core/interceptor/JdbiInterceptionChainHolder.java b/core/src/main/java/org/jdbi/v3/core/interceptor/JdbiInterceptionChainHolder.java
@@ -18,9 +18,9 @@
 import java.util.concurrent.CopyOnWriteArrayList;
 import java.util.function.Function;
 
-import javax.annotation.Nonnull;
-import javax.annotation.Nullable;
 
+import edu.umd.cs.findbugs.annotations.NonNull;
+import edu.umd.cs.findbugs.annotations.Nullable;
 import org.jdbi.v3.meta.Alpha;
 
 import static java.util.Objects.requireNonNull;
@@ -75,7 +75,7 @@ public JdbiInterceptionChainHolder(JdbiInterceptionChainHolder<S, T> that) {
      * @param source A source object.
      * @return A target object processed by one of the registered {@link JdbiInterceptor} instances.
      */
-    @Nonnull
+    @NonNull
     public T process(@Nullable S source) {
         ChainInstance instance = new ChainInstance(source);
         T result = instance.next();

diff --git a/core/src/main/java/org/jdbi/v3/core/interceptor/JdbiInterceptor.java b/core/src/main/java/org/jdbi/v3/core/interceptor/JdbiInterceptor.java
@@ -13,9 +13,8 @@
  */
 package org.jdbi.v3.core.interceptor;
 
-import javax.annotation.CheckForNull;
-import javax.annotation.Nullable;
-
+import edu.umd.cs.findbugs.annotations.CheckForNull;
+import edu.umd.cs.findbugs.annotations.Nullable;
 import org.jdbi.v3.meta.Alpha;
 
 /**

diff --git a/core/src/main/java/org/jdbi/v3/core/internal/exceptions/Sneaky.java b/core/src/main/java/org/jdbi/v3/core/internal/exceptions/Sneaky.java
@@ -19,9 +19,8 @@
 import java.io.UncheckedIOException;
 import java.lang.reflect.InvocationTargetException;
 
-import javax.annotation.CheckReturnValue;
-import javax.annotation.Nonnull;
-
+import edu.umd.cs.findbugs.annotations.CheckReturnValue;
+import edu.umd.cs.findbugs.annotations.NonNull;
 import org.jdbi.v3.core.internal.UtilityClassException;
 
 public class Sneaky {
@@ -33,7 +32,7 @@ private Sneaky() {
      * Will <b>always</b> throw an exception, so the caller should also always throw the dummy return value to make sure the control flow remains clear.
      */
     @CheckReturnValue
-    @Nonnull
+    @NonNull
     public static DummyException throwAnyway(Throwable t) {
         if (t instanceof Error) {
             throw (Error) t;

diff --git a/core/src/main/java/org/jdbi/v3/core/locator/internal/ClasspathBuilder.java b/core/src/main/java/org/jdbi/v3/core/locator/internal/ClasspathBuilder.java
@@ -19,8 +19,8 @@
 import java.util.regex.Pattern;
 import java.util.stream.Collectors;
 
-import javax.annotation.Nonnull;
-import javax.annotation.Nullable;
+import edu.umd.cs.findbugs.annotations.NonNull;
+import edu.umd.cs.findbugs.annotations.Nullable;
 
 public class ClasspathBuilder {
     private static final String DOT = ".";
@@ -42,17 +42,17 @@ public ClasspathBuilder setExtension(@Nullable String extension) {
     }
 
     // org.foo.Bar$Inner -> org/foo/Bar$Inner
-    public ClasspathBuilder appendFullyQualifiedClassName(@Nonnull Class<?> clazz) {
+    public ClasspathBuilder appendFullyQualifiedClassName(@NonNull Class<?> clazz) {
         return appendDotPath(clazz.getName());
     }
 
     // com.google.guava -> com/google/guava
-    public ClasspathBuilder appendDotPath(@Nonnull String path) {
+    public ClasspathBuilder appendDotPath(@NonNull String path) {
         return appendVerbatim(path.replace(DOT, SLASH));
     }
 
     // because sometimes you just don't have fancy data structures or patterns to work on
-    public ClasspathBuilder appendVerbatim(@Nonnull String s) {
+    public ClasspathBuilder appendVerbatim(@NonNull String s) {
         return addCarefully(s);
     }
 

diff --git a/core/src/main/java/org/jdbi/v3/core/mapper/reflect/ColumnName.java b/core/src/main/java/org/jdbi/v3/core/mapper/reflect/ColumnName.java
@@ -25,7 +25,7 @@
  * Specify the mapping name for a property or parameter explicitly. This annotation is respected by:
  *
  * <ul>
- *     <li>BeanMapper, FieldMapper, and ConstructorMapper in core</li>
+ *     <li>{@link BeanMapper}, {@link FieldMapper}, and {@link ConstructorMapper} in core</li>
  *     <li>The Kotlin data class mapper in KotlinPlugin</li>
  *     <li>Immutables property definitions</li>
  * </ul>

diff --git a/core/src/main/java/org/jdbi/v3/core/mapper/reflect/ConstructorMapper.java b/core/src/main/java/org/jdbi/v3/core/mapper/reflect/ConstructorMapper.java
@@ -303,15 +303,17 @@ private boolean isNullable(Parameter parameter) {
     private static String paramName(Parameter[] parameters,
                                     int position,
                                     ConstructorProperties parameterNames) {
+
         final Parameter parameter = parameters[position];
-        ColumnName dbName = parameter.getAnnotation(ColumnName.class);
-        if (dbName != null) {
-            return dbName.value();
-        }
-        if (parameterNames != null) {
-            return parameterNames.value()[position];
-        }
-        return parameter.getName();
+        return Optional.ofNullable(parameter.getAnnotation(ColumnName.class))
+            .map(ColumnName::value)
+            .orElseGet(() -> {
+                if (parameterNames != null) {
+                    return parameterNames.value()[position];
+                } else {
+                    return parameter.getName();
+                }
+            });
     }
 
     private String debugName(Parameter parameter) {

diff --git a/core/src/main/java/org/jdbi/v3/core/mapper/reflect/InstanceFactory.java b/core/src/main/java/org/jdbi/v3/core/mapper/reflect/InstanceFactory.java
@@ -19,7 +19,7 @@
 import java.util.Objects;
 import java.util.stream.Stream;
 
-import javax.annotation.Nullable;
+import edu.umd.cs.findbugs.annotations.Nullable;
 
 import static java.util.Objects.requireNonNull;
 

diff --git a/core/src/main/java/org/jdbi/v3/core/mapper/reflect/internal/PojoBuilderUtils.java b/core/src/main/java/org/jdbi/v3/core/mapper/reflect/internal/PojoBuilderUtils.java
@@ -22,6 +22,7 @@
 import java.util.Arrays;
 import java.util.LinkedHashSet;
 import java.util.List;
+import java.util.Optional;
 import java.util.Set;
 
 import org.jdbi.v3.core.generic.GenericTypes;
@@ -42,11 +43,10 @@ public static boolean isProperty(Method m) {
     }
 
     public static String propertyName(Method m) {
-        ColumnName colName = m.getAnnotation(ColumnName.class);
-        if (colName != null) {
-            return colName.value();
-        }
-        return defaultSetterName(m.getName());
+
+        return Optional.ofNullable(m.getAnnotation(ColumnName.class))
+            .map(ColumnName::value)
+            .orElseGet(() -> defaultSetterName(m.getName()));
     }
 
     public static String defaultSetterName(String name) {
@@ -83,10 +83,16 @@ public static MethodHandle findBuilderSetter(final Class<?> builderClass, String
         if (name.length() > 1) {
             names.addAll(setterNames(name));
         }
-        ColumnName columnName = decl.getAnnotation(ColumnName.class);
-        if (columnName != null && columnName.value().equals(name)) {
-            names.addAll(setterNames(PojoBuilderUtils.defaultSetterName(decl.getName())));
-        }
+
+        Optional<String> columnName = Optional.ofNullable(decl.getAnnotation(ColumnName.class))
+            .map(ColumnName::value);
+
+        columnName.ifPresent(n -> {
+            if (n.equals(name)) {
+                names.addAll(setterNames(PojoBuilderUtils.defaultSetterName(decl.getName())));
+            }
+        });
+
         for (String tryName : names) {
             try {
                 return MethodHandles.lookup().unreflect(builderClass.getMethod(tryName, GenericTypes.getErasedType(type)));

diff --git a/core/src/main/java/org/jdbi/v3/core/statement/SqlStatements.java b/core/src/main/java/org/jdbi/v3/core/statement/SqlStatements.java
@@ -27,8 +27,7 @@
 import java.util.concurrent.CopyOnWriteArraySet;
 import java.util.function.Function;
 
-import javax.annotation.Nullable;
-
+import edu.umd.cs.findbugs.annotations.Nullable;
 import org.jdbi.v3.core.Handle;
 import org.jdbi.v3.core.Jdbi;
 import org.jdbi.v3.core.cache.JdbiCache;

diff --git a/core/src/main/java/org/jdbi/v3/core/statement/StatementContext.java b/core/src/main/java/org/jdbi/v3/core/statement/StatementContext.java
@@ -32,8 +32,7 @@
 import java.util.Set;
 import java.util.stream.Collector;
 
-import javax.annotation.Nullable;
-
+import edu.umd.cs.findbugs.annotations.Nullable;
 import org.jdbi.v3.core.CloseException;
 import org.jdbi.v3.core.Handle;
 import org.jdbi.v3.core.argument.Argument;

diff --git a/core/src/test/java/org/jdbi/v3/core/interceptor/JdbiInterceptionChainHolderTest.java b/core/src/test/java/org/jdbi/v3/core/interceptor/JdbiInterceptionChainHolderTest.java
@@ -16,9 +16,6 @@
 import java.util.UUID;
 import java.util.function.Function;
 
-import javax.annotation.CheckForNull;
-import javax.annotation.Nullable;
-
 import org.junit.jupiter.api.Test;
 
 import static org.assertj.core.api.Assertions.assertThat;
@@ -263,9 +260,8 @@ String getTarget() {
             return target;
         }
 
-        @CheckForNull
         @Override
-        public String intercept(@Nullable S s, JdbiInterceptionChain<String> chain) {
+        public String intercept(S s, JdbiInterceptionChain<String> chain) {
             this.source = s;
             if (doIt) {
                 return this.target;

diff --git a/core/src/test/java/org/jdbi/v3/core/mapper/reflect/BeanMapperPropagateNullableTest.java b/core/src/test/java/org/jdbi/v3/core/mapper/reflect/BeanMapperPropagateNullableTest.java
@@ -14,7 +14,7 @@
 
 package org.jdbi.v3.core.mapper.reflect;
 
-import javax.annotation.Nullable;
+import jakarta.annotation.Nullable;
 
 import org.jdbi.v3.core.Handle;
 import org.jdbi.v3.core.junit5.H2DatabaseExtension;

diff --git a/core/src/test/java/org/jdbi/v3/core/mapper/reflect/ConstructorMapperTest.java b/core/src/test/java/org/jdbi/v3/core/mapper/reflect/ConstructorMapperTest.java
@@ -15,7 +15,7 @@
 
 import java.beans.ConstructorProperties;
 
-import javax.annotation.Nullable;
+import jakarta.annotation.Nullable;
 
 import org.jdbi.v3.core.Handle;
 import org.jdbi.v3.core.junit5.H2DatabaseExtension;

diff --git a/core/src/test/java/org/jdbi/v3/core/mapper/reflect/FieldMapperTest.java b/core/src/test/java/org/jdbi/v3/core/mapper/reflect/FieldMapperTest.java
@@ -13,7 +13,7 @@
  */
 package org.jdbi.v3.core.mapper.reflect;
 
-import javax.annotation.Nullable;
+import jakarta.annotation.Nullable;
 
 import org.jdbi.v3.core.Handle;
 import org.jdbi.v3.core.SampleBean;

diff --git a/docs/pom.xml b/docs/pom.xml
@@ -162,11 +162,6 @@
             <artifactId>spotbugs-annotations</artifactId>
         </dependency>
 
-        <dependency>
-            <groupId>com.google.code.findbugs</groupId>
-            <artifactId>jsr305</artifactId>
-        </dependency>
-
         <dependency>
             <groupId>jakarta.inject</groupId>
             <artifactId>jakarta.inject-api</artifactId>