unit-arg - improve suggestion
This commit is contained in:
parent
6220dff504
commit
2ecc2ac864
@ -11,8 +11,8 @@
|
||||
use rustc_hir::intravisit::{walk_body, walk_expr, walk_ty, FnKind, NestedVisitorMap, Visitor};
|
||||
use rustc_hir::{
|
||||
BinOpKind, Block, Body, Expr, ExprKind, FnDecl, FnRetTy, FnSig, GenericArg, GenericParamKind, HirId, ImplItem,
|
||||
ImplItemKind, Item, ItemKind, Lifetime, Local, MatchSource, MutTy, Mutability, QPath, Stmt, StmtKind, TraitFn,
|
||||
TraitItem, TraitItemKind, TyKind, UnOp,
|
||||
ImplItemKind, Item, ItemKind, Lifetime, Local, MatchSource, MutTy, Mutability, Node, QPath, Stmt, StmtKind,
|
||||
TraitFn, TraitItem, TraitItemKind, TyKind, UnOp,
|
||||
};
|
||||
use rustc_lint::{LateContext, LateLintPass, LintContext};
|
||||
use rustc_middle::hir::map::Map;
|
||||
@ -29,10 +29,10 @@
|
||||
use crate::consts::{constant, Constant};
|
||||
use crate::utils::paths;
|
||||
use crate::utils::{
|
||||
clip, comparisons, differing_macro_contexts, higher, in_constant, indent_of, int_bits, is_type_diagnostic_item,
|
||||
clip, comparisons, differing_macro_contexts, higher, in_constant, int_bits, is_type_diagnostic_item,
|
||||
last_path_segment, match_def_path, match_path, method_chain_args, multispan_sugg, numeric_literal::NumericLiteral,
|
||||
qpath_res, sext, snippet, snippet_block_with_applicability, snippet_opt, snippet_with_applicability,
|
||||
snippet_with_macro_callsite, span_lint, span_lint_and_help, span_lint_and_sugg, span_lint_and_then, unsext,
|
||||
qpath_res, sext, snippet, snippet_opt, snippet_with_applicability, snippet_with_macro_callsite, span_lint,
|
||||
span_lint_and_help, span_lint_and_sugg, span_lint_and_then, unsext,
|
||||
};
|
||||
|
||||
declare_clippy_lint! {
|
||||
@ -844,43 +844,54 @@ fn lint_unit_args(cx: &LateContext<'_>, expr: &Expr<'_>, args_to_recover: &[&Exp
|
||||
Applicability::MaybeIncorrect,
|
||||
);
|
||||
or = "or ";
|
||||
applicability = Applicability::MaybeIncorrect;
|
||||
});
|
||||
let sugg = args_to_recover
|
||||
|
||||
let arg_snippets: Vec<String> = args_to_recover
|
||||
.iter()
|
||||
.filter_map(|arg| snippet_opt(cx, arg.span))
|
||||
.collect();
|
||||
let arg_snippets_without_empty_blocks: Vec<String> = args_to_recover
|
||||
.iter()
|
||||
.filter(|arg| !is_empty_block(arg))
|
||||
.enumerate()
|
||||
.map(|(i, arg)| {
|
||||
let indent = if i == 0 {
|
||||
0
|
||||
} else {
|
||||
indent_of(cx, expr.span).unwrap_or(0)
|
||||
};
|
||||
format!(
|
||||
"{}{};",
|
||||
" ".repeat(indent),
|
||||
snippet_block_with_applicability(cx, arg.span, "..", Some(expr.span), &mut applicability)
|
||||
)
|
||||
})
|
||||
.collect::<Vec<String>>();
|
||||
let mut and = "";
|
||||
if !sugg.is_empty() {
|
||||
let plural = if sugg.len() > 1 { "s" } else { "" };
|
||||
db.span_suggestion(
|
||||
expr.span.with_hi(expr.span.lo()),
|
||||
&format!("{}move the expression{} in front of the call...", or, plural),
|
||||
format!("{}\n", sugg.join("\n")),
|
||||
applicability,
|
||||
);
|
||||
and = "...and "
|
||||
.filter_map(|arg| snippet_opt(cx, arg.span))
|
||||
.collect();
|
||||
|
||||
if let Some(mut sugg) = snippet_opt(cx, expr.span) {
|
||||
arg_snippets.iter().for_each(|arg| {
|
||||
sugg = sugg.replacen(arg, "()", 1);
|
||||
});
|
||||
sugg = format!("{}{}{}", arg_snippets_without_empty_blocks.join("; "), "; ", sugg);
|
||||
let parent_node = cx.tcx.hir().find(cx.tcx.hir().get_parent_node(expr.hir_id));
|
||||
if !matches!(parent_node, Some(Node::Block(_))) && !matches!(parent_node, Some(Node::Stmt(_))) {
|
||||
// expr is not in a block statement or result expression position, wrap in a block
|
||||
sugg = format!("{{ {} }}", sugg);
|
||||
}
|
||||
|
||||
if arg_snippets_without_empty_blocks.is_empty() {
|
||||
db.multipart_suggestion(
|
||||
&format!("use {}unit literal{} instead", singular, plural),
|
||||
args_to_recover
|
||||
.iter()
|
||||
.map(|arg| (arg.span, "()".to_string()))
|
||||
.collect::<Vec<_>>(),
|
||||
applicability,
|
||||
);
|
||||
} else {
|
||||
let plural = arg_snippets_without_empty_blocks.len() > 1;
|
||||
let empty_or_s = if plural { "s" } else { "" };
|
||||
let it_or_them = if plural { "them" } else { "it" };
|
||||
db.span_suggestion(
|
||||
expr.span,
|
||||
&format!(
|
||||
"{}move the expression{} in front of the call and replace {} with the unit literal `()`",
|
||||
or, empty_or_s, it_or_them
|
||||
),
|
||||
sugg,
|
||||
applicability,
|
||||
);
|
||||
}
|
||||
}
|
||||
db.multipart_suggestion(
|
||||
&format!("{}use {}unit literal{} instead", and, singular, plural),
|
||||
args_to_recover
|
||||
.iter()
|
||||
.map(|arg| (arg.span, "()".to_string()))
|
||||
.collect::<Vec<_>>(),
|
||||
applicability,
|
||||
);
|
||||
},
|
||||
);
|
||||
}
|
||||
@ -2055,6 +2066,7 @@ fn partial_cmp(&self, other: &Self) -> Option<Ordering> {
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
impl Ord for FullInt {
|
||||
#[must_use]
|
||||
fn cmp(&self, other: &Self) -> Ordering {
|
||||
|
@ -1,5 +1,5 @@
|
||||
#![warn(clippy::unit_arg)]
|
||||
#![allow(clippy::no_effect, unused_must_use, unused_variables)]
|
||||
#![allow(clippy::no_effect, unused_must_use, unused_variables, clippy::unused_unit)]
|
||||
|
||||
use std::fmt::Debug;
|
||||
|
||||
@ -47,6 +47,11 @@ fn bad() {
|
||||
foo(3);
|
||||
},
|
||||
);
|
||||
// here Some(foo(2)) isn't the top level statement expression, wrap the suggestion in a block
|
||||
None.or(Some(foo(2)));
|
||||
// in this case, the suggestion can be inlined, no need for a surrounding block
|
||||
// foo(()); foo(()) instead of { foo(()); foo(()) }
|
||||
foo(foo(()))
|
||||
}
|
||||
|
||||
fn ok() {
|
||||
|
@ -11,16 +11,12 @@ help: remove the semicolon from the last statement in the block
|
||||
|
|
||||
LL | 1
|
||||
|
|
||||
help: or move the expression in front of the call...
|
||||
help: or move the expression in front of the call and replace it with the unit literal `()`
|
||||
|
|
||||
LL | {
|
||||
LL | 1;
|
||||
LL | };
|
||||
LL | }; foo(());
|
||||
|
|
||||
help: ...and use a unit literal instead
|
||||
|
|
||||
LL | foo(());
|
||||
| ^^
|
||||
|
||||
error: passing a unit value to a function
|
||||
--> $DIR/unit_arg.rs:26:5
|
||||
@ -28,14 +24,10 @@ error: passing a unit value to a function
|
||||
LL | foo(foo(1));
|
||||
| ^^^^^^^^^^^
|
||||
|
|
||||
help: move the expression in front of the call...
|
||||
help: move the expression in front of the call and replace it with the unit literal `()`
|
||||
|
|
||||
LL | foo(1);
|
||||
|
|
||||
help: ...and use a unit literal instead
|
||||
|
|
||||
LL | foo(());
|
||||
| ^^
|
||||
LL | foo(1); foo(());
|
||||
| ^^^^^^^^^^^^^^^
|
||||
|
||||
error: passing a unit value to a function
|
||||
--> $DIR/unit_arg.rs:27:5
|
||||
@ -50,17 +42,13 @@ help: remove the semicolon from the last statement in the block
|
||||
|
|
||||
LL | foo(2)
|
||||
|
|
||||
help: or move the expression in front of the call...
|
||||
help: or move the expression in front of the call and replace it with the unit literal `()`
|
||||
|
|
||||
LL | {
|
||||
LL | foo(1);
|
||||
LL | foo(2);
|
||||
LL | };
|
||||
LL | }; foo(());
|
||||
|
|
||||
help: ...and use a unit literal instead
|
||||
|
|
||||
LL | foo(());
|
||||
| ^^
|
||||
|
||||
error: passing a unit value to a function
|
||||
--> $DIR/unit_arg.rs:32:5
|
||||
@ -74,16 +62,12 @@ help: remove the semicolon from the last statement in the block
|
||||
|
|
||||
LL | 1
|
||||
|
|
||||
help: or move the expression in front of the call...
|
||||
help: or move the expression in front of the call and replace it with the unit literal `()`
|
||||
|
|
||||
LL | {
|
||||
LL | 1;
|
||||
LL | };
|
||||
LL | }; b.bar(());
|
||||
|
|
||||
help: ...and use a unit literal instead
|
||||
|
|
||||
LL | b.bar(());
|
||||
| ^^
|
||||
|
||||
error: passing unit values to a function
|
||||
--> $DIR/unit_arg.rs:35:5
|
||||
@ -91,15 +75,10 @@ error: passing unit values to a function
|
||||
LL | taking_multiple_units(foo(0), foo(1));
|
||||
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
||||
|
|
||||
help: move the expressions in front of the call...
|
||||
help: move the expressions in front of the call and replace them with the unit literal `()`
|
||||
|
|
||||
LL | foo(0);
|
||||
LL | foo(1);
|
||||
|
|
||||
help: ...and use unit literals instead
|
||||
|
|
||||
LL | taking_multiple_units((), ());
|
||||
| ^^ ^^
|
||||
LL | foo(0); foo(1); taking_multiple_units((), ());
|
||||
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
||||
|
||||
error: passing unit values to a function
|
||||
--> $DIR/unit_arg.rs:36:5
|
||||
@ -114,18 +93,13 @@ help: remove the semicolon from the last statement in the block
|
||||
|
|
||||
LL | foo(2)
|
||||
|
|
||||
help: or move the expressions in front of the call...
|
||||
help: or move the expressions in front of the call and replace them with the unit literal `()`
|
||||
|
|
||||
LL | foo(0);
|
||||
LL | {
|
||||
LL | foo(0); {
|
||||
LL | foo(1);
|
||||
LL | foo(2);
|
||||
LL | };
|
||||
LL | }; taking_multiple_units((), ());
|
||||
|
|
||||
help: ...and use unit literals instead
|
||||
|
|
||||
LL | taking_multiple_units((), ());
|
||||
| ^^ ^^
|
||||
|
||||
error: passing unit values to a function
|
||||
--> $DIR/unit_arg.rs:40:5
|
||||
@ -147,35 +121,56 @@ help: remove the semicolon from the last statement in the block
|
||||
|
|
||||
LL | foo(3)
|
||||
|
|
||||
help: or move the expressions in front of the call...
|
||||
help: or move the expressions in front of the call and replace them with the unit literal `()`
|
||||
|
|
||||
LL | {
|
||||
LL | foo(0);
|
||||
LL | foo(1);
|
||||
LL | };
|
||||
LL | {
|
||||
LL | foo(2);
|
||||
LL | foo(0);
|
||||
LL | foo(1);
|
||||
LL | }; {
|
||||
LL | foo(2);
|
||||
LL | foo(3);
|
||||
...
|
||||
help: ...and use unit literals instead
|
||||
|
||||
error: use of `or` followed by a function call
|
||||
--> $DIR/unit_arg.rs:51:10
|
||||
|
|
||||
LL | (),
|
||||
LL | (),
|
||||
LL | None.or(Some(foo(2)));
|
||||
| ^^^^^^^^^^^^^^^^ help: try this: `or_else(|| Some(foo(2)))`
|
||||
|
|
||||
= note: `-D clippy::or-fun-call` implied by `-D warnings`
|
||||
|
||||
error: passing a unit value to a function
|
||||
--> $DIR/unit_arg.rs:51:13
|
||||
|
|
||||
LL | None.or(Some(foo(2)));
|
||||
| ^^^^^^^^^^^^
|
||||
|
|
||||
help: move the expression in front of the call and replace it with the unit literal `()`
|
||||
|
|
||||
LL | None.or({ foo(2); Some(()) });
|
||||
| ^^^^^^^^^^^^^^^^^^^^
|
||||
|
||||
error: passing a unit value to a function
|
||||
--> $DIR/unit_arg.rs:54:5
|
||||
|
|
||||
LL | foo(foo(()))
|
||||
| ^^^^^^^^^^^^
|
||||
|
|
||||
help: move the expression in front of the call and replace it with the unit literal `()`
|
||||
|
|
||||
LL | foo(()); foo(())
|
||||
|
|
||||
|
||||
error: passing a unit value to a function
|
||||
--> $DIR/unit_arg.rs:82:5
|
||||
--> $DIR/unit_arg.rs:87:5
|
||||
|
|
||||
LL | Some(foo(1))
|
||||
| ^^^^^^^^^^^^
|
||||
|
|
||||
help: move the expression in front of the call...
|
||||
help: move the expression in front of the call and replace it with the unit literal `()`
|
||||
|
|
||||
LL | foo(1);
|
||||
LL | foo(1); Some(())
|
||||
|
|
||||
help: ...and use a unit literal instead
|
||||
|
|
||||
LL | Some(())
|
||||
| ^^
|
||||
|
||||
error: aborting due to 8 previous errors
|
||||
error: aborting due to 11 previous errors
|
||||
|
||||
|
@ -22,14 +22,10 @@ error: passing unit values to a function
|
||||
LL | taking_two_units({}, foo(0));
|
||||
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
||||
|
|
||||
help: move the expression in front of the call...
|
||||
help: move the expression in front of the call and replace it with the unit literal `()`
|
||||
|
|
||||
LL | foo(0);
|
||||
|
|
||||
help: ...and use unit literals instead
|
||||
|
|
||||
LL | taking_two_units((), ());
|
||||
| ^^ ^^
|
||||
LL | foo(0); taking_two_units((), ());
|
||||
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
||||
|
||||
error: passing unit values to a function
|
||||
--> $DIR/unit_arg_empty_blocks.rs:18:5
|
||||
@ -37,15 +33,10 @@ error: passing unit values to a function
|
||||
LL | taking_three_units({}, foo(0), foo(1));
|
||||
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
||||
|
|
||||
help: move the expressions in front of the call...
|
||||
help: move the expressions in front of the call and replace them with the unit literal `()`
|
||||
|
|
||||
LL | foo(0);
|
||||
LL | foo(1);
|
||||
|
|
||||
help: ...and use unit literals instead
|
||||
|
|
||||
LL | taking_three_units((), (), ());
|
||||
| ^^ ^^ ^^
|
||||
LL | foo(0); foo(1); taking_three_units((), (), ());
|
||||
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
||||
|
||||
error: aborting due to 4 previous errors
|
||||
|
||||
|
Loading…
Reference in New Issue
Block a user