diff --git a/pyrefly/lib/binding/bindings.rs b/pyrefly/lib/binding/bindings.rs index a23cc46ab..a014f6d27 100644 --- a/pyrefly/lib/binding/bindings.rs +++ b/pyrefly/lib/binding/bindings.rs @@ -14,6 +14,7 @@ use std::sync::Arc; use dupe::Dupe; use pyrefly_python::ast::Ast; use pyrefly_python::module_name::ModuleName; +use pyrefly_python::module_path::ModulePathDetails; use pyrefly_python::nesting_context::NestingContext; use pyrefly_python::short_identifier::ShortIdentifier; use pyrefly_python::sys_info::SysInfo; @@ -198,6 +199,7 @@ pub struct BindingsBuilder<'a> { pub has_docstring: bool, pub scopes: Scopes, table: BindingTable, + error_suppression_depth: usize, pub untyped_def_behavior: UntypedDefBehavior, unused_parameters: Vec, unused_imports: Vec, @@ -405,6 +407,7 @@ impl Bindings { has_docstring: Ast::has_docstring(&x), scopes: Scopes::module(x.range, enable_trace), table: Default::default(), + error_suppression_depth: 0, untyped_def_behavior, unused_parameters: Vec::new(), unused_imports: Vec::new(), @@ -748,6 +751,7 @@ impl<'a> BindingsBuilder<'a> { } pub fn init_static_scope(&mut self, x: &[Stmt], top_level: bool) { + let include_unreachable_defs = self.should_bind_unreachable_branches(); self.scopes.init_current_static( x, &self.module_info, @@ -760,6 +764,7 @@ impl<'a> BindingsBuilder<'a> { .0 .insert(KeyAnnotation::Annotation(x)) }, + include_unreachable_defs, ); } @@ -769,6 +774,29 @@ impl<'a> BindingsBuilder<'a> { } } + pub fn with_error_suppression( + &mut self, + f: impl FnOnce(&mut BindingsBuilder<'a>) -> R, + ) -> R { + self.error_suppression_depth += 1; + let result = f(self); + self.error_suppression_depth -= 1; + result + } + + #[inline] + fn errors_suppressed(&self) -> bool { + self.error_suppression_depth > 0 + } + + pub(crate) fn should_bind_unreachable_branches(&self) -> bool { + matches!( + self.module_info.path().details(), + ModulePathDetails::FileSystem(_) | ModulePathDetails::Memory(_) + ) && self.module_info.name() != ModuleName::builtins() + && self.module_info.name() != ModuleName::extra_builtins() + } + fn inject_globals(&mut self) { for global in ImplicitGlobal::implicit_globals(self.has_docstring) { let key = Key::ImplicitGlobal(global.name().clone()); @@ -891,10 +919,16 @@ impl<'a> BindingsBuilder<'a> { } pub fn error(&self, range: TextRange, info: ErrorInfo, msg: String) { + if self.errors_suppressed() { + return; + } self.errors.add(range, info, vec1![msg]); } pub fn error_multiline(&self, range: TextRange, info: ErrorInfo, msg: Vec1) { + if self.errors_suppressed() { + return; + } self.errors.add(range, info, msg); } @@ -1078,19 +1112,38 @@ impl<'a> BindingsBuilder<'a> { idx: Idx, style: FlowStyle, ) -> Option> { - let name = Hashed::new(name); - let write_info = self - .scopes - .define_in_current_flow(name, idx, style) - .unwrap_or_else(|| { - panic!( - "Name `{name}` not found in static scope of module `{}`.", - self.module_info.name(), - ) - }); + let mut hashed_name = Hashed::new(name); + let allow_unreachable_defs = + self.errors_suppressed() && self.should_bind_unreachable_branches(); + let mut write_info = self.scopes.define_in_current_flow( + hashed_name, + idx, + style.clone(), + allow_unreachable_defs, + ); + if write_info.is_none() && allow_unreachable_defs { + let key_range = self.table.types.0.idx_to_key(idx).range(); + self.scopes.add_synthetic_definition(name, key_range); + hashed_name = Hashed::new(name); + write_info = self.scopes.define_in_current_flow( + hashed_name, + idx, + style.clone(), + allow_unreachable_defs, + ); + } + let write_info = write_info.unwrap_or_else(|| { + panic!( + "Name `{name}` not found in static scope of module `{}`.", + self.module_info.name(), + ) + }); + if !write_info.reachability.is_reachable() { + debug_assert!(allow_unreachable_defs); + } if let Some(range) = write_info.anywhere_range { self.table - .record_bind_in_anywhere(name.into_key().clone(), range, idx); + .record_bind_in_anywhere(hashed_name.into_key().clone(), range, idx); } write_info.annotation } diff --git a/pyrefly/lib/binding/scope.rs b/pyrefly/lib/binding/scope.rs index f095c8668..54dc2a554 100644 --- a/pyrefly/lib/binding/scope.rs +++ b/pyrefly/lib/binding/scope.rs @@ -69,6 +69,7 @@ use crate::export::definitions::Definition; use crate::export::definitions::DefinitionStyle; use crate::export::definitions::Definitions; use crate::export::definitions::MutableCaptureKind; +use crate::export::definitions::Reachability; use crate::export::exports::ExportLocation; use crate::export::exports::LookupExport; use crate::export::special::SpecialExport; @@ -115,6 +116,8 @@ pub struct NameWriteInfo { /// If this name only has one assignment, we will skip the `Anywhere` as /// an optimization, and this field will be `None`. pub anywhere_range: Option, + /// Whether the definition is reachable for the current sys_info configuration. + pub reachability: Reachability, } #[derive(Clone, Debug)] @@ -172,6 +175,7 @@ struct Static(SmallMap); struct StaticInfo { range: TextRange, style: StaticStyle, + reachability: Reachability, } #[derive(Clone, Debug)] @@ -249,7 +253,7 @@ impl StaticStyle { fn of_definition( name: Hashed<&Name>, - definition: Definition, + definition: &Definition, scopes: Option<&Scopes>, get_annotation_idx: &mut impl FnMut(ShortIdentifier) -> Idx, ) -> Self { @@ -315,15 +319,26 @@ impl StaticInfo { } else { None }, + reachability: self.reachability, } } } impl Static { - fn upsert(&mut self, name: Hashed, range: TextRange, style: StaticStyle) { + fn upsert( + &mut self, + name: Hashed, + range: TextRange, + style: StaticStyle, + reachability: Reachability, + ) { match self.0.entry_hashed(name) { Entry::Vacant(e) => { - e.insert(StaticInfo { range, style }); + e.insert(StaticInfo { + range, + style, + reachability, + }); } Entry::Occupied(mut e) => { let found = e.get_mut(); @@ -360,6 +375,7 @@ impl Static { } } } + found.reachability = found.reachability.combine(reachability); } } } @@ -373,12 +389,14 @@ impl Static { sys_info: &SysInfo, get_annotation_idx: &mut impl FnMut(ShortIdentifier) -> Idx, scopes: Option<&Scopes>, + include_unreachable_defs: bool, ) { let mut d = Definitions::new( x, module_info.name(), module_info.path().is_init(), sys_info, + include_unreachable_defs, ); if top_level { if module_info.name() != ModuleName::builtins() { @@ -404,13 +422,18 @@ impl Static { // same name in this scope. let range = definition.range; let style = - StaticStyle::of_definition(name.as_ref(), definition, scopes, get_annotation_idx); - self.upsert(name, range, style); + StaticStyle::of_definition(name.as_ref(), &definition, scopes, get_annotation_idx); + self.upsert(name, range, style, definition.reachability); } for (range, wildcard) in wildcards { for name in wildcard.iter_hashed() { // TODO: semantics of import * and global var with same name - self.upsert(name.cloned(), range, StaticStyle::MergeableImport) + self.upsert( + name.cloned(), + range, + StaticStyle::MergeableImport, + Reachability::Reachable, + ) } } } @@ -421,6 +444,7 @@ impl Static { Hashed::new(name.id.clone()), name.range, StaticStyle::SingleDef(None), + Reachability::Reachable, ) }; Ast::expr_lvalue(x, &mut add); @@ -1263,6 +1287,7 @@ impl Scopes { lookup: &dyn LookupExport, sys_info: &SysInfo, get_annotation_idx: &mut impl FnMut(ShortIdentifier) -> Idx, + include_unreachable_defs: bool, ) { let mut initialize = |scope: &mut Scope, myself: Option<&Self>| { scope.stat.stmts( @@ -1273,6 +1298,7 @@ impl Scopes { sys_info, get_annotation_idx, myself, + include_unreachable_defs, ); // Presize the flow, as its likely to need as much space as static scope.flow.info.reserve(scope.stat.0.capacity()); @@ -1566,6 +1592,7 @@ impl Scopes { name: Hashed<&Name>, idx: Idx, style: FlowStyle, + allow_unreachable: bool, ) -> Option { let in_loop = self.loop_depth() != 0; match self.current_mut().flow.info.entry_hashed(name.cloned()) { @@ -1577,6 +1604,9 @@ impl Scopes { } } let static_info = self.current().stat.0.get_hashed(name)?; + if !allow_unreachable && !static_info.reachability.is_reachable() { + return None; + } Some(static_info.as_name_write_info()) } @@ -1708,6 +1738,7 @@ impl Scopes { Hashed::new(name.id.clone()), name.range, StaticStyle::SingleDef(ann), + Reachability::Reachable, ) } @@ -1805,6 +1836,7 @@ impl Scopes { Hashed::new(name.id.clone()), name.range, StaticStyle::PossibleLegacyTParam, + Reachability::Reachable, ) } @@ -1821,6 +1853,21 @@ impl Scopes { self.current_mut().stat.expr_lvalue(x); } + /// Synthesize a static definition entry for `name` in the current scope if it + /// is missing. Used when we analyze unreachable code for IDE metadata. + pub fn add_synthetic_definition(&mut self, name: &Name, range: TextRange) { + let hashed_ref = Hashed::new(name); + if self.current().stat.0.get_hashed(hashed_ref).is_some() { + return; + } + self.current_mut().stat.upsert( + Hashed::new(name.clone()), + range, + StaticStyle::SingleDef(None), + Reachability::Reachable, + ); + } + /// Add a loop exit point to the current innermost loop with the current flow. /// /// Return a bool indicating whether we were in a loop (if we weren't, we do nothing). @@ -2310,6 +2357,9 @@ impl ScopeTrace { { definition = Some(key); } + Key::Anywhere(_, range) if range.contains_inclusive(position) => { + definition = Some(key); + } _ => {} } }); diff --git a/pyrefly/lib/binding/stmt.rs b/pyrefly/lib/binding/stmt.rs index afefec026..ad0579635 100644 --- a/pyrefly/lib/binding/stmt.rs +++ b/pyrefly/lib/binding/stmt.rs @@ -717,14 +717,22 @@ impl<'a> BindingsBuilder<'a> { Some(x) => self.sys_info.evaluate_bool(x), }; self.ensure_expr_opt(test.as_mut(), &mut Usage::Narrowing(None)); - let new_narrow_ops = if this_branch_chosen == Some(false) { - // Skip the body in this case - it typically means a check (e.g. a sys version, - // platform, or TYPE_CHECKING check) where the body is not statically analyzable. + let new_narrow_ops = NarrowOps::from_expr(self, test.as_ref()); + if this_branch_chosen == Some(false) { + // Skip contributing to flow merges, but still bind names so IDE features work. + if self.should_bind_unreachable_branches() { + self.bind_narrow_ops( + &new_narrow_ops, + NarrowUseLocation::Span(range), + &Usage::Narrowing(None), + ); + self.with_error_suppression(|builder| { + builder.stmts(body, parent); + }); + } self.abandon_branch(); continue; - } else { - NarrowOps::from_expr(self, test.as_ref()) - }; + } if let Some(test_expr) = test { // Typecheck the test condition during solving. self.insert_binding( diff --git a/pyrefly/lib/export/definitions.rs b/pyrefly/lib/export/definitions.rs index 995a7e60f..116af02ca 100644 --- a/pyrefly/lib/export/definitions.rs +++ b/pyrefly/lib/export/definitions.rs @@ -46,6 +46,27 @@ pub enum MutableCaptureKind { Nonlocal, } +#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash, Default)] +pub enum Reachability { + #[default] + Reachable, + Unreachable, +} + +impl Reachability { + pub fn combine(self, other: Self) -> Self { + if matches!(self, Self::Reachable) || matches!(other, Self::Reachable) { + Self::Reachable + } else { + Self::Unreachable + } + } + + pub fn is_reachable(self) -> bool { + matches!(self, Self::Reachable) + } +} + /// How a name is defined. If a name is defined outside of this /// module, we additionally store the module we got it from. /// @@ -90,6 +111,8 @@ pub struct Definition { /// If the first statement in a definition (class, function) is a string literal, PEP 257 convention /// states that is the docstring. pub docstring_range: Option, + /// Whether this definition can run given the current configuration. + pub reachability: Reachability, } impl Definition { @@ -100,7 +123,7 @@ impl Definition { } } - fn merge(&mut self, other: DefinitionStyle, range: TextRange) { + fn merge(&mut self, other: DefinitionStyle, range: TextRange, reachability: Reachability) { // To ensure binding code cannot produce invalid lookups, we ensure that // `self.style` and `self.range` always match. if other < self.style { @@ -119,6 +142,7 @@ impl Definition { DefinitionStyle::MutableCapture(..) | DefinitionStyle::Delete => false, _ => true, }; + self.reachability = self.reachability.combine(reachability); } } @@ -187,6 +211,8 @@ struct DefinitionsBuilder<'a> { module_name: ModuleName, is_init: bool, sys_info: &'a SysInfo, + include_unreachable: bool, + reachability: Reachability, inner: Definitions, } @@ -213,11 +239,19 @@ fn is_overload_decorator(decorator: &Decorator) -> bool { } impl Definitions { - pub fn new(x: &[Stmt], module_name: ModuleName, is_init: bool, sys_info: &SysInfo) -> Self { + pub fn new( + x: &[Stmt], + module_name: ModuleName, + is_init: bool, + sys_info: &SysInfo, + include_unreachable: bool, + ) -> Self { let mut builder = DefinitionsBuilder { module_name, sys_info, is_init, + include_unreachable, + reachability: Reachability::Reachable, inner: Definitions::default(), }; builder.stmts(x); @@ -242,6 +276,7 @@ impl Definitions { style: DefinitionStyle::ImplicitGlobal, needs_anywhere: false, docstring_range: None, + reachability: Reachability::Reachable, }, ); } @@ -286,6 +321,18 @@ impl Definitions { } impl<'a> DefinitionsBuilder<'a> { + fn with_reachability( + &mut self, + reachability: Reachability, + f: impl FnOnce(&mut Self) -> R, + ) -> R { + let previous = self.reachability; + self.reachability = reachability; + let result = f(self); + self.reachability = previous; + result + } + fn stmts(&mut self, xs: &[Stmt]) { for x in xs { self.stmt(x); @@ -299,9 +346,10 @@ impl<'a> DefinitionsBuilder<'a> { style: DefinitionStyle, body: Option<&[Stmt]>, ) { + let reachability = self.reachability; match self.inner.definitions.entry(x.clone()) { Entry::Occupied(mut e) => { - e.get_mut().merge(style, range); + e.get_mut().merge(style, range, reachability); } Entry::Vacant(e) => { e.insert(Definition { @@ -309,6 +357,7 @@ impl<'a> DefinitionsBuilder<'a> { style, needs_anywhere: false, docstring_range: body.and_then(Docstring::range_from_stmts), + reachability, }); } } @@ -622,9 +671,25 @@ impl<'a> DefinitionsBuilder<'a> { } } Stmt::If(x) => { - self.named_in_expr(&x.test); - for (_, body) in self.sys_info.pruned_if_branches(x) { - self.stmts(body); + for (test, body) in Ast::if_branches(x) { + if let Some(test_expr) = test { + self.named_in_expr(test_expr); + } + let evaluation = test + .map(|expr| self.sys_info.evaluate_bool(expr)) + .unwrap_or(Some(true)); + if evaluation == Some(false) && !self.include_unreachable { + continue; + } + let branch_reachability = if evaluation == Some(false) { + Reachability::Unreachable + } else { + Reachability::Reachable + }; + self.with_reachability(branch_reachability, |builder| builder.stmts(body)); + if evaluation == Some(true) { + break; // Later branches are not evaluated in this configuration + } } return; // We went through the relevant branches already } @@ -722,25 +787,44 @@ mod tests { } } - fn calculate_unranged_definitions( + fn calculate_unranged_definitions_with_config( contents: &str, module_name: ModuleName, is_init: bool, + include_unreachable: bool, ) -> Definitions { let mut res = Definitions::new( &Ast::parse(contents, PySourceType::Python).0.body, module_name, is_init, &SysInfo::default(), + include_unreachable, ); res.dunder_all.iter_mut().for_each(unrange); res } + fn calculate_unranged_definitions( + contents: &str, + module_name: ModuleName, + is_init: bool, + ) -> Definitions { + calculate_unranged_definitions_with_config(contents, module_name, is_init, false) + } + fn calculate_unranged_definitions_with_defaults(contents: &str) -> Definitions { calculate_unranged_definitions(contents, ModuleName::from_str("main"), false) } + fn calculate_unranged_definitions_with_unreachable(contents: &str) -> Definitions { + calculate_unranged_definitions_with_config( + contents, + ModuleName::from_str("main"), + false, + true, + ) + } + fn assert_import_all(defs: &Definitions, expected_import_all: &[&str]) { assert_eq!( expected_import_all, @@ -771,6 +855,40 @@ mod tests { ); } + #[test] + fn test_unreachable_if_defs_respected() { + let contents = r#" +if False: + foo: TypeIs[int] +else: + bar = 1 +"#; + let defs = calculate_unranged_definitions_with_unreachable(contents); + let foo = defs.definitions.get(&Name::new("foo")).unwrap(); + assert!(matches!(foo.reachability, Reachability::Unreachable)); + let bar = defs.definitions.get(&Name::new("bar")).unwrap(); + assert!(matches!(bar.reachability, Reachability::Reachable)); + + let defs_pruned = calculate_unranged_definitions_with_defaults(contents); + assert!(defs_pruned.definitions.get(&Name::new("foo")).is_none()); + assert!(defs_pruned.definitions.get(&Name::new("bar")).is_some()); + } + + #[test] + fn test_unreachable_if_comparison_defs_respected() { + let contents = r#" +if 1 == 0: + foo = 1 + +bar = 2 +"#; + let defs = calculate_unranged_definitions_with_unreachable(contents); + assert!(defs.definitions.get(&Name::new("foo")).is_some()); + + let defs_pruned = calculate_unranged_definitions_with_defaults(contents); + assert!(defs_pruned.definitions.get(&Name::new("foo")).is_none()); + } + #[test] fn test_definitions() { let defs = calculate_unranged_definitions_with_defaults( diff --git a/pyrefly/lib/export/exports.rs b/pyrefly/lib/export/exports.rs index 7820281d6..3c482abbe 100644 --- a/pyrefly/lib/export/exports.rs +++ b/pyrefly/lib/export/exports.rs @@ -96,6 +96,7 @@ impl Exports { module_info.name(), module_info.path().is_init(), sys_info, + false, ); definitions.inject_implicit_globals(); definitions.ensure_dunder_all(module_info.path().style()); diff --git a/pyrefly/lib/test/lsp/definition.rs b/pyrefly/lib/test/lsp/definition.rs index 0183aaab0..19600c246 100644 --- a/pyrefly/lib/test/lsp/definition.rs +++ b/pyrefly/lib/test/lsp/definition.rs @@ -96,11 +96,15 @@ if not TYPE_CHECKING: # main.py 5 | x = 1 ^ -Definition Result: None +Definition Result: +5 | x = 1 + ^ 7 | y = x ^ -Definition Result: None +Definition Result: +5 | x = 1 + ^ "# .trim(), report.trim(), @@ -1572,7 +1576,9 @@ if False: # main.py 4 | print(x) ^ -Definition Result: None +Definition Result: +2 | x = 5 + ^ "# .trim(), diff --git a/pyrefly/lib/test/lsp/hover_type.rs b/pyrefly/lib/test/lsp/hover_type.rs index b2ceb095b..c0fe6da48 100644 --- a/pyrefly/lib/test/lsp/hover_type.rs +++ b/pyrefly/lib/test/lsp/hover_type.rs @@ -89,7 +89,7 @@ def f(): ... # main.py 2 | def f(): ... ^ -Hover Result: None +Hover Result: `() -> None` 4 | def f(): ... ^ @@ -289,19 +289,19 @@ if False: # main.py 3 | def f(): ^ -Hover Result: None +Hover Result: `() -> None` 7 | x = 3 ^ -Hover Result: None +Hover Result: `Literal[3]` 9 | x ^ -Hover Result: None +Hover Result: `Literal[3]` 11 | f ^ -Hover Result: None +Hover Result: `() -> None` 14 | def f(): ^ @@ -309,15 +309,15 @@ Hover Result: None 18 | x = 3 ^ -Hover Result: None +Hover Result: `Literal[3]` 20 | x ^ -Hover Result: None +Hover Result: `Literal[3]` 22 | f ^ -Hover Result: None +Hover Result: `() -> None` "# .trim(), report.trim(),