Skip to content

Commit 6e4663d

Browse files
nahuakangY-Nak
nahuakang
authored and
Y-Nak
committed
Refactor check_for_mut_range_bound to its own module
1 parent 7b0f588 commit 6e4663d

File tree

2 files changed

+116
-109
lines changed

2 files changed

+116
-109
lines changed

‎clippy_lints/src/loops/for_mut_range_bound.rs

Lines changed: 114 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,114 @@
1+
use crate::utils::{higher, path_to_local, span_lint};
2+
use if_chain::if_chain;
3+
use rustc_hir::{BindingAnnotation, Expr, HirId, Node, PatKind};
4+
use rustc_infer::infer::TyCtxtInferExt;
5+
use rustc_lint::LateContext;
6+
use rustc_middle::ty;
7+
use rustc_span::source_map::Span;
8+
use rustc_typeck::expr_use_visitor::{ConsumeMode, Delegate, ExprUseVisitor, PlaceBase, PlaceWithHirId};
9+
10+
pub(super) fn check_for_mut_range_bound(cx: &LateContext<'_>, arg: &Expr<'_>, body: &Expr<'_>) {
11+
if let Some(higher::Range {
12+
start: Some(start),
13+
end: Some(end),
14+
..
15+
}) = higher::range(arg)
16+
{
17+
let mut_ids = vec![check_for_mutability(cx, start), check_for_mutability(cx, end)];
18+
if mut_ids[0].is_some() || mut_ids[1].is_some() {
19+
let (span_low, span_high) = check_for_mutation(cx, body, &mut_ids);
20+
mut_warn_with_span(cx, span_low);
21+
mut_warn_with_span(cx, span_high);
22+
}
23+
}
24+
}
25+
26+
fn mut_warn_with_span(cx: &LateContext<'_>, span: Option<Span>) {
27+
if let Some(sp) = span {
28+
span_lint(
29+
cx,
30+
super::MUT_RANGE_BOUND,
31+
sp,
32+
"attempt to mutate range bound within loop; note that the range of the loop is unchanged",
33+
);
34+
}
35+
}
36+
37+
fn check_for_mutability(cx: &LateContext<'_>, bound: &Expr<'_>) -> Option<HirId> {
38+
if_chain! {
39+
if let Some(hir_id) = path_to_local(bound);
40+
if let Node::Binding(pat) = cx.tcx.hir().get(hir_id);
41+
if let PatKind::Binding(BindingAnnotation::Mutable, ..) = pat.kind;
42+
then {
43+
return Some(hir_id);
44+
}
45+
}
46+
None
47+
}
48+
49+
fn check_for_mutation<'tcx>(
50+
cx: &LateContext<'tcx>,
51+
body: &Expr<'_>,
52+
bound_ids: &[Option<HirId>],
53+
) -> (Option<Span>, Option<Span>) {
54+
let mut delegate = MutatePairDelegate {
55+
cx,
56+
hir_id_low: bound_ids[0],
57+
hir_id_high: bound_ids[1],
58+
span_low: None,
59+
span_high: None,
60+
};
61+
cx.tcx.infer_ctxt().enter(|infcx| {
62+
ExprUseVisitor::new(
63+
&mut delegate,
64+
&infcx,
65+
body.hir_id.owner,
66+
cx.param_env,
67+
cx.typeck_results(),
68+
)
69+
.walk_expr(body);
70+
});
71+
delegate.mutation_span()
72+
}
73+
74+
struct MutatePairDelegate<'a, 'tcx> {
75+
cx: &'a LateContext<'tcx>,
76+
hir_id_low: Option<HirId>,
77+
hir_id_high: Option<HirId>,
78+
span_low: Option<Span>,
79+
span_high: Option<Span>,
80+
}
81+
82+
impl<'tcx> Delegate<'tcx> for MutatePairDelegate<'_, 'tcx> {
83+
fn consume(&mut self, _: &PlaceWithHirId<'tcx>, _: HirId, _: ConsumeMode) {}
84+
85+
fn borrow(&mut self, cmt: &PlaceWithHirId<'tcx>, diag_expr_id: HirId, bk: ty::BorrowKind) {
86+
if let ty::BorrowKind::MutBorrow = bk {
87+
if let PlaceBase::Local(id) = cmt.place.base {
88+
if Some(id) == self.hir_id_low {
89+
self.span_low = Some(self.cx.tcx.hir().span(diag_expr_id))
90+
}
91+
if Some(id) == self.hir_id_high {
92+
self.span_high = Some(self.cx.tcx.hir().span(diag_expr_id))
93+
}
94+
}
95+
}
96+
}
97+
98+
fn mutate(&mut self, cmt: &PlaceWithHirId<'tcx>, diag_expr_id: HirId) {
99+
if let PlaceBase::Local(id) = cmt.place.base {
100+
if Some(id) == self.hir_id_low {
101+
self.span_low = Some(self.cx.tcx.hir().span(diag_expr_id))
102+
}
103+
if Some(id) == self.hir_id_high {
104+
self.span_high = Some(self.cx.tcx.hir().span(diag_expr_id))
105+
}
106+
}
107+
}
108+
}
109+
110+
impl MutatePairDelegate<'_, '_> {
111+
fn mutation_span(&self) -> (Option<Span>, Option<Span>) {
112+
(self.span_low, self.span_high)
113+
}
114+
}

‎clippy_lints/src/loops/mod.rs

Lines changed: 2 additions & 109 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
mod for_loop_arg;
22
mod for_loop_over_map_kv;
3+
mod for_mut_range_bound;
34
mod manual_flatten;
45
mod utils;
56

@@ -24,7 +25,6 @@ use rustc_hir::{
2425
def_id, BinOpKind, BindingAnnotation, Block, BorrowKind, Expr, ExprKind, GenericArg, HirId, InlineAsmOperand,
2526
Local, LoopSource, MatchSource, Mutability, Node, Pat, PatKind, QPath, Stmt, StmtKind,
2627
};
27-
use rustc_infer::infer::TyCtxtInferExt;
2828
use rustc_lint::{LateContext, LateLintPass, LintContext};
2929
use rustc_middle::hir::map::Map;
3030
use rustc_middle::lint::in_external_macro;
@@ -33,7 +33,6 @@ use rustc_middle::ty::{self, Ty};
3333
use rustc_session::{declare_lint_pass, declare_tool_lint};
3434
use rustc_span::source_map::Span;
3535
use rustc_span::symbol::{sym, Ident, Symbol};
36-
use rustc_typeck::expr_use_visitor::{ConsumeMode, Delegate, ExprUseVisitor, PlaceBase, PlaceWithHirId};
3736
use std::iter::{once, Iterator};
3837
use std::mem;
3938
use utils::make_iterator_snippet;
@@ -865,7 +864,7 @@ fn check_for_loop<'tcx>(
865864
}
866865
for_loop_arg::check_for_loop_arg(cx, pat, arg, expr);
867866
for_loop_over_map_kv::check_for_loop_over_map_kv(cx, pat, arg, body, expr);
868-
check_for_mut_range_bound(cx, arg, body);
867+
for_mut_range_bound::check_for_mut_range_bound(cx, arg, body);
869868
check_for_single_element_loop(cx, pat, arg, body, expr);
870869
detect_same_item_push(cx, pat, arg, body, expr);
871870
manual_flatten::check_manual_flatten(cx, pat, arg, body, span);
@@ -1769,112 +1768,6 @@ fn check_for_single_element_loop<'tcx>(
17691768
}
17701769
}
17711770

