Skip to content

Commit 23aa2f8

Browse files
JarchoJarcho
committed
Fix dogfood errors
1 parent efe33f9 commit 23aa2f8

File tree

4 files changed

+60
-30
lines changed

4 files changed

+60
-30
lines changed

‎clippy_lints/src/manual_map.rs

+26-14
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,14 @@
1-
use crate::utils::{
2-
is_type_diagnostic_item, match_def_path, paths, peel_hir_expr_refs, peel_mid_ty_refs_is_mutable,
3-
snippet_with_applicability, span_lint_and_sugg,
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+
},
48
};
59
use rustc_ast::util::parser::PREC_POSTFIX;
610
use rustc_errors::Applicability;
7-
use rustc_hir::{Arm, BindingAnnotation, Block, Expr, ExprKind, Mutability, Pat, PatKind, Path, QPath};
11+
use rustc_hir::{Arm, BindingAnnotation, Block, Expr, ExprKind, Mutability, Pat, PatKind, QPath};
812
use rustc_lint::{LateContext, LateLintPass, LintContext};
913
use rustc_middle::lint::in_external_macro;
1014
use rustc_session::{declare_lint_pass, declare_tool_lint};
@@ -37,6 +41,7 @@ declare_clippy_lint! {
3741
declare_lint_pass!(ManualMap => [MANUAL_MAP]);
3842

3943
impl LateLintPass<'_> for ManualMap {
44+
#[allow(clippy::too_many_lines)]
4045
fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
4146
if in_external_macro(cx.sess(), expr.span) {
4247
return;
@@ -88,14 +93,17 @@ impl LateLintPass<'_> for ManualMap {
8893
None => return,
8994
};
9095

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+
91102
// Determine which binding mode to use.
92103
let explicit_ref = some_pat.contains_explicit_ref_binding();
93-
let binding_mutability = explicit_ref.or(if ty_ref_count != pat_ref_count {
94-
Some(ty_mutability)
95-
} else {
96-
None
97-
});
98-
let as_ref_str = match binding_mutability {
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 {
99107
Some(Mutability::Mut) => ".as_mut()",
100108
Some(Mutability::Not) => ".as_ref()",
101109
None => "",
@@ -118,6 +126,13 @@ impl LateLintPass<'_> for ManualMap {
118126
if let Some(func) = can_pass_as_func(cx, some_binding, some_expr) {
119127
snippet_with_applicability(cx, func.span, "..", &mut app).into_owned()
120128
} 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+
121136
// `ref` and `ref mut` annotations were handled earlier.
122137
let annotation = if matches!(annotation, BindingAnnotation::Mutable) {
123138
"mut "
@@ -161,10 +176,7 @@ impl LateLintPass<'_> for ManualMap {
161176
fn can_pass_as_func(cx: &LateContext<'tcx>, binding: Ident, expr: &'tcx Expr<'_>) -> Option<&'tcx Expr<'tcx>> {
162177
match expr.kind {
163178
ExprKind::Call(func, [arg])
164-
if matches!(arg.kind,
165-
ExprKind::Path(QPath::Resolved(None, Path { segments: [path], ..}))
166-
if path.ident == binding
167-
) && cx.typeck_results().expr_adjustments(arg).is_empty() =>
179+
if match_var(arg, binding.name) && cx.typeck_results().expr_adjustments(arg).is_empty() =>
168180
{
169181
Some(func)
170182
},

‎tests/ui/manual_map_option.fixed

+11-2
Original file line numberDiff line numberDiff line change
@@ -32,9 +32,18 @@ fn main() {
3232

3333
Some(0).map(|x| x + x);
3434

35-
Some(String::new()).as_mut().map(|x| x.push_str(""));
35+
#[warn(clippy::option_map_unit_fn)]
36+
match &mut Some(String::new()) {
37+
Some(x) => Some(x.push_str("")),
38+
None => None,
39+
};
3640

37-
Some(String::new()).as_ref().map(|x| &**x);
41+
#[allow(clippy::option_map_unit_fn)]
42+
{
43+
Some(String::new()).as_mut().map(|x| x.push_str(""));
44+
}
45+
46+
Some(String::new()).as_ref().map(|x| x.len());
3847

3948
Some(String::new()).as_ref().map(|x| x.is_empty());
4049

‎tests/ui/manual_map_option.rs

+10-1
Original file line numberDiff line numberDiff line change
@@ -66,13 +66,22 @@ fn main() {
6666
&&_ => None,
6767
};
6868

69+
#[warn(clippy::option_map_unit_fn)]
6970
match &mut Some(String::new()) {
7071
Some(x) => Some(x.push_str("")),
7172
None => None,
7273
};
7374

75+
#[allow(clippy::option_map_unit_fn)]
76+
{
77+
match &mut Some(String::new()) {
78+
Some(x) => Some(x.push_str("")),
79+
None => None,
80+
};
81+
}
82+
7483
match &mut Some(String::new()) {
75-
Some(ref x) => Some(&**x),
84+
Some(ref x) => Some(x.len()),
7685
None => None,
7786
};
7887

‎tests/ui/manual_map_option.stderr

+13-13
Original file line numberDiff line numberDiff line change
@@ -101,25 +101,25 @@ LL | | };
101101
| |_____^ help: try this: `Some(0).map(|x| x + x)`
102102

103103
error: manual implementation of `Option::map`
104-
--> $DIR/manual_map_option.rs:69:5
104+
--> $DIR/manual_map_option.rs:77:9
105105
|
106-
LL | / match &mut Some(String::new()) {
107-
LL | | Some(x) => Some(x.push_str("")),
108-
LL | | None => None,
109-
LL | | };
110-
| |_____^ help: try this: `Some(String::new()).as_mut().map(|x| x.push_str(""))`
106+
LL | / match &mut Some(String::new()) {
107+
LL | | Some(x) => Some(x.push_str("")),
108+
LL | | None => None,
109+
LL | | };
110+
| |_________^ help: try this: `Some(String::new()).as_mut().map(|x| x.push_str(""))`
111111

112112
error: manual implementation of `Option::map`
113-
--> $DIR/manual_map_option.rs:74:5
113+
--> $DIR/manual_map_option.rs:83:5
114114
|
115115
LL | / match &mut Some(String::new()) {
116-
LL | | Some(ref x) => Some(&**x),
116+
LL | | Some(ref x) => Some(x.len()),
117117
LL | | None => None,
118118
LL | | };
119-
| |_____^ help: try this: `Some(String::new()).as_ref().map(|x| &**x)`
119+
| |_____^ help: try this: `Some(String::new()).as_ref().map(|x| x.len())`
120120

121121
error: manual implementation of `Option::map`
122-
--> $DIR/manual_map_option.rs:79:5
122+
--> $DIR/manual_map_option.rs:88:5
123123
|
124124
LL | / match &mut &Some(String::new()) {
125125
LL | | Some(x) => Some(x.is_empty()),
@@ -128,7 +128,7 @@ LL | | };
128128
| |_____^ help: try this: `Some(String::new()).as_ref().map(|x| x.is_empty())`
129129

130130
error: manual implementation of `Option::map`
131-
--> $DIR/manual_map_option.rs:84:5
131+
--> $DIR/manual_map_option.rs:93:5
132132
|
133133
LL | / match Some((0, 1, 2)) {
134134
LL | | Some((x, y, z)) => Some(x + y + z),
@@ -137,7 +137,7 @@ LL | | };
137137
| |_____^ help: try this: `Some((0, 1, 2)).map(|(x, y, z)| x + y + z)`
138138

139139
error: manual implementation of `Option::map`
140-
--> $DIR/manual_map_option.rs:89:5
140+
--> $DIR/manual_map_option.rs:98:5
141141
|
142142
LL | / match Some([1, 2, 3]) {
143143
LL | | Some([first, ..]) => Some(first),
@@ -146,7 +146,7 @@ LL | | };
146146
| |_____^ help: try this: `Some([1, 2, 3]).map(|[first, ..]| first)`
147147

148148
error: manual implementation of `Option::map`
149-
--> $DIR/manual_map_option.rs:94:5
149+
--> $DIR/manual_map_option.rs:103:5
150150
|
151151
LL | / match &Some((String::new(), "test")) {
152152
LL | | Some((x, y)) => Some((y, x)),

0 commit comments

Comments
 (0)