-
-
Notifications
You must be signed in to change notification settings - Fork 34.5k
gh-146406: Add cross-language method suggestions for builtin AttributeError #146407
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.
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
base: main
Are you sure you want to change the base?
Changes from 12 commits
7adad8b
aecaacb
6d58cdc
8a39e32
196dbe4
99e106a
57a4d39
2a2c0d5
f702e79
3ad54c8
0089761
e2c12ec
564ca3a
e1eb6f4
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1147,12 +1147,20 @@ def __init__(self, exc_type, exc_value, exc_traceback, *, limit=None, | |
| elif exc_type and issubclass(exc_type, AttributeError) and \ | ||
| getattr(exc_value, "name", None) is not None: | ||
| wrong_name = getattr(exc_value, "name", None) | ||
| suggestion = _compute_suggestion_error(exc_value, exc_traceback, wrong_name) | ||
| if suggestion: | ||
| if suggestion.isascii(): | ||
| self._str += f". Did you mean '.{suggestion}' instead of '.{wrong_name}'?" | ||
| else: | ||
| self._str += f". Did you mean '.{suggestion}' ({suggestion!a}) instead of '.{wrong_name}' ({wrong_name!a})?" | ||
| # Check cross-language/wrong-type hints first (more specific), | ||
| # then fall back to Levenshtein distance suggestions. | ||
| hint = None | ||
| if hasattr(exc_value, 'obj'): | ||
| hint = _get_cross_language_hint(exc_value.obj, wrong_name) | ||
| if hint: | ||
| self._str += f". {hint}" | ||
| else: | ||
| suggestion = _compute_suggestion_error(exc_value, exc_traceback, wrong_name) | ||
| if suggestion: | ||
| if suggestion.isascii(): | ||
| self._str += f". Did you mean '.{suggestion}' instead of '.{wrong_name}'?" | ||
| else: | ||
| self._str += f". Did you mean '.{suggestion}' ({suggestion!a}) instead of '.{wrong_name}' ({wrong_name!a})?" | ||
| elif exc_type and issubclass(exc_type, NameError) and \ | ||
| getattr(exc_value, "name", None) is not None: | ||
| wrong_name = getattr(exc_value, "name", None) | ||
|
|
@@ -1649,6 +1657,72 @@ def print(self, *, file=None, chain=True, **kwargs): | |
| _MOVE_COST = 2 | ||
| _CASE_COST = 1 | ||
|
|
||
| # Cross-language method suggestions for builtin types. | ||
| # Consulted as a fallback when Levenshtein-based suggestions find no match. | ||
| # | ||
| # Inclusion criteria: | ||
| # | ||
| # 1. Must have evidence of real cross-language confusion (Stack Overflow | ||
|
vstinner marked this conversation as resolved.
|
||
| # traffic, bug reports in production repos, developer survey data). | ||
| # 2. Must not be catchable by Levenshtein distance (too different from | ||
| # the correct Python method name). | ||
| # | ||
| # Each entry maps a wrong method name to a list of (type, suggestion, is_raw) | ||
| # tuples. The lookup checks isinstance() so subclasses are also matched. | ||
| # If is_raw is False, the suggestion is wrapped in "Did you mean '.X'?". | ||
| # If is_raw is True, the suggestion is rendered as-is. | ||
| # | ||
| # See https://github.com/python/cpython/issues/146406. | ||
| _CROSS_LANGUAGE_HINTS = { | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You might use a
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Applied in f702e79 - wrapped in
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You don't need
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nice - frozendict is cleaner than the MappingProxyType wrapper. Thanks for applying it.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It seems like the frozendict was lost in the meanwhile. |
||
| # list -- JavaScript/Ruby equivalents | ||
| "push": [(list, "append", False)], | ||
| "concat": [(list, "extend", False)], | ||
| # list -- Java/C# equivalents | ||
| "addAll": [(list, "extend", False)], | ||
| "contains": [(list, "Use 'x in list'.", True)], | ||
| # list -- wrong-type suggestion (user expected a set) | ||
| "add": [(list, "Did you mean to use a 'set' object?", True), | ||
| (frozenset, "Did you mean to use a 'set' object?", True)], | ||
| # str -- JavaScript equivalents | ||
| "toUpperCase": [(str, "upper", False)], | ||
| "toLowerCase": [(str, "lower", False)], | ||
| "trimStart": [(str, "lstrip", False)], | ||
| "trimEnd": [(str, "rstrip", False)], | ||
| # dict -- Java/JavaScript equivalents | ||
| "keySet": [(dict, "keys", False)], | ||
| "entrySet": [(dict, "items", False)], | ||
| "entries": [(dict, "items", False)], | ||
| "putAll": [(dict, "update", False)], | ||
| "put": [(dict, "Use d[k] = v.", True)], | ||
| # tuple -- mutable method on immutable type (user expected a list) | ||
| "append": [(tuple, "Did you mean to use a 'list' object?", True)], | ||
| "extend": [(tuple, "Did you mean to use a 'list' object?", True)], | ||
| "insert": [(tuple, "Did you mean to use a 'list' object?", True)], | ||
| "remove": [(tuple, "Did you mean to use a 'list' object?", True), | ||
| (frozenset, "Did you mean to use a 'set' object?", True)], | ||
| # frozenset -- mutable method on immutable type (user expected a set) | ||
| "discard": [(frozenset, "Did you mean to use a 'set' object?", True)], | ||
| # frozendict -- mutable method on immutable type (user expected a dict) | ||
| "update": [(frozenset, "Did you mean to use a 'set' object?", True), | ||
| (frozendict, "Did you mean to use a 'dict' object?", True)], | ||
| # float -- bitwise operators belong to int | ||
| "__or__": [(float, "Did you mean to use an 'int' object? Bitwise operators are not supported by 'float'.", True)], | ||
| "__and__": [(float, "Did you mean to use an 'int' object? Bitwise operators are not supported by 'float'.", True)], | ||
| "__xor__": [(float, "Did you mean to use an 'int' object? Bitwise operators are not supported by 'float'.", True)], | ||
| "__lshift__": [(float, "Did you mean to use an 'int' object? Bitwise operators are not supported by 'float'.", True)], | ||
| "__rshift__": [(float, "Did you mean to use an 'int' object? Bitwise operators are not supported by 'float'.", True)], | ||
| # NoneType -- common methods tried on None (got None instead of expected type) | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not convinced that suggestions on None are useful, but @serhiy-storchaka suggested them. For me, they sound a little bit too generic. |
||
| "keys": [(type(None), "Did you expect a 'dict'?", True)], | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You can replace |
||
| "values": [(type(None), "Did you expect a 'dict'?", True)], | ||
| "items": [(type(None), "Did you expect a 'dict'?", True)], | ||
| "upper": [(type(None), "Did you expect a 'str'?", True)], | ||
| "lower": [(type(None), "Did you expect a 'str'?", True)], | ||
| "strip": [(type(None), "Did you expect a 'str'?", True)], | ||
| "split": [(type(None), "Did you expect a 'str'?", True)], | ||
| "sort": [(type(None), "Did you expect a 'list'?", True)], | ||
| "pop": [(type(None), "Did you expect a 'list' or 'dict'?", True)], | ||
| } | ||
|
|
||
|
|
||
| def _substitution_cost(ch_a, ch_b): | ||
| if ch_a == ch_b: | ||
|
|
@@ -1711,6 +1785,24 @@ def _check_for_nested_attribute(obj, wrong_name, attrs): | |
| return None | ||
|
|
||
|
|
||
| def _get_cross_language_hint(obj, wrong_name): | ||
| """Check if wrong_name is a common method name from another language, | ||
| a mutable method on an immutable type, or a method tried on None. | ||
|
|
||
| Uses isinstance() so subclasses of builtin types also get hints. | ||
| Returns a formatted hint string, or None. | ||
| """ | ||
| entries = _CROSS_LANGUAGE_HINTS.get(wrong_name) | ||
| if entries is None: | ||
| return None | ||
| for check_type, hint, is_raw in entries: | ||
| if isinstance(obj, check_type): | ||
| if is_raw: | ||
| return hint | ||
| return f"Did you mean '.{hint}'?" | ||
| return None | ||
|
|
||
|
|
||
| def _get_safe___dir__(obj): | ||
| # Use obj.__dir__() to avoid a TypeError when calling dir(obj). | ||
| # See gh-131001 and gh-139933. | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,6 @@ | ||
| Cross-language method suggestions are now shown for :exc:`AttributeError` on | ||
| builtin types and their subclasses. | ||
| For example, ``[].push()`` suggests ``append``, | ||
| ``(1,2).append(3)`` suggests using a ``list``, | ||
| ``None.keys()`` suggests expecting a ``dict``, | ||
| and ``1.0.__or__`` suggests using an ``int``. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't understand the None suggestions. Yes, None has no keys() method, but I don't see how I'm supposed to replace it with a dict in this example.
I expected an example where a variable is set to None by mistake, such as:
cc @serhiy-storchaka