Skip to content

Commit

Permalink
Rollup merge of #74324 - flip1995:clippyup, r=Manishearth
Browse files Browse the repository at this point in the history
Update Clippy

~~I'm not sure, if we can/should land this before beta is branched.~~ (Nvm, beta is already branched) The last Clippy update was 3 weeks ago: #73660

This includes, besides other minor things:

- New lints
- One lint deprecation
- One lint was moved to pedantic
- Some FP fixes
- I think an ICE fix?

cc @Mark-Simulacrum

r? @Manishearth

---

We probably should also think of some process when and how often we should sync Clippy to the rust repo, so that we don't end up with those huge updates. Maybe every 2 weeks? Or even every week? cc @rust-lang/clippy
  • Loading branch information
Manishearth committed Jul 14, 2020
2 parents 1d72e72 + f82ac4d commit 4c2d7ee
Show file tree
Hide file tree
Showing 143 changed files with 4,316 additions and 1,083 deletions.
2 changes: 1 addition & 1 deletion src/tools/clippy/.github/ISSUE_TEMPLATE/new_lint.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ labels: L-lint

- Kind: *See <https://github.com/rust-lang/rust-clippy/blob/master/README.md#clippy> for list of lint kinds*

*What benefit of this lint over old code?*
*What is the advantage of the recommended code over the original code*

