fix: detect overlapping targetPath in generateFiles (#4961)

* refactor: define module overlap finders as functions

Since now, have multiple criteria to find different module overlaps. Each one can be represented with its own matching function.

* fix: detect overlapping `targetPath`s in `generateFiles`

* refactor: extract module sorting comparator as lambda

* chore: introduce module overlap type

To render different kinds of overlaps properly.

* refactor: move module overlap detection code to a separate file

* refactor: move module overlap error rendering function to `module-overlap.ts`

* refactor: move some stateless helpers to the root level

Closure-free lambdas can be declared only once.

* refactor: named interface for overlap error description

* refactor: change `makePathOverlapError` to return list of errors

Also renamed it to `makePathOverlapErrors`.

* chore: render all types of module overlap errors

* chore: remove `switch` and reduce code duplication

DRY and more types for safety.

* chore: fail-fast check for module overlap handler availability

* chore: more specific error details

Print the list of intersecting modules.

* test: fix trivial assertion failures

Use explicit types for expected values to ensure that all required fields are set.

* chore: print overlapping paths in `generateFiles[].targetPath`

* refactor: rename `module` to `config` in `ModuleOverlap`

Because its type is `ModuleConfig`, not `GardenModule`.

* refactor: rename local variable

* refactor: rename `matches` to `overlaps` in anonymous return type

* chore: remove outdated todo-comment

* refactor: extract function

DRY + some cleanup.

* refactor(test): introduce surrounding test context

* test: test module intersection by `generateFiles[].targetPath`

* test: mix of different types of module overlaps

* perf: halve the number of module overlap checks

* refactor: move lambdas outside the overlap detection function

Also introduced parameter object for `ModuleOverlapFinder`.
Additionally, made lambdas closure-free.

* refactor: change the shape of `ModuleOverlapFinderResult`

* fix: ensure commutativity of `ModuleOverlapMatcher` functions

* refactor: simplified error detail rendering

No need to traverse all possible overlap types.

* refactor: named interfaces for better readability

* chore: print all found module overlapping errors

* chore: fix typo

* fix: `generateFiles` presence check

* test: fix assertions

Now module overlap detector prints more details.

* fix: report no overlap on non-intersecting `generateFiles`

* perf: remove unnecessary sorting

The module overlap lists are constructed as sorted ones.

* chore: re-arranged code

---------

Co-authored-by: Thorarinn Sigurdsson <thorarinnsigurdsson@gmail.com>

core/src/garden.ts

@@ -9,7 +9,6 @@
 import Bluebird from "bluebird"
 import chalk from "chalk"
 import { ensureDir } from "fs-extra"
-import dedent from "dedent"
 import { platform, arch } from "os"
 import { relative, resolve } from "path"
 import cloneDeep from "fast-copy"
@@ -42,7 +41,7 @@ import {
   getCloudDistributionName,
   getCloudLogSectionName,
 } from "./util/util"
-import { ConfigurationError, InternalError, isGardenError, GardenError, PluginError, RuntimeError } from "./exceptions"
+import { ConfigurationError, isGardenError, GardenError, InternalError, PluginError, RuntimeError } from "./exceptions"
 import { VcsHandler, ModuleVersion, getModuleVersionString, VcsInfo } from "./vcs/vcs"
 import { GitHandler } from "./vcs/git"
 import { BuildStaging } from "./build-staging/build-staging"
@@ -79,8 +78,6 @@ import {
   findConfigPathsInPath,
   getWorkingCopyId,
   fixedProjectExcludes,
-  detectModuleOverlap,
-  ModuleOverlap,
   defaultConfigFilename,
   defaultDotIgnoreFile,
 } from "./util/fs"
@@ -154,6 +151,7 @@ import { OtelTraced } from "./util/open-telemetry/decorators"
 import { wrapActiveSpan } from "./util/open-telemetry/spans"
 import { GitRepoHandler } from "./vcs/git-repo"
 import { configureNoOpExporter } from "./util/open-telemetry/tracing"
+import { detectModuleOverlap, makeOverlapErrors, ModuleOverlapDescription } from "./util/module-overlap"
 
 const defaultLocalAddress = "localhost"
 
@@ -368,7 +366,10 @@ export class Garden {
     }
 
     if (!SUPPORTED_ARCHITECTURES.includes(currentArch)) {
-      throw new RuntimeError({ message: `Unsupported CPU architecture: ${currentArch}`, detail: { arch: currentArch } })
+      throw new RuntimeError({
+        message: `Unsupported CPU architecture: ${currentArch}`,
+        detail: { arch: currentArch },
+      })
     }
 
     this.state.configsScanned = false
@@ -990,8 +991,18 @@ export class Garden {
       moduleConfigs: resolvedModules,
     })
     if (overlaps.length > 0) {
-      const { message, detail } = this.makeOverlapError(overlaps)
-      throw new ConfigurationError({ message, detail })
+      const overlapErrors = makeOverlapErrors(this.projectRoot, overlaps)
+      const messages: string[] = []
+      const overlappingModules: ModuleOverlapDescription[] = []
+      for (const overlapError of overlapErrors) {
+        const { message, detail } = overlapError
+        messages.push(message)
+        overlappingModules.push(...detail.overlappingModules)
+      }
+      throw new ConfigurationError({
+        message: messages.join("\n\n"),
+        detail: { overlappingModules },
+      })
     }
 
     // Convert modules to actions
