Skip to content

Commit 9cdffee

Browse files
committed
Upgrade the DontRepeatTypeInStaticProperties rule.
- Using the `visit` method to examine all the variable declarations should allow specific decls to be ignored. - Update the PipelineGenerator to call `visitPost` methods - Modify the PipelineGenerator to produce output matching the current generated files.
1 parent ae1ef61 commit 9cdffee

10 files changed

+99
-81
lines changed

Documentation/RuleDocumentation.md

+1-1
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44

55
Use the rules below in the `rules` block of your `.swift-format`
66
configuration file, as described in
7-
[Configuration](Configuration.md). All of these rules can be
7+
[Configuration](Documentation/Configuration.md). All of these rules can be
88
applied in the linter, but only some of them can format your source code
99
automatically.
1010

Sources/SwiftFormat/Core/LintPipeline.swift

+7
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,13 @@ extension LintPipeline {
6565
/// - Parameters:
6666
/// - rule: The type of the syntax rule we're cleaning up.
6767
/// - node: The syntax node htat our traversal has left.
68+
func visitPost<Rule: SyntaxLintRule, Node: SyntaxProtocol>(
69+
_ visitPost: (Rule) -> (Node) -> Void, for node: Node
70+
) {
71+
guard context.shouldFormat(Rule.self, node: Syntax(node)) else { return }
72+
let rule = self.rule(Rule.self)
73+
visitPost(rule)(node)
74+
}
6875
func onVisitPost<R: Rule, Node: SyntaxProtocol>(
6976
rule: R.Type, for node: Node
7077
) {

Sources/SwiftFormat/Core/Pipelines+Generated.swift

+3-10
Original file line numberDiff line numberDiff line change
@@ -76,17 +76,16 @@ class LintPipeline: SyntaxVisitor {
7676
visitIfEnabled(AllPublicDeclarationsHaveDocumentation.visit, for: node)
7777
visitIfEnabled(AlwaysUseLowerCamelCase.visit, for: node)
7878
visitIfEnabled(BeginDocumentationCommentWithOneLineSummary.visit, for: node)
79-
visitIfEnabled(DontRepeatTypeInStaticProperties.visit, for: node)
8079
visitIfEnabled(NoLeadingUnderscores.visit, for: node)
8180
visitIfEnabled(TypeNamesShouldBeCapitalized.visit, for: node)
8281
visitIfEnabled(UseTripleSlashForDocumentationComments.visit, for: node)
8382
return .visitChildren
8483
}
8584
override func visitPost(_ node: ClassDeclSyntax) {
8685
onVisitPost(rule: AllPublicDeclarationsHaveDocumentation.self, for: node)
86+
visitPost(AlwaysUseLowerCamelCase.visitPost, for: node)
8787
onVisitPost(rule: AlwaysUseLowerCamelCase.self, for: node)
8888
onVisitPost(rule: BeginDocumentationCommentWithOneLineSummary.self, for: node)
89-
onVisitPost(rule: DontRepeatTypeInStaticProperties.self, for: node)
9089
onVisitPost(rule: NoLeadingUnderscores.self, for: node)
9190
onVisitPost(rule: TypeNamesShouldBeCapitalized.self, for: node)
9291
onVisitPost(rule: UseTripleSlashForDocumentationComments.self, for: node)
@@ -182,7 +181,6 @@ class LintPipeline: SyntaxVisitor {
182181

183182
override func visit(_ node: EnumDeclSyntax) -> SyntaxVisitorContinueKind {
184183
visitIfEnabled(BeginDocumentationCommentWithOneLineSummary.visit, for: node)
185-
visitIfEnabled(DontRepeatTypeInStaticProperties.visit, for: node)
186184
visitIfEnabled(FullyIndirectEnum.visit, for: node)
187185
visitIfEnabled(NoLeadingUnderscores.visit, for: node)
188186
visitIfEnabled(OneCasePerLine.visit, for: node)
@@ -192,7 +190,6 @@ class LintPipeline: SyntaxVisitor {
192190
}
193191
override func visitPost(_ node: EnumDeclSyntax) {
194192
onVisitPost(rule: BeginDocumentationCommentWithOneLineSummary.self, for: node)
195-
onVisitPost(rule: DontRepeatTypeInStaticProperties.self, for: node)
196193
onVisitPost(rule: FullyIndirectEnum.self, for: node)
197194
onVisitPost(rule: NoLeadingUnderscores.self, for: node)
198195
onVisitPost(rule: OneCasePerLine.self, for: node)
@@ -202,14 +199,12 @@ class LintPipeline: SyntaxVisitor {
202199

203200
override func visit(_ node: ExtensionDeclSyntax) -> SyntaxVisitorContinueKind {
204201
visitIfEnabled(AvoidRetroactiveConformances.visit, for: node)
205-
visitIfEnabled(DontRepeatTypeInStaticProperties.visit, for: node)
206202
visitIfEnabled(NoAccessLevelOnExtensionDeclaration.visit, for: node)
207203
visitIfEnabled(UseTripleSlashForDocumentationComments.visit, for: node)
208204
return .visitChildren
209205
}
210206
override func visitPost(_ node: ExtensionDeclSyntax) {
211207
onVisitPost(rule: AvoidRetroactiveConformances.self, for: node)
212-
onVisitPost(rule: DontRepeatTypeInStaticProperties.self, for: node)
213208
onVisitPost(rule: NoAccessLevelOnExtensionDeclaration.self, for: node)
214209
onVisitPost(rule: UseTripleSlashForDocumentationComments.self, for: node)
215210
}
@@ -423,7 +418,6 @@ class LintPipeline: SyntaxVisitor {
423418
override func visit(_ node: ProtocolDeclSyntax) -> SyntaxVisitorContinueKind {
424419
visitIfEnabled(AllPublicDeclarationsHaveDocumentation.visit, for: node)
425420
visitIfEnabled(BeginDocumentationCommentWithOneLineSummary.visit, for: node)
426-
visitIfEnabled(DontRepeatTypeInStaticProperties.visit, for: node)
427421
visitIfEnabled(NoLeadingUnderscores.visit, for: node)
428422
visitIfEnabled(TypeNamesShouldBeCapitalized.visit, for: node)
429423
visitIfEnabled(UseTripleSlashForDocumentationComments.visit, for: node)
@@ -432,7 +426,6 @@ class LintPipeline: SyntaxVisitor {
432426
override func visitPost(_ node: ProtocolDeclSyntax) {
433427
onVisitPost(rule: AllPublicDeclarationsHaveDocumentation.self, for: node)
434428
onVisitPost(rule: BeginDocumentationCommentWithOneLineSummary.self, for: node)
435-
onVisitPost(rule: DontRepeatTypeInStaticProperties.self, for: node)
436429
onVisitPost(rule: NoLeadingUnderscores.self, for: node)
437430
onVisitPost(rule: TypeNamesShouldBeCapitalized.self, for: node)
438431
onVisitPost(rule: UseTripleSlashForDocumentationComments.self, for: node)
@@ -469,7 +462,6 @@ class LintPipeline: SyntaxVisitor {
469462
override func visit(_ node: StructDeclSyntax) -> SyntaxVisitorContinueKind {
470463
visitIfEnabled(AllPublicDeclarationsHaveDocumentation.visit, for: node)
471464
visitIfEnabled(BeginDocumentationCommentWithOneLineSummary.visit, for: node)
472-
visitIfEnabled(DontRepeatTypeInStaticProperties.visit, for: node)
473465
visitIfEnabled(NoLeadingUnderscores.visit, for: node)
474466
visitIfEnabled(TypeNamesShouldBeCapitalized.visit, for: node)
475467
visitIfEnabled(UseSynthesizedInitializer.visit, for: node)
@@ -479,7 +471,6 @@ class LintPipeline: SyntaxVisitor {
479471
override func visitPost(_ node: StructDeclSyntax) {
480472
onVisitPost(rule: AllPublicDeclarationsHaveDocumentation.self, for: node)
481473
onVisitPost(rule: BeginDocumentationCommentWithOneLineSummary.self, for: node)
482-
onVisitPost(rule: DontRepeatTypeInStaticProperties.self, for: node)
483474
onVisitPost(rule: NoLeadingUnderscores.self, for: node)
484475
onVisitPost(rule: TypeNamesShouldBeCapitalized.self, for: node)
485476
onVisitPost(rule: UseSynthesizedInitializer.self, for: node)
@@ -568,6 +559,7 @@ class LintPipeline: SyntaxVisitor {
568559
visitIfEnabled(AllPublicDeclarationsHaveDocumentation.visit, for: node)
569560
visitIfEnabled(AlwaysUseLowerCamelCase.visit, for: node)
570561
visitIfEnabled(BeginDocumentationCommentWithOneLineSummary.visit, for: node)
562+
visitIfEnabled(DontRepeatTypeInStaticProperties.visit, for: node)
571563
visitIfEnabled(NeverUseImplicitlyUnwrappedOptionals.visit, for: node)
572564
visitIfEnabled(UseTripleSlashForDocumentationComments.visit, for: node)
573565
return .visitChildren
@@ -576,6 +568,7 @@ class LintPipeline: SyntaxVisitor {
576568
onVisitPost(rule: AllPublicDeclarationsHaveDocumentation.self, for: node)
577569
onVisitPost(rule: AlwaysUseLowerCamelCase.self, for: node)
578570
onVisitPost(rule: BeginDocumentationCommentWithOneLineSummary.self, for: node)
571+
onVisitPost(rule: DontRepeatTypeInStaticProperties.self, for: node)
579572
onVisitPost(rule: NeverUseImplicitlyUnwrappedOptionals.self, for: node)
580573
onVisitPost(rule: UseTripleSlashForDocumentationComments.self, for: node)
581574
}

Sources/SwiftFormat/Rules/AlwaysUseLowerCamelCase.swift

+1
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import SwiftSyntax
1717
///
1818
/// This rule does not apply to test code, defined as code which:
1919
/// * Contains the line `import XCTest`
20+
/// * The function is marked with `@Test` attribute
2021
///
2122
/// Lint: If an identifier contains underscores or begins with a capital letter, a lint error is
2223
/// raised.

Sources/SwiftFormat/Rules/DontRepeatTypeInStaticProperties.swift

+49-58
Original file line numberDiff line numberDiff line change
@@ -22,69 +22,26 @@ import SwiftSyntax
2222
@_spi(Rules)
2323
public final class DontRepeatTypeInStaticProperties: SyntaxLintRule {
2424

25-
public override func visit(_ node: ClassDeclSyntax) -> SyntaxVisitorContinueKind {
26-
diagnoseStaticMembers(node.memberBlock.members, endingWith: node.name.text)
27-
return .skipChildren
28-
}
29-
30-
public override func visit(_ node: EnumDeclSyntax) -> SyntaxVisitorContinueKind {
31-
diagnoseStaticMembers(node.memberBlock.members, endingWith: node.name.text)
32-
return .skipChildren
33-
}
34-
35-
public override func visit(_ node: ProtocolDeclSyntax) -> SyntaxVisitorContinueKind {
36-
diagnoseStaticMembers(node.memberBlock.members, endingWith: node.name.text)
37-
return .skipChildren
38-
}
39-
40-
public override func visit(_ node: StructDeclSyntax) -> SyntaxVisitorContinueKind {
41-
diagnoseStaticMembers(node.memberBlock.members, endingWith: node.name.text)
42-
return .skipChildren
43-
}
44-
45-
public override func visit(_ node: ExtensionDeclSyntax) -> SyntaxVisitorContinueKind {
46-
let members = node.memberBlock.members
47-
48-
switch Syntax(node.extendedType).as(SyntaxEnum.self) {
49-
case .identifierType(let simpleType):
50-
diagnoseStaticMembers(members, endingWith: simpleType.name.text)
51-
case .memberType(let memberType):
52-
// We don't need to drill recursively into this structure because types with more than two
53-
// components are constructed left-heavy; that is, `A.B.C.D` is structured as `((A.B).C).D`,
54-
// and the final component of the top type is what we want.
55-
diagnoseStaticMembers(members, endingWith: memberType.name.text)
56-
default:
57-
// Do nothing for non-nominal types. If Swift adds support for extensions on non-nominals,
58-
// we'll need to update this if we need to support some subset of those.
59-
break
25+
/// Visits the static/class properties and diagnoses any where the name has the containing
26+
/// type name (excluding possible namespace prefixes, like `NS` or `UI`) as a suffix.
27+
public override func visit(_ node: VariableDeclSyntax) -> SyntaxVisitorContinueKind {
28+
guard node.modifiers.contains(anyOf: [.class, .static]), let typeName = Syntax(node).typeName() else {
29+
return .visitChildren
6030
}
6131

62-
return .skipChildren
63-
}
64-
65-
/// Iterates over the static/class properties in the given member list and diagnoses any where the
66-
/// name has the containing type name (excluding possible namespace prefixes, like `NS` or `UI`)
67-
/// as a suffix.
68-
private func diagnoseStaticMembers(_ members: MemberBlockItemListSyntax, endingWith typeName: String) {
69-
for member in members {
70-
guard
71-
let varDecl = member.decl.as(VariableDeclSyntax.self),
72-
varDecl.modifiers.contains(anyOf: [.class, .static])
73-
else { continue }
74-
75-
let bareTypeName = removingPossibleNamespacePrefix(from: typeName)
76-
77-
for binding in varDecl.bindings {
78-
guard let identifierPattern = binding.pattern.as(IdentifierPatternSyntax.self) else {
79-
continue
80-
}
32+
let bareTypeName = removingPossibleNamespacePrefix(from: typeName)
33+
for binding in node.bindings {
34+
guard let identifierPattern = binding.pattern.as(IdentifierPatternSyntax.self) else {
35+
continue
36+
}
8137

82-
let varName = identifierPattern.identifier.text
83-
if varName.contains(bareTypeName) {
84-
diagnose(.removeTypeFromName(name: varName, type: bareTypeName), on: identifierPattern)
85-
}
38+
let varName = identifierPattern.identifier.text
39+
if varName.contains(bareTypeName) {
40+
diagnose(.removeTypeFromName(name: varName, type: bareTypeName), on: identifierPattern)
8641
}
8742
}
43+
44+
return .visitChildren
8845
}
8946

9047
/// Returns the portion of the given string that excludes a possible Objective-C-style capitalized
@@ -110,3 +67,37 @@ extension Finding.Message {
11067
"remove the suffix '\(type)' from the name of the variable '\(name)'"
11168
}
11269
}
70+
71+
extension Syntax {
72+
/// Returns the name of the immediately enclosing type of this node if there is one,
73+
/// otherwise nil.
74+
fileprivate func typeName() -> String? {
75+
if let node = self.as(ClassDeclSyntax.self) {
76+
return node.name.text
77+
} else if let node = self.as(EnumDeclSyntax.self) {
78+
return node.name.text
79+
} else if let node = self.as(ProtocolDeclSyntax.self) {
80+
return node.name.text
81+
} else if let node = self.as(StructDeclSyntax.self) {
82+
return node.name.text
83+
} else if let node = self.as(ExtensionDeclSyntax.self) {
84+
switch Syntax(node.extendedType).as(SyntaxEnum.self) {
85+
case .identifierType(let simpleType):
86+
return simpleType.name.text
87+
case .memberType(let memberType):
88+
// the final component of the top type `A.B.C.D` is what we want `D`.
89+
return memberType.name.text
90+
default:
91+
// Do nothing for non-nominal types. If Swift adds support for extensions on non-nominals,
92+
// we'll need to update this if we need to support some subset of those.
93+
break
94+
}
95+
} else if let parent = self.parent {
96+
return parent.typeName()
97+
}
98+
99+
return nil
100+
}
101+
102+
103+
}

Sources/SwiftFormat/Rules/NeverForceUnwrap.swift

+1
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import SwiftSyntax
1616
///
1717
/// This rule does not apply to test code, defined as code which:
1818
/// * Contains the line `import XCTest`
19+
/// * The function is marked with `@Test` attribute
1920
///
2021
/// Lint: If a force unwrap is used, a lint warning is raised.
2122
@_spi(Rules)

Sources/SwiftFormat/Rules/NeverUseForceTry.swift

+1
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import SwiftSyntax
1616
///
1717
/// This rule does not apply to test code, defined as code which:
1818
/// * Contains the line `import XCTest`
19+
/// * The function is marked with `@Test` attribute
1920
///
2021
/// Lint: Using `try!` results in a lint error.
2122
///

Sources/SwiftFormat/Rules/NeverUseImplicitlyUnwrappedOptionals.swift

+1
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ import SwiftSyntax
1818
///
1919
/// This rule does not apply to test code, defined as code which:
2020
/// * Contains the line `import XCTest`
21+
/// * The function is marked with `@Test` attribute
2122
///
2223
/// TODO: Create exceptions for other UI elements (ex: viewDidLoad)
2324
///

Sources/generate-swift-format/PipelineGenerator.swift

+13-4
Original file line numberDiff line numberDiff line change
@@ -68,17 +68,18 @@ final class PipelineGenerator: FileGenerator {
6868
)
6969

7070
for (nodeType, lintRules) in ruleCollector.syntaxNodeLinters.sorted(by: { $0.key < $1.key }) {
71+
let lintRules = lintRules.sorted(by: { $0.name < $1.name })
7172
handle.write(
7273
"""
7374
7475
override func visit(_ node: \(nodeType)) -> SyntaxVisitorContinueKind {
7576
7677
""")
7778

78-
for ruleName in lintRules.sorted() {
79+
for ruleName in lintRules {
7980
handle.write(
8081
"""
81-
visitIfEnabled(\(ruleName).visit, for: node)
82+
visitIfEnabled(\(ruleName.name).visit, for: node)
8283
8384
""")
8485
}
@@ -93,12 +94,20 @@ final class PipelineGenerator: FileGenerator {
9394
handle.write(
9495
"""
9596
override func visitPost(_ node: \(nodeType)) {
97+
9698
"""
9799
)
98-
for ruleName in lintRules.sorted() {
100+
for ruleName in lintRules {
101+
if ruleName.visitPost {
102+
handle.write(
103+
"""
104+
visitPost(\(ruleName.name).visitPost, for: node)
105+
106+
""")
107+
}
99108
handle.write(
100109
"""
101-
onVisitPost(rule: \(ruleName).self, for: node)
110+
onVisitPost(rule: \(ruleName.name).self, for: node)
102111
103112
""")
104113
}

Sources/generate-swift-format/RuleCollector.swift

+22-8
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,9 @@ final class RuleCollector {
3030
/// The syntax node types visited by the rule type.
3131
let visitedNodes: [String]
3232

33+
/// The syntax node types visited by the rule type.
34+
let visitedPostNodes: [String]
35+
3336
/// Indicates whether the rule can format code (all rules can lint).
3437
let canFormat: Bool
3538

@@ -44,7 +47,7 @@ final class RuleCollector {
4447
var allFormatters = Set<DetectedRule>()
4548

4649
/// A dictionary mapping syntax node types to the lint/format rules that visit them.
47-
var syntaxNodeLinters = [String: [String]]()
50+
var syntaxNodeLinters = [String: [(name: String, visitPost: Bool)]]()
4851

4952
/// Populates the internal collections with rules in the given directory.
5053
///
@@ -79,7 +82,8 @@ final class RuleCollector {
7982
// tree.
8083
allLinters.insert(detectedRule)
8184
for visitedNode in detectedRule.visitedNodes {
82-
syntaxNodeLinters[visitedNode, default: []].append(detectedRule.typeName)
85+
let visitPost = detectedRule.visitedPostNodes.contains(visitedNode)
86+
syntaxNodeLinters[visitedNode, default: []].append((detectedRule.typeName, visitPost: visitPost))
8387
}
8488
}
8589
}
@@ -126,16 +130,25 @@ final class RuleCollector {
126130
continue
127131
}
128132

129-
// Now that we know it's a format or lint rule, collect the `visit` methods.
133+
// Now that we know it's a format or lint rule, collect the `visit` and `visitPost` methods.
130134
var visitedNodes = [String]()
135+
var visitedPostNodes = [String]()
131136
for member in members {
132137
guard let function = member.decl.as(FunctionDeclSyntax.self) else { continue }
133-
guard function.name.text == "visit" else { continue }
134-
let params = function.signature.parameterClause.parameters
135-
guard let firstType = params.firstAndOnly?.type.as(IdentifierTypeSyntax.self) else {
136-
continue
138+
if function.name.text == "visit" {
139+
let params = function.signature.parameterClause.parameters
140+
guard let firstType = params.firstAndOnly?.type.as(IdentifierTypeSyntax.self) else {
141+
continue
142+
}
143+
visitedNodes.append(firstType.name.text)
144+
}
145+
if function.name.text == "visitPost" {
146+
let params = function.signature.parameterClause.parameters
147+
guard let firstType = params.firstAndOnly?.type.as(IdentifierTypeSyntax.self) else {
148+
continue
149+
}
150+
visitedPostNodes.append(firstType.name.text)
137151
}
138-
visitedNodes.append(firstType.name.text)
139152
}
140153

141154
/// Ignore it if it doesn't have any; there's no point in putting no-op rules in the pipeline.
@@ -148,6 +161,7 @@ final class RuleCollector {
148161
typeName: typeName,
149162
description: description?.text,
150163
visitedNodes: visitedNodes,
164+
visitedPostNodes: visitedPostNodes,
151165
canFormat: canFormat,
152166
isOptIn: ruleType.isOptIn)
153167
}

0 commit comments

Comments
 (0)