diff --git a/pyrefly/lib/alt/solve.rs b/pyrefly/lib/alt/solve.rs index c993a6c007..910cb21320 100644 --- a/pyrefly/lib/alt/solve.rs +++ b/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( diff --git a/pyrefly/lib/binding/binding.rs b/pyrefly/lib/binding/binding.rs index 3eaa23ccaf..774295b879 100644 --- a/pyrefly/lib/binding/binding.rs +++ b/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, + pub self_name: Option, +} + impl DisplayWith 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, TextRange), subject_range: TextRange, }, + /// Track private attribute accesses that need semantic validation. + PrivateAttributeAccess(PrivateAttributeAccessExpect), } impl DisplayWith for BindingExpect { @@ -644,6 +654,13 @@ impl DisplayWith 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, diff --git a/pyrefly/lib/binding/expr.rs b/pyrefly/lib/binding/expr.rs index 1bdfbd6431..c5360ca1ce 100644 --- a/pyrefly/lib/binding/expr.rs +++ b/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 { diff --git a/pyrefly/lib/binding/scope.rs b/pyrefly/lib/binding/scope.rs index e60cbe4ede..4fce9fcf93 100644 --- a/pyrefly/lib/binding/scope.rs +++ b/pyrefly/lib/binding/scope.rs @@ -817,6 +817,12 @@ struct ScopeFunction { is_async: bool, } +#[derive(Clone, Debug)] +pub struct MethodScopeContext { + pub class_idx: Idx, + pub self_name: Option, +} + #[derive(Clone, Debug)] struct ParameterUsage { range: TextRange, @@ -2161,6 +2167,33 @@ impl Scopes { } } + pub fn current_method_context(&self) -> Option { + let mut in_method_scope = false; + let mut method_self_name: Option = 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() { diff --git a/pyrefly/lib/test/attributes.rs b/pyrefly/lib/test/attributes.rs index 8a4167840d..e3b9767b82 100644 --- a/pyrefly/lib/test/attributes.rs +++ b/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#" diff --git a/pyrefly/lib/test/enums.rs b/pyrefly/lib/test/enums.rs index 167ae9cc56..ddd631725a 100644 --- a/pyrefly/lib/test/enums.rs +++ b/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)