Merge pull request #7 from jayrave/update_formatter

jayrave · web-flow · commit 4ec9055f329c · 2020-02-15T22:03:32.000-08:00
Update formatter &amp; pull version number from cargo for release
diff --git a/.travis.yml b/.travis.yml
@@ -33,20 +33,21 @@ before_deploy:
 deploy:
   provider: releases
   api_key: $GITHUB_TOKEN
-  file: android_localization-$TRAVIS_TAG-$TARGET.tar.gz
+  file_glob: true
+  file: android_localization-*-$TARGET.tar.gz
   skip_cleanup: true
   draft: true
   overwrite: false
   on:
     branch: master
-    tags: true
+    condition: $TRAVIS_TAG = release
 
 branches:
   only:
     # Pushes and PR to the master & develop branch
     - master
-    - develop
-    - /^\d+\.\d+\.\d+$/
+    # To let our `release` tag trigger CI builds
+    - release
 
 notifications:
   email:
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,3 +1,6 @@
+# 0.1.9
+- Report each unlocalized string as a separate error
+
 # 0.1.8
 - Make validate fail if there is unlocalized text with an option to turn it off
 
diff --git a/ci/before_deploy.sh b/ci/before_deploy.sh
@@ -7,4 +7,6 @@ cargo build --all --release --target $TARGET --verbose
 cp target/$TARGET/release/android_localization_cli$EXE_SUFFIX android_localization$EXE_SUFFIX
 
 # Zip up the executable with name that resembles android_localization-0.1.4-x86_64-unknown-linux-gnu
-tar czf android_localization-$TRAVIS_TAG-$TARGET.tar.gz android_localization$EXE_SUFFIX
+current_version=`grep "version" cli/Cargo.toml | sed "s/version = //g" | sed "s/'//g"`
+echo "using $current_version as version from cli/Cargo.toml"
+tar czf android_localization-$current_version-$TARGET.tar.gz android_localization$EXE_SUFFIX
diff --git a/ci/script.sh b/ci/script.sh
@@ -2,5 +2,5 @@ set -ex
 
 cargo build --all --target $TARGET --verbose
 cargo test --all --target $TARGET --verbose
-cargo fmt -- --check
-cargo clippy -- -D warnings
+cargo fmt --all -- --check
+cargo clippy --all-targets --all-features -- -D warnings
diff --git a/cli/Cargo.toml b/cli/Cargo.toml
@@ -1,6 +1,6 @@
 [package]
 name = 'android_localization_cli'
-version = '0.1.8'
+version = '0.1.9'
 authors = ['jayrave <jayanthan.raveendiran@gmail.com>']
 edition = '2018'
 
diff --git a/cli/tests/file_utilities/mod.rs b/cli/tests/file_utilities/mod.rs
diff --git a/cli/tests/localize.rs b/cli/tests/localize.rs
@@ -2,7 +2,7 @@ use std::process::{Command, Output};
 
 use tempfile::TempDir;
 
