mirror of
https://github.com/astral-sh/ruff.git
synced 2025-08-04 18:58:26 +00:00
[ty] Use RHS inferred type for bare Final symbols (#19142)
## Summary
Infer the type of symbols with a `Final` qualifier as their
right-hand-side inferred type:
```py
x: Final = 1
y: Final[int] = 1
def _():
reveal_type(x) # previously: Unknown, now: Literal[1]
reveal_type(y) # int, same as before
```
Part of https://github.com/astral-sh/ty/issues/158
## Ecosystem analysis
### aiohttp
```diff
aiohttp (https://github.com/aio-libs/aiohttp)
+ error[invalid-argument-type] aiohttp/compression_utils.py:131:54: Argument to bound method `__init__` is incorrect: Expected `ZLibBackendProtocol`, found `<module 'zlib'>`
```
This code [creates a
protocol](a83597fa88/aiohttp/compression_utils.py (L52-L77))
that looks like
```pyi
class ZLibBackendProtocol(Protocol):
Z_FULL_FLUSH: int
Z_SYNC_FLUSH: int
# more fields…
```
It then [tries to
assign](a83597fa88/aiohttp/compression_utils.py (L131))
the module literal `zlib` to that protocol. Howefer, in typeshed, these
`zlib` members are annotated like this:
```pyi
Z_FULL_FLUSH: Final = 3
Z_SYNC_FLUSH: Final = 2
```
With the proposed change here, we now infer these as `Literal[3]` /
`Literal[2]`. Since protocol members have to be assignable both ways
(invariance), we do not consider `zlib` assignable to this protocol
anymore.
That seems rather unfortunate. Not sure who is to blame here? That
`ZLibBackendProtocol` protocol should probably not annotate the members
with `int`, given that `typeshed` doesn't use an explicit annotation
here either? But what should they do instead? Annotate those fields with
`Any`?
Or is it another case where we should consider literal-widening?
FYI @AlexWaygood
### cloud-init
```diff
cloud-init (https://github.com/canonical/cloud-init)
+ error[invalid-argument-type] tests/unittests/sources/test_smartos.py:575:32: Argument to function `oct` is incorrect: Expected `SupportsIndex`, found `int | float`
+ error[invalid-argument-type] tests/unittests/sources/test_smartos.py:593:32: Argument to function `oct` is incorrect: Expected `SupportsIndex`, found `int | float`
+ error[invalid-argument-type] tests/unittests/sources/test_smartos.py:647:35: Argument to function `oct` is incorrect: Expected `SupportsIndex`, found `int | float`
```
New false positives on expressions like
`oct(os.stat(legacy_script_f)[stat.ST_MODE])`. We now correctly infer
`stat.ST_MODE` as `Literal[1]`, because in typeshed, it is annotated as
`ST_MODE: Final = 0`. `os.stat` returns a `stat_result` which is a tuple
subclass. Accessing it at index 0 should return an `int`, but we
currently return `int | float`, presumably due to missing support for
tuple subclasses (FYI @AlexWaygood):
```pyi
class stat_result(structseq[float], tuple[int, int, int, int, int, int, int, float, float, float]):
```
In terms of `typing.Final`, things are working as expected here.
### pywin-32
Many new false positives similar to:
```diff
pywin32 (https://github.com/mhammond/pywin32)
+ error[invalid-argument-type] Pythonwin/pywin/docking/DockingBar.py:288:55: Argument to function `LoadCursor` is incorrect: Expected `PyResourceId`, found `Literal[32645]`
```
The line in question calls `win32api.LoadCursor(0, win32con.IDC_ARROW)`.
The `win32con.IDC_ARROW` symbol is annotated as [`IDC_ARROW: Final =
32512` in
typeshed](2408c028f4/stubs/pywin32/win32/lib/win32con.pyi (L594)),
but
[`LoadCursor`](2408c028f4/stubs/pywin32/win32/win32api.pyi (L197))
expects a
[`PyResourceId`](2408c028f4/stubs/pywin32/_win32typing.pyi (L1252)),
which is an empty class. So.. this seems like a true positive to me,
unless that typeshed annotation of `IDC_ARROW` is meant to imply that
the type should be `Unknown`/`Any`?
### streamlit
```diff
streamlit (https://github.com/streamlit/streamlit)
+ error[invalid-argument-type] lib/streamlit/string_util.py:163:37: Argument to bound method `translate` is incorrect: Expected `bytes`, found `bytearray`
```
This looks like a true positive? The code calls `inp.translate(None,
TEXTCHARS)`. `inp` is `bytes`, and `TEXTCHARS` is:
```py
TEXTCHARS: Final = bytearray(
{7, 8, 9, 10, 12, 13, 27} | set(range(0x20, 0x100)) - {0x7F}
)
```
~~We now infer this as `bytearray`, but `bytes.translate` [expects
`bytes` for its `delete`
parameter](2408c028f4/stdlib/builtins.pyi (L710)).
This seems to work at runtime, so maybe the typeshed annotation is
wrong?~~ (Edit: this is now fixed in typeshed)
```pycon
>>> b"abc".translate(None, bytearray(b"b"))
b'ac'
```
## rotki
```diff
+ error[invalid-return-type] rotkehlchen/chain/ethereum/modules/yearn/decoder.py:412:13: Return type does not match returned value: expected `dict[Unknown, str]`, found `dict[Unknown, Literal["yearn-v1", "yearn-v2"]]`
```
The code in question looks like
```py
def addresses_to_counterparties(self) -> dict[ChecksumEvmAddress, str]:
return dict.fromkeys(self.vaults, CPT_BEEFY_FINANCE)
```
where `CPT_BEEFY_FINANCE: Final = 'beefy_finance'. We previously
inferred the value type of the returned `dict` as `Unknown`, and now we
infer it as `Literal["beefy_finance"]`, which does not match the
annotated return type because `dict` is invariant in the value type.
```diff
+ error[invalid-argument-type] rotkehlchen/tests/unit/decoders/test_curve.py:249:9: Argument is incorrect: Expected `int`, found `FVal`
```
There are true positives that were previously silenced through the
`Unknown`.
## Test Plan
New Markdown tests
This commit is contained in:
David Peter
GitHub
parent
e0b7f496f2
commit
c15aa572ff
2 changed files with 126 additions and 19 deletions
|
|
@ -3,44 +3,124 @@
|
|||
[`typing.Final`] is a type qualifier that is used to indicate that a symbol may not be reassigned in
|
||||
any scope. Final names declared in class scopes cannot be overridden in subclasses.
|
||||
|
||||
## Basic
|
||||
## Basic type inference
|
||||
|
||||
### `Final` with type
|
||||
|
||||
Declared symbols that are additionally qualified with `Final` use the declared type when accessed
|
||||
from another scope. Local uses of the symbol will use the inferred type, which may be more specific:
|
||||
|
||||
`mod.py`:
|
||||
|
||||
```py
|
||||
from typing import Final, Annotated
|
||||
|
||||
FINAL_A: int = 1
|
||||
FINAL_A: Final[int] = 1
|
||||
FINAL_B: Annotated[Final[int], "the annotation for FINAL_B"] = 1
|
||||
FINAL_C: Final[Annotated[int, "the annotation for FINAL_C"]] = 1
|
||||
FINAL_D: Final = 1
|
||||
FINAL_E: "Final[int]" = 1
|
||||
FINAL_D: "Final[int]" = 1
|
||||
FINAL_F: Final[int]
|
||||
FINAL_F = 1
|
||||
|
||||
reveal_type(FINAL_A) # revealed: Literal[1]
|
||||
reveal_type(FINAL_B) # revealed: Literal[1]
|
||||
reveal_type(FINAL_C) # revealed: Literal[1]
|
||||
reveal_type(FINAL_D) # revealed: Literal[1]
|
||||
reveal_type(FINAL_E) # revealed: Literal[1]
|
||||
reveal_type(FINAL_D) # revealed: Literal[1]
|
||||
|
||||
# TODO: All of these should be errors:
|
||||
def nonlocal_uses():
|
||||
reveal_type(FINAL_A) # revealed: int
|
||||
reveal_type(FINAL_B) # revealed: int
|
||||
reveal_type(FINAL_C) # revealed: int
|
||||
reveal_type(FINAL_D) # revealed: int
|
||||
reveal_type(FINAL_F) # revealed: int
|
||||
```
|
||||
|
||||
Imported types:
|
||||
|
||||
```py
|
||||
from mod import FINAL_A, FINAL_B, FINAL_C, FINAL_D, FINAL_F
|
||||
|
||||
reveal_type(FINAL_A) # revealed: int
|
||||
reveal_type(FINAL_B) # revealed: int
|
||||
reveal_type(FINAL_C) # revealed: int
|
||||
reveal_type(FINAL_D) # revealed: int
|
||||
reveal_type(FINAL_F) # revealed: int
|
||||
```
|
||||
|
||||
### `Final` without a type
|
||||
|
||||
When a symbol is qualified with `Final` but no type is specified, the type is inferred from the
|
||||
right-hand side of the assignment. We do not union the inferred type with `Unknown`, because the
|
||||
symbol cannot be modified:
|
||||
|
||||
`mod.py`:
|
||||
|
||||
```py
|
||||
from typing import Final
|
||||
|
||||
FINAL_A: Final = 1
|
||||
|
||||
reveal_type(FINAL_A) # revealed: Literal[1]
|
||||
|
||||
def nonlocal_uses():
|
||||
reveal_type(FINAL_A) # revealed: Literal[1]
|
||||
```
|
||||
|
||||
`main.py`:
|
||||
|
||||
```py
|
||||
from mod import FINAL_A
|
||||
|
||||
reveal_type(FINAL_A) # revealed: Literal[1]
|
||||
```
|
||||
|
||||
### In class definitions
|
||||
|
||||
```py
|
||||
from typing import Final
|
||||
|
||||
class C:
|
||||
FINAL_A: Final[int] = 1
|
||||
FINAL_B: Final = 1
|
||||
|
||||
def __init__(self):
|
||||
self.FINAL_C: Final[int] = 1
|
||||
self.FINAL_D: Final = 1
|
||||
|
||||
reveal_type(C.FINAL_A) # revealed: int
|
||||
reveal_type(C.FINAL_B) # revealed: Literal[1]
|
||||
|
||||
reveal_type(C().FINAL_A) # revealed: int
|
||||
reveal_type(C().FINAL_B) # revealed: Literal[1]
|
||||
reveal_type(C().FINAL_C) # revealed: int
|
||||
# TODO: this should be `Literal[1]`
|
||||
reveal_type(C().FINAL_D) # revealed: Unknown
|
||||
```
|
||||
|
||||
## Not modifiable
|
||||
|
||||
Symbols qualified with `Final` cannot be reassigned, and attempting to do so will result in an
|
||||
error:
|
||||
|
||||
```py
|
||||
from typing import Final, Annotated
|
||||
|
||||
FINAL_A: Final[int] = 1
|
||||
FINAL_B: Annotated[Final[int], "the annotation for FINAL_B"] = 1
|
||||
FINAL_C: Final[Annotated[int, "the annotation for FINAL_C"]] = 1
|
||||
FINAL_D: "Final[int]" = 1
|
||||
FINAL_E: Final[int]
|
||||
FINAL_E = 1
|
||||
FINAL_F: Final = 1
|
||||
|
||||
# TODO: all of these should be errors
|
||||
FINAL_A = 2
|
||||
FINAL_B = 2
|
||||
FINAL_C = 2
|
||||
FINAL_D = 2
|
||||
FINAL_E = 2
|
||||
```
|
||||
|
||||
Public types:
|
||||
|
||||
```py
|
||||
from mod import FINAL_A, FINAL_B, FINAL_C, FINAL_D, FINAL_E
|
||||
|
||||
# TODO: All of these should be Literal[1]
|
||||
reveal_type(FINAL_A) # revealed: int
|
||||
reveal_type(FINAL_B) # revealed: int
|
||||
reveal_type(FINAL_C) # revealed: int
|
||||
reveal_type(FINAL_D) # revealed: Unknown
|
||||
reveal_type(FINAL_E) # revealed: int
|
||||
FINAL_F = 2
|
||||
```
|
||||
|
||||
## Too many arguments
|
||||
|
|
|
|||
|
|
@ -524,6 +524,21 @@ impl<'db> PlaceAndQualifiers<'db> {
|
|||
self.qualifiers.contains(TypeQualifiers::CLASS_VAR)
|
||||
}
|
||||
|
||||
/// Returns `Some(…)` if the place is qualified with `typing.Final` without a specified type.
|
||||
pub(crate) fn is_bare_final(&self) -> Option<TypeQualifiers> {
|
||||
match self {
|
||||
PlaceAndQualifiers { place, qualifiers }
|
||||
if (qualifiers.contains(TypeQualifiers::FINAL)
|
||||
&& place
|
||||
.ignore_possibly_unbound()
|
||||
.is_some_and(|ty| ty.is_unknown())) =>
|
||||
{
|
||||
Some(*qualifiers)
|
||||
}
|
||||
_ => None,
|
||||
}
|
||||
}
|
||||
|
||||
#[must_use]
|
||||
pub(crate) fn map_type(
|
||||
self,
|
||||
|
|
@ -645,6 +660,18 @@ fn place_by_id<'db>(
|
|||
ConsideredDefinitions::AllReachable => use_def.all_reachable_bindings(place_id),
|
||||
};
|
||||
|
||||
// If a symbol is undeclared, but qualified with `typing.Final`, we use the right-hand side
|
||||
// inferred type, without unioning with `Unknown`, because it can not be modified.
|
||||
if let Some(qualifiers) = declared
|
||||
.as_ref()
|
||||
.ok()
|
||||
.and_then(PlaceAndQualifiers::is_bare_final)
|
||||
{
|
||||
let bindings = all_considered_bindings();
|
||||
return place_from_bindings_impl(db, bindings, requires_explicit_reexport)
|
||||
.with_qualifiers(qualifiers);
|
||||
}
|
||||
|
||||
match declared {
|
||||
// Place is declared, trust the declared type
|
||||
Ok(
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue