Skip to content

Commit aa78536

Browse files
SimonSapinSimonSapin
authored
Don’t emit a validation error for relying argument default (#700)
1 parent 22aa666 commit aa78536

File tree

6 files changed

+227
-5
lines changed

6 files changed

+227
-5
lines changed

‎crates/apollo-compiler/CHANGELOG.md‎

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -51,20 +51,26 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
5151
```
5252
## Fixes
5353

54+
- **Don’t emit a validation error for relying argument default - [SimonSapin], [pull/700]**
55+
A field argument or directive argument was incorrectly considered required
56+
as soon as it had a non-null type, even if it had a default value.
57+
5458
## Maintenance
5559
## Documentation
5660

5761
[lrlna]: https://github.com/lrlna
5862
[goto-bus-stop]: https://github.com/goto-bus-stop
63+
[SimonSapin]: https://github.com/SimonSapin
5964
[pull/698]: https://github.com/apollographql/apollo-rs/pull/698
6065
[pull/668]: https://github.com/apollographql/apollo-rs/pull/668
66+
[pull/700]: https://github.com/apollographql/apollo-rs/pull/700
6167
[JSON error format]: https://spec.graphql.org/draft/#sec-Errors.Error-Result-Format
6268

6369
# [1.0.0-beta.3](https://crates.io/crates/apollo-compiler/1.0.0-beta.3) - 2023-10-13
6470

6571
## BREAKING
6672

67-
- **Keep source files in `Arc<Map<…>>` everywhere - [SimonSapin], [pull/696]:**
73+
- **Keep source files in `Arc<Map<…>>` everywhere - [SimonSapin], [pull/696]**
6874
Change struct fields from `sources: IndexMap<FileId, Arc<SourceFile>>` (in `Schema`)
6975
or `source: Option<(FileId, Arc<SourceFile>)>` (in `Document`, `ExecutablDocument`, `FieldSet`)
7076
to `sources: SourceMap`, with:
@@ -92,7 +98,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
9298

9399
## Fixes
94100

95-
- **Don’t panic in validation or omit diagnostics when a source location is missing - [SimonSapin], [pull/697]:**
101+
- **Don’t panic in validation or omit diagnostics when a source location is missing - [SimonSapin], [pull/697]**
96102
In apollo-compiler 0.11 every element of the HIR always had a source location because
97103
it always came from a parsed input file.
98104
In 1.0 source location is always optional.
@@ -109,7 +115,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
109115

110116
## BREAKING
111117

112-
**Assorted `Schema` API changes - [SimonSapin], [pull/678]:**
118+
**Assorted `Schema` API changes - [SimonSapin], [pull/678]**
113119
- Type of the `schema_definition` field changed
114120
from `Option<SchemaDefinition>` to `SchemaDefinition`.
115121
Default root operations based on object type names

‎crates/apollo-compiler/src/validation/directive.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -314,7 +314,7 @@ pub(crate) fn validate_directives<'dir>(
314314
Some(value) => value.is_null(),
315315
};
316316

317-
if arg_def.is_required() && is_null {
317+
if arg_def.is_required() && is_null && arg_def.default_value.is_none() {
318318
let mut diagnostic = ApolloDiagnostic::new(
319319
db,
320320
dir.location(),

‎crates/apollo-compiler/src/validation/field.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,7 @@ pub(crate) fn validate_field(
9191
Some(value) => value.is_null(),
9292
};
9393

94-
if arg_definition.is_required() && is_null {
94+
if arg_definition.is_required() && is_null && arg_definition.default_value.is_none() {
9595
let mut diagnostic = ApolloDiagnostic::new(
9696
db,
9797
field.location(),

‎crates/apollo-compiler/test_data/ok/0038_argument_default.graphql‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
directive @defer(
2+
label: String
3+
if: Boolean! = true
4+
) on FRAGMENT_SPREAD | INLINE_FRAGMENT
5+
6+
type Query {
7+
guitarAmp(upTo: Int! = 11): String
8+
}
9+
10+
{
11+
... @defer {
12+
guitarAmp
13+
}
14+
}

‎crates/apollo-compiler/test_data/ok/0038_argument_default.txt‎

Lines changed: 191 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,191 @@
1+
Schema {
2+
sources: {
3+
-1: SourceFile {
4+
path: "built_in.graphql",
5+
source_text: include_str!("built_in.graphql"),
6+
parse_errors: [],
7+
},
8+
37: SourceFile {
9+
path: "0038_argument_default.graphql",
10+
source_text: "directive @defer(\n label: String\n if: Boolean! = true\n) on FRAGMENT_SPREAD | INLINE_FRAGMENT\n\ntype Query {\n guitarAmp(upTo: Int! = 11): String\n}\n\n{\n ... @defer {\n guitarAmp\n }\n}\n",
11+
parse_errors: [],
12+
},
13+
},
14+
build_errors: [],
15+
schema_definition: SchemaDefinition {
16+
description: None,
17+
directives: [],
18+
query: Some(
19+
ComponentStr {
20+
origin: Definition,
21+
node: "Query",
22+
},
23+
),
24+
mutation: None,
25+
subscription: None,
26+
},
27+
directive_definitions: {
28+
"skip": built_in_directive!("skip"),
29+
"include": built_in_directive!("include"),
30+
"deprecated": built_in_directive!("deprecated"),
31+
"specifiedBy": built_in_directive!("specifiedBy"),
32+
"defer": 0..94 @37 DirectiveDefinition {
33+
description: None,
34+
name: "defer",
35+
arguments: [
36+
20..33 @37 InputValueDefinition {
37+
description: None,
38+
name: "label",
39+
ty: 27..33 @37 Named(
40+
"String",
41+
),
42+
default_value: None,
43+
directives: [],
44+
},
45+
36..55 @37 InputValueDefinition {
46+
description: None,
47+
name: "if",
48+
ty: 40..48 @37 NonNullNamed(
49+
"Boolean",
50+
),
51+
default_value: Some(
52+
51..55 @37 Boolean(
53+
true,
54+
),
55+
),
56+
directives: [],
57+
},
58+
],
59+
repeatable: false,
60+
locations: [
61+
"FRAGMENT_SPREAD",
62+
"INLINE_FRAGMENT",
63+
],
64+
},
65+
},
66+
types: {
67+
"__Schema": built_in_type!("__Schema"),
68+
"__Type": built_in_type!("__Type"),
69+
"__TypeKind": built_in_type!("__TypeKind"),
70+
"__Field": built_in_type!("__Field"),
71+
"__InputValue": built_in_type!("__InputValue"),
72+
"__EnumValue": built_in_type!("__EnumValue"),
73+
"__Directive": built_in_type!("__Directive"),
74+
"__DirectiveLocation": built_in_type!("__DirectiveLocation"),
75+
"Int": built_in_type!("Int"),
76+
"Float": built_in_type!("Float"),
77+
"String": built_in_type!("String"),
78+
"Boolean": built_in_type!("Boolean"),
79+
"ID": built_in_type!("ID"),
80+
"Query": Object(
81+
96..147 @37 ObjectType {
82+
description: None,
83+
implements_interfaces: {},
84+
directives: [],
85+
fields: {
86+
"guitarAmp": Component {
87+
origin: Definition,
88+
node: 111..145 @37 FieldDefinition {
89+
description: None,
90+
name: "guitarAmp",
91+
arguments: [
92+
121..136 @37 InputValueDefinition {
93+
description: None,
94+
name: "upTo",
95+
ty: 127..131 @37 NonNullNamed(
96+
"Int",
97+
),
98+
default_value: Some(
99+
134..136 @37 Int(
100+
11,
101+
),
102+
),
103+
directives: [],
104+
},
105+
],
106+
ty: Named(
107+
"String",
108+
),
109+
directives: [],
110+
},
111+
},
112+
},
113+
},
114+
),
115+
},
116+
}
117+
ExecutableDocument {
118+
sources: {
119+
37: SourceFile {
120+
path: "0038_argument_default.graphql",
121+
source_text: "directive @defer(\n label: String\n if: Boolean! = true\n) on FRAGMENT_SPREAD | INLINE_FRAGMENT\n\ntype Query {\n guitarAmp(upTo: Int! = 11): String\n}\n\n{\n ... @defer {\n guitarAmp\n }\n}\n",
122+
parse_errors: [],
123+
},
124+
},
125+
build_errors: [],
126+
anonymous_operation: Some(
127+
149..185 @37 Operation {
128+
operation_type: Query,
129+
variables: [],
130+
directives: [],
131+
selection_set: SelectionSet {
132+
ty: "Query",
133+
selections: [
134+
InlineFragment(
135+
153..183 @37 InlineFragment {
136+
type_condition: None,
137+
directives: [
138+
157..163 @37 Directive {
139+
name: "defer",
140+
arguments: [],
141+
},
142+
],
143+
selection_set: SelectionSet {
144+
ty: "Query",
145+
selections: [
146+
Field(
147+
170..179 @37 Field {
148+
definition: 111..145 @37 FieldDefinition {
149+
description: None,
150+
name: "guitarAmp",
151+
arguments: [
152+
121..136 @37 InputValueDefinition {
153+
description: None,
154+
name: "upTo",
155+
ty: 127..131 @37 NonNullNamed(
156+
"Int",
157+
),
158+
default_value: Some(
159+
134..136 @37 Int(
160+
11,
161+
),
162+
),
163+
directives: [],
164+
},
165+
],
166+
ty: Named(
167+
"String",
168+
),
169+
directives: [],
170+
},
171+
alias: None,
172+
name: "guitarAmp",
173+
arguments: [],
174+
directives: [],
175+
selection_set: SelectionSet {
176+
ty: "String",
177+
selections: [],
178+
},
179+
},
180+
),
181+
],
182+
},
183+
},
184+
),
185+
],
186+
},
187+
},
188+
),
189+
named_operations: {},
190+
fragments: {},
191+
}

‎crates/apollo-compiler/test_data/serializer/ok/0038_argument_default.graphql‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
directive @defer(label: String, if: Boolean! = true) on FRAGMENT_SPREAD | INLINE_FRAGMENT
2+
3+
type Query {
4+
guitarAmp(upTo: Int! = 11): String
5+
}
6+
7+
query {
8+
... @defer {
9+
guitarAmp
10+
}
11+
}

0 commit comments

Comments
 (0)