Skip to content

Commit dd87cc2

Browse files
allevatoallevato
authored
Merge pull request swiftlang#249 from dylansturg/synth_initializer_bug
Apply private and fileprivate rules to UseSynthesizedInitializer.
2 parents 22118db + e0e96b6 commit dd87cc2

File tree

2 files changed

+212
-19
lines changed

2 files changed

+212
-19
lines changed

‎Sources/SwiftFormatRules/UseSynthesizedInitializer.swift

Lines changed: 58 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -39,9 +39,6 @@ public final class UseSynthesizedInitializer: SyntaxLintRule {
3939
storedProperties.append(varDecl)
4040
// Collect any possible redundant initializers into a list
4141
} else if let initDecl = member.as(InitializerDeclSyntax.self) {
42-
guard initDecl.modifiers == nil || initDecl.modifiers!.has(modifier: "internal") else {
43-
continue
44-
}
4542
guard initDecl.optionalMark == nil else { continue }
4643
guard initDecl.throwsOrRethrowsKeyword == nil else { continue }
4744
initializers.append(initDecl)
@@ -62,6 +59,8 @@ public final class UseSynthesizedInitializer: SyntaxLintRule {
6259
variables: storedProperties,
6360
initBody: initializer.body)
6461
else { continue }
62+
guard matchesAccessLevel(modifiers: initializer.modifiers, properties: storedProperties)
63+
else { continue }
6564
extraneousInitializers.append(initializer)
6665
}
6766

@@ -76,6 +75,29 @@ public final class UseSynthesizedInitializer: SyntaxLintRule {
7675
return .skipChildren
7776
}
7877

78+
/// Compares the actual access level of an initializer with the access level of a synthesized
79+
/// memberwise initializer.
80+
///
81+
/// - Parameters:
82+
/// - modifiers: The modifier list from the initializer.
83+
/// - properties: The properties from the enclosing type.
84+
/// - Returns: Whether the initializer has the same access level as the synthesized initializer.
85+
private func matchesAccessLevel(
86+
modifiers: ModifierListSyntax?, properties: [VariableDeclSyntax]
87+
) -> Bool {
88+
let synthesizedAccessLevel = synthesizedInitAccessLevel(using: properties)
89+
let accessLevel = modifiers?.accessLevelModifier
90+
switch synthesizedAccessLevel {
91+
case .internal:
92+
// No explicit access level or internal are equivalent.
93+
return accessLevel == nil || accessLevel!.name.tokenKind == .internalKeyword
94+
case .fileprivate:
95+
return accessLevel != nil && accessLevel!.name.tokenKind == .fileprivateKeyword
96+
case .private:
97+
return accessLevel != nil && accessLevel!.name.tokenKind == .privateKeyword
98+
}
99+
}
100+
79101
// Compares initializer parameters to stored properties of the struct
80102
private func matchesPropertyList(
81103
parameters: FunctionParameterListSyntax,
@@ -160,3 +182,36 @@ extension Diagnostic.Message {
160182
.warning,
161183
"remove initializer and use the synthesized initializer")
162184
}
185+
186+
/// Defines the access levels which may be assigned to a synthesized memberwise initializer.
187+
fileprivate enum AccessLevel {
188+
case `internal`
189+
case `fileprivate`
190+
case `private`
191+
}
192+
193+
/// Computes the access level which would be applied to the synthesized memberwise initializer of
194+
/// a struct that contains the given properties.
195+
///
196+
/// The rules for default memberwise initializer access levels are defined in The Swift
197+
/// Programming Languge:
198+
/// https://docs.swift.org/swift-book/LanguageGuide/AccessControl.html#ID21
199+
///
200+
/// - Parameter properties: The properties contained within the struct.
201+
/// - Returns: The synthesized memberwise initializer's access level.
202+
fileprivate func synthesizedInitAccessLevel(using properties: [VariableDeclSyntax]) -> AccessLevel {
203+
var hasFileprivate = false
204+
for property in properties {
205+
guard let modifiers = property.modifiers else { continue }
206+
207+
// Private takes precedence, so finding 1 private property defines the access level.
208+
if modifiers.contains(where: {$0.name.tokenKind == .privateKeyword && $0.detail == nil}) {
209+
return .private
210+
}
211+
if modifiers.contains(where: {$0.name.tokenKind == .fileprivateKeyword && $0.detail == nil}) {
212+
hasFileprivate = true
213+
// Can't break here because a later property might be private.
214+
}
215+
}
216+
return hasFileprivate ? .fileprivate : .internal
217+
}

‎Tests/SwiftFormatRulesTests/UseSynthesizedInitializerTests.swift

Lines changed: 154 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,18 @@
11
import SwiftFormatRules
22

