Implement lint to check for usages of from_raw_parts where the same expression is used for the length and the capacity

Author: km274

CHANGELOG.md

@@ -6591,6 +6591,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

clippy_lints/src/declared_lints.rs

@@ -660,6 +660,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,

clippy_lints/src/lib.rs

@@ -327,6 +327,7 @@ mod repeat_vec_with_capacity;
 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;
@@ -831,5 +832,6 @@ pub fn register_lint_passes(store: &mut rustc_lint::LintStore, conf: &'static Co
     store.register_late_pass(|_| Box::new(infallible_try_from::InfallibleTryFrom));
     store.register_late_pass(|_| Box::new(coerce_container_to_any::CoerceContainerToAny));
     store.register_late_pass(|_| Box::new(toplevel_ref_arg::ToplevelRefArg));
+    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`
 }

clippy_lints/src/same_length_and_capacity.rs

@@ -0,0 +1,83 @@
+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
+    /// let mut original: Vec::<i32> = Vec::with_capacity(20);
+    /// original.extend([1, 2, 3, 4, 5]);
+    ///
+    /// let (ptr, mut len, cap) = original.into_raw_parts();
+    ///
+    /// // Pretend we added three more integers:
+    /// len = 8;
+    ///
+    /// // But I forgot the capacity was separate from the length:
+    /// let reconstructed = unsafe { Vec::from_raw_parts(ptr, len, len) };
+    /// ```
+    ///
+    /// Use instead:
+    /// ```no_run
+    /// // Correction to the last line of the given example code:
+    /// 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(...)))`",
+            );
+        }
+    }
+}

clippy_utils/src/sym.rs

@@ -160,6 +160,7 @@ generate! {
     from_ne_bytes,
     from_ptr,
     from_raw,
+    from_raw_parts,
     from_str,
     from_str_radix,
     fs,

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<i32> = 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) };
+}

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
+