fix Reading __v from the outside of the class, pyrefly doesn't give any error while Python interpreter gives error #1971

pyrefly/lib/alt/solve.rs

@@ -87,6 +87,7 @@ use crate::binding::binding::KeyUndecoratedFunction;
 use crate::binding::binding::LastStmt;
 use crate::binding::binding::LinkedKey;
 use crate::binding::binding::NoneIfRecursive;
+use crate::binding::binding::PrivateAttributeAccessExpect;
 use crate::binding::binding::RaisedException;
 use crate::binding::binding::ReturnTypeKind;
 use crate::binding::binding::SizeExpectation;
@@ -1795,10 +1796,97 @@ impl<'a, Ans: LookupAnswer> AnswersSolver<'a, Ans> {
                 range,
                 errors,
             ),
+            BindingExpect::PrivateAttributeAccess(expectation) => {
+                self.check_private_attribute_access(expectation, errors);
+            }
         }
         Arc::new(EmptyAnswer)
     }
 
+    fn check_private_attribute_access(
+        &self,
+        expect: &PrivateAttributeAccessExpect,
+        errors: &ErrorCollector,
+    ) {
+        let class_binding = self.get_idx(expect.class_idx);
+        let Some(owner) = class_binding.0.as_ref() else {
+            return;
+        };
+        if expect
+            .self_name
+            .as_ref()
+            .is_some_and(|name| Self::expr_matches_name(&expect.value, name))
+        {
+            return;
+        }
+        let value_type = self.expr_infer(&expect.value, errors);
+        if self.private_attr_type_allows(owner, &value_type) {
+            return;
+        }
+        self.error(
+            errors,
+            expect.attr.range(),
+            ErrorInfo::Kind(ErrorKind::NoAccess),
+            format!(
+                "Private attribute `{}` cannot be accessed outside of its defining class",
+                expect.attr.id
+            ),
+        );
+    }
+
+    fn expr_matches_name(expr: &Expr, expected: &Name) -> bool {
+        matches!(expr, Expr::Name(name) if &name.id == expected)
+    }
+
+    fn private_attr_type_allows(&self, owner: &Class, ty: &Type) -> bool {
+        match ty {
+            Type::ClassType(cls) | Type::SelfType(cls) => {
+                self.has_superclass(cls.class_object(), owner)
+            }
+            Type::ClassDef(cls) => self.has_superclass(cls, owner),
+            Type::Union(box union) => union
+                .members
+                .iter()
+                .all(|member| self.private_attr_type_allows(owner, member)),
+            Type::Intersect(box (members, fallback)) => {
+                members
+                    .iter()
+                    .all(|member| self.private_attr_type_allows(owner, member))
+                    && self.private_attr_type_allows(owner, fallback)
+            }
+            Type::Type(inner) => self.private_attr_type_allows(owner, inner),
+            Type::TypeAlias(alias) => {
+                self.private_attr_type_allows(owner, &alias.as_value(self.stdlib))
+            }
+            Type::Quantified(q) | Type::QuantifiedValue(q) => {
+                self.private_attr_restriction_allows(owner, q.restriction())
+            }
+            Type::TypeVar(var) => self.private_attr_restriction_allows(owner, var.restriction()),
+            Type::Literal(lit) => match lit {
+                Lit::Enum(lit_enum) => self.has_superclass(lit_enum.class.class_object(), owner),
+                _ => false,
+            },
+            Type::Never(_) => true,
+            Type::TypeGuard(inner) | Type::TypeIs(inner) => {
+                self.private_attr_type_allows(owner, inner)
+            }
+            _ => false,
+        }
+    }
+
+    fn private_attr_restriction_allows(&self, owner: &Class, restriction: &Restriction) -> bool {
+        match restriction {
+            Restriction::Bound(bound) => self.private_attr_type_allows(owner, bound),
+            Restriction::Constraints(constraints) => {
+                !constraints.is_empty()
+                    && constraints
+                        .iter()
+                        .all(|constraint| self.private_attr_type_allows(owner, constraint))
+            }
+            Restriction::Unrestricted => false,
+        }
+    }
+
     /// Check if a module path should be skipped for indexing purposes.
     /// Skips typeshed (bundled stdlib and third-party stubs) and site-packages (external libraries).
     fn should_skip_module_for_indexing(

pyrefly/lib/binding/binding.rs

@@ -100,7 +100,7 @@ assert_words!(KeyDecoratedFunction, 1);
 assert_words!(KeyUndecoratedFunction, 1);
 
 assert_words!(Binding, 11);
-assert_words!(BindingExpect, 11);
+assert_words!(BindingExpect, 19);
 assert_words!(BindingAnnotation, 15);
 assert_words!(BindingClass, 23);
 assert_words!(BindingTParams, 10);
@@ -555,6 +555,14 @@ pub enum ExprOrBinding {
     Binding(Binding),
 }
 
+#[derive(Clone, Debug)]
+pub struct PrivateAttributeAccessExpect {
+    pub value: Expr,
+    pub attr: Identifier,
+    pub class_idx: Idx<KeyClass>,
+    pub self_name: Option<Name>,
+}
+
 impl DisplayWith<Bindings> for ExprOrBinding {
     fn fmt(&self, f: &mut fmt::Formatter<'_>, ctx: &Bindings) -> fmt::Result {
         match self {
@@ -594,6 +602,8 @@ pub enum BindingExpect {
         narrow_ops_for_fall_through: (Box<NarrowOp>, TextRange),
         subject_range: TextRange,
     },
+    /// Track private attribute accesses that need semantic validation.
+    PrivateAttributeAccess(PrivateAttributeAccessExpect),
 }
 
 impl DisplayWith<Bindings> for BindingExpect {
@@ -644,6 +654,13 @@ impl DisplayWith<Bindings> for BindingExpect {
                 ctx.display(*existing),
                 name
             ),
+            Self::PrivateAttributeAccess(expectation) => write!(
+                f,
+                "PrivateAttributeAccess({}, {}, {})",
+                m.display(&expectation.value),
+                expectation.attr.id,
+                ctx.display(expectation.class_idx)
+            ),
             Self::MatchExhaustiveness {
                 subject_idx,
                 subject_range: range,

pyrefly/lib/binding/expr.rs

@@ -33,15 +33,18 @@ use vec1::vec1;
 
 use crate::binding::binding::Binding;
 use crate::binding::binding::BindingDecorator;
+use crate::binding::binding::BindingExpect;
 use crate::binding::binding::BindingYield;
 use crate::binding::binding::BindingYieldFrom;
 use crate::binding::binding::IsAsync;
 use crate::binding::binding::Key;
 use crate::binding::binding::KeyDecorator;
+use crate::binding::binding::KeyExpect;
 use crate::binding::binding::KeyYield;
 use crate::binding::binding::KeyYieldFrom;
 use crate::binding::binding::LinkedKey;
 use crate::binding::binding::NarrowUseLocation;
+use crate::binding::binding::PrivateAttributeAccessExpect;
 use crate::binding::binding::SuperStyle;
 use crate::binding::bindings::AwaitContext;
 use crate::binding::bindings::BindingsBuilder;
@@ -518,6 +521,10 @@ impl<'a> BindingsBuilder<'a> {
         self.with_semantic_checker(|semantic, context| semantic.visit_expr(x, context));
 
         match x {
+            Expr::Attribute(attr) => {
+                self.check_private_attribute_usage(attr);
+                self.ensure_expr(&mut attr.value, usage);
+            }
             Expr::If(x) => {
                 // Ternary operation. We treat it like an if/else statement.
                 self.start_fork_and_branch(x.range);
@@ -799,6 +806,33 @@ impl<'a> BindingsBuilder<'a> {
         }
     }
 
+    fn check_private_attribute_usage(&mut self, attr: &ExprAttribute) {
+        if !Ast::is_mangled_attr(&attr.attr.id) {
+            return;
+        }
+        if let Some(method_ctx) = self.scopes.current_method_context() {
+            let expect = PrivateAttributeAccessExpect {
+                value: (*attr.value).clone(),
+                attr: attr.attr.clone(),
+                class_idx: method_ctx.class_idx,
+                self_name: method_ctx.self_name.clone(),
+            };
+            self.insert_binding(
+                KeyExpect(attr.attr.range()),
+                BindingExpect::PrivateAttributeAccess(expect),
+            );
+        } else {
+            self.error(
+                attr.attr.range,
+                ErrorInfo::Kind(ErrorKind::NoAccess),
+                format!(
+                    "Private attribute `{}` cannot be accessed outside of its defining class",
+                    attr.attr.id
+                ),
+            );
+        }
+    }
+
     /// Execute through the expr, ensuring every name has a binding.
     pub fn ensure_expr_opt(&mut self, x: Option<&mut Expr>, usage: &mut Usage) {
         if let Some(x) = x {

pyrefly/lib/binding/scope.rs

@@ -817,6 +817,12 @@ struct ScopeFunction {
     is_async: bool,
 }
 
+#[derive(Clone, Debug)]
+pub struct MethodScopeContext {
+    pub class_idx: Idx<KeyClass>,
+    pub self_name: Option<Name>,
+}
+
 #[derive(Clone, Debug)]
 struct ParameterUsage {
     range: TextRange,
@@ -2161,6 +2167,33 @@ impl Scopes {
         }
     }
 
+    pub fn current_method_context(&self) -> Option<MethodScopeContext> {
+        let mut in_method_scope = false;
+        let mut method_self_name: Option<Name> = None;
+        for scope in self.iter_rev() {
+            match &scope.kind {
+                ScopeKind::Function(_) if !in_method_scope => {
+                    return None;
+                }
+                ScopeKind::Method(method_scope) if !in_method_scope => {
+                    in_method_scope = true;
+                    method_self_name = method_scope
+                        .self_name
+                        .as_ref()
+                        .map(|identifier| identifier.id.clone());
+                }
+                ScopeKind::Class(class_scope) if in_method_scope => {
+                    return Some(MethodScopeContext {
+                        class_idx: class_scope.indices.class_idx,
+                        self_name: method_self_name,
+                    });
+                }
+                _ => {}
+            }
+        }
+        None
+    }
+
     /// Get the name of the (innermost) enclosing class, if any.
     pub fn enclosing_class_name(&self) -> Option<&Identifier> {
         for scope in self.iter_rev() {

pyrefly/lib/test/attributes.rs

@@ -1329,6 +1329,61 @@ def test(x: LiteralString):
 "#,
 );
 
+testcase!(
+    test_private_attribute_outside_class,
+    r#"
+class A:
+    __secret: int = 0
+
+exposed = A.__secret  # E: Private attribute `__secret` cannot be accessed outside of its defining class
+
+class B:
+    leaked = A.__secret  # E: Private attribute `__secret` cannot be accessed outside of its defining class
+"#,
+);
+
+testcase!(
+    test_private_attribute_inside_class,
+    r#"
+class A:
+    __secret: int = 0
+
+    def reveal(self) -> int:
+        return self.__secret
+
+    @classmethod
+    def reveal_cls(cls) -> int:
+        return cls.__secret
+
+    @staticmethod
+    def reveal_static() -> int:
+        return A.__secret
+"#,
+);
+
+testcase!(
+    test_private_attribute_on_peer_instance,
+    r#"
+class F:
+    __v: int
+
+    def equals(self, other: "F") -> bool:
+        return self.__v == other.__v
+"#,
+);
+
+testcase!(
+    test_private_attribute_in_subclass_method,
+    r#"
+class A:
+    __secret: int = 0
+
+class B(A):
+    def leak(self) -> int:
+        return A.__secret  # E: Private attribute `__secret` cannot be accessed outside of its defining class
+"#,
+);
+
 testcase!(
     test_attribute_access_on_type_callable,
     r#"

pyrefly/lib/test/enums.rs

@@ -45,7 +45,7 @@ class MyEnum(Enum):
 
 assert_type(MyEnum.X, Literal[MyEnum.X])
 assert_type(MyEnum["X"], Literal[MyEnum.X])
-assert_type(MyEnum.__PRIVATE, int)
+assert_type(MyEnum.__PRIVATE, int)  # E: Private attribute `__PRIVATE` cannot be accessed outside of its defining class
 assert_type(MyEnum.X.name, Literal["X"])
 assert_type(MyEnum.X._name_, Literal["X"])
 assert_type(MyEnum.X.value, int)