Skip to content

Commit 7ff1a29

Browse files
detrumidetrumi
committed
Fix false negative
1 parent 25d4c90 commit 7ff1a29

File tree

2 files changed

+57
-46
lines changed

2 files changed

+57
-46
lines changed

‎clippy_lints/src/methods/option_map_unwrap_or.rs

Lines changed: 15 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -2,9 +2,9 @@ use crate::utils::paths;
22
use crate::utils::{is_copy, match_type, snippet, span_lint, span_note_and_lint};
33
use rustc::hir::intravisit::{walk_path, NestedVisitorMap, Visitor};
44
use rustc::hir::{self, *};
5-
use rustc::hir::def_id::DefId;
65
use rustc::lint::LateContext;
76
use rustc_data_structures::fx::FxHashSet;
7+
use syntax::symbol::Symbol;
88

99
use super::OPTION_MAP_UNWRAP_OR;
1010

@@ -17,7 +17,6 @@ pub(super) fn lint<'a, 'tcx>(
1717
) {
1818
// lint if the caller of `map()` is an `Option`
1919
if match_type(cx, cx.tables.expr_ty(&map_args[0]), &paths::OPTION) {
20-
2120
if !is_copy(cx, cx.tables.expr_ty(&unwrap_args[1])) {
2221
// Do not lint if the `map` argument uses identifiers in the `map`
2322
// argument that are also used in the `unwrap_or` argument
@@ -80,14 +79,12 @@ pub(super) fn lint<'a, 'tcx>(
8079

8180
struct UnwrapVisitor<'a, 'tcx: 'a> {
8281
cx: &'a LateContext<'a, 'tcx>,
83-
identifiers: FxHashSet<DefId>,
82+
identifiers: FxHashSet<Symbol>,
8483
}
8584

8685
impl<'a, 'tcx: 'a> Visitor<'tcx> for UnwrapVisitor<'a, 'tcx> {
8786
fn visit_path(&mut self, path: &'tcx Path, _id: HirId) {
88-
if let Some(def_id) = path.def.opt_def_id() {
89-
self.identifiers.insert(def_id);
90-
}
87+
self.identifiers.insert(ident(path));
9188
walk_path(self, path);
9289
}
9390

@@ -98,17 +95,15 @@ impl<'a, 'tcx: 'a> Visitor<'tcx> for UnwrapVisitor<'a, 'tcx> {
9895

9996
struct MapExprVisitor<'a, 'tcx: 'a> {
10097
cx: &'a LateContext<'a, 'tcx>,
101-
identifiers: FxHashSet<DefId>,
98+
identifiers: FxHashSet<Symbol>,
10299
found_identifier: bool,
103100
}
104101

105102
impl<'a, 'tcx: 'a> Visitor<'tcx> for MapExprVisitor<'a, 'tcx> {
106103
fn visit_path(&mut self, path: &'tcx Path, _id: HirId) {
107-
if let Some(def_id) = path.def.opt_def_id() {
108-
if self.identifiers.contains(&def_id) {
109-
self.found_identifier = true;
110-
return;
111-
}
104+
if self.identifiers.contains(&ident(path)) {
105+
self.found_identifier = true;
106+
return;
112107
}
113108
walk_path(self, path);
114109
}
@@ -117,3 +112,11 @@ impl<'a, 'tcx: 'a> Visitor<'tcx> for MapExprVisitor<'a, 'tcx> {
117112
NestedVisitorMap::All(&self.cx.tcx.hir())
118113
}
119114
}
115+
116+
fn ident(path: &Path) -> Symbol {
117+
path.segments
118+
.last()
119+
.expect("segments should be composed of at least 1 element")
120+
.ident
121+
.name
122+
}

‎tests/ui/methods.stderr

Lines changed: 42 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -87,9 +87,17 @@ LL | | .unwrap_or(None);
8787
|
8888
= note: replace `map(|x| Some(x + 1)).unwrap_or(None)` with `and_then(|x| Some(x + 1))`
8989

90-
error: called `map(f).unwrap_or_else(g)` on an Option value. This can be done more directly by calling `map_or_else(g, f)` instead
90+
error: called `map(f).unwrap_or(a)` on an Option value. This can be done more directly by calling `map_or(a, f)` instead
9191
--> $DIR/methods.rs:141:13
9292
|
93+
LL | let _ = Some("prefix").map(|p| format!("{}.", p)).unwrap_or(id);
94+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
95+
|
96+
= note: replace `map(|p| format!("{}.", p)).unwrap_or(id)` with `map_or(id, |p| format!("{}.", p))`
97+
98+
error: called `map(f).unwrap_or_else(g)` on an Option value. This can be done more directly by calling `map_or_else(g, f)` instead
99+
--> $DIR/methods.rs:145:13
100+
|
93101
LL | let _ = opt.map(|x| x + 1)
94102
| _____________^
95103
LL | |
@@ -100,7 +108,7 @@ LL | | .unwrap_or_else(|| 0); // should lint even though this cal
100108
= note: replace `map(|x| x + 1).unwrap_or_else(|| 0)` with `map_or_else(|| 0, |x| x + 1)`
101109

102110
error: called `map(f).unwrap_or_else(g)` on an Option value. This can be done more directly by calling `map_or_else(g, f)` instead
103-
--> $DIR/methods.rs:145:13
111+
--> $DIR/methods.rs:149:13
104112
|
105113
LL | let _ = opt.map(|x| {
106114
| _____________^
@@ -110,7 +118,7 @@ LL | | ).unwrap_or_else(|| 0);
110118
| |____________________________________^
111119

112120
error: called `map(f).unwrap_or_else(g)` on an Option value. This can be done more directly by calling `map_or_else(g, f)` instead
113-
--> $DIR/methods.rs:149:13
121+
--> $DIR/methods.rs:153:13
114122
|
115123
LL | let _ = opt.map(|x| x + 1)
116124
| _____________^
@@ -120,15 +128,15 @@ LL | | );
120128
| |_________________^
121129

122130
error: called `map_or(None, f)` on an Option value. This can be done more directly by calling `and_then(f)` instead
123-
--> $DIR/methods.rs:158:13
131+
--> $DIR/methods.rs:162:13
124132
|
125133
LL | let _ = opt.map_or(None, |x| Some(x + 1));
126134
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try using and_then instead: `opt.and_then(|x| Some(x + 1))`
127135
|
128136
= note: `-D clippy::option-map-or-none` implied by `-D warnings`
129137

130138
error: called `map_or(None, f)` on an Option value. This can be done more directly by calling `and_then(f)` instead
131-
--> $DIR/methods.rs:160:13
139+
--> $DIR/methods.rs:164:13
132140
|
133141
LL | let _ = opt.map_or(None, |x| {
134142
| _____________^
@@ -144,7 +152,7 @@ LL | });
144152
|
145153

146154
error: called `filter(p).next()` on an `Iterator`. This is more succinctly expressed by calling `.find(p)` instead.
147-
--> $DIR/methods.rs:185:13
155+
--> $DIR/methods.rs:189:13
148156
|
149157
LL | let _ = v.iter().filter(|&x| *x < 0).next();
150158
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
@@ -153,7 +161,7 @@ LL | let _ = v.iter().filter(|&x| *x < 0).next();
153161
= note: replace `filter(|&x| *x < 0).next()` with `find(|&x| *x < 0)`
154162

155163
error: called `filter(p).next()` on an `Iterator`. This is more succinctly expressed by calling `.find(p)` instead.
156-
--> $DIR/methods.rs:188:13
164+
--> $DIR/methods.rs:192:13
157165
|
158166
LL | let _ = v.iter().filter(|&x| {
159167
| _____________^
@@ -163,7 +171,7 @@ LL | | ).next();
163171
| |___________________________^
164172

165173
error: called `is_some()` after searching an `Iterator` with find. This is more succinctly expressed by calling `any()`.
166-
--> $DIR/methods.rs:203:13
174+
--> $DIR/methods.rs:207:13
167175
|
168176
LL | let _ = v.iter().find(|&x| *x < 0).is_some();
169177
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
@@ -172,7 +180,7 @@ LL | let _ = v.iter().find(|&x| *x < 0).is_some();
172180
= note: replace `find(|&x| *x < 0).is_some()` with `any(|&x| *x < 0)`
173181

174182
error: called `is_some()` after searching an `Iterator` with find. This is more succinctly expressed by calling `any()`.
175-
--> $DIR/methods.rs:206:13
183+
--> $DIR/methods.rs:210:13
176184
|
177185
LL | let _ = v.iter().find(|&x| {
178186
| _____________^
@@ -182,15 +190,15 @@ LL | | ).is_some();
182190
| |______________________________^
183191

184192
error: called `is_some()` after searching an `Iterator` with position. This is more succinctly expressed by calling `any()`.
185-
--> $DIR/methods.rs:212:13
193+
--> $DIR/methods.rs:216:13
186194
|
187195
LL | let _ = v.iter().position(|&x| x < 0).is_some();
188196
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
189197
|
190198
= note: replace `position(|&x| x < 0).is_some()` with `any(|&x| x < 0)`
191199

192200
error: called `is_some()` after searching an `Iterator` with position. This is more succinctly expressed by calling `any()`.
193-
--> $DIR/methods.rs:215:13
201+
--> $DIR/methods.rs:219:13
194202
|
195203
LL | let _ = v.iter().position(|&x| {
196204
| _____________^
@@ -200,15 +208,15 @@ LL | | ).is_some();
200208
| |______________________________^
201209

202210
error: called `is_some()` after searching an `Iterator` with rposition. This is more succinctly expressed by calling `any()`.
203-
--> $DIR/methods.rs:221:13
211+
--> $DIR/methods.rs:225:13
204212
|
205213
LL | let _ = v.iter().rposition(|&x| x < 0).is_some();
206214
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
207215
|
208216
= note: replace `rposition(|&x| x < 0).is_some()` with `any(|&x| x < 0)`
209217

210218
error: called `is_some()` after searching an `Iterator` with rposition. This is more succinctly expressed by calling `any()`.
211-
--> $DIR/methods.rs:224:13
219+
--> $DIR/methods.rs:228:13
212220
|
213221
LL | let _ = v.iter().rposition(|&x| {
214222
| _____________^
@@ -218,130 +226,130 @@ LL | | ).is_some();
218226
| |______________________________^
219227

220228
error: use of `unwrap_or` followed by a function call
221-
--> $DIR/methods.rs:259:22
229+
--> $DIR/methods.rs:263:22
222230
|
223231
LL | with_constructor.unwrap_or(make());
224232
| ^^^^^^^^^^^^^^^^^ help: try this: `unwrap_or_else(make)`
225233
|
226234
= note: `-D clippy::or-fun-call` implied by `-D warnings`
227235

228236
error: use of `unwrap_or` followed by a call to `new`
229-
--> $DIR/methods.rs:262:5
237+
--> $DIR/methods.rs:266:5
230238
|
231239
LL | with_new.unwrap_or(Vec::new());
232240
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `with_new.unwrap_or_default()`
233241

234242
error: use of `unwrap_or` followed by a function call
235-
--> $DIR/methods.rs:265:21
243+
--> $DIR/methods.rs:269:21
236244
|
237245
LL | with_const_args.unwrap_or(Vec::with_capacity(12));
238246
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `unwrap_or_else(|| Vec::with_capacity(12))`
239247

240248
error: use of `unwrap_or` followed by a function call
241-
--> $DIR/methods.rs:268:14
249+
--> $DIR/methods.rs:272:14
242250
|
243251
LL | with_err.unwrap_or(make());
244252
| ^^^^^^^^^^^^^^^^^ help: try this: `unwrap_or_else(|_| make())`
245253

246254
error: use of `unwrap_or` followed by a function call
247-
--> $DIR/methods.rs:271:19
255+
--> $DIR/methods.rs:275:19
248256
|
249257
LL | with_err_args.unwrap_or(Vec::with_capacity(12));
250258
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `unwrap_or_else(|_| Vec::with_capacity(12))`
251259

252260
error: use of `unwrap_or` followed by a call to `default`
253-
--> $DIR/methods.rs:274:5
261+
--> $DIR/methods.rs:278:5
254262
|
255263
LL | with_default_trait.unwrap_or(Default::default());
256264
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `with_default_trait.unwrap_or_default()`
257265

258266
error: use of `unwrap_or` followed by a call to `default`
259-
--> $DIR/methods.rs:277:5
267+
--> $DIR/methods.rs:281:5
260268
|
261269
LL | with_default_type.unwrap_or(u64::default());
262270
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `with_default_type.unwrap_or_default()`
263271

264272
error: use of `unwrap_or` followed by a function call
265-
--> $DIR/methods.rs:280:14
273+
--> $DIR/methods.rs:284:14
266274
|
267275
LL | with_vec.unwrap_or(vec![]);
268276
| ^^^^^^^^^^^^^^^^^ help: try this: `unwrap_or_else(|| vec![])`
269277

270278
error: use of `unwrap_or` followed by a function call
271-
--> $DIR/methods.rs:285:21
279+
--> $DIR/methods.rs:289:21
272280
|
273281
LL | without_default.unwrap_or(Foo::new());
274282
| ^^^^^^^^^^^^^^^^^^^^^ help: try this: `unwrap_or_else(Foo::new)`
275283

276284
error: use of `or_insert` followed by a function call
277-
--> $DIR/methods.rs:288:19
285+
--> $DIR/methods.rs:292:19
278286
|
279287
LL | map.entry(42).or_insert(String::new());
280288
| ^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `or_insert_with(String::new)`
281289

282290
error: use of `or_insert` followed by a function call
283-
--> $DIR/methods.rs:291:21
291+
--> $DIR/methods.rs:295:21
284292
|
285293
LL | btree.entry(42).or_insert(String::new());
286294
| ^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `or_insert_with(String::new)`
287295

288296
error: use of `unwrap_or` followed by a function call
289-
--> $DIR/methods.rs:294:21
297+
--> $DIR/methods.rs:298:21
290298
|
291299
LL | let _ = stringy.unwrap_or("".to_owned());
292300
| ^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `unwrap_or_else(|| "".to_owned())`
293301

294302
error: called `.iter().nth()` on a Vec. Calling `.get()` is both faster and more readable
295-
--> $DIR/methods.rs:305:23
303+
--> $DIR/methods.rs:309:23
296304
|
297305
LL | let bad_vec = some_vec.iter().nth(3);
298306
| ^^^^^^^^^^^^^^^^^^^^^^
299307
|
300308
= note: `-D clippy::iter-nth` implied by `-D warnings`
301309

302310
error: called `.iter().nth()` on a slice. Calling `.get()` is both faster and more readable
303-
--> $DIR/methods.rs:306:26
311+
--> $DIR/methods.rs:310:26
304312
|
305313
LL | let bad_slice = &some_vec[..].iter().nth(3);
306314
| ^^^^^^^^^^^^^^^^^^^^^^^^^^
307315

308316
error: called `.iter().nth()` on a slice. Calling `.get()` is both faster and more readable
309-
--> $DIR/methods.rs:307:31
317+
--> $DIR/methods.rs:311:31
310318
|
311319
LL | let bad_boxed_slice = boxed_slice.iter().nth(3);
312320
| ^^^^^^^^^^^^^^^^^^^^^^^^^
313321

314322
error: called `.iter().nth()` on a VecDeque. Calling `.get()` is both faster and more readable
315-
--> $DIR/methods.rs:308:29
323+
--> $DIR/methods.rs:312:29
316324
|
317325
LL | let bad_vec_deque = some_vec_deque.iter().nth(3);
318326
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
319327

320328
error: called `.iter_mut().nth()` on a Vec. Calling `.get_mut()` is both faster and more readable
321-
--> $DIR/methods.rs:313:23
329+
--> $DIR/methods.rs:317:23
322330
|
323331
LL | let bad_vec = some_vec.iter_mut().nth(3);
324332
| ^^^^^^^^^^^^^^^^^^^^^^^^^^
325333

326334
error: called `.iter_mut().nth()` on a slice. Calling `.get_mut()` is both faster and more readable
327-
--> $DIR/methods.rs:316:26
335+
--> $DIR/methods.rs:320:26
328336
|
329337
LL | let bad_slice = &some_vec[..].iter_mut().nth(3);
330338
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
331339

332340
error: called `.iter_mut().nth()` on a VecDeque. Calling `.get_mut()` is both faster and more readable
333-
--> $DIR/methods.rs:319:29
341+
--> $DIR/methods.rs:323:29
334342
|
335343
LL | let bad_vec_deque = some_vec_deque.iter_mut().nth(3);
336344
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
337345

338346
error: used unwrap() on an Option value. If you don't want to handle the None case gracefully, consider using expect() to provide a better panic message
339-
--> $DIR/methods.rs:331:13
347+
--> $DIR/methods.rs:335:13
340348
|
341349
LL | let _ = opt.unwrap();
342350
| ^^^^^^^^^^^^
343351
|
344352
= note: `-D clippy::option-unwrap-used` implied by `-D warnings`
345353

346-
error: aborting due to 43 previous errors
354+
error: aborting due to 44 previous errors
347355

0 commit comments

Comments
 (0)