red_knot_python_semantic: replace one use of "old" secondary diagnostic messages
This is the first use of the new `lint()` reporter. I somewhat skipped a step here and also modified the actual diagnostic message itself. The snapshots should tell the story. We couldn't do this before because we had no way of differentiating between "message for the diagnostic as a whole" and "message for a specific code annotation." Now we can, so we can write more precise messages based on the assumption that users are also seeing the code snippet. The downside here is that the actual message text can become quite vague in the absence of the code snippet. This occurs, for example, with concise diagnostic formatting. It's unclear if we should do anything about it. I don't really see a way to make it better that doesn't involve creating diagnostics with messages for each mode, which I think would be a major PITA. The upside is that this code gets a bit simpler, and we very specifically avoid doing extra work if this specific lint is disabled.
This commit is contained in:
committed by
Andrew Gallant
parent
ba408f4231
commit
7e2eb591bc
@@ -21,7 +21,7 @@ use crate::types::{
|
||||
todo_type, BoundMethodType, FunctionDecorators, KnownClass, KnownFunction, KnownInstanceType,
|
||||
MethodWrapperKind, PropertyInstanceType, UnionType, WrapperDescriptorKind,
|
||||
};
|
||||
use ruff_db::diagnostic::{OldSecondaryDiagnosticMessage, Span};
|
||||
use ruff_db::diagnostic::{Annotation, Severity, Span, SubDiagnostic};
|
||||
use ruff_python_ast as ast;
|
||||
use ruff_text_size::Ranged;
|
||||
|
||||
@@ -1183,15 +1183,23 @@ pub(crate) enum BindingError<'db> {
|
||||
}
|
||||
|
||||
impl<'db> BindingError<'db> {
|
||||
/// Returns a tuple of two spans. The first is
|
||||
/// the span for the identifier of the function
|
||||
/// definition for `callable_ty`. The second is
|
||||
/// the span for the parameter in the function
|
||||
/// definition for `callable_ty`.
|
||||
///
|
||||
/// If there are no meaningful spans, then this
|
||||
/// returns `None`.
|
||||
fn parameter_span_from_index(
|
||||
db: &'db dyn Db,
|
||||
callable_ty: Type<'db>,
|
||||
parameter_index: usize,
|
||||
) -> Option<Span> {
|
||||
) -> Option<(Span, Span)> {
|
||||
match callable_ty {
|
||||
Type::FunctionLiteral(function) => {
|
||||
let function_scope = function.body_scope(db);
|
||||
let mut span = Span::from(function_scope.file(db));
|
||||
let span = Span::from(function_scope.file(db));
|
||||
let node = function_scope.node(db);
|
||||
if let Some(func_def) = node.as_function() {
|
||||
let range = func_def
|
||||
@@ -1200,8 +1208,9 @@ impl<'db> BindingError<'db> {
|
||||
.nth(parameter_index)
|
||||
.map(|param| param.range())
|
||||
.unwrap_or(func_def.parameters.range);
|
||||
span = span.with_range(range);
|
||||
Some(span)
|
||||
let name_span = span.clone().with_range(func_def.name.range);
|
||||
let parameter_span = span.with_range(range);
|
||||
Some((name_span, parameter_span))
|
||||
} else {
|
||||
None
|
||||
}
|
||||
@@ -1229,32 +1238,29 @@ impl<'db> BindingError<'db> {
|
||||
expected_ty,
|
||||
provided_ty,
|
||||
} => {
|
||||
let mut messages = vec![];
|
||||
if let Some(span) =
|
||||
Self::parameter_span_from_index(context.db(), callable_ty, parameter.index)
|
||||
{
|
||||
messages.push(OldSecondaryDiagnosticMessage::new(
|
||||
span,
|
||||
"parameter declared in function definition here",
|
||||
));
|
||||
}
|
||||
let Some(builder) = context.lint(&INVALID_ARGUMENT_TYPE) else {
|
||||
return;
|
||||
};
|
||||
|
||||
let provided_ty_display = provided_ty.display(context.db());
|
||||
let expected_ty_display = expected_ty.display(context.db());
|
||||
context.report_lint_with_secondary_messages(
|
||||
&INVALID_ARGUMENT_TYPE,
|
||||
Self::get_node(node, *argument_index),
|
||||
format_args!(
|
||||
"Object of type `{provided_ty_display}` cannot be assigned to \
|
||||
parameter {parameter}{}; expected type `{expected_ty_display}`",
|
||||
if let Some(CallableDescription { kind, name }) = callable_description {
|
||||
format!(" of {kind} `{name}`")
|
||||
} else {
|
||||
String::new()
|
||||
}
|
||||
),
|
||||
&messages,
|
||||
);
|
||||
let mut reporter = builder.build("Argument to this function is incorrect");
|
||||
|
||||
let diag = reporter.diagnostic();
|
||||
let span = context.span(Self::get_node(node, *argument_index));
|
||||
diag.annotate(Annotation::primary(span).message(format_args!(
|
||||
"Expected `{expected_ty_display}`, found `{provided_ty_display}`"
|
||||
)));
|
||||
if let Some((name_span, parameter_span)) =
|
||||
Self::parameter_span_from_index(context.db(), callable_ty, parameter.index)
|
||||
{
|
||||
let mut sub = SubDiagnostic::new(Severity::Info, "Function defined here");
|
||||
sub.annotate(Annotation::primary(name_span));
|
||||
sub.annotate(
|
||||
Annotation::secondary(parameter_span).message("Parameter declared here"),
|
||||
);
|
||||
diag.sub(sub);
|
||||
}
|
||||
}
|
||||
|
||||
Self::TooManyPositionalArguments {
|
||||
|
||||
Reference in New Issue
Block a user