fix: do not trim whitespaces around non-block strings

v2/pkg/astnormalization/astnormalization.go

@@ -208,6 +208,7 @@ func (o *OperationNormalizer) setupOperationWalkers() {
 	}
 
 	directivesIncludeSkip := astvisitor.NewWalker(8)
+	preventFragmentCycles(&directivesIncludeSkip)
 	directiveIncludeSkip(&directivesIncludeSkip)
 
 	cleanup := astvisitor.NewWalker(8)
@@ -392,3 +393,99 @@ func (v *VariablesNormalizer) NormalizeOperation(operation, definition *ast.Docu
 
 	return v.variablesExtractionVisitor.uploadsPath
 }
+
+type fragmentCycleVisitor struct {
+	*astvisitor.Walker
+	operation, definition *ast.Document
+	currentFragmentRef    int           // current fragment ref
+	spreadsInFragments    map[int][]int // fragment ref -> spread refs
+}
+
+func (f *fragmentCycleVisitor) LeaveDocument(operation, _ *ast.Document) {
+	report := f.Walker.Report
+	if report == nil {
+		return
+	}
+
+	visited := make(map[string]bool)
+	stack := make(map[string]bool)
+
+	for fragmentIdx := range f.spreadsInFragments {
+		f.detectFragmentCycle(fragmentIdx, []int{fragmentIdx}, visited, stack, operation)
+	}
+}
+
+func (f *fragmentCycleVisitor) detectFragmentCycle(fragmentIdx int, path []int, visited, stack map[string]bool, operation *ast.Document) bool {
+	fragName := string(operation.FragmentDefinitionNameBytes(fragmentIdx))
+	if stack[fragName] {
+		// Cycle detected, report using the spread that closes the cycle
+		cycleStart := 0
+		for i, idx := range path {
+			if string(operation.FragmentDefinitionNameBytes(idx)) == fragName {
+				cycleStart = i
+				break
+			}
+		}
+		cyclePath := path[cycleStart:]
+		if len(cyclePath) > 0 {
+			// The spread that closes the cycle is the first spread in the cycle
+			cycleFragIdx := cyclePath[0]
+			spreadName := operation.FragmentDefinitionNameBytes(cycleFragIdx)
+			f.Walker.Report.AddExternalError(operationreport.ErrFragmentSpreadFormsCycle(spreadName))
+		}
+		return true
+	}
+	if visited[fragName] {
+		return false
+	}
+	visited[fragName] = true
+	stack[fragName] = true
+	for _, spreadRef := range f.spreadsInFragments[fragmentIdx] {
+		// Find the fragment definition index for this spread name
+		fragName := operation.FragmentSpreadNameBytes(spreadRef)
+		fragRef, exists := operation.FragmentDefinitionRef(fragName)
+		if exists && f.detectFragmentCycle(fragRef, append(path, fragRef), visited, stack, operation) {
+			return true
+		}
+	}
+	stack[fragName] = false
+	return false
+}
+
+func (f *fragmentCycleVisitor) EnterDocument(operation, definition *ast.Document) {
+	f.operation = operation
+	f.definition = definition
+	f.currentFragmentRef = -1
+	f.spreadsInFragments = make(map[int][]int)
+}
+
+func (f *fragmentCycleVisitor) LeaveFragmentDefinition(ref int) {
+	f.currentFragmentRef = -1
+}
+
+func (f *fragmentCycleVisitor) EnterFragmentDefinition(ref int) {
+	f.currentFragmentRef = ref
+}
+
+func (f *fragmentCycleVisitor) EnterFragmentSpread(ref int) {
+	if f.currentFragmentRef == -1 {
+		return
+	}
+	if _, exists := f.spreadsInFragments[f.currentFragmentRef]; !exists {
+		f.spreadsInFragments[f.currentFragmentRef] = []int{ref}
+		return
+	}
+	f.spreadsInFragments[f.currentFragmentRef] = append(f.spreadsInFragments[f.currentFragmentRef], ref)
+}
+
+func preventFragmentCycles(walker *astvisitor.Walker) *fragmentCycleVisitor {
+	visitor := &fragmentCycleVisitor{
+		Walker:     walker,
+		operation:  nil,
+		definition: nil,
+	}
+	walker.RegisterDocumentVisitor(visitor)
+	walker.RegisterEnterFragmentSpreadVisitor(visitor)
+	walker.RegisterFragmentDefinitionVisitor(visitor)
+	return visitor
+}

