From 687f26bb79b7c2c8ac18ea9f33b6293c1946afa1 Mon Sep 17 00:00:00 2001 From: Sunli Date: Sat, 25 Jun 2022 10:51:59 +0800 Subject: [PATCH] Fixes #943 --- .../rules/overlapping_fields_can_be_merged.rs | 97 +++++++++++++++++-- 1 file changed, 88 insertions(+), 9 deletions(-) diff --git a/src/validation/rules/overlapping_fields_can_be_merged.rs b/src/validation/rules/overlapping_fields_can_be_merged.rs index 50ebbf4dd..4a7e2a422 100644 --- a/src/validation/rules/overlapping_fields_can_be_merged.rs +++ b/src/validation/rules/overlapping_fields_can_be_merged.rs @@ -20,18 +20,18 @@ impl<'a> Visitor<'a> for OverlappingFieldsCanBeMerged { visited: Default::default(), ctx, }; - find_conflicts.find(selection_set); + find_conflicts.find(None, selection_set); } } struct FindConflicts<'a, 'ctx> { - outputs: HashMap<&'a str, &'a Positioned>, + outputs: HashMap<(Option<&'a str>, &'a str), &'a Positioned>, visited: HashSet<&'a str>, ctx: &'a mut VisitorContext<'ctx>, } impl<'a, 'ctx> FindConflicts<'a, 'ctx> { - pub fn find(&mut self, selection_set: &'a Positioned) { + pub fn find(&mut self, on_type: Option<&'a str>, selection_set: &'a Positioned) { for selection in &selection_set.node.items { match &selection.node { Selection::Field(field) => { @@ -41,15 +41,22 @@ impl<'a, 'ctx> FindConflicts<'a, 'ctx> { .as_ref() .map(|name| &name.node) .unwrap_or_else(|| &field.node.name.node); - self.add_output(&output_name, field); + self.add_output(on_type, &output_name, field); } Selection::InlineFragment(inline_fragment) => { - self.find(&inline_fragment.node.selection_set); + let on_type = inline_fragment + .node + .type_condition + .as_ref() + .map(|cond| cond.node.on.node.as_str()); + self.find(on_type, &inline_fragment.node.selection_set); } Selection::FragmentSpread(fragment_spread) => { if let Some(fragment) = self.ctx.fragment(&fragment_spread.node.fragment_name.node) { + let on_type = Some(fragment.node.type_condition.node.on.node.as_str()); + if !self .visited .insert(fragment_spread.node.fragment_name.node.as_str()) @@ -58,15 +65,21 @@ impl<'a, 'ctx> FindConflicts<'a, 'ctx> { // `NoFragmentCycles` validator. continue; } - self.find(&fragment.node.selection_set); + + self.find(on_type, &fragment.node.selection_set); } } } } } - fn add_output(&mut self, name: &'a str, field: &'a Positioned) { - if let Some(prev_field) = self.outputs.get(name) { + fn add_output( + &mut self, + on_type: Option<&'a str>, + name: &'a str, + field: &'a Positioned, + ) { + if let Some(prev_field) = self.outputs.get(&(on_type, name)) { if prev_field.node.name.node != field.node.name.node { self.ctx.report_error( vec![prev_field.pos, field.pos], @@ -90,7 +103,73 @@ impl<'a, 'ctx> FindConflicts<'a, 'ctx> { } } } else { - self.outputs.insert(name, field); + self.outputs.insert((on_type, name), field); } } } + +#[cfg(test)] +mod tests { + use super::*; + + pub fn factory() -> OverlappingFieldsCanBeMerged { + OverlappingFieldsCanBeMerged + } + + #[test] + fn same_field_on_different_type() { + expect_passes_rule!( + factory, + r#" + { + pet { + ... on Dog { + doesKnowCommand(dogCommand: SIT) + } + ... on Cat { + doesKnowCommand(catCommand: JUMP) + } + } + } + "#, + ); + } + + #[test] + fn same_field_on_same_type() { + expect_fails_rule!( + factory, + r#" + { + pet { + ... on Dog { + doesKnowCommand(dogCommand: SIT) + } + ... on Dog { + doesKnowCommand(dogCommand: Heel) + } + } + } + "#, + ); + } + + #[test] + fn same_alias_on_different_type() { + expect_passes_rule!( + factory, + r#" + { + pet { + ... on Dog { + volume: barkVolume + } + ... on Cat { + volume: meowVolume + } + } + } + "#, + ); + } +}