Merge branch 'master' of github.com:rust-lang/rust-clippy

Author: HarrisonMc555

CHANGELOG.md

@@ -4,7 +4,38 @@ All notable changes to this project will be documented in this file.
 
 ## Unreleased / In Rust Beta or Nightly
 
-[b2601be...master](https://github.com/rust-lang/rust-clippy/compare/b2601be...master)
+[1b89724...master](https://github.com/rust-lang/rust-clippy/compare/1b89724...master)
+
+## Rust 1.33 (2019-02-26)
+
+[b2601be...1b89724](https://github.com/rust-lang/rust-clippy/compare/b2601be...1b89724)
+
+* New lints: [`implicit_return`], [`vec_box`], [`cast_ref_to_mut`]
+* The `rust-clippy` repository is now part of the `rust-lang` org.
+* Rename `stutter` to `module_name_repetitions`
+* Merge `new_without_default_derive` into `new_without_default` lint
+* Move `large_digit_groups` from `style` group to `pedantic`
+* Expand `bool_comparison` to check for `<`, `<=`, `>`, `>=`, and `!=`
+  comparisons against booleans
+* Expand `no_effect` to detect writes to constants such as `A_CONST.field = 2`
+* Expand `redundant_clone` to work on struct fields
+* Expand `suspicious_else_formatting` to detect `if .. {..} {..}`
+* Expand `use_self` to work on tuple structs and also in local macros
+* Fix ICE in `result_map_unit_fn` and `option_map_unit_fn`
+* Fix false positives in `implicit_return`
+* Fix false positives in `use_self`
+* Fix false negative in `clone_on_copy`
+* Fix false positive in `doc_markdown`
+* Fix false positive in `empty_loop`
+* Fix false positive in `if_same_then_else`
+* Fix false positive in `infinite_iter`
+* Fix false positive in `question_mark`
+* Fix false positive in `useless_asref`
+* Fix false positive in `wildcard_dependencies`
+* Fix false positive in `write_with_newline`
+* Add suggestion to `explicit_write`
+* Improve suggestions for `question_mark` lint
+* Fix incorrect suggestion for `get_unwrap`
 
 ## Rust 1.32 (2019-01-17)
 
@@ -785,6 +816,7 @@ All notable changes to this project will be documented in this file.
 [`double_comparisons`]: https://rust-lang.github.io/rust-clippy/master/index.html#double_comparisons
 [`double_neg`]: https://rust-lang.github.io/rust-clippy/master/index.html#double_neg
 [`double_parens`]: https://rust-lang.github.io/rust-clippy/master/index.html#double_parens
+[`drop_bounds`]: https://rust-lang.github.io/rust-clippy/master/index.html#drop_bounds
 [`drop_copy`]: https://rust-lang.github.io/rust-clippy/master/index.html#drop_copy
 [`drop_ref`]: https://rust-lang.github.io/rust-clippy/master/index.html#drop_ref
 [`duplicate_underscore_argument`]: https://rust-lang.github.io/rust-clippy/master/index.html#duplicate_underscore_argument

README.md

@@ -11,14 +11,14 @@ A collection of lints to catch common mistakes and improve your [Rust](https://g
 
 We have a bunch of lint categories to allow you to choose how much Clippy is supposed to ~~annoy~~ help you:
 
-* `clippy::all` (everything that has no false positives)
-* `clippy::pedantic` (everything)
-* `clippy::nursery` (new lints that aren't quite ready yet)
+* `clippy::all` (everything that is on by default: all the categories below except for `nursery`, `pedantic`, and `cargo`)
+* **`clippy::correctness`** (code that is just outright wrong or very very useless, causes hard errors by default)
 * `clippy::style` (code that should be written in a more idiomatic way)
 * `clippy::complexity` (code that does something simple but in a complex way)
 * `clippy::perf` (code that can be written in a faster way)
-* `clippy::cargo` (checks against the cargo manifest)
-* **`clippy::correctness`** (code that is just outright wrong or very very useless)
+* `clippy::pedantic` (lints which are rather strict, off by default)
+* `clippy::nursery` (new lints that aren't quite ready yet, off by default)
+* `clippy::cargo` (checks against the cargo manifest, off by default)
 
 More to come, please [file an issue](https://github.com/rust-lang/rust-clippy/issues) if you have ideas!
 
@@ -31,6 +31,8 @@ Only the following of those categories are enabled by default:
 
 Other categories need to be enabled in order for their lints to be executed.
 
+The [lint list](https://rust-lang.github.io/rust-clippy/master/index.html) also contains "restriction lints", which are for things which are usually not considered "bad", but may be useful to turn on in specific cases. These should be used very selectively, if at all.
+
 Table of contents:
 
 *   [Usage instructions](#usage)

ci/base-tests.sh

@@ -59,7 +59,7 @@ rustup override set nightly
 # avoid loop spam and allow cmds with exit status != 0
 set +ex
 
-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
+for file in `find tests -not -path "tests/ui/doc.rs" -not -path "tests/ui/unused_unit.rs" | grep "\.rs$"` ; do
   rustfmt ${file} --check
   if [ $? -ne 0 ]; then
     echo "${file} needs reformatting!"

clippy_dev/src/lib.rs

@@ -47,7 +47,7 @@ impl Lint {
             name: name.to_lowercase(),
             group: group.to_string(),
             desc: NL_ESCAPE_RE.replace(&desc.replace("\\\"", "\""), "").to_string(),
-            deprecation: deprecation.map(|d| d.to_string()),
+            deprecation: deprecation.map(std::string::ToString::to_string),
             module: module.to_string(),
         }
     }
@@ -178,7 +178,7 @@ fn lint_files() -> impl Iterator<Item = walkdir::DirEntry> {
     // Otherwise we would not collect all the lints, for example in `clippy_lints/src/methods/`.
     WalkDir::new("../clippy_lints/src")
         .into_iter()
-        .filter_map(|f| f.ok())
+        .filter_map(std::result::Result::ok)
         .filter(|f| f.path().extension() == Some(OsStr::new("rs")))
 }
 

clippy_lints/src/assertions_on_constants.rs

@@ -3,7 +3,7 @@ use crate::rustc::hir::{Expr, ExprKind};
 use crate::rustc::lint::{LateContext, LateLintPass, LintArray, LintPass};
 use crate::rustc::{declare_tool_lint, lint_array};
 use crate::syntax::ast::LitKind;
-use crate::utils::{is_direct_expn_of, span_help_and_lint};
+use crate::utils::{in_macro, is_direct_expn_of, span_help_and_lint};
 use if_chain::if_chain;
 
 /// **What it does:** Check to call assert!(true/false)
@@ -43,7 +43,9 @@ impl LintPass for AssertionsOnConstants {
 impl<'a, 'tcx> LateLintPass<'a, 'tcx> for AssertionsOnConstants {
     fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, e: &'tcx Expr) {
         if_chain! {
-            if is_direct_expn_of(e.span, "assert").is_some();
+            if let Some(assert_span) = is_direct_expn_of(e.span, "assert");
+            if !in_macro(assert_span)
+                || is_direct_expn_of(assert_span, "debug_assert").map_or(false, |span| !in_macro(span));
             if let ExprKind::Unary(_, ref lit) = e.node;
             then {
                 if let ExprKind::Lit(ref inner) = lit.node {

clippy_lints/src/assign_ops.rs

@@ -6,7 +6,6 @@ use rustc::hir::intravisit::{walk_expr, NestedVisitorMap, Visitor};
 use rustc::lint::{LateContext, LateLintPass, LintArray, LintPass};
 use rustc::{declare_tool_lint, lint_array};
 use rustc_errors::Applicability;
-use syntax::ast;
 
 /// **What it does:** Checks for `a = a op b` or `a = b commutative_op a`
 /// patterns.
@@ -140,12 +139,12 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for AssignOps {
                                             return; // useless if the trait doesn't exist
                                         };
                                         // check that we are not inside an `impl AssignOp` of this exact operation
-                                        let parent_fn = cx.tcx.hir().get_parent(e.id);
-                                        let parent_impl = cx.tcx.hir().get_parent(parent_fn);
+                                        let parent_fn = cx.tcx.hir().get_parent_item(e.hir_id);
+                                        let parent_impl = cx.tcx.hir().get_parent_item(parent_fn);
                                         // the crate node is the only one that is not in the map
                                         if_chain! {
-                                            if parent_impl != ast::CRATE_NODE_ID;
-                                            if let hir::Node::Item(item) = cx.tcx.hir().get(parent_impl);
+                                            if parent_impl != hir::CRATE_HIR_ID;
+                                            if let hir::Node::Item(item) = cx.tcx.hir().get_by_hir_id(parent_impl);
                                             if let hir::ItemKind::Impl(_, _, _, _, Some(trait_ref), _, _) =
                                                 &item.node;
                                             if trait_ref.path.def.def_id() == trait_id;

clippy_lints/src/attrs.rs

@@ -326,7 +326,7 @@ fn check_clippy_lint_names(cx: &LateContext<'_, '_>, items: &[NestedMetaItem]) {
                     lint.span,
                     &format!("unknown clippy lint: clippy::{}", name),
                     |db| {
-                        if name.as_str().chars().any(|c| c.is_uppercase()) {
+                        if name.as_str().chars().any(char::is_uppercase) {
                             let name_lower = name.as_str().to_lowercase();
                             match lint_store.check_lint_name(
                                 &name_lower,

clippy_lints/src/booleans.rs

@@ -7,7 +7,7 @@ use rustc::lint::{LateContext, LateLintPass, LintArray, LintPass};
 use rustc::{declare_tool_lint, lint_array};
 use rustc_data_structures::thin_vec::ThinVec;
 use rustc_errors::Applicability;
-use syntax::ast::{LitKind, NodeId, DUMMY_NODE_ID};
+use syntax::ast::LitKind;
 use syntax::source_map::{dummy_spanned, Span, DUMMY_SP};
 
 /// **What it does:** Checks for boolean expressions that can be written more
@@ -72,7 +72,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for NonminimalBool {
         _: &'tcx FnDecl,
         body: &'tcx Body,
         _: Span,
-        _: NodeId,
+        _: HirId,
     ) {
         NonminimalBoolVisitor { cx }.visit_body(body)
     }
@@ -132,7 +132,6 @@ impl<'a, 'tcx, 'v> Hir2Qmm<'a, 'tcx, 'v> {
                     }
 
                     let mk_expr = |op| Expr {
-                        id: DUMMY_NODE_ID,
                         hir_id: DUMMY_HIR_ID,
                         span: DUMMY_SP,
                         attrs: ThinVec::new(),

clippy_lints/src/cargo_common_metadata.rs

@@ -53,7 +53,7 @@ fn is_empty_str(value: &Option<String>) -> bool {
 
 fn is_empty_vec(value: &[String]) -> bool {
     // This works because empty iterators return true
-    value.iter().all(|v| v.is_empty())
+    value.iter().all(std::string::String::is_empty)
 }
 
 pub struct Pass;

clippy_lints/src/consts.rs

@@ -5,7 +5,7 @@ use if_chain::if_chain;
 use rustc::hir::def::Def;
 use rustc::hir::*;
 use rustc::lint::LateContext;
-use rustc::ty::subst::{Subst, Substs};
+use rustc::ty::subst::{Subst, SubstsRef};
 use rustc::ty::{self, Instance, Ty, TyCtxt};
 use rustc::{bug, span_bug};
 use rustc_data_structures::sync::Lrc;
@@ -209,7 +209,7 @@ pub struct ConstEvalLateContext<'a, 'tcx: 'a> {
     tables: &'a ty::TypeckTables<'tcx>,
     param_env: ty::ParamEnv<'tcx>,
     needed_resolution: bool,
-    substs: &'tcx Substs<'tcx>,
+    substs: SubstsRef<'tcx>,
 }
 
 impl<'c, 'cc> ConstEvalLateContext<'c, 'cc> {

clippy_lints/src/copies.rs

@@ -125,7 +125,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for CopyAndPaste {
                 ..
             }) = get_parent_expr(cx, expr)
             {
-                if else_expr.id == expr.id {
+                if else_expr.hir_id == expr.hir_id {
                     return;
                 }
             }

clippy_lints/src/cyclomatic_complexity.rs

@@ -6,7 +6,7 @@ use rustc::hir::*;
 use rustc::lint::{LateContext, LateLintPass, LintArray, LintContext, LintPass};
 use rustc::ty;
 use rustc::{declare_tool_lint, lint_array};
-use syntax::ast::{Attribute, NodeId};
+use syntax::ast::Attribute;
 use syntax::source_map::Span;
 
 use crate::utils::{in_macro, is_allowed, match_type, paths, span_help_and_lint, LimitStack};
@@ -78,7 +78,7 @@ impl CyclomaticComplexity {
             returns,
             ..
         } = helper;
-        let ret_ty = cx.tables.node_id_to_type(expr.hir_id);
+        let ret_ty = cx.tables.node_type(expr.hir_id);
         let ret_adjust = if match_type(cx, ret_ty, &paths::RESULT) {
             returns
         } else {
@@ -94,7 +94,7 @@ impl CyclomaticComplexity {
                 short_circuits,
                 ret_adjust,
                 span,
-                body.id().node_id,
+                body.id().hir_id,
             );
         } else {
             let mut rust_cc = cc + divergence - match_arms - short_circuits;
@@ -123,9 +123,9 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for CyclomaticComplexity {
         _: &'tcx FnDecl,
         body: &'tcx Body,
         span: Span,
-        node_id: NodeId,
+        hir_id: HirId,
     ) {
-        let def_id = cx.tcx.hir().local_def_id(node_id);
+        let def_id = cx.tcx.hir().local_def_id_from_hir_id(hir_id);
         if !cx.tcx.has_attr(def_id, "test") {
             self.check(cx, body, span);
         }
@@ -159,7 +159,7 @@ impl<'a, 'tcx> Visitor<'tcx> for CCHelper<'a, 'tcx> {
             },
             ExprKind::Call(ref callee, _) => {
                 walk_expr(self, e);
-                let ty = self.cx.tables.node_id_to_type(callee.hir_id);
+                let ty = self.cx.tables.node_type(callee.hir_id);
                 match ty.sty {
                     ty::FnDef(..) | ty::FnPtr(_) => {
                         let sig = ty.fn_sig(self.cx.tcx);
@@ -197,7 +197,7 @@ fn report_cc_bug(
     shorts: u64,
     returns: u64,
     span: Span,
-    _: NodeId,
+    _: HirId,
 ) {
     span_bug!(
         span,
@@ -220,7 +220,7 @@ fn report_cc_bug(
     shorts: u64,
     returns: u64,
     span: Span,
-    id: NodeId,
+    id: HirId,
 ) {
     if !is_allowed(cx, CYCLOMATIC_COMPLEXITY, id) {
         cx.sess().span_note_without_error(

clippy_lints/src/default_trait_access.rs

@@ -45,7 +45,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for DefaultTraitAccess {
     fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr) {
         if_chain! {
             if let ExprKind::Call(ref path, ..) = expr.node;
-            if !any_parent_is_automatically_derived(cx.tcx, expr.id);
+            if !any_parent_is_automatically_derived(cx.tcx, expr.hir_id);
             if let ExprKind::Path(ref qpath) = path.node;
             if let Some(def_id) = opt_def_id(cx.tables.qpath_def(qpath, path.hir_id));
             if match_def_path(cx.tcx, def_id, &paths::DEFAULT_TRAIT_METHOD);

clippy_lints/src/drop_bounds.rs

@@ -0,0 +1,79 @@
+use crate::utils::{match_def_path, paths, span_lint};
+use if_chain::if_chain;
+use rustc::hir::*;
+use rustc::lint::{LateLintPass, LintArray, LintPass};
+use rustc::{declare_tool_lint, lint_array};
+
+/// **What it does:** Checks for generics with `std::ops::Drop` as bounds.
+///
+/// **Why is this bad?** `Drop` bounds do not really accomplish anything.
+/// A type may have compiler-generated drop glue without implementing the
+/// `Drop` trait itself. The `Drop` trait also only has one method,
+/// `Drop::drop`, and that function is by fiat not callable in user code.
+/// So there is really no use case for using `Drop` in trait bounds.
+///
+/// The most likely use case of a drop bound is to distinguish between types
+/// that have destructors and types that don't. Combined with specialization,
+/// a naive coder would write an implementation that assumed a type could be
+/// trivially dropped, then write a specialization for `T: Drop` that actually
+/// calls the destructor. Except that doing so is not correct; String, for
+/// example, doesn't actually implement Drop, but because String contains a
+/// Vec, assuming it can be trivially dropped will leak memory.
+///
+/// **Known problems:** None.
+///
+/// **Example:**
+/// ```rust
+/// fn foo<T: Drop>() {}
+/// ```
+declare_clippy_lint! {
+    pub DROP_BOUNDS,
+    correctness,
+    "Bounds of the form `T: Drop` are useless"
+}
+
+const DROP_BOUNDS_SUMMARY: &str = "Bounds of the form `T: Drop` are useless. \
+                                   Use `std::mem::needs_drop` to detect if a type has drop glue.";
+
+pub struct Pass;
+
+impl LintPass for Pass {
+    fn get_lints(&self) -> LintArray {
+        lint_array!(DROP_BOUNDS)
+    }
+
+    fn name(&self) -> &'static str {
+        "DropBounds"
+    }
+}
+
+impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Pass {
+    fn check_generic_param(&mut self, cx: &rustc::lint::LateContext<'a, 'tcx>, p: &'tcx GenericParam) {
+        for bound in &p.bounds {
+            lint_bound(cx, bound);
+        }
+    }
+    fn check_where_predicate(&mut self, cx: &rustc::lint::LateContext<'a, 'tcx>, p: &'tcx WherePredicate) {
+        if let WherePredicate::BoundPredicate(WhereBoundPredicate { bounds, .. }) = p {
+            for bound in bounds {
+                lint_bound(cx, bound);
+            }
+        }
+    }
+}
+
+fn lint_bound<'a, 'tcx>(cx: &rustc::lint::LateContext<'a, 'tcx>, bound: &'tcx GenericBound) {
+    if_chain! {
+        if let GenericBound::Trait(t, _) = bound;
+        if let Some(def_id) = t.trait_ref.path.def.opt_def_id();
+        if match_def_path(cx.tcx, def_id, &paths::DROP_TRAIT);
+        then {
+            span_lint(
+                cx,
+                DROP_BOUNDS,
+                t.span,
+                DROP_BOUNDS_SUMMARY
+            );
+        }
+    }
+}

clippy_lints/src/enum_clike.rs

@@ -7,7 +7,7 @@ use rustc::hir::*;
 use rustc::lint::{LateContext, LateLintPass, LintArray, LintPass};
 use rustc::mir::interpret::GlobalId;
 use rustc::ty;
-use rustc::ty::subst::Substs;
+use rustc::ty::subst::InternalSubsts;
 use rustc::ty::util::IntTypeExt;
 use rustc::{declare_tool_lint, lint_array};
 use syntax::ast::{IntTy, UintTy};
@@ -58,7 +58,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for UnportableVariant {
                 if let Some(ref anon_const) = variant.disr_expr {
                     let param_env = ty::ParamEnv::empty();
                     let def_id = cx.tcx.hir().body_owner_def_id(anon_const.body);
-                    let substs = Substs::identity_for_item(cx.tcx.global_tcx(), def_id);
+                    let substs = InternalSubsts::identity_for_item(cx.tcx.global_tcx(), def_id);
                     let instance = ty::Instance::new(def_id, substs);
                     let c_id = GlobalId {
                         instance,