Skip to content

Commit 8ad61f0

Browse files
committed
Actually implement NoPlaygroundLiterals rule.
This file has been stubbed out forever, so actually implement it. It was originally meant to be a format rule, but those replacements are either tricky or undesirable: - `#fileLiteral` is implemented as `Bundle.main.url(forResource:withExtension:)`, but I don't want to "endorse" `Bundle.main` since it's almost always better to use `Bundle(for: AnyClass)` so that code works when it's pulled into a framework target or when running unit tests. - `#colorLiteral` and `#imageLiteral` are platform-dependent, being implemented as either `{NS,UI}Color` and `{NS,UI}Image` respectively. Sometimes this can be determined by checking for imports of `AppKit/Cocoa` or `UIKit`, but other frameworks like `SwiftUI` can re-export those and thus the source file may not actually have an import that we can unambiguously determine the right replacement for. It can also be impossible to determine if there are multi-platform conditions that import all of those modules. So, we just make these literals lint warnings and give the user rough ideas about how to fix them.
1 parent 0ebb3c8 commit 8ad61f0

File tree

5 files changed

+157
-12
lines changed

5 files changed

+157
-12
lines changed

Sources/SwiftFormat/Core/Pipelines+Generated.swift

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -183,6 +183,11 @@ class LintPipeline: SyntaxVisitor {
183183
return .visitChildren
184184
}
185185

