vektah / gqlparser

A port of the parser from graphql-js into golang
MIT License
498 stars 123 forks source link

call with empty parameter list is considered a parse error #300

Closed jhelberg closed 5 months ago

jhelberg commented 5 months ago

What happened?

I have the following in my definition file under Queries:

    "Return the participant representing the logged on user."
    mmi(): participant

gqlparse bombs out with:

expected at least one definition, found )

What did you expect?

I think it is allowed to have a query without paratemers. I expect gqlparse to accept this line.

Minimal graphql.schema and models to reproduce

schema {
    query: Query
}

type participant {
          id:        ID!
    nickname:        String!
}
type Query {
    "Return the participant representing the logged on user."
    mmi(): participant
}

versions

StevenACoffman commented 5 months ago

Thank you for taking the time to report this, but I'm not sure this is a bug, even if it is a change in behavior from the past.

The GraphQL documentation gives these examples showing omitting parenthesis for queries when there are no arguments:

type Query {
  books: [Book]
  authors: [Author]
}

And the spec states that if you try to add empty parenthesis like:

query GetSomething() { # <-- empty arguments list here
  user() { # <-- empty argument list here
    name
  }
}

There should be a validation error in Line 1 and Line 2 each because in GraphQL, Arguments must contain at least one Argument. In GraphQL spec, the notation [list] means one or more -- see spec definition

Note that the entire "Arguments" portion is optional in both contexts, therefore query GetSomething { ... } is allowed and so is user { ... }, but if the braces () are present they must contain something.

graphql-js reports errors on both cases correctly.

This parser used to allow empty parenthesis, but we are trying to maintain spec compliance. The change was made here: https://github.com/vektah/gqlparser/pull/293

jhelberg commented 5 months ago

thx for taking the time. Seems that () is not allowed indeed. regards.