[JENKINS-37566] - Annotate methods which may explicitly return null (…

…#168) * [JENKINS-37566] - Annotate methods which may explicitly return `null` Just a bulk change of annotations + some Javadoc, no behavior changes * [JENKINS-37566] - Annotate the MultiClassLoaderSerializer#readClassLoader() method * Explicitly Restrict MimicException to prevent external usages. Hence we do not care about the make() annotation, I’d gueess * Change MimicException#make() annotations to make @stephenc a :happypanda:
jenkinsci · Jun 27, 2017 · 5532667 · 5532667
1 parent 78bcc32
commit 5532667
Show file tree

Hide file tree

Showing 9 changed files with 46 additions and 3 deletions.
diff --git a/src/main/java/hudson/remoting/Channel.java b/src/main/java/hudson/remoting/Channel.java
@@ -1377,13 +1377,15 @@ public <T> T setProperty(ChannelProperty<T> key, T value) {
     /**
      * Gets the property set on the remote peer.
      *
-     * @return null
+     * @return {@code null}
      *      if the property of the said key isn't set.
      */
+    @CheckForNull
     public Object getRemoteProperty(Object key) {
         return remoteChannel.getProperty(key);
     }
 
+    @CheckForNull
     public <T> T getRemoteProperty(ChannelProperty<T> key) {
         return key.type.cast(getRemoteProperty((Object) key));
     }
@@ -1607,9 +1609,10 @@ public long getLastHeard() {
      * objects when they are transferred to the remote {@link Channel},
      * as well as during {@link Callable#call()} is invoked. 
      *
-     * @return null
+     * @return {@code null}
      *      if the calling thread is not performing serialization.
      */
+    @CheckForNull
     public static Channel current() {
         return CURRENT.get();
     }

diff --git a/src/main/java/hudson/remoting/CommandTransport.java b/src/main/java/hudson/remoting/CommandTransport.java
@@ -4,6 +4,7 @@
 import java.io.ObjectInputStream;
 import java.io.ObjectOutputStream;
 import java.io.OutputStream;
+import javax.annotation.CheckForNull;
 
 /**
  * Lower level abstraction under {@link Channel} for sending and receiving commands
@@ -179,6 +180,7 @@ static interface CommandReceiver {
      * This method is package private, to prevent new {@link CommandTransport} from
      * providing this feature.
      */
+    @CheckForNull
     OutputStream getUnderlyingStream() {
         return null;
     }

diff --git a/src/main/java/hudson/remoting/ExportedClassLoaderTable.java b/src/main/java/hudson/remoting/ExportedClassLoaderTable.java
@@ -27,6 +27,7 @@
 import java.util.HashMap;
 import java.util.Map;
 import java.util.WeakHashMap;
+import javax.annotation.CheckForNull;
 
 /**
  * Manages unique ID for classloaders.
@@ -54,6 +55,7 @@ public synchronized int intern(ClassLoader cl) {
         return id;
     }
 
+    @CheckForNull
     public synchronized ClassLoader get(int id) {
         WeakReference<ClassLoader> ref = table.get(id);
         if(ref==null)   return null;

diff --git a/src/main/java/hudson/remoting/MimicException.java b/src/main/java/hudson/remoting/MimicException.java
@@ -1,5 +1,11 @@
 package hudson.remoting;
 
+import javax.annotation.CheckForNull;
+import javax.annotation.Nonnull;
+import javax.annotation.Nullable;
+import org.kohsuke.accmod.Restricted;
+import org.kohsuke.accmod.restrictions.DoNotUse;
+
 /**
  * Exception that prints like the specified exception.
  *
@@ -11,6 +17,7 @@
  * @deprecated Use {@link ProxyException} instead.
  */
 @Deprecated
+@Restricted(DoNotUse.class)
 class MimicException extends Exception {
     private final String className;
     MimicException(Throwable cause) {
@@ -29,7 +36,8 @@ public String toString() {
         return (message != null) ? (s + ": " + message) : s;
     }
 
-    public static Throwable make(Channel ch, Throwable cause) {
+    @Nullable
+    public static Throwable make(@Nonnull Channel ch, @Nullable Throwable cause) {
         if (cause == null)  return null;
 
         // make sure the remoting layer of the other end supports this

diff --git a/src/main/java/hudson/remoting/MultiClassLoaderSerializer.java b/src/main/java/hudson/remoting/MultiClassLoaderSerializer.java
@@ -16,6 +16,7 @@
 import java.util.concurrent.ExecutionException;
 import java.util.logging.Level;
 import java.util.logging.Logger;
+import javax.annotation.CheckForNull;
 
 /**
  * {@link ObjectInputStream}/{@link ObjectOutputStream} pair that can handle object graph that spans across
@@ -87,6 +88,7 @@ static final class Input extends ObjectInputStream {
             this.channel = channel;
         }
 
+        @CheckForNull
         private ClassLoader readClassLoader() throws IOException, ClassNotFoundException {
             ClassLoader cl;
             int code = readInt();
@@ -122,6 +124,7 @@ protected Class<?> resolveClass(ObjectStreamClass desc) throws IOException, Clas
             channel.classFilter.check(name);
             try {
                 ClassLoader cl = readClassLoader();
+                // TODO: handle null classloader as a System one?
                 Class<?> c = Class.forName(name, false, cl);
                 channel.classFilter.check(c);
                 return c;
@@ -136,6 +139,7 @@ protected Class<?> resolveProxyClass(String[] interfaces) throws IOException, Cl
 
             Class[] classes = new Class[interfaces.length];
             for (int i = 0; i < interfaces.length; i++)
+                // TODO: handle null classloader as a System one?
                 classes[i] = Class.forName(interfaces[i], false, cl);
 
             /*

diff --git a/src/main/java/hudson/remoting/RemoteClassLoader.java b/src/main/java/hudson/remoting/RemoteClassLoader.java
@@ -402,6 +402,7 @@ private void definePackage(String name) {
         definePackage(packageName, null, null, null, null, null, null, null);
     }
 
+    @CheckForNull
     public URL findResource(String name) {
         // first attempt to load from locally fetched jars
         URL url = super.findResource(name);
@@ -894,6 +895,7 @@ public Map<String,ClassFile2> fetch3(String className) throws ClassNotFoundExcep
             return all;
         }
 
+        @CheckForNull
         private URL getResourceURL(String name) throws IOException {
             URL resource = cl.getResource(name);
            	if (resource == null) {
@@ -910,6 +912,7 @@ private URL getResourceURL(String name) throws IOException {
             return resource;
         }
 
+        @CheckForNull
         public ResourceFile getResource2(String name) throws IOException {
             URL resource = getResourceURL(name);
             if (resource == null) return null;
@@ -938,6 +941,7 @@ private ResourceFile makeResource(String name, URL resource) throws IOException
         @Override
         @SuppressFBWarnings(value = "PZLA_PREFER_ZERO_LENGTH_ARRAYS", 
                 justification = "Null return value is a part of the public interface")
+        @CheckForNull
         public byte[] getResource(String name) throws IOException {
         	URL resource = getResourceURL(name);
         	if (resource == null)   return null;

diff --git a/src/main/java/hudson/remoting/RemoteInvocationHandler.java b/src/main/java/hudson/remoting/RemoteInvocationHandler.java
@@ -51,6 +51,7 @@
 import java.util.logging.Logger;
 import javax.annotation.CheckForNull;
 import javax.annotation.Nonnull;
+import javax.annotation.Nullable;
 import org.jenkinsci.remoting.RoleChecker;
 
 /**
@@ -215,6 +216,7 @@ public static int unwrap(Object proxy, Channel src) {
      * If the given object is a proxy object, return the {@link Channel}
      * object that it's associated with. Otherwise null.
      */
+    @CheckForNull
     public static Channel unwrap(Object proxy) {
         InvocationHandler h = Proxy.getInvocationHandler(proxy);
         if (h instanceof RemoteInvocationHandler) {
@@ -224,6 +226,7 @@ public static Channel unwrap(Object proxy) {
         return null;
     }
 
+    @Nullable
     public Object invoke(Object proxy, Method method, Object[] args) throws Throwable {
         if(method.getDeclaringClass()==IReadResolve.class) {
             // readResolve on the proxy.

diff --git a/src/main/java/org/jenkinsci/remoting/engine/JnlpAgentEndpointResolver.java b/src/main/java/org/jenkinsci/remoting/engine/JnlpAgentEndpointResolver.java
@@ -134,6 +134,7 @@ public void setTunnel(String tunnel) {
         this.tunnel = tunnel;
     }
 
+    @CheckForNull
     public JnlpAgentEndpoint resolve() throws IOException {
         IOException firstError = null;
         for (String jenkinsUrl : jenkinsUrls) {

diff --git a/src/main/java/org/jenkinsci/remoting/nio/SelectableFileChannelFactory.java b/src/main/java/org/jenkinsci/remoting/nio/SelectableFileChannelFactory.java
@@ -22,6 +22,7 @@
 import java.nio.channels.spi.SelectorProvider;
 import java.util.logging.Level;
 import java.util.logging.Logger;
+import javax.annotation.CheckForNull;
 
 /**
  * Extracts out {@link SelectableChannel} from {@link InputStream} or {@link OutputStream}.
@@ -43,6 +44,8 @@
  * @since 2.38
  */
 public class SelectableFileChannelFactory {
+
+    @CheckForNull
     protected FileInputStream unwrap(InputStream i) {
         if (i instanceof BufferedInputStream) {
             try {
@@ -63,6 +66,7 @@ protected FileInputStream unwrap(InputStream i) {
         return null;    // unknown type
     }
 
+    @CheckForNull
     protected FileOutputStream unwrap(OutputStream i) {
         if (i instanceof BufferedOutputStream) {
             try {
@@ -83,24 +87,36 @@ protected FileOutputStream unwrap(OutputStream i) {
         return null;    // unknown type
     }
 
+    @CheckForNull
     public SocketChannel create(InputStream in) throws IOException {
         return create(unwrap(in));
     }
 
+    @CheckForNull
     public SocketChannel create(OutputStream out) throws IOException {
         return create(unwrap(out));
     }
 
+    @CheckForNull
     public SocketChannel create(FileInputStream in) throws IOException {
         if (in==null)   return null;
         return create(in.getFD());
     }
 
+    @CheckForNull
     public SocketChannel create(FileOutputStream out) throws IOException {
         if (out==null)   return null;
         return create(out.getFD());
     }
 
+    /**
+     * Create channel using the specified file descriptor.
+     * 
+     * @param fd File Descriptor
+     * @return {@code null} if the platform does not support it (e.g. Windows) OR the socket channel cannot be created.
+     *         In the latter case the error message will be printed to {@link  #LOGGER} then.
+     */
+    @CheckForNull
     public SocketChannel create(FileDescriptor fd) {
         if (File.pathSeparatorChar==';')
             return null; // not selectable on Windows