fix(providence-analytics): allowlist takes precedence over -mode

.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

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,

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,

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,32 +341,47 @@ 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);
       }
     });
 
     let globPattern = this.getGlobPattern(startPath, cfg);
     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}'`);
     }

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,15 +238,15 @@ 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',
       ]);
     });
 
     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',
       ]);
     });

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',