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

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

README.md

@@ -4,7 +4,7 @@
 
 A collection of lints to catch common mistakes and improve your [Rust](https://github.com/rust-lang/rust) code.
 
-[There are over 750 lints included in this crate!](https://rust-lang.github.io/rust-clippy/master/index.html)
+[There are over 800 lints included in this crate!](https://rust-lang.github.io/rust-clippy/master/index.html)
 
 Lints are divided into categories, each with a default [lint level](https://doc.rust-lang.org/rustc/lints/levels.html).
 You can choose how much Clippy is supposed to ~~annoy~~ help you by changing the lint level by category.

book/src/README.md

@@ -5,7 +5,7 @@
 A collection of lints to catch common mistakes and improve your
 [Rust](https://github.com/rust-lang/rust) code.
 
-[There are over 750 lints included in this crate!](https://rust-lang.github.io/rust-clippy/master/index.html)
+[There are over 800 lints included in this crate!](https://rust-lang.github.io/rust-clippy/master/index.html)
 
 Lints are divided into categories, each with a default [lint
 level](https://doc.rust-lang.org/rustc/lints/levels.html). You can choose how

clippy_lints/src/declared_lints.rs

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

@@ -318,6 +318,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;
@@ -852,6 +853,7 @@ pub fn register_lint_passes(store: &mut rustc_lint::LintStore, conf: &'static Co
         Box::new(|_| Box::new(volatile_composites::VolatileComposites)),
         Box::new(|_| Box::<replace_box::ReplaceBox>::default()),
         Box::new(move |_| Box::new(manual_ilog2::ManualIlog2::new(conf))),
+        Box::new(|_| Box::new(same_length_and_capacity::SameLengthAndCapacity)),
         // add late passes here, used by `cargo dev new_lint`
     ];
     store.late_passes.extend(late_lints);

clippy_lints/src/same_length_and_capacity.rs

@@ -0,0 +1,105 @@
+use clippy_utils::diagnostics::span_lint_and_help;
+use clippy_utils::res::MaybeDef;
+use clippy_utils::{eq_expr_value, 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::<i32> = 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::<i32> = 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.93.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
+            && fn_path.ident.name == sym::from_raw_parts
+            && args.len() >= 3
+            && eq_expr_value(cx, &args[1], &args[2])
+        {
+            let middle_ty = cx.typeck_results().node_type(ty.hir_id);
+            if middle_ty.is_diag_item(cx, rustc_sym::Vec) {
+                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,
+                    "try `Box::from(slice::from_raw_parts(...)).into::<Vec<_>>()`",
+                );
+            } else if middle_ty.is_lang_item(cx, LangItem::String) {
+                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,
+                    "try `String::from(str::from_utf8_unchecked(slice::from_raw_parts(...)))`",
+                );
+            }
+        }
+    }
+}

clippy_utils/src/sym.rs

@@ -167,6 +167,7 @@ generate! {
     from_ne_bytes,
     from_ptr,
     from_raw,
+    from_raw_parts,
     from_str_radix,
     fs,
     fuse,

tests/ui/same_length_and_capacity.rs

@@ -0,0 +1,27 @@
+#![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) };
+
+    // Don't want to lint if len and cap are distinct variables but happen to be equal
+    let len_from_cap = cap;
+    let _another_properly_reconstructed_vec = unsafe { Vec::from_raw_parts(ptr, len_from_cap, 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:9:39
+   |
+LL |     let _reconstructed_vec = unsafe { Vec::from_raw_parts(ptr, len, len) };
+   |                                       ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+   = help: try `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:22:42
+   |
+LL |     let _reconstructed_string = unsafe { String::from_raw_parts(string_ptr, string_len, string_len) };
+   |                                          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+   = help: try `String::from(str::from_utf8_unchecked(slice::from_raw_parts(...)))`
+
+error: aborting due to 2 previous errors
+