Recognize an all-different check, and say what replacing it costs

crates/infact-core/src/lib.rs

@@ -2,7 +2,7 @@
 
 use std::path::PathBuf;
 
-pub use infact_normalize::{Form, Pattern};
+pub use infact_normalize::{Coverage, Form, Pattern, Resolved};
 use serde::{Deserialize, Serialize};
 use strum::IntoStaticStr;
 
@@ -397,6 +397,69 @@ pub struct LibraryBehaviorMatch {
     /// something else is interleaved with it.
     #[serde(default)]
     pub fused: bool,
+    /// What has to hold for the swap to be sound, that this cannot check.
+    ///
+    /// A match says the code computes what the API computes. It does not say
+    /// the two are interchangeable here, and for some behaviors they are not:
+    /// the API may need a stronger bound on the element type, or allocate where
+    /// the code does not, or reach the same answer by a different route that a
+    /// caller could tell apart. None of that is in the syntax.
+    ///
+    /// Reporting the gap is what makes this a recommendation rather than a
+    /// lint. Where the gap IS visible — a `const fn` that cannot allocate at
+    /// all — the recognizer refuses instead, because a condition a reader must
+    /// check is worse than a finding they never see.
+    #[serde(default, skip_serializing_if = "Vec::is_empty")]
+    pub conditions: Vec<Condition>,
+}
+
+/// Something a recommendation depends on that the syntax does not settle.
+#[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Ord, Hash, Serialize, Deserialize)]
+#[serde(rename_all = "kebab-case")]
+pub enum Condition {
+    /// The API needs a bound on the element type that the code does not.
+    ///
+    /// A pairwise `==` needs only `PartialEq`; reaching the same answer through
+    /// a hash set needs `Eq + Hash`. The difference is not pedantic: `f64` is
+    /// `PartialEq` and not `Eq`, and two `NaN`s are unequal to each other, so
+    /// the loop calls them distinct and the set cannot be built at all.
+    ElementBound {
+        requires: String,
+        code_requires: String,
+    },
+    /// The API allocates where the code does not.
+    Allocates,
+    /// The API reaches the answer without making the comparisons the code makes.
+    ///
+    /// An operator with an observable effect — one that logs, counts, or panics
+    /// — runs a quadratic number of times here and need not run at all there.
+    ComparisonObservable,
+    /// The code is cheaper at the sizes it is actually called with.
+    ///
+    /// A quadratic scan of four elements beats allocating a hash set. Which one
+    /// this is depends on the caller, and nothing in the callee says.
+    SmallInputsFavourTheCode,
+}
+
+impl std::fmt::Display for Condition {
+    fn fmt(&self, formatter: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        match self {
+            Self::ElementBound {
+                requires,
+                code_requires,
+            } => write!(
+                formatter,
+                "the element type must be {requires}; the code needs only {code_requires}"
+            ),
+            Self::Allocates => formatter.write_str("the API allocates and the code does not"),
+            Self::ComparisonObservable => formatter.write_str(
+                "a comparison with an observable effect runs here and need not run there",
+            ),
+            Self::SmallInputsFavourTheCode => {
+                formatter.write_str("the code is faster at small sizes")
+            }
+        }
+    }
 }
 
 #[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Ord, Hash, Serialize, Deserialize)]

crates/infact-normalize/src/lib.rs

@@ -15,13 +15,14 @@ mod renaming;
 mod simplify;
 
 use matching::Bindings;
+pub use matching::Resolved;
 use renaming::Renaming;
 
 use std::fmt::{self, Display, Formatter};
 
 use serde::{Deserialize, Serialize};
 
-pub const NORMALIZED_FORM_SCHEMA: u32 = 1;
+pub const NORMALIZED_FORM_SCHEMA: u32 = 2;
 
 /// The deepest a form may nest and still describe an operation.
 ///
