Merge pull request #18 from dominykas/list-all

Report all errors at once
dominykas · Mar 14, 2019 · c86dc14 · c86dc14
2 parents 1036a6c + 04c92ba
commit c86dc14
Show file tree

Hide file tree

Showing 4 changed files with 48 additions and 17 deletions.
diff --git a/bin/allow-scripts.js b/bin/allow-scripts.js
@@ -2,6 +2,8 @@
 
 'use strict';
 
+const Ansicolors = require('ansicolors');
+
 const Allow = require('..');
 
 const options = {
@@ -10,6 +12,13 @@ const options = {
 
 Allow.run(options).catch((err) => {
 
-    console.error(err);
+    let log = err;
+    if (err.stack) {
+
+        const [message, ...rest] = err.stack.split('\n');
+        log = `${Ansicolors.red(message)}\n${rest.join('\n')}`;
+    }
+
+    console.error(`\n\n${log}`);
     process.exit(1);
 });
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,56 @@ 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, childPkg } = entry;
+            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: ${allowScripts[name]})`);
+                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/package.json b/package.json
@@ -16,6 +16,7 @@
   "author": "Dominykas Blyžė <hello@dominykas.com>",
   "license": "MIT",
   "dependencies": {
+    "ansicolors": "0.3.x",
     "libnpm": "2.x.x",
     "semver": "5.x.x",
     "topo": "3.x.x"

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: not-a-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('');