From 6da164bed51ac8cee44b2550a01cc1e92e0b98a2 Mon Sep 17 00:00:00 2001 From: Ivan Goncharov Date: Fri, 12 Mar 2021 19:40:41 +0200 Subject: [PATCH] visitor: remove 4th form of visitor Visitor in 4th form can always be written as 2nd form. This is PR part of general effort to simplify `visit` typings before TS migration. --- src/language/__tests__/visitor-test.js | 28 +- src/language/printer.js | 478 ++++++++++++++----------- src/language/visitor.d.ts | 24 +- src/language/visitor.js | 33 +- 4 files changed, 270 insertions(+), 293 deletions(-) diff --git a/src/language/__tests__/visitor-test.js b/src/language/__tests__/visitor-test.js index 15a12a6253..bfe01c7719 100644 --- a/src/language/__tests__/visitor-test.js +++ b/src/language/__tests__/visitor-test.js @@ -56,6 +56,11 @@ function getValue(node: ASTNode) { } describe('Visitor', () => { + it('handles empty visitor', () => { + const ast = parse('{ a }', { noLocation: true }); + expect(() => visit(ast, {})).to.not.throw(); + }); + it('validates path argument', () => { const visited = []; @@ -114,29 +119,6 @@ describe('Visitor', () => { }); }); - it('allows visiting only specified nodes', () => { - const ast = parse('{ a }', { noLocation: true }); - const visited = []; - - visit(ast, { - enter: { - Field(node) { - visited.push(['enter', node.kind]); - }, - }, - leave: { - Field(node) { - visited.push(['leave', node.kind]); - }, - }, - }); - - expect(visited).to.deep.equal([ - ['enter', 'Field'], - ['leave', 'Field'], - ]); - }); - it('allows editing a node both on enter and on leave', () => { const ast = parse('{ a, b, c { a, b, c } }', { noLocation: true }); diff --git a/src/language/printer.js b/src/language/printer.js index 30247dedab..551befb901 100644 --- a/src/language/printer.js +++ b/src/language/printer.js @@ -8,256 +8,296 @@ import { printBlockString } from './blockString'; * formatting rules. */ export function print(ast: ASTNode): string { - return visit(ast, { leave: printDocASTReducer }); + return visit(ast, printDocASTReducer); } const MAX_LINE_LENGTH = 80; // TODO: provide better type coverage in future const printDocASTReducer: any = { - Name: (node) => node.value, - Variable: (node) => '$' + node.name, + Name: { leave: (node) => node.value }, + Variable: { leave: (node) => '$' + node.name }, // Document - Document: (node) => join(node.definitions, '\n\n') + '\n', - - OperationDefinition(node) { - const op = node.operation; - const name = node.name; - const varDefs = wrap('(', join(node.variableDefinitions, ', '), ')'); - const directives = join(node.directives, ' '); - const selectionSet = node.selectionSet; - // Anonymous queries with no directives or variable definitions can use - // the query short form. - return !name && !directives && !varDefs && op === 'query' - ? selectionSet - : join([op, join([name, varDefs]), directives, selectionSet], ' '); + Document: { + leave: (node) => join(node.definitions, '\n\n') + '\n', }, - VariableDefinition: ({ variable, type, defaultValue, directives }) => - variable + - ': ' + - type + - wrap(' = ', defaultValue) + - wrap(' ', join(directives, ' ')), - SelectionSet: ({ selections }) => block(selections), + OperationDefinition: { + leave(node) { + const op = node.operation; + const name = node.name; + const varDefs = wrap('(', join(node.variableDefinitions, ', '), ')'); + const directives = join(node.directives, ' '); + const selectionSet = node.selectionSet; + // Anonymous queries with no directives or variable definitions can use + // the query short form. + return !name && !directives && !varDefs && op === 'query' + ? selectionSet + : join([op, join([name, varDefs]), directives, selectionSet], ' '); + }, + }, + + VariableDefinition: { + leave: ({ variable, type, defaultValue, directives }) => + variable + + ': ' + + type + + wrap(' = ', defaultValue) + + wrap(' ', join(directives, ' ')), + }, + SelectionSet: { leave: ({ selections }) => block(selections) }, - Field: ({ alias, name, arguments: args, directives, selectionSet }) => { - const prefix = wrap('', alias, ': ') + name; - let argsLine = prefix + wrap('(', join(args, ', '), ')'); + Field: { + leave({ alias, name, arguments: args, directives, selectionSet }) { + const prefix = wrap('', alias, ': ') + name; + let argsLine = prefix + wrap('(', join(args, ', '), ')'); - if (argsLine.length > MAX_LINE_LENGTH) { - argsLine = prefix + wrap('(\n', indent(join(args, '\n')), '\n)'); - } + if (argsLine.length > MAX_LINE_LENGTH) { + argsLine = prefix + wrap('(\n', indent(join(args, '\n')), '\n)'); + } - return join([argsLine, join(directives, ' '), selectionSet], ' '); + return join([argsLine, join(directives, ' '), selectionSet], ' '); + }, }, - Argument: ({ name, value }) => name + ': ' + value, + Argument: { leave: ({ name, value }) => name + ': ' + value }, // Fragments - FragmentSpread: ({ name, directives }) => - '...' + name + wrap(' ', join(directives, ' ')), - - InlineFragment: ({ typeCondition, directives, selectionSet }) => - join( - ['...', wrap('on ', typeCondition), join(directives, ' '), selectionSet], - ' ', - ), - - FragmentDefinition: ({ - name, - typeCondition, - variableDefinitions, - directives, - selectionSet, - }) => - // Note: fragment variable definitions are experimental and may be changed - // or removed in the future. - `fragment ${name}${wrap('(', join(variableDefinitions, ', '), ')')} ` + - `on ${typeCondition} ${wrap('', join(directives, ' '), ' ')}` + - selectionSet, + FragmentSpread: { + leave: ({ name, directives }) => + '...' + name + wrap(' ', join(directives, ' ')), + }, + + InlineFragment: { + leave: ({ typeCondition, directives, selectionSet }) => + join( + [ + '...', + wrap('on ', typeCondition), + join(directives, ' '), + selectionSet, + ], + ' ', + ), + }, + + FragmentDefinition: { + leave: ({ + name, + typeCondition, + variableDefinitions, + directives, + selectionSet, + }) => + // Note: fragment variable definitions are experimental and may be changed + // or removed in the future. + `fragment ${name}${wrap('(', join(variableDefinitions, ', '), ')')} ` + + `on ${typeCondition} ${wrap('', join(directives, ' '), ' ')}` + + selectionSet, + }, // Value - IntValue: ({ value }) => value, - FloatValue: ({ value }) => value, - StringValue: ({ value, block: isBlockString }) => - isBlockString ? printBlockString(value) : JSON.stringify(value), - BooleanValue: ({ value }) => (value ? 'true' : 'false'), - NullValue: () => 'null', - EnumValue: ({ value }) => value, - ListValue: ({ values }) => '[' + join(values, ', ') + ']', - ObjectValue: ({ fields }) => '{' + join(fields, ', ') + '}', - ObjectField: ({ name, value }) => name + ': ' + value, + IntValue: { leave: ({ value }) => value }, + FloatValue: { leave: ({ value }) => value }, + StringValue: { + leave: ({ value, block: isBlockString }) => + isBlockString ? printBlockString(value) : JSON.stringify(value), + }, + BooleanValue: { leave: ({ value }) => (value ? 'true' : 'false') }, + NullValue: { leave: () => 'null' }, + EnumValue: { leave: ({ value }) => value }, + ListValue: { leave: ({ values }) => '[' + join(values, ', ') + ']' }, + ObjectValue: { leave: ({ fields }) => '{' + join(fields, ', ') + '}' }, + ObjectField: { leave: ({ name, value }) => name + ': ' + value }, // Directive - Directive: ({ name, arguments: args }) => - '@' + name + wrap('(', join(args, ', '), ')'), + Directive: { + leave: ({ name, arguments: args }) => + '@' + name + wrap('(', join(args, ', '), ')'), + }, // Type - NamedType: ({ name }) => name, - ListType: ({ type }) => '[' + type + ']', - NonNullType: ({ type }) => type + '!', + NamedType: { leave: ({ name }) => name }, + ListType: { leave: ({ type }) => '[' + type + ']' }, + NonNullType: { leave: ({ type }) => type + '!' }, // Type System Definitions - SchemaDefinition: ({ description, directives, operationTypes }) => - wrap('', description, '\n') + - join(['schema', join(directives, ' '), block(operationTypes)], ' '), - - OperationTypeDefinition: ({ operation, type }) => operation + ': ' + type, - - ScalarTypeDefinition: ({ description, name, directives }) => - wrap('', description, '\n') + - join(['scalar', name, join(directives, ' ')], ' '), - - ObjectTypeDefinition: ({ - description, - name, - interfaces, - directives, - fields, - }) => - wrap('', description, '\n') + - join( - [ - 'type', - name, - wrap('implements ', join(interfaces, ' & ')), - join(directives, ' '), - block(fields), - ], - ' ', - ), - - FieldDefinition: ({ description, name, arguments: args, type, directives }) => - wrap('', description, '\n') + - name + - (hasMultilineItems(args) - ? wrap('(\n', indent(join(args, '\n')), '\n)') - : wrap('(', join(args, ', '), ')')) + - ': ' + - type + - wrap(' ', join(directives, ' ')), - - InputValueDefinition: ({ - description, - name, - type, - defaultValue, - directives, - }) => - wrap('', description, '\n') + - join( - [name + ': ' + type, wrap('= ', defaultValue), join(directives, ' ')], - ' ', - ), - - InterfaceTypeDefinition: ({ - description, - name, - interfaces, - directives, - fields, - }) => - wrap('', description, '\n') + - join( - [ - 'interface', - name, - wrap('implements ', join(interfaces, ' & ')), - join(directives, ' '), - block(fields), - ], - ' ', - ), - - UnionTypeDefinition: ({ description, name, directives, types }) => - wrap('', description, '\n') + - join( - ['union', name, join(directives, ' '), wrap('= ', join(types, ' | '))], - ' ', - ), - - EnumTypeDefinition: ({ description, name, directives, values }) => - wrap('', description, '\n') + - join(['enum', name, join(directives, ' '), block(values)], ' '), - - EnumValueDefinition: ({ description, name, directives }) => - wrap('', description, '\n') + join([name, join(directives, ' ')], ' '), - - InputObjectTypeDefinition: ({ description, name, directives, fields }) => - wrap('', description, '\n') + - join(['input', name, join(directives, ' '), block(fields)], ' '), - - DirectiveDefinition: ({ - description, - name, - arguments: args, - repeatable, - locations, - }) => - wrap('', description, '\n') + - 'directive @' + - name + - (hasMultilineItems(args) - ? wrap('(\n', indent(join(args, '\n')), '\n)') - : wrap('(', join(args, ', '), ')')) + - (repeatable ? ' repeatable' : '') + - ' on ' + - join(locations, ' | '), - - SchemaExtension: ({ directives, operationTypes }) => - join(['extend schema', join(directives, ' '), block(operationTypes)], ' '), - - ScalarTypeExtension: ({ name, directives }) => - join(['extend scalar', name, join(directives, ' ')], ' '), - - ObjectTypeExtension: ({ name, interfaces, directives, fields }) => - join( - [ - 'extend type', - name, - wrap('implements ', join(interfaces, ' & ')), - join(directives, ' '), - block(fields), - ], - ' ', - ), - - InterfaceTypeExtension: ({ name, interfaces, directives, fields }) => - join( - [ - 'extend interface', - name, - wrap('implements ', join(interfaces, ' & ')), - join(directives, ' '), - block(fields), - ], - ' ', - ), - - UnionTypeExtension: ({ name, directives, types }) => - join( - [ - 'extend union', - name, - join(directives, ' '), - wrap('= ', join(types, ' | ')), - ], - ' ', - ), - - EnumTypeExtension: ({ name, directives, values }) => - join(['extend enum', name, join(directives, ' '), block(values)], ' '), - - InputObjectTypeExtension: ({ name, directives, fields }) => - join(['extend input', name, join(directives, ' '), block(fields)], ' '), + SchemaDefinition: { + leave: ({ description, directives, operationTypes }) => + wrap('', description, '\n') + + join(['schema', join(directives, ' '), block(operationTypes)], ' '), + }, + + OperationTypeDefinition: { + leave: ({ operation, type }) => operation + ': ' + type, + }, + + ScalarTypeDefinition: { + leave: ({ description, name, directives }) => + wrap('', description, '\n') + + join(['scalar', name, join(directives, ' ')], ' '), + }, + + ObjectTypeDefinition: { + leave: ({ description, name, interfaces, directives, fields }) => + wrap('', description, '\n') + + join( + [ + 'type', + name, + wrap('implements ', join(interfaces, ' & ')), + join(directives, ' '), + block(fields), + ], + ' ', + ), + }, + + FieldDefinition: { + leave: ({ description, name, arguments: args, type, directives }) => + wrap('', description, '\n') + + name + + (hasMultilineItems(args) + ? wrap('(\n', indent(join(args, '\n')), '\n)') + : wrap('(', join(args, ', '), ')')) + + ': ' + + type + + wrap(' ', join(directives, ' ')), + }, + + InputValueDefinition: { + leave: ({ description, name, type, defaultValue, directives }) => + wrap('', description, '\n') + + join( + [name + ': ' + type, wrap('= ', defaultValue), join(directives, ' ')], + ' ', + ), + }, + + InterfaceTypeDefinition: { + leave: ({ description, name, interfaces, directives, fields }) => + wrap('', description, '\n') + + join( + [ + 'interface', + name, + wrap('implements ', join(interfaces, ' & ')), + join(directives, ' '), + block(fields), + ], + ' ', + ), + }, + + UnionTypeDefinition: { + leave: ({ description, name, directives, types }) => + wrap('', description, '\n') + + join( + ['union', name, join(directives, ' '), wrap('= ', join(types, ' | '))], + ' ', + ), + }, + + EnumTypeDefinition: { + leave: ({ description, name, directives, values }) => + wrap('', description, '\n') + + join(['enum', name, join(directives, ' '), block(values)], ' '), + }, + + EnumValueDefinition: { + leave: ({ description, name, directives }) => + wrap('', description, '\n') + join([name, join(directives, ' ')], ' '), + }, + + InputObjectTypeDefinition: { + leave: ({ description, name, directives, fields }) => + wrap('', description, '\n') + + join(['input', name, join(directives, ' '), block(fields)], ' '), + }, + + DirectiveDefinition: { + leave: ({ description, name, arguments: args, repeatable, locations }) => + wrap('', description, '\n') + + 'directive @' + + name + + (hasMultilineItems(args) + ? wrap('(\n', indent(join(args, '\n')), '\n)') + : wrap('(', join(args, ', '), ')')) + + (repeatable ? ' repeatable' : '') + + ' on ' + + join(locations, ' | '), + }, + + SchemaExtension: { + leave: ({ directives, operationTypes }) => + join( + ['extend schema', join(directives, ' '), block(operationTypes)], + ' ', + ), + }, + + ScalarTypeExtension: { + leave: ({ name, directives }) => + join(['extend scalar', name, join(directives, ' ')], ' '), + }, + + ObjectTypeExtension: { + leave: ({ name, interfaces, directives, fields }) => + join( + [ + 'extend type', + name, + wrap('implements ', join(interfaces, ' & ')), + join(directives, ' '), + block(fields), + ], + ' ', + ), + }, + + InterfaceTypeExtension: { + leave: ({ name, interfaces, directives, fields }) => + join( + [ + 'extend interface', + name, + wrap('implements ', join(interfaces, ' & ')), + join(directives, ' '), + block(fields), + ], + ' ', + ), + }, + + UnionTypeExtension: { + leave: ({ name, directives, types }) => + join( + [ + 'extend union', + name, + join(directives, ' '), + wrap('= ', join(types, ' | ')), + ], + ' ', + ), + }, + + EnumTypeExtension: { + leave: ({ name, directives, values }) => + join(['extend enum', name, join(directives, ' '), block(values)], ' '), + }, + + InputObjectTypeExtension: { + leave: ({ name, directives, fields }) => + join(['extend input', name, join(directives, ' '), block(fields)], ' '), + }, }; /** diff --git a/src/language/visitor.d.ts b/src/language/visitor.d.ts index 9ef0a66a4a..6926191279 100644 --- a/src/language/visitor.d.ts +++ b/src/language/visitor.d.ts @@ -6,18 +6,13 @@ import { ASTNode, ASTKindToNode } from './ast'; * A visitor is provided to visit, it contains the collection of * relevant functions to be called during the visitor's traversal. */ -export type ASTVisitor = EnterLeaveVisitor | ShapeMapVisitor; +export type ASTVisitor = EnterLeave> | ShapeMapVisitor; interface EnterLeave { readonly enter?: T; readonly leave?: T; } -type EnterLeaveVisitor = EnterLeave< - | ASTVisitFn - | { [K in keyof ASTKindToNode]?: ASTVisitFn } ->; - type ShapeMapVisitor = { [K in keyof ASTKindToNode]?: | ASTVisitFn @@ -82,7 +77,7 @@ export const BREAK: any; * * Alternatively to providing enter() and leave() functions, a visitor can * instead provide functions named the same as the kinds of AST nodes, or - * enter/leave visitors at a named key, leading to four permutations of the + * enter/leave visitors at a named key, leading to three permutations of the * visitor API: * * 1) Named visitors triggered when entering a node of a specific kind. @@ -117,21 +112,6 @@ export const BREAK: any; * // leave any node * } * }) - * - * 4) Parallel visitors for entering and leaving nodes of a specific kind. - * - * visit(ast, { - * enter: { - * Kind(node) { - * // enter the "Kind" node - * } - * }, - * leave: { - * Kind(node) { - * // leave the "Kind" node - * } - * } - * }) */ export function visit(root: ASTNode, visitor: ASTVisitor): any; diff --git a/src/language/visitor.js b/src/language/visitor.js index dd3359ace5..b8eac40613 100644 --- a/src/language/visitor.js +++ b/src/language/visitor.js @@ -8,10 +8,7 @@ import { isNode } from './ast'; * relevant functions to be called during the visitor's traversal. */ export type ASTVisitor = - | EnterLeave< - | ASTVisitFn - | ShapeMap(Node) => ASTVisitFn>, - > + | EnterLeave> | ShapeMap< ASTKindToNode, (Node) => ASTVisitFn | EnterLeave>, @@ -161,7 +158,7 @@ export const BREAK: { ... } = Object.freeze({}); * * Alternatively to providing enter() and leave() functions, a visitor can * instead provide functions named the same as the kinds of AST nodes, or - * enter/leave visitors at a named key, leading to four permutations of the + * enter/leave visitors at a named key, leading to three permutations of the * visitor API: * * 1) Named visitors triggered when entering a node of a specific kind. @@ -196,21 +193,6 @@ export const BREAK: { ... } = Object.freeze({}); * // leave any node * } * }) - * - * 4) Parallel visitors for entering and leaving nodes of a specific kind. - * - * visit(ast, { - * enter: { - * Kind(node) { - * // enter the "Kind" node - * } - * }, - * leave: { - * Kind(node) { - * // leave the "Kind" node - * } - * } - * }) */ export function visit(root: ASTNode, visitor: ASTVisitor): any { /* eslint-disable no-undef-init */ @@ -410,15 +392,8 @@ export function getVisitFn( // $FlowFixMe[prop-missing] const specificVisitor = isLeaving ? visitor.leave : visitor.enter; if (specificVisitor) { - if (typeof specificVisitor === 'function') { - // { enter() {}, leave() {} } - return specificVisitor; - } - const specificKindVisitor = specificVisitor[kind]; - if (typeof specificKindVisitor === 'function') { - // { enter: { Kind() {} }, leave: { Kind() {} } } - return specificKindVisitor; - } + // { enter() {}, leave() {} } + return specificVisitor; } } }