Skip to content

Commit

Permalink
Merge commit 'c1664c50b27a51f7a78c93ba65558e7c33eabee6' into clippyup
Browse files Browse the repository at this point in the history
  • Loading branch information
flip1995 committed Dec 6, 2020
1 parent 3be53bc commit 8eca423
Show file tree
Hide file tree
Showing 139 changed files with 4,334 additions and 576 deletions.
12 changes: 6 additions & 6 deletions .github/workflows/clippy_bors.yml
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
3 changes: 3 additions & 0 deletions CHANGELOG.md
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 Expand Up @@ -2056,6 +2057,7 @@ Released 2018-09-13
[`single_element_loop`]: https://rust-lang.github.io/rust-clippy/master/index.html#single_element_loop
[`single_match`]: https://rust-lang.github.io/rust-clippy/master/index.html#single_match
[`single_match_else`]: https://rust-lang.github.io/rust-clippy/master/index.html#single_match_else
[`size_of_in_element_count`]: https://rust-lang.github.io/rust-clippy/master/index.html#size_of_in_element_count
[`skip_while_next`]: https://rust-lang.github.io/rust-clippy/master/index.html#skip_while_next
[`slow_vector_initialization`]: https://rust-lang.github.io/rust-clippy/master/index.html#slow_vector_initialization
[`stable_sort_primitive`]: https://rust-lang.github.io/rust-clippy/master/index.html#stable_sort_primitive
Expand All @@ -2073,6 +2075,7 @@ Released 2018-09-13
[`suspicious_else_formatting`]: https://rust-lang.github.io/rust-clippy/master/index.html#suspicious_else_formatting
[`suspicious_map`]: https://rust-lang.github.io/rust-clippy/master/index.html#suspicious_map
[`suspicious_op_assign_impl`]: https://rust-lang.github.io/rust-clippy/master/index.html#suspicious_op_assign_impl
[`suspicious_operation_groupings`]: https://rust-lang.github.io/rust-clippy/master/index.html#suspicious_operation_groupings
[`suspicious_unary_op_formatting`]: https://rust-lang.github.io/rust-clippy/master/index.html#suspicious_unary_op_formatting
[`tabs_in_doc_comments`]: https://rust-lang.github.io/rust-clippy/master/index.html#tabs_in_doc_comments
[`temporary_assignment`]: https://rust-lang.github.io/rust-clippy/master/index.html#temporary_assignment
Expand Down
9 changes: 7 additions & 2 deletions CONTRIBUTING.md
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
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"]
29 changes: 28 additions & 1 deletion README.md
Expand Up @@ -182,7 +182,7 @@ cargo clippy -- -W clippy::lint_name
```

This also works with lint groups. For example you
can run Clippy with warnings for all lints enabled:
can run Clippy with warnings for all lints enabled:
```terminal
cargo clippy -- -W clippy::pedantic
```
Expand All @@ -194,6 +194,33 @@ cargo clippy -- -A clippy::all -W clippy::useless_format -W clippy::...
```
Note that if you've run clippy before, this may only take effect after you've modified a file or ran `cargo clean`.

### Specifying the minimum supported Rust version

Projects that intend to support old versions of Rust can disable lints pertaining to newer features by
specifying the minimum supported Rust version (MSRV) in the clippy configuration file.

```toml
msrv = "1.30.0"
```

The MSRV can also be specified as an inner attribute, like below.

```rust
#![feature(custom_inner_attributes)]
#![clippy::msrv = "1.30.0"]

fn main() {
...
}
```

You can also omit the patch version when specifying the MSRV, so `msrv = 1.30`
is equivalent to `msrv = 1.30.0`.

Note: `custom_inner_attributes` is an unstable feature so it has to be enabled explicitly.

Lints that recognize this configuration option can be found [here](https://rust-lang.github.io/rust-clippy/master/index.html#msrv)

## Contributing

If you want to contribute to Clippy, you can find more information in [CONTRIBUTING.md](https://github.com/rust-lang/rust-clippy/blob/master/CONTRIBUTING.md).
Expand Down
32 changes: 23 additions & 9 deletions clippy_dev/src/lib.rs
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
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
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 = []
2 changes: 1 addition & 1 deletion clippy_lints/src/attrs.rs
Expand Up @@ -5,7 +5,6 @@ use crate::utils::{
span_lint_and_sugg, span_lint_and_then, without_block_comments,
};
use if_chain::if_chain;
use rustc_span::lev_distance::find_best_match_for_name;
use rustc_ast::{AttrKind, AttrStyle, Attribute, Lit, LitKind, MetaItemKind, NestedMetaItem};
use rustc_errors::Applicability;
use rustc_hir::{
Expand All @@ -15,6 +14,7 @@ use rustc_lint::{CheckLintNameResult, EarlyContext, EarlyLintPass, LateContext,
use rustc_middle::lint::in_external_macro;
use rustc_middle::ty;
use rustc_session::{declare_lint_pass, declare_tool_lint};
use rustc_span::lev_distance::find_best_match_for_name;
use rustc_span::source_map::Span;
use rustc_span::sym;
use rustc_span::symbol::{Symbol, SymbolStr};
Expand Down
172 changes: 172 additions & 0 deletions clippy_lints/src/collapsible_match.rs
@@ -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: 2 additions & 1 deletion clippy_lints/src/comparison_chain.rs
Expand Up @@ -12,7 +12,8 @@ declare_clippy_lint! {
/// **Why is this bad?** `if` is not guaranteed to be exhaustive and conditionals can get
/// repetitive
///
/// **Known problems:** None.
/// **Known problems:** The match statement may be slower due to the compiler
/// not inlining the call to cmp. See issue #5354
///
/// **Example:**
/// ```rust,ignore
Expand Down

0 comments on commit 8eca423

Please sign in to comment.