Skip to content

[3.9.x] MavenPluginJavaPrerequisiteChecker: Handle 8/1.8 Java version in ranges as well #11577

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
cstamas merged 5 commits into from
Dec 19, 2025
Merged

[3.9.x] MavenPluginJavaPrerequisiteChecker: Handle 8/1.8 Java version in ranges as well #11577

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
fb7da96
Make up for users inconsistency
cstamas Dec 18, 2025
d0b3a12
More tests
cstamas Dec 18, 2025
c3ae642
More UTs
cstamas Dec 18, 2025
8c00268
Parameterized test
cstamas Dec 18, 2025
f8b23af
Revert "Parameterized test"
cstamas Dec 18, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion ...re/src/main/java/org/apache/maven/plugin/internal/MavenPluginJavaPrerequisiteChecker.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,9 +47,13 @@ public void accept(PluginDescriptor pluginDescriptor) {
}

boolean matchesVersion(String requiredVersion, String currentVersion) {
// Java 8 is consistent: it will always say "1.8...". Users are not consistent.
if (!requiredVersion.contains("1.8") && currentVersion.startsWith("1.8")) {
currentVersion = currentVersion.substring(2);
}
VersionConstraint constraint;
try {
constraint = versionScheme.parseVersionConstraint(requiredVersion.equals("8") ? "1.8" : requiredVersion);
Copy link
Contributor

@Bukama Bukama Dec 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

two lines above you substring currentVersion here it's about requiredVersion. Is it intended or are you accidently mixing variables?

constraint = versionScheme.parseVersionConstraint(requiredVersion);
} catch (InvalidVersionSpecificationException e) {
throw new IllegalArgumentException("Invalid 'requiredJavaVersion' given in plugin descriptor", e);
}
Expand Down
15 changes: 15 additions & 0 deletions ...rc/test/java/org/apache/maven/plugin/internal/MavenPluginJavaPrerequisiteCheckerTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,22 @@ public class MavenPluginJavaPrerequisiteCheckerTest {
public void testMatchesVersion() {
Copy link
Contributor

@Bukama Bukama Dec 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use Parameterized tests here

Copy link
Member Author

@cstamas cstamas Dec 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did the change as you asked, but now fails with checkstyle:

Error:  src/test/java/org/apache/maven/plugin/internal/MavenPluginJavaPrerequisiteCheckerTest.java:[67,19] (design) VisibilityModifier: Variable 'requiredVersion' must be private and have accessor methods.
Error:  src/test/java/org/apache/maven/plugin/internal/MavenPluginJavaPrerequisiteCheckerTest.java:[70,19] (design) VisibilityModifier: Variable 'currentVersion' must be private and have accessor methods.
Error:  src/test/java/org/apache/maven/plugin/internal/MavenPluginJavaPrerequisiteCheckerTest.java:[73,20] (design) VisibilityModifier: Variable 'expected' must be private and have accessor methods.

Copy link
Member Author

@cstamas cstamas Dec 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Clearly the two are not aligned: as if I make field private or just non-public, I get

Cannot set parameter 'requiredVersion'. Ensure that the field 'requiredVersion' is public.

So one want this, other want that.

Copy link
Member

@slawekjaranowski slawekjaranowski Dec 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do it simple, you can try to add suppress for checkstyle, by the way I'm working on move test to JUnit 5 in #11547

MavenPluginJavaPrerequisiteChecker checker = new MavenPluginJavaPrerequisiteChecker();
assertTrue(checker.matchesVersion("8", "1.8"));
assertTrue(checker.matchesVersion("[8,)", "1.8"));
assertTrue(checker.matchesVersion("(,8]", "1.8"));
assertFalse(checker.matchesVersion("(,8)", "1.8"));
assertTrue(checker.matchesVersion("8", "9.0.1+11"));
assertTrue(checker.matchesVersion("[8,)", "9.0.1+11"));
assertFalse(checker.matchesVersion("(,8]", "9.0.1+11"));
assertFalse(checker.matchesVersion("(,8)", "9.0.1+11"));
assertTrue(checker.matchesVersion("1.8", "1.8"));
assertTrue(checker.matchesVersion("[1.8,)", "1.8"));
assertTrue(checker.matchesVersion("(,1.8]", "1.8"));
assertFalse(checker.matchesVersion("(,1.8)", "1.8"));
assertTrue(checker.matchesVersion("1.8", "9.0.1+11"));
assertTrue(checker.matchesVersion("[1.8,)", "9.0.1+11"));
assertFalse(checker.matchesVersion("(,1.8]", "9.0.1+11"));
assertFalse(checker.matchesVersion("(,1.8)", "9.0.1+11"));

assertTrue(checker.matchesVersion("1.0", "1.8"));
assertTrue(checker.matchesVersion("1.8", "9.0.1+11"));
assertFalse(checker.matchesVersion("[1.0,2],[3,4]", "2.1"));
Expand Down
Loading