gnodet-bot commented on code in PR #13392:
URL: https://github.com/apache/maven/pull/13392#discussion_r4228669190


##########
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 
G:A:v:g[@e]}, {@code P}, {@code P:v:g},
+     * {@code P:v:g@e}, {@code phase(name)}.
+     */
+    @Config
+    public static final String MAVEN_LIFECYCLE_FILTER = 
"maven.lifecycle.filter";

Review Comment:
   💡 **Missing `@since` tag**
   
   Every other constant and class added in this PR has a `@since 4.1.0` 
annotation (`CoordinatePredicate`, `FilterPredicate`, `MojoExecutionFilter`, 
`PhasePredicate`), but `MAVEN_LIFECYCLE_FILTER` is missing it.
   
   ```suggestion
        * {@code P:v:g@e}, {@code phase(name)}.
        *
        * @since 4.1.0
        */
       @Config
       public static final String MAVEN_LIFECYCLE_FILTER = 
"maven.lifecycle.filter";
   ```



##########
impl/maven-core/src/main/java/org/apache/maven/lifecycle/internal/filter/MojoExecutionFilter.java:
##########
@@ -0,0 +1,117 @@
+/*
+ * 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");
+            }
+            if (phaseName.contains(")")) {
+                throw new IllegalArgumentException(
+                        "Invalid phase() predicate '" + token + "': phase name 
must not contain ')'");
+            }
+            return new PhasePredicate(phaseName);
+        }
+        return CoordinatePredicate.parse(token);
+    }

Review Comment:
   ⚠️ **Silent misparse for malformed `phase(` tokens**
   
   The condition `token.startsWith("phase(") && token.endsWith(")")` only 
matches well-formed `phase(name)` tokens. A malformed input like `phase(test` 
(missing closing paren) passes the `startsWith` check but fails `endsWith`, so 
it silently falls through to `CoordinatePredicate.parse(token)`. There, 
`phase(test` has no `:` separator, so it is treated as a plugin prefix — 
becoming a `CoordinatePredicate` that matches on goal prefix `"phase(test"`. No 
real plugin has such a prefix, so the user's intended phase filter silently 
does nothing, with no error or warning.
   
   Consider throwing an `IllegalArgumentException` for tokens that start with 
`phase(` but don't match the full `phase(...)` form:
   
   ```suggestion
       private static FilterPredicate parseToken(String token) {
           if (token.startsWith("phase(")) {
               if (!token.endsWith(")")) {
                   throw new IllegalArgumentException(
                           "Invalid phase() predicate '" + token + "': missing 
closing ')'");
               }
               String phaseName = token.substring("phase(".length(), 
token.length() - 1);
               if (phaseName.isBlank()) {
                   throw new IllegalArgumentException("phase() predicate 
requires a non-blank phase name");
               }
               if (phaseName.contains(")")) {
                   throw new IllegalArgumentException(
                           "Invalid phase() predicate '" + token + "': phase 
name must not contain ')'");
               }
               return new PhasePredicate(phaseName);
           }
           return CoordinatePredicate.parse(token);
       }
   ```



-- 
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]

Reply via email to