fix: warn message on engine mismatch

- Fixed setting engine check warnings to each package
- Added proper catching of package warnings during validation phase
- Added tap test that validates print of engine warn msgs

Fix: https://npm.community/t/engines-and-engines-strict-ignored/4792

PR-URL: #257
Credit: @ruyadorno
Close: #257
Reviewed-by: @isaacs

Author: ruyadorno

lib/install/validate-args.js

@@ -31,10 +31,14 @@ function hasMinimumFields (pkg, cb) {
   }
 }
 
-function getWarnings (pkg) {
-  while (pkg.parent) pkg = pkg.parent
+function setWarnings (pkg, warn) {
   if (!pkg.warnings) pkg.warnings = []
-  return pkg.warnings
+  if (pkg.warnings.every(i => (
+    i.code !== warn.code ||
+    i.required !== warn.required ||
+    i.pkgid !== warn.pkgid))) {
+    pkg.warnings.push(warn)
+  }
 }
 
 var isInstallable = module.exports.isInstallable = function (pkg, next) {
@@ -48,7 +52,7 @@ var isInstallable = module.exports.isInstallable = function (pkg, next) {
   var strict = npm.config.get('engine-strict')
   checkEngine(pkg, npm.version, nodeVersion, force, strict, iferr(next, thenWarnEngineIssues))
   function thenWarnEngineIssues (warn) {
-    if (warn) getWarnings(pkg).push(warn)
+    if (warn) setWarnings(pkg, warn)
     checkPlatform(pkg, force, next)
   }
 }

lib/install/validate-tree.js

@@ -22,7 +22,8 @@ module.exports = function (idealTree, log, next) {
     [asyncMap, modules, function (mod, done) {
       chain([
         mod.parent && !mod.isLink && [checkGit, mod.realpath],
-        [checkErrors, mod, idealTree]
+        [checkErrors, mod, idealTree],
+        [checkWarnings, mod, idealTree]
       ], done)
     }],
     [thenValidateAllPeerDeps, idealTree],
@@ -36,6 +37,14 @@ function checkErrors (mod, idealTree, next) {
   next()
 }
 
+function checkWarnings (mod, idealTree, next) {
+  const warnings = mod.package.warnings
+  if (warnings && (mod.parent || path.resolve(npm.globalDir, '..'))) {
+    warnings.forEach(warn => idealTree.warnings.push(warn))
+  }
+  next()
+}
+
 function thenValidateAllPeerDeps (idealTree, next) {
   validate('OF', arguments)
   validateAllPeerDeps(idealTree, function (tree, pkgname, version) {

test/tap/check-engine-reqs.js

@@ -27,7 +27,6 @@ test('setup', function (t) {
 
 var INSTALL_OPTS = ['--loglevel', 'silly']
 var EXEC_OPTS = {cwd: installIn}
-
 test('install bad engine', function (t) {
   common.npm(['install', '--engine-strict', installFrom].concat(INSTALL_OPTS), EXEC_OPTS, function (err, code) {
     t.ifError(err, 'npm ran without issue')
@@ -43,6 +42,18 @@ test('force install bad engine', function (t) {
   })
 })
 
+test('warns on bad engine not strict', function (t) {
+  common.npm(['install', '--json', installFrom], EXEC_OPTS, function (err, code, stdout, stderr) {
+    t.ifError(err, 'npm ran without issue')
+    t.is(code, 0, 'result code')
+    var result = JSON.parse(stdout)
+    t.match(result.warnings[1], /Unsupported engine/, 'reason for optional failure in JSON')
+    t.match(result.warnings[1], /1.0.0-not-a-real-version/, 'should print mismatch version info')
+    t.match(result.warnings[1], /Not compatible with your version of node/, 'incompatibility message')
+    t.done()
+  })
+})
+
 test('cleanup', function (t) {
   cleanup()
   t.end()

test/tap/validate-tree-check-warnings.js

@@ -0,0 +1,44 @@
+'use strict'
+var test = require('tap').test
+var log = require('npmlog')
+var npm = require('../../lib/npm.js')
+var checkEngine = require('npm-install-checks').checkEngine
+
+var idealTree = {
+  package: {
+    name: 'a b c',
+    version: '3.what'
+  },
+  children: [{
+    name: 'faulty-engine',
+    version: '0.0.1',
+    children: [],
+    engines: {
+      node: '>=2.0.0'
+    },
+    package: {
+      name: 'faulty-engine',
+      version: '0.0.1'
+    }
+  }],
+  warnings: []
+}
+
+test('setup', function (t) {
+  const faultyEnginePkg = idealTree.children[0]
+  checkEngine(faultyEnginePkg, '1.0.0', '1.0.0', false, false, (err, warn) => {
+    t.ifError(err, 'check engine ran without issue')
+    faultyEnginePkg.package.warnings = [warn]
+    npm.load({}, t.end)
+  })
+})
+
+test('validate-tree should collect warnings from modules', function (t) {
+  log.disableProgress()
+  var validateTree = require('../../lib/install/validate-tree.js')
+  validateTree(idealTree, log.newGroup('validate'), function (er, a, b) {
+    t.equal(idealTree.warnings[0].code, 'ENOTSUP', 'should have the correct error')
+    t.match(idealTree.warnings[0].message, /Unsupported engine/, 'reason for optional failure in JSON')
+    t.end()
+  })
+})