Warn when profile ID matches a lifecycle phase name#11926
Conversation
Emit a validation warning when a profile ID matches a default Maven lifecycle phase (clean, compile, test, package, deploy, etc.). Using lifecycle phase names as profile IDs can lead to accidental phase execution when the name is misused on the command line. Closes apache#10310
gnodet
left a comment
There was a problem hiding this comment.
Claude Code on behalf of Guillaume Nodet
The intent is good -- warning users when a profile ID collides with a lifecycle phase name is a useful safeguard.
However, the hardcoded LIFECYCLE_PHASES set has two problems:
-
It will become stale. Maven 4 defines different phase names in
org.apache.maven.api.Lifecycle.Phase(e.g.all,each,build,sources,resources,ready,unit-test) which are missing from this list. Conversely, some of the names in the list (generate-sources,process-sources,process-resources,process-classes,generate-test-sources,process-test-sources,generate-test-resources,process-test-resources,process-test-classes,prepare-package,pre-integration-test,post-integration-test,site-deploy) are Maven 3 legacy aliases. Maintaining a manually-curated duplicate is error-prone. -
Constants already exist. The
Lifecycle.Phaseinterface inmaven-api-core(whichmaven-implalready depends on) defines all the canonical phase name constants. The set should be derived from those constants rather than duplicated as raw strings.
The LIFECYCLE_PHASES set should be built from the Lifecycle.Phase constants and the Lifecycle.CLEAN / Lifecycle.SITE lifecycle names, so it stays in sync automatically. You could also consider querying the lifecycle registry at validation time, but a constant set from the API is a simpler first step.
| "verify", | ||
| "install", | ||
| "deploy", | ||
| "site", |
There was a problem hiding this comment.
This hardcoded set duplicates phase names that are already defined as constants in org.apache.maven.api.Lifecycle.Phase (in maven-api-core, which is a dependency of this module). It also mixes Maven 3 legacy phase names (e.g. generate-sources, process-sources, process-classes, prepare-package, pre-integration-test, post-integration-test, site-deploy) with the standard ones, while missing Maven 4 phases like all, each, build, sources, resources, ready, and unit-test.
Please build this set from the existing Lifecycle.Phase constants instead of duplicating strings. For example:
| "site", | |
| private static final Set<String> LIFECYCLE_PHASES = Set.of( | |
| Lifecycle.Phase.ALL, | |
| Lifecycle.Phase.EACH, | |
| Lifecycle.Phase.BUILD, | |
| Lifecycle.Phase.INITIALIZE, | |
| Lifecycle.Phase.VALIDATE, | |
| Lifecycle.Phase.SOURCES, | |
| Lifecycle.Phase.RESOURCES, | |
| Lifecycle.Phase.COMPILE, | |
| Lifecycle.Phase.READY, | |
| Lifecycle.Phase.PACKAGE, | |
| Lifecycle.Phase.VERIFY, | |
| Lifecycle.Phase.UNIT_TEST, | |
| Lifecycle.Phase.TEST_SOURCES, | |
| Lifecycle.Phase.TEST_RESOURCES, | |
| Lifecycle.Phase.TEST_COMPILE, | |
| Lifecycle.Phase.TEST, | |
| Lifecycle.Phase.INTEGRATION_TEST, | |
| Lifecycle.Phase.INSTALL, | |
| Lifecycle.Phase.DEPLOY, | |
| Lifecycle.Phase.CLEAN, | |
| Lifecycle.SITE); |
You may also want to include the legacy Maven 3 aliases (via Lifecycle.aliases()) if the goal is to catch those as well, but at minimum the canonical names should come from the constants.
When a profile ID matches a default Maven lifecycle phase name (e.g. "test", "deploy", "integration-test"), users can accidentally trigger that phase. This adds a validation warning to alert about the potential confusion.
Includes a new test case with a POM containing both phase-matching and safe profile IDs.
Closes #10310