Merge pull request #67 from rust-lang-nursery/cool-replacements

Use a more clever way to replace parts of a file

Author: alexcrichton

.travis.yml

@@ -3,6 +3,7 @@ rust:
   - nightly
 script:
   - cargo test --all
+  - cargo test --all -- --ignored
 notifications:
   email:
     on_success: never

Cargo.toml

@@ -20,13 +20,15 @@ exclude = [
 serde = "1.0"
 serde_json = "1.0"
 serde_derive = "1.0"
+failure = "0.1.1"
 
 [dev-dependencies]
 duct = "0.8.2"
 env_logger = "0.5.0-rc.1"
 log = "0.4.1"
 pretty_assertions = "0.4.1"
 tempdir = "0.3.5"
+proptest = "0.7.0"
 
 [workspace]
 members = [

cargo-fix/src/main.rs

@@ -177,7 +177,7 @@ fn rustfix_crate(rustc: &Path, filename: &str) -> Result<(), Error> {
         }
         debug!("applying {} fixes to {}", suggestions.len(), file);
 
-        let new_code = rustfix::apply_suggestions(&code, &suggestions);
+        let new_code = rustfix::apply_suggestions(&code, &suggestions)?;
         File::create(&file)
             .and_then(|mut f| f.write_all(new_code.as_bytes()))
             .with_context(|_| format!("failed to write file `{}`", file))?;

cargo-fix/tests/all/main.rs

@@ -2,8 +2,8 @@ extern crate difference;
 extern crate url;
 
 use std::env;
-use std::fs;
-use std::io::Write;
+use std::fs::{self, File};
+use std::io::{Read, Write};
 use std::path::{Path, PathBuf};
 use std::process::Command;
 use std::str;
@@ -84,6 +84,15 @@ impl Project {
             cwd: None,
         }
     }
+
+    fn read(&self, path: &str) -> String {
+        let mut ret = String::new();
+        File::open(self.root.join(path))
+            .unwrap()
+            .read_to_string(&mut ret)
+            .unwrap();
+        return ret
+    }
 }
 
 struct ExpectCmd<'a> {

cargo-fix/tests/all/smoke.rs

@@ -87,3 +87,44 @@ fn tricky_ampersand() {
         .stderr(stderr)
         .run();
 }
