fix: evaluate all patterns before throwing EDUPLICATEWORKSPACE

lib/index.js

@@ -7,7 +7,7 @@ const rpj = require('read-package-json-fast')
 const glob = require('glob')
 const pGlob = promisify(glob)
 
-function appendNegatedPatterns (patterns) {
+function appendNegatedPatterns(patterns) {
   const results = []
   for (let pattern of patterns) {
     const excl = pattern.match(/^!+/)
@@ -26,7 +26,7 @@ function appendNegatedPatterns (patterns) {
   return results
 }
 
-function getPatterns (workspaces) {
+function getPatterns(workspaces) {
   const workspacesDeclaration =
     Array.isArray(workspaces.packages)
       ? workspaces.packages
@@ -42,35 +42,35 @@ function getPatterns (workspaces) {
   return appendNegatedPatterns(workspacesDeclaration)
 }
 
-function getPackageName (pkg, pathname) {
+function getPackageName(pkg, pathname) {
   const { name } = pkg
   return name || getName(pathname)
 }
 
-function pkgPathmame (opts) {
+function pkgPathmame(opts) {
   return (...args) => {
     const cwd = opts.cwd ? opts.cwd : process.cwd()
     return path.join.apply(null, [cwd, ...args])
   }
 }
 
 // make sure glob pattern only matches folders
-function getGlobPattern (pattern) {
+function getGlobPattern(pattern) {
   pattern = pattern.replace(/\\/g, '/')
   return pattern.endsWith('/')
     ? pattern
     : `${pattern}/`
 }
 
-function getError ({ Type = TypeError, message, code }) {
+function getError({ Type = TypeError, message, code }) {
   return Object.assign(new Type(message), { code })
 }
 
-function reverseResultMap (map) {
+function reverseResultMap(map) {
   return new Map(Array.from(map, item => item.reverse()))
 }
 
-async function mapWorkspaces (opts = {}) {
+async function mapWorkspaces(opts = {}) {
   if (!opts || !opts.pkg) {
     throw getError({
       message: 'mapWorkspaces missing pkg info',
@@ -117,28 +117,52 @@ async function mapWorkspaces (opts = {}) {
 
       const name = getPackageName(pkg, packagePathname)
 
+      let seenPackagePathnames = seen.get(name)
+      if (!seenPackagePathnames) {
+        seenPackagePathnames = new Set()
+        seen.set(name, seenPackagePathnames)
+      }
       if (item.negate) {
-        results.delete(packagePathname, name)
+        seenPackagePathnames.delete(packagePathname)
       } else {
-        if (seen.has(name) && seen.get(name) !== packagePathname) {
-          throw getError({
-            Type: Error,
-            message: [
-              'must not have multiple workspaces with the same name',
-              `package '${name}' has conflicts in the following paths:`,
-              '    ' + seen.get(name),
-              '    ' + packagePathname,
-            ].join('\n'),
-            code: 'EDUPLICATEWORKSPACE',
-          })
-        }
-
-        seen.set(name, packagePathname)
-        results.set(packagePathname, name)
+        seenPackagePathnames.add(packagePathname)
       }
     }
   }
-  return reverseResultMap(results)
+
+  const errorMessageArray = ['must not have multiple workspaces with the same name']
+  for (const [packageName, seenPackagePathnames] of seen) {
+    if (seenPackagePathnames.size === 0) {
+      continue
+    }
+    if (seenPackagePathnames.size > 1) {
+      addDuplicateErrorMessages(errorMessageArray, packageName, seenPackagePathnames)
+    } else {
+      results.set(packageName, seenPackagePathnames.values().next().value)
+    }
+  }
+
+  if (errorMessageArray.length > 1) {
+    throw getError({
+      Type: Error,
+      message: errorMessageArray.join('\n'),
+      code: 'EDUPLICATEWORKSPACE',
+    })
+  }
+
+  return results
+}
+
+function addDuplicateErrorMessages(messageArray, packageName, packagePathnames) {
+  messageArray.push(
+    `package '${packageName}' has conflicts in the following paths:`
+  )
+
+  for (const packagePathname of packagePathnames) {
+    messageArray.push(
+      '    ' + packagePathname
+    )
+  }
 }
 
 mapWorkspaces.virtual = function (opts = {}) {

tap-snapshots/test/test.js.test.cjs

@@ -41,6 +41,22 @@ Map {
 }
 `
 
+exports[`test/test.js TAP match duplicates then exclude one > should include the non-excluded item on returned Map 1`] = `
+Map {
+  "a" => "{CWD}/test/tap-testdir-test-match-duplicates-then-exclude-one/packages/a",
+}
+`
+
+exports[`test/test.js TAP matched then negated then match again > should include item on returned Map 1`] = `
+Map {
+  "a" => "{CWD}/test/tap-testdir-test-matched-then-negated-then-match-again/packages/b/a",
+}
+`
+
+exports[`test/test.js TAP matched then negated then match again with wildcards > should exclude item on returned Map 1`] = `
+Map {}
+`
+
 exports[`test/test.js TAP missing pkg info > should return an empty map 1`] = `
 Array [
   Map {},
@@ -49,6 +65,19 @@ Array [
 ]
 `
 
+exports[`test/test.js TAP multiple duplicated workspaces config > should throw an error listing all duplicates 1`] = `
+Error: must not have multiple workspaces with the same name
+package 'a' has conflicts in the following paths:
+    C:{CWD}/test/tap-testdir-test-multiple-duplicated-workspaces-config/packages/a
+    C:{CWD}/test/tap-testdir-test-multiple-duplicated-workspaces-config/packages/b
+    C:{CWD}/test/tap-testdir-test-multiple-duplicated-workspaces-config/packages/c
+package 'b' has conflicts in the following paths:
+    C:{CWD}/test/tap-testdir-test-multiple-duplicated-workspaces-config/packages/d
+    C:{CWD}/test/tap-testdir-test-multiple-duplicated-workspaces-config/packages/e {
+  "code": "EDUPLICATEWORKSPACE",
+}
+`
+
 exports[`test/test.js TAP multiple negate patterns > should not include any negated pattern 1`] = `
 Map {}
 `

test/test.js

@@ -229,6 +229,42 @@ test('duplicated workspaces glob pattern', t => {
   )
 })
 
+test('multiple duplicated workspaces config', t => {
+  const cwd = t.testdir({
+    packages: {
+      a: {
+        'package.json': '{ "name": "a" }',
+      },
+      b: {
+        'package.json': '{ "name": "a" }',
+      },
+      c: {
+        'package.json': '{ "name": "a" }',
+      },
+      d: {
+        'package.json': '{ "name": "b" }',
+      },
+      e: {
+        'package.json': '{ "name": "b" }',
+      },
+    },
+  })
+
+  return t.resolveMatchSnapshot(
+    mapWorkspaces({
+      cwd,
+      pkg: {
+        workspaces: {
+          packages: [
+            'packages/*',
+          ],
+        },
+      },
+    }).catch(error => Promise.resolve(error)),
+    'should throw an error listing all duplicates'
+  )
+})
+
 test('empty packages declaration', t => {
   const cwd = t.testdir({
     packages: {
@@ -793,3 +829,82 @@ test('backslashes are normalized', t => {
     'matches with backslashes'
   )
 })
+
+test('matched then negated then match again with wildcards', t => {
+  const cwd = t.testdir({
+    packages: {
+      b: {
+        a: {
+          'package.json': '{ "name": "a" }',
+        },
+      },
+    },
+  })
+
+  return t.resolveMatchSnapshot(
+    mapWorkspaces({
+      cwd,
+      pkg: {
+        workspaces: [
+          'packages/**',
+          '!packages/b/**',
+        ],
+      },
+    }),
+    'should exclude item on returned Map'
+  )
+})
+
+test('matched then negated then match again', t => {
+  const cwd = t.testdir({
+    packages: {
+      b: {
+        a: {
+          'package.json': '{ "name": "a" }',
+        },
+      },
+    },
+  })
+
+  return t.resolveMatchSnapshot(
+    mapWorkspaces({
+      cwd,
+      pkg: {
+        workspaces: [
+          'packages/**',
+          '!packages/b/**',
+          'packages/b/a',
+        ],
+      },
+    }),
+    'should include item on returned Map'
+  )
+})
+
+test('match duplicates then exclude one', t => {
+  const cwd = t.testdir({
+    packages: {
+      a: {
+        'package.json': '{ "name": "a" }',
+      },
+      b: {
+        a: {
+          'package.json': '{ "name": "a" }',
+        },
+      },
+    },
+  })
+
+  return t.resolveMatchSnapshot(
+    mapWorkspaces({
+      cwd,
+      pkg: {
+        workspaces: [
+          'packages/**',
+          '!packages/b/**',
+        ],
+      },
+    }),
+    'should include the non-excluded item on returned Map'
+  )
+})