From 4170241c402675d7c447de7a01beb76dd4a07ede Mon Sep 17 00:00:00 2001 From: Charles Cooper Date: Wed, 14 Feb 2024 00:56:59 +0000 Subject: [PATCH 1/5] fix: levenshtein performance defer the call to `levenshtein` until we need an error message --- vyper/exceptions.py | 7 +++++-- vyper/semantics/analysis/levenshtein_utils.py | 6 ++++-- vyper/semantics/analysis/utils.py | 4 ++-- vyper/semantics/namespace.py | 4 ++-- vyper/semantics/types/base.py | 4 ++-- vyper/semantics/types/user.py | 4 ++-- vyper/semantics/types/utils.py | 5 +++-- 7 files changed, 20 insertions(+), 14 deletions(-) diff --git a/vyper/exceptions.py b/vyper/exceptions.py index 53ad6f7bb8..903fb32c30 100644 --- a/vyper/exceptions.py +++ b/vyper/exceptions.py @@ -82,8 +82,11 @@ def with_annotation(self, *annotations): @property def message(self): msg = self._message - if self._hint: - msg += f"\n\n (hint: {self._hint})" + hint = self._hint + if callable(hint): + hint = hint() + if hint: + msg += f"\n\n (hint: {hint})" return msg def __str__(self): diff --git a/vyper/semantics/analysis/levenshtein_utils.py b/vyper/semantics/analysis/levenshtein_utils.py index 1d8f87dfbd..fbdcf36cc1 100644 --- a/vyper/semantics/analysis/levenshtein_utils.py +++ b/vyper/semantics/analysis/levenshtein_utils.py @@ -1,4 +1,4 @@ -from typing import Any, Dict +from typing import Any, Callable def levenshtein_norm(source: str, target: str) -> float: @@ -73,7 +73,9 @@ def levenshtein(source: str, target: str) -> int: return matrix[len(source)][len(target)] -def get_levenshtein_error_suggestions(key: str, namespace: Dict[str, Any], threshold: float) -> str: +def get_levenshtein_error_suggestions(*args, **kwargs) -> Callable: + return lambda: _get_levenshtein_error_suggestions(*args, **kwargs) +def _get_levenshtein_error_suggestions(key: str, namespace: dict[str, Any], threshold: float) -> str: """ Generate an error message snippet for the suggested closest values in the provided namespace with the shortest normalized Levenshtein distance from the given key if that distance diff --git a/vyper/semantics/analysis/utils.py b/vyper/semantics/analysis/utils.py index 034cd8c46e..fa4dfcc1d1 100644 --- a/vyper/semantics/analysis/utils.py +++ b/vyper/semantics/analysis/utils.py @@ -208,9 +208,9 @@ def _raise_invalid_reference(name, node): if name in self.namespace: _raise_invalid_reference(name, node) - suggestions_str = get_levenshtein_error_suggestions(name, t.members, 0.4) + hint = get_levenshtein_error_suggestions(name, t.members, 0.4) raise UndeclaredDefinition( - f"Storage variable '{name}' has not been declared. {suggestions_str}", node + f"Storage variable '{name}' has not been declared.", node, hint=hint ) from None def types_from_BinOp(self, node): diff --git a/vyper/semantics/namespace.py b/vyper/semantics/namespace.py index 4df2511a29..d59343edfb 100644 --- a/vyper/semantics/namespace.py +++ b/vyper/semantics/namespace.py @@ -45,8 +45,8 @@ def __setitem__(self, attr, obj): def __getitem__(self, key): if key not in self: - suggestions_str = get_levenshtein_error_suggestions(key, self, 0.2) - raise UndeclaredDefinition(f"'{key}' has not been declared. {suggestions_str}") + hint = get_levenshtein_error_suggestions(key, self, 0.2) + raise UndeclaredDefinition(f"'{key}' has not been declared.", hint=hint) return super().__getitem__(key) def __enter__(self): diff --git a/vyper/semantics/types/base.py b/vyper/semantics/types/base.py index c5e10b52be..37de263319 100644 --- a/vyper/semantics/types/base.py +++ b/vyper/semantics/types/base.py @@ -321,8 +321,8 @@ def get_member(self, key: str, node: vy_ast.VyperNode) -> "VyperType": if not self.members: raise StructureException(f"{self} instance does not have members", node) - suggestions_str = get_levenshtein_error_suggestions(key, self.members, 0.3) - raise UnknownAttribute(f"{self} has no member '{key}'. {suggestions_str}", node) + hint = get_levenshtein_error_suggestions(key, self.members, 0.3) + raise UnknownAttribute(f"{self} has no member '{key}'.", node, hint=hint) def __repr__(self): return self._id diff --git a/vyper/semantics/types/user.py b/vyper/semantics/types/user.py index 92a455e3d8..6d6db5142d 100644 --- a/vyper/semantics/types/user.py +++ b/vyper/semantics/types/user.py @@ -399,9 +399,9 @@ def _ctor_call_return(self, node: vy_ast.Call) -> "StructT": keys = list(self.member_types.keys()) for i, (key, value) in enumerate(zip(node.args[0].keys, node.args[0].values)): if key is None or key.get("id") not in members: - suggestions_str = get_levenshtein_error_suggestions(key.get("id"), members, 1.0) + hint = get_levenshtein_error_suggestions(key.get("id"), members, 1.0) raise UnknownAttribute( - f"Unknown or duplicate struct member. {suggestions_str}", key or value + f"Unknown or duplicate struct member.", key or value, hint=hint ) expected_key = keys[i] if key.id != expected_key: diff --git a/vyper/semantics/types/utils.py b/vyper/semantics/types/utils.py index 96c661021f..d0cdf94f96 100644 --- a/vyper/semantics/types/utils.py +++ b/vyper/semantics/types/utils.py @@ -146,10 +146,11 @@ def _type_from_annotation(node: vy_ast.VyperNode) -> VyperType: raise InvalidType(err_msg, node) if node.id not in namespace: # type: ignore - suggestions_str = get_levenshtein_error_suggestions(node.node_source_code, namespace, 0.3) + hint = get_levenshtein_error_suggestions(node.node_source_code, namespace, 0.3) raise UnknownType( - f"No builtin or user-defined type named '{node.node_source_code}'. {suggestions_str}", + f"No builtin or user-defined type named '{node.node_source_code}'.", node, + hint=hint ) from None typ_ = namespace[node.id] From 23a96d2b938321c4e2b7dc44e5f0af8a80b719f5 Mon Sep 17 00:00:00 2001 From: Charles Cooper Date: Wed, 14 Feb 2024 01:00:24 +0000 Subject: [PATCH 2/5] fix lint --- vyper/semantics/analysis/levenshtein_utils.py | 6 +++++- vyper/semantics/types/user.py | 2 +- vyper/semantics/types/utils.py | 4 +--- 3 files changed, 7 insertions(+), 5 deletions(-) diff --git a/vyper/semantics/analysis/levenshtein_utils.py b/vyper/semantics/analysis/levenshtein_utils.py index fbdcf36cc1..fc6e497d43 100644 --- a/vyper/semantics/analysis/levenshtein_utils.py +++ b/vyper/semantics/analysis/levenshtein_utils.py @@ -75,7 +75,11 @@ def levenshtein(source: str, target: str) -> int: def get_levenshtein_error_suggestions(*args, **kwargs) -> Callable: return lambda: _get_levenshtein_error_suggestions(*args, **kwargs) -def _get_levenshtein_error_suggestions(key: str, namespace: dict[str, Any], threshold: float) -> str: + + +def _get_levenshtein_error_suggestions( + key: str, namespace: dict[str, Any], threshold: float +) -> str: """ Generate an error message snippet for the suggested closest values in the provided namespace with the shortest normalized Levenshtein distance from the given key if that distance diff --git a/vyper/semantics/types/user.py b/vyper/semantics/types/user.py index 6d6db5142d..0c9b5d70da 100644 --- a/vyper/semantics/types/user.py +++ b/vyper/semantics/types/user.py @@ -401,7 +401,7 @@ def _ctor_call_return(self, node: vy_ast.Call) -> "StructT": if key is None or key.get("id") not in members: hint = get_levenshtein_error_suggestions(key.get("id"), members, 1.0) raise UnknownAttribute( - f"Unknown or duplicate struct member.", key or value, hint=hint + "Unknown or duplicate struct member.", key or value, hint=hint ) expected_key = keys[i] if key.id != expected_key: diff --git a/vyper/semantics/types/utils.py b/vyper/semantics/types/utils.py index d0cdf94f96..0546668900 100644 --- a/vyper/semantics/types/utils.py +++ b/vyper/semantics/types/utils.py @@ -148,9 +148,7 @@ def _type_from_annotation(node: vy_ast.VyperNode) -> VyperType: if node.id not in namespace: # type: ignore hint = get_levenshtein_error_suggestions(node.node_source_code, namespace, 0.3) raise UnknownType( - f"No builtin or user-defined type named '{node.node_source_code}'.", - node, - hint=hint + f"No builtin or user-defined type named '{node.node_source_code}'.", node, hint=hint ) from None typ_ = namespace[node.id] From 71d20ded5bc1fc4eb1dc124d068c450ac6ae69c8 Mon Sep 17 00:00:00 2001 From: Charles Cooper Date: Tue, 13 Feb 2024 20:38:28 -0500 Subject: [PATCH 3/5] add a comment --- vyper/exceptions.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/vyper/exceptions.py b/vyper/exceptions.py index 903fb32c30..11dd0b2365 100644 --- a/vyper/exceptions.py +++ b/vyper/exceptions.py @@ -2,6 +2,7 @@ import copy import textwrap import types +from functools import cached_property from vyper.compiler.settings import VYPER_ERROR_CONTEXT_LINES, VYPER_ERROR_LINE_NUMBERS @@ -79,12 +80,17 @@ def with_annotation(self, *annotations): exc.annotations = annotations return exc - @property + @cached_property def message(self): msg = self._message hint = self._hint + + # some hints are expensive to compute, so we wait until the last + # minute when the formatted message is actually requested to compute + # them. if callable(hint): hint = hint() + if hint: msg += f"\n\n (hint: {hint})" return msg From bdd6debb6650466009c55acdc7d6aa2ae0888b80 Mon Sep 17 00:00:00 2001 From: Charles Cooper Date: Wed, 14 Feb 2024 01:58:56 +0000 Subject: [PATCH 4/5] fix tests --- .../syntax/modules/test_initializers.py | 2 +- tests/functional/syntax/test_for_range.py | 39 +++++++++++++++---- vyper/exceptions.py | 19 ++++----- 3 files changed, 43 insertions(+), 17 deletions(-) diff --git a/tests/functional/syntax/modules/test_initializers.py b/tests/functional/syntax/modules/test_initializers.py index d0965ae61d..66a201a33d 100644 --- a/tests/functional/syntax/modules/test_initializers.py +++ b/tests/functional/syntax/modules/test_initializers.py @@ -1178,4 +1178,4 @@ def test_ownership_decl_errors_not_swallowed(make_input_bundle): input_bundle = make_input_bundle({"lib1.vy": lib1}) with pytest.raises(UndeclaredDefinition) as e: compile_code(main, input_bundle=input_bundle) - assert e.value._message == "'lib2' has not been declared. " + assert e.value._message == "'lib2' has not been declared." diff --git a/tests/functional/syntax/test_for_range.py b/tests/functional/syntax/test_for_range.py index a486d11738..94eed58dd4 100644 --- a/tests/functional/syntax/test_for_range.py +++ b/tests/functional/syntax/test_for_range.py @@ -21,6 +21,7 @@ def foo(): """, StructureException, "Invalid syntax for loop iterator", + None, "a[1]", ), ( @@ -32,6 +33,7 @@ def bar(): """, StructureException, "Bound must be at least 1", + None, "0", ), ( @@ -44,6 +46,7 @@ def foo(): """, StateAccessViolation, "Bound must be a literal", + None, "x", ), ( @@ -55,6 +58,7 @@ def foo(): """, StructureException, "Please remove the `bound=` kwarg when using range with constants", + None, "5", ), ( @@ -66,6 +70,7 @@ def foo(): """, StructureException, "Bound must be at least 1", + None, "0", ), ( @@ -78,6 +83,7 @@ def bar(): """, ArgumentException, "Invalid keyword argument 'extra'", + None, "extra=3", ), ( @@ -89,6 +95,7 @@ def bar(): """, StructureException, "End must be greater than start", + None, "0", ), ( @@ -101,6 +108,7 @@ def bar(): """, StateAccessViolation, "Value must be a literal integer, unless a bound is specified", + None, "x", ), ( @@ -113,6 +121,7 @@ def bar(): """, StateAccessViolation, "Value must be a literal integer, unless a bound is specified", + None, "x", ), ( @@ -125,6 +134,7 @@ def repeat(n: uint256) -> uint256: """, StateAccessViolation, "Value must be a literal integer, unless a bound is specified", + None, "n * 10", ), ( @@ -137,6 +147,7 @@ def bar(): """, StateAccessViolation, "Value must be a literal integer, unless a bound is specified", + None, "x + 1", ), ( @@ -148,6 +159,7 @@ def bar(): """, StructureException, "End must be greater than start", + None, "1", ), ( @@ -160,6 +172,7 @@ def bar(): """, StateAccessViolation, "Value must be a literal integer, unless a bound is specified", + None, "x", ), ( @@ -172,6 +185,7 @@ def foo(): """, StateAccessViolation, "Value must be a literal integer, unless a bound is specified", + None, "x", ), ( @@ -184,6 +198,7 @@ def repeat(n: uint256) -> uint256: """, StateAccessViolation, "Value must be a literal integer, unless a bound is specified", + None, "n", ), ( @@ -196,6 +211,7 @@ def foo(x: int128): """, StateAccessViolation, "Value must be a literal integer, unless a bound is specified", + None, "x", ), ( @@ -207,6 +223,7 @@ def bar(x: uint256): """, StateAccessViolation, "Value must be a literal integer, unless a bound is specified", + None, "x", ), ( @@ -221,6 +238,7 @@ def foo(): """, TypeMismatch, "Given reference has type int128, expected uint256", + None, "FOO", ), ( @@ -234,6 +252,7 @@ def foo(): """, StructureException, "Bound must be at least 1", + None, "FOO", ), ( @@ -244,7 +263,8 @@ def foo(): pass """, UnknownType, - "No builtin or user-defined type named 'DynArra'. Did you mean 'DynArray'?", + "No builtin or user-defined type named 'DynArra'.", + "Did you mean 'DynArray'?", "DynArra", ), ( @@ -262,7 +282,8 @@ def foo(): pass """, UnknownType, - "No builtin or user-defined type named 'uint9'. Did you mean 'uint96', or maybe 'uint8'?", + "No builtin or user-defined type named 'uint9'.", + "Did you mean 'uint96', or maybe 'uint8'?", "uint9", ), ( @@ -278,7 +299,8 @@ def foo(): pass """, UnknownType, - "No builtin or user-defined type named 'uint9'. Did you mean 'uint96', or maybe 'uint8'?", + "No builtin or user-defined type named 'uint9'.", + "Did you mean 'uint96', or maybe 'uint8'?", "uint9", ), ] @@ -289,15 +311,18 @@ def foo(): f"{i:02d}: {for_code_regex.search(code).group(1)}" # type: ignore[union-attr] f" raises {type(err).__name__}" ) - for i, (code, err, msg, src) in enumerate(fail_list) + for i, (code, err, msg, hint, src) in enumerate(fail_list) ] -@pytest.mark.parametrize("bad_code,error_type,message,source_code", fail_list, ids=fail_test_names) -def test_range_fail(bad_code, error_type, message, source_code): +@pytest.mark.parametrize( + "bad_code,error_type,message,hint,source_code", fail_list, ids=fail_test_names +) +def test_range_fail(bad_code, error_type, message, hint, source_code): with pytest.raises(error_type) as exc_info: compiler.compile_code(bad_code) - assert message == exc_info.value.message + assert message == exc_info.value._message + assert hint == exc_info.value.hint assert source_code == exc_info.value.args[1].get_original_node().node_source_code diff --git a/vyper/exceptions.py b/vyper/exceptions.py index 11dd0b2365..59ce9e2800 100644 --- a/vyper/exceptions.py +++ b/vyper/exceptions.py @@ -80,19 +80,20 @@ def with_annotation(self, *annotations): exc.annotations = annotations return exc - @cached_property - def message(self): - msg = self._message - hint = self._hint - + @property + def hint(self): # some hints are expensive to compute, so we wait until the last # minute when the formatted message is actually requested to compute # them. - if callable(hint): - hint = hint() + if callable(self._hint): + return self._hint() + return self._hint - if hint: - msg += f"\n\n (hint: {hint})" + @property + def message(self): + msg = self._message + if self.hint: + msg += f"\n\n (hint: {self.hint})" return msg def __str__(self): From 4d717d05bd06661ac41f1550d60f63867c0746c9 Mon Sep 17 00:00:00 2001 From: Charles Cooper Date: Wed, 14 Feb 2024 02:01:55 +0000 Subject: [PATCH 5/5] fix lint again --- vyper/exceptions.py | 1 - 1 file changed, 1 deletion(-) diff --git a/vyper/exceptions.py b/vyper/exceptions.py index 59ce9e2800..f57cdabe9d 100644 --- a/vyper/exceptions.py +++ b/vyper/exceptions.py @@ -2,7 +2,6 @@ import copy import textwrap import types -from functools import cached_property from vyper.compiler.settings import VYPER_ERROR_CONTEXT_LINES, VYPER_ERROR_LINE_NUMBERS