Skip to content

Commit e91e2f4

Browse files
authored
[red-knot] Trust module-level undeclared symbols in stubs (#17577)
## Summary Many symbols in typeshed are defined without being declared. For example: ```pyi # builtins: IOError = OSError # types LambdaType = FunctionType NotImplementedType = _NotImplementedType # typing Text = str # random uniform = _inst.uniform # optparse make_option = Option # all over the place: _T = TypeVar("_T") ``` Here, we introduce a change that skips widening the public type of these symbols (by unioning with `Unknown`). fixes #17032 ## Ecosystem analysis This is difficult to analyze in detail, but I went over most changes and it looks very favorable to me overall. The diff on the overall numbers is: ``` errors: 1287 -> 859 (reduction by 428) warnings: 45 -> 59 (increase by 14) ``` ### Removed false positives `invalid-base` examples: ```diff - error[lint:invalid-base] /tmp/mypy_primer/projects/pip/src/pip/_vendor/rich/console.py:548:27: Invalid class base with type `Unknown | Literal[_local]` (all bases must be a class, `Any`, `Unknown` or `Todo`) - error[lint:invalid-base] /tmp/mypy_primer/projects/tornado/tornado/iostream.py:84:25: Invalid class base with type `Unknown | Literal[OSError]` (all bases must be a class, `Any`, `Unknown` or `Todo`) - error[lint:invalid-base] /tmp/mypy_primer/projects/mitmproxy/test/conftest.py:35:40: Invalid class base with type `Unknown | Literal[_UnixDefaultEventLoopPolicy]` (all bases must be a class, `Any`, `Unknown` or `Todo`) ``` `invalid-exception-caught` examples: ```diff - error[lint:invalid-exception-caught] /tmp/mypy_primer/projects/cloud-init/cloudinit/cmd/status.py:334:16: Cannot catch object of type `Literal[ProcessExecutionError]` in an exception handler (must be a `BaseException` subclass or a tuple of `BaseException` subclasses) - error[lint:invalid-exception-caught] /tmp/mypy_primer/projects/jinja/src/jinja2/loaders.py:537:16: Cannot catch object of type `Literal[TemplateNotFound]` in an exception handler (must be a `BaseException` subclass or a tuple of `BaseException` subclasses) ``` `unresolved-reference` examples https://github.com/canonical/cloud-init/blob/7a0265d36e01e649f72005548f17dca9ac0150ad/cloudinit/handlers/jinja_template.py#L120-L123 (we now understand the `isinstance` narrowing) ```diff - error[lint:unresolved-attribute] /tmp/mypy_primer/projects/cloud-init/cloudinit/handlers/jinja_template.py:123:16: Type `Exception` has no attribute `errno` ``` `unknown-argument` examples https://github.com/hauntsaninja/boostedblob/blob/master/boostedblob/request.py#L53 ```diff - error[lint:unknown-argument] /tmp/mypy_primer/projects/boostedblob/boostedblob/request.py:53:17: Argument `connect` does not match any known parameter of bound method `__init__` ``` `unknown-argument` There are a lot of `__init__`-related changes because we now understand [`@attr.s`](https://github.com/python-attrs/attrs/blob/3d42a6978ac60b487135db39218cfb742b100899/src/attr/__init__.pyi#L387) as a `@dataclass_transform` annotated symbol. For example: ```diff - error[lint:unknown-argument] /tmp/mypy_primer/projects/attrs/tests/test_hooks.py:72:18: Argument `x` does not match any known parameter of bound method `__init__` ``` ### New false positives This can happen if a symbol that previously was inferred as `X | Unknown` was assigned-to, but we don't yet understand the assignability to `X`: https://github.com/strawberry-graphql/strawberry/blob/main/strawberry/exceptions/handler.py#L90 ```diff + error[lint:invalid-assignment] /tmp/mypy_primer/projects/strawberry/strawberry/exceptions/handler.py:90:9: Object of type `def strawberry_threading_exception_handler(args: tuple[type[BaseException], BaseException | None, TracebackType | None, Thread | None]) -> None` is not assignable to attribute `excepthook` of type `(_ExceptHookArgs, /) -> Any` ``` ### New true positives https://github.com/DataDog/dd-trace-py/blob/6bbb5519fe4b3964f9ca73b21cf35df8387618b2/tests/tracer/test_span.py#L714 ```diff + error[lint:invalid-argument-type] /tmp/mypy_primer/projects/dd-trace-py/tests/tracer/test_span.py:714:33: Argument to this function is incorrect: Expected `str`, found `Literal[b"\xf0\x9f\xa4\x94"]` ``` ### Changed diagnostics A lot of changed diagnostics because we now show `@Todo(Support for `typing.TypeVar` instances in type expressions)` instead of `Unknown` for all kinds of symbols that used a `_T = TypeVar("_T")` as a type. One prominent example is the `list.__getitem__` method: `builtins.pyi`: ```pyi _T = TypeVar("_T") # previously `TypeVar | Unknown`, now just `TypeVar` # … class list(MutableSequence[_T]): # … @overload def __getitem__(self, i: SupportsIndex, /) -> _T: ... # … ``` which causes this change in diagnostics: ```py xs = [1, 2] reveal_type(xs[0]) # previously `Unknown`, now `@Todo(Support for `typing.TypeVar` instances in type expressions)` ``` ## Test Plan Updated Markdown tests
1 parent b537552 commit e91e2f4

File tree

6 files changed

+24
-12
lines changed

6 files changed

+24
-12
lines changed

crates/red_knot_python_semantic/resources/mdtest/scopes/eager.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -404,7 +404,7 @@ x = int
404404
class C:
405405
var: ClassVar[x]
406406

407-
reveal_type(C.var) # revealed: Unknown | str
407+
reveal_type(C.var) # revealed: str
408408

409409
x = str
410410
```

crates/red_knot_python_semantic/resources/mdtest/subscript/lists.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ x = [1, 2, 3]
1212
reveal_type(x) # revealed: list
1313

1414
# TODO reveal int
15-
reveal_type(x[0]) # revealed: Unknown
15+
reveal_type(x[0]) # revealed: @Todo(Support for `typing.TypeVar` instances in type expressions)
1616

1717
# TODO reveal list
1818
reveal_type(x[0:1]) # revealed: @Todo(specialized non-generic class)

crates/red_knot_python_semantic/resources/mdtest/type_properties/is_singleton.md

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -128,10 +128,7 @@ python-version = "3.10"
128128
import types
129129
from knot_extensions import static_assert, is_singleton
130130

131-
# TODO: types.NotImplementedType is a TypeAlias of builtins._NotImplementedType
132-
# Once TypeAlias support is added, it should satisfy `is_singleton`
133-
reveal_type(types.NotImplementedType) # revealed: Unknown | Literal[_NotImplementedType]
134-
static_assert(not is_singleton(types.NotImplementedType))
131+
static_assert(is_singleton(types.NotImplementedType))
135132
```
136133

137134
### Callables

crates/red_knot_python_semantic/src/semantic_index/symbol.rs

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -115,6 +115,10 @@ impl<'db> ScopeId<'db> {
115115
self.node(db).scope_kind().is_function_like()
116116
}
117117

118+
pub(crate) fn is_module_scope(self, db: &'db dyn Db) -> bool {
119+
self.node(db).scope_kind().is_module()
120+
}
121+
118122
pub(crate) fn is_type_parameter(self, db: &'db dyn Db) -> bool {
119123
self.node(db).scope_kind().is_type_parameter()
120124
}
@@ -263,6 +267,10 @@ impl ScopeKind {
263267
matches!(self, ScopeKind::Class)
264268
}
265269

270+
pub(crate) fn is_module(self) -> bool {
271+
matches!(self, ScopeKind::Module)
272+
}
273+
266274
pub(crate) fn is_type_parameter(self) -> bool {
267275
matches!(self, ScopeKind::Annotation | ScopeKind::TypeAlias)
268276
}

crates/red_knot_python_semantic/src/symbol.rs

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -593,8 +593,18 @@ fn symbol_by_id<'db>(
593593
"__slots__" | "TYPE_CHECKING"
594594
);
595595

