diff --git a/README.md b/README.md index 6199ccd8e..95ba6b085 100644 --- a/README.md +++ b/README.md @@ -14,7 +14,7 @@ Table of contents: * [License](#license) ##Lints -There are 135 lints included in this crate: +There are 136 lints included in this crate: name | default | meaning ---------------------------------------------------------------------------------------------------------------------|---------|------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ @@ -133,6 +133,7 @@ name [temporary_assignment](https://github.com/Manishearth/rust-clippy/wiki#temporary_assignment) | warn | assignments to temporaries [too_many_arguments](https://github.com/Manishearth/rust-clippy/wiki#too_many_arguments) | warn | functions with too many arguments [toplevel_ref_arg](https://github.com/Manishearth/rust-clippy/wiki#toplevel_ref_arg) | warn | An entire binding was declared as `ref`, in a function argument (`fn foo(ref x: Bar)`), or a `let` statement (`let ref x = foo()`). In such cases, it is preferred to take references with `&`. +[transmute_ptr_to_ref](https://github.com/Manishearth/rust-clippy/wiki#transmute_ptr_to_ref) | warn | transmutes from a pointer to a reference type [trivial_regex](https://github.com/Manishearth/rust-clippy/wiki#trivial_regex) | warn | finds trivial regular expressions in `Regex::new(_)` invocations [type_complexity](https://github.com/Manishearth/rust-clippy/wiki#type_complexity) | warn | usage of very complex types; recommends factoring out parts into `type` definitions [unicode_not_nfc](https://github.com/Manishearth/rust-clippy/wiki#unicode_not_nfc) | allow | using a unicode literal not in NFC normal form (see http://www.unicode.org/reports/tr15/ for further information) diff --git a/src/lib.rs b/src/lib.rs index 8a4a84f5d..7d24b2c89 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -195,8 +195,7 @@ pub fn plugin_registrar(reg: &mut Registry) { reg.register_late_lint_pass(box no_effect::NoEffectPass); reg.register_late_lint_pass(box map_clone::MapClonePass); reg.register_late_lint_pass(box temporary_assignment::TemporaryAssignmentPass); - reg.register_late_lint_pass(box transmute::CrosspointerTransmute); - reg.register_late_lint_pass(box transmute::UselessTransmute); + reg.register_late_lint_pass(box transmute::Transmute); reg.register_late_lint_pass(box cyclomatic_complexity::CyclomaticComplexity::new(conf.cyclomatic_complexity_threshold)); reg.register_late_lint_pass(box escape::EscapePass); reg.register_early_lint_pass(box misc_early::MiscEarly); @@ -352,6 +351,7 @@ pub fn plugin_registrar(reg: &mut Registry) { swap::MANUAL_SWAP, temporary_assignment::TEMPORARY_ASSIGNMENT, transmute::CROSSPOINTER_TRANSMUTE, + transmute::TRANSMUTE_PTR_TO_REF, transmute::USELESS_TRANSMUTE, types::ABSURD_EXTREME_COMPARISONS, types::BOX_VEC, diff --git a/src/misc.rs b/src/misc.rs index 49426d5d5..5a787ba6d 100644 --- a/src/misc.rs +++ b/src/misc.rs @@ -312,7 +312,7 @@ impl LateLintPass for ModuloOne { if let ExprBinary(ref cmp, _, ref right) = expr.node { if let Spanned {node: BinOp_::BiRem, ..} = *cmp { if is_integer_literal(right, 1) { - cx.span_lint(MODULO_ONE, expr.span, "any number modulo 1 will be 0"); + span_lint(cx, MODULO_ONE, expr.span, "any number modulo 1 will be 0"); } } } @@ -347,11 +347,12 @@ impl LateLintPass for PatternPass { fn check_pat(&mut self, cx: &LateContext, pat: &Pat) { if let PatKind::Ident(_, ref ident, Some(ref right)) = pat.node { if right.node == PatKind::Wild { - cx.span_lint(REDUNDANT_PATTERN, - pat.span, - &format!("the `{} @ _` pattern can be written as just `{}`", - ident.node.name, - ident.node.name)); + span_lint(cx, + REDUNDANT_PATTERN, + pat.span, + &format!("the `{} @ _` pattern can be written as just `{}`", + ident.node.name, + ident.node.name)); } } } @@ -408,10 +409,11 @@ impl LateLintPass for UsedUnderscoreBinding { _ => false, }; if needs_lint { - cx.span_lint(USED_UNDERSCORE_BINDING, - expr.span, - "used binding which is prefixed with an underscore. A leading underscore signals that a \ - binding will not be used."); + span_lint(cx, + USED_UNDERSCORE_BINDING, + expr.span, + "used binding which is prefixed with an underscore. A leading underscore signals that a \ + binding will not be used."); } } } diff --git a/src/ranges.rs b/src/ranges.rs index 766d98b4e..23bd3d110 100644 --- a/src/ranges.rs +++ b/src/ranges.rs @@ -1,7 +1,7 @@ use rustc::lint::*; use rustc_front::hir::*; use syntax::codemap::Spanned; -use utils::{is_integer_literal, match_type, snippet, unsugar_range, UnsugaredRange}; +use utils::{is_integer_literal, match_type, snippet, span_lint, unsugar_range, UnsugaredRange}; /// **What it does:** This lint checks for iterating over ranges with a `.step_by(0)`, which never terminates. /// @@ -41,10 +41,11 @@ impl LateLintPass for StepByZero { // Range with step_by(0). if name.as_str() == "step_by" && args.len() == 2 && is_range(cx, &args[0]) && is_integer_literal(&args[1], 0) { - cx.span_lint(RANGE_STEP_BY_ZERO, - expr.span, - "Range::step_by(0) produces an infinite iterator. Consider using `std::iter::repeat()` \ - instead") + span_lint(cx, + RANGE_STEP_BY_ZERO, + expr.span, + "Range::step_by(0) produces an infinite iterator. Consider using `std::iter::repeat()` \ + instead"); } else if name.as_str() == "zip" && args.len() == 2 { let iter = &args[0].node; let zip_arg = &args[1]; @@ -64,9 +65,11 @@ impl LateLintPass for StepByZero { let ExprPath(_, Path { segments: ref len_path, .. }) = len_args[0].node, iter_path == len_path ], { - cx.span_lint(RANGE_ZIP_WITH_LEN, expr.span, - &format!("It is more idiomatic to use {}.iter().enumerate()", - snippet(cx, iter_args[0].span, "_"))); + span_lint(cx, + RANGE_ZIP_WITH_LEN, + expr.span, + &format!("It is more idiomatic to use {}.iter().enumerate()", + snippet(cx, iter_args[0].span, "_"))); } } } diff --git a/src/transmute.rs b/src/transmute.rs index 8689a7e36..ef049ba4a 100644 --- a/src/transmute.rs +++ b/src/transmute.rs @@ -1,9 +1,9 @@ use rustc::lint::*; +use rustc::ty::TypeVariants::{TyRawPtr, TyRef}; +use rustc::ty; use rustc_front::hir::*; -use rustc::ty::TyS; -use rustc::ty::TypeVariants::TyRawPtr; -use utils; use utils::TRANSMUTE_PATH; +use utils::{match_def_path, snippet_opt, span_lint, span_lint_and_then}; /// **What it does:** This lint checks for transmutes to the original type of the object. /// @@ -31,28 +31,63 @@ declare_lint! { "transmutes that have to or from types that are a pointer to the other" } -pub struct UselessTransmute; +/// **What it does:*** This lint checks for transmutes from a pointer to a reference. +/// +/// **Why is this bad?** This can always be rewritten with `&` and `*`. +/// +/// **Known problems:** None. +/// +/// **Example:** +/// ```rust +/// let _: &T = std::mem::transmute(p); // where p: *const T +/// // can be written: +/// let _: &T = &*p; +/// ``` +declare_lint! { + pub TRANSMUTE_PTR_TO_REF, + Warn, + "transmutes from a pointer to a reference type" +} -impl LintPass for UselessTransmute { +pub struct Transmute; + +impl LintPass for Transmute { fn get_lints(&self) -> LintArray { - lint_array!(USELESS_TRANSMUTE) + lint_array! [ + CROSSPOINTER_TRANSMUTE, + TRANSMUTE_PTR_TO_REF, + USELESS_TRANSMUTE + ] } } -impl LateLintPass for UselessTransmute { +impl LateLintPass for Transmute { fn check_expr(&mut self, cx: &LateContext, e: &Expr) { if let ExprCall(ref path_expr, ref args) = e.node { if let ExprPath(None, _) = path_expr.node { let def_id = cx.tcx.def_map.borrow()[&path_expr.id].def_id(); - if utils::match_def_path(cx, def_id, &TRANSMUTE_PATH) { + if match_def_path(cx, def_id, &TRANSMUTE_PATH) { let from_ty = cx.tcx.expr_ty(&args[0]); let to_ty = cx.tcx.expr_ty(e); if from_ty == to_ty { - cx.span_lint(USELESS_TRANSMUTE, - e.span, - &format!("transmute from a type (`{}`) to itself", from_ty)); + span_lint(cx, + USELESS_TRANSMUTE, + e.span, + &format!("transmute from a type (`{}`) to itself", from_ty)); + } else if is_ptr_to(to_ty, from_ty) { + span_lint(cx, + CROSSPOINTER_TRANSMUTE, + e.span, + &format!("transmute from a type (`{}`) to a pointer to that type (`{}`)", from_ty, to_ty)); + } else if is_ptr_to(from_ty, to_ty) { + span_lint(cx, + CROSSPOINTER_TRANSMUTE, + e.span, + &format!("transmute from a type (`{}`) to the type that it points to (`{}`)", from_ty, to_ty)); + } else { + check_ptr_to_ref(cx, from_ty, to_ty, e, &args[0]); } } } @@ -60,15 +95,7 @@ impl LateLintPass for UselessTransmute { } } -pub struct CrosspointerTransmute; - -impl LintPass for CrosspointerTransmute { - fn get_lints(&self) -> LintArray { - lint_array!(CROSSPOINTER_TRANSMUTE) - } -} - -fn is_ptr_to(from: &TyS, to: &TyS) -> bool { +fn is_ptr_to(from: ty::Ty, to: ty::Ty) -> bool { if let TyRawPtr(from_ptr) = from.sty { from_ptr.ty == to } else { @@ -76,27 +103,34 @@ fn is_ptr_to(from: &TyS, to: &TyS) -> bool { } } -impl LateLintPass for CrosspointerTransmute { - fn check_expr(&mut self, cx: &LateContext, e: &Expr) { - if let ExprCall(ref path_expr, ref args) = e.node { - if let ExprPath(None, _) = path_expr.node { - let def_id = cx.tcx.def_map.borrow()[&path_expr.id].def_id(); +fn check_ptr_to_ref<'tcx>(cx: &LateContext, + from_ty: ty::Ty<'tcx>, + to_ty: ty::Ty<'tcx>, + e: &Expr, arg: &Expr) { + if let TyRawPtr(ref from_pty) = from_ty.sty { + if let TyRef(_, ref to_rty) = to_ty.sty { + let mess = format!("transmute from a pointer type (`{}`) to a reference type (`{}`)", + from_ty, + to_ty); + span_lint_and_then(cx, TRANSMUTE_PTR_TO_REF, e.span, &mess, |db| { + if let Some(arg) = snippet_opt(cx, arg.span) { + let (deref, cast) = if to_rty.mutbl == Mutability::MutMutable { + ("&mut *", "*mut") + } else { + ("&*", "*const") + }; - if utils::match_def_path(cx, def_id, &TRANSMUTE_PATH) { - let from_ty = cx.tcx.expr_ty(&args[0]); - let to_ty = cx.tcx.expr_ty(e); - if is_ptr_to(to_ty, from_ty) { - cx.span_lint(CROSSPOINTER_TRANSMUTE, - e.span, - &format!("transmute from a type (`{}`) to a pointer to that type (`{}`)", from_ty, to_ty)); - } else if is_ptr_to(from_ty, to_ty) { - cx.span_lint(CROSSPOINTER_TRANSMUTE, - e.span, - &format!("transmute from a type (`{}`) to the type that it points to (`{}`)", from_ty, to_ty)); + let sugg = if from_pty.ty == to_rty.ty { + format!("{}{}", deref, arg) } + else { + format!("{}({} as {} {})", deref, arg, cast, to_rty.ty) + }; + + db.span_suggestion(e.span, "try", sugg); } - } + }); } } } diff --git a/tests/compile-fail/transmute.rs b/tests/compile-fail/transmute.rs index b87550080..5bae2c726 100644 --- a/tests/compile-fail/transmute.rs +++ b/tests/compile-fail/transmute.rs @@ -19,6 +19,45 @@ unsafe fn _generic<'a, T, U: 'a>(t: &'a T) { let _: &'a U = core::intrinsics::transmute(t); } +#[deny(transmute_ptr_to_ref)] +unsafe fn _ptr_to_ref(p: *const T, m: *mut T, o: *const U, om: *mut U) { + let _: &T = std::mem::transmute(p); + //~^ ERROR transmute from a pointer type (`*const T`) to a reference type (`&T`) + //~| HELP try + //~| SUGGESTION = &*p; + let _: &T = &*p; + + let _: &mut T = std::mem::transmute(m); + //~^ ERROR transmute from a pointer type (`*mut T`) to a reference type (`&mut T`) + //~| HELP try + //~| SUGGESTION = &mut *m; + let _: &mut T = &mut *m; + + let _: &T = std::mem::transmute(m); + //~^ ERROR transmute from a pointer type (`*mut T`) to a reference type (`&T`) + //~| HELP try + //~| SUGGESTION = &*m; + let _: &T = &*m; + + let _: &T = std::mem::transmute(o); + //~^ ERROR transmute from a pointer type (`*const U`) to a reference type (`&T`) + //~| HELP try + //~| SUGGESTION = &*(o as *const T); + let _: &T = &*(o as *const T); + + let _: &mut T = std::mem::transmute(om); + //~^ ERROR transmute from a pointer type (`*mut U`) to a reference type (`&mut T`) + //~| HELP try + //~| SUGGESTION = &mut *(om as *mut T); + let _: &mut T = &mut *(om as *mut T); + + let _: &T = std::mem::transmute(om); + //~^ ERROR transmute from a pointer type (`*mut U`) to a reference type (`&T`) + //~| HELP try + //~| SUGGESTION = &*(om as *const T); + let _: &T = &*(om as *const T); +} + #[deny(useless_transmute)] fn useless() { unsafe {