From 56dbb6b4c295d9b76f557779af14f5d21f6cf1d0 Mon Sep 17 00:00:00 2001 From: Luke Karrys Date: Thu, 2 May 2024 12:27:59 -0700 Subject: [PATCH] fix: properly check for diff with multiple files from overwrite:false Fixes #437 --- lib/check/check-apply.js | 2 +- lib/util/files.js | 23 ++++++++++++++++------- lib/util/parser.js | 9 +++++++++ test/apply/overwrite-false.js | 21 +++++++++++++++++++++ 4 files changed, 47 insertions(+), 8 deletions(-) diff --git a/lib/check/check-apply.js b/lib/check/check-apply.js index b1cde376..2f47946a 100644 --- a/lib/check/check-apply.js +++ b/lib/check/check-apply.js @@ -12,7 +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 parseOpts = { command: 'check' } const [add, update] = partition(await parseEach(dir, addFiles, options, parseOpts, async (p) => { const diff = await p.applyDiff() const target = rel(p.target) diff --git a/lib/util/files.js b/lib/util/files.js index 713c01e0..ad52f31e 100644 --- a/lib/util/files.js +++ b/lib/util/files.js @@ -31,7 +31,7 @@ const mergeFiles = mergeWithCustomizers((value, srcValue, key, target, source, s } }, customizers.overwriteArrays) -const fileEntries = (dir, files, options, { allowMultipleSources = true } = {}) => { +const fileEntries = (dir, files, options, { command } = {}) => { const results = [] for (const [key, source] of Object.entries(files)) { @@ -45,12 +45,21 @@ const fileEntries = (dir, files, options, { allowMultipleSources = true } = {}) 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])) + // multiples when applying and checking changes. When applying we can + // run them each as [[t,s1],[t,s2],...] because they can be applied in order + // and get the correct result. When checking for changes we need to run them + // as [target,{file:[s1,s2...]}] so that all files can be merged first and then + // we can diff our target against that. + if (command === 'check') { + const sources = source.reduce((acc, { file, ...rest }) => { + acc.file.push(file) + Object.assign(acc, rest) + return acc + }, { file: [] }) + results.push([target, sources]) + } else { + results.push(...source.map(s => [target, s])) + } } else { results.push([target, source]) } diff --git a/lib/util/parser.js b/lib/util/parser.js index 27f498b7..48b5f4e8 100644 --- a/lib/util/parser.js +++ b/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) } diff --git a/test/apply/overwrite-false.js b/test/apply/overwrite-false.js index ca0bedbd..260d7f7f 100644 --- a/test/apply/overwrite-false.js +++ b/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(), []) })