From 0031eb4343fd4672742fd6ff839da9b4f5120646 Mon Sep 17 00:00:00 2001 From: oxalica Date: Wed, 29 Nov 2023 10:31:38 +0800 Subject: [PATCH] Record AstPtr of the first Expr for merged attrsets Otherwise, it causes a panic of missing node when reporting diagnostics on the merged attrset. Fixes #114 --- crates/ide/src/def/liveness.rs | 20 +++++++++++++++++++- crates/ide/src/def/lower.rs | 13 +++++++++---- crates/ide/src/diagnostic.rs | 6 ++++++ 3 files changed, 34 insertions(+), 5 deletions(-) diff --git a/crates/ide/src/def/liveness.rs b/crates/ide/src/def/liveness.rs index 6ccc8f9..4a3af9b 100644 --- a/crates/ide/src/def/liveness.rs +++ b/crates/ide/src/def/liveness.rs @@ -234,7 +234,14 @@ mod tests { let (db, f) = TestDB::from_fixture(fixture).unwrap(); assert_eq!(f.files().len(), 1); let file = f.files()[0]; - assert_eq!(db.source_map(file).diagnostics(), Vec::new(), "Lower error"); + + let source_map = db.source_map(file); + let lower_diags = source_map.diagnostics(); + assert!( + !lower_diags.iter().any(|diag| diag.severity().is_fatal()), + "Lower error: {lower_diags:?}", + ); + let expect = f.markers().iter().map(|p| p.pos).collect::>(); let mut got = db .liveness_check(file) @@ -369,4 +376,15 @@ mod tests { check("x: _y: x"); check("let __findFile = 42; in "); } + + // Issue #114 + #[test] + fn merged_rec_attrset() { + check("{ test = $0rec { }; test.bar = 42; }"); + check("{ test = rec { foo = bar; }; test.bar = 42; }"); + check("{ test = rec { foo = 42; }; test.bar = foo; }"); + check("{ test = rec { }; test.foo = bar; test.bar = 42; }"); + + check("{ test = $0rec { }; test = rec { };}"); + } } diff --git a/crates/ide/src/def/lower.rs b/crates/ide/src/def/lower.rs index 1c8ac0a..b1a243a 100644 --- a/crates/ide/src/def/lower.rs +++ b/crates/ide/src/def/lower.rs @@ -509,10 +509,15 @@ impl MergingSet { ctx.source_map.name_map_rev[ent.name].push(attr_ptr.clone()); if let Some(prev_set) = &mut ent.set { - // Erase the source information when merging occurs. - // Since the previous definition is not exhaustive now. - // `{ a = { b = 1; }; a.c = 2; }` - prev_set.ptr = None; + // Here we keep the `ptr` of the first definition attrset, + // which is the determinant `rec` one if it happens to be. + // This may be used to report diagnostics in liveness check. + // Covered by test `liveness::tests::merge_rec_attrset`. + // Eg. `{ a = rec { b = 1; }; a.c = 2; }` + // ^^^ Unused rec. + // + // This does result that the node pointed by `ptr` is non-exhaustive. + // Hope this does not break anything. if prev_set.name_kind.is_definition() { ctx.diagnostic(Diagnostic::new( diff --git a/crates/ide/src/diagnostic.rs b/crates/ide/src/diagnostic.rs index d9eab6b..8b204dd 100644 --- a/crates/ide/src/diagnostic.rs +++ b/crates/ide/src/diagnostic.rs @@ -41,6 +41,12 @@ pub enum Severity { IncompleteSyntax, } +impl Severity { + pub fn is_fatal(self) -> bool { + self != Self::Warning + } +} + impl Diagnostic { pub fn new(range: TextRange, kind: DiagnosticKind) -> Self { Self {