From dc888f0a7a366eb047f2409f404d9e8e548ab906 Mon Sep 17 00:00:00 2001 From: surajk-m Date: Tue, 27 Aug 2024 22:35:05 +0530 Subject: [PATCH 1/3] refactor(parser): Use `peek_token()` to check `token_kind and data` together (#830) --- .../src/parser/grammar/object.rs | 84 ++++++++++++------- 1 file changed, 54 insertions(+), 30 deletions(-) diff --git a/crates/apollo-parser/src/parser/grammar/object.rs b/crates/apollo-parser/src/parser/grammar/object.rs index e4f029f04..9e8363975 100644 --- a/crates/apollo-parser/src/parser/grammar/object.rs +++ b/crates/apollo-parser/src/parser/grammar/object.rs @@ -19,31 +19,45 @@ use crate::T; pub(crate) fn object_type_definition(p: &mut Parser) { let _g = p.start_node(SyntaxKind::OBJECT_TYPE_DEFINITION); - if let Some(TokenKind::StringValue) = p.peek() { - description::description(p); + if let Some(token) = p.peek_token() { + if token.kind() == TokenKind::StringValue { + description::description(p); + } } - if let Some("type") = p.peek_data() { - p.bump(SyntaxKind::type_KW); + if let Some(token) = p.peek_token() { + if token.data() == "type" { + p.bump(SyntaxKind::type_KW); + } } - match p.peek() { - Some(TokenKind::Name) => name::name(p), - _ => p.err("expected a name"), + if let Some(token) = p.peek_token() { + match token.kind() { + TokenKind::Name => name::name(p), + _ => p.err("expected a name"), + } } - if let Some(TokenKind::Name) = p.peek() { - if p.peek_data().unwrap() == "implements" { - implements_interfaces(p); + if let Some(token) = p.peek_token() { + if token.kind() == TokenKind::Name { + if token.data() == "implements" { + implements_interfaces(p); + } else { + p.err("unexpected Name"); + } } } - if let Some(T![@]) = p.peek() { - directive::directives(p, Constness::Const); + if let Some(token) = p.peek_token() { + if token.kind() == T![@] { + directive::directives(p, Constness::Const); + } } - if let Some(T!['{']) = p.peek() { - field::fields_definition(p); + if let Some(token) = p.peek_token() { + if token.kind() == T!['{'] { + field::fields_definition(p); + } } } @@ -62,24 +76,32 @@ pub(crate) fn object_type_extension(p: &mut Parser) { // FieldsDefinitions is provided. If none are present, we push an error. let mut meets_requirements = false; - match p.peek() { - Some(TokenKind::Name) => name::name(p), - _ => p.err("expected a Name"), + if let Some(token) = p.peek_token() { + match token.kind() { + TokenKind::Name => name::name(p), + _ => p.err("expected a Name"), + } } - if let Some("implements") = p.peek_data() { - meets_requirements = true; - implements_interfaces(p); + if let Some(token) = p.peek_token() { + if token.data() == "implements" { + meets_requirements = true; + implements_interfaces(p); + } } - if let Some(T![@]) = p.peek() { - meets_requirements = true; - directive::directives(p, Constness::Const) + if let Some(token) = p.peek_token() { + if token.kind() == T![@] { + meets_requirements = true; + directive::directives(p, Constness::Const); + } } - if let Some(T!['{']) = p.peek() { - meets_requirements = true; - field::fields_definition(p) + if let Some(token) = p.peek_token() { + if token.kind() == T!['{'] { + meets_requirements = true; + field::fields_definition(p); + } } if !meets_requirements { @@ -97,10 +119,12 @@ pub(crate) fn implements_interfaces(p: &mut Parser) { p.bump(SyntaxKind::implements_KW); p.parse_separated_list(T![&], S![&], |p| { - if let Some(TokenKind::Name) = p.peek() { - ty::named_type(p); - } else { - p.err("expected an Interface name"); + if let Some(token) = p.peek_token() { + if token.kind() == TokenKind::Name { + ty::named_type(p); + } else { + p.err("expected an Interface name"); + } } }); } From 65083215e56c256f4a92bb30817408b18ec98852 Mon Sep 17 00:00:00 2001 From: surajk-m Date: Mon, 16 Sep 2024 19:49:08 +0530 Subject: [PATCH 2/3] Revert `peek_token()` where `peek_data()` is sufficient --- .../src/parser/grammar/object.rs | 84 +++++++------------ 1 file changed, 30 insertions(+), 54 deletions(-) diff --git a/crates/apollo-parser/src/parser/grammar/object.rs b/crates/apollo-parser/src/parser/grammar/object.rs index 9e8363975..28333ac7b 100644 --- a/crates/apollo-parser/src/parser/grammar/object.rs +++ b/crates/apollo-parser/src/parser/grammar/object.rs @@ -19,45 +19,31 @@ use crate::T; pub(crate) fn object_type_definition(p: &mut Parser) { let _g = p.start_node(SyntaxKind::OBJECT_TYPE_DEFINITION); - if let Some(token) = p.peek_token() { - if token.kind() == TokenKind::StringValue { - description::description(p); - } + if let Some(TokenKind::StringValue) = p.peek() { + description::description(p); } - if let Some(token) = p.peek_token() { - if token.data() == "type" { - p.bump(SyntaxKind::type_KW); - } + if let Some("type") = p.peek_data() { + p.bump(SyntaxKind::type_KW); } - if let Some(token) = p.peek_token() { - match token.kind() { - TokenKind::Name => name::name(p), - _ => p.err("expected a name"), - } + match p.peek() { + Some(TokenKind::Name) => name::name(p), + _ => p.err("expected a name"), } if let Some(token) = p.peek_token() { - if token.kind() == TokenKind::Name { - if token.data() == "implements" { - implements_interfaces(p); - } else { - p.err("unexpected Name"); - } + if token.kind() == TokenKind::Name && token.data() == "implements" { + implements_interfaces(p); } } - if let Some(token) = p.peek_token() { - if token.kind() == T![@] { - directive::directives(p, Constness::Const); - } + if let Some(T![@]) = p.peek() { + directive::directives(p, Constness::Const); } - if let Some(token) = p.peek_token() { - if token.kind() == T!['{'] { - field::fields_definition(p); - } + if let Some(T!['{']) = p.peek() { + field::fields_definition(p); } } @@ -72,36 +58,28 @@ pub(crate) fn object_type_extension(p: &mut Parser) { p.bump(SyntaxKind::extend_KW); p.bump(SyntaxKind::type_KW); - // Use this variable to see if any of ImplementsInterfacs, Directives or + // Use this variable to see if any of ImplementsInterfaces, Directives or // FieldsDefinitions is provided. If none are present, we push an error. let mut meets_requirements = false; - if let Some(token) = p.peek_token() { - match token.kind() { - TokenKind::Name => name::name(p), - _ => p.err("expected a Name"), - } + match p.peek() { + Some(TokenKind::Name) => name::name(p), + _ => p.err("expected a Name"), } - if let Some(token) = p.peek_token() { - if token.data() == "implements" { - meets_requirements = true; - implements_interfaces(p); - } + if let Some("implements") = p.peek_data() { + meets_requirements = true; + implements_interfaces(p); } - if let Some(token) = p.peek_token() { - if token.kind() == T![@] { - meets_requirements = true; - directive::directives(p, Constness::Const); - } + if let Some(T![@]) = p.peek() { + meets_requirements = true; + directive::directives(p, Constness::Const) } - if let Some(token) = p.peek_token() { - if token.kind() == T!['{'] { - meets_requirements = true; - field::fields_definition(p); - } + if let Some(T!['{']) = p.peek() { + meets_requirements = true; + field::fields_definition(p) } if !meets_requirements { @@ -119,12 +97,10 @@ pub(crate) fn implements_interfaces(p: &mut Parser) { p.bump(SyntaxKind::implements_KW); p.parse_separated_list(T![&], S![&], |p| { - if let Some(token) = p.peek_token() { - if token.kind() == TokenKind::Name { - ty::named_type(p); - } else { - p.err("expected an Interface name"); - } + if let Some(TokenKind::Name) = p.peek() { + ty::named_type(p); + } else { + p.err("expected an Interface name"); } }); } From 2a3f8bf885b4ae333de5b169b6998c3bddf76146 Mon Sep 17 00:00:00 2001 From: surajk-m Date: Tue, 17 Sep 2024 00:58:23 +0530 Subject: [PATCH 3/3] update `peek_data().unwrap()` usage in parser grammar files --- .../src/parser/grammar/document.rs | 14 +++++-- .../src/parser/grammar/fragment.rs | 39 ++++++++++--------- .../apollo-parser/src/parser/grammar/name.rs | 10 +++-- .../apollo-parser/src/parser/grammar/value.rs | 29 +++++++------- 4 files changed, 51 insertions(+), 41 deletions(-) diff --git a/crates/apollo-parser/src/parser/grammar/document.rs b/crates/apollo-parser/src/parser/grammar/document.rs index 32c203c60..9935f6ce4 100644 --- a/crates/apollo-parser/src/parser/grammar/document.rs +++ b/crates/apollo-parser/src/parser/grammar/document.rs @@ -36,12 +36,18 @@ pub(crate) fn document(p: &mut Parser) { } } TokenKind::Name => { - let def = p.peek_data().unwrap(); - select_definition(def, p); + if let Some(def) = p.peek_data() { + select_definition(def, p); + } else { + p.err_and_pop("expected a definition after this Name"); + } } TokenKind::LCurly => { - let def = p.peek_data().unwrap(); - select_definition(def, p); + if let Some(def) = p.peek_data() { + select_definition(def, p); + } else { + p.err_and_pop("expected a definition"); + } } TokenKind::Eof => return ControlFlow::Break(()), _ => p.err_and_pop("expected a StringValue, Name or OperationDefinition"), diff --git a/crates/apollo-parser/src/parser/grammar/fragment.rs b/crates/apollo-parser/src/parser/grammar/fragment.rs index bfc532984..f1a86dd41 100644 --- a/crates/apollo-parser/src/parser/grammar/fragment.rs +++ b/crates/apollo-parser/src/parser/grammar/fragment.rs @@ -36,15 +36,17 @@ pub(crate) fn fragment_definition(p: &mut Parser) { /// Name *but not* **on** pub(crate) fn fragment_name(p: &mut Parser) { let _g = p.start_node(SyntaxKind::FRAGMENT_NAME); - match p.peek() { - Some(TokenKind::Name) => { - if p.peek_data().unwrap() == "on" { - return p.err("Fragment Name cannot be 'on'"); + if let Some(token) = p.peek_token() { + if token.kind() == TokenKind::Name { + if token.data() != "on" { + name::name(p); + } else { + p.err("Fragment Name cannot be 'on'"); } - name::name(p) + return; } - _ => p.err("expected Fragment Name"), } + p.err("expected Fragment Name"); } /// See: https://spec.graphql.org/October2021/#TypeCondition @@ -53,21 +55,20 @@ pub(crate) fn fragment_name(p: &mut Parser) { /// **on** NamedType pub(crate) fn type_condition(p: &mut Parser) { let _g = p.start_node(SyntaxKind::TYPE_CONDITION); - match p.peek() { - Some(TokenKind::Name) => { - if p.peek_data().unwrap() == "on" { - p.bump(SyntaxKind::on_KW); - } else { - p.err("expected 'on'"); - } + if let Some(token) = p.peek_token() { + if token.kind() == TokenKind::Name && token.data() == "on" { + p.bump(SyntaxKind::on_KW); + } else { + p.err("expected 'on'"); + } - if let Some(TokenKind::Name) = p.peek() { - ty::named_type(p) - } else { - p.err("expected a Name in Type Condition") - } + if let Some(TokenKind::Name) = p.peek() { + ty::named_type(p) + } else { + p.err("expected a Name in Type Condition") } - _ => p.err("expected Type Condition"), + } else { + p.err("expected Type Condition") } } diff --git a/crates/apollo-parser/src/parser/grammar/name.rs b/crates/apollo-parser/src/parser/grammar/name.rs index a478387fe..b9d158881 100644 --- a/crates/apollo-parser/src/parser/grammar/name.rs +++ b/crates/apollo-parser/src/parser/grammar/name.rs @@ -8,14 +8,16 @@ use crate::S; /// *Name*: /// [_A-Za-z][_0-9A-Za-z] pub(crate) fn name(p: &mut Parser) { - match p.peek() { - Some(TokenKind::Name) => { + if let Some(token) = p.peek_token() { + if token.kind() == TokenKind::Name { + let name_data = token.data(); let _g = p.start_node(SyntaxKind::NAME); - validate_name(p.peek_data().unwrap(), p); + validate_name(name_data, p); p.bump(SyntaxKind::IDENT); + return; } - _ => p.err("expected a Name"), } + p.err("expected a Name"); } pub(crate) fn validate_name(name: &str, p: &mut Parser) { diff --git a/crates/apollo-parser/src/parser/grammar/value.rs b/crates/apollo-parser/src/parser/grammar/value.rs index 595951931..30936feaf 100644 --- a/crates/apollo-parser/src/parser/grammar/value.rs +++ b/crates/apollo-parser/src/parser/grammar/value.rs @@ -52,21 +52,22 @@ pub(crate) fn value(p: &mut Parser, constness: Constness, pop_on_error: bool) { p.bump(SyntaxKind::STRING); } Some(TokenKind::Name) => { - let node = p.peek_data().unwrap(); - match node { - "true" => { - let _g = p.start_node(SyntaxKind::BOOLEAN_VALUE); - p.bump(SyntaxKind::true_KW); - } - "false" => { - let _g = p.start_node(SyntaxKind::BOOLEAN_VALUE); - p.bump(SyntaxKind::false_KW); - } - "null" => { - let _g = p.start_node(SyntaxKind::NULL_VALUE); - p.bump(SyntaxKind::null_KW) + if let Some(token) = p.peek_token() { + match token.data() { + "true" => { + let _g = p.start_node(SyntaxKind::BOOLEAN_VALUE); + p.bump(SyntaxKind::true_KW); + } + "false" => { + let _g = p.start_node(SyntaxKind::BOOLEAN_VALUE); + p.bump(SyntaxKind::false_KW); + } + "null" => { + let _g = p.start_node(SyntaxKind::NULL_VALUE); + p.bump(SyntaxKind::null_KW) + } + _ => enum_value(p), } - _ => enum_value(p), } } Some(T!['[']) => list_value(p, constness),