Skip to content

Commit 4ffa2e0

Browse files
fangyi-zhoufacebook-github-bot
authored andcommitted
Handle scoping correctly for class fields in body (reland) (#1079)
Summary: This is a reland of PR #1024 / D81681451 for addressing #264 The original change was reverted in 2747227 / D82227101 due to regression (#1073 and #1074). This reland addresses the regression: - allows annotation scope to access class fields (instead of static type information which was incorrect) - allows comprehension scopre to access class fields (this leaves the test still broken --- since comprehension scoping is incorrect for iterator expressions in a comprehension) Pull Request resolved: #1079 Reviewed By: samwgoldman Differential Revision: D82332705 Pulled By: stroxler fbshipit-source-id: 8ae342c6b15993d9e06b4f7b29c2a13efcd5036c
1 parent f3c7a96 commit 4ffa2e0

File tree

2 files changed

+27
-3
lines changed

2 files changed

+27
-3
lines changed

pyrefly/lib/binding/bindings.rs

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -788,9 +788,32 @@ impl<'a> BindingsBuilder<'a> {
788788
) -> Result<(Idx<Key>, Option<Idx<Key>>), LookupError> {
789789
let mut barrier = false;
790790
let ok_no_usage = |idx| Ok((idx, None));
791+
let is_current_scope_class = matches!(self.scopes.current().kind, ScopeKind::Class(_));
792+
let is_current_scope_annotation =
793+
matches!(self.scopes.current().kind, ScopeKind::Annotation);
794+
// FIXME: Temporarily allow comprehensions to see all class fields, but
795+
// class fields should be only allowed be used in the place of iterator,
796+
// i.e. in `[expr1 for x in expr2]`, `expr2` can see class fields, but not `expr1`.
797+
// See https://github.com/facebook/pyrefly/issues/264#issuecomment-3282180275 for details.
798+
let is_current_scope_comprehension =
799+
matches!(self.scopes.current().kind, ScopeKind::Comprehension);
800+
let mut is_current_scope = true;
791801
for scope in self.scopes.iter_rev() {
792802
if let Some(flow) = scope.flow.info.get_hashed(name)
793803
&& !barrier
804+
// Handles the special case of class fields:
805+
// From https://docs.python.org/3/reference/executionmodel.html#resolution-of-names:
806+
// """The scope of names defined in a class block is limited to the
807+
// class block; it does not extend to the code blocks of
808+
// methods. This includes comprehensions and generator
809+
// expressions, but it does not include annotation scopes, which
810+
// have access to their enclosing class scopes."""
811+
&& (
812+
(is_current_scope_class && is_current_scope) // The class body can see class fields in the current scope
813+
|| is_current_scope_annotation // Annotations can see class fields in enclosing scopes
814+
|| is_current_scope_comprehension // Comprehensions can see class fields in the current scope, but only in the place of iterators
815+
|| !matches!(flow.style, FlowStyle::ClassField { .. }) // Other scopes cannot see class fields
816+
)
794817
{
795818
let (idx, maybe_pinned_idx) = self.detect_possible_first_use(flow.key, usage);
796819
if let Some(pinned_idx) = maybe_pinned_idx {
@@ -814,6 +837,7 @@ impl<'a> BindingsBuilder<'a> {
814837
}
815838
}
816839
}
840+
is_current_scope = false;
817841
barrier = barrier || scope.barrier;
818842
}
819843
Err(LookupError::NotFound)

pyrefly/lib/test/scope.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -636,7 +636,7 @@ __all__ += [] # E: Could not find name `__all__`
636636

637637
// https://github.com/facebook/pyrefly/issues/264
638638
testcase!(
639-
bug = "This was implemented but we backed it out due to subtleties - see D82224938. All these should show `Literal['string']`",
639+
bug = "All these should show `Literal['string']`. The issue with comprehension persists, see also the next test case.",
640640
test_class_scope_lookups_when_skip,
641641
r#"
642642
from typing import reveal_type
@@ -650,9 +650,9 @@ class A:
650650
x = 42
651651
def f():
652652
reveal_type(x) # E: revealed type: Literal['string']
653-
lambda_f = lambda: reveal_type(x) # E: revealed type: Literal[42]
653+
lambda_f = lambda: reveal_type(x) # E: revealed type: Literal['string']
654654
class B:
655-
reveal_type(x) # E: revealed type: Literal[42]
655+
reveal_type(x) # E: revealed type: Literal['string']
656656
[reveal_type(x) for _ in range(1)] # E: revealed type: Literal[42]
657657
"#,
658658
);

0 commit comments

Comments
 (0)