polish: remove check for collecting forbidden directives from execution

benchmark/validateSubscription-benchmark.js

@@ -0,0 +1,67 @@
+import { parse } from 'graphql/language/parser.js';
+import { buildSchema } from 'graphql/utilities/buildASTSchema.js';
+import { validate } from 'graphql/validation/validate.js';
+
+const subscriptionSchemaSDL = `
+  type Email {
+    from: String
+    subject: String
+    asyncSubject: String
+    unread: Boolean
+  }
+
+  type Inbox {
+    total: Int
+    unread: Int
+    emails: [Email]
+  }
+
+  type Query {
+    inbox: Inbox
+  }
+
+  type EmailEvent {
+    email: Email
+    inbox: Inbox
+  }
+
+  type Subscription {
+    importantEmail(priority: Int): EmailEvent
+  }
+`;
+
+const schema = buildSchema(subscriptionSchemaSDL, { assumeValid: true });
+
+const operationAST = parse(`
+  subscription (
+    $priority: Int = 0
+    $shouldDefer: Boolean = false
+    $shouldStream: Boolean = false
+    $asyncResolver: Boolean = false
+  ) {
+    importantEmail(priority: $priority) {
+      email {
+        from
+        subject
+        ... @include(if: $asyncResolver) {
+          asyncSubject
+        }
+      }
+      ... @defer(if: $shouldDefer) {
+        inbox {
+          emails @include(if: $shouldStream) @stream(if: $shouldStream)
+          unread
+          total
+        }
+      }
+    }
+  }
+`);
+
+export const benchmark = {
+  name: 'Validate Subscription Operation',
+  count: 50,
+  measure() {
+    validate(schema, operationAST);
+  },
+};

src/execution/collectFields.ts

