diff --git a/maven-model-builder/src/main/java/org/apache/maven/model/validation/DefaultModelValidator.java b/maven-model-builder/src/main/java/org/apache/maven/model/validation/DefaultModelValidator.java index 47a92ab967e0..dc544d455cc9 100644 --- a/maven-model-builder/src/main/java/org/apache/maven/model/validation/DefaultModelValidator.java +++ b/maven-model-builder/src/main/java/org/apache/maven/model/validation/DefaultModelValidator.java @@ -24,17 +24,24 @@ import java.io.File; import java.util.Arrays; +import java.util.Deque; import java.util.HashMap; import java.util.HashSet; +import java.util.Iterator; +import java.util.LinkedList; import java.util.List; import java.util.Map; import java.util.Objects; +import java.util.Optional; import java.util.Set; +import java.util.function.Consumer; +import java.util.function.Supplier; import java.util.regex.Matcher; import java.util.regex.Pattern; +import java.util.stream.Collectors; +import java.util.stream.StreamSupport; import org.apache.maven.model.Activation; -import org.apache.maven.model.ActivationFile; import org.apache.maven.model.Build; import org.apache.maven.model.BuildBase; import org.apache.maven.model.Dependency; @@ -236,42 +243,97 @@ public void validateRawModel(Model m, ModelBuildingRequest request, ModelProblem } private void validate30RawProfileActivation(ModelProblemCollector problems, Activation activation, String prefix) { - if (activation == null || activation.getFile() == null) { + if (activation == null) { return; } + class ActivationFrame { + String location; + Optional parent; - ActivationFile file = activation.getFile(); - - String path; - String location; - - if (file.getExists() != null && !file.getExists().isEmpty()) { - path = file.getExists(); - location = "exists"; - } else if (file.getMissing() != null && !file.getMissing().isEmpty()) { - path = file.getMissing(); - location = "missing"; - } else { - return; + ActivationFrame(String location, Optional parent) { + this.location = location; + this.parent = parent; + } } - - if (hasProjectExpression(path)) { - Matcher matcher = EXPRESSION_PROJECT_NAME_PATTERN.matcher(path); - while (matcher.find()) { - String propertyName = matcher.group(0); - if (!"${project.basedir}".equals(propertyName)) { + final Deque stk = new LinkedList<>(); + + final Supplier pathSupplier = () -> { + final boolean parallel = false; + return StreamSupport.stream(((Iterable) stk::descendingIterator).spliterator(), parallel) + .map(f -> f.location) + .collect(Collectors.joining(".")); + }; + final Supplier locationSupplier = () -> { + if (stk.size() < 2) { + return null; + } + Iterator f = stk.iterator(); + + String location = f.next().location; + ActivationFrame parent = f.next(); + + return parent.parent.map(p -> p.getLocation(location)).orElse(null); + }; + final Consumer validator = s -> { + if (hasProjectExpression(s)) { + String path = pathSupplier.get(); + Matcher matcher = EXPRESSION_PROJECT_NAME_PATTERN.matcher(s); + while (matcher.find()) { + String propertyName = matcher.group(0); + + if (path.startsWith("activation.file.") && "${project.basedir}".equals(propertyName)) { + continue; + } addViolation( problems, Severity.WARNING, Version.V30, - prefix + "activation.file." + location, + prefix + path, null, - "Failed to interpolate file location " + path + ": " + propertyName + "Failed to interpolate profile activation property " + s + ": " + propertyName + " expressions are not supported during profile activation.", - file.getLocation(location)); + locationSupplier.get()); } } - } + }; + Optional root = Optional.of(activation); + stk.push(new ActivationFrame("activation", root)); + root.map(Activation::getFile).ifPresent(fa -> { + stk.push(new ActivationFrame("file", Optional.of(fa))); + stk.push(new ActivationFrame("exists", Optional.empty())); + validator.accept(fa.getExists()); + stk.peek().location = "missing"; + validator.accept(fa.getMissing()); + stk.pop(); + stk.pop(); + }); + root.map(Activation::getOs).ifPresent(oa -> { + stk.push(new ActivationFrame("os", Optional.of(oa))); + stk.push(new ActivationFrame("arch", Optional.empty())); + validator.accept(oa.getArch()); + stk.peek().location = "family"; + validator.accept(oa.getFamily()); + stk.peek().location = "name"; + validator.accept(oa.getName()); + stk.peek().location = "version"; + validator.accept(oa.getVersion()); + stk.pop(); + stk.pop(); + }); + root.map(Activation::getProperty).ifPresent(pa -> { + stk.push(new ActivationFrame("property", Optional.of(pa))); + stk.push(new ActivationFrame("name", Optional.empty())); + validator.accept(pa.getName()); + stk.peek().location = "value"; + validator.accept(pa.getValue()); + stk.pop(); + stk.pop(); + }); + root.map(Activation::getJdk).ifPresent(jdk -> { + stk.push(new ActivationFrame("jdk", Optional.empty())); + validator.accept(jdk); + stk.pop(); + }); } private void validate20RawPlugins( diff --git a/maven-model-builder/src/test/java/org/apache/maven/model/validation/DefaultModelValidatorTest.java b/maven-model-builder/src/test/java/org/apache/maven/model/validation/DefaultModelValidatorTest.java index 37d7546edc7d..b839ee3fd1c0 100644 --- a/maven-model-builder/src/test/java/org/apache/maven/model/validation/DefaultModelValidatorTest.java +++ b/maven-model-builder/src/test/java/org/apache/maven/model/validation/DefaultModelValidatorTest.java @@ -20,6 +20,7 @@ import java.io.InputStream; import java.util.List; +import java.util.Properties; import java.util.function.UnaryOperator; import junit.framework.TestCase; @@ -761,24 +762,64 @@ public void testParentVersionRELEASE() throws Exception { result.getWarnings().get(0)); } - public void testProfileActivationWithAllowedExpression() throws Exception { - SimpleProblemCollector result = validateRaw("raw-model/profile-activation-file-with-allowed-expressions.xml"); + public void repositoryWithExpression() throws Exception { + SimpleProblemCollector result = validateRaw("raw-model/repository-with-expression.xml"); + assertViolations(result, 0, 1, 0); + assertEquals( + "'repositories.repository.[repo].url' contains an expression but should be a constant.", + result.getErrors().get(0)); + } + + public void repositoryWithBasedirExpression() throws Exception { + SimpleProblemCollector result = validateRaw("raw-model/repository-with-basedir-expression.xml"); assertViolations(result, 0, 0, 0); } - public void testProfileActivationWithProjectExpression() throws Exception { + public void profileActivationWithAllowedExpression() throws Exception { + SimpleProblemCollector result = validateRaw( + "raw-model/profile-activation-file-with-allowed-expressions.xml", + mbr -> mbr.setUserProperties(new Properties() { + private static final long serialVersionUID = 1L; + + { + setProperty("foo", "foo"); + setProperty("bar", "foo"); + } + })); + assertViolations(result, 0, 0, 0); + } + + public void profileActivationFileWithProjectExpression() throws Exception { SimpleProblemCollector result = validateRaw("raw-model/profile-activation-file-with-project-expressions.xml"); assertViolations(result, 0, 0, 2); assertEquals( "'profiles.profile[exists-project-version].activation.file.exists' " - + "Failed to interpolate file location ${project.version}/test.txt: " + + "Failed to interpolate profile activation property ${project.version}/test.txt: " + "${project.version} expressions are not supported during profile activation.", result.getWarnings().get(0)); assertEquals( "'profiles.profile[missing-project-version].activation.file.missing' " - + "Failed to interpolate file location ${project.version}/test.txt: " + + "Failed to interpolate profile activation property ${project.version}/test.txt: " + + "${project.version} expressions are not supported during profile activation.", + result.getWarnings().get(1)); + } + + public void profileActivationPropertyWithProjectExpression() throws Exception { + SimpleProblemCollector result = + validateRaw("raw-model/profile-activation-property-with-project-expressions.xml"); + assertViolations(result, 0, 0, 2); + + assertEquals( + "'profiles.profile[property-name-project-version].activation.property.name' " + + "Failed to interpolate profile activation property ${project.version}: " + + "${project.version} expressions are not supported during profile activation.", + result.getWarnings().get(0)); + + assertEquals( + "'profiles.profile[property-value-project-version].activation.property.value' " + + "Failed to interpolate profile activation property ${project.version}: " + "${project.version} expressions are not supported during profile activation.", result.getWarnings().get(1)); } diff --git a/maven-model-builder/src/test/resources/poms/validation/raw-model/profile-activation-file-with-allowed-expressions.xml b/maven-model-builder/src/test/resources/poms/validation/raw-model/profile-activation-file-with-allowed-expressions.xml index a4beb6238f4a..72b6747f1835 100644 --- a/maven-model-builder/src/test/resources/poms/validation/raw-model/profile-activation-file-with-allowed-expressions.xml +++ b/maven-model-builder/src/test/resources/poms/validation/raw-model/profile-activation-file-with-allowed-expressions.xml @@ -60,5 +60,23 @@ under the License. + + dynamic-property-available + + + ${activationProperty} + + + + + + matches-another-property + + + foo + ${bar} + + + diff --git a/maven-model-builder/src/test/resources/poms/validation/raw-model/profile-activation-property-with-project-expressions.xml b/maven-model-builder/src/test/resources/poms/validation/raw-model/profile-activation-property-with-project-expressions.xml new file mode 100644 index 000000000000..8bcf89f66f72 --- /dev/null +++ b/maven-model-builder/src/test/resources/poms/validation/raw-model/profile-activation-property-with-project-expressions.xml @@ -0,0 +1,49 @@ + + + + 4.0.0 + aid + gid + 0.1 + pom + + + + + property-name-project-version + + + ${project.version} + + + + + property-value-project-version + + + project.version + ${project.version} + + + + + +