For example:
- Remove bounce checking inserted by ...
Expand Down
3 changes: 2 additions & 1 deletion src/tools/clippy/.github/workflows/clippy_bors.yml
Original file line number Diff line number Diff line change
Expand Up @@ -240,7 +240,8 @@ jobs:
- 'Geal/nom'
- 'rust-lang/stdarch'
- 'serde-rs/serde'
- 'chronotope/chrono'
# FIXME: chrono currently cannot be compiled with `--all-targets`
# - 'chronotope/chrono'
- 'hyperium/hyper'
- 'rust-random/rand'
- 'rust-lang/futures-rs'
Expand Down
6 changes: 6 additions & 0 deletions src/tools/clippy/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -1352,6 +1352,7 @@ Released 2018-09-13
[`bad_bit_mask`]: https://rust-lang.github.io/rust-clippy/master/index.html#bad_bit_mask
[`bind_instead_of_map`]: https://rust-lang.github.io/rust-clippy/master/index.html#bind_instead_of_map
[`blacklisted_name`]: https://rust-lang.github.io/rust-clippy/master/index.html#blacklisted_name
[`blanket_clippy_restriction_lints`]: https://rust-lang.github.io/rust-clippy/master/index.html#blanket_clippy_restriction_lints
[`blocks_in_if_conditions`]: https://rust-lang.github.io/rust-clippy/master/index.html#blocks_in_if_conditions
[`bool_comparison`]: https://rust-lang.github.io/rust-clippy/master/index.html#bool_comparison
[`borrow_interior_mutable_const`]: https://rust-lang.github.io/rust-clippy/master/index.html#borrow_interior_mutable_const
Expand Down Expand Up @@ -1508,9 +1509,11 @@ Released 2018-09-13
[`map_clone`]: https://rust-lang.github.io/rust-clippy/master/index.html#map_clone
[`map_entry`]: https://rust-lang.github.io/rust-clippy/master/index.html#map_entry
[`map_flatten`]: https://rust-lang.github.io/rust-clippy/master/index.html#map_flatten
[`map_identity`]: https://rust-lang.github.io/rust-clippy/master/index.html#map_identity
[`map_unwrap_or`]: https://rust-lang.github.io/rust-clippy/master/index.html#map_unwrap_or
[`match_as_ref`]: https://rust-lang.github.io/rust-clippy/master/index.html#match_as_ref
[`match_bool`]: https://rust-lang.github.io/rust-clippy/master/index.html#match_bool
[`match_like_matches_macro`]: https://rust-lang.github.io/rust-clippy/master/index.html#match_like_matches_macro
[`match_on_vec_items`]: https://rust-lang.github.io/rust-clippy/master/index.html#match_on_vec_items
[`match_overlapping_arm`]: https://rust-lang.github.io/rust-clippy/master/index.html#match_overlapping_arm
[`match_ref_pats`]: https://rust-lang.github.io/rust-clippy/master/index.html#match_ref_pats
Expand Down Expand Up @@ -1575,6 +1578,7 @@ Released 2018-09-13
[`op_ref`]: https://rust-lang.github.io/rust-clippy/master/index.html#op_ref
[`option_as_ref_deref`]: https://rust-lang.github.io/rust-clippy/master/index.html#option_as_ref_deref
[`option_env_unwrap`]: https://rust-lang.github.io/rust-clippy/master/index.html#option_env_unwrap
[`option_if_let_else`]: https://rust-lang.github.io/rust-clippy/master/index.html#option_if_let_else
[`option_map_or_none`]: https://rust-lang.github.io/rust-clippy/master/index.html#option_map_or_none
[`option_map_unit_fn`]: https://rust-lang.github.io/rust-clippy/master/index.html#option_map_unit_fn
[`option_option`]: https://rust-lang.github.io/rust-clippy/master/index.html#option_option
Expand All @@ -1586,6 +1590,7 @@ Released 2018-09-13
[`panicking_unwrap`]: https://rust-lang.github.io/rust-clippy/master/index.html#panicking_unwrap
[`partialeq_ne_impl`]: https://rust-lang.github.io/rust-clippy/master/index.html#partialeq_ne_impl
[`path_buf_push_overwrite`]: https://rust-lang.github.io/rust-clippy/master/index.html#path_buf_push_overwrite
[`pattern_type_mismatch`]: https://rust-lang.github.io/rust-clippy/master/index.html#pattern_type_mismatch
[`possible_missing_comma`]: https://rust-lang.github.io/rust-clippy/master/index.html#possible_missing_comma
[`precedence`]: https://rust-lang.github.io/rust-clippy/master/index.html#precedence
[`print_literal`]: https://rust-lang.github.io/rust-clippy/master/index.html#print_literal
Expand All @@ -1612,6 +1617,7 @@ Released 2018-09-13
[`redundant_static_lifetimes`]: https://rust-lang.github.io/rust-clippy/master/index.html#redundant_static_lifetimes
[`ref_in_deref`]: https://rust-lang.github.io/rust-clippy/master/index.html#ref_in_deref
[`regex_macro`]: https://rust-lang.github.io/rust-clippy/master/index.html#regex_macro
[`repeat_once`]: https://rust-lang.github.io/rust-clippy/master/index.html#repeat_once
[`replace_consts`]: https://rust-lang.github.io/rust-clippy/master/index.html#replace_consts
[`rest_pat_in_fully_bound_structs`]: https://rust-lang.github.io/rust-clippy/master/index.html#rest_pat_in_fully_bound_structs
[`result_map_or_into_option`]: https://rust-lang.github.io/rust-clippy/master/index.html#result_map_or_into_option
Expand Down
2 changes: 1 addition & 1 deletion src/tools/clippy/CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -245,7 +245,7 @@ this to work, you will need the fix of `git subtree` available
[here][gitgitgadget-pr].
[gitgitgadget-pr]: https://github.com/gitgitgadget/git/pull/493
[subtree]: https://github.com/rust-lang/rust/blob/master/CONTRIBUTING.md#external-dependencies-subtree
[subtree]: https://rustc-dev-guide.rust-lang.org/contributing.html#external-dependencies-subtree
[`rust-lang/rust`]: https://github.com/rust-lang/rust
## Issue and PR triage
Expand Down
115 changes: 76 additions & 39 deletions src/tools/clippy/clippy_lints/src/attrs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@

use crate::reexport::Name;
use crate::utils::{
first_line_of_span, is_present_in_source, match_def_path, paths, snippet_opt, span_lint, span_lint_and_sugg,
span_lint_and_then, without_block_comments,
first_line_of_span, is_present_in_source, match_def_path, paths, snippet_opt, span_lint, span_lint_and_help,
span_lint_and_sugg, span_lint_and_then, without_block_comments,
};
use if_chain::if_chain;
use rustc_ast::ast::{AttrKind, AttrStyle, Attribute, Lit, LitKind, MetaItemKind, NestedMetaItem};
Expand All @@ -17,7 +17,7 @@ use rustc_middle::lint::in_external_macro;
use rustc_middle::ty;
use rustc_session::{declare_lint_pass, declare_tool_lint};
use rustc_span::source_map::Span;
use rustc_span::symbol::Symbol;
use rustc_span::symbol::{Symbol, SymbolStr};
use semver::Version;

