fix: merge inline fragment and field selections together (#1240)

This change fixes normalization for cases like the following:

    query fragmentsWithFields {
	    dog {
		    extras {
			    ... on DogExtra {
				    string
			    }
		    }
		    extras {
			    ... on DogExtra {
				    string
				    string1
			    }
		    }
	    }
    }

I have marged two different visitors and their tests into a single
visitor. EnterSelectionSets from two stages were merged into one, but
with the following differences:

    * Internal loop was optimized,
    * Early return happens only when something is merged.

Author: ysmolski

v2/pkg/ast/ast_selection.go

@@ -241,11 +241,3 @@ func (d *Document) SelectionSetFieldNames(set int) (fieldNames []string) {
 	}
 	return
 }
-
-func (d *Document) SelectionIsFieldSelection(ref int) bool {
-	return d.Selections[ref].Kind == SelectionKindField
-}
-
-func (d *Document) SelectionIsInlineFragmentSelection(ref int) bool {
-	return d.Selections[ref].Kind == SelectionKindInlineFragment
-}

v2/pkg/astnormalization/abstract_field_normalizer.go

@@ -31,7 +31,6 @@ func NewAbstractFieldNormalizer(operation *ast.Document, definition *ast.Documen
 
 	mergeInlineFragmentSelections(&walker)
 	inlineSelectionsFromInlineFragments(&walker)
-	mergeFieldSelections(&walker)
 	deduplicateFields(&walker)
 
 	return normalizer

v2/pkg/astnormalization/astnormalization.go

@@ -3,7 +3,7 @@ Package astnormalization helps to transform parsed GraphQL AST's into a easier t
 
 # Example
 
-This examples shows how the normalization package helps "simplifying" a GraphQL AST.
+This example shows how the normalization package helps "simplify" a GraphQL AST.
 
 Input:
 
@@ -76,7 +76,7 @@ import (
 )
 
 // NormalizeOperation creates a default Normalizer and applies all rules to a given AST
-// In case you're using OperationNormalizer in a hot path you shouldn't be using this function.
+// In case you're using OperationNormalizer in a hot path, you shouldn't be using this function.
 // Create a new OperationNormalizer using NewNormalizer() instead and re-use it.
 func NormalizeOperation(operation, definition *ast.Document, report *operationreport.Report) {
 	normalizer := NewNormalizer(false, false)
@@ -193,11 +193,11 @@ func WithNormalizeDefinition() Option {
 func (o *OperationNormalizer) setupOperationWalkers() {
 	o.operationWalkers = make([]walkerStage, 0, 9)
 
-	// NOTE: normalization rules for variables relies on the fact that
-	// we will visit only single operation, so it is important to remove non-matching operations
+	// NOTE: normalization rules for variables rely on the fact that
+	// we will visit only a single operation, so it is important to remove non-matching operations
 	if o.options.removeNotMatchingOperationDefinitions {
 		removeNotMatchingOperationDefinitionsWalker := astvisitor.NewWalker(2)
-		// this rule do not walk deep into ast, so separate walk not expensive,
+		// This rule does not walk deep into ast, so a separate walk is not expensive,
 		// but we could not mix this walk with other rules, because they need to go deep
 		o.removeOperationDefinitionsVisitor = removeOperationDefinitions(&removeNotMatchingOperationDefinitionsWalker)
 
@@ -212,7 +212,6 @@ func (o *OperationNormalizer) setupOperationWalkers() {
 	directiveIncludeSkip(&directivesIncludeSkip)
 
 	cleanup := astvisitor.NewWalker(8)
-	mergeFieldSelections(&cleanup)
 	deduplicateFields(&cleanup)
 	if o.options.removeUnusedVariables {
 		del := deleteUnusedVariables(&cleanup)
@@ -272,7 +271,7 @@ func (o *OperationNormalizer) setupOperationWalkers() {
 	}
 
 	o.operationWalkers = append(o.operationWalkers, walkerStage{
-		name:   "mergeFieldSelections, deduplicateFields, deleteUnusedVariables",
+		name:   "deduplicateFields, deleteUnusedVariables",
 		walker: &cleanup,
 	})
 
@@ -332,7 +331,7 @@ func (o *OperationNormalizer) NormalizeNamedOperation(operation, definition *ast
 		}
 
 		// NOTE: debug code - do not remove
-		// printed, _ := astprinter.PrintStringIndent(operation, definition, "  ")
+		// printed, _ := astprinter.PrintStringIndent(operation, "  ")
 		// fmt.Println("\n\nNormalizeOperation stage:", o.operationWalkers[i].name)
 		// fmt.Println(printed)
 		// fmt.Println("variables:", string(operation.Input.Variables))

v2/pkg/astnormalization/astnormalization_test.go

@@ -197,6 +197,176 @@ func TestNormalizeOperation(t *testing.T) {
 				}`, ``, ``)
 	})
 
+	// The following 3 tests are to test the combined logic of inline fragment selection merging
+	// and a field selection merging. When applied separately, they could not merge the field and
+	// fragments.
+	t.Run("combined inline fragment fields merging 1", func(t *testing.T) {
+		run(t, testDefinition, `
+				query q {
+					pet {
+						... on Dog {
+								... on Pet {
+									...DogName
+									...DogExtraString
+								}
+						}
+					}
+					pet {
+						... on Dog {
+								...DogBarkVolume
+								...DogExtraString
+						}
+					}
+				}
+				fragment DogName on Pet { ... on Dog { name } }
+				fragment DogBarkVolume on Pet { ... on Dog { barkVolume } }
+				fragment DogExtraString on Dog {
+					...DogName
+					extras { 
+						... on DogExtra {
+							string 
+						}
+					}
+				} `, `
+				query q {
+					pet {
+						... on Dog {
+							name
+							extras { 
+								string 
+							}
+							barkVolume
+						}
+					}
+				}`, ``, ``)
+	})
+
+	t.Run("combined inline fragment fields merging 2", func(t *testing.T) {
+		run(t, testDefinition, `
+				query q {
+					pet {
+						...DogExtraString
+					}
+					pet {
+						... on Dog {
+							extras {
+								... on DogExtra {
+									string1
+								}
+							}
+						}
+					}
+				}
+				fragment DogExtraString on Dog {
+					extras { 
+						... on DogExtra {
+							string
+						}
+						... on Pet {
+							__typename
+						}
+					}
+				} `, `
+				query q {
+					pet {
+						... on Dog {
+							extras {
+								string
+								... on Pet {
+									__typename
+								}
+								string1
+							}
+						}
+					}
+				}`, ``, ``)
+	})
+
+	t.Run("combined inline fragment fields merging 3", func(t *testing.T) {
+		run(t, ` schema { query: Query }
+				scalar ID
+				scalar String
+
+				type Query {
+					requestForQuote(id: ID!): Request
+				}
+				type Request { draftQuotePackage: QuotePackage }
+				type QuotePackage { requestedParts: Parts }
+				type Parts { edges: [PartEdge] }
+				type PartEdge { quote: Quote }
+				type Quote { quoteRevision: QuoteRevisionUnion }
+				union QuoteRevisionUnion = QuoteRevision | Node
+				type QuoteRevision { partBreakdown: PartBreakdown }
+				type Node { name: String! }
+				type PartBreakdown {
+					breakdownType: String!
+					cost: String!
+				} `, `
+				query SimplePartQuoteEditorQuery($requestForQuoteId: ID!) {
+				  requestForQuote(id: $requestForQuoteId) {
+					draftQuotePackage {
+					  ...useBreakdownTypeFilter_quotePackage
+					  requestedParts {
+						edges {
+						  quote {
+							quoteRevision {
+							  ... on QuoteRevision {
+								partBreakdown {
+								  cost
+								}
+							  }
+							}
+						  }
+						}
+					  }
+					}
+				  }
+				}
+
+				fragment useBreakdownTypeFilter_quotePackage on QuotePackage {
+				  requestedParts {
+					edges {
+					  quote {
+						quoteRevision {
+						  ... on QuoteRevision {
+							partBreakdown {
+							  breakdownType
+							}
+						  }
+						  ... on Node {
+							__isNode: __typename
+						  }
+						}
+					  }
+					}
+				  }
+				}
+`, `
+				query SimplePartQuoteEditorQuery($requestForQuoteId: ID!) {
+				  requestForQuote(id: $requestForQuoteId) {
+					draftQuotePackage {
+					  requestedParts {
+						edges {
+						  quote {
+							quoteRevision {
+							  ... on QuoteRevision {
+								partBreakdown {
+								  breakdownType
+								  cost
+								}
+							  }
+							  ... on Node {
+								__isNode: __typename
+							  }
+							}
+						  }
+						}
+					  }
+					}
+				  }
+				}`, `123`, `123`)
+	})
+
 	t.Run("fragments", func(t *testing.T) {
 		run(t, variablesExtractionDefinition, `
 			mutation HttpBinPost{

v2/pkg/astnormalization/field_deduplication.go

@@ -5,6 +5,7 @@ import (
 	"github.com/wundergraph/graphql-go-tools/v2/pkg/astvisitor"
 )
 
+// deduplicateFields registers a visitor to remove duplicate fields in a GraphQL operation.
 func deduplicateFields(walker *astvisitor.Walker) {
 	visitor := deduplicateFieldsVisitor{
 		Walker: walker,