From 5029cc238ce5ce03b0fb431a278889f8b788c0a7 Mon Sep 17 00:00:00 2001
From: Tamas Cservenak <tamas@cservenak.net>
Date: Fri, 1 Mar 2024 18:27:21 +0100
Subject: [PATCH] [MNG-8012] Warn if in-reactor BOM import happens (#1434)

This should be in fact prevented IMHO. Also, model builder missed File uses are corrected.

---

https://issues.apache.org/jira/browse/MNG-8012
---
 .../model/building/DefaultModelBuilder.java   | 38 +++++++++++++++++--
 .../maven/model/building/ModelBuilder.java    | 21 ++++++++++
 2 files changed, 56 insertions(+), 3 deletions(-)

diff --git a/maven-model-builder/src/main/java/org/apache/maven/model/building/DefaultModelBuilder.java b/maven-model-builder/src/main/java/org/apache/maven/model/building/DefaultModelBuilder.java
index 22971e4f2e..bc84b81c21 100644
--- a/maven-model-builder/src/main/java/org/apache/maven/model/building/DefaultModelBuilder.java
+++ b/maven-model-builder/src/main/java/org/apache/maven/model/building/DefaultModelBuilder.java
@@ -25,7 +25,17 @@ import javax.inject.Singleton;
 import java.io.File;
 import java.io.IOException;
 import java.lang.reflect.Field;
-import java.util.*;
+import java.nio.file.Path;
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.HashMap;
+import java.util.Iterator;
+import java.util.LinkedHashSet;
+import java.util.List;
+import java.util.Map;
+import java.util.Objects;
+import java.util.Optional;
+import java.util.Properties;
 import java.util.concurrent.Callable;
 import java.util.function.Supplier;
 import java.util.stream.Collectors;
@@ -1032,14 +1042,27 @@ public class DefaultModelBuilder implements ModelBuilder {
         }
     }
 
+    @Deprecated
     @Override
     public Result<? extends Model> buildRawModel(File pomFile, int validationLevel, boolean locationTracking) {
+        return buildRawModel(pomFile.toPath(), validationLevel, locationTracking, null);
+    }
+
+    @Override
+    public Result<? extends Model> buildRawModel(Path pomFile, int validationLevel, boolean locationTracking) {
         return buildRawModel(pomFile, validationLevel, locationTracking, null);
     }
 
+    @Deprecated
+    @Override
+    public Result<? extends Model> buildRawModel(
+            File pomFile, int validationLevel, boolean locationTracking, TransformerContext context) {
+        return buildRawModel(pomFile.toPath(), validationLevel, locationTracking, context);
+    }
+
     @Override
     public Result<? extends Model> buildRawModel(
-            File pomFile, int validationLevel, boolean locationTracking, TransformerContext context) {
+            Path pomFile, int validationLevel, boolean locationTracking, TransformerContext context) {
         final ModelBuildingRequest request = new DefaultModelBuildingRequest()
                 .setValidationLevel(validationLevel)
                 .setLocationTracking(locationTracking)
@@ -1050,7 +1073,7 @@ public class DefaultModelBuilder implements ModelBuilder {
 
             try {
                 if (transformer != null && context != null) {
-                    transformer.transform(pomFile.toPath(), context, model);
+                    transformer.transform(pomFile, context, model);
                 }
             } catch (TransformerException e) {
                 problems.add(
@@ -1830,6 +1853,15 @@ public class DefaultModelBuilder implements ModelBuilder {
                 return null;
             }
 
+            if (importSource instanceof FileModelSource && request.getRootDirectory() != null) {
+                Path sourcePath = ((FileModelSource) importSource).getPath();
+                if (sourcePath.startsWith(request.getRootDirectory())) {
+                    problems.add(new ModelProblemCollectorRequest(Severity.WARNING, ModelProblem.Version.BASE)
+                            .setMessage("BOM imports from within reactor should be avoided")
+                            .setLocation(dependency.getLocation("")));
+                }
+            }
+
             final ModelBuildingResult importResult;
             try {
                 ModelBuildingRequest importRequest = new DefaultModelBuildingRequest();
diff --git a/maven-model-builder/src/main/java/org/apache/maven/model/building/ModelBuilder.java b/maven-model-builder/src/main/java/org/apache/maven/model/building/ModelBuilder.java
index 90e702065b..0af089b252 100644
--- a/maven-model-builder/src/main/java/org/apache/maven/model/building/ModelBuilder.java
+++ b/maven-model-builder/src/main/java/org/apache/maven/model/building/ModelBuilder.java
@@ -19,6 +19,7 @@
 package org.apache.maven.model.building;
 
 import java.io.File;
+import java.nio.file.Path;
 
 import org.apache.maven.model.Model;
 
@@ -52,11 +53,31 @@ public interface ModelBuilder {
 
     /**
      * Performs only the part of {@link ModelBuilder#build(ModelBuildingRequest)} that loads the raw model
+     *
+     * @deprecated Use {@link #buildRawModel(Path, int, boolean)} instead.
      */
+    @Deprecated
     Result<? extends Model> buildRawModel(File pomFile, int validationLevel, boolean locationTracking);
 
+    /**
+     * Performs only the part of {@link ModelBuilder#build(ModelBuildingRequest)} that loads the raw model
+     *
+     * @since 4.0.0
+     */
+    Result<? extends Model> buildRawModel(Path pomFile, int validationLevel, boolean locationTracking);
+
+    /**
+     * @deprecated Use {@link #buildRawModel(Path, int, boolean, TransformerContext)} instead.
+     */
+    @Deprecated
     Result<? extends Model> buildRawModel(
             File pomFile, int validationLevel, boolean locationTracking, TransformerContext context);
 
+    /**
+     * @since 4.0.0
+     */
+    Result<? extends Model> buildRawModel(
+            Path pomFile, int validationLevel, boolean locationTracking, TransformerContext context);
+
     TransformerContextBuilder newTransformerContextBuilder();
 }