diff --git a/crates/pyrefly_config/src/error_kind.rs b/crates/pyrefly_config/src/error_kind.rs index 3dea6f55f3..bfe9b22e62 100644 --- a/crates/pyrefly_config/src/error_kind.rs +++ b/crates/pyrefly_config/src/error_kind.rs @@ -154,6 +154,8 @@ pub enum ErrorKind { Deprecated, /// Division, floor division, or modulo by a literal zero value. DivisionByZero, + /// A function has an empty body despite declaring a non-None return type. + EmptyBody, /// Explicit usage of `typing.Any` in an annotation. ExplicitAny, /// Raised when a class implicitly becomes abstract by defining abstract members without diff --git a/crates/pyrefly_python/src/sys_info.rs b/crates/pyrefly_python/src/sys_info.rs index 8499559b2b..8b753a1145 100644 --- a/crates/pyrefly_python/src/sys_info.rs +++ b/crates/pyrefly_python/src/sys_info.rs @@ -520,6 +520,21 @@ impl SysInfo { x == "TYPE_CHECKING" || x == "TYPE_CHECKING_WITH_PYREFLY" } + /// Returns whether the expression is syntactically guarded by `TYPE_CHECKING`. + pub fn is_type_checking_guard(x: &Expr) -> bool { + match x { + Expr::Name(name) => Self::is_type_checking_constant_name(name.id()), + Expr::Attribute(ExprAttribute { value, attr, .. }) => { + value.is_name_expr() && Self::is_type_checking_constant_name(attr.as_str()) + } + Expr::BoolOp(x) => match x.op { + BoolOp::And => x.values.iter().any(Self::is_type_checking_guard), + BoolOp::Or => x.values.iter().all(Self::is_type_checking_guard), + }, + _ => false, + } + } + fn evaluate(self, x: &Expr) -> Option { match x { Expr::Compare(x) if x.ops.len() == 1 && x.comparators.len() == 1 => Some(Value::Bool( diff --git a/pyrefly/lib/alt/function.rs b/pyrefly/lib/alt/function.rs index 7e2369a435..abf6a64c1b 100644 --- a/pyrefly/lib/alt/function.rs +++ b/pyrefly/lib/alt/function.rs @@ -435,6 +435,7 @@ impl<'a, Ans: LookupAnswer> AnswersSolver<'a, Ans> { def: &FunctionDefData, def_index: FuncDefIndex, stub_or_impl: FunctionStubOrImpl, + has_ellipsis_body: bool, placeholder_body_kind: Option, is_return_inferred: bool, class_key: Option<&Idx>, @@ -635,6 +636,7 @@ impl<'a, Ans: LookupAnswer> AnswersSolver<'a, Ans> { params, paramspec, stub_or_impl, + has_ellipsis_body, defining_cls, resolved_param_types, }) @@ -659,6 +661,33 @@ impl<'a, Ans: LookupAnswer> AnswersSolver<'a, Ans> { format!("`{}` is missing a return annotation", stmt.name), ); } + let none_is_assignable_to_return = if stmt.is_async { + self.unwrap_coroutine(&ret) + .is_some_and(|(_, _, return_ty)| { + self.is_subset_eq(&self.heap.mk_none(), &return_ty) + }) + } else { + self.is_subset_eq(&self.heap.mk_none(), &ret) + }; + if def.has_ellipsis_body + && has_return_annotation + && !none_is_assignable_to_return + && !def.metadata.flags.defined_in_stub_file + && !def.metadata.flags.is_overload + && !def.metadata.flags.is_abstract_method + && !def + .defining_cls + .as_ref() + .is_some_and(|cls| self.get_metadata_for_class(cls).is_protocol()) + { + self.error( + errors, + stmt.name.range(), + ErrorKind::EmptyBody, + "Function body cannot consist only of `...` when the return type is not `None`" + .to_owned(), + ); + } // The first parameter of a non-static method is the implicit self/cls // parameter and does not require an annotation, regardless of its name. // __new__ is an implicit staticmethod but still takes cls as its first parameter. diff --git a/pyrefly/lib/alt/solve.rs b/pyrefly/lib/alt/solve.rs index bb519fd536..af11caad07 100644 --- a/pyrefly/lib/alt/solve.rs +++ b/pyrefly/lib/alt/solve.rs @@ -5415,6 +5415,7 @@ impl<'a, Ans: LookupAnswer> AnswersSolver<'a, Ans> { &x.def, x.def_index, x.stub_or_impl, + x.has_ellipsis_body, x.placeholder_body_kind, x.is_return_inferred, x.class_key.as_ref(), diff --git a/pyrefly/lib/alt/types/decorated_function.rs b/pyrefly/lib/alt/types/decorated_function.rs index 3f5756ea89..801591bef1 100644 --- a/pyrefly/lib/alt/types/decorated_function.rs +++ b/pyrefly/lib/alt/types/decorated_function.rs @@ -53,6 +53,7 @@ pub struct UndecoratedFunction { pub params: Vec, pub paramspec: Option, pub stub_or_impl: FunctionStubOrImpl, + pub has_ellipsis_body: bool, pub defining_cls: Option, /// Maps parameter names to their resolved types - used to connect /// FunctionParameter and KeyUndecoratedFunction. @@ -124,6 +125,7 @@ impl UndecoratedFunction { params: Vec::new(), paramspec: None, stub_or_impl: FunctionStubOrImpl::Stub, + has_ellipsis_body: false, defining_cls: None, resolved_param_types: SmallMap::new(), } diff --git a/pyrefly/lib/binding/binding.rs b/pyrefly/lib/binding/binding.rs index f179639d42..ba11afb9c0 100644 --- a/pyrefly/lib/binding/binding.rs +++ b/pyrefly/lib/binding/binding.rs @@ -136,7 +136,7 @@ assert_words!(BindingYield, 4); assert_words!(BindingYieldFrom, 4); assert_words!(BindingDecorator, 13); assert_bytes!(BindingDecoratedFunction, 20); -assert_words!(BindingUndecoratedFunction, 20); +assert_words!(BindingUndecoratedFunction, 21); #[derive(Clone, Dupe, Debug, PartialEq, Eq, PartialOrd, Ord, Hash)] pub enum AnyIdx { @@ -1859,6 +1859,7 @@ pub struct BindingUndecoratedFunction { pub def_index: FuncDefIndex, pub def: FunctionDefData, pub stub_or_impl: FunctionStubOrImpl, + pub has_ellipsis_body: bool, /// `Some` if the function body is a single placeholder statement /// (`raise NotImplementedError(...)` or `return NotImplemented`); `None` otherwise. pub placeholder_body_kind: Option, diff --git a/pyrefly/lib/binding/bindings.rs b/pyrefly/lib/binding/bindings.rs index fb603857b8..2bdcb64e0b 100644 --- a/pyrefly/lib/binding/bindings.rs +++ b/pyrefly/lib/binding/bindings.rs @@ -309,6 +309,7 @@ pub struct BindingsBuilder<'a> { /// set by `stmts()` and consumed by namedtuple synthesis in `stmt()`. pub adjacent_namedtuple_defaults: Option>, pub promote_ranges: SmallSet, + pub type_checking_depth: usize, } /// An enum tracking whether we are in a generator expression @@ -638,6 +639,7 @@ impl Bindings { subsequently_initialized: SmallSet::new(), adjacent_namedtuple_defaults: None, promote_ranges: SmallSet::new(), + type_checking_depth: 0, }; builder.init_static_scope(&x.body, true); if module_info.name() != ModuleName::builtins() { diff --git a/pyrefly/lib/binding/function.rs b/pyrefly/lib/binding/function.rs index 962ba035bb..15671bfadc 100644 --- a/pyrefly/lib/binding/function.rs +++ b/pyrefly/lib/binding/function.rs @@ -612,6 +612,7 @@ impl<'a> BindingsBuilder<'a> { class_key: Option>, ) -> ( FunctionStubOrImpl, + bool, Option, bool, Option, @@ -774,6 +775,7 @@ impl<'a> BindingsBuilder<'a> { ( stub_or_impl, + body_is_ellipse, placeholder_body_kind, is_return_inferred, self_assignments, @@ -889,19 +891,24 @@ impl<'a> BindingsBuilder<'a> { self.function_header(&mut x, &func_name, class_key, def_idx.usage(), parent); let docstring_range = Docstring::range_from_stmts(x.body.as_slice()); - let (stub_or_impl, placeholder_body_kind, is_return_inferred, self_assignments) = self - .function_body( - &mut x.parameters, - mem::take(&mut x.body), - &decorators, - x.range, - x.is_async, - return_ann_with_range, - &func_name, - parent, - undecorated_idx, - class_key, - ); + let ( + stub_or_impl, + has_ellipsis_body, + placeholder_body_kind, + is_return_inferred, + self_assignments, + ) = self.function_body( + &mut x.parameters, + mem::take(&mut x.body), + &decorators, + x.range, + x.is_async, + return_ann_with_range, + &func_name, + parent, + undecorated_idx, + class_key, + ); // Pop the annotation scope to get back to the parent scope, and handle this // case where we need to track assignments to `self` from methods. @@ -915,6 +922,7 @@ impl<'a> BindingsBuilder<'a> { def_index: func_def_index, def: FunctionDefData::new(x), stub_or_impl, + has_ellipsis_body: has_ellipsis_body && self.type_checking_depth == 0, placeholder_body_kind, is_return_inferred, class_key, diff --git a/pyrefly/lib/binding/stmt.rs b/pyrefly/lib/binding/stmt.rs index bf3cbe3953..39c5724700 100644 --- a/pyrefly/lib/binding/stmt.rs +++ b/pyrefly/lib/binding/stmt.rs @@ -10,6 +10,7 @@ use pyrefly_python::ast::Ast; use pyrefly_python::module_name::ModuleName; use pyrefly_python::nesting_context::NestingContext; use pyrefly_python::short_identifier::ShortIdentifier; +use pyrefly_python::sys_info::SysInfo; use ruff_python_ast::Arguments; use ruff_python_ast::AtomicNodeIndex; use ruff_python_ast::Expr; @@ -1257,6 +1258,8 @@ impl<'a> BindingsBuilder<'a> { } else { NarrowOps::from_expr(self, test.as_ref()) }; + let is_type_checking_branch = + test.as_ref().is_some_and(SysInfo::is_type_checking_guard); if let Some(test_expr) = test { // Typecheck the test condition during solving. self.insert_binding( @@ -1270,7 +1273,13 @@ impl<'a> BindingsBuilder<'a> { &Usage::Narrowing(None), ); negated_prev_ops.and_all(new_narrow_ops.negate()); - self.stmts(body, parent); + if is_type_checking_branch { + self.type_checking_depth += 1; + self.stmts(body, parent); + self.type_checking_depth -= 1; + } else { + self.stmts(body, parent); + } self.finish_branch(); if this_branch_chosen == Some(true) { exhaustive = true; diff --git a/pyrefly/lib/test/callable.rs b/pyrefly/lib/test/callable.rs index 63c3db0de1..a84b90c320 100644 --- a/pyrefly/lib/test/callable.rs +++ b/pyrefly/lib/test/callable.rs @@ -1033,14 +1033,46 @@ class Foo: testcase!( test_ellipsis_body, r#" -from typing import Any, assert_type +from typing import TYPE_CHECKING, Protocol, assert_type, overload +from abc import abstractmethod + def f(): ... -# This is technically wrong (`g()` returns `None`), but `...` is often used to stub out the bodies -# of things like overload signatures and abstractmethods. For simplicity, we just always allow this -# stubbing behavior. -def g() -> str: ... +def g() -> None: ... +def h() -> int | None: ... +def i() -> str: ... # E: Function body cannot consist only of `...` when the return type is not `None` + +async def j() -> None: ... +async def k() -> str: ... # E: Function body cannot consist only of `...` when the return type is not `None` + +if TYPE_CHECKING: + def tc() -> str: ... + +class P(Protocol): + def m(self) -> str: ... + +class A: + @abstractmethod + def m(self) -> str: ... + +@overload +def ov(x: int) -> int: ... +@overload +def ov(x: str) -> str: ... +def ov(x: int | str) -> int | str: + return x + assert_type(f(), None) -assert_type(g(), str) +assert_type(g(), None) + "#, +); + +testcase!( + test_ellipsis_body_in_pyi, + TestEnv::one_with_path("foo", "foo.pyi", "def f() -> int: ..."), + r#" +from typing import assert_type +from foo import f +assert_type(f(), int) "#, ); diff --git a/website/docs/error-kinds.mdx b/website/docs/error-kinds.mdx index 8da9026fc0..d364bdcf50 100644 --- a/website/docs/error-kinds.mdx +++ b/website/docs/error-kinds.mdx @@ -472,6 +472,20 @@ y = 10 // 0 # error: division by zero z = 10 % 0 # error: division by zero ``` +## empty-body + +Default severity: `error` + +A function body consists only of `...` even though the function declares a +return type that `None` is not assignable to. + +Empty ellipsis bodies are allowed in stub files, protocol methods, abstract +methods, overload declarations, and `if TYPE_CHECKING` blocks. + +```python +def f() -> int: ... # error: empty body +``` + ## explicit-any Default severity: `ignore`