Lots of JPMS fixes

Once actually building with Java 11 the module-info.java files proved insufficient so these have been updated to reflect the shortcomings identified. One side effect of this is that in some modules the tests now have a separate tests package and associated module to make all the tests run properly under JPMS.
rvesse · Nov 23, 2023 · 1cdde39 · 1cdde39
1 parent 173df02
commit 1cdde39
Show file tree

Hide file tree

Showing 275 changed files with 792 additions and 713 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -20,6 +20,8 @@ files.
     deprecated
   - Added a `JpmsResourceLocator` as an additional `ResourceLocator` to allow finding resources when running in a JPMS
     context (requires additional module `airline-jpms-resources`)
+  - **BREAKING** - Only `@AirlineModule` is used as a composition annotation by default, use of the older
+    `@javax.inject.Inject` or `@jakarta.inject.Inject` annotations **MUST** now be explicitly configured.
 - Help Improvements
   - Added an `@SeeAlso` annotation to Airline Core (#51)
   - **BREAKING** - `airline-help-bash` has moved `@BashCompletion` annotation into
@@ -29,7 +31,7 @@ files.
   - A `ParserBuilder` created by calling `withParser()` on a `CliBuilder` can now return control back to its parent via
     the `parent()` method for cleaner Fluid CLI definitions
 - Dependency Updates
-  - Minimum JDK Version is now 11
+  - **BREAKING** - Minimum JDK Version is now 11
   - Apache Commons Collections upgraded to 4.4
   - Apache Commons Lang upgraded to 3.14.0
   - `jakarta.inject` and `airline-backcompat-javaxinject` were made `optional` so will no longer be pulled in

diff --git a/Migrating.md b/Migrating.md
@@ -14,11 +14,14 @@ this annotation was moved into a new sub-package `com.github.rvesse.airline.anno
 
 ## Inject Dependencies are now optional
 
-As noted in [Migrating to Airline 2.9](#migration-to-airline-29) Airline is moving away from usage of the `@Inject` 
+As noted in [Migrating to Airline 2.9](#migration-to-airline-29) Airline is moving away from usage of the `@Inject`
 annotation for composition in favour of its own `@AirlineModule` annotation.  As part of this move the `jakarta.
-inject-api` and `airline-backcompat-javaxinject` dependencies became `optional` in 3.0.0.  This means that if you have an
-existing Airline based application that is using `@Inject` annotations you must now provide the relevant dependency 
-yourself as you will not automatically pick it up as a transitive dependency of Airline.
+inject-api` and `airline-backcompat-javaxinject` dependencies became `optional` in `3.0.0`.  This means that if you have
+an existing Airline based application that is using `@Inject` annotations you **MUST** now provide the relevant
+dependency yourself as you will not automatically pick it up as a transitive dependency of Airline.
+
+You **MUST** also explicitly configure the `@Parser` configuration to use the old composition annotations if you still
+need to use these.
 
 # Migration to Airline 2.9
 

diff --git a/airline-core/pom.xml b/airline-core/pom.xml
@@ -17,7 +17,6 @@
   <properties>
     <license.header.path>${project.parent.basedir}</license.header.path>
     <coveralls.skip>true</coveralls.skip>
-    <moditect.moduleName>com.github.rvesse.airline</moditect.moduleName>
   </properties>
 
   <dependencies>

diff --git a/airline-core/src/main/java/com/github/rvesse/airline/annotations/help/Version.java b/airline-core/src/main/java/com/github/rvesse/airline/annotations/help/Version.java
@@ -23,6 +23,7 @@
 
 import com.github.rvesse.airline.parser.resources.ClasspathLocator;
 import com.github.rvesse.airline.parser.resources.FileLocator;
+import com.github.rvesse.airline.parser.resources.ModulePathLocator;
 import com.github.rvesse.airline.parser.resources.ResourceLocator;
 
 @Retention(java.lang.annotation.RetentionPolicy.RUNTIME)
@@ -127,6 +128,7 @@
      */
     Class<? extends ResourceLocator>[] sourceLocators() default {
             ClasspathLocator.class,
+            ModulePathLocator.class,
             FileLocator.class
     };
 }
