diff --git a/CHANGELOG.md b/CHANGELOG.md index 962f9067a4e4..f685c5c28c8c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1226,6 +1226,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 6133fa4c3a57..4106ee278fa6 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 340 lints included in this crate!](https://rust-lang.github.io/rust-clippy/master/index.html) +[There are 341 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 736ff30c81a7..32ea21e2829c 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -290,6 +290,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; @@ -770,6 +771,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, @@ -937,6 +939,7 @@ pub fn register_plugins(store: &mut lint::LintStore, sess: &Session, conf: &Conf store.register_late_pass(|| box trait_bounds::TraitBounds); store.register_late_pass(|| box comparison_chain::ComparisonChain); store.register_late_pass(|| box mul_add::MulAddCheck); + 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); @@ -1300,6 +1303,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), @@ -1547,6 +1551,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..5aa3f1cf6f1d --- /dev/null +++ b/clippy_lints/src/unbound_return_lifetimes.rs @@ -0,0 +1,128 @@ +use if_chain::if_chain; +use rustc::declare_lint_pass; +use rustc::hir::*; +use rustc::lint::{LateContext, LateLintPass, LintArray, LintPass}; +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 + /// // Bad: unbound return lifetime causing unsoundnes + /// fn foo<'a>(x: impl AsRef + 'a) -> &'a str { + /// let s = x.as_ref(); + /// unsafe { &*(s as *const str) } + /// } + /// // Good: bound return lifetime is sound + /// struct WrappedStr(str); + /// fn foo<'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) { + 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) { + 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, + generics: &'tcx Generics, + parent_generics: Option<(&'tcx Generics, Option<&'tcx Generics>, Option<&'tcx Generics>)>, +) { + if let FunctionRetTy::Return(ref typ) = decl.output { + if let TyKind::Rptr(ref lifetime, _) = typ.kind { + if let LifetimeName::Param(_) = lifetime.name { + let target_lifetime = lifetime; + + // check function generics + // the target lifetime parameter must appear on the left of some outlives relation + if let Some(param) = generics.get_named(target_lifetime.name.ident().name) { + if param.bounds.iter().any(|b| if let GenericBound::Outlives(_) = b { true } else { false }) { + return; + } + } + + // check parent generics + // the target lifetime parameter must appear on the left of some outlives relation + if let Some((ref parent_generics, _, _)) = parent_generics { + if let Some(param) = parent_generics.get_named(target_lifetime.name.ident().name) { + if param.bounds.iter().any(|b| if let GenericBound::Outlives(_) = b { true } else { false }) { + return; + } + } + } + + // check type generics + // the target lifetime parameter must be included in the struct + if let Some((_, _, Some(ref typ_generics))) = parent_generics { + if typ_generics.get_named(target_lifetime.name.ident().name).is_some() { + return; + } + } + + // check arguments + // the target lifetime parameter must be included as a lifetime of a reference + for input in decl.inputs.iter() { + if let TyKind::Rptr(ref lifetime, _) = input.kind { + if lifetime.name == target_lifetime.name { + return; + } + } + } + + span_lint( + cx, + UNBOUND_RETURN_LIFETIMES, + target_lifetime.span, + "lifetime is unconstrained" + ); + } + } + } +} + +fn impl_generics<'tcx>(cx: &LateContext<'_, 'tcx>, hir_id: HirId) -> Option<(&'tcx Generics, Option<&'tcx Generics>, Option<&'tcx Generics>)> { + 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 _trait_ref, ref ty, _) = item.kind; + then { + if let TyKind::Path(ref qpath) = ty.kind { + if let Some(typ_def_id) = cx.tables.qpath_res(qpath, ty.hir_id).opt_def_id() { + let typ_generics = cx.tcx.hir().get_generics(typ_def_id); + return Some((parent_generics, None, typ_generics)) + } + } + } + } + None +} diff --git a/src/lintlist/mod.rs b/src/lintlist/mod.rs index f4ebf6cbd918..4ece18ed7d89 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; 340] = [ +pub const ALL_LINTS: [Lint; 341] = [ Lint { name: "absurd_extreme_comparisons", group: "correctness", @@ -2037,6 +2037,13 @@ pub const ALL_LINTS: [Lint; 340] = [ 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/unbound_return_lifetimes.rs b/tests/ui/unbound_return_lifetimes.rs new file mode 100644 index 000000000000..80e1f36c5e3a --- /dev/null +++ b/tests/ui/unbound_return_lifetimes.rs @@ -0,0 +1,57 @@ +#![allow( + unused, + dead_code, + clippy::needless_lifetimes, +)] +#![warn(clippy::unbound_return_lifetimes)] + +use std::hash::Hash; +use std::collections::HashMap; + +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!() + } +} + + +fn main() {} + + diff --git a/tests/ui/unbound_return_lifetimes.stderr b/tests/ui/unbound_return_lifetimes.stderr new file mode 100644 index 000000000000..5af3d6eb8325 --- /dev/null +++ b/tests/ui/unbound_return_lifetimes.stderr @@ -0,0 +1,10 @@ +error: lifetime is unconstrained + --> $DIR/unbound_return_lifetimes.rs:13: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