Introduce SyntaxErrorKind and TextRange to SyntaxError
#188
Conversation
|
As part of this PR I am trying to use a visitor instead of casting, but for some reason this version returns no character validation errors. Am I missing something? Maybe I am using the visitor the wrong way... |
|
(I was under the idea that a visitor would visit the descendants of the node as well, but maybe that isn't the case?) |
crates/ra_syntax/src/validation.rs
Outdated
| errors | ||
|
|
||
| if !components.has_closing_quote { | ||
| errors.push(SyntaxError { |
There was a problem hiding this comment.
I think it makes sense to create a helper funciton for construction SyntaxError.
aochagavia
changed the title
SyntaxErrorKind and TextRange to SyntaxError
|
I started working on |
| ParseError(msg) => write!(f, "{}", msg.0), | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
+1, moving this to separate file makes sense!
| #[derive(Debug, Clone, PartialEq, Eq, Hash)] | ||
| pub struct SyntaxError { | ||
| pub kind: SyntaxErrorKind, | ||
| pub range: TextRange, |
There was a problem hiding this comment.
I am slightly worried that we know expose a rather elaborate internal structure of the error. Let's make these fields private, but add
enum Location {
Offest(TextUnit),
Range(TextRange),
}
impl SyntaxError {
fn location() -> ErrorLocation
}
I think it maybe make sense to distinguish between offset/range for location, because there are errors for which range does not make sense, due to error recovery. Consider, for example, this snippet:
fn
fn foo() {}
Here, we should report expected ident. However, the fn foo() {} is correctly parsed as a function, so we can't mark fn token as an error. I think the expected behavior is to say that the error happened after the fn token, and let it to the UI layer to select the most appropriate presentation, which, in this case, is a single-character range after \n.
There was a problem hiding this comment.
None that we don't need fn message() -> String, because SyntaxError: fmt::Display
|
Just pushed a commit with the suggested changes. Any additional comments? |
|
LGTM! bors r+ |
Build succeeded |
No description provided.