mikros/rust
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Auto merge of #6889 - Y-Nak:refactor-unit-types, r=flip1995

Browse Source 
Refactor unit types

Ref:  #6724
r? `@flip1995`

Changes:
1. Extract `unit_types` from `types` group.
2. Move lints of `unit_types` to their own modules.

Notes:
Other lints of `unit_types` is still scattered around the `clippy_lints`, e.g. `result_unit_err` or `option_map_unit_fn`.
These should be addressed in another PR.

changelog: none
This commit is contained in:
bors 2021-03-16 02:57:33 +00:00
commit c701c301d4
7 changed files with 433 additions and 402 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

21
clippy_lints/src/lib.rs
View File

@ -350,6 +350,7 @@ mod types;
mod undropped_manually_drops;
mod unicode;
mod unit_return_expecting_ord;
mod unit_types;
mod unnamed_address;
mod unnecessary_sort_by;
mod unnecessary_wraps;
@ -960,20 +961,20 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
&types::BOX_VEC,
&types::IMPLICIT_HASHER,
&types::INVALID_UPCAST_COMPARISONS,
&types::LET_UNIT_VALUE,
&types::LINKEDLIST,
&types::OPTION_OPTION,
&types::RC_BUFFER,
&types::REDUNDANT_ALLOCATION,
&types::TYPE_COMPLEXITY,
&types::UNIT_ARG,
&types::UNIT_CMP,
&types::VEC_BOX,
&undropped_manually_drops::UNDROPPED_MANUALLY_DROPS,
&unicode::INVISIBLE_CHARACTERS,
&unicode::NON_ASCII_LITERAL,
&unicode::UNICODE_NOT_NFC,
&unit_return_expecting_ord::UNIT_RETURN_EXPECTING_ORD,
&unit_types::LET_UNIT_VALUE,
&unit_types::UNIT_ARG,
&unit_types::UNIT_CMP,
&unnamed_address::FN_ADDRESS_COMPARISONS,
&unnamed_address::VTABLE_ADDRESS_COMPARISONS,
&unnecessary_sort_by::UNNECESSARY_SORT_BY,
@ -1084,8 +1085,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
store.register_late_pass(|| box map_clone::MapClone);
store.register_late_pass(|| box map_err_ignore::MapErrIgnore);
store.register_late_pass(|| box shadow::Shadow);
store.register_late_pass(|| box types::LetUnitValue);
store.register_late_pass(|| box types::UnitCmp);
store.register_late_pass(|| box unit_types::UnitTypes);
store.register_late_pass(|| box loops::Loops);
store.register_late_pass(|| box main_recursion::MainRecursion::default());
store.register_late_pass(|| box lifetimes::Lifetimes);
@ -1160,7 +1160,6 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
store.register_late_pass(|| box useless_conversion::UselessConversion::default());
store.register_late_pass(|| box types::ImplicitHasher);
store.register_late_pass(|| box fallible_impl_from::FallibleImplFrom);
store.register_late_pass(|| box types::UnitArg);
store.register_late_pass(|| box double_comparison::DoubleComparisons);
store.register_late_pass(|| box question_mark::QuestionMark);
store.register_early_pass(|| box suspicious_operation_groupings::SuspiciousOperationGroupings);
@ -1415,11 +1414,11 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
LintId::of(&trait_bounds::TYPE_REPETITION_IN_BOUNDS),
LintId::of(&types::IMPLICIT_HASHER),
LintId::of(&types::INVALID_UPCAST_COMPARISONS),
LintId::of(&types::LET_UNIT_VALUE),
LintId::of(&types::LINKEDLIST),
LintId::of(&types::OPTION_OPTION),
LintId::of(&unicode::NON_ASCII_LITERAL),
LintId::of(&unicode::UNICODE_NOT_NFC),
LintId::of(&unit_types::LET_UNIT_VALUE),
LintId::of(&unnecessary_wraps::UNNECESSARY_WRAPS),
LintId::of(&unnested_or_patterns::UNNESTED_OR_PATTERNS),
LintId::of(&unused_self::UNUSED_SELF),
@ -1708,12 +1707,12 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
LintId::of(&types::BOX_VEC),
LintId::of(&types::REDUNDANT_ALLOCATION),
LintId::of(&types::TYPE_COMPLEXITY),
LintId::of(&types::UNIT_ARG),
LintId::of(&types::UNIT_CMP),
LintId::of(&types::VEC_BOX),
LintId::of(&undropped_manually_drops::UNDROPPED_MANUALLY_DROPS),
LintId::of(&unicode::INVISIBLE_CHARACTERS),
LintId::of(&unit_return_expecting_ord::UNIT_RETURN_EXPECTING_ORD),
LintId::of(&unit_types::UNIT_ARG),
LintId::of(&unit_types::UNIT_CMP),
LintId::of(&unnamed_address::FN_ADDRESS_COMPARISONS),
LintId::of(&unnamed_address::VTABLE_ADDRESS_COMPARISONS),
LintId::of(&unnecessary_sort_by::UNNECESSARY_SORT_BY),
@ -1934,8 +1933,8 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
LintId::of(&transmute::TRANSMUTE_PTR_TO_REF),
LintId::of(&types::BORROWED_BOX),
LintId::of(&types::TYPE_COMPLEXITY),
LintId::of(&types::UNIT_ARG),
LintId::of(&types::VEC_BOX),
LintId::of(&unit_types::UNIT_ARG),
LintId::of(&unnecessary_sort_by::UNNECESSARY_SORT_BY),
LintId::of(&unwrap::UNNECESSARY_UNWRAP),
LintId::of(&useless_conversion::USELESS_CONVERSION),
@ -2005,10 +2004,10 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
LintId::of(&transmute::WRONG_TRANSMUTE),
LintId::of(&transmuting_null::TRANSMUTING_NULL),
LintId::of(&types::ABSURD_EXTREME_COMPARISONS),
LintId::of(&types::UNIT_CMP),
LintId::of(&undropped_manually_drops::UNDROPPED_MANUALLY_DROPS),
LintId::of(&unicode::INVISIBLE_CHARACTERS),
LintId::of(&unit_return_expecting_ord::UNIT_RETURN_EXPECTING_ORD),
LintId::of(&unit_types::UNIT_CMP),
LintId::of(&unnamed_address::FN_ADDRESS_COMPARISONS),
LintId::of(&unnamed_address::VTABLE_ADDRESS_COMPARISONS),
LintId::of(&unused_io_amount::UNUSED_IO_AMOUNT),

