Skip to content

Commit 0d92918

Browse files
slavapestovslavapestov
committed
Sema: Check access control in 'where' clauses of protocols and associated types
We weren't checking this before, which would let you define a public protocol that no public type could conform to. This is a source-breaking change, so stage it in with a warning. It becomes an error in -swift-version 5 mode.
1 parent d067a31 commit 0d92918

File tree

4 files changed

+188
-15
lines changed

4 files changed

+188
-15
lines changed

‎include/swift/AST/DiagnosticsSema.def

Lines changed: 12 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1565,20 +1565,22 @@ NOTE(witness_fix_access,none,
15651565
"mark the %0 as '%select{%error|fileprivate|internal|public|%error}1' to "
15661566
"satisfy the requirement", (DescriptiveDeclKind, AccessLevel))
15671567

1568-
ERROR(protocol_refine_access,none,
1568+
ERROR(protocol_access,none,
15691569
"%select{protocol must be declared %select{"
15701570
"%select{private|fileprivate|internal|%error|%error}1"
1571-
"|private or fileprivate}3 because it refines"
1571+
"|private or fileprivate}4 because %select{it refines|its 'where' clause uses}2"
15721572
"|%select{in this context|fileprivate|internal|public|%error}1 "
1573-
"protocol cannot refine}0 "
1574-
"%select{a private|a fileprivate|an internal|%error|%error}2 protocol",
1575-
(bool, AccessLevel, AccessLevel, bool))
1576-
WARNING(protocol_refine_access_warn,none,
1573+
"%select{protocol cannot refine|protocol's 'where' clause cannot use}2}0 "
1574+
"%select{a private|a fileprivate|an internal|%error|%error}3 protocol",
1575+
(bool, AccessLevel, bool, AccessLevel, bool))
1576+
WARNING(protocol_access_warn,none,
15771577
"%select{protocol should be declared "
1578-
"%select{private|fileprivate|internal|%error|%error}2 because it refines"
1579-
"|%select{in this context|fileprivate|internal|public|%error}1 protocol should not "
1580-
"refine}0 %select{a private|a fileprivate|an internal|%error|%error}2 protocol",
1581-
(bool, AccessLevel, AccessLevel, bool))
1578+
"%select{private|fileprivate|internal|%error|%error}1 because "
1579+
"%select{it refines|its 'where' clause uses}2"
1580+
"|%select{in this context|fileprivate|internal|public|%error}1 "
1581+
"%select{protocol should not refine|protocol's 'where' clause should not use}2}0 "
1582+
"%select{a private|a fileprivate|an internal|%error|%error}3 protocol",
1583+
(bool, AccessLevel, bool, AccessLevel, bool))
15821584
ERROR(protocol_property_must_be_computed_var,none,
15831585
"immutable property requirement must be declared as 'var' with a "
15841586
"'{ get }' specifier", ())

‎lib/Sema/TypeCheckDecl.cpp

