diff options
| -rw-r--r-- | CHANGELOG.md | 1 | ||||
| -rw-r--r-- | README.md | 6 | ||||
| -rw-r--r-- | clippy_lints/src/lib.rs | 3 | ||||
| -rw-r--r-- | clippy_lints/src/new_without_default.rs | 2 | ||||
| -rw-r--r-- | clippy_lints/src/types.rs | 61 | ||||
| -rw-r--r-- | clippy_lints/src/use_self.rs | 18 | ||||
| -rw-r--r-- | tests/ui/cast_ref_to_mut.rs | 31 | ||||
| -rw-r--r-- | tests/ui/cast_ref_to_mut.stderr | 22 | ||||
| -rw-r--r-- | tests/ui/use_self.rs | 39 | ||||
| -rw-r--r-- | tests/ui/use_self.stderr | 14 |
10 files changed, 173 insertions, 24 deletions
diff --git a/CHANGELOG.md b/CHANGELOG.md index 69d0350eb09..14c1b0e4d3f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -635,6 +635,7 @@ All notable changes to this project will be documented in this file. [`cast_possible_wrap`]: https://rust-lang.github.io/rust-clippy/master/index.html#cast_possible_wrap [`cast_precision_loss`]: https://rust-lang.github.io/rust-clippy/master/index.html#cast_precision_loss [`cast_ptr_alignment`]: https://rust-lang.github.io/rust-clippy/master/index.html#cast_ptr_alignment +[`cast_ref_to_mut`]: https://rust-lang.github.io/rust-clippy/master/index.html#cast_ref_to_mut [`cast_sign_loss`]: https://rust-lang.github.io/rust-clippy/master/index.html#cast_sign_loss [`char_lit_as_u8`]: https://rust-lang.github.io/rust-clippy/master/index.html#char_lit_as_u8 [`chars_last_cmp`]: https://rust-lang.github.io/rust-clippy/master/index.html#chars_last_cmp diff --git a/README.md b/README.md index 751f0add8eb..8ca10da416d 100644 --- a/README.md +++ b/README.md @@ -7,13 +7,7 @@ A collection of lints to catch common mistakes and improve your [Rust](https://github.com/rust-lang/rust) code. -<<<<<<< HEAD [There are 291 lints included in this crate!](https://rust-lang.github.io/rust-clippy/master/index.html) -||||||| merged common ancestors -[There are 291 lints included in this crate!](https://rust-lang.github.io/rust-clippy/master/index.html) -======= -[There are 291 lints included in this crate!](https://rust-lang.github.io/rust-clippy/master/index.html) ->>>>>>> run ./util/dev update_lints 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 index 192c9226b69..1c362313e77 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -487,6 +487,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry<'_>, conf: &Conf) { reg.register_late_lint_pass(box ptr_offset_with_cast::Pass); reg.register_late_lint_pass(box redundant_clone::RedundantClone); reg.register_late_lint_pass(box slow_vector_initialization::Pass); + reg.register_late_lint_pass(box types::RefToMut); reg.register_late_lint_pass(box assertions_on_constants::AssertionsOnConstants); reg.register_lint_group("clippy::restriction", Some("clippy_restriction"), vec /// implementation is required and also when it can be created with -/// `#[derive(Default)] +/// `#[derive(Default)]` /// /// **Why is this bad?** The user might expect to be able to use /// [`Default`](https://doc.rust-lang.org/std/default/trait.Default.html) as the diff --git a/clippy_lints/src/types.rs b/clippy_lints/src/types.rs index f4ca8209d67..f9ed38e52a0 100644 --- a/clippy_lints/src/types.rs +++ b/clippy_lints/src/types.rs @@ -2240,3 +2240,64 @@ impl<'a, 'b, 'tcx: 'a + 'b> Visitor<'tcx> for ImplicitHasherConstructorVisitor<' NestedVisitorMap::OnlyBodies(&self.cx.tcx.hir()) } } + +/// **What it does:** Checks for casts of `&T` to `&mut T` anywhere in the code. +/// +/// **Why is this bad?** It’s basically guaranteed to be undefined behaviour. +/// `UnsafeCell` is the only way to obtain aliasable data that is considered +/// mutable. +/// +/// **Known problems:** None. +/// +/// **Example:** +/// ```rust +/// fn x(r: &i32) { +/// unsafe { +/// *(r as *const _ as *mut _) += 1; +/// } +/// } +/// ``` +/// +/// Instead consider using interior mutability types. +/// +/// ```rust +/// fn x(r: &UnsafeCell<i32>) { +/// unsafe { +/// *r.get() += 1; +/// } +/// } +/// ``` +declare_clippy_lint! { + pub CAST_REF_TO_MUT, + correctness, + "a cast of reference to a mutable pointer" +} + +pub struct RefToMut; + +impl LintPass for RefToMut { + fn get_lints(&self) -> LintArray { + lint_array!(CAST_REF_TO_MUT) + } +} + +impl<'a, 'tcx> LateLintPass<'a, 'tcx> for RefToMut { + fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr) { + if_chain! { + if let ExprKind::Unary(UnOp::UnDeref, e) = &expr.node; + if let ExprKind::Cast(e, t) = &e.node; + if let TyKind::Ptr(MutTy { mutbl: Mutability::MutMutable, .. }) = t.node; + if let ExprKind::Cast(e, t) = &e.node; + if let TyKind::Ptr(MutTy { mutbl: Mutability::MutImmutable, .. }) = t.node; + if let ty::Ref(..) = cx.tables.node_id_to_type(e.hir_id).sty; + then { + span_lint( + cx, + CAST_REF_TO_MUT, + expr.span, + "casting &T to &mut T may cause undefined behaviour, consider instead using an UnsafeCell", + ); + } + } + } +} diff --git a/clippy_lints/src/use_self.rs b/clippy_lints/src/use_self.rs index aa4302e12a7..5ec809f1b76 100644 --- a/clippy_lints/src/use_self.rs +++ b/clippy_lints/src/use_self.rs @@ -10,13 +10,12 @@ use crate::utils::span_lint_and_sugg; use if_chain::if_chain; use rustc::hir::def::{CtorKind, Def}; -use rustc::hir::intravisit::{walk_path, walk_ty, NestedVisitorMap, Visitor}; +use rustc::hir::intravisit::{walk_item, walk_path, walk_ty, NestedVisitorMap, Visitor}; use rustc::hir::*; use rustc::lint::{in_external_macro, LateContext, LateLintPass, LintArray, LintContext, LintPass}; use rustc::ty; use rustc::{declare_tool_lint, lint_array}; use rustc_errors::Applicability; -use syntax::ast::NodeId; use syntax_pos::symbol::keywords::SelfUpper; /// **What it does:** Checks for unnecessary repetition of structure name when a @@ -29,7 +28,6 @@ use syntax_pos::symbol::keywords::SelfUpper; /// **Known problems:** /// - False positive when using associated types (#2843) /// - False positives in some situations when using generics (#3410) -/// - False positive when type from outer function can't be used (#3463) /// /// **Example:** /// ```rust @@ -242,8 +240,18 @@ impl<'a, 'tcx> Visitor<'tcx> for UseSelfVisitor<'a, 'tcx> { walk_path(self, path); } - fn visit_use(&mut self, _path: &'tcx Path, _id: NodeId, _hir_id: HirId) { - // Don't check use statements + fn visit_item(&mut self, item: &'tcx Item) { + match item.node { + ItemKind::Use(..) + | ItemKind::Static(..) + | ItemKind::Enum(..) + | ItemKind::Struct(..) + | ItemKind::Union(..) + | ItemKind::Impl(..) => { + // Don't check statements that shadow `Self` or where `Self` can't be used + }, + _ => walk_item(self, item), + } } fn nested_visit_map<'this>(&'this mut self) -> NestedVisitorMap<'this, 'tcx> { diff --git a/tests/ui/cast_ref_to_mut.rs b/tests/ui/cast_ref_to_mut.rs new file mode 100644 index 00000000000..967e8e46739 --- /dev/null +++ b/tests/ui/cast_ref_to_mut.rs @@ -0,0 +1,31 @@ +#![warn(clippy::cast_ref_to_mut)] +#![allow(clippy::no_effect)] + +extern "C" { + // NB. Mutability can be easily incorrect in FFI calls, as + // in C, the default are mutable pointers. + fn ffi(c: *mut u8); + fn int_ffi(c: *mut i32); +} + +fn main() { + let s = String::from("Hello"); + let a = &s; + unsafe { + let num = &3i32; + let mut_num = &mut 3i32; + // Should be warned against + (*(a as *const _ as *mut String)).push_str(" world"); + *(a as *const _ as *mut _) = String::from("Replaced"); + *(a as *const _ as *mut String) += " world"; + // Shouldn't be warned against + println!("{}", *(num as *const _ as *const i16)); + println!("{}", *(mut_num as *mut _ as *mut i16)); + ffi(a.as_ptr() as *mut _); + int_ffi(num as *const _ as *mut _); + int_ffi(&3 as *const _ as *mut _); + let mut value = 3; + let value: *const i32 = &mut value; + *(value as *const i16 as *mut i16) = 42; + } +} diff --git a/tests/ui/cast_ref_to_mut.stderr b/tests/ui/cast_ref_to_mut.stderr new file mode 100644 index 00000000000..448a66cfcce --- /dev/null +++ b/tests/ui/cast_ref_to_mut.stderr @@ -0,0 +1,22 @@ +error: casting &T to &mut T may cause undefined behaviour, consider instead using an UnsafeCell + --> $DIR/cast_ref_to_mut.rs:18:9 + | +LL | (*(a as *const _ as *mut String)).push_str(" world"); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: `-D clippy::cast-ref-to-mut` implied by `-D warnings` + +error: casting &T to &mut T may cause undefined behaviour, consider instead using an UnsafeCell + --> $DIR/cast_ref_to_mut.rs:19:9 + | +LL | *(a as *const _ as *mut _) = String::from("Replaced"); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: casting &T to &mut T may cause undefined behaviour, consider instead using an UnsafeCell + --> $DIR/cast_ref_to_mut.rs:20:9 + | +LL | *(a as *const _ as *mut String) += " world"; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: aborting due to 3 previous errors + diff --git a/tests/ui/use_self.rs b/tests/ui/use_self.rs index a01cb3e7021..a117ce5894b 100644 --- a/tests/ui/use_self.rs +++ b/tests/ui/use_self.rs @@ -242,6 +242,34 @@ mod macros { } } +mod nesting { + struct Foo {} + impl Foo { + fn foo() { + use self::Foo; // Can't use Self here + struct Bar { + foo: Foo, // Foo != Self + } + + impl Bar { + fn bar() -> Bar { + Bar { foo: Foo {} } + } + } + } + } + + enum Enum { + A, + } + impl Enum { + fn method() { + use self::Enum::*; // Issue 3425 + static STATIC: Enum = Enum::A; // Can't use Self as type + } + } +} + mod issue3410 { struct A; @@ -255,14 +283,3 @@ mod issue3410 { fn a(_: Vec<A>) {} } } - -mod issue3425 { - enum Enum { - A, - } - impl Enum { - fn a() { - use self::Enum::*; - } - } -} diff --git a/tests/ui/use_self.stderr b/tests/ui/use_self.stderr index 6c5dbf9111d..72b60db7fd2 100644 --- a/tests/ui/use_self.stderr +++ b/tests/ui/use_self.stderr @@ -150,5 +150,17 @@ LL | Foo {} LL | use_self_expand!(); // Should lint in local macros | ------------------- in this macro invocation -error: aborting due to 24 previous errors +error: unnecessary structure name repetition + --> $DIR/use_self.rs:255:29 + | +LL | fn bar() -> Bar { + | ^^^ help: use the applicable keyword: `Self` + +error: unnecessary structure name repetition + --> $DIR/use_self.rs:256:21 + | +LL | Bar { foo: Foo {} } + | ^^^ help: use the applicable keyword: `Self` + +error: aborting due to 26 previous errors |