feat: report all errors at once, if any

dominykas · Mar 14, 2019 · 467c6e7 · 467c6e7
1 parent 1036a6c
commit 467c6e7
Show file tree

Hide file tree

Showing 2 changed files with 38 additions and 16 deletions.
diff --git a/lib/index.js b/lib/index.js
@@ -1,5 +1,6 @@
 'use strict';
 
+const Assert = require('assert');
 const Cp = require('child_process');
 const Fs = require('fs');
 const Npm = require('libnpm');
@@ -115,7 +116,7 @@ exports.run = async (options) => {
 
     const allowScripts = pkg.allowScripts || {};
 
-    const allowedPackages = queue
+    const packagesWithScripts = queue
         .map((path) => {
 
             const childPkg = require(Path.join(cwd, path, 'package.json'));
@@ -125,36 +126,57 @@ exports.run = async (options) => {
         .filter(({ childPkg }) => {
 
             return childPkg.scripts && (childPkg.scripts.install || childPkg.scripts.preinstall || childPkg.scripts.postinstall);
-        })
-        .filter(({ path, childPkg }) => {
+        });
+
+    const allowedPackages = [];
+    const warnings = [];
+    const errors = [];
 
-            const name = childPkg.name;
+    packagesWithScripts
+        .forEach((entry) => {
+
+            const path = entry.path;
+            const childPkg = entry.childPkg;
+            const name = entry.childPkg.name;
 
             if (allowScripts[name] === undefined) {
-                throw new Error(`No entry for ${name}`);
+                errors.push(`${name} (no entry)`);
+                return;
             }
 
             if (allowScripts[name] === false) {
-                console.warn(`==========> skip ${path} (because it is not allowed in package.json)`);
-                return false;
+                warnings.push(`==========> skip ${path} (because it is not allowed in package.json)`);
+                return;
             }
 
             if (allowScripts[name] === true) {
-                return true;
+                allowedPackages.push(entry);
+                return;
             }
 
             if (!Semver.validRange(allowScripts[name])) {
-                throw new Error(`Invalid version range in allowScripts[${name}]: ${allowScripts[name]}`);
+                errors.push(`${name} (invalid semver range)`);
+                return;
             }
 
-            if (!Semver.satisfies(childPkg.version, allowScripts[name])) {
-                console.warn(`==========> skip ${path} (because ${childPkg.version} is outside of allowed range: ${allowScripts[name]})`);
-                return false;
+            if (!Semver.satisfies(entry.childPkg.version, allowScripts[name])) {
+                warnings.push(`==========> skip ${path} (because ${childPkg.version} is outside of allowed range: ${allowScripts[name]})`);
+                return;
             }
 
-            return true;
+            allowedPackages.push(entry);
         });
 
+    Assert.strictEqual(packagesWithScripts.length, errors.length + warnings.length + allowedPackages.length, 'Package count does not match. Please raise an issue at https://github.com/dominykas/allow-scripts');
+
+    if (errors.length) {
+        throw new Error(`Mis-configured allowedScripts: ${errors.join(', ')}`);
+    }
+
+    if (warnings.length) {
+        console.warn(warnings.join('\n'));
+    }
+
     await internals.runScript('preinstall', { pkg, path: '', cwd, unsafePerm: true }, options);
 
     for (const { path, childPkg } of allowedPackages) {

diff --git a/test/index.js b/test/index.js
@@ -146,7 +146,7 @@ describe('allow-scripts', () => {
                 'without-scripts'
             ]);
 
-            await expect(Allow.run({})).to.reject('No entry for @example/with-install-script');
+            await expect(Allow.run({})).to.reject('Mis-configured allowedScripts: @example/with-install-script (no entry)');
 
             expect(fixture.getActualResult()).to.equal('');
             expect(fixture.getLog()).to.equal('');
@@ -190,7 +190,7 @@ describe('allow-scripts', () => {
                 'without-scripts'
             ]);
 
-            await expect(Allow.run({})).to.reject('Invalid version range in allowScripts[@example/with-install-script]: not-a-semver-range');
+            await expect(Allow.run({})).to.reject('Mis-configured allowedScripts: @example/with-install-script (invalid semver range)');
 
             expect(fixture.getActualResult()).to.equal('');
             expect(fixture.getLog()).to.equal('');
@@ -204,7 +204,7 @@ describe('allow-scripts', () => {
                 'without-scripts'
             ]);
 
-            await expect(Allow.run({})).to.reject('No entry for @example/with-install-script');
+            await expect(Allow.run({})).to.reject('Mis-configured allowedScripts: @example/with-install-script (no entry), @example/with-postinstall-script (no entry)');
 
             expect(fixture.getActualResult()).to.equal('');
             expect(fixture.getLog()).to.equal('');