Skip to content

Commit 0253916

Browse files
authored
Refactor parser error context API by implementing ErrorContext for Error (#5103)
Summary This PR refactors parser error context handling by making `Error` implement `ErrorContext` directly and removing duplicate inherent methods. Closes #5094. What changed - Implemented `ErrorContext` for `Error`. - Kept `ParseResult<T>` using the same trait-based API. - Removed redundant `Error::set_context` and `Error::context` inherent methods. - Removed outdated dead-code/unused noise tied to the old duplicated methods. - Kept behavior unchanged (refactor only). Why `set_context` and `context` were not actually dead code, but the previous structure and comments suggested they might be. This makes the API consistent and clearer: both `Error` and `ParseResult<T>` use `ErrorContext`. Testing - `cargo test -p boa_parser error::tests` - `cargo test -p boa_parser` Both passed.
1 parent 3e6d34c commit 0253916

File tree

1 file changed

+39
-32
lines changed

1 file changed

+39
-32
lines changed

core/parser/src/error/mod.rs

Lines changed: 39 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,6 @@ pub(crate) trait ErrorContext {
1616
fn set_context(self, context: &'static str) -> Self;
1717

1818
/// Gets the context of the error, if any.
19-
#[allow(dead_code)]
2019
fn context(&self) -> Option<&'static str>;
2120
}
2221

@@ -26,7 +25,29 @@ impl<T> ErrorContext for ParseResult<T> {
2625
}
2726

2827
fn context(&self) -> Option<&'static str> {
29-
self.as_ref().err().and_then(Error::context)
28+
self.as_ref().err().and_then(ErrorContext::context)
29+
}
30+
}
31+
32+
impl ErrorContext for Error {
33+
fn set_context(self, new_context: &'static str) -> Self {
34+
match self {
35+
Self::Expected {
36+
expected,
37+
found,
38+
span,
39+
..
40+
} => Self::expected(expected, found, span, new_context),
41+
e => e,
42+
}
43+
}
44+
45+
fn context(&self) -> Option<&'static str> {
46+
if let Self::Expected { context, .. } = self {
47+
Some(context)
48+
} else {
49+
None
50+
}
3051
}
3152
}
3253

@@ -93,29 +114,6 @@ pub enum Error {
93114
}
94115

95116
impl Error {
96-
/// Changes the context of the error, if any.
97-
fn set_context(self, new_context: &'static str) -> Self {
98-
match self {
99-
Self::Expected {
100-
expected,
101-
found,
102-
span,
103-
..
104-
} => Self::expected(expected, found, span, new_context),
105-
e => e,
106-
}
107-
}
108-
109-
/// Gets the context of the error, if any.
110-
#[allow(unused)] // TODO: context method is unused, candidate for removal?
111-
const fn context(&self) -> Option<&'static str> {
112-
if let Self::Expected { context, .. } = self {
113-
Some(context)
114-
} else {
115-
None
116-
}
117-
}
118-
119117
/// Creates an `Expected` parsing error.
120118
pub(crate) fn expected<E, F>(expected: E, found: F, span: Span, context: &'static str) -> Self
121119
where
@@ -197,7 +195,7 @@ impl fmt::Display for Error {
197195
expected,
198196
found,
199197
span,
200-
context,
198+
..
201199
} => {
202200
write!(f, "expected ")?;
203201
match &**expected {
@@ -216,12 +214,21 @@ impl fmt::Display for Error {
216214
}
217215
}
218216
}
219-
write!(
220-
f,
221-
", got '{found}' in {context} at line {}, col {}",
222-
span.start().line_number(),
223-
span.start().column_number()
224-
)
217+
if let Some(context) = self.context() {
218+
write!(
219+
f,
220+
", got '{found}' in {context} at line {}, col {}",
221+
span.start().line_number(),
222+
span.start().column_number()
223+
)
224+
} else {
225+
write!(
226+
f,
227+
", got '{found}' at line {}, col {}",
228+
span.start().line_number(),
229+
span.start().column_number()
230+
)
231+
}
225232
}
226233
Self::Unexpected {
227234
found,

0 commit comments

Comments
 (0)