analysis: restore duplicate-FnKey tripwire tolerant of parse-recovered keys

CHANGELOG.md

@@ -322,6 +322,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - Whole-program call graph for the module hierarchy, keyed on the shared `FnKey` ([#63])
   - A035 (recursion) and A036 (stack depth) span files: cross-file `::` / `root::` call edges are resolved and an imported struct's frame is sized from its defining file, so cross-file recursion and >64 KB cross-file stack chains are caught instead of compiling and overflowing at runtime
   - The call graph indexes the structured `FnKey` from `inference-fn-key`, never a flattened name, so same-named functions across files stay distinct nodes
+- Restore the duplicate-`FnKey` tripwire in `resolve_adjacency`, now tolerant of parse-recovered keys ([#255])
+  - The LSP server ([#239]) rewrote `resolve_adjacency` to keep-first on any duplicate `FnKey` in every build, silently dropping the previous `debug_assert!(false)` that guarded `FnKey` injectivity; a genuine duplicate means a recursive self-edge can resolve to the wrong same-keyed node and mask a cycle from A035/A036 (the #63 canonical-key bug class)
+  - That removal was necessary because the resilient IDE path lowers every unparseable construct to an `<error>` placeholder function, so a broken parse legitimately yields two nodes under one key and the old assert aborted debug builds (and the LSP process) on it
+  - The tripwire now fires in debug builds only when the duplicate key carries no parser recovery marker (`is_parse_recovered`); recovered keys are exempt and the keep-first behavior is unchanged in every build, so release builds and resilient parses still degrade deterministically
 - Add `core/analysis/` crate with rule-based static analysis between type checking and codegen ([#156])
   - Five analysis rules: A001 break-outside-loop, A002 break-in-nondet, A003 return-in-loop, A004 infinite-loop-without-break, A005 return-in-nondet
   - `Rule` trait with `rule!` declarative macro for zero-boilerplate rule definitions
@@ -924,3 +928,5 @@ Initial tagged release.
 [#227]: https://github.com/Inferara/inference/issues/227
 [#217]: https://github.com/Inferara/inference/issues/217
 [#33]: https://github.com/Inferara/inference/issues/33
+[#239]: https://github.com/Inferara/inference/pull/239
+[#255]: https://github.com/Inferara/inference/issues/255

core/analysis/src/call_graph.rs

@@ -337,6 +337,42 @@
     }
 }
 
+/// The parser's error-recovery sentinel. Every unparseable top-level construct
+/// lowers to a placeholder `fn <error> { }`, and a recovered struct or spec name
+/// is synthesized the same way (see the parser's `create_error_definition` and
+/// `lower_name_or_error`). A [`FnKey`] built from such a definition carries this
+/// marker in one of its identity segments, which is how [`resolve_adjacency`]
+/// tells a benign resilient-path collision from a genuine identity break.
+const PARSE_RECOVERY_MARKER: &str = "<error>";
+
+/// Whether any identity segment of `key` is the parser's error-recovery
+/// placeholder — i.e. the key came from a parse-recovered definition, struct, or
+/// spec rather than a real one. The defining `module_path` is filesystem-derived
+/// and never synthesized by recovery, so only the name/struct/spec segments are
+/// inspected. Such keys legitimately collide on the resilient IDE path and are
+/// exempt from the duplicate-key tripwire in [`resolve_adjacency`].
+fn is_parse_recovered(key: &FnKey) -> bool {
+    match key {
+        FnKey::Free { name, .. } => name == PARSE_RECOVERY_MARKER,
+        FnKey::Method {
+            struct_name, name, ..
+        } => struct_name == PARSE_RECOVERY_MARKER || name == PARSE_RECOVERY_MARKER,
+        FnKey::SpecFree { spec, name, .. } => {
+            spec == PARSE_RECOVERY_MARKER || name == PARSE_RECOVERY_MARKER
+        }
+        FnKey::SpecMethod {
+            spec,
+            struct_name,
+            name,
+            ..
+        } => {
+            spec == PARSE_RECOVERY_MARKER
+                || struct_name == PARSE_RECOVERY_MARKER
+                || name == PARSE_RECOVERY_MARKER
+        }
+    }
+}
+
 /// Resolves each node's edges into a directed adjacency list of node indices,
 /// paired with the call-site location of the edge.
 ///
@@ -348,24 +384,44 @@
 /// (`Free`) target distinct nodes. If no candidate names an existing node the
 /// edge is dropped, so callers can never manufacture a false edge.
 pub(crate) fn resolve_adjacency(nodes: &[FnNode]) -> Vec<Vec<(usize, Location)>> {
-    // Build the key → index map, keeping the first node on a duplicate key.
+    // Build the key → index map, keeping the first node on a duplicate key and
+    // tripping a debug-build assertion on a duplicate that is *not* the product of
+    // parse recovery.
     //
     // On the fail-fast compiler path `FnKey` is injective: the type checker
     // rejects genuine duplicate definitions before analysis runs, so no two nodes
-    // share a key and this map is a plain bijection — the branch that discards a
-    // later duplicate is never taken, and A035/A036 outputs on valid programs are
-    // unchanged. The IDE resilient path has no such guarantee: error recovery
-    // lowers every unparseable top-level construct to an `<error>` placeholder
-    // function (a single stray `forall { }` lowers to two), and a program caught
-    // mid-edit can momentarily name the same function twice, so distinct nodes can
-    // carry the same key. Keeping the first node is a deterministic, total
+    // share a key and this map is a plain bijection. A duplicate that survives to
+    // here on that path is an upstream identity break — two real functions folded
+    // to one key — which is exactly the same-named-type / canonical-key bug class
+    // this repo has shipped (#63): a recursive function's self-edge can resolve to
+    // the wrong same-keyed node, masking the cycle from A035/A036. That warrants a
+    // loud failure in debug builds, so the tripwire fires unless the offending key
+    // is parse-recovered.
+    //
+    // The IDE resilient path has no injectivity guarantee, and its duplicates are
+    // benign: error recovery lowers every unparseable top-level construct to an
+    // `<error>` placeholder function (a single stray `forall { }` lowers to two),
+    // and a program caught mid-edit can momentarily name the same function twice,
+    // so distinct nodes legitimately carry the same key. Such a key always carries
+    // the parser's recovery marker, so [`is_parse_recovered`] exempts it.
+    //
+    // Either way the first node wins: keeping it is a deterministic, total
     // degradation — an already-broken program may have a self-edge resolve to the
     // wrong same-keyed node, at worst making A035/A036 miss a diagnostic on code
-    // that does not yet compile. That is never a false positive, and never a panic
-    // that would abort the analysis (or, in debug builds, the whole LSP process).
+    // that does not yet compile. That is never a false positive, and in release
+    // never a panic that would abort the analysis (or the whole LSP process).
     let mut index: HashMap<&FnKey, usize> = HashMap::with_capacity(nodes.len());
     for (i, n) in nodes.iter().enumerate() {
-        index.entry(&n.key).or_insert(i);
+        if let Some(&existing) = index.get(&n.key) {
+            debug_assert!(
+                is_parse_recovered(&n.key),
+                "duplicate call-graph key `{}` at node {i} (already node {existing}); \
+                 FnKey must be injective or A035/A036 may miss a self-edge",
+                n.key
+            );
+            continue;
+        }
+        index.insert(&n.key, i);
     }
 
     nodes
@@ -564,12 +620,13 @@
 
     /// The IDE resilient path can present two nodes under the same `FnKey`: a
     /// single stray `forall { }` lowers to two `<error>` placeholder functions,
-    /// and a program mid-edit can name the same function twice. Building the
-    /// adjacency must not panic; it keeps the first same-keyed node
-    /// deterministically, so an edge naming that key resolves to it and every
-    /// other node still yields a row.
+    /// and a program mid-edit can name the same function twice. Because the key
+    /// carries the parser's recovery marker, the duplicate-key tripwire is
+    /// suppressed in *every* build — even debug — so building the adjacency must
+    /// not panic; it keeps the first same-keyed node deterministically, so an edge
+    /// naming that key resolves to it and every other node still yields a row.
     #[test]
-    fn adjacency_keeps_first_node_on_duplicate_key() {
+    fn adjacency_tolerates_duplicate_recovered_key() {
         let dup_key = || FnKey::free_in(Vec::new(), "<error>");
         let nodes = vec![
             node(dup_key(), vec![]),
@@ -588,6 +645,97 @@
         );
     }
 
+    /// `is_parse_recovered` must catch the recovery marker wherever the parser can
+    /// synthesize it — a free/method name, a recovered struct name, or a recovered
+    /// spec name — while a fully real key is rejected. The `module_path` qualifier
+    /// is filesystem-derived and never counts, even if a segment literally spells
+    /// the marker.
+    #[test]
+    fn parse_recovered_predicate_matches_every_synthesized_segment() {
+        assert!(is_parse_recovered(&FnKey::free_in(Vec::new(), "<error>")));
+        assert!(is_parse_recovered(&FnKey::method_in(
+            path(&["a"]),
+            "<error>",
+            "m"
+        )));
+        assert!(is_parse_recovered(&FnKey::method_in(
+            path(&["a"]),
+            "T",
+            "<error>"
+        )));
+        assert!(is_parse_recovered(&FnKey::spec_free_folded(
+            &[],
+            "<error>",
+            "f"
+        )));
+        assert!(is_parse_recovered(&FnKey::spec_method_folded(
+            &[],
+            "S",
+            "<error>",
+            "m"
+        )));
+
+        assert!(!is_parse_recovered(&FnKey::free_in(path(&["a"]), "real")));
+        assert!(!is_parse_recovered(&FnKey::method_in(
+            path(&["a"]),
+            "T",
+            "m"
+        )));
+        assert!(
+            !is_parse_recovered(&FnKey::free_in(path(&["<error>"]), "real")),
+            "a filesystem-derived module segment is not a recovery marker"
+        );
+    }
+
+    /// A genuine duplicate — two *real* functions folded to one `FnKey` — is the
+    /// #63 identity-bug class the tripwire guards: a recursive self-edge could
+    /// resolve to the wrong same-keyed node and mask a cycle from A035/A036. In
+    /// debug builds the `debug_assert` must fire. Gated to debug builds (the
+    /// assert is compiled out in release) and catching the unwind rather than
+    /// using `#[should_panic]` (banned), with the default hook muted so the
+    /// intentional panic does not litter the test log.
+    #[cfg(debug_assertions)]
+    #[test]
+    fn adjacency_tripwire_fires_on_genuine_duplicate_in_debug() {
+        let dup_key = || FnKey::free_in(path(&["lib"]), "collide");
+        let prev_hook = std::panic::take_hook();
+        std::panic::set_hook(Box::new(|_| {}));
+        let result = std::panic::catch_unwind(|| {
+            let nodes = vec![node(dup_key(), vec![]), node(dup_key(), vec![])];
+            resolve_adjacency(&nodes)
+        });
+        std::panic::set_hook(prev_hook);
+        assert!(
+            result.is_err(),
+            "a genuine (non-recovered) duplicate FnKey must trip the debug tripwire"
+        );
+    }
+
+    /// In release builds the tripwire is compiled out, so even a genuine duplicate
+    /// must degrade gracefully: no panic, first node wins deterministically, and
+    /// every node still yields an adjacency row. Gated to release builds because
+    /// the same input panics in debug (see the tripwire test above).
+    #[cfg(not(debug_assertions))]
+    #[test]
+    fn adjacency_keeps_first_on_genuine_duplicate_in_release() {
+        let dup_key = || FnKey::free_in(path(&["lib"]), "collide");
+        let nodes = vec![
+            node(dup_key(), vec![]),
+            node(dup_key(), vec![]),
+            node(
+                FnKey::free_in(Vec::new(), "caller"),
+                vec![free_edge("collide", path(&["lib"]))],
+            ),
+        ];
+        let adj = resolve_adjacency(&nodes);
+        assert_eq!(adj.len(), 3, "every node yields an adjacency row");
+        assert_eq!(
+            adj[2],
+            vec![(0, loc())],
+            "the first same-keyed node wins deterministically"
+        );
+    }
+
     /// The FAMILY 2 regression witness at the graph level: a struct associated
     /// function (`mid::make` in file `a`, a `Method` key) and a same-named free
     /// function in the sibling file (`make` in `a/mid`, a `Free` key) are