Lines changed: 62 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1828,8 +1828,8 @@ static void highlightOffendingType(TypeChecker &TC, InFlightDiagnostic &diag,
18281828

18291829
static void checkRequirementAccess(TypeChecker &TC,
18301830
ArrayRef<RequirementRepr> requirements,
1831-
const DeclContext *useDC,
18321831
AccessScope accessScope,
1832+
const DeclContext *useDC,
18331833
llvm::function_ref<CheckTypeAccessCallback> diagnose) {
18341834
for (auto &requirement : requirements) {
18351835
switch (requirement.getKind()) {
@@ -1897,7 +1897,7 @@ static void checkGenericParamAccess(TypeChecker &TC,
18971897
callbackACEK = ACEK::Requirement;
18981898

18991899
checkRequirementAccess(TC, params->getRequirements(),
1900-
owner->getDeclContext(), accessScope,
1900+
accessScope, owner->getDeclContext(),
19011901
callback);
19021902

19031903
if (minAccessScope.isPublic())
@@ -2113,6 +2113,29 @@ static void checkAccessControl(TypeChecker &TC, const Decl *D) {
21132113
}
21142114
});
21152115

2116+
if (auto *trailingWhereClause = assocType->getTrailingWhereClause()) {
2117+
checkRequirementAccess(TC, trailingWhereClause->getRequirements(),
2118+
assocType->getFormalAccessScope(),
2119+
assocType->getDeclContext(),
2120+
[&](AccessScope typeAccessScope,
2121+
const TypeRepr *thisComplainRepr,
2122+
DowngradeToWarning downgradeDiag) {
2123+
if (typeAccessScope.isChildOf(minAccessScope) ||
2124+
(!complainRepr &&
2125+
typeAccessScope.hasEqualDeclContextWith(minAccessScope))) {
2126+
minAccessScope = typeAccessScope;
2127+
complainRepr = thisComplainRepr;
2128+
accessControlErrorKind = ACEK_Requirement;
2129+
downgradeToWarning = downgradeDiag;
2130+
2131+
// Swift versions before 5.0 did not check requirements on the
2132+
// protocol's where clause, so emit a warning.
2133+
if (!TC.Context.isSwiftVersionAtLeast(5))
2134+
downgradeToWarning = DowngradeToWarning::Yes;
2135+
}
2136+
});
2137+
}
2138+
21162139
if (!minAccessScope.isPublic()) {
21172140
auto minAccess = minAccessScope.accessLevelForDiagnostics();
21182141
auto diagID = diag::associated_type_access;
@@ -2232,6 +2255,12 @@ static void checkAccessControl(TypeChecker &TC, const Decl *D) {
22322255
case DeclKind::Protocol: {
22332256
auto proto = cast<ProtocolDecl>(D);
22342257

2258+
// This must stay in sync with diag::protocol_access.
2259+
enum {
2260+
PCEK_Refine = 0,
2261+
PCEK_Requirement
2262+
} protocolControlErrorKind;
2263+
22352264
auto minAccessScope = AccessScope::getPublic();
22362265
const TypeRepr *complainRepr = nullptr;
22372266
auto downgradeToWarning = DowngradeToWarning::No;
@@ -2248,19 +2277,47 @@ static void checkAccessControl(TypeChecker &TC, const Decl *D) {
22482277
typeAccessScope.hasEqualDeclContextWith(minAccessScope))) {
22492278
minAccessScope = typeAccessScope;
22502279
complainRepr = thisComplainRepr;
2280+
protocolControlErrorKind = PCEK_Refine;
22512281
downgradeToWarning = downgradeDiag;
22522282
}
22532283
});
22542284
});
22552285

2286+
if (auto *trailingWhereClause = proto->getTrailingWhereClause()) {
2287+
checkRequirementAccess(TC, trailingWhereClause->getRequirements(),
2288+
proto->getFormalAccessScope(),
2289+
proto->getDeclContext(),
2290+
[&](AccessScope typeAccessScope,
2291+
const TypeRepr *thisComplainRepr,
2292+
DowngradeToWarning downgradeDiag) {
2293+
if (typeAccessScope.isChildOf(minAccessScope) ||
2294+
(!complainRepr &&
2295+
typeAccessScope.hasEqualDeclContextWith(minAccessScope))) {
2296+
minAccessScope = typeAccessScope;
2297+
complainRepr = thisComplainRepr;
2298+
protocolControlErrorKind = PCEK_Requirement;
2299+
downgradeToWarning = downgradeDiag;
2300+
2301+
// Swift versions before 5.0 did not check requirements on the
2302+
// protocol's where clause, so emit a warning.
2303+
if (!TC.Context.isSwiftVersionAtLeast(5))
2304+
downgradeToWarning = DowngradeToWarning::Yes;
2305+
}
2306+
});
2307+
}
2308+
22562309
if (!minAccessScope.isPublic()) {
22572310
auto minAccess = minAccessScope.accessLevelForDiagnostics();
22582311
bool isExplicit = proto->getAttrs().hasAttribute<AccessControlAttr>();
2259-
auto diagID = diag::protocol_refine_access;
2312+
auto protoAccess = isExplicit
2313+
? proto->getFormalAccess()
2314+
: minAccessScope.requiredAccessForDiagnostics();
2315+
auto diagID = diag::protocol_access;
22602316
if (downgradeToWarning == DowngradeToWarning::Yes)
2261-
diagID = diag::protocol_refine_access_warn;
2317+
diagID = diag::protocol_access_warn;
22622318
auto diag = TC.diagnose(proto, diagID,
2263-
isExplicit, proto->getFormalAccess(), minAccess,
2319+
isExplicit, protoAccess,
2320+
protocolControlErrorKind, minAccess,
22642321
isa<FileUnit>(proto->getDeclContext()));
22652322
highlightOffendingType(TC, diag, complainRepr);
22662323
}

‎test/Compatibility/accessibility_where.swift

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,57 @@
1+
// RUN: %target-typecheck-verify-swift -swift-version 4
2+
3+
private struct PrivateStruct {} // expected-note 6{{type declared here}}
4+
internal struct InternalStruct {} // expected-note 2{{type declared here}}
5+
public struct PublicStruct {}
6+
7+
public protocol BaseProtocol {
8+
associatedtype T
9+
}
10+
11+
public protocol PublicProtocol1 : BaseProtocol where T == PrivateStruct {
12+
// expected-warning@-1 {{public protocol's 'where' clause should not use a private protocol}}
13+
14+
associatedtype X : BaseProtocol where X.T == PrivateStruct
15+
// expected-warning@-1 {{associated type in a public protocol uses a private type in its requirement}}
16+
}
17+
18+
public protocol PublicProtocol2 : BaseProtocol where T == InternalStruct {
19+
// expected-warning@-1 {{public protocol's 'where' clause should not use an internal protocol}}
20+
21+
associatedtype X : BaseProtocol where X.T == InternalStruct
22+
// expected-warning@-1 {{associated type in a public protocol uses an internal type in its requirement}}
23+
}
24+
25+
public protocol PublicProtocol3 : BaseProtocol where T == PublicStruct {
26+
associatedtype X : BaseProtocol where X.T == PublicStruct
27+
}
28+
29+
internal protocol InternalProtocol1 : BaseProtocol where T == PrivateStruct {
30+
// expected-warning@-1 {{internal protocol's 'where' clause should not use a private protocol}}
31+
32+
associatedtype X : BaseProtocol where X.T == PrivateStruct
33+
// expected-warning@-1 {{associated type in an internal protocol uses a private type in its requirement}}
34+
}
35+
36+
internal protocol InternalProtocol2 : BaseProtocol where T == InternalStruct {
37+
associatedtype X : BaseProtocol where X.T == InternalStruct
38+
}
39+
40+
internal protocol InternalProtocol3 : BaseProtocol where T == PublicStruct {
41+
associatedtype X : BaseProtocol where X.T == PublicStruct
42+
}
43+
44+
protocol Protocol1 : BaseProtocol where T == PrivateStruct {
45+
// expected-warning@-1 {{protocol should be declared fileprivate because its 'where' clause uses a private protocol}}
46+
47+
associatedtype X : BaseProtocol where X.T == PrivateStruct
48+
// expected-warning@-1 {{associated type in an internal protocol uses a private type in its requirement}}
49+
}
50+
51+
protocol Protocol2 : BaseProtocol where T == InternalStruct {
52+
associatedtype X : BaseProtocol where X.T == InternalStruct
53+
}
54+
55+
protocol Protocol3 : BaseProtocol where T == PublicStruct {
56+
associatedtype X : BaseProtocol where X.T == PublicStruct
57+
}

‎test/Sema/accessibility_where.swift

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,57 @@
1+
// RUN: %target-typecheck-verify-swift -swift-version 5
2+
3+
private struct PrivateStruct {} // expected-note 6{{type declared here}}
4+
internal struct InternalStruct {} // expected-note 2{{type declared here}}
5+
public struct PublicStruct {}
6+
7+
public protocol BaseProtocol {
8+
associatedtype T
9+
}
10+
11+
public protocol PublicProtocol1 : BaseProtocol where T == PrivateStruct {
12+
// expected-error@-1 {{public protocol's 'where' clause cannot use a private protocol}}
13+
14+
associatedtype X : BaseProtocol where X.T == PrivateStruct
15+
// expected-error@-1 {{associated type in a public protocol uses a private type in its requirement}}
16+
}
17+
18+
public protocol PublicProtocol2 : BaseProtocol where T == InternalStruct {
19+
// expected-error@-1 {{public protocol's 'where' clause cannot use an internal protocol}}
20+
21+
associatedtype X : BaseProtocol where X.T == InternalStruct
22+
// expected-error@-1 {{associated type in a public protocol uses an internal type in its requirement}}
23+
}
24+
25+
public protocol PublicProtocol3 : BaseProtocol where T == PublicStruct {
26+
associatedtype X : BaseProtocol where X.T == PublicStruct
27+
}
28+
29+
internal protocol InternalProtocol1 : BaseProtocol where T == PrivateStruct {
30+
// expected-error@-1 {{internal protocol's 'where' clause cannot use a private protocol}}
31+
32+
associatedtype X : BaseProtocol where X.T == PrivateStruct
33+
// expected-error@-1 {{associated type in an internal protocol uses a private type in its requirement}}
34+
}
35+
36+
internal protocol InternalProtocol2 : BaseProtocol where T == InternalStruct {
37+
associatedtype X : BaseProtocol where X.T == InternalStruct
38+
}
39+
40+
internal protocol InternalProtocol3 : BaseProtocol where T == PublicStruct {
41+
associatedtype X : BaseProtocol where X.T == PublicStruct
42+
}
43+
44+
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}}
46+
47+
associatedtype X : BaseProtocol where X.T == PrivateStruct
48+
// expected-error@-1 {{associated type in an internal protocol uses a private type in its requirement}}
49+
}
50+
51+
protocol Protocol2 : BaseProtocol where T == InternalStruct {
52+
associatedtype X : BaseProtocol where X.T == InternalStruct
53+
}
54+
55+
protocol Protocol3 : BaseProtocol where T == PublicStruct {
56+
associatedtype X : BaseProtocol where X.T == PublicStruct
57+
}

0 commit comments

Comments
 (0)