Merge remote-tracking branch 'upstream/master' into rustup

CHANGELOG.md

@@ -1755,6 +1755,7 @@ Released 2018-09-13
 [`out_of_bounds_indexing`]: https://rust-lang.github.io/rust-clippy/master/index.html#out_of_bounds_indexing
 [`overflow_check_conditional`]: https://rust-lang.github.io/rust-clippy/master/index.html#overflow_check_conditional
 [`panic`]: https://rust-lang.github.io/rust-clippy/master/index.html#panic
+[`panic_in_result_fn`]: https://rust-lang.github.io/rust-clippy/master/index.html#panic_in_result_fn
 [`panic_params`]: https://rust-lang.github.io/rust-clippy/master/index.html#panic_params
 [`panicking_unwrap`]: https://rust-lang.github.io/rust-clippy/master/index.html#panicking_unwrap
 [`partialeq_ne_impl`]: https://rust-lang.github.io/rust-clippy/master/index.html#partialeq_ne_impl

clippy_dev/src/ra_setup.rs

@@ -14,7 +14,7 @@ pub fn run(rustc_path: Option<&str>) {
     // we can unwrap here because the arg is required here
     let rustc_path = PathBuf::from(rustc_path.unwrap());
     assert!(rustc_path.is_dir(), "path is not a directory");
-    let rustc_source_basedir = rustc_path.join("src");
+    let rustc_source_basedir = rustc_path.join("compiler");
     assert!(
         rustc_source_basedir.is_dir(),
         "are you sure the path leads to a rustc repo?"
@@ -61,7 +61,7 @@ fn inject_deps_into_manifest(
     let new_deps = extern_crates.map(|dep| {
         // format the dependencies that are going to be put inside the Cargo.toml
         format!(
-            "{dep} = {{ path = \"{source_path}/lib{dep}\" }}\n",
+            "{dep} = {{ path = \"{source_path}/{dep}\" }}\n",
             dep = dep,
             source_path = rustc_source_dir.display()
         )

clippy_lints/src/await_holding_lock.rs

@@ -10,7 +10,7 @@
     /// **What it does:** Checks for calls to await while holding a
     /// non-async-aware MutexGuard.
     ///
-    /// **Why is this bad?** The Mutex types found in syd::sync and parking_lot
+    /// **Why is this bad?** The Mutex types found in std::sync and parking_lot
     /// are not designed to operate in an async context across await points.
     ///
     /// There are two potential solutions. One is to use an asynx-aware Mutex

clippy_lints/src/lib.rs

@@ -269,6 +269,7 @@ macro_rules! declare_clippy_lint {
 mod option_env_unwrap;
 mod option_if_let_else;
 mod overflow_check_conditional;
+mod panic_in_result_fn;
 mod panic_unimplemented;
 mod partialeq_ne_impl;
 mod path_buf_push_overwrite;
@@ -751,6 +752,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
         &option_env_unwrap::OPTION_ENV_UNWRAP,
         &option_if_let_else::OPTION_IF_LET_ELSE,
         &overflow_check_conditional::OVERFLOW_CHECK_CONDITIONAL,
+        &panic_in_result_fn::PANIC_IN_RESULT_FN,
         &panic_unimplemented::PANIC,
         &panic_unimplemented::PANIC_PARAMS,
         &panic_unimplemented::TODO,
@@ -1091,6 +1093,8 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
     store.register_late_pass(|| box manual_async_fn::ManualAsyncFn);
     store.register_early_pass(|| box redundant_field_names::RedundantFieldNames);
     store.register_late_pass(|| box vec_resize_to_zero::VecResizeToZero);
+    store.register_late_pass(|| box panic_in_result_fn::PanicInResultFn);
+
     let single_char_binding_names_threshold = conf.single_char_binding_names_threshold;
     store.register_early_pass(move || box non_expressive_names::NonExpressiveNames {
         single_char_binding_names_threshold,
@@ -1135,6 +1139,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
         LintId::of(&missing_doc::MISSING_DOCS_IN_PRIVATE_ITEMS),
         LintId::of(&missing_inline::MISSING_INLINE_IN_PUBLIC_ITEMS),
         LintId::of(&modulo_arithmetic::MODULO_ARITHMETIC),
+        LintId::of(&panic_in_result_fn::PANIC_IN_RESULT_FN),
         LintId::of(&panic_unimplemented::PANIC),
         LintId::of(&panic_unimplemented::TODO),
         LintId::of(&panic_unimplemented::UNIMPLEMENTED),

clippy_lints/src/mut_key.rs

@@ -12,8 +12,10 @@
     /// `BtreeSet` rely on either the hash or the order of keys be unchanging,
     /// so having types with interior mutability is a bad idea.
     ///
-    /// **Known problems:** We don't currently account for `Rc` or `Arc`, so
-    /// this may yield false positives.
+    /// **Known problems:** It's correct to use a struct, that contains interior mutability
+    /// as a key, when its `Hash` implementation doesn't access any of the interior mutable types.
+    /// However, this lint is unable to recognize this, so it causes a false positive in theses cases.
+    /// The `bytes` crate is a great example of this.
     ///
     /// **Example:**
     /// ```rust

clippy_lints/src/panic_in_result_fn.rs

@@ -0,0 +1,90 @@
+use crate::utils::{is_expn_of, is_type_diagnostic_item, return_ty, span_lint_and_then};
+use rustc_hir as hir;
+use rustc_hir::intravisit::{self, FnKind, NestedVisitorMap, Visitor};
+use rustc_hir::Expr;
+use rustc_lint::{LateContext, LateLintPass};
+use rustc_middle::hir::map::Map;
+use rustc_session::{declare_lint_pass, declare_tool_lint};
+use rustc_span::Span;
+
+declare_clippy_lint! {
+    /// **What it does:** Checks for usage of `panic!`, `unimplemented!`, `todo!` or `unreachable!` in a function of type result.
+    ///
+    /// **Why is this bad?** For some codebases, it is desirable for functions of type result to return an error instead of crashing. Hence unimplemented, panic and unreachable should be avoided.
+    ///
+    /// **Known problems:** None.
+    ///
+    /// **Example:**
+    ///
+    /// ```rust
+    /// fn result_with_panic() -> Result<bool, String>
+    /// {
+    ///     panic!("error");
+    /// }
+    /// ```
+    pub PANIC_IN_RESULT_FN,
+    restriction,
+    "functions of type `Result<..>` that contain `panic!()`, `todo!()` or `unreachable()` or `unimplemented()` "
+}
+
+declare_lint_pass!(PanicInResultFn  => [PANIC_IN_RESULT_FN]);
+
+impl<'tcx> LateLintPass<'tcx> for PanicInResultFn {
+    fn check_fn(
+        &mut self,
+        cx: &LateContext<'tcx>,
+        fn_kind: FnKind<'tcx>,
+        _: &'tcx hir::FnDecl<'tcx>,
+        body: &'tcx hir::Body<'tcx>,
+        span: Span,
+        hir_id: hir::HirId,
+    ) {
+        if !matches!(fn_kind, FnKind::Closure(_))
+            && is_type_diagnostic_item(cx, return_ty(cx, hir_id), sym!(result_type))
+        {
+            lint_impl_body(cx, span, body);
+        }
+    }
+}
+
+struct FindPanicUnimplementedUnreachable {
+    result: Vec<Span>,
+}
+
+impl<'tcx> Visitor<'tcx> for FindPanicUnimplementedUnreachable {
+    type Map = Map<'tcx>;
+
+    fn visit_expr(&mut self, expr: &'tcx Expr<'_>) {
+        if ["unimplemented", "unreachable", "panic", "todo"]
+            .iter()
+            .any(|fun| is_expn_of(expr.span, fun).is_some())
+        {
+            self.result.push(expr.span);
+        }
+        // and check sub-expressions
+        intravisit::walk_expr(self, expr);
+    }
+
+    fn nested_visit_map(&mut self) -> NestedVisitorMap<Self::Map> {
+        NestedVisitorMap::None
+    }
+}
+
+fn lint_impl_body<'tcx>(cx: &LateContext<'tcx>, impl_span: Span, body: &'tcx hir::Body<'tcx>) {
+    let mut panics = FindPanicUnimplementedUnreachable { result: Vec::new() };
+    panics.visit_expr(&body.value);
+    if !panics.result.is_empty() {
+        span_lint_and_then(
+            cx,
+            PANIC_IN_RESULT_FN,
+            impl_span,
+            "used `unimplemented!()`, `unreachable!()`, `todo!()` or `panic!()` in a function that returns `Result`",
+            move |diag| {
+                diag.help(
+                    "`unimplemented!()`, `unreachable!()`, `todo!()` or `panic!()` should not be used in a function that returns `Result` as `Result` is expected to return an error instead of crashing",
+                );
+                diag.span_note(panics.result, "return Err() instead of panicking");
+            },
+        );
+    }
+}

src/lintlist/mod.rs

@@ -1718,6 +1718,13 @@
         deprecation: None,
         module: "panic_unimplemented",
     },
+    Lint {
+        name: "panic_in_result_fn",
+        group: "restriction",
+        desc: "functions of type `Result<..>` that contain `panic!()`, `todo!()` or `unreachable()` or `unimplemented()` ",
+        deprecation: None,
+        module: "panic_in_result_fn",
+    },
     Lint {
         name: "panic_params",
         group: "style",

tests/ui/panic_in_result_fn.rs

@@ -0,0 +1,70 @@
+#![warn(clippy::panic_in_result_fn)]
+
+struct A;
+
+impl A {
+    fn result_with_panic() -> Result<bool, String> // should emit lint
+    {
+        panic!("error");
+    }
+
+    fn result_with_unimplemented() -> Result<bool, String> // should emit lint
+    {
+        unimplemented!();
+    }
+
+    fn result_with_unreachable() -> Result<bool, String> // should emit lint
+    {
+        unreachable!();
+    }
+
+    fn result_with_todo() -> Result<bool, String> // should emit lint
+    {
+        todo!("Finish this");
+    }
+
+    fn other_with_panic() // should not emit lint
+    {
+        panic!("");
+    }
+
+    fn other_with_unreachable() // should not emit lint
+    {
+        unreachable!();
+    }
+
+    fn other_with_unimplemented() // should not emit lint
+    {
+        unimplemented!();
+    }
+
+    fn other_with_todo() // should not emit lint
+    {
+        todo!("finish this")
+    }
+
+    fn result_without_banned_functions() -> Result<bool, String> // should not emit lint
+    {
+        Ok(true)
+    }
+}
+
+fn function_result_with_panic() -> Result<bool, String> // should emit lint
+{
+    panic!("error");
+}
+
+fn todo() {
+    println!("something");
+}
+
+fn function_result_with_custom_todo() -> Result<bool, String> // should not emit lint
+{
+    todo();
+    Ok(true)
+}
+
+fn main() -> Result<(), String> {
+    todo!("finish main method");
+    Ok(())
+}

tests/ui/panic_in_result_fn.stderr

@@ -0,0 +1,105 @@
+error: used `unimplemented!()`, `unreachable!()`, `todo!()` or `panic!()` in a function that returns `Result`
+  --> $DIR/panic_in_result_fn.rs:6:5
+   |
+LL | /     fn result_with_panic() -> Result<bool, String> // should emit lint
+LL | |     {
+LL | |         panic!("error");
+LL | |     }
+   | |_____^
+   |
+   = note: `-D clippy::panic-in-result-fn` implied by `-D warnings`
+   = help: `unimplemented!()`, `unreachable!()`, `todo!()` or `panic!()` should not be used in a function that returns `Result` as `Result` is expected to return an error instead of crashing
+note: return Err() instead of panicking
+  --> $DIR/panic_in_result_fn.rs:8:9
+   |
+LL |         panic!("error");
+   |         ^^^^^^^^^^^^^^^^
+   = note: this error originates in a macro (in Nightly builds, run with -Z macro-backtrace for more info)
+
+error: used `unimplemented!()`, `unreachable!()`, `todo!()` or `panic!()` in a function that returns `Result`
+  --> $DIR/panic_in_result_fn.rs:11:5
+   |
+LL | /     fn result_with_unimplemented() -> Result<bool, String> // should emit lint
+LL | |     {
+LL | |         unimplemented!();
+LL | |     }
+   | |_____^
+   |
+   = help: `unimplemented!()`, `unreachable!()`, `todo!()` or `panic!()` should not be used in a function that returns `Result` as `Result` is expected to return an error instead of crashing
+note: return Err() instead of panicking
+  --> $DIR/panic_in_result_fn.rs:13:9
+   |
+LL |         unimplemented!();
+   |         ^^^^^^^^^^^^^^^^^
+   = note: this error originates in a macro (in Nightly builds, run with -Z macro-backtrace for more info)
+
+error: used `unimplemented!()`, `unreachable!()`, `todo!()` or `panic!()` in a function that returns `Result`
+  --> $DIR/panic_in_result_fn.rs:16:5
+   |
+LL | /     fn result_with_unreachable() -> Result<bool, String> // should emit lint
+LL | |     {
+LL | |         unreachable!();
+LL | |     }
+   | |_____^
+   |
+   = help: `unimplemented!()`, `unreachable!()`, `todo!()` or `panic!()` should not be used in a function that returns `Result` as `Result` is expected to return an error instead of crashing
+note: return Err() instead of panicking
+  --> $DIR/panic_in_result_fn.rs:18:9
+   |
+LL |         unreachable!();
+   |         ^^^^^^^^^^^^^^^
+   = note: this error originates in a macro (in Nightly builds, run with -Z macro-backtrace for more info)
+
+error: used `unimplemented!()`, `unreachable!()`, `todo!()` or `panic!()` in a function that returns `Result`
+  --> $DIR/panic_in_result_fn.rs:21:5
+   |
+LL | /     fn result_with_todo() -> Result<bool, String> // should emit lint
+LL | |     {
+LL | |         todo!("Finish this");
+LL | |     }
+   | |_____^
+   |
+   = help: `unimplemented!()`, `unreachable!()`, `todo!()` or `panic!()` should not be used in a function that returns `Result` as `Result` is expected to return an error instead of crashing
+note: return Err() instead of panicking
+  --> $DIR/panic_in_result_fn.rs:23:9
+   |
+LL |         todo!("Finish this");
+   |         ^^^^^^^^^^^^^^^^^^^^^
+   = note: this error originates in a macro (in Nightly builds, run with -Z macro-backtrace for more info)
+
+error: used `unimplemented!()`, `unreachable!()`, `todo!()` or `panic!()` in a function that returns `Result`
+  --> $DIR/panic_in_result_fn.rs:52:1
+   |
+LL | / fn function_result_with_panic() -> Result<bool, String> // should emit lint
+LL | | {
+LL | |     panic!("error");
+LL | | }
+   | |_^
+   |
+   = help: `unimplemented!()`, `unreachable!()`, `todo!()` or `panic!()` should not be used in a function that returns `Result` as `Result` is expected to return an error instead of crashing
+note: return Err() instead of panicking
+  --> $DIR/panic_in_result_fn.rs:54:5
+   |
+LL |     panic!("error");
+   |     ^^^^^^^^^^^^^^^^
+   = note: this error originates in a macro (in Nightly builds, run with -Z macro-backtrace for more info)
+
+error: used `unimplemented!()`, `unreachable!()`, `todo!()` or `panic!()` in a function that returns `Result`
+  --> $DIR/panic_in_result_fn.rs:67:1
+   |
+LL | / fn main() -> Result<(), String> {
+LL | |     todo!("finish main method");
+LL | |     Ok(())
+LL | | }
+   | |_^
+   |
+   = help: `unimplemented!()`, `unreachable!()`, `todo!()` or `panic!()` should not be used in a function that returns `Result` as `Result` is expected to return an error instead of crashing
+note: return Err() instead of panicking
+  --> $DIR/panic_in_result_fn.rs:68:5
+   |
+LL |     todo!("finish main method");
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   = note: this error originates in a macro (in Nightly builds, run with -Z macro-backtrace for more info)
+
+error: aborting due to 6 previous errors
+

tests/ui/temporary_assignment.rs

@@ -1,5 +1,4 @@
 #![warn(clippy::temporary_assignment)]
-#![allow(const_item_mutation)]
 
 use std::ops::{Deref, DerefMut};
 

tests/ui/temporary_assignment.stderr

@@ -1,5 +1,5 @@
 error: assignment to temporary
-  --> $DIR/temporary_assignment.rs:48:5
+  --> $DIR/temporary_assignment.rs:47:5
    |
 LL |     Struct { field: 0 }.field = 1;
    |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
@@ -7,7 +7,7 @@ LL |     Struct { field: 0 }.field = 1;
    = note: `-D clippy::temporary-assignment` implied by `-D warnings`
 
 error: assignment to temporary
-  --> $DIR/temporary_assignment.rs:49:5
+  --> $DIR/temporary_assignment.rs:48:5
    |
 LL | /     MultiStruct {
 LL | |         structure: Struct { field: 0 },
@@ -17,13 +17,13 @@ LL | |     .field = 1;
    | |______________^
 
 error: assignment to temporary
-  --> $DIR/temporary_assignment.rs:54:5
+  --> $DIR/temporary_assignment.rs:53:5
    |
 LL |     ArrayStruct { array: [0] }.array[0] = 1;
    |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
 error: assignment to temporary
-  --> $DIR/temporary_assignment.rs:55:5
+  --> $DIR/temporary_assignment.rs:54:5
    |
 LL |     (0, 0).0 = 1;
    |     ^^^^^^^^^^^^

util/dev

@@ -1,7 +0,0 @@
-#!/bin/sh
-CARGO_TARGET_DIR=$(pwd)/target/
-export CARGO_TARGET_DIR
-
-echo 'Deprecated! `util/dev` usage is deprecated, please use `cargo dev` instead.'
-
-cd clippy_dev && cargo run -- "$@"