static UNIX_SYSTEMS: &[&str] = &[
Expand Down Expand Up @@ -182,6 +182,29 @@ declare_clippy_lint! {
"unknown_lints for scoped Clippy lints"
}

declare_clippy_lint! {
/// **What it does:** Checks for `warn`/`deny`/`forbid` attributes targeting the whole clippy::restriction category.
///
/// **Why is this bad?** Restriction lints sometimes are in contrast with other lints or even go against idiomatic rust.
/// These lints should only be enabled on a lint-by-lint basis and with careful consideration.
///
/// **Known problems:** None.
///
/// **Example:**
/// Bad:
/// ```rust
/// #![deny(clippy::restriction)]
/// ```
///
/// Good:
/// ```rust
/// #![deny(clippy::as_conversions)]
/// ```
pub BLANKET_CLIPPY_RESTRICTION_LINTS,
style,
"enabling the complete restriction group"
}

declare_clippy_lint! {
/// **What it does:** Checks for `#[cfg_attr(rustfmt, rustfmt_skip)]` and suggests to replace it
/// with `#[rustfmt::skip]`.
Expand Down Expand Up @@ -249,15 +272,17 @@ declare_lint_pass!(Attributes => [
DEPRECATED_SEMVER,
USELESS_ATTRIBUTE,
UNKNOWN_CLIPPY_LINTS,
BLANKET_CLIPPY_RESTRICTION_LINTS,
]);

impl<'tcx> LateLintPass<'tcx> for Attributes {
fn check_attribute(&mut self, cx: &LateContext<'tcx>, attr: &'tcx Attribute) {
if let Some(items) = &attr.meta_item_list() {
if let Some(ident) = attr.ident() {
match &*ident.as_str() {
let ident = &*ident.as_str();
match ident {
"allow" | "warn" | "deny" | "forbid" => {
check_clippy_lint_names(cx, items);
check_clippy_lint_names(cx, ident, items);
},
_ => {},
}
Expand Down Expand Up @@ -363,38 +388,43 @@ impl<'tcx> LateLintPass<'tcx> for Attributes {
}
}

#[allow(clippy::single_match_else)]
fn check_clippy_lint_names(cx: &LateContext<'_>, items: &[NestedMetaItem]) {
let lint_store = cx.lints();
for lint in items {
fn check_clippy_lint_names(cx: &LateContext<'_>, ident: &str, items: &[NestedMetaItem]) {
fn extract_name(lint: &NestedMetaItem) -> Option<SymbolStr> {
if_chain! {
if let Some(meta_item) = lint.meta_item();
if meta_item.path.segments.len() > 1;
if let tool_name = meta_item.path.segments[0].ident;
if tool_name.as_str() == "clippy";
let name = meta_item.path.segments.last().unwrap().ident.name;
if let CheckLintNameResult::Tool(Err((None, _))) = lint_store.check_lint_name(
&name.as_str(),
Some(tool_name.name),
);
let lint_name = meta_item.path.segments.last().unwrap().ident.name;
then {
return Some(lint_name.as_str());
}
}
None
}

let lint_store = cx.lints();
for lint in items {
if let Some(lint_name) = extract_name(lint) {
if let CheckLintNameResult::Tool(Err((None, _))) =
lint_store.check_lint_name(&lint_name, Some(sym!(clippy)))
{
span_lint_and_then(
cx,
UNKNOWN_CLIPPY_LINTS,
lint.span(),
&format!("unknown clippy lint: clippy::{}", name),
&format!("unknown clippy lint: clippy::{}", lint_name),
|diag| {
let name_lower = name.as_str().to_lowercase();
let symbols = lint_store.get_lints().iter().map(
|l| Symbol::intern(&l.name_lower())
).collect::<Vec<_>>();
let sugg = find_best_match_for_name(
symbols.iter(),
&format!("clippy::{}", name_lower),
None,
);
if name.as_str().chars().any(char::is_uppercase)
&& lint_store.find_lints(&format!("clippy::{}", name_lower)).is_ok() {
let name_lower = lint_name.to_lowercase();
let symbols = lint_store
.get_lints()
.iter()
.map(|l| Symbol::intern(&l.name_lower()))
.collect::<Vec<_>>();
let sugg = find_best_match_for_name(symbols.iter(), &format!("clippy::{}", name_lower), None);
if lint_name.chars().any(char::is_uppercase)
&& lint_store.find_lints(&format!("clippy::{}", name_lower)).is_ok()
{
diag.span_suggestion(
lint.span(),
"lowercase the lint name",
Expand All @@ -409,10 +439,19 @@ fn check_clippy_lint_names(cx: &LateContext<'_>, items: &[NestedMetaItem]) {
Applicability::MachineApplicable,
);
}
}
},
);
} else if lint_name == "restriction" && ident != "allow" {
span_lint_and_help(
cx,
BLANKET_CLIPPY_RESTRICTION_LINTS,
lint.span(),
"restriction lints are not meant to be all enabled",
None,
"try enabling only the lints you really need",
);
}
};
}
}
}

Expand Down Expand Up @@ -442,15 +481,14 @@ fn is_relevant_trait(cx: &LateContext<'_>, item: &TraitItem<'_>) -> bool {
}

fn is_relevant_block(cx: &LateContext<'_>, tables: &ty::TypeckTables<'_>, block: &Block<'_>) -> bool {
if let Some(stmt) = block.stmts.first() {
match &stmt.kind {
block.stmts.first().map_or(
block.expr.as_ref().map_or(false, |e| is_relevant_expr(cx, tables, e)),
|stmt| match &stmt.kind {
StmtKind::Local(_) => true,
StmtKind::Expr(expr) | StmtKind::Semi(expr) => is_relevant_expr(cx, tables, expr),
_ => false,
}
} else {
block.expr.as_ref().map_or(false, |e| is_relevant_expr(cx, tables, e))
}
},
)
}

fn is_relevant_expr(cx: &LateContext<'_>, tables: &ty::TypeckTables<'_>, expr: &Expr<'_>) -> bool {
Expand All @@ -460,11 +498,10 @@ fn is_relevant_expr(cx: &LateContext<'_>, tables: &ty::TypeckTables<'_>, expr: &
ExprKind::Ret(None) | ExprKind::Break(_, None) => false,
ExprKind::Call(path_expr, _) => {
if let ExprKind::Path(qpath) = &path_expr.kind {
if let Some(fun_id) = tables.qpath_res(qpath, path_expr.hir_id).opt_def_id() {
!match_def_path(cx, fun_id, &paths::BEGIN_PANIC)
} else {
true
}
tables
.qpath_res(qpath, path_expr.hir_id)
.opt_def_id()
.map_or(true, |fun_id| !match_def_path(cx, fun_id, &paths::BEGIN_PANIC))
} else {
true
}
Expand Down
2 changes: 1 addition & 1 deletion src/tools/clippy/clippy_lints/src/await_holding_lock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ declare_clippy_lint! {
/// non-async-aware MutexGuard.
///
/// **Why is this bad?** The Mutex types found in syd::sync and parking_lot
/// are not designed to operator in an async context across await points.
/// are not designed to operate in an async context across await points.
///
/// There are two potential solutions. One is to use an asynx-aware Mutex
/// type. Many asynchronous foundation crates provide such a Mutex type. The
Expand Down
4 changes: 2 additions & 2 deletions src/tools/clippy/clippy_lints/src/collapsible_if.rs
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ fn check_collapsible_maybe_if_let(cx: &EarlyContext<'_>, else_: &ast::Expr) {
COLLAPSIBLE_IF,
block.span,
"this `else { if .. }` block can be collapsed",
"try",
"collapse nested if block",
snippet_block_with_applicability(cx, else_.span, "..", Some(block.span), &mut applicability).into_owned(),
applicability,
);
Expand All @@ -142,7 +142,7 @@ fn check_collapsible_no_if_let(cx: &EarlyContext<'_>, expr: &ast::Expr, check: &
let rhs = Sugg::ast(cx, check_inner, "..");
diag.span_suggestion(
expr.span,
"try",
"collapse nested if block",
format!(
"if {} {}",
lhs.and(&rhs),
Expand Down
5 changes: 1 addition & 4 deletions src/tools/clippy/clippy_lints/src/comparison_chain.rs
Original file line number Diff line number Diff line change
Expand Up @@ -122,8 +122,5 @@ impl<'tcx> LateLintPass<'tcx> for ComparisonChain {
}

fn kind_is_cmp(kind: BinOpKind) -> bool {
match kind {
BinOpKind::Lt | BinOpKind::Gt | BinOpKind::Eq => true,
_ => false,
}
matches!(kind, BinOpKind::Lt | BinOpKind::Gt | BinOpKind::Eq)
}
10 changes: 9 additions & 1 deletion src/tools/clippy/clippy_lints/src/deprecated_lints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -153,5 +153,13 @@ declare_deprecated_lint! {
///
/// **Deprecation reason:** Associated-constants are now preferred.
pub REPLACE_CONSTS,
"associated-constants `MIN`/`MAX` of integers are prefer to `{min,max}_value()` and module constants"
"associated-constants `MIN`/`MAX` of integers are prefered to `{min,max}_value()` and module constants"
}

declare_deprecated_lint! {
/// **What it does:** Nothing. This lint has been deprecated.
///
/// **Deprecation reason:** The regex! macro does not exist anymore.
pub REGEX_MACRO,
"the regex! macro has been removed from the regex crate in 2018"
}
10 changes: 6 additions & 4 deletions src/tools/clippy/clippy_lints/src/dereference.rs
Original file line number Diff line number Diff line change
Expand Up @@ -73,9 +73,10 @@ impl<'tcx> LateLintPass<'tcx> for Dereferencing {
fn lint_deref(cx: &LateContext<'_>, method_name: &str, call_expr: &Expr<'_>, var_span: Span, expr_span: Span) {
match method_name {
"deref" => {
if cx.tcx.lang_items().deref_trait().map_or(false, |id| {
let impls_deref_trait = cx.tcx.lang_items().deref_trait().map_or(false, |id| {
implements_trait(cx, cx.tables().expr_ty(&call_expr), id, &[])
}) {
});
if impls_deref_trait {
span_lint_and_sugg(
cx,
EXPLICIT_DEREF_METHODS,
Expand All @@ -88,9 +89,10 @@ fn lint_deref(cx: &LateContext<'_>, method_name: &str, call_expr: &Expr<'_>, var
}
},
"deref_mut" => {
if cx.tcx.lang_items().deref_mut_trait().map_or(false, |id| {
let impls_deref_mut_trait = cx.tcx.lang_items().deref_mut_trait().map_or(false, |id| {
implements_trait(cx, cx.tables().expr_ty(&call_expr), id, &[])
}) {
});
if impls_deref_mut_trait {
span_lint_and_sugg(
cx,
EXPLICIT_DEREF_METHODS,
Expand Down
30 changes: 15 additions & 15 deletions src/tools/clippy/clippy_lints/src/eq_op.rs
Original file line number Diff line number Diff line change
Expand Up @@ -214,20 +214,20 @@ impl<'tcx> LateLintPass<'tcx> for EqOp {
}

fn is_valid_operator(op: BinOp) -> bool {
match op.node {
matches!(
op.node,
BinOpKind::Sub
| BinOpKind::Div
| BinOpKind::Eq
| BinOpKind::Lt
| BinOpKind::Le
| BinOpKind::Gt
| BinOpKind::Ge
| BinOpKind::Ne
| BinOpKind::And
| BinOpKind::Or
| BinOpKind::BitXor
| BinOpKind::BitAnd
| BinOpKind::BitOr => true,
_ => false,
}
| BinOpKind::Div
| BinOpKind::Eq
| BinOpKind::Lt
| BinOpKind::Le
| BinOpKind::Gt
| BinOpKind::Ge
| BinOpKind::Ne
| BinOpKind::And
| BinOpKind::Or
| BinOpKind::BitXor
| BinOpKind::BitAnd
| BinOpKind::BitOr
)
}
5 changes: 1 addition & 4 deletions src/tools/clippy/clippy_lints/src/escape.rs
Original file line number Diff line number Diff line change
Expand Up @@ -105,10 +105,7 @@ fn is_argument(map: rustc_middle::hir::map::Map<'_>, id: HirId) -> bool {
_ => return false,
}

match map.find(map.get_parent_node(id)) {
Some(Node::Param(_)) => true,
_ => false,
}
matches!(map.find(map.get_parent_node(id)), Some(Node::Param(_)))
}

impl<'a, 'tcx> Delegate<'tcx> for EscapeDelegate<'a, 'tcx> {
Expand Down
Loading

0 comments on commit 4c2d7ee

Please sign in to comment.