Merge pull request #6095 from HSLdevcom/null-annotation-conventions

Clarify @nullable and @nonnull usage [ci skip]

.gitignore

@@ -29,6 +29,7 @@ o_o_standalone_config_IncludeFileDirectiveTest_part.json
 .venv/
 _site/
 build/
+!/application/src/build/
 dist/
 doc/user/_build/
 gen-java/

application/pom.xml

@@ -513,6 +513,29 @@
                     </execution>
                 </executions>
             </plugin>
+            <plugin>
+                <groupId>com.github.bohnman</groupId>
+                <artifactId>package-info-maven-plugin</artifactId>
+                <version>1.1.0</version>
+                <configuration>
+                    <!-- Couldn't get this to work for main and ext code at the same time. -->
+                    <sourceDirectory>${project.basedir}/src/main/java</sourceDirectory>
+                    <outputDirectory>${project.basedir}/target/generated-sources</outputDirectory>
+                    <packages>
+                        <package>
+                            <pattern>**</pattern>
+                            <template>${project.basedir}/src/build/templates/package-info-template.java</template>
+                        </package>
+                    </packages>
+                </configuration>
+                <executions>
+                    <execution>
+                        <goals>
+                            <goal>generate</goal>
+                        </goals>
+                    </execution>
+                </executions>
+            </plugin>
         </plugins>
     </build>
 </project>

application/src/build/templates/package-info-template.java

@@ -0,0 +1,4 @@
+@ParametersAreNonnullByDefault
+package org.opentripplanner;
+
+import javax.annotation.ParametersAreNonnullByDefault;

application/src/ext/java/org/opentripplanner/ext/datastore/gs/package-info.md

@@ -0,0 +1,6 @@
+# Google cloud storage integration
+
+Add support for Google Cloud Storage, getting all input files and storing the graph.obj in the
+cloud.
+
+This implementation will use the existing OtpConfigLoader to load config from the local disk.

application/src/ext/java/org/opentripplanner/ext/restapi/resources/package-info.md

@@ -0,0 +1,4 @@
+# JAX-RS-annotated REST resource classes
+
+This package contains the JAX-RS-annotated REST resource classes for the OpenTripPlanner public
+API, i.e. the Jersey REST endpoints.

application/src/main/java/org/opentripplanner/api/parameter/package-info.md

@@ -0,0 +1,5 @@
+# HTTP query parameters
+
+This package contains classes which interpret incoming HTTP query parameters. Query parameters
+arrive as Strings, and Jersey will automatically call constructors with a single String
+argument.

application/src/main/java/org/opentripplanner/graph_builder/issues/package-info.md

@@ -0,0 +1,6 @@
+# Graph building issues
+
+Graph builder data import issues represent errors or exceptional conditions encountered during
+the graph building process. They contain descriptive messages and potentially references to the
+objects in the graph that they annotate which facilitate visualization and cataloging/mapping of
+problems.

application/src/main/java/org/opentripplanner/transit/model/package.md

@@ -23,10 +23,9 @@ All transit entities must have an ID. Transit entities ar "root" level are consi
 roots_.
 
 
-#### @Nonnull and @Nullable entity fields
+#### Non-null and nullable entity fields
 
-All fields getters(except primitive types) should be annotated with `@Nullable` or `@Nonnull`. None 
-null field should be enforced in the Entity constructor by using `Objects.requireNonNull`, 
+Non-nullability of fields should be enforced in the Entity constructor by using `Objects.requireNonNull`,
 `Objects.requireNonNullElse` or `ObjectUtils.ifNotNull`. We should enforce this for all fields 
 required in both GTFS and in the Nordic NeTEx Profile. For enumeration types using a special value
 like `UNKNOWN` is preferred over making the field optional.

application/src/main/java/org/opentripplanner/visualizer/package-info.md

@@ -0,0 +1,5 @@
+# Graph visualizer
+
+This package contains classes used for visualizing OpenTripPlanner graphs. This graph visualizer
+is intended for debugging purposes and may therefore have arcane developer-oriented features and
+grow new UI components as needed.

application/src/test/java/org/opentripplanner/mmri/package-info.md

@@ -0,0 +1,19 @@
+# MMRI ("MultiModale ReisInformatie" => "multimodal travel information")
+
+What is this package doing here?
+
+In 2013, significant improvements were made to OTP as part of a precommercial procurement project
+in The Netherlands called MMRI ("MultiModale ReisInformatie" => "multimodal travel information").
+This project is itself part of a larger project called "Better Benutten" => "better utilization".
+Most effort concentrated on the implementation of GTFS-RT updates and related improvements to the
+architecture of OTP. Additionally, a testing module was developed to verify that all the planners
+that were involved in the project (not just OTP) met a minimum set of requirements. OTP was first
+to pass all tests, ahead of two different solutions. Unfortunately, having two sets of tests does
+not make it simpler to continuously verify that OTP still functions correctly, which is why these
+MMRI tests have now been added to OTP's own test suite. These versions are intended to be a close
+approximation of reality, but several minor shortcuts have been taken, like applying trip updates
+directly to the graph instead of going through the thread-safe graph writer framework. Given that
+thread-safety is a technical issue and not a functional one, this is considered to be
+acceptable.
+
+The test cases are described [here](https://github.com/plannerstack/testset) and [here](./MMRI_Testdocument.pdf)

doc/dev/decisionrecords/Codestyle.md

@@ -157,6 +157,19 @@ context. Please avoid including trivial Javadoc or the empty Javadoc stubs added
     - Is it immutable or should anything be treated as immutable
     - Is it a utility class of static methods that should not be instantiated
 
+### Annotations
+
+- On methods:
+    - Method should be marked as `@Nullable` if they can return null values
+    - Method parameters should be marked as `@Nullable` if they can take null values.
+- On fields:
+    - Fields should be marked as `@Nullable` if they are nullable.
+
+Use of `@Nonnull` annotation is not allowed. It should be assumed methods/parameters/fields
+are non-null if they are not marked as `@Nullable`. However, there are places where the
+`@Nullable` annotation is missing even if it should have been used. Those can be updated
+to use the `@Nullable` annotation.
+
 ## JavaScript
 
 As of #206, we