Skip to content

Commit

Permalink
Auto merge of #5741 - flip1995:rollup-8chbwhy, r=flip1995
Browse files Browse the repository at this point in the history
Rollup of 9 pull requests

Successful merges:

 - #5178 (clippy-driver: pass all args to rustc if --rustc is present)
 - #5705 (Downgrade unnested_or_patterns to pedantic)
 - #5709 (Fix ICE in consts::binop)
 - #5710 (typo)
 - #5712 (Remove `bar` from blacklisted names)
 - #5713 (Use lints in Clippy that are enabled in rustc bootstrap)
 - #5716 (Fix typo in wildcard_imports)
 - #5724 (redundant_pattern_matching: avoid non-`const fn` calls in const contexts)
 - #5726 (Fix typo)

Failed merges:

r? @ghost

changelog: rollup
  • Loading branch information
bors committed Jun 23, 2020
2 parents 4cc2fa9 + e4cbd1d commit b452ad3
Show file tree
Hide file tree
Showing 25 changed files with 454 additions and 125 deletions.
12 changes: 12 additions & 0 deletions .github/driver.sh
Original file line number Diff line number Diff line change
Expand Up @@ -26,4 +26,16 @@ unset CARGO_MANIFEST_DIR
sed -e "s,tests/ui,\$DIR," -e "/= help/d" cstring.stderr > normalized.stderr
diff normalized.stderr tests/ui/cstring.stderr


# make sure "clippy-driver --rustc --arg" and "rustc --arg" behave the same
SYSROOT=`rustc --print sysroot`
diff <(LD_LIBRARY_PATH=${SYSROOT}/lib ./target/debug/clippy-driver --rustc --version --verbose) <(rustc --version --verbose)


echo "fn main() {}" > target/driver_test.rs
# we can't run 2 rustcs on the same file at the same time
CLIPPY=`LD_LIBRARY_PATH=${SYSROOT}/lib ./target/debug/clippy-driver ./target/driver_test.rs --rustc`
RUSTC=`rustc ./target/driver_test.rs`
diff <($CLIPPY) <($RUSTC)

