Skip to content

Commit

Permalink
Auto merge of #5141 - xiongmao86:issue5095, r=flip1995
Browse files Browse the repository at this point in the history
Fixes issue 5095

fixes #5095.

- [x] Followed [lint naming conventions][lint_naming]
- [x] Added passing UI tests (including committed `.stderr` file)
- [x] `cargo test` passes locally
- [x] Executed `cargo dev update_lints`
- [x] Added lint documentation
- [x] Run `cargo dev fmt`

[lint_naming]: https://rust-lang.github.io/rfcs/0344-conventions-galore.html#lints

changelog: (internal) warn about collapsible `span_lint_and_then` calls.
  • Loading branch information
bors committed Apr 19, 2020
2 parents 2efc2d6 + 7aeb3a4 commit 6dcc8d5
Show file tree
Hide file tree
Showing 52 changed files with 758 additions and 199 deletions.
1 change: 1 addition & 0 deletions clippy_lints/src/as_conversions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ impl EarlyLintPass for AsConversions {
AS_CONVERSIONS,
expr.span,
"using a potentially dangerous silent `as` conversion",
None,
"consider using a safe wrapper for this conversion",
);
}
Expand Down
3 changes: 3 additions & 0 deletions clippy_lints/src/assertions_on_constants.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for AssertionsOnConstants {
} else {
"`assert!(true)` will be optimized out by the compiler"
},
None,
"remove it",
);
};
Expand All @@ -50,6 +51,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for AssertionsOnConstants {
ASSERTIONS_ON_CONSTANTS,
e.span,
"`assert!(false)` should probably be replaced",
None,
"use `panic!()` or `unreachable!()`",
);
};
Expand All @@ -59,6 +61,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for AssertionsOnConstants {
ASSERTIONS_ON_CONSTANTS,
e.span,
&format!("`assert!(false, {})` should probably be replaced", panic_message),
None,
&format!("use `panic!({})` or `unreachable!({})`", panic_message, panic_message),
)
};
Expand Down
3 changes: 3 additions & 0 deletions clippy_lints/src/atomic_ordering.rs
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,7 @@ fn check_atomic_load_store(cx: &LateContext<'_, '_>, expr: &Expr<'_>) {
INVALID_ATOMIC_ORDERING,
ordering_arg.span,
"atomic loads cannot have `Release` and `AcqRel` ordering",
None,
"consider using ordering modes `Acquire`, `SeqCst` or `Relaxed`"
);
} else if method == "store" &&
Expand All @@ -94,6 +95,7 @@ fn check_atomic_load_store(cx: &LateContext<'_, '_>, expr: &Expr<'_>) {
INVALID_ATOMIC_ORDERING,
ordering_arg.span,
"atomic stores cannot have `Acquire` and `AcqRel` ordering",
None,
"consider using ordering modes `Release`, `SeqCst` or `Relaxed`"
);
}
Expand All @@ -118,6 +120,7 @@ fn check_memory_fence(cx: &LateContext<'_, '_>, expr: &Expr<'_>) {
INVALID_ATOMIC_ORDERING,
args[0].span,
"memory fences cannot have `Relaxed` ordering",
None,
"consider using ordering modes `Acquire`, `Release`, `AcqRel` or `SeqCst`"
);
}
Expand Down
1 change: 1 addition & 0 deletions clippy_lints/src/cognitive_complexity.rs
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,7 @@ impl CognitiveComplexity {
rust_cc,
self.limit.limit()
),
None,
"you could split it up into multiple smaller functions",
);
}
Expand Down
1 change: 1 addition & 0 deletions clippy_lints/src/comparison_chain.rs
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for ComparisonChain {
COMPARISON_CHAIN,
expr.span,
"`if` chain can be rewritten with `match`",
None,
"Consider rewriting the `if` chain to use `cmp` and `match`.",
)
}
Expand Down
6 changes: 3 additions & 3 deletions clippy_lints/src/copies.rs
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,7 @@ fn lint_same_then_else(cx: &LateContext<'_, '_>, blocks: &[&Block<'_>]) {
IF_SAME_THEN_ELSE,
j.span,
"this `if` has identical blocks",
i.span,
Some(i.span),
"same as this",
);
}
Expand All @@ -206,7 +206,7 @@ fn lint_same_cond(cx: &LateContext<'_, '_>, conds: &[&Expr<'_>]) {
IFS_SAME_COND,
j.span,
"this `if` has the same condition as a previous `if`",
i.span,
Some(i.span),
"same as this",
);
}
Expand Down Expand Up @@ -234,7 +234,7 @@ fn lint_same_fns_in_if_cond(cx: &LateContext<'_, '_>, conds: &[&Expr<'_>]) {
SAME_FUNCTIONS_IN_IF_CONDITION,
j.span,
"this `if` has the same function call as a previous `if`",
i.span,
Some(i.span),
"same as this",
);
}
Expand Down
2 changes: 1 addition & 1 deletion clippy_lints/src/copy_iterator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for CopyIterator {
COPY_ITERATOR,
item.span,
"you are implementing `Iterator` on a `Copy` type",
item.span,
None,
"consider implementing `IntoIterator` instead",
);
}
Expand Down
1 change: 1 addition & 0 deletions clippy_lints/src/dbg_macro.rs
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ impl EarlyLintPass for DbgMacro {
DBG_MACRO,
mac.span(),
"`dbg!` macro is intended as a debugging tool",
None,
"ensure to avoid having uses of it in version control",
);
}
Expand Down
9 changes: 4 additions & 5 deletions clippy_lints/src/derive.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use crate::utils::paths;
use crate::utils::{is_automatically_derived, is_copy, match_path, span_lint_and_then};
use crate::utils::{is_automatically_derived, is_copy, match_path, span_lint_and_note, span_lint_and_then};
use if_chain::if_chain;
use rustc_hir::{Item, ItemKind, TraitRef};
use rustc_lint::{LateContext, LateLintPass};
Expand Down Expand Up @@ -163,14 +163,13 @@ fn check_copy_clone<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, item: &Item<'_>, trait
_ => (),
}

