user0/rust
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

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

Browse source
This commit is contained in:
Philipp Krones 2023-09-07 21:43:06 +02:00
commit d2b08432db
No known key found for this signature in database
GPG key ID: 1CA0DF2AF59D68A5
158 changed files with 3894 additions and 1449 deletions
Download patch file Download diff file Expand all files Collapse all files

2
clippy_lints/src/declared_lints.rs
View file

@ -365,6 +365,7 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[
crate::methods::ITER_NTH_ZERO_INFO,
crate::methods::ITER_ON_EMPTY_COLLECTIONS_INFO,
crate::methods::ITER_ON_SINGLE_ITEMS_INFO,
crate::methods::ITER_OUT_OF_BOUNDS_INFO,
crate::methods::ITER_OVEREAGER_CLONED_INFO,
crate::methods::ITER_SKIP_NEXT_INFO,
crate::methods::ITER_SKIP_ZERO_INFO,
@ -456,6 +457,7 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[
crate::misc_early::ZERO_PREFIXED_LITERAL_INFO,
crate::mismatching_type_param_order::MISMATCHING_TYPE_PARAM_ORDER_INFO,
crate::missing_assert_message::MISSING_ASSERT_MESSAGE_INFO,
crate::missing_asserts_for_indexing::MISSING_ASSERTS_FOR_INDEXING_INFO,
crate::missing_const_for_fn::MISSING_CONST_FOR_FN_INFO,
crate::missing_doc::MISSING_DOCS_IN_PRIVATE_ITEMS_INFO,
crate::missing_enforced_import_rename::MISSING_ENFORCED_IMPORT_RENAMES_INFO,

3
clippy_lints/src/default_union_representation.rs
View file

@ -69,6 +69,9 @@ impl<'tcx> LateLintPass<'tcx> for DefaultUnionRepresentation {
}
/// Returns true if the given item is a union with at least two non-ZST fields.
/// (ZST fields having an arbitrary offset is completely inconsequential, and
/// if there is only one field left after ignoring ZST fields then the offset
/// of that field does not matter either.)
fn is_union_with_two_non_zst_fields(cx: &LateContext<'_>, item: &Item<'_>) -> bool {
if let ItemKind::Union(data, _) = &item.kind {
data.fields().iter().filter(|f| !is_zst(cx, f.ty)).count() >= 2

24
clippy_lints/src/dereference.rs
View file

@ -3,7 +3,7 @@ use clippy_utils::mir::{enclosing_mir, expr_local, local_assignments, used_exact
use clippy_utils::msrvs::{self, Msrv};
use clippy_utils::source::{snippet_with_applicability, snippet_with_context};
use clippy_utils::sugg::has_enclosing_paren;
use clippy_utils::ty::{is_copy, peel_mid_ty_refs};
use clippy_utils::ty::{implements_trait, is_copy, peel_mid_ty_refs};
use clippy_utils::{
expr_use_ctxt, get_parent_expr, get_parent_node, is_lint_allowed, path_to_local, DefinedTy, ExprUseNode,
};
@ -33,7 +33,6 @@ use rustc_middle::ty::{
use rustc_session::{declare_tool_lint, impl_lint_pass};
use rustc_span::symbol::sym;
use rustc_span::{Span, Symbol};
use rustc_trait_selection::infer::InferCtxtExt as _;
use rustc_trait_selection::traits::query::evaluate_obligation::InferCtxtExt as _;
use rustc_trait_selection::traits::{Obligation, ObligationCause};
use std::collections::VecDeque;
@ -452,13 +451,12 @@ impl<'tcx> LateLintPass<'tcx> for Dereferencing<'tcx> {
// Trait methods taking `self`
arg_ty
} && impl_ty.is_ref()
&& cx.tcx.infer_ctxt().build()
.type_implements_trait(
trait_id,
[impl_ty.into()].into_iter().chain(args.iter().copied()),
cx.param_env,
)
.must_apply_modulo_regions()
&& implements_trait(
cx,
impl_ty,
trait_id,
&args[..cx.tcx.generics_of(trait_id).params.len() - 1],
)
{
false
} else {
@ -609,12 +607,14 @@ impl<'tcx> LateLintPass<'tcx> for Dereferencing<'tcx> {
adjusted_ty,
},
));
} else if stability.is_deref_stable() {
} else if stability.is_deref_stable()
&& let Some(parent) = get_parent_expr(cx, expr)
{
self.state = Some((
State::ExplicitDeref { mutability: None },
StateData {
span: expr.span,
hir_id: expr.hir_id,
span: parent.span,
hir_id: parent.hir_id,
adjusted_ty,
},
));

3
clippy_lints/src/derivable_impls.rs
View file

@ -217,8 +217,7 @@ impl<'tcx> LateLintPass<'tcx> for DerivableImpls {
if let &Adt(adt_def, args) = cx.tcx.type_of(item.owner_id).instantiate_identity().kind();
if let attrs = cx.tcx.hir().attrs(item.hir_id());
if !attrs.iter().any(|attr| attr.doc_str().is_some());
if let child_attrs = cx.tcx.hir().attrs(impl_item_hir);
if !child_attrs.iter().any(|attr| attr.doc_str().is_some());
if cx.tcx.hir().attrs(impl_item_hir).is_empty();
then {
if adt_def.is_struct() {

11
clippy_lints/src/disallowed_macros.rs
View file

@ -1,8 +1,9 @@
use clippy_utils::diagnostics::span_lint_and_then;
use clippy_utils::macros::macro_backtrace;
use rustc_ast::Attribute;
use rustc_data_structures::fx::FxHashSet;
use rustc_hir::def_id::DefIdMap;
use rustc_hir::{Expr, ForeignItem, HirId, ImplItem, Item, Pat, Path, Stmt, TraitItem, Ty};
use rustc_hir::{Expr, ExprKind, ForeignItem, HirId, ImplItem, Item, Pat, Path, Stmt, TraitItem, Ty};
use rustc_lint::{LateContext, LateLintPass};
use rustc_session::{declare_tool_lint, impl_lint_pass};
use rustc_span::{ExpnId, Span};
@ -111,6 +112,10 @@ impl LateLintPass<'_> for DisallowedMacros {
fn check_expr(&mut self, cx: &LateContext<'_>, expr: &Expr<'_>) {
self.check(cx, expr.span);
// `$t + $t` can have the context of $t, check also the span of the binary operator
if let ExprKind::Binary(op, ..) = expr.kind {
self.check(cx, op.span);
}
}
fn check_stmt(&mut self, cx: &LateContext<'_>, stmt: &Stmt<'_>) {
@ -147,4 +152,8 @@ impl LateLintPass<'_> for DisallowedMacros {
fn check_path(&mut self, cx: &LateContext<'_>, path: &Path<'_>, _: HirId) {
self.check(cx, path.span);
}
fn check_attribute(&mut self, cx: &LateContext<'_>, attr: &Attribute) {
self.check(cx, attr.span);
}
}

2
clippy_lints/src/doc.rs
View file

@ -508,7 +508,7 @@ struct DocHeaders {
fn check_attrs(cx: &LateContext<'_>, valid_idents: &FxHashSet<String>, attrs: &[Attribute]) -> Option<DocHeaders> {
/// We don't want the parser to choke on intra doc links. Since we don't
/// actually care about rendering them, just pretend that all broken links are
/// actually care about rendering them, just pretend that all broken links
/// point to a fake address.
#[expect(clippy::unnecessary_wraps)] // we're following a type signature
fn fake_broken_link_callback<'a>(_: BrokenLink<'_>) -> Option<(CowStr<'a>, CowStr<'a>)> {

18
clippy_lints/src/if_then_some_else_none.rs
View file

@ -6,7 +6,7 @@ use clippy_utils::sugg::Sugg;
use clippy_utils::{contains_return, higher, is_else_clause, is_res_lang_ctor, path_res, peel_blocks};
use rustc_errors::Applicability;
use rustc_hir::LangItem::{OptionNone, OptionSome};
use rustc_hir::{Expr, ExprKind, Stmt, StmtKind};
use rustc_hir::{Expr, ExprKind};
use rustc_lint::{LateContext, LateLintPass, LintContext};
use rustc_middle::lint::in_external_macro;
use rustc_session::{declare_tool_lint, impl_lint_pass};
@ -83,7 +83,7 @@ impl<'tcx> LateLintPass<'tcx> for IfThenSomeElseNone {
&& then_expr.span.ctxt() == ctxt
&& is_res_lang_ctor(cx, path_res(cx, then_call), OptionSome)
&& is_res_lang_ctor(cx, path_res(cx, peel_blocks(els)), OptionNone)
&& !stmts_contains_early_return(then_block.stmts)
&& !contains_return(then_block.stmts)
{
let mut app = Applicability::Unspecified;
let cond_snip = Sugg::hir_with_context(cx, cond, expr.span.ctxt(), "[condition]", &mut app).maybe_par().to_string();
@ -116,17 +116,3 @@ impl<'tcx> LateLintPass<'tcx> for IfThenSomeElseNone {
extract_msrv_attr!(LateContext);
}
fn stmts_contains_early_return(stmts: &[Stmt<'_>]) -> bool {
stmts.iter().any(|stmt| {
let Stmt {
kind: StmtKind::Semi(e),
..
} = stmt
else {
return false;
};
contains_return(e)
})
}

13
clippy_lints/src/ignored_unit_patterns.rs
View file

@ -1,5 +1,5 @@
use clippy_utils::diagnostics::span_lint_and_sugg;
use hir::PatKind;
use hir::{Node, PatKind};
use rustc_errors::Applicability;
use rustc_hir as hir;
use rustc_lint::{LateContext, LateLintPass};
@ -37,6 +37,17 @@ declare_lint_pass!(IgnoredUnitPatterns => [IGNORED_UNIT_PATTERNS]);
impl<'tcx> LateLintPass<'tcx> for IgnoredUnitPatterns {
fn check_pat(&mut self, cx: &LateContext<'tcx>, pat: &'tcx hir::Pat<'tcx>) {
match cx.tcx.hir().get_parent(pat.hir_id) {
Node::Param(param) if matches!(cx.tcx.hir().get_parent(param.hir_id), Node::Item(_)) => {
// Ignore function parameters
return;
},
Node::Local(local) if local.ty.is_some() => {
// Ignore let bindings with explicit type
return;
},
_ => {},
}
if matches!(pat.kind, PatKind::Wild) && cx.typeck_results().pat_ty(pat).is_unit() {
span_lint_and_sugg(
cx,

117
clippy_lints/src/implied_bounds_in_impls.rs
View file

@ -1,7 +1,7 @@
use clippy_utils::diagnostics::span_lint_and_then;
use clippy_utils::source::snippet;
use rustc_errors::{Applicability, SuggestionStyle};
use rustc_hir::def_id::LocalDefId;
use rustc_hir::def_id::{DefId, LocalDefId};
use rustc_hir::intravisit::FnKind;
use rustc_hir::{
Body, FnDecl, FnRetTy, GenericArg, GenericBound, ImplItem, ImplItemKind, ItemKind, TraitBoundModifier, TraitItem,
@ -9,7 +9,7 @@ use rustc_hir::{
};
use rustc_hir_analysis::hir_ty_to_ty;
use rustc_lint::{LateContext, LateLintPass};
use rustc_middle::ty::{self, ClauseKind, TyCtxt};
use rustc_middle::ty::{self, ClauseKind, Generics, Ty, TyCtxt};
use rustc_session::{declare_lint_pass, declare_tool_lint};
use rustc_span::Span;
@ -45,52 +45,80 @@ declare_clippy_lint! {
/// ```
#[clippy::version = "1.73.0"]
pub IMPLIED_BOUNDS_IN_IMPLS,
complexity,
nursery,
"specifying bounds that are implied by other bounds in `impl Trait` type"
}
declare_lint_pass!(ImpliedBoundsInImpls => [IMPLIED_BOUNDS_IN_IMPLS]);
/// This function tries to, for all type parameters in a supertype predicate `GenericTrait<U>`,
/// check if the substituted type in the implied-by bound matches with what's subtituted in the
/// implied type.
/// Tries to "resolve" a type.
/// The index passed to this function must start with `Self=0`, i.e. it must be a valid
/// type parameter index.
/// If the index is out of bounds, it means that the generic parameter has a default type.
fn try_resolve_type<'tcx>(
tcx: TyCtxt<'tcx>,
args: &'tcx [GenericArg<'tcx>],
generics: &'tcx Generics,
index: usize,
) -> Option<Ty<'tcx>> {
match args.get(index - 1) {
Some(GenericArg::Type(ty)) => Some(hir_ty_to_ty(tcx, ty)),
Some(_) => None,
None => Some(tcx.type_of(generics.params[index].def_id).skip_binder()),
}
}
/// This function tries to, for all generic type parameters in a supertrait predicate `trait ...<U>:
/// GenericTrait<U>`, check if the substituted type in the implied-by bound matches with what's
/// subtituted in the implied bound.
///
/// Consider this example.
/// ```rust,ignore
/// trait GenericTrait<T> {}
/// trait GenericSubTrait<T, U, V>: GenericTrait<U> {}
/// ^ trait_predicate_args: [Self#0, U#2]
/// ^^^^^^^^^^^^^^^ trait_predicate_args: [Self#0, U#2]
/// (the Self#0 is implicit: `<Self as GenericTrait<U>>`)
/// impl GenericTrait<i32> for () {}
/// impl GenericSubTrait<(), i32, ()> for () {}
/// impl GenericSubTrait<(), [u8; 8], ()> for () {}
/// impl GenericSubTrait<(), i64, ()> for () {}
///
/// fn f() -> impl GenericTrait<i32> + GenericSubTrait<(), [u8; 8], ()> {
/// ^^^ implied_args ^^^^^^^^^^^^^^^ implied_by_args
/// (we are interested in `[u8; 8]` specifically, as that
/// is what `U` in `GenericTrait<U>` is substituted with)
/// ()
/// fn f() -> impl GenericTrait<i32> + GenericSubTrait<(), i64, ()> {
/// ^^^ implied_args ^^^^^^^^^^^ implied_by_args
/// (we are interested in `i64` specifically, as that
/// is what `U` in `GenericTrait<U>` is substituted with)
/// }
/// ```
/// Here i32 != [u8; 8], so this will return false.
fn is_same_generics(
tcx: TyCtxt<'_>,
trait_predicate_args: &[ty::GenericArg<'_>],
implied_by_args: &[GenericArg<'_>],
implied_args: &[GenericArg<'_>],
/// Here i32 != i64, so this will return false.
fn is_same_generics<'tcx>(
tcx: TyCtxt<'tcx>,
trait_predicate_args: &'tcx [ty::GenericArg<'tcx>],
implied_by_args: &'tcx [GenericArg<'tcx>],
implied_args: &'tcx [GenericArg<'tcx>],
implied_by_def_id: DefId,
implied_def_id: DefId,
) -> bool {
// Get the generics of the two traits to be able to get default generic parameter.
let implied_by_generics = tcx.generics_of(implied_by_def_id);
let implied_generics = tcx.generics_of(implied_def_id);
trait_predicate_args
.iter()
.enumerate()
.skip(1) // skip `Self` implicit arg
.all(|(arg_index, arg)| {
if let Some(ty) = arg.as_type()
&& let &ty::Param(ty::ParamTy{ index, .. }) = ty.kind()
// Since `trait_predicate_args` and type params in traits start with `Self=0`
// and generic argument lists `GenericTrait<i32>` don't have `Self`,
// we need to subtract 1 from the index.
&& let GenericArg::Type(ty_a) = implied_by_args[index as usize - 1]
&& let GenericArg::Type(ty_b) = implied_args[arg_index - 1]
{
hir_ty_to_ty(tcx, ty_a) == hir_ty_to_ty(tcx, ty_b)
if let Some(ty) = arg.as_type() {
if let &ty::Param(ty::ParamTy { index, .. }) = ty.kind()
// `index == 0` means that it's referring to `Self`,
// in which case we don't try to substitute it
&& index != 0
&& let Some(ty_a) = try_resolve_type(tcx, implied_by_args, implied_by_generics, index as usize)
&& let Some(ty_b) = try_resolve_type(tcx, implied_args, implied_generics, arg_index)
{
ty_a == ty_b
} else if let Some(ty_b) = try_resolve_type(tcx, implied_args, implied_generics, arg_index) {
ty == ty_b
} else {
false
}
} else {
false
}
@ -121,7 +149,7 @@ fn check(cx: &LateContext<'_>, decl: &FnDecl<'_>) {
&& let predicates = cx.tcx.super_predicates_of(trait_def_id).predicates
&& !predicates.is_empty() // If the trait has no supertrait, there is nothing to add.
{
Some((bound.span(), path.args.map_or([].as_slice(), |a| a.args), predicates))
Some((bound.span(), path.args.map_or([].as_slice(), |a| a.args), predicates, trait_def_id))
} else {
None
}
@ -135,18 +163,27 @@ fn check(cx: &LateContext<'_>, decl: &FnDecl<'_>) {
&& let [.., path] = poly_trait.trait_ref.path.segments
&& let implied_args = path.args.map_or([].as_slice(), |a| a.args)
&& let Some(def_id) = poly_trait.trait_ref.path.res.opt_def_id()
&& let Some(implied_by_span) = implied_bounds.iter().find_map(|&(span, implied_by_args, preds)| {
preds.iter().find_map(|(clause, _)| {
if let ClauseKind::Trait(tr) = clause.kind().skip_binder()
&& tr.def_id() == def_id
&& is_same_generics(cx.tcx, tr.trait_ref.args, implied_by_args, implied_args)
{
Some(span)
} else {
None
}
&& let Some(implied_by_span) = implied_bounds
.iter()
.find_map(|&(span, implied_by_args, preds, implied_by_def_id)| {
preds.iter().find_map(|(clause, _)| {
if let ClauseKind::Trait(tr) = clause.kind().skip_binder()
&& tr.def_id() == def_id
&& is_same_generics(
cx.tcx,
tr.trait_ref.args,
implied_by_args,
implied_args,
implied_by_def_id,
def_id,
)
{
Some(span)
} else {
None
}
})
})
})
{
let implied_by = snippet(cx, implied_by_span, "..");
span_lint_and_then(

10
clippy_lints/src/incorrect_impls.rs
View file

@ -5,6 +5,7 @@ use clippy_utils::{get_parent_node, is_res_lang_ctor, last_path_segment, match_d
use rustc_errors::Applicability;
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};
@ -125,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 {
@ -188,12 +189,7 @@ impl LateLintPass<'_> for IncorrectImpls {
.diagnostic_items(trait_impl.def_id.krate)
.name_to_id
.get(&sym::Ord)
&& implements_trait(
cx,
trait_impl.self_ty(),
*ord_def_id,
&[],
)
&& implements_trait(cx, hir_ty_to_ty(cx.tcx, imp.self_ty), *ord_def_id, &[])
{
// 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

5
clippy_lints/src/lib.rs
View file

@ -210,6 +210,7 @@ mod misc;
mod misc_early;
mod mismatching_type_param_order;
mod missing_assert_message;
mod missing_asserts_for_indexing;
mod missing_const_for_fn;
mod missing_doc;
mod missing_enforced_import_rename;
@ -695,7 +696,8 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
});
store.register_late_pass(|_| Box::<shadow::Shadow>::default());
store.register_late_pass(|_| Box::new(unit_types::UnitTypes));
store.register_late_pass(move |_| Box::new(loops::Loops::new(msrv())));
let enforce_iter_loop_reborrow = conf.enforce_iter_loop_reborrow;
store.register_late_pass(move |_| Box::new(loops::Loops::new(msrv(), enforce_iter_loop_reborrow)));
store.register_late_pass(|_| Box::<main_recursion::MainRecursion>::default());
store.register_late_pass(|_| Box::new(lifetimes::Lifetimes));
store.register_late_pass(|_| Box::new(entry::HashMapPass));
@ -1099,6 +1101,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
store.register_late_pass(|_| Box::new(ignored_unit_patterns::IgnoredUnitPatterns));
store.register_late_pass(|_| Box::<reserve_after_initialization::ReserveAfterInitialization>::default());
store.register_late_pass(|_| Box::new(implied_bounds_in_impls::ImpliedBoundsInImpls));
store.register_late_pass(|_| Box::new(missing_asserts_for_indexing::MissingAssertsForIndexing));
// add lints here, do not remove this comment, it's used in `new_lint`
}

17
clippy_lints/src/loops/explicit_iter_loop.rs
View file

@ -13,8 +13,14 @@ use rustc_middle::ty::adjustment::{Adjust, Adjustment, AutoBorrow, AutoBorrowMut
use rustc_middle::ty::{self, EarlyBinder, Ty, TypeAndMut};
use rustc_span::sym;
pub(super) fn check(cx: &LateContext<'_>, self_arg: &Expr<'_>, call_expr: &Expr<'_>, msrv: &Msrv) {
let Some((adjust, ty)) = is_ref_iterable(cx, self_arg, call_expr) else {
pub(super) fn check(
cx: &LateContext<'_>,
self_arg: &Expr<'_>,
call_expr: &Expr<'_>,
msrv: &Msrv,
enforce_iter_loop_reborrow: bool,
) {
let Some((adjust, ty)) = is_ref_iterable(cx, self_arg, call_expr, enforce_iter_loop_reborrow) else {
return;
};
if let ty::Array(_, count) = *ty.peel_refs().kind() {
@ -102,6 +108,7 @@ fn is_ref_iterable<'tcx>(
cx: &LateContext<'tcx>,
self_arg: &Expr<'_>,
call_expr: &Expr<'_>,
enforce_iter_loop_reborrow: bool,
) -> Option<(AdjustKind, Ty<'tcx>)> {
let typeck = cx.typeck_results();
if let Some(trait_id) = cx.tcx.get_diagnostic_item(sym::IntoIterator)
@ -142,7 +149,8 @@ fn is_ref_iterable<'tcx>(
{
return Some((AdjustKind::None, self_ty));
}
} else if let ty::Ref(region, ty, Mutability::Mut) = *self_ty.kind()
} else if enforce_iter_loop_reborrow
&& let ty::Ref(region, ty, Mutability::Mut) = *self_ty.kind()
&& let Some(mutbl) = mutbl
{
// Attempt to reborrow the mutable reference
@ -186,7 +194,8 @@ fn is_ref_iterable<'tcx>(
},
..
] => {
if target != self_ty
if enforce_iter_loop_reborrow
&& target != self_ty
&& implements_trait(cx, target, trait_id, &[])
&& let Some(ty) =
make_normalized_projection(cx.tcx, cx.param_env, trait_id, sym!(IntoIter), [target])

10
clippy_lints/src/loops/mod.rs
View file

@ -609,10 +609,14 @@ declare_clippy_lint! {
pub struct Loops {
msrv: Msrv,
enforce_iter_loop_reborrow: bool,
}
impl Loops {
pub fn new(msrv: Msrv) -> Self {
Self { msrv }
pub fn new(msrv: Msrv, enforce_iter_loop_reborrow: bool) -> Self {
Self {
msrv,
enforce_iter_loop_reborrow,
}
}
}
impl_lint_pass!(Loops => [
@ -719,7 +723,7 @@ impl Loops {
if let ExprKind::MethodCall(method, self_arg, [], _) = arg.kind {
match method.ident.as_str() {
"iter" | "iter_mut" => {
explicit_iter_loop::check(cx, self_arg, arg, &self.msrv);
explicit_iter_loop::check(cx, self_arg, arg, &self.msrv, self.enforce_iter_loop_reborrow);
},
"into_iter" => {
explicit_into_iter_loop::check(cx, self_arg, arg);

268
clippy_lints/src/loops/never_loop.rs
View file

@ -1,13 +1,13 @@
use super::utils::make_iterator_snippet;
use super::NEVER_LOOP;
use clippy_utils::consts::{constant, Constant};
use clippy_utils::diagnostics::span_lint_and_then;
use clippy_utils::higher::ForLoop;
use clippy_utils::macros::root_macro_call_first_node;
use clippy_utils::source::snippet;
use rustc_errors::Applicability;
use rustc_hir::{Block, Destination, Expr, ExprKind, HirId, InlineAsmOperand, Pat, Stmt, StmtKind};
use rustc_lint::LateContext;
use rustc_span::Span;
use rustc_span::{sym, Span};
use std::iter::{once, Iterator};
pub(super) fn check<'tcx>(
@ -18,7 +18,7 @@ pub(super) fn check<'tcx>(
for_loop: Option<&ForLoop<'_>>,
) {
match never_loop_block(cx, block, &mut Vec::new(), loop_id) {
NeverLoopResult::AlwaysBreak => {
NeverLoopResult::Diverging => {
span_lint_and_then(cx, NEVER_LOOP, span, "this loop never actually loops", |diag| {
if let Some(ForLoop {
arg: iterator,
@ -39,67 +39,76 @@ pub(super) fn check<'tcx>(
}
});
},
NeverLoopResult::MayContinueMainLoop | NeverLoopResult::Otherwise => (),
NeverLoopResult::IgnoreUntilEnd(_) => unreachable!(),
NeverLoopResult::MayContinueMainLoop | NeverLoopResult::Normal => (),
}
}
/// The `never_loop` analysis keeps track of three things:
///
/// * Has any (reachable) code path hit a `continue` of the main loop?
/// * Is the current code path diverging (that is, the next expression is not reachable)
/// * For each block label `'a` inside the main loop, has any (reachable) code path encountered a
/// `break 'a`?
///
/// The first two bits of information are in this enum, and the last part is in the
/// `local_labels` variable, which contains a list of `(block_id, reachable)` pairs ordered by
/// scope.
#[derive(Copy, Clone)]
enum NeverLoopResult {
// A break/return always get triggered but not necessarily for the main loop.
AlwaysBreak,
// A continue may occur for the main loop.
/// A continue may occur for the main loop.
MayContinueMainLoop,
// Ignore everything until the end of the block with this id
IgnoreUntilEnd(HirId),
Otherwise,
/// We have not encountered any main loop continue,
/// but we are diverging (subsequent control flow is not reachable)
Diverging,
/// We have not encountered any main loop continue,
/// and subsequent control flow is (possibly) reachable
Normal,
}
#[must_use]
fn absorb_break(arg: NeverLoopResult) -> NeverLoopResult {
match arg {
NeverLoopResult::AlwaysBreak | NeverLoopResult::Otherwise => NeverLoopResult::Otherwise,
NeverLoopResult::Diverging | NeverLoopResult::Normal => NeverLoopResult::Normal,
NeverLoopResult::MayContinueMainLoop => NeverLoopResult::MayContinueMainLoop,
NeverLoopResult::IgnoreUntilEnd(id) => NeverLoopResult::IgnoreUntilEnd(id),
}
}
// Combine two results for parts that are called in order.
#[must_use]
fn combine_seq(first: NeverLoopResult, second: NeverLoopResult) -> NeverLoopResult {
fn combine_seq(first: NeverLoopResult, second: impl FnOnce() -> NeverLoopResult) -> NeverLoopResult {
match first {
NeverLoopResult::AlwaysBreak | NeverLoopResult::MayContinueMainLoop | NeverLoopResult::IgnoreUntilEnd(_) => {
first
},
NeverLoopResult::Otherwise => second,
NeverLoopResult::Diverging | NeverLoopResult::MayContinueMainLoop => first,
NeverLoopResult::Normal => second(),
}
}
// Combine an iterator of results for parts that are called in order.
#[must_use]
fn combine_seq_many(iter: impl IntoIterator<Item = NeverLoopResult>) -> NeverLoopResult {
for e in iter {
if let NeverLoopResult::Diverging | NeverLoopResult::MayContinueMainLoop = e {
return e;
}
}
NeverLoopResult::Normal
}
// Combine two results where only one of the part may have been executed.
#[must_use]
fn combine_branches(b1: NeverLoopResult, b2: NeverLoopResult, ignore_ids: &[HirId]) -> NeverLoopResult {
fn combine_branches(b1: NeverLoopResult, b2: NeverLoopResult) -> NeverLoopResult {
match (b1, b2) {
(NeverLoopResult::IgnoreUntilEnd(a), NeverLoopResult::IgnoreUntilEnd(b)) => {
if ignore_ids.iter().find(|&e| e == &a || e == &b).unwrap() == &a {
NeverLoopResult::IgnoreUntilEnd(b)
} else {
NeverLoopResult::IgnoreUntilEnd(a)
}
},
(i @ NeverLoopResult::IgnoreUntilEnd(_), NeverLoopResult::AlwaysBreak)
| (NeverLoopResult::AlwaysBreak, i @ NeverLoopResult::IgnoreUntilEnd(_)) => i,
(NeverLoopResult::AlwaysBreak, NeverLoopResult::AlwaysBreak) => NeverLoopResult::AlwaysBreak,
(NeverLoopResult::MayContinueMainLoop, _) | (_, NeverLoopResult::MayContinueMainLoop) => {
NeverLoopResult::MayContinueMainLoop
},
(NeverLoopResult::Otherwise, _) | (_, NeverLoopResult::Otherwise) => NeverLoopResult::Otherwise,
(NeverLoopResult::Normal, _) | (_, NeverLoopResult::Normal) => NeverLoopResult::Normal,
(NeverLoopResult::Diverging, NeverLoopResult::Diverging) => NeverLoopResult::Diverging,
}
}
fn never_loop_block<'tcx>(
cx: &LateContext<'tcx>,
block: &Block<'tcx>,
ignore_ids: &mut Vec<HirId>,
local_labels: &mut Vec<(HirId, bool)>,
main_loop_id: HirId,
) -> NeverLoopResult {
let iter = block
@ -107,15 +116,21 @@ fn never_loop_block<'tcx>(
.iter()
.filter_map(stmt_to_expr)
.chain(block.expr.map(|expr| (expr, None)));
iter.map(|(e, els)| {
let e = never_loop_expr(cx, e, ignore_ids, main_loop_id);
combine_seq_many(iter.map(|(e, els)| {
let e = never_loop_expr(cx, e, local_labels, main_loop_id);
// els is an else block in a let...else binding
els.map_or(e, |els| {
combine_branches(e, never_loop_block(cx, els, ignore_ids, main_loop_id), ignore_ids)
combine_seq(e, || match never_loop_block(cx, els, local_labels, main_loop_id) {
// Returning MayContinueMainLoop here means that
// we will not evaluate the rest of the body
NeverLoopResult::MayContinueMainLoop => NeverLoopResult::MayContinueMainLoop,
// An else block always diverges, so the Normal case should not happen,
// but the analysis is approximate so it might return Normal anyway.
// Returning Normal here says that nothing more happens on the main path
NeverLoopResult::Diverging | NeverLoopResult::Normal => NeverLoopResult::Normal,
})
})
})
.fold(NeverLoopResult::Otherwise, combine_seq)
}))
}
fn stmt_to_expr<'tcx>(stmt: &Stmt<'tcx>) -> Option<(&'tcx Expr<'tcx>, Option<&'tcx Block<'tcx>>)> {
@ -131,76 +146,69 @@ fn stmt_to_expr<'tcx>(stmt: &Stmt<'tcx>) -> Option<(&'tcx Expr<'tcx>, Option<&'t
fn never_loop_expr<'tcx>(
cx: &LateContext<'tcx>,
expr: &Expr<'tcx>,
ignore_ids: &mut Vec<HirId>,
local_labels: &mut Vec<(HirId, bool)>,
main_loop_id: HirId,
) -> NeverLoopResult {
match expr.kind {
let result = match expr.kind {
ExprKind::Unary(_, e)
| ExprKind::Cast(e, _)
| ExprKind::Type(e, _)
| ExprKind::Field(e, _)
| ExprKind::AddrOf(_, _, e)
| ExprKind::Repeat(e, _)
| ExprKind::DropTemps(e) => never_loop_expr(cx, e, ignore_ids, main_loop_id),
ExprKind::Let(let_expr) => never_loop_expr(cx, let_expr.init, ignore_ids, main_loop_id),
ExprKind::Array(es) | ExprKind::Tup(es) => never_loop_expr_all(cx, &mut es.iter(), ignore_ids, main_loop_id),
| ExprKind::DropTemps(e) => never_loop_expr(cx, e, local_labels, main_loop_id),
ExprKind::Let(let_expr) => never_loop_expr(cx, let_expr.init, local_labels, main_loop_id),
ExprKind::Array(es) | ExprKind::Tup(es) => never_loop_expr_all(cx, es.iter(), local_labels, main_loop_id),
ExprKind::MethodCall(_, receiver, es, _) => never_loop_expr_all(
cx,
&mut std::iter::once(receiver).chain(es.iter()),
ignore_ids,
std::iter::once(receiver).chain(es.iter()),
local_labels,
main_loop_id,
),
ExprKind::Struct(_, fields, base) => {
let fields = never_loop_expr_all(cx, &mut fields.iter().map(|f| f.expr), ignore_ids, main_loop_id);
let fields = never_loop_expr_all(cx, fields.iter().map(|f| f.expr), local_labels, main_loop_id);
if let Some(base) = base {
combine_seq(fields, never_loop_expr(cx, base, ignore_ids, main_loop_id))
combine_seq(fields, || never_loop_expr(cx, base, local_labels, main_loop_id))
} else {
fields
}
},
ExprKind::Call(e, es) => never_loop_expr_all(cx, &mut once(e).chain(es.iter()), ignore_ids, main_loop_id),
ExprKind::Call(e, es) => never_loop_expr_all(cx, once(e).chain(es.iter()), local_labels, main_loop_id),
ExprKind::Binary(_, e1, e2)
| ExprKind::Assign(e1, e2, _)
| ExprKind::AssignOp(_, e1, e2)
| ExprKind::Index(e1, e2, _) => {
never_loop_expr_all(cx, &mut [e1, e2].iter().copied(), ignore_ids, main_loop_id)
},
| ExprKind::Index(e1, e2, _) => never_loop_expr_all(cx, [e1, e2].iter().copied(), local_labels, main_loop_id),
ExprKind::Loop(b, _, _, _) => {
// Break can come from the inner loop so remove them.
absorb_break(never_loop_block(cx, b, ignore_ids, main_loop_id))
// We don't attempt to track reachability after a loop,
// just assume there may have been a break somewhere
absorb_break(never_loop_block(cx, b, local_labels, main_loop_id))
},
ExprKind::If(e, e2, e3) => {
let e1 = never_loop_expr(cx, e, ignore_ids, main_loop_id);
let e2 = never_loop_expr(cx, e2, ignore_ids, main_loop_id);
// If we know the `if` condition evaluates to `true`, don't check everything past it; it
// should just return whatever's evaluated for `e1` and `e2` since `e3` is unreachable
if let Some(Constant::Bool(true)) = constant(cx, cx.typeck_results(), e) {
return combine_seq(e1, e2);
}
let e3 = e3.as_ref().map_or(NeverLoopResult::Otherwise, |e| {
never_loop_expr(cx, e, ignore_ids, main_loop_id)
});
combine_seq(e1, combine_branches(e2, e3, ignore_ids))
let e1 = never_loop_expr(cx, e, local_labels, main_loop_id);
combine_seq(e1, || {
let e2 = never_loop_expr(cx, e2, local_labels, main_loop_id);
let e3 = e3.as_ref().map_or(NeverLoopResult::Normal, |e| {
never_loop_expr(cx, e, local_labels, main_loop_id)
});
combine_branches(e2, e3)
})
},
ExprKind::Match(e, arms, _) => {
let e = never_loop_expr(cx, e, ignore_ids, main_loop_id);
if arms.is_empty() {
e
} else {
let arms = never_loop_expr_branch(cx, &mut arms.iter().map(|a| a.body), ignore_ids, main_loop_id);
combine_seq(e, arms)
}
let e = never_loop_expr(cx, e, local_labels, main_loop_id);
combine_seq(e, || {
arms.iter().fold(NeverLoopResult::Diverging, |a, b| {
combine_branches(a, never_loop_expr(cx, b.body, local_labels, main_loop_id))
})
})
},
ExprKind::Block(b, l) => {
if l.is_some() {
ignore_ids.push(b.hir_id);
}
let ret = never_loop_block(cx, b, ignore_ids, main_loop_id);
if l.is_some() {
ignore_ids.pop();
local_labels.push((b.hir_id, false));
}
let ret = never_loop_block(cx, b, local_labels, main_loop_id);
let jumped_to = l.is_some() && local_labels.pop().unwrap().1;
match ret {
NeverLoopResult::IgnoreUntilEnd(a) if a == b.hir_id => NeverLoopResult::Otherwise,
NeverLoopResult::Diverging if jumped_to => NeverLoopResult::Normal,
_ => ret,
}
},
@ -211,74 +219,78 @@ fn never_loop_expr<'tcx>(
if id == main_loop_id {
NeverLoopResult::MayContinueMainLoop
} else {
NeverLoopResult::AlwaysBreak
NeverLoopResult::Diverging
}
},
// checks if break targets a block instead of a loop
ExprKind::Break(Destination { target_id: Ok(t), .. }, e) if ignore_ids.contains(&t) => e
.map_or(NeverLoopResult::IgnoreUntilEnd(t), |e| {
never_loop_expr(cx, e, ignore_ids, main_loop_id)
}),
ExprKind::Break(_, e) | ExprKind::Ret(e) => e.as_ref().map_or(NeverLoopResult::AlwaysBreak, |e| {
combine_seq(
never_loop_expr(cx, e, ignore_ids, main_loop_id),
NeverLoopResult::AlwaysBreak,
)
}),
ExprKind::Become(e) => combine_seq(
never_loop_expr(cx, e, ignore_ids, main_loop_id),
NeverLoopResult::AlwaysBreak,
),
ExprKind::InlineAsm(asm) => asm
.operands
.iter()
.map(|(o, _)| match o {
InlineAsmOperand::In { expr, .. } | InlineAsmOperand::InOut { expr, .. } => {
never_loop_expr(cx, expr, ignore_ids, main_loop_id)
},
InlineAsmOperand::Out { expr, .. } => {
never_loop_expr_all(cx, &mut expr.iter().copied(), ignore_ids, main_loop_id)
},
InlineAsmOperand::SplitInOut { in_expr, out_expr, .. } => never_loop_expr_all(
cx,
&mut once(*in_expr).chain(out_expr.iter().copied()),
ignore_ids,
main_loop_id,
),
InlineAsmOperand::Const { .. }
| InlineAsmOperand::SymFn { .. }
| InlineAsmOperand::SymStatic { .. } => NeverLoopResult::Otherwise,
ExprKind::Break(_, e) | ExprKind::Ret(e) => {
let first = e.as_ref().map_or(NeverLoopResult::Normal, |e| {
never_loop_expr(cx, e, local_labels, main_loop_id)
});
combine_seq(first, || {
// checks if break targets a block instead of a loop
if let ExprKind::Break(Destination { target_id: Ok(t), .. }, _) = expr.kind {
if let Some((_, reachable)) = local_labels.iter_mut().find(|(label, _)| *label == t) {
*reachable = true;
}
}
NeverLoopResult::Diverging
})
.fold(NeverLoopResult::Otherwise, combine_seq),
},
ExprKind::Become(e) => combine_seq(never_loop_expr(cx, e, local_labels, main_loop_id), || {
NeverLoopResult::Diverging
}),
ExprKind::InlineAsm(asm) => combine_seq_many(asm.operands.iter().map(|(o, _)| match o {
InlineAsmOperand::In { expr, .. } | InlineAsmOperand::InOut { expr, .. } => {
never_loop_expr(cx, expr, local_labels, main_loop_id)
},
InlineAsmOperand::Out { expr, .. } => {
never_loop_expr_all(cx, expr.iter().copied(), local_labels, main_loop_id)
},
InlineAsmOperand::SplitInOut { in_expr, out_expr, .. } => never_loop_expr_all(
cx,
once(*in_expr).chain(out_expr.iter().copied()),
local_labels,
main_loop_id,
),
InlineAsmOperand::Const { .. } | InlineAsmOperand::SymFn { .. } | InlineAsmOperand::SymStatic { .. } => {
NeverLoopResult::Normal
},
})),
ExprKind::OffsetOf(_, _)
| ExprKind::Yield(_, _)
| ExprKind::Closure { .. }
| ExprKind::Path(_)
| ExprKind::ConstBlock(_)
| ExprKind::Lit(_)
| ExprKind::Err(_) => NeverLoopResult::Otherwise,
| ExprKind::Err(_) => NeverLoopResult::Normal,
};
let result = combine_seq(result, || {
if cx.typeck_results().expr_ty(expr).is_never() {
NeverLoopResult::Diverging
} else {
NeverLoopResult::Normal
}
});
if let NeverLoopResult::Diverging = result &&
let Some(macro_call) = root_macro_call_first_node(cx, expr) &&
let Some(sym::todo_macro) = cx.tcx.get_diagnostic_name(macro_call.def_id)
{
// We return MayContinueMainLoop here because we treat `todo!()`
// as potentially containing any code, including a continue of the main loop.
// This effectively silences the lint whenever a loop contains this macro anywhere.
NeverLoopResult::MayContinueMainLoop
} else {
result
}
}
fn never_loop_expr_all<'tcx, T: Iterator<Item = &'tcx Expr<'tcx>>>(
cx: &LateContext<'tcx>,
es: &mut T,
ignore_ids: &mut Vec<HirId>,
es: T,
local_labels: &mut Vec<(HirId, bool)>,
main_loop_id: HirId,
) -> NeverLoopResult {
es.map(|e| never_loop_expr(cx, e, ignore_ids, main_loop_id))
.fold(NeverLoopResult::Otherwise, combine_seq)
}
fn never_loop_expr_branch<'tcx, T: Iterator<Item = &'tcx Expr<'tcx>>>(
cx: &LateContext<'tcx>,
e: &mut T,
ignore_ids: &mut Vec<HirId>,
main_loop_id: HirId,
) -> NeverLoopResult {
e.fold(NeverLoopResult::AlwaysBreak, |a, b| {
combine_branches(a, never_loop_expr(cx, b, ignore_ids, main_loop_id), ignore_ids)
})
combine_seq_many(es.map(|e| never_loop_expr(cx, e, local_labels, main_loop_id)))
}
fn for_to_if_let_sugg(cx: &LateContext<'_>, iterator: &Expr<'_>, pat: &Pat<'_>) -> String {

3
clippy_lints/src/macro_use.rs
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,

129
clippy_lints/src/manual_range_patterns.rs
View file

@ -1,4 +1,5 @@
use clippy_utils::diagnostics::span_lint_and_sugg;
use clippy_utils::diagnostics::span_lint_and_then;
use clippy_utils::source::snippet_opt;
use rustc_ast::LitKind;
use rustc_data_structures::fx::FxHashSet;
use rustc_errors::Applicability;
@ -6,6 +7,7 @@ use rustc_hir::{Expr, ExprKind, PatKind, RangeEnd, UnOp};
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::{Span, DUMMY_SP};
declare_clippy_lint! {
/// ### What it does
@ -49,6 +51,29 @@ fn expr_as_i128(expr: &Expr<'_>) -> Option<i128> {
}
}
#[derive(Copy, Clone)]
struct Num {
val: i128,
span: Span,
}
impl Num {
fn new(expr: &Expr<'_>) -> Option<Self> {
Some(Self {
val: expr_as_i128(expr)?,
span: expr.span,
})
}
fn dummy(val: i128) -> Self {
Self { val, span: DUMMY_SP }
}
fn min(self, other: Self) -> Self {
if self.val < other.val { self } else { other }
}
}
impl LateLintPass<'_> for ManualRangePatterns {
fn check_pat(&mut self, cx: &LateContext<'_>, pat: &'_ rustc_hir::Pat<'_>) {
if in_external_macro(cx.sess(), pat.span) {
@ -56,71 +81,83 @@ impl LateLintPass<'_> for ManualRangePatterns {
}
// a pattern like 1 | 2 seems fine, lint if there are at least 3 alternatives
// or at least one range
if let PatKind::Or(pats) = pat.kind
&& pats.len() >= 3
&& (pats.len() >= 3 || pats.iter().any(|p| matches!(p.kind, PatKind::Range(..))))
{
let mut min = i128::MAX;
let mut max = i128::MIN;
let mut min = Num::dummy(i128::MAX);
let mut max = Num::dummy(i128::MIN);
let mut range_kind = RangeEnd::Included;
let mut numbers_found = FxHashSet::default();
let mut ranges_found = Vec::new();
for pat in pats {
if let PatKind::Lit(lit) = pat.kind
&& let Some(num) = expr_as_i128(lit)
&& let Some(num) = Num::new(lit)
{
numbers_found.insert(num);
numbers_found.insert(num.val);
min = min.min(num);
max = max.max(num);
if num.val >= max.val {
max = num;
range_kind = RangeEnd::Included;
}
} else if let PatKind::Range(Some(left), Some(right), end) = pat.kind
&& let Some(left) = expr_as_i128(left)
&& let Some(right) = expr_as_i128(right)
&& right >= left
&& let Some(left) = Num::new(left)
&& let Some(mut right) = Num::new(right)
{
if let RangeEnd::Excluded = end {
right.val -= 1;
}
min = min.min(left);
max = max.max(right);
ranges_found.push(left..=match end {
RangeEnd::Included => right,
RangeEnd::Excluded => right - 1,
});
if right.val > max.val {
max = right;
range_kind = end;
}
ranges_found.push(left.val..=right.val);
} else {
return;
}
}
let contains_whole_range = 'contains: {
let mut num = min;
while num <= max {
if numbers_found.contains(&num) {
num += 1;
}
// Given a list of (potentially overlapping) ranges like:
// 1..=5, 3..=7, 6..=10
// We want to find the range with the highest end that still contains the current number
else if let Some(range) = ranges_found
.iter()
.filter(|range| range.contains(&num))
.max_by_key(|range| range.end())
{
num = range.end() + 1;
} else {
break 'contains false;
}
let mut num = min.val;
while num <= max.val {
if numbers_found.contains(&num) {
num += 1;
}
// Given a list of (potentially overlapping) ranges like:
// 1..=5, 3..=7, 6..=10
// We want to find the range with the highest end that still contains the current number
else if let Some(range) = ranges_found
.iter()
.filter(|range| range.contains(&num))
.max_by_key(|range| range.end())
{
num = range.end() + 1;
} else {
return;
}
break 'contains true;
};
if contains_whole_range {
span_lint_and_sugg(
cx,
MANUAL_RANGE_PATTERNS,
pat.span,
"this OR pattern can be rewritten using a range",
"try",
format!("{min}..={max}"),
Applicability::MachineApplicable,
);
}
span_lint_and_then(
cx,
MANUAL_RANGE_PATTERNS,
pat.span,
"this OR pattern can be rewritten using a range",
|diag| {
if let Some(min) = snippet_opt(cx, min.span)
&& let Some(max) = snippet_opt(cx, max.span)
{
diag.span_suggestion(
pat.span,
"try",
format!("{min}{range_kind}{max}"),
Applicability::MachineApplicable,
);
}
},
);
}
}
}

106
clippy_lints/src/methods/iter_out_of_bounds.rs Normal file
View file

@ -0,0 +1,106 @@
use clippy_utils::diagnostics::span_lint_and_note;
use clippy_utils::higher::VecArgs;
use clippy_utils::{expr_or_init, is_trait_method, match_def_path, paths};
use rustc_ast::LitKind;
use rustc_hir::{Expr, ExprKind};
use rustc_lint::LateContext;
use rustc_middle::ty::{self};
use rustc_span::sym;
use super::ITER_OUT_OF_BOUNDS;
fn expr_as_u128(cx: &LateContext<'_>, e: &Expr<'_>) -> Option<u128> {
if let ExprKind::Lit(lit) = expr_or_init(cx, e).kind
&& let LitKind::Int(n, _) = lit.node
{
Some(n)
} else {
None
}
}
/// Attempts to extract the length out of an iterator expression.
fn get_iterator_length<'tcx>(cx: &LateContext<'tcx>, iter: &'tcx Expr<'tcx>) -> Option<u128> {
let ty::Adt(adt, substs) = cx.typeck_results().expr_ty(iter).kind() else {
return None;
};
let did = adt.did();
if match_def_path(cx, did, &paths::ARRAY_INTO_ITER) {
// For array::IntoIter<T, const N: usize>, the length is the second generic
// parameter.
substs
.const_at(1)
.try_eval_target_usize(cx.tcx, cx.param_env)
.map(u128::from)
} else if match_def_path(cx, did, &paths::SLICE_ITER)
&& let ExprKind::MethodCall(_, recv, ..) = iter.kind
{
if let ty::Array(_, len) = cx.typeck_results().expr_ty(recv).peel_refs().kind() {
// For slice::Iter<'_, T>, the receiver might be an array literal: [1,2,3].iter().skip(..)
len.try_eval_target_usize(cx.tcx, cx.param_env).map(u128::from)
} else if let Some(args) = VecArgs::hir(cx, expr_or_init(cx, recv)) {
match args {
VecArgs::Vec(vec) => vec.len().try_into().ok(),
VecArgs::Repeat(_, len) => expr_as_u128(cx, len),
}
} else {
None
}
} else if match_def_path(cx, did, &paths::ITER_EMPTY) {
Some(0)
} else if match_def_path(cx, did, &paths::ITER_ONCE) {
Some(1)
} else {
None
}
}
fn check<'tcx>(
cx: &LateContext<'tcx>,
expr: &'tcx Expr<'tcx>,
recv: &'tcx Expr<'tcx>,
arg: &'tcx Expr<'tcx>,
message: &'static str,
note: &'static str,
) {
if is_trait_method(cx, expr, sym::Iterator)
&& let Some(len) = get_iterator_length(cx, recv)
&& let Some(skipped) = expr_as_u128(cx, arg)
&& skipped > len
{
span_lint_and_note(cx, ITER_OUT_OF_BOUNDS, expr.span, message, None, note);
}
}
pub(super) fn check_skip<'tcx>(
cx: &LateContext<'tcx>,
expr: &'tcx Expr<'tcx>,
recv: &'tcx Expr<'tcx>,
arg: &'tcx Expr<'tcx>,
) {
check(
cx,
expr,
recv,
arg,
"this `.skip()` call skips more items than the iterator will produce",
"this operation is useless and will create an empty iterator",
);
}
pub(super) fn check_take<'tcx>(
cx: &LateContext<'tcx>,
expr: &'tcx Expr<'tcx>,
recv: &'tcx Expr<'tcx>,
arg: &'tcx Expr<'tcx>,
) {
check(
cx,
expr,
recv,
arg,
"this `.take()` call takes more items than the iterator will produce",
"this operation is useless and the returned iterator will simply yield the same items",
);
}

2
clippy_lints/src/methods/iter_overeager_cloned.rs
View file

@ -21,7 +21,7 @@ pub(super) enum Op<'a> {
RmCloned,
// rm `.cloned()`
// e.g. `map` `for_each`
// e.g. `map` `for_each` `all` `any`
NeedlessMove(&'a str, &'a Expr<'a>),
// later `.cloned()`

58
clippy_lints/src/methods/mod.rs
View file

@ -43,6 +43,7 @@ mod iter_next_slice;
mod iter_nth;
mod iter_nth_zero;
mod iter_on_single_or_empty_collections;
mod iter_out_of_bounds;
mod iter_overeager_cloned;
mod iter_skip_next;
mod iter_skip_zero;
@ -3054,12 +3055,12 @@ declare_clippy_lint! {
///
/// ### Example
/// ```rust
/// vec!(1, 2, 3, 4, 5).resize(0, 5)
/// vec![1, 2, 3, 4, 5].resize(0, 5)
/// ```
///
/// Use instead:
/// ```rust
/// vec!(1, 2, 3, 4, 5).clear()
/// vec![1, 2, 3, 4, 5].clear()
/// ```
#[clippy::version = "1.46.0"]
pub VEC_RESIZE_TO_ZERO,
@ -3538,6 +3539,30 @@ declare_clippy_lint! {
"acquiring a write lock when a read lock would work"
}
declare_clippy_lint! {
/// ### What it does
/// Looks for iterator combinator calls such as `.take(x)` or `.skip(x)`
/// where `x` is greater than the amount of items that an iterator will produce.
///
/// ### Why is this bad?
/// Taking or skipping more items than there are in an iterator either creates an iterator
/// with all items from the original iterator or an iterator with no items at all.
/// This is most likely not what the user intended to do.
///
/// ### Example
/// ```rust
/// for _ in [1, 2, 3].iter().take(4) {}
/// ```
/// Use instead:
/// ```rust
/// for _ in [1, 2, 3].iter() {}
/// ```
#[clippy::version = "1.74.0"]
pub ITER_OUT_OF_BOUNDS,
suspicious,
"calls to `.take()` or `.skip()` that are out of bounds"
}
pub struct Methods {
avoid_breaking_exported_api: bool,
msrv: Msrv,
@ -3676,7 +3701,8 @@ impl_lint_pass!(Methods => [
STRING_LIT_CHARS_ANY,
ITER_SKIP_ZERO,
FILTER_MAP_BOOL_THEN,
READONLY_WRITE_LOCK
READONLY_WRITE_LOCK,
ITER_OUT_OF_BOUNDS,
]);
/// Extracts a method call name, args, and `Span` of the method name.
@ -3873,6 +3899,12 @@ impl Methods {
("add" | "offset" | "sub" | "wrapping_offset" | "wrapping_add" | "wrapping_sub", [_arg]) => {
zst_offset::check(cx, expr, recv);
},
("all", [arg]) => {
if let Some(("cloned", recv2, [], _, _)) = method_call(recv) {
iter_overeager_cloned::check(cx, expr, recv, recv2,
iter_overeager_cloned::Op::NeedlessMove(name, arg), false);
}
}
("and_then", [arg]) => {
let biom_option_linted = bind_instead_of_map::OptionAndThenSome::check(cx, expr, recv, arg);
let biom_result_linted = bind_instead_of_map::ResultAndThenOk::check(cx, expr, recv, arg);
@ -3880,12 +3912,16 @@ impl Methods {
unnecessary_lazy_eval::check(cx, expr, recv, arg, "and");
}
},
("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);
("any", [arg]) => {
match method_call(recv) {
Some(("cloned", recv2, [], _, _)) => iter_overeager_cloned::check(cx, expr, recv, recv2, iter_overeager_cloned::Op::NeedlessMove(name, arg), false),
Some(("chars", recv, _, _, _)) if let ExprKind::Closure(arg) = arg.kind
&& let body = cx.tcx.hir().body(arg.body)
&& let [param] = body.params => {
string_lit_chars_any::check(cx, expr, recv, param, peel_blocks(body.value), &self.msrv);
}
_ => {}
}
}
("arg", [arg]) => {
suspicious_command_arg_space::check(cx, recv, arg, span);
@ -4136,6 +4172,7 @@ impl Methods {
},
("skip", [arg]) => {
iter_skip_zero::check(cx, expr, arg);
iter_out_of_bounds::check_skip(cx, expr, recv, arg);
if let Some(("cloned", recv2, [], _span2, _)) = method_call(recv) {
iter_overeager_cloned::check(cx, expr, recv, recv2,
@ -4163,7 +4200,8 @@ impl Methods {
}
},
("step_by", [arg]) => iterator_step_by_zero::check(cx, expr, arg),
("take", [_arg]) => {
("take", [arg]) => {
iter_out_of_bounds::check_take(cx, expr, recv, arg);
if let Some(("cloned", recv2, [], _span2, _)) = method_call(recv) {
iter_overeager_cloned::check(cx, expr, recv, recv2,
iter_overeager_cloned::Op::LaterCloned, false);

3
clippy_lints/src/methods/unnecessary_sort_by.rs
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;

391
clippy_lints/src/missing_asserts_for_indexing.rs Normal file
View file

@ -0,0 +1,391 @@
use std::mem;
use std::ops::ControlFlow;
use clippy_utils::comparisons::{normalize_comparison, Rel};
use clippy_utils::diagnostics::span_lint_and_then;
use clippy_utils::source::snippet;
use clippy_utils::visitors::for_each_expr;
use clippy_utils::{eq_expr_value, hash_expr, higher};
use rustc_ast::{LitKind, RangeLimits};
use rustc_data_structures::unhash::UnhashMap;
use rustc_errors::{Applicability, Diagnostic};
use rustc_hir::{BinOp, Block, Expr, ExprKind, UnOp};
use rustc_lint::{LateContext, LateLintPass};
use rustc_session::{declare_lint_pass, declare_tool_lint};
use rustc_span::source_map::Spanned;
use rustc_span::{sym, Span};
declare_clippy_lint! {
/// ### What it does
/// Checks for repeated slice indexing without asserting beforehand that the length
/// is greater than the largest index used to index into the slice.
///
/// ### Why is this bad?
/// In the general case where the compiler does not have a lot of information
/// about the length of a slice, indexing it repeatedly will generate a bounds check
/// for every single index.
///
/// Asserting that the length of the slice is at least as large as the largest value
/// to index beforehand gives the compiler enough information to elide the bounds checks,
/// effectively reducing the number of bounds checks from however many times
/// the slice was indexed to just one (the assert).
///
/// ### Drawbacks
/// False positives. It is, in general, very difficult to predict how well
/// the optimizer will be able to elide bounds checks and it very much depends on
/// the surrounding code. For example, indexing into the slice yielded by the
/// [`slice::chunks_exact`](https://doc.rust-lang.org/stable/std/primitive.slice.html#method.chunks_exact)
/// iterator will likely have all of the bounds checks elided even without an assert
/// if the `chunk_size` is a constant.
///
/// Asserts are not tracked across function calls. Asserting the length of a slice
/// in a different function likely gives the optimizer enough information
/// about the length of a slice, but this lint will not detect that.
///
/// ### Example
/// ```rust
/// fn sum(v: &[u8]) -> u8 {
/// // 4 bounds checks
/// v[0] + v[1] + v[2] + v[3]
/// }
/// ```
/// Use instead:
/// ```rust
/// fn sum(v: &[u8]) -> u8 {
/// assert!(v.len() > 4);
/// // no bounds checks
/// v[0] + v[1] + v[2] + v[3]
/// }
/// ```
#[clippy::version = "1.70.0"]
pub MISSING_ASSERTS_FOR_INDEXING,
restriction,
"indexing into a slice multiple times without an `assert`"
}
declare_lint_pass!(MissingAssertsForIndexing => [MISSING_ASSERTS_FOR_INDEXING]);
fn report_lint<F>(cx: &LateContext<'_>, full_span: Span, msg: &str, indexes: &[Span], f: F)
where
F: FnOnce(&mut Diagnostic),
{
span_lint_and_then(cx, MISSING_ASSERTS_FOR_INDEXING, full_span, msg, |diag| {
f(diag);
for span in indexes {
diag.span_note(*span, "slice indexed here");
}
diag.note("asserting the length before indexing will elide bounds checks");
});
}
#[derive(Copy, Clone, Debug)]
enum LengthComparison {
/// `v.len() < 5`
LengthLessThanInt,
/// `5 < v.len()`
IntLessThanLength,
/// `v.len() <= 5`
LengthLessThanOrEqualInt,
/// `5 <= v.len()`
IntLessThanOrEqualLength,
}
/// Extracts parts out of a length comparison expression.
///
/// E.g. for `v.len() > 5` this returns `Some((LengthComparison::IntLessThanLength, 5, `v.len()`))`
fn len_comparison<'hir>(
bin_op: BinOp,
left: &'hir Expr<'hir>,
right: &'hir Expr<'hir>,
) -> Option<(LengthComparison, usize, &'hir Expr<'hir>)> {
macro_rules! int_lit_pat {
($id:ident) => {
ExprKind::Lit(Spanned {
node: LitKind::Int($id, _),
..
})
};
}
// normalize comparison, `v.len() > 4` becomes `4 < v.len()`
// this simplifies the logic a bit
let (op, left, right) = normalize_comparison(bin_op.node, left, right)?;
match (op, &left.kind, &right.kind) {
(Rel::Lt, int_lit_pat!(left), _) => Some((LengthComparison::IntLessThanLength, *left as usize, right)),
(Rel::Lt, _, int_lit_pat!(right)) => Some((LengthComparison::LengthLessThanInt, *right as usize, left)),
(Rel::Le, int_lit_pat!(left), _) => Some((LengthComparison::IntLessThanOrEqualLength, *left as usize, right)),
(Rel::Le, _, int_lit_pat!(right)) => Some((LengthComparison::LengthLessThanOrEqualInt, *right as usize, left)),
_ => None,
}
}
/// Attempts to extract parts out of an `assert!`-like expression
/// in the form `assert!(some_slice.len() > 5)`.
///
/// `assert!` has expanded to an if expression at the HIR, so this
/// actually works not just with `assert!` specifically, but anything
/// that has a never type expression in the `then` block (e.g. `panic!`).
fn assert_len_expr<'hir>(
cx: &LateContext<'_>,
expr: &'hir Expr<'hir>,
) -> Option<(LengthComparison, usize, &'hir Expr<'hir>)> {
if let Some(higher::If { cond, then, .. }) = higher::If::hir(expr)
&& let ExprKind::Unary(UnOp::Not, condition) = &cond.kind
&& let ExprKind::Binary(bin_op, left, right) = &condition.kind
&& let Some((cmp, asserted_len, slice_len)) = len_comparison(*bin_op, left, right)
&& let ExprKind::MethodCall(method, recv, ..) = &slice_len.kind
&& cx.typeck_results().expr_ty_adjusted(recv).peel_refs().is_slice()
&& method.ident.name == sym::len
// check if `then` block has a never type expression
&& let ExprKind::Block(Block { expr: Some(then_expr), .. }, _) = then.kind
&& cx.typeck_results().expr_ty(then_expr).is_never()
{
Some((cmp, asserted_len, recv))
} else {
None
}
}
#[derive(Debug)]
enum IndexEntry<'hir> {
/// `assert!` without any indexing (so far)
StrayAssert {
asserted_len: usize,
comparison: LengthComparison,
assert_span: Span,
slice: &'hir Expr<'hir>,
},
/// `assert!` with indexing
///
/// We also store the highest index to be able to check
/// if the `assert!` asserts the right length.
AssertWithIndex {
highest_index: usize,
asserted_len: usize,
assert_span: Span,
slice: &'hir Expr<'hir>,
indexes: Vec<Span>,
comparison: LengthComparison,
},
/// Indexing without an `assert!`
IndexWithoutAssert {
highest_index: usize,
indexes: Vec<Span>,
slice: &'hir Expr<'hir>,
},
}
impl<'hir> IndexEntry<'hir> {
pub fn slice(&self) -> &'hir Expr<'hir> {
match self {
IndexEntry::StrayAssert { slice, .. }
| IndexEntry::AssertWithIndex { slice, .. }
| IndexEntry::IndexWithoutAssert { slice, .. } => slice,
}
}
pub fn index_spans(&self) -> Option<&[Span]> {
match self {
IndexEntry::StrayAssert { .. } => None,
IndexEntry::AssertWithIndex { indexes, .. } | IndexEntry::IndexWithoutAssert { indexes, .. } => {
Some(indexes)
},
}
}
}
/// Extracts the upper index of a slice indexing expression.
///
/// E.g. for `5` this returns `Some(5)`, for `..5` this returns `Some(4)`,
/// for `..=5` this returns `Some(5)`
fn upper_index_expr(expr: &Expr<'_>) -> Option<usize> {
if let ExprKind::Lit(lit) = &expr.kind && let LitKind::Int(index, _) = lit.node {
Some(index as usize)
} else if let Some(higher::Range { end: Some(end), limits, .. }) = higher::Range::hir(expr)
&& let ExprKind::Lit(lit) = &end.kind
&& let LitKind::Int(index @ 1.., _) = lit.node
{
match limits {
RangeLimits::HalfOpen => Some(index as usize - 1),
RangeLimits::Closed => Some(index as usize),
}
} else {
None
}
}
/// Checks if the expression is an index into a slice and adds it to `indexes`
fn check_index<'hir>(cx: &LateContext<'_>, expr: &'hir Expr<'hir>, map: &mut UnhashMap<u64, Vec<IndexEntry<'hir>>>) {
if let ExprKind::Index(slice, index_lit, _) = expr.kind
&& cx.typeck_results().expr_ty_adjusted(slice).peel_refs().is_slice()
&& let Some(index) = upper_index_expr(index_lit)
{
let hash = hash_expr(cx, slice);
let indexes = map.entry(hash).or_default();
let entry = indexes.iter_mut().find(|entry| eq_expr_value(cx, entry.slice(), slice));
if let Some(entry) = entry {
match entry {
IndexEntry::StrayAssert { asserted_len, comparison, assert_span, slice } => {
*entry = IndexEntry::AssertWithIndex {
highest_index: index,
asserted_len: *asserted_len,
assert_span: *assert_span,
slice,
indexes: vec![expr.span],
comparison: *comparison,
};
},
IndexEntry::IndexWithoutAssert { highest_index, indexes, .. }
| IndexEntry::AssertWithIndex { highest_index, indexes, .. } => {
indexes.push(expr.span);
*highest_index = (*highest_index).max(index);
},
}
} else {
indexes.push(IndexEntry::IndexWithoutAssert {
highest_index: index,
indexes: vec![expr.span],
slice,
});
}
}
}
/// Checks if the expression is an `assert!` expression and adds it to `asserts`
fn check_assert<'hir>(cx: &LateContext<'_>, expr: &'hir Expr<'hir>, map: &mut UnhashMap<u64, Vec<IndexEntry<'hir>>>) {
if let Some((comparison, asserted_len, slice)) = assert_len_expr(cx, expr) {
let hash = hash_expr(cx, slice);
let indexes = map.entry(hash).or_default();
let entry = indexes.iter_mut().find(|entry| eq_expr_value(cx, entry.slice(), slice));
if let Some(entry) = entry {
if let IndexEntry::IndexWithoutAssert {
highest_index,
indexes,
slice,
} = entry
{
*entry = IndexEntry::AssertWithIndex {
highest_index: *highest_index,
indexes: mem::take(indexes),
slice,
assert_span: expr.span,
comparison,
asserted_len,
};
}
} else {
indexes.push(IndexEntry::StrayAssert {
asserted_len,
comparison,
assert_span: expr.span,
slice,
});
}
}
}
/// Inspects indexes and reports lints.
///
/// Called at the end of this lint after all indexing and `assert!` expressions have been collected.
fn report_indexes(cx: &LateContext<'_>, map: &UnhashMap<u64, Vec<IndexEntry<'_>>>) {
for bucket in map.values() {
for entry in bucket {
let Some(full_span) = entry
.index_spans()
.and_then(|spans| spans.first().zip(spans.last()))
.map(|(low, &high)| low.to(high))
else {
continue;
};
match entry {
IndexEntry::AssertWithIndex {
highest_index,
asserted_len,
indexes,
comparison,
assert_span,
slice,
} if indexes.len() > 1 => {
// if we have found an `assert!`, let's also check that it's actually right
// and if it convers the highest index and if not, suggest the correct length
let sugg = match comparison {
// `v.len() < 5` and `v.len() <= 5` does nothing in terms of bounds checks.
// The user probably meant `v.len() > 5`
LengthComparison::LengthLessThanInt | LengthComparison::LengthLessThanOrEqualInt => Some(
format!("assert!({}.len() > {highest_index})", snippet(cx, slice.span, "..")),
),
// `5 < v.len()` == `v.len() > 5`
LengthComparison::IntLessThanLength if asserted_len < highest_index => Some(format!(
"assert!({}.len() > {highest_index})",
snippet(cx, slice.span, "..")
)),
// `5 <= v.len() == `v.len() >= 5`
LengthComparison::IntLessThanOrEqualLength if asserted_len <= highest_index => Some(format!(
"assert!({}.len() > {highest_index})",
snippet(cx, slice.span, "..")
)),
_ => None,
};
if let Some(sugg) = sugg {
report_lint(
cx,
full_span,
"indexing into a slice multiple times with an `assert` that does not cover the highest index",
indexes,
|diag| {
diag.span_suggestion(
*assert_span,
"provide the highest index that is indexed with",
sugg,
Applicability::MachineApplicable,
);
},
);
}
},
IndexEntry::IndexWithoutAssert {
indexes,
highest_index,
slice,
} if indexes.len() > 1 => {
// if there was no `assert!` but more than one index, suggest
// adding an `assert!` that covers the highest index
report_lint(
cx,
full_span,
"indexing into a slice multiple times without an `assert`",
indexes,
|diag| {
diag.help(format!(
"consider asserting the length before indexing: `assert!({}.len() > {highest_index});`",
snippet(cx, slice.span, "..")
));
},
);
},
_ => {},
}
}
}
}
impl LateLintPass<'_> for MissingAssertsForIndexing {
fn check_block(&mut self, cx: &LateContext<'_>, block: &Block<'_>) {
let mut map = UnhashMap::default();
for_each_expr(block, |expr| {
check_index(cx, expr, &mut map);
check_assert(cx, expr, &mut map);
ControlFlow::<!, ()>::Continue(())
});
report_indexes(cx, &map);
}
}

4
clippy_lints/src/mutable_debug_assertion.rs
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

16
clippy_lints/src/operators/arithmetic_side_effects.rs
View file

@ -14,7 +14,12 @@ use {rustc_ast as ast, rustc_hir as hir};
const HARD_CODED_ALLOWED_BINARY: &[[&str; 2]] = &[["f32", "f32"], ["f64", "f64"], ["std::string::String", "str"]];
const HARD_CODED_ALLOWED_UNARY: &[&str] = &["f32", "f64", "std::num::Saturating", "std::num::Wrapping"];
const INTEGER_METHODS: &[Symbol] = &[sym::saturating_div, sym::wrapping_div, sym::wrapping_rem, sym::wrapping_rem_euclid];
const INTEGER_METHODS: &[Symbol] = &[
sym::saturating_div,
sym::wrapping_div,
sym::wrapping_rem,
sym::wrapping_rem_euclid,
];
#[derive(Debug)]
pub struct ArithmeticSideEffects {
@ -93,7 +98,14 @@ impl ArithmeticSideEffects {
let is_non_zero_u = |symbol: Option<Symbol>| {
matches!(
symbol,
Some(sym::NonZeroU128 | sym::NonZeroU16 | sym::NonZeroU32 | sym::NonZeroU64 | sym::NonZeroU8 | sym::NonZeroUsize)
Some(
sym::NonZeroU128
| sym::NonZeroU16
| sym::NonZeroU32
| sym::NonZeroU64
| sym::NonZeroU8
| sym::NonZeroUsize
)
)
};
let is_sat_or_wrap = |ty: Ty<'_>| {

2
clippy_lints/src/operators/float_cmp.rs
View file

@ -17,7 +17,7 @@ pub(crate) fn check<'tcx>(
left: &'tcx Expr<'_>,
right: &'tcx Expr<'_>,
) {
if (op == BinOpKind::Eq || op == BinOpKind::Ne) && (is_float(cx, left) || is_float(cx, right)) {
if (op == BinOpKind::Eq || op == BinOpKind::Ne) && is_float(cx, left) {
let left_is_local = match constant_with_source(cx, cx.typeck_results(), left) {
Some((c, s)) if !is_allowed(&c) => s.is_local(),
Some(_) => return,

80
clippy_lints/src/raw_strings.rs
View file

@ -1,7 +1,7 @@
use std::iter::once;
use std::ops::ControlFlow;
use clippy_utils::diagnostics::span_lint_and_sugg;
use clippy_utils::diagnostics::span_lint_and_then;
use clippy_utils::source::snippet;
use rustc_ast::ast::{Expr, ExprKind};
use rustc_ast::token::LitKind;
@ -9,6 +9,7 @@ use rustc_errors::Applicability;
use rustc_lint::{EarlyContext, EarlyLintPass, LintContext};
use rustc_middle::lint::in_external_macro;
use rustc_session::{declare_tool_lint, impl_lint_pass};
use rustc_span::{BytePos, Pos, Span};
declare_clippy_lint! {
/// ### What it does
@ -76,14 +77,24 @@ impl EarlyLintPass for RawStrings {
}
if !str.contains(['\\', '"']) {
span_lint_and_sugg(
span_lint_and_then(
cx,
NEEDLESS_RAW_STRINGS,
expr.span,
"unnecessary raw string literal",
"try",
format!("{}\"{}\"", prefix.replace('r', ""), lit.symbol),
Applicability::MachineApplicable,
|diag| {
let (start, end) = hash_spans(expr.span, prefix, 0, max);
// BytePos: skip over the `b` in `br`, we checked the prefix appears in the source text
let r_pos = expr.span.lo() + BytePos::from_usize(prefix.len() - 1);
let start = start.with_lo(r_pos);
diag.multipart_suggestion(
"try",
vec![(start, String::new()), (end, String::new())],
Applicability::MachineApplicable,
);
},
);
return;
@ -96,13 +107,6 @@ impl EarlyLintPass for RawStrings {
let num = str.as_bytes().iter().chain(once(&0)).try_fold(0u8, |acc, &b| {
match b {
b'"' if !following_quote => (following_quote, req) = (true, 1),
// I'm a bit surprised the compiler didn't optimize this out, there's no
// branch but it still ends up doing an unnecessary comparison, it's:
// - cmp r9b,1h
// - sbb cl,-1h
// which will add 1 if it's true. With this change, it becomes:
// - add cl,r9b
// isn't that so much nicer?
b'#' => req += u8::from(following_quote),
_ => {
if following_quote {
@ -126,18 +130,58 @@ impl EarlyLintPass for RawStrings {
};
if req < max {
let hashes = "#".repeat(req as usize);
span_lint_and_sugg(
span_lint_and_then(
cx,
NEEDLESS_RAW_STRING_HASHES,
expr.span,
"unnecessary hashes around raw string literal",
"try",
format!(r#"{prefix}{hashes}"{}"{hashes}"#, lit.symbol),
Applicability::MachineApplicable,
|diag| {
let (start, end) = hash_spans(expr.span, prefix, req, max);
let message = match max - req {
_ if req == 0 => "remove all the hashes around the literal".to_string(),
1 => "remove one hash from both sides of the literal".to_string(),
n => format!("remove {n} hashes from both sides of the literal"),
};
diag.multipart_suggestion(
message,
vec![(start, String::new()), (end, String::new())],
Applicability::MachineApplicable,
);
},
);
}
}
}
}
/// Returns spans pointing at the unneeded hashes, e.g. for a `req` of `1` and `max` of `3`:
///
/// ```ignore
/// r###".."###
/// ^^ ^^
/// ```
fn hash_spans(literal_span: Span, prefix: &str, req: u8, max: u8) -> (Span, Span) {
let literal_span = literal_span.data();
// BytePos: we checked prefix appears literally in the source text
let hash_start = literal_span.lo + BytePos::from_usize(prefix.len());
let hash_end = literal_span.hi;
// BytePos: req/max are counts of the ASCII character #
let start = Span::new(
hash_start + BytePos(req.into()),
hash_start + BytePos(max.into()),
literal_span.ctxt,
None,
);
let end = Span::new(
hash_end - BytePos(req.into()),
hash_end - BytePos(max.into()),
literal_span.ctxt,
None,
);
(start, end)
}

16
clippy_lints/src/redundant_locals.rs
View file

@ -3,11 +3,9 @@ use clippy_utils::is_from_proc_macro;
use clippy_utils::ty::needs_ordered_drop;
use rustc_ast::Mutability;
use rustc_hir::def::Res;
use rustc_hir::{
BindingAnnotation, ByRef, Expr, ExprKind, HirId, Local, Node, Pat, PatKind, QPath,
};
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, is_from_async_await};
use rustc_middle::lint::in_external_macro;
use rustc_session::{declare_lint_pass, declare_tool_lint};
use rustc_span::symbol::Ident;
use rustc_span::DesugaringKind;
@ -72,9 +70,6 @@ impl<'tcx> LateLintPass<'tcx> for RedundantLocals {
// the local is user-controlled
if !in_external_macro(cx.sess(), local.span);
if !is_from_proc_macro(cx, expr);
// Async function parameters are lowered into the closure body, so we can't lint them.
// see `lower_maybe_async_body` in `rust_ast_lowering`
if !is_from_async_await(local.span);
then {
span_lint_and_help(
cx,
@ -111,12 +106,7 @@ fn affects_assignments(cx: &LateContext<'_>, mutability: Mutability, bind: HirId
}
/// 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 {
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

10
clippy_lints/src/same_name_method.rs
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) {

12
clippy_lints/src/slow_vector_initialization.rs
View file

@ -1,4 +1,5 @@
use clippy_utils::diagnostics::span_lint_and_then;
use clippy_utils::macros::root_macro_call;
use clippy_utils::sugg::Sugg;
use clippy_utils::{
get_enclosing_block, is_expr_path_def_path, is_integer_literal, is_path_diagnostic_item, path_to_local,
@ -148,6 +149,15 @@ impl SlowVectorInit {
/// - `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>> {
// Generally don't warn if the vec initializer comes from an expansion, except for the vec! macro.
// This lets us still warn on `vec![]`, while ignoring other kinds of macros that may output an
// empty vec
if expr.span.from_expansion()
&& root_macro_call(expr.span).map(|m| m.def_id) != cx.tcx.get_diagnostic_item(sym::vec_macro)
{
return None;
}
if let ExprKind::Call(func, [len_expr]) = expr.kind
&& is_expr_path_def_path(cx, func, &paths::VEC_WITH_CAPACITY)
{
@ -205,7 +215,7 @@ impl SlowVectorInit {
span_lint_and_then(cx, SLOW_VECTOR_INITIALIZATION, slow_fill.span, msg, |diag| {
diag.span_suggestion(
vec_alloc.allocation_expr.span,
vec_alloc.allocation_expr.span.source_callsite(),
"consider replacing this with",
format!("vec![0; {len_expr}]"),
Applicability::Unspecified,

107
clippy_lints/src/undocumented_unsafe_blocks.rs
View file

@ -341,44 +341,21 @@ fn block_parents_have_safety_comment(
id: hir::HirId,
) -> bool {
if let Some(node) = get_parent_node(cx.tcx, id) {
return match node {
Node::Expr(expr) => {
if let Some(
Node::Local(hir::Local { span, .. })
| Node::Item(hir::Item {
kind: hir::ItemKind::Const(..) | ItemKind::Static(..),
span,
..
}),
) = get_parent_node(cx.tcx, expr.hir_id)
{
let hir_id = match get_parent_node(cx.tcx, expr.hir_id) {
Some(Node::Local(hir::Local { hir_id, .. })) => *hir_id,
Some(Node::Item(hir::Item { owner_id, .. })) => {
cx.tcx.hir().local_def_id_to_hir_id(owner_id.def_id)
},
_ => unreachable!(),
};
// if unsafe block is part of a let/const/static statement,
// and accept_comment_above_statement is set to true
// we accept the safety comment in the line the precedes this statement.
accept_comment_above_statement
&& span_with_attrs_in_body_has_safety_comment(
cx,
*span,
hir_id,
accept_comment_above_attributes,
)
} else {
!is_branchy(expr)
&& span_with_attrs_in_body_has_safety_comment(
cx,
expr.span,
expr.hir_id,
accept_comment_above_attributes,
)
}
let (span, hir_id) = match node {
Node::Expr(expr) => match get_parent_node(cx.tcx, expr.hir_id) {
Some(Node::Local(hir::Local { span, hir_id, .. })) => (*span, *hir_id),
Some(Node::Item(hir::Item {
kind: hir::ItemKind::Const(..) | ItemKind::Static(..),
span,
owner_id,
..
})) => (*span, cx.tcx.hir().local_def_id_to_hir_id(owner_id.def_id)),
_ => {
if is_branchy(expr) {
return false;
}
(expr.span, expr.hir_id)
},
},
Node::Stmt(hir::Stmt {
kind:
@ -387,28 +364,27 @@ fn block_parents_have_safety_comment(
| hir::StmtKind::Semi(hir::Expr { span, hir_id, .. }),
..
})
| Node::Local(hir::Local { span, hir_id, .. }) => {
span_with_attrs_in_body_has_safety_comment(cx, *span, *hir_id, accept_comment_above_attributes)
},
| Node::Local(hir::Local { span, hir_id, .. }) => (*span, *hir_id),
Node::Item(hir::Item {
kind: hir::ItemKind::Const(..) | ItemKind::Static(..),
span,
owner_id,
..
}) => span_with_attrs_in_body_has_safety_comment(
cx,
*span,
cx.tcx.hir().local_def_id_to_hir_id(owner_id.def_id),
accept_comment_above_attributes,
),
_ => false,
}) => (*span, cx.tcx.hir().local_def_id_to_hir_id(owner_id.def_id)),
_ => return false,
};
// if unsafe block is part of a let/const/static statement,
// and accept_comment_above_statement is set to true
// we accept the safety comment in the line the precedes this statement.
accept_comment_above_statement
&& span_with_attrs_has_safety_comment(cx, span, hir_id, accept_comment_above_attributes)
} else {
false
}
false
}
/// Extends `span` to also include its attributes, then checks if that span has a safety comment.
fn span_with_attrs_in_body_has_safety_comment(
fn span_with_attrs_has_safety_comment(
cx: &LateContext<'_>,
span: Span,
hir_id: HirId,
@ -420,7 +396,7 @@ fn span_with_attrs_in_body_has_safety_comment(
span
};
span_in_body_has_safety_comment(cx, span)
span_has_safety_comment(cx, span)
}
/// Checks if an expression is "branchy", e.g. loop, match/if/etc.
@ -444,7 +420,7 @@ fn block_has_safety_comment(cx: &LateContext<'_>, span: Span) -> bool {
matches!(
span_from_macro_expansion_has_safety_comment(cx, span),
HasSafetyComment::Yes(_)
) || span_in_body_has_safety_comment(cx, span)
) || span_has_safety_comment(cx, span)
}
fn include_attrs_in_span(cx: &LateContext<'_>, hir_id: HirId, span: Span) -> Span {
@ -639,29 +615,36 @@ fn get_body_search_span(cx: &LateContext<'_>) -> Option<Span> {
let body = cx.enclosing_body?;
let map = cx.tcx.hir();
let mut span = map.body(body).value.span;
let mut maybe_global_var = false;
for (_, node) in map.parent_iter(body.hir_id) {
match node {
Node::Expr(e) => span = e.span,
Node::Block(_)
| Node::Arm(_)
| Node::Stmt(_)
| Node::Local(_)
| Node::Item(hir::Item {
Node::Block(_) | Node::Arm(_) | Node::Stmt(_) | Node::Local(_) => (),
Node::Item(hir::Item {
kind: hir::ItemKind::Const(..) | ItemKind::Static(..),
..
}) => (),
}) => maybe_global_var = true,
Node::Item(hir::Item {
kind: hir::ItemKind::Mod(_),
span: item_span,
..
}) => {
span = *item_span;
break;
},
Node::Crate(mod_) if maybe_global_var => {
span = mod_.spans.inner_span;
},
_ => break,
}
}
Some(span)
}
fn span_in_body_has_safety_comment(cx: &LateContext<'_>, span: Span) -> bool {
fn span_has_safety_comment(cx: &LateContext<'_>, span: Span) -> bool {
let source_map = cx.sess().source_map();
let ctxt = span.ctxt();
if ctxt == SyntaxContext::root()
&& let Some(search_span) = get_body_search_span(cx)
{
if ctxt.is_root() && let Some(search_span) = get_body_search_span(cx) {
if let Ok(unsafe_line) = source_map.lookup_line(span.lo())
&& let Some(body_span) = walk_span_to_context(search_span, SyntaxContext::root())
&& let Ok(body_line) = source_map.lookup_line(body_span.lo())

2
clippy_lints/src/unit_return_expecting_ord.rs
View file

@ -26,7 +26,7 @@ declare_clippy_lint! {
///
/// ### Example
/// ```rust
/// let mut twins = vec!((1, 1), (2, 2));
/// let mut twins = vec![(1, 1), (2, 2)];
/// twins.sort_by_key(|x| { x.1; });
/// ```
#[clippy::version = "1.47.0"]

111
clippy_lints/src/unwrap.rs
View file

@ -1,15 +1,18 @@
use clippy_utils::diagnostics::span_lint_hir_and_then;
use clippy_utils::ty::is_type_diagnostic_item;
use clippy_utils::usage::is_potentially_mutated;
use clippy_utils::usage::is_potentially_local_place;
use clippy_utils::{higher, path_to_local};
use if_chain::if_chain;
use rustc_errors::Applicability;
use rustc_hir::intravisit::{walk_expr, walk_fn, FnKind, Visitor};
use rustc_hir::{BinOpKind, Body, Expr, ExprKind, FnDecl, HirId, PathSegment, UnOp};
use rustc_hir::{BinOpKind, Body, Expr, ExprKind, FnDecl, HirId, Node, PathSegment, UnOp};
use rustc_hir_typeck::expr_use_visitor::{Delegate, ExprUseVisitor, PlaceWithHirId};
use rustc_infer::infer::TyCtxtInferExt;
use rustc_lint::{LateContext, LateLintPass};
use rustc_middle::hir::nested_filter;
use rustc_middle::lint::in_external_macro;
use rustc_middle::ty::Ty;
use rustc_middle::mir::FakeReadCause;
use rustc_middle::ty::{self, Ty, TyCtxt};
use rustc_session::{declare_lint_pass, declare_tool_lint};
use rustc_span::def_id::LocalDefId;
use rustc_span::source_map::Span;
@ -192,6 +195,55 @@ fn collect_unwrap_info<'tcx>(
Vec::new()
}
/// A HIR visitor delegate that checks if a local variable of type `Option<_>` is mutated,
/// *except* for if `Option::as_mut` is called.
/// The reason for why we allow that one specifically is that `.as_mut()` cannot change
/// the option to `None`, and that is important because this lint relies on the fact that
/// `is_some` + `unwrap` is equivalent to `if let Some(..) = ..`, which it would not be if
/// the option is changed to None between `is_some` and `unwrap`.
/// (And also `.as_mut()` is a somewhat common method that is still worth linting on.)
struct MutationVisitor<'tcx> {
is_mutated: bool,
local_id: HirId,
tcx: TyCtxt<'tcx>,
}
/// Checks if the parent of the expression pointed at by the given `HirId` is a call to
/// `Option::as_mut`.
///
/// Used by the mutation visitor to specifically allow `.as_mut()` calls.
/// In particular, the `HirId` that the visitor receives is the id of the local expression
/// (i.e. the `x` in `x.as_mut()`), and that is the reason for why we care about its parent
/// expression: that will be where the actual method call is.
fn is_option_as_mut_use(tcx: TyCtxt<'_>, expr_id: HirId) -> bool {
if let Node::Expr(mutating_expr) = tcx.hir().get_parent(expr_id)
&& let ExprKind::MethodCall(path, ..) = mutating_expr.kind
{
path.ident.name.as_str() == "as_mut"
} else {
false
}
}
impl<'tcx> Delegate<'tcx> for MutationVisitor<'tcx> {
fn borrow(&mut self, cat: &PlaceWithHirId<'tcx>, diag_expr_id: HirId, bk: ty::BorrowKind) {
if let ty::BorrowKind::MutBorrow = bk
&& is_potentially_local_place(self.local_id, &cat.place)
&& !is_option_as_mut_use(self.tcx, diag_expr_id)
{
self.is_mutated = true;
}
}
fn mutate(&mut self, _: &PlaceWithHirId<'tcx>, _: HirId) {
self.is_mutated = true;
}
fn consume(&mut self, _: &PlaceWithHirId<'tcx>, _: HirId) {}
fn fake_read(&mut self, _: &PlaceWithHirId<'tcx>, _: FakeReadCause, _: HirId) {}
}
impl<'a, 'tcx> UnwrappableVariablesVisitor<'a, 'tcx> {
fn visit_branch(
&mut self,
@ -202,10 +254,26 @@ impl<'a, 'tcx> UnwrappableVariablesVisitor<'a, 'tcx> {
) {
let prev_len = self.unwrappables.len();
for unwrap_info in collect_unwrap_info(self.cx, if_expr, cond, branch, else_branch, true) {
if is_potentially_mutated(unwrap_info.local_id, cond, self.cx)
|| is_potentially_mutated(unwrap_info.local_id, branch, self.cx)
{
// if the variable is mutated, we don't know whether it can be unwrapped:
let mut delegate = MutationVisitor {
tcx: self.cx.tcx,
is_mutated: false,
local_id: unwrap_info.local_id,
};
let infcx = self.cx.tcx.infer_ctxt().build();
let mut vis = ExprUseVisitor::new(
&mut delegate,
&infcx,
cond.hir_id.owner.def_id,
self.cx.param_env,
self.cx.typeck_results(),
);
vis.walk_expr(cond);
vis.walk_expr(branch);
if delegate.is_mutated {
// if the variable is mutated, we don't know whether it can be unwrapped.
// it might have been changed to `None` in between `is_some` + `unwrap`.
continue;
}
self.unwrappables.push(unwrap_info);
@ -215,6 +283,27 @@ impl<'a, 'tcx> UnwrappableVariablesVisitor<'a, 'tcx> {
}
}
enum AsRefKind {
AsRef,
AsMut,
}
/// Checks if the expression is a method call to `as_{ref,mut}` and returns the receiver of it.
/// If it isn't, the expression itself is returned.
fn consume_option_as_ref<'tcx>(expr: &'tcx Expr<'tcx>) -> (&'tcx Expr<'tcx>, Option<AsRefKind>) {
if let ExprKind::MethodCall(path, recv, ..) = expr.kind {
if path.ident.name == sym::as_ref {
(recv, Some(AsRefKind::AsRef))
} else if path.ident.name.as_str() == "as_mut" {
(recv, Some(AsRefKind::AsMut))
} else {
(expr, None)
}
} else {
(expr, None)
}
}
impl<'a, 'tcx> Visitor<'tcx> for UnwrappableVariablesVisitor<'a, 'tcx> {
type NestedFilter = nested_filter::OnlyBodies;
@ -233,6 +322,7 @@ impl<'a, 'tcx> Visitor<'tcx> for UnwrappableVariablesVisitor<'a, 'tcx> {
// find `unwrap[_err]()` calls:
if_chain! {
if let ExprKind::MethodCall(method_name, self_arg, ..) = expr.kind;
let (self_arg, as_ref_kind) = consume_option_as_ref(self_arg);
if let Some(id) = path_to_local(self_arg);
if [sym::unwrap, sym::expect, sym!(unwrap_err)].contains(&method_name.ident.name);
let call_to_unwrap = [sym::unwrap, sym::expect].contains(&method_name.ident.name);
@ -268,7 +358,12 @@ impl<'a, 'tcx> Visitor<'tcx> for UnwrappableVariablesVisitor<'a, 'tcx> {
unwrappable.check.span.with_lo(unwrappable.if_expr.span.lo()),
"try",
format!(
"if let {suggested_pattern} = {unwrappable_variable_name}",
"if let {suggested_pattern} = {borrow_prefix}{unwrappable_variable_name}",
borrow_prefix = match as_ref_kind {
Some(AsRefKind::AsRef) => "&",
Some(AsRefKind::AsMut) => "&mut ",
None => "",
},
),
// We don't track how the unwrapped value is used inside the
// block or suggest deleting the unwrap, so we can't offer a

20
clippy_lints/src/utils/conf.rs
View file

@ -561,6 +561,26 @@ define_Conf! {
/// Which crates to allow absolute paths from
(absolute_paths_allowed_crates: rustc_data_structures::fx::FxHashSet<String> =
rustc_data_structures::fx::FxHashSet::default()),
/// Lint: EXPLICIT_ITER_LOOP
///
/// Whether to recommend using implicit into iter for reborrowed values.
///
/// #### Example
/// ```
/// let mut vec = vec![1, 2, 3];
/// let rmvec = &mut vec;
/// for _ in rmvec.iter() {}
/// for _ in rmvec.iter_mut() {}
/// ```
///
/// Use instead:
/// ```
/// let mut vec = vec![1, 2, 3];
/// let rmvec = &mut vec;
/// for _ in &*rmvec {}
/// for _ in &mut *rmvec {}
/// ```
(enforce_iter_loop_reborrow: bool = false),
}
/// Search for the configuration file.