From 2611aa41bc8dcb6b1ae7e10853788a1f788e4255 Mon Sep 17 00:00:00 2001 From: Jovi De Croock Date: Sat, 15 Feb 2025 10:21:15 +0100 Subject: [PATCH] Add star syntax proposal --- src/language/__tests__/parser-test.ts | 13 +++--- src/language/__tests__/schema-printer-test.ts | 31 +++---------- src/language/lexer.ts | 13 ++---- src/language/parser.ts | 43 +++--------------- src/language/printer.ts | 22 ++-------- src/language/tokenKind.ts | 2 +- src/type/__tests__/introspection-test.ts | 10 ++--- src/type/definition.ts | 2 +- src/utilities/__tests__/TypeInfo-test.ts | 13 +++--- .../__tests__/buildClientSchema-test.ts | 10 ++--- src/utilities/__tests__/extendSchema-test.ts | 14 +++--- .../__tests__/findBreakingChanges-test.ts | 32 +++++--------- .../__tests__/lexicographicSortSchema-test.ts | 32 ++++++-------- src/utilities/printSchema.ts | 44 +++++-------------- .../OverlappingFieldsCanBeMergedRule-test.ts | 28 ++++++------ 15 files changed, 92 insertions(+), 217 deletions(-) diff --git a/src/language/__tests__/parser-test.ts b/src/language/__tests__/parser-test.ts index f3577ef64d..11200d5943 100644 --- a/src/language/__tests__/parser-test.ts +++ b/src/language/__tests__/parser-test.ts @@ -661,20 +661,19 @@ describe('Parser', () => { describe('parseDocumentDirective', () => { it("doesn't throw on document-level directive", () => { parse(dedent` - @SemanticNullability type Query { - hello: String - world: String? + hello: String* + world: String foo: String! } `); }); it('parses semantic-non-null types', () => { - const result = parseType('MyType', { allowSemanticNullability: true }); + const result = parseType('MyType*'); expectJSON(result).toDeepEqual({ kind: Kind.SEMANTIC_NON_NULL_TYPE, - loc: { start: 0, end: 6 }, + loc: { start: 0, end: 7 }, type: { kind: Kind.NAMED_TYPE, loc: { start: 0, end: 6 }, @@ -688,7 +687,7 @@ describe('Parser', () => { }); it('parses nullable types', () => { - const result = parseType('MyType?', { allowSemanticNullability: true }); + const result = parseType('MyType'); expectJSON(result).toDeepEqual({ kind: Kind.NAMED_TYPE, loc: { start: 0, end: 6 }, @@ -701,7 +700,7 @@ describe('Parser', () => { }); it('parses non-nullable types', () => { - const result = parseType('MyType!', { allowSemanticNullability: true }); + const result = parseType('MyType!'); expectJSON(result).toDeepEqual({ kind: Kind.NON_NULL_TYPE, loc: { start: 0, end: 7 }, diff --git a/src/language/__tests__/schema-printer-test.ts b/src/language/__tests__/schema-printer-test.ts index a2e3fa070d..052480b4fd 100644 --- a/src/language/__tests__/schema-printer-test.ts +++ b/src/language/__tests__/schema-printer-test.ts @@ -182,39 +182,18 @@ describe('Printer: SDL document', () => { }); it('prints NamedType', () => { - expect( - print(parseType('MyType', { allowSemanticNullability: false }), { - useSemanticNullability: false, - }), - ).to.equal(dedent`MyType`); + expect(print(parseType('MyType'))).to.equal(dedent`MyType`); }); - it('prints SemanticNullableType', () => { - expect( - print(parseType('MyType?', { allowSemanticNullability: true }), { - useSemanticNullability: true, - }), - ).to.equal(dedent`MyType?`); + it('prints nullable types', () => { + expect(print(parseType('MyType'))).to.equal(dedent`MyType`); }); it('prints SemanticNonNullType', () => { - expect( - print(parseType('MyType', { allowSemanticNullability: true }), { - useSemanticNullability: true, - }), - ).to.equal(dedent`MyType`); + expect(print(parseType('MyType*'))).to.equal(dedent`MyType*`); }); it('prints NonNullType', () => { - expect( - print(parseType('MyType!', { allowSemanticNullability: true }), { - useSemanticNullability: true, - }), - ).to.equal(dedent`MyType!`); - expect( - print(parseType('MyType!', { allowSemanticNullability: false }), { - useSemanticNullability: true, - }), - ).to.equal(dedent`MyType!`); + expect(print(parseType('MyType!'))).to.equal(dedent`MyType!`); }); }); diff --git a/src/language/lexer.ts b/src/language/lexer.ts index 86ff5edb6f..239e14d2f0 100644 --- a/src/language/lexer.ts +++ b/src/language/lexer.ts @@ -91,7 +91,7 @@ export class Lexer { export function isPunctuatorTokenKind(kind: TokenKind): boolean { return ( kind === TokenKind.BANG || - kind === TokenKind.QUESTION_MARK || + kind === TokenKind.STAR || kind === TokenKind.DOLLAR || kind === TokenKind.AMP || kind === TokenKind.PAREN_L || @@ -247,16 +247,11 @@ function readNextToken(lexer: Lexer, start: number): Token { // - FloatValue // - StringValue // - // Punctuator :: one of ! ? $ & ( ) ... : = @ [ ] { | } + // Punctuator :: one of ! * $ & ( ) ... : = @ [ ] { | } case 0x0021: // ! return createToken(lexer, TokenKind.BANG, position, position + 1); - case 0x003f: // ? - return createToken( - lexer, - TokenKind.QUESTION_MARK, - position, - position + 1, - ); + case 0x002a: // * + return createToken(lexer, TokenKind.STAR, position, position + 1); case 0x0024: // $ return createToken(lexer, TokenKind.DOLLAR, position, position + 1); case 0x0026: // & diff --git a/src/language/parser.ts b/src/language/parser.ts index c96dd25ca6..b7ca7da72a 100644 --- a/src/language/parser.ts +++ b/src/language/parser.ts @@ -104,18 +104,6 @@ export interface ParseOptions { * ``` */ allowLegacyFragmentVariables?: boolean; - - /** - * When enabled, the parser will understand and parse semantic nullability - * annotations. This means that every type suffixed with `!` will remain - * non-nullable, every type suffixed with `?` will be the classic nullable, and - * types without a suffix will be semantically nullable. Semantic nullability - * will be the new default when this is enabled. A semantically nullable type - * can only be null when there's an error associated with the field. - * - * @experimental - */ - allowSemanticNullability?: boolean; } /** @@ -271,16 +259,6 @@ export class Parser { * - InputObjectTypeDefinition */ parseDefinition(): DefinitionNode { - const directives = this.parseDirectives(false); - // If a document-level SemanticNullability directive exists as - // the first element in a document, then all parsing will - // happen in SemanticNullability mode. - for (const directive of directives) { - if (directive.name.value === 'SemanticNullability') { - this._options.allowSemanticNullability = true; - } - } - if (this.peek(TokenKind.BRACE_L)) { return this.parseOperationDefinition(); } @@ -788,27 +766,16 @@ export class Parser { type = this.parseNamedType(); } - if (this._options.allowSemanticNullability) { - if (this.expectOptionalToken(TokenKind.BANG)) { - return this.node(start, { - kind: Kind.NON_NULL_TYPE, - type, - }); - } else if (this.expectOptionalToken(TokenKind.QUESTION_MARK)) { - return type; - } - - return this.node(start, { - kind: Kind.SEMANTIC_NON_NULL_TYPE, - type, - }); - } - if (this.expectOptionalToken(TokenKind.BANG)) { return this.node(start, { kind: Kind.NON_NULL_TYPE, type, }); + } else if (this.expectOptionalToken(TokenKind.STAR)) { + return this.node(start, { + kind: Kind.SEMANTIC_NON_NULL_TYPE, + type, + }); } return type; diff --git a/src/language/printer.ts b/src/language/printer.ts index 66d591d619..8e177ed3bf 100644 --- a/src/language/printer.ts +++ b/src/language/printer.ts @@ -2,22 +2,14 @@ import type { Maybe } from '../jsutils/Maybe'; import type { ASTNode } from './ast'; import { printBlockString } from './blockString'; -import { Kind } from './kinds'; import { printString } from './printString'; import { visit } from './visitor'; -/** - * Configuration options to control parser behavior - */ -export interface PrintOptions { - useSemanticNullability?: boolean; -} - /** * Converts an AST into a string, using one set of reasonable * formatting rules. */ -export function print(ast: ASTNode, options: PrintOptions = {}): string { +export function print(ast: ASTNode): string { return visit(ast, { Name: { leave: (node) => node.value }, Variable: { leave: (node) => '$' + node.name }, @@ -131,19 +123,11 @@ export function print(ast: ASTNode, options: PrintOptions = {}): string { // Type NamedType: { - leave: ({ name }, _, parent) => - parent && - !Array.isArray(parent) && - ((parent as ASTNode).kind === Kind.SEMANTIC_NON_NULL_TYPE || - (parent as ASTNode).kind === Kind.NON_NULL_TYPE) - ? name - : options?.useSemanticNullability - ? `${name}?` - : name, + leave: ({ name }) => name, }, ListType: { leave: ({ type }) => '[' + type + ']' }, NonNullType: { leave: ({ type }) => type + '!' }, - SemanticNonNullType: { leave: ({ type }) => type }, + SemanticNonNullType: { leave: ({ type }) => type + '*' }, // Type System Definitions diff --git a/src/language/tokenKind.ts b/src/language/tokenKind.ts index 0b651d36b0..bd95d6e28e 100644 --- a/src/language/tokenKind.ts +++ b/src/language/tokenKind.ts @@ -6,7 +6,7 @@ enum TokenKind { SOF = '', EOF = '', BANG = '!', - QUESTION_MARK = '?', + STAR = '*', DOLLAR = '$', AMP = '&', PAREN_L = '(', diff --git a/src/type/__tests__/introspection-test.ts b/src/type/__tests__/introspection-test.ts index 09c12abb06..386f1f8b1a 100644 --- a/src/type/__tests__/introspection-test.ts +++ b/src/type/__tests__/introspection-test.ts @@ -1798,11 +1798,10 @@ describe('Introspection', () => { describe('semantic nullability', () => { it('casts semantic-non-null types to nullable types in traditional mode', () => { const schema = buildSchema(` - @SemanticNullability type Query { someField: String! - someField2: String - someField3: String? + someField2: String* + someField3: String } `); @@ -1847,11 +1846,10 @@ describe('Introspection', () => { it('returns semantic-non-null types in full mode', () => { const schema = buildSchema(` - @SemanticNullability type Query { someField: String! - someField2: String - someField3: String? + someField2: String* + someField3: String } `); diff --git a/src/type/definition.ts b/src/type/definition.ts index b0c7d0c52f..bfccf8c9e8 100644 --- a/src/type/definition.ts +++ b/src/type/definition.ts @@ -502,7 +502,7 @@ export class GraphQLSemanticNonNull { } toString(): string { - return String(this.ofType); + return String(this.ofType) + '*'; } toJSON(): string { diff --git a/src/utilities/__tests__/TypeInfo-test.ts b/src/utilities/__tests__/TypeInfo-test.ts index 48f50d21b7..fe6d323ed8 100644 --- a/src/utilities/__tests__/TypeInfo-test.ts +++ b/src/utilities/__tests__/TypeInfo-test.ts @@ -460,11 +460,10 @@ describe('visitWithTypeInfo', () => { it('supports traversals of semantic non-null types', () => { const schema = buildSchema(` - @SemanticNullability type Query { id: String! - name: String - something: String? + name: String* + something: String } `); @@ -506,10 +505,10 @@ describe('visitWithTypeInfo', () => { ['enter', 'Name', 'id', 'String!'], ['leave', 'Name', 'id', 'String!'], ['leave', 'Field', null, 'String!'], - ['enter', 'Field', null, 'String'], - ['enter', 'Name', 'name', 'String'], - ['leave', 'Name', 'name', 'String'], - ['leave', 'Field', null, 'String'], + ['enter', 'Field', null, 'String*'], + ['enter', 'Name', 'name', 'String*'], + ['leave', 'Name', 'name', 'String*'], + ['leave', 'Field', null, 'String*'], ['enter', 'Field', null, 'String'], ['enter', 'Name', 'something', 'String'], ['leave', 'Name', 'something', 'String'], diff --git a/src/utilities/__tests__/buildClientSchema-test.ts b/src/utilities/__tests__/buildClientSchema-test.ts index 59b78024e6..6b3e5a8f45 100644 --- a/src/utilities/__tests__/buildClientSchema-test.ts +++ b/src/utilities/__tests__/buildClientSchema-test.ts @@ -988,11 +988,9 @@ describe('Type System: build schema from introspection', () => { describe('SemanticNullability', () => { it('should build a client schema with semantic-non-null types', () => { const sdl = dedent` - @SemanticNullability - type Query { - foo: String - bar: String? + foo: String* + bar: String } `; const schema = buildSchema(sdl, { assumeValid: true }); @@ -1027,10 +1025,8 @@ describe('Type System: build schema from introspection', () => { it('should throw when semantic-non-null types are too deep', () => { const sdl = dedent` - @SemanticNullability - type Query { - bar: [[[[[[String?]]]]]]? + bar: [[[[[[String]*]*]*]*]*] } `; const schema = buildSchema(sdl, { assumeValid: true }); diff --git a/src/utilities/__tests__/extendSchema-test.ts b/src/utilities/__tests__/extendSchema-test.ts index a70ff2fb47..6bc636b6ec 100644 --- a/src/utilities/__tests__/extendSchema-test.ts +++ b/src/utilities/__tests__/extendSchema-test.ts @@ -93,16 +93,14 @@ describe('extendSchema', () => { it('extends objects by adding new fields in semantic nullability mode', () => { const schema = buildSchema(` - @SemanticNullability type Query { - someObject: String + someObject: String* } `); const extensionSDL = dedent` - @SemanticNullability extend type Query { - newSemanticNonNullField: String - newSemanticNullableField: String? + newSemanticNonNullField: String* + newSemanticNullableField: String newNonNullField: String! } `; @@ -111,9 +109,9 @@ describe('extendSchema', () => { expect(validateSchema(extendedSchema)).to.deep.equal([]); expectSchemaChanges(schema, extendedSchema, true).to.equal(dedent` type Query { - someObject: String - newSemanticNonNullField: String - newSemanticNullableField: String? + someObject: String* + newSemanticNonNullField: String* + newSemanticNullableField: String newNonNullField: String! } `); diff --git a/src/utilities/__tests__/findBreakingChanges-test.ts b/src/utilities/__tests__/findBreakingChanges-test.ts index f54b8c08ed..fd9e0305af 100644 --- a/src/utilities/__tests__/findBreakingChanges-test.ts +++ b/src/utilities/__tests__/findBreakingChanges-test.ts @@ -579,22 +579,20 @@ describe('findBreakingChanges', () => { it('should consider semantic non-null output types that change type as breaking', () => { const oldSchema = buildSchema(` - @SemanticNullability type Type1 { - field1: String + field1: String* } `); const newSchema = buildSchema(` - @SemanticNullability type Type1 { - field1: Int + field1: Int* } `); expect(findBreakingChanges(oldSchema, newSchema)).to.deep.equal([ { - description: 'Type1.field1 changed type from String to Int.', + description: 'Type1.field1 changed type from String* to Int*.', type: BreakingChangeType.FIELD_CHANGED_KIND, }, ]); @@ -602,14 +600,12 @@ describe('findBreakingChanges', () => { it('should consider output types that move away from SemanticNonNull to non-null as non-breaking', () => { const oldSchema = buildSchema(` - @SemanticNullability type Type1 { - field1: String + field1: String* } `); const newSchema = buildSchema(` - @SemanticNullability type Type1 { field1: String! } @@ -620,16 +616,14 @@ describe('findBreakingChanges', () => { it('should consider output types that move away from nullable to semantic non-null as non-breaking', () => { const oldSchema = buildSchema(` - @SemanticNullability type Type1 { - field1: String? + field1: String } `); const newSchema = buildSchema(` - @SemanticNullability type Type1 { - field1: String + field1: String* } `); @@ -638,16 +632,14 @@ describe('findBreakingChanges', () => { it('should consider list output types that move away from nullable to semantic non-null as non-breaking', () => { const oldSchema = buildSchema(` - @SemanticNullability type Type1 { - field1: [String?]? + field1: [String] } `); const newSchema = buildSchema(` - @SemanticNullability type Type1 { - field1: [String] + field1: [String*]* } `); @@ -656,22 +648,20 @@ describe('findBreakingChanges', () => { it('should consider output types that move away from SemanticNonNull to null as breaking', () => { const oldSchema = buildSchema(` - @SemanticNullability type Type1 { - field1: String + field1: String* } `); const newSchema = buildSchema(` - @SemanticNullability type Type1 { - field1: String? + field1: String } `); expect(findBreakingChanges(oldSchema, newSchema)).to.deep.equal([ { - description: 'Type1.field1 changed type from String to String.', + description: 'Type1.field1 changed type from String* to String.', type: BreakingChangeType.FIELD_CHANGED_KIND, }, ]); diff --git a/src/utilities/__tests__/lexicographicSortSchema-test.ts b/src/utilities/__tests__/lexicographicSortSchema-test.ts index 2187964740..953a83872d 100644 --- a/src/utilities/__tests__/lexicographicSortSchema-test.ts +++ b/src/utilities/__tests__/lexicographicSortSchema-test.ts @@ -65,54 +65,50 @@ describe('lexicographicSortSchema', () => { it('sort fields w/ semanticNonNull', () => { const sorted = sortSDL(` - @SemanticNullability - input Bar { barB: String! - barA: String - barC: [String] + barA: String* + barC: [String*]* } interface FooInterface { fooB: String! - fooA: String - fooC: [String] + fooA: String* + fooC: [String*]* } type FooType implements FooInterface { - fooC: [String] - fooA: String + fooC: [String*]* + fooA: String* fooB: String! } type Query { - dummy(arg: Bar): FooType? + dummy(arg: Bar): FooType } `); expect(sorted).to.equal(dedent` - @SemanticNullability - input Bar { - barA: String + barA: String* barB: String! - barC: [String] + barC: [String*]* } interface FooInterface { - fooA: String + fooA: String* fooB: String! - fooC: [String] + fooC: [String*]* } type FooType implements FooInterface { - fooA: String + fooA: String* fooB: String! - fooC: [String] + fooC: [String*]* } type Query { - dummy(arg: Bar): FooType? + dummy(arg: Bar): FooType } `); }); diff --git a/src/utilities/printSchema.ts b/src/utilities/printSchema.ts index e44c280e20..edac6262c5 100644 --- a/src/utilities/printSchema.ts +++ b/src/utilities/printSchema.ts @@ -18,11 +18,9 @@ import type { GraphQLUnionType, } from '../type/definition'; import { - GraphQLSemanticNonNull, isEnumType, isInputObjectType, isInterfaceType, - isNullableType, isObjectType, isScalarType, isUnionType, @@ -61,19 +59,11 @@ function printFilteredSchema( ): string { const directives = schema.getDirectives().filter(directiveFilter); const types = Object.values(schema.getTypeMap()).filter(typeFilter); - const hasSemanticNonNull = types.some( - (type) => - (isObjectType(type) || isInterfaceType(type)) && - Object.values(type.getFields()).some( - (field) => field.type instanceof GraphQLSemanticNonNull, - ), - ); return [ - hasSemanticNonNull ? '@SemanticNullability' : '', printSchemaDefinition(schema), ...directives.map((directive) => printDirective(directive)), - ...types.map((type) => printType(type, hasSemanticNonNull)), + ...types.map((type) => printType(type)), ] .filter(Boolean) .join('\n\n'); @@ -138,18 +128,15 @@ function isSchemaOfCommonNames(schema: GraphQLSchema): boolean { return true; } -export function printType( - type: GraphQLNamedType, - hasSemanticNonNull: boolean = false, -): string { +export function printType(type: GraphQLNamedType): string { if (isScalarType(type)) { return printScalar(type); } if (isObjectType(type)) { - return printObject(type, hasSemanticNonNull); + return printObject(type); } if (isInterfaceType(type)) { - return printInterface(type, hasSemanticNonNull); + return printInterface(type); } if (isUnionType(type)) { return printUnion(type); @@ -180,27 +167,21 @@ function printImplementedInterfaces( : ''; } -function printObject( - type: GraphQLObjectType, - hasSemanticNonNull: boolean, -): string { +function printObject(type: GraphQLObjectType): string { return ( printDescription(type) + `type ${type.name}` + printImplementedInterfaces(type) + - printFields(type, hasSemanticNonNull) + printFields(type) ); } -function printInterface( - type: GraphQLInterfaceType, - hasSemanticNonNull: boolean, -): string { +function printInterface(type: GraphQLInterfaceType): string { return ( printDescription(type) + `interface ${type.name}` + printImplementedInterfaces(type) + - printFields(type, hasSemanticNonNull) + printFields(type) ); } @@ -236,10 +217,7 @@ function printInputObject(type: GraphQLInputObjectType): string { ); } -function printFields( - type: GraphQLObjectType | GraphQLInterfaceType, - hasSemanticNonNull: boolean, -): string { +function printFields(type: GraphQLObjectType | GraphQLInterfaceType): string { const fields = Object.values(type.getFields()).map( (f, i) => printDescription(f, ' ', !i) + @@ -247,9 +225,7 @@ function printFields( f.name + printArgs(f.args, ' ') + ': ' + - (hasSemanticNonNull && isNullableType(f.type) - ? `${f.type}?` - : String(f.type)) + + String(f.type) + printDeprecated(f.deprecationReason), ); return printBlock(fields); diff --git a/src/validation/__tests__/OverlappingFieldsCanBeMergedRule-test.ts b/src/validation/__tests__/OverlappingFieldsCanBeMergedRule-test.ts index a9d7ef2d14..73b88f84ee 100644 --- a/src/validation/__tests__/OverlappingFieldsCanBeMergedRule-test.ts +++ b/src/validation/__tests__/OverlappingFieldsCanBeMergedRule-test.ts @@ -1195,27 +1195,26 @@ describe('Validate: Overlapping fields can be merged', () => { describe('semantic non-null', () => { const schema = buildSchema(` - @SemanticNullability type Query { - box: Box + box: Box* } interface Box { - id: String + id: String* } type IntBox implements Box { - id: String - field: Int - field2: Int? - field3: Int + id: String* + field: Int* + field2: Int + field3: Int* } type StringBox implements Box { - id: String - field: String - field2: Int - field3: Int + id: String* + field: String* + field2: Int* + field3: Int* } `); @@ -1273,7 +1272,7 @@ describe('Validate: Overlapping fields can be merged', () => { ).toDeepEqual([ { message: - 'Fields "field" conflict because they return conflicting types "Int" and "String". Use different aliases on the fields to fetch both if this was intentional.', + 'Fields "field" conflict because they return conflicting types "Int*" and "String*". Use different aliases on the fields to fetch both if this was intentional.', locations: [ { line: 5, column: 17 }, { line: 8, column: 17 }, @@ -1300,7 +1299,7 @@ describe('Validate: Overlapping fields can be merged', () => { ).toDeepEqual([ { message: - 'Fields "field2" conflict because they return conflicting types "Int" and "Int". Use different aliases on the fields to fetch both if this was intentional.', + 'Fields "field2" conflict because they return conflicting types "Int*" and "Int". Use different aliases on the fields to fetch both if this was intentional.', locations: [ { line: 5, column: 17 }, { line: 8, column: 17 }, @@ -1326,9 +1325,8 @@ describe('Validate: Overlapping fields can be merged', () => { `, ).toDeepEqual([ { - // TODO: inspect currently returns "Int" for both types message: - 'Fields "field2" conflict because they return conflicting types "Int" and "Int". Use different aliases on the fields to fetch both if this was intentional.', + 'Fields "field2" conflict because they return conflicting types "Int" and "Int*". Use different aliases on the fields to fetch both if this was intentional.', locations: [ { line: 5, column: 17 }, { line: 8, column: 17 },