Skip to content

Commit 867a103

Browse files
committed
printSchema: handle descriptions that are non-printable as block strings
Fixes #2577 Fixes #2589
1 parent 913fa9b commit 867a103

File tree

5 files changed

+165
-13
lines changed

5 files changed

+165
-13
lines changed

src/language/__tests__/blockString-fuzz.ts

Lines changed: 18 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ import { invariant } from '../../jsutils/invariant';
88

99
import { Lexer } from '../lexer';
1010
import { Source } from '../source';
11-
import { printBlockString } from '../blockString';
11+
import { printBlockString, isPrintableAsBlockString } from '../blockString';
1212

1313
function lexValue(str: string): string {
1414
const lexer = new Lexer(new Source(str));
@@ -35,6 +35,18 @@ function testPrintableBlockString(
3535
);
3636
}
3737

38+
function testNonPrintableBlockString(testValue: string): void {
39+
const blockString = printBlockString(testValue);
40+
const printedValue = lexValue(blockString);
41+
invariant(
42+
testValue !== printedValue,
43+
dedent`
44+
Expected lexValue(${inspectStr(blockString)})
45+
to not equal ${inspectStr(testValue)}
46+
`,
47+
);
48+
}
49+
3850
describe('printBlockString', () => {
3951
it('correctly print random strings', () => {
4052
// Testing with length >7 is taking exponentially more time. However it is
@@ -43,18 +55,13 @@ describe('printBlockString', () => {
4355
allowedChars: ['\n', '\t', ' ', '"', 'a', '\\'],
4456
maxLength: 7,
4557
})) {
46-
const testStr = '"""' + fuzzStr + '"""';
47-
48-
let testValue;
49-
try {
50-
testValue = lexValue(testStr);
51-
} catch (e) {
52-
continue; // skip invalid values
58+
if (!isPrintableAsBlockString(fuzzStr)) {
59+
testNonPrintableBlockString(fuzzStr);
60+
continue;
5361
}
54-
invariant(typeof testValue === 'string');
5562

56-
testPrintableBlockString(testValue);
57-
testPrintableBlockString(testValue, { minimize: true });
63+
testPrintableBlockString(fuzzStr);
64+
testPrintableBlockString(fuzzStr, { minimize: true });
5865
}
5966
}).timeout(20000);
6067
});

src/language/__tests__/blockString-test.ts

Lines changed: 68 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,11 @@
11
import { expect } from 'chai';
22
import { describe, it } from 'mocha';
33

4-
import { dedentBlockStringLines, printBlockString } from '../blockString';
4+
import {
5+
isPrintableAsBlockString,
6+
dedentBlockStringLines,
7+
printBlockString,
8+
} from '../blockString';
59

610
function joinLines(...args: ReadonlyArray<string>) {
711
return args.join('\n');
@@ -135,6 +139,69 @@ describe('dedentBlockStringLines', () => {
135139
});
136140
});
137141

