diff --git a/CHANGELOG.md b/CHANGELOG.md index 69694da15203..005de8d57b03 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1318,6 +1318,7 @@ Released 2018-09-13 [`try_err`]: https://rust-lang.github.io/rust-clippy/master/index.html#try_err [`type_complexity`]: https://rust-lang.github.io/rust-clippy/master/index.html#type_complexity [`type_repetition_in_bounds`]: https://rust-lang.github.io/rust-clippy/master/index.html#type_repetition_in_bounds +[`unbound_return_lifetimes`]: https://rust-lang.github.io/rust-clippy/master/index.html#unbound_return_lifetimes [`unicode_not_nfc`]: https://rust-lang.github.io/rust-clippy/master/index.html#unicode_not_nfc [`unimplemented`]: https://rust-lang.github.io/rust-clippy/master/index.html#unimplemented [`uninit_assumed_init`]: https://rust-lang.github.io/rust-clippy/master/index.html#uninit_assumed_init diff --git a/README.md b/README.md index 4de256e9e72f..7b1d14e9afca 100644 --- a/README.md +++ b/README.md @@ -6,7 +6,7 @@ A collection of lints to catch common mistakes and improve your [Rust](https://github.com/rust-lang/rust) code. -[There are 346 lints included in this crate!](https://rust-lang.github.io/rust-clippy/master/index.html) +[There are 347 lints included in this crate!](https://rust-lang.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 index 1a112c0d3709..d8e5231676ed 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -296,6 +296,7 @@ pub mod transmuting_null; pub mod trivially_copy_pass_by_ref; pub mod try_err; pub mod types; +pub mod unbound_return_lifetimes; pub mod unicode; pub mod unsafe_removed_from_name; pub mod unused_io_amount; @@ -786,6 +787,7 @@ pub fn register_plugins(store: &mut lint::LintStore, sess: &Session, conf: &Conf &types::UNIT_CMP, &types::UNNECESSARY_CAST, &types::VEC_BOX, + &unbound_return_lifetimes::UNBOUND_RETURN_LIFETIMES, &unicode::NON_ASCII_LITERAL, &unicode::UNICODE_NOT_NFC, &unicode::ZERO_WIDTH_SPACE, @@ -953,6 +955,7 @@ pub fn register_plugins(store: &mut lint::LintStore, sess: &Session, conf: &Conf store.register_late_pass(|| box mul_add::MulAddCheck); store.register_late_pass(|| box mut_key::MutableKeyType); store.register_late_pass(|| box modulo_arithmetic::ModuloArithmetic); + store.register_late_pass(|| box unbound_return_lifetimes::UnboundReturnLifetimes); store.register_early_pass(|| box reference::DerefAddrOf); store.register_early_pass(|| box reference::RefInDeref); store.register_early_pass(|| box double_parens::DoubleParens); @@ -1326,6 +1329,7 @@ pub fn register_plugins(store: &mut lint::LintStore, sess: &Session, conf: &Conf LintId::of(&types::UNIT_CMP), LintId::of(&types::UNNECESSARY_CAST), LintId::of(&types::VEC_BOX), + LintId::of(&unbound_return_lifetimes::UNBOUND_RETURN_LIFETIMES), LintId::of(&unicode::ZERO_WIDTH_SPACE), LintId::of(&unsafe_removed_from_name::UNSAFE_REMOVED_FROM_NAME), LintId::of(&unused_io_amount::UNUSED_IO_AMOUNT), @@ -1577,6 +1581,7 @@ pub fn register_plugins(store: &mut lint::LintStore, sess: &Session, conf: &Conf LintId::of(&types::CAST_PTR_ALIGNMENT), LintId::of(&types::CAST_REF_TO_MUT), LintId::of(&types::UNIT_CMP), + LintId::of(&unbound_return_lifetimes::UNBOUND_RETURN_LIFETIMES), LintId::of(&unicode::ZERO_WIDTH_SPACE), LintId::of(&unused_io_amount::UNUSED_IO_AMOUNT), LintId::of(&unwrap::PANICKING_UNWRAP), diff --git a/clippy_lints/src/unbound_return_lifetimes.rs b/clippy_lints/src/unbound_return_lifetimes.rs new file mode 100644 index 000000000000..89167d44ef5f --- /dev/null +++ b/clippy_lints/src/unbound_return_lifetimes.rs @@ -0,0 +1,185 @@ +use if_chain::if_chain; +use rustc::declare_lint_pass; +use rustc::hir::map::Map; +use rustc::lint::{LateContext, LateLintPass, LintArray, LintPass}; +use rustc_hir::intravisit::*; +use rustc_hir::*; +use rustc_session::declare_tool_lint; + +use crate::utils::span_lint; + +declare_clippy_lint! { + /// **What it does:** Checks for lifetime annotations on return values + /// which might not always get bound. + /// + /// **Why is this bad?** If the function contains unsafe code which + /// transmutes to the lifetime, the resulting lifetime may live longer + /// than was intended. + /// + /// **Known problems*: None. + /// + /// **Example:** + /// ```rust + /// struct WrappedStr(str); + /// + /// // Bad: unbound return lifetime causing unsoundness (e.g. when x is String) + /// fn unbound<'a>(x: impl AsRef + 'a) -> &'a WrappedStr { + /// let s = x.as_ref(); + /// unsafe { &*(s as *const str as *const WrappedStr) } + /// } + /// + /// // Good: bound return lifetime is sound + /// fn bound<'a>(x: &'a str) -> &'a WrappedStr { + /// unsafe { &*(x as *const str as *const WrappedStr) } + /// } + /// ``` + pub UNBOUND_RETURN_LIFETIMES, + correctness, + "unbound lifetimes in function return values" +} + +declare_lint_pass!(UnboundReturnLifetimes => [UNBOUND_RETURN_LIFETIMES]); + +impl<'a, 'tcx> LateLintPass<'a, 'tcx> for UnboundReturnLifetimes { + fn check_item(&mut self, cx: &LateContext<'a, 'tcx>, item: &'tcx Item<'tcx>) { + if let ItemKind::Fn(ref sig, ref generics, _id) = item.kind { + check_fn_inner(cx, &sig.decl, generics, None); + } + } + + fn check_impl_item(&mut self, cx: &LateContext<'a, 'tcx>, item: &'tcx ImplItem<'tcx>) { + if let ImplItemKind::Method(ref sig, _) = item.kind { + let parent_generics = impl_generics(cx, item.hir_id); + check_fn_inner(cx, &sig.decl, &item.generics, parent_generics); + } + } +} + +fn check_fn_inner<'a, 'tcx>( + cx: &LateContext<'a, 'tcx>, + decl: &'tcx FnDecl<'tcx>, + generics: &'tcx Generics<'tcx>, + parent_data: Option<(&'tcx Generics<'tcx>, &'tcx Ty<'tcx>)>, +) { + let output_type = if let FunctionRetTy::Return(ref output_type) = decl.output { + output_type + } else { + return; + }; + + let lt = if let TyKind::Rptr(ref lt, _) = output_type.kind { + lt + } else { + return; + }; + + if matches!(lt.name, LifetimeName::Param(_)) { + let target_lt = lt; + + // check function generics + // the target lifetime parameter must appear on the left of some outlives relation + if lifetime_outlives_something(target_lt, generics) { + return; + } + + // check parent generics + // the target lifetime parameter must appear on the left of some outlives relation + if let Some((ref parent_generics, _)) = parent_data { + if lifetime_outlives_something(target_lt, parent_generics) { + return; + } + } + + // check type generics + // the target lifetime parameter must be included in the type + if let Some((_, ref parent_ty)) = parent_data { + if TypeVisitor::type_contains_lifetime(parent_ty, target_lt) { + return; + } + } + + // check arguments the target lifetime parameter must be included as a + // lifetime of a reference, either directly or through the gneric + // parameters of the argument type. + for input in decl.inputs.iter() { + if TypeVisitor::type_contains_lifetime(input, target_lt) { + return; + } + } + + span_lint( + cx, + UNBOUND_RETURN_LIFETIMES, + target_lt.span, + "lifetime is unconstrained", + ); + } +} + +struct TypeVisitor<'tcx> { + found: bool, + target_lt: &'tcx Lifetime, +} + +impl<'tcx> TypeVisitor<'tcx> { + fn type_contains_lifetime(ty: &Ty<'_>, target_lt: &'tcx Lifetime) -> bool { + let mut visitor = TypeVisitor { + found: false, + target_lt, + }; + walk_ty(&mut visitor, ty); + visitor.found + } +} + +impl<'tcx> Visitor<'tcx> for TypeVisitor<'tcx> { + type Map = Map<'tcx>; + + fn visit_lifetime(&mut self, lt: &'tcx Lifetime) { + if lt.name == self.target_lt.name { + self.found = true; + } + } + + fn visit_ty(&mut self, ty: &'tcx Ty<'_>) { + match ty.kind { + TyKind::Rptr(ref lt, _) => { + if lt.name == self.target_lt.name { + self.found = true; + } + }, + TyKind::Path(ref qpath) => { + if !self.found { + walk_qpath(self, qpath, ty.hir_id, ty.span); + } + }, + _ => (), + } + } + + fn nested_visit_map(&mut self) -> NestedVisitorMap<'_, Self::Map> { + NestedVisitorMap::None + } +} + +fn lifetime_outlives_something<'tcx>(target_lt: &'tcx Lifetime, generics: &'tcx Generics<'tcx>) -> bool { + if let Some(param) = generics.get_named(target_lt.name.ident().name) { + if param.bounds.iter().any(|b| matches!(b, GenericBound::Outlives(_))) { + return true; + } + } + false +} + +fn impl_generics<'tcx>(cx: &LateContext<'_, 'tcx>, hir_id: HirId) -> Option<(&'tcx Generics<'tcx>, &'tcx Ty<'tcx>)> { + let parent_impl = cx.tcx.hir().get_parent_item(hir_id); + if_chain! { + if parent_impl != CRATE_HIR_ID; + if let Node::Item(item) = cx.tcx.hir().get(parent_impl); + if let ItemKind::Impl(_, _, _, ref parent_generics, _, ref ty, _) = item.kind; + then { + return Some((parent_generics, ty)) + } + } + None +} diff --git a/src/lintlist/mod.rs b/src/lintlist/mod.rs index f576bb152de9..451c1b2cd75a 100644 --- a/src/lintlist/mod.rs +++ b/src/lintlist/mod.rs @@ -6,7 +6,7 @@ pub use lint::Lint; pub use lint::LINT_LEVELS; // begin lint list, do not remove this comment, it’s used in `update_lints` -pub const ALL_LINTS: [Lint; 346] = [ +pub const ALL_LINTS: [Lint; 347] = [ Lint { name: "absurd_extreme_comparisons", group: "correctness", @@ -2079,6 +2079,13 @@ pub const ALL_LINTS: [Lint; 346] = [ deprecation: None, module: "trait_bounds", }, + Lint { + name: "unbound_return_lifetimes", + group: "correctness", + desc: "unbound lifetimes in function return values", + deprecation: None, + module: "unbound_return_lifetimes", + }, Lint { name: "unicode_not_nfc", group: "pedantic", diff --git a/tests/ui/extra_unused_lifetimes.rs b/tests/ui/extra_unused_lifetimes.rs index ba95fd63bf9a..00950c0dc80e 100644 --- a/tests/ui/extra_unused_lifetimes.rs +++ b/tests/ui/extra_unused_lifetimes.rs @@ -3,7 +3,8 @@ dead_code, clippy::needless_lifetimes, clippy::needless_pass_by_value, - clippy::trivially_copy_pass_by_ref + clippy::trivially_copy_pass_by_ref, + clippy::unbound_return_lifetimes )] #![warn(clippy::extra_unused_lifetimes)] diff --git a/tests/ui/extra_unused_lifetimes.stderr b/tests/ui/extra_unused_lifetimes.stderr index ebdb8e749520..173f79c97dcf 100644 --- a/tests/ui/extra_unused_lifetimes.stderr +++ b/tests/ui/extra_unused_lifetimes.stderr @@ -1,5 +1,5 @@ error: this lifetime isn't used in the function definition - --> $DIR/extra_unused_lifetimes.rs:14:14 + --> $DIR/extra_unused_lifetimes.rs:15:14 | LL | fn unused_lt<'a>(x: u8) {} | ^^ @@ -7,19 +7,19 @@ LL | fn unused_lt<'a>(x: u8) {} = note: `-D clippy::extra-unused-lifetimes` implied by `-D warnings` error: this lifetime isn't used in the function definition - --> $DIR/extra_unused_lifetimes.rs:16:25 + --> $DIR/extra_unused_lifetimes.rs:17:25 | LL | fn unused_lt_transitive<'a, 'b: 'a>(x: &'b u8) { | ^^ error: this lifetime isn't used in the function definition - --> $DIR/extra_unused_lifetimes.rs:41:10 + --> $DIR/extra_unused_lifetimes.rs:42:10 | LL | fn x<'a>(&self) {} | ^^ error: this lifetime isn't used in the function definition - --> $DIR/extra_unused_lifetimes.rs:67:22 + --> $DIR/extra_unused_lifetimes.rs:68:22 | LL | fn unused_lt<'a>(x: u8) {} | ^^ diff --git a/tests/ui/needless_lifetimes.rs b/tests/ui/needless_lifetimes.rs index f3fdd48633f8..0c9fa95987b3 100644 --- a/tests/ui/needless_lifetimes.rs +++ b/tests/ui/needless_lifetimes.rs @@ -1,5 +1,10 @@ #![warn(clippy::needless_lifetimes)] -#![allow(dead_code, clippy::needless_pass_by_value, clippy::trivially_copy_pass_by_ref)] +#![allow( + dead_code, + clippy::needless_pass_by_value, + clippy::trivially_copy_pass_by_ref, + clippy::unbound_return_lifetimes +)] fn distinct_lifetimes<'a, 'b>(_x: &'a u8, _y: &'b u8, _z: u8) {} diff --git a/tests/ui/needless_lifetimes.stderr b/tests/ui/needless_lifetimes.stderr index ad55fc5f750d..69ce6edfd463 100644 --- a/tests/ui/needless_lifetimes.stderr +++ b/tests/ui/needless_lifetimes.stderr @@ -1,5 +1,5 @@ error: explicit lifetimes given in parameter types where they could be elided (or replaced with `'_` if needed by type declaration) - --> $DIR/needless_lifetimes.rs:4:1 + --> $DIR/needless_lifetimes.rs:9:1 | LL | fn distinct_lifetimes<'a, 'b>(_x: &'a u8, _y: &'b u8, _z: u8) {} | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -7,13 +7,13 @@ LL | fn distinct_lifetimes<'a, 'b>(_x: &'a u8, _y: &'b u8, _z: u8) {} = note: `-D clippy::needless-lifetimes` implied by `-D warnings` error: explicit lifetimes given in parameter types where they could be elided (or replaced with `'_` if needed by type declaration) - --> $DIR/needless_lifetimes.rs:6:1 + --> $DIR/needless_lifetimes.rs:11:1 | LL | fn distinct_and_static<'a, 'b>(_x: &'a u8, _y: &'b u8, _z: &'static u8) {} | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: explicit lifetimes given in parameter types where they could be elided (or replaced with `'_` if needed by type declaration) - --> $DIR/needless_lifetimes.rs:16:1 + --> $DIR/needless_lifetimes.rs:21:1 | LL | / fn in_and_out<'a>(x: &'a u8, _y: u8) -> &'a u8 { LL | | x @@ -21,7 +21,7 @@ LL | | } | |_^ error: explicit lifetimes given in parameter types where they could be elided (or replaced with `'_` if needed by type declaration) - --> $DIR/needless_lifetimes.rs:45:1 + --> $DIR/needless_lifetimes.rs:50:1 | LL | / fn deep_reference_3<'a>(x: &'a u8, _y: u8) -> Result<&'a u8, ()> { LL | | Ok(x) @@ -29,7 +29,7 @@ LL | | } | |_^ error: explicit lifetimes given in parameter types where they could be elided (or replaced with `'_` if needed by type declaration) - --> $DIR/needless_lifetimes.rs:50:1 + --> $DIR/needless_lifetimes.rs:55:1 | LL | / fn where_clause_without_lt<'a, T>(x: &'a u8, _y: u8) -> Result<&'a u8, ()> LL | | where @@ -40,13 +40,13 @@ LL | | } | |_^ error: explicit lifetimes given in parameter types where they could be elided (or replaced with `'_` if needed by type declaration) - --> $DIR/needless_lifetimes.rs:62:1 + --> $DIR/needless_lifetimes.rs:67:1 | LL | fn lifetime_param_2<'a, 'b>(_x: Ref<'a>, _y: &'b u8) {} | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: explicit lifetimes given in parameter types where they could be elided (or replaced with `'_` if needed by type declaration) - --> $DIR/needless_lifetimes.rs:86:1 + --> $DIR/needless_lifetimes.rs:91:1 | LL | / fn fn_bound_2<'a, F, I>(_m: Lt<'a, I>, _f: F) -> Lt<'a, I> LL | | where @@ -57,7 +57,7 @@ LL | | } | |_^ error: explicit lifetimes given in parameter types where they could be elided (or replaced with `'_` if needed by type declaration) - --> $DIR/needless_lifetimes.rs:120:5 + --> $DIR/needless_lifetimes.rs:125:5 | LL | / fn self_and_out<'s>(&'s self) -> &'s u8 { LL | | &self.x @@ -65,13 +65,13 @@ LL | | } | |_____^ error: explicit lifetimes given in parameter types where they could be elided (or replaced with `'_` if needed by type declaration) - --> $DIR/needless_lifetimes.rs:129:5 + --> $DIR/needless_lifetimes.rs:134:5 | LL | fn distinct_self_and_in<'s, 't>(&'s self, _x: &'t u8) {} | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: explicit lifetimes given in parameter types where they could be elided (or replaced with `'_` if needed by type declaration) - --> $DIR/needless_lifetimes.rs:148:1 + --> $DIR/needless_lifetimes.rs:153:1 | LL | / fn struct_with_lt<'a>(_foo: Foo<'a>) -> &'a str { LL | | unimplemented!() @@ -79,7 +79,7 @@ LL | | } | |_^ error: explicit lifetimes given in parameter types where they could be elided (or replaced with `'_` if needed by type declaration) - --> $DIR/needless_lifetimes.rs:178:1 + --> $DIR/needless_lifetimes.rs:183:1 | LL | / fn trait_obj_elided2<'a>(_arg: &'a dyn Drop) -> &'a str { LL | | unimplemented!() @@ -87,7 +87,7 @@ LL | | } | |_^ error: explicit lifetimes given in parameter types where they could be elided (or replaced with `'_` if needed by type declaration) - --> $DIR/needless_lifetimes.rs:184:1 + --> $DIR/needless_lifetimes.rs:189:1 | LL | / fn alias_with_lt<'a>(_foo: FooAlias<'a>) -> &'a str { LL | | unimplemented!() @@ -95,7 +95,7 @@ LL | | } | |_^ error: explicit lifetimes given in parameter types where they could be elided (or replaced with `'_` if needed by type declaration) - --> $DIR/needless_lifetimes.rs:203:1 + --> $DIR/needless_lifetimes.rs:208:1 | LL | / fn named_input_elided_output<'a>(_arg: &'a str) -> &str { LL | | unimplemented!() @@ -103,7 +103,7 @@ LL | | } | |_^ error: explicit lifetimes given in parameter types where they could be elided (or replaced with `'_` if needed by type declaration) - --> $DIR/needless_lifetimes.rs:211:1 + --> $DIR/needless_lifetimes.rs:216:1 | LL | / fn trait_bound_ok<'a, T: WithLifetime<'static>>(_: &'a u8, _: T) { LL | | unimplemented!() @@ -111,7 +111,7 @@ LL | | } | |_^ error: explicit lifetimes given in parameter types where they could be elided (or replaced with `'_` if needed by type declaration) - --> $DIR/needless_lifetimes.rs:247:1 + --> $DIR/needless_lifetimes.rs:252:1 | LL | / fn out_return_type_lts<'a>(e: &'a str) -> Cow<'a> { LL | | unimplemented!() @@ -119,13 +119,13 @@ LL | | } | |_^ error: explicit lifetimes given in parameter types where they could be elided (or replaced with `'_` if needed by type declaration) - --> $DIR/needless_lifetimes.rs:254:9 + --> $DIR/needless_lifetimes.rs:259:9 | LL | fn needless_lt<'a>(x: &'a u8) {} | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: explicit lifetimes given in parameter types where they could be elided (or replaced with `'_` if needed by type declaration) - --> $DIR/needless_lifetimes.rs:258:9 + --> $DIR/needless_lifetimes.rs:263:9 | LL | fn needless_lt<'a>(_x: &'a u8) {} | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ diff --git a/tests/ui/unbound_return_lifetimes.rs b/tests/ui/unbound_return_lifetimes.rs new file mode 100644 index 000000000000..30cc493d1cd4 --- /dev/null +++ b/tests/ui/unbound_return_lifetimes.rs @@ -0,0 +1,74 @@ +#![allow(unused, dead_code, clippy::needless_lifetimes)] +#![warn(clippy::unbound_return_lifetimes)] + +use std::collections::HashMap; +use std::hash::Hash; + +struct FooStr(str); + +fn unbound_fun<'a>(x: impl AsRef + 'a) -> &'a FooStr { + let x = x.as_ref(); + unsafe { &*(x as *const str as *const FooStr) } +} + +fn bound_fun<'a>(x: &'a str) -> &'a FooStr { + unsafe { &*(x as *const str as *const FooStr) } +} + +fn bound_fun2<'a, 'b: 'a, S: 'b>(s: &'a S) -> &'b str { + unreachable!() +} + +type BarType<'a, T> = Bar<'a, &'a T>; + +struct Bar<'a, T> { + baz: &'a T, +} + +impl<'a, T> BarType<'a, T> { + fn bound_impl_fun(&self, _f: bool) -> &'a T { + unreachable!() + } +} + +pub struct BazMap<'a, K, V, W> { + alloc: &'a Vec, + map: HashMap, +} + +pub type BazRefMap<'a, K, V> = BazMap<'a, K, V, &'a V>; + +impl<'a, K, V> BazRefMap<'a, K, V> +where + K: Hash + PartialEq, +{ + pub fn or_insert(&self, key: K, value: V) -> &'a V { + unreachable!() + } +} + +struct Quux<'a> { + x: &'a str, +} + +fn quux<'b>(q: &Quux<'b>) -> &'b str { + unreachable!() +} + +struct Quuz { + x: T, +} + +impl Quuz { + fn into_inner(self) -> T { + self.x + } +} + +impl<'a> Into<&'a str> for Quuz<&'a str> { + fn into(self) -> &'a str { + self.into_inner() + } +} + +fn main() {} diff --git a/tests/ui/unbound_return_lifetimes.stderr b/tests/ui/unbound_return_lifetimes.stderr new file mode 100644 index 000000000000..0e3ebf688059 --- /dev/null +++ b/tests/ui/unbound_return_lifetimes.stderr @@ -0,0 +1,10 @@ +error: lifetime is unconstrained + --> $DIR/unbound_return_lifetimes.rs:9:49 + | +LL | fn unbound_fun<'a>(x: impl AsRef + 'a) -> &'a FooStr { + | ^^ + | + = note: `-D clippy::unbound-return-lifetimes` implied by `-D warnings` + +error: aborting due to previous error + diff --git a/tests/ui/unbound_return_lifetimes.stdout b/tests/ui/unbound_return_lifetimes.stdout new file mode 100644 index 000000000000..e69de29bb2d1