Add RequestServices to ValidationContext

[Shane32]

I tried to write a validation rule today and was quite surprised to find that I could not access the scoped service provider provided to the request.

Also, to add an error to the validation context, you have to add an instance of ValidationError, which currently requires that Number be provided. This is merely a NRT error, as Number is not actually required. Note that Number is defined as the following:

Gets or sets the rule number of this validation error corresponding to the paragraph number from the official specification.

So obviously there is no "paragraph number from the official specification" for custom validation errors. Hence Number should not be required for custom validation errors anyway.

[sungam3r]

Hence Number should not be required for custom validation errors anyway.

I would keep it as non-null. Writing non-standard rules is not a frequent work. Standard rules should not suffer because of this forcing callers to check Number for null. For non-standard rules number may be used as well. Code from auth project:

 public AuthorizationError(ASTNode? node, ValidationContext context, string message, AuthorizationResult result, OperationType? operationType = null)
            : base(context.Document.Source, "6.1.1", message, node == null ? Array.Empty<ASTNode>() : new ASTNode[] { node })

6.1.1 points to the paragraph with generic explanation of validation process.

[Shane32]

Then we should have an alternate way of adding a validation rule without a number. There is no practical reason why a number should be required. It is not part of the spec, and we already have codes, also not part of the spec. Why should a custom validation rule require a number? Clearly we have set up the system for custom validation rules, and clearly those will not typically have a number from the spec assigned to it.

I can add constructors that mirrors those of DocumentError. That is a better plan anyway.

Standard rules should not suffer because of this forcing callers to check Number for null.

I seriously don't know what you're talking about. The serializers already check Number for null as they always have. No user code needs to do so. Most likely this change is just the result of incorrect NRT annotations during the early part of 4.x. If we want a specific class for validation errors that relate to the spec, then we should add a class like SpecValidationError. Not require Number for EVERY validation error.

[Shane32]

I'm not sure what other people do, but this is the second validation rule I have written in the last couple weeks. Neither had to do with anything to do with the spec. Could I have punched a string in the field? Sure. But that is not the intent of the field.

[codecov-commenter]

Codecov Report

Merging #3051 (2e5fb90) into master (231e308) will increase coverage by 0.04%.
The diff coverage is 73.33%.

@@            Coverage Diff             @@
##           master    #3051      +/-   ##
==========================================
+ Coverage   83.61%   83.66%   +0.04%     
==========================================
  Files         360      360              
  Lines       15698    15710      +12     
  Branches     2557     2557              
==========================================
+ Hits        13126    13143      +17     
+ Misses       1898     1893       -5     
  Partials      674      674              

Impacted Files	Coverage Δ
src/GraphQL/Execution/ExecutionHelper.cs	87.80% <ø> (ø)
src/GraphQL/Execution/ExecutionStrategy.cs	82.52% <ø> (ø)
src/GraphQL/GlobalSwitches.cs	100.00% <ø> (ø)
src/GraphQL/GraphQLExtensions.cs	75.07% <ø> (ø)
src/GraphQL/Types/Collections/SchemaTypes.cs	87.27% <ø> (ø)
src/GraphQL/Types/Composite/NonNullGraphType.cs	100.00% <ø> (ø)
src/GraphQL/Utilities/NameValidator.cs	100.00% <ø> (ø)
src/GraphQL/Utilities/SchemaPrinter.cs	83.38% <ø> (ø)
src/GraphQL/Validation/ValidationError.cs	89.13% <55.55%> (-8.24%)	⬇️
src/GraphQL/Execution/DocumentExecuter.cs	90.00% <100.00%> (+0.04%)	⬆️
... and 5 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a548f11...2e5fb90. Read the comment docs.