Skip to content

Commit 4d5d0d6

Browse files
committed
feat(core): add escapeSingleQuotes option for enum change messages
1 parent 2d22d32 commit 4d5d0d6

File tree

7 files changed

+150
-16
lines changed

7 files changed

+150
-16
lines changed

.changeset/large-walls-wash.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'@graphql-inspector/core': minor
3+
---
4+
5+
Escape single quotes in all enum descriptions and deprecation reasons for better data integrity

packages/core/__tests__/diff/enum.test.ts

Lines changed: 102 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -274,4 +274,105 @@ describe('enum', () => {
274274
expect(change.criticality.reason).toBeDefined();
275275
expect(change.message).toEqual(`Enum value 'C' was added to enum 'enumA'`);
276276
});
277-
});
277+
278+
describe('string escaping', () => {
279+
test('deprecation reason changed with escaped single quotes', async () => {
280+
const a = buildSchema(/* GraphQL */ `
281+
type Query {
282+
fieldA: String
283+
}
284+
285+
enum enumA {
286+
A @deprecated(reason: "It's old")
287+
B
288+
}
289+
`);
290+
291+
const b = buildSchema(/* GraphQL */ `
292+
type Query {
293+
fieldA: String
294+
}
295+
296+
enum enumA {
297+
A @deprecated(reason: "It's new")
298+
B
299+
}
300+
`);
301+
302+
const changes = await diff(a, b);
303+
const change = findFirstChangeByPath(changes, 'enumA.A');
304+
305+
expect(changes.length).toEqual(1);
306+
expect(change.criticality.level).toEqual(CriticalityLevel.NonBreaking);
307+
expect(change.message).toEqual(
308+
`Enum value 'enumA.A' deprecation reason changed from 'It\\'s old' to 'It\\'s new'`,
309+
);
310+
});
311+
312+
test('deprecation reason added with escaped single quotes', async () => {
313+
const a = buildSchema(/* GraphQL */ `
314+
type Query {
315+
fieldA: String
316+
}
317+
318+
enum enumA {
319+
A
320+
B
321+
}
322+
`);
323+
324+
const b = buildSchema(/* GraphQL */ `
325+
type Query {
326+
fieldA: String
327+
}
328+
329+
enum enumA {
330+
A @deprecated(reason: "Don't use this")
331+
B
332+
}
333+
`);
334+
335+
const changes = await diff(a, b);
336+
const change = findFirstChangeByPath(changes, 'enumA.A');
337+
338+
expect(changes.length).toEqual(2);
339+
expect(change.criticality.level).toEqual(CriticalityLevel.NonBreaking);
340+
expect(change.message).toEqual(
341+
`Enum value 'enumA.A' was deprecated with reason 'Don\\'t use this'`,
342+
);
343+
});
344+
345+
test('deprecation reason without single quotes is unchanged', async () => {
346+
const a = buildSchema(/* GraphQL */ `
347+
type Query {
348+
fieldA: String
349+
}
350+
351+
enum enumA {
352+
A @deprecated(reason: "Old Reason")
353+
B
354+
}
355+
`);
356+
357+
const b = buildSchema(/* GraphQL */ `
358+
type Query {
359+
fieldA: String
360+
}
361+
362+
enum enumA {
363+
A @deprecated(reason: "New Reason")
364+
B
365+
}
366+
`);
367+
368+
const changes = await diff(a, b);
369+
const change = findFirstChangeByPath(changes, 'enumA.A');
370+
371+
expect(changes.length).toEqual(1);
372+
expect(change.criticality.level).toEqual(CriticalityLevel.NonBreaking);
373+
expect(change.message).toEqual(
374+
`Enum value 'enumA.A' deprecation reason changed from 'Old Reason' to 'New Reason'`,
375+
);
376+
});
377+
});
378+
});

packages/core/__tests__/utils/string.test.ts

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { safeString } from '../../src/utils/string.js';
1+
import { fmt, safeString } from '../../src/utils/string.js';
22

