diff --git a/compiler/crates/relay-transforms/src/errors.rs b/compiler/crates/relay-transforms/src/errors.rs index 86d604dd8ab8..2bd523ad4397 100644 --- a/compiler/crates/relay-transforms/src/errors.rs +++ b/compiler/crates/relay-transforms/src/errors.rs @@ -223,25 +223,27 @@ pub enum ValidationMessage { }, #[error( - "Field '{field_name}' marked with @exhaustive is missing selection for union members: {member_names}." + "Field '{field_name}' marked with @exhaustive is missing selection for {type_description}: {member_names}." )] - MissingExhaustiveUnionMembersOnField { + MissingExhaustiveMembersOnField { field_name: StringKey, member_names: String, + type_description: &'static str, }, #[error( - "Fragment '{fragment_name}' marked with @exhaustive is missing selection for union members: {member_names}." + "Fragment '{fragment_name}' marked with @exhaustive is missing selection for {type_description}: {member_names}." )] - MissingExhaustiveUnionMembersOnFragment { + MissingExhaustiveMembersOnFragment { fragment_name: StringKey, member_names: String, + type_description: &'static str, }, #[error( - "The @exhaustive directive can only be applied to fields or fragment definitions returning union types." + "The @exhaustive directive can only be applied to fields or fragment definitions returning union or interface types." )] - ExhaustiveDirectiveOnNonUnionField, + ExhaustiveDirectiveOnNonUnionOrInterfaceField, } #[derive( diff --git a/compiler/crates/relay-transforms/src/validations/validate_exhaustive_directive.rs b/compiler/crates/relay-transforms/src/validations/validate_exhaustive_directive.rs index 41a0ec68b3e3..28002cf0f6f6 100644 --- a/compiler/crates/relay-transforms/src/validations/validate_exhaustive_directive.rs +++ b/compiler/crates/relay-transforms/src/validations/validate_exhaustive_directive.rs @@ -18,10 +18,12 @@ use intern::string_key::Intern; use intern::string_key::StringKey; use lazy_static::lazy_static; use relay_config::ProjectConfig; +use schema::InterfaceID; use schema::ObjectID; use schema::SDLSchema; use schema::Schema; use schema::Type; +use schema::UnionID; use crate::ValidationMessage; @@ -93,9 +95,9 @@ impl<'schema, 'program, 'pc> ExhaustiveDirectiveValidator<'schema, 'program, 'pc (ignored, disabled) } - fn check_exhaustive_coverage( + fn check_exhaustive_union_coverage( &self, - union_id: schema::UnionID, + union_id: UnionID, ignored: &std::collections::HashSet, has_selection_fn: impl Fn(ObjectID) -> bool, ) -> Vec { @@ -114,6 +116,37 @@ impl<'schema, 'program, 'pc> ExhaustiveDirectiveValidator<'schema, 'program, 'pc missing_members } + fn check_exhaustive_interface_coverage( + &self, + interface_id: InterfaceID, + ignored: &std::collections::HashSet, + has_selection_fn: impl Fn(ObjectID) -> bool, + ) -> Vec { + let mut implementing_objects = self + .schema + .interface(interface_id) + .recursively_implementing_objects(self.schema.as_ref()) + .into_iter() + .collect::>(); + implementing_objects.sort_by_key(|object_id| { + self.schema.get_type_name(Type::Object(*object_id)) + }); + + let mut missing_members = Vec::new(); + + for object_id in implementing_objects { + let member_name = self.schema.get_type_name(Type::Object(object_id)); + if ignored.contains(&member_name) { + continue; + } + if !has_selection_fn(object_id) { + missing_members.push(member_name); + } + } + + missing_members + } + fn validate_exhaustive(&mut self, field: &LinkedField) { let mut ignored = std::collections::HashSet::new(); let mut has_directive = false; @@ -138,21 +171,28 @@ impl<'schema, 'program, 'pc> ExhaustiveDirectiveValidator<'schema, 'program, 'pc let field_def = self.schema.field(field.definition.item); let return_type = field_def.type_.inner(); - let union_id = match return_type { - Type::Union(id) => id, + let (missing_members, type_description) = match return_type { + Type::Union(id) => ( + self.check_exhaustive_union_coverage(id, &ignored, |object_id| { + self.has_selection_for_object(field, object_id) + }), + "union members", + ), + Type::Interface(id) => ( + self.check_exhaustive_interface_coverage(id, &ignored, |object_id| { + self.has_selection_for_object(field, object_id) + }), + "interface implementations", + ), _ => { self.errors.push(Diagnostic::error( - ValidationMessage::ExhaustiveDirectiveOnNonUnionField, + ValidationMessage::ExhaustiveDirectiveOnNonUnionOrInterfaceField, field.definition.location, )); return; } }; - let missing_members = self.check_exhaustive_coverage(union_id, &ignored, |object_id| { - self.has_selection_for_object(field, object_id) - }); - if !missing_members.is_empty() { let field_name = field.alias.map_or(field_def.name.item, |a| a.item); let member_names = missing_members @@ -162,9 +202,10 @@ impl<'schema, 'program, 'pc> ExhaustiveDirectiveValidator<'schema, 'program, 'pc .join(", "); self.errors.push(Diagnostic::error( - ValidationMessage::MissingExhaustiveUnionMembersOnField { + ValidationMessage::MissingExhaustiveMembersOnField { field_name, member_names, + type_description, }, field.definition.location, )); @@ -182,21 +223,28 @@ impl<'schema, 'program, 'pc> ExhaustiveDirectiveValidator<'schema, 'program, 'pc return; } - let union_id = match fragment.type_condition { - Type::Union(id) => id, + let (missing_members, type_description) = match fragment.type_condition { + Type::Union(id) => ( + self.check_exhaustive_union_coverage(id, &ignored, |object_id| { + self.has_selection_for_object_in_fragment(fragment, object_id) + }), + "union members", + ), + Type::Interface(id) => ( + self.check_exhaustive_interface_coverage(id, &ignored, |object_id| { + self.has_selection_for_object_in_fragment(fragment, object_id) + }), + "interface implementations", + ), _ => { self.errors.push(Diagnostic::error( - ValidationMessage::ExhaustiveDirectiveOnNonUnionField, + ValidationMessage::ExhaustiveDirectiveOnNonUnionOrInterfaceField, fragment.name.location, )); return; } }; - let missing_members = self.check_exhaustive_coverage(union_id, &ignored, |object_id| { - self.has_selection_for_object_in_fragment(fragment, object_id) - }); - if !missing_members.is_empty() { let fragment_name = fragment.name.item.into(); let member_names = missing_members @@ -206,9 +254,10 @@ impl<'schema, 'program, 'pc> ExhaustiveDirectiveValidator<'schema, 'program, 'pc .join(", "); self.errors.push(Diagnostic::error( - ValidationMessage::MissingExhaustiveUnionMembersOnFragment { + ValidationMessage::MissingExhaustiveMembersOnFragment { fragment_name, member_names, + type_description, }, fragment.name.location, )); @@ -295,7 +344,7 @@ impl Validator for ExhaustiveDirectiveValidator<'_, '_, '_> { fn validate_scalar_field(&mut self, field: &ScalarField) -> DiagnosticsResult<()> { if field.directives.named(*EXHAUSTIVE_DIRECTIVE_NAME).is_some() { self.errors.push(Diagnostic::error( - ValidationMessage::ExhaustiveDirectiveOnNonUnionField, + ValidationMessage::ExhaustiveDirectiveOnNonUnionOrInterfaceField, field.definition.location, )); } diff --git a/compiler/crates/relay-transforms/tests/validate_exhaustive_directive/fixtures/directive-on-non-union.invalid.expected b/compiler/crates/relay-transforms/tests/validate_exhaustive_directive/fixtures/directive-on-non-union.invalid.expected index 7a2c9bb95451..511264b1ed01 100644 --- a/compiler/crates/relay-transforms/tests/validate_exhaustive_directive/fixtures/directive-on-non-union.invalid.expected +++ b/compiler/crates/relay-transforms/tests/validate_exhaustive_directive/fixtures/directive-on-non-union.invalid.expected @@ -4,7 +4,7 @@ fragment DirectiveOnNonUnion on User { name @exhaustive } ==================================== ERROR ==================================== -✖︎ The @exhaustive directive can only be applied to fields or fragment definitions returning union types. +✖︎ The @exhaustive directive can only be applied to fields or fragment definitions returning union or interface types. directive-on-non-union.invalid.graphql:3:3 2 │ fragment DirectiveOnNonUnion on User { diff --git a/compiler/crates/relay-transforms/tests/validate_exhaustive_directive/fixtures/interface-all-members-valid.expected b/compiler/crates/relay-transforms/tests/validate_exhaustive_directive/fixtures/interface-all-members-valid.expected new file mode 100644 index 000000000000..42046961381f --- /dev/null +++ b/compiler/crates/relay-transforms/tests/validate_exhaustive_directive/fixtures/interface-all-members-valid.expected @@ -0,0 +1,34 @@ +==================================== INPUT ==================================== +fragment InterfaceAllMembersValid on User { + nameRenderable @exhaustive { + ... on PlainUserNameRenderer { + __typename + } + ... on MarkdownUserNameRenderer { + __typename + } + ... on ImplementsImplementsUserNameRenderable { + __typename + } + ... on ImplementsImplementsUserNameRenderableAndUserNameRenderable { + __typename + } + } +} +==================================== OUTPUT =================================== +fragment InterfaceAllMembersValid on User { + nameRenderable @exhaustive { + ... on PlainUserNameRenderer { + __typename + } + ... on MarkdownUserNameRenderer { + __typename + } + ... on ImplementsImplementsUserNameRenderable { + __typename + } + ... on ImplementsImplementsUserNameRenderableAndUserNameRenderable { + __typename + } + } +} diff --git a/compiler/crates/relay-transforms/tests/validate_exhaustive_directive/fixtures/interface-all-members-valid.graphql b/compiler/crates/relay-transforms/tests/validate_exhaustive_directive/fixtures/interface-all-members-valid.graphql new file mode 100644 index 000000000000..61cda4d01c35 --- /dev/null +++ b/compiler/crates/relay-transforms/tests/validate_exhaustive_directive/fixtures/interface-all-members-valid.graphql @@ -0,0 +1,16 @@ +fragment InterfaceAllMembersValid on User { + nameRenderable @exhaustive { + ... on PlainUserNameRenderer { + __typename + } + ... on MarkdownUserNameRenderer { + __typename + } + ... on ImplementsImplementsUserNameRenderable { + __typename + } + ... on ImplementsImplementsUserNameRenderableAndUserNameRenderable { + __typename + } + } +} diff --git a/compiler/crates/relay-transforms/tests/validate_exhaustive_directive/fixtures/interface-missing-member.invalid.expected b/compiler/crates/relay-transforms/tests/validate_exhaustive_directive/fixtures/interface-missing-member.invalid.expected new file mode 100644 index 000000000000..233f5986c74f --- /dev/null +++ b/compiler/crates/relay-transforms/tests/validate_exhaustive_directive/fixtures/interface-missing-member.invalid.expected @@ -0,0 +1,17 @@ +==================================== INPUT ==================================== +#expected-to-throw +fragment InterfaceMissingMember on User { + nameRenderable @exhaustive { + ... on PlainUserNameRenderer { __typename } + ... on MarkdownUserNameRenderer { __typename } + ... on ImplementsImplementsUserNameRenderable { __typename } + } +} +==================================== ERROR ==================================== +✖︎ Field 'nameRenderable' marked with @exhaustive is missing selection for interface implementations: 'ImplementsImplementsUserNameRenderableAndUserNameRenderable'. + + interface-missing-member.invalid.graphql:3:3 + 2 │ fragment InterfaceMissingMember on User { + 3 │ nameRenderable @exhaustive { + │ ^^^^^^^^^^^^^^ + 4 │ ... on PlainUserNameRenderer { __typename } diff --git a/compiler/crates/relay-transforms/tests/validate_exhaustive_directive/fixtures/interface-missing-member.invalid.graphql b/compiler/crates/relay-transforms/tests/validate_exhaustive_directive/fixtures/interface-missing-member.invalid.graphql new file mode 100644 index 000000000000..912e91a41de8 --- /dev/null +++ b/compiler/crates/relay-transforms/tests/validate_exhaustive_directive/fixtures/interface-missing-member.invalid.graphql @@ -0,0 +1,7 @@ +fragment InterfaceMissingMember on User { + nameRenderable @exhaustive { + ... on PlainUserNameRenderer { __typename } + ... on MarkdownUserNameRenderer { __typename } + ... on ImplementsImplementsUserNameRenderable { __typename } + } +} diff --git a/compiler/crates/relay-transforms/tests/validate_exhaustive_directive_test.rs b/compiler/crates/relay-transforms/tests/validate_exhaustive_directive_test.rs index 8ebb150993e8..a926876d3173 100644 --- a/compiler/crates/relay-transforms/tests/validate_exhaustive_directive_test.rs +++ b/compiler/crates/relay-transforms/tests/validate_exhaustive_directive_test.rs @@ -169,3 +169,41 @@ async fn multiple_missing_members() { ) .await; } + +#[tokio::test] +async fn interface_all_members_valid() { + let input = include_str!( + "validate_exhaustive_directive/fixtures/interface-all-members-valid.graphql" + ); + let expected = include_str!( + "validate_exhaustive_directive/fixtures/interface-all-members-valid.expected" + ); + test_fixture( + transform_fixture, + file!(), + "interface-all-members-valid.graphql", + "validate_exhaustive_directive/fixtures/interface-all-members-valid.expected", + input, + expected, + ) + .await; +} + +#[tokio::test] +async fn interface_missing_member() { + let input = include_str!( + "validate_exhaustive_directive/fixtures/interface-missing-member.invalid.graphql" + ); + let expected = include_str!( + "validate_exhaustive_directive/fixtures/interface-missing-member.invalid.expected" + ); + test_fixture( + transform_fixture, + file!(), + "interface-missing-member.invalid.graphql", + "validate_exhaustive_directive/fixtures/interface-missing-member.invalid.expected", + input, + expected, + ) + .await; +}