fix implementation type generation for fragment on a union

[zzh8829]

I have:

• Written a clear PR title and description (above)
• Signed the Khan Academy CLA
• Added tests covering my changes, if applicable
• Included a link to the issue fixed, if applicable
• Included documentation, for new features
• Added an entry to the changelog

before the fix

=== RUN   TestGenerate/ComplexNamedFragments.graphql/Build
# command-line-arguments
/genqlient/generate/testdata/tmp/ComplexNamedFragments.graphql_727791265.go:2008:8: cannot use new(TopicNewestContentNewestContentArticle) (value of type *TopicNewestContentNewestContentArticle) as type TopicNewestContentNewestContentLeafContent in assignment:
	*TopicNewestContentNewestContentArticle does not implement TopicNewestContentNewestContentLeafContent (missing implementsGraphQLInterfaceSimpleLeafContent method)
/genqlient/generate/testdata/tmp/ComplexNamedFragments.graphql_727791265.go:2011:8: cannot use new(TopicNewestContentNewestContentVideo) (value of type *TopicNewestContentNewestContentVideo) as type TopicNewestContentNewestContentLeafContent in assignment:
	*TopicNewestContentNewestContentVideo does not implement TopicNewestContentNewestContentLeafContent (missing implementsGraphQLInterfaceSimpleLeafContent method)
/genqlient/generate/testdata/tmp/ComplexNamedFragments.graphql_727791265.go:2026:7: impossible type switch case: *TopicNewestContentNewestContentArticle
	(*v) (variable of type TopicNewestContentNewestContentLeafContent) cannot have dynamic type *TopicNewestContentNewestContentArticle (missing implementsGraphQLInterfaceSimpleLeafContent method)
/genqlient/generate/testdata/tmp/ComplexNamedFragments.graphql_727791265.go:2034:7: impossible type switch case: *TopicNewestContentNewestContentVideo
	(*v) (variable of type TopicNewestContentNewestContentLeafContent) cannot have dynamic type *TopicNewestContentNewestContentVideo (missing implementsGraphQLInterfaceSimpleLeafContent method)
/genqlient/generate/testdata/tmp/ComplexNamedFragments.graphql_727791265.go:2177:8: cannot use new(UserLastContentLastContentArticle) (value of type *UserLastContentLastContentArticle) as type UserLastContentLastContentLeafContent in assignment:
	*UserLastContentLastContentArticle does not implement UserLastContentLastContentLeafContent (missing implementsGraphQLInterfaceSimpleLeafContent method)
/genqlient/generate/testdata/tmp/ComplexNamedFragments.graphql_727791265.go:2180:8: cannot use new(UserLastContentLastContentVideo) (value of type *UserLastContentLastContentVideo) as type UserLastContentLastContentLeafContent in assignment:
	*UserLastContentLastContentVideo does not implement UserLastContentLastContentLeafContent (missing implementsGraphQLInterfaceSimpleLeafContent method)
/genqlient/generate/testdata/tmp/ComplexNamedFragments.graphql_727791265.go:2195:7: impossible type switch case: *UserLastContentLastContentArticle
	(*v) (variable of type UserLastContentLastContentLeafContent) cannot have dynamic type *UserLastContentLastContentArticle (missing implementsGraphQLInterfaceSimpleLeafContent method)
/genqlient/generate/testdata/tmp/ComplexNamedFragments.graphql_727791265.go:2203:7: impossible type switch case: *UserLastContentLastContentVideo
	(*v) (variable of type UserLastContentLastContentLeafContent) cannot have dynamic type *UserLastContentLastContentVideo (missing implementsGraphQLInterfaceSimpleLeafContent method)
    /genqlient/generate/generate_test.go:120: generated code does not compile: exit status 2

after fix
https://github.com/Khan/genqlient/pull/310/files#diff-3eaa9f1b68120a67e7558f37dd5984e995a6df5d454656180befca84c2dbf53aR2164

I also took a look at #64 which is the real underlaying issue i wanted to fix but that seems to be extremely complicated and probably needs multiple weeks of work. the nested interface issues came up on a query with union spreading. this PR fixes the problem of spreading when the union is the same fragment, however if the spread contains different fragment we run into #64 .

[zzh8829]

this seems affect the covariant test for some reason that might need your expertise

it appears that

func (v *ContentFieldsArticle) implementsGraphQLInterfaceNext()          {}
func (v *ContentFieldsArticle) implementsGraphQLInterfaceRelated()       {}

are extraneous for some unknown reason

but

func (v *CovariantInterfaceImplementationRandomItemContentRelatedVideo) implementsGraphQLInterfaceContentFields() {
}

correctly fixed another generation issue

[zzh8829]

this seems affect the covariant test for some reason that might need your expertise

it appears that

func (v *ContentFieldsArticle) implementsGraphQLInterfaceNext()          {}
func (v *ContentFieldsArticle) implementsGraphQLInterfaceRelated()       {}

are extraneous for some unknown reason

but

func (v *CovariantInterfaceImplementationRandomItemContentRelatedVideo) implementsGraphQLInterfaceContentFields() {
}

correctly fixed another generation issue

nvm i figured it out! small issue with embedded type checking

[benjaminjkraft]

Hmm, I think I sort of understand what's going on here but I'm not sure if this is the right fix. Specifically, it seems a bit sketchy to me to be adding this as we add implementations -- couldn't we get duplicate methods or something? Shouldn't we be writing all implementations with the interface we are writing the implementations for?

I'm wondering if the actual problem is that, for example, TopicNewestContentNewestContentArticle is somehow ending up without a field SimpleLeafContent which would provide the method. Instead, it has only Typename. We can add the method, but using that fragment won't work since the field isn't there. That's what happens elsewhere in the test case, I think -- the added methods should be redundant.

But I may be misunderstanding -- this fragment stuff gets super confusing and it's been a year or two since I last thought it through. It's not a huge problem to add redundant methods if there's a reason, I'm just worried this doesn't actually suffice to fix the problem with this query.

[zzh8829]

I think you are right I was mostly following the compile error. the real fix is we need to make TopicNewestContentNewestContentArticle embed SimpleLeafContent and unmarshal it correctly.

a good example of that would be InnerQueryFragmentRandomItemArticle
the difference in implementation is that the previous nested marshalling only works for struct reference, it failed here because the embedded field is a fragment of a union.

[zzh8829]

okay i did some digging and found a better solution

• addType for the fragment SimpleLeafContent is fine
• convertSelectionSet for the interface TopicNewestContentNewestContentLeafContent is fine
• convertSelectionSet for the type TopicNewestContentNewestContentArticle is problematic bcause fragmentMatches complains that Article does not match SimpleLeafContent which is technically somewhat correct but not good for our use case.

so I added a special check for union type matching and it seems to solve the root cause

[csilvers]

I love how simple the new solution is! I'll leave it to @benjaminjkraft to do the actual reviewing, since I don't understand all the subtleties here, but I'm wondering if you want to change the title of this PR given what ended up being the actual problem.

[benjaminjkraft]

Ahh, this makes more sense! Seems very natural to match the interface case. Thanks!