mirror of
https://github.com/astral-sh/ruff.git
synced 2025-09-27 20:42:10 +00:00
[ty] Improve invalid-type-form
diagnostic where a module-literal type is used in a type expression and the module has a member which would be valid in a type expression (#18244)
This commit is contained in:
parent
41463396cf
commit
02394b8049
3 changed files with 136 additions and 7 deletions
|
@ -113,3 +113,35 @@ def _(
|
||||||
reveal_type(f) # revealed: Unknown
|
reveal_type(f) # revealed: Unknown
|
||||||
reveal_type(g) # revealed: Unknown
|
reveal_type(g) # revealed: Unknown
|
||||||
```
|
```
|
||||||
|
|
||||||
|
## Diagnostics for common errors
|
||||||
|
|
||||||
|
<!-- snapshot-diagnostics -->
|
||||||
|
|
||||||
|
### Module-literal used when you meant to use a class from that module
|
||||||
|
|
||||||
|
It's pretty common in Python to accidentally use a module-literal type in a type expression when you
|
||||||
|
*meant* to use a class by the same name that comes from that module. We emit a nice subdiagnostic
|
||||||
|
for this case:
|
||||||
|
|
||||||
|
`foo.py`:
|
||||||
|
|
||||||
|
```py
|
||||||
|
import datetime
|
||||||
|
|
||||||
|
def f(x: datetime): ... # error: [invalid-type-form]
|
||||||
|
```
|
||||||
|
|
||||||
|
`PIL/Image.py`:
|
||||||
|
|
||||||
|
```py
|
||||||
|
class Image: ...
|
||||||
|
```
|
||||||
|
|
||||||
|
`bar.py`:
|
||||||
|
|
||||||
|
```py
|
||||||
|
from PIL import Image
|
||||||
|
|
||||||
|
def g(x: Image): ... # error: [invalid-type-form]
|
||||||
|
```
|
||||||
|
|
|
@ -0,0 +1,62 @@
|
||||||
|
---
|
||||||
|
source: crates/ty_test/src/lib.rs
|
||||||
|
expression: snapshot
|
||||||
|
---
|
||||||
|
---
|
||||||
|
mdtest name: invalid.md - Tests for invalid types in type expressions - Diagnostics for common errors - Module-literal used when you meant to use a class from that module
|
||||||
|
mdtest path: crates/ty_python_semantic/resources/mdtest/annotations/invalid.md
|
||||||
|
---
|
||||||
|
|
||||||
|
# Python source files
|
||||||
|
|
||||||
|
## foo.py
|
||||||
|
|
||||||
|
```
|
||||||
|
1 | import datetime
|
||||||
|
2 |
|
||||||
|
3 | def f(x: datetime): ... # error: [invalid-type-form]
|
||||||
|
```
|
||||||
|
|
||||||
|
## PIL/Image.py
|
||||||
|
|
||||||
|
```
|
||||||
|
1 | class Image: ...
|
||||||
|
```
|
||||||
|
|
||||||
|
## bar.py
|
||||||
|
|
||||||
|
```
|
||||||
|
1 | from PIL import Image
|
||||||
|
2 |
|
||||||
|
3 | def g(x: Image): ... # error: [invalid-type-form]
|
||||||
|
```
|
||||||
|
|
||||||
|
# Diagnostics
|
||||||
|
|
||||||
|
```
|
||||||
|
error[invalid-type-form]: Variable of type `<module 'datetime'>` is not allowed in a type expression
|
||||||
|
--> src/foo.py:3:10
|
||||||
|
|
|
||||||
|
1 | import datetime
|
||||||
|
2 |
|
||||||
|
3 | def f(x: datetime): ... # error: [invalid-type-form]
|
||||||
|
| ^^^^^^^^
|
||||||
|
|
|
||||||
|
info: Did you mean to use the module's member `datetime.datetime` instead?
|
||||||
|
info: rule `invalid-type-form` is enabled by default
|
||||||
|
|
||||||
|
```
|
||||||
|
|
||||||
|
```
|
||||||
|
error[invalid-type-form]: Variable of type `<module 'PIL.Image'>` is not allowed in a type expression
|
||||||
|
--> src/bar.py:3:10
|
||||||
|
|
|
||||||
|
1 | from PIL import Image
|
||||||
|
2 |
|
||||||
|
3 | def g(x: Image): ... # error: [invalid-type-form]
|
||||||
|
| ^^^^^
|
||||||
|
|
|
||||||
|
info: Did you mean to use the module's member `Image.Image` instead?
|
||||||
|
info: rule `invalid-type-form` is enabled by default
|
||||||
|
|
||||||
|
```
|
|
@ -4821,7 +4821,7 @@ impl<'db> Type<'db> {
|
||||||
pub fn in_type_expression(
|
pub fn in_type_expression(
|
||||||
&self,
|
&self,
|
||||||
db: &'db dyn Db,
|
db: &'db dyn Db,
|
||||||
scope_id: ScopeId,
|
scope_id: ScopeId<'db>,
|
||||||
) -> Result<Type<'db>, InvalidTypeExpressionError<'db>> {
|
) -> Result<Type<'db>, InvalidTypeExpressionError<'db>> {
|
||||||
match self {
|
match self {
|
||||||
// Special cases for `float` and `complex`
|
// Special cases for `float` and `complex`
|
||||||
|
@ -4872,7 +4872,9 @@ impl<'db> Type<'db> {
|
||||||
| Type::BoundSuper(_)
|
| Type::BoundSuper(_)
|
||||||
| Type::ProtocolInstance(_)
|
| Type::ProtocolInstance(_)
|
||||||
| Type::PropertyInstance(_) => Err(InvalidTypeExpressionError {
|
| Type::PropertyInstance(_) => Err(InvalidTypeExpressionError {
|
||||||
invalid_expressions: smallvec::smallvec![InvalidTypeExpression::InvalidType(*self)],
|
invalid_expressions: smallvec::smallvec![InvalidTypeExpression::InvalidType(
|
||||||
|
*self, scope_id
|
||||||
|
)],
|
||||||
fallback_type: Type::unknown(),
|
fallback_type: Type::unknown(),
|
||||||
}),
|
}),
|
||||||
|
|
||||||
|
@ -4910,7 +4912,7 @@ impl<'db> Type<'db> {
|
||||||
return Err(InvalidTypeExpressionError {
|
return Err(InvalidTypeExpressionError {
|
||||||
fallback_type: Type::unknown(),
|
fallback_type: Type::unknown(),
|
||||||
invalid_expressions: smallvec::smallvec![
|
invalid_expressions: smallvec::smallvec![
|
||||||
InvalidTypeExpression::InvalidType(*self)
|
InvalidTypeExpression::InvalidType(*self, scope_id)
|
||||||
],
|
],
|
||||||
});
|
});
|
||||||
};
|
};
|
||||||
|
@ -5037,7 +5039,7 @@ impl<'db> Type<'db> {
|
||||||
)),
|
)),
|
||||||
_ => Err(InvalidTypeExpressionError {
|
_ => Err(InvalidTypeExpressionError {
|
||||||
invalid_expressions: smallvec::smallvec![InvalidTypeExpression::InvalidType(
|
invalid_expressions: smallvec::smallvec![InvalidTypeExpression::InvalidType(
|
||||||
*self
|
*self, scope_id
|
||||||
)],
|
)],
|
||||||
fallback_type: Type::unknown(),
|
fallback_type: Type::unknown(),
|
||||||
}),
|
}),
|
||||||
|
@ -5751,7 +5753,8 @@ impl<'db> InvalidTypeExpressionError<'db> {
|
||||||
let Some(builder) = context.report_lint(&INVALID_TYPE_FORM, node) else {
|
let Some(builder) = context.report_lint(&INVALID_TYPE_FORM, node) else {
|
||||||
continue;
|
continue;
|
||||||
};
|
};
|
||||||
builder.into_diagnostic(error.reason(context.db()));
|
let diagnostic = builder.into_diagnostic(error.reason(context.db()));
|
||||||
|
error.add_subdiagnostics(context.db(), diagnostic);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
fallback_type
|
fallback_type
|
||||||
|
@ -5778,7 +5781,7 @@ enum InvalidTypeExpression<'db> {
|
||||||
/// and which would require exactly one argument even if they appeared in an annotation expression
|
/// and which would require exactly one argument even if they appeared in an annotation expression
|
||||||
TypeQualifierRequiresOneArgument(KnownInstanceType<'db>),
|
TypeQualifierRequiresOneArgument(KnownInstanceType<'db>),
|
||||||
/// Some types are always invalid in type expressions
|
/// Some types are always invalid in type expressions
|
||||||
InvalidType(Type<'db>),
|
InvalidType(Type<'db>, ScopeId<'db>),
|
||||||
}
|
}
|
||||||
|
|
||||||
impl<'db> InvalidTypeExpression<'db> {
|
impl<'db> InvalidTypeExpression<'db> {
|
||||||
|
@ -5822,7 +5825,7 @@ impl<'db> InvalidTypeExpression<'db> {
|
||||||
"Type qualifier `{q}` is not allowed in type expressions (only in annotation expressions, and only with exactly one argument)",
|
"Type qualifier `{q}` is not allowed in type expressions (only in annotation expressions, and only with exactly one argument)",
|
||||||
q = qualifier.repr(self.db)
|
q = qualifier.repr(self.db)
|
||||||
),
|
),
|
||||||
InvalidTypeExpression::InvalidType(ty) => write!(
|
InvalidTypeExpression::InvalidType(ty, _) => write!(
|
||||||
f,
|
f,
|
||||||
"Variable of type `{ty}` is not allowed in a type expression",
|
"Variable of type `{ty}` is not allowed in a type expression",
|
||||||
ty = ty.display(self.db)
|
ty = ty.display(self.db)
|
||||||
|
@ -5833,6 +5836,38 @@ impl<'db> InvalidTypeExpression<'db> {
|
||||||
|
|
||||||
Display { error: self, db }
|
Display { error: self, db }
|
||||||
}
|
}
|
||||||
|
|
||||||
|
fn add_subdiagnostics(self, db: &'db dyn Db, mut diagnostic: LintDiagnosticGuard) {
|
||||||
|
let InvalidTypeExpression::InvalidType(ty, scope) = self else {
|
||||||
|
return;
|
||||||
|
};
|
||||||
|
let Type::ModuleLiteral(module_type) = ty else {
|
||||||
|
return;
|
||||||
|
};
|
||||||
|
let module = module_type.module(db);
|
||||||
|
let Some(module_name_final_part) = module.name().components().next_back() else {
|
||||||
|
return;
|
||||||
|
};
|
||||||
|
let Some(module_member_with_same_name) = ty
|
||||||
|
.member(db, module_name_final_part)
|
||||||
|
.symbol
|
||||||
|
.ignore_possibly_unbound()
|
||||||
|
else {
|
||||||
|
return;
|
||||||
|
};
|
||||||
|
if module_member_with_same_name
|
||||||
|
.in_type_expression(db, scope)
|
||||||
|
.is_err()
|
||||||
|
{
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
|
||||||
|
// TODO: showing a diff (and even having an autofix) would be even better
|
||||||
|
diagnostic.info(format_args!(
|
||||||
|
"Did you mean to use the module's member \
|
||||||
|
`{module_name_final_part}.{module_name_final_part}` instead?"
|
||||||
|
));
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
/// Whether this typecar was created via the legacy `TypeVar` constructor, or using PEP 695 syntax.
|
/// Whether this typecar was created via the legacy `TypeVar` constructor, or using PEP 695 syntax.
|
||||||
|
|
Loading…
Add table
Add a link
Reference in a new issue