Skip to content

Commit eb70a72

Browse files
detrumidetrumi
committed
Fix false negative
1 parent fec6e55 commit eb70a72

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

+15-12
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

+42-34
Original file line numberDiff line numberDiff line change
@@ -89,9 +89,17 @@ LL | | .unwrap_or(None);
8989
|
9090
= note: replace `map(|x| Some(x + 1)).unwrap_or(None)` with `and_then(|x| Some(x + 1))`
9191

92-
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
92+
error: called `map(f).unwrap_or(a)` on an Option value. This can be done more directly by calling `map_or(a, f)` instead
9393
--> $DIR/methods.rs:187:13
9494
|
95+
LL | let _ = Some("prefix").map(|p| format!("{}.", p)).unwrap_or(id);
96+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
97+
|
98+
= note: replace `map(|p| format!("{}.", p)).unwrap_or(id)` with `map_or(id, |p| format!("{}.", p))`
99+
100+
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
101+
--> $DIR/methods.rs:191:13
102+
|
95103
LL | let _ = opt.map(|x| x + 1)
96104
| _____________^
97105
LL | |
@@ -102,7 +110,7 @@ LL | | .unwrap_or_else(|| 0); // should lint even though this cal
102110
= note: replace `map(|x| x + 1).unwrap_or_else(|| 0)` with `map_or_else(|| 0, |x| x + 1)`
103111

104112
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
105-
--> $DIR/methods.rs:191:13
113+
--> $DIR/methods.rs:195:13
106114
|
107115
LL | let _ = opt.map(|x| {
108116
| _____________^
@@ -112,7 +120,7 @@ LL | | ).unwrap_or_else(|| 0);
112120
| |____________________________________^
113121

114122
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
115-
--> $DIR/methods.rs:195:13
123+
--> $DIR/methods.rs:199:13
116124
|
117125
LL | let _ = opt.map(|x| x + 1)
118126
| _____________^
@@ -122,15 +130,15 @@ LL | | );
122130
| |_________________^
123131

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

132140
error: called `map_or(None, f)` on an Option value. This can be done more directly by calling `and_then(f)` instead
133-
--> $DIR/methods.rs:206:13
141+
--> $DIR/methods.rs:210:13
134142
|
135143
LL | let _ = opt.map_or(None, |x| {
136144
| _____________^
@@ -146,7 +154,7 @@ LL | });
146154
|
147155

148156
error: called `filter(p).next()` on an `Iterator`. This is more succinctly expressed by calling `.find(p)` instead.
149-
--> $DIR/methods.rs:232:13
157+
--> $DIR/methods.rs:236:13
150158
|
151159
LL | let _ = v.iter().filter(|&x| *x < 0).next();
152160
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
@@ -155,7 +163,7 @@ LL | let _ = v.iter().filter(|&x| *x < 0).next();
155163
= note: replace `filter(|&x| *x < 0).next()` with `find(|&x| *x < 0)`
156164

