From 7f0f9137f1623a6e786d3b497b85e19210cfe32f Mon Sep 17 00:00:00 2001 From: Steve Landey <38225497+stevelandeyasana@users.noreply.github.com> Date: Mon, 8 Nov 2021 16:01:26 -0800 Subject: [PATCH] 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 --- .gitignore | 1 + Makefile | 2 +- Sources/LocheckCommand/main.swift | 2 +- Sources/LocheckLogic/ProblemReporter.swift | 6 +++ Sources/LocheckLogic/Problems.swift | 38 ++++++++++++++++++- .../parseAndValidateAndroidStrings.swift | 25 +++++++++--- .../parseAndValidateXCStrings.swift | 16 ++++++-- .../LocheckCommandTests/ExecutableTests.swift | 12 ++++++ 8 files changed, 88 insertions(+), 14 deletions(-) diff --git a/.gitignore b/.gitignore index 114cb8c..841ad35 100644 --- 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 index 82889c1..5b0e5d5 100644 --- 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 index 1bd54f3..516a0d1 100644 --- 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 index 1bd2443..e1de2f5 100644 --- 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 index 346e736..cc93482 100644 --- 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 ?? "")-\(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,6 +99,8 @@ 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" } } @@ -90,21 +108,25 @@ struct PhraseAndNativeArgumentsAreBothPresent: Problem, StringsProblem, Equatabl 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,6 +350,8 @@ 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 } @@ -327,5 +359,7 @@ 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 index b6a4b50..ccf3be0 100644 --- 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 index ad43964..ef3b795 100644 --- 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 index d69fb98..dbb973e 100644 --- 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