Repository: square/retrofit
Issue: Check for @Url as the first parameter doesn't work on Kotlin
Labels: Bug

I set @Url default value in function with last param，like this:
```
     @GET
     fun batch(@HeaderMap header: Map<String, String>, @QueryMap param: Map<String, String>, @Url url: String = "http://xxx.xxx./xxx"): Flowable<Result>
```

and the log:
```
java.lang.NullPointerException: Attempt to invoke virtual method 'okhttp3.HttpUrl$Builder okhttp3.HttpUrl$Builder.addQueryParameter(java.lang.String, java.lang.String)' on a null object reference
        at retrofit2.RequestBuilder.addQueryParam(RequestBuilder.java:162)
        at retrofit2.ParameterHandler$QueryMap.apply(ParameterHandler.java:176)
        at retrofit2.ParameterHandler$QueryMap.apply(ParameterHandler.java:139)
        at retrofit2.ServiceMethod.toCall(ServiceMethod.java:110)
        at retrofit2.OkHttpCall.createRawCall(OkHttpCall.java:184)
        at retrofit2.OkHttpCall.execute(OkHttpCall.java:167)
        at retrofit2.adapter.rxjava2.CallExecuteObservable.subscribeActual(CallExecuteObservable.java:42)
        at io.reactivex.Observable.subscribe(Observable.java:12051)
        at retrofit2.adapter.rxjava2.BodyObservable.subscribeActual(BodyObservable.java:34)
        at io.reactivex.Observable.subscribe(Observable.java:12051)
        at io.reactivex.internal.operators.flowable.FlowableFromObservable.subscribeActual(FlowableFromObservable.java:29)
        at io.reactivex.Flowable.subscribe(Flowable.java:14349)
        at io.reactivex.internal.operators.flowable.FlowableOnBackpressureLatest.subscribeActual(FlowableOnBackpressureLatest.java:32)
        at io.reactivex.Flowable.subscribe(Flowable.java:14349)
        at io.reactivex.Flowable.subscribe(Flowable.java:14295)
        at io.reactivex.internal.operators.flowable.FlowableSubscribeOn$SubscribeOnSubscriber.run(FlowableSubscribeOn.java:82)
        at io.reactivex.internal.schedulers.ScheduledRunnable.run(ScheduledRunnable.java:66)
        at io.reactivex.internal.schedulers.ScheduledRunnable.call(ScheduledRunnable.java:57)
        at java.util.concurrent.FutureTask.run(FutureTask.java:266)
        at java.util.concurrent.ScheduledThreadPoolExecutor$ScheduledFutureTask.run(ScheduledThreadPoolExecutor.java:301)
        at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1162)
        at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:636)
        at java.lang.Thread.run(Thread.java:764)
```


 

Issue comments:
`@Url` must be first. We have a test which enforces this invariant. I'll look into why it doesn't work here.
Thanks, set @Url as the first parameter, it's working for me.

Pull request review comments:

