Auto merge of #5140 - lzutao:cleanup-replace, r=flip1995

bors · bors · commit 4ad6fb3fb044 · 2020-02-07T22:41:04.000Z
Minor edit to text region replacement

changelog: none
diff --git a/clippy_dev/src/lib.rs b/clippy_dev/src/lib.rs
@@ -6,7 +6,6 @@ use regex::Regex;
 use std::collections::HashMap;
 use std::ffi::OsStr;
 use std::fs;
-use std::io::prelude::*;
 use std::path::{Path, PathBuf};
 use walkdir::WalkDir;
 
@@ -31,9 +30,10 @@ lazy_static! {
     )
     .unwrap();
     static ref NL_ESCAPE_RE: Regex = Regex::new(r#"\\\n\s*"#).unwrap();
-    pub static ref DOCS_LINK: String = "https://rust-lang.github.io/rust-clippy/master/index.html".to_string();
 }
 
+pub static DOCS_LINK: &str = "https://rust-lang.github.io/rust-clippy/master/index.html";
+
 /// Lint data parsed from the Clippy source code.
 #[derive(Clone, PartialEq, Debug)]
 pub struct Lint {
@@ -121,7 +121,7 @@ pub fn gen_changelog_lint_list(lints: Vec<Lint>) -> Vec<String> {
             if l.is_internal() {
                 None
             } else {
-                Some(format!("[`{}`]: {}#{}", l.name, DOCS_LINK.clone(), l.name))
+                Some(format!("[`{}`]: {}#{}", l.name, DOCS_LINK, l.name))
             }
         })
         .collect()
