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

feat(graphql_parser): parse enum extension #3044

Merged
merged 2 commits into from
Jun 5, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
52 changes: 29 additions & 23 deletions crates/biome_graphql_factory/src/generated/node_factory.rs

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

44 changes: 2 additions & 42 deletions crates/biome_graphql_factory/src/generated/syntax_factory.rs

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

24 changes: 23 additions & 1 deletion crates/biome_graphql_parser/src/parser/definitions/enum.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use crate::parser::{
directive::{is_at_directive, DirectiveList},
is_nth_at_name, is_nth_at_non_kw_name, parse_description,
parse_error::expected_name,
parse_error::{expected_enum_extension, expected_name},
parse_name,
value::{is_at_string, parse_enum_value},
GraphqlParser,
Expand Down Expand Up @@ -34,6 +34,28 @@ pub(crate) fn parse_enum_type_definition(p: &mut GraphqlParser) -> ParsedSyntax
Present(m.complete(p, GRAPHQL_ENUM_TYPE_DEFINITION))
}

/// Must only be called if the next 2 token is `extend` and `enum`, otherwise it will panic.
#[inline]
pub(crate) fn parse_enum_type_extension(p: &mut GraphqlParser) -> ParsedSyntax {
let m = p.start();

p.bump(T![extend]);
p.bump(T![enum]);
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
p.bump(T![enum]);
p.expect(T![enum]);

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't know if this is truly necessary, as this parsing rule should only be called in one place (here), and we must have already checked for the presence of both tokens before advancing. Correct me if you think otherwise.

Copy link
Contributor

@arendjr arendjr Jun 5, 2024

Choose a reason for hiding this comment

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

Yeah, it’s not a blocker, but it’s good practice for the parser functions to validate their preconditions instead of assuming to be called correctly. It wouldn’t change anything functionally now, but it makes the contract more explicit and brings some extra robustness in the face of future changes to the parser.


parse_name(p).or_add_diagnostic(p, expected_name);

let directive_list = DirectiveList.parse_list(p);
let directive_empty = directive_list.range(p).is_empty();

let enum_values_empty = parse_enum_values(p).is_absent();

if directive_empty && enum_values_empty {
p.error(expected_enum_extension(p, p.cur_range()));
}

Present(m.complete(p, GRAPHQL_ENUM_TYPE_EXTENSION))
}

