feat: Add nullness annotations on public API (Part 1) (#1009)

TimefoldAI · Oct 24, 2024 · 34dded5 · 34dded5
1 parent 1300d72
commit 34dded5
Show file tree

Hide file tree

Showing 403 changed files with 4,224 additions and 3,888 deletions.
diff --git a/benchmark/pom.xml b/benchmark/pom.xml
@@ -144,6 +144,12 @@
       <artifactId>mockito-core</artifactId>
       <scope>test</scope>
     </dependency>
+    <!-- standardized nullness annotations  -->
+    <dependency>
+      <groupId>org.jspecify</groupId>
+      <artifactId>jspecify</artifactId>
+      <scope>test</scope>
+    </dependency>
   </dependencies>
 
   <build>

diff --git a/benchmark/src/test/java/ai/timefold/solver/benchmark/api/PlannerBenchmarkFactoryTest.java b/benchmark/src/test/java/ai/timefold/solver/benchmark/api/PlannerBenchmarkFactoryTest.java
@@ -26,6 +26,7 @@
 import ai.timefold.solver.core.impl.testdata.domain.TestdataValue;
 import ai.timefold.solver.core.impl.testdata.util.PlannerTestUtils;
 
+import org.jspecify.annotations.NonNull;
 import org.junit.jupiter.api.BeforeAll;
 import org.junit.jupiter.api.Test;
 
@@ -321,7 +322,7 @@ void buildPlannerBenchmark() {
 
     public static class TestdataConstraintProvider implements ConstraintProvider {
         @Override
-        public Constraint[] defineConstraints(ConstraintFactory constraintFactory) {
+        public Constraint @NonNull [] defineConstraints(@NonNull ConstraintFactory constraintFactory) {
             return new Constraint[0];
         }
     }

diff --git a/build/build-parent/pom.xml b/build/build-parent/pom.xml
@@ -38,6 +38,7 @@
     <version.org.webjars.webjars-locator>0.52</version.org.webjars.webjars-locator>
     <version.org.webjars.bootstrap>5.2.3</version.org.webjars.bootstrap>
     <version.org.webjars.jquery>3.6.4</version.org.webjars.jquery>
+    <version.org.jspecify>1.0.0</version.org.jspecify>
 
     <!-- ************************************************************************ -->
     <!-- Plugins -->
@@ -134,6 +135,11 @@
         <artifactId>jfreechart</artifactId>
         <version>${version.org.jfree.jfreechart}</version>
       </dependency>
+      <dependency>
+        <groupId>org.jspecify</groupId>
+        <artifactId>jspecify</artifactId>
+        <version>${version.org.jspecify}</version>
+      </dependency>
 
       <!-- ASM -->
       <dependency>
@@ -184,6 +190,8 @@
         <version>${version.org.webjars.jquery}</version>
         <scope>runtime</scope>
       </dependency>
+
+
     </dependencies>
   </dependencyManagement>
 

diff --git a/core/pom.xml b/core/pom.xml
@@ -114,6 +114,11 @@
       <artifactId>mockito-junit-jupiter</artifactId>
       <scope>test</scope>
     </dependency>
+    <!-- standardized nullness annotations  -->
+    <dependency>
+      <groupId>org.jspecify</groupId>
+      <artifactId>jspecify</artifactId>
+    </dependency>
   </dependencies>
 
   <build>

diff --git a/core/src/build/revapi-differences.json b/core/src/build/revapi-differences.json
@@ -57,6 +57,29 @@
           "oldValue": "{\"terminationClass\", \"terminationCompositionStyle\", \"spentLimit\", \"millisecondsSpentLimit\", \"secondsSpentLimit\", \"minutesSpentLimit\", \"hoursSpentLimit\", \"daysSpentLimit\", \"unimprovedSpentLimit\", \"unimprovedMillisecondsSpentLimit\", \"unimprovedSecondsSpentLimit\", \"unimprovedMinutesSpentLimit\", \"unimprovedHoursSpentLimit\", \"unimprovedDaysSpentLimit\", \"unimprovedScoreDifferenceThreshold\", \"bestScoreLimit\", \"bestScoreFeasible\", \"stepCountLimit\", \"unimprovedStepCountLimit\", \"scoreCalculationCountLimit\", \"terminationConfigList\"}",
           "newValue": "{\"terminationClass\", \"terminationCompositionStyle\", \"spentLimit\", \"millisecondsSpentLimit\", \"secondsSpentLimit\", \"minutesSpentLimit\", \"hoursSpentLimit\", \"daysSpentLimit\", \"unimprovedSpentLimit\", \"unimprovedMillisecondsSpentLimit\", \"unimprovedSecondsSpentLimit\", \"unimprovedMinutesSpentLimit\", \"unimprovedHoursSpentLimit\", \"unimprovedDaysSpentLimit\", \"unimprovedScoreDifferenceThreshold\", \"bestScoreLimit\", \"bestScoreFeasible\", \"stepCountLimit\", \"unimprovedStepCountLimit\", \"scoreCalculationCountLimit\", \"moveCountLimit\", \"terminationConfigList\"}",
           "justification": "Add new termination configuration"
+        },
+        {
+          "ignore": true,
+          "code": "java.method.parameterTypeParameterChanged",
+          "old": "parameter <Temporal_ extends java.time.temporal.Temporal & java.lang.Comparable<? super Temporal_>> ai.timefold.solver.core.api.domain.valuerange.CountableValueRange<Temporal_> ai.timefold.solver.core.api.domain.valuerange.ValueRangeFactory::createTemporalValueRange(===Temporal_===, Temporal_, long, java.time.temporal.TemporalUnit)",
+          "new": "parameter <Temporal_ extends java.time.temporal.Temporal & java.lang.Comparable<? super Temporal_>> ai.timefold.solver.core.api.domain.valuerange.CountableValueRange<Temporal_> ai.timefold.solver.core.api.domain.valuerange.ValueRangeFactory::createTemporalValueRange(===Temporal_===, Temporal_, long, java.time.temporal.TemporalUnit)",
+          "parameterIndex": "0",
+          "justification": "False positive after addition of @NonNull annotation"
+        },
+        {
+          "ignore": true,
+          "code": "java.method.parameterTypeParameterChanged",
+          "old": "parameter <Temporal_ extends java.time.temporal.Temporal & java.lang.Comparable<? super Temporal_>> ai.timefold.solver.core.api.domain.valuerange.CountableValueRange<Temporal_> ai.timefold.solver.core.api.domain.valuerange.ValueRangeFactory::createTemporalValueRange(Temporal_, ===Temporal_===, long, java.time.temporal.TemporalUnit)",
+          "new": "parameter <Temporal_ extends java.time.temporal.Temporal & java.lang.Comparable<? super Temporal_>> ai.timefold.solver.core.api.domain.valuerange.CountableValueRange<Temporal_> ai.timefold.solver.core.api.domain.valuerange.ValueRangeFactory::createTemporalValueRange(Temporal_, ===Temporal_===, long, java.time.temporal.TemporalUnit)",
+          "parameterIndex": "1",
+          "justification": "False positive after addition of @NonNull annotation"
+        },
+        {
+          "ignore": true,
+          "code": "java.method.returnTypeTypeParametersChanged",
+          "old": "method Score_ ai.timefold.solver.core.api.score.constraint.ConstraintMatch<Score_ extends ai.timefold.solver.core.api.score.Score<Score_>>::getScore()",
+          "new": "method Score_ ai.timefold.solver.core.api.score.constraint.ConstraintMatch<Score_ extends ai.timefold.solver.core.api.score.Score<Score_>>::getScore()",
+          "justification": "False positive after addition of @NonNull annotation"
         }
       ]
     }

