Skip to content

Commit

Permalink
Merge #2834
Browse files Browse the repository at this point in the history
2834: refactor(ra_syntax.validation): removed code duplication from validate_literal() r=kiljacken a=Veetaha

Hi! This is my first ever contribution to this project.
I've taken some dirty job from issue #223

This is a simple atomic PR to remove code duplication according to FIXME comment in the function that is the main focus of the further development.

I just didn't want to mix refactoring with the implementation of new features...

I am not sure whether you prefer such atomic PRs here or you'd rather have a single PR that contains all atomic commits inside of it?

So if you want me to add all that validation in one PR I'll mark this one as WIP and update it when the work is finished, otherwise, I'll go with the option of creating separate PRs per each feature of validation of strings, numbers, and comments respectively.

### Comments about refactoring
Yeah, reducing the duplication is quite hard here, extracting into stateless functions could be another option but the number of their arguments would be very big and repeated across char and string implementations so that just writing their types and names would become cumbersome.
I tried the option of having everything captured implicitly in the closure but failed since rust doesn't have templated (or generic) closures as C++ does, this is needed because `unescape_byte*()` and `unescape_char|str()` have different return types...
Maybe I am missing something here? I may be wrong because I am not enough experienced in Rust...
Well, I am awaiting any kind of feedback!

Co-authored-by: Veetaha <gerzoh1@gmail.com>
  • Loading branch information
bors[bot] and Veetaha authored Jan 14, 2020
2 parents e406f2b + 60251da commit 6a5100f
Showing 1 changed file with 26 additions and 35 deletions.
61 changes: 26 additions & 35 deletions crates/ra_syntax/src/validation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -111,55 +111,46 @@ pub(crate) fn validate(root: &SyntaxNode) -> Vec<SyntaxError> {
errors
}

// FIXME: kill duplication
fn validate_literal(literal: ast::Literal, acc: &mut Vec<SyntaxError>) {
fn unquote(text: &str, prefix_len: usize, end_delimiter: char) -> Option<&str> {
text.rfind(end_delimiter).and_then(|end| text.get(prefix_len..end))
}

let token = literal.token();
let text = token.text().as_str();

let mut push_err = |prefix_len, (off, err): (usize, unescape::EscapeError)| {
let off = token.text_range().start() + TextUnit::from_usize(off + prefix_len);
acc.push(SyntaxError::new(err.into(), off));
};

match token.kind() {
BYTE => {
if let Some(end) = text.rfind('\'') {
if let Some(without_quotes) = text.get(2..end) {
if let Err((off, err)) = unescape::unescape_byte(without_quotes) {
let off = token.text_range().start() + TextUnit::from_usize(off + 2);
acc.push(SyntaxError::new(err.into(), off))
}
}
if let Some(Err(e)) = unquote(text, 2, '\'').map(unescape::unescape_byte) {
push_err(2, e);
}
}
CHAR => {
if let Some(end) = text.rfind('\'') {
if let Some(without_quotes) = text.get(1..end) {
if let Err((off, err)) = unescape::unescape_char(without_quotes) {
let off = token.text_range().start() + TextUnit::from_usize(off + 1);
acc.push(SyntaxError::new(err.into(), off))
}
}
if let Some(Err(e)) = unquote(text, 1, '\'').map(unescape::unescape_char) {
push_err(1, e);
}
}
BYTE_STRING => {
if let Some(end) = text.rfind('\"') {
if let Some(without_quotes) = text.get(2..end) {
unescape::unescape_byte_str(without_quotes, &mut |range, char| {
if let Err(err) = char {
let off = range.start;
let off = token.text_range().start() + TextUnit::from_usize(off + 2);
acc.push(SyntaxError::new(err.into(), off))
}
})
}
if let Some(without_quotes) = unquote(text, 2, '"') {
unescape::unescape_byte_str(without_quotes, &mut |range, char| {
if let Err(err) = char {
push_err(2, (range.start, err));
}
})
}
}
STRING => {
if let Some(end) = text.rfind('\"') {
if let Some(without_quotes) = text.get(1..end) {
unescape::unescape_str(without_quotes, &mut |range, char| {
if let Err(err) = char {
let off = range.start;
let off = token.text_range().start() + TextUnit::from_usize(off + 1);
acc.push(SyntaxError::new(err.into(), off))
}
})
}
if let Some(without_quotes) = unquote(text, 1, '"') {
unescape::unescape_str(without_quotes, &mut |range, char| {
if let Err(err) = char {
push_err(1, (range.start, err));
}
})
}
}
_ => (),
Expand Down

0 comments on commit 6a5100f

Please sign in to comment.