Skip to content

gh-156047: Rank tokens before probing for a keyword typo #156087

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
johnslavik wants to merge 2 commits into
base: main
Choose a base branch
from
+26 −18
Open

gh-156047: Rank tokens before probing for a keyword typo #156087

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
b9c5baa
Rank tokens before probing for a keyword typo
johnslavik Aug 20, 2026
fdcc3a0
Check token string first, then do the line math
johnslavik Aug 20, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Lib/test/test_traceback.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1827,6 +1827,7 @@ class TestKeywordTypoSuggestions(unittest.TestCase):
("function f():", "def"),
("func f():", "def"),
("void f():", "def"),
(f"{"a="*10}0;tpye x = int;{"z="*10}1", "type"),
]

def test_keyword_suggestions_from_file(self):
Expand Down
41 changes: 23 additions & 18 deletions Lib/traceback.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import collections.abc
import functools
import heapq
Comment thread
johnslavik marked this conversation as resolved.
import itertools
import linecache
import os
Expand All @@ -19,6 +20,7 @@

from contextlib import suppress
lazy import _colorize
lazy import difflib

try:
from _missing_stdlib_info import _MISSING_STDLIB_MODULE_MESSAGES
Expand Down Expand Up @@ -1427,20 +1429,17 @@ def _find_keyword_typos(self):
if not self._exc_metadata:
return

line, offset, source = self._exc_metadata
line, _, source = self._exc_metadata
end_line = int(self.lineno) if self.lineno is not None else 0
lines = None
from_filename = False

if source is None:
if self.filename:
try:
with open(self.filename) as f:
lines = f.read().splitlines()
except Exception:
line, end_line, offset = 0,1,0
else:
from_filename = True
line, end_line, _ = 0,1,0
lines = lines if lines is not None else self.text.splitlines()
else:
lines = source.splitlines()
Expand All @@ -1462,26 +1461,32 @@ def _find_keyword_typos(self):
return # Original code compiles or is incomplete - can't validate fixes

error_lines = error_code.splitlines()
tokens = tokenize.generate_tokens(io.StringIO(error_code).readline)
tokens = []
offset = self.end_offset
try:
for token in tokenize.generate_tokens(io.StringIO(error_code).readline):
if token.type != tokenize.NAME:
continue
if keyword.iskeyword(token.string):
continue
# Only consider NAME tokens on the same line as the error
the_end = end_line if line == 0 else end_line + 1
if token.start[0] + line != the_end:
continue
Comment on lines +1472 to +1475

@m-aciek m-aciek Aug 20, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Restricting ourselves to only tokens on the same line makes us ignore valid suggestions:

iff \
x:
    pass

The parser reports line 2, but the typo is on line 1.

  • main: Did you mean 'if'?
  • PR: no suggestion

Candidates on the error line should be prioritized, not made the exclusive candidate set.

@johnslavik johnslavik Aug 20, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And we should cover this case as well.

rank = abs(offset - token.end[1])

@pablogsal pablogsal Aug 20, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the rank anchor is a bit off here: self.end_offset is 1-based and relative to the original line, but token.end[1] is 0-based and relative to the dedented snippet, so for indented code the anchor drifts right by the dedent width and the neighbours get probed before the typo (you can see it in your traces, where x ranks above iff). Not incorrect because every candidate is validated by recompiling, but can we translate end_offset to the snippet coordinates before ranking?

@johnslavik johnslavik Aug 20, 2026 •
edited
Loading

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll also add a case where dedent() makes a difference. For the TYPO_CASES we had, dedent() was a no-op so this was harder to spot.

@pablogsal pablogsal Aug 20, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can prefer the token at the parser caret over the actual typo. retrun a + b now suggests and, because replacing a makes the expression compile. We need a ranking that keeps retrun first and a test for this case.

@johnslavik johnslavik Aug 20, 2026 •
edited
Loading

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Somewhat related fun fact, you can trigger this in main too:

def outer():
    if True:
        pass
    retrun a+b

The dedented block includes only the body of the function without the def, so retrun->return won't compile since it is "outside function".

@johnslavik johnslavik Aug 20, 2026 •
edited
Loading

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know I'm getting off-topic but in fact, any functions which have a body with first block being correct syntax and return / yield / await anywhere in that body, won't have any keyword typo suggestions at all, and it's a pre-existing bug. Probably not worth fixing now, since it would either require creating a virtual async function block and another layer of offset translation or adding new flags to compile() to allow all function-only keywords be top-level (the second is a much cleaner fix)... Nevermind, we can just strip lines more intelligently when we create error_code!

heapq.heappush(tokens, (rank, token))
except Exception:
pass
Comment on lines +1478 to +1479

@m-aciek m-aciek Aug 20, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
except Exception:
pass
except tokenize.TokenError:
# Incomplete input can still contain useful tokens.
pass

(non-blocking) TokenError handling is safer than catch-all because it continues only for the incomplete-input condition where partial tokens are known to be useful.

There's a catch-all in the caller already

cpython/Lib/traceback.py

Line 1561 in 5a0ab76

with suppress(Exception):

@johnslavik johnslavik Aug 20, 2026 •
edited
Loading

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In short: because we unroll the tokens early and process them later in potentially different order, it's best to catch-all on the unrolling part and then always move on to processing.

Details

The idea here is to avoid bailing out if an iteration of the loop fails for any reason, and still process any tokens gathered up until that failure.

While TokenError might be the only possible exception reachable here right now, the invariant is broader than that. The intent is to avoid bailing in case of any exception raised while iterating in this loop, since there can still be tokens to test prior to failure.

To illustrate this: if we kept the status quo, whatever exception during iterating wouldn't stop tokens from being processed, and we want to preserve this property here regardless of the current implementation details. While we may argue that this code will never raise anything else than TokenError ever, broader guard reflects the intent clearer, and is immune to drifts in the implementation details (i.e. from some point in the future something other than TokenError might be raised).

There's a catch-all in the caller already

cpython/Lib/traceback.py

Line 1561 in 5a0ab76

with suppress(Exception):

In this PR, the catch-all in the caller becomes irrelevant to avoiding bailing out specifically before the tokens are processed. The purpose is to ignore whatever exception and still process any tokens that we managed to collect prior to failure.

tokens_left_to_process = 10
import difflib
for token in tokens:
start, end = token.start, token.end
if token.type != tokenize.NAME:
continue
# Only consider NAME tokens on the same line as the error
the_end = end_line if line == 0 else end_line + 1
if from_filename and token.start[0]+line != the_end:
continue
while tokens:
rank, token = heapq.heappop(tokens)
wrong_name = token.string
if wrong_name in keyword.kwlist:
continue

# Limit the number of valid tokens to consider to not spend
# to much time in this function
tokens_left_to_process -= 1
if tokens_left_to_process < 0:
break
start, end = token.start, token.end
# Limit the number of possible matches to try
max_matches = 3
matches = []
Expand Down
2 changes: 2 additions & 0 deletions Misc/NEWS.d/next/Library/2026-08-19-13-07-18.gh-issue-156047.4l4U9N.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
When looking for possibly misspelled Python keywords after a :exc:`SyntaxError`,
candidate names are now ranked to improve accuracy. Patch by Bartosz Sławecki.
Loading