diff --git a/core/src/main/java/ai/timefold/solver/core/api/domain/entity/PinningFilter.java b/core/src/main/java/ai/timefold/solver/core/api/domain/entity/PinningFilter.java
@@ -2,6 +2,8 @@
 
 import ai.timefold.solver.core.api.domain.solution.PlanningSolution;
 
+import org.jspecify.annotations.NonNull;
+
 /**
  * Decides on accepting or discarding a {@link PlanningEntity}.
  * A pinned {@link PlanningEntity}'s planning variables are never changed.
@@ -13,9 +15,9 @@ public interface PinningFilter<Solution_, Entity_> {
 
     /**
      * @param solution working solution to which the entity belongs
-     * @param entity never null, a {@link PlanningEntity}
+     * @param entity a {@link PlanningEntity}
      * @return true if the entity it is pinned, false if the entity is movable.
      */
-    boolean accept(Solution_ solution, Entity_ entity);
+    boolean accept(@NonNull Solution_ solution, @NonNull Entity_ entity);
 
 }
diff --git a/.../src/main/java/ai/timefold/solver/core/api/domain/solution/ConstraintWeightOverrides.java b/.../src/main/java/ai/timefold/solver/core/api/domain/solution/ConstraintWeightOverrides.java
@@ -11,6 +11,9 @@
 import ai.timefold.solver.core.api.solver.change.ProblemChange;
 import ai.timefold.solver.core.impl.domain.solution.DefaultConstraintWeightOverrides;
 
+import org.jspecify.annotations.NonNull;
+import org.jspecify.annotations.Nullable;
+
 /**
  * Used to override constraint weights defined in Constraint Streams,
  * e.g., in {@link UniConstraintStream#penalize(Score)}.
@@ -46,16 +49,17 @@ static <Score_ extends Score<Score_>> ConstraintWeightOverrides<Score_> of(Map<S
     /**
      * Return a constraint weight for a particular constraint.
      *
-     * @param constraintName never null
      * @return null if the constraint name is not known
      */
-    Score_ getConstraintWeight(String constraintName);
+    @Nullable
+    Score_ getConstraintWeight(@NonNull String constraintName);
 
     /**
      * Returns all known constraints.
-     * 
+     *
      * @return All constraint names for which {@link #getConstraintWeight(String)} returns a non-null value.
      */
