Auto merge of #9136 - smoelius:enhance-needless-borrow, r=Jarcho

Enhance `needless_borrow` to consider trait implementations

The proposed enhancement causes `needless_borrow` to suggest removing `&` from `&e` when `&e` is an argument position requiring trait implementations, and `e` implements the required traits. Example:
```
error: the borrowed expression implements the required traits
  --> $DIR/needless_borrow.rs:131:51
   |
LL |     let _ = std::process::Command::new("ls").args(&["-a", "-l"]).status().unwrap();
   |                                                   ^^^^^^^^^^^^^ help: change this to: `["-a", "-l"]`
```

r? `@Jarcho`

changelog: Enhance `needless_borrow` to consider trait implementations
This commit is contained in:
bors 2022-08-18 15:57:37 +00:00
commit c419d0a8b5
19 changed files with 585 additions and 78 deletions

View file

@ -37,7 +37,7 @@ fn update_reference_file(test_output_entry: &DirEntry, ignore_timestamp: bool) {
return;
}
let test_output_file = fs::read(&test_output_path).expect("Unable to read test output file");
let test_output_file = fs::read(test_output_path).expect("Unable to read test output file");
let reference_file = fs::read(&reference_file_path).unwrap_or_default();
if test_output_file != reference_file {

View file

@ -46,7 +46,7 @@ pub fn run(check: bool, verbose: bool) {
// dependency
if fs::read_to_string(project_root.join("Cargo.toml"))
.expect("Failed to read clippy Cargo.toml")
.contains(&"[target.'cfg(NOT_A_PLATFORM)'.dependencies]")
.contains("[target.'cfg(NOT_A_PLATFORM)'.dependencies]")
{
return Err(CliError::IntellijSetupActive);
}
@ -193,10 +193,10 @@ fn rustfmt_test(context: &FmtContext) -> Result<(), CliError> {
let args = &["--version"];
if context.verbose {
println!("{}", format_command(&program, &dir, args));
println!("{}", format_command(program, &dir, args));
}
let output = Command::new(&program).current_dir(&dir).args(args.iter()).output()?;
let output = Command::new(program).current_dir(&dir).args(args.iter()).output()?;
if output.status.success() {
Ok(())
@ -207,7 +207,7 @@ fn rustfmt_test(context: &FmtContext) -> Result<(), CliError> {
Err(CliError::RustfmtNotInstalled)
} else {
Err(CliError::CommandFailed(
format_command(&program, &dir, args),
format_command(program, &dir, args),
std::str::from_utf8(&output.stderr).unwrap_or("").to_string(),
))
}

View file

@ -418,7 +418,7 @@ fn remove_lint_declaration(name: &str, path: &Path, lints: &mut Vec<Lint>) -> io
.expect("failed to find `impl_lint_pass` terminator");
impl_lint_pass_end += impl_lint_pass_start;
if let Some(lint_name_pos) = content[impl_lint_pass_start..impl_lint_pass_end].find(&lint_name_upper) {
if let Some(lint_name_pos) = content[impl_lint_pass_start..impl_lint_pass_end].find(lint_name_upper) {
let mut lint_name_end = impl_lint_pass_start + (lint_name_pos + lint_name_upper.len());
for c in content[lint_name_end..impl_lint_pass_end].chars() {
// Remove trailing whitespace
@ -451,7 +451,7 @@ fn remove_lint_declaration(name: &str, path: &Path, lints: &mut Vec<Lint>) -> io
}
let mut content =
fs::read_to_string(&path).unwrap_or_else(|_| panic!("failed to read `{}`", path.to_string_lossy()));
fs::read_to_string(path).unwrap_or_else(|_| panic!("failed to read `{}`", path.to_string_lossy()));
eprintln!(
"warn: you will have to manually remove any code related to `{}` from `{}`",

View file

@ -1,24 +1,31 @@
use clippy_utils::diagnostics::{span_lint_and_sugg, span_lint_hir_and_then};
use clippy_utils::source::{snippet_with_applicability, snippet_with_context};
use clippy_utils::sugg::has_enclosing_paren;
use clippy_utils::ty::{expr_sig, peel_mid_ty_refs, ty_sig, variant_of_res};
use clippy_utils::{get_parent_expr, is_lint_allowed, path_to_local, walk_to_expr_usage};
use clippy_utils::ty::{contains_ty, expr_sig, is_copy, peel_mid_ty_refs, ty_sig, variant_of_res};
use clippy_utils::{fn_def_id, get_parent_expr, is_lint_allowed, meets_msrv, msrvs, path_to_local, walk_to_expr_usage};
use rustc_ast::util::parser::{PREC_POSTFIX, PREC_PREFIX};
use rustc_data_structures::fx::FxIndexMap;
use rustc_errors::Applicability;
use rustc_hir::intravisit::{walk_ty, Visitor};
use rustc_hir::{
self as hir, BindingAnnotation, Body, BodyId, BorrowKind, Closure, Expr, ExprKind, FnRetTy, GenericArg, HirId,
ImplItem, ImplItemKind, Item, ItemKind, Local, MatchSource, Mutability, Node, Pat, PatKind, Path, QPath, TraitItem,
TraitItemKind, TyKind, UnOp,
self as hir, def_id::DefId, BindingAnnotation, Body, BodyId, BorrowKind, Closure, Expr, ExprKind, FnRetTy,
GenericArg, HirId, ImplItem, ImplItemKind, Item, ItemKind, Local, MatchSource, Mutability, Node, Pat, PatKind,
Path, QPath, TraitItem, TraitItemKind, TyKind, UnOp,
};
use rustc_index::bit_set::BitSet;
use rustc_infer::infer::TyCtxtInferExt;
use rustc_lint::{LateContext, LateLintPass};
use rustc_middle::ty::adjustment::{Adjust, Adjustment, AutoBorrow, AutoBorrowMutability};
use rustc_middle::ty::{self, Binder, BoundVariableKind, List, Ty, TyCtxt, TypeVisitable, TypeckResults};
use rustc_middle::ty::{
self, subst::Subst, Binder, BoundVariableKind, EarlyBinder, FnSig, GenericArgKind, List, ParamTy, PredicateKind,
ProjectionPredicate, Ty, TyCtxt, TypeVisitable, TypeckResults,
};
use rustc_semver::RustcVersion;
use rustc_session::{declare_tool_lint, impl_lint_pass};
use rustc_span::{symbol::sym, Span, Symbol, DUMMY_SP};
use rustc_trait_selection::infer::InferCtxtExt;
use rustc_trait_selection::infer::InferCtxtExt as _;
use rustc_trait_selection::traits::{query::evaluate_obligation::InferCtxtExt as _, Obligation, ObligationCause};
use std::collections::VecDeque;
declare_clippy_lint! {
/// ### What it does
@ -151,6 +158,7 @@ pub struct Dereferencing {
/// been finished. Note we can't lint at the end of every body as they can be nested within each
/// other.
current_body: Option<BodyId>,
/// The list of locals currently being checked by the lint.
/// If the value is `None`, then the binding has been seen as a ref pattern, but is not linted.
/// This is needed for or patterns where one of the branches can be linted, but another can not
@ -158,6 +166,19 @@ pub struct Dereferencing {
///
/// e.g. `m!(x) | Foo::Bar(ref x)`
ref_locals: FxIndexMap<HirId, Option<RefPat>>,
// `IntoIterator` for arrays requires Rust 1.53.
msrv: Option<RustcVersion>,
}
impl Dereferencing {
#[must_use]
pub fn new(msrv: Option<RustcVersion>) -> Self {
Self {
msrv,
..Dereferencing::default()
}
}
}
struct StateData {
@ -170,6 +191,7 @@ struct StateData {
struct DerefedBorrow {
count: usize,
msg: &'static str,
snip_expr: Option<HirId>,
}
enum State {
@ -250,7 +272,7 @@ impl<'tcx> LateLintPass<'tcx> for Dereferencing {
match (self.state.take(), kind) {
(None, kind) => {
let expr_ty = typeck.expr_ty(expr);
let (position, adjustments) = walk_parents(cx, expr);
let (position, adjustments) = walk_parents(cx, expr, self.msrv);
match kind {
RefOp::Deref => {
@ -331,20 +353,23 @@ impl<'tcx> LateLintPass<'tcx> for Dereferencing {
let deref_msg =
"this expression creates a reference which is immediately dereferenced by the compiler";
let borrow_msg = "this expression borrows a value the compiler would automatically borrow";
let impl_msg = "the borrowed expression implements the required traits";
let (required_refs, msg) = if position.can_auto_borrow() {
(1, if deref_count == 1 { borrow_msg } else { deref_msg })
let (required_refs, msg, snip_expr) = if position.can_auto_borrow() {
(1, if deref_count == 1 { borrow_msg } else { deref_msg }, None)
} else if let Position::ImplArg(hir_id) = position {
(0, impl_msg, Some(hir_id))
} else if let Some(&Adjust::Borrow(AutoBorrow::Ref(_, mutability))) =
next_adjust.map(|a| &a.kind)
{
if matches!(mutability, AutoBorrowMutability::Mut { .. }) && !position.is_reborrow_stable()
{
(3, deref_msg)
(3, deref_msg, None)
} else {
(2, deref_msg)
(2, deref_msg, None)
}
} else {
(2, deref_msg)
(2, deref_msg, None)
};
if deref_count >= required_refs {
@ -354,6 +379,7 @@ impl<'tcx> LateLintPass<'tcx> for Dereferencing {
// can't be removed without breaking the code. See earlier comment.
count: deref_count - required_refs,
msg,
snip_expr,
}),
StateData { span: expr.span, hir_id: expr.hir_id, position },
));
@ -510,7 +536,7 @@ impl<'tcx> LateLintPass<'tcx> for Dereferencing {
spans: vec![pat.span],
app,
replacements: vec![(pat.span, snip.into())],
hir_id: pat.hir_id
hir_id: pat.hir_id,
}),
);
}
@ -542,6 +568,8 @@ impl<'tcx> LateLintPass<'tcx> for Dereferencing {
self.current_body = None;
}
}
extract_msrv_attr!(LateContext);
}
fn try_parse_ref_op<'tcx>(
@ -594,6 +622,7 @@ enum Position {
/// The method is defined on a reference type. e.g. `impl Foo for &T`
MethodReceiverRefImpl,
Callee,
ImplArg(HirId),
FieldAccess(Symbol),
Postfix,
Deref,
@ -630,7 +659,7 @@ impl Position {
| Self::Callee
| Self::FieldAccess(_)
| Self::Postfix => PREC_POSTFIX,
Self::Deref => PREC_PREFIX,
Self::ImplArg(_) | Self::Deref => PREC_PREFIX,
Self::DerefStable(p, _) | Self::ReborrowStable(p) | Self::Other(p) => p,
}
}
@ -639,8 +668,12 @@ impl Position {
/// Walks up the parent expressions attempting to determine both how stable the auto-deref result
/// is, and which adjustments will be applied to it. Note this will not consider auto-borrow
/// locations as those follow different rules.
#[allow(clippy::too_many_lines)]
fn walk_parents<'tcx>(cx: &LateContext<'tcx>, e: &'tcx Expr<'_>) -> (Position, &'tcx [Adjustment<'tcx>]) {
#[expect(clippy::too_many_lines)]
fn walk_parents<'tcx>(
cx: &LateContext<'tcx>,
e: &'tcx Expr<'_>,
msrv: Option<RustcVersion>,
) -> (Position, &'tcx [Adjustment<'tcx>]) {
let mut adjustments = [].as_slice();
let mut precedence = 0i8;
let ctxt = e.span.ctxt();
@ -732,13 +765,20 @@ fn walk_parents<'tcx>(cx: &LateContext<'tcx>, e: &'tcx Expr<'_>) -> (Position, &
.iter()
.position(|arg| arg.hir_id == child_id)
.zip(expr_sig(cx, func))
.and_then(|(i, sig)| sig.input_with_hir(i))
.map(|(hir_ty, ty)| match hir_ty {
// Type inference for closures can depend on how they're called. Only go by the explicit
// types here.
Some(hir_ty) => binding_ty_auto_deref_stability(cx, hir_ty, precedence, ty.bound_vars()),
None => ty_auto_deref_stability(cx, cx.tcx.erase_late_bound_regions(ty), precedence)
.position_for_arg(),
.and_then(|(i, sig)| {
sig.input_with_hir(i).map(|(hir_ty, ty)| match hir_ty {
// Type inference for closures can depend on how they're called. Only go by the explicit
// types here.
Some(hir_ty) => binding_ty_auto_deref_stability(cx, hir_ty, precedence, ty.bound_vars()),
None => {
if let ty::Param(param_ty) = ty.skip_binder().kind() {
needless_borrow_impl_arg_position(cx, parent, i, *param_ty, e, precedence, msrv)
} else {
ty_auto_deref_stability(cx, cx.tcx.erase_late_bound_regions(ty), precedence)
.position_for_arg()
}
},
})
}),
ExprKind::MethodCall(_, args, _) => {
let id = cx.typeck_results().type_dependent_def_id(parent.hir_id).unwrap();
@ -779,12 +819,17 @@ fn walk_parents<'tcx>(cx: &LateContext<'tcx>, e: &'tcx Expr<'_>) -> (Position, &
Position::MethodReceiver
}
} else {
ty_auto_deref_stability(
cx,
cx.tcx.erase_late_bound_regions(cx.tcx.fn_sig(id).input(i)),
precedence,
)
.position_for_arg()
let ty = cx.tcx.fn_sig(id).skip_binder().inputs()[i];
if let ty::Param(param_ty) = ty.kind() {
needless_borrow_impl_arg_position(cx, parent, i, *param_ty, e, precedence, msrv)
} else {
ty_auto_deref_stability(
cx,
cx.tcx.erase_late_bound_regions(cx.tcx.fn_sig(id).input(i)),
precedence,
)
.position_for_arg()
}
}
})
},
@ -946,6 +991,205 @@ fn ty_contains_infer(ty: &hir::Ty<'_>) -> bool {
v.0
}
// Checks whether:
// * child is an expression of the form `&e` in an argument position requiring an `impl Trait`
// * `e`'s type implements `Trait` and is copyable
// If the conditions are met, returns `Some(Position::ImplArg(..))`; otherwise, returns `None`.
// The "is copyable" condition is to avoid the case where removing the `&` means `e` would have to
// be moved, but it cannot be.
fn needless_borrow_impl_arg_position<'tcx>(
cx: &LateContext<'tcx>,
parent: &Expr<'tcx>,
arg_index: usize,
param_ty: ParamTy,
mut expr: &Expr<'tcx>,
precedence: i8,
msrv: Option<RustcVersion>,
) -> Position {
let destruct_trait_def_id = cx.tcx.lang_items().destruct_trait();
let sized_trait_def_id = cx.tcx.lang_items().sized_trait();
let Some(callee_def_id) = fn_def_id(cx, parent) else { return Position::Other(precedence) };
let fn_sig = cx.tcx.fn_sig(callee_def_id).skip_binder();
let substs_with_expr_ty = cx
.typeck_results()
.node_substs(if let ExprKind::Call(callee, _) = parent.kind {
callee.hir_id
} else {
parent.hir_id
});
let predicates = cx.tcx.param_env(callee_def_id).caller_bounds();
let projection_predicates = predicates
.iter()
.filter_map(|predicate| {
if let PredicateKind::Projection(projection_predicate) = predicate.kind().skip_binder() {
Some(projection_predicate)
} else {
None
}
})
.collect::<Vec<_>>();
let mut trait_with_ref_mut_self_method = false;
// If no traits were found, or only the `Destruct`, `Sized`, or `Any` traits were found, return.
if predicates
.iter()
.filter_map(|predicate| {
if let PredicateKind::Trait(trait_predicate) = predicate.kind().skip_binder()
&& trait_predicate.trait_ref.self_ty() == param_ty.to_ty(cx.tcx)
{
Some(trait_predicate.trait_ref.def_id)
} else {
None
}
})
.inspect(|trait_def_id| {
trait_with_ref_mut_self_method |= has_ref_mut_self_method(cx, *trait_def_id);
})
.all(|trait_def_id| {
Some(trait_def_id) == destruct_trait_def_id
|| Some(trait_def_id) == sized_trait_def_id
|| cx.tcx.is_diagnostic_item(sym::Any, trait_def_id)
})
{
return Position::Other(precedence);
}
// `substs_with_referent_ty` can be constructed outside of `check_referent` because the same
// elements are modified each time `check_referent` is called.
let mut substs_with_referent_ty = substs_with_expr_ty.to_vec();
let mut check_referent = |referent| {
let referent_ty = cx.typeck_results().expr_ty(referent);
if !is_copy(cx, referent_ty) {
return false;
}
// https://github.com/rust-lang/rust-clippy/pull/9136#pullrequestreview-1037379321
if trait_with_ref_mut_self_method && !matches!(referent_ty.kind(), ty::Ref(_, _, Mutability::Mut)) {
return false;
}
if !replace_types(
cx,
param_ty,
referent_ty,
fn_sig,
arg_index,
&projection_predicates,
&mut substs_with_referent_ty,
) {
return false;
}
predicates.iter().all(|predicate| {
if let PredicateKind::Trait(trait_predicate) = predicate.kind().skip_binder()
&& cx.tcx.is_diagnostic_item(sym::IntoIterator, trait_predicate.trait_ref.def_id)
&& let ty::Param(param_ty) = trait_predicate.self_ty().kind()
&& let GenericArgKind::Type(ty) = substs_with_referent_ty[param_ty.index as usize].unpack()
&& ty.is_array()
&& !meets_msrv(msrv, msrvs::ARRAY_INTO_ITERATOR)
{
return false;
}
let predicate = EarlyBinder(predicate).subst(cx.tcx, &substs_with_referent_ty);
let obligation = Obligation::new(ObligationCause::dummy(), cx.param_env, predicate);
cx.tcx
.infer_ctxt()
.enter(|infcx| infcx.predicate_must_hold_modulo_regions(&obligation))
})
};
let mut needless_borrow = false;
while let ExprKind::AddrOf(_, _, referent) = expr.kind {
if !check_referent(referent) {
break;
}
expr = referent;
needless_borrow = true;
}
if needless_borrow {
Position::ImplArg(expr.hir_id)
} else {
Position::Other(precedence)
}
}
fn has_ref_mut_self_method(cx: &LateContext<'_>, trait_def_id: DefId) -> bool {
cx.tcx
.associated_items(trait_def_id)
.in_definition_order()
.any(|assoc_item| {
if assoc_item.fn_has_self_parameter {
let self_ty = cx.tcx.fn_sig(assoc_item.def_id).skip_binder().inputs()[0];
matches!(self_ty.kind(), ty::Ref(_, _, Mutability::Mut))
} else {
false
}
})
}
// Iteratively replaces `param_ty` with `new_ty` in `substs`, and similarly for each resulting
// projected type that is a type parameter. Returns `false` if replacing the types would have an
// effect on the function signature beyond substituting `new_ty` for `param_ty`.
// See: https://github.com/rust-lang/rust-clippy/pull/9136#discussion_r927212757
fn replace_types<'tcx>(
cx: &LateContext<'tcx>,
param_ty: ParamTy,
new_ty: Ty<'tcx>,
fn_sig: FnSig<'tcx>,
arg_index: usize,
projection_predicates: &[ProjectionPredicate<'tcx>],
substs: &mut [ty::GenericArg<'tcx>],
) -> bool {
let mut replaced = BitSet::new_empty(substs.len());
let mut deque = VecDeque::with_capacity(substs.len());
deque.push_back((param_ty, new_ty));
while let Some((param_ty, new_ty)) = deque.pop_front() {
// If `replaced.is_empty()`, then `param_ty` and `new_ty` are those initially passed in.
if !fn_sig
.inputs_and_output
.iter()
.enumerate()
.all(|(i, ty)| (replaced.is_empty() && i == arg_index) || !contains_ty(ty, param_ty.to_ty(cx.tcx)))
{
return false;
}
substs[param_ty.index as usize] = ty::GenericArg::from(new_ty);
// The `replaced.insert(...)` check provides some protection against infinite loops.
if replaced.insert(param_ty.index) {
for projection_predicate in projection_predicates {
if projection_predicate.projection_ty.self_ty() == param_ty.to_ty(cx.tcx)
&& let ty::Term::Ty(term_ty) = projection_predicate.term
&& let ty::Param(term_param_ty) = term_ty.kind()
{
let item_def_id = projection_predicate.projection_ty.item_def_id;
let assoc_item = cx.tcx.associated_item(item_def_id);
let projection = cx.tcx
.mk_projection(assoc_item.def_id, cx.tcx.mk_substs_trait(new_ty, &[]));
if let Ok(projected_ty) = cx.tcx.try_normalize_erasing_regions(cx.param_env, projection)
&& substs[term_param_ty.index as usize] != ty::GenericArg::from(projected_ty)
{
deque.push_back((*term_param_ty, projected_ty));
}
}
}
}
}
true
}
struct TyPosition<'tcx> {
position: Position,
ty: Option<Ty<'tcx>>,
@ -1084,7 +1328,8 @@ fn report<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>, state: State, data
},
State::DerefedBorrow(state) => {
let mut app = Applicability::MachineApplicable;
let (snip, snip_is_macro) = snippet_with_context(cx, expr.span, data.span.ctxt(), "..", &mut app);
let snip_expr = state.snip_expr.map_or(expr, |hir_id| cx.tcx.hir().expect_expr(hir_id));
let (snip, snip_is_macro) = snippet_with_context(cx, snip_expr.span, data.span.ctxt(), "..", &mut app);
span_lint_hir_and_then(cx, NEEDLESS_BORROW, data.hir_id, data.span, state.msg, |diag| {
let calls_field = matches!(expr.kind, ExprKind::Field(..)) && matches!(data.position, Position::Callee);
let sugg = if !snip_is_macro

View file

@ -821,7 +821,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
store.register_late_pass(|| Box::new(verbose_file_reads::VerboseFileReads));
store.register_late_pass(|| Box::new(redundant_pub_crate::RedundantPubCrate::default()));
store.register_late_pass(|| Box::new(unnamed_address::UnnamedAddress));
store.register_late_pass(|| Box::new(dereference::Dereferencing::default()));
store.register_late_pass(move || Box::new(dereference::Dereferencing::new(msrv)));
store.register_late_pass(|| Box::new(option_if_let_else::OptionIfLetElse));
store.register_late_pass(|| Box::new(future_not_send::FutureNotSend));
store.register_late_pass(|| Box::new(if_let_mutex::IfLetMutex));

View file

@ -6,9 +6,8 @@ use clippy_utils::ty::{
contains_ty, get_associated_type, get_iterator_item_ty, implements_trait, is_copy, is_type_diagnostic_item,
peel_mid_ty_refs,
};
use clippy_utils::{meets_msrv, msrvs};
use clippy_utils::{fn_def_id, get_parent_expr, is_diag_item_method, is_diag_trait_item};
use clippy_utils::{meets_msrv, msrvs};
use rustc_errors::Applicability;
use rustc_hir::{def_id::DefId, BorrowKind, Expr, ExprKind};
use rustc_lint::LateContext;
@ -373,25 +372,15 @@ fn get_input_traits_and_projections<'tcx>(
) -> (Vec<TraitPredicate<'tcx>>, Vec<ProjectionPredicate<'tcx>>) {
let mut trait_predicates = Vec::new();
let mut projection_predicates = Vec::new();
for (predicate, _) in cx.tcx.predicates_of(callee_def_id).predicates.iter() {
// `substs` should have 1 + n elements. The first is the type on the left hand side of an
// `as`. The remaining n are trait parameters.
let is_input_substs = |substs: SubstsRef<'tcx>| {
if_chain! {
if let Some(arg) = substs.iter().next();
if let GenericArgKind::Type(arg_ty) = arg.unpack();
if arg_ty == input;
then { true } else { false }
}
};
for predicate in cx.tcx.param_env(callee_def_id).caller_bounds() {
match predicate.kind().skip_binder() {
PredicateKind::Trait(trait_predicate) => {
if is_input_substs(trait_predicate.trait_ref.substs) {
if trait_predicate.trait_ref.self_ty() == input {
trait_predicates.push(trait_predicate);
}
},
PredicateKind::Projection(projection_predicate) => {
if is_input_substs(projection_predicate.projection_ty.substs) {
if projection_predicate.projection_ty.self_ty() == input {
projection_predicates.push(projection_predicate);
}
},

View file

@ -13,7 +13,7 @@ macro_rules! msrv_aliases {
// names may refer to stabilized feature flags or library items
msrv_aliases! {
1,62,0 { BOOL_THEN_SOME }
1,53,0 { OR_PATTERNS, MANUAL_BITS, BTREE_MAP_RETAIN, BTREE_SET_RETAIN }
1,53,0 { OR_PATTERNS, MANUAL_BITS, BTREE_MAP_RETAIN, BTREE_SET_RETAIN, ARRAY_INTO_ITERATOR }
1,52,0 { STR_SPLIT_ONCE, REM_EUCLID_CONST }
1,51,0 { BORROW_AS_PTR, UNSIGNED_ABS }
1,50,0 { BOOL_THEN }

View file

@ -84,7 +84,7 @@ impl std::fmt::Debug for VersionInfo {
#[must_use]
pub fn get_commit_hash() -> Option<String> {
std::process::Command::new("git")
.args(&["rev-parse", "--short", "HEAD"])
.args(["rev-parse", "--short", "HEAD"])
.output()
.ok()
.and_then(|r| String::from_utf8(r.stdout).ok())
@ -93,7 +93,7 @@ pub fn get_commit_hash() -> Option<String> {
#[must_use]
pub fn get_commit_date() -> Option<String> {
std::process::Command::new("git")
.args(&["log", "-1", "--date=short", "--pretty=format:%cd"])
.args(["log", "-1", "--date=short", "--pretty=format:%cd"])
.output()
.ok()
.and_then(|r| String::from_utf8(r.stdout).ok())

View file

@ -13,7 +13,7 @@ fn fmt() {
let root_dir = PathBuf::from(env!("CARGO_MANIFEST_DIR"));
let output = Command::new("cargo")
.current_dir(root_dir)
.args(&["dev", "fmt", "--check"])
.args(["dev", "fmt", "--check"])
.output()
.unwrap();

View file

@ -87,11 +87,11 @@ fn run_clippy_for_package(project: &str, args: &[&str]) {
if cfg!(feature = "internal") {
// internal lints only exist if we build with the internal feature
command.args(&["-D", "clippy::internal"]);
command.args(["-D", "clippy::internal"]);
} else {
// running a clippy built without internal lints on the clippy source
// that contains e.g. `allow(clippy::invalid_paths)`
command.args(&["-A", "unknown_lints"]);
command.args(["-A", "unknown_lints"]);
}
let output = command.output().unwrap();

View file

@ -19,7 +19,7 @@ fn integration_test() {
repo_dir.push(crate_name);
let st = Command::new("git")
.args(&[
.args([
OsStr::new("clone"),
OsStr::new("--depth=1"),
OsStr::new(&repo_url),
@ -37,7 +37,7 @@ fn integration_test() {
.current_dir(repo_dir)
.env("RUST_BACKTRACE", "full")
.env("CARGO_TARGET_DIR", target_dir)
.args(&[
.args([
"clippy",
"--all-targets",
"--all-features",

View file

@ -19,7 +19,7 @@ impl Message {
// we don't want the first letter after "error: ", "help: " ... to be capitalized
// also no punctuation (except for "?" ?) at the end of a line
static REGEX_SET: LazyLock<RegexSet> = LazyLock::new(|| {
RegexSet::new(&[
RegexSet::new([
r"error: [A-Z]",
r"help: [A-Z]",
r"warning: [A-Z]",
@ -37,7 +37,7 @@ impl Message {
// sometimes the first character is capitalized and it is legal (like in "C-like enum variants") or
// we want to ask a question ending in "?"
static EXCEPTIONS_SET: LazyLock<RegexSet> = LazyLock::new(|| {
RegexSet::new(&[
RegexSet::new([
r"\.\.\.$",
r".*C-like enum variant discriminant is not portable to 32-bit targets",
r".*Intel x86 assembly syntax used",

View file

@ -1,6 +1,6 @@
// run-rustfix
#![feature(lint_reasons)]
#![feature(custom_inner_attributes, lint_reasons)]
#[warn(clippy::all, clippy::needless_borrow)]
#[allow(unused_variables, clippy::unnecessary_mut_passed)]
@ -127,6 +127,20 @@ fn main() {
0
}
}
let _ = std::process::Command::new("ls").args(["-a", "-l"]).status().unwrap();
let _ = std::path::Path::new(".").join(".");
deref_target_is_x(X);
multiple_constraints([[""]]);
multiple_constraints_normalizes_to_same(X, X);
let _ = Some("").unwrap_or("");
only_sized(&""); // Don't lint. `Sized` is only bound
let _ = std::any::Any::type_id(&""); // Don't lint. `Any` is only bound
let _ = Box::new(&""); // Don't lint. Type parameter appears in return type
ref_as_ref_path(&""); // Don't lint. Argument type is not a type parameter
refs_only(&()); // Don't lint. `&T` implements trait, but `T` doesn't
multiple_constraints_normalizes_to_different(&[[""]], &[""]); // Don't lint. Projected type appears in arguments
}
#[allow(clippy::needless_borrowed_reference)]
@ -183,3 +197,104 @@ mod issue9160 {
}
}
}
#[derive(Clone, Copy)]
struct X;
impl std::ops::Deref for X {
type Target = X;
fn deref(&self) -> &Self::Target {
self
}
}
fn deref_target_is_x<T>(_: T)
where
T: std::ops::Deref<Target = X>,
{
}
fn multiple_constraints<T, U, V, X, Y>(_: T)
where
T: IntoIterator<Item = U> + IntoIterator<Item = X>,
U: IntoIterator<Item = V>,
V: AsRef<str>,
X: IntoIterator<Item = Y>,
Y: AsRef<std::ffi::OsStr>,
{
}
fn multiple_constraints_normalizes_to_same<T, U, V>(_: T, _: V)
where
T: std::ops::Deref<Target = U>,
U: std::ops::Deref<Target = V>,
{
}
fn only_sized<T>(_: T) {}
fn ref_as_ref_path<T: 'static>(_: &'static T)
where
&'static T: AsRef<std::path::Path>,
{
}
trait RefsOnly {
type Referent;
}
impl<T> RefsOnly for &T {
type Referent = T;
}
fn refs_only<T, U>(_: T)
where
T: RefsOnly<Referent = U>,
{
}
fn multiple_constraints_normalizes_to_different<T, U, V>(_: T, _: U)
where
T: IntoIterator<Item = U>,
U: IntoIterator<Item = V>,
V: AsRef<str>,
{
}
// https://github.com/rust-lang/rust-clippy/pull/9136#pullrequestreview-1037379321
#[allow(dead_code)]
mod copyable_iterator {
#[derive(Clone, Copy)]
struct Iter;
impl Iterator for Iter {
type Item = ();
fn next(&mut self) -> Option<Self::Item> {
None
}
}
fn takes_iter(_: impl Iterator) {}
fn dont_warn(mut x: Iter) {
takes_iter(&mut x);
}
fn warn(mut x: &mut Iter) {
takes_iter(&mut x)
}
}
mod under_msrv {
#![allow(dead_code)]
#![clippy::msrv = "1.52.0"]
fn foo() {
let _ = std::process::Command::new("ls").args(&["-a", "-l"]).status().unwrap();
}
}
mod meets_msrv {
#![allow(dead_code)]
#![clippy::msrv = "1.53.0"]
fn foo() {
let _ = std::process::Command::new("ls").args(["-a", "-l"]).status().unwrap();
}
}

View file

@ -1,6 +1,6 @@
// run-rustfix
#![feature(lint_reasons)]
#![feature(custom_inner_attributes, lint_reasons)]
#[warn(clippy::all, clippy::needless_borrow)]
#[allow(unused_variables, clippy::unnecessary_mut_passed)]
@ -127,6 +127,20 @@ fn main() {
0
}
}
let _ = std::process::Command::new("ls").args(&["-a", "-l"]).status().unwrap();
let _ = std::path::Path::new(".").join(&&".");
deref_target_is_x(&X);
multiple_constraints(&[[""]]);
multiple_constraints_normalizes_to_same(&X, X);
let _ = Some("").unwrap_or(&"");
only_sized(&""); // Don't lint. `Sized` is only bound
let _ = std::any::Any::type_id(&""); // Don't lint. `Any` is only bound
let _ = Box::new(&""); // Don't lint. Type parameter appears in return type
ref_as_ref_path(&""); // Don't lint. Argument type is not a type parameter
refs_only(&()); // Don't lint. `&T` implements trait, but `T` doesn't
multiple_constraints_normalizes_to_different(&[[""]], &[""]); // Don't lint. Projected type appears in arguments
}
#[allow(clippy::needless_borrowed_reference)]
@ -183,3 +197,104 @@ mod issue9160 {
}
}
}
#[derive(Clone, Copy)]
struct X;
impl std::ops::Deref for X {
type Target = X;
fn deref(&self) -> &Self::Target {
self
}
}
fn deref_target_is_x<T>(_: T)
where
T: std::ops::Deref<Target = X>,
{
}
fn multiple_constraints<T, U, V, X, Y>(_: T)
where
T: IntoIterator<Item = U> + IntoIterator<Item = X>,
U: IntoIterator<Item = V>,
V: AsRef<str>,
X: IntoIterator<Item = Y>,
Y: AsRef<std::ffi::OsStr>,
{
}
fn multiple_constraints_normalizes_to_same<T, U, V>(_: T, _: V)
where
T: std::ops::Deref<Target = U>,
U: std::ops::Deref<Target = V>,
{
}
fn only_sized<T>(_: T) {}
fn ref_as_ref_path<T: 'static>(_: &'static T)
where
&'static T: AsRef<std::path::Path>,
{
}
trait RefsOnly {
type Referent;
}
impl<T> RefsOnly for &T {
type Referent = T;
}
fn refs_only<T, U>(_: T)
where
T: RefsOnly<Referent = U>,
{
}
fn multiple_constraints_normalizes_to_different<T, U, V>(_: T, _: U)
where
T: IntoIterator<Item = U>,
U: IntoIterator<Item = V>,
V: AsRef<str>,
{
}
// https://github.com/rust-lang/rust-clippy/pull/9136#pullrequestreview-1037379321
#[allow(dead_code)]
mod copyable_iterator {
#[derive(Clone, Copy)]
struct Iter;
impl Iterator for Iter {
type Item = ();
fn next(&mut self) -> Option<Self::Item> {
None
}
}
fn takes_iter(_: impl Iterator) {}
fn dont_warn(mut x: Iter) {
takes_iter(&mut x);
}
fn warn(mut x: &mut Iter) {
takes_iter(&mut x)
}
}
mod under_msrv {
#![allow(dead_code)]
#![clippy::msrv = "1.52.0"]
fn foo() {
let _ = std::process::Command::new("ls").args(&["-a", "-l"]).status().unwrap();
}
}
mod meets_msrv {
#![allow(dead_code)]
#![clippy::msrv = "1.53.0"]
fn foo() {
let _ = std::process::Command::new("ls").args(&["-a", "-l"]).status().unwrap();
}
}

View file

@ -120,17 +120,59 @@ error: this expression creates a reference which is immediately dereferenced by
LL | (&&5).foo();
| ^^^^^ help: change this to: `(&5)`
error: the borrowed expression implements the required traits
--> $DIR/needless_borrow.rs:131:51
|
LL | let _ = std::process::Command::new("ls").args(&["-a", "-l"]).status().unwrap();
| ^^^^^^^^^^^^^ help: change this to: `["-a", "-l"]`
error: the borrowed expression implements the required traits
--> $DIR/needless_borrow.rs:132:44
|
LL | let _ = std::path::Path::new(".").join(&&".");
| ^^^^^ help: change this to: `"."`
error: the borrowed expression implements the required traits
--> $DIR/needless_borrow.rs:133:23
|
LL | deref_target_is_x(&X);
| ^^ help: change this to: `X`
error: the borrowed expression implements the required traits
--> $DIR/needless_borrow.rs:134:26
|
LL | multiple_constraints(&[[""]]);
| ^^^^^^^ help: change this to: `[[""]]`
error: the borrowed expression implements the required traits
--> $DIR/needless_borrow.rs:135:45
|
LL | multiple_constraints_normalizes_to_same(&X, X);
| ^^ help: change this to: `X`
error: this expression creates a reference which is immediately dereferenced by the compiler
--> $DIR/needless_borrow.rs:136:32
|
LL | let _ = Some("").unwrap_or(&"");
| ^^^ help: change this to: `""`
error: this expression borrows a value the compiler would automatically borrow
--> $DIR/needless_borrow.rs:173:13
--> $DIR/needless_borrow.rs:187:13
|
LL | (&self.f)()
| ^^^^^^^^^ help: change this to: `(self.f)`
error: this expression borrows a value the compiler would automatically borrow
--> $DIR/needless_borrow.rs:182:13
--> $DIR/needless_borrow.rs:196:13
|
LL | (&mut self.f)()
| ^^^^^^^^^^^^^ help: change this to: `(self.f)`
error: aborting due to 22 previous errors
error: the borrowed expression implements the required traits
--> $DIR/needless_borrow.rs:298:55
|
LL | let _ = std::process::Command::new("ls").args(&["-a", "-l"]).status().unwrap();
| ^^^^^^^^^^^^^ help: change this to: `["-a", "-l"]`
error: aborting due to 29 previous errors

View file

@ -1,4 +1,4 @@
#![allow(unused)]
#![allow(unused, clippy::needless_borrow)]
#![warn(clippy::invalid_regex, clippy::trivial_regex)]
extern crate regex;

View file

@ -151,6 +151,7 @@ fn main() {
// Fix #6987
let mut vec = Vec::new();
#[allow(clippy::needless_borrow)]
for _ in 0..10 {
vec.push(1);
vec.extend(&[2]);

View file

@ -18,7 +18,7 @@ fn main() -> std::io::Result<()> {
s.read_to_end();
s.read_to_string();
// Should catch this
let mut f = File::open(&path)?;
let mut f = File::open(path)?;
let mut buffer = Vec::new();
f.read_to_end(&mut buffer)?;
// ...and this

View file

@ -20,8 +20,8 @@ fn test_no_deps_ignores_path_deps_in_workspaces() {
.current_dir(&cwd)
.env("CARGO_TARGET_DIR", &target_dir)
.arg("clean")
.args(&["-p", "subcrate"])
.args(&["-p", "path_dep"])
.args(["-p", "subcrate"])
.args(["-p", "path_dep"])
.output()
.unwrap();
@ -32,11 +32,11 @@ fn test_no_deps_ignores_path_deps_in_workspaces() {
.env("CARGO_INCREMENTAL", "0")
.env("CARGO_TARGET_DIR", &target_dir)
.arg("clippy")
.args(&["-p", "subcrate"])
.args(["-p", "subcrate"])
.arg("--no-deps")
.arg("--")
.arg("-Cdebuginfo=0") // disable debuginfo to generate less data in the target dir
.args(&["--cfg", r#"feature="primary_package_test""#])
.args(["--cfg", r#"feature="primary_package_test""#])
.output()
.unwrap();
println!("status: {}", output.status);
@ -52,10 +52,10 @@ fn test_no_deps_ignores_path_deps_in_workspaces() {
.env("CARGO_INCREMENTAL", "0")
.env("CARGO_TARGET_DIR", &target_dir)
.arg("clippy")
.args(&["-p", "subcrate"])
.args(["-p", "subcrate"])
.arg("--")
.arg("-Cdebuginfo=0") // disable debuginfo to generate less data in the target dir
.args(&["--cfg", r#"feature="primary_package_test""#])
.args(["--cfg", r#"feature="primary_package_test""#])
.output()
.unwrap();
println!("status: {}", output.status);
@ -79,7 +79,7 @@ fn test_no_deps_ignores_path_deps_in_workspaces() {
.env("CARGO_INCREMENTAL", "0")
.env("CARGO_TARGET_DIR", &target_dir)
.arg("clippy")
.args(&["-p", "subcrate"])
.args(["-p", "subcrate"])
.arg("--")
.arg("-Cdebuginfo=0") // disable debuginfo to generate less data in the target dir
.output()