Skip to content

Commit

Permalink
Auto merge of rust-lang#6415 - flip1995:rollup-fz7872l, r=flip1995
Browse files Browse the repository at this point in the history
Rollup of 4 pull requests

Successful merges:

 - rust-lang#6308 (add `internal-lints` feature to enable clippys internal lints (off by default))
 - rust-lang#6395 (switch Version/VersionReq usages to RustcVersion )
 - rust-lang#6402 (Add Collapsible match lint)
 - rust-lang#6407 (CONTRIBUTING: update bors queue url from buildbot2.rlo to bors.rlo)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup

changelog: rollup
  • Loading branch information
bors committed Dec 3, 2020
2 parents 4785da6 + c9da866 commit 249b6fe
Show file tree
Hide file tree
Showing 59 changed files with 1,041 additions and 238 deletions.
12 changes: 6 additions & 6 deletions .github/workflows/clippy_bors.yml
Original file line number Diff line number Diff line change
Expand Up @@ -128,14 +128,14 @@ jobs:
SYSROOT=$(rustc --print sysroot)
echo "$SYSROOT/bin" >> $GITHUB_PATH
- name: Build
run: cargo build --features deny-warnings
- name: Build with internal lints
run: cargo build --features deny-warnings,internal-lints

- name: Test
run: cargo test --features deny-warnings
- name: Test with internal lints
run: cargo test --features deny-warnings,internal-lints

- name: Test clippy_lints
run: cargo test --features deny-warnings
- name: Test clippy_lints with internal lints
run: cargo test --features deny-warnings,internal-lints
working-directory: clippy_lints

- name: Test rustc_tools_util
Expand Down
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -1770,6 +1770,7 @@ Released 2018-09-13
[`cmp_owned`]: https://rust-lang.github.io/rust-clippy/master/index.html#cmp_owned
[`cognitive_complexity`]: https://rust-lang.github.io/rust-clippy/master/index.html#cognitive_complexity
[`collapsible_if`]: https://rust-lang.github.io/rust-clippy/master/index.html#collapsible_if
[`collapsible_match`]: https://rust-lang.github.io/rust-clippy/master/index.html#collapsible_match
[`comparison_chain`]: https://rust-lang.github.io/rust-clippy/master/index.html#comparison_chain
[`comparison_to_empty`]: https://rust-lang.github.io/rust-clippy/master/index.html#comparison_to_empty
[`copy_iterator`]: https://rust-lang.github.io/rust-clippy/master/index.html#copy_iterator
Expand Down
9 changes: 7 additions & 2 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,16 @@ All contributors are expected to follow the [Rust Code of Conduct].