186+
override func visit(_ node: GuardStmtSyntax) -> SyntaxVisitorContinueKind {
187+
visitIfEnabled(NoParensAroundConditions.visit, for: node)
188+
return .visitChildren
189+
}
190+
186191
override func visit(_ node: IdentifierPatternSyntax) -> SyntaxVisitorContinueKind {
187192
visitIfEnabled(IdentifiersMustBeASCII.visit, for: node)
188193
visitIfEnabled(NoLeadingUnderscores.visit, for: node)
@@ -217,6 +222,11 @@ class LintPipeline: SyntaxVisitor {
217222
return .visitChildren
218223
}
219224

225+
override func visit(_ node: MacroExpansionExprSyntax) -> SyntaxVisitorContinueKind {
226+
visitIfEnabled(NoPlaygroundLiterals.visit, for: node)
227+
return .visitChildren
228+
}
229+
220230
override func visit(_ node: MemberBlockItemListSyntax) -> SyntaxVisitorContinueKind {
221231
visitIfEnabled(DoNotUseSemicolons.visit, for: node)
222232
return .visitChildren
@@ -335,6 +345,11 @@ class LintPipeline: SyntaxVisitor {
335345
visitIfEnabled(UseTripleSlashForDocumentationComments.visit, for: node)
336346
return .visitChildren
337347
}
348+
349+
override func visit(_ node: WhileStmtSyntax) -> SyntaxVisitorContinueKind {
350+
visitIfEnabled(NoParensAroundConditions.visit, for: node)
351+
return .visitChildren
352+
}
338353
}
339354

340355
extension FormatPipeline {

Sources/SwiftFormat/Core/RuleNameCache+Generated.swift

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@ public let ruleNameCache: [ObjectIdentifier: String] = [
3636
ObjectIdentifier(NoLabelsInCasePatterns.self): "NoLabelsInCasePatterns",
3737
ObjectIdentifier(NoLeadingUnderscores.self): "NoLeadingUnderscores",
3838
ObjectIdentifier(NoParensAroundConditions.self): "NoParensAroundConditions",
39+
ObjectIdentifier(NoPlaygroundLiterals.self): "NoPlaygroundLiterals",
3940
ObjectIdentifier(NoVoidReturnOnFunctionSignature.self): "NoVoidReturnOnFunctionSignature",
4041
ObjectIdentifier(OmitExplicitReturns.self): "OmitExplicitReturns",
4142
ObjectIdentifier(OneCasePerLine.self): "OneCasePerLine",

Sources/SwiftFormat/Rules/NoPlaygroundLiterals.swift

Lines changed: 69 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -13,19 +13,76 @@
1313
import Foundation
1414
import SwiftSyntax
1515

16-
/// Playground literals (e.g. `#colorLiteral`) are forbidden.
16+
/// The playground literals (`#colorLiteral`, `#fileLiteral`, and `#imageLiteral`) are forbidden.
1717
///
18-
/// For the case of `#colorLiteral`, if `import AppKit` is present, `NSColor` will be used.
19-
/// If `import UIKit` is present, `UIColor` will be used.
20-
/// If neither `import` is present, `resolveAmbiguousColor` will be used to determine behavior.
21-
///
22-
/// Lint: Using a playground literal will yield a lint error.
23-
///
24-
/// Format: The playground literal will be replaced with the matching class; e.g.
25-
/// `#colorLiteral(...)` becomes `UIColor(...)`
26-
///
27-
/// Configuration: resolveAmbiguousColor
18+
/// Lint: Using a playground literal will yield a lint error with a suggestion of an API to replace
19+
/// it.
2820
@_spi(Rules)
29-
public final class NoPlaygroundLiterals: SyntaxFormatRule {
21+
public final class NoPlaygroundLiterals: SyntaxLintRule {
22+
override public func visit(_ node: MacroExpansionExprSyntax) -> SyntaxVisitorContinueKind {
23+
switch node.macroName.text {
24+
case "colorLiteral":
25+
diagnosedColorLiteralMacroExpansion(node)
26+
case "fileLiteral":
27+
diagnosedFileLiteralMacroExpansion(node)
28+
case "imageLiteral":
29+
diagnosedImageLiteralMacroExpansion(node)
30+
default:
31+
break
32+
}
33+
return .visitChildren
34+
}
35+
36+
private func diagnosedColorLiteralMacroExpansion(_ node: MacroExpansionExprSyntax) {
37+
guard isLiteralMacroCall(node, matchingLabels: ["red", "green", "blue", "alpha"]) else {
38+
return
39+
}
40+
diagnose(.replaceColorLiteral, on: node)
41+
}
42+
43+
private func diagnosedFileLiteralMacroExpansion(_ node: MacroExpansionExprSyntax) {
44+
guard isLiteralMacroCall(node, matchingLabels: ["resourceName"]) else {
45+
return
46+
}
47+
diagnose(.replaceFileLiteral, on: node)
48+
}
49+
50+
private func diagnosedImageLiteralMacroExpansion(_ node: MacroExpansionExprSyntax) {
51+
guard isLiteralMacroCall(node, matchingLabels: ["resourceName"]) else {
52+
return
53+
}
54+
diagnose(.replaceImageLiteral, on: node)
55+
}
56+
57+
/// Returns true if the given macro expansion is a correctly constructed call with the given
58+
/// argument labels and has no trailing closures or generic arguments.
59+
private func isLiteralMacroCall(
60+
_ node: MacroExpansionExprSyntax,
61+
matchingLabels labels: [String]
62+
) -> Bool {
63+
guard
64+
node.genericArgumentClause == nil,
65+
node.trailingClosure == nil,
66+
node.additionalTrailingClosures.isEmpty,
67+
node.arguments.count == labels.count
68+
else {
69+
return false
70+
}
71+
72+
for (actual, expected) in zip(node.arguments, labels) {
73+
guard actual.label?.text == expected else { return false }
74+
}
75+
return true
76+
}
77+
}
78+
79+
extension Finding.Message {
80+
fileprivate static let replaceColorLiteral: Finding.Message =
81+
"replace '#colorLiteral' with a call to an initializer on 'NSColor' or 'UIColor'"
82+
83+
fileprivate static let replaceFileLiteral: Finding.Message =
84+
"replace '#fileLiteral' with a call to a method such as 'Bundle.url(forResource:withExtension:)'"
3085

86+
fileprivate static let replaceImageLiteral: Finding.Message =
87+
"replace '#imageLiteral' with a call to an initializer on 'NSImage' or 'UIImage'"
3188
}

Sources/SwiftFormatConfiguration/RuleRegistry+Generated.swift

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@ enum RuleRegistry {
3535
"NoLabelsInCasePatterns": true,
3636
"NoLeadingUnderscores": false,
3737
"NoParensAroundConditions": true,
38+
"NoPlaygroundLiterals": true,
3839
"NoVoidReturnOnFunctionSignature": true,
3940
"OmitExplicitReturns": false,
4041
"OneCasePerLine": true,
Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,71 @@
1+
import _SwiftFormatTestSupport
2+
3+
@_spi(Rules) import SwiftFormat
4+
5+
final class NoPlaygroundLiteralsTests: LintOrFormatRuleTestCase {
6+
func testColorLiterals() {
7+
assertLint(
8+
NoPlaygroundLiterals.self,
9+
"""
10+
_ = 1️⃣#colorLiteral(red: 1.0, green: 0.0, blue: 0.0, alpha: 1.0)
11+
_ = #otherMacro(color: 2️⃣#colorLiteral(red: 1.0, green: 0.0, blue: 0.0, alpha: 1.0))
12+
_ = #otherMacro { 3️⃣#colorLiteral(red: 1.0, green: 0.0, blue: 0.0, alpha: 1.0) }
13+
14+
// Ignore invalid expansions.
15+
_ = #colorLiteral(1.0, 0.0, 0.0, 1.0)
16+
_ = #colorLiteral(r: 1.0, g: 0.0, b: 0.0, a: 1.0)
17+
_ = #colorLiteral(red: 1.0, green: 0.0, blue: 0.0, alpha: 1.0) { trailingClosure() }
18+
_ = #colorLiteral<SomeType>(red: 1.0, green: 0.0, blue: 0.0, alpha: 1.0)
19+
""",
20+
findings: [
21+
FindingSpec("1️⃣", message: "replace '#colorLiteral' with a call to an initializer on 'NSColor' or 'UIColor'"),
22+
FindingSpec("2️⃣", message: "replace '#colorLiteral' with a call to an initializer on 'NSColor' or 'UIColor'"),
23+
FindingSpec("3️⃣", message: "replace '#colorLiteral' with a call to an initializer on 'NSColor' or 'UIColor'"),
24+
]
25+
)
26+
}
27+
28+
func testFileLiterals() {
29+
assertLint(
30+
NoPlaygroundLiterals.self,
31+
"""
32+
_ = 1️⃣#fileLiteral(resourceName: "secrets.json")
33+
_ = #otherMacro(url: 2️⃣#fileLiteral(resourceName: "secrets.json"))
34+
_ = #otherMacro { 3️⃣#fileLiteral(resourceName: "secrets.json") }
35+
36+
// Ignore invalid expansions.
37+
_ = #fileLiteral("secrets.json")
38+
_ = #fileLiteral(name: "secrets.json")
39+
_ = #fileLiteral(resourceName: "secrets.json") { trailingClosure() }
40+
_ = #fileLiteral<SomeType>(resourceName: "secrets.json")
41+
""",
42+
findings: [
43+
FindingSpec("1️⃣", message: "replace '#fileLiteral' with a call to a method such as 'Bundle.url(forResource:withExtension:)'"),
44+
FindingSpec("2️⃣", message: "replace '#fileLiteral' with a call to a method such as 'Bundle.url(forResource:withExtension:)'"),
45+
FindingSpec("3️⃣", message: "replace '#fileLiteral' with a call to a method such as 'Bundle.url(forResource:withExtension:)'"),
46+
]
47+
)
48+
}
49+
50+
func testImageLiterals() {
51+
assertLint(
52+
NoPlaygroundLiterals.self,
53+
"""
54+
_ = 1️⃣#imageLiteral(resourceName: "image.png")
55+
_ = #otherMacro(url: 2️⃣#imageLiteral(resourceName: "image.png"))
56+
_ = #otherMacro { 3️⃣#imageLiteral(resourceName: "image.png") }
57+
58+
// Ignore invalid expansions.
59+
_ = #imageLiteral("image.png")
60+
_ = #imageLiteral(name: "image.pngn")
61+
_ = #imageLiteral(resourceName: "image.png") { trailingClosure() }
62+
_ = #imageLiteral<SomeType>(resourceName: "image.png")
63+
""",
64+
findings: [
65+
FindingSpec("1️⃣", message: "replace '#imageLiteral' with a call to an initializer on 'NSImage' or 'UIImage'"),
66+
FindingSpec("2️⃣", message: "replace '#imageLiteral' with a call to an initializer on 'NSImage' or 'UIImage'"),
67+
FindingSpec("3️⃣", message: "replace '#imageLiteral' with a call to an initializer on 'NSImage' or 'UIImage'"),
68+
]
69+
)
70+
}
71+
}

0 commit comments

Comments
 (0)