From 623b10a3d74f09dad766318264177832121e044f Mon Sep 17 00:00:00 2001 From: Thijs Louisse Date: Wed, 12 Aug 2020 13:34:39 +0200 Subject: [PATCH] fix(providence-analytics): allowlist takes precedence over -mode --- .changeset/witty-goats-attend.md | 15 ++++++ packages/providence-analytics/src/cli/cli.js | 4 +- .../src/cli/generate-extend-docs-data.js | 4 +- .../src/program/services/InputDataService.js | 52 +++++++++++++------ .../test-node/cli/cli.test.js | 6 +-- .../program/services/InputDataService.test.js | 33 +++++++++--- 6 files changed, 82 insertions(+), 32 deletions(-) create mode 100644 .changeset/witty-goats-attend.md diff --git a/.changeset/witty-goats-attend.md b/.changeset/witty-goats-attend.md new file mode 100644 index 000000000..fb0b9f64a --- /dev/null +++ b/.changeset/witty-goats-attend.md @@ -0,0 +1,15 @@ +--- +'providence-analytics': minor +--- + +Custom '--allowlist' takes precedence over '--allowlist-mode' + +#### Features + +- Custom '--allowlist' takes precedence over '--allowlist-mode' when conflicting. + For instance, when running CLI with '--allowlist-mode git --allowlist ./dist' + (and .gitignore contained '/dist'), './dist' will still be analyzed. + +#### Patches + +- Align naming conventions between CLI and InputDataService.gatherFilesFromDir diff --git a/packages/providence-analytics/src/cli/cli.js b/packages/providence-analytics/src/cli/cli.js index 58ef3fe73..be7d4f7b8 100755 --- a/packages/providence-analytics/src/cli/cli.js +++ b/packages/providence-analytics/src/cli/cli.js @@ -117,12 +117,12 @@ async function cli({ cwd } = {}) { gatherFilesConfig: { extensions: commander.extensions, allowlistMode: commander.allowlistMode, - filter: commander.allowlist, + allowlist: commander.allowlist, }, gatherFilesConfigReference: { extensions: commander.extensions, allowlistMode: commander.allowlistModeReference, - filter: commander.allowlistReference, + allowlist: commander.allowlistReference, }, debugEnabled: commander.debug, queryMethod, diff --git a/packages/providence-analytics/src/cli/generate-extend-docs-data.js b/packages/providence-analytics/src/cli/generate-extend-docs-data.js index 39279fdaa..2ba32b45e 100644 --- a/packages/providence-analytics/src/cli/generate-extend-docs-data.js +++ b/packages/providence-analytics/src/cli/generate-extend-docs-data.js @@ -22,11 +22,11 @@ async function launchProvidenceWithExtendDocs({ { gatherFilesConfig: { extensions: extensions || ['.js'], - filter: allowlist || ['!coverage', '!test'], + allowlist: allowlist || ['!coverage', '!test'], }, gatherFilesConfigReference: { extensions: extensions || ['.js'], - filter: allowlistReference || ['!coverage', '!test'], + allowlist: allowlistReference || ['!coverage', '!test'], }, queryMethod: 'ast', report: false, diff --git a/packages/providence-analytics/src/program/services/InputDataService.js b/packages/providence-analytics/src/program/services/InputDataService.js index d8b8fd6d3..d44849944 100644 --- a/packages/providence-analytics/src/program/services/InputDataService.js +++ b/packages/providence-analytics/src/program/services/InputDataService.js @@ -281,7 +281,7 @@ class InputDataService { static get defaultGatherFilesConfig() { return { extensions: ['.js'], - filter: ['!node_modules/**', '!bower_components/**', '!**/*.conf.js', '!**/*.config.js'], + allowlist: ['!node_modules/**', '!bower_components/**', '!**/*.conf.js', '!**/*.config.js'], depth: Infinity, }; } @@ -312,8 +312,11 @@ class InputDataService { ...this.defaultGatherFilesConfig, ...customConfig, }; - if (!customConfig.omitDefaultFilter) { - cfg.filter = [...this.defaultGatherFilesConfig.filter, ...(customConfig.filter || [])]; + if (!customConfig.omitDefaultAllowlist) { + cfg.allowlist = [ + ...this.defaultGatherFilesConfig.allowlist, + ...(customConfig.allowlist || []), + ]; } const allowlistModes = ['npm', 'git', 'all']; if (customConfig.allowlistMode && !allowlistModes.includes(customConfig.allowlistMode)) { @@ -338,12 +341,12 @@ class InputDataService { const removeFilter = gitIgnorePaths; const keepFilter = npmPackagePaths; - cfg.filter.forEach(filterEntry => { - const { negated, pattern } = isNegatedGlob(filterEntry); + cfg.allowlist.forEach(allowEntry => { + const { negated, pattern } = isNegatedGlob(allowEntry); if (negated) { removeFilter.push(pattern); } else { - keepFilter.push(filterEntry); + keepFilter.push(allowEntry); } }); @@ -351,19 +354,34 @@ class InputDataService { globPattern += `.{${cfg.extensions.map(e => e.slice(1)).join(',')},}`; const globRes = multiGlobSync(globPattern); - const filteredGlobRes = globRes.filter(filePath => { - const localFilePath = filePath.replace(`${startPath}/`, ''); - if (removeFilter.length) { - const remove = anymatch(removeFilter, localFilePath); - if (remove) { + let filteredGlobRes; + if (removeFilter.length || keepFilter.length) { + filteredGlobRes = globRes.filter(filePath => { + const localFilePath = filePath.replace(`${startPath}/`, ''); + let shouldRemove = removeFilter.length && anymatch(removeFilter, localFilePath); + let shouldKeep = keepFilter.length && anymatch(keepFilter, localFilePath); + + if (shouldRemove && shouldKeep) { + // Contradicting configs: the one defined by end user takes highest precedence + // If the match came from allowListMode, it loses. + if (allowlistMode === 'git' && anymatch(gitIgnorePaths, localFilePath)) { + // shouldRemove was caused by .gitignore, shouldKeep by custom allowlist + shouldRemove = false; + } else if (allowlistMode === 'npm' && anymatch(npmPackagePaths, localFilePath)) { + // shouldKeep was caused by npm "files", shouldRemove by custom allowlist + shouldKeep = false; + } + } + + if (removeFilter.length && shouldRemove) { return false; } - } - if (!keepFilter.length) { - return true; - } - return anymatch(keepFilter, localFilePath); - }); + if (!keepFilter.length) { + return true; + } + return shouldKeep; + }); + } if (!filteredGlobRes || !filteredGlobRes.length) { LogService.warn(`No files found for path '${startPath}'`); } diff --git a/packages/providence-analytics/test-node/cli/cli.test.js b/packages/providence-analytics/test-node/cli/cli.test.js index 227189c78..fbd067483 100644 --- a/packages/providence-analytics/test-node/cli/cli.test.js +++ b/packages/providence-analytics/test-node/cli/cli.test.js @@ -229,7 +229,7 @@ describe('Providence CLI', () => { it('"-a --allowlist"', async () => { await runCli(`${analyzeCmd} -a /mocked/path/example-project`, rootDir); expect(pathsArrayFromCsStub.args[0][0]).to.equal('/mocked/path/example-project'); - expect(providenceStub.args[0][1].gatherFilesConfig.filter).to.eql([ + expect(providenceStub.args[0][1].gatherFilesConfig.allowlist).to.eql([ '/mocked/path/example-project', ]); @@ -238,7 +238,7 @@ describe('Providence CLI', () => { await runCli(`${analyzeCmd} --allowlist /mocked/path/example-project`, rootDir); expect(pathsArrayFromCsStub.args[0][0]).to.equal('/mocked/path/example-project'); - expect(providenceStub.args[0][1].gatherFilesConfig.filter).to.eql([ + expect(providenceStub.args[0][1].gatherFilesConfig.allowlist).to.eql([ '/mocked/path/example-project', ]); }); @@ -246,7 +246,7 @@ describe('Providence CLI', () => { it('"--allowlist-reference"', async () => { await runCli(`${analyzeCmd} --allowlist-reference /mocked/path/example-project`, rootDir); expect(pathsArrayFromCsStub.args[0][0]).to.equal('/mocked/path/example-project'); - expect(providenceStub.args[0][1].gatherFilesConfigReference.filter).to.eql([ + expect(providenceStub.args[0][1].gatherFilesConfigReference.allowlist).to.eql([ '/mocked/path/example-project', ]); }); diff --git a/packages/providence-analytics/test-node/program/services/InputDataService.test.js b/packages/providence-analytics/test-node/program/services/InputDataService.test.js index d653388d8..910647446 100644 --- a/packages/providence-analytics/test-node/program/services/InputDataService.test.js +++ b/packages/providence-analytics/test-node/program/services/InputDataService.test.js @@ -121,7 +121,7 @@ describe('InputDataService', () => { it('allows passing excluded folders', async () => { const globOutput = InputDataService.gatherFilesFromDir('/fictional/project', { extensions: ['.html', '.js'], - filter: ['!nested/**'], + allowlist: ['!nested/**'], }); expect(globOutput).to.eql([ '/fictional/project/index.html', @@ -135,7 +135,7 @@ describe('InputDataService', () => { it('allows passing excluded files', async () => { const globOutput = InputDataService.gatherFilesFromDir('/fictional/project', { extensions: ['.html', '.js'], - filter: ['!index.js', '!**/*/index.js'], + allowlist: ['!index.js', '!**/*/index.js'], }); expect(globOutput).to.eql([ '/fictional/project/index.html', @@ -149,7 +149,7 @@ describe('InputDataService', () => { it('allows passing exclude globs', async () => { const globOutput = InputDataService.gatherFilesFromDir('/fictional/project', { extensions: ['.html', '.js'], - filter: ['!**/*.test.{html,js}'], + allowlist: ['!**/*.test.{html,js}'], }); expect(globOutput).to.eql([ '/fictional/project/index.html', @@ -183,7 +183,7 @@ describe('InputDataService', () => { './added/file.js': '', }); const globOutput = InputDataService.gatherFilesFromDir('/fictional/project', { - filter: ['*', 'added/**/*'], + allowlist: ['*', 'added/**/*'], }); expect(globOutput).to.eql([ '/fictional/project/added/file.js', @@ -311,7 +311,24 @@ build/ ]); }); - describe('Default filter', () => { + it('custom "allowlist" will take precedence over "allowlistMode"', async () => { + mockProject({ + './dist/bundle.js': '', // generated by build step + './src/a.js': '', + './src/b.js': '', + '.gitignore': '/dist', // Because we have a .gitignore, allowlistMode will be git + './package.json': JSON.stringify({ + files: ['dist'], // This will not be considered by default, unless explicitly configured in allowlist + }), + }); + const globOutput = InputDataService.gatherFilesFromDir('/fictional/project', { + allowlist: ['dist/**'], + allowlistMode: 'git', // for clarity, (would also be autodetected if not provided) + }); + expect(globOutput).to.eql(['/fictional/project/dist/bundle.js']); + }); + + describe('Default allowlist', () => { it('merges default config filter with configured filter', async () => { mockProject({ './node_modules/root-lvl.js': '', @@ -320,7 +337,7 @@ build/ './omit.js': '', }); const globOutput = InputDataService.gatherFilesFromDir('/fictional/project', { - filter: ['added*'], + allowlist: ['added*'], }); expect(globOutput).to.eql(['/fictional/project/added.js']); }); @@ -335,8 +352,8 @@ build/ './omit.js': '', }); const globOutput = InputDataService.gatherFilesFromDir('/fictional/project', { - filter: ['!omit*'], - omitDefaultFilter: true, + allowlist: ['!omit*'], + omitDefaultAllowlist: true, }); expect(globOutput).to.eql([ '/fictional/project/abc.config.js',