fix: properly check for diff with multiple files from overwrite:false (#442)

Author: lukekarrys

lib/check/check-apply.js

@@ -12,8 +12,7 @@ const run = async (type, dir, files, options) => {
   const { add: addFiles, rm: rmFiles } = files
 
   const rm = await rmEach(dir, rmFiles, options, (f) => rel(f))
-  const parseOpts = { allowMultipleSources: false }
-  const [add, update] = partition(await parseEach(dir, addFiles, options, parseOpts, async (p) => {
+  const [add, update] = partition(await parseEach(dir, addFiles, options, {}, async (p) => {
     const diff = await p.applyDiff()
     const target = rel(p.target)
     if (diff === null) {

lib/util/files.js

@@ -31,29 +31,28 @@ const mergeFiles = mergeWithCustomizers((value, srcValue, key, target, source, s
   }
 }, customizers.overwriteArrays)
 
-const fileEntries = (dir, files, options, { allowMultipleSources = true } = {}) => {
+const fileEntries = (dir, files, options) => {
   const results = []
 
-  for (const [key, source] of Object.entries(files)) {
+  for (const [key, value] of Object.entries(files)) {
     // remove any false values first since that means those targets are skipped
-    if (source === false) {
+    if (value === false) {
       continue
     }
 
     // target paths need to be joined with dir and templated
     const target = join(dir, template(key, options))
 
-    if (Array.isArray(source)) {
-      // When turning an object of files into all its entries, we allow
-      // multiples when applying changes, but not when checking for changes
-      // since earlier files would always return as needing an update. So we
-      // either allow multiples and return the array or only return the last
-      // source file in the array.
-      const sources = allowMultipleSources ? source : source.slice(-1)
-      results.push(...sources.map(s => [target, s]))
-    } else {
-      results.push([target, source])
-    }
+    // Allow an array of values to merge into a single source to be
+    // applied or diffed against the target. This is how overwrite:false
+    // works and they are merged.
+    const source = Array.isArray(value)
+      ? value.reduce((acc, { file, ...rest }) => {
+        acc.file.push(file)
+        return Object.assign(acc, rest)
+      }, { file: [] }) : value
+
+    results.push([target, source])
   }
 
   return results

lib/util/parser.js

@@ -62,10 +62,16 @@ class Base {
   }
 
   read (s) {
+    if (Array.isArray(s)) {
+      return Promise.all(s.map(f => this.read(f)))
+    }
     return fs.readFile(s, { encoding: 'utf-8' })
   }
 
   template (s) {
+    if (Array.isArray(s)) {
+      return Promise.all(s.map(f => this.template(f)))
+    }
     return template(s, this.options)
   }
 
@@ -285,6 +291,9 @@ class Json extends Base {
   }
 
   parse (s) {
+    if (Array.isArray(s)) {
+      return s.map(f => this.parse(f)).reduce((a, f) => this.merge(a, f), {})
+    }
     return jsonParse(s)
   }
 

test/apply/overwrite-false.js

@@ -31,10 +31,31 @@ t.test('json merge', async (t) => {
   })
 
   await s.apply()
+  t.strictSame(await s.check(), [])
 
   const pkg = await s.readJson('package.json')
   t.equal(pkg.scripts.test, 'tap test/')
   t.equal(pkg.scripts.snap, 'tap')
 
+  await s.writeJson('package.json', {
+    ...pkg,
+    author: 'What?',
+    scripts: {
+      ...pkg.scripts,
+      test: 'tap',
+    },
+    templateOSS: {
+      ...pkg.templateOSS,
+      version: '1.0.0',
+    },
+  })
+
+  const checks = await s.check()
+  t.equal(checks.length, 1)
+  t.match(checks[0].title, 'package.json needs to be updated')
+  t.match(checks[0].body[0], `"author" is "What?", expected "GitHub Inc."`)
+  t.match(checks[0].body[0], `scripts.test" is "tap", expected "tap test/"`)
+
+  await s.apply()
   t.strictSame(await s.check(), [])
 })