[JENKINS-43507] More documentation

src/main/java/jenkins/scm/api/trait/SCMBuilder.java

@@ -31,58 +31,161 @@
 import java.util.Collection;
 import jenkins.scm.api.SCMHead;
 import jenkins.scm.api.SCMRevision;
+import jenkins.scm.api.SCMSource;
 
+/**
+ * Builder for a {@link SCM} instance. Typically instantiated in {@link SCMSource#build(SCMHead, SCMRevision)} or
+ * {@link SCMSource#build(SCMHead)} and then decorated by {@link SCMSourceTrait#applyToBuilder(SCMBuilder)} before
+ * calling {@link #build()} to generate the return value. Conventions:
+ * <ul>
+ * <li>The builder is not designed to be shared by multiple threads.</li>
+ * <li>All methods should be either {@code final} or {@code abstract} unless there is a documented reason for
+ * allowing overrides</li>
+ * <li>All "setter" methods will return {@link B} and be called "withXxx"</li>
+ * <li>All "getter" methods will called "xxx()" callers should not assume that the returned value is resistent
+ * from concurrent changes but implementations should ensure that the returned value is immutable by the caller.
+ * In other words, it is intentional to reduce intermediate allocations by
+ * {@code return Collections.unmodifiableList(theList);} rather than the concurrency safe
+ * {@code return Collections.unmodifiableList(new ArrayList<>(theList));}
+ * </li>
+ * </ul>
+ *
+ * @param <B> the type of {@link SCMBuilder} so that subclasses can chain correctly in their {@link #withHead(SCMHead)}
+ *            etc methods.
+ * @param <S> the type of {@link SCM} returned by {@link #build()}
+ */
 public abstract class SCMBuilder<B extends SCMBuilder<B,S>,S extends SCM> {
 
+    /**
+     * The base class of {@link SCM} that will be produced by the {@link SCMBuilder}.
+     */
+    @NonNull
     private final Class<S> clazz;
+    /**
+     * The {@link SCMHead} to produce the {@link SCM} for.
+     */
+    @NonNull
     private SCMHead head;
+    /**
+     * The {@link SCMRevision} to produce the {@link SCM} for or {@code null} to produce the {@link SCM} for the head
+     * revision.
+     */
+    @CheckForNull
     private SCMRevision revision;
 
+    /**
+     * Constructor.
+     *
+     * @param clazz    The base class of {@link SCM} that will be produced by the {@link SCMBuilder}.
+     * @param head     The {@link SCMHead} to produce the {@link SCM} for.
+     * @param revision The {@link SCMRevision} to produce the {@link SCM} for or {@code null} to produce the
+     *                 {@link SCM} for the head revision.
+     */
     public SCMBuilder(Class<S> clazz, @NonNull SCMHead head, @CheckForNull SCMRevision revision) {
         this.clazz = clazz;
         this.head = head;
         this.revision = revision;
     }
 
-    public Class<S> scmClass() {
+    /**
+     * Returns the base class of {@link SCM} that will be produced by the {@link SCMBuilder}.
+     *
+     * @return the base class of {@link SCM} that will be produced by the {@link SCMBuilder}.
+     */
+    @NonNull
+    public final Class<S> scmClass() {
         return clazz;
     }
 
-    public SCMHead head() {
+    /**
+     * Returns the {@link SCMHead} to produce the {@link SCM} for.
+     *
+     * @return the {@link SCMHead} to produce the {@link SCM} for.
+     */
+    @NonNull
+    public final SCMHead head() {
         return head;
     }
 
-    public B withHead(@NonNull SCMHead head) {
+    /**
+     * Replace the {@link #head()} with a new {@link SCMHead}.
+     *
+     * @param head the {@link SCMHead} to produce the {@link SCM} for.
+     * @return {@code this} for method chaining.
+     */
+    @NonNull
+    public final B withHead(@NonNull SCMHead head) {
         this.head = head;
         return (B) this;
     }
 
-    public SCMRevision revision() {
+    /**
+     * Returns the {@link SCMRevision} to produce the {@link SCM} for or {@code null} to produce the {@link SCM} for
+     * the head revision.
+     *
+     * @return the {@link SCMRevision} to produce the {@link SCM} for or {@code null} to produce the {@link SCM} for
+     * the head revision.
+     */
+    @CheckForNull
+    public final SCMRevision revision() {
         return revision;
     }
 
-    public B withRevision(@CheckForNull SCMRevision revision) {
+    /**
+     * Replace the {@link #revision()}  with a new {@link SCMRevision}
+     *
+     * @param revision the {@link SCMRevision} to produce the {@link SCM} for or {@code null} to produce the
+     *                 {@link SCM} for the head revision.
+     * @return {@code this} for method chaining.
+     */
+    @NonNull
+    public final B withRevision(@CheckForNull SCMRevision revision) {
         this.revision = revision;
         return (B) this;
     }
 
-    public abstract S build();
-
+    /**
+     * Apply the {@link SCMSourceTrait} to this {@link SCMBuilder}.
+     * @param trait the {@link SCMSourceTrait}.
+     * @return {@code this} for method chaining.
+     */
     @SuppressWarnings("unchecked")
-    public B withTrait(@NonNull SCMSourceTrait trait) {
+    @NonNull
+    public final B withTrait(@NonNull SCMSourceTrait trait) {
         trait.applyToBuilder(this);
         return (B) this;
     }
 
-    public B withTraits(@NonNull SCMSourceTrait... traits) {
+    /**
+     * Apply the {@link SCMSourceTrait} instances to this {@link SCMBuilder}.
+     *
+     * @param traits the {@link SCMSourceTrait} instances.
+     * @return {@code this} for method chaining.
+     */
+    @NonNull
+    public final B withTraits(@NonNull SCMSourceTrait... traits) {
         return withTraits(Arrays.asList(traits));
     }
 
+    /**
+     * Apply the {@link SCMSourceTrait} instances to this {@link SCMBuilder}.
+     *
+     * @param traits the {@link SCMSourceTrait} instances.
+     * @return {@code this} for method chaining.
+     */
     @SuppressWarnings("unchecked")
-    public B withTraits(@NonNull Collection<SCMSourceTrait> traits) {
+    @NonNull
+    public final B withTraits(@NonNull Collection<SCMSourceTrait> traits) {
         for (SCMSourceTrait trait : traits) {
             withTrait(trait);
         }
         return (B) this;
     }
+
+    /**
+     * Instantiates the {@link SCM}.
+     * @return the {@link S} instance
+     */
+    @NonNull
+    public abstract S build();
 }

src/main/java/jenkins/scm/api/trait/SCMSourceBuilder.java

@@ -25,53 +25,143 @@
 package jenkins.scm.api.trait;
 
 import edu.umd.cs.findbugs.annotations.NonNull;
+import hudson.scm.SCM;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collection;
+import java.util.Collections;
 import java.util.List;
+import jenkins.scm.api.SCMNavigator;
 import jenkins.scm.api.SCMSource;
+import jenkins.scm.api.SCMSourceObserver;
 
-public abstract class SCMSourceBuilder<B extends SCMSourceBuilder<B,S>,S extends SCMSource> {
+/**
+ * Builder for a {@link SCMSource} instance. Typically instantiated in
+ * {@link SCMNavigator#visitSources(SCMSourceObserver)} and then decorated by
+ * {@link SCMNavigatorTrait#applyToBuilder(SCMSourceBuilder)} before calling {@link #build()} to generate the return
+ * value. Conventions:
+ * <ul>
+ * <li>The builder is not designed to be shared by multiple threads.</li>
+ * <li>All methods should be either {@code final} or {@code abstract} unless there is a documented reason for
+ * allowing overrides</li>
+ * <li>All "setter" methods will return {@link B} and be called "withXxx"</li>
+ * <li>All "getter" methods will called "xxx()" callers should not assume that the returned value is resistent
+ * from concurrent changes but implementations should ensure that the returned value is immutable by the caller.
+ * In other words, it is intentional to reduce intermediate allocations by
+ * {@code return Collections.unmodifiableList(theList);} rather than the concurrency safe
+ * {@code return Collections.unmodifiableList(new ArrayList<>(theList));}
+ * </li>
+ * </ul>
+ *
+ * @param <B> the type of {@link SCMSourceBuilder} so that subclasses can chain correctly in their
+ *            {@link #withTrait(SCMSourceTrait)}  etc methods.
+ * @param <S> the type of {@link SCMSource} returned by {@link #build()}. In general this should be a type that
+ *            has {@link SCMSourceTrait} support.
+ */
+public abstract class SCMSourceBuilder<B extends SCMSourceBuilder<B, S>, S extends SCMSource> {
 
+    /**
+     * The base class of {@link SCMSource} that will be produced by the {@link SCMSourceBuilder}.
+     */
+    @NonNull
     private final Class<S> clazz;
+    /**
+     * The project name.
+     */
+    @NonNull
     private final String projectName;
+    /**
+     * The {@link SCMSourceTrait} instances to provide to the {@link SCMSource} (assuming the {@link SCMSource} is
+     * {@link SCMSourceTrait} aware.
+     */
+    @NonNull
     private final List<SCMSourceTrait> traits = new ArrayList<SCMSourceTrait>();
 
+    /**
+     * Constructor.
+     * @param clazz the base class of {@link SCMSource} that will be produced by the {@link SCMSourceBuilder}.
+     * @param projectName the project name.
+     */
     public SCMSourceBuilder(Class<S> clazz, String projectName) {
         this.clazz = clazz;
         this.projectName = projectName;
     }
 
-    public Class<S> sourceClass() {
+    /**
+     * Returns the base class of {@link SCMSource} that will be produced by the {@link SCMSourceBuilder}.
+     * @return the base class of {@link SCMSource} that will be produced by the {@link SCMSourceBuilder}.
+     */
+    @NonNull
+    public final Class<S> sourceClass() {
         return clazz;
     }
 
-    public String projectName() {
+    /**
+     * Returns the project name.
+     * @return the project name.
+     */
+    @NonNull
+    public final String projectName() {
         return projectName;
     }
 
-    public abstract S build();
-
-    public List<SCMSourceTrait> traits() {
-        return new ArrayList<SCMSourceTrait>(traits);
+    /**
+     * Returns the {@link SCMSourceTrait} instances to provide to the {@link SCMSource} (assuming the {@link SCMSource}
+     * is {@link SCMSourceTrait} aware.
+     * @return the {@link SCMSourceTrait} instances to provide to the {@link SCMSource} (assuming the
+     * {@link SCMSource} is {@link SCMSourceTrait} aware.
+     */
+    @NonNull
+    public final List<SCMSourceTrait> traits() {
+        return Collections.unmodifiableList(traits);
     }
 
+
+    /**
+     * Apply the {@link SCMNavigatorTrait} to this {@link SCMSourceBuilder}.
+     *
+     * @param trait the {@link SCMNavigatorTrait}.
+     * @return {@code this} for method chaining.
+     */
     @SuppressWarnings("unchecked")
-    public B withTrait(@NonNull SCMNavigatorTrait trait) {
+    @NonNull
+    public final B withTrait(@NonNull SCMNavigatorTrait trait) {
         trait.applyToBuilder(this);
         return (B) this;
     }
 
+    /**
+     * Apply the {@link SCMSourceTrait} to this {@link SCMSourceBuilder} (that is add to {@link #traits()}).
+     *
+     * @param trait the {@link SCMSourceTrait}.
+     * @return {@code this} for method chaining.
+     */
     @SuppressWarnings("unchecked")
-    public B withTrait(@NonNull SCMSourceTrait trait) {
+    @NonNull
+    public final B withTrait(@NonNull SCMSourceTrait trait) {
         traits.add(trait);
         return (B) this;
     }
 
-    public B withTraits(@NonNull SCMTrait<? extends SCMTrait<?>>... traits) {
+    /**
+     * Apply the {@link SCMTrait} instances to this {@link SCMSourceBuilder}.
+     *
+     * @param traits the {@link SCMTrait} instances.
+     * @return {@code this} for method chaining.
+     */
+    @NonNull
+    public final B withTraits(@NonNull SCMTrait<? extends SCMTrait<?>>... traits) {
         return withTraits(Arrays.asList(traits));
     }
+
+    /**
+     * Apply the {@link SCMTrait} instances to this {@link SCMSourceBuilder}.
+     *
+     * @param traits the {@link SCMTrait} instances.
+     * @return {@code this} for method chaining.
+     */
     @SuppressWarnings("unchecked")
+    @NonNull
     public B withTraits(@NonNull Collection<? extends SCMTrait<?>> traits) {
         for (SCMTrait<?> trait : traits) {
             if (trait instanceof SCMNavigatorTrait) {
@@ -85,6 +175,13 @@ public B withTraits(@NonNull Collection<? extends SCMTrait<?>> traits) {
         return (B) this;
     }
 
+
+    /**
+     * Apply the {@link SCMNavigatorRequest} to this {@link SCMSourceBuilder}.
+     *
+     * @param request the {@link SCMNavigatorRequest} instance.
+     * @return {@code this} for method chaining.
+     */
     @SuppressWarnings("unchecked")
     public B withRequest(SCMNavigatorRequest request) {
         withTraits(request.traits());
@@ -93,4 +190,12 @@ public B withRequest(SCMNavigatorRequest request) {
         }
         return (B) this;
     }
+
+    /**
+     * Instantiates the {@link SCMSource}.
+     *
+     * @return the {@link S} instance
+     */
+    @NonNull
+    public abstract S build();
 }

src/test/java/jenkins/scm/impl/mock/MockSCMBuilder.java

@@ -40,6 +40,7 @@ public MockSCMBuilder(@NonNull MockSCMSource source, @NonNull SCMHead head,
         this.source = source;
     }
 
+    @NonNull
     @Override
     public MockSCM build() {
         SCMRevision revision = revision();

src/test/java/jenkins/scm/impl/mock/MockSCMSourceBuilder.java

@@ -24,6 +24,7 @@
 
 package jenkins.scm.impl.mock;
 
+import edu.umd.cs.findbugs.annotations.NonNull;
 import jenkins.scm.api.trait.SCMSourceBuilder;
 
 public class MockSCMSourceBuilder extends SCMSourceBuilder<MockSCMSourceBuilder, MockSCMSource> {
@@ -48,6 +49,7 @@ public MockSCMSourceBuilder(String id, String controllerId, String repository) {
         this.repository = repository;
     }
 
+    @NonNull
     @Override
     public MockSCMSource build() {
         return controller == null