142+
describe('isPrintableAsBlockString', () => {
143+
function expectPrintable(str: string) {
144+
return expect(isPrintableAsBlockString(str)).to.equal(true);
145+
}
146+
147+
function expectNonPrintable(str: string) {
148+
return expect(isPrintableAsBlockString(str)).to.equal(false);
149+
}
150+
151+
it('accepts valid strings', () => {
152+
expectPrintable('');
153+
expectPrintable(' a');
154+
expectPrintable('\t"\n"');
155+
expectNonPrintable('\t"\n \n\t"');
156+
});
157+
158+
it('rejects strings with only whitespace', () => {
159+
expectNonPrintable(' ');
160+
expectNonPrintable('\t');
161+
expectNonPrintable('\t ');
162+
expectNonPrintable(' \t');
163+
});
164+
165+
it('rejects strings with non-printable character', () => {
166+
expectNonPrintable('\x00');
167+
expectNonPrintable('a\x00b');
168+
});
169+
170+
it('rejects strings with non-printable character', () => {
171+
expectNonPrintable('\x00');
172+
expectNonPrintable('a\x00b');
173+
});
174+
175+
it('rejects strings with only empty lines', () => {
176+
expectNonPrintable('\n');
177+
expectNonPrintable('\n\n');
178+
expectNonPrintable('\n\n\n');
179+
expectNonPrintable(' \n \n');
180+
expectNonPrintable('\t\n\t\t\n');
181+
});
182+
183+
it('rejects strings with carriage return', () => {
184+
expectNonPrintable('\r');
185+
expectNonPrintable('\n\r');
186+
expectNonPrintable('\r\n');
187+
expectNonPrintable('a\rb');
188+
});
189+
190+
it('rejects strings with leading empty lines', () => {
191+
expectNonPrintable('\na');
192+
expectNonPrintable(' \na');
193+
expectNonPrintable('\t\na');
194+
expectNonPrintable('\n\na');
195+
});
196+
197+
it('rejects strings with leading empty lines', () => {
198+
expectNonPrintable('a\n');
199+
expectNonPrintable('a\n ');
200+
expectNonPrintable('a\n\t');
201+
expectNonPrintable('a\n\n');
202+
});
203+
});
204+
138205
describe('printBlockString', () => {
139206
function expectBlockString(str: string) {
140207
return {

src/language/blockString.ts

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,69 @@ function leadingWhitespace(str: string): number {
4848
return i;
4949
}
5050

51+
/**
52+
* @internal
53+
*/
54+
export function isPrintableAsBlockString(value: string): boolean {
55+
if (value === '') {
56+
return true; // empty string is printable
57+
}
58+
59+
let isEmptyLine = true;
60+
let hasIndent = false;
61+
let hasCommonIndent = true;
62+
let seenNonEmptyLine = false;
63+
64+
for (let i = 0; i < value.length; ++i) {
65+
switch (value.codePointAt(i)) {
66+
case 0x0000:
67+
case 0x0001:
68+
case 0x0002:
69+
case 0x0003:
70+
case 0x0004:
71+
case 0x0005:
72+
case 0x0006:
73+
case 0x0007:
74+
case 0x0008:
75+
case 0x000b:
76+
case 0x000c:
77+
case 0x000e:
78+
case 0x000f:
79+
return false; // Has non-printable characters
80+
81+
case 0x000d: // \r
82+
return false; // Has \r or \r\n which will be replaced as \n
83+
84+
case 10: // \n
85+
if (isEmptyLine && !seenNonEmptyLine) {
86+
return false; // Has leading new line
87+
}
88+
seenNonEmptyLine = true;
89+
90+
isEmptyLine = true;
91+
hasIndent = false;
92+
break;
93+
case 9: // \t
94+
case 32: // <space>
95+
hasIndent ||= isEmptyLine;
96+
break;
97+
default:
98+
hasCommonIndent &&= hasIndent;
99+
isEmptyLine = false;
100+
}
101+
}
102+
103+
if (isEmptyLine) {
104+
return false; // Has trailing empty lines
105+
}
106+
107+
if (hasCommonIndent && seenNonEmptyLine) {
108+
return false; // Has internal indent
109+
}
110+
111+
return true;
112+
}
113+
51114
/**
52115
* Print a block string in the indented block form by adding a leading and
53116
* trailing blank line. However, if a block string starts with whitespace and is

src/utilities/__tests__/printSchema-test.ts

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -593,6 +593,20 @@ describe('Type System Printer', () => {
593593
`);
594594
});
595595

596+
it('Prints an description with only whitespace', () => {
597+
const schema = buildSingleFieldSchema({
598+
type: GraphQLString,
599+
description: ' ',
600+
});
601+
602+
expectPrintedSchema(schema).to.equal(dedent`
603+
type Query {
604+
" "
605+
singleField: String
606+
}
607+
`);
608+
});
609+
596610
it('One-line prints a short description', () => {
597611
const schema = buildSingleFieldSchema({
598612
type: GraphQLString,

src/utilities/printSchema.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import type { Maybe } from '../jsutils/Maybe';
44

55
import { Kind } from '../language/kinds';
66
import { print } from '../language/printer';
7+
import { isPrintableAsBlockString } from '../language/blockString';
78

89
import type { GraphQLSchema } from '../type/schema';
910
import type { GraphQLDirective } from '../type/directives';
@@ -316,7 +317,7 @@ function printDescription(
316317
const blockString = print({
317318
kind: Kind.STRING,
318319
value: description,
319-
block: true,
320+
block: isPrintableAsBlockString(description),
320321
});
321322

322323
const prefix =

0 commit comments

Comments
 (0)