33
final class UseSynthesizedInitializerTests: LintOrFormatRuleTestCase {
4+
override func setUp() {
5+
self.shouldCheckForUnassertedDiagnostics = true
6+
}
7+
48
func testMemberwiseInitializerIsDiagnosed() {
59
let input =
610
"""
711
public struct Person {
812
913
public var name: String
1014
let phoneNumber: String
11-
private let address: String
15+
internal let address: String
1216
1317
init(name: String, phoneNumber: String, address: String) {
1418
self.name = name
@@ -19,8 +23,28 @@ final class UseSynthesizedInitializerTests: LintOrFormatRuleTestCase {
1923
"""
2024

2125
performLint(UseSynthesizedInitializer.self, input: input)
22-
XCTAssertDiagnosed(.removeRedundantInitializer)
23-
XCTAssertNotDiagnosed(.removeRedundantInitializer)
26+
XCTAssertDiagnosed(.removeRedundantInitializer, line: 7)
27+
}
28+
29+
func testInternalMemberwiseInitializerIsDiagnosed() {
30+
let input =
31+
"""
32+
public struct Person {
33+
34+
public var name: String
35+
let phoneNumber: String
36+
internal let address: String
37+
38+
internal init(name: String, phoneNumber: String, address: String) {
39+
self.name = name
40+
self.address = address
41+
self.phoneNumber = phoneNumber
42+
}
43+
}
44+
"""
45+
46+
performLint(UseSynthesizedInitializer.self, input: input)
47+
XCTAssertDiagnosed(.removeRedundantInitializer, line: 7)
2448
}
2549

2650
func testMemberwiseInitializerWithDefaultArgumentIsDiagnosed() {
@@ -30,7 +54,7 @@ final class UseSynthesizedInitializerTests: LintOrFormatRuleTestCase {
3054
3155
public var name: String = "John Doe"
3256
let phoneNumber: String
33-
private let address: String
57+
internal let address: String
3458
3559
init(name: String = "John Doe", phoneNumber: String, address: String) {
3660
self.name = name
@@ -40,9 +64,8 @@ final class UseSynthesizedInitializerTests: LintOrFormatRuleTestCase {
4064
}
4165
"""
4266

43-
performLint(UseSynthesizedInitializer.self, input: input)
44-
XCTAssertDiagnosed(.removeRedundantInitializer)
45-
XCTAssertNotDiagnosed(.removeRedundantInitializer)
67+
performLint(UseSynthesizedInitializer.self, input: input)
68+
XCTAssertDiagnosed(.removeRedundantInitializer, line: 7)
4669
}
4770

4871
func testCustomInitializerVoidsSynthesizedInitializerWarning() {
@@ -81,7 +104,7 @@ final class UseSynthesizedInitializerTests: LintOrFormatRuleTestCase {
81104
82105
public var name: String
83106
let phoneNumber: String
84-
private let address: String
107+
let address: String
85108
86109
init(name: String = "Jane Doe", phoneNumber: String, address: String) {
87110
self.name = name
@@ -102,7 +125,7 @@ final class UseSynthesizedInitializerTests: LintOrFormatRuleTestCase {
102125
103126
public var name: String = "John Doe"
104127
let phoneNumber: String
105-
private let address: String
128+
let address: String
106129
107130
init(name: String = "Jane Doe", phoneNumber: String, address: String) {
108131
self.name = name
@@ -125,7 +148,7 @@ final class UseSynthesizedInitializerTests: LintOrFormatRuleTestCase {
125148
126149
public var name: String
127150
var phoneNumber: String = "+15555550101"
128-
private let address: String
151+
let address: String
129152
130153
init(name: String, phoneNumber: String, address: String) {
131154
self.name = name
@@ -146,7 +169,7 @@ final class UseSynthesizedInitializerTests: LintOrFormatRuleTestCase {
146169
147170
public var name: String
148171
var phoneNumber: String?
149-
private let address: String
172+
let address: String
150173
151174
init(name: String, phoneNumber: String, address: String) {
152175
self.name = name
@@ -167,7 +190,7 @@ final class UseSynthesizedInitializerTests: LintOrFormatRuleTestCase {
167190
168191
public var name: String
169192
var phoneNumber: String?
170-
private let address: String
193+
let address: String
171194
172195
init(name: String, phoneNumber: String?, address: String, anotherArg: Int) {
173196
self.name = name
@@ -188,7 +211,7 @@ final class UseSynthesizedInitializerTests: LintOrFormatRuleTestCase {
188211
189212
public var name: String
190213
var phoneNumber: String?
191-
private let address: String
214+
let address: String
192215
193216
init(name: String, phoneNumber: String?, address: String) {
194217
self.name = name
@@ -211,7 +234,7 @@ final class UseSynthesizedInitializerTests: LintOrFormatRuleTestCase {
211234
212235
public var name: String
213236
let phoneNumber: String
214-
private let address: String
237+
let address: String
215238
216239
init?(name: String, phoneNumber: String, address: String) {
217240
self.name = name
@@ -232,7 +255,7 @@ final class UseSynthesizedInitializerTests: LintOrFormatRuleTestCase {
232255
233256
public var name: String
234257
let phoneNumber: String
235-
private let address: String
258+
let address: String
236259
237260
init(name: String, phoneNumber: String, address: String) throws {
238261
self.name = name
@@ -253,7 +276,7 @@ final class UseSynthesizedInitializerTests: LintOrFormatRuleTestCase {
253276
254277
public var name: String
255278
let phoneNumber: String
256-
private let address: String
279+
let address: String
257280
258281
public init(name: String, phoneNumber: String, address: String) {
259282
self.name = name
@@ -266,4 +289,119 @@ final class UseSynthesizedInitializerTests: LintOrFormatRuleTestCase {
266289
performLint(UseSynthesizedInitializer.self, input: input)
267290
XCTAssertNotDiagnosed(.removeRedundantInitializer)
268291
}
292+
293+
func testDefaultMemberwiseInitializerIsNotDiagnosed() {
294+
// The synthesized initializer is private when any member is private, so an initializer with
295+
// default access control (i.e. internal) is not equivalent to the synthesized initializer.
296+
let input =
297+
"""
298+
public struct Person {
299+
300+
let phoneNumber: String
301+
private let address: String
302+
303+
init(phoneNumber: String, address: String) {
304+
self.address = address
305+
self.phoneNumber = phoneNumber
306+
}
307+
}
308+
"""
309+
310+
performLint(UseSynthesizedInitializer.self, input: input)
311+
XCTAssertNotDiagnosed(.removeRedundantInitializer)
312+
}
313+
314+
func testPrivateMemberwiseInitializerWithPrivateMemberIsDiagnosed() {
315+
// The synthesized initializer is private when any member is private, so a private initializer
316+
// is equivalent to the synthesized initializer.
317+
let input =
318+
"""
319+
public struct Person {
320+
321+
let phoneNumber: String
322+
private let address: String
323+
324+
private init(phoneNumber: String, address: String) {
325+
self.address = address
326+
self.phoneNumber = phoneNumber
327+
}
328+
}
329+
"""
330+
331+
performLint(UseSynthesizedInitializer.self, input: input)
332+
XCTAssertDiagnosed(.removeRedundantInitializer, line: 6)
333+
}
334+
335+
func testFileprivateMemberwiseInitializerWithFileprivateMemberIsDiagnosed() {
336+
// The synthesized initializer is fileprivate when any member is fileprivate, so a fileprivate
337+
// initializer is equivalent to the synthesized initializer.
338+
let input =
339+
"""
340+
public struct Person {
341+
342+
let phoneNumber: String
343+
fileprivate let address: String
344+
345+
fileprivate init(phoneNumber: String, address: String) {
346+
self.address = address
347+
self.phoneNumber = phoneNumber
348+
}
349+
}
350+
"""
351+
352+
performLint(UseSynthesizedInitializer.self, input: input)
353+
XCTAssertDiagnosed(.removeRedundantInitializer, line: 6)
354+
}
355+
356+
func testCustomSetterAccessLevel() {
357+
// When a property has a different access level for its setter, the setter's access level
358+
// doesn't change the access level of the synthesized initializer.
359+
let input =
360+
"""
361+
public struct Person {
362+
let phoneNumber: String
363+
private(set) let address: String
364+
365+
init(phoneNumber: String, address: String) {
366+
self.address = address
367+
self.phoneNumber = phoneNumber
368+
}
369+
}
370+
371+
public struct Person2 {
372+
fileprivate let phoneNumber: String
373+
private(set) let address: String
374+
375+
fileprivate init(phoneNumber: String, address: String) {
376+
self.address = address
377+
self.phoneNumber = phoneNumber
378+
}
379+
}
380+
381+
public struct Person3 {
382+
fileprivate(set) let phoneNumber: String
383+
private(set) let address: String
384+
385+
init(phoneNumber: String, address: String) {
386+
self.address = address
387+
self.phoneNumber = phoneNumber
388+
}
389+
}
390+
391+
public struct Person4 {
392+
private fileprivate(set) let phoneNumber: String
393+
private(set) let address: String
394+
395+
init(phoneNumber: String, address: String) {
396+
self.address = address
397+
self.phoneNumber = phoneNumber
398+
}
399+
}
400+
"""
401+
402+
performLint(UseSynthesizedInitializer.self, input: input)
403+
XCTAssertDiagnosed(.removeRedundantInitializer, line: 5)
404+
XCTAssertDiagnosed(.removeRedundantInitializer, line: 15)
405+
XCTAssertDiagnosed(.removeRedundantInitializer, line: 25)
406+
}
269407
}

0 commit comments

Comments
 (0)