gnodet-bot commented on code in PR #2220:
URL: https://github.com/apache/maven-resolver/pull/2220#discussion_r4337740312
##########
maven-resolver-util/src/main/java/org/eclipse/aether/util/ConfigUtils.java:
##########
@@ -352,7 +352,8 @@ public static List<?> getList(RepositorySystemSession
session, List<?> defaultVa
* @param enumClass the enum class to read, must not be {@code null}
* @param defaultValue the default value to return in case none of the
property keys is set to a convertible String or T.
* @param keys the property keys to read, must not be {@code null}. The
specified keys are read one after one until
- * a enum type {@code T} or a string (parsed using {@link
Enum#valueOf(Class, String)} is found.
+ * a enum type {@code T} or a string (parsed using {@link
Enum#valueOf(Class, String)} in a case-insensitive
+ * manner) is found.
Review Comment:
⚠️ **Javadoc cross-reference is no longer accurate.** The description says
"parsed using `{@link Enum#valueOf(Class, String)}`" but the implementation no
longer calls `Enum.valueOf` — it iterates `getEnumConstants()` with
`equalsIgnoreCase`. The `{@link}` implies the old behavior (case-sensitive,
throws IAE). Also missing: a note that an unrecognized string falls through to
the next key (and ultimately `defaultValue`).
```suggestion
* a enum type {@code T} or a string (compared
case-insensitively against enum constant names;
* if no constant matches, the key is skipped) is found.
```
##########
maven-resolver-tools/src/main/resources/configuration.md.vm:
##########
@@ -62,13 +62,13 @@ All properties which have `yes` in the column `Supports
Repo ID Suffix` can be o
If the value is not given in the target type the following conversions are
applied.
-From | To | With
+From | To | How
--- | --- | ---
`String` | `boolean` |
[`Boolean.parseBoolean(...)`](https://docs.oracle.com/javase/7/docs/api/java/lang/Boolean.html#parseBoolean(java.lang.String))
`String` | `int` |
[`Integer.parseInt(...)`](https://docs.oracle.com/javase/7/docs/api/java/lang/Integer.html#parseInt(java.lang.String))
`String` | `long` |
[`Long.parseLong(...)`](https://docs.oracle.com/javase/7/docs/api/java/lang/Long.html#parseLong(java.lang.String))
`String` | `float` |
[`Float.parseFloat(...)`](https://docs.oracle.com/javase/7/docs/api/java/lang/Float.html#parseFloat(java.lang.String))
-`String` | `T extends Enum<T>` |
[`Enum.valueOf(...)`](https://docs.oracle.com/javase/7/docs/api/java/lang/Enum.html#valueOf(java.lang.Class,%20java.lang.String))
+`String` | `T extends Enum<T>` | Each
[`Enum.name(...)`](https://docs.oracle.com/javase/7/docs/api/java/lang/Enum.html#name())
compared with `String.equalsIgnoreCase(...)` until a match is found. If no
match is found an `IllegalArgumentException` is thrown.
Review Comment:
⚠️ **Documentation contradicts implementation.** The docs say "If no match
is found an `IllegalArgumentException` is thrown" but the actual code silently
returns `defaultValue` when no enum constant matches the input string. Must be
corrected.
```suggestion
`String` | `T extends Enum<T>` | Each
[`Enum.name()`](https://docs.oracle.com/javase/7/docs/api/java/lang/Enum.html#name())
compared with `String.equalsIgnoreCase(...)` until a match is found. If no
match is found the default value is returned.
```
##########
maven-resolver-util/src/main/java/org/eclipse/aether/util/ConfigUtils.java:
##########
@@ -379,7 +385,8 @@ public static <T extends Enum<T>> T getEnum(
* @param enumClass the enum class to read, must not be {@code null}
* @param defaultValue the default value to return in case none of the
property keys is set to a convertible String or T.
* @param keys the property keys to read, must not be {@code null}. The
specified keys are read one after one until
- * a enum type {@code T} or a string (parsed using {@link
Enum#valueOf(Class, String)} is found.
+ * a enum type {@code T} or a string (parsed using {@link
Enum#valueOf(Class, String)} in a case-insensitive
Review Comment:
⚠️ **Same Javadoc inaccuracy on the session-based overload.** Same fix
needed: replace the `{@link Enum#valueOf}` reference and note the
case-insensitive, skip-on-no-match behavior.
```suggestion
* a enum type {@code T} or a string (compared
case-insensitively against enum constant names;
* if no constant matches, the key is skipped) is found.
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
