Be more explicit about forbidding skip/include

Author: benjie

src/execution/collectFields.ts

@@ -78,6 +78,7 @@ export function collectFields(
   runtimeType: GraphQLObjectType,
   selectionSet: SelectionSetNode,
   hideSuggestions: boolean,
+  forbidSkipInclude = false,
 ): {
   groupedFieldSet: GroupedFieldSet;
   newDeferUsages: ReadonlyArray<DeferUsage>;
@@ -95,24 +96,22 @@ export function collectFields(
     forbiddenDirectiveInstances: [],
   };
 
-  collectFieldsImpl(context, selectionSet, groupedFieldSet, newDeferUsages);
+  collectFieldsImpl(
+    context,
+    selectionSet,
+    groupedFieldSet,
+    newDeferUsages,
+    undefined,
+    undefined,
+    forbidSkipInclude,
+  );
   return {
     groupedFieldSet,
     newDeferUsages,
     forbiddenDirectiveInstances: context.forbiddenDirectiveInstances,
   };
 }
 
-/**
- * This variable is the empty variables used during the validation phase (where
- * no variables exist) for field collection; if a `@skip` or `@include`
- * directive is ever seen when `variableValues` is set to this, it should
- * throw.
- */
-export const VALIDATION_PHASE_EMPTY_VARIABLES: VariableValues = Object.freeze(
-  Object.create(null),
-);
-
 /**
  * Given an array of field nodes, collects all of the subfields of the passed
  * in fields, and returns them at the end.
@@ -134,6 +133,7 @@ export function collectSubfields(
 ): {
   groupedFieldSet: GroupedFieldSet;
   newDeferUsages: ReadonlyArray<DeferUsage>;
+  forbiddenDirectiveInstances: ReadonlyArray<DirectiveNode>;
 } {
   const context: CollectFieldsContext = {
     schema,
@@ -177,6 +177,7 @@ function collectFieldsImpl(
   newDeferUsages: Array<DeferUsage>,
   deferUsage?: DeferUsage,
   fragmentVariableValues?: VariableValues,
+  forbidSkipInclude = false,
 ): void {
   const {
     schema,
@@ -197,6 +198,7 @@ function collectFieldsImpl(
             variableValues,
             fragmentVariableValues,
             hideSuggestions,
+            forbidSkipInclude,
           )
         ) {
           continue;
@@ -216,6 +218,7 @@ function collectFieldsImpl(
             variableValues,
             fragmentVariableValues,
             hideSuggestions,
+            forbidSkipInclude,
           ) ||
           !doesFragmentConditionMatch(schema, selection, runtimeType)
         ) {
@@ -263,6 +266,7 @@ function collectFieldsImpl(
             variableValues,
             fragmentVariableValues,
             hideSuggestions,
+            forbidSkipInclude,
           )
         ) {
           continue;
@@ -364,14 +368,12 @@ function shouldIncludeNode(
   variableValues: VariableValues,
   fragmentVariableValues: VariableValues | undefined,
   hideSuggestions: Maybe<boolean>,
+  forbidSkipInclude: boolean,
 ): boolean {
   const skipDirectiveNode = node.directives?.find(
     (directive) => directive.name.value === GraphQLSkipDirective.name,
   );
-  if (
-    skipDirectiveNode &&
-    variableValues === VALIDATION_PHASE_EMPTY_VARIABLES
-  ) {
+  if (skipDirectiveNode && forbidSkipInclude) {
     context.forbiddenDirectiveInstances.push(skipDirectiveNode);
     return false;
   }
@@ -391,10 +393,7 @@ function shouldIncludeNode(
   const includeDirectiveNode = node.directives?.find(
     (directive) => directive.name.value === GraphQLIncludeDirective.name,
   );
-  if (
-    includeDirectiveNode &&
-    variableValues === VALIDATION_PHASE_EMPTY_VARIABLES
-  ) {
+  if (includeDirectiveNode && forbidSkipInclude) {
     context.forbiddenDirectiveInstances.push(includeDirectiveNode);
     return false;
   }

src/validation/rules/SingleFieldSubscriptionsRule.ts

@@ -10,10 +10,8 @@ import type {
   FieldDetailsList,
   FragmentDetails,
 } from '../../execution/collectFields.js';
-import {
-  collectFields,
-  VALIDATION_PHASE_EMPTY_VARIABLES,
-} from '../../execution/collectFields.js';
+import { collectFields } from '../../execution/collectFields.js';
+import type { VariableValues } from '../../execution/values.js';
 
 import type { ValidationContext } from '../ValidationContext.js';
 
@@ -40,7 +38,7 @@ export function SingleFieldSubscriptionsRule(
         const subscriptionType = schema.getSubscriptionType();
         if (subscriptionType) {
           const operationName = node.name ? node.name.value : null;
-          const variableValues = VALIDATION_PHASE_EMPTY_VARIABLES;
+          const variableValues: VariableValues = Object.create(null);
           const document = context.getDocument();
           const fragments: ObjMap<FragmentDetails> = Object.create(null);
           for (const definition of document.definitions) {
@@ -56,6 +54,7 @@ export function SingleFieldSubscriptionsRule(
               subscriptionType,
               node.selectionSet,
               context.hideSuggestions,
+              true,
             );
           if (forbiddenDirectiveInstances.length > 0) {
             context.reportError(