Auto merge of #744 - ecstatic-morse:snap-test, r=jackh726

Allow tests to be updated automatically

Uses the `expect-test` crate (thanks for the recommendation `@lnicola!)` to implement snapshot testing. `expect-test` is a bit more flexible than `insta`, so it seems like a good fit. This gives nicer diffs as well, since we don't strip whitespace prior to the comparison.

Setting `UPDATE_EXPECT=1` before running `cargo test` will update all tests automatically.

## Blockers
- [x] Omit empty lists from `Solution`'s `Display` impl.
  * `chalk` currently compares prefixes to make things less noisy, but that seems error prone anyways.
  * Harder than it seems at first, since you need to unintern to check whether a list is empty.
- [x] Handle recursive/SLG comparisons better.
  * Many tests use `yields`, which compares the results of the two solvers against a known output. Unfortunately, the test macro will duplicate the `expect` invocation, so if neither output matches the expected one, `expect-test` will overwrite it *twice*, almost assuredly corrupting the test file.
  * There's many ways to fix this. ~~I've hacked around it here by adding a method to `Expect` (which is not yet upstream).~~ Most robust is to collect the various engine results first and only compare them against the output if they agree. *This is implemented now*.
- [x] rust-analyzer/expect-test#20

## Nice to haves
- [x] Don't use raw strings with a hash unconditionally when blessing (rust-analyzer/expect-test#28).
- [x] Shorter alias for `expect![[`? It's currently hard-coded into the `expect-test` crate (rust-analyzer/expect-test#26).
- [ ] Remove newline escapes from existing tests.

Cargo.toml

@@ -34,5 +34,6 @@ chalk-integration = { version = "0.77.0-dev.0", path = "chalk-integration" }
 [dev-dependencies]
 # used for program_writer test errors
 diff = "0.1"
+expect-test = "1.2.1"
 pretty_assertions = "0.6.1"
 regex = "1"

chalk-ir/src/debug.rs

@@ -1,9 +1,23 @@
 //! Debug impls for types.
 
-use std::fmt::{Debug, Display, Error, Formatter};
+use std::fmt::{self, Debug, Display, Error, Formatter};
 
 use super::*;
 
+/// Wrapper to allow forwarding to `Display::fmt`, `Debug::fmt`, etc.
+pub struct Fmt<F>(pub F)
+where
+    F: Fn(&mut fmt::Formatter<'_>) -> fmt::Result;
+
+impl<F> fmt::Display for Fmt<F>
+where
+    F: Fn(&mut fmt::Formatter<'_>) -> fmt::Result,
+{
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        (self.0)(f)
+    }
+}
+
 impl<I: Interner> Debug for TraitId<I> {
     fn fmt(&self, fmt: &mut Formatter<'_>) -> Result<(), Error> {
         I::debug_trait_id(*self, fmt).unwrap_or_else(|| write!(fmt, "TraitId({:?})", self.0))
@@ -959,14 +973,27 @@ impl<I: Interner> Debug for Constraint<I> {
 }
 
 impl<I: Interner> Display for ConstrainedSubst<I> {
+    #[rustfmt::skip]
     fn fmt(&self, f: &mut Formatter<'_>) -> Result<(), Error> {
         let ConstrainedSubst { subst, constraints } = self;
 
-        write!(
-            f,
-            "substitution {}, lifetime constraints {:?}",
-            subst, constraints,
-        )
+        let mut first = true;
+
+        let subst = format!("{}", Fmt(|f| Display::fmt(subst, f)));
+        if subst != "[]" {
+            write!(f, "substitution {}", subst)?;
+            first = false;
+        }
+
+        let constraints = format!("{}", Fmt(|f| Debug::fmt(constraints, f)));
+        if constraints != "[]" {
+            if !first { write!(f, ", ")?; }
+            write!(f, "lifetime constraints {}", constraints)?;
+            first = false;
+        }
+
+        let _ = first;
+        Ok(())
     }
 }
 

chalk-solve/src/solve.rs

@@ -159,12 +159,20 @@ pub struct SolutionDisplay<'a, I: Interner> {
 }
 
 impl<'a, I: Interner> fmt::Display for SolutionDisplay<'a, I> {
+    #[rustfmt::skip]
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> Result<(), fmt::Error> {
         let SolutionDisplay { solution, interner } = self;
         match solution {
-            Solution::Unique(constrained) => {
-                write!(f, "Unique; {}", constrained.display(*interner))
-            }
+            // If a `Unique` solution has no associated data, omit the trailing semicolon.
+            // This makes blessed test output nicer to read.
+            Solution::Unique(Canonical { binders, value: ConstrainedSubst { subst, constraints } } )
+                if interner.constraints_data(constraints.interned()).is_empty()
+                    && interner.substitution_data(subst.interned()).is_empty()
+                    && interner.canonical_var_kinds_data(binders.interned()).is_empty()
+                => write!(f, "Unique"),
+
+            Solution::Unique(constrained) => write!(f, "Unique; {}", constrained.display(*interner)),
+
             Solution::Ambig(Guidance::Definite(subst)) => write!(
                 f,
                 "Ambiguous; definite substitution {}",

tests/logging_db/util.rs

@@ -12,7 +12,9 @@ use chalk_solve::ext::*;
 use chalk_solve::logging_db::LoggingRustIrDatabase;
 use chalk_solve::RustIrDatabase;
 
-use crate::test::{assert_result, TestGoal};
+use crate::test::assert_result_str;
+
+type TestGoal = crate::test::TestGoal<&'static str>;
 
 macro_rules! logging_db_output_sufficient {
     ($($arg:tt)*) => {{
@@ -25,12 +27,16 @@ macro_rules! logging_db_output_sufficient {
 
 pub fn logging_db_output_sufficient(
     program_text: &str,
-    goals: Vec<(&str, SolverChoice, TestGoal)>,
+    goals: Vec<(&str, Vec<SolverChoice>, TestGoal)>,
 ) {
     println!("program {}", program_text);
     assert!(program_text.starts_with("{"));
     assert!(program_text.ends_with("}"));
 
+    let goals = goals
+        .iter()
+        .flat_map(|(a, bs, c)| bs.into_iter().map(move |b| (a, b, c)));
+
     let output_text = {
         let db = ChalkDatabase::with(
             &program_text[1..program_text.len() - 1],
@@ -39,6 +45,7 @@ pub fn logging_db_output_sufficient(
 
         let program = db.program_ir().unwrap();
         let wrapped = LoggingRustIrDatabase::<_, Program, _>::new(program.clone());
+
         chalk_integration::tls::set_current_program(&program, || {
             for (goal_text, solver_choice, expected) in goals.clone() {
                 let mut solver = solver_choice.into_solver();
@@ -59,7 +66,7 @@ pub fn logging_db_output_sufficient(
                 match expected {
                     TestGoal::Aggregated(expected) => {
                         let result = solver.solve(&wrapped, &peeled_goal);
-                        assert_result(result, expected, db.interner());
+                        assert_result_str(result, expected, db.interner());
                     }
                     _ => panic!("only aggregated test goals supported for logger goals"),
                 }
@@ -101,7 +108,7 @@ pub fn logging_db_output_sufficient(
             match expected {
                 TestGoal::Aggregated(expected) => {
                     let result = solver.solve(&db, &peeled_goal);
-                    assert_result(result, expected, db.interner());
+                    assert_result_str(result, expected, db.interner());
                 }
                 _ => panic!("only aggregated test goals supported for logger goals"),
             }

tests/test/arrays.rs

@@ -13,7 +13,7 @@ fn arrays_are_sized() {
                 [u32; N]: Sized
             }
         } yields {
-            "Unique"
+            expect![["Unique"]]
         }
 
     }
@@ -35,7 +35,7 @@ fn arrays_are_copy_if_element_copy() {
                 [Foo; N]: Copy
             }
         } yields {
-            "Unique"
+            expect![["Unique"]]
         }
     }
 }
@@ -55,7 +55,7 @@ fn arrays_are_not_copy_if_element_not_copy() {
                 [Foo; N]: Copy
             }
         } yields {
-            "No possible solution"
+            expect![["No possible solution"]]
         }
     }
 }
@@ -76,7 +76,7 @@ fn arrays_are_clone_if_element_clone() {
                 [Foo; N]: Clone
             }
         } yields {
-            "Unique"
+            expect![["Unique"]]
         }
     }
 }
@@ -96,7 +96,7 @@ fn arrays_are_not_clone_if_element_not_clone() {
                 [Foo; N]: Clone
             }
         } yields {
-            "No possible solution"
+            expect![["No possible solution"]]
         }
     }
 }
@@ -116,23 +116,23 @@ fn arrays_are_well_formed_if_elem_sized() {
                 }
             }
         } yields {
-            "Unique"
+            expect![["Unique"]]
         }
 
         goal {
             forall<const N, T> {
                 WellFormed([T; N])
             }
         } yields {
-            "No possible solution"
+            expect![["No possible solution"]]
         }
 
         goal {
             exists<const N, T> {
                 WellFormed([T; N])
             }
         } yields {
-            "Ambiguous; no inference guidance"
+            expect![["Ambiguous; no inference guidance"]]
         }
     }
 }