1772-
struct MutatePairDelegate<'a, 'tcx> {
1773-
cx: &'a LateContext<'tcx>,
1774-
hir_id_low: Option<HirId>,
1775-
hir_id_high: Option<HirId>,
1776-
span_low: Option<Span>,
1777-
span_high: Option<Span>,
1778-
}
1779-
1780-
impl<'tcx> Delegate<'tcx> for MutatePairDelegate<'_, 'tcx> {
1781-
fn consume(&mut self, _: &PlaceWithHirId<'tcx>, _: HirId, _: ConsumeMode) {}
1782-
1783-
fn borrow(&mut self, cmt: &PlaceWithHirId<'tcx>, diag_expr_id: HirId, bk: ty::BorrowKind) {
1784-
if let ty::BorrowKind::MutBorrow = bk {
1785-
if let PlaceBase::Local(id) = cmt.place.base {
1786-
if Some(id) == self.hir_id_low {
1787-
self.span_low = Some(self.cx.tcx.hir().span(diag_expr_id))
1788-
}
1789-
if Some(id) == self.hir_id_high {
1790-
self.span_high = Some(self.cx.tcx.hir().span(diag_expr_id))
1791-
}
1792-
}
1793-
}
1794-
}
1795-
1796-
fn mutate(&mut self, cmt: &PlaceWithHirId<'tcx>, diag_expr_id: HirId) {
1797-
if let PlaceBase::Local(id) = cmt.place.base {
1798-
if Some(id) == self.hir_id_low {
1799-
self.span_low = Some(self.cx.tcx.hir().span(diag_expr_id))
1800-
}
1801-
if Some(id) == self.hir_id_high {
1802-
self.span_high = Some(self.cx.tcx.hir().span(diag_expr_id))
1803-
}
1804-
}
1805-
}
1806-
}
1807-
1808-
impl MutatePairDelegate<'_, '_> {
1809-
fn mutation_span(&self) -> (Option<Span>, Option<Span>) {
1810-
(self.span_low, self.span_high)
1811-
}
1812-
}
1813-
1814-
fn check_for_mut_range_bound(cx: &LateContext<'_>, arg: &Expr<'_>, body: &Expr<'_>) {
1815-
if let Some(higher::Range {
1816-
start: Some(start),
1817-
end: Some(end),
1818-
..
1819-
}) = higher::range(arg)
1820-
{
1821-
let mut_ids = vec![check_for_mutability(cx, start), check_for_mutability(cx, end)];
1822-
if mut_ids[0].is_some() || mut_ids[1].is_some() {
1823-
let (span_low, span_high) = check_for_mutation(cx, body, &mut_ids);
1824-
mut_warn_with_span(cx, span_low);
1825-
mut_warn_with_span(cx, span_high);
1826-
}
1827-
}
1828-
}
1829-
1830-
fn mut_warn_with_span(cx: &LateContext<'_>, span: Option<Span>) {
1831-
if let Some(sp) = span {
1832-
span_lint(
1833-
cx,
1834-
MUT_RANGE_BOUND,
1835-
sp,
1836-
"attempt to mutate range bound within loop; note that the range of the loop is unchanged",
1837-
);
1838-
}
1839-
}
1840-
1841-
fn check_for_mutability(cx: &LateContext<'_>, bound: &Expr<'_>) -> Option<HirId> {
1842-
if_chain! {
1843-
if let Some(hir_id) = path_to_local(bound);
1844-
if let Node::Binding(pat) = cx.tcx.hir().get(hir_id);
1845-
if let PatKind::Binding(BindingAnnotation::Mutable, ..) = pat.kind;
1846-
then {
1847-
return Some(hir_id);
1848-
}
1849-
}
1850-
None
1851-
}
1852-
1853-
fn check_for_mutation<'tcx>(
1854-
cx: &LateContext<'tcx>,
1855-
body: &Expr<'_>,
1856-
bound_ids: &[Option<HirId>],
1857-
) -> (Option<Span>, Option<Span>) {
1858-
let mut delegate = MutatePairDelegate {
1859-
cx,
1860-
hir_id_low: bound_ids[0],
1861-
hir_id_high: bound_ids[1],
1862-
span_low: None,
1863-
span_high: None,
1864-
};
1865-
cx.tcx.infer_ctxt().enter(|infcx| {
1866-
ExprUseVisitor::new(
1867-
&mut delegate,
1868-
&infcx,
1869-
body.hir_id.owner,
1870-
cx.param_env,
1871-
cx.typeck_results(),
1872-
)
1873-
.walk_expr(body);
1874-
});
1875-
delegate.mutation_span()
1876-
}
1877-
18781771
struct VarVisitor<'a, 'tcx> {
18791772
/// context reference
18801773
cx: &'a LateContext<'tcx>,

0 commit comments

Comments
 (0)