Let the narrower behavior win where both landed

crates/infact-rust-behaviors/src/derivation.rs

@@ -158,6 +158,18 @@ fn collect_functions<'a>(
         "trait_item" => node
             .child_by_field_name("name")
             .and_then(|name| node_text(name, &file.source)),
+        // A function declared inside another function's body is a local helper,
+        // not a method of whatever type surrounds them both. Carrying the
+        // container down made every such helper answer to the same name a
+        // sibling method does, and the resolver refuses a name two callables
+        // answer to — so a method with a perfectly good body reported that no
+        // implementation was found.
+        //
+        // `Iterator::fold` is the case that matters: `max_by` and `min_by`
+        // declare their own `fn fold` helpers, so three callables claimed the
+        // name and none of them resolved. Everything built on `fold` went with
+        // it — `count`, `sum`, `last`, `max`, `min`, `reduce`, `product`, `nth`.
+        "function_item" => None,
         _ => container,
     };
     // an inline `mod` that is not public hides everything inside it

crates/infact-rust-behaviors/src/lib.rs

@@ -228,7 +228,11 @@ pub fn analyze_repository(
                     .and_modify(|chosen| chosen.0.push(behavior))
                     .or_insert_with(|| (vec![behavior], fused));
             }
-            for (mut sharing, fused) in best.into_values() {
+            // Where each matched behavior actually lands, worked out before
+            // anything is reported, because which behavior to report is a
+            // question about what else matched THERE.
+            let mut found = Vec::new();
+            for (program, (mut sharing, fused)) in best {
                 // Only callables whose catalog is present can be named.
                 sharing.retain(|behavior| catalog_for(catalogs, behavior).is_some());
                 // Name the API a caller should reach for, which is the one
@@ -248,9 +252,6 @@ pub fn analyze_repository(
                 let Some((behavior, rest)) = sharing.split_first() else {
                     continue;
                 };
-                let Some(catalog) = catalog_for(catalogs, behavior) else {
-                    continue;
-                };
                 let alternatives: Vec<LibraryTarget> = rest
                     .iter()
                     .filter_map(|other| {
@@ -267,23 +268,40 @@ pub fn analyze_repository(
                 // Reporting only the first surfaced the rest one re-run at a
                 // time, and a reader fixing the one they were shown had no way
                 // to know the others existed.
-                let mut located = function.form.locate_all(&behavior.program);
+                let mut located = function.form.locate_all(program);
                 if located.is_empty() {
-                    located = candidate.locate_all(&behavior.program);
+                    located = candidate.locate_all(program);
                 }
                 // A behavior that matched but cannot be placed is still a
                 // finding; it just has no span to point at.
-                let placements = if located.is_empty() {
+                let placements: Vec<Option<std::ops::Range<usize>>> = if located.is_empty() {
                     vec![None]
                 } else {
                     located.into_iter().map(Some).collect()
                 };
+                found.push((program, *behavior, alternatives, fused, placements));
+            }
+            for (index, (program, behavior, alternatives, fused, placements)) in
+                found.iter().enumerate()
+            {
+                let catalog = match catalog_for(catalogs, behavior) {
+                    Some(catalog) => catalog,
+                    None => continue,
+                };
                 for steps in placements {
+                    // A behavior that another matched behavior is broader than
+                    // says strictly less about this code than that one does,
+                    // and saying both is saying the weaker thing twice.
+                    if found.iter().enumerate().any(|(other, entry)| {
+                        other != index && entry.4.contains(steps) && is_broader(program, entry.0)
+                    }) {
+                        continue;
+                    }
                     matches.insert(behavior_match(
                         file,
                         &function,
-                        steps,
-                        fused,
+                        steps.clone(),
+                        *fused,
                         catalog,
                         behavior,
                         alternatives.clone(),
@@ -347,6 +365,24 @@ fn delegates_to(outer: &DerivedLibraryBehavior, inner: &DerivedLibraryBehavior)
             .any(|step| step.callable_path == target)
 }
 
+/// Whether one behavior's form describes everything another's does, and more.
+///
+/// A form used as a pattern accepts wherever it matches, so a form that matches
+/// ANOTHER behavior's form accepts everywhere that one does and elsewhere
+/// besides. `Option::and_then` is `match self { Some(x) => f(x), None => None }`
+/// and the hole swallows what every narrower way of consuming an `Option` puts
+/// there — `map`, `filter`, `ok_or`. Measured on clippy's `manual_map` test it
+/// landed on fifteen of the same lines `map` did, saying less about each.
+///
+/// Being broader is not being wrong, and it is not grounds for leaving a
+/// behavior out of a pack: code that really does reimplement `and_then` should
+/// hear about it. It is only grounds for standing aside where something
+/// narrower has already landed, which is why this is asked per placement rather
+/// than once per pack.
+fn is_broader(broad: &Form, narrow: &Form) -> bool {
+    broad != narrow && narrow.contains(broad) && !broad.contains(narrow)
+}
+
 fn catalog_for<'a>(
     catalogs: &'a [ExternalCatalog],
     behavior: &DerivedLibraryBehavior,

crates/infact-rust-behaviors/tests/collisions.rs

@@ -114,3 +114,60 @@ fn distinct_callables_derive_distinct_behaviors() {
         erased.join("\n  ")
     );
 }
+
+/// A behavior another one is broader than stands aside where that one landed.
+///
+/// `Option::and_then` is `match self { Some(x) => f(x), None => None }`, and the
+/// hole swallows what every narrower way of consuming an `Option` puts there.
+/// Measured on clippy's `manual_map` test it landed on fifteen of the same lines
+/// `Option::map` did, saying less about each.
+#[test]
+fn a_broader_behavior_stands_aside_where_a_narrower_one_landed() {
+    use infact_core::Form;
+    use infact_normalize::{Arm, Pattern};
+
+    let hole_applied = || Form::Call {
+        callee: Box::new(Form::Free(1)),
+        arguments: vec![Form::Local(0)],
+    };
+    let none = || Form::Variant {
+        name: "None".to_owned(),
+        payload: Vec::new(),
+    };
+    let consuming = |taken: Form| {
+        Form::select(
+            Form::Free(0),
+            vec![
+                Arm {
+                    pattern: Pattern::Variant {
+                        name: "Some".to_owned(),
+                        parts: vec![Pattern::Binding(0)],
+                    },
+                    body: taken,
+                },
+                Arm {
+                    pattern: Pattern::Variant {
+                        name: "None".to_owned(),
+                        parts: Vec::new(),
+                    },
+                    body: none(),
+                },
+            ],
+        )
+    };
+    // `and_then` hands back whatever the caller's function returned; `map`
+    // wraps it. The first accepts the second and not the other way about.
+    let and_then = consuming(hole_applied());
+    let map = consuming(Form::Variant {
+        name: "Some".to_owned(),
+        payload: vec![hole_applied()],
+    });
+    assert!(
+        map.contains(&and_then),
+        "and_then must accept map, or it is not the broader of the two"
+    );
+    assert!(
+        !and_then.contains(&map),
+        "map must not accept and_then, or neither is broader"
+    );
+}

crates/infact-rust-behaviors/tests/fixtures/nested-helper/src/lib.rs

@@ -0,0 +1,26 @@
+pub trait Walk {
+    fn items(&self) -> Vec<i32>;
+
+    /// A provided method with a real body and a name nothing else declares —
+    /// except a helper nested inside a sibling, below.
+    fn total(&self) -> i32 {
+        let mut sum = 0;
+        for value in self.items() {
+            sum += value;
+        }
+        sum
+    }
+
+    /// Declares its own `total`, the way `Iterator::max_by` declares its own
+    /// `fold`.
+    fn biggest(&self) -> i32 {
+        fn total(left: i32, right: i32) -> i32 {
+            if left > right { left } else { right }
+        }
+        let mut best = 0;
+        for value in self.items() {
+            best = total(best, value);
+        }
+        best
+    }
+}

crates/infact-rust-behaviors/tests/fixtures/subsumption/src/lib.rs

@@ -0,0 +1,10 @@
+pub fn mapped(value: Option<i32>) -> Option<i32> {
+    match value {
+        Some(inner) => Some(double(inner)),
+        None => None,
+    }
+}
+
+fn double(value: i32) -> i32 {
+    value * 2
+}

crates/infact-rust-behaviors/tests/resolution.rs

@@ -80,3 +80,38 @@ fn a_constructor_still_reaches_its_own_types_method() {
         "`keys` should carry its own Cursor::next, which reads `self.inner`:\n  {form}"
     );
 }
+
+/// A helper nested in a method body does not answer to the surrounding type.
+///
+/// Carried down, the container made a local `fn total` a second callable named
+/// `Walk::total`, and the resolver refuses a name two callables answer to — so
+/// the real `total`, with a perfectly good body, reported that no
+/// implementation was found.
+///
+/// `Iterator::fold` is the case this was found on: `max_by` and `min_by`
+/// declare their own `fn fold` helpers, and three callables claimed the name.
+#[test]
+fn a_helper_nested_in_a_method_does_not_shadow_its_sibling() {
+    let crate_root = std::path::PathBuf::from(env!("CARGO_MANIFEST_DIR"));
+    let parsers =
+        entl_tree_sitter::ParserCatalog::discover([crate_root.join("../../../entl/parser-packs")]);
+    assert!(parsers.errors.is_empty(), "{:?}", parsers.errors);
+
+    let derived = infact_rust_behaviors::derive_library(
+        crate_root.join("tests/fixtures/nested-helper"),
+        &parsers.catalog,
+        "probe",
+        "0.1.0",
+    )
+    .expect("deriving the fixture");
+
+    let paths: std::collections::BTreeSet<&str> = derived
+        .behaviors
+        .iter()
+        .map(|behavior| behavior.callable_path.as_str())
+        .collect();
+    assert!(
+        paths.contains("probe::Walk::total"),
+        "the method the nested helper shares a name with must still resolve: {paths:?}"
+    );
+}