Skip to content

Commit ca578d0

Browse files
asukaminato0721asukaminato0721
committed
comment
1 parent 521f586 commit ca578d0

9 files changed

Lines changed: 202 additions & 106 deletions

File tree

‎crates/pyrefly_config/src/error_kind.rs‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -252,8 +252,6 @@ pub enum ErrorKind {
252252
InconsistentOverloadDefault,
253253
/// Cannot assign to a class-body-declared instance variable via the class (e.g. `C.instance_var = ...`).
254254
InstanceVarAssign,
255-
/// A function declares a non-None, non-Never return type but can only loop forever.
256-
InfiniteLoop,
257255
/// Internal Pyrefly error.
258256
InternalError,
259257
/// An `@abstractmethod` is defined in a class that is not abstract.
@@ -446,6 +444,8 @@ pub enum ErrorKind {
446444
UnboundName,
447445
/// An error caused by a keyword argument used in the wrong place.
448446
UnexpectedKeyword,
447+
/// A function's inferred return type is `Never`, contrary to its annotation.
448+
UnexpectedNever,
449449
/// An error caused by passing a positional argument for a keyword-only parameter.
450450
UnexpectedPositionalArgument,
451451
/// Attempting to use a type checker directive without importing it from `typing`.
@@ -643,7 +643,7 @@ impl ErrorKind {
643643
ErrorKind::ImplicitlyDefinedAttribute => Severity::Ignore,
644644
ErrorKind::IncompatibleComparison => Severity::Ignore,
645645
ErrorKind::InstanceVarAssign => Severity::Ignore,
646-
ErrorKind::InfiniteLoop => Severity::Warn,
646+
ErrorKind::UnexpectedNever => Severity::Warn,
647647
ErrorKind::InvalidAbstractMethod => Severity::Ignore,
648648
ErrorKind::InvalidCast => Severity::Ignore,
649649
ErrorKind::InvalidDecorator => Severity::Warn,

‎pyrefly/lib/alt/solve.rs‎

Lines changed: 43 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ use pyrefly_graph::index::Idx;
1313
use pyrefly_python::ast::Ast;
1414
use pyrefly_python::dunder;
1515
use pyrefly_python::module_name::ModuleName;
16+
use pyrefly_python::module_path::ModuleStyle;
1617
use pyrefly_python::short_identifier::ShortIdentifier;
1718
use pyrefly_python::sys_info::SysInfo;
1819
use pyrefly_types::dimension::Int;
@@ -3094,8 +3095,8 @@ impl<'ctx, 'answer, Ans: LookupAnswer> AnswersSolver<'ctx, 'answer, Ans> {
30943095
implicit_return,
30953096
is_async,
30963097
is_generator,
3097-
has_explicit_return,
3098-
has_infinite_loop,
3098+
returns,
3099+
function,
30993100
} => {
31003101
let annotation = self.get_idx(*annotation).annotation.get_type().clone();
31013102
let implicit_return = self.get_idx(*implicit_return);
@@ -3104,11 +3105,49 @@ impl<'ctx, 'answer, Ans: LookupAnswer> AnswersSolver<'ctx, 'answer, Ans> {
31043105
&annotation,
31053106
*is_async,
31063107
*is_generator,
3107-
*has_explicit_return,
3108-
*has_infinite_loop,
3108+
!returns.is_empty(),
31093109
range,
31103110
errors,
31113111
);
3112+
if !is_generator
3113+
&& !annotation.is_never()
3114+
&& !annotation.is_none()
3115+
&& implicit_return.ty().is_never()
3116+
&& returns.iter().all(|key| self.get_idx(*key).ty().is_never())
3117+
{
3118+
let function = self.get_idx(*function);
3119+
let flags = &function.metadata.flags;
3120+
// Overrides keep the parent's return contract even when this body never returns.
3121+
let is_override = flags.is_override
3122+
|| function.defining_cls.as_ref().is_some_and(|cls| {
3123+
self.get_mro_for_class(cls)
3124+
.ancestors(self.stdlib)
3125+
.any(|base| {
3126+
self.get_class_member(
3127+
base.class_object(),
3128+
&function.metadata.kind.function_name(),
3129+
)
3130+
.is_some()
3131+
})
3132+
});
3133+
if !flags.facts().is_stub()
3134+
&& !flags.body_kind.is_placeholder_or_trivial()
3135+
&& !flags.is_overload
3136+
&& !flags.is_abstract_method
3137+
&& flags.module_style != ModuleStyle::Interface
3138+
&& !is_override
3139+
{
3140+
self.error(
3141+
errors,
3142+
range,
3143+
ErrorKind::UnexpectedNever,
3144+
format!(
3145+
"Function declared to return `{}` but can never return",
3146+
self.for_display(annotation.clone())
3147+
),
3148+
);
3149+
}
3150+
}
31123151
}
31133152
BindingExpect::MatchExhaustiveness {
31143153
subject_idx,
@@ -6947,27 +6986,9 @@ impl<'ctx, 'answer, Ans: LookupAnswer> AnswersSolver<'ctx, 'answer, Ans> {
69476986
is_async: bool,
69486987
is_generator: bool,
69496988
has_explicit_returns: bool,
6950-
has_infinite_loop: bool,
69516989
range: TextRange,
69526990
errors: &ErrorCollector,
69536991
) {
6954-
if has_infinite_loop
6955-
&& !has_explicit_returns
6956-
&& !is_generator
6957-
&& !annotation.is_never()
6958-
&& !annotation.is_none()
6959-
&& implicit_return.ty().is_never()
6960-
{
6961-
self.error(
6962-
errors,
6963-
range,
6964-
ErrorKind::InfiniteLoop,
6965-
format!(
6966-
"Function declared to return `{}` but can never return",
6967-
self.for_display(annotation.clone())
6968-
),
6969-
);
6970-
}
69716992
if is_async && is_generator {
69726993
let hints = self.decompose_hint(HintRef::soft(annotation), |hint| {
69736994
self.decompose_async_generator(hint)

‎pyrefly/lib/binding/binding.rs‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1294,15 +1294,15 @@ pub enum BindingExpect {
12941294
existing: Idx<KeyAnnotation>,
12951295
name: Name,
12961296
},
1297-
/// Validate a function implementation's implicit return without making the
1298-
/// published return type depend on the function body.
1297+
/// Validate a function's return annotation without making the published
1298+
/// return type depend on the function body.
12991299
ValidateImplicitReturn {
13001300
annotation: Idx<KeyAnnotation>,
13011301
implicit_return: Idx<Key>,
13021302
is_async: bool,
13031303
is_generator: bool,
1304-
has_explicit_return: bool,
1305-
has_infinite_loop: bool,
1304+
returns: Box<[Idx<Key>]>,
1305+
function: Idx<KeyUndecoratedFunction>,
13061306
},
13071307
/// Expression used in a boolean context (`bool()`, `if`, or `while`)
13081308
Bool(Expr),

‎pyrefly/lib/binding/function.rs‎

Lines changed: 23 additions & 58 deletions
Original file line numberDiff line numberDiff line change
@@ -485,10 +485,8 @@ impl<'a> BindingsBuilder<'a> {
485485
/// This function must not be called unless the function body statements will be bound;
486486
/// it relies on that binding to ensure we don't have a dangling `Idx<Key>` (which could lead
487487
/// to a panic).
488-
fn implicit_return(&mut self, body: &[Stmt], func_name: &Identifier) -> (Idx<Key>, bool) {
489-
let last_exprs = function_last_expressions(body, self.sys_info);
490-
let has_infinite_loop = last_exprs.as_ref().is_some_and(|x| x.has_infinite_loop);
491-
let last_exprs = last_exprs.map(|x| {
488+
fn implicit_return(&mut self, body: &[Stmt], func_name: &Identifier) -> Idx<Key> {
489+
let last_exprs = function_last_expressions(body, self.sys_info).map(|x| {
492490
x.clauses
493491
.into_map(|clause| {
494492
clause
@@ -497,12 +495,9 @@ impl<'a> BindingsBuilder<'a> {
497495
})
498496
.into_boxed_slice()
499497
});
500-
(
501-
self.insert_binding(
502-
Key::ReturnImplicit(ShortIdentifier::new(func_name)),
503-
Binding::ReturnImplicit(ReturnImplicit { last_exprs }),
504-
),
505-
has_infinite_loop,
498+
self.insert_binding(
499+
Key::ReturnImplicit(ShortIdentifier::new(func_name)),
500+
Binding::ReturnImplicit(ReturnImplicit { last_exprs }),
506501
)
507502
}
508503

@@ -514,7 +509,7 @@ impl<'a> BindingsBuilder<'a> {
514509
is_async: bool,
515510
yields_and_returns: YieldsAndReturns,
516511
return_ann_with_range: Option<(TextRange, Idx<KeyAnnotation>)>,
517-
implicit_return: Option<(Idx<Key>, bool)>,
512+
implicit_return: Option<Idx<Key>>,
518513
should_infer_return_type: bool,
519514
is_stub: bool,
520515
) {
@@ -526,11 +521,6 @@ impl<'a> BindingsBuilder<'a> {
526521
} else {
527522
None
528523
};
529-
let has_reachable_return = yields_and_returns
530-
.returns
531-
.iter()
532-
.any(|(_, _, is_unreachable)| !is_unreachable);
533-
534524
// Collect the keys of explicit returns.
535525
let return_keys = yields_and_returns
536526
.returns
@@ -579,20 +569,19 @@ impl<'a> BindingsBuilder<'a> {
579569

580570
let return_type_binding = {
581571
let kind = match (return_ann_with_range, implicit_return, is_stub) {
582-
(
583-
Some((range, annotation)),
584-
Some((implicit_return, has_infinite_loop)),
585-
false,
586-
) => {
572+
(Some((range, annotation)), Some(implicit_return), false) => {
573+
let function = self.idx_for_promise(KeyUndecoratedFunction(
574+
ShortIdentifier::new(func_name),
575+
));
587576
self.insert_binding(
588577
KeyExpect::ValidateImplicitReturn(range),
589578
BindingExpect::ValidateImplicitReturn {
590579
annotation,
591580
implicit_return,
592581
is_async,
593582
is_generator,
594-
has_explicit_return: has_reachable_return,
595-
has_infinite_loop,
583+
returns: return_keys,
584+
function,
596585
},
597586
);
598587
ReturnTypeKind::ShouldTrustAnnotation {
@@ -611,7 +600,7 @@ impl<'a> BindingsBuilder<'a> {
611600
is_generator,
612601
}
613602
}
614-
(None, Some((implicit_return, _)), _) if should_infer_return_type => {
603+
(None, Some(implicit_return), _) if should_infer_return_type => {
615604
// We don't have an explicit return annotation, but we want to infer it.
616605
ReturnTypeKind::ShouldInferType {
617606
returns: return_keys,
@@ -1035,8 +1024,6 @@ pub(crate) struct LastExpressions<'a> {
10351024
/// settling an assertion. The answer then holds for that configuration only, which suits
10361025
/// inferring a return type but not reporting code as dead.
10371026
pub decided_by_environment: bool,
1038-
/// Set when an unconditional loop is one of the terminal paths.
1039-
pub has_infinite_loop: bool,
10401027
}
10411028

10421029
fn or_dnf<'a>(
@@ -1088,7 +1075,6 @@ pub(crate) fn function_last_expressions<'a>(
10881075
x: &'a [Stmt],
10891076
guards: &mut Vec<TextRange>,
10901077
environment: &mut bool,
1091-
has_infinite_loop: &mut bool,
10921078
) -> Option<Vec<Vec<LastPathExpr<'a>>>> {
10931079
fn loop_body_has_break_statement(statement: &Stmt, has_break: &mut bool) {
10941080
match statement {
@@ -1122,7 +1108,7 @@ pub(crate) fn function_last_expressions<'a>(
11221108
guards: guards.as_slice().into(),
11231109
});
11241110
}
1125-
let body = f(sys_info, &x.body, guards, environment, has_infinite_loop)?;
1111+
let body = f(sys_info, &x.body, guards, environment)?;
11261112
and_dnf(Some(vec![with_clause]), Some(body))
11271113
}
11281114
Stmt::While(x) => {
@@ -1134,12 +1120,11 @@ pub(crate) fn function_last_expressions<'a>(
11341120
.visit(&mut |stmt| loop_body_has_break_statement(stmt, &mut has_break));
11351121
}
11361122
if test_value == Some(true) && !has_break {
1137-
*has_infinite_loop = true;
11381123
Some(vec![vec![]])
11391124
} else if has_break || x.orelse.is_empty() {
11401125
None
11411126
} else {
1142-
f(sys_info, &x.orelse, guards, environment, has_infinite_loop)
1127+
f(sys_info, &x.orelse, guards, environment)
11431128
}
11441129
}
11451130
Stmt::For(x) => {
@@ -1149,7 +1134,7 @@ pub(crate) fn function_last_expressions<'a>(
11491134
if has_break || x.orelse.is_empty() {
11501135
None
11511136
} else {
1152-
f(sys_info, &x.orelse, guards, environment, has_infinite_loop)
1137+
f(sys_info, &x.orelse, guards, environment)
11531138
}
11541139
}
11551140
Stmt::If(x) => {
@@ -1172,10 +1157,10 @@ pub(crate) fn function_last_expressions<'a>(
11721157
// evaluates as false.
11731158
// Neither can be dead by its test, so it needs no guard.
11741159
let branch = if index == 0 && test.is_none() {
1175-
f(sys_info, body, guards, environment, has_infinite_loop)?
1160+
f(sys_info, body, guards, environment)?
11761161
} else {
11771162
guards.push(range);
1178-
let branch = f(sys_info, body, guards, environment, has_infinite_loop);
1163+
let branch = f(sys_info, body, guards, environment);
11791164
guards.pop();
11801165
branch.unwrap_or_else(|| {
11811166
vec![vec![LastPathExpr {
@@ -1214,21 +1199,15 @@ pub(crate) fn function_last_expressions<'a>(
12141199
} else {
12151200
&x.orelse
12161201
};
1217-
match f(sys_info, body_suite, guards, environment, has_infinite_loop) {
1202+
match f(sys_info, body_suite, guards, environment) {
12181203
None => body_acc = None,
12191204
Some(p) => body_acc = and_dnf(body_acc, Some(p)),
12201205
}
12211206
if body_acc.is_some() {
12221207
for handler in &x.handlers {
12231208
match handler {
12241209
ExceptHandler::ExceptHandler(h) => {
1225-
match f(
1226-
sys_info,
1227-
&h.body,
1228-
guards,
1229-
environment,
1230-
has_infinite_loop,
1231-
) {
1210+
match f(sys_info, &h.body, guards, environment) {
12321211
None => {
12331212
body_acc = None;
12341213
break;
@@ -1244,21 +1223,15 @@ pub(crate) fn function_last_expressions<'a>(
12441223
if x.finalbody.is_empty() {
12451224
body_paths
12461225
} else {
1247-
let finally_paths = f(
1248-
sys_info,
1249-
&x.finalbody,
1250-
guards,
1251-
environment,
1252-
has_infinite_loop,
1253-
);
1226+
let finally_paths = f(sys_info, &x.finalbody, guards, environment);
12541227
or_dnf(finally_paths, body_paths)
12551228
}
12561229
}
12571230
Stmt::Match(x) => {
12581231
let mut syntactically_exhaustive = false;
12591232
let mut acc: Option<Vec<Vec<LastPathExpr<'a>>>> = Some(vec![vec![]]);
12601233
for case in x.cases.iter() {
1261-
let branch = f(sys_info, &case.body, guards, environment, has_infinite_loop)?;
1234+
let branch = f(sys_info, &case.body, guards, environment)?;
12621235
acc = and_dnf(acc, Some(branch));
12631236
if match_case_always_matches(&case.pattern, &x.subject, case.guard.as_deref()) {
12641237
syntactically_exhaustive = true;
@@ -1280,18 +1253,10 @@ pub(crate) fn function_last_expressions<'a>(
12801253
}
12811254

12821255
let mut environment = false;
1283-
let mut has_infinite_loop = false;
1284-
let clauses = f(
1285-
sys_info,
1286-
x,
1287-
&mut Vec::new(),
1288-
&mut environment,
1289-
&mut has_infinite_loop,
1290-
)?;
1256+
let clauses = f(sys_info, x, &mut Vec::new(), &mut environment)?;
12911257
Some(LastExpressions {
12921258
clauses,
12931259
decided_by_environment: environment,
1294-
has_infinite_loop,
12951260
})
12961261
}
12971262

‎pyrefly/lib/test/assign.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -324,7 +324,7 @@ testcase!(
324324
test_never,
325325
r#"
326326
from typing import Any, Never, NoReturn
327-
def foo(x: Never) -> Any:
327+
def foo(x: Never) -> Any: # E: Function declared to return `Any` but can never return
328328
y: NoReturn = x
329329
z: int = x
330330
return x

‎pyrefly/lib/test/flow_branching.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1912,7 +1912,7 @@ assert_type(vari, Literal["test"]) # E: `vari` is uninitialized
19121912
testcase!(
19131913
test_assert_false_terminates_flow,
19141914
r#"
1915-
def test1() -> int:
1915+
def test1() -> int: # E: Function declared to return `int` but can never return
19161916
assert False
19171917
def test2() -> int: # E: Function declared to return `int` but is missing an explicit `return`
19181918
assert True

0 commit comments

Comments
 (0)