[ty] Better invalid-assignment diagnostics

Author: sharkdp

crates/ty/tests/cli/main.rs

@@ -41,22 +41,24 @@ fn test_quiet_output() -> anyhow::Result<()> {
     let case = CliTest::with_file("test.py", "x: int = 'foo'")?;
 
     // By default, we emit a diagnostic
-    assert_cmd_snapshot!(case.command(), @r###"
+    assert_cmd_snapshot!(case.command(), @r#"
     success: false
     exit_code: 1
     ----- stdout -----
     error[invalid-assignment]: Object of type `Literal["foo"]` is not assignable to `int`
-     --> test.py:1:1
+     --> test.py:1:4
       |
     1 | x: int = 'foo'
-      | ^
+      |    ---   ^^^^^ Incompatible value of type `Literal["foo"]`
+      |    |
+      |    Declared type
       |
     info: rule `invalid-assignment` is enabled by default
 
     Found 1 diagnostic
 
     ----- stderr -----
-    "###);
+    "#);
 
     // With `quiet`, the diagnostic is not displayed, just the summary message
     assert_cmd_snapshot!(case.command().arg("--quiet"), @r"

crates/ty_python_semantic/resources/mdtest/diagnostics/invalid_assignment.md

@@ -0,0 +1,28 @@
+# Invalid assignment diagnostics
+
+<!-- snapshot-diagnostics -->
+
+## Annotated assignment
+
+```py
+x: int = "three"  # error: [invalid-assignment]
+```
+
+## Unannotated assignment
+
+```py
+x: int
+x = "three"  # error: [invalid-assignment]
+```
+
+## Named expression
+
+```py
+x: int
+
+(x := "three")  # error: [invalid-assignment]
+```
+
+## Shadowing of classes and functions
+
+See [shadowing.md](./shadowing.md).

crates/ty_python_semantic/resources/mdtest/snapshots/invalid_assignment.m…_-_Invalid_assignment_d…_-_Annotated_assignment_(4b799ca1eeb857b9).snap

@@ -0,0 +1,31 @@
+---
+source: crates/ty_test/src/lib.rs
+expression: snapshot
+---
+---
+mdtest name: invalid_assignment.md - Invalid assignment diagnostics - Annotated assignment
+mdtest path: crates/ty_python_semantic/resources/mdtest/diagnostics/invalid_assignment.md
+---
+
+# Python source files
+
+## mdtest_snippet.py
+
+```
+1 | x: int = "three"  # error: [invalid-assignment]
+```
+
+# Diagnostics
+
+```
+error[invalid-assignment]: Object of type `Literal["three"]` is not assignable to `int`
+ --> src/mdtest_snippet.py:1:4
+  |
+1 | x: int = "three"  # error: [invalid-assignment]
+  |    ---   ^^^^^^^ Incompatible value of type `Literal["three"]`
+  |    |
+  |    Declared type
+  |
+info: rule `invalid-assignment` is enabled by default
+
+```

crates/ty_python_semantic/resources/mdtest/snapshots/invalid_assignment.m…_-_Invalid_assignment_d…_-_Named_expression_(35c120b3bd9929f8).snap

@@ -0,0 +1,35 @@
+---
+source: crates/ty_test/src/lib.rs
+expression: snapshot
+---
+---
+mdtest name: invalid_assignment.md - Invalid assignment diagnostics - Named expression
+mdtest path: crates/ty_python_semantic/resources/mdtest/diagnostics/invalid_assignment.md
+---
+
+# Python source files
+
+## mdtest_snippet.py
+
+```
+1 | x: int
+2 | 
+3 | (x := "three")  # error: [invalid-assignment]
+```
+
+# Diagnostics
+
+```
+error[invalid-assignment]: Object of type `Literal["three"]` is not assignable to `int`
+ --> src/mdtest_snippet.py:3:2
+  |
+1 | x: int
+2 |
+3 | (x := "three")  # error: [invalid-assignment]
+  |  -    ^^^^^^^ Incompatible value of type `Literal["three"]`
+  |  |
+  |  Declared type `int`
+  |
+info: rule `invalid-assignment` is enabled by default
+
+```

crates/ty_python_semantic/resources/mdtest/snapshots/invalid_assignment.m…_-_Invalid_assignment_d…_-_Unannotated_assignme…_(67e4b9239d5681a).snap

@@ -0,0 +1,33 @@
+---
+source: crates/ty_test/src/lib.rs
+expression: snapshot
+---
+---
+mdtest name: invalid_assignment.md - Invalid assignment diagnostics - Unannotated assignment
+mdtest path: crates/ty_python_semantic/resources/mdtest/diagnostics/invalid_assignment.md
+---
+
+# Python source files
+
+## mdtest_snippet.py
+
+```
+1 | x: int
+2 | x = "three"  # error: [invalid-assignment]
+```
+
+# Diagnostics
+
+```
+error[invalid-assignment]: Object of type `Literal["three"]` is not assignable to `int`
+ --> src/mdtest_snippet.py:2:1
+  |
+1 | x: int
+2 | x = "three"  # error: [invalid-assignment]
+  | -   ^^^^^^^ Incompatible value of type `Literal["three"]`
+  | |
+  | Declared type `int`
+  |
+info: rule `invalid-assignment` is enabled by default
+
+```

crates/ty_python_semantic/resources/mdtest/snapshots/shadowing.md_-_Shadowing_-_Implicit_class_shado…_(c8ff9e3a079e8bd5).snap

@@ -26,7 +26,9 @@ error[invalid-assignment]: Implicit shadowing of class `C`
 1 | class C: ...
 2 |
 3 | C = 1  # error: [invalid-assignment]
-  | ^
+  | -   ^ Incompatible value of type `Literal[1]`
+  | |
+  | Declared type `<class 'C'>`
   |
 info: Annotate to make it explicit if this is intentional
 info: rule `invalid-assignment` is enabled by default

crates/ty_python_semantic/resources/mdtest/snapshots/shadowing.md_-_Shadowing_-_Implicit_function_sh…_(a1515328b775ebc1).snap

@@ -26,7 +26,9 @@ error[invalid-assignment]: Implicit shadowing of function `f`
 1 | def f(): ...
 2 |
 3 | f = 1  # error: [invalid-assignment]
-  | ^
+  | -   ^ Incompatible value of type `Literal[1]`
+  | |
+  | Declared type `def f() -> Unknown`
   |
 info: Annotate to make it explicit if this is intentional
 info: rule `invalid-assignment` is enabled by default

crates/ty_python_semantic/resources/mdtest/stubs/ellipsis.md

@@ -59,10 +59,10 @@ In a non-stub file, there's no special treatment of ellipsis literals. An ellips
 be assigned if `EllipsisType` is actually assignable to the annotated type.
 
 ```py
-# error: 7 [invalid-parameter-default] "Default value of type `EllipsisType` is not assignable to annotated parameter type `int`"
+# error: [invalid-parameter-default] "Default value of type `EllipsisType` is not assignable to annotated parameter type `int`"
 def f(x: int = ...) -> None: ...
 
-# error: 1 [invalid-assignment] "Object of type `EllipsisType` is not assignable to `int`"
+# error: [invalid-assignment] "Object of type `EllipsisType` is not assignable to `int`"
 a: int = ...
 b = ...
 reveal_type(b)  # revealed: EllipsisType
@@ -73,6 +73,6 @@ reveal_type(b)  # revealed: EllipsisType
 There is no special treatment of the builtin name `Ellipsis` in stubs, only of `...` literals.
 
 ```pyi
-# error: 7 [invalid-parameter-default] "Default value of type `EllipsisType` is not assignable to annotated parameter type `int`"
+# error: [invalid-parameter-default] "Default value of type `EllipsisType` is not assignable to annotated parameter type `int`"
 def f(x: int = Ellipsis) -> None: ...
 ```

crates/ty_python_semantic/src/types/diagnostic.rs

@@ -2068,76 +2068,106 @@ pub(crate) fn is_invalid_typed_dict_literal(
         && matches!(source, AnyNodeRef::ExprDict(_))
 }
 
-fn report_invalid_assignment_with_message(
-    context: &InferContext,
+fn report_invalid_assignment_with_message<'db, 'ctx: 'db>(
+    context: &'ctx InferContext,
     node: AnyNodeRef,
-    target_ty: Type,
+    target_ty: Type<'db>,
     message: std::fmt::Arguments,
-) {
-    let Some(builder) = context.report_lint(&INVALID_ASSIGNMENT, node) else {
-        return;
-    };
+) -> Option<LintDiagnosticGuard<'db, 'ctx>> {
+    let builder = context.report_lint(&INVALID_ASSIGNMENT, node)?;
     match target_ty {
         Type::ClassLiteral(class) => {
             let mut diag = builder.into_diagnostic(format_args!(
                 "Implicit shadowing of class `{}`",
                 class.name(context.db()),
             ));
             diag.info("Annotate to make it explicit if this is intentional");
+            Some(diag)
         }
         Type::FunctionLiteral(function) => {
             let mut diag = builder.into_diagnostic(format_args!(
                 "Implicit shadowing of function `{}`",
                 function.name(context.db()),
             ));
             diag.info("Annotate to make it explicit if this is intentional");
+            Some(diag)
         }
+
         _ => {
-            builder.into_diagnostic(message);
+            let diag = builder.into_diagnostic(message);
+            Some(diag)
         }
     }
 }
 
 pub(super) fn report_invalid_assignment<'db>(
     context: &InferContext<'db, '_>,
-    node: AnyNodeRef,
+    target_node: AnyNodeRef,
     definition: Definition<'db>,
     target_ty: Type,
-    mut source_ty: Type<'db>,
+    mut value_ty: Type<'db>,
 ) {
-    let value_expr = match definition.kind(context.db()) {
+    let definition_kind = definition.kind(context.db());
+    let value_node = match definition_kind {
         DefinitionKind::Assignment(def) => Some(def.value(context.module())),
         DefinitionKind::AnnotatedAssignment(def) => def.value(context.module()),
         DefinitionKind::NamedExpression(def) => Some(&*def.node(context.module()).value),
         _ => None,
     };
 
-    if let Some(value_expr) = value_expr
-        && is_invalid_typed_dict_literal(context.db(), target_ty, value_expr.into())
+    if let Some(value_node) = value_node
+        && is_invalid_typed_dict_literal(context.db(), target_ty, value_node.into())
     {
         return;
     }
 
     let settings =
-        DisplaySettings::from_possibly_ambiguous_type_pair(context.db(), target_ty, source_ty);
+        DisplaySettings::from_possibly_ambiguous_type_pair(context.db(), target_ty, value_ty);
 
-    if let Some(value_expr) = value_expr {
+    if let Some(value_node) = value_node {
         // Re-infer the RHS of the annotated assignment, ignoring the type context for more precise
         // error messages.
-        source_ty =
-            infer_isolated_expression(context.db(), definition.scope(context.db()), value_expr);
+        value_ty =
+            infer_isolated_expression(context.db(), definition.scope(context.db()), value_node);
     }
 
-    report_invalid_assignment_with_message(
+    let Some(mut diag) = report_invalid_assignment_with_message(
         context,
-        node,
+        value_node.map(AnyNodeRef::from).unwrap_or(target_node),
         target_ty,
         format_args!(
             "Object of type `{}` is not assignable to `{}`",
-            source_ty.display_with(context.db(), settings.clone()),
+            value_ty.display_with(context.db(), settings.clone()),
             target_ty.display_with(context.db(), settings)
         ),
-    );
+    ) else {
+        return;
+    };
+
+    if value_node.is_some() {
+        match definition_kind {
+            DefinitionKind::AnnotatedAssignment(assignment) => {
+                // For annotated assignments, just point to the annotation in the source code.
+                diag.annotate(
+                    context
+                        .secondary(assignment.annotation(context.module()))
+                        .message("Declared type"),
+                );
+            }
+            _ => {
+                // Otherwise, annotate the target with its declared type.
+                diag.annotate(context.secondary(target_node).message(format_args!(
+                    "Declared type `{}`",
+                    target_ty.display(context.db()),
+                )));
+            }
+        }
+
+        diag.set_primary_message(format_args!(
+            "Incompatible value of type `{}`",
+            value_ty.display(context.db()),
+        ));
+    }
 }
 
 pub(super) fn report_invalid_attribute_assignment(

crates/ty_python_semantic/src/types/infer/builder.rs

@@ -7744,7 +7744,7 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> {
 
         self.infer_expression(target, TypeContext::default());
 
-        self.add_binding(named.into(), definition, |builder, tcx| {
+        self.add_binding(named.target.as_ref().into(), definition, |builder, tcx| {
             builder.infer_expression(value, tcx)
         })
     }