Patch that resolved the issue:
diff --git a/retrofit/src/main/java/retrofit2/RequestFactory.java b/retrofit/src/main/java/retrofit2/RequestFactory.java
index c6098b2a93..ee7cdba415 100644
--- a/retrofit/src/main/java/retrofit2/RequestFactory.java
+++ b/retrofit/src/main/java/retrofit2/RequestFactory.java
@@ -127,6 +127,8 @@ static final class Builder {
     boolean gotBody;
     boolean gotPath;
     boolean gotQuery;
+    boolean gotQueryName;
+    boolean gotQueryMap;
     boolean gotUrl;
     String httpMethod;
     boolean hasBody;
@@ -322,7 +324,13 @@ private ParameterHandler<?> parseParameterAnnotation(
           throw parameterError(method, p, "@Path parameters may not be used with @Url.");
         }
         if (gotQuery) {
-          throw parameterError(method, p, "A @Url parameter must not come after a @Query");
+          throw parameterError(method, p, "A @Url parameter must not come after a @Query.");
+        }
+        if (gotQueryName) {
+          throw parameterError(method, p, "A @Url parameter must not come after a @QueryName.");
+        }
+        if (gotQueryMap) {
+          throw parameterError(method, p, "A @Url parameter must not come after a @QueryMap.");
         }
         if (relativeUrl != null) {
           throw parameterError(method, p, "@Url cannot be used with @%s URL", httpMethod);
@@ -344,6 +352,12 @@ private ParameterHandler<?> parseParameterAnnotation(
         if (gotQuery) {
           throw parameterError(method, p, "A @Path parameter must not come after a @Query.");
         }
+        if (gotQueryName) {
+          throw parameterError(method, p, "A @Path parameter must not come after a @QueryName.");
+        }
+        if (gotQueryMap) {
+          throw parameterError(method, p, "A @Path parameter must not come after a @QueryMap.");
+        }
         if (gotUrl) {
           throw parameterError(method, p, "@Path parameters may not be used with @Url.");
         }
@@ -395,7 +409,7 @@ private ParameterHandler<?> parseParameterAnnotation(
         boolean encoded = query.encoded();
 
         Class<?> rawParameterType = Utils.getRawType(type);
-        gotQuery = true;
+        gotQueryName = true;
         if (Iterable.class.isAssignableFrom(rawParameterType)) {
           if (!(type instanceof ParameterizedType)) {
             throw parameterError(method, p, rawParameterType.getSimpleName()
@@ -421,6 +435,7 @@ private ParameterHandler<?> parseParameterAnnotation(
 
       } else if (annotation instanceof QueryMap) {
         Class<?> rawParameterType = Utils.getRawType(type);
+        gotQueryMap = true;
         if (!Map.class.isAssignableFrom(rawParameterType)) {
           throw parameterError(method, p, "@QueryMap parameter type must be Map.");
         }


Test patch:
diff --git a/retrofit/src/test/java/retrofit2/RequestFactoryTest.java b/retrofit/src/test/java/retrofit2/RequestFactoryTest.java
index cfee28e19f..97d178677a 100644
--- a/retrofit/src/test/java/retrofit2/RequestFactoryTest.java
+++ b/retrofit/src/test/java/retrofit2/RequestFactoryTest.java
@@ -1005,6 +1005,40 @@ Call<ResponseBody> method(@Query("kit") String kit, @Path("ping") String ping) {
     }
   }
 
+  @Test public void getWithQueryNameThenPathThrows() {
+    class Example {
+      @GET("/foo/bar/{ping}/") //
+      Call<ResponseBody> method(@QueryName String kit, @Path("ping") String ping) {
+        throw new AssertionError();
+      }
+    }
+
+    try {
+      buildRequest(Example.class, "kat", "pong");
+      fail();
+    } catch (IllegalArgumentException e) {
+      assertThat(e).hasMessage("A @Path parameter must not come after a @QueryName. (parameter #2)\n"
+          + "    for method Example.method");
+    }
+  }
+
+  @Test public void getWithQueryMapThenPathThrows() {
+    class Example {
+      @GET("/foo/bar/{ping}/") //
+      Call<ResponseBody> method(@QueryMap Map<String, String> queries, @Path("ping") String ping) {
+        throw new AssertionError();
+      }
+    }
+
+    try {
+      buildRequest(Example.class, Collections.singletonMap("kit", "kat"), "pong");
+      fail();
+    } catch (IllegalArgumentException e) {
+      assertThat(e).hasMessage("A @Path parameter must not come after a @QueryMap. (parameter #2)\n"
+          + "    for method Example.method");
+    }
+  }
+
   @Test public void getWithPathAndQueryQuestionMarkParam() {
     class Example {
       @GET("/foo/bar/{ping}/") //
@@ -1444,7 +1478,41 @@ Call<ResponseBody> method(@Query("hey") String hey, @Url Object url) {
       buildRequest(Example.class, "hey", "foo/bar/");
       fail();
     } catch (IllegalArgumentException e) {
-      assertThat(e).hasMessage("A @Url parameter must not come after a @Query (parameter #2)\n"
+      assertThat(e).hasMessage("A @Url parameter must not come after a @Query. (parameter #2)\n"
+          + "    for method Example.method");
+    }
+  }
+
+  @Test public void getWithQueryNameThenUrlThrows() {
+    class Example {
+      @GET
+      Call<ResponseBody> method(@QueryName String name, @Url String url) {
+        throw new AssertionError();
+      }
+    }
+
+    try {
+      buildRequest(Example.class, Collections.singletonMap("kit", "kat"), "foo/bar/");
+      fail();
+    } catch (IllegalArgumentException e) {
+      assertThat(e).hasMessage("A @Url parameter must not come after a @QueryName. (parameter #2)\n"
+          + "    for method Example.method");
+    }
+  }
+
+  @Test public void getWithQueryMapThenUrlThrows() {
+    class Example {
+      @GET
+      Call<ResponseBody> method(@QueryMap Map<String, String> queries, @Url String url) {
+        throw new AssertionError();
+      }
+    }
+
+    try {
+      buildRequest(Example.class, Collections.singletonMap("kit", "kat"), "foo/bar/");
+      fail();
+    } catch (IllegalArgumentException e) {
+      assertThat(e).hasMessage("A @Url parameter must not come after a @QueryMap. (parameter #2)\n"
           + "    for method Example.method");
     }
   }