gnodet-bot commented on code in PR #1022:
URL:
https://github.com/apache/maven-compiler-plugin/pull/1022#discussion_r4082173785
##########
src/main/java/org/apache/maven/plugin/compiler/Options.java:
##########
@@ -306,21 +306,22 @@ public boolean addMemoryValue(String option, String
label, String value, boolean
* If not, a warning is logged if {@code immediate} is {@code true}, or
stored in the
* {@link #warning} field if {@code immediate} is {@code false}.
*
- * <p>If a message is stored in {@link #warning}, then it will always end
with a dot.
- * This guarantee allows callers to delete the last character and replace
it by a coma
+ * <p>The message stored in {@link #warning} always ends with a dot.
+ * This guarantee allows callers to delete the last character and replace
it with a comma
* for continuing the sentence.</p>
*
* @param option the option to validate
* @param count the number of arguments that the caller wants to provide
* @param immediate whether to log immediately or to store the message in
{@link #warning}
- * @return whether the given option is supported and accepts the specified
number of arguments
+ * @return true if the given option is supported and accepts the specified
number of arguments,
+ * false otherwise
*/
private boolean checkNumberOfArguments(String option, int count, boolean
immediate) {
int expected = checker.isSupportedOption(option);
if (expected == count) {
warning = null;
return true;
- } else if (expected < 1) {
+ } else if (expected < 0) {
Review Comment:
💡 **The fix is correct**, but the pre-existing `expected == 0` branch was
dead code in the original — meaning this scenario was never tested.
`CompilerStub.isSupportedOption()` already returns `0` for `-Xlint`, so a test
covering that case (e.g. calling `addXlintX("-Xlint", "unsupported-arg")` or
similar) would prevent this from regressing silently.
The `javax.tools.OptionChecker` contract: `-1` = unsupported, `0` = flag (no
args), `≥1` = N args. With the old `< 1`, any option returning `0` fell into
the wrong branch and produced `"The option is not supported"` instead of `"The
option does not expect any argument."` — a misleading message. This fix is the
right correction.
--
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]
