Merge pull request #13 from nnethercote/fix-JSON-handling

Fix JSON handling

src/main.rs

@@ -542,16 +542,16 @@ impl Fixer {
         };
 
         let before = &captures[1];
-        let func_name = &captures[2];
+        let in_func_name = &captures[2];
-        let file_name = &captures[3];
+        let in_file_name = &captures[3];
         let address = u64::from_str_radix(&captures[4], 16).unwrap();
         let after = &captures[5];
 
         // If we haven't seen this file yet, parse and record its contents, for
         // this lookup and any future lookups.
-        let file_info = match self.file_infos.entry(file_name.to_string()) {
+        let file_info = match self.file_infos.entry(in_file_name.to_string()) {
             Entry::Occupied(o) => o.into_mut(),
-            Entry::Vacant(v) => match Fixer::build_file_info(file_name) {
+            Entry::Vacant(v) => match Fixer::build_file_info(in_file_name) {
                 Ok(file_info) => v.insert(file_info),
                 Err(op) => {
                     // Print an error message and then set up an empty
@@ -561,42 +561,57 @@ impl Fixer {
                     //   first occurrence.
                     // - The line will still receive some transformation, using
                     //   the "no symbols or debug info" case below.
-                    eprintln!("fix-stacks error: failed to {} `{}`", op, file_name);
+                    eprintln!("fix-stacks error: failed to {} `{}`", op, in_file_name);
                     v.insert(FileInfo::default())
                 }
             },
         };
 
-        let mut func_name_and_locn = if let Some(func_info) = file_info.func_info(address) {
+        // If JSON escaping is enabled, we need to escape any new strings we
-            if let Some(line_info) = func_info.line_info(address) {
+        // produce. However, strings that came in from the text (i.e.
-                // We have the filename and line number from the debug info.
+        // `in_func_name` and `in_file_name`), will already be escaped, so if
-                format!(
+        // they become part of the output they shouldn't be escaped.
-                    "{} ({}:{})",
+        if let Some(func_info) = file_info.func_info(address) {
-                    func_info.demangled_name(),
+            let raw_func_name = func_info.demangled_name();
-                    file_info.interner.get(line_info.path),
+            let out_func_name = if let JsonEscaping::Yes = self.json_escaping {
-                    line_info.line
+                Fixer::json_escape(&raw_func_name)
-                )
             } else {
-                // We have the filename from the debug info, but no line number.
+                raw_func_name
-                format!(
-                    "{} ({} +0x{:x})",
-                    func_info.demangled_name(),
-                    file_name,
-                    address
-                )
-            }
-        } else {
-            // We have nothing from the symbols or debug info. Use the file name
-            // from original input, which is probably "???". The end result is the
-            // same as the original line, but with the address removed and slightly
-            // different formatting.
-            format!("{} ({} +0x{:x})", func_name, file_name, address)
             };
 
-        if let JsonEscaping::Yes = self.json_escaping {
+            if let Some(line_info) = func_info.line_info(address) {
-            func_name_and_locn = Fixer::json_escape(&func_name_and_locn);
+                // We have the function name, filename, and line number from
+                // the debug info.
+                let raw_file_name = file_info.interner.get(line_info.path);
+                let out_file_name = if let JsonEscaping::Yes = self.json_escaping {
+                    Fixer::json_escape(&raw_file_name)
+                } else {
+                    raw_file_name.to_string()
+                };
+
+                format!(
+                    "{}{} ({}:{}){}",
+                    before, out_func_name, out_file_name, line_info.line, after
+                )
+            } else {
+                // We have the function name from the debug info, but no file
+                // name or line number. Use the file name and address from the
+                // original input.
+                format!(
+                    "{}{} ({} +0x{:x}){}",
+                    before, out_func_name, in_file_name, address, after
+                )
+            }
+        } else {
+            // We have nothing from the debug info. Use the function name, file
+            // name, and address from the original input. The end result is the
+            // same as the original line, but with slightly different
+            // formatting.
+            format!(
+                "{}{} ({} +0x{:x}{})",
+                before, in_func_name, in_file_name, address, after
+            )
         }
-        format!("{}{}{}", before, func_name_and_locn, after)
     }
 }
 

src/tests.rs

@@ -280,11 +280,6 @@ fn test_regex() {
     unchanged("#00: ???[tests/example-linux +1130]"); // Missing the '0x`.
     unchanged("#00: ???(tests/example-linux +0x1130)"); // Wrong parentheses.
 
-    // An error message is also printed to stderr for file errors, but we don't
-    // test for that.
-    unchanged("#00: ??? (tests/no-such-file)"); // No such file.
-    unchanged("#00: ??? (src/main.rs)"); // File exists, but has wrong format.
-
     // Test various different changed line forms that do match the regex.
     let mut changed = |line1: &str, line2_expected| {
         let line2_actual = fixer.fix(line1.to_string());
@@ -303,3 +298,30 @@ fn test_regex() {
         "#01: main (/home/njn/moz/fix-stacks/tests/example.c:24)",
     );
 }
+
+#[test]
+fn test_files() {
+    let mut fixer = Fixer::new(JsonEscaping::Yes);
+
+    // Test various different file errors. An error message is also printed to
+    // stderr for each one, but we don't test for that.
+    let mut file_error = |line1: &str, line2_expected| {
+        let line2_actual = fixer.fix(line1.to_string());
+        assert_eq!(line2_expected, line2_actual);
+    };
+    // No such file.
+    file_error(
+        "#00: ???[tests/no-such-file +0x0]",
+        "#00: ??? (tests/no-such-file +0x0)",
+    );
+    // No such file, with backslashes (which tests JSON escaping).
+    file_error(
+        "#00: ???[tests\\no-such-dir\\\\no-such-file +0x0]",
+        "#00: ??? (tests\\no-such-dir\\\\no-such-file +0x0)",
+    );
+    // File exists, but has the wrong format.
+    file_error(
+        "#00: ???[src/main.rs +0x0]",
+        "#00: ??? (src/main.rs +0x0)",
+    );
+}