Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

#

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Kickstart the usage of let_chains #8360

Closed
wants to merge 2 commits into from
Closed

Kickstart the usage of let_chains #8360

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
004d459
Kickstart the usage of `let_chains`
c410-f3r Jan 27, 2022
250e93c
Add space
c410-f3r Jan 27, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
59 changes: 29 additions & 30 deletions clippy_lints/src/absurd_extreme_comparisons.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,36 +46,35 @@ declare_lint_pass!(AbsurdExtremeComparisons => [ABSURD_EXTREME_COMPARISONS]);

impl<'tcx> LateLintPass<'tcx> for AbsurdExtremeComparisons {
fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
if let ExprKind::Binary(ref cmp, lhs, rhs) = expr.kind {
if let Some((culprit, result)) = detect_absurd_comparison(cx, cmp.node, lhs, rhs) {
if !expr.span.from_expansion() {
let msg = "this comparison involving the minimum or maximum element for this \
type contains a case that is always true or always false";

let conclusion = match result {
AbsurdComparisonResult::AlwaysFalse => "this comparison is always false".to_owned(),
AbsurdComparisonResult::AlwaysTrue => "this comparison is always true".to_owned(),
AbsurdComparisonResult::InequalityImpossible => format!(
"the case where the two sides are not equal never occurs, consider using `{} == {}` \
instead",
snippet(cx, lhs.span, "lhs"),
snippet(cx, rhs.span, "rhs")
),
};

let help = format!(
"because `{}` is the {} value for this type, {}",
snippet(cx, culprit.expr.span, "x"),
match culprit.which {
ExtremeType::Minimum => "minimum",
ExtremeType::Maximum => "maximum",
},
conclusion
);

span_lint_and_help(cx, ABSURD_EXTREME_COMPARISONS, expr.span, msg, None, &help);
}
}
if let ExprKind::Binary(ref cmp, lhs, rhs) = expr.kind
&& let Some((culprit, result)) = detect_absurd_comparison(cx, cmp.node, lhs, rhs)
&& !expr.span.from_expansion()
{
let msg = "this comparison involving the minimum or maximum element for this \
type contains a case that is always true or always false";

let conclusion = match result {
AbsurdComparisonResult::AlwaysFalse => "this comparison is always false".to_owned(),
AbsurdComparisonResult::AlwaysTrue => "this comparison is always true".to_owned(),
AbsurdComparisonResult::InequalityImpossible => format!(
"the case where the two sides are not equal never occurs, consider using `{} == {}` \
instead",
snippet(cx, lhs.span, "lhs"),
snippet(cx, rhs.span, "rhs")
),
};

let help = format!(
"because `{}` is the {} value for this type, {}",
snippet(cx, culprit.expr.span, "x"),
match culprit.which {
ExtremeType::Minimum => "minimum",
ExtremeType::Maximum => "maximum",
},
conclusion
);

span_lint_and_help(cx, ABSURD_EXTREME_COMPARISONS, expr.span, msg, None, &help);
}
}
}
Expand Down
52 changes: 24 additions & 28 deletions clippy_lints/src/assign_ops.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ use clippy_utils::source::snippet_opt;
use clippy_utils::ty::implements_trait;
use clippy_utils::{binop_traits, sugg};
use clippy_utils::{eq_expr_value, trait_ref_of_method};
use if_chain::if_chain;
use rustc_errors::Applicability;
use rustc_hir as hir;
use rustc_hir::intravisit::{walk_expr, Visitor};
Expand Down Expand Up @@ -92,33 +91,30 @@ impl<'tcx> LateLintPass<'tcx> for AssignOps {
let lint = |assignee: &hir::Expr<'_>, rhs: &hir::Expr<'_>| {
let ty = cx.typeck_results().expr_ty(assignee);
let rty = cx.typeck_results().expr_ty(rhs);
if_chain! {
if let Some((_, lang_item)) = binop_traits(op.node);
if let Ok(trait_id) = cx.tcx.lang_items().require(lang_item);
let parent_fn = cx.tcx.hir().get_parent_item(e.hir_id);
if trait_ref_of_method(cx, parent_fn)
.map_or(true, |t| t.path.res.def_id() != trait_id);
if implements_trait(cx, ty, trait_id, &[rty.into()]);
then {
span_lint_and_then(
cx,
ASSIGN_OP_PATTERN,
expr.span,
"manual implementation of an assign operation",
|diag| {
if let (Some(snip_a), Some(snip_r)) =
(snippet_opt(cx, assignee.span), snippet_opt(cx, rhs.span))
{
diag.span_suggestion(
expr.span,
"replace it with",
format!("{} {}= {}", snip_a, op.node.as_str(), snip_r),
Applicability::MachineApplicable,
);
}
},
);
}
if let Some((_, lang_item)) = binop_traits(op.node)
&& let Ok(trait_id) = cx.tcx.lang_items().require(lang_item)
&& let parent_fn = cx.tcx.hir().get_parent_item(e.hir_id)
&& trait_ref_of_method(cx, parent_fn).map_or(true, |t| t.path.res.def_id() != trait_id)
&& implements_trait(cx, ty, trait_id, &[rty.into()])
{
span_lint_and_then(
cx,
ASSIGN_OP_PATTERN,
expr.span,
"manual implementation of an assign operation",
|diag| {
if let (Some(snip_a), Some(snip_r)) =
(snippet_opt(cx, assignee.span), snippet_opt(cx, rhs.span))
{
diag.span_suggestion(
expr.span,
"replace it with",
format!("{} {}= {}", snip_a, op.node.as_str(), snip_r),
Applicability::MachineApplicable,
);
}
},
);
}
};

Expand Down
64 changes: 32 additions & 32 deletions clippy_lints/src/async_yields_async.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,39 +47,39 @@ impl<'tcx> LateLintPass<'tcx> for AsyncYieldsAsync {
use AsyncGeneratorKind::{Block, Closure};
// For functions, with explicitly defined types, don't warn.
// XXXkhuey maybe we should?
if let Some(GeneratorKind::Async(Block | Closure)) = body.generator_kind {
if let Some(future_trait_def_id) = cx.tcx.lang_items().future_trait() {
let body_id = BodyId {
hir_id: body.value.hir_id,
};
let typeck_results = cx.tcx.typeck_body(body_id);
let expr_ty = typeck_results.expr_ty(&body.value);
if let Some(GeneratorKind::Async(Block | Closure)) = body.generator_kind
&& let Some(future_trait_def_id) = cx.tcx.lang_items().future_trait()
{
let body_id = BodyId {
hir_id: body.value.hir_id,
};
let typeck_results = cx.tcx.typeck_body(body_id);
let expr_ty = typeck_results.expr_ty(&body.value);

if implements_trait(cx, expr_ty, future_trait_def_id, &[]) {
let return_expr_span = match &body.value.kind {
// XXXkhuey there has to be a better way.
ExprKind::Block(block, _) => block.expr.map(|e| e.span),
ExprKind::Path(QPath::Resolved(_, path)) => Some(path.span),
_ => None,
};
if let Some(return_expr_span) = return_expr_span {
span_lint_and_then(
cx,
ASYNC_YIELDS_ASYNC,
return_expr_span,
"an async construct yields a type which is itself awaitable",
|db| {
db.span_label(body.value.span, "outer async construct");
db.span_label(return_expr_span, "awaitable value not awaited");
db.span_suggestion(
return_expr_span,
"consider awaiting this value",
format!("{}.await", snippet(cx, return_expr_span, "..")),
Applicability::MaybeIncorrect,
);
},
);
}
if implements_trait(cx, expr_ty, future_trait_def_id, &[]) {
let return_expr_span = match &body.value.kind {
// XXXkhuey there has to be a better way.
ExprKind::Block(block, _) => block.expr.map(|e| e.span),
ExprKind::Path(QPath::Resolved(_, path)) => Some(path.span),
_ => None,
};
if let Some(return_expr_span) = return_expr_span {
span_lint_and_then(
cx,
ASYNC_YIELDS_ASYNC,
return_expr_span,
"an async construct yields a type which is itself awaitable",
|db| {
db.span_label(body.value.span, "outer async construct");
db.span_label(return_expr_span, "awaitable value not awaited");
db.span_suggestion(
return_expr_span,
"consider awaiting this value",
format!("{}.await", snippet(cx, return_expr_span, "..")),
Applicability::MaybeIncorrect,
);
},
);
}
}
}
Expand Down
141 changes: 63 additions & 78 deletions clippy_lints/src/attrs.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ use clippy_utils::macros::{is_panic, macro_backtrace};
use clippy_utils::msrvs;
use clippy_utils::source::{first_line_of_span, is_present_in_source, snippet_opt, without_block_comments};
use clippy_utils::{extract_msrv_attr, meets_msrv};
use if_chain::if_chain;
use rustc_ast::{AttrKind, AttrStyle, Attribute, Lit, LitKind, MetaItemKind, NestedMetaItem};
use rustc_errors::Applicability;
use rustc_hir::{
Expand Down Expand Up @@ -264,23 +263,19 @@ declare_lint_pass!(Attributes => [

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() {
if is_lint_level(ident.name) {
check_clippy_lint_names(cx, ident.name, items);
}
if items.is_empty() || !attr.has_name(sym::deprecated) {
return;
}
for item in items {
if_chain! {
if let NestedMetaItem::MetaItem(mi) = &item;
if let MetaItemKind::NameValue(lit) = &mi.kind;
if mi.has_name(sym::since);
then {
check_semver(cx, item.span(), lit);
}
}
if let Some(items) = &attr.meta_item_list() && let Some(ident) = attr.ident() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think && let should always wrap to the next line. Does rustfmt allow either way?

if is_lint_level(ident.name) {
check_clippy_lint_names(cx, ident.name, items);
}
if items.is_empty() || !attr.has_name(sym::deprecated) {
return;
}
for item in items {
if let NestedMetaItem::MetaItem(mi) = &item
&& let MetaItemKind::NameValue(lit) = &mi.kind
&& mi.has_name(sym::since)
{
check_semver(cx, item.span(), lit);
}
}
}
Expand Down Expand Up @@ -374,15 +369,13 @@ impl<'tcx> LateLintPass<'tcx> for Attributes {

/// Returns the lint name if it is clippy lint.
fn extract_clippy_lint(lint: &NestedMetaItem) -> Option<Symbol> {
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.name == sym::clippy;
then {
let lint_name = meta_item.path.segments.last().unwrap().ident.name;
return Some(lint_name);
}
if let Some(meta_item) = lint.meta_item()
&& meta_item.path.segments.len() > 1
&& let tool_name = meta_item.path.segments[0].ident
&& tool_name.name == sym::clippy
{
let lint_name = meta_item.path.segments.last().unwrap().ident.name;
return Some(lint_name);
}
None
}
Expand Down Expand Up @@ -562,33 +555,30 @@ fn check_empty_line_after_outer_attr(cx: &EarlyContext<'_>, item: &rustc_ast::It
}

fn check_deprecated_cfg_attr(cx: &EarlyContext<'_>, attr: &Attribute, msrv: Option<RustcVersion>) {
if_chain! {
if meets_msrv(msrv.as_ref(), &msrvs::TOOL_ATTRIBUTES);
if meets_msrv(msrv.as_ref(), &msrvs::TOOL_ATTRIBUTES)
// check cfg_attr
if attr.has_name(sym::cfg_attr);
if let Some(items) = attr.meta_item_list();
if items.len() == 2;
&& attr.has_name(sym::cfg_attr)
&& let Some(items) = attr.meta_item_list()
&& items.len() == 2
// check for `rustfmt`
if let Some(feature_item) = items[0].meta_item();
if feature_item.has_name(sym::rustfmt);
&& let Some(feature_item) = items[0].meta_item()
&& feature_item.has_name(sym::rustfmt)
// check for `rustfmt_skip` and `rustfmt::skip`
if let Some(skip_item) = &items[1].meta_item();
if skip_item.has_name(sym!(rustfmt_skip)) ||
skip_item.path.segments.last().expect("empty path in attribute").ident.name == sym::skip;
&& let Some(skip_item) = items[1].meta_item()
&& (skip_item.has_name(sym!(rustfmt_skip)) || skip_item.path.segments.last().expect("empty path in attribute").ident.name == sym::skip)
// Only lint outer attributes, because custom inner attributes are unstable
// Tracking issue: https://github.com/rust-lang/rust/issues/54726
if attr.style == AttrStyle::Outer;
then {
span_lint_and_sugg(
cx,
DEPRECATED_CFG_ATTR,
attr.span,
"`cfg_attr` is deprecated for rustfmt and got replaced by tool attributes",
"use",
"#[rustfmt::skip]".to_string(),
Applicability::MachineApplicable,
);
}
&& attr.style == AttrStyle::Outer
{
span_lint_and_sugg(
cx,
DEPRECATED_CFG_ATTR,
attr.span,
"`cfg_attr` is deprecated for rustfmt and got replaced by tool attributes",
"use",
"#[rustfmt::skip]".to_string(),
Applicability::MachineApplicable,
);
}
}

Expand All @@ -615,12 +605,9 @@ fn check_mismatched_target_os(cx: &EarlyContext<'_>, attr: &Attribute) {
mismatched.extend(find_mismatched_target_os(list));
},
MetaItemKind::Word => {
if_chain! {
if let Some(ident) = meta.ident();
if let Some(os) = find_os(ident.name.as_str());
then {
mismatched.push((os, ident.span));
}
if let Some(ident) = meta.ident()
&& let Some(os) = find_os(ident.name.as_str()) {
mismatched.push((os, ident.span));
}
},
MetaItemKind::NameValue(..) => {},
Expand All @@ -631,30 +618,28 @@ fn check_mismatched_target_os(cx: &EarlyContext<'_>, attr: &Attribute) {
mismatched
}

if_chain! {
if attr.has_name(sym::cfg);
if let Some(list) = attr.meta_item_list();
let mismatched = find_mismatched_target_os(&list);
if !mismatched.is_empty();
then {
let mess = "operating system used in target family position";

span_lint_and_then(cx, MISMATCHED_TARGET_OS, attr.span, mess, |diag| {
// Avoid showing the unix suggestion multiple times in case
// we have more than one mismatch for unix-like systems
let mut unix_suggested = false;

for (os, span) in mismatched {
let sugg = format!("target_os = \"{}\"", os);
diag.span_suggestion(span, "try", sugg, Applicability::MaybeIncorrect);

if !unix_suggested && is_unix(os) {
diag.help("did you mean `unix`?");
unix_suggested = true;
}
if attr.has_name(sym::cfg)
&& let Some(list) = attr.meta_item_list()
&& let mismatched = find_mismatched_target_os(&list)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh nice, you can even put irrefutable patterns in let chains 👍

&& !mismatched.is_empty()
{
let mess = "operating system used in target family position";

span_lint_and_then(cx, MISMATCHED_TARGET_OS, attr.span, mess, |diag| {
// Avoid showing the unix suggestion multiple times in case
// we have more than one mismatch for unix-like systems
let mut unix_suggested = false;

for (os, span) in mismatched {
let sugg = format!("target_os = \"{}\"", os);
diag.span_suggestion(span, "try", sugg, Applicability::MaybeIncorrect);

if !unix_suggested && is_unix(os) {
diag.help("did you mean `unix`?");
unix_suggested = true;
}
});
}
}
});
}
}

Expand Down
Loading