gnodet-bot commented on code in PR #13601:
URL: https://github.com/apache/maven/pull/13601#discussion_r4304856817
##########
impl/maven-core/src/main/java/org/apache/maven/project/DefaultProjectBuilder.java:
##########
@@ -476,9 +477,20 @@ void prepare() throws ProjectBuildingException {
: (parent
?
ModelBuilderRequest.RequestType.CONSUMER_PARENT
:
ModelBuilderRequest.RequestType.CONSUMER_DEPENDENCY);
+ Integer validationLevel = request.getValidationLevel();
+ if (validationLevel != null) {
+ if (validationLevel ==
ModelBuildingRequest.VALIDATION_LEVEL_STRICT) {
+ validationLevel =
ModelValidator.VALIDATION_LEVEL_STRICT;
+ } else if (validationLevel ==
ModelBuildingRequest.VALIDATION_LEVEL_MINIMAL) {
+ validationLevel =
ModelValidator.VALIDATION_LEVEL_MINIMAL;
+ } else {
+ validationLevel = null;
+ }
+ }
Review Comment:
⛔ **Broken null guard — autoboxing makes this dead code:**
`ProjectBuildingRequest.getValidationLevel()` returns a primitive `int`, not
`Integer`. The assignment `Integer validationLevel =
request.getValidationLevel()` autoboxes the primitive — the result is **never
null**. The `if (validationLevel != null)` check is always `true`, so:
1. The `else { validationLevel = null; }` branch is unreachable — unknown
compat levels (e.g. `VALIDATION_LEVEL_MAVEN_2_0 = 20`,
`VALIDATION_LEVEL_MAVEN_3_1 = 31`) will silently get passed as arbitrary
integers rather than falling back to the default.
2. More critically: the default
`DefaultProjectBuildingRequest.validationLevel` is `VALIDATION_LEVEL_STRICT =
30`, which maps to `ModelValidator.VALIDATION_LEVEL_STRICT = 42`. So even when
no explicit level is set by the caller,
`DefaultModelBuilder.getValidationLevel()` always receives `42` and never falls
back to its `isBuildRequest()` logic. Before this PR, `CONSUMER_PARENT` and
`CONSUMER_DEPENDENCY` requests received `VALIDATION_LEVEL_MINIMAL`; now they
receive `42` — this is a behavioral regression for all dependency and
parent-POM resolution.
Fix: use `int` directly and translate unconditionally (since there is no way
to distinguish "explicitly set" from "defaulted" for a primitive):
```suggestion
int rawLevel = request.getValidationLevel();
Integer validationLevel;
if (rawLevel ==
ModelBuildingRequest.VALIDATION_LEVEL_STRICT) {
validationLevel =
ModelValidator.VALIDATION_LEVEL_STRICT;
} else if (rawLevel ==
ModelBuildingRequest.VALIDATION_LEVEL_MINIMAL) {
validationLevel =
ModelValidator.VALIDATION_LEVEL_MINIMAL;
} else {
validationLevel = null;
}
```
--
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]
