Merge remote-tracking branch 'upstream/master' into rustup

This commit is contained in:
Philipp Krones 2023-07-28 23:40:04 +02:00
commit 3d60241841
No known key found for this signature in database
GPG key ID: 1CA0DF2AF59D68A5
215 changed files with 7469 additions and 1950 deletions

View file

@ -0,0 +1,100 @@
use clippy_utils::diagnostics::span_lint;
use clippy_utils::source::snippet_opt;
use rustc_data_structures::fx::FxHashSet;
use rustc_hir::def::{DefKind, Res};
use rustc_hir::def_id::{DefId, CRATE_DEF_INDEX};
use rustc_hir::{HirId, ItemKind, Node, Path};
use rustc_lint::{LateContext, LateLintPass};
use rustc_session::{declare_tool_lint, impl_lint_pass};
use rustc_span::symbol::kw;
declare_clippy_lint! {
/// ### What it does
/// Checks for usage of items through absolute paths, like `std::env::current_dir`.
///
/// ### Why is this bad?
/// Many codebases have their own style when it comes to importing, but one that is seldom used
/// is using absolute paths *everywhere*. This is generally considered unidiomatic, and you
/// should add a `use` statement.
///
/// The default maximum segments (2) is pretty strict, you may want to increase this in
/// `clippy.toml`.
///
/// Note: One exception to this is code from macro expansion - this does not lint such cases, as
/// using absolute paths is the proper way of referencing items in one.
///
/// ### Example
/// ```rust
/// let x = std::f64::consts::PI;
/// ```
/// Use any of the below instead, or anything else:
/// ```rust
/// use std::f64;
/// use std::f64::consts;
/// use std::f64::consts::PI;
/// let x = f64::consts::PI;
/// let x = consts::PI;
/// let x = PI;
/// use std::f64::consts as f64_consts;
/// let x = f64_consts::PI;
/// ```
#[clippy::version = "1.73.0"]
pub ABSOLUTE_PATHS,
restriction,
"checks for usage of an item without a `use` statement"
}
impl_lint_pass!(AbsolutePaths => [ABSOLUTE_PATHS]);
pub struct AbsolutePaths {
pub absolute_paths_max_segments: u64,
pub absolute_paths_allowed_crates: FxHashSet<String>,
}
impl LateLintPass<'_> for AbsolutePaths {
// We should only lint `QPath::Resolved`s, but since `Path` is only used in `Resolved` and `UsePath`
// we don't need to use a visitor or anything as we can just check if the `Node` for `hir_id` isn't
// a `Use`
#[expect(clippy::cast_possible_truncation)]
fn check_path(&mut self, cx: &LateContext<'_>, path: &Path<'_>, hir_id: HirId) {
let Self {
absolute_paths_max_segments,
absolute_paths_allowed_crates,
} = self;
if !path.span.from_expansion()
&& let Some(node) = cx.tcx.hir().find(hir_id)
&& !matches!(node, Node::Item(item) if matches!(item.kind, ItemKind::Use(_, _)))
&& let [first, rest @ ..] = path.segments
// Handle `::std`
&& let (segment, len) = if first.ident.name == kw::PathRoot {
// Indexing is fine as `PathRoot` must be followed by another segment. `len() - 1`
// is fine here for the same reason
(&rest[0], path.segments.len() - 1)
} else {
(first, path.segments.len())
}
&& len > *absolute_paths_max_segments as usize
&& let Some(segment_snippet) = snippet_opt(cx, segment.ident.span)
&& segment_snippet == segment.ident.as_str()
{
let is_abs_external =
matches!(segment.res, Res::Def(DefKind::Mod, DefId { index, .. }) if index == CRATE_DEF_INDEX);
let is_abs_crate = segment.ident.name == kw::Crate;
if is_abs_external && absolute_paths_allowed_crates.contains(segment.ident.name.as_str())
|| is_abs_crate && absolute_paths_allowed_crates.contains("crate")
{
return;
}
if is_abs_external || is_abs_crate {
span_lint(
cx,
ABSOLUTE_PATHS,
path.span,
"consider bringing this path into scope with the `use` keyword",
);
}
}
}
}

View file