@@ -2,7 +2,6 @@ import { AccumulatorMap } from '../jsutils/AccumulatorMap.js';
 import type { ObjMap } from '../jsutils/ObjMap.js';
 
 import type {
-  DirectiveNode,
   FieldNode,
   FragmentDefinitionNode,
   FragmentSpreadNode,
@@ -57,8 +56,7 @@ interface CollectFieldsContext {
   runtimeType: GraphQLObjectType;
   visitedFragmentNames: Set<string>;
   hideSuggestions: boolean;
-  forbiddenDirectiveInstances: Array<DirectiveNode>;
-  forbidSkipAndInclude: boolean;
+  shouldIncludeNodeFn: typeof shouldIncludeNode;
 }
 
 /**
@@ -78,11 +76,10 @@ export function collectFields(
   runtimeType: GraphQLObjectType,
   selectionSet: SelectionSetNode,
   hideSuggestions: boolean,
-  forbidSkipAndInclude = false,
+  shouldIncludeNodeFn = shouldIncludeNode,
 ): {
   groupedFieldSet: GroupedFieldSet;
   newDeferUsages: ReadonlyArray<DeferUsage>;
-  forbiddenDirectiveInstances: ReadonlyArray<DirectiveNode>;
 } {
   const groupedFieldSet = new AccumulatorMap<string, FieldDetails>();
   const newDeferUsages: Array<DeferUsage> = [];
@@ -93,15 +90,13 @@ export function collectFields(
     runtimeType,
     visitedFragmentNames: new Set(),
     hideSuggestions,
-    forbiddenDirectiveInstances: [],
-    forbidSkipAndInclude,
+    shouldIncludeNodeFn,
   };
 
   collectFieldsImpl(context, selectionSet, groupedFieldSet, newDeferUsages);
   return {
     groupedFieldSet,
     newDeferUsages,
-    forbiddenDirectiveInstances: context.forbiddenDirectiveInstances,
   };
 }
 
@@ -134,8 +129,7 @@ export function collectSubfields(
     runtimeType: returnType,
     visitedFragmentNames: new Set(),
     hideSuggestions,
-    forbiddenDirectiveInstances: [],
-    forbidSkipAndInclude: false,
+    shouldIncludeNodeFn: shouldIncludeNode,
   };
   const subGroupedFieldSet = new AccumulatorMap<string, FieldDetails>();
   const newDeferUsages: Array<DeferUsage> = [];
@@ -177,17 +171,18 @@ function collectFieldsImpl(
     runtimeType,
     visitedFragmentNames,
     hideSuggestions,
+    shouldIncludeNodeFn,
   } = context;
 
   for (const selection of selectionSet.selections) {
     switch (selection.kind) {
       case Kind.FIELD: {
         if (
-          !shouldIncludeNode(
-            context,
+          !shouldIncludeNodeFn(
             selection,
             variableValues,
             fragmentVariableValues,
+            hideSuggestions,
           )
         ) {
           continue;
@@ -201,11 +196,11 @@ function collectFieldsImpl(
       }
       case Kind.INLINE_FRAGMENT: {
         if (
-          !shouldIncludeNode(
-            context,
+          !shouldIncludeNodeFn(
             selection,
             variableValues,
             fragmentVariableValues,
+            hideSuggestions,
           ) ||
           !doesFragmentConditionMatch(schema, selection, runtimeType)
         ) {
@@ -247,11 +242,11 @@ function collectFieldsImpl(
 
         if (
           visitedFragmentNames.has(fragName) ||
-          !shouldIncludeNode(
-            context,
+          !shouldIncludeNodeFn(
             selection,
             variableValues,
             fragmentVariableValues,
+            hideSuggestions,
           )
         ) {
           continue;
@@ -348,25 +343,21 @@ function getDeferUsage(
  * directives, where `@skip` has higher precedence than `@include`.
  */
 function shouldIncludeNode(
-  context: CollectFieldsContext,
   node: FragmentSpreadNode | FieldNode | InlineFragmentNode,
   variableValues: VariableValues,
   fragmentVariableValues: VariableValues | undefined,
+  hideSuggestions: boolean,
 ): boolean {
   const skipDirectiveNode = node.directives?.find(
     (directive) => directive.name.value === GraphQLSkipDirective.name,
   );
-  if (skipDirectiveNode && context.forbidSkipAndInclude) {
-    context.forbiddenDirectiveInstances.push(skipDirectiveNode);
-    return false;
-  }
   const skip = skipDirectiveNode
     ? experimentalGetArgumentValues(
         skipDirectiveNode,
         GraphQLSkipDirective.args,
         variableValues,
         fragmentVariableValues,
-        context.hideSuggestions,
+        hideSuggestions,
       )
     : undefined;
   if (skip?.if === true) {
@@ -376,17 +367,13 @@ function shouldIncludeNode(
   const includeDirectiveNode = node.directives?.find(
     (directive) => directive.name.value === GraphQLIncludeDirective.name,
   );
-  if (includeDirectiveNode && context.forbidSkipAndInclude) {
-    context.forbiddenDirectiveInstances.push(includeDirectiveNode);
-    return false;
-  }
   const include = includeDirectiveNode
     ? experimentalGetArgumentValues(
         includeDirectiveNode,
         GraphQLIncludeDirective.args,
         variableValues,
         fragmentVariableValues,
-        context.hideSuggestions,
+        hideSuggestions,
       )
     : undefined;
   if (include?.if === false) {

src/validation/rules/SingleFieldSubscriptionsRule.ts

@@ -2,10 +2,19 @@ import type { ObjMap } from '../../jsutils/ObjMap.js';
 
 import { GraphQLError } from '../../error/GraphQLError.js';
 
-import type { FieldNode, OperationDefinitionNode } from '../../language/ast.js';
+import type {
+  DirectiveNode,
+  FieldNode,
+  OperationDefinitionNode,
+} from '../../language/ast.js';
 import { Kind } from '../../language/kinds.js';
 import type { ASTVisitor } from '../../language/visitor.js';
 
+import {
+  GraphQLIncludeDirective,
+  GraphQLSkipDirective,
+} from '../../type/directives.js';
+
 import type {
   FieldDetailsList,
   FragmentDetails,
@@ -46,16 +55,30 @@ export function SingleFieldSubscriptionsRule(
               fragments[definition.name.value] = { definition };
             }
           }
-          const { groupedFieldSet, forbiddenDirectiveInstances } =
-            collectFields(
-              schema,
-              fragments,
-              variableValues,
-              subscriptionType,
-              node.selectionSet,
-              context.hideSuggestions,
-              true,
-            );
+          const forbiddenDirectiveInstances: Array<DirectiveNode> = [];
+          const { groupedFieldSet } = collectFields(
+            schema,
+            fragments,
+            variableValues,
+            subscriptionType,
+            node.selectionSet,
+            context.hideSuggestions,
+            (selection) => {
+              for (const directive of [
+                GraphQLSkipDirective,
+                GraphQLIncludeDirective,
+              ]) {
+                const directiveNode = selection.directives?.find(
+                  (d) => d.name.value === directive.name,
+                );
+                if (directiveNode !== undefined) {
+                  forbiddenDirectiveInstances.push(directiveNode);
+                  return false;
+                }
+              }
+              return true;
+            },
+          );
           if (forbiddenDirectiveInstances.length > 0) {
             context.reportError(
               new GraphQLError(