+
+#[test]
+fn preserve_line_endings() {
+    let p = project()
+        .file("src/lib.rs", "\
+            fn add(a: &u32) -> u32 { a + 1 }\r\n\
+            pub fn foo() -> u32 { add(1) }\r\n\
+        ")
+        .build();
+
+    p.expect_cmd("cargo fix").run();
+    assert!(p.read("src/lib.rs").contains("\r\n"));
+}
+
+#[test]
+fn multiple_suggestions_for_the_same_thing() {
+    let p = project()
+        .file("src/lib.rs", "\
+            fn main() {
+                let xs = vec![String::from(\"foo\")];
+                // error: no diplay in scope, and there are two options
+                // (std::path::Display and std::fmt::Display)
+                let d: &Display = &xs;
+                println!(\"{}\", d);
+            }
+        ")
+        .build();
+
+    let stderr = "\
+[CHECKING] foo v0.1.0 (CWD)
+error: Cannot replace slice of data that was already replaced
+error: Could not compile `foo`.
+
+To learn more, run the command again with --verbose.
+";
+
+    p.expect_cmd("cargo fix")
+        .stderr(stderr)
+        .status(101)
+        .run();
+}

src/diagnostics.rs

@@ -1,5 +1,5 @@
 //! Rustc Diagnostic JSON Output
-//! 
+//!
 //! The following data types are copied from [rust-lang/rust](https://github.com/rust-lang/rust/blob/de78655bca47cac8e783dbb563e7e5c25c1fae40/src/libsyntax/json.rs)
 
 #[derive(Deserialize, Debug)]
@@ -21,8 +21,8 @@ pub struct Diagnostic {
 #[derive(Deserialize, Debug)]
 pub struct DiagnosticSpan {
     pub file_name: String,
-    byte_start: u32,
-    byte_end: u32,
+    pub byte_start: u32,
+    pub byte_end: u32,
     /// 1-based.
     pub line_start: usize,
     pub line_end: usize,

src/lib.rs

@@ -1,11 +1,20 @@
 #[macro_use]
 extern crate serde_derive;
 extern crate serde_json;
+#[macro_use]
+extern crate failure;
+#[cfg(test)]
+#[macro_use]
+extern crate proptest;
 
 use std::collections::HashSet;
+use std::ops::Range;
+
+use failure::Error;
 
 pub mod diagnostics;
 use diagnostics::{Diagnostic, DiagnosticSpan};
+mod replace;
 
 pub fn get_suggestions_from_json<S: ::std::hash::BuildHasher>(
     input: &str,
@@ -61,6 +70,7 @@ pub struct Solution {
 pub struct Snippet {
     pub file_name: String,
     pub line_range: LineRange,
+    pub range: Range<usize>,
     /// leading surrounding text, text to replace, trailing surrounding text
     ///
     /// This split is useful for higlighting the part that gets replaced
@@ -109,6 +119,7 @@ fn parse_snippet(span: &DiagnosticSpan) -> Snippet {
                 column: span.column_end,
             },
         },
+        range: (span.byte_start as usize)..(span.byte_end as usize),
         text: (lead, body, tail),
     }
 }
@@ -166,58 +177,22 @@ pub fn collect_suggestions<S: ::std::hash::BuildHasher>(diagnostic: &Diagnostic,
     }
 }
 
-pub fn apply_suggestion(file_content: &mut String, suggestion: &Replacement) -> String {
-    use std::cmp::max;
-
-    let mut new_content = String::new();
-
-    // Add the lines before the section we want to replace
-    new_content.push_str(&file_content.lines()
-        .take(max(suggestion.snippet.line_range.start.line - 1, 0) as usize)
-        .collect::<Vec<_>>()
-        .join("\n"));
-    new_content.push_str("\n");
-
-    // Parts of line before replacement
-    new_content.push_str(&file_content.lines()
-        .nth(suggestion.snippet.line_range.start.line - 1)
-        .unwrap_or("")
-        .chars()
-        .take(suggestion.snippet.line_range.start.column - 1)
-        .collect::<String>());
-
-    // Insert new content! Finally!
-    new_content.push_str(&suggestion.replacement);
-
-    // Parts of line after replacement
-    new_content.push_str(&file_content.lines()
-        .nth(suggestion.snippet.line_range.end.line - 1)
-        .unwrap_or("")
-        .chars()
-        .skip(suggestion.snippet.line_range.end.column - 1)
-        .collect::<String>());
-
-    // Add the lines after the section we want to replace
-    new_content.push_str("\n");
-    new_content.push_str(&file_content.lines()
-        .skip(suggestion.snippet.line_range.end.line as usize)
-        .collect::<Vec<_>>()
-        .join("\n"));
-    new_content.push_str("\n");
-
-    new_content
-}
+pub fn apply_suggestions(code: &str, suggestions: &[Suggestion]) -> Result<String, Error> {
+    use replace::Data;
 
-pub fn apply_suggestions(code: &str, suggestions: &[Suggestion]) -> String {
-    let mut fixed = code.to_string();
+    let mut fixed = Data::new(code.as_bytes());
 
     for sug in suggestions.iter().rev() {
         for sol in &sug.solutions {
             for r in &sol.replacements {
-                fixed = apply_suggestion(&mut fixed, r);
+                fixed.replace_range(
+                    r.snippet.range.start,
+                    r.snippet.range.end.saturating_sub(1),
+                    r.replacement.as_bytes(),
+                )?;
             }
         }
     }
 
-    fixed
+    Ok(String::from_utf8(fixed.to_vec())?)
 }