Error in section 5.3.2 "Field Selection Merging"

[matprec]

From section 5.3.2 "Field Selection Merging"

5. If typeA or typeB is Scalar or Enum.
   a. typeA and typeB must be the same type.

6. Assert: typeA and typeB are both composite types.

7. Let mergedSet be the result of adding the selection set of fieldA and the selection set of fieldB.

Wouldn't the assertion in 6. contradict 5.a) ? 5. checks if a type is an enum or scalar and if so, ensures that both types are of the same type. It then falls through to 6., which asserts that it both are composite types. If believe that these are incongruous, first of all, because primitive types aren't compositions. Also, scalars (shouldn't this also apply to enums?) cannot contain selection sets per 5.3.3 ("must be empty").

I think it would make sense if the assertion in 6. would be an else and 7.-9. would be indented.

5. If typeA or typeB is Scalar or Enum.
   a. typeA and typeB must be the same type.

6. Else: typeA and typeB are both composite types.
   a. Let mergedSet be the result of adding the selection set of fieldA and the selection set of fieldB.
   b. Let fieldsForName be the set of selections with a given response name in mergedSet including visiting fragments and inline fragments.
   c. Given each pair of members subfieldA and subfieldB in fieldsForName:

   • SameResponseShape(subfieldA, subfieldB) must be true.

It could well be that i'm missing something, in which case i want to apologize upfront.

[IvanGoncharov]

It then falls through to 6., which asserts that it both are composite types. If believe that these are incongruous, first of all, because primitive types aren't compositions.

@MSleepyPanda I think you absolutely right.
@leebyron planned to release next version shortly and it would be great to include this fix.
Do you want to work on PR?

I think it would make sense if the assertion in 6. would be an else and 7.-9. would be indented.

It sims like spec doesn't use else in any other algorithm so I would suggest making it similar to:
http://facebook.github.io/graphql/draft/#DoesFragmentTypeApply()

a. if typeA and typeB are the same type, return true, otherwise return false.

It will also fix confusion with this statement:

SameResponseShape(fieldA, fieldB) must be true.

It conflicts with SameResponseShape definition that currently uses must be instead of return true, otherwise return false.

Also, scalars (shouldn't this also apply to enums?)

Agree 👍 If you want to work on a fix, please open a separate PR.