Skip to content

Rollup of 5 pull requests #5527

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.

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

Merged
merged 14 commits into from
Apr 25, 2020
Merged
Changes from all commits
Commits
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: 0 additions & 2 deletions .github/workflows/clippy_bors.yml
Original file line number Diff line number Diff line change
@@ -77,8 +77,6 @@ jobs:
run: |
sudo dpkg --add-architecture i386
sudo apt-get update
# perform system upgrade to work around https://github.com/rust-lang/rust-clippy/issues/5477 , revert as soon as that is fixed
sudo apt-get -y upgrade
sudo apt-get install gcc-multilib libssl-dev:i386 libgit2-dev:i386
if: matrix.host == 'i686-unknown-linux-gnu'

6 changes: 6 additions & 0 deletions .github/workflows/deploy.yml
Original file line number Diff line number Diff line change
@@ -38,6 +38,12 @@ jobs:
- name: Set beta to true
if: github.ref == 'refs/heads/beta'
run: echo "::set-env name=BETA::true"

- name: Use scripts and templates from master branch
run: |
git fetch --no-tags --prune --depth=1 origin master
git checkout origin/master -- .github/deploy.sh util/gh-pages/ util/*.py
- name: Deploy
run: |
eval "$(ssh-agent -s)"
18 changes: 11 additions & 7 deletions clippy_lints/src/cargo_common_metadata.rs
Original file line number Diff line number Diff line change
@@ -2,9 +2,9 @@
use std::path::PathBuf;

use crate::utils::span_lint;
use rustc_ast::ast::Crate;
use rustc_lint::{EarlyContext, EarlyLintPass};
use crate::utils::{run_lints, span_lint};
use rustc_hir::{hir_id::CRATE_HIR_ID, Crate};
use rustc_lint::{LateContext, LateLintPass};
use rustc_session::{declare_lint_pass, declare_tool_lint};
use rustc_span::source_map::DUMMY_SP;

@@ -35,11 +35,11 @@ declare_clippy_lint! {
"common metadata is defined in `Cargo.toml`"
}

fn warning(cx: &EarlyContext<'_>, message: &str) {
fn warning(cx: &LateContext<'_, '_>, message: &str) {
span_lint(cx, CARGO_COMMON_METADATA, DUMMY_SP, message);
}

fn missing_warning(cx: &EarlyContext<'_>, package: &cargo_metadata::Package, field: &str) {
fn missing_warning(cx: &LateContext<'_, '_>, package: &cargo_metadata::Package, field: &str) {
let message = format!("package `{}` is missing `{}` metadata", package.name, field);
warning(cx, &message);
}
@@ -59,8 +59,12 @@ fn is_empty_vec(value: &[String]) -> bool {

declare_lint_pass!(CargoCommonMetadata => [CARGO_COMMON_METADATA]);

impl EarlyLintPass for CargoCommonMetadata {
fn check_crate(&mut self, cx: &EarlyContext<'_>, _: &Crate) {
impl LateLintPass<'_, '_> for CargoCommonMetadata {
fn check_crate(&mut self, cx: &LateContext<'_, '_>, _: &Crate<'_>) {
if !run_lints(cx, &[CARGO_COMMON_METADATA], CRATE_HIR_ID) {
return;
}

let metadata = if let Ok(metadata) = cargo_metadata::MetadataCommand::new().no_deps().exec() {
metadata
} else {
9 changes: 4 additions & 5 deletions clippy_lints/src/lib.rs
Original file line number Diff line number Diff line change
@@ -1024,9 +1024,9 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
store.register_early_pass(|| box precedence::Precedence);
store.register_early_pass(|| box needless_continue::NeedlessContinue);
store.register_early_pass(|| box redundant_static_lifetimes::RedundantStaticLifetimes);
store.register_early_pass(|| box cargo_common_metadata::CargoCommonMetadata);
store.register_early_pass(|| box multiple_crate_versions::MultipleCrateVersions);
store.register_early_pass(|| box wildcard_dependencies::WildcardDependencies);
store.register_late_pass(|| box cargo_common_metadata::CargoCommonMetadata);
store.register_late_pass(|| box multiple_crate_versions::MultipleCrateVersions);
store.register_late_pass(|| box wildcard_dependencies::WildcardDependencies);
store.register_early_pass(|| box literal_representation::LiteralDigitGrouping);
let literal_representation_threshold = conf.literal_representation_threshold;
store.register_early_pass(move || box literal_representation::DecimalLiteralRepresentation::new(literal_representation_threshold));
@@ -1134,6 +1134,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
LintId::of(&loops::EXPLICIT_INTO_ITER_LOOP),
LintId::of(&loops::EXPLICIT_ITER_LOOP),
LintId::of(&macro_use::MACRO_USE_IMPORTS),
LintId::of(&matches::MATCH_BOOL),
LintId::of(&matches::SINGLE_MATCH_ELSE),
LintId::of(&methods::FILTER_MAP),
LintId::of(&methods::FILTER_MAP_NEXT),
@@ -1279,7 +1280,6 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
LintId::of(&map_unit_fn::RESULT_MAP_UNIT_FN),
LintId::of(&matches::INFALLIBLE_DESTRUCTURING_MATCH),
LintId::of(&matches::MATCH_AS_REF),
LintId::of(&matches::MATCH_BOOL),
LintId::of(&matches::MATCH_OVERLAPPING_ARM),
LintId::of(&matches::MATCH_REF_PATS),
LintId::of(&matches::MATCH_SINGLE_BINDING),
@@ -1470,7 +1470,6 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
LintId::of(&main_recursion::MAIN_RECURSION),
LintId::of(&map_clone::MAP_CLONE),
LintId::of(&matches::INFALLIBLE_DESTRUCTURING_MATCH),
LintId::of(&matches::MATCH_BOOL),
LintId::of(&matches::MATCH_OVERLAPPING_ARM),
LintId::of(&matches::MATCH_REF_PATS),
LintId::of(&matches::MATCH_WILD_ERR_ARM),
2 changes: 1 addition & 1 deletion clippy_lints/src/matches.rs
Original file line number Diff line number Diff line change
@@ -138,7 +138,7 @@ declare_clippy_lint! {
/// }
/// ```
pub MATCH_BOOL,
style,
pedantic,
"a `match` on a boolean expression instead of an `if..else` block"
}

14 changes: 9 additions & 5 deletions clippy_lints/src/multiple_crate_versions.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
//! lint on multiple versions of a crate being used
use crate::utils::span_lint;
use rustc_ast::ast::Crate;
use rustc_lint::{EarlyContext, EarlyLintPass};
use crate::utils::{run_lints, span_lint};
use rustc_hir::{Crate, CRATE_HIR_ID};
use rustc_lint::{LateContext, LateLintPass};
use rustc_session::{declare_lint_pass, declare_tool_lint};
use rustc_span::source_map::DUMMY_SP;

@@ -33,8 +33,12 @@ declare_clippy_lint! {

declare_lint_pass!(MultipleCrateVersions => [MULTIPLE_CRATE_VERSIONS]);

impl EarlyLintPass for MultipleCrateVersions {
fn check_crate(&mut self, cx: &EarlyContext<'_>, _: &Crate) {
impl LateLintPass<'_, '_> for MultipleCrateVersions {
fn check_crate(&mut self, cx: &LateContext<'_, '_>, _: &Crate<'_>) {
if !run_lints(cx, &[MULTIPLE_CRATE_VERSIONS], CRATE_HIR_ID) {
return;
}

let metadata = if let Ok(metadata) = cargo_metadata::MetadataCommand::new().exec() {
metadata
} else {
32 changes: 25 additions & 7 deletions clippy_lints/src/utils/internal_lints.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use crate::utils::SpanlessEq;
use crate::utils::{
is_expn_of, match_def_path, match_qpath, match_type, method_calls, paths, snippet, span_lint, span_lint_and_help,
span_lint_and_sugg, walk_ptrs_ty,
is_expn_of, match_def_path, match_qpath, match_type, method_calls, paths, run_lints, snippet, span_lint,
span_lint_and_help, span_lint_and_sugg, walk_ptrs_ty,
};
use if_chain::if_chain;
use rustc_ast::ast::{Crate as AstCrate, ItemKind, LitKind, Name, NodeId};
@@ -10,7 +10,8 @@ use rustc_data_structures::fx::{FxHashMap, FxHashSet};
use rustc_errors::Applicability;
use rustc_hir as hir;
use rustc_hir::def::{DefKind, Res};
use rustc_hir::intravisit::{walk_expr, NestedVisitorMap, Visitor};
use rustc_hir::hir_id::CRATE_HIR_ID;
use rustc_hir::intravisit::{NestedVisitorMap, Visitor};
use rustc_hir::{Crate, Expr, ExprKind, HirId, Item, MutTy, Mutability, Path, StmtKind, Ty, TyKind};
use rustc_lint::{EarlyContext, EarlyLintPass, LateContext, LateLintPass};
use rustc_middle::hir::map::Map;
@@ -252,6 +253,10 @@ impl_lint_pass!(LintWithoutLintPass => [DEFAULT_LINT, LINT_WITHOUT_LINT_PASS]);

impl<'a, 'tcx> LateLintPass<'a, 'tcx> for LintWithoutLintPass {
fn check_item(&mut self, cx: &LateContext<'a, 'tcx>, item: &'tcx Item<'_>) {
if !run_lints(cx, &[DEFAULT_LINT], item.hir_id) {
return;
}

if let hir::ItemKind::Static(ref ty, Mutability::Not, body_id) = item.kind {
if is_lint_ref_type(cx, ty) {
let expr = &cx.tcx.hir().body(body_id).value;
@@ -306,6 +311,10 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for LintWithoutLintPass {
}

fn check_crate_post(&mut self, cx: &LateContext<'a, 'tcx>, _: &'tcx Crate<'_>) {
if !run_lints(cx, &[LINT_WITHOUT_LINT_PASS], CRATE_HIR_ID) {
return;
}

for (lint_name, &lint_span) in &self.declared_lints {
// When using the `declare_tool_lint!` macro, the original `lint_span`'s
// file points to "<rustc macros>".
@@ -355,15 +364,12 @@ struct LintCollector<'a, 'tcx> {
impl<'a, 'tcx> Visitor<'tcx> for LintCollector<'a, 'tcx> {
type Map = Map<'tcx>;

fn visit_expr(&mut self, expr: &'tcx Expr<'_>) {
walk_expr(self, expr);
}

fn visit_path(&mut self, path: &'tcx Path<'_>, _: HirId) {
if path.segments.len() == 1 {
self.output.insert(path.segments[0].ident.name);
}
}

fn nested_visit_map(&mut self) -> NestedVisitorMap<Self::Map> {
NestedVisitorMap::All(self.cx.tcx.hir())
}
@@ -391,6 +397,10 @@ impl_lint_pass!(CompilerLintFunctions => [COMPILER_LINT_FUNCTIONS]);

impl<'a, 'tcx> LateLintPass<'a, 'tcx> for CompilerLintFunctions {
fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr<'_>) {
if !run_lints(cx, &[COMPILER_LINT_FUNCTIONS], expr.hir_id) {
return;
}

if_chain! {
if let ExprKind::MethodCall(ref path, _, ref args) = expr.kind;
let fn_name = path.ident;
@@ -416,6 +426,10 @@ declare_lint_pass!(OuterExpnDataPass => [OUTER_EXPN_EXPN_DATA]);

impl<'a, 'tcx> LateLintPass<'a, 'tcx> for OuterExpnDataPass {
fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, expr: &'tcx hir::Expr<'_>) {
if !run_lints(cx, &[OUTER_EXPN_EXPN_DATA], expr.hir_id) {
return;
}

let (method_names, arg_lists, spans) = method_calls(expr, 2);
let method_names: Vec<SymbolStr> = method_names.iter().map(|s| s.as_str()).collect();
let method_names: Vec<&str> = method_names.iter().map(|s| &**s).collect();
@@ -462,6 +476,10 @@ declare_lint_pass!(CollapsibleCalls => [COLLAPSIBLE_SPAN_LINT_CALLS]);

impl<'a, 'tcx> LateLintPass<'a, 'tcx> for CollapsibleCalls {
fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, expr: &'tcx hir::Expr<'_>) {
if !run_lints(cx, &[COLLAPSIBLE_SPAN_LINT_CALLS], expr.hir_id) {
return;
}

if_chain! {
if let ExprKind::Call(ref func, ref and_then_args) = expr.kind;
if let ExprKind::Path(ref path) = func.kind;
9 changes: 9 additions & 0 deletions clippy_lints/src/utils/mod.rs
Original file line number Diff line number Diff line change
@@ -1399,6 +1399,15 @@ pub fn fn_has_unsatisfiable_preds(cx: &LateContext<'_, '_>, did: DefId) -> bool
)
}

pub fn run_lints(cx: &LateContext<'_, '_>, lints: &[&'static Lint], id: HirId) -> bool {
lints.iter().any(|lint| {
matches!(
cx.tcx.lint_level_at_node(lint, id),
(Level::Forbid | Level::Deny | Level::Warn, _)
)
})
}

#[cfg(test)]
mod test {
use super::{trim_multiline, without_block_comments};
14 changes: 9 additions & 5 deletions clippy_lints/src/wildcard_dependencies.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use crate::utils::span_lint;
use rustc_ast::ast::Crate;
use rustc_lint::{EarlyContext, EarlyLintPass};
use crate::utils::{run_lints, span_lint};
use rustc_hir::{hir_id::CRATE_HIR_ID, Crate};
use rustc_lint::{LateContext, LateLintPass};
use rustc_session::{declare_lint_pass, declare_tool_lint};
use rustc_span::source_map::DUMMY_SP;

@@ -28,8 +28,12 @@ declare_clippy_lint! {

declare_lint_pass!(WildcardDependencies => [WILDCARD_DEPENDENCIES]);

impl EarlyLintPass for WildcardDependencies {
fn check_crate(&mut self, cx: &EarlyContext<'_>, _: &Crate) {
impl LateLintPass<'_, '_> for WildcardDependencies {
fn check_crate(&mut self, cx: &LateContext<'_, '_>, _: &Crate<'_>) {
if !run_lints(cx, &[WILDCARD_DEPENDENCIES], CRATE_HIR_ID) {
return;
}

let metadata = if let Ok(metadata) = cargo_metadata::MetadataCommand::new().no_deps().exec() {
metadata
} else {
12 changes: 11 additions & 1 deletion doc/release.md
Original file line number Diff line number Diff line change
@@ -63,6 +63,16 @@ to the beta Rust release. The remerge is then necessary, to make sure that the
Clippy commit, that was used by the now stable Rust release, persists in the
tree of the Clippy repository.

To find out if this step is necessary run

```bash
# Assumes that the local master branch is up-to-date
$ git fetch upstream
$ git branch master --contains upstream/beta
```

If this command outputs `master`, this step is **not** necessary.

```bash
# Assuming `HEAD` is the current `master` branch of rust-lang/rust-clippy
$ git checkout -b backport_remerge
@@ -97,5 +107,5 @@ be updated.
# Assuming the current directory corresponds to the Clippy repository
$ git checkout beta
$ git rebase $BETA_SHA
$ git push upstream beta [-f] # This requires a force push, if a remerge was done
$ git push upstream beta
```
2 changes: 1 addition & 1 deletion src/lintlist/mod.rs
Original file line number Diff line number Diff line change
@@ -1139,7 +1139,7 @@ pub static ref ALL_LINTS: Vec<Lint> = vec![
},
Lint {
name: "match_bool",
group: "style",
group: "pedantic",
desc: "a `match` on a boolean expression instead of an `if..else` block",
deprecation: None,
module: "matches",
3 changes: 1 addition & 2 deletions tests/ui/implicit_return.fixed
Original file line number Diff line number Diff line change
@@ -21,7 +21,6 @@ fn test_if_block() -> bool {
}
}

#[allow(clippy::match_bool)]
#[rustfmt::skip]
fn test_match(x: bool) -> bool {
match x {
@@ -30,7 +29,7 @@ fn test_match(x: bool) -> bool {
}
}

#[allow(clippy::match_bool, clippy::needless_return)]
#[allow(clippy::needless_return)]
fn test_match_with_unreachable(x: bool) -> bool {
match x {
true => return false,
3 changes: 1 addition & 2 deletions tests/ui/implicit_return.rs
Original file line number Diff line number Diff line change
@@ -21,7 +21,6 @@ fn test_if_block() -> bool {
}
}

#[allow(clippy::match_bool)]
#[rustfmt::skip]
fn test_match(x: bool) -> bool {
match x {
@@ -30,7 +29,7 @@ fn test_match(x: bool) -> bool {
}
}

#[allow(clippy::match_bool, clippy::needless_return)]
#[allow(clippy::needless_return)]
fn test_match_with_unreachable(x: bool) -> bool {
match x {
true => return false,
16 changes: 8 additions & 8 deletions tests/ui/implicit_return.stderr
Original file line number Diff line number Diff line change
@@ -19,49 +19,49 @@ LL | false
| ^^^^^ help: add `return` as shown: `return false`

error: missing `return` statement
--> $DIR/implicit_return.rs:28:17
--> $DIR/implicit_return.rs:27:17
|
LL | true => false,
| ^^^^^ help: add `return` as shown: `return false`

error: missing `return` statement
--> $DIR/implicit_return.rs:29:20
--> $DIR/implicit_return.rs:28:20
|
LL | false => { true },
| ^^^^ help: add `return` as shown: `return true`

error: missing `return` statement
--> $DIR/implicit_return.rs:44:9
--> $DIR/implicit_return.rs:43:9
|
LL | break true;
| ^^^^^^^^^^ help: change `break` to `return` as shown: `return true`

error: missing `return` statement
--> $DIR/implicit_return.rs:52:13
--> $DIR/implicit_return.rs:51:13
|
LL | break true;
| ^^^^^^^^^^ help: change `break` to `return` as shown: `return true`

error: missing `return` statement
--> $DIR/implicit_return.rs:61:13
--> $DIR/implicit_return.rs:60:13
|
LL | break true;
| ^^^^^^^^^^ help: change `break` to `return` as shown: `return true`

error: missing `return` statement
--> $DIR/implicit_return.rs:79:18
--> $DIR/implicit_return.rs:78:18
|
LL | let _ = || { true };
| ^^^^ help: add `return` as shown: `return true`

error: missing `return` statement
--> $DIR/implicit_return.rs:80:16
--> $DIR/implicit_return.rs:79:16
|
LL | let _ = || true;
| ^^^^ help: add `return` as shown: `return true`

error: missing `return` statement
--> $DIR/implicit_return.rs:88:5
--> $DIR/implicit_return.rs:87:5
|
LL | format!("test {}", "test")
| ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: add `return` as shown: `return format!("test {}", "test")`
2 changes: 2 additions & 0 deletions tests/ui/match_bool.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
#![deny(clippy::match_bool)]

fn match_bool() {
let test: bool = true;

22 changes: 13 additions & 9 deletions tests/ui/match_bool.stderr
Original file line number Diff line number Diff line change
@@ -1,24 +1,28 @@
error: this boolean expression can be simplified
--> $DIR/match_bool.rs:29:11
--> $DIR/match_bool.rs:31:11
|
LL | match test && test {
| ^^^^^^^^^^^^ help: try: `test`
|
= note: `-D clippy::nonminimal-bool` implied by `-D warnings`

error: you seem to be trying to match on a boolean expression
--> $DIR/match_bool.rs:4:5
--> $DIR/match_bool.rs:6:5
|
LL | / match test {
LL | | true => 0,
LL | | false => 42,
LL | | };
| |_____^ help: consider using an `if`/`else` expression: `if test { 0 } else { 42 }`
|
= note: `-D clippy::match-bool` implied by `-D warnings`
note: the lint level is defined here
--> $DIR/match_bool.rs:1:9
|
LL | #![deny(clippy::match_bool)]
| ^^^^^^^^^^^^^^^^^^

error: you seem to be trying to match on a boolean expression
--> $DIR/match_bool.rs:10:5
--> $DIR/match_bool.rs:12:5
|
LL | / match option == 1 {
LL | | true => 1,
@@ -27,7 +31,7 @@ LL | | };
| |_____^ help: consider using an `if`/`else` expression: `if option == 1 { 1 } else { 0 }`

error: you seem to be trying to match on a boolean expression
--> $DIR/match_bool.rs:15:5
--> $DIR/match_bool.rs:17:5
|
LL | / match test {
LL | | true => (),
@@ -45,7 +49,7 @@ LL | };
|

error: you seem to be trying to match on a boolean expression
--> $DIR/match_bool.rs:22:5
--> $DIR/match_bool.rs:24:5
|
LL | / match test {
LL | | false => {
@@ -63,7 +67,7 @@ LL | };
|

error: you seem to be trying to match on a boolean expression
--> $DIR/match_bool.rs:29:5
--> $DIR/match_bool.rs:31:5
|
LL | / match test && test {
LL | | false => {
@@ -81,15 +85,15 @@ LL | };
|

error: equal expressions as operands to `&&`
--> $DIR/match_bool.rs:29:11
--> $DIR/match_bool.rs:31:11
|
LL | match test && test {
| ^^^^^^^^^^^^
|
= note: `#[deny(clippy::eq_op)]` on by default

error: you seem to be trying to match on a boolean expression
--> $DIR/match_bool.rs:36:5
--> $DIR/match_bool.rs:38:5
|
LL | / match test {
LL | | false => {
2 changes: 1 addition & 1 deletion tests/ui/needless_return.fixed
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
// run-rustfix

#![allow(unused, clippy::needless_bool, clippy::match_bool)]
#![allow(unused, clippy::needless_bool)]
#![allow(clippy::if_same_then_else, clippy::single_match)]
#![warn(clippy::needless_return)]

2 changes: 1 addition & 1 deletion tests/ui/needless_return.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
// run-rustfix

#![allow(unused, clippy::needless_bool, clippy::match_bool)]
#![allow(unused, clippy::needless_bool)]
#![allow(clippy::if_same_then_else, clippy::single_match)]
#![warn(clippy::needless_return)]

11 changes: 11 additions & 0 deletions tests/ui/new_ret_no_self.rs
Original file line number Diff line number Diff line change
@@ -199,3 +199,14 @@ impl NestedReturnerOk3 {
unimplemented!();
}
}

struct WithLifetime<'a> {
cat: &'a str,
}

impl<'a> WithLifetime<'a> {
// should not trigger the lint, because the lifetimes are different
pub fn new<'b: 'a>(s: &'b str) -> WithLifetime<'b> {
unimplemented!();
}
}