Skip to content

Commit 5412cab

Browse files
authored
Merge pull request #918 from TTOzzi/fix-DontRepeatTypeInStaticProperties-diagnostic
Improve the DontRepeatTypeInStaticProperties rule to better align with its original intent
2 parents 26930a6 + cc02552 commit 5412cab

File tree

2 files changed

+104
-10
lines changed

2 files changed

+104
-10
lines changed

Sources/SwiftFormat/Rules/DontRepeatTypeInStaticProperties.swift

Lines changed: 28 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -26,19 +26,23 @@ public final class DontRepeatTypeInStaticProperties: SyntaxLintRule {
2626
/// type name (excluding possible namespace prefixes, like `NS` or `UI`) as a suffix.
2727
public override func visit(_ node: VariableDeclSyntax) -> SyntaxVisitorContinueKind {
2828
guard node.modifiers.contains(anyOf: [.class, .static]),
29-
let typeName = Syntax(node).containingDeclName
29+
let typeName = Syntax(node).containingDeclName,
30+
let variableTypeName = node.typeName,
31+
typeName.hasSuffix(variableTypeName) || variableTypeName == "Self"
3032
else {
3133
return .visitChildren
3234
}
3335

34-
let bareTypeName = removingPossibleNamespacePrefix(from: typeName)
36+
// the final component of the top type `A.B.C.D` is what we want `D`.
37+
let lastTypeName = typeName.components(separatedBy: ".").last!
38+
let bareTypeName = removingPossibleNamespacePrefix(from: lastTypeName)
3539
for binding in node.bindings {
3640
guard let identifierPattern = binding.pattern.as(IdentifierPatternSyntax.self) else {
3741
continue
3842
}
3943

4044
let varName = identifierPattern.identifier.text
41-
if varName.contains(bareTypeName) {
45+
if varName.hasSuffix(bareTypeName) {
4246
diagnose(.removeTypeFromName(name: varName, type: bareTypeName), on: identifierPattern)
4347
}
4448
}
@@ -90,8 +94,7 @@ extension Syntax {
9094
case .identifierType(let simpleType):
9195
return simpleType.name.text
9296
case .memberType(let memberType):
93-
// the final component of the top type `A.B.C.D` is what we want `D`.
94-
return memberType.name.text
97+
return memberType.description.trimmingCharacters(in: .whitespacesAndNewlines)
9598
default:
9699
// Do nothing for non-nominal types. If Swift adds support for extensions on non-nominals,
97100
// we'll need to update this if we need to support some subset of those.
@@ -106,3 +109,23 @@ extension Syntax {
106109
}
107110
}
108111
}
112+
113+
extension VariableDeclSyntax {
114+
fileprivate var typeName: String? {
115+
if let typeAnnotation = bindings.first?.typeAnnotation {
116+
return typeAnnotation.type.description
117+
} else if let initializerCalledExpression = bindings.first?.initializer?.value.as(FunctionCallExprSyntax.self)?
118+
.calledExpression
119+
{
120+
if let memberAccessExprSyntax = initializerCalledExpression.as(MemberAccessExprSyntax.self),
121+
memberAccessExprSyntax.declName.baseName.tokenKind == .keyword(.`init`)
122+
{
123+
return memberAccessExprSyntax.base?.description
124+
} else {
125+
return initializerCalledExpression.description
126+
}
127+
} else {
128+
return nil
129+
}
130+
}
131+
}

Tests/SwiftFormatTests/Rules/DontRepeatTypeInStaticPropertiesTests.swift

Lines changed: 76 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -56,8 +56,10 @@ final class DontRepeatTypeInStaticPropertiesTests: LintOrFormatRuleTestCase {
5656
extension A {
5757
static let b = C()
5858
}
59-
""",
60-
findings: []
59+
extension UIImage {
60+
static let fooImage: Int
61+
}
62+
"""
6163
)
6264
}
6365

@@ -75,20 +77,89 @@ final class DontRepeatTypeInStaticPropertiesTests: LintOrFormatRuleTestCase {
7577
)
7678
}
7779

80+
func testDottedExtendedTypeWithNamespacePrefix() {
81+
assertLint(
82+
DontRepeatTypeInStaticProperties.self,
83+
"""
84+
extension Dotted.RANDThing {
85+
static let 1️⃣defaultThing: Dotted.RANDThing
86+
}
87+
""",
88+
findings: [
89+
FindingSpec("1️⃣", message: "remove the suffix 'Thing' from the name of the variable 'defaultThing'")
90+
]
91+
)
92+
}
93+
94+
func testSelfType() {
95+
assertLint(
96+
DontRepeatTypeInStaticProperties.self,
97+
"""
98+
extension Dotted.Thing {
99+
static let 1️⃣defaultThing: Self
100+
}
101+
""",
102+
findings: [
103+
FindingSpec("1️⃣", message: "remove the suffix 'Thing' from the name of the variable 'defaultThing'")
104+
]
105+
)
106+
}
107+
108+
func testDottedExtendedTypeInitializer() {
109+
assertLint(
110+
DontRepeatTypeInStaticProperties.self,
111+
"""
112+
extension Dotted.Thing {
113+
static let 1️⃣defaultThing = Dotted.Thing()
114+
}
115+
""",
116+
findings: [
117+
FindingSpec("1️⃣", message: "remove the suffix 'Thing' from the name of the variable 'defaultThing'")
118+
]
119+
)
120+
}
121+
122+
func testExplicitInitializer() {
123+
assertLint(
124+
DontRepeatTypeInStaticProperties.self,
125+
"""
126+
struct Foo {
127+
static let 1️⃣defaultFoo = Foo.init()
128+
}
129+
""",
130+
findings: [
131+
FindingSpec("1️⃣", message: "remove the suffix 'Foo' from the name of the variable 'defaultFoo'")
132+
]
133+
)
134+
}
135+
136+
func testSelfTypeInitializer() {
137+
assertLint(
138+
DontRepeatTypeInStaticProperties.self,
139+
"""
140+
struct Foo {
141+
static let 1️⃣defaultFoo = Self()
142+
}
143+
""",
144+
findings: [
145+
FindingSpec("1️⃣", message: "remove the suffix 'Foo' from the name of the variable 'defaultFoo'")
146+
]
147+
)
148+
}
149+
78150
func testIgnoreSingleDecl() {
79151
assertLint(
80152
DontRepeatTypeInStaticProperties.self,
81153
"""
82154
struct Foo {
83155
// swift-format-ignore: DontRepeatTypeInStaticProperties
84-
static let defaultFoo: Int
85-
static let 1️⃣alternateFoo: Int
156+
static let defaultFoo: Foo
157+
static let 1️⃣alternateFoo: Foo
86158
}
87159
""",
88160
findings: [
89161
FindingSpec("1️⃣", message: "remove the suffix 'Foo' from the name of the variable 'alternateFoo'")
90162
]
91163
)
92164
}
93-
94165
}

0 commit comments

Comments
 (0)