Skip to content
New issue

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

Sign up for GitHub

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

Already on GitHub? Sign in to your account

Jump to bottom

Rustup to rustc 1.39.0-nightly (acf7b50c7 2019-09-25) #4574

Merged
merged 4 commits into from
Sep 25, 2019
Merged

Rustup to rustc 1.39.0-nightly (acf7b50c7 2019-09-25) #4574

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
68c26b3
Rustup to rustc 1.39.0-nightly (acf7b50c7 2019-09-25)
Manishearth Sep 25, 2019
982c51e
arm.pats -> arm.pat
Manishearth Sep 25, 2019
b5cadd7
ignore single-match for or patterns
Manishearth Sep 25, 2019
a756b9b
allow osx failures
Manishearth Sep 25, 2019
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
2 changes: 2 additions & 0 deletions .travis.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,8 @@ matrix:
allow_failures:
- os: windows
env: CARGO_INCREMENTAL=0 BASE_TESTS=true OS_WINDOWS=true
- os: osx # run base tests on both platforms
env: BASE_TESTS=true
# prevent these jobs with default env vars
exclude:
- os: linux
Expand Down
2 changes: 1 addition & 1 deletion Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ rustc_tools_util = { version = "0.2.0", path = "rustc_tools_util"}

[dev-dependencies]
cargo_metadata = "0.8.0"
compiletest_rs = { version = "0.3.22", features = ["tmp"] }
compiletest_rs = { version = "0.3.23", features = ["tmp"] }
lazy_static = "1.0"
clippy-mini-macro-test = { version = "0.2", path = "mini-macro" }
serde = { version = "1.0", features = ["derive"] }
Expand Down
3 changes: 1 addition & 2 deletions clippy_lints/src/cognitive_complexity.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -112,8 +112,7 @@ impl<'tcx> Visitor<'tcx> for CCHelper {
walk_expr(self, e);
match e.node {
ExprKind::Match(_, ref arms, _) => {
let arms_n: u64 = arms.iter().map(|arm| arm.pats.len() as u64).sum();
if arms_n > 1 {
if arms.len() > 1 {
Copy link
Contributor

Choose a reason for hiding this comment

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

If I'm reading the intent of this code you might want to recurse and count each or-pattern as cc += 1 but I think the change you made makes more sense.

Copy link
Member Author

Choose a reason for hiding this comment

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

yeah i debated this but i think we might need a more careful CC accounting for pats here

self.cc += 1;
}
self.cc += arms.iter().filter(|arm| arm.guard.is_some()).count() as u64;
Expand Down
39 changes: 17 additions & 22 deletions clippy_lints/src/copies.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -193,7 +193,7 @@ fn lint_match_arms<'tcx>(cx: &LateContext<'_, 'tcx>, expr: &Expr) {
(min_index..=max_index).all(|index| arms[index].guard.is_none()) &&
SpanlessEq::new(cx).eq_expr(&lhs.body, &rhs.body) &&
// all patterns should have the same bindings
same_bindings(cx, &bindings(cx, &lhs.pats[0]), &bindings(cx, &rhs.pats[0]))
same_bindings(cx, &bindings(cx, &lhs.pat), &bindings(cx, &rhs.pat))
};

let indexed_arms: Vec<(usize, &Arm)> = arms.iter().enumerate().collect();
Expand All @@ -213,27 +213,22 @@ fn lint_match_arms<'tcx>(cx: &LateContext<'_, 'tcx>, expr: &Expr) {
// span for the whole pattern, the suggestion is only shown when there is only
// one pattern. The user should know about `|` if they are already using it…

if i.pats.len() == 1 && j.pats.len() == 1 {
let lhs = snippet(cx, i.pats[0].span, "<pat1>");
let rhs = snippet(cx, j.pats[0].span, "<pat2>");

if let PatKind::Wild = j.pats[0].node {
// if the last arm is _, then i could be integrated into _
// note that i.pats[0] cannot be _, because that would mean that we're
// hiding all the subsequent arms, and rust won't compile
db.span_note(
i.body.span,
&format!(
"`{}` has the same arm body as the `_` wildcard, consider removing it`",
lhs
),
);
} else {
db.span_help(
i.pats[0].span,
&format!("consider refactoring into `{} | {}`", lhs, rhs),
);
}
let lhs = snippet(cx, i.pat.span, "<pat1>");
let rhs = snippet(cx, j.pat.span, "<pat2>");

if let PatKind::Wild = j.pat.node {
// if the last arm is _, then i could be integrated into _
// note that i.pat cannot be _, because that would mean that we're
// hiding all the subsequent arms, and rust won't compile
db.span_note(
i.body.span,
&format!(
"`{}` has the same arm body as the `_` wildcard, consider removing it`",
lhs
),
);
} else {
db.span_help(i.pat.span, &format!("consider refactoring into `{} | {}`", lhs, rhs));
Copy link
Contributor

Choose a reason for hiding this comment

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

Aside: In the spirit of this lint, in the future you may want a lint that suggests C(p0) | C(p1) => C(p0 | p1).

}
},
);
Expand Down
3 changes: 1 addition & 2 deletions clippy_lints/src/format.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,9 +84,8 @@ fn on_argumentv1_new<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr, arm
if let ExprKind::Path(ref qpath) = args[1].node;
if let Some(did) = resolve_node(cx, qpath, args[1].hir_id).opt_def_id();
if match_def_path(cx, did, &paths::DISPLAY_FMT_METHOD);
if arms[0].pats.len() == 1;
// check `(arg0,)` in match block
if let PatKind::Tuple(ref pats, None) = arms[0].pats[0].node;
if let PatKind::Tuple(ref pats, None) = arms[0].pat.node;
if pats.len() == 1;
then {
let ty = walk_ptrs_ty(cx.tables.pat_ty(&pats[0]));
Expand Down
4 changes: 2 additions & 2 deletions clippy_lints/src/infallible_destructuring_match.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,8 +47,8 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for InfallibleDestructingMatch {
if_chain! {
if let Some(ref expr) = local.init;
if let ExprKind::Match(ref target, ref arms, MatchSource::Normal) = expr.node;
if arms.len() == 1 && arms[0].pats.len() == 1 && arms[0].guard.is_none();
if let PatKind::TupleStruct(QPath::Resolved(None, ref variant_name), ref args, _) = arms[0].pats[0].node;
if arms.len() == 1 && arms[0].guard.is_none();
if let PatKind::TupleStruct(QPath::Resolved(None, ref variant_name), ref args, _) = arms[0].pat.node;
if args.len() == 1;
if let Some(arg) = get_arg_name(&args[0]);
let body = remove_blocks(&arms[0].body);
Expand Down
6 changes: 2 additions & 4 deletions clippy_lints/src/loops.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -517,9 +517,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Loops {
match *source {
MatchSource::Normal | MatchSource::IfLetDesugar { .. } => {
if arms.len() == 2
&& arms[0].pats.len() == 1
&& arms[0].guard.is_none()
&& arms[1].pats.len() == 1
&& arms[1].guard.is_none()
&& is_simple_break_expr(&arms[1].body)
{
Expand All @@ -541,7 +539,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Loops {
"try",
format!(
"while let {} = {} {{ .. }}",
snippet_with_applicability(cx, arms[0].pats[0].span, "..", &mut applicability),
snippet_with_applicability(cx, arms[0].pat.span, "..", &mut applicability),
snippet_with_applicability(cx, matchexpr.span, "..", &mut applicability),
),
applicability,
Expand All @@ -554,7 +552,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Loops {
}
}
if let ExprKind::Match(ref match_expr, ref arms, MatchSource::WhileLetDesugar) = expr.node {
let pat = &arms[0].pats[0].node;
let pat = &arms[0].pat.node;
if let (
&PatKind::TupleStruct(ref qpath, ref pat_args, _),
&ExprKind::MethodCall(ref method_path, _, ref method_args),
Expand Down
88 changes: 37 additions & 51 deletions clippy_lints/src/matches.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ use rustc::{declare_lint_pass, declare_tool_lint};
use rustc_errors::Applicability;
use std::cmp::Ordering;
use std::collections::Bound;
use std::ops::Deref;
use syntax::ast::LitKind;
use syntax::source_map::Span;

Expand Down Expand Up @@ -255,9 +254,12 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Matches {

#[rustfmt::skip]
fn check_single_match(cx: &LateContext<'_, '_>, ex: &Expr, arms: &[Arm], expr: &Expr) {
if arms.len() == 2 &&
arms[0].pats.len() == 1 && arms[0].guard.is_none() &&
arms[1].pats.len() == 1 && arms[1].guard.is_none() {
if arms.len() == 2 && arms[0].guard.is_none() && arms[1].guard.is_none() {
if let PatKind::Or(..) = arms[0].pat.node {
// don't lint for or patterns for now, this makes
// the lint noisy in unnecessary situations
return;
}
let els = remove_blocks(&arms[1].body);
let els = if is_unit_expr(els) {
None
Expand All @@ -283,7 +285,7 @@ fn check_single_match_single_pattern(
expr: &Expr,
els: Option<&Expr>,
) {
if is_wild(&arms[1].pats[0]) {
if is_wild(&arms[1].pat) {
report_single_match_single_pattern(cx, ex, arms, expr, els);
}
}
Expand All @@ -308,7 +310,7 @@ fn report_single_match_single_pattern(
"try this",
format!(
"if let {} = {} {}{}",
snippet(cx, arms[0].pats[0].span, ".."),
snippet(cx, arms[0].pat.span, ".."),
snippet(cx, ex.span, ".."),
expr_block(cx, &arms[0].body, None, ".."),
els_str,
Expand Down Expand Up @@ -336,7 +338,7 @@ fn check_single_match_opt_like(
(&paths::RESULT, "Ok"),
];

let path = match arms[1].pats[0].node {
let path = match arms[1].pat.node {
PatKind::TupleStruct(ref path, ref inner, _) => {
// Contains any non wildcard patterns (e.g., `Err(err)`)?
if !inner.iter().all(is_wild) {
Expand Down Expand Up @@ -365,9 +367,9 @@ fn check_match_bool(cx: &LateContext<'_, '_>, ex: &Expr, arms: &[Arm], expr: &Ex
expr.span,
"you seem to be trying to match on a boolean expression",
move |db| {
if arms.len() == 2 && arms[0].pats.len() == 1 {
if arms.len() == 2 {
// no guards
let exprs = if let PatKind::Lit(ref arm_bool) = arms[0].pats[0].node {
let exprs = if let PatKind::Lit(ref arm_bool) = arms[0].pat.node {
if let ExprKind::Lit(ref lit) = arm_bool.node {
match lit.node {
LitKind::Bool(true) => Some((&*arms[0].body, &*arms[1].body)),
Expand Down Expand Up @@ -446,7 +448,7 @@ fn check_wild_err_arm(cx: &LateContext<'_, '_>, ex: &Expr, arms: &[Arm]) {
let ex_ty = walk_ptrs_ty(cx.tables.expr_ty(ex));
if match_type(cx, ex_ty, &paths::RESULT) {
for arm in arms {
if let PatKind::TupleStruct(ref path, ref inner, _) = arm.pats[0].node {
if let PatKind::TupleStruct(ref path, ref inner, _) = arm.pat.node {
let path_str = print::to_string(print::NO_ANN, |s| s.print_qpath(path, false));
if_chain! {
if path_str == "Err";
Expand All @@ -457,9 +459,9 @@ fn check_wild_err_arm(cx: &LateContext<'_, '_>, ex: &Expr, arms: &[Arm]) {
// `Err(_)` arm with `panic!` found
span_note_and_lint(cx,
MATCH_WILD_ERR_ARM,
arm.pats[0].span,
arm.pat.span,
"Err(_) will match all errors, maybe not a good idea",
arm.pats[0].span,
arm.pat.span,
"to remove this warning, match each error separately \
or use unreachable macro");
}
Expand All @@ -482,13 +484,11 @@ fn check_wild_enum_match(cx: &LateContext<'_, '_>, ex: &Expr, arms: &[Arm]) {
let mut wildcard_span = None;
let mut wildcard_ident = None;
for arm in arms {
for pat in &arm.pats {
if let PatKind::Wild = pat.node {
wildcard_span = Some(pat.span);
} else if let PatKind::Binding(_, _, ident, None) = pat.node {
wildcard_span = Some(pat.span);
wildcard_ident = Some(ident);
}
if let PatKind::Wild = arm.pat.node {
wildcard_span = Some(arm.pat.span);
} else if let PatKind::Binding(_, _, ident, None) = arm.pat.node {
wildcard_span = Some(arm.pat.span);
wildcard_ident = Some(ident);
}
}

Expand All @@ -510,15 +510,13 @@ fn check_wild_enum_match(cx: &LateContext<'_, '_>, ex: &Expr, arms: &[Arm]) {
// covered by the set of guards that cover it, but that's really hard to do.
continue;
}
for pat in &arm.pats {
if let PatKind::Path(ref path) = pat.deref().node {
if let QPath::Resolved(_, p) = path {
missing_variants.retain(|e| e.ctor_def_id != Some(p.res.def_id()));
}
} else if let PatKind::TupleStruct(ref path, ..) = pat.deref().node {
if let QPath::Resolved(_, p) = path {
missing_variants.retain(|e| e.ctor_def_id != Some(p.res.def_id()));
}
if let PatKind::Path(ref path) = arm.pat.node {
if let QPath::Resolved(_, p) = path {
missing_variants.retain(|e| e.ctor_def_id != Some(p.res.def_id()));
}
} else if let PatKind::TupleStruct(ref path, ..) = arm.pat.node {
if let QPath::Resolved(_, p) = path {
missing_variants.retain(|e| e.ctor_def_id != Some(p.res.def_id()));
}
}
}
Expand Down Expand Up @@ -588,9 +586,9 @@ fn check_match_ref_pats(cx: &LateContext<'_, '_>, ex: &Expr, arms: &[Arm], expr:
)
};

suggs.extend(arms.iter().flat_map(|a| &a.pats).filter_map(|p| {
if let PatKind::Ref(ref refp, _) = p.node {
Some((p.span, snippet(cx, refp.span, "..").to_string()))
suggs.extend(arms.iter().filter_map(|a| {
if let PatKind::Ref(ref refp, _) = a.pat.node {
Some((a.pat.span, snippet(cx, refp.span, "..").to_string()))
} else {
None
}
Expand All @@ -605,12 +603,7 @@ fn check_match_ref_pats(cx: &LateContext<'_, '_>, ex: &Expr, arms: &[Arm], expr:
}

fn check_match_as_ref(cx: &LateContext<'_, '_>, ex: &Expr, arms: &[Arm], expr: &Expr) {
if arms.len() == 2
&& arms[0].pats.len() == 1
&& arms[0].guard.is_none()
&& arms[1].pats.len() == 1
&& arms[1].guard.is_none()
{
if arms.len() == 2 && arms[0].guard.is_none() && arms[1].guard.is_none() {
Copy link
Contributor

Choose a reason for hiding this comment

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

I feel that I've seen this pattern a few times now btw... maybe refactor in a follow up PR.

let arm_ref: Option<BindingAnnotation> = if is_none_arm(&arms[0]) {
is_ref_some_arm(&arms[1])
} else if is_none_arm(&arms[1]) {
Expand Down Expand Up @@ -666,14 +659,9 @@ fn all_ranges<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, arms: &'tcx [Arm]) -> Vec<Sp
arms.iter()
.flat_map(|arm| {
if let Arm {
ref pats, guard: None, ..
ref pat, guard: None, ..
} = *arm
{
pats.iter()
} else {
[].iter()
}
.filter_map(|pat| {
if let PatKind::Range(ref lhs, ref rhs, ref range_end) = pat.node {
let lhs = constant(cx, cx.tables, lhs)?.0;
let rhs = constant(cx, cx.tables, rhs)?.0;
Expand All @@ -694,9 +682,8 @@ fn all_ranges<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, arms: &'tcx [Arm]) -> Vec<Sp
node: (value.clone(), Bound::Included(value)),
});
}

None
})
}
None
})
.collect()
}
Expand Down Expand Up @@ -743,7 +730,7 @@ fn is_unit_expr(expr: &Expr) -> bool {

// Checks if arm has the form `None => None`
fn is_none_arm(arm: &Arm) -> bool {
match arm.pats[0].node {
match arm.pat.node {
PatKind::Path(ref path) if match_qpath(path, &paths::OPTION_NONE) => true,
_ => false,
}
Expand All @@ -752,7 +739,7 @@ fn is_none_arm(arm: &Arm) -> bool {
// Checks if arm has the form `Some(ref v) => Some(v)` (checks for `ref` and `ref mut`)
fn is_ref_some_arm(arm: &Arm) -> Option<BindingAnnotation> {
if_chain! {
if let PatKind::TupleStruct(ref path, ref pats, _) = arm.pats[0].node;
if let PatKind::TupleStruct(ref path, ref pats, _) = arm.pat.node;
if pats.len() == 1 && match_qpath(path, &paths::OPTION_SOME);
if let PatKind::Binding(rb, .., ident, _) = pats[0].node;
if rb == BindingAnnotation::Ref || rb == BindingAnnotation::RefMut;
Expand All @@ -772,9 +759,8 @@ fn is_ref_some_arm(arm: &Arm) -> Option<BindingAnnotation> {
fn has_only_ref_pats(arms: &[Arm]) -> bool {
let mapped = arms
.iter()
.flat_map(|a| &a.pats)
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd just inline into arm.p.node here.

.map(|p| {
match p.node {
.map(|a| {
match a.pat.node {
PatKind::Ref(..) => Some(true), // &-patterns
PatKind::Wild => Some(false), // an "anything" wildcard is also fine
_ => None, // any other pattern is not fine
Expand Down
2 changes: 1 addition & 1 deletion clippy_lints/src/ok_if_let.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for OkIfLet {
if let ExprKind::Match(ref op, ref body, ref source) = expr.node; //test if expr is a match
if let MatchSource::IfLetDesugar { .. } = *source; //test if it is an If Let
if let ExprKind::MethodCall(_, _, ref result_types) = op.node; //check is expr.ok() has type Result<T,E>.ok()
if let PatKind::TupleStruct(QPath::Resolved(_, ref x), ref y, _) = body[0].pats[0].node; //get operation
if let PatKind::TupleStruct(QPath::Resolved(_, ref x), ref y, _) = body[0].pat.node; //get operation
if method_chain_args(op, &["ok"]).is_some(); //test to see if using ok() methoduse std::marker::Sized;

then {
Expand Down
Loading