Auto merge of #5438 - flip1995:rollup-pi762oy, r=flip1995

Rollup of 11 pull requests

Successful merges:

 - #5406 (Fix update_lints)
 - #5409 (Downgrade let_unit_value to pedantic)
 - #5410 (Downgrade trivially_copy_pass_by_ref to pedantic)
 - #5412 (Downgrade inefficient_to_string to pedantic)
 - #5415 (Add new lint for `Result<T, E>.map_or(None, Some(T))`)
 - #5417 (Update doc links and mentioned names in docs)
 - #5419 (Downgrade unreadable_literal to pedantic)
 - #5420 (Downgrade new_ret_no_self to pedantic)
 - #5422 (CONTRIBUTING.md: fix broken triage link)
 - #5424 (Incorrect suspicious_op_assign_impl)
 - #5425 (Ehance opt_as_ref_deref lint.)

Failed merges:

 - #5345 (Add lint for float in array comparison)
 - #5411 (Downgrade implicit_hasher to pedantic)
 - #5428 (Move cognitive_complexity to nursery)

r? @ghost

changelog: rollup

CHANGELOG.md

@@ -1448,6 +1448,7 @@ Released 2018-09-13
 [`replace_consts`]: https://rust-lang.github.io/rust-clippy/master/index.html#replace_consts
 [`rest_pat_in_fully_bound_structs`]: https://rust-lang.github.io/rust-clippy/master/index.html#rest_pat_in_fully_bound_structs
 [`result_expect_used`]: https://rust-lang.github.io/rust-clippy/master/index.html#result_expect_used
+[`result_map_or_into_option`]: https://rust-lang.github.io/rust-clippy/master/index.html#result_map_or_into_option
 [`result_map_unit_fn`]: https://rust-lang.github.io/rust-clippy/master/index.html#result_map_unit_fn
 [`result_map_unwrap_or_else`]: https://rust-lang.github.io/rust-clippy/master/index.html#result_map_unwrap_or_else
 [`result_unwrap_used`]: https://rust-lang.github.io/rust-clippy/master/index.html#result_unwrap_used

CONTRIBUTING.md

@@ -36,7 +36,8 @@ High level approach:
 
 ### Finding something to fix/improve
 
-All issues on Clippy are mentored, if you want help with a bug just ask @Manishearth, @llogiq, @mcarton or @oli-obk.
+All issues on Clippy are mentored, if you want help with a bug just ask
+@Manishearth, @flip1995, @phansch or @yaahc.
 
 Some issues are easier than others. The [`good first issue`] label can be used to find the easy issues.
 If you want to work on an issue, please leave a comment so that we can assign it to you!
@@ -70,24 +71,22 @@ an AST expression). `match_def_path()` in Clippy's `utils` module can also be us
 [`T-AST`]: https://github.com/rust-lang/rust-clippy/labels/T-AST
 [`T-middle`]: https://github.com/rust-lang/rust-clippy/labels/T-middle
 [`E-medium`]: https://github.com/rust-lang/rust-clippy/labels/E-medium
-[`ty`]: https://doc.rust-lang.org/nightly/nightly-rustc/rustc/ty
+[`ty`]: https://doc.rust-lang.org/nightly/nightly-rustc/rustc_middle/ty
 [nodes in the AST docs]: https://doc.rust-lang.org/nightly/nightly-rustc/rustc_ast/ast/
 [deep-nesting]: https://github.com/rust-lang/rust-clippy/blob/557f6848bd5b7183f55c1e1522a326e9e1df6030/clippy_lints/src/mem_forget.rs#L29-L43
 [if_chain]: https://docs.rs/if_chain/*/if_chain
 [nest-less]: https://github.com/rust-lang/rust-clippy/blob/557f6848bd5b7183f55c1e1522a326e9e1df6030/clippy_lints/src/bit_mask.rs#L124-L150
 
 ## Writing code
 