diff --git a/airline-core/src/main/java/com/github/rvesse/airline/builder/ParserBuilder.java b/airline-core/src/main/java/com/github/rvesse/airline/builder/ParserBuilder.java
@@ -135,12 +135,9 @@ public ParserBuilder<C> withCompositionAnnotations(String... annotationClassName
      * </p>
      * <ul>
      * <li>{@code com.github.rvesse.airline.annotations.AirlineModule}</li>
-     * <li>{@code javax.inject.Inject}</li>
-     * <li>{@code jakarta.inject.Inject}</li>
-     * <li>{@code com.google.inject.Inject}</li>
      * </ul>
      * <p>
-     * <strong>NB:</strong> Future releases will reduce the default set to just
+     * <strong>NB:</strong> As of {@code 3.0.0) the default set was reduced to just
      * {@code com.github.rvesse.airline.annotations.AirlineModule} and require that users explicitly configure
      * additional annotation classes as they see fit.  If you are not currently using a dependency injection framework
      * that requires some form of {@code Inject} annotation we would recommend that you transition to using
@@ -151,9 +148,7 @@ public ParserBuilder<C> withCompositionAnnotations(String... annotationClassName
      * @since 2.9.0
      */
     public ParserBuilder<C> withDefaultCompositionAnnotations() {
-        return withCompositionAnnotations(AirlineModule.class.getCanonicalName(), MetadataLoader.JAVAX_INJECT_INJECT,
-                                          MetadataLoader.JAKARTA_INJECT_INJECT,
-                                          MetadataLoader.COM_GOOGLE_INJECT_INJECT);
+        return withCompositionAnnotations(AirlineModule.class.getCanonicalName());
     }
 
     /**
@@ -574,15 +569,15 @@ public ParserBuilder<C> withFlagNegationPrefix(String prefix) {
 
     /**
      * Gets the parent CLI builder (if any)
-     * 
+     *
      * @return Parent CLI builder
-     * @throws IllegalStateException
-     *             Thrown if there is no parent CLI builder
+     * @throws IllegalStateException Thrown if there is no parent CLI builder
      */
     public CliBuilder<C> parent() {
-        if (this.cliBuilder == null)
+        if (this.cliBuilder == null) {
             throw new IllegalStateException(
                     "This Parser Builder was not created via a CLI builder and so cannot call parent() to obtain a parent CLI builder");
+        }
         return this.cliBuilder;
     }
 

diff --git a/airline-core/src/main/java/com/github/rvesse/airline/model/MetadataLoader.java b/airline-core/src/main/java/com/github/rvesse/airline/model/MetadataLoader.java
@@ -57,6 +57,11 @@
  */
 public class MetadataLoader {
 
+    /**
+     * Constant for the {@link AirlineModule} annotation class
+     */
+    public static final String AIRLINE_MODULE = "com.github.rvesse.airline.annotations.AirlineModule";
+
     /**
      * Constant for the {@code javax.inject.Inject} annotation class
      */
@@ -574,14 +579,9 @@ public static SuggesterMetadata loadSuggester(Class<? extends Suggester> suggest
      * migrated into the {@code jakarta} namespace.  As of <strong>2.9.0</strong> Airline makes the choice of annotation
      * fully configurable via the parser configuration.  To avoid potential class loading issues these are specified as
      * string class names with the metadata loader dynamically loading the relevant annotation classes if they are
-     * present on the runtime classpath.  For backwards compatibility if this piece of configuration is not customised
-     * then we support the following annotations by default:
-     * </p>
-     * <ul>
-     *     <li>{@value JAVAX_INJECT_INJECT}</li>
-     *     <li>{@value JAKARTA_INJECT_INJECT}</li>
-     *     <li>{@value COM_GOOGLE_INJECT_INJECT}</li>
-     * </ul>
+     * present on the runtime classpath.  As of <strong>3.10.0</strong> we only look for our own {@link AirlineModule}
+     * annotation and any other composition annotation e.g. {@code jakarta.inject.Inject} <strong>MUST</strong> be
+     * explicitly configured.
      *
      * @param type         Class
      * @param parserConfig Parser Configuration

diff --git a/airline-core/src/main/java/com/github/rvesse/airline/parser/resources/ModulePathLocator.java b/airline-core/src/main/java/com/github/rvesse/airline/parser/resources/ModulePathLocator.java
@@ -0,0 +1,67 @@
+/**
+ * Copyright (C) 2010-16 the original author or authors.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package com.github.rvesse.airline.parser.resources;
+
+import java.io.IOException;
+import java.io.InputStream;
+
+public class ModulePathLocator implements ResourceLocator {
+    @Override
+    public InputStream open(String searchLocation, String filename) throws IOException {
+        if (searchLocation == null) {
+            return null;
+        }
+
+        // Strip off Classpath URI prefix if present
+        if (searchLocation.startsWith(ClasspathLocator.CLASSPATH_URI_PREFIX)) {
+            searchLocation = searchLocation.substring(ClasspathLocator.CLASSPATH_URI_PREFIX.length());
+        }
+
+        // Strip off leading / if present
+        // This is because when running on the Module Path the JVM translates the package name of the resource
+        // to its associated module by simply replacing / characters with . characters.  Thus, if we have a leading
+        // slash we end up with an invalid module name like .foo.bar instead of foo.bar as was intended.  And as a
+        // result we never correctly locate resources when running on the Module Path if we didn't do this.
+        if (searchLocation.startsWith("/")) {
+            searchLocation = searchLocation.length() > 1 ? searchLocation.substring(1) : "";
+        }
+
+        // Build the expected resource name
+        StringBuilder resourceName = new StringBuilder();
+        resourceName.append(searchLocation);
+        if (!searchLocation.endsWith("/") && searchLocation.length() > 0) {
+            resourceName.append("/");
+        }
+        resourceName.append(filename);
+
+        // Try to open the classpath resource
+        InputStream resourceStream = ClassLoader.getSystemResourceAsStream(resourceName.toString());
+        if (resourceStream != null) {
+            return resourceStream;
+        }
+
+        // If the search location is not a package then return that directly if
+        // it is a valid location
+        if (!searchLocation.endsWith("/")) {
+            InputStream locStream = ClassLoader.getSystemResourceAsStream(searchLocation);
+            if (locStream != null) {
+                return locStream;
+            }
+        }
+
+        return null;
+    }
+}
diff --git a/...e/src/main/java/com/github/rvesse/airline/restrictions/factories/RestrictionRegistry.java b/...e/src/main/java/com/github/rvesse/airline/restrictions/factories/RestrictionRegistry.java
@@ -16,10 +16,8 @@
 package com.github.rvesse.airline.restrictions.factories;
 
 import java.lang.annotation.Annotation;
-import java.util.HashMap;
-import java.util.Map;
-import java.util.ServiceLoader;
-import java.util.Set;
+import java.util.*;
+import java.util.function.Function;
 
 import com.github.rvesse.airline.restrictions.ArgumentsRestriction;
 import com.github.rvesse.airline.restrictions.GlobalRestriction;
@@ -30,46 +28,50 @@
  */
 public class RestrictionRegistry {
 
-    private static final Map<Class<? extends Annotation>, OptionRestrictionFactory> OPTION_RESTRICTION_FACTORIES = new HashMap<>();
-    private static final Map<Class<? extends Annotation>, ArgumentsRestrictionFactory> ARGUMENT_RESTRICTION_FACTORIES = new HashMap<>();
-    private static final Map<Class<? extends Annotation>, GlobalRestrictionFactory> GLOBAL_RESTRICTION_FACTORIES = new HashMap<>();
+    private static final Map<Class<? extends Annotation>, OptionRestrictionFactory> OPTION_RESTRICTION_FACTORIES =
+            new HashMap<>();
+    private static final Map<Class<? extends Annotation>, ArgumentsRestrictionFactory> ARGUMENT_RESTRICTION_FACTORIES =
+            new HashMap<>();
+    private static final Map<Class<? extends Annotation>, GlobalRestrictionFactory> GLOBAL_RESTRICTION_FACTORIES =
+            new HashMap<>();
 
     private static volatile boolean init = false;
 
     static {
         init();
     }
 
+    static <T> void loadRestrictions(Class<T> cls, Function<T, List<Class<? extends Annotation>>> annotationsSelector,
+                                     Map<Class<? extends Annotation>, T> registry) {
+        try {
+            ServiceLoader<T> factories = ServiceLoader.load(cls);
+            Iterator<T> iter = factories.iterator();
+            while (iter.hasNext()) {
+                T factory = iter.next();
+                for (Class<? extends Annotation> annotationClass : annotationsSelector.apply(factory)) {
+                    registry.put(annotationClass, factory);
+                }
+            }
+        } catch (Throwable e) {
+            System.err.println("Failed to load " + cls.getSimpleName() + ": " + e.getMessage());
+        }
+    }
+
     /**
-     * Initializes the base set of restrictions using the {@link ServiceLoader}
-     * mechanism
+     * Initializes the base set of restrictions using the {@link ServiceLoader} mechanism
      */
     static synchronized void init() {
-        if (init)
+        if (init) {
             return;
+        }
 
         // Use ServerLoader to obtain restrictions
-        ServiceLoader<OptionRestrictionFactory> optionRestrictionFactories = ServiceLoader
-                .load(OptionRestrictionFactory.class);
-        for (OptionRestrictionFactory factory : optionRestrictionFactories) {
-            for (Class<? extends Annotation> cls : factory.supportedOptionAnnotations()) {
-                OPTION_RESTRICTION_FACTORIES.put(cls, factory);
-            }
-        }
-        ServiceLoader<ArgumentsRestrictionFactory> argumentsRestrictionFactories = ServiceLoader
-                .load(ArgumentsRestrictionFactory.class);
-        for (ArgumentsRestrictionFactory factory : argumentsRestrictionFactories) {
-            for (Class<? extends Annotation> cls : factory.supportedArgumentsAnnotations()) {
-                ARGUMENT_RESTRICTION_FACTORIES.put(cls, factory);
-            }
-        }
-        ServiceLoader<GlobalRestrictionFactory> globalRestrictionFactories = ServiceLoader
-                .load(GlobalRestrictionFactory.class);
-        for (GlobalRestrictionFactory factory : globalRestrictionFactories) {
-            for (Class<? extends Annotation> cls : factory.supportedGlobalAnnotations()) {
-                GLOBAL_RESTRICTION_FACTORIES.put(cls, factory);
-            }
-        }
+        loadRestrictions(OptionRestrictionFactory.class, x -> x.supportedOptionAnnotations(),
+                         OPTION_RESTRICTION_FACTORIES);
+        loadRestrictions(ArgumentsRestrictionFactory.class, x -> x.supportedArgumentsAnnotations(),
+                         ARGUMENT_RESTRICTION_FACTORIES);
+        loadRestrictions(GlobalRestrictionFactory.class, x -> x.supportedGlobalAnnotations(),
+                         GLOBAL_RESTRICTION_FACTORIES);
 
         init = true;
     }
@@ -90,22 +92,25 @@ public static Set<Class<? extends Annotation>> getOptionRestrictionAnnotationCla
     }
 
     public static void addOptionRestriction(Class<? extends Annotation> cls, OptionRestrictionFactory factory) {
-        if (cls == null)
+        if (cls == null) {
             throw new NullPointerException("cls cannot be null");
+        }
         OPTION_RESTRICTION_FACTORIES.put(cls, factory);
     }
 
     public static <T extends Annotation> OptionRestriction getOptionRestriction(Class<? extends Annotation> cls,
-            T annotation) {
+                                                                                T annotation) {
         OptionRestrictionFactory factory = OPTION_RESTRICTION_FACTORIES.get(cls);
-        if (factory != null)
+        if (factory != null) {
             return factory.createOptionRestriction(annotation);
+        }
         return null;
     }
 
     public static void addArgumentsRestriction(Class<? extends Annotation> cls, ArgumentsRestrictionFactory factory) {
-        if (cls == null)
+        if (cls == null) {
             throw new NullPointerException("cls cannot be null");
+        }
         ARGUMENT_RESTRICTION_FACTORIES.put(cls, factory);
     }
 
@@ -114,10 +119,11 @@ public static Set<Class<? extends Annotation>> getArgumentsRestrictionAnnotation
     }
 
     public static <T extends Annotation> ArgumentsRestriction getArgumentsRestriction(Class<? extends Annotation> cls,
-            T annotation) {
+                                                                                      T annotation) {
         ArgumentsRestrictionFactory factory = ARGUMENT_RESTRICTION_FACTORIES.get(cls);
-        if (factory != null)
+        if (factory != null) {
             return factory.createArgumentsRestriction(annotation);
+        }
         return null;
     }
 
@@ -126,16 +132,18 @@ public static Set<Class<? extends Annotation>> getGlobalRestrictionAnnotationCla
     }
 
     public static void addGlobalRestriction(Class<? extends Annotation> cls, GlobalRestrictionFactory factory) {
-        if (cls == null)
+        if (cls == null) {
             throw new NullPointerException("cls cannot be null");
+        }
         GLOBAL_RESTRICTION_FACTORIES.put(cls, factory);
     }
 
     public static <T extends Annotation> GlobalRestriction getGlobalRestriction(Class<? extends Annotation> cls,
-            T annotation) {
+                                                                                T annotation) {
         GlobalRestrictionFactory factory = GLOBAL_RESTRICTION_FACTORIES.get(cls);
-        if (factory != null)
+        if (factory != null) {
             return factory.createGlobalRestriction(annotation);
+        }
         return null;
     }
 }
diff --git a/...e-core/src/main/moditect/module-info.java → airline-core/src/main/java/module-info.java b/...e-core/src/main/moditect/module-info.java → airline-core/src/main/java/module-info.java
@@ -20,10 +20,6 @@
   requires org.apache.commons.lang3;
   requires org.apache.commons.collections4;
 
-  // Optional dependencies that a user might choose to add
-  requires static java.inject;
-  requires static jakarta.inject;
-
   // Exported packages
   exports com.github.rvesse.airline;
   exports com.github.rvesse.airline.annotations;
@@ -72,6 +68,7 @@
       com.github.rvesse.airline.restrictions.factories.PathRestrictionFactory,
       com.github.rvesse.airline.restrictions.factories.PortRestrictionFactory,
       com.github.rvesse.airline.restrictions.factories.RangeRestrictionFactory,
+      com.github.rvesse.airline.restrictions.factories.RequireFromRestrictionFactory,
       com.github.rvesse.airline.restrictions.factories.SimpleRestrictionsFactory,
       com.github.rvesse.airline.restrictions.factories.StringRestrictionFactory;
 

diff --git a/airline-core/src/main/resources/META-INF/services/com.github.rvesse.airline.ChannelFactory b/airline-core/src/main/resources/META-INF/services/com.github.rvesse.airline.ChannelFactory
@@ -1 +1 @@
-com.github.rvesse.airline.SystemChannelFactory
+com.github.rvesse.airline.SystemChannelFactory
diff --git a/...es/META-INF/services/com.github.rvesse.airline.help.sections.factories.HelpSectionFactory b/...es/META-INF/services/com.github.rvesse.airline.help.sections.factories.HelpSectionFactory
@@ -1 +1 @@
-com.github.rvesse.airline.help.sections.factories.CommonSectionsFactory
+com.github.rvesse.airline.help.sections.factories.CommonSectionsFactory
diff --git a/...INF/services/com.github.rvesse.airline.restrictions.factories.ArgumentsRestrictionFactory b/...INF/services/com.github.rvesse.airline.restrictions.factories.ArgumentsRestrictionFactory
@@ -4,6 +4,6 @@ com.github.rvesse.airline.restrictions.factories.OccurrencesRestrictionFactory
 com.github.rvesse.airline.restrictions.factories.PathRestrictionFactory
 com.github.rvesse.airline.restrictions.factories.PortRestrictionFactory
 com.github.rvesse.airline.restrictions.factories.RangeRestrictionFactory
+com.github.rvesse.airline.restrictions.factories.RequireFromRestrictionFactory
 com.github.rvesse.airline.restrictions.factories.SimpleRestrictionsFactory
 com.github.rvesse.airline.restrictions.factories.StringRestrictionFactory
-com.github.rvesse.airline.restrictions.factories.RequireFromRestrictionFactory