Merge branch 'master' into use_last

Author: HarrisonMc555

CHANGELOG.md

@@ -785,6 +785,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/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
   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/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/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/cyclomatic_complexity.rs

@@ -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;
@@ -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,9 +220,10 @@ fn report_cc_bug(
     shorts: u64,
     returns: u64,
     span: Span,
-    id: NodeId,
+    id: HirId,
 ) {
-    if !is_allowed(cx, CYCLOMATIC_COMPLEXITY, id) {
+    let node_id = cx.tcx.hir().hir_to_node_id(id);
+    if !is_allowed(cx, CYCLOMATIC_COMPLEXITY, node_id) {
         cx.sess().span_note_without_error(
             span,
             &format!(

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
+            );
+        }
+    }
+}