-
Notifications
You must be signed in to change notification settings - Fork 23
Y068: Don't use @override #528
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
Changes from 6 commits
611cdbe
d634223
2fd34f0
8ad7519
1e68e2e
89c0287
cec1170
1e0c7d1
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 |
|---|---|---|
| @@ -1,5 +1,8 @@ | ||
| # Change Log | ||
|
|
||
| New error codes: | ||
| * Introduce Y068: Don't use `@override` in stub files. | ||
|
|
||
| ## 5.5.0 | ||
|
|
||
| New error codes: | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -226,6 +226,7 @@ def _is_object(node: ast.AST | None, name: str, *, from_: Container[str]) -> boo | |
| ) | ||
| _is_Generic = partial(_is_object, name="Generic", from_=_TYPING_MODULES) | ||
| _is_Unpack = partial(_is_object, name="Unpack", from_=_TYPING_MODULES) | ||
| _is_override = partial(_is_object, name="override", from_=_TYPING_MODULES) | ||
|
|
||
|
|
||
| def _is_union(node: ast.expr | None) -> TypeIs[ast.BinOp]: | ||
|
|
@@ -2032,6 +2033,12 @@ def check_protocol_param_kinds( | |
| pos_or_kw, errors.Y091.format(arg=pos_or_kw.arg, method=node.name) | ||
| ) | ||
|
|
||
| def check_for_override(self, node: ast.FunctionDef | ast.AsyncFunctionDef) -> None: | ||
| for deco in node.decorator_list: | ||
| if _is_override(deco): | ||
| self.error(deco, errors.Y068) | ||
| return | ||
|
Comment on lines
+2036
to
+2040
Collaborator
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. or we could just ban it from being imported by adding a branch to
Member
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. I thought about it, but I like this direct approach a bit more: While it may spam a bit in case someone uses it, it's more "direct". "Here's your error", instead of "You're importing something here that's going to be a problem later." It still allows overriding the warning on a case-by-cases basis (although I'm not sure why you would need that). No strong opinion, though.
Collaborator
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 feel like emitting the diagnostic at the location of the import (or attribute access) is more in keeping with our other rules such as Y024 and Y057... but I also don't have a strong opinion :-)
Member
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. Ok, then let's laziness win and not change it? |
||
|
|
||
| @staticmethod | ||
| def _is_positional_pre_570_argname(name: str) -> bool: | ||
| # https://peps.python.org/pep-0484/#positional-only-arguments | ||
|
|
@@ -2094,6 +2101,7 @@ def _visit_function(self, node: ast.FunctionDef | ast.AsyncFunctionDef) -> None: | |
| self.check_self_typevars(node, decorator_names) | ||
| if self.enclosing_class_ctx.is_protocol_class: | ||
| self.check_protocol_param_kinds(node, decorator_names) | ||
| self.check_for_override(node) | ||
|
|
||
| def visit_arg(self, node: ast.arg) -> None: | ||
| if _is_NoReturn(node.annotation): | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,24 @@ | ||
| import typing | ||
| import typing as t | ||
| from typing import override, override as over | ||
|
|
||
| import typing_extensions | ||
|
|
||
| class Foo: | ||
| def f(self) -> None: ... | ||
| def g(self) -> None: ... | ||
| def h(self) -> None: ... | ||
| def j(self) -> None: ... | ||
| def k(self) -> None: ... | ||
|
|
||
| class Bar(Foo): | ||
| @override # Y068 Do not use "@override" in stub files. | ||
| def f(self) -> None: ... | ||
| @typing.override # Y068 Do not use "@override" in stub files. | ||
| def g(self) -> None: ... | ||
| @t.override # Ideally we'd catch this too, but the infrastructure is not in place. | ||
| def h(self) -> None: ... | ||
| @over # Ideally we'd catch this too, but the infrastructure is not in place. | ||
| def j(self) -> None: ... | ||
| @typing_extensions.override # Y068 Do not use "@override" in stub files. | ||
| def k(self) -> None: ... |
Uh oh!
There was an error while loading. Please reload this page.