mirror of
https://github.com/astral-sh/ruff.git
synced 2025-10-17 22:07:42 +00:00
[ty] Improve diagnostics for bad @overload
definitions (#20745)
Some checks are pending
CI / Determine changes (push) Waiting to run
CI / cargo fmt (push) Waiting to run
CI / cargo clippy (push) Blocked by required conditions
CI / cargo test (linux) (push) Blocked by required conditions
CI / cargo test (linux, release) (push) Blocked by required conditions
CI / cargo test (windows) (push) Blocked by required conditions
CI / cargo test (wasm) (push) Blocked by required conditions
CI / cargo build (release) (push) Waiting to run
CI / cargo build (msrv) (push) Blocked by required conditions
CI / cargo fuzz build (push) Blocked by required conditions
CI / fuzz parser (push) Blocked by required conditions
CI / test scripts (push) Blocked by required conditions
CI / ecosystem (push) Blocked by required conditions
CI / Fuzz for new ty panics (push) Blocked by required conditions
CI / cargo shear (push) Blocked by required conditions
CI / python package (push) Waiting to run
CI / pre-commit (push) Waiting to run
CI / mkdocs (push) Waiting to run
CI / formatter instabilities and black similarity (push) Blocked by required conditions
CI / test ruff-lsp (push) Blocked by required conditions
CI / check playground (push) Blocked by required conditions
CI / benchmarks instrumented (ruff) (push) Blocked by required conditions
CI / benchmarks instrumented (ty) (push) Blocked by required conditions
CI / benchmarks-walltime (push) Blocked by required conditions
[ty Playground] Release / publish (push) Waiting to run
Some checks are pending
CI / Determine changes (push) Waiting to run
CI / cargo fmt (push) Waiting to run
CI / cargo clippy (push) Blocked by required conditions
CI / cargo test (linux) (push) Blocked by required conditions
CI / cargo test (linux, release) (push) Blocked by required conditions
CI / cargo test (windows) (push) Blocked by required conditions
CI / cargo test (wasm) (push) Blocked by required conditions
CI / cargo build (release) (push) Waiting to run
CI / cargo build (msrv) (push) Blocked by required conditions
CI / cargo fuzz build (push) Blocked by required conditions
CI / fuzz parser (push) Blocked by required conditions
CI / test scripts (push) Blocked by required conditions
CI / ecosystem (push) Blocked by required conditions
CI / Fuzz for new ty panics (push) Blocked by required conditions
CI / cargo shear (push) Blocked by required conditions
CI / python package (push) Waiting to run
CI / pre-commit (push) Waiting to run
CI / mkdocs (push) Waiting to run
CI / formatter instabilities and black similarity (push) Blocked by required conditions
CI / test ruff-lsp (push) Blocked by required conditions
CI / check playground (push) Blocked by required conditions
CI / benchmarks instrumented (ruff) (push) Blocked by required conditions
CI / benchmarks instrumented (ty) (push) Blocked by required conditions
CI / benchmarks-walltime (push) Blocked by required conditions
[ty Playground] Release / publish (push) Waiting to run
This commit is contained in:
parent
1bf4969c96
commit
ff386b4797
10 changed files with 390 additions and 88 deletions
|
@ -645,13 +645,9 @@ S = TypeVar("S")
|
|||
|
||||
class WithOverloadedMethod(Generic[T]):
|
||||
@overload
|
||||
def method(self, x: T) -> T:
|
||||
return x
|
||||
|
||||
def method(self, x: T) -> T: ...
|
||||
@overload
|
||||
def method(self, x: S) -> S | T:
|
||||
return x
|
||||
|
||||
def method(self, x: S) -> S | T: ...
|
||||
def method(self, x: S | T) -> S | T:
|
||||
return x
|
||||
|
||||
|
|
|
@ -553,13 +553,9 @@ from typing import overload
|
|||
|
||||
class WithOverloadedMethod[T]:
|
||||
@overload
|
||||
def method(self, x: T) -> T:
|
||||
return x
|
||||
|
||||
def method(self, x: T) -> T: ...
|
||||
@overload
|
||||
def method[S](self, x: S) -> S | T:
|
||||
return x
|
||||
|
||||
def method[S](self, x: S) -> S | T: ...
|
||||
def method[S](self, x: S | T) -> S | T:
|
||||
return x
|
||||
|
||||
|
|
|
@ -359,14 +359,14 @@ from typing import overload
|
|||
@overload
|
||||
def func(x: int) -> int: ...
|
||||
@overload
|
||||
# error: [invalid-overload] "Overloaded non-stub function `func` must have an implementation"
|
||||
# error: [invalid-overload] "Overloads for function `func` must be followed by a non-`@overload`-decorated implementation function"
|
||||
def func(x: str) -> str: ...
|
||||
|
||||
class Foo:
|
||||
@overload
|
||||
def method(self, x: int) -> int: ...
|
||||
@overload
|
||||
# error: [invalid-overload] "Overloaded non-stub function `method` must have an implementation"
|
||||
# error: [invalid-overload] "Overloads for function `method` must be followed by a non-`@overload`-decorated implementation function"
|
||||
def method(self, x: str) -> str: ...
|
||||
```
|
||||
|
||||
|
@ -448,6 +448,55 @@ class PartialFoo(ABC):
|
|||
def f(self, x: str) -> str: ...
|
||||
```
|
||||
|
||||
### `@overload`-decorated functions with non-stub bodies
|
||||
|
||||
<!-- snapshot-diagnostics -->
|
||||
|
||||
If an `@overload`-decorated function has a non-trivial body, it likely indicates a misunderstanding
|
||||
on the part of the user. We emit a warning-level diagnostic to alert them of this.
|
||||
|
||||
`...`, `pass` and docstrings are all fine:
|
||||
|
||||
```py
|
||||
from typing import overload
|
||||
|
||||
@overload
|
||||
def x(y: int) -> int: ...
|
||||
@overload
|
||||
def x(y: str) -> str:
|
||||
"""Docstring"""
|
||||
|
||||
@overload
|
||||
def x(y: bytes) -> bytes:
|
||||
pass
|
||||
|
||||
@overload
|
||||
def x(y: memoryview) -> memoryview:
|
||||
"""More docs"""
|
||||
pass
|
||||
...
|
||||
|
||||
def x(y):
|
||||
return y
|
||||
```
|
||||
|
||||
Anything else, however, will trigger the lint:
|
||||
|
||||
```py
|
||||
@overload
|
||||
def foo(x: int) -> int:
|
||||
return x # error: [useless-overload-body]
|
||||
|
||||
@overload
|
||||
def foo(x: str) -> None:
|
||||
"""Docstring"""
|
||||
pass
|
||||
print("oh no, a string") # error: [useless-overload-body]
|
||||
|
||||
def foo(x):
|
||||
return x
|
||||
```
|
||||
|
||||
### Inconsistent decorators
|
||||
|
||||
#### `@staticmethod`
|
||||
|
|
|
@ -17,43 +17,49 @@ mdtest path: crates/ty_python_semantic/resources/mdtest/overloads.md
|
|||
3 | @overload
|
||||
4 | def func(x: int) -> int: ...
|
||||
5 | @overload
|
||||
6 | # error: [invalid-overload] "Overloaded non-stub function `func` must have an implementation"
|
||||
6 | # error: [invalid-overload] "Overloads for function `func` must be followed by a non-`@overload`-decorated implementation function"
|
||||
7 | def func(x: str) -> str: ...
|
||||
8 |
|
||||
9 | class Foo:
|
||||
10 | @overload
|
||||
11 | def method(self, x: int) -> int: ...
|
||||
12 | @overload
|
||||
13 | # error: [invalid-overload] "Overloaded non-stub function `method` must have an implementation"
|
||||
13 | # error: [invalid-overload] "Overloads for function `method` must be followed by a non-`@overload`-decorated implementation function"
|
||||
14 | def method(self, x: str) -> str: ...
|
||||
```
|
||||
|
||||
# Diagnostics
|
||||
|
||||
```
|
||||
error[invalid-overload]: Overloaded non-stub function `func` must have an implementation
|
||||
error[invalid-overload]: Overloads for function `func` must be followed by a non-`@overload`-decorated implementation function
|
||||
--> src/mdtest_snippet.py:7:5
|
||||
|
|
||||
5 | @overload
|
||||
6 | # error: [invalid-overload] "Overloaded non-stub function `func` must have an implementation"
|
||||
6 | # error: [invalid-overload] "Overloads for function `func` must be followed by a non-`@overload`-decorated implementation function"
|
||||
7 | def func(x: str) -> str: ...
|
||||
| ^^^^
|
||||
8 |
|
||||
9 | class Foo:
|
||||
|
|
||||
info: Attempting to call `func` will raise `TypeError` at runtime
|
||||
info: Overloaded functions without implementations are only permitted in stub files, on protocols, or for abstract methods
|
||||
info: See https://docs.python.org/3/library/typing.html#typing.overload for more details
|
||||
info: rule `invalid-overload` is enabled by default
|
||||
|
||||
```
|
||||
|
||||
```
|
||||
error[invalid-overload]: Overloaded non-stub function `method` must have an implementation
|
||||
error[invalid-overload]: Overloads for function `method` must be followed by a non-`@overload`-decorated implementation function
|
||||
--> src/mdtest_snippet.py:14:9
|
||||
|
|
||||
12 | @overload
|
||||
13 | # error: [invalid-overload] "Overloaded non-stub function `method` must have an implementation"
|
||||
13 | # error: [invalid-overload] "Overloads for function `method` must be followed by a non-`@overload`-decorated implementation functi…
|
||||
14 | def method(self, x: str) -> str: ...
|
||||
| ^^^^^^
|
||||
|
|
||||
info: Attempting to call `method` will raise `TypeError` at runtime
|
||||
info: Overloaded functions without implementations are only permitted in stub files, on protocols, or for abstract methods
|
||||
info: See https://docs.python.org/3/library/typing.html#typing.overload for more details
|
||||
info: rule `invalid-overload` is enabled by default
|
||||
|
||||
```
|
||||
|
|
|
@ -0,0 +1,83 @@
|
|||
---
|
||||
source: crates/ty_test/src/lib.rs
|
||||
expression: snapshot
|
||||
---
|
||||
---
|
||||
mdtest name: overloads.md - Overloads - Invalid - `@overload`-decorated functions with non-stub bodies
|
||||
mdtest path: crates/ty_python_semantic/resources/mdtest/overloads.md
|
||||
---
|
||||
|
||||
# Python source files
|
||||
|
||||
## mdtest_snippet.py
|
||||
|
||||
```
|
||||
1 | from typing import overload
|
||||
2 |
|
||||
3 | @overload
|
||||
4 | def x(y: int) -> int: ...
|
||||
5 | @overload
|
||||
6 | def x(y: str) -> str:
|
||||
7 | """Docstring"""
|
||||
8 |
|
||||
9 | @overload
|
||||
10 | def x(y: bytes) -> bytes:
|
||||
11 | pass
|
||||
12 |
|
||||
13 | @overload
|
||||
14 | def x(y: memoryview) -> memoryview:
|
||||
15 | """More docs"""
|
||||
16 | pass
|
||||
17 | ...
|
||||
18 |
|
||||
19 | def x(y):
|
||||
20 | return y
|
||||
21 | @overload
|
||||
22 | def foo(x: int) -> int:
|
||||
23 | return x # error: [useless-overload-body]
|
||||
24 |
|
||||
25 | @overload
|
||||
26 | def foo(x: str) -> None:
|
||||
27 | """Docstring"""
|
||||
28 | pass
|
||||
29 | print("oh no, a string") # error: [useless-overload-body]
|
||||
30 |
|
||||
31 | def foo(x):
|
||||
32 | return x
|
||||
```
|
||||
|
||||
# Diagnostics
|
||||
|
||||
```
|
||||
warning[useless-overload-body]: Useless body for `@overload`-decorated function `foo`
|
||||
--> src/mdtest_snippet.py:23:5
|
||||
|
|
||||
21 | @overload
|
||||
22 | def foo(x: int) -> int:
|
||||
23 | return x # error: [useless-overload-body]
|
||||
| ^^^^^^^^ This statement will never be executed
|
||||
24 |
|
||||
25 | @overload
|
||||
|
|
||||
info: `@overload`-decorated functions are solely for type checkers and must be overwritten at runtime by a non-`@overload`-decorated implementation
|
||||
help: Consider replacing this function body with `...` or `pass`
|
||||
info: rule `useless-overload-body` is enabled by default
|
||||
|
||||
```
|
||||
|
||||
```
|
||||
warning[useless-overload-body]: Useless body for `@overload`-decorated function `foo`
|
||||
--> src/mdtest_snippet.py:29:5
|
||||
|
|
||||
27 | """Docstring"""
|
||||
28 | pass
|
||||
29 | print("oh no, a string") # error: [useless-overload-body]
|
||||
| ^^^^^^^^^^^^^^^^^^^^^^^^ This statement will never be executed
|
||||
30 |
|
||||
31 | def foo(x):
|
||||
|
|
||||
info: `@overload`-decorated functions are solely for type checkers and must be overwritten at runtime by a non-`@overload`-decorated implementation
|
||||
help: Consider replacing this function body with `...` or `pass`
|
||||
info: rule `useless-overload-body` is enabled by default
|
||||
|
||||
```
|
|
@ -64,6 +64,7 @@ pub(crate) fn register_lints(registry: &mut LintRegistryBuilder) {
|
|||
registry.register_lint(&INVALID_TYPE_ALIAS_TYPE);
|
||||
registry.register_lint(&INVALID_METACLASS);
|
||||
registry.register_lint(&INVALID_OVERLOAD);
|
||||
registry.register_lint(&USELESS_OVERLOAD_BODY);
|
||||
registry.register_lint(&INVALID_PARAMETER_DEFAULT);
|
||||
registry.register_lint(&INVALID_PROTOCOL);
|
||||
registry.register_lint(&INVALID_NAMED_TUPLE);
|
||||
|
@ -958,6 +959,62 @@ declare_lint! {
|
|||
}
|
||||
}
|
||||
|
||||
declare_lint! {
|
||||
/// ## What it does
|
||||
/// Checks for various `@overload`-decorated functions that have non-stub bodies.
|
||||
///
|
||||
/// ## Why is this bad?
|
||||
/// Functions decorated with `@overload` are ignored at runtime; they are overridden
|
||||
/// by the implementation function that follows the series of overloads. While it is
|
||||
/// not illegal to provide a body for an `@overload`-decorated function, it may indicate
|
||||
/// a misunderstanding of how the `@overload` decorator works.
|
||||
///
|
||||
/// ## Example
|
||||
///
|
||||
/// ```py
|
||||
/// from typing import overload
|
||||
///
|
||||
/// @overload
|
||||
/// def foo(x: int) -> int:
|
||||
/// return x + 1 # will never be executed
|
||||
///
|
||||
/// @overload
|
||||
/// def foo(x: str) -> str:
|
||||
/// return "Oh no, got a string" # will never be executed
|
||||
///
|
||||
/// def foo(x: int | str) -> int | str:
|
||||
/// raise Exception("unexpected type encountered")
|
||||
/// ```
|
||||
///
|
||||
/// Use instead:
|
||||
///
|
||||
/// ```py
|
||||
/// from typing import assert_never, overload
|
||||
///
|
||||
/// @overload
|
||||
/// def foo(x: int) -> int: ...
|
||||
///
|
||||
/// @overload
|
||||
/// def foo(x: str) -> str: ...
|
||||
///
|
||||
/// def foo(x: int | str) -> int | str:
|
||||
/// if isinstance(x, int):
|
||||
/// return x + 1
|
||||
/// elif isinstance(x, str):
|
||||
/// return "Oh no, got a string"
|
||||
/// else:
|
||||
/// assert_never(x)
|
||||
/// ```
|
||||
///
|
||||
/// ## References
|
||||
/// - [Python documentation: `@overload`](https://docs.python.org/3/library/typing.html#typing.overload)
|
||||
pub(crate) static USELESS_OVERLOAD_BODY = {
|
||||
summary: "detects `@overload`-decorated functions with non-stub bodies",
|
||||
status: LintStatus::preview("1.0.0"),
|
||||
default_level: Level::Warn,
|
||||
}
|
||||
}
|
||||
|
||||
declare_lint! {
|
||||
/// ## What it does
|
||||
/// Checks for default values that can't be
|
||||
|
|
|
@ -57,7 +57,7 @@ use crate::types::diagnostic::{
|
|||
INVALID_TYPE_FORM, INVALID_TYPE_GUARD_CALL, INVALID_TYPE_VARIABLE_CONSTRAINTS,
|
||||
IncompatibleBases, NON_SUBSCRIPTABLE, POSSIBLY_MISSING_IMPLICIT_CALL, POSSIBLY_MISSING_IMPORT,
|
||||
UNDEFINED_REVEAL, UNRESOLVED_ATTRIBUTE, UNRESOLVED_GLOBAL, UNRESOLVED_IMPORT,
|
||||
UNRESOLVED_REFERENCE, UNSUPPORTED_OPERATOR, report_bad_dunder_set_call,
|
||||
UNRESOLVED_REFERENCE, UNSUPPORTED_OPERATOR, USELESS_OVERLOAD_BODY, report_bad_dunder_set_call,
|
||||
report_cannot_pop_required_field_on_typed_dict, report_implicit_return_type,
|
||||
report_instance_layout_conflict, report_invalid_assignment,
|
||||
report_invalid_attribute_assignment, report_invalid_generator_function_return_type,
|
||||
|
@ -1005,10 +1005,22 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> {
|
|||
.context
|
||||
.report_lint(&INVALID_OVERLOAD, &function_node.name)
|
||||
{
|
||||
builder.into_diagnostic(format_args!(
|
||||
"Overloaded non-stub function `{}` must have an implementation",
|
||||
let mut diagnostic = builder.into_diagnostic(format_args!(
|
||||
"Overloads for function `{}` must be followed by a non-`@overload`-decorated implementation function",
|
||||
&function_node.name
|
||||
));
|
||||
diagnostic.info(format_args!(
|
||||
"Attempting to call `{}` will raise `TypeError` at runtime",
|
||||
&function_node.name
|
||||
));
|
||||
diagnostic.info(
|
||||
"Overloaded functions without implementations are only permitted \
|
||||
in stub files, on protocols, or for abstract methods",
|
||||
);
|
||||
diagnostic.info(
|
||||
"See https://docs.python.org/3/library/typing.html#typing.overload \
|
||||
for more details",
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
@ -2169,6 +2181,37 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> {
|
|||
definition,
|
||||
&DeclaredAndInferredType::are_the_same_type(inferred_ty),
|
||||
);
|
||||
|
||||
if function_decorators.contains(FunctionDecorators::OVERLOAD) {
|
||||
for stmt in &function.body {
|
||||
match stmt {
|
||||
ast::Stmt::Pass(_) => continue,
|
||||
ast::Stmt::Expr(ast::StmtExpr { value, .. }) => {
|
||||
if matches!(
|
||||
&**value,
|
||||
ast::Expr::StringLiteral(_) | ast::Expr::EllipsisLiteral(_)
|
||||
) {
|
||||
continue;
|
||||
}
|
||||
}
|
||||
_ => {}
|
||||
}
|
||||
let Some(builder) = self.context.report_lint(&USELESS_OVERLOAD_BODY, stmt) else {
|
||||
continue;
|
||||
};
|
||||
let mut diagnostic = builder.into_diagnostic(format_args!(
|
||||
"Useless body for `@overload`-decorated function `{}`",
|
||||
&function.name
|
||||
));
|
||||
diagnostic.set_primary_message("This statement will never be executed");
|
||||
diagnostic.info(
|
||||
"`@overload`-decorated functions are solely for type checkers \
|
||||
and must be overwritten at runtime by a non-`@overload`-decorated implementation",
|
||||
);
|
||||
diagnostic.help("Consider replacing this function body with `...` or `pass`");
|
||||
break;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
fn infer_return_type_annotation(
|
||||
|
|
Loading…
Add table
Add a link
Reference in a new issue