396
clippy_lints/src/types/mod.rs
View File

@ -14,23 +14,21 @@ use std::cmp::Ordering;
use std::collections::BTreeMap;
use clippy_utils::diagnostics::{multispan_sugg, span_lint, span_lint_and_help, span_lint_and_then};
use clippy_utils::source::{indent_of, reindent_multiline, snippet, snippet_opt, snippet_with_macro_callsite};
use clippy_utils::source::{snippet, snippet_opt};
use clippy_utils::ty::{is_isize_or_usize, is_type_diagnostic_item};
use if_chain::if_chain;
use rustc_errors::{Applicability, DiagnosticBuilder};
use rustc_errors::DiagnosticBuilder;
use rustc_hir as hir;
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, Local, MatchSource, MutTy, Node, QPath, Stmt, StmtKind, TraitFn, TraitItem,
TraitItemKind, TyKind,
BinOpKind, Body, Expr, ExprKind, FnDecl, FnRetTy, FnSig, GenericArg, GenericParamKind, HirId, ImplItem,
ImplItemKind, Item, ItemKind, Local, MutTy, QPath, TraitFn, TraitItem, TraitItemKind, TyKind,
};
use rustc_lint::{LateContext, LateLintPass, LintContext};
use rustc_middle::hir::map::Map;
use rustc_middle::lint::in_external_macro;
use rustc_middle::ty::{self, IntTy, Ty, TyS, TypeckResults, UintTy};
use rustc_session::{declare_lint_pass, declare_tool_lint, impl_lint_pass};
use rustc_span::hygiene::{ExpnKind, MacroKind};
use rustc_span::source_map::Span;
use rustc_span::symbol::sym;
use rustc_target::abi::LayoutOf;
@ -39,7 +37,7 @@ use rustc_typeck::hir_ty_to_ty;
use crate::consts::{constant, Constant};
use crate::utils::paths;
use crate::utils::{clip, comparisons, differing_macro_contexts, higher, int_bits, match_path, sext, unsext};
use crate::utils::{clip, comparisons, differing_macro_contexts, int_bits, match_path, sext, unsext};
declare_clippy_lint! {
/// **What it does:** Checks for use of `Box<Vec<_>>` anywhere in the code.
@ -389,390 +387,6 @@ impl Types {
}
}
declare_clippy_lint! {
/// **What it does:** Checks for binding a unit value.
///
/// **Why is this bad?** A unit value cannot usefully be used anywhere. So
/// binding one is kind of pointless.
///
/// **Known problems:** None.
///
/// **Example:**
/// ```rust
/// let x = {
/// 1;
/// };
/// ```
pub LET_UNIT_VALUE,
pedantic,
"creating a `let` binding to a value of unit type, which usually can't be used afterwards"
}
declare_lint_pass!(LetUnitValue => [LET_UNIT_VALUE]);
impl<'tcx> LateLintPass<'tcx> for LetUnitValue {
fn check_stmt(&mut self, cx: &LateContext<'tcx>, stmt: &'tcx Stmt<'_>) {
if let StmtKind::Local(ref local) = stmt.kind {
if is_unit(cx.typeck_results().pat_ty(&local.pat)) {
if in_external_macro(cx.sess(), stmt.span) || local.pat.span.from_expansion() {
return;
}
if higher::is_from_for_desugar(local) {
return;
}
span_lint_and_then(
cx,
LET_UNIT_VALUE,
stmt.span,
"this let-binding has unit value",
|diag| {
if let Some(expr) = &local.init {
let snip = snippet_with_macro_callsite(cx, expr.span, "()");
diag.span_suggestion(
stmt.span,
"omit the `let` binding",
format!("{};", snip),
Applicability::MachineApplicable, // snippet
);
}
},
);
}
}
}
}
declare_clippy_lint! {
/// **What it does:** Checks for comparisons to unit. This includes all binary
/// comparisons (like `==` and `<`) and asserts.
///
/// **Why is this bad?** Unit is always equal to itself, and thus is just a
/// clumsily written constant. Mostly this happens when someone accidentally
/// adds semicolons at the end of the operands.
///
/// **Known problems:** None.
///
/// **Example:**
/// ```rust
/// # fn foo() {};
/// # fn bar() {};
/// # fn baz() {};
/// if {
/// foo();
/// } == {
/// bar();
/// } {
/// baz();
/// }
/// ```
/// is equal to
/// ```rust
/// # fn foo() {};
/// # fn bar() {};
/// # fn baz() {};
/// {
/// foo();
/// bar();
/// baz();
/// }
/// ```
///
/// For asserts:
/// ```rust
/// # fn foo() {};
/// # fn bar() {};
/// assert_eq!({ foo(); }, { bar(); });
/// ```
/// will always succeed
pub UNIT_CMP,
correctness,
"comparing unit values"
}
declare_lint_pass!(UnitCmp => [UNIT_CMP]);
impl<'tcx> LateLintPass<'tcx> for UnitCmp {
fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) {
if expr.span.from_expansion() {
if let Some(callee) = expr.span.source_callee() {
if let ExpnKind::Macro(MacroKind::Bang, symbol) = callee.kind {
if let ExprKind::Binary(ref cmp, ref left, _) = expr.kind {
let op = cmp.node;
if op.is_comparison() && is_unit(cx.typeck_results().expr_ty(left)) {
let result = match &*symbol.as_str() {
"assert_eq" | "debug_assert_eq" => "succeed",
"assert_ne" | "debug_assert_ne" => "fail",
_ => return,
};
span_lint(
cx,
UNIT_CMP,
expr.span,
&format!(
"`{}` of unit values detected. This will always {}",
symbol.as_str(),
result
),
);
}
}
}
}
return;
}
if let ExprKind::Binary(ref cmp, ref left, _) = expr.kind {
let op = cmp.node;
if op.is_comparison() && is_unit(cx.typeck_results().expr_ty(left)) {
let result = match op {
BinOpKind::Eq | BinOpKind::Le | BinOpKind::Ge => "true",
_ => "false",
};
span_lint(
cx,
UNIT_CMP,
expr.span,
&format!(
"{}-comparison of unit values detected. This will always be {}",
op.as_str(),
result
),
);
}
}
}
}
declare_clippy_lint! {
/// **What it does:** Checks for passing a unit value as an argument to a function without using a
/// unit literal (`()`).
///
/// **Why is this bad?** This is likely the result of an accidental semicolon.
///
/// **Known problems:** None.
///
/// **Example:**
/// ```rust,ignore
/// foo({
/// let a = bar();
/// baz(a);
/// })
/// ```
pub UNIT_ARG,
complexity,
"passing unit to a function"
}
declare_lint_pass!(UnitArg => [UNIT_ARG]);
impl<'tcx> LateLintPass<'tcx> for UnitArg {
fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
if expr.span.from_expansion() {
return;
}
// apparently stuff in the desugaring of `?` can trigger this
// so check for that here
// only the calls to `Try::from_error` is marked as desugared,
// so we need to check both the current Expr and its parent.
if is_questionmark_desugar_marked_call(expr) {
return;
}
if_chain! {
let map = &cx.tcx.hir();
let opt_parent_node = map.find(map.get_parent_node(expr.hir_id));
if let Some(hir::Node::Expr(parent_expr)) = opt_parent_node;
if is_questionmark_desugar_marked_call(parent_expr);
then {
return;
}
}
match expr.kind {
ExprKind::Call(_, args) | ExprKind::MethodCall(_, _, args, _) => {
let args_to_recover = args
.iter()
.filter(|arg| {
if is_unit(cx.typeck_results().expr_ty(arg)) && !is_unit_literal(arg) {
!matches!(
&arg.kind,
ExprKind::Match(.., MatchSource::TryDesugar) | ExprKind::Path(..)
)
} else {
false
}
})
.collect::<Vec<_>>();
if !args_to_recover.is_empty() {
lint_unit_args(cx, expr, &args_to_recover);
}
},
_ => (),
}
}
}
fn fmt_stmts_and_call(
cx: &LateContext<'_>,
call_expr: &Expr<'_>,
call_snippet: &str,
args_snippets: &[impl AsRef<str>],
non_empty_block_args_snippets: &[impl AsRef<str>],
) -> String {
let call_expr_indent = indent_of(cx, call_expr.span).unwrap_or(0);
let call_snippet_with_replacements = args_snippets
.iter()
.fold(call_snippet.to_owned(), |acc, arg| acc.replacen(arg.as_ref(), "()", 1));
let mut stmts_and_call = non_empty_block_args_snippets
.iter()
.map(|it| it.as_ref().to_owned())
.collect::<Vec<_>>();
stmts_and_call.push(call_snippet_with_replacements);
stmts_and_call = stmts_and_call
.into_iter()
.map(|v| reindent_multiline(v.into(), true, Some(call_expr_indent)).into_owned())
.collect();
let mut stmts_and_call_snippet = stmts_and_call.join(&format!("{}{}", ";\n", " ".repeat(call_expr_indent)));
// expr is not in a block statement or result expression position, wrap in a block
let parent_node = cx.tcx.hir().find(cx.tcx.hir().get_parent_node(call_expr.hir_id));
if !matches!(parent_node, Some(Node::Block(_))) && !matches!(parent_node, Some(Node::Stmt(_))) {
let block_indent = call_expr_indent + 4;
stmts_and_call_snippet =
reindent_multiline(stmts_and_call_snippet.into(), true, Some(block_indent)).into_owned();
stmts_and_call_snippet = format!(
"{{\n{}{}\n{}}}",
" ".repeat(block_indent),
&stmts_and_call_snippet,
" ".repeat(call_expr_indent)
);
}
stmts_and_call_snippet
}
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 {
("", "s")
} else {
("a ", "")
};
span_lint_and_then(
cx,
UNIT_ARG,
expr.span,
&format!("passing {}unit value{} to a function", singular, plural),
|db| {
let mut or = "";
args_to_recover
.iter()
.filter_map(|arg| {
if_chain! {
if let ExprKind::Block(block, _) = arg.kind;
if block.expr.is_none();
if let Some(last_stmt) = block.stmts.iter().last();
if let StmtKind::Semi(last_expr) = last_stmt.kind;
if let Some(snip) = snippet_opt(cx, last_expr.span);
then {
Some((
last_stmt.span,
snip,
))
}
else {
None
}
}
})
.for_each(|(span, sugg)| {
db.span_suggestion(
span,
"remove the semicolon from the last statement in the block",
sugg,
Applicability::MaybeIncorrect,
);
or = "or ";
applicability = Applicability::MaybeIncorrect;
});
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))
.filter_map(|arg| snippet_opt(cx, arg.span))
.collect();
if let Some(call_snippet) = snippet_opt(cx, expr.span) {
let sugg = fmt_stmts_and_call(
cx,
expr,
&call_snippet,
&arg_snippets,
&arg_snippets_without_empty_blocks,
);
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,
);
}
}
},
);
}
fn is_empty_block(expr: &Expr<'_>) -> bool {
matches!(
expr.kind,
ExprKind::Block(
Block {
stmts: &[],
expr: None,
..
},
_,
)
)
}
fn is_questionmark_desugar_marked_call(expr: &Expr<'_>) -> bool {
use rustc_span::hygiene::DesugaringKind;
if let ExprKind::Call(ref callee, _) = expr.kind {
callee.span.is_desugaring(DesugaringKind::QuestionMark)
} else {
false
}
}
fn is_unit(ty: Ty<'_>) -> bool {
matches!(ty.kind(), ty::Tuple(slice) if slice.is_empty())
}
fn is_unit_literal(expr: &Expr<'_>) -> bool {
matches!(expr.kind, ExprKind::Tup(ref slice) if slice.is_empty())
}
declare_clippy_lint! {
/// **What it does:** Checks for types used in structs, parameters and `let`
/// declarations above a certain complexity threshold.

40
clippy_lints/src/unit_types/let_unit_value.rs Normal file
View File

@ -0,0 +1,40 @@
use rustc_errors::Applicability;
use rustc_hir::{Stmt, StmtKind};
use rustc_lint::{LateContext, LintContext};
use rustc_middle::lint::in_external_macro;
use crate::utils::diagnostics::span_lint_and_then;
use crate::utils::higher;
use crate::utils::source::snippet_with_macro_callsite;
use super::LET_UNIT_VALUE;
pub(super) fn check(cx: &LateContext<'_>, stmt: &Stmt<'_>) {
if let StmtKind::Local(ref local) = stmt.kind {
if cx.typeck_results().pat_ty(&local.pat).is_unit() {
if in_external_macro(cx.sess(), stmt.span) || local.pat.span.from_expansion() {
return;
}
if higher::is_from_for_desugar(local) {
return;
}
span_lint_and_then(
cx,
LET_UNIT_VALUE,
stmt.span,
"this let-binding has unit value",
|diag| {
if let Some(expr) = &local.init {
let snip = snippet_with_macro_callsite(cx, expr.span, "()");
diag.span_suggestion(
stmt.span,
"omit the `let` binding",
format!("{};", snip),
Applicability::MachineApplicable, // snippet
);
}
},
);
}
}
}

107
clippy_lints/src/unit_types/mod.rs Normal file
View File

@ -0,0 +1,107 @@
mod let_unit_value;
mod unit_arg;
mod unit_cmp;
mod utils;
use rustc_hir::{Expr, Stmt};
use rustc_lint::{LateContext, LateLintPass};
use rustc_session::{declare_lint_pass, declare_tool_lint};
declare_clippy_lint! {
/// **What it does:** Checks for binding a unit value.
///
/// **Why is this bad?** A unit value cannot usefully be used anywhere. So
/// binding one is kind of pointless.
///
/// **Known problems:** None.
///
/// **Example:**
/// ```rust
/// let x = {
/// 1;
/// };
/// ```
pub LET_UNIT_VALUE,
pedantic,
"creating a `let` binding to a value of unit type, which usually can't be used afterwards"
}
declare_clippy_lint! {
/// **What it does:** Checks for comparisons to unit. This includes all binary
/// comparisons (like `==` and `<`) and asserts.
///
/// **Why is this bad?** Unit is always equal to itself, and thus is just a
/// clumsily written constant. Mostly this happens when someone accidentally
/// adds semicolons at the end of the operands.
///
/// **Known problems:** None.
///
/// **Example:**
/// ```rust
/// # fn foo() {};
/// # fn bar() {};
/// # fn baz() {};
/// if {
/// foo();
/// } == {
/// bar();
/// } {
/// baz();
/// }
/// ```
/// is equal to
/// ```rust
/// # fn foo() {};
/// # fn bar() {};
/// # fn baz() {};
/// {
/// foo();
/// bar();
/// baz();
/// }
/// ```
///
/// For asserts:
/// ```rust
/// # fn foo() {};
/// # fn bar() {};
/// assert_eq!({ foo(); }, { bar(); });
/// ```
/// will always succeed
pub UNIT_CMP,
correctness,
"comparing unit values"
}
declare_clippy_lint! {
/// **What it does:** Checks for passing a unit value as an argument to a function without using a
/// unit literal (`()`).
///
/// **Why is this bad?** This is likely the result of an accidental semicolon.
///
/// **Known problems:** None.
///
/// **Example:**
/// ```rust,ignore
/// foo({
/// let a = bar();
/// baz(a);
/// })
/// ```
pub UNIT_ARG,
complexity,
"passing unit to a function"
}
declare_lint_pass!(UnitTypes => [LET_UNIT_VALUE, UNIT_CMP, UNIT_ARG]);
impl LateLintPass<'_> for UnitTypes {
fn check_stmt(&mut self, cx: &LateContext<'_>, stmt: &Stmt<'_>) {
let_unit_value::check(cx, stmt);
}
fn check_expr(&mut self, cx: &LateContext<'_>, expr: &Expr<'_>) {
unit_cmp::check(cx, expr);
unit_arg::check(cx, expr);
}
}

