From a7ac441760ae034ff7401439b38da821f4e2df3a Mon Sep 17 00:00:00 2001 From: Hirochika Matsumoto Date: Sun, 20 Sep 2020 02:03:14 +0900 Subject: [PATCH 01/29] Add new lint to detect unnecessarily wrapped value --- CHANGELOG.md | 1 + clippy_lints/src/lib.rs | 5 + clippy_lints/src/unnecessary_wrap.rs | 177 +++++++++++++++++++++++++++ src/lintlist/mod.rs | 7 ++ tests/ui/unnecessary_wrap.fixed | 47 +++++++ tests/ui/unnecessary_wrap.rs | 47 +++++++ tests/ui/unnecessary_wrap.stderr | 34 +++++ 7 files changed, 318 insertions(+) create mode 100644 clippy_lints/src/unnecessary_wrap.rs create mode 100644 tests/ui/unnecessary_wrap.fixed create mode 100644 tests/ui/unnecessary_wrap.rs create mode 100644 tests/ui/unnecessary_wrap.stderr diff --git a/CHANGELOG.md b/CHANGELOG.md index 816d25bcd93e..02b862d31965 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2008,6 +2008,7 @@ Released 2018-09-13 [`unnecessary_operation`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_operation [`unnecessary_sort_by`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_sort_by [`unnecessary_unwrap`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_unwrap +[`unnecessary_wrap`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_wrap [`unneeded_field_pattern`]: https://rust-lang.github.io/rust-clippy/master/index.html#unneeded_field_pattern [`unneeded_wildcard_pattern`]: https://rust-lang.github.io/rust-clippy/master/index.html#unneeded_wildcard_pattern [`unnested_or_patterns`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnested_or_patterns diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 20b38cbb6d0d..2d1f75391bb5 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -323,6 +323,7 @@ mod unicode; mod unit_return_expecting_ord; mod unnamed_address; mod unnecessary_sort_by; +mod unnecessary_wrap; mod unnested_or_patterns; mod unsafe_removed_from_name; mod unused_io_amount; @@ -892,6 +893,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: &unnamed_address::FN_ADDRESS_COMPARISONS, &unnamed_address::VTABLE_ADDRESS_COMPARISONS, &unnecessary_sort_by::UNNECESSARY_SORT_BY, + &unnecessary_wrap::UNNECESSARY_WRAP, &unnested_or_patterns::UNNESTED_OR_PATTERNS, &unsafe_removed_from_name::UNSAFE_REMOVED_FROM_NAME, &unused_io_amount::UNUSED_IO_AMOUNT, @@ -1064,6 +1066,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: store.register_late_pass(|| box redundant_clone::RedundantClone); store.register_late_pass(|| box slow_vector_initialization::SlowVectorInit); store.register_late_pass(|| box unnecessary_sort_by::UnnecessarySortBy); + store.register_late_pass(|| box unnecessary_wrap::UnnecessaryWrap); store.register_late_pass(|| box types::RefToMut); store.register_late_pass(|| box assertions_on_constants::AssertionsOnConstants); store.register_late_pass(|| box missing_const_for_fn::MissingConstForFn); @@ -1571,6 +1574,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: LintId::of(&unnamed_address::FN_ADDRESS_COMPARISONS), LintId::of(&unnamed_address::VTABLE_ADDRESS_COMPARISONS), LintId::of(&unnecessary_sort_by::UNNECESSARY_SORT_BY), + LintId::of(&unnecessary_wrap::UNNECESSARY_WRAP), LintId::of(&unsafe_removed_from_name::UNSAFE_REMOVED_FROM_NAME), LintId::of(&unused_io_amount::UNUSED_IO_AMOUNT), LintId::of(&unused_unit::UNUSED_UNIT), @@ -1775,6 +1779,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: LintId::of(&types::UNNECESSARY_CAST), LintId::of(&types::VEC_BOX), LintId::of(&unnecessary_sort_by::UNNECESSARY_SORT_BY), + LintId::of(&unnecessary_wrap::UNNECESSARY_WRAP), LintId::of(&unwrap::UNNECESSARY_UNWRAP), LintId::of(&useless_conversion::USELESS_CONVERSION), LintId::of(&zero_div_zero::ZERO_DIVIDED_BY_ZERO), diff --git a/clippy_lints/src/unnecessary_wrap.rs b/clippy_lints/src/unnecessary_wrap.rs new file mode 100644 index 000000000000..26a575172584 --- /dev/null +++ b/clippy_lints/src/unnecessary_wrap.rs @@ -0,0 +1,177 @@ +use crate::utils::{ + is_type_diagnostic_item, match_qpath, multispan_sugg_with_applicability, paths, return_ty, snippet, + span_lint_and_then, +}; +use if_chain::if_chain; +use rustc_errors::Applicability; +use rustc_hir::intravisit::{FnKind, NestedVisitorMap, Visitor}; +use rustc_hir::*; +use rustc_lint::{LateContext, LateLintPass}; +use rustc_middle::hir::map::Map; +use rustc_session::{declare_lint_pass, declare_tool_lint}; +use rustc_span::Span; + +declare_clippy_lint! { + /// **What it does:** Checks for private functions that only return `Ok` or `Some`. + /// + /// **Why is this bad?** It is not meaningful to wrap values when no `None` or `Err` is returned. + /// + /// **Known problems:** Since this lint changes function type signature, you may need to + /// adjust some codes at callee side. + /// + /// **Example:** + /// + /// ```rust + /// pub fn get_cool_number(a: bool, b: bool) -> Option { + /// if a && b { + /// return Some(50); + /// } + /// if a { + /// Some(0) + /// } else { + /// Some(10) + /// } + /// } + /// ``` + /// Use instead: + /// ```rust + /// pub fn get_cool_number(a: bool, b: bool) -> i32 { + /// if a && b { + /// return 50; + /// } + /// if a { + /// 0 + /// } else { + /// 10 + /// } + /// } + /// ``` + pub UNNECESSARY_WRAP, + complexity, + "functions that only return `Ok` or `Some`" +} + +declare_lint_pass!(UnnecessaryWrap => [UNNECESSARY_WRAP]); + +impl<'tcx> LateLintPass<'tcx> for UnnecessaryWrap { + fn check_fn( + &mut self, + cx: &LateContext<'tcx>, + fn_kind: FnKind<'tcx>, + fn_decl: &FnDecl<'tcx>, + body: &Body<'tcx>, + span: Span, + hir_id: HirId, + ) { + if_chain! { + if let FnKind::ItemFn(.., visibility, _) = fn_kind; + if visibility.node.is_pub(); + then { + return; + } + } + + if let ExprKind::Block(ref block, ..) = body.value.kind { + let path = if is_type_diagnostic_item(cx, return_ty(cx, hir_id), sym!(option_type)) { + &paths::OPTION_SOME + } else if is_type_diagnostic_item(cx, return_ty(cx, hir_id), sym!(result_type)) { + &paths::RESULT_OK + } else { + return; + }; + + let mut visitor = UnnecessaryWrapVisitor { result: Vec::new() }; + visitor.visit_block(block); + let result = visitor.result; + + if result.iter().any(|expr| { + if_chain! { + if let ExprKind::Call(ref func, ref args) = expr.kind; + if let ExprKind::Path(ref qpath) = func.kind; + if match_qpath(qpath, path); + if args.len() == 1; + then { + false + } else { + true + } + } + }) { + return; + } + + let suggs = result.iter().filter_map(|expr| { + let snippet = if let ExprKind::Call(_, ref args) = expr.kind { + Some(snippet(cx, args[0].span, "..").to_string()) + } else { + None + }; + snippet.map(|snip| (expr.span, snip)) + }); + + span_lint_and_then( + cx, + UNNECESSARY_WRAP, + span, + "this function returns unnecessarily wrapping data", + move |diag| { + multispan_sugg_with_applicability( + diag, + "factor this out to", + Applicability::MachineApplicable, + suggs, + ); + }, + ); + } + } +} + +struct UnnecessaryWrapVisitor<'tcx> { + result: Vec<&'tcx Expr<'tcx>>, +} + +impl<'tcx> Visitor<'tcx> for UnnecessaryWrapVisitor<'tcx> { + type Map = Map<'tcx>; + + fn visit_block(&mut self, block: &'tcx Block<'tcx>) { + for stmt in block.stmts { + self.visit_stmt(stmt); + } + if let Some(expr) = block.expr { + self.visit_expr(expr) + } + } + + fn visit_stmt(&mut self, stmt: &'tcx Stmt<'tcx>) { + match stmt.kind { + StmtKind::Semi(ref expr) => { + if let ExprKind::Ret(Some(value)) = expr.kind { + self.result.push(value); + } + }, + StmtKind::Expr(ref expr) => self.visit_expr(expr), + _ => (), + } + } + + fn visit_expr(&mut self, expr: &'tcx Expr<'tcx>) { + match expr.kind { + ExprKind::Ret(Some(value)) => self.result.push(value), + ExprKind::Call(..) | ExprKind::Path(..) => self.result.push(expr), + ExprKind::Block(ref block, _) | ExprKind::Loop(ref block, ..) => { + self.visit_block(block); + }, + ExprKind::Match(_, arms, _) => { + for arm in arms { + self.visit_expr(arm.body); + } + }, + _ => intravisit::walk_expr(self, expr), + } + } + + fn nested_visit_map(&mut self) -> NestedVisitorMap { + NestedVisitorMap::None + } +} diff --git a/src/lintlist/mod.rs b/src/lintlist/mod.rs index 69acd3d9b8bc..4a0cdc5d82f9 100644 --- a/src/lintlist/mod.rs +++ b/src/lintlist/mod.rs @@ -2608,6 +2608,13 @@ vec![ deprecation: None, module: "unwrap", }, + Lint { + name: "unnecessary_wrap", + group: "complexity", + desc: "functions that only return `Ok` or `Some`", + deprecation: None, + module: "unnecessary_wrap", + }, Lint { name: "unneeded_field_pattern", group: "restriction", diff --git a/tests/ui/unnecessary_wrap.fixed b/tests/ui/unnecessary_wrap.fixed new file mode 100644 index 000000000000..1657a3173dbf --- /dev/null +++ b/tests/ui/unnecessary_wrap.fixed @@ -0,0 +1,47 @@ +// run-rustfix +#![warn(clippy::unnecessary_wrap)] +#![allow(clippy::no_effect)] +#![allow(clippy::needless_return)] +#![allow(clippy::if_same_then_else)] + +// should be linted +fn func1(a: bool, b: bool) -> Option { + if a && b { + return Some(42); + } + if a { + Some(-1); + Some(2) + } else { + return Some(1337); + } +} + +// public fns should not be linted +pub fn func2(a: bool) -> Option { + if a { + Some(1) + } else { + Some(1) + } +} + +// should not be linted +fn func3(a: bool) -> Option { + if a { + Some(1) + } else { + None + } +} + +// should be linted +fn func4() -> Option { + 1 +} + +fn main() { + // method calls are not linted + func1(true, true); + func2(true); +} diff --git a/tests/ui/unnecessary_wrap.rs b/tests/ui/unnecessary_wrap.rs new file mode 100644 index 000000000000..edf41dad7905 --- /dev/null +++ b/tests/ui/unnecessary_wrap.rs @@ -0,0 +1,47 @@ +// run-rustfix +#![warn(clippy::unnecessary_wrap)] +#![allow(clippy::no_effect)] +#![allow(clippy::needless_return)] +#![allow(clippy::if_same_then_else)] + +// should be linted +fn func1(a: bool, b: bool) -> Option { + if a && b { + return Some(42); + } + if a { + Some(-1); + Some(2) + } else { + return Some(1337); + } +} + +// public fns should not be linted +pub fn func2(a: bool) -> Option { + if a { + Some(1) + } else { + Some(1) + } +} + +// should not be linted +fn func3(a: bool) -> Option { + if a { + Some(1) + } else { + None + } +} + +// should be linted +fn func4() -> Option { + Some(1) +} + +fn main() { + // method calls are not linted + func1(true, true); + func2(true); +} diff --git a/tests/ui/unnecessary_wrap.stderr b/tests/ui/unnecessary_wrap.stderr new file mode 100644 index 000000000000..8473bd818396 --- /dev/null +++ b/tests/ui/unnecessary_wrap.stderr @@ -0,0 +1,34 @@ +error: this function unnecessarily wrapping data + --> $DIR/unnecessary_wrap.rs:8:1 + | +LL | / fn func1(a: bool, b: bool) -> Option { +LL | | if a && b { +LL | | return Some(42); +LL | | } +... | +LL | | } +LL | | } + | |_^ + | + = note: `-D clippy::unnecessary-wrap` implied by `-D warnings` +help: factor this out to + | +LL | return 42; +LL | } +LL | if a { +LL | Some(-1); +LL | 2 +LL | } else { + ... + +error: this function unnecessarily wrapping data + --> $DIR/unnecessary_wrap.rs:39:1 + | +LL | / fn func4() -> Option { +LL | | Some(1) + | | ------- help: factor this out to: `1` +LL | | } + | |_^ + +error: aborting due to 2 previous errors + From 750c118b347af938383c5bff53040480e0974071 Mon Sep 17 00:00:00 2001 From: Hirochika Matsumoto Date: Sun, 20 Sep 2020 18:22:01 +0900 Subject: [PATCH 02/29] Add suggestion on type signatures --- clippy_lints/src/unnecessary_wrap.rs | 31 +++++++++++++++++------- tests/ui/unnecessary_wrap.fixed | 18 +++++++++++++- tests/ui/unnecessary_wrap.rs | 16 +++++++++++++ tests/ui/unnecessary_wrap.stderr | 35 +++++++++++++++++++++------- 4 files changed, 82 insertions(+), 18 deletions(-) diff --git a/clippy_lints/src/unnecessary_wrap.rs b/clippy_lints/src/unnecessary_wrap.rs index 26a575172584..7b586c1df0c2 100644 --- a/clippy_lints/src/unnecessary_wrap.rs +++ b/clippy_lints/src/unnecessary_wrap.rs @@ -7,7 +7,7 @@ use rustc_errors::Applicability; use rustc_hir::intravisit::{FnKind, NestedVisitorMap, Visitor}; use rustc_hir::*; use rustc_lint::{LateContext, LateLintPass}; -use rustc_middle::hir::map::Map; +use rustc_middle::{hir::map::Map, ty::subst::GenericArgKind}; use rustc_session::{declare_lint_pass, declare_tool_lint}; use rustc_span::Span; @@ -100,14 +100,27 @@ impl<'tcx> LateLintPass<'tcx> for UnnecessaryWrap { return; } - let suggs = result.iter().filter_map(|expr| { - let snippet = if let ExprKind::Call(_, ref args) = expr.kind { - Some(snippet(cx, args[0].span, "..").to_string()) - } else { - None - }; - snippet.map(|snip| (expr.span, snip)) - }); + let suggs = result + .iter() + .filter_map(|expr| { + let snippet = if let ExprKind::Call(_, ref args) = expr.kind { + Some(snippet(cx, args[0].span, "..").to_string()) + } else { + None + }; + snippet.map(|snip| (expr.span, snip)) + }) + .chain({ + let inner_ty = return_ty(cx, hir_id) + .walk() + .skip(1) // skip `std::option::Option` or `std::result::Result` + .take(1) // first outermost inner type is needed + .filter_map(|inner| match inner.unpack() { + GenericArgKind::Type(inner_ty) => Some(inner_ty.to_string()), + _ => None, + }); + inner_ty.map(|inner_ty| (fn_decl.output.span(), inner_ty)) + }); span_lint_and_then( cx, diff --git a/tests/ui/unnecessary_wrap.fixed b/tests/ui/unnecessary_wrap.fixed index 1657a3173dbf..749bb95c4178 100644 --- a/tests/ui/unnecessary_wrap.fixed +++ b/tests/ui/unnecessary_wrap.fixed @@ -1,8 +1,10 @@ // run-rustfix + #![warn(clippy::unnecessary_wrap)] #![allow(clippy::no_effect)] #![allow(clippy::needless_return)] #![allow(clippy::if_same_then_else)] +#![allow(dead_code)] // should be linted fn func1(a: bool, b: bool) -> Option { @@ -37,7 +39,21 @@ fn func3(a: bool) -> Option { // should be linted fn func4() -> Option { - 1 + Some(1) +} + +// should be linted +fn func5() -> Result { + Ok(1) +} + +// should not be linted +fn func6(a: bool) -> Result { + if a { + Ok(1) + } else { + Err(()) + } } fn main() { diff --git a/tests/ui/unnecessary_wrap.rs b/tests/ui/unnecessary_wrap.rs index edf41dad7905..749bb95c4178 100644 --- a/tests/ui/unnecessary_wrap.rs +++ b/tests/ui/unnecessary_wrap.rs @@ -1,8 +1,10 @@ // run-rustfix + #![warn(clippy::unnecessary_wrap)] #![allow(clippy::no_effect)] #![allow(clippy::needless_return)] #![allow(clippy::if_same_then_else)] +#![allow(dead_code)] // should be linted fn func1(a: bool, b: bool) -> Option { @@ -40,6 +42,20 @@ fn func4() -> Option { Some(1) } +// should be linted +fn func5() -> Result { + Ok(1) +} + +// should not be linted +fn func6(a: bool) -> Result { + if a { + Ok(1) + } else { + Err(()) + } +} + fn main() { // method calls are not linted func1(true, true); diff --git a/tests/ui/unnecessary_wrap.stderr b/tests/ui/unnecessary_wrap.stderr index 8473bd818396..511d085c82fe 100644 --- a/tests/ui/unnecessary_wrap.stderr +++ b/tests/ui/unnecessary_wrap.stderr @@ -1,5 +1,5 @@ -error: this function unnecessarily wrapping data - --> $DIR/unnecessary_wrap.rs:8:1 +error: this function returns unnecessarily wrapping data + --> $DIR/unnecessary_wrap.rs:10:1 | LL | / fn func1(a: bool, b: bool) -> Option { LL | | if a && b { @@ -13,22 +13,41 @@ LL | | } = note: `-D clippy::unnecessary-wrap` implied by `-D warnings` help: factor this out to | +LL | fn func1(a: bool, b: bool) -> i32 { +LL | if a && b { LL | return 42; LL | } LL | if a { LL | Some(-1); -LL | 2 -LL | } else { ... -error: this function unnecessarily wrapping data - --> $DIR/unnecessary_wrap.rs:39:1 +error: this function returns unnecessarily wrapping data + --> $DIR/unnecessary_wrap.rs:41:1 | LL | / fn func4() -> Option { LL | | Some(1) - | | ------- help: factor this out to: `1` LL | | } | |_^ + | +help: factor this out to + | +LL | fn func4() -> i32 { +LL | 1 + | + +error: this function returns unnecessarily wrapping data + --> $DIR/unnecessary_wrap.rs:46:1 + | +LL | / fn func5() -> Result { +LL | | Ok(1) +LL | | } + | |_^ + | +help: factor this out to + | +LL | fn func5() -> i32 { +LL | 1 + | -error: aborting due to 2 previous errors +error: aborting due to 3 previous errors From 0335b8d6a747672c0a4ff6cb14b12eced3848325 Mon Sep 17 00:00:00 2001 From: Hirochika Matsumoto Date: Mon, 21 Sep 2020 00:06:25 +0900 Subject: [PATCH 03/29] Fix lint example --- clippy_lints/src/unnecessary_wrap.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/clippy_lints/src/unnecessary_wrap.rs b/clippy_lints/src/unnecessary_wrap.rs index 7b586c1df0c2..c5d0d5100796 100644 --- a/clippy_lints/src/unnecessary_wrap.rs +++ b/clippy_lints/src/unnecessary_wrap.rs @@ -22,7 +22,7 @@ declare_clippy_lint! { /// **Example:** /// /// ```rust - /// pub fn get_cool_number(a: bool, b: bool) -> Option { + /// fn get_cool_number(a: bool, b: bool) -> Option { /// if a && b { /// return Some(50); /// } @@ -35,7 +35,7 @@ declare_clippy_lint! { /// ``` /// Use instead: /// ```rust - /// pub fn get_cool_number(a: bool, b: bool) -> i32 { + /// fn get_cool_number(a: bool, b: bool) -> i32 { /// if a && b { /// return 50; /// } From 0e9d227c043c1b990912508662e2e5158383ea54 Mon Sep 17 00:00:00 2001 From: Hirochika Matsumoto Date: Mon, 21 Sep 2020 00:11:28 +0900 Subject: [PATCH 04/29] Add test cases --- tests/ui/unnecessary_wrap.fixed | 14 ++++++++++++-- tests/ui/unnecessary_wrap.rs | 14 ++++++++++++-- tests/ui/unnecessary_wrap.stderr | 6 +++--- 3 files changed, 27 insertions(+), 7 deletions(-) diff --git a/tests/ui/unnecessary_wrap.fixed b/tests/ui/unnecessary_wrap.fixed index 749bb95c4178..e7e3120f82f3 100644 --- a/tests/ui/unnecessary_wrap.fixed +++ b/tests/ui/unnecessary_wrap.fixed @@ -42,13 +42,18 @@ fn func4() -> Option { Some(1) } +// should not be linted +fn func5() -> Option { + None +} + // should be linted -fn func5() -> Result { +fn func6() -> Result { Ok(1) } // should not be linted -fn func6(a: bool) -> Result { +fn func7(a: bool) -> Result { if a { Ok(1) } else { @@ -56,6 +61,11 @@ fn func6(a: bool) -> Result { } } +// should not be linted +fn func8(a: bool) -> Result { + Err(()) +} + fn main() { // method calls are not linted func1(true, true); diff --git a/tests/ui/unnecessary_wrap.rs b/tests/ui/unnecessary_wrap.rs index 749bb95c4178..e7e3120f82f3 100644 --- a/tests/ui/unnecessary_wrap.rs +++ b/tests/ui/unnecessary_wrap.rs @@ -42,13 +42,18 @@ fn func4() -> Option { Some(1) } +// should not be linted +fn func5() -> Option { + None +} + // should be linted -fn func5() -> Result { +fn func6() -> Result { Ok(1) } // should not be linted -fn func6(a: bool) -> Result { +fn func7(a: bool) -> Result { if a { Ok(1) } else { @@ -56,6 +61,11 @@ fn func6(a: bool) -> Result { } } +// should not be linted +fn func8(a: bool) -> Result { + Err(()) +} + fn main() { // method calls are not linted func1(true, true); diff --git a/tests/ui/unnecessary_wrap.stderr b/tests/ui/unnecessary_wrap.stderr index 511d085c82fe..76859fac589f 100644 --- a/tests/ui/unnecessary_wrap.stderr +++ b/tests/ui/unnecessary_wrap.stderr @@ -36,16 +36,16 @@ LL | 1 | error: this function returns unnecessarily wrapping data - --> $DIR/unnecessary_wrap.rs:46:1 + --> $DIR/unnecessary_wrap.rs:51:1 | -LL | / fn func5() -> Result { +LL | / fn func6() -> Result { LL | | Ok(1) LL | | } | |_^ | help: factor this out to | -LL | fn func5() -> i32 { +LL | fn func6() -> i32 { LL | 1 | From ebdd4e2c723c6902851a050ec8bdc7b966dc2c64 Mon Sep 17 00:00:00 2001 From: Hirochika Matsumoto Date: Wed, 23 Sep 2020 02:53:55 +0900 Subject: [PATCH 05/29] Refactor code according to reivews --- clippy_lints/src/unnecessary_wrap.rs | 224 +++++++++++++++++---------- 1 file changed, 141 insertions(+), 83 deletions(-) diff --git a/clippy_lints/src/unnecessary_wrap.rs b/clippy_lints/src/unnecessary_wrap.rs index c5d0d5100796..53ade7baec7b 100644 --- a/clippy_lints/src/unnecessary_wrap.rs +++ b/clippy_lints/src/unnecessary_wrap.rs @@ -4,7 +4,7 @@ use crate::utils::{ }; use if_chain::if_chain; use rustc_errors::Applicability; -use rustc_hir::intravisit::{FnKind, NestedVisitorMap, Visitor}; +use rustc_hir::intravisit::{FnKind, Visitor}; use rustc_hir::*; use rustc_lint::{LateContext, LateLintPass}; use rustc_middle::{hir::map::Map, ty::subst::GenericArgKind}; @@ -71,57 +71,31 @@ impl<'tcx> LateLintPass<'tcx> for UnnecessaryWrap { } } - if let ExprKind::Block(ref block, ..) = body.value.kind { - let path = if is_type_diagnostic_item(cx, return_ty(cx, hir_id), sym!(option_type)) { - &paths::OPTION_SOME - } else if is_type_diagnostic_item(cx, return_ty(cx, hir_id), sym!(result_type)) { - &paths::RESULT_OK - } else { - return; - }; - - let mut visitor = UnnecessaryWrapVisitor { result: Vec::new() }; - visitor.visit_block(block); - let result = visitor.result; - - if result.iter().any(|expr| { - if_chain! { - if let ExprKind::Call(ref func, ref args) = expr.kind; - if let ExprKind::Path(ref qpath) = func.kind; - if match_qpath(qpath, path); - if args.len() == 1; - then { - false - } else { - true - } + let path = if is_type_diagnostic_item(cx, return_ty(cx, hir_id), sym!(option_type)) { + &paths::OPTION_SOME + } else if is_type_diagnostic_item(cx, return_ty(cx, hir_id), sym!(result_type)) { + &paths::RESULT_OK + } else { + return; + }; + + let mut suggs = Vec::new(); + let can_sugg = find_all_ret_expressions(cx, &body.value, |ret_expr| { + if_chain! { + if let ExprKind::Call(ref func, ref args) = ret_expr.kind; + if let ExprKind::Path(ref qpath) = func.kind; + if match_qpath(qpath, path); + if args.len() == 1; + then { + suggs.push((ret_expr.span, snippet(cx, args[0].span.source_callsite(), "..").to_string())); + true + } else { + false } - }) { - return; } + }); - let suggs = result - .iter() - .filter_map(|expr| { - let snippet = if let ExprKind::Call(_, ref args) = expr.kind { - Some(snippet(cx, args[0].span, "..").to_string()) - } else { - None - }; - snippet.map(|snip| (expr.span, snip)) - }) - .chain({ - let inner_ty = return_ty(cx, hir_id) - .walk() - .skip(1) // skip `std::option::Option` or `std::result::Result` - .take(1) // first outermost inner type is needed - .filter_map(|inner| match inner.unpack() { - GenericArgKind::Type(inner_ty) => Some(inner_ty.to_string()), - _ => None, - }); - inner_ty.map(|inner_ty| (fn_decl.output.span(), inner_ty)) - }); - + if can_sugg { span_lint_and_then( cx, UNNECESSARY_WRAP, @@ -132,7 +106,17 @@ impl<'tcx> LateLintPass<'tcx> for UnnecessaryWrap { diag, "factor this out to", Applicability::MachineApplicable, - suggs, + suggs.into_iter().chain({ + let inner_ty = return_ty(cx, hir_id) + .walk() + .skip(1) // skip `std::option::Option` or `std::result::Result` + .take(1) // take the first outermost inner type + .filter_map(|inner| match inner.unpack() { + GenericArgKind::Type(inner_ty) => Some(inner_ty.to_string()), + _ => None, + }); + inner_ty.map(|inner_ty| (fn_decl.output.span(), inner_ty)) + }), ); }, ); @@ -140,51 +124,125 @@ impl<'tcx> LateLintPass<'tcx> for UnnecessaryWrap { } } -struct UnnecessaryWrapVisitor<'tcx> { - result: Vec<&'tcx Expr<'tcx>>, -} +// code below is copied from `bind_instead_of_map` + +fn find_all_ret_expressions<'hir, F>(_cx: &LateContext<'_>, expr: &'hir Expr<'hir>, callback: F) -> bool +where + F: FnMut(&'hir Expr<'hir>) -> bool, +{ + struct RetFinder { + in_stmt: bool, + failed: bool, + cb: F, + } + + struct WithStmtGuarg<'a, F> { + val: &'a mut RetFinder, + prev_in_stmt: bool, + } -impl<'tcx> Visitor<'tcx> for UnnecessaryWrapVisitor<'tcx> { - type Map = Map<'tcx>; + impl RetFinder { + fn inside_stmt(&mut self, in_stmt: bool) -> WithStmtGuarg<'_, F> { + let prev_in_stmt = std::mem::replace(&mut self.in_stmt, in_stmt); + WithStmtGuarg { + val: self, + prev_in_stmt, + } + } + } - fn visit_block(&mut self, block: &'tcx Block<'tcx>) { - for stmt in block.stmts { - self.visit_stmt(stmt); + impl std::ops::Deref for WithStmtGuarg<'_, F> { + type Target = RetFinder; + + fn deref(&self) -> &Self::Target { + self.val } - if let Some(expr) = block.expr { - self.visit_expr(expr) + } + + impl std::ops::DerefMut for WithStmtGuarg<'_, F> { + fn deref_mut(&mut self) -> &mut Self::Target { + self.val } } - fn visit_stmt(&mut self, stmt: &'tcx Stmt<'tcx>) { - match stmt.kind { - StmtKind::Semi(ref expr) => { - if let ExprKind::Ret(Some(value)) = expr.kind { - self.result.push(value); - } - }, - StmtKind::Expr(ref expr) => self.visit_expr(expr), - _ => (), + impl Drop for WithStmtGuarg<'_, F> { + fn drop(&mut self) { + self.val.in_stmt = self.prev_in_stmt; } } - fn visit_expr(&mut self, expr: &'tcx Expr<'tcx>) { - match expr.kind { - ExprKind::Ret(Some(value)) => self.result.push(value), - ExprKind::Call(..) | ExprKind::Path(..) => self.result.push(expr), - ExprKind::Block(ref block, _) | ExprKind::Loop(ref block, ..) => { - self.visit_block(block); - }, - ExprKind::Match(_, arms, _) => { - for arm in arms { - self.visit_expr(arm.body); + impl<'hir, F: FnMut(&'hir Expr<'hir>) -> bool> intravisit::Visitor<'hir> for RetFinder { + type Map = Map<'hir>; + + fn nested_visit_map(&mut self) -> intravisit::NestedVisitorMap { + intravisit::NestedVisitorMap::None + } + + fn visit_stmt(&mut self, stmt: &'hir Stmt<'_>) { + intravisit::walk_stmt(&mut *self.inside_stmt(true), stmt) + } + + fn visit_expr(&mut self, expr: &'hir Expr<'_>) { + if self.failed { + return; + } + if self.in_stmt { + match expr.kind { + ExprKind::Ret(Some(expr)) => self.inside_stmt(false).visit_expr(expr), + _ => intravisit::walk_expr(self, expr), + } + } else { + match expr.kind { + ExprKind::Match(cond, arms, _) => { + self.inside_stmt(true).visit_expr(cond); + for arm in arms { + self.visit_expr(arm.body); + } + }, + ExprKind::Block(..) => intravisit::walk_expr(self, expr), + ExprKind::Ret(Some(expr)) => self.visit_expr(expr), + _ => self.failed |= !(self.cb)(expr), } - }, - _ => intravisit::walk_expr(self, expr), + } } } - fn nested_visit_map(&mut self) -> NestedVisitorMap { - NestedVisitorMap::None + !contains_try(expr) && { + let mut ret_finder = RetFinder { + in_stmt: false, + failed: false, + cb: callback, + }; + ret_finder.visit_expr(expr); + !ret_finder.failed + } +} + +/// returns `true` if expr contains match expr desugared from try +fn contains_try(expr: &Expr<'_>) -> bool { + struct TryFinder { + found: bool, + } + + impl<'hir> intravisit::Visitor<'hir> for TryFinder { + type Map = Map<'hir>; + + fn nested_visit_map(&mut self) -> intravisit::NestedVisitorMap { + intravisit::NestedVisitorMap::None + } + + fn visit_expr(&mut self, expr: &'hir Expr<'hir>) { + if self.found { + return; + } + match expr.kind { + ExprKind::Match(_, _, MatchSource::TryDesugar) => self.found = true, + _ => intravisit::walk_expr(self, expr), + } + } } + + let mut visitor = TryFinder { found: false }; + visitor.visit_expr(expr); + visitor.found } From 6a62390c86cb9f72465f1c314b64c94c273956b7 Mon Sep 17 00:00:00 2001 From: Hirochika Matsumoto Date: Wed, 23 Sep 2020 02:57:47 +0900 Subject: [PATCH 06/29] Optout rustfix test --- tests/ui/unnecessary_wrap.fixed | 73 -------------------------------- tests/ui/unnecessary_wrap.rs | 2 - tests/ui/unnecessary_wrap.stderr | 6 +-- 3 files changed, 3 insertions(+), 78 deletions(-) delete mode 100644 tests/ui/unnecessary_wrap.fixed diff --git a/tests/ui/unnecessary_wrap.fixed b/tests/ui/unnecessary_wrap.fixed deleted file mode 100644 index e7e3120f82f3..000000000000 --- a/tests/ui/unnecessary_wrap.fixed +++ /dev/null @@ -1,73 +0,0 @@ -// run-rustfix - -#![warn(clippy::unnecessary_wrap)] -#![allow(clippy::no_effect)] -#![allow(clippy::needless_return)] -#![allow(clippy::if_same_then_else)] -#![allow(dead_code)] - -// should be linted -fn func1(a: bool, b: bool) -> Option { - if a && b { - return Some(42); - } - if a { - Some(-1); - Some(2) - } else { - return Some(1337); - } -} - -// public fns should not be linted -pub fn func2(a: bool) -> Option { - if a { - Some(1) - } else { - Some(1) - } -} - -// should not be linted -fn func3(a: bool) -> Option { - if a { - Some(1) - } else { - None - } -} - -// should be linted -fn func4() -> Option { - Some(1) -} - -// should not be linted -fn func5() -> Option { - None -} - -// should be linted -fn func6() -> Result { - Ok(1) -} - -// should not be linted -fn func7(a: bool) -> Result { - if a { - Ok(1) - } else { - Err(()) - } -} - -// should not be linted -fn func8(a: bool) -> Result { - Err(()) -} - -fn main() { - // method calls are not linted - func1(true, true); - func2(true); -} diff --git a/tests/ui/unnecessary_wrap.rs b/tests/ui/unnecessary_wrap.rs index e7e3120f82f3..f78a7604a5a3 100644 --- a/tests/ui/unnecessary_wrap.rs +++ b/tests/ui/unnecessary_wrap.rs @@ -1,5 +1,3 @@ -// run-rustfix - #![warn(clippy::unnecessary_wrap)] #![allow(clippy::no_effect)] #![allow(clippy::needless_return)] diff --git a/tests/ui/unnecessary_wrap.stderr b/tests/ui/unnecessary_wrap.stderr index 76859fac589f..cd05104f4900 100644 --- a/tests/ui/unnecessary_wrap.stderr +++ b/tests/ui/unnecessary_wrap.stderr @@ -1,5 +1,5 @@ error: this function returns unnecessarily wrapping data - --> $DIR/unnecessary_wrap.rs:10:1 + --> $DIR/unnecessary_wrap.rs:8:1 | LL | / fn func1(a: bool, b: bool) -> Option { LL | | if a && b { @@ -22,7 +22,7 @@ LL | Some(-1); ... error: this function returns unnecessarily wrapping data - --> $DIR/unnecessary_wrap.rs:41:1 + --> $DIR/unnecessary_wrap.rs:39:1 | LL | / fn func4() -> Option { LL | | Some(1) @@ -36,7 +36,7 @@ LL | 1 | error: this function returns unnecessarily wrapping data - --> $DIR/unnecessary_wrap.rs:51:1 + --> $DIR/unnecessary_wrap.rs:49:1 | LL | / fn func6() -> Result { LL | | Ok(1) From 3ed89026231a17beed9e93b055ea1b6192997a68 Mon Sep 17 00:00:00 2001 From: Hirochika Matsumoto Date: Wed, 23 Sep 2020 03:06:52 +0900 Subject: [PATCH 07/29] Fix typo Co-authored-by: Philipp Krones --- clippy_lints/src/unnecessary_wrap.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clippy_lints/src/unnecessary_wrap.rs b/clippy_lints/src/unnecessary_wrap.rs index 53ade7baec7b..e5ef7cde7891 100644 --- a/clippy_lints/src/unnecessary_wrap.rs +++ b/clippy_lints/src/unnecessary_wrap.rs @@ -17,7 +17,7 @@ declare_clippy_lint! { /// **Why is this bad?** It is not meaningful to wrap values when no `None` or `Err` is returned. /// /// **Known problems:** Since this lint changes function type signature, you may need to - /// adjust some codes at callee side. + /// adjust some code at callee side. /// /// **Example:** /// From c77585672720285db5180b2ee4207c7ee9b51072 Mon Sep 17 00:00:00 2001 From: Hirochika Matsumoto Date: Wed, 23 Sep 2020 03:42:58 +0900 Subject: [PATCH 08/29] Call `diag.multipart_suggestion` instead --- clippy_lints/src/unnecessary_wrap.rs | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/clippy_lints/src/unnecessary_wrap.rs b/clippy_lints/src/unnecessary_wrap.rs index e5ef7cde7891..d581912284f9 100644 --- a/clippy_lints/src/unnecessary_wrap.rs +++ b/clippy_lints/src/unnecessary_wrap.rs @@ -1,5 +1,5 @@ use crate::utils::{ - is_type_diagnostic_item, match_qpath, multispan_sugg_with_applicability, paths, return_ty, snippet, + is_type_diagnostic_item, match_qpath, paths, return_ty, snippet, span_lint_and_then, }; use if_chain::if_chain; @@ -102,10 +102,8 @@ impl<'tcx> LateLintPass<'tcx> for UnnecessaryWrap { span, "this function returns unnecessarily wrapping data", move |diag| { - multispan_sugg_with_applicability( - diag, + diag.multipart_suggestion( "factor this out to", - Applicability::MachineApplicable, suggs.into_iter().chain({ let inner_ty = return_ty(cx, hir_id) .walk() @@ -116,7 +114,8 @@ impl<'tcx> LateLintPass<'tcx> for UnnecessaryWrap { _ => None, }); inner_ty.map(|inner_ty| (fn_decl.output.span(), inner_ty)) - }), + }).collect(), + Applicability::MachineApplicable, ); }, ); From a433d4690e04c8c8eceb1f59c78fded1f04cf6f0 Mon Sep 17 00:00:00 2001 From: Hirochika Matsumoto Date: Wed, 23 Sep 2020 03:48:15 +0900 Subject: [PATCH 09/29] Run rustfmt --- clippy_lints/src/unnecessary_wrap.rs | 30 ++++++++++++++-------------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/clippy_lints/src/unnecessary_wrap.rs b/clippy_lints/src/unnecessary_wrap.rs index d581912284f9..1c8a08172a3d 100644 --- a/clippy_lints/src/unnecessary_wrap.rs +++ b/clippy_lints/src/unnecessary_wrap.rs @@ -1,7 +1,4 @@ -use crate::utils::{ - is_type_diagnostic_item, match_qpath, paths, return_ty, snippet, - span_lint_and_then, -}; +use crate::utils::{is_type_diagnostic_item, match_qpath, paths, return_ty, snippet, span_lint_and_then}; use if_chain::if_chain; use rustc_errors::Applicability; use rustc_hir::intravisit::{FnKind, Visitor}; @@ -104,17 +101,20 @@ impl<'tcx> LateLintPass<'tcx> for UnnecessaryWrap { move |diag| { diag.multipart_suggestion( "factor this out to", - suggs.into_iter().chain({ - let inner_ty = return_ty(cx, hir_id) - .walk() - .skip(1) // skip `std::option::Option` or `std::result::Result` - .take(1) // take the first outermost inner type - .filter_map(|inner| match inner.unpack() { - GenericArgKind::Type(inner_ty) => Some(inner_ty.to_string()), - _ => None, - }); - inner_ty.map(|inner_ty| (fn_decl.output.span(), inner_ty)) - }).collect(), + suggs + .into_iter() + .chain({ + let inner_ty = return_ty(cx, hir_id) + .walk() + .skip(1) // skip `std::option::Option` or `std::result::Result` + .take(1) // take the first outermost inner type + .filter_map(|inner| match inner.unpack() { + GenericArgKind::Type(inner_ty) => Some(inner_ty.to_string()), + _ => None, + }); + inner_ty.map(|inner_ty| (fn_decl.output.span(), inner_ty)) + }) + .collect(), Applicability::MachineApplicable, ); }, From cdb72df6f9f9d6946b0614b01e70bc9c46edfe89 Mon Sep 17 00:00:00 2001 From: Hirochika Matsumoto Date: Sat, 26 Sep 2020 15:39:39 +0900 Subject: [PATCH 10/29] Split lint suggestion into two --- clippy_lints/src/unnecessary_wrap.rs | 41 +++++++++++++++------------- tests/ui/unnecessary_wrap.stderr | 18 +++++++++--- 2 files changed, 36 insertions(+), 23 deletions(-) diff --git a/clippy_lints/src/unnecessary_wrap.rs b/clippy_lints/src/unnecessary_wrap.rs index 1c8a08172a3d..3ddf921a04b9 100644 --- a/clippy_lints/src/unnecessary_wrap.rs +++ b/clippy_lints/src/unnecessary_wrap.rs @@ -68,10 +68,10 @@ impl<'tcx> LateLintPass<'tcx> for UnnecessaryWrap { } } - let path = if is_type_diagnostic_item(cx, return_ty(cx, hir_id), sym!(option_type)) { - &paths::OPTION_SOME + let (return_type, path) = if is_type_diagnostic_item(cx, return_ty(cx, hir_id), sym!(option_type)) { + ("Option", &paths::OPTION_SOME) } else if is_type_diagnostic_item(cx, return_ty(cx, hir_id), sym!(result_type)) { - &paths::RESULT_OK + ("Result", &paths::RESULT_OK) } else { return; }; @@ -98,23 +98,26 @@ impl<'tcx> LateLintPass<'tcx> for UnnecessaryWrap { UNNECESSARY_WRAP, span, "this function returns unnecessarily wrapping data", - move |diag| { + |diag| { + let inner_ty = return_ty(cx, hir_id) + .walk() + .skip(1) // skip `std::option::Option` or `std::result::Result` + .take(1) // take the first outermost inner type + .filter_map(|inner| match inner.unpack() { + GenericArgKind::Type(inner_ty) => Some(inner_ty.to_string()), + _ => None, + }); + inner_ty.for_each(|inner_ty| { + diag.span_suggestion( + fn_decl.output.span(), + format!("remove `{}` from the return type...", return_type).as_str(), + inner_ty, + Applicability::MachineApplicable, + ); + }); diag.multipart_suggestion( - "factor this out to", - suggs - .into_iter() - .chain({ - let inner_ty = return_ty(cx, hir_id) - .walk() - .skip(1) // skip `std::option::Option` or `std::result::Result` - .take(1) // take the first outermost inner type - .filter_map(|inner| match inner.unpack() { - GenericArgKind::Type(inner_ty) => Some(inner_ty.to_string()), - _ => None, - }); - inner_ty.map(|inner_ty| (fn_decl.output.span(), inner_ty)) - }) - .collect(), + "...and change the returning expressions", + suggs, Applicability::MachineApplicable, ); }, diff --git a/tests/ui/unnecessary_wrap.stderr b/tests/ui/unnecessary_wrap.stderr index cd05104f4900..7833ee4b213a 100644 --- a/tests/ui/unnecessary_wrap.stderr +++ b/tests/ui/unnecessary_wrap.stderr @@ -11,14 +11,18 @@ LL | | } | |_^ | = note: `-D clippy::unnecessary-wrap` implied by `-D warnings` -help: factor this out to +help: remove `Option` from the return type... | LL | fn func1(a: bool, b: bool) -> i32 { -LL | if a && b { + | ^^^ +help: ...and change the returning expressions + | LL | return 42; LL | } LL | if a { LL | Some(-1); +LL | 2 +LL | } else { ... error: this function returns unnecessarily wrapping data @@ -29,9 +33,12 @@ LL | | Some(1) LL | | } | |_^ | -help: factor this out to +help: remove `Option` from the return type... | LL | fn func4() -> i32 { + | ^^^ +help: ...and change the returning expressions + | LL | 1 | @@ -43,9 +50,12 @@ LL | | Ok(1) LL | | } | |_^ | -help: factor this out to +help: remove `Result` from the return type... | LL | fn func6() -> i32 { + | ^^^ +help: ...and change the returning expressions + | LL | 1 | From 6b55f3fec98fa8fd4ca15edbf1cc1ab86d22f08f Mon Sep 17 00:00:00 2001 From: Hirochika Matsumoto Date: Sat, 26 Sep 2020 16:27:10 +0900 Subject: [PATCH 11/29] Add test case --- tests/ui/unnecessary_wrap.rs | 28 +++++++++++++++------- tests/ui/unnecessary_wrap.stderr | 40 ++++++++++++++++++++++++++------ 2 files changed, 53 insertions(+), 15 deletions(-) diff --git a/tests/ui/unnecessary_wrap.rs b/tests/ui/unnecessary_wrap.rs index f78a7604a5a3..6037f5807d3e 100644 --- a/tests/ui/unnecessary_wrap.rs +++ b/tests/ui/unnecessary_wrap.rs @@ -17,8 +17,20 @@ fn func1(a: bool, b: bool) -> Option { } } +// should be linted +fn func2(a: bool, b: bool) -> Option { + if a && b { + return Some(10); + } + if a { + Some(20) + } else { + Some(30) + } +} + // public fns should not be linted -pub fn func2(a: bool) -> Option { +pub fn func3(a: bool) -> Option { if a { Some(1) } else { @@ -27,7 +39,7 @@ pub fn func2(a: bool) -> Option { } // should not be linted -fn func3(a: bool) -> Option { +fn func4(a: bool) -> Option { if a { Some(1) } else { @@ -36,22 +48,22 @@ fn func3(a: bool) -> Option { } // should be linted -fn func4() -> Option { +fn func5() -> Option { Some(1) } // should not be linted -fn func5() -> Option { +fn func6() -> Option { None } // should be linted -fn func6() -> Result { +fn func7() -> Result { Ok(1) } // should not be linted -fn func7(a: bool) -> Result { +fn func8(a: bool) -> Result { if a { Ok(1) } else { @@ -60,12 +72,12 @@ fn func7(a: bool) -> Result { } // should not be linted -fn func8(a: bool) -> Result { +fn func9(a: bool) -> Result { Err(()) } fn main() { // method calls are not linted func1(true, true); - func2(true); + func2(true, true); } diff --git a/tests/ui/unnecessary_wrap.stderr b/tests/ui/unnecessary_wrap.stderr index 7833ee4b213a..a3481330e99b 100644 --- a/tests/ui/unnecessary_wrap.stderr +++ b/tests/ui/unnecessary_wrap.stderr @@ -26,16 +26,42 @@ LL | } else { ... error: this function returns unnecessarily wrapping data - --> $DIR/unnecessary_wrap.rs:39:1 + --> $DIR/unnecessary_wrap.rs:21:1 | -LL | / fn func4() -> Option { +LL | / fn func2(a: bool, b: bool) -> Option { +LL | | if a && b { +LL | | return Some(10); +LL | | } +... | +LL | | } +LL | | } + | |_^ + | +help: remove `Option` from the return type... + | +LL | fn func2(a: bool, b: bool) -> i32 { + | ^^^ +help: ...and change the returning expressions + | +LL | return 10; +LL | } +LL | if a { +LL | 20 +LL | } else { +LL | 30 + | + +error: this function returns unnecessarily wrapping data + --> $DIR/unnecessary_wrap.rs:51:1 + | +LL | / fn func5() -> Option { LL | | Some(1) LL | | } | |_^ | help: remove `Option` from the return type... | -LL | fn func4() -> i32 { +LL | fn func5() -> i32 { | ^^^ help: ...and change the returning expressions | @@ -43,21 +69,21 @@ LL | 1 | error: this function returns unnecessarily wrapping data - --> $DIR/unnecessary_wrap.rs:49:1 + --> $DIR/unnecessary_wrap.rs:61:1 | -LL | / fn func6() -> Result { +LL | / fn func7() -> Result { LL | | Ok(1) LL | | } | |_^ | help: remove `Result` from the return type... | -LL | fn func6() -> i32 { +LL | fn func7() -> i32 { | ^^^ help: ...and change the returning expressions | LL | 1 | -error: aborting due to 3 previous errors +error: aborting due to 4 previous errors From df0d565e597ad9c436fadfb8e11a16a92bcb4114 Mon Sep 17 00:00:00 2001 From: Hirochika Matsumoto Date: Fri, 2 Oct 2020 18:21:17 +0900 Subject: [PATCH 12/29] Move `find_all_ret_expressions` into `utils` --- .../src/methods/bind_instead_of_map.rs | 125 +---------------- clippy_lints/src/unnecessary_wrap.rs | 132 +----------------- clippy_lints/src/utils/mod.rs | 1 + clippy_lints/src/utils/visitors.rs | 125 +++++++++++++++++ 4 files changed, 133 insertions(+), 250 deletions(-) create mode 100644 clippy_lints/src/utils/visitors.rs diff --git a/clippy_lints/src/methods/bind_instead_of_map.rs b/clippy_lints/src/methods/bind_instead_of_map.rs index ae37942e55a1..540a1484a855 100644 --- a/clippy_lints/src/methods/bind_instead_of_map.rs +++ b/clippy_lints/src/methods/bind_instead_of_map.rs @@ -1,14 +1,12 @@ use super::{contains_return, BIND_INSTEAD_OF_MAP}; use crate::utils::{ in_macro, match_qpath, match_type, method_calls, multispan_sugg_with_applicability, paths, remove_blocks, snippet, - snippet_with_macro_callsite, span_lint_and_sugg, span_lint_and_then, + snippet_with_macro_callsite, span_lint_and_sugg, span_lint_and_then, visitors::find_all_ret_expressions, }; use if_chain::if_chain; use rustc_errors::Applicability; use rustc_hir as hir; -use rustc_hir::intravisit::{self, Visitor}; use rustc_lint::LateContext; -use rustc_middle::hir::map::Map; use rustc_span::Span; pub(crate) struct OptionAndThenSome; @@ -193,124 +191,3 @@ pub(crate) trait BindInsteadOfMap { } } } - -/// returns `true` if expr contains match expr desugared from try -fn contains_try(expr: &hir::Expr<'_>) -> bool { - struct TryFinder { - found: bool, - } - - impl<'hir> intravisit::Visitor<'hir> for TryFinder { - type Map = Map<'hir>; - - fn nested_visit_map(&mut self) -> intravisit::NestedVisitorMap { - intravisit::NestedVisitorMap::None - } - - fn visit_expr(&mut self, expr: &'hir hir::Expr<'hir>) { - if self.found { - return; - } - match expr.kind { - hir::ExprKind::Match(_, _, hir::MatchSource::TryDesugar) => self.found = true, - _ => intravisit::walk_expr(self, expr), - } - } - } - - let mut visitor = TryFinder { found: false }; - visitor.visit_expr(expr); - visitor.found -} - -fn find_all_ret_expressions<'hir, F>(_cx: &LateContext<'_>, expr: &'hir hir::Expr<'hir>, callback: F) -> bool -where - F: FnMut(&'hir hir::Expr<'hir>) -> bool, -{ - struct RetFinder { - in_stmt: bool, - failed: bool, - cb: F, - } - - struct WithStmtGuarg<'a, F> { - val: &'a mut RetFinder, - prev_in_stmt: bool, - } - - impl RetFinder { - fn inside_stmt(&mut self, in_stmt: bool) -> WithStmtGuarg<'_, F> { - let prev_in_stmt = std::mem::replace(&mut self.in_stmt, in_stmt); - WithStmtGuarg { - val: self, - prev_in_stmt, - } - } - } - - impl std::ops::Deref for WithStmtGuarg<'_, F> { - type Target = RetFinder; - - fn deref(&self) -> &Self::Target { - self.val - } - } - - impl std::ops::DerefMut for WithStmtGuarg<'_, F> { - fn deref_mut(&mut self) -> &mut Self::Target { - self.val - } - } - - impl Drop for WithStmtGuarg<'_, F> { - fn drop(&mut self) { - self.val.in_stmt = self.prev_in_stmt; - } - } - - impl<'hir, F: FnMut(&'hir hir::Expr<'hir>) -> bool> intravisit::Visitor<'hir> for RetFinder { - type Map = Map<'hir>; - - fn nested_visit_map(&mut self) -> intravisit::NestedVisitorMap { - intravisit::NestedVisitorMap::None - } - - fn visit_stmt(&mut self, stmt: &'hir hir::Stmt<'_>) { - intravisit::walk_stmt(&mut *self.inside_stmt(true), stmt) - } - - fn visit_expr(&mut self, expr: &'hir hir::Expr<'_>) { - if self.failed { - return; - } - if self.in_stmt { - match expr.kind { - hir::ExprKind::Ret(Some(expr)) => self.inside_stmt(false).visit_expr(expr), - _ => intravisit::walk_expr(self, expr), - } - } else { - match expr.kind { - hir::ExprKind::Match(cond, arms, _) => { - self.inside_stmt(true).visit_expr(cond); - for arm in arms { - self.visit_expr(arm.body); - } - }, - hir::ExprKind::Block(..) => intravisit::walk_expr(self, expr), - hir::ExprKind::Ret(Some(expr)) => self.visit_expr(expr), - _ => self.failed |= !(self.cb)(expr), - } - } - } - } - - !contains_try(expr) && { - let mut ret_finder = RetFinder { - in_stmt: false, - failed: false, - cb: callback, - }; - ret_finder.visit_expr(expr); - !ret_finder.failed - } -} diff --git a/clippy_lints/src/unnecessary_wrap.rs b/clippy_lints/src/unnecessary_wrap.rs index 3ddf921a04b9..de13301381e2 100644 --- a/clippy_lints/src/unnecessary_wrap.rs +++ b/clippy_lints/src/unnecessary_wrap.rs @@ -1,10 +1,13 @@ -use crate::utils::{is_type_diagnostic_item, match_qpath, paths, return_ty, snippet, span_lint_and_then}; +use crate::utils::{ + is_type_diagnostic_item, match_qpath, paths, return_ty, snippet, span_lint_and_then, + visitors::find_all_ret_expressions, +}; use if_chain::if_chain; use rustc_errors::Applicability; -use rustc_hir::intravisit::{FnKind, Visitor}; +use rustc_hir::intravisit::FnKind; use rustc_hir::*; use rustc_lint::{LateContext, LateLintPass}; -use rustc_middle::{hir::map::Map, ty::subst::GenericArgKind}; +use rustc_middle::ty::subst::GenericArgKind; use rustc_session::{declare_lint_pass, declare_tool_lint}; use rustc_span::Span; @@ -125,126 +128,3 @@ impl<'tcx> LateLintPass<'tcx> for UnnecessaryWrap { } } } - -// code below is copied from `bind_instead_of_map` - -fn find_all_ret_expressions<'hir, F>(_cx: &LateContext<'_>, expr: &'hir Expr<'hir>, callback: F) -> bool -where - F: FnMut(&'hir Expr<'hir>) -> bool, -{ - struct RetFinder { - in_stmt: bool, - failed: bool, - cb: F, - } - - struct WithStmtGuarg<'a, F> { - val: &'a mut RetFinder, - prev_in_stmt: bool, - } - - impl RetFinder { - fn inside_stmt(&mut self, in_stmt: bool) -> WithStmtGuarg<'_, F> { - let prev_in_stmt = std::mem::replace(&mut self.in_stmt, in_stmt); - WithStmtGuarg { - val: self, - prev_in_stmt, - } - } - } - - impl std::ops::Deref for WithStmtGuarg<'_, F> { - type Target = RetFinder; - - fn deref(&self) -> &Self::Target { - self.val - } - } - - impl std::ops::DerefMut for WithStmtGuarg<'_, F> { - fn deref_mut(&mut self) -> &mut Self::Target { - self.val - } - } - - impl Drop for WithStmtGuarg<'_, F> { - fn drop(&mut self) { - self.val.in_stmt = self.prev_in_stmt; - } - } - - impl<'hir, F: FnMut(&'hir Expr<'hir>) -> bool> intravisit::Visitor<'hir> for RetFinder { - type Map = Map<'hir>; - - fn nested_visit_map(&mut self) -> intravisit::NestedVisitorMap { - intravisit::NestedVisitorMap::None - } - - fn visit_stmt(&mut self, stmt: &'hir Stmt<'_>) { - intravisit::walk_stmt(&mut *self.inside_stmt(true), stmt) - } - - fn visit_expr(&mut self, expr: &'hir Expr<'_>) { - if self.failed { - return; - } - if self.in_stmt { - match expr.kind { - ExprKind::Ret(Some(expr)) => self.inside_stmt(false).visit_expr(expr), - _ => intravisit::walk_expr(self, expr), - } - } else { - match expr.kind { - ExprKind::Match(cond, arms, _) => { - self.inside_stmt(true).visit_expr(cond); - for arm in arms { - self.visit_expr(arm.body); - } - }, - ExprKind::Block(..) => intravisit::walk_expr(self, expr), - ExprKind::Ret(Some(expr)) => self.visit_expr(expr), - _ => self.failed |= !(self.cb)(expr), - } - } - } - } - - !contains_try(expr) && { - let mut ret_finder = RetFinder { - in_stmt: false, - failed: false, - cb: callback, - }; - ret_finder.visit_expr(expr); - !ret_finder.failed - } -} - -/// returns `true` if expr contains match expr desugared from try -fn contains_try(expr: &Expr<'_>) -> bool { - struct TryFinder { - found: bool, - } - - impl<'hir> intravisit::Visitor<'hir> for TryFinder { - type Map = Map<'hir>; - - fn nested_visit_map(&mut self) -> intravisit::NestedVisitorMap { - intravisit::NestedVisitorMap::None - } - - fn visit_expr(&mut self, expr: &'hir Expr<'hir>) { - if self.found { - return; - } - match expr.kind { - ExprKind::Match(_, _, MatchSource::TryDesugar) => self.found = true, - _ => intravisit::walk_expr(self, expr), - } - } - } - - let mut visitor = TryFinder { found: false }; - visitor.visit_expr(expr); - visitor.found -} diff --git a/clippy_lints/src/utils/mod.rs b/clippy_lints/src/utils/mod.rs index 253b7823bd9d..bd62e67f80e3 100644 --- a/clippy_lints/src/utils/mod.rs +++ b/clippy_lints/src/utils/mod.rs @@ -21,6 +21,7 @@ pub mod ptr; pub mod qualify_min_const_fn; pub mod sugg; pub mod usage; +pub mod visitors; pub use self::attrs::*; pub use self::diagnostics::*; diff --git a/clippy_lints/src/utils/visitors.rs b/clippy_lints/src/utils/visitors.rs new file mode 100644 index 000000000000..b0837b6c43e7 --- /dev/null +++ b/clippy_lints/src/utils/visitors.rs @@ -0,0 +1,125 @@ +use rustc_hir as hir; +use rustc_hir::intravisit::{self, Visitor}; +use rustc_lint::LateContext; +use rustc_middle::hir::map::Map; + +/// returns `true` if expr contains match expr desugared from try +fn contains_try(expr: &hir::Expr<'_>) -> bool { + struct TryFinder { + found: bool, + } + + impl<'hir> intravisit::Visitor<'hir> for TryFinder { + type Map = Map<'hir>; + + fn nested_visit_map(&mut self) -> intravisit::NestedVisitorMap { + intravisit::NestedVisitorMap::None + } + + fn visit_expr(&mut self, expr: &'hir hir::Expr<'hir>) { + if self.found { + return; + } + match expr.kind { + hir::ExprKind::Match(_, _, hir::MatchSource::TryDesugar) => self.found = true, + _ => intravisit::walk_expr(self, expr), + } + } + } + + let mut visitor = TryFinder { found: false }; + visitor.visit_expr(expr); + visitor.found +} + +pub fn find_all_ret_expressions<'hir, F>(_cx: &LateContext<'_>, expr: &'hir hir::Expr<'hir>, callback: F) -> bool +where + F: FnMut(&'hir hir::Expr<'hir>) -> bool, +{ + struct RetFinder { + in_stmt: bool, + failed: bool, + cb: F, + } + + struct WithStmtGuarg<'a, F> { + val: &'a mut RetFinder, + prev_in_stmt: bool, + } + + impl RetFinder { + fn inside_stmt(&mut self, in_stmt: bool) -> WithStmtGuarg<'_, F> { + let prev_in_stmt = std::mem::replace(&mut self.in_stmt, in_stmt); + WithStmtGuarg { + val: self, + prev_in_stmt, + } + } + } + + impl std::ops::Deref for WithStmtGuarg<'_, F> { + type Target = RetFinder; + + fn deref(&self) -> &Self::Target { + self.val + } + } + + impl std::ops::DerefMut for WithStmtGuarg<'_, F> { + fn deref_mut(&mut self) -> &mut Self::Target { + self.val + } + } + + impl Drop for WithStmtGuarg<'_, F> { + fn drop(&mut self) { + self.val.in_stmt = self.prev_in_stmt; + } + } + + impl<'hir, F: FnMut(&'hir hir::Expr<'hir>) -> bool> intravisit::Visitor<'hir> for RetFinder { + type Map = Map<'hir>; + + fn nested_visit_map(&mut self) -> intravisit::NestedVisitorMap { + intravisit::NestedVisitorMap::None + } + + fn visit_stmt(&mut self, stmt: &'hir hir::Stmt<'_>) { + intravisit::walk_stmt(&mut *self.inside_stmt(true), stmt) + } + + fn visit_expr(&mut self, expr: &'hir hir::Expr<'_>) { + if self.failed { + return; + } + if self.in_stmt { + match expr.kind { + hir::ExprKind::Ret(Some(expr)) => self.inside_stmt(false).visit_expr(expr), + _ => intravisit::walk_expr(self, expr), + } + } else { + match expr.kind { + hir::ExprKind::Match(cond, arms, _) => { + self.inside_stmt(true).visit_expr(cond); + for arm in arms { + self.visit_expr(arm.body); + } + }, + hir::ExprKind::Block(..) => intravisit::walk_expr(self, expr), + hir::ExprKind::Ret(Some(expr)) => self.visit_expr(expr), + _ => self.failed |= !(self.cb)(expr), + } + } + } + } + + !contains_try(expr) && { + let mut ret_finder = RetFinder { + in_stmt: false, + failed: false, + cb: callback, + }; + ret_finder.visit_expr(expr); + !ret_finder.failed + } +} From eec7f5c1113d3d49dd7f42462ea25d0b36b4b49b Mon Sep 17 00:00:00 2001 From: Hirochika Matsumoto Date: Mon, 12 Oct 2020 19:21:04 +0900 Subject: [PATCH 13/29] Update clippy_lints/src/unnecessary_wrap.rs Co-authored-by: Philipp Krones --- clippy_lints/src/unnecessary_wrap.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clippy_lints/src/unnecessary_wrap.rs b/clippy_lints/src/unnecessary_wrap.rs index de13301381e2..28762b8d2658 100644 --- a/clippy_lints/src/unnecessary_wrap.rs +++ b/clippy_lints/src/unnecessary_wrap.rs @@ -100,7 +100,7 @@ impl<'tcx> LateLintPass<'tcx> for UnnecessaryWrap { cx, UNNECESSARY_WRAP, span, - "this function returns unnecessarily wrapping data", + format!("this function's return value is unnecessarily wrapped by `{}`, return_type)", |diag| { let inner_ty = return_ty(cx, hir_id) .walk() From 1bdac8712868e1418dcb13e817254620a8c01158 Mon Sep 17 00:00:00 2001 From: Hirochika Matsumoto Date: Mon, 12 Oct 2020 19:27:47 +0900 Subject: [PATCH 14/29] Improve lint message --- clippy_lints/src/unnecessary_wrap.rs | 2 +- tests/ui/unnecessary_wrap.stderr | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/clippy_lints/src/unnecessary_wrap.rs b/clippy_lints/src/unnecessary_wrap.rs index 28762b8d2658..db51ee681ef2 100644 --- a/clippy_lints/src/unnecessary_wrap.rs +++ b/clippy_lints/src/unnecessary_wrap.rs @@ -100,7 +100,7 @@ impl<'tcx> LateLintPass<'tcx> for UnnecessaryWrap { cx, UNNECESSARY_WRAP, span, - format!("this function's return value is unnecessarily wrapped by `{}`, return_type)", + format!("this function's return value is unnecessarily wrapped by `{}`", return_type).as_str(), |diag| { let inner_ty = return_ty(cx, hir_id) .walk() diff --git a/tests/ui/unnecessary_wrap.stderr b/tests/ui/unnecessary_wrap.stderr index a3481330e99b..958bc9980225 100644 --- a/tests/ui/unnecessary_wrap.stderr +++ b/tests/ui/unnecessary_wrap.stderr @@ -1,4 +1,4 @@ -error: this function returns unnecessarily wrapping data +error: this function's return value is unnecessarily wrapped by `Option` --> $DIR/unnecessary_wrap.rs:8:1 | LL | / fn func1(a: bool, b: bool) -> Option { @@ -25,7 +25,7 @@ LL | 2 LL | } else { ... -error: this function returns unnecessarily wrapping data +error: this function's return value is unnecessarily wrapped by `Option` --> $DIR/unnecessary_wrap.rs:21:1 | LL | / fn func2(a: bool, b: bool) -> Option { @@ -51,7 +51,7 @@ LL | } else { LL | 30 | -error: this function returns unnecessarily wrapping data +error: this function's return value is unnecessarily wrapped by `Option` --> $DIR/unnecessary_wrap.rs:51:1 | LL | / fn func5() -> Option { @@ -68,7 +68,7 @@ help: ...and change the returning expressions LL | 1 | -error: this function returns unnecessarily wrapping data +error: this function's return value is unnecessarily wrapped by `Result` --> $DIR/unnecessary_wrap.rs:61:1 | LL | / fn func7() -> Result { From 8392bc7946a212a85bc5a411f5321a1f76d5ccf6 Mon Sep 17 00:00:00 2001 From: Hirochika Matsumoto Date: Mon, 12 Oct 2020 19:37:27 +0900 Subject: [PATCH 15/29] Run `cargo dev fmt` --- clippy_lints/src/unnecessary_wrap.rs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/clippy_lints/src/unnecessary_wrap.rs b/clippy_lints/src/unnecessary_wrap.rs index db51ee681ef2..fc1a33fc6cd7 100644 --- a/clippy_lints/src/unnecessary_wrap.rs +++ b/clippy_lints/src/unnecessary_wrap.rs @@ -100,7 +100,11 @@ impl<'tcx> LateLintPass<'tcx> for UnnecessaryWrap { cx, UNNECESSARY_WRAP, span, - format!("this function's return value is unnecessarily wrapped by `{}`", return_type).as_str(), + format!( + "this function's return value is unnecessarily wrapped by `{}`", + return_type + ) + .as_str(), |diag| { let inner_ty = return_ty(cx, hir_id) .walk() From 2f85aa736e85544803ff6d3e6b13e44c0729bbdd Mon Sep 17 00:00:00 2001 From: Hirochika Matsumoto Date: Sun, 18 Oct 2020 16:30:46 +0900 Subject: [PATCH 16/29] Make lint skip closures --- clippy_lints/src/unnecessary_wrap.rs | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/clippy_lints/src/unnecessary_wrap.rs b/clippy_lints/src/unnecessary_wrap.rs index fc1a33fc6cd7..a20d1f82e9ef 100644 --- a/clippy_lints/src/unnecessary_wrap.rs +++ b/clippy_lints/src/unnecessary_wrap.rs @@ -63,12 +63,10 @@ impl<'tcx> LateLintPass<'tcx> for UnnecessaryWrap { span: Span, hir_id: HirId, ) { - if_chain! { - if let FnKind::ItemFn(.., visibility, _) = fn_kind; - if visibility.node.is_pub(); - then { - return; - } + match fn_kind { + FnKind::ItemFn(.., visibility, _) if visibility.node.is_pub() => return, + FnKind::Closure(..) => return, + _ => (), } let (return_type, path) = if is_type_diagnostic_item(cx, return_ty(cx, hir_id), sym!(option_type)) { From 12474c62ff89ab122c2e6881b00680913fdf1d35 Mon Sep 17 00:00:00 2001 From: Hirochika Matsumoto Date: Sun, 18 Oct 2020 16:53:18 +0900 Subject: [PATCH 17/29] Add support for methods --- clippy_lints/src/unnecessary_wrap.rs | 6 +++++- tests/ui/unnecessary_wrap.rs | 14 ++++++++++++++ tests/ui/unnecessary_wrap.stderr | 19 ++++++++++++++++++- 3 files changed, 37 insertions(+), 2 deletions(-) diff --git a/clippy_lints/src/unnecessary_wrap.rs b/clippy_lints/src/unnecessary_wrap.rs index a20d1f82e9ef..365e9c9984e8 100644 --- a/clippy_lints/src/unnecessary_wrap.rs +++ b/clippy_lints/src/unnecessary_wrap.rs @@ -64,7 +64,11 @@ impl<'tcx> LateLintPass<'tcx> for UnnecessaryWrap { hir_id: HirId, ) { match fn_kind { - FnKind::ItemFn(.., visibility, _) if visibility.node.is_pub() => return, + FnKind::ItemFn(.., visibility, _) | FnKind::Method(.., Some(visibility), _) => { + if visibility.node.is_pub() { + return; + } + }, FnKind::Closure(..) => return, _ => (), } diff --git a/tests/ui/unnecessary_wrap.rs b/tests/ui/unnecessary_wrap.rs index 6037f5807d3e..0ced20b7a566 100644 --- a/tests/ui/unnecessary_wrap.rs +++ b/tests/ui/unnecessary_wrap.rs @@ -76,6 +76,20 @@ fn func9(a: bool) -> Result { Err(()) } +struct A; + +impl A { + // should not be linted + pub fn func10() -> Option { + Some(1) + } + + // should be linted + fn func11() -> Option { + Some(1) + } +} + fn main() { // method calls are not linted func1(true, true); diff --git a/tests/ui/unnecessary_wrap.stderr b/tests/ui/unnecessary_wrap.stderr index 958bc9980225..dbc5748c29ec 100644 --- a/tests/ui/unnecessary_wrap.stderr +++ b/tests/ui/unnecessary_wrap.stderr @@ -85,5 +85,22 @@ help: ...and change the returning expressions LL | 1 | -error: aborting due to 4 previous errors +error: this function's return value is unnecessarily wrapped by `Option` + --> $DIR/unnecessary_wrap.rs:88:5 + | +LL | / fn func11() -> Option { +LL | | Some(1) +LL | | } + | |_____^ + | +help: remove `Option` from the return type... + | +LL | fn func11() -> i32 { + | ^^^ +help: ...and change the returning expressions + | +LL | 1 + | + +error: aborting due to 5 previous errors From c5447eb3c167025fcc1b842fabd7bb43a2eb1e9e Mon Sep 17 00:00:00 2001 From: Hirochika Matsumoto Date: Sun, 18 Oct 2020 17:17:45 +0900 Subject: [PATCH 18/29] Make lint skip macros --- clippy_lints/src/unnecessary_wrap.rs | 3 ++- tests/ui/unnecessary_wrap.rs | 9 +++++++-- tests/ui/unnecessary_wrap.stderr | 6 +++--- 3 files changed, 12 insertions(+), 6 deletions(-) diff --git a/clippy_lints/src/unnecessary_wrap.rs b/clippy_lints/src/unnecessary_wrap.rs index 365e9c9984e8..7ec523293c8e 100644 --- a/clippy_lints/src/unnecessary_wrap.rs +++ b/clippy_lints/src/unnecessary_wrap.rs @@ -1,5 +1,5 @@ use crate::utils::{ - is_type_diagnostic_item, match_qpath, paths, return_ty, snippet, span_lint_and_then, + in_macro, is_type_diagnostic_item, match_qpath, paths, return_ty, snippet, span_lint_and_then, visitors::find_all_ret_expressions, }; use if_chain::if_chain; @@ -84,6 +84,7 @@ impl<'tcx> LateLintPass<'tcx> for UnnecessaryWrap { let mut suggs = Vec::new(); let can_sugg = find_all_ret_expressions(cx, &body.value, |ret_expr| { if_chain! { + if !in_macro(ret_expr.span); if let ExprKind::Call(ref func, ref args) = ret_expr.kind; if let ExprKind::Path(ref qpath) = func.kind; if match_qpath(qpath, path); diff --git a/tests/ui/unnecessary_wrap.rs b/tests/ui/unnecessary_wrap.rs index 0ced20b7a566..618c452065b9 100644 --- a/tests/ui/unnecessary_wrap.rs +++ b/tests/ui/unnecessary_wrap.rs @@ -76,16 +76,21 @@ fn func9(a: bool) -> Result { Err(()) } +// should not be linted +fn func10() -> Option<()> { + unimplemented!() +} + struct A; impl A { // should not be linted - pub fn func10() -> Option { + pub fn func11() -> Option { Some(1) } // should be linted - fn func11() -> Option { + fn func12() -> Option { Some(1) } } diff --git a/tests/ui/unnecessary_wrap.stderr b/tests/ui/unnecessary_wrap.stderr index dbc5748c29ec..5f21b74bc760 100644 --- a/tests/ui/unnecessary_wrap.stderr +++ b/tests/ui/unnecessary_wrap.stderr @@ -86,16 +86,16 @@ LL | 1 | error: this function's return value is unnecessarily wrapped by `Option` - --> $DIR/unnecessary_wrap.rs:88:5 + --> $DIR/unnecessary_wrap.rs:93:5 | -LL | / fn func11() -> Option { +LL | / fn func12() -> Option { LL | | Some(1) LL | | } | |_____^ | help: remove `Option` from the return type... | -LL | fn func11() -> i32 { +LL | fn func12() -> i32 { | ^^^ help: ...and change the returning expressions | From c7692cf7493facefd520a98bf86aa1f4a031f435 Mon Sep 17 00:00:00 2001 From: Hirochika Matsumoto Date: Sun, 18 Oct 2020 17:27:08 +0900 Subject: [PATCH 19/29] Skip function with no exprs contained --- clippy_lints/src/unnecessary_wrap.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clippy_lints/src/unnecessary_wrap.rs b/clippy_lints/src/unnecessary_wrap.rs index 7ec523293c8e..28cc20f00075 100644 --- a/clippy_lints/src/unnecessary_wrap.rs +++ b/clippy_lints/src/unnecessary_wrap.rs @@ -98,7 +98,7 @@ impl<'tcx> LateLintPass<'tcx> for UnnecessaryWrap { } }); - if can_sugg { + if can_sugg && !suggs.is_empty() { span_lint_and_then( cx, UNNECESSARY_WRAP, From 30632fb8e6e4d6b998f2af7da93931ddb9f5de03 Mon Sep 17 00:00:00 2001 From: Hirochika Matsumoto Date: Sun, 18 Oct 2020 17:55:25 +0900 Subject: [PATCH 20/29] Allow this lint on lint tests --- tests/ui/derive_ord_xor_partial_ord.rs | 1 + tests/ui/derive_ord_xor_partial_ord.stderr | 16 ++++----- tests/ui/doc_errors.rs | 1 + tests/ui/drop_ref.rs | 1 + tests/ui/forget_ref.rs | 1 + tests/ui/forget_ref.stderr | 36 +++++++++---------- tests/ui/let_underscore_must_use.rs | 1 + tests/ui/let_underscore_must_use.stderr | 24 ++++++------- tests/ui/map_flatten.fixed | 1 + tests/ui/map_flatten.rs | 1 + tests/ui/needless_lifetimes.rs | 2 +- tests/ui/option_map_unit_fn_fixable.fixed | 1 + tests/ui/option_map_unit_fn_fixable.rs | 1 + tests/ui/option_map_unit_fn_fixable.stderr | 36 +++++++++---------- tests/ui/option_option.rs | 1 + tests/ui/option_option.stderr | 20 +++++------ tests/ui/or_fun_call.fixed | 1 + tests/ui/or_fun_call.rs | 1 + tests/ui/or_fun_call.stderr | 20 +++++------ tests/ui/panic_in_result_fn.rs | 1 + tests/ui/panic_in_result_fn.stderr | 24 ++++++------- tests/ui/question_mark.fixed | 1 + tests/ui/question_mark.rs | 1 + tests/ui/question_mark.stderr | 22 ++++++------ tests/ui/redundant_pattern_matching.fixed | 1 + tests/ui/redundant_pattern_matching.rs | 1 + tests/ui/redundant_pattern_matching.stderr | 6 ++-- tests/ui/try_err.fixed | 1 + tests/ui/try_err.rs | 1 + tests/ui/try_err.stderr | 8 ++--- tests/ui/unit_arg.rs | 1 + tests/ui/unit_arg.stderr | 20 +++++------ tests/ui/unnecessary_clone.rs | 2 +- tests/ui/useless_conversion.fixed | 1 + tests/ui/useless_conversion.rs | 1 + tests/ui/useless_conversion.stderr | 22 ++++++------ tests/ui/wildcard_imports.fixed | 1 + tests/ui/wildcard_imports.rs | 1 + tests/ui/wildcard_imports.stderr | 40 +++++++++++----------- 39 files changed, 173 insertions(+), 149 deletions(-) diff --git a/tests/ui/derive_ord_xor_partial_ord.rs b/tests/ui/derive_ord_xor_partial_ord.rs index b82dc518a3ba..78ec1727fc11 100644 --- a/tests/ui/derive_ord_xor_partial_ord.rs +++ b/tests/ui/derive_ord_xor_partial_ord.rs @@ -1,4 +1,5 @@ #![warn(clippy::derive_ord_xor_partial_ord)] +#![allow(clippy::unnecessary_wrap)] use std::cmp::Ordering; diff --git a/tests/ui/derive_ord_xor_partial_ord.stderr b/tests/ui/derive_ord_xor_partial_ord.stderr index 66bc4d42ce8c..97b46a4aa898 100644 --- a/tests/ui/derive_ord_xor_partial_ord.stderr +++ b/tests/ui/derive_ord_xor_partial_ord.stderr @@ -1,12 +1,12 @@ error: you are deriving `Ord` but have implemented `PartialOrd` explicitly - --> $DIR/derive_ord_xor_partial_ord.rs:20:10 + --> $DIR/derive_ord_xor_partial_ord.rs:21:10 | LL | #[derive(Ord, PartialEq, Eq)] | ^^^ | = note: `-D clippy::derive-ord-xor-partial-ord` implied by `-D warnings` note: `PartialOrd` implemented here - --> $DIR/derive_ord_xor_partial_ord.rs:23:1 + --> $DIR/derive_ord_xor_partial_ord.rs:24:1 | LL | / impl PartialOrd for DeriveOrd { LL | | fn partial_cmp(&self, other: &Self) -> Option { @@ -17,13 +17,13 @@ LL | | } = note: this error originates in a derive macro (in Nightly builds, run with -Z macro-backtrace for more info) error: you are deriving `Ord` but have implemented `PartialOrd` explicitly - --> $DIR/derive_ord_xor_partial_ord.rs:29:10 + --> $DIR/derive_ord_xor_partial_ord.rs:30:10 | LL | #[derive(Ord, PartialEq, Eq)] | ^^^ | note: `PartialOrd` implemented here - --> $DIR/derive_ord_xor_partial_ord.rs:32:1 + --> $DIR/derive_ord_xor_partial_ord.rs:33:1 | LL | / impl PartialOrd for DeriveOrdWithExplicitTypeVariable { LL | | fn partial_cmp(&self, other: &Self) -> Option { @@ -34,7 +34,7 @@ LL | | } = note: this error originates in a derive macro (in Nightly builds, run with -Z macro-backtrace for more info) error: you are implementing `Ord` explicitly but have derived `PartialOrd` - --> $DIR/derive_ord_xor_partial_ord.rs:41:1 + --> $DIR/derive_ord_xor_partial_ord.rs:42:1 | LL | / impl std::cmp::Ord for DerivePartialOrd { LL | | fn cmp(&self, other: &Self) -> Ordering { @@ -44,14 +44,14 @@ LL | | } | |_^ | note: `PartialOrd` implemented here - --> $DIR/derive_ord_xor_partial_ord.rs:38:10 + --> $DIR/derive_ord_xor_partial_ord.rs:39:10 | LL | #[derive(PartialOrd, PartialEq, Eq)] | ^^^^^^^^^^ = note: this error originates in a derive macro (in Nightly builds, run with -Z macro-backtrace for more info) error: you are implementing `Ord` explicitly but have derived `PartialOrd` - --> $DIR/derive_ord_xor_partial_ord.rs:61:5 + --> $DIR/derive_ord_xor_partial_ord.rs:62:5 | LL | / impl Ord for DerivePartialOrdInUseOrd { LL | | fn cmp(&self, other: &Self) -> Ordering { @@ -61,7 +61,7 @@ LL | | } | |_____^ | note: `PartialOrd` implemented here - --> $DIR/derive_ord_xor_partial_ord.rs:58:14 + --> $DIR/derive_ord_xor_partial_ord.rs:59:14 | LL | #[derive(PartialOrd, PartialEq, Eq)] | ^^^^^^^^^^ diff --git a/tests/ui/doc_errors.rs b/tests/ui/doc_errors.rs index f47b81a450ea..77df7f176f02 100644 --- a/tests/ui/doc_errors.rs +++ b/tests/ui/doc_errors.rs @@ -1,6 +1,7 @@ // edition:2018 #![warn(clippy::missing_errors_doc)] #![allow(clippy::result_unit_err)] +#![allow(clippy::unnecessary_wrap)] use std::io; diff --git a/tests/ui/drop_ref.rs b/tests/ui/drop_ref.rs index 6b5bcdaa78e2..ba12e7638215 100644 --- a/tests/ui/drop_ref.rs +++ b/tests/ui/drop_ref.rs @@ -1,6 +1,7 @@ #![warn(clippy::drop_ref)] #![allow(clippy::toplevel_ref_arg)] #![allow(clippy::map_err_ignore)] +#![allow(clippy::unnecessary_wrap)] use std::mem::drop; diff --git a/tests/ui/forget_ref.rs b/tests/ui/forget_ref.rs index 447fdbe1fac5..a9c2a92ce6be 100644 --- a/tests/ui/forget_ref.rs +++ b/tests/ui/forget_ref.rs @@ -1,5 +1,6 @@ #![warn(clippy::forget_ref)] #![allow(clippy::toplevel_ref_arg)] +#![allow(clippy::unnecessary_wrap)] use std::mem::forget; diff --git a/tests/ui/forget_ref.stderr b/tests/ui/forget_ref.stderr index f90bcc2762ce..b2c7f2023bfb 100644 --- a/tests/ui/forget_ref.stderr +++ b/tests/ui/forget_ref.stderr @@ -1,108 +1,108 @@ error: calls to `std::mem::forget` with a reference instead of an owned value. Forgetting a reference does nothing. - --> $DIR/forget_ref.rs:9:5 + --> $DIR/forget_ref.rs:10:5 | LL | forget(&SomeStruct); | ^^^^^^^^^^^^^^^^^^^ | = note: `-D clippy::forget-ref` implied by `-D warnings` note: argument has type `&SomeStruct` - --> $DIR/forget_ref.rs:9:12 + --> $DIR/forget_ref.rs:10:12 | LL | forget(&SomeStruct); | ^^^^^^^^^^^ error: calls to `std::mem::forget` with a reference instead of an owned value. Forgetting a reference does nothing. - --> $DIR/forget_ref.rs:12:5 + --> $DIR/forget_ref.rs:13:5 | LL | forget(&owned); | ^^^^^^^^^^^^^^ | note: argument has type `&SomeStruct` - --> $DIR/forget_ref.rs:12:12 + --> $DIR/forget_ref.rs:13:12 | LL | forget(&owned); | ^^^^^^ error: calls to `std::mem::forget` with a reference instead of an owned value. Forgetting a reference does nothing. - --> $DIR/forget_ref.rs:13:5 + --> $DIR/forget_ref.rs:14:5 | LL | forget(&&owned); | ^^^^^^^^^^^^^^^ | note: argument has type `&&SomeStruct` - --> $DIR/forget_ref.rs:13:12 + --> $DIR/forget_ref.rs:14:12 | LL | forget(&&owned); | ^^^^^^^ error: calls to `std::mem::forget` with a reference instead of an owned value. Forgetting a reference does nothing. - --> $DIR/forget_ref.rs:14:5 + --> $DIR/forget_ref.rs:15:5 | LL | forget(&mut owned); | ^^^^^^^^^^^^^^^^^^ | note: argument has type `&mut SomeStruct` - --> $DIR/forget_ref.rs:14:12 + --> $DIR/forget_ref.rs:15:12 | LL | forget(&mut owned); | ^^^^^^^^^^ error: calls to `std::mem::forget` with a reference instead of an owned value. Forgetting a reference does nothing. - --> $DIR/forget_ref.rs:18:5 + --> $DIR/forget_ref.rs:19:5 | LL | forget(&*reference1); | ^^^^^^^^^^^^^^^^^^^^ | note: argument has type `&SomeStruct` - --> $DIR/forget_ref.rs:18:12 + --> $DIR/forget_ref.rs:19:12 | LL | forget(&*reference1); | ^^^^^^^^^^^^ error: calls to `std::mem::forget` with a reference instead of an owned value. Forgetting a reference does nothing. - --> $DIR/forget_ref.rs:21:5 + --> $DIR/forget_ref.rs:22:5 | LL | forget(reference2); | ^^^^^^^^^^^^^^^^^^ | note: argument has type `&mut SomeStruct` - --> $DIR/forget_ref.rs:21:12 + --> $DIR/forget_ref.rs:22:12 | LL | forget(reference2); | ^^^^^^^^^^ error: calls to `std::mem::forget` with a reference instead of an owned value. Forgetting a reference does nothing. - --> $DIR/forget_ref.rs:24:5 + --> $DIR/forget_ref.rs:25:5 | LL | forget(reference3); | ^^^^^^^^^^^^^^^^^^ | note: argument has type `&SomeStruct` - --> $DIR/forget_ref.rs:24:12 + --> $DIR/forget_ref.rs:25:12 | LL | forget(reference3); | ^^^^^^^^^^ error: calls to `std::mem::forget` with a reference instead of an owned value. Forgetting a reference does nothing. - --> $DIR/forget_ref.rs:29:5 + --> $DIR/forget_ref.rs:30:5 | LL | forget(&val); | ^^^^^^^^^^^^ | note: argument has type `&T` - --> $DIR/forget_ref.rs:29:12 + --> $DIR/forget_ref.rs:30:12 | LL | forget(&val); | ^^^^ error: calls to `std::mem::forget` with a reference instead of an owned value. Forgetting a reference does nothing. - --> $DIR/forget_ref.rs:37:5 + --> $DIR/forget_ref.rs:38:5 | LL | std::mem::forget(&SomeStruct); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | note: argument has type `&SomeStruct` - --> $DIR/forget_ref.rs:37:22 + --> $DIR/forget_ref.rs:38:22 | LL | std::mem::forget(&SomeStruct); | ^^^^^^^^^^^ diff --git a/tests/ui/let_underscore_must_use.rs b/tests/ui/let_underscore_must_use.rs index 27dda606067a..e3800cda1b18 100644 --- a/tests/ui/let_underscore_must_use.rs +++ b/tests/ui/let_underscore_must_use.rs @@ -1,4 +1,5 @@ #![warn(clippy::let_underscore_must_use)] +#![allow(clippy::unnecessary_wrap)] // Debug implementations can fire this lint, // so we shouldn't lint external macros diff --git a/tests/ui/let_underscore_must_use.stderr b/tests/ui/let_underscore_must_use.stderr index 447f2419e3bd..5b751ea56def 100644 --- a/tests/ui/let_underscore_must_use.stderr +++ b/tests/ui/let_underscore_must_use.stderr @@ -1,5 +1,5 @@ error: non-binding let on a result of a `#[must_use]` function - --> $DIR/let_underscore_must_use.rs:66:5 + --> $DIR/let_underscore_must_use.rs:67:5 | LL | let _ = f(); | ^^^^^^^^^^^^ @@ -8,7 +8,7 @@ LL | let _ = f(); = help: consider explicitly using function result error: non-binding let on an expression with `#[must_use]` type - --> $DIR/let_underscore_must_use.rs:67:5 + --> $DIR/let_underscore_must_use.rs:68:5 | LL | let _ = g(); | ^^^^^^^^^^^^ @@ -16,7 +16,7 @@ LL | let _ = g(); = help: consider explicitly using expression value error: non-binding let on a result of a `#[must_use]` function - --> $DIR/let_underscore_must_use.rs:69:5 + --> $DIR/let_underscore_must_use.rs:70:5 | LL | let _ = l(0_u32); | ^^^^^^^^^^^^^^^^^ @@ -24,7 +24,7 @@ LL | let _ = l(0_u32); = help: consider explicitly using function result error: non-binding let on a result of a `#[must_use]` function - --> $DIR/let_underscore_must_use.rs:73:5 + --> $DIR/let_underscore_must_use.rs:74:5 | LL | let _ = s.f(); | ^^^^^^^^^^^^^^ @@ -32,7 +32,7 @@ LL | let _ = s.f(); = help: consider explicitly using function result error: non-binding let on an expression with `#[must_use]` type - --> $DIR/let_underscore_must_use.rs:74:5 + --> $DIR/let_underscore_must_use.rs:75:5 | LL | let _ = s.g(); | ^^^^^^^^^^^^^^ @@ -40,7 +40,7 @@ LL | let _ = s.g(); = help: consider explicitly using expression value error: non-binding let on a result of a `#[must_use]` function - --> $DIR/let_underscore_must_use.rs:77:5 + --> $DIR/let_underscore_must_use.rs:78:5 | LL | let _ = S::h(); | ^^^^^^^^^^^^^^^ @@ -48,7 +48,7 @@ LL | let _ = S::h(); = help: consider explicitly using function result error: non-binding let on an expression with `#[must_use]` type - --> $DIR/let_underscore_must_use.rs:78:5 + --> $DIR/let_underscore_must_use.rs:79:5 | LL | let _ = S::p(); | ^^^^^^^^^^^^^^^ @@ -56,7 +56,7 @@ LL | let _ = S::p(); = help: consider explicitly using expression value error: non-binding let on a result of a `#[must_use]` function - --> $DIR/let_underscore_must_use.rs:80:5 + --> $DIR/let_underscore_must_use.rs:81:5 | LL | let _ = S::a(); | ^^^^^^^^^^^^^^^ @@ -64,7 +64,7 @@ LL | let _ = S::a(); = help: consider explicitly using function result error: non-binding let on an expression with `#[must_use]` type - --> $DIR/let_underscore_must_use.rs:82:5 + --> $DIR/let_underscore_must_use.rs:83:5 | LL | let _ = if true { Ok(()) } else { Err(()) }; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -72,7 +72,7 @@ LL | let _ = if true { Ok(()) } else { Err(()) }; = help: consider explicitly using expression value error: non-binding let on a result of a `#[must_use]` function - --> $DIR/let_underscore_must_use.rs:86:5 + --> $DIR/let_underscore_must_use.rs:87:5 | LL | let _ = a.is_ok(); | ^^^^^^^^^^^^^^^^^^ @@ -80,7 +80,7 @@ LL | let _ = a.is_ok(); = help: consider explicitly using function result error: non-binding let on an expression with `#[must_use]` type - --> $DIR/let_underscore_must_use.rs:88:5 + --> $DIR/let_underscore_must_use.rs:89:5 | LL | let _ = a.map(|_| ()); | ^^^^^^^^^^^^^^^^^^^^^^ @@ -88,7 +88,7 @@ LL | let _ = a.map(|_| ()); = help: consider explicitly using expression value error: non-binding let on an expression with `#[must_use]` type - --> $DIR/let_underscore_must_use.rs:90:5 + --> $DIR/let_underscore_must_use.rs:91:5 | LL | let _ = a; | ^^^^^^^^^^ diff --git a/tests/ui/map_flatten.fixed b/tests/ui/map_flatten.fixed index a7ab5a12cb76..b4a51837b2fb 100644 --- a/tests/ui/map_flatten.fixed +++ b/tests/ui/map_flatten.fixed @@ -4,6 +4,7 @@ #![allow(clippy::let_underscore_drop)] #![allow(clippy::missing_docs_in_private_items)] #![allow(clippy::map_identity)] +#![allow(clippy::unnecessary_wrap)] fn main() { // mapping to Option on Iterator diff --git a/tests/ui/map_flatten.rs b/tests/ui/map_flatten.rs index e364a05f3760..e83cc46bda28 100644 --- a/tests/ui/map_flatten.rs +++ b/tests/ui/map_flatten.rs @@ -4,6 +4,7 @@ #![allow(clippy::let_underscore_drop)] #![allow(clippy::missing_docs_in_private_items)] #![allow(clippy::map_identity)] +#![allow(clippy::unnecessary_wrap)] fn main() { // mapping to Option on Iterator diff --git a/tests/ui/needless_lifetimes.rs b/tests/ui/needless_lifetimes.rs index 6001ef37eb78..e5973bbef8d4 100644 --- a/tests/ui/needless_lifetimes.rs +++ b/tests/ui/needless_lifetimes.rs @@ -1,5 +1,5 @@ #![warn(clippy::needless_lifetimes)] -#![allow(dead_code, clippy::needless_pass_by_value)] +#![allow(dead_code, clippy::needless_pass_by_value, clippy::unnecessary_wrap)] fn distinct_lifetimes<'a, 'b>(_x: &'a u8, _y: &'b u8, _z: u8) {} diff --git a/tests/ui/option_map_unit_fn_fixable.fixed b/tests/ui/option_map_unit_fn_fixable.fixed index 96d1c54946c0..de2e91559060 100644 --- a/tests/ui/option_map_unit_fn_fixable.fixed +++ b/tests/ui/option_map_unit_fn_fixable.fixed @@ -2,6 +2,7 @@ #![warn(clippy::option_map_unit_fn)] #![allow(unused)] +#![allow(clippy::unnecessary_wrap)] fn do_nothing(_: T) {} diff --git a/tests/ui/option_map_unit_fn_fixable.rs b/tests/ui/option_map_unit_fn_fixable.rs index 931ffc186659..f0887c8a4bce 100644 --- a/tests/ui/option_map_unit_fn_fixable.rs +++ b/tests/ui/option_map_unit_fn_fixable.rs @@ -2,6 +2,7 @@ #![warn(clippy::option_map_unit_fn)] #![allow(unused)] +#![allow(clippy::unnecessary_wrap)] fn do_nothing(_: T) {} diff --git a/tests/ui/option_map_unit_fn_fixable.stderr b/tests/ui/option_map_unit_fn_fixable.stderr index d7d45ef9b0b3..8abdbcafb6e9 100644 --- a/tests/ui/option_map_unit_fn_fixable.stderr +++ b/tests/ui/option_map_unit_fn_fixable.stderr @@ -1,5 +1,5 @@ error: called `map(f)` on an `Option` value where `f` is a function that returns the unit type `()` - --> $DIR/option_map_unit_fn_fixable.rs:38:5 + --> $DIR/option_map_unit_fn_fixable.rs:39:5 | LL | x.field.map(do_nothing); | ^^^^^^^^^^^^^^^^^^^^^^^- @@ -9,7 +9,7 @@ LL | x.field.map(do_nothing); = note: `-D clippy::option-map-unit-fn` implied by `-D warnings` error: called `map(f)` on an `Option` value where `f` is a function that returns the unit type `()` - --> $DIR/option_map_unit_fn_fixable.rs:40:5 + --> $DIR/option_map_unit_fn_fixable.rs:41:5 | LL | x.field.map(do_nothing); | ^^^^^^^^^^^^^^^^^^^^^^^- @@ -17,7 +17,7 @@ LL | x.field.map(do_nothing); | help: try this: `if let Some(x_field) = x.field { do_nothing(x_field) }` error: called `map(f)` on an `Option` value where `f` is a function that returns the unit type `()` - --> $DIR/option_map_unit_fn_fixable.rs:42:5 + --> $DIR/option_map_unit_fn_fixable.rs:43:5 | LL | x.field.map(diverge); | ^^^^^^^^^^^^^^^^^^^^- @@ -25,7 +25,7 @@ LL | x.field.map(diverge); | help: try this: `if let Some(x_field) = x.field { diverge(x_field) }` error: called `map(f)` on an `Option` value where `f` is a closure that returns the unit type `()` - --> $DIR/option_map_unit_fn_fixable.rs:48:5 + --> $DIR/option_map_unit_fn_fixable.rs:49:5 | LL | x.field.map(|value| x.do_option_nothing(value + captured)); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^- @@ -33,7 +33,7 @@ LL | x.field.map(|value| x.do_option_nothing(value + captured)); | help: try this: `if let Some(value) = x.field { x.do_option_nothing(value + captured) }` error: called `map(f)` on an `Option` value where `f` is a closure that returns the unit type `()` - --> $DIR/option_map_unit_fn_fixable.rs:50:5 + --> $DIR/option_map_unit_fn_fixable.rs:51:5 | LL | x.field.map(|value| { x.do_option_plus_one(value + captured); }); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^- @@ -41,7 +41,7 @@ LL | x.field.map(|value| { x.do_option_plus_one(value + captured); }); | help: try this: `if let Some(value) = x.field { x.do_option_plus_one(value + captured); }` error: called `map(f)` on an `Option` value where `f` is a closure that returns the unit type `()` - --> $DIR/option_map_unit_fn_fixable.rs:53:5 + --> $DIR/option_map_unit_fn_fixable.rs:54:5 | LL | x.field.map(|value| do_nothing(value + captured)); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^- @@ -49,7 +49,7 @@ LL | x.field.map(|value| do_nothing(value + captured)); | help: try this: `if let Some(value) = x.field { do_nothing(value + captured) }` error: called `map(f)` on an `Option` value where `f` is a closure that returns the unit type `()` - --> $DIR/option_map_unit_fn_fixable.rs:55:5 + --> $DIR/option_map_unit_fn_fixable.rs:56:5 | LL | x.field.map(|value| { do_nothing(value + captured) }); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^- @@ -57,7 +57,7 @@ LL | x.field.map(|value| { do_nothing(value + captured) }); | help: try this: `if let Some(value) = x.field { do_nothing(value + captured) }` error: called `map(f)` on an `Option` value where `f` is a closure that returns the unit type `()` - --> $DIR/option_map_unit_fn_fixable.rs:57:5 + --> $DIR/option_map_unit_fn_fixable.rs:58:5 | LL | x.field.map(|value| { do_nothing(value + captured); }); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^- @@ -65,7 +65,7 @@ LL | x.field.map(|value| { do_nothing(value + captured); }); | help: try this: `if let Some(value) = x.field { do_nothing(value + captured); }` error: called `map(f)` on an `Option` value where `f` is a closure that returns the unit type `()` - --> $DIR/option_map_unit_fn_fixable.rs:59:5 + --> $DIR/option_map_unit_fn_fixable.rs:60:5 | LL | x.field.map(|value| { { do_nothing(value + captured); } }); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^- @@ -73,7 +73,7 @@ LL | x.field.map(|value| { { do_nothing(value + captured); } }); | help: try this: `if let Some(value) = x.field { do_nothing(value + captured); }` error: called `map(f)` on an `Option` value where `f` is a closure that returns the unit type `()` - --> $DIR/option_map_unit_fn_fixable.rs:62:5 + --> $DIR/option_map_unit_fn_fixable.rs:63:5 | LL | x.field.map(|value| diverge(value + captured)); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^- @@ -81,7 +81,7 @@ LL | x.field.map(|value| diverge(value + captured)); | help: try this: `if let Some(value) = x.field { diverge(value + captured) }` error: called `map(f)` on an `Option` value where `f` is a closure that returns the unit type `()` - --> $DIR/option_map_unit_fn_fixable.rs:64:5 + --> $DIR/option_map_unit_fn_fixable.rs:65:5 | LL | x.field.map(|value| { diverge(value + captured) }); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^- @@ -89,7 +89,7 @@ LL | x.field.map(|value| { diverge(value + captured) }); | help: try this: `if let Some(value) = x.field { diverge(value + captured) }` error: called `map(f)` on an `Option` value where `f` is a closure that returns the unit type `()` - --> $DIR/option_map_unit_fn_fixable.rs:66:5 + --> $DIR/option_map_unit_fn_fixable.rs:67:5 | LL | x.field.map(|value| { diverge(value + captured); }); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^- @@ -97,7 +97,7 @@ LL | x.field.map(|value| { diverge(value + captured); }); | help: try this: `if let Some(value) = x.field { diverge(value + captured); }` error: called `map(f)` on an `Option` value where `f` is a closure that returns the unit type `()` - --> $DIR/option_map_unit_fn_fixable.rs:68:5 + --> $DIR/option_map_unit_fn_fixable.rs:69:5 | LL | x.field.map(|value| { { diverge(value + captured); } }); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^- @@ -105,7 +105,7 @@ LL | x.field.map(|value| { { diverge(value + captured); } }); | help: try this: `if let Some(value) = x.field { diverge(value + captured); }` error: called `map(f)` on an `Option` value where `f` is a closure that returns the unit type `()` - --> $DIR/option_map_unit_fn_fixable.rs:73:5 + --> $DIR/option_map_unit_fn_fixable.rs:74:5 | LL | x.field.map(|value| { let y = plus_one(value + captured); }); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^- @@ -113,7 +113,7 @@ LL | x.field.map(|value| { let y = plus_one(value + captured); }); | help: try this: `if let Some(value) = x.field { let y = plus_one(value + captured); }` error: called `map(f)` on an `Option` value where `f` is a closure that returns the unit type `()` - --> $DIR/option_map_unit_fn_fixable.rs:75:5 + --> $DIR/option_map_unit_fn_fixable.rs:76:5 | LL | x.field.map(|value| { plus_one(value + captured); }); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^- @@ -121,7 +121,7 @@ LL | x.field.map(|value| { plus_one(value + captured); }); | help: try this: `if let Some(value) = x.field { plus_one(value + captured); }` error: called `map(f)` on an `Option` value where `f` is a closure that returns the unit type `()` - --> $DIR/option_map_unit_fn_fixable.rs:77:5 + --> $DIR/option_map_unit_fn_fixable.rs:78:5 | LL | x.field.map(|value| { { plus_one(value + captured); } }); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^- @@ -129,7 +129,7 @@ LL | x.field.map(|value| { { plus_one(value + captured); } }); | help: try this: `if let Some(value) = x.field { plus_one(value + captured); }` error: called `map(f)` on an `Option` value where `f` is a closure that returns the unit type `()` - --> $DIR/option_map_unit_fn_fixable.rs:80:5 + --> $DIR/option_map_unit_fn_fixable.rs:81:5 | LL | x.field.map(|ref value| { do_nothing(value + captured) }); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^- @@ -137,7 +137,7 @@ LL | x.field.map(|ref value| { do_nothing(value + captured) }); | help: try this: `if let Some(ref value) = x.field { do_nothing(value + captured) }` error: called `map(f)` on an `Option` value where `f` is a function that returns the unit type `()` - --> $DIR/option_map_unit_fn_fixable.rs:82:5 + --> $DIR/option_map_unit_fn_fixable.rs:83:5 | LL | option().map(do_nothing);} | ^^^^^^^^^^^^^^^^^^^^^^^^- diff --git a/tests/ui/option_option.rs b/tests/ui/option_option.rs index 243448336415..557d29dff67a 100644 --- a/tests/ui/option_option.rs +++ b/tests/ui/option_option.rs @@ -1,4 +1,5 @@ #![deny(clippy::option_option)] +#![allow(clippy::unnecessary_wrap)] fn input(_: Option>) {} diff --git a/tests/ui/option_option.stderr b/tests/ui/option_option.stderr index 8ae1d23a8e33..8ace8338fcf0 100644 --- a/tests/ui/option_option.stderr +++ b/tests/ui/option_option.stderr @@ -1,5 +1,5 @@ error: consider using `Option` instead of `Option>` or a custom enum if you need to distinguish all 3 cases - --> $DIR/option_option.rs:3:13 + --> $DIR/option_option.rs:4:13 | LL | fn input(_: Option>) {} | ^^^^^^^^^^^^^^^^^^ @@ -11,55 +11,55 @@ LL | #![deny(clippy::option_option)] | ^^^^^^^^^^^^^^^^^^^^^ error: consider using `Option` instead of `Option>` or a custom enum if you need to distinguish all 3 cases - --> $DIR/option_option.rs:5:16 + --> $DIR/option_option.rs:6:16 | LL | fn output() -> Option> { | ^^^^^^^^^^^^^^^^^^ error: consider using `Option` instead of `Option>` or a custom enum if you need to distinguish all 3 cases - --> $DIR/option_option.rs:9:27 + --> $DIR/option_option.rs:10:27 | LL | fn output_nested() -> Vec>> { | ^^^^^^^^^^^^^^^^^^ error: consider using `Option` instead of `Option>` or a custom enum if you need to distinguish all 3 cases - --> $DIR/option_option.rs:14:30 + --> $DIR/option_option.rs:15:30 | LL | fn output_nested_nested() -> Option>> { | ^^^^^^^^^^^^^^^^^^^^^^^^^^ error: consider using `Option` instead of `Option>` or a custom enum if you need to distinguish all 3 cases - --> $DIR/option_option.rs:19:8 + --> $DIR/option_option.rs:20:8 | LL | x: Option>, | ^^^^^^^^^^^^^^^^^^ error: consider using `Option` instead of `Option>` or a custom enum if you need to distinguish all 3 cases - --> $DIR/option_option.rs:23:23 + --> $DIR/option_option.rs:24:23 | LL | fn struct_fn() -> Option> { | ^^^^^^^^^^^^^^^^^^ error: consider using `Option` instead of `Option>` or a custom enum if you need to distinguish all 3 cases - --> $DIR/option_option.rs:29:22 + --> $DIR/option_option.rs:30:22 | LL | fn trait_fn() -> Option>; | ^^^^^^^^^^^^^^^^^^ error: consider using `Option` instead of `Option>` or a custom enum if you need to distinguish all 3 cases - --> $DIR/option_option.rs:33:11 + --> $DIR/option_option.rs:34:11 | LL | Tuple(Option>), | ^^^^^^^^^^^^^^^^^^ error: consider using `Option` instead of `Option>` or a custom enum if you need to distinguish all 3 cases - --> $DIR/option_option.rs:34:17 + --> $DIR/option_option.rs:35:17 | LL | Struct { x: Option> }, | ^^^^^^^^^^^^^^^^^^ error: consider using `Option` instead of `Option>` or a custom enum if you need to distinguish all 3 cases - --> $DIR/option_option.rs:75:14 + --> $DIR/option_option.rs:78:14 | LL | foo: Option>>, | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ diff --git a/tests/ui/or_fun_call.fixed b/tests/ui/or_fun_call.fixed index 20e5016bc17a..e1735bc88f57 100644 --- a/tests/ui/or_fun_call.fixed +++ b/tests/ui/or_fun_call.fixed @@ -2,6 +2,7 @@ #![warn(clippy::or_fun_call)] #![allow(dead_code)] +#![allow(clippy::unnecessary_wrap)] use std::collections::BTreeMap; use std::collections::HashMap; diff --git a/tests/ui/or_fun_call.rs b/tests/ui/or_fun_call.rs index e7192deeebb3..a6abd2e8b349 100644 --- a/tests/ui/or_fun_call.rs +++ b/tests/ui/or_fun_call.rs @@ -2,6 +2,7 @@ #![warn(clippy::or_fun_call)] #![allow(dead_code)] +#![allow(clippy::unnecessary_wrap)] use std::collections::BTreeMap; use std::collections::HashMap; diff --git a/tests/ui/or_fun_call.stderr b/tests/ui/or_fun_call.stderr index d0c4df0e0081..8a7b5574c84e 100644 --- a/tests/ui/or_fun_call.stderr +++ b/tests/ui/or_fun_call.stderr @@ -1,5 +1,5 @@ error: use of `unwrap_or` followed by a function call - --> $DIR/or_fun_call.rs:32:19 + --> $DIR/or_fun_call.rs:33:19 | LL | with_const_fn.unwrap_or(Duration::from_secs(5)); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `unwrap_or_else(|| Duration::from_secs(5))` @@ -7,55 +7,55 @@ LL | with_const_fn.unwrap_or(Duration::from_secs(5)); = note: `-D clippy::or-fun-call` implied by `-D warnings` error: use of `unwrap_or` followed by a function call - --> $DIR/or_fun_call.rs:35:22 + --> $DIR/or_fun_call.rs:36:22 | LL | with_constructor.unwrap_or(make()); | ^^^^^^^^^^^^^^^^^ help: try this: `unwrap_or_else(make)` error: use of `unwrap_or` followed by a call to `new` - --> $DIR/or_fun_call.rs:38:5 + --> $DIR/or_fun_call.rs:39:5 | LL | with_new.unwrap_or(Vec::new()); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `with_new.unwrap_or_default()` error: use of `unwrap_or` followed by a function call - --> $DIR/or_fun_call.rs:41:21 + --> $DIR/or_fun_call.rs:42:21 | LL | with_const_args.unwrap_or(Vec::with_capacity(12)); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `unwrap_or_else(|| Vec::with_capacity(12))` error: use of `unwrap_or` followed by a function call - --> $DIR/or_fun_call.rs:44:14 + --> $DIR/or_fun_call.rs:45:14 | LL | with_err.unwrap_or(make()); | ^^^^^^^^^^^^^^^^^ help: try this: `unwrap_or_else(|_| make())` error: use of `unwrap_or` followed by a function call - --> $DIR/or_fun_call.rs:47:19 + --> $DIR/or_fun_call.rs:48:19 | LL | with_err_args.unwrap_or(Vec::with_capacity(12)); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `unwrap_or_else(|_| Vec::with_capacity(12))` error: use of `unwrap_or` followed by a call to `default` - --> $DIR/or_fun_call.rs:50:5 + --> $DIR/or_fun_call.rs:51:5 | LL | with_default_trait.unwrap_or(Default::default()); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `with_default_trait.unwrap_or_default()` error: use of `unwrap_or` followed by a call to `default` - --> $DIR/or_fun_call.rs:53:5 + --> $DIR/or_fun_call.rs:54:5 | LL | with_default_type.unwrap_or(u64::default()); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `with_default_type.unwrap_or_default()` error: use of `unwrap_or` followed by a call to `new` - --> $DIR/or_fun_call.rs:56:5 + --> $DIR/or_fun_call.rs:57:5 | LL | with_vec.unwrap_or(vec![]); | ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `with_vec.unwrap_or_default()` error: use of `unwrap_or` followed by a function call - --> $DIR/or_fun_call.rs:59:21 + --> $DIR/or_fun_call.rs:60:21 | LL | without_default.unwrap_or(Foo::new()); | ^^^^^^^^^^^^^^^^^^^^^ help: try this: `unwrap_or_else(Foo::new)` diff --git a/tests/ui/panic_in_result_fn.rs b/tests/ui/panic_in_result_fn.rs index 287726f7a2d4..be4e85d05a72 100644 --- a/tests/ui/panic_in_result_fn.rs +++ b/tests/ui/panic_in_result_fn.rs @@ -1,4 +1,5 @@ #![warn(clippy::panic_in_result_fn)] +#![allow(clippy::unnecessary_wrap)] struct A; diff --git a/tests/ui/panic_in_result_fn.stderr b/tests/ui/panic_in_result_fn.stderr index c6936fd86923..ca73ac5a4111 100644 --- a/tests/ui/panic_in_result_fn.stderr +++ b/tests/ui/panic_in_result_fn.stderr @@ -1,5 +1,5 @@ error: used `unimplemented!()`, `unreachable!()`, `todo!()` or `panic!()` in a function that returns `Result` - --> $DIR/panic_in_result_fn.rs:6:5 + --> $DIR/panic_in_result_fn.rs:7:5 | LL | / fn result_with_panic() -> Result // should emit lint LL | | { @@ -10,14 +10,14 @@ LL | | } = note: `-D clippy::panic-in-result-fn` implied by `-D warnings` = help: `unimplemented!()`, `unreachable!()`, `todo!()` or `panic!()` should not be used in a function that returns `Result` as `Result` is expected to return an error instead of crashing note: return Err() instead of panicking - --> $DIR/panic_in_result_fn.rs:8:9 + --> $DIR/panic_in_result_fn.rs:9:9 | LL | panic!("error"); | ^^^^^^^^^^^^^^^^ = note: this error originates in a macro (in Nightly builds, run with -Z macro-backtrace for more info) error: used `unimplemented!()`, `unreachable!()`, `todo!()` or `panic!()` in a function that returns `Result` - --> $DIR/panic_in_result_fn.rs:11:5 + --> $DIR/panic_in_result_fn.rs:12:5 | LL | / fn result_with_unimplemented() -> Result // should emit lint LL | | { @@ -27,14 +27,14 @@ LL | | } | = help: `unimplemented!()`, `unreachable!()`, `todo!()` or `panic!()` should not be used in a function that returns `Result` as `Result` is expected to return an error instead of crashing note: return Err() instead of panicking - --> $DIR/panic_in_result_fn.rs:13:9 + --> $DIR/panic_in_result_fn.rs:14:9 | LL | unimplemented!(); | ^^^^^^^^^^^^^^^^^ = note: this error originates in a macro (in Nightly builds, run with -Z macro-backtrace for more info) error: used `unimplemented!()`, `unreachable!()`, `todo!()` or `panic!()` in a function that returns `Result` - --> $DIR/panic_in_result_fn.rs:16:5 + --> $DIR/panic_in_result_fn.rs:17:5 | LL | / fn result_with_unreachable() -> Result // should emit lint LL | | { @@ -44,14 +44,14 @@ LL | | } | = help: `unimplemented!()`, `unreachable!()`, `todo!()` or `panic!()` should not be used in a function that returns `Result` as `Result` is expected to return an error instead of crashing note: return Err() instead of panicking - --> $DIR/panic_in_result_fn.rs:18:9 + --> $DIR/panic_in_result_fn.rs:19:9 | LL | unreachable!(); | ^^^^^^^^^^^^^^^ = note: this error originates in a macro (in Nightly builds, run with -Z macro-backtrace for more info) error: used `unimplemented!()`, `unreachable!()`, `todo!()` or `panic!()` in a function that returns `Result` - --> $DIR/panic_in_result_fn.rs:21:5 + --> $DIR/panic_in_result_fn.rs:22:5 | LL | / fn result_with_todo() -> Result // should emit lint LL | | { @@ -61,14 +61,14 @@ LL | | } | = help: `unimplemented!()`, `unreachable!()`, `todo!()` or `panic!()` should not be used in a function that returns `Result` as `Result` is expected to return an error instead of crashing note: return Err() instead of panicking - --> $DIR/panic_in_result_fn.rs:23:9 + --> $DIR/panic_in_result_fn.rs:24:9 | LL | todo!("Finish this"); | ^^^^^^^^^^^^^^^^^^^^^ = note: this error originates in a macro (in Nightly builds, run with -Z macro-backtrace for more info) error: used `unimplemented!()`, `unreachable!()`, `todo!()` or `panic!()` in a function that returns `Result` - --> $DIR/panic_in_result_fn.rs:52:1 + --> $DIR/panic_in_result_fn.rs:53:1 | LL | / fn function_result_with_panic() -> Result // should emit lint LL | | { @@ -78,14 +78,14 @@ LL | | } | = help: `unimplemented!()`, `unreachable!()`, `todo!()` or `panic!()` should not be used in a function that returns `Result` as `Result` is expected to return an error instead of crashing note: return Err() instead of panicking - --> $DIR/panic_in_result_fn.rs:54:5 + --> $DIR/panic_in_result_fn.rs:55:5 | LL | panic!("error"); | ^^^^^^^^^^^^^^^^ = note: this error originates in a macro (in Nightly builds, run with -Z macro-backtrace for more info) error: used `unimplemented!()`, `unreachable!()`, `todo!()` or `panic!()` in a function that returns `Result` - --> $DIR/panic_in_result_fn.rs:67:1 + --> $DIR/panic_in_result_fn.rs:68:1 | LL | / fn main() -> Result<(), String> { LL | | todo!("finish main method"); @@ -95,7 +95,7 @@ LL | | } | = help: `unimplemented!()`, `unreachable!()`, `todo!()` or `panic!()` should not be used in a function that returns `Result` as `Result` is expected to return an error instead of crashing note: return Err() instead of panicking - --> $DIR/panic_in_result_fn.rs:68:5 + --> $DIR/panic_in_result_fn.rs:69:5 | LL | todo!("finish main method"); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ diff --git a/tests/ui/question_mark.fixed b/tests/ui/question_mark.fixed index 11dff94a2886..df3c98a0aa57 100644 --- a/tests/ui/question_mark.fixed +++ b/tests/ui/question_mark.fixed @@ -1,5 +1,6 @@ // run-rustfix #![allow(unreachable_code)] +#![allow(clippy::unnecessary_wrap)] fn some_func(a: Option) -> Option { a?; diff --git a/tests/ui/question_mark.rs b/tests/ui/question_mark.rs index 1d0ee82b4f77..62b3e96b65a8 100644 --- a/tests/ui/question_mark.rs +++ b/tests/ui/question_mark.rs @@ -1,5 +1,6 @@ // run-rustfix #![allow(unreachable_code)] +#![allow(clippy::unnecessary_wrap)] fn some_func(a: Option) -> Option { if a.is_none() { diff --git a/tests/ui/question_mark.stderr b/tests/ui/question_mark.stderr index 502615fb175a..6f330cfa385d 100644 --- a/tests/ui/question_mark.stderr +++ b/tests/ui/question_mark.stderr @@ -1,5 +1,5 @@ error: this block may be rewritten with the `?` operator - --> $DIR/question_mark.rs:5:5 + --> $DIR/question_mark.rs:6:5 | LL | / if a.is_none() { LL | | return None; @@ -9,7 +9,7 @@ LL | | } = note: `-D clippy::question-mark` implied by `-D warnings` error: this block may be rewritten with the `?` operator - --> $DIR/question_mark.rs:50:9 + --> $DIR/question_mark.rs:51:9 | LL | / if (self.opt).is_none() { LL | | return None; @@ -17,7 +17,7 @@ LL | | } | |_________^ help: replace it with: `(self.opt)?;` error: this block may be rewritten with the `?` operator - --> $DIR/question_mark.rs:54:9 + --> $DIR/question_mark.rs:55:9 | LL | / if self.opt.is_none() { LL | | return None @@ -25,7 +25,7 @@ LL | | } | |_________^ help: replace it with: `self.opt?;` error: this block may be rewritten with the `?` operator - --> $DIR/question_mark.rs:58:17 + --> $DIR/question_mark.rs:59:17 | LL | let _ = if self.opt.is_none() { | _________________^ @@ -36,7 +36,7 @@ LL | | }; | |_________^ help: replace it with: `Some(self.opt?)` error: this if-let-else may be rewritten with the `?` operator - --> $DIR/question_mark.rs:64:17 + --> $DIR/question_mark.rs:65:17 | LL | let _ = if let Some(x) = self.opt { | _________________^ @@ -47,7 +47,7 @@ LL | | }; | |_________^ help: replace it with: `self.opt?` error: this block may be rewritten with the `?` operator - --> $DIR/question_mark.rs:81:9 + --> $DIR/question_mark.rs:82:9 | LL | / if self.opt.is_none() { LL | | return None; @@ -55,7 +55,7 @@ LL | | } | |_________^ help: replace it with: `self.opt.as_ref()?;` error: this block may be rewritten with the `?` operator - --> $DIR/question_mark.rs:89:9 + --> $DIR/question_mark.rs:90:9 | LL | / if self.opt.is_none() { LL | | return None; @@ -63,7 +63,7 @@ LL | | } | |_________^ help: replace it with: `self.opt.as_ref()?;` error: this block may be rewritten with the `?` operator - --> $DIR/question_mark.rs:97:9 + --> $DIR/question_mark.rs:98:9 | LL | / if self.opt.is_none() { LL | | return None; @@ -71,7 +71,7 @@ LL | | } | |_________^ help: replace it with: `self.opt.as_ref()?;` error: this if-let-else may be rewritten with the `?` operator - --> $DIR/question_mark.rs:104:26 + --> $DIR/question_mark.rs:105:26 | LL | let v: &Vec<_> = if let Some(ref v) = self.opt { | __________________________^ @@ -82,7 +82,7 @@ LL | | }; | |_________^ help: replace it with: `self.opt.as_ref()?` error: this if-let-else may be rewritten with the `?` operator - --> $DIR/question_mark.rs:114:17 + --> $DIR/question_mark.rs:115:17 | LL | let v = if let Some(v) = self.opt { | _________________^ @@ -93,7 +93,7 @@ LL | | }; | |_________^ help: replace it with: `self.opt?` error: this block may be rewritten with the `?` operator - --> $DIR/question_mark.rs:129:5 + --> $DIR/question_mark.rs:130:5 | LL | / if f().is_none() { LL | | return None; diff --git a/tests/ui/redundant_pattern_matching.fixed b/tests/ui/redundant_pattern_matching.fixed index fe8f62503b76..2e7ba8bc04d8 100644 --- a/tests/ui/redundant_pattern_matching.fixed +++ b/tests/ui/redundant_pattern_matching.fixed @@ -7,6 +7,7 @@ unused_must_use, clippy::needless_bool, clippy::match_like_matches_macro, + clippy::unnecessary_wrap, deprecated )] diff --git a/tests/ui/redundant_pattern_matching.rs b/tests/ui/redundant_pattern_matching.rs index 09426a6e5908..ea0e18f5970a 100644 --- a/tests/ui/redundant_pattern_matching.rs +++ b/tests/ui/redundant_pattern_matching.rs @@ -7,6 +7,7 @@ unused_must_use, clippy::needless_bool, clippy::match_like_matches_macro, + clippy::unnecessary_wrap, deprecated )] diff --git a/tests/ui/redundant_pattern_matching.stderr b/tests/ui/redundant_pattern_matching.stderr index 3473ceea00e2..5341e81836a4 100644 --- a/tests/ui/redundant_pattern_matching.stderr +++ b/tests/ui/redundant_pattern_matching.stderr @@ -1,5 +1,5 @@ error: redundant pattern matching, consider using `is_ok()` - --> $DIR/redundant_pattern_matching.rs:15:12 + --> $DIR/redundant_pattern_matching.rs:16:12 | LL | if let Ok(_) = &result {} | -------^^^^^---------- help: try this: `if result.is_ok()` @@ -7,13 +7,13 @@ LL | if let Ok(_) = &result {} = note: `-D clippy::redundant-pattern-matching` implied by `-D warnings` error: redundant pattern matching, consider using `is_ok()` - --> $DIR/redundant_pattern_matching.rs:17:12 + --> $DIR/redundant_pattern_matching.rs:18:12 | LL | if let Ok(_) = Ok::(42) {} | -------^^^^^--------------------- help: try this: `if Ok::(42).is_ok()` error: redundant pattern matching, consider using `is_err()` - --> $DIR/redundant_pattern_matching.rs:19:12 + --> $DIR/redundant_pattern_matching.rs:20:12 | LL | if let Err(_) = Err::(42) {} | -------^^^^^^---------------------- help: try this: `if Err::(42).is_err()` diff --git a/tests/ui/try_err.fixed b/tests/ui/try_err.fixed index aa43e69f79e8..19458ddf68ef 100644 --- a/tests/ui/try_err.fixed +++ b/tests/ui/try_err.fixed @@ -2,6 +2,7 @@ // aux-build:macro_rules.rs #![deny(clippy::try_err)] +#![allow(clippy::unnecessary_wrap)] #[macro_use] extern crate macro_rules; diff --git a/tests/ui/try_err.rs b/tests/ui/try_err.rs index df3a9dc5367f..ab6a32dbe4d0 100644 --- a/tests/ui/try_err.rs +++ b/tests/ui/try_err.rs @@ -2,6 +2,7 @@ // aux-build:macro_rules.rs #![deny(clippy::try_err)] +#![allow(clippy::unnecessary_wrap)] #[macro_use] extern crate macro_rules; diff --git a/tests/ui/try_err.stderr b/tests/ui/try_err.stderr index 3905ed2476b0..ababa64e6d84 100644 --- a/tests/ui/try_err.stderr +++ b/tests/ui/try_err.stderr @@ -1,5 +1,5 @@ error: returning an `Err(_)` with the `?` operator - --> $DIR/try_err.rs:18:9 + --> $DIR/try_err.rs:19:9 | LL | Err(err)?; | ^^^^^^^^^ help: try this: `return Err(err)` @@ -11,19 +11,19 @@ LL | #![deny(clippy::try_err)] | ^^^^^^^^^^^^^^^ error: returning an `Err(_)` with the `?` operator - --> $DIR/try_err.rs:28:9 + --> $DIR/try_err.rs:29:9 | LL | Err(err)?; | ^^^^^^^^^ help: try this: `return Err(err.into())` error: returning an `Err(_)` with the `?` operator - --> $DIR/try_err.rs:48:17 + --> $DIR/try_err.rs:49:17 | LL | Err(err)?; | ^^^^^^^^^ help: try this: `return Err(err)` error: returning an `Err(_)` with the `?` operator - --> $DIR/try_err.rs:67:17 + --> $DIR/try_err.rs:68:17 | LL | Err(err)?; | ^^^^^^^^^ help: try this: `return Err(err.into())` diff --git a/tests/ui/unit_arg.rs b/tests/ui/unit_arg.rs index fec115ff29d6..6bf704c09e0e 100644 --- a/tests/ui/unit_arg.rs +++ b/tests/ui/unit_arg.rs @@ -4,6 +4,7 @@ unused_must_use, unused_variables, clippy::unused_unit, + clippy::unnecessary_wrap, clippy::or_fun_call )] diff --git a/tests/ui/unit_arg.stderr b/tests/ui/unit_arg.stderr index 90fee3aab23b..c3a839a9bf81 100644 --- a/tests/ui/unit_arg.stderr +++ b/tests/ui/unit_arg.stderr @@ -1,5 +1,5 @@ error: passing a unit value to a function - --> $DIR/unit_arg.rs:29:5 + --> $DIR/unit_arg.rs:30:5 | LL | / foo({ LL | | 1; @@ -20,7 +20,7 @@ LL | foo(()); | error: passing a unit value to a function - --> $DIR/unit_arg.rs:32:5 + --> $DIR/unit_arg.rs:33:5 | LL | foo(foo(1)); | ^^^^^^^^^^^ @@ -32,7 +32,7 @@ LL | foo(()); | error: passing a unit value to a function - --> $DIR/unit_arg.rs:33:5 + --> $DIR/unit_arg.rs:34:5 | LL | / foo({ LL | | foo(1); @@ -54,7 +54,7 @@ LL | foo(()); | error: passing a unit value to a function - --> $DIR/unit_arg.rs:38:5 + --> $DIR/unit_arg.rs:39:5 | LL | / b.bar({ LL | | 1; @@ -74,7 +74,7 @@ LL | b.bar(()); | error: passing unit values to a function - --> $DIR/unit_arg.rs:41:5 + --> $DIR/unit_arg.rs:42:5 | LL | taking_multiple_units(foo(0), foo(1)); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -87,7 +87,7 @@ LL | taking_multiple_units((), ()); | error: passing unit values to a function - --> $DIR/unit_arg.rs:42:5 + --> $DIR/unit_arg.rs:43:5 | LL | / taking_multiple_units(foo(0), { LL | | foo(1); @@ -110,7 +110,7 @@ LL | taking_multiple_units((), ()); | error: passing unit values to a function - --> $DIR/unit_arg.rs:46:5 + --> $DIR/unit_arg.rs:47:5 | LL | / taking_multiple_units( LL | | { @@ -140,7 +140,7 @@ LL | foo(2); ... error: passing a unit value to a function - --> $DIR/unit_arg.rs:57:13 + --> $DIR/unit_arg.rs:58:13 | LL | None.or(Some(foo(2))); | ^^^^^^^^^^^^ @@ -154,7 +154,7 @@ LL | }); | error: passing a unit value to a function - --> $DIR/unit_arg.rs:60:5 + --> $DIR/unit_arg.rs:61:5 | LL | foo(foo(())) | ^^^^^^^^^^^^ @@ -166,7 +166,7 @@ LL | foo(()) | error: passing a unit value to a function - --> $DIR/unit_arg.rs:93:5 + --> $DIR/unit_arg.rs:94:5 | LL | Some(foo(1)) | ^^^^^^^^^^^^ diff --git a/tests/ui/unnecessary_clone.rs b/tests/ui/unnecessary_clone.rs index e785ac02feb3..3b1a6cf57c65 100644 --- a/tests/ui/unnecessary_clone.rs +++ b/tests/ui/unnecessary_clone.rs @@ -1,7 +1,7 @@ // does not test any rustfixable lints #![warn(clippy::clone_on_ref_ptr)] -#![allow(unused, clippy::redundant_clone)] +#![allow(unused, clippy::redundant_clone, clippy::unnecessary_wrap)] use std::cell::RefCell; use std::rc::{self, Rc}; diff --git a/tests/ui/useless_conversion.fixed b/tests/ui/useless_conversion.fixed index 8a9b0cd3cf01..45ee367d649b 100644 --- a/tests/ui/useless_conversion.fixed +++ b/tests/ui/useless_conversion.fixed @@ -1,6 +1,7 @@ // run-rustfix #![deny(clippy::useless_conversion)] +#![allow(clippy::unnecessary_wrap)] fn test_generic(val: T) -> T { let _ = val; diff --git a/tests/ui/useless_conversion.rs b/tests/ui/useless_conversion.rs index 4faa1572973b..e5bdffed20d9 100644 --- a/tests/ui/useless_conversion.rs +++ b/tests/ui/useless_conversion.rs @@ -1,6 +1,7 @@ // run-rustfix #![deny(clippy::useless_conversion)] +#![allow(clippy::unnecessary_wrap)] fn test_generic(val: T) -> T { let _ = T::from(val); diff --git a/tests/ui/useless_conversion.stderr b/tests/ui/useless_conversion.stderr index 11c6efb25cce..26a33595031b 100644 --- a/tests/ui/useless_conversion.stderr +++ b/tests/ui/useless_conversion.stderr @@ -1,5 +1,5 @@ error: useless conversion to the same type: `T` - --> $DIR/useless_conversion.rs:6:13 + --> $DIR/useless_conversion.rs:7:13 | LL | let _ = T::from(val); | ^^^^^^^^^^^^ help: consider removing `T::from()`: `val` @@ -11,61 +11,61 @@ LL | #![deny(clippy::useless_conversion)] | ^^^^^^^^^^^^^^^^^^^^^^^^^^ error: useless conversion to the same type: `T` - --> $DIR/useless_conversion.rs:7:5 + --> $DIR/useless_conversion.rs:8:5 | LL | val.into() | ^^^^^^^^^^ help: consider removing `.into()`: `val` error: useless conversion to the same type: `i32` - --> $DIR/useless_conversion.rs:19:22 + --> $DIR/useless_conversion.rs:20:22 | LL | let _: i32 = 0i32.into(); | ^^^^^^^^^^^ help: consider removing `.into()`: `0i32` error: useless conversion to the same type: `std::string::String` - --> $DIR/useless_conversion.rs:60:21 + --> $DIR/useless_conversion.rs:61:21 | LL | let _: String = "foo".to_string().into(); | ^^^^^^^^^^^^^^^^^^^^^^^^ help: consider removing `.into()`: `"foo".to_string()` error: useless conversion to the same type: `std::string::String` - --> $DIR/useless_conversion.rs:61:21 + --> $DIR/useless_conversion.rs:62:21 | LL | let _: String = From::from("foo".to_string()); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider removing `From::from()`: `"foo".to_string()` error: useless conversion to the same type: `std::string::String` - --> $DIR/useless_conversion.rs:62:13 + --> $DIR/useless_conversion.rs:63:13 | LL | let _ = String::from("foo".to_string()); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider removing `String::from()`: `"foo".to_string()` error: useless conversion to the same type: `std::string::String` - --> $DIR/useless_conversion.rs:63:13 + --> $DIR/useless_conversion.rs:64:13 | LL | let _ = String::from(format!("A: {:04}", 123)); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider removing `String::from()`: `format!("A: {:04}", 123)` error: useless conversion to the same type: `std::str::Lines` - --> $DIR/useless_conversion.rs:64:13 + --> $DIR/useless_conversion.rs:65:13 | LL | let _ = "".lines().into_iter(); | ^^^^^^^^^^^^^^^^^^^^^^ help: consider removing `.into_iter()`: `"".lines()` error: useless conversion to the same type: `std::vec::IntoIter` - --> $DIR/useless_conversion.rs:65:13 + --> $DIR/useless_conversion.rs:66:13 | LL | let _ = vec![1, 2, 3].into_iter().into_iter(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider removing `.into_iter()`: `vec![1, 2, 3].into_iter()` error: useless conversion to the same type: `std::string::String` - --> $DIR/useless_conversion.rs:66:21 + --> $DIR/useless_conversion.rs:67:21 | LL | let _: String = format!("Hello {}", "world").into(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider removing `.into()`: `format!("Hello {}", "world")` error: useless conversion to the same type: `i32` - --> $DIR/useless_conversion.rs:71:13 + --> $DIR/useless_conversion.rs:72:13 | LL | let _ = i32::from(a + b) * 3; | ^^^^^^^^^^^^^^^^ help: consider removing `i32::from()`: `(a + b)` diff --git a/tests/ui/wildcard_imports.fixed b/tests/ui/wildcard_imports.fixed index 287f8935327c..14007e5f251d 100644 --- a/tests/ui/wildcard_imports.fixed +++ b/tests/ui/wildcard_imports.fixed @@ -4,6 +4,7 @@ #![warn(clippy::wildcard_imports)] //#![allow(clippy::redundant_pub_crate)] #![allow(unused)] +#![allow(clippy::unnecessary_wrap)] #![warn(unused_imports)] extern crate wildcard_imports_helper; diff --git a/tests/ui/wildcard_imports.rs b/tests/ui/wildcard_imports.rs index 1f261159f4a9..0e8631ca7047 100644 --- a/tests/ui/wildcard_imports.rs +++ b/tests/ui/wildcard_imports.rs @@ -4,6 +4,7 @@ #![warn(clippy::wildcard_imports)] //#![allow(clippy::redundant_pub_crate)] #![allow(unused)] +#![allow(clippy::unnecessary_wrap)] #![warn(unused_imports)] extern crate wildcard_imports_helper; diff --git a/tests/ui/wildcard_imports.stderr b/tests/ui/wildcard_imports.stderr index 351988f31ead..66267dd27b84 100644 --- a/tests/ui/wildcard_imports.stderr +++ b/tests/ui/wildcard_imports.stderr @@ -1,5 +1,5 @@ error: usage of wildcard import - --> $DIR/wildcard_imports.rs:11:5 + --> $DIR/wildcard_imports.rs:12:5 | LL | use crate::fn_mod::*; | ^^^^^^^^^^^^^^^^ help: try: `crate::fn_mod::foo` @@ -7,85 +7,85 @@ LL | use crate::fn_mod::*; = note: `-D clippy::wildcard-imports` implied by `-D warnings` error: usage of wildcard import - --> $DIR/wildcard_imports.rs:12:5 + --> $DIR/wildcard_imports.rs:13:5 | LL | use crate::mod_mod::*; | ^^^^^^^^^^^^^^^^^ help: try: `crate::mod_mod::inner_mod` error: usage of wildcard import - --> $DIR/wildcard_imports.rs:13:5 + --> $DIR/wildcard_imports.rs:14:5 | LL | use crate::multi_fn_mod::*; | ^^^^^^^^^^^^^^^^^^^^^^ help: try: `crate::multi_fn_mod::{multi_bar, multi_foo, multi_inner_mod}` error: usage of wildcard import - --> $DIR/wildcard_imports.rs:15:5 + --> $DIR/wildcard_imports.rs:16:5 | LL | use crate::struct_mod::*; | ^^^^^^^^^^^^^^^^^^^^ help: try: `crate::struct_mod::{A, inner_struct_mod}` error: usage of wildcard import - --> $DIR/wildcard_imports.rs:19:5 + --> $DIR/wildcard_imports.rs:20:5 | LL | use wildcard_imports_helper::inner::inner_for_self_import::*; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `wildcard_imports_helper::inner::inner_for_self_import::inner_extern_bar` error: usage of wildcard import - --> $DIR/wildcard_imports.rs:20:5 + --> $DIR/wildcard_imports.rs:21:5 | LL | use wildcard_imports_helper::*; | ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `wildcard_imports_helper::{ExternA, extern_foo}` error: usage of wildcard import - --> $DIR/wildcard_imports.rs:91:13 + --> $DIR/wildcard_imports.rs:92:13 | LL | use crate::fn_mod::*; | ^^^^^^^^^^^^^^^^ help: try: `crate::fn_mod::foo` error: usage of wildcard import - --> $DIR/wildcard_imports.rs:97:75 + --> $DIR/wildcard_imports.rs:98:75 | LL | use wildcard_imports_helper::inner::inner_for_self_import::{self, *}; | ^ help: try: `inner_extern_foo` error: usage of wildcard import - --> $DIR/wildcard_imports.rs:98:13 + --> $DIR/wildcard_imports.rs:99:13 | LL | use wildcard_imports_helper::*; | ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `wildcard_imports_helper::{ExternA, extern_foo}` error: usage of wildcard import - --> $DIR/wildcard_imports.rs:109:20 + --> $DIR/wildcard_imports.rs:110:20 | LL | use self::{inner::*, inner2::*}; | ^^^^^^^^ help: try: `inner::inner_foo` error: usage of wildcard import - --> $DIR/wildcard_imports.rs:109:30 + --> $DIR/wildcard_imports.rs:110:30 | LL | use self::{inner::*, inner2::*}; | ^^^^^^^^^ help: try: `inner2::inner_bar` error: usage of wildcard import - --> $DIR/wildcard_imports.rs:116:13 + --> $DIR/wildcard_imports.rs:117:13 | LL | use wildcard_imports_helper::*; | ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `wildcard_imports_helper::{ExternExportedEnum, ExternExportedStruct, extern_exported}` error: usage of wildcard import - --> $DIR/wildcard_imports.rs:145:9 + --> $DIR/wildcard_imports.rs:146:9 | LL | use crate::in_fn_test::*; | ^^^^^^^^^^^^^^^^^^^^ help: try: `crate::in_fn_test::{ExportedEnum, ExportedStruct, exported}` error: usage of wildcard import - --> $DIR/wildcard_imports.rs:154:9 + --> $DIR/wildcard_imports.rs:155:9 | LL | use crate:: in_fn_test:: * ; | ^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `crate:: in_fn_test::exported` error: usage of wildcard import - --> $DIR/wildcard_imports.rs:155:9 + --> $DIR/wildcard_imports.rs:156:9 | LL | use crate:: fn_mod:: | _________^ @@ -93,31 +93,31 @@ LL | | *; | |_________^ help: try: `crate:: fn_mod::foo` error: usage of wildcard import - --> $DIR/wildcard_imports.rs:166:13 + --> $DIR/wildcard_imports.rs:167:13 | LL | use super::*; | ^^^^^^^^ help: try: `super::foofoo` error: usage of wildcard import - --> $DIR/wildcard_imports.rs:201:17 + --> $DIR/wildcard_imports.rs:202:17 | LL | use super::*; | ^^^^^^^^ help: try: `super::insidefoo` error: usage of wildcard import - --> $DIR/wildcard_imports.rs:209:13 + --> $DIR/wildcard_imports.rs:210:13 | LL | use super_imports::*; | ^^^^^^^^^^^^^^^^ help: try: `super_imports::foofoo` error: usage of wildcard import - --> $DIR/wildcard_imports.rs:218:17 + --> $DIR/wildcard_imports.rs:219:17 | LL | use super::super::*; | ^^^^^^^^^^^^^^^ help: try: `super::super::foofoo` error: usage of wildcard import - --> $DIR/wildcard_imports.rs:227:13 + --> $DIR/wildcard_imports.rs:228:13 | LL | use super::super::super_imports::*; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `super::super::super_imports::foofoo` From e998d61fe6a1407f73504a2fee16df2f9167a37c Mon Sep 17 00:00:00 2001 From: Hirochika Matsumoto Date: Sun, 18 Oct 2020 18:36:37 +0900 Subject: [PATCH 21/29] Downgrade applicability to MaybeIncorrect --- clippy_lints/src/unnecessary_wrap.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clippy_lints/src/unnecessary_wrap.rs b/clippy_lints/src/unnecessary_wrap.rs index 28cc20f00075..6434b3033dd8 100644 --- a/clippy_lints/src/unnecessary_wrap.rs +++ b/clippy_lints/src/unnecessary_wrap.rs @@ -122,7 +122,7 @@ impl<'tcx> LateLintPass<'tcx> for UnnecessaryWrap { fn_decl.output.span(), format!("remove `{}` from the return type...", return_type).as_str(), inner_ty, - Applicability::MachineApplicable, + Applicability::MaybeIncorrect, ); }); diag.multipart_suggestion( From 9d96311d7354d99b6f3b3956405b7defac81461c Mon Sep 17 00:00:00 2001 From: Hirochika Matsumoto Date: Sun, 18 Oct 2020 18:46:16 +0900 Subject: [PATCH 22/29] Remove wildcard use --- clippy_lints/src/unnecessary_wrap.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clippy_lints/src/unnecessary_wrap.rs b/clippy_lints/src/unnecessary_wrap.rs index 6434b3033dd8..7a9cf627059f 100644 --- a/clippy_lints/src/unnecessary_wrap.rs +++ b/clippy_lints/src/unnecessary_wrap.rs @@ -5,7 +5,7 @@ use crate::utils::{ use if_chain::if_chain; use rustc_errors::Applicability; use rustc_hir::intravisit::FnKind; -use rustc_hir::*; +use rustc_hir::{Body, ExprKind, FnDecl, HirId}; use rustc_lint::{LateContext, LateLintPass}; use rustc_middle::ty::subst::GenericArgKind; use rustc_session::{declare_lint_pass, declare_tool_lint}; From 532d205218c7aa28c1718f9e9088eeced6e2c5ac Mon Sep 17 00:00:00 2001 From: Hirochika Matsumoto Date: Sun, 18 Oct 2020 19:24:11 +0900 Subject: [PATCH 23/29] Skip functions in PartialOrd --- clippy_lints/src/unnecessary_wrap.rs | 12 ++++++++++-- clippy_lints/src/utils/paths.rs | 1 + 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/clippy_lints/src/unnecessary_wrap.rs b/clippy_lints/src/unnecessary_wrap.rs index 7a9cf627059f..ec6c823a4ec0 100644 --- a/clippy_lints/src/unnecessary_wrap.rs +++ b/clippy_lints/src/unnecessary_wrap.rs @@ -1,6 +1,6 @@ use crate::utils::{ - in_macro, is_type_diagnostic_item, match_qpath, paths, return_ty, snippet, span_lint_and_then, - visitors::find_all_ret_expressions, + in_macro, is_type_diagnostic_item, match_path, match_qpath, paths, return_ty, snippet, span_lint_and_then, + trait_ref_of_method, visitors::find_all_ret_expressions, }; use if_chain::if_chain; use rustc_errors::Applicability; @@ -63,6 +63,14 @@ impl<'tcx> LateLintPass<'tcx> for UnnecessaryWrap { span: Span, hir_id: HirId, ) { + if_chain! { + if let Some(trait_ref) = trait_ref_of_method(cx, hir_id); + if match_path(trait_ref.path, &paths::PARTIAL_ORD); + then { + return; + } + } + match fn_kind { FnKind::ItemFn(.., visibility, _) | FnKind::Method(.., Some(visibility), _) => { if visibility.node.is_pub() { diff --git a/clippy_lints/src/utils/paths.rs b/clippy_lints/src/utils/paths.rs index 2be5ff93f869..97e01f445ffe 100644 --- a/clippy_lints/src/utils/paths.rs +++ b/clippy_lints/src/utils/paths.rs @@ -81,6 +81,7 @@ pub const OS_STR_TO_OS_STRING: [&str; 5] = ["std", "ffi", "os_str", "OsStr", "to pub const PARKING_LOT_MUTEX_GUARD: [&str; 2] = ["parking_lot", "MutexGuard"]; pub const PARKING_LOT_RWLOCK_READ_GUARD: [&str; 2] = ["parking_lot", "RwLockReadGuard"]; pub const PARKING_LOT_RWLOCK_WRITE_GUARD: [&str; 2] = ["parking_lot", "RwLockWriteGuard"]; +pub const PARTIAL_ORD: [&str; 3] = ["std", "cmp", "PartialOrd"]; pub const PATH: [&str; 3] = ["std", "path", "Path"]; pub const PATH_BUF: [&str; 3] = ["std", "path", "PathBuf"]; pub const PATH_BUF_AS_PATH: [&str; 4] = ["std", "path", "PathBuf", "as_path"]; From bf46f78ca7e15631f08253c14975896030ca24a8 Mon Sep 17 00:00:00 2001 From: Hirochika Matsumoto Date: Sun, 18 Oct 2020 19:35:59 +0900 Subject: [PATCH 24/29] Fix clippy error --- clippy_lints/src/redundant_clone.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/clippy_lints/src/redundant_clone.rs b/clippy_lints/src/redundant_clone.rs index b4a9804fb25a..c4ca7fcdecc2 100644 --- a/clippy_lints/src/redundant_clone.rs +++ b/clippy_lints/src/redundant_clone.rs @@ -320,11 +320,11 @@ fn find_stmt_assigns_to<'tcx>( match (by_ref, &*rvalue) { (true, mir::Rvalue::Ref(_, _, place)) | (false, mir::Rvalue::Use(mir::Operand::Copy(place))) => { - base_local_and_movability(cx, mir, *place) + Some(base_local_and_movability(cx, mir, *place)) }, (false, mir::Rvalue::Ref(_, _, place)) => { if let [mir::ProjectionElem::Deref] = place.as_ref().projection { - base_local_and_movability(cx, mir, *place) + Some(base_local_and_movability(cx, mir, *place)) } else { None } @@ -341,7 +341,7 @@ fn base_local_and_movability<'tcx>( cx: &LateContext<'tcx>, mir: &mir::Body<'tcx>, place: mir::Place<'tcx>, -) -> Option<(mir::Local, CannotMoveOut)> { +) -> (mir::Local, CannotMoveOut) { use rustc_middle::mir::PlaceRef; // Dereference. You cannot move things out from a borrowed value. @@ -362,7 +362,7 @@ fn base_local_and_movability<'tcx>( && !is_copy(cx, mir::Place::ty_from(local, projection, &mir.local_decls, cx.tcx).ty); } - Some((local, deref || field || slice)) + (local, deref || field || slice) } struct LocalUseVisitor { From 86331a46e4030ddea41e1f9f9b8c313f58ce1eb1 Mon Sep 17 00:00:00 2001 From: Hirochika Matsumoto Date: Mon, 2 Nov 2020 23:59:47 +0900 Subject: [PATCH 25/29] Update stderr files --- tests/ui/doc_errors.stderr | 14 ++++---- tests/ui/drop_ref.stderr | 36 ++++++++++---------- tests/ui/manual_unwrap_or.fixed | 2 +- tests/ui/manual_unwrap_or.rs | 2 +- tests/ui/map_err.rs | 1 + tests/ui/map_err.stderr | 2 +- tests/ui/or_fun_call.stderr | 6 ++-- tests/ui/redundant_pattern_matching.stderr | 38 +++++++++++----------- tests/ui/result_unit_error.rs | 1 + tests/ui/result_unit_error.stderr | 8 ++--- 10 files changed, 56 insertions(+), 54 deletions(-) diff --git a/tests/ui/doc_errors.stderr b/tests/ui/doc_errors.stderr index c7b616e28970..b5a81419daee 100644 --- a/tests/ui/doc_errors.stderr +++ b/tests/ui/doc_errors.stderr @@ -1,5 +1,5 @@ error: docs for function returning `Result` missing `# Errors` section - --> $DIR/doc_errors.rs:7:1 + --> $DIR/doc_errors.rs:8:1 | LL | / pub fn pub_fn_missing_errors_header() -> Result<(), ()> { LL | | unimplemented!(); @@ -9,7 +9,7 @@ LL | | } = note: `-D clippy::missing-errors-doc` implied by `-D warnings` error: docs for function returning `Result` missing `# Errors` section - --> $DIR/doc_errors.rs:11:1 + --> $DIR/doc_errors.rs:12:1 | LL | / pub async fn async_pub_fn_missing_errors_header() -> Result<(), ()> { LL | | unimplemented!(); @@ -17,7 +17,7 @@ LL | | } | |_^ error: docs for function returning `Result` missing `# Errors` section - --> $DIR/doc_errors.rs:16:1 + --> $DIR/doc_errors.rs:17:1 | LL | / pub fn pub_fn_returning_io_result() -> io::Result<()> { LL | | unimplemented!(); @@ -25,7 +25,7 @@ LL | | } | |_^ error: docs for function returning `Result` missing `# Errors` section - --> $DIR/doc_errors.rs:21:1 + --> $DIR/doc_errors.rs:22:1 | LL | / pub async fn async_pub_fn_returning_io_result() -> io::Result<()> { LL | | unimplemented!(); @@ -33,7 +33,7 @@ LL | | } | |_^ error: docs for function returning `Result` missing `# Errors` section - --> $DIR/doc_errors.rs:51:5 + --> $DIR/doc_errors.rs:52:5 | LL | / pub fn pub_method_missing_errors_header() -> Result<(), ()> { LL | | unimplemented!(); @@ -41,7 +41,7 @@ LL | | } | |_____^ error: docs for function returning `Result` missing `# Errors` section - --> $DIR/doc_errors.rs:56:5 + --> $DIR/doc_errors.rs:57:5 | LL | / pub async fn async_pub_method_missing_errors_header() -> Result<(), ()> { LL | | unimplemented!(); @@ -49,7 +49,7 @@ LL | | } | |_____^ error: docs for function returning `Result` missing `# Errors` section - --> $DIR/doc_errors.rs:85:5 + --> $DIR/doc_errors.rs:86:5 | LL | fn trait_method_missing_errors_header() -> Result<(), ()>; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ diff --git a/tests/ui/drop_ref.stderr b/tests/ui/drop_ref.stderr index 7974bf56d44c..10087cb4820a 100644 --- a/tests/ui/drop_ref.stderr +++ b/tests/ui/drop_ref.stderr @@ -1,108 +1,108 @@ error: calls to `std::mem::drop` with a reference instead of an owned value. Dropping a reference does nothing. - --> $DIR/drop_ref.rs:10:5 + --> $DIR/drop_ref.rs:11:5 | LL | drop(&SomeStruct); | ^^^^^^^^^^^^^^^^^ | = note: `-D clippy::drop-ref` implied by `-D warnings` note: argument has type `&SomeStruct` - --> $DIR/drop_ref.rs:10:10 + --> $DIR/drop_ref.rs:11:10 | LL | drop(&SomeStruct); | ^^^^^^^^^^^ error: calls to `std::mem::drop` with a reference instead of an owned value. Dropping a reference does nothing. - --> $DIR/drop_ref.rs:13:5 + --> $DIR/drop_ref.rs:14:5 | LL | drop(&owned1); | ^^^^^^^^^^^^^ | note: argument has type `&SomeStruct` - --> $DIR/drop_ref.rs:13:10 + --> $DIR/drop_ref.rs:14:10 | LL | drop(&owned1); | ^^^^^^^ error: calls to `std::mem::drop` with a reference instead of an owned value. Dropping a reference does nothing. - --> $DIR/drop_ref.rs:14:5 + --> $DIR/drop_ref.rs:15:5 | LL | drop(&&owned1); | ^^^^^^^^^^^^^^ | note: argument has type `&&SomeStruct` - --> $DIR/drop_ref.rs:14:10 + --> $DIR/drop_ref.rs:15:10 | LL | drop(&&owned1); | ^^^^^^^^ error: calls to `std::mem::drop` with a reference instead of an owned value. Dropping a reference does nothing. - --> $DIR/drop_ref.rs:15:5 + --> $DIR/drop_ref.rs:16:5 | LL | drop(&mut owned1); | ^^^^^^^^^^^^^^^^^ | note: argument has type `&mut SomeStruct` - --> $DIR/drop_ref.rs:15:10 + --> $DIR/drop_ref.rs:16:10 | LL | drop(&mut owned1); | ^^^^^^^^^^^ error: calls to `std::mem::drop` with a reference instead of an owned value. Dropping a reference does nothing. - --> $DIR/drop_ref.rs:19:5 + --> $DIR/drop_ref.rs:20:5 | LL | drop(reference1); | ^^^^^^^^^^^^^^^^ | note: argument has type `&SomeStruct` - --> $DIR/drop_ref.rs:19:10 + --> $DIR/drop_ref.rs:20:10 | LL | drop(reference1); | ^^^^^^^^^^ error: calls to `std::mem::drop` with a reference instead of an owned value. Dropping a reference does nothing. - --> $DIR/drop_ref.rs:22:5 + --> $DIR/drop_ref.rs:23:5 | LL | drop(reference2); | ^^^^^^^^^^^^^^^^ | note: argument has type `&mut SomeStruct` - --> $DIR/drop_ref.rs:22:10 + --> $DIR/drop_ref.rs:23:10 | LL | drop(reference2); | ^^^^^^^^^^ error: calls to `std::mem::drop` with a reference instead of an owned value. Dropping a reference does nothing. - --> $DIR/drop_ref.rs:25:5 + --> $DIR/drop_ref.rs:26:5 | LL | drop(reference3); | ^^^^^^^^^^^^^^^^ | note: argument has type `&SomeStruct` - --> $DIR/drop_ref.rs:25:10 + --> $DIR/drop_ref.rs:26:10 | LL | drop(reference3); | ^^^^^^^^^^ error: calls to `std::mem::drop` with a reference instead of an owned value. Dropping a reference does nothing. - --> $DIR/drop_ref.rs:30:5 + --> $DIR/drop_ref.rs:31:5 | LL | drop(&val); | ^^^^^^^^^^ | note: argument has type `&T` - --> $DIR/drop_ref.rs:30:10 + --> $DIR/drop_ref.rs:31:10 | LL | drop(&val); | ^^^^ error: calls to `std::mem::drop` with a reference instead of an owned value. Dropping a reference does nothing. - --> $DIR/drop_ref.rs:38:5 + --> $DIR/drop_ref.rs:39:5 | LL | std::mem::drop(&SomeStruct); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ | note: argument has type `&SomeStruct` - --> $DIR/drop_ref.rs:38:20 + --> $DIR/drop_ref.rs:39:20 | LL | std::mem::drop(&SomeStruct); | ^^^^^^^^^^^ diff --git a/tests/ui/manual_unwrap_or.fixed b/tests/ui/manual_unwrap_or.fixed index 5aa5a43cb92c..740b2f667288 100644 --- a/tests/ui/manual_unwrap_or.fixed +++ b/tests/ui/manual_unwrap_or.fixed @@ -1,6 +1,6 @@ // run-rustfix #![allow(dead_code)] -#![allow(unused_variables)] +#![allow(unused_variables, clippy::unnecessary_wrap)] fn option_unwrap_or() { // int case diff --git a/tests/ui/manual_unwrap_or.rs b/tests/ui/manual_unwrap_or.rs index df534031f54c..6750662c58c0 100644 --- a/tests/ui/manual_unwrap_or.rs +++ b/tests/ui/manual_unwrap_or.rs @@ -1,6 +1,6 @@ // run-rustfix #![allow(dead_code)] -#![allow(unused_variables)] +#![allow(unused_variables, clippy::unnecessary_wrap)] fn option_unwrap_or() { // int case diff --git a/tests/ui/map_err.rs b/tests/ui/map_err.rs index 617b64228726..231562507a85 100644 --- a/tests/ui/map_err.rs +++ b/tests/ui/map_err.rs @@ -1,4 +1,5 @@ #![warn(clippy::map_err_ignore)] +#![allow(clippy::unnecessary_wrap)] use std::convert::TryFrom; use std::error::Error; use std::fmt; diff --git a/tests/ui/map_err.stderr b/tests/ui/map_err.stderr index 7273f4603807..390d7ce2e4e7 100644 --- a/tests/ui/map_err.stderr +++ b/tests/ui/map_err.stderr @@ -1,5 +1,5 @@ error: `map_err(|_|...` ignores the original error - --> $DIR/map_err.rs:22:32 + --> $DIR/map_err.rs:23:32 | LL | println!("{:?}", x.map_err(|_| Errors::Ignored)); | ^^^ diff --git a/tests/ui/or_fun_call.stderr b/tests/ui/or_fun_call.stderr index 8a7b5574c84e..a29e8fb58f5e 100644 --- a/tests/ui/or_fun_call.stderr +++ b/tests/ui/or_fun_call.stderr @@ -61,19 +61,19 @@ LL | without_default.unwrap_or(Foo::new()); | ^^^^^^^^^^^^^^^^^^^^^ help: try this: `unwrap_or_else(Foo::new)` error: use of `or_insert` followed by a function call - --> $DIR/or_fun_call.rs:62:19 + --> $DIR/or_fun_call.rs:63:19 | LL | map.entry(42).or_insert(String::new()); | ^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `or_insert_with(String::new)` error: use of `or_insert` followed by a function call - --> $DIR/or_fun_call.rs:65:21 + --> $DIR/or_fun_call.rs:66:21 | LL | btree.entry(42).or_insert(String::new()); | ^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `or_insert_with(String::new)` error: use of `unwrap_or` followed by a function call - --> $DIR/or_fun_call.rs:68:21 + --> $DIR/or_fun_call.rs:69:21 | LL | let _ = stringy.unwrap_or("".to_owned()); | ^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `unwrap_or_else(|| "".to_owned())` diff --git a/tests/ui/redundant_pattern_matching.stderr b/tests/ui/redundant_pattern_matching.stderr index 5341e81836a4..aeb309f5ba12 100644 --- a/tests/ui/redundant_pattern_matching.stderr +++ b/tests/ui/redundant_pattern_matching.stderr @@ -19,19 +19,19 @@ LL | if let Err(_) = Err::(42) {} | -------^^^^^^---------------------- help: try this: `if Err::(42).is_err()` error: redundant pattern matching, consider using `is_ok()` - --> $DIR/redundant_pattern_matching.rs:21:15 + --> $DIR/redundant_pattern_matching.rs:22:15 | LL | while let Ok(_) = Ok::(10) {} | ----------^^^^^--------------------- help: try this: `while Ok::(10).is_ok()` error: redundant pattern matching, consider using `is_err()` - --> $DIR/redundant_pattern_matching.rs:23:15 + --> $DIR/redundant_pattern_matching.rs:24:15 | LL | while let Err(_) = Ok::(10) {} | ----------^^^^^^--------------------- help: try this: `while Ok::(10).is_err()` error: redundant pattern matching, consider using `is_ok()` - --> $DIR/redundant_pattern_matching.rs:33:5 + --> $DIR/redundant_pattern_matching.rs:34:5 | LL | / match Ok::(42) { LL | | Ok(_) => true, @@ -40,7 +40,7 @@ LL | | }; | |_____^ help: try this: `Ok::(42).is_ok()` error: redundant pattern matching, consider using `is_err()` - --> $DIR/redundant_pattern_matching.rs:38:5 + --> $DIR/redundant_pattern_matching.rs:39:5 | LL | / match Ok::(42) { LL | | Ok(_) => false, @@ -49,7 +49,7 @@ LL | | }; | |_____^ help: try this: `Ok::(42).is_err()` error: redundant pattern matching, consider using `is_err()` - --> $DIR/redundant_pattern_matching.rs:43:5 + --> $DIR/redundant_pattern_matching.rs:44:5 | LL | / match Err::(42) { LL | | Ok(_) => false, @@ -58,7 +58,7 @@ LL | | }; | |_____^ help: try this: `Err::(42).is_err()` error: redundant pattern matching, consider using `is_ok()` - --> $DIR/redundant_pattern_matching.rs:48:5 + --> $DIR/redundant_pattern_matching.rs:49:5 | LL | / match Err::(42) { LL | | Ok(_) => true, @@ -67,73 +67,73 @@ LL | | }; | |_____^ help: try this: `Err::(42).is_ok()` error: redundant pattern matching, consider using `is_ok()` - --> $DIR/redundant_pattern_matching.rs:53:20 + --> $DIR/redundant_pattern_matching.rs:54:20 | LL | let _ = if let Ok(_) = Ok::(4) { true } else { false }; | -------^^^^^--------------------- help: try this: `if Ok::(4).is_ok()` error: redundant pattern matching, consider using `is_ok()` - --> $DIR/redundant_pattern_matching.rs:59:20 + --> $DIR/redundant_pattern_matching.rs:60:20 | LL | let _ = if let Ok(_) = gen_res() { | -------^^^^^------------ help: try this: `if gen_res().is_ok()` error: redundant pattern matching, consider using `is_err()` - --> $DIR/redundant_pattern_matching.rs:61:19 + --> $DIR/redundant_pattern_matching.rs:62:19 | LL | } else if let Err(_) = gen_res() { | -------^^^^^^------------ help: try this: `if gen_res().is_err()` error: redundant pattern matching, consider using `is_some()` - --> $DIR/redundant_pattern_matching.rs:84:19 + --> $DIR/redundant_pattern_matching.rs:85:19 | LL | while let Some(_) = r#try!(result_opt()) {} | ----------^^^^^^^----------------------- help: try this: `while r#try!(result_opt()).is_some()` error: redundant pattern matching, consider using `is_some()` - --> $DIR/redundant_pattern_matching.rs:85:16 + --> $DIR/redundant_pattern_matching.rs:86:16 | LL | if let Some(_) = r#try!(result_opt()) {} | -------^^^^^^^----------------------- help: try this: `if r#try!(result_opt()).is_some()` error: redundant pattern matching, consider using `is_some()` - --> $DIR/redundant_pattern_matching.rs:91:12 + --> $DIR/redundant_pattern_matching.rs:92:12 | LL | if let Some(_) = m!() {} | -------^^^^^^^------- help: try this: `if m!().is_some()` error: redundant pattern matching, consider using `is_some()` - --> $DIR/redundant_pattern_matching.rs:92:15 + --> $DIR/redundant_pattern_matching.rs:93:15 | LL | while let Some(_) = m!() {} | ----------^^^^^^^------- help: try this: `while m!().is_some()` error: redundant pattern matching, consider using `is_ok()` - --> $DIR/redundant_pattern_matching.rs:110:12 + --> $DIR/redundant_pattern_matching.rs:111:12 | LL | if let Ok(_) = Ok::(42) {} | -------^^^^^--------------------- help: try this: `if Ok::(42).is_ok()` error: redundant pattern matching, consider using `is_err()` - --> $DIR/redundant_pattern_matching.rs:112:12 + --> $DIR/redundant_pattern_matching.rs:113:12 | LL | if let Err(_) = Err::(42) {} | -------^^^^^^---------------------- help: try this: `if Err::(42).is_err()` error: redundant pattern matching, consider using `is_ok()` - --> $DIR/redundant_pattern_matching.rs:114:15 + --> $DIR/redundant_pattern_matching.rs:115:15 | LL | while let Ok(_) = Ok::(10) {} | ----------^^^^^--------------------- help: try this: `while Ok::(10).is_ok()` error: redundant pattern matching, consider using `is_err()` - --> $DIR/redundant_pattern_matching.rs:116:15 + --> $DIR/redundant_pattern_matching.rs:117:15 | LL | while let Err(_) = Ok::(10) {} | ----------^^^^^^--------------------- help: try this: `while Ok::(10).is_err()` error: redundant pattern matching, consider using `is_ok()` - --> $DIR/redundant_pattern_matching.rs:118:5 + --> $DIR/redundant_pattern_matching.rs:119:5 | LL | / match Ok::(42) { LL | | Ok(_) => true, @@ -142,7 +142,7 @@ LL | | }; | |_____^ help: try this: `Ok::(42).is_ok()` error: redundant pattern matching, consider using `is_err()` - --> $DIR/redundant_pattern_matching.rs:123:5 + --> $DIR/redundant_pattern_matching.rs:124:5 | LL | / match Err::(42) { LL | | Ok(_) => false, diff --git a/tests/ui/result_unit_error.rs b/tests/ui/result_unit_error.rs index a66f581b2159..1b4a702377ea 100644 --- a/tests/ui/result_unit_error.rs +++ b/tests/ui/result_unit_error.rs @@ -1,3 +1,4 @@ +#![allow(clippy::unnecessary_wrap)] #[warn(clippy::result_unit_err)] #[allow(unused)] diff --git a/tests/ui/result_unit_error.stderr b/tests/ui/result_unit_error.stderr index b8230032491b..12901b354f91 100644 --- a/tests/ui/result_unit_error.stderr +++ b/tests/ui/result_unit_error.stderr @@ -1,5 +1,5 @@ error: this returns a `Result<_, ()> - --> $DIR/result_unit_error.rs:4:1 + --> $DIR/result_unit_error.rs:5:1 | LL | pub fn returns_unit_error() -> Result { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -8,7 +8,7 @@ LL | pub fn returns_unit_error() -> Result { = help: use a custom Error type instead error: this returns a `Result<_, ()> - --> $DIR/result_unit_error.rs:13:5 + --> $DIR/result_unit_error.rs:14:5 | LL | fn get_that_error(&self) -> Result; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -16,7 +16,7 @@ LL | fn get_that_error(&self) -> Result; = help: use a custom Error type instead error: this returns a `Result<_, ()> - --> $DIR/result_unit_error.rs:15:5 + --> $DIR/result_unit_error.rs:16:5 | LL | fn get_this_one_too(&self) -> Result { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -24,7 +24,7 @@ LL | fn get_this_one_too(&self) -> Result { = help: use a custom Error type instead error: this returns a `Result<_, ()> - --> $DIR/result_unit_error.rs:33:5 + --> $DIR/result_unit_error.rs:34:5 | LL | pub fn unit_error(&self) -> Result { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ From 4c8d248190819f09753d4f6a9f89e3804e232ae7 Mon Sep 17 00:00:00 2001 From: Hirochika Matsumoto Date: Sat, 14 Nov 2020 18:54:16 +0900 Subject: [PATCH 26/29] Update stderr files --- tests/ui/map_flatten.stderr | 12 ++++++------ tests/ui/option_option.stderr | 2 +- tests/ui/or_fun_call.stderr | 8 ++++---- tests/ui/try_err.stderr | 12 ++++++------ 4 files changed, 17 insertions(+), 17 deletions(-) diff --git a/tests/ui/map_flatten.stderr b/tests/ui/map_flatten.stderr index d4e27f9aa075..756e6e818ad4 100644 --- a/tests/ui/map_flatten.stderr +++ b/tests/ui/map_flatten.stderr @@ -1,5 +1,5 @@ error: called `map(..).flatten()` on an `Iterator` - --> $DIR/map_flatten.rs:15:46 + --> $DIR/map_flatten.rs:16:46 | LL | let _: Vec<_> = vec![5_i8; 6].into_iter().map(option_id).flatten().collect(); | ^^^^^^^^^^^^^^^^^^^^^^^^^ help: try using `filter_map` instead: `.filter_map(option_id)` @@ -7,31 +7,31 @@ LL | let _: Vec<_> = vec![5_i8; 6].into_iter().map(option_id).flatten().coll = note: `-D clippy::map-flatten` implied by `-D warnings` error: called `map(..).flatten()` on an `Iterator` - --> $DIR/map_flatten.rs:16:46 + --> $DIR/map_flatten.rs:17:46 | LL | let _: Vec<_> = vec![5_i8; 6].into_iter().map(option_id_ref).flatten().collect(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try using `filter_map` instead: `.filter_map(option_id_ref)` error: called `map(..).flatten()` on an `Iterator` - --> $DIR/map_flatten.rs:17:46 + --> $DIR/map_flatten.rs:18:46 | LL | let _: Vec<_> = vec![5_i8; 6].into_iter().map(option_id_closure).flatten().collect(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try using `filter_map` instead: `.filter_map(option_id_closure)` error: called `map(..).flatten()` on an `Iterator` - --> $DIR/map_flatten.rs:18:46 + --> $DIR/map_flatten.rs:19:46 | LL | let _: Vec<_> = vec![5_i8; 6].into_iter().map(|x| x.checked_add(1)).flatten().collect(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try using `filter_map` instead: `.filter_map(|x| x.checked_add(1))` error: called `map(..).flatten()` on an `Iterator` - --> $DIR/map_flatten.rs:21:46 + --> $DIR/map_flatten.rs:22:46 | LL | let _: Vec<_> = vec![5_i8; 6].into_iter().map(|x| 0..x).flatten().collect(); | ^^^^^^^^^^^^^^^^^^^^^^^^ help: try using `flat_map` instead: `.flat_map(|x| 0..x)` error: called `map(..).flatten()` on an `Option` - --> $DIR/map_flatten.rs:24:39 + --> $DIR/map_flatten.rs:25:39 | LL | let _: Option<_> = (Some(Some(1))).map(|x| x).flatten(); | ^^^^^^^^^^^^^^^^^^^^^ help: try using `and_then` instead: `.and_then(|x| x)` diff --git a/tests/ui/option_option.stderr b/tests/ui/option_option.stderr index 8ace8338fcf0..ad7f081c7139 100644 --- a/tests/ui/option_option.stderr +++ b/tests/ui/option_option.stderr @@ -59,7 +59,7 @@ LL | Struct { x: Option> }, | ^^^^^^^^^^^^^^^^^^ error: consider using `Option` instead of `Option>` or a custom enum if you need to distinguish all 3 cases - --> $DIR/option_option.rs:78:14 + --> $DIR/option_option.rs:76:14 | LL | foo: Option>>, | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ diff --git a/tests/ui/or_fun_call.stderr b/tests/ui/or_fun_call.stderr index a29e8fb58f5e..fb8bf339828f 100644 --- a/tests/ui/or_fun_call.stderr +++ b/tests/ui/or_fun_call.stderr @@ -79,25 +79,25 @@ LL | let _ = stringy.unwrap_or("".to_owned()); | ^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `unwrap_or_else(|| "".to_owned())` error: use of `unwrap_or` followed by a function call - --> $DIR/or_fun_call.rs:76:21 + --> $DIR/or_fun_call.rs:77:21 | LL | let _ = Some(1).unwrap_or(map[&1]); | ^^^^^^^^^^^^^^^^^^ help: try this: `unwrap_or_else(|| map[&1])` error: use of `unwrap_or` followed by a function call - --> $DIR/or_fun_call.rs:78:21 + --> $DIR/or_fun_call.rs:79:21 | LL | let _ = Some(1).unwrap_or(map[&1]); | ^^^^^^^^^^^^^^^^^^ help: try this: `unwrap_or_else(|| map[&1])` error: use of `or` followed by a function call - --> $DIR/or_fun_call.rs:102:35 + --> $DIR/or_fun_call.rs:103:35 | LL | let _ = Some("a".to_string()).or(Some("b".to_string())); | ^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `or_else(|| Some("b".to_string()))` error: use of `or` followed by a function call - --> $DIR/or_fun_call.rs:106:10 + --> $DIR/or_fun_call.rs:107:10 | LL | .or(Some(Bar(b, Duration::from_secs(2)))); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `or_else(|| Some(Bar(b, Duration::from_secs(2))))` diff --git a/tests/ui/try_err.stderr b/tests/ui/try_err.stderr index ababa64e6d84..2c01d37192e8 100644 --- a/tests/ui/try_err.stderr +++ b/tests/ui/try_err.stderr @@ -29,7 +29,7 @@ LL | Err(err)?; | ^^^^^^^^^ help: try this: `return Err(err.into())` error: returning an `Err(_)` with the `?` operator - --> $DIR/try_err.rs:86:23 + --> $DIR/try_err.rs:87:23 | LL | Err(_) => Err(1)?, | ^^^^^^^ help: try this: `return Err(1)` @@ -40,7 +40,7 @@ LL | try_validation!(Ok::<_, i32>(5)); = note: this error originates in a macro (in Nightly builds, run with -Z macro-backtrace for more info) error: returning an `Err(_)` with the `?` operator - --> $DIR/try_err.rs:101:23 + --> $DIR/try_err.rs:102:23 | LL | Err(_) => Err(ret_one!())?, | ^^^^^^^^^^^^^^^^ help: try this: `return Err(ret_one!())` @@ -51,25 +51,25 @@ LL | try_validation_in_macro!(Ok::<_, i32>(5)); = note: this error originates in a macro (in Nightly builds, run with -Z macro-backtrace for more info) error: returning an `Err(_)` with the `?` operator - --> $DIR/try_err.rs:140:9 + --> $DIR/try_err.rs:141:9 | LL | Err(foo!())?; | ^^^^^^^^^^^^ help: try this: `return Err(foo!())` error: returning an `Err(_)` with the `?` operator - --> $DIR/try_err.rs:147:9 + --> $DIR/try_err.rs:148:9 | LL | Err(io::ErrorKind::WriteZero)? | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `return Poll::Ready(Err(io::ErrorKind::WriteZero.into()))` error: returning an `Err(_)` with the `?` operator - --> $DIR/try_err.rs:149:9 + --> $DIR/try_err.rs:150:9 | LL | Err(io::Error::new(io::ErrorKind::InvalidInput, "error"))? | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `return Poll::Ready(Err(io::Error::new(io::ErrorKind::InvalidInput, "error")))` error: returning an `Err(_)` with the `?` operator - --> $DIR/try_err.rs:157:9 + --> $DIR/try_err.rs:158:9 | LL | Err(io::ErrorKind::NotFound)? | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `return Poll::Ready(Some(Err(io::ErrorKind::NotFound.into())))` From 4e5c02e8980d16feeee953f112f940c598180ddc Mon Sep 17 00:00:00 2001 From: Hirochika Matsumoto Date: Sat, 14 Nov 2020 19:25:54 +0900 Subject: [PATCH 27/29] Ignore trait implementations --- clippy_lints/src/unnecessary_wrap.rs | 20 +++++++++----------- clippy_lints/src/utils/paths.rs | 1 - tests/ui/unnecessary_wrap.rs | 14 ++++++++++++++ 3 files changed, 23 insertions(+), 12 deletions(-) diff --git a/clippy_lints/src/unnecessary_wrap.rs b/clippy_lints/src/unnecessary_wrap.rs index ec6c823a4ec0..2960ffc5352e 100644 --- a/clippy_lints/src/unnecessary_wrap.rs +++ b/clippy_lints/src/unnecessary_wrap.rs @@ -1,11 +1,11 @@ use crate::utils::{ - in_macro, is_type_diagnostic_item, match_path, match_qpath, paths, return_ty, snippet, span_lint_and_then, - trait_ref_of_method, visitors::find_all_ret_expressions, + in_macro, is_type_diagnostic_item, match_qpath, paths, return_ty, snippet, span_lint_and_then, + visitors::find_all_ret_expressions, }; use if_chain::if_chain; use rustc_errors::Applicability; use rustc_hir::intravisit::FnKind; -use rustc_hir::{Body, ExprKind, FnDecl, HirId}; +use rustc_hir::{Body, ExprKind, FnDecl, HirId, ItemKind, Node}; use rustc_lint::{LateContext, LateLintPass}; use rustc_middle::ty::subst::GenericArgKind; use rustc_session::{declare_lint_pass, declare_tool_lint}; @@ -63,14 +63,6 @@ impl<'tcx> LateLintPass<'tcx> for UnnecessaryWrap { span: Span, hir_id: HirId, ) { - if_chain! { - if let Some(trait_ref) = trait_ref_of_method(cx, hir_id); - if match_path(trait_ref.path, &paths::PARTIAL_ORD); - then { - return; - } - } - match fn_kind { FnKind::ItemFn(.., visibility, _) | FnKind::Method(.., Some(visibility), _) => { if visibility.node.is_pub() { @@ -81,6 +73,12 @@ impl<'tcx> LateLintPass<'tcx> for UnnecessaryWrap { _ => (), } + if let Some(Node::Item(item)) = cx.tcx.hir().find(cx.tcx.hir().get_parent_node(hir_id)) { + if matches!(item.kind, ItemKind::Impl{ of_trait: Some(_), ..} | ItemKind::Trait(..)) { + return; + } + } + let (return_type, path) = if is_type_diagnostic_item(cx, return_ty(cx, hir_id), sym!(option_type)) { ("Option", &paths::OPTION_SOME) } else if is_type_diagnostic_item(cx, return_ty(cx, hir_id), sym!(result_type)) { diff --git a/clippy_lints/src/utils/paths.rs b/clippy_lints/src/utils/paths.rs index 97e01f445ffe..2be5ff93f869 100644 --- a/clippy_lints/src/utils/paths.rs +++ b/clippy_lints/src/utils/paths.rs @@ -81,7 +81,6 @@ pub const OS_STR_TO_OS_STRING: [&str; 5] = ["std", "ffi", "os_str", "OsStr", "to pub const PARKING_LOT_MUTEX_GUARD: [&str; 2] = ["parking_lot", "MutexGuard"]; pub const PARKING_LOT_RWLOCK_READ_GUARD: [&str; 2] = ["parking_lot", "RwLockReadGuard"]; pub const PARKING_LOT_RWLOCK_WRITE_GUARD: [&str; 2] = ["parking_lot", "RwLockWriteGuard"]; -pub const PARTIAL_ORD: [&str; 3] = ["std", "cmp", "PartialOrd"]; pub const PATH: [&str; 3] = ["std", "path", "Path"]; pub const PATH_BUF: [&str; 3] = ["std", "path", "PathBuf"]; pub const PATH_BUF_AS_PATH: [&str; 4] = ["std", "path", "PathBuf", "as_path"]; diff --git a/tests/ui/unnecessary_wrap.rs b/tests/ui/unnecessary_wrap.rs index 618c452065b9..112086904282 100644 --- a/tests/ui/unnecessary_wrap.rs +++ b/tests/ui/unnecessary_wrap.rs @@ -95,6 +95,20 @@ impl A { } } +trait B { + // trait impls are not linted + fn func13() -> Option { + Some(1) + } +} + +impl A for B { + // trait impls are not linted + fn func13() -> Option { + Some(0) + } +} + fn main() { // method calls are not linted func1(true, true); From 1f577c030049d974b7982f46e90d2bf96e665ea1 Mon Sep 17 00:00:00 2001 From: Hirochika Matsumoto Date: Sat, 14 Nov 2020 19:39:41 +0900 Subject: [PATCH 28/29] Fix embarrassing grammatical error --- tests/ui/unnecessary_wrap.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/ui/unnecessary_wrap.rs b/tests/ui/unnecessary_wrap.rs index 112086904282..58178e2e04b9 100644 --- a/tests/ui/unnecessary_wrap.rs +++ b/tests/ui/unnecessary_wrap.rs @@ -102,7 +102,7 @@ trait B { } } -impl A for B { +impl B for A { // trait impls are not linted fn func13() -> Option { Some(0) From c7445d7f2c3eece2b9056d05ea92fb1d1112b3ca Mon Sep 17 00:00:00 2001 From: Hirochika Matsumoto Date: Wed, 18 Nov 2020 01:01:22 +0900 Subject: [PATCH 29/29] Pluralize lint name --- CHANGELOG.md | 2 +- clippy_lints/src/lib.rs | 10 +++++----- .../{unnecessary_wrap.rs => unnecessary_wraps.rs} | 8 ++++---- src/lintlist/mod.rs | 4 ++-- tests/ui/derive_ord_xor_partial_ord.rs | 2 +- tests/ui/doc_errors.rs | 2 +- tests/ui/drop_ref.rs | 2 +- tests/ui/forget_ref.rs | 2 +- tests/ui/let_underscore_must_use.rs | 2 +- tests/ui/manual_unwrap_or.fixed | 2 +- tests/ui/manual_unwrap_or.rs | 2 +- tests/ui/map_err.rs | 2 +- tests/ui/map_flatten.fixed | 2 +- tests/ui/map_flatten.rs | 2 +- tests/ui/needless_lifetimes.rs | 2 +- tests/ui/option_map_unit_fn_fixable.fixed | 2 +- tests/ui/option_map_unit_fn_fixable.rs | 2 +- tests/ui/option_option.rs | 2 +- tests/ui/or_fun_call.fixed | 2 +- tests/ui/or_fun_call.rs | 2 +- tests/ui/panic_in_result_fn.rs | 2 +- tests/ui/question_mark.fixed | 2 +- tests/ui/question_mark.rs | 2 +- tests/ui/redundant_pattern_matching.fixed | 2 +- tests/ui/redundant_pattern_matching.rs | 2 +- tests/ui/result_unit_error.rs | 2 +- tests/ui/try_err.fixed | 2 +- tests/ui/try_err.rs | 2 +- tests/ui/unit_arg.rs | 2 +- tests/ui/unnecessary_clone.rs | 2 +- .../ui/{unnecessary_wrap.rs => unnecessary_wraps.rs} | 2 +- ...ecessary_wrap.stderr => unnecessary_wraps.stderr} | 12 ++++++------ tests/ui/useless_conversion.fixed | 2 +- tests/ui/useless_conversion.rs | 2 +- tests/ui/wildcard_imports.fixed | 2 +- tests/ui/wildcard_imports.rs | 2 +- 36 files changed, 49 insertions(+), 49 deletions(-) rename clippy_lints/src/{unnecessary_wrap.rs => unnecessary_wraps.rs} (96%) rename tests/ui/{unnecessary_wrap.rs => unnecessary_wraps.rs} (97%) rename tests/ui/{unnecessary_wrap.stderr => unnecessary_wraps.stderr} (89%) diff --git a/CHANGELOG.md b/CHANGELOG.md index 02b862d31965..64f67680b6c3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2008,7 +2008,7 @@ Released 2018-09-13 [`unnecessary_operation`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_operation [`unnecessary_sort_by`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_sort_by [`unnecessary_unwrap`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_unwrap -[`unnecessary_wrap`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_wrap +[`unnecessary_wraps`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_wraps [`unneeded_field_pattern`]: https://rust-lang.github.io/rust-clippy/master/index.html#unneeded_field_pattern [`unneeded_wildcard_pattern`]: https://rust-lang.github.io/rust-clippy/master/index.html#unneeded_wildcard_pattern [`unnested_or_patterns`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnested_or_patterns diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 2d1f75391bb5..f0c1cb8d6e5e 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -323,7 +323,7 @@ mod unicode; mod unit_return_expecting_ord; mod unnamed_address; mod unnecessary_sort_by; -mod unnecessary_wrap; +mod unnecessary_wraps; mod unnested_or_patterns; mod unsafe_removed_from_name; mod unused_io_amount; @@ -893,7 +893,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: &unnamed_address::FN_ADDRESS_COMPARISONS, &unnamed_address::VTABLE_ADDRESS_COMPARISONS, &unnecessary_sort_by::UNNECESSARY_SORT_BY, - &unnecessary_wrap::UNNECESSARY_WRAP, + &unnecessary_wraps::UNNECESSARY_WRAPS, &unnested_or_patterns::UNNESTED_OR_PATTERNS, &unsafe_removed_from_name::UNSAFE_REMOVED_FROM_NAME, &unused_io_amount::UNUSED_IO_AMOUNT, @@ -1066,7 +1066,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: store.register_late_pass(|| box redundant_clone::RedundantClone); store.register_late_pass(|| box slow_vector_initialization::SlowVectorInit); store.register_late_pass(|| box unnecessary_sort_by::UnnecessarySortBy); - store.register_late_pass(|| box unnecessary_wrap::UnnecessaryWrap); + store.register_late_pass(|| box unnecessary_wraps::UnnecessaryWraps); store.register_late_pass(|| box types::RefToMut); store.register_late_pass(|| box assertions_on_constants::AssertionsOnConstants); store.register_late_pass(|| box missing_const_for_fn::MissingConstForFn); @@ -1574,7 +1574,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: LintId::of(&unnamed_address::FN_ADDRESS_COMPARISONS), LintId::of(&unnamed_address::VTABLE_ADDRESS_COMPARISONS), LintId::of(&unnecessary_sort_by::UNNECESSARY_SORT_BY), - LintId::of(&unnecessary_wrap::UNNECESSARY_WRAP), + LintId::of(&unnecessary_wraps::UNNECESSARY_WRAPS), LintId::of(&unsafe_removed_from_name::UNSAFE_REMOVED_FROM_NAME), LintId::of(&unused_io_amount::UNUSED_IO_AMOUNT), LintId::of(&unused_unit::UNUSED_UNIT), @@ -1779,7 +1779,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: LintId::of(&types::UNNECESSARY_CAST), LintId::of(&types::VEC_BOX), LintId::of(&unnecessary_sort_by::UNNECESSARY_SORT_BY), - LintId::of(&unnecessary_wrap::UNNECESSARY_WRAP), + LintId::of(&unnecessary_wraps::UNNECESSARY_WRAPS), LintId::of(&unwrap::UNNECESSARY_UNWRAP), LintId::of(&useless_conversion::USELESS_CONVERSION), LintId::of(&zero_div_zero::ZERO_DIVIDED_BY_ZERO), diff --git a/clippy_lints/src/unnecessary_wrap.rs b/clippy_lints/src/unnecessary_wraps.rs similarity index 96% rename from clippy_lints/src/unnecessary_wrap.rs rename to clippy_lints/src/unnecessary_wraps.rs index 2960ffc5352e..25ecc7a82f18 100644 --- a/clippy_lints/src/unnecessary_wrap.rs +++ b/clippy_lints/src/unnecessary_wraps.rs @@ -46,14 +46,14 @@ declare_clippy_lint! { /// } /// } /// ``` - pub UNNECESSARY_WRAP, + pub UNNECESSARY_WRAPS, complexity, "functions that only return `Ok` or `Some`" } -declare_lint_pass!(UnnecessaryWrap => [UNNECESSARY_WRAP]); +declare_lint_pass!(UnnecessaryWraps => [UNNECESSARY_WRAPS]); -impl<'tcx> LateLintPass<'tcx> for UnnecessaryWrap { +impl<'tcx> LateLintPass<'tcx> for UnnecessaryWraps { fn check_fn( &mut self, cx: &LateContext<'tcx>, @@ -107,7 +107,7 @@ impl<'tcx> LateLintPass<'tcx> for UnnecessaryWrap { if can_sugg && !suggs.is_empty() { span_lint_and_then( cx, - UNNECESSARY_WRAP, + UNNECESSARY_WRAPS, span, format!( "this function's return value is unnecessarily wrapped by `{}`", diff --git a/src/lintlist/mod.rs b/src/lintlist/mod.rs index 4a0cdc5d82f9..a2edd6cd0bd4 100644 --- a/src/lintlist/mod.rs +++ b/src/lintlist/mod.rs @@ -2609,11 +2609,11 @@ vec![ module: "unwrap", }, Lint { - name: "unnecessary_wrap", + name: "unnecessary_wraps", group: "complexity", desc: "functions that only return `Ok` or `Some`", deprecation: None, - module: "unnecessary_wrap", + module: "unnecessary_wraps", }, Lint { name: "unneeded_field_pattern", diff --git a/tests/ui/derive_ord_xor_partial_ord.rs b/tests/ui/derive_ord_xor_partial_ord.rs index 78ec1727fc11..6f12d36d777d 100644 --- a/tests/ui/derive_ord_xor_partial_ord.rs +++ b/tests/ui/derive_ord_xor_partial_ord.rs @@ -1,5 +1,5 @@ #![warn(clippy::derive_ord_xor_partial_ord)] -#![allow(clippy::unnecessary_wrap)] +#![allow(clippy::unnecessary_wraps)] use std::cmp::Ordering; diff --git a/tests/ui/doc_errors.rs b/tests/ui/doc_errors.rs index 77df7f176f02..c77a74a58f22 100644 --- a/tests/ui/doc_errors.rs +++ b/tests/ui/doc_errors.rs @@ -1,7 +1,7 @@ // edition:2018 #![warn(clippy::missing_errors_doc)] #![allow(clippy::result_unit_err)] -#![allow(clippy::unnecessary_wrap)] +#![allow(clippy::unnecessary_wraps)] use std::io; diff --git a/tests/ui/drop_ref.rs b/tests/ui/drop_ref.rs index ba12e7638215..e1a15c609fd2 100644 --- a/tests/ui/drop_ref.rs +++ b/tests/ui/drop_ref.rs @@ -1,7 +1,7 @@ #![warn(clippy::drop_ref)] #![allow(clippy::toplevel_ref_arg)] #![allow(clippy::map_err_ignore)] -#![allow(clippy::unnecessary_wrap)] +#![allow(clippy::unnecessary_wraps)] use std::mem::drop; diff --git a/tests/ui/forget_ref.rs b/tests/ui/forget_ref.rs index a9c2a92ce6be..c49e6756a6c5 100644 --- a/tests/ui/forget_ref.rs +++ b/tests/ui/forget_ref.rs @@ -1,6 +1,6 @@ #![warn(clippy::forget_ref)] #![allow(clippy::toplevel_ref_arg)] -#![allow(clippy::unnecessary_wrap)] +#![allow(clippy::unnecessary_wraps)] use std::mem::forget; diff --git a/tests/ui/let_underscore_must_use.rs b/tests/ui/let_underscore_must_use.rs index e3800cda1b18..a842e872a37b 100644 --- a/tests/ui/let_underscore_must_use.rs +++ b/tests/ui/let_underscore_must_use.rs @@ -1,5 +1,5 @@ #![warn(clippy::let_underscore_must_use)] -#![allow(clippy::unnecessary_wrap)] +#![allow(clippy::unnecessary_wraps)] // Debug implementations can fire this lint, // so we shouldn't lint external macros diff --git a/tests/ui/manual_unwrap_or.fixed b/tests/ui/manual_unwrap_or.fixed index 740b2f667288..81d903c15d32 100644 --- a/tests/ui/manual_unwrap_or.fixed +++ b/tests/ui/manual_unwrap_or.fixed @@ -1,6 +1,6 @@ // run-rustfix #![allow(dead_code)] -#![allow(unused_variables, clippy::unnecessary_wrap)] +#![allow(unused_variables, clippy::unnecessary_wraps)] fn option_unwrap_or() { // int case diff --git a/tests/ui/manual_unwrap_or.rs b/tests/ui/manual_unwrap_or.rs index 6750662c58c0..16105d379c30 100644 --- a/tests/ui/manual_unwrap_or.rs +++ b/tests/ui/manual_unwrap_or.rs @@ -1,6 +1,6 @@ // run-rustfix #![allow(dead_code)] -#![allow(unused_variables, clippy::unnecessary_wrap)] +#![allow(unused_variables, clippy::unnecessary_wraps)] fn option_unwrap_or() { // int case diff --git a/tests/ui/map_err.rs b/tests/ui/map_err.rs index 231562507a85..05b9949f1021 100644 --- a/tests/ui/map_err.rs +++ b/tests/ui/map_err.rs @@ -1,5 +1,5 @@ #![warn(clippy::map_err_ignore)] -#![allow(clippy::unnecessary_wrap)] +#![allow(clippy::unnecessary_wraps)] use std::convert::TryFrom; use std::error::Error; use std::fmt; diff --git a/tests/ui/map_flatten.fixed b/tests/ui/map_flatten.fixed index b4a51837b2fb..773b5914439d 100644 --- a/tests/ui/map_flatten.fixed +++ b/tests/ui/map_flatten.fixed @@ -4,7 +4,7 @@ #![allow(clippy::let_underscore_drop)] #![allow(clippy::missing_docs_in_private_items)] #![allow(clippy::map_identity)] -#![allow(clippy::unnecessary_wrap)] +#![allow(clippy::unnecessary_wraps)] fn main() { // mapping to Option on Iterator diff --git a/tests/ui/map_flatten.rs b/tests/ui/map_flatten.rs index e83cc46bda28..578bd8772679 100644 --- a/tests/ui/map_flatten.rs +++ b/tests/ui/map_flatten.rs @@ -4,7 +4,7 @@ #![allow(clippy::let_underscore_drop)] #![allow(clippy::missing_docs_in_private_items)] #![allow(clippy::map_identity)] -#![allow(clippy::unnecessary_wrap)] +#![allow(clippy::unnecessary_wraps)] fn main() { // mapping to Option on Iterator diff --git a/tests/ui/needless_lifetimes.rs b/tests/ui/needless_lifetimes.rs index e5973bbef8d4..44972c8c6396 100644 --- a/tests/ui/needless_lifetimes.rs +++ b/tests/ui/needless_lifetimes.rs @@ -1,5 +1,5 @@ #![warn(clippy::needless_lifetimes)] -#![allow(dead_code, clippy::needless_pass_by_value, clippy::unnecessary_wrap)] +#![allow(dead_code, clippy::needless_pass_by_value, clippy::unnecessary_wraps)] fn distinct_lifetimes<'a, 'b>(_x: &'a u8, _y: &'b u8, _z: u8) {} diff --git a/tests/ui/option_map_unit_fn_fixable.fixed b/tests/ui/option_map_unit_fn_fixable.fixed index de2e91559060..7d29445e66c8 100644 --- a/tests/ui/option_map_unit_fn_fixable.fixed +++ b/tests/ui/option_map_unit_fn_fixable.fixed @@ -2,7 +2,7 @@ #![warn(clippy::option_map_unit_fn)] #![allow(unused)] -#![allow(clippy::unnecessary_wrap)] +#![allow(clippy::unnecessary_wraps)] fn do_nothing(_: T) {} diff --git a/tests/ui/option_map_unit_fn_fixable.rs b/tests/ui/option_map_unit_fn_fixable.rs index f0887c8a4bce..b6f834f686f9 100644 --- a/tests/ui/option_map_unit_fn_fixable.rs +++ b/tests/ui/option_map_unit_fn_fixable.rs @@ -2,7 +2,7 @@ #![warn(clippy::option_map_unit_fn)] #![allow(unused)] -#![allow(clippy::unnecessary_wrap)] +#![allow(clippy::unnecessary_wraps)] fn do_nothing(_: T) {} diff --git a/tests/ui/option_option.rs b/tests/ui/option_option.rs index 557d29dff67a..6859ba8e5bb8 100644 --- a/tests/ui/option_option.rs +++ b/tests/ui/option_option.rs @@ -1,5 +1,5 @@ #![deny(clippy::option_option)] -#![allow(clippy::unnecessary_wrap)] +#![allow(clippy::unnecessary_wraps)] fn input(_: Option>) {} diff --git a/tests/ui/or_fun_call.fixed b/tests/ui/or_fun_call.fixed index e1735bc88f57..2a63318c8c7a 100644 --- a/tests/ui/or_fun_call.fixed +++ b/tests/ui/or_fun_call.fixed @@ -2,7 +2,7 @@ #![warn(clippy::or_fun_call)] #![allow(dead_code)] -#![allow(clippy::unnecessary_wrap)] +#![allow(clippy::unnecessary_wraps)] use std::collections::BTreeMap; use std::collections::HashMap; diff --git a/tests/ui/or_fun_call.rs b/tests/ui/or_fun_call.rs index a6abd2e8b349..026ef437caa1 100644 --- a/tests/ui/or_fun_call.rs +++ b/tests/ui/or_fun_call.rs @@ -2,7 +2,7 @@ #![warn(clippy::or_fun_call)] #![allow(dead_code)] -#![allow(clippy::unnecessary_wrap)] +#![allow(clippy::unnecessary_wraps)] use std::collections::BTreeMap; use std::collections::HashMap; diff --git a/tests/ui/panic_in_result_fn.rs b/tests/ui/panic_in_result_fn.rs index be4e85d05a72..3d3c19a1be51 100644 --- a/tests/ui/panic_in_result_fn.rs +++ b/tests/ui/panic_in_result_fn.rs @@ -1,5 +1,5 @@ #![warn(clippy::panic_in_result_fn)] -#![allow(clippy::unnecessary_wrap)] +#![allow(clippy::unnecessary_wraps)] struct A; diff --git a/tests/ui/question_mark.fixed b/tests/ui/question_mark.fixed index df3c98a0aa57..0b5746cb5227 100644 --- a/tests/ui/question_mark.fixed +++ b/tests/ui/question_mark.fixed @@ -1,6 +1,6 @@ // run-rustfix #![allow(unreachable_code)] -#![allow(clippy::unnecessary_wrap)] +#![allow(clippy::unnecessary_wraps)] fn some_func(a: Option) -> Option { a?; diff --git a/tests/ui/question_mark.rs b/tests/ui/question_mark.rs index 62b3e96b65a8..0f0825c93346 100644 --- a/tests/ui/question_mark.rs +++ b/tests/ui/question_mark.rs @@ -1,6 +1,6 @@ // run-rustfix #![allow(unreachable_code)] -#![allow(clippy::unnecessary_wrap)] +#![allow(clippy::unnecessary_wraps)] fn some_func(a: Option) -> Option { if a.is_none() { diff --git a/tests/ui/redundant_pattern_matching.fixed b/tests/ui/redundant_pattern_matching.fixed index 2e7ba8bc04d8..aa20512296aa 100644 --- a/tests/ui/redundant_pattern_matching.fixed +++ b/tests/ui/redundant_pattern_matching.fixed @@ -7,7 +7,7 @@ unused_must_use, clippy::needless_bool, clippy::match_like_matches_macro, - clippy::unnecessary_wrap, + clippy::unnecessary_wraps, deprecated )] diff --git a/tests/ui/redundant_pattern_matching.rs b/tests/ui/redundant_pattern_matching.rs index ea0e18f5970a..d76f9c288ffd 100644 --- a/tests/ui/redundant_pattern_matching.rs +++ b/tests/ui/redundant_pattern_matching.rs @@ -7,7 +7,7 @@ unused_must_use, clippy::needless_bool, clippy::match_like_matches_macro, - clippy::unnecessary_wrap, + clippy::unnecessary_wraps, deprecated )] diff --git a/tests/ui/result_unit_error.rs b/tests/ui/result_unit_error.rs index 1b4a702377ea..5e57c752b5a0 100644 --- a/tests/ui/result_unit_error.rs +++ b/tests/ui/result_unit_error.rs @@ -1,4 +1,4 @@ -#![allow(clippy::unnecessary_wrap)] +#![allow(clippy::unnecessary_wraps)] #[warn(clippy::result_unit_err)] #[allow(unused)] diff --git a/tests/ui/try_err.fixed b/tests/ui/try_err.fixed index 19458ddf68ef..652b611208b7 100644 --- a/tests/ui/try_err.fixed +++ b/tests/ui/try_err.fixed @@ -2,7 +2,7 @@ // aux-build:macro_rules.rs #![deny(clippy::try_err)] -#![allow(clippy::unnecessary_wrap)] +#![allow(clippy::unnecessary_wraps)] #[macro_use] extern crate macro_rules; diff --git a/tests/ui/try_err.rs b/tests/ui/try_err.rs index ab6a32dbe4d0..6bd479657b70 100644 --- a/tests/ui/try_err.rs +++ b/tests/ui/try_err.rs @@ -2,7 +2,7 @@ // aux-build:macro_rules.rs #![deny(clippy::try_err)] -#![allow(clippy::unnecessary_wrap)] +#![allow(clippy::unnecessary_wraps)] #[macro_use] extern crate macro_rules; diff --git a/tests/ui/unit_arg.rs b/tests/ui/unit_arg.rs index 6bf704c09e0e..9ad16d365094 100644 --- a/tests/ui/unit_arg.rs +++ b/tests/ui/unit_arg.rs @@ -4,7 +4,7 @@ unused_must_use, unused_variables, clippy::unused_unit, - clippy::unnecessary_wrap, + clippy::unnecessary_wraps, clippy::or_fun_call )] diff --git a/tests/ui/unnecessary_clone.rs b/tests/ui/unnecessary_clone.rs index 3b1a6cf57c65..6770a7fac90f 100644 --- a/tests/ui/unnecessary_clone.rs +++ b/tests/ui/unnecessary_clone.rs @@ -1,7 +1,7 @@ // does not test any rustfixable lints #![warn(clippy::clone_on_ref_ptr)] -#![allow(unused, clippy::redundant_clone, clippy::unnecessary_wrap)] +#![allow(unused, clippy::redundant_clone, clippy::unnecessary_wraps)] use std::cell::RefCell; use std::rc::{self, Rc}; diff --git a/tests/ui/unnecessary_wrap.rs b/tests/ui/unnecessary_wraps.rs similarity index 97% rename from tests/ui/unnecessary_wrap.rs rename to tests/ui/unnecessary_wraps.rs index 58178e2e04b9..a53dec8f91ac 100644 --- a/tests/ui/unnecessary_wrap.rs +++ b/tests/ui/unnecessary_wraps.rs @@ -1,4 +1,4 @@ -#![warn(clippy::unnecessary_wrap)] +#![warn(clippy::unnecessary_wraps)] #![allow(clippy::no_effect)] #![allow(clippy::needless_return)] #![allow(clippy::if_same_then_else)] diff --git a/tests/ui/unnecessary_wrap.stderr b/tests/ui/unnecessary_wraps.stderr similarity index 89% rename from tests/ui/unnecessary_wrap.stderr rename to tests/ui/unnecessary_wraps.stderr index 5f21b74bc760..410f054b8efc 100644 --- a/tests/ui/unnecessary_wrap.stderr +++ b/tests/ui/unnecessary_wraps.stderr @@ -1,5 +1,5 @@ error: this function's return value is unnecessarily wrapped by `Option` - --> $DIR/unnecessary_wrap.rs:8:1 + --> $DIR/unnecessary_wraps.rs:8:1 | LL | / fn func1(a: bool, b: bool) -> Option { LL | | if a && b { @@ -10,7 +10,7 @@ LL | | } LL | | } | |_^ | - = note: `-D clippy::unnecessary-wrap` implied by `-D warnings` + = note: `-D clippy::unnecessary-wraps` implied by `-D warnings` help: remove `Option` from the return type... | LL | fn func1(a: bool, b: bool) -> i32 { @@ -26,7 +26,7 @@ LL | } else { ... error: this function's return value is unnecessarily wrapped by `Option` - --> $DIR/unnecessary_wrap.rs:21:1 + --> $DIR/unnecessary_wraps.rs:21:1 | LL | / fn func2(a: bool, b: bool) -> Option { LL | | if a && b { @@ -52,7 +52,7 @@ LL | 30 | error: this function's return value is unnecessarily wrapped by `Option` - --> $DIR/unnecessary_wrap.rs:51:1 + --> $DIR/unnecessary_wraps.rs:51:1 | LL | / fn func5() -> Option { LL | | Some(1) @@ -69,7 +69,7 @@ LL | 1 | error: this function's return value is unnecessarily wrapped by `Result` - --> $DIR/unnecessary_wrap.rs:61:1 + --> $DIR/unnecessary_wraps.rs:61:1 | LL | / fn func7() -> Result { LL | | Ok(1) @@ -86,7 +86,7 @@ LL | 1 | error: this function's return value is unnecessarily wrapped by `Option` - --> $DIR/unnecessary_wrap.rs:93:5 + --> $DIR/unnecessary_wraps.rs:93:5 | LL | / fn func12() -> Option { LL | | Some(1) diff --git a/tests/ui/useless_conversion.fixed b/tests/ui/useless_conversion.fixed index 45ee367d649b..03977de9455e 100644 --- a/tests/ui/useless_conversion.fixed +++ b/tests/ui/useless_conversion.fixed @@ -1,7 +1,7 @@ // run-rustfix #![deny(clippy::useless_conversion)] -#![allow(clippy::unnecessary_wrap)] +#![allow(clippy::unnecessary_wraps)] fn test_generic(val: T) -> T { let _ = val; diff --git a/tests/ui/useless_conversion.rs b/tests/ui/useless_conversion.rs index e5bdffed20d9..f6e094c16616 100644 --- a/tests/ui/useless_conversion.rs +++ b/tests/ui/useless_conversion.rs @@ -1,7 +1,7 @@ // run-rustfix #![deny(clippy::useless_conversion)] -#![allow(clippy::unnecessary_wrap)] +#![allow(clippy::unnecessary_wraps)] fn test_generic(val: T) -> T { let _ = T::from(val); diff --git a/tests/ui/wildcard_imports.fixed b/tests/ui/wildcard_imports.fixed index 14007e5f251d..ee9c9045fff5 100644 --- a/tests/ui/wildcard_imports.fixed +++ b/tests/ui/wildcard_imports.fixed @@ -4,7 +4,7 @@ #![warn(clippy::wildcard_imports)] //#![allow(clippy::redundant_pub_crate)] #![allow(unused)] -#![allow(clippy::unnecessary_wrap)] +#![allow(clippy::unnecessary_wraps)] #![warn(unused_imports)] extern crate wildcard_imports_helper; diff --git a/tests/ui/wildcard_imports.rs b/tests/ui/wildcard_imports.rs index 0e8631ca7047..efaa8f9ef664 100644 --- a/tests/ui/wildcard_imports.rs +++ b/tests/ui/wildcard_imports.rs @@ -4,7 +4,7 @@ #![warn(clippy::wildcard_imports)] //#![allow(clippy::redundant_pub_crate)] #![allow(unused)] -#![allow(clippy::unnecessary_wrap)] +#![allow(clippy::unnecessary_wraps)] #![warn(unused_imports)] extern crate wildcard_imports_helper;