Update clippy

Author: tesuji

CHANGELOG.md

@@ -1050,12 +1050,14 @@ Released 2018-09-13
 [`mem_discriminant_non_enum`]: https://rust-lang.github.io/rust-clippy/master/index.html#mem_discriminant_non_enum
 [`mem_forget`]: https://rust-lang.github.io/rust-clippy/master/index.html#mem_forget
 [`mem_replace_option_with_none`]: https://rust-lang.github.io/rust-clippy/master/index.html#mem_replace_option_with_none
+[`mem_replace_with_uninit`]: https://rust-lang.github.io/rust-clippy/master/index.html#mem_replace_with_uninit
 [`min_max`]: https://rust-lang.github.io/rust-clippy/master/index.html#min_max
 [`misaligned_transmute`]: https://rust-lang.github.io/rust-clippy/master/index.html#misaligned_transmute
 [`misrefactored_assign_op`]: https://rust-lang.github.io/rust-clippy/master/index.html#misrefactored_assign_op
 [`missing_const_for_fn`]: https://rust-lang.github.io/rust-clippy/master/index.html#missing_const_for_fn
 [`missing_docs_in_private_items`]: https://rust-lang.github.io/rust-clippy/master/index.html#missing_docs_in_private_items
 [`missing_inline_in_public_items`]: https://rust-lang.github.io/rust-clippy/master/index.html#missing_inline_in_public_items
+[`missing_safety_doc`]: https://rust-lang.github.io/rust-clippy/master/index.html#missing_safety_doc
 [`mistyped_literal_suffixes`]: https://rust-lang.github.io/rust-clippy/master/index.html#mistyped_literal_suffixes
 [`mixed_case_hex_literals`]: https://rust-lang.github.io/rust-clippy/master/index.html#mixed_case_hex_literals
 [`module_inception`]: https://rust-lang.github.io/rust-clippy/master/index.html#module_inception

README.md

@@ -6,7 +6,7 @@
 
 A collection of lints to catch common mistakes and improve your [Rust](https://github.com/rust-lang/rust) code.
 
-[There are 313 lints included in this crate!](https://rust-lang.github.io/rust-clippy/master/index.html)
+[There are 315 lints included in this crate!](https://rust-lang.github.io/rust-clippy/master/index.html)
 
 We have a bunch of lint categories to allow you to choose how much Clippy is supposed to ~~annoy~~ help you:
 

clippy_lints/Cargo.toml

@@ -27,7 +27,7 @@ semver = "0.9.0"
 serde = { version = "1.0", features = ["derive"] }
 toml = "0.5.3"
 unicode-normalization = "0.1"
-pulldown-cmark = "0.5.3"
+pulldown-cmark = "0.6.0"
 url = { version =  "2.1.0", features = ["serde"] } # cargo requires serde feat in its url dep
 # see https://github.com/rust-lang/rust/pull/63587#issuecomment-522343864
 if_chain = "1.0.0"

clippy_lints/src/assign_ops.rs

@@ -216,13 +216,13 @@ fn lint_misrefactored_assign_op(
                         long
                     ),
                     format!("{} {}= {}", snip_a, op.node.as_str(), snip_r),
-                    Applicability::MachineApplicable,
+                    Applicability::MaybeIncorrect,
                 );
                 db.span_suggestion(
                     expr.span,
                     "or",
                     long,
-                    Applicability::MachineApplicable, // snippet
+                    Applicability::MaybeIncorrect, // snippet
                 );
             }
         },

clippy_lints/src/block_in_if_condition.rs

@@ -2,7 +2,7 @@ use crate::utils::*;
 use matches::matches;
 use rustc::hir::intravisit::{walk_expr, NestedVisitorMap, Visitor};
 use rustc::hir::*;
