Prefer satisfying engines req, refactor heuristic

This refactors a lot of the logic in this module, so that it's a
somewhat less haphazard collection of conditionals.

Logic now:

- Include policyRestrictions in the search if present.
- Include stagedVersions in the search if present, and includeStaged set
- If a dist-tag is specified, and it's not after the 'before' date, pull
  it from the versions, staged, or restricted.
- If a dist-tag is specified, and it IS after the 'before' date, then
  get the highest precedence version that is less than or equal to the
  dist-tagged version
- If a version is specified, return it from the
  versions/restricted/staged set if before the 'before' date.  If it's
  not old enough, raise ETARGET.  (This is a breaking change from v5,
  where specifying a definite version number would the override 'before'
  setting.)
- If the default dist-tag satisfies the range (or if the range is `'*'`
  or `''`) then return the default dist-tag version if it's older than
  the 'before' setting.

At this point, we know it's a range, and have to apply heuristics to
find the best fit.  Sort all the available versions by:

- Whether or not it's in the policyRestrictions versions.  (All
  non-restricted versions come before all restricted versions.)
- Whether or not it's a staged version.  (Properly published versions
  take precedence over staged versions.)
- All versions that are not deprecated, and where the declared engine
  requirements are met.  (OS/CPU are *not* checked, since they are
  much less unlikely to change over time in a given module, and will
  crash the install anyway.)
- All versions where the engine requirements are met (but which may be
  deprecated).
- Deprecated versions
- Beyond that (within a given set of all the above heuristics), sort by
  SemVer precedence.

Then we take the first thing on the list.

If there are no versions (or none created before the `before` option),
then `ENOVERSIONS` is raised.

If the version selected is in the policyRestrictions set, then `E403` is
raised.

If there are versions, but none of them satisfy the given requirements,
then `ETARGET` is raised.

BREAKING CHANGES:

- Deprecated versions MAY be returned if there are no other options, but
  will always be avoided if possible.  (Along with this, the
  `includeDeprecated` flag no longer is relevant.)
- Unsatisfied stated engines requirements are avoided if possible.
- Remove the hole in `before` filtering, where declaring a specific
  version would work, even if it was published _after_ the date.

FEATURE CHANGES:

- Add support for stagedVersions
- Add the `includeStaged` flag

index.js

@@ -2,133 +2,154 @@
 
 const npa = require('npm-package-arg')
 const semver = require('semver')
+const { checkEngine } = require('npm-install-checks')
+
+const engineOk = (manifest, npmVersion, nodeVersion) => {
+  try {
+    checkEngine(manifest, npmVersion, nodeVersion)
+    return true
+  } catch (_) {
+    return false
+  }
+}
+
+const isBefore = (verTimes, ver, time) =>
+  !verTimes || Date.parse(verTimes[ver]) <= time
 