209
clippy_lints/src/unit_types/unit_arg.rs Normal file
View File

@ -0,0 +1,209 @@
use rustc_errors::Applicability;
use rustc_hir::{self as hir, Block, Expr, ExprKind, MatchSource, Node, StmtKind};
use rustc_lint::LateContext;
use if_chain::if_chain;
use crate::utils::diagnostics::span_lint_and_then;
use crate::utils::source::{indent_of, reindent_multiline, snippet_opt};
use super::{utils, UNIT_ARG};
pub(super) fn check(cx: &LateContext<'_>, expr: &Expr<'_>) {
if expr.span.from_expansion() {
return;
}
// apparently stuff in the desugaring of `?` can trigger this
// so check for that here
// only the calls to `Try::from_error` is marked as desugared,
// so we need to check both the current Expr and its parent.
if is_questionmark_desugar_marked_call(expr) {
return;
}
if_chain! {
let map = &cx.tcx.hir();
let opt_parent_node = map.find(map.get_parent_node(expr.hir_id));
if let Some(hir::Node::Expr(parent_expr)) = opt_parent_node;
if is_questionmark_desugar_marked_call(parent_expr);
then {
return;
}
}
match expr.kind {
ExprKind::Call(_, args) | ExprKind::MethodCall(_, _, args, _) => {
let args_to_recover = args
.iter()
.filter(|arg| {
if cx.typeck_results().expr_ty(arg).is_unit() && !utils::is_unit_literal(arg) {
!matches!(
&arg.kind,
ExprKind::Match(.., MatchSource::TryDesugar) | ExprKind::Path(..)
)
} else {
false
}
})
.collect::<Vec<_>>();
if !args_to_recover.is_empty() {
lint_unit_args(cx, expr, &args_to_recover);
}
},
_ => (),
}
}
fn is_questionmark_desugar_marked_call(expr: &Expr<'_>) -> bool {
use rustc_span::hygiene::DesugaringKind;
if let ExprKind::Call(ref callee, _) = expr.kind {
callee.span.is_desugaring(DesugaringKind::QuestionMark)
} else {
false
}
}
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 {
("", "s")
} else {
("a ", "")
};
span_lint_and_then(
cx,
UNIT_ARG,
expr.span,
&format!("passing {}unit value{} to a function", singular, plural),
|db| {
let mut or = "";
args_to_recover
.iter()
.filter_map(|arg| {
if_chain! {
if let ExprKind::Block(block, _) = arg.kind;
if block.expr.is_none();
if let Some(last_stmt) = block.stmts.iter().last();
if let StmtKind::Semi(last_expr) = last_stmt.kind;
if let Some(snip) = snippet_opt(cx, last_expr.span);
then {
Some((
last_stmt.span,
snip,
))
}
else {
None
}
}
})
.for_each(|(span, sugg)| {
db.span_suggestion(
span,
"remove the semicolon from the last statement in the block",
sugg,
Applicability::MaybeIncorrect,
);
or = "or ";
applicability = Applicability::MaybeIncorrect;
});
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))
.filter_map(|arg| snippet_opt(cx, arg.span))
.collect();
if let Some(call_snippet) = snippet_opt(cx, expr.span) {
let sugg = fmt_stmts_and_call(
cx,
expr,
&call_snippet,
&arg_snippets,
&arg_snippets_without_empty_blocks,
);
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,
);
}
}
},
);
}
fn is_empty_block(expr: &Expr<'_>) -> bool {
matches!(
expr.kind,
ExprKind::Block(
Block {
stmts: &[],
expr: None,
..
},
_,
)
)
}
fn fmt_stmts_and_call(
cx: &LateContext<'_>,
call_expr: &Expr<'_>,
call_snippet: &str,
args_snippets: &[impl AsRef<str>],
non_empty_block_args_snippets: &[impl AsRef<str>],
) -> String {
let call_expr_indent = indent_of(cx, call_expr.span).unwrap_or(0);
let call_snippet_with_replacements = args_snippets
.iter()
.fold(call_snippet.to_owned(), |acc, arg| acc.replacen(arg.as_ref(), "()", 1));
let mut stmts_and_call = non_empty_block_args_snippets
.iter()
.map(|it| it.as_ref().to_owned())
.collect::<Vec<_>>();
stmts_and_call.push(call_snippet_with_replacements);
stmts_and_call = stmts_and_call
.into_iter()
.map(|v| reindent_multiline(v.into(), true, Some(call_expr_indent)).into_owned())
.collect();
let mut stmts_and_call_snippet = stmts_and_call.join(&format!("{}{}", ";\n", " ".repeat(call_expr_indent)));
// expr is not in a block statement or result expression position, wrap in a block
let parent_node = cx.tcx.hir().find(cx.tcx.hir().get_parent_node(call_expr.hir_id));
if !matches!(parent_node, Some(Node::Block(_))) && !matches!(parent_node, Some(Node::Stmt(_))) {
let block_indent = call_expr_indent + 4;
stmts_and_call_snippet =
reindent_multiline(stmts_and_call_snippet.into(), true, Some(block_indent)).into_owned();
stmts_and_call_snippet = format!(
"{{\n{}{}\n{}}}",
" ".repeat(block_indent),
&stmts_and_call_snippet,
" ".repeat(call_expr_indent)
);
}
stmts_and_call_snippet
}

