Fix validation, typo in 4.1.0 model version, fix POMs in tests, add @…

…deprecated to modules field in model

api/maven-api-core/src/main/java/org/apache/maven/api/services/ModelBuilder.java

@@ -27,7 +27,7 @@ public interface ModelBuilder extends Service {
 
     String MODEL_VERSION_4_0_0 = "4.0.0";
 
-    String MODEL_VERSION_4_1_0 = "4.0.0";
+    String MODEL_VERSION_4_1_0 = "4.1.0";
 
     List<String> VALID_MODEL_VERSIONS = List.of(MODEL_VERSION_4_0_0, MODEL_VERSION_4_1_0);
 

api/maven-api-model/src/main/mdo/maven.mdo

@@ -313,7 +313,7 @@
         <!-- SCM                                                                    -->
         <!-- ====================================================================== -->
 
-        <field xdoc.separator="blank" xml.insertParentFieldsUpTo="modules">
+        <field xdoc.separator="blank" xml.insertParentFieldsUpTo="subprojects">
           <name>scm</name>
           <version>4.0.0+</version>
           <description>Specification for the SCM used by the project, such as CVS, Subversion, etc.</description>
@@ -535,6 +535,9 @@
             <type>String</type>
             <multiplicity>*</multiplicity>
           </association>
+          <annotations>
+            <annotation>@Deprecated</annotation>
+          </annotations>
         </field>
         <field xdoc.separator="blank">
           <name>subprojects</name>

maven-api-impl/pom.xml

@@ -29,6 +29,11 @@ under the License.
   <name>Maven API Implementation</name>
   <description>Provides the implementation classes for the Maven API</description>
 
+  <properties>
+    <!-- in: DefaultModelValidator, DefaultModelBuilder -->
+    <checkstyle.violation.ignore>FileLength</checkstyle.violation.ignore>
+  </properties>
+
   <dependencies>
     <dependency>
       <groupId>org.apache.maven</groupId>

maven-api-impl/src/main/java/org/apache/maven/internal/impl/model/DefaultModelTransformerContextBuilder.java

@@ -151,12 +151,16 @@ private void doLoadFullReactor() {
                         ModelBuilderRequest gaBuildingRequest =
                                 ModelBuilderRequest.build(request, ModelSource.fromPath(pom));
                         Model rawModel = defaultModelBuilder.readFileModel(gaBuildingRequest, problems);
-                        for (String module : rawModel.getModules()) {
-                            Path moduleFile = defaultModelBuilder
+                        List<String> subprojects = rawModel.getSubprojects();
+                        if (subprojects == null) {
+                            subprojects = rawModel.getModules();
+                        }
+                        for (String subproject : subprojects) {
+                            Path subprojectFile = defaultModelBuilder
                                     .getModelProcessor()
-                                    .locateExistingPom(pom.getParent().resolve(module));
-                            if (moduleFile != null) {
-                                toLoad.add(moduleFile);
+                                    .locateExistingPom(pom.getParent().resolve(subproject));
+                            if (subprojectFile != null) {
+                                toLoad.add(subprojectFile);
                             }
                         }
                     } catch (ModelBuilderException e) {

maven-api-impl/src/main/java/org/apache/maven/internal/impl/model/DefaultModelValidator.java

@@ -20,7 +20,6 @@
 
 import java.io.File;
 import java.util.Arrays;
-import java.util.Collections;
 import java.util.Deque;
 import java.util.HashMap;
 import java.util.HashSet;
@@ -65,6 +64,7 @@
 import org.apache.maven.api.model.Repository;
 import org.apache.maven.api.model.Resource;
 import org.apache.maven.api.services.BuilderProblem.Severity;
+import org.apache.maven.api.services.ModelBuilder;
 import org.apache.maven.api.services.ModelBuilderRequest;
 import org.apache.maven.api.services.ModelProblem.Version;
 import org.apache.maven.api.services.ModelProblemCollector;
@@ -73,14 +73,15 @@
 import org.apache.maven.model.v4.MavenModelVersion;
 import org.apache.maven.model.v4.MavenTransformer;
 
+import static org.apache.maven.internal.impl.model.DefaultModelBuilder.NAMESPACE_PREFIX;
+
 /**
  */
 @Named
 @Singleton
 public class DefaultModelValidator implements ModelValidator {
 
-    public static final List<String> VALID_MODEL_VERSIONS =
-            Collections.unmodifiableList(Arrays.asList("4.0.0", "4.1.0"));
+    public static final List<String> VALID_MODEL_VERSIONS = ModelBuilder.VALID_MODEL_VERSIONS;
 
     private static final Pattern EXPRESSION_NAME_PATTERN = Pattern.compile("\\$\\{(.+?)}");
     private static final Pattern EXPRESSION_PROJECT_NAME_PATTERN = Pattern.compile("\\$\\{(project.+?)}");
@@ -134,7 +135,6 @@ protected Activation.Builder transformActivation_ActiveByDefault(
         protected Activation.Builder transformActivation_File(
                 Supplier<? extends Activation.Builder> creator, Activation.Builder builder, Activation target) {
             stk.push(nextFrame("file", Activation::getFile));
-            Optional.ofNullable(target.getFile());
             try {
                 return super.transformActivation_File(creator, builder, target);
             } finally {
@@ -333,7 +333,7 @@ public void validateFileModel(Model m, ModelBuilderRequest request, ModelProblem
         if (request.getValidationLevel() == ModelBuilderRequest.VALIDATION_LEVEL_MINIMAL) {
             // profiles: they are essential for proper model building (may contribute profiles, dependencies...)
             HashSet<String> minProfileIds = new HashSet<>();
-            for (org.apache.maven.api.model.Profile profile : m.getProfiles()) {
+            for (Profile profile : m.getProfiles()) {
                 if (!minProfileIds.add(profile.getId())) {
                     addViolation(
                             problems,
@@ -360,6 +360,61 @@ public void validateFileModel(Model m, ModelBuilderRequest request, ModelProblem
                             m.getLocation("modules"));
                 }
             }
+            String modelVersion = m.getModelVersion();
+            if (modelVersion == null) {
+                String namespace = m.getNamespaceUri();
+                if (namespace != null && namespace.startsWith(NAMESPACE_PREFIX)) {
+                    modelVersion = namespace.substring(NAMESPACE_PREFIX.length());
+                }
+            }
+            if (Objects.equals(modelVersion, ModelBuilder.MODEL_VERSION_4_0_0)) {
+                if (!m.getSubprojects().isEmpty()) {
+                    addViolation(
+                            problems,
+                            Severity.ERROR,
+                            Version.V40,
+                            "subprojects",
+                            null,
+                            "unexpected subprojects element",
+                            m.getLocation("subprojects"));
+                }
+            } else {
+                Set<String> subprojects = new HashSet<>();
+                for (int i = 0, n = m.getSubprojects().size(); i < n; i++) {
+                    String subproject = m.getSubprojects().get(i);
+                    if (!subprojects.add(subproject)) {
+                        addViolation(
+                                problems,
+                                Severity.ERROR,
+                                Version.V41,
+                                "subprojects.subproject[" + i + "]",
+                                null,
+                                "specifies duplicate subproject " + subproject,
+                                m.getLocation("subprojects"));
+                    }
+                }
+                if (!modules.isEmpty()) {
+                    if (subprojects.isEmpty()) {
+                        addViolation(
+                                problems,
+                                Severity.WARNING,
+                                Version.V41,
+                                "modules",
+                                null,
+                                "deprecated modules element, use subprojects instead",
+                                m.getLocation("modules"));
+                    } else {
+                        addViolation(
+                                problems,
+                                Severity.WARNING,
+                                Version.V41,
+                                "modules",
+                                null,
+                                "cannot use both modules and subprojects element",
+                                m.getLocation("modules"));
+                    }
+                }
+            }
 
             Severity errOn30 = getSeverity(request, ModelBuilderRequest.VALIDATION_LEVEL_MAVEN_3_0);
 
@@ -644,10 +699,6 @@ public void validateEffectiveModel(Model m, ModelBuilderRequest request, ModelPr
 
         validateStringNotEmpty("packaging", problems, Severity.ERROR, Version.BASE, m.getPackaging(), m);
 
-        // TODO: if the model is a 4.1.0:
-        //   * modules should be empty, else issue a warning
-        //   * validate subprojects
-
         if (!m.getModules().isEmpty()) {
             if (!"pom".equals(m.getPackaging())) {
                 addViolation(
@@ -914,7 +965,7 @@ private void validate20RawDependencies(
 
     private void validate20RawDependenciesSelfReferencing(
             ModelProblemCollector problems,
-            org.apache.maven.api.model.Model m,
+            Model m,
             List<Dependency> dependencies,
             String prefix,
             ModelBuilderRequest request) {
@@ -945,7 +996,7 @@ private void validate20RawDependenciesSelfReferencing(
 
     private void validateEffectiveDependencies(
             ModelProblemCollector problems,
-            org.apache.maven.api.model.Model m,
+            Model m,
             List<Dependency> dependencies,
             boolean management,
             ModelBuilderRequest request) {
@@ -1006,11 +1057,7 @@ private void validateEffectiveDependencies(
     }
 
     private void validateEffectiveModelAgainstDependency(
-            String prefix,
-            ModelProblemCollector problems,
-            org.apache.maven.api.model.Model m,
-            Dependency d,
-            ModelBuilderRequest request) {
+            String prefix, ModelProblemCollector problems, Model m, Dependency d, ModelBuilderRequest request) {
         String key = d.getGroupId() + ":" + d.getArtifactId() + ":" + d.getVersion()
                 + (d.getClassifier() != null ? ":" + d.getClassifier() : EMPTY);
         String mKey = m.getGroupId() + ":" + m.getArtifactId() + ":" + m.getVersion();

maven-core/src/main/java/org/apache/maven/internal/transformation/impl/DefaultConsumerPomBuilder.java

@@ -225,7 +225,7 @@ static Model transform(Model model, MavenProject project) {
         String packaging = model.getPackaging();
         if (POM_PACKAGING.equals(packaging)) {
             // raw to consumer transform
-            model = model.withRoot(false).withModules(null);
+            model = model.withRoot(false).withModules(null).withSubprojects(null);
             if (model.getParent() != null) {
                 model = model.withParent(model.getParent().withRelativePath(null));
             }

maven-core/src/test/java/org/apache/maven/internal/transformation/impl/ConsumerPomArtifactTransformerTest.java

@@ -65,7 +65,7 @@ void transform() throws Exception {
 
             t.transform(project, systemSessionMock, beforePomFile, tempFile);
         }
-        XmlAssert.assertThat(afterPomFile.toFile()).and(tempFile.toFile()).areIdentical();
+        XmlAssert.assertThat(tempFile.toFile()).and(afterPomFile.toFile()).areIdentical();
     }
 
     @Test

maven-core/src/test/resources/consumer/simple/pom.xml

@@ -18,15 +18,15 @@ specific language governing permissions and limitations
 under the License.
 -->
 <project root="true" xmlns="http://maven.apache.org/POM/4.1.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
-         xsi:schemaLocation="http://maven.apache.org/POM/4.1.0 https://maven.apache.org/xsd/maven-4.1.0-alpha-8.xsd">
+         xsi:schemaLocation="http://maven.apache.org/POM/4.1.0 https://maven.apache.org/xsd/maven-4.1.0.xsd">
   <groupId>org.sonatype.mavenbook.multi</groupId>
   <artifactId>parent</artifactId>
   <version>0.9-${changelist}-SNAPSHOT</version>
   <packaging>pom</packaging>
   <name>Multi Chapter Parent Project</name>
 
   <!-- Optimized from https://github.com/sonatype/maven-example-en/tree/master/examples/ch-multi -->
-  <modules>
-    <module>simple-parent</module>
-  </modules>
+  <subprojects>
+    <subproject>simple-parent</subproject>
+  </subprojects>
 </project>

maven-core/src/test/resources/consumer/simple/simple-parent/pom.xml

@@ -17,7 +17,7 @@ KIND, either express or implied.  See the License for the
 specific language governing permissions and limitations
 under the License.
 -->
-<project xmlns="http://maven.apache.org/POM/4.1.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://maven.apache.org/POM/4.1.0 https://maven.apache.org/xsd/maven-4.1.0-alpha-8.xsd">
+<project xmlns="http://maven.apache.org/POM/4.1.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://maven.apache.org/POM/4.1.0 https://maven.apache.org/xsd/maven-4.1.0.xsd">
   <parent>
     <groupId>org.sonatype.mavenbook.multi</groupId>
     <artifactId>parent</artifactId>
@@ -26,15 +26,15 @@ under the License.
   <packaging>pom</packaging>
   <name>Multi Chapter Simple Parent Project</name>
 
-  <modules>
-    <module>simple-weather</module>
-    <module>simple-webapp</module>
+  <subprojects>
+    <subproject>simple-weather</subproject>
+    <subproject>simple-webapp</subproject>
 
     <!-- On purpose at the end, project graph is responsible for ordering -->
     <!-- Build/consumer should not be effected by reverse order -->
-    <module>simple-testutils</module>
-    <module>utils-parent</module>
-  </modules>
+    <subproject>simple-testutils</subproject>
+    <subproject>utils-parent</subproject>
+  </subprojects>
 
   <build>
     <pluginManagement>

maven-core/src/test/resources/consumer/simple/simple-parent/simple-testutils/pom.xml

@@ -17,7 +17,7 @@ KIND, either express or implied.  See the License for the
 specific language governing permissions and limitations
 under the License.
 -->
-<project xmlns="http://maven.apache.org/POM/4.1.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://maven.apache.org/POM/4.1.0 https://maven.apache.org/xsd/maven-4.1.0-alpha-8.xsd">
+<project xmlns="http://maven.apache.org/POM/4.1.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://maven.apache.org/POM/4.1.0 https://maven.apache.org/xsd/maven-4.1.0.xsd">
   <parent>
     <groupId>org.sonatype.mavenbook.multi</groupId>
     <artifactId>utils-parent</artifactId>

maven-core/src/test/resources/consumer/simple/simple-parent/simple-weather/pom.xml

@@ -17,7 +17,7 @@ KIND, either express or implied.  See the License for the
 specific language governing permissions and limitations
 under the License.
 -->
-<project xmlns="http://maven.apache.org/POM/4.1.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://maven.apache.org/POM/4.1.0 https://maven.apache.org/xsd/maven-4.1.0-alpha-8.xsd">
+<project xmlns="http://maven.apache.org/POM/4.1.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://maven.apache.org/POM/4.1.0 https://maven.apache.org/xsd/maven-4.1.0.xsd">
   <parent>
     <groupId>org.sonatype.mavenbook.multi</groupId>
     <artifactId>simple-parent</artifactId>

maven-core/src/test/resources/consumer/simple/simple-parent/utils-parent/pom.xml

@@ -17,7 +17,7 @@ KIND, either express or implied.  See the License for the
 specific language governing permissions and limitations
 under the License.
 -->
-<project xmlns="http://maven.apache.org/POM/4.1.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://maven.apache.org/POM/4.1.0 https://maven.apache.org/xsd/maven-4.1.0-alpha-8.xsd">
+<project xmlns="http://maven.apache.org/POM/4.1.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://maven.apache.org/POM/4.1.0 https://maven.apache.org/xsd/maven-4.1.0.xsd">
   <parent>
     <groupId>org.sonatype.mavenbook.multi</groupId>
     <artifactId>simple-parent</artifactId>

maven-core/src/test/resources/consumer/trivial/pom.xml

@@ -4,9 +4,9 @@
   <version>1.0-SNAPSHOT</version>
   <packaging>pom</packaging>
 
-  <modules>
-    <module>child.xml</module>
-  </modules>
+  <subprojects>
+    <subproject>child.xml</subproject>
+  </subprojects>
 
   <dependencyManagement>
     <dependencies>

maven-core/src/test/resources/projects/transform/before.pom

@@ -30,10 +30,10 @@ under the License.
   <version>0.1-SNAPSHOT</version>
   <packaging>pom</packaging>
 
-  <modules>
-    <module>lib</module> <!-- the library -->
-    <module>app</module> <!-- the application -->
-  </modules>
+  <subprojects>
+    <subproject>lib</subproject> <!-- the library -->
+    <subproject>app</subproject> <!-- the application -->
+  </subprojects>
 
   <build>
     <pluginManagement>

src/mdo/model.vm

@@ -132,6 +132,9 @@ public class ${class.name}
      * ${line.trim()}
       #end
      */
+      #foreach( $ann in ${field.annotations} )
+    ${ann}
+      #end
     final ${type} $field.name;
     #end
     #if ( $locationTracking )
@@ -236,6 +239,9 @@ public class ${class.name}
      *
      * @return a {@code ${type}}
      */
+      #foreach( $ann in ${field.annotations} )
+    ${ann}
+      #end
       #if ( $field.type == "java.util.List" || $field.type == "java.util.Properties" )
     @Nonnull
       #end
@@ -274,6 +280,9 @@ public class ${class.name}
      * @param ${field.name} the new {@code $type} to use
      * @return a {@code ${class.name}} with the specified ${field.name}
      */
+      #foreach( $ann in ${field.annotations} )
+    ${ann}
+      #end
     @Nonnull
     public ${class.name} with${cap}($type $field.name) {
         return newBuilder(this, true).${field.name}($field.name).build();
@@ -440,6 +449,9 @@ public class ${class.name}
       #set ( $type = ${types.getOrDefault($field,${types.getOrDefault($field.type,$field.type)})} )
       #if ( $type.startsWith("List<") )
         #set ( $type = ${type.replace('List<','Collection<')} )
+      #end
+      #foreach( $ann in ${field.annotations} )
+        ${ann}
       #end
         @Nonnull
         public Builder ${field.name}(${type} ${field.name}) {