Skip to content

Commit e4c0a84

Browse files
committed
Try to fix false positive undefined variable in match cases.
That won't be perfect as the way pyflyback work the scope management is a bit complex.
1 parent 143fc01 commit e4c0a84

File tree

4 files changed

+107
-8
lines changed

4 files changed

+107
-8
lines changed

lib/python/pyflyby/_autoimp.py

+48-4
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@
1919
from pyflyby._log import logger
2020
from pyflyby._modules import ModuleHandle
2121
from pyflyby._parse import (PythonBlock, _is_ast_str,
22-
infer_compile_mode)
22+
infer_compile_mode, MatchAs)
2323

2424
from six import reraise
2525
import sys
@@ -491,8 +491,11 @@ def visit(self, node):
491491
logger.debug(
492492
"_MissingImportFinder has no method %r, using generic_visit", method
493493
)
494-
495-
visitor = getattr(self, method, self.generic_visit)
494+
if hasattr(self, method):
495+
visitor = getattr(self, method)
496+
else:
497+
logger.debug("No method `%s`, using `generic_visit`", method)
498+
visitor = self.generic_visit
496499
return visitor(node)
497500
else:
498501
raise TypeError("unexpected %s" % (type(node).__name__,))
@@ -827,6 +830,47 @@ def visit_alias(self, node, modulename=None):
827830
self._visit_StoreImport(node, modulename)
828831
self.generic_visit(node)
829832

833+
if sys.version_info >= (3,10):
834+
def visit_match_case(self, node:ast.match_case):
835+
logger.debug("visit_match_case(%r)", node)
836+
return self.generic_visit(node)
837+
838+
def visit_Match(self, node:ast.Match):
839+
logger.debug("visit_Match(%r)", node)
840+
return self.generic_visit(node)
841+
842+
def visit_MatchMapping(self, node:ast.MatchMapping):
843+
logger.debug("visit_MatchMapping(%r)", node)
844+
return self.generic_visit(node)
845+
846+
def visit_MatchAs(self, node:MatchAs):
847+
logger.debug("visit_MatchAs(%r)", node)
848+
if node.name is None:
849+
return
850+
isinstance(node.name, str), node.name
851+
return self._visit_Store(node.name)
852+
853+
def visit_Call(self, node:ast.Call):
854+
logger.debug("visit_Call(%r)", node)
855+
return self.generic_visit(node)
856+
857+
def visit_Pass(self, node:ast.Pass):
858+
logger.debug("visit_Pass(%r)", node)
859+
return self.generic_visit(node)
860+
861+
def visit_Constant(self, node:ast.Constant):
862+
logger.debug("visit_Constant(%r)", node)
863+
return self.generic_visit(node)
864+
865+
def visit_Module(self, node:ast.Module):
866+
logger.debug("visit_Module(%r)", node)
867+
return self.generic_visit(node)
868+
869+
def visit_Expr(self, node:ast.Expr):
870+
logger.debug("visit_Expr(%r)", node)
871+
return self.generic_visit(node)
872+
873+
830874
def visit_Name(self, node):
831875
logger.debug("visit_Name(%r)", node.id)
832876
self._visit_fullname(node.id, node.ctx)
@@ -884,7 +928,7 @@ def _visit_StoreImport(self, node, modulename):
884928
value = _UseChecker(name, imp, self._lineno)
885929
self._visit_Store(name, value)
886930

887-
def _visit_Store(self, fullname, value=None):
931+
def _visit_Store(self, fullname:str, value=None):
888932
logger.debug("_visit_Store(%r)", fullname)
889933
if fullname is None:
890934
return

lib/python/pyflyby/_parse.py

+1
Original file line numberDiff line numberDiff line change
@@ -208,6 +208,7 @@ def _iter_child_nodes_in_order_internal_1(node):
208208
yield node.name,
209209
elif isinstance(node, MatchMapping):
210210
for k, p in zip(node.keys, node.patterns):
211+
pass
211212
yield k, p
212213
else:
213214
# Default behavior.

