diff options
Diffstat (limited to 'src')
83 files changed, 1995 insertions, 671 deletions
diff --git a/src/tools/clippy/Cargo.toml b/src/tools/clippy/Cargo.toml index f69e5bee4bb..3a76c61489e 100644 --- a/src/tools/clippy/Cargo.toml +++ b/src/tools/clippy/Cargo.toml @@ -42,7 +42,7 @@ walkdir = "2.3" filetime = "0.2.9" itertools = "0.12" pulldown-cmark = { version = "0.11", default-features = false, features = ["html"] } -askama = { version = "0.13", default-features = false, features = ["alloc", "config", "derive"] } +askama = { version = "0.14", default-features = false, features = ["alloc", "config", "derive"] } # UI test dependencies if_chain = "1.0" diff --git a/src/tools/clippy/book/src/development/trait_checking.md b/src/tools/clippy/book/src/development/trait_checking.md index cc4eb966f59..6d01496eebe 100644 --- a/src/tools/clippy/book/src/development/trait_checking.md +++ b/src/tools/clippy/book/src/development/trait_checking.md @@ -17,7 +17,7 @@ providing the `LateContext` (`cx`), our expression at hand, and the symbol of the trait in question: ```rust -use clippy_utils::is_trait_method; +use clippy_utils::ty::implements_trait; use rustc_hir::Expr; use rustc_lint::{LateContext, LateLintPass}; use rustc_span::symbol::sym; @@ -25,7 +25,7 @@ use rustc_span::symbol::sym; impl LateLintPass<'_> for CheckIteratorTraitLint { fn check_expr(&mut self, cx: &LateContext<'_>, expr: &Expr<'_>) { let implements_iterator = cx.tcx.get_diagnostic_item(sym::Iterator).map_or(false, |id| { - implements_trait(cx, cx.typeck_results().expr_ty(arg), id, &[]) + implements_trait(cx, cx.typeck_results().expr_ty(expr), id, &[]) }); if implements_iterator { // [...] diff --git a/src/tools/clippy/book/src/lint_configuration.md b/src/tools/clippy/book/src/lint_configuration.md index 9809e32de8a..7c850b4b023 100644 --- a/src/tools/clippy/book/src/lint_configuration.md +++ b/src/tools/clippy/book/src/lint_configuration.md @@ -1026,7 +1026,7 @@ The order of associated items in traits. The maximum size (in bytes) to consider a `Copy` type for passing by value instead of by reference. -**Default Value:** `target_pointer_width * 2` +**Default Value:** `target_pointer_width` --- **Affected lints:** diff --git a/src/tools/clippy/clippy_config/src/conf.rs b/src/tools/clippy/clippy_config/src/conf.rs index 4ce8d001c2f..87158cec42b 100644 --- a/src/tools/clippy/clippy_config/src/conf.rs +++ b/src/tools/clippy/clippy_config/src/conf.rs @@ -828,7 +828,7 @@ define_Conf! { trait_assoc_item_kinds_order: SourceItemOrderingTraitAssocItemKinds = DEFAULT_TRAIT_ASSOC_ITEM_KINDS_ORDER.into(), /// The maximum size (in bytes) to consider a `Copy` type for passing by value instead of by /// reference. - #[default_text = "target_pointer_width * 2"] + #[default_text = "target_pointer_width"] #[lints(trivially_copy_pass_by_ref)] trivial_copy_size_limit: Option<u64> = None, /// The maximum complexity a type can have diff --git a/src/tools/clippy/clippy_dev/src/fmt.rs b/src/tools/clippy/clippy_dev/src/fmt.rs index 13d6b1285dc..c1b6b370706 100644 --- a/src/tools/clippy/clippy_dev/src/fmt.rs +++ b/src/tools/clippy/clippy_dev/src/fmt.rs @@ -1,5 +1,6 @@ use crate::utils::{ - ClippyInfo, ErrAction, FileUpdater, UpdateMode, UpdateStatus, panic_action, run_with_args_split, run_with_output, + ErrAction, FileUpdater, UpdateMode, UpdateStatus, expect_action, run_with_output, split_args_for_threads, + walk_dir_no_dot_or_target, }; use itertools::Itertools; use rustc_lexer::{TokenKind, tokenize}; @@ -9,7 +10,6 @@ use std::io::{self, Read}; use std::ops::ControlFlow; use std::path::PathBuf; use std::process::{self, Command, Stdio}; -use walkdir::WalkDir; pub enum Error { Io(io::Error), @@ -260,7 +260,7 @@ fn fmt_syms(update_mode: UpdateMode) { ); } -fn run_rustfmt(clippy: &ClippyInfo, update_mode: UpdateMode) { +fn run_rustfmt(update_mode: UpdateMode) { let mut rustfmt_path = String::from_utf8(run_with_output( "rustup which rustfmt", Command::new("rustup").args(["which", "rustfmt"]), @@ -268,42 +268,19 @@ fn run_rustfmt(clippy: &ClippyInfo, update_mode: UpdateMode) { .expect("invalid rustfmt path"); rustfmt_path.truncate(rustfmt_path.trim_end().len()); - let mut cargo_path = String::from_utf8(run_with_output( - "rustup which cargo", - Command::new("rustup").args(["which", "cargo"]), - )) - .expect("invalid cargo path"); - cargo_path.truncate(cargo_path.trim_end().len()); - - // Start all format jobs first before waiting on the results. - let mut children = Vec::with_capacity(16); - for &path in &[ - ".", - "clippy_config", - "clippy_dev", - "clippy_lints", - "clippy_lints_internal", - "clippy_utils", - "rustc_tools_util", - "lintcheck", - ] { - let mut cmd = Command::new(&cargo_path); - cmd.current_dir(clippy.path.join(path)) - .args(["fmt"]) - .env("RUSTFMT", &rustfmt_path) - .stdout(Stdio::null()) - .stdin(Stdio::null()) - .stderr(Stdio::piped()); - if update_mode.is_check() { - cmd.arg("--check"); - } - match cmd.spawn() { - Ok(x) => children.push(("cargo fmt", x)), - Err(ref e) => panic_action(&e, ErrAction::Run, "cargo fmt".as_ref()), - } - } + let args: Vec<_> = walk_dir_no_dot_or_target() + .filter_map(|e| { + let e = expect_action(e, ErrAction::Read, "."); + e.path() + .as_os_str() + .as_encoded_bytes() + .ends_with(b".rs") + .then(|| e.into_path().into_os_string()) + }) + .collect(); - run_with_args_split( + let mut children: Vec<_> = split_args_for_threads( + 32, || { let mut cmd = Command::new(&rustfmt_path); if update_mode.is_check() { @@ -312,66 +289,44 @@ fn run_rustfmt(clippy: &ClippyInfo, update_mode: UpdateMode) { cmd.stdout(Stdio::null()) .stdin(Stdio::null()) .stderr(Stdio::piped()) - .args(["--config", "show_parse_errors=false"]); + .args(["--unstable-features", "--skip-children"]); cmd }, - |cmd| match cmd.spawn() { - Ok(x) => children.push(("rustfmt", x)), - Err(ref e) => panic_action(&e, ErrAction::Run, "rustfmt".as_ref()), - }, - WalkDir::new("tests") - .into_iter() - .filter_entry(|p| p.path().file_name().is_none_or(|x| x != "skip_rustfmt")) - .filter_map(|e| { - let e = e.expect("error reading `tests`"); - e.path() - .as_os_str() - .as_encoded_bytes() - .ends_with(b".rs") - .then(|| e.into_path().into_os_string()) - }), - ); + args.iter(), + ) + .map(|mut cmd| expect_action(cmd.spawn(), ErrAction::Run, "rustfmt")) + .collect(); - for (name, child) in &mut children { - match child.wait() { - Ok(status) => match (update_mode, status.exit_ok()) { - (UpdateMode::Check | UpdateMode::Change, Ok(())) => {}, - (UpdateMode::Check, Err(_)) => { - let mut s = String::new(); - if let Some(mut stderr) = child.stderr.take() - && stderr.read_to_string(&mut s).is_ok() - { - eprintln!("{s}"); - } - eprintln!("Formatting check failed!\nRun `cargo dev fmt` to update."); - process::exit(1); - }, - (UpdateMode::Change, Err(e)) => { - let mut s = String::new(); - if let Some(mut stderr) = child.stderr.take() - && stderr.read_to_string(&mut s).is_ok() - { - eprintln!("{s}"); - } - panic_action(&e, ErrAction::Run, name.as_ref()); - }, + for child in &mut children { + let status = expect_action(child.wait(), ErrAction::Run, "rustfmt"); + match (update_mode, status.exit_ok()) { + (UpdateMode::Check | UpdateMode::Change, Ok(())) => {}, + (UpdateMode::Check, Err(_)) => { + let mut s = String::new(); + if let Some(mut stderr) = child.stderr.take() + && stderr.read_to_string(&mut s).is_ok() + { + eprintln!("{s}"); + } + eprintln!("Formatting check failed!\nRun `cargo dev fmt` to update."); + process::exit(1); + }, + (UpdateMode::Change, e) => { + let mut s = String::new(); + if let Some(mut stderr) = child.stderr.take() + && stderr.read_to_string(&mut s).is_ok() + { + eprintln!("{s}"); + } + expect_action(e, ErrAction::Run, "rustfmt"); }, - Err(ref e) => panic_action(e, ErrAction::Run, name.as_ref()), } } } // the "main" function of cargo dev fmt -pub fn run(clippy: &ClippyInfo, update_mode: UpdateMode) { - if clippy.has_intellij_hook { - eprintln!( - "error: a local rustc repo is enabled as path dependency via `cargo dev setup intellij`.\n\ - Not formatting because that would format the local repo as well!\n\ - Please revert the changes to `Cargo.toml`s with `cargo dev remove intellij`." - ); - return; - } - run_rustfmt(clippy, update_mode); +pub fn run(update_mode: UpdateMode) { + run_rustfmt(update_mode); fmt_syms(update_mode); if let Err(e) = fmt_conf(update_mode.is_check()) { e.display(); diff --git a/src/tools/clippy/clippy_dev/src/main.rs b/src/tools/clippy/clippy_dev/src/main.rs index ebcd8611d78..26aa269fb63 100644 --- a/src/tools/clippy/clippy_dev/src/main.rs +++ b/src/tools/clippy/clippy_dev/src/main.rs @@ -26,7 +26,7 @@ fn main() { allow_staged, allow_no_vcs, } => dogfood::dogfood(fix, allow_dirty, allow_staged, allow_no_vcs), - DevCommand::Fmt { check } => fmt::run(&clippy, utils::UpdateMode::from_check(check)), + DevCommand::Fmt { check } => fmt::run(utils::UpdateMode::from_check(check)), DevCommand::UpdateLints { check } => update_lints::update(utils::UpdateMode::from_check(check)), DevCommand::NewLint { pass, diff --git a/src/tools/clippy/clippy_dev/src/rename_lint.rs b/src/tools/clippy/clippy_dev/src/rename_lint.rs index be8b27c7a9e..d62597428e2 100644 --- a/src/tools/clippy/clippy_dev/src/rename_lint.rs +++ b/src/tools/clippy/clippy_dev/src/rename_lint.rs @@ -1,13 +1,12 @@ use crate::update_lints::{RenamedLint, find_lint_decls, generate_lint_files, read_deprecated_lints}; use crate::utils::{ - FileUpdater, RustSearcher, Token, UpdateMode, UpdateStatus, Version, delete_dir_if_exists, delete_file_if_exists, - try_rename_dir, try_rename_file, + ErrAction, FileUpdater, RustSearcher, Token, UpdateMode, UpdateStatus, Version, delete_dir_if_exists, + delete_file_if_exists, expect_action, try_rename_dir, try_rename_file, walk_dir_no_dot_or_target, }; use rustc_lexer::TokenKind; use std::ffi::OsString; use std::fs; use std::path::Path; -use walkdir::WalkDir; /// Runs the `rename_lint` command. /// @@ -133,17 +132,10 @@ pub fn rename(clippy_version: Version, old_name: &str, new_name: &str, uplift: b } let mut update_fn = file_update_fn(old_name, new_name, mod_edit); - for file in WalkDir::new(".").into_iter().filter_entry(|e| { - // Skip traversing some of the larger directories. - e.path() - .as_os_str() - .as_encoded_bytes() - .get(2..) - .is_none_or(|x| x != "target".as_bytes() && x != ".git".as_bytes()) - }) { - let file = file.expect("error reading clippy directory"); - if file.path().as_os_str().as_encoded_bytes().ends_with(b".rs") { - updater.update_file(file.path(), &mut update_fn); + for e in walk_dir_no_dot_or_target() { + let e = expect_action(e, ErrAction::Read, "."); + if e.path().as_os_str().as_encoded_bytes().ends_with(b".rs") { + updater.update_file(e.path(), &mut update_fn); } } generate_lint_files(UpdateMode::Change, &lints, &deprecated_lints, &renamed_lints); diff --git a/src/tools/clippy/clippy_dev/src/update_lints.rs b/src/tools/clippy/clippy_dev/src/update_lints.rs index 25ba2c72049..320462a2c96 100644 --- a/src/tools/clippy/clippy_dev/src/update_lints.rs +++ b/src/tools/clippy/clippy_dev/src/update_lints.rs @@ -1,5 +1,5 @@ use crate::utils::{ - ErrAction, File, FileUpdater, RustSearcher, Token, UpdateMode, UpdateStatus, panic_action, update_text_region_fn, + ErrAction, File, FileUpdater, RustSearcher, Token, UpdateMode, UpdateStatus, expect_action, update_text_region_fn, }; use itertools::Itertools; use std::collections::HashSet; @@ -201,10 +201,7 @@ pub fn find_lint_decls() -> Vec<Lint> { /// Reads the source files from the given root directory fn read_src_with_module(src_root: &Path) -> impl use<'_> + Iterator<Item = (DirEntry, String)> { WalkDir::new(src_root).into_iter().filter_map(move |e| { - let e = match e { - Ok(e) => e, - Err(ref e) => panic_action(e, ErrAction::Read, src_root), - }; + let e = expect_action(e, ErrAction::Read, src_root); let path = e.path().as_os_str().as_encoded_bytes(); if let Some(path) = path.strip_suffix(b".rs") && let Some(path) = path.get("clippy_lints/src/".len()..) diff --git a/src/tools/clippy/clippy_dev/src/utils.rs b/src/tools/clippy/clippy_dev/src/utils.rs index 255e36afe69..c4808b7048b 100644 --- a/src/tools/clippy/clippy_dev/src/utils.rs +++ b/src/tools/clippy/clippy_dev/src/utils.rs @@ -1,14 +1,16 @@ use core::fmt::{self, Display}; +use core::num::NonZero; use core::ops::Range; use core::slice; use core::str::FromStr; use rustc_lexer::{self as lexer, FrontmatterAllowed}; -use std::env; use std::ffi::OsStr; use std::fs::{self, OpenOptions}; use std::io::{self, Read as _, Seek as _, SeekFrom, Write}; use std::path::{Path, PathBuf}; use std::process::{self, Command, ExitStatus, Stdio}; +use std::{env, thread}; +use walkdir::WalkDir; #[cfg(not(windows))] static CARGO_CLIPPY_EXE: &str = "cargo-clippy"; @@ -45,6 +47,14 @@ pub fn panic_action(err: &impl Display, action: ErrAction, path: &Path) -> ! { panic!("error {} `{}`: {}", action.as_str(), path.display(), *err) } +#[track_caller] +pub fn expect_action<T>(res: Result<T, impl Display>, action: ErrAction, path: impl AsRef<Path>) -> T { + match res { + Ok(x) => x, + Err(ref e) => panic_action(e, action, path.as_ref()), + } +} + /// Wrapper around `std::fs::File` which panics with a path on failure. pub struct File<'a> { pub inner: fs::File, @@ -55,9 +65,9 @@ impl<'a> File<'a> { #[track_caller] pub fn open(path: &'a (impl AsRef<Path> + ?Sized), options: &mut OpenOptions) -> Self { let path = path.as_ref(); - match options.open(path) { - Ok(inner) => Self { inner, path }, - Err(e) => panic_action(&e, ErrAction::Open, path), + Self { + inner: expect_action(options.open(path), ErrAction::Open, path), + path, } } @@ -84,10 +94,7 @@ impl<'a> File<'a> { /// Read the entire contents of a file to the given buffer. #[track_caller] pub fn read_append_to_string<'dst>(&mut self, dst: &'dst mut String) -> &'dst mut String { - match self.inner.read_to_string(dst) { - Ok(_) => {}, - Err(e) => panic_action(&e, ErrAction::Read, self.path), - } + expect_action(self.inner.read_to_string(dst), ErrAction::Read, self.path); dst } @@ -107,9 +114,7 @@ impl<'a> File<'a> { }, Err(e) => Err(e), }; - if let Err(e) = res { - panic_action(&e, ErrAction::Write, self.path); - } + expect_action(res, ErrAction::Write, self.path); } } @@ -660,47 +665,91 @@ pub fn try_rename_dir(old_name: &Path, new_name: &Path) -> bool { } pub fn write_file(path: &Path, contents: &str) { - fs::write(path, contents).unwrap_or_else(|e| panic_action(&e, ErrAction::Write, path)); + expect_action(fs::write(path, contents), ErrAction::Write, path); } #[must_use] pub fn run_with_output(path: &(impl AsRef<Path> + ?Sized), cmd: &mut Command) -> Vec<u8> { fn f(path: &Path, cmd: &mut Command) -> Vec<u8> { - match cmd - .stdin(Stdio::null()) - .stdout(Stdio::piped()) - .stderr(Stdio::inherit()) - .output() - { - Ok(x) => match x.status.exit_ok() { - Ok(()) => x.stdout, - Err(ref e) => panic_action(e, ErrAction::Run, path), - }, - Err(ref e) => panic_action(e, ErrAction::Run, path), - } + let output = expect_action( + cmd.stdin(Stdio::null()) + .stdout(Stdio::piped()) + .stderr(Stdio::inherit()) + .output(), + ErrAction::Run, + path, + ); + expect_action(output.status.exit_ok(), ErrAction::Run, path); + output.stdout } f(path.as_ref(), cmd) } -pub fn run_with_args_split( - mut make_cmd: impl FnMut() -> Command, - mut run_cmd: impl FnMut(&mut Command), - args: impl Iterator<Item: AsRef<OsStr>>, -) { - let mut cmd = make_cmd(); - let mut len = 0; - for arg in args { - len += arg.as_ref().len(); - cmd.arg(arg); - // Very conservative limit - if len > 10000 { - run_cmd(&mut cmd); - cmd = make_cmd(); - len = 0; +/// Splits an argument list across multiple `Command` invocations. +/// +/// The argument list will be split into a number of batches based on +/// `thread::available_parallelism`, with `min_batch_size` setting a lower bound on the size of each +/// batch. +/// +/// If the size of the arguments would exceed the system limit additional batches will be created. +pub fn split_args_for_threads( + min_batch_size: usize, + make_cmd: impl FnMut() -> Command, + args: impl ExactSizeIterator<Item: AsRef<OsStr>>, +) -> impl Iterator<Item = Command> { + struct Iter<F, I> { + make_cmd: F, + args: I, + min_batch_size: usize, + batch_size: usize, + thread_count: usize, + } + impl<F, I> Iterator for Iter<F, I> + where + F: FnMut() -> Command, + I: ExactSizeIterator<Item: AsRef<OsStr>>, + { + type Item = Command; + fn next(&mut self) -> Option<Self::Item> { + if self.thread_count > 1 { + self.thread_count -= 1; + } + let mut cmd = (self.make_cmd)(); + let mut cmd_len = 0usize; + for arg in self.args.by_ref().take(self.batch_size) { + cmd.arg(arg.as_ref()); + // `+ 8` to account for the `argv` pointer on unix. + // Windows is complicated since the arguments are first converted to UTF-16ish, + // but this needs to account for the space between arguments and whatever additional + // is needed to escape within an argument. + cmd_len += arg.as_ref().len() + 8; + cmd_len += 8; + + // Windows has a command length limit of 32767. For unix systems this is more + // complicated since the limit includes environment variables and room needs to be + // left to edit them once the program starts, but the total size comes from + // `getconf ARG_MAX`. + // + // For simplicity we use 30000 here under a few assumptions. + // * Individual arguments aren't super long (the final argument is still added) + // * `ARG_MAX` is set to a reasonable amount. Basically every system will be configured way above + // what windows supports, but POSIX only requires `4096`. + if cmd_len > 30000 { + self.batch_size = self.args.len().div_ceil(self.thread_count).max(self.min_batch_size); + break; + } + } + (cmd_len != 0).then_some(cmd) } } - if len != 0 { - run_cmd(&mut cmd); + let thread_count = thread::available_parallelism().map_or(1, NonZero::get); + let batch_size = args.len().div_ceil(thread_count).max(min_batch_size); + Iter { + make_cmd, + args, + min_batch_size, + batch_size, + thread_count, } } @@ -720,3 +769,12 @@ pub fn delete_dir_if_exists(path: &Path) { Err(ref e) => panic_action(e, ErrAction::Delete, path), } } + +/// Walks all items excluding top-level dot files/directories and any target directories. +pub fn walk_dir_no_dot_or_target() -> impl Iterator<Item = ::walkdir::Result<::walkdir::DirEntry>> { + WalkDir::new(".").into_iter().filter_entry(|e| { + e.path() + .file_name() + .is_none_or(|x| x != "target" && x.as_encoded_bytes().first().copied() != Some(b'.')) + }) +} diff --git a/src/tools/clippy/clippy_lints/src/dbg_macro.rs b/src/tools/clippy/clippy_lints/src/dbg_macro.rs index 06376c57119..152516baf73 100644 --- a/src/tools/clippy/clippy_lints/src/dbg_macro.rs +++ b/src/tools/clippy/clippy_lints/src/dbg_macro.rs @@ -5,7 +5,7 @@ use clippy_utils::macros::{MacroCall, macro_backtrace}; use clippy_utils::source::snippet_with_applicability; use rustc_data_structures::fx::FxHashSet; use rustc_errors::Applicability; -use rustc_hir::{Expr, ExprKind, Node}; +use rustc_hir::{Closure, ClosureKind, CoroutineKind, Expr, ExprKind, LetStmt, LocalSource, Node, Stmt, StmtKind}; use rustc_lint::{LateContext, LateLintPass, LintContext}; use rustc_session::impl_lint_pass; use rustc_span::{Span, SyntaxContext, sym}; @@ -60,6 +60,8 @@ impl LateLintPass<'_> for DbgMacro { if cur_syntax_ctxt != self.prev_ctxt && let Some(macro_call) = first_dbg_macro_in_expansion(cx, expr.span) && !macro_call.span.in_external_macro(cx.sess().source_map()) && + // avoids exprs generated by the desugaring of coroutines + !is_coroutine_desugar(expr) && self.checked_dbg_call_site.insert(macro_call.span) && // allows `dbg!` in test code if allow-dbg-in-test is set to true in clippy.toml !(self.allow_dbg_in_tests && is_in_test(cx.tcx, expr.hir_id)) @@ -73,50 +75,51 @@ impl LateLintPass<'_> for DbgMacro { "the `dbg!` macro is intended as a debugging tool", |diag| { let mut applicability = Applicability::MachineApplicable; - - let (sugg_span, suggestion) = match expr.peel_drop_temps().kind { - // dbg!() - ExprKind::Block(..) => { - // If the `dbg!` macro is a "free" statement and not contained within other expressions, - // remove the whole statement. - if let Node::Stmt(_) = cx.tcx.parent_hir_node(expr.hir_id) - && let Some(semi_span) = cx.sess().source_map().mac_call_stmt_semi_span(macro_call.span) - { - (macro_call.span.to(semi_span), String::new()) - } else { - (macro_call.span, String::from("()")) - } - }, - // dbg!(1) - ExprKind::Match(val, ..) => ( - macro_call.span, - snippet_with_applicability(cx, val.span.source_callsite(), "..", &mut applicability) - .to_string(), - ), - // dbg!(2, 3) - ExprKind::Tup( - [ - Expr { - kind: ExprKind::Match(first, ..), - .. - }, - .., - Expr { - kind: ExprKind::Match(last, ..), - .. - }, - ], - ) => { - let snippet = snippet_with_applicability( - cx, - first.span.source_callsite().to(last.span.source_callsite()), - "..", - &mut applicability, - ); - (macro_call.span, format!("({snippet})")) - }, - _ => unreachable!(), - }; + let (sugg_span, suggestion) = + match is_async_move_desugar(expr).unwrap_or(expr).peel_drop_temps().kind { + // dbg!() + ExprKind::Block(..) => { + // If the `dbg!` macro is a "free" statement and not contained within other expressions, + // remove the whole statement. + if let Node::Stmt(_) = cx.tcx.parent_hir_node(expr.hir_id) + && let Some(semi_span) = + cx.sess().source_map().mac_call_stmt_semi_span(macro_call.span) + { + (macro_call.span.to(semi_span), String::new()) + } else { + (macro_call.span, String::from("()")) + } + }, + // dbg!(1) + ExprKind::Match(val, ..) => ( + macro_call.span, + snippet_with_applicability(cx, val.span.source_callsite(), "..", &mut applicability) + .to_string(), + ), + // dbg!(2, 3) + ExprKind::Tup( + [ + Expr { + kind: ExprKind::Match(first, ..), + .. + }, + .., + Expr { + kind: ExprKind::Match(last, ..), + .. + }, + ], + ) => { + let snippet = snippet_with_applicability( + cx, + first.span.source_callsite().to(last.span.source_callsite()), + "..", + &mut applicability, + ); + (macro_call.span, format!("({snippet})")) + }, + _ => unreachable!(), + }; diag.span_suggestion( sugg_span, @@ -134,6 +137,35 @@ impl LateLintPass<'_> for DbgMacro { } } +fn is_coroutine_desugar(expr: &Expr<'_>) -> bool { + matches!( + expr.kind, + ExprKind::Closure(Closure { + kind: ClosureKind::Coroutine(CoroutineKind::Desugared(..)) | ClosureKind::CoroutineClosure(..), + .. + }) + ) +} + +fn is_async_move_desugar<'tcx>(expr: &'tcx Expr<'tcx>) -> Option<&'tcx Expr<'tcx>> { + if let ExprKind::Block(block, _) = expr.kind + && let [ + Stmt { + kind: + StmtKind::Let(LetStmt { + source: LocalSource::AsyncFn, + .. + }), + .. + }, + ] = block.stmts + { + return block.expr; + } + + None +} + fn first_dbg_macro_in_expansion(cx: &LateContext<'_>, span: Span) -> Option<MacroCall> { macro_backtrace(span).find(|mc| cx.tcx.is_diagnostic_item(sym::dbg_macro, mc.def_id)) } diff --git a/src/tools/clippy/clippy_lints/src/dereference.rs b/src/tools/clippy/clippy_lints/src/dereference.rs index a22a2ee66d2..cde9528cd87 100644 --- a/src/tools/clippy/clippy_lints/src/dereference.rs +++ b/src/tools/clippy/clippy_lints/src/dereference.rs @@ -305,7 +305,7 @@ impl<'tcx> LateLintPass<'tcx> for Dereferencing<'tcx> { RefOp::Method { mutbl, is_ufcs } if !is_lint_allowed(cx, EXPLICIT_DEREF_METHODS, expr.hir_id) // Allow explicit deref in method chains. e.g. `foo.deref().bar()` - && (is_ufcs || !in_postfix_position(cx, expr)) => + && (is_ufcs || !is_in_method_chain(cx, expr)) => { let ty_changed_count = usize::from(!deref_method_same_type(expr_ty, typeck.expr_ty(sub_expr))); self.state = Some(( @@ -728,7 +728,13 @@ fn deref_method_same_type<'tcx>(result_ty: Ty<'tcx>, arg_ty: Ty<'tcx>) -> bool { } } -fn in_postfix_position<'tcx>(cx: &LateContext<'tcx>, e: &'tcx Expr<'tcx>) -> bool { +fn is_in_method_chain<'tcx>(cx: &LateContext<'tcx>, e: &'tcx Expr<'tcx>) -> bool { + if let ExprKind::MethodCall(_, recv, _, _) = e.kind + && matches!(recv.kind, ExprKind::MethodCall(..)) + { + return true; + } + if let Some(parent) = get_parent_expr(cx, e) && parent.span.eq_ctxt(e.span) { @@ -986,6 +992,15 @@ fn report<'tcx>( ); }, State::DerefedBorrow(state) => { + // Do not suggest removing a non-mandatory `&` in `&*rawptr` in an `unsafe` context, + // as this may make rustc trigger its `dangerous_implicit_autorefs` lint. + if let ExprKind::AddrOf(BorrowKind::Ref, _, subexpr) = data.first_expr.kind + && let ExprKind::Unary(UnOp::Deref, subsubexpr) = subexpr.kind + && cx.typeck_results().expr_ty_adjusted(subsubexpr).is_raw_ptr() + { + return; + } + let mut app = Applicability::MachineApplicable; let (snip, snip_is_macro) = snippet_with_context(cx, expr.span, data.first_expr.span.ctxt(), "..", &mut app); diff --git a/src/tools/clippy/clippy_lints/src/doc/lazy_continuation.rs b/src/tools/clippy/clippy_lints/src/doc/lazy_continuation.rs index 8aeb835fe39..cb9d68224db 100644 --- a/src/tools/clippy/clippy_lints/src/doc/lazy_continuation.rs +++ b/src/tools/clippy/clippy_lints/src/doc/lazy_continuation.rs @@ -2,11 +2,10 @@ use clippy_utils::diagnostics::{span_lint_and_sugg, span_lint_and_then}; use itertools::Itertools; use rustc_errors::Applicability; use rustc_lint::LateContext; -use rustc_span::{BytePos, Span}; -use std::cmp::Ordering; +use rustc_span::BytePos; use std::ops::Range; -use super::{DOC_LAZY_CONTINUATION, DOC_OVERINDENTED_LIST_ITEMS}; +use super::{DOC_LAZY_CONTINUATION, DOC_OVERINDENTED_LIST_ITEMS, Fragments}; fn map_container_to_text(c: &super::Container) -> &'static str { match c { @@ -19,29 +18,27 @@ fn map_container_to_text(c: &super::Container) -> &'static str { pub(super) fn check( cx: &LateContext<'_>, doc: &str, - range: Range<usize>, - mut span: Span, + cooked_range: Range<usize>, + fragments: &Fragments<'_>, containers: &[super::Container], ) { - if doc[range.clone()].contains('\t') { - // We don't do tab stops correctly. - return; - } - - // Blockquote - let ccount = doc[range.clone()].chars().filter(|c| *c == '>').count(); + // Get blockquotes + let ccount = doc[cooked_range.clone()].chars().filter(|c| *c == '>').count(); let blockquote_level = containers .iter() .filter(|c| matches!(c, super::Container::Blockquote)) .count(); - if ccount < blockquote_level { + + if ccount < blockquote_level + && let Some(mut span) = fragments.span(cx, cooked_range.clone()) + { span_lint_and_then( cx, DOC_LAZY_CONTINUATION, span, "doc quote line without `>` marker", |diag| { - let mut doc_start_range = &doc[range]; + let mut doc_start_range = &doc[cooked_range]; let mut suggested = String::new(); for c in containers { let text = map_container_to_text(c); @@ -78,7 +75,7 @@ pub(super) fn check( } // List - let leading_spaces = doc[range].chars().filter(|c| *c == ' ').count(); + let leading_spaces = doc[cooked_range.clone()].chars().filter(|c| *c == ' ').count(); let list_indentation = containers .iter() .map(|c| { @@ -89,36 +86,41 @@ pub(super) fn check( } }) .sum(); - match leading_spaces.cmp(&list_indentation) { - Ordering::Less => span_lint_and_then( - cx, - DOC_LAZY_CONTINUATION, - span, - "doc list item without indentation", - |diag| { - // simpler suggestion style for indentation - let indent = list_indentation - leading_spaces; - diag.span_suggestion_verbose( - span.shrink_to_hi(), - "indent this line", - std::iter::repeat_n(" ", indent).join(""), - Applicability::MaybeIncorrect, - ); - diag.help("if this is supposed to be its own paragraph, add a blank line"); - }, - ), - Ordering::Greater => { - let sugg = std::iter::repeat_n(" ", list_indentation).join(""); - span_lint_and_sugg( + + if leading_spaces != list_indentation + && let Some(span) = fragments.span(cx, cooked_range.clone()) + { + if leading_spaces < list_indentation { + span_lint_and_then( cx, - DOC_OVERINDENTED_LIST_ITEMS, + DOC_LAZY_CONTINUATION, span, - "doc list item overindented", - format!("try using `{sugg}` ({list_indentation} spaces)"), - sugg, - Applicability::MaybeIncorrect, + "doc list item without indentation", + |diag| { + // simpler suggestion style for indentation + let indent = list_indentation - leading_spaces; + diag.span_suggestion_verbose( + span.shrink_to_hi(), + "indent this line", + std::iter::repeat_n(" ", indent).join(""), + Applicability::MaybeIncorrect, + ); + diag.help("if this is supposed to be its own paragraph, add a blank line"); + }, ); - }, - Ordering::Equal => {}, + + return; + } + + let sugg = std::iter::repeat_n(" ", list_indentation).join(""); + span_lint_and_sugg( + cx, + DOC_OVERINDENTED_LIST_ITEMS, + span, + "doc list item overindented", + format!("try using `{sugg}` ({list_indentation} spaces)"), + sugg, + Applicability::MaybeIncorrect, + ); } } diff --git a/src/tools/clippy/clippy_lints/src/doc/markdown.rs b/src/tools/clippy/clippy_lints/src/doc/markdown.rs index 7a1c7c675d2..69c3b9150c3 100644 --- a/src/tools/clippy/clippy_lints/src/doc/markdown.rs +++ b/src/tools/clippy/clippy_lints/src/doc/markdown.rs @@ -6,13 +6,15 @@ use rustc_lint::LateContext; use rustc_span::{BytePos, Pos, Span}; use url::Url; -use crate::doc::DOC_MARKDOWN; +use crate::doc::{DOC_MARKDOWN, Fragments}; +use std::ops::Range; pub fn check( cx: &LateContext<'_>, valid_idents: &FxHashSet<String>, text: &str, - span: Span, + fragments: &Fragments<'_>, + fragment_range: Range<usize>, code_level: isize, blockquote_level: isize, ) { @@ -64,20 +66,31 @@ pub fn check( close_parens += 1; } + // We'll use this offset to calculate the span to lint. + let fragment_offset = word.as_ptr() as usize - text.as_ptr() as usize; + // Adjust for the current word - let offset = word.as_ptr() as usize - text.as_ptr() as usize; - let span = Span::new( - span.lo() + BytePos::from_usize(offset), - span.lo() + BytePos::from_usize(offset + word.len()), - span.ctxt(), - span.parent(), + check_word( + cx, + word, + fragments, + &fragment_range, + fragment_offset, + code_level, + blockquote_level, ); - - check_word(cx, word, span, code_level, blockquote_level); } } -fn check_word(cx: &LateContext<'_>, word: &str, span: Span, code_level: isize, blockquote_level: isize) { +fn check_word( + cx: &LateContext<'_>, + word: &str, + fragments: &Fragments<'_>, + range: &Range<usize>, + fragment_offset: usize, + code_level: isize, + blockquote_level: isize, +) { /// Checks if a string is upper-camel-case, i.e., starts with an uppercase and /// contains at least two uppercase letters (`Clippy` is ok) and one lower-case /// letter (`NASA` is ok). @@ -117,6 +130,16 @@ fn check_word(cx: &LateContext<'_>, word: &str, span: Span, code_level: isize, b // try to get around the fact that `foo::bar` parses as a valid URL && !url.cannot_be_a_base() { + let Some(fragment_span) = fragments.span(cx, range.clone()) else { + return; + }; + let span = Span::new( + fragment_span.lo() + BytePos::from_usize(fragment_offset), + fragment_span.lo() + BytePos::from_usize(fragment_offset + word.len()), + fragment_span.ctxt(), + fragment_span.parent(), + ); + span_lint_and_sugg( cx, DOC_MARKDOWN, @@ -137,6 +160,17 @@ fn check_word(cx: &LateContext<'_>, word: &str, span: Span, code_level: isize, b } if has_underscore(word) || word.contains("::") || is_camel_case(word) || word.ends_with("()") { + let Some(fragment_span) = fragments.span(cx, range.clone()) else { + return; + }; + + let span = Span::new( + fragment_span.lo() + BytePos::from_usize(fragment_offset), + fragment_span.lo() + BytePos::from_usize(fragment_offset + word.len()), + fragment_span.ctxt(), + fragment_span.parent(), + ); + span_lint_and_then( cx, DOC_MARKDOWN, diff --git a/src/tools/clippy/clippy_lints/src/doc/mod.rs b/src/tools/clippy/clippy_lints/src/doc/mod.rs index 87da380e954..c46dd09d60c 100644 --- a/src/tools/clippy/clippy_lints/src/doc/mod.rs +++ b/src/tools/clippy/clippy_lints/src/doc/mod.rs @@ -3,7 +3,6 @@ use clippy_config::Conf; use clippy_utils::attrs::is_doc_hidden; use clippy_utils::diagnostics::{span_lint, span_lint_and_help, span_lint_and_then}; -use clippy_utils::source::snippet_opt; use clippy_utils::{is_entrypoint_fn, is_trait_impl_item}; use pulldown_cmark::Event::{ Code, DisplayMath, End, FootnoteReference, HardBreak, Html, InlineHtml, InlineMath, Rule, SoftBreak, Start, @@ -730,7 +729,10 @@ struct Fragments<'a> { } impl Fragments<'_> { - fn span(self, cx: &LateContext<'_>, range: Range<usize>) -> Option<Span> { + /// get the span for the markdown range. Note that this function is not cheap, use it with + /// caution. + #[must_use] + fn span(&self, cx: &LateContext<'_>, range: Range<usize>) -> Option<Span> { source_span_for_markdown_range(cx.tcx, self.doc, &range, self.fragments) } } @@ -1068,9 +1070,7 @@ fn check_doc<'a, Events: Iterator<Item = (pulldown_cmark::Event<'a>, Range<usize ); } else { for (text, range, assoc_code_level) in text_to_check { - if let Some(span) = fragments.span(cx, range) { - markdown::check(cx, valid_idents, &text, span, assoc_code_level, blockquote_level); - } + markdown::check(cx, valid_idents, &text, &fragments, range, assoc_code_level, blockquote_level); } } text_to_check = Vec::new(); @@ -1081,26 +1081,27 @@ fn check_doc<'a, Events: Iterator<Item = (pulldown_cmark::Event<'a>, Range<usize | TaskListMarker(_) | Code(_) | Rule | InlineMath(..) | DisplayMath(..) => (), SoftBreak | HardBreak => { if !containers.is_empty() - && let Some((next_event, next_range)) = events.peek() - && let Some(next_span) = fragments.span(cx, next_range.clone()) - && let Some(span) = fragments.span(cx, range.clone()) && !in_footnote_definition + // Tabs aren't handled correctly vvvv + && !doc[range.clone()].contains('\t') + && let Some((next_event, next_range)) = events.peek() && !matches!(next_event, End(_)) { lazy_continuation::check( cx, doc, range.end..next_range.start, - Span::new(span.hi(), next_span.lo(), span.ctxt(), span.parent()), + &fragments, &containers[..], ); } - if let Some(span) = fragments.span(cx, range.clone()) + + if event == HardBreak + && !doc[range.clone()].trim().starts_with('\\') + && let Some(span) = fragments.span(cx, range.clone()) && !span.from_expansion() - && let Some(snippet) = snippet_opt(cx, span) - && !snippet.trim().starts_with('\\') - && event == HardBreak { + { collected_breaks.push(span); } }, diff --git a/src/tools/clippy/clippy_lints/src/loops/manual_find.rs b/src/tools/clippy/clippy_lints/src/loops/manual_find.rs index 35737f3eafe..f99989ec6ba 100644 --- a/src/tools/clippy/clippy_lints/src/loops/manual_find.rs +++ b/src/tools/clippy/clippy_lints/src/loops/manual_find.rs @@ -83,6 +83,13 @@ pub(super) fn check<'tcx>( )[..], ); } + + // If the return type requires adjustments, we need to add a `.map` after the iterator + let inner_ret_adjust = cx.typeck_results().expr_adjustments(inner_ret); + if !inner_ret_adjust.is_empty() { + snippet.push_str(".map(|v| v as _)"); + } + // Extends to `last_stmt` to include semicolon in case of `return None;` let lint_span = span.to(last_stmt.span).to(last_ret.span); span_lint_and_then( diff --git a/src/tools/clippy/clippy_lints/src/loops/manual_flatten.rs b/src/tools/clippy/clippy_lints/src/loops/manual_flatten.rs index 9b6f97b9a2e..81f14b7b2b0 100644 --- a/src/tools/clippy/clippy_lints/src/loops/manual_flatten.rs +++ b/src/tools/clippy/clippy_lints/src/loops/manual_flatten.rs @@ -3,7 +3,7 @@ use super::utils::make_iterator_snippet; use clippy_utils::diagnostics::span_lint_and_then; use clippy_utils::msrvs::{self, Msrv}; use clippy_utils::visitors::is_local_used; -use clippy_utils::{higher, path_to_local_id, peel_blocks_with_stmt}; +use clippy_utils::{higher, is_refutable, path_to_local_id, peel_blocks_with_stmt}; use rustc_errors::Applicability; use rustc_hir::def::{DefKind, Res}; use rustc_hir::{Expr, Pat, PatKind}; @@ -28,7 +28,7 @@ pub(super) fn check<'tcx>( && let PatKind::Binding(_, pat_hir_id, _, _) = pat.kind && path_to_local_id(let_expr, pat_hir_id) // Ensure the `if let` statement is for the `Some` variant of `Option` or the `Ok` variant of `Result` - && let PatKind::TupleStruct(ref qpath, _, _) = let_pat.kind + && let PatKind::TupleStruct(ref qpath, [inner_pat], _) = let_pat.kind && let Res::Def(DefKind::Ctor(..), ctor_id) = cx.qpath_res(qpath, let_pat.hir_id) && let Some(variant_id) = cx.tcx.opt_parent(ctor_id) && let some_ctor = cx.tcx.lang_items().option_some_variant() == Some(variant_id) @@ -37,6 +37,7 @@ pub(super) fn check<'tcx>( // Ensure expr in `if let` is not used afterwards && !is_local_used(cx, if_then, pat_hir_id) && msrv.meets(cx, msrvs::ITER_FLATTEN) + && !is_refutable(cx, inner_pat) { let if_let_type = if some_ctor { "Some" } else { "Ok" }; // Prepare the error message diff --git a/src/tools/clippy/clippy_lints/src/loops/while_let_loop.rs b/src/tools/clippy/clippy_lints/src/loops/while_let_loop.rs index bd04827a1f0..845edb9cae1 100644 --- a/src/tools/clippy/clippy_lints/src/loops/while_let_loop.rs +++ b/src/tools/clippy/clippy_lints/src/loops/while_let_loop.rs @@ -1,68 +1,65 @@ use super::WHILE_LET_LOOP; use clippy_utils::diagnostics::span_lint_and_sugg; -use clippy_utils::higher; -use clippy_utils::source::snippet_with_applicability; +use clippy_utils::source::{snippet, snippet_indent, snippet_opt}; use clippy_utils::ty::needs_ordered_drop; use clippy_utils::visitors::any_temporaries_need_ordered_drop; +use clippy_utils::{higher, peel_blocks}; +use rustc_ast::BindingMode; use rustc_errors::Applicability; -use rustc_hir::{Block, Expr, ExprKind, LetStmt, MatchSource, Pat, StmtKind}; +use rustc_hir::{Block, Expr, ExprKind, LetStmt, MatchSource, Pat, PatKind, Path, QPath, StmtKind, Ty}; use rustc_lint::LateContext; pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>, loop_block: &'tcx Block<'_>) { - let (init, has_trailing_exprs) = match (loop_block.stmts, loop_block.expr) { - ([stmt, stmts @ ..], expr) => { - if let StmtKind::Let(&LetStmt { + let (init, let_info) = match (loop_block.stmts, loop_block.expr) { + ([stmt, ..], _) => match stmt.kind { + StmtKind::Let(LetStmt { init: Some(e), els: None, + pat, + ty, .. - }) - | StmtKind::Semi(e) - | StmtKind::Expr(e) = stmt.kind - { - (e, !stmts.is_empty() || expr.is_some()) - } else { - return; - } + }) => (*e, Some((*pat, *ty))), + StmtKind::Semi(e) | StmtKind::Expr(e) => (e, None), + _ => return, }, - ([], Some(e)) => (e, false), + ([], Some(e)) => (e, None), _ => return, }; + let has_trailing_exprs = loop_block.stmts.len() + usize::from(loop_block.expr.is_some()) > 1; if let Some(if_let) = higher::IfLet::hir(cx, init) && let Some(else_expr) = if_let.if_else && is_simple_break_expr(else_expr) { - could_be_while_let(cx, expr, if_let.let_pat, if_let.let_expr, has_trailing_exprs); + could_be_while_let( + cx, + expr, + if_let.let_pat, + if_let.let_expr, + has_trailing_exprs, + let_info, + if_let.if_then, + ); } else if let ExprKind::Match(scrutinee, [arm1, arm2], MatchSource::Normal) = init.kind && arm1.guard.is_none() && arm2.guard.is_none() && is_simple_break_expr(arm2.body) { - could_be_while_let(cx, expr, arm1.pat, scrutinee, has_trailing_exprs); + could_be_while_let(cx, expr, arm1.pat, scrutinee, has_trailing_exprs, let_info, arm1.body); } } -/// Returns `true` if expr contains a single break expression without a label or eub-expression. +/// Returns `true` if expr contains a single break expression without a label or sub-expression, +/// possibly embedded in blocks. fn is_simple_break_expr(e: &Expr<'_>) -> bool { - matches!(peel_blocks(e).kind, ExprKind::Break(dest, None) if dest.label.is_none()) -} - -/// Removes any blocks containing only a single expression. -fn peel_blocks<'tcx>(e: &'tcx Expr<'tcx>) -> &'tcx Expr<'tcx> { if let ExprKind::Block(b, _) = e.kind { match (b.stmts, b.expr) { - ([s], None) => { - if let StmtKind::Expr(e) | StmtKind::Semi(e) = s.kind { - peel_blocks(e) - } else { - e - } - }, - ([], Some(e)) => peel_blocks(e), - _ => e, + ([s], None) => matches!(s.kind, StmtKind::Expr(e) | StmtKind::Semi(e) if is_simple_break_expr(e)), + ([], Some(e)) => is_simple_break_expr(e), + _ => false, } } else { - e + matches!(e.kind, ExprKind::Break(dest, None) if dest.label.is_none()) } } @@ -72,6 +69,8 @@ fn could_be_while_let<'tcx>( let_pat: &'tcx Pat<'_>, let_expr: &'tcx Expr<'_>, has_trailing_exprs: bool, + let_info: Option<(&Pat<'_>, Option<&Ty<'_>>)>, + inner_expr: &Expr<'_>, ) { if has_trailing_exprs && (needs_ordered_drop(cx, cx.typeck_results().expr_ty(let_expr)) @@ -86,7 +85,24 @@ fn could_be_while_let<'tcx>( // 1) it was ugly with big bodies; // 2) it was not indented properly; // 3) it wasn’t very smart (see #675). - let mut applicability = Applicability::HasPlaceholders; + let inner_content = if let Some((pat, ty)) = let_info + // Prevent trivial reassignments such as `let x = x;` or `let _ = …;`, but + // keep them if the type has been explicitly specified. + && (!is_trivial_assignment(pat, peel_blocks(inner_expr)) || ty.is_some()) + && let Some(pat_str) = snippet_opt(cx, pat.span) + && let Some(init_str) = snippet_opt(cx, peel_blocks(inner_expr).span) + { + let ty_str = ty + .map(|ty| format!(": {}", snippet(cx, ty.span, "_"))) + .unwrap_or_default(); + format!( + "\n{indent} let {pat_str}{ty_str} = {init_str};\n{indent} ..\n{indent}", + indent = snippet_indent(cx, expr.span).unwrap_or_default(), + ) + } else { + " .. ".into() + }; + span_lint_and_sugg( cx, WHILE_LET_LOOP, @@ -94,10 +110,21 @@ fn could_be_while_let<'tcx>( "this loop could be written as a `while let` loop", "try", format!( - "while let {} = {} {{ .. }}", - snippet_with_applicability(cx, let_pat.span, "..", &mut applicability), - snippet_with_applicability(cx, let_expr.span, "..", &mut applicability), + "while let {} = {} {{{inner_content}}}", + snippet(cx, let_pat.span, ".."), + snippet(cx, let_expr.span, ".."), ), - applicability, + Applicability::HasPlaceholders, ); } + +fn is_trivial_assignment(pat: &Pat<'_>, init: &Expr<'_>) -> bool { + match (pat.kind, init.kind) { + (PatKind::Wild, _) => true, + ( + PatKind::Binding(BindingMode::NONE, _, pat_ident, None), + ExprKind::Path(QPath::Resolved(None, Path { segments: [init], .. })), + ) => pat_ident.name == init.ident.name, + _ => false, + } +} diff --git a/src/tools/clippy/clippy_lints/src/methods/manual_is_variant_and.rs b/src/tools/clippy/clippy_lints/src/methods/manual_is_variant_and.rs index 40aad03960c..4a61c223d2c 100644 --- a/src/tools/clippy/clippy_lints/src/methods/manual_is_variant_and.rs +++ b/src/tools/clippy/clippy_lints/src/methods/manual_is_variant_and.rs @@ -1,18 +1,22 @@ use clippy_utils::diagnostics::span_lint_and_sugg; +use clippy_utils::get_parent_expr; use clippy_utils::msrvs::{self, Msrv}; -use clippy_utils::source::snippet; +use clippy_utils::source::{snippet, snippet_opt}; use clippy_utils::ty::is_type_diagnostic_item; use rustc_errors::Applicability; +use rustc_hir::def::{CtorKind, CtorOf, DefKind, Res}; +use rustc_hir::{BinOpKind, Expr, ExprKind, QPath}; use rustc_lint::LateContext; -use rustc_span::{Span, sym}; +use rustc_middle::ty; +use rustc_span::{BytePos, Span, sym}; use super::MANUAL_IS_VARIANT_AND; -pub(super) fn check<'tcx>( +pub(super) fn check( cx: &LateContext<'_>, - expr: &'tcx rustc_hir::Expr<'_>, - map_recv: &'tcx rustc_hir::Expr<'_>, - map_arg: &'tcx rustc_hir::Expr<'_>, + expr: &Expr<'_>, + map_recv: &Expr<'_>, + map_arg: &Expr<'_>, map_span: Span, msrv: Msrv, ) { @@ -57,3 +61,57 @@ pub(super) fn check<'tcx>( Applicability::MachineApplicable, ); } + +fn emit_lint(cx: &LateContext<'_>, op: BinOpKind, parent: &Expr<'_>, method_span: Span, is_option: bool) { + if let Some(before_map_snippet) = snippet_opt(cx, parent.span.with_hi(method_span.lo())) + && let Some(after_map_snippet) = snippet_opt(cx, method_span.with_lo(method_span.lo() + BytePos(3))) + { + span_lint_and_sugg( + cx, + MANUAL_IS_VARIANT_AND, + parent.span, + format!( + "called `.map() {}= {}()`", + if op == BinOpKind::Eq { '=' } else { '!' }, + if is_option { "Some" } else { "Ok" }, + ), + "use", + if is_option && op == BinOpKind::Ne { + format!("{before_map_snippet}is_none_or{after_map_snippet}",) + } else { + format!( + "{}{before_map_snippet}{}{after_map_snippet}", + if op == BinOpKind::Eq { "" } else { "!" }, + if is_option { "is_some_and" } else { "is_ok_and" }, + ) + }, + Applicability::MachineApplicable, + ); + } +} + +pub(super) fn check_map(cx: &LateContext<'_>, expr: &Expr<'_>) { + if let Some(parent_expr) = get_parent_expr(cx, expr) + && let ExprKind::Binary(op, left, right) = parent_expr.kind + && matches!(op.node, BinOpKind::Eq | BinOpKind::Ne) + && op.span.eq_ctxt(expr.span) + { + // Check `left` and `right` expression in any order, and for `Option` and `Result` + for (expr1, expr2) in [(left, right), (right, left)] { + for item in [sym::Option, sym::Result] { + if let ExprKind::Call(call, ..) = expr1.kind + && let ExprKind::Path(QPath::Resolved(_, path)) = call.kind + && let Res::Def(DefKind::Ctor(CtorOf::Variant, CtorKind::Fn), _) = path.res + && let ty = cx.typeck_results().expr_ty(expr1) + && let ty::Adt(adt, args) = ty.kind() + && cx.tcx.is_diagnostic_item(item, adt.did()) + && args.type_at(0).is_bool() + && let ExprKind::MethodCall(_, recv, _, span) = expr2.kind + && is_type_diagnostic_item(cx, cx.typeck_results().expr_ty(recv), item) + { + return emit_lint(cx, op.node, parent_expr, span, item == sym::Option); + } + } + } + } +} diff --git a/src/tools/clippy/clippy_lints/src/methods/mod.rs b/src/tools/clippy/clippy_lints/src/methods/mod.rs index d2d59f0013c..bc159206985 100644 --- a/src/tools/clippy/clippy_lints/src/methods/mod.rs +++ b/src/tools/clippy/clippy_lints/src/methods/mod.rs @@ -4439,7 +4439,7 @@ declare_clippy_lint! { /// Checks for usage of `iter().any()` on slices when it can be replaced with `contains()` and suggests doing so. /// /// ### Why is this bad? - /// `contains()` is more concise and idiomatic, sometimes more fast. + /// `contains()` is more concise and idiomatic, while also being faster in some cases. /// /// ### Example /// ```no_run @@ -5203,6 +5203,7 @@ impl Methods { unused_enumerate_index::check(cx, expr, recv, m_arg); map_clone::check(cx, expr, recv, m_arg, self.msrv); map_with_unused_argument_over_ranges::check(cx, expr, recv, m_arg, self.msrv, span); + manual_is_variant_and::check_map(cx, expr); match method_call(recv) { Some((map_name @ (sym::iter | sym::into_iter), recv2, _, _, _)) => { iter_kv_map::check(cx, map_name, expr, recv2, m_arg, self.msrv); diff --git a/src/tools/clippy/clippy_lints/src/methods/or_fun_call.rs b/src/tools/clippy/clippy_lints/src/methods/or_fun_call.rs index 38cb4d51ca0..7bdd999bbba 100644 --- a/src/tools/clippy/clippy_lints/src/methods/or_fun_call.rs +++ b/src/tools/clippy/clippy_lints/src/methods/or_fun_call.rs @@ -11,8 +11,7 @@ use clippy_utils::{ use rustc_errors::Applicability; use rustc_lint::LateContext; use rustc_middle::ty; -use rustc_span::Span; -use rustc_span::Symbol; +use rustc_span::{Span, Symbol}; use {rustc_ast as ast, rustc_hir as hir}; use super::{OR_FUN_CALL, UNWRAP_OR_DEFAULT}; diff --git a/src/tools/clippy/clippy_lints/src/methods/unnecessary_to_owned.rs b/src/tools/clippy/clippy_lints/src/methods/unnecessary_to_owned.rs index 768bbebccd4..fdccf1fb33d 100644 --- a/src/tools/clippy/clippy_lints/src/methods/unnecessary_to_owned.rs +++ b/src/tools/clippy/clippy_lints/src/methods/unnecessary_to_owned.rs @@ -44,7 +44,7 @@ pub fn check<'tcx>( return; } // At this point, we know the call is of a `to_owned`-like function. The functions - // `check_addr_of_expr` and `check_call_arg` determine whether the call is unnecessary + // `check_addr_of_expr` and `check_into_iter_call_arg` determine whether the call is unnecessary // based on its context, that is, whether it is a referent in an `AddrOf` expression, an // argument in a `into_iter` call, or an argument in the call of some other function. if check_addr_of_expr(cx, expr, method_name, method_def_id, receiver) { diff --git a/src/tools/clippy/clippy_lints/src/mut_reference.rs b/src/tools/clippy/clippy_lints/src/mut_reference.rs index 2fd1049f42e..2f1ab3d2652 100644 --- a/src/tools/clippy/clippy_lints/src/mut_reference.rs +++ b/src/tools/clippy/clippy_lints/src/mut_reference.rs @@ -79,25 +79,19 @@ fn check_arguments<'tcx>( name: &str, fn_kind: &str, ) { - match type_definition.kind() { - ty::FnDef(..) | ty::FnPtr(..) => { - let parameters = type_definition.fn_sig(cx.tcx).skip_binder().inputs(); - for (argument, parameter) in iter::zip(arguments, parameters) { - match parameter.kind() { - ty::Ref(_, _, Mutability::Not) | ty::RawPtr(_, Mutability::Not) => { - if let ExprKind::AddrOf(BorrowKind::Ref, Mutability::Mut, _) = argument.kind { - span_lint( - cx, - UNNECESSARY_MUT_PASSED, - argument.span, - format!("the {fn_kind} `{name}` doesn't need a mutable reference"), - ); - } - }, - _ => (), - } + if let ty::FnDef(..) | ty::FnPtr(..) = type_definition.kind() { + let parameters = type_definition.fn_sig(cx.tcx).skip_binder().inputs(); + for (argument, parameter) in iter::zip(arguments, parameters) { + if let ty::Ref(_, _, Mutability::Not) | ty::RawPtr(_, Mutability::Not) = parameter.kind() + && let ExprKind::AddrOf(BorrowKind::Ref, Mutability::Mut, _) = argument.kind + { + span_lint( + cx, + UNNECESSARY_MUT_PASSED, + argument.span, + format!("the {fn_kind} `{name}` doesn't need a mutable reference"), + ); } - }, - _ => (), + } } } diff --git a/src/tools/clippy/clippy_lints/src/needless_for_each.rs b/src/tools/clippy/clippy_lints/src/needless_for_each.rs index 7dd96f1f037..6a7c8436bad 100644 --- a/src/tools/clippy/clippy_lints/src/needless_for_each.rs +++ b/src/tools/clippy/clippy_lints/src/needless_for_each.rs @@ -74,7 +74,7 @@ impl<'tcx> LateLintPass<'tcx> for NeedlessForEach { && let body = cx.tcx.hir_body(body) // Skip the lint if the body is not safe, so as not to suggest `for … in … unsafe {}` // and suggesting `for … in … { unsafe { } }` is a little ugly. - && let ExprKind::Block(Block { rules: BlockCheckMode::DefaultBlock, .. }, ..) = body.value.kind + && !matches!(body.value.kind, ExprKind::Block(Block { rules: BlockCheckMode::UnsafeBlock(_), .. }, ..)) { let mut ret_collector = RetCollector::default(); ret_collector.visit_expr(body.value); @@ -99,11 +99,21 @@ impl<'tcx> LateLintPass<'tcx> for NeedlessForEach { ) }; + let body_param_sugg = snippet_with_applicability(cx, body.params[0].pat.span, "..", &mut applicability); + let for_each_rev_sugg = snippet_with_applicability(cx, for_each_recv.span, "..", &mut applicability); + let body_value_sugg = snippet_with_applicability(cx, body.value.span, "..", &mut applicability); + let sugg = format!( "for {} in {} {}", - snippet_with_applicability(cx, body.params[0].pat.span, "..", &mut applicability), - snippet_with_applicability(cx, for_each_recv.span, "..", &mut applicability), - snippet_with_applicability(cx, body.value.span, "..", &mut applicability), + body_param_sugg, + for_each_rev_sugg, + match body.value.kind { + ExprKind::Block(block, _) if is_let_desugar(block) => { + format!("{{ {body_value_sugg} }}") + }, + ExprKind::Block(_, _) => body_value_sugg.to_string(), + _ => format!("{{ {body_value_sugg}; }}"), + } ); span_lint_and_then(cx, NEEDLESS_FOR_EACH, stmt.span, "needless use of `for_each`", |diag| { @@ -116,6 +126,20 @@ impl<'tcx> LateLintPass<'tcx> for NeedlessForEach { } } +/// Check if the block is a desugared `_ = expr` statement. +fn is_let_desugar(block: &Block<'_>) -> bool { + matches!( + block, + Block { + stmts: [Stmt { + kind: StmtKind::Let(_), + .. + },], + .. + } + ) +} + /// This type plays two roles. /// 1. Collect spans of `return` in the closure body. /// 2. Detect use of `return` in `Loop` in the closure body. diff --git a/src/tools/clippy/clippy_lints/src/operators/assign_op_pattern.rs b/src/tools/clippy/clippy_lints/src/operators/assign_op_pattern.rs index 4be42267b14..9c6141d8222 100644 --- a/src/tools/clippy/clippy_lints/src/operators/assign_op_pattern.rs +++ b/src/tools/clippy/clippy_lints/src/operators/assign_op_pattern.rs @@ -1,8 +1,10 @@ use clippy_utils::diagnostics::span_lint_and_then; +use clippy_utils::msrvs::Msrv; +use clippy_utils::qualify_min_const_fn::is_stable_const_fn; use clippy_utils::source::SpanRangeExt; use clippy_utils::ty::implements_trait; use clippy_utils::visitors::for_each_expr_without_closures; -use clippy_utils::{binop_traits, eq_expr_value, trait_ref_of_method}; +use clippy_utils::{binop_traits, eq_expr_value, is_in_const_context, trait_ref_of_method}; use core::ops::ControlFlow; use rustc_errors::Applicability; use rustc_hir as hir; @@ -19,6 +21,7 @@ pub(super) fn check<'tcx>( expr: &'tcx hir::Expr<'_>, assignee: &'tcx hir::Expr<'_>, e: &'tcx hir::Expr<'_>, + msrv: Msrv, ) { if let hir::ExprKind::Binary(op, l, r) = &e.kind { let lint = |assignee: &hir::Expr<'_>, rhs: &hir::Expr<'_>| { @@ -40,6 +43,15 @@ pub(super) fn check<'tcx>( return; } } + + // Skip if the trait is not stable in const contexts + if is_in_const_context(cx) + && let Some(binop_id) = cx.tcx.associated_item_def_ids(trait_id).first() + && !is_stable_const_fn(cx, *binop_id, msrv) + { + return; + } + span_lint_and_then( cx, ASSIGN_OP_PATTERN, diff --git a/src/tools/clippy/clippy_lints/src/operators/mod.rs b/src/tools/clippy/clippy_lints/src/operators/mod.rs index d32c062cf56..2f4e8e99588 100644 --- a/src/tools/clippy/clippy_lints/src/operators/mod.rs +++ b/src/tools/clippy/clippy_lints/src/operators/mod.rs @@ -919,7 +919,7 @@ impl<'tcx> LateLintPass<'tcx> for Operators { modulo_arithmetic::check(cx, e, bin_op, lhs, rhs, false); }, ExprKind::Assign(lhs, rhs, _) => { - assign_op_pattern::check(cx, e, lhs, rhs); + assign_op_pattern::check(cx, e, lhs, rhs, self.msrv); self_assignment::check(cx, e, lhs, rhs); }, ExprKind::Unary(op, arg) => { diff --git a/src/tools/clippy/clippy_lints/src/operators/modulo_arithmetic.rs b/src/tools/clippy/clippy_lints/src/operators/modulo_arithmetic.rs index 691d7b904ef..b79461663d7 100644 --- a/src/tools/clippy/clippy_lints/src/operators/modulo_arithmetic.rs +++ b/src/tools/clippy/clippy_lints/src/operators/modulo_arithmetic.rs @@ -34,14 +34,10 @@ pub(super) fn check<'tcx>( } fn used_in_comparison_with_zero(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool { - let Node::Expr(parent_expr) = cx.tcx.parent_hir_node(expr.hir_id) else { - return false; - }; - let ExprKind::Binary(op, lhs, rhs) = parent_expr.kind else { - return false; - }; - - if op.node == BinOpKind::Eq || op.node == BinOpKind::Ne { + if let Node::Expr(parent_expr) = cx.tcx.parent_hir_node(expr.hir_id) + && let ExprKind::Binary(op, lhs, rhs) = parent_expr.kind + && let BinOpKind::Eq | BinOpKind::Ne = op.node + { let ecx = ConstEvalCtxt::new(cx); matches!(ecx.eval(lhs), Some(Constant::Int(0))) || matches!(ecx.eval(rhs), Some(Constant::Int(0))) } else { @@ -56,35 +52,28 @@ struct OperandInfo { } fn analyze_operand(operand: &Expr<'_>, cx: &LateContext<'_>, expr: &Expr<'_>) -> Option<OperandInfo> { - match ConstEvalCtxt::new(cx).eval(operand) { - Some(Constant::Int(v)) => match *cx.typeck_results().expr_ty(expr).kind() { + match ConstEvalCtxt::new(cx).eval(operand)? { + Constant::Int(v) => match *cx.typeck_results().expr_ty(expr).kind() { ty::Int(ity) => { let value = sext(cx.tcx, v, ity); - return Some(OperandInfo { + Some(OperandInfo { string_representation: Some(value.to_string()), is_negative: value < 0, is_integral: true, - }); - }, - ty::Uint(_) => { - return Some(OperandInfo { - string_representation: None, - is_negative: false, - is_integral: true, - }); + }) }, - _ => {}, + ty::Uint(_) => Some(OperandInfo { + string_representation: None, + is_negative: false, + is_integral: true, + }), + _ => None, }, // FIXME(f16_f128): add when casting is available on all platforms - Some(Constant::F32(f)) => { - return Some(floating_point_operand_info(&f)); - }, - Some(Constant::F64(f)) => { - return Some(floating_point_operand_info(&f)); - }, - _ => {}, + Constant::F32(f) => Some(floating_point_operand_info(&f)), + Constant::F64(f) => Some(floating_point_operand_info(&f)), + _ => None, } - None } fn floating_point_operand_info<T: Display + PartialOrd + From<f32>>(f: &T) -> OperandInfo { diff --git a/src/tools/clippy/clippy_lints/src/panic_unimplemented.rs b/src/tools/clippy/clippy_lints/src/panic_unimplemented.rs index 8962f36db1e..449d3da7639 100644 --- a/src/tools/clippy/clippy_lints/src/panic_unimplemented.rs +++ b/src/tools/clippy/clippy_lints/src/panic_unimplemented.rs @@ -152,7 +152,6 @@ impl<'tcx> LateLintPass<'tcx> for PanicUnimplemented { expr.span, "`panic_any` should not be present in production code", ); - return; } } } diff --git a/src/tools/clippy/clippy_lints/src/pass_by_ref_or_value.rs b/src/tools/clippy/clippy_lints/src/pass_by_ref_or_value.rs index 5d30b66def2..dadf49b64e5 100644 --- a/src/tools/clippy/clippy_lints/src/pass_by_ref_or_value.rs +++ b/src/tools/clippy/clippy_lints/src/pass_by_ref_or_value.rs @@ -1,5 +1,3 @@ -use std::{cmp, iter}; - use clippy_config::Conf; use clippy_utils::diagnostics::span_lint_and_sugg; use clippy_utils::source::snippet; @@ -20,6 +18,7 @@ use rustc_middle::ty::{self, RegionKind, TyCtxt}; use rustc_session::impl_lint_pass; use rustc_span::def_id::LocalDefId; use rustc_span::{Span, sym}; +use std::iter; declare_clippy_lint! { /// ### What it does @@ -33,10 +32,8 @@ declare_clippy_lint! { /// registers. /// /// ### Known problems - /// This lint is target register size dependent, it is - /// limited to 32-bit to try and reduce portability problems between 32 and - /// 64-bit, but if you are compiling for 8 or 16-bit targets then the limit - /// will be different. + /// This lint is target dependent, some cases will lint on 64-bit targets but + /// not 32-bit or lower targets. /// /// The configuration option `trivial_copy_size_limit` can be set to override /// this limit for a project. @@ -112,16 +109,9 @@ pub struct PassByRefOrValue { impl PassByRefOrValue { pub fn new(tcx: TyCtxt<'_>, conf: &'static Conf) -> Self { - let ref_min_size = conf.trivial_copy_size_limit.unwrap_or_else(|| { - let bit_width = u64::from(tcx.sess.target.pointer_width); - // Cap the calculated bit width at 32-bits to reduce - // portability problems between 32 and 64-bit targets - let bit_width = cmp::min(bit_width, 32); - #[expect(clippy::integer_division)] - let byte_width = bit_width / 8; - // Use a limit of 2 times the register byte width - byte_width * 2 - }); + let ref_min_size = conf + .trivial_copy_size_limit + .unwrap_or_else(|| u64::from(tcx.sess.target.pointer_width / 8)); Self { ref_min_size, diff --git a/src/tools/clippy/clippy_lints/src/returns.rs b/src/tools/clippy/clippy_lints/src/returns.rs index 6bc5af268ff..e0c93153a77 100644 --- a/src/tools/clippy/clippy_lints/src/returns.rs +++ b/src/tools/clippy/clippy_lints/src/returns.rs @@ -435,7 +435,9 @@ fn check_final_expr<'tcx>( ExprKind::If(_, then, else_clause_opt) => { check_block_return(cx, &then.kind, peeled_drop_expr.span, semi_spans.clone()); if let Some(else_clause) = else_clause_opt { - check_block_return(cx, &else_clause.kind, peeled_drop_expr.span, semi_spans); + // The `RetReplacement` won't be used there as `else_clause` will be either a block or + // a `if` expression. + check_final_expr(cx, else_clause, semi_spans, RetReplacement::Empty, match_ty_opt); } }, // a match expr, check all arms diff --git a/src/tools/clippy/clippy_lints/src/unit_return_expecting_ord.rs b/src/tools/clippy/clippy_lints/src/unit_return_expecting_ord.rs index 67ceac92dbc..39f4130afcf 100644 --- a/src/tools/clippy/clippy_lints/src/unit_return_expecting_ord.rs +++ b/src/tools/clippy/clippy_lints/src/unit_return_expecting_ord.rs @@ -5,7 +5,7 @@ use rustc_lint::{LateContext, LateLintPass}; use rustc_middle::ty; use rustc_middle::ty::{ClauseKind, GenericPredicates, ProjectionPredicate, TraitPredicate}; use rustc_session::declare_lint_pass; -use rustc_span::{BytePos, Span, sym}; +use rustc_span::{BytePos, Span, Symbol, sym}; declare_clippy_lint! { /// ### What it does @@ -36,21 +36,26 @@ declare_clippy_lint! { declare_lint_pass!(UnitReturnExpectingOrd => [UNIT_RETURN_EXPECTING_ORD]); -fn get_trait_predicates_for_trait_id<'tcx>( +// For each +fn get_trait_predicates_for_trait_ids<'tcx>( cx: &LateContext<'tcx>, generics: GenericPredicates<'tcx>, - trait_id: Option<DefId>, -) -> Vec<TraitPredicate<'tcx>> { - let mut preds = Vec::new(); + trait_ids: &[Option<DefId>], // At least 2 ids +) -> [Vec<TraitPredicate<'tcx>>; 3] { + debug_assert!(trait_ids.len() >= 2); + let mut preds = [Vec::new(), Vec::new(), Vec::new()]; for (pred, _) in generics.predicates { - if let ClauseKind::Trait(poly_trait_pred) = pred.kind().skip_binder() - && let trait_pred = cx + if let ClauseKind::Trait(poly_trait_pred) = pred.kind().skip_binder() { + let trait_pred = cx .tcx - .instantiate_bound_regions_with_erased(pred.kind().rebind(poly_trait_pred)) - && let Some(trait_def_id) = trait_id - && trait_def_id == trait_pred.trait_ref.def_id - { - preds.push(trait_pred); + .instantiate_bound_regions_with_erased(pred.kind().rebind(poly_trait_pred)); + for (i, tid) in trait_ids.iter().enumerate() { + if let Some(tid) = tid + && *tid == trait_pred.trait_ref.def_id + { + preds[i].push(trait_pred); + } + } } } preds @@ -74,15 +79,24 @@ fn get_projection_pred<'tcx>( }) } -fn get_args_to_check<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) -> Vec<(usize, String)> { +fn get_args_to_check<'tcx>( + cx: &LateContext<'tcx>, + expr: &'tcx Expr<'tcx>, + args_len: usize, + fn_mut_trait: DefId, + ord_trait: Option<DefId>, + partial_ord_trait: Option<DefId>, +) -> Vec<(usize, Symbol)> { let mut args_to_check = Vec::new(); if let Some(def_id) = cx.typeck_results().type_dependent_def_id(expr.hir_id) { let fn_sig = cx.tcx.fn_sig(def_id).instantiate_identity(); let generics = cx.tcx.predicates_of(def_id); - let fn_mut_preds = get_trait_predicates_for_trait_id(cx, generics, cx.tcx.lang_items().fn_mut_trait()); - let ord_preds = get_trait_predicates_for_trait_id(cx, generics, cx.tcx.get_diagnostic_item(sym::Ord)); - let partial_ord_preds = - get_trait_predicates_for_trait_id(cx, generics, cx.tcx.lang_items().partial_ord_trait()); + let [fn_mut_preds, ord_preds, partial_ord_preds] = + get_trait_predicates_for_trait_ids(cx, generics, &[Some(fn_mut_trait), ord_trait, partial_ord_trait]); + if fn_mut_preds.is_empty() { + return vec![]; + } + // Trying to call instantiate_bound_regions_with_erased on fn_sig.inputs() gives the following error // The trait `rustc::ty::TypeFoldable<'_>` is not implemented for // `&[rustc_middle::ty::Ty<'_>]` @@ -102,12 +116,18 @@ fn get_args_to_check<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) -> Ve .iter() .any(|ord| Some(ord.self_ty()) == return_ty_pred.term.as_type()) { - args_to_check.push((i, "Ord".to_string())); + args_to_check.push((i, sym::Ord)); + if args_to_check.len() == args_len - 1 { + break; + } } else if partial_ord_preds .iter() .any(|pord| pord.self_ty() == return_ty_pred.term.expect_type()) { - args_to_check.push((i, "PartialOrd".to_string())); + args_to_check.push((i, sym::PartialOrd)); + if args_to_check.len() == args_len - 1 { + break; + } } } } @@ -142,38 +162,50 @@ fn check_arg<'tcx>(cx: &LateContext<'tcx>, arg: &'tcx Expr<'tcx>) -> Option<(Spa impl<'tcx> LateLintPass<'tcx> for UnitReturnExpectingOrd { fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) { - if let ExprKind::MethodCall(_, receiver, args, _) = expr.kind { - let arg_indices = get_args_to_check(cx, expr); + if let ExprKind::MethodCall(_, receiver, args, _) = expr.kind + && args.iter().any(|arg| { + matches!( + arg.peel_blocks().peel_borrows().peel_drop_temps().kind, + ExprKind::Path(_) | ExprKind::Closure(_) + ) + }) + && let Some(fn_mut_trait) = cx.tcx.lang_items().fn_mut_trait() + { + let ord_trait = cx.tcx.get_diagnostic_item(sym::Ord); + let partial_ord_trait = cx.tcx.lang_items().partial_ord_trait(); + if (ord_trait, partial_ord_trait) == (None, None) { + return; + } + let args = std::iter::once(receiver).chain(args.iter()).collect::<Vec<_>>(); + let arg_indices = get_args_to_check(cx, expr, args.len(), fn_mut_trait, ord_trait, partial_ord_trait); for (i, trait_name) in arg_indices { - if i < args.len() { - match check_arg(cx, args[i]) { - Some((span, None)) => { - span_lint( - cx, - UNIT_RETURN_EXPECTING_ORD, - span, - format!( - "this closure returns \ + match check_arg(cx, args[i]) { + Some((span, None)) => { + span_lint( + cx, + UNIT_RETURN_EXPECTING_ORD, + span, + format!( + "this closure returns \ the unit type which also implements {trait_name}" - ), - ); - }, - Some((span, Some(last_semi))) => { - span_lint_and_help( - cx, - UNIT_RETURN_EXPECTING_ORD, - span, - format!( - "this closure returns \ + ), + ); + }, + Some((span, Some(last_semi))) => { + span_lint_and_help( + cx, + UNIT_RETURN_EXPECTING_ORD, + span, + format!( + "this closure returns \ the unit type which also implements {trait_name}" - ), - Some(last_semi), - "probably caused by this trailing semicolon", - ); - }, - None => {}, - } + ), + Some(last_semi), + "probably caused by this trailing semicolon", + ); + }, + None => {}, } } } diff --git a/src/tools/clippy/clippy_utils/README.md b/src/tools/clippy/clippy_utils/README.md index c9083f654c4..efbacbd72db 100644 --- a/src/tools/clippy/clippy_utils/README.md +++ b/src/tools/clippy/clippy_utils/README.md @@ -8,7 +8,7 @@ This crate is only guaranteed to build with this `nightly` toolchain: <!-- begin autogenerated nightly --> ``` -nightly-2025-05-21 +nightly-2025-05-31 ``` <!-- end autogenerated nightly --> diff --git a/src/tools/clippy/clippy_utils/src/ast_utils/mod.rs b/src/tools/clippy/clippy_utils/src/ast_utils/mod.rs index a0503a699e6..6c186ab4a6f 100644 --- a/src/tools/clippy/clippy_utils/src/ast_utils/mod.rs +++ b/src/tools/clippy/clippy_utils/src/ast_utils/mod.rs @@ -437,10 +437,10 @@ pub fn eq_item_kind(l: &ItemKind, r: &ItemKind) -> bool { && both(lt.as_ref(), rt.as_ref(), |l, r| eq_ty(l, r)) }, (Enum(li, lg, le), Enum(ri, rg, re)) => { - eq_id(*li, *ri) && eq_generics(lg, rg) && over(&le.variants, &re.variants, eq_variant) + eq_id(*li, *ri) && eq_generics(lg, rg) && over(&le.variants, &re.variants, eq_variant) }, (Struct(li, lg, lv), Struct(ri, rg, rv)) | (Union(li, lg, lv), Union(ri, rg, rv)) => { - eq_id(*li, *ri) && eq_generics(lg, rg) && eq_variant_data(lv, rv) + eq_id(*li, *ri) && eq_generics(lg, rg) && eq_variant_data(lv, rv) }, ( Trait(box ast::Trait { diff --git a/src/tools/clippy/clippy_utils/src/lib.rs b/src/tools/clippy/clippy_utils/src/lib.rs index 2020f3d6b5b..f6ef638e618 100644 --- a/src/tools/clippy/clippy_utils/src/lib.rs +++ b/src/tools/clippy/clippy_utils/src/lib.rs @@ -1565,10 +1565,10 @@ pub fn is_ctor_or_promotable_const_function(cx: &LateContext<'_>, expr: &Expr<'_ /// Returns `true` if a pattern is refutable. // TODO: should be implemented using rustc/mir_build/thir machinery pub fn is_refutable(cx: &LateContext<'_>, pat: &Pat<'_>) -> bool { - fn is_enum_variant(cx: &LateContext<'_>, qpath: &QPath<'_>, id: HirId) -> bool { - matches!( + fn is_qpath_refutable(cx: &LateContext<'_>, qpath: &QPath<'_>, id: HirId) -> bool { + !matches!( cx.qpath_res(qpath, id), - Res::Def(DefKind::Variant, ..) | Res::Def(DefKind::Ctor(def::CtorOf::Variant, _), _) + Res::Def(DefKind::Struct, ..) | Res::Def(DefKind::Ctor(def::CtorOf::Struct, _), _) ) } @@ -1585,16 +1585,18 @@ pub fn is_refutable(cx: &LateContext<'_>, pat: &Pat<'_>) -> bool { kind: PatExprKind::Path(qpath), hir_id, .. - }) => is_enum_variant(cx, qpath, *hir_id), + }) => is_qpath_refutable(cx, qpath, *hir_id), PatKind::Or(pats) => { // TODO: should be the honest check, that pats is exhaustive set are_refutable(cx, pats) }, PatKind::Tuple(pats, _) => are_refutable(cx, pats), PatKind::Struct(ref qpath, fields, _) => { - is_enum_variant(cx, qpath, pat.hir_id) || are_refutable(cx, fields.iter().map(|field| field.pat)) + is_qpath_refutable(cx, qpath, pat.hir_id) || are_refutable(cx, fields.iter().map(|field| field.pat)) + }, + PatKind::TupleStruct(ref qpath, pats, _) => { + is_qpath_refutable(cx, qpath, pat.hir_id) || are_refutable(cx, pats) }, - PatKind::TupleStruct(ref qpath, pats, _) => is_enum_variant(cx, qpath, pat.hir_id) || are_refutable(cx, pats), PatKind::Slice(head, middle, tail) => { match &cx.typeck_results().node_type(pat.hir_id).kind() { rustc_ty::Slice(..) => { diff --git a/src/tools/clippy/clippy_utils/src/qualify_min_const_fn.rs b/src/tools/clippy/clippy_utils/src/qualify_min_const_fn.rs index bb04520c6b7..5b4ec12cbec 100644 --- a/src/tools/clippy/clippy_utils/src/qualify_min_const_fn.rs +++ b/src/tools/clippy/clippy_utils/src/qualify_min_const_fn.rs @@ -393,7 +393,8 @@ fn check_terminator<'tcx>( } } -fn is_stable_const_fn(cx: &LateContext<'_>, def_id: DefId, msrv: Msrv) -> bool { +/// Checks if the given `def_id` is a stable const fn, in respect to the given MSRV. +pub fn is_stable_const_fn(cx: &LateContext<'_>, def_id: DefId, msrv: Msrv) -> bool { cx.tcx.is_const_fn(def_id) && cx .tcx diff --git a/src/tools/clippy/clippy_utils/src/sugg.rs b/src/tools/clippy/clippy_utils/src/sugg.rs index f4dfc8f4b5a..6974e6512e2 100644 --- a/src/tools/clippy/clippy_utils/src/sugg.rs +++ b/src/tools/clippy/clippy_utils/src/sugg.rs @@ -940,7 +940,7 @@ impl<'tcx> Delegate<'tcx> for DerefDelegate<'_, 'tcx> { // note: unable to trigger `Subslice` kind in tests ProjectionKind::Subslice | // Doesn't have surface syntax. Only occurs in patterns. - ProjectionKind::OpaqueCast => (), + ProjectionKind::OpaqueCast | // Only occurs in closure captures. ProjectionKind::UnwrapUnsafeBinder => (), ProjectionKind::Deref => { diff --git a/src/tools/clippy/lintcheck/src/input.rs b/src/tools/clippy/lintcheck/src/input.rs index 83eb0a577d6..408a2e087af 100644 --- a/src/tools/clippy/lintcheck/src/input.rs +++ b/src/tools/clippy/lintcheck/src/input.rs @@ -8,7 +8,7 @@ use std::time::Duration; use serde::Deserialize; use walkdir::{DirEntry, WalkDir}; -use crate::{Crate, LINTCHECK_DOWNLOADS, LINTCHECK_SOURCES}; +use crate::{Crate, lintcheck_sources, target_dir}; const DEFAULT_DOCS_LINK: &str = "https://docs.rs/{krate}/{version}/src/{krate_}/{file}.html#{line}"; const DEFAULT_GITHUB_LINK: &str = "{url}/blob/{hash}/src/{file}#L{line}"; @@ -201,8 +201,10 @@ impl CrateWithSource { let file_link = &self.file_link; match &self.source { CrateSource::CratesIo { version } => { - let extract_dir = PathBuf::from(LINTCHECK_SOURCES); - let krate_download_dir = PathBuf::from(LINTCHECK_DOWNLOADS); + let extract_dir = PathBuf::from(lintcheck_sources()); + // Keep constant downloads path to avoid repeating work and + // filling up disk space unnecessarily. + let krate_download_dir = PathBuf::from("target/lintcheck/downloads/"); // url to download the crate from crates.io let url = format!("https://crates.io/api/v1/crates/{name}/{version}/download"); @@ -211,7 +213,7 @@ impl CrateWithSource { let krate_file_path = krate_download_dir.join(format!("{name}-{version}.crate.tar.gz")); // don't download/extract if we already have done so - if !krate_file_path.is_file() { + if !krate_file_path.is_file() || !extract_dir.join(format!("{name}-{version}")).exists() { // create a file path to download and write the crate data into let mut krate_dest = fs::File::create(&krate_file_path).unwrap(); let mut krate_req = get(&url).unwrap().into_reader(); @@ -236,7 +238,7 @@ impl CrateWithSource { }, CrateSource::Git { url, commit } => { let repo_path = { - let mut repo_path = PathBuf::from(LINTCHECK_SOURCES); + let mut repo_path = PathBuf::from(lintcheck_sources()); // add a -git suffix in case we have the same crate from crates.io and a git repo repo_path.push(format!("{name}-git")); repo_path @@ -286,7 +288,7 @@ impl CrateWithSource { // copy path into the dest_crate_root but skip directories that contain a CACHEDIR.TAG file. // The target/ directory contains a CACHEDIR.TAG file so it is the most commonly skipped directory // as a result of this filter. - let dest_crate_root = PathBuf::from(LINTCHECK_SOURCES).join(name); + let dest_crate_root = PathBuf::from(lintcheck_sources()).join(name); if dest_crate_root.exists() { println!("Deleting existing directory at `{}`", dest_crate_root.display()); fs::remove_dir_all(&dest_crate_root).unwrap(); @@ -326,15 +328,16 @@ impl CrateWithSource { /// /// This function panics if creating one of the dirs fails. fn create_dirs(krate_download_dir: &Path, extract_dir: &Path) { - fs::create_dir("target/lintcheck/").unwrap_or_else(|err| { + fs::create_dir(format!("{}/lintcheck/", target_dir())).unwrap_or_else(|err| { assert_eq!( err.kind(), ErrorKind::AlreadyExists, "cannot create lintcheck target dir" ); }); - fs::create_dir(krate_download_dir).unwrap_or_else(|err| { - assert_eq!(err.kind(), ErrorKind::AlreadyExists, "cannot create crate download dir"); + fs::create_dir_all(krate_download_dir).unwrap_or_else(|err| { + // We are allowed to reuse download dirs + assert_ne!(err.kind(), ErrorKind::AlreadyExists); }); fs::create_dir(extract_dir).unwrap_or_else(|err| { assert_eq!( diff --git a/src/tools/clippy/lintcheck/src/main.rs b/src/tools/clippy/lintcheck/src/main.rs index d4bf6cd48a1..84183831432 100644 --- a/src/tools/clippy/lintcheck/src/main.rs +++ b/src/tools/clippy/lintcheck/src/main.rs @@ -43,8 +43,14 @@ use input::read_crates; use output::{ClippyCheckOutput, ClippyWarning, RustcIce}; use rayon::prelude::*; -const LINTCHECK_DOWNLOADS: &str = "target/lintcheck/downloads"; -const LINTCHECK_SOURCES: &str = "target/lintcheck/sources"; +#[must_use] +pub fn target_dir() -> String { + env::var("CARGO_TARGET_DIR").unwrap_or("target".to_owned()) +} + +fn lintcheck_sources() -> String { + format!("{}/lintcheck/sources", target_dir()) +} /// Represents the actual source code of a crate that we ran "cargo clippy" on #[derive(Debug)] @@ -307,7 +313,8 @@ fn main() { fn lintcheck(config: LintcheckConfig) { let clippy_ver = build_clippy(config.perf); let clippy_driver_path = fs::canonicalize(format!( - "target/{}/clippy-driver{EXE_SUFFIX}", + "{}/{}/clippy-driver{EXE_SUFFIX}", + target_dir(), if config.perf { "release" } else { "debug" } )) .unwrap(); @@ -315,7 +322,8 @@ fn lintcheck(config: LintcheckConfig) { // assert that clippy is found assert!( clippy_driver_path.is_file(), - "target/{}/clippy-driver binary not found! {}", + "{}/{}/clippy-driver binary not found! {}", + target_dir(), if config.perf { "release" } else { "debug" }, clippy_driver_path.display() ); @@ -386,7 +394,7 @@ fn lintcheck(config: LintcheckConfig) { .unwrap(); let server = config.recursive.then(|| { - let _: io::Result<()> = fs::remove_dir_all("target/lintcheck/shared_target_dir/recursive"); + let _: io::Result<()> = fs::remove_dir_all(format!("{}/lintcheck/shared_target_dir/recursive", target_dir())); LintcheckServer::spawn(recursive_options) }); @@ -488,7 +496,7 @@ fn clippy_project_root() -> &'static Path { #[must_use] fn shared_target_dir(qualifier: &str) -> PathBuf { clippy_project_root() - .join("target/lintcheck/shared_target_dir") + .join(format!("{}/lintcheck/shared_target_dir", target_dir())) .join(qualifier) } diff --git a/src/tools/clippy/lintcheck/src/output.rs b/src/tools/clippy/lintcheck/src/output.rs index dcc1ec339ef..d7fe0915121 100644 --- a/src/tools/clippy/lintcheck/src/output.rs +++ b/src/tools/clippy/lintcheck/src/output.rs @@ -162,9 +162,9 @@ pub fn summarize_and_print_changes( fn gather_stats(warnings: &[ClippyWarning]) -> (String, HashMap<&String, usize>) { // count lint type occurrences let mut counter: HashMap<&String, usize> = HashMap::new(); - warnings - .iter() - .for_each(|wrn| *counter.entry(&wrn.name).or_insert(0) += 1); + for wrn in warnings { + *counter.entry(&wrn.name).or_insert(0) += 1; + } // collect into a tupled list for sorting let mut stats: Vec<(&&String, &usize)> = counter.iter().collect(); diff --git a/src/tools/clippy/rust-toolchain.toml b/src/tools/clippy/rust-toolchain.toml index 0b9ebe554f5..b6817d9a146 100644 --- a/src/tools/clippy/rust-toolchain.toml +++ b/src/tools/clippy/rust-toolchain.toml @@ -1,6 +1,6 @@ [toolchain] # begin autogenerated nightly -channel = "nightly-2025-05-21" +channel = "nightly-2025-05-31" # end autogenerated nightly components = ["cargo", "llvm-tools", "rust-src", "rust-std", "rustc", "rustc-dev", "rustfmt"] profile = "minimal" diff --git a/src/tools/clippy/rustfmt.toml b/src/tools/clippy/rustfmt.toml index 0dc6adce7bf..0ed58a2dfc1 100644 --- a/src/tools/clippy/rustfmt.toml +++ b/src/tools/clippy/rustfmt.toml @@ -6,4 +6,8 @@ edition = "2024" error_on_line_overflow = true imports_granularity = "Module" style_edition = "2024" -ignore = ["tests/ui/crashes/ice-10912.rs"] +ignore = [ + "tests/ui/crashes/ice-9405.rs", + "tests/ui/crashes/ice-10912.rs", + "tests/ui/non_expressive_names_error_recovery.rs", +] diff --git a/src/tools/clippy/tests/headers.rs b/src/tools/clippy/tests/headers.rs deleted file mode 100644 index d1f986ef526..00000000000 --- a/src/tools/clippy/tests/headers.rs +++ /dev/null @@ -1,34 +0,0 @@ -use regex::Regex; -use std::fs; -use walkdir::WalkDir; - -#[test] -fn old_test_headers() { - let old_headers = Regex::new( - r"^//( ?\[\w+\])? ?((check|build|run|ignore|aux|only|needs|rustc|unset|no|normalize|run|compile)-|edition|incremental|revisions).*", - ) - .unwrap(); - let mut failed = false; - - for entry in WalkDir::new("tests") { - let entry = entry.unwrap(); - let is_hidden_file = entry - .file_name() - .to_str() - .expect("non-UTF-8 file name") - .starts_with('.'); - if is_hidden_file || !entry.file_type().is_file() { - continue; - } - - let file = fs::read_to_string(entry.path()).unwrap_or_else(|err| panic!("{}: {err}", entry.path().display())); - - if let Some(header) = old_headers.find(&file) { - println!("Found header `{}` in {}", header.as_str(), entry.path().display()); - - failed = true; - } - } - - assert!(!failed, "use `//@foo` style test headers instead"); -} diff --git a/src/tools/clippy/tests/ui/assign_ops.fixed b/src/tools/clippy/tests/ui/assign_ops.fixed index 429c20f95e9..3bc6885d7c3 100644 --- a/src/tools/clippy/tests/ui/assign_ops.fixed +++ b/src/tools/clippy/tests/ui/assign_ops.fixed @@ -1,5 +1,6 @@ #![allow(clippy::useless_vec)] #![warn(clippy::assign_op_pattern)] +#![feature(const_trait_impl, const_ops)] use core::num::Wrapping; use std::ops::{Mul, MulAssign}; @@ -73,3 +74,33 @@ impl MulAssign<i64> for Wrap { *self = *self * rhs } } + +mod issue14871 { + + use std::ops::{Add, AddAssign}; + + pub trait Number: Copy + Add<Self, Output = Self> + AddAssign { + const ZERO: Self; + const ONE: Self; + } + + #[const_trait] + pub trait NumberConstants { + fn constant(value: usize) -> Self; + } + + impl<T> const NumberConstants for T + where + T: Number + ~const core::ops::Add, + { + fn constant(value: usize) -> Self { + let mut res = Self::ZERO; + let mut count = 0; + while count < value { + res = res + Self::ONE; + count += 1; + } + res + } + } +} diff --git a/src/tools/clippy/tests/ui/assign_ops.rs b/src/tools/clippy/tests/ui/assign_ops.rs index 480ff07f150..f1f8f9daff9 100644 --- a/src/tools/clippy/tests/ui/assign_ops.rs +++ b/src/tools/clippy/tests/ui/assign_ops.rs @@ -1,5 +1,6 @@ #![allow(clippy::useless_vec)] #![warn(clippy::assign_op_pattern)] +#![feature(const_trait_impl, const_ops)] use core::num::Wrapping; use std::ops::{Mul, MulAssign}; @@ -73,3 +74,33 @@ impl MulAssign<i64> for Wrap { *self = *self * rhs } } + +mod issue14871 { + + use std::ops::{Add, AddAssign}; + + pub trait Number: Copy + Add<Self, Output = Self> + AddAssign { + const ZERO: Self; + const ONE: Self; + } + + #[const_trait] + pub trait NumberConstants { + fn constant(value: usize) -> Self; + } + + impl<T> const NumberConstants for T + where + T: Number + ~const core::ops::Add, + { + fn constant(value: usize) -> Self { + let mut res = Self::ZERO; + let mut count = 0; + while count < value { + res = res + Self::ONE; + count += 1; + } + res + } + } +} diff --git a/src/tools/clippy/tests/ui/assign_ops.stderr b/src/tools/clippy/tests/ui/assign_ops.stderr index 881a333fbe4..c5e698b3ee1 100644 --- a/src/tools/clippy/tests/ui/assign_ops.stderr +++ b/src/tools/clippy/tests/ui/assign_ops.stderr @@ -1,5 +1,5 @@ error: manual implementation of an assign operation - --> tests/ui/assign_ops.rs:9:5 + --> tests/ui/assign_ops.rs:10:5 | LL | a = a + 1; | ^^^^^^^^^ help: replace it with: `a += 1` @@ -8,67 +8,67 @@ LL | a = a + 1; = help: to override `-D warnings` add `#[allow(clippy::assign_op_pattern)]` error: manual implementation of an assign operation - --> tests/ui/assign_ops.rs:11:5 + --> tests/ui/assign_ops.rs:12:5 | LL | a = 1 + a; | ^^^^^^^^^ help: replace it with: `a += 1` error: manual implementation of an assign operation - --> tests/ui/assign_ops.rs:13:5 + --> tests/ui/assign_ops.rs:14:5 | LL | a = a - 1; | ^^^^^^^^^ help: replace it with: `a -= 1` error: manual implementation of an assign operation - --> tests/ui/assign_ops.rs:15:5 + --> tests/ui/assign_ops.rs:16:5 | LL | a = a * 99; | ^^^^^^^^^^ help: replace it with: `a *= 99` error: manual implementation of an assign operation - --> tests/ui/assign_ops.rs:17:5 + --> tests/ui/assign_ops.rs:18:5 | LL | a = 42 * a; | ^^^^^^^^^^ help: replace it with: `a *= 42` error: manual implementation of an assign operation - --> tests/ui/assign_ops.rs:19:5 + --> tests/ui/assign_ops.rs:20:5 | LL | a = a / 2; | ^^^^^^^^^ help: replace it with: `a /= 2` error: manual implementation of an assign operation - --> tests/ui/assign_ops.rs:21:5 + --> tests/ui/assign_ops.rs:22:5 | LL | a = a % 5; | ^^^^^^^^^ help: replace it with: `a %= 5` error: manual implementation of an assign operation - --> tests/ui/assign_ops.rs:23:5 + --> tests/ui/assign_ops.rs:24:5 | LL | a = a & 1; | ^^^^^^^^^ help: replace it with: `a &= 1` error: manual implementation of an assign operation - --> tests/ui/assign_ops.rs:30:5 + --> tests/ui/assign_ops.rs:31:5 | LL | s = s + "bla"; | ^^^^^^^^^^^^^ help: replace it with: `s += "bla"` error: manual implementation of an assign operation - --> tests/ui/assign_ops.rs:35:5 + --> tests/ui/assign_ops.rs:36:5 | LL | a = a + Wrapping(1u32); | ^^^^^^^^^^^^^^^^^^^^^^ help: replace it with: `a += Wrapping(1u32)` error: manual implementation of an assign operation - --> tests/ui/assign_ops.rs:38:5 + --> tests/ui/assign_ops.rs:39:5 | LL | v[0] = v[0] + v[1]; | ^^^^^^^^^^^^^^^^^^ help: replace it with: `v[0] += v[1]` error: manual implementation of an assign operation - --> tests/ui/assign_ops.rs:51:5 + --> tests/ui/assign_ops.rs:52:5 | LL | buf = buf + cows.clone(); | ^^^^^^^^^^^^^^^^^^^^^^^^ help: replace it with: `buf += cows.clone()` diff --git a/src/tools/clippy/tests/ui/dbg_macro/dbg_macro.fixed b/src/tools/clippy/tests/ui/dbg_macro/dbg_macro.fixed index 3b9dee81898..5993c2faf0d 100644 --- a/src/tools/clippy/tests/ui/dbg_macro/dbg_macro.fixed +++ b/src/tools/clippy/tests/ui/dbg_macro/dbg_macro.fixed @@ -123,3 +123,19 @@ mod issue12131 { //~^ dbg_macro } } + +mod issue14914 { + use std::future::Future; + + fn takes_async_fn<F, Fut>(_f: F) + where + F: FnOnce(i32) -> Fut, + Fut: Future<Output = i32>, + { + } + + fn should_not_panic() { + takes_async_fn(async |val| val); + //~^ dbg_macro + } +} diff --git a/src/tools/clippy/tests/ui/dbg_macro/dbg_macro.rs b/src/tools/clippy/tests/ui/dbg_macro/dbg_macro.rs index 1dbbc6fe984..58d7e106e23 100644 --- a/src/tools/clippy/tests/ui/dbg_macro/dbg_macro.rs +++ b/src/tools/clippy/tests/ui/dbg_macro/dbg_macro.rs @@ -123,3 +123,19 @@ mod issue12131 { //~^ dbg_macro } } + +mod issue14914 { + use std::future::Future; + + fn takes_async_fn<F, Fut>(_f: F) + where + F: FnOnce(i32) -> Fut, + Fut: Future<Output = i32>, + { + } + + fn should_not_panic() { + takes_async_fn(async |val| dbg!(val)); + //~^ dbg_macro + } +} diff --git a/src/tools/clippy/tests/ui/dbg_macro/dbg_macro.stderr b/src/tools/clippy/tests/ui/dbg_macro/dbg_macro.stderr index f1412023cc8..5a65b38a85c 100644 --- a/src/tools/clippy/tests/ui/dbg_macro/dbg_macro.stderr +++ b/src/tools/clippy/tests/ui/dbg_macro/dbg_macro.stderr @@ -230,5 +230,17 @@ LL - print!("{}", dbg!(s)); LL + print!("{}", s); | -error: aborting due to 19 previous errors +error: the `dbg!` macro is intended as a debugging tool + --> tests/ui/dbg_macro/dbg_macro.rs:138:36 + | +LL | takes_async_fn(async |val| dbg!(val)); + | ^^^^^^^^^ + | +help: remove the invocation before committing it to a version control system + | +LL - takes_async_fn(async |val| dbg!(val)); +LL + takes_async_fn(async |val| val); + | + +error: aborting due to 20 previous errors diff --git a/src/tools/clippy/tests/ui/doc/doc_markdown-issue_13097.fixed b/src/tools/clippy/tests/ui/doc/doc_markdown-issue_13097.fixed index fb0f40b34a4..e0136584f3d 100644 --- a/src/tools/clippy/tests/ui/doc/doc_markdown-issue_13097.fixed +++ b/src/tools/clippy/tests/ui/doc/doc_markdown-issue_13097.fixed @@ -1,11 +1,37 @@ -// This test checks that words starting with capital letters and ending with "ified" don't -// trigger the lint. - #![deny(clippy::doc_markdown)] +#![allow(clippy::doc_lazy_continuation)] + +mod issue13097 { + // This test checks that words starting with capital letters and ending with "ified" don't + // trigger the lint. + pub enum OutputFormat { + /// `HumaNified` + //~^ ERROR: item in documentation is missing backticks + Plain, + // Should not warn! + /// JSONified console output + Json, + } +} +#[rustfmt::skip] pub enum OutputFormat { - /// `HumaNified` - //~^ ERROR: item in documentation is missing backticks + /** + * `HumaNified` + //~^ ERROR: item in documentation is missing backticks + * Before \u{08888} `HumaNified` \{u08888} After + //~^ ERROR: item in documentation is missing backticks + * meow meow \[`meow_meow`\] meow meow? + //~^ ERROR: item in documentation is missing backticks + * \u{08888} `meow_meow` \[meow meow] meow? + //~^ ERROR: item in documentation is missing backticks + * Above + * \u{08888} + * \[hi\](<https://example.com>) `HumaNified` \[example](<https://example.com>) + //~^ ERROR: item in documentation is missing backticks + * \u{08888} + * Below + */ Plain, // Should not warn! /// JSONified console output diff --git a/src/tools/clippy/tests/ui/doc/doc_markdown-issue_13097.rs b/src/tools/clippy/tests/ui/doc/doc_markdown-issue_13097.rs index 8c1e1a3cd6c..2e89fe6c56b 100644 --- a/src/tools/clippy/tests/ui/doc/doc_markdown-issue_13097.rs +++ b/src/tools/clippy/tests/ui/doc/doc_markdown-issue_13097.rs @@ -1,11 +1,37 @@ -// This test checks that words starting with capital letters and ending with "ified" don't -// trigger the lint. - #![deny(clippy::doc_markdown)] +#![allow(clippy::doc_lazy_continuation)] + +mod issue13097 { + // This test checks that words starting with capital letters and ending with "ified" don't + // trigger the lint. + pub enum OutputFormat { + /// HumaNified + //~^ ERROR: item in documentation is missing backticks + Plain, + // Should not warn! + /// JSONified console output + Json, + } +} +#[rustfmt::skip] pub enum OutputFormat { - /// HumaNified - //~^ ERROR: item in documentation is missing backticks + /** + * HumaNified + //~^ ERROR: item in documentation is missing backticks + * Before \u{08888} HumaNified \{u08888} After + //~^ ERROR: item in documentation is missing backticks + * meow meow \[meow_meow\] meow meow? + //~^ ERROR: item in documentation is missing backticks + * \u{08888} meow_meow \[meow meow] meow? + //~^ ERROR: item in documentation is missing backticks + * Above + * \u{08888} + * \[hi\](<https://example.com>) HumaNified \[example](<https://example.com>) + //~^ ERROR: item in documentation is missing backticks + * \u{08888} + * Below + */ Plain, // Should not warn! /// JSONified console output diff --git a/src/tools/clippy/tests/ui/doc/doc_markdown-issue_13097.stderr b/src/tools/clippy/tests/ui/doc/doc_markdown-issue_13097.stderr index 65b8f2ed80b..cea788301d4 100644 --- a/src/tools/clippy/tests/ui/doc/doc_markdown-issue_13097.stderr +++ b/src/tools/clippy/tests/ui/doc/doc_markdown-issue_13097.stderr @@ -1,19 +1,79 @@ error: item in documentation is missing backticks - --> tests/ui/doc/doc_markdown-issue_13097.rs:7:9 + --> tests/ui/doc/doc_markdown-issue_13097.rs:8:13 | -LL | /// HumaNified - | ^^^^^^^^^^ +LL | /// HumaNified + | ^^^^^^^^^^ | note: the lint level is defined here - --> tests/ui/doc/doc_markdown-issue_13097.rs:4:9 + --> tests/ui/doc/doc_markdown-issue_13097.rs:1:9 | LL | #![deny(clippy::doc_markdown)] | ^^^^^^^^^^^^^^^^^^^^ help: try | -LL - /// HumaNified -LL + /// `HumaNified` +LL - /// HumaNified +LL + /// `HumaNified` | -error: aborting due to 1 previous error +error: item in documentation is missing backticks + --> tests/ui/doc/doc_markdown-issue_13097.rs:20:8 + | +LL | * HumaNified + | ^^^^^^^^^^ + | +help: try + | +LL - * HumaNified +LL + * `HumaNified` + | + +error: item in documentation is missing backticks + --> tests/ui/doc/doc_markdown-issue_13097.rs:22:25 + | +LL | * Before \u{08888} HumaNified \{u08888} After + | ^^^^^^^^^^ + | +help: try + | +LL - * Before \u{08888} HumaNified \{u08888} After +LL + * Before \u{08888} `HumaNified` \{u08888} After + | + +error: item in documentation is missing backticks + --> tests/ui/doc/doc_markdown-issue_13097.rs:24:20 + | +LL | * meow meow \[meow_meow\] meow meow? + | ^^^^^^^^^ + | +help: try + | +LL - * meow meow \[meow_meow\] meow meow? +LL + * meow meow \[`meow_meow`\] meow meow? + | + +error: item in documentation is missing backticks + --> tests/ui/doc/doc_markdown-issue_13097.rs:26:18 + | +LL | * \u{08888} meow_meow \[meow meow] meow? + | ^^^^^^^^^ + | +help: try + | +LL - * \u{08888} meow_meow \[meow meow] meow? +LL + * \u{08888} `meow_meow` \[meow meow] meow? + | + +error: item in documentation is missing backticks + --> tests/ui/doc/doc_markdown-issue_13097.rs:30:38 + | +LL | * \[hi\](<https://example.com>) HumaNified \[example](<https://example.com>) + | ^^^^^^^^^^ + | +help: try + | +LL - * \[hi\](<https://example.com>) HumaNified \[example](<https://example.com>) +LL + * \[hi\](<https://example.com>) `HumaNified` \[example](<https://example.com>) + | + +error: aborting due to 6 previous errors diff --git a/src/tools/clippy/tests/ui/explicit_deref_methods.fixed b/src/tools/clippy/tests/ui/explicit_deref_methods.fixed index 619329a6ade..52c4d1b1f30 100644 --- a/src/tools/clippy/tests/ui/explicit_deref_methods.fixed +++ b/src/tools/clippy/tests/ui/explicit_deref_methods.fixed @@ -81,12 +81,10 @@ fn main() { let b: String = concat(just_return(a)); //~^ explicit_deref_methods - let b: &str = &**a; - //~^ explicit_deref_methods + let b: &str = a.deref().deref(); let opt_a = Some(a.clone()); - let b = &*opt_a.unwrap(); - //~^ explicit_deref_methods + let b = opt_a.unwrap().deref(); Aaa::deref(&Aaa); Aaa::deref_mut(&mut Aaa); diff --git a/src/tools/clippy/tests/ui/explicit_deref_methods.rs b/src/tools/clippy/tests/ui/explicit_deref_methods.rs index 9f2d513283c..706d6cb2b79 100644 --- a/src/tools/clippy/tests/ui/explicit_deref_methods.rs +++ b/src/tools/clippy/tests/ui/explicit_deref_methods.rs @@ -82,11 +82,9 @@ fn main() { //~^ explicit_deref_methods let b: &str = a.deref().deref(); - //~^ explicit_deref_methods let opt_a = Some(a.clone()); let b = opt_a.unwrap().deref(); - //~^ explicit_deref_methods Aaa::deref(&Aaa); Aaa::deref_mut(&mut Aaa); diff --git a/src/tools/clippy/tests/ui/explicit_deref_methods.stderr b/src/tools/clippy/tests/ui/explicit_deref_methods.stderr index a81e2f60317..5036884366c 100644 --- a/src/tools/clippy/tests/ui/explicit_deref_methods.stderr +++ b/src/tools/clippy/tests/ui/explicit_deref_methods.stderr @@ -56,40 +56,28 @@ LL | let b: String = concat(just_return(a).deref()); | ^^^^^^^^^^^^^^^^^^^^^^ help: try: `just_return(a)` error: explicit `deref` method call - --> tests/ui/explicit_deref_methods.rs:84:19 - | -LL | let b: &str = a.deref().deref(); - | ^^^^^^^^^^^^^^^^^ help: try: `&**a` - -error: explicit `deref` method call - --> tests/ui/explicit_deref_methods.rs:88:13 - | -LL | let b = opt_a.unwrap().deref(); - | ^^^^^^^^^^^^^^^^^^^^^^ help: try: `&*opt_a.unwrap()` - -error: explicit `deref` method call - --> tests/ui/explicit_deref_methods.rs:123:31 + --> tests/ui/explicit_deref_methods.rs:121:31 | LL | let b: &str = expr_deref!(a.deref()); | ^^^^^^^^^ help: try: `&*a` error: explicit `deref` method call - --> tests/ui/explicit_deref_methods.rs:141:14 + --> tests/ui/explicit_deref_methods.rs:139:14 | LL | let _ = &Deref::deref(&"foo"); | ^^^^^^^^^^^^^^^^^^^^ help: try: `*&"foo"` error: explicit `deref_mut` method call - --> tests/ui/explicit_deref_methods.rs:143:14 + --> tests/ui/explicit_deref_methods.rs:141:14 | LL | let _ = &DerefMut::deref_mut(&mut x); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `&mut **&mut x` error: explicit `deref_mut` method call - --> tests/ui/explicit_deref_methods.rs:144:14 + --> tests/ui/explicit_deref_methods.rs:142:14 | LL | let _ = &DerefMut::deref_mut((&mut &mut x).deref_mut()); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `&mut ***(&mut &mut x)` -error: aborting due to 15 previous errors +error: aborting due to 13 previous errors diff --git a/src/tools/clippy/tests/ui/manual_find_fixable.fixed b/src/tools/clippy/tests/ui/manual_find_fixable.fixed index 5e6849a4dfb..01b3ebacbeb 100644 --- a/src/tools/clippy/tests/ui/manual_find_fixable.fixed +++ b/src/tools/clippy/tests/ui/manual_find_fixable.fixed @@ -179,3 +179,13 @@ fn two_bindings(v: Vec<(u8, u8)>) -> Option<u8> { } fn main() {} + +mod issue14826 { + fn adjust_fixable(needle: &str) -> Option<&'static str> { + ["foo", "bar"].iter().find(|&candidate| candidate.eq_ignore_ascii_case(needle)).map(|v| v as _) + } + + fn adjust_unfixable(needle: &str) -> Option<*const str> { + ["foo", "bar"].iter().find(|&&candidate| candidate.eq_ignore_ascii_case(needle)).copied().map(|v| v as _) + } +} diff --git a/src/tools/clippy/tests/ui/manual_find_fixable.rs b/src/tools/clippy/tests/ui/manual_find_fixable.rs index 08a7dd2c6ee..ce62a4beba1 100644 --- a/src/tools/clippy/tests/ui/manual_find_fixable.rs +++ b/src/tools/clippy/tests/ui/manual_find_fixable.rs @@ -251,3 +251,25 @@ fn two_bindings(v: Vec<(u8, u8)>) -> Option<u8> { } fn main() {} + +mod issue14826 { + fn adjust_fixable(needle: &str) -> Option<&'static str> { + for candidate in &["foo", "bar"] { + //~^ manual_find + if candidate.eq_ignore_ascii_case(needle) { + return Some(candidate); + } + } + None + } + + fn adjust_unfixable(needle: &str) -> Option<*const str> { + for &candidate in &["foo", "bar"] { + //~^ manual_find + if candidate.eq_ignore_ascii_case(needle) { + return Some(candidate); + } + } + None + } +} diff --git a/src/tools/clippy/tests/ui/manual_find_fixable.stderr b/src/tools/clippy/tests/ui/manual_find_fixable.stderr index afa453c5a87..020635d90bb 100644 --- a/src/tools/clippy/tests/ui/manual_find_fixable.stderr +++ b/src/tools/clippy/tests/ui/manual_find_fixable.stderr @@ -139,5 +139,27 @@ LL | | return Some(x); LL | | None | |____________^ help: replace with an iterator: `arr.into_iter().find(|&x| x < 1)` -error: aborting due to 12 previous errors +error: manual implementation of `Iterator::find` + --> tests/ui/manual_find_fixable.rs:257:9 + | +LL | / for candidate in &["foo", "bar"] { +LL | | +LL | | if candidate.eq_ignore_ascii_case(needle) { +LL | | return Some(candidate); +... | +LL | | None + | |____________^ help: replace with an iterator: `["foo", "bar"].iter().find(|&candidate| candidate.eq_ignore_ascii_case(needle)).map(|v| v as _)` + +error: manual implementation of `Iterator::find` + --> tests/ui/manual_find_fixable.rs:267:9 + | +LL | / for &candidate in &["foo", "bar"] { +LL | | +LL | | if candidate.eq_ignore_ascii_case(needle) { +LL | | return Some(candidate); +... | +LL | | None + | |____________^ help: replace with an iterator: `["foo", "bar"].iter().find(|&&candidate| candidate.eq_ignore_ascii_case(needle)).copied().map(|v| v as _)` + +error: aborting due to 14 previous errors diff --git a/src/tools/clippy/tests/ui/manual_flatten.rs b/src/tools/clippy/tests/ui/manual_flatten.rs index 97f35c36e24..f1a0053ef38 100644 --- a/src/tools/clippy/tests/ui/manual_flatten.rs +++ b/src/tools/clippy/tests/ui/manual_flatten.rs @@ -123,6 +123,13 @@ fn main() { println!("{}", n); } + // Using nested `Some` pattern should not trigger the lint + for n in vec![Some((1, Some(2)))] { + if let Some((_, Some(n))) = n { + println!("{}", n); + } + } + run_unformatted_tests(); } diff --git a/src/tools/clippy/tests/ui/manual_flatten.stderr b/src/tools/clippy/tests/ui/manual_flatten.stderr index 5ab25658017..9a846fe17f3 100644 --- a/src/tools/clippy/tests/ui/manual_flatten.stderr +++ b/src/tools/clippy/tests/ui/manual_flatten.stderr @@ -178,7 +178,7 @@ LL | | } | |_________^ error: unnecessary `if let` since only the `Some` variant of the iterator element is used - --> tests/ui/manual_flatten.rs:132:5 + --> tests/ui/manual_flatten.rs:139:5 | LL | / for n in vec![ LL | | @@ -189,7 +189,7 @@ LL | | } | |_____^ | help: remove the `if let` statement in the for loop and then... - --> tests/ui/manual_flatten.rs:139:9 + --> tests/ui/manual_flatten.rs:146:9 | LL | / if let Some(n) = n { LL | | println!("{:?}", n); diff --git a/src/tools/clippy/tests/ui/manual_is_variant_and.fixed b/src/tools/clippy/tests/ui/manual_is_variant_and.fixed index c9c184561dd..18a72188ab5 100644 --- a/src/tools/clippy/tests/ui/manual_is_variant_and.fixed +++ b/src/tools/clippy/tests/ui/manual_is_variant_and.fixed @@ -4,6 +4,44 @@ #[macro_use] extern crate option_helpers; +struct Foo<T>(T); + +impl<T> Foo<T> { + fn map<F: FnMut(T) -> bool>(self, mut f: F) -> Option<bool> { + Some(f(self.0)) + } +} + +fn foo() -> Option<bool> { + Some(true) +} + +macro_rules! some_true { + () => { + Some(true) + }; +} +macro_rules! some_false { + () => { + Some(false) + }; +} + +macro_rules! mac { + (some $e:expr) => { + Some($e) + }; + (some_map $e:expr) => { + Some($e).map(|x| x % 2 == 0) + }; + (map $e:expr) => { + $e.map(|x| x % 2 == 0) + }; + (eq $a:expr, $b:expr) => { + $a == $b + }; +} + #[rustfmt::skip] fn option_methods() { let opt = Some(1); @@ -21,6 +59,15 @@ fn option_methods() { let _ = opt .is_some_and(|x| x > 1); + let _ = Some(2).is_some_and(|x| x % 2 == 0); + //~^ manual_is_variant_and + let _ = Some(2).is_none_or(|x| x % 2 == 0); + //~^ manual_is_variant_and + let _ = Some(2).is_some_and(|x| x % 2 == 0); + //~^ manual_is_variant_and + let _ = Some(2).is_none_or(|x| x % 2 == 0); + //~^ manual_is_variant_and + // won't fix because the return type of the closure is not `bool` let _ = opt.map(|x| x + 1).unwrap_or_default(); @@ -28,6 +75,14 @@ fn option_methods() { let _ = opt2.is_some_and(char::is_alphanumeric); // should lint //~^ manual_is_variant_and let _ = opt_map!(opt2, |x| x == 'a').unwrap_or_default(); // should not lint + + // Should not lint. + let _ = Foo::<u32>(0).map(|x| x % 2 == 0) == Some(true); + let _ = Some(2).map(|x| x % 2 == 0) != foo(); + let _ = mac!(eq Some(2).map(|x| x % 2 == 0), Some(true)); + let _ = mac!(some 2).map(|x| x % 2 == 0) == Some(true); + let _ = mac!(some_map 2) == Some(true); + let _ = mac!(map Some(2)) == Some(true); } #[rustfmt::skip] @@ -41,6 +96,13 @@ fn result_methods() { }); let _ = res.is_ok_and(|x| x > 1); + let _ = Ok::<usize, ()>(2).is_ok_and(|x| x % 2 == 0); + //~^ manual_is_variant_and + let _ = !Ok::<usize, ()>(2).is_ok_and(|x| x % 2 == 0); + //~^ manual_is_variant_and + let _ = !Ok::<usize, ()>(2).is_ok_and(|x| x % 2 == 0); + //~^ manual_is_variant_and + // won't fix because the return type of the closure is not `bool` let _ = res.map(|x| x + 1).unwrap_or_default(); diff --git a/src/tools/clippy/tests/ui/manual_is_variant_and.rs b/src/tools/clippy/tests/ui/manual_is_variant_and.rs index 52c7b56804c..a92f7c04369 100644 --- a/src/tools/clippy/tests/ui/manual_is_variant_and.rs +++ b/src/tools/clippy/tests/ui/manual_is_variant_and.rs @@ -4,6 +4,44 @@ #[macro_use] extern crate option_helpers; +struct Foo<T>(T); + +impl<T> Foo<T> { + fn map<F: FnMut(T) -> bool>(self, mut f: F) -> Option<bool> { + Some(f(self.0)) + } +} + +fn foo() -> Option<bool> { + Some(true) +} + +macro_rules! some_true { + () => { + Some(true) + }; +} +macro_rules! some_false { + () => { + Some(false) + }; +} + +macro_rules! mac { + (some $e:expr) => { + Some($e) + }; + (some_map $e:expr) => { + Some($e).map(|x| x % 2 == 0) + }; + (map $e:expr) => { + $e.map(|x| x % 2 == 0) + }; + (eq $a:expr, $b:expr) => { + $a == $b + }; +} + #[rustfmt::skip] fn option_methods() { let opt = Some(1); @@ -27,6 +65,15 @@ fn option_methods() { //~^ manual_is_variant_and .unwrap_or_default(); + let _ = Some(2).map(|x| x % 2 == 0) == Some(true); + //~^ manual_is_variant_and + let _ = Some(2).map(|x| x % 2 == 0) != Some(true); + //~^ manual_is_variant_and + let _ = Some(2).map(|x| x % 2 == 0) == some_true!(); + //~^ manual_is_variant_and + let _ = Some(2).map(|x| x % 2 == 0) != some_false!(); + //~^ manual_is_variant_and + // won't fix because the return type of the closure is not `bool` let _ = opt.map(|x| x + 1).unwrap_or_default(); @@ -34,6 +81,14 @@ fn option_methods() { let _ = opt2.map(char::is_alphanumeric).unwrap_or_default(); // should lint //~^ manual_is_variant_and let _ = opt_map!(opt2, |x| x == 'a').unwrap_or_default(); // should not lint + + // Should not lint. + let _ = Foo::<u32>(0).map(|x| x % 2 == 0) == Some(true); + let _ = Some(2).map(|x| x % 2 == 0) != foo(); + let _ = mac!(eq Some(2).map(|x| x % 2 == 0), Some(true)); + let _ = mac!(some 2).map(|x| x % 2 == 0) == Some(true); + let _ = mac!(some_map 2) == Some(true); + let _ = mac!(map Some(2)) == Some(true); } #[rustfmt::skip] @@ -50,6 +105,13 @@ fn result_methods() { //~^ manual_is_variant_and .unwrap_or_default(); + let _ = Ok::<usize, ()>(2).map(|x| x % 2 == 0) == Ok(true); + //~^ manual_is_variant_and + let _ = Ok::<usize, ()>(2).map(|x| x % 2 == 0) != Ok(true); + //~^ manual_is_variant_and + let _ = Ok::<usize, ()>(2).map(|x| x % 2 == 0) != Ok(true); + //~^ manual_is_variant_and + // won't fix because the return type of the closure is not `bool` let _ = res.map(|x| x + 1).unwrap_or_default(); diff --git a/src/tools/clippy/tests/ui/manual_is_variant_and.stderr b/src/tools/clippy/tests/ui/manual_is_variant_and.stderr index a4fa500580d..1fb437a8bc7 100644 --- a/src/tools/clippy/tests/ui/manual_is_variant_and.stderr +++ b/src/tools/clippy/tests/ui/manual_is_variant_and.stderr @@ -1,5 +1,5 @@ error: called `map(<f>).unwrap_or_default()` on an `Option` value - --> tests/ui/manual_is_variant_and.rs:13:17 + --> tests/ui/manual_is_variant_and.rs:51:17 | LL | let _ = opt.map(|x| x > 1) | _________________^ @@ -11,7 +11,7 @@ LL | | .unwrap_or_default(); = help: to override `-D warnings` add `#[allow(clippy::manual_is_variant_and)]` error: called `map(<f>).unwrap_or_default()` on an `Option` value - --> tests/ui/manual_is_variant_and.rs:18:17 + --> tests/ui/manual_is_variant_and.rs:56:17 | LL | let _ = opt.map(|x| { | _________________^ @@ -30,13 +30,13 @@ LL ~ }); | error: called `map(<f>).unwrap_or_default()` on an `Option` value - --> tests/ui/manual_is_variant_and.rs:23:17 + --> tests/ui/manual_is_variant_and.rs:61:17 | LL | let _ = opt.map(|x| x > 1).unwrap_or_default(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use: `is_some_and(|x| x > 1)` error: called `map(<f>).unwrap_or_default()` on an `Option` value - --> tests/ui/manual_is_variant_and.rs:26:10 + --> tests/ui/manual_is_variant_and.rs:64:10 | LL | .map(|x| x > 1) | __________^ @@ -44,14 +44,38 @@ LL | | LL | | .unwrap_or_default(); | |____________________________^ help: use: `is_some_and(|x| x > 1)` +error: called `.map() == Some()` + --> tests/ui/manual_is_variant_and.rs:68:13 + | +LL | let _ = Some(2).map(|x| x % 2 == 0) == Some(true); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use: `Some(2).is_some_and(|x| x % 2 == 0)` + +error: called `.map() != Some()` + --> tests/ui/manual_is_variant_and.rs:70:13 + | +LL | let _ = Some(2).map(|x| x % 2 == 0) != Some(true); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use: `Some(2).is_none_or(|x| x % 2 == 0)` + +error: called `.map() == Some()` + --> tests/ui/manual_is_variant_and.rs:72:13 + | +LL | let _ = Some(2).map(|x| x % 2 == 0) == some_true!(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use: `Some(2).is_some_and(|x| x % 2 == 0)` + +error: called `.map() != Some()` + --> tests/ui/manual_is_variant_and.rs:74:13 + | +LL | let _ = Some(2).map(|x| x % 2 == 0) != some_false!(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use: `Some(2).is_none_or(|x| x % 2 == 0)` + error: called `map(<f>).unwrap_or_default()` on an `Option` value - --> tests/ui/manual_is_variant_and.rs:34:18 + --> tests/ui/manual_is_variant_and.rs:81:18 | LL | let _ = opt2.map(char::is_alphanumeric).unwrap_or_default(); // should lint | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use: `is_some_and(char::is_alphanumeric)` error: called `map(<f>).unwrap_or_default()` on a `Result` value - --> tests/ui/manual_is_variant_and.rs:44:17 + --> tests/ui/manual_is_variant_and.rs:99:17 | LL | let _ = res.map(|x| { | _________________^ @@ -70,7 +94,7 @@ LL ~ }); | error: called `map(<f>).unwrap_or_default()` on a `Result` value - --> tests/ui/manual_is_variant_and.rs:49:17 + --> tests/ui/manual_is_variant_and.rs:104:17 | LL | let _ = res.map(|x| x > 1) | _________________^ @@ -78,11 +102,29 @@ LL | | LL | | .unwrap_or_default(); | |____________________________^ help: use: `is_ok_and(|x| x > 1)` +error: called `.map() == Ok()` + --> tests/ui/manual_is_variant_and.rs:108:13 + | +LL | let _ = Ok::<usize, ()>(2).map(|x| x % 2 == 0) == Ok(true); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use: `Ok::<usize, ()>(2).is_ok_and(|x| x % 2 == 0)` + +error: called `.map() != Ok()` + --> tests/ui/manual_is_variant_and.rs:110:13 + | +LL | let _ = Ok::<usize, ()>(2).map(|x| x % 2 == 0) != Ok(true); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use: `!Ok::<usize, ()>(2).is_ok_and(|x| x % 2 == 0)` + +error: called `.map() != Ok()` + --> tests/ui/manual_is_variant_and.rs:112:13 + | +LL | let _ = Ok::<usize, ()>(2).map(|x| x % 2 == 0) != Ok(true); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use: `!Ok::<usize, ()>(2).is_ok_and(|x| x % 2 == 0)` + error: called `map(<f>).unwrap_or_default()` on a `Result` value - --> tests/ui/manual_is_variant_and.rs:57:18 + --> tests/ui/manual_is_variant_and.rs:119:18 | LL | let _ = res2.map(char::is_alphanumeric).unwrap_or_default(); // should lint | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use: `is_ok_and(char::is_alphanumeric)` -error: aborting due to 8 previous errors +error: aborting due to 15 previous errors diff --git a/src/tools/clippy/tests/ui/needless_borrow.fixed b/src/tools/clippy/tests/ui/needless_borrow.fixed index d7d344452c5..54cad2e393f 100644 --- a/src/tools/clippy/tests/ui/needless_borrow.fixed +++ b/src/tools/clippy/tests/ui/needless_borrow.fixed @@ -107,9 +107,6 @@ fn main() { let x = (1, 2); let _ = x.0; //~^ needless_borrow - let x = &x as *const (i32, i32); - let _ = unsafe { (*x).0 }; - //~^ needless_borrow // Issue #8367 trait Foo { @@ -289,3 +286,15 @@ fn issue_12268() { // compiler } + +fn issue_14743<T>(slice: &[T]) { + let _ = slice.len(); + //~^ needless_borrow + + let slice = slice as *const [T]; + let _ = unsafe { (&*slice).len() }; + + // Check that rustc would actually warn if Clippy had suggested removing the reference + #[expect(dangerous_implicit_autorefs)] + let _ = unsafe { (*slice).len() }; +} diff --git a/src/tools/clippy/tests/ui/needless_borrow.rs b/src/tools/clippy/tests/ui/needless_borrow.rs index 1f05b90b472..b698c6bfc96 100644 --- a/src/tools/clippy/tests/ui/needless_borrow.rs +++ b/src/tools/clippy/tests/ui/needless_borrow.rs @@ -107,9 +107,6 @@ fn main() { let x = (1, 2); let _ = (&x).0; //~^ needless_borrow - let x = &x as *const (i32, i32); - let _ = unsafe { (&*x).0 }; - //~^ needless_borrow // Issue #8367 trait Foo { @@ -289,3 +286,15 @@ fn issue_12268() { // compiler } + +fn issue_14743<T>(slice: &[T]) { + let _ = (&slice).len(); + //~^ needless_borrow + + let slice = slice as *const [T]; + let _ = unsafe { (&*slice).len() }; + + // Check that rustc would actually warn if Clippy had suggested removing the reference + #[expect(dangerous_implicit_autorefs)] + let _ = unsafe { (*slice).len() }; +} diff --git a/src/tools/clippy/tests/ui/needless_borrow.stderr b/src/tools/clippy/tests/ui/needless_borrow.stderr index b036b1e47d1..172d36bd73a 100644 --- a/src/tools/clippy/tests/ui/needless_borrow.stderr +++ b/src/tools/clippy/tests/ui/needless_borrow.stderr @@ -103,71 +103,71 @@ error: this expression borrows a value the compiler would automatically borrow LL | let _ = (&x).0; | ^^^^ help: change this to: `x` -error: this expression borrows a value the compiler would automatically borrow - --> tests/ui/needless_borrow.rs:111:22 - | -LL | let _ = unsafe { (&*x).0 }; - | ^^^^^ help: change this to: `(*x)` - error: this expression creates a reference which is immediately dereferenced by the compiler - --> tests/ui/needless_borrow.rs:122:5 + --> tests/ui/needless_borrow.rs:119:5 | LL | (&&()).foo(); | ^^^^^^ help: change this to: `(&())` error: this expression creates a reference which is immediately dereferenced by the compiler - --> tests/ui/needless_borrow.rs:132:5 + --> tests/ui/needless_borrow.rs:129:5 | LL | (&&5).foo(); | ^^^^^ help: change this to: `(&5)` error: this expression creates a reference which is immediately dereferenced by the compiler - --> tests/ui/needless_borrow.rs:159:23 + --> tests/ui/needless_borrow.rs:156:23 | LL | let x: (&str,) = (&"",); | ^^^ help: change this to: `""` error: this expression borrows a value the compiler would automatically borrow - --> tests/ui/needless_borrow.rs:202:13 + --> tests/ui/needless_borrow.rs:199:13 | LL | (&self.f)() | ^^^^^^^^^ help: change this to: `(self.f)` error: this expression borrows a value the compiler would automatically borrow - --> tests/ui/needless_borrow.rs:212:13 + --> tests/ui/needless_borrow.rs:209:13 | LL | (&mut self.f)() | ^^^^^^^^^^^^^ help: change this to: `(self.f)` error: this expression borrows a value the compiler would automatically borrow - --> tests/ui/needless_borrow.rs:250:22 + --> tests/ui/needless_borrow.rs:247:22 | LL | let _ = &mut (&mut { x.u }).x; | ^^^^^^^^^^^^^^ help: change this to: `{ x.u }` error: this expression borrows a value the compiler would automatically borrow - --> tests/ui/needless_borrow.rs:258:22 + --> tests/ui/needless_borrow.rs:255:22 | LL | let _ = &mut (&mut { x.u }).x; | ^^^^^^^^^^^^^^ help: change this to: `{ x.u }` error: this expression borrows a value the compiler would automatically borrow - --> tests/ui/needless_borrow.rs:263:22 + --> tests/ui/needless_borrow.rs:260:22 | LL | let _ = &mut (&mut x.u).x; | ^^^^^^^^^^ help: change this to: `x.u` error: this expression borrows a value the compiler would automatically borrow - --> tests/ui/needless_borrow.rs:265:22 + --> tests/ui/needless_borrow.rs:262:22 | LL | let _ = &mut (&mut { x.u }).x; | ^^^^^^^^^^^^^^ help: change this to: `{ x.u }` error: this expression creates a reference which is immediately dereferenced by the compiler - --> tests/ui/needless_borrow.rs:287:23 + --> tests/ui/needless_borrow.rs:284:23 | LL | option.unwrap_or((&x.0,)); | ^^^^ help: change this to: `x.0` +error: this expression creates a reference which is immediately dereferenced by the compiler + --> tests/ui/needless_borrow.rs:291:13 + | +LL | let _ = (&slice).len(); + | ^^^^^^^^ help: change this to: `slice` + error: aborting due to 28 previous errors diff --git a/src/tools/clippy/tests/ui/needless_for_each_fixable.fixed b/src/tools/clippy/tests/ui/needless_for_each_fixable.fixed index fa23e18318f..a73aff55639 100644 --- a/src/tools/clippy/tests/ui/needless_for_each_fixable.fixed +++ b/src/tools/clippy/tests/ui/needless_for_each_fixable.fixed @@ -128,3 +128,18 @@ fn should_not_lint() { } fn main() {} + +mod issue14734 { + fn let_desugar(rows: &[u8]) { + let mut v = vec![]; + for x in rows.iter() { _ = v.push(x) } + //~^ needless_for_each + } + + fn do_something(_: &u8, _: u8) {} + + fn single_expr(rows: &[u8]) { + for x in rows.iter() { do_something(x, 1u8); } + //~^ needless_for_each + } +} diff --git a/src/tools/clippy/tests/ui/needless_for_each_fixable.rs b/src/tools/clippy/tests/ui/needless_for_each_fixable.rs index 2c7e68a6f51..d92f055d3f4 100644 --- a/src/tools/clippy/tests/ui/needless_for_each_fixable.rs +++ b/src/tools/clippy/tests/ui/needless_for_each_fixable.rs @@ -128,3 +128,18 @@ fn should_not_lint() { } fn main() {} + +mod issue14734 { + fn let_desugar(rows: &[u8]) { + let mut v = vec![]; + rows.iter().for_each(|x| _ = v.push(x)); + //~^ needless_for_each + } + + fn do_something(_: &u8, _: u8) {} + + fn single_expr(rows: &[u8]) { + rows.iter().for_each(|x| do_something(x, 1u8)); + //~^ needless_for_each + } +} diff --git a/src/tools/clippy/tests/ui/needless_for_each_fixable.stderr b/src/tools/clippy/tests/ui/needless_for_each_fixable.stderr index 013a3fa3e36..f8014456097 100644 --- a/src/tools/clippy/tests/ui/needless_for_each_fixable.stderr +++ b/src/tools/clippy/tests/ui/needless_for_each_fixable.stderr @@ -136,5 +136,17 @@ LL + acc += elem; LL + } | -error: aborting due to 8 previous errors +error: needless use of `for_each` + --> tests/ui/needless_for_each_fixable.rs:135:9 + | +LL | rows.iter().for_each(|x| _ = v.push(x)); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `for x in rows.iter() { _ = v.push(x) }` + +error: needless use of `for_each` + --> tests/ui/needless_for_each_fixable.rs:142:9 + | +LL | rows.iter().for_each(|x| do_something(x, 1u8)); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `for x in rows.iter() { do_something(x, 1u8); }` + +error: aborting due to 10 previous errors diff --git a/src/tools/clippy/tests/ui/needless_return.fixed b/src/tools/clippy/tests/ui/needless_return.fixed index 17d3862cd86..d571b97f519 100644 --- a/src/tools/clippy/tests/ui/needless_return.fixed +++ b/src/tools/clippy/tests/ui/needless_return.fixed @@ -452,3 +452,68 @@ pub unsafe fn issue_12157() -> *const i32 { (unsafe { todo() } as *const i32) //~^ needless_return } + +mod else_ifs { + fn test1(a: i32) -> u32 { + if a == 0 { + 1 + //~^ needless_return + } else if a < 10 { + 2 + //~^ needless_return + } else { + 3 + //~^ needless_return + } + } + + fn test2(a: i32) -> u32 { + if a == 0 { + 1 + //~^ needless_return + } else if a < 10 { + 2 + } else { + 3 + //~^ needless_return + } + } + + fn test3(a: i32) -> u32 { + if a == 0 { + 1 + //~^ needless_return + } else if a < 10 { + 2 + } else { + 3 + //~^ needless_return + } + } + + #[allow(clippy::match_single_binding, clippy::redundant_pattern)] + fn test4(a: i32) -> u32 { + if a == 0 { + 1 + //~^ needless_return + } else if if if a > 0x1_1 { + return 2; + } else { + return 5; + } { + true + } else { + true + } { + 0xDEADC0DE + } else if match a { + b @ _ => { + return 1; + }, + } { + 0xDEADBEEF + } else { + 1 + } + } +} diff --git a/src/tools/clippy/tests/ui/needless_return.rs b/src/tools/clippy/tests/ui/needless_return.rs index 1c6e7ffa1ee..2e4348ea338 100644 --- a/src/tools/clippy/tests/ui/needless_return.rs +++ b/src/tools/clippy/tests/ui/needless_return.rs @@ -461,3 +461,68 @@ pub unsafe fn issue_12157() -> *const i32 { return unsafe { todo() } as *const i32; //~^ needless_return } + +mod else_ifs { + fn test1(a: i32) -> u32 { + if a == 0 { + return 1; + //~^ needless_return + } else if a < 10 { + return 2; + //~^ needless_return + } else { + return 3; + //~^ needless_return + } + } + + fn test2(a: i32) -> u32 { + if a == 0 { + return 1; + //~^ needless_return + } else if a < 10 { + 2 + } else { + return 3; + //~^ needless_return + } + } + + fn test3(a: i32) -> u32 { + if a == 0 { + return 1; + //~^ needless_return + } else if a < 10 { + 2 + } else { + return 3; + //~^ needless_return + } + } + + #[allow(clippy::match_single_binding, clippy::redundant_pattern)] + fn test4(a: i32) -> u32 { + if a == 0 { + return 1; + //~^ needless_return + } else if if if a > 0x1_1 { + return 2; + } else { + return 5; + } { + true + } else { + true + } { + 0xDEADC0DE + } else if match a { + b @ _ => { + return 1; + }, + } { + 0xDEADBEEF + } else { + 1 + } + } +} diff --git a/src/tools/clippy/tests/ui/needless_return.stderr b/src/tools/clippy/tests/ui/needless_return.stderr index 26dd265379b..206bd8ee5af 100644 --- a/src/tools/clippy/tests/ui/needless_return.stderr +++ b/src/tools/clippy/tests/ui/needless_return.stderr @@ -685,5 +685,101 @@ LL - return unsafe { todo() } as *const i32; LL + (unsafe { todo() } as *const i32) | -error: aborting due to 55 previous errors +error: unneeded `return` statement + --> tests/ui/needless_return.rs:468:13 + | +LL | return 1; + | ^^^^^^^^ + | +help: remove `return` + | +LL - return 1; +LL + 1 + | + +error: unneeded `return` statement + --> tests/ui/needless_return.rs:471:13 + | +LL | return 2; + | ^^^^^^^^ + | +help: remove `return` + | +LL - return 2; +LL + 2 + | + +error: unneeded `return` statement + --> tests/ui/needless_return.rs:474:13 + | +LL | return 3; + | ^^^^^^^^ + | +help: remove `return` + | +LL - return 3; +LL + 3 + | + +error: unneeded `return` statement + --> tests/ui/needless_return.rs:481:13 + | +LL | return 1; + | ^^^^^^^^ + | +help: remove `return` + | +LL - return 1; +LL + 1 + | + +error: unneeded `return` statement + --> tests/ui/needless_return.rs:486:13 + | +LL | return 3; + | ^^^^^^^^ + | +help: remove `return` + | +LL - return 3; +LL + 3 + | + +error: unneeded `return` statement + --> tests/ui/needless_return.rs:493:13 + | +LL | return 1; + | ^^^^^^^^ + | +help: remove `return` + | +LL - return 1; +LL + 1 + | + +error: unneeded `return` statement + --> tests/ui/needless_return.rs:498:13 + | +LL | return 3; + | ^^^^^^^^ + | +help: remove `return` + | +LL - return 3; +LL + 3 + | + +error: unneeded `return` statement + --> tests/ui/needless_return.rs:506:13 + | +LL | return 1; + | ^^^^^^^^ + | +help: remove `return` + | +LL - return 1; +LL + 1 + | + +error: aborting due to 63 previous errors diff --git a/src/tools/clippy/tests/ui/skip_rustfmt/non_expressive_names_error_recovery.fixed b/src/tools/clippy/tests/ui/non_expressive_names_error_recovery.fixed index c96a53ba2cd..c96a53ba2cd 100644 --- a/src/tools/clippy/tests/ui/skip_rustfmt/non_expressive_names_error_recovery.fixed +++ b/src/tools/clippy/tests/ui/non_expressive_names_error_recovery.fixed diff --git a/src/tools/clippy/tests/ui/skip_rustfmt/non_expressive_names_error_recovery.rs b/src/tools/clippy/tests/ui/non_expressive_names_error_recovery.rs index a3a35eb26d1..a3a35eb26d1 100644 --- a/src/tools/clippy/tests/ui/skip_rustfmt/non_expressive_names_error_recovery.rs +++ b/src/tools/clippy/tests/ui/non_expressive_names_error_recovery.rs diff --git a/src/tools/clippy/tests/ui/skip_rustfmt/non_expressive_names_error_recovery.stderr b/src/tools/clippy/tests/ui/non_expressive_names_error_recovery.stderr index 4998b9bd2cc..28d9a42a9a1 100644 --- a/src/tools/clippy/tests/ui/skip_rustfmt/non_expressive_names_error_recovery.stderr +++ b/src/tools/clippy/tests/ui/non_expressive_names_error_recovery.stderr @@ -1,5 +1,5 @@ error: expected one of `!`, `(`, `+`, `,`, `::`, `<`, or `>`, found `)` - --> tests/ui/skip_rustfmt/non_expressive_names_error_recovery.rs:6:19 + --> tests/ui/non_expressive_names_error_recovery.rs:6:19 | LL | fn aa(a: Aa<String) { | ^ expected one of 7 possible tokens diff --git a/src/tools/clippy/tests/ui/question_mark.fixed b/src/tools/clippy/tests/ui/question_mark.fixed index 507bc2b29d8..60dc1c101b6 100644 --- a/src/tools/clippy/tests/ui/question_mark.fixed +++ b/src/tools/clippy/tests/ui/question_mark.fixed @@ -443,3 +443,13 @@ fn issue_14615(a: MutexGuard<Option<u32>>) -> Option<String> { //~^^^ question_mark Some(format!("{a}")) } + +fn const_in_pattern(x: Option<(i32, i32)>) -> Option<()> { + const N: i32 = 0; + + let Some((x, N)) = x else { + return None; + }; + + None +} diff --git a/src/tools/clippy/tests/ui/question_mark.rs b/src/tools/clippy/tests/ui/question_mark.rs index 64b51b849ed..99d0122a98f 100644 --- a/src/tools/clippy/tests/ui/question_mark.rs +++ b/src/tools/clippy/tests/ui/question_mark.rs @@ -539,3 +539,13 @@ fn issue_14615(a: MutexGuard<Option<u32>>) -> Option<String> { //~^^^ question_mark Some(format!("{a}")) } + +fn const_in_pattern(x: Option<(i32, i32)>) -> Option<()> { + const N: i32 = 0; + + let Some((x, N)) = x else { + return None; + }; + + None +} diff --git a/src/tools/clippy/tests/ui/trivially_copy_pass_by_ref.fixed b/src/tools/clippy/tests/ui/trivially_copy_pass_by_ref.fixed new file mode 100644 index 00000000000..af7d82130f0 --- /dev/null +++ b/src/tools/clippy/tests/ui/trivially_copy_pass_by_ref.fixed @@ -0,0 +1,179 @@ +//@normalize-stderr-test: "\(\d+ byte\)" -> "(N byte)" +//@normalize-stderr-test: "\(limit: \d+ byte\)" -> "(limit: N byte)" +#![deny(clippy::trivially_copy_pass_by_ref)] +#![allow( + clippy::disallowed_names, + clippy::extra_unused_lifetimes, + clippy::needless_lifetimes, + clippy::needless_pass_by_ref_mut, + clippy::redundant_field_names, + clippy::uninlined_format_args +)] + +#[derive(Copy, Clone)] +struct Foo(u32); + +#[derive(Copy, Clone)] +struct Bar([u8; 24]); + +#[derive(Copy, Clone)] +pub struct Color { + pub r: u8, + pub g: u8, + pub b: u8, + pub a: u8, +} + +struct FooRef<'a> { + foo: &'a Foo, +} + +type Baz = u32; + +fn good(a: &mut u32, b: u32, c: &Bar) {} + +fn good_return_implicit_lt_ref(foo: &Foo) -> &u32 { + &foo.0 +} + +#[allow(clippy::needless_lifetimes)] +fn good_return_explicit_lt_ref<'a>(foo: &'a Foo) -> &'a u32 { + &foo.0 +} + +fn good_return_implicit_lt_struct(foo: &Foo) -> FooRef { + FooRef { foo } +} + +#[allow(clippy::needless_lifetimes)] +fn good_return_explicit_lt_struct<'a>(foo: &'a Foo) -> FooRef<'a> { + FooRef { foo } +} + +fn bad(x: u32, y: Foo, z: Baz) {} +//~^ ERROR: this argument (4 byte) is passed by reference, but would be more efficient if passed by +//~| ERROR: this argument (4 byte) is passed by reference, but would be more efficient if passed by +//~| ERROR: this argument (4 byte) is passed by reference, but would be more efficient if passed by + +impl Foo { + fn good(self, a: &mut u32, b: u32, c: &Bar) {} + + fn good2(&mut self) {} + + fn bad(self, x: u32, y: Foo, z: Baz) {} + //~^ ERROR: this argument (4 byte) is passed by reference, but would be more efficient if passed by + //~| ERROR: this argument (4 byte) is passed by reference, but would be more efficient if passed by + //~| ERROR: this argument (4 byte) is passed by reference, but would be more efficient if passed by + //~| ERROR: this argument (4 byte) is passed by reference, but would be more efficient if passed by + + fn bad2(x: u32, y: Foo, z: Baz) {} + //~^ ERROR: this argument (4 byte) is passed by reference, but would be more efficient if passed by + //~| ERROR: this argument (4 byte) is passed by reference, but would be more efficient if passed by + //~| ERROR: this argument (4 byte) is passed by reference, but would be more efficient if passed by + + fn bad_issue7518(self, other: Self) {} + //~^ ERROR: this argument (4 byte) is passed by reference, but would be more efficient if +} + +impl AsRef<u32> for Foo { + fn as_ref(&self) -> &u32 { + &self.0 + } +} + +impl Bar { + fn good(&self, a: &mut u32, b: u32, c: &Bar) {} + + fn bad2(x: u32, y: Foo, z: Baz) {} + //~^ ERROR: this argument (4 byte) is passed by reference, but would be more efficient if + //~| ERROR: this argument (4 byte) is passed by reference, but would be more efficient if + //~| ERROR: this argument (4 byte) is passed by reference, but would be more efficient if +} + +trait MyTrait { + fn trait_method(&self, foo: Foo); + //~^ ERROR: this argument (4 byte) is passed by reference, but would be more efficient if +} + +pub trait MyTrait2 { + fn trait_method2(&self, color: &Color); +} + +trait MyTrait3 { + #[expect(clippy::trivially_copy_pass_by_ref)] + fn trait_method(&self, foo: &Foo); +} + +// Trait impls should not warn +impl MyTrait3 for Foo { + fn trait_method(&self, foo: &Foo) { + unimplemented!() + } +} + +mod issue3992 { + pub trait A { + #[allow(clippy::trivially_copy_pass_by_ref)] + fn a(b: &u16) {} + } + + #[allow(clippy::trivially_copy_pass_by_ref)] + pub fn c(d: &u16) {} +} + +mod issue5876 { + // Don't lint here as it is always inlined + #[inline(always)] + fn foo_always(x: &i32) { + println!("{}", x); + } + + #[inline(never)] + fn foo_never(x: i32) { + //~^ ERROR: this argument (4 byte) is passed by reference, but would be more efficient if passed by + println!("{}", x); + } + + #[inline] + fn foo(x: i32) { + //~^ ERROR: this argument (4 byte) is passed by reference, but would be more efficient if passed by + println!("{}", x); + } +} + +fn ref_to_opt_ref_implicit(x: &u32) -> Option<&u32> { + Some(x) +} + +fn ref_to_opt_ref_explicit<'a>(x: &'a u32) -> Option<&'a u32> { + Some(x) +} + +fn with_constraint<'a, 'b: 'a>(x: &'b u32, y: &'a u32) -> &'a u32 { + if true { x } else { y } +} + +async fn async_implicit(x: &u32) -> &u32 { + x +} + +async fn async_explicit<'a>(x: &'a u32) -> &'a u32 { + x +} + +fn unrelated_lifetimes<'a, 'b>(_x: u32, y: &'b u32) -> &'b u32 { + //~^ ERROR: this argument (4 byte) is passed by reference, but would be more efficient if passed by + y +} + +fn return_ptr(x: &u32) -> *const u32 { + x +} + +fn return_field_ptr(x: &(u32, u32)) -> *const u32 { + &x.0 +} + +fn return_field_ptr_addr_of(x: &(u32, u32)) -> *const u32 { + core::ptr::addr_of!(x.0) +} diff --git a/src/tools/clippy/tests/ui/trivially_copy_pass_by_ref.rs b/src/tools/clippy/tests/ui/trivially_copy_pass_by_ref.rs index 37bc6f89a20..00e11a1ea28 100644 --- a/src/tools/clippy/tests/ui/trivially_copy_pass_by_ref.rs +++ b/src/tools/clippy/tests/ui/trivially_copy_pass_by_ref.rs @@ -1,14 +1,15 @@ //@normalize-stderr-test: "\(\d+ byte\)" -> "(N byte)" -//@normalize-stderr-test: "\(limit: \d+ byte\)" -> "(limit: 8 byte)" +//@normalize-stderr-test: "\(limit: \d+ byte\)" -> "(limit: N byte)" #![deny(clippy::trivially_copy_pass_by_ref)] #![allow( clippy::disallowed_names, + clippy::extra_unused_lifetimes, clippy::needless_lifetimes, + clippy::needless_pass_by_ref_mut, clippy::redundant_field_names, - clippy::uninlined_format_args, - clippy::needless_pass_by_ref_mut + clippy::uninlined_format_args )] -//@no-rustfix + #[derive(Copy, Clone)] struct Foo(u32); @@ -90,21 +91,26 @@ impl Bar { } trait MyTrait { - fn trait_method(&self, _foo: &Foo); + fn trait_method(&self, foo: &Foo); //~^ ERROR: this argument (4 byte) is passed by reference, but would be more efficient if } pub trait MyTrait2 { - fn trait_method2(&self, _color: &Color); + fn trait_method2(&self, color: &Color); +} + +trait MyTrait3 { + #[expect(clippy::trivially_copy_pass_by_ref)] + fn trait_method(&self, foo: &Foo); } -impl MyTrait for Foo { - fn trait_method(&self, _foo: &Foo) { +// Trait impls should not warn +impl MyTrait3 for Foo { + fn trait_method(&self, foo: &Foo) { unimplemented!() } } -#[allow(unused_variables)] mod issue3992 { pub trait A { #[allow(clippy::trivially_copy_pass_by_ref)] @@ -135,57 +141,39 @@ mod issue5876 { } } -fn _ref_to_opt_ref_implicit(x: &u32) -> Option<&u32> { +fn ref_to_opt_ref_implicit(x: &u32) -> Option<&u32> { Some(x) } -#[allow(clippy::needless_lifetimes)] -fn _ref_to_opt_ref_explicit<'a>(x: &'a u32) -> Option<&'a u32> { +fn ref_to_opt_ref_explicit<'a>(x: &'a u32) -> Option<&'a u32> { Some(x) } -fn _with_constraint<'a, 'b: 'a>(x: &'b u32, y: &'a u32) -> &'a u32 { +fn with_constraint<'a, 'b: 'a>(x: &'b u32, y: &'a u32) -> &'a u32 { if true { x } else { y } } -async fn _async_implicit(x: &u32) -> &u32 { +async fn async_implicit(x: &u32) -> &u32 { x } -#[allow(clippy::needless_lifetimes)] -async fn _async_explicit<'a>(x: &'a u32) -> &'a u32 { +async fn async_explicit<'a>(x: &'a u32) -> &'a u32 { x } -fn _unrelated_lifetimes<'a, 'b>(_x: &'a u32, y: &'b u32) -> &'b u32 { +fn unrelated_lifetimes<'a, 'b>(_x: &'a u32, y: &'b u32) -> &'b u32 { //~^ ERROR: this argument (4 byte) is passed by reference, but would be more efficient if passed by y } -fn _return_ptr(x: &u32) -> *const u32 { +fn return_ptr(x: &u32) -> *const u32 { x } -fn _return_field_ptr(x: &(u32, u32)) -> *const u32 { +fn return_field_ptr(x: &(u32, u32)) -> *const u32 { &x.0 } -fn _return_field_ptr_addr_of(x: &(u32, u32)) -> *const u32 { +fn return_field_ptr_addr_of(x: &(u32, u32)) -> *const u32 { core::ptr::addr_of!(x.0) } - -fn main() { - let (mut foo, bar) = (Foo(0), Bar([0; 24])); - let (mut a, b, c, x, y, z) = (0, 0, Bar([0; 24]), 0, Foo(0), 0); - good(&mut a, b, &c); - good_return_implicit_lt_ref(&y); - good_return_explicit_lt_ref(&y); - bad(&x, &y, &z); - foo.good(&mut a, b, &c); - foo.good2(); - foo.bad(&x, &y, &z); - Foo::bad2(&x, &y, &z); - bar.good(&mut a, b, &c); - Bar::bad2(&x, &y, &z); - foo.as_ref(); -} diff --git a/src/tools/clippy/tests/ui/trivially_copy_pass_by_ref.stderr b/src/tools/clippy/tests/ui/trivially_copy_pass_by_ref.stderr index e813fecf653..f101ac5ccd6 100644 --- a/src/tools/clippy/tests/ui/trivially_copy_pass_by_ref.stderr +++ b/src/tools/clippy/tests/ui/trivially_copy_pass_by_ref.stderr @@ -1,5 +1,5 @@ -error: this argument (N byte) is passed by reference, but would be more efficient if passed by value (limit: 8 byte) - --> tests/ui/trivially_copy_pass_by_ref.rs:52:11 +error: this argument (N byte) is passed by reference, but would be more efficient if passed by value (limit: N byte) + --> tests/ui/trivially_copy_pass_by_ref.rs:53:11 | LL | fn bad(x: &u32, y: &Foo, z: &Baz) {} | ^^^^ help: consider passing by value instead: `u32` @@ -10,107 +10,107 @@ note: the lint level is defined here LL | #![deny(clippy::trivially_copy_pass_by_ref)] | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ -error: this argument (N byte) is passed by reference, but would be more efficient if passed by value (limit: 8 byte) - --> tests/ui/trivially_copy_pass_by_ref.rs:52:20 +error: this argument (N byte) is passed by reference, but would be more efficient if passed by value (limit: N byte) + --> tests/ui/trivially_copy_pass_by_ref.rs:53:20 | LL | fn bad(x: &u32, y: &Foo, z: &Baz) {} | ^^^^ help: consider passing by value instead: `Foo` -error: this argument (N byte) is passed by reference, but would be more efficient if passed by value (limit: 8 byte) - --> tests/ui/trivially_copy_pass_by_ref.rs:52:29 +error: this argument (N byte) is passed by reference, but would be more efficient if passed by value (limit: N byte) + --> tests/ui/trivially_copy_pass_by_ref.rs:53:29 | LL | fn bad(x: &u32, y: &Foo, z: &Baz) {} | ^^^^ help: consider passing by value instead: `Baz` -error: this argument (N byte) is passed by reference, but would be more efficient if passed by value (limit: 8 byte) - --> tests/ui/trivially_copy_pass_by_ref.rs:62:12 +error: this argument (N byte) is passed by reference, but would be more efficient if passed by value (limit: N byte) + --> tests/ui/trivially_copy_pass_by_ref.rs:63:12 | LL | fn bad(&self, x: &u32, y: &Foo, z: &Baz) {} | ^^^^^ help: consider passing by value instead: `self` -error: this argument (N byte) is passed by reference, but would be more efficient if passed by value (limit: 8 byte) - --> tests/ui/trivially_copy_pass_by_ref.rs:62:22 +error: this argument (N byte) is passed by reference, but would be more efficient if passed by value (limit: N byte) + --> tests/ui/trivially_copy_pass_by_ref.rs:63:22 | LL | fn bad(&self, x: &u32, y: &Foo, z: &Baz) {} | ^^^^ help: consider passing by value instead: `u32` -error: this argument (N byte) is passed by reference, but would be more efficient if passed by value (limit: 8 byte) - --> tests/ui/trivially_copy_pass_by_ref.rs:62:31 +error: this argument (N byte) is passed by reference, but would be more efficient if passed by value (limit: N byte) + --> tests/ui/trivially_copy_pass_by_ref.rs:63:31 | LL | fn bad(&self, x: &u32, y: &Foo, z: &Baz) {} | ^^^^ help: consider passing by value instead: `Foo` -error: this argument (N byte) is passed by reference, but would be more efficient if passed by value (limit: 8 byte) - --> tests/ui/trivially_copy_pass_by_ref.rs:62:40 +error: this argument (N byte) is passed by reference, but would be more efficient if passed by value (limit: N byte) + --> tests/ui/trivially_copy_pass_by_ref.rs:63:40 | LL | fn bad(&self, x: &u32, y: &Foo, z: &Baz) {} | ^^^^ help: consider passing by value instead: `Baz` -error: this argument (N byte) is passed by reference, but would be more efficient if passed by value (limit: 8 byte) - --> tests/ui/trivially_copy_pass_by_ref.rs:68:16 +error: this argument (N byte) is passed by reference, but would be more efficient if passed by value (limit: N byte) + --> tests/ui/trivially_copy_pass_by_ref.rs:69:16 | LL | fn bad2(x: &u32, y: &Foo, z: &Baz) {} | ^^^^ help: consider passing by value instead: `u32` -error: this argument (N byte) is passed by reference, but would be more efficient if passed by value (limit: 8 byte) - --> tests/ui/trivially_copy_pass_by_ref.rs:68:25 +error: this argument (N byte) is passed by reference, but would be more efficient if passed by value (limit: N byte) + --> tests/ui/trivially_copy_pass_by_ref.rs:69:25 | LL | fn bad2(x: &u32, y: &Foo, z: &Baz) {} | ^^^^ help: consider passing by value instead: `Foo` -error: this argument (N byte) is passed by reference, but would be more efficient if passed by value (limit: 8 byte) - --> tests/ui/trivially_copy_pass_by_ref.rs:68:34 +error: this argument (N byte) is passed by reference, but would be more efficient if passed by value (limit: N byte) + --> tests/ui/trivially_copy_pass_by_ref.rs:69:34 | LL | fn bad2(x: &u32, y: &Foo, z: &Baz) {} | ^^^^ help: consider passing by value instead: `Baz` -error: this argument (N byte) is passed by reference, but would be more efficient if passed by value (limit: 8 byte) - --> tests/ui/trivially_copy_pass_by_ref.rs:73:35 +error: this argument (N byte) is passed by reference, but would be more efficient if passed by value (limit: N byte) + --> tests/ui/trivially_copy_pass_by_ref.rs:74:35 | LL | fn bad_issue7518(self, other: &Self) {} | ^^^^^ help: consider passing by value instead: `Self` -error: this argument (N byte) is passed by reference, but would be more efficient if passed by value (limit: 8 byte) - --> tests/ui/trivially_copy_pass_by_ref.rs:86:16 +error: this argument (N byte) is passed by reference, but would be more efficient if passed by value (limit: N byte) + --> tests/ui/trivially_copy_pass_by_ref.rs:87:16 | LL | fn bad2(x: &u32, y: &Foo, z: &Baz) {} | ^^^^ help: consider passing by value instead: `u32` -error: this argument (N byte) is passed by reference, but would be more efficient if passed by value (limit: 8 byte) - --> tests/ui/trivially_copy_pass_by_ref.rs:86:25 +error: this argument (N byte) is passed by reference, but would be more efficient if passed by value (limit: N byte) + --> tests/ui/trivially_copy_pass_by_ref.rs:87:25 | LL | fn bad2(x: &u32, y: &Foo, z: &Baz) {} | ^^^^ help: consider passing by value instead: `Foo` -error: this argument (N byte) is passed by reference, but would be more efficient if passed by value (limit: 8 byte) - --> tests/ui/trivially_copy_pass_by_ref.rs:86:34 +error: this argument (N byte) is passed by reference, but would be more efficient if passed by value (limit: N byte) + --> tests/ui/trivially_copy_pass_by_ref.rs:87:34 | LL | fn bad2(x: &u32, y: &Foo, z: &Baz) {} | ^^^^ help: consider passing by value instead: `Baz` -error: this argument (N byte) is passed by reference, but would be more efficient if passed by value (limit: 8 byte) - --> tests/ui/trivially_copy_pass_by_ref.rs:93:34 +error: this argument (N byte) is passed by reference, but would be more efficient if passed by value (limit: N byte) + --> tests/ui/trivially_copy_pass_by_ref.rs:94:33 | -LL | fn trait_method(&self, _foo: &Foo); - | ^^^^ help: consider passing by value instead: `Foo` +LL | fn trait_method(&self, foo: &Foo); + | ^^^^ help: consider passing by value instead: `Foo` -error: this argument (N byte) is passed by reference, but would be more efficient if passed by value (limit: 8 byte) - --> tests/ui/trivially_copy_pass_by_ref.rs:126:21 +error: this argument (N byte) is passed by reference, but would be more efficient if passed by value (limit: N byte) + --> tests/ui/trivially_copy_pass_by_ref.rs:132:21 | LL | fn foo_never(x: &i32) { | ^^^^ help: consider passing by value instead: `i32` -error: this argument (N byte) is passed by reference, but would be more efficient if passed by value (limit: 8 byte) - --> tests/ui/trivially_copy_pass_by_ref.rs:132:15 +error: this argument (N byte) is passed by reference, but would be more efficient if passed by value (limit: N byte) + --> tests/ui/trivially_copy_pass_by_ref.rs:138:15 | LL | fn foo(x: &i32) { | ^^^^ help: consider passing by value instead: `i32` -error: this argument (N byte) is passed by reference, but would be more efficient if passed by value (limit: 8 byte) - --> tests/ui/trivially_copy_pass_by_ref.rs:160:37 +error: this argument (N byte) is passed by reference, but would be more efficient if passed by value (limit: N byte) + --> tests/ui/trivially_copy_pass_by_ref.rs:164:36 | -LL | fn _unrelated_lifetimes<'a, 'b>(_x: &'a u32, y: &'b u32) -> &'b u32 { - | ^^^^^^^ help: consider passing by value instead: `u32` +LL | fn unrelated_lifetimes<'a, 'b>(_x: &'a u32, y: &'b u32) -> &'b u32 { + | ^^^^^^^ help: consider passing by value instead: `u32` error: aborting due to 18 previous errors diff --git a/src/tools/clippy/tests/ui/while_let_loop.rs b/src/tools/clippy/tests/ui/while_let_loop.rs index d591ab984cf..95062c9f46c 100644 --- a/src/tools/clippy/tests/ui/while_let_loop.rs +++ b/src/tools/clippy/tests/ui/while_let_loop.rs @@ -154,3 +154,89 @@ fn issue_5715(mut m: core::cell::RefCell<Option<u32>>) { m = core::cell::RefCell::new(Some(x + 1)); } } + +mod issue_362 { + pub fn merge_sorted<T>(xs: Vec<T>, ys: Vec<T>) -> Vec<T> + where + T: PartialOrd, + { + let total_len = xs.len() + ys.len(); + let mut res = Vec::with_capacity(total_len); + let mut ix = xs.into_iter().peekable(); + let mut iy = ys.into_iter().peekable(); + loop { + //~^ while_let_loop + let lt = match (ix.peek(), iy.peek()) { + (Some(x), Some(y)) => x < y, + _ => break, + }; + res.push(if lt { &mut ix } else { &mut iy }.next().unwrap()); + } + res.extend(ix); + res.extend(iy); + res + } +} + +fn let_assign() { + loop { + //~^ while_let_loop + let x = if let Some(y) = Some(3) { + y + } else { + break; + }; + if x == 3 { + break; + } + } + + loop { + //~^ while_let_loop + let x: u32 = if let Some(y) = Some(3) { + y + } else { + break; + }; + if x == 3 { + break; + } + } + + loop { + //~^ while_let_loop + let x = if let Some(x) = Some(3) { + x + } else { + break; + }; + if x == 3 { + break; + } + } + + loop { + //~^ while_let_loop + let x: u32 = if let Some(x) = Some(3) { + x + } else { + break; + }; + if x == 3 { + break; + } + } + + loop { + //~^ while_let_loop + let x = if let Some(x) = Some(2) { + let t = 1; + t + x + } else { + break; + }; + if x == 3 { + break; + } + } +} diff --git a/src/tools/clippy/tests/ui/while_let_loop.stderr b/src/tools/clippy/tests/ui/while_let_loop.stderr index bd482857e67..ed42628a53e 100644 --- a/src/tools/clippy/tests/ui/while_let_loop.stderr +++ b/src/tools/clippy/tests/ui/while_let_loop.stderr @@ -57,7 +57,125 @@ LL | | let (e, l) = match "".split_whitespace().next() { ... | LL | | let _ = (e, l); LL | | } - | |_____^ help: try: `while let Some(word) = "".split_whitespace().next() { .. }` + | |_____^ + | +help: try + | +LL ~ while let Some(word) = "".split_whitespace().next() { +LL + let (e, l) = (word.is_empty(), word.len()); +LL + .. +LL + } + | + +error: this loop could be written as a `while let` loop + --> tests/ui/while_let_loop.rs:167:9 + | +LL | / loop { +LL | | +LL | | let lt = match (ix.peek(), iy.peek()) { +LL | | (Some(x), Some(y)) => x < y, +... | +LL | | res.push(if lt { &mut ix } else { &mut iy }.next().unwrap()); +LL | | } + | |_________^ + | +help: try + | +LL ~ while let (Some(x), Some(y)) = (ix.peek(), iy.peek()) { +LL + let lt = x < y; +LL + .. +LL + } + | + +error: this loop could be written as a `while let` loop + --> tests/ui/while_let_loop.rs:182:5 + | +LL | / loop { +LL | | +LL | | let x = if let Some(y) = Some(3) { +LL | | y +... | +LL | | } + | |_____^ + | +help: try + | +LL ~ while let Some(y) = Some(3) { +LL + let x = y; +LL + .. +LL + } + | + +error: this loop could be written as a `while let` loop + --> tests/ui/while_let_loop.rs:194:5 + | +LL | / loop { +LL | | +LL | | let x: u32 = if let Some(y) = Some(3) { +LL | | y +... | +LL | | } + | |_____^ + | +help: try + | +LL ~ while let Some(y) = Some(3) { +LL + let x: u32 = y; +LL + .. +LL + } + | + +error: this loop could be written as a `while let` loop + --> tests/ui/while_let_loop.rs:206:5 + | +LL | / loop { +LL | | +LL | | let x = if let Some(x) = Some(3) { +LL | | x +... | +LL | | } + | |_____^ help: try: `while let Some(x) = Some(3) { .. }` + +error: this loop could be written as a `while let` loop + --> tests/ui/while_let_loop.rs:218:5 + | +LL | / loop { +LL | | +LL | | let x: u32 = if let Some(x) = Some(3) { +LL | | x +... | +LL | | } + | |_____^ + | +help: try + | +LL ~ while let Some(x) = Some(3) { +LL + let x: u32 = x; +LL + .. +LL + } + | + +error: this loop could be written as a `while let` loop + --> tests/ui/while_let_loop.rs:230:5 + | +LL | / loop { +LL | | +LL | | let x = if let Some(x) = Some(2) { +LL | | let t = 1; +... | +LL | | } + | |_____^ + | +help: try + | +LL ~ while let Some(x) = Some(2) { +LL + let x = { +LL + let t = 1; +LL + t + x +LL + }; +LL + .. +LL + } + | -error: aborting due to 5 previous errors +error: aborting due to 11 previous errors diff --git a/src/tools/clippy/triagebot.toml b/src/tools/clippy/triagebot.toml index 389f22c6a2c..16557a4bebb 100644 --- a/src/tools/clippy/triagebot.toml +++ b/src/tools/clippy/triagebot.toml @@ -45,6 +45,7 @@ contributing_url = "https://github.com/rust-lang/rust-clippy/blob/master/CONTRIB users_on_vacation = [ "matthiaskrgr", "Manishearth", + "blyxyas", ] [assign.owners] diff --git a/src/tools/clippy/util/gh-pages/index_template.html b/src/tools/clippy/util/gh-pages/index_template.html index 19dc1ec0b0c..865b9523c39 100644 --- a/src/tools/clippy/util/gh-pages/index_template.html +++ b/src/tools/clippy/util/gh-pages/index_template.html @@ -50,7 +50,7 @@ Otherwise, have a great day =^.^= <div class="container"> {# #} <div class="page-header"> {# #} - <h1>Clippy Lints</h1> {# #} + <h1>Clippy Lints <span id="lint-count" class="badge"></span></h1> {# #} </div> {# #} <noscript> {# #} diff --git a/src/tools/clippy/util/gh-pages/script.js b/src/tools/clippy/util/gh-pages/script.js index c942a6a05a1..fec883938d6 100644 --- a/src/tools/clippy/util/gh-pages/script.js +++ b/src/tools/clippy/util/gh-pages/script.js @@ -71,6 +71,7 @@ window.searchState = { } else { window.location.hash = ''; } + updateLintCount(); }, }; @@ -598,3 +599,14 @@ generateSearch(); parseURLFilters(); scrollToLintByURL(); filters.filterLints(); +updateLintCount(); + +function updateLintCount() { + const allLints = filters.getAllLints(); + const totalLints = allLints.length; + + const countElement = document.getElementById("lint-count"); + if (countElement) { + countElement.innerText = `Total number: ${totalLints}`; + } +} |