cargo-rail: fixing the schema/config for the 'undeclared features; resolution/fixing; added tests for it

Author: loadingalias

docs/config.md

@@ -109,6 +109,7 @@ Controls workspace dependency unification behavior. All options are optional wit
 | `preserve_features` | `string[]` | `[]` | Features to preserve from dead feature pruning. Supports glob patterns (e.g., `"unstable-*"`, `"bench*"`). Use this to keep features intended for future use or external consumers. |
 | `detect_undeclared_features` | `bool` | `true` | Detect crates that rely on Cargo's feature unification to "borrow" features from other workspace members. These crates will fail when built standalone after unification. Reports as warnings (or auto-fixes if `fix_undeclared_features` is enabled). |
 | `fix_undeclared_features` | `bool` | `true` | Auto-fix undeclared feature dependencies by adding missing features to each crate's Cargo.toml. Produces a cleaner graph where standalone builds work correctly. Requires `detect_undeclared_features = true`. |
+| `skip_undeclared_patterns` | `string[]` | `["default", "std", "alloc", "*_backend", "*_impl"]` | Patterns for features to skip in undeclared feature detection. Supports glob patterns. Default patterns filter out features that are typically not actionable (standard library features, internal implementation details). |
 | `max_backups` | `usize` | `3` | Maximum number of backup archives to keep. Older backups are automatically cleaned up after successful operations. Set to `0` to disable backup creation entirely. |
 
 **Example:**
@@ -123,6 +124,7 @@ prune_dead_features = true
 preserve_features = ["future-api", "unstable-*"]  # Keep these from pruning
 detect_undeclared_features = true  # Catch borrowed features
 fix_undeclared_features = true    # Auto-fix them (default)
