Carve out parser as a separate artifact

[rpiaggio]

This allows us to use the parser in clue for client-side validation of queries.

[hugo-vrijswijk]

Isn't this already defined in the cross-platform settings?

[rpiaggio]

It is — thanks. The extra line was only there to undo nativeSettings, which sets
tlVersionIntroduced to 0.26.1 (when Native moved to 0.5); that's wrong for a module
that's new in 0.31.0 on every platform. Simpler not to apply nativeSettings here at
all, so both lines are gone and Native picks up 0.31.0 from the cross-platform settings.

[armanbilge]

Actually, I think tlVersionIntroduced shouldn't be necessary anymore at all.

that's wrong for a module that's new in 0.31.0 on every platform

Indeed, but it won't be checking for bincompat for any previous version anyway.

[hugo-vrijswijk]

I think these should probably also be moved:

• schema.scala, including the Schema AST, SchemaParser and SchemaRenderer
• QueryParser from compiler.scala (but not QueryCompiler)
• Maybe minimizer.scala (QueryMinimizer). Mostly for rendering, not parsing. So maybe not needed
• syntax2.scala + syntax3.scala: parser macros

[rpiaggio]

I think these should probably also be moved:

• schema.scala, including the Schema AST, SchemaParser and SchemaRenderer
• QueryParser from compiler.scala (but not QueryCompiler)
• Maybe minimizer.scala (QueryMinimizer). Mostly for rendering, not parsing. So maybe not needed
• syntax2.scala + syntax3.scala: parser macros

I looked at each of these, and I don't think any of them can move without taking most
of core along:

• schema.scala (2615 lines) isn't just SchemaParser/SchemaRenderer: it also defines
  Schema, the entire typed type-system model (Type, ObjectType, InterfaceType,
  UnionType, InputObjectType, ...) and Value. It imports grackle.Query._,
  grackle.UntypedOperation._ and grackle.syntax._, and query.scala/operation.scala
  import back into it, so schema, query, operation and syntax are one cycle: they move
  together or not at all.
• QueryParser returns Result[(List[UntypedOperation], List[UntypedFragment])], so it
  needs operation.scala and therefore that whole cycle.
• QueryMinimizer renders through SchemaRenderer, so it's in the same cluster (you
  suspected as much).
• syntax2.scala/syntax3.scala define schema"..." next to doc"...", and
  schema"..." needs Schema and SchemaParser.

Together that's ~3,800 lines, i.e. everything except the compiler, mapping, interpreter
and cursor. That may well be a module worth having, but it wouldn't be a parser — it'd be
grackle's language and type model — and it would want a different name and a much more
opinionated cut than this PR makes.

The current boundary is "GraphQL text in, Ast out": ast.scala and parser.scala, with
result.scala and problem.scala only because parseText returns Result. That's
self-contained and complete for the use case that motivated it (a compile-time GraphQL
interpolator that needs an AST and nothing else). Happy to look at the larger split as a
follow-up if that's the direction you'd prefer.

[milessabin]

Together that's ~3,800 lines, i.e. everything except the compiler, mapping, interpreter
and cursor. That may well be a module worth having, but it wouldn't be a parser — it'd be
grackle's language and type model — and it would want a different name and a much more
opinionated cut than this PR makes.

I think that's right, and I think this is a better partitioning. I don't doubt that you have uses for "text in, ast out", but I think it will be far more common to also need the semantics, and ability to check and normalize queries.

[rpiaggio]

Together that's ~3,800 lines, i.e. everything except the compiler, mapping, interpreter
and cursor. That may well be a module worth having, but it wouldn't be a parser — it'd be
grackle's language and type model — and it would want a different name and a much more
opinionated cut than this PR makes.

I think that's right, and I think this is a better partitioning. I don't doubt that you have uses for "text in, ast out", but I think it will be far more common to also need the semantics, and ability to check and normalize queries.

The use case that motivates this cut is client query validation in clue, which only needs the AST for compile-time validation. Having more functionality in the new module wouldn't hurt this scenario except that the current cut makes the new module mostly spec-driven, which makes it quite stable. I'm worried that moving more code into it will induce more frequent change and that will mean that clue's release cycle will have to follow grackle's more closely. This is not just maintenance laziness: having this module introduces a dependency diamond for us, so it's a boon that it's as stable as possible.

If we do want to extract the full functionality involving semantics and schema validation, I'd push for a 3rd module. Also, this is a refactor that implies splitting files, not just moving them. For example, Query is entangled with the runtime: Query.Filter holds a Predicate, Query.Component holds a Mapping[F] and a (Query, Cursor) => Result[Query].

EDIT: We don't even need an AST in clue, a lexer will do. That would make the module even more stable. But there's no such thing in grackle.

[milessabin]

The use case that motivates this cut is client query validation in clue, which only needs the AST for compile-time validation.

The AST can only tell you that a query is syntactically valid. You need the schema to know if it will actually be accepted by the server.