v2/pkg/astparser/parser_test.go

@@ -2368,8 +2368,7 @@ this is a schema \
 							panic("want schema description to be defined")
 						}
 						description := doc.Input.ByteSliceString(schema.Description.Content)
-						expectedDescription := `this is a schema \`
-						require.Equal(t, expectedDescription, description)
+						require.Equal(t, `this is a schema \ `, description)
 						query := doc.RootOperationTypeDefinitions[schema.RootOperationTypeDefinitions.Refs[0]]
 						if query.OperationType != ast.OperationTypeQuery {
 							panic("want OperationTypeQuery")
@@ -2436,8 +2435,7 @@ this is a schema \
 							panic("want schema description to be defined")
 						}
 						description := doc.Input.ByteSliceString(schema.Description.Content)
-						expectedDescription := `this is a schema \`
-						require.Equal(t, expectedDescription, description)
+						require.Equal(t, `this is a schema \ `, description)
 						query := doc.RootOperationTypeDefinitions[schema.RootOperationTypeDefinitions.Refs[0]]
 						if query.OperationType != ast.OperationTypeQuery {
 							panic("want OperationTypeQuery")
@@ -2504,9 +2502,7 @@ this is a schema \
 						if name.DefaultValue.Value.Kind != ast.ValueKindString {
 							panic("want ValueKindString")
 						}
-						if doc.Input.ByteSliceString(doc.StringValues[name.DefaultValue.Value.Ref].Content) != `Gopher \` {
-							panic("want Gopher")
-						}
+						assert.Equal(t, doc.Input.ByteSliceString(doc.StringValues[name.DefaultValue.Value.Ref].Content), `Gopher \ `)
 					})
 			})
 
@@ -2530,9 +2526,7 @@ this is a schema \
 						if name.DefaultValue.Value.Kind != ast.ValueKindString {
 							panic("want ValueKindString")
 						}
-						if doc.Input.ByteSliceString(doc.StringValues[name.DefaultValue.Value.Ref].Content) != `Gopher \\` {
-							panic("want Gopher")
-						}
+						assert.Equal(t, doc.Input.ByteSliceString(doc.StringValues[name.DefaultValue.Value.Ref].Content), `Gopher \\ `)
 					})
 			})
 		})
@@ -2580,6 +2574,18 @@ func TestErrorReport(t *testing.T) {
 			t.Fatalf("want:\n%s\ngot:\n%s\n", want, report.Error())
 		}
 	})
+	t.Run("ident incomplete block string", func(t *testing.T) {
+		_, report := ParseGraphqlDocumentString(`union"""`)
+
+		if !report.HasErrors() {
+			t.Fatalf("want err, got nil")
+		}
+
+		want := "external: unexpected token - got: BLOCKSTRING want one of: [IDENT], locations: [{Line:1 Column:6}], path: []"
+		if report.Error() != want {
+			t.Fatalf("want:\n%s\ngot:\n%s\n", want, report.Error())
+		}
+	})
 }
 
 func TestParseStarwars(t *testing.T) {

v2/pkg/astparser/testdata/starwars.schema.graphql

@@ -172,7 +172,7 @@ scalar Boolean
 scalar ID
 "Directs the executor to include this field or fragment only when the argument is true."
 directive @include(
-    " Included when true."
+    "Included when true."
     if: Boolean!
 ) on FIELD | FRAGMENT_SPREAD | INLINE_FRAGMENT
 "Directs the executor to skip this field or fragment when the argument is true."

v2/pkg/astparser/testdata/todo.graphql

@@ -56,7 +56,7 @@ scalar Boolean
 scalar ID
 "Directs the executor to include this field or fragment only when the argument is true."
 directive @include(
-    " Included when true."
+    "Included when true."
     if: Boolean!
 ) on FIELD | FRAGMENT_SPREAD | INLINE_FRAGMENT
 "Directs the executor to skip this field or fragment when the argument is true."

v2/pkg/astprinter/fixtures/starwars_schema_definition.golden

@@ -210,7 +210,7 @@ scalar ID
 
 "Directs the executor to include this field or fragment only when the argument is true."
 directive @include(
-    "Included whentrue."
+    "Included when true."
     if: Boolean!
 ) on FIELD | FRAGMENT_SPREAD | INLINE_FRAGMENT
 

v2/pkg/astprinter/testdata/starwars.schema.graphql

@@ -204,7 +204,7 @@ scalar Boolean
 scalar ID
 "Directs the executor to include this field or fragment only when the argument is true."
 directive @include(
-    " Included whentrue."
+    "Included when true."
     if: Boolean!
 ) on FIELD | FRAGMENT_SPREAD | INLINE_FRAGMENT
 "Directs the executor to skip this field or fragment when the argument is true."

v2/pkg/asttransform/baseschema.go

@@ -148,7 +148,7 @@ scalar Boolean
 scalar ID
 "Directs the executor to include this field or fragment only when the argument is true."
 directive @include(
-    " Included when true."
+    "Included when true."
     if: Boolean!
 ) on FIELD | FRAGMENT_SPREAD | INLINE_FRAGMENT
 "Directs the executor to skip this field or fragment when the argument is true."

v2/pkg/astvalidation/operation_validation_test.go

@@ -5075,7 +5075,7 @@ scalar Boolean
 scalar ID @custom(typeName: "string")
 "Directs the executor to include this field or fragment only when the argument is true."
 directive @include(
-    " Included when true."
+    "Included when true."
     if: Boolean!
 ) on FIELD | FRAGMENT_SPREAD | INLINE_FRAGMENT
 "Directs the executor to skip this field or fragment when the argument is true."

v2/pkg/introspection/testdata/starwars.schema.graphql

@@ -178,7 +178,7 @@ scalar Boolean
 scalar ID
 "Directs the executor to include this field or fragment only when the argument is true."
 directive @include(
-    " Included when true."
+    "Included when true."
     if: Boolean!
 ) on FIELD | FRAGMENT_SPREAD | INLINE_FRAGMENT
 "Directs the executor to skip this field or fragment when the argument is true."

v2/pkg/lexer/lexer.go

@@ -352,6 +352,9 @@ func (l *Lexer) readBlockString(tok *token.Token) {
 			quoteCount = 0
 			whitespaceCount++
 		case runes.EOF:
+			tok.SetEnd(l.input.InputPosition, l.input.TextPosition)
+			tok.Literal.Start += uint32(leadingWhitespaceToken)
+			tok.Literal.End -= uint32(whitespaceCount)
 			return
 		case runes.QUOTE:
 			if escaped {
@@ -385,27 +388,20 @@ func (l *Lexer) readBlockString(tok *token.Token) {
 }
 
 func (l *Lexer) readSingleLineString(tok *token.Token) {
-
 	tok.Keyword = keyword.STRING
 
 	tok.SetStart(l.input.InputPosition, l.input.TextPosition)
 	tok.TextPosition.CharStart -= 1
 
 	escaped := false
-	whitespaceCount := 0
-	reachedFirstNonWhitespace := false
-	leadingWhitespaceToken := 0
 
 	for {
 		next := l.readRune()
 		switch next {
 		case runes.SPACE, runes.TAB:
 			escaped = false
-			whitespaceCount++
 		case runes.EOF:
 			tok.SetEnd(l.input.InputPosition, l.input.TextPosition)
-			tok.Literal.Start += uint32(leadingWhitespaceToken)
-			tok.Literal.End -= uint32(whitespaceCount)
 			return
 		case runes.QUOTE, runes.CARRIAGERETURN, runes.LINETERMINATOR:
 			if escaped {
@@ -414,19 +410,11 @@ func (l *Lexer) readSingleLineString(tok *token.Token) {
 			}
 
 			tok.SetEnd(l.input.InputPosition-1, l.input.TextPosition)
-			tok.Literal.Start += uint32(leadingWhitespaceToken)
-			tok.Literal.End -= uint32(whitespaceCount)
 			return
 		case runes.BACKSLASH:
 			escaped = !escaped
-			whitespaceCount = 0
 		default:
-			if !reachedFirstNonWhitespace {
-				reachedFirstNonWhitespace = true
-				leadingWhitespaceToken = whitespaceCount
-			}
 			escaped = false
-			whitespaceCount = 0
 		}
 	}
 }