+skip_undeclared_patterns = ["default", "std", "alloc", "*_backend", "*_impl"]  # Features to skip
 max_backups = 5
 ```
 
@@ -470,6 +472,9 @@ detect_unused = true
 remove_unused = true
 prune_dead_features = true
 preserve_features = []  # Glob patterns: ["unstable-*", "future-api"]
+detect_undeclared_features = true  # Catch borrowed features
+fix_undeclared_features = true     # Auto-fix them
+skip_undeclared_patterns = ["default", "std", "alloc", "*_backend", "*_impl"]
 max_backups = 3
 
 # Version handling

src/cargo/unify_analyzer.rs

@@ -593,6 +593,15 @@ impl UnifyAnalyzer {
   /// unification, standalone builds (cargo test -p <crate>) will fail because
   /// the borrowed features are no longer available.
   ///
+  /// Key distinctions:
+  /// - **Workspace policy**: Features in `[workspace.dependencies]` are intentional
+  ///   workspace-level declarations. Members using these don't need to re-declare.
+  /// - **Member borrowing**: Features that only come from another member are
+  ///   implicit coupling and should be flagged.
+  /// - **Conditional features**: Features declared via `[features]` table (e.g.,
+  ///   `my-feature = ["dep/feat"]`) count as declared - the author made an
+  ///   intentional choice.
+  ///
   /// Example:
   /// - crate A declares: `backoff = "0.4"` (no features)
   /// - crate B declares: `backoff = { version = "0.4", features = ["tokio"] }`
@@ -602,21 +611,32 @@ impl UnifyAnalyzer {
   fn detect_undeclared_features(&self) -> Vec<UndeclaredFeature> {
     let mut undeclared = Vec::new();
 
-    // Common features that are typically not actionable:
-    // - "default", "std" - usually implicit
-    // - Internal features like "*_backend", "*_impl" - implementation details
-    let should_skip_feature = |f: &str| -> bool {
-      f == "default" || f == "std" || f == "alloc" || f.ends_with("_backend") || f.ends_with("_impl")
-    };
+    // Step 1: Get workspace baseline (features declared in [workspace.dependencies])
+    // These are workspace policy - members don't need to re-declare them
+    let workspace_baseline: HashMap<String, HashSet<String>> = self
+      .existing_workspace_deps
+      .iter()
+      .map(|(name, dep)| {
+        let mut feats: HashSet<String> = dep.features.iter().cloned().collect();
+        if dep.default_features {
+          feats.insert("default".to_string());
+        }
+        (name.clone(), feats)
+      })
+      .collect();
 
-    // Build a map of what each member explicitly declares for each dependency
-    // We want to find cases where member A would need a feature that ONLY comes from member B
+    // Step 2: Build a map of what each member explicitly declares for each dependency
+    // Include both unconditional_features AND conditional_features (from [features] table)
+    // Also track which features each member contributes (for borrowed_from)
     let mut member_declared_features: HashMap<String, HashMap<String, HashSet<String>>> = HashMap::new();
 
     for member in &self.manifests.members {
       let mut dep_features: HashMap<String, HashSet<String>> = HashMap::new();
       for (dep_key, usage) in &member.dependencies {
         let mut feats: HashSet<String> = usage.unconditional_features.iter().cloned().collect();
+        // Include conditional features (from [features] table like `my-feat = ["dep/feature"]`)
+        // The author made an intentional choice - this counts as declared
+        feats.extend(usage.conditional_features.iter().cloned());
         // If default_features = true (default), add "default"
         if usage.default_features {
           feats.insert("default".to_string());
@@ -626,49 +646,87 @@ impl UnifyAnalyzer {
       member_declared_features.insert(member.package_name.clone(), dep_features);
     }
 
-    // Compute the unified features (what will end up in workspace.dependencies)
-    // This is the UNION of all declared features across all members
-    let mut unified_features: HashMap<String, HashSet<String>> = HashMap::new();
-    for dep_features in member_declared_features.values() {
+    // Step 3: Track which members contribute which features (beyond workspace baseline)
+    // This powers the `borrowed_from` field for transparency
+    // Structure: dep_name -> feature -> set of member names
+    let mut feature_sources: HashMap<String, HashMap<String, HashSet<String>>> = HashMap::new();
+
+    for (member_name, dep_features) in &member_declared_features {
       for (dep_name, features) in dep_features {
-        unified_features
-          .entry(dep_name.clone())
-          .or_default()
-          .extend(features.iter().cloned());
+        let baseline = workspace_baseline.get(dep_name);
+
+        for feat in features {
+          // Skip features that are in workspace baseline (workspace policy)
+          if baseline.is_some_and(|b| b.contains(feat)) {
+            continue;
+          }
+
+          feature_sources
+            .entry(dep_name.clone())
+            .or_default()
+            .entry(feat.clone())
+            .or_default()
+            .insert(member_name.clone());
+        }
       }
     }
 
-    // Now check: for each member, are there features in unified that it doesn't declare?
-    // These are features "borrowed" from other workspace members
+    // Step 4: Find borrowed features for each member
+    // A feature is borrowed if:
+    // - It's contributed by OTHER members (not this member)
+    // - It's NOT in workspace baseline (workspace policy doesn't count as borrowing)
+    // - It passes the skip filter (configurable via skip_undeclared_patterns)
     for member in &self.manifests.members {
       for (dep_key, usage) in &member.dependencies {
-        let Some(unified) = unified_features.get(&*dep_key.name) else {
+        let Some(feat_sources) = feature_sources.get(&*dep_key.name) else {
           continue;
         };
 
-        // Get declared features from this member's Cargo.toml
+        // Get this member's declared features (unconditional + conditional)
         let mut declared: HashSet<String> = usage.unconditional_features.iter().cloned().collect();
+        declared.extend(usage.conditional_features.iter().cloned());
         if usage.default_features {
           declared.insert("default".to_string());
         }
 
-        // Find features that are unified but not declared by this member
-        // These are features this member will lose access to after unification
-        let borrowed: Vec<String> = unified
-          .difference(&declared)
-          .filter(|f| !should_skip_feature(f))
-          .cloned()
-          .collect();
+        // Find features this member borrows from others
+        let mut borrowed_features: Vec<String> = Vec::new();
+        let mut borrowed_from_members: HashSet<String> = HashSet::new();
+
+        for (feat, sources) in feat_sources {
+          // Skip if this member declares the feature itself
+          if declared.contains(feat) {
+            continue;
+          }
+
+          // Skip if configured to skip this feature pattern
+          if self.config.should_skip_undeclared_feature(feat) {
+            continue;
+          }
+
+          // This feature is borrowed from other members
+          borrowed_features.push(feat.clone());
+          for source in sources {
+            if source != &member.package_name {
+              borrowed_from_members.insert(source.clone());
+            }
+          }
+        }
+
+        if !borrowed_features.is_empty() {
+          borrowed_features.sort();
+          let mut borrowed_from: Vec<String> = borrowed_from_members.into_iter().collect();
+          borrowed_from.sort();
 
-        if !borrowed.is_empty() {
           undeclared.push(UndeclaredFeature {
             member: member.package_name.clone(),
             dep_name: dep_key.name.to_string(),
-            undeclared_features: borrowed,
+            undeclared_features: borrowed_features,
             declared_features: declared.into_iter().collect(),
-            resolved_features: unified.iter().cloned().collect(),
+            resolved_features: feat_sources.keys().cloned().collect(),
             dep_kind: usage.kind,
             target: usage.target.clone(),
+            borrowed_from,
           });
         }
       }

src/cargo/unify_types.rs

@@ -212,6 +212,9 @@ pub struct UndeclaredFeature {
   pub dep_kind: DepKind,
   /// Target platform constraint (if in target-specific section)
   pub target: Option<String>,
+  /// Which workspace members provide the borrowed features
+  /// Shows where the feature is coming from (for transparency)
+  pub borrowed_from: Vec<String>,
 }
 
 // ============================================================================
@@ -482,12 +485,17 @@ impl UnificationPlan {
         self.undeclared_features.len()
       ));
       for uf in &self.undeclared_features {
+        let borrowed_from_str = if uf.borrowed_from.is_empty() {
+          String::new()
+        } else {
+          format!(" (borrowed from {})", uf.borrowed_from.join(", "))
+        };
         s.push_str(&format!(
-          "  - {} in {}: needs features [{}] but only declares [{}]\n",
-          uf.dep_name,
+          "  - {}/{}: [{}]{}\n",
           uf.member,
+          uf.dep_name,
           uf.undeclared_features.join(", "),
-          uf.declared_features.join(", ")
+          borrowed_from_str
         ));
       }
       s.push_str("  These crates rely on feature unification from other workspace members.\n");
@@ -594,6 +602,7 @@ mod tests {
       resolved_features: vec!["default".to_string(), "futures".to_string(), "tokio".to_string()],
       dep_kind: DepKind::Normal,
       target: None,
+      borrowed_from: vec!["other-crate".to_string()],
     };
 
     assert_eq!(uf.member, "my-crate");
@@ -603,6 +612,7 @@ mod tests {
     assert_eq!(uf.resolved_features.len(), 3);
     assert_eq!(uf.dep_kind, DepKind::Normal);
     assert!(uf.target.is_none());
+    assert_eq!(uf.borrowed_from.len(), 1);
   }
 
   #[test]
@@ -615,6 +625,7 @@ mod tests {
       resolved_features: vec!["extra_traits".to_string()],
       dep_kind: DepKind::Normal,
       target: Some("cfg(unix)".to_string()),
+      borrowed_from: vec!["unix-crate".to_string()],
     };
 
     assert_eq!(uf.target, Some("cfg(unix)".to_string()));
@@ -630,6 +641,7 @@ mod tests {
       resolved_features: vec!["rt".to_string(), "macros".to_string()],
       dep_kind: DepKind::Dev,
       target: None,
+      borrowed_from: vec!["main-crate".to_string()],
     };
 
     assert_eq!(uf.dep_kind, DepKind::Dev);
@@ -746,6 +758,7 @@ mod tests {
         resolved_features: vec!["futures".to_string()],
         dep_kind: DepKind::Normal,
         target: None,
+        borrowed_from: vec!["other-crate".to_string()],
       }],
     };
 
@@ -778,6 +791,7 @@ mod tests {
         resolved_features: vec!["futures".to_string()],
         dep_kind: DepKind::Normal,
         target: None,
+        borrowed_from: vec!["other-crate".to_string()],
       }],
     };
 
@@ -839,12 +853,84 @@ mod tests {
       resolved_features: vec!["f1".to_string(), "f2".to_string()],
       dep_kind: DepKind::Dev,
       target: Some("cfg(windows)".to_string()),
+      borrowed_from: vec!["source-crate".to_string()],
     };
 
     let cloned = uf.clone();
     assert_eq!(uf.member, cloned.member);
     assert_eq!(uf.dep_name, cloned.dep_name);
     assert_eq!(uf.undeclared_features, cloned.undeclared_features);
     assert_eq!(uf.target, cloned.target);
+    assert_eq!(uf.borrowed_from, cloned.borrowed_from);
+  }
+
+  #[test]
+  fn test_summary_shows_borrowed_from_in_undeclared_warning() {
+    let plan = UnificationPlan {
+      workspace_deps: vec![],
+      member_edits: std::collections::HashMap::new(),
+      member_paths: std::collections::HashMap::new(),
+      transitive_pins: vec![],
+      validation_results: vec![],
+      issues: vec![],
+      computed_msrv: None,
+      duplicates_cleaned: vec![],
+      pruned_features: vec![],
+      optional_features: vec![],
+      version_mismatches: vec![],
+      unused_deps: vec![],
+      undeclared_features: vec![UndeclaredFeature {
+        member: "my-crate".to_string(),
+        dep_name: "tokio".to_string(),
+        undeclared_features: vec!["macros".to_string(), "rt".to_string()],
+        declared_features: vec![],
+        resolved_features: vec!["macros".to_string(), "rt".to_string()],
+        dep_kind: DepKind::Normal,
+        target: None,
+        borrowed_from: vec!["other-crate".to_string(), "third-crate".to_string()],
+      }],
+    };
+
+    // No AddFeatures edits = warn mode with borrowed_from
+    let summary = plan.summary();
+    assert!(summary.contains("⚠️  Undeclared features detected"));
+    assert!(summary.contains("my-crate/tokio"));
+    assert!(summary.contains("macros"));
+    assert!(summary.contains("rt"));
+    assert!(summary.contains("(borrowed from other-crate, third-crate)"));
+  }
+
+  #[test]
+  fn test_summary_borrowed_from_empty_not_shown() {
+    let plan = UnificationPlan {
+      workspace_deps: vec![],
+      member_edits: std::collections::HashMap::new(),
+      member_paths: std::collections::HashMap::new(),
+      transitive_pins: vec![],
+      validation_results: vec![],
+      issues: vec![],
+      computed_msrv: None,
+      duplicates_cleaned: vec![],
+      pruned_features: vec![],
+      optional_features: vec![],
+      version_mismatches: vec![],
+      unused_deps: vec![],
+      undeclared_features: vec![UndeclaredFeature {
+        member: "my-crate".to_string(),
+        dep_name: "backoff".to_string(),
+        undeclared_features: vec!["futures".to_string()],
+        declared_features: vec![],
+        resolved_features: vec!["futures".to_string()],
+        dep_kind: DepKind::Normal,
+        target: None,
+        borrowed_from: vec![], // Empty - shouldn't show "borrowed from"
+      }],
+    };
+
+    let summary = plan.summary();
+    assert!(summary.contains("my-crate/backoff"));
+    assert!(summary.contains("futures"));
+    // Should NOT contain "borrowed from" when empty
+    assert!(!summary.contains("borrowed from"));
   }
 }