diff --git a/CHANGELOG.md b/CHANGELOG.md index b72387f0d71..671450a120d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,8 @@ # Change Log All notable changes to this project will be documented in this file. +* New [`mut_from_ref`] lint + ## 0.0.114 — 2017-02-08 * Rustup to rustc 1.17.0-nightly (c49d10207 2017-02-07) * Tests are now ui tests (testing the exact output of rustc) @@ -369,6 +371,7 @@ All notable changes to this project will be documented in this file. [`mixed_case_hex_literals`]: https://github.com/Manishearth/rust-clippy/wiki#mixed_case_hex_literals [`module_inception`]: https://github.com/Manishearth/rust-clippy/wiki#module_inception [`modulo_one`]: https://github.com/Manishearth/rust-clippy/wiki#modulo_one +[`mut_from_ref`]: https://github.com/Manishearth/rust-clippy/wiki#mut_from_ref [`mut_mut`]: https://github.com/Manishearth/rust-clippy/wiki#mut_mut [`mutex_atomic`]: https://github.com/Manishearth/rust-clippy/wiki#mutex_atomic [`mutex_integer`]: https://github.com/Manishearth/rust-clippy/wiki#mutex_integer diff --git a/README.md b/README.md index 3f3ed135e0c..17282a7fc36 100644 --- a/README.md +++ b/README.md @@ -180,7 +180,7 @@ transparently: ## Lints -There are 186 lints included in this crate: +There are 187 lints included in this crate: name | default | triggers on -----------------------------------------------------------------------------------------------------------------------|---------|---------------------------------------------------------------------------------------------------------------------------------- @@ -278,6 +278,7 @@ name [mixed_case_hex_literals](https://github.com/Manishearth/rust-clippy/wiki#mixed_case_hex_literals) | warn | hex literals whose letter digits are not consistently upper- or lowercased [module_inception](https://github.com/Manishearth/rust-clippy/wiki#module_inception) | warn | modules that have the same name as their parent module [modulo_one](https://github.com/Manishearth/rust-clippy/wiki#modulo_one) | warn | taking a number modulo 1, which always returns 0 +[mut_from_ref](https://github.com/Manishearth/rust-clippy/wiki#mut_from_ref) | warn | fns that create mutable refs from immutable ref args [mut_mut](https://github.com/Manishearth/rust-clippy/wiki#mut_mut) | allow | usage of double-mut refs, e.g. `&mut &mut ...` [mutex_atomic](https://github.com/Manishearth/rust-clippy/wiki#mutex_atomic) | warn | using a mutex where an atomic value could be used instead [mutex_integer](https://github.com/Manishearth/rust-clippy/wiki#mutex_integer) | allow | using a mutex for an integer type diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 1099738d799..da8b04bdf6f 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -464,6 +464,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry) { precedence::PRECEDENCE, print::PRINT_WITH_NEWLINE, ptr::CMP_NULL, + ptr::MUT_FROM_REF, ptr::PTR_ARG, ranges::RANGE_STEP_BY_ZERO, ranges::RANGE_ZIP_WITH_LEN, diff --git a/clippy_lints/src/ptr.rs b/clippy_lints/src/ptr.rs index 590e3d587d4..7a7631d9769 100644 --- a/clippy_lints/src/ptr.rs +++ b/clippy_lints/src/ptr.rs @@ -44,13 +44,30 @@ declare_lint! { "comparing a pointer to a null pointer, suggesting to use `.is_null()` instead." } +/// **What it does:** This lint checks for functions that take immutable refs and return +/// mutable ones. +/// +/// **Why is this bad?** This is trivially unsound, as one can create two mutable refs +/// from the same source. +/// +/// **Known problems:** This lint will overlook functions where input and output lifetimes differ +/// +/// **Example:** +/// ```rust +/// fn foo(&Foo) -> &mut Bar { .. } +/// ``` +declare_lint! { + pub MUT_FROM_REF, + Warn, + "fns that create mutable refs from immutable ref args" +} #[derive(Copy,Clone)] pub struct PointerPass; impl LintPass for PointerPass { fn get_lints(&self) -> LintArray { - lint_array!(PTR_ARG, CMP_NULL) + lint_array!(PTR_ARG, CMP_NULL, MUT_FROM_REF) } } @@ -111,6 +128,28 @@ fn check_fn(cx: &LateContext, decl: &FnDecl, fn_id: NodeId) { } } } + + if let FunctionRetTy::Return(ref ty) = decl.output { + if let Some((out, MutMutable)) = get_rptr_lm(ty) { + if let Some(MutImmutable) = decl.inputs.iter() + .filter_map(|ty| get_rptr_lm(ty)) + .filter(|&(lt, _)| lt.name == out.name) + .fold(None, |x, (_, m)| match (x, m) { + (Some(MutMutable), _) | + (_, MutMutable) => Some(MutMutable), + (_, m) => Some(m), + }) { + span_lint(cx, + MUT_FROM_REF, + ty.span, + "this function takes an immutable ref to return a mutable one:"); + } + } + } +} + +fn get_rptr_lm(ty: &Ty) -> Option<(&Lifetime, Mutability)> { + if let Ty_::TyRptr(ref lt, ref m) = ty.node { Some((lt, m.mutbl)) } else { None } } fn is_null_path(expr: &Expr) -> bool { diff --git a/tests/ui/mut_from_ref.rs b/tests/ui/mut_from_ref.rs new file mode 100644 index 00000000000..09cc8a6061c --- /dev/null +++ b/tests/ui/mut_from_ref.rs @@ -0,0 +1,40 @@ +#![feature(plugin)] +#![plugin(clippy)] +#![allow(unused)] +#![deny(mut_from_ref)] + +struct Foo; + +impl Foo { + fn this_wont_hurt_a_bit(&self) -> &mut Foo { + unimplemented!() + } +} + +trait Ouch { + fn ouch(x: &Foo) -> &mut Foo; +} + +impl Ouch for Foo { + fn ouch(x: &Foo) -> &mut Foo { + unimplemented!() + } +} + +fn fail(x: &u32) -> &mut u16 { + unimplemented!() +} + +// this is OK, because the result borrows y +fn works<'a>(x: &u32, y: &'a mut u32) -> &'a mut u32 { + unimplemented!() +} + +// this is also OK, because the result could borrow y +fn also_works<'a>(x: &'a u32, y: &'a mut u32) -> &'a mut u32 { + unimplemented!() +} + +fn main() { + //TODO +} diff --git a/tests/ui/mut_from_ref.stderr b/tests/ui/mut_from_ref.stderr new file mode 100644 index 00000000000..23fc20d9a4b --- /dev/null +++ b/tests/ui/mut_from_ref.stderr @@ -0,0 +1,26 @@ +error: this function takes an immutable ref to return a mutable one: + --> $DIR/mut_from_ref.rs:9:39 + | +9 | fn this_wont_hurt_a_bit(&self) -> &mut Foo { + | ^^^^^^^^ + | +note: lint level defined here + --> $DIR/mut_from_ref.rs:4:9 + | +4 | #![deny(mut_from_ref)] + | ^^^^^^^^^^^^ + +error: this function takes an immutable ref to return a mutable one: + --> $DIR/mut_from_ref.rs:15:25 + | +15 | fn ouch(x: &Foo) -> &mut Foo; + | ^^^^^^^^ + +error: this function takes an immutable ref to return a mutable one: + --> $DIR/mut_from_ref.rs:24:21 + | +24 | fn fail(x: &u32) -> &mut u16 { + | ^^^^^^^^ + +error: aborting due to 3 previous errors +