Add lint for calling mem::discriminant on a non-enum type

HMPerson1 · HMPerson1 · commit aabf8083bd7e · 2018-10-24T23:39:54.000-04:00
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -744,6 +744,7 @@ All notable changes to this project will be documented in this file.
 [`match_same_arms`]: https://rust-lang-nursery.github.io/rust-clippy/master/index.html#match_same_arms
 [`match_wild_err_arm`]: https://rust-lang-nursery.github.io/rust-clippy/master/index.html#match_wild_err_arm
 [`maybe_infinite_iter`]: https://rust-lang-nursery.github.io/rust-clippy/master/index.html#maybe_infinite_iter
+[`mem_discriminant_non_enum`]: https://rust-lang-nursery.github.io/rust-clippy/master/index.html#mem_discriminant_non_enum
 [`mem_forget`]: https://rust-lang-nursery.github.io/rust-clippy/master/index.html#mem_forget
 [`mem_replace_option_with_none`]: https://rust-lang-nursery.github.io/rust-clippy/master/index.html#mem_replace_option_with_none
 [`min_max`]: https://rust-lang-nursery.github.io/rust-clippy/master/index.html#min_max
diff --git a/README.md b/README.md
@@ -9,7 +9,7 @@ We are currently in the process of discussing Clippy 1.0 via the RFC process in
 
 A collection of lints to catch common mistakes and improve your [Rust](https://github.com/rust-lang/rust) code.
 
-[There are 281 lints included in this crate!](https://rust-lang-nursery.github.io/rust-clippy/master/index.html)
+[There are 282 lints included in this crate!](https://rust-lang-nursery.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:
 
diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs
@@ -144,6 +144,7 @@ pub mod loops;
 pub mod map_clone;
 pub mod map_unit_fn;
 pub mod matches;
+pub mod mem_discriminant;
 pub mod mem_forget;
 pub mod mem_replace;
 pub mod methods;
@@ -398,6 +399,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry<'_>, conf: &Conf) {
     reg.register_early_lint_pass(box doc::Doc::new(conf.doc_valid_idents.clone()));
     reg.register_late_lint_pass(box neg_multiply::NegMultiply);
     reg.register_early_lint_pass(box unsafe_removed_from_name::UnsafeNameRemoval);
+    reg.register_late_lint_pass(box mem_discriminant::MemDiscriminant);
     reg.register_late_lint_pass(box mem_forget::MemForget);
     reg.register_late_lint_pass(box mem_replace::MemReplace);
     reg.register_late_lint_pass(box arithmetic::Arithmetic::default());
@@ -612,6 +614,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry<'_>, conf: &Conf) {
         matches::MATCH_REF_PATS,
         matches::MATCH_WILD_ERR_ARM,
         matches::SINGLE_MATCH,
+        mem_discriminant::MEM_DISCRIMINANT_NON_ENUM,
         mem_replace::MEM_REPLACE_OPTION_WITH_NONE,
         methods::CHARS_LAST_CMP,
         methods::CHARS_NEXT_CMP,
@@ -924,6 +927,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry<'_>, conf: &Conf) {
         loops::NEVER_LOOP,
         loops::REVERSE_RANGE_LOOP,
         loops::WHILE_IMMUTABLE_CONDITION,
+        mem_discriminant::MEM_DISCRIMINANT_NON_ENUM,
         methods::CLONE_DOUBLE_REF,
         methods::TEMPORARY_CSTRING_AS_PTR,
         minmax::MIN_MAX,
diff --git a/clippy_lints/src/mem_discriminant.rs b/clippy_lints/src/mem_discriminant.rs
@@ -0,0 +1,93 @@
+// Copyright 2014-2018 The Rust Project Developers. See the COPYRIGHT
+// file at the top-level directory of this distribution.
+//
+// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
+// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
+// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
+// option. This file may not be copied, modified, or distributed
+// except according to those terms.
+
+
+use crate::rustc::hir::{Expr, ExprKind};
+use crate::rustc::lint::{LateContext, LateLintPass, LintArray, LintPass};
+use crate::rustc::{declare_tool_lint, lint_array};
+use crate::utils::{match_def_path, opt_def_id, paths, snippet, span_lint_and_then, walk_ptrs_ty_depth};
+use if_chain::if_chain;
+
+use std::iter;
+
+/// **What it does:** Checks for calls of `mem::discriminant()` on a non-enum type.
+///
+/// **Why is this bad?** The value of `mem::discriminant()` on non-enum types
+/// is unspecified.
+///
+/// **Known problems:** None.
+///
+/// **Example:**
+/// ```rust
+/// mem::discriminant(&"hello");
+/// mem::discriminant(&&Some(2));
+/// ```
+declare_clippy_lint! {
+    pub MEM_DISCRIMINANT_NON_ENUM,
+    correctness,
+    "calling mem::descriminant on non-enum type"
+}
+
+pub struct MemDiscriminant;
+
+impl LintPass for MemDiscriminant {
+    fn get_lints(&self) -> LintArray {
+        lint_array![MEM_DISCRIMINANT_NON_ENUM]
+    }
+}
+
+impl<'a, 'tcx> LateLintPass<'a, 'tcx> for MemDiscriminant {
+    fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr) {
+        if_chain! {
+            if let ExprKind::Call(ref func, ref func_args) = expr.node;
+            // is `mem::discriminant`
+            if let ExprKind::Path(ref func_qpath) = func.node;
+            if let Some(def_id) = opt_def_id(cx.tables.qpath_def(func_qpath, func.hir_id));
+            if match_def_path(cx.tcx, def_id, &paths::MEM_DISCRIMINANT);
+            // type is non-enum
+            let ty_param = cx.tables.node_substs(func.hir_id).type_at(0);
+            if !ty_param.is_enum();
+
+            then {
+                span_lint_and_then(
+                    cx,
+                    MEM_DISCRIMINANT_NON_ENUM,
+                    expr.span,
+                    &format!("calling `mem::discriminant` on non-enum type `{}`", ty_param),
+                    |db| {
+                        // if this is a reference to an enum, suggest dereferencing
+                        let (base_ty, ptr_depth) = walk_ptrs_ty_depth(ty_param);
+                        if ptr_depth >= 1 && base_ty.is_enum() {
+                            let param = &func_args[0];
+
+                            // cancel out '&'s first
+                            let mut derefs_needed = ptr_depth;
+                            let mut cur_expr = param;
+                            while derefs_needed > 0  {
+                                if let ExprKind::AddrOf(_, ref inner_expr) = cur_expr.node {
+                                    derefs_needed -= 1;
+                                    cur_expr = inner_expr;
+                                } else {
+                                    break;
+                                }
+                            }
+
+                            let derefs: String = iter::repeat('*').take(derefs_needed).collect();
+                            db.span_suggestion(
+                                param.span,
+                                "try dereferencing",
+                                format!("{}{}", derefs, snippet(cx, cur_expr.span, "<param>")),
+                            );
+                        }
+                    },
+                )
+            }
+        }
+    }
+}
diff --git a/clippy_lints/src/utils/paths.rs b/clippy_lints/src/utils/paths.rs
@@ -55,6 +55,7 @@ pub const LATE_CONTEXT: [&str; 4] = ["rustc", "lint", "context", "LateContext"];
 pub const LINKED_LIST: [&str; 4] = ["alloc", "collections", "linked_list", "LinkedList"];
 pub const LINT: [&str; 3] = ["rustc", "lint", "Lint"];
 pub const LINT_ARRAY: [&str; 3] = ["rustc", "lint", "LintArray"];
+pub const MEM_DISCRIMINANT: [&str; 3] = ["core", "mem", "discriminant"];
 pub const MEM_FORGET: [&str; 3] = ["core", "mem", "forget"];
 pub const MEM_REPLACE: [&str; 3] = ["core", "mem", "replace"];
 pub const MEM_UNINIT: [&str; 3] = ["core", "mem", "uninitialized"];
diff --git a/tests/ui/mem_discriminant.rs b/tests/ui/mem_discriminant.rs
@@ -0,0 +1,44 @@
+// Copyright 2014-2018 The Rust Project Developers. See the COPYRIGHT
+// file at the top-level directory of this distribution.
+//
+// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
+// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
+// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
+// option. This file may not be copied, modified, or distributed
+// except according to those terms.
+
+
+#![deny(clippy::mem_discriminant_non_enum)]
+
+use std::mem;
+
+enum Foo {
+    One(usize),
+    Two(u8),
+}
+
+struct A(Foo);
+
+fn main() {
+    // bad
+    mem::discriminant(&"hello");
+    mem::discriminant(&&Some(2));
+    mem::discriminant(&&None::<u8>);
+    mem::discriminant(&&Foo::One(5));
+    mem::discriminant(&&Foo::Two(5));
+    mem::discriminant(&A(Foo::One(0)));
+
+    let ro = &Some(3);
+    let rro = &ro;
+    mem::discriminant(&ro);
+    mem::discriminant(rro);
+    mem::discriminant(&rro);
+
+
+    // ok
+    mem::discriminant(&Some(2));
+    mem::discriminant(&None::<u8>);
+    mem::discriminant(&Foo::One(5));
+    mem::discriminant(&Foo::Two(5));
+    mem::discriminant(ro);
+}
diff --git a/tests/ui/mem_discriminant.stderr b/tests/ui/mem_discriminant.stderr
@@ -0,0 +1,76 @@
+error: calling `mem::discriminant` on non-enum type `&str`
+  --> $DIR/mem_discriminant.rs:24:5
+   |
+24 |     mem::discriminant(&"hello");
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+note: lint level defined here
+  --> $DIR/mem_discriminant.rs:11:9
+   |
+11 | #![deny(clippy::mem_discriminant_non_enum)]
+   |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+error: calling `mem::discriminant` on non-enum type `&std::option::Option<i32>`
+  --> $DIR/mem_discriminant.rs:25:5
+   |
+25 |     mem::discriminant(&&Some(2));
+   |     ^^^^^^^^^^^^^^^^^^---------^
+   |                       |
+   |                       help: try dereferencing: `&Some(2)`
+
+error: calling `mem::discriminant` on non-enum type `&std::option::Option<u8>`
+  --> $DIR/mem_discriminant.rs:26:5
+   |
+26 |     mem::discriminant(&&None::<u8>);
+   |     ^^^^^^^^^^^^^^^^^^------------^
+   |                       |
+   |                       help: try dereferencing: `&None::<u8>`
+
+error: calling `mem::discriminant` on non-enum type `&Foo`
+  --> $DIR/mem_discriminant.rs:27:5
+   |
+27 |     mem::discriminant(&&Foo::One(5));
+   |     ^^^^^^^^^^^^^^^^^^-------------^
+   |                       |
+   |                       help: try dereferencing: `&Foo::One(5)`
+
+error: calling `mem::discriminant` on non-enum type `&Foo`
+  --> $DIR/mem_discriminant.rs:28:5
+   |
+28 |     mem::discriminant(&&Foo::Two(5));
+   |     ^^^^^^^^^^^^^^^^^^-------------^
+   |                       |
+   |                       help: try dereferencing: `&Foo::Two(5)`
+
+error: calling `mem::discriminant` on non-enum type `A`
+  --> $DIR/mem_discriminant.rs:29:5
+   |
+29 |     mem::discriminant(&A(Foo::One(0)));
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+error: calling `mem::discriminant` on non-enum type `&std::option::Option<i32>`
+  --> $DIR/mem_discriminant.rs:33:5
+   |
+33 |     mem::discriminant(&ro);
+   |     ^^^^^^^^^^^^^^^^^^---^
+   |                       |
+   |                       help: try dereferencing: `ro`
+
+error: calling `mem::discriminant` on non-enum type `&std::option::Option<i32>`
+  --> $DIR/mem_discriminant.rs:34:5
+   |
+34 |     mem::discriminant(rro);
+   |     ^^^^^^^^^^^^^^^^^^---^
+   |                       |
+   |                       help: try dereferencing: `*rro`
+
+error: calling `mem::discriminant` on non-enum type `&&std::option::Option<i32>`
+  --> $DIR/mem_discriminant.rs:35:5
+   |
+35 |     mem::discriminant(&rro);
+   |     ^^^^^^^^^^^^^^^^^^----^
+   |                       |
+   |                       help: try dereferencing: `*rro`
+
+error: aborting due to 9 previous errors
+
