Merge pull request #130 from krrish175-byte/fix/bloop-config-suffix

Fix bloop config suffix collision and update dependency resolution

Author: tgodzik

src/main/scala/bloop/integrations/maven/MojoImplementation.scala

@@ -129,6 +129,18 @@ object MojoImplementation {
       }
     }
 
+    val reactorArtifactIds = session.getProjects().asScala.map(_.getArtifactId).toSet
+
+    def getBloopName(artifactId: String, configuration: String): String = {
+      configuration match {
+        case "compile" => artifactId
+        case _ =>
+          val defaultName = s"$artifactId-$configuration"
+          if (reactorArtifactIds.contains(defaultName)) s"$artifactId-$configuration-scope"
+          else defaultName
+      }
+    }
+
     val reactorProjectsSet = mojo
       .getReactorProjects()
       .asScala
@@ -153,7 +165,7 @@ object MojoImplementation {
       log.info(s"Dependency $dep, $matchingArtifacts")
       matchingArtifacts
         .collect {
-          case artifact if artifact.getType == "test-jar" => dep.getArtifactId + "-test"
+          case artifact if artifact.getType == "test-jar" => getBloopName(dep.getArtifactId, "test")
         }
         .toList
         .appended(dep.getArtifactId)
@@ -201,8 +213,7 @@ object MojoImplementation {
         launcher: Option[AppLauncher],
         configuration: String
     ): Unit = {
-      val suffix = if (configuration == "compile") "" else s"-$configuration"
-      val name = project.getArtifactId() + suffix
+      val name = getBloopName(project.getArtifactId(), configuration)
       val build = project.getBuild()
       val baseDirectory = abs(project.getBasedir())
       val out = baseDirectory.resolve("target")
@@ -335,6 +346,12 @@ object MojoImplementation {
       log.info(s"Generated $finalTarget")
       log.debug(s"Configuration to be serialized:\n$config")
       bloop.config.write(config, configTarget.toPath)
+
+      log.info(s"Starting to write configuration for project: ${project.getArtifactId()} with configuration: $configuration")
+      log.debug(s"Source directories: ${sourceDirs0.map(_.getAbsolutePath).mkString(", ")}")
+      log.debug(s"Classpath: ${classpath0().asScala.mkString(", ")}")
+      log.debug(s"Resources: ${resources0.asScala.mkString(", ")}")
+      log.debug(s"Output directory: ${classesDir0.getAbsolutePath}")
     }
 
     writeConfig(

src/test/resources/conflicting_modules/module1-test/pom.xml

@@ -0,0 +1,33 @@
+<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/maven-v4_0_0.xsd">
+  <modelVersion>4.0.0</modelVersion>
+  <groupId>com.example</groupId>
+  <artifactId>module1-test</artifactId>
+  <version>0.1</version>
+  <name>module1-test</name>
+
+  <parent>
+    <groupId>com.example</groupId>
+    <artifactId>conflicting_modules</artifactId>
+    <version>0.1</version>
+  </parent>
+
+  <dependencies>
+    <dependency>
+      <groupId>org.scala-lang</groupId>
+      <artifactId>scala-library</artifactId>
+      <version>2.13.6</version>
+    </dependency>
+  </dependencies>
+  
+  <build>
+    <sourceDirectory>src/main/scala</sourceDirectory>
+    <testSourceDirectory>src/test/scala</testSourceDirectory>
+    <plugins>
+      <plugin>
+        <groupId>net.alchim31.maven</groupId>
+        <artifactId>scala-maven-plugin</artifactId>
+        <version>3.3.2</version>
+      </plugin>
+    </plugins>
+  </build>
+</project>

src/test/resources/conflicting_modules/module1/pom.xml

@@ -0,0 +1,33 @@
+<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/maven-v4_0_0.xsd">
+  <modelVersion>4.0.0</modelVersion>
+  <groupId>com.example</groupId>
+  <artifactId>module1</artifactId>
+  <version>0.1</version>
+  <name>module1</name>
+
+  <parent>
+    <groupId>com.example</groupId>
+    <artifactId>conflicting_modules</artifactId>
+    <version>0.1</version>
+  </parent>
+
+  <dependencies>
+    <dependency>
+      <groupId>org.scala-lang</groupId>
+      <artifactId>scala-library</artifactId>
+      <version>2.13.6</version>
+    </dependency>
+  </dependencies>
+  
+  <build>
+    <sourceDirectory>src/main/scala</sourceDirectory>
+    <testSourceDirectory>src/test/scala</testSourceDirectory>
+    <plugins>
+      <plugin>
+        <groupId>net.alchim31.maven</groupId>
+        <artifactId>scala-maven-plugin</artifactId>
+        <version>3.3.2</version>
+      </plugin>
+    </plugins>
+  </build>
+</project>

src/test/resources/conflicting_modules/pom.xml

@@ -0,0 +1,21 @@
+<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/maven-v4_0_0.xsd">
+  <modelVersion>4.0.0</modelVersion>
+  <groupId>com.example</groupId>
+  <artifactId>conflicting_modules</artifactId>
+  <version>0.1</version>
+  <packaging>pom</packaging>
+  <name>conflicting_modules</name>
+  <description>Project with conflicting module names.</description>
+
+  <properties>
+    <maven.compiler.source>1.8</maven.compiler.source>
+    <maven.compiler.target>1.8</maven.compiler.target>
+    <encoding>UTF-8</encoding>
+  </properties>
+
+    <modules>
+        <module>module1</module>       
+        <module>module1-test</module>      
+    </modules>
+
+</project>

src/test/resources/multi_module_test_jar/bar/pom.xml

@@ -17,6 +17,7 @@
     <dependency>
       <groupId>org.scala-lang</groupId>
       <artifactId>scala-library</artifactId>
+      <version>${scala.version}</version>
     </dependency>
     <dependency>
       <groupId>com.example</groupId>

src/test/resources/multi_module_test_jar/foo/pom.xml

@@ -17,6 +17,7 @@
     <dependency>
       <groupId>org.scala-lang</groupId>
       <artifactId>scala-library</artifactId>
+      <version>${scala.version}</version>
     </dependency>
   </dependencies>
 

src/test/scala/bloop/integrations/maven/MavenConfigGenerationTest.scala

@@ -202,6 +202,45 @@ class MavenConfigGenerationTest extends BaseConfigSuite {
     }
   }
 
+  @Test
+  def conflictingSubmodules() = {
+    check(
+      "conflicting_modules/pom.xml",
+      submodules = List(
+        "conflicting_modules/module1/pom.xml",
+        "conflicting_modules/module1-test/pom.xml"
+      )
+    ) {
+      case (configFile, projectName, List(module1, module2)) =>
+        // module1 is "module1"
+        // module2 is "module1-test"
+        
+        // module1's test configuration should be renamed to avoid conflict with module2
+        // Default would be "module1-test", but "module1-test" exists as a reactor artifact (module2)
+        // So it should be "module1-test-scope"
+        assertEquals("module1", module1.project.name)
+        
+        // We need to check the test configuration name for module1.
+        // check() function loads the "compile" configuration (default expectation in this test suite assumption?)
+        // Actually check() loads the config based on the project file name.
+        // module1 comes from "conflicting_modules/module1/pom.xml" -> parent dir is "module1".
+        // The bloop file loaded is "module1.json".
+        
+        // Let's check the test config of module1
+        val module1TestConfigPath = configFile.project.directory.resolve(".bloop").resolve("module1-test-scope.json")
+        assertTrue(s"Test config for module1 should be renamed to module1-test-scope.json", Files.exists(module1TestConfigPath))
+        
+        val module1TestConfig = readValidBloopConfig(module1TestConfigPath.toFile())
+        assertEquals("module1-test-scope", module1TestConfig.project.name)
+
+        // module2 should start with module1-test
+        assertEquals("module1-test", module2.project.name)
+
+      case _ =>
+        fail("Conflicting modules test should have 2 submodules")
+    }
+  }
+
   @Test
   def dependencyTestJars() = {
     check("test_jars/pom.xml") { (configFile, projectName, subprojects) =>
@@ -260,6 +299,29 @@ class MavenConfigGenerationTest extends BaseConfigSuite {
     }
   }
 
+  @Test
+  def testFallbackNamingForTestScope() = {
+    check(
+      "multi_dependency/pom.xml",
+      submodules = List("multi_dependency/module1/pom.xml", "multi_dependency/module2/pom.xml")
+    ) {
+      case (configFile, projectName, submodulesList) =>
+        val (module1: Config.File, module2: Config.File) = submodulesList match {
+          case List(m1, m2) => (m1, m2)
+          case _ => fail(s"Expected 2 submodules, but got ${submodulesList.size}")
+        }
+        
+        // Standard naming should be preserved when no collision exists
+        assert(!configFile.project.name.contains("-compile"))
+        assert(!module1.project.name.contains("-compile"))
+        assert(!module2.project.name.contains("-compile"))
+        
+        // Note: To test actual collision, we would need to construct a project structure
+        // where a submodule name conflicts with the test suffix of another module.
+        // For now, we verify that the default behavior is correct (no suffixes).
+    }
+  }
+
   @Test
   def junitSupport() = {
     check("junit_project/pom.xml") { (configFile, projectName, subprojects) =>
@@ -328,6 +390,7 @@ class MavenConfigGenerationTest extends BaseConfigSuite {
       val projectName = projectPath.toFile().getName()
       val bloopDir = projectPath.resolve(".bloop")
       val projectFile = bloopDir.resolve(s"${projectName}.json")
+
       val configFile = readValidBloopConfig(projectFile.toFile())
 
       val subProjects = submodules.map { mod =>
@@ -362,7 +425,7 @@ class MavenConfigGenerationTest extends BaseConfigSuite {
       val processBuilder = new ProcessBuilder()
       val out = new StringBuilder()
       processBuilder.directory(cwd)
-      processBuilder.command(cmd: _*);
+      processBuilder.command(cmd: _*)
       var process = processBuilder.start()
 
       val reader =
@@ -373,6 +436,9 @@ class MavenConfigGenerationTest extends BaseConfigSuite {
         out.append(line + "\n")
         line = reader.readLine()
       }
+
+      val exitCode = process.waitFor()
+
       out.toString()
     }
   }