157165
error: called `filter(p).next()` on an `Iterator`. This is more succinctly expressed by calling `.find(p)` instead.
158-
--> $DIR/methods.rs:235:13
166+
--> $DIR/methods.rs:239:13
159167
|
160168
LL | let _ = v.iter().filter(|&x| {
161169
| _____________^
@@ -165,7 +173,7 @@ LL | | ).next();
165173
| |___________________________^
166174

167175
error: called `is_some()` after searching an `Iterator` with find. This is more succinctly expressed by calling `any()`.
168-
--> $DIR/methods.rs:251:13
176+
--> $DIR/methods.rs:255:13
169177
|
170178
LL | let _ = v.iter().find(|&x| *x < 0).is_some();
171179
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
@@ -174,7 +182,7 @@ LL | let _ = v.iter().find(|&x| *x < 0).is_some();
174182
= note: replace `find(|&x| *x < 0).is_some()` with `any(|&x| *x < 0)`
175183

176184
error: called `is_some()` after searching an `Iterator` with find. This is more succinctly expressed by calling `any()`.
177-
--> $DIR/methods.rs:254:13
185+
--> $DIR/methods.rs:258:13
178186
|
179187
LL | let _ = v.iter().find(|&x| {
180188
| _____________^
@@ -184,15 +192,15 @@ LL | | ).is_some();
184192
| |______________________________^
185193

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

194202
error: called `is_some()` after searching an `Iterator` with position. This is more succinctly expressed by calling `any()`.
195-
--> $DIR/methods.rs:263:13
203+
--> $DIR/methods.rs:267:13
196204
|
197205
LL | let _ = v.iter().position(|&x| {
198206
| _____________^
@@ -202,15 +210,15 @@ LL | | ).is_some();
202210
| |______________________________^
203211

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

212220
error: called `is_some()` after searching an `Iterator` with rposition. This is more succinctly expressed by calling `any()`.
213-
--> $DIR/methods.rs:272:13
221+
--> $DIR/methods.rs:276:13
214222
|
215223
LL | let _ = v.iter().rposition(|&x| {
216224
| _____________^
@@ -220,130 +228,130 @@ LL | | ).is_some();
220228
| |______________________________^
221229

222230
error: use of `unwrap_or` followed by a function call
223-
--> $DIR/methods.rs:309:22
231+
--> $DIR/methods.rs:313:22
224232
|
225233
LL | with_constructor.unwrap_or(make());
226234
| ^^^^^^^^^^^^^^^^^ help: try this: `unwrap_or_else(make)`
227235
|
228236
= note: `-D clippy::or-fun-call` implied by `-D warnings`
229237

230238
error: use of `unwrap_or` followed by a call to `new`
231-
--> $DIR/methods.rs:312:5
239+
--> $DIR/methods.rs:316:5
232240
|
233241
LL | with_new.unwrap_or(Vec::new());
234242
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `with_new.unwrap_or_default()`
235243

236244
error: use of `unwrap_or` followed by a function call
237-
--> $DIR/methods.rs:315:21
245+
--> $DIR/methods.rs:319:21
238246
|
239247
LL | with_const_args.unwrap_or(Vec::with_capacity(12));
240248
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `unwrap_or_else(|| Vec::with_capacity(12))`
241249

242250
error: use of `unwrap_or` followed by a function call
243-
--> $DIR/methods.rs:318:14
251+
--> $DIR/methods.rs:322:14
244252
|
245253
LL | with_err.unwrap_or(make());
246254
| ^^^^^^^^^^^^^^^^^ help: try this: `unwrap_or_else(|_| make())`
247255

248256
error: use of `unwrap_or` followed by a function call
249-
--> $DIR/methods.rs:321:19
257+
--> $DIR/methods.rs:325:19
250258
|
251259
LL | with_err_args.unwrap_or(Vec::with_capacity(12));
252260
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `unwrap_or_else(|_| Vec::with_capacity(12))`
253261

254262
error: use of `unwrap_or` followed by a call to `default`
255-
--> $DIR/methods.rs:324:5
263+
--> $DIR/methods.rs:328:5
256264
|
257265
LL | with_default_trait.unwrap_or(Default::default());
258266
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `with_default_trait.unwrap_or_default()`
259267

260268
error: use of `unwrap_or` followed by a call to `default`
261-
--> $DIR/methods.rs:327:5
269+
--> $DIR/methods.rs:331:5
262270
|
263271
LL | with_default_type.unwrap_or(u64::default());
264272
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `with_default_type.unwrap_or_default()`
265273

266274
error: use of `unwrap_or` followed by a function call
267-
--> $DIR/methods.rs:330:14
275+
--> $DIR/methods.rs:334:14
268276
|
269277
LL | with_vec.unwrap_or(vec![]);
270278
| ^^^^^^^^^^^^^^^^^ help: try this: `unwrap_or_else(|| vec![])`
271279

272280
error: use of `unwrap_or` followed by a function call
273-
--> $DIR/methods.rs:335:21
281+
--> $DIR/methods.rs:339:21
274282
|
275283
LL | without_default.unwrap_or(Foo::new());
276284
| ^^^^^^^^^^^^^^^^^^^^^ help: try this: `unwrap_or_else(Foo::new)`
277285

278286
error: use of `or_insert` followed by a function call
279-
--> $DIR/methods.rs:338:19
287+
--> $DIR/methods.rs:342:19
280288
|
281289
LL | map.entry(42).or_insert(String::new());
282290
| ^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `or_insert_with(String::new)`
283291

284292
error: use of `or_insert` followed by a function call
285-
--> $DIR/methods.rs:341:21
293+
--> $DIR/methods.rs:345:21
286294
|
287295
LL | btree.entry(42).or_insert(String::new());
288296
| ^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `or_insert_with(String::new)`
289297

290298
error: use of `unwrap_or` followed by a function call
291-
--> $DIR/methods.rs:344:21
299+
--> $DIR/methods.rs:348:21
292300
|
293301
LL | let _ = stringy.unwrap_or("".to_owned());
294302
| ^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `unwrap_or_else(|| "".to_owned())`
295303

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

304312
error: called `.iter().nth()` on a slice. Calling `.get()` is both faster and more readable
305-
--> $DIR/methods.rs:356:26
313+
--> $DIR/methods.rs:360:26
306314
|
307315
LL | let bad_slice = &some_vec[..].iter().nth(3);
308316
| ^^^^^^^^^^^^^^^^^^^^^^^^^^
309317

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

316324
error: called `.iter().nth()` on a VecDeque. Calling `.get()` is both faster and more readable
317-
--> $DIR/methods.rs:358:29
325+
--> $DIR/methods.rs:362:29
318326
|
319327
LL | let bad_vec_deque = some_vec_deque.iter().nth(3);
320328
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
321329

322330
error: called `.iter_mut().nth()` on a Vec. Calling `.get_mut()` is both faster and more readable
323-
--> $DIR/methods.rs:363:23
331+
--> $DIR/methods.rs:367:23
324332
|
325333
LL | let bad_vec = some_vec.iter_mut().nth(3);
326334
| ^^^^^^^^^^^^^^^^^^^^^^^^^^
327335

328336
error: called `.iter_mut().nth()` on a slice. Calling `.get_mut()` is both faster and more readable
329-
--> $DIR/methods.rs:366:26
337+
--> $DIR/methods.rs:370:26
330338
|
331339
LL | let bad_slice = &some_vec[..].iter_mut().nth(3);
332340
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
333341

334342
error: called `.iter_mut().nth()` on a VecDeque. Calling `.get_mut()` is both faster and more readable
335-
--> $DIR/methods.rs:369:29
343+
--> $DIR/methods.rs:373:29
336344
|
337345
LL | let bad_vec_deque = some_vec_deque.iter_mut().nth(3);
338346
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
339347

340348
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
341-
--> $DIR/methods.rs:381:13
349+
--> $DIR/methods.rs:385:13
342350
|
343351
LL | let _ = opt.unwrap();
344352
| ^^^^^^^^^^^^
345353
|
346354
= note: `-D clippy::option-unwrap-used` implied by `-D warnings`
347355

348-
error: aborting due to 43 previous errors
356+
error: aborting due to 44 previous errors
349357

0 commit comments

Comments
 (0)