SchemaPrinter: Directives support 🤩

[LastDragon-ru]

Features:

• Seems no API breaking changes (but output for long lines may be a bit different, see below);
• By default it has the same behavior as before; directives printing should be enabled explicitly via printDirectives option;
• Supported location: ARGUMENT_DEFINITION, ENUM, ENUM_VALUE, FIELD_DEFINITION, INPUT_FIELD_DEFINITION, INPUT_OBJECT, INTERFACE, OBJECT, SCALAR, UNION;
• Long directives (= with a lot of arguments) will be split into multiple lines
• Long list of arguments will be split into multiple lines 🆕
• Long List values ([1,2,3]) will be split into multiple lines 🆕
• Better separation for children with long-line/description 🆕

# Example
input InputB
@test(
  value: "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"
) {
  a: [String] = [
    "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"
    "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"
    "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"
  ]
}

# Before
enum Color {
  RED

  """Not a creative color"""
  GREEN
  BLUE
}

#After
enum Color {
  RED

  """Not a creative color"""
  GREEN

  BLUE
}

Closes: #552, #875

[codecov]

Codecov Report

Merging #996 (5c51b1b) into master (e18d52d) will increase coverage by 0.04%.
The diff coverage is 99.29%.

@@             Coverage Diff              @@
##             master     #996      +/-   ##
============================================
+ Coverage     94.56%   94.61%   +0.04%     
  Complexity       53       53              
============================================
  Files           120      120              
  Lines          9609     9711     +102     
============================================
+ Hits           9087     9188     +101     
- Misses          522      523       +1     

Impacted Files	Coverage Δ
src/Utils/SchemaPrinter.php	99.30% <99.29%> (-0.16%)	⬇️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e18d52d...5c51b1b. Read the comment docs.

[vhenzl]

Some of my thoughts from the first impression:

• This PR seems to be solving more unrelated problems – directive printing and long lines/args/lists printing. Breaking it into more scoped PRs would make it easier to understand.
• As for the main topic, directive printing, I believe arguments from Include directives in SchemaPrinter output #552 still holds. It's a complex problem and this library should wait until it is resolved in the reference implementation.
• Regarding long lines/args/lists wrapping, what problem is it trying to solve? How does it match the reference implementation and GraphQL spec? Ideally, schema building and printing should be reversible, i.e. sdl === printSchema(buildSchema(sdl)). The library doesn't work like that on 100% yet (see Printed schema should preserve original order of types #954), but there have been some other changes done in this direction (e.g. Unify Printer and SchemaPrinter output for block strings #938 removes long description wrapping). These changes seem to go against this effort.

[LastDragon-ru]

This PR seems to be solving more unrelated problems – directive printing and long lines/args/lists printing.

Nope, it is a free bonus 😁 But you are welcome to split it if you want.

Regarding long lines/args/lists wrapping, what problem is it trying to solve?

The same problem as the multiline description - readability. This is impossible to read:

input InputB @test(value: "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", a: "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa") @deprecated(reason: "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa") {
  a: [String] = ["aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa",  "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa",  "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"] @test(value: "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", a: "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa") @deprecated(reason: "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa")
}

As for the main topic, directive printing, I believe arguments from Include directives in SchemaPrinter output #552 still holds. It's a complex problem and this library should wait until it is resolved in the reference implementation.

1. I don't see any arguments why directives should not be printed in related issues. Yes, some of the directives probably should not be included in the output, and this PR allows you to control which directives should be printed;
2. Directives are very important: for example, we use directives to check permissions and they are required for UI team, right now we are using descriptions but it sucks.
3. They are extremely useful to check schema after automatic transformation (more related to Lighthouse), right now there is no way to check added directives and this is a big problem (eg my @searcBy adds found operators as arguments, but I cannot just check the list; or in our project we generate some fields with directives automatically and also cannot check final schema)
4. Moreover, they are already used by many others: GitHub GraphQL API, Lighthouse Federated printer (and Apollo), etc;
5. As I can see the upstream issue is dead.

Ideally, schema building and printing should be reversible, i.e. sdl === printSchema(buildSchema(sdl))

Parser strips whitespace while parsing so I guess that is not possible... But you are right, my second goal - generate GraphlQL that can be read and compared by humans even if the schema was generated by robots (= without formatting). Because from my point of view if readability doesn't matter there are no reasons to add LFs and paddings at all.

Also, all original tests except two related to enum members are passed, the difference will be only in (this case is not covered by original tests):

test(aaaaa: String, bbbb: String, cccc: String, dddddddddcccc: String, fsdfsdfsdfcccc: String, dsafsdcccc: String, sdfdfsdfcccc: String, sdfsdfsdcccc: String, sdfsdfsdfsfsdfcccc: String, sdfsfsdfdfsdfcccc: String): String;

That will be converted into much more readable form:

test(
  aaaaa: String
  bbbb: String
  cccc: String
  dddddddddcccc: String
  fsdfsdfsdfcccc: String
  dsafsdcccc: String
  sdfdfsdfcccc: String
  sdfsdfsdcccc: String
  sdfsdfsdfsfsdfcccc: String
  sdfsfsdfdfsdfcccc: String
): String;

[vhenzl]

Readability can be an issue, the last example is a good one. There actually has been some work done on that topic in the reference implementation (graphql/graphql-js#2797). But in general, I would argue that the library shouldn't do any sophisticated formatting. Instead, tools like Prettier can be used to format a printed GraphQL if needed. Anyway, it's unrelated to directive printing.

[spawnia]

See graphql/graphql-js#3362 for an attempt to get support into the reference implementation first. Let's see what they say before continuing here.

[LastDragon-ru]

PR is outdated and I have no time to update it. If someone need directives maybe you can try to use (much much more customizable) SchemaPrinter from lara-asp package.