From 695de4f27f66c4968b79d913826488deee19cb52 Mon Sep 17 00:00:00 2001 From: lipefree <43332207+lipefree@users.noreply.github.com> Date: Fri, 30 May 2025 01:17:18 +0200 Subject: [PATCH] [ty] Add diagnosis for function with no return statement but with return type annotation (#18359) ## Summary Partially implement https://github.com/astral-sh/ty/issues/538, ```py from pathlib import Path def setup_test_project(registry_name: str, registry_url: str, project_dir: str) -> Path: pyproject_file = Path(project_dir) / "pyproject.toml" pyproject_file.write_text("...", encoding="utf-8") ``` As no return statement is defined in the function `setup_test_project` with annotated return type `Path`, we provide the following diagnosis : - error[invalid-return-type]: Function **always** implicitly returns `None`, which is not assignable to return type `Path` with a subdiagnostic : - note: Consider changing your return annotation to `-> None` or adding a `return` statement ## Test Plan mdtests with snapshots to capture the subdiagnostic. I have to mention that existing snapshots were modified since they now fall in this category. --------- Co-authored-by: Carl Meyer --- .../resources/mdtest/function/return_type.md | 14 ++++++++ ...gnostics_for_`inv…_(35563a74094b14d5).snap | 3 +- ...alid_implicit_ret…_(393cb38bf7119649).snap | 3 +- ...alid_implicit_ret…_(3d2d19aa49b28f1c).snap | 34 +++++++++++++++++++ ...nvalid_return_type_(a91e0c67519cd77f).snap | 6 ++-- ...alid_return_type_…_(c3a523878447af6b).snap | 6 ++-- .../src/types/diagnostic.rs | 22 +++++++++--- crates/ty_python_semantic/src/types/infer.rs | 2 ++ 8 files changed, 80 insertions(+), 10 deletions(-) create mode 100644 crates/ty_python_semantic/resources/mdtest/snapshots/return_type.md_-_Function_return_type_-_Invalid_implicit_ret…_(3d2d19aa49b28f1c).snap diff --git a/crates/ty_python_semantic/resources/mdtest/function/return_type.md b/crates/ty_python_semantic/resources/mdtest/function/return_type.md index 93d4843b70..1ca2b10dca 100644 --- a/crates/ty_python_semantic/resources/mdtest/function/return_type.md +++ b/crates/ty_python_semantic/resources/mdtest/function/return_type.md @@ -278,6 +278,20 @@ def f(cond: bool) -> int: return 2 ``` +## Invalid implicit return type always None + + + +If the function has no `return` statement or if it has only bare `return` statement (no variable in +the return statement), then we show a diagnostic hint that the return annotation should be `-> None` +or a `return` statement should be added. + +```py +# error: [invalid-return-type] +def f() -> int: + print("hello") +``` + ## NotImplemented ### Default Python version diff --git a/crates/ty_python_semantic/resources/mdtest/snapshots/return_type.md_-_Function_return_type_-_Diagnostics_for_`inv…_(35563a74094b14d5).snap b/crates/ty_python_semantic/resources/mdtest/snapshots/return_type.md_-_Function_return_type_-_Diagnostics_for_`inv…_(35563a74094b14d5).snap index 67c96c3139..d9a0e67f6d 100644 --- a/crates/ty_python_semantic/resources/mdtest/snapshots/return_type.md_-_Function_return_type_-_Diagnostics_for_`inv…_(35563a74094b14d5).snap +++ b/crates/ty_python_semantic/resources/mdtest/snapshots/return_type.md_-_Function_return_type_-_Diagnostics_for_`inv…_(35563a74094b14d5).snap @@ -24,13 +24,14 @@ mdtest path: crates/ty_python_semantic/resources/mdtest/function/return_type.md # Diagnostics ``` -error[invalid-return-type]: Function can implicitly return `None`, which is not assignable to return type `str` +error[invalid-return-type]: Function always implicitly returns `None`, which is not assignable to return type `str` --> src/mdtest_snippet.py:7:25 | 6 | class Concrete(Abstract): 7 | def method(self) -> str: ... # error: [invalid-return-type] | ^^^ | +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: Class `Concrete` has `typing.Protocol` in its MRO, but it is not a protocol class info: Only classes that directly inherit from `typing.Protocol` or `typing_extensions.Protocol` are considered protocol classes diff --git a/crates/ty_python_semantic/resources/mdtest/snapshots/return_type.md_-_Function_return_type_-_Invalid_implicit_ret…_(393cb38bf7119649).snap b/crates/ty_python_semantic/resources/mdtest/snapshots/return_type.md_-_Function_return_type_-_Invalid_implicit_ret…_(393cb38bf7119649).snap index e77bd91b65..889888f0a3 100644 --- a/crates/ty_python_semantic/resources/mdtest/snapshots/return_type.md_-_Function_return_type_-_Invalid_implicit_ret…_(393cb38bf7119649).snap +++ b/crates/ty_python_semantic/resources/mdtest/snapshots/return_type.md_-_Function_return_type_-_Invalid_implicit_ret…_(393cb38bf7119649).snap @@ -71,7 +71,7 @@ info: rule `invalid-return-type` is enabled by default ``` ``` -error[invalid-return-type]: Function can implicitly return `None`, which is not assignable to return type `int` +error[invalid-return-type]: Function always implicitly returns `None`, which is not assignable to return type `int` --> src/mdtest_snippet.py:12:22 | 11 | # error: [invalid-return-type] @@ -80,6 +80,7 @@ error[invalid-return-type]: Function can implicitly return `None`, which is not 13 | if cond: 14 | raise ValueError() | +info: Consider changing the return annotation to `-> None` or adding a `return` statement info: rule `invalid-return-type` is enabled by default ``` diff --git a/crates/ty_python_semantic/resources/mdtest/snapshots/return_type.md_-_Function_return_type_-_Invalid_implicit_ret…_(3d2d19aa49b28f1c).snap b/crates/ty_python_semantic/resources/mdtest/snapshots/return_type.md_-_Function_return_type_-_Invalid_implicit_ret…_(3d2d19aa49b28f1c).snap new file mode 100644 index 0000000000..a4fd88d692 --- /dev/null +++ b/crates/ty_python_semantic/resources/mdtest/snapshots/return_type.md_-_Function_return_type_-_Invalid_implicit_ret…_(3d2d19aa49b28f1c).snap @@ -0,0 +1,34 @@ +--- +source: crates/ty_test/src/lib.rs +expression: snapshot +--- +--- +mdtest name: return_type.md - Function return type - Invalid implicit return type always None +mdtest path: crates/ty_python_semantic/resources/mdtest/function/return_type.md +--- + +# Python source files + +## mdtest_snippet.py + +``` +1 | # error: [invalid-return-type] +2 | def f() -> int: +3 | print("hello") +``` + +# Diagnostics + +``` +error[invalid-return-type]: Function always implicitly returns `None`, which is not assignable to return type `int` + --> src/mdtest_snippet.py:2:12 + | +1 | # error: [invalid-return-type] +2 | def f() -> int: + | ^^^ +3 | print("hello") + | +info: Consider changing the return annotation to `-> None` or adding a `return` statement +info: rule `invalid-return-type` is enabled by default + +``` diff --git a/crates/ty_python_semantic/resources/mdtest/snapshots/return_type.md_-_Function_return_type_-_Invalid_return_type_(a91e0c67519cd77f).snap b/crates/ty_python_semantic/resources/mdtest/snapshots/return_type.md_-_Function_return_type_-_Invalid_return_type_(a91e0c67519cd77f).snap index d452f17820..4f36ded376 100644 --- a/crates/ty_python_semantic/resources/mdtest/snapshots/return_type.md_-_Function_return_type_-_Invalid_return_type_(a91e0c67519cd77f).snap +++ b/crates/ty_python_semantic/resources/mdtest/snapshots/return_type.md_-_Function_return_type_-_Invalid_return_type_(a91e0c67519cd77f).snap @@ -35,7 +35,7 @@ mdtest path: crates/ty_python_semantic/resources/mdtest/function/return_type.md # Diagnostics ``` -error[invalid-return-type]: Function can implicitly return `None`, which is not assignable to return type `int` +error[invalid-return-type]: Function always implicitly returns `None`, which is not assignable to return type `int` --> src/mdtest_snippet.py:2:12 | 1 | # error: [invalid-return-type] @@ -43,6 +43,7 @@ error[invalid-return-type]: Function can implicitly return `None`, which is not | ^^^ 3 | 1 | +info: Consider changing the return annotation to `-> None` or adding a `return` statement info: rule `invalid-return-type` is enabled by default ``` @@ -84,13 +85,14 @@ info: rule `invalid-return-type` is enabled by default ``` ``` -error[invalid-return-type]: Function can implicitly return `None`, which is not assignable to return type `T` +error[invalid-return-type]: Function always implicitly returns `None`, which is not assignable to return type `T` --> src/mdtest_snippet.py:18:16 | 17 | # error: [invalid-return-type] 18 | def m(x: T) -> T: ... | ^ | +info: Consider changing the return annotation to `-> None` or adding a `return` statement info: rule `invalid-return-type` is enabled by default ``` diff --git a/crates/ty_python_semantic/resources/mdtest/snapshots/return_type.md_-_Function_return_type_-_Invalid_return_type_…_(c3a523878447af6b).snap b/crates/ty_python_semantic/resources/mdtest/snapshots/return_type.md_-_Function_return_type_-_Invalid_return_type_…_(c3a523878447af6b).snap index bf469ce066..6f834b61e3 100644 --- a/crates/ty_python_semantic/resources/mdtest/snapshots/return_type.md_-_Function_return_type_-_Invalid_return_type_…_(c3a523878447af6b).snap +++ b/crates/ty_python_semantic/resources/mdtest/snapshots/return_type.md_-_Function_return_type_-_Invalid_return_type_…_(c3a523878447af6b).snap @@ -46,7 +46,7 @@ info: rule `invalid-return-type` is enabled by default ``` ``` -error[invalid-return-type]: Function can implicitly return `None`, which is not assignable to return type `int` +error[invalid-return-type]: Function always implicitly returns `None`, which is not assignable to return type `int` --> src/mdtest_snippet.pyi:6:14 | 5 | # error: [invalid-return-type] @@ -55,12 +55,13 @@ error[invalid-return-type]: Function can implicitly return `None`, which is not 7 | print("...") 8 | ... | +info: Consider changing the return annotation to `-> None` or adding a `return` statement info: rule `invalid-return-type` is enabled by default ``` ``` -error[invalid-return-type]: Function can implicitly return `None`, which is not assignable to return type `int` +error[invalid-return-type]: Function always implicitly returns `None`, which is not assignable to return type `int` --> src/mdtest_snippet.pyi:11:14 | 10 | # error: [invalid-return-type] @@ -69,6 +70,7 @@ error[invalid-return-type]: Function can implicitly return `None`, which is not 12 | f"""{foo} is a function that ...""" 13 | ... | +info: Consider changing the return annotation to `-> None` or adding a `return` statement info: rule `invalid-return-type` is enabled by default ``` diff --git a/crates/ty_python_semantic/src/types/diagnostic.rs b/crates/ty_python_semantic/src/types/diagnostic.rs index 7943b53d76..e931013755 100644 --- a/crates/ty_python_semantic/src/types/diagnostic.rs +++ b/crates/ty_python_semantic/src/types/diagnostic.rs @@ -1685,15 +1685,29 @@ pub(super) fn report_implicit_return_type( expected_ty: Type, has_empty_body: bool, enclosing_class_of_method: Option, + no_return: bool, ) { let Some(builder) = context.report_lint(&INVALID_RETURN_TYPE, range) else { return; }; let db = context.db(); - let mut diagnostic = builder.into_diagnostic(format_args!( - "Function can implicitly return `None`, which is not assignable to return type `{}`", - expected_ty.display(db) - )); + + // If no return statement is defined in the function, then the function always returns `None` + let mut diagnostic = if no_return { + let mut diag = builder.into_diagnostic(format_args!( + "Function always implicitly returns `None`, which is not assignable to return type `{}`", + expected_ty.display(db), + )); + diag.info( + "Consider changing the return annotation to `-> None` or adding a `return` statement", + ); + diag + } else { + builder.into_diagnostic(format_args!( + "Function can implicitly return `None`, which is not assignable to return type `{}`", + expected_ty.display(db), + )) + }; if !has_empty_body { return; } diff --git a/crates/ty_python_semantic/src/types/infer.rs b/crates/ty_python_semantic/src/types/infer.rs index 6187e87ca1..50964ded3d 100644 --- a/crates/ty_python_semantic/src/types/infer.rs +++ b/crates/ty_python_semantic/src/types/infer.rs @@ -1872,12 +1872,14 @@ impl<'db> TypeInferenceBuilder<'db> { if use_def.can_implicit_return(self.db()) && !Type::none(self.db()).is_assignable_to(self.db(), declared_ty) { + let no_return = self.return_types_and_ranges.is_empty(); report_implicit_return_type( &self.context, returns.range(), declared_ty, has_empty_body, enclosing_class_context, + no_return, ); } }