Skip to content

Commit

Permalink
printSchema: handle descriptions that are non-printable as block strings
Browse files Browse the repository at this point in the history
Fixes #2577
Fixes #2589
  • Loading branch information
IvanGoncharov committed Nov 22, 2021
1 parent 913fa9b commit 867a103
Show file tree
Hide file tree
Showing 5 changed files with 165 additions and 13 deletions.
29 changes: 18 additions & 11 deletions src/language/__tests__/blockString-fuzz.ts
Expand Up @@ -8,7 +8,7 @@ import { invariant } from '../../jsutils/invariant';

import { Lexer } from '../lexer';
import { Source } from '../source';
import { printBlockString } from '../blockString';
import { printBlockString, isPrintableAsBlockString } from '../blockString';

function lexValue(str: string): string {
const lexer = new Lexer(new Source(str));
Expand All @@ -35,6 +35,18 @@ function testPrintableBlockString(
);
}

function testNonPrintableBlockString(testValue: string): void {
const blockString = printBlockString(testValue);
const printedValue = lexValue(blockString);
invariant(
testValue !== printedValue,
dedent`
Expected lexValue(${inspectStr(blockString)})
to not equal ${inspectStr(testValue)}
`,
);
}

describe('printBlockString', () => {
it('correctly print random strings', () => {
// Testing with length >7 is taking exponentially more time. However it is
Expand All @@ -43,18 +55,13 @@ describe('printBlockString', () => {
allowedChars: ['\n', '\t', ' ', '"', 'a', '\\'],
maxLength: 7,
})) {
const testStr = '"""' + fuzzStr + '"""';

let testValue;
try {
testValue = lexValue(testStr);
} catch (e) {
continue; // skip invalid values
if (!isPrintableAsBlockString(fuzzStr)) {
testNonPrintableBlockString(fuzzStr);
continue;
}
invariant(typeof testValue === 'string');

testPrintableBlockString(testValue);
testPrintableBlockString(testValue, { minimize: true });
testPrintableBlockString(fuzzStr);
testPrintableBlockString(fuzzStr, { minimize: true });
}
}).timeout(20000);
});
69 changes: 68 additions & 1 deletion src/language/__tests__/blockString-test.ts
@@ -1,7 +1,11 @@
import { expect } from 'chai';
import { describe, it } from 'mocha';

import { dedentBlockStringLines, printBlockString } from '../blockString';
import {
isPrintableAsBlockString,
dedentBlockStringLines,
printBlockString,
} from '../blockString';

function joinLines(...args: ReadonlyArray<string>) {
return args.join('\n');
Expand Down Expand Up @@ -135,6 +139,69 @@ describe('dedentBlockStringLines', () => {
});
});

describe('isPrintableAsBlockString', () => {
function expectPrintable(str: string) {
return expect(isPrintableAsBlockString(str)).to.equal(true);
}

function expectNonPrintable(str: string) {
return expect(isPrintableAsBlockString(str)).to.equal(false);
}

it('accepts valid strings', () => {
expectPrintable('');
expectPrintable(' a');
expectPrintable('\t"\n"');
expectNonPrintable('\t"\n \n\t"');
});

it('rejects strings with only whitespace', () => {
expectNonPrintable(' ');
expectNonPrintable('\t');
expectNonPrintable('\t ');
expectNonPrintable(' \t');
});

it('rejects strings with non-printable character', () => {
expectNonPrintable('\x00');
expectNonPrintable('a\x00b');
});

it('rejects strings with non-printable character', () => {
expectNonPrintable('\x00');
expectNonPrintable('a\x00b');
});

it('rejects strings with only empty lines', () => {
expectNonPrintable('\n');
expectNonPrintable('\n\n');
expectNonPrintable('\n\n\n');
expectNonPrintable(' \n \n');
expectNonPrintable('\t\n\t\t\n');
});

it('rejects strings with carriage return', () => {
expectNonPrintable('\r');
expectNonPrintable('\n\r');
expectNonPrintable('\r\n');
expectNonPrintable('a\rb');
});

it('rejects strings with leading empty lines', () => {
expectNonPrintable('\na');
expectNonPrintable(' \na');
expectNonPrintable('\t\na');
expectNonPrintable('\n\na');
});

it('rejects strings with leading empty lines', () => {
expectNonPrintable('a\n');
expectNonPrintable('a\n ');
expectNonPrintable('a\n\t');
expectNonPrintable('a\n\n');
});
});

describe('printBlockString', () => {
function expectBlockString(str: string) {
return {
Expand Down
63 changes: 63 additions & 0 deletions src/language/blockString.ts
Expand Up @@ -48,6 +48,69 @@ function leadingWhitespace(str: string): number {
return i;
}

/**
* @internal
*/
export function isPrintableAsBlockString(value: string): boolean {
if (value === '') {
return true; // empty string is printable
}

let isEmptyLine = true;
let hasIndent = false;
let hasCommonIndent = true;
let seenNonEmptyLine = false;

for (let i = 0; i < value.length; ++i) {
switch (value.codePointAt(i)) {
case 0x0000:
case 0x0001:
case 0x0002:
case 0x0003:
case 0x0004:
case 0x0005:
case 0x0006:
case 0x0007:
case 0x0008:
case 0x000b:
case 0x000c:
case 0x000e:
case 0x000f:
return false; // Has non-printable characters

case 0x000d: // \r
return false; // Has \r or \r\n which will be replaced as \n

case 10: // \n
if (isEmptyLine && !seenNonEmptyLine) {
return false; // Has leading new line
}
seenNonEmptyLine = true;

isEmptyLine = true;
hasIndent = false;
break;
case 9: // \t
case 32: // <space>
hasIndent ||= isEmptyLine;
break;
default:
hasCommonIndent &&= hasIndent;
isEmptyLine = false;
}
}

if (isEmptyLine) {
return false; // Has trailing empty lines
}

if (hasCommonIndent && seenNonEmptyLine) {
return false; // Has internal indent
}

return true;
}

/**
* Print a block string in the indented block form by adding a leading and
* trailing blank line. However, if a block string starts with whitespace and is
Expand Down
14 changes: 14 additions & 0 deletions src/utilities/__tests__/printSchema-test.ts
Expand Up @@ -593,6 +593,20 @@ describe('Type System Printer', () => {
`);
});

it('Prints an description with only whitespace', () => {
const schema = buildSingleFieldSchema({
type: GraphQLString,
description: ' ',
});

expectPrintedSchema(schema).to.equal(dedent`
type Query {
" "
singleField: String
}
`);
});

it('One-line prints a short description', () => {
const schema = buildSingleFieldSchema({
type: GraphQLString,
Expand Down
3 changes: 2 additions & 1 deletion src/utilities/printSchema.ts
Expand Up @@ -4,6 +4,7 @@ import type { Maybe } from '../jsutils/Maybe';

import { Kind } from '../language/kinds';
import { print } from '../language/printer';
import { isPrintableAsBlockString } from '../language/blockString';

import type { GraphQLSchema } from '../type/schema';
import type { GraphQLDirective } from '../type/directives';
Expand Down Expand Up @@ -316,7 +317,7 @@ function printDescription(
const blockString = print({
kind: Kind.STRING,
value: description,
block: true,
block: isPrintableAsBlockString(description),
});

const prefix =
Expand Down

0 comments on commit 867a103

Please sign in to comment.