Skip to content

Commit

Permalink
Make variables in const contexts validation errors (#925)
Browse files Browse the repository at this point in the history
Fixes #852

GraphQL values can be of different kinds, including variables with the `$x` syntax.
Variables are not allowed in some contexts, such as schemas.
The parser already rejects them as syntax errors, but the Rust API allows mutating
a document and representing variable values in contexts where they are not allowed.

It was already the case that would usually cause a validation error like
``variable `$x` is not defined``. This PR ensures and tests that this is the case
for every relevant context. It also makes related drive-by fixes:

* Validate the default value of input fields and arguments
* Validate the default value of variables once at their definition,
  not at every usage of the variable. This fixes duplicate diagnostics
  for a default value of incorrect type for a variable correctly used multiple times.
  • Loading branch information
SimonSapin authored Nov 5, 2024
1 parent 1cb621d commit 973254d
Show file tree
Hide file tree
Showing 9 changed files with 443 additions and 21 deletions.
2 changes: 2 additions & 0 deletions crates/apollo-compiler/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,11 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm

## Fixes
- **Validate against reserved names starting with `__` in schemas - [SimonSapin], [pull/923].**
- **Validate the default value of input fields and arguments - [SimonSapin], [pull/925].**

[SimonSapin]: https://github.com/SimonSapin
[pull/923]: https://github.com/apollographql/apollo-rs/issues/923
[pull/925]: https://github.com/apollographql/apollo-rs/issues/925


# [1.0.0-beta.24](https://crates.io/crates/apollo-compiler/1.0.0-beta.24) - 2024-09-24
Expand Down
5 changes: 5 additions & 0 deletions crates/apollo-compiler/src/validation/input_object.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ use crate::collections::HashMap;
use crate::schema::validation::BuiltInScalars;
use crate::schema::InputObjectType;
use crate::validation::diagnostics::DiagnosticData;
use crate::validation::value::value_of_correct_type;
use crate::validation::CycleError;
use crate::validation::DiagnosticList;
use crate::validation::RecursionGuard;
Expand Down Expand Up @@ -208,6 +209,10 @@ pub(crate) fn validate_input_value_definitions(
},
);
}
if let Some(default) = &input_value.default_value {
let var_defs = &[];
value_of_correct_type(diagnostics, schema, &input_value.ty, default, var_defs);
}
} else if is_built_in {
// `validate_schema()` will insert the missing definition
} else {
Expand Down
12 changes: 0 additions & 12 deletions crates/apollo-compiler/src/validation/value.rs
Original file line number Diff line number Diff line change
Expand Up @@ -143,18 +143,6 @@ pub(crate) fn value_of_correct_type(
// TODO(@goto-bus-stop) This should use the is_assignable_to check
if var_def.ty.inner_named_type() != ty.inner_named_type() {
unsupported_type(diagnostics, arg_value, ty);
} else if let Some(default_value) = &var_def.default_value {
if var_def.ty.is_non_null() && default_value.is_null() {
unsupported_type(diagnostics, default_value, &var_def.ty)
} else {
value_of_correct_type(
diagnostics,
schema,
&var_def.ty,
default_value,
var_defs,
)
}
}
}
_ => unsupported_type(diagnostics, arg_value, ty),
Expand Down
7 changes: 6 additions & 1 deletion crates/apollo-compiler/src/validation/variable.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ use crate::ast;
use crate::collections::HashMap;
use crate::executable;
use crate::validation::diagnostics::DiagnosticData;
use crate::validation::value::value_of_correct_type;
use crate::validation::DiagnosticList;
use crate::validation::RecursionGuard;
use crate::validation::RecursionLimitError;
Expand Down Expand Up @@ -35,7 +36,11 @@ pub(crate) fn validate_variable_definitions(

match type_definition {
Some(type_definition) if type_definition.is_input_type() => {
// OK!
if let Some(default) = &variable.default_value {
// Default values are "const", not allowed to refer to other variables:
let var_defs_in_scope = &[];
value_of_correct_type(diagnostics, schema, ty, default, var_defs_in_scope);
}
}
Some(type_definition) => {
diagnostics.push(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -311,6 +311,7 @@ query variablesWithInvalidDefaultValues(
complexArgField(complexArg: $c)
intArgField(intArg: $a)
stringArgField(stringArg: $b)
again: stringArgField(stringArg: $b)
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -512,42 +512,42 @@ Error: expected value of type ComplexInput, found a string
│ ╰───────── provided value is a string
─────╯
Error: expected value of type Boolean!, found an integer
╭─[0102_invalid_string_values.graphql:318:39]
╭─[0102_invalid_string_values.graphql:319:39]
32 │ requiredField: Boolean!
│ ────┬───
│ ╰───── expected type declared here as Boolean!
318 │ $a: ComplexInput = { requiredField: 123, intField: "abc" }
319 │ $a: ComplexInput = { requiredField: 123, intField: "abc" }
│ ─┬─
│ ╰─── provided value is an integer
─────╯
Error: expected value of type Int, found a string
╭─[0102_invalid_string_values.graphql:318:54]
╭─[0102_invalid_string_values.graphql:319:54]
34 │ intField: Int
│ ─┬─
│ ╰─── expected type declared here as Int
318 │ $a: ComplexInput = { requiredField: 123, intField: "abc" }
319 │ $a: ComplexInput = { requiredField: 123, intField: "abc" }
│ ──┬──
│ ╰──── provided value is a string
─────╯
Error: the required field `ComplexInput.requiredField` is not provided
╭─[0102_invalid_string_values.graphql:326:22]
╭─[0102_invalid_string_values.graphql:327:22]
32 │ requiredField: Boolean!
│ ───────────┬───────────
│ ╰───────────── field defined here
326 │ $a: ComplexInput = {intField: 3}
327 │ $a: ComplexInput = {intField: 3}
│ ──────┬──────
│ ╰──────── missing value for field `requiredField`
─────╯
Error: expected value of type String, found an integer
╭─[0102_invalid_string_values.graphql:335:26]
╭─[0102_invalid_string_values.graphql:336:26]
335 │ $a: [String] = ["one", 2]
336 │ $a: [String] = ["one", 2]
│ ────┬─── ┬
│ ╰───────────────── expected type declared here as String
│ │
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,13 @@ Error: syntax error: unexpected variable value in a Const context
│ ┬
│ ╰── unexpected variable value in a Const context
───╯
Error: variable `$var1` is not defined
╭─[0111_const_value.graphql:3:23]
3 │ $var2: Boolean! = $var1
│ ──┬──
│ ╰──── not found in this scope
───╯
Error: syntax error: unexpected variable value in a Const context
╭─[0111_const_value.graphql:11:26]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -284,6 +284,7 @@ query variablesWithInvalidDefaultValues($a: Int = "one", $b: String = 4, $c: Com
complexArgField(complexArg: $c)
intArgField(intArg: $a)
stringArgField(stringArg: $b)
again: stringArgField(stringArg: $b)
}
}

Expand Down
Loading

0 comments on commit 973254d

Please sign in to comment.