Skip to content

Commit 9222dfd

Browse files
owenvjrose-apple
authored andcommitted
[Sema] Fix protocol refinement access control message (#26855)
Previously, the protocol refinement access control error would always assume a protocol was being refined. Change it to instead refer to the appropriate declaration kind. Resolves SR-9195
1 parent 7aad090 commit 9222dfd

File tree

5 files changed

+87
-35
lines changed

5 files changed

+87
-35
lines changed

include/swift/AST/DiagnosticsSema.def

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1836,16 +1836,16 @@ ERROR(protocol_access,none,
18361836
"|private or fileprivate}4 because %select{it refines|its 'where' clause uses}2"
18371837
"|%select{in this context|fileprivate|internal|public|%error}1 "
18381838
"%select{protocol cannot refine|protocol's 'where' clause cannot use}2}0 "
1839-
"%select{a private|a fileprivate|an internal|%error|%error}3 protocol",
1840-
(bool, AccessLevel, bool, AccessLevel, bool))
1839+
"%select{a private|a fileprivate|an internal|%error|%error}3 %5",
1840+
(bool, AccessLevel, bool, AccessLevel, bool, DescriptiveDeclKind))
18411841
WARNING(protocol_access_warn,none,
18421842
"%select{protocol should be declared "
18431843
"%select{private|fileprivate|internal|%error|%error}1 because "
18441844
"%select{it refines|its 'where' clause uses}2"
18451845
"|%select{in this context|fileprivate|internal|public|%error}1 "
18461846
"%select{protocol should not refine|protocol's 'where' clause should not use}2}0 "
1847-
"%select{a private|a fileprivate|an internal|%error|%error}3 protocol",
1848-
(bool, AccessLevel, bool, AccessLevel, bool))
1847+
"%select{a private|a fileprivate|an internal|%error|%error}3 %5",
1848+
(bool, AccessLevel, bool, AccessLevel, bool, DescriptiveDeclKind))
18491849
ERROR(protocol_usable_from_inline,none,
18501850
"protocol %select{refined|used}0 by '@usableFromInline' protocol "
18511851
"must be '@usableForInline' or public", (bool))

lib/Sema/TypeCheckAccess.cpp

Lines changed: 33 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -779,6 +779,7 @@ class AccessControlChecker : public AccessControlCheckerBase,
779779
auto minAccessScope = AccessScope::getPublic();
780780
const TypeRepr *complainRepr = nullptr;
781781
auto downgradeToWarning = DowngradeToWarning::No;
782+
DescriptiveDeclKind declKind = DescriptiveDeclKind::Protocol;
782783

783784
// FIXME: Hack to ensure that we've computed the types involved here.
784785
ASTContext &ctx = proto->getASTContext();
@@ -789,6 +790,15 @@ class AccessControlChecker : public AccessControlCheckerBase,
789790
Type());
790791
}
791792

793+
auto declKindForType = [](Type type) {
794+
if (isa<TypeAliasType>(type.getPointer()))
795+
return DescriptiveDeclKind::TypeAlias;
796+
else if (auto nominal = type->getAnyNominal())
797+
return nominal->getDescriptiveKind();
798+
else
799+
return DescriptiveDeclKind::Type;
800+
};
801+
792802
std::for_each(proto->getInherited().begin(),
793803
proto->getInherited().end(),
794804
[&](TypeLoc requirement) {
@@ -803,29 +813,31 @@ class AccessControlChecker : public AccessControlCheckerBase,
803813
complainRepr = thisComplainRepr;
804814
protocolControlErrorKind = PCEK_Refine;
805815
downgradeToWarning = downgradeDiag;
816+
declKind = declKindForType(requirement.getType());
806817
}
807818
});
808819
});
809820

