fix(arborist): pass the edge to fromPath in order to determine correct path (#5233)

by passing in the edge we can determine if the edge is overridden, and if it is the path we want to return is the project root since that's what user's will have define their overrides relative to

3 files changed, 60 insertions, 25 deletions

diff --git a/workspaces/arborist/test/dep-valid.js b/workspaces/arborist/test/dep-valid.js
index 901325b23..8c0e76165 100644
--- a/workspaces/arborist/test/dep-valid.js
+++ b/workspaces/arborist/test/dep-valid.js
@@ -4,7 +4,12 @@ const npa = require('npm-package-arg')
 const { normalizePaths } = require('./fixtures/utils.js')
 const { resolve } = require('path')
 
-t.ok(depValid({}, '', null, {}), '* is always ok')
+// dep-valid reads from requestor.edgesOut so we use this instead of {} in these tests
+const emptyRequestor = {
+  edgesOut: new Map(),
+}
+
+t.ok(depValid({}, '', null, emptyRequestor), '* is always ok')
 
 t.ok(depValid({
   package: {
@@ -13,7 +18,7 @@ t.ok(depValid({
   get version () {
     return this.package.version
   },
-}, '1.x', null, {}), 'range that is satisfied')
+}, '1.x', null, emptyRequestor), 'range that is satisfied')
 
 t.ok(depValid({
   package: {
@@ -22,21 +27,21 @@ t.ok(depValid({
   get version () {
     return this.package.version
   },
-}, '1.x', '2.x', {}), 'range that is acceptable')
+}, '1.x', '2.x', emptyRequestor), 'range that is acceptable')
 
 t.ok(depValid({
   isLink: true,
   realpath: '/some/path',
-}, normalizePaths(npa('file:/some/path')), null, {}), 'links must point at intended target')
+}, normalizePaths(npa('file:/some/path')), null, emptyRequestor), 'links must point at intended target')
 
 t.notOk(depValid({
   isLink: true,
   realpath: '/some/other/path',
-}, 'file:/some/path', null, {}), 'links must point at intended target')
+}, 'file:/some/path', null, emptyRequestor), 'links must point at intended target')
 
 t.notOk(depValid({
   realpath: '/some/path',
-}, 'file:/some/path', null, {}), 'file:// must be a link')
+}, 'file:/some/path', null, emptyRequestor), 'file:// must be a link')
 
 t.ok(depValid({
   name: 'foo',
@@ -47,7 +52,7 @@ t.ok(depValid({
   get version () {
     return this.package.version
   },
-}, 'git://host/repo#semver:1.x', null, {}), 'git url with semver range')
+}, 'git://host/repo#semver:1.x', null, emptyRequestor), 'git url with semver range')
 
 t.ok(depValid({
   name: 'foo',
@@ -58,7 +63,7 @@ t.ok(depValid({
   get version () {
     return this.package.version
   },
-}, 'npm:bar@1.2.3', null, {}), 'alias is ok')
+}, 'npm:bar@1.2.3', null, emptyRequestor), 'alias is ok')
 
 t.ok(depValid({
   resolved: 'https://registry/abbrev-1.1.1.tgz',
@@ -66,7 +71,7 @@ t.ok(depValid({
   get version () {
     return this.package.version
   },
-}, 'https://registry/abbrev-1.1.1.tgz', null, {}), 'remote url match')
+}, 'https://registry/abbrev-1.1.1.tgz', null, emptyRequestor), 'remote url match')
 
 t.ok(depValid({
   resolved: 'git+ssh://git@github.com/foo/bar',
@@ -74,7 +79,7 @@ t.ok(depValid({
   get version () {
     return this.package.version
   },
-}, 'git+ssh://git@github.com/foo/bar.git', null, {}), 'matching _from saveSpec')
+}, 'git+ssh://git@github.com/foo/bar.git', null, emptyRequestor), 'matching _from saveSpec')
 
 t.notOk(depValid({
   resolved: 'git+ssh://git@github.com/foo/bar',
@@ -82,26 +87,26 @@ t.notOk(depValid({
   get version () {
     return this.package.version
   },
-}, 'git+ssh://git@github.com/bar/foo.git', null, {}), 'different repo')
+}, 'git+ssh://git@github.com/bar/foo.git', null, emptyRequestor), 'different repo')
 
 t.notOk(depValid({
   package: {},
   get version () {
     return this.package.version
   },
-}, 'git+ssh://git@github.com/bar/foo.git', null, {}), 'missing repo')
+}, 'git+ssh://git@github.com/bar/foo.git', null, emptyRequestor), 'missing repo')
 
 t.ok(depValid({
   resolved: `file:${resolve('/path/to/tarball.tgz')}`,
-}, resolve('/path/to/tarball.tgz'), null, {}), 'same tarball')
+}, resolve('/path/to/tarball.tgz'), null, emptyRequestor), 'same tarball')
 
 t.notOk(depValid({
   resolved: 'file:/path/to/other/tarball.tgz',
-}, '/path/to/tarball.tgz', null, {}), 'different tarball')
+}, '/path/to/tarball.tgz', null, emptyRequestor), 'different tarball')
 
 t.notOk(depValid({
   isLink: true,
-}, '/path/to/tarball.tgz', null, {}), 'links are not tarballs')
+}, '/path/to/tarball.tgz', null, emptyRequestor), 'links are not tarballs')
 
 t.ok(depValid({
   package: {
@@ -112,28 +117,28 @@ t.ok(depValid({
   get version () {
     return this.package.version
   },
-}, './tarball.tgz', null, {}), 'probably the same-ish, hopefully')
+}, './tarball.tgz', null, emptyRequestor), 'probably the same-ish, hopefully')
 
 t.notOk(depValid({
   package: {},
   get version () {
     return this.package.version
   },
-}, './tarball.tgz', null, {}), 'too uncertain, nope')
+}, './tarball.tgz', null, emptyRequestor), 'too uncertain, nope')
 
 t.ok(depValid({
   resolved: 'https://registry.npmjs.org/foo/foo-1.2.3.tgz',
-}, 'latest', null, {}), 'tagged registry version needs remote tarball')
+}, 'latest', null, emptyRequestor), 'tagged registry version needs remote tarball')
 
 t.notOk(depValid({
   resolved: 'git+https://registry.npmjs.org/foo/foo-1.2.3.git',
-}, 'latest', null, {}), 'tagged registry version needs remote tarball, not git')
+}, 'latest', null, emptyRequestor), 'tagged registry version needs remote tarball, not git')
 
