Skip to content

Commit a2c25fa

Browse files
committed
Auto merge of rust-lang#6573 - Jarcho:option_match_map, r=llogiq
New lint: option_manual_map fixes: rust-lang#6 changelog: Added lint: `match_map`
2 parents a5c5c8f + 23aa2f8 commit a2c25fa

File tree

7 files changed

+656
-1
lines changed

7 files changed

+656
-1
lines changed

CHANGELOG.md

+1
Original file line numberDiff line numberDiff line change
@@ -2162,6 +2162,7 @@ Released 2018-09-13
21622162
[`manual_filter_map`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_filter_map
21632163
[`manual_find_map`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_find_map
21642164
[`manual_flatten`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_flatten
2165+
[`manual_map`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_map
21652166
[`manual_memcpy`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_memcpy
21662167
[`manual_non_exhaustive`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_non_exhaustive
21672168
[`manual_ok_or`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_ok_or

clippy_lints/src/lib.rs

+5
Original file line numberDiff line numberDiff line change
@@ -242,6 +242,7 @@ mod loops;
242242
mod macro_use;
243243
mod main_recursion;
244244
mod manual_async_fn;
245+
mod manual_map;
245246
mod manual_non_exhaustive;
246247
mod manual_ok_or;
247248
mod manual_strip;
@@ -708,6 +709,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
708709
&macro_use::MACRO_USE_IMPORTS,
709710
&main_recursion::MAIN_RECURSION,
710711
&manual_async_fn::MANUAL_ASYNC_FN,
712+
&manual_map::MANUAL_MAP,
711713
&manual_non_exhaustive::MANUAL_NON_EXHAUSTIVE,
712714
&manual_ok_or::MANUAL_OK_OR,
713715
&manual_strip::MANUAL_STRIP,
@@ -1265,6 +1267,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
12651267
store.register_late_pass(|| box case_sensitive_file_extension_comparisons::CaseSensitiveFileExtensionComparisons);
12661268
store.register_late_pass(|| box redundant_slicing::RedundantSlicing);
12671269
store.register_late_pass(|| box from_str_radix_10::FromStrRadix10);
1270+
store.register_late_pass(|| box manual_map::ManualMap);
12681271

12691272
store.register_group(true, "clippy::restriction", Some("clippy_restriction"), vec![
12701273
LintId::of(&arithmetic::FLOAT_ARITHMETIC),
@@ -1526,6 +1529,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
15261529
LintId::of(&loops::WHILE_LET_ON_ITERATOR),
15271530
LintId::of(&main_recursion::MAIN_RECURSION),
15281531
LintId::of(&manual_async_fn::MANUAL_ASYNC_FN),
1532+
LintId::of(&manual_map::MANUAL_MAP),
15291533
LintId::of(&manual_non_exhaustive::MANUAL_NON_EXHAUSTIVE),
15301534
LintId::of(&manual_strip::MANUAL_STRIP),
15311535
LintId::of(&manual_unwrap_or::MANUAL_UNWRAP_OR),
@@ -1755,6 +1759,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
17551759
LintId::of(&loops::WHILE_LET_ON_ITERATOR),
17561760
LintId::of(&main_recursion::MAIN_RECURSION),
17571761
LintId::of(&manual_async_fn::MANUAL_ASYNC_FN),
1762+
LintId::of(&manual_map::MANUAL_MAP),
17581763
LintId::of(&manual_non_exhaustive::MANUAL_NON_EXHAUSTIVE),
17591764
LintId::of(&map_clone::MAP_CLONE),
17601765
LintId::of(&matches::INFALLIBLE_DESTRUCTURING_MATCH),

clippy_lints/src/manual_map.rs

+274
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,274 @@
1+
use crate::{
2+
map_unit_fn::OPTION_MAP_UNIT_FN,
3+
matches::MATCH_AS_REF,
4+
utils::{
5+
is_allowed, is_type_diagnostic_item, match_def_path, match_var, paths, peel_hir_expr_refs,
6+
peel_mid_ty_refs_is_mutable, snippet_with_applicability, span_lint_and_sugg,
7+
},
8+
};
9+
use rustc_ast::util::parser::PREC_POSTFIX;
10+
use rustc_errors::Applicability;
11+
use rustc_hir::{Arm, BindingAnnotation, Block, Expr, ExprKind, Mutability, Pat, PatKind, QPath};
12+
use rustc_lint::{LateContext, LateLintPass, LintContext};
13+
use rustc_middle::lint::in_external_macro;
14+
use rustc_session::{declare_lint_pass, declare_tool_lint};
15+
use rustc_span::symbol::{sym, Ident};
16+
17+
declare_clippy_lint! {
18+
/// **What it does:** Checks for usages of `match` which could be implemented using `map`
19+
///
20+
/// **Why is this bad?** Using the `map` method is clearer and more concise.
21+
///
22+
/// **Known problems:** None.
23+
///
24+
/// **Example:**
25+
///
26+
/// ```rust
27+
/// match Some(0) {
28+
/// Some(x) => Some(x + 1),
29+
/// None => None,
30+
/// };
31+
/// ```
32+
/// Use instead:
33+
/// ```rust
34+
/// Some(0).map(|x| x + 1);
35+
/// ```
36+
pub MANUAL_MAP,
37+
style,
38+
"reimplementation of `map`"
39+
}
40+
41+
declare_lint_pass!(ManualMap => [MANUAL_MAP]);
42+
43+
impl LateLintPass<'_> for ManualMap {
44+
#[allow(clippy::too_many_lines)]
45+
fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
46+
if in_external_macro(cx.sess(), expr.span) {
47+
return;
48+
}
49+
50+
if let ExprKind::Match(scrutinee, [arm1 @ Arm { guard: None, .. }, arm2 @ Arm { guard: None, .. }], _) =
51+
expr.kind
52+
{
53+
let (scrutinee_ty, ty_ref_count, ty_mutability) =
54+
peel_mid_ty_refs_is_mutable(cx.typeck_results().expr_ty(scrutinee));
55+
if !is_type_diagnostic_item(cx, scrutinee_ty, sym::option_type)
56+
|| !is_type_diagnostic_item(cx, cx.typeck_results().expr_ty(expr), sym::option_type)
57+
{
58+
return;
59+
}
60+
61+
let (some_expr, some_pat, pat_ref_count, is_wild_none) =
62+
match (try_parse_pattern(cx, arm1.pat), try_parse_pattern(cx, arm2.pat)) {
63+
(Some(OptionPat::Wild), Some(OptionPat::Some { pattern, ref_count }))
64+
if is_none_expr(cx, arm1.body) =>
65+
{
66+
(arm2.body, pattern, ref_count, true)
67+
},
68+
(Some(OptionPat::None), Some(OptionPat::Some { pattern, ref_count }))
69+
if is_none_expr(cx, arm1.body) =>
70+
{
71+
(arm2.body, pattern, ref_count, false)
72+
},
73+
(Some(OptionPat::Some { pattern, ref_count }), Some(OptionPat::Wild))
74+
if is_none_expr(cx, arm2.body) =>
75+
{
76+
(arm1.body, pattern, ref_count, true)
77+
},
78+
(Some(OptionPat::Some { pattern, ref_count }), Some(OptionPat::None))
79+
if is_none_expr(cx, arm2.body) =>
80+
{
81+
(arm1.body, pattern, ref_count, false)
82+
},
83+
_ => return,
84+
};
85+
86+
// Top level or patterns aren't allowed in closures.
87+
if matches!(some_pat.kind, PatKind::Or(_)) {
88+
return;
89+
}
90+
91+
let some_expr = match get_some_expr(cx, some_expr) {
92+
Some(expr) => expr,
93+
None => return,
94+
};
95+
96+
if cx.typeck_results().expr_ty(some_expr) == cx.tcx.types.unit
97+
&& !is_allowed(cx, OPTION_MAP_UNIT_FN, expr.hir_id)
98+
{
99+
return;
100+
}
101+
102+
// Determine which binding mode to use.
103+
let explicit_ref = some_pat.contains_explicit_ref_binding();
104+
let binding_ref = explicit_ref.or_else(|| (ty_ref_count != pat_ref_count).then(|| ty_mutability));
105+
106+
let as_ref_str = match binding_ref {
107+
Some(Mutability::Mut) => ".as_mut()",
108+
Some(Mutability::Not) => ".as_ref()",
109+
None => "",
110+
};
111+
112+
let mut app = Applicability::MachineApplicable;
113+
114+
// Remove address-of expressions from the scrutinee. `as_ref` will be called,
115+
// the type is copyable, or the option is being passed by value.
116+
let scrutinee = peel_hir_expr_refs(scrutinee).0;
117+
let scrutinee_str = snippet_with_applicability(cx, scrutinee.span, "_", &mut app);
118+
let scrutinee_str = if expr.precedence().order() < PREC_POSTFIX {
119+
// Parens are needed to chain method calls.
120+
format!("({})", scrutinee_str)
121+
} else {
122+
scrutinee_str.into()
123+
};
124+
125+
let body_str = if let PatKind::Binding(annotation, _, some_binding, None) = some_pat.kind {
126+
if let Some(func) = can_pass_as_func(cx, some_binding, some_expr) {
127+
snippet_with_applicability(cx, func.span, "..", &mut app).into_owned()
128+
} else {
129+
if match_var(some_expr, some_binding.name)
130+
&& !is_allowed(cx, MATCH_AS_REF, expr.hir_id)
131+
&& binding_ref.is_some()
132+
{
133+
return;
134+
}
135+
136+
// `ref` and `ref mut` annotations were handled earlier.
137+
let annotation = if matches!(annotation, BindingAnnotation::Mutable) {
138+
"mut "
139+
} else {
140+
""
141+
};
142+
format!(
143+
"|{}{}| {}",
144+
annotation,
145+
some_binding,
146+
snippet_with_applicability(cx, some_expr.span, "..", &mut app)
147+
)
148+
}
149+
} else if !is_wild_none && explicit_ref.is_none() {
150+
// TODO: handle explicit reference annotations.
151+
format!(
152+
"|{}| {}",
153+
snippet_with_applicability(cx, some_pat.span, "..", &mut app),
154+
snippet_with_applicability(cx, some_expr.span, "..", &mut app)
155+
)
156+
} else {
157+
// Refutable bindings and mixed reference annotations can't be handled by `map`.
158+
return;
159+
};
160+
161+
span_lint_and_sugg(
162+
cx,
163+
MANUAL_MAP,
164+
expr.span,
165+
"manual implementation of `Option::map`",
166+
"try this",
167+
format!("{}{}.map({})", scrutinee_str, as_ref_str, body_str),
168+
app,
169+
);
170+
}
171+
}
172+
}
173+
174+
// Checks whether the expression could be passed as a function, or whether a closure is needed.
175+
// Returns the function to be passed to `map` if it exists.
176+
fn can_pass_as_func(cx: &LateContext<'tcx>, binding: Ident, expr: &'tcx Expr<'_>) -> Option<&'tcx Expr<'tcx>> {
177+
match expr.kind {
178+
ExprKind::Call(func, [arg])
179+
if match_var(arg, binding.name) && cx.typeck_results().expr_adjustments(arg).is_empty() =>
180+
{
181+
Some(func)
182+
},
183+
_ => None,
184+
}
185+
}
186+
187+
enum OptionPat<'a> {
188+
Wild,
189+
None,
190+
Some {
191+
// The pattern contained in the `Some` tuple.
192+
pattern: &'a Pat<'a>,
193+
// The number of references before the `Some` tuple.
194+
// e.g. `&&Some(_)` has a ref count of 2.
195+
ref_count: usize,
196+
},
197+
}
198+
199+
// Try to parse into a recognized `Option` pattern.
200+
// i.e. `_`, `None`, `Some(..)`, or a reference to any of those.
201+
fn try_parse_pattern(cx: &LateContext<'tcx>, pat: &'tcx Pat<'_>) -> Option<OptionPat<'tcx>> {
202+
fn f(cx: &LateContext<'tcx>, pat: &'tcx Pat<'_>, ref_count: usize) -> Option<OptionPat<'tcx>> {
203+
match pat.kind {
204+
PatKind::Wild => Some(OptionPat::Wild),
205+
PatKind::Ref(pat, _) => f(cx, pat, ref_count + 1),
206+
PatKind::Path(QPath::Resolved(None, path))
207+
if path
208+
.res
209+
.opt_def_id()
210+
.map_or(false, |id| match_def_path(cx, id, &paths::OPTION_NONE)) =>
211+
{
212+
Some(OptionPat::None)
213+
},
214+
PatKind::TupleStruct(QPath::Resolved(None, path), [pattern], _)
215+
if path
216+
.res
217+
.opt_def_id()
218+
.map_or(false, |id| match_def_path(cx, id, &paths::OPTION_SOME)) =>
219+
{
220+
Some(OptionPat::Some { pattern, ref_count })
221+
},
222+
_ => None,
223+
}
224+
}
225+
f(cx, pat, 0)
226+
}
227+
228+
// Checks for an expression wrapped by the `Some` constructor. Returns the contained expression.
229+
fn get_some_expr(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) -> Option<&'tcx Expr<'tcx>> {
230+
// TODO: Allow more complex expressions.
231+
match expr.kind {
232+
ExprKind::Call(
233+
Expr {
234+
kind: ExprKind::Path(QPath::Resolved(None, path)),
235+
..
236+
},
237+
[arg],
238+
) => {
239+
if match_def_path(cx, path.res.opt_def_id()?, &paths::OPTION_SOME) {
240+
Some(arg)
241+
} else {
242+
None
243+
}
244+
},
245+
ExprKind::Block(
246+
Block {
247+
stmts: [],
248+
expr: Some(expr),
249+
..
250+
},
251+
_,
252+
) => get_some_expr(cx, expr),
253+
_ => None,
254+
}
255+
}
256+
257+
// Checks for the `None` value.
258+
fn is_none_expr(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) -> bool {
259+
match expr.kind {
260+
ExprKind::Path(QPath::Resolved(None, path)) => path
261+
.res
262+
.opt_def_id()
263+
.map_or(false, |id| match_def_path(cx, id, &paths::OPTION_NONE)),
264+
ExprKind::Block(
265+
Block {
266+
stmts: [],
267+
expr: Some(expr),
268+
..
269+
},
270+
_,
271+
) => is_none_expr(cx, expr),
272+
_ => false,
273+
}
274+
}

clippy_lints/src/utils/mod.rs

+26-1
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ use std::collections::hash_map::Entry;
3232
use std::hash::BuildHasherDefault;
3333

3434
use if_chain::if_chain;
35-
use rustc_ast::ast::{self, Attribute, LitKind};
35+
use rustc_ast::ast::{self, Attribute, BorrowKind, LitKind, Mutability};
3636
use rustc_data_structures::fx::FxHashMap;
3737
use rustc_errors::Applicability;
3838
use rustc_hir as hir;
@@ -1672,6 +1672,18 @@ pub fn peel_n_hir_expr_refs(expr: &'a Expr<'a>, count: usize) -> (&'a Expr<'a>,
16721672
f(expr, 0, count)
16731673
}
16741674

1675+
/// Peels off all references on the expression. Returns the underlying expression and the number of
1676+
/// references removed.
1677+
pub fn peel_hir_expr_refs(expr: &'a Expr<'a>) -> (&'a Expr<'a>, usize) {
1678+
fn f(expr: &'a Expr<'a>, count: usize) -> (&'a Expr<'a>, usize) {
1679+
match expr.kind {
1680+
ExprKind::AddrOf(BorrowKind::Ref, _, expr) => f(expr, count + 1),
1681+
_ => (expr, count),
1682+
}
1683+
}
1684+
f(expr, 0)
1685+
}
1686+
16751687
/// Peels off all references on the type. Returns the underlying type and the number of references
16761688
/// removed.
16771689
pub fn peel_mid_ty_refs(ty: Ty<'_>) -> (Ty<'_>, usize) {
@@ -1685,6 +1697,19 @@ pub fn peel_mid_ty_refs(ty: Ty<'_>) -> (Ty<'_>, usize) {
16851697
peel(ty, 0)
16861698
}
16871699

1700+
/// Peels off all references on the type.Returns the underlying type, the number of references
1701+
/// removed, and whether the pointer is ultimately mutable or not.
1702+
pub fn peel_mid_ty_refs_is_mutable(ty: Ty<'_>) -> (Ty<'_>, usize, Mutability) {
1703+
fn f(ty: Ty<'_>, count: usize, mutability: Mutability) -> (Ty<'_>, usize, Mutability) {
1704+
match ty.kind() {
1705+
ty::Ref(_, ty, Mutability::Mut) => f(ty, count + 1, mutability),
1706+
ty::Ref(_, ty, Mutability::Not) => f(ty, count + 1, Mutability::Not),
1707+
_ => (ty, count, mutability),
1708+
}
1709+
}
1710+
f(ty, 0, Mutability::Mut)
1711+
}
1712+
16881713
#[macro_export]
16891714
macro_rules! unwrap_cargo_metadata {
16901715
($cx: ident, $lint: ident, $deps: expr) => {{

0 commit comments

Comments
 (0)