Merge pull request #13 from s2mr/fix/prioritizeTodoOverMixedChinese

Fix/prioritize todo over mixed chinese

.l10nlint.yml

@@ -1,4 +1,5 @@
-base_path: Tests/L10nLintFrameworkTests/Resources/Fixtures/Localizables4
+base_path: Tests/L10nLintFrameworkTests/Resources/Fixtures/Localizables7
+prioritize_todo_over_mixed_chinese: true
 
 disabled_rules:
   - empty_value

Sources/L10nLintFramework/Models/LintRunner.swift

@@ -22,6 +22,7 @@ public final class LintRunner {
             }
             return rule
         }
+
         let linters = projects.map { project in
             return Linter(baseProject: baseProject, project: project, rules: rules)
         }
@@ -30,15 +31,54 @@ public final class LintRunner {
             try $0.lint()
         }
 
-        guard configuration.prioritizeTodoOverMixedChinese else { return violations }
-        let todoViolations = violations.filter { $0.ruleIdentifier == TodoRule.description.identifier }
+        return violations
+            .filterByPrioritizeTodoOverMixedChinese(for: configuration)
+            .modifyByTodoSummary(for: configuration)
+    }
+}
+
+private extension [StyleViolation] {
+    func filterByPrioritizeTodoOverMixedChinese(for configuration: Configuration) -> [StyleViolation] {
+        guard configuration.prioritizeTodoOverMixedChinese else { return self }
+        let todoViolations = filter { $0.ruleIdentifier == TodoRule.description.identifier }
 
-        return violations.filter { violation in
-            guard violation.ruleIdentifier == MixedChineseRule.description.identifier else { return true }
+        let filteredMixedChineseViolations = filter { violation in
+            guard violation.ruleIdentifier == MixedChineseRule.description.identifier else { return false }
 
             return !todoViolations.contains(where: { todoViolation in
                 violation.location.file == todoViolation.location.file && violation.location.line == todoViolation.location.line
             })
         }
+
+        let otherViolations = filter {
+            $0.ruleIdentifier != MixedChineseRule.description.identifier &&
+            $0.ruleIdentifier != TodoRule.description.identifier
+        }
+        return todoViolations + filteredMixedChineseViolations + otherViolations
+    }
+
+    func modifyByTodoSummary(for configuration: Configuration) -> [StyleViolation] {
+        let todoConfiguration = configuration.ruleConfigurations.todo ?? .init()
+        let todoViolations = filter { $0.ruleIdentifier == TodoRule.description.identifier }
+        let otherViolations = filter { $0.ruleIdentifier != TodoRule.description.identifier }
+
+        guard todoConfiguration.isSummaryEnabled && todoViolations.count > todoConfiguration.summaryViolationLimit else { return self }
+        var todoViolationsByFile = todoViolations.reduce(into: [String: [StyleViolation]]()) { partialResult, violation in
+            if partialResult[violation.location.file ?? ""] == nil {
+                partialResult[violation.location.file ?? ""] = []
+            }
+            partialResult[violation.location.file ?? ""]?.append(violation)
+        }
+
+        for violations in todoViolationsByFile {
+            let lines = violations.value.compactMap(\.location.line).map(String.init).joined(separator: ",")
+            if var violation = violations.value.first {
+                violation.location.line = 1
+                violation.reason = "Lines has FIXME or TODO. line: \(lines)"
+                todoViolationsByFile[violations.key] = [violation]
+            }
+        }
+
+        return todoViolationsByFile.flatMap(\.value) + otherViolations
     }
 }

Sources/L10nLintFramework/Models/SourceKitten/Location.swift

@@ -5,7 +5,7 @@ public struct Location: CustomStringConvertible, Comparable, Codable {
     /// The file path on disk for this location.
     public let file: String?
     /// The line offset in the file for this location. 1-indexed.
-    public let line: Int?
+    public var line: Int?
     /// The character offset in the file for this location. 1-indexed.
     public let character: Int?
 

Sources/L10nLintFramework/Models/SwiftLint/StyleViolation.swift

@@ -13,10 +13,10 @@ public struct StyleViolation: CustomStringConvertible, Equatable, Codable {
     public private(set) var severity: ViolationSeverity
 
     /// The location of this violation.
-    public private(set) var location: Location
+    public var location: Location
 
     /// The justification for this violation.
-    public let reason: String
+    public var reason: String
 
     /// A printable description for this violation.
     public var description: String {

Sources/L10nLintFramework/Rules/TodoRule.swift

@@ -24,19 +24,7 @@ public struct TodoRule: ConfigurationProviderRule {
             )
         }
 
-        if configuration.isSummaryEnabled && dataSets.count > configuration.summaryViolationLimit {
-            let lines = dataSets.compactMap(\.violation.location.line).map(String.init).joined(separator: ",")
-
-            return [StyleViolation(
-                ruleDescription: Self.description,
-                severity: .warning,
-                location: Location(file: project.stringsFile.path, line: 1),
-                reason: "Lines has FIXME or TODO. line: \(lines)"
-            )]
-        }
-        else {
-            return dataSets.map(\.violation)
-        }
+        return dataSets.map(\.violation)
     }
 }
 

Tests/L10nLintFrameworkTests/LintRunnerTests.swift

@@ -2,7 +2,7 @@ import XCTest
 @testable import L10nLintFramework
 
 final class LintRunnerTests: XCTestCase {
-    func testPreferFixMeThanMixedChineseFalse() throws {
+    func testPrioritizeTodoOverMixedChineseFalse() throws {
         let violations = try LintRunner.run(configuration: Configuration(
             basePath: TestHelper.fixtureURL(fixtureName: "Localizables7").path,
             prioritizeTodoOverMixedChinese: false
@@ -15,15 +15,61 @@ final class LintRunnerTests: XCTestCase {
         ])
     }
 
-    func testPreferFixMeThanMixedChineseTrue() throws {
+    func testPrioritizeTodoOverMixedChineseTrue() throws {
         let violations = try LintRunner.run(configuration: Configuration(
             basePath: TestHelper.fixtureURL(fixtureName: "Localizables7").path,
             prioritizeTodoOverMixedChinese: true
         ))
         XCTAssertEqual(violations.map(\.ruleWithLocationPoint), [
             RuleWithLocationPoint(ruleIdentifier: "todo", file: "zh-Hans.lproj", line: 2, character: 1),
-            RuleWithLocationPoint(ruleIdentifier: "mixed_chinese", file: "zh-Hans.lproj", line: 3, character: 12),
-            RuleWithLocationPoint(ruleIdentifier: "todo", file: "zh-Hant.lproj", line: 2, character: 1)
+            RuleWithLocationPoint(ruleIdentifier: "todo", file: "zh-Hant.lproj", line: 2, character: 1),
+            RuleWithLocationPoint(ruleIdentifier: "mixed_chinese", file: "zh-Hans.lproj", line: 3, character: 12)
+        ])
+    }
+
+    func testTodoSummaryTrue() throws {
+        let violations = try LintRunner.run(configuration: Configuration(
+            basePath: TestHelper.fixtureURL(fixtureName: "Localizables8").path,
+            prioritizeTodoOverMixedChinese: false,
+            ruleConfigurations: .init(todo: .init(
+                isSummaryEnabled: true,
+                summaryViolationLimit: 3
+            ))
+        ))
+        XCTAssertEqual(violations.map(\.ruleWithLocationPoint), [
+            RuleWithLocationPoint(ruleIdentifier: "todo", file: "Base.lproj", line: 1, character: 1)
+        ])
+    }
+
+    func testTodoSummaryTrueLimit5() throws {
+        let violations = try LintRunner.run(configuration: Configuration(
+            basePath: TestHelper.fixtureURL(fixtureName: "Localizables8").path,
+            prioritizeTodoOverMixedChinese: false,
+            ruleConfigurations: .init(todo: .init(
+                isSummaryEnabled: true,
+                summaryViolationLimit: 4
+            ))
+        ))
+        XCTAssertEqual(violations.map(\.ruleWithLocationPoint), [
+            RuleWithLocationPoint(ruleIdentifier: "todo", file: "Base.lproj", line: 1, character: 1)
+        ])
+    }
+
+    func testTodoSummaryFalse() throws {
+        let violations = try LintRunner.run(configuration: Configuration(
+            basePath: TestHelper.fixtureURL(fixtureName: "Localizables8").path,
+            prioritizeTodoOverMixedChinese: false,
+            ruleConfigurations: .init(todo: .init(
+                isSummaryEnabled: false,
+                summaryViolationLimit: 3
+            ))
+        ))
+        XCTAssertEqual(violations.map(\.ruleWithLocationPoint), [
+            RuleWithLocationPoint(ruleIdentifier: "todo", file: "Base.lproj", line: 2, character: 1),
+            RuleWithLocationPoint(ruleIdentifier: "todo", file: "Base.lproj", line: 3, character: 1),
+            RuleWithLocationPoint(ruleIdentifier: "todo", file: "Base.lproj", line: 4, character: 1),
+            RuleWithLocationPoint(ruleIdentifier: "todo", file: "Base.lproj", line: 5, character: 1),
+            RuleWithLocationPoint(ruleIdentifier: "todo", file: "Base.lproj", line: 6, character: 1)
         ])
     }
 }

Tests/L10nLintFrameworkTests/Resources/Fixtures/Localizables8/Base.lproj/Localizable.strings

@@ -0,0 +1,6 @@
+// MARK: TodoSummary
+"Key1" = "FIXME: A";
+"Key2" = "FIXME: B";
+"Key3" = "FIXME: C";
+"Key4" = "FIXME: D";
+"Key5" = "FIXME: E";

Tests/L10nLintFrameworkTests/Resources/Fixtures/Localizables8/zh-Hans.lproj/Localizable.strings

@@ -0,0 +1,6 @@
+// MARK: TodoSummary
+"Key1" = "A";
+"Key2" = "B";
+"Key3" = "C";
+"Key4" = "D";
+"Key5" = "E";