Skip to content

Commit 4a53f00

Browse files
committed
Ignore Destruct and check msrv
1 parent 5c5bf96 commit 4a53f00

File tree

6 files changed

+101
-16
lines changed

6 files changed

+101
-16
lines changed

clippy_lints/src/dereference.rs

Lines changed: 44 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ use clippy_utils::diagnostics::{span_lint_and_sugg, span_lint_hir_and_then};
22
use clippy_utils::source::{snippet_with_applicability, snippet_with_context};
33
use clippy_utils::sugg::has_enclosing_paren;
44
use clippy_utils::ty::{contains_ty, expr_sig, is_copy, peel_mid_ty_refs, variant_of_res};
5-
use clippy_utils::{fn_def_id, get_parent_expr, is_lint_allowed, path_to_local, walk_to_expr_usage};
5+
use clippy_utils::{fn_def_id, get_parent_expr, is_lint_allowed, meets_msrv, msrvs, path_to_local, walk_to_expr_usage};
66
use rustc_ast::util::parser::{PREC_POSTFIX, PREC_PREFIX};
77
use rustc_data_structures::fx::FxIndexMap;
88
use rustc_errors::Applicability;
@@ -17,9 +17,10 @@ use rustc_infer::infer::TyCtxtInferExt;
1717
use rustc_lint::{LateContext, LateLintPass};
1818
use rustc_middle::ty::adjustment::{Adjust, Adjustment, AutoBorrow, AutoBorrowMutability};
1919
use rustc_middle::ty::{
20-
self, subst::Subst, EarlyBinder, FnSig, ParamTy, PredicateKind, ProjectionPredicate, Ty, TyCtxt, TypeVisitable,
21-
TypeckResults,
20+
self, subst::Subst, EarlyBinder, FnSig, GenericArgKind, ParamTy, PredicateKind, ProjectionPredicate, Ty, TyCtxt,
21+
TypeVisitable, TypeckResults,
2222
};
23+
use rustc_semver::RustcVersion;
2324
use rustc_session::{declare_tool_lint, impl_lint_pass};
2425
use rustc_span::{symbol::sym, Span, Symbol};
2526
use rustc_trait_selection::infer::InferCtxtExt as _;
@@ -157,13 +158,27 @@ pub struct Dereferencing {
157158
/// been finished. Note we can't lint at the end of every body as they can be nested within each
158159
/// other.
159160
current_body: Option<BodyId>,
161+
160162
/// The list of locals currently being checked by the lint.
161163
/// If the value is `None`, then the binding has been seen as a ref pattern, but is not linted.
162164
/// This is needed for or patterns where one of the branches can be linted, but another can not
163165
/// be.
164166
///
165167
/// e.g. `m!(x) | Foo::Bar(ref x)`
166168
ref_locals: FxIndexMap<HirId, Option<RefPat>>,
169+
170+
// `IntoIterator` for arrays requires Rust 1.53.
171+
msrv: Option<RustcVersion>,
172+
}
173+
174+
impl Dereferencing {
175+
#[must_use]
176+
pub fn new(msrv: Option<RustcVersion>) -> Self {
177+
Self {
178+
msrv,
179+
..Dereferencing::default()
180+
}
181+
}
167182
}
168183

