Skip to content

Commit 9622650

Browse files
more strict
1 parent d63a8e5 commit 9622650

File tree

5 files changed

+197
-3
lines changed

5 files changed

+197
-3
lines changed

pyrefly/lib/alt/solve.rs

Lines changed: 88 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -87,6 +87,7 @@ use crate::binding::binding::KeyUndecoratedFunction;
8787
use crate::binding::binding::LastStmt;
8888
use crate::binding::binding::LinkedKey;
8989
use crate::binding::binding::NoneIfRecursive;
90+
use crate::binding::binding::PrivateAttributeAccessExpect;
9091
use crate::binding::binding::RaisedException;
9192
use crate::binding::binding::ReturnTypeKind;
9293
use crate::binding::binding::SizeExpectation;
@@ -1783,10 +1784,97 @@ impl<'a, Ans: LookupAnswer> AnswersSolver<'a, Ans> {
17831784
);
17841785
}
17851786
}
1787+
BindingExpect::PrivateAttributeAccess(expectation) => {
1788+
self.check_private_attribute_access(expectation, errors);
1789+
}
17861790
}
17871791
Arc::new(EmptyAnswer)
17881792
}
17891793

1794+
fn check_private_attribute_access(
1795+
&self,
1796+
expect: &PrivateAttributeAccessExpect,
1797+
errors: &ErrorCollector,
1798+
) {
1799+
let class_binding = self.get_idx(expect.class_idx);
1800+
let Some(owner) = class_binding.0.as_ref() else {
1801+
return;
1802+
};
1803+
if expect
1804+
.self_name
1805+
.as_ref()
1806+
.is_some_and(|name| Self::expr_matches_name(&expect.value, name))
1807+
{
1808+
return;
1809+
}
1810+
let value_type = self.expr_infer(&expect.value, errors);
1811+
if self.private_attr_type_allows(owner, &value_type) {
1812+
return;
1813+
}
1814+
self.error(
1815+
errors,
1816+
expect.attr.range(),
1817+
ErrorInfo::Kind(ErrorKind::NoAccess),
1818+
format!(
1819+
"Private attribute `{}` cannot be accessed outside of its defining class",
1820+
expect.attr.id
1821+
),
1822+
);
1823+
}
1824+
1825+
fn expr_matches_name(expr: &Expr, expected: &Name) -> bool {
1826+
matches!(expr, Expr::Name(name) if &name.id == expected)
1827+
}
1828+
1829+
fn private_attr_type_allows(&self, owner: &Class, ty: &Type) -> bool {
1830+
match ty {
1831+
Type::ClassType(cls) | Type::SelfType(cls) => {
1832+
self.has_superclass(cls.class_object(), owner)
1833+
}
1834+
Type::ClassDef(cls) => self.has_superclass(cls, owner),
1835+
Type::Union(box union) => union
1836+
.members
1837+
.iter()
1838+
.all(|member| self.private_attr_type_allows(owner, member)),
1839+
Type::Intersect(box (members, fallback)) => {
1840+
members
1841+
.iter()
1842+
.all(|member| self.private_attr_type_allows(owner, member))
1843+
&& self.private_attr_type_allows(owner, fallback)
1844+
}
1845+
Type::Type(inner) => self.private_attr_type_allows(owner, inner),
1846+
Type::TypeAlias(alias) => {
1847+
self.private_attr_type_allows(owner, &alias.as_value(self.stdlib))
1848+
}
1849+
Type::Quantified(q) | Type::QuantifiedValue(q) => {
1850+
self.private_attr_restriction_allows(owner, q.restriction())
1851+
}
1852+
Type::TypeVar(var) => self.private_attr_restriction_allows(owner, var.restriction()),
1853+
Type::Literal(lit) => match lit {
1854+
Lit::Enum(lit_enum) => self.has_superclass(lit_enum.class.class_object(), owner),
1855+
_ => false,
1856+
},
1857+
Type::Never(_) => true,
1858+
Type::TypeGuard(inner) | Type::TypeIs(inner) => {
1859+
self.private_attr_type_allows(owner, inner)
1860+
}
1861+
_ => false,
1862+
}
1863+
}
1864+
1865+
fn private_attr_restriction_allows(&self, owner: &Class, restriction: &Restriction) -> bool {
1866+
match restriction {
1867+
Restriction::Bound(bound) => self.private_attr_type_allows(owner, bound),
1868+
Restriction::Constraints(constraints) => {
1869+
!constraints.is_empty()
1870+
&& constraints
1871+
.iter()
1872+
.all(|constraint| self.private_attr_type_allows(owner, constraint))
1873+
}
1874+
Restriction::Unrestricted => false,
1875+
}
1876+
}
1877+
17901878
/// Check if a module path should be skipped for indexing purposes.
17911879
/// Skips typeshed (bundled stdlib and third-party stubs) and site-packages (external libraries).
17921880
fn should_skip_module_for_indexing(

pyrefly/lib/binding/binding.rs

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -99,7 +99,7 @@ assert_words!(KeyDecoratedFunction, 1);
9999
assert_words!(KeyUndecoratedFunction, 1);
100100

101101
assert_words!(Binding, 11);
102-
assert_words!(BindingExpect, 11);
102+
assert_words!(BindingExpect, 19);
103103
assert_words!(BindingAnnotation, 15);
104104
assert_words!(BindingClass, 23);
105105
assert_words!(BindingTParams, 10);
@@ -554,6 +554,14 @@ pub enum ExprOrBinding {
554554
Binding(Binding),
555555
}
556556

557+
#[derive(Clone, Debug)]
558+
pub struct PrivateAttributeAccessExpect {
559+
pub value: Expr,
560+
pub attr: Identifier,
561+
pub class_idx: Idx<KeyClass>,
562+
pub self_name: Option<Name>,
563+
}
564+
557565
impl DisplayWith<Bindings> for ExprOrBinding {
558566
fn fmt(&self, f: &mut fmt::Formatter<'_>, ctx: &Bindings) -> fmt::Result {
559567
match self {
@@ -582,6 +590,8 @@ pub enum BindingExpect {
582590
},
583591
/// Expression used in a boolean context (`bool()`, `if`, or `while`)
584592
Bool(Expr),
593+
/// Track private attribute accesses that need semantic validation.
594+
PrivateAttributeAccess(PrivateAttributeAccessExpect),
585595
}
586596

587597
impl DisplayWith<Bindings> for BindingExpect {
@@ -632,6 +642,13 @@ impl DisplayWith<Bindings> for BindingExpect {
632642
ctx.display(*existing),
633643
name
634644
),
645+
Self::PrivateAttributeAccess(expectation) => write!(
646+
f,
647+
"PrivateAttributeAccess({}, {}, {})",
648+
m.display(&expectation.value),
649+
expectation.attr.id,
650+
ctx.display(expectation.class_idx)
651+
),
635652
}
636653
}
637654
}

pyrefly/lib/binding/expr.rs

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -33,15 +33,18 @@ use vec1::vec1;
3333

3434
use crate::binding::binding::Binding;
3535
use crate::binding::binding::BindingDecorator;
36+
use crate::binding::binding::BindingExpect;
3637
use crate::binding::binding::BindingYield;
3738
use crate::binding::binding::BindingYieldFrom;
3839
use crate::binding::binding::IsAsync;
3940
use crate::binding::binding::Key;
4041
use crate::binding::binding::KeyDecorator;
42+
use crate::binding::binding::KeyExpect;
4143
use crate::binding::binding::KeyYield;
4244
use crate::binding::binding::KeyYieldFrom;
4345
use crate::binding::binding::LinkedKey;
4446
use crate::binding::binding::NarrowUseLocation;
47+
use crate::binding::binding::PrivateAttributeAccessExpect;
4548
use crate::binding::binding::SuperStyle;
4649
use crate::binding::bindings::AwaitContext;
4750
use crate::binding::bindings::BindingsBuilder;
@@ -801,10 +804,21 @@ impl<'a> BindingsBuilder<'a> {
801804
}
802805

803806
fn check_private_attribute_usage(&mut self, attr: &ExprAttribute) {
804-
if self.scopes.in_function_scope() {
807+
if !Ast::is_mangled_attr(&attr.attr.id) {
805808
return;
806809
}
807-
if Ast::is_mangled_attr(&attr.attr.id) {
810+
if let Some(method_ctx) = self.scopes.current_method_context() {
811+
let expect = PrivateAttributeAccessExpect {
812+
value: (*attr.value).clone(),
813+
attr: attr.attr.clone(),
814+
class_idx: method_ctx.class_idx,
815+
self_name: method_ctx.self_name.clone(),
816+
};
817+
self.insert_binding(
818+
KeyExpect(attr.attr.range()),
819+
BindingExpect::PrivateAttributeAccess(expect),
820+
);
821+
} else {
808822
self.error(
809823
attr.attr.range,
810824
ErrorInfo::Kind(ErrorKind::NoAccess),

pyrefly/lib/binding/scope.rs

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -817,6 +817,12 @@ struct ScopeFunction {
817817
is_async: bool,
818818
}
819819

820+
#[derive(Clone, Debug)]
821+
pub struct MethodScopeContext {
822+
pub class_idx: Idx<KeyClass>,
823+
pub self_name: Option<Name>,
824+
}
825+
820826
#[derive(Clone, Debug)]
821827
struct ParameterUsage {
822828
range: TextRange,
@@ -2161,6 +2167,33 @@ impl Scopes {
21612167
}
21622168
}
21632169

2170+
pub fn current_method_context(&self) -> Option<MethodScopeContext> {
2171+
let mut in_method_scope = false;
2172+
let mut method_self_name: Option<Name> = None;
2173+
for scope in self.iter_rev() {
2174+
match &scope.kind {
2175+
ScopeKind::Function(_) if !in_method_scope => {
2176+
return None;
2177+
}
2178+
ScopeKind::Method(method_scope) if !in_method_scope => {
2179+
in_method_scope = true;
2180+
method_self_name = method_scope
2181+
.self_name
2182+
.as_ref()
2183+
.map(|identifier| identifier.id.clone());
2184+
}
2185+
ScopeKind::Class(class_scope) if in_method_scope => {
2186+
return Some(MethodScopeContext {
2187+
class_idx: class_scope.indices.class_idx,
2188+
self_name: method_self_name,
2189+
});
2190+
}
2191+
_ => {}
2192+
}
2193+
}
2194+
None
2195+
}
2196+
21642197
/// Get the name of the (innermost) enclosing class, if any.
21652198
pub fn enclosing_class_name(&self) -> Option<&Identifier> {
21662199
for scope in self.iter_rev() {

pyrefly/lib/test/attributes.rs

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1261,6 +1261,48 @@ class B:
12611261
"#,
12621262
);
12631263

1264+
testcase!(
1265+
test_private_attribute_inside_class,
1266+
r#"
1267+
class A:
1268+
__secret: int = 0
1269+
1270+
def reveal(self) -> int:
1271+
return self.__secret
1272+
1273+
@classmethod
1274+
def reveal_cls(cls) -> int:
1275+
return cls.__secret
1276+
1277+
@staticmethod
1278+
def reveal_static() -> int:
1279+
return A.__secret
1280+
"#,
1281+
);
1282+
1283+
testcase!(
1284+
test_private_attribute_on_peer_instance,
1285+
r#"
1286+
class F:
1287+
__v: int
1288+
1289+
def equals(self, other: "F") -> bool:
1290+
return self.__v == other.__v
1291+
"#,
1292+
);
1293+
1294+
testcase!(
1295+
test_private_attribute_in_subclass_method,
1296+
r#"
1297+
class A:
1298+
__secret: int = 0
1299+
1300+
class B(A):
1301+
def leak(self) -> int:
1302+
return A.__secret # E: Private attribute `__secret` cannot be accessed outside of its defining class
1303+
"#,
1304+
);
1305+
12641306
testcase!(
12651307
test_attribute_access_on_type_callable,
12661308
r#"

0 commit comments

Comments
 (0)