Fix bug with features and optional dependencies (#45)

* Add bug repro

* Fix feature bug

* Add --no-dev to example

* Add direct_dependencies

* Add tests, always keep sorted

examples/graph.rs

@@ -12,6 +12,8 @@ struct Args {
     all_features: bool,
     #[arg(long)]
     no_default_features: bool,
+    #[arg(long)]
+    no_dev: bool,
 }
 
 pub struct Simple {
@@ -54,7 +56,11 @@ fn main() {
         cmd
     };
 
-    let builder = krates::Builder::new();
+    let mut builder = krates::Builder::new();
+    if args.no_dev {
+        builder.ignore_kind(krates::DepKind::Dev, krates::Scope::All);
+    }
+
     let graph: Graph = builder.build(cmd, krates::NoneFilter).unwrap();
 
     let dot = krates::petgraph::dot::Dot::new(graph.graph()).to_string();

src/builder.rs

@@ -949,7 +949,7 @@ impl Builder {
                         rnode.features.binary_search_by(|f| f.as_str().cmp("default")).is_ok()
                     };
 
-                    let edges: Vec<_> = rdep.dep_kinds.iter().filter_map(move |dk| {
+                    let edges: Vec<_> = rdep.dep_kinds.iter().filter_map(|dk| {
                         let mask = match dk.kind {
                             DepKind::Normal => 0x1,
                             DepKind::Dev => 0x8,
@@ -980,11 +980,13 @@ impl Builder {
                         // up as resolved in the graph.
                         // https://github.com/EmbarkStudios/krates/issues/41
                         // https://github.com/rust-lang/cargo/issues/10801
-                        if dep.optional && !rnode
-                                .features
+                        if dep.optional && !enabled_features
                                 .iter()
-                                .any(|feat| *feat == rdep.name || *feat == maybe_real_name) {
-                            //println!("skipping {}", rdep.name);
+                                .any(|(_feat, sub_feats)| {
+                                    sub_feats.iter().any(|sf| {
+                                        sf.kid == &rdep.pkg
+                                    })
+                                }) {
                             return None;
                         }
 

src/lib.rs

@@ -270,11 +270,43 @@ impl<N, E> Krates<N, E> {
             })
     }
 
+    /// Gets crates directly depended upon by the specified node
+    #[inline]
+    pub fn direct_dependencies(&self, nid: NodeId) -> Vec<DirectDependency<'_, N>> {
+        let graph = self.graph();
+        let mut direct_dependencies = Vec::new();
+        let mut stack = vec![nid];
+        let mut visited = std::collections::BTreeSet::new();
+
+        while let Some(nid) = stack.pop() {
+            for edge in graph.edges_directed(nid, Direction::Outgoing) {
+                match &self.graph[edge.target()] {
+                    Node::Krate { krate, .. } => {
+                        if visited.insert(edge.target()) {
+                            direct_dependencies.push(DirectDependency {
+                                krate,
+                                node_id: edge.target(),
+                                edge_id: edge.id(),
+                            });
+                        }
+                    }
+                    Node::Feature { .. } => {
+                        if visited.insert(edge.target()) {
+                            stack.push(edge.target());
+                        }
+                    }
+                }
+            }
+        }
+
+        direct_dependencies
+    }
+
     /// Gets the crates that have a direct dependency on the specified node
     #[inline]
     pub fn direct_dependents(&self, nid: NodeId) -> Vec<DirectDependent<'_, N>> {
         let graph = self.graph();
-        let mut direct_dependencies = Vec::new();
+        let mut direct_dependents = Vec::new();
         let mut stack = vec![nid];
         let mut visited = std::collections::BTreeSet::new();
 
@@ -283,7 +315,7 @@ impl<N, E> Krates<N, E> {
                 match &self.graph[edge.source()] {
                     Node::Krate { krate, .. } => {
                         if visited.insert(edge.source()) {
-                            direct_dependencies.push(DirectDependent {
+                            direct_dependents.push(DirectDependent {
                                 krate,
                                 node_id: edge.source(),
                                 edge_id: edge.id(),
@@ -299,7 +331,7 @@ impl<N, E> Krates<N, E> {
             }
         }
 
-        direct_dependencies
+        direct_dependents
     }
 
     /// Get the node identifier for the specified crate identifier
@@ -365,6 +397,16 @@ impl<N, E> Krates<N, E> {
     }
 }
 
+/// A direct dependency of a crate
+pub struct DirectDependency<'krates, N> {
+    /// The crate in the node
+    pub krate: &'krates N,
+    /// The crate's node id
+    pub node_id: NodeId,
+    /// The edge that links the crate with the crate that depends on it
+    pub edge_id: EdgeId,
+}
+
 /// A crate that has a direct dependency on another crate
 pub struct DirectDependent<'krates, N> {
     /// The crate in the node

tests/bug/Cargo.toml

@@ -0,0 +1,9 @@
+[package]
+name = "bug"
+version = "0.1.0"
+edition = "2021"
+
+[dependencies.wasmtime]
+version = "2.0.0"
+default-features = false
+features = ["cranelift", "incremental-cache", "parallel-compilation", "vtune"]

tests/bug/src/lib.rs

@@ -0,0 +1 @@
+

tests/misc.rs

@@ -145,15 +145,56 @@ fn direct_dependents() {
         .find(|k| k.0.repr.starts_with("reqwest"))
         .unwrap();
 
-    let dd = grafs
+    let mut ids: Vec<_> = grafs
         .actual
-        .direct_dependents(grafs.actual.nid_for_kid(&id.0).unwrap())
+        .direct_dependencies(grafs.actual.nid_for_kid(&id.0).unwrap())
         .into_iter()
-        .fold(String::new(), |mut acc, jid| {
-            acc.push_str(&jid.krate.0.repr);
-            acc.push('\n');
-            acc
-        });
+        .map(|jid| jid.krate.0.repr.as_str())
+        .collect();
+
+    ids.sort();
+    let dd = ids.join("\n");
 
     insta::assert_snapshot!(dd);
 }
+
+#[test]
+fn direct_dependencies() {
+    let mut kb = krates::Builder::new();
+    kb.include_targets(std::iter::once((
+        krates::cfg_expr::targets::get_builtin_target_by_triple("x86_64-unknown-linux-gnu")
+            .unwrap()
+            .triple
+            .clone(),
+        vec![],
+    )));
+
+    let grafs = util::build("direct.json", kb).unwrap();
+
+    let id = grafs
+        .actual
+        .krates()
+        .find(|k| k.0.repr.starts_with("reqwest"))
+        .unwrap();
+
+    let mut ids: Vec<_> = grafs
+        .actual
+        .direct_dependencies(grafs.actual.nid_for_kid(&id.0).unwrap())
+        .into_iter()
+        .map(|jid| jid.krate.0.repr.as_str())
+        .collect();
+
+    ids.sort();
+    let dd = ids.join("\n");
+
+    insta::assert_snapshot!(dd);
+}
+
+#[test]
+fn bug_repro() {
+    let kb = krates::Builder::new();
+
+    let grafs = util::build("bug.json", kb).unwrap();
+
+    insta::assert_snapshot!(grafs.dotgraph());
+}