fix: node.isDescendantOf follows fs hierarchy

Previously, the `node.isDescendantOf` method would only check for direct
parents of that node, this may cause unexpected side effects on build
ideal tree and reify when working with linked dependencies.

This change modifies it so that `node.isDescendantOf` now looks at
`node.resolveParent` (making sure `fsParents` are included in the
lookup) to determine wether that node is a descendant of another.

This also made it possible to refactor the logic that checks for an
`external` dep in build-ideal-tree to use `node.isDescendantOf` instead
of a regex check in the realpath of the link.

Relates to: npm/cli#2900

lib/arborist/build-ideal-tree.js

@@ -1421,7 +1421,7 @@ This is a one-time fix-up, please be patient...
     // prune anything deeper in the tree that can be replaced by this
     if (this.idealTree) {
       for (const node of this.idealTree.inventory.query('name', newDep.name)) {
-        if (node.isDescendantOf(target))
+        if (!node.isTop && node.isDescendantOf(target))
           this[_pruneDedupable](node, false)
       }
     }
@@ -1819,7 +1819,7 @@ This is a one-time fix-up, please be patient...
     const current = target !== entryEdge.from && target.resolve(dep.name)
     if (current) {
       for (const edge of current.edgesIn.values()) {
-        if (edge.from.isDescendantOf(target) && edge.valid) {
+        if (!edge.from.isTop && edge.from.isDescendantOf(target) && edge.valid) {
           if (!edge.satisfiedBy(dep))
             return CONFLICT
         }
@@ -1876,7 +1876,8 @@ This is a one-time fix-up, please be patient...
       if (link.root !== this.idealTree)
         continue
 
-      const external = /^\.\.(\/|$)/.test(relpath(this.path, link.realpath))
+      const tree = this.idealTree.target || this.idealTree
+      const external = !link.target.isDescendantOf(tree)
 
       // outside the root, somebody else's problem, ignore it
       if (external && !this[_follow])

lib/arborist/reify.js

@@ -407,7 +407,7 @@ module.exports = cls => class Reifier extends cls {
     // node.parent is checked to make sure this is a node that's in the tree, and
     // not the parent-less top level nodes
     const filter = node =>
-      node.isDescendantOf(this.idealTree) &&
+      node.top.isProjectRoot &&
         (node.peer && this[_omitPeer] ||
           node.dev && this[_omitDev] ||
           node.optional && this[_omitOptional] ||

lib/node.js

@@ -409,7 +409,7 @@ class Node {
   }
 
   isDescendantOf (node) {
-    for (let p = this; p; p = p.parent) {
+    for (let p = this; p; p = p.resolveParent) {
       if (p === node)
         return true
     }
@@ -1197,6 +1197,14 @@ class Node {
     return this.isTop ? this : this.parent.top
   }
 
+  get isFsTop () {
+    return !this.fsParent
+  }
+
+  get fsTop () {
+    return this.isFsTop ? this : this.fsParent.fsTop
+  }
+
   get resolveParent () {
     return this.parent || this.fsParent
   }

test/arborist/audit.js

@@ -101,7 +101,7 @@ t.test('audit in a workspace', async t => {
   t.equal(auditReport.get('mkdirp').nodes.size, 1)
   t.strictSame(auditReport.toJSON().vulnerabilities.mkdirp.nodes, ['packages/a/node_modules/mkdirp'])
   t.equal(auditReport.get('minimist').nodes.size, 1)
-  t.strictSame(auditReport.toJSON().vulnerabilities.minimist.nodes, ['packages/a/node_modules/minimist'])
+  t.strictSame(auditReport.toJSON().vulnerabilities.minimist.nodes, ['node_modules/minimist'])
 
   const fixed = await newArb(path, { workspaces: ['b'] }).audit({ fix: true })
   t.equal(fixed.children.get('a').target.children.get('mkdirp').version, '0.5.0', 'did not fix a')

test/node.js

@@ -2496,6 +2496,12 @@ t.test('canDedupe()', t => {
 
   t.equal(top.children.get('a').canDedupe(), true)
 
+  // check fsTop and isDescendantOf
+  t.equal(top.isDescendantOf(root), true)
+  t.equal(top.isFsTop, false)
+  t.equal(top.fsTop, root)
+  t.equal(top.children.get('a').isFsTop, true)
+
   t.end()
 })