From 75243323b2581a5403c76c9b02a29dbd2d83e4a1 Mon Sep 17 00:00:00 2001 From: arxhr007 Date: Tue, 3 Nov 2026 11:28:27 +0000 Subject: [PATCH] Fix line counting in get_location and print_block_string Use the same GraphQL line terminator set (LF, CR, CRLF) that graphql-js and print_source_location() use, instead of str.splitlines() which also splits on VT, FF, FS/GS/RS, NEL and U+2028/U+2029 and drops a trailing empty line segment. This fixes three issues: - characters right after a line terminator were reported on the previous line - characters after non-GraphQL line breaks got wrong line numbers - print_source_location() could crash with IndexError when an error location pointed past the last printed line (issue #294) print_block_string() also splits raw values into lines and had the same drift: values containing e.g. VT printed as multi-line in Python but single-line in graphql-js. --- src/graphql/language/block_string.py | 9 +++++++-- src/graphql/language/source.py | 18 +++++++++++------- tests/error/test_print_location.py | 14 ++++++++++++++ tests/language/test_block_string.py | 6 ++++++ tests/language/test_source.py | 12 ++++++++++++ 5 files changed, 50 insertions(+), 9 deletions(-) diff --git a/src/graphql/language/block_string.py b/src/graphql/language/block_string.py index 7ae55bdf8..7af3b61cd 100644 --- a/src/graphql/language/block_string.py +++ b/src/graphql/language/block_string.py @@ -2,6 +2,7 @@ from __future__ import annotations +import re from sys import maxsize from typing import TYPE_CHECKING @@ -14,6 +15,8 @@ "print_block_string", ] +_re_newline = re.compile(r"\r\n|[\n\r]") + def dedent_block_string_lines(lines: Collection[str]) -> list[str]: """Produce the value of a block string from its parsed raw value. @@ -120,8 +123,10 @@ def print_block_string(value: str, minimize: bool = False) -> str: escaped_value = value.replace('"""', '\\"""') - # Expand a block string's raw value into independent lines. - lines = escaped_value.splitlines() or [""] + # Expand a block string's raw value into independent lines. Split on the + # GraphQL line terminators only (LF, CR, CRLF) like graphql-js; Python's + # str.splitlines() would also split on characters like VT, FF and NEL. + lines = _re_newline.split(escaped_value) or [""] num_lines = len(lines) is_single_line = num_lines == 1 diff --git a/src/graphql/language/source.py b/src/graphql/language/source.py index 8d05c9331..87bc31628 100644 --- a/src/graphql/language/source.py +++ b/src/graphql/language/source.py @@ -2,12 +2,15 @@ from __future__ import annotations +import re from typing import Any, TypeGuard from .location import SourceLocation __all__ = ["Source", "is_source"] +_re_newline = re.compile(r"\r\n|[\n\r]") + DEFAULT_NAME = "GraphQL request" DEFAULT_SOURCE_LOCATION = SourceLocation(1, 1) @@ -87,13 +90,14 @@ def get_location(self, position: int) -> SourceLocation: >>> Source('type Query {\n hello: String\n}').get_location(15) SourceLocation(line=2, column=3) """ - lines = self.body[:position].splitlines() - if lines: - line = len(lines) - column = len(lines[-1]) + 1 - else: - line = 1 - column = 1 + # Split only on GraphQL line terminators (LF, CR and CRLF), which is + # how graphql-js and print_source_location() count lines. Python's + # str.splitlines() also splits on characters like VT, FF, NEL and + # U+2028/U+2029, and drops a trailing empty line, which produced both + # wrong locations and crashes when printing error locations. + lines = _re_newline.split(self.body[:position]) + line = len(lines) + column = len(lines[-1]) + 1 return SourceLocation(line, column) def __repr__(self) -> str: diff --git a/tests/error/test_print_location.py b/tests/error/test_print_location.py index 46b3497dc..e175d4cd9 100644 --- a/tests/error/test_print_location.py +++ b/tests/error/test_print_location.py @@ -79,3 +79,17 @@ def prints_line_numbers_with_correct_padding(): 10 | """ ) + + def prints_locations_from_get_location_on_non_graphql_line_breaks(): + # Characters like VT, FF, NEL or U+2028/U+2029 are not GraphQL line + # terminators, so Source.get_location() reports line 1 for positions + # after them. Rendering that location must not fail. + source = Source("x\vy") + result = print_source_location(source, source.get_location(3)) + assert result == dedent( + """ + GraphQL request:1:4 + 1 | x\x0by + | ^ + """ + ) diff --git a/tests/language/test_block_string.py b/tests/language/test_block_string.py index 014a2e960..f04b73ecc 100644 --- a/tests/language/test_block_string.py +++ b/tests/language/test_block_string.py @@ -216,3 +216,9 @@ def __str__(self) -> str: return "lazy" _assert_block_string(cast("str", LazyString()), '"""lazy"""') + + def treats_non_graphql_line_breaks_as_single_line(): + # VT, FF, NEL and U+2028/U+2029 are not GraphQL line terminators, so + # values containing them print as a single line (graphql-js parity). + for char in "\v\f\x85\u2028\u2029": + _assert_block_string(f"a{char}b", f'"""a{char}b"""') diff --git a/tests/language/test_source.py b/tests/language/test_source.py index 0b0a67a23..7504dc7bc 100644 --- a/tests/language/test_source.py +++ b/tests/language/test_source.py @@ -46,6 +46,18 @@ def can_get_location(): assert isinstance(location, SourceLocation) assert location == (2, 6) + def counts_only_graphql_line_terminators_for_locations(): + # Positions right after a line terminator are on the next line. + assert Source("a\nb").get_location(2) == (2, 1) + assert Source("a\rb").get_location(2) == (2, 1) + assert Source("a\r\nb").get_location(3) == (2, 1) + # Characters that splitlines() counts as line boundaries, but which + # are not GraphQL line terminators, stay on the same line (graphql-js + # parity). + for char in "\v\f\x1c\x1d\x1e\x85\u2028\u2029": + source = Source(f"x{char}y") + assert source.get_location(3) == (1, 4) + def can_be_stringified(): source = Source("") assert str(source) == ""