gnodet-bot commented on code in PR #13392:
URL: https://github.com/apache/maven/pull/13392#discussion_r4227842622
##########
impl/maven-core/src/main/java/org/apache/maven/lifecycle/internal/MojoExecutor.java:
##########
@@ -162,20 +165,38 @@ public void execute(final MavenSession session, final
List<MojoExecution> mojoEx
final PhaseRecorder phaseRecorder = new
PhaseRecorder(session.getCurrentProject());
+ // Resolve the mojo execution filter once per project execution.
+ final List<FilterPredicate> filterPredicates =
resolveFilterPredicates(session);
+
mojosExecutionStrategy.get().execute(mojoExecutions, session, new
MojoExecutionRunner() {
@Override
public void run(MojoExecution mojoExecution) throws
LifecycleExecutionException {
- MojoExecutor.this.execute(session, mojoExecution,
dependencyContext, phaseRecorder);
+ MojoExecutor.this.execute(session, mojoExecution,
dependencyContext, phaseRecorder, filterPredicates);
}
});
}
+ /**
+ * Resolves the mojo execution filter predicates for this build.
+ * Uses {@code -Dmaven.lifecycle.filter} user property; returns an empty
list if absent.
+ */
+ private List<FilterPredicate> resolveFilterPredicates(MavenSession
session) {
+ Properties userProps = session.getUserProperties();
+ String filterExpression = userProps != null ?
userProps.getProperty(MojoExecutionFilter.PROPERTY_NAME) : null;
+ return MojoExecutionFilter.parse(filterExpression);
+ }
+
private void execute(
MavenSession session,
MojoExecution mojoExecution,
DependencyContext dependencyContext,
- PhaseRecorder phaseRecorder)
+ PhaseRecorder phaseRecorder,
+ List<FilterPredicate> filterPredicates)
throws LifecycleExecutionException {
+ if (MojoExecutionFilter.matches(mojoExecution, filterPredicates)) {
+ eventCatapult.fire(ExecutionEvent.Type.MojoSkipped, session,
mojoExecution);
Review Comment:
⚠️ **Misleading event type:** `ExecutionEvent.Type.MojoSkipped` is already
handled by `ExecutionEventLogger.mojoSkipped()`, which logs:
> `"[plugin] requires online mode for execution but Maven is currently
offline, skipping"`
Firing this event for filter-skipped executions will print that message for
every filtered mojo — even when the build is online. The user sees a confusing
lie.
Consider either:
- Introducing a new `MojoFiltered` event type (or a sub-type)
- Or guarding `ExecutionEventLogger.mojoSkipped()` with a check on the skip
reason
At minimum, the existing `MojoSkipped` log message must be updated to not
imply offline mode when the skip was caused by the filter.
##########
impl/maven-core/src/main/java/org/apache/maven/lifecycle/internal/filter/MojoExecutionFilter.java:
##########
@@ -0,0 +1,113 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.maven.lifecycle.internal.filter;
+
+import java.util.ArrayList;
+import java.util.List;
+
+import org.apache.maven.api.Constants;
+import org.apache.maven.api.MojoExecution;
+import org.apache.maven.internal.impl.DefaultMojoExecution;
+
+/**
+ * Parses the {@code maven.lifecycle.filter} user property value into a list
of {@link FilterPredicate}s,
+ * and applies them at mojo execution time in {@code MojoExecutor}.
+ *
+ * <p>The property value is a comma-separated list of predicates, OR-ed
together:
+ * a mojo execution matching <em>any</em> predicate is skipped (a {@code
MojoSkipped} event is fired).
+ *
+ * <p>Supported predicate forms:
+ * <ul>
+ * <li>{@code *} — skip all mojo executions</li>
+ * <li>{@code :A} — skip by artifactId (e.g. {@code
:maven-enforcer-plugin})</li>
+ * <li>{@code G:A} — skip by groupId:artifactId</li>
+ * <li>{@code P} — skip by plugin prefix (e.g. {@code enforcer})</li>
+ * <li>{@code P:v:g} — skip by prefix:version:goal</li>
+ * <li>{@code P:v:g@e} — skip by prefix:version:goal@executionId</li>
+ * <li>{@code phase(name)} — skip all mojos bound to the named phase</li>
+ * </ul>
+ *
+ * @since 4.1.0
+ */
+public class MojoExecutionFilter {
+
+ /** The name of the user property that activates the filter. */
+ public static final String PROPERTY_NAME =
Constants.MAVEN_LIFECYCLE_FILTER;
+
+ private MojoExecutionFilter() {
+ // utility class
+ }
+
+ /**
+ * Parses a filter expression string into a list of {@link
FilterPredicate}s.
+ *
+ * @param expression the comma-separated filter expression (may be {@code
null} or blank)
+ * @return list of parsed predicates; empty if the expression is absent or
blank
+ */
+ public static List<FilterPredicate> parse(String expression) {
+ if (expression == null || expression.isBlank()) {
+ return List.of();
+ }
+ List<FilterPredicate> predicates = new ArrayList<>();
+ for (String token : expression.split(",")) {
+ token = token.strip();
+ if (token.isEmpty()) {
+ continue;
+ }
+ predicates.add(parseToken(token));
+ }
+ return List.copyOf(predicates);
+ }
+
+ private static FilterPredicate parseToken(String token) {
+ if (token.startsWith("phase(") && token.endsWith(")")) {
+ String phaseName = token.substring("phase(".length(),
token.length() - 1);
+ if (phaseName.isBlank()) {
+ throw new IllegalArgumentException("phase() predicate requires
a non-blank phase name");
+ }
Review Comment:
⚠️ **Silent mismatch on extra closing paren:** The detection
`startsWith("phase(") && endsWith(")")` accepts `phase(test))`. For that input,
`phaseName` is extracted as `test)`, which passes `isBlank()` but never matches
any phase — silently doing nothing instead of rejecting the typo.
Fix: use an exact regex or require that the content between the parens
contains no `)`:
```suggestion
private static FilterPredicate parseToken(String token) {
if (token.startsWith("phase(") && token.endsWith(")") &&
!token.substring("phase(".length(), token.length() - 1).contains(")")) {
String phaseName = token.substring("phase(".length(),
token.length() - 1);
if (phaseName.isBlank()) {
throw new IllegalArgumentException("phase() predicate
requires a non-blank phase name");
}
return new PhasePredicate(phaseName);
}
return CoordinatePredicate.parse(token);
}
```
Alternatively, reject any `token` not matching `phase\([^)]+\)` explicitly.
##########
impl/maven-core/src/main/java/org/apache/maven/lifecycle/internal/filter/MojoExecutionFilter.java:
##########
@@ -0,0 +1,113 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.maven.lifecycle.internal.filter;
+
+import java.util.ArrayList;
+import java.util.List;
+
+import org.apache.maven.api.Constants;
+import org.apache.maven.api.MojoExecution;
+import org.apache.maven.internal.impl.DefaultMojoExecution;
+
+/**
+ * Parses the {@code maven.lifecycle.filter} user property value into a list
of {@link FilterPredicate}s,
+ * and applies them at mojo execution time in {@code MojoExecutor}.
+ *
+ * <p>The property value is a comma-separated list of predicates, OR-ed
together:
+ * a mojo execution matching <em>any</em> predicate is skipped (a {@code
MojoSkipped} event is fired).
+ *
+ * <p>Supported predicate forms:
+ * <ul>
+ * <li>{@code *} — skip all mojo executions</li>
+ * <li>{@code :A} — skip by artifactId (e.g. {@code
:maven-enforcer-plugin})</li>
+ * <li>{@code G:A} — skip by groupId:artifactId</li>
+ * <li>{@code P} — skip by plugin prefix (e.g. {@code enforcer})</li>
+ * <li>{@code P:v:g} — skip by prefix:version:goal</li>
+ * <li>{@code P:v:g@e} — skip by prefix:version:goal@executionId</li>
+ * <li>{@code phase(name)} — skip all mojos bound to the named phase</li>
+ * </ul>
+ *
+ * @since 4.1.0
+ */
+public class MojoExecutionFilter {
+
+ /** The name of the user property that activates the filter. */
+ public static final String PROPERTY_NAME =
Constants.MAVEN_LIFECYCLE_FILTER;
+
+ private MojoExecutionFilter() {
+ // utility class
+ }
+
+ /**
+ * Parses a filter expression string into a list of {@link
FilterPredicate}s.
+ *
+ * @param expression the comma-separated filter expression (may be {@code
null} or blank)
+ * @return list of parsed predicates; empty if the expression is absent or
blank
+ */
+ public static List<FilterPredicate> parse(String expression) {
+ if (expression == null || expression.isBlank()) {
+ return List.of();
+ }
+ List<FilterPredicate> predicates = new ArrayList<>();
+ for (String token : expression.split(",")) {
+ token = token.strip();
+ if (token.isEmpty()) {
+ continue;
+ }
+ predicates.add(parseToken(token));
+ }
+ return List.copyOf(predicates);
+ }
+
+ private static FilterPredicate parseToken(String token) {
+ if (token.startsWith("phase(") && token.endsWith(")")) {
+ String phaseName = token.substring("phase(".length(),
token.length() - 1);
+ if (phaseName.isBlank()) {
+ throw new IllegalArgumentException("phase() predicate requires
a non-blank phase name");
+ }
+ return new PhasePredicate(phaseName);
+ }
+ return CoordinatePredicate.parse(token);
+ }
+
+ /**
+ * Returns {@code true} if the given legacy mojo execution matches any
predicate in the list
+ * and should be skipped.
+ *
+ * <p>Uses {@link DefaultMojoExecution} with a {@code null} session: only
string fields
+ * ({@link MojoExecution#getDescriptor()}, {@link
MojoExecution#getGoal()}, etc.) are accessed
+ * by the predicates.
+ *
+ * @param execution the legacy mojo execution to test
+ * @param predicates the predicates to apply (OR-ed)
+ * @return {@code true} if the execution should be skipped
+ */
+ public static boolean matches(org.apache.maven.plugin.MojoExecution
execution, List<FilterPredicate> predicates) {
+ if (predicates.isEmpty()) {
+ return false;
+ }
+ MojoExecution apiExecution = new DefaultMojoExecution(null, execution);
+ for (FilterPredicate predicate : predicates) {
Review Comment:
⚠️ **`IAE` from `parseToken()` propagates as a raw stack trace** —
`resolveFilterPredicates()` in `MojoExecutor` calls
`MojoExecutionFilter.parse()` with no error handling. An invalid expression
like `phase()` (empty phase name) will throw `IllegalArgumentException` and
surface as an unformatted stack dump rather than a clean Maven build error.
Wrap in a `LifecycleExecutionException` or `MavenExecutionException` in
`resolveFilterPredicates()`, citing the property name and invalid value:
```java
try {
return MojoExecutionFilter.parse(filterExpression);
} catch (IllegalArgumentException e) {
throw new LifecycleExecutionException(
"Invalid value for '" + MojoExecutionFilter.PROPERTY_NAME + "': " +
e.getMessage(), e);
}
```
Also note: `DefaultMojoExecution(null, execution)` passes a `null` session
explicitly. This works today because predicates only access string fields, but
a future predicate calling `execution.getSession()` will NPE without any
indication that null was passed intentionally. Consider documenting the
invariant or using a sentinel instead.
##########
impl/maven-core/src/main/java/org/apache/maven/lifecycle/internal/filter/CoordinatePredicate.java:
##########
@@ -0,0 +1,250 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.maven.lifecycle.internal.filter;
+
+import org.apache.maven.api.MojoExecution;
+import org.apache.maven.api.plugin.descriptor.PluginDescriptor;
+
+/**
+ * A {@link FilterPredicate} that matches {@link MojoExecution}s by plugin
coordinate or prefix.
+ *
+ * <h2>Syntax: {@code ([G[:A]]|P)[:v][:g[@e]]}</h2>
+ *
+ * <p>The {@code @} separator for execution ID is compatible with Maven's
existing
+ * {@code plugin:version:goal@executionId} notation used in
+ * {@code DefaultLifecycleExecutionPlanCalculator} for goal tasks.
+ *
+ * <p>Matching uses {@link MojoExecution#getDescriptor()} for goal-level
fields and
+ * {@link MojoExecution#getPlugin()} for plugin-level coordinates (groupId,
artifactId, version,
+ * goal prefix).
+ *
+ * <p>Forms:
+ * <ul>
+ * <li>{@code *} — matches every mojo execution</li>
+ * <li>{@code :A} — any groupId, specific artifactId (e.g. {@code
:maven-enforcer-plugin})</li>
+ * <li>{@code G:A} — exact groupId:artifactId (e.g. {@code
org.apache.maven.plugins:maven-enforcer-plugin}).
+ * The groupId <strong>must contain at least one {@code '.'}</strong>
for this form to be recognised;
+ * groupIds without a dot (e.g. {@code commons-io}) are routed to
prefix-based matching instead.
+ * Use the {@code :A} form to match by artifactId only when the groupId
has no dot.</li>
+ * <li>{@code P} — plugin prefix (e.g. {@code enforcer}), resolved against
the plugin descriptor's
+ * goal prefix.
+ * The prefix <strong>must not contain a {@code '.'}</strong>; prefixes
with a dot
+ * (e.g. {@code io.smallrye}) are routed to groupId:artifactId matching
instead.</li>
+ * <li>{@code P:v:g} — prefix + version + goal</li>
+ * <li>{@code P:v:g@e} — prefix + version + goal + executionId</li>
+ * </ul>
+ *
+ * <p>When any field is {@code null} or not specified, it is treated as a
wildcard (matches any value).
+ *
+ * @since 4.1.0
+ */
+public class CoordinatePredicate implements FilterPredicate {
+
+ /** Wildcard token — matches any value. */
+ private static final String ANY = null;
+
+ private final boolean matchAll;
+ private final String groupId; // null = any, non-null = exact match
+ private final String artifactId; // null = any, non-null = exact match
+ private final String prefix; // null = not used, non-null = match by goal
prefix
+ private final String version; // null = any
+ private final String goal; // null = any
+ private final String executionId; // null = any
+
+ /** Wildcard predicate — matches everything. */
+ public static final CoordinatePredicate MATCH_ALL = new
CoordinatePredicate();
+
+ private CoordinatePredicate() {
+ this.matchAll = true;
+ this.groupId = ANY;
+ this.artifactId = ANY;
+ this.prefix = ANY;
+ this.version = ANY;
+ this.goal = ANY;
+ this.executionId = ANY;
+ }
+
+ private CoordinatePredicate(
+ String groupId, String artifactId, String prefix, String version,
String goal, String executionId) {
+ this.matchAll = false;
+ this.groupId = groupId;
+ this.artifactId = artifactId;
+ this.prefix = prefix;
+ this.version = version;
+ this.goal = goal;
+ this.executionId = executionId;
+ }
+
+ /**
+ * Parses a coordinate predicate from a string token.
+ *
+ * <p>Supported forms:
+ * <ul>
+ * <li>{@code *} — match all</li>
+ * <li>{@code :A} — by artifactId only</li>
+ * <li>{@code G:A} — by groupId:artifactId. The groupId must contain at
least one {@code '.'} to be
+ * recognised as a G:A form; groupIds without a dot are treated as a
plugin prefix instead.</li>
+ * <li>{@code P} — by prefix</li>
+ * <li>{@code P:v:g} — by prefix + version + goal</li>
+ * <li>{@code P:v:g@e} — by prefix + version + goal + executionId</li>
+ * </ul>
+ *
+ * @param token the filter expression token (not {@code null}, not blank)
+ * @return the parsed predicate
+ */
+ public static CoordinatePredicate parse(String token) {
+ if ("*".equals(token)) {
+ return MATCH_ALL;
+ }
+
+ if (token.startsWith(":")) {
+ // :A form — any groupId, specific artifactId, optional :v:g[@e]
+ // e.g. ":maven-enforcer-plugin" or
":maven-enforcer-plugin:3.0.0:enforce@enforce-id"
+ String rest = token.substring(1); // remove leading ':'
+ String[] parts = rest.split(":", 3);
+ String artifactId = emptyToNull(parts[0]);
+ String version = parts.length > 1 ? emptyToNull(parts[1]) : null;
+ String goalAndExec = parts.length > 2 ? parts[2] : null;
+ String[] ge = splitGoalExecution(goalAndExec);
+ return new CoordinatePredicate(ANY, artifactId, ANY, version,
ge[0], ge[1]);
+ }
+
+ // Try to detect G:A form: the token contains ':' AND the part before
the first ':' looks like a
+ // groupId (contains a '.' suggesting it's a Java package name like
org.apache.maven).
+ // This distinguishes "org.apache.maven.plugins:maven-enforcer-plugin"
from "enforcer:3.1.0:enforce".
+ // Limitation: groupIds that do not contain a '.' (e.g.
"commons-io:commons-io") are misrouted
+ // to prefix-based matching. For such groupIds use the explicit ":A"
form for artifactId-only
+ // matching, or the full "G:A" form only when the groupId contains at
least one '.'.
+ int firstColon = token.indexOf(':');
+ if (firstColon > 0 && token.substring(0, firstColon).contains(".")) {
+ // G:A[:v[:g[@e]]] form
+ String[] parts = token.split(":", 4);
+ String groupId = emptyToNull(parts[0]);
+ String artifactId = parts.length > 1 ? emptyToNull(parts[1]) :
null;
+ String version = parts.length > 2 ? emptyToNull(parts[2]) : null;
+ String goalAndExec = parts.length > 3 ? parts[3] : null;
+ String[] ge = splitGoalExecution(goalAndExec);
+ return new CoordinatePredicate(groupId, artifactId, ANY, version,
ge[0], ge[1]);
+ }
+
+ // P[:v[:g[@e]]] form — prefix-based
+ String[] parts = token.split(":", 3);
+ String prefix = emptyToNull(parts[0]);
+ String version = parts.length > 1 ? emptyToNull(parts[1]) : null;
+ String goalAndExec = parts.length > 2 ? parts[2] : null;
+ String[] ge = splitGoalExecution(goalAndExec);
+ return new CoordinatePredicate(ANY, ANY, prefix, version, ge[0],
ge[1]);
+ }
+
+ /**
+ * Splits a {@code "goal"} or {@code "goal@executionId"} string into a
2-element array
+ * {@code [goal, executionId]}. Either element may be {@code null}.
+ */
+ private static String[] splitGoalExecution(String goalAndExec) {
+ if (goalAndExec == null || goalAndExec.isEmpty()) {
+ return new String[] {null, null};
+ }
+ int atIdx = goalAndExec.indexOf('@');
+ if (atIdx < 0) {
+ return new String[] {emptyToNull(goalAndExec), null};
+ }
+ return new String[] {emptyToNull(goalAndExec.substring(0, atIdx)),
emptyToNull(goalAndExec.substring(atIdx + 1))
+ };
+ }
+
+ private static String emptyToNull(String s) {
+ return (s == null || s.isEmpty()) ? null : s;
+ }
+
+ @Override
+ public boolean matches(MojoExecution execution) {
+ if (matchAll) {
+ return true;
+ }
+
+ PluginDescriptor descriptor =
+ execution.getPlugin() != null ?
execution.getPlugin().getDescriptor() : null;
+
+ // prefix-based matching
Review Comment:
💡 **Dead null guard:** `execution.getPlugin()` on `DefaultMojoExecution` is
`@Nonnull` — this null check is unreachable. The actual NPE risk is one level
deeper: `descriptor.getGoalPrefix()`, `descriptor.getArtifactId()`, etc. can
return `null` on a partially-initialised descriptor (though the surrounding
`descriptor != null` guard mitigates most of it).
Consider simplifying to:
```suggestion
PluginDescriptor descriptor = execution.getPlugin().getDescriptor();
```
This makes the invariant explicit and removes the dead branch.
##########
api/maven-api-core/src/main/java/org/apache/maven/api/Constants.java:
##########
@@ -868,5 +868,13 @@ public final class Constants {
@Config(type = "java.lang.Boolean", defaultValue = "false")
public static final String MAVEN_MODEL_DEPENDENCY_INTERPOLATION_FULL =
"maven.model.dependencyInterpolation.full";
+ /**
+ * Comma-separated list of mojo execution filter predicates. Matching
executions are skipped at runtime.
+ * Supported forms: {@code *}, {@code :A}, {@code G:A}, {@code P}, {@code
P:v:g}, {@code P:v:g@e},
Review Comment:
📝 **Missing `G:A:v:g[@e]` form from Javadoc:** The constant lists `{@code
G:A}` and `{@code P:v:g[@e]}` but not `{@code G:A:v:g[@e]}`, even though
`CoordinatePredicate` supports it (the G:A branch calls `split(":", 4)`,
parsing version and goal). Please add the extended form to the Javadoc:
```suggestion
* Supported forms: {@code *}, {@code :A}, {@code G:A}, {@code
G:A:v:g[@e]}, {@code P}, {@code P:v:g},
* {@code P:v:g@e}, {@code phase(name)}.
```
--
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]