33
test('scalars', () => {
44
expect(safeString(0)).toBe('0');
@@ -33,3 +33,18 @@ test('array', () => {
3333
'[ { foo: 42 } ]',
3434
);
3535
});
36+
37+
describe('fmt', () => {
38+
test('escapes single quotes in strings', () => {
39+
expect(fmt("It's a test")).toBe("It\\'s a test");
40+
expect(fmt("Don't do this")).toBe("Don\\'t do this");
41+
expect(fmt("'quoted'")).toBe("\\'quoted\\'");
42+
});
43+
44+
test('handles strings without single quotes', () => {
45+
expect(fmt('test')).toBe('test');
46+
expect(fmt('Old Reason')).toBe('Old Reason');
47+
expect(fmt('enumA.B')).toBe('enumA.B');
48+
expect(fmt('')).toBe('');
49+
});
50+
});

packages/core/src/diff/changes/enum.ts

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -11,11 +11,10 @@ import {
1111
EnumValueDescriptionChangedChange,
1212
EnumValueRemovedChange,
1313
} from './change.js';
14+
import { fmt } from '../../utils/string.js';
1415

1516
function buildEnumValueRemovedMessage(args: EnumValueRemovedChange['meta']) {
16-
return `Enum value '${args.removedEnumValueName}' ${
17-
args.isEnumValueDeprecated ? '(deprecated) ' : ''
18-
}was removed from enum '${args.enumName}'`;
17+
return `Enum value '${args.removedEnumValueName}' ${args.isEnumValueDeprecated ? '(deprecated) ' : ''}was removed from enum '${args.enumName}'`;
1918
}
2019

2120
const enumValueRemovedCriticalityBreakingReason = `Removing an enum value will cause existing queries that use this enum value to error.`;
@@ -80,13 +79,11 @@ export function enumValueAdded(
8079
}
8180

8281
function buildEnumValueDescriptionChangedMessage(args: EnumValueDescriptionChangedChange['meta']) {
82+
const oldDesc = fmt(args.oldEnumValueDescription ?? 'undefined');
83+
const newDesc = fmt(args.newEnumValueDescription ?? 'undefined');
8384
return args.oldEnumValueDescription === null
84-
? `Description '${args.newEnumValueDescription ?? 'undefined'}' was added to enum value '${
85-
args.enumName
86-
}.${args.enumValueName}'`
87-
: `Description for enum value '${args.enumName}.${args.enumValueName}' changed from '${
88-
args.oldEnumValueDescription ?? 'undefined'
89-
}' to '${args.newEnumValueDescription ?? 'undefined'}'`;
85+
? `Description '${newDesc}' was added to enum value '${args.enumName}.${args.enumValueName}'`
86+
: `Description for enum value '${args.enumName}.${args.enumValueName}' changed from '${oldDesc}' to '${newDesc}'`;
9087
}
9188

9289
export function enumValueDescriptionChangedFromMeta(
@@ -122,7 +119,9 @@ export function enumValueDescriptionChanged(
122119
function buildEnumValueDeprecationChangedMessage(
123120
args: EnumValueDeprecationReasonChangedChange['meta'],
124121
) {
125-
return `Enum value '${args.enumName}.${args.enumValueName}' deprecation reason changed from '${args.oldEnumValueDeprecationReason}' to '${args.newEnumValueDeprecationReason}'`;
122+
const oldReason = fmt(args.oldEnumValueDeprecationReason);
123+
const newReason = fmt(args.newEnumValueDeprecationReason);
124+
return `Enum value '${args.enumName}.${args.enumValueName}' deprecation reason changed from '${oldReason}' to '${newReason}'`;
126125
}
127126

128127
export function enumValueDeprecationReasonChangedFromMeta(
@@ -158,7 +157,8 @@ export function enumValueDeprecationReasonChanged(
158157
function buildEnumValueDeprecationReasonAddedMessage(
159158
args: EnumValueDeprecationReasonAddedChange['meta'],
160159
) {
161-
return `Enum value '${args.enumName}.${args.enumValueName}' was deprecated with reason '${args.addedValueDeprecationReason}'`;
160+
const reason = fmt(args.addedValueDeprecationReason);
161+
return `Enum value '${args.enumName}.${args.enumValueName}' was deprecated with reason '${reason}'`;
162162
}
163163

164164
export function enumValueDeprecationReasonAddedFromMeta(

packages/core/src/diff/index.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ export function diff(
1616
rules: Rule[] = [],
1717
config?: rules.ConsiderUsageConfig,
1818
): Promise<Change[]> {
19-
const changes = diffSchema(oldSchema, newSchema);
19+
const changes = diffSchema(oldSchema, newSchema, config);
2020

2121
return rules.reduce(async (prev, rule) => {
2222
const prevChanges = await prev;

packages/core/src/diff/schema.ts

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -35,10 +35,15 @@ import { changesInInterface } from './interface.js';
3535
import { changesInObject } from './object.js';
3636
import { changesInScalar } from './scalar.js';
3737
import { changesInUnion } from './union.js';
38+
import { ConsiderUsageConfig } from './rules/consider-usage.js';
3839

3940
export type AddChange = (change: Change) => void;
4041

41-
export function diffSchema(oldSchema: GraphQLSchema, newSchema: GraphQLSchema): Change[] {
42+
export function diffSchema(
43+
oldSchema: GraphQLSchema,
44+
newSchema: GraphQLSchema,
45+
_config?: ConsiderUsageConfig,
46+
): Change[] {
4247
const changes: Change[] = [];
4348

4449
function addChange(change: Change) {
@@ -123,7 +128,11 @@ function changesInSchema(oldSchema: GraphQLSchema, newSchema: GraphQLSchema, add
123128
}
124129
}
125130

126-
function changesInType(oldType: GraphQLNamedType, newType: GraphQLNamedType, addChange: AddChange) {
131+
function changesInType(
132+
oldType: GraphQLNamedType,
133+
newType: GraphQLNamedType,
134+
addChange: AddChange,
135+
) {
127136
if (isEnumType(oldType) && isEnumType(newType)) {
128137
changesInEnum(oldType, newType, addChange);
129138
} else if (isUnionType(oldType) && isUnionType(newType)) {

packages/core/src/utils/string.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -80,3 +80,7 @@ export function safeString(obj: unknown) {
8080
.replace(/\[Object: null prototype\] /g, '')
8181
.replace(/(^')|('$)/g, '');
8282
}
83+
84+
export function fmt(reason: string): string {
85+
return reason.replace(/'/g, "\\'");
86+
}

0 commit comments

Comments
 (0)