Auto merge of rust-lang#106266 - matthiaskrgr:rollup-cxrdbzy, r=matth…

…iaskrgr Rollup of 9 pull requests Successful merges: - rust-lang#104531 (Provide a better error and a suggestion for `Fn` traits with lifetime params) - rust-lang#105899 (`./x doc library --open` opens `std`) - rust-lang#106190 (Account for multiple multiline spans with empty padding) - rust-lang#106202 (Trim more paths in obligation types) - rust-lang#106234 (rustdoc: simplify settings, help, and copy button CSS by not reusing) - rust-lang#106236 (docs/test: add docs and a UI test for `E0514` and `E0519`) - rust-lang#106259 (Update Clippy) - rust-lang#106260 (Fix index out of bounds issues in rustdoc) - rust-lang#106263 (Formatter should not try to format non-Rust files) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup
flip1995 · Dec 29, 2022 · cd579d6 · cd579d6
2 parents c617a8e + d8365f9
commit cd579d6
Show file tree

Hide file tree

Showing 99 changed files with 2,558 additions and 364 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -4203,6 +4203,7 @@ Released 2018-09-13
 [`float_cmp_const`]: https://rust-lang.github.io/rust-clippy/master/index.html#float_cmp_const
 [`float_equality_without_abs`]: https://rust-lang.github.io/rust-clippy/master/index.html#float_equality_without_abs
 [`fn_address_comparisons`]: https://rust-lang.github.io/rust-clippy/master/index.html#fn_address_comparisons
+[`fn_null_check`]: https://rust-lang.github.io/rust-clippy/master/index.html#fn_null_check
 [`fn_params_excessive_bools`]: https://rust-lang.github.io/rust-clippy/master/index.html#fn_params_excessive_bools
 [`fn_to_numeric_cast`]: https://rust-lang.github.io/rust-clippy/master/index.html#fn_to_numeric_cast
 [`fn_to_numeric_cast_any`]: https://rust-lang.github.io/rust-clippy/master/index.html#fn_to_numeric_cast_any
@@ -4460,6 +4461,7 @@ Released 2018-09-13
 [`partialeq_to_none`]: https://rust-lang.github.io/rust-clippy/master/index.html#partialeq_to_none
 [`path_buf_push_overwrite`]: https://rust-lang.github.io/rust-clippy/master/index.html#path_buf_push_overwrite
 [`pattern_type_mismatch`]: https://rust-lang.github.io/rust-clippy/master/index.html#pattern_type_mismatch
+[`permissions_set_readonly_false`]: https://rust-lang.github.io/rust-clippy/master/index.html#permissions_set_readonly_false
 [`positional_named_format_parameters`]: https://rust-lang.github.io/rust-clippy/master/index.html#positional_named_format_parameters
 [`possible_missing_comma`]: https://rust-lang.github.io/rust-clippy/master/index.html#possible_missing_comma
 [`precedence`]: https://rust-lang.github.io/rust-clippy/master/index.html#precedence
@@ -4545,6 +4547,7 @@ Released 2018-09-13
 [`single_match`]: https://rust-lang.github.io/rust-clippy/master/index.html#single_match
 [`single_match_else`]: https://rust-lang.github.io/rust-clippy/master/index.html#single_match_else
 [`size_of_in_element_count`]: https://rust-lang.github.io/rust-clippy/master/index.html#size_of_in_element_count
+[`size_of_ref`]: https://rust-lang.github.io/rust-clippy/master/index.html#size_of_ref
 [`skip_while_next`]: https://rust-lang.github.io/rust-clippy/master/index.html#skip_while_next
 [`slow_vector_initialization`]: https://rust-lang.github.io/rust-clippy/master/index.html#slow_vector_initialization
 [`stable_sort_primitive`]: https://rust-lang.github.io/rust-clippy/master/index.html#stable_sort_primitive