57
clippy_lints/src/unit_types/unit_cmp.rs Normal file
View File

@ -0,0 +1,57 @@
use rustc_hir::{BinOpKind, Expr, ExprKind};
use rustc_lint::LateContext;
use rustc_span::hygiene::{ExpnKind, MacroKind};
use crate::utils::diagnostics::span_lint;
use super::UNIT_CMP;
pub(super) fn check(cx: &LateContext<'_>, expr: &Expr<'_>) {
if expr.span.from_expansion() {
if let Some(callee) = expr.span.source_callee() {
if let ExpnKind::Macro(MacroKind::Bang, symbol) = callee.kind {
if let ExprKind::Binary(ref cmp, ref left, _) = expr.kind {
let op = cmp.node;
if op.is_comparison() && cx.typeck_results().expr_ty(left).is_unit() {
let result = match &*symbol.as_str() {
"assert_eq" | "debug_assert_eq" => "succeed",
"assert_ne" | "debug_assert_ne" => "fail",
_ => return,
};
span_lint(
cx,
UNIT_CMP,
expr.span,
&format!(
"`{}` of unit values detected. This will always {}",
symbol.as_str(),
result
),
);
}
}
}
}
return;
}
if let ExprKind::Binary(ref cmp, ref left, _) = expr.kind {
let op = cmp.node;
if op.is_comparison() && cx.typeck_results().expr_ty(left).is_unit() {
let result = match op {
BinOpKind::Eq | BinOpKind::Le | BinOpKind::Ge => "true",
_ => "false",
};
span_lint(
cx,
UNIT_CMP,
expr.span,
&format!(
"{}-comparison of unit values detected. This will always be {}",
op.as_str(),
result
),
);
}
}
}

5
clippy_lints/src/unit_types/utils.rs Normal file
View File

@ -0,0 +1,5 @@
use rustc_hir::{Expr, ExprKind};
pub(super) fn is_unit_literal(expr: &Expr<'_>) -> bool {
matches!(expr.kind, ExprKind::Tup(ref slice) if slice.is_empty())
}