@@ -215,16 +216,22 @@ pub enum Form {
     /// special case: it is a normalization that lets a written-out loop compare
     /// against a library API that exists.
     ///
-    /// Only the distinct-pairs walk reduces to this. Walking adjacent pairs is
-    /// `windows(2)` and a different coverage; walking every ordered pair
-    /// including an element with itself is a third. Both stay two traversals
-    /// until something needs them, because a coverage field with one inhabitant
-    /// says nothing and a wrong one would claim a walk the code does not make.
+    /// Walking adjacent pairs is `windows(2)` and is not this: it is a third
+    /// coverage, and it stays two traversals until something needs it.
     Pairwise {
         sequence: Box<Form>,
         left: Box<Pattern>,
         right: Box<Pattern>,
         body: Box<Form>,
+        /// Which of the pairs the walk actually reaches.
+        ///
+        /// Written out, the two are a triangular nested loop and a square one
+        /// with the diagonal guarded away, and both are common. They reach the
+        /// same pairs and differ in how often, which is behavior: a decision
+        /// that does not care how many times it sees a pair gets the same
+        /// answer from either, and a count gets double. Recording it is what
+        /// lets a reader of the form tell which they have.
+        coverage: Coverage,
     },
     /// Producing a new sequence by transforming each element.
     Transform {
@@ -340,6 +347,25 @@ pub enum Form {
     },
 }
 
+/// How often a pairwise walk reaches each pair.
+#[derive(
+    Debug, Clone, Copy, Default, PartialEq, Eq, PartialOrd, Ord, Hash, Serialize, Deserialize,
+)]
+#[serde(rename_all = "kebab-case")]
+pub enum Coverage {
+    /// Each unordered pair once: `for i in 0..n { for j in i + 1..n { .. } }`,
+    /// which is what `itertools::tuple_combinations` offers.
+    #[default]
+    Once,
+    /// Each unordered pair both ways round, and no element with itself.
+    ///
+    /// A square nested loop over one sequence with an `i != j` guard. Every
+    /// pair is visited twice, in both orders, so this says strictly less than
+    /// [`Coverage::Once`] about anything order- or count-sensitive and exactly
+    /// as much about anything that is neither.
+    BothWays,
+}
+
 /// Which way a walk runs.
 ///
 /// Only a walk that can stop early is changed by this: searching from the front
@@ -558,11 +584,13 @@ impl Form {
                 left,
                 right,
                 body,
+                coverage,
             } => Self::Pairwise {
                 sequence: apply(sequence),
                 left: left.clone(),
                 right: right.clone(),
                 body: apply(body),
+                coverage: *coverage,
             },
             Self::Transform {
                 sequence,
@@ -756,6 +784,34 @@ impl Form {
         renaming.form(self)
     }
 
+    /// Every place `pattern` matches, with what its roles stood for.
+    ///
+    /// The companion to [`Form::contains`], which answers whether a pattern is
+    /// here and discards what it found. A recognizer that has to say something
+    /// about a *part* of what matched — this hole must not mention that name —
+    /// needs the parts, and working them out by walking the subject again is
+    /// how a matcher comes to be reimplemented beside itself.
+    ///
+    /// Matches at nodes, not at runs of statements: a pattern spread over
+    /// several steps has no single node to have matched, so [`Form::locate_all`]
+    /// is what places those.
+    #[must_use]
+    pub fn resolve_all(&self, pattern: &Self) -> Vec<Resolved> {
+        let mut found = Vec::new();
+        self.resolve_into(pattern, &mut found);
+        found
+    }
+
+    fn resolve_into(&self, pattern: &Self, found: &mut Vec<Resolved>) {
+        let mut bindings = Bindings::default();
+        if bindings.form(self, pattern) {
+            found.push(bindings.resolved());
+        }
+        for child in self.children() {
+            child.resolve_into(pattern, found);
+        }
+    }
+
     /// Whether this form contains `pattern` anywhere within it.
     ///
     /// Repository code rarely consists of nothing but the behavior in question,
@@ -1025,6 +1081,52 @@ impl Form {
             && self.is_comparable()
     }
 
+    /// The one sequence a body indexes at every named position.
+    ///
+    /// A loop bound is often a variable rather than the sequence's own length —
+    /// `for i in 0..n` far more often than `for i in 0..v.len()` — so the span
+    /// does not always say what is being walked. The body does: whatever it
+    /// reads at those positions is the sequence, and it has to be exactly one
+    /// of them, or the loop is walking positions into two things at once and is
+    /// not a walk over either.
+    fn sole_indexed_sequence(&self, positions: &[u32]) -> Option<&Self> {
+        let mut sequence = None;
+        let mut seen = Vec::new();
+        self.collect_indexed(positions, &mut sequence, &mut seen)?;
+        positions
+            .iter()
+            .all(|position| seen.contains(position))
+            .then_some(sequence)?
+    }
+
+    /// Gather the sequence indexed at each position, failing on disagreement.
+    fn collect_indexed<'a>(
+        &'a self,
+        positions: &[u32],
+        sequence: &mut Option<&'a Self>,
+        seen: &mut Vec<u32>,
+    ) -> Option<()> {
+        if let Self::Index {
+            sequence: indexed,
+            position,
+        } = self
+            && let Self::Local(index) = position.as_ref()
+            && positions.contains(index)
+        {
+            if sequence.is_some_and(|found| found != indexed.as_ref()) {
+                return None;
+            }
+            *sequence = Some(indexed.as_ref());
+            if !seen.contains(index) {
+                seen.push(*index);
+            }
+        }
+        for child in self.children() {
+            child.collect_indexed(positions, sequence, seen)?;
+        }
+        Some(())
+    }
+
     /// Whether a body reads a sequence only by indexing it at named positions.
     ///
     /// This is the licence to forget the index. `for i in 0..v.len()` visits