-Have a look at the [docs for writing lints][adding_lints] for more details. [Llogiq's blog post on lints]
-is also a nice primer to lint-writing, though it does get into advanced stuff and may be a bit outdated.
+Have a look at the [docs for writing lints][adding_lints] for more details.
 
 If you want to add a new lint or change existing ones apart from bugfixing, it's
 also a good idea to give the [stability guarantees][rfc_stability] and
 [lint categories][rfc_lint_cats] sections of the [Clippy 1.0 RFC][clippy_rfc] a
 quick read.
 
 [adding_lints]: https://github.com/rust-lang/rust-clippy/blob/master/doc/adding_lints.md
-[Llogiq's blog post on lints]: https://llogiq.github.io/2015/06/04/workflows.html
 [clippy_rfc]: https://github.com/rust-lang/rfcs/blob/master/text/2476-clippy-uno.md
 [rfc_stability]: https://github.com/rust-lang/rfcs/blob/master/text/2476-clippy-uno.md#stability-guarantees
 [rfc_lint_cats]: https://github.com/rust-lang/rfcs/blob/master/text/2476-clippy-uno.md#lint-audit-and-categories
@@ -223,7 +222,7 @@ You can find the Clippy bors queue [here][homu_queue].
 If you have @bors permissions, you can find an overview of the available
 commands [here][homu_instructions].
 
-[triage]: https://forge.rust-lang.org/triage-procedure.html
+[triage]: https://forge.rust-lang.org/release/triage-procedure.html
 [l-crash]: https://github.com/rust-lang/rust-clippy/labels/L-crash%20%3Aboom%3A
 [l-bug]: https://github.com/rust-lang/rust-clippy/labels/L-bug%20%3Abeetle%3A
 [homu]: https://github.com/rust-lang/homu

clippy_dev/src/lib.rs

@@ -62,110 +62,89 @@ impl Lint {
     }
 
     /// Returns all non-deprecated lints and non-internal lints
-    pub fn usable_lints(lints: impl Iterator<Item = Self>) -> impl Iterator<Item = Self> {
-        lints.filter(|l| l.deprecation.is_none() && !l.is_internal())
+    #[must_use]
+    pub fn usable_lints(lints: &[Self]) -> Vec<Self> {
+        lints
+            .iter()
+            .filter(|l| l.deprecation.is_none() && !l.group.starts_with("internal"))
+            .cloned()
+            .collect()
     }
 
     /// Returns all internal lints (not `internal_warn` lints)
-    pub fn internal_lints(lints: impl Iterator<Item = Self>) -> impl Iterator<Item = Self> {
-        lints.filter(|l| l.group == "internal")
+    #[must_use]
+    pub fn internal_lints(lints: &[Self]) -> Vec<Self> {
+        lints.iter().filter(|l| l.group == "internal").cloned().collect()
     }
 
-    /// Returns the lints in a `HashMap`, grouped by the different lint groups
+    /// Returns all deprecated lints
     #[must_use]
-    pub fn by_lint_group(lints: impl Iterator<Item = Self>) -> HashMap<String, Vec<Self>> {
-        lints.map(|lint| (lint.group.to_string(), lint)).into_group_map()
+    pub fn deprecated_lints(lints: &[Self]) -> Vec<Self> {
+        lints.iter().filter(|l| l.deprecation.is_some()).cloned().collect()
     }
 
+    /// Returns the lints in a `HashMap`, grouped by the different lint groups
     #[must_use]
-    pub fn is_internal(&self) -> bool {
-        self.group.starts_with("internal")
+    pub fn by_lint_group(lints: impl Iterator<Item = Self>) -> HashMap<String, Vec<Self>> {
+        lints.map(|lint| (lint.group.to_string(), lint)).into_group_map()
     }
 }
 
 /// Generates the Vec items for `register_lint_group` calls in `clippy_lints/src/lib.rs`.
 #[must_use]
-pub fn gen_lint_group_list(lints: Vec<Lint>) -> Vec<String> {
+pub fn gen_lint_group_list<'a>(lints: impl Iterator<Item = &'a Lint>) -> Vec<String> {
     lints
-        .into_iter()
-        .filter_map(|l| {
-            if l.deprecation.is_some() {
-                None
-            } else {
-                Some(format!("        LintId::of(&{}::{}),", l.module, l.name.to_uppercase()))
-            }
-        })
+        .map(|l| format!("        LintId::of(&{}::{}),", l.module, l.name.to_uppercase()))
         .sorted()
         .collect::<Vec<String>>()
 }
 
 /// Generates the `pub mod module_name` list in `clippy_lints/src/lib.rs`.
 #[must_use]
-pub fn gen_modules_list(lints: Vec<Lint>) -> Vec<String> {
+pub fn gen_modules_list<'a>(lints: impl Iterator<Item = &'a Lint>) -> Vec<String> {
     lints
-        .into_iter()
-        .filter_map(|l| {
-            if l.is_internal() || l.deprecation.is_some() {
-                None
-            } else {
-                Some(l.module)
-            }
-        })
+        .map(|l| &l.module)
         .unique()
-        .map(|module| format!("pub mod {};", module))
+        .map(|module| format!("mod {};", module))
         .sorted()
         .collect::<Vec<String>>()
 }
 
 /// Generates the list of lint links at the bottom of the README
 #[must_use]
-pub fn gen_changelog_lint_list(lints: Vec<Lint>) -> Vec<String> {
-    let mut lint_list_sorted: Vec<Lint> = lints;
-    lint_list_sorted.sort_by_key(|l| l.name.clone());
-    lint_list_sorted
-        .iter()
-        .filter_map(|l| {
-            if l.is_internal() {
-                None
-            } else {
-                Some(format!("[`{}`]: {}#{}", l.name, DOCS_LINK, l.name))
-            }
-        })
+pub fn gen_changelog_lint_list<'a>(lints: impl Iterator<Item = &'a Lint>) -> Vec<String> {
+    lints
+        .sorted_by_key(|l| &l.name)
+        .map(|l| format!("[`{}`]: {}#{}", l.name, DOCS_LINK, l.name))
         .collect()
 }
 
 /// Generates the `register_removed` code in `./clippy_lints/src/lib.rs`.
 #[must_use]
-pub fn gen_deprecated(lints: &[Lint]) -> Vec<String> {
+pub fn gen_deprecated<'a>(lints: impl Iterator<Item = &'a Lint>) -> Vec<String> {
     lints
-        .iter()
-        .filter_map(|l| {
-            l.clone().deprecation.map(|depr_text| {
-                vec![
-                    "    store.register_removed(".to_string(),
-                    format!("        \"clippy::{}\",", l.name),
-                    format!("        \"{}\",", depr_text),
-                    "    );".to_string(),
-                ]
-            })
+        .flat_map(|l| {
+            l.deprecation
+                .clone()
+                .map(|depr_text| {
+                    vec![
+                        "    store.register_removed(".to_string(),
+                        format!("        \"clippy::{}\",", l.name),
+                        format!("        \"{}\",", depr_text),
+                        "    );".to_string(),
+                    ]
+                })
+                .expect("only deprecated lints should be passed")
         })
-        .flatten()
         .collect::<Vec<String>>()
 }
 
 #[must_use]
-pub fn gen_register_lint_list(lints: &[Lint]) -> Vec<String> {
+pub fn gen_register_lint_list<'a>(lints: impl Iterator<Item = &'a Lint>) -> Vec<String> {
     let pre = "    store.register_lints(&[".to_string();
     let post = "    ]);".to_string();
     let mut inner = lints
-        .iter()
-        .filter_map(|l| {
-            if !l.is_internal() && l.deprecation.is_none() {
-                Some(format!("        &{}::{},", l.module, l.name.to_uppercase()))
-            } else {
-                None
-            }
-        })
+        .map(|l| format!("        &{}::{},", l.module, l.name.to_uppercase()))
         .sorted()
         .collect::<Vec<String>>();
     inner.insert(0, pre);
@@ -439,7 +418,7 @@ fn test_usable_lints() {
         None,
         "module_name",
     )];
-    assert_eq!(expected, Lint::usable_lints(lints.into_iter()).collect::<Vec<Lint>>());
+    assert_eq!(expected, Lint::usable_lints(&lints));
 }
 
 #[test]
@@ -469,13 +448,12 @@ fn test_gen_changelog_lint_list() {
     let lints = vec![
         Lint::new("should_assert_eq", "group1", "abc", None, "module_name"),
         Lint::new("should_assert_eq2", "group2", "abc", None, "module_name"),
-        Lint::new("incorrect_internal", "internal_style", "abc", None, "module_name"),
     ];
     let expected = vec![
         format!("[`should_assert_eq`]: {}#should_assert_eq", DOCS_LINK.to_string()),
         format!("[`should_assert_eq2`]: {}#should_assert_eq2", DOCS_LINK.to_string()),
     ];
-    assert_eq!(expected, gen_changelog_lint_list(lints));
+    assert_eq!(expected, gen_changelog_lint_list(lints.iter()));
 }
 
 #[test]
@@ -495,7 +473,6 @@ fn test_gen_deprecated() {
             Some("will be removed"),
             "module_name",
         ),
-        Lint::new("should_assert_eq2", "group2", "abc", None, "module_name"),
     ];
     let expected: Vec<String> = vec![
         "    store.register_removed(",
@@ -510,36 +487,37 @@ fn test_gen_deprecated() {
     .into_iter()
     .map(String::from)
     .collect();
-    assert_eq!(expected, gen_deprecated(&lints));
+    assert_eq!(expected, gen_deprecated(lints.iter()));
+}
+
+#[test]
+#[should_panic]
+fn test_gen_deprecated_fail() {
+    let lints = vec![Lint::new("should_assert_eq2", "group2", "abc", None, "module_name")];
+    let _ = gen_deprecated(lints.iter());
 }
 
 #[test]
 fn test_gen_modules_list() {
     let lints = vec![
         Lint::new("should_assert_eq", "group1", "abc", None, "module_name"),
-        Lint::new("should_assert_eq2", "group2", "abc", Some("abc"), "deprecated"),
         Lint::new("incorrect_stuff", "group3", "abc", None, "another_module"),
-        Lint::new("incorrect_internal", "internal_style", "abc", None, "module_name"),
-    ];
-    let expected = vec![
-        "pub mod another_module;".to_string(),
-        "pub mod module_name;".to_string(),
     ];
-    assert_eq!(expected, gen_modules_list(lints));
+    let expected = vec!["mod another_module;".to_string(), "mod module_name;".to_string()];
+    assert_eq!(expected, gen_modules_list(lints.iter()));
 }
 
 #[test]
 fn test_gen_lint_group_list() {
     let lints = vec![
         Lint::new("abc", "group1", "abc", None, "module_name"),
         Lint::new("should_assert_eq", "group1", "abc", None, "module_name"),
-        Lint::new("should_assert_eq2", "group2", "abc", Some("abc"), "deprecated"),
         Lint::new("internal", "internal_style", "abc", None, "module_name"),
     ];
     let expected = vec![
         "        LintId::of(&module_name::ABC),".to_string(),
         "        LintId::of(&module_name::INTERNAL),".to_string(),
         "        LintId::of(&module_name::SHOULD_ASSERT_EQ),".to_string(),
     ];
-    assert_eq!(expected, gen_lint_group_list(lints));
+    assert_eq!(expected, gen_lint_group_list(lints.iter()));
 }

clippy_dev/src/update_lints.rs

@@ -14,14 +14,14 @@ pub enum UpdateMode {
 pub fn run(update_mode: UpdateMode) {
     let lint_list: Vec<Lint> = gather_all().collect();
 
-    let internal_lints = Lint::internal_lints(lint_list.clone().into_iter());
-
-    let usable_lints: Vec<Lint> = Lint::usable_lints(lint_list.clone().into_iter()).collect();
-    let usable_lint_count = round_to_fifty(usable_lints.len());
-
+    let internal_lints = Lint::internal_lints(&lint_list);
+    let deprecated_lints = Lint::deprecated_lints(&lint_list);
+    let usable_lints = Lint::usable_lints(&lint_list);
     let mut sorted_usable_lints = usable_lints.clone();
     sorted_usable_lints.sort_by_key(|lint| lint.name.clone());
 
+    let usable_lint_count = round_to_fifty(usable_lints.len());
+
     let mut file_change = replace_region_in_file(
         Path::new("src/lintlist/mod.rs"),
         "begin lint list",
@@ -61,7 +61,7 @@ pub fn run(update_mode: UpdateMode) {
         "<!-- end autogenerated links to lint list -->",
         false,
         update_mode == UpdateMode::Change,
-        || gen_changelog_lint_list(lint_list.clone()),
+        || gen_changelog_lint_list(usable_lints.iter().chain(deprecated_lints.iter())),
     )
     .changed;
 
@@ -71,7 +71,7 @@ pub fn run(update_mode: UpdateMode) {
         "end deprecated lints",
         false,
         update_mode == UpdateMode::Change,
-        || gen_deprecated(&lint_list),
+        || gen_deprecated(deprecated_lints.iter()),
     )
     .changed;
 
@@ -81,7 +81,7 @@ pub fn run(update_mode: UpdateMode) {
         "end register lints",
         false,
         update_mode == UpdateMode::Change,
-        || gen_register_lint_list(&lint_list),
+        || gen_register_lint_list(usable_lints.iter().chain(internal_lints.iter())),
     )
     .changed;
 
@@ -91,7 +91,7 @@ pub fn run(update_mode: UpdateMode) {
         "end lints modules",
         false,
         update_mode == UpdateMode::Change,
-        || gen_modules_list(lint_list.clone()),
+        || gen_modules_list(usable_lints.iter()),
     )
     .changed;
 
@@ -104,13 +104,9 @@ pub fn run(update_mode: UpdateMode) {
         update_mode == UpdateMode::Change,
         || {
             // clippy::all should only include the following lint groups:
-            let all_group_lints = usable_lints
-                .clone()
-                .into_iter()
-                .filter(|l| {
-                    l.group == "correctness" || l.group == "style" || l.group == "complexity" || l.group == "perf"
-                })
-                .collect();
+            let all_group_lints = usable_lints.iter().filter(|l| {
+                l.group == "correctness" || l.group == "style" || l.group == "complexity" || l.group == "perf"
+            });
 
             gen_lint_group_list(all_group_lints)
         },
@@ -125,7 +121,7 @@ pub fn run(update_mode: UpdateMode) {
             r#"\]\);"#,
             false,
             update_mode == UpdateMode::Change,
-            || gen_lint_group_list(lints.clone()),
+            || gen_lint_group_list(lints.iter()),
         )
         .changed;
     }
@@ -140,8 +136,8 @@ pub fn run(update_mode: UpdateMode) {
 }
 
 pub fn print_lints() {
-    let lint_list = gather_all();
-    let usable_lints: Vec<Lint> = Lint::usable_lints(lint_list).collect();
+    let lint_list: Vec<Lint> = gather_all().collect();
+    let usable_lints = Lint::usable_lints(&lint_list);
     let usable_lint_count = usable_lints.len();
     let grouped_by_lint_group = Lint::by_lint_group(usable_lints.into_iter());