@@ -172,9 +172,7 @@ pub fn gather_all() -> impl Iterator<Item = Lint> {
 }
 
 fn gather_from_file(dir_entry: &walkdir::DirEntry) -> impl Iterator<Item = Lint> {
-    let mut file = fs::File::open(dir_entry.path()).unwrap();
-    let mut content = String::new();
-    file.read_to_string(&mut content).unwrap();
+    let content = fs::read_to_string(dir_entry.path()).unwrap();
     let mut filename = dir_entry.path().file_stem().unwrap().to_str().unwrap();
     // If the lints are stored in mod.rs, we get the module name from
     // the containing directory:
@@ -209,7 +207,7 @@ fn lint_files() -> impl Iterator<Item = walkdir::DirEntry> {
     let path = clippy_project_root().join("clippy_lints/src");
     WalkDir::new(path)
         .into_iter()
-        .filter_map(std::result::Result::ok)
+        .filter_map(Result::ok)
         .filter(|f| f.path().extension() == Some(OsStr::new("rs")))
 }
 
@@ -225,7 +223,6 @@ pub struct FileChange {
 /// `path` is the relative path to the file on which you want to perform the replacement.
 ///
 /// See `replace_region_in_text` for documentation of the other options.
-#[allow(clippy::expect_fun_call)]
 pub fn replace_region_in_file<F>(
     path: &Path,
     start: &str,
@@ -235,22 +232,15 @@ pub fn replace_region_in_file<F>(
     replacements: F,
 ) -> FileChange
 where
-    F: Fn() -> Vec<String>,
+    F: FnOnce() -> Vec<String>,
 {
-    let path = clippy_project_root().join(path);
-    let mut f = fs::File::open(&path).expect(&format!("File not found: {}", path.to_string_lossy()));
-    let mut contents = String::new();
-    f.read_to_string(&mut contents)
-        .expect("Something went wrong reading the file");
+    let contents = fs::read_to_string(path).unwrap_or_else(|e| panic!("Cannot read from {}: {}", path.display(), e));
     let file_change = replace_region_in_text(&contents, start, end, replace_start, replacements);
 
     if write_back {
-        let mut f = fs::File::create(&path).expect(&format!("File not found: {}", path.to_string_lossy()));
-        f.write_all(file_change.new_lines.as_bytes())
-            .expect("Unable to write file");
-        // Ensure we write the changes with a trailing newline so that
-        // the file has the proper line endings.
-        f.write_all(b"\n").expect("Unable to write file");
+        if let Err(e) = fs::write(path, file_change.new_lines.as_bytes()) {
+            panic!("Cannot write to {}: {}", path.display(), e);
+        }
     }
     file_change
 }
@@ -273,31 +263,32 @@ where
 ///
 /// ```
 /// let the_text = "replace_start\nsome text\nthat will be replaced\nreplace_end";
-/// let result = clippy_dev::replace_region_in_text(the_text, r#"replace_start"#, r#"replace_end"#, false, || {
-///     vec!["a different".to_string(), "text".to_string()]
-/// })
-/// .new_lines;
+/// let result =
+///     clippy_dev::replace_region_in_text(the_text, "replace_start", "replace_end", false, || {
+///         vec!["a different".to_string(), "text".to_string()]
+///     })
+///     .new_lines;
 /// assert_eq!("replace_start\na different\ntext\nreplace_end", result);
 /// ```
 pub fn replace_region_in_text<F>(text: &str, start: &str, end: &str, replace_start: bool, replacements: F) -> FileChange
 where
-    F: Fn() -> Vec<String>,
+    F: FnOnce() -> Vec<String>,
 {
-    let lines = text.lines();
+    let replace_it = replacements();
     let mut in_old_region = false;
     let mut found = false;
     let mut new_lines = vec![];
     let start = Regex::new(start).unwrap();
     let end = Regex::new(end).unwrap();
 
-    for line in lines.clone() {
+    for line in text.lines() {
         if in_old_region {
-            if end.is_match(&line) {
+            if end.is_match(line) {
                 in_old_region = false;
-                new_lines.extend(replacements());
+                new_lines.extend(replace_it.clone());
                 new_lines.push(line.to_string());
             }
-        } else if start.is_match(&line) {
+        } else if start.is_match(line) {
             if !replace_start {
                 new_lines.push(line.to_string());
             }
@@ -315,10 +306,12 @@ where
         eprintln!("error: regex `{:?}` not found. You may have to update it.", start);
     }
 
-    FileChange {
-        changed: lines.ne(new_lines.clone()),
-        new_lines: new_lines.join("\n"),
+    let mut new_lines = new_lines.join("\n");
+    if text.ends_with('\n') {
+        new_lines.push('\n');
     }
+    let changed = new_lines != text;
+    FileChange { changed, new_lines }
 }
 
 /// Returns the path to the Clippy project directory
diff --git a/clippy_dev/src/main.rs b/clippy_dev/src/main.rs
@@ -8,7 +8,7 @@ mod fmt;
 mod new_lint;
 mod stderr_length_check;
 
-#[derive(PartialEq)]
+#[derive(Clone, Copy, PartialEq)]
 enum UpdateMode {
     Check,
     Change,
@@ -113,9 +113,9 @@ fn main() {
             if matches.is_present("print-only") {
                 print_lints();
             } else if matches.is_present("check") {
-                update_lints(&UpdateMode::Check);
+                update_lints(UpdateMode::Check);
             } else {
-                update_lints(&UpdateMode::Change);
+                update_lints(UpdateMode::Change);
             }
         },
         ("new_lint", Some(matches)) => {
@@ -124,7 +124,7 @@ fn main() {
                 matches.value_of("name"),
                 matches.value_of("category"),
             ) {
-                Ok(_) => update_lints(&UpdateMode::Change),
+                Ok(_) => update_lints(UpdateMode::Change),
                 Err(e) => eprintln!("Unable to create lint: {}", e),
             }
         },
@@ -150,7 +150,7 @@ fn print_lints() {
             println!(
                 "* [{}]({}#{}) ({})",
                 lint.name,
-                clippy_dev::DOCS_LINK.clone(),
+                clippy_dev::DOCS_LINK,
                 lint.name,
                 lint.desc
             );
@@ -161,7 +161,7 @@ fn print_lints() {
 }
 
 #[allow(clippy::too_many_lines)]
-fn update_lints(update_mode: &UpdateMode) {
+fn update_lints(update_mode: UpdateMode) {
     let lint_list: Vec<Lint> = gather_all().collect();
 
     let usable_lints: Vec<Lint> = Lint::usable_lints(lint_list.clone().into_iter()).collect();
@@ -175,7 +175,7 @@ fn update_lints(update_mode: &UpdateMode) {
         "begin lint list",
         "end lint list",
         false,
-        update_mode == &UpdateMode::Change,
+        update_mode == UpdateMode::Change,
         || {
             format!(
                 "pub const ALL_LINTS: [Lint; {}] = {:#?};",
@@ -191,23 +191,25 @@ fn update_lints(update_mode: &UpdateMode) {
 
     file_change |= replace_region_in_file(
         Path::new("README.md"),
-        r#"\[There are \d+ lints included in this crate!\]\(https://rust-lang.github.io/rust-clippy/master/index.html\)"#,
+        &format!(r#"\[There are \d+ lints included in this crate!\]\({}\)"#, DOCS_LINK),
         "",
         true,
-        update_mode == &UpdateMode::Change,
+        update_mode == UpdateMode::Change,
         || {
-            vec![
-                format!("[There are {} lints included in this crate!](https://rust-lang.github.io/rust-clippy/master/index.html)", lint_count)
-            ]
-        }
-    ).changed;
+            vec![format!(
+                "[There are {} lints included in this crate!]({})",
+                lint_count, DOCS_LINK
+            )]
+        },
+    )
+    .changed;
 
     file_change |= replace_region_in_file(
         Path::new("CHANGELOG.md"),
         "<!-- begin autogenerated links to lint list -->",
         "<!-- end autogenerated links to lint list -->",
         false,
-        update_mode == &UpdateMode::Change,
+        update_mode == UpdateMode::Change,
         || gen_changelog_lint_list(lint_list.clone()),
     )
     .changed;
@@ -217,7 +219,7 @@ fn update_lints(update_mode: &UpdateMode) {
         "begin deprecated lints",
         "end deprecated lints",
         false,
-        update_mode == &UpdateMode::Change,
+        update_mode == UpdateMode::Change,
         || gen_deprecated(&lint_list),
     )
     .changed;
@@ -227,7 +229,7 @@ fn update_lints(update_mode: &UpdateMode) {
         "begin register lints",
         "end register lints",
         false,
-        update_mode == &UpdateMode::Change,
+        update_mode == UpdateMode::Change,
         || gen_register_lint_list(&lint_list),
     )
     .changed;
@@ -237,7 +239,7 @@ fn update_lints(update_mode: &UpdateMode) {
         "begin lints modules",
         "end lints modules",
         false,
-        update_mode == &UpdateMode::Change,
+        update_mode == UpdateMode::Change,
         || gen_modules_list(lint_list.clone()),
     )
     .changed;
@@ -248,7 +250,7 @@ fn update_lints(update_mode: &UpdateMode) {
         r#"store.register_group\(true, "clippy::all""#,
         r#"\]\);"#,
         false,
-        update_mode == &UpdateMode::Change,
+        update_mode == UpdateMode::Change,
         || {
             // clippy::all should only include the following lint groups:
             let all_group_lints = usable_lints
@@ -271,13 +273,13 @@ fn update_lints(update_mode: &UpdateMode) {
             &format!("store.register_group\\(true, \"clippy::{}\"", lint_group),
             r#"\]\);"#,
             false,
-            update_mode == &UpdateMode::Change,
+            update_mode == UpdateMode::Change,
             || gen_lint_group_list(lints.clone()),
         )
         .changed;
     }
 
-    if update_mode == &UpdateMode::Check && file_change {
+    if update_mode == UpdateMode::Check && file_change {
         println!(
             "Not all lints defined properly. \
              Please run `cargo dev update_lints` to make sure all lints are defined properly."
