-
Notifications
You must be signed in to change notification settings - Fork 0
Clone bugfix/st synthetic named expr in match #12
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from all commits
b3ec408
2da85ea
0160d1d
01b92c9
9967f27
6e09fd0
5e0b259
968a8bd
85d16bd
ded98cb
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -67,6 +67,7 @@ | |||||||||||||||||||||||||||||||
CallExpr, | ||||||||||||||||||||||||||||||||
ClassDef, | ||||||||||||||||||||||||||||||||
ComparisonExpr, | ||||||||||||||||||||||||||||||||
ComplexExpr, | ||||||||||||||||||||||||||||||||
Context, | ||||||||||||||||||||||||||||||||
ContinueStmt, | ||||||||||||||||||||||||||||||||
Decorator, | ||||||||||||||||||||||||||||||||
|
@@ -350,6 +351,9 @@ class TypeChecker(NodeVisitor[None], TypeCheckerSharedApi): | |||||||||||||||||||||||||||||||
# functions such as open(), etc. | ||||||||||||||||||||||||||||||||
plugin: Plugin | ||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||
# A helper state to produce unique temporary names on demand. | ||||||||||||||||||||||||||||||||
_unique_id: int | ||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||
def __init__( | ||||||||||||||||||||||||||||||||
self, | ||||||||||||||||||||||||||||||||
errors: Errors, | ||||||||||||||||||||||||||||||||
|
@@ -414,6 +418,7 @@ def __init__( | |||||||||||||||||||||||||||||||
self, self.msg, self.plugin, per_line_checking_time_ns | ||||||||||||||||||||||||||||||||
) | ||||||||||||||||||||||||||||||||
self.pattern_checker = PatternChecker(self, self.msg, self.plugin, options) | ||||||||||||||||||||||||||||||||
self._unique_id = 0 | ||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||
@property | ||||||||||||||||||||||||||||||||
def expr_checker(self) -> mypy.checkexpr.ExpressionChecker: | ||||||||||||||||||||||||||||||||
|
@@ -5413,21 +5418,7 @@ def visit_continue_stmt(self, s: ContinueStmt) -> None: | |||||||||||||||||||||||||||||||
return | ||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||
def visit_match_stmt(self, s: MatchStmt) -> None: | ||||||||||||||||||||||||||||||||
named_subject: Expression | ||||||||||||||||||||||||||||||||
if isinstance(s.subject, CallExpr): | ||||||||||||||||||||||||||||||||
# Create a dummy subject expression to handle cases where a match statement's subject | ||||||||||||||||||||||||||||||||
# is not a literal value. This lets us correctly narrow types and check exhaustivity | ||||||||||||||||||||||||||||||||
# This is hack! | ||||||||||||||||||||||||||||||||
if s.subject_dummy is None: | ||||||||||||||||||||||||||||||||
id = s.subject.callee.fullname if isinstance(s.subject.callee, RefExpr) else "" | ||||||||||||||||||||||||||||||||
name = "dummy-match-" + id | ||||||||||||||||||||||||||||||||
v = Var(name) | ||||||||||||||||||||||||||||||||
s.subject_dummy = NameExpr(name) | ||||||||||||||||||||||||||||||||
s.subject_dummy.node = v | ||||||||||||||||||||||||||||||||
named_subject = s.subject_dummy | ||||||||||||||||||||||||||||||||
else: | ||||||||||||||||||||||||||||||||
named_subject = s.subject | ||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||
named_subject = self._make_named_statement_for_match(s) | ||||||||||||||||||||||||||||||||
with self.binder.frame_context(can_skip=False, fall_through=0): | ||||||||||||||||||||||||||||||||
subject_type = get_proper_type(self.expr_checker.accept(s.subject)) | ||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||
|
@@ -5459,6 +5450,12 @@ def visit_match_stmt(self, s: MatchStmt) -> None: | |||||||||||||||||||||||||||||||
pattern_map, else_map = conditional_types_to_typemaps( | ||||||||||||||||||||||||||||||||
named_subject, pattern_type.type, pattern_type.rest_type | ||||||||||||||||||||||||||||||||
) | ||||||||||||||||||||||||||||||||
# Maybe the subject type can be inferred from constraints on | ||||||||||||||||||||||||||||||||
# its attribute/item? | ||||||||||||||||||||||||||||||||
if pattern_map and named_subject in pattern_map: | ||||||||||||||||||||||||||||||||
pattern_map[s.subject] = pattern_map[named_subject] | ||||||||||||||||||||||||||||||||
if else_map and named_subject in else_map: | ||||||||||||||||||||||||||||||||
else_map[s.subject] = else_map[named_subject] | ||||||||||||||||||||||||||||||||
pattern_map = self.propagate_up_typemap_info(pattern_map) | ||||||||||||||||||||||||||||||||
else_map = self.propagate_up_typemap_info(else_map) | ||||||||||||||||||||||||||||||||
self.remove_capture_conflicts(pattern_type.captures, inferred_types) | ||||||||||||||||||||||||||||||||
|
@@ -5506,6 +5503,36 @@ def visit_match_stmt(self, s: MatchStmt) -> None: | |||||||||||||||||||||||||||||||
with self.binder.frame_context(can_skip=False, fall_through=2): | ||||||||||||||||||||||||||||||||
pass | ||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||
def _make_named_statement_for_match(self, s: MatchStmt) -> Expression: | ||||||||||||||||||||||||||||||||
"""Construct a fake NameExpr for inference if a match clause is complex.""" | ||||||||||||||||||||||||||||||||
subject = s.subject | ||||||||||||||||||||||||||||||||
expressions_to_preserve = ( | ||||||||||||||||||||||||||||||||
# Already named - we should infer type of it as given | ||||||||||||||||||||||||||||||||
NameExpr, | ||||||||||||||||||||||||||||||||
AssignmentExpr, | ||||||||||||||||||||||||||||||||
# Primitive literals - their type is known, no need to name them | ||||||||||||||||||||||||||||||||
IntExpr, | ||||||||||||||||||||||||||||||||
StrExpr, | ||||||||||||||||||||||||||||||||
BytesExpr, | ||||||||||||||||||||||||||||||||
FloatExpr, | ||||||||||||||||||||||||||||||||
ComplexExpr, | ||||||||||||||||||||||||||||||||
EllipsisExpr, | ||||||||||||||||||||||||||||||||
) | ||||||||||||||||||||||||||||||||
Comment on lines
+5509
to
+5520
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Enhanced Pattern CoverageExpression preservation tuple could benefit from additional literal types like ListExpr, DictExpr, SetExpr for comprehensive literal coverage. Current implementation handles basic literals but may create unnecessary dummy names for compound literals with known types. Standards
|
||||||||||||||||||||||||||||||||
if isinstance(subject, expressions_to_preserve): | ||||||||||||||||||||||||||||||||
return subject | ||||||||||||||||||||||||||||||||
elif s.subject_dummy is not None: | ||||||||||||||||||||||||||||||||
return s.subject_dummy | ||||||||||||||||||||||||||||||||
else: | ||||||||||||||||||||||||||||||||
# Create a dummy subject expression to handle cases where a match statement's subject | ||||||||||||||||||||||||||||||||
# is not a literal value. This lets us correctly narrow types and check exhaustivity | ||||||||||||||||||||||||||||||||
# This is hack! | ||||||||||||||||||||||||||||||||
name = self.new_unique_dummy_name("match") | ||||||||||||||||||||||||||||||||
v = Var(name) | ||||||||||||||||||||||||||||||||
named_subject = NameExpr(name) | ||||||||||||||||||||||||||||||||
named_subject.node = v | ||||||||||||||||||||||||||||||||
s.subject_dummy = named_subject | ||||||||||||||||||||||||||||||||
return named_subject | ||||||||||||||||||||||||||||||||
Comment on lines
+5529
to
+5534
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Source Location PreservationThe synthetic NameExpr creation does not preserve source location information from the original subject expression. This violates the repository guideline requiring preservation of source-location info when synthesising dummy subjects. Missing location data can impact debugging and error reporting accuracy.
Commitable Suggestion
Suggested change
Standards
|
||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||
def _get_recursive_sub_patterns_map( | ||||||||||||||||||||||||||||||||
self, expr: Expression, typ: Type | ||||||||||||||||||||||||||||||||
) -> dict[Expression, Type]: | ||||||||||||||||||||||||||||||||
|
@@ -7885,6 +7912,12 @@ def warn_deprecated_overload_item( | |||||||||||||||||||||||||||||||
if candidate == target: | ||||||||||||||||||||||||||||||||
self.warn_deprecated(item.func, context) | ||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||
def new_unique_dummy_name(self, namespace: str) -> str: | ||||||||||||||||||||||||||||||||
"""Generate a name that is guaranteed to be unique for this TypeChecker instance.""" | ||||||||||||||||||||||||||||||||
name = f"dummy-{namespace}-{self._unique_id}" | ||||||||||||||||||||||||||||||||
self._unique_id += 1 | ||||||||||||||||||||||||||||||||
return name | ||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||
Comment on lines
+7915
to
+7920
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Identifier safety for dummy names Hyphens in "dummy-match-" are non-identifier. Use an identifier-safe pattern to avoid surprises. Apply: - name = f"dummy-{namespace}-{self._unique_id}"
+ name = f"__mypy_{namespace}_subject_{self._unique_id}" 📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents
|
||||||||||||||||||||||||||||||||
# leafs | ||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||
def visit_pass_stmt(self, o: PassStmt, /) -> None: | ||||||||||||||||||||||||||||||||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For better maintainability and a minor performance improvement, consider defining
expressions_to_preserve
as a constant at the module or class level, rather than recreating it on each call to_make_named_statement_for_match
.For example, you could define it at the module level: