From 672d89f943045adbf07af9c5f4c689777062df19 Mon Sep 17 00:00:00 2001 From: serge-sans-paille Date: Mon, 20 Jan 2025 08:21:30 +0100 Subject: [PATCH] Improve support of `del` statement When an `ast.Name` is deleted, create a fake definition that shadows previous definition but which is handled differently by the remaining logic Fix #112 --- beniget/beniget.py | 43 +++++++++++++++++++++++++++---------------- tests/test_chains.py | 30 ++++++++++++++++++++++++++++++ 2 files changed, 57 insertions(+), 16 deletions(-) diff --git a/beniget/beniget.py b/beniget/beniget.py index 9c9ee25..35dcc16 100644 --- a/beniget/beniget.py +++ b/beniget/beniget.py @@ -8,6 +8,12 @@ from .ordered_set import ordered_set +def is_deleted_def(dnode): + ast = pkg(dnode.node) + if not isinstance(dnode.node, ast.Name): + return False + return isinstance(dnode.node.ctx, ast.Del) + def pkg(node): """ @@ -504,7 +510,11 @@ def compute_defs(self, node, quiet=False): if defs is StopIteration: break elif name in defs: - return defs[name] if not stars else stars + list(defs[name]) + name_defs = {dnode for dnode in defs[name] if not + is_deleted_def(dnode)} + if not name_defs: + break + return name_defs if not stars else stars + list(name_defs) elif "*" in defs: stars.extend(defs["*"]) @@ -1273,7 +1283,22 @@ def _first_non_comprehension_scope(self): def visit_Name(self, node, skip_annotation=False, named_expr=False): ast = pkg(node) - if isinstance(node.ctx, (ast.Param, ast.Store)): + + if isinstance(node.ctx, (ast.Load, ast.Del)): + node_in_chains = node in self.chains + if node_in_chains: + dnode = self.chains[node] + else: + dnode = Def(node) + for d in self.defs(node): + d.add_user(dnode) + if not node_in_chains: + self.chains[node] = dnode + + if isinstance(node.ctx, ast.Del): + node = ast.Name(node.id, ast.Del()) + + if isinstance(node.ctx, (ast.Param, ast.Store, ast.Del)): dnode = self.chains.setdefault(node, Def(node)) # FIXME: find a smart way to merge the code below with add_to_locals if any(node.id in _globals for _globals in self._globals): @@ -1298,20 +1323,6 @@ def visit_Name(self, node, skip_annotation=False, named_expr=False): if getattr(node, 'annotation', None) is not None and not skip_annotation and not self.future_annotations: self.visit(node.annotation) - - elif isinstance(node.ctx, (ast.Load, ast.Del)): - node_in_chains = node in self.chains - if node_in_chains: - dnode = self.chains[node] - else: - dnode = Def(node) - for d in self.defs(node): - d.add_user(dnode) - if not node_in_chains: - self.chains[node] = dnode - # currently ignore the effect of a del - else: - raise NotImplementedError() return dnode def visit_Destructured(self, node): diff --git a/tests/test_chains.py b/tests/test_chains.py index daaa395..dd50912 100644 --- a/tests/test_chains.py +++ b/tests/test_chains.py @@ -547,6 +547,36 @@ def test_unbound_local_identifier_nonlocal_points_to_scoped_global(self): self.check_message(code, ["W: unbound identifier 'x' at :3:2"]) + def test_unbound_deleted_identifier(self): + code = "x = 1; del x; x" + self.check_message(code, + ["W: unbound identifier 'x' at :1:14"]) + + def test_unbound_deleted_identifier_in_class(self): + code = "class X:\n x = 1\n del x\n x" + self.check_message(code, + ["W: unbound identifier 'x' at :4:1"]) + + def test_bound_deleted_identifier(self): + code = "x = 1; del x; x = 1; x" + self.check_message(code, + []) + + def test_bound_deleted_identifier_in_if(self): + code = "x = 1\ndel x\nif 1:\n x = 1\nx" + self.check_message(code, + []) + + def test_maybe_unbound_in_if(self): + code = "def foo(x):\n if x: del x\n print(x)" + self.check_message(code, + []) + + def test_always_unbound_in_if(self): + code = "def foo(x):\n if x: del x\n else: del x\n x" + self.check_message(code, + ["W: unbound identifier 'x' at :4:2"]) + def test_assign_uses_class_level_name(self): code = ''' visit_Name = object