Clarify problemEdges logic

Author: bonkydog

lib/arborist/build-ideal-tree.js

@@ -768,12 +768,43 @@ This is a one-time fix-up, please be patient...
     const bundled = new Set(bd || [])
 
     return [...node.edgesOut.values()]
-      .filter(edge => !bundled.has(edge.name) &&
-        !(edge.to && this[_loadFailures].has(edge.to)) &&
-        !(edge.to && edge.to.inShrinkwrap) &&
-        (!edge.valid || !edge.to || this[_updateNames].includes(edge.name) ||
-          this[_isVulnerable](edge.to) ||
-          node.isRoot && this[_explicitRequests].has(edge.name)))
+      .filter(edge => {
+
+        // If it's included in a bundle, we take whatever is specified.
+        if (bundled.has(edge.name))
+          return false
+
+        // If it's already been logged as a load failure, skip it.
+        if (edge.to && this[_loadFailures].has(edge.to))
+          return false
+
+        // If it's shrinkwrapped, we use what the shrinkwap wants.
+        if (edge.to && edge.to.inShrinkwrap)
+          return false
+
+        // If the edge has an error, there's a problem.
+        if (!edge.valid)
+          return true
+
+        // If the edge has no destination, that's a problem.
+        if (!edge.to)
+          return true
+
+        // If user has explicitly asked to update this package by name, it's a problem.
+        if (this[_updateNames].includes(edge.name))
+          return true
+
+        // If we're fixing a security vulnerability with this package, it's a problem.
+        if (this[_isVulnerable](edge.to))
+          return true
+
+        // If the user has explicitly asked to install this package, it's a problem.
+        if (node.isRoot && this[_explicitRequests].has(edge.name))
+          return true
+
+        // No problems!
+        return false
+      })
   }
 
   async [_fetchManifest] (spec) {