810-
checkRequirementAccess(proto,
811-
proto->getFormalAccessScope(),
812-
proto->getDeclContext(),
813-
[&](AccessScope typeAccessScope,
814-
const TypeRepr *thisComplainRepr,
815-
DowngradeToWarning downgradeDiag) {
816-
if (typeAccessScope.isChildOf(minAccessScope) ||
817-
(!complainRepr &&
818-
typeAccessScope.hasEqualDeclContextWith(minAccessScope))) {
819-
minAccessScope = typeAccessScope;
820-
complainRepr = thisComplainRepr;
821-
protocolControlErrorKind = PCEK_Requirement;
822-
downgradeToWarning = downgradeDiag;
823-
824-
// Swift versions before 5.0 did not check requirements on the
825-
// protocol's where clause, so emit a warning.
826-
if (!TC.Context.isSwiftVersionAtLeast(5))
827-
downgradeToWarning = DowngradeToWarning::Yes;
828-
}
821+
forAllRequirementTypes(proto, [&](Type type, TypeRepr *typeRepr) {
822+
checkTypeAccess(
823+
type, typeRepr, proto,
824+
/*mayBeInferred*/ false,
825+
[&](AccessScope typeAccessScope, const TypeRepr *thisComplainRepr,
826+
DowngradeToWarning downgradeDiag) {
827+
if (typeAccessScope.isChildOf(minAccessScope) ||
828+
(!complainRepr &&
829+
typeAccessScope.hasEqualDeclContextWith(minAccessScope))) {
830+
minAccessScope = typeAccessScope;
831+
complainRepr = thisComplainRepr;
832+
protocolControlErrorKind = PCEK_Requirement;
833+
downgradeToWarning = downgradeDiag;
834+
declKind = declKindForType(type);
835+
// Swift versions before 5.0 did not check requirements on the
836+
// protocol's where clause, so emit a warning.
837+
if (!TC.Context.isSwiftVersionAtLeast(5))
838+
downgradeToWarning = DowngradeToWarning::Yes;
839+
}
840+
});
829841
});
830842

831843
if (!minAccessScope.isPublic()) {
@@ -837,10 +849,9 @@ class AccessControlChecker : public AccessControlCheckerBase,
837849
auto diagID = diag::protocol_access;
838850
if (downgradeToWarning == DowngradeToWarning::Yes)
839851
diagID = diag::protocol_access_warn;
840-
auto diag = TC.diagnose(proto, diagID,
841-
isExplicit, protoAccess,
852+
auto diag = TC.diagnose(proto, diagID, isExplicit, protoAccess,
842853
protocolControlErrorKind, minAccess,
843-
isa<FileUnit>(proto->getDeclContext()));
854+
isa<FileUnit>(proto->getDeclContext()), declKind);
844855
highlightOffendingType(TC, diag, complainRepr);
845856
}
846857
}

test/Compatibility/accessibility_where.swift

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -9,14 +9,14 @@ public protocol BaseProtocol {
99
}
1010

1111
public protocol PublicProtocol1 : BaseProtocol where T == PrivateStruct {
12-
// expected-warning@-1 {{public protocol's 'where' clause should not use a private protocol}}
12+
// expected-warning@-1 {{public protocol's 'where' clause should not use a private struct}}
1313

1414
associatedtype X : BaseProtocol where X.T == PrivateStruct
1515
// expected-warning@-1 {{associated type in a public protocol uses a private type in its requirement}}
1616
}
1717

1818
public protocol PublicProtocol2 : BaseProtocol where T == InternalStruct {
19-
// expected-warning@-1 {{public protocol's 'where' clause should not use an internal protocol}}
19+
// expected-warning@-1 {{public protocol's 'where' clause should not use an internal struct}}
2020

2121
associatedtype X : BaseProtocol where X.T == InternalStruct
2222
// expected-warning@-1 {{associated type in a public protocol uses an internal type in its requirement}}
@@ -27,7 +27,7 @@ public protocol PublicProtocol3 : BaseProtocol where T == PublicStruct {
2727
}
2828

2929
internal protocol InternalProtocol1 : BaseProtocol where T == PrivateStruct {
30-
// expected-warning@-1 {{internal protocol's 'where' clause should not use a private protocol}}
30+
// expected-warning@-1 {{internal protocol's 'where' clause should not use a private struct}}
3131

3232
associatedtype X : BaseProtocol where X.T == PrivateStruct
3333
// expected-warning@-1 {{associated type in an internal protocol uses a private type in its requirement}}
@@ -42,7 +42,7 @@ internal protocol InternalProtocol3 : BaseProtocol where T == PublicStruct {
4242
}
4343

4444
protocol Protocol1 : BaseProtocol where T == PrivateStruct {
45-
// expected-warning@-1 {{protocol should be declared fileprivate because its 'where' clause uses a private protocol}}
45+
// expected-warning@-1 {{protocol should be declared fileprivate because its 'where' clause uses a private struct}}
4646

4747
associatedtype X : BaseProtocol where X.T == PrivateStruct
4848
// expected-warning@-1 {{associated type in an internal protocol uses a private type in its requirement}}

test/Sema/accessibility.swift

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -435,6 +435,15 @@ public protocol PublicRefinesInternal : InternalProto {} // expected-error {{pub
435435
public protocol PublicRefinesPI : PrivateProto, InternalProto {} // expected-error {{public protocol cannot refine a private protocol}}
436436
public protocol PublicRefinesIP : InternalProto, PrivateProto {} // expected-error {{public protocol cannot refine a private protocol}}
437437

438+
private typealias PrivateTypeAlias = PublicProto; // expected-note {{type declared here}}
439+
private typealias PrivateCompoundTypeAlias = PublicProto & AnyObject // expected-note {{type declared here}}
440+
441+
protocol DefaultRefinesPrivateClass: PrivateClass {} // expected-error {{protocol must be declared private or fileprivate because it refines a private class}}
442+
public protocol PublicRefinesPrivateClass: PrivateClass {} // expected-error {{public protocol cannot refine a private class}}
443+
public protocol PublicRefinesPrivateTypeAlias: PrivateTypeAlias {} // expected-error {{public protocol cannot refine a private type alias}}
444+
public protocol PublicRefinesPrivateCompoundTypeAlias: PrivateCompoundTypeAlias {} // expected-error {{public protocol cannot refine a private type alias}}
445+
446+
438447

439448
// expected-note@+1 * {{type declared here}}
440449
private typealias PrivateInt = Int

test/Sema/accessibility_where.swift

Lines changed: 37 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,22 +1,26 @@
11
// RUN: %target-typecheck-verify-swift -swift-version 5
22

3-
private struct PrivateStruct {} // expected-note 6{{type declared here}}
3+
private struct PrivateStruct {} // expected-note 8{{type declared here}}
44
internal struct InternalStruct {} // expected-note 2{{type declared here}}
55
public struct PublicStruct {}
6+
private class PrivateClass {} // expected-note 4{{type declared here}}
67

78
public protocol BaseProtocol {
89
associatedtype T
910
}
1011

12+
public protocol BaseProtocol2 {}
13+
private typealias PrivateTypeAlias = BaseProtocol2 // expected-note 2{{type declared here}}
14+
1115
public protocol PublicProtocol1 : BaseProtocol where T == PrivateStruct {
12-
// expected-error@-1 {{public protocol's 'where' clause cannot use a private protocol}}
16+
// expected-error@-1 {{public protocol's 'where' clause cannot use a private struct}}
1317

1418
associatedtype X : BaseProtocol where X.T == PrivateStruct
1519
// expected-error@-1 {{associated type in a public protocol uses a private type in its requirement}}
1620
}
1721

1822
public protocol PublicProtocol2 : BaseProtocol where T == InternalStruct {
19-
// expected-error@-1 {{public protocol's 'where' clause cannot use an internal protocol}}
23+
// expected-error@-1 {{public protocol's 'where' clause cannot use an internal struct}}
2024

2125
associatedtype X : BaseProtocol where X.T == InternalStruct
2226
// expected-error@-1 {{associated type in a public protocol uses an internal type in its requirement}}
@@ -27,7 +31,7 @@ public protocol PublicProtocol3 : BaseProtocol where T == PublicStruct {
2731
}
2832

2933
internal protocol InternalProtocol1 : BaseProtocol where T == PrivateStruct {
30-
// expected-error@-1 {{internal protocol's 'where' clause cannot use a private protocol}}
34+
// expected-error@-1 {{internal protocol's 'where' clause cannot use a private struct}}
3135

3236
associatedtype X : BaseProtocol where X.T == PrivateStruct
3337
// expected-error@-1 {{associated type in an internal protocol uses a private type in its requirement}}
@@ -42,7 +46,7 @@ internal protocol InternalProtocol3 : BaseProtocol where T == PublicStruct {
4246
}
4347

4448
protocol Protocol1 : BaseProtocol where T == PrivateStruct {
45-
// expected-error@-1 {{protocol must be declared private or fileprivate because its 'where' clause uses a private protocol}}
49+
// expected-error@-1 {{protocol must be declared private or fileprivate because its 'where' clause uses a private struct}}
4650

4751
associatedtype X : BaseProtocol where X.T == PrivateStruct
4852
// expected-error@-1 {{associated type in an internal protocol uses a private type in its requirement}}
@@ -55,3 +59,31 @@ protocol Protocol2 : BaseProtocol where T == InternalStruct {
5559
protocol Protocol3 : BaseProtocol where T == PublicStruct {
5660
associatedtype X : BaseProtocol where X.T == PublicStruct
5761
}
62+
63+
protocol Protocol4 : BaseProtocol where T == PrivateClass {
64+
// expected-error@-1 {{protocol must be declared private or fileprivate because its 'where' clause uses a private class}}
65+
66+
associatedtype X : BaseProtocol where X.T == PrivateClass
67+
// expected-error@-1 {{associated type in an internal protocol uses a private type in its requirement}}
68+
}
69+
70+
protocol Protocol5 : BaseProtocol where T == PrivateTypeAlias {
71+
// expected-error@-1 {{protocol must be declared private or fileprivate because its 'where' clause uses a private type alias}}
72+
73+
associatedtype X : BaseProtocol where X.T == PrivateTypeAlias
74+
// expected-error@-1 {{associated type in an internal protocol uses a private type in its requirement}}
75+
}
76+
77+
protocol Protocol6 : BaseProtocol where T == (PrivateClass, AnyObject) {
78+
// expected-error@-1 {{protocol must be declared private or fileprivate because its 'where' clause uses a private type}}
79+
80+
associatedtype X : BaseProtocol where X.T == (PrivateClass, AnyObject)
81+
// expected-error@-1 {{associated type in an internal protocol uses a private type in its requirement}}
82+
}
83+
84+
protocol Protocol7 : BaseProtocol where T == (PrivateStruct) -> Void {
85+
// expected-error@-1 {{protocol must be declared private or fileprivate because its 'where' clause uses a private type}}
86+
87+
associatedtype X : BaseProtocol where X.T == (PrivateStruct) -> Void
88+
// expected-error@-1 {{associated type in an internal protocol uses a private type in its requirement}}
89+
}

0 commit comments

Comments
 (0)