Update Clippy

flip1995 · Dec 13, 2018 · 7a19419 · 7a19419
1 parent 70ce94f
commit 7a19419
Show file tree

Hide file tree

Showing 483 changed files with 7,313 additions and 7,108 deletions.
diff --git a/.travis.yml b/.travis.yml
@@ -18,15 +18,6 @@ env:
  global:
    - RUST_BACKTRACE=1
 
-before_install:
- - |
-    # work-around for issue https://github.com/travis-ci/travis-ci/issues/6307
-    # might not be necessary in the future
-    if [ "$TRAVIS_OS_NAME" == "osx" ]; then
-     command curl -sSL https://rvm.io/mpapis.asc | gpg --import -
-     rvm get stable
-    fi
-
 install:
   - |
     if [ -z ${INTEGRATION} ]; then
@@ -46,6 +37,7 @@ install:
 # if: fork = false
 # but this is currently buggy travis-ci/travis-ci#9118
 matrix:
+  fast_finish: true
   include:
     - os: osx # run base tests on both platforms
       env: BASE_TESTS=true

diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md
@@ -19,10 +19,10 @@ All contributors are expected to follow the [Rust Code of Conduct](http://www.ru
   * [Running test suite](#running-test-suite)
   * [Running rustfmt](#running-rustfmt)
   * [Testing manually](#testing-manually)
-  * [Linting Clippy with your local changes](#linting-clippy-with-your-local-changes)
   * [How Clippy works](#how-clippy-works)
   * [Fixing nightly build failures](#fixing-build-failures-caused-by-rust)
 * [Issue and PR Triage](#issue-and-pr-triage)
+* [Bors and Homu](#bors-and-homu)
 * [Contributions](#contributions)
 
 ## Getting started
@@ -156,7 +156,7 @@ to style guidelines. The code has to be formatted by `rustfmt` before a PR will
 
 It can be installed via `rustup`:
 ```bash
-rustup component add rustfmt-preview
+rustup component add rustfmt
 ```
 
 Use `cargo fmt --all` to format the whole codebase.
@@ -220,7 +220,7 @@ That's why the `else_if_without_else` example uses the `register_early_lint_pass
 
 ### Fixing build failures caused by Rust
 
-Clippy will sometimes break because it still depends on unstable internal Rust features. Most of the times we have to adapt to the changes and only very rarely there's an actual bug in Rust. Fixing build failures caused by Rust updates, can be a good way to learn about Rust internals.
+Clippy will sometimes fail to build from source because building it depends on unstable internal Rust features. Most of the times we have to adapt to the changes and only very rarely there's an actual bug in Rust. Fixing build failures caused by Rust updates, can be a good way to learn about Rust internals.
 
 In order to find out why Clippy does not work properly with a new Rust commit, you can use the [rust-toolstate commit history][toolstate_commit_history].
 You will then have to look for the last commit that contains `test-pass -> build-fail` or `test-pass` -> `test-fail` for the `clippy-driver` component. [Here][toolstate_commit] is an example.
@@ -257,6 +257,17 @@ Our highest priority is fixing [crashes][l-crash] and [bugs][l-bug]. We don't
 want Clippy to crash on your code and we want it to be as reliable as the
 suggestions from Rust compiler errors.
 
+## Bors and Homu
+
+We use a bot powered by [Homu][homu] to help automate testing and landing of pull
+requests in Clippy. The bot's username is @bors.
+
+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].
+
+
 ## Contributions
 
 Contributions to Clippy should be made in the form of GitHub pull requests. Each pull request will
@@ -288,3 +299,6 @@ or the [MIT](http://opensource.org/licenses/MIT) license.
 [triage]: https://forge.rust-lang.org/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/servo/homu
+[homu_instructions]: https://buildbot2.rust-lang.org/homu/
+[homu_queue]: https://buildbot2.rust-lang.org/homu/queue/clippy
diff --git a/README.md b/README.md
@@ -106,8 +106,8 @@ script:
   # etc.
 ```
 
-It might happen that clippy is not available for a certain nightly release.
-In this case you can try to conditionally install clippy from the git repo.
+It might happen that Clippy is not available for a certain nightly release.
+In this case you can try to conditionally install Clippy from the git repo.
 
 ```yaml
 language: rust
@@ -149,21 +149,7 @@ You can add options to your code to `allow`/`warn`/`deny` Clippy lints:
 
 Note: `deny` produces errors instead of warnings.
 
-Note: To use the new `clippy::lint_name` syntax, a recent compiler has to be used
-currently. If you want to compile your code with the stable toolchain you can use a `cfg_attr` to
-activate the `tool_lints` feature:
-```rust
-#![cfg_attr(feature = "cargo-clippy", allow(clippy::lint_name))]
-```
-
-For this to work you have to use Clippy on the nightly toolchain: `cargo +nightly clippy`. If you
-want to use Clippy with the stable toolchain, you can stick to the old unscoped method to
-enable/disable Clippy lints until `tool_lints` are stable:
-```rust
-#![cfg_attr(feature = "cargo-clippy", allow(clippy_lint))]
-```
-
-If you do not want to include your lint levels in your code, you can globally enable/disable lints by passing extra flags to clippy during the run: `cargo clippy -- -A lint_name` will run clippy with `lint_name` disabled and `cargo clippy -- -W lint_name` will run it with that enabled. On newer compilers you may need to use `clippy::lint_name` instead.
+If you do not want to include your lint levels in your code, you can globally enable/disable lints by passing extra flags to Clippy during the run: `cargo clippy -- -A clippy::lint_name` will run Clippy with `lint_name` disabled and `cargo clippy -- -W clippy::lint_name` will run it with that enabled. This also works with lint groups. For example you can run Clippy with warnings for all lints enabled: `cargo clippy -- -W clippy::pedantic`
 
 ## License
 

diff --git a/ci/base-tests.sh b/ci/base-tests.sh
@@ -20,10 +20,29 @@ fi
 # build clippy in debug mode and run tests
 cargo build --features debugging
 cargo test --features debugging
+# for faster build, share target dir between subcrates
+export CARGO_TARGET_DIR=`pwd`/target/
 cd clippy_lints && cargo test && cd ..
 cd rustc_tools_util && cargo test && cd ..
 cd clippy_dev && cargo test && cd ..
 
 # Perform various checks for lint registration
 ./util/dev update_lints --check
 cargo +nightly fmt --all -- --check
+
+
+#avoid loop spam
+set +x
+# make sure tests are formatted
+
+# some lints are sensitive to formatting, exclude some files
+needs_formatting=false
+for file in `find tests -not -path "tests/ui/methods.rs" -not -path "tests/ui/format.rs" -not -path "tests/ui/formatting.rs" -not -path "tests/ui/empty_line_after_outer_attribute.rs" -not -path "tests/ui/double_parens.rs" -not -path "tests/ui/doc.rs" -not -path "tests/ui/unused_unit.rs" | grep "\.rs$"` ; do
+rustfmt ${file} --check  || echo "${file} needs reformatting!" ; needs_formatting=true
+done
+
+if [ "${needs_reformatting}" = true ] ; then
+    echo "Tests need reformatting!"
+    exit 2
+fi
+set -x
diff --git a/clippy_lints/src/attrs.rs b/clippy_lints/src/attrs.rs
@@ -332,7 +332,7 @@ fn check_clippy_lint_names(cx: &LateContext<'_, '_>, items: &[NestedMetaItem]) {
                     &format!("unknown clippy lint: clippy::{}", name),
                     |db| {
                         if name.as_str().chars().any(|c| c.is_uppercase()) {
-                            let name_lower = name.as_str().to_lowercase().to_string();
+                            let name_lower = name.as_str().to_lowercase();
                             match lint_store.check_lint_name(
                                 &name_lower,
                                 Some(tool_name.as_str())

diff --git a/clippy_lints/src/doc.rs b/clippy_lints/src/doc.rs
@@ -238,7 +238,7 @@ fn check_doc<'a, Events: Iterator<Item = (usize, pulldown_cmark::Event<'a>)>>(
 }
 
 fn check_text(cx: &EarlyContext<'_>, valid_idents: &[String], text: &str, span: Span) {
-    for word in text.split_whitespace() {
+    for word in text.split(|c: char| c.is_whitespace() || c == '\'') {
         // Trim punctuation as in `some comment (see foo::bar).`
         //                                                   ^^
         // Or even as in `_foo bar_` which is emphasized.

diff --git a/clippy_lints/src/excessive_precision.rs b/clippy_lints/src/excessive_precision.rs
@@ -109,10 +109,9 @@ impl ExcessivePrecision {
     }
 }
 
-#[allow(clippy::doc_markdown)]
 /// Should we exclude the float because it has a `.0` or `.` suffix
-/// Ex 1_000_000_000.0
-/// Ex 1_000_000_000.
+/// Ex `1_000_000_000.0`
+/// Ex `1_000_000_000.`
 fn dot_zero_exclusion(s: &str) -> bool {
     if let Some(after_dec) = s.split('.').nth(1) {
         let mut decpart = after_dec.chars().take_while(|c| *c != 'e' || *c != 'E');

diff --git a/clippy_lints/src/methods/mod.rs b/clippy_lints/src/methods/mod.rs
@@ -1358,7 +1358,10 @@ fn lint_clone_on_copy(cx: &LateContext<'_, '_>, expr: &hir::Expr, arg: &hir::Exp
                     _ => {},
                 }
 
-                let deref_count = cx.tables.expr_adjustments(arg).iter()
+                let deref_count = cx
+                    .tables
+                    .expr_adjustments(arg)
+                    .iter()
                     .filter(|adj| {
                         if let ty::adjustment::Adjust::Deref(_) = adj.kind {
                             true

diff --git a/clippy_lints/src/no_effect.rs b/clippy_lints/src/no_effect.rs
@@ -57,7 +57,7 @@ fn has_no_effect(cx: &LateContext<'_, '_>, expr: &Expr) -> bool {
     }
     match expr.node {
         ExprKind::Lit(..) | ExprKind::Closure(.., _) => true,
-        ExprKind::Path(..) => !has_drop(cx, expr),
+        ExprKind::Path(..) => !has_drop(cx, cx.tables.expr_ty(expr)),
         ExprKind::Index(ref a, ref b) | ExprKind::Binary(_, ref a, ref b) => {
             has_no_effect(cx, a) && has_no_effect(cx, b)
         },
@@ -70,7 +70,7 @@ fn has_no_effect(cx: &LateContext<'_, '_>, expr: &Expr) -> bool {
         | ExprKind::AddrOf(_, ref inner)
         | ExprKind::Box(ref inner) => has_no_effect(cx, inner),
         ExprKind::Struct(_, ref fields, ref base) => {
-            !has_drop(cx, expr)
+            !has_drop(cx, cx.tables.expr_ty(expr))
                 && fields.iter().all(|field| has_no_effect(cx, &field.expr))
                 && match *base {
                     Some(ref base) => has_no_effect(cx, base),
@@ -82,7 +82,7 @@ fn has_no_effect(cx: &LateContext<'_, '_>, expr: &Expr) -> bool {
                 let def = cx.tables.qpath_def(qpath, callee.hir_id);
                 match def {
                     Def::Struct(..) | Def::Variant(..) | Def::StructCtor(..) | Def::VariantCtor(..) => {
-                        !has_drop(cx, expr) && args.iter().all(|arg| has_no_effect(cx, arg))
+                        !has_drop(cx, cx.tables.expr_ty(expr)) && args.iter().all(|arg| has_no_effect(cx, arg))
                     },
                     _ => false,
                 }
@@ -161,7 +161,7 @@ fn reduce_expression<'a>(cx: &LateContext<'_, '_>, expr: &'a Expr) -> Option<Vec
         | ExprKind::AddrOf(_, ref inner)
         | ExprKind::Box(ref inner) => reduce_expression(cx, inner).or_else(|| Some(vec![inner])),
         ExprKind::Struct(_, ref fields, ref base) => {
-            if has_drop(cx, expr) {
+            if has_drop(cx, cx.tables.expr_ty(expr)) {
                 None
             } else {
                 Some(fields.iter().map(|f| &f.expr).chain(base).map(Deref::deref).collect())
@@ -172,7 +172,7 @@ fn reduce_expression<'a>(cx: &LateContext<'_, '_>, expr: &'a Expr) -> Option<Vec
                 let def = cx.tables.qpath_def(qpath, callee.hir_id);
                 match def {
                     Def::Struct(..) | Def::Variant(..) | Def::StructCtor(..) | Def::VariantCtor(..)
-                        if !has_drop(cx, expr) =>
+                        if !has_drop(cx, cx.tables.expr_ty(expr)) =>
                     {
                         Some(args.iter().collect())
                     },

diff --git a/clippy_lints/src/redundant_clone.rs b/clippy_lints/src/redundant_clone.rs
@@ -12,7 +12,7 @@ use crate::rustc::hir::{def_id, Body, FnDecl};
 use crate::rustc::lint::{LateContext, LateLintPass, LintArray, LintPass};
 use crate::rustc::mir::{
     self, traversal,
-    visit::{MutatingUseContext, NonUseContext, PlaceContext, Visitor},
+    visit::{MutatingUseContext, PlaceContext, Visitor},
     TerminatorKind,
 };
 use crate::rustc::ty;
@@ -23,10 +23,11 @@ use crate::syntax::{
     source_map::{BytePos, Span},
 };
 use crate::utils::{
-    in_macro, is_copy, match_def_path, match_type, paths, snippet_opt, span_lint_node, span_lint_node_and_then,
-    walk_ptrs_ty_depth,
+    has_drop, in_macro, is_copy, match_def_path, match_type, paths, snippet_opt, span_lint_node,
+    span_lint_node_and_then, walk_ptrs_ty_depth,
 };
 use if_chain::if_chain;
+use matches::matches;
 use std::convert::TryFrom;
 
 macro_rules! unwrap_or_continue {
@@ -126,7 +127,17 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for RedundantClone {
             // _1 in MIR `{ _2 = &_1; clone(move _2); }` or `{ _2 = _1; to_path_buf(_2); } (from_deref)
             // In case of `from_deref`, `arg` is already a reference since it is `deref`ed in the previous
             // block.
-            let cloned = unwrap_or_continue!(find_stmt_assigns_to(arg, from_borrow, bbdata.statements.iter().rev()));
+            let (cloned, cannot_move_out) = unwrap_or_continue!(find_stmt_assigns_to(
+                cx,
+                mir,
+                arg,
+                from_borrow,
+                bbdata.statements.iter()
+            ));
+
+            if from_borrow && cannot_move_out {
+                continue;
+            }
 
             // _1 in MIR `{ _2 = &_1; _3 = deref(move _2); } -> { _4 = _3; to_path_buf(move _4); }`
             let referent = if from_deref {
@@ -150,7 +161,17 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for RedundantClone {
                     }
                 };
 
-                unwrap_or_continue!(find_stmt_assigns_to(pred_arg, true, mir[ps[0]].statements.iter().rev()))
+                let (local, cannot_move_out) = unwrap_or_continue!(find_stmt_assigns_to(
+                    cx,
+                    mir,
+                    pred_arg,
+                    true,
+                    mir[ps[0]].statements.iter()
+                ));
+                if cannot_move_out {
+                    continue;
+                }
+                local
             } else {
                 cloned
             };
@@ -227,27 +248,69 @@ fn is_call_with_ref_arg<'tcx>(
     }
 }
 
-/// Finds the first `to = (&)from`, and returns `Some(from)`.
+type CannotMoveOut = bool;
+
+/// Finds the first `to = (&)from`, and returns
+/// ``Some((from, [`true` if `from` cannot be moved out]))``.
 fn find_stmt_assigns_to<'a, 'tcx: 'a>(
+    cx: &LateContext<'_, 'tcx>,
+    mir: &mir::Mir<'tcx>,
     to: mir::Local,
     by_ref: bool,
-    mut stmts: impl Iterator<Item = &'a mir::Statement<'tcx>>,
-) -> Option<mir::Local> {
-    stmts.find_map(|stmt| {
-        if let mir::StatementKind::Assign(mir::Place::Local(local), v) = &stmt.kind {
-            if *local == to {
-                if by_ref {
-                    if let mir::Rvalue::Ref(_, _, mir::Place::Local(r)) = **v {
-                        return Some(r);
-                    }
-                } else if let mir::Rvalue::Use(mir::Operand::Copy(mir::Place::Local(r))) = **v {
-                    return Some(r);
+    stmts: impl DoubleEndedIterator<Item = &'a mir::Statement<'tcx>>,
+) -> Option<(mir::Local, CannotMoveOut)> {
+    stmts
+        .rev()
+        .find_map(|stmt| {
+            if let mir::StatementKind::Assign(mir::Place::Local(local), v) = &stmt.kind {
+                if *local == to {
+                    return Some(v);
                 }
             }
-        }
 
-        None
-    })
+            None
+        })
+        .and_then(|v| {
+            if by_ref {
+                if let mir::Rvalue::Ref(_, _, ref place) = **v {
+                    return base_local_and_movability(cx, mir, place);
+                }
+            } else if let mir::Rvalue::Use(mir::Operand::Copy(ref place)) = **v {
+                return base_local_and_movability(cx, mir, place);
+            }
+            None
+        })
+}
+
+/// Extracts and returns the undermost base `Local` of given `place`. Returns `place` itself
+/// if it is already a `Local`.
+///
+/// Also reports whether given `place` cannot be moved out.
+fn base_local_and_movability<'tcx>(
+    cx: &LateContext<'_, 'tcx>,
+    mir: &mir::Mir<'tcx>,
+    mut place: &mir::Place<'tcx>,
+) -> Option<(mir::Local, CannotMoveOut)> {
+    use rustc::mir::Place::*;
+
+    // Dereference. You cannot move things out from a borrowed value.
+    let mut deref = false;
+    // Accessing a field of an ADT that has `Drop`. Moving the field out will cause E0509.
+    let mut field = false;
+
+    loop {
+        match place {
+            Local(local) => return Some((*local, deref || field)),
+            Projection(proj) => {
+                place = &proj.base;
+                deref = deref || matches!(proj.elem, mir::ProjectionElem::Deref);
+                if !field && matches!(proj.elem, mir::ProjectionElem::Field(..)) {
+                    field = has_drop(cx, place.ty(&mir.local_decls, cx.tcx).to_ty(cx.tcx));
+                }
+            },
+            _ => return None,
+        }
+    }
 }
 
 struct LocalUseVisitor {
@@ -279,9 +342,7 @@ impl<'tcx> mir::visit::Visitor<'tcx> for LocalUseVisitor {
 
     fn visit_local(&mut self, local: &mir::Local, ctx: PlaceContext<'tcx>, _: mir::Location) {
         match ctx {
-            PlaceContext::MutatingUse(MutatingUseContext::Drop) | PlaceContext::NonUse(NonUseContext::StorageDead) => {
-                return;
-            },
+            PlaceContext::MutatingUse(MutatingUseContext::Drop) | PlaceContext::NonUse(_) => return,
             _ => {},
         }