Workflow improvements & Android correctness (#29)

* Workflow improvements & Android correctness 1. "string_has_extra_arguments" -> "phrase_has_extra_arguments" (was a typo) 2. "phrase_has_missing_arguments" is now an error instead of a warning, since this problem causes a runtime crash 3. Display base string and translation in summary output so engineers can make bug reports more conveniently * Bump version to 0.9.5
Asana · Nov 9, 2021 · 7f0f913 · 7f0f913
1 parent 8a61126
commit 7f0f913
Show file tree

Hide file tree

Showing 8 changed files with 88 additions and 14 deletions.
diff --git a/.gitignore b/.gitignore
@@ -6,3 +6,4 @@ xcuserdata/
 DerivedData/
 .swiftpm/xcode/package.xcworkspace/contents.xcworkspacedata
 .swiftpm/xcode/xcshareddata/
+locheck.zip
diff --git a/Makefile b/Makefile
@@ -1,6 +1,6 @@
 EXECUTABLE_NAME = locheck
 REPO = https://github.com/Asana/locheck
-VERSION = 0.9.3
+VERSION = 0.9.5
 
 PREFIX = /usr/local
 INSTALL_PATH = $(PREFIX)/bin/$(EXECUTABLE_NAME)

diff --git a/Sources/LocheckCommand/main.swift b/Sources/LocheckCommand/main.swift
@@ -11,7 +11,7 @@ import Files
 import Foundation
 import LocheckLogic
 
-let version = "0.9.2"
+let version = "0.9.5"
 
 struct Locheck: ParsableCommand {
     static let configuration = CommandConfiguration(

diff --git a/Sources/LocheckLogic/ProblemReporter.swift b/Sources/LocheckLogic/ProblemReporter.swift
@@ -26,6 +26,8 @@ public protocol Problem {
     var uniquifyingInformation: String { get }
     var severity: Severity { get }
     var message: String { get }
+    var base: String? { get }
+    var translation: String? { get }
 }
 
 public protocol SummarizableProblem: Problem {
@@ -117,6 +119,10 @@ public class ProblemReporter {
                 for problem in summarizableProblems.filter({ $0.key == key }) {
                     print(
                         "    \(problem.severity.rawValue.uppercased()): \(problem.message) (\(problem.kindIdentifier))")
+                    if let translation = problem.translation, let base = problem.base {
+                        print("      Base: \(base)")
+                        print("      Translation: \(translation)")
+                    }
                 }
             }
         }

diff --git a/Sources/LocheckLogic/Problems.swift b/Sources/LocheckLogic/Problems.swift
@@ -11,6 +11,11 @@ protocol StringsdictProblem: SummarizableProblem {
     var key: String { get }
 }
 
+extension StringsdictProblem {
+    var base: String? { nil } // Never report translations for Stringsdict problems; they all require inspection
+    var translation: String? { nil } // Never report translations for Stringsdict problems; they all require inspection
+}
+
 protocol StringsProblem: SummarizableProblem {
     var key: String { get }
 }
@@ -19,6 +24,8 @@ struct CDATACannotBeDecoded: Problem, Equatable, SummarizableProblem {
     var kindIdentifier: String { "cdata_cannot_be_decoded" }
     var uniquifyingInformation: String { "\(key)" }
     var severity: Severity { .error }
+    var base: String? { nil }
+    var translation: String? { nil }
     let key: String
 
     var message: String { "'\(key)' has CDATA that cannot be decoded as UTF-8" }
@@ -28,6 +35,8 @@ struct SwiftError: Problem {
     var kindIdentifier: String { "swift_error" }
     var uniquifyingInformation: String { description }
     var severity: Severity { .error }
+    var base: String? { nil }
+    var translation: String? { nil }
     let description: String
 
     var message: String { description }
@@ -37,6 +46,8 @@ struct DuplicateEntries: Problem, Equatable {
     var kindIdentifier: String { "duplicate_entries" }
     var uniquifyingInformation: String { "\(context ?? "<root>")-\(name)" }
     var severity: Severity { .error }
+    var base: String? { nil }
+    var translation: String? { nil }
     let context: String?
     let name: String
 
@@ -53,6 +64,7 @@ struct KeyMissingFromBase: Problem, StringsdictProblem, Equatable {
     var kindIdentifier: String { "key_missing_from_base" }
     var uniquifyingInformation: String { "\(key)" }
     var severity: Severity { .warning }
+    var translation: String? { nil }
     let key: String
 
     var message: String { "'\(key)' is missing from the base translation" }
@@ -62,6 +74,8 @@ struct KeyMissingFromTranslation: Problem, StringsProblem, Equatable {
     var kindIdentifier: String { "key_missing_from_translation" }
     var uniquifyingInformation: String { "\(language)-\(key)" }
     var severity: Severity { .warning }
+    var base: String? { nil }
+    var translation: String? { nil }
     let key: String
     let language: String
 
@@ -72,6 +86,8 @@ struct LprojFileMissingFromTranslation: Problem, Equatable {
     var kindIdentifier: String { "lproj_file_missing_from_translation" }
     var uniquifyingInformation: String { "\(language)-\(key)" }
     var severity: Severity { .warning }
+    var base: String? { nil }
+    var translation: String? { nil }
     let key: String
     let language: String
 
@@ -83,28 +99,34 @@ struct PhraseAndNativeArgumentsAreBothPresent: Problem, StringsProblem, Equatabl
     var uniquifyingInformation: String { key }
     var severity: Severity { .warning }
     let key: String
+    let base: String?
+    let translation: String?
 
     var message: String { "'\(key)' contains both native (%d) and phrase-style ({arg}) arguments" }
 }
 
 struct PhraseHasMissingArguments: Problem, StringsProblem, Equatable {
     var kindIdentifier: String { "phrase_has_missing_arguments" }
     var uniquifyingInformation: String { "\(language)-\(key)" }
-    var severity: Severity { .warning }
+    var severity: Severity { .error }
     let key: String
     let language: String
     let args: [String]
+    let base: String?
+    let translation: String?
 
     var message: String { "'\(key)' does not include argument(s): \(args.joined(separator: ", "))" }
 }
 
 struct PhraseHasExtraArguments: Problem, StringsProblem, Equatable {
-    var kindIdentifier: String { "string_has_extra_arguments" }
+    var kindIdentifier: String { "phrase_has_extra_arguments" }
     var uniquifyingInformation: String { "\(language)-\(key)" }
     var severity: Severity { .error }
     let key: String
     let language: String
     let args: [String]
+    let base: String?
+    let translation: String?
 
     var message: String {
         "Translation of '\(key)' includes arguments that don't exist in the source: \(args.joined(separator: ", "))"
@@ -117,6 +139,8 @@ struct StringHasDuplicateArguments: Problem, StringsProblem, Equatable {
     var severity: Severity { .warning }
     let key: String
     let language: String
+    let base: String?
+    let translation: String?
 
     var message: String {
         "Some arguments appear more than once in this translation"
@@ -130,6 +154,8 @@ struct StringHasExtraArguments: Problem, StringsProblem, Equatable {
     let key: String
     let language: String
     let args: [String]
+    let base: String?
+    let translation: String?
 
     var message: String {
         "Translation of '\(key)' includes arguments that don't exist in the source: \(args.joined(separator: ", "))"
@@ -145,6 +171,8 @@ struct StringHasInvalidArgument: Problem, StringsProblem, Equatable {
     let argPosition: Int
     let baseArgSpecifier: String
     let argSpecifier: String
+    let base: String?
+    let translation: String?
 
     var message: String {
         "Specifier for argument \(argPosition) does not match (should be \(baseArgSpecifier), is \(argSpecifier))"
@@ -158,6 +186,8 @@ struct StringHasMissingArguments: Problem, StringsProblem, Equatable {
     let key: String
     let language: String
     let args: [String]
+    let base: String?
+    let translation: String?
 
     var message: String { "'\(key)' does not include argument(s) at \(args.joined(separator: ", "))" }
 }
@@ -320,12 +350,16 @@ struct XMLErrorProblem: Problem, Equatable {
     var kindIdentifier: String { "xml_error" }
     var uniquifyingInformation: String { message }
     var severity: Severity { .error }
+    var base: String? { nil }
+    var translation: String? { nil }
     let message: String
 }
 
 struct XMLSchemaProblem: Problem, Equatable {
     var kindIdentifier: String { "xml_schema_error" }
     var uniquifyingInformation: String { message }
     var severity: Severity { .error }
+    var base: String? { nil }
+    var translation: String? { nil }
     let message: String
 }
diff --git a/Sources/LocheckLogic/Validators/parseAndValidateAndroidStrings.swift b/Sources/LocheckLogic/Validators/parseAndValidateAndroidStrings.swift
@@ -69,7 +69,10 @@ func validateAndroidStrings(
 
         if !translationPhraseArgs.isEmpty && !translationArgs.isEmpty {
             problemReporter.report(
-                PhraseAndNativeArgumentsAreBothPresent(key: translationString.key),
+                PhraseAndNativeArgumentsAreBothPresent(
+                    key: translationString.key,
+                    base: baseString.value.string,
+                    translation: translationString.value.string),
                 path: translation.path,
                 lineNumber: translationString.line)
         }
@@ -84,7 +87,9 @@ func validateAndroidStrings(
                 StringHasMissingArguments(
                     key: translationString.key,
                     language: translationLanguageName,
-                    args: Array(argsMissingFromTranslation.sorted().map { String($0) })),
+                    args: Array(argsMissingFromTranslation.sorted().map { String($0) }),
+                    base: baseString.value.string,
+                    translation: translationString.value.string),
                 path: translation.path,
                 lineNumber: translationString.line)
         }
@@ -93,7 +98,9 @@ func validateAndroidStrings(
                 StringHasExtraArguments(
                     key: translationString.key,
                     language: translationLanguageName,
-                    args: Array(argsMissingFromTranslation.sorted().map { String($0) })),
+                    args: Array(argsMissingFromTranslation.sorted().map { String($0) }),
+                    base: baseString.value.string,
+                    translation: translationString.value.string),
                 path: translation.path,
                 lineNumber: baseString.line)
         }
@@ -102,7 +109,9 @@ func validateAndroidStrings(
                 PhraseHasMissingArguments(
                     key: translationString.key,
                     language: translationLanguageName,
-                    args: Array(phraseMissingFromTranslation).sorted()),
+                    args: Array(phraseMissingFromTranslation).sorted(),
+                    base: baseString.value.string,
+                    translation: translationString.value.string),
                 path: translation.path,
                 lineNumber: translationString.line)
         }
@@ -111,7 +120,9 @@ func validateAndroidStrings(
                 PhraseHasExtraArguments(
                     key: translationString.key,
                     language: translationLanguageName,
-                    args: Array(phraseMissingFromBase).sorted()),
+                    args: Array(phraseMissingFromBase).sorted(),
+                    base: baseString.value.string,
+                    translation: translationString.value.string),
                 path: translation.path,
                 lineNumber: baseString.line)
         }
@@ -127,7 +138,9 @@ func validateAndroidStrings(
                         language: translationLanguageName,
                         argPosition: arg.position,
                         baseArgSpecifier: baseArg.specifier,
-                        argSpecifier: arg.specifier),
+                        argSpecifier: arg.specifier,
+                        base: baseString.value.string,
+                        translation: translationString.value.string),
                     path: translation.path,
                     lineNumber: translationString.line)
             }

diff --git a/Sources/LocheckLogic/Validators/parseAndValidateXCStrings.swift b/Sources/LocheckLogic/Validators/parseAndValidateXCStrings.swift
@@ -80,7 +80,9 @@ func validateStrings(
                 StringHasMissingArguments(
                     key: translationString.key,
                     language: translationLanguageName,
-                    args: args),
+                    args: args,
+                    base: translationString.base.string,
+                    translation: translationString.translation.string),
                 path: translationString.path,
                 lineNumber: translationString.line)
         }
@@ -91,7 +93,9 @@ func validateStrings(
                 StringHasExtraArguments(
                     key: translationString.key,
                     language: translationLanguageName,
-                    args: args),
+                    args: args,
+                    base: translationString.base.string,
+                    translation: translationString.translation.string),
                 path: translationString.path,
                 lineNumber: translationString.line)
         }
@@ -100,7 +104,9 @@ func validateStrings(
             problemReporter.report(
                 StringHasDuplicateArguments(
                     key: translationString.key,
-                    language: translationLanguageName),
+                    language: translationLanguageName,
+                    base: translationString.base.string,
+                    translation: translationString.translation.string),
                 path: translationString.path,
                 lineNumber: translationString.line)
         }
@@ -118,7 +124,9 @@ func validateStrings(
                         language: translationLanguageName,
                         argPosition: arg.position,
                         baseArgSpecifier: baseArg.specifier,
-                        argSpecifier: arg.specifier),
+                        argSpecifier: arg.specifier,
+                        base: translationString.base.string,
+                        translation: translationString.translation.string),
                     path: translationString.path,
                     lineNumber: translationString.line)
             }

