Skip to content

Commit 5dace22

Browse files
jrose-applejrose-apple
authored
Merge pull request #18623 from dingobye/sr8453
[Sema] Warn for redundant access-level modifiers on setters or used in an extension.
2 parents 52764c8 + d5d17fa commit 5dace22

26 files changed

+329
-117
lines changed

‎include/swift/AST/DiagnosticsSema.def

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1180,9 +1180,19 @@ ERROR(access_control_setter_more,none,
11801180
"%select{variable|property|subscript}1 cannot have "
11811181
"%select{%error|a fileprivate|an internal|a public|an open}2 setter",
11821182
(AccessLevel, unsigned, AccessLevel))
1183+
WARNING(access_control_setter_redundant,none,
1184+
"'%select{private|fileprivate|internal|public|open}0(set)' modifier is "
1185+
"redundant for %select{a private|a fileprivate|an internal|a public|an open}2 "
1186+
"%1",
1187+
(AccessLevel, DescriptiveDeclKind, AccessLevel))
11831188
WARNING(access_control_ext_member_more,none,
11841189
"declaring %select{%error|a fileprivate|an internal|a public|open}0 %1 in "
1185-
"%select{a private|a fileprivate|an internal|public|%error}2 extension",
1190+
"%select{a private|a fileprivate|an internal|a public|%error}2 extension",
1191+
(AccessLevel, DescriptiveDeclKind, AccessLevel))
1192+
WARNING(access_control_ext_member_redundant,none,
1193+
"'%select{%error|fileprivate|internal|public|%error}0' modifier is redundant "
1194+
"for %1 declared in %select{a private (equivalent to fileprivate)|a fileprivate"
1195+
"|an internal|a public|%error}2 extension",
11861196
(AccessLevel, DescriptiveDeclKind, AccessLevel))
11871197
ERROR(access_control_ext_requirement_member_more,none,
11881198
"cannot declare %select{%error|a fileprivate|an internal|a public|an open}0 %1 "

‎lib/Sema/MiscDiagnostics.cpp

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3917,7 +3917,8 @@ void swift::performStmtDiagnostics(TypeChecker &TC, const Stmt *S) {
39173917
//===----------------------------------------------------------------------===//
39183918

39193919
void swift::fixItAccess(InFlightDiagnostic &diag, ValueDecl *VD,
3920-
AccessLevel desiredAccess, bool isForSetter) {
3920+
AccessLevel desiredAccess, bool isForSetter,
3921+
bool shouldUseDefaultAccess) {
39213922
StringRef fixItString;
39223923
switch (desiredAccess) {
39233924
case AccessLevel::Private: fixItString = "private "; break;
@@ -3963,9 +3964,14 @@ void swift::fixItAccess(InFlightDiagnostic &diag, ValueDecl *VD,
39633964
// this function is sometimes called to make access narrower, so assuming
39643965
// that a broader scope is acceptable breaks some diagnostics.
39653966
if (attr->getAccess() != desiredAccess) {
3966-
// This uses getLocation() instead of getRange() because we don't want to
3967-
// replace the "(set)" part of a setter attribute.
3968-
diag.fixItReplace(attr->getLocation(), fixItString.drop_back());
3967+
if (shouldUseDefaultAccess) {
3968+
// Remove the attribute if replacement is not preferred.
3969+
diag.fixItRemove(attr->getRange());
3970+
} else {
3971+
// This uses getLocation() instead of getRange() because we don't want to
3972+
// replace the "(set)" part of a setter attribute.
3973+
diag.fixItReplace(attr->getLocation(), fixItString.drop_back());
3974+
}
39693975
attr->setInvalid();
39703976
}
39713977

‎lib/Sema/MiscDiagnostics.h

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -51,8 +51,11 @@ void performTopLevelDeclDiagnostics(TypeChecker &TC, TopLevelCodeDecl *TLCD);
5151
/// Emit a fix-it to set the access of \p VD to \p desiredAccess.
5252
///
5353
/// This actually updates \p VD as well.
54-
void fixItAccess(InFlightDiagnostic &diag, ValueDecl *VD,
55-
AccessLevel desiredAccess, bool isForSetter = false);
54+
void fixItAccess(InFlightDiagnostic &diag,
55+
ValueDecl *VD,
56+
AccessLevel desiredAccess,
57+
bool isForSetter = false,
58+
bool shouldUseDefaultAccess = false);
5659

5760
/// Emit fix-its to correct the argument labels in \p expr, which is the
5861
/// argument tuple or single argument of a call.

‎lib/Sema/TypeCheckAttr.cpp

Lines changed: 21 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1509,17 +1509,23 @@ void AttributeChecker::visitAccessControlAttr(AccessControlAttr *attr) {
15091509

15101510
if (auto extAttr =
15111511
extension->getAttrs().getAttribute<AccessControlAttr>()) {
1512-
// Extensions are top level declarations, for which the literally lowest
1513-
// access level `private` is equivalent to `fileprivate`.
1514-
AccessLevel extAccess = std::max(extAttr->getAccess(),
1515-
AccessLevel::FilePrivate);
1516-
if (attr->getAccess() > extAccess) {
1512+
AccessLevel defaultAccess = extension->getDefaultAccessLevel();
1513+
if (attr->getAccess() > defaultAccess) {
15171514
auto diag = TC.diagnose(attr->getLocation(),
15181515
diag::access_control_ext_member_more,
15191516
attr->getAccess(),
15201517
D->getDescriptiveKind(),
15211518
extAttr->getAccess());
1522-
swift::fixItAccess(diag, cast<ValueDecl>(D), extAccess);
1519+
swift::fixItAccess(diag, cast<ValueDecl>(D), defaultAccess, false,
1520+
true);
1521+
return;
1522+
} else if (attr->getAccess() == defaultAccess) {
1523+
TC.diagnose(attr->getLocation(),
1524+
diag::access_control_ext_member_redundant,
1525+
attr->getAccess(),
1526+
D->getDescriptiveKind(),
1527+
extAttr->getAccess())
1528+
.fixItRemove(attr->getRange());
15231529
return;
15241530
}
15251531
}
@@ -1555,6 +1561,15 @@ AttributeChecker::visitSetterAccessAttr(SetterAccessAttr *attr) {
15551561
getterAccess, storageKind, attr->getAccess());
15561562
attr->setInvalid();
15571563
return;
1564+
1565+
} else if (attr->getAccess() == getterAccess) {
1566+
TC.diagnose(attr->getLocation(),
1567+
diag::access_control_setter_redundant,
1568+
attr->getAccess(),
1569+
D->getDescriptiveKind(),
1570+
getterAccess)
1571+
.fixItRemove(attr->getRange());
1572+
return;
15581573
}
15591574
}
15601575

‎stdlib/public/SDK/AppKit/AppKit_FoundationExtensions.swift

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ import Foundation
1414
@_exported import AppKit
1515

1616
// NSCollectionView extensions
17-
public extension IndexPath {
17+
extension IndexPath {
1818

1919
/// Initialize for use with `NSCollectionView`.
2020
public init(item: Int, section: Int) {
@@ -51,7 +51,7 @@ public extension IndexPath {
5151

5252
}
5353

54-
public extension URLResourceValues {
54+
extension URLResourceValues {
5555
/// Returns all thumbnails as a single NSImage.
5656
@available(macOS 10.10, *)
5757
public var thumbnail : NSImage? {

‎stdlib/public/SDK/CloudKit/NestedNames.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@
1616

1717
@nonobjc
1818
@available(macOS 10.10, iOS 8.0, watchOS 3.0, *)
19-
public extension CKContainer {
19+
extension CKContainer {
2020
@available(swift 4.2)
2121
public enum Application {
2222
public typealias Permissions = CKContainer_Application_Permissions

‎stdlib/public/SDK/CoreGraphics/CoreGraphics.swift

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -429,10 +429,10 @@ public extension CGRect {
429429
}
430430

431431
@available(*, unavailable, renamed: "minX")
432-
public var x: CGFloat { return minX }
432+
var x: CGFloat { return minX }
433433

434434
@available(*, unavailable, renamed: "minY")
435-
public var y: CGFloat { return minY }
435+
var y: CGFloat { return minY }
436436
}
437437

438438
extension CGRect : CustomReflectable {

‎stdlib/public/SDK/Dispatch/Dispatch.swift

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -129,7 +129,7 @@ public enum DispatchTimeoutResult {
129129

130130
/// dispatch_group
131131

132-
public extension DispatchGroup {
132+
extension DispatchGroup {
133133
public func notify(qos: DispatchQoS = .unspecified, flags: DispatchWorkItemFlags = [], queue: DispatchQueue, execute work: @escaping @convention(block) () -> Void) {
134134
if #available(macOS 10.10, iOS 8.0, *), qos != .unspecified || !flags.isEmpty {
135135
let item = DispatchWorkItem(qos: qos, flags: flags, block: work)
@@ -159,7 +159,7 @@ public extension DispatchGroup {
159159

160160
/// dispatch_semaphore
161161

162-
public extension DispatchSemaphore {
162+
extension DispatchSemaphore {
163163
@discardableResult
164164
public func signal() -> Int {
165165
return __dispatch_semaphore_signal(self)

‎stdlib/public/SDK/Dispatch/IO.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@
1010
//
1111
//===----------------------------------------------------------------------===//
1212

13-
public extension DispatchIO {
13+
extension DispatchIO {
1414

1515
public enum StreamType : UInt {
1616
case stream = 0

‎stdlib/public/SDK/Dispatch/Queue.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ internal class _DispatchSpecificValue<T> {
2323
internal init(value: T) { self.value = value }
2424
}
2525

26-
public extension DispatchQueue {
26+
extension DispatchQueue {
2727
public struct Attributes : OptionSet {
2828
public let rawValue: UInt64
2929
public init(rawValue: UInt64) { self.rawValue = rawValue }

0 commit comments

Comments
 (0)