Skip to content

Commit 10797d4

Browse files
nicopapnicopap
authored
Refactor parsing in bevy_reflect path module (#9048)
# Objective - Follow up to #8887 - The parsing code in `bevy_reflect/src/path/mod.rs` could also do with some cleanup ## Solution - Create the `parse.rs` module, move all parsing code to this module - The parsing errors also now keep track of the whole parsed string, and are much more fine-grained ### Detailed changes - Move `PathParser` to `parse.rs` submodule - Rename `token_to_access` to `access_following` (yep, goes from 132 lines to 16) - Move parsing tests into the `parse.rs` file
1 parent e52af83 commit 10797d4

2 files changed

Lines changed: 194 additions & 182 deletions

File tree

‎crates/bevy_reflect/src/path/mod.rs‎

Lines changed: 21 additions & 182 deletions
Original file line numberDiff line numberDiff line change
@@ -1,49 +1,40 @@
11
mod access;
2+
mod parse;
23

34
use std::fmt;
4-
use std::num::ParseIntError;
55

66
use crate::Reflect;
77
use access::Access;
8+
use parse::PathParser;
89
use thiserror::Error;
910

10-
type ParseResult<T> = Result<T, ReflectPathParseError>;
11+
pub use parse::ParseError;
1112

1213
/// An error specific to accessing a field/index on a `Reflect`.
1314
#[derive(Debug, PartialEq, Eq, Error)]
1415
#[error(transparent)]
1516
pub struct AccessError<'a>(access::Error<'a>);
1617

