Consolidate validation rules into single OperationValidator and improve performance by andimarek · Pull Request #4228 · graphql-java/graphql-java

@andimarek @claude

Replace the AbstractRule + RulesVisitor + 31 individual rule class
architecture with a single OperationValidator class that implements
DocumentVisitor directly.

BREAKING CHANGE: The rule filtering predicate type changes from
Predicate<Class<?>> to Predicate<OperationValidationRule> in
Validator, ParseAndValidate, and GraphQL. Code that filters
validation rules by class reference must migrate to the new
OperationValidationRule enum.

- Create OperationValidationRule enum with 31 values (one per rule)
- Create OperationValidator implementing DocumentVisitor with all
  validation logic consolidated from the 31 rule classes
- Simplify Validator to create OperationValidator directly
- Update ParseAndValidate and GraphQL predicate types
- Delete AbstractRule, RulesVisitor, and all 31 rule classes
  (keep VariablesTypesMatcher as utility)
- Convert all test files from mock-based rule instantiation to
  integration tests or OperationValidator with rule predicates
- Update JMH benchmarks

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Add integration tests replacing removed mock-based tests:
- Per-operation fragment visiting (RulesVisitorTest)
- Invalid input object field, list, nested list, and simple list types (ArgumentsOfCorrectTypeTest)
- Null value for non-null input object field (ArgumentsOfCorrectTypeTest)
- Unknown type on inline fragment not triggering composite type error (FragmentsOnCompositeTypeTest)
- Directive argument scope isolation (KnownArgumentNamesTest)
- Defaulted non-null field and directive arguments (ProvidedNonNullArgumentsTest)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
…ckage to validation package

The rules sub-package is no longer needed since all 31 rule classes
were consolidated into OperationValidator. Move the remaining utility
class (VariablesTypesMatcher) and all 32 test files into the parent
graphql.validation package, then delete the empty rules directories.

Also remove stale JSpecifyAnnotationsCheck entries for deleted rule
classes and update comments referencing the old package.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Rename methods for clarity:
  - shouldRunNonFragmentSpreadChecks() -> shouldRunDocumentLevelRules()
  - shouldRunFragmentSpreadChecks() -> shouldRunOperationScopedRules()
  - fragmentSpreadVisitDepth -> fragmentRetraversalDepth

- Add comprehensive class Javadoc explaining:
  - Two independent state variables and their three valid combinations
  - Visual matrices showing when each rule category runs
  - Step-by-step traversal example with state at each step

- Add detailed Javadoc to OperationValidationRule enum categorizing
  all rules by their traversal behavior

- Simplify code by removing duplicate checks and redundant comments

Co-authored-by: Cursor <cursoragent@cursor.com>
These checks are always true in methods that are only called at
document level (depth=0):
- checkDocument(): Document node visited once at start
- checkOperationDefinition(): Operations are never inside fragments
- checkVariableDefinition(): Variable definitions only exist in operations
- documentFinished(): Called when leaving Document

Co-authored-by: Cursor <cursoragent@cursor.com>
…onValidator

ValidationContext:
- Add @NullMarked at class level
- Add @nullable to methods that can return null based on traversal state:
  getFragment, getParentType, getInputType, getDefaultValue, getFieldDef,
  getDirective, getArgument, getOutputType, getQueryPath

OperationValidator:
- Add @NullMarked at class level
- Add @nullable to methods that can return null:
  getQueryPath, requireSameNameAndArguments, findArgumentByName,
  requireSameOutputTypeShape
- Add @nullable to parameters that can be null:
  addError (SourceLocation), mkNotSameTypeError (typeA, typeB),
  overlappingFieldsImpl, overlappingFields_collectFields,
  overlappingFields_collectFieldsForInlineFragment,
  overlappingFields_collectFieldsForField, sameType, sameArguments
- Add @nullable to fields that can be null:
  variableDefinitionMap, FieldAndType.graphQLType

Co-authored-by: Cursor <cursoragent@cursor.com>
- Add @NullMarked at class level
- Add @nullable to fields: directive, argument
- Add @nullable to public methods that can return null:
  getOutputType, getParentType, getInputType, getDefaultValue,
  getFieldDef, getQueryPath, getDirective, getArgument
- Add @nullable to private methods that can return null:
  lastElement, find, getFieldDef(schema, parentType, field),
  getNullableType
- Add @nullable to parameters that accept null:
  addOutputType, addParentType, addInputType, addDefaultValue,
  addFieldDef, getNullableType

Co-authored-by: Cursor <cursoragent@cursor.com>
Fix NullAway errors from master's @NullMarked additions to
GraphQLSchema and GraphQLTypeUtil. Add null checks in
OperationValidator and TraversalContext for nullable
getCodeRegistry(), unwrapAll(), and type comparison methods.

Update JSpecifyAnnotationsCheck exemption list to match master's
annotated classes, removing deleted DeferDirective rule entries.
Add @NullMarked to ParseAndValidate to match master.
Replace HashMap/LinkedHashMap allocations with linear scans for small
argument lists, single-pass partitioning in groupByCommonParents, pre-sized
maps, computeIfAbsent, HashSet for fragment tracking, and ArrayList over
LinkedList. Short-circuit isRuleEnabled when all rules are enabled.

~12% improvement on ValidatorBenchmark (largeSchema4: 7.86→6.94ms,
manyFragments: 5.83→5.16ms).

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

@andimarek andimarek changed the title Consolidate validation rules into single OperationValidator Consolidate validation rules into single OperationValidator and improve performance

Feb 15, 2026

@andimarek

GraphQLSchema.getCodeRegistry() is no longer @nullable, so these
guards are unnecessary.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Keep the original behavior of passing locale through as-is from
ExecutionInput.getLocale() without falling back to Locale.getDefault().

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Remove unused import (GraphQLCodeRegistry)
- Remove unused method parameters (checkSelectionSet, leaveOperationDefinition,
  leaveSelectionSet, documentFinished, validateUniqueArgumentNames_directive)
- Remove always-false null check (validationContext in isExperimentalApiKeyEnabled)
- Remove redundant @SuppressWarnings("deprecation")
- Remove always-false value == null check in validateProvidedNonNullArguments
- Simplify typeA/typeB null check (typeA never null in context)
- Rename sameType -> notSameType, doTypesOverlap -> typesDoNotOverlap (always inverted)
- Extract common validateUniqueArgumentNames to deduplicate field/directive variants
- Convert indexed for-loop to enhanced for-each
- Simplify ifArgumentMightBeFalse return
- Remove dead query-type branch (schema always has query type)
- Add null safety for getSourceLocation() and StringValue.getValue()
- Add @nullable to GraphQLSchema.getDirective() (can return null per javadoc)

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>