diff --git a/Tests/LocheckCommandTests/ExecutableTests.swift b/Tests/LocheckCommandTests/ExecutableTests.swift
@@ -59,13 +59,25 @@ class ExecutableTests: XCTestCase {
         Examples/Demo_Translation.strings
           bad pos %ld %@:
             WARNING: 'bad pos %ld %@' does not include argument(s) at 1 (string_has_missing_arguments)
+              Base: %1$ld %2$@
+              Translation: %2$ld %2$@
             WARNING: Some arguments appear more than once in this translation (string_has_duplicate_arguments)
+              Base: %1$ld %2$@
+              Translation: %2$ld %2$@
             ERROR: Specifier for argument 2 does not match (should be @, is ld) (string_has_invalid_argument)
+              Base: %1$ld %2$@
+              Translation: %2$ld %2$@
           bad position %d:
             WARNING: 'bad position %d' does not include argument(s) at 1 (string_has_missing_arguments)
+              Base: bad position %d
+              Translation: bad position %$d
           mismatch %@ types %d:
             ERROR: Specifier for argument 2 does not match (should be d, is @) (string_has_invalid_argument)
+              Base: mismatch %@ types %d
+              Translation: mismatch %2$@ types %1$d
             ERROR: Specifier for argument 1 does not match (should be @, is d) (string_has_invalid_argument)
+              Base: mismatch %@ types %d
+              Translation: mismatch %2$@ types %1$d
         4 warnings, 3 errors
         Errors found