diff --git a/src/diagnostics/member_visibility.rs b/src/diagnostics/member_visibility.rs index 64b094946..800ad6aee 100644 --- a/src/diagnostics/member_visibility.rs +++ b/src/diagnostics/member_visibility.rs @@ -129,10 +129,34 @@ struct Rejection { /// The class whose scope the reader has to enter for the access to /// become legal. owner: Arc, - visibility: Visibility, + restriction: Restriction, kind: MemberKind, } +/// The two visibilities that can bar an access. +/// +/// `Visibility` has a third, `Public`, which no rejection can ever +/// carry: the check settles a public member before a `Rejection` is +/// built. Spelling the pair that remains lets the compiler hold that +/// invariant, instead of leaving every `match` on a rejection with an +/// arm for a state that cannot arise. +#[derive(Clone, Copy, PartialEq, Eq)] +enum Restriction { + Private, + Protected, +} + +impl Restriction { + /// `None` for a public member, which nothing can make unreachable. + fn from_visibility(visibility: Visibility) -> Option { + match visibility { + Visibility::Private => Some(Self::Private), + Visibility::Protected => Some(Self::Protected), + Visibility::Public => None, + } + } +} + /// Build the diagnostic message for an access PHP would reject, or /// `None` when the access is fine — or cannot be judged. /// @@ -253,9 +277,9 @@ fn judge_branch( // nothing about the class can make one unreachable — so it is // settled before the scan for a magic handler, which is the only // work here proportional to the class's size. - if visibility == Visibility::Public { + let Some(restriction) = Restriction::from_visibility(visibility) else { return BranchVerdict::Permitted; - } + }; // A class that answers for members the caller cannot see directly // turns the access into a magic-method call rather than an error. @@ -288,13 +312,13 @@ fn judge_branch( member_name, is_static, is_method_call, - visibility, + restriction, class_loader, ) .unwrap_or_else(|| Arc::clone(merged)); return BranchVerdict::Rejected(Rejection { owner, - visibility, + restriction, kind, }); } @@ -302,7 +326,7 @@ fn judge_branch( // Anything at or below the receiver is at or below whatever declared // a protected member, so this settles the common inside-the-hierarchy // case without looking for the declaring class. - if visibility == Visibility::Protected + if restriction == Restriction::Protected && scopes.iter().any(|scope| { scope.fqn() == merged.fqn() || is_subtype_of(scope, merged.fqn().as_str(), class_loader) }) @@ -316,7 +340,7 @@ fn judge_branch( member_name, is_static, is_method_call, - visibility, + restriction, class_loader, ) else { // The provenance walk came up empty, which means something in @@ -327,7 +351,7 @@ fn judge_branch( let rejection = Rejection { owner, - visibility, + restriction, kind, }; if scopes @@ -357,18 +381,17 @@ fn declaring_class( member_name: &str, is_static: bool, is_method_call: bool, - visibility: Visibility, + restriction: Restriction, class_loader: &dyn Fn(&str) -> Option>, ) -> Option> { let raw = class_loader(merged.fqn().as_str())?; - match visibility { - Visibility::Public => None, + match restriction { // A private member is never inherited, so the merge can only // have taken it from the class itself or from a trait used // somewhere up the chain. The nearest level that supplies it // is the scope it belongs to. - Visibility::Private => { + Restriction::Private => { if declared_member(&raw, member_name, is_static, is_method_call).is_some() || declares_through_own_traits( &raw, @@ -395,7 +418,7 @@ fn declaring_class( // declares it non-privately. A private namesake further up is // a different member that the nearer declaration shadows, and // must not be mistaken for the introducer. - Visibility::Protected => { + Restriction::Protected => { let mut owner = None; if declares_non_privately(&raw, member_name, is_static, is_method_call, class_loader) { owner = Some(Arc::clone(&raw)); @@ -530,7 +553,7 @@ fn private_ancestor_declaration( } return Some(Rejection { owner: ancestor, - visibility, + restriction: Restriction::Private, kind, }); } @@ -619,16 +642,13 @@ fn is_accessible( current_class: Option<&ClassInfo>, class_loader: &dyn Fn(&str) -> Option>, ) -> bool { - if rejection.visibility == Visibility::Public { - return true; - } let Some(current) = current_class else { return false; }; if current.fqn() == rejection.owner.fqn() { return true; } - rejection.visibility == Visibility::Protected + rejection.restriction == Restriction::Protected && is_subtype_of(current, rejection.owner.fqn().as_str(), class_loader) } @@ -673,10 +693,9 @@ fn handles_inaccessible_access(class: &ClassInfo, is_static: bool, is_method_cal /// declares the member rather than the one the access went through — /// that is the class whose scope the reader has to enter to fix it. fn build_message(rejection: &Rejection, member_name: &str) -> String { - let modifier = match rejection.visibility { - Visibility::Private => "private", - Visibility::Protected => "protected", - Visibility::Public => "public", + let modifier = match rejection.restriction { + Restriction::Private => "private", + Restriction::Protected => "protected", }; let owner = display_class_name(&rejection.owner); @@ -684,9 +703,9 @@ fn build_message(rejection: &Rejection, member_name: &str) -> String { MemberKind::Method => "call", _ => "access", }; - let scope = match rejection.visibility { - Visibility::Protected => format!("outside {} or its subclasses", owner), - _ => "outside its declaring class".to_string(), + let scope = match rejection.restriction { + Restriction::Protected => format!("outside {} or its subclasses", owner), + Restriction::Private => "outside its declaring class".to_string(), }; format!(