diff --git a/CHANGELOG.md b/CHANGELOG.md index 89e3bd336c5a..f1333b9a3816 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6750,6 +6750,7 @@ Released 2018-09-13 [`reversed_empty_ranges`]: https://rust-lang.github.io/rust-clippy/master/index.html#reversed_empty_ranges [`same_functions_in_if_condition`]: https://rust-lang.github.io/rust-clippy/master/index.html#same_functions_in_if_condition [`same_item_push`]: https://rust-lang.github.io/rust-clippy/master/index.html#same_item_push +[`same_length_and_capacity`]: https://rust-lang.github.io/rust-clippy/master/index.html#same_length_and_capacity [`same_name_method`]: https://rust-lang.github.io/rust-clippy/master/index.html#same_name_method [`search_is_some`]: https://rust-lang.github.io/rust-clippy/master/index.html#search_is_some [`seek_from_current`]: https://rust-lang.github.io/rust-clippy/master/index.html#seek_from_current diff --git a/clippy_lints/src/declared_lints.rs b/clippy_lints/src/declared_lints.rs index 2a9c6efb52ee..1c225f2e32ee 100644 --- a/clippy_lints/src/declared_lints.rs +++ b/clippy_lints/src/declared_lints.rs @@ -661,6 +661,7 @@ pub static LINTS: &[&::declare_clippy_lint::LintInfo] = &[ crate::returns::LET_AND_RETURN_INFO, crate::returns::NEEDLESS_RETURN_INFO, crate::returns::NEEDLESS_RETURN_WITH_QUESTION_MARK_INFO, + crate::same_length_and_capacity::SAME_LENGTH_AND_CAPACITY_INFO, crate::same_name_method::SAME_NAME_METHOD_INFO, crate::self_named_constructors::SELF_NAMED_CONSTRUCTORS_INFO, crate::semicolon_block::SEMICOLON_INSIDE_BLOCK_INFO, diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 149785c59447..22e6f0e7cf4d 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -327,6 +327,7 @@ mod replace_box; mod reserve_after_initialization; mod return_self_not_must_use; mod returns; +mod same_length_and_capacity; mod same_name_method; mod self_named_constructors; mod semicolon_block; @@ -834,5 +835,6 @@ pub fn register_lint_passes(store: &mut rustc_lint::LintStore, conf: &'static Co store.register_late_pass(|_| Box::new(toplevel_ref_arg::ToplevelRefArg)); store.register_late_pass(|_| Box::new(volatile_composites::VolatileComposites)); store.register_late_pass(|_| Box::new(replace_box::ReplaceBox)); + store.register_late_pass(|_| Box::new(same_length_and_capacity::SameLengthAndCapacity)); // add lints here, do not remove this comment, it's used in `new_lint` } diff --git a/clippy_lints/src/same_length_and_capacity.rs b/clippy_lints/src/same_length_and_capacity.rs new file mode 100644 index 000000000000..f4c01e0b92fc --- /dev/null +++ b/clippy_lints/src/same_length_and_capacity.rs @@ -0,0 +1,107 @@ +use clippy_utils::diagnostics::span_lint_and_help; +use clippy_utils::ty::{is_type_diagnostic_item, is_type_lang_item}; +use clippy_utils::{SpanlessEq, sym}; +use rustc_hir::{Expr, ExprKind, LangItem, QPath}; +use rustc_lint::{LateContext, LateLintPass}; +use rustc_session::declare_lint_pass; +use rustc_span::symbol::sym as rustc_sym; + +declare_clippy_lint! { + /// ### What it does + /// + /// Checks for usages of Vec::from_raw_parts and String::from_raw_parts + /// where the same expression is used for the length and the capacity. + /// + /// ### Why is this bad? + /// + /// If the same expression is being passed for the length and + /// capacity, it is most likely a semantic error. In the case of a + /// Vec, for example, the only way to end up with one that has + /// the same length and capacity is by going through a boxed slice, + /// e.g. Box::from(some_vec), which shrinks the capacity to match + /// the length. + /// + /// ### Example + /// + /// ```no_run + /// #![feature(vec_into_raw_parts)] + /// let mut original: Vec:: = Vec::with_capacity(20); + /// original.extend([1, 2, 3, 4, 5]); + /// + /// let (ptr, mut len, cap) = original.into_raw_parts(); + /// + /// // I will add three more integers: + /// unsafe { + /// let ptr = ptr as *mut i32; + /// + /// for i in 6..9 { + /// *ptr.add(i - 1) = i as i32; + /// len += 1; + /// } + /// } + /// + /// // But I forgot the capacity was separate from the length: + /// let reconstructed = unsafe { Vec::from_raw_parts(ptr, len, len) }; + /// ``` + /// + /// Use instead: + /// ```no_run + /// #![feature(vec_into_raw_parts)] + /// let mut original: Vec:: = Vec::with_capacity(20); + /// original.extend([1, 2, 3, 4, 5]); + /// + /// let (ptr, mut len, cap) = original.into_raw_parts(); + /// + /// // I will add three more integers: + /// unsafe { + /// let ptr = ptr as *mut i32; + /// + /// for i in 6..9 { + /// *ptr.add(i - 1) = i as i32; + /// len += 1; + /// } + /// } + /// + /// // This time, leverage the previously saved capacity: + /// let reconstructed = unsafe { Vec::from_raw_parts(ptr, len, cap) }; + /// ``` + #[clippy::version = "1.91.0"] + pub SAME_LENGTH_AND_CAPACITY, + pedantic, + "`from_raw_parts` with same length and capacity" +} +declare_lint_pass!(SameLengthAndCapacity => [SAME_LENGTH_AND_CAPACITY]); + +impl<'tcx> LateLintPass<'tcx> for SameLengthAndCapacity { + fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) { + if let ExprKind::Call(path_expr, args) = expr.kind + && let ExprKind::Path(QPath::TypeRelative(ty, fn_path)) = path_expr.kind + && is_type_diagnostic_item(cx, cx.typeck_results().node_type(ty.hir_id), rustc_sym::Vec) + && fn_path.ident.name == sym::from_raw_parts + && SpanlessEq::new(cx).eq_expr(&args[1], &args[2]) + { + span_lint_and_help( + cx, + SAME_LENGTH_AND_CAPACITY, + expr.span, + "usage of `Vec::from_raw_parts` with the same expression for length and capacity", + None, + "if the length and capacity are the same, you most likely went through a boxed slice; consider reconstructing the `Vec` using a `Box` instead, e.g. `Box::from(slice::from_raw_parts(...)).into_vec()`", + ); + } else if let ExprKind::Call(path_expr, args) = expr.kind + && let ExprKind::Path(QPath::TypeRelative(ty, fn_path)) = path_expr.kind + && is_type_lang_item(cx, cx.typeck_results().node_type(ty.hir_id), LangItem::String) + && fn_path.ident.name == sym::from_raw_parts + && SpanlessEq::new(cx).eq_expr(&args[1], &args[2]) + { + span_lint_and_help( + cx, + SAME_LENGTH_AND_CAPACITY, + expr.span, + "usage of `String::from_raw_parts` with the same expression for length and capacity", + None, + "if the length and capacity are the same, you most likely went through a boxed `str`; consider reconstructing the `String` using `String::from` instead, e.g. `String::from(str::from_utf8_unchecked(slice::from_raw_parts(...)))`", + ); + } + } +} diff --git a/clippy_utils/src/sym.rs b/clippy_utils/src/sym.rs index fee45c670962..a423094522e5 100644 --- a/clippy_utils/src/sym.rs +++ b/clippy_utils/src/sym.rs @@ -161,6 +161,7 @@ generate! { from_ne_bytes, from_ptr, from_raw, + from_raw_parts, from_str, from_str_radix, fs, diff --git a/tests/ui/same_length_and_capacity.rs b/tests/ui/same_length_and_capacity.rs new file mode 100644 index 000000000000..e4f015f255a9 --- /dev/null +++ b/tests/ui/same_length_and_capacity.rs @@ -0,0 +1,24 @@ +#![feature(vec_into_raw_parts)] +#![warn(clippy::same_length_and_capacity)] + +fn main() { + let mut my_vec: Vec = Vec::with_capacity(20); + my_vec.extend([1, 2, 3, 4, 5]); + let (ptr, mut len, cap) = my_vec.into_raw_parts(); + len = 8; + + let _reconstructed_vec = unsafe { Vec::from_raw_parts(ptr, len, len) }; + //~^ same_length_and_capacity + + // Don't want to lint different expressions for len and cap + let _properly_reconstructed_vec = unsafe { Vec::from_raw_parts(ptr, len, cap) }; + + let my_string = String::from("hello"); + let (string_ptr, string_len, string_cap) = my_string.into_raw_parts(); + + let _reconstructed_string = unsafe { String::from_raw_parts(string_ptr, string_len, string_len) }; + //~^ same_length_and_capacity + + // Don't want to lint different expressions for len and cap + let _properly_reconstructed_string = unsafe { String::from_raw_parts(string_ptr, string_len, string_cap) }; +} diff --git a/tests/ui/same_length_and_capacity.stderr b/tests/ui/same_length_and_capacity.stderr new file mode 100644 index 000000000000..ad529f814486 --- /dev/null +++ b/tests/ui/same_length_and_capacity.stderr @@ -0,0 +1,20 @@ +error: usage of `Vec::from_raw_parts` with the same expression for length and capacity + --> tests/ui/same_length_and_capacity.rs:10:39 + | +LL | let _reconstructed_vec = unsafe { Vec::from_raw_parts(ptr, len, len) }; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = help: if the length and capacity are the same, you most likely went through a boxed slice; consider reconstructing the `Vec` using a `Box` instead, e.g. `Box::from(slice::from_raw_parts(...)).into_vec()` + = note: `-D clippy::same-length-and-capacity` implied by `-D warnings` + = help: to override `-D warnings` add `#[allow(clippy::same_length_and_capacity)]` + +error: usage of `String::from_raw_parts` with the same expression for length and capacity + --> tests/ui/same_length_and_capacity.rs:19:42 + | +LL | let _reconstructed_string = unsafe { String::from_raw_parts(string_ptr, string_len, string_len) }; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = help: if the length and capacity are the same, you most likely went through a boxed `str`; consider reconstructing the `String` using `String::from` instead, e.g. `String::from(str::from_utf8_unchecked(slice::from_raw_parts(...)))` + +error: aborting due to 2 previous errors +