span_lint_and_then(
span_lint_and_note(
cx,
EXPL_IMPL_CLONE_ON_COPY,
item.span,
"you are implementing `Clone` explicitly on a `Copy` type",
|diag| {
diag.span_note(item.span, "consider deriving `Clone` or removing `Copy`");
},
Some(item.span),
"consider deriving `Clone` or removing `Copy`",
);
}
}
4 changes: 2 additions & 2 deletions clippy_lints/src/drop_forget_ref.rs
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for DropForgetRef {
lint,
expr.span,
&msg,
arg.span,
Some(arg.span),
&format!("argument has type `{}`", arg_ty));
} else if is_copy(cx, arg_ty) {
if match_def_path(cx, def_id, &paths::DROP) {
Expand All @@ -151,7 +151,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for DropForgetRef {
lint,
expr.span,
&msg,
arg.span,
Some(arg.span),
&format!("argument has type {}", arg_ty));
}
}
Expand Down
1 change: 1 addition & 0 deletions clippy_lints/src/else_if_without_else.rs
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ impl EarlyLintPass for ElseIfWithoutElse {
ELSE_IF_WITHOUT_ELSE,
els.span,
"`if` expression with an `else if`, but without a final `else`",
None,
"add an `else` block here",
);
}
Expand Down
18 changes: 10 additions & 8 deletions clippy_lints/src/empty_enum.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
//! lint when there is an enum with no variants

