Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(apollo-parser): fix a bad parsing on fragment detected by apollo-smith #164

Merged
merged 1 commit into from
Feb 8, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 16 additions & 8 deletions crates/apollo-parser/src/parser/grammar/selection.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,14 +28,22 @@ pub(crate) fn selection(p: &mut Parser) {
while let Some(node) = p.peek() {
match node {
T![...] => {
if let Some(node) = p.peek_data_n(2) {
match node.as_str() {
"on" | "{" => fragment::inline_fragment(p),
_ => fragment::fragment_spread(p),
let next_token = p.peek_token_n(2);
match next_token {
Some(next_token) => {
if next_token.kind() == TokenKind::Name && next_token.data() != "on" {
fragment::fragment_spread(p);
} else if matches!(
next_token.kind(),
TokenKind::At | TokenKind::Name | TokenKind::LCurly
) {
fragment::inline_fragment(p);
} else {
p.err("expected an Inline Fragment or a Fragment Spread");
}
has_selection = true;
}
has_selection = true;
} else {
p.err("expected an Inline Fragment or a Fragment Spread");
None => p.err("expected an Inline Fragment or a Fragment Spread"),
}
}
T!['{'] => {
Expand All @@ -47,7 +55,7 @@ pub(crate) fn selection(p: &mut Parser) {
}
_ => {
if !has_selection {
p.err("exepcted at least one Selection in Selection Set");
p.err("expected at least one Selection in Selection Set");
}
break;
}
Expand Down
9 changes: 9 additions & 0 deletions crates/apollo-parser/src/parser/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -244,6 +244,15 @@ impl Parser {
self.tokens.last()
}

/// Peek Token `n` and return it.
pub(crate) fn peek_token_n(&self, n: usize) -> Option<&Token> {
self.tokens
.iter()
.rev()
.filter(|token| !matches!(token.kind(), TokenKind::Whitespace | TokenKind::Comment))
.nth(n - 1)
}

/// Peek Token `n` and return its TokenKind.
pub(crate) fn peek_n(&self, n: usize) -> Option<TokenKind> {
self.tokens
Expand Down
15 changes: 10 additions & 5 deletions crates/apollo-parser/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -62,11 +62,16 @@ fn assert_errors_are_present(errors: Iter<'_, Error>, path: &Path) {
}

fn assert_errors_are_absent(errors: Iter<'_, Error>, path: &Path) {
assert!(
errors.len() == 0,
"There should be no errors in the file {:?}",
path.display(),
);
if errors.len() > 0 {
println!(
"errors: {}",
errors
.map(|e| e.message())
.collect::<Vec<&str>>()
.join("\n")
);
panic!("There should be no errors in the file {:?}", path.display(),);
}
}

/// Concatenate tokens and errors.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,4 +6,4 @@
- [email protected]
- [email protected] "{"
- [email protected] "}"
- ERROR@7:8 "exepcted at least one Selection in Selection Set" }
- ERROR@7:8 "expected at least one Selection in Selection Set" }
Original file line number Diff line number Diff line change
Expand Up @@ -11,4 +11,4 @@
- [email protected] "{"
- [email protected] "}"
- ERROR@0:93 "expected an Operation Type or a Selection Set" "after this PR this should not be an issue: https://github.com/graphql/graphql-spec/pull/892"
- ERROR@107:108 "exepcted at least one Selection in Selection Set" }
- ERROR@107:108 "expected at least one Selection in Selection Set" }
Original file line number Diff line number Diff line change
@@ -1,4 +1,7 @@
{
pet
...snackSelection
}
pet
...snackSelection
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

since you're already editing these selection_with_fragment tests, could you also add the other fragment variants?

Suggested change
...snackSelection
...snackSelection
... on Nap {
cozyLocation
durationOfNap
}
...snackSelection @deprecated
... on Nap @provides(duration: "2 hours") {
cozyLocation
}

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ahhh oops just saw that you merged! i'll just quickly add these in another branch

... @J(N: 0) {
a
}
}
Original file line number Diff line number Diff line change
@@ -1,16 +1,45 @@
- DOCUMENT@0..33
- OPERATION_DEFINITION@0..33
- SELECTION_SET@0..33
- DOCUMENT@0..57
- OPERATION_DEFINITION@0..57
- SELECTION_SET@0..57
- [email protected] "{"
- [email protected] "\n "
- [email protected]
- [email protected]
- [email protected] "pet"
- [email protected] "\n "
- [email protected]
- [email protected] "..."
- [email protected]
- [email protected]
- [email protected] "snackSelection"
- [email protected] "\n"
- [email protected] "}"
- [email protected] "\n "
- [email protected]
- [email protected]
- [email protected] "pet"
- [email protected] "\n "
- [email protected]
- [email protected] "..."
- [email protected]
- [email protected]
- [email protected] "snackSelection"
- [email protected] "\n "
- [email protected]
- [email protected] "..."
- [email protected] " "
- [email protected]
- [email protected]
- [email protected] "@"
- [email protected]
- [email protected] "J"
- [email protected]
- [email protected] "("
- [email protected]
- [email protected]
- [email protected] "N"
- [email protected] ":"
- [email protected] " "
- [email protected]
- [email protected] "0"
- [email protected] ")"
- [email protected] " "
- [email protected]
- [email protected] "{"
- [email protected] "\n "
- [email protected]
- [email protected]
- [email protected] "a"
- [email protected] "\n "
- [email protected] "}"
- [email protected] "\n"
- [email protected] "}"
- [email protected] "\n"