From ac3d93e0ed5b8d6b489f83d96c770f4932969622 Mon Sep 17 00:00:00 2001 From: figsoda Date: Wed, 8 Mar 2023 09:44:22 -0500 Subject: [PATCH 1/5] Implement type inference for fallback attributes --- crates/ide/src/ty/display.rs | 14 ++++-- crates/ide/src/ty/infer.rs | 9 ++-- crates/ide/src/ty/known.rs | 11 +++-- crates/ide/src/ty/mod.rs | 82 ++++++++++++++++++++++++++---------- crates/ide/src/ty/tests.rs | 18 ++++++++ 5 files changed, 101 insertions(+), 33 deletions(-) diff --git a/crates/ide/src/ty/display.rs b/crates/ide/src/ty/display.rs index 47bba7e..068fd23 100644 --- a/crates/ide/src/ty/display.rs +++ b/crates/ide/src/ty/display.rs @@ -93,10 +93,16 @@ impl fmt::Display for TyDisplay<'_> { let value = Self { ty, config }; write!(f, " {name}: {value}")?; } - if set.len() > config.max_attrset_fields { - ", … }".fmt(f) - } else { - " }".fmt(f) + match (config.max_attrset_fields.checked_sub(set.len()), &*set.rest) { + (Some(1..), Some((ty, _))) => { + if !set.is_empty() { + ",".fmt(f)?; + } + let value = Self { ty, config }; + write!(f, " _: {value} }}") + } + (Some(_), _) => " }".fmt(f), + (None, _) => ", … }".fmt(f), } } } diff --git a/crates/ide/src/ty/infer.rs b/crates/ide/src/ty/infer.rs index 1b95462..c053574 100644 --- a/crates/ide/src/ty/infer.rs +++ b/crates/ide/src/ty/infer.rs @@ -532,13 +532,16 @@ impl<'a> Collector<'a> { let b = self.collect(b); super::Ty::Lambda(a.into(), b.into()) } - Ty::Attrset(set) => { - let set = set + Ty::Attrset(named) => { + let named = named .0 .into_iter() .map(|(name, (ty, src))| (name, self.collect(ty), src)) .collect(); - super::Ty::Attrset(super::Attrset(set)) + super::Ty::Attrset(super::Attrset { + named, + rest: Arc::new(None), + }) } Ty::External(ty) => ty, } diff --git a/crates/ide/src/ty/known.rs b/crates/ide/src/ty/known.rs index 9a03ec0..759dee1 100644 --- a/crates/ide/src/ty/known.rs +++ b/crates/ide/src/ty/known.rs @@ -57,15 +57,18 @@ fn merge_attrset(lhs: &Ty, rhs: &Ty) -> Ty { let rhs = rhs.as_attrset().unwrap(); // Put the RHS on the front and ... let mut xs = rhs - .0 + .named .iter() - .chain(lhs.0.iter()) + .chain(lhs.named.iter()) .map(|(name, ty, src)| (name.clone(), ty.clone(), *src)) .collect::>(); // ... run stable sort to prefer RHS when duplicated. xs.sort_by(|(lhs, ..), (rhs, ..)| lhs.cmp(rhs)); xs.dedup_by(|(lhs, ..), (rhs, ..)| lhs == rhs); - Ty::Attrset(Attrset(xs.into())) + Ty::Attrset(Attrset { + named: xs.into(), + rest: rhs.rest.clone(), + }) } /// https://nixos.wiki/wiki/Flakes @@ -87,6 +90,7 @@ pub fn flake(inputs: &[&str]) -> Ty { .iter() .copied() .map(|name| (name, input_ty.clone(), AttrSource::Unknown)), + None, )); let outputs_param_ty = Ty::Attrset(Attrset::from_internal( @@ -95,6 +99,7 @@ pub fn flake(inputs: &[&str]) -> Ty { .copied() .chain(Some("self")) .map(|name| (name, FLAKE.clone(), AttrSource::Unknown)), + None, )); ty!({ diff --git a/crates/ide/src/ty/mod.rs b/crates/ide/src/ty/mod.rs index 5066d70..bd9e563 100644 --- a/crates/ide/src/ty/mod.rs +++ b/crates/ide/src/ty/mod.rs @@ -28,18 +28,37 @@ macro_rules! ty { (($($inner:tt)*)) => { ty!($($inner)*) }; ([$($inner:tt)*]) => { $crate::ty::Ty::List(::std::sync::Arc::new(ty!($($inner)*)))}; - ({ $($key:literal : $ty:tt),* $(,)? $(_ : $rest_ty:tt)? }) => {{ - // TODO: Rest type. - $(let _ = ty!($rest_ty);)? - $crate::ty::Ty::Attrset($crate::ty::Attrset::from_internal([ - $(($key, ty!($ty), $crate::ty::AttrSource::Unknown),)* - ])) + ({ $($key:literal : $ty:tt),* $(,)? }) => {{ + $crate::ty::Ty::Attrset($crate::ty::Attrset::from_internal( + [ + $(($key, ty!($ty), $crate::ty::AttrSource::Unknown),)* + ], + None, + )) }}; - ({($src:expr) $($key:literal : $ty:tt),* $(,)? $(_ : $rest_ty:tt)? }) => {{ - $(let _ = ty!($rest_ty);)? - $crate::ty::Ty::Attrset($crate::ty::Attrset::from_internal([ - $(($key, ty!($ty), $src),)* - ])) + ({ $($key:literal : $ty:tt),* $(,)? _ : $rest_ty:tt }) => {{ + $crate::ty::Ty::Attrset($crate::ty::Attrset::from_internal( + [ + $(($key, ty!($ty), $crate::ty::AttrSource::Unknown),)* + ], + Some((ty!($rest_ty), $crate::ty::AttrSource::Unknown)), + )) + }}; + ({($src:expr) $($key:literal : $ty:tt),* $(,)? }) => {{ + $crate::ty::Ty::Attrset($crate::ty::Attrset::from_internal( + [ + $(($key, ty!($ty), $src),)* + ], + None + )) + }}; + ({($src:expr) $($key:literal : $ty:tt),* $(,)? _ : $rest_ty:tt }) => {{ + $crate::ty::Ty::Attrset($crate::ty::Attrset::from_internal( + [ + $(($key, ty!($ty), $src),)* + ], + Some((ty!($rest_ty), $src)) + )) }}; ($arg:tt -> $($ret:tt)*) => { $crate::ty::Ty::Lambda( @@ -122,11 +141,17 @@ impl fmt::Debug for Ty { // Invariant: sorted by names. #[derive(Debug, Clone, PartialEq, Eq)] -pub struct Attrset(Arc<[(SmolStr, Ty, AttrSource)]>); +pub struct Attrset { + named: Arc<[(SmolStr, Ty, AttrSource)]>, + rest: Arc>, +} impl Default for Attrset { fn default() -> Self { - Self(Arc::new([])) + Self { + named: Arc::new([]), + rest: Arc::new(None), + } } } @@ -136,32 +161,43 @@ impl Attrset { /// # Panics /// The given iterator must have no duplicated fields, or it'll panic. #[track_caller] - pub fn from_internal<'a>(iter: impl IntoIterator) -> Self { - let mut set = iter + pub fn from_internal<'a>( + iter: impl IntoIterator, + rest: Option<(Ty, AttrSource)>, + ) -> Self { + let mut named = iter .into_iter() .map(|(name, ty, src)| (SmolStr::from(name), ty, src)) .collect::>(); - Arc::get_mut(&mut set) + Arc::get_mut(&mut named) .unwrap() .sort_by(|(lhs, ..), (rhs, ..)| lhs.cmp(rhs)); assert!( - set.windows(2).all(|w| w[0].0 != w[1].0), + named.windows(2).all(|w| w[0].0 != w[1].0), "Duplicated fields", ); - Self(set) + Self { + named, + rest: Arc::new(rest), + } } pub fn is_empty(&self) -> bool { - self.0.is_empty() + self.named.is_empty() } pub fn len(&self) -> usize { - self.0.len() + self.named.len() } fn get_all(&self, field: &str) -> Option<(&Ty, AttrSource)> { - let i = self.0.binary_search_by(|p| (*p.0).cmp(field)).ok()?; - Some((&self.0[i].1, self.0[i].2)) + if let Ok(i) = self.named.binary_search_by(|p| (*p.0).cmp(field)) { + Some((&self.named[i].1, self.named[i].2)) + } else if let Some((ty, src)) = &*self.rest { + Some((ty, *src)) + } else { + None + } } pub fn get(&self, field: &str) -> Option<&Ty> { @@ -173,7 +209,7 @@ impl Attrset { } pub fn iter(&self) -> impl Iterator + '_ { - self.0.iter().map(|(k, ty, src)| (k, ty, *src)) + self.named.iter().map(|(k, ty, src)| (k, ty, *src)) } } diff --git a/crates/ide/src/ty/tests.rs b/crates/ide/src/ty/tests.rs index 526876b..3a83be5 100644 --- a/crates/ide/src/ty/tests.rs +++ b/crates/ide/src/ty/tests.rs @@ -226,10 +226,28 @@ fn flake_file() { ", expect!["{ inputs: { }, lastModified: int, lastModifiedDate: string, narHash: string, outPath: string, outputs: { }, rev: string, revCount: int, shortRev: string, sourceInfo: { dir: string, id: string, narHash: string, owner: string, ref: string, repo: string, rev: string, submodules: bool, type: string, url: string }, submodules: bool }"], ); + + // Placeholders + check_name( + "bar", + r" +#- /flake.nix +{ + outputs = { self }: { + apps.x86_64-linux = { + foo = let bar = bar; in bar; + }; + }; +} + ", + expect!["{ program: string, type: string }"], + ); } #[test] fn builtins() { check("true", expect!["bool"]); check("builtins.length [ ]", expect!["int"]); + check("builtins.readDir ./.", expect!["{ _: string }"]); + check("(builtins.readDir ./.).foo", expect!["string"]); } From 445ab8db259beea0d74aedcb66e172a6a8d22c71 Mon Sep 17 00:00:00 2001 From: figsoda Date: Wed, 8 Mar 2023 15:23:44 -0500 Subject: [PATCH 2/5] Change struct definition of ide::ty::Attrset - Rename named to fields - Transpose Option and Arc --- crates/ide/src/ty/display.rs | 9 ++++++--- crates/ide/src/ty/infer.rs | 9 +++------ crates/ide/src/ty/known.rs | 6 +++--- crates/ide/src/ty/mod.rs | 32 +++++++++++++++----------------- 4 files changed, 27 insertions(+), 29 deletions(-) diff --git a/crates/ide/src/ty/display.rs b/crates/ide/src/ty/display.rs index 068fd23..90fad07 100644 --- a/crates/ide/src/ty/display.rs +++ b/crates/ide/src/ty/display.rs @@ -93,12 +93,15 @@ impl fmt::Display for TyDisplay<'_> { let value = Self { ty, config }; write!(f, " {name}: {value}")?; } - match (config.max_attrset_fields.checked_sub(set.len()), &*set.rest) { - (Some(1..), Some((ty, _))) => { + match (config.max_attrset_fields.checked_sub(set.len()), &set.rest) { + (Some(1..), Some(rest)) => { if !set.is_empty() { ",".fmt(f)?; } - let value = Self { ty, config }; + let value = Self { + ty: &rest.0, + config, + }; write!(f, " _: {value} }}") } (Some(_), _) => " }".fmt(f), diff --git a/crates/ide/src/ty/infer.rs b/crates/ide/src/ty/infer.rs index c053574..b555d9c 100644 --- a/crates/ide/src/ty/infer.rs +++ b/crates/ide/src/ty/infer.rs @@ -532,16 +532,13 @@ impl<'a> Collector<'a> { let b = self.collect(b); super::Ty::Lambda(a.into(), b.into()) } - Ty::Attrset(named) => { - let named = named + Ty::Attrset(fields) => { + let fields = fields .0 .into_iter() .map(|(name, (ty, src))| (name, self.collect(ty), src)) .collect(); - super::Ty::Attrset(super::Attrset { - named, - rest: Arc::new(None), - }) + super::Ty::Attrset(super::Attrset { fields, rest: None }) } Ty::External(ty) => ty, } diff --git a/crates/ide/src/ty/known.rs b/crates/ide/src/ty/known.rs index 759dee1..bebfa08 100644 --- a/crates/ide/src/ty/known.rs +++ b/crates/ide/src/ty/known.rs @@ -57,16 +57,16 @@ fn merge_attrset(lhs: &Ty, rhs: &Ty) -> Ty { let rhs = rhs.as_attrset().unwrap(); // Put the RHS on the front and ... let mut xs = rhs - .named + .fields .iter() - .chain(lhs.named.iter()) + .chain(lhs.fields.iter()) .map(|(name, ty, src)| (name.clone(), ty.clone(), *src)) .collect::>(); // ... run stable sort to prefer RHS when duplicated. xs.sort_by(|(lhs, ..), (rhs, ..)| lhs.cmp(rhs)); xs.dedup_by(|(lhs, ..), (rhs, ..)| lhs == rhs); Ty::Attrset(Attrset { - named: xs.into(), + fields: xs.into(), rest: rhs.rest.clone(), }) } diff --git a/crates/ide/src/ty/mod.rs b/crates/ide/src/ty/mod.rs index bd9e563..cb270a3 100644 --- a/crates/ide/src/ty/mod.rs +++ b/crates/ide/src/ty/mod.rs @@ -142,15 +142,15 @@ impl fmt::Debug for Ty { // Invariant: sorted by names. #[derive(Debug, Clone, PartialEq, Eq)] pub struct Attrset { - named: Arc<[(SmolStr, Ty, AttrSource)]>, - rest: Arc>, + fields: Arc<[(SmolStr, Ty, AttrSource)]>, + rest: Option>, } impl Default for Attrset { fn default() -> Self { Self { - named: Arc::new([]), - rest: Arc::new(None), + fields: Arc::new([]), + rest: None, } } } @@ -165,38 +165,36 @@ impl Attrset { iter: impl IntoIterator, rest: Option<(Ty, AttrSource)>, ) -> Self { - let mut named = iter + let mut fields = iter .into_iter() .map(|(name, ty, src)| (SmolStr::from(name), ty, src)) .collect::>(); - Arc::get_mut(&mut named) + Arc::get_mut(&mut fields) .unwrap() .sort_by(|(lhs, ..), (rhs, ..)| lhs.cmp(rhs)); assert!( - named.windows(2).all(|w| w[0].0 != w[1].0), + fields.windows(2).all(|w| w[0].0 != w[1].0), "Duplicated fields", ); Self { - named, - rest: Arc::new(rest), + fields, + rest: rest.map(Arc::new), } } pub fn is_empty(&self) -> bool { - self.named.is_empty() + self.fields.is_empty() } pub fn len(&self) -> usize { - self.named.len() + self.fields.len() } fn get_all(&self, field: &str) -> Option<(&Ty, AttrSource)> { - if let Ok(i) = self.named.binary_search_by(|p| (*p.0).cmp(field)) { - Some((&self.named[i].1, self.named[i].2)) - } else if let Some((ty, src)) = &*self.rest { - Some((ty, *src)) + if let Ok(i) = self.fields.binary_search_by(|p| (*p.0).cmp(field)) { + Some((&self.fields[i].1, self.fields[i].2)) } else { - None + self.rest.as_ref().map(|rest| (&rest.0, rest.1)) } } @@ -209,7 +207,7 @@ impl Attrset { } pub fn iter(&self) -> impl Iterator + '_ { - self.named.iter().map(|(k, ty, src)| (k, ty, *src)) + self.fields.iter().map(|(k, ty, src)| (k, ty, *src)) } } From 5fffd6932952b25f668cfb58b5a6235b1af94797 Mon Sep 17 00:00:00 2001 From: figsoda Date: Wed, 8 Mar 2023 15:27:07 -0500 Subject: [PATCH 3/5] Move test to a separate rest_type function --- crates/ide/src/ty/tests.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/crates/ide/src/ty/tests.rs b/crates/ide/src/ty/tests.rs index 3a83be5..4ea2851 100644 --- a/crates/ide/src/ty/tests.rs +++ b/crates/ide/src/ty/tests.rs @@ -226,8 +226,10 @@ fn flake_file() { ", expect!["{ inputs: { }, lastModified: int, lastModifiedDate: string, narHash: string, outPath: string, outputs: { }, rev: string, revCount: int, shortRev: string, sourceInfo: { dir: string, id: string, narHash: string, owner: string, ref: string, repo: string, rev: string, submodules: bool, type: string, url: string }, submodules: bool }"], ); +} - // Placeholders +#[test] +fn rest_type() { check_name( "bar", r" From d3d19a1609e700bbfecc61b61270c23b00a05a8e Mon Sep 17 00:00:00 2001 From: figsoda Date: Wed, 8 Mar 2023 15:58:45 -0500 Subject: [PATCH 4/5] Change formatting of attrset type annotations --- crates/ide/src/ty/display.rs | 71 +++++++++++++++++++++++++++++------- crates/ide/src/ty/tests.rs | 2 +- 2 files changed, 58 insertions(+), 15 deletions(-) diff --git a/crates/ide/src/ty/display.rs b/crates/ide/src/ty/display.rs index 90fad07..603ec8f 100644 --- a/crates/ide/src/ty/display.rs +++ b/crates/ide/src/ty/display.rs @@ -75,7 +75,7 @@ impl fmt::Display for TyDisplay<'_> { Ok(()) } Ty::Attrset(set) => { - if config.max_attrset_depth == 0 { + if config.max_attrset_depth == 0 || config.max_attrset_fields == 0 { return "{…}".fmt(f); } config.max_attrset_depth -= 1; @@ -83,7 +83,10 @@ impl fmt::Display for TyDisplay<'_> { "{".fmt(f)?; let mut first = true; - for (name, ty, _src) in set.iter().take(config.max_attrset_fields) { + let max_fields = config + .max_attrset_fields + .saturating_sub(set.rest.is_some() as usize); + for (name, ty, _src) in set.iter().take(max_fields) { if first { first = false; } else { @@ -93,21 +96,61 @@ impl fmt::Display for TyDisplay<'_> { let value = Self { ty, config }; write!(f, " {name}: {value}")?; } - match (config.max_attrset_fields.checked_sub(set.len()), &set.rest) { - (Some(1..), Some(rest)) => { - if !set.is_empty() { - ",".fmt(f)?; - } - let value = Self { - ty: &rest.0, - config, - }; - write!(f, " _: {value} }}") + if let Some(rest) = &set.rest { + if !first { + ",".fmt(f)?; } - (Some(_), _) => " }".fmt(f), - (None, _) => ", … }".fmt(f), + let value = Self { + ty: &rest.0, + config, + }; + write!(f, " …: {value} }}") + } else if set.len() > max_fields { + ", … }".fmt(f) + } else { + " }".fmt(f) } } } } } + +#[cfg(test)] +mod tests { + use expect_test::{expect, Expect}; + + use super::{Config, TyDisplay}; + use crate::ty::Ty; + + #[track_caller] + fn check_max_fields(max_attrset_fields: usize, ty: &Ty, expect: Expect) { + let disp = TyDisplay { + ty, + config: Config { + max_attrset_fields, + ..super::Config::FULL + }, + }; + expect.assert_eq(&disp.to_string()); + } + + #[test] + fn attrset() { + let ty = &ty!({ "a": int, "b": string, "c": bool }); + check_max_fields(0, ty, expect!["{…}"]); + check_max_fields(1, ty, expect!["{ a: int, … }"]); + check_max_fields(2, ty, expect!["{ a: int, b: string, … }"]); + check_max_fields(3, ty, expect!["{ a: int, b: string, c: bool }"]); + check_max_fields(4, ty, expect!["{ a: int, b: string, c: bool }"]); + } + + #[test] + fn attrset_rest() { + let ty = &ty!({ "a": int, "b": string, _: bool }); + check_max_fields(0, ty, expect!["{…}"]); + check_max_fields(1, ty, expect!["{ …: bool }"]); + check_max_fields(2, ty, expect!["{ a: int, …: bool }"]); + check_max_fields(3, ty, expect!["{ a: int, b: string, …: bool }"]); + check_max_fields(4, ty, expect!["{ a: int, b: string, …: bool }"]); + } +} diff --git a/crates/ide/src/ty/tests.rs b/crates/ide/src/ty/tests.rs index 4ea2851..d051b63 100644 --- a/crates/ide/src/ty/tests.rs +++ b/crates/ide/src/ty/tests.rs @@ -250,6 +250,6 @@ fn rest_type() { fn builtins() { check("true", expect!["bool"]); check("builtins.length [ ]", expect!["int"]); - check("builtins.readDir ./.", expect!["{ _: string }"]); + check("builtins.readDir ./.", expect!["{ …: string }"]); check("(builtins.readDir ./.).foo", expect!["string"]); } From 0abaad5c3a7663b5e82b62782bc9016e496f42d4 Mon Sep 17 00:00:00 2001 From: figsoda Date: Wed, 8 Mar 2023 16:39:06 -0500 Subject: [PATCH 5/5] Add TODO for attrset type infererence for `rest` --- crates/ide/src/ty/infer.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/crates/ide/src/ty/infer.rs b/crates/ide/src/ty/infer.rs index b555d9c..f1432d0 100644 --- a/crates/ide/src/ty/infer.rs +++ b/crates/ide/src/ty/infer.rs @@ -37,6 +37,7 @@ enum Ty { List(TyVar), Lambda(TyVar, TyVar), + // TODO: Add support for `rest` similar to super::Attrset. Attrset(Attrset), External(super::Ty),