Repository: apollographql/apollo-rs
Pull request #528 (2023-04-20 12:05:27)

feat(compiler): validate enum extensions, fixes #479
New errors for:

- duplicate members
  ```graphql
  enum E { MEMBER_A }
  extend enum E { MEMBER_A } # ERROR: the enum value `MEMBER_2` is defined multiple times in the document
  ```
  (the diagnostic message is not ideal there as it's not related to the document, could be improved by adding a DiagnosticData type specifically for enum values)

- duplicate directive applications
  ```graphql
  enum E @nonRepeatable {}
  extend enum E @nonRepeatable # ERROR: non-repeatable directive nonRepeatable can only be used once per location
  ```



Patch:
diff --git a/crates/apollo-compiler/src/validation/enum_.rs b/crates/apollo-compiler/src/validation/enum_.rs
index f52312f..d8a0af4 100644
--- a/crates/apollo-compiler/src/validation/enum_.rs
+++ b/crates/apollo-compiler/src/validation/enum_.rs
@@ -17,6 +17,15 @@ pub fn validate_enum_definitions(db: &dyn ValidationDatabase) -> Vec<ApolloDiagn
     diagnostics
 }
 
+fn iter_with_extensions<'a, Item, Ext>(
+    base: &'a [Item],
+    extensions: &'a [Arc<Ext>],
+    method: impl Fn(&'a Ext) -> &'a [Item],
+) -> impl Iterator<Item = &'a Item> {
+    base.iter()
+        .chain(extensions.iter().flat_map(move |ext| method(ext).iter()))
+}
+
 pub fn validate_enum_definition(
     db: &dyn ValidationDatabase,
     enum_def: Arc<hir::EnumTypeDefinition>,
@@ -26,8 +35,14 @@ pub fn validate_enum_definition(
         hir::DirectiveLocation::Enum,
     );
 
+    let enum_values = iter_with_extensions(
+        enum_def.self_values(),
+        enum_def.extensions(),
+        hir::EnumTypeExtension::enum_values_definition,
+    );
+
     let mut seen: HashMap<&str, &EnumValueDefinition> = HashMap::new();
-    for enum_val in enum_def.self_values() {
+    for enum_val in enum_values {
         diagnostics.extend(db.validate_enum_value(enum_val.clone()));
 
         // An Enum type must define one or more unique enum values.
@@ -42,7 +57,7 @@ pub fn validate_enum_definition(
                     db,
                     redefined_definition.into(),
                     DiagnosticData::UniqueDefinition {
-                        ty: "enum",
+                        ty: "enum value",
                         name: value.into(),
                         original_definition: original_definition.into(),
                         redefined_definition: redefined_definition.into(),


Test patch:
diff --git a/crates/apollo-compiler/test_data/diagnostics/0019_enum_definition_with_duplicate_values.txt b/crates/apollo-compiler/test_data/diagnostics/0019_enum_definition_with_duplicate_values.txt
index 8fcc2a5..cca3c5c 100644
--- a/crates/apollo-compiler/test_data/diagnostics/0019_enum_definition_with_duplicate_values.txt
+++ b/crates/apollo-compiler/test_data/diagnostics/0019_enum_definition_with_duplicate_values.txt
@@ -37,7 +37,7 @@
             "CAT must only be defined once in this enum.",
         ),
         data: UniqueDefinition {
-            ty: "enum",
+            ty: "enum value",
             name: "CAT",
             original_definition: DiagnosticLocation {
                 file_id: FileId {
@@ -93,7 +93,7 @@
             "THRIVE_PET_FOODS must only be defined once in this enum.",
         ),
         data: UniqueDefinition {
-            ty: "enum",
+            ty: "enum value",
             name: "THRIVE_PET_FOODS",
             original_definition: DiagnosticLocation {
                 file_id: FileId {
diff --git a/crates/apollo-compiler/test_data/diagnostics/0070_self_referential_directive_definition.graphql b/crates/apollo-compiler/test_data/diagnostics/0070_self_referential_directive_definition.graphql
index 3abb5b0..fb7ee8c 100644
--- a/crates/apollo-compiler/test_data/diagnostics/0070_self_referential_directive_definition.graphql
+++ b/crates/apollo-compiler/test_data/diagnostics/0070_self_referential_directive_definition.graphql
@@ -18,6 +18,6 @@ enum Enum { NON_RECURSIVE }
 extend enum Enum { RECURSIVE @wrong }
 input Name { value: String!, type: Enum }
 input InputObject { name: Name }
-directive @wrong(input: InputObject) on INPUT_FIELD_DEFINITION
+directive @wrong(input: InputObject) on INPUT_FIELD_DEFINITION | ENUM_VALUE
 
 directive @thisOneShouldNotError(arg: Boolean @loopC) on FIELD
diff --git a/crates/apollo-compiler/test_data/diagnostics/0070_self_referential_directive_definition.txt b/crates/apollo-compiler/test_data/diagnostics/0070_self_referential_directive_definition.txt
index a334058..3f9b2e8 100644
--- a/crates/apollo-compiler/test_data/diagnostics/0070_self_referential_directive_definition.txt
+++ b/crates/apollo-compiler/test_data/diagnostics/0070_self_referential_directive_definition.txt
@@ -204,7 +204,7 @@
                 id: 70,
             },
             offset: 705,
-            length: 62,
+            length: 75,
         },
         labels: [
             Label {
diff --git a/crates/apollo-compiler/test_data/diagnostics/0097_enum_extensions.graphql b/crates/apollo-compiler/test_data/diagnostics/0097_enum_extensions.graphql
new file mode 100644
index 0000000..a8804dc
--- /dev/null
+++ b/crates/apollo-compiler/test_data/diagnostics/0097_enum_extensions.graphql
@@ -0,0 +1,16 @@
+directive @nonRepeatable on ENUM
+
+enum E {
+  MEMBER_1
+  MEMBER_2
+}
+
+extend enum E @nonRepeatable {
+  MEMBER_3
+  MEMBER_4
+}
+
+extend enum E @nonRepeatable {
+  MEMBER_2
+  MEMBER_4
+}
diff --git a/crates/apollo-compiler/test_data/diagnostics/0097_enum_extensions.txt b/crates/apollo-compiler/test_data/diagnostics/0097_enum_extensions.txt
new file mode 100644
index 0000000..4dc229b
--- /dev/null
+++ b/crates/apollo-compiler/test_data/diagnostics/0097_enum_extensions.txt
@@ -0,0 +1,167 @@
+[
+    ApolloDiagnostic {
+        cache: {
+            0: "built_in_types.graphql",
+            94: "0097_enum_extensions.graphql",
+        },
+        location: DiagnosticLocation {
+            file_id: FileId {
+                id: 94,
+            },
+            offset: 138,
+            length: 14,
+        },
+        labels: [
+            Label {
+                location: DiagnosticLocation {
+                    file_id: FileId {
+                        id: 94,
+                    },
+                    offset: 82,
+                    length: 14,
+                },
+                text: "directive nonRepeatable first called here",
+            },
+            Label {
+                location: DiagnosticLocation {
+                    file_id: FileId {
+                        id: 94,
+                    },
+                    offset: 138,
+                    length: 14,
+                },
+                text: "directive nonRepeatable called again here",
+            },
+        ],
+        help: None,
+        data: UniqueDirective {
+            name: "nonRepeatable",
+            original_call: DiagnosticLocation {
+                file_id: FileId {
+                    id: 94,
+                },
+                offset: 82,
+                length: 14,
+            },
+            conflicting_call: DiagnosticLocation {
+                file_id: FileId {
+                    id: 94,
+                },
+                offset: 138,
+                length: 14,
+            },
+        },
+    },
+    ApolloDiagnostic {
+        cache: {
+            0: "built_in_types.graphql",
+            94: "0097_enum_extensions.graphql",
+        },
+        location: DiagnosticLocation {
+            file_id: FileId {
+                id: 94,
+            },
+            offset: 157,
+            length: 8,
+        },
+        labels: [
+            Label {
+                location: DiagnosticLocation {
+                    file_id: FileId {
+                        id: 94,
+                    },
+                    offset: 56,
+                    length: 8,
+                },
+                text: "previous definition of `MEMBER_2` here",
+            },
+            Label {
+                location: DiagnosticLocation {
+                    file_id: FileId {
+                        id: 94,
+                    },
+                    offset: 157,
+                    length: 8,
+                },
+                text: "`MEMBER_2` redefined here",
+            },
+        ],
+        help: Some(
+            "MEMBER_2 must only be defined once in this enum.",
+        ),
+        data: UniqueDefinition {
+            ty: "enum value",
+            name: "MEMBER_2",
+            original_definition: DiagnosticLocation {
+                file_id: FileId {
+                    id: 94,
+                },
+                offset: 56,
+                length: 8,
+            },
+            redefined_definition: DiagnosticLocation {
+                file_id: FileId {
+                    id: 94,
+                },
+                offset: 157,
+                length: 8,
+            },
+        },
+    },
+    ApolloDiagnostic {
+        cache: {
+            0: "built_in_types.graphql",
+            94: "0097_enum_extensions.graphql",
+        },
+        location: DiagnosticLocation {
+            file_id: FileId {
+                id: 94,
+            },
+            offset: 168,
+            length: 8,
+        },
+        labels: [
+            Label {
+                location: DiagnosticLocation {
+                    file_id: FileId {
+                        id: 94,
+                    },
+                    offset: 112,
+                    length: 8,
+                },
+                text: "previous definition of `MEMBER_4` here",
+            },
+            Label {
+                location: DiagnosticLocation {
+                    file_id: FileId {
+                        id: 94,
+                    },
+                    offset: 168,
+                    length: 8,
+                },
+                text: "`MEMBER_4` redefined here",
+            },
+        ],
+        help: Some(
+            "MEMBER_4 must only be defined once in this enum.",
+        ),
+        data: UniqueDefinition {
+            ty: "enum value",
+            name: "MEMBER_4",
+            original_definition: DiagnosticLocation {
+                file_id: FileId {
+                    id: 94,
+                },
+                offset: 112,
+                length: 8,
+            },
+            redefined_definition: DiagnosticLocation {
+                file_id: FileId {
+                    id: 94,
+                },
+                offset: 168,
+                length: 8,
+            },
+        },
+    },
+]