Skip to content

Commit 0b4404e

Browse files
grievejiafacebook-github-bot
authored andcommitted
Perform implicit return validation even when untyped-def-behavior is set to check-and-infer-return-any
Summary: When `untyped-def-behavior` is set to `check-and-infer-return-any` and a given function has an explicit return annotation, Pyrefly currently refuses to validate implicit return type against that annotation. e.g. ``` def foo() -> int: pass # No error! ``` It turns out that pre-existing logic conflates the two concept of "whether to infer return type" with "whether an implicit return type exists". As a result when return type inference is disabled, we disable implicit return validation as well which is not good. Let's separate those two pieces of info out! Reviewed By: rchen152 Differential Revision: D81830223 fbshipit-source-id: a1cb04002fce42a008ab84c83bd736ca77b71926
1 parent bca51e1 commit 0b4404e

File tree

2 files changed

+37
-15
lines changed

2 files changed

+37
-15
lines changed

pyrefly/lib/binding/function.rs

Lines changed: 34 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -327,17 +327,14 @@ impl<'a> BindingsBuilder<'a> {
327327
}
328328

329329
/// Handles both checking yield / return expressions and binding the return type.
330-
///
331-
/// The `implicit_return_if_inferring_return_type` argument should be None when
332-
/// return type inference is disabled; it must be `Some(implicit_return_key)` to
333-
/// get return type inference.
334330
fn analyze_return_type(
335331
&mut self,
336332
func_name: &Identifier,
337333
is_async: bool,
338334
yields_and_returns: YieldsAndReturns,
339335
return_ann_with_range: Option<(TextRange, Idx<KeyAnnotation>)>,
340-
implicit_return_if_inferring_return_type: Option<Idx<Key>>,
336+
implicit_return: Option<Idx<Key>>,
337+
should_infer_return_type: bool,
341338
stub_or_impl: FunctionStubOrImpl,
342339
decorators: Box<[(Idx<Key>, TextRange)]>,
343340
) {
@@ -377,10 +374,7 @@ impl<'a> BindingsBuilder<'a> {
377374
.into_boxed_slice();
378375

379376
let return_type_binding = {
380-
let kind = match (
381-
return_ann_with_range,
382-
implicit_return_if_inferring_return_type,
383-
) {
377+
let kind = match (return_ann_with_range, implicit_return) {
384378
(Some((range, annotation)), Some(implicit_return)) => {
385379
// We have an explicit return annotation and we want to validate it.
386380
ReturnTypeKind::ShouldValidateAnnotation {
@@ -400,7 +394,7 @@ impl<'a> BindingsBuilder<'a> {
400394
is_generator: !(yield_keys.is_empty() && yield_from_keys.is_empty()),
401395
}
402396
}
403-
(None, Some(implicit_return)) => {
397+
(None, Some(implicit_return)) if should_infer_return_type => {
404398
// We don't have an explicit return annotation, but we want to infer it.
405399
ReturnTypeKind::ShouldInferType {
406400
returns: return_keys,
@@ -409,8 +403,9 @@ impl<'a> BindingsBuilder<'a> {
409403
yield_froms: yield_from_keys,
410404
}
411405
}
412-
(None, None) => {
413-
// We don't have an explicit return annotation, and we want to just treat it as returning `Any`.
406+
(None, _) => {
407+
// We don't have an explicit return annotation, or we don't want to infer return type.
408+
// Just treat the return type as `Any`.
414409
ReturnTypeKind::ShouldReturnAny {
415410
is_generator: !(yield_keys.is_empty() && yield_from_keys.is_empty()),
416411
}
@@ -494,8 +489,7 @@ impl<'a> BindingsBuilder<'a> {
494489
)
495490
} else {
496491
match self.untyped_def_behavior {
497-
UntypedDefBehavior::SkipAndInferReturnAny
498-
| UntypedDefBehavior::CheckAndInferReturnAny => {
492+
UntypedDefBehavior::SkipAndInferReturnAny => {
499493
let (yields_and_returns, self_assignments) = self.function_body_scope(
500494
parameters,
501495
body,
@@ -510,7 +504,31 @@ impl<'a> BindingsBuilder<'a> {
510504
is_async,
511505
yields_and_returns,
512506
return_ann_with_range,
513-
None, // this disables return type inference
507+
None,
508+
false, // this disables return type inference
509+
stub_or_impl,
510+
decorators.decorators.clone(),
511+
);
512+
self_assignments
513+
}
514+
UntypedDefBehavior::CheckAndInferReturnAny => {
515+
let implicit_return = self.implicit_return(&body, func_name);
516+
let (yields_and_returns, self_assignments) = self.function_body_scope(
517+
parameters,
518+
body,
519+
range,
520+
func_name,
521+
undecorated_idx,
522+
class_key,
523+
is_async,
524+
);
525+
self.analyze_return_type(
526+
func_name,
527+
is_async,
528+
yields_and_returns,
529+
return_ann_with_range,
530+
Some(implicit_return),
531+
false, // this disables return type inference
514532
stub_or_impl,
515533
decorators.decorators.clone(),
516534
);
@@ -533,6 +551,7 @@ impl<'a> BindingsBuilder<'a> {
533551
yields_and_returns,
534552
return_ann_with_range,
535553
Some(implicit_return),
554+
true,
536555
stub_or_impl,
537556
decorators.decorators.clone(),
538557
);

pyrefly/lib/test/untyped_def_behaviors.rs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -131,6 +131,9 @@ from typing import assert_type, Any, Callable, Coroutine, Generator, AsyncGenera
131131
def simple_return() -> int:
132132
return "oops" # E: Returned type `Literal['oops']` is not assignable to declared return type `int`
133133
134+
def simple_implicit_return() -> int: # E: missing an explicit `return`
135+
pass
136+
134137
def generator_with_return() -> Generator[int, Any, str]:
135138
# TODO(stroxler): this yield error message needs some wordsmithing!
136139
yield "oops" # E: Type of yielded value `Literal['oops']` is not assignable to declared return type `int`

0 commit comments

Comments
 (0)