-t.notOk(depValid({}, 'latest', null, {}),
+t.notOk(depValid({}, 'latest', null, emptyRequestor),
   'tagged registry version needs remote tarball resolution')
 
 t.test('unsupported dependency type', t => {
-  const requestor = { errors: [] }
+  const requestor = { errors: [], edgesOut: new Map() }
   const child = { name: 'kid' }
   const request = { type: 'not a type' }
   t.notOk(depValid(child, request, null, requestor))
@@ -148,7 +153,7 @@ t.test('unsupported dependency type', t => {
 })
 
 t.test('invalid tag name', t => {
-  const requestor = { errors: [] }
+  const requestor = { errors: [], edgesOut: new Map() }
   const child = { name: 'kid' }
   const request = '!!@#$%!#@$!'
   t.notOk(depValid(child, request, null, requestor))
@@ -163,7 +168,7 @@ t.test('invalid tag name', t => {
 })
 
 t.test('invalid request all together', t => {
-  const requestor = { errors: [] }
+  const requestor = { errors: [], edgesOut: new Map() }
   const child = { name: 'kid' }
   const request = null
   t.notOk(depValid(child, request, null, requestor))
@@ -178,7 +183,7 @@ t.test('invalid request all together', t => {
 })
 
 t.test('installLinks makes Link nodes invalid', t => {
-  const requestor = { errors: [], installLinks: true }
+  const requestor = { errors: [], installLinks: true, edgesOut: new Map() }
   const child = { isLink: true, name: 'kid' }
   const request = { type: 'directory' }
   t.notOk(depValid(child, request, null, requestor))
diff --git a/workspaces/arborist/test/edge.js b/workspaces/arborist/test/edge.js
index 9d1352351..f45cb8791 100644
--- a/workspaces/arborist/test/edge.js
+++ b/workspaces/arborist/test/edge.js
@@ -71,6 +71,7 @@ const a = {
   },
   isTop: false,
   parent: top,
+  root: top,
   resolve (n) {
     return n === 'aa' ? aa : this.parent.resolve(n)
   },
@@ -1008,6 +1009,7 @@ t.test('shrinkwrapped and bundled deps are not overridden and remain valid', (t)
       return this.package.version
     },
     parent: root,
+    root,
     resolve (n) {
       return n === 'bar' ? bar : this.parent.resolve(n)
     },
@@ -1036,6 +1038,7 @@ t.test('shrinkwrapped and bundled deps are not overridden and remain valid', (t)
       return this.package.version
     },
     parent: foo,
+    root,
     resolve (n) {
       return this.parent.resolve(n)
     },
diff --git a/workspaces/arborist/test/from-path.js b/workspaces/arborist/test/from-path.js
index 5b5ea2084..3bfd0b639 100644
--- a/workspaces/arborist/test/from-path.js
+++ b/workspaces/arborist/test/from-path.js
@@ -1,7 +1,7 @@
 const t = require('tap')
 const { normalizePath } = require('./fixtures/utils.js')
 const fp = require('../lib/from-path.js')
-const fromPath = obj => normalizePath(fp(obj))
+const fromPath = (node, edge) => normalizePath(fp(node, edge))
 
 t.equal(fromPath({
   realpath: '/some/path',
@@ -21,3 +21,30 @@ t.equal(fromPath({
   realpath: '/some/path/to/install/target',
   resolved: 'https://registry.com/package.tgz',
 }), '/some/path/to/install/target', 'use dirname if not dir or file type')
+
+t.equal(fromPath({
+  root: {
+    realpath: '/some/root',
+  },
+}, {
+  name: 'foo',
+  overrides: {
+    name: 'foo',
+    value: 'foo@2',
+  },
+}), '/some/root', 'uses root realpath for overridden edges')
+
+t.equal(fromPath({
+  sourceReference: {
+    root: {
+      realpath: '/some/root',
+    },
+  },
+  realpath: '/some/red/herring',
+}, {
+  name: 'foo',
+  overrides: {
+    name: 'foo',
+    value: 'foo@2',
+  },
+}), '/some/root', 'uses sourceReferences root realpath for overridden edges')