- [Contributing to Clippy](#contributing-to-clippy)
- [Getting started](#getting-started)
- [High level approach](#high-level-approach)
- [Finding something to fix/improve](#finding-something-to-fiximprove)
- [Writing code](#writing-code)
- [Getting code-completion for rustc internals to work](#getting-code-completion-for-rustc-internals-to-work)
- [How Clippy works](#how-clippy-works)
- [Fixing build failures caused by Rust](#fixing-build-failures-caused-by-rust)
- [Patching git-subtree to work with big repos](#patching-git-subtree-to-work-with-big-repos)
- [Performing the sync](#performing-the-sync)
- [Syncing back changes in Clippy to [`rust-lang/rust`]](#syncing-back-changes-in-clippy-to-rust-langrust)
- [Defining remotes](#defining-remotes)
- [Issue and PR triage](#issue-and-pr-triage)
- [Bors and Homu](#bors-and-homu)
- [Contributions](#contributions)
Expand Down Expand Up @@ -320,8 +325,8 @@ commands [here][homu_instructions].
[l-crash]: https://github.com/rust-lang/rust-clippy/labels/L-crash
[l-bug]: https://github.com/rust-lang/rust-clippy/labels/L-bug
[homu]: https://github.com/rust-lang/homu
[homu_instructions]: https://buildbot2.rust-lang.org/homu/
[homu_queue]: https://buildbot2.rust-lang.org/homu/queue/clippy
[homu_instructions]: https://bors.rust-lang.org/
[homu_queue]: https://bors.rust-lang.org/queue/clippy

## Contributions

Expand Down
5 changes: 3 additions & 2 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ path = "src/driver.rs"
clippy_lints = { version = "0.0.212", path = "clippy_lints" }
# end automatic update
semver = "0.11"
rustc_tools_util = { version = "0.2.0", path = "rustc_tools_util"}
rustc_tools_util = { version = "0.2.0", path = "rustc_tools_util" }
tempfile = { version = "3.1.0", optional = true }

[dev-dependencies]
Expand All @@ -49,8 +49,9 @@ derive-new = "0.5"
rustc-workspace-hack = "1.0.0"

[build-dependencies]
rustc_tools_util = { version = "0.2.0", path = "rustc_tools_util"}
rustc_tools_util = { version = "0.2.0", path = "rustc_tools_util" }

[features]
deny-warnings = []
integration = ["tempfile"]
internal-lints = ["clippy_lints/internal-lints"]
32 changes: 23 additions & 9 deletions clippy_dev/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -146,16 +146,30 @@ pub fn gen_deprecated<'a>(lints: impl Iterator<Item = &'a Lint>) -> Vec<String>
}

#[must_use]
pub fn gen_register_lint_list<'a>(lints: impl Iterator<Item = &'a Lint>) -> Vec<String> {
let pre = " store.register_lints(&[".to_string();
let post = " ]);".to_string();
let mut inner = lints
pub fn gen_register_lint_list<'a>(
internal_lints: impl Iterator<Item = &'a Lint>,
usable_lints: impl Iterator<Item = &'a Lint>,
) -> Vec<String> {
let header = " store.register_lints(&[".to_string();
let footer = " ]);".to_string();
let internal_lints = internal_lints
.sorted_by_key(|l| format!(" &{}::{},", l.module, l.name.to_uppercase()))
.map(|l| {
format!(
" #[cfg(feature = \"internal-lints\")]\n &{}::{},",
l.module,
l.name.to_uppercase()
)
});
let other_lints = usable_lints
.sorted_by_key(|l| format!(" &{}::{},", l.module, l.name.to_uppercase()))
.map(|l| format!(" &{}::{},", l.module, l.name.to_uppercase()))
.sorted()
.collect::<Vec<String>>();
inner.insert(0, pre);
inner.push(post);
inner
.sorted();
let mut lint_list = vec![header];
lint_list.extend(internal_lints);
lint_list.extend(other_lints);
lint_list.push(footer);
lint_list
}

/// Gathers all files in `src/clippy_lints` and gathers all lints inside
Expand Down
2 changes: 1 addition & 1 deletion clippy_dev/src/update_lints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ pub fn run(update_mode: UpdateMode) {
"end register lints",
false,
update_mode == UpdateMode::Change,
|| gen_register_lint_list(usable_lints.iter().chain(internal_lints.iter())),
|| gen_register_lint_list(internal_lints.iter(), usable_lints.iter()),
)
.changed;

Expand Down
3 changes: 3 additions & 0 deletions clippy_lints/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ smallvec = { version = "1", features = ["union"] }
toml = "0.5.3"
unicode-normalization = "0.1"
semver = "0.11"
rustc-semver="1.1.0"
# NOTE: cargo requires serde feat in its url dep
# see <https://github.com/rust-lang/rust/pull/63587#issuecomment-522343864>
url = { version = "2.1.0", features = ["serde"] }
Expand All @@ -36,3 +37,5 @@ syn = { version = "1", features = ["full"] }

[features]
deny-warnings = []
# build clippy with internal lints enabled, off by default
internal-lints = []
172 changes: 172 additions & 0 deletions clippy_lints/src/collapsible_match.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,172 @@
use crate::utils::visitors::LocalUsedVisitor;
use crate::utils::{span_lint_and_then, SpanlessEq};
use if_chain::if_chain;
use rustc_hir::def::{CtorKind, CtorOf, DefKind, Res};
use rustc_hir::{Arm, Expr, ExprKind, Guard, HirId, Pat, PatKind, QPath, StmtKind};
use rustc_lint::{LateContext, LateLintPass};
use rustc_middle::ty::{DefIdTree, TyCtxt};
use rustc_session::{declare_lint_pass, declare_tool_lint};
use rustc_span::{MultiSpan, Span};

declare_clippy_lint! {
/// **What it does:** Finds nested `match` or `if let` expressions where the patterns may be "collapsed" together
/// without adding any branches.
///
/// Note that this lint is not intended to find _all_ cases where nested match patterns can be merged, but only
/// cases where merging would most likely make the code more readable.
///
/// **Why is this bad?** It is unnecessarily verbose and complex.
///
/// **Known problems:** None.
///
/// **Example:**
///
/// ```rust
/// fn func(opt: Option<Result<u64, String>>) {
/// let n = match opt {
/// Some(n) => match n {
/// Ok(n) => n,
/// _ => return,
/// }
/// None => return,
/// };
/// }
/// ```
/// Use instead:
/// ```rust
/// fn func(opt: Option<Result<u64, String>>) {
/// let n = match opt {
/// Some(Ok(n)) => n,
/// _ => return,
/// };
/// }
/// ```
pub COLLAPSIBLE_MATCH,
style,
"Nested `match` or `if let` expressions where the patterns may be \"collapsed\" together."
}

declare_lint_pass!(CollapsibleMatch => [COLLAPSIBLE_MATCH]);

impl<'tcx> LateLintPass<'tcx> for CollapsibleMatch {
fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &Expr<'tcx>) {
if let ExprKind::Match(_expr, arms, _source) = expr.kind {
if let Some(wild_arm) = arms.iter().rfind(|arm| arm_is_wild_like(arm, cx.tcx)) {
for arm in arms {
check_arm(arm, wild_arm, cx);
}
}
}
}
}

fn check_arm(arm: &Arm<'_>, wild_outer_arm: &Arm<'_>, cx: &LateContext<'_>) {
if_chain! {
let expr = strip_singleton_blocks(arm.body);
if let ExprKind::Match(expr_in, arms_inner, _) = expr.kind;
// the outer arm pattern and the inner match
if expr_in.span.ctxt() == arm.pat.span.ctxt();
// there must be no more than two arms in the inner match for this lint
if arms_inner.len() == 2;
// no if guards on the inner match
if arms_inner.iter().all(|arm| arm.guard.is_none());
// match expression must be a local binding
// match <local> { .. }
if let ExprKind::Path(QPath::Resolved(None, path)) = expr_in.kind;
if let Res::Local(binding_id) = path.res;
// one of the branches must be "wild-like"
if let Some(wild_inner_arm_idx) = arms_inner.iter().rposition(|arm_inner| arm_is_wild_like(arm_inner, cx.tcx));
let (wild_inner_arm, non_wild_inner_arm) =
(&arms_inner[wild_inner_arm_idx], &arms_inner[1 - wild_inner_arm_idx]);
if !pat_contains_or(non_wild_inner_arm.pat);
// the binding must come from the pattern of the containing match arm
// ..<local>.. => match <local> { .. }
if let Some(binding_span) = find_pat_binding(arm.pat, binding_id);
// the "wild-like" branches must be equal
if SpanlessEq::new(cx).eq_expr(wild_inner_arm.body, wild_outer_arm.body);
// the binding must not be used in the if guard
if !matches!(arm.guard, Some(Guard::If(guard)) if LocalUsedVisitor::new(binding_id).check_expr(guard));
// ...or anywhere in the inner match
if !arms_inner.iter().any(|arm| LocalUsedVisitor::new(binding_id).check_arm(arm));
then {
span_lint_and_then(
cx,
COLLAPSIBLE_MATCH,
expr.span,
"Unnecessary nested match",
|diag| {
let mut help_span = MultiSpan::from_spans(vec![binding_span, non_wild_inner_arm.pat.span]);
help_span.push_span_label(binding_span, "Replace this binding".into());
help_span.push_span_label(non_wild_inner_arm.pat.span, "with this pattern".into());
diag.span_help(help_span, "The outer pattern can be modified to include the inner pattern.");
},
);
}
}
}

fn strip_singleton_blocks<'hir>(mut expr: &'hir Expr<'hir>) -> &'hir Expr<'hir> {
while let ExprKind::Block(block, _) = expr.kind {
match (block.stmts, block.expr) {
([stmt], None) => match stmt.kind {
StmtKind::Expr(e) | StmtKind::Semi(e) => expr = e,
_ => break,
},
([], Some(e)) => expr = e,
_ => break,
}
}
expr
}

/// A "wild-like" pattern is wild ("_") or `None`.
/// For this lint to apply, both the outer and inner match expressions
/// must have "wild-like" branches that can be combined.
fn arm_is_wild_like(arm: &Arm<'_>, tcx: TyCtxt<'_>) -> bool {
if arm.guard.is_some() {
return false;
}
match arm.pat.kind {
PatKind::Binding(..) | PatKind::Wild => true,
PatKind::Path(QPath::Resolved(None, path)) if is_none_ctor(path.res, tcx) => true,
_ => false,
}
}

fn find_pat_binding(pat: &Pat<'_>, hir_id: HirId) -> Option<Span> {
let mut span = None;
pat.walk_short(|p| match &p.kind {
// ignore OR patterns
PatKind::Or(_) => false,
PatKind::Binding(_bm, _, _ident, _) => {
let found = p.hir_id == hir_id;
if found {
span = Some(p.span);
}
!found
},
_ => true,
});
span
}

fn pat_contains_or(pat: &Pat<'_>) -> bool {
let mut result = false;
pat.walk(|p| {
let is_or = matches!(p.kind, PatKind::Or(_));
result |= is_or;
!is_or
});
result
}

fn is_none_ctor(res: Res, tcx: TyCtxt<'_>) -> bool {
if let Some(none_id) = tcx.lang_items().option_none_variant() {
if let Res::Def(DefKind::Ctor(CtorOf::Variant, CtorKind::Const), id) = res {
if let Some(variant_id) = tcx.parent(id) {
return variant_id == none_id;
}
}
}
false
}
3 changes: 1 addition & 2 deletions clippy_lints/src/default.rs
Original file line number Diff line number Diff line change
Expand Up @@ -280,8 +280,7 @@ fn field_reassigned_by_stmt<'tcx>(this: &Stmt<'tcx>, binding_name: Symbol) -> Op
// only take assignments to fields where the left-hand side field is a field of
// the same binding as the previous statement
if let ExprKind::Field(ref binding, field_ident) = assign_lhs.kind;
if let ExprKind::Path(ref qpath) = binding.kind;
if let QPath::Resolved(_, path) = qpath;
if let ExprKind::Path(QPath::Resolved(_, path)) = binding.kind;
if let Some(second_binding_name) = path.segments.last();
if second_binding_name.ident.name == binding_name;
then {
Expand Down
3 changes: 1 addition & 2 deletions clippy_lints/src/if_let_some_result.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,8 +41,7 @@ declare_lint_pass!(OkIfLet => [IF_LET_SOME_RESULT]);
impl<'tcx> LateLintPass<'tcx> for OkIfLet {
fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
if_chain! { //begin checking variables
if let ExprKind::Match(ref op, ref body, source) = expr.kind; //test if expr is a match
if let MatchSource::IfLetDesugar { .. } = source; //test if it is an If Let
if let ExprKind::Match(ref op, ref body, MatchSource::IfLetDesugar { .. }) = expr.kind; //test if expr is if let
if let ExprKind::MethodCall(_, ok_span, ref result_types, _) = op.kind; //check is expr.ok() has type Result<T,E>.ok(, _)
if let PatKind::TupleStruct(QPath::Resolved(_, ref x), ref y, _) = body[0].pat.kind; //get operation
if method_chain_args(op, &["ok"]).is_some(); //test to see if using ok() methoduse std::marker::Sized;
Expand Down
3 changes: 1 addition & 2 deletions clippy_lints/src/implicit_return.rs
Original file line number Diff line number Diff line change
Expand Up @@ -68,8 +68,7 @@ fn expr_match(cx: &LateContext<'_>, expr: &Expr<'_>) {
if_chain! {
if let StmtKind::Semi(expr, ..) = &stmt.kind;
// make sure it's a break, otherwise we want to skip
if let ExprKind::Break(.., break_expr) = &expr.kind;
if let Some(break_expr) = break_expr;
if let ExprKind::Break(.., Some(break_expr)) = &expr.kind;
then {
lint(cx, expr.span, break_expr.span, LINT_BREAK);
}
Expand Down
3 changes: 1 addition & 2 deletions clippy_lints/src/implicit_saturating_sub.rs
Original file line number Diff line number Diff line change
Expand Up @@ -59,8 +59,7 @@ impl<'tcx> LateLintPass<'tcx> for ImplicitSaturatingSub {
if let Some(target) = subtracts_one(cx, e);

// Extracting out the variable name
if let ExprKind::Path(ref assign_path) = target.kind;
if let QPath::Resolved(_, ref ares_path) = assign_path;
if let ExprKind::Path(QPath::Resolved(_, ref ares_path)) = target.kind;

then {
// Handle symmetric conditions in the if statement
Expand Down
3 changes: 1 addition & 2 deletions clippy_lints/src/large_const_arrays.rs
Original file line number Diff line number Diff line change
Expand Up @@ -52,8 +52,7 @@ impl<'tcx> LateLintPass<'tcx> for LargeConstArrays {
if let ItemKind::Const(hir_ty, _) = &item.kind;
let ty = hir_ty_to_ty(cx.tcx, hir_ty);
if let ty::Array(element_type, cst) = ty.kind();
if let ConstKind::Value(val) = cst.val;
if let ConstValue::Scalar(element_count) = val;
if let ConstKind::Value(ConstValue::Scalar(element_count)) = cst.val;
if let Ok(element_count) = element_count.to_machine_usize(&cx.tcx);
if let Ok(element_size) = cx.layout_of(element_type).map(|l| l.size.bytes());
if self.maximum_allowed_size < element_count * element_size;
Expand Down
3 changes: 1 addition & 2 deletions clippy_lints/src/large_stack_arrays.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,7 @@ impl<'tcx> LateLintPass<'tcx> for LargeStackArrays {
if_chain! {
if let ExprKind::Repeat(_, _) = expr.kind;
if let ty::Array(element_type, cst) = cx.typeck_results().expr_ty(expr).kind();
if let ConstKind::Value(val) = cst.val;
if let ConstValue::Scalar(element_count) = val;
if let ConstKind::Value(ConstValue::Scalar(element_count)) = cst.val;
if let Ok(element_count) = element_count.to_machine_usize(&cx.tcx);
if let Ok(element_size) = cx.layout_of(element_type).map(|l| l.size.bytes());
if self.maximum_allowed_size < element_count * element_size;
Expand Down
Loading

0 comments on commit 249b6fe

Please sign in to comment.