rust/clippy_lints/src/unnecessary_wraps.rs

178 lines
6.0 KiB
Rust
Raw Normal View History

use clippy_utils::diagnostics::span_lint_and_then;
use clippy_utils::source::snippet;
use clippy_utils::{contains_return, is_lang_ctor, return_ty, visitors::find_all_ret_expressions};
use if_chain::if_chain;
use rustc_errors::Applicability;
use rustc_hir::intravisit::FnKind;
use rustc_hir::LangItem::{OptionSome, ResultOk};
use rustc_hir::{Body, ExprKind, FnDecl, HirId, Impl, ItemKind, Node};
use rustc_lint::{LateContext, LateLintPass};
use rustc_middle::ty;
use rustc_session::{declare_tool_lint, impl_lint_pass};
use rustc_span::symbol::sym;
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
/// There can be false positives if the function signature is designed to
/// fit some external requirement.
///
/// ### Example
/// ```rust
/// fn get_cool_number(a: bool, b: bool) -> Option<i32> {
/// if a && b {
/// return Some(50);
/// }
/// if a {
/// Some(0)
/// } else {
/// Some(10)
/// }
/// }
/// ```
/// Use instead:
/// ```rust
/// fn get_cool_number(a: bool, b: bool) -> i32 {
/// if a && b {
/// return 50;
/// }
/// if a {
/// 0
/// } else {
/// 10
/// }
/// }
/// ```
Added `clippy::version` attribute to all normal lints So, some context for this, well, more a story. I'm not used to scripting, I've never really scripted anything, even if it's a valuable skill. I just never really needed it. Now, `@flip1995` correctly suggested using a script for this in `rust-clippy#7813`... And I decided to write a script using nushell because why not? This was a mistake... I spend way more time on this than I would like to admit. It has definitely been more than 4 hours. It shouldn't take that long, but me being new to scripting and nushell just wasn't a good mixture... Anyway, here is the script that creates another script which adds the versions. Fun... Just execute this on the `gh-pages` branch and the resulting `replacer.sh` in `clippy_lints` and it should all work. ```nu mv v0.0.212 rust-1.00.0; mv beta rust-1.57.0; mv master rust-1.58.0; let paths = (open ./rust-1.58.0/lints.json | select id id_span | flatten | select id path); let versions = ( ls | where name =~ "rust-" | select name | format {name}/lints.json | each { open $it | select id | insert version $it | str substring "5,11" version} | group-by id | rotate counter-clockwise id version | update version {get version | first 1} | flatten | select id version); $paths | each { |row| let version = ($versions | where id == ($row.id) | format {version}) let idu = ($row.id | str upcase) $"sed -i '0,/($idu),/{s/pub ($idu),/#[clippy::version = "($version)"]\n pub ($idu),/}' ($row.path)" } | str collect ";" | str find-replace --all '1.00.0' 'pre 1.29.0' | save "replacer.sh"; ``` And this still has some problems, but at this point I just want to be done -.-
2021-10-21 14:06:26 -05:00
#[clippy::version = "1.50.0"]
pub UNNECESSARY_WRAPS,
pedantic,
"functions that only return `Ok` or `Some`"
}
pub struct UnnecessaryWraps {
avoid_breaking_exported_api: bool,
}
impl_lint_pass!(UnnecessaryWraps => [UNNECESSARY_WRAPS]);
impl UnnecessaryWraps {
pub fn new(avoid_breaking_exported_api: bool) -> Self {
Self {
avoid_breaking_exported_api,
}
}
}
impl<'tcx> LateLintPass<'tcx> for UnnecessaryWraps {
fn check_fn(
&mut self,
cx: &LateContext<'tcx>,
fn_kind: FnKind<'tcx>,
fn_decl: &FnDecl<'tcx>,
body: &Body<'tcx>,
span: Span,
hir_id: HirId,
) {
// Abort if public function/method or closure.
match fn_kind {
FnKind::ItemFn(..) | FnKind::Method(..) => {
2021-07-28 17:07:32 -05:00
let def_id = cx.tcx.hir().local_def_id(hir_id);
if self.avoid_breaking_exported_api && cx.access_levels.is_exported(def_id) {
return;
}
},
2020-11-27 02:24:42 -06:00
FnKind::Closure => return,
}
// 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,
ItemKind::Impl(Impl { of_trait: Some(_), .. }) | ItemKind::Trait(..)
) {
return;
}
}
// Get the wrapper and inner types, if can't, abort.
let (return_type_label, lang_item, inner_type) = if let ty::Adt(adt_def, subst) = return_ty(cx, hir_id).kind() {
2021-10-02 18:51:01 -05:00
if cx.tcx.is_diagnostic_item(sym::Option, adt_def.did) {
("Option", OptionSome, subst.type_at(0))
2021-10-02 18:51:01 -05:00
} else if cx.tcx.is_diagnostic_item(sym::Result, adt_def.did) {
("Result", ResultOk, 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 !ret_expr.span.from_expansion();
// Check if a function call.
if let ExprKind::Call(func, [arg]) = ret_expr.kind;
// Check if OPTION_SOME or RESULT_OK, depending on return type.
if let ExprKind::Path(qpath) = &func.kind;
if is_lang_ctor(cx, qpath, lang_item);
// Make sure the function argument does not contain a return expression.
if !contains_return(arg);
then {
suggs.push(
(
ret_expr.span,
if inner_type.is_unit() {
"".to_string()
} else {
snippet(cx, arg.span.source_callsite(), "..").to_string()
}
)
);
true
} else {
false
}
}
});
if can_sugg && !suggs.is_empty() {
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",
)
} 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);
});
}
}
}