X-Git-Url: https://git.lizzy.rs/?a=blobdiff_plain;f=clippy_lints%2Fsrc%2Funnecessary_wraps.rs;h=c2be457e9dcf96066544102f1e6620c47466d3a2;hb=9f6b5de7deaf4dc9e7917370ad09ab85dc23997c;hp=8ac5dd696b7620d85216c281603c286582ed5411;hpb=a1b89f07f00bda6d10e341fda01385f571a243b6;p=rust.git diff --git a/clippy_lints/src/unnecessary_wraps.rs b/clippy_lints/src/unnecessary_wraps.rs index 8ac5dd696b7..c2be457e9dc 100644 --- a/clippy_lints/src/unnecessary_wraps.rs +++ b/clippy_lints/src/unnecessary_wraps.rs @@ -1,13 +1,12 @@ -use crate::utils::{ - contains_return, in_macro, is_type_diagnostic_item, match_qpath, paths, return_ty, snippet, span_lint_and_then, - visitors::find_all_ret_expressions, -}; +use clippy_utils::diagnostics::span_lint_and_then; +use clippy_utils::source::snippet; +use clippy_utils::{contains_return, in_macro, match_qpath, paths, return_ty, 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, Impl, ItemKind, Node}; use rustc_lint::{LateContext, LateLintPass}; -use rustc_middle::ty::subst::GenericArgKind; +use rustc_middle::ty; use rustc_session::{declare_lint_pass, declare_tool_lint}; use rustc_span::symbol::sym; use rustc_span::Span; @@ -17,8 +16,8 @@ /// /// **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 code at callee side. + /// **Known problems:** There can be false positives if the function signature is designed to + /// fit some external requirement. /// /// **Example:** /// @@ -48,7 +47,7 @@ /// } /// ``` pub UNNECESSARY_WRAPS, - complexity, + pedantic, "functions that only return `Ok` or `Some`" } @@ -64,16 +63,18 @@ fn check_fn( span: Span, hir_id: HirId, ) { + // Abort if public function/method or closure. match fn_kind { - FnKind::ItemFn(.., visibility, _) | FnKind::Method(.., Some(visibility), _) => { + FnKind::ItemFn(.., visibility) | FnKind::Method(.., Some(visibility)) => { if visibility.node.is_pub() { return; } }, - FnKind::Closure(..) => return, - _ => (), + FnKind::Closure => return, + FnKind::Method(..) => (), } + // Abort if the method is implementing a trait or of it a trait method. if let Some(Node::Item(item)) = cx.tcx.hir().find(cx.tcx.hir().get_parent_node(hir_id)) { if matches!( item.kind, @@ -83,25 +84,44 @@ fn check_fn( } } - 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) { - ("Result", &paths::RESULT_OK) + // Get the wrapper and inner types, if can't, abort. + let (return_type_label, path, inner_type) = if let ty::Adt(adt_def, subst) = return_ty(cx, hir_id).kind() { + if cx.tcx.is_diagnostic_item(sym::option_type, adt_def.did) { + ("Option", &paths::OPTION_SOME, subst.type_at(0)) + } else if cx.tcx.is_diagnostic_item(sym::result_type, adt_def.did) { + ("Result", &paths::RESULT_OK, subst.type_at(0)) + } else { + return; + } } else { return; }; + // Check if all return expression respect the following condition and collect them. 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); + // Check if a function call. if let ExprKind::Call(ref func, ref args) = ret_expr.kind; + // Get the Path of the function call. if let ExprKind::Path(ref qpath) = func.kind; + // Check if OPTION_SOME or RESULT_OK, depending on return type. if match_qpath(qpath, path); if args.len() == 1; + // Make sure the function argument does not contain a return expression. if !contains_return(&args[0]); then { - suggs.push((ret_expr.span, snippet(cx, args[0].span.source_callsite(), "..").to_string())); + suggs.push( + ( + ret_expr.span, + if inner_type.is_unit() { + "".to_string() + } else { + snippet(cx, args[0].span.source_callsite(), "..").to_string() + } + ) + ); true } else { false @@ -110,39 +130,34 @@ fn check_fn( }); if can_sugg && !suggs.is_empty() { - span_lint_and_then( - cx, - UNNECESSARY_WRAPS, - span, - format!( - "this function's return value is unnecessarily wrapped by `{}`", - return_type + let (lint_msg, return_type_sugg_msg, return_type_sugg, body_sugg_msg) = if inner_type.is_unit() { + ( + "this function's return value is unnecessary".to_string(), + "remove the return type...".to_string(), + snippet(cx, fn_decl.output.span(), "..").to_string(), + "...and then remove returned values", ) - .as_str(), - |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::MaybeIncorrect, - ); - }); - diag.multipart_suggestion( - "...and change the returning expressions", - suggs, - Applicability::MaybeIncorrect, - ); - }, - ); + } else { + ( + format!( + "this function's return value is unnecessarily wrapped by `{}`", + return_type_label + ), + format!("remove `{}` from the return type...", return_type_label), + inner_type.to_string(), + "...and then change returning expressions", + ) + }; + + span_lint_and_then(cx, UNNECESSARY_WRAPS, span, lint_msg.as_str(), |diag| { + diag.span_suggestion( + fn_decl.output.span(), + return_type_sugg_msg.as_str(), + return_type_sugg, + Applicability::MaybeIncorrect, + ); + diag.multipart_suggestion(body_sugg_msg, suggs, Applicability::MaybeIncorrect); + }); } } }