@ -1,6 +1,6 @@
use clippy_utils::diagnostics::span_lint_and_then;
use clippy_utils::last_path_segment;
use clippy_utils::ty::{implements_trait, is_type_diagnostic_item};
use clippy_utils::{is_from_proc_macro, last_path_segment};
use rustc_hir::{Expr, ExprKind};
use rustc_lint::{LateContext, LateLintPass};
use rustc_middle::ty;
@ -38,10 +38,11 @@ declare_clippy_lint! {
}
declare_lint_pass!(ArcWithNonSendSync => [ARC_WITH_NON_SEND_SYNC]);
impl LateLintPass<'_> for ArcWithNonSendSync {
fn check_expr(&mut self, cx: &LateContext<'_>, expr: &Expr<'_>) {
let ty = cx.typeck_results().expr_ty(expr);
if is_type_diagnostic_item(cx, ty, sym::Arc)
impl<'tcx> LateLintPass<'tcx> for ArcWithNonSendSync {
fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) {
if !expr.span.from_expansion()
&& let ty = cx.typeck_results().expr_ty(expr)
&& is_type_diagnostic_item(cx, ty, sym::Arc)
&& let ExprKind::Call(func, [arg]) = expr.kind
&& let ExprKind::Path(func_path) = func.kind
&& last_path_segment(&func_path).ident.name == sym::new
@ -54,6 +55,7 @@ impl LateLintPass<'_> for ArcWithNonSendSync {
&& let Some(sync) = cx.tcx.lang_items().sync_trait()
&& let [is_send, is_sync] = [send, sync].map(|id| implements_trait(cx, arg_ty, id, &[]))
&& !(is_send && is_sync)
&& !is_from_proc_macro(cx, expr)
{
span_lint_and_then(
cx,

View file

@ -181,6 +181,14 @@ declare_clippy_lint! {
/// ### Why is this bad?
/// It's just unnecessary.
///
/// ### Known problems
/// When the expression on the left is a function call, the lint considers the return type to be
/// a type alias if it's aliased through a `use` statement
/// (like `use std::io::Result as IoResult`). It will not lint such cases.
///
/// This check is also rather primitive. It will only work on primitive types without any
/// intermediate references, raw pointers and trait objects may or may not work.
///
/// ### Example
/// ```rust
/// let _ = 2i32 as i32;

View file

@ -85,11 +85,6 @@ pub(super) fn check<'tcx>(
}
}
// skip cast of fn call that returns type alias
if let ExprKind::Cast(inner, ..) = expr.kind && is_cast_from_ty_alias(cx, inner, cast_from) {
return false;
}
// skip cast to non-primitive type
if_chain! {
if let ExprKind::Cast(_, cast_to) = expr.kind;
@ -101,6 +96,11 @@ pub(super) fn check<'tcx>(
}
}
// skip cast of fn call that returns type alias
if let ExprKind::Cast(inner, ..) = expr.kind && is_cast_from_ty_alias(cx, inner, cast_from) {
return false;
}
if let Some(lit) = get_numeric_literal(cast_expr) {
let literal_str = &cast_str;
@ -254,14 +254,12 @@ fn is_cast_from_ty_alias<'tcx>(cx: &LateContext<'tcx>, expr: impl Visitable<'tcx
// function's declaration snippet is exactly equal to the `Ty`. That way, we can
// see whether it's a type alias.
//
// Will this work for more complex types? Probably not!
// FIXME: This won't work if the type is given an alias through `use`, should we
// consider this a type alias as well?
if !snippet
.split("->")
.skip(0)
.map(|s| {
s.trim() == cast_from.to_string()
|| s.split("where").any(|ty| ty.trim() == cast_from.to_string())
})
.skip(1)
.map(|s| snippet_eq_ty(s, cast_from) || s.split("where").any(|ty| snippet_eq_ty(ty, cast_from)))
.any(|a| a)
{
return ControlFlow::Break(());
@ -288,3 +286,7 @@ fn is_cast_from_ty_alias<'tcx>(cx: &LateContext<'tcx>, expr: impl Visitable<'tcx
})
.is_some()
}
fn snippet_eq_ty(snippet: &str, ty: Ty<'_>) -> bool {
snippet.trim() == ty.to_string() || snippet.trim().contains(&format!("::{ty}"))
}

View file

@ -37,6 +37,7 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[
crate::utils::internal_lints::produce_ice::PRODUCE_ICE_INFO,
#[cfg(feature = "internal")]
crate::utils::internal_lints::unnecessary_def_path::UNNECESSARY_DEF_PATH_INFO,
crate::absolute_paths::ABSOLUTE_PATHS_INFO,
crate::allow_attributes::ALLOW_ATTRIBUTES_INFO,
crate::almost_complete_range::ALMOST_COMPLETE_RANGE_INFO,
crate::approx_const::APPROX_CONSTANT_INFO,
@ -155,6 +156,7 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[
crate::enum_variants::MODULE_INCEPTION_INFO,
crate::enum_variants::MODULE_NAME_REPETITIONS_INFO,
crate::equatable_if_let::EQUATABLE_IF_LET_INFO,
crate::error_impl_error::ERROR_IMPL_ERROR_INFO,
crate::escape::BOXED_LOCAL_INFO,
crate::eta_reduction::REDUNDANT_CLOSURE_INFO,
crate::eta_reduction::REDUNDANT_CLOSURE_FOR_METHOD_CALLS_INFO,
@ -183,6 +185,7 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[
crate::formatting::SUSPICIOUS_ASSIGNMENT_FORMATTING_INFO,
crate::formatting::SUSPICIOUS_ELSE_FORMATTING_INFO,
crate::formatting::SUSPICIOUS_UNARY_OP_FORMATTING_INFO,
crate::four_forward_slashes::FOUR_FORWARD_SLASHES_INFO,
crate::from_over_into::FROM_OVER_INTO_INFO,
crate::from_raw_with_void_ptr::FROM_RAW_WITH_VOID_PTR_INFO,
crate::from_str_radix_10::FROM_STR_RADIX_10_INFO,
@ -305,6 +308,7 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[
crate::matches::MATCH_WILDCARD_FOR_SINGLE_VARIANTS_INFO,
crate::matches::MATCH_WILD_ERR_ARM_INFO,
crate::matches::NEEDLESS_MATCH_INFO,
crate::matches::REDUNDANT_GUARDS_INFO,
crate::matches::REDUNDANT_PATTERN_MATCHING_INFO,
crate::matches::REST_PAT_IN_FULLY_BOUND_STRUCTS_INFO,
crate::matches::SIGNIFICANT_DROP_IN_SCRUTINEE_INFO,
@ -333,11 +337,13 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[
crate::methods::EXPECT_USED_INFO,
crate::methods::EXTEND_WITH_DRAIN_INFO,
crate::methods::FILETYPE_IS_FILE_INFO,
crate::methods::FILTER_MAP_BOOL_THEN_INFO,
crate::methods::FILTER_MAP_IDENTITY_INFO,
crate::methods::FILTER_MAP_NEXT_INFO,
crate::methods::FILTER_NEXT_INFO,
crate::methods::FLAT_MAP_IDENTITY_INFO,
crate::methods::FLAT_MAP_OPTION_INFO,
crate::methods::FORMAT_COLLECT_INFO,
crate::methods::FROM_ITER_INSTEAD_OF_COLLECT_INFO,
crate::methods::GET_FIRST_INFO,
crate::methods::GET_LAST_WITH_LEN_INFO,
@ -358,6 +364,7 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[
crate::methods::ITER_ON_SINGLE_ITEMS_INFO,
crate::methods::ITER_OVEREAGER_CLONED_INFO,
crate::methods::ITER_SKIP_NEXT_INFO,
crate::methods::ITER_SKIP_ZERO_INFO,
crate::methods::ITER_WITH_DRAIN_INFO,
crate::methods::MANUAL_FILTER_MAP_INFO,
crate::methods::MANUAL_FIND_MAP_INFO,
@ -391,6 +398,7 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[
crate::methods::OR_THEN_UNWRAP_INFO,
crate::methods::PATH_BUF_PUSH_OVERWRITE_INFO,
crate::methods::RANGE_ZIP_WITH_LEN_INFO,
crate::methods::READONLY_WRITE_LOCK_INFO,
crate::methods::READ_LINE_WITHOUT_TRIM_INFO,
crate::methods::REPEAT_ONCE_INFO,
crate::methods::RESULT_MAP_OR_INTO_OPTION_INFO,
@ -403,6 +411,7 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[
crate::methods::SKIP_WHILE_NEXT_INFO,
crate::methods::STABLE_SORT_PRIMITIVE_INFO,
crate::methods::STRING_EXTEND_CHARS_INFO,
crate::methods::STRING_LIT_CHARS_ANY_INFO,
crate::methods::SUSPICIOUS_COMMAND_ARG_SPACE_INFO,
crate::methods::SUSPICIOUS_MAP_INFO,
crate::methods::SUSPICIOUS_SPLITN_INFO,
@ -418,7 +427,7 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[
crate::methods::UNNECESSARY_LITERAL_UNWRAP_INFO,
crate::methods::UNNECESSARY_SORT_BY_INFO,
crate::methods::UNNECESSARY_TO_OWNED_INFO,
crate::methods::UNWRAP_OR_ELSE_DEFAULT_INFO,
crate::methods::UNWRAP_OR_DEFAULT_INFO,
crate::methods::UNWRAP_USED_INFO,
crate::methods::USELESS_ASREF_INFO,
crate::methods::VEC_RESIZE_TO_ZERO_INFO,
@ -555,6 +564,7 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[
crate::redundant_closure_call::REDUNDANT_CLOSURE_CALL_INFO,
crate::redundant_else::REDUNDANT_ELSE_INFO,
crate::redundant_field_names::REDUNDANT_FIELD_NAMES_INFO,
crate::redundant_locals::REDUNDANT_LOCALS_INFO,
crate::redundant_pub_crate::REDUNDANT_PUB_CRATE_INFO,
crate::redundant_slicing::DEREF_BY_SLICING_INFO,
crate::redundant_slicing::REDUNDANT_SLICING_INFO,
@ -568,6 +578,7 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[
crate::return_self_not_must_use::RETURN_SELF_NOT_MUST_USE_INFO,
crate::returns::LET_AND_RETURN_INFO,
crate::returns::NEEDLESS_RETURN_INFO,
crate::returns::NEEDLESS_RETURN_WITH_QUESTION_MARK_INFO,
crate::same_name_method::SAME_NAME_METHOD_INFO,
crate::self_named_constructors::SELF_NAMED_CONSTRUCTORS_INFO,
crate::semicolon_block::SEMICOLON_INSIDE_BLOCK_INFO,

File diff suppressed because it is too large Load diff

View file

@ -0,0 +1,87 @@
use clippy_utils::diagnostics::{span_lint, span_lint_hir_and_then};
use clippy_utils::path_res;
use clippy_utils::ty::implements_trait;
use rustc_hir::def_id::{DefId, LocalDefId};
use rustc_hir::{Item, ItemKind};
use rustc_hir_analysis::hir_ty_to_ty;
use rustc_lint::{LateContext, LateLintPass};
use rustc_middle::ty::Visibility;
use rustc_session::{declare_lint_pass, declare_tool_lint};
use rustc_span::sym;
declare_clippy_lint! {
/// ### What it does
/// Checks for types named `Error` that implement `Error`.
///
/// ### Why is this bad?
/// It can become confusing when a codebase has 20 types all named `Error`, requiring either
/// aliasing them in the `use` statement or qualifying them like `my_module::Error`. This
/// hinders comprehension, as it requires you to memorize every variation of importing `Error`
/// used across a codebase.
///
/// ### Example
/// ```rust,ignore
/// #[derive(Debug)]
/// pub enum Error { ... }
///
/// impl std::fmt::Display for Error { ... }
///
/// impl std::error::Error for Error { ... }
/// ```
#[clippy::version = "1.72.0"]
pub ERROR_IMPL_ERROR,
restriction,
"exported types named `Error` that implement `Error`"
}
declare_lint_pass!(ErrorImplError => [ERROR_IMPL_ERROR]);
impl<'tcx> LateLintPass<'tcx> for ErrorImplError {
fn check_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx Item<'tcx>) {
let Some(error_def_id) = cx.tcx.get_diagnostic_item(sym::Error) else {
return;
};
match item.kind {
ItemKind::TyAlias(ty, _) if implements_trait(cx, hir_ty_to_ty(cx.tcx, ty), error_def_id, &[])
&& item.ident.name == sym::Error
&& is_visible_outside_module(cx, item.owner_id.def_id) =>
{
span_lint(
cx,
ERROR_IMPL_ERROR,
item.ident.span,
"exported type alias named `Error` that implements `Error`",
);
},
ItemKind::Impl(imp) if let Some(trait_def_id) = imp.of_trait.and_then(|t| t.trait_def_id())
&& error_def_id == trait_def_id
&& let Some(def_id) = path_res(cx, imp.self_ty).opt_def_id().and_then(DefId::as_local)
&& let hir_id = cx.tcx.hir().local_def_id_to_hir_id(def_id)
&& let Some(ident) = cx.tcx.opt_item_ident(def_id.to_def_id())
&& ident.name == sym::Error
&& is_visible_outside_module(cx, def_id) =>
{
span_lint_hir_and_then(
cx,
ERROR_IMPL_ERROR,
hir_id,
ident.span,
"exported type named `Error` that implements `Error`",
|diag| {
diag.span_note(item.span, "`Error` was implemented here");
}
);
}
_ => {},
}
}
}
/// Do not lint private `Error`s, i.e., ones without any `pub` (minus `pub(self)` of course) and
/// which aren't reexported
fn is_visible_outside_module(cx: &LateContext<'_>, def_id: LocalDefId) -> bool {
!matches!(
cx.tcx.visibility(def_id),
Visibility::Restricted(mod_def_id) if cx.tcx.parent_module_from_def_id(def_id).to_def_id() == mod_def_id
)
}

View file

@ -0,0 +1,99 @@
use clippy_utils::diagnostics::span_lint_and_then;
use rustc_errors::Applicability;
use rustc_hir::Item;
use rustc_lint::{LateContext, LateLintPass, LintContext};
use rustc_session::{declare_lint_pass, declare_tool_lint};
use rustc_span::Span;
declare_clippy_lint! {
/// ### What it does
/// Checks for outer doc comments written with 4 forward slashes (`////`).
///
/// ### Why is this bad?
/// This is (probably) a typo, and results in it not being a doc comment; just a regular
/// comment.
///
/// ### Example
/// ```rust
/// //// My amazing data structure
/// pub struct Foo {
/// // ...
/// }
/// ```
///
/// Use instead:
/// ```rust
/// /// My amazing data structure
/// pub struct Foo {
/// // ...
/// }
/// ```
#[clippy::version = "1.72.0"]
pub FOUR_FORWARD_SLASHES,
suspicious,
"comments with 4 forward slashes (`////`) likely intended to be doc comments (`///`)"
}
declare_lint_pass!(FourForwardSlashes => [FOUR_FORWARD_SLASHES]);
impl<'tcx> LateLintPass<'tcx> for FourForwardSlashes {
fn check_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx Item<'tcx>) {
if item.span.from_expansion() {
return;
}
let sm = cx.sess().source_map();
let mut span = cx
.tcx
.hir()
.attrs(item.hir_id())
.iter()
.fold(item.span.shrink_to_lo(), |span, attr| span.to(attr.span));
let (Some(file), _, _, end_line, _) = sm.span_to_location_info(span) else {
return;
};
let mut bad_comments = vec![];
for line in (0..end_line.saturating_sub(1)).rev() {
let Some(contents) = file.get_line(line).map(|c| c.trim().to_owned()) else {
return;
};
// Keep searching until we find the next item
if !contents.is_empty() && !contents.starts_with("//") && !contents.starts_with("#[") {
break;
}
if contents.starts_with("////") && !matches!(contents.chars().nth(4), Some('/' | '!')) {
let bounds = file.line_bounds(line);
let line_span = Span::with_root_ctxt(bounds.start, bounds.end);
span = line_span.to(span);
bad_comments.push((line_span, contents));
}
}
if !bad_comments.is_empty() {
span_lint_and_then(
cx,
FOUR_FORWARD_SLASHES,
span,
"this item has comments with 4 forward slashes (`////`). These look like doc comments, but they aren't",
|diag| {
let msg = if bad_comments.len() == 1 {
"make this a doc comment by removing one `/`"
} else {
"turn these into doc comments by removing one `/`"
};
diag.multipart_suggestion(
msg,
bad_comments
.into_iter()
// It's a little unfortunate but the span includes the `\n` yet the contents
// do not, so we must add it back. If some codebase uses `\r\n` instead they
// will need normalization but it should be fine
.map(|(span, c)| (span, c.replacen("////", "///", 1) + "\n"))
.collect(),
Applicability::MachineApplicable,
);
},
);
}
}
}

View file

@ -1,9 +1,11 @@
use clippy_utils::diagnostics::{span_lint_and_sugg, span_lint_and_then};
use clippy_utils::paths::ORD_CMP;
use clippy_utils::ty::implements_trait;
use clippy_utils::{get_parent_node, is_res_lang_ctor, last_path_segment, path_res};
use clippy_utils::{get_parent_node, is_res_lang_ctor, last_path_segment, match_def_path, path_res, std_or_core};
use rustc_errors::Applicability;
use rustc_hir::def::Res;
use rustc_hir::def_id::LocalDefId;
use rustc_hir::{Expr, ExprKind, ImplItem, ImplItemKind, ItemKind, LangItem, Node, UnOp};
use rustc_hir_analysis::hir_ty_to_ty;
use rustc_lint::{LateContext, LateLintPass};
use rustc_middle::ty::EarlyBinder;
use rustc_session::{declare_lint_pass, declare_tool_lint};
@ -59,6 +61,10 @@ declare_clippy_lint! {
/// wrapping the result of `cmp` in `Some` for `partial_cmp`. Not doing this may silently
/// introduce an error upon refactoring.
///
/// ### Known issues
/// Code that calls the `.into()` method instead will be flagged as incorrect, despite `.into()`
/// wrapping it in `Some`.
///
/// ### Limitations
/// Will not lint if `Self` and `Rhs` do not have the same type.
///
@ -120,7 +126,7 @@ impl LateLintPass<'_> for IncorrectImpls {
if cx.tcx.is_automatically_derived(item.owner_id.to_def_id()) {
return;
}
let ItemKind::Impl(_) = item.kind else {
let ItemKind::Impl(imp) = item.kind else {
return;
};
let ImplItemKind::Fn(_, impl_item_id) = cx.tcx.hir().impl_item(impl_item.impl_item_id()).kind else {
@ -185,11 +191,16 @@ impl LateLintPass<'_> for IncorrectImpls {
.get(&sym::Ord)
&& implements_trait(
cx,
trait_impl.self_ty(),
hir_ty_to_ty(cx.tcx, imp.self_ty),
*ord_def_id,
&[],
trait_impl.args,
)
{
// If the `cmp` call likely needs to be fully qualified in the suggestion
// (like `std::cmp::Ord::cmp`). It's unfortunate we must put this here but we can't
// access `cmp_expr` in the suggestion without major changes, as we lint in `else`.
let mut needs_fully_qualified = false;
if block.stmts.is_empty()
&& let Some(expr) = block.expr
&& let ExprKind::Call(
@ -201,9 +212,8 @@ impl LateLintPass<'_> for IncorrectImpls {
[cmp_expr],
) = expr.kind
&& is_res_lang_ctor(cx, cx.qpath_res(some_path, *some_hir_id), LangItem::OptionSome)
&& let ExprKind::MethodCall(cmp_path, _, [other_expr], ..) = cmp_expr.kind
&& cmp_path.ident.name == sym::cmp
&& let Res::Local(..) = path_res(cx, other_expr)
// Fix #11178, allow `Self::cmp(self, ..)` too
&& self_cmp_call(cx, cmp_expr, impl_item.owner_id.def_id, &mut needs_fully_qualified)
{} else {
// If `Self` and `Rhs` are not the same type, bail. This makes creating a valid
// suggestion tons more complex.
@ -220,14 +230,29 @@ impl LateLintPass<'_> for IncorrectImpls {
let [_, other] = body.params else {
return;
};
let Some(std_or_core) = std_or_core(cx) else {
return;
};
let suggs = if let Some(other_ident) = other.pat.simple_ident() {
vec![(block.span, format!("{{ Some(self.cmp({})) }}", other_ident.name))]
} else {
vec![
let suggs = match (other.pat.simple_ident(), needs_fully_qualified) {
(Some(other_ident), true) => vec![(
block.span,
format!("{{ Some({std_or_core}::cmp::Ord::cmp(self, {})) }}", other_ident.name),
)],
(Some(other_ident), false) => {
vec![(block.span, format!("{{ Some(self.cmp({})) }}", other_ident.name))]
},
(None, true) => vec![
(
block.span,
format!("{{ Some({std_or_core}::cmp::Ord::cmp(self, other)) }}"),
),
(other.pat.span, "other".to_owned()),
],
(None, false) => vec![
(block.span, "{ Some(self.cmp(other)) }".to_owned()),
(other.pat.span, "other".to_owned()),
]
],
};
diag.multipart_suggestion(
@ -241,3 +266,31 @@ impl LateLintPass<'_> for IncorrectImpls {
}
}
}
/// Returns whether this is any of `self.cmp(..)`, `Self::cmp(self, ..)` or `Ord::cmp(self, ..)`.
fn self_cmp_call<'tcx>(
cx: &LateContext<'tcx>,
cmp_expr: &'tcx Expr<'tcx>,
def_id: LocalDefId,
needs_fully_qualified: &mut bool,
) -> bool {
match cmp_expr.kind {
ExprKind::Call(path, [_self, _other]) => path_res(cx, path)
.opt_def_id()
.is_some_and(|def_id| match_def_path(cx, def_id, &ORD_CMP)),
ExprKind::MethodCall(_, _, [_other], ..) => {
// We can set this to true here no matter what as if it's a `MethodCall` and goes to the
// `else` branch, it must be a method named `cmp` that isn't `Ord::cmp`
*needs_fully_qualified = true;
// It's a bit annoying but `typeck_results` only gives us the CURRENT body, which we
// have none, not of any `LocalDefId` we want, so we must call the query itself to avoid
// an immediate ICE
cx.tcx
.typeck(def_id)
.type_dependent_def_id(cmp_expr.hir_id)
.is_some_and(|def_id| match_def_path(cx, def_id, &ORD_CMP))
},
_ => false,
}
}

View file

@ -1,11 +1,11 @@
use clippy_utils::diagnostics::span_lint_and_help;
use clippy_utils::ty::{implements_trait, is_type_lang_item};
use clippy_utils::{return_ty, trait_ref_of_method};
use if_chain::if_chain;
use rustc_hir::{GenericParamKind, ImplItem, ImplItemKind, LangItem};
use rustc_hir::{GenericParamKind, ImplItem, ImplItemKind, LangItem, Unsafety};
use rustc_lint::{LateContext, LateLintPass};
use rustc_session::{declare_lint_pass, declare_tool_lint};
use rustc_span::sym;
use rustc_target::spec::abi::Abi;
declare_clippy_lint! {
/// ### What it does
@ -95,24 +95,23 @@ impl<'tcx> LateLintPass<'tcx> for InherentToString {
return;
}
if_chain! {
// Check if item is a method, called to_string and has a parameter 'self'
if let ImplItemKind::Fn(ref signature, _) = impl_item.kind;
if impl_item.ident.name == sym::to_string;
let decl = &signature.decl;
if decl.implicit_self.has_implicit_self();
if decl.inputs.len() == 1;
if impl_item.generics.params.iter().all(|p| matches!(p.kind, GenericParamKind::Lifetime { .. }));
// Check if item is a method called `to_string` and has a parameter 'self'
if let ImplItemKind::Fn(ref signature, _) = impl_item.kind
// #11201
&& let header = signature.header
&& header.unsafety == Unsafety::Normal
&& header.abi == Abi::Rust
&& impl_item.ident.name == sym::to_string
&& let decl = signature.decl
&& decl.implicit_self.has_implicit_self()
&& decl.inputs.len() == 1
&& impl_item.generics.params.iter().all(|p| matches!(p.kind, GenericParamKind::Lifetime { .. }))
// Check if return type is String
if is_type_lang_item(cx, return_ty(cx, impl_item.owner_id), LangItem::String);
&& is_type_lang_item(cx, return_ty(cx, impl_item.owner_id), LangItem::String)
// Filters instances of to_string which are required by a trait
if trait_ref_of_method(cx, impl_item.owner_id.def_id).is_none();
then {
show_lint(cx, impl_item);
}
&& trait_ref_of_method(cx, impl_item.owner_id.def_id).is_none()
{
show_lint(cx, impl_item);
}
}
}

View file

@ -7,11 +7,10 @@ use rustc_ast::ast::LitKind;
use rustc_errors::Applicability;
use rustc_hir::def::Res;
use rustc_hir::def_id::{DefId, DefIdSet};
use rustc_hir::lang_items::LangItem;
use rustc_hir::{
AssocItemKind, BinOpKind, Expr, ExprKind, FnRetTy, GenericArg, GenericBound, ImplItem, ImplItemKind,
ImplicitSelfKind, Item, ItemKind, Mutability, Node, PathSegment, PrimTy, QPath, TraitItemRef, TyKind,
TypeBindingKind,
ImplicitSelfKind, Item, ItemKind, LangItem, Mutability, Node, PatKind, PathSegment, PrimTy, QPath, TraitItemRef,
TyKind, TypeBindingKind,
};
use rustc_lint::{LateContext, LateLintPass};
use rustc_middle::ty::{self, AssocKind, FnSig, Ty};
@ -171,6 +170,31 @@ impl<'tcx> LateLintPass<'tcx> for LenZero {
return;
}
if let ExprKind::Let(lt) = expr.kind
&& has_is_empty(cx, lt.init)
&& match lt.pat.kind {
PatKind::Slice([], None, []) => true,
PatKind::Lit(lit) if is_empty_string(lit) => true,
_ => false,
}
{
let mut applicability = Applicability::MachineApplicable;
let lit1 = peel_ref_operators(cx, lt.init);
let lit_str =
Sugg::hir_with_context(cx, lit1, lt.span.ctxt(), "_", &mut applicability).maybe_par();
span_lint_and_sugg(
cx,
COMPARISON_TO_EMPTY,
lt.span,
"comparison to empty slice using `if let`",
"using `is_empty` is clearer and more explicit",
format!("{lit_str}.is_empty()"),
applicability,
);
}
if let ExprKind::Binary(Spanned { node: cmp, .. }, left, right) = expr.kind {
// expr.span might contains parenthesis, see issue #10529
let actual_span = left.span.with_hi(right.span.hi());

View file

@ -65,6 +65,7 @@ mod declared_lints;
mod renamed_lints;
// begin lints modules, do not remove this comment, its used in `update_lints`
mod absolute_paths;
mod allow_attributes;
mod almost_complete_range;
mod approx_const;
@ -120,6 +121,7 @@ mod entry;
mod enum_clike;
mod enum_variants;
mod equatable_if_let;
mod error_impl_error;
mod escape;
mod eta_reduction;
mod excessive_bools;
@ -136,6 +138,7 @@ mod format_args;
mod format_impl;
mod format_push_string;
mod formatting;
mod four_forward_slashes;
mod from_over_into;
mod from_raw_with_void_ptr;
mod from_str_radix_10;
@ -272,6 +275,7 @@ mod redundant_clone;
mod redundant_closure_call;
mod redundant_else;
mod redundant_field_names;
mod redundant_locals;
mod redundant_pub_crate;
mod redundant_slicing;
mod redundant_static_lifetimes;
@ -909,7 +913,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
store.register_late_pass(move |_| Box::new(if_then_some_else_none::IfThenSomeElseNone::new(msrv())));
store.register_late_pass(|_| Box::new(bool_assert_comparison::BoolAssertComparison));
store.register_early_pass(move || Box::new(module_style::ModStyle));
store.register_late_pass(|_| Box::new(unused_async::UnusedAsync));
store.register_late_pass(|_| Box::<unused_async::UnusedAsync>::default());
let disallowed_types = conf.disallowed_types.clone();
store.register_late_pass(move |_| Box::new(disallowed_types::DisallowedTypes::new(disallowed_types.clone())));
let import_renames = conf.enforced_import_renames.clone();
@ -1078,6 +1082,17 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
store.register_early_pass(|| Box::new(visibility::Visibility));
store.register_late_pass(move |_| Box::new(tuple_array_conversions::TupleArrayConversions { msrv: msrv() }));
store.register_late_pass(|_| Box::new(manual_float_methods::ManualFloatMethods));
store.register_late_pass(|_| Box::new(four_forward_slashes::FourForwardSlashes));
store.register_late_pass(|_| Box::new(error_impl_error::ErrorImplError));
let absolute_paths_max_segments = conf.absolute_paths_max_segments;
let absolute_paths_allowed_crates = conf.absolute_paths_allowed_crates.clone();
store.register_late_pass(move |_| {
Box::new(absolute_paths::AbsolutePaths {
absolute_paths_max_segments,
absolute_paths_allowed_crates: absolute_paths_allowed_crates.clone(),
})
});
store.register_late_pass(|_| Box::new(redundant_locals::RedundantLocals));
// add lints here, do not remove this comment, it's used in `new_lint`
}

View file

@ -15,6 +15,7 @@ use rustc_hir::{
PredicateOrigin, TraitFn, TraitItem, TraitItemKind, Ty, TyKind, WherePredicate,
};
use rustc_lint::{LateContext, LateLintPass, LintContext};
use rustc_middle::hir::map::Map;
use rustc_middle::hir::nested_filter as middle_nested_filter;
use rustc_middle::lint::in_external_macro;
use rustc_session::{declare_lint_pass, declare_tool_lint};
@ -620,7 +621,7 @@ impl<'cx, 'tcx, F> Visitor<'tcx> for LifetimeChecker<'cx, 'tcx, F>
where
F: NestedFilter<'tcx>,
{
type Map = rustc_middle::hir::map::Map<'tcx>;
type Map = Map<'tcx>;
type NestedFilter = F;
// for lifetimes as parameters of generics

View file

@ -9,6 +9,7 @@ use rustc_errors::Applicability;
use rustc_hir::{is_range_literal, BorrowKind, Expr, ExprKind, Pat};
use rustc_lint::LateContext;
use rustc_span::edition::Edition;
use rustc_span::sym;
pub(super) fn check<'tcx>(
cx: &LateContext<'tcx>,
@ -51,7 +52,7 @@ pub(super) fn check<'tcx>(
},
[],
_,
) if method.ident.name.as_str() == "iter_mut" => (arg, "&mut "),
) if method.ident.name == sym::iter_mut => (arg, "&mut "),
ExprKind::MethodCall(
method,
Expr {

View file

@ -76,7 +76,7 @@ impl<'a, 'tcx> Visitor<'tcx> for IncrementVisitor<'a, 'tcx> {
ExprKind::Assign(lhs, _, _) if lhs.hir_id == expr.hir_id => {
*state = IncrementVisitorVarState::DontWarn;
},
ExprKind::AddrOf(BorrowKind::Ref, mutability, _) if mutability == Mutability::Mut => {
ExprKind::AddrOf(BorrowKind::Ref, Mutability::Mut, _) => {
*state = IncrementVisitorVarState::DontWarn;
},
_ => (),
@ -226,7 +226,7 @@ impl<'a, 'tcx> Visitor<'tcx> for InitializeVisitor<'a, 'tcx> {
InitializeVisitorState::DontWarn
}
},
ExprKind::AddrOf(BorrowKind::Ref, mutability, _) if mutability == Mutability::Mut => {
ExprKind::AddrOf(BorrowKind::Ref, Mutability::Mut, _) => {
self.state = InitializeVisitorState::DontWarn;
},
_ => (),

View file

@ -36,7 +36,8 @@ struct PathAndSpan {
span: Span,
}
/// `MacroRefData` includes the name of the macro.
/// `MacroRefData` includes the name of the macro
/// and the path from `SourceMap::span_to_filename`.
#[derive(Debug, Clone)]
pub struct MacroRefData {
name: String,

View file

@ -16,6 +16,7 @@ mod match_wild_enum;
mod match_wild_err_arm;
mod needless_match;
mod overlapping_arms;
mod redundant_guards;
mod redundant_pattern_match;
mod rest_pat_in_fully_bound_struct;
mod significant_drop_in_scrutinee;
@ -936,6 +937,36 @@ declare_clippy_lint! {
"reimplementation of `filter`"
}
declare_clippy_lint! {
/// ### What it does
/// Checks for unnecessary guards in match expressions.
///
/// ### Why is this bad?
/// It's more complex and much less readable. Making it part of the pattern can improve
/// exhaustiveness checking as well.
///
/// ### Example
/// ```rust,ignore
/// match x {
/// Some(x) if matches!(x, Some(1)) => ..,
/// Some(x) if x == Some(2) => ..,
/// _ => todo!(),
/// }
/// ```
/// Use instead:
/// ```rust,ignore
/// match x {
/// Some(Some(1)) => ..,
/// Some(Some(2)) => ..,
/// _ => todo!(),
/// }
/// ```
#[clippy::version = "1.72.0"]
pub REDUNDANT_GUARDS,
complexity,
"checks for unnecessary guards in match expressions"
}
#[derive(Default)]
pub struct Matches {
msrv: Msrv,
@ -978,6 +1009,7 @@ impl_lint_pass!(Matches => [
TRY_ERR,
MANUAL_MAP,
MANUAL_FILTER,
REDUNDANT_GUARDS,
]);
impl<'tcx> LateLintPass<'tcx> for Matches {
@ -1025,6 +1057,7 @@ impl<'tcx> LateLintPass<'tcx> for Matches {
needless_match::check_match(cx, ex, arms, expr);
match_on_vec_items::check(cx, ex);
match_str_case_mismatch::check(cx, ex, arms);
redundant_guards::check(cx, arms);
if !in_constant(cx, expr.hir_id) {
manual_unwrap_or::check(cx, expr, ex, arms);

View file

@ -0,0 +1,190 @@
use clippy_utils::diagnostics::span_lint_and_then;
use clippy_utils::path_to_local;
use clippy_utils::source::snippet_with_applicability;
use clippy_utils::visitors::{for_each_expr, is_local_used};
use rustc_errors::Applicability;
use rustc_hir::def::{DefKind, Res};
use rustc_hir::{Arm, BinOpKind, Expr, ExprKind, Guard, MatchSource, Node, Pat, PatKind};
use rustc_lint::LateContext;
use rustc_span::Span;
use std::ops::ControlFlow;
use super::REDUNDANT_GUARDS;
pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, arms: &'tcx [Arm<'tcx>]) {
for outer_arm in arms {
let Some(guard) = outer_arm.guard else {
continue;
};
// `Some(x) if matches!(x, y)`
if let Guard::If(if_expr) = guard
&& let ExprKind::Match(
scrutinee,
[
arm,
Arm {
pat: Pat {
kind: PatKind::Wild,
..
},
..
},
],
MatchSource::Normal,
) = if_expr.kind
{
emit_redundant_guards(
cx,
outer_arm,
if_expr.span,
scrutinee,
arm.pat.span,
arm.guard,
);
}
// `Some(x) if let Some(2) = x`
else if let Guard::IfLet(let_expr) = guard {
emit_redundant_guards(
cx,
outer_arm,
let_expr.span,
let_expr.init,
let_expr.pat.span,
None,
);
}
// `Some(x) if x == Some(2)`
else if let Guard::If(if_expr) = guard
&& let ExprKind::Binary(bin_op, local, pat) = if_expr.kind
&& matches!(bin_op.node, BinOpKind::Eq)
&& expr_can_be_pat(cx, pat)
// Ensure they have the same type. If they don't, we'd need deref coercion which isn't
// possible (currently) in a pattern. In some cases, you can use something like
// `as_deref` or similar but in general, we shouldn't lint this as it'd create an
// extraordinary amount of FPs.
//
// This isn't necessary in the other two checks, as they must be a pattern already.
&& cx.typeck_results().expr_ty(local) == cx.typeck_results().expr_ty(pat)
{
emit_redundant_guards(
cx,
outer_arm,
if_expr.span,
local,
pat.span,
None,
);
}
}
}
fn get_pat_binding<'tcx>(cx: &LateContext<'tcx>, guard_expr: &Expr<'_>, outer_arm: &Arm<'tcx>) -> Option<(Span, bool)> {
if let Some(local) = path_to_local(guard_expr) && !is_local_used(cx, outer_arm.body, local) {
let mut span = None;
let mut multiple_bindings = false;
// `each_binding` gives the `HirId` of the `Pat` itself, not the binding
outer_arm.pat.walk(|pat| {
if let PatKind::Binding(_, hir_id, _, _) = pat.kind
&& hir_id == local
&& span.replace(pat.span).is_some()
{
multiple_bindings = true;
return false;
}
true
});
// Ignore bindings from or patterns, like `First(x) | Second(x, _) | Third(x, _, _)`
if !multiple_bindings {
return span.map(|span| {
(
span,
!matches!(cx.tcx.hir().get_parent(local), Node::PatField(_)),
)
});
}
}
None
}
fn emit_redundant_guards<'tcx>(
cx: &LateContext<'tcx>,
outer_arm: &Arm<'tcx>,
guard_span: Span,
local: &Expr<'_>,
pat_span: Span,
inner_guard: Option<Guard<'_>>,
) {
let mut app = Applicability::MaybeIncorrect;
let Some((pat_binding, can_use_shorthand)) = get_pat_binding(cx, local, outer_arm) else {
return;
};
span_lint_and_then(
cx,
REDUNDANT_GUARDS,
guard_span.source_callsite(),
"redundant guard",
|diag| {
let binding_replacement = snippet_with_applicability(cx, pat_span, "<binding_repl>", &mut app);
diag.multipart_suggestion_verbose(
"try",
vec![
if can_use_shorthand {
(pat_binding, binding_replacement.into_owned())
} else {
(pat_binding.shrink_to_hi(), format!(": {binding_replacement}"))
},
(
guard_span.source_callsite().with_lo(outer_arm.pat.span.hi()),
inner_guard.map_or_else(String::new, |guard| {
let (prefix, span) = match guard {
Guard::If(e) => ("if", e.span),
Guard::IfLet(l) => ("if let", l.span),
};
format!(
" {prefix} {}",
snippet_with_applicability(cx, span, "<guard>", &mut app),
)
}),
),
],
app,
);
},
);
}
/// Checks if the given `Expr` can also be represented as a `Pat`.
fn expr_can_be_pat(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool {
for_each_expr(expr, |expr| {
if match expr.kind {
ExprKind::ConstBlock(..) => cx.tcx.features().inline_const_pat,
ExprKind::Call(c, ..) if let ExprKind::Path(qpath) = c.kind => {
// Allow ctors
matches!(cx.qpath_res(&qpath, c.hir_id), Res::Def(DefKind::Ctor(..), ..))
},
ExprKind::Path(qpath) => {
matches!(
cx.qpath_res(&qpath, expr.hir_id),
Res::Def(DefKind::Struct | DefKind::Enum | DefKind::Ctor(..), ..),
)
},
ExprKind::AddrOf(..)
| ExprKind::Array(..)
| ExprKind::Tup(..)
| ExprKind::Struct(..)
| ExprKind::Lit(..) => true,
_ => false,
} {
return ControlFlow::Continue(());
}
ControlFlow::Break(())
})
.is_none()
}

View file

@ -3,17 +3,19 @@ use clippy_utils::diagnostics::{span_lint_and_sugg, span_lint_and_then};
use clippy_utils::source::{snippet, walk_span_to_context};
use clippy_utils::sugg::Sugg;
use clippy_utils::ty::{is_type_diagnostic_item, needs_ordered_drop};
use clippy_utils::visitors::any_temporaries_need_ordered_drop;
use clippy_utils::visitors::{any_temporaries_need_ordered_drop, for_each_expr};
use clippy_utils::{higher, is_expn_of, is_trait_method};
use if_chain::if_chain;
use rustc_ast::ast::LitKind;
use rustc_errors::Applicability;
use rustc_hir::def::{DefKind, Res};
use rustc_hir::LangItem::{self, OptionNone, OptionSome, PollPending, PollReady, ResultErr, ResultOk};
use rustc_hir::{Arm, Expr, ExprKind, Node, Pat, PatKind, QPath, UnOp};
use rustc_hir::{Arm, Expr, ExprKind, Guard, Node, Pat, PatKind, QPath, UnOp};
use rustc_lint::LateContext;
use rustc_middle::ty::{self, GenericArgKind, Ty};
use rustc_span::{sym, Symbol};
use std::fmt::Write;
use std::ops::ControlFlow;
pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
if let Some(higher::WhileLet { let_pat, let_expr, .. }) = higher::WhileLet::hir(expr) {
@ -201,30 +203,58 @@ pub(super) fn check_match<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>, op
if arms.len() == 2 {
let node_pair = (&arms[0].pat.kind, &arms[1].pat.kind);
if let Some(good_method) = found_good_method(cx, arms, node_pair) {
if let Some((good_method, maybe_guard)) = found_good_method(cx, arms, node_pair) {
let span = is_expn_of(expr.span, "matches").unwrap_or(expr.span.to(op.span));
let result_expr = match &op.kind {
ExprKind::AddrOf(_, _, borrowed) => borrowed,
_ => op,
};
let mut sugg = format!("{}.{good_method}", snippet(cx, result_expr.span, "_"));
if let Some(guard) = maybe_guard {
let Guard::If(guard) = *guard else { return }; // `...is_none() && let ...` is a syntax error
// wow, the HIR for match guards in `PAT if let PAT = expr && expr => ...` is annoying!
// `guard` here is `Guard::If` with the let expression somewhere deep in the tree of exprs,
// counter to the intuition that it should be `Guard::IfLet`, so we need another check
// to see that there aren't any let chains anywhere in the guard, as that would break
// if we suggest `t.is_none() && (let X = y && z)` for:
// `match t { None if let X = y && z => true, _ => false }`
let has_nested_let_chain = for_each_expr(guard, |expr| {
if matches!(expr.kind, ExprKind::Let(..)) {
ControlFlow::Break(())
} else {
ControlFlow::Continue(())
}
})
.is_some();
if has_nested_let_chain {
return;
}
let guard = Sugg::hir(cx, guard, "..");
let _ = write!(sugg, " && {}", guard.maybe_par());
}
span_lint_and_sugg(
cx,
REDUNDANT_PATTERN_MATCHING,
span,
&format!("redundant pattern matching, consider using `{good_method}`"),
"try",
format!("{}.{good_method}", snippet(cx, result_expr.span, "_")),
sugg,
Applicability::MachineApplicable,
);
}
}
}
fn found_good_method<'a>(
fn found_good_method<'tcx>(
cx: &LateContext<'_>,
arms: &[Arm<'_>],
arms: &'tcx [Arm<'tcx>],
node: (&PatKind<'_>, &PatKind<'_>),
) -> Option<&'a str> {
) -> Option<(&'static str, Option<&'tcx Guard<'tcx>>)> {
match node {
(
PatKind::TupleStruct(ref path_left, patterns_left, _),
@ -310,7 +340,11 @@ fn get_ident(path: &QPath<'_>) -> Option<rustc_span::symbol::Ident> {
}
}
fn get_good_method<'a>(cx: &LateContext<'_>, arms: &[Arm<'_>], path_left: &QPath<'_>) -> Option<&'a str> {
fn get_good_method<'tcx>(
cx: &LateContext<'_>,
arms: &'tcx [Arm<'tcx>],
path_left: &QPath<'_>,
) -> Option<(&'static str, Option<&'tcx Guard<'tcx>>)> {
if let Some(name) = get_ident(path_left) {
return match name.as_str() {
"Ok" => {
@ -376,16 +410,16 @@ fn is_pat_variant(cx: &LateContext<'_>, pat: &Pat<'_>, path: &QPath<'_>, expecte
}
#[expect(clippy::too_many_arguments)]
fn find_good_method_for_match<'a>(
fn find_good_method_for_match<'a, 'tcx>(
cx: &LateContext<'_>,
arms: &[Arm<'_>],
arms: &'tcx [Arm<'tcx>],
path_left: &QPath<'_>,
path_right: &QPath<'_>,
expected_item_left: Item,
expected_item_right: Item,
should_be_left: &'a str,
should_be_right: &'a str,
) -> Option<&'a str> {
) -> Option<(&'a str, Option<&'tcx Guard<'tcx>>)> {
let first_pat = arms[0].pat;
let second_pat = arms[1].pat;
@ -403,22 +437,22 @@ fn find_good_method_for_match<'a>(
match body_node_pair {
(ExprKind::Lit(lit_left), ExprKind::Lit(lit_right)) => match (&lit_left.node, &lit_right.node) {
(LitKind::Bool(true), LitKind::Bool(false)) => Some(should_be_left),
(LitKind::Bool(false), LitKind::Bool(true)) => Some(should_be_right),
(LitKind::Bool(true), LitKind::Bool(false)) => Some((should_be_left, arms[0].guard.as_ref())),
(LitKind::Bool(false), LitKind::Bool(true)) => Some((should_be_right, arms[1].guard.as_ref())),
_ => None,
},
_ => None,
}
}
fn find_good_method_for_matches_macro<'a>(
fn find_good_method_for_matches_macro<'a, 'tcx>(
cx: &LateContext<'_>,
arms: &[Arm<'_>],
arms: &'tcx [Arm<'tcx>],
path_left: &QPath<'_>,
expected_item_left: Item,
should_be_left: &'a str,
should_be_right: &'a str,
) -> Option<&'a str> {
) -> Option<(&'a str, Option<&'tcx Guard<'tcx>>)> {
let first_pat = arms[0].pat;
let body_node_pair = if is_pat_variant(cx, first_pat, path_left, expected_item_left) {
@ -429,8 +463,8 @@ fn find_good_method_for_matches_macro<'a>(
match body_node_pair {
(ExprKind::Lit(lit_left), ExprKind::Lit(lit_right)) => match (&lit_left.node, &lit_right.node) {
(LitKind::Bool(true), LitKind::Bool(false)) => Some(should_be_left),
(LitKind::Bool(false), LitKind::Bool(true)) => Some(should_be_right),
(LitKind::Bool(true), LitKind::Bool(false)) => Some((should_be_left, arms[0].guard.as_ref())),
(LitKind::Bool(false), LitKind::Bool(true)) => Some((should_be_right, arms[1].guard.as_ref())),
_ => None,
},
_ => None,

View file

@ -1,7 +1,9 @@
use clippy_utils::diagnostics::span_lint_and_sugg;
use clippy_utils::diagnostics::{span_lint_and_sugg, span_lint_and_then};
use clippy_utils::macros::{is_panic, root_macro_call};
use clippy_utils::source::{indent_of, reindent_multiline, snippet};
use clippy_utils::ty::is_type_diagnostic_item;
use clippy_utils::{is_trait_method, path_to_local_id, peel_blocks, SpanlessEq};
use clippy_utils::{higher, is_trait_method, path_to_local_id, peel_blocks, SpanlessEq};
use hir::{Body, HirId, MatchSource, Pat};
use if_chain::if_chain;
use rustc_errors::Applicability;
use rustc_hir as hir;
@ -10,7 +12,7 @@ use rustc_hir::{Closure, Expr, ExprKind, PatKind, PathSegment, QPath, UnOp};
use rustc_lint::LateContext;
use rustc_middle::ty::adjustment::Adjust;
use rustc_span::source_map::Span;
use rustc_span::symbol::{sym, Symbol};
use rustc_span::symbol::{sym, Ident, Symbol};
use std::borrow::Cow;
use super::{MANUAL_FILTER_MAP, MANUAL_FIND_MAP, OPTION_FILTER_MAP};
@ -48,6 +50,214 @@ fn is_option_filter_map(cx: &LateContext<'_>, filter_arg: &hir::Expr<'_>, map_ar
is_method(cx, map_arg, sym::unwrap) && is_method(cx, filter_arg, sym!(is_some))
}
#[derive(Debug, Copy, Clone)]
enum OffendingFilterExpr<'tcx> {
/// `.filter(|opt| opt.is_some())`
IsSome {
/// The receiver expression
receiver: &'tcx Expr<'tcx>,
/// If `Some`, then this contains the span of an expression that possibly contains side
/// effects: `.filter(|opt| side_effect(opt).is_some())`
/// ^^^^^^^^^^^^^^^^
///
/// We will use this later for warning the user that the suggested fix may change
/// the behavior.
side_effect_expr_span: Option<Span>,
},
/// `.filter(|res| res.is_ok())`
IsOk {
/// The receiver expression
receiver: &'tcx Expr<'tcx>,
/// See `IsSome`
side_effect_expr_span: Option<Span>,
},
/// `.filter(|enum| matches!(enum, Enum::A(_)))`
Matches {
/// The DefId of the variant being matched
variant_def_id: hir::def_id::DefId,
},
}
#[derive(Debug)]
enum CalledMethod {
OptionIsSome,
ResultIsOk,
}
/// The result of checking a `map` call, returned by `OffendingFilterExpr::check_map_call`
#[derive(Debug)]
enum CheckResult<'tcx> {
Method {
map_arg: &'tcx Expr<'tcx>,
/// The method that was called inside of `filter`
method: CalledMethod,
/// See `OffendingFilterExpr::IsSome`
side_effect_expr_span: Option<Span>,
},
PatternMatching {
/// The span of the variant being matched
/// if let Some(s) = enum
/// ^^^^^^^
variant_span: Span,
/// if let Some(s) = enum
/// ^
variant_ident: Ident,
},
}
impl<'tcx> OffendingFilterExpr<'tcx> {
pub fn check_map_call(
&mut self,
cx: &LateContext<'tcx>,
map_body: &'tcx Body<'tcx>,
map_param_id: HirId,
filter_param_id: HirId,
is_filter_param_ref: bool,
) -> Option<CheckResult<'tcx>> {
match *self {
OffendingFilterExpr::IsSome {
receiver,
side_effect_expr_span,
}
| OffendingFilterExpr::IsOk {
receiver,
side_effect_expr_span,
} => {
// check if closure ends with expect() or unwrap()
if let ExprKind::MethodCall(seg, map_arg, ..) = map_body.value.kind
&& matches!(seg.ident.name, sym::expect | sym::unwrap | sym::unwrap_or)
// .map(|y| f(y).copied().unwrap())
// ~~~~
&& let map_arg_peeled = match map_arg.kind {
ExprKind::MethodCall(method, original_arg, [], _) if acceptable_methods(method) => {
original_arg
},
_ => map_arg,
}
// .map(|y| y[.acceptable_method()].unwrap())
&& let simple_equal = (path_to_local_id(receiver, filter_param_id)
&& path_to_local_id(map_arg_peeled, map_param_id))
&& let eq_fallback = (|a: &Expr<'_>, b: &Expr<'_>| {
// in `filter(|x| ..)`, replace `*x` with `x`
let a_path = if_chain! {
if !is_filter_param_ref;
if let ExprKind::Unary(UnOp::Deref, expr_path) = a.kind;
then { expr_path } else { a }
};
// let the filter closure arg and the map closure arg be equal
path_to_local_id(a_path, filter_param_id)
&& path_to_local_id(b, map_param_id)
&& cx.typeck_results().expr_ty_adjusted(a) == cx.typeck_results().expr_ty_adjusted(b)
})
&& (simple_equal
|| SpanlessEq::new(cx).expr_fallback(eq_fallback).eq_expr(receiver, map_arg_peeled))
{
Some(CheckResult::Method {
map_arg,
side_effect_expr_span,
method: match self {
OffendingFilterExpr::IsSome { .. } => CalledMethod::OptionIsSome,
OffendingFilterExpr::IsOk { .. } => CalledMethod::ResultIsOk,
OffendingFilterExpr::Matches { .. } => unreachable!("only IsSome and IsOk can get here"),
}
})
} else {
None
}
},
OffendingFilterExpr::Matches { variant_def_id } => {
let expr_uses_local = |pat: &Pat<'_>, expr: &Expr<'_>| {
if let PatKind::TupleStruct(QPath::Resolved(_, path), [subpat], _) = pat.kind
&& let PatKind::Binding(_, local_id, ident, _) = subpat.kind
&& path_to_local_id(expr.peel_blocks(), local_id)
&& let Some(local_variant_def_id) = path.res.opt_def_id()
&& local_variant_def_id == variant_def_id
{
Some((ident, pat.span))
} else {
None
}
};
// look for:
// `if let Variant (v) = enum { v } else { unreachable!() }`
// ^^^^^^^ ^ ^^^^ ^^^^^^^^^^^^^^^^^^
// variant_span variant_ident scrutinee else_ (blocks peeled later)
// OR
// `match enum { Variant (v) => v, _ => unreachable!() }`
// ^^^^ ^^^^^^^ ^ ^^^^^^^^^^^^^^
// scrutinee variant_span variant_ident else_
let (scrutinee, else_, variant_ident, variant_span) =
match higher::IfLetOrMatch::parse(cx, map_body.value) {
// For `if let` we want to check that the variant matching arm references the local created by its pattern
Some(higher::IfLetOrMatch::IfLet(sc, pat, then, Some(else_)))
if let Some((ident, span)) = expr_uses_local(pat, then) =>
{
(sc, else_, ident, span)
},
// For `match` we want to check that the "else" arm is the wildcard (`_`) pattern
// and that the variant matching arm references the local created by its pattern
Some(higher::IfLetOrMatch::Match(sc, [arm, wild_arm], MatchSource::Normal))
if let PatKind::Wild = wild_arm.pat.kind
&& let Some((ident, span)) = expr_uses_local(arm.pat, arm.body.peel_blocks()) =>
{
(sc, wild_arm.body, ident, span)
},
_ => return None,
};
if path_to_local_id(scrutinee, map_param_id)
// else branch should be a `panic!` or `unreachable!` macro call
&& let Some(mac) = root_macro_call(else_.peel_blocks().span)
&& (is_panic(cx, mac.def_id) || cx.tcx.opt_item_name(mac.def_id) == Some(sym::unreachable))
{
Some(CheckResult::PatternMatching { variant_span, variant_ident })
} else {
None
}
},
}
}
fn hir(cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>, filter_param_id: HirId) -> Option<Self> {
if let ExprKind::MethodCall(path, receiver, [], _) = expr.kind
&& let Some(recv_ty) = cx.typeck_results().expr_ty(receiver).peel_refs().ty_adt_def()
{
// we still want to lint if the expression possibly contains side effects,
// *but* it can't be machine-applicable then, because that can change the behavior of the program:
// .filter(|x| effect(x).is_some()).map(|x| effect(x).unwrap())
// vs.
// .filter_map(|x| effect(x))
//
// the latter only calls `effect` once
let side_effect_expr_span = receiver.can_have_side_effects().then_some(receiver.span);
if cx.tcx.is_diagnostic_item(sym::Option, recv_ty.did())
&& path.ident.name == sym!(is_some)
{
Some(Self::IsSome { receiver, side_effect_expr_span })
} else if cx.tcx.is_diagnostic_item(sym::Result, recv_ty.did())
&& path.ident.name == sym!(is_ok)
{
Some(Self::IsOk { receiver, side_effect_expr_span })
} else {
None
}
} else if let Some(macro_call) = root_macro_call(expr.span)
&& cx.tcx.get_diagnostic_name(macro_call.def_id) == Some(sym::matches_macro)
// we know for a fact that the wildcard pattern is the second arm
&& let ExprKind::Match(scrutinee, [arm, _], _) = expr.kind
&& path_to_local_id(scrutinee, filter_param_id)
&& let PatKind::TupleStruct(QPath::Resolved(_, path), ..) = arm.pat.kind
&& let Some(variant_def_id) = path.res.opt_def_id()
{
Some(OffendingFilterExpr::Matches { variant_def_id })
} else {
None
}
}
}
/// is `filter(|x| x.is_some()).map(|x| x.unwrap())`
fn is_filter_some_map_unwrap(
cx: &LateContext<'_>,
@ -102,55 +312,18 @@ pub(super) fn check(
} else {
(filter_param.pat, false)
};
// closure ends with is_some() or is_ok()
if let PatKind::Binding(_, filter_param_id, _, None) = filter_pat.kind;
if let ExprKind::MethodCall(path, filter_arg, [], _) = filter_body.value.kind;
if let Some(opt_ty) = cx.typeck_results().expr_ty(filter_arg).peel_refs().ty_adt_def();
if let Some(is_result) = if cx.tcx.is_diagnostic_item(sym::Option, opt_ty.did()) {
Some(false)
} else if cx.tcx.is_diagnostic_item(sym::Result, opt_ty.did()) {
Some(true)
} else {
None
};
if path.ident.name.as_str() == if is_result { "is_ok" } else { "is_some" };
// ...map(|x| ...unwrap())
if let PatKind::Binding(_, filter_param_id, _, None) = filter_pat.kind;
if let Some(mut offending_expr) = OffendingFilterExpr::hir(cx, filter_body.value, filter_param_id);
if let ExprKind::Closure(&Closure { body: map_body_id, .. }) = map_arg.kind;
let map_body = cx.tcx.hir().body(map_body_id);
if let [map_param] = map_body.params;
if let PatKind::Binding(_, map_param_id, map_param_ident, None) = map_param.pat.kind;
// closure ends with expect() or unwrap()
if let ExprKind::MethodCall(seg, map_arg, ..) = map_body.value.kind;
if matches!(seg.ident.name, sym::expect | sym::unwrap | sym::unwrap_or);
// .filter(..).map(|y| f(y).copied().unwrap())
// ~~~~
let map_arg_peeled = match map_arg.kind {
ExprKind::MethodCall(method, original_arg, [], _) if acceptable_methods(method) => {
original_arg
},
_ => map_arg,
};
if let Some(check_result) =
offending_expr.check_map_call(cx, map_body, map_param_id, filter_param_id, is_filter_param_ref);
// .filter(|x| x.is_some()).map(|y| y[.acceptable_method()].unwrap())
let simple_equal = path_to_local_id(filter_arg, filter_param_id)
&& path_to_local_id(map_arg_peeled, map_param_id);
let eq_fallback = |a: &Expr<'_>, b: &Expr<'_>| {
// in `filter(|x| ..)`, replace `*x` with `x`
let a_path = if_chain! {
if !is_filter_param_ref;
if let ExprKind::Unary(UnOp::Deref, expr_path) = a.kind;
then { expr_path } else { a }
};
// let the filter closure arg and the map closure arg be equal
path_to_local_id(a_path, filter_param_id)
&& path_to_local_id(b, map_param_id)
&& cx.typeck_results().expr_ty_adjusted(a) == cx.typeck_results().expr_ty_adjusted(b)
};
if simple_equal || SpanlessEq::new(cx).expr_fallback(eq_fallback).eq_expr(filter_arg, map_arg_peeled);
then {
let span = filter_span.with_hi(expr.span.hi());
let (filter_name, lint) = if is_find {
@ -159,22 +332,53 @@ pub(super) fn check(
("filter", MANUAL_FILTER_MAP)
};
let msg = format!("`{filter_name}(..).map(..)` can be simplified as `{filter_name}_map(..)`");
let (to_opt, deref) = if is_result {
(".ok()", String::new())
} else {
let derefs = cx.typeck_results()
.expr_adjustments(map_arg)
.iter()
.filter(|adj| matches!(adj.kind, Adjust::Deref(_)))
.count();
("", "*".repeat(derefs))
let (sugg, note_and_span, applicability) = match check_result {
CheckResult::Method { map_arg, method, side_effect_expr_span } => {
let (to_opt, deref) = match method {
CalledMethod::ResultIsOk => (".ok()", String::new()),
CalledMethod::OptionIsSome => {
let derefs = cx.typeck_results()
.expr_adjustments(map_arg)
.iter()
.filter(|adj| matches!(adj.kind, Adjust::Deref(_)))
.count();
("", "*".repeat(derefs))
}
};
let sugg = format!(
"{filter_name}_map(|{map_param_ident}| {deref}{}{to_opt})",
snippet(cx, map_arg.span, ".."),
);
let (note_and_span, applicability) = if let Some(span) = side_effect_expr_span {
let note = "the suggestion might change the behavior of the program when merging `filter` and `map`, \
because this expression potentially contains side effects and will only execute once";
(Some((note, span)), Applicability::MaybeIncorrect)
} else {
(None, Applicability::MachineApplicable)
};
(sugg, note_and_span, applicability)
}
CheckResult::PatternMatching { variant_span, variant_ident } => {
let pat = snippet(cx, variant_span, "<pattern>");
(format!("{filter_name}_map(|{map_param_ident}| match {map_param_ident} {{ \
{pat} => Some({variant_ident}), \
_ => None \
}})"), None, Applicability::MachineApplicable)
}
};
let sugg = format!(
"{filter_name}_map(|{map_param_ident}| {deref}{}{to_opt})",
snippet(cx, map_arg.span, ".."),
);
span_lint_and_sugg(cx, lint, span, &msg, "try", sugg, Applicability::MachineApplicable);
span_lint_and_then(cx, lint, span, &msg, |diag| {
diag.span_suggestion(span, "try", sugg, applicability);
if let Some((note, span)) = note_and_span {
diag.span_note(span, note);
}
});
}
}
}

View file

@ -0,0 +1,45 @@
use clippy_utils::diagnostics::span_lint_and_sugg;
use clippy_utils::paths::BOOL_THEN;
use clippy_utils::source::snippet_opt;
use clippy_utils::ty::is_copy;
use clippy_utils::{is_from_proc_macro, is_trait_method, match_def_path, peel_blocks};
use rustc_errors::Applicability;
use rustc_hir::{Expr, ExprKind};
use rustc_lint::{LateContext, LintContext};
use rustc_middle::lint::in_external_macro;
use rustc_span::{sym, Span};
use super::FILTER_MAP_BOOL_THEN;
pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>, arg: &Expr<'_>, call_span: Span) {
if !in_external_macro(cx.sess(), expr.span)
&& is_trait_method(cx, expr, sym::Iterator)
&& let ExprKind::Closure(closure) = arg.kind
&& let body = cx.tcx.hir().body(closure.body)
&& let value = peel_blocks(body.value)
// Indexing should be fine as `filter_map` always has 1 input, we unfortunately need both
// `inputs` and `params` here as we need both the type and the span
&& let param_ty = closure.fn_decl.inputs[0]
&& let param = body.params[0]
&& is_copy(cx, cx.typeck_results().node_type(param_ty.hir_id).peel_refs())
&& let ExprKind::MethodCall(_, recv, [then_arg], _) = value.kind
&& let ExprKind::Closure(then_closure) = then_arg.kind
&& let then_body = peel_blocks(cx.tcx.hir().body(then_closure.body).value)
&& let Some(def_id) = cx.typeck_results().type_dependent_def_id(value.hir_id)
&& match_def_path(cx, def_id, &BOOL_THEN)
&& !is_from_proc_macro(cx, expr)
&& let Some(param_snippet) = snippet_opt(cx, param.span)
&& let Some(filter) = snippet_opt(cx, recv.span)
&& let Some(map) = snippet_opt(cx, then_body.span)
{
span_lint_and_sugg(
cx,
FILTER_MAP_BOOL_THEN,
call_span,
"usage of `bool::then` in `filter_map`",
"use `filter` then `map` instead",
format!("filter(|&{param_snippet}| {filter}).map(|{param_snippet}| {map})"),
Applicability::MachineApplicable,
);
}
}

View file

@ -0,0 +1,33 @@
use super::FORMAT_COLLECT;
use clippy_utils::diagnostics::span_lint_and_then;
use clippy_utils::macros::{is_format_macro, root_macro_call_first_node};
use clippy_utils::ty::is_type_lang_item;
use rustc_hir::{Expr, ExprKind, LangItem};
use rustc_lint::LateContext;
use rustc_span::Span;
/// Same as `peel_blocks` but only actually considers blocks that are not from an expansion.
/// This is needed because always calling `peel_blocks` would otherwise remove parts of the
/// `format!` macro, which would cause `root_macro_call_first_node` to return `None`.
fn peel_non_expn_blocks<'tcx>(expr: &'tcx Expr<'tcx>) -> Option<&'tcx Expr<'tcx>> {
match expr.kind {
ExprKind::Block(block, _) if !expr.span.from_expansion() => peel_non_expn_blocks(block.expr?),
_ => Some(expr),
}
}
pub(super) fn check(cx: &LateContext<'_>, expr: &Expr<'_>, map_arg: &Expr<'_>, map_span: Span) {
if is_type_lang_item(cx, cx.typeck_results().expr_ty(expr), LangItem::String)
&& let ExprKind::Closure(closure) = map_arg.kind
&& let body = cx.tcx.hir().body(closure.body)
&& let Some(value) = peel_non_expn_blocks(body.value)
&& let Some(mac) = root_macro_call_first_node(cx, value)
&& is_format_macro(cx, mac.def_id)
{
span_lint_and_then(cx, FORMAT_COLLECT, expr.span, "use of `format!` to build up a string from an iterator", |diag| {
diag.span_help(map_span, "call `fold` instead")
.span_help(value.span.source_callsite(), "... and use the `write!` macro here")
.note("this can be written more efficiently by appending to a `String` directly");
});
}
}

View file

@ -0,0 +1,34 @@
use clippy_utils::consts::{constant, Constant};
use clippy_utils::diagnostics::span_lint_and_then;
use clippy_utils::{is_from_proc_macro, is_trait_method};
use rustc_errors::Applicability;
use rustc_hir::Expr;
use rustc_lint::LateContext;
use rustc_span::sym;
use super::ITER_SKIP_ZERO;
pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>, arg_expr: &Expr<'_>) {
if !expr.span.from_expansion()
&& is_trait_method(cx, expr, sym::Iterator)
&& let Some(arg) = constant(cx, cx.typeck_results(), arg_expr).and_then(|constant| {
if let Constant::Int(arg) = constant {
Some(arg)
} else {
None
}
})
&& arg == 0
&& !is_from_proc_macro(cx, expr)
{
span_lint_and_then(cx, ITER_SKIP_ZERO, arg_expr.span, "usage of `.skip(0)`", |diag| {
diag.span_suggestion(
arg_expr.span,
"if you meant to skip the first element, use",
"1",
Applicability::MaybeIncorrect,
)
.note("this call to `skip` does nothing and is useless; remove it");
});
}
}

View file

@ -21,11 +21,13 @@ mod expect_used;
mod extend_with_drain;
mod filetype_is_file;
mod filter_map;
mod filter_map_bool_then;
mod filter_map_identity;
mod filter_map_next;
mod filter_next;
mod flat_map_identity;
mod flat_map_option;
mod format_collect;
mod from_iter_instead_of_collect;
mod get_first;
mod get_last_with_len;
@ -44,6 +46,7 @@ mod iter_nth_zero;
mod iter_on_single_or_empty_collections;
mod iter_overeager_cloned;
mod iter_skip_next;
mod iter_skip_zero;
mod iter_with_drain;
mod iterator_step_by_zero;
mod manual_next_back;
@ -73,6 +76,7 @@ mod or_then_unwrap;
mod path_buf_push_overwrite;
mod range_zip_with_len;
mod read_line_without_trim;
mod readonly_write_lock;
mod repeat_once;
mod search_is_some;
mod seek_from_current;
@ -85,6 +89,7 @@ mod skip_while_next;
mod stable_sort_primitive;
mod str_splitn;
mod string_extend_chars;
mod string_lit_chars_any;
mod suspicious_command_arg_space;
mod suspicious_map;
mod suspicious_splitn;
@ -100,7 +105,6 @@ mod unnecessary_lazy_eval;
mod unnecessary_literal_unwrap;
mod unnecessary_sort_by;
mod unnecessary_to_owned;
mod unwrap_or_else_default;
mod unwrap_used;
mod useless_asref;
mod utils;
@ -114,7 +118,7 @@ use clippy_utils::consts::{constant, Constant};
use clippy_utils::diagnostics::{span_lint, span_lint_and_help};
use clippy_utils::msrvs::{self, Msrv};
use clippy_utils::ty::{contains_ty_adt_constructor_opaque, implements_trait, is_copy, is_type_diagnostic_item};
use clippy_utils::{contains_return, is_bool, is_trait_method, iter_input_pats, return_ty};
use clippy_utils::{contains_return, is_bool, is_trait_method, iter_input_pats, peel_blocks, return_ty};
use if_chain::if_chain;
use rustc_hir as hir;
use rustc_hir::{Expr, ExprKind, Node, Stmt, StmtKind, TraitItem, TraitItemKind};
@ -473,29 +477,40 @@ declare_clippy_lint! {
declare_clippy_lint! {
/// ### What it does
/// Checks for usage of `_.unwrap_or_else(Default::default)` on `Option` and
/// `Result` values.
/// Checks for usages of the following functions with an argument that constructs a default value
/// (e.g., `Default::default` or `String::new`):
/// - `unwrap_or`
/// - `unwrap_or_else`
/// - `or_insert`
/// - `or_insert_with`
///
/// ### Why is this bad?
/// Readability, these can be written as `_.unwrap_or_default`, which is
/// simpler and more concise.
/// Readability. Using `unwrap_or_default` in place of `unwrap_or`/`unwrap_or_else`, or `or_default`
/// in place of `or_insert`/`or_insert_with`, is simpler and more concise.
///
/// ### Known problems
/// In some cases, the argument of `unwrap_or`, etc. is needed for type inference. The lint uses a
/// heuristic to try to identify such cases. However, the heuristic can produce false negatives.
///
/// ### Examples
/// ```rust
/// # let x = Some(1);
/// x.unwrap_or_else(Default::default);
/// x.unwrap_or_else(u32::default);
/// # let mut map = std::collections::HashMap::<u64, String>::new();
/// x.unwrap_or(Default::default());
/// map.entry(42).or_insert_with(String::new);
/// ```
///
/// Use instead:
/// ```rust
/// # let x = Some(1);
/// # let mut map = std::collections::HashMap::<u64, String>::new();
/// x.unwrap_or_default();
/// map.entry(42).or_default();
/// ```
#[clippy::version = "1.56.0"]
pub UNWRAP_OR_ELSE_DEFAULT,
pub UNWRAP_OR_DEFAULT,
style,
"using `.unwrap_or_else(Default::default)`, which is more succinctly expressed as `.unwrap_or_default()`"
"using `.unwrap_or`, etc. with an argument that constructs a default value"
}
declare_clippy_lint! {
@ -3378,6 +3393,152 @@ declare_clippy_lint! {
"calling `Stdin::read_line`, then trying to parse it without first trimming"
}
declare_clippy_lint! {
/// ### What it does
/// Checks for `<string_lit>.chars().any(|i| i == c)`.
///
/// ### Why is this bad?
/// It's significantly slower than using a pattern instead, like
/// `matches!(c, '\\' | '.' | '+')`.
///
/// Despite this being faster, this is not `perf` as this is pretty common, and is a rather nice
/// way to check if a `char` is any in a set. In any case, this `restriction` lint is available
/// for situations where that additional performance is absolutely necessary.
///
/// ### Example
/// ```rust
/// # let c = 'c';
/// "\\.+*?()|[]{}^$#&-~".chars().any(|x| x == c);
/// ```
/// Use instead:
/// ```rust
/// # let c = 'c';
/// matches!(c, '\\' | '.' | '+' | '*' | '(' | ')' | '|' | '[' | ']' | '{' | '}' | '^' | '$' | '#' | '&' | '-' | '~');
/// ```
#[clippy::version = "1.72.0"]
pub STRING_LIT_CHARS_ANY,
restriction,
"checks for `<string_lit>.chars().any(|i| i == c)`"
}
declare_clippy_lint! {
/// ### What it does
/// Checks for usage of `.map(|_| format!(..)).collect::<String>()`.
///
/// ### Why is this bad?
/// This allocates a new string for every element in the iterator.
/// This can be done more efficiently by creating the `String` once and appending to it in `Iterator::fold`,
/// using either the `write!` macro which supports exactly the same syntax as the `format!` macro,
/// or concatenating with `+` in case the iterator yields `&str`/`String`.
///
/// Note also that `write!`-ing into a `String` can never fail, despite the return type of `write!` being `std::fmt::Result`,
/// so it can be safely ignored or unwrapped.
///
/// ### Example
/// ```rust
/// fn hex_encode(bytes: &[u8]) -> String {
/// bytes.iter().map(|b| format!("{b:02X}")).collect()
/// }
/// ```
/// Use instead:
/// ```rust
/// use std::fmt::Write;
/// fn hex_encode(bytes: &[u8]) -> String {
/// bytes.iter().fold(String::new(), |mut output, b| {
/// let _ = write!(output, "{b:02X}");
/// output
/// })
/// }
/// ```
#[clippy::version = "1.72.0"]
pub FORMAT_COLLECT,
perf,
"`format!`ing every element in a collection, then collecting the strings into a new `String`"
}
declare_clippy_lint! {
/// ### What it does
/// Checks for usage of `.skip(0)` on iterators.
///
/// ### Why is this bad?
/// This was likely intended to be `.skip(1)` to skip the first element, as `.skip(0)` does
/// nothing. If not, the call should be removed.
///
/// ### Example
/// ```rust
/// let v = vec![1, 2, 3];
/// let x = v.iter().skip(0).collect::<Vec<_>>();
/// let y = v.iter().collect::<Vec<_>>();
/// assert_eq!(x, y);
/// ```
#[clippy::version = "1.72.0"]
pub ITER_SKIP_ZERO,
correctness,
"disallows `.skip(0)`"
}
declare_clippy_lint! {
/// ### What it does
/// Checks for usage of `bool::then` in `Iterator::filter_map`.
///
/// ### Why is this bad?
/// This can be written with `filter` then `map` instead, which would reduce nesting and
/// separates the filtering from the transformation phase. This comes with no cost to
/// performance and is just cleaner.
///
/// ### Limitations
/// Does not lint `bool::then_some`, as it eagerly evaluates its arguments rather than lazily.
/// This can create differing behavior, so better safe than sorry.
///
/// ### Example
/// ```rust
/// # fn really_expensive_fn(i: i32) -> i32 { i }
/// # let v = vec![];
/// _ = v.into_iter().filter_map(|i| (i % 2 == 0).then(|| really_expensive_fn(i)));
/// ```
/// Use instead:
/// ```rust
/// # fn really_expensive_fn(i: i32) -> i32 { i }
/// # let v = vec![];
/// _ = v.into_iter().filter(|i| i % 2 == 0).map(|i| really_expensive_fn(i));
/// ```
#[clippy::version = "1.72.0"]
pub FILTER_MAP_BOOL_THEN,
style,
"checks for usage of `bool::then` in `Iterator::filter_map`"
}
declare_clippy_lint! {
/// ### What it does
/// Looks for calls to `RwLock::write` where the lock is only used for reading.
///
/// ### Why is this bad?
/// The write portion of `RwLock` is exclusive, meaning that no other thread
/// can access the lock while this writer is active.
///
/// ### Example
/// ```rust
/// use std::sync::RwLock;
/// fn assert_is_zero(lock: &RwLock<i32>) {
/// let num = lock.write().unwrap();
/// assert_eq!(*num, 0);
/// }
/// ```
///
/// Use instead:
/// ```rust
/// use std::sync::RwLock;
/// fn assert_is_zero(lock: &RwLock<i32>) {
/// let num = lock.read().unwrap();
/// assert_eq!(*num, 0);
/// }
/// ```
#[clippy::version = "1.73.0"]
pub READONLY_WRITE_LOCK,
nursery,
"acquiring a write lock when a read lock would work"
}
pub struct Methods {
avoid_breaking_exported_api: bool,
msrv: Msrv,
@ -3408,7 +3569,7 @@ impl_lint_pass!(Methods => [
SHOULD_IMPLEMENT_TRAIT,
WRONG_SELF_CONVENTION,
OK_EXPECT,
UNWRAP_OR_ELSE_DEFAULT,
UNWRAP_OR_DEFAULT,
MAP_UNWRAP_OR,
RESULT_MAP_OR_INTO_OPTION,
OPTION_MAP_OR_NONE,
@ -3512,6 +3673,11 @@ impl_lint_pass!(Methods => [
UNNECESSARY_LITERAL_UNWRAP,
DRAIN_COLLECT,
MANUAL_TRY_FOLD,
FORMAT_COLLECT,
STRING_LIT_CHARS_ANY,
ITER_SKIP_ZERO,
FILTER_MAP_BOOL_THEN,
READONLY_WRITE_LOCK
]);
/// Extracts a method call name, args, and `Span` of the method name.
@ -3666,8 +3832,7 @@ impl<'tcx> LateLintPass<'tcx> for Methods {
then {
let first_arg_span = first_arg_ty.span;
let first_arg_ty = hir_ty_to_ty(cx.tcx, first_arg_ty);
let self_ty = TraitRef::identity(cx.tcx, item.owner_id.to_def_id())
.self_ty();
let self_ty = TraitRef::identity(cx.tcx, item.owner_id.to_def_id()).self_ty();
wrong_self_convention::check(
cx,
item.ident.name.as_str(),
@ -3684,8 +3849,7 @@ impl<'tcx> LateLintPass<'tcx> for Methods {
if item.ident.name == sym::new;
if let TraitItemKind::Fn(_, _) = item.kind;
let ret_ty = return_ty(cx, item.owner_id);
let self_ty = TraitRef::identity(cx.tcx, item.owner_id.to_def_id())
.self_ty();
let self_ty = TraitRef::identity(cx.tcx, item.owner_id.to_def_id()).self_ty();
if !ret_ty.contains(self_ty);
then {
@ -3733,8 +3897,9 @@ impl Methods {
Some((name @ ("cloned" | "copied"), recv2, [], _, _)) => {
iter_cloned_collect::check(cx, name, expr, recv2);
},
Some(("map", m_recv, [m_arg], _, _)) => {
Some(("map", m_recv, [m_arg], m_ident_span, _)) => {
map_collect_result_unit::check(cx, expr, m_recv, m_arg);
format_collect::check(cx, expr, m_arg, m_ident_span);
},
Some(("take", take_self_arg, [take_arg], _, _)) => {
if self.msrv.meets(msrvs::STR_REPEAT) {
@ -3790,6 +3955,7 @@ impl Methods {
},
("filter_map", [arg]) => {
unnecessary_filter_map::check(cx, expr, arg, name);
filter_map_bool_then::check(cx, expr, arg, call_span);
filter_map_identity::check(cx, expr, arg, span);
},
("find_map", [arg]) => {
@ -3833,7 +3999,16 @@ impl Methods {
unnecessary_join::check(cx, expr, recv, join_arg, span);
}
},
("last", []) | ("skip", [_]) => {
("skip", [arg]) => {
iter_skip_zero::check(cx, expr, arg);
if let Some((name2, recv2, args2, _span2, _)) = method_call(recv) {
if let ("cloned", []) = (name2, args2) {
iter_overeager_cloned::check(cx, expr, recv, recv2, false, false);
}
}
}
("last", []) => {
if let Some((name2, recv2, args2, _span2, _)) = method_call(recv) {
if let ("cloned", []) = (name2, args2) {
iter_overeager_cloned::check(cx, expr, recv, recv2, false, false);
@ -3885,6 +4060,13 @@ impl Methods {
}
}
},
("any", [arg]) if let ExprKind::Closure(arg) = arg.kind
&& let body = cx.tcx.hir().body(arg.body)
&& let [param] = body.params
&& let Some(("chars", recv, _, _, _)) = method_call(recv) =>
{
string_lit_chars_any::check(cx, expr, recv, param, peel_blocks(body.value), &self.msrv);
}
("nth", [n_arg]) => match method_call(recv) {
Some(("bytes", recv2, [], _, _)) => bytes_nth::check(cx, expr, recv2, n_arg),
Some(("cloned", recv2, [], _, _)) => iter_overeager_cloned::check(cx, expr, recv, recv2, false, false),
@ -4027,7 +4209,6 @@ impl Methods {
Some(("map", recv, [map_arg], _, _))
if map_unwrap_or::check(cx, expr, recv, map_arg, u_arg, &self.msrv) => {},
_ => {
unwrap_or_else_default::check(cx, expr, recv, u_arg);
unnecessary_lazy_eval::check(cx, expr, recv, u_arg, "unwrap_or");
},
}
@ -4040,6 +4221,9 @@ impl Methods {
range_zip_with_len::check(cx, expr, iter_recv, arg);
}
},
("write", []) => {
readonly_write_lock::check(cx, expr, recv);
}
_ => {},
}
}

View file

@ -1,16 +1,17 @@
use clippy_utils::diagnostics::span_lint_and_sugg;
use clippy_utils::eager_or_lazy::switch_to_lazy_eval;
use clippy_utils::source::snippet_with_context;
use clippy_utils::ty::{implements_trait, is_type_diagnostic_item};
use clippy_utils::{contains_return, is_trait_item, last_path_segment};
use clippy_utils::ty::{expr_type_is_certain, implements_trait, is_type_diagnostic_item};
use clippy_utils::{contains_return, is_default_equivalent, is_default_equivalent_call, last_path_segment};
use if_chain::if_chain;
use rustc_errors::Applicability;
use rustc_hir as hir;
use rustc_lint::LateContext;
use rustc_middle::ty;
use rustc_span::source_map::Span;
use rustc_span::symbol::{kw, sym, Symbol};
use rustc_span::symbol::{self, sym, Symbol};
use {rustc_ast as ast, rustc_hir as hir};
use super::OR_FUN_CALL;
use super::{OR_FUN_CALL, UNWRAP_OR_DEFAULT};
/// Checks for the `OR_FUN_CALL` lint.
#[allow(clippy::too_many_lines)]
@ -24,53 +25,72 @@ pub(super) fn check<'tcx>(
) {
/// Checks for `unwrap_or(T::new())`, `unwrap_or(T::default())`,
/// `or_insert(T::new())` or `or_insert(T::default())`.
/// Similarly checks for `unwrap_or_else(T::new)`, `unwrap_or_else(T::default)`,
/// `or_insert_with(T::new)` or `or_insert_with(T::default)`.
#[allow(clippy::too_many_arguments)]
fn check_unwrap_or_default(
cx: &LateContext<'_>,
name: &str,
receiver: &hir::Expr<'_>,
fun: &hir::Expr<'_>,
arg: &hir::Expr<'_>,
or_has_args: bool,
call_expr: Option<&hir::Expr<'_>>,
span: Span,
method_span: Span,
) -> bool {
let is_default_default = || is_trait_item(cx, fun, sym::Default);
if !expr_type_is_certain(cx, receiver) {
return false;
}
let implements_default = |arg, default_trait_id| {
let arg_ty = cx.typeck_results().expr_ty(arg);
implements_trait(cx, arg_ty, default_trait_id, &[])
};
if_chain! {
if !or_has_args;
if let Some(sugg) = match name {
"unwrap_or" => Some("unwrap_or_default"),
"or_insert" => Some("or_default"),
_ => None,
};
if let hir::ExprKind::Path(ref qpath) = fun.kind;
if let Some(default_trait_id) = cx.tcx.get_diagnostic_item(sym::Default);
let path = last_path_segment(qpath).ident.name;
// needs to target Default::default in particular or be *::new and have a Default impl
// available
if (matches!(path, kw::Default) && is_default_default())
|| (matches!(path, sym::new) && implements_default(arg, default_trait_id));
then {
span_lint_and_sugg(
cx,
OR_FUN_CALL,
method_span.with_hi(span.hi()),
&format!("use of `{name}` followed by a call to `{path}`"),
"try",
format!("{sugg}()"),
Applicability::MachineApplicable,
);
true
let is_new = |fun: &hir::Expr<'_>| {
if let hir::ExprKind::Path(ref qpath) = fun.kind {
let path = last_path_segment(qpath).ident.name;
matches!(path, sym::new)
} else {
false
}
};
let output_type_implements_default = |fun| {
let fun_ty = cx.typeck_results().expr_ty(fun);
if let ty::FnDef(def_id, args) = fun_ty.kind() {
let output_ty = cx.tcx.fn_sig(def_id).instantiate(cx.tcx, args).skip_binder().output();
cx.tcx
.get_diagnostic_item(sym::Default)
.map_or(false, |default_trait_id| {
implements_trait(cx, output_ty, default_trait_id, args)
})
} else {
false
}
};
let sugg = match (name, call_expr.is_some()) {
("unwrap_or", true) | ("unwrap_or_else", false) => "unwrap_or_default",
("or_insert", true) | ("or_insert_with", false) => "or_default",
_ => return false,
};
// needs to target Default::default in particular or be *::new and have a Default impl
// available
if (is_new(fun) && output_type_implements_default(fun))
|| match call_expr {
Some(call_expr) => is_default_equivalent(cx, call_expr),
None => is_default_equivalent_call(cx, fun) || closure_body_returns_empty_to_string(cx, fun),
}
{
span_lint_and_sugg(
cx,
UNWRAP_OR_DEFAULT,
method_span.with_hi(span.hi()),
&format!("use of `{name}` to construct default value"),
"try",
format!("{sugg}()"),
Applicability::MachineApplicable,
);
true
} else {
false
}
}
@ -168,11 +188,16 @@ pub(super) fn check<'tcx>(
match inner_arg.kind {
hir::ExprKind::Call(fun, or_args) => {
let or_has_args = !or_args.is_empty();
if !check_unwrap_or_default(cx, name, fun, arg, or_has_args, expr.span, method_span) {
if or_has_args
|| !check_unwrap_or_default(cx, name, receiver, fun, Some(inner_arg), expr.span, method_span)
{
let fun_span = if or_has_args { None } else { Some(fun.span) };
check_general_case(cx, name, method_span, receiver, arg, None, expr.span, fun_span);
}
},
hir::ExprKind::Path(..) | hir::ExprKind::Closure(..) => {
check_unwrap_or_default(cx, name, receiver, inner_arg, None, expr.span, method_span);
},
hir::ExprKind::Index(..) | hir::ExprKind::MethodCall(..) => {
check_general_case(cx, name, method_span, receiver, arg, None, expr.span, None);
},
@ -189,3 +214,22 @@ pub(super) fn check<'tcx>(
}
}
}
fn closure_body_returns_empty_to_string(cx: &LateContext<'_>, e: &hir::Expr<'_>) -> bool {
if let hir::ExprKind::Closure(&hir::Closure { body, .. }) = e.kind {
let body = cx.tcx.hir().body(body);
if body.params.is_empty()
&& let hir::Expr{ kind, .. } = &body.value
&& let hir::ExprKind::MethodCall(hir::PathSegment {ident, ..}, self_arg, _, _) = kind
&& ident.name == sym::to_string
&& let hir::Expr{ kind, .. } = self_arg
&& let hir::ExprKind::Lit(lit) = kind
&& let ast::LitKind::Str(symbol::kw::Empty, _) = lit.node
{
return true;
}
}
false
}

View file

@ -35,8 +35,8 @@ pub fn check(cx: &LateContext<'_>, call: &Expr<'_>, recv: &Expr<'_>, arg: &Expr<
&& segment.ident.name == sym!(parse)
&& let parse_result_ty = cx.typeck_results().expr_ty(parent)
&& is_type_diagnostic_item(cx, parse_result_ty, sym::Result)
&& let ty::Adt(_, substs) = parse_result_ty.kind()
&& let Some(ok_ty) = substs[0].as_type()
&& let ty::Adt(_, args) = parse_result_ty.kind()
&& let Some(ok_ty) = args[0].as_type()
&& parse_fails_on_trailing_newline(ok_ty)
{
let local_snippet = snippet(cx, expr.span, "<expr>");

View file

@ -0,0 +1,52 @@
use super::READONLY_WRITE_LOCK;
use clippy_utils::diagnostics::span_lint_and_sugg;
use clippy_utils::mir::{enclosing_mir, visit_local_usage};
use clippy_utils::source::snippet;
use clippy_utils::ty::is_type_diagnostic_item;
use rustc_errors::Applicability;
use rustc_hir::{Expr, ExprKind, Node};
use rustc_lint::LateContext;
use rustc_middle::mir::{Location, START_BLOCK};
use rustc_span::sym;
fn is_unwrap_call(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool {
if let ExprKind::MethodCall(path, receiver, ..) = expr.kind
&& path.ident.name == sym::unwrap
{
is_type_diagnostic_item(cx, cx.typeck_results().expr_ty(receiver).peel_refs(), sym::Result)
} else {
false
}
}
pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>, receiver: &Expr<'_>) {
if is_type_diagnostic_item(cx, cx.typeck_results().expr_ty(receiver).peel_refs(), sym::RwLock)
&& let Node::Expr(unwrap_call_expr) = cx.tcx.hir().get_parent(expr.hir_id)
&& is_unwrap_call(cx, unwrap_call_expr)
&& let parent = cx.tcx.hir().get_parent(unwrap_call_expr.hir_id)
&& let Node::Local(local) = parent
&& let Some(mir) = enclosing_mir(cx.tcx, expr.hir_id)
&& let Some((local, _)) = mir.local_decls.iter_enumerated().find(|(_, decl)| {
local.span.contains(decl.source_info.span)
})
&& let Some(usages) = visit_local_usage(&[local], mir, Location {
block: START_BLOCK,
statement_index: 0,
})
&& let [usage] = usages.as_slice()
{
let writer_never_mutated = usage.local_consume_or_mutate_locs.is_empty();
if writer_never_mutated {
span_lint_and_sugg(
cx,
READONLY_WRITE_LOCK,
expr.span,
"this write lock is used only for reading",
"consider using a read lock instead",
format!("{}.read()", snippet(cx, receiver.span, "<receiver>")),
Applicability::MaybeIncorrect // write lock might be intentional for enforcing exclusiveness
);
}
}
}

View file

@ -0,0 +1,58 @@
use clippy_utils::diagnostics::span_lint_and_then;
use clippy_utils::msrvs::{Msrv, MATCHES_MACRO};
use clippy_utils::source::snippet_opt;
use clippy_utils::{is_from_proc_macro, is_trait_method, path_to_local};
use itertools::Itertools;
use rustc_ast::LitKind;
use rustc_errors::Applicability;
use rustc_hir::{BinOpKind, Expr, ExprKind, Param, PatKind};
use rustc_lint::LateContext;
use rustc_span::sym;
use super::STRING_LIT_CHARS_ANY;
pub(super) fn check<'tcx>(
cx: &LateContext<'tcx>,
expr: &'tcx Expr<'tcx>,
recv: &Expr<'_>,
param: &'tcx Param<'tcx>,
body: &Expr<'_>,
msrv: &Msrv,
) {
if msrv.meets(MATCHES_MACRO)
&& is_trait_method(cx, expr, sym::Iterator)
&& let PatKind::Binding(_, arg, _, _) = param.pat.kind
&& let ExprKind::Lit(lit_kind) = recv.kind
&& let LitKind::Str(val, _) = lit_kind.node
&& let ExprKind::Binary(kind, lhs, rhs) = body.kind
&& let BinOpKind::Eq = kind.node
&& let Some(lhs_path) = path_to_local(lhs)
&& let Some(rhs_path) = path_to_local(rhs)
&& let scrutinee = match (lhs_path == arg, rhs_path == arg) {
(true, false) => rhs,
(false, true) => lhs,
_ => return,
}
&& !is_from_proc_macro(cx, expr)
&& let Some(scrutinee_snip) = snippet_opt(cx, scrutinee.span)
{
// Normalize the char using `map` so `join` doesn't use `Display`, if we don't then
// something like `r"\"` will become `'\'`, which is of course invalid
let pat_snip = val.as_str().chars().map(|c| format!("{c:?}")).join(" | ");
span_lint_and_then(
cx,
STRING_LIT_CHARS_ANY,
expr.span,
"usage of `.chars().any(...)` to check if a char matches any from a string literal",
|diag| {
diag.span_suggestion_verbose(
expr.span,
"use `matches!(...)` instead",
format!("matches!({scrutinee_snip}, {pat_snip})"),
Applicability::MachineApplicable,
);
}
);
}
}

View file

@ -24,9 +24,9 @@ pub(super) fn check(cx: &LateContext<'_>, receiver: &Expr<'_>, call_span: Span)
if let Some(Adjustment { target: recv_ty, .. }) = recv_adjusts.last()
&& let ty::Ref(_, ty, _) = recv_ty.kind()
&& let ty::Adt(adt, substs) = ty.kind()
&& let ty::Adt(adt, args) = ty.kind()
&& adt.is_box()
&& is_dyn_any(cx, substs.type_at(0))
&& is_dyn_any(cx, args.type_at(0))
{
span_lint_and_then(
cx,

View file

@ -3,6 +3,8 @@ use clippy_utils::{is_res_lang_ctor, last_path_segment, path_res, MaybePath};
use rustc_errors::Applicability;
use rustc_hir as hir;
use rustc_lint::LateContext;
use rustc_middle::ty;
use rustc_middle::ty::print::with_forced_trimmed_paths;
use super::UNNECESSARY_LITERAL_UNWRAP;
@ -22,6 +24,7 @@ fn get_ty_from_args<'a>(args: Option<&'a [hir::GenericArg<'a>]>, index: usize) -
}
}
#[expect(clippy::too_many_lines)]
pub(super) fn check(
cx: &LateContext<'_>,
expr: &hir::Expr<'_>,
@ -84,6 +87,34 @@ pub(super) fn check(
}
Some(suggs)
},
("None", "unwrap_or_default", _) => {
let ty = cx.typeck_results().expr_ty(expr);
let default_ty_string = if let ty::Adt(def, ..) = ty.kind() {
with_forced_trimmed_paths!(format!("{}", cx.tcx.def_path_str(def.did())))
} else {
"Default".to_string()
};
Some(vec![(expr.span, format!("{default_ty_string}::default()"))])
},
("None", "unwrap_or", _) => Some(vec![
(expr.span.with_hi(args[0].span.lo()), String::new()),
(expr.span.with_lo(args[0].span.hi()), String::new()),
]),
("None", "unwrap_or_else", _) => match args[0].kind {
hir::ExprKind::Closure(hir::Closure {
fn_decl:
hir::FnDecl {
output: hir::FnRetTy::DefaultReturn(span) | hir::FnRetTy::Return(hir::Ty { span, .. }),
..
},
..
}) => Some(vec![
(expr.span.with_hi(span.hi()), String::new()),
(expr.span.with_lo(args[0].span.hi()), String::new()),
]),
_ => None,
},
_ if call_args.is_empty() => None,
(_, _, Some(_)) => None,
("Ok", "unwrap_err", None) | ("Err", "unwrap", None) => Some(vec![
(

View file

@ -6,7 +6,8 @@ use if_chain::if_chain;
use rustc_errors::Applicability;
use rustc_hir::{Closure, Expr, ExprKind, Mutability, Param, Pat, PatKind, Path, PathSegment, QPath};
use rustc_lint::LateContext;
use rustc_middle::ty::{self, GenericArgKind};
use rustc_middle::ty;
use rustc_middle::ty::GenericArgKind;
use rustc_span::sym;
use rustc_span::symbol::Ident;
use std::iter;

View file

@ -1,66 +0,0 @@
//! Lint for `some_result_or_option.unwrap_or_else(Default::default)`
use super::UNWRAP_OR_ELSE_DEFAULT;
use clippy_utils::diagnostics::span_lint_and_sugg;
use clippy_utils::is_default_equivalent_call;
use clippy_utils::source::snippet_with_applicability;
use clippy_utils::ty::is_type_diagnostic_item;
use rustc_ast::ast::LitKind;
use rustc_errors::Applicability;
use rustc_hir as hir;
use rustc_lint::LateContext;
use rustc_span::{sym, symbol};
pub(super) fn check<'tcx>(
cx: &LateContext<'tcx>,
expr: &'tcx hir::Expr<'_>,
recv: &'tcx hir::Expr<'_>,
u_arg: &'tcx hir::Expr<'_>,
) {
// something.unwrap_or_else(Default::default)
// ^^^^^^^^^- recv ^^^^^^^^^^^^^^^^- u_arg
// ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^- expr
let recv_ty = cx.typeck_results().expr_ty(recv);
let is_option = is_type_diagnostic_item(cx, recv_ty, sym::Option);
let is_result = is_type_diagnostic_item(cx, recv_ty, sym::Result);
if_chain! {
if is_option || is_result;
if closure_body_returns_empty_to_string(cx, u_arg) || is_default_equivalent_call(cx, u_arg);
then {
let mut applicability = Applicability::MachineApplicable;
span_lint_and_sugg(
cx,
UNWRAP_OR_ELSE_DEFAULT,
expr.span,
"use of `.unwrap_or_else(..)` to construct default value",
"try",
format!(
"{}.unwrap_or_default()",
snippet_with_applicability(cx, recv.span, "..", &mut applicability)
),
applicability,
);
}
}
}
fn closure_body_returns_empty_to_string(cx: &LateContext<'_>, e: &hir::Expr<'_>) -> bool {
if let hir::ExprKind::Closure(&hir::Closure { body, .. }) = e.kind {
let body = cx.tcx.hir().body(body);
if body.params.is_empty()
&& let hir::Expr{ kind, .. } = &body.value
&& let hir::ExprKind::MethodCall(hir::PathSegment {ident, ..}, self_arg, _, _) = kind
&& ident == &symbol::Ident::from_str("to_string")
&& let hir::Expr{ kind, .. } = self_arg
&& let hir::ExprKind::Lit(lit) = kind
&& let LitKind::Str(symbol::kw::Empty, _) = lit.node
{
return true;
}
}
false
}

View file

@ -129,6 +129,14 @@ impl Visitor<'_> for IdentVisitor<'_, '_> {
return;
}
// `struct Array<T, const N: usize>([T; N])`
// ^
if let Node::GenericParam(generic_param) = node
&& let GenericParamKind::Const { .. } = generic_param.kind
{
return;
}
if is_from_proc_macro(cx, &ident) {
return;
}

View file

@ -96,10 +96,6 @@ impl<'a, 'tcx> Visitor<'tcx> for MutArgVisitor<'a, 'tcx> {
self.found = true;
return;
},
ExprKind::If(..) => {
self.found = true;
return;
},
ExprKind::Path(_) => {
if let Some(adj) = self.cx.typeck_results().adjustments().get(expr.hir_id) {
if adj

View file

@ -1,16 +1,18 @@
use super::needless_pass_by_value::requires_exact_signature;
use clippy_utils::diagnostics::span_lint_and_then;
use clippy_utils::diagnostics::span_lint_hir_and_then;
use clippy_utils::source::snippet;
use clippy_utils::{is_from_proc_macro, is_self};
use if_chain::if_chain;
use clippy_utils::{get_parent_node, inherits_cfg, is_from_proc_macro, is_self};
use rustc_data_structures::fx::{FxHashSet, FxIndexMap};
use rustc_errors::Applicability;
use rustc_hir::intravisit::FnKind;
use rustc_hir::{Body, FnDecl, HirId, HirIdMap, HirIdSet, Impl, ItemKind, Mutability, Node, PatKind};
use rustc_hir::intravisit::{walk_qpath, FnKind, Visitor};
use rustc_hir::{Body, ExprKind, FnDecl, HirId, HirIdMap, HirIdSet, Impl, ItemKind, Mutability, Node, PatKind, QPath};
use rustc_hir_typeck::expr_use_visitor as euv;
use rustc_infer::infer::TyCtxtInferExt;
use rustc_lint::{LateContext, LateLintPass};
use rustc_middle::hir::map::associated_body;
use rustc_middle::hir::nested_filter::OnlyBodies;
use rustc_middle::mir::FakeReadCause;
use rustc_middle::ty::{self, Ty};
use rustc_middle::ty::{self, Ty, UpvarId, UpvarPath};
use rustc_session::{declare_tool_lint, impl_lint_pass};
use rustc_span::def_id::LocalDefId;
use rustc_span::symbol::kw;
@ -46,20 +48,24 @@ declare_clippy_lint! {
"using a `&mut` argument when it's not mutated"
}
#[derive(Copy, Clone)]
pub struct NeedlessPassByRefMut {
#[derive(Clone)]
pub struct NeedlessPassByRefMut<'tcx> {
avoid_breaking_exported_api: bool,
used_fn_def_ids: FxHashSet<LocalDefId>,
fn_def_ids_to_maybe_unused_mut: FxIndexMap<LocalDefId, Vec<rustc_hir::Ty<'tcx>>>,
}
impl NeedlessPassByRefMut {
impl NeedlessPassByRefMut<'_> {
pub fn new(avoid_breaking_exported_api: bool) -> Self {
Self {
avoid_breaking_exported_api,
used_fn_def_ids: FxHashSet::default(),
fn_def_ids_to_maybe_unused_mut: FxIndexMap::default(),
}
}
}
impl_lint_pass!(NeedlessPassByRefMut => [NEEDLESS_PASS_BY_REF_MUT]);
impl_lint_pass!(NeedlessPassByRefMut<'_> => [NEEDLESS_PASS_BY_REF_MUT]);
fn should_skip<'tcx>(
cx: &LateContext<'tcx>,
@ -87,12 +93,12 @@ fn should_skip<'tcx>(
is_from_proc_macro(cx, &input)
}
impl<'tcx> LateLintPass<'tcx> for NeedlessPassByRefMut {
impl<'tcx> LateLintPass<'tcx> for NeedlessPassByRefMut<'tcx> {
fn check_fn(
&mut self,
cx: &LateContext<'tcx>,
kind: FnKind<'tcx>,
decl: &'tcx FnDecl<'_>,
decl: &'tcx FnDecl<'tcx>,
body: &'tcx Body<'_>,
span: Span,
fn_def_id: LocalDefId,
@ -102,17 +108,17 @@ impl<'tcx> LateLintPass<'tcx> for NeedlessPassByRefMut {
}
let hir_id = cx.tcx.hir().local_def_id_to_hir_id(fn_def_id);
match kind {
let is_async = match kind {
FnKind::ItemFn(.., header) => {
let attrs = cx.tcx.hir().attrs(hir_id);
if header.abi != Abi::Rust || requires_exact_signature(attrs) {
return;
}
header.is_async()
},
FnKind::Method(..) => (),
FnKind::Method(.., sig) => sig.header.is_async(),
FnKind::Closure => return,
}
};
// Exclude non-inherent impls
if let Some(Node::Item(item)) = cx.tcx.hir().find_parent(hir_id) {
@ -128,25 +134,6 @@ impl<'tcx> LateLintPass<'tcx> for NeedlessPassByRefMut {
let fn_sig = cx.tcx.liberate_late_bound_regions(fn_def_id.to_def_id(), fn_sig);
// If there are no `&mut` argument, no need to go any further.
if !decl
.inputs
.iter()
.zip(fn_sig.inputs())
.zip(body.params)
.any(|((&input, &ty), arg)| !should_skip(cx, input, ty, arg))
{
return;
}
// Collect variables mutably used and spans which will need dereferencings from the
// function body.
let MutablyUsedVariablesCtxt { mutably_used_vars, .. } = {
let mut ctx = MutablyUsedVariablesCtxt::default();
let infcx = cx.tcx.infer_ctxt().build();
euv::ExprUseVisitor::new(&mut ctx, &infcx, fn_def_id, cx.param_env, cx.typeck_results()).consume_body(body);
ctx
};
let mut it = decl
.inputs
.iter()
@ -157,34 +144,79 @@ impl<'tcx> LateLintPass<'tcx> for NeedlessPassByRefMut {
if it.peek().is_none() {
return;
}
let show_semver_warning = self.avoid_breaking_exported_api && cx.effective_visibilities.is_exported(fn_def_id);
// Collect variables mutably used and spans which will need dereferencings from the
// function body.
let MutablyUsedVariablesCtxt { mutably_used_vars, .. } = {
let mut ctx = MutablyUsedVariablesCtxt::default();
let infcx = cx.tcx.infer_ctxt().build();
euv::ExprUseVisitor::new(&mut ctx, &infcx, fn_def_id, cx.param_env, cx.typeck_results()).consume_body(body);
if is_async {
let closures = ctx.async_closures.clone();
let hir = cx.tcx.hir();
for closure in closures {
ctx.prev_bind = None;
ctx.prev_move_to_closure.clear();
if let Some(body) = hir
.find_by_def_id(closure)
.and_then(associated_body)
.map(|(_, body_id)| hir.body(body_id))
{
euv::ExprUseVisitor::new(&mut ctx, &infcx, closure, cx.param_env, cx.typeck_results())
.consume_body(body);
}
}
}
ctx
};
for ((&input, &_), arg) in it {
// Only take `&mut` arguments.
if_chain! {
if let PatKind::Binding(_, canonical_id, ..) = arg.pat.kind;
if !mutably_used_vars.contains(&canonical_id);
if let rustc_hir::TyKind::Ref(_, inner_ty) = input.kind;
then {
// If the argument is never used mutably, we emit the warning.
let sp = input.span;
span_lint_and_then(
if let PatKind::Binding(_, canonical_id, ..) = arg.pat.kind
&& !mutably_used_vars.contains(&canonical_id)
{
self.fn_def_ids_to_maybe_unused_mut.entry(fn_def_id).or_default().push(input);
}
}
}
fn check_crate_post(&mut self, cx: &LateContext<'tcx>) {
cx.tcx.hir().visit_all_item_likes_in_crate(&mut FnNeedsMutVisitor {
cx,
used_fn_def_ids: &mut self.used_fn_def_ids,
});
for (fn_def_id, unused) in self
.fn_def_ids_to_maybe_unused_mut
.iter()
.filter(|(def_id, _)| !self.used_fn_def_ids.contains(def_id))
{
let show_semver_warning =
self.avoid_breaking_exported_api && cx.effective_visibilities.is_exported(*fn_def_id);
let mut is_cfged = None;
for input in unused {
// If the argument is never used mutably, we emit the warning.
let sp = input.span;
if let rustc_hir::TyKind::Ref(_, inner_ty) = input.kind {
let is_cfged = is_cfged.get_or_insert_with(|| inherits_cfg(cx.tcx, *fn_def_id));
span_lint_hir_and_then(
cx,
NEEDLESS_PASS_BY_REF_MUT,
cx.tcx.hir().local_def_id_to_hir_id(*fn_def_id),
sp,
"this argument is a mutable reference, but not used mutably",
|diag| {
diag.span_suggestion(
sp,
"consider changing to".to_string(),
format!(
"&{}",
snippet(cx, cx.tcx.hir().span(inner_ty.ty.hir_id), "_"),
),
format!("&{}", snippet(cx, cx.tcx.hir().span(inner_ty.ty.hir_id), "_"),),
Applicability::Unspecified,
);
if show_semver_warning {
diag.warn("changing this function will impact semver compatibility");
}
if *is_cfged {
diag.note("this is cfg-gated and may require further changes");
}
},
);
}
@ -197,7 +229,9 @@ impl<'tcx> LateLintPass<'tcx> for NeedlessPassByRefMut {
struct MutablyUsedVariablesCtxt {
mutably_used_vars: HirIdSet,
prev_bind: Option<HirId>,
prev_move_to_closure: HirIdSet,
aliases: HirIdMap<HirId>,
async_closures: FxHashSet<LocalDefId>,
}
impl MutablyUsedVariablesCtxt {
@ -213,16 +247,27 @@ impl MutablyUsedVariablesCtxt {
impl<'tcx> euv::Delegate<'tcx> for MutablyUsedVariablesCtxt {
fn consume(&mut self, cmt: &euv::PlaceWithHirId<'tcx>, _id: HirId) {
if let euv::Place {
base: euv::PlaceBase::Local(vid),
base:
euv::PlaceBase::Local(vid)
| euv::PlaceBase::Upvar(UpvarId {
var_path: UpvarPath { hir_id: vid },
..
}),
base_ty,
..
} = &cmt.place
{
if let Some(bind_id) = self.prev_bind.take() {
self.aliases.insert(bind_id, *vid);
} else if matches!(base_ty.ref_mutability(), Some(Mutability::Mut)) {
if bind_id != *vid {
self.aliases.insert(bind_id, *vid);
}
} else if !self.prev_move_to_closure.contains(vid)
&& matches!(base_ty.ref_mutability(), Some(Mutability::Mut))
{
self.add_mutably_used_var(*vid);
}
self.prev_bind = None;
self.prev_move_to_closure.remove(vid);
}
}
@ -265,9 +310,73 @@ impl<'tcx> euv::Delegate<'tcx> for MutablyUsedVariablesCtxt {
self.prev_bind = None;
}
fn fake_read(&mut self, _: &rustc_hir_typeck::expr_use_visitor::PlaceWithHirId<'tcx>, _: FakeReadCause, _: HirId) {}
fn fake_read(
&mut self,
cmt: &rustc_hir_typeck::expr_use_visitor::PlaceWithHirId<'tcx>,
cause: FakeReadCause,
_id: HirId,
) {
if let euv::Place {
base:
euv::PlaceBase::Upvar(UpvarId {
var_path: UpvarPath { hir_id: vid },
..
}),
..
} = &cmt.place
{
if let FakeReadCause::ForLet(Some(inner)) = cause {
// Seems like we are inside an async function. We need to store the closure `DefId`
// to go through it afterwards.
self.async_closures.insert(inner);
self.aliases.insert(cmt.hir_id, *vid);
self.prev_move_to_closure.insert(*vid);
}
}
}
fn bind(&mut self, _cmt: &euv::PlaceWithHirId<'tcx>, id: HirId) {
self.prev_bind = Some(id);
}
}
/// A final pass to check for paths referencing this function that require the argument to be
/// `&mut`, basically if the function is ever used as a `fn`-like argument.
struct FnNeedsMutVisitor<'a, 'tcx> {
cx: &'a LateContext<'tcx>,
used_fn_def_ids: &'a mut FxHashSet<LocalDefId>,
}
impl<'tcx> Visitor<'tcx> for FnNeedsMutVisitor<'_, 'tcx> {
type NestedFilter = OnlyBodies;
fn nested_visit_map(&mut self) -> Self::Map {
self.cx.tcx.hir()
}
fn visit_qpath(&mut self, qpath: &'tcx QPath<'tcx>, hir_id: HirId, _: Span) {
walk_qpath(self, qpath, hir_id);
let Self { cx, used_fn_def_ids } = self;
// #11182; do not lint if mutability is required elsewhere
if let Node::Expr(expr) = cx.tcx.hir().get(hir_id)
&& let Some(parent) = get_parent_node(cx.tcx, expr.hir_id)
&& let ty::FnDef(def_id, _) = cx.tcx.typeck(cx.tcx.hir().enclosing_body_owner(hir_id)).expr_ty(expr).kind()
&& let Some(def_id) = def_id.as_local()
{
if let Node::Expr(e) = parent
&& let ExprKind::Call(call, _) = e.kind
&& call.hir_id == expr.hir_id
{
return;
}
// We don't need to check each argument individually as you cannot coerce a function
// taking `&mut` -> `&`, for some reason, so if we've gotten this far we know it's
// passed as a `fn`-like argument (or is unified) and should ignore every "unused"
// argument entirely
used_fn_def_ids.insert(def_id);
}
}
}

View file

@ -154,7 +154,7 @@ fn is_value_unfrozen_raw<'tcx>(
ty::Adt(def, ..) if def.is_union() => false,
ty::Array(ty, _) => val.unwrap_branch().iter().any(|field| inner(cx, *field, ty)),
ty::Adt(def, _) if def.is_union() => false,
ty::Adt(def, substs) if def.is_enum() => {
ty::Adt(def, args) if def.is_enum() => {
let (&variant_index, fields) = val.unwrap_branch().split_first().unwrap();
let variant_index = VariantIdx::from_u32(variant_index.unwrap_leaf().try_to_u32().ok().unwrap());
fields
@ -164,19 +164,14 @@ fn is_value_unfrozen_raw<'tcx>(
def.variants()[variant_index]
.fields
.iter()
.map(|field| field.ty(cx.tcx, substs)),
.map(|field| field.ty(cx.tcx, args)),
)
.any(|(field, ty)| inner(cx, field, ty))
},
ty::Adt(def, substs) => val
ty::Adt(def, args) => val
.unwrap_branch()
.iter()
.zip(
def.non_enum_variant()
.fields
.iter()
.map(|field| field.ty(cx.tcx, substs)),
)
.zip(def.non_enum_variant().fields.iter().map(|field| field.ty(cx.tcx, args)))
.any(|(field, ty)| inner(cx, *field, ty)),
ty::Tuple(tys) => val
.unwrap_branch()

View file

@ -1,10 +1,9 @@
use clippy_utils::diagnostics::span_lint_and_help;
use clippy_utils::is_direct_expn_of;
use if_chain::if_chain;
use rustc_ast::ast::{Expr, ExprKind, MethodCall};
use rustc_lint::{EarlyContext, EarlyLintPass};
use rustc_session::{declare_lint_pass, declare_tool_lint};
use rustc_span::sym;
use rustc_span::{sym, Span};
declare_clippy_lint! {
/// ### What it does
@ -36,21 +35,27 @@ declare_lint_pass!(OptionEnvUnwrap => [OPTION_ENV_UNWRAP]);
impl EarlyLintPass for OptionEnvUnwrap {
fn check_expr(&mut self, cx: &EarlyContext<'_>, expr: &Expr) {
if_chain! {
if let ExprKind::MethodCall(box MethodCall { seg, receiver, .. }) = &expr.kind;
if matches!(seg.ident.name, sym::expect | sym::unwrap);
if let ExprKind::Call(caller, _) = &receiver.kind;
if is_direct_expn_of(caller.span, "option_env").is_some();
then {
span_lint_and_help(
cx,
OPTION_ENV_UNWRAP,
expr.span,
"this will panic at run-time if the environment variable doesn't exist at compile-time",
None,
"consider using the `env!` macro instead"
);
}
fn lint(cx: &EarlyContext<'_>, span: Span) {
span_lint_and_help(
cx,
OPTION_ENV_UNWRAP,
span,
"this will panic at run-time if the environment variable doesn't exist at compile-time",
None,
"consider using the `env!` macro instead",
);
}
if let ExprKind::MethodCall(box MethodCall { seg, receiver, .. }) = &expr.kind &&
matches!(seg.ident.name, sym::expect | sym::unwrap) {
if let ExprKind::Call(caller, _) = &receiver.kind &&
// If it exists, it will be ::core::option::Option::Some("<env var>").unwrap() (A method call in the HIR)
is_direct_expn_of(caller.span, "option_env").is_some() {
lint(cx, expr.span);
} else if let ExprKind::Path(_, caller) = &receiver.kind && // If it doesn't exist, it will be ::core::option::Option::None::<&'static str>.unwrap() (A path in the HIR)
is_direct_expn_of(caller.span, "option_env").is_some() {
lint(cx, expr.span);
}
}
}
}

View file

@ -26,6 +26,7 @@ use rustc_session::{declare_lint_pass, declare_tool_lint};
use rustc_span::source_map::Span;
use rustc_span::sym;
use rustc_span::symbol::Symbol;
use rustc_target::spec::abi::Abi;
use rustc_trait_selection::infer::InferCtxtExt as _;
use rustc_trait_selection::traits::query::evaluate_obligation::InferCtxtExt as _;
use std::{fmt, iter};
@ -163,6 +164,12 @@ impl<'tcx> LateLintPass<'tcx> for Ptr {
}
check_mut_from_ref(cx, sig, None);
if !matches!(sig.header.abi, Abi::Rust) {
// Ignore `extern` functions with non-Rust calling conventions
return;
}
for arg in check_fn_args(
cx,
cx.tcx
@ -222,6 +229,12 @@ impl<'tcx> LateLintPass<'tcx> for Ptr {
};
check_mut_from_ref(cx, sig, Some(body));
if !matches!(sig.header.abi, Abi::Rust) {
// Ignore `extern` functions with non-Rust calling conventions
return;
}
let decl = sig.decl;
let sig = cx.tcx.fn_sig(item_id).instantiate_identity().skip_binder();
let lint_args: Vec<_> = check_fn_args(cx, sig.inputs(), decl.inputs, &decl.output, body.params)

View file

@ -0,0 +1,103 @@
use clippy_utils::diagnostics::span_lint_and_help;
use clippy_utils::is_from_proc_macro;
use clippy_utils::ty::needs_ordered_drop;
use rustc_hir::def::Res;
use rustc_hir::{BindingAnnotation, ByRef, Expr, ExprKind, HirId, Local, Node, Pat, PatKind, QPath};
use rustc_lint::{LateContext, LateLintPass, LintContext};
use rustc_middle::lint::in_external_macro;
use rustc_session::{declare_lint_pass, declare_tool_lint};
use rustc_span::symbol::Ident;
declare_clippy_lint! {
/// ### What it does
/// Checks for redundant redefinitions of local bindings.
///
/// ### Why is this bad?
/// Redundant redefinitions of local bindings do not change behavior and are likely to be unintended.
///
/// Note that although these bindings do not affect your code's meaning, they _may_ affect `rustc`'s stack allocation.
///
/// ### Example
/// ```rust
/// let a = 0;
/// let a = a;
///
/// fn foo(b: i32) {
/// let b = b;
/// }
/// ```
/// Use instead:
/// ```rust
/// let a = 0;
/// // no redefinition with the same name
///
/// fn foo(b: i32) {
/// // no redefinition with the same name
/// }
/// ```
#[clippy::version = "1.72.0"]
pub REDUNDANT_LOCALS,
correctness,
"redundant redefinition of a local binding"
}
declare_lint_pass!(RedundantLocals => [REDUNDANT_LOCALS]);
impl<'tcx> LateLintPass<'tcx> for RedundantLocals {
fn check_local(&mut self, cx: &LateContext<'tcx>, local: &'tcx Local<'tcx>) {
if_chain! {
// the pattern is a single by-value binding
if let PatKind::Binding(BindingAnnotation(ByRef::No, mutability), _, ident, None) = local.pat.kind;
// the binding is not type-ascribed
if local.ty.is_none();
// the expression is a resolved path
if let Some(expr) = local.init;
if let ExprKind::Path(qpath @ QPath::Resolved(None, path)) = expr.kind;
// the path is a single segment equal to the local's name
if let [last_segment] = path.segments;
if last_segment.ident == ident;
// resolve the path to its defining binding pattern
if let Res::Local(binding_id) = cx.qpath_res(&qpath, expr.hir_id);
if let Node::Pat(binding_pat) = cx.tcx.hir().get(binding_id);
// the previous binding has the same mutability
if find_binding(binding_pat, ident).unwrap().1 == mutability;
// the local does not affect the code's drop behavior
if !affects_drop_behavior(cx, binding_id, local.hir_id, expr);
// the local is user-controlled
if !in_external_macro(cx.sess(), local.span);
if !is_from_proc_macro(cx, expr);
then {
span_lint_and_help(
cx,
REDUNDANT_LOCALS,
vec![binding_pat.span, local.span],
"redundant redefinition of a binding",
None,
&format!("remove the redefinition of `{ident}`"),
);
}
}
}
}
/// Find the annotation of a binding introduced by a pattern, or `None` if it's not introduced.
fn find_binding(pat: &Pat<'_>, name: Ident) -> Option<BindingAnnotation> {
let mut ret = None;
pat.each_binding_or_first(&mut |annotation, _, _, ident| {
if ident == name {
ret = Some(annotation);
}
});
ret
}
/// Check if a rebinding of a local affects the code's drop behavior.
fn affects_drop_behavior<'tcx>(cx: &LateContext<'tcx>, bind: HirId, rebind: HirId, rebind_expr: &Expr<'tcx>) -> bool {
let hir = cx.tcx.hir();
// the rebinding is in a different scope than the original binding
// and the type of the binding cares about drop order
hir.get_enclosing_scope(bind) != hir.get_enclosing_scope(rebind)
&& needs_ordered_drop(cx, cx.typeck_results().expr_ty(rebind_expr))
}

View file

@ -5,6 +5,7 @@ use rustc_ast::ast::{ConstItem, Item, ItemKind, StaticItem, Ty, TyKind};
use rustc_errors::Applicability;
use rustc_lint::{EarlyContext, EarlyLintPass};
use rustc_session::{declare_tool_lint, impl_lint_pass};
use rustc_span::symbol::kw;
declare_clippy_lint! {
/// ### What it does
@ -64,7 +65,7 @@ impl RedundantStaticLifetimes {
if let Some(lifetime) = *optional_lifetime {
match borrow_type.ty.kind {
TyKind::Path(..) | TyKind::Slice(..) | TyKind::Array(..) | TyKind::Tup(..) => {
if lifetime.ident.name == rustc_span::symbol::kw::StaticLifetime {
if lifetime.ident.name == kw::StaticLifetime {
let snip = snippet(cx, borrow_type.ty.span, "<type>");
let sugg = format!("&{}{snip}", borrow_type.mutbl.prefix_str());
span_lint_and_then(

View file

@ -30,6 +30,7 @@ pub static RENAMED_LINTS: &[(&str, &str)] = &[
("clippy::single_char_push_str", "clippy::single_char_add_str"),
("clippy::stutter", "clippy::module_name_repetitions"),
("clippy::to_string_in_display", "clippy::recursive_format_impl"),
("clippy::unwrap_or_else_default", "clippy::unwrap_or_default"),
("clippy::zero_width_space", "clippy::invisible_characters"),
("clippy::cast_ref_to_mut", "cast_ref_to_mut"),
("clippy::clone_double_ref", "suspicious_double_ref_op"),

View file

@ -1,12 +1,14 @@
use clippy_utils::diagnostics::{span_lint_and_then, span_lint_hir_and_then};
use clippy_utils::diagnostics::{span_lint_and_sugg, span_lint_and_then, span_lint_hir_and_then};
use clippy_utils::source::{snippet_opt, snippet_with_context};
use clippy_utils::visitors::{for_each_expr_with_closures, Descend};
use clippy_utils::{fn_def_id, path_to_local_id, span_find_starting_semi};
use clippy_utils::{fn_def_id, is_from_proc_macro, path_to_local_id, span_find_starting_semi};
use core::ops::ControlFlow;
use if_chain::if_chain;
use rustc_errors::Applicability;
use rustc_hir::intravisit::FnKind;
use rustc_hir::{Block, Body, Expr, ExprKind, FnDecl, LangItem, MatchSource, PatKind, QPath, StmtKind};
use rustc_hir::{
Block, Body, Expr, ExprKind, FnDecl, ItemKind, LangItem, MatchSource, OwnerNode, PatKind, QPath, Stmt, StmtKind,
};
use rustc_lint::{LateContext, LateLintPass, LintContext};
use rustc_middle::lint::in_external_macro;
use rustc_middle::ty::{self, GenericArgKind, Ty};
@ -76,6 +78,46 @@ declare_clippy_lint! {
"using a return statement like `return expr;` where an expression would suffice"
}
declare_clippy_lint! {
/// ### What it does
/// Checks for return statements on `Err` paired with the `?` operator.
///
/// ### Why is this bad?
/// The `return` is unnecessary.
///
/// ### Example
/// ```rust,ignore
/// fn foo(x: usize) -> Result<(), Box<dyn Error>> {
/// if x == 0 {
/// return Err(...)?;
/// }
/// Ok(())
/// }
/// ```
/// simplify to
/// ```rust,ignore
/// fn foo(x: usize) -> Result<(), Box<dyn Error>> {
/// if x == 0 {
/// Err(...)?;
/// }
/// Ok(())
/// }
/// ```
/// if paired with `try_err`, use instead:
/// ```rust,ignore
/// fn foo(x: usize) -> Result<(), Box<dyn Error>> {
/// if x == 0 {
/// return Err(...);
/// }
/// Ok(())
/// }
/// ```
#[clippy::version = "1.73.0"]
pub NEEDLESS_RETURN_WITH_QUESTION_MARK,
style,
"using a return statement like `return Err(expr)?;` where removing it would suffice"
}
#[derive(PartialEq, Eq)]
enum RetReplacement<'tcx> {
Empty,
@ -115,9 +157,35 @@ impl<'tcx> ToString for RetReplacement<'tcx> {
}
}
declare_lint_pass!(Return => [LET_AND_RETURN, NEEDLESS_RETURN]);
declare_lint_pass!(Return => [LET_AND_RETURN, NEEDLESS_RETURN, NEEDLESS_RETURN_WITH_QUESTION_MARK]);
impl<'tcx> LateLintPass<'tcx> for Return {
fn check_stmt(&mut self, cx: &LateContext<'tcx>, stmt: &'tcx Stmt<'_>) {
if !in_external_macro(cx.sess(), stmt.span)
&& let StmtKind::Semi(expr) = stmt.kind
&& let ExprKind::Ret(Some(ret)) = expr.kind
&& let ExprKind::Match(.., MatchSource::TryDesugar) = ret.kind
// Ensure this is not the final stmt, otherwise removing it would cause a compile error
&& let OwnerNode::Item(item) = cx.tcx.hir().owner(cx.tcx.hir().get_parent_item(expr.hir_id))
&& let ItemKind::Fn(_, _, body) = item.kind
&& let block = cx.tcx.hir().body(body).value
&& let ExprKind::Block(block, _) = block.kind
&& let [.., final_stmt] = block.stmts
&& final_stmt.hir_id != stmt.hir_id
&& !is_from_proc_macro(cx, expr)
{
span_lint_and_sugg(
cx,
NEEDLESS_RETURN_WITH_QUESTION_MARK,
expr.span.until(ret.span),
"unneeded `return` statement with `?` operator",
"remove it",
String::new(),
Applicability::MachineApplicable,
);
}
}
fn check_block(&mut self, cx: &LateContext<'tcx>, block: &'tcx Block<'_>) {
// we need both a let-binding stmt and an expr
if_chain! {
@ -173,6 +241,10 @@ impl<'tcx> LateLintPass<'tcx> for Return {
sp: Span,
_: LocalDefId,
) {
if sp.from_expansion() {
return;
}
match kind {
FnKind::Closure => {
// when returning without value in closure, replace this `return`

View file

@ -55,11 +55,11 @@ impl<'tcx> LateLintPass<'tcx> for SameNameMethod {
if matches!(cx.tcx.def_kind(id.owner_id), DefKind::Impl { .. })
&& let item = cx.tcx.hir().item(id)
&& let ItemKind::Impl(Impl {
items,
of_trait,
self_ty,
..
}) = &item.kind
items,
of_trait,
self_ty,
..
}) = &item.kind
&& let TyKind::Path(QPath::Resolved(_, Path { res, .. })) = self_ty.kind
{
if !map.contains_key(res) {

View file

@ -43,7 +43,7 @@ impl<'tcx> LateLintPass<'tcx> for SemicolonIfNothingReturned {
if let Some(expr) = block.expr;
let t_expr = cx.typeck_results().expr_ty(expr);
if t_expr.is_unit();
let mut app = Applicability::MaybeIncorrect;
let mut app = Applicability::MachineApplicable;
if let snippet = snippet_with_context(cx, expr.span, block.span.ctxt(), "}", &mut app).0;
if !snippet.ends_with('}') && !snippet.ends_with(';');
if cx.sess().source_map().is_multiline(block.span);

View file

@ -1,6 +1,6 @@
use clippy_utils::diagnostics::span_lint_and_then;
use clippy_utils::source::{indent_of, snippet};
use clippy_utils::{expr_or_init, get_attr, path_to_local};
use clippy_utils::{expr_or_init, get_attr, path_to_local, peel_hir_expr_unary};
use rustc_data_structures::fx::{FxHashMap, FxHashSet, FxIndexMap};
use rustc_errors::Applicability;
use rustc_hir::def::{DefKind, Res};
@ -235,7 +235,7 @@ impl<'ap, 'lc, 'others, 'stmt, 'tcx> StmtsChecker<'ap, 'lc, 'others, 'stmt, 'tcx
fn manage_has_expensive_expr_after_last_attr(&mut self) {
let has_expensive_stmt = match self.ap.curr_stmt.kind {
hir::StmtKind::Expr(expr) if !is_expensive_expr(expr) => false,
hir::StmtKind::Expr(expr) if is_inexpensive_expr(expr) => false,
hir::StmtKind::Local(local) if let Some(expr) = local.init
&& let hir::ExprKind::Path(_) = expr.kind => false,
_ => true
@ -330,13 +330,13 @@ impl<'ap, 'lc, 'others, 'stmt, 'tcx> Visitor<'tcx> for StmtsChecker<'ap, 'lc, 'o
apa.last_method_span = span;
}
},
hir::StmtKind::Semi(expr) => {
if has_drop(expr, &apa.first_bind_ident, self.cx) {
hir::StmtKind::Semi(semi_expr) => {
if has_drop(semi_expr, &apa.first_bind_ident, self.cx) {
apa.has_expensive_expr_after_last_attr = false;
apa.last_stmt_span = DUMMY_SP;
return;
}
if let hir::ExprKind::MethodCall(_, _, _, span) = expr.kind {
if let hir::ExprKind::MethodCall(_, _, _, span) = semi_expr.kind {
apa.last_method_span = span;
}
},
@ -434,16 +434,31 @@ fn has_drop(expr: &hir::Expr<'_>, first_bind_ident: &Ident, lcx: &LateContext<'_
&& let Res::Def(DefKind::Fn, did) = fun_path.res
&& lcx.tcx.is_diagnostic_item(sym::mem_drop, did)
&& let [first_arg, ..] = args
&& let hir::ExprKind::Path(hir::QPath::Resolved(_, arg_path)) = &first_arg.kind
&& let [first_arg_ps, .. ] = arg_path.segments
{
&first_arg_ps.ident == first_bind_ident
}
else {
false
let has_ident = |local_expr: &hir::Expr<'_>| {
if let hir::ExprKind::Path(hir::QPath::Resolved(_, arg_path)) = &local_expr.kind
&& let [first_arg_ps, .. ] = arg_path.segments
&& &first_arg_ps.ident == first_bind_ident
{
true
}
else {
false
}
};
if has_ident(first_arg) {
return true;
}
if let hir::ExprKind::Tup(value) = &first_arg.kind && value.iter().any(has_ident) {
return true;
}
}
false
}
fn is_expensive_expr(expr: &hir::Expr<'_>) -> bool {
!matches!(expr.kind, hir::ExprKind::Path(_))
fn is_inexpensive_expr(expr: &hir::Expr<'_>) -> bool {
let actual = peel_hir_expr_unary(expr).0;
let is_path = matches!(actual.kind, hir::ExprKind::Path(_));
let is_lit = matches!(actual.kind, hir::ExprKind::Lit(_));
is_path || is_lit
}

View file

@ -1,13 +1,13 @@
use clippy_utils::diagnostics::span_lint_and_then;
use clippy_utils::sugg::Sugg;
use clippy_utils::ty::is_type_diagnostic_item;
use clippy_utils::{
get_enclosing_block, is_integer_literal, is_path_diagnostic_item, path_to_local, path_to_local_id, SpanlessEq,
get_enclosing_block, is_expr_path_def_path, is_integer_literal, is_path_diagnostic_item, path_to_local,
path_to_local_id, paths, SpanlessEq,
};
use if_chain::if_chain;
use rustc_errors::Applicability;
use rustc_hir::intravisit::{walk_block, walk_expr, walk_stmt, Visitor};
use rustc_hir::{BindingAnnotation, Block, Expr, ExprKind, HirId, PatKind, QPath, Stmt, StmtKind};
use rustc_hir::{BindingAnnotation, Block, Expr, ExprKind, HirId, PatKind, Stmt, StmtKind};
use rustc_lint::{LateContext, LateLintPass};
use rustc_session::{declare_lint_pass, declare_tool_lint};
use rustc_span::symbol::sym;
@ -60,7 +60,24 @@ struct VecAllocation<'tcx> {
/// Reference to the expression used as argument on `with_capacity` call. This is used
/// to only match slow zero-filling idioms of the same length than vector initialization.
len_expr: &'tcx Expr<'tcx>,
size_expr: InitializedSize<'tcx>,
}
/// Initializer for the creation of the vector.
///
/// When `Vec::with_capacity(size)` is found, the `size` expression will be in
/// `InitializedSize::Initialized`.
///
/// Otherwise, for `Vec::new()` calls, there is no allocation initializer yet, so
/// `InitializedSize::Uninitialized` is used.
/// Later, when a call to `.resize(size, 0)` or similar is found, it's set
/// to `InitializedSize::Initialized(size)`.
///
/// Since it will be set to `InitializedSize::Initialized(size)` when a slow initialization is
/// found, it is always safe to "unwrap" it at lint time.
enum InitializedSize<'tcx> {
Initialized(&'tcx Expr<'tcx>),
Uninitialized,
}
/// Type of slow initialization
@ -77,18 +94,14 @@ impl<'tcx> LateLintPass<'tcx> for SlowVectorInit {
// Matches initialization on reassignments. For example: `vec = Vec::with_capacity(100)`
if_chain! {
if let ExprKind::Assign(left, right, _) = expr.kind;
// Extract variable
if let Some(local_id) = path_to_local(left);
// Extract len argument
if let Some(len_arg) = Self::is_vec_with_capacity(cx, right);
if let Some(size_expr) = Self::as_vec_initializer(cx, right);
then {
let vi = VecAllocation {
local_id,
allocation_expr: right,
len_expr: len_arg,
size_expr,
};
Self::search_initialization(cx, vi, expr.hir_id);
@ -98,17 +111,18 @@ impl<'tcx> LateLintPass<'tcx> for SlowVectorInit {
fn check_stmt(&mut self, cx: &LateContext<'tcx>, stmt: &'tcx Stmt<'_>) {
// Matches statements which initializes vectors. For example: `let mut vec = Vec::with_capacity(10)`
// or `Vec::new()`
if_chain! {
if let StmtKind::Local(local) = stmt.kind;
if let PatKind::Binding(BindingAnnotation::MUT, local_id, _, None) = local.pat.kind;
if let Some(init) = local.init;
if let Some(len_arg) = Self::is_vec_with_capacity(cx, init);
if let Some(size_expr) = Self::as_vec_initializer(cx, init);
then {
let vi = VecAllocation {
local_id,
allocation_expr: init,
len_expr: len_arg,
size_expr,
};
Self::search_initialization(cx, vi, stmt.hir_id);
@ -118,19 +132,20 @@ impl<'tcx> LateLintPass<'tcx> for SlowVectorInit {
}
impl SlowVectorInit {
/// Checks if the given expression is `Vec::with_capacity(..)`. It will return the expression
/// of the first argument of `with_capacity` call if it matches or `None` if it does not.
fn is_vec_with_capacity<'tcx>(cx: &LateContext<'_>, expr: &Expr<'tcx>) -> Option<&'tcx Expr<'tcx>> {
if_chain! {
if let ExprKind::Call(func, [arg]) = expr.kind;
if let ExprKind::Path(QPath::TypeRelative(ty, name)) = func.kind;
if name.ident.as_str() == "with_capacity";
if is_type_diagnostic_item(cx, cx.typeck_results().node_type(ty.hir_id), sym::Vec);
then {
Some(arg)
} else {
None
}
/// Looks for `Vec::with_capacity(size)` or `Vec::new()` calls and returns the initialized size,
/// if any. More specifically, it returns:
/// - `Some(InitializedSize::Initialized(size))` for `Vec::with_capacity(size)`
/// - `Some(InitializedSize::Uninitialized)` for `Vec::new()`
/// - `None` for other, unrelated kinds of expressions
fn as_vec_initializer<'tcx>(cx: &LateContext<'_>, expr: &'tcx Expr<'tcx>) -> Option<InitializedSize<'tcx>> {
if let ExprKind::Call(func, [len_expr]) = expr.kind
&& is_expr_path_def_path(cx, func, &paths::VEC_WITH_CAPACITY)
{
Some(InitializedSize::Initialized(len_expr))
} else if matches!(expr.kind, ExprKind::Call(func, _) if is_expr_path_def_path(cx, func, &paths::VEC_NEW)) {
Some(InitializedSize::Uninitialized)
} else {
None
}
}
@ -169,12 +184,19 @@ impl SlowVectorInit {
}
fn emit_lint(cx: &LateContext<'_>, slow_fill: &Expr<'_>, vec_alloc: &VecAllocation<'_>, msg: &str) {
let len_expr = Sugg::hir(cx, vec_alloc.len_expr, "len");
let len_expr = Sugg::hir(
cx,
match vec_alloc.size_expr {
InitializedSize::Initialized(expr) => expr,
InitializedSize::Uninitialized => unreachable!("size expression must be set by this point"),
},
"len",
);
span_lint_and_then(cx, SLOW_VECTOR_INITIALIZATION, slow_fill.span, msg, |diag| {
diag.span_suggestion(
vec_alloc.allocation_expr.span,
"consider replace allocation with",
"consider replacing this with",
format!("vec![0; {len_expr}]"),
Applicability::Unspecified,
);
@ -214,36 +236,45 @@ impl<'a, 'tcx> VectorInitializationVisitor<'a, 'tcx> {
}
/// Checks if the given expression is resizing a vector with 0
fn search_slow_resize_filling(&mut self, expr: &'tcx Expr<'_>) {
fn search_slow_resize_filling(&mut self, expr: &'tcx Expr<'tcx>) {
if self.initialization_found
&& let ExprKind::MethodCall(path, self_arg, [len_arg, fill_arg], _) = expr.kind
&& path_to_local_id(self_arg, self.vec_alloc.local_id)
&& path.ident.name == sym!(resize)
// Check that is filled with 0
&& is_integer_literal(fill_arg, 0) {
// Check that len expression is equals to `with_capacity` expression
if SpanlessEq::new(self.cx).eq_expr(len_arg, self.vec_alloc.len_expr) {
self.slow_expression = Some(InitializationType::Resize(expr));
} else if let ExprKind::MethodCall(path, ..) = len_arg.kind && path.ident.as_str() == "capacity" {
self.slow_expression = Some(InitializationType::Resize(expr));
}
&& is_integer_literal(fill_arg, 0)
{
let is_matching_resize = if let InitializedSize::Initialized(size_expr) = self.vec_alloc.size_expr {
// If we have a size expression, check that it is equal to what's passed to `resize`
SpanlessEq::new(self.cx).eq_expr(len_arg, size_expr)
|| matches!(len_arg.kind, ExprKind::MethodCall(path, ..) if path.ident.as_str() == "capacity")
} else {
self.vec_alloc.size_expr = InitializedSize::Initialized(len_arg);
true
};
if is_matching_resize {
self.slow_expression = Some(InitializationType::Resize(expr));
}
}
}
/// Returns `true` if give expression is `repeat(0).take(...)`
fn is_repeat_take(&self, expr: &Expr<'_>) -> bool {
fn is_repeat_take(&mut self, expr: &'tcx Expr<'tcx>) -> bool {
if_chain! {
if let ExprKind::MethodCall(take_path, recv, [len_arg, ..], _) = expr.kind;
if take_path.ident.name == sym!(take);
// Check that take is applied to `repeat(0)`
if self.is_repeat_zero(recv);
then {
// Check that len expression is equals to `with_capacity` expression
if SpanlessEq::new(self.cx).eq_expr(len_arg, self.vec_alloc.len_expr) {
return true;
} else if let ExprKind::MethodCall(path, ..) = len_arg.kind && path.ident.as_str() == "capacity" {
return true;
if let InitializedSize::Initialized(size_expr) = self.vec_alloc.size_expr {
// Check that len expression is equals to `with_capacity` expression
return SpanlessEq::new(self.cx).eq_expr(len_arg, size_expr)
|| matches!(len_arg.kind, ExprKind::MethodCall(path, ..) if path.ident.as_str() == "capacity")
}
self.vec_alloc.size_expr = InitializedSize::Initialized(len_arg);
return true;
}
}

View file

@ -1,21 +1,29 @@
use clippy_utils::diagnostics::span_lint_and_help;
use clippy_utils::msrvs::{self, Msrv};
use clippy_utils::visitors::for_each_local_use_after_expr;
use clippy_utils::{is_from_proc_macro, path_to_local};
use itertools::Itertools;
use rustc_ast::LitKind;
use rustc_hir::{Expr, ExprKind, HirId, Node, Pat};
use rustc_hir::{Expr, ExprKind, Node, PatKind};
use rustc_lint::{LateContext, LateLintPass, LintContext};
use rustc_middle::lint::in_external_macro;
use rustc_middle::ty;
use rustc_middle::ty::{self, Ty};
use rustc_session::{declare_tool_lint, impl_lint_pass};
use std::iter::once;
use std::ops::ControlFlow;
declare_clippy_lint! {
/// ### What it does
/// Checks for tuple<=>array conversions that are not done with `.into()`.
///
/// ### Why is this bad?
/// It may be unnecessary complexity. `.into()` works for converting tuples
/// <=> arrays of up to 12 elements and may convey intent more clearly.
/// It may be unnecessary complexity. `.into()` works for converting tuples<=> arrays of up to
/// 12 elements and conveys the intent more clearly, while also leaving less room for hard to
/// spot bugs!
///
/// ### Known issues
/// The suggested code may hide potential asymmetry in some cases. See
/// [#11085](https://github.com/rust-lang/rust-clippy/issues/11085) for more info.
///
/// ### Example
/// ```rust,ignore
@ -29,7 +37,7 @@ declare_clippy_lint! {
/// ```
#[clippy::version = "1.72.0"]
pub TUPLE_ARRAY_CONVERSIONS,
pedantic,
nursery,
"checks for tuple<=>array conversions that are not done with `.into()`"
}
impl_lint_pass!(TupleArrayConversions => [TUPLE_ARRAY_CONVERSIONS]);
@ -41,130 +49,152 @@ pub struct TupleArrayConversions {
impl LateLintPass<'_> for TupleArrayConversions {
fn check_expr<'tcx>(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) {
if !in_external_macro(cx.sess(), expr.span) && self.msrv.meets(msrvs::TUPLE_ARRAY_CONVERSIONS) {
match expr.kind {
ExprKind::Array(elements) if (1..=12).contains(&elements.len()) => check_array(cx, expr, elements),
ExprKind::Tup(elements) if (1..=12).contains(&elements.len()) => check_tuple(cx, expr, elements),
_ => {},
}
if in_external_macro(cx.sess(), expr.span) || !self.msrv.meets(msrvs::TUPLE_ARRAY_CONVERSIONS) {
return;
}
match expr.kind {
ExprKind::Array(elements) if (1..=12).contains(&elements.len()) => check_array(cx, expr, elements),
ExprKind::Tup(elements) if (1..=12).contains(&elements.len()) => check_tuple(cx, expr, elements),
_ => {},
}
}
extract_msrv_attr!(LateContext);
}
#[expect(
clippy::blocks_in_if_conditions,
reason = "not a FP, but this is much easier to understand"
)]
fn check_array<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>, elements: &'tcx [Expr<'tcx>]) {
if should_lint(
cx,
elements,
// This is cursed.
Some,
|(first_id, local)| {
if let Node::Pat(pat) = local
&& let parent = parent_pat(cx, pat)
&& parent.hir_id == first_id
{
return matches!(
cx.typeck_results().pat_ty(parent).peel_refs().kind(),
ty::Tuple(len) if len.len() == elements.len()
);
}
let (ty::Array(ty, _) | ty::Slice(ty)) = cx.typeck_results().expr_ty(expr).kind() else {
unreachable!("`expr` must be an array or slice due to `ExprKind::Array`");
};
false
},
) || should_lint(
cx,
elements,
|(i, expr)| {
if let ExprKind::Field(path, field) = expr.kind && field.as_str() == i.to_string() {
return Some((i, path));
};
None
},
|(first_id, local)| {
if let Node::Pat(pat) = local
&& let parent = parent_pat(cx, pat)
&& parent.hir_id == first_id
{
return matches!(
cx.typeck_results().pat_ty(parent).peel_refs().kind(),
ty::Tuple(len) if len.len() == elements.len()
);
}
false
},
) {
emit_lint(cx, expr, ToType::Array);
if let [first, ..] = elements
&& let Some(locals) = (match first.kind {
ExprKind::Field(_, _) => elements
.iter()
.enumerate()
.map(|(i, f)| -> Option<&'tcx Expr<'tcx>> {
let ExprKind::Field(lhs, ident) = f.kind else {
return None;
};
(ident.name.as_str() == i.to_string()).then_some(lhs)
})
.collect::<Option<Vec<_>>>(),
ExprKind::Path(_) => Some(elements.iter().collect()),
_ => None,
})
&& all_bindings_are_for_conv(cx, &[*ty], expr, elements, &locals, ToType::Array)
&& !is_from_proc_macro(cx, expr)
{
span_lint_and_help(
cx,
TUPLE_ARRAY_CONVERSIONS,
expr.span,
"it looks like you're trying to convert a tuple to an array",
None,
"use `.into()` instead, or `<[T; N]>::from` if type annotations are needed",
);
}
}
#[expect(
clippy::blocks_in_if_conditions,
reason = "not a FP, but this is much easier to understand"
)]
#[expect(clippy::cast_possible_truncation)]
fn check_tuple<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>, elements: &'tcx [Expr<'tcx>]) {
if should_lint(cx, elements, Some, |(first_id, local)| {
if let Node::Pat(pat) = local
&& let parent = parent_pat(cx, pat)
&& parent.hir_id == first_id
{
return matches!(
cx.typeck_results().pat_ty(parent).peel_refs().kind(),
ty::Array(_, len) if len.eval_target_usize(cx.tcx, cx.param_env) as usize == elements.len()
);
}
if let ty::Tuple(tys) = cx.typeck_results().expr_ty(expr).kind()
&& let [first, ..] = elements
// Fix #11100
&& tys.iter().all_equal()
&& let Some(locals) = (match first.kind {
ExprKind::Index(_, _) => elements
.iter()
.enumerate()
.map(|(i, i_expr)| -> Option<&'tcx Expr<'tcx>> {
if let ExprKind::Index(lhs, index) = i_expr.kind
&& let ExprKind::Lit(lit) = index.kind
&& let LitKind::Int(val, _) = lit.node
{
return (val == i as u128).then_some(lhs);
};
false
}) || should_lint(
cx,
elements,
|(i, expr)| {
if let ExprKind::Index(path, index) = expr.kind
&& let ExprKind::Lit(lit) = index.kind
&& let LitKind::Int(val, _) = lit.node
&& val as usize == i
{
return Some((i, path));
};
None
},
|(first_id, local)| {
if let Node::Pat(pat) = local
&& let parent = parent_pat(cx, pat)
&& parent.hir_id == first_id
{
return matches!(
cx.typeck_results().pat_ty(parent).peel_refs().kind(),
ty::Array(_, len) if len.eval_target_usize(cx.tcx, cx.param_env) as usize == elements.len()
);
}
false
},
) {
emit_lint(cx, expr, ToType::Tuple);
None
})
.collect::<Option<Vec<_>>>(),
ExprKind::Path(_) => Some(elements.iter().collect()),
_ => None,
})
&& all_bindings_are_for_conv(cx, tys, expr, elements, &locals, ToType::Tuple)
&& !is_from_proc_macro(cx, expr)
{
span_lint_and_help(
cx,
TUPLE_ARRAY_CONVERSIONS,
expr.span,
"it looks like you're trying to convert an array to a tuple",
None,
"use `.into()` instead, or `<(T0, T1, ..., Tn)>::from` if type annotations are needed",
);
}
}
/// Walks up the `Pat` until it's reached the final containing `Pat`.
fn parent_pat<'tcx>(cx: &LateContext<'tcx>, start: &'tcx Pat<'tcx>) -> &'tcx Pat<'tcx> {
let mut end = start;
for (_, node) in cx.tcx.hir().parent_iter(start.hir_id) {
if let Node::Pat(pat) = node {
end = pat;
} else {
break;
}
}
end
/// Checks that every binding in `elements` comes from the same parent `Pat` with the kind if there
/// is a parent `Pat`. Returns false in any of the following cases:
/// * `kind` does not match `pat.kind`
/// * one or more elements in `elements` is not a binding
/// * one or more bindings does not have the same parent `Pat`
/// * one or more bindings are used after `expr`
/// * the bindings do not all have the same type
#[expect(clippy::cast_possible_truncation)]
fn all_bindings_are_for_conv<'tcx>(
cx: &LateContext<'tcx>,
final_tys: &[Ty<'tcx>],
expr: &Expr<'_>,
elements: &[Expr<'_>],
locals: &[&Expr<'_>],
kind: ToType,
) -> bool {
let Some(locals) = locals.iter().map(|e| path_to_local(e)).collect::<Option<Vec<_>>>() else {
return false;
};
let Some(local_parents) = locals
.iter()
.map(|&l| cx.tcx.hir().find_parent(l))
.collect::<Option<Vec<_>>>()
else {
return false;
};
local_parents
.iter()
.map(|node| match node {
Node::Pat(pat) => kind.eq(&pat.kind).then_some(pat.hir_id),
Node::Local(l) => Some(l.hir_id),
_ => None,
})
.all_equal()
// Fix #11124, very convenient utils function! ❤️
&& locals
.iter()
.all(|&l| for_each_local_use_after_expr(cx, l, expr.hir_id, |_| ControlFlow::Break::<()>(())).is_continue())
&& local_parents.first().is_some_and(|node| {
let Some(ty) = match node {
Node::Pat(pat) => Some(pat.hir_id),
Node::Local(l) => Some(l.hir_id),
_ => None,
}
.map(|hir_id| cx.typeck_results().node_type(hir_id)) else {
return false;
};
match (kind, ty.kind()) {
// Ensure the final type and the original type have the same length, and that there
// is no implicit `&mut`<=>`&` anywhere (#11100). Bit ugly, I know, but it works.
(ToType::Array, ty::Tuple(tys)) => {
tys.len() == elements.len() && tys.iter().chain(final_tys.iter().copied()).all_equal()
},
(ToType::Tuple, ty::Array(ty, len)) => {
len.eval_target_usize(cx.tcx, cx.param_env) as usize == elements.len()
&& final_tys.iter().chain(once(ty)).all_equal()
},
_ => false,
}
})
}
#[derive(Clone, Copy)]
@ -173,61 +203,11 @@ enum ToType {
Tuple,
}
impl ToType {
fn msg(self) -> &'static str {
impl PartialEq<PatKind<'_>> for ToType {
fn eq(&self, other: &PatKind<'_>) -> bool {
match self {
ToType::Array => "it looks like you're trying to convert a tuple to an array",
ToType::Tuple => "it looks like you're trying to convert an array to a tuple",
}
}
fn help(self) -> &'static str {
match self {
ToType::Array => "use `.into()` instead, or `<[T; N]>::from` if type annotations are needed",
ToType::Tuple => "use `.into()` instead, or `<(T0, T1, ..., Tn)>::from` if type annotations are needed",
ToType::Array => matches!(other, PatKind::Tuple(_, _)),
ToType::Tuple => matches!(other, PatKind::Slice(_, _, _)),
}
}
}
fn emit_lint<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>, to_type: ToType) -> bool {
if !is_from_proc_macro(cx, expr) {
span_lint_and_help(
cx,
TUPLE_ARRAY_CONVERSIONS,
expr.span,
to_type.msg(),
None,
to_type.help(),
);
return true;
}
false
}
fn should_lint<'tcx>(
cx: &LateContext<'tcx>,
elements: &'tcx [Expr<'tcx>],
map: impl FnMut((usize, &'tcx Expr<'tcx>)) -> Option<(usize, &Expr<'_>)>,
predicate: impl FnMut((HirId, &Node<'tcx>)) -> bool,
) -> bool {
if let Some(elements) = elements
.iter()
.enumerate()
.map(map)
.collect::<Option<Vec<_>>>()
&& let Some(locals) = elements
.iter()
.map(|(_, element)| path_to_local(element).and_then(|local| cx.tcx.hir().find(local)))
.collect::<Option<Vec<_>>>()
&& let [first, rest @ ..] = &*locals
&& let Node::Pat(first_pat) = first
&& let parent = parent_pat(cx, first_pat).hir_id
&& rest.iter().chain(once(first)).map(|i| (parent, i)).all(predicate)
{
return true;
}
false
}

View file

@ -1,11 +1,12 @@
use clippy_utils::diagnostics::span_lint_and_then;
use clippy_utils::diagnostics::span_lint_hir_and_then;
use clippy_utils::is_def_id_trait_method;
use rustc_hir::def::DefKind;
use rustc_hir::intravisit::{walk_body, walk_expr, walk_fn, FnKind, Visitor};
use rustc_hir::{Body, Expr, ExprKind, FnDecl, YieldSource};
use rustc_hir::{Body, Expr, ExprKind, FnDecl, Node, YieldSource};
use rustc_lint::{LateContext, LateLintPass};
use rustc_middle::hir::nested_filter;
use rustc_session::{declare_lint_pass, declare_tool_lint};
use rustc_span::def_id::LocalDefId;
use rustc_session::{declare_tool_lint, impl_lint_pass};
use rustc_span::def_id::{LocalDefId, LocalDefIdSet};
use rustc_span::Span;
declare_clippy_lint! {
@ -38,7 +39,24 @@ declare_clippy_lint! {
"finds async functions with no await statements"
}
declare_lint_pass!(UnusedAsync => [UNUSED_ASYNC]);
#[derive(Default)]
pub struct UnusedAsync {
/// Keeps track of async functions used as values (i.e. path expressions to async functions that
/// are not immediately called)
async_fns_as_value: LocalDefIdSet,
/// Functions with unused `async`, linted post-crate after we've found all uses of local async
/// functions
unused_async_fns: Vec<UnusedAsyncFn>,
}
#[derive(Copy, Clone)]
struct UnusedAsyncFn {
def_id: LocalDefId,
fn_span: Span,
await_in_async_block: Option<Span>,
}
impl_lint_pass!(UnusedAsync => [UNUSED_ASYNC]);
struct AsyncFnVisitor<'a, 'tcx> {
cx: &'a LateContext<'tcx>,
@ -101,24 +119,70 @@ impl<'tcx> LateLintPass<'tcx> for UnusedAsync {
};
walk_fn(&mut visitor, fn_kind, fn_decl, body.id(), def_id);
if !visitor.found_await {
span_lint_and_then(
cx,
UNUSED_ASYNC,
span,
"unused `async` for function with no await statements",
|diag| {
diag.help("consider removing the `async` from this function");
if let Some(span) = visitor.await_in_async_block {
diag.span_note(
span,
"`await` used in an async block, which does not require \
the enclosing function to be `async`",
);
}
},
);
// Don't lint just yet, but store the necessary information for later.
// The actual linting happens in `check_crate_post`, once we've found all
// uses of local async functions that do require asyncness to pass typeck
self.unused_async_fns.push(UnusedAsyncFn {
await_in_async_block: visitor.await_in_async_block,
fn_span: span,
def_id,
});
}
}
}
fn check_path(&mut self, cx: &LateContext<'tcx>, path: &rustc_hir::Path<'tcx>, hir_id: rustc_hir::HirId) {
fn is_node_func_call(node: Node<'_>, expected_receiver: Span) -> bool {
matches!(
node,
Node::Expr(Expr {
kind: ExprKind::Call(Expr { span, .. }, _) | ExprKind::MethodCall(_, Expr { span, .. }, ..),
..
}) if *span == expected_receiver
)
}
// Find paths to local async functions that aren't immediately called.
// E.g. `async fn f() {}; let x = f;`
// Depending on how `x` is used, f's asyncness might be required despite not having any `await`
// statements, so don't lint at all if there are any such paths.
if let Some(def_id) = path.res.opt_def_id()
&& let Some(local_def_id) = def_id.as_local()
&& let Some(DefKind::Fn) = cx.tcx.opt_def_kind(def_id)
&& cx.tcx.asyncness(def_id).is_async()
&& !is_node_func_call(cx.tcx.hir().get_parent(hir_id), path.span)
{
self.async_fns_as_value.insert(local_def_id);
}
}
// After collecting all unused `async` and problematic paths to such functions,
// lint those unused ones that didn't have any path expressions to them.
fn check_crate_post(&mut self, cx: &LateContext<'tcx>) {
let iter = self
.unused_async_fns
.iter()
.filter(|UnusedAsyncFn { def_id, .. }| (!self.async_fns_as_value.contains(def_id)));
for fun in iter {
span_lint_hir_and_then(
cx,
UNUSED_ASYNC,
cx.tcx.local_def_id_to_hir_id(fun.def_id),
fun.fn_span,
"unused `async` for function with no await statements",
|diag| {
diag.help("consider removing the `async` from this function");
if let Some(span) = fun.await_in_async_block {
diag.span_note(
span,
"`await` used in an async block, which does not require \
the enclosing function to be `async`",
);
}
},
);
}
}
}

View file

@ -551,6 +551,16 @@ define_Conf! {
///
/// Whether to allow `r#""#` when `r""` can be used
(allow_one_hash_in_raw_strings: bool = false),
/// Lint: ABSOLUTE_PATHS.
///
/// The maximum number of segments a path can have before being linted, anything above this will
/// be linted.
(absolute_paths_max_segments: u64 = 2),
/// Lint: ABSOLUTE_PATHS.
///
/// Which crates to allow absolute paths from
(absolute_paths_allowed_crates: rustc_data_structures::fx::FxHashSet<String> =
rustc_data_structures::fx::FxHashSet::default()),
}
/// Search for the configuration file.

View file

@ -18,7 +18,7 @@ const BOOK_CONFIGS_PATH: &str = "https://doc.rust-lang.org/clippy/lint_configura
// ==================================================================
// Configuration
// ==================================================================
#[derive(Debug, Clone, Default)] //~ ERROR no such field
#[derive(Debug, Clone, Default)]
pub struct ClippyConfiguration {
pub name: String,
#[allow(dead_code)]