# TODO: CLIPPY_CONF_DIR / CARGO_MANIFEST_DIR
2 changes: 1 addition & 1 deletion clippy_lints/src/consts.rs
Original file line number Diff line number Diff line change
Expand Up @@ -396,7 +396,7 @@ impl<'c, 'cc> ConstEvalLateContext<'c, 'cc> {
let l = self.expr(left)?;
let r = self.expr(right);
match (l, r) {
(Constant::Int(l), Some(Constant::Int(r))) => match self.tables.expr_ty(left).kind {
(Constant::Int(l), Some(Constant::Int(r))) => match self.tables.expr_ty_opt(left)?.kind {
ty::Int(ity) => {
let l = sext(self.lcx.tcx, l, ity);
let r = sext(self.lcx.tcx, r, ity);
Expand Down
2 changes: 1 addition & 1 deletion clippy_lints/src/let_underscore.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ declare_clippy_lint! {
/// **What it does:** Checks for `let _ = sync_lock`
///
/// **Why is this bad?** This statement immediately drops the lock instead of
/// extending it's lifetime to the end of the scope, which is often not intended.
/// extending its lifetime to the end of the scope, which is often not intended.
/// To extend lock lifetime to the end of the scope, use an underscore-prefixed
/// name instead (i.e. _lock). If you want to explicitly drop the lock,
/// `std::mem::drop` conveys your intention better and is less error-prone.
Expand Down
20 changes: 11 additions & 9 deletions clippy_lints/src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,19 +1,22 @@
// error-pattern:cargo-clippy

#![feature(bindings_after_at)]
#![feature(box_syntax)]
#![feature(box_patterns)]
#![feature(box_syntax)]
#![feature(concat_idents)]
#![feature(crate_visibility_modifier)]
#![feature(drain_filter)]
#![feature(or_patterns)]
#![feature(rustc_private)]
#![feature(stmt_expr_attributes)]
#![allow(clippy::missing_docs_in_private_items, clippy::must_use_candidate)]
#![recursion_limit = "512"]
#![warn(rust_2018_idioms, trivial_casts, trivial_numeric_casts)]
#![deny(rustc::internal)]
#![cfg_attr(feature = "deny-warnings", deny(warnings))]
#![feature(crate_visibility_modifier)]
#![feature(concat_idents)]
#![feature(drain_filter)]
#![allow(clippy::missing_docs_in_private_items, clippy::must_use_candidate)]
#![warn(trivial_casts, trivial_numeric_casts)]
// warn on lints, that are included in `rust-lang/rust`s bootstrap
#![warn(rust_2018_idioms, unused_lifetimes)]
// warn on rustc internal lints
#![deny(rustc::internal)]

// FIXME: switch to something more ergonomic here, once available.
// (Currently there is no way to opt into sysroot crates without `extern crate`.)
Expand Down Expand Up @@ -1187,6 +1190,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
LintId::of(&types::OPTION_OPTION),
LintId::of(&unicode::NON_ASCII_LITERAL),
LintId::of(&unicode::UNICODE_NOT_NFC),
LintId::of(&unnested_or_patterns::UNNESTED_OR_PATTERNS),
LintId::of(&unused_self::UNUSED_SELF),
LintId::of(&wildcard_imports::ENUM_GLOB_USE),
LintId::of(&wildcard_imports::WILDCARD_IMPORTS),
Expand Down Expand Up @@ -1440,7 +1444,6 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
LintId::of(&unnamed_address::FN_ADDRESS_COMPARISONS),
LintId::of(&unnamed_address::VTABLE_ADDRESS_COMPARISONS),
LintId::of(&unnecessary_sort_by::UNNECESSARY_SORT_BY),
LintId::of(&unnested_or_patterns::UNNESTED_OR_PATTERNS),
LintId::of(&unsafe_removed_from_name::UNSAFE_REMOVED_FROM_NAME),
LintId::of(&unused_io_amount::UNUSED_IO_AMOUNT),
LintId::of(&unwrap::PANICKING_UNWRAP),
Expand Down Expand Up @@ -1624,7 +1627,6 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
LintId::of(&types::UNNECESSARY_CAST),
LintId::of(&types::VEC_BOX),
LintId::of(&unnecessary_sort_by::UNNECESSARY_SORT_BY),
LintId::of(&unnested_or_patterns::UNNESTED_OR_PATTERNS),
LintId::of(&unwrap::UNNECESSARY_UNWRAP),
LintId::of(&useless_conversion::USELESS_CONVERSION),
LintId::of(&zero_div_zero::ZERO_DIVIDED_BY_ZERO),
Expand Down
6 changes: 3 additions & 3 deletions clippy_lints/src/loops.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1497,7 +1497,7 @@ struct MutatePairDelegate<'a, 'tcx> {
span_high: Option<Span>,
}

impl<'a, 'tcx> Delegate<'tcx> for MutatePairDelegate<'a, 'tcx> {
impl<'tcx> Delegate<'tcx> for MutatePairDelegate<'_, 'tcx> {
fn consume(&mut self, _: &PlaceWithHirId<'tcx>, _: ConsumeMode) {}

fn borrow(&mut self, cmt: &PlaceWithHirId<'tcx>, bk: ty::BorrowKind) {
Expand Down Expand Up @@ -1525,7 +1525,7 @@ impl<'a, 'tcx> Delegate<'tcx> for MutatePairDelegate<'a, 'tcx> {
}
}

impl<'a, 'tcx> MutatePairDelegate<'a, 'tcx> {
impl MutatePairDelegate<'_, '_> {
fn mutation_span(&self) -> (Option<Span>, Option<Span>) {
(self.span_low, self.span_high)
}
Expand Down Expand Up @@ -2292,7 +2292,7 @@ struct HasBreakOrReturnVisitor {
has_break_or_return: bool,
}

impl<'a, 'tcx> Visitor<'tcx> for HasBreakOrReturnVisitor {
impl<'tcx> Visitor<'tcx> for HasBreakOrReturnVisitor {
type Map = Map<'tcx>;

fn visit_expr(&mut self, expr: &'tcx Expr<'_>) {
Expand Down
2 changes: 1 addition & 1 deletion clippy_lints/src/new_without_default.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ declare_clippy_lint! {
/// }
/// ```
///
/// To fix the lint, and a `Default` implementation that delegates to `new`:
/// To fix the lint, add a `Default` implementation that delegates to `new`:
///
/// ```ignore
/// struct Foo(Bar);
Expand Down
80 changes: 62 additions & 18 deletions clippy_lints/src/redundant_pattern_matching.rs
Original file line number Diff line number Diff line change
@@ -1,10 +1,13 @@
use crate::utils::{match_qpath, match_trait_method, paths, snippet, span_lint_and_then};
use crate::utils::{in_constant, match_qpath, match_trait_method, paths, snippet, span_lint_and_then};
use if_chain::if_chain;
use rustc_ast::ast::LitKind;
use rustc_errors::Applicability;
use rustc_hir::{Arm, Expr, ExprKind, MatchSource, PatKind, QPath};
use rustc_hir::{Arm, Expr, ExprKind, HirId, MatchSource, PatKind, QPath};
use rustc_lint::{LateContext, LateLintPass};
use rustc_middle::ty;
use rustc_mir::const_eval::is_const_fn;
use rustc_session::{declare_lint_pass, declare_tool_lint};
use rustc_span::source_map::Symbol;

declare_clippy_lint! {
/// **What it does:** Lint for redundant pattern matching over `Result` or
Expand Down Expand Up @@ -64,26 +67,37 @@ fn find_sugg_for_if_let<'a, 'tcx>(
arms: &[Arm<'_>],
keyword: &'static str,
) {
fn find_suggestion(cx: &LateContext<'_, '_>, hir_id: HirId, path: &QPath<'_>) -> Option<&'static str> {
if match_qpath(path, &paths::RESULT_OK) && can_suggest(cx, hir_id, sym!(result_type), "is_ok") {
return Some("is_ok()");
}
if match_qpath(path, &paths::RESULT_ERR) && can_suggest(cx, hir_id, sym!(result_type), "is_err") {
return Some("is_err()");
}
if match_qpath(path, &paths::OPTION_SOME) && can_suggest(cx, hir_id, sym!(option_type), "is_some") {
return Some("is_some()");
}
if match_qpath(path, &paths::OPTION_NONE) && can_suggest(cx, hir_id, sym!(option_type), "is_none") {
return Some("is_none()");
}
None
}

let hir_id = expr.hir_id;
let good_method = match arms[0].pat.kind {
PatKind::TupleStruct(ref path, ref patterns, _) if patterns.len() == 1 => {
if let PatKind::Wild = patterns[0].kind {
if match_qpath(path, &paths::RESULT_OK) {
"is_ok()"
} else if match_qpath(path, &paths::RESULT_ERR) {
"is_err()"
} else if match_qpath(path, &paths::OPTION_SOME) {
"is_some()"
} else {
return;
}
find_suggestion(cx, hir_id, path)
} else {
return;
None
}
},

PatKind::Path(ref path) if match_qpath(path, &paths::OPTION_NONE) => "is_none()",

_ => return,
PatKind::Path(ref path) => find_suggestion(cx, hir_id, path),
_ => None,
};
let good_method = match good_method {
Some(method) => method,
None => return,
};

// check that `while_let_on_iterator` lint does not trigger
Expand Down Expand Up @@ -128,6 +142,7 @@ fn find_sugg_for_match<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr<'_
if arms.len() == 2 {
let node_pair = (&arms[0].pat.kind, &arms[1].pat.kind);

let hir_id = expr.hir_id;
let found_good_method = match node_pair {
(
PatKind::TupleStruct(ref path_left, ref patterns_left, _),
Expand All @@ -142,6 +157,8 @@ fn find_sugg_for_match<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr<'_
&paths::RESULT_ERR,
"is_ok()",
"is_err()",
|| can_suggest(cx, hir_id, sym!(result_type), "is_ok"),
|| can_suggest(cx, hir_id, sym!(result_type), "is_err"),
)
} else {
None
Expand All @@ -160,6 +177,8 @@ fn find_sugg_for_match<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr<'_
&paths::OPTION_NONE,
"is_some()",
"is_none()",
|| can_suggest(cx, hir_id, sym!(option_type), "is_some"),
|| can_suggest(cx, hir_id, sym!(option_type), "is_none"),
)
} else {
None
Expand Down Expand Up @@ -188,6 +207,7 @@ fn find_sugg_for_match<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr<'_
}
}

#[allow(clippy::too_many_arguments)]
fn find_good_method_for_match<'a>(
arms: &[Arm<'_>],
path_left: &QPath<'_>,
Expand All @@ -196,6 +216,8 @@ fn find_good_method_for_match<'a>(
expected_right: &[&str],
should_be_left: &'a str,
should_be_right: &'a str,
can_suggest_left: impl Fn() -> bool,
can_suggest_right: impl Fn() -> bool,
) -> Option<&'a str> {
let body_node_pair = if match_qpath(path_left, expected_left) && match_qpath(path_right, expected_right) {
(&(*arms[0].body).kind, &(*arms[1].body).kind)
Expand All @@ -207,10 +229,32 @@ fn find_good_method_for_match<'a>(

match body_node_pair {
(ExprKind::Lit(ref lit_left), ExprKind::Lit(ref lit_right)) => match (&lit_left.node, &lit_right.node) {
(LitKind::Bool(true), LitKind::Bool(false)) => Some(should_be_left),
(LitKind::Bool(false), LitKind::Bool(true)) => Some(should_be_right),
(LitKind::Bool(true), LitKind::Bool(false)) if can_suggest_left() => Some(should_be_left),
(LitKind::Bool(false), LitKind::Bool(true)) if can_suggest_right() => Some(should_be_right),
_ => None,
},
_ => None,
}
}

fn can_suggest(cx: &LateContext<'_, '_>, hir_id: HirId, diag_item: Symbol, name: &str) -> bool {
if !in_constant(cx, hir_id) {
return true;
}

// Avoid suggesting calls to non-`const fn`s in const contexts, see #5697.
cx.tcx
.get_diagnostic_item(diag_item)
.and_then(|def_id| {
cx.tcx.inherent_impls(def_id).iter().find_map(|imp| {
cx.tcx
.associated_items(*imp)
.in_definition_order()
.find_map(|item| match item.kind {
ty::AssocKind::Fn if item.ident.name.as_str() == name => Some(item.def_id),
_ => None,
})
})
})
.map_or(false, |def_id| is_const_fn(cx.tcx, def_id))
}
2 changes: 1 addition & 1 deletion clippy_lints/src/suspicious_trait_impl.rs
Original file line number Diff line number Diff line change
Expand Up @@ -184,7 +184,7 @@ struct BinaryExprVisitor {
in_binary_expr: bool,
}

impl<'a, 'tcx> Visitor<'tcx> for BinaryExprVisitor {
impl<'tcx> Visitor<'tcx> for BinaryExprVisitor {
type Map = Map<'tcx>;

fn visit_expr(&mut self, expr: &'tcx hir::Expr<'_>) {
Expand Down
2 changes: 1 addition & 1 deletion clippy_lints/src/trivially_copy_pass_by_ref.rs
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ pub struct TriviallyCopyPassByRef {
limit: u64,
}

impl<'a, 'tcx> TriviallyCopyPassByRef {
impl<'tcx> TriviallyCopyPassByRef {
pub fn new(limit: Option<u64>, target: &SessionConfig) -> Self {
let limit = limit.unwrap_or_else(|| {
let bit_width = u64::from(target.ptr_width);
Expand Down
2 changes: 1 addition & 1 deletion clippy_lints/src/unnested_or_patterns.rs
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ declare_clippy_lint! {
/// }
/// ```
pub UNNESTED_OR_PATTERNS,
complexity,
pedantic,
"unnested or-patterns, e.g., `Foo(Bar) | Foo(Baz) instead of `Foo(Bar | Baz)`"
}

Expand Down
4 changes: 2 additions & 2 deletions clippy_lints/src/utils/conf.rs
Original file line number Diff line number Diff line change
Expand Up @@ -106,8 +106,8 @@ macro_rules! define_Conf {

pub use self::helpers::Conf;
define_Conf! {
/// Lint: BLACKLISTED_NAME. The list of blacklisted names to lint about
(blacklisted_names, "blacklisted_names": Vec<String>, ["foo", "bar", "baz", "quux"].iter().map(ToString::to_string).collect()),
/// Lint: BLACKLISTED_NAME. The list of blacklisted names to lint about. NB: `bar` is not here since it has legitimate uses
(blacklisted_names, "blacklisted_names": Vec<String>, ["foo", "baz", "quux"].iter().map(ToString::to_string).collect()),
/// Lint: COGNITIVE_COMPLEXITY. The maximum cognitive complexity a function can have
(cognitive_complexity_threshold, "cognitive_complexity_threshold": u64, 25),
/// DEPRECATED LINT: CYCLOMATIC_COMPLEXITY. Use the Cognitive Complexity lint instead.
Expand Down
4 changes: 2 additions & 2 deletions clippy_lints/src/utils/sugg.rs
Original file line number Diff line number Diff line change
Expand Up @@ -509,7 +509,7 @@ fn indentation<T: LintContext>(cx: &T, span: Span) -> Option<String> {
}

/// Convenience extension trait for `DiagnosticBuilder`.
pub trait DiagnosticBuilderExt<'a, T: LintContext> {
pub trait DiagnosticBuilderExt<T: LintContext> {
/// Suggests to add an attribute to an item.
///
/// Correctly handles indentation of the attribute and item.
Expand Down Expand Up @@ -556,7 +556,7 @@ pub trait DiagnosticBuilderExt<'a, T: LintContext> {
fn suggest_remove_item(&mut self, cx: &T, item: Span, msg: &str, applicability: Applicability);
}

impl<'a, 'b, 'c, T: LintContext> DiagnosticBuilderExt<'c, T> for rustc_errors::DiagnosticBuilder<'b> {
impl<T: LintContext> DiagnosticBuilderExt<T> for rustc_errors::DiagnosticBuilder<'_> {
fn suggest_item_with_attr<D: Display + ?Sized>(
&mut self,
cx: &T,
Expand Down
2 changes: 1 addition & 1 deletion clippy_lints/src/wildcard_imports.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ declare_clippy_lint! {
declare_clippy_lint! {
/// **What it does:** Checks for wildcard imports `use _::*`.
///
/// **Why is this bad?** wildcard imports can polute the namespace. This is especially bad if
/// **Why is this bad?** wildcard imports can pollute the namespace. This is especially bad if
/// you try to import something through a wildcard, that already has been imported by name from
/// a different source:
///
Expand Down
Loading

0 comments on commit b452ad3

Please sign in to comment.