@@ -1262,8 +1273,8 @@ export class Garden {
     })
   }
 
-  /*
-    Scans the project root for modules and workflows and adds them to the context.
+  /**
+   * Scans the project root for modules and workflows and adds them to the context.
    */
   @OtelTraced({
     name: "scanAndAddConfigs",
@@ -1548,40 +1559,6 @@ export class Garden {
     return path
   }
 
-  public makeOverlapError(moduleOverlaps: ModuleOverlap[]) {
-    const overlapList = sortBy(moduleOverlaps, (o) => o.module.name)
-      .map(({ module, overlaps }) => {
-        const formatted = overlaps.map((o) => {
-          const detail = o.path === module.path ? "same path" : "nested"
-          return `${chalk.bold(o.name)} (${detail})`
-        })
-        return `Module ${chalk.bold(module.name)} overlaps with module(s) ${naturalList(formatted)}.`
-      })
-      .join("\n\n")
-    const message = chalk.red(dedent`
-      Found multiple enabled modules that share the same garden.yml file or are nested within another:
-
-      ${overlapList}
-
-      If this was intentional, there are two options to resolve this error:
-
-      - You can add ${chalk.bold("include")} and/or ${chalk.bold("exclude")} directives on the affected modules.
-        With explicitly including / encluding files, the modules are actually allowed to overlap in case that is
-        what you want.
-      - You can use the ${chalk.bold("disabled")} directive to make sure that only one of the modules is enabled
-        in any given moment. For example, you can make sure that the modules are enabled only in their exclusive
-        environment.
-    `)
-    // Sanitize error details
-    const overlappingModules = moduleOverlaps.map(({ module, overlaps }) => {
-      return {
-        module: { name: module.name, path: resolve(this.projectRoot, module.path) },
-        overlaps: overlaps.map(({ name, path }) => ({ name, path: resolve(this.projectRoot, path) })),
-      }
-    })
-    return { message, detail: { overlappingModules } }
-  }
-
   public getEnvironmentConfig() {
     for (const config of this.projectConfig.environments) {
       if (config.name === this.environmentName) {
@@ -1899,7 +1876,12 @@ export const resolveGardenParams = profileAsync(async function _resolveGardenPar
         try {
           secrets = await wrapActiveSpan(
             "getSecrets",
-            async () => await cloudApi.getSecrets({ log: cloudLog, projectId: cloudProject!.id, environmentName })
+            async () =>
+              await cloudApi.getSecrets({
+                log: cloudLog,
+                projectId: cloudProject!.id,
+                environmentName,
+              })
           )
           cloudLog.verbose(chalk.green("Ready"))
           cloudLog.debug(`Fetched ${Object.keys(secrets).length} secrets from ${cloudDomain}`)

core/src/util/fs.ts

@@ -17,8 +17,6 @@ import { platform } from "os"
 import { FilesystemError } from "../exceptions"
 import { VcsHandler } from "../vcs/vcs"
 import { Log } from "../logger/log-entry"
-import { ModuleConfig } from "../config/module"
-import pathIsInside from "path-is-inside"
 import { exec } from "./util"
 import type Micromatch from "micromatch"
 import { uuidv4 } from "./random"
@@ -63,54 +61,6 @@ export async function* scanDirectory(path: string, opts?: klaw.Options): AsyncIt
   }
 }
 
-/**
- * Returns a list of overlapping modules.
- *
- * If a module does not set `include` or `exclude`, and another module is in its path (including
- * when the other module has the same path), the module overlaps with the other module.
- */
-export interface ModuleOverlap {
-  module: ModuleConfig
-  overlaps: ModuleConfig[]
-}
-
-export function detectModuleOverlap({
-  projectRoot,
-  gardenDirPath,
-  moduleConfigs,
-}: {
-  projectRoot: string
-  gardenDirPath: string
-  moduleConfigs: ModuleConfig[]
-}): ModuleOverlap[] {
-  // Don't consider overlap between disabled modules, or where one of the modules is disabled
-  const enabledModules = moduleConfigs.filter((m) => !m.disabled)
-
-  let overlaps: ModuleOverlap[] = []
-  for (const config of enabledModules) {
-    if (!!config.include || !!config.exclude) {
-      continue
-    }
-    const matches = enabledModules
-      .filter(
-        (compare) =>
-          config.name !== compare.name &&
-          pathIsInside(compare.path, config.path) &&
-          // Don't consider overlap between modules in root and those in the .garden directory
-          !(config.path === projectRoot && pathIsInside(compare.path, gardenDirPath))
-      )
-      .sort((a, b) => (a.name > b.name ? 1 : -1))
-
-    if (matches.length > 0) {
-      overlaps.push({
-        module: config,
-        overlaps: matches,
-      })
-    }
-  }
-  return overlaps
-}
-
 /**
  * Helper function to check whether a given filename is a valid Garden config filename
  */