diff --git a/protographic/src/sdl-validation-visitor.ts b/protographic/src/sdl-validation-visitor.ts index aa8036977e..39b2027181 100644 --- a/protographic/src/sdl-validation-visitor.ts +++ b/protographic/src/sdl-validation-visitor.ts @@ -13,12 +13,8 @@ import { NamedTypeNode, GraphQLID, ConstArgumentNode, - GraphQLObjectType, - GraphQLSchema, - buildSchema, } from 'graphql'; -import { CONNECT_FIELD_RESOLVER, CONTEXT, FIELDS, REQUIRES_DIRECTIVE_NAME } from './string-constants.js'; -import { SelectionSetValidationVisitor } from './selection-set-validation-visitor.js'; +import { CONNECT_FIELD_RESOLVER, CONTEXT } from './string-constants.js'; /** * Type mapping from Kind enum values to their corresponding AST node types @@ -90,7 +86,6 @@ interface MessageContext { */ export class SDLValidationVisitor { private readonly schema: string; - private readonly schemaObject: GraphQLSchema; private readonly validationResult: ValidationResult; private lintingRules: LintingRule[] = []; private visitor: ASTVisitor; @@ -101,7 +96,6 @@ export class SDLValidationVisitor { */ constructor(schema: string) { this.schema = schema; - this.schemaObject = buildSchema(schema, { assumeValid: true, assumeValidSDL: true }); this.validationResult = { errors: [], warnings: [], @@ -149,21 +143,7 @@ export class SDLValidationVisitor { validationFunction: (ctx) => this.validateInvalidResolverContext(ctx), }; - const disallowAbstractTypesForRequiresRule: LintingRule = { - name: 'disallow-abstract-types-for-requires', - description: 'Validates that abstract types are not used in requires directives', - enabled: true, - nodeKind: Kind.FIELD_DEFINITION, - validationFunction: (ctx) => this.validateDisallowAbstractTypesForRequires(ctx), - }; - - this.lintingRules = [ - objectTypeRule, - listTypeRule, - providesRule, - resolverContextRule, - disallowAbstractTypesForRequiresRule, - ]; + this.lintingRules = [objectTypeRule, listTypeRule, providesRule, resolverContextRule]; } /** @@ -312,21 +292,6 @@ export class SDLValidationVisitor { } } - /** - * Validate `@requires` directive usage (currently not supported) - * @param ctx - The VisitContext containing the field definition node to check for `@requires` directive - * @private - */ - private validateRequiresDirective(ctx: VisitContext): void { - const hasRequiresDirective = ctx.node.directives?.some( - (directive) => directive.name.value === REQUIRES_DIRECTIVE_NAME, - ); - - if (hasRequiresDirective) { - this.addWarning('Use of requires is not supported yet', ctx.node.loc); - } - } - /** * Validate `@provides` directive usage. This is not supported in connect subgraphs. * However `@requires` will be supported in the future. @@ -407,51 +372,6 @@ export class SDLValidationVisitor { } } - private validateDisallowAbstractTypesForRequires(ctx: VisitContext): void { - const requiredDirective = ctx.node.directives?.find( - (directive) => directive.name.value === REQUIRES_DIRECTIVE_NAME, - ); - if (!requiredDirective) { - return; - } - - const fieldSelections = requiredDirective.arguments?.find((arg) => arg.name.value === FIELDS)?.value; - if (!fieldSelections || fieldSelections.kind !== Kind.STRING) { - return; - } - - const parentType = ctx.ancestors.at(-1); - if (!parentType) { - return; - } - - if (!this.isASTObjectTypeNode(parentType)) { - return; - } - - let operationDoc; - try { - operationDoc = parse(`{ ${fieldSelections.value} }`); - } catch (e) { - const errorMessage = e instanceof Error ? e.message : String(e); - this.addError(`Invalid @${REQUIRES_DIRECTIVE_NAME} field selection syntax: ${errorMessage}`, ctx.node.loc); - return; - } - - const selectionSetValidationVisitor = new SelectionSetValidationVisitor( - operationDoc, - this.schemaObject.getType(parentType.name.value) as GraphQLObjectType, - ); - selectionSetValidationVisitor.visit(); - for (const error of selectionSetValidationVisitor.getValidationResult().errors) { - this.addError(error, ctx.node.loc); - } - - for (const warning of selectionSetValidationVisitor.getValidationResult().warnings) { - this.addWarning(warning, ctx.node.loc); - } - } - private getResolverContext(node: FieldDefinitionNode): ConstArgumentNode | undefined { return node.directives ?.find((directive) => directive.name.value === CONNECT_FIELD_RESOLVER) diff --git a/protographic/src/selection-set-validation-visitor.ts b/protographic/src/selection-set-validation-visitor.ts deleted file mode 100644 index 7159f9f823..0000000000 --- a/protographic/src/selection-set-validation-visitor.ts +++ /dev/null @@ -1,245 +0,0 @@ -import { - ASTNode, - ASTVisitor, - BREAK, - DocumentNode, - FieldNode, - GraphQLField, - GraphQLNamedType, - GraphQLObjectType, - GraphQLType, - InlineFragmentNode, - isInterfaceType, - isNamedType, - isObjectType, - isUnionType, - Kind, - SelectionSetNode, - visit, -} from 'graphql'; -import { VisitContext } from './types.js'; -import { ValidationResult } from './sdl-validation-visitor.js'; - -/** - * Validates selection sets within @requires directive field sets. - * - * This visitor traverses a parsed field set document and enforces constraints - * specific to @requires directives: - * - Abstract types (interfaces, unions) are not allowed - * - Inline fragments are not allowed - * - * @example - * ```typescript - * const doc = parse('{ address { street city } }'); - * const visitor = new SelectionSetValidationVisitor(doc, ProductType); - * visitor.visit(); - * const result = visitor.getValidationResult(); - * if (result.errors.length > 0) { - * console.error('Validation failed:', result.errors); - * } - * ``` - */ -export class SelectionSetValidationVisitor { - private currentType: GraphQLObjectType; - private ancestors: GraphQLObjectType[] = []; - private readonly operationDocument: DocumentNode; - - private validationResult: ValidationResult = { - errors: [], - warnings: [], - }; - - /** - * Creates a new SelectionSetValidationVisitor. - * - * @param operationDocument - The parsed GraphQL document representing the field set - * @param objectType - The root GraphQL object type to validate against - */ - constructor(operationDocument: DocumentNode, objectType: GraphQLObjectType) { - this.operationDocument = operationDocument; - this.currentType = objectType; - } - - /** - * Executes the validation by traversing the operation document. - * After calling this method, use `getValidationResult()` to retrieve any errors or warnings. - */ - public visit(): void { - visit(this.operationDocument, this.createASTVisitor()); - } - - /** - * Returns the validation result containing any errors and warnings found during traversal. - * - * @returns The validation result with errors and warnings arrays - */ - public getValidationResult(): ValidationResult { - return this.validationResult; - } - - /** - * Creates the AST visitor configuration for traversing the document. - * - * @returns An ASTVisitor object with handlers for Field and SelectionSet nodes - */ - private createASTVisitor(): ASTVisitor { - return { - Field: { - enter: (node, key, parent, path, ancestors) => { - return this.onEnterField({ node, key, parent, path, ancestors }); - }, - }, - SelectionSet: { - enter: (node, key, parent, path, ancestors) => { - return this.onEnterSelectionSet({ node, key, parent, path, ancestors }); - }, - leave: (node, key, parent, path, ancestors) => { - this.onLeaveSelectionSet({ node, key, parent, path, ancestors }); - }, - }, - }; - } - - /** - * Handles entering a field node during traversal. - * Validates that the field's type is not an abstract type (interface or union). - * - * @param ctx - The visit context containing the field node and its ancestors - * @returns BREAK if validation fails to stop traversal, undefined otherwise - */ - private onEnterField(ctx: VisitContext): any { - const fieldDefinition = this.getFieldDefinition(ctx.node); - if (!fieldDefinition) { - return false; - } - - const namedType = this.getUnderlyingType(fieldDefinition.type); - - if (this.isAbstractType(namedType)) { - this.validationResult.errors.push( - `Abstract types are not allowed in requires directives. Found ${namedType.name} in ${this.currentType.name}.${ctx.node.name.value}`, - ); - return BREAK; - } - } - - /** - * Unwraps a GraphQL type to get its underlying named type. - * Strips NonNull and List wrappers to get the base type. - * - * @param type - The GraphQL type to unwrap - * @returns The underlying named type - */ - private getUnderlyingType(type: GraphQLType): GraphQLNamedType { - while (!isNamedType(type)) { - type = type.ofType; - } - - return type; - } - - /** - * Retrieves the field definition for a field node from the current type. - * If the field is not found, a validation error is recorded and null is returned. - * - * @param node - The field node to look up - * @returns The GraphQL field definition, or null if not found - */ - private getFieldDefinition(node: FieldNode): GraphQLField | null { - const fieldDef = this.currentType.getFields()[node.name.value]; - if (!fieldDef) { - this.validationResult.errors.push(`Field '${node.name.value}' not found on type '${this.currentType.name}'`); - return null; - } - return fieldDef; - } - - /** - * Handles entering a selection set node during traversal. - * Validates that inline fragments are not used and updates the current type - * context when descending into nested object types. - * - * @param ctx - The visit context containing the selection set node and its parent - * @returns BREAK if validation fails to stop traversal, undefined otherwise - */ - private onEnterSelectionSet(ctx: VisitContext): any { - if (!ctx.parent) { - return; - } - - if (this.isInlineFragment(ctx.parent)) { - this.validationResult.errors.push('Inline fragments are not allowed in requires directives'); - return BREAK; - } - - if (!this.isFieldNode(ctx.parent)) { - return; - } - - const fieldDefinition = this.getFieldDefinition(ctx.parent); - if (!fieldDefinition) { - return; - } - - const namedType = this.getUnderlyingType(fieldDefinition.type); - if (isObjectType(namedType)) { - this.ancestors.push(this.currentType); - this.currentType = namedType; - } - } - - /** - * Handles leaving a selection set node during traversal. - * Restores the previous type context when ascending back up the tree. - * - * @param ctx - The visit context containing the selection set node and its parent - */ - private onLeaveSelectionSet(ctx: VisitContext): void { - if (!ctx.parent) { - return; - } - - if (!this.isFieldNode(ctx.parent)) { - return; - } - - this.currentType = this.ancestors.pop() ?? this.currentType; - } - - /** - * Type guard to check if a node is an InlineFragmentNode. - * - * @param node - The AST node or array of nodes to check - * @returns True if the node is an InlineFragmentNode - */ - private isInlineFragment(node: ASTNode | readonly ASTNode[]): node is InlineFragmentNode { - if (Array.isArray(node)) { - return false; - } - - return (node as ASTNode).kind === Kind.INLINE_FRAGMENT; - } - - /** - * Type guard to check if a node is a FieldNode. - * - * @param node - The AST node or array of nodes to check - * @returns True if the node is a FieldNode - */ - private isFieldNode(node: ASTNode | ReadonlyArray): node is FieldNode { - if (Array.isArray(node)) { - return false; - } - return (node as ASTNode).kind === Kind.FIELD; - } - - /** - * Checks if a named type is an abstract type (interface or union). - * - * @param node - The GraphQL named type to check - * @returns True if the type is an interface or union type - */ - private isAbstractType(node: GraphQLNamedType): boolean { - return isInterfaceType(node) || isUnionType(node); - } -} diff --git a/protographic/tests/sdl-validation/01-basic-validation.test.ts b/protographic/tests/sdl-validation/01-basic-validation.test.ts index ef599c0adf..ecc8b6ef92 100644 --- a/protographic/tests/sdl-validation/01-basic-validation.test.ts +++ b/protographic/tests/sdl-validation/01-basic-validation.test.ts @@ -507,39 +507,6 @@ describe('SDL Validation', () => { ); }); - test('should return an error if an abstract type is used in a requires directive', () => { - const sdl = ` - type Query { - user: User! - } - - type User @key(fields: "id name") { - id: ID! - pet: Animal! @external - name: String! @requires(fields: "pet { ... on Cat { name } }") - } - - type Cat { - name: String! - } - - type Dog { - name: String! - } - - union Animal = Cat | Dog - `; - - const visitor = new SDLValidationVisitor(sdl); - const result = visitor.visit(); - - expect(result.errors).toHaveLength(1); - expect(result.warnings).toHaveLength(0); - expect(result.errors[0]).toContain( - '[Error] Abstract types are not allowed in requires directives. Found Animal in User.pet at', - ); - }); - test('should correctly handle nested fields in requires directives', () => { const sdl = ` type Warehouse @key(fields: "id") { @@ -562,50 +529,4 @@ describe('SDL Validation', () => { expect(result.errors).toHaveLength(0); expect(result.warnings).toHaveLength(0); }); - - test('should return an error for invalid @requires field selection syntax', () => { - const sdl = ` - type Query { - user: User! - } - - type User @key(fields: "id") { - id: ID! - name: String! @external - age: Int! @requires(fields: "name {") - } - `; - - const visitor = new SDLValidationVisitor(sdl); - const result = visitor.visit(); - - expect(result.errors).toHaveLength(1); - expect(result.warnings).toHaveLength(0); - expect(result.errors[0]).toContain('Invalid @requires field selection syntax'); - }); - - test('should return an error for @requires with unclosed braces', () => { - const sdl = ` - type Query { - user: User! - } - - type User @key(fields: "id") { - id: ID! - details: Details! @external - computed: String! @requires(fields: "details { foo") - } - - type Details { - foo: String! - } - `; - - const visitor = new SDLValidationVisitor(sdl); - const result = visitor.visit(); - - expect(result.errors).toHaveLength(1); - expect(result.warnings).toHaveLength(0); - expect(result.errors[0]).toContain('Invalid @requires field selection syntax'); - }); });