+    @NonNull
     Set<String> getKnownConstraintNames();
 
 }
diff --git a/core/src/main/java/ai/timefold/solver/core/api/domain/solution/PlanningSolution.java b/core/src/main/java/ai/timefold/solver/core/api/domain/solution/PlanningSolution.java
@@ -12,6 +12,8 @@
 import ai.timefold.solver.core.api.domain.solution.cloner.SolutionCloner;
 import ai.timefold.solver.core.api.score.stream.ConstraintProvider;
 
+import org.jspecify.annotations.NonNull;
+
 /**
  * Specifies that the class is a planning solution.
  * A solution represents a problem and a possible solution of that problem.
@@ -53,9 +55,8 @@
      * This feature is not supported under Quarkus.
      * When using Quarkus,
      * setting this to anything other than {@link AutoDiscoverMemberType#NONE} will result in a build-time exception.
-     *
-     * @return never null
      */
+    @NonNull
     AutoDiscoverMemberType autoDiscoverMemberType() default AutoDiscoverMemberType.NONE;
 
     /**
@@ -74,9 +75,9 @@ interface NullSolutionCloner extends SolutionCloner {
 
     /**
      * @deprecated When multi-threaded solving, ensure your domain classes use @{@link PlanningId} instead.
-     * @return never null
      */
     @Deprecated(forRemoval = true, since = "1.10.0")
+    @NonNull
     LookUpStrategyType lookUpStrategyType() default LookUpStrategyType.PLANNING_ID_OR_NONE;
 
 }
diff --git a/core/src/main/java/ai/timefold/solver/core/api/domain/solution/cloner/SolutionCloner.java b/core/src/main/java/ai/timefold/solver/core/api/domain/solution/cloner/SolutionCloner.java
@@ -2,6 +2,8 @@
 
 import ai.timefold.solver.core.api.domain.solution.PlanningSolution;
 
+import org.jspecify.annotations.NonNull;
+
 /**
  * Clones a {@link PlanningSolution} during planning.
  * Used to remember the state of a good {@link PlanningSolution} so it can be recalled at a later time
@@ -34,9 +36,10 @@ public interface SolutionCloner<Solution_> {
      * <p>
      * This method is thread-safe.
      *
-     * @param original never null, the original {@link PlanningSolution}
-     * @return never null, the cloned {@link PlanningSolution}
+     * @param original the original {@link PlanningSolution}
+     * @return the cloned {@link PlanningSolution}
      */
-    Solution_ cloneSolution(Solution_ original);
+    @NonNull
+    Solution_ cloneSolution(@NonNull Solution_ original);
 
 }
diff --git a/core/src/main/java/ai/timefold/solver/core/api/domain/valuerange/CountableValueRange.java b/core/src/main/java/ai/timefold/solver/core/api/domain/valuerange/CountableValueRange.java
@@ -4,6 +4,9 @@
 
 import ai.timefold.solver.core.api.domain.variable.PlanningVariable;
 
+import org.jspecify.annotations.NonNull;
+import org.jspecify.annotations.Nullable;
+
 /**
  * A {@link ValueRange} that is ending. Therefore, it has a discrete (as in non-continuous) range.
  *
@@ -27,13 +30,13 @@ public interface CountableValueRange<T> extends ValueRange<T> {
      * @param index always {@code <} {@link #getSize()}
      * @return sometimes null (if {@link PlanningVariable#allowsUnassigned()} is true)
      */
+    @Nullable
     T get(long index);
 
     /**
      * Select the elements in original (natural) order.
-     *
-     * @return never null
      */
+    @NonNull
     Iterator<T> createOriginalIterator();
 
 }
diff --git a/core/src/main/java/ai/timefold/solver/core/api/domain/valuerange/ValueRange.java b/core/src/main/java/ai/timefold/solver/core/api/domain/valuerange/ValueRange.java
@@ -8,6 +8,9 @@
 
 import ai.timefold.solver.core.api.domain.variable.PlanningVariable;
 
+import org.jspecify.annotations.NonNull;
+import org.jspecify.annotations.Nullable;
+
 /**
  * A ValueRange is a set of a values for a {@link PlanningVariable}.
  * These values might be stored in memory as a {@link Collection} (usually a {@link List} or {@link Set}),
@@ -34,17 +37,17 @@ public interface ValueRange<T> {
      * @param value sometimes null
      * @return true if the ValueRange contains that value
      */
-    boolean contains(T value);
+    boolean contains(@Nullable T value);
 
     /**
      * Select in random order, but without shuffling the elements.
      * Each element might be selected multiple times.
      * Scales well because it does not require caching.
      *
-     * @param workingRandom never null, the {@link Random} to use when any random number is needed,
+     * @param workingRandom the {@link Random} to use when any random number is needed,
      *        so runs are reproducible.
-     * @return never null
      */
-    Iterator<T> createRandomIterator(Random workingRandom);
+    @NonNull
+    Iterator<T> createRandomIterator(@NonNull Random workingRandom);
 
 }