-mod file_utilities;
+use test_utilities;
 
 #[test]
 fn succeeds_with_mapping() {
@@ -111,7 +111,7 @@ fn assert_status_and_stdout(output: Output) {
 }
 
 fn assert_output_files(temp_dir: TempDir, expected_output_dir_path: &str) {
-    file_utilities::assert_eq_of_file_contents_to_either_or(
+    test_utilities::file::assert_eq_of_file_contents_to_either_or(
         &format!("{}/to_localize_1.csv", temp_dir.path().to_str().unwrap()),
         &format!("{}/es_fr.csv", expected_output_dir_path),
         &format!("{}/fr_es.csv", expected_output_dir_path),
diff --git a/cli/tests/localized.rs b/cli/tests/localized.rs
@@ -5,8 +5,6 @@ use tempfile::TempDir;
 
 use test_utilities;
 
-mod file_utilities;
-
 #[test]
 fn succeeds_with_mapping() {
     execute_with_copied_sample_res(
@@ -70,7 +68,7 @@ fn warns_if_nothing_new_localized() {
                     "run",
                     "localized",
                     "--res-dir",
-                    &output_res_path.clone(),
+                    &output_res_path,
                     "--input-file",
                     "./tests_data/localized/warn/input/localized.csv",
                 ])
@@ -194,12 +192,12 @@ fn assert_status_and_stdout(output: Output) {
 }
 
 fn assert_output_files(output_res_path: String) {
-    file_utilities::assert_eq_of_file_contents(
+    test_utilities::file::assert_eq_of_file_contents(
         "./tests_data/localized/success/output/french_strings.xml",
         &format!("{}/values-fr/strings.xml", output_res_path),
     );
 
-    file_utilities::assert_eq_of_file_contents(
+    test_utilities::file::assert_eq_of_file_contents(
         "./tests_data/localized/success/output/spanish_strings.xml",
         &format!("{}/values-es/strings.xml", output_res_path),
     );
diff --git a/core/src/localized.rs b/core/src/localized.rs
@@ -240,7 +240,7 @@ mod tests {
 
         xml_writer::write(&mut zh_strings.file, chinese_android_strings.clone()).unwrap();
 
-        fs::create_dir_all(localized_dir_path.clone()).unwrap();
+        fs::create_dir_all(localized_dir_path).unwrap();
         let mut localized_file = File::create(localized_file_path.clone()).unwrap();
         localized_file
             .write_all(
diff --git a/core/src/validate/formatter.rs b/core/src/validate/formatter.rs
@@ -92,33 +92,23 @@ fn format_errors_from_one_file(
     }
 
     if let Some(missing_strings) = invalid_strings_file.missing_strings_error {
-        if !missing_strings.extra_in_default_locale.is_empty() {
+        for extra in missing_strings.extra_in_default_locale {
             issues_count_in_file += 1;
             writeln!(
                 &mut file_output,
-                "Error {} (unlocalized string(s)): {}",
+                "Error {} (unlocalized string): {}",
                 issues_count_in_file,
-                missing_strings
-                    .extra_in_default_locale
-                    .iter()
-                    .map(|s| s.value())
-                    .collect::<Vec<&str>>()
-                    .join(", ")
+                extra.value()
             )?;
         }
 
-        if !missing_strings.extra_in_foreign_locale.is_empty() {
+        for extra in missing_strings.extra_in_foreign_locale {
             issues_count_in_file += 1;
             writeln!(
                 &mut file_output,
-                "Error {} (string(s) not in defaut locale): {}",
+                "Error {} (string not in defaut locale): {}",
                 issues_count_in_file,
-                missing_strings
-                    .extra_in_foreign_locale
-                    .iter()
-                    .map(|s| s.value())
-                    .collect::<Vec<&str>>()
-                    .join(", ")
+                extra.value()
             )?;
         }
     }
@@ -149,6 +139,8 @@ mod tests {
 
     #[test]
     fn formats() {
+        // These strings needn't be invalid. We can include them in the
+        // error & the formatter will happily report it
         let default_s1 = AndroidString::localizable("s1", "default_value1");
         let default_s2 = AndroidString::localizable("s2", "default_value2");
         let french_s1 = AndroidString::localizable("s1", "french_value1");
@@ -184,23 +176,38 @@ mod tests {
             InvalidStringsFile {
                 file_path: String::from("spanish"),
                 apostrophe_error: Some(apostrophe::InvalidStrings {
-                    invalid_strings: vec![spanish_s1.clone()],
+                    invalid_strings: vec![spanish_s1.clone(), spanish_s2.clone()],
                 }),
                 format_string_error: Some(format_string::Mismatches {
-                    mismatches: vec![format_string::Mismatch {
-                        default_parsed_data: format_string::ParsedData {
-                            android_string: default_s1.clone(),
-                            sorted_format_strings: vec![String::from("%1$s"), String::from("%1$d")],
+                    mismatches: vec![
+                        format_string::Mismatch {
+                            default_parsed_data: format_string::ParsedData {
+                                android_string: default_s1.clone(),
+                                sorted_format_strings: vec![
+                                    String::from("%1$s"),
+                                    String::from("%1$d"),
+                                ],
+                            },
+                            foreign_parsed_data: format_string::ParsedData {
+                                android_string: spanish_s1.clone(),
+                                sorted_format_strings: vec![String::from("%1$d")],
+                            },
                         },
-                        foreign_parsed_data: format_string::ParsedData {
-                            android_string: spanish_s1.clone(),
-                            sorted_format_strings: vec![String::from("%1$d")],
+                        format_string::Mismatch {
+                            default_parsed_data: format_string::ParsedData {
+                                android_string: default_s2.clone(),
+                                sorted_format_strings: vec![],
+                            },
+                            foreign_parsed_data: format_string::ParsedData {
+                                android_string: spanish_s2.clone(),
+                                sorted_format_strings: vec![String::from("%1$s")],
+                            },
                         },
-                    }],
+                    ],
                 }),
                 missing_strings_error: Some(missing_strings::MissingStrings {
-                    extra_in_default_locale: vec![default_s1.clone(), default_s2.clone()],
-                    extra_in_foreign_locale: vec![spanish_s1.clone(), spanish_s2.clone()],
+                    extra_in_default_locale: vec![default_s1, default_s2],
+                    extra_in_foreign_locale: vec![spanish_s1, spanish_s2],
                 }),
             },
         ];
@@ -215,14 +222,19 @@ Path: french (1 issue)
 Error 1 (mismatched format string(s)): Found [asdf, qwer] in french_value1
                                        Found [] in default_value1
 
-Path: spanish (4 issues)
+Path: spanish (8 issues)
 Error 1 (unescaped apostrophe): spanish_value1
-Error 2 (mismatched format string(s)): Found [%1$d] in spanish_value1
+Error 2 (unescaped apostrophe): spanish_value2
+Error 3 (mismatched format string(s)): Found [%1$d] in spanish_value1
                                        Found [%1$s, %1$d] in default_value1
-Error 3 (unlocalized string(s)): default_value1, default_value2
-Error 4 (string(s) not in defaut locale): spanish_value1, spanish_value2
-
-Found 6 issues across 3 files!"#
+Error 4 (mismatched format string(s)): Found [%1$s] in spanish_value2
+                                       Found [] in default_value2
+Error 5 (unlocalized string): default_value1
+Error 6 (unlocalized string): default_value2
+Error 7 (string not in defaut locale): spanish_value1
+Error 8 (string not in defaut locale): spanish_value2
+
+Found 10 issues across 3 files!"#
             )
         );
     }
diff --git a/core/src/validate/validator.rs b/core/src/validate/validator.rs
@@ -239,7 +239,7 @@ mod tests {
 
             Some(missing_strings::MissingStrings {
                 extra_in_foreign_locale: vec![],
-                extra_in_default_locale: vec![default_s1.clone()],
+                extra_in_default_locale: vec![default_s1],
             })
         } else {
             missing_strings_error_for_fr = None;
diff --git a/test_utilities/src/file.rs b/test_utilities/src/file.rs
@@ -1,3 +1,4 @@
+use crate::eq;
 use std::fs::File;
 use std::io::Read;
 use std::io::Write;
@@ -14,3 +15,28 @@ pub fn write_content<P: AsRef<Path>, S: Into<String>>(file_path: P, content: S)
     let mut file = File::create(file_path).unwrap();
     file.write_all(content.into().as_bytes()).unwrap();
 }
+
+pub fn assert_eq_of_file_contents(actual_file_path: &str, expected_file_path: &str) {
+    let actual_file_lines = read_file_contents_as_lines(actual_file_path);
+    let expected_file_lines = read_file_contents_as_lines(expected_file_path);
+    assert_eq!(actual_file_lines, expected_file_lines)
+}
+
+pub fn assert_eq_of_file_contents_to_either_or(
+    actual_file_path: &str,
+    expected_file_path1: &str,
+    expected_file_path2: &str,
+) {
+    eq::assert_eq_to_either_or(
+        read_file_contents_as_lines(actual_file_path),
+        read_file_contents_as_lines(expected_file_path1),
+        read_file_contents_as_lines(expected_file_path2),
+    );
+}
+
+fn read_file_contents_as_lines(file_path: &str) -> Vec<String> {
+    // By default, the writers we employ, use \n as line terminator which
+    // wouldn't match when run on Windows! To work around this, we are using
+    // lines instead (`String#lines` takes care of handling both \n & \r\n)
+    read_content(file_path).lines().map(String::from).collect()
+}

-Original file line number
+Diff line change
@@ @@ -1,3 +1,6 @@ @@
 +# 0.1.9
 +- Report each unlocalized string as a separate error
++
 # 0.1.8
 - Make validate fail if there is unlocalized text with an option to turn it off