mirror of
https://github.com/astral-sh/ruff.git
synced 2025-08-04 10:48:32 +00:00
red_knot_python_semantic: remove the "old" secondary message type
This finally completes the deletion of all old diagnostic types. We do this by migrating the second (and last) use of secondary diagnostic messages: to highlight the return type of a function definition when its return value is inconsistent with the type. Like the last diagnostic, we do actually change the message here a bit. We don't need a sub-diagnostic here, and we can instead just add a secondary annotation to highlight the return type.
This commit is contained in:
Andrew Gallant
Andrew Gallant
parent
7e2eb591bc
commit
7d958a9ee5
10 changed files with 63 additions and 143 deletions
|
|
@ -310,7 +310,7 @@ def f(cond: bool) -> str:
|
|||
return "hello" if cond else NotImplemented
|
||||
|
||||
def f(cond: bool) -> int:
|
||||
# error: [invalid-return-type] "Object of type `Literal["hello"]` is not assignable to return type `int`"
|
||||
# error: [invalid-return-type] "Return type does not match returned value: Expected `int`, found `Literal["hello"]`"
|
||||
return "hello" if cond else NotImplemented
|
||||
```
|
||||
|
||||
|
|
|
|||
|
|
@ -107,7 +107,7 @@ def good_return[T: int](x: T) -> T:
|
|||
return x
|
||||
|
||||
def bad_return[T: int](x: T) -> T:
|
||||
# error: [invalid-return-type] "Object of type `int` is not assignable to return type `T`"
|
||||
# error: [invalid-return-type] "Return type does not match returned value: Expected `T`, found `int`"
|
||||
return x + 1
|
||||
```
|
||||
|
||||
|
|
@ -120,7 +120,7 @@ def different_types[T, S](cond: bool, t: T, s: S) -> T:
|
|||
if cond:
|
||||
return t
|
||||
else:
|
||||
# error: [invalid-return-type] "Object of type `S` is not assignable to return type `T`"
|
||||
# error: [invalid-return-type] "Return type does not match returned value: Expected `T`, found `S`"
|
||||
return s
|
||||
|
||||
def same_types[T](cond: bool, t1: T, t2: T) -> T:
|
||||
|
|
|
|||
|
|
@ -31,67 +31,56 @@ mdtest path: crates/red_knot_python_semantic/resources/mdtest/function/return_ty
|
|||
# Diagnostics
|
||||
|
||||
```
|
||||
error: lint:invalid-return-type
|
||||
--> /src/mdtest_snippet.py:6:16
|
||||
|
|
||||
4 | else:
|
||||
5 | # error: [invalid-return-type]
|
||||
6 | return 1
|
||||
| ^ Object of type `Literal[1]` is not assignable to return type `str`
|
||||
7 |
|
||||
8 | def f(cond: bool) -> str:
|
||||
|
|
||||
info
|
||||
error: lint:invalid-return-type: Return type does not match returned value
|
||||
--> /src/mdtest_snippet.py:1:22
|
||||
|
|
||||
1 | def f(cond: bool) -> str:
|
||||
| --- Return type is declared here as `str`
|
||||
| --- Expected `str` because of return type
|
||||
2 | if cond:
|
||||
3 | return "a"
|
||||
4 | else:
|
||||
5 | # error: [invalid-return-type]
|
||||
6 | return 1
|
||||
| ^ Expected `str`, found `Literal[1]`
|
||||
7 |
|
||||
8 | def f(cond: bool) -> str:
|
||||
|
|
||||
|
||||
```
|
||||
|
||||
```
|
||||
error: lint:invalid-return-type
|
||||
--> /src/mdtest_snippet.py:11:16
|
||||
|
|
||||
9 | if cond:
|
||||
10 | # error: [invalid-return-type]
|
||||
11 | return 1
|
||||
| ^ Object of type `Literal[1]` is not assignable to return type `str`
|
||||
12 | else:
|
||||
13 | # error: [invalid-return-type]
|
||||
|
|
||||
info
|
||||
error: lint:invalid-return-type: Return type does not match returned value
|
||||
--> /src/mdtest_snippet.py:8:22
|
||||
|
|
||||
6 | return 1
|
||||
7 |
|
||||
8 | def f(cond: bool) -> str:
|
||||
| --- Return type is declared here as `str`
|
||||
| --- Expected `str` because of return type
|
||||
9 | if cond:
|
||||
10 | # error: [invalid-return-type]
|
||||
11 | return 1
|
||||
| ^ Expected `str`, found `Literal[1]`
|
||||
12 | else:
|
||||
13 | # error: [invalid-return-type]
|
||||
|
|
||||
|
||||
```
|
||||
|
||||
```
|
||||
error: lint:invalid-return-type
|
||||
error: lint:invalid-return-type: Return type does not match returned value
|
||||
--> /src/mdtest_snippet.py:14:16
|
||||
|
|
||||
12 | else:
|
||||
13 | # error: [invalid-return-type]
|
||||
14 | return 2
|
||||
| ^ Object of type `Literal[2]` is not assignable to return type `str`
|
||||
| ^ Expected `str`, found `Literal[2]`
|
||||
|
|
||||
info
|
||||
--> /src/mdtest_snippet.py:8:22
|
||||
::: /src/mdtest_snippet.py:8:22
|
||||
|
|
||||
6 | return 1
|
||||
7 |
|
||||
8 | def f(cond: bool) -> str:
|
||||
| --- Return type is declared here as `str`
|
||||
| --- Expected `str` because of return type
|
||||
9 | if cond:
|
||||
10 | # error: [invalid-return-type]
|
||||
|
|
||||
|
|
|
|||
|
|
@ -40,23 +40,17 @@ mdtest path: crates/red_knot_python_semantic/resources/mdtest/function/return_ty
|
|||
# Diagnostics
|
||||
|
||||
```
|
||||
error: lint:invalid-return-type
|
||||
--> /src/mdtest_snippet.py:4:16
|
||||
|
|
||||
2 | if False:
|
||||
3 | # error: [invalid-return-type]
|
||||
4 | return 1
|
||||
| ^ Object of type `Literal[1]` is not assignable to return type `None`
|
||||
5 |
|
||||
6 | # error: [invalid-return-type]
|
||||
|
|
||||
info
|
||||
error: lint:invalid-return-type: Return type does not match returned value
|
||||
--> /src/mdtest_snippet.py:1:12
|
||||
|
|
||||
1 | def f() -> None:
|
||||
| ---- Return type is declared here as `None`
|
||||
| ---- Expected `None` because of return type
|
||||
2 | if False:
|
||||
3 | # error: [invalid-return-type]
|
||||
4 | return 1
|
||||
| ^ Expected `None`, found `Literal[1]`
|
||||
5 |
|
||||
6 | # error: [invalid-return-type]
|
||||
|
|
||||
|
||||
```
|
||||
|
|
|
|||
|
|
@ -47,49 +47,35 @@ error: lint:invalid-return-type
|
|||
```
|
||||
|
||||
```
|
||||
error: lint:invalid-return-type
|
||||
--> /src/mdtest_snippet.py:7:12
|
||||
|
|
||||
5 | def f() -> str:
|
||||
6 | # error: [invalid-return-type]
|
||||
7 | return 1
|
||||
| ^ Object of type `Literal[1]` is not assignable to return type `str`
|
||||
8 |
|
||||
9 | def f() -> int:
|
||||
|
|
||||
info
|
||||
error: lint:invalid-return-type: Return type does not match returned value
|
||||
--> /src/mdtest_snippet.py:5:12
|
||||
|
|
||||
3 | 1
|
||||
4 |
|
||||
5 | def f() -> str:
|
||||
| --- Return type is declared here as `str`
|
||||
| --- Expected `str` because of return type
|
||||
6 | # error: [invalid-return-type]
|
||||
7 | return 1
|
||||
| ^ Expected `str`, found `Literal[1]`
|
||||
8 |
|
||||
9 | def f() -> int:
|
||||
|
|
||||
|
||||
```
|
||||
|
||||
```
|
||||
error: lint:invalid-return-type
|
||||
--> /src/mdtest_snippet.py:11:5
|
||||
|
|
||||
9 | def f() -> int:
|
||||
10 | # error: [invalid-return-type]
|
||||
11 | return
|
||||
| ^^^^^^ Object of type `None` is not assignable to return type `int`
|
||||
12 |
|
||||
13 | from typing import TypeVar
|
||||
|
|
||||
info
|
||||
error: lint:invalid-return-type: Return type does not match returned value
|
||||
--> /src/mdtest_snippet.py:9:12
|
||||
|
|
||||
7 | return 1
|
||||
8 |
|
||||
9 | def f() -> int:
|
||||
| --- Return type is declared here as `int`
|
||||
| --- Expected `int` because of return type
|
||||
10 | # error: [invalid-return-type]
|
||||
11 | return
|
||||
| ^^^^^^ Expected `int`, found `None`
|
||||
12 |
|
||||
13 | from typing import TypeVar
|
||||
|
|
||||
|
||||
```
|
||||
|
|
|
|||
|
|
@ -30,23 +30,16 @@ mdtest path: crates/red_knot_python_semantic/resources/mdtest/function/return_ty
|
|||
# Diagnostics
|
||||
|
||||
```
|
||||
error: lint:invalid-return-type
|
||||
--> /src/mdtest_snippet.pyi:3:12
|
||||
|
|
||||
1 | def f() -> int:
|
||||
2 | # error: [invalid-return-type]
|
||||
3 | return ...
|
||||
| ^^^ Object of type `ellipsis` is not assignable to return type `int`
|
||||
4 |
|
||||
5 | # error: [invalid-return-type]
|
||||
|
|
||||
info
|
||||
error: lint:invalid-return-type: Return type does not match returned value
|
||||
--> /src/mdtest_snippet.pyi:1:12
|
||||
|
|
||||
1 | def f() -> int:
|
||||
| --- Return type is declared here as `int`
|
||||
| --- Expected `int` because of return type
|
||||
2 | # error: [invalid-return-type]
|
||||
3 | return ...
|
||||
| ^^^ Expected `int`, found `ellipsis`
|
||||
4 |
|
||||
5 | # error: [invalid-return-type]
|
||||
|
|
||||
|
||||
```
|
||||
|
|
|
|||
|
|
@ -2,9 +2,7 @@ use std::fmt;
|
|||
|
||||
use drop_bomb::DebugDropBomb;
|
||||
use ruff_db::{
|
||||
diagnostic::{
|
||||
Annotation, Diagnostic, DiagnosticId, OldSecondaryDiagnosticMessage, Severity, Span,
|
||||
},
|
||||
diagnostic::{Annotation, Diagnostic, DiagnosticId, Severity, Span},
|
||||
files::File,
|
||||
};
|
||||
use ruff_text_size::Ranged;
|
||||
|
|
@ -83,28 +81,12 @@ impl<'db> InferContext<'db> {
|
|||
message: fmt::Arguments,
|
||||
) where
|
||||
T: Ranged,
|
||||
{
|
||||
self.report_lint_with_secondary_messages(lint, ranged, message, &[]);
|
||||
}
|
||||
|
||||
/// Reports a lint located at `ranged`.
|
||||
pub(super) fn report_lint_with_secondary_messages<T>(
|
||||
&self,
|
||||
lint: &'static LintMetadata,
|
||||
ranged: T,
|
||||
message: fmt::Arguments,
|
||||
secondary_messages: &[OldSecondaryDiagnosticMessage],
|
||||
) where
|
||||
T: Ranged,
|
||||
{
|
||||
let Some(builder) = self.lint(lint) else {
|
||||
return;
|
||||
};
|
||||
let mut reporter = builder.build("");
|
||||
let diag = reporter.diagnostic();
|
||||
for secondary_msg in secondary_messages {
|
||||
diag.sub(secondary_msg.to_sub_diagnostic());
|
||||
}
|
||||
let span = Span::from(self.file).with_range(ranged.range());
|
||||
diag.annotate(Annotation::primary(span).message(message));
|
||||
}
|
||||
|
|
|
|||
|
|
@ -8,7 +8,7 @@ use crate::types::string_annotation::{
|
|||
RAW_STRING_TYPE_ANNOTATION,
|
||||
};
|
||||
use crate::types::{KnownInstanceType, Type};
|
||||
use ruff_db::diagnostic::{Diagnostic, OldSecondaryDiagnosticMessage, Span};
|
||||
use ruff_db::diagnostic::{Annotation, Diagnostic, Span};
|
||||
use ruff_python_ast::{self as ast, AnyNodeRef};
|
||||
use ruff_text_size::Ranged;
|
||||
use rustc_hash::FxHashSet;
|
||||
|
|
@ -1085,22 +1085,26 @@ pub(super) fn report_invalid_return_type(
|
|||
expected_ty: Type,
|
||||
actual_ty: Type,
|
||||
) {
|
||||
let Some(builder) = context.lint(&INVALID_RETURN_TYPE) else {
|
||||
return;
|
||||
};
|
||||
|
||||
let object_span = Span::from(context.file()).with_range(object_range.range());
|
||||
let return_type_span = Span::from(context.file()).with_range(return_type_range.range());
|
||||
context.report_lint_with_secondary_messages(
|
||||
&INVALID_RETURN_TYPE,
|
||||
object_range,
|
||||
format_args!(
|
||||
"Object of type `{}` is not assignable to return type `{}`",
|
||||
actual_ty.display(context.db()),
|
||||
expected_ty.display(context.db())
|
||||
),
|
||||
&[OldSecondaryDiagnosticMessage::new(
|
||||
return_type_span,
|
||||
format!(
|
||||
"Return type is declared here as `{}`",
|
||||
expected_ty.display(context.db())
|
||||
),
|
||||
)],
|
||||
|
||||
let mut reporter = builder.build("Return type does not match returned value");
|
||||
|
||||
let diag = reporter.diagnostic();
|
||||
diag.annotate(Annotation::primary(object_span).message(format_args!(
|
||||
"Expected `{expected_ty}`, found `{actual_ty}`",
|
||||
expected_ty = expected_ty.display(context.db()),
|
||||
actual_ty = actual_ty.display(context.db()),
|
||||
)));
|
||||
diag.annotate(
|
||||
Annotation::secondary(return_type_span).message(format_args!(
|
||||
"Expected `{expected_ty}` because of return type",
|
||||
expected_ty = expected_ty.display(context.db()),
|
||||
)),
|
||||
);
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -6,15 +6,10 @@ use ruff_annotate_snippets::Level as AnnotateLevel;
|
|||
use ruff_text_size::TextRange;
|
||||
|
||||
pub use self::render::DisplayDiagnostic;
|
||||
pub use crate::diagnostic::old::OldSecondaryDiagnosticMessage;
|
||||
use crate::files::File;
|
||||
use crate::Db;
|
||||
|
||||
use self::render::FileResolver;
|
||||
|
||||
// This module should not be exported. We are planning to migrate off
|
||||
// the APIs in this module.
|
||||
mod old;
|
||||
mod render;
|
||||
|
||||
/// A collection of information that can be rendered into a diagnostic.
|
||||
|
|
|
|||
|
|
@ -1,23 +0,0 @@
|
|||
use crate::diagnostic::{Annotation, Severity, Span, SubDiagnostic};
|
||||
|
||||
/// A single secondary message assigned to a `Diagnostic`.
|
||||
#[derive(Clone, Debug, Eq, PartialEq)]
|
||||
pub struct OldSecondaryDiagnosticMessage {
|
||||
span: Span,
|
||||
message: String,
|
||||
}
|
||||
|
||||
impl OldSecondaryDiagnosticMessage {
|
||||
pub fn new(span: Span, message: impl Into<String>) -> OldSecondaryDiagnosticMessage {
|
||||
OldSecondaryDiagnosticMessage {
|
||||
span,
|
||||
message: message.into(),
|
||||
}
|
||||
}
|
||||
|
||||
pub fn to_sub_diagnostic(&self) -> SubDiagnostic {
|
||||
let mut sub = SubDiagnostic::new(Severity::Info, "");
|
||||
sub.annotate(Annotation::secondary(self.span.clone()).message(&self.message));
|
||||
sub
|
||||
}
|
||||
}
|
||||
Loading…
Add table
Add a link
Reference in a new issue