Skip to content
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

chore: improve error message for bytestring length check #3313

Merged
merged 5 commits into from
May 5, 2023

Conversation

tserg
Copy link
Collaborator

@tserg tserg commented Mar 7, 2023

What I did

The following invalid syntax runs into an error when raising an exception - (and similarly for Bytes[N]):

@external
def foo():
    a: String = "abc"
TypeError: _BytestringT.compare_type() missing 1 required positional argument: 'other'

This should have been caught here:

def from_annotation(cls, node: vy_ast.VyperNode) -> "_BytestringT":
if not isinstance(node, vy_ast.Subscript) or not isinstance(node.slice, vy_ast.Index):
raise StructureException(
f"Cannot declare {cls._id} type without a maximum length, e.g. {cls._id}[5]", node
)

Additionally, the error message for DynArray and HashMap can be improved:

@external
def foo():
    a: DynArray = [1, 2, 3]
vyper.exceptions.InvalidType: Expected <class 'vyper.semantics.types.subscriptable.DArrayT'> but literal can only be cast as DynArray[int104, 3] or uint96[3].

This compiles when it should raise:

b: HashMap

How I did it

Call from_annotation for its side effects in type_from_annotation if the attribute exists for the retrieved type.

How to verify it

See tests.

Commit message

chore: improve error message for bytestring length check

Description for the changelog

Improve error message for bytestring length check

Cute Animal Picture

Put a link to a cute animal picture inside the parenthesis-->

@codecov-commenter
Copy link

codecov-commenter commented Mar 7, 2023

Codecov Report

Merging #3313 (a9b0176) into master (a5995a9) will decrease coverage by 0.17%.
The diff coverage is 100.00%.

📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

@@            Coverage Diff             @@
##           master    #3313      +/-   ##
==========================================
- Coverage   88.93%   88.76%   -0.17%     
==========================================
  Files          84       85       +1     
  Lines       10606    10711     +105     
  Branches     2216     2234      +18     
==========================================
+ Hits         9432     9508      +76     
- Misses        768      802      +34     
+ Partials      406      401       -5     
Impacted Files Coverage Δ
vyper/semantics/types/utils.py 95.65% <100.00%> (+0.19%) ⬆️

... and 41 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

Comment on lines -5 to -21
valid_list = [
"""
@external
def foo() -> String[10]:
return "badminton"
""",
"""
@external
def foo():
x: String[11] = "¡très bien!"
""",
"""
@external
def test() -> String[100]:
return "hello world!"
""",
]
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Duplicated tests:

valid_list = [
"""
@external
def foo() -> String[10]:
return "badminton"
""",
"""
@external
def foo():
x: String[11] = "¡très bien!"
""",

I moved the last example to this file.

@tserg tserg marked this pull request as ready for review March 8, 2023 01:33
@charles-cooper charles-cooper enabled auto-merge (squash) May 5, 2023 20:21
@charles-cooper charles-cooper merged commit 9cc56b6 into vyperlang:master May 5, 2023
@tserg tserg deleted the fix/bytestring_length_check branch May 6, 2023 03:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants