'nslocalizedstring_key' now validates the comment argument in additio…

…n to the key argument (realm#3373)
agoda-com · Nov 7, 2020 · 580314d · 580314d
1 parent c8d8d91
commit 580314d
Show file tree

Hide file tree

Showing 2 changed files with 66 additions and 18 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -37,6 +37,11 @@
   [Zsolt Kovács](https://github.com/lordzsolt)
   [#1921](https://github.com/realm/SwiftLint/issues/1921)
 
+* Update the `nslocalizedstring_key` rule to validate the `comment`
+  argument in addition to the `key` argument.  
+  [Dalton Claybrook](https://github.com/daltonclaybrook)
+  [#3334](https://github.com/realm/SwiftLint/issues/3334)
+
 #### Bug Fixes
 
 * Fix parsing of Xcode 12 compiler logs for analyzer rules.  

diff --git a/Source/SwiftLintFramework/Rules/Lint/NSLocalizedStringKeyRule.swift b/Source/SwiftLintFramework/Rules/Lint/NSLocalizedStringKeyRule.swift
@@ -8,35 +8,78 @@ public struct NSLocalizedStringKeyRule: ASTRule, OptInRule, ConfigurationProvide
     public static let description = RuleDescription(
         identifier: "nslocalizedstring_key",
         name: "NSLocalizedString Key",
-        description: "Static strings should be used as key in NSLocalizedString in order to genstrings work.",
+        description: "Static strings should be used as key/comment" +
+            " in NSLocalizedString in order for genstrings to work.",
         kind: .lint,
         nonTriggeringExamples: [
-            Example("NSLocalizedString(\"key\", comment: nil)"),
-            Example("NSLocalizedString(\"key\" + \"2\", comment: nil)")
+            Example("NSLocalizedString(\"key\", comment: \"\")"),
+            Example("NSLocalizedString(\"key\" + \"2\", comment: \"\")"),
+            Example("NSLocalizedString(\"key\", comment: \"comment\")"),
+            Example("""
+            NSLocalizedString("This is a multi-" +
+                "line string", comment: "")
+            """),
+            Example("""
+            let format = NSLocalizedString("%@, %@.", comment: "Accessibility label for a post in the post list." +
+            " The parameters are the title, and date respectively." +
+            " For example, \"Let it Go, 1 hour ago.\"")
+            """)
         ],
         triggeringExamples: [
-            Example("NSLocalizedString(↓method(), comment: nil)"),
-            Example("NSLocalizedString(↓\"key_\\(param)\", comment: nil)")
+            Example("NSLocalizedString(↓method(), comment: \"\")"),
+            Example("NSLocalizedString(↓\"key_\\(param)\", comment: \"\")"),
+            Example("NSLocalizedString(\"key\", comment: ↓\"comment with \\(param)\")"),
+            Example("NSLocalizedString(↓\"key_\\(param)\", comment: ↓method())")
         ]
     )
 
     public func validate(file: SwiftLintFile,
                          kind: SwiftExpressionKind,
                          dictionary: SourceKittenDictionary) -> [StyleViolation] {
-        guard kind == .call,
-            dictionary.name == "NSLocalizedString",
-            let firstArgument = dictionary.enclosedArguments.first,
-            firstArgument.name == nil,
-            let byteRange = firstArgument.byteRange,
-            case let kinds = file.syntaxMap.kinds(inByteRange: byteRange),
-            !kinds.allSatisfy({ $0 == .string }) else {
-                return []
-        }
+        guard kind == .call, dictionary.name == "NSLocalizedString" else { return [] }
 
         return [
-            StyleViolation(ruleDescription: Self.description,
-                           severity: configuration.severity,
-                           location: Location(file: file, byteOffset: byteRange.location))
-        ]
+            getViolationForKey(file: file, dictionary: dictionary),
+            getViolationForComment(file: file, dictionary: dictionary)
+        ].compactMap { $0 }
+    }
+
+    // MARK: - Private helpers
+
+    private func getViolationForKey(file: SwiftLintFile,
+                                    dictionary: SourceKittenDictionary) -> StyleViolation? {
+        guard let keyArgument = dictionary.enclosedArguments
+                .first(where: { $0.name == nil }),
+              let byteRange = keyArgument.byteRange
+        else { return nil }
+
+        let kinds = file.syntaxMap.kinds(inByteRange: byteRange)
+        guard !kinds.allSatisfy({ $0 == .string }) else { return nil }
+
+        return makeViolation(file: file, byteRange: byteRange)
+    }
+
+    private func getViolationForComment(file: SwiftLintFile,
+                                        dictionary: SourceKittenDictionary) -> StyleViolation? {
+        guard let commentArgument = dictionary.enclosedArguments
+                .first(where: { $0.name == "comment" }),
+              let bodyByteRange = commentArgument.bodyByteRange
+        else { return nil }
+
+        let tokens = file.syntaxMap.tokens(inByteRange: bodyByteRange)
+        guard !tokens.isEmpty else { return nil }
+
+        if tokens.allSatisfy({ $0.kind == .string }) {
+            // All tokens are string literals
+            return nil
+        }
+
+        return makeViolation(file: file, byteRange: bodyByteRange)
+    }
+
+    private func makeViolation(file: SwiftLintFile, byteRange: ByteRange) -> StyleViolation {
+        StyleViolation(ruleDescription: Self.description,
+                       severity: configuration.severity,
+                       location: Location(file: file, byteOffset: byteRange.location))
     }
 }