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 bae4eebf8508..9ad5752b371a 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -12,6 +12,7 @@ #![cfg_attr(feature = "deny-warnings", deny(warnings))] #![feature(crate_visibility_modifier)] #![feature(concat_idents)] +#![feature(matches_macro)] // FIXME: switch to something more ergonomic here, once available. // (Currently there is no way to opt into sysroot crates without `extern crate`.) @@ -289,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; @@ -769,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, @@ -936,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); @@ -1299,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), @@ -1546,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..7a0f6cefd81e --- /dev/null +++ b/clippy_lints/src/unbound_return_lifetimes.rs @@ -0,0 +1,146 @@ +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 + /// 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) { + 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>)>, +) { + let output_type = if let FunctionRetTy::Return(ref output_type) = decl.output { + output_type + } else { + return; + }; + + let lifetime = if let TyKind::Rptr(ref lifetime, _) = output_type.kind { + lifetime + } else { + return; + }; + + if matches!(lifetime.name, LifetimeName::Param(_)) { + let target_lifetime = lifetime; + + // check function generics + // the target lifetime parameter must appear on the left of some outlives relation + if lifetime_outlives_something(target_lifetime, 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_generics { + if lifetime_outlives_something(target_lifetime, parent_generics) { + 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 lifetime_outlives_something<'tcx>(target_lifetime: &'tcx Lifetime, generics: &'tcx Generics) -> bool { + if let Some(param) = generics.get_named(target_lifetime.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, 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/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..5d07b93368f9 --- /dev/null +++ b/tests/ui/unbound_return_lifetimes.rs @@ -0,0 +1,50 @@ +#![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!() + } +} + +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