Repository: apollographql/apollo-rs
Pull request #700 (2023-10-16 14:00:14)

Don’t emit a validation error for relying argument default


Patch:
diff --git a/crates/apollo-compiler/CHANGELOG.md b/crates/apollo-compiler/CHANGELOG.md
index 17e129a..1c405ce 100644
--- a/crates/apollo-compiler/CHANGELOG.md
+++ b/crates/apollo-compiler/CHANGELOG.md
@@ -51,20 +51,26 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
   ```
 ## Fixes
 
+- **Don’t emit a validation error for relying argument default - [SimonSapin], [pull/700]**
+  A field argument or directive argument was incorrectly considered required
+  as soon as it had a non-null type, even if it had a default value.
+
 ## Maintenance
 ## Documentation
 
 [lrlna]: https://github.com/lrlna
 [goto-bus-stop]: https://github.com/goto-bus-stop
+[SimonSapin]: https://github.com/SimonSapin
 [pull/698]: https://github.com/apollographql/apollo-rs/pull/698
 [pull/668]: https://github.com/apollographql/apollo-rs/pull/668
+[pull/700]: https://github.com/apollographql/apollo-rs/pull/700
 [JSON error format]: https://spec.graphql.org/draft/#sec-Errors.Error-Result-Format
 
 # [1.0.0-beta.3](https://crates.io/crates/apollo-compiler/1.0.0-beta.3) - 2023-10-13
 
 ## BREAKING
 
-- **Keep source files in `Arc<Map<…>>` everywhere - [SimonSapin], [pull/696]:**
+- **Keep source files in `Arc<Map<…>>` everywhere - [SimonSapin], [pull/696]**
   Change struct fields from `sources: IndexMap<FileId, Arc<SourceFile>>` (in `Schema`)
   or `source: Option<(FileId, Arc<SourceFile>)>` (in `Document`, `ExecutablDocument`, `FieldSet`)
   to `sources: SourceMap`, with:
@@ -92,7 +98,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
 
 ## Fixes
 
-- **Don’t panic in validation or omit diagnostics when a source location is missing - [SimonSapin], [pull/697]:**
+- **Don’t panic in validation or omit diagnostics when a source location is missing - [SimonSapin], [pull/697]**
   In apollo-compiler 0.11 every element of the HIR always had a source location because
   it always came from a parsed input file.
   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
 
 ## BREAKING
 
-**Assorted `Schema` API changes - [SimonSapin], [pull/678]:**
+**Assorted `Schema` API changes - [SimonSapin], [pull/678]**
 - Type of the `schema_definition` field changed
   from `Option<SchemaDefinition>` to `SchemaDefinition`.
   Default root operations based on object type names
diff --git a/crates/apollo-compiler/src/validation/directive.rs b/crates/apollo-compiler/src/validation/directive.rs
index 00ad875..dc41024 100644
--- a/crates/apollo-compiler/src/validation/directive.rs
+++ b/crates/apollo-compiler/src/validation/directive.rs
@@ -314,7 +314,7 @@ pub(crate) fn validate_directives<'dir>(
                     Some(value) => value.is_null(),
                 };
 
-                if arg_def.is_required() && is_null {
+                if arg_def.is_required() && is_null && arg_def.default_value.is_none() {
                     let mut diagnostic = ApolloDiagnostic::new(
                         db,
                         dir.location(),
diff --git a/crates/apollo-compiler/src/validation/field.rs b/crates/apollo-compiler/src/validation/field.rs
index 73f9f74..3500bf5 100644
--- a/crates/apollo-compiler/src/validation/field.rs
+++ b/crates/apollo-compiler/src/validation/field.rs
@@ -91,7 +91,7 @@ pub(crate) fn validate_field(
                 Some(value) => value.is_null(),
             };
 
-            if arg_definition.is_required() && is_null {
+            if arg_definition.is_required() && is_null && arg_definition.default_value.is_none() {
                 let mut diagnostic = ApolloDiagnostic::new(
                     db,
                     field.location(),


Test patch:
diff --git a/crates/apollo-compiler/test_data/ok/0038_argument_default.graphql b/crates/apollo-compiler/test_data/ok/0038_argument_default.graphql
new file mode 100644
index 0000000..a6a96f1
--- /dev/null
+++ b/crates/apollo-compiler/test_data/ok/0038_argument_default.graphql
@@ -0,0 +1,14 @@
+directive @defer(
+  label: String
+  if: Boolean! = true
+) on FRAGMENT_SPREAD | INLINE_FRAGMENT
+
+type Query {
+  guitarAmp(upTo: Int! = 11): String
+}
+
+{
+  ... @defer {
+    guitarAmp
+  }
+}
diff --git a/crates/apollo-compiler/test_data/ok/0038_argument_default.txt b/crates/apollo-compiler/test_data/ok/0038_argument_default.txt
new file mode 100644
index 0000000..74d9059
--- /dev/null
+++ b/crates/apollo-compiler/test_data/ok/0038_argument_default.txt
@@ -0,0 +1,191 @@
+Schema {
+    sources: {
+        -1: SourceFile {
+            path: "built_in.graphql",
+            source_text: include_str!("built_in.graphql"),
+            parse_errors: [],
+        },
+        37: SourceFile {
+            path: "0038_argument_default.graphql",
+            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",
+            parse_errors: [],
+        },
+    },
+    build_errors: [],
+    schema_definition: SchemaDefinition {
+        description: None,
+        directives: [],
+        query: Some(
+            ComponentStr {
+                origin: Definition,
+                node: "Query",
+            },
+        ),
+        mutation: None,
+        subscription: None,
+    },
+    directive_definitions: {
+        "skip": built_in_directive!("skip"),
+        "include": built_in_directive!("include"),
+        "deprecated": built_in_directive!("deprecated"),
+        "specifiedBy": built_in_directive!("specifiedBy"),
+        "defer": 0..94 @37 DirectiveDefinition {
+            description: None,
+            name: "defer",
+            arguments: [
+                20..33 @37 InputValueDefinition {
+                    description: None,
+                    name: "label",
+                    ty: 27..33 @37 Named(
+                        "String",
+                    ),
+                    default_value: None,
+                    directives: [],
+                },
+                36..55 @37 InputValueDefinition {
+                    description: None,
+                    name: "if",
+                    ty: 40..48 @37 NonNullNamed(
+                        "Boolean",
+                    ),
+                    default_value: Some(
+                        51..55 @37 Boolean(
+                            true,
+                        ),
+                    ),
+                    directives: [],
+                },
+            ],
+            repeatable: false,
+            locations: [
+                "FRAGMENT_SPREAD",
+                "INLINE_FRAGMENT",
+            ],
+        },
+    },
+    types: {
+        "__Schema": built_in_type!("__Schema"),
+        "__Type": built_in_type!("__Type"),
+        "__TypeKind": built_in_type!("__TypeKind"),
+        "__Field": built_in_type!("__Field"),
+        "__InputValue": built_in_type!("__InputValue"),
+        "__EnumValue": built_in_type!("__EnumValue"),
+        "__Directive": built_in_type!("__Directive"),
+        "__DirectiveLocation": built_in_type!("__DirectiveLocation"),
+        "Int": built_in_type!("Int"),
+        "Float": built_in_type!("Float"),
+        "String": built_in_type!("String"),
+        "Boolean": built_in_type!("Boolean"),
+        "ID": built_in_type!("ID"),
+        "Query": Object(
+            96..147 @37 ObjectType {
+                description: None,
+                implements_interfaces: {},
+                directives: [],
+                fields: {
+                    "guitarAmp": Component {
+                        origin: Definition,
+                        node: 111..145 @37 FieldDefinition {
+                            description: None,
+                            name: "guitarAmp",
+                            arguments: [
+                                121..136 @37 InputValueDefinition {
+                                    description: None,
+                                    name: "upTo",
+                                    ty: 127..131 @37 NonNullNamed(
+                                        "Int",
+                                    ),
+                                    default_value: Some(
+                                        134..136 @37 Int(
+                                            11,
+                                        ),
+                                    ),
+                                    directives: [],
+                                },
+                            ],
+                            ty: Named(
+                                "String",
+                            ),
+                            directives: [],
+                        },
+                    },
+                },
+            },
+        ),
+    },
+}
+ExecutableDocument {
+    sources: {
+        37: SourceFile {
+            path: "0038_argument_default.graphql",
+            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",
+            parse_errors: [],
+        },
+    },
+    build_errors: [],
+    anonymous_operation: Some(
+        149..185 @37 Operation {
+            operation_type: Query,
+            variables: [],
+            directives: [],
+            selection_set: SelectionSet {
+                ty: "Query",
+                selections: [
+                    InlineFragment(
+                        153..183 @37 InlineFragment {
+                            type_condition: None,
+                            directives: [
+                                157..163 @37 Directive {
+                                    name: "defer",
+                                    arguments: [],
+                                },
+                            ],
+                            selection_set: SelectionSet {
+                                ty: "Query",
+                                selections: [
+                                    Field(
+                                        170..179 @37 Field {
+                                            definition: 111..145 @37 FieldDefinition {
+                                                description: None,
+                                                name: "guitarAmp",
+                                                arguments: [
+                                                    121..136 @37 InputValueDefinition {
+                                                        description: None,
+                                                        name: "upTo",
+                                                        ty: 127..131 @37 NonNullNamed(
+                                                            "Int",
+                                                        ),
+                                                        default_value: Some(
+                                                            134..136 @37 Int(
+                                                                11,
+                                                            ),
+                                                        ),
+                                                        directives: [],
+                                                    },
+                                                ],
+                                                ty: Named(
+                                                    "String",
+                                                ),
+                                                directives: [],
+                                            },
+                                            alias: None,
+                                            name: "guitarAmp",
+                                            arguments: [],
+                                            directives: [],
+                                            selection_set: SelectionSet {
+                                                ty: "String",
+                                                selections: [],
+                                            },
+                                        },
+                                    ),
+                                ],
+                            },
+                        },
+                    ),
+                ],
+            },
+        },
+    ),
+    named_operations: {},
+    fragments: {},
+}
diff --git a/crates/apollo-compiler/test_data/serializer/ok/0038_argument_default.graphql b/crates/apollo-compiler/test_data/serializer/ok/0038_argument_default.graphql
new file mode 100644
index 0000000..eb6661a
--- /dev/null
+++ b/crates/apollo-compiler/test_data/serializer/ok/0038_argument_default.graphql
@@ -0,0 +1,11 @@
+directive @defer(label: String, if: Boolean! = true) on FRAGMENT_SPREAD | INLINE_FRAGMENT
+
+type Query {
+  guitarAmp(upTo: Int! = 11): String
+}
+
+query {
+  ... @defer {
+    guitarAmp
+  }
+}