-
-
Notifications
You must be signed in to change notification settings - Fork 795
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
feat[lang]!: make @external
modifier optional in .vyi
files
#4178
feat[lang]!: make @external
modifier optional in .vyi
files
#4178
Conversation
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.
thanks!
@external
modifier optional in .vyi
files
d for d in funcdef.decorator_list if d.id in FunctionVisibility.values() | ||
) | ||
raise FunctionDeclarationException( | ||
"Interface functions can only be marked as `@external`", nonexternal |
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 think the error message can be confusing since it could imply that payable
is disallowed (which is not). @cyberthirst how about:
Interface functions' visibility can only be marked as `@external`
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.
hmm, we've improved the reporting a bit, so the message refers to the corresponding source construct
@internal
@payable
def bar():
...
would yield:
vyper.exceptions.FunctionDeclarationException: Interface functions can only be marked as `@external`
contract "tests/custom/i.vyi:1", function "bar", line 1:1
---> 1 @internal
--------^
2 @payable
would you say that it's still confusing?
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 personally think error messages should be as precise as possible and thus I still think we should mention somehow that it's a visibility decorator.
What I did
@external
visibility invyi
files optionalHow to verify it
Commit Message
edit: linked wrong issue