Skip to content

Commit

Permalink
unit-arg - pr comments
Browse files Browse the repository at this point in the history
  • Loading branch information
@tnielens
tnielens committed Aug 26, 2020
1 parent 2ecc2ac commit d127d3e
Show file tree
Hide file tree
Showing 4 changed files with 84 additions and 49 deletions.
41 changes: 31 additions & 10 deletions clippy_lints/src/types.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,11 +25,12 @@ use rustc_span::symbol::sym;
use rustc_target::abi::LayoutOf;
use rustc_target::spec::abi::Abi;
use rustc_typeck::hir_ty_to_ty;
use std::iter;

use crate::consts::{constant, Constant};
use crate::utils::paths;
use crate::utils::{
clip, comparisons, differing_macro_contexts, higher, in_constant, int_bits, is_type_diagnostic_item,
clip, comparisons, differing_macro_contexts, higher, in_constant, indent_of, 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_opt, snippet_with_applicability, snippet_with_macro_callsite, span_lint,
span_lint_and_help, span_lint_and_sugg, span_lint_and_then, unsext,
Expand Down Expand Up @@ -802,6 +803,7 @@ impl<'tcx> LateLintPass<'tcx> for UnitArg {
}
}

#[allow(clippy::too_many_lines)]
fn lint_unit_args(cx: &LateContext<'_>, expr: &Expr<'_>, args_to_recover: &[&Expr<'_>]) {
let mut applicability = Applicability::MachineApplicable;
let (singular, plural) = if args_to_recover.len() > 1 {
Expand Down Expand Up @@ -856,17 +858,36 @@ fn lint_unit_args(cx: &LateContext<'_>, expr: &Expr<'_>, args_to_recover: &[&Exp
.filter(|arg| !is_empty_block(arg))
.filter_map(|arg| snippet_opt(cx, arg.span))
.collect();
let indent = indent_of(cx, expr.span).unwrap_or(0);

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);
if let Some(expr_str) = snippet_opt(cx, expr.span) {
let expr_with_replacements = arg_snippets
.iter()
.fold(expr_str, |acc, arg| acc.replacen(arg, "()", 1));
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);
}
let sugg =
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
let indent_str = iter::repeat(' ').take(indent).collect::<String>();
let inner_indent_str = iter::repeat(' ').take(indent + 4).collect::<String>();
let mut block_content: Vec<String> = arg_snippets_without_empty_blocks.clone();
block_content.push(expr_with_replacements);
block_content = block_content
.iter()
.map(|item| inner_indent_str.clone() + item)
.collect();
let block_content_str = block_content.join(";\n");
format!("{{\n{}\n{}}}", &block_content_str, &indent_str)
} else {
let indent_str = iter::repeat(' ').take(indent).collect::<String>();
let join_str = format!(";\n{}", indent_str);
format!(
"{}{}{}",
arg_snippets_without_empty_blocks.join(&join_str),
&join_str,
&expr_with_replacements
)
};

if arg_snippets_without_empty_blocks.is_empty() {
db.multipart_suggestion(
Expand Down
8 changes: 7 additions & 1 deletion tests/ui/unit_arg.rs
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,11 @@
#![warn(clippy::unit_arg)]
#![allow(clippy::no_effect, unused_must_use, unused_variables, clippy::unused_unit)]
#![allow(
clippy::no_effect,
unused_must_use,
unused_variables,
clippy::unused_unit,
clippy::or_fun_call
)]

use std::fmt::Debug;

Expand Down
73 changes: 39 additions & 34 deletions tests/ui/unit_arg.stderr
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
error: passing a unit value to a function
--> $DIR/unit_arg.rs:23:5
--> $DIR/unit_arg.rs:29:5
|
LL | / foo({
LL | | 1;
Expand All @@ -15,22 +15,24 @@ help: or move the expression in front of the call and replace it with the unit l
|
LL | {
LL | 1;
LL | }; foo(());
LL | };
LL | foo(());
|

error: passing a unit value to a function
--> $DIR/unit_arg.rs:26:5
--> $DIR/unit_arg.rs:32:5
|
LL | foo(foo(1));
| ^^^^^^^^^^^
|
help: move the expression in front of the call and replace it with the unit literal `()`
|
LL | foo(1); foo(());
| ^^^^^^^^^^^^^^^
LL | foo(1);
LL | foo(());
|

error: passing a unit value to a function
--> $DIR/unit_arg.rs:27:5
--> $DIR/unit_arg.rs:33:5
|
LL | / foo({
LL | | foo(1);
Expand All @@ -47,11 +49,12 @@ help: or move the expression in front of the call and replace it with the unit l
LL | {
LL | foo(1);
LL | foo(2);
LL | }; foo(());
LL | };
LL | foo(());
|

error: passing a unit value to a function
--> $DIR/unit_arg.rs:32:5
--> $DIR/unit_arg.rs:38:5
|
LL | / b.bar({
LL | | 1;
Expand All @@ -66,22 +69,25 @@ help: or move the expression in front of the call and replace it with the unit l
|
LL | {
LL | 1;
LL | }; b.bar(());
LL | };
LL | b.bar(());
|

error: passing unit values to a function
--> $DIR/unit_arg.rs:35:5
--> $DIR/unit_arg.rs:41:5
|
LL | taking_multiple_units(foo(0), foo(1));
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
help: move the expressions in front of the call and replace them with the unit literal `()`
|
LL | foo(0); foo(1); taking_multiple_units((), ());
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
LL | foo(0);
LL | foo(1);
LL | taking_multiple_units((), ());
|

error: passing unit values to a function
--> $DIR/unit_arg.rs:36:5
--> $DIR/unit_arg.rs:42:5
|
LL | / taking_multiple_units(foo(0), {
LL | | foo(1);
Expand All @@ -95,14 +101,16 @@ LL | foo(2)
|
help: or move the expressions in front of the call and replace them with the unit literal `()`
|
LL | foo(0); {
LL | foo(0);
LL | {
LL | foo(1);
LL | foo(2);
LL | }; taking_multiple_units((), ());
LL | };
LL | taking_multiple_units((), ());
|

error: passing unit values to a function
--> $DIR/unit_arg.rs:40:5
--> $DIR/unit_arg.rs:46:5
|
LL | / taking_multiple_units(
LL | | {
Expand All @@ -126,51 +134,48 @@ help: or move the expressions in front of the call and replace them with the uni
LL | {
LL | foo(0);
LL | foo(1);
LL | }; {
LL | };
LL | {
LL | foo(2);
LL | foo(3);
...

error: use of `or` followed by a function call
--> $DIR/unit_arg.rs:51:10
|
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
--> $DIR/unit_arg.rs:57: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(()) });
| ^^^^^^^^^^^^^^^^^^^^
LL | None.or({
LL | foo(2);
LL | Some(())
LL | });
|

error: passing a unit value to a function
--> $DIR/unit_arg.rs:54:5
--> $DIR/unit_arg.rs:60:5
|
LL | foo(foo(()))
| ^^^^^^^^^^^^
|
help: move the expression in front of the call and replace it with the unit literal `()`
|
LL | foo(()); foo(())
LL | foo(());
LL | foo(())
|

error: passing a unit value to a function
--> $DIR/unit_arg.rs:87:5
--> $DIR/unit_arg.rs:93:5
|
LL | Some(foo(1))
| ^^^^^^^^^^^^
|
help: move the expression in front of the call and replace it with the unit literal `()`
|
LL | foo(1); Some(())
LL | foo(1);
LL | Some(())
|

error: aborting due to 11 previous errors
error: aborting due to 10 previous errors

11 changes: 7 additions & 4 deletions tests/ui/unit_arg_empty_blocks.stderr
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,9 @@ LL | taking_two_units({}, foo(0));
|
help: move the expression in front of the call and replace it with the unit literal `()`
|
LL | foo(0); taking_two_units((), ());
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
LL | foo(0);
LL | taking_two_units((), ());
|

error: passing unit values to a function
--> $DIR/unit_arg_empty_blocks.rs:18:5
Expand All @@ -35,8 +36,10 @@ LL | taking_three_units({}, foo(0), foo(1));
|
help: move the expressions in front of the call and replace them with the unit literal `()`
|
LL | foo(0); foo(1); taking_three_units((), (), ());
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
LL | foo(0);
LL | foo(1);
LL | taking_three_units((), (), ());
|

error: aborting due to 4 previous errors

0 comments on commit d127d3e

Please sign in to comment.