Cyclic0007/ruff
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

[ty] Better invalid-assignment diagnostics (#21476)

Browse Source 
## Summary

Improve the diagnostic range for `invalid-assignment` diagnostics, and
add source annotations for the value and target type.

closes https://github.com/astral-sh/ty/issues/1556

### Before

<img width="836" height="601" alt="image"
src="https://github.com/user-attachments/assets/a48219bb-58a8-4a83-b290-d09ef50ce5f0"
/>

### After

<img width="857" height="742" alt="image"
src="https://github.com/user-attachments/assets/cfcaa4f4-94fb-459e-8d64-97050dfecb50"
/>

## Ecosystem impact

Very good! Due to the wider diagnostic range, we now pick up more `#
type: ignore` directives that were supposed to suppress an invalid
assignment diagnostic.

## Test Plan

New snapshot tests
This commit is contained in:
David Peter
2025-11-18 14:31:04 +01:00
committed by GitHub
parent 7a739d6b76
commit 5ca9c15fc8
16 changed files with 450 additions and 102 deletions
Download Patch File Download Diff File Expand all files Collapse all files

99
crates/ty_python_semantic/src/types/diagnostic.rs
View File

@@ -31,8 +31,11 @@ use crate::{
};
use itertools::Itertools;
use ruff_db::diagnostic::{Annotation, Diagnostic, Span, SubDiagnostic, SubDiagnosticSeverity};
use ruff_db::source::source_text;
use ruff_python_ast::name::Name;
use ruff_python_ast::parenthesize::parentheses_iterator;
use ruff_python_ast::{self as ast, AnyNodeRef, Identifier};
use ruff_python_trivia::CommentRanges;
use ruff_text_size::{Ranged, TextRange};
use rustc_hash::FxHashSet;
use std::fmt::Formatter;
@@ -2068,15 +2071,13 @@ pub(crate) fn is_invalid_typed_dict_literal(
&& matches!(source, AnyNodeRef::ExprDict(_))
}
fn report_invalid_assignment_with_message(
context: &InferContext,
node: AnyNodeRef,
target_ty: Type,
fn report_invalid_assignment_with_message<'db, 'ctx: 'db, T: Ranged>(
context: &'ctx InferContext,
node: T,
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!(
@@ -2084,6 +2085,7 @@ fn report_invalid_assignment_with_message(
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!(
@@ -2091,53 +2093,106 @@ fn report_invalid_assignment_with_message(
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 diagnostic_range = if let Some(value_node) = value_node {
// Expand the range to include parentheses around the value, if any. This allows
// invalid-assignment diagnostics to be suppressed on the opening or closing parenthesis:
// ```py
// x: str = ( # ty: ignore <- here
// 1 + 2 + 3
// ) # ty: ignore <- or here
// ```
let comment_ranges = CommentRanges::from(context.module().tokens());
let source = source_text(context.db(), context.file());
parentheses_iterator(value_node.into(), None, &comment_ranges, &source)
.last()
.unwrap_or(value_node.range())
} else {
target_node.range()
};
let Some(mut diag) = report_invalid_assignment_with_message(
context,
node,
diagnostic_range,
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()),
));
// Overwrite the concise message to avoid showing the value type twice
let message = diag.primary_message().to_string();
diag.set_concise_message(message);
}
}
pub(super) fn report_invalid_attribute_assignment(

2
crates/ty_python_semantic/src/types/infer/builder.rs
View File

@@ -7761,7 +7761,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)
})
}