596-
widen_type_for_undeclared_public_symbol(db, inferred, is_considered_non_modifiable)
597-
.into()
596+
if scope.is_module_scope(db) && scope.file(db).is_stub(db.upcast()) {
597+
// We generally trust module-level undeclared symbols in stubs and do not union
598+
// with `Unknown`. If we don't do this, simple aliases like `IOError = OSError` in
599+
// stubs would result in `IOError` being a union of `OSError` and `Unknown`, which
600+
// leads to all sorts of downstream problems. Similarly, type variables are often
601+
// defined as `_T = TypeVar("_T")`, without being declared.
602+
603+
inferred.into()
604+
} else {
605+
widen_type_for_undeclared_public_symbol(db, inferred, is_considered_non_modifiable)
606+
.into()
607+
}
598608
}
599609
// Symbol has conflicting declared types
600610
Err((declared, _)) => {

crates/red_knot_python_semantic/src/types/signatures.rs

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1631,10 +1631,7 @@ mod tests {
16311631
assert_eq!(a_name, "a");
16321632
assert_eq!(b_name, "b");
16331633
// Parameter resolution deferred; we should see B
1634-
assert_eq!(
1635-
a_annotated_ty.unwrap().display(&db).to_string(),
1636-
"Unknown | B"
1637-
);
1634+
assert_eq!(a_annotated_ty.unwrap().display(&db).to_string(), "B");
16381635
assert_eq!(b_annotated_ty.unwrap().display(&db).to_string(), "T");
16391636
}
16401637

0 commit comments

Comments
 (0)