-use rustc::lint::{LateContext, LateLintPass, LintArray, LintPass};
+use rustc::lint::{in_external_macro, LateContext, LateLintPass, LintArray, LintContext, LintPass};
 use rustc::{declare_lint_pass, declare_tool_lint};
 
 declare_clippy_lint! {
@@ -72,6 +72,9 @@ const COMPLEX_BLOCK_MESSAGE: &str = "in an 'if' condition, avoid complex blocks
 
 impl<'a, 'tcx> LateLintPass<'a, 'tcx> for BlockInIfCondition {
     fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr) {
+        if in_external_macro(cx.sess(), expr.span) {
+            return;
+        }
         if let Some((check, then, _)) = higher::if_block(&expr) {
             if let ExprKind::Block(block, _) = &check.node {
                 if block.rules == DefaultBlock {

clippy_lints/src/cargo_common_metadata.rs

@@ -1,5 +1,7 @@
 //! lint on missing cargo common metadata
 
+use std::path::PathBuf;
+
 use crate::utils::span_lint;
 use rustc::lint::{EarlyContext, EarlyLintPass, LintArray, LintPass};
 use rustc::{declare_lint_pass, declare_tool_lint};
@@ -47,6 +49,10 @@ fn is_empty_str(value: &Option<String>) -> bool {
     value.as_ref().map_or(true, String::is_empty)
 }
 
+fn is_empty_path(value: &Option<PathBuf>) -> bool {
+    value.as_ref().and_then(|x| x.to_str()).map_or(true, str::is_empty)
+}
+
 fn is_empty_vec(value: &[String]) -> bool {
     // This works because empty iterators return true
     value.iter().all(std::string::String::is_empty)
@@ -72,8 +78,8 @@ impl EarlyLintPass for CargoCommonMetadata {
                 missing_warning(cx, &package, "package.description");
             }
 
-            if is_empty_str(&package.license) {
-                missing_warning(cx, &package, "package.license");
+            if is_empty_str(&package.license) && is_empty_path(&package.license_file) {
+                missing_warning(cx, &package, "either package.license or package.license_file");
             }
 
             if is_empty_str(&package.repository) {

clippy_lints/src/doc.rs

@@ -34,6 +34,40 @@ declare_clippy_lint! {
     "presence of `_`, `::` or camel-case outside backticks in documentation"
 }
 
+declare_clippy_lint! {
+    /// **What it does:** Checks for the doc comments of publicly visible
+    /// unsafe functions and warns if there is no `# Safety` section.
+    ///
+    /// **Why is this bad?** Unsafe functions should document their safety
+    /// preconditions, so that users can be sure they are using them safely.
+    ///
+    /// **Known problems:** None.
+    ///
+    /// **Examples**:
+    /// ```rust
+    ///# type Universe = ();
+    /// /// This function should really be documented
+    /// pub unsafe fn start_apocalypse(u: &mut Universe) {
+    ///     unimplemented!();
+    /// }
+    /// ```
+    ///
+    /// At least write a line about safety:
+    ///
+    /// ```rust
+    ///# type Universe = ();
+    /// /// # Safety
+    /// ///
+    /// /// This function should not be called before the horsemen are ready.
+    /// pub unsafe fn start_apocalypse(u: &mut Universe) {
+    ///     unimplemented!();
+    /// }
+    /// ```
+    pub MISSING_SAFETY_DOC,
+    style,
+    "`pub unsafe fn` without `# Safety` docs"
+}
+
 #[allow(clippy::module_name_repetitions)]
 #[derive(Clone)]
 pub struct DocMarkdown {
@@ -46,15 +80,28 @@ impl DocMarkdown {
     }
 }
 
-impl_lint_pass!(DocMarkdown => [DOC_MARKDOWN]);
+impl_lint_pass!(DocMarkdown => [DOC_MARKDOWN, MISSING_SAFETY_DOC]);
 
 impl EarlyLintPass for DocMarkdown {
     fn check_crate(&mut self, cx: &EarlyContext<'_>, krate: &ast::Crate) {
         check_attrs(cx, &self.valid_idents, &krate.attrs);
     }
 
     fn check_item(&mut self, cx: &EarlyContext<'_>, item: &ast::Item) {
-        check_attrs(cx, &self.valid_idents, &item.attrs);
+        if check_attrs(cx, &self.valid_idents, &item.attrs) {
+            return;
+        }
+        // no safety header
+        if let ast::ItemKind::Fn(_, ref header, ..) = item.node {
+            if item.vis.node.is_pub() && header.unsafety == ast::Unsafety::Unsafe {
+                span_lint(
+                    cx,
+                    MISSING_SAFETY_DOC,
+                    item.span,
+                    "unsafe function's docs miss `# Safety` section",
+                );
+            }
+        }
     }
 }
 
@@ -115,7 +162,7 @@ pub fn strip_doc_comment_decoration(comment: &str, span: Span) -> (String, Vec<(
     panic!("not a doc-comment: {}", comment);
 }
 
-pub fn check_attrs<'a>(cx: &EarlyContext<'_>, valid_idents: &FxHashSet<String>, attrs: &'a [ast::Attribute]) {
+pub fn check_attrs<'a>(cx: &EarlyContext<'_>, valid_idents: &FxHashSet<String>, attrs: &'a [ast::Attribute]) -> bool {
     let mut doc = String::new();
     let mut spans = vec![];
 
@@ -129,7 +176,7 @@ pub fn check_attrs<'a>(cx: &EarlyContext<'_>, valid_idents: &FxHashSet<String>,
             }
         } else if attr.check_name(sym!(doc)) {
             // ignore mix of sugared and non-sugared doc
-            return;
+            return true; // don't trigger the safety check
         }
     }
 
@@ -140,57 +187,64 @@ pub fn check_attrs<'a>(cx: &EarlyContext<'_>, valid_idents: &FxHashSet<String>,
         current += offset_copy;
     }
 
-    if !doc.is_empty() {
-        let parser = pulldown_cmark::Parser::new(&doc).into_offset_iter();
-        // Iterate over all `Events` and combine consecutive events into one
-        let events = parser.coalesce(|previous, current| {
-            use pulldown_cmark::Event::*;
-
-            let previous_range = previous.1;
-            let current_range = current.1;
-
-            match (previous.0, current.0) {
-                (Text(previous), Text(current)) => {
-                    let mut previous = previous.to_string();
-                    previous.push_str(&current);
-                    Ok((Text(previous.into()), previous_range))
-                },
-                (previous, current) => Err(((previous, previous_range), (current, current_range))),
-            }
-        });
-        check_doc(cx, valid_idents, events, &spans);
+    if doc.is_empty() {
+        return false;
     }
+
+    let parser = pulldown_cmark::Parser::new(&doc).into_offset_iter();
+    // Iterate over all `Events` and combine consecutive events into one
+    let events = parser.coalesce(|previous, current| {
+        use pulldown_cmark::Event::*;
+
+        let previous_range = previous.1;
+        let current_range = current.1;
+
+        match (previous.0, current.0) {
+            (Text(previous), Text(current)) => {
+                let mut previous = previous.to_string();
+                previous.push_str(&current);
+                Ok((Text(previous.into()), previous_range))
+            },
+            (previous, current) => Err(((previous, previous_range), (current, current_range))),
+        }
+    });
+    check_doc(cx, valid_idents, events, &spans)
 }
 
 fn check_doc<'a, Events: Iterator<Item = (pulldown_cmark::Event<'a>, Range<usize>)>>(
     cx: &EarlyContext<'_>,
     valid_idents: &FxHashSet<String>,
     events: Events,
     spans: &[(usize, Span)],
-) {
+) -> bool {
+    // true if a safety header was found
     use pulldown_cmark::Event::*;
     use pulldown_cmark::Tag::*;
 
+    let mut safety_header = false;
     let mut in_code = false;
     let mut in_link = None;
+    let mut in_heading = false;
 
     for (event, range) in events {
         match event {
             Start(CodeBlock(_)) => in_code = true,
             End(CodeBlock(_)) => in_code = false,
             Start(Link(_, url, _)) => in_link = Some(url),
             End(Link(..)) => in_link = None,
-            Start(_tag) | End(_tag) => (),         // We don't care about other tags
-            Html(_html) | InlineHtml(_html) => (), // HTML is weird, just ignore it
-            SoftBreak | HardBreak | TaskListMarker(_) | Code(_) => (),
+            Start(Heading(_)) => in_heading = true,
+            End(Heading(_)) => in_heading = false,
+            Start(_tag) | End(_tag) => (), // We don't care about other tags
+            Html(_html) => (),             // HTML is weird, just ignore it
+            SoftBreak | HardBreak | TaskListMarker(_) | Code(_) | Rule => (),
             FootnoteReference(text) | Text(text) => {
                 if Some(&text) == in_link.as_ref() {
                     // Probably a link of the form `<http://example.com>`
                     // Which are represented as a link to "http://example.com" with
                     // text "http://example.com" by pulldown-cmark
                     continue;
                 }
-
+                safety_header |= in_heading && text.trim() == "Safety";
                 if !in_code {
                     let index = match spans.binary_search_by(|c| c.0.cmp(&range.start)) {
                         Ok(o) => o,
@@ -207,6 +261,7 @@ fn check_doc<'a, Events: Iterator<Item = (pulldown_cmark::Event<'a>, Range<usize
             },
         }
     }
+    safety_header
 }
 
 fn check_text(cx: &EarlyContext<'_>, valid_idents: &FxHashSet<String>, text: &str, span: Span) {

clippy_lints/src/drop_forget_ref.rs

@@ -1,4 +1,4 @@
-use crate::utils::{is_copy, match_def_path, paths, span_note_and_lint};
+use crate::utils::{is_copy, match_def_path, paths, qpath_res, span_note_and_lint};
 use if_chain::if_chain;
 use rustc::hir::*;
 use rustc::lint::{LateContext, LateLintPass, LintArray, LintPass};
@@ -114,7 +114,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for DropForgetRef {
             if let ExprKind::Call(ref path, ref args) = expr.node;
             if let ExprKind::Path(ref qpath) = path.node;
             if args.len() == 1;
-            if let Some(def_id) = cx.tables.qpath_res(qpath, path.hir_id).opt_def_id();
+            if let Some(def_id) = qpath_res(cx, qpath, path.hir_id).opt_def_id();
             then {
                 let lint;
                 let msg;

clippy_lints/src/explicit_write.rs

@@ -140,7 +140,8 @@ fn write_output_string(write_args: &HirVec<Expr>) -> Option<String> {
         if output_args.len() > 0;
         if let ExprKind::AddrOf(_, ref output_string_expr) = output_args[0].node;
         if let ExprKind::Array(ref string_exprs) = output_string_expr.node;
-        if string_exprs.len() > 0;
+        // we only want to provide an automatic suggestion for simple (non-format) strings
+        if string_exprs.len() == 1;
         if let ExprKind::Lit(ref lit) = string_exprs[0].node;
         if let LitKind::Str(ref write_output, _) = lit.node;
         then {

clippy_lints/src/functions.rs

@@ -1,6 +1,6 @@
 use std::convert::TryFrom;
 
-use crate::utils::{iter_input_pats, snippet, snippet_opt, span_lint, type_is_unsafe_function};
+use crate::utils::{iter_input_pats, qpath_res, snippet, snippet_opt, span_lint, type_is_unsafe_function};
 use matches::matches;
 use rustc::hir;
 use rustc::hir::def::Res;
@@ -318,7 +318,7 @@ impl<'a, 'tcx> hir::intravisit::Visitor<'tcx> for DerefVisitor<'a, 'tcx> {
 impl<'a, 'tcx> DerefVisitor<'a, 'tcx> {
     fn check_arg(&self, ptr: &hir::Expr) {
         if let hir::ExprKind::Path(ref qpath) = ptr.node {
-            if let Res::Local(id) = self.cx.tables.qpath_res(qpath, ptr.hir_id) {
+            if let Res::Local(id) = qpath_res(self.cx, qpath, ptr.hir_id) {
                 if self.ptrs.contains(&id) {
                     span_lint(
                         self.cx,