Pest parser does not handle significant whitespaces correctly

[Sytten]

EDIT: I kinda opened a larger can or worms, please read all comments to understand the full issue

Expected Behavior

Given a query like:

fragment onboardingFull on OnboardingState {
  license
}

query globalConfig {
  globalConfig {
    onboarding {
      ...onboardingFull
    }
  }
}

It used to work before to update from 4.0.1 to 4.0.10. But it now interprets the "on" in the fragment name.

Actual Behavior

Parsing error:

 --> 9:5
  |
9 |     }
  |     ^---
  |
  = expected selection_set or directive

Steps to Reproduce the Problem

1. Create a fragment where the name starts with "on"
2. Send it to the server
3. See failure

Specifications

• Version: 4.0.10
• Platform: All
• Subsystem: All

[Sytten]

I believe the issue is this change in the parser: 1f67e3a#diff-7fe48281e06de04d34f56adf27094658fcfd043c278255b994be6a9357409231R19

Because the whitespace are ignored with

async-graphql/parser/src/graphql.pest

Line 1 in 69dfe25

WHITESPACE = _{ " " | "," | "\t" | "\u{feff}" | line_terminator }

It thinks the fragment spread is using a type_condition

async-graphql/parser/src/graphql.pest

Line 23 in 69dfe25

type_condition = { "on" ~ name }

I believe we can fix the issue by setting type_condition = { "on " ~ name } to tell pest that the space after on is significant. Maybe there is another way though so feel free to fix it as you like.

[Sytten]

Further reading of the current pest grammer indicates that a lot of significant whitespaces are currently ignored and this is not good. For example fragment_definition = { "fragment" ~ name ~ type_condition ~ directives? ~ selection_set } makes it so that fragmentmyfragmentonMyType is valid while it is clearly not.

The problem is to fix that we would need to make rules atomic (with ${ }) and all other rules not atomic (with !{ }). At least until pest 3 is released with pest-parser/pest#271.

For fragment that would look like:

fragment_definition = @{ "fragment" ~ WHITESPACE+ ~ name ~ WHITESPACE+ ~ type_condition ~ WHITESPACE* ~ directives? ~ WHITESPACE* ~ selection_set }
type_condition = !{ "on " ~ name }
selection_set = !{ "{" ~ selection+ ~ "}" }
directives       = !{       directive+ }

I am unsure how you want to proceed with this significant vs non significant spacing issue @sunli829, but let me know.

[sunli829]

I fixed it in the master branch, please help to test it. 🙂

[Sytten]

Will do, I believe the rest is still broken though but I guess its all right.