-module.exports = pickManifest
-function pickManifest (packument, wanted, opts = {}) {
+const pickManifest = (packument, wanted, opts) => {
   const {
     defaultTag = 'latest',
     before = null,
-    includeDeprecated = false
+    nodeVersion = process.version,
+    npmVersion = null,
+    includeStaged = false
   } = opts
 
-  const time = before && packument.time && +(new Date(before))
-  const spec = npa.resolve(packument.name, wanted)
+  const { name, time: verTimes } = packument
+  const versions = packument.versions || {}
+  const staged = (includeStaged && packument.stagedVersions &&
+    packument.stagedVersions.versions) || {}
+  const restricted = (packument.policyRestrictions &&
+    packument.policyRestrictions.versions) || {}
+
+  const time = before && verTimes ? Date.parse(before) : Infinity
+  const spec = npa.resolve(name, wanted || defaultTag)
   const type = spec.type
-  if (type === 'version' || type === 'range') {
-    wanted = semver.clean(wanted, true) || wanted
-  }
   const distTags = packument['dist-tags'] || {}
-  const versions = Object.keys(packument.versions || {}).filter(v => {
-    return semver.valid(v, true)
-  })
-  const policyRestrictions = packument.policyRestrictions
-  const restrictedVersions = policyRestrictions
-    ? Object.keys(policyRestrictions.versions) : []
 
-  function enjoyableBy (v) {
-    return !time || (
-      packument.time[v] && time >= +(new Date(packument.time[v]))
-    )
+  if (type !== 'tag' && type !== 'version' && type !== 'range') {
+    throw new Error('Only tag, version, and range are supported')
   }
 
-  let err
-
-  if (!versions.length && !restrictedVersions.length) {
-    err = new Error(`No valid versions available for ${packument.name}`)
-    err.code = 'ENOVERSIONS'
-    err.name = packument.name
-    err.type = type
-    err.wanted = wanted
-    throw err
+  // if the type is 'tag', and not just the implicit default, then it must
+  // be that exactly, or nothing else will do.
+  if (wanted && type === 'tag') {
+    const ver = distTags[wanted]
+    // if the version in the dist-tags is before the before date, then
+    // we use that.  Otherwise, we get the highest precedence version
+    // prior to the dist-tag.
+    if (isBefore(verTimes, ver, time)) {
+      return versions[ver] || staged[ver] || restricted[ver]
+    } else {
+      return pickManifest(packument, `<=${ver}`, opts)
+    }
   }
 
-  let target
-
-  if (type === 'tag' && enjoyableBy(distTags[wanted])) {
-    target = distTags[wanted]
-  } else if (type === 'version') {
-    target = wanted
-  } else if (type !== 'range' && enjoyableBy(distTags[wanted])) {
-    throw new Error('Only tag, version, and range are supported')
+  // similarly, if a specific version, then only that version will do
+  if (wanted && type === 'version') {
+    const ver = semver.clean(wanted, { loose: true })
+    const mani = versions[ver] || staged[ver] || restricted[ver]
+    return isBefore(verTimes, ver, time) ? mani : null
   }
 
-  const tagVersion = distTags[defaultTag]
+  // ok, sort based on our heuristics, and pick the best fit
+  const range = type === 'range' ? wanted : '*'
 
-  if (
-    !target &&
-    tagVersion &&
-    packument.versions[tagVersion] &&
-    enjoyableBy(tagVersion) &&
-    semver.satisfies(tagVersion, wanted, true)
-  ) {
-    target = tagVersion
+  // if the range is *, then we prefer the 'latest' if available
+  const defaultVer = distTags[defaultTag]
+  if (defaultVer && (range === '*' || semver.satisfies(defaultVer, range))) {
+    const mani = versions[defaultVer]
+    if (mani && isBefore(verTimes, defaultVer, time)) {
+      return mani
+    }
   }
 
-  if (!target && !includeDeprecated) {
-    const undeprecated = versions.filter(v => !packument.versions[v].deprecated && enjoyableBy(v)
-    )
-    target = semver.maxSatisfying(undeprecated, wanted, true)
-  }
-  if (!target) {
-    const stillFresh = versions.filter(enjoyableBy)
-    target = semver.maxSatisfying(stillFresh, wanted, true)
+  // ok, actually have to sort the list and take the winner
+  const allEntries = Object.entries(versions)
+    .concat(Object.entries(staged))
+    .concat(Object.entries(restricted))
+    .filter(([ver, mani]) => isBefore(verTimes, ver, time))
+
+  if (!allEntries.length) {
+    throw Object.assign(new Error(`No valid versions available for ${name}`), {
+      code: 'ENOVERSIONS',
+      name,
+      type,
+      wanted,
+      versions: Object.keys(versions)
+    })
   }
 
-  if (!target && wanted === '*' && enjoyableBy(tagVersion)) {
-    // This specific corner is meant for the case where
-    // someone is using `*` as a selector, but all versions
-    // are pre-releases, which don't match ranges at all.
-    target = tagVersion
-  }
+  const entries = allEntries.filter(([ver, mani]) =>
+    semver.satisfies(ver, range, { loose: true }))
+    .sort((a, b) => {
+      const [vera, mania] = a
+      const [verb, manib] = b
+      const notrestra = !restricted[a]
+      const notrestrb = !restricted[b]
+      const notstagea = !staged[a]
+      const notstageb = !staged[b]
+      const notdepra = !mania.deprecated
+      const notdeprb = !manib.deprecated
+      const enginea = engineOk(mania, npmVersion, nodeVersion)
+      const engineb = engineOk(manib, npmVersion, nodeVersion)
+      // sort by:
+      // - not restricted
+      // - not staged
+      // - not deprecated and engine ok
+      // - engine ok
+      // - not deprecated
+      // - semver
+      return (notrestrb - notrestra) ||
+        (notstageb - notstagea) ||
+        ((notdeprb && engineb) - (notdepra && enginea)) ||
+        (engineb - enginea) ||
+        (notdeprb - notdepra) ||
+        semver.rcompare(vera, verb)
+    })
+
+  return entries[0] && entries[0][1]
+}
 
-  if (
-    !target &&
-    time &&
-    type === 'tag' &&
-    distTags[wanted] &&
-    !enjoyableBy(distTags[wanted])
-  ) {
-    const stillFresh = versions.filter(v =>
-      enjoyableBy(v) && semver.lte(v, distTags[wanted], true)
-    ).sort(semver.rcompare)
-    target = stillFresh[0]
-  }
+module.exports = (packument, wanted, opts = {}) => {
+  const picked = pickManifest(packument, wanted, opts)
+  const policyRestrictions = packument.policyRestrictions
+  const restricted = (policyRestrictions && policyRestrictions.versions) || {}
 
-  if (!target && restrictedVersions) {
-    target = semver.maxSatisfying(restrictedVersions, wanted, true)
+  if (picked && !restricted[picked.version]) {
+    return picked
   }
 
-  const manifest = (
-    target &&
-    packument.versions[target]
-  )
-  if (!manifest) {
-    // Check if target is forbidden
-    const isForbidden = target && policyRestrictions && policyRestrictions.versions[target]
-    const pckg = `${packument.name}@${wanted}${
-      before
-        ? ` with an Enjoy By date of ${
-          new Date(before).toLocaleString()
-        }. Maybe try a different date?`
-        : ''
-    }`
-
-    if (isForbidden) {
-      err = new Error(`Could not download ${pckg} due to policy violations.\n${policyRestrictions.message}\n`)
-      err.code = 'E403'
-    } else {
-      err = new Error(`No matching version found for ${pckg}.`)
-      err.code = 'ETARGET'
-    }
-
-    err.name = packument.name
-    err.type = type
-    err.wanted = wanted
-    err.versions = versions
-    err.distTags = distTags
-    err.defaultTag = defaultTag
-    throw err
-  } else {
-    return manifest
-  }
+  const { before = null, defaultTag = 'latest' } = opts
+  const bstr = before ? new Date(before).toLocaleString() : ''
+  const { name } = packument
+  const pckg = `${name}@${wanted}` +
+    (before ? ` with a date before ${bstr}` : '')
+
+  const isForbidden = picked && !!restricted[picked.version]
+  const polMsg = isForbidden ? policyRestrictions.message : ''
+
+  const msg = !isForbidden ? `No matching version found for ${pckg}.`
+    : `Could not download ${pckg} due to policy violations:\n${polMsg}`
+
+  const code = isForbidden ? 'E403' : 'ETARGET'
+  throw Object.assign(new Error(msg), {
+    code,
+    type: npa.resolve(packument.name, wanted).type,
+    wanted,
+    versions: Object.keys(packument.versions),
+    name,
+    distTags: packument['dist-tags'],
+    defaultTag
+  })
 }

package.json

@@ -28,6 +28,7 @@
   "license": "ISC",
   "dependencies": {
     "figgy-pudding": "^3.5.1",
+    "npm-install-checks": "^4.0.0",
     "npm-package-arg": "^8.0.0",
     "semver": "^7.0.0"
   },