From 75e8b6e8477237697152ef2807c53130ce846cd1 Mon Sep 17 00:00:00 2001 From: Imbris Date: Wed, 4 Mar 2026 12:18:33 -0500 Subject: [PATCH 01/25] Require first asset in all files to be a top level asset with '0' as the identifier --- bauble/src/parse/parser.rs | 41 ++++++++++++++++++----- bauble/src/parse/value.rs | 42 +++++++++++++++++++++-- bauble/src/types/path.rs | 4 +++ bauble/src/value/display.rs | 15 ++++++--- bauble/src/value/mod.rs | 67 +++++++++++++++++-------------------- 5 files changed, 116 insertions(+), 53 deletions(-) diff --git a/bauble/src/parse/parser.rs b/bauble/src/parse/parser.rs index 9d0c450..7ba8fad 100644 --- a/bauble/src/parse/parser.rs +++ b/bauble/src/parse/parser.rs @@ -7,7 +7,7 @@ use crate::{ Attributes, BaubleContext, FieldsKind, PrimitiveValue, Value, context::FileId, parse::{ - Binding, ParseVal, + Binding, BindingIdent, ParseVal, value::{ParseValues, Path, PathEnd, PathTreeEnd, PathTreeNode}, }, spanned::{SpanExt, Spanned}, @@ -688,20 +688,45 @@ pub fn parser<'a>() -> impl Parser<'a, ParserSource<'a>, ParseValues, Extra<'a>> }, |mut values, (i, (ident, type_path, value))| { let is_first = i == 0; + let has_top_level_ident = *ident == BindingIdent::TOP_LEVEL_IDENTIFIER; + + match (is_first, has_top_level_ident) { + (true, true) | (false, false) => {} + (true, false) => { + emitter.emit(Rich::custom( + ident.span, + format!( + "The first item must '{}' as the identifier", + BindingIdent::TOP_LEVEL_IDENTIFIER + ), + )); + } + (false, true) => { + emitter.emit(Rich::custom( + ident.span, + format!( + "Identifier '{}' is only allowed for the first item", + BindingIdent::TOP_LEVEL_IDENTIFIER + ), + )); + } + } - let binding = Binding { - type_path, - value, - is_first, + let binding_ident = if has_top_level_ident { + BindingIdent::TopLevel(ident.map(|_| ())) + } else { + BindingIdent::Local(ident) }; - if values.values.contains_key(&ident) { + let binding = Binding { type_path, value }; + + if values.values.contains_key(&binding_ident) { emitter.emit(Rich::custom( - ident.span, + binding_ident.span(), "This identifier was already used".to_string(), )); } - values.values.insert(ident, binding); + values.values.insert(binding_ident, binding); values }, ) diff --git a/bauble/src/parse/value.rs b/bauble/src/parse/value.rs index 4efaa7d..8ee0d8c 100644 --- a/bauble/src/parse/value.rs +++ b/bauble/src/parse/value.rs @@ -2,7 +2,7 @@ use core::fmt; use crate::{ SpanExt, - spanned::Spanned, + spanned::{Span, Spanned}, value::{AnyVal, Ident, SpannedValue, ValueTrait}, }; use indexmap::IndexMap; @@ -168,11 +168,47 @@ impl SpannedValue for ParseVal { pub struct Binding { pub type_path: Option, pub value: ParseVal, - pub is_first: bool, +} + +#[derive(Debug, PartialEq, Eq, Hash)] +pub enum BindingIdent { + /// This is the top level asset in a parsed file. + /// + /// It has the special cased identifier `0` and appears as the first item in the file.. + /// + /// This holds no identifier string because it will have the same path as the file containing + /// it. + TopLevel(Spanned<()>), + /// This is a local asset. I.e. any additional assets in a parsed file. + Local(Ident), +} + +impl BindingIdent { + /// Special cased identifier that is required and only allowed for the first asset in a file + /// (i.e. the top level asset that is named after the file). + pub const TOP_LEVEL_IDENTIFIER: &str = "0"; + + pub fn as_str(&self) -> &str { + match self { + Self::TopLevel(_) => Self::TOP_LEVEL_IDENTIFIER, + Self::Local(ident) => &*ident, + } + } + + pub fn span(&self) -> Span { + match self { + Self::TopLevel(span) => span.span, + Self::Local(ident) => ident.span, + } + } + + pub fn is_top_level(&self) -> bool { + matches!(self, Self::TopLevel(_)) + } } #[derive(Debug)] pub struct ParseValues { pub uses: Vec>, - pub values: IndexMap, + pub values: IndexMap, } diff --git a/bauble/src/types/path.rs b/bauble/src/types/path.rs index 8391630..3d4389b 100644 --- a/bauble/src/types/path.rs +++ b/bauble/src/types/path.rs @@ -666,6 +666,10 @@ impl<'a> TypePath<&'a str> { Some((TypePath(""), TypePathElem(*self))) } } + + pub fn into_str(self) -> &'a str { + self.0 + } } /// An iterator of segments in a path. diff --git a/bauble/src/value/display.rs b/bauble/src/value/display.rs index 7dccb45..39528c7 100644 --- a/bauble/src/value/display.rs +++ b/bauble/src/value/display.rs @@ -575,11 +575,16 @@ impl> IndentedDisplay for ParseVal { impl, V: IndentedDisplay + ValueTrait> IndentedDisplay for Object { fn indented_display(&self, mut w: LineWriter) { - let Some((_, ident)) = self.object_path.get_end() else { - return; + let ident = if self.top_level { + crate::value::BindingIdent::TOP_LEVEL_IDENTIFIER + } else { + let Some((_, end)) = self.object_path.get_end() else { + return; + }; + end.into_str() }; - w.write(ident.as_str()); + w.write(ident); if let Some(registry) = w.ctx().type_registry() { let ty = self.value.ty(); @@ -755,7 +760,7 @@ impl> IndentedDisplay for ParseValues { w.write("\n\n"); } - w.write(ident); + w.write(ident.as_str()); if let Some(ty) = &binding.type_path { w.write(": "); w.fmt(ty); @@ -767,7 +772,7 @@ impl> IndentedDisplay for ParseValues { for (ident, binding) in iter { w.write("\n\n"); - w.write(ident); + w.write(ident.as_str()); if let Some(ty) = &binding.type_path { w.write(": "); w.fmt(ty); diff --git a/bauble/src/value/mod.rs b/bauble/src/value/mod.rs index 9379e01..5394bbb 100644 --- a/bauble/src/value/mod.rs +++ b/bauble/src/value/mod.rs @@ -6,7 +6,7 @@ use rust_decimal::Decimal; use crate::{ BaubleErrors, FileId, VariantKind, context::PathReference, - parse::{ParseVal, ParseValues, Path, PathEnd}, + parse::{BindingIdent, ParseVal, ParseValues, Path, PathEnd}, path::{TypePath, TypePathElem}, spanned::{SpanExt, Spanned}, types::{self, TypeId}, @@ -726,28 +726,24 @@ pub(crate) fn resolve_delayed( } } -/// Computes the path of an object. +/// Returns the identifier and the path of an object. /// -/// Normally, the path is just the files's bauble path joined with the object name. +/// The top level object in each file will have a path that matches the path of the file containing +/// it. The identifier for these objects is always "0" and it can be referred to locally in the +/// same file using this identifier. /// -/// If an object is the first in the file and its name matches the file name, it receives a special -/// path that is just the file's bauble path. -fn object_path( +/// TODO: make sure this is updated +/// For other objects, the path is just the files's bauble path joined with the object identifier. +fn object_ident_path<'a>( file_path: TypePath<&str>, - ident: &TypePathElem<&str>, - binding: &crate::parse::Binding, -) -> TypePath { - if binding.is_first && { - let file_name = file_path - .split_end() - .expect("file_path must not be empty") - .1; - *ident == file_name - } { - file_path.to_owned() - } else { - file_path.join(ident) - } + binding_ident: &'a BindingIdent, +) -> (TypePathElem<&'a str>, TypePath) { + let ident = TypePathElem::new(binding_ident.as_str()).expect("Invariant"); + let path = match binding_ident { + BindingIdent::TopLevel(_) => file_path.to_owned(), + BindingIdent::Local(_) => file_path.join(&ident), + }; + (ident, path) } pub(crate) fn register_assets( @@ -773,15 +769,13 @@ pub(crate) fn register_assets( // TODO: Register these in a correct order to allow for assets referencing assets. for (ident, binding) in &values.values { - let span = ident.span; - let ident = &TypePathElem::new(ident.as_str()).expect("Invariant"); - let path = object_path(file_path, ident, binding); - let top_level = path.borrow() == file_path; - let kind = if top_level { + let span = ident.span(); + let kind = if ident.is_top_level() { crate::AssetKind::TopLevel } else { crate::AssetKind::Local }; + let (ident, path) = object_ident_path(file_path, ident); let symbols = Symbols { ctx: &*ctx, uses }; // To register an asset we need to determine its type. @@ -897,11 +891,10 @@ pub(crate) fn convert_values( let file_path = symbols.ctx.get_file_path(file); - // Add asset - for (ident, binding) in &values.values { - let span = ident.span; - let ident = TypePathElem::new(ident.as_str()).expect("Invariant"); - let path = object_path(file_path, &ident, binding); + // Add assets from this file to symbols. + for ident in values.values.keys() { + let span = ident.span(); + let (ident, path) = object_ident_path(file_path, &ident); if let Some(PathReference { asset: Some(asset), .. @@ -932,13 +925,14 @@ pub(crate) fn convert_values( let mut ok = Vec::new(); let mut err = use_errors; - for (ident, binding) in values.values.iter() { + for (ident, binding) in &values.values { let ref_ty = match symbols.resolve_asset( &Path { - leading: Vec::new().spanned(ident.span.sub_span(0..0)), - last: PathEnd::Ident(ident.clone()).spanned(ident.span), + leading: Vec::new().spanned(ident.span().sub_span(0..0)), + last: PathEnd::Ident(ident.as_str().to_owned().spanned(ident.span())) + .spanned(ident.span()), } - .spanned(ident.span), + .spanned(ident.span()), ) { Ok((ty, _)) => ty, Err(e) => { @@ -952,8 +946,8 @@ pub(crate) fn convert_values( _ => unreachable!("Invariant"), }; - let ident = TypePathElem::new(ident.as_str()).expect("Invariant"); - let path = object_path(file_path, &ident, binding); + let top_level = ident.is_top_level(); + let (ident, path) = object_ident_path(file_path, &ident); let convert_meta = ConvertMeta { symbols: &symbols, @@ -961,7 +955,6 @@ pub(crate) fn convert_values( object_name: ident, default_span, }; - let top_level = path.borrow() == file_path; match convert_object(path, top_level, &binding.value, ty, convert_meta) { Ok(obj) => ok.push(obj), Err(e) => err.push(e), From 4fe56973adda0d703540f9da71149049ff3bdb8c Mon Sep 17 00:00:00 2001 From: Imbris Date: Mon, 9 Mar 2026 12:35:17 -0400 Subject: [PATCH 02/25] Update tests --- bauble/src/parse/parser.rs | 14 ++-- bauble/tests/integration.rs | 136 ++++++++++++++++++++-------------- bauble_macros/tests/derive.rs | 20 ++--- 3 files changed, 99 insertions(+), 71 deletions(-) diff --git a/bauble/src/parse/parser.rs b/bauble/src/parse/parser.rs index 7ba8fad..71e8baa 100644 --- a/bauble/src/parse/parser.rs +++ b/bauble/src/parse/parser.rs @@ -690,16 +690,17 @@ pub fn parser<'a>() -> impl Parser<'a, ParserSource<'a>, ParseValues, Extra<'a>> let is_first = i == 0; let has_top_level_ident = *ident == BindingIdent::TOP_LEVEL_IDENTIFIER; - match (is_first, has_top_level_ident) { - (true, true) | (false, false) => {} + let error_emitted = match (is_first, has_top_level_ident) { + (true, true) | (false, false) => false, (true, false) => { emitter.emit(Rich::custom( ident.span, format!( - "The first item must '{}' as the identifier", + "The first item must have '{}' as the identifier", BindingIdent::TOP_LEVEL_IDENTIFIER ), )); + true } (false, true) => { emitter.emit(Rich::custom( @@ -709,8 +710,9 @@ pub fn parser<'a>() -> impl Parser<'a, ParserSource<'a>, ParseValues, Extra<'a>> BindingIdent::TOP_LEVEL_IDENTIFIER ), )); + true } - } + }; let binding_ident = if has_top_level_ident { BindingIdent::TopLevel(ident.map(|_| ())) @@ -720,7 +722,9 @@ pub fn parser<'a>() -> impl Parser<'a, ParserSource<'a>, ParseValues, Extra<'a>> let binding = Binding { type_path, value }; - if values.values.contains_key(&binding_ident) { + // Note, we don't emit this if a more specific error about the identifier was + // already emitted above. + if values.values.contains_key(&binding_ident) && !error_emitted { emitter.emit(Rich::custom( binding_ident.span(), "This identifier was already used".to_string(), diff --git a/bauble/tests/integration.rs b/bauble/tests/integration.rs index 3e7b6fd..48fdcf2 100644 --- a/bauble/tests/integration.rs +++ b/bauble/tests/integration.rs @@ -154,23 +154,23 @@ fn test_load_partial( fn new_nested_reload_paths() { let a = &test_file!( "a", - r#"test = integration::Test { x: -5, y: 5 }"#, + r#"0 = integration::Test { x: -5, y: 5 }"#, Test { x: -5, y: 5 }, ); let new_a = &test_file!( "a", - r#"test = integration::Test { x: -15, y: 15 }"#, + r#"0 = integration::Test { x: -15, y: 15 }"#, Test { x: -15, y: 15 }, ); let new_ab = &test_file!( "a::b", - r#"test = integration::Test { x: -3, y: 3 }"#, + r#"0 = integration::Test { x: -3, y: 3 }"#, Test { x: -3, y: 3 }, ); let new_abc = &test_file!( "a::b::c", - r#"test = integration::Test { x: -4, y: 1 }"#, + r#"0 = integration::Test { x: -4, y: 1 }"#, Test { x: -4, y: 1 }, ); @@ -195,8 +195,10 @@ fn new_nested_reload_paths() { fn duplicate_objects() { let a = &test_file!( "a", - "test = integration::Test{ x: -5, y: 5 }\n\ - test = integration::Test{ x: -5, y: 4 }", + "0 = integration::Test{ x: -5, y: 5 }\n\ + a = integration::Test{ x: -5, y: 4 }\n\ + a = integration::Test{ x: -5, y: 4 }", + Test { x: -5, y: 5 }, Test { x: -5, y: 5 }, ); @@ -218,7 +220,7 @@ fn duplicate_objects_across_files() { "b::test = integration::Test{ x: -5, y: 5 }", Test { x: -5, y: 5 }, ); - let ab = &test_file!("a::b", "test = integration::Test{ x: -5, y: 5 }",); + let ab = &test_file!("a::b", "0 = integration::Test{ x: -5, y: 5 }",); test_load( &|ctx| { @@ -233,7 +235,7 @@ fn empty_module() { let a = &test_file!( "a", "use a::empty_module;\n\ - test = integration::Test { x: -5, y: 5 }", + 0 = integration::Test { x: -5, y: 5 }", Test { x: -5, y: 5 }, ); @@ -249,8 +251,8 @@ fn empty_module() { #[test] fn default_uses() { - let a = &test_file!("a", "test = Test { x: -5, y: 5 }", Test { x: -5, y: 5 },); - let ab = &test_file!("a::b", "test = Test { x: -4, y: 3 }", Test { x: -4, y: 3 },); + let a = &test_file!("a", "0 = Test { x: -5, y: 5 }", Test { x: -5, y: 5 },); + let ab = &test_file!("a::b", "0 = Test { x: -4, y: 3 }", Test { x: -4, y: 3 },); test_load( &|ctx| { @@ -269,13 +271,13 @@ fn default_uses() { fn some_files_fail() { let a = &test_file!( "a", - "test = integration::Test { x: -5, y: 5 }", + "0 = integration::Test { x: -5, y: 5 }", Test { x: -5, y: 5 }, ); let b = &test_file!("b", "This file fails to parse",); let c = &test_file!( "c", - "test = integration::Test { x: -3, y: 3 }", + "0 = integration::Test { x: -3, y: 3 }", Test { x: -3, y: 3 }, ); @@ -323,10 +325,10 @@ impl bauble::Bauble<'_> for TestRef { fn same_file_references() { let a = &test_file!( "a", - "test = integration::Test { x: -5, y: 5 }\n\ - test_ref = $test", + "0 = integration::Test { x: -5, y: 5 }\n\ + test_ref = $0", Test { x: -5, y: 5 }, - TestRef("a::test".into()), + TestRef("a".into()), ); test_load( @@ -344,7 +346,7 @@ fn same_file_references() { fn same_file_references_reverse() { let a = &test_file!( "a", - "test_ref = $test\n\ + "0 = $test\n\ test = integration::Test { x: -5, y: 5 }", TestRef("a::test".into()), Test { x: -5, y: 5 }, @@ -363,7 +365,7 @@ fn same_file_references_reverse() { fn same_file_references_reverse_full() { let a = &test_file!( "a", - "test_ref = $a::test\n\ + "0 = $a::test\n\ test = integration::Test { x: -5, y: 5 }", TestRef("a::test".into()), Test { x: -5, y: 5 }, @@ -384,12 +386,12 @@ fn reference_with_use() { let a = &test_file!( "a", "use b::test;\n\ - test_ref = $test", - TestRef("b::test".into()), + 0 = $test", + TestRef("b".into()), ); let b = &test_file!( "b", - "test = integration::Test { x: -5, y: 5 }", + "0 = integration::Test { x: -5, y: 5 }", Test { x: -5, y: 5 }, ); @@ -405,17 +407,17 @@ fn reference_with_use() { pub fn ref_implicit_type() { bauble::bauble_test!( [Test] - "t = integration::Test{ x: -5, y: 5 }\n\ - r = $t" + "0 = integration::Test{ x: -5, y: 5 }\n\ + r = $0" [ Test { x: -5, y: 5 }, - Ref::::from_path(TypePath::new_unchecked("test::t").to_owned()), + Ref::::from_path(TypePath::new_unchecked("test").to_owned()), ] ); bauble::bauble_test!( [Test] - "r = $test::t\n\ + "0 = $test::t\n\ t = integration::Test{ x: -5, y: 5 }" [ Ref::::from_path(TypePath::new_unchecked("test::t").to_owned()), @@ -428,17 +430,17 @@ pub fn ref_implicit_type() { pub fn ref_explicit_type() { bauble::bauble_test!( [Test] - "t = integration::Test{ x: -2, y: 2 }\n\ - r: Ref = $t" + "0 = integration::Test{ x: -2, y: 2 }\n\ + r: Ref = $0" [ Test { x: -2, y: 2 }, - Ref::::from_path(TypePath::new_unchecked("test::t").to_owned()), + Ref::::from_path(TypePath::new_unchecked("test").to_owned()), ] ); bauble::bauble_test!( [Test] - "r: Ref = $test::t\n\ + "0: Ref = $test::t\n\ t = integration::Test{ x: -2, y: 2 }" [ Ref::::from_path(TypePath::new_unchecked("test::t").to_owned()), @@ -452,23 +454,23 @@ pub fn ref_explicit_type_multiple_files() { bauble::bauble_test!( [Test] [ - "t = integration::Test{ x: -5, y: 5 }", - "r: Ref = $test0::t" + "0 = integration::Test{ x: -5, y: 5 }", + "0: Ref = $test0" ] [ Test { x: -5, y: 5 }, - Ref::::from_path(TypePath::new_unchecked("test0::t").to_owned()), + Ref::::from_path(TypePath::new_unchecked("test0").to_owned()), ] ); bauble::bauble_test!( [Test] [ - "r: Ref = $test1::t", - "t = integration::Test{ x: -5, y: 5 }" + "0: Ref = $test1", + "0 = integration::Test{ x: -5, y: 5 }" ] [ - Ref::::from_path(TypePath::new_unchecked("test1::t").to_owned()), + Ref::::from_path(TypePath::new_unchecked("test1").to_owned()), Test { x: -5, y: 5 }, ] ); @@ -479,23 +481,23 @@ pub fn ref_implicit_type_multiple_files() { bauble::bauble_test!( [Test] [ - "t = integration::Test{ x: -5, y: 5 }", - "r = $test0::t" + "0 = integration::Test{ x: -5, y: 5 }", + "0 = $test0" ] [ Test { x: -5, y: 5 }, - Ref::::from_path(TypePath::new_unchecked("test0::t").to_owned()), + Ref::::from_path(TypePath::new_unchecked("test0").to_owned()), ] ); bauble::bauble_test!( [Test] [ - "r = $test1::t", - "t = integration::Test{ x: -5, y: 5 }" + "0 = $test1", + "0 = integration::Test{ x: -5, y: 5 }" ] [ - Ref::::from_path(TypePath::new_unchecked("test1::t").to_owned()), + Ref::::from_path(TypePath::new_unchecked("test1").to_owned()), Test { x: -5, y: 5 }, ] ); @@ -509,7 +511,7 @@ pub fn ref_explicit_type_incorrect() { bauble::bauble_test!( [Test, Incorrect] - "i: Incorrect = Incorrect(0)\n\ + "0: Incorrect = Incorrect(0)\n\ r: Ref = $test::t\n\ t = integration::Test{ x: -2, y: 2 }" [ @@ -524,13 +526,17 @@ pub fn ref_explicit_type_incorrect() { fn decimal_digits_identifiers() { let a = &test_file!( "a", - "2 = integration::Test { x: -5, y: 5 }\n\ + "0 = integration::Test { x: -5, y: 5 }\n\ + 2 = integration::Test { x: -5, y: 5 }\n\ 123 = integration::Test { x: -5, y: 5 }\n\ + test_ref1 = $0 test_ref2 = $2 - test_ref23 = $123 + test_ref3 = $123 ", Test { x: -5, y: 5 }, Test { x: -5, y: 5 }, + Test { x: -5, y: 5 }, + TestRef("a".into()), TestRef("a::2".into()), TestRef("a::123".into()), ); @@ -627,7 +633,7 @@ impl<'alloc_lifetime> bauble::Bauble<'alloc_lifetime, bauble::DefaultAllocator> fn two_part_field() { let a = &test_file!( "a", - "test = integration::TestNamespaceFieldIdent{ x: -5, mynamespace::y: 5 }", + "0 = integration::TestNamespaceFieldIdent{ x: -5, mynamespace::y: 5 }", TestNamespaceFieldIdent { x: -5, mynamespace_y: 5 @@ -646,8 +652,8 @@ fn two_part_field() { fn name_matching_file_is_simplified() { let a = &TestFile::new( "a", - "a = integration::Test { x: -5, y: 5 } - a_ref = $a", // local and full path are the same here + "0 = integration::Test { x: -5, y: 5 } + a_ref = $0", vec![ Box::new(|object, ctx| { assert!(object.top_level); @@ -659,9 +665,9 @@ fn name_matching_file_is_simplified() { // test non-top-level file let ac = &TestFile::new( "a::c", - "c = integration::Test { x: -5, y: 5 }\n\ - c_ref_local = $c\n\ - c_ref_full = $a::c", + "0 = integration::Test { x: -5, y: 5 }\n\ + ref_local = $0\n\ + ref_full = $a::c", vec![ Box::new(|object, ctx| { assert!(object.top_level); @@ -674,7 +680,7 @@ fn name_matching_file_is_simplified() { // test refering to them from a separate file let b = &test_file!( "b", - "a_ref = $a\n\ + "0 = $a\n\ c_ref = $a::c", TestRef("a".into()), TestRef("a::c".into()), @@ -694,7 +700,7 @@ fn name_matching_file_is_simplified() { fn duplicate_name_after_simplification() { let a = &TestFile::new( "a", - "a = integration::Test { x: -5, y: 5 }\n\ + "0 = integration::Test { x: -5, y: 5 }\n\ 1 = integration::Test { x: -5, y: 5 }", // local and full path are the same here vec![ Box::new(|object, ctx| { @@ -710,7 +716,7 @@ fn duplicate_name_after_simplification() { // test non-top-level file let a1 = &TestFile::new( "a::1", - "1 = integration::Test { x: -5, y: 5 }", + "0 = integration::Test { x: -5, y: 5 }", vec![Box::new(|object, ctx| { assert!(object.top_level); (expected_value_fn(Test { x: -5, y: 5 }))(object, ctx) @@ -728,12 +734,12 @@ fn duplicate_name_after_simplification() { /// Paths won't collide after simplification but we don't want to allow names of objects in the /// same file to collide. #[test] -#[should_panic = "This identifier was already used"] +#[should_panic = "Identifier '0' is only allowed for the first item"] fn duplicate_name_before_simplification() { let a = &TestFile::new( "a", - "a = integration::Test { x: -5, y: 5 }\n\ - a = integration::Test { x: -5, y: 5 }", + "0 = integration::Test { x: -5, y: 5 }\n\ + 0 = integration::Test { x: -5, y: 5 }", vec![ Box::new(|object, ctx| { assert!(object.top_level); @@ -754,4 +760,22 @@ fn duplicate_name_before_simplification() { ); } -// TODO: in stage 2, test that only first object can be named `0`. +#[test] +#[should_panic = "The first item must have '0' as the identifier"] +fn special_identifier_required_for_first_object() { + let a = &TestFile::new( + "a", + "1 = integration::Test { x: -5, y: 5 }", + vec![Box::new(|object, ctx| { + assert!(object.top_level); + (expected_value_fn(Test { x: -5, y: 5 }))(object, ctx) + })], + ); + + test_load( + &|ctx| { + ctx.register_type::(); + }, + &[a], + ); +} diff --git a/bauble_macros/tests/derive.rs b/bauble_macros/tests/derive.rs index 7a0911c..a527397 100644 --- a/bauble_macros/tests/derive.rs +++ b/bauble_macros/tests/derive.rs @@ -14,7 +14,7 @@ fn test_struct() { bauble_test!( [Test] r#" - a = derive::Test { x: -5, y: 5, z: Some(true) } + 0 = derive::Test { x: -5, y: 5, z: Some(true) } "# [Test { x: -5, @@ -31,7 +31,7 @@ fn test_tuple() { bauble_test!( [Test] - "a = derive::Test(-5, 5)" + "0 = derive::Test(-5, 5)" [Test(-5, 5)] ); } @@ -50,7 +50,7 @@ fn test_enum() { r#" use derive::Test; - a = Test::Foo(-10, 2) + 0 = Test::Foo(-10, 2) b = Test::Bar { x: -5, y: 5 } c = Test::Baz "# @@ -84,7 +84,7 @@ fn test_flattened() { r#" use derive::{Test, Test2}; - a: Test = -10 + 0: Test = -10 b: Test = true c: Test2 = #[count = 10] "foo" d: Test2 = "bar" @@ -116,7 +116,7 @@ fn test_std_types() { bauble_test!( [Test] r#" - a = derive::Test { + 0 = derive::Test { a: [(2, 0), (1, -1), (5, 10)], b: { "🔑": [true, true, false], @@ -159,7 +159,7 @@ fn test_complex_flatten() { bauble_test!( [Transparent] r#" - a: derive::Transparent = #[a = 1, b = 2] 3 + 0: derive::Transparent = #[a = 1, b = 2] 3 "# [ Transparent(Inner(3, 0, 2), 1), @@ -199,7 +199,7 @@ fn test_from() { bauble_test!( [NumberRepr, TestEnum] r#" - a: derive::NumberRepr = 1553 + 0: derive::NumberRepr = 1553 b: derive::TestEnum = 555 c: derive::TestEnum = 1333 @@ -222,7 +222,7 @@ fn test_default() { r#" use derive::{Foo, Bar}; - a: Foo = default + 0: Foo = default b: Bar = default c: Foo = #[foo = 2] default d: Bar = #[test = 10] default @@ -360,7 +360,7 @@ fn test_trait() { [r#" use derive::{Trans, Foo, Bar}; - a: Trans = Foo(32) + 0: Trans = Foo(32) b: Trans = "meow" "#] @@ -384,7 +384,7 @@ fn test_generic() { r#" use derive::{Foo, Bar, Str}; - a: Foo = Foo(Bar(24)) + 0: Foo = Foo(Bar(24)) b: Foo = Foo(Str("test")) "# [ From 16218c7a7c8fcdf1f1221cc8cf262b06d58966e2 Mon Sep 17 00:00:00 2001 From: Imbris Date: Wed, 4 Mar 2026 13:58:42 -0500 Subject: [PATCH 03/25] more comments in code --- bauble/src/parse/parser.rs | 13 +++++++------ bauble/src/parse/value.rs | 4 ++++ bauble/src/value/convert.rs | 6 +++++- 3 files changed, 16 insertions(+), 7 deletions(-) diff --git a/bauble/src/parse/parser.rs b/bauble/src/parse/parser.rs index 71e8baa..c734a55 100644 --- a/bauble/src/parse/parser.rs +++ b/bauble/src/parse/parser.rs @@ -516,13 +516,13 @@ pub fn parser<'a>() -> impl Parser<'a, ParserSource<'a>, ParseValues, Extra<'a>> let path_p = path.clone().padded_by(comments).padded(); - // Parser for tuple structs - let unnamed_struct = path_p + // Parser for tuple structs (unnamed fields). + let tuple_struct = path_p .clone() .then(tuple.clone()) .map(|(name, fields)| (Some(name), Value::Struct(FieldsKind::Unnamed(fields)))); - // Parser for structs + // Parser for structs with named fields let named_struct = path_p .clone() .then(structure.clone()) @@ -567,7 +567,8 @@ pub fn parser<'a>() -> impl Parser<'a, ParserSource<'a>, ParseValues, Extra<'a>> .map(|p| p.into_iter().collect())) .map(Value::Or); - let path_value = path + // Parser for unit structs (no fields). + let unit_struct = path .clone() .map(|path: Path| (Some(path), Value::Struct(FieldsKind::Unit))); @@ -603,10 +604,10 @@ pub fn parser<'a>() -> impl Parser<'a, ParserSource<'a>, ParseValues, Extra<'a>> array.map(no_type), tuple.map(no_type), map.map(no_type), - unnamed_struct, + tuple_struct, named_struct, path_or.map(no_type), - path_value, + unit_struct, raw.map(no_type), literal.map(no_type), )) diff --git a/bauble/src/parse/value.rs b/bauble/src/parse/value.rs index 8ee0d8c..4f7855e 100644 --- a/bauble/src/parse/value.rs +++ b/bauble/src/parse/value.rs @@ -116,6 +116,8 @@ pub struct PathTreeNode { #[derive(Debug, Clone)] pub struct ParseVal { + /// Type known from the value (i.e. for struct types) or explicitly specified by prefixing the + /// value with ``. pub ty: Option, pub attributes: Spanned>, pub value: Spanned>, @@ -166,6 +168,8 @@ impl SpannedValue for ParseVal { #[derive(Debug, Clone)] pub struct Binding { + /// Type explicitly specified in the binding definition. This is the syntax where `: type` + /// appears after the identifier before `=`. pub type_path: Option, pub value: ParseVal, } diff --git a/bauble/src/value/convert.rs b/bauble/src/value/convert.rs index dddf68a..7701fbc 100644 --- a/bauble/src/value/convert.rs +++ b/bauble/src/value/convert.rs @@ -92,14 +92,18 @@ fn resolve_type( Ok(ty) } +/// Returns type if it can be determined from the provided `ParseVal` without further context +/// (other than resolving types and looking at the type of referenced objects). pub(super) fn value_type(value: &ParseVal, symbols: &Symbols) -> Result>> { let types = symbols.ctx.type_registry(); + // Type was specified via a prefixed "" or this is value where the type is explicit like a struct value. if let Some(ty) = &value.ty { return Ok(Some(symbols.resolve_type(ty)?.spanned(ty.span()))); }; let ty = match &*value.value { + // Determine referenced value type by looking up referenced path. Value::Ref(path) => Some(symbols.resolve_asset(path)?.0.spanned(path.span())), Value::Or(paths) => { let mut ty = None; @@ -115,8 +119,8 @@ pub(super) fn value_type(value: &ParseVal, symbols: &Symbols) -> Result { if let Some(instance) = types.iter_type_set(type_set).next() && let types::TypeKind::EnumVariant { - fields: types::Fields::Unit, enum_type, + fields: types::Fields::Unit, .. } = &types.key_type(instance).kind { From e3e91d14b76ce0eba8fcbe910f91386c9d9d794c Mon Sep 17 00:00:00 2001 From: Imbris Date: Fri, 6 Mar 2026 16:09:20 -0500 Subject: [PATCH 04/25] Test that generic types that are referenced are resolved properly (this currently fails) --- bauble_macros/tests/derive.rs | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/bauble_macros/tests/derive.rs b/bauble_macros/tests/derive.rs index a527397..75ea381 100644 --- a/bauble_macros/tests/derive.rs +++ b/bauble_macros/tests/derive.rs @@ -1,6 +1,6 @@ use std::collections::HashMap; -use bauble::{Bauble, SpannedValue, bauble_test}; +use bauble::{Bauble, Ref, SpannedValue, bauble_test, path::TypePath}; #[test] fn test_struct() { @@ -385,11 +385,15 @@ fn test_generic() { use derive::{Foo, Bar, Str}; 0: Foo = Foo(Bar(24)) - b: Foo = Foo(Str("test")) + b: Ref> = $test::c + c: Foo = Foo(Str("test")) + d: Ref> = $0 "# [ Foo(Bar(24)), + Ref::>::from_path(TypePath::new("test::c").unwrap().to_owned()), Foo(Str(String::from("test"))), + Ref::>::from_path(TypePath::new("test").unwrap().to_owned()), ] ); } From 9059ef26890447bab0b151f3b78dcc9231fb45db Mon Sep 17 00:00:00 2001 From: Imbris Date: Fri, 6 Mar 2026 16:53:27 -0500 Subject: [PATCH 05/25] Another reference pattern where type resolution doesn't work --- bauble/tests/integration.rs | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/bauble/tests/integration.rs b/bauble/tests/integration.rs index 48fdcf2..7866161 100644 --- a/bauble/tests/integration.rs +++ b/bauble/tests/integration.rs @@ -430,19 +430,25 @@ pub fn ref_implicit_type() { pub fn ref_explicit_type() { bauble::bauble_test!( [Test] - "0 = integration::Test{ x: -2, y: 2 }\n\ - r: Ref = $0" + "use integration::Test;\n\ + 0 = integration::Test{ x: -2, y: 2 }\n\ + r1: Ref = $0\n\ + r2: Ref = $0" [ Test { x: -2, y: 2 }, Ref::::from_path(TypePath::new_unchecked("test").to_owned()), + Ref::::from_path(TypePath::new_unchecked("test").to_owned()), ] ); bauble::bauble_test!( [Test] - "0: Ref = $test::t\n\ + "use integration::Test;\n\ + 0: Ref = $test::t\n\ + r2: Ref = $test::t\n\ t = integration::Test{ x: -2, y: 2 }" [ + Ref::::from_path(TypePath::new_unchecked("test::t").to_owned()), Ref::::from_path(TypePath::new_unchecked("test::t").to_owned()), Test { x: -2, y: 2 }, ] From 4519d348a89016c2a3e8a1b6948fa5873b465f44 Mon Sep 17 00:00:00 2001 From: Imbris Date: Mon, 9 Mar 2026 12:07:11 -0400 Subject: [PATCH 06/25] Fix a couple issues with resolve_path usage and generic types (tests added in previous commits), also minimize places using `Symbols` --- bauble/src/context.rs | 13 +++- bauble/src/value/convert.rs | 30 ++++----- bauble/src/value/mod.rs | 28 +++++---- bauble/src/value/symbols.rs | 118 +++++++++++++++--------------------- 4 files changed, 88 insertions(+), 101 deletions(-) diff --git a/bauble/src/context.rs b/bauble/src/context.rs index aa69636..abf451e 100644 --- a/bauble/src/context.rs +++ b/bauble/src/context.rs @@ -526,6 +526,13 @@ impl BaubleContext { self.files.iter().map(|e| (e.0.borrow(), e.1.text())) } + /// Given an asset type registers the internal Ref type for that asset and returns its ID. + pub(crate) fn register_asset_ref_ty(&mut self, ty: TypeId) -> TypeId { + let ref_ty = self.registry.get_or_register_asset_ref(ty); + self.root_node.build_type(ref_ty, &self.registry); + ref_ty + } + /// Registers an asset. This is done automatically for any objects in a file that gets registered. /// /// With this method you can expose assets that aren't in bauble. @@ -544,10 +551,10 @@ impl BaubleContext { ty: TypeId, kind: AssetKind, ) -> Result { - let ref_ty = self.registry.get_or_register_asset_ref(ty); - self.root_node.build_type(ref_ty, &self.registry); + let ref_ty = self.register_asset_ref_ty(ty); self.root_node .build_asset(path, ref_ty, kind) + // TODO: this error should no longer be possible? .map_err(|()| { crate::CustomError::new(format!( "'{path}' refers to an existing asset in another file. This can be \n\ @@ -686,7 +693,7 @@ impl BaubleContext { // Skip files with errors .filter(|(file, _)| skip_iter.next_if_eq(file).is_none()) { - match crate::value::convert_values(file, values, &crate::value::Symbols::new(&*self)) { + match crate::value::convert_values(file, values, &self) { Ok(o) => objects.extend(o), Err(e) => errors.extend(e), } diff --git a/bauble/src/value/convert.rs b/bauble/src/value/convert.rs index 7701fbc..cf06f9e 100644 --- a/bauble/src/value/convert.rs +++ b/bauble/src/value/convert.rs @@ -5,7 +5,7 @@ use crate::{ parse::ParseVal, path::{TypePath, TypePathElem}, spanned::{SpanExt, Spanned}, - types::{self, TypeId}, + types::{self, TypeId, TypeRegistry}, value::{ Attributes, Fields, Ident, SpannedValue, Symbols, UnspannedVal, Val, Value, ValueContainer, ValueTrait, error::Result, @@ -52,17 +52,16 @@ fn set_attributes( } fn resolve_type( - symbols: &Symbols, + types: &TypeRegistry, expected_type: TypeId, val_type: &mut Option>, primitive_type: Option, span: crate::Span, ) -> Result> { - let types = symbols.ctx.type_registry(); let ty = if types.key_type(expected_type).kind.instanciable() { expected_type.spanned(val_type.map(|s| s.span).unwrap_or(span)) } else { - match default_value_type(symbols, primitive_type, *val_type) { + match default_value_type(types, primitive_type, *val_type) { Some(ty) => { let ty = ty.spanned(val_type.map_or(span, |s| s.span)); *val_type = Some(ty); @@ -186,11 +185,10 @@ pub(super) fn value_type(value: &ParseVal, symbols: &Symbols) -> Result, value_type: Option>, ) -> Option { - let types = symbols.ctx.type_registry(); if let Some(value_type) = value_type { let enum_type = match &types.key_type(value_type.value).kind { types::TypeKind::EnumVariant { enum_type, .. } => *enum_type, @@ -280,7 +278,7 @@ impl AdditionalObjects { &mut self, name: TypePathElem<&str>, val: Val, - symbols: &Symbols, + types: &TypeRegistry, ) -> Result { let idx = *self .name_allocs @@ -294,7 +292,7 @@ impl AdditionalObjects { self.file_path.join(&name), false, val, - symbols, + types, )?); Ok(Value::Ref(self.file_path.join(&name))) @@ -308,7 +306,7 @@ impl AdditionalObjects { &mut self, span: Span, object_name: TypePathElem<&str>, - symbols: &Symbols, + types: &TypeRegistry, f: impl FnOnce(&mut AdditionalUnspannedObjects) -> R, ) -> Result { let mut unspanned = AdditionalUnspannedObjects::new_with_name_allocs( @@ -324,7 +322,7 @@ impl AdditionalObjects { self.file_path.join(&name), false, value.into_spanned(span), - symbols, + types, )?); } @@ -487,15 +485,15 @@ where ) .then_some(expected_type.spanned(value.span))); + let types = meta.symbols.ctx.type_registry(); let ty_id = resolve_type( - meta.symbols, + types, expected_type, &mut val_ty, value.value.primitive_type(), value.span, )?; - let types = meta.symbols.ctx.type_registry(); let ty = types.key_type(ty_id.value); let span = value.span; @@ -825,7 +823,7 @@ where let mut v = meta.additional_objects.with_additional_unspanned( span, meta.object_name, - meta.symbols, + meta.symbols.ctx.type_registry(), |additional| { ty.meta .default @@ -1075,8 +1073,9 @@ where *ty, )?; + let types = meta.symbols.ctx.type_registry(); meta.additional_objects - .add_object(object_name.borrow(), val, meta.symbols)? + .add_object(object_name.borrow(), val, types)? } (types::TypeKind::Primitive(primitive), Value::Primitive(value)) if !matches!(value, PrimitiveValue::Default) => @@ -1140,10 +1139,11 @@ where )?; } (_, Value::Primitive(PrimitiveValue::Default)) => { + let types = meta.symbols.ctx.type_registry(); let mut v = meta.additional_objects.with_additional_unspanned( span, meta.object_name, - meta.symbols, + types, |additional| -> Result<_> { Ok(types .instantiate(*ty_id, additional) diff --git a/bauble/src/value/mod.rs b/bauble/src/value/mod.rs index 5394bbb..929fffa 100644 --- a/bauble/src/value/mod.rs +++ b/bauble/src/value/mod.rs @@ -9,7 +9,7 @@ use crate::{ parse::{BindingIdent, ParseVal, ParseValues, Path, PathEnd}, path::{TypePath, TypePathElem}, spanned::{SpanExt, Spanned}, - types::{self, TypeId}, + types::{self, TypeId, TypeRegistry}, }; mod convert; @@ -795,7 +795,7 @@ pub(crate) fn register_assets( let res = value_type(&binding.value, &symbols) .map(|v| { convert::default_value_type( - &symbols, + symbols.ctx.type_registry(), binding.value.value.value.primitive_type(), v, ) @@ -814,10 +814,10 @@ pub(crate) fn register_assets( if res.is_err() && let Value::Ref(reference) = &*binding.value.value - && let Ok(reference) = symbols.resolve_path(reference) + && let Ok(reference) = symbols.resolve_path(reference, false) { let expected_ty_path = if let Some(expected_ty_path) = &binding.type_path { - match symbols.resolve_path(expected_ty_path) { + match symbols.resolve_path(expected_ty_path, true) { Ok(s) => Some(s), Err(e) => { errors.push(e); @@ -877,9 +877,9 @@ pub(crate) fn register_assets( pub(crate) fn convert_values( file: FileId, values: ParseValues, - default_symbols: &Symbols, + ctx: &crate::context::BaubleContext, ) -> std::result::Result, BaubleErrors> { - let mut use_symbols = Symbols::new(default_symbols.ctx); + let mut use_symbols = Symbols::new(ctx); let mut use_errors = Vec::new(); for use_path in values.uses { if let Err(e) = use_symbols.add_use(&use_path) { @@ -887,11 +887,11 @@ pub(crate) fn convert_values( } } - let mut symbols = default_symbols.clone(); + let mut symbols = Symbols::new(ctx); let file_path = symbols.ctx.get_file_path(file); - // Add assets from this file to symbols. + // Add assets from this file to Symbols::use. for ident in values.values.keys() { let span = ident.span(); let (ident, path) = object_ident_path(file_path, &ident); @@ -941,7 +941,8 @@ pub(crate) fn convert_values( } }; - let ty = match symbols.ctx.type_registry().key_type(ref_ty).kind { + let type_registry = symbols.ctx.type_registry(); + let ty = match type_registry.key_type(ref_ty).kind { types::TypeKind::Ref(type_id) => type_id, _ => unreachable!("Invariant"), }; @@ -981,16 +982,17 @@ fn convert_object( mut meta: ConvertMeta, ) -> Result { let value = value.convert(meta.reborrow(), expected_type, no_attr())?; - create_object(object_path, top_level, value, meta.symbols) + let types = meta.symbols.ctx.type_registry(); + create_object(object_path, top_level, value, types) } fn create_object( object_path: TypePath, top_level: bool, value: Val, - symbols: &Symbols, + type_registry: &TypeRegistry, ) -> Result { - if symbols.ctx.type_registry().impls_top_level_trait(*value.ty) { + if type_registry.impls_top_level_trait(*value.ty) { Ok(Object { object_path, top_level, @@ -998,7 +1000,7 @@ fn create_object( }) } else { Err(ConversionError::MissingRequiredTrait { - tr: symbols.ctx.type_registry().top_level_trait(), + tr: type_registry.top_level_trait(), ty: *value.ty, } .spanned(value.span())) diff --git a/bauble/src/value/symbols.rs b/bauble/src/value/symbols.rs index 5125eb4..f30bf14 100644 --- a/bauble/src/value/symbols.rs +++ b/bauble/src/value/symbols.rs @@ -157,7 +157,7 @@ impl<'a> Symbols<'a> { .and_then(|reference| reference.module.clone()) } - pub fn resolve_path(&self, raw_path: &Path) -> Result> { + pub fn resolve_path(&self, raw_path: &Path, for_type: bool) -> Result> { let mut leading = TypePath::empty(); let mut path_iter = raw_path.leading.iter(); @@ -195,19 +195,35 @@ impl<'a> Symbols<'a> { } } + let generic_with_non_type_error = || { + ConversionError::Custom(CustomError::new("Cannot use generics for non-type paths")) + .spanned(raw_path.span()) + }; let path = match &raw_path.last.value { PathEnd::WithIdent(ident) => PathKind::Indirect( leading, TypePathElem::new(ident.to_string()).map_err(|p| p.spanned(raw_path.span()))?, ), PathEnd::Ident(ident) => { - leading - .push_str(ident.as_str()) - .map_err(|p| p.spanned(raw_path.span()))?; - PathKind::Direct(leading) + let path = if leading.is_empty() + && for_type + && let Some(r) = self.uses.get(ident.as_str()) + && let Some(ty) = r.ty + { + self.ctx.type_registry().key_type(ty).meta.path.clone() + } else { + leading + .push_str(ident.as_str()) + .map_err(|p| p.spanned(raw_path.span()))?; + leading + }; + PathKind::Direct(path) } PathEnd::WithIdentGeneric(ident, generic) => { - let generic = self.resolve_path(&generic.value)?; + if !for_type { + return Err(generic_with_non_type_error()); + } + let generic = self.resolve_path(&generic.value, true)?; PathKind::Indirect( leading, TypePathElem::new(format!("{ident}<{generic}>")) @@ -215,11 +231,24 @@ impl<'a> Symbols<'a> { ) } PathEnd::IdentGeneric(ident, generic) => { - let generic = self.resolve_path(&generic.value)?; - leading - .push_str(&format!("{ident}<{generic}>")) - .map_err(|p| p.spanned(raw_path.span()))?; - PathKind::Direct(leading) + if !for_type { + return Err(generic_with_non_type_error()); + } + let generic = self.resolve_path(&generic.value, true)?; + let path = if leading.is_empty() + && let Some(r) = self.uses.get(ident.as_str()) + && let Some(ty) = r.ty + { + let outer_path = &self.ctx.type_registry().key_type(ty).meta.path; + TypePath::new(format!("{outer_path}<{generic}>")) + .map_err(|p| p.spanned(raw_path.span()))? + } else { + leading + .push_str(&format!("{ident}<{generic}>")) + .map_err(|p| p.spanned(raw_path.span()))?; + leading + }; + PathKind::Direct(path) } }; Ok(path.spanned(raw_path.span())) @@ -230,63 +259,12 @@ impl<'a> Symbols<'a> { raw_path: &Path, ref_kind: RefKind, ) -> Result> { - fn resolve_path( - symbols: &Symbols, - raw_path: &Path, - ref_kind: RefKind, - ) -> Result> { - let raw_path_split = raw_path.split_generic(); - let is_generic = raw_path_split.is_some(); - let (path, &generic) = raw_path_split - .as_ref() - .map(|(l, r)| (l, r)) - .unwrap_or((raw_path, &raw_path)); - - let path = symbols.resolve_path(path)?; - - Ok(if matches!(ref_kind, RefKind::Type) { - match path.value { - PathKind::Direct(type_path) => { - if let Some(r) = symbols.uses.get(type_path.as_str()) - && let Some(ty) = r.ty - { - let path = &symbols.ctx.type_registry().key_type(ty).meta.path; - if is_generic { - let generic = resolve_path(symbols, generic, ref_kind)?; - PathKind::Direct( - TypePath::new(format!("{path}<{generic}>")).unwrap(), - ) - } else { - PathKind::Direct(TypePath::new(path.to_string()).unwrap()) - } - } else if is_generic { - let generic = resolve_path(symbols, generic, ref_kind)?; - PathKind::Direct( - TypePath::new(format!("{type_path}<{generic}>")).unwrap(), - ) - } else { - PathKind::Direct(TypePath::new(format!("{type_path}")).unwrap()) - } - } - PathKind::Indirect(type_path, type_path_elem) => { - if is_generic { - let generic = resolve_path(symbols, generic, ref_kind)?; - PathKind::Indirect( - type_path, - TypePathElem::new(format!("{type_path_elem}<{generic}>")).unwrap(), - ) - } else { - PathKind::Indirect(type_path, type_path_elem) - } - } - } - .spanned(path.span) - } else { - path - }) - } - - let path = resolve_path(self, raw_path, ref_kind)?; + // NOTE: Resolve path resolves the full path for types referred to via `uses`. This is + // unnecessary for the usage here (outside of types with generic parameters) because the + // type info is available directly in `self.uses`. However, this is neccessary for other + // uses of `resolve_path` where the type needs to be looked up later when `Symbols` is not + // available. + let path = self.resolve_path(raw_path, matches!(ref_kind, RefKind::Type))?; let reference = match &path.value { PathKind::Direct(path) => { @@ -336,7 +314,7 @@ impl<'a> Symbols<'a> { } else { Err(ConversionError::RefError(Box::new(RefError { uses: Some(self.uses.clone()), - path: self.resolve_path(path)?.value, + path: self.resolve_path(path, false)?.value, path_ref: item, kind: RefKind::Asset, })) @@ -352,7 +330,7 @@ impl<'a> Symbols<'a> { } else { Err(ConversionError::RefError(Box::new(RefError { uses: Some(self.uses.clone()), - path: self.resolve_path(path)?.value, + path: self.resolve_path(path, true)?.value, path_ref: item.into_owned(), kind: RefKind::Type, })) From bea87f5d4e2f749f753b66f017b98088a1c0675a Mon Sep 17 00:00:00 2001 From: Imbris Date: Mon, 9 Mar 2026 12:07:48 -0400 Subject: [PATCH 07/25] Add LocalContext type (not used yet) --- bauble/src/lib.rs | 1 + bauble/src/local_context.rs | 53 +++++++++++++++++++++++++++++++++++++ 2 files changed, 54 insertions(+) create mode 100644 bauble/src/local_context.rs diff --git a/bauble/src/lib.rs b/bauble/src/lib.rs index 4504fb4..dd4d5e1 100644 --- a/bauble/src/lib.rs +++ b/bauble/src/lib.rs @@ -5,6 +5,7 @@ mod builtin; mod context; mod error; +mod local_context; mod parse; mod spanned; mod traits; diff --git a/bauble/src/local_context.rs b/bauble/src/local_context.rs new file mode 100644 index 0000000..123351f --- /dev/null +++ b/bauble/src/local_context.rs @@ -0,0 +1,53 @@ +use crate::context::BaubleContext; +use crate::path::{TypePath, TypePathElem}; +use crate::types::TypeId; +use indexmap::IndexMap; + +/// Assets are only referencable from other assets in the same file. +pub(crate) struct LocalAsset { + /// Type of a Ref to this asset. + ty: TypeId, + // TODO: might not be necessary? + /// Full path to this asset. + /// + /// This is the path to reach this node from the root. + path: TypePath, +} + +pub(crate) struct LocalAssets { + assets: IndexMap, +} + +/// Transient context that is built and used when loading a file. +/// +/// Contains registery of local assets. +pub(crate) struct LocalContext<'a> { + local: IndexMap, + ctx: &'a mut BaubleContext, +} + +impl LocalContext<'_> { + /// Returns ID of internal Ref type for `ty`. + pub fn register_asset(&mut self, path: TypePath, ty: TypeId) -> TypeId { + let ref_ty = self.ctx.register_asset_ref_ty(ty); + let asset = LocalAsset { ty: ref_ty, path }; + let (file, ident) = asset + .path + .get_end() + .expect("Register asset with empty path"); + let assets = &mut self + .local + .entry(file.to_owned()) + .or_insert_with(|| LocalAssets { + assets: IndexMap::new(), + }) + .assets; + if assets.contains_key(ident.as_str()) { + panic!("{} refers to an existing asset", asset.path); + } else { + assets.insert(ident.to_owned(), asset); + } + + ref_ty + } +} From de0cbf4340281951ef8d4f7972d52cb9f38c5c7d Mon Sep 17 00:00:00 2001 From: Imbris Date: Wed, 11 Mar 2026 11:39:09 -0400 Subject: [PATCH 08/25] Simplify Symbols usage in convert_values and produce error when a use statement conflicts with an asset identifiers in the current file --- bauble/src/value/mod.rs | 42 ++++++++++++++++--------------------- bauble/src/value/symbols.rs | 5 ----- 2 files changed, 18 insertions(+), 29 deletions(-) diff --git a/bauble/src/value/mod.rs b/bauble/src/value/mod.rs index 929fffa..b6bb616 100644 --- a/bauble/src/value/mod.rs +++ b/bauble/src/value/mod.rs @@ -879,16 +879,15 @@ pub(crate) fn convert_values( values: ParseValues, ctx: &crate::context::BaubleContext, ) -> std::result::Result, BaubleErrors> { - let mut use_symbols = Symbols::new(ctx); - let mut use_errors = Vec::new(); + let mut errors = Vec::new(); + + let mut symbols = Symbols::new(ctx); for use_path in values.uses { - if let Err(e) = use_symbols.add_use(&use_path) { - use_errors.push(e); + if let Err(e) = symbols.add_use(&use_path) { + errors.push(e); } } - let mut symbols = Symbols::new(ctx); - let file_path = symbols.ctx.get_file_path(file); // Add assets from this file to Symbols::use. @@ -908,22 +907,17 @@ pub(crate) fn convert_values( module: None, }, ) { - use_errors.push(e.spanned(span)); + errors.push(e.spanned(span)); } } else { // Didn't pre-register assets. - use_errors.push(ConversionError::UnregisteredAsset.spanned(span)); + errors.push(ConversionError::UnregisteredAsset.spanned(span)); } } - symbols.add(use_symbols); - - let mut additional_objects = AdditionalObjects::new(file_path.to_owned()); - let default_span = crate::Span::new(file, 0..0); - - let mut ok = Vec::new(); - let mut err = use_errors; + let mut additional_objects = AdditionalObjects::new(file_path.to_owned()); + let mut objects = Vec::new(); for (ident, binding) in &values.values { let ref_ty = match symbols.resolve_asset( @@ -936,7 +930,7 @@ pub(crate) fn convert_values( ) { Ok((ty, _)) => ty, Err(e) => { - err.push(e); + errors.push(e); continue; } }; @@ -944,7 +938,7 @@ pub(crate) fn convert_values( let type_registry = symbols.ctx.type_registry(); let ty = match type_registry.key_type(ref_ty).kind { types::TypeKind::Ref(type_id) => type_id, - _ => unreachable!("Invariant"), + _ => unreachable!("The type registered with an object is always a reference"), }; let top_level = ident.is_top_level(); @@ -957,18 +951,18 @@ pub(crate) fn convert_values( default_span, }; match convert_object(path, top_level, &binding.value, ty, convert_meta) { - Ok(obj) => ok.push(obj), - Err(e) => err.push(e), + Ok(obj) => objects.push(obj), + Err(e) => errors.push(e), } } - let mut objects = additional_objects.into_objects(); + let mut all_objects = additional_objects.into_objects(); - if err.is_empty() { - objects.extend(ok); - Ok(objects) + if errors.is_empty() { + all_objects.extend(objects); + Ok(all_objects) } else { - Err(err.into()) + Err(errors.into()) } } diff --git a/bauble/src/value/symbols.rs b/bauble/src/value/symbols.rs index f30bf14..2a634b7 100644 --- a/bauble/src/value/symbols.rs +++ b/bauble/src/value/symbols.rs @@ -45,11 +45,6 @@ impl<'a> Symbols<'a> { Ok(()) } - pub fn add(&mut self, symbols: Symbols) { - // TODO: what about conflicting entries? - self.uses.extend(symbols.uses) - } - pub fn add_use(&mut self, use_path: &Spanned) -> Result<()> { fn add_use_inner( this: &mut Symbols, From 13dbbfa12bfb57a3ed906eb615309c0e2ddf8119 Mon Sep 17 00:00:00 2001 From: Imbris Date: Wed, 11 Mar 2026 11:48:05 -0400 Subject: [PATCH 09/25] Clarify note on delaying type resolution for references --- bauble/src/value/mod.rs | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/bauble/src/value/mod.rs b/bauble/src/value/mod.rs index b6bb616..5886a67 100644 --- a/bauble/src/value/mod.rs +++ b/bauble/src/value/mod.rs @@ -780,14 +780,12 @@ pub(crate) fn register_assets( // To register an asset we need to determine its type. let ty = if let Some(ty) = &binding.type_path - // If the value is a reference, then an explicit type should - // still be delayed. The reason why it should be delayed is - // because the type of the reference `Ref`, where `T` is - // the type of the referenced object, may not exist at this - // point, and as such trying to resolve the type may cause - // issues because the type is not known and depends on the - // order the reference was created compared to the - // referenced object, which is undesired behaviour. + // If the explicit type is a reference, resolving to a type ID + // should be delayed. The type of the reference `Ref` is only + // registered when registering an object of type `T`. So the when + // the referenced object has yet to be registered, the reference + // type may not exist. Thus, trying to resolve the type at this + // point can fail. && !matches!(binding.value.value.value, Value::Ref(_)) { symbols.resolve_type(ty) From bb22e449a750d852e5dd89a1a6bf9db249a42ac3 Mon Sep 17 00:00:00 2001 From: Imbris Date: Wed, 11 Mar 2026 12:20:52 -0400 Subject: [PATCH 10/25] More notes on reference type resolution --- bauble/src/value/mod.rs | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/bauble/src/value/mod.rs b/bauble/src/value/mod.rs index 5886a67..c8f7b77 100644 --- a/bauble/src/value/mod.rs +++ b/bauble/src/value/mod.rs @@ -780,20 +780,20 @@ pub(crate) fn register_assets( // To register an asset we need to determine its type. let ty = if let Some(ty) = &binding.type_path - // If the explicit type is a reference, resolving to a type ID - // should be delayed. The type of the reference `Ref` is only + // If the value is a reference, resolving an explicit type to a type + // ID should be delayed. The type of the reference `Ref` is only // registered when registering an object of type `T`. So the when // the referenced object has yet to be registered, the reference // type may not exist. Thus, trying to resolve the type at this // point can fail. - && !matches!(binding.value.value.value, Value::Ref(_)) + && !matches!(&*binding.value.value, Value::Ref(_)) { symbols.resolve_type(ty) } else { let res = value_type(&binding.value, &symbols) .map(|v| { convert::default_value_type( - symbols.ctx.type_registry(), + ctx.type_registry(), binding.value.value.value.primitive_type(), v, ) @@ -810,11 +810,21 @@ pub(crate) fn register_assets( } }); + // We rely on this property so that an explicit type in + // `binding.type_path` is not ignored. + debug_assert!( + res.is_err() || !matches!(&*binding.value.value, Value::Ref(_)), + "Initial resolution steps should always fail for reference values", + ); + if res.is_err() && let Value::Ref(reference) = &*binding.value.value + // TODO: Will the error be helpful when this part fails, since + // we just pass on the error from an earlier step? && let Ok(reference) = symbols.resolve_path(reference, false) { let expected_ty_path = if let Some(expected_ty_path) = &binding.type_path { + // Resolving to a full path won't fail even if the reference type is not yet registered. match symbols.resolve_path(expected_ty_path, true) { Ok(s) => Some(s), Err(e) => { From 98c5d9764418ed0c2df1c47a3b8d08cc99d21d96 Mon Sep 17 00:00:00 2001 From: Imbris Date: Thu, 12 Mar 2026 12:30:10 -0400 Subject: [PATCH 11/25] Fix should_panic test that had the wrong panic and add a new failing should_panic test for explicit ref type being wrong when referencing something from another file that is already registered --- bauble/tests/integration.rs | 25 ++++++++++++++++++++++--- 1 file changed, 22 insertions(+), 3 deletions(-) diff --git a/bauble/tests/integration.rs b/bauble/tests/integration.rs index 7866161..897ca17 100644 --- a/bauble/tests/integration.rs +++ b/bauble/tests/integration.rs @@ -510,15 +510,15 @@ pub fn ref_implicit_type_multiple_files() { } #[test] -#[should_panic = "Expected this path to refer to a type"] +#[should_panic = "Expected `Ref` which is a reference to `integration::Incorrect`, which is a struct with unnamed fields, but got `Ref` which is a reference to `integration::Test`, which is a struct with named fields"] pub fn ref_explicit_type_incorrect() { #[derive(Bauble, PartialEq, Eq, Debug)] struct Incorrect(u32); bauble::bauble_test!( [Test, Incorrect] - "0: Incorrect = Incorrect(0)\n\ - r: Ref = $test::t\n\ + "0: integration::Incorrect = Incorrect(0)\n\ + r: Ref = $test::t\n\ t = integration::Test{ x: -2, y: 2 }" [ Incorrect(0), @@ -528,6 +528,25 @@ pub fn ref_explicit_type_incorrect() { ); } +#[test] +#[should_panic = "TODO"] +pub fn ref_explicit_type_incorrect_multiple_files() { + #[derive(Bauble, PartialEq, Eq, Debug)] + struct Incorrect(u32); + + bauble::bauble_test!( + [Test, Incorrect] + [ + "0 = integration::Test{ x: -5, y: 5 }", + "0: Ref = $test0" + ] + [ + Test { x: -5, y: 5 }, + Ref::::from_path(TypePath::new_unchecked("test0").to_owned()), + ] + ); +} + #[test] fn decimal_digits_identifiers() { let a = &test_file!( From 479e7b3fdf1aee5046c0924a03ab43043b4603a7 Mon Sep 17 00:00:00 2001 From: Imbris Date: Thu, 12 Mar 2026 12:58:53 -0400 Subject: [PATCH 12/25] Fix case where explicit type could be ignored for reference values --- bauble/src/value/mod.rs | 27 ++++++++++++++++++--------- bauble/tests/integration.rs | 22 +++++++++++++++++++++- 2 files changed, 39 insertions(+), 10 deletions(-) diff --git a/bauble/src/value/mod.rs b/bauble/src/value/mod.rs index c8f7b77..536d651 100644 --- a/bauble/src/value/mod.rs +++ b/bauble/src/value/mod.rs @@ -767,7 +767,9 @@ pub(crate) fn register_assets( // points. let Symbols { mut uses, .. } = symbols; - // TODO: Register these in a correct order to allow for assets referencing assets. + // TODO: Register these in a correct order to allow for assets referencing assets. (NOTE: This + // is impossible to do in all cases since some of those assets may need to be delayed anyway + // due to referencing assets external to this file that are not registered yet). for (ident, binding) in &values.values { let span = ident.span(); let kind = if ident.is_top_level() { @@ -810,13 +812,6 @@ pub(crate) fn register_assets( } }); - // We rely on this property so that an explicit type in - // `binding.type_path` is not ignored. - debug_assert!( - res.is_err() || !matches!(&*binding.value.value, Value::Ref(_)), - "Initial resolution steps should always fail for reference values", - ); - if res.is_err() && let Value::Ref(reference) = &*binding.value.value // TODO: Will the error be helpful when this part fails, since @@ -846,7 +841,21 @@ pub(crate) fn register_assets( continue; } - res + if let Ok(_res) = res + // We skipped `resolve_type` above because `Ref` might not be registered, but if we + // found referenced asset in `value_type`, then the asset is already registered so + // `Ref` will either also be registered or it will be the wrong type. + && let Some(ty) = &binding.type_path + { + // TODO: Unfortunately, the error is "Expected this path to refer to a type" when + // the `Ref` isn't registered which is misleading as the actual issue is that it + // is the wrong type for `T`. Maybe we should just register the `Ref` when + // registering each new type `T`, rather than when encountering an asset of type + // `T`. Is there any downside to this approach? + symbols.resolve_type(ty) + } else { + res + } }; Symbols { uses, .. } = symbols; diff --git a/bauble/tests/integration.rs b/bauble/tests/integration.rs index 897ca17..80c78dc 100644 --- a/bauble/tests/integration.rs +++ b/bauble/tests/integration.rs @@ -529,7 +529,7 @@ pub fn ref_explicit_type_incorrect() { } #[test] -#[should_panic = "TODO"] +#[should_panic = "Expected this path to refer to a type"] pub fn ref_explicit_type_incorrect_multiple_files() { #[derive(Bauble, PartialEq, Eq, Debug)] struct Incorrect(u32); @@ -547,6 +547,26 @@ pub fn ref_explicit_type_incorrect_multiple_files() { ); } +#[test] +#[should_panic = "Expected `Ref` which is a reference to `integration::Incorrect`, which is a struct with unnamed fields, but got `Ref` which is a reference to `integration::Test`, which is a struct with named fields"] +pub fn ref_explicit_type_incorrect_multiple_files_ref_already_registered() { + #[derive(Bauble, PartialEq, Eq, Debug)] + struct Incorrect(u32); + + bauble::bauble_test!( + [Test, Incorrect] + [ + "0 = integration::Test{ x: -5, y: 5 }", + "0 = integration::Incorrect(345)", + "0: Ref = $test0" + ] + [ + Test { x: -5, y: 5 }, + Ref::::from_path(TypePath::new_unchecked("test0").to_owned()), + ] + ); +} + #[test] fn decimal_digits_identifiers() { let a = &test_file!( From 1f8d55e7336ab2c4241d1062e317aa72432adc99 Mon Sep 17 00:00:00 2001 From: Imbris Date: Fri, 13 Mar 2026 12:25:25 -0400 Subject: [PATCH 13/25] Make Symbols::get_module and Symbols::resolve_item private --- bauble/src/value/symbols.rs | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/bauble/src/value/symbols.rs b/bauble/src/value/symbols.rs index 2a634b7..9c1b5df 100644 --- a/bauble/src/value/symbols.rs +++ b/bauble/src/value/symbols.rs @@ -146,7 +146,7 @@ impl<'a> Symbols<'a> { add_use_inner(self, leading, &use_path.end) } - pub fn get_module(&self, ident: &str) -> Option { + fn get_module(&self, ident: &str) -> Option { self.uses .get(ident) .and_then(|reference| reference.module.clone()) @@ -249,11 +249,7 @@ impl<'a> Symbols<'a> { Ok(path.spanned(raw_path.span())) } - pub fn resolve_item( - &self, - raw_path: &Path, - ref_kind: RefKind, - ) -> Result> { + fn resolve_item(&self, raw_path: &Path, ref_kind: RefKind) -> Result> { // NOTE: Resolve path resolves the full path for types referred to via `uses`. This is // unnecessary for the usage here (outside of types with generic parameters) because the // type info is available directly in `self.uses`. However, this is neccessary for other From 8baf6bda6affe1df794cd6d05e8a0cdf193c71bb Mon Sep 17 00:00:00 2001 From: Imbris Date: Wed, 25 Mar 2026 14:38:12 -0400 Subject: [PATCH 14/25] Add new pass when loading files. This pass pre-registers asset/module paths, so we can properly resolve the full path of reference to asset which has yet to be processed in the next pass (where types of assets are resolved). The full path needs to be resolved so that delayed resolution of types can happen without the Symbols structure being available. This would be difficult since the delayed resolution handles assets across multiple files. Part of this change is duplicating structures from `BaubleContext` so we can represent the pending asset/module paths but without requiring that assets have a known type. This is done in the new `early_context` module. --- bauble/src/context.rs | 49 ++- bauble/src/parse/value.rs | 2 +- bauble/src/value/convert.rs | 17 +- bauble/src/value/early_context.rs | 408 +++++++++++++++++++++++++ bauble/src/value/error.rs | 10 +- bauble/src/value/mod.rs | 145 +++++---- bauble/src/value/symbols.rs | 481 +++++++++++++++++++++++++++--- bauble/tests/integration.rs | 39 ++- 8 files changed, 1029 insertions(+), 122 deletions(-) create mode 100644 bauble/src/value/early_context.rs diff --git a/bauble/src/context.rs b/bauble/src/context.rs index abf451e..b2c1de3 100644 --- a/bauble/src/context.rs +++ b/bauble/src/context.rs @@ -12,7 +12,7 @@ pub type Source = ariadne::Source; struct DefaultUses(IndexMap); /// Assets can be either top level or local. -#[derive(Clone, Copy, Debug)] +#[derive(Clone, Copy, Debug, PartialEq)] pub enum AssetKind { // TODO: update this in stage 2 /// The first asset in a file, if it has the same as the file. @@ -228,9 +228,8 @@ struct CtxNode { /// This name can potentially reference: /// * A type /// * An asset - /// * A default use (aka `InnerReference::redirect`) (TODO: actually look into this, I don't - /// understand why these are only added to the root node so I don't actually know how they - /// work) + /// * A default use (aka `InnerReference::redirect`) (NOTE: These are only added to the root + /// node). /// * A module (not represented here but via the `Self::children` field). reference: InnerReference, /// Full path to this node. @@ -649,25 +648,51 @@ impl BaubleContext { } } - let mut delayed = Vec::new(); let mut skip = Vec::new(); + let mut early_ctx = crate::value::EarlyContext::new(self); - // Register assets from each successfully parsed file into the context. + // Register assets paths from each successfully parsed file into the early context (before + // types are known). for (file, values) in file_values.iter() { // Need a partial borrow here. - let (path, _) = self.file(*file); + let (path, _) = early_ctx.ctx.file(*file); let path = path.to_owned(); - match crate::value::register_assets(path.borrow(), self, values) { + match crate::value::pre_register_assets(&mut early_ctx, path.borrow(), values) { + Ok(()) => {} + Err(e) => { + // Skip files that errored on registering. + skip.push(*file); + errors.extend(BaubleErrors::from(e)); + } + } + } + + let mut delayed = Vec::new(); + let mut skip_iter = skip.iter().copied().peekable(); + let mut skip_more = Vec::new(); + + // Then, register assets from each successfully parsed file into the context (while + // resolving types). + for (file, values) in file_values + .iter() + // Skip files with errors + .filter(|(file, _)| skip_iter.next_if_eq(file).is_none()) + { + // Need a partial borrow here. + let (path, _) = early_ctx.ctx.file(*file); + let path = path.to_owned(); + match crate::value::register_assets(&mut early_ctx, path.borrow(), values) { Ok(d) => { delayed.extend(d); } Err(e) => { // Skip files that errored on registering. - skip.push(*file); + skip_more.push(*file); errors.extend(BaubleErrors::from(e)); } } } + skip.extend(skip_more); // TODO: Less hacky way to get which files errored here? if let Err(e) = crate::value::resolve_delayed(delayed, self) { @@ -693,7 +718,7 @@ impl BaubleContext { // Skip files with errors .filter(|(file, _)| skip_iter.next_if_eq(file).is_none()) { - match crate::value::convert_values(file, values, &self) { + match crate::value::convert_values(file, values, self) { Ok(o) => objects.extend(o), Err(e) => errors.extend(e), } @@ -713,7 +738,7 @@ impl BaubleContext { } /// Takes a path in bauble, and if the path is valid, return meta information about the - /// bauble item at that path. + /// bauble item(s) at that path. pub fn get_ref(&self, path: TypePath<&str>) -> Option { self.root_node .node_at(path) @@ -760,7 +785,7 @@ impl BaubleContext { /// `path` doesn't need to be the path of a file, it can be the path of anything in a file. /// /// Note, if a file `a` exists and a file `a::b::c` exists, `a::b` will get the ID of the file - /// at `a`. + /// at `a` even if nothing exists at `a::b`. pub fn get_file_id(&self, path: TypePath<&str>) -> Option { self.root_node .walk_find(path, |node| node.source) diff --git a/bauble/src/parse/value.rs b/bauble/src/parse/value.rs index 4f7855e..39cf5ac 100644 --- a/bauble/src/parse/value.rs +++ b/bauble/src/parse/value.rs @@ -195,7 +195,7 @@ impl BindingIdent { pub fn as_str(&self) -> &str { match self { Self::TopLevel(_) => Self::TOP_LEVEL_IDENTIFIER, - Self::Local(ident) => &*ident, + Self::Local(ident) => ident, } } diff --git a/bauble/src/value/convert.rs b/bauble/src/value/convert.rs index cf06f9e..811f4b4 100644 --- a/bauble/src/value/convert.rs +++ b/bauble/src/value/convert.rs @@ -8,7 +8,7 @@ use crate::{ types::{self, TypeId, TypeRegistry}, value::{ Attributes, Fields, Ident, SpannedValue, Symbols, UnspannedVal, Val, Value, ValueContainer, - ValueTrait, error::Result, + ValueTrait, error::Result, symbols::SymbolsCommon, }, }; @@ -93,8 +93,11 @@ fn resolve_type( /// Returns type if it can be determined from the provided `ParseVal` without further context /// (other than resolving types and looking at the type of referenced objects). -pub(super) fn value_type(value: &ParseVal, symbols: &Symbols) -> Result>> { - let types = symbols.ctx.type_registry(); +pub(super) fn value_type( + value: &ParseVal, + symbols: &S, +) -> Result>> { + let types = symbols.type_registry(); // Type was specified via a prefixed "" or this is value where the type is explicit like a struct value. if let Some(ty) = &value.ty { @@ -103,7 +106,13 @@ pub(super) fn value_type(value: &ParseVal, symbols: &Symbols) -> Result Some(symbols.resolve_asset(path)?.0.spanned(path.span())), + Value::Ref(path) => Some( + symbols + .resolve_asset_type(path)? + // This happens if the asset exists but its type has not been resolved yet. + .ok_or_else(|| ConversionError::UnresolvedType.spanned(path.span()))? + .spanned(path.span()), + ), Value::Or(paths) => { let mut ty = None; for path in paths { diff --git a/bauble/src/value/early_context.rs b/bauble/src/value/early_context.rs new file mode 100644 index 0000000..d795e71 --- /dev/null +++ b/bauble/src/value/early_context.rs @@ -0,0 +1,408 @@ +use crate::context::{AssetKind, BaubleContext, PathReference}; +use crate::path::{TypePath, TypePathElem}; +use crate::types::TypeId; +use indexmap::IndexMap; + +fn try_reduce_option( + a: Option, + b: Option, + f: impl FnOnce(T, T) -> Result, +) -> Result, ()> { + match (a, b) { + (Some(a), Some(b)) => f(a, b).map(Some), + (Some(t), None) | (None, Some(t)) => Ok(Some(t)), + (None, None) => Ok(None), + } +} + +/// - Returns `None` if both are `Some`. +/// - Returns `Some(None)` if both are `None`. +/// - Returns `Some(Some(_))` when only one of the provided options is `Some`. +fn xor_option(a: Option, b: Option) -> Option> { + try_reduce_option(a, b, |_, _| Err(())).ok() +} + +/// A type containing multiple references generally derived from a path. +/// +/// Generalization of both [`PathReference`] and [`EarlyPathReference`]. +/// +/// Unlike [`PathReference`] the type for an asset may not yet be known. +#[derive(Default, Clone, Debug)] +pub(crate) struct CombinedPathReference { + /// The type referenced by a path. + pub ty: Option, + /// The asset (and its path) referenced by the path. + pub asset: Option<(Option, TypePath, AssetKind)>, + /// If the reference references a module. + pub module: Option, +} + +impl CombinedPathReference { + /// Combines an [`EarlyPathReference`] into this. + /// + /// Like [`combined`](Self::combined), except: + /// 1. This mutates `self` in-place. + /// 2. `Some` is allowed in matching fields if the contents are the same. + /// + /// This is used to combine the [`EarlyPathReference`] obtained from pre-registered assets with + /// the [`PathReference`] for existing items in [`BaubleContext`]. During the type resolution + /// process assets/modules will exist in both places as they are registered into + /// [`BaubleContext`]. + /// + /// Returns an error if there is a collision where matching fields both have `Some(_)` but they + /// are for different items so their contents don't match. + fn combine_early(&mut self, other: EarlyPathReference) -> Result<(), ()> { + let EarlyPathReference { + asset: other_asset, + module: other_module, + } = other; + + if let (Some((other_path, other_kind)), Some((_this_ty, this_path, this_kind))) = + (&other_asset, &self.asset) + && (this_path, this_kind) != (other_path, other_kind) + { + return Err(()); + } + if let (Some(other_module), Some(module)) = (&other_module, &self.module) + && module != other_module + { + return Err(()); + } + // Only mutate after checking that no conflicts exist. + if self.asset.is_none() { + self.asset = other_asset.map(|(other_path, other_kind)| (None, other_path, other_kind)); + } + if self.module.is_none() { + self.module = other_module; + } + + Ok(()) + } + + /// Take the exclusive properties of `self` and `other`, essentially "xor"ing them together by producing + /// the combined result where each field where both are `Some` are `None`. + /// + /// Returns `None` if there is a collision (i.e. matching fields are `Some`). + pub(super) fn combined(self, other: Self) -> Option { + Some(Self { + ty: xor_option(self.ty, other.ty)?, + asset: xor_option(self.asset, other.asset)?, + module: xor_option(self.module, other.module)?, + }) + } +} + +/// A type containing multiple references generally derived from a path. +/// +/// Unlike [`PathReference`] the type for an asset is not known. Also this doesn't include +/// information about a potential type referenced by this path. +/// +/// This can be combined into a [`CombinedPathReference`] which can be constructed from a standard +/// [`PathReference`]. +#[derive(Default, Clone, Debug)] +struct EarlyPathReference { + /// The asset (and its path) referenced by the path. + pub asset: Option<(TypePath, AssetKind)>, + /// If the reference references a module. + pub module: Option, +} + +impl EarlyPathReference { + /// Overrides references of `self` with references of `other`, returns true if + /// anything was overriden. + fn combine_override(&mut self, other: Self) -> bool { + let mut o = false; + if other.asset.is_some() { + o = true; + self.asset = other.asset; + } + if other.module.is_some() { + o = true; + self.module = other.module; + } + o + } +} + +impl From for CombinedPathReference { + fn from(reference: EarlyPathReference) -> Self { + Self { + ty: None, + asset: reference.asset.map(|(path, kind)| (None, path, kind)), + module: reference.module, + } + } +} + +impl From for CombinedPathReference { + fn from(reference: PathReference) -> Self { + Self { + ty: reference.ty, + asset: reference + .asset + .map(|(ty, path, kind)| (Some(ty), path, kind)), + module: reference.module, + } + } +} + +/// Represents a name in one or multiple of the asset and module namespaces. +/// +/// If there is a module at this name, this holds a list of the `CtxNode`s in that module. +#[derive(Clone, Debug)] +struct CtxNode { + /// Full path to this node. + /// + /// This is the path to reach this node from the root. + path: TypePath, + /// This name can potentially reference: + /// * An asset + /// * A module (not represented here but via the `Self::children` field). + // TODO: stage 2: only hold top level assets here, local assets should not need to exist here + asset: Option, + children: IndexMap, +} + +impl CtxNode { + fn new(path: TypePath) -> Self { + Self { + path, + asset: None, + children: IndexMap::<_, _>::default(), + } + } + + fn reference(&self) -> EarlyPathReference { + EarlyPathReference { + asset: self.asset.map(|kind| (self.path.clone(), kind)), + module: (!self.children.is_empty()).then(|| self.path.clone()), + } + } + + /// Recursively iterate all children of this node with an optional max depth of `max_depth`. + fn iter_all_children<'a>( + &'a self, + max_depth: Option, + ) -> impl Iterator + Clone { + // pre-order depth first traversal + let mut stack: Vec<(usize, indexmap::map::Values<'a, TypePathElem, CtxNode>)> = Vec::new(); + stack.push((0, self.children.values())); + std::iter::from_fn(move || { + while let Some((depth, iter)) = stack.last_mut() { + let Some(inner) = iter.next() else { + stack.pop(); + continue; + }; + + if max_depth.is_none_or(|d| *depth < d) { + let new_depth = *depth + 1; + stack.push((new_depth, inner.children.values())); + } + + return Some(inner); + } + + None + }) + } + + /// Gets node found at the end of the walking the provided path from the current node. + /// + /// Returns `None` if no node exists at this path. + fn node_at(&self, path: TypePath<&str>) -> Option<&Self> { + if let Some((root, rest)) = path.split_start() { + self.children.get(&root).and_then(|node| node.node_at(rest)) + // Path is empty, get current node. + } else { + Some(self) + } + } + + /// Builds all path elements as modules + fn build_nodes(&mut self, child_path: TypePath<&str>) -> &mut CtxNode { + let Some((child, rest)) = child_path.split_start() else { + return self; + }; + self.add_node(child).build_nodes(rest) + } + + fn add_node(&mut self, child: TypePathElem<&str>) -> &mut Self { + self.children + .entry(child.to_owned()) + .or_insert_with(|| CtxNode::new(self.path.join(&child))) + } + + fn build_asset(&mut self, path: TypePath<&str>, kind: AssetKind) -> Result<(), ()> { + let node = self.build_nodes(path); + if node.asset.is_some() { + // Multiple assets with the same path + return Err(()); + } + + node.asset = Some(kind); + Ok(()) + } +} + +/// New assets that are known to exist but have not had their types resolved and aren't registered +/// in `BaubleContext`. +/// +/// Structured as a node tree because we need to be able to iterate items in a particular module. +pub(crate) struct EarlyContext<'a> { + /// Already registered types, assets, and modules. Used to look these up to check for conflicts + /// and combine with the pending assets when querying for items. + /// + /// This does not need mutable access but the consumer of `EarlyContext` needs to mutate + /// `BaubleContext`. + pub(crate) ctx: &'a mut BaubleContext, + /// Pending assets and modules that have not fully loaded yet. At this point their types are + /// unknown. When their types are resolved they will be registered into `BaubleContext`. + root_node: CtxNode, +} + +impl<'a> EarlyContext<'a> { + pub fn new(ctx: &'a mut BaubleContext) -> Self { + let root_node = CtxNode::new(TypePath::empty()); + Self { ctx, root_node } + } + + /// Registers an asset. This is done automatically for any objects in a file that gets registered. + /// + /// With this method you can expose assets that aren't in bauble. + /// + /// Returns ID of internal Ref type for `ty`. + /// + /// Returns an error if an asset was already registered at this path. This can occur due to + /// simplification of the top level asset path to match the current file. E.g. the top-level + /// asset in `a::1` will conflict with the path of object `1` in file `a`. + // + // TODO: in stage 2 we might be able to adjust this so that local object paths are + // distinguished from top level objects, such that they don't clash. + pub fn register_asset( + &mut self, + path: TypePath<&str>, + kind: AssetKind, + ) -> Result<(), crate::CustomError> { + // Make sure asset doesn't already exist in `BaubleContext`. + if self.ctx.get_ref(path).is_some_and(|r| r.asset.is_some()) { + // TODO: this error should no longer be possible? + return Err(crate::CustomError::new(format!( + "'{path}' refers to an existing asset in another file. This can be \n\ + caused by special cased path simplification for the first object in a \n\ + file.", + ))); + } + + self.root_node + .build_asset(path, kind) + // TODO: this error should no longer be possible? + .map_err(|()| { + crate::CustomError::new(format!( + "'{path}' refers to an existing asset in another file. This can be \n\ + caused by special cased path simplification for the first object in a \n\ + file.", + )) + }) + } + + /// Takes a path in bauble, and if the path is valid, return meta information about the + /// bauble item(s) at that path. + /// + /// Looks up from both pending items and items already registered in [`BaubleContext`]. + pub fn get_ref(&self, path: TypePath<&str>) -> Option { + let a = self.ctx.get_ref(path).map(CombinedPathReference::from); + let b = self.root_node.node_at(path).map(|node| node.reference()); + + if let Some(mut a) = a { + if let Some(b) = b { + a.combine_early(b).expect("Unexpected collision"); + } + + Some(a) + } else { + b.map(CombinedPathReference::from) + } + } + + /// Recursively searches all children of the node at `path` for node with path ending in + /// `ident`. + /// + /// Returns the [`PathReference`] from [`CtxNode::reference`]. If multiple nodes are found with + /// `ident`, the refs from these will be combined (potentially overriding each other). + /// + /// Looks up from both pending items and items already registered in [`BaubleContext`]. + pub fn ref_with_ident( + &self, + path: TypePath<&str>, + ident: TypePathElem<&str>, + ) -> Option { + let a = self + .ctx + .ref_with_ident(path, ident) + .map(CombinedPathReference::from); + let b = self.root_node.node_at(path).and_then(|node| { + node.iter_all_children(None) + .filter(|node| node.path.ends_with(*ident.borrow())) + .map(|node| node.reference()) + .reduce(|a, mut b| { + // TODO: produce error when multiple results collide. + b.combine_override(a); + b + }) + }); + + // TODO: produce error on collision instead of panicking + if let Some(mut a) = a { + if let Some(b) = b { + a.combine_early(b).unwrap(); + } + + Some(a) + } else { + b.map(CombinedPathReference::from) + } + } + + /// Takes a path to a module in bauble, and if the path is valid, return the meta information + /// of all items inside of that module (not recursive). + /// + /// Looks up from both pending items and items already registered in [`BaubleContext`]. + /// + /// Note, the order of returned items won't match after these pending items are registered into + /// [`BaubleContext`]. + pub fn all_in( + &self, + path: TypePath<&str>, + ) -> Option> { + let maybe_items = self.ctx.all_in(path).map(|items| { + items + .into_iter() + .map(|(ident, reference)| (ident, CombinedPathReference::from(reference))) + }); + if let Some(node) = self.root_node.node_at(path) { + let mut items = maybe_items + .map(|items| items.collect::>()) + .unwrap_or_default(); + node.children.iter().for_each(|(key, child_node)| { + let key = key.to_owned(); + let r = child_node.reference(); + use indexmap::map::Entry; + match items.entry(key) { + Entry::Occupied(mut e) => { + e.get_mut().combine_early(r).expect("Unexpected collision") + } + Entry::Vacant(e) => { + e.insert(CombinedPathReference::from(r)); + } + } + }); + Some(items.into_iter().collect()) + } else { + maybe_items.map(|items| items.collect()) + } + } + + pub fn type_registry(&self) -> &crate::types::TypeRegistry { + self.ctx.type_registry() + } +} diff --git a/bauble/src/value/error.rs b/bauble/src/value/error.rs index da007bd..8568616 100644 --- a/bauble/src/value/error.rs +++ b/bauble/src/value/error.rs @@ -1,21 +1,21 @@ -use std::{borrow::Cow, collections::HashMap}; +use std::{borrow::Cow, collections::HashSet}; use crate::{ BaubleContext, BaubleError, CustomError, - context::PathReference, error::Level, path::{TypePath, TypePathElem}, spanned::{SpanExt, Spanned}, types::{self, TypeId}, + value::early_context::CombinedPathReference, }; use super::{Ident, PathKind}; #[derive(Clone, Debug)] pub struct RefError { - pub(super) uses: Option>, + pub(super) uses: Option>, pub(super) path: PathKind, - pub(super) path_ref: PathReference, + pub(super) path_ref: CombinedPathReference, pub(super) kind: RefKind, } @@ -573,7 +573,7 @@ impl BaubleError for Spanned { && let Some(uses) = &ref_err.uses { if let Some(suggestions) = get_suggestions( - uses.keys().map(|ident| ident.as_str()).chain(options), + uses.iter().map(|ident| ident.as_str()).chain(options), path.as_str(), ) { errs.push(( diff --git a/bauble/src/value/mod.rs b/bauble/src/value/mod.rs index 536d651..5cc8f76 100644 --- a/bauble/src/value/mod.rs +++ b/bauble/src/value/mod.rs @@ -1,3 +1,9 @@ +mod convert; +mod display; +mod early_context; +mod error; +mod symbols; + use std::{collections::HashMap, hash::Hash}; use indexmap::IndexMap; @@ -12,17 +18,15 @@ use crate::{ types::{self, TypeId, TypeRegistry}, }; -mod convert; -mod display; -mod error; -mod symbols; - pub use convert::AdditionalUnspannedObjects; pub(crate) use convert::AnyVal; -use convert::{AdditionalObjects, ConvertMeta, ConvertValue, no_attr, value_type}; +use convert::{AdditionalObjects, ConvertMeta, ConvertValue}; pub use display::{DisplayConfig, IndentedDisplay, display_formatted}; +use early_context::CombinedPathReference; +pub(crate) use early_context::EarlyContext; use error::Result; pub use error::{ConversionError, RefError, RefKind}; +use symbols::EarlySymbols; pub(crate) use symbols::Symbols; // TODO(@docs) @@ -698,7 +702,7 @@ pub(crate) fn resolve_delayed( // TODO: Could pass uses here for better suggestions. uses: None, path: map[&scc[0]].value.clone(), - path_ref: PathReference::empty(), + path_ref: PathReference::empty().into(), kind: RefKind::Asset, })) .spanned(map[&scc[0]].span), @@ -746,30 +750,87 @@ fn object_ident_path<'a>( (ident, path) } +/// Registers all new asset paths into [`EarlyContext`] so they will be known for resolving full +/// paths from `use`s in [`register_assets`]. +/// +/// We need to know what items brought into scope with `use` are assets rather than types or +/// modules to properly dertermine the full path (otherwise there could be multiple candidates +/// because items from different namespaces can have the same name). +pub(crate) fn pre_register_assets( + ctx: &mut EarlyContext<'_>, + file_path: TypePath<&str>, + values: &ParseValues, +) -> std::result::Result<(), Vec>> { + let mut errors = Vec::new(); + + for ident in values.values.keys() { + let span = ident.span(); + let kind = if ident.is_top_level() { + crate::AssetKind::TopLevel + } else { + crate::AssetKind::Local + }; + let (_ident, path) = object_ident_path(file_path, ident); + + match ctx.register_asset(path.borrow(), kind) { + Ok(()) => {} + Err(e) => errors.push(ConversionError::Custom(e).spanned(span)), + } + } + + if errors.is_empty() { + Ok(()) + } else { + Err(errors) + } +} + pub(crate) fn register_assets( + ctx: &mut EarlyContext<'_>, file_path: TypePath<&str>, - ctx: &mut crate::context::BaubleContext, values: &ParseValues, ) -> std::result::Result, Vec>> { let mut errors = Vec::new(); let mut delayed = Vec::new(); - let mut symbols = Symbols::new(ctx); - // Add `uses` to local `Symbols` instance + let mut symbols = EarlySymbols::new(ctx); + // Add `uses` to local symbols instance. for use_path in &values.uses { - // TODO: This will error if the referenced file is yet to be passed register_assets if let Err(e) = symbols.add_use(use_path) { errors.push(e); } } - // Move `uses` in/out of `Symbols` every loop so we have mutable access to `ctx` at certain - // points. - let Symbols { mut uses, .. } = symbols; + // Add assets from this file to Symbols::use. + for ident in values.values.keys() { + let span = ident.span(); + let (ident, path) = object_ident_path(file_path, ident); - // TODO: Register these in a correct order to allow for assets referencing assets. (NOTE: This - // is impossible to do in all cases since some of those assets may need to be delayed anyway - // due to referencing assets external to this file that are not registered yet). + if let Some(CombinedPathReference { + asset: Some(asset), .. + }) = symbols.ctx.get_ref(path.borrow()) + { + if let Err(e) = symbols.add_ref( + ident.to_owned(), + CombinedPathReference { + ty: None, + asset: Some(asset), + module: None, + }, + ) { + errors.push(e.spanned(span)); + } + } else { + // Didn't pre-register assets. + errors.push(ConversionError::UnregisteredAsset.spanned(span)); + } + } + + // Resolve asset types and register the assets into `BaubleContext`. + // + // If the asset is a reference to another asset whose type is yet to be resolved, type + // resolution will be delayed by pushing an entry to `delayed`. These are then handled by + // `resolve_delayed`. for (ident, binding) in &values.values { let span = ident.span(); let kind = if ident.is_top_level() { @@ -777,8 +838,7 @@ pub(crate) fn register_assets( } else { crate::AssetKind::Local }; - let (ident, path) = object_ident_path(file_path, ident); - let symbols = Symbols { ctx: &*ctx, uses }; + let (_ident, path) = object_ident_path(file_path, ident); // To register an asset we need to determine its type. let ty = if let Some(ty) = &binding.type_path @@ -792,10 +852,10 @@ pub(crate) fn register_assets( { symbols.resolve_type(ty) } else { - let res = value_type(&binding.value, &symbols) + let res = convert::value_type(&binding.value, &symbols) .map(|v| { convert::default_value_type( - ctx.type_registry(), + symbols.ctx.type_registry(), binding.value.value.value.primitive_type(), v, ) @@ -805,7 +865,7 @@ pub(crate) fn register_assets( ConversionError::UnresolvedType.spanned(binding.value.value.span) )) .and_then(|v| { - if ctx.type_registry().key_type(v).kind.instanciable() { + if symbols.ctx.type_registry().key_type(v).kind.instanciable() { Ok(v) } else { Err(ConversionError::UnresolvedType.spanned(binding.value.value.span)) @@ -816,11 +876,11 @@ pub(crate) fn register_assets( && let Value::Ref(reference) = &*binding.value.value // TODO: Will the error be helpful when this part fails, since // we just pass on the error from an earlier step? - && let Ok(reference) = symbols.resolve_path(reference, false) + && let Ok(reference) = symbols.resolve_path(reference, symbols::ResolveKind::Asset) { let expected_ty_path = if let Some(expected_ty_path) = &binding.type_path { // Resolving to a full path won't fail even if the reference type is not yet registered. - match symbols.resolve_path(expected_ty_path, true) { + match symbols.resolve_path(expected_ty_path, symbols::ResolveKind::Type) { Ok(s) => Some(s), Err(e) => { errors.push(e); @@ -837,7 +897,6 @@ pub(crate) fn register_assets( asset: path.spanned(span), reference, }); - Symbols { uses, .. } = symbols; continue; } @@ -858,30 +917,14 @@ pub(crate) fn register_assets( } }; - Symbols { uses, .. } = symbols; - let ref_ty = ty.and_then(|ty| { - ctx.register_asset(path.borrow(), ty, kind) + if let Err(e) = ty.and_then(|ty| { + symbols + .bauble_ctx() + .register_asset(path.borrow(), ty, kind) .map_err(|e| ConversionError::Custom(e).spanned(span)) - }); - match ref_ty { - Ok(ref_ty) => { - let mut symbols = Symbols { ctx: &*ctx, uses }; - // Add to Symbols::uses so other items in the same file can directly reference this - // without full path. - if let Err(e) = symbols.add_ref( - ident.to_owned(), - PathReference { - ty: None, - asset: Some((ref_ty, path.clone(), kind)), - module: None, - }, - ) { - errors.push(e.spanned(binding.value.value.span)); - } - Symbols { uses, .. } = symbols; - } - Err(err) => errors.push(err), - }; + }) { + errors.push(e); + } } if errors.is_empty() { @@ -910,7 +953,7 @@ pub(crate) fn convert_values( // Add assets from this file to Symbols::use. for ident in values.values.keys() { let span = ident.span(); - let (ident, path) = object_ident_path(file_path, &ident); + let (ident, path) = object_ident_path(file_path, ident); if let Some(PathReference { asset: Some(asset), .. @@ -959,7 +1002,7 @@ pub(crate) fn convert_values( }; let top_level = ident.is_top_level(); - let (ident, path) = object_ident_path(file_path, &ident); + let (ident, path) = object_ident_path(file_path, ident); let convert_meta = ConvertMeta { symbols: &symbols, @@ -992,7 +1035,7 @@ fn convert_object( expected_type: TypeId, mut meta: ConvertMeta, ) -> Result { - let value = value.convert(meta.reborrow(), expected_type, no_attr())?; + let value = value.convert(meta.reborrow(), expected_type, convert::no_attr())?; let types = meta.symbols.ctx.type_registry(); create_object(object_path, top_level, value, types) } diff --git a/bauble/src/value/symbols.rs b/bauble/src/value/symbols.rs index 9c1b5df..84a56fa 100644 --- a/bauble/src/value/symbols.rs +++ b/bauble/src/value/symbols.rs @@ -6,10 +6,28 @@ use crate::{ parse::{Path, PathEnd, PathTreeEnd, PathTreeNode}, path::{TypePath, TypePathElem}, spanned::{SpanExt, Spanned}, - types::{self, TypeId}, + types::{self, TypeId, TypeRegistry}, }; -use super::{ConversionError, PathKind, RefError, RefKind, Result}; +use super::early_context::CombinedPathReference; +use super::{ConversionError, EarlyContext, PathKind, RefError, RefKind, Result}; + +/// Indicates kind of item being resolved by [`Symbols::resolve_path`] or +/// [`EarlySymbols::resolve_path`]. +#[derive(Clone, Copy)] +pub(crate) enum ResolveKind { + Type, + Asset, +} + +impl From for RefKind { + fn from(kind: ResolveKind) -> Self { + match kind { + ResolveKind::Type => RefKind::Type, + ResolveKind::Asset => RefKind::Asset, + } + } +} /// Representation of item names available in the current module. /// @@ -61,7 +79,7 @@ impl<'a> Symbols<'a> { return Err(ConversionError::RefError(Box::new(RefError { uses: None, path: PathKind::Direct(leading), - path_ref: PathReference::empty(), + path_ref: PathReference::empty().into(), kind: RefKind::Module, })) .spanned(s.span)); @@ -80,7 +98,7 @@ impl<'a> Symbols<'a> { return Err(ConversionError::RefError(Box::new(RefError { uses: None, path: PathKind::Direct(leading), - path_ref: PathReference::empty(), + path_ref: PathReference::empty().into(), kind: RefKind::Module, })) .spanned(end.span)); @@ -97,7 +115,7 @@ impl<'a> Symbols<'a> { return Err(ConversionError::RefError(Box::new(RefError { uses: None, path: PathKind::Direct(path), - path_ref: PathReference::empty(), + path_ref: PathReference::empty().into(), kind: RefKind::Any, })) .spanned(end.span)); @@ -113,7 +131,7 @@ impl<'a> Symbols<'a> { return Err(ConversionError::RefError(Box::new(RefError { uses: None, path: PathKind::Indirect(leading, path_end.to_owned()), - path_ref: PathReference::empty(), + path_ref: PathReference::empty().into(), kind: RefKind::Any, })) .spanned(end.span)); @@ -137,7 +155,7 @@ impl<'a> Symbols<'a> { return Err(ConversionError::RefError(Box::new(RefError { uses: None, path: PathKind::Direct(leading), - path_ref: PathReference::empty(), + path_ref: PathReference::empty().into(), kind: RefKind::Module, })) .spanned(l.span)); @@ -152,7 +170,7 @@ impl<'a> Symbols<'a> { .and_then(|reference| reference.module.clone()) } - pub fn resolve_path(&self, raw_path: &Path, for_type: bool) -> Result> { + fn resolve_path(&self, raw_path: &Path, kind: ResolveKind) -> Result> { let mut leading = TypePath::empty(); let mut path_iter = raw_path.leading.iter(); @@ -167,7 +185,7 @@ impl<'a> Symbols<'a> { return Err(ConversionError::RefError(Box::new(RefError { uses: None, path: PathKind::Direct(leading), - path_ref: PathReference::empty(), + path_ref: PathReference::empty().into(), kind: RefKind::Module, })) .spanned(first.span)); @@ -182,7 +200,7 @@ impl<'a> Symbols<'a> { return Err(ConversionError::RefError(Box::new(RefError { uses: None, path: PathKind::Direct(leading), - path_ref: PathReference::empty(), + path_ref: PathReference::empty().into(), kind: RefKind::Module, })) .spanned(ident.span)); @@ -200,12 +218,21 @@ impl<'a> Symbols<'a> { TypePathElem::new(ident.to_string()).map_err(|p| p.spanned(raw_path.span()))?, ), PathEnd::Ident(ident) => { - let path = if leading.is_empty() - && for_type - && let Some(r) = self.uses.get(ident.as_str()) + let used = if leading.is_empty() { + self.uses.get(ident.as_str()) + } else { + None + }; + let path = if let Some(r) = used + && let ResolveKind::Type = kind && let Some(ty) = r.ty { self.ctx.type_registry().key_type(ty).meta.path.clone() + } else if let Some(r) = used + && let ResolveKind::Asset = kind + && let Some((_ty, path, _kind)) = &r.asset + { + path.clone() } else { leading .push_str(ident.as_str()) @@ -215,10 +242,10 @@ impl<'a> Symbols<'a> { PathKind::Direct(path) } PathEnd::WithIdentGeneric(ident, generic) => { - if !for_type { + if !matches!(kind, ResolveKind::Type) { return Err(generic_with_non_type_error()); } - let generic = self.resolve_path(&generic.value, true)?; + let generic = self.resolve_path(&generic.value, ResolveKind::Type)?; PathKind::Indirect( leading, TypePathElem::new(format!("{ident}<{generic}>")) @@ -226,10 +253,10 @@ impl<'a> Symbols<'a> { ) } PathEnd::IdentGeneric(ident, generic) => { - if !for_type { + if !matches!(kind, ResolveKind::Type) { return Err(generic_with_non_type_error()); } - let generic = self.resolve_path(&generic.value, true)?; + let generic = self.resolve_path(&generic.value, ResolveKind::Type)?; let path = if leading.is_empty() && let Some(r) = self.uses.get(ident.as_str()) && let Some(ty) = r.ty @@ -249,22 +276,23 @@ impl<'a> Symbols<'a> { Ok(path.spanned(raw_path.span())) } - fn resolve_item(&self, raw_path: &Path, ref_kind: RefKind) -> Result> { - // NOTE: Resolve path resolves the full path for types referred to via `uses`. This is - // unnecessary for the usage here (outside of types with generic parameters) because the - // type info is available directly in `self.uses`. However, this is neccessary for other - // uses of `resolve_path` where the type needs to be looked up later when `Symbols` is not - // available. - let path = self.resolve_path(raw_path, matches!(ref_kind, RefKind::Type))?; + /// Note, the returned `PathReference` only makes sense to use for the `kind` passed here. The + /// same path for a different `kind` may return a different `PathReference`. + fn resolve_item(&self, raw_path: &Path, kind: ResolveKind) -> Result> { + // NOTE: This resolves the full path referred to via `uses`. This is unnecessary in many + // cases because there is a `PathReference` is available in `self.uses`. + // + // However, there would be a few complications: + // 1. Paths for generic types always need to be fully resolved. + // 2. If a particular identifier is available at the root of the `BaubleContext` in one + // namespace (e.g. type namespace) and in `self.uses` in another namespace (e.g. asset + // namespace) then we could miss something if we naively return the `PathReference` from + // `self.uses` (we would need to at least check `BaubleContext` if the `kind` of item + // was `None` in `self.uses`). + let path = self.resolve_path(raw_path, kind)?; let reference = match &path.value { - PathKind::Direct(path) => { - if let Some(r) = self.uses.get(path.as_str()) { - Some(Cow::Borrowed(r)) - } else { - self.ctx.get_ref(path.borrow()).map(Cow::Owned) - } - } + PathKind::Direct(path) => self.ctx.get_ref(path.borrow()).map(Cow::Owned), PathKind::Indirect(path, ident) => self .ctx .ref_with_ident(path.borrow(), ident.borrow()) @@ -287,10 +315,10 @@ impl<'a> Symbols<'a> { } } else { ConversionError::RefError(Box::new(RefError { - uses: Some(self.uses.clone()), + uses: Some(self.uses.keys().cloned().collect()), path: path.value.clone(), - path_ref: PathReference::empty(), - kind: ref_kind, + path_ref: PathReference::empty().into(), + kind: kind.into(), })) } .spanned(raw_path.span()) @@ -298,15 +326,15 @@ impl<'a> Symbols<'a> { } pub fn resolve_asset(&self, path: &Path) -> Result<(TypeId, TypePath)> { - let item = self.resolve_item(path, RefKind::Asset)?.into_owned(); + let item = self.resolve_item(path, ResolveKind::Asset)?.into_owned(); if let Some((ty, path, _kind)) = item.asset { Ok((ty, path)) } else { Err(ConversionError::RefError(Box::new(RefError { - uses: Some(self.uses.clone()), - path: self.resolve_path(path, false)?.value, - path_ref: item, + uses: Some(self.uses.keys().cloned().collect()), + path: self.resolve_path(path, ResolveKind::Asset)?.value, + path_ref: item.into(), kind: RefKind::Asset, })) .spanned(path.span())) @@ -314,14 +342,338 @@ impl<'a> Symbols<'a> { } pub fn resolve_type(&self, path: &Path) -> Result { - let item = self.resolve_item(path, RefKind::Type)?; + let item = self.resolve_item(path, ResolveKind::Type)?; if let Some(ty) = item.ty { Ok(ty) } else { Err(ConversionError::RefError(Box::new(RefError { - uses: Some(self.uses.clone()), - path: self.resolve_path(path, true)?.value, + uses: Some(self.uses.keys().cloned().collect()), + path: self.resolve_path(path, ResolveKind::Type)?.value, + path_ref: item.into_owned().into(), + kind: RefKind::Type, + })) + .spanned(path.span())) + } + } +} + +/// Representation of item names available in the current module. +/// +/// This is used before types of assets are fully resolved. Afterwards, [`Symbols`] can be used. +/// +/// There are multiple namespaces: types, assets (i.e. values defined in bauble), and modules. +pub(crate) struct EarlySymbols<'a, 'b> { + /// Context for looking up things referenced by full path. + /// + /// This does not need mutable access but the user of `EarlySymbols` needs to mutate + /// `BaubleContext`, so it is convenient to hold a mutable reference here. + pub(super) ctx: &'a mut EarlyContext<'b>, + /// Map of identifiers to path references. + pub(super) uses: HashMap, +} + +impl<'a, 'b> EarlySymbols<'a, 'b> { + pub fn new(ctx: &'a mut EarlyContext<'b>) -> Self { + Self { + ctx, + uses: HashMap::default(), + } + } + + pub fn bauble_ctx(&mut self) -> &mut BaubleContext { + self.ctx.ctx + } + + pub fn add_ref( + &mut self, + ident: TypePathElem, + reference: CombinedPathReference, + ) -> std::result::Result<(), ConversionError> { + let r = self.uses.entry(ident.clone()).or_default(); + + *r = r + .clone() + .combined(reference) + .ok_or(ConversionError::AmbiguousUse { ident })?; + + Ok(()) + } + + pub fn add_use(&mut self, use_path: &Spanned) -> Result<()> { + fn add_use_inner( + this: &mut EarlySymbols, + leading: TypePath, + end: &Spanned, + ) -> Result<()> { + match &end.value { + PathTreeEnd::Group(g) => { + for node in g { + let mut leading = leading.clone(); + for s in &node.leading.value { + leading.push_str(&s.value).map_err(|e| e.spanned(s.span))?; + if this.ctx.get_ref(leading.borrow()).is_none() { + return Err(ConversionError::RefError(Box::new(RefError { + uses: None, + path: PathKind::Direct(leading), + path_ref: PathReference::empty().into(), + kind: RefKind::Module, + })) + .spanned(s.span)); + } + } + add_use_inner(this, leading, &node.end)?; + } + } + PathTreeEnd::Everything => { + if let Some(uses) = this.ctx.all_in(leading.borrow()) { + for (ident, reference) in uses { + this.add_ref(ident, reference) + .map_err(|e| e.spanned(end.span))?; + } + } else { + return Err(ConversionError::RefError(Box::new(RefError { + uses: None, + path: PathKind::Direct(leading), + path_ref: PathReference::empty().into(), + kind: RefKind::Module, + })) + .spanned(end.span)); + } + } + PathTreeEnd::PathEnd(PathEnd::Ident(ident)) => { + let path_end = + TypePathElem::new(ident.as_str()).map_err(|e| e.spanned(ident.span))?; + let path = leading.join(&path_end); + if let Some(reference) = this.ctx.get_ref(path.borrow()) { + this.add_ref(path_end.to_owned(), reference) + .map_err(|e| e.spanned(ident.span))?; + } else { + return Err(ConversionError::RefError(Box::new(RefError { + uses: None, + path: PathKind::Direct(path), + path_ref: PathReference::empty().into(), + kind: RefKind::Any, + })) + .spanned(end.span)); + } + } + PathTreeEnd::PathEnd(PathEnd::WithIdent(ident)) => { + let path_end = + TypePathElem::new(ident.as_str()).map_err(|e| e.spanned(ident.span))?; + if let Some(reference) = this.ctx.ref_with_ident(leading.borrow(), path_end) { + this.add_ref(path_end.to_owned(), reference) + .map_err(|e| e.spanned(ident.span))?; + } else { + return Err(ConversionError::RefError(Box::new(RefError { + uses: None, + path: PathKind::Indirect(leading, path_end.to_owned()), + path_ref: PathReference::empty().into(), + kind: RefKind::Any, + })) + .spanned(end.span)); + } + } + PathTreeEnd::PathEnd(PathEnd::IdentGeneric(ident, ..)) + | PathTreeEnd::PathEnd(PathEnd::WithIdentGeneric(ident, ..)) => { + return Err(ConversionError::Custom(CustomError::new( + "Use cannot use generics", + )) + .spanned(ident.span)); + } + } + Ok(()) + } + + let mut leading = TypePath::empty(); + for l in use_path.leading.iter() { + leading.push_str(l).map_err(|e| e.spanned(l.span))?; + if self.ctx.get_ref(leading.borrow()).is_none() { + return Err(ConversionError::RefError(Box::new(RefError { + uses: None, + path: PathKind::Direct(leading), + path_ref: PathReference::empty().into(), + kind: RefKind::Module, + })) + .spanned(l.span)); + } + } + add_use_inner(self, leading, &use_path.end) + } + + fn get_module(&self, ident: &str) -> Option { + self.uses + .get(ident) + .and_then(|reference| reference.module.clone()) + } + + pub fn resolve_path(&self, raw_path: &Path, kind: ResolveKind) -> Result> { + let mut leading = TypePath::empty(); + + let mut path_iter = raw_path.leading.iter(); + if let Some(first) = path_iter.next() { + leading = self.get_module(first.as_str()).unwrap_or( + TypePath::new(first.as_str()) + .map_err(|e| e.spanned(first.span))? + .to_owned(), + ); + + if self.ctx.get_ref(leading.borrow()).is_none() { + return Err(ConversionError::RefError(Box::new(RefError { + uses: None, + path: PathKind::Direct(leading), + path_ref: PathReference::empty().into(), + kind: RefKind::Module, + })) + .spanned(first.span)); + } + + for ident in path_iter { + leading + .push_str(ident.as_str()) + .map_err(|e| e.spanned(ident.span))?; + + if self.ctx.get_ref(leading.borrow()).is_none() { + return Err(ConversionError::RefError(Box::new(RefError { + uses: None, + path: PathKind::Direct(leading), + path_ref: PathReference::empty().into(), + kind: RefKind::Module, + })) + .spanned(ident.span)); + } + } + } + + let generic_with_non_type_error = || { + ConversionError::Custom(CustomError::new("Cannot use generics for non-type paths")) + .spanned(raw_path.span()) + }; + let path = match &raw_path.last.value { + PathEnd::WithIdent(ident) => PathKind::Indirect( + leading, + TypePathElem::new(ident.to_string()).map_err(|p| p.spanned(raw_path.span()))?, + ), + PathEnd::Ident(ident) => { + let used = if leading.is_empty() { + self.uses.get(ident.as_str()) + } else { + None + }; + let path = if let Some(r) = used + && let ResolveKind::Type = kind + && let Some(ty) = r.ty + { + self.ctx.type_registry().key_type(ty).meta.path.clone() + } else if let Some(r) = used + && let ResolveKind::Asset = kind + && let Some((_ty, path, _kind)) = &r.asset + { + path.clone() + } else { + leading + .push_str(ident.as_str()) + .map_err(|p| p.spanned(raw_path.span()))?; + leading + }; + PathKind::Direct(path) + } + PathEnd::WithIdentGeneric(ident, generic) => { + if !matches!(kind, ResolveKind::Type) { + return Err(generic_with_non_type_error()); + } + let generic = self.resolve_path(&generic.value, ResolveKind::Type)?; + PathKind::Indirect( + leading, + TypePathElem::new(format!("{ident}<{generic}>")) + .map_err(|p| p.spanned(raw_path.span()))?, + ) + } + PathEnd::IdentGeneric(ident, generic) => { + if !matches!(kind, ResolveKind::Type) { + return Err(generic_with_non_type_error()); + } + let generic = self.resolve_path(&generic.value, ResolveKind::Type)?; + let path = if leading.is_empty() + && let Some(r) = self.uses.get(ident.as_str()) + && let Some(ty) = r.ty + { + let outer_path = &self.ctx.type_registry().key_type(ty).meta.path; + TypePath::new(format!("{outer_path}<{generic}>")) + .map_err(|p| p.spanned(raw_path.span()))? + } else { + leading + .push_str(&format!("{ident}<{generic}>")) + .map_err(|p| p.spanned(raw_path.span()))?; + leading + }; + PathKind::Direct(path) + } + }; + Ok(path.spanned(raw_path.span())) + } + + /// Note, the returned `PathReference` only makes sense to use for the `kind` passed here. The + /// same path for a different `kind` may return a different `PathReference`. + fn resolve_item( + &self, + raw_path: &Path, + kind: ResolveKind, + ) -> Result> { + // NOTE: Similar to `Symbols::resolve_item`, this resolves the full path referred which is + // often unneccessary. + // + // But avoiding this has the same complications along with an additional ones: + // 3. There are other uses of `resolve_path` that need a full path because the resolved + // path needs to be looked up later when `EarlySymbols` is no longer available. + // 4. Once an asset is registered with its type, the path reference stored in `uses` will + // be outdated since it won't include the type (an up-to-date value isn't essential but + // would lead to fewer assets to process in resolve_delayed). + let path = self.resolve_path(raw_path, kind)?; + + let reference = match &path.value { + PathKind::Direct(path) => self.ctx.get_ref(path.borrow()).map(Cow::Owned), + PathKind::Indirect(path, ident) => self + .ctx + .ref_with_ident(path.borrow(), ident.borrow()) + .map(Cow::Owned), + }; + + reference.ok_or_else(|| { + if let PathKind::Direct(path) = &*path + && let Some((leading, ident)) = path.get_end() + && let Some(r) = self.ctx.get_ref(leading) + && let Some(ty) = r.ty + && matches!( + self.ctx.type_registry().key_type(ty).kind, + types::TypeKind::Enum { .. } | types::TypeKind::Or(_) + ) + { + ConversionError::UnknownVariant { + variant: ident.to_owned().spanned(raw_path.last.span), + ty, + } + } else { + ConversionError::RefError(Box::new(RefError { + uses: Some(self.uses.keys().cloned().collect()), + path: path.value.clone(), + path_ref: PathReference::empty().into(), + kind: kind.into(), + })) + } + .spanned(raw_path.span()) + }) + } + + pub fn resolve_type(&self, path: &Path) -> Result { + let item = self.resolve_item(path, ResolveKind::Type)?; + + if let Some(ty) = item.ty { + Ok(ty) + } else { + Err(ConversionError::RefError(Box::new(RefError { + uses: Some(self.uses.keys().cloned().collect()), + path: self.resolve_path(path, ResolveKind::Type)?.value, path_ref: item.into_owned(), kind: RefKind::Type, })) @@ -329,3 +681,50 @@ impl<'a> Symbols<'a> { } } } + +pub(crate) trait SymbolsCommon { + fn resolve_asset_type(&self, path: &Path) -> Result>; + fn resolve_type(&self, path: &Path) -> Result; + fn type_registry(&self) -> &TypeRegistry; +} + +impl SymbolsCommon for Symbols<'_> { + fn resolve_asset_type(&self, path: &Path) -> Result> { + self.resolve_asset(path).map(|(ty, _)| Some(ty)) + } + + fn resolve_type(&self, path: &Path) -> Result { + self.resolve_type(path) + } + + fn type_registry(&self) -> &TypeRegistry { + self.ctx.type_registry() + } +} + +impl SymbolsCommon for EarlySymbols<'_, '_> { + /// Note, if an asset isn't registered in `BaubleContext` yet, its type will be unknown. + fn resolve_asset_type(&self, path: &Path) -> Result> { + let item = self.resolve_item(path, ResolveKind::Asset)?.into_owned(); + + if let Some((ty, _path, _kind)) = item.asset { + Ok(ty) + } else { + Err(ConversionError::RefError(Box::new(RefError { + uses: Some(self.uses.keys().cloned().collect()), + path: self.resolve_path(path, ResolveKind::Asset)?.value, + path_ref: item, + kind: RefKind::Asset, + })) + .spanned(path.span())) + } + } + + fn resolve_type(&self, path: &Path) -> Result { + self.resolve_type(path) + } + + fn type_registry(&self) -> &TypeRegistry { + self.ctx.type_registry() + } +} diff --git a/bauble/tests/integration.rs b/bauble/tests/integration.rs index 80c78dc..15d6a30 100644 --- a/bauble/tests/integration.rs +++ b/bauble/tests/integration.rs @@ -247,6 +247,12 @@ fn empty_module() { }, &[a, empty_module], ); + test_load( + &|ctx| { + ctx.register_type::(); + }, + &[empty_module, a], + ); } #[test] @@ -334,15 +340,13 @@ fn same_file_references() { test_load( &|ctx| { ctx.register_type::(); - // TODO: TestRef doesn't need to be registered?! + // NOTE: TestRef doesn't need to be registered?! }, &[a], ); } #[test] -// TODO: see todo in `register_assets` about registering assets in the correct order. -#[should_panic = "Expected this path to refer to an asset"] fn same_file_references_reverse() { let a = &test_file!( "a", @@ -355,7 +359,7 @@ fn same_file_references_reverse() { test_load( &|ctx| { ctx.register_type::(); - // TODO: TestRef doesn't need to be registered?! + // NOTE: TestRef doesn't need to be registered?! }, &[a], ); @@ -380,17 +384,15 @@ fn same_file_references_reverse_full() { } #[test] -// TODO: see todo in `register_assets` where `add_use` is called. -#[should_panic = "Expected this path to refer to a valid reference"] fn reference_with_use() { let a = &test_file!( "a", "use b::test;\n\ 0 = $test", - TestRef("b".into()), + TestRef("b::test".into()), ); let b = &test_file!( - "b", + "b::test", "0 = integration::Test { x: -5, y: 5 }", Test { x: -5, y: 5 }, ); @@ -399,6 +401,7 @@ fn reference_with_use() { &|ctx| { ctx.register_type::(); }, + // Test when the referencing file is loaded before the referenced file &[a, b], ); } @@ -547,6 +550,26 @@ pub fn ref_explicit_type_incorrect_multiple_files() { ); } +/// Like above, but with file load order reversed. +#[test] +#[should_panic = "Error converting: \n\u{1b}[31mError:\u{1b}[0m Invalid explicit reference path 'Ref"] +pub fn ref_explicit_type_incorrect_multiple_files_reverse() { + #[derive(Bauble, PartialEq, Eq, Debug)] + struct Incorrect(u32); + + bauble::bauble_test!( + [Test, Incorrect] + [ + "0: Ref = $test1", + "0 = integration::Test{ x: -5, y: 5 }", + ] + [ + Ref::::from_path(TypePath::new_unchecked("test1").to_owned()), + Test { x: -5, y: 5 }, + ] + ); +} + #[test] #[should_panic = "Expected `Ref` which is a reference to `integration::Incorrect`, which is a struct with unnamed fields, but got `Ref` which is a reference to `integration::Test`, which is a struct with named fields"] pub fn ref_explicit_type_incorrect_multiple_files_ref_already_registered() { From 8382f9f966d7a3b165ac72514cd54cd6861b1636 Mon Sep 17 00:00:00 2001 From: Imbris Date: Thu, 26 Mar 2026 16:43:27 -0400 Subject: [PATCH 15/25] Produce an error when the indirect with ident syntax refers to an ambiguous item --- bauble/src/context.rs | 49 +++++++++++-------- bauble/src/value/early_context.rs | 62 +++++++++++++++--------- bauble/src/value/error.rs | 32 +++++++++++-- bauble/src/value/mod.rs | 46 ++++++++++++------ bauble/src/value/symbols.rs | 14 +++++- bauble/tests/integration.rs | 80 +++++++++++++++++++++++++++++++ 6 files changed, 220 insertions(+), 63 deletions(-) diff --git a/bauble/src/context.rs b/bauble/src/context.rs index b2c1de3..45ffef8 100644 --- a/bauble/src/context.rs +++ b/bauble/src/context.rs @@ -2,6 +2,7 @@ use crate::{ Bauble, BaubleAllocator, BaubleErrors, path::{TypePath, TypePathElem}, types::{BaubleTrait, TypeId, TypeRegistry}, + value::AmbiguousWithIdent, }; use indexmap::IndexMap; @@ -73,23 +74,17 @@ impl PathReference { }) } - /// Overrides references of `self` with references of `other`, returns true if - /// anything was overriden. - pub fn combine_override(&mut self, other: Self) -> bool { - let mut o = false; + /// Overrides references of `self` with references of `other`. + pub fn combine_override(&mut self, other: Self) { if other.ty.is_some() { - o = true; self.ty = other.ty; } if other.asset.is_some() { - o = true; self.asset = other.asset; } if other.module.is_some() { - o = true; self.module = other.module; } - o } } @@ -760,24 +755,38 @@ impl BaubleContext { /// `ident`. /// /// Returns the [`PathReference`] from [`CtxNode::reference`]. If multiple nodes are found with - /// `ident`, the refs from these will be combined (potentially overriding each other). - // - // TODO: couldn't this overriding lead to unexpected behavior? Should we return an error when - // there are multiple results in the same namespace? + /// `ident`, they will be combined and this will return an error if they have items in the same + /// namespace. pub fn ref_with_ident( &self, path: TypePath<&str>, ident: TypePathElem<&str>, - ) -> Option { - self.root_node.node_at(path).and_then(|node| { - node.iter_all_children(None) + ) -> Result, AmbiguousWithIdent> { + if let Some(node) = self.root_node.node_at(path) { + let mut combined = None::; + for reference in node + .iter_all_children(None) .filter(|node| node.path.ends_with(*ident.borrow())) .map(|node| node.reference(&self.root_node)) - .reduce(|a, mut b| { - b.combine_override(a); - b - }) - }) + { + let new = if let Some(combined) = combined.take() { + let Some(new) = combined.combined(reference) else { + // collision if `combined()` returns `None` + return Err(AmbiguousWithIdent { + path: path.to_owned(), + ident: ident.to_owned(), + }); + }; + new + } else { + reference + }; + combined = Some(new); + } + Ok(combined) + } else { + Ok(None) + } } /// If there is any associated file for `path`, get the ID of that file. diff --git a/bauble/src/value/early_context.rs b/bauble/src/value/early_context.rs index d795e71..23b7f0e 100644 --- a/bauble/src/value/early_context.rs +++ b/bauble/src/value/early_context.rs @@ -1,6 +1,7 @@ use crate::context::{AssetKind, BaubleContext, PathReference}; use crate::path::{TypePath, TypePathElem}; use crate::types::TypeId; +use crate::value::AmbiguousWithIdent; use indexmap::IndexMap; fn try_reduce_option( @@ -108,19 +109,23 @@ struct EarlyPathReference { } impl EarlyPathReference { - /// Overrides references of `self` with references of `other`, returns true if - /// anything was overriden. - fn combine_override(&mut self, other: Self) -> bool { - let mut o = false; - if other.asset.is_some() { - o = true; - self.asset = other.asset; - } - if other.module.is_some() { - o = true; - self.module = other.module; + /// Combine references of `self` with references of `other`. + /// + /// If there is a collision, `self` will be unchanged and this returns `false`. + fn combine(&mut self, other: Self) -> bool { + if self.asset.is_some() && other.asset.is_some() + || self.module.is_some() && other.module.is_some() + { + false + } else { + if other.asset.is_some() { + self.asset = other.asset; + } + if other.module.is_some() { + self.module = other.module; + } + true } - o } } @@ -328,39 +333,50 @@ impl<'a> EarlyContext<'a> { /// `ident`. /// /// Returns the [`PathReference`] from [`CtxNode::reference`]. If multiple nodes are found with - /// `ident`, the refs from these will be combined (potentially overriding each other). + /// `ident`, they will be combined and this will return an error if they have items in the same + /// namespace. /// /// Looks up from both pending items and items already registered in [`BaubleContext`]. pub fn ref_with_ident( &self, path: TypePath<&str>, ident: TypePathElem<&str>, - ) -> Option { + ) -> Result, AmbiguousWithIdent> { + let mut collided = false; let a = self .ctx - .ref_with_ident(path, ident) + .ref_with_ident(path, ident)? .map(CombinedPathReference::from); let b = self.root_node.node_at(path).and_then(|node| { node.iter_all_children(None) .filter(|node| node.path.ends_with(*ident.borrow())) .map(|node| node.reference()) - .reduce(|a, mut b| { - // TODO: produce error when multiple results collide. - b.combine_override(a); - b + .reduce(|mut a, b| { + if !a.combine(b) { + collided = true; + } + a }) }); + if collided { + return Err(AmbiguousWithIdent { + path: path.to_owned(), + ident: ident.to_owned(), + }); + } - // TODO: produce error on collision instead of panicking - if let Some(mut a) = a { + Ok(if let Some(mut a) = a { if let Some(b) = b { - a.combine_early(b).unwrap(); + a.combine_early(b).map_err(|()| AmbiguousWithIdent { + path: path.to_owned(), + ident: ident.to_owned(), + })?; } Some(a) } else { b.map(CombinedPathReference::from) - } + }) } /// Takes a path to a module in bauble, and if the path is valid, return the meta information diff --git a/bauble/src/value/error.rs b/bauble/src/value/error.rs index 8568616..eef187d 100644 --- a/bauble/src/value/error.rs +++ b/bauble/src/value/error.rs @@ -19,10 +19,11 @@ pub struct RefError { pub(super) kind: RefKind, } -impl From for ConversionError { - fn from(value: crate::path::PathError) -> Self { - Self::PathError(value) - } +/// `path::*::ident` refers to multiple items in the same namespace. +#[derive(Clone, Debug)] +pub struct AmbiguousWithIdent { + pub(crate) path: TypePath, + pub(crate) ident: TypePathElem, } /// An error type for conversions that happen inside of Bauble. @@ -38,6 +39,7 @@ pub enum ConversionError { AmbiguousUse { ident: TypePathElem, }, + AmbiguousWithIdent(AmbiguousWithIdent), ExpectedBitfield { got: TypeId, }, @@ -105,6 +107,7 @@ impl BaubleError for Spanned { ConversionError::Cycle(_) => Cow::Borrowed("A cycle was found"), ConversionError::PathError(_) => Cow::Borrowed("Path error"), ConversionError::AmbiguousUse { .. } => Cow::Borrowed("Ambiguous use"), + ConversionError::AmbiguousWithIdent(_) => Cow::Borrowed("Ambiguous with ident path"), ConversionError::ExpectedBitfield { .. } => Cow::Borrowed("Expected bitfield"), ConversionError::UnresolvedType => Cow::Borrowed("Unresolved type"), ConversionError::WrongLength { ty, .. } => Cow::Owned(format!( @@ -340,6 +343,9 @@ impl BaubleError for Spanned { ConversionError::AmbiguousUse { ident } => Cow::Owned(format!( "The identifier `{ident}` has been imported multiple times" )), + ConversionError::AmbiguousWithIdent(AmbiguousWithIdent { path, ident }) => Cow::Owned( + format!("`{path}::*::{ident}` refers to multiple items in the same namespace"), + ), ConversionError::ExpectedBitfield { got } => Cow::Owned(format!( "But got the type `{}` which is {}", types.key_type(*got).meta.path, @@ -664,6 +670,18 @@ impl BaubleError for Spanned { } } +impl From for ConversionError { + fn from(value: crate::path::PathError) -> Self { + Self::PathError(value) + } +} + +impl From for ConversionError { + fn from(value: AmbiguousWithIdent) -> Self { + Self::AmbiguousWithIdent(value) + } +} + pub(super) type Result = std::result::Result>; impl From> for Spanned { @@ -671,3 +689,9 @@ impl From> for Spanned { value.map(ConversionError::PathError) } } + +impl From> for Spanned { + fn from(value: Spanned) -> Self { + value.map(ConversionError::AmbiguousWithIdent) + } +} diff --git a/bauble/src/value/mod.rs b/bauble/src/value/mod.rs index 5cc8f76..b41b3bf 100644 --- a/bauble/src/value/mod.rs +++ b/bauble/src/value/mod.rs @@ -25,7 +25,7 @@ pub use display::{DisplayConfig, IndentedDisplay, display_formatted}; use early_context::CombinedPathReference; pub(crate) use early_context::EarlyContext; use error::Result; -pub use error::{ConversionError, RefError, RefKind}; +pub use error::{AmbiguousWithIdent, ConversionError, RefError, RefKind}; use symbols::EarlySymbols; pub(crate) use symbols::Symbols; @@ -576,6 +576,7 @@ impl PathKind { /// We can delay registering `Ref` assets if what they're referencing hasn't been loaded yet. /// /// What they are referencing needs to be loaded in order to determine their type. +#[derive(Debug)] pub(crate) struct DelayedRegister { asset: Spanned, asset_kind: crate::AssetKind, @@ -596,10 +597,17 @@ pub(crate) fn resolve_delayed( delayed.retain(|d| { if let Some(r) = match &d.reference.value { PathKind::Direct(path) => ctx.get_ref(path.borrow()), - // TODO: what if indirect path becomes ambiguous due to later registered items that - // were delayed? + // NOTE: If indirect path becomes ambiguous due to later registered items that + // were delayed, then we can miss that here. However, this will be caught later in + // `convert_values` when resolving this to a concrete path via `get_asset`. PathKind::Indirect(path, ident) => { - ctx.ref_with_ident(path.borrow(), ident.borrow()) + match ctx.ref_with_ident(path.borrow(), ident.borrow()) { + Ok(reference) => reference, + Err(e) => { + errors.push(e.spanned(d.reference.span).into()); + return false; + } + } } } && let Some((ty, _, _)) = &r.asset { @@ -619,7 +627,13 @@ pub(crate) fn resolve_delayed( let Some(desired_ty) = (match path { PathKind::Direct(path) => ctx.get_ref(path.borrow()), PathKind::Indirect(path, ident) => { - ctx.ref_with_ident(path.borrow(), ident.borrow()) + match ctx.ref_with_ident(path.borrow(), ident.borrow()) { + Ok(reference) => reference, + Err(e) => { + errors.push(e.spanned(span).into()); + return false; + } + } } }) else { errors.push( @@ -684,28 +698,32 @@ pub(crate) fn resolve_delayed( } for scc in petgraph::algo::tarjan_scc(&graph) { - if scc.len() == 1 { - if map[&scc[0]].could_be(scc[0].borrow()) { + // len == 1 + if let [referer] = scc.as_slice() { + let referenced = map[referer]; + if referenced.could_be(referer.borrow()) { // Ref refers to itself errors.push( ConversionError::Cycle(vec![( - scc[0].to_string().spanned(scc[0].span), - vec![map[&scc[0]].to_string().spanned(map[&scc[0]].span)], + referer.map(|r| r.to_string()), + vec![referenced.map(|r| r.to_string())], )]) - .spanned(scc[0].span), + .spanned(referer.span), ) } else { - // TODO: Is this path possible? Wouldn't Ref have to refer to itself for - // scc.len() == 1? + // In this case, `referer` remained unresolved because the referenced asset + // is not available. This means an error occured when processing the + // referenced asset above, or the referenced asset itself has a cycle error + // or this error. errors.push( ConversionError::RefError(Box::new(RefError { // TODO: Could pass uses here for better suggestions. uses: None, - path: map[&scc[0]].value.clone(), + path: referenced.value.clone(), path_ref: PathReference::empty().into(), kind: RefKind::Asset, })) - .spanned(map[&scc[0]].span), + .spanned(referenced.span), ) } } else { diff --git a/bauble/src/value/symbols.rs b/bauble/src/value/symbols.rs index 84a56fa..54427ea 100644 --- a/bauble/src/value/symbols.rs +++ b/bauble/src/value/symbols.rs @@ -124,7 +124,11 @@ impl<'a> Symbols<'a> { PathTreeEnd::PathEnd(PathEnd::WithIdent(ident)) => { let path_end = TypePathElem::new(ident.as_str()).map_err(|e| e.spanned(ident.span))?; - if let Some(reference) = this.ctx.ref_with_ident(leading.borrow(), path_end) { + if let Some(reference) = this + .ctx + .ref_with_ident(leading.borrow(), path_end) + .map_err(|e| e.spanned(ident.span))? + { this.add_ref(path_end.to_owned(), reference) .map_err(|e| e.spanned(ident.span))?; } else { @@ -296,6 +300,7 @@ impl<'a> Symbols<'a> { PathKind::Indirect(path, ident) => self .ctx .ref_with_ident(path.borrow(), ident.borrow()) + .map_err(|e| e.spanned(raw_path.span()))? .map(Cow::Owned), }; @@ -461,7 +466,11 @@ impl<'a, 'b> EarlySymbols<'a, 'b> { PathTreeEnd::PathEnd(PathEnd::WithIdent(ident)) => { let path_end = TypePathElem::new(ident.as_str()).map_err(|e| e.spanned(ident.span))?; - if let Some(reference) = this.ctx.ref_with_ident(leading.borrow(), path_end) { + if let Some(reference) = this + .ctx + .ref_with_ident(leading.borrow(), path_end) + .map_err(|e| e.spanned(ident.span))? + { this.add_ref(path_end.to_owned(), reference) .map_err(|e| e.spanned(ident.span))?; } else { @@ -636,6 +645,7 @@ impl<'a, 'b> EarlySymbols<'a, 'b> { PathKind::Indirect(path, ident) => self .ctx .ref_with_ident(path.borrow(), ident.borrow()) + .map_err(|e| e.spanned(raw_path.span()))? .map(Cow::Owned), }; diff --git a/bauble/tests/integration.rs b/bauble/tests/integration.rs index 15d6a30..d053f15 100644 --- a/bauble/tests/integration.rs +++ b/bauble/tests/integration.rs @@ -847,3 +847,83 @@ fn special_identifier_required_for_first_object() { &[a], ); } + +#[test] +#[should_panic = "`a::*::test` refers to multiple items in the same namespace"] +fn ambiguous_ref_with_ident_via_reference() { + let a = &test_file!("a", "0 = $a::*::test", TestRef("b::test".into()),); + let a_b_test = &test_file!( + "a::b::test", + "0 = integration::Test { x: -5, y: 5 }", + Test { x: -5, y: 5 }, + ); + let a_c_test = &test_file!( + "a::c::test", + "0 = integration::Test { x: -5, y: 5 }", + Test { x: -5, y: 5 }, + ); + + test_load( + &|ctx| { + ctx.register_type::(); + }, + &[a, a_b_test, a_c_test], + ); +} + +#[test] +#[should_panic = "`a::*::test` refers to multiple items in the same namespace"] +fn ambiguous_ref_with_ident_via_use() { + let a = &test_file!( + "a", + "use a::*::test;\n\ + 0 = integration::Test { x: -5, y: 5 }", + Test { x: -5, y: 5 }, + ); + let a_b_test = &test_file!( + "a::b::test", + "0 = integration::Test { x: -5, y: 5 }", + Test { x: -5, y: 5 }, + ); + let a_c_test = &test_file!( + "a::c::test", + "0 = integration::Test { x: -5, y: 5 }", + Test { x: -5, y: 5 }, + ); + + test_load( + &|ctx| { + ctx.register_type::(); + }, + &[a, a_b_test, a_c_test], + ); +} + +#[test] +#[should_panic = "`a::*::test` refers to multiple items in the same namespace"] +fn amiguous_ref_with_ident_multilayer() { + let a = &test_file!("a", "0 = $a::*::test", TestRef("b::test".into())); + let d = &test_file!( + "d", + "0 = integration::Test { x: -5, y: 5 }", + Test { x: -5, y: 5 }, + ); + let a_b_test = &test_file!( + "a::b::test", + "0 = integration::Test { x: -5, y: 5 }", + Test { x: -5, y: 5 }, + ); + let a_c_test = &test_file!("a::c::test", "0 = $d", TestRef("d".into())); + + test_load( + &|ctx| { + ctx.register_type::(); + }, + // a and a_c_test will be delayed (they reference assets that aren't registered when they + // are processed) + // then a will be processed first (before a_c_test is registered) + // we want to ensure an error is still produced in this case + // right now it seems to be caught later on in the convert_values step + &[a_b_test, a, a_c_test, d], + ); +} From b45d0783982fa778b2ac7165e16a74fe29363788 Mon Sep 17 00:00:00 2001 From: Imbris Date: Tue, 31 Mar 2026 13:35:55 -0400 Subject: [PATCH 16/25] Report error earlier when we know asset will not exist, make error more specific when referenced asset fails to be registerer, and test that local assets will be suggsted in errors --- bauble/src/value/error.rs | 5 ++ bauble/src/value/mod.rs | 103 +++++++++++++++++------------------- bauble/src/value/symbols.rs | 31 ++++++----- bauble/tests/integration.rs | 22 ++++++++ 4 files changed, 94 insertions(+), 67 deletions(-) diff --git a/bauble/src/value/error.rs b/bauble/src/value/error.rs index eef187d..8a25205 100644 --- a/bauble/src/value/error.rs +++ b/bauble/src/value/error.rs @@ -578,6 +578,8 @@ impl BaubleError for Spanned { if path.len() == 1 && let Some(uses) = &ref_err.uses { + // TODO: suggestions from `uses` could be improved by filtering by the + // desired `kind`. if let Some(suggestions) = get_suggestions( uses.iter().map(|ident| ident.as_str()).chain(options), path.as_str(), @@ -645,6 +647,9 @@ impl BaubleError for Spanned { } fn help(&self, ctx: &BaubleContext) -> Option> { + // The case where the user has some use statements (or local assets), but the `ident` they + // are using wasn't available in `uses`. This suggests potential items with a matching + // ident at the end of their path that could be brought into scope with a `use`. if let ConversionError::RefError(ref_err) = &self.value && ref_err.uses.is_some() && let PathKind::Direct(ident) = &ref_err.path diff --git a/bauble/src/value/mod.rs b/bauble/src/value/mod.rs index b41b3bf..8fb8c66 100644 --- a/bauble/src/value/mod.rs +++ b/bauble/src/value/mod.rs @@ -580,7 +580,10 @@ impl PathKind { pub(crate) struct DelayedRegister { asset: Spanned, asset_kind: crate::AssetKind, - reference: Spanned, + /// Full path to the referenced asset. + reference: TypePath, + /// Unresolved path to the referenced asset. + reference_original: Spanned, /// The type we want a potential reference to resolve into. expected_ty_path: Option>, } @@ -595,21 +598,8 @@ pub(crate) fn resolve_delayed( // Try to register delayed registers, and remove them as they succeed. delayed.retain(|d| { - if let Some(r) = match &d.reference.value { - PathKind::Direct(path) => ctx.get_ref(path.borrow()), - // NOTE: If indirect path becomes ambiguous due to later registered items that - // were delayed, then we can miss that here. However, this will be caught later in - // `convert_values` when resolving this to a concrete path via `get_asset`. - PathKind::Indirect(path, ident) => { - match ctx.ref_with_ident(path.borrow(), ident.borrow()) { - Ok(reference) => reference, - Err(e) => { - errors.push(e.spanned(d.reference.span).into()); - return false; - } - } - } - } && let Some((ty, _, _)) = &r.asset + if let Some(r) = ctx.get_ref(d.reference.borrow()) + && let Some((ty, _, _)) = &r.asset { // TODO: for now, it is assumed all references which explicitly // specify their inner type should have that inner type resolved @@ -688,10 +678,10 @@ pub(crate) fn resolve_delayed( let mut map = HashMap::new(); for a in delayed.iter() { let node_a = graph.add_node(a.asset.as_ref().map(|p| p.borrow())); - map.insert(node_a, a.reference.as_ref()); + map.insert(node_a, (&a.reference, &a.reference_original)); for b in delayed.iter() { - if a.reference.could_be(b.asset.borrow()) { + if a.reference == *b.asset { graph.add_edge(node_a, b.asset.as_ref().map(|p| p.borrow()), ()); } } @@ -700,46 +690,39 @@ pub(crate) fn resolve_delayed( for scc in petgraph::algo::tarjan_scc(&graph) { // len == 1 if let [referer] = scc.as_slice() { - let referenced = map[referer]; - if referenced.could_be(referer.borrow()) { + let (referenced, referenced_original) = map[referer]; + if referenced.borrow() == **referer { // Ref refers to itself errors.push( ConversionError::Cycle(vec![( referer.map(|r| r.to_string()), - vec![referenced.map(|r| r.to_string())], + vec![referenced_original.as_ref().map(|r| r.to_string())], )]) .spanned(referer.span), ) } else { - // In this case, `referer` remained unresolved because the referenced asset - // is not available. This means an error occured when processing the - // referenced asset above, or the referenced asset itself has a cycle error - // or this error. + // We make sure that the referenced asset exists in the pre-registered + // assets before producing `DelayedRegister`. So this error will only occur + // if the referenced asset's type failed to resolve (such that it wasn't + // registered in resolve_delayed). errors.push( - ConversionError::RefError(Box::new(RefError { - // TODO: Could pass uses here for better suggestions. - uses: None, - path: referenced.value.clone(), - path_ref: PathReference::empty().into(), - kind: RefKind::Asset, - })) - .spanned(referenced.span), - ) + ConversionError::Custom(crate::CustomError::new(format!( + "{referenced_original} refers to an asset that failed to register", + ))) + .spanned(referenced_original.span), + ); } } else { - errors.push( - ConversionError::Cycle( - scc.iter() - .map(|s| { - ( - s.to_string().spanned(s.span), - vec![map[s].to_string().spanned(map[s].span)], - ) - }) - .collect(), - ) - .spanned(scc[0].span), - ); + let cycle = scc + .iter() + .map(|s| { + ( + s.to_string().spanned(s.span), + vec![map[s].1.as_ref().map(|r| r.to_string())], + ) + }) + .collect(); + errors.push(ConversionError::Cycle(cycle).spanned(scc[0].span)); } } @@ -891,13 +874,24 @@ pub(crate) fn register_assets( }); if res.is_err() - && let Value::Ref(reference) = &*binding.value.value - // TODO: Will the error be helpful when this part fails, since - // we just pass on the error from an earlier step? - && let Ok(reference) = symbols.resolve_path(reference, symbols::ResolveKind::Asset) + && let Value::Ref(ref_path) = &*binding.value.value + // Note: If this fails, then `value_type()` would have produced the same error + // already in `res` from calling `symbols.resolve_asset_type()` which uses + // `resolve_path` internally. So there is no extra information from this error. + // + // We use `resolve_asset` instead of just `resolve_path` because the asset should + // exist due to `pre_register_assets` or we will invevitably produce an error + // anyway, and the errors produced in register_assets are better than + // `resolve_delayed` because `symbols.uses` is available. + && let Ok((maybe_ty, reference)) = symbols.resolve_asset(ref_path) { + debug_assert!( + maybe_ty.is_none(), + "value_type should not produce an error if the referenced type is known" + ); let expected_ty_path = if let Some(expected_ty_path) = &binding.type_path { - // Resolving to a full path won't fail even if the reference type is not yet registered. + // Resolving to a full path won't fail even if the reference type is not yet + // registered. match symbols.resolve_path(expected_ty_path, symbols::ResolveKind::Type) { Ok(s) => Some(s), Err(e) => { @@ -910,10 +904,11 @@ pub(crate) fn register_assets( }; delayed.push(DelayedRegister { - expected_ty_path, - asset_kind: kind, asset: path.spanned(span), + asset_kind: kind, reference, + reference_original: ref_path.clone().spanned(binding.value.span()), + expected_ty_path, }); continue; } diff --git a/bauble/src/value/symbols.rs b/bauble/src/value/symbols.rs index 54427ea..32fda49 100644 --- a/bauble/src/value/symbols.rs +++ b/bauble/src/value/symbols.rs @@ -675,6 +675,23 @@ impl<'a, 'b> EarlySymbols<'a, 'b> { }) } + /// Note, if an asset isn't registered in `BaubleContext` yet, its type will be unknown. + pub fn resolve_asset(&self, path: &Path) -> Result<(Option, TypePath)> { + let item = self.resolve_item(path, ResolveKind::Asset)?.into_owned(); + + if let Some((ty, path, _kind)) = item.asset { + Ok((ty, path)) + } else { + Err(ConversionError::RefError(Box::new(RefError { + uses: Some(self.uses.keys().cloned().collect()), + path: self.resolve_path(path, ResolveKind::Asset)?.value, + path_ref: item, + kind: RefKind::Asset, + })) + .spanned(path.span())) + } + } + pub fn resolve_type(&self, path: &Path) -> Result { let item = self.resolve_item(path, ResolveKind::Type)?; @@ -715,19 +732,7 @@ impl SymbolsCommon for Symbols<'_> { impl SymbolsCommon for EarlySymbols<'_, '_> { /// Note, if an asset isn't registered in `BaubleContext` yet, its type will be unknown. fn resolve_asset_type(&self, path: &Path) -> Result> { - let item = self.resolve_item(path, ResolveKind::Asset)?.into_owned(); - - if let Some((ty, _path, _kind)) = item.asset { - Ok(ty) - } else { - Err(ConversionError::RefError(Box::new(RefError { - uses: Some(self.uses.keys().cloned().collect()), - path: self.resolve_path(path, ResolveKind::Asset)?.value, - path_ref: item, - kind: RefKind::Asset, - })) - .spanned(path.span())) - } + self.resolve_asset(path).map(|(ty, _path)| ty) } fn resolve_type(&self, path: &Path) -> Result { diff --git a/bauble/tests/integration.rs b/bauble/tests/integration.rs index d053f15..d4b87d2 100644 --- a/bauble/tests/integration.rs +++ b/bauble/tests/integration.rs @@ -927,3 +927,25 @@ fn amiguous_ref_with_ident_multilayer() { &[a_b_test, a, a_c_test, d], ); } + +/// Test for error message suggesting local asset. +#[test] +#[should_panic = " Did you mean `test`?"] +fn local_asset_suggested() { + let a = &test_file!( + "a", + "\n\ + 0 = $tet\n\ + test = integration::Test { x: -5, y: 5 }", + TestRef("a::test".into()), + Test { x: -5, y: 5 }, + ); + + test_load( + &|ctx| { + ctx.register_type::(); + }, + // Test when the referencing file is loaded before the referenced file + &[a], + ); +} From c8169c6521a4484ac4b01a1540a4030ca5d913d6 Mon Sep 17 00:00:00 2001 From: Imbris Date: Mon, 6 Apr 2026 03:35:08 -0400 Subject: [PATCH 17/25] Refactor Symbols path resolution to delay uses lookup to prepare for distinguishing local objects. --- bauble/src/value/early_context.rs | 13 ++ bauble/src/value/mod.rs | 20 +- bauble/src/value/symbols.rs | 309 +++++++++++++++++++----------- 3 files changed, 218 insertions(+), 124 deletions(-) diff --git a/bauble/src/value/early_context.rs b/bauble/src/value/early_context.rs index 23b7f0e..f392200 100644 --- a/bauble/src/value/early_context.rs +++ b/bauble/src/value/early_context.rs @@ -91,6 +91,19 @@ impl CombinedPathReference { module: xor_option(self.module, other.module)?, }) } + + /// Overrides references of `self` with references of `other`. + pub(super) fn combine_override(&mut self, other: Self) { + if other.ty.is_some() { + self.ty = other.ty; + } + if other.asset.is_some() { + self.asset = other.asset; + } + if other.module.is_some() { + self.module = other.module; + } + } } /// A type containing multiple references generally derived from a path. diff --git a/bauble/src/value/mod.rs b/bauble/src/value/mod.rs index 8fb8c66..0560e80 100644 --- a/bauble/src/value/mod.rs +++ b/bauble/src/value/mod.rs @@ -559,18 +559,12 @@ impl std::fmt::Display for PathKind { } } -impl PathKind { - fn could_be(&self, path: TypePath<&str>) -> bool { - match self { - PathKind::Direct(p) => p.borrow() == path, - PathKind::Indirect(leading, ident) => { - path.starts_with(leading.borrow()) - && path.ends_with(*ident.borrow()) - // If `leading` ends with `ident` we could have false positives if we didn't include this check. - && path.byte_len() > leading.byte_len() + ident.byte_len() - } - } - } +#[derive(Clone, Debug)] +pub enum ObjectPath { + /// Top-level object or external asset. There is at most one per file. + Top(TypePath), + /// Local object. Can only be referenced from other objects in the same file. + Local(TypePath), } /// We can delay registering `Ref` assets if what they're referencing hasn't been loaded yet. @@ -892,7 +886,7 @@ pub(crate) fn register_assets( let expected_ty_path = if let Some(expected_ty_path) = &binding.type_path { // Resolving to a full path won't fail even if the reference type is not yet // registered. - match symbols.resolve_path(expected_ty_path, symbols::ResolveKind::Type) { + match symbols.resolve_full_path_for_type(expected_ty_path) { Ok(s) => Some(s), Err(e) => { errors.push(e); diff --git a/bauble/src/value/symbols.rs b/bauble/src/value/symbols.rs index 32fda49..e043e71 100644 --- a/bauble/src/value/symbols.rs +++ b/bauble/src/value/symbols.rs @@ -15,7 +15,7 @@ use super::{ConversionError, EarlyContext, PathKind, RefError, RefKind, Result}; /// Indicates kind of item being resolved by [`Symbols::resolve_path`] or /// [`EarlySymbols::resolve_path`]. #[derive(Clone, Copy)] -pub(crate) enum ResolveKind { +enum ResolveKind { Type, Asset, } @@ -29,6 +29,18 @@ impl From for RefKind { } } +fn generic_with_non_type_error(raw_path: &Path) -> Spanned { + ConversionError::Custom(CustomError::new("Cannot use generics for non-type paths")) + .spanned(raw_path.span()) +} + +fn generic_param_using_with_ident(span: crate::Span) -> Spanned { + ConversionError::Custom(CustomError::new( + "Cannot use path::*::ident syntax in generic parameters", + )) + .spanned(span) +} + /// Representation of item names available in the current module. /// /// There are multiple namespaces: types, assets (i.e. values defined in bauble), and modules. @@ -212,65 +224,82 @@ impl<'a> Symbols<'a> { } } - let generic_with_non_type_error = || { - ConversionError::Custom(CustomError::new("Cannot use generics for non-type paths")) - .spanned(raw_path.span()) - }; let path = match &raw_path.last.value { PathEnd::WithIdent(ident) => PathKind::Indirect( leading, TypePathElem::new(ident.to_string()).map_err(|p| p.spanned(raw_path.span()))?, ), PathEnd::Ident(ident) => { - let used = if leading.is_empty() { - self.uses.get(ident.as_str()) - } else { - None - }; - let path = if let Some(r) = used - && let ResolveKind::Type = kind - && let Some(ty) = r.ty - { - self.ctx.type_registry().key_type(ty).meta.path.clone() - } else if let Some(r) = used - && let ResolveKind::Asset = kind - && let Some((_ty, path, _kind)) = &r.asset - { - path.clone() - } else { - leading - .push_str(ident.as_str()) - .map_err(|p| p.spanned(raw_path.span()))?; - leading - }; - PathKind::Direct(path) + leading + .push_str(ident.as_str()) + .map_err(|p| p.spanned(raw_path.span()))?; + PathKind::Direct(leading) } PathEnd::WithIdentGeneric(ident, generic) => { if !matches!(kind, ResolveKind::Type) { - return Err(generic_with_non_type_error()); + return Err(generic_with_non_type_error(raw_path)); } let generic = self.resolve_path(&generic.value, ResolveKind::Type)?; + let inner_path = match &generic.value { + PathKind::Direct(generic) => { + if let Some(r) = self.uses.get(generic.as_str()) + && let Some(ty) = r.ty + { + &self.ctx.type_registry().key_type(ty).meta.path + } else { + generic + } + } + PathKind::Indirect(_, _) => { + return Err(generic_param_using_with_ident(generic.span)); + } + }; PathKind::Indirect( leading, - TypePathElem::new(format!("{ident}<{generic}>")) + TypePathElem::new(format!("{ident}<{inner_path}>")) .map_err(|p| p.spanned(raw_path.span()))?, ) } PathEnd::IdentGeneric(ident, generic) => { if !matches!(kind, ResolveKind::Type) { - return Err(generic_with_non_type_error()); + return Err(generic_with_non_type_error(raw_path)); } + + // The outer type and inner parameter in generic types need to be fully expanded + // based on `uses` here (since the resolved path for a concrete instance of the + // generic type only makes sense to lookup via full paths in the context and not + // via `uses`). + + // Note: We could use `resolve_type` for the parameter to simplify computing + // `inner_path` except there are cases with reference types where the ref type + // isn't registered until an asset with the type is registered. So if there was + // `MyGeneric>`, then calling `resolve_type` here could fail. let generic = self.resolve_path(&generic.value, ResolveKind::Type)?; + let inner_path = match &generic.value { + PathKind::Direct(generic) => { + if let Some(r) = self.uses.get(generic.as_str()) + && let Some(ty) = r.ty + { + &self.ctx.type_registry().key_type(ty).meta.path + } else { + generic + } + } + PathKind::Indirect(_, _) => { + return Err(generic_param_using_with_ident(generic.span)); + } + }; + let path = if leading.is_empty() && let Some(r) = self.uses.get(ident.as_str()) && let Some(ty) = r.ty { let outer_path = &self.ctx.type_registry().key_type(ty).meta.path; - TypePath::new(format!("{outer_path}<{generic}>")) + TypePath::new(format!("{outer_path}<{inner_path}>")) .map_err(|p| p.spanned(raw_path.span()))? } else { leading - .push_str(&format!("{ident}<{generic}>")) + .push_str(&format!("{ident}<{inner_path}>")) .map_err(|p| p.spanned(raw_path.span()))?; leading }; @@ -280,23 +309,30 @@ impl<'a> Symbols<'a> { Ok(path.spanned(raw_path.span())) } - /// Note, the returned `PathReference` only makes sense to use for the `kind` passed here. The - /// same path for a different `kind` may return a different `PathReference`. - fn resolve_item(&self, raw_path: &Path, kind: ResolveKind) -> Result> { - // NOTE: This resolves the full path referred to via `uses`. This is unnecessary in many - // cases because there is a `PathReference` is available in `self.uses`. - // - // However, there would be a few complications: - // 1. Paths for generic types always need to be fully resolved. - // 2. If a particular identifier is available at the root of the `BaubleContext` in one - // namespace (e.g. type namespace) and in `self.uses` in another namespace (e.g. asset - // namespace) then we could miss something if we naively return the `PathReference` from - // `self.uses` (we would need to at least check `BaubleContext` if the `kind` of item - // was `None` in `self.uses`). + fn resolve_item( + &self, + raw_path: &Path, + kind: ResolveKind, + ) -> Result<(Cow<'_, PathReference>, PathKind)> { let path = self.resolve_path(raw_path, kind)?; let reference = match &path.value { - PathKind::Direct(path) => self.ctx.get_ref(path.borrow()).map(Cow::Owned), + PathKind::Direct(path) => { + let r_uses = self.uses.get(path.as_str()); + let r_ctx = self.ctx.get_ref(path.borrow()); + // There can be overlap between items that are children of the root of ctx and the + // uses here. Items from uses take priority and collisions aren't errors. + if let Some(r_uses) = r_uses { + Some(if let Some(mut r_ctx) = r_ctx { + r_ctx.combine_override(r_uses.clone()); + Cow::Owned(r_ctx) + } else { + Cow::Borrowed(r_uses) + }) + } else { + r_ctx.map(Cow::Owned) + } + } PathKind::Indirect(path, ident) => self .ctx .ref_with_ident(path.borrow(), ident.borrow()) @@ -304,16 +340,17 @@ impl<'a> Symbols<'a> { .map(Cow::Owned), }; - reference.ok_or_else(|| { - if let PathKind::Direct(path) = &*path + if let Some(reference) = reference { + Ok((reference, path.value)) + } else { + Err(if let PathKind::Direct(path) = &*path && let Some((leading, ident)) = path.get_end() && let Some(r) = self.ctx.get_ref(leading) && let Some(ty) = r.ty && matches!( self.ctx.type_registry().key_type(ty).kind, types::TypeKind::Enum { .. } | types::TypeKind::Or(_) - ) - { + ) { ConversionError::UnknownVariant { variant: ident.to_owned().spanned(raw_path.last.span), ty, @@ -326,19 +363,20 @@ impl<'a> Symbols<'a> { kind: kind.into(), })) } - .spanned(raw_path.span()) - }) + .spanned(raw_path.span())) + } } pub fn resolve_asset(&self, path: &Path) -> Result<(TypeId, TypePath)> { - let item = self.resolve_item(path, ResolveKind::Asset)?.into_owned(); + let (item, resolved_path) = self.resolve_item(path, ResolveKind::Asset)?; + let item = item.into_owned(); if let Some((ty, path, _kind)) = item.asset { Ok((ty, path)) } else { Err(ConversionError::RefError(Box::new(RefError { uses: Some(self.uses.keys().cloned().collect()), - path: self.resolve_path(path, ResolveKind::Asset)?.value, + path: resolved_path, path_ref: item.into(), kind: RefKind::Asset, })) @@ -347,14 +385,14 @@ impl<'a> Symbols<'a> { } pub fn resolve_type(&self, path: &Path) -> Result { - let item = self.resolve_item(path, ResolveKind::Type)?; + let (item, resolved_path) = self.resolve_item(path, ResolveKind::Type)?; if let Some(ty) = item.ty { Ok(ty) } else { Err(ConversionError::RefError(Box::new(RefError { uses: Some(self.uses.keys().cloned().collect()), - path: self.resolve_path(path, ResolveKind::Type)?.value, + path: resolved_path, path_ref: item.into_owned().into(), kind: RefKind::Type, })) @@ -516,7 +554,7 @@ impl<'a, 'b> EarlySymbols<'a, 'b> { .and_then(|reference| reference.module.clone()) } - pub fn resolve_path(&self, raw_path: &Path, kind: ResolveKind) -> Result> { + fn resolve_path(&self, raw_path: &Path, kind: ResolveKind) -> Result> { let mut leading = TypePath::empty(); let mut path_iter = raw_path.leading.iter(); @@ -554,65 +592,75 @@ impl<'a, 'b> EarlySymbols<'a, 'b> { } } - let generic_with_non_type_error = || { - ConversionError::Custom(CustomError::new("Cannot use generics for non-type paths")) - .spanned(raw_path.span()) - }; let path = match &raw_path.last.value { PathEnd::WithIdent(ident) => PathKind::Indirect( leading, TypePathElem::new(ident.to_string()).map_err(|p| p.spanned(raw_path.span()))?, ), PathEnd::Ident(ident) => { - let used = if leading.is_empty() { - self.uses.get(ident.as_str()) - } else { - None - }; - let path = if let Some(r) = used - && let ResolveKind::Type = kind - && let Some(ty) = r.ty - { - self.ctx.type_registry().key_type(ty).meta.path.clone() - } else if let Some(r) = used - && let ResolveKind::Asset = kind - && let Some((_ty, path, _kind)) = &r.asset - { - path.clone() - } else { - leading - .push_str(ident.as_str()) - .map_err(|p| p.spanned(raw_path.span()))?; - leading - }; - PathKind::Direct(path) + leading + .push_str(ident.as_str()) + .map_err(|p| p.spanned(raw_path.span()))?; + PathKind::Direct(leading) } PathEnd::WithIdentGeneric(ident, generic) => { if !matches!(kind, ResolveKind::Type) { - return Err(generic_with_non_type_error()); + return Err(generic_with_non_type_error(raw_path)); } + let generic = self.resolve_path(&generic.value, ResolveKind::Type)?; + let inner_path = match &generic.value { + PathKind::Direct(generic) => { + if let Some(r) = self.uses.get(generic.as_str()) + && let Some(ty) = r.ty + { + &self.ctx.type_registry().key_type(ty).meta.path + } else { + generic + } + } + PathKind::Indirect(_, _) => { + return Err(generic_param_using_with_ident(generic.span)); + } + }; + PathKind::Indirect( leading, - TypePathElem::new(format!("{ident}<{generic}>")) + TypePathElem::new(format!("{ident}<{inner_path}>")) .map_err(|p| p.spanned(raw_path.span()))?, ) } PathEnd::IdentGeneric(ident, generic) => { if !matches!(kind, ResolveKind::Type) { - return Err(generic_with_non_type_error()); + return Err(generic_with_non_type_error(raw_path)); } + let generic = self.resolve_path(&generic.value, ResolveKind::Type)?; + let inner_path = match &generic.value { + PathKind::Direct(generic) => { + if let Some(r) = self.uses.get(generic.as_str()) + && let Some(ty) = r.ty + { + &self.ctx.type_registry().key_type(ty).meta.path + } else { + generic + } + } + PathKind::Indirect(_, _) => { + return Err(generic_param_using_with_ident(generic.span)); + } + }; + let path = if leading.is_empty() && let Some(r) = self.uses.get(ident.as_str()) && let Some(ty) = r.ty { let outer_path = &self.ctx.type_registry().key_type(ty).meta.path; - TypePath::new(format!("{outer_path}<{generic}>")) + TypePath::new(format!("{outer_path}<{inner_path}>")) .map_err(|p| p.spanned(raw_path.span()))? } else { leading - .push_str(&format!("{ident}<{generic}>")) + .push_str(&format!("{ident}<{inner_path}>")) .map_err(|p| p.spanned(raw_path.span()))?; leading }; @@ -628,20 +676,26 @@ impl<'a, 'b> EarlySymbols<'a, 'b> { &self, raw_path: &Path, kind: ResolveKind, - ) -> Result> { - // NOTE: Similar to `Symbols::resolve_item`, this resolves the full path referred which is - // often unneccessary. - // - // But avoiding this has the same complications along with an additional ones: - // 3. There are other uses of `resolve_path` that need a full path because the resolved - // path needs to be looked up later when `EarlySymbols` is no longer available. - // 4. Once an asset is registered with its type, the path reference stored in `uses` will - // be outdated since it won't include the type (an up-to-date value isn't essential but - // would lead to fewer assets to process in resolve_delayed). + ) -> Result<(Cow<'_, CombinedPathReference>, PathKind)> { let path = self.resolve_path(raw_path, kind)?; let reference = match &path.value { - PathKind::Direct(path) => self.ctx.get_ref(path.borrow()).map(Cow::Owned), + PathKind::Direct(path) => { + let r_uses = self.uses.get(path.as_str()); + let r_ctx = self.ctx.get_ref(path.borrow()); + // There can be overlap between items that are children of the root of ctx and the + // uses here. Items from uses take priority and collisions aren't errors. + if let Some(r_uses) = r_uses { + Some(if let Some(mut r_ctx) = r_ctx { + r_ctx.combine_override(r_uses.clone()); + Cow::Owned(r_ctx) + } else { + Cow::Borrowed(r_uses) + }) + } else { + r_ctx.map(Cow::Owned) + } + } PathKind::Indirect(path, ident) => self .ctx .ref_with_ident(path.borrow(), ident.borrow()) @@ -649,16 +703,17 @@ impl<'a, 'b> EarlySymbols<'a, 'b> { .map(Cow::Owned), }; - reference.ok_or_else(|| { - if let PathKind::Direct(path) = &*path + if let Some(reference) = reference { + Ok((reference, path.value)) + } else { + Err(if let PathKind::Direct(path) = &*path && let Some((leading, ident)) = path.get_end() && let Some(r) = self.ctx.get_ref(leading) && let Some(ty) = r.ty && matches!( self.ctx.type_registry().key_type(ty).kind, types::TypeKind::Enum { .. } | types::TypeKind::Or(_) - ) - { + ) { ConversionError::UnknownVariant { variant: ident.to_owned().spanned(raw_path.last.span), ty, @@ -671,20 +726,52 @@ impl<'a, 'b> EarlySymbols<'a, 'b> { kind: kind.into(), })) } - .spanned(raw_path.span()) - }) + .spanned(raw_path.span())) + } + } + + /// Resolves full path without requiring that the type has been registered yet. + /// + /// Useful to later lookup the type when `EarlySymbols` is no longer available. + pub fn resolve_full_path_for_type(&self, path: &Path) -> Result> { + let path = self.resolve_path(path, ResolveKind::Type)?; + Ok(match path.value { + PathKind::Direct(path) => { + if let Some(r) = self.uses.get(path.as_str()) + && let Some(ty) = r.ty + { + PathKind::Direct(self.ctx.type_registry().key_type(ty).meta.path.clone()) + } else { + PathKind::Direct(path) + } + } + p @ PathKind::Indirect(_, _) => p, + } + .spanned(path.span)) } /// Note, if an asset isn't registered in `BaubleContext` yet, its type will be unknown. pub fn resolve_asset(&self, path: &Path) -> Result<(Option, TypePath)> { - let item = self.resolve_item(path, ResolveKind::Asset)?.into_owned(); - - if let Some((ty, path, _kind)) = item.asset { + let (item, resolved_path) = self.resolve_item(path, ResolveKind::Asset)?; + let item = item.into_owned(); + + if let Some((mut ty, path, _kind)) = item.asset { + // Once an asset is registered with its type, the path reference stored in `uses` will + // be outdated since it won't include the type (an up-to-date value isn't essential but + // will lead to fewer assets to process in resolve_delayed). + if ty.is_none() { + ty = self + .ctx + .get_ref(path.borrow()) + .and_then(|r| r.asset) + .expect("This asset is in uses, so it will exist in ctx.") + .0; + } Ok((ty, path)) } else { Err(ConversionError::RefError(Box::new(RefError { uses: Some(self.uses.keys().cloned().collect()), - path: self.resolve_path(path, ResolveKind::Asset)?.value, + path: resolved_path, path_ref: item, kind: RefKind::Asset, })) @@ -693,14 +780,14 @@ impl<'a, 'b> EarlySymbols<'a, 'b> { } pub fn resolve_type(&self, path: &Path) -> Result { - let item = self.resolve_item(path, ResolveKind::Type)?; + let (item, resolved_path) = self.resolve_item(path, ResolveKind::Type)?; if let Some(ty) = item.ty { Ok(ty) } else { Err(ConversionError::RefError(Box::new(RefError { uses: Some(self.uses.keys().cloned().collect()), - path: self.resolve_path(path, ResolveKind::Type)?.value, + path: resolved_path, path_ref: item.into_owned(), kind: RefKind::Type, })) From 0f054bfdec8a03c353fd8817def8458b70bf0817 Mon Sep 17 00:00:00 2001 From: Imbris Date: Tue, 7 Apr 2026 16:57:36 -0400 Subject: [PATCH 18/25] Use new ObjectPath enum for fully resolved paths to objects, remove local objects from BaubleContext, and prevent them from being referenced outside the current file. ObjectPath uses enum variants to distinguish top-level, local, and inline objects. Objects of different kinds can now have the same path without colliding. --- bauble/src/builtin.rs | 14 +- bauble/src/context.rs | 117 ++++--------- bauble/src/lib.rs | 3 +- bauble/src/local_context.rs | 71 ++++---- bauble/src/object_path.rs | 152 ++++++++++++++++ bauble/src/parse/parser.rs | 6 +- bauble/src/parse/value.rs | 15 +- bauble/src/traits.rs | 13 +- bauble/src/types.rs | 24 +-- bauble/src/types/path.rs | 1 + bauble/src/value/convert.rs | 83 +++++---- bauble/src/value/display.rs | 100 +++++++---- bauble/src/value/early_context.rs | 137 ++++----------- bauble/src/value/error.rs | 38 ++-- bauble/src/value/mod.rs | 246 ++++++++++++-------------- bauble/src/value/symbols.rs | 279 +++++++++++++++++++++++++----- bauble/tests/integration.rs | 146 +++++++++------- bauble_macros/tests/derive.rs | 16 +- 18 files changed, 867 insertions(+), 594 deletions(-) create mode 100644 bauble/src/object_path.rs diff --git a/bauble/src/builtin.rs b/bauble/src/builtin.rs index da2d321..589ad23 100644 --- a/bauble/src/builtin.rs +++ b/bauble/src/builtin.rs @@ -1,6 +1,6 @@ use crate::{ Bauble, BaubleAllocator, ToRustErrorKind, Val, Value, - path::TypePath, + object_path::ObjectPath, types::{Type, TypeRegistry}, }; use std::{cell::UnsafeCell, fmt::Debug, marker::PhantomData}; @@ -25,14 +25,14 @@ use std::{cell::UnsafeCell, fmt::Debug, marker::PhantomData}; /// `S` is the inner representation used for `TypePath`. pub struct Ref { /// The path to the referenced asset. - pub path: TypePath, + pub path: ObjectPath, /// Invariant over `T`. _mark: PhantomData>, } impl Ref { /// Create a reference from the specified path. The path must not be valid. - pub fn from_path(path: TypePath) -> Self { + pub fn from_path(path: ObjectPath) -> Self { Self { path, _mark: PhantomData, @@ -40,12 +40,12 @@ impl Ref { } } -impl PartialEq for Ref { +impl> PartialEq for Ref { fn eq(&self, other: &Self) -> bool { self.path == other.path } } -impl Eq for Ref {} +impl> Eq for Ref {} impl Debug for Ref { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { @@ -53,7 +53,7 @@ impl Debug for Ref { } } -impl<'b, A: BaubleAllocator<'b>, T: Bauble<'b, A>> Bauble<'b, A> for Ref { +impl<'b, A: BaubleAllocator<'b>, T: Bauble<'b, A>> Bauble<'b, A> for Ref { fn builtin(registry: &mut TypeRegistry) -> Option { let inner = registry.get_or_register_type::(); Some(registry.get_or_register_asset_ref(inner)) @@ -69,7 +69,7 @@ impl<'b, A: BaubleAllocator<'b>, T: Bauble<'b, A>> Bauble<'b, A> for Ref Result<>::Out, crate::ToRustError> { match val.value.value { Value::Ref(r) => Ok({ - let path = allocator.wrap_type_path(r); + let path = allocator.wrap_object_path(r); let value = Self { // SAFETY: path was derived from `allocator`. path: unsafe { allocator.validate(path)? }, diff --git a/bauble/src/context.rs b/bauble/src/context.rs index 45ffef8..e98bf14 100644 --- a/bauble/src/context.rs +++ b/bauble/src/context.rs @@ -12,27 +12,13 @@ pub type Source = ariadne::Source; #[derive(Clone, Default)] struct DefaultUses(IndexMap); -/// Assets can be either top level or local. -#[derive(Clone, Copy, Debug, PartialEq)] -pub enum AssetKind { - // TODO: update this in stage 2 - /// The first asset in a file, if it has the same as the file. - /// - /// These assets share the same path as the file and are generally intended to be globally - /// visible and referencable. - TopLevel, - /// All assets that aren't considered top level. These are generally intended to only be - /// referenced from the same file (although this isn't enforced yet). - Local, -} - /// A type containing multiple references generally derived from a path. #[derive(Default, Clone, Debug)] pub struct PathReference { /// The type referenced by a path. pub ty: Option, /// The asset (and its path) referenced by the path. - pub asset: Option<(TypeId, TypePath, AssetKind)>, + pub asset: Option<(TypeId, TypePath)>, /// If the reference references a module. pub module: Option, } @@ -48,10 +34,10 @@ impl PathReference { } /// Constructs a path reference referencing the 'any' type. - pub fn any(path: TypePath, kind: AssetKind) -> Self { + pub fn any(path: TypePath) -> Self { Self { ty: Some(TypeRegistry::any_type()), - asset: Some((TypeRegistry::any_type(), path.clone(), kind)), + asset: Some((TypeRegistry::any_type(), path.clone())), module: Some(path.clone()), } } @@ -211,7 +197,7 @@ impl BaubleContextBuilder { #[derive(Default, Clone, Debug)] struct InnerReference { ty: Option, - asset: Option<(TypeId, AssetKind)>, + asset: Option, redirect: Option, } @@ -252,10 +238,7 @@ impl CtxNode { fn reference(&self, root: &Self) -> PathReference { let mut this = PathReference { ty: self.reference.ty, - asset: self - .reference - .asset - .map(|(ty, kind)| (ty, self.path.clone(), kind)), + asset: self.reference.asset.map(|ty| (ty, self.path.clone())), module: (!self.children.is_empty()).then(|| self.path.clone()), }; @@ -398,11 +381,6 @@ impl CtxNode { /// that feature is added). fn clear_file_assets(&mut self) { self.children.retain(|_, node| { - // `AssetKind::TopLevel` will be from other files and we don't want to clear those. - node.reference - .asset - .take_if(|(_, kind)| matches!(kind, AssetKind::Local)); - // Avoid clearing assets from other files, but recurse into inline submodules (if that // feature is added). if node.source.is_none() { @@ -412,11 +390,9 @@ impl CtxNode { !node.is_empty() }); - // Top level assets match the path of their file so this will be from this file if it is - // top level. - self.reference - .asset - .take_if(|(_, kind)| matches!(kind, AssetKind::TopLevel)); + // Top level assets match the path of their file (and all assets registered in + // BaubleContext are top level). + self.reference.asset.take(); } /// Builds all path elements as modules @@ -448,14 +424,14 @@ impl CtxNode { node.reference.ty = Some(id); } - fn build_asset(&mut self, path: TypePath<&str>, ty: TypeId, kind: AssetKind) -> Result<(), ()> { + fn build_asset(&mut self, path: TypePath<&str>, ty: TypeId) -> Result<(), ()> { let node = self.build_nodes(path); if node.reference.asset.is_some() { // Multiple assets with the same path return Err(()); } - node.reference.asset = Some((ty, kind)); + node.reference.asset = Some(ty); Ok(()) } } @@ -529,34 +505,18 @@ impl BaubleContext { /// Registers an asset. This is done automatically for any objects in a file that gets registered. /// - /// With this method you can expose assets that aren't in bauble. + /// With this method you can expose assets that aren't in bauble. Externally registered assets + /// are expected to never collide with paths of bauble objects from loaded files. /// - /// Returns ID of internal Ref type for `ty`. - /// - /// Returns an error if an asset was already registered at this path. This can occur due to - /// simplification of the top level asset path to match the current file. E.g. the top-level - /// asset in `a::1` will conflict with the path of object `1` in file `a`. - // - // TODO: in stage 2 we might be able to adjust this so that local object paths are - // distinguished from top level objects, such that they don't clash. - pub fn register_asset( - &mut self, - path: TypePath<&str>, - ty: TypeId, - kind: AssetKind, - ) -> Result { + /// # Panics + /// Panics if an asset was already registered at this path. + pub fn register_asset(&mut self, path: TypePath<&str>, ty: TypeId) { let ref_ty = self.register_asset_ref_ty(ty); self.root_node - .build_asset(path, ref_ty, kind) - // TODO: this error should no longer be possible? - .map_err(|()| { - crate::CustomError::new(format!( - "'{path}' refers to an existing asset in another file. This can be \n\ - caused by special cased path simplification for the first object in a \n\ - file.", - )) - })?; - Ok(ref_ty) + .build_asset(path, ref_ty) + // An error should produced during parsing when identifiers overlap, and it isn't + // possible to have overlapping paths from different files. + .expect("Multiple assets with the same path"); } fn file(&self, file: FileId) -> (TypePath<&str>, &Source) { @@ -643,7 +603,6 @@ impl BaubleContext { } } - let mut skip = Vec::new(); let mut early_ctx = crate::value::EarlyContext::new(self); // Register assets paths from each successfully parsed file into the early context (before @@ -652,45 +611,39 @@ impl BaubleContext { // Need a partial borrow here. let (path, _) = early_ctx.ctx.file(*file); let path = path.to_owned(); - match crate::value::pre_register_assets(&mut early_ctx, path.borrow(), values) { - Ok(()) => {} - Err(e) => { - // Skip files that errored on registering. - skip.push(*file); - errors.extend(BaubleErrors::from(e)); - } - } + crate::value::pre_register_assets(&mut early_ctx, path.borrow(), values); } + let mut local_ctx = crate::local_context::LocalContext::new(); + let mut delayed = Vec::new(); - let mut skip_iter = skip.iter().copied().peekable(); - let mut skip_more = Vec::new(); + let mut skip = Vec::new(); // Then, register assets from each successfully parsed file into the context (while // resolving types). - for (file, values) in file_values - .iter() - // Skip files with errors - .filter(|(file, _)| skip_iter.next_if_eq(file).is_none()) - { + for (file, values) in file_values.iter() { // Need a partial borrow here. let (path, _) = early_ctx.ctx.file(*file); let path = path.to_owned(); - match crate::value::register_assets(&mut early_ctx, path.borrow(), values) { + match crate::value::register_assets( + &mut early_ctx, + &mut local_ctx, + path.borrow(), + values, + ) { Ok(d) => { delayed.extend(d); } Err(e) => { // Skip files that errored on registering. - skip_more.push(*file); + skip.push(*file); errors.extend(BaubleErrors::from(e)); } } } - skip.extend(skip_more); // TODO: Less hacky way to get which files errored here? - if let Err(e) = crate::value::resolve_delayed(delayed, self) { + if let Err(e) = crate::value::resolve_delayed(delayed, self, &mut local_ctx) { // We want to skip any files that had errors. for e in e { skip.push(e.span.file()); @@ -713,7 +666,7 @@ impl BaubleContext { // Skip files with errors .filter(|(file, _)| skip_iter.next_if_eq(file).is_none()) { - match crate::value::convert_values(file, values, self) { + match crate::value::convert_values(file, values, self, &local_ctx) { Ok(o) => objects.extend(o), Err(e) => errors.extend(e), } @@ -827,7 +780,7 @@ impl BaubleContext { &self, path: TypePath<&str>, max_depth: Option, - ) -> impl Iterator, AssetKind)> { + ) -> impl Iterator> { self.root_node .node_at(path) .map(|node| node.iter_all_children(max_depth)) @@ -836,7 +789,7 @@ impl BaubleContext { .filter_map(|node| { node.reference(&self.root_node) .asset - .map(|(_, _, kind)| (node.path.borrow(), kind)) + .map(|(_, _)| node.path.borrow()) }) } diff --git a/bauble/src/lib.rs b/bauble/src/lib.rs index dd4d5e1..8cab17b 100644 --- a/bauble/src/lib.rs +++ b/bauble/src/lib.rs @@ -11,12 +11,13 @@ mod spanned; mod traits; mod value; +pub mod object_path; pub mod types; pub use bauble_macros::Bauble; pub use builtin::Ref; -pub use context::{AssetKind, BaubleContext, BaubleContextBuilder, FileId, PathReference, Source}; +pub use context::{BaubleContext, BaubleContextBuilder, FileId, PathReference, Source}; pub use error::{BaubleError, BaubleErrors, CustomError, Level}; pub use spanned::{Span, SpanExt, Spanned}; pub use traits::{ diff --git a/bauble/src/local_context.rs b/bauble/src/local_context.rs index 123351f..786dffb 100644 --- a/bauble/src/local_context.rs +++ b/bauble/src/local_context.rs @@ -1,53 +1,42 @@ use crate::context::BaubleContext; -use crate::path::{TypePath, TypePathElem}; +use crate::path::TypePath; use crate::types::TypeId; use indexmap::IndexMap; -/// Assets are only referencable from other assets in the same file. -pub(crate) struct LocalAsset { - /// Type of a Ref to this asset. - ty: TypeId, - // TODO: might not be necessary? - /// Full path to this asset. - /// - /// This is the path to reach this node from the root. - path: TypePath, -} - -pub(crate) struct LocalAssets { - assets: IndexMap, -} - -/// Transient context that is built and used when loading a file. +/// Transient context that is built and used when loading file(s). /// -/// Contains registery of local assets. -pub(crate) struct LocalContext<'a> { - local: IndexMap, - ctx: &'a mut BaubleContext, +/// Contains registery of local objects. These are objects that are only referencable by other +/// objects in the same file. +pub(crate) struct LocalContext { + /// Map of local object paths to type ID of a Ref type to each object. + objects: IndexMap, } -impl LocalContext<'_> { - /// Returns ID of internal Ref type for `ty`. - pub fn register_asset(&mut self, path: TypePath, ty: TypeId) -> TypeId { - let ref_ty = self.ctx.register_asset_ref_ty(ty); - let asset = LocalAsset { ty: ref_ty, path }; - let (file, ident) = asset - .path - .get_end() - .expect("Register asset with empty path"); - let assets = &mut self - .local - .entry(file.to_owned()) - .or_insert_with(|| LocalAssets { - assets: IndexMap::new(), - }) - .assets; - if assets.contains_key(ident.as_str()) { - panic!("{} refers to an existing asset", asset.path); +impl LocalContext { + /// Creates a new local context. + pub fn new() -> Self { + Self { + objects: IndexMap::new(), + } + } + + /// Registers a local object. + /// + /// `BaubleContext` parameter is used to retrieve/register `Ref` type for this object. + /// + /// # Panics + /// Panics if an object was already registered at this path. + pub fn register(&mut self, path: TypePath, ty: TypeId, ctx: &mut BaubleContext) { + let ref_ty = ctx.register_asset_ref_ty(ty); + if self.objects.contains_key(path.as_str()) { + panic!("{} refers to an existing object", path); } else { - assets.insert(ident.to_owned(), asset); + self.objects.insert(path, ref_ty); } + } - ref_ty + /// Returns type of local bauble object at this path (if one exists). + pub fn get(&self, path: TypePath<&str>) -> Option { + self.objects.get(&path).copied() } } diff --git a/bauble/src/object_path.rs b/bauble/src/object_path.rs new file mode 100644 index 0000000..646df36 --- /dev/null +++ b/bauble/src/object_path.rs @@ -0,0 +1,152 @@ +//! Bauble objects can either be top-level, local, or inline. +//! +//! For a reference to an object, this is known once the full path to an +//! object is resolved during value loading. That resolved path is represented using +//! [`ObjectPath`]. +//! +//! When written in a file the top level object of each file uses a special identifier +//! [`TOP_LEVEL_IDENTIFIER`]. +//! +//! The [`ObjectPathKey`] trait exists to work past limitations in existing `HashMap` trait bounds +//! on keys so that borrowed versions of `ObjectPath` can be used to lookup values in a `HashMap` +//! using owned `ObjectPath` keys. + +use crate::types::path::TypePath; +use std::hash::{Hash, Hasher}; + +/// Special cased identifier that is required and only allowed for the first asset in a file +/// (i.e. the top level asset that is named after the file). +pub const TOP_LEVEL_IDENTIFIER: &str = "0"; + +/// Full path to a bauble object (or external asset). +/// +/// Path format documented in [`TypePath`]. +#[derive(Copy, Clone, Debug)] +pub enum ObjectPath { + /// Top-level object or external asset. There is at most one per file. + /// + /// This shares the path of the containing file and uses the special identifier + /// [`TOP_LEVEL_IDENTIFIER`]. + /// + /// These objects are generally intended to be globally visible and referencable. + Top(TypePath), + /// Local object. + /// + /// All objects that aren't considered top level or inline. These can only be referenced from + /// the same file. + Local(TypePath), + /// Object defined inline in the definition of the parent object that references it. + /// + /// This is implicitly only referencable by the parent object. + Inline(TypePath), +} + +impl> ObjectPath { + /// Identifier that is used for this object when it appears in a bauble file. + /// + /// `None` for inline objects and for local objects that have an empty path. + pub fn ident(&self) -> Option<&str> { + match self { + Self::Top(_) => Some(TOP_LEVEL_IDENTIFIER), + Self::Local(path) => path.get_end().map(|(_, end)| end.into_str()), + Self::Inline(_) => None, + } + } + + /// Gets a borrowed version of the path. + pub fn borrow(&self) -> ObjectPath<&str> { + match self { + Self::Top(path) => ObjectPath::Top(path.borrow()), + Self::Local(path) => ObjectPath::Local(path.borrow()), + Self::Inline(path) => ObjectPath::Inline(path.borrow()), + } + } + + /// Casts reference to trait object that `ObjectPath` implements `Borrow` for. + /// + /// This useful to mix owned and borrowed paths when using `ObjectPath` as a key type in a map + /// that requires the `Borrow` trait. + pub fn as_key(&self) -> &dyn ObjectPathKey { + self + } +} + +#[derive(Copy, Clone, PartialEq, Eq, Hash, PartialOrd, Ord)] +enum ObjectPathKeyImpl<'a> { + Top(TypePath<&'a str>), + Local(TypePath<&'a str>), + Inline(TypePath<&'a str>), +} + +impl<'a> From> for ObjectPathKeyImpl<'a> { + fn from(path: ObjectPath<&'a str>) -> Self { + match path { + ObjectPath::Top(path) => Self::Top(path), + ObjectPath::Local(path) => Self::Local(path), + ObjectPath::Inline(path) => Self::Inline(path), + } + } +} + +impl> Hash for ObjectPath { + fn hash(&self, state: &mut H) { + ObjectPathKeyImpl::from(self.borrow()).hash(state) + } +} + +impl> PartialEq for ObjectPath { + fn eq(&self, other: &Self) -> bool { + ObjectPathKeyImpl::from(self.borrow()) == ObjectPathKeyImpl::from(other.borrow()) + } +} + +impl> Eq for ObjectPath {} + +impl> PartialOrd for ObjectPath { + fn partial_cmp(&self, other: &Self) -> Option { + Some(self.cmp(other)) + } +} + +impl> Ord for ObjectPath { + fn cmp(&self, other: &Self) -> std::cmp::Ordering { + ObjectPathKeyImpl::from(self.borrow()).cmp(&ObjectPathKeyImpl::from(other.borrow())) + } +} + +/// Trait that allows mixing borrowed and owned paths when using them as keys for methods like +/// `HashMap::get`. +/// +/// See https://github.com/rust-lang/libs-team/issues/699#issue-3643767617 +/// +/// Works around lack of https://github.com/rust-lang/rust/issues/145986 +pub trait ObjectPathKey { + #[doc(hidden)] + fn key(&self) -> ObjectPath<&str>; +} + +impl> ObjectPathKey for ObjectPath { + fn key(&self) -> ObjectPath<&str> { + self.borrow() + } +} + +impl<'a, S: AsRef + 'a> std::borrow::Borrow for ObjectPath { + fn borrow(&self) -> &(dyn ObjectPathKey + 'a) { + self + } +} + +impl Hash for dyn ObjectPathKey { + fn hash(&self, state: &mut H) { + self.key().hash(state) + } +} + +impl PartialEq for dyn ObjectPathKey { + fn eq(&self, other: &Self) -> bool { + self.key() == other.key() + } +} + +impl Eq for dyn ObjectPathKey {} diff --git a/bauble/src/parse/parser.rs b/bauble/src/parse/parser.rs index c734a55..9e352c4 100644 --- a/bauble/src/parse/parser.rs +++ b/bauble/src/parse/parser.rs @@ -689,7 +689,7 @@ pub fn parser<'a>() -> impl Parser<'a, ParserSource<'a>, ParseValues, Extra<'a>> }, |mut values, (i, (ident, type_path, value))| { let is_first = i == 0; - let has_top_level_ident = *ident == BindingIdent::TOP_LEVEL_IDENTIFIER; + let has_top_level_ident = *ident == crate::object_path::TOP_LEVEL_IDENTIFIER; let error_emitted = match (is_first, has_top_level_ident) { (true, true) | (false, false) => false, @@ -698,7 +698,7 @@ pub fn parser<'a>() -> impl Parser<'a, ParserSource<'a>, ParseValues, Extra<'a>> ident.span, format!( "The first item must have '{}' as the identifier", - BindingIdent::TOP_LEVEL_IDENTIFIER + crate::object_path::TOP_LEVEL_IDENTIFIER ), )); true @@ -708,7 +708,7 @@ pub fn parser<'a>() -> impl Parser<'a, ParserSource<'a>, ParseValues, Extra<'a>> ident.span, format!( "Identifier '{}' is only allowed for the first item", - BindingIdent::TOP_LEVEL_IDENTIFIER + crate::object_path::TOP_LEVEL_IDENTIFIER ), )); true diff --git a/bauble/src/parse/value.rs b/bauble/src/parse/value.rs index 39cf5ac..2bea8a5 100644 --- a/bauble/src/parse/value.rs +++ b/bauble/src/parse/value.rs @@ -9,8 +9,11 @@ use indexmap::IndexMap; #[derive(Clone, Debug, PartialEq)] pub enum PathEnd { - // TODO: document how this syntax works? /// path::*::ident + /// + /// This refers to an item with the identifier `ident` in some path that starts with + /// `path`. If multiple such paths exist within the same item namespace, an error will be + /// generated. WithIdent(Ident), /// path::ident Ident(Ident), @@ -188,13 +191,9 @@ pub enum BindingIdent { } impl BindingIdent { - /// Special cased identifier that is required and only allowed for the first asset in a file - /// (i.e. the top level asset that is named after the file). - pub const TOP_LEVEL_IDENTIFIER: &str = "0"; - pub fn as_str(&self) -> &str { match self { - Self::TopLevel(_) => Self::TOP_LEVEL_IDENTIFIER, + Self::TopLevel(_) => crate::object_path::TOP_LEVEL_IDENTIFIER, Self::Local(ident) => ident, } } @@ -205,10 +204,6 @@ impl BindingIdent { Self::Local(ident) => ident.span, } } - - pub fn is_top_level(&self) -> bool { - matches!(self, Self::TopLevel(_)) - } } #[derive(Debug)] diff --git a/bauble/src/traits.rs b/bauble/src/traits.rs index b134bf7..028fd01 100644 --- a/bauble/src/traits.rs +++ b/bauble/src/traits.rs @@ -4,6 +4,7 @@ use crate::{ CustomError, SpannedValue, UnspannedVal, Val, Value, context::BaubleContext, error::{BaubleError, ErrorMsg, Level}, + object_path::ObjectPath, path::{TypePath, TypePathElem}, spanned::{Span, Spanned}, types::{self, Extra, FieldType, TypeId}, @@ -243,8 +244,8 @@ pub trait BaubleAllocator<'a> { where Self: 'a; - /// The inner representation of a type path created by this allocator. - type TypePathInner: 'static; + /// The inner representation of an object path created by this allocator. + type PathInner: 'static; /// # Safety /// Allocations in `value` have to be allocated with this allocator @@ -253,8 +254,8 @@ pub trait BaubleAllocator<'a> { /// If validated an item must be placed within the same allocator. unsafe fn validate(&self, value: Self::Out) -> Result; - /// Create a type path from this allocator. - fn wrap_type_path(&self, path: TypePath) -> Self::Out>; + /// Create a object path from this allocator. + fn wrap_object_path(&self, path: ObjectPath) -> Self::Out>; } /// The standard Rust allocator when used by Bauble. @@ -262,7 +263,7 @@ pub struct DefaultAllocator; impl BaubleAllocator<'_> for DefaultAllocator { type Out = T; - type TypePathInner = String; + type PathInner = String; unsafe fn wrap(&self, value: T) -> Self::Out { value @@ -272,7 +273,7 @@ impl BaubleAllocator<'_> for DefaultAllocator { Ok(value) } - fn wrap_type_path(&self, path: TypePath) -> TypePath { + fn wrap_object_path(&self, path: ObjectPath) -> ObjectPath { path } } diff --git a/bauble/src/types.rs b/bauble/src/types.rs index 211437d..d705c62 100644 --- a/bauble/src/types.rs +++ b/bauble/src/types.rs @@ -18,7 +18,10 @@ pub mod path; use indexmap::IndexMap; use path::{TypePath, TypePathElem}; -use crate::{AdditionalUnspannedObjects, Bauble, BaubleAllocator, value::UnspannedVal}; +use crate::{ + AdditionalUnspannedObjects, Bauble, BaubleAllocator, object_path::ObjectPath, + value::UnspannedVal, +}; #[allow(missing_docs)] pub type Extra = IndexMap; @@ -202,8 +205,8 @@ pub enum TypeSystemError<'a> { InstantiableErrors, ConstructInequality(String, UnspannedVal, UnspannedVal), MissingObjects { - instantiated_missing: Vec, - loaded_unknown: Vec, + instantiated_missing: Vec, + loaded_unknown: Vec, }, } @@ -559,6 +562,8 @@ impl TypeRegistry { } let file = TypePath::new("validate").unwrap(); + // TODO: add dummy top level object + if assert_instanciable { let mut objects = Vec::new(); for (i, ty_id) in self @@ -579,7 +584,7 @@ impl TypeRegistry { let object_path = path_end.strip_generic().append(&format!("_{i}")).unwrap(); let object_name = object_path.get_end().unwrap().1; - let object_path = file.join(&object_name); + let object_path = ObjectPath::Local(file.join(&object_name)); let mut additonal = AdditionalUnspannedObjects::new(file, object_name.borrow()); @@ -590,19 +595,14 @@ impl TypeRegistry { }); }; - for (name, value) in additonal.into_objects() { + for (path, value) in additonal.into_objects() { objects.push(crate::Object { - object_path: file.join(&name), - top_level: false, + object_path: path, value, }) } - objects.push(crate::Object { - object_path, - top_level: false, - value, - }) + objects.push(crate::Object { object_path, value }) } // Check that instantiated objects match after being serialized to bauble text and diff --git a/bauble/src/types/path.rs b/bauble/src/types/path.rs index 3d4389b..3c167cf 100644 --- a/bauble/src/types/path.rs +++ b/bauble/src/types/path.rs @@ -487,6 +487,7 @@ impl> TypePath { }) } + // TODO: can this method be phased out now that `ObjectPath` exists? /// Determines if a path referencing an object is a path to a sub-object. /// The path is assumed to be valid. /// diff --git a/bauble/src/value/convert.rs b/bauble/src/value/convert.rs index 811f4b4..2eaff45 100644 --- a/bauble/src/value/convert.rs +++ b/bauble/src/value/convert.rs @@ -7,8 +7,8 @@ use crate::{ spanned::{SpanExt, Spanned}, types::{self, TypeId, TypeRegistry}, value::{ - Attributes, Fields, Ident, SpannedValue, Symbols, UnspannedVal, Val, Value, ValueContainer, - ValueTrait, error::Result, symbols::SymbolsCommon, + Attributes, Fields, Ident, ObjectPath, SpannedValue, Symbols, UnspannedVal, Val, Value, + ValueContainer, ValueTrait, error::Result, symbols::SymbolsCommon, }, }; @@ -234,11 +234,11 @@ pub enum AnyVal<'a> { pub struct ConvertMeta<'a> { pub symbols: &'a Symbols<'a>, pub additional_objects: &'a mut AdditionalObjects, - // TODO: in stage 2 (of asset per file impl) this probably needs to be `0` rather than anything - // that could conflict with the name of a local asset. Because this is used to name inline - // objects uniquely. - // /// Used as a prefix to uniquely name inline objects. + /// + /// Note, for top level objects we specifically use `0` and not the file name. If we used the + /// file name inline object names could collide with inline objects of a local object in + /// another file. pub object_name: TypePathElem<&'a str>, pub default_span: Span, } @@ -296,15 +296,11 @@ impl AdditionalObjects { .or_insert(0); let name = TypePathElem::new(format!("{name}@{idx}")) .expect("idx is just a number, and we know name is a valid path elem."); + let path = ObjectPath::Inline(self.file_path.join(&name)); + self.objects + .push(super::create_object(path.clone(), val, types)?); - self.objects.push(super::create_object( - self.file_path.join(&name), - false, - val, - types, - )?); - - Ok(Value::Ref(self.file_path.join(&name))) + Ok(Value::Ref(path)) } pub(super) fn into_objects(self) -> Vec { @@ -326,13 +322,9 @@ impl AdditionalObjects { let res = f(&mut unspanned); - for (name, value) in unspanned.into_objects() { - self.objects.push(super::create_object( - self.file_path.join(&name), - false, - value.into_spanned(span), - types, - )?); + for (path, value) in unspanned.into_objects() { + self.objects + .push(super::create_object(path, value.into_spanned(span), types)?); } Ok(res) @@ -343,7 +335,8 @@ impl AdditionalObjects { enum NameAllocs<'a> { Owned(HashMap), Borrowed(&'a mut HashMap), - Custom(&'a mut dyn FnMut(TypePathElem<&str>) -> TypePathElem), + // Parameters: (file, parent_object_name) + Custom(&'a mut dyn FnMut(TypePath<&str>, TypePathElem<&str>) -> ObjectPath), } impl NameAllocs<'_> { @@ -355,21 +348,26 @@ impl NameAllocs<'_> { } } - /// Allocate a new name for an object that will be referenced by the parent object + /// Allocate a new path for an object that will be referenced by the parent object /// `object_name`. - fn allocate_name(&mut self, object_name: TypePathElem<&str>) -> TypePathElem { + fn allocate_path( + &mut self, + file: TypePath<&str>, + object_name: TypePathElem<&str>, + ) -> ObjectPath { let from_map = |m: &mut HashMap| { let idx = *m .entry(object_name.to_owned()) .and_modify(|i| *i += 1u64) .or_insert(0); - TypePathElem::new(format!("{}@{idx}", object_name)) - .expect("idx is just a number, and we know name is a valid path elem.") + let name = TypePathElem::new(format!("{}@{idx}", object_name)) + .expect("idx is just a number, and we know name is a valid path elem."); + ObjectPath::Inline(file.join(&name)) }; match self { Self::Owned(m) => from_map(m), Self::Borrowed(m) => from_map(m), - Self::Custom(f) => f(object_name), + Self::Custom(f) => f(file, object_name), } } } @@ -378,7 +376,7 @@ impl NameAllocs<'_> { pub struct AdditionalUnspannedObjects<'a> { file_path: TypePath<&'a str>, object_name: TypePathElem<&'a str>, - objects: Vec<(TypePathElem, UnspannedVal)>, + objects: Vec<(ObjectPath, UnspannedVal)>, name_allocs: NameAllocs<'a>, } @@ -412,13 +410,14 @@ impl<'a> AdditionalUnspannedObjects<'a> { /// This allows creating the additional objects as objects that aren't sub-objects (aka inline /// objects). /// - /// The closure is passed the name of the parent object and can optionally use that in its - /// naming logic. Note, a number representing the type id of the current subobject may be - /// appended to the parent object name. + /// The closure is passed the file path and the name of the parent object. The generated name + /// should be joined to the provided file path. The parent object name can optionally use that + /// in its naming logic. Note, a number representing the type id of the current subobject may + /// be appended to the parent object name. pub fn new_with_custom_namer( file_path: TypePath<&'a str>, object_name: TypePathElem<&'a str>, - namer: &'a mut impl FnMut(TypePathElem<&str>) -> TypePathElem, + namer: &'a mut impl FnMut(TypePath<&str>, TypePathElem<&str>) -> ObjectPath, ) -> Self { Self { file_path, @@ -453,16 +452,16 @@ impl<'a> AdditionalUnspannedObjects<'a> { /// Add an additional object, and get a reference to it. pub fn add_object(&mut self, val: UnspannedVal) -> Value { - let name = self.name_allocs.allocate_name(self.object_name); - let res = Value::Ref(self.file_path.join(&name)); - self.objects.push((name, val)); + let path = self + .name_allocs + .allocate_path(self.file_path, self.object_name); + let res = Value::Ref(path.clone()); + self.objects.push((path, val)); res } /// Get the additional objects. - pub fn into_objects( - self, - ) -> impl ExactSizeIterator + use<> { + pub fn into_objects(self) -> impl ExactSizeIterator + use<> { self.objects.into_iter() } } @@ -477,7 +476,7 @@ where fn get_variant(ident: &Self::Variant, symbols: &Symbols) -> Result; - fn get_asset(ident: &Self::Ref, symbols: &Symbols) -> Result; + fn get_asset(ident: &Self::Ref, symbols: &Symbols) -> Result; fn convert_inner<'a, C: ConvertValue>( value: Spanned<&Value>, @@ -1235,7 +1234,7 @@ impl ConvertValueInner for UnspannedVal { Ok(ident.clone()) } - fn get_asset(asset: &Self::Ref, _symbols: &Symbols) -> Result { + fn get_asset(asset: &Self::Ref, _symbols: &Symbols) -> Result { Ok(asset.clone()) } @@ -1272,7 +1271,7 @@ impl ConvertValueInner for Val { Ok(ident.value.clone()) } - fn get_asset(asset: &Self::Ref, _symbols: &Symbols) -> Result { + fn get_asset(asset: &Self::Ref, _symbols: &Symbols) -> Result { Ok(asset.clone()) } @@ -1372,7 +1371,7 @@ impl ConvertValueInner for ParseVal { .to_owned()) } - fn get_asset(asset: &Self::Ref, symbols: &Symbols) -> Result { + fn get_asset(asset: &Self::Ref, symbols: &Symbols) -> Result { symbols.resolve_asset(asset).map(|(_, p)| p) } diff --git a/bauble/src/value/display.rs b/bauble/src/value/display.rs index 39528c7..670199e 100644 --- a/bauble/src/value/display.rs +++ b/bauble/src/value/display.rs @@ -8,11 +8,13 @@ use std::{borrow::Borrow, collections::HashMap}; use crate::{ Spanned, parse::{ParseVal, ParseValues, PathTreeEnd, PathTreeNode, allowed_in_raw_literal}, - path::TypePath, - types::{TypeKind, TypeRegistry}, + types::{TypeKind, TypeRegistry, path::TypePath}, }; -use super::{Attributes, FieldsKind, Object, UnspannedVal, Val, Value, ValueContainer, ValueTrait}; +use super::{ + Attributes, FieldsKind, Object, ObjectPath, UnspannedVal, Val, Value, ValueContainer, + ValueTrait, +}; /// Config to be used when formatting bauble. pub struct DisplayConfig { /// String inserted for tabs. @@ -69,6 +71,7 @@ mod formatter { pub fn is_typed(&self) -> bool { self.0.is_typed() } + pub fn write(&mut self, s: &str) { self.0.string.push_str(s) } @@ -253,21 +256,25 @@ fn slice_display>(slice: &[T], mut w: LineWriter, V: ValueTrait> IndentedDisplay for Value where - V::Ref: std::fmt::Display, + V::Ref: std::fmt::Debug, V::Variant: std::fmt::Display, V::Field: std::fmt::Display, V::Inner: IndentedDisplay + ValueContainer, { fn indented_display(&self, mut w: LineWriter) { match self { - Value::Ref(r) => { - if let Some(inline) = w.ctx().inline(r) { - inline.indented_display(w); - } else { + Value::Ref(r) => match w.ctx().display_ref(r) { + Some(DisplayRef::Inline(inline)) => inline.indented_display(w), + Some(DisplayRef::Path(path)) => { w.write("$"); - w.fmt(r); + w.fmt(path); } - } + None => { + // TODO: this is an error (empty path or missing inline object), but + // IndentedDisplay has no path for reporting errors. + w.write("ERROR {r:?} is empty or points to missing inline object"); + } + }, Value::Tuple(items) => { w.write("("); slice_display(items, w.reborrow()); @@ -575,13 +582,8 @@ impl> IndentedDisplay for ParseVal { impl, V: IndentedDisplay + ValueTrait> IndentedDisplay for Object { fn indented_display(&self, mut w: LineWriter) { - let ident = if self.top_level { - crate::value::BindingIdent::TOP_LEVEL_IDENTIFIER - } else { - let Some((_, end)) = self.object_path.get_end() else { - return; - }; - end.into_str() + let Some(ident) = self.object_path.ident() else { + return; }; w.write(ident); @@ -626,15 +628,32 @@ struct ValueDisplayCtx<'a, V: ValueTrait> { types: &'a TypeRegistry, } +enum DisplayRef<'a, R: ?Sized, P: ?Sized> { + Inline(&'a R), + Path(&'a P), +} + trait ValueCtx { - fn inline(&self, path: &V::Ref) -> Option<&V::Inner>; + type RefPath: std::fmt::Display + ?Sized; + + /// Returns `None` if the referenced path is an empty local path or if it refers to an inline + /// object that can't be found. + fn display_ref<'a>( + &'a self, + path: &'a V::Ref, + ) -> Option>; fn type_registry(&self) -> Option<&TypeRegistry>; } impl ValueCtx for () { - fn inline(&self, _path: &crate::parse::Path) -> Option<&ParseVal> { - None + type RefPath = crate::parse::Path; + + fn display_ref<'a>( + &'a self, + path: &'a crate::parse::Path, + ) -> Option> { + Some(DisplayRef::Path(path)) } fn type_registry(&self) -> Option<&TypeRegistry> { @@ -643,8 +662,18 @@ impl ValueCtx for () { } impl ValueCtx for ValueDisplayCtx<'_, Val> { - fn inline(&self, path: &TypePath) -> Option<&Val> { - self.inlined_refs.get(path.as_str()).copied() + type RefPath = str; + + fn display_ref<'a>(&'a self, path: &'a ObjectPath) -> Option> { + Some(match path { + ObjectPath::Top(path) => DisplayRef::Path(path.as_str()), + // Only use identifier for local object, full path is never necessary since it can only + // be referenced from the same file. + ObjectPath::Local(path) => DisplayRef::Path(path.get_end()?.1.into_str()), + ObjectPath::Inline(path) => { + DisplayRef::Inline(self.inlined_refs.get(&path.borrow()).copied()?) + } + }) } fn type_registry(&self) -> Option<&TypeRegistry> { @@ -653,8 +682,21 @@ impl ValueCtx for ValueDisplayCtx<'_, Val> { } impl ValueCtx for ValueDisplayCtx<'_, UnspannedVal> { - fn inline(&self, path: &TypePath) -> Option<&UnspannedVal> { - self.inlined_refs.get(path.as_str()).copied() + type RefPath = str; + + fn display_ref<'a>( + &'a self, + path: &'a ObjectPath, + ) -> Option> { + Some(match path { + ObjectPath::Top(path) => DisplayRef::Path(path.as_str()), + // Only use identifier for local object, full path is never necessary since it can only + // be referenced from the same file. + ObjectPath::Local(path) => DisplayRef::Path(path.get_end()?.1.into_str()), + ObjectPath::Inline(path) => { + DisplayRef::Inline(self.inlined_refs.get(&path.borrow()).copied()?) + } + }) } fn type_registry(&self) -> Option<&TypeRegistry> { @@ -673,8 +715,8 @@ where let mut inlined_refs = HashMap::new(); let objects = w.ctx().1; for object in objects.iter() { - if object.object_path.is_subobject() { - inlined_refs.insert(object.object_path.borrow(), &object.value); + if let ObjectPath::Inline(path) = &object.object_path { + inlined_refs.insert(path.borrow(), &object.value); } } let ctx = ValueDisplayCtx::<'_, V> { @@ -695,10 +737,10 @@ where let mut inlined_refs = HashMap::new(); let mut written = Vec::new(); for object in self.iter() { - if !object.object_path.is_subobject() { - written.push(object); + if let ObjectPath::Inline(path) = &object.object_path { + inlined_refs.insert(path.borrow(), &object.value); } else { - inlined_refs.insert(object.object_path.borrow(), &object.value); + written.push(object); } } let ctx = ValueDisplayCtx { diff --git a/bauble/src/value/early_context.rs b/bauble/src/value/early_context.rs index f392200..2eb4ab6 100644 --- a/bauble/src/value/early_context.rs +++ b/bauble/src/value/early_context.rs @@ -1,39 +1,20 @@ -use crate::context::{AssetKind, BaubleContext, PathReference}; +use crate::context::{BaubleContext, PathReference}; use crate::path::{TypePath, TypePathElem}; use crate::types::TypeId; use crate::value::AmbiguousWithIdent; use indexmap::IndexMap; -fn try_reduce_option( - a: Option, - b: Option, - f: impl FnOnce(T, T) -> Result, -) -> Result, ()> { - match (a, b) { - (Some(a), Some(b)) => f(a, b).map(Some), - (Some(t), None) | (None, Some(t)) => Ok(Some(t)), - (None, None) => Ok(None), - } -} - -/// - Returns `None` if both are `Some`. -/// - Returns `Some(None)` if both are `None`. -/// - Returns `Some(Some(_))` when only one of the provided options is `Some`. -fn xor_option(a: Option, b: Option) -> Option> { - try_reduce_option(a, b, |_, _| Err(())).ok() -} - /// A type containing multiple references generally derived from a path. /// /// Generalization of both [`PathReference`] and [`EarlyPathReference`]. /// /// Unlike [`PathReference`] the type for an asset may not yet be known. #[derive(Default, Clone, Debug)] -pub(crate) struct CombinedPathReference { +pub(super) struct CombinedPathReference { /// The type referenced by a path. pub ty: Option, /// The asset (and its path) referenced by the path. - pub asset: Option<(Option, TypePath, AssetKind)>, + pub asset: Option<(Option, TypePath)>, /// If the reference references a module. pub module: Option, } @@ -58,9 +39,8 @@ impl CombinedPathReference { module: other_module, } = other; - if let (Some((other_path, other_kind)), Some((_this_ty, this_path, this_kind))) = - (&other_asset, &self.asset) - && (this_path, this_kind) != (other_path, other_kind) + if let (Some(other_path), Some((_this_ty, this_path))) = (&other_asset, &self.asset) + && this_path != other_path { return Err(()); } @@ -71,7 +51,7 @@ impl CombinedPathReference { } // Only mutate after checking that no conflicts exist. if self.asset.is_none() { - self.asset = other_asset.map(|(other_path, other_kind)| (None, other_path, other_kind)); + self.asset = other_asset.map(|other_path| (None, other_path)); } if self.module.is_none() { self.module = other_module; @@ -79,31 +59,6 @@ impl CombinedPathReference { Ok(()) } - - /// Take the exclusive properties of `self` and `other`, essentially "xor"ing them together by producing - /// the combined result where each field where both are `Some` are `None`. - /// - /// Returns `None` if there is a collision (i.e. matching fields are `Some`). - pub(super) fn combined(self, other: Self) -> Option { - Some(Self { - ty: xor_option(self.ty, other.ty)?, - asset: xor_option(self.asset, other.asset)?, - module: xor_option(self.module, other.module)?, - }) - } - - /// Overrides references of `self` with references of `other`. - pub(super) fn combine_override(&mut self, other: Self) { - if other.ty.is_some() { - self.ty = other.ty; - } - if other.asset.is_some() { - self.asset = other.asset; - } - if other.module.is_some() { - self.module = other.module; - } - } } /// A type containing multiple references generally derived from a path. @@ -116,7 +71,7 @@ impl CombinedPathReference { #[derive(Default, Clone, Debug)] struct EarlyPathReference { /// The asset (and its path) referenced by the path. - pub asset: Option<(TypePath, AssetKind)>, + pub asset: Option, /// If the reference references a module. pub module: Option, } @@ -146,7 +101,7 @@ impl From for CombinedPathReference { fn from(reference: EarlyPathReference) -> Self { Self { ty: None, - asset: reference.asset.map(|(path, kind)| (None, path, kind)), + asset: reference.asset.map(|path| (None, path)), module: reference.module, } } @@ -156,9 +111,7 @@ impl From for CombinedPathReference { fn from(reference: PathReference) -> Self { Self { ty: reference.ty, - asset: reference - .asset - .map(|(ty, path, kind)| (Some(ty), path, kind)), + asset: reference.asset.map(|(ty, path)| (Some(ty), path)), module: reference.module, } } @@ -173,11 +126,10 @@ struct CtxNode { /// /// This is the path to reach this node from the root. path: TypePath, - /// This name can potentially reference: + /// The path to this node can potentially reference: /// * An asset /// * A module (not represented here but via the `Self::children` field). - // TODO: stage 2: only hold top level assets here, local assets should not need to exist here - asset: Option, + asset: bool, children: IndexMap, } @@ -185,14 +137,14 @@ impl CtxNode { fn new(path: TypePath) -> Self { Self { path, - asset: None, + asset: false, children: IndexMap::<_, _>::default(), } } fn reference(&self) -> EarlyPathReference { EarlyPathReference { - asset: self.asset.map(|kind| (self.path.clone(), kind)), + asset: self.asset.then(|| self.path.clone()), module: (!self.children.is_empty()).then(|| self.path.clone()), } } @@ -250,14 +202,13 @@ impl CtxNode { .or_insert_with(|| CtxNode::new(self.path.join(&child))) } - fn build_asset(&mut self, path: TypePath<&str>, kind: AssetKind) -> Result<(), ()> { + fn build_asset(&mut self, path: TypePath<&str>) -> Result<(), ()> { let node = self.build_nodes(path); - if node.asset.is_some() { + if node.asset { // Multiple assets with the same path return Err(()); } - - node.asset = Some(kind); + node.asset = true; Ok(()) } } @@ -284,50 +235,36 @@ impl<'a> EarlyContext<'a> { Self { ctx, root_node } } - /// Registers an asset. This is done automatically for any objects in a file that gets registered. - /// - /// With this method you can expose assets that aren't in bauble. + /// Pre-registers an asset before its type is known. /// - /// Returns ID of internal Ref type for `ty`. + /// This allows `use` based paths to the asset to be resolved properly + /// before the types are fully resolved. /// - /// Returns an error if an asset was already registered at this path. This can occur due to - /// simplification of the top level asset path to match the current file. E.g. the top-level - /// asset in `a::1` will conflict with the path of object `1` in file `a`. - // - // TODO: in stage 2 we might be able to adjust this so that local object paths are - // distinguished from top level objects, such that they don't clash. - pub fn register_asset( - &mut self, - path: TypePath<&str>, - kind: AssetKind, - ) -> Result<(), crate::CustomError> { + /// # Panics + /// Panics if an asset was already registered at this path. + pub(super) fn register_asset(&mut self, path: TypePath<&str>) { + // An error should produced during parsing when identifiers overlap, and it isn't + // possible to have overlapping paths from different files. Caller expected to clear + // old objects from the context when reloading a file. + // // Make sure asset doesn't already exist in `BaubleContext`. - if self.ctx.get_ref(path).is_some_and(|r| r.asset.is_some()) { - // TODO: this error should no longer be possible? - return Err(crate::CustomError::new(format!( - "'{path}' refers to an existing asset in another file. This can be \n\ - caused by special cased path simplification for the first object in a \n\ - file.", - ))); - } + assert!( + self.ctx.get_ref(path).is_none_or(|r| r.asset.is_none()), + "Multiple assets with the same path" + ); self.root_node - .build_asset(path, kind) - // TODO: this error should no longer be possible? - .map_err(|()| { - crate::CustomError::new(format!( - "'{path}' refers to an existing asset in another file. This can be \n\ - caused by special cased path simplification for the first object in a \n\ - file.", - )) - }) + .build_asset(path) + // An error should produced during parsing when identifiers overlap, and it isn't + // possible to have overlapping paths from different files. + .expect("Multiple assets with the same path"); } /// Takes a path in bauble, and if the path is valid, return meta information about the /// bauble item(s) at that path. /// /// Looks up from both pending items and items already registered in [`BaubleContext`]. - pub fn get_ref(&self, path: TypePath<&str>) -> Option { + pub(super) fn get_ref(&self, path: TypePath<&str>) -> Option { let a = self.ctx.get_ref(path).map(CombinedPathReference::from); let b = self.root_node.node_at(path).map(|node| node.reference()); @@ -350,7 +287,7 @@ impl<'a> EarlyContext<'a> { /// namespace. /// /// Looks up from both pending items and items already registered in [`BaubleContext`]. - pub fn ref_with_ident( + pub(super) fn ref_with_ident( &self, path: TypePath<&str>, ident: TypePathElem<&str>, @@ -399,7 +336,7 @@ impl<'a> EarlyContext<'a> { /// /// Note, the order of returned items won't match after these pending items are registered into /// [`BaubleContext`]. - pub fn all_in( + pub(super) fn all_in( &self, path: TypePath<&str>, ) -> Option> { diff --git a/bauble/src/value/error.rs b/bauble/src/value/error.rs index 8a25205..95f881a 100644 --- a/bauble/src/value/error.rs +++ b/bauble/src/value/error.rs @@ -6,16 +6,22 @@ use crate::{ path::{TypePath, TypePathElem}, spanned::{SpanExt, Spanned}, types::{self, TypeId}, - value::early_context::CombinedPathReference, }; use super::{Ident, PathKind}; +#[derive(Clone, Debug)] +pub struct ErrorPathReference { + pub(super) ty: bool, + pub(super) asset: bool, + pub(super) module: bool, +} + #[derive(Clone, Debug)] pub struct RefError { pub(super) uses: Option>, pub(super) path: PathKind, - pub(super) path_ref: CombinedPathReference, + pub(super) path_ref: Option, pub(super) kind: RefKind, } @@ -541,20 +547,20 @@ impl BaubleError for Spanned { return errors; } ConversionError::RefError(ref_err) => { - let inner = match ( - &ref_err.path_ref.module, - &ref_err.path_ref.asset, - &ref_err.path_ref.ty, - ) { - (None, None, None) => "", - (None, None, Some(_)) => ", but it refers to a type", - (None, Some(_), None) => ", but it refers to an asset", - (None, Some(_), Some(_)) => ", but it refers to an asset and a type", - (Some(_), None, None) => ", but it refers to a module", - (Some(_), None, Some(_)) => ", but it refers to a module and a type", - (Some(_), Some(_), None) => ", but it refers to a module and an asset", - (Some(_), Some(_), Some(_)) => "", - }; + let inner = + ref_err + .path_ref + .as_ref() + .map_or("", |r| match (r.module, r.asset, r.ty) { + (false, false, false) => "", + (false, false, true) => ", but it refers to a type", + (false, true, false) => ", but it refers to an asset", + (false, true, true) => ", but it refers to an asset and a type", + (true, false, false) => ", but it refers to a module", + (true, false, true) => ", but it refers to a module and a type", + (true, true, false) => ", but it refers to a module and an asset", + (true, true, true) => "", + }); let mut errs = vec![( Spanned::new( self.span, diff --git a/bauble/src/value/mod.rs b/bauble/src/value/mod.rs index 0560e80..4e6aaab 100644 --- a/bauble/src/value/mod.rs +++ b/bauble/src/value/mod.rs @@ -11,7 +11,7 @@ use rust_decimal::Decimal; use crate::{ BaubleErrors, FileId, VariantKind, - context::PathReference, + object_path::ObjectPath, parse::{BindingIdent, ParseVal, ParseValues, Path, PathEnd}, path::{TypePath, TypePathElem}, spanned::{SpanExt, Spanned}, @@ -22,7 +22,6 @@ pub use convert::AdditionalUnspannedObjects; pub(crate) use convert::AnyVal; use convert::{AdditionalObjects, ConvertMeta, ConvertValue}; pub use display::{DisplayConfig, IndentedDisplay, display_formatted}; -use early_context::CombinedPathReference; pub(crate) use early_context::EarlyContext; use error::Result; pub use error::{AmbiguousWithIdent, ConversionError, RefError, RefKind}; @@ -228,7 +227,7 @@ pub struct Val { impl ValueTrait for Val { type Inner = Self; - type Ref = TypePath; + type Ref = ObjectPath; type Variant = Spanned; @@ -328,7 +327,7 @@ pub struct UnspannedVal { impl ValueTrait for UnspannedVal { type Inner = Self; - type Ref = TypePath; + type Ref = ObjectPath; type Variant = TypePathElem; @@ -524,11 +523,7 @@ impl Value { #[derive(Debug, Clone, PartialEq)] pub struct Object { /// Path that refers to this object. - pub object_path: TypePath, - // TODO: update doc in stage 2 - /// `true` when this object appears at the top of a file and its name matches the file. The - /// object's path will be reduced to just the path of the file. - pub top_level: bool, + pub object_path: ObjectPath, pub value: Inner, } @@ -537,7 +532,6 @@ impl Object { pub fn into_unspanned(self) -> Object { Object { object_path: self.object_path, - top_level: self.top_level, value: self.value.into_unspanned(), } } @@ -559,23 +553,14 @@ impl std::fmt::Display for PathKind { } } -#[derive(Clone, Debug)] -pub enum ObjectPath { - /// Top-level object or external asset. There is at most one per file. - Top(TypePath), - /// Local object. Can only be referenced from other objects in the same file. - Local(TypePath), -} - /// We can delay registering `Ref` assets if what they're referencing hasn't been loaded yet. /// /// What they are referencing needs to be loaded in order to determine their type. #[derive(Debug)] pub(crate) struct DelayedRegister { - asset: Spanned, - asset_kind: crate::AssetKind, + path: Spanned, /// Full path to the referenced asset. - reference: TypePath, + reference: ObjectPath, /// Unresolved path to the referenced asset. reference_original: Spanned, /// The type we want a potential reference to resolve into. @@ -585,6 +570,7 @@ pub(crate) struct DelayedRegister { pub(crate) fn resolve_delayed( mut delayed: Vec, ctx: &mut crate::context::BaubleContext, + local_ctx: &mut crate::local_context::LocalContext, ) -> std::result::Result<(), Vec>> { loop { let mut errors = Vec::new(); @@ -592,9 +578,15 @@ pub(crate) fn resolve_delayed( // Try to register delayed registers, and remove them as they succeed. delayed.retain(|d| { - if let Some(r) = ctx.get_ref(d.reference.borrow()) - && let Some((ty, _, _)) = &r.asset - { + let ty = match &d.reference { + ObjectPath::Top(path) => ctx + .get_ref(path.borrow()) + .and_then(|r| r.asset) + .map(|(ty, _)| ty), + ObjectPath::Local(path) => local_ctx.get(path.borrow()), + ObjectPath::Inline(_) => unreachable!(), + }; + if let Some(ty) = ty { // TODO: for now, it is assumed all references which explicitly // specify their inner type should have that inner type resolved // by this point. If that is not the case, this should be a @@ -639,19 +631,25 @@ pub(crate) fn resolve_delayed( return false; }; - if desired_ty != *ty { + if desired_ty != ty { errors.push( ConversionError::ExpectedExactType { expected: desired_ty, - got: Some(*ty), + got: Some(ty), } .spanned(span), ); } } - if let Err(e) = ctx.register_asset(d.asset.value.borrow(), *ty, d.asset_kind) { - errors.push(ConversionError::Custom(e).spanned(d.asset.span)) + + match &*d.path { + ObjectPath::Top(path) => ctx.register_asset(path.borrow(), ty), + ObjectPath::Local(path) => local_ctx.register(path.clone(), ty, ctx), + ObjectPath::Inline(_) => { + unreachable!("can't be returned from object_ident_path") + } } + false } else { true @@ -671,12 +669,12 @@ pub(crate) fn resolve_delayed( let mut graph = petgraph::graphmap::DiGraphMap::new(); let mut map = HashMap::new(); for a in delayed.iter() { - let node_a = graph.add_node(a.asset.as_ref().map(|p| p.borrow())); + let node_a = graph.add_node(a.path.as_ref().map(|p| p.borrow())); map.insert(node_a, (&a.reference, &a.reference_original)); for b in delayed.iter() { - if a.reference == *b.asset { - graph.add_edge(node_a, b.asset.as_ref().map(|p| p.borrow()), ()); + if a.reference == *b.path { + graph.add_edge(node_a, b.path.as_ref().map(|p| p.borrow()), ()); } } } @@ -689,8 +687,8 @@ pub(crate) fn resolve_delayed( // Ref refers to itself errors.push( ConversionError::Cycle(vec![( - referer.map(|r| r.to_string()), - vec![referenced_original.as_ref().map(|r| r.to_string())], + referer.map(|r| format!("{r:?}")), + vec![referenced_original.as_ref().map(|r| format!("{r:?}"))], )]) .spanned(referer.span), ) @@ -711,8 +709,8 @@ pub(crate) fn resolve_delayed( .iter() .map(|s| { ( - s.to_string().spanned(s.span), - vec![map[s].1.as_ref().map(|r| r.to_string())], + format!("{s:?}").spanned(s.span), + vec![map[s].1.as_ref().map(|r| format!("{r:?}"))], ) }) .collect(); @@ -736,17 +734,17 @@ pub(crate) fn resolve_delayed( fn object_ident_path<'a>( file_path: TypePath<&str>, binding_ident: &'a BindingIdent, -) -> (TypePathElem<&'a str>, TypePath) { +) -> (TypePathElem<&'a str>, ObjectPath) { let ident = TypePathElem::new(binding_ident.as_str()).expect("Invariant"); let path = match binding_ident { - BindingIdent::TopLevel(_) => file_path.to_owned(), - BindingIdent::Local(_) => file_path.join(&ident), + BindingIdent::TopLevel(_) => ObjectPath::Top(file_path.to_owned()), + BindingIdent::Local(_) => ObjectPath::Local(file_path.join(&ident)), }; (ident, path) } -/// Registers all new asset paths into [`EarlyContext`] so they will be known for resolving full -/// paths from `use`s in [`register_assets`]. +/// Registers all new top level asset paths into [`EarlyContext`] so they will be known for +/// resolving full paths from `use`s in [`register_assets`]. /// /// We need to know what items brought into scope with `use` are assets rather than types or /// modules to properly dertermine the full path (otherwise there could be multiple candidates @@ -755,40 +753,31 @@ pub(crate) fn pre_register_assets( ctx: &mut EarlyContext<'_>, file_path: TypePath<&str>, values: &ParseValues, -) -> std::result::Result<(), Vec>> { - let mut errors = Vec::new(); - +) { for ident in values.values.keys() { - let span = ident.span(); - let kind = if ident.is_top_level() { - crate::AssetKind::TopLevel - } else { - crate::AssetKind::Local - }; let (_ident, path) = object_ident_path(file_path, ident); - match ctx.register_asset(path.borrow(), kind) { - Ok(()) => {} - Err(e) => errors.push(ConversionError::Custom(e).spanned(span)), + match path { + ObjectPath::Top(path) => ctx.register_asset(path.borrow()), + // These don't need to be pre-registered, because we can add their full path directly + // to Symbols, and they aren't needed to resolve `use` based paths since they can only + // be referenced from the same file.. + ObjectPath::Local(_) => {} + ObjectPath::Inline(_) => unreachable!("can't be returned from object_ident_path"), } } - - if errors.is_empty() { - Ok(()) - } else { - Err(errors) - } } pub(crate) fn register_assets( ctx: &mut EarlyContext<'_>, + local_ctx: &mut crate::local_context::LocalContext, file_path: TypePath<&str>, values: &ParseValues, ) -> std::result::Result, Vec>> { let mut errors = Vec::new(); let mut delayed = Vec::new(); - let mut symbols = EarlySymbols::new(ctx); + let mut symbols = EarlySymbols::new(ctx, local_ctx); // Add `uses` to local symbols instance. for use_path in &values.uses { if let Err(e) = symbols.add_use(use_path) { @@ -801,23 +790,25 @@ pub(crate) fn register_assets( let span = ident.span(); let (ident, path) = object_ident_path(file_path, ident); - if let Some(CombinedPathReference { - asset: Some(asset), .. - }) = symbols.ctx.get_ref(path.borrow()) - { - if let Err(e) = symbols.add_ref( - ident.to_owned(), - CombinedPathReference { - ty: None, - asset: Some(asset), - module: None, - }, - ) { - errors.push(e.spanned(span)); + // Note, we don't need to lookup these in contexts because we know the full paths and that + // the types are not known for any of them. + let path = match path { + ObjectPath::Top(path) => { + debug_assert!( + symbols + .ctx + .get_ref(path.borrow()) + .is_some_and(|r| r.asset.is_some_and(|(ty, p)| ty.is_none() && p == path)) + ); + path } - } else { - // Didn't pre-register assets. - errors.push(ConversionError::UnregisteredAsset.spanned(span)); + ObjectPath::Local(path) => path, + ObjectPath::Inline(_) => unreachable!("can't be returned from object_ident_path"), + }; + let ty = None; + + if let Err(e) = symbols.add_local_object(ident.to_owned(), ty, path) { + errors.push(e.spanned(span)); } } @@ -828,11 +819,6 @@ pub(crate) fn register_assets( // `resolve_delayed`. for (ident, binding) in &values.values { let span = ident.span(); - let kind = if ident.is_top_level() { - crate::AssetKind::TopLevel - } else { - crate::AssetKind::Local - }; let (_ident, path) = object_ident_path(file_path, ident); // To register an asset we need to determine its type. @@ -898,8 +884,7 @@ pub(crate) fn register_assets( }; delayed.push(DelayedRegister { - asset: path.spanned(span), - asset_kind: kind, + path: path.spanned(span), reference, reference_original: ref_path.clone().spanned(binding.value.span()), expected_ty_path, @@ -924,13 +909,14 @@ pub(crate) fn register_assets( } }; - if let Err(e) = ty.and_then(|ty| { - symbols - .bauble_ctx() - .register_asset(path.borrow(), ty, kind) - .map_err(|e| ConversionError::Custom(e).spanned(span)) - }) { - errors.push(e); + let (ctx, local_ctx) = symbols.ctx_for_register(); + match ty { + Ok(ty) => match path { + ObjectPath::Top(path) => ctx.register_asset(path.borrow(), ty), + ObjectPath::Local(path) => local_ctx.register(path.clone(), ty, ctx), + ObjectPath::Inline(_) => unreachable!("can't be returned from object_ident_path"), + }, + Err(e) => errors.push(e), } } @@ -945,6 +931,7 @@ pub(crate) fn convert_values( file: FileId, values: ParseValues, ctx: &crate::context::BaubleContext, + local_ctx: &crate::local_context::LocalContext, ) -> std::result::Result, BaubleErrors> { let mut errors = Vec::new(); @@ -962,23 +949,20 @@ pub(crate) fn convert_values( let span = ident.span(); let (ident, path) = object_ident_path(file_path, ident); - if let Some(PathReference { - asset: Some(asset), .. - }) = symbols.ctx.get_ref(path.borrow()) - { - if let Err(e) = symbols.add_ref( - ident.to_owned(), - PathReference { - ty: None, - asset: Some(asset), - module: None, - }, - ) { - errors.push(e.spanned(span)); - } - } else { - // Didn't pre-register assets. + let asset = match path { + ObjectPath::Top(path) => symbols.ctx.get_ref(path.borrow()).and_then(|r| r.asset), + ObjectPath::Local(path) => local_ctx.get(path.borrow()).map(|ty| (ty, path.clone())), + ObjectPath::Inline(_) => unreachable!("can't be returned from object_ident_path"), + }; + + let Some((ty, path)) = asset else { + // Didn't register assets. errors.push(ConversionError::UnregisteredAsset.spanned(span)); + continue; + }; + + if let Err(e) = symbols.add_local_object(ident.to_owned(), ty, path) { + errors.push(e.spanned(span)); } } @@ -1008,7 +992,6 @@ pub(crate) fn convert_values( _ => unreachable!("The type registered with an object is always a reference"), }; - let top_level = ident.is_top_level(); let (ident, path) = object_ident_path(file_path, ident); let convert_meta = ConvertMeta { @@ -1017,7 +1000,7 @@ pub(crate) fn convert_values( object_name: ident, default_span, }; - match convert_object(path, top_level, &binding.value, ty, convert_meta) { + match convert_object(path, &binding.value, ty, convert_meta) { Ok(obj) => objects.push(obj), Err(e) => errors.push(e), } @@ -1036,29 +1019,23 @@ pub(crate) fn convert_values( /// Converts a parsed value to a object value using a conversion context and existing symbols. Also /// does some rudimentary checking if the symbols are okay. fn convert_object( - object_path: TypePath, - top_level: bool, + object_path: ObjectPath, value: &ParseVal, expected_type: TypeId, mut meta: ConvertMeta, ) -> Result { let value = value.convert(meta.reborrow(), expected_type, convert::no_attr())?; let types = meta.symbols.ctx.type_registry(); - create_object(object_path, top_level, value, types) + create_object(object_path, value, types) } fn create_object( - object_path: TypePath, - top_level: bool, + object_path: ObjectPath, value: Val, type_registry: &TypeRegistry, ) -> Result { if type_registry.impls_top_level_trait(*value.ty) { - Ok(Object { - object_path, - top_level, - value, - }) + Ok(Object { object_path, value }) } else { Err(ConversionError::MissingRequiredTrait { tr: type_registry.top_level_trait(), @@ -1077,8 +1054,8 @@ fn create_object( fn compare_objects( original: &UnspannedVal, loaded: &UnspannedVal, - orig_map: &HashMap, - loaded_map: &HashMap, + orig_map: &HashMap, + loaded_map: &HashMap, ) -> std::result::Result<(), (UnspannedVal, UnspannedVal)> { let inquality_err = || (original.clone(), loaded.clone()); @@ -1092,11 +1069,13 @@ fn compare_objects( match (&original.value, &loaded.value) { (crate::Value::Ref(a), crate::Value::Ref(b)) => { - // Compare the sub assets rather than the paths to them. + // Compare the inline object rather than the paths to them. // - // Note, this means object comparison will pass even when the paths to sub - // assets change. - if a.is_subobject() { + // Note, this means object comparison will pass even when + // the paths to inline objects change. + if let ObjectPath::Inline(_) = a + && let ObjectPath::Inline(_) = b + { let a = orig_map.get(a).unwrap(); let (_, b) = loaded_map.get(b).unwrap(); compare_objects(a, b, orig_map, loaded_map) @@ -1180,11 +1159,11 @@ fn compare_objects( /// Error returned by [`compare_object_sets`]. pub struct CompareObjectsError { /// Objects with the same path but non-equal content. - pub mismatched: Vec<(TypePath, crate::Span, UnspannedVal, UnspannedVal)>, + pub mismatched: Vec<(ObjectPath, crate::Span, UnspannedVal, UnspannedVal)>, /// Objects from the original set that are missing in the new set. - pub missing: Vec<(TypePath, UnspannedVal)>, + pub missing: Vec<(ObjectPath, UnspannedVal)>, /// Objects only found in the new set. - pub new: Vec<(TypePath, UnspannedVal)>, + pub new: Vec<(ObjectPath, UnspannedVal)>, } /// Compares two sets of objects and returns an error with the list of mismatched, missing, and new @@ -1213,8 +1192,11 @@ pub fn compare_object_sets( let mut mismatched = Vec::new(); for (k, a) in original_object_map.iter() { - // Don't compare sub-assets, they will be compared by recursion in `compare_objects` - if !k.is_subobject() { + // Don't compare inline objects, they will be compared by recursion in `compare_objects` + // + // Note, this means unreferenced inline objects won't be considered (but those should not + // be possible). + if !matches!(k, ObjectPath::Inline(_)) { if let Some((span, b)) = loaded_object_map.get(k) { if let Err((original, new)) = compare_objects(a, b, &original_object_map, &loaded_object_map) @@ -1229,9 +1211,11 @@ pub fn compare_object_sets( let new = loaded_object_map .into_iter() - // `compare_objects` handles checking for sub-asset so we don't produce - // an error if their paths change. - .filter(|(k, _)| !original_object_map.contains_key(k) && !k.is_subobject()) + // `compare_objects` handles checking for inline object equality and we specifically don't + // produce an error if their paths change. + .filter(|(k, _)| { + !original_object_map.contains_key(k) && !matches!(k, ObjectPath::Inline(_)) + }) .map(|(k, (_span, b))| (k, b)) .collect::>(); diff --git a/bauble/src/value/symbols.rs b/bauble/src/value/symbols.rs index e043e71..0e59dee 100644 --- a/bauble/src/value/symbols.rs +++ b/bauble/src/value/symbols.rs @@ -3,6 +3,7 @@ use std::{borrow::Cow, collections::HashMap}; use crate::{ BaubleContext, CustomError, context::PathReference, + local_context::LocalContext, parse::{Path, PathEnd, PathTreeEnd, PathTreeNode}, path::{TypePath, TypePathElem}, spanned::{SpanExt, Spanned}, @@ -10,7 +11,10 @@ use crate::{ }; use super::early_context::CombinedPathReference; -use super::{ConversionError, EarlyContext, PathKind, RefError, RefKind, Result}; +use super::{ + ConversionError, EarlyContext, ObjectPath, PathKind, RefError, RefKind, Result, + error::ErrorPathReference, +}; /// Indicates kind of item being resolved by [`Symbols::resolve_path`] or /// [`EarlySymbols::resolve_path`]. @@ -41,15 +45,125 @@ fn generic_param_using_with_ident(span: crate::Span) -> Spanned .spanned(span) } +/// `PathReference` for `Symbols` `uses`. Distinguishes between local and top level objects. +#[derive(Default, Clone)] +struct UseReference { + ty: Option, + asset: Option<(TypeId, ObjectPath)>, + module: Option, +} + +impl UseReference { + /// Take the exclusive properties of `self` and `other`, essentially "xor"ing them together by producing + /// the combined result where each field where both are `Some` are `None`. + pub fn combined(self, other: Self) -> Option { + Some(Self { + ty: xor_option(self.ty, other.ty)?, + asset: xor_option(self.asset, other.asset)?, + module: xor_option(self.module, other.module)?, + }) + } + + /// Overrides references of `self` with references of `other`. + pub fn combine_override(&mut self, other: Self) { + if other.ty.is_some() { + self.ty = other.ty; + } + if other.asset.is_some() { + self.asset = other.asset; + } + if other.module.is_some() { + self.module = other.module; + } + } +} + +/// `CombinedPathReference` for `EarlySymbols` `uses`. Distinguishes between local and top level objects. +#[derive(Default, Clone, Debug)] +struct EarlyUseReference { + ty: Option, + asset: Option<(Option, ObjectPath)>, + module: Option, +} + +impl EarlyUseReference { + /// Take the exclusive properties of `self` and `other`, essentially "xor"ing them together by producing + /// the combined result where each field where both are `Some` are `None`. + pub fn combined(self, other: Self) -> Option { + Some(Self { + ty: xor_option(self.ty, other.ty)?, + asset: xor_option(self.asset, other.asset)?, + module: xor_option(self.module, other.module)?, + }) + } + + /// Overrides references of `self` with references of `other`. + pub fn combine_override(&mut self, other: Self) { + if other.ty.is_some() { + self.ty = other.ty; + } + if other.asset.is_some() { + self.asset = other.asset; + } + if other.module.is_some() { + self.module = other.module; + } + } +} + +impl From for UseReference { + fn from(reference: PathReference) -> Self { + Self { + ty: reference.ty, + asset: reference + .asset + .map(|(ty, path)| (ty, ObjectPath::Top(path))), + module: reference.module, + } + } +} + +impl From for EarlyUseReference { + fn from(reference: CombinedPathReference) -> Self { + Self { + ty: reference.ty, + asset: reference + .asset + .map(|(ty, path)| (ty, ObjectPath::Top(path))), + module: reference.module, + } + } +} + +impl From for ErrorPathReference { + fn from(reference: UseReference) -> Self { + Self { + ty: reference.ty.is_some(), + asset: reference.asset.is_some(), + module: reference.module.is_some(), + } + } +} + +impl From for ErrorPathReference { + fn from(reference: EarlyUseReference) -> Self { + Self { + ty: reference.ty.is_some(), + asset: reference.asset.is_some(), + module: reference.module.is_some(), + } + } +} + /// Representation of item names available in the current module. /// /// There are multiple namespaces: types, assets (i.e. values defined in bauble), and modules. -#[derive(Clone)] +//#[derive(Clone)] pub(crate) struct Symbols<'a> { /// Context for looking up things referenced by full path. pub(super) ctx: &'a BaubleContext, /// Map of identifiers to path references. - pub(super) uses: HashMap, + uses: HashMap, } impl<'a> Symbols<'a> { @@ -60,21 +174,44 @@ impl<'a> Symbols<'a> { } } - pub fn add_ref( + fn add_ref( &mut self, ident: TypePathElem, - reference: PathReference, + reference: impl Into, ) -> std::result::Result<(), ConversionError> { let r = self.uses.entry(ident.clone()).or_default(); *r = r .clone() - .combined(reference) + .combined(reference.into()) .ok_or(ConversionError::AmbiguousUse { ident })?; Ok(()) } + /// Note, this can also add the top level object of the current file under its local + /// identifier. + pub fn add_local_object( + &mut self, + ident: TypePathElem, + ty: TypeId, + full_path: TypePath, + ) -> std::result::Result<(), ConversionError> { + let path = if ident.as_str() == crate::object_path::TOP_LEVEL_IDENTIFIER { + ObjectPath::Top(full_path) + } else { + ObjectPath::Local(full_path) + }; + self.add_ref( + ident, + UseReference { + ty: None, + asset: Some((ty, path)), + module: None, + }, + ) + } + pub fn add_use(&mut self, use_path: &Spanned) -> Result<()> { fn add_use_inner( this: &mut Symbols, @@ -91,7 +228,7 @@ impl<'a> Symbols<'a> { return Err(ConversionError::RefError(Box::new(RefError { uses: None, path: PathKind::Direct(leading), - path_ref: PathReference::empty().into(), + path_ref: None, kind: RefKind::Module, })) .spanned(s.span)); @@ -110,7 +247,7 @@ impl<'a> Symbols<'a> { return Err(ConversionError::RefError(Box::new(RefError { uses: None, path: PathKind::Direct(leading), - path_ref: PathReference::empty().into(), + path_ref: None, kind: RefKind::Module, })) .spanned(end.span)); @@ -127,7 +264,7 @@ impl<'a> Symbols<'a> { return Err(ConversionError::RefError(Box::new(RefError { uses: None, path: PathKind::Direct(path), - path_ref: PathReference::empty().into(), + path_ref: None, kind: RefKind::Any, })) .spanned(end.span)); @@ -147,7 +284,7 @@ impl<'a> Symbols<'a> { return Err(ConversionError::RefError(Box::new(RefError { uses: None, path: PathKind::Indirect(leading, path_end.to_owned()), - path_ref: PathReference::empty().into(), + path_ref: None, kind: RefKind::Any, })) .spanned(end.span)); @@ -171,7 +308,7 @@ impl<'a> Symbols<'a> { return Err(ConversionError::RefError(Box::new(RefError { uses: None, path: PathKind::Direct(leading), - path_ref: PathReference::empty().into(), + path_ref: None, kind: RefKind::Module, })) .spanned(l.span)); @@ -201,7 +338,7 @@ impl<'a> Symbols<'a> { return Err(ConversionError::RefError(Box::new(RefError { uses: None, path: PathKind::Direct(leading), - path_ref: PathReference::empty().into(), + path_ref: None, kind: RefKind::Module, })) .spanned(first.span)); @@ -216,7 +353,7 @@ impl<'a> Symbols<'a> { return Err(ConversionError::RefError(Box::new(RefError { uses: None, path: PathKind::Direct(leading), - path_ref: PathReference::empty().into(), + path_ref: None, kind: RefKind::Module, })) .spanned(ident.span)); @@ -313,13 +450,13 @@ impl<'a> Symbols<'a> { &self, raw_path: &Path, kind: ResolveKind, - ) -> Result<(Cow<'_, PathReference>, PathKind)> { + ) -> Result<(Cow<'_, UseReference>, PathKind)> { let path = self.resolve_path(raw_path, kind)?; let reference = match &path.value { PathKind::Direct(path) => { let r_uses = self.uses.get(path.as_str()); - let r_ctx = self.ctx.get_ref(path.borrow()); + let r_ctx = self.ctx.get_ref(path.borrow()).map(UseReference::from); // There can be overlap between items that are children of the root of ctx and the // uses here. Items from uses take priority and collisions aren't errors. if let Some(r_uses) = r_uses { @@ -337,6 +474,7 @@ impl<'a> Symbols<'a> { .ctx .ref_with_ident(path.borrow(), ident.borrow()) .map_err(|e| e.spanned(raw_path.span()))? + .map(UseReference::from) .map(Cow::Owned), }; @@ -359,7 +497,7 @@ impl<'a> Symbols<'a> { ConversionError::RefError(Box::new(RefError { uses: Some(self.uses.keys().cloned().collect()), path: path.value.clone(), - path_ref: PathReference::empty().into(), + path_ref: None, kind: kind.into(), })) } @@ -367,17 +505,17 @@ impl<'a> Symbols<'a> { } } - pub fn resolve_asset(&self, path: &Path) -> Result<(TypeId, TypePath)> { + pub fn resolve_asset(&self, path: &Path) -> Result<(TypeId, ObjectPath)> { let (item, resolved_path) = self.resolve_item(path, ResolveKind::Asset)?; let item = item.into_owned(); - if let Some((ty, path, _kind)) = item.asset { + if let Some((ty, path)) = item.asset { Ok((ty, path)) } else { Err(ConversionError::RefError(Box::new(RefError { uses: Some(self.uses.keys().cloned().collect()), path: resolved_path, - path_ref: item.into(), + path_ref: Some(item.into()), kind: RefKind::Asset, })) .spanned(path.span())) @@ -393,7 +531,7 @@ impl<'a> Symbols<'a> { Err(ConversionError::RefError(Box::new(RefError { uses: Some(self.uses.keys().cloned().collect()), path: resolved_path, - path_ref: item.into_owned().into(), + path_ref: Some(item.into_owned().into()), kind: RefKind::Type, })) .spanned(path.span())) @@ -412,37 +550,64 @@ pub(crate) struct EarlySymbols<'a, 'b> { /// This does not need mutable access but the user of `EarlySymbols` needs to mutate /// `BaubleContext`, so it is convenient to hold a mutable reference here. pub(super) ctx: &'a mut EarlyContext<'b>, + /// Context for local objects. + local_ctx: &'a mut LocalContext, /// Map of identifiers to path references. - pub(super) uses: HashMap, + uses: HashMap, } impl<'a, 'b> EarlySymbols<'a, 'b> { - pub fn new(ctx: &'a mut EarlyContext<'b>) -> Self { + pub fn new(ctx: &'a mut EarlyContext<'b>, local_ctx: &'a mut LocalContext) -> Self { Self { ctx, + local_ctx, uses: HashMap::default(), } } - pub fn bauble_ctx(&mut self) -> &mut BaubleContext { - self.ctx.ctx + /// Get contexts for registering new assets. + pub fn ctx_for_register(&mut self) -> (&mut BaubleContext, &mut LocalContext) { + (self.ctx.ctx, self.local_ctx) } - pub fn add_ref( + fn add_ref( &mut self, ident: TypePathElem, - reference: CombinedPathReference, + reference: impl Into, ) -> std::result::Result<(), ConversionError> { let r = self.uses.entry(ident.clone()).or_default(); *r = r .clone() - .combined(reference) + .combined(reference.into()) .ok_or(ConversionError::AmbiguousUse { ident })?; Ok(()) } + /// Note, this can also add the top level object of the current file under its local + /// identifier. + pub fn add_local_object( + &mut self, + ident: TypePathElem, + ty: Option, + full_path: TypePath, + ) -> std::result::Result<(), ConversionError> { + let path = if ident.as_str() == crate::object_path::TOP_LEVEL_IDENTIFIER { + ObjectPath::Top(full_path) + } else { + ObjectPath::Local(full_path) + }; + self.add_ref( + ident, + EarlyUseReference { + ty: None, + asset: Some((ty, path)), + module: None, + }, + ) + } + pub fn add_use(&mut self, use_path: &Spanned) -> Result<()> { fn add_use_inner( this: &mut EarlySymbols, @@ -459,7 +624,7 @@ impl<'a, 'b> EarlySymbols<'a, 'b> { return Err(ConversionError::RefError(Box::new(RefError { uses: None, path: PathKind::Direct(leading), - path_ref: PathReference::empty().into(), + path_ref: None, kind: RefKind::Module, })) .spanned(s.span)); @@ -478,7 +643,7 @@ impl<'a, 'b> EarlySymbols<'a, 'b> { return Err(ConversionError::RefError(Box::new(RefError { uses: None, path: PathKind::Direct(leading), - path_ref: PathReference::empty().into(), + path_ref: None, kind: RefKind::Module, })) .spanned(end.span)); @@ -495,7 +660,7 @@ impl<'a, 'b> EarlySymbols<'a, 'b> { return Err(ConversionError::RefError(Box::new(RefError { uses: None, path: PathKind::Direct(path), - path_ref: PathReference::empty().into(), + path_ref: None, kind: RefKind::Any, })) .spanned(end.span)); @@ -515,7 +680,7 @@ impl<'a, 'b> EarlySymbols<'a, 'b> { return Err(ConversionError::RefError(Box::new(RefError { uses: None, path: PathKind::Indirect(leading, path_end.to_owned()), - path_ref: PathReference::empty().into(), + path_ref: None, kind: RefKind::Any, })) .spanned(end.span)); @@ -539,7 +704,7 @@ impl<'a, 'b> EarlySymbols<'a, 'b> { return Err(ConversionError::RefError(Box::new(RefError { uses: None, path: PathKind::Direct(leading), - path_ref: PathReference::empty().into(), + path_ref: None, kind: RefKind::Module, })) .spanned(l.span)); @@ -569,7 +734,7 @@ impl<'a, 'b> EarlySymbols<'a, 'b> { return Err(ConversionError::RefError(Box::new(RefError { uses: None, path: PathKind::Direct(leading), - path_ref: PathReference::empty().into(), + path_ref: None, kind: RefKind::Module, })) .spanned(first.span)); @@ -584,7 +749,7 @@ impl<'a, 'b> EarlySymbols<'a, 'b> { return Err(ConversionError::RefError(Box::new(RefError { uses: None, path: PathKind::Direct(leading), - path_ref: PathReference::empty().into(), + path_ref: None, kind: RefKind::Module, })) .spanned(ident.span)); @@ -676,13 +841,13 @@ impl<'a, 'b> EarlySymbols<'a, 'b> { &self, raw_path: &Path, kind: ResolveKind, - ) -> Result<(Cow<'_, CombinedPathReference>, PathKind)> { + ) -> Result<(Cow<'_, EarlyUseReference>, PathKind)> { let path = self.resolve_path(raw_path, kind)?; let reference = match &path.value { PathKind::Direct(path) => { let r_uses = self.uses.get(path.as_str()); - let r_ctx = self.ctx.get_ref(path.borrow()); + let r_ctx = self.ctx.get_ref(path.borrow()).map(EarlyUseReference::from); // There can be overlap between items that are children of the root of ctx and the // uses here. Items from uses take priority and collisions aren't errors. if let Some(r_uses) = r_uses { @@ -700,6 +865,7 @@ impl<'a, 'b> EarlySymbols<'a, 'b> { .ctx .ref_with_ident(path.borrow(), ident.borrow()) .map_err(|e| e.spanned(raw_path.span()))? + .map(EarlyUseReference::from) .map(Cow::Owned), }; @@ -722,7 +888,7 @@ impl<'a, 'b> EarlySymbols<'a, 'b> { ConversionError::RefError(Box::new(RefError { uses: Some(self.uses.keys().cloned().collect()), path: path.value.clone(), - path_ref: PathReference::empty().into(), + path_ref: None, kind: kind.into(), })) } @@ -751,28 +917,39 @@ impl<'a, 'b> EarlySymbols<'a, 'b> { } /// Note, if an asset isn't registered in `BaubleContext` yet, its type will be unknown. - pub fn resolve_asset(&self, path: &Path) -> Result<(Option, TypePath)> { + pub fn resolve_asset(&self, path: &Path) -> Result<(Option, ObjectPath)> { let (item, resolved_path) = self.resolve_item(path, ResolveKind::Asset)?; let item = item.into_owned(); - if let Some((mut ty, path, _kind)) = item.asset { + if let Some((mut ty, path)) = item.asset { // Once an asset is registered with its type, the path reference stored in `uses` will // be outdated since it won't include the type (an up-to-date value isn't essential but // will lead to fewer assets to process in resolve_delayed). if ty.is_none() { - ty = self - .ctx - .get_ref(path.borrow()) - .and_then(|r| r.asset) - .expect("This asset is in uses, so it will exist in ctx.") - .0; + match &path { + ObjectPath::Top(path) => { + ty = self + .ctx + .get_ref(path.borrow()) + .and_then(|r| r.asset) + .expect("This asset is in uses, so it will exist in ctx.") + .0; + } + ObjectPath::Local(path) => { + ty = self.local_ctx.get(path.borrow()); + } + ObjectPath::Inline(_) => { + #[cfg(debug_assertions)] + unreachable!(); + } + } } Ok((ty, path)) } else { Err(ConversionError::RefError(Box::new(RefError { uses: Some(self.uses.keys().cloned().collect()), path: resolved_path, - path_ref: item, + path_ref: Some(item.into()), kind: RefKind::Asset, })) .spanned(path.span())) @@ -788,7 +965,7 @@ impl<'a, 'b> EarlySymbols<'a, 'b> { Err(ConversionError::RefError(Box::new(RefError { uses: Some(self.uses.keys().cloned().collect()), path: resolved_path, - path_ref: item.into_owned(), + path_ref: Some(item.into_owned().into()), kind: RefKind::Type, })) .spanned(path.span())) @@ -830,3 +1007,11 @@ impl SymbolsCommon for EarlySymbols<'_, '_> { self.ctx.type_registry() } } + +fn xor_option(a: Option, b: Option) -> Option> { + match (a, b) { + (Some(_), Some(_)) => None, + (Some(t), None) | (None, Some(t)) => Some(Some(t)), + (None, None) => Some(None), + } +} diff --git a/bauble/tests/integration.rs b/bauble/tests/integration.rs index d4b87d2..faf71d1 100644 --- a/bauble/tests/integration.rs +++ b/bauble/tests/integration.rs @@ -3,6 +3,7 @@ use bauble::Bauble; use bauble::BaubleContext; use bauble::Object; use bauble::Ref; +use bauble::object_path::ObjectPath; use bauble::path::{TypePath, TypePathElem}; #[derive(Bauble, PartialEq, Debug)] @@ -296,7 +297,11 @@ fn some_files_fail() { } #[derive(PartialEq, Debug)] -struct TestRef(String); +enum TestRef { + Top(String), + Local(String), + Inline(String), +} impl bauble::Bauble<'_> for TestRef { fn construct_type(registry: &mut bauble::types::TypeRegistry) -> bauble::types::Type { @@ -318,7 +323,11 @@ impl bauble::Bauble<'_> for TestRef { _allocator: &bauble::DefaultAllocator, ) -> std::result::Result { match val.value.value { - bauble::Value::Ref(r) => Ok(Self(String::from(r.as_str()))), + bauble::Value::Ref(r) => Ok(match r { + ObjectPath::Top(path) => Self::Top(String::from(path.as_str())), + ObjectPath::Local(path) => Self::Local(String::from(path.as_str())), + ObjectPath::Inline(path) => Self::Inline(String::from(path.as_str())), + }), _ => Err(Self::error( val.value.span, bauble::ToRustErrorKind::WrongType { found: val.ty }, @@ -332,9 +341,11 @@ fn same_file_references() { let a = &test_file!( "a", "0 = integration::Test { x: -5, y: 5 }\n\ - test_ref = $0", + test_ref = $0\n\ + test_ref2 = $a", // test both local identifier and full path for top level object Test { x: -5, y: 5 }, - TestRef("a".into()), + TestRef::Top("a".into()), + TestRef::Top("a".into()), ); test_load( @@ -352,7 +363,7 @@ fn same_file_references_reverse() { "a", "0 = $test\n\ test = integration::Test { x: -5, y: 5 }", - TestRef("a::test".into()), + TestRef::Local("a::test".into()), Test { x: -5, y: 5 }, ); @@ -365,31 +376,13 @@ fn same_file_references_reverse() { ); } -#[test] -fn same_file_references_reverse_full() { - let a = &test_file!( - "a", - "0 = $a::test\n\ - test = integration::Test { x: -5, y: 5 }", - TestRef("a::test".into()), - Test { x: -5, y: 5 }, - ); - - test_load( - &|ctx| { - ctx.register_type::(); - }, - &[a], - ); -} - #[test] fn reference_with_use() { let a = &test_file!( "a", "use b::test;\n\ 0 = $test", - TestRef("b::test".into()), + TestRef::Top("b::test".into()), ); let b = &test_file!( "b::test", @@ -406,6 +399,14 @@ fn reference_with_use() { ); } +fn top_ref(path: &str) -> Ref { + Ref::from_path(ObjectPath::Top(TypePath::new(path).unwrap().to_owned())) +} + +fn local_ref(path: &str) -> Ref { + Ref::from_path(ObjectPath::Local(TypePath::new(path).unwrap().to_owned())) +} + #[test] pub fn ref_implicit_type() { bauble::bauble_test!( @@ -414,16 +415,16 @@ pub fn ref_implicit_type() { r = $0" [ Test { x: -5, y: 5 }, - Ref::::from_path(TypePath::new_unchecked("test").to_owned()), + top_ref("test"), ] ); bauble::bauble_test!( [Test] - "0 = $test::t\n\ + "0 = $t\n\ t = integration::Test{ x: -5, y: 5 }" [ - Ref::::from_path(TypePath::new_unchecked("test::t").to_owned()), + local_ref("test::t"), Test { x: -5, y: 5 }, ] ); @@ -439,20 +440,20 @@ pub fn ref_explicit_type() { r2: Ref = $0" [ Test { x: -2, y: 2 }, - Ref::::from_path(TypePath::new_unchecked("test").to_owned()), - Ref::::from_path(TypePath::new_unchecked("test").to_owned()), + top_ref("test"), + top_ref("test"), ] ); bauble::bauble_test!( [Test] "use integration::Test;\n\ - 0: Ref = $test::t\n\ - r2: Ref = $test::t\n\ + 0: Ref = $t\n\ + r2: Ref = $t\n\ t = integration::Test{ x: -2, y: 2 }" [ - Ref::::from_path(TypePath::new_unchecked("test::t").to_owned()), - Ref::::from_path(TypePath::new_unchecked("test::t").to_owned()), + local_ref("test::t"), + local_ref("test::t"), Test { x: -2, y: 2 }, ] ); @@ -468,7 +469,7 @@ pub fn ref_explicit_type_multiple_files() { ] [ Test { x: -5, y: 5 }, - Ref::::from_path(TypePath::new_unchecked("test0").to_owned()), + top_ref("test0"), ] ); @@ -479,7 +480,7 @@ pub fn ref_explicit_type_multiple_files() { "0 = integration::Test{ x: -5, y: 5 }" ] [ - Ref::::from_path(TypePath::new_unchecked("test1").to_owned()), + top_ref("test1"), Test { x: -5, y: 5 }, ] ); @@ -495,7 +496,7 @@ pub fn ref_implicit_type_multiple_files() { ] [ Test { x: -5, y: 5 }, - Ref::::from_path(TypePath::new_unchecked("test0").to_owned()), + top_ref("test0"), ] ); @@ -506,7 +507,7 @@ pub fn ref_implicit_type_multiple_files() { "0 = integration::Test{ x: -5, y: 5 }" ] [ - Ref::::from_path(TypePath::new_unchecked("test1").to_owned()), + top_ref("test1"), Test { x: -5, y: 5 }, ] ); @@ -521,11 +522,11 @@ pub fn ref_explicit_type_incorrect() { bauble::bauble_test!( [Test, Incorrect] "0: integration::Incorrect = Incorrect(0)\n\ - r: Ref = $test::t\n\ + r: Ref = $t\n\ t = integration::Test{ x: -2, y: 2 }" [ Incorrect(0), - Ref::::from_path(TypePath::new_unchecked("test::t").to_owned()), + local_ref("test::t"), Test { x: -2, y: 2 }, ] ); @@ -545,7 +546,7 @@ pub fn ref_explicit_type_incorrect_multiple_files() { ] [ Test { x: -5, y: 5 }, - Ref::::from_path(TypePath::new_unchecked("test0").to_owned()), + top_ref("test0"), ] ); } @@ -564,7 +565,7 @@ pub fn ref_explicit_type_incorrect_multiple_files_reverse() { "0 = integration::Test{ x: -5, y: 5 }", ] [ - Ref::::from_path(TypePath::new_unchecked("test1").to_owned()), + top_ref("test1"), Test { x: -5, y: 5 }, ] ); @@ -585,7 +586,26 @@ pub fn ref_explicit_type_incorrect_multiple_files_ref_already_registered() { ] [ Test { x: -5, y: 5 }, - Ref::::from_path(TypePath::new_unchecked("test0").to_owned()), + top_ref("test0"), + ] + ); +} + +/// Test that local assets can not be referenced outside of the current file. +#[test] +#[should_panic = "Expected this path to refer to an asset"] +pub fn ref_local_from_another_file() { + bauble::bauble_test!( + [Test] + [ + "0 = $my_local\n\ + my_local = integration::Test{ x: -5, y: 5 }", + "0: Ref = $test0::my_local" + ] + [ + local_ref("test0::my_local"), + Test { x: -5, y: 5 }, + local_ref("test0::my_local"), ] ); } @@ -604,9 +624,9 @@ fn decimal_digits_identifiers() { Test { x: -5, y: 5 }, Test { x: -5, y: 5 }, Test { x: -5, y: 5 }, - TestRef("a".into()), - TestRef("a::2".into()), - TestRef("a::123".into()), + TestRef::Top("a".into()), + TestRef::Local("a::2".into()), + TestRef::Local("a::123".into()), ); test_load( @@ -724,10 +744,10 @@ fn name_matching_file_is_simplified() { a_ref = $0", vec![ Box::new(|object, ctx| { - assert!(object.top_level); + assert!(matches!(object.object_path, ObjectPath::Top(_))); (expected_value_fn(Test { x: -5, y: 5 }))(object, ctx) }), - expected_value_fn(TestRef("a".into())), + expected_value_fn(TestRef::Top("a".into())), ], ); // test non-top-level file @@ -738,11 +758,11 @@ fn name_matching_file_is_simplified() { ref_full = $a::c", vec![ Box::new(|object, ctx| { - assert!(object.top_level); + assert!(matches!(object.object_path, ObjectPath::Top(_))); (expected_value_fn(Test { x: -5, y: 5 }))(object, ctx) }), - expected_value_fn(TestRef("a::c".into())), - expected_value_fn(TestRef("a::c".into())), + expected_value_fn(TestRef::Top("a::c".into())), + expected_value_fn(TestRef::Top("a::c".into())), ], ); // test refering to them from a separate file @@ -750,8 +770,8 @@ fn name_matching_file_is_simplified() { "b", "0 = $a\n\ c_ref = $a::c", - TestRef("a".into()), - TestRef("a::c".into()), + TestRef::Top("a".into()), + TestRef::Top("a::c".into()), ); test_reload( @@ -763,8 +783,8 @@ fn name_matching_file_is_simplified() { ); } +// Note, this doesn't panic because local and top level asset paths can no longer collide. #[test] -#[should_panic = "'a::1' refers to an existing asset"] fn duplicate_name_after_simplification() { let a = &TestFile::new( "a", @@ -772,11 +792,11 @@ fn duplicate_name_after_simplification() { 1 = integration::Test { x: -5, y: 5 }", // local and full path are the same here vec![ Box::new(|object, ctx| { - assert!(object.top_level); + assert!(matches!(object.object_path, ObjectPath::Top(_))); (expected_value_fn(Test { x: -5, y: 5 }))(object, ctx) }), Box::new(|object, ctx| { - assert!(!object.top_level); + assert!(matches!(object.object_path, ObjectPath::Local(_))); (expected_value_fn(Test { x: -5, y: 5 }))(object, ctx) }), ], @@ -786,7 +806,7 @@ fn duplicate_name_after_simplification() { "a::1", "0 = integration::Test { x: -5, y: 5 }", vec![Box::new(|object, ctx| { - assert!(object.top_level); + assert!(matches!(object.object_path, ObjectPath::Top(_))); (expected_value_fn(Test { x: -5, y: 5 }))(object, ctx) })], ); @@ -810,11 +830,11 @@ fn duplicate_name_before_simplification() { 0 = integration::Test { x: -5, y: 5 }", vec![ Box::new(|object, ctx| { - assert!(object.top_level); + assert!(matches!(object.object_path, ObjectPath::Top(_))); (expected_value_fn(Test { x: -5, y: 5 }))(object, ctx) }), Box::new(|object, ctx| { - assert!(!object.top_level); + assert!(matches!(object.object_path, ObjectPath::Local(_))); (expected_value_fn(Test { x: -5, y: 5 }))(object, ctx) }), ], @@ -835,7 +855,7 @@ fn special_identifier_required_for_first_object() { "a", "1 = integration::Test { x: -5, y: 5 }", vec![Box::new(|object, ctx| { - assert!(object.top_level); + assert!(matches!(object.object_path, ObjectPath::Top(_))); (expected_value_fn(Test { x: -5, y: 5 }))(object, ctx) })], ); @@ -851,7 +871,7 @@ fn special_identifier_required_for_first_object() { #[test] #[should_panic = "`a::*::test` refers to multiple items in the same namespace"] fn ambiguous_ref_with_ident_via_reference() { - let a = &test_file!("a", "0 = $a::*::test", TestRef("b::test".into()),); + let a = &test_file!("a", "0 = $a::*::test", TestRef::Top("b::test".into()),); let a_b_test = &test_file!( "a::b::test", "0 = integration::Test { x: -5, y: 5 }", @@ -902,7 +922,7 @@ fn ambiguous_ref_with_ident_via_use() { #[test] #[should_panic = "`a::*::test` refers to multiple items in the same namespace"] fn amiguous_ref_with_ident_multilayer() { - let a = &test_file!("a", "0 = $a::*::test", TestRef("b::test".into())); + let a = &test_file!("a", "0 = $a::*::test", TestRef::Top("b::test".into())); let d = &test_file!( "d", "0 = integration::Test { x: -5, y: 5 }", @@ -913,7 +933,7 @@ fn amiguous_ref_with_ident_multilayer() { "0 = integration::Test { x: -5, y: 5 }", Test { x: -5, y: 5 }, ); - let a_c_test = &test_file!("a::c::test", "0 = $d", TestRef("d".into())); + let a_c_test = &test_file!("a::c::test", "0 = $d", TestRef::Top("d".into())); test_load( &|ctx| { @@ -937,7 +957,7 @@ fn local_asset_suggested() { "\n\ 0 = $tet\n\ test = integration::Test { x: -5, y: 5 }", - TestRef("a::test".into()), + TestRef::Local("a::test".into()), Test { x: -5, y: 5 }, ); diff --git a/bauble_macros/tests/derive.rs b/bauble_macros/tests/derive.rs index 75ea381..acc6191 100644 --- a/bauble_macros/tests/derive.rs +++ b/bauble_macros/tests/derive.rs @@ -1,6 +1,6 @@ use std::collections::HashMap; -use bauble::{Bauble, Ref, SpannedValue, bauble_test, path::TypePath}; +use bauble::{Bauble, Ref, SpannedValue, bauble_test, object_path::ObjectPath, path::TypePath}; #[test] fn test_struct() { @@ -368,6 +368,14 @@ fn test_trait() { ); } +fn top_ref(path: &str) -> Ref { + Ref::from_path(ObjectPath::Top(TypePath::new(path).unwrap().to_owned())) +} + +fn local_ref(path: &str) -> Ref { + Ref::from_path(ObjectPath::Local(TypePath::new(path).unwrap().to_owned())) +} + #[test] fn test_generic() { #[derive(Bauble, Debug, PartialEq, Eq)] @@ -385,15 +393,15 @@ fn test_generic() { use derive::{Foo, Bar, Str}; 0: Foo = Foo(Bar(24)) - b: Ref> = $test::c + b: Ref> = $c c: Foo = Foo(Str("test")) d: Ref> = $0 "# [ Foo(Bar(24)), - Ref::>::from_path(TypePath::new("test::c").unwrap().to_owned()), + local_ref::>("test::c"), Foo(Str(String::from("test"))), - Ref::>::from_path(TypePath::new("test").unwrap().to_owned()), + top_ref::>("test"), ] ); } From 6cccd0e23176fdc19d9e531399e30a7f12fdf78d Mon Sep 17 00:00:00 2001 From: Imbris Date: Thu, 9 Apr 2026 15:41:13 -0400 Subject: [PATCH 19/25] Small fix (otherwise, 'static is required for key) --- bauble/src/object_path.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/bauble/src/object_path.rs b/bauble/src/object_path.rs index 646df36..22be28f 100644 --- a/bauble/src/object_path.rs +++ b/bauble/src/object_path.rs @@ -137,16 +137,16 @@ impl<'a, S: AsRef + 'a> std::borrow::Borrow for Obj } } -impl Hash for dyn ObjectPathKey { +impl Hash for dyn ObjectPathKey + '_ { fn hash(&self, state: &mut H) { self.key().hash(state) } } -impl PartialEq for dyn ObjectPathKey { +impl PartialEq for dyn ObjectPathKey + '_ { fn eq(&self, other: &Self) -> bool { self.key() == other.key() } } -impl Eq for dyn ObjectPathKey {} +impl Eq for dyn ObjectPathKey + '_ {} From 8ee3a0b6ed2aa8ec9696776e81e92bb4e2baf603 Mon Sep 17 00:00:00 2001 From: Imbris Date: Thu, 23 Apr 2026 11:43:17 -0400 Subject: [PATCH 20/25] More useful ObjectPath methods --- bauble/src/object_path.rs | 29 +++++++++++++++++++++++++++++ bauble/src/types/path.rs | 14 +++++++------- 2 files changed, 36 insertions(+), 7 deletions(-) diff --git a/bauble/src/object_path.rs b/bauble/src/object_path.rs index 22be28f..31f590a 100644 --- a/bauble/src/object_path.rs +++ b/bauble/src/object_path.rs @@ -53,6 +53,11 @@ impl> ObjectPath { } } + /// Get the file path that this object belongs to. + pub fn file_path(&self) -> TypePath<&str> { + self.borrow().into_file_path() + } + /// Gets a borrowed version of the path. pub fn borrow(&self) -> ObjectPath<&str> { match self { @@ -62,6 +67,15 @@ impl> ObjectPath { } } + /// Convert this into an owned path. + pub fn to_owned(&self) -> ObjectPath { + match self { + Self::Top(path) => ObjectPath::Top(path.to_owned()), + Self::Local(path) => ObjectPath::Local(path.to_owned()), + Self::Inline(path) => ObjectPath::Inline(path.to_owned()), + } + } + /// Casts reference to trait object that `ObjectPath` implements `Borrow` for. /// /// This useful to mix owned and borrowed paths when using `ObjectPath` as a key type in a map @@ -71,6 +85,21 @@ impl> ObjectPath { } } +impl<'a> ObjectPath<&'a str> { + /// Get the file path that this object belongs to. + pub fn into_file_path(&self) -> TypePath<&'a str> { + match *self { + Self::Top(path) => path, + Self::Local(path) => path + .split_end() + .map_or(TypePath::empty(), |(prefix, _)| prefix), + Self::Inline(path) => path + .split_end() + .map_or(TypePath::empty(), |(prefix, _)| prefix), + } + } +} + #[derive(Copy, Clone, PartialEq, Eq, Hash, PartialOrd, Ord)] enum ObjectPathKeyImpl<'a> { Top(TypePath<&'a str>), diff --git a/bauble/src/types/path.rs b/bauble/src/types/path.rs index 3c167cf..98eec37 100644 --- a/bauble/src/types/path.rs +++ b/bauble/src/types/path.rs @@ -189,13 +189,6 @@ pub type Result = std::result::Result; /// /// Returns None if this isn't a valid path. fn path_len(path: &str) -> Result { - if path.is_empty() { - return Ok(0); - } - let mut count = 1; - let mut current_path_delim = PATH_SEPERATOR.chars(); - let mut path_iter = path.char_indices(); - fn end_delim(path_iter: &mut std::str::CharIndices<'_>, end: char, index: usize) -> Result<()> { while let Some((i, c)) = path_iter.next() { if c == end { @@ -222,6 +215,13 @@ fn path_len(path: &str) -> Result { } } + if path.is_empty() { + return Ok(0); + } + + let mut count = 1; + let mut current_path_delim = PATH_SEPERATOR.chars(); + let mut path_iter = path.char_indices(); let mut is_empty = true; while let Some((i, c)) = path_iter.next() { From 71276dbb1fe8a6d1e5894da4a27e91e32f80602c Mon Sep 17 00:00:00 2001 From: Imbris Date: Mon, 4 May 2026 12:01:57 -0400 Subject: [PATCH 21/25] Add dummy top level object to validation file since a top level object is now required --- bauble/src/types.rs | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/bauble/src/types.rs b/bauble/src/types.rs index d705c62..d592a86 100644 --- a/bauble/src/types.rs +++ b/bauble/src/types.rs @@ -562,10 +562,9 @@ impl TypeRegistry { } let file = TypePath::new("validate").unwrap(); - // TODO: add dummy top level object - if assert_instanciable { let mut objects = Vec::new(); + for (i, ty_id) in self .iter_type_set(self.key_trait(Self::any_trait())) .enumerate() @@ -611,7 +610,9 @@ impl TypeRegistry { // Changes in sub-asset paths are specifically ignored, only the content of the // sub-assets must match. - let source = crate::display_formatted( + // dummy top level object + let mut source = "0 = ()\n".to_string(); + source += &crate::display_formatted( objects.as_slice(), self, &crate::DisplayConfig { @@ -637,8 +638,13 @@ impl TypeRegistry { mismatched, missing, new, - }) = crate::compare_object_sets(objects.into_iter(), loaded_objects.into_iter()) - { + }) = crate::compare_object_sets( + objects.into_iter(), + loaded_objects.into_iter().filter(|obj| { + // skip dummy top level object + obj.object_path.borrow() != ObjectPath::Top(TypePath::new("validate").unwrap()) + }), + ) { return Err( if let Some((_, span, original, new)) = mismatched.into_iter().next() { let src = source From ddd90005dc42bfb93d4648cc7de339dc4db5bcec Mon Sep 17 00:00:00 2001 From: Imbris Date: Fri, 8 May 2026 12:48:59 -0400 Subject: [PATCH 22/25] Remove is_subobject, we no longer need this method with the ObjectPath enum distinguishing objects kinds --- bauble/src/types/path.rs | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/bauble/src/types/path.rs b/bauble/src/types/path.rs index 98eec37..9c6a52c 100644 --- a/bauble/src/types/path.rs +++ b/bauble/src/types/path.rs @@ -487,16 +487,6 @@ impl> TypePath { }) } - // TODO: can this method be phased out now that `ObjectPath` exists? - /// Determines if a path referencing an object is a path to a sub-object. - /// The path is assumed to be valid. - /// - /// This means that: - /// - The path contains the special '@' sub-object character. - pub fn is_subobject(&self) -> bool { - self.iter().any(|part| part.as_str().contains('@')) - } - /// Appends `end` onto `self`. /// /// Appending is different from `join` in that it will not insert a separator, From 11af202123366b2b64383831381e0dbdbf88207a Mon Sep 17 00:00:00 2001 From: Imbris Date: Fri, 8 May 2026 13:23:45 -0400 Subject: [PATCH 23/25] Add examples of contents of ObjectPath variants --- bauble/src/object_path.rs | 15 ++++++++++++++ bauble/tests/integration.rs | 40 +++++++++++++++++++++++++++++++++++++ 2 files changed, 55 insertions(+) diff --git a/bauble/src/object_path.rs b/bauble/src/object_path.rs index 31f590a..7375447 100644 --- a/bauble/src/object_path.rs +++ b/bauble/src/object_path.rs @@ -21,6 +21,21 @@ pub const TOP_LEVEL_IDENTIFIER: &str = "0"; /// Full path to a bauble object (or external asset). /// /// Path format documented in [`TypePath`]. +/// +/// ### Example +/// +/// ```ignore +/// // in my_file.bbl +/// 0 = Foo { bar: 1 } +/// top_ref = $0 +/// my_local_foo = Foo { bar: 2 } +/// local_ref = $my_local_foo +/// inline_ref: Ref = Foo { bar: 3 } +/// ``` +/// +/// - `top_ref` will be `ObjectPath::Top("my_file") +/// - `local_ref` will be `ObjectPath::Local("my_file::my_local_foo") +/// - `inline_ref` will be `ObjectPath::Inline("my_file::inline_ref&6@0") #[derive(Copy, Clone, Debug)] pub enum ObjectPath { /// Top-level object or external asset. There is at most one per file. diff --git a/bauble/tests/integration.rs b/bauble/tests/integration.rs index faf71d1..d22114a 100644 --- a/bauble/tests/integration.rs +++ b/bauble/tests/integration.rs @@ -969,3 +969,43 @@ fn local_asset_suggested() { &[a], ); } + +#[test] +fn object_path_examples() { + #[derive(bauble::Bauble, Debug, PartialEq)] + struct Foo { + bar: u8, + } + + fn top_ref(path: &str) -> Ref { + Ref::from_path(ObjectPath::Top(TypePath::new(path).unwrap().to_owned())) + } + + fn local_ref(path: &str) -> Ref { + Ref::from_path(ObjectPath::Local(TypePath::new(path).unwrap().to_owned())) + } + + fn inline_ref(path: &str) -> Ref { + Ref::from_path(ObjectPath::Inline(TypePath::new(path).unwrap().to_owned())) + } + + bauble::bauble_test!( + [Foo] + r#" + 0 = integration::Foo { bar: 1 } + top_ref = $0 + my_local_foo = integration::Foo { bar: 2 } + local_ref = $my_local_foo + inline_ref: Ref = integration::Foo { bar: 3 } + "# + // NOTE: Update `ObjectPath` docs if this test needs changes! + [ + Foo { bar: 3 }, + Foo { bar: 1 }, + top_ref("test"), + Foo { bar: 2 }, + local_ref("test::my_local_foo"), + inline_ref("test::inline_ref&6@0"), + ] + ); +} From a61e4a92a6b3702e5f49f2bb035c5d8724f1cd6a Mon Sep 17 00:00:00 2001 From: Imbris Date: Fri, 8 May 2026 19:17:14 -0400 Subject: [PATCH 24/25] Typos and comment cleanup --- bauble/src/object_path.rs | 3 ++- bauble/src/parse/value.rs | 2 +- bauble/src/value/convert.rs | 2 +- bauble/src/value/mod.rs | 8 ++++---- bauble/src/value/symbols.rs | 1 - 5 files changed, 8 insertions(+), 8 deletions(-) diff --git a/bauble/src/object_path.rs b/bauble/src/object_path.rs index 7375447..cab7b33 100644 --- a/bauble/src/object_path.rs +++ b/bauble/src/object_path.rs @@ -18,7 +18,8 @@ use std::hash::{Hash, Hasher}; /// (i.e. the top level asset that is named after the file). pub const TOP_LEVEL_IDENTIFIER: &str = "0"; -/// Full path to a bauble object (or external asset). +/// Full path to a bauble object (or external asset). See [the module level documentation](self) +/// for more. /// /// Path format documented in [`TypePath`]. /// diff --git a/bauble/src/parse/value.rs b/bauble/src/parse/value.rs index 2bea8a5..e90667c 100644 --- a/bauble/src/parse/value.rs +++ b/bauble/src/parse/value.rs @@ -181,7 +181,7 @@ pub struct Binding { pub enum BindingIdent { /// This is the top level asset in a parsed file. /// - /// It has the special cased identifier `0` and appears as the first item in the file.. + /// It has the special cased identifier `0` and appears as the first item in the file. /// /// This holds no identifier string because it will have the same path as the file containing /// it. diff --git a/bauble/src/value/convert.rs b/bauble/src/value/convert.rs index 2eaff45..8f7d0be 100644 --- a/bauble/src/value/convert.rs +++ b/bauble/src/value/convert.rs @@ -237,7 +237,7 @@ pub struct ConvertMeta<'a> { /// Used as a prefix to uniquely name inline objects. /// /// Note, for top level objects we specifically use `0` and not the file name. If we used the - /// file name inline object names could collide with inline objects of a local object in + /// file name, inline object names could collide with inline objects of a local object in /// another file. pub object_name: TypePathElem<&'a str>, pub default_span: Span, diff --git a/bauble/src/value/mod.rs b/bauble/src/value/mod.rs index 4e6aaab..bafeb42 100644 --- a/bauble/src/value/mod.rs +++ b/bauble/src/value/mod.rs @@ -726,11 +726,11 @@ pub(crate) fn resolve_delayed( /// Returns the identifier and the path of an object. /// /// The top level object in each file will have a path that matches the path of the file containing -/// it. The identifier for these objects is always "0" and it can be referred to locally in the -/// same file using this identifier. +/// it and be wrapped in `ObjectPath::Top`. The identifier for these objects is always "0" and it +/// can be referred to locally in the same file using this identifier. /// -/// TODO: make sure this is updated -/// For other objects, the path is just the files's bauble path joined with the object identifier. +/// For all other objects with binding are local object. For these, the path is just the file's +/// bauble path joined with the object identifier and they are wrapped in `ObjectPath::Local`. fn object_ident_path<'a>( file_path: TypePath<&str>, binding_ident: &'a BindingIdent, diff --git a/bauble/src/value/symbols.rs b/bauble/src/value/symbols.rs index 0e59dee..67e3ef6 100644 --- a/bauble/src/value/symbols.rs +++ b/bauble/src/value/symbols.rs @@ -158,7 +158,6 @@ impl From for ErrorPathReference { /// Representation of item names available in the current module. /// /// There are multiple namespaces: types, assets (i.e. values defined in bauble), and modules. -//#[derive(Clone)] pub(crate) struct Symbols<'a> { /// Context for looking up things referenced by full path. pub(super) ctx: &'a BaubleContext, From a23b1c496984adfc2dc997d9cea9eea502b48663 Mon Sep 17 00:00:00 2001 From: Imbris Date: Fri, 8 May 2026 19:20:23 -0400 Subject: [PATCH 25/25] Rename Symbols::add_local_object to more accurate name --- bauble/src/value/mod.rs | 4 ++-- bauble/src/value/symbols.rs | 12 ++++++------ 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/bauble/src/value/mod.rs b/bauble/src/value/mod.rs index bafeb42..1e40605 100644 --- a/bauble/src/value/mod.rs +++ b/bauble/src/value/mod.rs @@ -807,7 +807,7 @@ pub(crate) fn register_assets( }; let ty = None; - if let Err(e) = symbols.add_local_object(ident.to_owned(), ty, path) { + if let Err(e) = symbols.add_current_file_object(ident.to_owned(), ty, path) { errors.push(e.spanned(span)); } } @@ -961,7 +961,7 @@ pub(crate) fn convert_values( continue; }; - if let Err(e) = symbols.add_local_object(ident.to_owned(), ty, path) { + if let Err(e) = symbols.add_current_file_object(ident.to_owned(), ty, path) { errors.push(e.spanned(span)); } } diff --git a/bauble/src/value/symbols.rs b/bauble/src/value/symbols.rs index 67e3ef6..2f26e7e 100644 --- a/bauble/src/value/symbols.rs +++ b/bauble/src/value/symbols.rs @@ -188,9 +188,9 @@ impl<'a> Symbols<'a> { Ok(()) } - /// Note, this can also add the top level object of the current file under its local - /// identifier. - pub fn add_local_object( + /// Note, in addition to local objects, this is also used to add the top level object of the + /// current file under its local identifier. + pub fn add_current_file_object( &mut self, ident: TypePathElem, ty: TypeId, @@ -584,9 +584,9 @@ impl<'a, 'b> EarlySymbols<'a, 'b> { Ok(()) } - /// Note, this can also add the top level object of the current file under its local - /// identifier. - pub fn add_local_object( + /// Note, in addition to local objects, this is also used to add the top level object of the + /// current file under its local identifier. + pub fn add_current_file_object( &mut self, ident: TypePathElem, ty: Option,