#[inline]
fn parse_enum_values(p: &mut GraphqlParser) -> ParsedSyntax {
if !is_at_enum_values(p) {
Expand Down
3 changes: 2 additions & 1 deletion crates/biome_graphql_parser/src/parser/definitions/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ use self::{
interface::{parse_interface_type_definition, parse_interface_type_extension},
object::{parse_object_type_definition, parse_object_type_extension},
operation::{parse_operation_definition, parse_selection_set},
r#enum::parse_enum_type_definition,
r#enum::{parse_enum_type_definition, parse_enum_type_extension},
scalar::{parse_scalar_type_definition, parse_scalar_type_extension},
schema::{parse_schema_definition, parse_schema_extension},
union::{parse_union_type_definition, parse_union_type_extension},
Expand Down Expand Up @@ -101,6 +101,7 @@ fn parse_extension(p: &mut GraphqlParser) -> ParsedSyntax {
T![type] => parse_object_type_extension(p),
T![interface] => parse_interface_type_extension(p),
T![union] => parse_union_type_extension(p),
T![enum] => parse_enum_type_extension(p),
_ => Absent,
}
}
Expand Down
11 changes: 9 additions & 2 deletions crates/biome_graphql_parser/src/parser/parse_error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -109,14 +109,21 @@ pub(crate) fn expected_directive_location(p: &GraphqlParser, range: TextRange) -

pub(crate) fn expected_object_extension(p: &GraphqlParser, range: TextRange) -> ParseDiagnostic {
p.err_builder(
"Expected at least one directive, implements interface or fields definition",
"Expected at least one directive, implements interface or a set of fields definition",
range,
)
}

pub(crate) fn expected_union_extension(p: &GraphqlParser, range: TextRange) -> ParseDiagnostic {
p.err_builder(
"Expected at least one directive or union member types",
"Expected at least one directive or a set union member types",
range,
)
}

pub(crate) fn expected_enum_extension(p: &GraphqlParser, range: TextRange) -> ParseDiagnostic {
p.err_builder(
"Expected at least one directive or a set of enum values",
range,
)
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
extend enum Direction

extend enum Direction
NORTH
}

Original file line number Diff line number Diff line change
@@ -0,0 +1,118 @@
---
source: crates/biome_graphql_parser/tests/spec_test.rs
expression: snapshot
---
## Input
```graphql
extend enum Direction

extend enum Direction
NORTH
}


```

## AST

```
GraphqlRoot {
bom_token: missing (optional),
definitions: GraphqlDefinitionList [
GraphqlEnumTypeExtension {
extend_token: EXTEND_KW@0..7 "extend" [] [Whitespace(" ")],
enum_token: ENUM_KW@7..12 "enum" [] [Whitespace(" ")],
name: GraphqlName {
value_token: GRAPHQL_NAME@12..21 "Direction" [] [],
},
directives: GraphqlDirectiveList [],
enum_values: missing (optional),
},
GraphqlEnumTypeExtension {
extend_token: EXTEND_KW@21..30 "extend" [Newline("\n"), Newline("\n")] [Whitespace(" ")],
enum_token: ENUM_KW@30..35 "enum" [] [Whitespace(" ")],
name: GraphqlName {
value_token: GRAPHQL_NAME@35..44 "Direction" [] [],
},
directives: GraphqlDirectiveList [],
enum_values: GraphqlEnumValuesDefinition {
l_curly_token: missing (required),
values: GraphqlEnumValueList [
GraphqlEnumValueDefinition {
description: missing (optional),
value: GraphqlEnumValue {
graphql_name: GraphqlName {
value_token: GRAPHQL_NAME@44..52 "NORTH" [Newline("\n"), Whitespace(" ")] [],
},
},
directives: GraphqlDirectiveList [],
},
],
r_curly_token: R_CURLY@52..54 "}" [Newline("\n")] [],
},
},
],
eof_token: EOF@54..56 "" [Newline("\n"), Newline("\n")] [],
}
```

## CST

```
0: GRAPHQL_ROOT@0..56
0: (empty)
1: GRAPHQL_DEFINITION_LIST@0..54
0: GRAPHQL_ENUM_TYPE_EXTENSION@0..21
0: EXTEND_KW@0..7 "extend" [] [Whitespace(" ")]
1: ENUM_KW@7..12 "enum" [] [Whitespace(" ")]
2: GRAPHQL_NAME@12..21
0: GRAPHQL_NAME@12..21 "Direction" [] []
3: GRAPHQL_DIRECTIVE_LIST@21..21
4: (empty)
1: GRAPHQL_ENUM_TYPE_EXTENSION@21..54
0: EXTEND_KW@21..30 "extend" [Newline("\n"), Newline("\n")] [Whitespace(" ")]
1: ENUM_KW@30..35 "enum" [] [Whitespace(" ")]
2: GRAPHQL_NAME@35..44
0: GRAPHQL_NAME@35..44 "Direction" [] []
3: GRAPHQL_DIRECTIVE_LIST@44..44
4: GRAPHQL_ENUM_VALUES_DEFINITION@44..54
0: (empty)
1: GRAPHQL_ENUM_VALUE_LIST@44..52
0: GRAPHQL_ENUM_VALUE_DEFINITION@44..52
0: (empty)
1: GRAPHQL_ENUM_VALUE@44..52
0: GRAPHQL_NAME@44..52
0: GRAPHQL_NAME@44..52 "NORTH" [Newline("\n"), Whitespace(" ")] []
2: GRAPHQL_DIRECTIVE_LIST@52..52
2: R_CURLY@52..54 "}" [Newline("\n")] []
2: EOF@54..56 "" [Newline("\n"), Newline("\n")] []

```

## Diagnostics

```
enum_extension.graphql:3:1 parse ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━

× Expected at least one directive or a set of enum values

1 │ extend enum Direction
2 │
> 3 │ extend enum Direction
│ ^^^^^^
4 │ NORTH
5 │ }

enum_extension.graphql:4:3 parse ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━

× expected `{` but instead found `NORTH`

3 │ extend enum Direction
> 4 │ NORTH
│ ^^^^^
5 │ }
6 │

i Remove NORTH

```
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ GraphqlRoot {
```
interface_extension.graphql:3:1 parse ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━

× Expected at least one directive, implements interface or fields definition
× Expected at least one directive, implements interface or a set of fields definition

1 │ extend interface Story
2 │
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ GraphqlRoot {
```
object_extension.graphql:3:1 parse ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━

× Expected at least one directive, implements interface or fields definition
× Expected at least one directive, implements interface or a set of fields definition

1 │ extend type Story
2 │
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ GraphqlRoot {
```
union_extension.graphql:3:1 parse ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━

× Expected at least one directive or union member types
× Expected at least one directive or a set union member types

1 │ extend union SearchResult
2 │
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
extend enum Direction @deprecated

extend enum Direction {
NORTH
}

extend enum Direction @deprecated {
NORTH
}
Loading
Loading