-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Use overload
to type get_global_option
#4978
Conversation
Pull Request Test Coverage Report for Build 1212335532
π - Coveralls |
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 like this a lot π
I also though of overload
. That was the reason I wanted a separate PR for it π
Furthermore the change is quite concise which makes it really easy to review.
Co-authored-by: Marc Mueller <30130371+cdce8p@users.noreply.github.com>
Not really sure about the naming of the type alias and |
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.
The TypeVar
seems to work. It will prevent things like this where the return type doesn't match the default
:
@overload
def get_global_option(
checker: "BaseChecker", option: GLOBAL_OPTION_BOOL, default: Optional[bool] = None
) -> str:
...
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.
Looks good! Thanks @DanielNoord π
Type of Changes
Description
The next step in splitting #4954 in more manageable PR's.
I opted to go for
overload
as it allows us to immediately tell whatget_global_option
will return or throw an error if it is unspecified.The globals added to
pylint/utils/utils.py
could go inpylint/typing
but since they won't be used outside of this file I thought it would be best to add them here.Sorry for making a second PR. The fix turned out to be much easier than anticipated.