diff --git a/bauble/src/context.rs b/bauble/src/context.rs index 486141e..aa69636 100644 --- a/bauble/src/context.rs +++ b/bauble/src/context.rs @@ -11,13 +11,27 @@ pub type Source = ariadne::Source; #[derive(Clone, Default)] struct DefaultUses(IndexMap); +/// Assets can be either top level or local. +#[derive(Clone, Copy, Debug)] +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)>, + pub asset: Option<(TypeId, TypePath, AssetKind)>, /// If the reference references a module. pub module: Option, } @@ -33,10 +47,10 @@ impl PathReference { } /// Constructs a path reference referencing the 'any' type. - pub fn any(path: TypePath) -> Self { + pub fn any(path: TypePath, kind: AssetKind) -> Self { Self { ty: Some(TypeRegistry::any_type()), - asset: Some((TypeRegistry::any_type(), path.clone())), + asset: Some((TypeRegistry::any_type(), path.clone(), kind)), module: Some(path.clone()), } } @@ -202,7 +216,7 @@ impl BaubleContextBuilder { #[derive(Default, Clone, Debug)] struct InnerReference { ty: Option, - asset: Option, + asset: Option<(TypeId, AssetKind)>, redirect: Option, } @@ -244,7 +258,10 @@ impl CtxNode { fn reference(&self, root: &Self) -> PathReference { let mut this = PathReference { ty: self.reference.ty, - asset: self.reference.asset.map(|ty| (ty, self.path.clone())), + asset: self + .reference + .asset + .map(|(ty, kind)| (ty, self.path.clone(), kind)), module: (!self.children.is_empty()).then(|| self.path.clone()), }; @@ -381,22 +398,31 @@ impl CtxNode { && self.source.is_none() } - /// Clears all assets in the module this node references (if any). + /// Clears all assets in the file this node references (if any). /// /// Does not recurse into other files, but will clear inline submodules in the current file (if /// that feature is added). - fn clear_child_assets(&mut self) { + fn clear_file_assets(&mut self) { self.children.retain(|_, node| { - if node.source.is_some() { - return true; + // `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() { + node.clear_file_assets(); } - node.clear_child_assets(); - - node.reference.asset = None; - !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)); } /// Builds all path elements as modules @@ -428,13 +454,15 @@ impl CtxNode { node.reference.ty = Some(id); } - fn build_asset(&mut self, path: TypePath<&str>, ty: TypeId) { + fn build_asset(&mut self, path: TypePath<&str>, ty: TypeId, kind: AssetKind) -> Result<(), ()> { let node = self.build_nodes(path); if node.reference.asset.is_some() { - panic!("Multiple types with the same path"); + // Multiple assets with the same path + return Err(()); } - node.reference.asset = Some(ty); + node.reference.asset = Some((ty, kind)); + Ok(()) } } @@ -503,11 +531,31 @@ impl BaubleContext { /// With this method you can expose assets that aren't in bauble. /// /// Returns ID of internal Ref type for `ty`. - pub fn register_asset(&mut self, path: TypePath<&str>, ty: TypeId) -> TypeId { + /// + /// 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 { let ref_ty = self.registry.get_or_register_asset_ref(ty); self.root_node.build_type(ref_ty, &self.registry); - self.root_node.build_asset(path, ref_ty); - ref_ty + self.root_node + .build_asset(path, ref_ty, kind) + .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) } fn file(&self, file: FileId) -> (TypePath<&str>, &Source) { @@ -575,7 +623,7 @@ impl BaubleContext { // Need a partial borrow here. let (path, _) = &self.files[file.0]; if let Some(node) = self.root_node.node_at_mut(path.borrow()) { - node.clear_child_assets(); + node.clear_file_assets(); } } @@ -731,18 +779,24 @@ impl BaubleContext { } /// Get all the assets starting from `path`, with an optional maximum depth of `max_depth`. + /// + /// Inline objects are not registered as assets, they are exclusively visible in the list of + /// objects returned when loading/reloading files. pub fn assets( &self, path: TypePath<&str>, max_depth: Option, - ) -> impl Iterator> { + ) -> impl Iterator, AssetKind)> { self.root_node .node_at(path) .map(|node| node.iter_all_children(max_depth)) .into_iter() .flatten() - .filter(|node| node.reference(&self.root_node).asset.is_some()) - .map(|node| node.path.borrow()) + .filter_map(|node| { + node.reference(&self.root_node) + .asset + .map(|(_, _, kind)| (node.path.borrow(), kind)) + }) } /// Get all the types starting from `path`, with an optional maximum depth of `max_depth`. diff --git a/bauble/src/lib.rs b/bauble/src/lib.rs index dde2603..4504fb4 100644 --- a/bauble/src/lib.rs +++ b/bauble/src/lib.rs @@ -15,7 +15,7 @@ pub mod types; pub use bauble_macros::Bauble; pub use builtin::Ref; -pub use context::{BaubleContext, BaubleContextBuilder, FileId, PathReference, Source}; +pub use context::{AssetKind, BaubleContext, BaubleContextBuilder, FileId, PathReference, Source}; pub use error::{BaubleError, BaubleErrors, CustomError, Level}; pub use spanned::{Span, SpanExt, Spanned}; pub use traits::{ @@ -110,7 +110,7 @@ pub mod private { let mut error_msg = errors.try_to_string(&ctx.read().unwrap()).unwrap(); for (path, re_source) in re_path_sources { use std::fmt::Write; - writeln!(&mut error_msg, "In file \"{path}\": {re_source}").unwrap(); + writeln!(&mut error_msg, "In file \"{path}\":\n{re_source}").unwrap(); } panic!("Error re-converting: \n{error_msg}"); } diff --git a/bauble/src/parse/parser.rs b/bauble/src/parse/parser.rs index 77bc902..9d0c450 100644 --- a/bauble/src/parse/parser.rs +++ b/bauble/src/parse/parser.rs @@ -681,13 +681,19 @@ pub fn parser<'a>() -> impl Parser<'a, ParserSource<'a>, ParseValues, Extra<'a>> .collect::>(), ) .validate(|(uses, values), _, emitter| { - values.into_iter().fold( + values.into_iter().enumerate().fold( ParseValues { uses, values: IndexMap::default(), }, - |mut values, (ident, type_path, value)| { - let binding = Binding { type_path, value }; + |mut values, (i, (ident, type_path, value))| { + let is_first = i == 0; + + let binding = Binding { + type_path, + value, + is_first, + }; if values.values.contains_key(&ident) { emitter.emit(Rich::custom( diff --git a/bauble/src/parse/value.rs b/bauble/src/parse/value.rs index ca6142f..4efaa7d 100644 --- a/bauble/src/parse/value.rs +++ b/bauble/src/parse/value.rs @@ -168,6 +168,7 @@ impl SpannedValue for ParseVal { pub struct Binding { pub type_path: Option, pub value: ParseVal, + pub is_first: bool, } #[derive(Debug)] diff --git a/bauble/src/types.rs b/bauble/src/types.rs index 931e3f5..211437d 100644 --- a/bauble/src/types.rs +++ b/bauble/src/types.rs @@ -248,6 +248,9 @@ impl Display for TypeSystemError<'_> { } impl TypeRegistry { + /// The path of the builtin in generic `Ref<_>` type without the generics. + pub const REF_TYPE_PATH: &str = "Ref"; + /// This is not performant and should not be exposed API, it is used for tests. #[doc(hidden)] pub fn find_rust_type(&self, ty: TypeId) -> Option { @@ -429,8 +432,12 @@ impl TypeRegistry { this.asset_refs.insert(inner, id); let ty = Type { meta: TypeMeta { - path: TypePath::new(format!("Ref<{}>", this.key_type(inner).meta.path)) - .expect("Invariant"), + path: TypePath::new(format!( + "{}<{}>", + Self::REF_TYPE_PATH, + this.key_type(inner).meta.path + )) + .expect("Invariant"), traits: this.key_type(inner).meta.traits.clone(), ..Default::default() }, @@ -586,11 +593,16 @@ impl TypeRegistry { for (name, value) in additonal.into_objects() { objects.push(crate::Object { object_path: file.join(&name), + top_level: false, value, }) } - objects.push(crate::Object { object_path, value }) + objects.push(crate::Object { + object_path, + top_level: false, + value, + }) } // Check that instantiated objects match after being serialized to bauble text and @@ -721,7 +733,7 @@ impl TypeRegistry { } } - /// Register `T` if it is not registerted already, then get the trait ID for `T`. + /// Register `T` if it is not registered already, then get the trait ID for `T`. pub fn get_or_register_trait(&mut self) -> TraitId { let rust_id = std::any::TypeId::of::(); if let Some(id) = self.type_from_rust.get(&rust_id) { diff --git a/bauble/src/types/path.rs b/bauble/src/types/path.rs index b5e5ef2..8391630 100644 --- a/bauble/src/types/path.rs +++ b/bauble/src/types/path.rs @@ -380,11 +380,15 @@ impl> TypePath { } /// Split the start segment with the remaining segments. + /// + /// Returns `None` if self is empty. pub fn get_start(&self) -> Option<(TypePathElem<&str>, TypePath<&str>)> { self.borrow().split_start() } /// Split the end segment with the remaining segments. + /// + /// Returns `None` if self is empty. pub fn get_end(&self) -> Option<(TypePath<&str>, TypePathElem<&str>)> { self.borrow().split_end() } diff --git a/bauble/src/value/convert.rs b/bauble/src/value/convert.rs index e527cd7..dddf68a 100644 --- a/bauble/src/value/convert.rs +++ b/bauble/src/value/convert.rs @@ -6,12 +6,10 @@ use crate::{ path::{TypePath, TypePathElem}, spanned::{SpanExt, Spanned}, types::{self, TypeId}, - value::Fields, -}; - -use super::{ - Attributes, Ident, Result, SpannedValue, Symbols, UnspannedVal, Val, Value, ValueContainer, - ValueTrait, + value::{ + Attributes, Fields, Ident, SpannedValue, Symbols, UnspannedVal, Val, Value, ValueContainer, + ValueTrait, error::Result, + }, }; pub fn no_attr() -> Option<&'static Attributes> { @@ -225,6 +223,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. pub object_name: TypePathElem<&'a str>, pub default_span: Span, } @@ -251,10 +254,10 @@ impl ConvertMeta<'_> { /// Represents additional objects added when parsing other objects. /// -/// Also known as sub-assets. +/// Also known as sub-objects/sub-assets. pub(super) struct AdditionalObjects { objects: Vec, - name_allocs: std::collections::HashMap, + name_allocs: HashMap, file_path: TypePath, } @@ -284,8 +287,8 @@ impl AdditionalObjects { .expect("idx is just a number, and we know name is a valid path elem."); self.objects.push(super::create_object( - self.file_path.borrow(), - name.borrow(), + self.file_path.join(&name), + false, val, symbols, )?); @@ -314,8 +317,8 @@ impl AdditionalObjects { for (name, value) in unspanned.into_objects() { self.objects.push(super::create_object( - self.file_path.borrow(), - name.borrow(), + self.file_path.join(&name), + false, value.into_spanned(span), symbols, )?); @@ -325,16 +328,37 @@ impl AdditionalObjects { } } +/// Keeps track of existing names used for additional objects and allocates new names. enum NameAllocs<'a> { - Owned(std::collections::HashMap), - Borrowed(&'a mut std::collections::HashMap), + Owned(HashMap), + Borrowed(&'a mut HashMap), + Custom(&'a mut dyn FnMut(TypePathElem<&str>) -> TypePathElem), } impl NameAllocs<'_> { - fn get_mut(&mut self) -> &mut std::collections::HashMap { + fn reborrow(&mut self) -> NameAllocs<'_> { + match self { + Self::Owned(m) => NameAllocs::Borrowed(m), + Self::Borrowed(m) => NameAllocs::Borrowed(m), + Self::Custom(f) => NameAllocs::Custom(f), + } + } + + /// Allocate a new name for an object that will be referenced by the parent object + /// `object_name`. + fn allocate_name(&mut self, object_name: TypePathElem<&str>) -> TypePathElem { + 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.") + }; match self { - NameAllocs::Owned(m) => m, - NameAllocs::Borrowed(m) => m, + Self::Owned(m) => from_map(m), + Self::Borrowed(m) => from_map(m), + Self::Custom(f) => f(object_name), } } } @@ -354,7 +378,7 @@ impl<'a> AdditionalUnspannedObjects<'a> { file_path, object_name, objects: Vec::new(), - name_allocs: NameAllocs::Owned(std::collections::HashMap::default()), + name_allocs: NameAllocs::Owned(HashMap::default()), } } @@ -372,7 +396,29 @@ impl<'a> AdditionalUnspannedObjects<'a> { } } - /// Add the type id to the path this keeps track of, for better unique names for sub-objects. + /// Create a new instance with a custom closure to create unique names. + /// + /// 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. + pub fn new_with_custom_namer( + file_path: TypePath<&'a str>, + object_name: TypePathElem<&'a str>, + namer: &'a mut impl FnMut(TypePathElem<&str>) -> TypePathElem, + ) -> Self { + Self { + file_path, + object_name, + objects: Vec::new(), + name_allocs: NameAllocs::Custom(namer), + } + } + + /// Add the type id to the parent object name this keeps track of, for better unique names for + /// sub-objects. pub fn in_type( &mut self, ty: TypeId, @@ -380,11 +426,12 @@ impl<'a> AdditionalUnspannedObjects<'a> { ) -> R { let name = TypePathElem::new(format!("{}&{}", self.object_name, ty.inner())).unwrap(); - let mut inner = AdditionalUnspannedObjects::new_with_name_allocs( - self.file_path, - name.borrow(), - self.name_allocs.get_mut(), - ); + let mut inner = AdditionalUnspannedObjects { + file_path: self.file_path, + object_name: name.borrow(), + objects: Vec::new(), + name_allocs: self.name_allocs.reborrow(), + }; let res = f(&mut inner); @@ -395,19 +442,9 @@ impl<'a> AdditionalUnspannedObjects<'a> { /// Add an additional object, and get a reference to it. pub fn add_object(&mut self, val: UnspannedVal) -> Value { - let idx = *self - .name_allocs - .get_mut() - .entry(self.object_name.to_owned()) - .and_modify(|i| *i += 1u64) - .or_insert(0); - let name = TypePathElem::new(format!("{}@{idx}", self.object_name)) - .expect("idx is just a number, and we know name is a valid path elem."); - + let name = self.name_allocs.allocate_name(self.object_name); let res = Value::Ref(self.file_path.join(&name)); - self.objects.push((name, val)); - res } @@ -893,13 +930,13 @@ where { let variant = Self::get_variant(variant, meta.symbols)? .spanned(Self::variant_span(variant, &meta)); - let ty = variants.get(&*variant).ok_or( + let ty = variants.get(&*variant).ok_or_else(|| { ConversionError::UnknownVariant { variant: variant.clone(), ty: ty_id.value, } - .spanned(span), - )?; + .spanned(span) + })?; ( variant.map(|v| v.to_owned()), extra_attributes.convert_with(value, meta.reborrow(), ty)?, diff --git a/bauble/src/value/display.rs b/bauble/src/value/display.rs index b4ffc4b..7dccb45 100644 --- a/bauble/src/value/display.rs +++ b/bauble/src/value/display.rs @@ -96,7 +96,9 @@ mod formatter { LineWriter(self.0.reborrow()) } - pub fn write_recursive(&mut self, f: impl FnOnce(Formatter)) { + /// Executes `f` with a formatter that has an additional level of indention and writes a + /// newline afterwards. + pub fn write_indented(&mut self, f: impl FnOnce(Formatter)) { f(self.0.bump_indent()); self.0.new_line(); } @@ -203,7 +205,15 @@ mod formatter { } // pub fn map_ctx<'b, C>(&'b mut self, c: impl FnOnce(&CTX) -> C) + pub fn write_line(&mut self, f: impl FnOnce(LineWriter)) { + f(LineWriter(self.reborrow())); + self.new_line(); + } + + /// Like [`write_line`](Self::write_line) but the newline comes before the content written + /// by `f`. + pub fn write_line_prefix(&mut self, f: impl FnOnce(LineWriter)) { self.new_line(); f(LineWriter(self.reborrow())); } @@ -229,9 +239,9 @@ fn slice_display>(slice: &[T], mut w: LineWriter {} [item] => item.indented_display(w), items => { - w.write_recursive(|mut f| { + w.write_indented(|mut f| { for item in items { - f.write_line(|mut l| { + f.write_line_prefix(|mut l| { item.indented_display(l.reborrow()); l.write(","); }); @@ -271,9 +281,9 @@ where Value::Map(items) => { w.write("{"); if !items.is_empty() { - w.write_recursive(|mut f| { + w.write_indented(|mut f| { for (key, value) in items { - f.write_line(|mut l| { + f.write_line_prefix(|mut l| { key.indented_display(l.reborrow()); l.write(": "); value.indented_display(l.reborrow()); @@ -296,9 +306,9 @@ where FieldsKind::Named(items) => { w.write(" {"); if !items.is_empty() { - w.write_recursive(|mut f| { + w.write_indented(|mut f| { for (field, value) in items { - f.write_line(|mut l| { + f.write_line_prefix(|mut l| { l.fmt(field); l.write(": "); value.indented_display(l.reborrow()); @@ -416,9 +426,9 @@ where } _ => { w.write("#["); - w.write_recursive(|mut f| { + w.write_indented(|mut f| { for (ident, val) in self.iter() { - f.write_line(|mut w| { + f.write_line_prefix(|mut w| { w.fmt(ident); w.write(" = "); val.indented_display(w.reborrow()); @@ -500,6 +510,8 @@ impl IndentedDisplay> for Val { w.write(" "); } + // If the type is transparent over a trait type or a ref to a trait type, then the concrete + // type of the inner value will not be apparent from the outer context, so `typed` is false. let typed = match w.ctx().types.key_type(*self.ty).kind { TypeKind::Transparent(t) | TypeKind::Ref(t) => { !matches!(w.ctx().types.key_type(t).kind, TypeKind::Trait(_)) @@ -516,6 +528,7 @@ impl IndentedDisplay> for Val { let mut w = w.with_type(path.as_str()); + // If we are not in a typed context the type may need to be explicitly specified. if !w.is_typed() && !path.is_empty() && match w.ctx().types.key_type(*self.ty).kind { diff --git a/bauble/src/value/mod.rs b/bauble/src/value/mod.rs index a541be8..9379e01 100644 --- a/bauble/src/value/mod.rs +++ b/bauble/src/value/mod.rs @@ -519,7 +519,12 @@ impl Value { #[allow(missing_docs)] #[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 value: Inner, } @@ -528,6 +533,7 @@ impl Object { pub fn into_unspanned(self) -> Object { Object { object_path: self.object_path, + top_level: self.top_level, value: self.value.into_unspanned(), } } @@ -566,11 +572,12 @@ 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. -pub struct DelayedRegister { - pub asset: Spanned, - pub reference: Spanned, +pub(crate) struct DelayedRegister { + asset: Spanned, + asset_kind: crate::AssetKind, + reference: Spanned, /// The type we want a potential reference to resolve into. - pub expected_ty_path: Option>, + expected_ty_path: Option>, } pub(crate) fn resolve_delayed( @@ -590,7 +597,7 @@ pub(crate) fn resolve_delayed( PathKind::Indirect(path, ident) => { ctx.ref_with_ident(path.borrow(), ident.borrow()) } - } && let Some((ty, _)) = &r.asset + } && 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 @@ -640,7 +647,9 @@ pub(crate) fn resolve_delayed( ); } } - ctx.register_asset(d.asset.value.borrow(), *ty); + if let Err(e) = ctx.register_asset(d.asset.value.borrow(), *ty, d.asset_kind) { + errors.push(ConversionError::Custom(e).spanned(d.asset.span)) + } false } else { true @@ -717,8 +726,32 @@ pub(crate) fn resolve_delayed( } } +/// Computes the path of an object. +/// +/// Normally, the path is just the files's bauble path joined with the object name. +/// +/// 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( + 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) + } +} + pub(crate) fn register_assets( - path: TypePath<&str>, + file_path: TypePath<&str>, ctx: &mut crate::context::BaubleContext, values: &ParseValues, ) -> std::result::Result, Vec>> { @@ -742,7 +775,13 @@ pub(crate) fn register_assets( for (ident, binding) in &values.values { let span = ident.span; let ident = &TypePathElem::new(ident.as_str()).expect("Invariant"); - let path = path.join(ident); + let path = object_path(file_path, ident, binding); + let top_level = path.borrow() == file_path; + let kind = if top_level { + crate::AssetKind::TopLevel + } else { + crate::AssetKind::Local + }; let symbols = Symbols { ctx: &*ctx, uses }; // To register an asset we need to determine its type. @@ -797,6 +836,7 @@ pub(crate) fn register_assets( delayed.push(DelayedRegister { expected_ty_path, + asset_kind: kind, asset: path.spanned(span), reference, }); @@ -807,10 +847,13 @@ pub(crate) fn register_assets( res }; - match ty { - Ok(ty) => { - Symbols { uses, .. } = symbols; - let ref_ty = ctx.register_asset(path.borrow(), ty); + Symbols { uses, .. } = symbols; + let ref_ty = ty.and_then(|ty| { + 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. @@ -818,7 +861,7 @@ pub(crate) fn register_assets( ident.to_owned(), PathReference { ty: None, - asset: Some((ref_ty, path.clone())), + asset: Some((ref_ty, path.clone(), kind)), module: None, }, ) { @@ -826,10 +869,7 @@ pub(crate) fn register_assets( } Symbols { uses, .. } = symbols; } - Err(err) => { - Symbols { uses, .. } = symbols; - errors.push(err) - } + Err(err) => errors.push(err), }; } @@ -855,12 +895,13 @@ pub(crate) fn convert_values( let mut symbols = default_symbols.clone(); - let path = symbols.ctx.get_file_path(file); + let file_path = symbols.ctx.get_file_path(file); // Add asset - for (symbol, _) in &values.values { - let ident = TypePathElem::new(symbol.as_str()).expect("Invariant"); - let path = path.join(&ident); + 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); if let Some(PathReference { asset: Some(asset), .. @@ -874,31 +915,20 @@ pub(crate) fn convert_values( module: None, }, ) { - use_errors.push(e.spanned(symbol.span)); + use_errors.push(e.spanned(span)); } } else { // Didn't pre-register assets. - use_errors.push(ConversionError::UnregisteredAsset.spanned(symbol.span)); + use_errors.push(ConversionError::UnregisteredAsset.spanned(span)); } } symbols.add(use_symbols); - let mut additional_objects = AdditionalObjects::new(path.to_owned()); + let mut additional_objects = AdditionalObjects::new(file_path.to_owned()); let default_span = crate::Span::new(file, 0..0); - macro_rules! meta { - ($name:expr) => { - ConvertMeta { - symbols: &symbols, - additional_objects: &mut additional_objects, - object_name: $name, - default_span, - } - }; - } - let mut ok = Vec::new(); let mut err = use_errors; @@ -923,8 +953,16 @@ pub(crate) fn convert_values( }; let ident = TypePathElem::new(ident.as_str()).expect("Invariant"); + let path = object_path(file_path, &ident, binding); - match convert_object(path, &binding.value, ty, meta!(ident)) { + let convert_meta = ConvertMeta { + symbols: &symbols, + additional_objects: &mut additional_objects, + 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), } @@ -943,24 +981,26 @@ 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( - path: TypePath<&str>, + object_path: TypePath, + top_level: bool, value: &ParseVal, expected_type: TypeId, mut meta: ConvertMeta, ) -> Result { let value = value.convert(meta.reborrow(), expected_type, no_attr())?; - create_object(path, meta.object_name, value, meta.symbols) + create_object(object_path, top_level, value, meta.symbols) } fn create_object( - path: TypePath<&str>, - name: TypePathElem<&str>, + object_path: TypePath, + top_level: bool, value: Val, symbols: &Symbols, ) -> Result { if symbols.ctx.type_registry().impls_top_level_trait(*value.ty) { Ok(Object { - object_path: path.join(&name), + object_path, + top_level, value, }) } else { diff --git a/bauble/src/value/symbols.rs b/bauble/src/value/symbols.rs index 9630b55..5125eb4 100644 --- a/bauble/src/value/symbols.rs +++ b/bauble/src/value/symbols.rs @@ -329,30 +329,34 @@ impl<'a> Symbols<'a> { } pub fn resolve_asset(&self, path: &Path) -> Result<(TypeId, TypePath)> { - let item = self.resolve_item(path, RefKind::Asset)?; + let item = self.resolve_item(path, RefKind::Asset)?.into_owned(); - item.asset.clone().ok_or( - ConversionError::RefError(Box::new(RefError { + 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)?.value, - path_ref: item.into_owned(), + path_ref: item, kind: RefKind::Asset, })) - .spanned(path.span()), - ) + .spanned(path.span())) + } } pub fn resolve_type(&self, path: &Path) -> Result { let item = self.resolve_item(path, RefKind::Type)?; - item.ty.ok_or( - ConversionError::RefError(Box::new(RefError { + 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)?.value, path_ref: item.into_owned(), kind: RefKind::Type, })) - .spanned(path.span()), - ) + .spanned(path.span())) + } } } diff --git a/bauble/tests/integration.rs b/bauble/tests/integration.rs index 1eea570..3e7b6fd 100644 --- a/bauble/tests/integration.rs +++ b/bauble/tests/integration.rs @@ -33,13 +33,27 @@ struct TestFile { expected_values: Vec>, } +impl TestFile { + fn new( + path: &str, + content: &str, + expected_values: Vec>, + ) -> Self { + Self { + path: TypePath::new(String::from(path)).unwrap(), + content: String::from(content), + expected_values, + } + } +} + macro_rules! test_file { ($path:expr, $content:expr, $($expected_value:expr),* $(,)?) => { - TestFile { - path: TypePath::new(String::from($path)).unwrap(), - content: String::from($content), - expected_values: vec![$(expected_value_fn($expected_value)),*], - } + TestFile::new( + $path, + $content, + vec![$(expected_value_fn($expected_value)),*], + ) }; } @@ -391,20 +405,20 @@ fn reference_with_use() { pub fn ref_implicit_type() { bauble::bauble_test!( [Test] - "test = integration::Test{ x: -5, y: 5 }\n\ - r = $test" + "t = integration::Test{ x: -5, y: 5 }\n\ + r = $t" [ Test { x: -5, y: 5 }, - Ref::::from_path(TypePath::new_unchecked("test::test").to_owned()), + Ref::::from_path(TypePath::new_unchecked("test::t").to_owned()), ] ); bauble::bauble_test!( [Test] - "r = $test::test\n\ - test = integration::Test{ x: -5, y: 5 }" + "r = $test::t\n\ + t = integration::Test{ x: -5, y: 5 }" [ - Ref::::from_path(TypePath::new_unchecked("test::test").to_owned()), + Ref::::from_path(TypePath::new_unchecked("test::t").to_owned()), Test { x: -5, y: 5 }, ] ); @@ -414,20 +428,20 @@ pub fn ref_implicit_type() { pub fn ref_explicit_type() { bauble::bauble_test!( [Test] - "test = integration::Test{ x: -2, y: 2 }\n\ - r: Ref = $test" + "t = integration::Test{ x: -2, y: 2 }\n\ + r: Ref = $t" [ Test { x: -2, y: 2 }, - Ref::::from_path(TypePath::new_unchecked("test::test").to_owned()), + Ref::::from_path(TypePath::new_unchecked("test::t").to_owned()), ] ); bauble::bauble_test!( [Test] - "r: Ref = $test::test\n\ - test = integration::Test{ x: -2, y: 2 }" + "r: Ref = $test::t\n\ + t = integration::Test{ x: -2, y: 2 }" [ - Ref::::from_path(TypePath::new_unchecked("test::test").to_owned()), + Ref::::from_path(TypePath::new_unchecked("test::t").to_owned()), Test { x: -2, y: 2 }, ] ); @@ -438,23 +452,23 @@ pub fn ref_explicit_type_multiple_files() { bauble::bauble_test!( [Test] [ - "test = integration::Test{ x: -5, y: 5 }", - "r: Ref = $test0::test" + "t = integration::Test{ x: -5, y: 5 }", + "r: Ref = $test0::t" ] [ Test { x: -5, y: 5 }, - Ref::::from_path(TypePath::new_unchecked("test0::test").to_owned()), + Ref::::from_path(TypePath::new_unchecked("test0::t").to_owned()), ] ); bauble::bauble_test!( [Test] [ - "r: Ref = $test1::test", - "test = integration::Test{ x: -5, y: 5 }" + "r: Ref = $test1::t", + "t = integration::Test{ x: -5, y: 5 }" ] [ - Ref::::from_path(TypePath::new_unchecked("test1::test").to_owned()), + Ref::::from_path(TypePath::new_unchecked("test1::t").to_owned()), Test { x: -5, y: 5 }, ] ); @@ -465,23 +479,23 @@ pub fn ref_implicit_type_multiple_files() { bauble::bauble_test!( [Test] [ - "test = integration::Test{ x: -5, y: 5 }", - "r = $test0::test" + "t = integration::Test{ x: -5, y: 5 }", + "r = $test0::t" ] [ Test { x: -5, y: 5 }, - Ref::::from_path(TypePath::new_unchecked("test0::test").to_owned()), + Ref::::from_path(TypePath::new_unchecked("test0::t").to_owned()), ] ); bauble::bauble_test!( [Test] [ - "r = $test1::test", - "test = integration::Test{ x: -5, y: 5 }" + "r = $test1::t", + "t = integration::Test{ x: -5, y: 5 }" ] [ - Ref::::from_path(TypePath::new_unchecked("test1::test").to_owned()), + Ref::::from_path(TypePath::new_unchecked("test1::t").to_owned()), Test { x: -5, y: 5 }, ] ); @@ -496,11 +510,11 @@ pub fn ref_explicit_type_incorrect() { bauble::bauble_test!( [Test, Incorrect] "i: Incorrect = Incorrect(0)\n\ - r: Ref = $test::test\n\ - test = integration::Test{ x: -2, y: 2 }" + r: Ref = $test::t\n\ + t = integration::Test{ x: -2, y: 2 }" [ Incorrect(0), - Ref::::from_path(TypePath::new_unchecked("test::test").to_owned()), + Ref::::from_path(TypePath::new_unchecked("test::t").to_owned()), Test { x: -2, y: 2 }, ] ); @@ -627,3 +641,117 @@ fn two_part_field() { &[a], ); } + +#[test] +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 + vec![ + Box::new(|object, ctx| { + assert!(object.top_level); + (expected_value_fn(Test { x: -5, y: 5 }))(object, ctx) + }), + expected_value_fn(TestRef("a".into())), + ], + ); + // 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", + vec![ + Box::new(|object, ctx| { + assert!(object.top_level); + (expected_value_fn(Test { x: -5, y: 5 }))(object, ctx) + }), + expected_value_fn(TestRef("a::c".into())), + expected_value_fn(TestRef("a::c".into())), + ], + ); + // test refering to them from a separate file + let b = &test_file!( + "b", + "a_ref = $a\n\ + c_ref = $a::c", + TestRef("a".into()), + TestRef("a::c".into()), + ); + + test_reload( + &|ctx| { + ctx.register_type::(); + }, + &[a, ac, b], + &[a, ac, b], + ); +} + +#[test] +#[should_panic = "'a::1' refers to an existing asset"] +fn duplicate_name_after_simplification() { + let a = &TestFile::new( + "a", + "a = 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| { + assert!(object.top_level); + (expected_value_fn(Test { x: -5, y: 5 }))(object, ctx) + }), + Box::new(|object, ctx| { + assert!(!object.top_level); + (expected_value_fn(Test { x: -5, y: 5 }))(object, ctx) + }), + ], + ); + // test non-top-level file + let a1 = &TestFile::new( + "a::1", + "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, a1], + ); +} + +/// 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"] +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 }", + vec![ + Box::new(|object, ctx| { + assert!(object.top_level); + (expected_value_fn(Test { x: -5, y: 5 }))(object, ctx) + }), + 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], + ); +} + +// TODO: in stage 2, test that only first object can be named `0`. diff --git a/bauble_macro_util/src/lib.rs b/bauble_macro_util/src/lib.rs index 0ecc62d..dfad27d 100644 --- a/bauble_macro_util/src/lib.rs +++ b/bauble_macro_util/src/lib.rs @@ -433,7 +433,7 @@ fn parse_fields( let ident = meta.input.parse::()?; attribute = Some(ident); } else { - attribute = Some(field.ident.clone().ok_or(meta.error("For unnamed fields the attribute specifier needs to be annotated with `attribute = ident`"))?); + attribute = Some(field.ident.clone().ok_or_else(|| meta.error("For unnamed fields the attribute specifier needs to be annotated with `attribute = ident`"))?); } Ok(()) diff --git a/bauble_macros/tests/derive.rs b/bauble_macros/tests/derive.rs index 8841743..7a0911c 100644 --- a/bauble_macros/tests/derive.rs +++ b/bauble_macros/tests/derive.rs @@ -14,7 +14,7 @@ fn test_struct() { bauble_test!( [Test] r#" - test = derive::Test { x: -5, y: 5, z: Some(true) } + a = derive::Test { x: -5, y: 5, z: Some(true) } "# [Test { x: -5, @@ -31,7 +31,7 @@ fn test_tuple() { bauble_test!( [Test] - "test = derive::Test(-5, 5)" + "a = derive::Test(-5, 5)" [Test(-5, 5)] ); } @@ -116,7 +116,7 @@ fn test_std_types() { bauble_test!( [Test] r#" - test = derive::Test { + a = derive::Test { a: [(2, 0), (1, -1), (5, 10)], b: { "🔑": [true, true, false],