169184
struct StateData {
@@ -257,7 +272,7 @@ impl<'tcx> LateLintPass<'tcx> for Dereferencing {
257272
match (self.state.take(), kind) {
258273
(None, kind) => {
259274
let expr_ty = typeck.expr_ty(expr);
260-
let (position, adjustments) = walk_parents(cx, expr);
275+
let (position, adjustments) = walk_parents(cx, expr, self.msrv);
261276

262277
match kind {
263278
RefOp::Deref => {
@@ -564,6 +579,8 @@ impl<'tcx> LateLintPass<'tcx> for Dereferencing {
564579
self.current_body = None;
565580
}
566581
}
582+
583+
extract_msrv_attr!(LateContext);
567584
}
568585

569586
fn try_parse_ref_op<'tcx>(
@@ -660,7 +677,11 @@ impl Position {
660677
/// is, and which adjustments will be applied to it. Note this will not consider auto-borrow
661678
/// locations as those follow different rules.
662679
#[expect(clippy::too_many_lines)]
663-
fn walk_parents<'tcx>(cx: &LateContext<'tcx>, e: &'tcx Expr<'_>) -> (Position, &'tcx [Adjustment<'tcx>]) {
680+
fn walk_parents<'tcx>(
681+
cx: &LateContext<'tcx>,
682+
e: &'tcx Expr<'_>,
683+
msrv: Option<RustcVersion>,
684+
) -> (Position, &'tcx [Adjustment<'tcx>]) {
664685
let mut adjustments = [].as_slice();
665686
let mut precedence = 0i8;
666687
let ctxt = e.span.ctxt();
@@ -770,7 +791,7 @@ fn walk_parents<'tcx>(cx: &LateContext<'tcx>, e: &'tcx Expr<'_>) -> (Position, &
770791
Some(ty) => binding_ty_auto_deref_stability(ty, precedence),
771792
None => {
772793
if let ty::Param(param_ty) = ty.skip_binder().kind() {
773-
needless_borrow_impl_arg_position(cx, parent, i, *param_ty, e, precedence)
794+
needless_borrow_impl_arg_position(cx, parent, i, *param_ty, e, precedence, msrv)
774795
} else {
775796
param_auto_deref_stability(ty.skip_binder(), precedence)
776797
}
@@ -818,7 +839,7 @@ fn walk_parents<'tcx>(cx: &LateContext<'tcx>, e: &'tcx Expr<'_>) -> (Position, &
818839
} else {
819840
let ty = cx.tcx.fn_sig(id).skip_binder().inputs()[i];
820841
if let ty::Param(param_ty) = ty.kind() {
821-
needless_borrow_impl_arg_position(cx, parent, i, *param_ty, e, precedence)
842+
needless_borrow_impl_arg_position(cx, parent, i, *param_ty, e, precedence, msrv)
822843
} else {
823844
param_auto_deref_stability(ty, precedence)
824845
}
@@ -957,7 +978,9 @@ fn needless_borrow_impl_arg_position<'tcx>(
957978
param_ty: ParamTy,
958979
mut expr: &Expr<'tcx>,
959980
precedence: i8,
981+
msrv: Option<RustcVersion>,
960982
) -> Position {
983+
let destruct_trait_def_id = cx.tcx.lang_items().destruct_trait();
961984
let sized_trait_def_id = cx.tcx.lang_items().sized_trait();
962985

963986
let Some(callee_def_id) = fn_def_id(cx, parent) else { return Position::Other(precedence) };
@@ -984,7 +1007,7 @@ fn needless_borrow_impl_arg_position<'tcx>(
9841007

9851008
let mut trait_with_ref_mut_self_method = false;
9861009

987-
// If no traits were found, or only the `Sized` or `Any` traits were found, return.
1010+
// If no traits were found, or only the `Destruct`, `Sized`, or `Any` traits were found, return.
9881011
if predicates
9891012
.iter()
9901013
.filter_map(|predicate| {
@@ -1000,7 +1023,9 @@ fn needless_borrow_impl_arg_position<'tcx>(
10001023
trait_with_ref_mut_self_method |= has_ref_mut_self_method(cx, *trait_def_id);
10011024
})
10021025
.all(|trait_def_id| {
1003-
Some(trait_def_id) == sized_trait_def_id || cx.tcx.is_diagnostic_item(sym::Any, trait_def_id)
1026+
Some(trait_def_id) == destruct_trait_def_id
1027+
|| Some(trait_def_id) == sized_trait_def_id
1028+
|| cx.tcx.is_diagnostic_item(sym::Any, trait_def_id)
10041029
})
10051030
{
10061031
return Position::Other(precedence);
@@ -1035,6 +1060,16 @@ fn needless_borrow_impl_arg_position<'tcx>(
10351060
}
10361061

10371062
predicates.iter().all(|predicate| {
1063+
if let PredicateKind::Trait(trait_predicate) = predicate.kind().skip_binder()
1064+
&& cx.tcx.is_diagnostic_item(sym::IntoIterator, trait_predicate.trait_ref.def_id)
1065+
&& let ty::Param(param_ty) = trait_predicate.self_ty().kind()
1066+
&& let GenericArgKind::Type(ty) = substs_with_referent_ty[param_ty.index as usize].unpack()
1067+
&& ty.is_array()
1068+
&& !meets_msrv(msrv, msrvs::ARRAY_INTO_ITERATOR)
1069+
{
1070+
return false;
1071+
}
1072+
10381073
let predicate = EarlyBinder(predicate).subst(cx.tcx, &substs_with_referent_ty);
10391074
let obligation = Obligation::new(ObligationCause::dummy(), cx.param_env, predicate);
10401075
cx.tcx

clippy_lints/src/lib.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -807,7 +807,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
807807
store.register_late_pass(|| Box::new(verbose_file_reads::VerboseFileReads));
808808
store.register_late_pass(|| Box::new(redundant_pub_crate::RedundantPubCrate::default()));
809809
store.register_late_pass(|| Box::new(unnamed_address::UnnamedAddress));
810-
store.register_late_pass(|| Box::new(dereference::Dereferencing::default()));
810+
store.register_late_pass(move || Box::new(dereference::Dereferencing::new(msrv)));
811811
store.register_late_pass(|| Box::new(option_if_let_else::OptionIfLetElse));
812812
store.register_late_pass(|| Box::new(future_not_send::FutureNotSend));
813813
store.register_late_pass(|| Box::new(if_let_mutex::IfLetMutex));

clippy_utils/src/msrvs.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ macro_rules! msrv_aliases {
1313
// names may refer to stabilized feature flags or library items
1414
msrv_aliases! {
1515
1,62,0 { BOOL_THEN_SOME }
16-
1,53,0 { OR_PATTERNS, MANUAL_BITS, BTREE_MAP_RETAIN, BTREE_SET_RETAIN }
16+
1,53,0 { OR_PATTERNS, MANUAL_BITS, BTREE_MAP_RETAIN, BTREE_SET_RETAIN, ARRAY_INTO_ITERATOR }
1717
1,52,0 { STR_SPLIT_ONCE, REM_EUCLID_CONST }
1818
1,51,0 { BORROW_AS_PTR, UNSIGNED_ABS }
1919
1,50,0 { BOOL_THEN }

tests/ui/needless_borrow.fixed

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
// run-rustfix
22

3-
#![feature(lint_reasons)]
3+
#![feature(custom_inner_attributes, lint_reasons)]
44

55
#[warn(clippy::all, clippy::needless_borrow)]
66
#[allow(unused_variables, clippy::unnecessary_mut_passed)]
@@ -133,6 +133,7 @@ fn main() {
133133
deref_target_is_x(X);
134134
multiple_constraints([[""]]);
135135
multiple_constraints_normalizes_to_same(X, X);
136+
let _ = Some("").unwrap_or("");
136137

137138
only_sized(&""); // Don't lint. `Sized` is only bound
138139
let _ = std::any::Any::type_id(&""); // Don't lint. `Any` is only bound
@@ -264,3 +265,21 @@ mod copyable_iterator {
264265
takes_iter(&mut x)
265266
}
266267
}
268+
269+
mod under_msrv {
270+
#![allow(dead_code)]
271+
#![clippy::msrv = "1.52.0"]
272+
273+
fn foo() {
274+
let _ = std::process::Command::new("ls").args(&["-a", "-l"]).status().unwrap();
275+
}
276+
}
277+
278+
mod meets_msrv {
279+
#![allow(dead_code)]
280+
#![clippy::msrv = "1.53.0"]
281+
282+
fn foo() {
283+
let _ = std::process::Command::new("ls").args(["-a", "-l"]).status().unwrap();
284+
}
285+
}

tests/ui/needless_borrow.rs

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
// run-rustfix
22

3-
#![feature(lint_reasons)]
3+
#![feature(custom_inner_attributes, lint_reasons)]
44

55
#[warn(clippy::all, clippy::needless_borrow)]
66
#[allow(unused_variables, clippy::unnecessary_mut_passed)]
@@ -133,6 +133,7 @@ fn main() {
133133
deref_target_is_x(&X);
134134
multiple_constraints(&[[""]]);
135135
multiple_constraints_normalizes_to_same(&X, X);
136+
let _ = Some("").unwrap_or(&"");
136137

137138
only_sized(&""); // Don't lint. `Sized` is only bound
138139
let _ = std::any::Any::type_id(&""); // Don't lint. `Any` is only bound
@@ -264,3 +265,21 @@ mod copyable_iterator {
264265
takes_iter(&mut x)
265266
}
266267
}
268+
269+
mod under_msrv {
270+
#![allow(dead_code)]
271+
#![clippy::msrv = "1.52.0"]
272+
273+
fn foo() {
274+
let _ = std::process::Command::new("ls").args(&["-a", "-l"]).status().unwrap();
275+
}
276+
}
277+
278+
mod meets_msrv {
279+
#![allow(dead_code)]
280+
#![clippy::msrv = "1.53.0"]
281+
282+
fn foo() {
283+
let _ = std::process::Command::new("ls").args(&["-a", "-l"]).status().unwrap();
284+
}
285+
}

tests/ui/needless_borrow.stderr

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -150,17 +150,29 @@ error: the borrowed expression implements the required traits
150150
LL | multiple_constraints_normalizes_to_same(&X, X);
151151
| ^^ help: change this to: `X`
152152

153+
error: this expression creates a reference which is immediately dereferenced by the compiler
154+
--> $DIR/needless_borrow.rs:136:32
155+
|
156+
LL | let _ = Some("").unwrap_or(&"");
157+
| ^^^ help: change this to: `""`
158+
153159
error: this expression borrows a value the compiler would automatically borrow
154-
--> $DIR/needless_borrow.rs:185:13
160+
--> $DIR/needless_borrow.rs:186:13
155161
|
156162
LL | (&self.f)()
157163
| ^^^^^^^^^ help: change this to: `(self.f)`
158164

159165
error: this expression borrows a value the compiler would automatically borrow
160-
--> $DIR/needless_borrow.rs:194:13
166+
--> $DIR/needless_borrow.rs:195:13
161167
|
162168
LL | (&mut self.f)()
163169
| ^^^^^^^^^^^^^ help: change this to: `(self.f)`
164170

165-
error: aborting due to 27 previous errors
171+
error: the borrowed expression implements the required traits
172+
--> $DIR/needless_borrow.rs:283:55
173+
|
174+
LL | let _ = std::process::Command::new("ls").args(&["-a", "-l"]).status().unwrap();
175+
| ^^^^^^^^^^^^^ help: change this to: `["-a", "-l"]`
176+
177+
error: aborting due to 29 previous errors
166178

0 commit comments

Comments
 (0)