@@ -4590,6 +4593,7 @@ Released 2018-09-13
 [`transmute_int_to_bool`]: https://rust-lang.github.io/rust-clippy/master/index.html#transmute_int_to_bool
 [`transmute_int_to_char`]: https://rust-lang.github.io/rust-clippy/master/index.html#transmute_int_to_char
 [`transmute_int_to_float`]: https://rust-lang.github.io/rust-clippy/master/index.html#transmute_int_to_float
+[`transmute_null_to_fn`]: https://rust-lang.github.io/rust-clippy/master/index.html#transmute_null_to_fn
 [`transmute_num_to_bytes`]: https://rust-lang.github.io/rust-clippy/master/index.html#transmute_num_to_bytes
 [`transmute_ptr_to_ptr`]: https://rust-lang.github.io/rust-clippy/master/index.html#transmute_ptr_to_ptr
 [`transmute_ptr_to_ref`]: https://rust-lang.github.io/rust-clippy/master/index.html#transmute_ptr_to_ref

diff --git a/clippy_lints/src/casts/cast_slice_different_sizes.rs b/clippy_lints/src/casts/cast_slice_different_sizes.rs
@@ -54,7 +54,7 @@ pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, expr: &Expr<'tcx>, msrv: &Msrv
 
                         diag.span_suggestion(
                             expr.span,
-                            &format!("replace with `ptr::slice_from_raw_parts{mutbl_fn_str}`"),
+                            format!("replace with `ptr::slice_from_raw_parts{mutbl_fn_str}`"),
                             sugg,
                             rustc_errors::Applicability::HasPlaceholders,
                         );

