square / retrofit

A type-safe HTTP client for Android and the JVM
https://square.github.io/retrofit/
Apache License 2.0
43.02k stars 7.3k forks source link

Unused custom annotations on method parameters should fail matching for default string converter. #2694

Open NightlyNexus opened 6 years ago

NightlyNexus commented 6 years ago

@GET("/") Call<ResponseBody> queryParameter(@Foo @Query("foo") Object foo); This method currently will work without a custom string converter. The built-in string converter will simply call foo.toString(), and @Foo will be silently ignored. (...apparently, my first GH issue on Retrofit.)

JakeWharton commented 6 years ago

The biggest problem here is @NonNull/@Nullable annotations.

NightlyNexus commented 6 years ago

yeah, awkward. My first thought was for Retrofit 3 to have explicit qualifier annotations for methods and parameters that are given in the factory, instead of all the annotations like Retrofit 2. That's more API overhead, though.

NightlyNexus commented 6 years ago

Does anybody inspect the retrofit2.http annotations in a Converter.Factory? I can't recall if there was a use case or if it was convenience.

JakeWharton commented 6 years ago

It was mostly laziness to not have to filter them for each parameter handler. Also the string converter is supposed to be a catch-all. It emulates old behavior before we added string converters where everything had toString() called on it. I think we document this somewhere.

On Thu, Mar 15, 2018, 6:52 PM Eric Cochran notifications@github.com wrote:

Does anybody inspect the retrofit2.http annotations in a Converter.Factory? I can't recall if there was a use case or if it was convenience.

— You are receiving this because you commented.

Reply to this email directly, view it on GitHub https://github.com/square/retrofit/issues/2694#issuecomment-373548134, or mute the thread https://github.com/notifications/unsubscribe-auth/AAEEEaxAj0O8e5hCXQ2X3KmbmoQGGkd-ks5tevCcgaJpZM4Ss-I0 .

JakeWharton commented 5 years ago

Moving this to an enhancement in 3.0 where we'll probably go the Dagger / Moshi route.