@@ -1182,7 +1284,14 @@ impl Display for Form {
                 left,
                 right,
                 body,
-            } => write!(formatter, "(pairwise {sequence} {left} {right} {body})"),
+                coverage,
+            } => {
+                let kind = match coverage {
+                    Coverage::Once => "pairwise",
+                    Coverage::BothWays => "pairwise-both-ways",
+                };
+                write!(formatter, "({kind} {sequence} {left} {right} {body})")
+            }
             Self::Accumulate {
                 sequence,
                 initial,
@@ -1381,6 +1490,44 @@ mod tests {
         }
     }
 
+    fn pairwise(coverage: Coverage) -> Form {
+        Form::Pairwise {
+            sequence: Box::new(Form::Free(0)),
+            left: Box::new(Pattern::Binding(0)),
+            right: Box::new(Pattern::Binding(1)),
+            body: Box::new(Form::Binary {
+                operator: "==".to_owned(),
+                left: Box::new(Form::Local(0)),
+                right: Box::new(Form::Local(1)),
+            }),
+            coverage,
+        }
+    }
+
+    /// A walk over pairs takes part in matching like every other form.
+    ///
+    /// Adding a variant without teaching the unifier about it does not fail to
+    /// compile: the fallthrough answers `false`, so the form silently matches
+    /// nothing and every behavior written over it goes quiet.
+    #[test]
+    fn a_walk_over_pairs_matches_itself() {
+        assert!(pairwise(Coverage::Once).contains(&pairwise(Coverage::Once)));
+        assert!(
+            Form::Sequence(vec![Form::Literal, pairwise(Coverage::Once)])
+                .contains(&pairwise(Coverage::Once))
+        );
+    }
+
+    /// Seeing each pair once is not seeing it both ways round.
+    ///
+    /// The two reach the same pairs and differ in how often, which is behavior
+    /// for anything that counts.
+    #[test]
+    fn the_two_coverages_do_not_match_each_other() {
+        assert!(!pairwise(Coverage::Once).contains(&pairwise(Coverage::BothWays)));
+        assert!(!pairwise(Coverage::BothWays).contains(&pairwise(Coverage::Once)));
+    }
+
     /// Code that does a thing four times has four findings.
     ///
     /// Reporting only the first meant a reader who fixed what they were shown