Typecheck rewrite by rmosolgo · Pull Request #268 · rmosolgo/graphql-ruby

(This isn't done yet -- just a WIP branch in case you're interested in the progress here.)

GraphQL::StaticValidation is one of the oldest, dustiest parts of graphql-ruby. It makes sense: it was important to do right away, and it's basically worked, so I haven't messed with it. But, I think revisiting this problem could yield dividends. Here are my goals in rewriting that functionality:

  • Support validation without a schema: some validations can be run a query string alone. If we decouple the code properly, we get that behavior at no extra cost!
  • Improve performance: I have doubts about some of the rules, see below.
  • Reduce ad-hoc traversals: some existing rules re-traverse the AST at the end of validation. Besides being a huge waste of time, this is buggy, since each rule has to implement its own prevention of infinite loops (see Circular references in fragments, but shouldn't! #257)
  • Find more errors: there are many cases where the current implementation finds one error then stops the traversal. Technically, we can do better: we can continue traversing, looking for type-agnostic usage errors, and possibly regaining type information (eg, inline fragment) so we can continue to type-check.
  • Improve maintainability: currently, the rules are order-dependent: some of them require that previous rules halt traversal in case of an error. So, if you reorder them, 💥. Also, I want to run all tests through the whole validation stack instead of one-validator-at-a-time. This is because there have been lots of bugs about how validators work together (or don't work together)

My approach has a few components:

  • Only one tree traversal. Gather important info during the one traversal, then distribute it to individual rules for their own purposes.
  • Recover from type errors. Use AnyType, AnyField and friends to inject "magical" types in that case. This requires adding some methods to the "real" types and fields so that they can be easily imitated.
  • Lump more behavior together. It might result in bigger files, but I want to avoid cases where code dependencies are not clear because they've been (wrongfully) extracted to other files. Also, I had a previous goal of matching Ruby files one-to-one with paragraphs in the spec. I don't have that goal anymore -- now my goal is to implement the spec as best I can, in Ruby.
  • Explicit separation of schema-dependent rules and schema-independent rules. It doesn't have to be a great API, but there should be an API for validating a document only. (As long as the behavior is there, we can improve the API later.)

Generally, the steps are:

  • Reimplement (and retest 😩 ) document validation
  • Run some benchmarks
  • Make it faster, if it isn't already faster than the current implementation
  • Document the new APIs
  • 🚢

TODO:

  • Reimplement validations
  • Refactor: type_check.rb should only hold calls on shared state -- everything else should be extracted
  • Refactor: rename typecheck modules & functions
  • Test: run existing tests through new code, make sure there are no regressions
  • Refactor: Migrate new code to StaticValidation