Skip to content

Asset per file scheme, stage 2#70

Merged
Imberflur merged 25 commits into
mainfrom
asset-per-file-stage-2
May 8, 2026
Merged

Asset per file scheme, stage 2#70
Imberflur merged 25 commits into
mainfrom
asset-per-file-stage-2

Conversation

@Imberflur

@Imberflur Imberflur commented May 4, 2026

Copy link
Copy Markdown
Contributor

This follows up from #69

Main changes

  • A special 0 identifier is introduced for top level objects so their name does not need to be repeated from the file name.
  • Files now require a top level object unless they are empty.
  • Local objects can no longer be referenced outside of the current file.
  • Local objects no longer registered into BaubleContext (similarly to inline objects). E.g. BaubleContext::assets now only lists top level objects.
  • Object paths now use a use the ObjectPath enum instead of being plain TypePaths. The enum has 3 variants: Top, Local, and Inline. Uses of a top_level bool were replaced with this.

Other changes

  • Resolved various edge cases around registration ordering of assets referencing each other.
  • Added tests for these edge cases.
  • Fix case where explicit type on binding can be ignored for objects that are a referenced asset that is already registered.
  • path::*::ident now produces an error when multiple items could be referenced by it instead of selecting one of those items

Imberflur added 21 commits March 9, 2026 12:22
…added in previous commits), also minimize places using `Symbols`
… statement conflicts with an asset identifiers in the current file
…should_panic test for explicit ref type being wrong when referencing something from another file that is already registered
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.
…re specific when referenced asset fails to be registerer, and test that local assets will be suggsted in errors
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.

@Imberflur Imberflur left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here are notes for reviewers, and a few cleanup tasks for myself.

emitter.emit(Rich::custom(
ident.span,
format!(
"The first item must have '{}' as the identifier",

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We now required the first object to be a top level object and subsequent objects must not use the special identifier.

Comment thread bauble/src/parse/value.rs Outdated
Comment thread bauble/src/types/path.rs
}
}

if path.is_empty() {

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

moved all the code down past the inner function definitions so it is easier to follow

Comment thread bauble/src/types/path.rs Outdated
Comment thread bauble/src/value/convert.rs Outdated
Comment thread bauble/src/context.rs
/// `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?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TODO resolved by producing an error on ambiguity

///
/// Contains registery of local objects. These are objects that are only referencable by other
/// objects in the same file.
pub(crate) struct LocalContext {

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Local objects are now registered in this ephemeral context type instead of BaubleContext. It only exists during the loading process.

This ended up being a super simple type!

Comment thread bauble/src/object_path.rs
///
/// Path format documented in [`TypePath`].
#[derive(Copy, Clone, Debug)]
pub enum ObjectPath<S = String> {

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

New ObjectPath enum, I hope the docs are sufficient explanation.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It might be helpful to have examples of what the contents of each variant might look like.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added a set of examples

Comment thread bauble/src/traits.rs
/// 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;

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

renamed for clarity

let a = &test_file!(
"a",
r#"test = integration::Test { x: -5, y: 5 }"#,
r#"0 = integration::Test { x: -5, y: 5 }"#,

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

all the tests needed updating for the new syntax

@MavethGH

MavethGH commented May 6, 2026

Copy link
Copy Markdown
Contributor

Testing whether I can comment without it being on a specific line.

@Imberflur Imberflur merged commit 967a486 into main May 8, 2026
1 check passed
@Imberflur Imberflur deleted the asset-per-file-stage-2 branch May 15, 2026 17:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants