Skip to content

Commit 973254d

Browse files
authored
Make variables in const contexts validation errors (#925)
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.
1 parent 1cb621d commit 973254d

File tree

9 files changed

+443
-21
lines changed

9 files changed

+443
-21
lines changed

crates/apollo-compiler/CHANGELOG.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,9 +21,11 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
2121

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

2526
[SimonSapin]: https://github.com/SimonSapin
2627
[pull/923]: https://github.com/apollographql/apollo-rs/issues/923
28+
[pull/925]: https://github.com/apollographql/apollo-rs/issues/925
2729

2830

2931
# [1.0.0-beta.24](https://crates.io/crates/apollo-compiler/1.0.0-beta.24) - 2024-09-24

crates/apollo-compiler/src/validation/input_object.rs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ use crate::collections::HashMap;
33
use crate::schema::validation::BuiltInScalars;
44
use crate::schema::InputObjectType;
55
use crate::validation::diagnostics::DiagnosticData;
6+
use crate::validation::value::value_of_correct_type;
67
use crate::validation::CycleError;
78
use crate::validation::DiagnosticList;
89
use crate::validation::RecursionGuard;
@@ -208,6 +209,10 @@ pub(crate) fn validate_input_value_definitions(
208209
},
209210
);
210211
}
212+
if let Some(default) = &input_value.default_value {
213+
let var_defs = &[];
214+
value_of_correct_type(diagnostics, schema, &input_value.ty, default, var_defs);
215+
}
211216
} else if is_built_in {
212217
// `validate_schema()` will insert the missing definition
213218
} else {

crates/apollo-compiler/src/validation/value.rs

Lines changed: 0 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -143,18 +143,6 @@ pub(crate) fn value_of_correct_type(
143143
// TODO(@goto-bus-stop) This should use the is_assignable_to check
144144
if var_def.ty.inner_named_type() != ty.inner_named_type() {
145145
unsupported_type(diagnostics, arg_value, ty);
146-
} else if let Some(default_value) = &var_def.default_value {
147-
if var_def.ty.is_non_null() && default_value.is_null() {
148-
unsupported_type(diagnostics, default_value, &var_def.ty)
149-
} else {
150-
value_of_correct_type(
151-
diagnostics,
152-
schema,
153-
&var_def.ty,
154-
default_value,
155-
var_defs,
156-
)
157-
}
158146
}
159147
}
160148
_ => unsupported_type(diagnostics, arg_value, ty),

crates/apollo-compiler/src/validation/variable.rs

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ use crate::ast;
22
use crate::collections::HashMap;
33
use crate::executable;
44
use crate::validation::diagnostics::DiagnosticData;
5+
use crate::validation::value::value_of_correct_type;
56
use crate::validation::DiagnosticList;
67
use crate::validation::RecursionGuard;
78
use crate::validation::RecursionLimitError;
@@ -35,7 +36,11 @@ pub(crate) fn validate_variable_definitions(
3536

3637
match type_definition {
3738
Some(type_definition) if type_definition.is_input_type() => {
38-
// OK!
39+
if let Some(default) = &variable.default_value {
40+
// Default values are "const", not allowed to refer to other variables:
41+
let var_defs_in_scope = &[];
42+
value_of_correct_type(diagnostics, schema, ty, default, var_defs_in_scope);
43+
}
3944
}
4045
Some(type_definition) => {
4146
diagnostics.push(

crates/apollo-compiler/test_data/diagnostics/0102_invalid_string_values.graphql

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -311,6 +311,7 @@ query variablesWithInvalidDefaultValues(
311311
complexArgField(complexArg: $c)
312312
intArgField(intArg: $a)
313313
stringArgField(stringArg: $b)
314+
again: stringArgField(stringArg: $b)
314315
}
315316
}
316317

crates/apollo-compiler/test_data/diagnostics/0102_invalid_string_values.txt

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -512,42 +512,42 @@ Error: expected value of type ComplexInput, found a string
512512
│ ╰───────── provided value is a string
513513
─────╯
514514
Error: expected value of type Boolean!, found an integer
515-
╭─[0102_invalid_string_values.graphql:318:39]
515+
╭─[0102_invalid_string_values.graphql:319:39]
516516
517517
32 │ requiredField: Boolean!
518518
│ ────┬───
519519
│ ╰───── expected type declared here as Boolean!
520520
521-
318 │ $a: ComplexInput = { requiredField: 123, intField: "abc" }
521+
319 │ $a: ComplexInput = { requiredField: 123, intField: "abc" }
522522
│ ─┬─
523523
│ ╰─── provided value is an integer
524524
─────╯
525525
Error: expected value of type Int, found a string
526-
╭─[0102_invalid_string_values.graphql:318:54]
526+
╭─[0102_invalid_string_values.graphql:319:54]
527527
528528
34 │ intField: Int
529529
│ ─┬─
530530
│ ╰─── expected type declared here as Int
531531
532-
318 │ $a: ComplexInput = { requiredField: 123, intField: "abc" }
532+
319 │ $a: ComplexInput = { requiredField: 123, intField: "abc" }
533533
│ ──┬──
534534
│ ╰──── provided value is a string
535535
─────╯
536536
Error: the required field `ComplexInput.requiredField` is not provided
537-
╭─[0102_invalid_string_values.graphql:326:22]
537+
╭─[0102_invalid_string_values.graphql:327:22]
538538
539539
32 │ requiredField: Boolean!
540540
│ ───────────┬───────────
541541
│ ╰───────────── field defined here
542542
543-
326 │ $a: ComplexInput = {intField: 3}
543+
327 │ $a: ComplexInput = {intField: 3}
544544
│ ──────┬──────
545545
│ ╰──────── missing value for field `requiredField`
546546
─────╯
547547
Error: expected value of type String, found an integer
548-
╭─[0102_invalid_string_values.graphql:335:26]
548+
╭─[0102_invalid_string_values.graphql:336:26]
549549
550-
335 │ $a: [String] = ["one", 2]
550+
336 │ $a: [String] = ["one", 2]
551551
│ ────┬─── ┬
552552
│ ╰───────────────── expected type declared here as String
553553
│ │

crates/apollo-compiler/test_data/diagnostics/0111_const_value.txt

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,13 @@ Error: syntax error: unexpected variable value in a Const context
55
│ ┬
66
│ ╰── unexpected variable value in a Const context
77
───╯
8+
Error: variable `$var1` is not defined
9+
╭─[0111_const_value.graphql:3:23]
10+
11+
3 │ $var2: Boolean! = $var1
12+
│ ──┬──
13+
│ ╰──── not found in this scope
14+
───╯
815
Error: syntax error: unexpected variable value in a Const context
916
╭─[0111_const_value.graphql:11:26]
1017

crates/apollo-compiler/test_data/serializer/diagnostics/0102_invalid_string_values.graphql

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -284,6 +284,7 @@ query variablesWithInvalidDefaultValues($a: Int = "one", $b: String = 4, $c: Com
284284
complexArgField(complexArg: $c)
285285
intArgField(intArg: $a)
286286
stringArgField(stringArg: $b)
287+
again: stringArgField(stringArg: $b)
287288
}
288289
}
289290

0 commit comments

Comments
 (0)