diff --git a/clippy_lints/src/declared_lints.rs b/clippy_lints/src/declared_lints.rs
@@ -161,6 +161,7 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[
     crate::float_literal::LOSSY_FLOAT_LITERAL_INFO,
     crate::floating_point_arithmetic::IMPRECISE_FLOPS_INFO,
     crate::floating_point_arithmetic::SUBOPTIMAL_FLOPS_INFO,
+    crate::fn_null_check::FN_NULL_CHECK_INFO,
     crate::format::USELESS_FORMAT_INFO,
     crate::format_args::FORMAT_IN_FORMAT_ARGS_INFO,
     crate::format_args::TO_STRING_IN_FORMAT_ARGS_INFO,
@@ -494,6 +495,7 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[
     crate::pass_by_ref_or_value::LARGE_TYPES_PASSED_BY_VALUE_INFO,
     crate::pass_by_ref_or_value::TRIVIALLY_COPY_PASS_BY_REF_INFO,
     crate::pattern_type_mismatch::PATTERN_TYPE_MISMATCH_INFO,
+    crate::permissions_set_readonly_false::PERMISSIONS_SET_READONLY_FALSE_INFO,
     crate::precedence::PRECEDENCE_INFO,
     crate::ptr::CMP_NULL_INFO,
     crate::ptr::INVALID_NULL_PTR_USAGE_INFO,
@@ -535,6 +537,7 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[
     crate::single_char_lifetime_names::SINGLE_CHAR_LIFETIME_NAMES_INFO,
     crate::single_component_path_imports::SINGLE_COMPONENT_PATH_IMPORTS_INFO,
     crate::size_of_in_element_count::SIZE_OF_IN_ELEMENT_COUNT_INFO,
+    crate::size_of_ref::SIZE_OF_REF_INFO,
     crate::slow_vector_initialization::SLOW_VECTOR_INITIALIZATION_INFO,
     crate::std_instead_of_core::ALLOC_INSTEAD_OF_CORE_INFO,
     crate::std_instead_of_core::STD_INSTEAD_OF_ALLOC_INFO,
@@ -568,6 +571,7 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[
     crate::transmute::TRANSMUTE_INT_TO_BOOL_INFO,
     crate::transmute::TRANSMUTE_INT_TO_CHAR_INFO,
     crate::transmute::TRANSMUTE_INT_TO_FLOAT_INFO,
+    crate::transmute::TRANSMUTE_NULL_TO_FN_INFO,
     crate::transmute::TRANSMUTE_NUM_TO_BYTES_INFO,
     crate::transmute::TRANSMUTE_PTR_TO_PTR_INFO,
     crate::transmute::TRANSMUTE_PTR_TO_REF_INFO,

diff --git a/clippy_lints/src/dereference.rs b/clippy_lints/src/dereference.rs
@@ -1282,10 +1282,10 @@ fn referent_used_exactly_once<'tcx>(
             possible_borrowers.push((body_owner_local_def_id, PossibleBorrowerMap::new(cx, mir)));
         }
         let possible_borrower = &mut possible_borrowers.last_mut().unwrap().1;
-        // If `only_borrowers` were used here, the `copyable_iterator::warn` test would fail. The reason is
-        // that `PossibleBorrowerVisitor::visit_terminator` considers `place.local` a possible borrower of
-        // itself. See the comment in that method for an explanation as to why.
-        possible_borrower.bounded_borrowers(&[local], &[local, place.local], place.local, location)
+        // If `place.local` were not included here, the `copyable_iterator::warn` test would fail. The
+        // reason is that `PossibleBorrowerVisitor::visit_terminator` considers `place.local` a possible
+        // borrower of itself. See the comment in that method for an explanation as to why.
+        possible_borrower.at_most_borrowers(cx, &[local, place.local], place.local, location)
             && used_exactly_once(mir, place.local).unwrap_or(false)
     } else {
         false

diff --git a/clippy_lints/src/floating_point_arithmetic.rs b/clippy_lints/src/floating_point_arithmetic.rs
@@ -324,7 +324,7 @@ fn check_powi(cx: &LateContext<'_>, expr: &Expr<'_>, receiver: &Expr<'_>, args:
                     let maybe_neg_sugg = |expr, hir_id| {
                         let sugg = Sugg::hir(cx, expr, "..");
                         if matches!(op, BinOpKind::Sub) && hir_id == rhs.hir_id {
-                            format!("-{sugg}")
+                            format!("-{}", sugg.maybe_par())
                         } else {
                             sugg.to_string()
                         }

diff --git a/clippy_lints/src/fn_null_check.rs b/clippy_lints/src/fn_null_check.rs
@@ -0,0 +1,106 @@
+use clippy_utils::consts::{constant, Constant};
+use clippy_utils::diagnostics::span_lint_and_help;
+use clippy_utils::{is_integer_literal, is_path_diagnostic_item};
+use rustc_hir::{BinOpKind, Expr, ExprKind, TyKind};
+use rustc_lint::{LateContext, LateLintPass};
+use rustc_session::{declare_lint_pass, declare_tool_lint};
+use rustc_span::sym;
+
+declare_clippy_lint! {
+    /// ### What it does
+    /// Checks for comparing a function pointer to null.
+    ///
+    /// ### Why is this bad?
+    /// Function pointers are assumed to not be null.
+    ///
+    /// ### Example
+    /// ```rust,ignore
+    /// let fn_ptr: fn() = /* somehow obtained nullable function pointer */
+    ///
+    /// if (fn_ptr as *const ()).is_null() { ... }
+    /// ```
+    /// Use instead:
+    /// ```rust,ignore
+    /// let fn_ptr: Option<fn()> = /* somehow obtained nullable function pointer */
+    ///
+    /// if fn_ptr.is_none() { ... }
+    /// ```
+    #[clippy::version = "1.67.0"]
+    pub FN_NULL_CHECK,
+    correctness,
+    "`fn()` type assumed to be nullable"
+}
+declare_lint_pass!(FnNullCheck => [FN_NULL_CHECK]);
+
+fn lint_expr(cx: &LateContext<'_>, expr: &Expr<'_>) {
+    span_lint_and_help(
+        cx,
+        FN_NULL_CHECK,
+        expr.span,
+        "function pointer assumed to be nullable, even though it isn't",
+        None,
+        "try wrapping your function pointer type in `Option<T>` instead, and using `is_none` to check for null pointer value",
+    );
+}
+
+fn is_fn_ptr_cast(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool {
+    if let ExprKind::Cast(cast_expr, cast_ty) = expr.kind
+        && let TyKind::Ptr(_) = cast_ty.kind
+    {
+        cx.typeck_results().expr_ty_adjusted(cast_expr).is_fn()
+    } else {
+        false
+    }
+}
+
+impl<'tcx> LateLintPass<'tcx> for FnNullCheck {
+    fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
+        match expr.kind {
+            // Catching:
+            // (fn_ptr as *<const/mut> <ty>).is_null()
+            ExprKind::MethodCall(method_name, receiver, _, _)
+                if method_name.ident.as_str() == "is_null" && is_fn_ptr_cast(cx, receiver) =>
+            {
+                lint_expr(cx, expr);
+            },
+
+            ExprKind::Binary(op, left, right) if matches!(op.node, BinOpKind::Eq) => {
+                let to_check: &Expr<'_>;
+                if is_fn_ptr_cast(cx, left) {
+                    to_check = right;
+                } else if is_fn_ptr_cast(cx, right) {
+                    to_check = left;
+                } else {
+                    return;
+                }
+
+                match to_check.kind {
+                    // Catching:
+                    // (fn_ptr as *<const/mut> <ty>) == (0 as <ty>)
+                    ExprKind::Cast(cast_expr, _) if is_integer_literal(cast_expr, 0) => {
+                        lint_expr(cx, expr);
+                    },
+
+                    // Catching:
+                    // (fn_ptr as *<const/mut> <ty>) == std::ptr::null()
+                    ExprKind::Call(func, []) if is_path_diagnostic_item(cx, func, sym::ptr_null) => {
+                        lint_expr(cx, expr);
+                    },
+
+                    // Catching:
+                    // (fn_ptr as *<const/mut> <ty>) == <const that evaluates to null_ptr>
+                    _ if matches!(
+                        constant(cx, cx.typeck_results(), to_check),
+                        Some((Constant::RawPtr(0), _))
+                    ) =>
+                    {
+                        lint_expr(cx, expr);
+                    },
+
+                    _ => {},
+                }
+            },
+            _ => {},
+        }
+    }
+}
diff --git a/clippy_lints/src/format_args.rs b/clippy_lints/src/format_args.rs
@@ -382,7 +382,7 @@ fn check_format_in_format_args(
         call_site,
         &format!("`format!` in `{name}!` args"),
         |diag| {
-            diag.help(&format!(
+            diag.help(format!(
                 "combine the `format!(..)` arguments with the outer `{name}!(..)` call"
             ));
             diag.help("or consider changing `format!` to `format_args!`");

diff --git a/clippy_lints/src/functions/mod.rs b/clippy_lints/src/functions/mod.rs
@@ -63,30 +63,53 @@ declare_clippy_lint! {
     /// arguments but are not marked `unsafe`.
     ///
     /// ### Why is this bad?
-    /// The function should probably be marked `unsafe`, since
-    /// for an arbitrary raw pointer, there is no way of telling for sure if it is
-    /// valid.
+    /// The function should almost definitely be marked `unsafe`, since for an
+    /// arbitrary raw pointer, there is no way of telling for sure if it is valid.
+    ///
+    /// In general, this lint should **never be disabled** unless it is definitely a
+    /// false positive (please submit an issue if so) since it breaks Rust's
+    /// soundness guarantees, directly exposing API users to potentially dangerous
+    /// program behavior. This is also true for internal APIs, as it is easy to leak
+    /// unsoundness.
+    ///
+    /// ### Context
+    /// In Rust, an `unsafe {...}` block is used to indicate that the code in that
+    /// section has been verified in some way that the compiler can not. For a
+    /// function that accepts a raw pointer then accesses the pointer's data, this is
+    /// generally impossible as the incoming pointer could point anywhere, valid or
+    /// not. So, the signature should be marked `unsafe fn`: this indicates that the
+    /// function's caller must provide some verification that the arguments it sends
+    /// are valid (and then call the function within an `unsafe` block).
     ///
     /// ### Known problems
     /// * It does not check functions recursively so if the pointer is passed to a
     /// private non-`unsafe` function which does the dereferencing, the lint won't
-    /// trigger.
+    /// trigger (false negative).
     /// * It only checks for arguments whose type are raw pointers, not raw pointers
     /// got from an argument in some other way (`fn foo(bar: &[*const u8])` or
-    /// `some_argument.get_raw_ptr()`).
+    /// `some_argument.get_raw_ptr()`) (false negative).
     ///
     /// ### Example
     /// ```rust,ignore
     /// pub fn foo(x: *const u8) {
     ///     println!("{}", unsafe { *x });
     /// }
+    ///
+    /// // this call "looks" safe but will segfault or worse!
+    /// // foo(invalid_ptr);
     /// ```
     ///
     /// Use instead:
     /// ```rust,ignore
     /// pub unsafe fn foo(x: *const u8) {
     ///     println!("{}", unsafe { *x });
     /// }
+    ///
+    /// // this would cause a compiler error for calling without `unsafe`
+    /// // foo(invalid_ptr);
+    ///
+    /// // sound call if the caller knows the pointer is valid
+    /// unsafe { foo(valid_ptr); }
     /// ```
     #[clippy::version = "pre 1.29.0"]
     pub NOT_UNSAFE_PTR_ARG_DEREF,

diff --git a/clippy_lints/src/large_const_arrays.rs b/clippy_lints/src/large_const_arrays.rs
@@ -34,12 +34,12 @@ declare_clippy_lint! {
 }
 
 pub struct LargeConstArrays {
-    maximum_allowed_size: u64,
+    maximum_allowed_size: u128,
 }
 
 impl LargeConstArrays {
     #[must_use]
-    pub fn new(maximum_allowed_size: u64) -> Self {
+    pub fn new(maximum_allowed_size: u128) -> Self {
         Self { maximum_allowed_size }
     }
 }
@@ -56,7 +56,7 @@ impl<'tcx> LateLintPass<'tcx> for LargeConstArrays {
             if let ConstKind::Value(ty::ValTree::Leaf(element_count)) = cst.kind();
             if let Ok(element_count) = element_count.try_to_machine_usize(cx.tcx);
             if let Ok(element_size) = cx.layout_of(*element_type).map(|l| l.size.bytes());
-            if self.maximum_allowed_size < element_count * element_size;
+            if self.maximum_allowed_size < u128::from(element_count) * u128::from(element_size);
 
             then {
                 let hi_pos = item.ident.span.lo() - BytePos::from_usize(1);

diff --git a/clippy_lints/src/large_enum_variant.rs b/clippy_lints/src/large_enum_variant.rs
@@ -111,7 +111,7 @@ impl<'tcx> LateLintPass<'tcx> for LargeEnumVariant {
                         );
                         diag.span_label(
                             def.variants[variants_size[1].ind].span,
-                            &if variants_size[1].fields_size.is_empty() {
+                            if variants_size[1].fields_size.is_empty() {
                                 "the second-largest variant carries no data at all".to_owned()
                             } else {
                                 format!(

diff --git a/clippy_lints/src/large_stack_arrays.rs b/clippy_lints/src/large_stack_arrays.rs
@@ -24,12 +24,12 @@ declare_clippy_lint! {
 }
 
 pub struct LargeStackArrays {
-    maximum_allowed_size: u64,
+    maximum_allowed_size: u128,
 }
 
 impl LargeStackArrays {
     #[must_use]
-    pub fn new(maximum_allowed_size: u64) -> Self {
+    pub fn new(maximum_allowed_size: u128) -> Self {
         Self { maximum_allowed_size }
     }
 }
@@ -45,7 +45,7 @@ impl<'tcx> LateLintPass<'tcx> for LargeStackArrays {
           && let Ok(element_size) = cx.layout_of(*element_type).map(|l| l.size.bytes())
           && !cx.tcx.hir().parent_iter(expr.hir_id)
               .any(|(_, node)| matches!(node, Node::Item(Item { kind: ItemKind::Static(..), .. })))
-          && self.maximum_allowed_size < element_count * element_size {
+          && self.maximum_allowed_size < u128::from(element_count) * u128::from(element_size) {
               span_lint_and_help(
                   cx,
                   LARGE_STACK_ARRAYS,

diff --git a/clippy_lints/src/len_zero.rs b/clippy_lints/src/len_zero.rs
@@ -361,7 +361,7 @@ fn check_for_is_empty<'tcx>(
             db.span_note(span, "`is_empty` defined here");
         }
         if let Some(self_kind) = self_kind {
-            db.note(&output.expected_sig(self_kind));
+            db.note(output.expected_sig(self_kind));
         }
     });
 }

diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs
@@ -125,6 +125,7 @@ mod explicit_write;
 mod fallible_impl_from;
 mod float_literal;
 mod floating_point_arithmetic;
+mod fn_null_check;
 mod format;
 mod format_args;
 mod format_impl;
@@ -234,6 +235,7 @@ mod partialeq_ne_impl;
 mod partialeq_to_none;
 mod pass_by_ref_or_value;
 mod pattern_type_mismatch;
+mod permissions_set_readonly_false;
 mod precedence;
 mod ptr;
 mod ptr_offset_with_cast;
@@ -263,6 +265,7 @@ mod shadow;
 mod single_char_lifetime_names;
 mod single_component_path_imports;
 mod size_of_in_element_count;
+mod size_of_ref;
 mod slow_vector_initialization;
 mod std_instead_of_core;
 mod strings;
@@ -334,7 +337,7 @@ pub fn read_conf(sess: &Session, path: &io::Result<Option<PathBuf>>) -> Conf {
         Ok(Some(path)) => path,
         Ok(None) => return Conf::default(),
         Err(error) => {
-            sess.struct_err(&format!("error finding Clippy's configuration file: {error}"))
+            sess.struct_err(format!("error finding Clippy's configuration file: {error}"))
                 .emit();
             return Conf::default();
         },
@@ -902,6 +905,9 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
     store.register_late_pass(|_| Box::new(suspicious_xor_used_as_pow::ConfusingXorAndPow));
     store.register_late_pass(move |_| Box::new(manual_is_ascii_check::ManualIsAsciiCheck::new(msrv())));
     store.register_late_pass(|_| Box::new(semicolon_block::SemicolonBlock));
+    store.register_late_pass(|_| Box::new(fn_null_check::FnNullCheck));
+    store.register_late_pass(|_| Box::new(permissions_set_readonly_false::PermissionsSetReadonlyFalse));
+    store.register_late_pass(|_| Box::new(size_of_ref::SizeOfRef));
     // add lints here, do not remove this comment, it's used in `new_lint`
 }
 

diff --git a/clippy_lints/src/loops/explicit_counter_loop.rs b/clippy_lints/src/loops/explicit_counter_loop.rs
@@ -77,7 +77,7 @@ pub(super) fn check<'tcx>(
                                 applicability,
                             );
 
-                            diag.note(&format!(
+                            diag.note(format!(
                                 "`{name}` is of type `{int_name}`, making it ineligible for `Iterator::enumerate`"
                             ));
                         },

diff --git a/clippy_lints/src/manual_async_fn.rs b/clippy_lints/src/manual_async_fn.rs
@@ -76,7 +76,7 @@ impl<'tcx> LateLintPass<'tcx> for ManualAsyncFn {
                                 let help = format!("make the function `async` and {ret_sugg}");
                                 diag.span_suggestion(
                                     header_span,
-                                    &help,
+                                    help,
                                     format!("async {}{ret_snip}", &header_snip[..ret_pos]),
                                     Applicability::MachineApplicable
                                 );