From 08ea1d18aed1a7996c9abeef566a6bcc5e023fc2 Mon Sep 17 00:00:00 2001 From: Nicola Papale Date: Sun, 30 Jul 2023 21:17:07 +0200 Subject: [PATCH] Refactor `path` module of `bevy_reflect` (#8887) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit # Objective - The `path` module was getting fairly large. - The code in `AccessRef::read_element` and mut equivalent was very complex and difficult to understand. - The `ReflectPathError` had a lot of variants, and was difficult to read. ## Solution - Split the file in two, `access` now has its own module - Rewrite the `read_element` methods, they were ~200 lines long, they are now ~70 lines long — I didn't change any of the logic. It's really just the same code, but error handling is separated. - Split the `ReflectPathError` error - Merge `AccessRef` and `Access` - A few other changes that aim to reduce code complexity ### Fully detailed change list - `Display` impl of `ParsedPath` now includes prefix dots — this allows simplifying its implementation, and IMO `.path.to.field` is a better way to express a "path" than `path.to.field` which could suggest we are reading the `to` field of a variable named `path` - Add a test to check that dot prefixes and other are correctly parsed — Until now, no test contained a prefixing dot - Merge `Access` and `AccessRef`, using a `Cow<'a, str>`. Generated code seems to agree with this decision (`ParsedPath::parse` sheds 5% of instructions) - Remove `Access::as_ref` since there is no such thing as an `AccessRef` anymore. - Rename `AccessRef::to_owned` into `AccessRef::into_owned()` since it takes ownership of `self` now. - Add a `parse_static` that doesn't allocate new strings for named fields! - Add a section about path reflection in the `bevy_reflect` crate root doc — I saw a few people that weren't aware of path reflection, so I thought it was pertinent to add it to the root doc - a lot of nits - rename `index` to `offset` when it refers to offset in the path string — There is no more confusion with the other kind of indices in this context, also it's a common naming convention for parsing. - Make a dedicated enum for parsing errors - rename the `read_element` methods to `element` — shorter, but also `read_element_mut` was a fairly poor name - The error values now not only contain the expected type but also the actual type. - Remove lifetimes that could be inferred from the `GetPath` trait methods. --- ## Change log - Added the `ParsedPath::parse_static` method, avoids allocating when parsing `&'static str`. ## Migration Guide If you were matching on the `Err(ReflectPathError)` value returned by `GetPath` and `ParsedPath` methods, now only the parse-related errors and the offset are publicly accessible. You can always use the `fmt::Display` to get a clear error message, but if you need programmatic access to the error types, please open an issue. --------- Co-authored-by: Alice Cecile Co-authored-by: Gino Valente <49806985+MrGVSV@users.noreply.github.com> --- crates/bevy_reflect/src/lib.rs | 23 + crates/bevy_reflect/src/path/access.rs | 225 +++++++ .../bevy_reflect/src/{path.rs => path/mod.rs} | 579 ++++++------------ 3 files changed, 423 insertions(+), 404 deletions(-) create mode 100644 crates/bevy_reflect/src/path/access.rs rename crates/bevy_reflect/src/{path.rs => path/mod.rs} (56%) diff --git a/crates/bevy_reflect/src/lib.rs b/crates/bevy_reflect/src/lib.rs index 9a70da803f215..bb0a012888d6e 100644 --- a/crates/bevy_reflect/src/lib.rs +++ b/crates/bevy_reflect/src/lib.rs @@ -229,6 +229,29 @@ //! //! All primitives and simple types implement `FromReflect` by relying on their [`Default`] implementation. //! +//! # Path navigation +//! +//! The [`GetPath`] trait allows accessing arbitrary nested fields of a [`Reflect`] type. +//! +//! Using `GetPath`, it is possible to use a path string to access a specific field +//! of a reflected type. +//! +//! ``` +//! # use bevy_reflect::{Reflect, GetPath}; +//! #[derive(Reflect)] +//! struct MyStruct { +//! value: Vec> +//! } +//! +//! let my_struct = MyStruct { +//! value: vec![None, None, Some(123)], +//! }; +//! assert_eq!( +//! my_struct.path::(".value[2].0").unwrap(), +//! &123, +//! ); +//! ``` +//! //! # Type Registration //! //! This crate also comes with a [`TypeRegistry`] that can be used to store and retrieve additional type metadata at runtime, diff --git a/crates/bevy_reflect/src/path/access.rs b/crates/bevy_reflect/src/path/access.rs new file mode 100644 index 0000000000000..204e99a472986 --- /dev/null +++ b/crates/bevy_reflect/src/path/access.rs @@ -0,0 +1,225 @@ +use std::{borrow::Cow, fmt}; + +use super::{AccessError, ReflectPathError}; +use crate::{Reflect, ReflectMut, ReflectRef, VariantType}; +use thiserror::Error; + +type InnerResult = Result, Error<'static>>; + +#[derive(Debug, PartialEq, Eq, Error)] +pub(super) enum Error<'a> { + #[error( + "the current {ty} doesn't have the {} {}", + access.kind(), + access.display_value(), + )] + Access { ty: TypeShape, access: Access<'a> }, + + #[error("invalid type shape: expected {expected} but found a reflect {actual}")] + Type { + expected: TypeShape, + actual: TypeShape, + }, + + #[error("invalid enum access: expected {expected} variant but found {actual} variant")] + Enum { + expected: TypeShape, + actual: TypeShape, + }, +} + +impl<'a> Error<'a> { + fn with_offset(self, offset: usize) -> ReflectPathError<'a> { + let error = AccessError(self); + ReflectPathError::InvalidAccess { offset, error } + } + + fn access(ty: TypeShape, access: Access<'a>) -> Self { + Self::Access { ty, access } + } +} +impl Error<'static> { + fn bad_enum_variant(expected: TypeShape, actual: impl Into) -> Self { + let actual = actual.into(); + Error::Enum { expected, actual } + } + fn bad_type(expected: TypeShape, actual: impl Into) -> Self { + let actual = actual.into(); + Error::Type { expected, actual } + } +} + +#[derive(Debug, Copy, Clone, PartialEq, Eq)] +pub(super) enum TypeShape { + Struct, + TupleStruct, + Tuple, + List, + Array, + Map, + Enum, + Value, + Unit, +} + +impl fmt::Display for TypeShape { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + let name = match self { + TypeShape::Struct => "struct", + TypeShape::TupleStruct => "tuple struct", + TypeShape::Tuple => "tuple", + TypeShape::List => "list", + TypeShape::Array => "array", + TypeShape::Map => "map", + TypeShape::Enum => "enum", + TypeShape::Value => "value", + TypeShape::Unit => "unit", + }; + write!(f, "{name}") + } +} +impl<'a> From> for TypeShape { + fn from(value: ReflectRef<'a>) -> Self { + match value { + ReflectRef::Struct(_) => TypeShape::Struct, + ReflectRef::TupleStruct(_) => TypeShape::TupleStruct, + ReflectRef::Tuple(_) => TypeShape::Tuple, + ReflectRef::List(_) => TypeShape::List, + ReflectRef::Array(_) => TypeShape::Array, + ReflectRef::Map(_) => TypeShape::Map, + ReflectRef::Enum(_) => TypeShape::Enum, + ReflectRef::Value(_) => TypeShape::Value, + } + } +} +impl From for TypeShape { + fn from(value: VariantType) -> Self { + match value { + VariantType::Struct => TypeShape::Struct, + VariantType::Tuple => TypeShape::Tuple, + VariantType::Unit => TypeShape::Unit, + } + } +} + +/// A singular element access within a path. +/// +/// Can be applied to a `dyn Reflect` to get a reference to the targeted element. +#[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Ord, Hash)] +pub(super) enum Access<'a> { + Field(Cow<'a, str>), + FieldIndex(usize), + TupleIndex(usize), + ListIndex(usize), +} + +impl fmt::Display for Access<'_> { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + match self { + Access::Field(field) => write!(f, ".{field}"), + Access::FieldIndex(index) => write!(f, "#{index}"), + Access::TupleIndex(index) => write!(f, ".{index}"), + Access::ListIndex(index) => write!(f, "[{index}]"), + } + } +} + +impl<'a> Access<'a> { + pub(super) fn into_owned(self) -> Access<'static> { + match self { + Self::Field(value) => Access::Field(value.to_string().into()), + Self::FieldIndex(value) => Access::FieldIndex(value), + Self::TupleIndex(value) => Access::TupleIndex(value), + Self::ListIndex(value) => Access::ListIndex(value), + } + } + + fn display_value(&self) -> &dyn fmt::Display { + match self { + Self::Field(value) => value, + Self::FieldIndex(value) | Self::TupleIndex(value) | Self::ListIndex(value) => value, + } + } + fn kind(&self) -> &'static str { + match self { + Self::Field(_) => "field", + Self::FieldIndex(_) => "field index", + Self::TupleIndex(_) | Self::ListIndex(_) => "index", + } + } + + pub(super) fn element<'r>( + &self, + base: &'r dyn Reflect, + offset: usize, + ) -> Result<&'r dyn Reflect, ReflectPathError<'a>> { + let ty = base.reflect_ref().into(); + self.element_inner(base) + .and_then(|maybe| maybe.ok_or(Error::access(ty, self.clone()))) + .map_err(|err| err.with_offset(offset)) + } + + fn element_inner<'r>(&self, base: &'r dyn Reflect) -> InnerResult<&'r dyn Reflect> { + use ReflectRef::*; + match (self, base.reflect_ref()) { + (Self::Field(field), Struct(struct_ref)) => Ok(struct_ref.field(field.as_ref())), + (Self::Field(field), Enum(enum_ref)) => match enum_ref.variant_type() { + VariantType::Struct => Ok(enum_ref.field(field.as_ref())), + actual => Err(Error::bad_enum_variant(TypeShape::Struct, actual)), + }, + (&Self::FieldIndex(index), Struct(struct_ref)) => Ok(struct_ref.field_at(index)), + (&Self::FieldIndex(index), Enum(enum_ref)) => match enum_ref.variant_type() { + VariantType::Struct => Ok(enum_ref.field_at(index)), + actual => Err(Error::bad_enum_variant(TypeShape::Struct, actual)), + }, + (&Self::TupleIndex(index), TupleStruct(tuple)) => Ok(tuple.field(index)), + (&Self::TupleIndex(index), Tuple(tuple)) => Ok(tuple.field(index)), + (&Self::TupleIndex(index), Enum(enum_ref)) => match enum_ref.variant_type() { + VariantType::Tuple => Ok(enum_ref.field_at(index)), + actual => Err(Error::bad_enum_variant(TypeShape::Tuple, actual)), + }, + (&Self::ListIndex(index), List(list)) => Ok(list.get(index)), + (&Self::ListIndex(index), Array(list)) => Ok(list.get(index)), + (&Self::ListIndex(_), actual) => Err(Error::bad_type(TypeShape::List, actual)), + (_, actual) => Err(Error::bad_type(TypeShape::Struct, actual)), + } + } + + pub(super) fn element_mut<'r>( + &self, + base: &'r mut dyn Reflect, + offset: usize, + ) -> Result<&'r mut dyn Reflect, ReflectPathError<'a>> { + let ty = base.reflect_ref().into(); + self.element_inner_mut(base) + .and_then(|maybe| maybe.ok_or(Error::access(ty, self.clone()))) + .map_err(|err| err.with_offset(offset)) + } + + fn element_inner_mut<'r>(&self, base: &'r mut dyn Reflect) -> InnerResult<&'r mut dyn Reflect> { + use ReflectMut::*; + let base_shape: TypeShape = base.reflect_ref().into(); + match (self, base.reflect_mut()) { + (Self::Field(field), Struct(struct_mut)) => Ok(struct_mut.field_mut(field.as_ref())), + (Self::Field(field), Enum(enum_mut)) => match enum_mut.variant_type() { + VariantType::Struct => Ok(enum_mut.field_mut(field.as_ref())), + actual => Err(Error::bad_enum_variant(TypeShape::Struct, actual)), + }, + (&Self::FieldIndex(index), Struct(struct_mut)) => Ok(struct_mut.field_at_mut(index)), + (&Self::FieldIndex(index), Enum(enum_mut)) => match enum_mut.variant_type() { + VariantType::Struct => Ok(enum_mut.field_at_mut(index)), + actual => Err(Error::bad_enum_variant(TypeShape::Struct, actual)), + }, + (&Self::TupleIndex(index), TupleStruct(tuple)) => Ok(tuple.field_mut(index)), + (&Self::TupleIndex(index), Tuple(tuple)) => Ok(tuple.field_mut(index)), + (&Self::TupleIndex(index), Enum(enum_mut)) => match enum_mut.variant_type() { + VariantType::Tuple => Ok(enum_mut.field_at_mut(index)), + actual => Err(Error::bad_enum_variant(TypeShape::Tuple, actual)), + }, + (&Self::ListIndex(index), List(list)) => Ok(list.get_mut(index)), + (&Self::ListIndex(index), Array(list)) => Ok(list.get_mut(index)), + (&Self::ListIndex(_), _) => Err(Error::bad_type(TypeShape::List, base_shape)), + (_, _) => Err(Error::bad_type(TypeShape::Struct, base_shape)), + } + } +} diff --git a/crates/bevy_reflect/src/path.rs b/crates/bevy_reflect/src/path/mod.rs similarity index 56% rename from crates/bevy_reflect/src/path.rs rename to crates/bevy_reflect/src/path/mod.rs index a9fd7ffe2febe..6ea95da503b11 100644 --- a/crates/bevy_reflect/src/path.rs +++ b/crates/bevy_reflect/src/path/mod.rs @@ -1,50 +1,49 @@ +mod access; + use std::fmt; use std::num::ParseIntError; -use crate::{Reflect, ReflectMut, ReflectRef, VariantType}; +use crate::Reflect; +use access::Access; use thiserror::Error; -/// An error returned from a failed path string query. +type ParseResult = Result; + +/// An error specific to accessing a field/index on a `Reflect`. #[derive(Debug, PartialEq, Eq, Error)] -pub enum ReflectPathError<'a> { - #[error("expected an identifier at index {index}")] - ExpectedIdent { index: usize }, - #[error("the current struct doesn't have a field with the name `{field}`")] - InvalidField { index: usize, field: &'a str }, - #[error("the current struct doesn't have a field at the given index")] - InvalidFieldIndex { index: usize, field_index: usize }, - #[error("the current tuple struct doesn't have a field with the index {tuple_struct_index}")] - InvalidTupleStructIndex { - index: usize, - tuple_struct_index: usize, - }, - #[error("the current tuple doesn't have a field with the index {tuple_index}")] - InvalidTupleIndex { index: usize, tuple_index: usize }, - #[error("the current struct variant doesn't have a field with the name `{field}`")] - InvalidStructVariantField { index: usize, field: &'a str }, - #[error("the current tuple variant doesn't have a field with the index {tuple_variant_index}")] - InvalidTupleVariantIndex { - index: usize, - tuple_variant_index: usize, - }, - #[error("the current list doesn't have a value at the index {list_index}")] - InvalidListIndex { index: usize, list_index: usize }, +#[error(transparent)] +pub struct AccessError<'a>(access::Error<'a>); + +/// A parse error for a path string. +#[derive(Debug, PartialEq, Eq, Error)] +pub enum ReflectPathParseError { + #[error("expected an identifier at offset {offset}")] + ExpectedIdent { offset: usize }, + #[error("encountered an unexpected token `{token}`")] - UnexpectedToken { index: usize, token: &'a str }, + UnexpectedToken { offset: usize, token: &'static str }, + #[error("expected token `{token}`, but it wasn't there.")] - ExpectedToken { index: usize, token: &'a str }, - #[error("expected a struct, but found a different reflect value")] - ExpectedStruct { index: usize }, - #[error("expected a list, but found a different reflect value")] - ExpectedList { index: usize }, - #[error("expected a struct variant, but found a different reflect value")] - ExpectedStructVariant { index: usize }, - #[error("expected a tuple variant, but found a different reflect value")] - ExpectedTupleVariant { index: usize }, + ExpectedToken { offset: usize, token: &'static str }, + #[error("failed to parse a usize")] IndexParseError(#[from] ParseIntError), +} + +/// An error returned from a failed path string query. +#[derive(Debug, PartialEq, Eq, Error)] +pub enum ReflectPathError<'a> { + #[error("{error}")] + InvalidAccess { + /// Position in the path string. + offset: usize, + error: AccessError<'a>, + }, #[error("failed to downcast to the path result to the given type")] InvalidDowncast, + + #[error(transparent)] + Parse(#[from] ReflectPathParseError), } /// A trait which allows nested [`Reflect`] values to be retrieved with path strings. @@ -191,19 +190,16 @@ pub trait GetPath { /// /// To retrieve a statically typed reference, use /// [`path`][GetPath::path]. - fn reflect_path<'r, 'p>( - &'r self, - path: &'p str, - ) -> Result<&'r dyn Reflect, ReflectPathError<'p>>; + fn reflect_path<'p>(&self, path: &'p str) -> Result<&dyn Reflect, ReflectPathError<'p>>; /// Returns a mutable reference to the value specified by `path`. /// /// To retrieve a statically typed mutable reference, use /// [`path_mut`][GetPath::path_mut]. - fn reflect_path_mut<'r, 'p>( - &'r mut self, + fn reflect_path_mut<'p>( + &mut self, path: &'p str, - ) -> Result<&'r mut dyn Reflect, ReflectPathError<'p>>; + ) -> Result<&mut dyn Reflect, ReflectPathError<'p>>; /// Returns a statically typed reference to the value specified by `path`. /// @@ -212,7 +208,7 @@ pub trait GetPath { /// (which may be the case when using dynamic types like [`DynamicStruct`]). /// /// [`DynamicStruct`]: crate::DynamicStruct - fn path<'r, 'p, T: Reflect>(&'r self, path: &'p str) -> Result<&'r T, ReflectPathError<'p>> { + fn path<'p, T: Reflect>(&self, path: &'p str) -> Result<&T, ReflectPathError<'p>> { self.reflect_path(path).and_then(|p| { p.downcast_ref::() .ok_or(ReflectPathError::InvalidDowncast) @@ -226,10 +222,7 @@ pub trait GetPath { /// (which may be the case when using dynamic types like [`DynamicStruct`]). /// /// [`DynamicStruct`]: crate::DynamicStruct - fn path_mut<'r, 'p, T: Reflect>( - &'r mut self, - path: &'p str, - ) -> Result<&'r mut T, ReflectPathError<'p>> { + fn path_mut<'p, T: Reflect>(&mut self, path: &'p str) -> Result<&mut T, ReflectPathError<'p>> { self.reflect_path_mut(path).and_then(|p| { p.downcast_mut::() .ok_or(ReflectPathError::InvalidDowncast) @@ -238,40 +231,34 @@ pub trait GetPath { } impl GetPath for T { - fn reflect_path<'r, 'p>( - &'r self, - path: &'p str, - ) -> Result<&'r dyn Reflect, ReflectPathError<'p>> { + fn reflect_path<'p>(&self, path: &'p str) -> Result<&dyn Reflect, ReflectPathError<'p>> { (self as &dyn Reflect).reflect_path(path) } - fn reflect_path_mut<'r, 'p>( - &'r mut self, + fn reflect_path_mut<'p>( + &mut self, path: &'p str, - ) -> Result<&'r mut dyn Reflect, ReflectPathError<'p>> { + ) -> Result<&mut dyn Reflect, ReflectPathError<'p>> { (self as &mut dyn Reflect).reflect_path_mut(path) } } impl GetPath for dyn Reflect { - fn reflect_path<'r, 'p>( - &'r self, - path: &'p str, - ) -> Result<&'r dyn Reflect, ReflectPathError<'p>> { + fn reflect_path<'p>(&self, path: &'p str) -> Result<&dyn Reflect, ReflectPathError<'p>> { let mut current: &dyn Reflect = self; - for (access, current_index) in PathParser::new(path) { - current = access?.read_element(current, current_index)?; + for (access, offset) in PathParser::new(path) { + current = access?.element(current, offset)?; } Ok(current) } - fn reflect_path_mut<'r, 'p>( - &'r mut self, + fn reflect_path_mut<'p>( + &mut self, path: &'p str, - ) -> Result<&'r mut dyn Reflect, ReflectPathError<'p>> { + ) -> Result<&mut dyn Reflect, ReflectPathError<'p>> { let mut current: &mut dyn Reflect = self; - for (access, current_index) in PathParser::new(path) { - current = access?.read_element_mut(current, current_index)?; + for (access, offset) in PathParser::new(path) { + current = access?.element_mut(current, offset)?; } Ok(current) } @@ -292,7 +279,7 @@ pub struct ParsedPath( /// index of the start of the access within the parsed path string. /// /// The index is mainly used for more helpful error reporting. - Box<[(Access, usize)]>, + Box<[(Access<'static>, usize)]>, ); impl ParsedPath { @@ -343,8 +330,18 @@ impl ParsedPath { /// pub fn parse(string: &str) -> Result> { let mut parts = Vec::new(); - for (access, idx) in PathParser::new(string) { - parts.push((access?.to_owned(), idx)); + for (access, offset) in PathParser::new(string) { + parts.push((access?.into_owned(), offset)); + } + Ok(Self(parts.into_boxed_slice())) + } + + /// Similar to [`Self::parse`] but only works on `&'static str` + /// and does not allocate per named field. + pub fn parse_static(string: &'static str) -> Result> { + let mut parts = Vec::new(); + for (access, offset) in PathParser::new(string) { + parts.push((access?, offset)); } Ok(Self(parts.into_boxed_slice())) } @@ -359,8 +356,8 @@ impl ParsedPath { root: &'r dyn Reflect, ) -> Result<&'r dyn Reflect, ReflectPathError<'p>> { let mut current = root; - for (access, current_index) in self.0.iter() { - current = access.to_ref().read_element(current, *current_index)?; + for (access, offset) in self.0.iter() { + current = access.element(current, *offset)?; } Ok(current) } @@ -375,8 +372,8 @@ impl ParsedPath { root: &'r mut dyn Reflect, ) -> Result<&'r mut dyn Reflect, ReflectPathError<'p>> { let mut current = root; - for (access, current_index) in self.0.iter() { - current = access.to_ref().read_element_mut(current, *current_index)?; + for (access, offset) in self.0.iter() { + current = access.element_mut(current, *offset)?; } Ok(current) } @@ -414,272 +411,13 @@ impl ParsedPath { impl fmt::Display for ParsedPath { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - for (idx, (access, _)) in self.0.iter().enumerate() { - match access { - Access::Field(field) => { - if idx != 0 { - Token::DOT.fmt(f)?; - } - f.write_str(field.as_str())?; - } - Access::FieldIndex(index) => { - Token::CROSSHATCH.fmt(f)?; - index.fmt(f)?; - } - Access::TupleIndex(index) => { - if idx != 0 { - Token::DOT.fmt(f)?; - } - index.fmt(f)?; - } - Access::ListIndex(index) => { - Token::OPEN_BRACKET.fmt(f)?; - index.fmt(f)?; - Token::CLOSE_BRACKET.fmt(f)?; - } - } + for (access, _) in self.0.iter() { + write!(f, "{access}")?; } Ok(()) } } -/// A singular owned element access within a path. -/// -/// Can be applied to a `dyn Reflect` to get a reference to the targeted element. -/// -/// A path is composed of multiple accesses in sequence. -#[derive(Debug, Clone, PartialEq, Eq, Hash, PartialOrd, Ord)] -enum Access { - Field(String), - FieldIndex(usize), - TupleIndex(usize), - ListIndex(usize), -} - -impl Access { - fn to_ref(&self) -> AccessRef<'_> { - match self { - Self::Field(value) => AccessRef::Field(value), - Self::FieldIndex(value) => AccessRef::FieldIndex(*value), - Self::TupleIndex(value) => AccessRef::TupleIndex(*value), - Self::ListIndex(value) => AccessRef::ListIndex(*value), - } - } -} - -/// A singular borrowed element access within a path. -/// -/// Can be applied to a `dyn Reflect` to get a reference to the targeted element. -/// -/// Does not own the backing store it's sourced from. -/// For an owned version, you can convert one to an [`Access`]. -#[derive(Debug)] -enum AccessRef<'a> { - Field(&'a str), - FieldIndex(usize), - TupleIndex(usize), - ListIndex(usize), -} - -impl<'a> AccessRef<'a> { - fn to_owned(&self) -> Access { - match self { - Self::Field(value) => Access::Field(value.to_string()), - Self::FieldIndex(value) => Access::FieldIndex(*value), - Self::TupleIndex(value) => Access::TupleIndex(*value), - Self::ListIndex(value) => Access::ListIndex(*value), - } - } - - fn read_element<'r>( - &self, - current: &'r dyn Reflect, - current_index: usize, - ) -> Result<&'r dyn Reflect, ReflectPathError<'a>> { - match (self, current.reflect_ref()) { - (Self::Field(field), ReflectRef::Struct(reflect_struct)) => reflect_struct - .field(field) - .ok_or(ReflectPathError::InvalidField { - index: current_index, - field, - }), - (Self::FieldIndex(field_index), ReflectRef::Struct(reflect_struct)) => reflect_struct - .field_at(*field_index) - .ok_or(ReflectPathError::InvalidFieldIndex { - index: current_index, - field_index: *field_index, - }), - (Self::TupleIndex(tuple_index), ReflectRef::TupleStruct(reflect_struct)) => { - reflect_struct.field(*tuple_index).ok_or( - ReflectPathError::InvalidTupleStructIndex { - index: current_index, - tuple_struct_index: *tuple_index, - }, - ) - } - (Self::TupleIndex(tuple_index), ReflectRef::Tuple(reflect_tuple)) => reflect_tuple - .field(*tuple_index) - .ok_or(ReflectPathError::InvalidTupleIndex { - index: current_index, - tuple_index: *tuple_index, - }), - (Self::ListIndex(list_index), ReflectRef::List(reflect_list)) => reflect_list - .get(*list_index) - .ok_or(ReflectPathError::InvalidListIndex { - index: current_index, - list_index: *list_index, - }), - (Self::ListIndex(list_index), ReflectRef::Array(reflect_list)) => reflect_list - .get(*list_index) - .ok_or(ReflectPathError::InvalidListIndex { - index: current_index, - list_index: *list_index, - }), - (Self::ListIndex(_), _) => Err(ReflectPathError::ExpectedList { - index: current_index, - }), - (Self::Field(field), ReflectRef::Enum(reflect_enum)) => { - match reflect_enum.variant_type() { - VariantType::Struct => { - reflect_enum - .field(field) - .ok_or(ReflectPathError::InvalidField { - index: current_index, - field, - }) - } - _ => Err(ReflectPathError::ExpectedStructVariant { - index: current_index, - }), - } - } - (Self::FieldIndex(field_index), ReflectRef::Enum(reflect_enum)) => { - match reflect_enum.variant_type() { - VariantType::Struct => reflect_enum.field_at(*field_index).ok_or( - ReflectPathError::InvalidFieldIndex { - index: current_index, - field_index: *field_index, - }, - ), - _ => Err(ReflectPathError::ExpectedStructVariant { - index: current_index, - }), - } - } - (Self::TupleIndex(tuple_variant_index), ReflectRef::Enum(reflect_enum)) => { - match reflect_enum.variant_type() { - VariantType::Tuple => reflect_enum.field_at(*tuple_variant_index).ok_or( - ReflectPathError::InvalidTupleVariantIndex { - index: current_index, - tuple_variant_index: *tuple_variant_index, - }, - ), - _ => Err(ReflectPathError::ExpectedTupleVariant { - index: current_index, - }), - } - } - _ => Err(ReflectPathError::ExpectedStruct { - index: current_index, - }), - } - } - - fn read_element_mut<'r>( - &self, - current: &'r mut dyn Reflect, - current_index: usize, - ) -> Result<&'r mut dyn Reflect, ReflectPathError<'a>> { - match (self, current.reflect_mut()) { - (Self::Field(field), ReflectMut::Struct(reflect_struct)) => reflect_struct - .field_mut(field) - .ok_or(ReflectPathError::InvalidField { - index: current_index, - field, - }), - (Self::FieldIndex(field_index), ReflectMut::Struct(reflect_struct)) => reflect_struct - .field_at_mut(*field_index) - .ok_or(ReflectPathError::InvalidFieldIndex { - index: current_index, - field_index: *field_index, - }), - (Self::TupleIndex(tuple_index), ReflectMut::TupleStruct(reflect_struct)) => { - reflect_struct.field_mut(*tuple_index).ok_or( - ReflectPathError::InvalidTupleStructIndex { - index: current_index, - tuple_struct_index: *tuple_index, - }, - ) - } - (Self::TupleIndex(tuple_index), ReflectMut::Tuple(reflect_tuple)) => reflect_tuple - .field_mut(*tuple_index) - .ok_or(ReflectPathError::InvalidTupleIndex { - index: current_index, - tuple_index: *tuple_index, - }), - (Self::ListIndex(list_index), ReflectMut::List(reflect_list)) => reflect_list - .get_mut(*list_index) - .ok_or(ReflectPathError::InvalidListIndex { - index: current_index, - list_index: *list_index, - }), - (Self::ListIndex(list_index), ReflectMut::Array(reflect_list)) => reflect_list - .get_mut(*list_index) - .ok_or(ReflectPathError::InvalidListIndex { - index: current_index, - list_index: *list_index, - }), - (Self::ListIndex(_), _) => Err(ReflectPathError::ExpectedList { - index: current_index, - }), - (Self::Field(field), ReflectMut::Enum(reflect_enum)) => { - match reflect_enum.variant_type() { - VariantType::Struct => { - reflect_enum - .field_mut(field) - .ok_or(ReflectPathError::InvalidField { - index: current_index, - field, - }) - } - _ => Err(ReflectPathError::ExpectedStructVariant { - index: current_index, - }), - } - } - (Self::FieldIndex(field_index), ReflectMut::Enum(reflect_enum)) => { - match reflect_enum.variant_type() { - VariantType::Struct => reflect_enum.field_at_mut(*field_index).ok_or( - ReflectPathError::InvalidFieldIndex { - index: current_index, - field_index: *field_index, - }, - ), - _ => Err(ReflectPathError::ExpectedStructVariant { - index: current_index, - }), - } - } - (Self::TupleIndex(tuple_variant_index), ReflectMut::Enum(reflect_enum)) => { - match reflect_enum.variant_type() { - VariantType::Tuple => reflect_enum.field_at_mut(*tuple_variant_index).ok_or( - ReflectPathError::InvalidTupleVariantIndex { - index: current_index, - tuple_variant_index: *tuple_variant_index, - }, - ), - _ => Err(ReflectPathError::ExpectedTupleVariant { - index: current_index, - }), - } - } - _ => Err(ReflectPathError::ExpectedStruct { - index: current_index, - }), - } - } -} - struct PathParser<'a> { path: &'a str, index: usize, @@ -731,62 +469,62 @@ impl<'a> PathParser<'a> { Some(ident) } - fn token_to_access(&mut self, token: Token<'a>) -> Result, ReflectPathError<'a>> { - let current_index = self.index; + fn token_to_access(&mut self, token: Token<'a>) -> ParseResult> { + let current_offset = self.index; match token { Token::Dot => { if let Some(Token::Ident(value)) = self.next_token() { value .parse::() - .map(AccessRef::TupleIndex) - .or(Ok(AccessRef::Field(value))) + .map(Access::TupleIndex) + .or(Ok(Access::Field(value.into()))) } else { - Err(ReflectPathError::ExpectedIdent { - index: current_index, + Err(ReflectPathParseError::ExpectedIdent { + offset: current_offset, }) } } Token::CrossHatch => { if let Some(Token::Ident(value)) = self.next_token() { - Ok(AccessRef::FieldIndex(value.parse::()?)) + Ok(Access::FieldIndex(value.parse::()?)) } else { - Err(ReflectPathError::ExpectedIdent { - index: current_index, + Err(ReflectPathParseError::ExpectedIdent { + offset: current_offset, }) } } Token::OpenBracket => { let access = if let Some(Token::Ident(value)) = self.next_token() { - AccessRef::ListIndex(value.parse::()?) + Access::ListIndex(value.parse::()?) } else { - return Err(ReflectPathError::ExpectedIdent { - index: current_index, + return Err(ReflectPathParseError::ExpectedIdent { + offset: current_offset, }); }; if !matches!(self.next_token(), Some(Token::CloseBracket)) { - return Err(ReflectPathError::ExpectedToken { - index: current_index, + return Err(ReflectPathParseError::ExpectedToken { + offset: current_offset, token: Token::OPEN_BRACKET_STR, }); } Ok(access) } - Token::CloseBracket => Err(ReflectPathError::UnexpectedToken { - index: current_index, + Token::CloseBracket => Err(ReflectPathParseError::UnexpectedToken { + offset: current_offset, token: Token::CLOSE_BRACKET_STR, }), Token::Ident(value) => value .parse::() - .map(AccessRef::TupleIndex) - .or(Ok(AccessRef::Field(value))), + .map(Access::TupleIndex) + .or(Ok(Access::Field(value.into()))), } } } impl<'a> Iterator for PathParser<'a> { - type Item = (Result, ReflectPathError<'a>>, usize); + type Item = (ParseResult>, usize); fn next(&mut self) -> Option { let token = self.next_token()?; @@ -818,6 +556,7 @@ mod tests { use super::*; use crate as bevy_reflect; use crate::*; + use access::TypeShape; #[derive(Reflect)] struct A { @@ -856,54 +595,69 @@ mod tests { Struct { value: char }, } + fn a_sample() -> A { + A { + w: 1, + x: B { + foo: 10, + bar: C { baz: 3.14 }, + }, + y: vec![C { baz: 1.0 }, C { baz: 2.0 }], + z: D(E(10.0, 42)), + unit_variant: F::Unit, + tuple_variant: F::Tuple(123, 321), + struct_variant: F::Struct { value: 'm' }, + array: [86, 75, 309], + tuple: (true, 1.23), + } + } + + fn access_field(field: &'static str) -> Access { + Access::Field(field.into()) + } + #[test] fn parsed_path_parse() { assert_eq!( &*ParsedPath::parse("w").unwrap().0, - &[(Access::Field("w".to_string()), 1)] + &[(access_field("w"), 1)] ); assert_eq!( &*ParsedPath::parse("x.foo").unwrap().0, - &[ - (Access::Field("x".to_string()), 1), - (Access::Field("foo".to_string()), 2) - ] + &[(access_field("x"), 1), (access_field("foo"), 2)] ); assert_eq!( &*ParsedPath::parse("x.bar.baz").unwrap().0, &[ - (Access::Field("x".to_string()), 1), - (Access::Field("bar".to_string()), 2), - (Access::Field("baz".to_string()), 6) + (access_field("x"), 1), + (access_field("bar"), 2), + (access_field("baz"), 6) ] ); assert_eq!( &*ParsedPath::parse("y[1].baz").unwrap().0, &[ - (Access::Field("y".to_string()), 1), + (access_field("y"), 1), (Access::ListIndex(1), 2), - (Access::Field("baz".to_string()), 5) + (access_field("baz"), 5) ] ); assert_eq!( &*ParsedPath::parse("z.0.1").unwrap().0, &[ - (Access::Field("z".to_string()), 1), + (access_field("z"), 1), (Access::TupleIndex(0), 2), (Access::TupleIndex(1), 4), ] ); assert_eq!( &*ParsedPath::parse("x#0").unwrap().0, - &[ - (Access::Field("x".to_string()), 1), - (Access::FieldIndex(0), 2), - ] + &[(access_field("x"), 1), (Access::FieldIndex(0), 2)] ); assert_eq!( &*ParsedPath::parse("x#0#1").unwrap().0, &[ - (Access::Field("x".to_string()), 1), + (access_field("x"), 1), (Access::FieldIndex(0), 2), (Access::FieldIndex(1), 4) ] @@ -912,20 +666,7 @@ mod tests { #[test] fn parsed_path_get_field() { - let a = A { - w: 1, - x: B { - foo: 10, - bar: C { baz: 3.14 }, - }, - y: vec![C { baz: 1.0 }, C { baz: 2.0 }], - z: D(E(10.0, 42)), - unit_variant: F::Unit, - tuple_variant: F::Tuple(123, 321), - struct_variant: F::Struct { value: 'm' }, - array: [86, 75, 309], - tuple: (true, 1.23), - }; + let a = a_sample(); let b = ParsedPath::parse("w").unwrap(); let c = ParsedPath::parse("x.foo").unwrap(); @@ -1002,20 +743,7 @@ mod tests { #[test] fn reflect_path() { - let mut a = A { - w: 1, - x: B { - foo: 10, - bar: C { baz: 3.14 }, - }, - y: vec![C { baz: 1.0 }, C { baz: 2.0 }], - z: D(E(10.0, 42)), - unit_variant: F::Unit, - tuple_variant: F::Tuple(123, 321), - struct_variant: F::Struct { value: 'm' }, - array: [86, 75, 309], - tuple: (true, 1.23), - }; + let mut a = a_sample(); assert_eq!(*a.path::("w").unwrap(), 1); assert_eq!(*a.path::("x.foo").unwrap(), 10); @@ -1044,35 +772,78 @@ mod tests { assert_eq!( a.reflect_path("x.notreal").err().unwrap(), - ReflectPathError::InvalidField { - index: 2, - field: "notreal" + ReflectPathError::InvalidAccess { + offset: 2, + error: AccessError(access::Error::Access { + ty: TypeShape::Struct, + access: access_field("notreal"), + }), } ); assert_eq!( a.reflect_path("unit_variant.0").err().unwrap(), - ReflectPathError::ExpectedTupleVariant { index: 13 } + ReflectPathError::InvalidAccess { + offset: 13, + error: AccessError(access::Error::Enum { + actual: TypeShape::Unit, + expected: TypeShape::Tuple + }), + } ); assert_eq!( a.reflect_path("x..").err().unwrap(), - ReflectPathError::ExpectedIdent { index: 2 } + ReflectPathError::Parse(ReflectPathParseError::ExpectedIdent { offset: 2 }) ); assert_eq!( a.reflect_path("x[0]").err().unwrap(), - ReflectPathError::ExpectedList { index: 2 } + ReflectPathError::InvalidAccess { + offset: 2, + error: AccessError(access::Error::Type { + actual: TypeShape::Struct, + expected: TypeShape::List + }), + } ); assert_eq!( a.reflect_path("y.x").err().unwrap(), - ReflectPathError::ExpectedStruct { index: 2 } + ReflectPathError::InvalidAccess { + offset: 2, + error: AccessError(access::Error::Type { + actual: TypeShape::List, + expected: TypeShape::Struct + }), + } ); assert!(matches!( a.reflect_path("y[badindex]"), - Err(ReflectPathError::IndexParseError(_)) + Err(ReflectPathError::Parse( + ReflectPathParseError::IndexParseError(_) + )) )); } + + #[test] + fn accept_leading_tokens() { + assert_eq!( + &*ParsedPath::parse(".w").unwrap().0, + &[(access_field("w"), 1)] + ); + assert_eq!( + &*ParsedPath::parse("#0.foo").unwrap().0, + &[(Access::FieldIndex(0), 1), (access_field("foo"), 3)] + ); + assert_eq!( + &*ParsedPath::parse(".5").unwrap().0, + &[(Access::TupleIndex(5), 1)] + ); + assert_eq!( + &*ParsedPath::parse("[0].bar").unwrap().0, + &[(Access::ListIndex(0), 1), (access_field("bar"), 4)] + ); + } }