Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -260,6 +260,11 @@ def f(cond: bool) -> int:

<!-- snapshot-diagnostics -->

```toml
[environment]
python-version = "3.12"
```

```py
# error: [invalid-return-type]
def f() -> int:
Expand All @@ -279,6 +284,18 @@ T = TypeVar("T")

# error: [invalid-return-type]
def m(x: T) -> T: ...

class A[T]: ...

def f() -> A[int]:
class A[T]: ...
return A[int]() # error: [invalid-return-type]

class B: ...

def g() -> B:
class B: ...
return B() # error: [invalid-return-type]
```

## Invalid return type in stub file
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,18 @@ mdtest path: crates/ty_python_semantic/resources/mdtest/function/return_type.md
16 |
17 | # error: [invalid-return-type]
18 | def m(x: T) -> T: ...
19 |
20 | class A[T]: ...
21 |
22 | def f() -> A[int]:
23 | class A[T]: ...
24 | return A[int]() # error: [invalid-return-type]
25 |
26 | class B: ...
27 |
28 | def g() -> B:
29 | class B: ...
30 | return B() # error: [invalid-return-type]
```

# Diagnostics
Expand Down Expand Up @@ -91,9 +103,45 @@ error[invalid-return-type]: Function always implicitly returns `None`, which is
17 | # error: [invalid-return-type]
18 | def m(x: T) -> T: ...
| ^
19 |
20 | class A[T]: ...
|
info: Consider changing the return annotation to `-> None` or adding a `return` statement
info: Only functions in stub files, methods on protocol classes, or methods with `@abstractmethod` are permitted to have empty bodies
info: rule `invalid-return-type` is enabled by default

```

```
error[invalid-return-type]: Return type does not match returned value
--> src/mdtest_snippet.py:22:12
|
20 | class A[T]: ...
21 |
22 | def f() -> A[int]:
| ------ Expected `mdtest_snippet.A[int]` because of return type
23 | class A[T]: ...
24 | return A[int]() # error: [invalid-return-type]
| ^^^^^^^^ expected `mdtest_snippet.A[int]`, found `mdtest_snippet.<locals of function 'f'>.A[int]`
25 |
26 | class B: ...
|
info: rule `invalid-return-type` is enabled by default

```

```
error[invalid-return-type]: Return type does not match returned value
--> src/mdtest_snippet.py:28:12
|
26 | class B: ...
27 |
28 | def g() -> B:
| - Expected `mdtest_snippet.B` because of return type
29 | class B: ...
30 | return B() # error: [invalid-return-type]
| ^^^ expected `mdtest_snippet.B`, found `mdtest_snippet.<locals of function 'g'>.B`
|
info: rule `invalid-return-type` is enabled by default

```
54 changes: 8 additions & 46 deletions crates/ty_python_semantic/src/types/diagnostic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ use crate::semantic_index::SemanticIndex;
use crate::semantic_index::definition::Definition;
use crate::semantic_index::place::{PlaceTable, ScopedPlaceId};
use crate::suppression::FileSuppressionId;
use crate::types::class::{ClassType, DisjointBase, DisjointBaseKind, Field};
use crate::types::class::{DisjointBase, DisjointBaseKind, Field};
use crate::types::function::KnownFunction;
use crate::types::string_annotation::{
BYTE_STRING_TYPE_ANNOTATION, ESCAPE_CHARACTER_IN_FORWARD_ANNOTATION, FSTRING_TYPE_ANNOTATION,
Expand Down Expand Up @@ -1945,18 +1945,8 @@ pub(super) fn report_invalid_assignment(
target_ty: Type,
source_ty: Type,
) {
let mut settings = DisplaySettings::default();
// Handles the situation where the report naming is confusing, such as class with the same Name,
// but from different scopes.
if let Some(target_class) = type_to_class_literal(target_ty, context.db()) {
if let Some(source_class) = type_to_class_literal(source_ty, context.db()) {
if target_class != source_class
&& target_class.name(context.db()) == source_class.name(context.db())
{
settings = settings.qualified();
}
}
}
let settings =
DisplaySettings::from_possibly_ambiguous_type_pair(context.db(), target_ty, source_ty);

report_invalid_assignment_with_message(
context,
Expand All @@ -1970,36 +1960,6 @@ pub(super) fn report_invalid_assignment(
);
}

// TODO: generalize this to a method that takes any two types, walks them recursively, and returns
// a set of types with ambiguous names whose display should be qualified. Then we can use this in
// any diagnostic that displays two types.
fn type_to_class_literal<'db>(ty: Type<'db>, db: &'db dyn crate::Db) -> Option<ClassLiteral<'db>> {
match ty {
Type::ClassLiteral(class) => Some(class),
Type::NominalInstance(instance) => match instance.class(db) {
crate::types::class::ClassType::NonGeneric(class) => Some(class),
crate::types::class::ClassType::Generic(alias) => Some(alias.origin(db)),
},
Type::EnumLiteral(enum_literal) => Some(enum_literal.enum_class(db)),
Type::GenericAlias(alias) => Some(alias.origin(db)),
Type::ProtocolInstance(ProtocolInstanceType {
inner: Protocol::FromClass(class),
..
}) => match class {
ClassType::NonGeneric(class) => Some(class),
ClassType::Generic(alias) => Some(alias.origin(db)),
},
Type::TypedDict(typed_dict) => match typed_dict.defining_class() {
ClassType::NonGeneric(class) => Some(class),
ClassType::Generic(alias) => Some(alias.origin(db)),
},
Type::SubclassOf(subclass_of) => {
type_to_class_literal(Type::from(subclass_of.subclass_of().into_class()?), db)
}
_ => None,
}
}

pub(super) fn report_invalid_attribute_assignment(
context: &InferContext,
node: AnyNodeRef,
Expand Down Expand Up @@ -2030,18 +1990,20 @@ pub(super) fn report_invalid_return_type(
return;
};

let settings =
DisplaySettings::from_possibly_ambiguous_type_pair(context.db(), expected_ty, actual_ty);
let return_type_span = context.span(return_type_range);

let mut diag = builder.into_diagnostic("Return type does not match returned value");
diag.set_primary_message(format_args!(
"expected `{expected_ty}`, found `{actual_ty}`",
expected_ty = expected_ty.display(context.db()),
actual_ty = actual_ty.display(context.db()),
expected_ty = expected_ty.display_with(context.db(), settings),
actual_ty = actual_ty.display_with(context.db(), settings),
));
diag.annotate(
Annotation::secondary(return_type_span).message(format_args!(
"Expected `{expected_ty}` because of return type",
expected_ty = expected_ty.display(context.db()),
expected_ty = expected_ty.display_with(context.db(), settings),
)),
);
}
Expand Down
Loading
Loading