[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)

Author: AlexWaygood

crates/ty_python_semantic/resources/mdtest/annotations/invalid.md

@@ -113,3 +113,35 @@ def _(
     reveal_type(f)  # 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]
+```

crates/ty_python_semantic/resources/mdtest/snapshots/invalid.md_-_Tests_for_invalid_ty…_-_Diagnostics_for_comm…_-_Module-literal_used_…_(652fec4fd4a6c63a).snap

@@ -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
+
+```

crates/ty_python_semantic/src/types.rs

@@ -4821,7 +4821,7 @@ impl<'db> Type<'db> {
     pub fn in_type_expression(
         &self,
         db: &'db dyn Db,
-        scope_id: ScopeId,
+        scope_id: ScopeId<'db>,
     ) -> Result<Type<'db>, InvalidTypeExpressionError<'db>> {
         match self {
             // Special cases for `float` and `complex`
@@ -4872,7 +4872,9 @@ impl<'db> Type<'db> {
             | Type::BoundSuper(_)
             | Type::ProtocolInstance(_)
             | Type::PropertyInstance(_) => Err(InvalidTypeExpressionError {
-                invalid_expressions: smallvec::smallvec![InvalidTypeExpression::InvalidType(*self)],
+                invalid_expressions: smallvec::smallvec![InvalidTypeExpression::InvalidType(
+                    *self, scope_id
+                )],
                 fallback_type: Type::unknown(),
             }),
 
@@ -4910,7 +4912,7 @@ impl<'db> Type<'db> {
                         return Err(InvalidTypeExpressionError {
                             fallback_type: Type::unknown(),
                             invalid_expressions: smallvec::smallvec![
-                                InvalidTypeExpression::InvalidType(*self)
+                                InvalidTypeExpression::InvalidType(*self, scope_id)
                             ],
                         });
                     };
@@ -5037,7 +5039,7 @@ impl<'db> Type<'db> {
                 )),
                 _ => Err(InvalidTypeExpressionError {
                     invalid_expressions: smallvec::smallvec![InvalidTypeExpression::InvalidType(
-                        *self
+                        *self, scope_id
                     )],
                     fallback_type: Type::unknown(),
                 }),
@@ -5751,7 +5753,8 @@ impl<'db> InvalidTypeExpressionError<'db> {
                 let Some(builder) = context.report_lint(&INVALID_TYPE_FORM, node) else {
                     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
@@ -5778,7 +5781,7 @@ enum InvalidTypeExpression<'db> {
     /// and which would require exactly one argument even if they appeared in an annotation expression
     TypeQualifierRequiresOneArgument(KnownInstanceType<'db>),
     /// Some types are always invalid in type expressions
-    InvalidType(Type<'db>),
+    InvalidType(Type<'db>, ScopeId<'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)",
                         q = qualifier.repr(self.db)
                     ),
-                    InvalidTypeExpression::InvalidType(ty) => write!(
+                    InvalidTypeExpression::InvalidType(ty, _) => write!(
                         f,
                         "Variable of type `{ty}` is not allowed in a type expression",
                         ty = ty.display(self.db)
@@ -5833,6 +5836,38 @@ impl<'db> InvalidTypeExpression<'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.