Fix printed schema descriptions that end with a double quote (")

[sethtippetts]

Schemas printed with printSchema that contain descriptions ending with a double-quote character are not parseable and throw a GraphQLError: Syntax Error: Unterminated string because the SDL description ends with 4 contiguous double-quotes

I've included a failing test case:

const Root = new GraphQLObjectType({
      name: 'Root',
      fields: {
        onlyField: {
          type: GraphQLString,
          description: 'This field is "awesome"',
        },
      },
    });
    const Schema = new GraphQLSchema({ query: Root });
    const output = printSchema(Schema);
    expect(() => parse(output)).not.to.throw(GraphQLError);

[leebyron]

cc @IvanGoncharov - I know you had a similar fix land earlier. Care to review this?

[IvanGoncharov]

cc @IvanGoncharov - I know you had a similar fix land earlier. Care to review this?

@leebyron @mostcowbell Yes I fixed similar issue with print function in #1190

Adding space changes description string and if we want to omit it than possible solution would be:

"""
This field is "awesome"
"""

Or:

"""This field is "awesome"
"""

Also, you need to handle the same edge case as print function:

"""    space-led value "quoted string"
"""

On the other hand descriptionLines already changes description string.
Moreover, all description values are in Markdown format so trailing spaces are ignored.

Personally, I think print should try to produce an output as close as possible to original but printSchema output should be readable and less-surprising. So I think @mostcowbell solution is better but it should be explicitly documented in the tests.

[sethtippetts]

@IvanGoncharov so I should add another assertion to my test?

expect(output).to.equal(`
  schema {
    query: Root
  }
  
  type Root {
    """This field is "awesome" """
    onlyField: String
  }
`);

or just a comment about the output of a description ending in a double-quote?

// Will be printed as `"""This field is "awesome" """` to avoid parsing issues

[IvanGoncharov]

@mostcowbell I would suggest being as explicit as possible:

    const description = 'This field is "awesome"';
    const output = printSingleFieldSchema({ type: GraphQLString, description });
    expect(output).to.equal(`
      ...
    `);

    const recreatedRoot = buildSchema(output).getTypeMap()['Root'];
    const recreatedField = recreatedRoot.getFields()['singleField'];

    /* Note: Additional space added to prevent parser confusing trailing double
     * quote inside description with the end of the block string. It shouldn't
     * affect result since descriptions are interpreted as Markdown so trailing
     * spaces are ignored.
     */
    expect(recreatedField.description).to.equal(description + ' ');

Disclaimer: I'm not a native speaker :)

Prints a parseable schema when a comment ends with a doublequote (")s

And change test description to something like: Prints a field description that ends with a doublequote (")s

[sethtippetts]

@IvanGoncharov I've updated my test and removed the extra dependencies I was importing into that file

[IvanGoncharov]

@mostcowbell It's better to reuse printSingleFieldSchema similar to other tests since it allows you test that printSchema output not only parse-ready but you can recreate schema based on it. I also think it's important to have the output of printSchema inside test so you would see how doublequote is escaped. So I suggest to change it like this:

    const description = 'This field is "awesome"';
    const output = printSingleFieldSchema({ type: GraphQLString, description });
    expect(output).to.equal(dedent`
      schema {
        query: Root
      }

      type Root {
        """This field is "awesome" """
        singleField: String
      }
    `);

I experiment a bit and expend test to ensure that description stays unchanged:

    const recreatedDescription = buildSchema(output)
      .getTypeMap()['Root']
      .getFields()['singleField']
      .description;
    expect(recreatedDescription).to.equal(description);

And it fails 😞

AssertionError: expected 'This field is "awesome" ' to equal 'This field is "awesome"'
                                                 ^ Note space here

[IvanGoncharov]

After giving it a thought I don't think that adding space to a description is such a big issue but it should be explicitly documented in the test, like:

expect(recreatedDescription).to.equal(description + ' ');

And since it's not so obvious issue, both test and implementation should contain a comment with an explanation for why it's needed.

[leebyron]

I'd like to see a solution that also handles the edge cases you referenced, @IvanGoncharov - I'll pull in these test cases and ensure the solution accounts for them.