Let the compiler hold the invariant a rejected access is not public

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<ClassInfo>,
-    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<Self> {
+        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,21 +312,21 @@ 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,
         });
     }
 
     // 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<Arc<ClassInfo>>,
 ) -> Option<Arc<ClassInfo>> {
     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<Arc<ClassInfo>>,
 ) -> 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,20 +693,19 @@ 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);
     let verb = match rejection.kind {
         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!(