17-
/// A parse error for a path string.
18-
#[derive(Debug, PartialEq, Eq, Error)]
19-
pub enum ReflectPathParseError {
20-
#[error("expected an identifier at offset {offset}")]
21-
ExpectedIdent { offset: usize },
22-
23-
#[error("encountered an unexpected token `{token}`")]
24-
UnexpectedToken { offset: usize, token: &'static str },
25-
26-
#[error("expected token `{token}`, but it wasn't there.")]
27-
ExpectedToken { offset: usize, token: &'static str },
28-
29-
#[error("failed to parse a usize")]
30-
IndexParseError(#[from] ParseIntError),
31-
}
32-
3318
/// An error returned from a failed path string query.
3419
#[derive(Debug, PartialEq, Eq, Error)]
3520
pub enum ReflectPathError<'a> {
36-
#[error("{error}")]
21+
#[error("at {offset} in path specification: {error}")]
3722
InvalidAccess {
3823
/// Position in the path string.
3924
offset: usize,
4025
error: AccessError<'a>,
4126
},
27+
4228
#[error("failed to downcast to the path result to the given type")]
4329
InvalidDowncast,
4430

45-
#[error(transparent)]
46-
Parse(#[from] ReflectPathParseError),
31+
#[error("at {offset} in '{path}': {error}")]
32+
ParseError {
33+
/// Position in `path`.
34+
offset: usize,
35+
path: &'a str,
36+
error: ParseError<'a>,
37+
},
4738
}
4839

4940
/// A trait which allows nested [`Reflect`] values to be retrieved with path strings.
@@ -417,139 +408,6 @@ impl fmt::Display for ParsedPath {
417408
Ok(())
418409
}
419410
}
420-
421-
struct PathParser<'a> {
422-
path: &'a str,
423-
index: usize,
424-
}
425-
426-
impl<'a> PathParser<'a> {
427-
fn new(path: &'a str) -> Self {
428-
Self { path, index: 0 }
429-
}
430-
431-
fn next_token(&mut self) -> Option<Token<'a>> {
432-
if self.index >= self.path.len() {
433-
return None;
434-
}
435-
436-
match self.path[self.index..].chars().next().unwrap() {
437-
Token::DOT => {
438-
self.index += 1;
439-
return Some(Token::Dot);
440-
}
441-
Token::CROSSHATCH => {
442-
self.index += 1;
443-
return Some(Token::CrossHatch);
444-
}
445-
Token::OPEN_BRACKET => {
446-
self.index += 1;
447-
return Some(Token::OpenBracket);
448-
}
449-
Token::CLOSE_BRACKET => {
450-
self.index += 1;
451-
return Some(Token::CloseBracket);
452-
}
453-
_ => {}
454-
}
455-
456-
// we can assume we are parsing an ident now
457-
for (char_index, character) in self.path[self.index..].chars().enumerate() {
458-
match character {
459-
Token::DOT | Token::CROSSHATCH | Token::OPEN_BRACKET | Token::CLOSE_BRACKET => {
460-
let ident = Token::Ident(&self.path[self.index..self.index + char_index]);
461-
self.index += char_index;
462-
return Some(ident);
463-
}
464-
_ => {}
465-
}
466-
}
467-
let ident = Token::Ident(&self.path[self.index..]);
468-
self.index = self.path.len();
469-
Some(ident)
470-
}
471-
472-
fn token_to_access(&mut self, token: Token<'a>) -> ParseResult<Access<'a>> {
473-
let current_offset = self.index;
474-
match token {
475-
Token::Dot => {
476-
if let Some(Token::Ident(value)) = self.next_token() {
477-
value
478-
.parse::<usize>()
479-
.map(Access::TupleIndex)
480-
.or(Ok(Access::Field(value.into())))
481-
} else {
482-
Err(ReflectPathParseError::ExpectedIdent {
483-
offset: current_offset,
484-
})
485-
}
486-
}
487-
Token::CrossHatch => {
488-
if let Some(Token::Ident(value)) = self.next_token() {
489-
Ok(Access::FieldIndex(value.parse::<usize>()?))
490-
} else {
491-
Err(ReflectPathParseError::ExpectedIdent {
492-
offset: current_offset,
493-
})
494-
}
495-
}
496-
Token::OpenBracket => {
497-
let access = if let Some(Token::Ident(value)) = self.next_token() {
498-
Access::ListIndex(value.parse::<usize>()?)
499-
} else {
500-
return Err(ReflectPathParseError::ExpectedIdent {
501-
offset: current_offset,
502-
});
503-
};
504-
505-
if !matches!(self.next_token(), Some(Token::CloseBracket)) {
506-
return Err(ReflectPathParseError::ExpectedToken {
507-
offset: current_offset,
508-
token: Token::OPEN_BRACKET_STR,
509-
});
510-
}
511-
512-
Ok(access)
513-
}
514-
Token::CloseBracket => Err(ReflectPathParseError::UnexpectedToken {
515-
offset: current_offset,
516-
token: Token::CLOSE_BRACKET_STR,
517-
}),
518-
Token::Ident(value) => value
519-
.parse::<usize>()
520-
.map(Access::TupleIndex)
521-
.or(Ok(Access::Field(value.into()))),
522-
}
523-
}
524-
}
525-
526-
impl<'a> Iterator for PathParser<'a> {
527-
type Item = (ParseResult<Access<'a>>, usize);
528-
529-
fn next(&mut self) -> Option<Self::Item> {
530-
let token = self.next_token()?;
531-
let index = self.index;
532-
Some((self.token_to_access(token), index))
533-
}
534-
}
535-
536-
enum Token<'a> {
537-
Dot,
538-
CrossHatch,
539-
OpenBracket,
540-
CloseBracket,
541-
Ident(&'a str),
542-
}
543-
544-
impl<'a> Token<'a> {
545-
const DOT: char = '.';
546-
const CROSSHATCH: char = '#';
547-
const OPEN_BRACKET: char = '[';
548-
const CLOSE_BRACKET: char = ']';
549-
const OPEN_BRACKET_STR: &'static str = "[";
550-
const CLOSE_BRACKET_STR: &'static str = "]";
551-
}
552-
553411
#[cfg(test)]
554412
#[allow(clippy::float_cmp, clippy::approx_constant)]
555413
mod tests {
@@ -616,6 +474,13 @@ mod tests {
616474
Access::Field(field.into())
617475
}
618476

477+
type StaticError = ReflectPathError<'static>;
478+
479+
fn invalid_access(offset: usize, actual: TypeShape, expected: TypeShape) -> StaticError {
480+
let error = AccessError(access::Error::Type { actual, expected });
481+
ReflectPathError::InvalidAccess { offset, error }
482+
}
483+
619484
#[test]
620485
fn parsed_path_parse() {
621486
assert_eq!(
@@ -791,40 +656,14 @@ mod tests {
791656
}),
792657
}
793658
);
794-
795-
assert_eq!(
796-
a.reflect_path("x..").err().unwrap(),
797-
ReflectPathError::Parse(ReflectPathParseError::ExpectedIdent { offset: 2 })
798-
);
799-
800659
assert_eq!(
801660
a.reflect_path("x[0]").err().unwrap(),
802-
ReflectPathError::InvalidAccess {
803-
offset: 2,
804-
error: AccessError(access::Error::Type {
805-
actual: TypeShape::Struct,
806-
expected: TypeShape::List
807-
}),
808-
}
661+
invalid_access(2, TypeShape::Struct, TypeShape::List)
809662
);
810-
811663
assert_eq!(
812664
a.reflect_path("y.x").err().unwrap(),
813-
ReflectPathError::InvalidAccess {
814-
offset: 2,
815-
error: AccessError(access::Error::Type {
816-
actual: TypeShape::List,
817-
expected: TypeShape::Struct
818-
}),
819-
}
665+
invalid_access(2, TypeShape::List, TypeShape::Struct)
820666
);
821-
822-
assert!(matches!(
823-
a.reflect_path("y[badindex]"),
824-
Err(ReflectPathError::Parse(
825-
ReflectPathParseError::IndexParseError(_)
826-
))
827-
));
828667
}
829668

830669
#[test]

0 commit comments

Comments
 (0)