use crate::utils::span_lint_and_then;
use crate::utils::span_lint_and_help;
use rustc_hir::{Item, ItemKind};
use rustc_lint::{LateContext, LateLintPass};
use rustc_session::{declare_lint_pass, declare_tool_lint};
Expand Down Expand Up @@ -45,13 +45,15 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for EmptyEnum {
let ty = cx.tcx.type_of(did);
let adt = ty.ty_adt_def().expect("already checked whether this is an enum");
if adt.variants.is_empty() {
span_lint_and_then(cx, EMPTY_ENUM, item.span, "enum with no variants", |diag| {
diag.span_help(
item.span,
"consider using the uninhabited type `!` (never type) or a wrapper \
around it to introduce a type which can't be instantiated",
);
});
span_lint_and_help(
cx,
EMPTY_ENUM,
item.span,
"enum with no variants",
None,
"consider using the uninhabited type `!` (never type) or a wrapper \
around it to introduce a type which can't be instantiated",
);
}
}
}
Expand Down
1 change: 1 addition & 0 deletions clippy_lints/src/enum_variants.rs
Original file line number Diff line number Diff line change
Expand Up @@ -206,6 +206,7 @@ fn check_variant(
lint,
span,
&format!("All variants have the same {}fix: `{}`", what, value),
None,
&format!(
"remove the {}fixes and use full paths to \
the variants instead of glob imports",
Expand Down
20 changes: 11 additions & 9 deletions clippy_lints/src/eta_reduction.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,8 @@ use rustc_middle::ty::{self, Ty};
use rustc_session::{declare_lint_pass, declare_tool_lint};

use crate::utils::{
implements_trait, is_adjusted, iter_input_pats, snippet_opt, span_lint_and_then, type_is_unsafe_function,
implements_trait, is_adjusted, iter_input_pats, snippet_opt, span_lint_and_sugg, span_lint_and_then,
type_is_unsafe_function,
};

declare_clippy_lint! {
Expand Down Expand Up @@ -131,14 +132,15 @@ fn check_closure(cx: &LateContext<'_, '_>, expr: &Expr<'_>) {
if let Some(name) = get_ufcs_type_name(cx, method_def_id, &args[0]);

then {
span_lint_and_then(cx, REDUNDANT_CLOSURE_FOR_METHOD_CALLS, expr.span, "redundant closure found", |diag| {
diag.span_suggestion(
expr.span,
"remove closure as shown",
format!("{}::{}", name, path.ident.name),
Applicability::MachineApplicable,
);
});
span_lint_and_sugg(
cx,
REDUNDANT_CLOSURE_FOR_METHOD_CALLS,
expr.span,
"redundant closure found",
"remove closure as shown",
format!("{}::{}", name, path.ident.name),
Applicability::MachineApplicable,
);
}
);
}
Expand Down
2 changes: 1 addition & 1 deletion clippy_lints/src/eval_order_dependence.rs
Original file line number Diff line number Diff line change
Expand Up @@ -310,7 +310,7 @@ impl<'a, 'tcx> Visitor<'tcx> for ReadVisitor<'a, 'tcx> {
EVAL_ORDER_DEPENDENCE,
expr.span,
"unsequenced read of a variable",
self.write_expr.span,
Some(self.write_expr.span),
"whether read occurs before this write depends on evaluation order"
);
}
Expand Down
2 changes: 2 additions & 0 deletions clippy_lints/src/excessive_bools.rs
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,7 @@ impl ExcessiveBools {
FN_PARAMS_EXCESSIVE_BOOLS,
span,
&format!("more than {} bools in function parameters", self.max_fn_params_bools),
None,
"consider refactoring bools into two-variant enums",
);
}
Expand Down Expand Up @@ -153,6 +154,7 @@ impl EarlyLintPass for ExcessiveBools {
STRUCT_EXCESSIVE_BOOLS,
item.span,
&format!("more than {} bools in a struct", self.max_struct_bools),
None,
"consider using a state machine or refactoring bools into two-variant enums",
);
}
Expand Down
9 changes: 5 additions & 4 deletions clippy_lints/src/formatting.rs
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ fn check_assign(cx: &EarlyContext<'_>, expr: &Expr) {
really are doing `.. = ({op} ..)`",
op = op
),
eqop_span,
None,
&format!("to remove this lint, use either `{op}=` or `= {op}`", op = op),
);
}
Expand Down Expand Up @@ -188,6 +188,7 @@ fn check_unop(cx: &EarlyContext<'_>, expr: &Expr) {
binop = binop_str,
unop = unop_str
),
None,
&format!(
"put a space between `{binop}` and `{unop}` and remove the space after `{unop}`",
binop = binop_str,
Expand Down Expand Up @@ -226,7 +227,7 @@ fn check_else(cx: &EarlyContext<'_>, expr: &Expr) {
SUSPICIOUS_ELSE_FORMATTING,
else_span,
&format!("this is an `else {}` but the formatting might hide it", else_desc),
else_span,
None,
&format!(
"to remove this lint, remove the `else` or remove the new line between \
`else` and `{}`",
Expand Down Expand Up @@ -265,7 +266,7 @@ fn check_array(cx: &EarlyContext<'_>, expr: &Expr) {
POSSIBLE_MISSING_COMMA,
lint_span,
"possibly missing a comma here",
lint_span,
None,
"to remove this lint, add a comma or write the expr in a single line",
);
}
Expand Down Expand Up @@ -296,7 +297,7 @@ fn check_missing_else(cx: &EarlyContext<'_>, first: &Expr, second: &Expr) {
SUSPICIOUS_ELSE_FORMATTING,
else_span,
&format!("this looks like {} but the `else` is missing", looks_like),
else_span,
None,
&format!(
"to remove this lint, add the missing `else` or add a new line before {}",
next_thing,
Expand Down
1 change: 1 addition & 0 deletions clippy_lints/src/functions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -431,6 +431,7 @@ fn check_needless_must_use(
DOUBLE_MUST_USE,
fn_header_span,
"this function has an empty `#[must_use]` attribute, but returns a type already marked as `#[must_use]`",
None,
"either add some descriptive text or remove the attribute",
);
}
Expand Down
53 changes: 28 additions & 25 deletions clippy_lints/src/identity_conversion.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use crate::utils::{
match_def_path, match_trait_method, paths, same_tys, snippet, snippet_with_macro_callsite, span_lint_and_then,
match_def_path, match_trait_method, paths, same_tys, snippet, snippet_with_macro_callsite, span_lint_and_sugg,
};
use rustc_errors::Applicability;
use rustc_hir::{Expr, ExprKind, HirId, MatchSource};
Expand Down Expand Up @@ -58,29 +58,31 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for IdentityConversion {
if same_tys(cx, a, b) {
let sugg = snippet_with_macro_callsite(cx, args[0].span, "<expr>").to_string();

span_lint_and_then(cx, IDENTITY_CONVERSION, e.span, "identical conversion", |diag| {
diag.span_suggestion(
e.span,
"consider removing `.into()`",
sugg,
Applicability::MachineApplicable, // snippet
);
});
span_lint_and_sugg(
cx,
IDENTITY_CONVERSION,
e.span,
"identical conversion",
"consider removing `.into()`",
sugg,
Applicability::MachineApplicable, // snippet
);
}
}
if match_trait_method(cx, e, &paths::INTO_ITERATOR) && &*name.ident.as_str() == "into_iter" {
let a = cx.tables.expr_ty(e);
let b = cx.tables.expr_ty(&args[0]);
if same_tys(cx, a, b) {
let sugg = snippet(cx, args[0].span, "<expr>").into_owned();
span_lint_and_then(cx, IDENTITY_CONVERSION, e.span, "identical conversion", |diag| {
diag.span_suggestion(
e.span,
"consider removing `.into_iter()`",
sugg,
Applicability::MachineApplicable, // snippet
);
});
span_lint_and_sugg(
cx,
IDENTITY_CONVERSION,
e.span,
"identical conversion",
"consider removing `.into_iter()`",
sugg,
Applicability::MachineApplicable, // snippet
);
}
}
},
Expand All @@ -95,14 +97,15 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for IdentityConversion {
let sugg = snippet(cx, args[0].span.source_callsite(), "<expr>").into_owned();
let sugg_msg =
format!("consider removing `{}()`", snippet(cx, path.span, "From::from"));
span_lint_and_then(cx, IDENTITY_CONVERSION, e.span, "identical conversion", |diag| {
diag.span_suggestion(
e.span,
&sugg_msg,
sugg,
Applicability::MachineApplicable, // snippet
);
});
span_lint_and_sugg(
cx,
IDENTITY_CONVERSION,
e.span,
"identical conversion",
&sugg_msg,
sugg,
Applicability::MachineApplicable, // snippet
);
}
}
}
Expand Down
Loading

0 comments on commit 6dcc8d5

Please sign in to comment.