Skip to content

Commit c3e2a4b

Browse files
committed
Fix overlapping fields validator
As a follow up to 6f2af8d, this ensures that fields can only be merged if their directive arguments align as well
1 parent 6ec76f4 commit c3e2a4b

File tree

2 files changed

+49
-12
lines changed

2 files changed

+49
-12
lines changed

src/validator/__tests__/OverlappingFieldsCanBeMerged.js

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -143,6 +143,21 @@ describe('Validate: Overlapping fields can be merged', () => {
143143
]);
144144
});
145145

146+
it('conflicting directive args', () => {
147+
expectFailsRule(OverlappingFieldsCanBeMerged, `
148+
fragment conflictingDirectiveArgs on Dog {
149+
name @include(if: true)
150+
name @include(if: false)
151+
}
152+
`, [
153+
{ message: fieldsConflictMessage(
154+
'name',
155+
'they have differing directives'
156+
),
157+
locations: [ { line: 3, column: 9 }, { line: 4, column: 9 } ] }
158+
]);
159+
});
160+
146161
it('conflicting args with matching directives', () => {
147162
expectFailsRule(OverlappingFieldsCanBeMerged, `
148163
fragment conflictingArgsWithMatchingDirectiveArgs on Dog {

src/validator/rules/OverlappingFieldsCanBeMerged.js

Lines changed: 34 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -90,9 +90,9 @@ export default function OverlappingFieldsCanBeMerged(
9090
];
9191
}
9292

93-
var args1 = ast1.arguments || [];
94-
var args2 = ast2.arguments || [];
95-
if (!sameNameValuePairs(args1, args2)) {
93+
var arguments1 = ast1.arguments || [];
94+
var arguments2 = ast2.arguments || [];
95+
if (!sameArguments(arguments1, arguments2)) {
9696
return [
9797
[responseName, 'they have differing arguments'],
9898
[ast1, ast2]
@@ -101,7 +101,7 @@ export default function OverlappingFieldsCanBeMerged(
101101

102102
var directives1 = ast1.directives || [];
103103
var directives2 = ast2.directives || [];
104-
if (!sameNameValuePairs(directives1, directives2)) {
104+
if (!sameDirectives(directives1, directives2)) {
105105
return [
106106
[responseName, 'they have differing directives'],
107107
[ast1, ast2]
@@ -166,19 +166,41 @@ type Conflict = [ConflictReason, Array<Field>];
166166
// Field name and reason, or field name and list of sub-conflicts.
167167
type ConflictReason = [string, string] | [string, Array<ConflictReason>];
168168

169-
function sameNameValuePairs(
170-
pairs1: Array<Argument | Directive>,
171-
pairs2: Array<Argument | Directive>
169+
function sameDirectives(
170+
directives1: Array<Directive>,
171+
directives2: Array<Directive>
172172
): boolean {
173-
if (pairs1.length !== pairs2.length) {
173+
if (directives1.length !== directives2.length) {
174174
return false;
175175
}
176-
return pairs1.every(pair1 => {
177-
var pair2 = find(pairs2, pair => pair.name.value === pair1.name.value);
178-
if (!pair2) {
176+
return directives1.every(directive1 => {
177+
var directive2 = find(
178+
directives2,
179+
directive => directive.name.value === directive1.name.value
180+
);
181+
if (!directive2) {
179182
return false;
180183
}
181-
return sameValue(pair1.value, pair2.value);
184+
return sameArguments(directive1.arguments, directive2.arguments);
185+
});
186+
}
187+
188+
function sameArguments(
189+
arguments1: Array<Argument>,
190+
arguments2: Array<Argument>
191+
): boolean {
192+
if (arguments1.length !== arguments2.length) {
193+
return false;
194+
}
195+
return arguments1.every(argument1 => {
196+
var argument2 = find(
197+
arguments2,
198+
argument => argument.name.value === argument1.name.value
199+
);
200+
if (!argument2) {
201+
return false;
202+
}
203+
return sameValue(argument1.value, argument2.value);
182204
});
183205
}
184206

0 commit comments

Comments
 (0)