diff --git a/pyrefly/lib/binding/bindings.rs b/pyrefly/lib/binding/bindings.rs index 4b9e0b82d2..7d5454cb15 100644 --- a/pyrefly/lib/binding/bindings.rs +++ b/pyrefly/lib/binding/bindings.rs @@ -195,6 +195,8 @@ struct BindingsInner { /// here, before the AST is evicted from memory. Used for error suppression. module_ranges: Arc, scope_trace: Option, + /// Names `del`eted at module scope; retained outside the LSP `scope_trace`. + module_deletes: SmallSet, unused_parameters: Vec, unused_imports: Vec, unused_variables: Vec, @@ -347,6 +349,7 @@ impl Bindings { ignore_all: SmallMap::new(), }), scope_trace: None, + module_deletes: SmallSet::new(), unused_parameters: Vec::new(), unused_imports: Vec::new(), unused_variables: Vec::new(), @@ -437,6 +440,11 @@ impl Bindings { self.0.subsequently_initialized.contains(&ann) } + /// Names `del`eted at module scope. + pub fn module_deletes(&self) -> &SmallSet { + &self.0.module_deletes + } + pub fn available_definitions(&self, position: TextSize) -> SmallSet> { if let Some(trace) = &self.0.scope_trace { trace.available_definitions(&self.0.table, position) @@ -708,6 +716,7 @@ impl Bindings { ); } } + let module_deletes = scope_trace.module_deletes().clone(); Self(Arc::new(BindingsInner { module_info, table: builder.table, @@ -718,6 +727,7 @@ impl Bindings { } else { None }, + module_deletes, unused_parameters: builder.unused_parameters, unused_imports: builder.unused_imports, unused_variables: builder.unused_variables, diff --git a/pyrefly/lib/binding/scope.rs b/pyrefly/lib/binding/scope.rs index 70784c1888..44ee25b3dd 100644 --- a/pyrefly/lib/binding/scope.rs +++ b/pyrefly/lib/binding/scope.rs @@ -1213,6 +1213,8 @@ pub struct Scope { /// Names marked `Final` with string literal values, e.g. `X: Final = "x"`. /// Used to resolve Final variable references in synthesized class field names. final_string_values: SmallMap, + /// Names removed by a `del NAME` statement in this scope. + deleted_names: SmallSet, } impl Scope { @@ -1233,6 +1235,7 @@ impl Scope { implicit_captures: SmallSet::new(), final_names: SmallSet::new(), final_string_values: SmallMap::new(), + deleted_names: SmallSet::new(), } } @@ -2089,9 +2092,11 @@ impl Scopes { /// Don't change the type if one is present - downstream we'll emit /// uninitialized local errors but keep using our best guess for the type. pub fn mark_as_deleted(&mut self, name: &Name) { - if let Some(value) = self.current_mut().flow.get_value_mut(name) { + let scope = self.current_mut(); + if let Some(value) = scope.flow.get_value_mut(name) { value.style = FlowStyle::Uninitialized; } + scope.deleted_names.insert(name.clone()); } fn get_flow_info(&self, name: &Name) -> Option<&FlowInfo> { @@ -3052,6 +3057,11 @@ impl ScopeTrace { &self.0.scope } + /// Names `del`eted at module scope. + pub fn module_deletes(&self) -> &SmallSet { + &self.0.scope.deleted_names + } + pub fn exportables(&self) -> SmallMap { let mut exportables = SmallMap::new(); let scope = self.toplevel_scope(); diff --git a/pyrefly/lib/commands/report.rs b/pyrefly/lib/commands/report.rs index 9a5f643eb3..c1a9c0b6c6 100644 --- a/pyrefly/lib/commands/report.rs +++ b/pyrefly/lib/commands/report.rs @@ -831,6 +831,7 @@ impl ReportArgs { } else { String::new() }; + let deleted = bindings.module_deletes(); // Collect names already reported as functions or classes so we can skip them. let reported_names: SmallSet<&str> = functions .iter() @@ -841,9 +842,11 @@ impl ReportArgs { let mut variables = Vec::new(); for idx in bindings.keys::() { let KeyExport(name) = bindings.idx_to_key(idx); - // Skip non-public module-level names and excluded dunders. + // Skip non-public module-level names, excluded dunders, and `del`eted names. let name_str = name.as_str(); - if !Self::is_public_name(name_str) || Self::EXCLUDED_MODULE_DUNDERS.contains(&name_str) + if !Self::is_public_name(name_str) + || Self::EXCLUDED_MODULE_DUNDERS.contains(&name_str) + || deleted.contains(name) { continue; } @@ -1124,6 +1127,7 @@ impl ReportArgs { } else { String::new() }; + let deleted = bindings.module_deletes(); for idx in bindings.keys::() { if let Key::Definition(id) = bindings.idx_to_key(idx) @@ -1180,13 +1184,11 @@ impl ReportArgs { } } } else { - // Skip functions not present in the module's exports - // (e.g. functions nested inside other functions). - if !exports.contains_key(&fun.def.name.id) { - continue; - } - // Skip non-public module-level functions. - if !Self::is_public_name(fun.def.name.as_str()) { + // Keep only public, exported, non-deleted module-level functions. + if !exports.contains_key(&fun.def.name.id) + || !Self::is_public_name(fun.def.name.as_str()) + || deleted.contains(&fun.def.name.id) + { continue; } format!("{}{}", module_prefix, fun.def.name) @@ -1600,6 +1602,7 @@ impl ReportArgs { } else { String::new() }; + let deleted = bindings.module_deletes(); for class_idx in bindings.keys::() { // Skip @type_check_only classes. if tco_classes.contains(&class_idx) { @@ -1610,8 +1613,14 @@ impl ReportArgs { BindingClass::ClassDef(cls) => cls, BindingClass::FunctionalClassDef(..) => continue, }; + let parent = &cls_binding.parent; + let name = &cls_binding.def.name; // Skip classes nested inside functions, since they are not public symbols. - if Self::has_function_ancestor(&cls_binding.parent) { + if Self::has_function_ancestor(parent) { + continue; + } + // Skip top-level classes `del`eted at module scope. + if parent.is_toplevel() && deleted.contains(&name.id) { continue; } let class_type = match answers.get_idx(class_idx) { @@ -1623,7 +1632,7 @@ impl ReportArgs { }; let class_name = format!( "{module_prefix}{}", - Self::class_qualified_name(module, &cls_binding.parent, &cls_binding.def.name) + Self::class_qualified_name(module, parent, name) ); let mro = answers .get_idx(bindings.key_to_idx(&KeyClassMro(ClassDefIndex(class_type.index().0)))) @@ -2881,6 +2890,13 @@ mod tests { compare_snapshot("implicit_module_globals.expected.json", &report); } + /// Names removed by `del` must not appear in the report (issue #3576). + #[test] + fn test_report_del_module_level() { + let report = build_module_report_for_test("del_module_level.py"); + compare_snapshot("del_module_level.expected.json", &report); + } + /// --module name override: entity counts (n_functions vs n_methods) must /// be correct even when the output module name differs from the derived name. #[test] diff --git a/pyrefly/lib/test/report/test_files/del_module_level.expected.json b/pyrefly/lib/test/report/test_files/del_module_level.expected.json new file mode 100644 index 0000000000..c38d65b808 --- /dev/null +++ b/pyrefly/lib/test/report/test_files/del_module_level.expected.json @@ -0,0 +1,63 @@ +{ + "name": "test", + "path": "test.py", + "names": [ + "test.keep_me", + "test.lst", + "test.helper" + ], + "line_count": 40, + "symbol_reports": [ + { + "kind": "attr", + "name": "test.keep_me", + "n_typable": 1, + "n_typed": 1, + "n_any": 0, + "n_untyped": 0, + "location": { + "line": 17, + "column": 1 + } + }, + { + "kind": "attr", + "name": "test.lst", + "n_typable": 1, + "n_typed": 1, + "n_any": 0, + "n_untyped": 0, + "location": { + "line": 25, + "column": 1 + } + }, + { + "kind": "function", + "name": "test.helper", + "n_typable": 1, + "n_typed": 1, + "n_any": 0, + "n_untyped": 0, + "location": { + "line": 20, + "column": 1 + } + } + ], + "type_ignores": [], + "n_typable": 3, + "n_typed": 3, + "n_any": 0, + "n_untyped": 0, + "coverage": 100.0, + "strict_coverage": 100.0, + "n_functions": 1, + "n_methods": 0, + "n_function_params": 0, + "n_method_params": 0, + "n_classes": 0, + "n_attrs": 2, + "n_properties": 0, + "n_type_ignores": 0 +} diff --git a/pyrefly/lib/test/report/test_files/del_module_level.py b/pyrefly/lib/test/report/test_files/del_module_level.py new file mode 100644 index 0000000000..dd75322c4f --- /dev/null +++ b/pyrefly/lib/test/report/test_files/del_module_level.py @@ -0,0 +1,39 @@ +# Copyright (c) Meta Platforms, Inc. and affiliates. +# +# This source code is licensed under the MIT license found in the +# LICENSE file in the root directory of this source tree. + +# gh-3576: module-scope `del` names must not appear in the report. + +for ta in ["a", "b"]: + pass +del ta + + +tmp = 1 +del tmp + + +keep_me: int = 42 # must survive a function-local `del` of a same-named local. + + +def helper() -> None: + keep_me = 1 + del keep_me + + +lst: list[int] = [1] +del lst[0] + + +def setup() -> None: ... + + +setup() +del setup + + +class _Tmp: ... + + +del _Tmp