tests/test_autoimp.py

+46-3
Original file line numberDiff line numberDiff line change
@@ -479,7 +479,7 @@ def foo(self):
479479
assert expected == result
480480

481481

482-
@pytest.mark.xfail
482+
@pytest.mark.xfail(strict=True)
483483
def test_find_missing_import_xfail_after_pr_152():
484484
code = dedent(
485485
"""
@@ -538,7 +538,8 @@ class B:
538538

539539

540540
@pytest.mark.xfail(
541-
reason="Had to deactivate as part of https://github.com/deshaw/pyflyby/pull/269/files conflicting requirements"
541+
reason="Had to deactivate as part of https://github.com/deshaw/pyflyby/pull/269/files conflicting requirements",
542+
strict=True,
542543
)
543544
def test_find_missing_imports_class_name_1():
544545
code = dedent(
@@ -1146,6 +1147,48 @@ def test_find_missing_imports_true_false_none_1():
11461147
assert expected == result
11471148

11481149

1150+
@pytest.mark.skipif(sys.version_info < (3, 10), reason='No pattern matching before 3.10')
1151+
def test_find_missing_imports_pattern_match_1():
1152+
code = dedent("""
1153+
match {"foo": 1, "bar": 2}:
1154+
case {
1155+
"foo": the_foo_value,
1156+
"bar": the_bar_value,
1157+
**rest,
1158+
}:
1159+
print(the_foo_value)
1160+
case _:
1161+
pass
1162+
""")
1163+
result = find_missing_imports(code, [{}])
1164+
result = _dilist2strlist(result)
1165+
expected = []
1166+
assert expected == result
1167+
1168+
@pytest.mark.xfail(reason='''The way the scope work in pyflyby it is hard to define a variable...
1169+
only in one case I believe. We would need a scope stack in `def
1170+
visit_match_case`, but that would remove the variable definition
1171+
when leaving the match statement.
1172+
''',strict=True)
1173+
@pytest.mark.skipif(sys.version_info < (3, 10), reason='No pattern matching before 3.10')
1174+
def test_find_missing_imports_pattern_match_2():
1175+
code = dedent("""
1176+
match {"foo": 1, "bar": 2}:
1177+
case {
1178+
"foo": the_foo_value,
1179+
"bar": the_bar_value,
1180+
**rest,
1181+
}:
1182+
print(the_foo_value)
1183+
case _:
1184+
print('here the_x_value might be unknown', the_foo_value)
1185+
""")
1186+
result = find_missing_imports(code, [{}])
1187+
result = _dilist2strlist(result)
1188+
expected = [DottedIdentifier('the_foo_value')]
1189+
assert expected == result
1190+
1191+
11491192
def test_find_missing_imports_matmul_1():
11501193
code = dedent("""
11511194
a@b
@@ -1572,7 +1615,7 @@ def test_scan_for_import_issues_comprehension_attribute_1():
15721615
assert unused == []
15731616

15741617

1575-
@pytest.mark.xfail
1618+
@pytest.mark.xfail(strict=True)
15761619
def test_scan_for_import_issues_comprehension_attribute_missing_1():
15771620
code = dedent("""
15781621
[123 for xx.yy in []]

tests/test_parse.py

+12-1
Original file line numberDiff line numberDiff line change
@@ -624,7 +624,18 @@ def http_error(status):
624624
case _:
625625
return "Something's wrong with the Internet"
626626
627-
"""
627+
""",
628+
"""
629+
match {"foo": 1, "bar": 2}:
630+
case {
631+
"foo": foo,
632+
"bar": bar,
633+
**rest,
634+
}:
635+
print(foo)
636+
case _:
637+
pass
638+
""",
628639
]
629640
]
630641
)

0 commit comments

Comments
 (0)