Merge pull request #613 from allevato/playground-literals

Actually implement `NoPlaygroundLiterals` rule.

Author: allevato

Sources/SwiftFormat/Core/Pipelines+Generated.swift

@@ -183,6 +183,11 @@ class LintPipeline: SyntaxVisitor {
     return .visitChildren
   }
 
+  override func visit(_ node: GuardStmtSyntax) -> SyntaxVisitorContinueKind {
+    visitIfEnabled(NoParensAroundConditions.visit, for: node)
+    return .visitChildren
+  }
+
   override func visit(_ node: IdentifierPatternSyntax) -> SyntaxVisitorContinueKind {
     visitIfEnabled(IdentifiersMustBeASCII.visit, for: node)
     visitIfEnabled(NoLeadingUnderscores.visit, for: node)
@@ -217,6 +222,11 @@ class LintPipeline: SyntaxVisitor {
     return .visitChildren
   }
 
+  override func visit(_ node: MacroExpansionExprSyntax) -> SyntaxVisitorContinueKind {
+    visitIfEnabled(NoPlaygroundLiterals.visit, for: node)
+    return .visitChildren
+  }
+
   override func visit(_ node: MemberBlockItemListSyntax) -> SyntaxVisitorContinueKind {
     visitIfEnabled(DoNotUseSemicolons.visit, for: node)
     return .visitChildren
@@ -335,6 +345,11 @@ class LintPipeline: SyntaxVisitor {
     visitIfEnabled(UseTripleSlashForDocumentationComments.visit, for: node)
     return .visitChildren
   }
+
+  override func visit(_ node: WhileStmtSyntax) -> SyntaxVisitorContinueKind {
+    visitIfEnabled(NoParensAroundConditions.visit, for: node)
+    return .visitChildren
+  }
 }
 
 extension FormatPipeline {

Sources/SwiftFormat/Core/RuleNameCache+Generated.swift

@@ -36,6 +36,7 @@ public let ruleNameCache: [ObjectIdentifier: String] = [
   ObjectIdentifier(NoLabelsInCasePatterns.self): "NoLabelsInCasePatterns",
   ObjectIdentifier(NoLeadingUnderscores.self): "NoLeadingUnderscores",
   ObjectIdentifier(NoParensAroundConditions.self): "NoParensAroundConditions",
+  ObjectIdentifier(NoPlaygroundLiterals.self): "NoPlaygroundLiterals",
   ObjectIdentifier(NoVoidReturnOnFunctionSignature.self): "NoVoidReturnOnFunctionSignature",
   ObjectIdentifier(OmitExplicitReturns.self): "OmitExplicitReturns",
   ObjectIdentifier(OneCasePerLine.self): "OneCasePerLine",

Sources/SwiftFormat/Rules/NoPlaygroundLiterals.swift

@@ -13,19 +13,76 @@
 import Foundation
 import SwiftSyntax
 
-/// Playground literals (e.g. `#colorLiteral`) are forbidden.
+/// The playground literals (`#colorLiteral`, `#fileLiteral`, and `#imageLiteral`) are forbidden.
 ///
-/// For the case of `#colorLiteral`, if `import AppKit` is present, `NSColor` will be used.
-/// If `import UIKit` is present, `UIColor` will be used.
-/// If neither `import` is present, `resolveAmbiguousColor` will be used to determine behavior.
-///
-/// Lint: Using a playground literal will yield a lint error.
-///
-/// Format: The playground literal will be replaced with the matching class; e.g.
-///         `#colorLiteral(...)` becomes `UIColor(...)`
-///
-/// Configuration: resolveAmbiguousColor
+/// Lint: Using a playground literal will yield a lint error with a suggestion of an API to replace
+/// it.
 @_spi(Rules)
-public final class NoPlaygroundLiterals: SyntaxFormatRule {
+public final class NoPlaygroundLiterals: SyntaxLintRule {
+  override public func visit(_ node: MacroExpansionExprSyntax) -> SyntaxVisitorContinueKind {
+    switch node.macroName.text {
+    case "colorLiteral":
+      diagnosedColorLiteralMacroExpansion(node)
+    case "fileLiteral":
+      diagnosedFileLiteralMacroExpansion(node)
+    case "imageLiteral":
+      diagnosedImageLiteralMacroExpansion(node)
+    default:
+      break
+    }
+    return .visitChildren
+  }
+
+  private func diagnosedColorLiteralMacroExpansion(_ node: MacroExpansionExprSyntax) {
+    guard isLiteralMacroCall(node, matchingLabels: ["red", "green", "blue", "alpha"]) else {
+      return
+    }
+    diagnose(.replaceColorLiteral, on: node)
+  }
+
+  private func diagnosedFileLiteralMacroExpansion(_ node: MacroExpansionExprSyntax) {
+    guard isLiteralMacroCall(node, matchingLabels: ["resourceName"]) else {
+      return
+    }
+    diagnose(.replaceFileLiteral, on: node)
+  }
+
+  private func diagnosedImageLiteralMacroExpansion(_ node: MacroExpansionExprSyntax) {
+    guard isLiteralMacroCall(node, matchingLabels: ["resourceName"]) else {
+      return
+    }
+    diagnose(.replaceImageLiteral, on: node)
+  }
+
+  /// Returns true if the given macro expansion is a correctly constructed call with the given
+  /// argument labels and has no trailing closures or generic arguments.
+  private func isLiteralMacroCall(
+    _ node: MacroExpansionExprSyntax,
+    matchingLabels labels: [String]
+  ) -> Bool {
+    guard
+      node.genericArgumentClause == nil,
+      node.trailingClosure == nil,
+      node.additionalTrailingClosures.isEmpty,
+      node.arguments.count == labels.count
+    else {
+      return false
+    }
+
+    for (actual, expected) in zip(node.arguments, labels) {
+      guard actual.label?.text == expected else { return false }
+    }
+    return true
+  }
+}
+
+extension Finding.Message {
+  fileprivate static let replaceColorLiteral: Finding.Message =
+    "replace '#colorLiteral' with a call to an initializer on 'NSColor' or 'UIColor'"
+
+  fileprivate static let replaceFileLiteral: Finding.Message =
+    "replace '#fileLiteral' with a call to a method such as 'Bundle.url(forResource:withExtension:)'"
 
+  fileprivate static let replaceImageLiteral: Finding.Message =
+    "replace '#imageLiteral' with a call to an initializer on 'NSImage' or 'UIImage'"
 }

Sources/SwiftFormatConfiguration/RuleRegistry+Generated.swift

@@ -35,6 +35,7 @@ enum RuleRegistry {
     "NoLabelsInCasePatterns": true,
     "NoLeadingUnderscores": false,
     "NoParensAroundConditions": true,
+    "NoPlaygroundLiterals": true,
     "NoVoidReturnOnFunctionSignature": true,
     "OmitExplicitReturns": false,
     "OneCasePerLine": true,

Tests/SwiftFormatTests/Rules/NoPlaygroundLiteralsTests.swift

@@ -0,0 +1,71 @@
+import _SwiftFormatTestSupport
+
+@_spi(Rules) import SwiftFormat
+
+final class NoPlaygroundLiteralsTests: LintOrFormatRuleTestCase {
+  func testColorLiterals() {
+    assertLint(
+      NoPlaygroundLiterals.self,
+      """
+      _ = 1️⃣#colorLiteral(red: 1.0, green: 0.0, blue: 0.0, alpha: 1.0)
+      _ = #otherMacro(color: 2️⃣#colorLiteral(red: 1.0, green: 0.0, blue: 0.0, alpha: 1.0))
+      _ = #otherMacro { 3️⃣#colorLiteral(red: 1.0, green: 0.0, blue: 0.0, alpha: 1.0) }
+
+      // Ignore invalid expansions.
+      _ = #colorLiteral(1.0, 0.0, 0.0, 1.0)
+      _ = #colorLiteral(r: 1.0, g: 0.0, b: 0.0, a: 1.0)
+      _ = #colorLiteral(red: 1.0, green: 0.0, blue: 0.0, alpha: 1.0) { trailingClosure() }
+      _ = #colorLiteral<SomeType>(red: 1.0, green: 0.0, blue: 0.0, alpha: 1.0)
+      """,
+      findings: [
+        FindingSpec("1️⃣", message: "replace '#colorLiteral' with a call to an initializer on 'NSColor' or 'UIColor'"),
+        FindingSpec("2️⃣", message: "replace '#colorLiteral' with a call to an initializer on 'NSColor' or 'UIColor'"),
+        FindingSpec("3️⃣", message: "replace '#colorLiteral' with a call to an initializer on 'NSColor' or 'UIColor'"),
+      ]
+    )
+  }
+
+  func testFileLiterals() {
+    assertLint(
+      NoPlaygroundLiterals.self,
+      """
+      _ = 1️⃣#fileLiteral(resourceName: "secrets.json")
+      _ = #otherMacro(url: 2️⃣#fileLiteral(resourceName: "secrets.json"))
+      _ = #otherMacro { 3️⃣#fileLiteral(resourceName: "secrets.json") }
+
+      // Ignore invalid expansions.
+      _ = #fileLiteral("secrets.json")
+      _ = #fileLiteral(name: "secrets.json")
+      _ = #fileLiteral(resourceName: "secrets.json") { trailingClosure() }
+      _ = #fileLiteral<SomeType>(resourceName: "secrets.json")
+      """,
+      findings: [
+        FindingSpec("1️⃣", message: "replace '#fileLiteral' with a call to a method such as 'Bundle.url(forResource:withExtension:)'"),
+        FindingSpec("2️⃣", message: "replace '#fileLiteral' with a call to a method such as 'Bundle.url(forResource:withExtension:)'"),
+        FindingSpec("3️⃣", message: "replace '#fileLiteral' with a call to a method such as 'Bundle.url(forResource:withExtension:)'"),
+      ]
+    )
+  }
+
+  func testImageLiterals() {
+    assertLint(
+      NoPlaygroundLiterals.self,
+      """
+      _ = 1️⃣#imageLiteral(resourceName: "image.png")
+      _ = #otherMacro(url: 2️⃣#imageLiteral(resourceName: "image.png"))
+      _ = #otherMacro { 3️⃣#imageLiteral(resourceName: "image.png") }
+
+      // Ignore invalid expansions.
+      _ = #imageLiteral("image.png")
+      _ = #imageLiteral(name: "image.pngn")
+      _ = #imageLiteral(resourceName: "image.png") { trailingClosure() }
+      _ = #imageLiteral<SomeType>(resourceName: "image.png")
+      """,
+      findings: [
+        FindingSpec("1️⃣", message: "replace '#imageLiteral' with a call to an initializer on 'NSImage' or 'UIImage'"),
+        FindingSpec("2️⃣", message: "replace '#imageLiteral' with a call to an initializer on 'NSImage' or 'UIImage'"),
+        FindingSpec("3️⃣", message: "replace '#imageLiteral' with a call to an initializer on 'NSImage' or 'UIImage'"),
+      ]
+    )
+  }
+}