potential lookahead lexer bug?

[yaacovCR]

Revisiting graphql/graphql-js#2764

I'm not quite sure the reference implementation has a bug, see failing test when implementing the fix as advised:

graphql/graphql-js#4293

[yaacovCR]

Which would imply that the implementation here has a potential bug, as it assumed that the reference implementation was flawed:

GraphQL/Sources/GraphQL/Language/Lexer.swift

Lines 137 to 141 in 5e098b3

	// restore these since both `positionAfterWhitespace` & `readBlockString`
	// can potentially modify them and commment for `lookahead` says no lexer modification.
	// (the latter is true in the canonical js lexer also and is likely a bug)
	line = savedLine
	lineStart = savedLineStart

[yaacovCR]

Basically, when the comment says the lexer start is not changed, I think it means with respect to the current token only, but not with respect to other internal state.

In fact, I think there is an optimization that means additional lexer state can and must change.

Specifically, when we look ahead, we save the next token within the linked list, even though we don't modify the current token pointer, such that when we advance() after calling lookahead() we don't have to reprocess the next token, we just read from token.next. (advance() works by calling lookahead() and then specifically advancing the current token pointer.)

Considering that we never re-lex a token, the line and start position of the line within the body must be permanently advanced by lookahead() otherwise the lexer will think that is on the old line. From what I can tell, parsing will continue as normal, because we lex from the end position of the last token, and that won't be incorrect, but every token from that point on will have the wrong line number and presumably the wrong column.

I don't see the corresponding lexSecond tests from graphql-js in https://github.com/GraphQLSwift/GraphQL/blob/main/Tests/GraphQLTests/LanguageTests/LexerTests.swift but I assume that's where you would find the bug, if not in more involved tests.