Split protected member diagnostics from no-access

crates/pyrefly_config/src/error_kind.rs

@@ -384,6 +384,8 @@ pub enum ErrorKind {
     /// guaranteed. This is a separate error code from BadKeywordArgument to allow
     /// users to opt-in to this stricter check.
     PotentialBadKeywordArgument,
+    /// Accessing a protected class member from outside its defining class or a subclass.
+    ProtectedAccess,
     /// A protocol attribute was first defined inside a method instead of the class body.
     ProtocolImplicitlyDefinedAttribute,
     /// Calling `.cuda()` on a `torch.Tensor` hard-codes the target device.
@@ -657,6 +659,7 @@ impl ErrorKind {
             ErrorKind::OrderedDataclassInheritance => Severity::Warn,
             ErrorKind::PossiblyUninitializedAttribute => Severity::Ignore,
             ErrorKind::PotentialBadKeywordArgument => Severity::Ignore,
+            ErrorKind::ProtectedAccess => Severity::Ignore,
             ErrorKind::PytorchEfficiencyLintCudaCall => Severity::Ignore,
             ErrorKind::PytorchEfficiencyLintItemCall => Severity::Ignore,
             ErrorKind::PytorchEfficiencyLintPrintTensor => Severity::Ignore,

crates/pyrefly_config/src/migration/pyright.rs

@@ -423,7 +423,7 @@ impl RuleOverrides {
         add(self.report_operator_issue, ErrorKind::NotCallable);
         add(self.report_return_type, ErrorKind::BadReturn);
         add(self.report_return_type, ErrorKind::InvalidYield);
-        add(self.report_private_usage, ErrorKind::NoAccess);
+        add(self.report_private_usage, ErrorKind::ProtectedAccess);
         add(self.report_deprecated, ErrorKind::Deprecated);
         add(
             self.report_incompatible_method_override,
@@ -876,4 +876,14 @@ include = ["basedpyright.py"]
         assert!(!config.project_includes.is_empty());
         Ok(())
     }
+
+    #[test]
+    fn test_report_private_usage_mapping() -> anyhow::Result<()> {
+        let pyr = serde_json::from_str::<PyrightConfig>(r#"{"reportPrivateUsage": "warning"}"#)?;
+        let config = pyr.convert();
+        let errors = config.root.errors.expect("expected a diagnostic override");
+        assert_eq!(errors.severity(ErrorKind::ProtectedAccess), Severity::Warn);
+        assert_eq!(errors.severity(ErrorKind::NoAccess), Severity::Error);
+        Ok(())
+    }
 }

crates/pyrefly_python/src/ast.rs

@@ -595,6 +595,10 @@ impl Ast {
         name.starts_with("__") && !name.ends_with("__")
     }
 
+    pub fn is_protected_attr(name: &Name) -> bool {
+        name.len() > 1 && name.starts_with('_') && !name.starts_with("__")
+    }
+
     // Parameters and variables that are prefixed (but not suffixed) with a single underscore
     // are potentially unused, so we should skip some diagnostics/errors.
     // Examples: `_`, `_x`

pyrefly/lib/alt/solve.rs

@@ -3394,6 +3394,71 @@ impl<'ctx, 'answer, Ans: LookupAnswer> AnswersSolver<'ctx, 'answer, Ans> {
         }
     }
 
+    fn check_protected_attribute_access(
+        &self,
+        expect: &PrivateAttributeAccessCheck,
+        value_type: &Type,
+        errors: &ErrorCollector,
+    ) {
+        let Some(owners) = expect
+            .class_idxs
+            .iter()
+            .map(|class_idx| self.get_idx(*class_idx).0.clone())
+            .collect::<Option<Vec<_>>>()
+        else {
+            return;
+        };
+        let inaccessible = value_type.clone().into_unions().into_iter().any(|ty| {
+            let accessed_class = match ty {
+                Type::ClassDef(cls) => Some(cls),
+                Type::ClassType(cls) | Type::SelfType(cls) => Some(cls.into_class_object()),
+                Type::Type(inner) => match *inner {
+                    Type::ClassType(cls) | Type::SelfType(cls) => Some(cls.into_class_object()),
+                    _ => None,
+                },
+                _ => None,
+            };
+            let Some(accessed_class) = accessed_class else {
+                return false;
+            };
+            if matches!(
+                expect.attr.id.as_str(),
+                "_asdict" | "_field_defaults" | "_fields" | "_make" | "_replace"
+            ) && self
+                .get_metadata_for_class(&accessed_class)
+                .named_tuple_metadata()
+                .is_some()
+            {
+                return false;
+            }
+            let Some(member) =
+                self.get_class_member_with_defining_class(&accessed_class, &expect.attr.id)
+            else {
+                return false;
+            };
+            !member.defining_class.module_path().is_interface()
+                && !owners.iter().any(|owner| {
+                    member.defining_class == *owner
+                        || self
+                            .get_mro_for_class(owner)
+                            .ancestors_no_object()
+                            .iter()
+                            .any(|ancestor| ancestor.class_object() == &member.defining_class)
+                })
+        });
+        if inaccessible && self.has_static_attr(value_type, &expect.attr.id) {
+            self.error(
+                errors,
+                expect.attr.range(),
+                ErrorKind::ProtectedAccess,
+                format!(
+                    "Protected attribute `{}` cannot be accessed outside of its defining class or a subclass",
+                    expect.attr.id
+                ),
+            );
+        }
+    }
+
     fn check_private_attribute_access(
         &self,
         expect: &PrivateAttributeAccessCheck,
@@ -3407,28 +3472,32 @@ impl<'ctx, 'answer, Ans: LookupAnswer> AnswersSolver<'ctx, 'answer, Ans> {
         ) {
             return;
         }
-        if let Some(class_idx) = expect.class_idx {
-            let class_binding = self.get_idx(class_idx);
-            let Some(owner) = class_binding.0.as_ref() else {
-                return;
-            };
-            if self
+        if Ast::is_protected_attr(&expect.attr.id) {
+            self.check_protected_attribute_access(expect, &value_type, errors);
+            return;
+        }
+        let owner = expect
+            .class_idxs
+            .first()
+            .and_then(|class_idx| self.get_idx(*class_idx).0.clone());
+        if !expect.class_idxs.is_empty() && owner.is_none() {
+            return;
+        }
+        if let Some(owner) = owner.as_ref()
+            && self
                 .get_class_fields(owner)
                 .is_some_and(|f| f.contains(&expect.attr.id))
-                && self.is_subset_eq(
-                    &value_type,
-                    &self.union(
-                        self.heap.mk_class_def(owner.dupe()),
-                        self.instantiate(owner),
-                    ),
-                )
-            {
-                return; // Valid private attribute access
-            }
+            && self.is_subset_eq(
+                &value_type,
+                &self.union(
+                    self.heap.mk_class_def(owner.dupe()),
+                    self.instantiate(owner),
+                ),
+            )
+        {
+            return;
         }
-        // No defining class to restrict access to — the attribute is only
-        // reachable via dynamic fallback (__getattr__/__getattribute__), so
-        // name mangling is irrelevant.
+        // Dynamic fallback does not establish a class declaration to restrict access to.
         if !self.has_static_attr(&value_type, &expect.attr.id) {
             return;
         }

pyrefly/lib/binding/binding.rs

@@ -128,7 +128,7 @@ assert_words!(KeyDecoratedFunction, 1);
 assert_words!(KeyUndecoratedFunction, 1);
 
 assert_words!(Binding, 4);
-assert_words!(BindingExpect, 12);
+assert_words!(BindingExpect, 8);
 assert_words!(BindingTypeAlias, 6);
 assert_words!(BindingAnnotation, 11);
 assert_words!(BindingClass, 11);
@@ -1215,7 +1215,7 @@ pub enum ExprOrBinding {
 pub struct PrivateAttributeAccessCheck {
     pub value: Expr,
     pub attr: Identifier,
-    pub class_idx: Option<Idx<KeyClass>>,
+    pub class_idxs: Vec<Idx<KeyClass>>,
 }
 
 #[derive(Clone, Debug)]
@@ -1345,7 +1345,7 @@ pub enum BindingExpect {
         range: TextRange,
     },
     /// Track private attribute accesses that need semantic validation.
-    PrivateAttributeAccess(PrivateAttributeAccessCheck),
+    PrivateAttributeAccess(Box<PrivateAttributeAccessCheck>),
     /// Deferred check for uninitialized variables. This is a "dangling" binding
     /// that doesn't affect any other types - it only exists to emit an error at
     /// solve time if any of the termination keys don't have Never type.
@@ -1456,11 +1456,10 @@ impl DisplayWith<Bindings> for BindingExpect {
                 "PrivateAttributeAccess({}, {}, {})",
                 m.display(&expectation.value),
                 expectation.attr.id,
-                if let Some(class_idx) = expectation.class_idx {
-                    format!("{}", ctx.display(class_idx))
-                } else {
-                    "None".to_owned()
-                }
+                intersperse_iter(", ", || expectation
+                    .class_idxs
+                    .iter()
+                    .map(|idx| ctx.display(*idx)))
             ),
             Self::MatchExhaustiveness {
                 subject_idx,

pyrefly/lib/binding/expr.rs

@@ -783,8 +783,8 @@ impl<'a> BindingsBuilder<'a> {
 
         match x {
             Expr::Attribute(attr) => {
-                self.check_private_attribute_usage(attr);
                 self.ensure_expr(&mut attr.value, usage);
+                self.check_private_attribute_usage(attr);
             }
             Expr::Subscript(ExprSubscript { value, slice, .. }) => {
                 // Some subscripts are (or contain) type expressions even when they appear in a
@@ -1255,17 +1255,26 @@ impl<'a> BindingsBuilder<'a> {
     }
 
     fn check_private_attribute_usage(&mut self, attr: &ExprAttribute) {
-        if !Ast::is_mangled_attr(&attr.attr.id) {
+        if !Ast::is_mangled_attr(&attr.attr.id) && !Ast::is_protected_attr(&attr.attr.id) {
             return;
         }
+        let class_idxs = if Ast::is_protected_attr(&attr.attr.id) {
+            self.scopes.enclosing_class_keys()
+        } else {
+            self.scopes
+                .current_class_key()
+                .or_else(|| self.scopes.current_method_context())
+                .into_iter()
+                .collect()
+        };
         let expect = PrivateAttributeAccessCheck {
             value: (*attr.value).clone(),
             attr: attr.attr.clone(),
-            class_idx: self.scopes.current_method_context(),
+            class_idxs,
         };
         self.insert_binding(
             KeyExpect::PrivateAttributeAccess(attr.attr.range()),
-            BindingExpect::PrivateAttributeAccess(expect),
+            BindingExpect::PrivateAttributeAccess(Box::new(expect)),
         );
     }
 

pyrefly/lib/binding/scope.rs

@@ -1931,6 +1931,11 @@ impl Scopes {
         self.current().class_key()
     }
 
+    /// Return enclosing classes from innermost to outermost.
+    pub fn enclosing_class_keys(&self) -> Vec<Idx<KeyClass>> {
+        self.iter_rev().filter_map(Scope::class_key).collect()
+    }
+
     /// Are we anywhere inside a class? If so, return the class object idx.
     /// This function looks at enclosing scopes.
     pub fn enclosing_class_object_idx(&self) -> Option<Idx<Key>> {