Use actual annotations instead of comments. (google#169)

Dependency on android.support.annotations is declared as "provided" so it won't propagate to callers. Note that by using the real @GuardedBy, ErrorProne now picks up on places that a variable is accessed without a lock, so these had to be fixed as well.
jinwong001 · Apr 23, 2018 · a842400 · a842400
1 parent bdc8055
commit a842400
Show file tree

Hide file tree

Showing 9 changed files with 36 additions and 15 deletions.
diff --git a/build.gradle b/build.gradle
@@ -37,6 +37,10 @@ apply plugin: 'com.android.library'
 
 repositories {
     jcenter()
+    maven {
+        url 'https://maven.google.com/'
+        name 'Google'
+    }
 }
 
 group = 'com.android.volley'

diff --git a/rules.gradle b/rules.gradle
@@ -19,6 +19,10 @@ tasks.withType(JavaCompile) {
   options.compilerArgs << "-Werror"
 }
 
+dependencies {
+  provided "com.android.support:support-annotations:27.1.1"
+}
+
 // Check if the android plugin version supports unit testing.
 if (configurations.findByName("testCompile")) {
   dependencies {

diff --git a/src/main/java/com/android/volley/Request.java b/src/main/java/com/android/volley/Request.java
@@ -20,6 +20,8 @@
 import android.net.Uri;
 import android.os.Handler;
 import android.os.Looper;
+import android.support.annotation.CallSuper;
+import android.support.annotation.GuardedBy;
 import android.text.TextUtils;
 import com.android.volley.VolleyLog.MarkerLog;
 import java.io.UnsupportedEncodingException;
@@ -79,7 +81,7 @@ public interface Method {
     private final Object mLock = new Object();
 
     /** Listener interface for errors. */
-    // @GuardedBy("mLock")
+    @GuardedBy("mLock")
     private Response.ErrorListener mErrorListener;
 
     /** Sequence number of this request, used to enforce FIFO ordering. */
@@ -92,11 +94,11 @@ public interface Method {
     private boolean mShouldCache = true;
 
     /** Whether or not this request has been canceled. */
-    // @GuardedBy("mLock")
+    @GuardedBy("mLock")
     private boolean mCanceled = false;
 
     /** Whether or not a response has been delivered for this request yet. */
-    // @GuardedBy("mLock")
+    @GuardedBy("mLock")
     private boolean mResponseDelivered = false;
 
     /** Whether the request should be retried in the event of an HTTP 5xx (server) error. */
@@ -116,7 +118,7 @@ public interface Method {
     private Object mTag;
 
     /** Listener that will be notified when a response has been delivered. */
-    // @GuardedBy("mLock")
+    @GuardedBy("mLock")
     private NetworkRequestCompleteListener mRequestCompleteListener;
 
     /**
@@ -173,7 +175,9 @@ public Object getTag() {
 
     /** @return this request's {@link com.android.volley.Response.ErrorListener}. */
     public Response.ErrorListener getErrorListener() {
-        return mErrorListener;
+        synchronized (mLock) {
+            return mErrorListener;
+        }
     }
 
     /** @return A tag for use with {@link TrafficStats#setThreadStatsTag(int)} */
@@ -313,7 +317,7 @@ public Cache.Entry getCacheEntry() {
      *
      * <p>There are no guarantees if both of these conditions aren't met.
      */
-    // @CallSuper
+    @CallSuper
     public void cancel() {
         synchronized (mLock) {
             mCanceled = true;
@@ -649,7 +653,7 @@ public int compareTo(Request<T> other) {
     @Override
     public String toString() {
         String trafficStatsTag = "0x" + Integer.toHexString(getTrafficStatsTag());
-        return (mCanceled ? "[X] " : "[ ] ")
+        return (isCanceled() ? "[X] " : "[ ] ")
                 + getUrl()
                 + " "
                 + trafficStatsTag

diff --git a/src/main/java/com/android/volley/toolbox/AndroidAuthenticator.java b/src/main/java/com/android/volley/toolbox/AndroidAuthenticator.java
@@ -23,6 +23,7 @@
 import android.content.Context;
 import android.content.Intent;
 import android.os.Bundle;
+import android.support.annotation.VisibleForTesting;
 import com.android.volley.AuthFailureError;
 
 /**
@@ -61,7 +62,7 @@ public AndroidAuthenticator(
         this(AccountManager.get(context), account, authTokenType, notifyAuthFailure);
     }
 
-    // Visible for testing. Allows injection of a mock AccountManager.
+    @VisibleForTesting
     AndroidAuthenticator(
             AccountManager accountManager,
             Account account,

diff --git a/src/main/java/com/android/volley/toolbox/DiskBasedCache.java b/src/main/java/com/android/volley/toolbox/DiskBasedCache.java
@@ -17,6 +17,7 @@
 package com.android.volley.toolbox;
 
 import android.os.SystemClock;
+import android.support.annotation.VisibleForTesting;
 import android.text.TextUtils;
 import com.android.volley.Cache;
 import com.android.volley.Header;
@@ -482,7 +483,7 @@ boolean writeHeader(OutputStream os) {
         }
     }
 
-    // VisibleForTesting
+    @VisibleForTesting
     static class CountingInputStream extends FilterInputStream {
         private final long length;
         private long bytesRead;
@@ -510,7 +511,7 @@ public int read(byte[] buffer, int offset, int count) throws IOException {
             return result;
         }
 
-        // VisibleForTesting
+        @VisibleForTesting
         long bytesRead() {
             return bytesRead;
         }

diff --git a/src/main/java/com/android/volley/toolbox/HurlStack.java b/src/main/java/com/android/volley/toolbox/HurlStack.java
@@ -16,6 +16,7 @@
 
 package com.android.volley.toolbox;
 
+import android.support.annotation.VisibleForTesting;
 import com.android.volley.AuthFailureError;
 import com.android.volley.Header;
 import com.android.volley.Request;
@@ -106,7 +107,7 @@ public HttpResponse executeRequest(Request<?> request, Map<String, String> addit
                 inputStreamFromConnection(connection));
     }
 
-    // VisibleForTesting
+    @VisibleForTesting
     static List<Header> convertHeaders(Map<String, List<String>> responseHeaders) {
         List<Header> headerList = new ArrayList<>(responseHeaders.size());
         for (Map.Entry<String, List<String>> entry : responseHeaders.entrySet()) {

diff --git a/src/main/java/com/android/volley/toolbox/ImageRequest.java b/src/main/java/com/android/volley/toolbox/ImageRequest.java
@@ -19,6 +19,8 @@
 import android.graphics.Bitmap;
 import android.graphics.Bitmap.Config;
 import android.graphics.BitmapFactory;
+import android.support.annotation.GuardedBy;
+import android.support.annotation.VisibleForTesting;
 import android.widget.ImageView.ScaleType;
 import com.android.volley.DefaultRetryPolicy;
 import com.android.volley.NetworkResponse;
@@ -41,8 +43,9 @@ public class ImageRequest extends Request<Bitmap> {
     /** Lock to guard mListener as it is cleared on cancel() and read on delivery. */
     private final Object mLock = new Object();
 
-    // @GuardedBy("mLock")
+    @GuardedBy("mLock")
     private Response.Listener<Bitmap> mListener;
+
     private final Config mDecodeConfig;
     private final int mMaxWidth;
     private final int mMaxHeight;
@@ -262,7 +265,7 @@ protected void deliverResponse(Bitmap response) {
      * @param desiredWidth Desired width of the bitmap
      * @param desiredHeight Desired height of the bitmap
      */
-    // Visible for testing.
+    @VisibleForTesting
     static int findBestSampleSize(
             int actualWidth, int actualHeight, int desiredWidth, int desiredHeight) {
         double wr = (double) actualWidth / desiredWidth;

diff --git a/src/main/java/com/android/volley/toolbox/JsonRequest.java b/src/main/java/com/android/volley/toolbox/JsonRequest.java
@@ -16,6 +16,7 @@
 
 package com.android.volley.toolbox;
 
+import android.support.annotation.GuardedBy;
 import com.android.volley.NetworkResponse;
 import com.android.volley.Request;
 import com.android.volley.Response;
@@ -41,8 +42,9 @@ public abstract class JsonRequest<T> extends Request<T> {
     /** Lock to guard mListener as it is cleared on cancel() and read on delivery. */
     private final Object mLock = new Object();
 
-    // @GuardedBy("mLock")
+    @GuardedBy("mLock")
     private Listener<T> mListener;
+
     private final String mRequestBody;
 
     /**

diff --git a/src/main/java/com/android/volley/toolbox/StringRequest.java b/src/main/java/com/android/volley/toolbox/StringRequest.java
@@ -16,6 +16,7 @@
 
 package com.android.volley.toolbox;
 
+import android.support.annotation.GuardedBy;
 import com.android.volley.NetworkResponse;
 import com.android.volley.Request;
 import com.android.volley.Response;
@@ -29,7 +30,7 @@ public class StringRequest extends Request<String> {
     /** Lock to guard mListener as it is cleared on cancel() and read on delivery. */
     private final Object mLock = new Object();
 
-    // @GuardedBy("mLock")
+    @GuardedBy("mLock")
     private Listener<String> mListener;
 
     /**