gh-151128: Improve SyntaxError message for cross language keywords#151129
gh-151128: Improve SyntaxError message for cross language keywords#151129zang-langyan wants to merge 7 commits into
Conversation
zang-langyan
commented
Jun 9, 2026
zang-langyan
commented
|
I do not think that it is needed to add 'switch' to keywords. Even if we only add one special case, it is worth to write more general code which will allow to add more keywords in future. See #146407 as example. |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Thank you for the reference, it is really a good example. I will look into it. |
zang-langyan
changed the title
| hint = _CROSS_LANGUAGE_KEYWORD_HINTS.get(wrong_name) | ||
| return hint |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
serhiy-storchaka
left a comment
serhiy-storchaka
left a comment
There was a problem hiding this comment.
You can split this on two features -- for keywords and for operators.
Anyway, it needs a NEWS entry, a What's New entry, and tests.
| | a=expression ':=' expression { | ||
| RAISE_SYNTAX_ERROR_KNOWN_LOCATION( | ||
| a, "cannot use assignment expressions with %s", _PyPegen_get_expr_name(a)) } | ||
| | a=expression '&''&' b=expression { |
There was a problem hiding this comment.
Nice. This is actually a different feature. You can even open a separate issue and PR for this (if there is no already open). Anyway, this is a nice improvement.
I suggest to add also cases for <> ("not equal" in Pascal and Python 2), "=<", "=>" and "=!" (typo in "<=", ">=" and "!="), "===" ("is" in JavaScript and PHP).
There was a problem hiding this comment.
Actually there is already a PR for improving SyntaxError message of && and ||: #150906
There was a problem hiding this comment.
Thank you for review, great suggestion, I will add those~
There was a problem hiding this comment.
Then let's focus on keywords in this PR.
| suggestion = _suggestions._generate_suggestions(keyword.kwlist + keyword.softkwlist, wrong_name) | ||
| if suggestion: | ||
| matches.append(suggestion) | ||
| matches.extend(difflib.get_close_matches(wrong_name, keyword.kwlist, n=max_matches, cutoff=0.5)) |
There was a problem hiding this comment.
| matches.extend(difflib.get_close_matches(wrong_name, keyword.kwlist, n=max_matches, cutoff=0.5)) | |
| matches.extend(difflib.get_close_matches(wrong_name, keyword.kwlist+keyword.softkwlist, n=max_matches, cutoff=0.5)) |
Maybe we should add softkwlist here too?
serhiy-storchaka
left a comment
serhiy-storchaka
left a comment
There was a problem hiding this comment.
Could you please add also tests for soft keywords? And document this change in the NEWS entry.
Why aliases for None were removed? Were there issues with them?
Sure, I will add the tests for soft keywords. I removed the aliases for None because I realized that names like Null, nil, etc. don't work well for keyword syntax detection. I can't think of a situation where these tokens would be correctly identified as keywords. Any suggestions? |
|
You are right. Maybe they can be handled where NameError is handed. |
| ("typ x = int", "type"), | ||
| ("typed x = int", "type"), |
There was a problem hiding this comment.
Two tests for match and type, no tests for lazy.
| 'delete': 'del', | ||
| # function define equivalents | ||
| 'function': 'def', | ||
| 'func': 'def', |
There was a problem hiding this comment.
| 'func': 'def', | |
| 'func': 'def', | |
| 'void': 'def' |
Long shot. But maybe?
| max_matches = 3 | ||
| matches = [] | ||
|
|
||
| hint = _get_cross_language_keyword_hint(wrong_name) |
There was a problem hiding this comment.
| hint = _get_cross_language_keyword_hint(wrong_name) | |
| hint = _CROSS_LANGUAGE_KEYWORD_HINTS.get(wrong_name) |
If it is only used once, and is marked as a "private" method, I would suggest calling the body directly
4 participants
