Implement type/union suppression

[LWprogramming]

Update: I decided to cut scope for this PR and the other one with its tests to only focus on suppressing object types and unions to make this easier to read.

Things not implemented here but coming soon:

• field suppression

• suppressing enums and interfaces-- makes more sense after implementing field suppression

• renamings error checking (e.g. if a value in renamings doesn't exist in the schema)

• looking into reverse_name_map optimization (as described below)

--

This PR implements:

• Suppressing object types, and unions

• Error-reporting for any illegal states that this can produce.

WIP: suppressing enums and interfaces + various documentation. I suspect enums might already be taken care of, but I haven't confirmed it. Also, basic linting, formatting, readability, better error messages, etc.

Tests are in PR 858 to keep the PRs from getting too big.

--

Questions:

• The code doesn't touch user-defined scalars and enums, even if renamings includes it. What makes them different from types/ interfaces/ etc? Is it because we can't query for scalars and enums? I haven't been able to find anything that explicitly explains this.

• If we have a field of a type that we suppress, should we automatically suppress the field as well or raise a CascadingSuppressionError requiring the user explicitly suppress those fields as well (when we implement field suppression)? I implemented the latter because we chose to make things as explicit as possible but wonder if this might make it annoying if many fields are of a type that gets suppressed.

• From here: "A concern was preserving an interface but not all of its implementations, since then a user could query for all instances of that interface and get back objects that are of types that don't exist in the schema." Did we ever decide on what to do?

--

Questions that don't affect end-user functionality

• From here: what should we do if renamings attempts to rename or suppress a type/ interface/ etc. if that type doesn't appear in the schema. Should we simply ignore it (as we do now), or show a warning/ error? In addition, what if renaming attempts to do a 1-1 renaming from something to itself? Right now it doesn't change anything, but should we warn the user at least that this won't do anything (in case there was a typo)?

• What was the reasoning behind having reverse_name_map in _rename_types include non-renamed types? If it's not necessary anymore to store everything, I can refactor this to avoid problems when the schema gets very large.

[obi1kenobi]

Great questions! Replies inline:

Questions:

• The code doesn't touch user-defined scalars and enums, even if renamings includes it. What makes them different from types/ interfaces/ etc? Is it because we can't query for scalars and enums? I haven't been able to find anything that explicitly explains this.

I'm not sure there's a particular good reason. The current code is a proof-of-concept-level prototype, and I wouldn't assume all the design decisions there are meant to be meaningful and cover all edge cases.

We should definitely not silently ignore renamings input. We should either error out if a user attempts to rename enum/scalar/input types, or actually implement that renaming. We don't have to pick the same decision for all of these special types (e.g. I'd error for input types for now at least), and we should be explicit about the combination we do and do not support.

• If we have a field of a type that we suppress, should we automatically suppress the field as well or raise a CascadingSuppressionError requiring the user explicitly suppress those fields as well (when we implement field suppression)? I implemented the latter because we chose to make things as explicit as possible but wonder if this might make it annoying if many fields are of a type that gets suppressed.

Let's follow our overall strategy:

• Renaming types only renames types, and complains via errors if such renaming is illegal and would require doing more work first, such as field suppression.
• Offer a helper function that, for a given set of desired type renamings, calculates the exact "more work" needed and produces it in a form that is easy for a human to review and then feed into the functions that will apply the required schema changes such as the necessary field suppressions.

In a sense, we are designing a process that separates the "make diff" from the "apply diff" step:

• allows a human to specify type renamings
• get a reviewable and complete diff of the resulting changes to the schema as a whole
• review and sign off on it
• apply the prerequisite changes (field suppressions etc.)
• finally, apply the desired type renamings to get the final schema.

• From here: "A concern was preserving an interface but not all of its implementations, since then a user could query for all instances of that interface and get back objects that are of types that don't exist in the schema." Did we ever decide on what to do?

I've given this some thought, and the only thing that I can convince myself is safe is to require that all fields (both root schema query fields as well as edges) that point to that interface type are suppressed. The interface type can be kept purely to signal to a user that the remaining implementations share some common structure, but no query should be able to be constructed where a scope within that query is of that interface type while not also being a more-specific type (e.g. another interface that implements that one, or a type that implements that interface).

It seems to me that any approach other than this would have to deal with the "get back objects of types that don't exist in the schema" problem, and would require the compiler to do additional query rewriting such as adding __typename @filter(op_name: "!=", value: ["$removed_type"]) clauses. While this is something that would definitely be possible to build, let's save it for a future version and implement the simpler approach first.

• From here: what should we do if renamings attempts to rename or suppress a type/ interface/ etc. if that type doesn't appear in the schema. Should we simply ignore it (as we do now), or show a warning/ error? In addition, what if renaming attempts to do a 1-1 renaming from something to itself? Right now it doesn't change anything, but should we warn the user at least that this won't do anything (in case there was a typo)?

Let's raise an error in both cases, and verify this with tests. These are both cases where the user may have either made a typo or may be operating under a mental model that doesn't match the schema the compiler is looking at, and we should let them know of this fact by raising an error.

Ideally, let's find all such "invalid input" type problems with the specified renamings, and raise an error with all of them at once. If there are more than a couple such problems, it would be infuriating from a UX perspective to have to find and fix them one at a time. Let's aim for better UX than "infuriating" 😄

• What was the reasoning behind having reverse_name_map in _rename_types include non-renamed types? If it's not necessary anymore to store everything, I can refactor this to avoid problems when the schema gets very large.

To be honest, I don't remember if there was a specific reason. I think we may have been attempting to err on the side of correctness rather than performance, and weren't worrying about the memory footprint. If it doesn't seem necessary, feel free to amend the code to not store everything, since we now know our schemas can be reasonably measured in GB of RAM 😄

[obi1kenobi]

Looks headed in a great direction!

[LWprogramming]

Noting here that, after this PR gets merged, we'll be able to update some type hints and remove unnecessary code based on the changes in the latest GraphQL-core release.

[obi1kenobi]

A couple of minor nits, the code itself looks great.

[obi1kenobi]

The code seemed fine, so I did a pass on the documentation and left a few comments. Nothing major.

[obi1kenobi]

The module docstring looks great btw! I think we'll be able to turn it into documentation on ReadTheDocs pretty easily given its present structure.

[obi1kenobi]

Awesome job!