Skip to content

Commit cefb8c4

Browse files
committed
Add. hint message for missing @param value
- Added a helpful hint in the exception message when @param is missing a value and parameter name can't be resolved. - Added a unit test to cover this case. - The test is skipped if the code was compiled with the `-parameters` compiler flag. Files affected: - Contract.java - DefaultContractTest.java Fixes OpenFeign#2337
1 parent 99d859f commit cefb8c4

File tree

2 files changed

+48
-1
lines changed

2 files changed

+48
-1
lines changed

core/src/main/java/feign/Contract.java

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -327,7 +327,12 @@ public Default() {
327327
name = annotationName;
328328
}
329329
checkState(
330-
emptyToNull(name) != null, "Param annotation was empty on param %s.", paramIndex);
330+
emptyToNull(name) != null,
331+
"Param annotation was empty on param %s.\nHint: %s",
332+
paramIndex,
333+
"Prefer using @Param(value=\"name\"), or compile your code with the -parameters flag.\n"
334+
+ "If the value is missing, Feign attempts to retrieve the parameter name from bytecode, "
335+
+ "which only works if the class was compiled with the -parameters flag.");
331336
nameParam(data, name, paramIndex);
332337
final Class<? extends Param.Expander> expander = paramAnnotation.expander();
333338
if (expander != Param.ToStringExpander.class) {

core/src/test/java/feign/DefaultContractTest.java

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,8 @@
2626
import java.lang.annotation.Retention;
2727
import java.lang.annotation.RetentionPolicy;
2828
import java.lang.annotation.Target;
29+
import java.lang.reflect.Method;
30+
import java.lang.reflect.Parameter;
2931
import java.net.URI;
3032
import java.time.Clock;
3133
import java.time.Instant;
@@ -37,6 +39,7 @@
3739
import java.util.Map;
3840
import java.util.SortedMap;
3941
import org.assertj.core.api.Fail;
42+
import org.junit.jupiter.api.Assumptions;
4043
import org.junit.jupiter.api.Test;
4144

4245
/**
@@ -901,4 +904,43 @@ public Instant instant() {
901904
return Instant.ofEpochMilli(millis);
902905
}
903906
}
907+
908+
interface NoValueParamsInterface {
909+
@RequestLine("GET /getSomething?id={id}")
910+
String getSomething(@Param String id);
911+
}
912+
913+
@Test
914+
void errorMessageOnMissingParamNames() {
915+
Assumptions.assumeTrue(
916+
!isCompiledWithParameters(NoValueParamsInterface.class), "Skip if -parameters is enabled");
917+
918+
Throwable exception =
919+
assertThrows(
920+
IllegalStateException.class,
921+
() -> contract.parseAndValidateMetadata(NoValueParamsInterface.class));
922+
923+
assertThat(exception.getMessage())
924+
.contains("Param annotation was empty on param 0")
925+
.contains("Hint");
926+
}
927+
928+
/**
929+
* Checks whether the given class was compiled with the `-parameters` compiler flag. If parameter
930+
* names are preserved (i.e., not default like arg0, arg1...), we assume it was.
931+
*
932+
* @param clazz the class to inspect
933+
* @return true if `-parameters` was likely used, false otherwise
934+
*/
935+
private static boolean isCompiledWithParameters(Class<?> clazz) {
936+
for (Method method : clazz.getDeclaredMethods()) {
937+
for (Parameter parameter : method.getParameters()) {
938+
String paramName = parameter.getName();
939+
if (!paramName.matches("arg\\d+")) {
940+
return true;
941+
}
942+
}
943+
}
944+
return false;
945+
}
904946
}

0 commit comments

Comments
 (0)