Diagnose public protocol witnessed by an internal protocol extension

Private member witnessing a public constraint should be deprecated.
Previously existing workaround is checked and compiler emits a warning
when strict access check is not passed. Test files were fixed to expect
the corresponding warning.

rdar://74904373

Author: sepy97

include/swift/AST/DiagnosticsSema.def

@@ -3077,6 +3077,12 @@ ERROR(witness_not_as_sendable,none,
 NOTE(less_sendable_reqt_here,none,
      "expected sendability to match requirement here",
      ())
+WARNING(witness_not_accessible_strict_check,none,
+      "%select{initializer %1|method %1|%select{|setter for }2property %1"
+      "|subscript%select{| setter}2}0 must be as accessible as its enclosing "
+      "type because it matches a requirement in protocol %5",
+      (RequirementKind, const ValueDecl *, bool, AccessLevel, AccessLevel,
+       const ProtocolDecl *))
 ERROR(witness_not_accessible_type,none,
       "%select{initializer %1|method %1|%select{|setter for }2property %1"
       "|subscript%select{| setter}2}0 must be as accessible as its enclosing "

include/swift/AST/RequirementMatch.h

@@ -218,6 +218,9 @@ enum class CheckKind : unsigned {
   /// The witness is less accessible than the requirement.
   Access,
 
+  /// Strict check for access holes making swiftinterface not usable
+  AccessStrict,
+
   /// The witness needs to be @usableFromInline.
   UsableFromInline,
 
@@ -265,8 +268,12 @@ class RequirementCheck {
     ASSERT(kind != CheckKind::Availability);
   }
 
-  RequirementCheck(AccessScope requiredAccessScope, bool forSetter)
-      : Kind(CheckKind::Access), Access{requiredAccessScope, forSetter} {}
+  RequirementCheck(CheckKind accessKind, AccessScope requiredAccessScope,
+                   bool forSetter)
+      : Kind(accessKind), Access{requiredAccessScope, forSetter} {
+    ASSERT(accessKind == CheckKind::Access ||
+           accessKind == CheckKind::AccessStrict);
+  }
 
   RequirementCheck(AvailabilityConstraint constraint,
                    AvailabilityContext requiredContext)
@@ -291,7 +298,7 @@ class RequirementCheck {
   /// The required access scope for checks that failed due to the witness being
   /// less accessible than the requirement.
   AccessScope getRequiredAccessScope() const {
-    ASSERT(Kind == CheckKind::Access);
+    ASSERT(Kind == CheckKind::Access || Kind == CheckKind::AccessStrict);
     return Access.requiredScope;
   }
 

lib/Sema/TypeCheckProtocol.cpp

@@ -1865,8 +1865,15 @@ RequirementCheck WitnessChecker::checkWitness(ValueDecl *requirement,
       std::make_pair(AccessScope::getPublic(), false));
 
   bool isSetter = false;
+  if (match.Witness->isAccessibleFrom(requiredAccessLevel.getDeclContext(),
+                                      /*forConformance=*/true) &&
+      !match.Witness->isAccessibleFrom(requiredAccessLevel.getDeclContext(),
+                                       /*forConformance=*/false)) {
+    return RequirementCheck(CheckKind::AccessStrict, requiredAccessLevel,
+                            isSetter);
+  }
   if (checkWitnessAccess(DC, requirement, match.Witness, &isSetter))
-    return RequirementCheck(requiredAccessLevel, isSetter);
+    return RequirementCheck(CheckKind::Access, requiredAccessLevel, isSetter);
 
   if (mustBeUsableFromInline) {
     bool witnessIsUsableFromInline = match.Witness->getFormalAccessScope(
@@ -4414,7 +4421,7 @@ ConformanceChecker::resolveWitnessViaLookup(ValueDecl *requirement) {
     switch (check.getKind()) {
     case CheckKind::Success:
       break;
-
+    case CheckKind::AccessStrict:
     case CheckKind::Access: {
       // Swift 4.2 relaxed some rules for protocol witness matching.
       //
@@ -4446,14 +4453,15 @@ ConformanceChecker::resolveWitnessViaLookup(ValueDecl *requirement) {
         auto diagKind = protoForcesAccess
                           ? diag::witness_not_accessible_proto
                           : diag::witness_not_accessible_type;
+        if (check.getKind() == CheckKind::AccessStrict)
+          diagKind = diag::witness_not_accessible_strict_check;
         bool isSetter = check.isForSetterAccess();
 
         auto &diags = DC->getASTContext().Diags;
         diags.diagnose(getLocForDiagnosingWitness(conformance, witness),
                        diagKind, getProtocolRequirementKind(requirement),
                        witness, isSetter, requiredAccess,
-                       protoAccessScope.accessLevelForDiagnostics(),
-                       proto);
+                       protoAccessScope.accessLevelForDiagnostics(), proto);
 
         auto *decl = dyn_cast<AbstractFunctionDecl>(witness);
         if (decl && decl->isSynthesized())

test/Generics/rdar123013710.swift

@@ -9,13 +9,14 @@ public protocol P {
 }
 
 protocol Q: P where B == Never {} // expected-error@:10 {{circular reference}}
-
+// expected-warning@+1 {{property 'b' must be as accessible as its enclosing type because it matches a requirement in protocol 'P'}}
 extension Never: Q, P { // expected-note@:1 {{through reference here}}
    public typealias A = Never
    public static func f() -> Any? { nil }
 }
 
 extension Q {
+// expected-note@+1 {{mark the property as 'public' to satisfy the requirement}}
    public var b: Never { fatalError() } // expected-note {{through reference here}}
 }
 

test/SILGen/internal_protocol_refines_public_protocol_with_public_default_implementation.swift

@@ -8,8 +8,8 @@ public protocol A {
 protocol B: A { }
 
 extension B {
-    public subscript() -> Int { return 0 }
+    public subscript() -> Int { return 0 } // expected-note {{mark the subscript as 'public' to satisfy the requirement}}
 }
 
-public struct S: B {
+public struct S: B { // expected-warning {{subscript must be as accessible as its enclosing type because it matches a requirement in protocol 'A'}}
 }

test/decl/protocol/conforms/access_corner_case.swift

@@ -26,8 +26,8 @@ private protocol S : R {
 }
 
 extension S {
-  public func publicRequirement() {}
-  internal func internalRequirement() {}
+  public func publicRequirement() {} // expected-note {{mark the instance method as 'public' to satisfy the requirement}}
+  internal func internalRequirement() {} // expected-note {{mark the instance method as 'internal' to satisfy the requirement}}
   fileprivate func fileprivateRequirement() {}
   fileprivate func privateRequirement() {}
 
@@ -38,6 +38,8 @@ extension S {
 public struct T : S {}
 // expected-error@-1 {{type 'T' does not conform to protocol 'S'}}
 // expected-note@-2 {{add stubs for conformance}}
+// expected-warning@-3 {{method 'internalRequirement()' must be as accessible as its enclosing type because it matches a requirement in protocol 'Q'}}
+// expected-warning@-4 {{method 'publicRequirement()' must be as accessible as its enclosing type because it matches a requirement in protocol 'P'}}
 
 protocol Qpkg : Pkg {
   func internalRequirement()
@@ -53,9 +55,9 @@ private protocol Spkg : Rpkg {
 }
 
 extension Spkg {
-  public func publicRequirement() {}
-  package func packageRequirement() {}
-  internal func internalRequirement() {}
+  public func publicRequirement() {} // expected-note {{mark the instance method as 'public' to satisfy the requirement}}
+  package func packageRequirement() {} // expected-note {{mark the instance method as 'package' to satisfy the requirement}}
+  internal func internalRequirement() {} // expected-note {{mark the instance method as 'internal' to satisfy the requirement}}
   fileprivate func fileprivateRequirement() {}
   fileprivate func privateRequirement() {}
 
@@ -66,6 +68,9 @@ extension Spkg {
 public struct Tpkg : Spkg {}
 // expected-error@-1 {{type 'Tpkg' does not conform to protocol 'Spkg'}}
 // expected-note@-2 {{add stubs for conformance}}
+// expected-warning@-3 {{method 'internalRequirement()' must be as accessible as its enclosing type because it matches a requirement in protocol 'Qpkg'}}
+// expected-warning@-4 {{method 'packageRequirement()' must be as accessible as its enclosing type because it matches a requirement in protocol 'Pkg'}}
+// expected-warning@-5 {{method 'publicRequirement()' must be as accessible as its enclosing type because it matches a requirement in protocol 'P'}}
 
 // This is also OK
 @usableFromInline
@@ -111,3 +116,15 @@ extension Q2pkg {
 }
 
 public struct T2pkg : Q2pkg {} // expected-error {{method 'publicRequirement()' must be declared public because it matches a requirement in public protocol 'P2'}}
+
+public struct Foo {
+  public init(value: Int) {}
+}
+public protocol PublicProtocol {
+  init?(integer: Int)
+}
+protocol InternalProtocol: PublicProtocol {}
+extension InternalProtocol {
+  public init(integer: Int) {} // expected-note {{mark the initializer as 'public' to satisfy the requirement}}
+}
+extension Foo: PublicProtocol, InternalProtocol {} // expected-warning {{initializer 'init(integer:)' must be as accessible as its enclosing type because it matches a requirement in protocol 'PublicProtocol'}}