diff --git a/src/tools/clippy/.github/ISSUE_TEMPLATE.md b/src/tools/clippy/.github/ISSUE_TEMPLATE.md deleted file mode 100644 index 15006a07b44f2..0000000000000 --- a/src/tools/clippy/.github/ISSUE_TEMPLATE.md +++ /dev/null @@ -1,8 +0,0 @@ - diff --git a/src/tools/clippy/.github/ISSUE_TEMPLATE/blank_issue.md b/src/tools/clippy/.github/ISSUE_TEMPLATE/blank_issue.md new file mode 100644 index 0000000000000..9aef3ebe637a1 --- /dev/null +++ b/src/tools/clippy/.github/ISSUE_TEMPLATE/blank_issue.md @@ -0,0 +1,4 @@ +--- +name: Blank Issue +about: Create a blank issue. +--- diff --git a/src/tools/clippy/.github/ISSUE_TEMPLATE/bug_report.md b/src/tools/clippy/.github/ISSUE_TEMPLATE/bug_report.md new file mode 100644 index 0000000000000..d8f0c44148cae --- /dev/null +++ b/src/tools/clippy/.github/ISSUE_TEMPLATE/bug_report.md @@ -0,0 +1,47 @@ +--- +name: Bug Report +about: Create a bug report for Clippy +labels: L-bug +--- + + +I tried this code: + +```rust + +``` + +I expected to see this happen: *explanation* + +Instead, this happened: *explanation* + +### Meta + +- `cargo clippy -V`: e.g. clippy 0.0.212 (f455e46 2020-06-20) +- `rustc -Vv`: + ``` + rustc 1.46.0-nightly (f455e46ea 2020-06-20) + binary: rustc + commit-hash: f455e46eae1a227d735091091144601b467e1565 + commit-date: 2020-06-20 + host: x86_64-unknown-linux-gnu + release: 1.46.0-nightly + LLVM version: 10.0 + ``` + + +
Backtrace +

+ + ``` + + ``` + +

+
diff --git a/src/tools/clippy/.github/ISSUE_TEMPLATE/config.yml b/src/tools/clippy/.github/ISSUE_TEMPLATE/config.yml new file mode 100644 index 0000000000000..bd7dc0ac95c1f --- /dev/null +++ b/src/tools/clippy/.github/ISSUE_TEMPLATE/config.yml @@ -0,0 +1,5 @@ +blank_issues_enabled: true +contact_links: + - name: Rust Programming Language Forum + url: https://users.rust-lang.org + about: Please ask and answer questions about Rust here. diff --git a/src/tools/clippy/.github/ISSUE_TEMPLATE/ice.md b/src/tools/clippy/.github/ISSUE_TEMPLATE/ice.md new file mode 100644 index 0000000000000..3abe76bf2c497 --- /dev/null +++ b/src/tools/clippy/.github/ISSUE_TEMPLATE/ice.md @@ -0,0 +1,53 @@ +--- +name: Internal Compiler Error +about: Create a report for an internal compiler error in Clippy. +labels: L-bug, L-crash +--- + + +### Code + +```rust + +``` + +### Meta + +- `cargo clippy -V`: e.g. clippy 0.0.212 (f455e46 2020-06-20) +- `rustc -Vv`: + ``` + rustc 1.46.0-nightly (f455e46ea 2020-06-20) + binary: rustc + commit-hash: f455e46eae1a227d735091091144601b467e1565 + commit-date: 2020-06-20 + host: x86_64-unknown-linux-gnu + release: 1.46.0-nightly + LLVM version: 10.0 + ``` + +### Error output + +``` + +``` + + +
Backtrace +

+ + ``` + + ``` + +

+
diff --git a/src/tools/clippy/.github/ISSUE_TEMPLATE/new_lint.md b/src/tools/clippy/.github/ISSUE_TEMPLATE/new_lint.md new file mode 100644 index 0000000000000..70445d7ef2503 --- /dev/null +++ b/src/tools/clippy/.github/ISSUE_TEMPLATE/new_lint.md @@ -0,0 +1,35 @@ +--- +name: New lint suggestion +about: Suggest a new Clippy lint. +labels: L-lint +--- + +### What it does + +*What does this lint do?* + +### Categories (optional) + +- Kind: *See for list of lint kinds* + +*What benefit of this lint over old code?* + +For example: +- Remove bounce checking inserted by ... +- Remove the need to duplicating/storing/typo ... + +### Drawbacks + +None. + +### Example + +```rust + +``` + +Could be written as: + +```rust + +``` diff --git a/src/tools/clippy/.github/PULL_REQUEST_TEMPLATE.md b/src/tools/clippy/.github/PULL_REQUEST_TEMPLATE.md index 97aa220afea54..137a73630940a 100644 --- a/src/tools/clippy/.github/PULL_REQUEST_TEMPLATE.md +++ b/src/tools/clippy/.github/PULL_REQUEST_TEMPLATE.md @@ -28,4 +28,5 @@ Delete this line and everything above before opening your PR. --- +*Please keep the line below* changelog: none diff --git a/src/tools/clippy/.github/driver.sh b/src/tools/clippy/.github/driver.sh index a2e87f5eb3745..2c17c4203ae5c 100644 --- a/src/tools/clippy/.github/driver.sh +++ b/src/tools/clippy/.github/driver.sh @@ -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 diff --git a/src/tools/clippy/clippy_lints/src/consts.rs b/src/tools/clippy/clippy_lints/src/consts.rs index 22c5acca064e9..550752396c732 100644 --- a/src/tools/clippy/clippy_lints/src/consts.rs +++ b/src/tools/clippy/clippy_lints/src/consts.rs @@ -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); diff --git a/src/tools/clippy/clippy_lints/src/escape.rs b/src/tools/clippy/clippy_lints/src/escape.rs index 59af475af175e..77e90eeac4958 100644 --- a/src/tools/clippy/clippy_lints/src/escape.rs +++ b/src/tools/clippy/clippy_lints/src/escape.rs @@ -6,7 +6,7 @@ use rustc_middle::ty::{self, Ty}; use rustc_session::{declare_tool_lint, impl_lint_pass}; use rustc_span::source_map::Span; use rustc_target::abi::LayoutOf; -use rustc_typeck::expr_use_visitor::{ConsumeMode, Delegate, ExprUseVisitor, PlaceWithHirId, PlaceBase}; +use rustc_typeck::expr_use_visitor::{ConsumeMode, Delegate, ExprUseVisitor, PlaceBase, PlaceWithHirId}; use crate::utils::span_lint; diff --git a/src/tools/clippy/clippy_lints/src/len_zero.rs b/src/tools/clippy/clippy_lints/src/len_zero.rs index 13e85fda8ffeb..7838e8e8ab774 100644 --- a/src/tools/clippy/clippy_lints/src/len_zero.rs +++ b/src/tools/clippy/clippy_lints/src/len_zero.rs @@ -211,7 +211,8 @@ fn check_impl_items(cx: &LateContext<'_, '_>, item: &Item<'_>, impl_items: &[Imp } fn check_cmp(cx: &LateContext<'_, '_>, span: Span, method: &Expr<'_>, lit: &Expr<'_>, op: &str, compare_to: u32) { - if let (&ExprKind::MethodCall(ref method_path, _, ref args, _), &ExprKind::Lit(ref lit)) = (&method.kind, &lit.kind) { + if let (&ExprKind::MethodCall(ref method_path, _, ref args, _), &ExprKind::Lit(ref lit)) = (&method.kind, &lit.kind) + { // check if we are in an is_empty() method if let Some(name) = get_item_name(cx, method) { if name.as_str() == "is_empty" { diff --git a/src/tools/clippy/clippy_lints/src/let_underscore.rs b/src/tools/clippy/clippy_lints/src/let_underscore.rs index 710dec8d33fc9..acd628bbaca59 100644 --- a/src/tools/clippy/clippy_lints/src/let_underscore.rs +++ b/src/tools/clippy/clippy_lints/src/let_underscore.rs @@ -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. diff --git a/src/tools/clippy/clippy_lints/src/lib.rs b/src/tools/clippy/clippy_lints/src/lib.rs index cd258c7b506c3..501220f28e5db 100644 --- a/src/tools/clippy/clippy_lints/src/lib.rs +++ b/src/tools/clippy/clippy_lints/src/lib.rs @@ -1,63 +1,44 @@ // 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`.) -#[allow(unused_extern_crates)] extern crate rustc_ast; -#[allow(unused_extern_crates)] extern crate rustc_ast_pretty; -#[allow(unused_extern_crates)] extern crate rustc_attr; -#[allow(unused_extern_crates)] extern crate rustc_data_structures; -#[allow(unused_extern_crates)] -extern crate rustc_driver; -#[allow(unused_extern_crates)] extern crate rustc_errors; -#[allow(unused_extern_crates)] extern crate rustc_hir; -#[allow(unused_extern_crates)] extern crate rustc_hir_pretty; -#[allow(unused_extern_crates)] extern crate rustc_index; -#[allow(unused_extern_crates)] extern crate rustc_infer; -#[allow(unused_extern_crates)] extern crate rustc_lexer; -#[allow(unused_extern_crates)] extern crate rustc_lint; -#[allow(unused_extern_crates)] extern crate rustc_middle; -#[allow(unused_extern_crates)] extern crate rustc_mir; -#[allow(unused_extern_crates)] extern crate rustc_parse; -#[allow(unused_extern_crates)] extern crate rustc_parse_format; -#[allow(unused_extern_crates)] extern crate rustc_session; -#[allow(unused_extern_crates)] extern crate rustc_span; -#[allow(unused_extern_crates)] extern crate rustc_target; -#[allow(unused_extern_crates)] extern crate rustc_trait_selection; -#[allow(unused_extern_crates)] extern crate rustc_typeck; use rustc_data_structures::fx::FxHashSet; @@ -82,14 +63,10 @@ use rustc_session::Session; /// # Example /// /// ``` -/// # #![feature(rustc_private)] -/// # #[allow(unused_extern_crates)] -/// # extern crate rustc_middle; -/// # #[allow(unused_extern_crates)] -/// # extern crate rustc_session; -/// # #[macro_use] -/// # use clippy_lints::declare_clippy_lint; +/// #![feature(rustc_private)] +/// extern crate rustc_session; /// use rustc_session::declare_tool_lint; +/// use clippy_lints::declare_clippy_lint; /// /// declare_clippy_lint! { /// /// **What it does:** Checks for ... (describe what the lint matches). @@ -1062,7 +1039,6 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: store.register_early_pass(|| box option_env_unwrap::OptionEnvUnwrap); let warn_on_all_wildcard_imports = conf.warn_on_all_wildcard_imports; store.register_late_pass(move || box wildcard_imports::WildcardImports::new(warn_on_all_wildcard_imports)); - store.register_early_pass(|| box macro_use::MacroUseImports); store.register_late_pass(|| box verbose_file_reads::VerboseFileReads); store.register_late_pass(|| box redundant_pub_crate::RedundantPubCrate::default()); store.register_late_pass(|| box unnamed_address::UnnamedAddress); @@ -1080,6 +1056,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: single_char_binding_names_threshold, }); store.register_early_pass(|| box unnested_or_patterns::UnnestedOrPatterns); + store.register_late_pass(|| box macro_use::MacroUseImports::default()); store.register_group(true, "clippy::restriction", Some("clippy_restriction"), vec![ LintId::of(&arithmetic::FLOAT_ARITHMETIC), @@ -1187,6 +1164,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), @@ -1440,7 +1418,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), @@ -1624,7 +1601,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), diff --git a/src/tools/clippy/clippy_lints/src/loops.rs b/src/tools/clippy/clippy_lints/src/loops.rs index 83093ec51bd90..9c8e8d8fabf4e 100644 --- a/src/tools/clippy/clippy_lints/src/loops.rs +++ b/src/tools/clippy/clippy_lints/src/loops.rs @@ -28,7 +28,7 @@ use rustc_middle::ty::{self, Ty, TyS}; use rustc_session::{declare_lint_pass, declare_tool_lint}; use rustc_span::source_map::Span; use rustc_span::symbol::Symbol; -use rustc_typeck::expr_use_visitor::{ConsumeMode, Delegate, ExprUseVisitor, PlaceWithHirId, PlaceBase}; +use rustc_typeck::expr_use_visitor::{ConsumeMode, Delegate, ExprUseVisitor, PlaceBase, PlaceWithHirId}; use std::iter::{once, Iterator}; use std::mem; @@ -1497,7 +1497,7 @@ struct MutatePairDelegate<'a, 'tcx> { span_high: Option, } -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) { @@ -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, Option) { (self.span_low, self.span_high) } @@ -1580,13 +1580,13 @@ fn check_for_mutability(cx: &LateContext<'_, '_>, bound: &Expr<'_>) -> Option ( +fn check_for_mutation<'a, 'tcx>( cx: &LateContext<'a, 'tcx>, body: &Expr<'_>, bound_ids: &[Option], ) -> (Option, Option) { let mut delegate = MutatePairDelegate { - cx: cx, + cx, hir_id_low: bound_ids[0], hir_id_high: bound_ids[1], span_low: None, @@ -2042,7 +2042,7 @@ impl<'a, 'tcx> Visitor<'tcx> for InitializeVisitor<'a, 'tcx> { if self.state == VarState::DontWarn { return; } - if SpanlessEq::new(self.cx).eq_expr(&expr, self.end_expr) { + if expr.hir_id == self.end_expr.hir_id { self.past_loop = true; return; } @@ -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<'_>) { diff --git a/src/tools/clippy/clippy_lints/src/macro_use.rs b/src/tools/clippy/clippy_lints/src/macro_use.rs index b1345d0b751ae..b845b20d2c012 100644 --- a/src/tools/clippy/clippy_lints/src/macro_use.rs +++ b/src/tools/clippy/clippy_lints/src/macro_use.rs @@ -1,10 +1,13 @@ -use crate::utils::{snippet, span_lint_and_sugg}; +use crate::utils::{in_macro, snippet, span_lint_and_sugg}; +use hir::def::{DefKind, Res}; use if_chain::if_chain; use rustc_ast::ast; +use rustc_data_structures::fx::{FxHashMap, FxHashSet}; use rustc_errors::Applicability; -use rustc_lint::{EarlyContext, EarlyLintPass}; -use rustc_session::{declare_lint_pass, declare_tool_lint}; -use rustc_span::edition::Edition; +use rustc_hir as hir; +use rustc_lint::{LateContext, LateLintPass, LintContext}; +use rustc_session::{declare_tool_lint, impl_lint_pass}; +use rustc_span::{edition::Edition, Span}; declare_clippy_lint! { /// **What it does:** Checks for `#[macro_use] use...`. @@ -12,7 +15,7 @@ declare_clippy_lint! { /// **Why is this bad?** Since the Rust 2018 edition you can import /// macro's directly, this is considered idiomatic. /// - /// **Known problems:** This lint does not generate an auto-applicable suggestion. + /// **Known problems:** None. /// /// **Example:** /// ```rust @@ -24,29 +27,205 @@ declare_clippy_lint! { "#[macro_use] is no longer needed" } -declare_lint_pass!(MacroUseImports => [MACRO_USE_IMPORTS]); +const BRACKETS: &[char] = &['<', '>']; -impl EarlyLintPass for MacroUseImports { - fn check_item(&mut self, ecx: &EarlyContext<'_>, item: &ast::Item) { +#[derive(Clone, Debug, PartialEq, Eq)] +struct PathAndSpan { + path: String, + span: Span, +} + +/// `MacroRefData` includes the name of the macro +/// and the path from `SourceMap::span_to_filename`. +#[derive(Debug, Clone)] +pub struct MacroRefData { + name: String, + path: String, +} + +impl MacroRefData { + pub fn new(name: String, callee: Span, cx: &LateContext<'_, '_>) -> Self { + let mut path = cx.sess().source_map().span_to_filename(callee).to_string(); + + // std lib paths are <::std::module::file type> + // so remove brackets, space and type. + if path.contains('<') { + path = path.replace(BRACKETS, ""); + } + if path.contains(' ') { + path = path.split(' ').next().unwrap().to_string(); + } + Self { name, path } + } +} + +#[derive(Default)] +#[allow(clippy::module_name_repetitions)] +pub struct MacroUseImports { + /// the actual import path used and the span of the attribute above it. + imports: Vec<(String, Span)>, + /// the span of the macro reference, kept to ensure only one reference is used per macro call. + collected: FxHashSet, + mac_refs: Vec, +} + +impl_lint_pass!(MacroUseImports => [MACRO_USE_IMPORTS]); + +impl MacroUseImports { + fn push_unique_macro(&mut self, cx: &LateContext<'_, '_>, span: Span) { + let call_site = span.source_callsite(); + let name = snippet(cx, cx.sess().source_map().span_until_char(call_site, '!'), "_"); + if let Some(callee) = span.source_callee() { + if !self.collected.contains(&call_site) { + let name = if name.contains("::") { + name.split("::").last().unwrap().to_string() + } else { + name.to_string() + }; + + self.mac_refs.push(MacroRefData::new(name, callee.def_site, cx)); + self.collected.insert(call_site); + } + } + } + + fn push_unique_macro_pat_ty(&mut self, cx: &LateContext<'_, '_>, span: Span) { + let call_site = span.source_callsite(); + let name = snippet(cx, cx.sess().source_map().span_until_char(call_site, '!'), "_"); + if let Some(callee) = span.source_callee() { + if !self.collected.contains(&call_site) { + self.mac_refs + .push(MacroRefData::new(name.to_string(), callee.def_site, cx)); + self.collected.insert(call_site); + } + } + } +} + +impl<'l, 'txc> LateLintPass<'l, 'txc> for MacroUseImports { + fn check_item(&mut self, cx: &LateContext<'_, '_>, item: &hir::Item<'_>) { if_chain! { - if ecx.sess.opts.edition == Edition::Edition2018; - if let ast::ItemKind::Use(use_tree) = &item.kind; + if cx.sess().opts.edition == Edition::Edition2018; + if let hir::ItemKind::Use(path, _kind) = &item.kind; if let Some(mac_attr) = item .attrs .iter() .find(|attr| attr.ident().map(|s| s.to_string()) == Some("macro_use".to_string())); + if let Res::Def(DefKind::Mod, id) = path.res; then { - let msg = "`macro_use` attributes are no longer needed in the Rust 2018 edition"; - let help = format!("use {}::", snippet(ecx, use_tree.span, "_")); + for kid in cx.tcx.item_children(id).iter() { + if let Res::Def(DefKind::Macro(_mac_type), mac_id) = kid.res { + let span = mac_attr.span; + let def_path = cx.tcx.def_path_str(mac_id); + self.imports.push((def_path, span)); + } + } + } else { + if in_macro(item.span) { + self.push_unique_macro_pat_ty(cx, item.span); + } + } + } + } + fn check_attribute(&mut self, cx: &LateContext<'_, '_>, attr: &ast::Attribute) { + if in_macro(attr.span) { + self.push_unique_macro(cx, attr.span); + } + } + fn check_expr(&mut self, cx: &LateContext<'_, '_>, expr: &hir::Expr<'_>) { + if in_macro(expr.span) { + self.push_unique_macro(cx, expr.span); + } + } + fn check_stmt(&mut self, cx: &LateContext<'_, '_>, stmt: &hir::Stmt<'_>) { + if in_macro(stmt.span) { + self.push_unique_macro(cx, stmt.span); + } + } + fn check_pat(&mut self, cx: &LateContext<'_, '_>, pat: &hir::Pat<'_>) { + if in_macro(pat.span) { + self.push_unique_macro_pat_ty(cx, pat.span); + } + } + fn check_ty(&mut self, cx: &LateContext<'_, '_>, ty: &hir::Ty<'_>) { + if in_macro(ty.span) { + self.push_unique_macro_pat_ty(cx, ty.span); + } + } + #[allow(clippy::too_many_lines)] + fn check_crate_post(&mut self, cx: &LateContext<'_, '_>, _krate: &hir::Crate<'_>) { + let mut used = FxHashMap::default(); + let mut check_dup = vec![]; + for (import, span) in &self.imports { + let found_idx = self.mac_refs.iter().position(|mac| import.ends_with(&mac.name)); + + if let Some(idx) = found_idx { + let _ = self.mac_refs.remove(idx); + let seg = import.split("::").collect::>(); + + match seg.as_slice() { + // an empty path is impossible + // a path should always consist of 2 or more segments + [] | [_] => return, + [root, item] => { + if !check_dup.contains(&(*item).to_string()) { + used.entry(((*root).to_string(), span)) + .or_insert_with(Vec::new) + .push((*item).to_string()); + check_dup.push((*item).to_string()); + } + }, + [root, rest @ ..] => { + if rest.iter().all(|item| !check_dup.contains(&(*item).to_string())) { + let filtered = rest + .iter() + .filter_map(|item| { + if check_dup.contains(&(*item).to_string()) { + None + } else { + Some((*item).to_string()) + } + }) + .collect::>(); + used.entry(((*root).to_string(), span)) + .or_insert_with(Vec::new) + .push(filtered.join("::")); + check_dup.extend(filtered); + } else { + let rest = rest.to_vec(); + used.entry(((*root).to_string(), span)) + .or_insert_with(Vec::new) + .push(rest.join("::")); + check_dup.extend(rest.iter().map(ToString::to_string)); + } + }, + } + } + } + + let mut suggestions = vec![]; + for ((root, span), path) in used { + if path.len() == 1 { + suggestions.push((span, format!("{}::{}", root, path[0]))) + } else { + suggestions.push((span, format!("{}::{{{}}}", root, path.join(", ")))) + } + } + + // If mac_refs is not empty we have encountered an import we could not handle + // such as `std::prelude::v1::foo` or some other macro that expands to an import. + if self.mac_refs.is_empty() { + for (span, import) in suggestions { + let help = format!("use {};", import); span_lint_and_sugg( - ecx, + cx, MACRO_USE_IMPORTS, - mac_attr.span, - msg, + *span, + "`macro_use` attributes are no longer needed in the Rust 2018 edition", "remove the attribute and import the macro directly, try", help, - Applicability::HasPlaceholders, - ); + Applicability::MaybeIncorrect, + ) } } } diff --git a/src/tools/clippy/clippy_lints/src/mem_replace.rs b/src/tools/clippy/clippy_lints/src/mem_replace.rs index ab6865bf0f3b7..e2672e02b36da 100644 --- a/src/tools/clippy/clippy_lints/src/mem_replace.rs +++ b/src/tools/clippy/clippy_lints/src/mem_replace.rs @@ -135,33 +135,59 @@ fn check_replace_option_with_none(cx: &LateContext<'_, '_>, src: &Expr<'_>, dest } } -fn check_replace_with_uninit(cx: &LateContext<'_, '_>, src: &Expr<'_>, expr_span: Span) { - if let ExprKind::Call(ref repl_func, ref repl_args) = src.kind { - if_chain! { - if repl_args.is_empty(); - if let ExprKind::Path(ref repl_func_qpath) = repl_func.kind; - if let Some(repl_def_id) = cx.tables.qpath_res(repl_func_qpath, repl_func.hir_id).opt_def_id(); - then { - if cx.tcx.is_diagnostic_item(sym::mem_uninitialized, repl_def_id) { - span_lint_and_help( - cx, - MEM_REPLACE_WITH_UNINIT, - expr_span, - "replacing with `mem::uninitialized()`", - None, - "consider using the `take_mut` crate instead", - ); - } else if cx.tcx.is_diagnostic_item(sym::mem_zeroed, repl_def_id) && - !cx.tables.expr_ty(src).is_primitive() { - span_lint_and_help( - cx, - MEM_REPLACE_WITH_UNINIT, - expr_span, - "replacing with `mem::zeroed()`", - None, - "consider using a default value or the `take_mut` crate instead", - ); - } +fn check_replace_with_uninit(cx: &LateContext<'_, '_>, src: &Expr<'_>, dest: &Expr<'_>, expr_span: Span) { + if_chain! { + // check if replacement is mem::MaybeUninit::uninit().assume_init() + if let Some(method_def_id) = cx.tables.type_dependent_def_id(src.hir_id); + if cx.tcx.is_diagnostic_item(sym::assume_init, method_def_id); + then { + let mut applicability = Applicability::MachineApplicable; + span_lint_and_sugg( + cx, + MEM_REPLACE_WITH_UNINIT, + expr_span, + "replacing with `mem::MaybeUninit::uninit().assume_init()`", + "consider using", + format!( + "std::ptr::read({})", + snippet_with_applicability(cx, dest.span, "", &mut applicability) + ), + applicability, + ); + return; + } + } + + if_chain! { + if let ExprKind::Call(ref repl_func, ref repl_args) = src.kind; + if repl_args.is_empty(); + if let ExprKind::Path(ref repl_func_qpath) = repl_func.kind; + if let Some(repl_def_id) = cx.tables.qpath_res(repl_func_qpath, repl_func.hir_id).opt_def_id(); + then { + if cx.tcx.is_diagnostic_item(sym::mem_uninitialized, repl_def_id) { + let mut applicability = Applicability::MachineApplicable; + span_lint_and_sugg( + cx, + MEM_REPLACE_WITH_UNINIT, + expr_span, + "replacing with `mem::uninitialized()`", + "consider using", + format!( + "std::ptr::read({})", + snippet_with_applicability(cx, dest.span, "", &mut applicability) + ), + applicability, + ); + } else if cx.tcx.is_diagnostic_item(sym::mem_zeroed, repl_def_id) && + !cx.tables.expr_ty(src).is_primitive() { + span_lint_and_help( + cx, + MEM_REPLACE_WITH_UNINIT, + expr_span, + "replacing with `mem::zeroed()`", + None, + "consider using a default value or the `take_mut` crate instead", + ); } } } @@ -209,7 +235,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for MemReplace { if let [dest, src] = &**func_args; then { check_replace_option_with_none(cx, src, dest, expr.span); - check_replace_with_uninit(cx, src, expr.span); + check_replace_with_uninit(cx, src, dest, expr.span); check_replace_with_default(cx, src, dest, expr.span); } } diff --git a/src/tools/clippy/clippy_lints/src/new_without_default.rs b/src/tools/clippy/clippy_lints/src/new_without_default.rs index dd236535c18ad..42200385932b0 100644 --- a/src/tools/clippy/clippy_lints/src/new_without_default.rs +++ b/src/tools/clippy/clippy_lints/src/new_without_default.rs @@ -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); diff --git a/src/tools/clippy/clippy_lints/src/redundant_pattern_matching.rs b/src/tools/clippy/clippy_lints/src/redundant_pattern_matching.rs index f16b916441ae8..3c528a295b044 100644 --- a/src/tools/clippy/clippy_lints/src/redundant_pattern_matching.rs +++ b/src/tools/clippy/clippy_lints/src/redundant_pattern_matching.rs @@ -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 @@ -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 @@ -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, _), @@ -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 @@ -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 @@ -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<'_>, @@ -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) @@ -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)) +} diff --git a/src/tools/clippy/clippy_lints/src/suspicious_trait_impl.rs b/src/tools/clippy/clippy_lints/src/suspicious_trait_impl.rs index a9e6fa329c0f0..cf71c3144a2eb 100644 --- a/src/tools/clippy/clippy_lints/src/suspicious_trait_impl.rs +++ b/src/tools/clippy/clippy_lints/src/suspicious_trait_impl.rs @@ -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<'_>) { diff --git a/src/tools/clippy/clippy_lints/src/trivially_copy_pass_by_ref.rs b/src/tools/clippy/clippy_lints/src/trivially_copy_pass_by_ref.rs index 8e0cb94317aff..146ac4b09d5a4 100644 --- a/src/tools/clippy/clippy_lints/src/trivially_copy_pass_by_ref.rs +++ b/src/tools/clippy/clippy_lints/src/trivially_copy_pass_by_ref.rs @@ -58,7 +58,7 @@ pub struct TriviallyCopyPassByRef { limit: u64, } -impl<'a, 'tcx> TriviallyCopyPassByRef { +impl<'tcx> TriviallyCopyPassByRef { pub fn new(limit: Option, target: &SessionConfig) -> Self { let limit = limit.unwrap_or_else(|| { let bit_width = u64::from(target.ptr_width); diff --git a/src/tools/clippy/clippy_lints/src/types.rs b/src/tools/clippy/clippy_lints/src/types.rs index d59a2f1bae031..98de08f79f3d7 100644 --- a/src/tools/clippy/clippy_lints/src/types.rs +++ b/src/tools/clippy/clippy_lints/src/types.rs @@ -1945,16 +1945,12 @@ fn detect_extreme_expr<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr<'_ let which = match (&ty.kind, cv) { (&ty::Bool, Constant::Bool(false)) | (&ty::Uint(_), Constant::Int(0)) => Minimum, - (&ty::Int(ity), Constant::Int(i)) - if i == unsext(cx.tcx, i128::MIN >> (128 - int_bits(cx.tcx, ity)), ity) => - { + (&ty::Int(ity), Constant::Int(i)) if i == unsext(cx.tcx, i128::MIN >> (128 - int_bits(cx.tcx, ity)), ity) => { Minimum }, (&ty::Bool, Constant::Bool(true)) => Maximum, - (&ty::Int(ity), Constant::Int(i)) - if i == unsext(cx.tcx, i128::MAX >> (128 - int_bits(cx.tcx, ity)), ity) => - { + (&ty::Int(ity), Constant::Int(i)) if i == unsext(cx.tcx, i128::MAX >> (128 - int_bits(cx.tcx, ity)), ity) => { Maximum }, (&ty::Uint(uty), Constant::Int(i)) if clip(cx.tcx, u128::MAX, uty) == i => Maximum, @@ -2083,50 +2079,20 @@ fn numeric_cast_precast_bounds<'a>(cx: &LateContext<'_, '_>, expr: &'a Expr<'_>) } match pre_cast_ty.kind { ty::Int(int_ty) => Some(match int_ty { - IntTy::I8 => ( - FullInt::S(i128::from(i8::MIN)), - FullInt::S(i128::from(i8::MAX)), - ), - IntTy::I16 => ( - FullInt::S(i128::from(i16::MIN)), - FullInt::S(i128::from(i16::MAX)), - ), - IntTy::I32 => ( - FullInt::S(i128::from(i32::MIN)), - FullInt::S(i128::from(i32::MAX)), - ), - IntTy::I64 => ( - FullInt::S(i128::from(i64::MIN)), - FullInt::S(i128::from(i64::MAX)), - ), + IntTy::I8 => (FullInt::S(i128::from(i8::MIN)), FullInt::S(i128::from(i8::MAX))), + IntTy::I16 => (FullInt::S(i128::from(i16::MIN)), FullInt::S(i128::from(i16::MAX))), + IntTy::I32 => (FullInt::S(i128::from(i32::MIN)), FullInt::S(i128::from(i32::MAX))), + IntTy::I64 => (FullInt::S(i128::from(i64::MIN)), FullInt::S(i128::from(i64::MAX))), IntTy::I128 => (FullInt::S(i128::MIN), FullInt::S(i128::MAX)), - IntTy::Isize => ( - FullInt::S(isize::MIN as i128), - FullInt::S(isize::MAX as i128), - ), + IntTy::Isize => (FullInt::S(isize::MIN as i128), FullInt::S(isize::MAX as i128)), }), ty::Uint(uint_ty) => Some(match uint_ty { - UintTy::U8 => ( - FullInt::U(u128::from(u8::MIN)), - FullInt::U(u128::from(u8::MAX)), - ), - UintTy::U16 => ( - FullInt::U(u128::from(u16::MIN)), - FullInt::U(u128::from(u16::MAX)), - ), - UintTy::U32 => ( - FullInt::U(u128::from(u32::MIN)), - FullInt::U(u128::from(u32::MAX)), - ), - UintTy::U64 => ( - FullInt::U(u128::from(u64::MIN)), - FullInt::U(u128::from(u64::MAX)), - ), + UintTy::U8 => (FullInt::U(u128::from(u8::MIN)), FullInt::U(u128::from(u8::MAX))), + UintTy::U16 => (FullInt::U(u128::from(u16::MIN)), FullInt::U(u128::from(u16::MAX))), + UintTy::U32 => (FullInt::U(u128::from(u32::MIN)), FullInt::U(u128::from(u32::MAX))), + UintTy::U64 => (FullInt::U(u128::from(u64::MIN)), FullInt::U(u128::from(u64::MAX))), UintTy::U128 => (FullInt::U(u128::MIN), FullInt::U(u128::MAX)), - UintTy::Usize => ( - FullInt::U(usize::MIN as u128), - FullInt::U(usize::MAX as u128), - ), + UintTy::Usize => (FullInt::U(usize::MIN as u128), FullInt::U(usize::MAX as u128)), }), _ => None, } diff --git a/src/tools/clippy/clippy_lints/src/unnecessary_sort_by.rs b/src/tools/clippy/clippy_lints/src/unnecessary_sort_by.rs index e94eebb88e497..6ac6a12529c86 100644 --- a/src/tools/clippy/clippy_lints/src/unnecessary_sort_by.rs +++ b/src/tools/clippy/clippy_lints/src/unnecessary_sort_by.rs @@ -95,7 +95,10 @@ fn mirrored_exprs( // The two exprs are method calls. // Check to see that the function is the same and the arguments are mirrored // This is enough because the receiver of the method is listed in the arguments - (ExprKind::MethodCall(left_segment, _, left_args, _), ExprKind::MethodCall(right_segment, _, right_args, _)) => { + ( + ExprKind::MethodCall(left_segment, _, left_args, _), + ExprKind::MethodCall(right_segment, _, right_args, _), + ) => { left_segment.ident == right_segment.ident && left_args .iter() diff --git a/src/tools/clippy/clippy_lints/src/unnested_or_patterns.rs b/src/tools/clippy/clippy_lints/src/unnested_or_patterns.rs index 8c281126c32bf..4d3682263f14f 100644 --- a/src/tools/clippy/clippy_lints/src/unnested_or_patterns.rs +++ b/src/tools/clippy/clippy_lints/src/unnested_or_patterns.rs @@ -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)`" } diff --git a/src/tools/clippy/clippy_lints/src/utils/author.rs b/src/tools/clippy/clippy_lints/src/utils/author.rs index 8b58bbb5e6575..910b665ccb75e 100644 --- a/src/tools/clippy/clippy_lints/src/utils/author.rs +++ b/src/tools/clippy/clippy_lints/src/utils/author.rs @@ -251,7 +251,10 @@ impl<'tcx> Visitor<'tcx> for PrintVisitor { } }, ExprKind::MethodCall(ref _method_name, ref _generics, ref _args, ref _fn_span) => { - println!("MethodCall(ref method_name, ref generics, ref args, ref fn_span) = {};", current); + println!( + "MethodCall(ref method_name, ref generics, ref args, ref fn_span) = {};", + current + ); println!(" // unimplemented: `ExprKind::MethodCall` is not further destructured at the moment"); }, ExprKind::Tup(ref elements) => { diff --git a/src/tools/clippy/clippy_lints/src/utils/conf.rs b/src/tools/clippy/clippy_lints/src/utils/conf.rs index 9e8e0ff30ec6b..c41befbf147b8 100644 --- a/src/tools/clippy/clippy_lints/src/utils/conf.rs +++ b/src/tools/clippy/clippy_lints/src/utils/conf.rs @@ -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, ["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, ["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. diff --git a/src/tools/clippy/clippy_lints/src/utils/hir_utils.rs b/src/tools/clippy/clippy_lints/src/utils/hir_utils.rs index 7a84f1c986aa7..9a9aa3f94eb4b 100644 --- a/src/tools/clippy/clippy_lints/src/utils/hir_utils.rs +++ b/src/tools/clippy/clippy_lints/src/utils/hir_utils.rs @@ -309,18 +309,15 @@ fn swap_binop<'a>( rhs: &'a Expr<'a>, ) -> Option<(BinOpKind, &'a Expr<'a>, &'a Expr<'a>)> { match binop { - BinOpKind::Add - | BinOpKind::Mul - | BinOpKind::Eq - | BinOpKind::Ne - | BinOpKind::BitAnd - | BinOpKind::BitXor - | BinOpKind::BitOr => Some((binop, rhs, lhs)), + BinOpKind::Add | BinOpKind::Eq | BinOpKind::Ne | BinOpKind::BitAnd | BinOpKind::BitXor | BinOpKind::BitOr => { + Some((binop, rhs, lhs)) + }, BinOpKind::Lt => Some((BinOpKind::Gt, rhs, lhs)), BinOpKind::Le => Some((BinOpKind::Ge, rhs, lhs)), BinOpKind::Ge => Some((BinOpKind::Le, rhs, lhs)), BinOpKind::Gt => Some((BinOpKind::Lt, rhs, lhs)), - BinOpKind::Shl + BinOpKind::Mul // Not always commutative, e.g. with matrices. See issue #5698 + | BinOpKind::Shl | BinOpKind::Shr | BinOpKind::Rem | BinOpKind::Sub diff --git a/src/tools/clippy/clippy_lints/src/utils/sugg.rs b/src/tools/clippy/clippy_lints/src/utils/sugg.rs index 73758b7eeb7eb..e919b1522d89a 100644 --- a/src/tools/clippy/clippy_lints/src/utils/sugg.rs +++ b/src/tools/clippy/clippy_lints/src/utils/sugg.rs @@ -509,7 +509,7 @@ fn indentation(cx: &T, span: Span) -> Option { } /// Convenience extension trait for `DiagnosticBuilder`. -pub trait DiagnosticBuilderExt<'a, T: LintContext> { +pub trait DiagnosticBuilderExt { /// Suggests to add an attribute to an item. /// /// Correctly handles indentation of the attribute and item. @@ -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 DiagnosticBuilderExt for rustc_errors::DiagnosticBuilder<'_> { fn suggest_item_with_attr( &mut self, cx: &T, diff --git a/src/tools/clippy/clippy_lints/src/utils/usage.rs b/src/tools/clippy/clippy_lints/src/utils/usage.rs index 6a7a1f1ceaaef..0492878fc272f 100644 --- a/src/tools/clippy/clippy_lints/src/utils/usage.rs +++ b/src/tools/clippy/clippy_lints/src/utils/usage.rs @@ -8,7 +8,7 @@ use rustc_lint::LateContext; use rustc_middle::hir::map::Map; use rustc_middle::ty; use rustc_span::symbol::{Ident, Symbol}; -use rustc_typeck::expr_use_visitor::{ConsumeMode, Delegate, ExprUseVisitor, PlaceWithHirId, PlaceBase}; +use rustc_typeck::expr_use_visitor::{ConsumeMode, Delegate, ExprUseVisitor, PlaceBase, PlaceWithHirId}; /// Returns a set of mutated local variable IDs, or `None` if mutations could not be determined. pub fn mutated_variables<'a, 'tcx>(expr: &'tcx Expr<'_>, cx: &'a LateContext<'a, 'tcx>) -> Option> { diff --git a/src/tools/clippy/clippy_lints/src/wildcard_imports.rs b/src/tools/clippy/clippy_lints/src/wildcard_imports.rs index b637253bd0264..79f7705e281e5 100644 --- a/src/tools/clippy/clippy_lints/src/wildcard_imports.rs +++ b/src/tools/clippy/clippy_lints/src/wildcard_imports.rs @@ -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: /// diff --git a/src/tools/clippy/src/driver.rs b/src/tools/clippy/src/driver.rs index 4453ae5ce4414..decd3a79cce18 100644 --- a/src/tools/clippy/src/driver.rs +++ b/src/tools/clippy/src/driver.rs @@ -1,15 +1,16 @@ -#![cfg_attr(feature = "deny-warnings", deny(warnings))] #![feature(rustc_private)] +#![cfg_attr(feature = "deny-warnings", deny(warnings))] +// 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`.) -#[allow(unused_extern_crates)] +extern crate rustc_data_structures; extern crate rustc_driver; -#[allow(unused_extern_crates)] extern crate rustc_errors; -#[allow(unused_extern_crates)] extern crate rustc_interface; -#[allow(unused_extern_crates)] extern crate rustc_middle; use rustc_interface::interface; @@ -93,7 +94,7 @@ impl rustc_driver::Callbacks for ClippyCallbacks { #[allow(clippy::find_map, clippy::filter_map)] fn describe_lints() { use lintlist::{Level, Lint, ALL_LINTS, LINT_LEVELS}; - use std::collections::HashSet; + use rustc_data_structures::fx::FxHashSet; println!( " @@ -137,7 +138,7 @@ Available lint options: let scoped = |x: &str| format!("clippy::{}", x); - let lint_groups: HashSet<_> = lints.iter().map(|lint| lint.group).collect(); + let lint_groups: FxHashSet<_> = lints.iter().map(|lint| lint.group).collect(); println!("Lint checks provided by clippy:\n"); println!(" {} {:7.7} meaning", padded("name"), "default"); @@ -207,6 +208,7 @@ Usage: Common options: -h, --help Print this message + --rustc Pass all args to rustc -V, --version Print version info and exit Other options are the same as `cargo check`. @@ -297,12 +299,6 @@ pub fn main() { exit(rustc_driver::catch_with_exit_code(move || { let mut orig_args: Vec = env::args().collect(); - if orig_args.iter().any(|a| a == "--version" || a == "-V") { - let version_info = rustc_tools_util::get_version_info!(); - println!("{}", version_info); - exit(0); - } - // Get the sysroot, looking from most specific to this invocation to the least: // - command line // - runtime environment @@ -348,6 +344,28 @@ pub fn main() { .map(|pb| pb.to_string_lossy().to_string()) .expect("need to specify SYSROOT env var during clippy compilation, or use rustup or multirust"); + // make "clippy-driver --rustc" work like a subcommand that passes further args to "rustc" + // for example `clippy-driver --rustc --version` will print the rustc version that clippy-driver + // uses + if let Some(pos) = orig_args.iter().position(|arg| arg == "--rustc") { + orig_args.remove(pos); + orig_args[0] = "rustc".to_string(); + + // if we call "rustc", we need to pass --sysroot here as well + let mut args: Vec = orig_args.clone(); + if !have_sys_root_arg { + args.extend(vec!["--sysroot".into(), sys_root]); + }; + + return rustc_driver::run_compiler(&args, &mut DefaultCallbacks, None, None); + } + + if orig_args.iter().any(|a| a == "--version" || a == "-V") { + let version_info = rustc_tools_util::get_version_info!(); + println!("{}", version_info); + exit(0); + } + // Setting RUSTC_WRAPPER causes Cargo to pass 'rustc' as the first argument. // We're invoking the compiler programmatically, so we ignore this/ let wrapper_mode = orig_args.get(1).map(Path::new).and_then(Path::file_stem) == Some("rustc".as_ref()); diff --git a/src/tools/clippy/src/lintlist/mod.rs b/src/tools/clippy/src/lintlist/mod.rs index cac3cc6bdb316..edceb75518008 100644 --- a/src/tools/clippy/src/lintlist/mod.rs +++ b/src/tools/clippy/src/lintlist/mod.rs @@ -2329,7 +2329,7 @@ pub static ref ALL_LINTS: Vec = vec![ }, Lint { name: "unnested_or_patterns", - group: "complexity", + group: "pedantic", desc: "unnested or-patterns, e.g., `Foo(Bar) | Foo(Baz) instead of `Foo(Bar | Baz)`", deprecation: None, module: "unnested_or_patterns", diff --git a/src/tools/clippy/src/main.rs b/src/tools/clippy/src/main.rs index bc43a34ed5d4a..6739a4cf2245e 100644 --- a/src/tools/clippy/src/main.rs +++ b/src/tools/clippy/src/main.rs @@ -1,4 +1,6 @@ #![cfg_attr(feature = "deny-warnings", deny(warnings))] +// warn on lints, that are included in `rust-lang/rust`s bootstrap +#![warn(rust_2018_idioms, unused_lifetimes)] use rustc_tools_util::VersionInfo; use std::env; diff --git a/src/tools/clippy/tests/ui/auxiliary/macro_use_helper.rs b/src/tools/clippy/tests/ui/auxiliary/macro_use_helper.rs new file mode 100644 index 0000000000000..ecb55d8cb48d5 --- /dev/null +++ b/src/tools/clippy/tests/ui/auxiliary/macro_use_helper.rs @@ -0,0 +1,60 @@ +extern crate macro_rules; + +// STMT +#[macro_export] +macro_rules! pub_macro { + () => { + let _ = "hello Mr. Vonnegut"; + }; +} + +pub mod inner { + pub use super::*; + + // RE-EXPORT + // this will stick in `inner` module + pub use macro_rules::foofoo; + pub use macro_rules::try_err; + + pub mod nested { + pub use macro_rules::string_add; + } + + // ITEM + #[macro_export] + macro_rules! inner_mod_macro { + () => { + #[allow(dead_code)] + pub struct Tardis; + }; + } +} + +// EXPR +#[macro_export] +macro_rules! function_macro { + () => { + if true { + } else { + } + }; +} + +// TYPE +#[macro_export] +macro_rules! ty_macro { + () => { + Vec + }; +} + +mod extern_exports { + pub(super) mod private_inner { + #[macro_export] + macro_rules! pub_in_private_macro { + ($name:ident) => { + let $name = String::from("secrets and lies"); + }; + } + } +} diff --git a/src/tools/clippy/tests/ui/blacklisted_name.rs b/src/tools/clippy/tests/ui/blacklisted_name.rs index ca9d8d16b787d..cb15bdd2f1b2d 100644 --- a/src/tools/clippy/tests/ui/blacklisted_name.rs +++ b/src/tools/clippy/tests/ui/blacklisted_name.rs @@ -12,29 +12,34 @@ fn test(foo: ()) {} fn main() { let foo = 42; - let bar = 42; let baz = 42; + let quux = 42; + // Unlike these others, `bar` is actually considered an acceptable name. + // Among many other legitimate uses, bar commonly refers to a period of time in music. + // See https://github.com/rust-lang/rust-clippy/issues/5225. + let bar = 42; - let barb = 42; - let barbaric = 42; + let food = 42; + let foodstuffs = 42; + let bazaar = 42; match (42, Some(1337), Some(0)) { - (foo, Some(bar), baz @ Some(_)) => (), + (foo, Some(baz), quux @ Some(_)) => (), _ => (), } } fn issue_1647(mut foo: u8) { - let mut bar = 0; - if let Some(mut baz) = Some(42) {} + let mut baz = 0; + if let Some(mut quux) = Some(42) {} } fn issue_1647_ref() { - let ref bar = 0; - if let Some(ref baz) = Some(42) {} + let ref baz = 0; + if let Some(ref quux) = Some(42) {} } fn issue_1647_ref_mut() { - let ref mut bar = 0; - if let Some(ref mut baz) = Some(42) {} + let ref mut baz = 0; + if let Some(ref mut quux) = Some(42) {} } diff --git a/src/tools/clippy/tests/ui/blacklisted_name.stderr b/src/tools/clippy/tests/ui/blacklisted_name.stderr index 44123829fb0f6..70dbdaece8b6b 100644 --- a/src/tools/clippy/tests/ui/blacklisted_name.stderr +++ b/src/tools/clippy/tests/ui/blacklisted_name.stderr @@ -12,77 +12,77 @@ error: use of a blacklisted/placeholder name `foo` LL | let foo = 42; | ^^^ -error: use of a blacklisted/placeholder name `bar` +error: use of a blacklisted/placeholder name `baz` --> $DIR/blacklisted_name.rs:15:9 | -LL | let bar = 42; +LL | let baz = 42; | ^^^ -error: use of a blacklisted/placeholder name `baz` +error: use of a blacklisted/placeholder name `quux` --> $DIR/blacklisted_name.rs:16:9 | -LL | let baz = 42; - | ^^^ +LL | let quux = 42; + | ^^^^ error: use of a blacklisted/placeholder name `foo` - --> $DIR/blacklisted_name.rs:22:10 + --> $DIR/blacklisted_name.rs:27:10 | -LL | (foo, Some(bar), baz @ Some(_)) => (), +LL | (foo, Some(baz), quux @ Some(_)) => (), | ^^^ -error: use of a blacklisted/placeholder name `bar` - --> $DIR/blacklisted_name.rs:22:20 +error: use of a blacklisted/placeholder name `baz` + --> $DIR/blacklisted_name.rs:27:20 | -LL | (foo, Some(bar), baz @ Some(_)) => (), +LL | (foo, Some(baz), quux @ Some(_)) => (), | ^^^ -error: use of a blacklisted/placeholder name `baz` - --> $DIR/blacklisted_name.rs:22:26 +error: use of a blacklisted/placeholder name `quux` + --> $DIR/blacklisted_name.rs:27:26 | -LL | (foo, Some(bar), baz @ Some(_)) => (), - | ^^^ +LL | (foo, Some(baz), quux @ Some(_)) => (), + | ^^^^ error: use of a blacklisted/placeholder name `foo` - --> $DIR/blacklisted_name.rs:27:19 + --> $DIR/blacklisted_name.rs:32:19 | LL | fn issue_1647(mut foo: u8) { | ^^^ -error: use of a blacklisted/placeholder name `bar` - --> $DIR/blacklisted_name.rs:28:13 +error: use of a blacklisted/placeholder name `baz` + --> $DIR/blacklisted_name.rs:33:13 | -LL | let mut bar = 0; +LL | let mut baz = 0; | ^^^ -error: use of a blacklisted/placeholder name `baz` - --> $DIR/blacklisted_name.rs:29:21 +error: use of a blacklisted/placeholder name `quux` + --> $DIR/blacklisted_name.rs:34:21 | -LL | if let Some(mut baz) = Some(42) {} - | ^^^ +LL | if let Some(mut quux) = Some(42) {} + | ^^^^ -error: use of a blacklisted/placeholder name `bar` - --> $DIR/blacklisted_name.rs:33:13 +error: use of a blacklisted/placeholder name `baz` + --> $DIR/blacklisted_name.rs:38:13 | -LL | let ref bar = 0; +LL | let ref baz = 0; | ^^^ -error: use of a blacklisted/placeholder name `baz` - --> $DIR/blacklisted_name.rs:34:21 +error: use of a blacklisted/placeholder name `quux` + --> $DIR/blacklisted_name.rs:39:21 | -LL | if let Some(ref baz) = Some(42) {} - | ^^^ +LL | if let Some(ref quux) = Some(42) {} + | ^^^^ -error: use of a blacklisted/placeholder name `bar` - --> $DIR/blacklisted_name.rs:38:17 +error: use of a blacklisted/placeholder name `baz` + --> $DIR/blacklisted_name.rs:43:17 | -LL | let ref mut bar = 0; +LL | let ref mut baz = 0; | ^^^ -error: use of a blacklisted/placeholder name `baz` - --> $DIR/blacklisted_name.rs:39:25 +error: use of a blacklisted/placeholder name `quux` + --> $DIR/blacklisted_name.rs:44:25 | -LL | if let Some(ref mut baz) = Some(42) {} - | ^^^ +LL | if let Some(ref mut quux) = Some(42) {} + | ^^^^ error: aborting due to 14 previous errors diff --git a/src/tools/clippy/tests/ui/crashes/ice-5389.rs b/src/tools/clippy/tests/ui/crashes/ice-5389.rs new file mode 100644 index 0000000000000..de262199004b0 --- /dev/null +++ b/src/tools/clippy/tests/ui/crashes/ice-5389.rs @@ -0,0 +1,13 @@ +#![allow(clippy::explicit_counter_loop)] + +fn main() { + let v = vec![1, 2, 3]; + let mut i = 0; + let max_storage_size = [0; 128 * 1024]; + for item in &v { + bar(i, *item); + i += 1; + } +} + +fn bar(_: usize, _: u32) {} diff --git a/src/tools/clippy/tests/ui/if_same_then_else.rs b/src/tools/clippy/tests/ui/if_same_then_else.rs index 6bbf79edfcf70..9c5fe02f7519b 100644 --- a/src/tools/clippy/tests/ui/if_same_then_else.rs +++ b/src/tools/clippy/tests/ui/if_same_then_else.rs @@ -142,4 +142,16 @@ fn func() { fn f(val: &[u8]) {} +mod issue_5698 { + fn mul_not_always_commutative(x: i32, y: i32) -> i32 { + if x == 42 { + x * y + } else if x == 21 { + y * x + } else { + 0 + } + } +} + fn main() {} diff --git a/src/tools/clippy/tests/ui/macro_use_imports.fixed b/src/tools/clippy/tests/ui/macro_use_imports.fixed new file mode 100644 index 0000000000000..91e34c62160a1 --- /dev/null +++ b/src/tools/clippy/tests/ui/macro_use_imports.fixed @@ -0,0 +1,43 @@ +// compile-flags: --edition 2018 +// aux-build:macro_rules.rs +// aux-build:macro_use_helper.rs +// run-rustfix +// ignore-32bit + +#![allow(unused_imports, unreachable_code, unused_variables, dead_code)] +#![allow(clippy::single_component_path_imports)] +#![warn(clippy::macro_use_imports)] + +#[macro_use] +extern crate macro_use_helper as mac; + +#[macro_use] +extern crate clippy_mini_macro_test as mini_mac; + +mod a { + use mac::{pub_macro, inner_mod_macro, function_macro, ty_macro, pub_in_private_macro}; + use mac; + use mini_mac::ClippyMiniMacroTest; + use mini_mac; + use mac::{inner::foofoo, inner::try_err}; + use mac::inner; + use mac::inner::nested::string_add; + use mac::inner::nested; + + #[derive(ClippyMiniMacroTest)] + struct Test; + + fn test() { + pub_macro!(); + inner_mod_macro!(); + pub_in_private_macro!(_var); + function_macro!(); + let v: ty_macro!() = Vec::default(); + + inner::try_err!(); + inner::foofoo!(); + nested::string_add!(); + } +} + +fn main() {} diff --git a/src/tools/clippy/tests/ui/macro_use_imports.rs b/src/tools/clippy/tests/ui/macro_use_imports.rs index 60c64ee8146e5..9c3c50c5d49f2 100644 --- a/src/tools/clippy/tests/ui/macro_use_imports.rs +++ b/src/tools/clippy/tests/ui/macro_use_imports.rs @@ -1,11 +1,43 @@ -// edition:2018 +// compile-flags: --edition 2018 +// aux-build:macro_rules.rs +// aux-build:macro_use_helper.rs +// run-rustfix +// ignore-32bit + +#![allow(unused_imports, unreachable_code, unused_variables, dead_code)] +#![allow(clippy::single_component_path_imports)] #![warn(clippy::macro_use_imports)] -use std::collections::HashMap; #[macro_use] -use std::prelude; +extern crate macro_use_helper as mac; + +#[macro_use] +extern crate clippy_mini_macro_test as mini_mac; + +mod a { + #[macro_use] + use mac; + #[macro_use] + use mini_mac; + #[macro_use] + use mac::inner; + #[macro_use] + use mac::inner::nested; -fn main() { - let _ = HashMap::::new(); - println!(); + #[derive(ClippyMiniMacroTest)] + struct Test; + + fn test() { + pub_macro!(); + inner_mod_macro!(); + pub_in_private_macro!(_var); + function_macro!(); + let v: ty_macro!() = Vec::default(); + + inner::try_err!(); + inner::foofoo!(); + nested::string_add!(); + } } + +fn main() {} diff --git a/src/tools/clippy/tests/ui/macro_use_imports.stderr b/src/tools/clippy/tests/ui/macro_use_imports.stderr index b5e3dbec57277..f8c86c8d9179f 100644 --- a/src/tools/clippy/tests/ui/macro_use_imports.stderr +++ b/src/tools/clippy/tests/ui/macro_use_imports.stderr @@ -1,10 +1,28 @@ error: `macro_use` attributes are no longer needed in the Rust 2018 edition - --> $DIR/macro_use_imports.rs:5:1 + --> $DIR/macro_use_imports.rs:18:5 | -LL | #[macro_use] - | ^^^^^^^^^^^^ help: remove the attribute and import the macro directly, try: `use std::prelude::` +LL | #[macro_use] + | ^^^^^^^^^^^^ help: remove the attribute and import the macro directly, try: `use mac::{pub_macro, inner_mod_macro, function_macro, ty_macro, pub_in_private_macro};` | = note: `-D clippy::macro-use-imports` implied by `-D warnings` -error: aborting due to previous error +error: `macro_use` attributes are no longer needed in the Rust 2018 edition + --> $DIR/macro_use_imports.rs:20:5 + | +LL | #[macro_use] + | ^^^^^^^^^^^^ help: remove the attribute and import the macro directly, try: `use mini_mac::ClippyMiniMacroTest;` + +error: `macro_use` attributes are no longer needed in the Rust 2018 edition + --> $DIR/macro_use_imports.rs:22:5 + | +LL | #[macro_use] + | ^^^^^^^^^^^^ help: remove the attribute and import the macro directly, try: `use mac::{inner::foofoo, inner::try_err};` + +error: `macro_use` attributes are no longer needed in the Rust 2018 edition + --> $DIR/macro_use_imports.rs:24:5 + | +LL | #[macro_use] + | ^^^^^^^^^^^^ help: remove the attribute and import the macro directly, try: `use mac::inner::nested::string_add;` + +error: aborting due to 4 previous errors diff --git a/src/tools/clippy/tests/ui/redundant_pattern_matching.fixed b/src/tools/clippy/tests/ui/redundant_pattern_matching.fixed index fc8cb0e747c73..6ba5cfb1d7177 100644 --- a/src/tools/clippy/tests/ui/redundant_pattern_matching.fixed +++ b/src/tools/clippy/tests/ui/redundant_pattern_matching.fixed @@ -1,5 +1,7 @@ // run-rustfix +#![feature(const_if_match)] +#![feature(const_loop)] #![warn(clippy::all)] #![warn(clippy::redundant_pattern_matching)] #![allow(clippy::unit_arg, unused_must_use, clippy::needless_bool, deprecated)] @@ -67,6 +69,7 @@ fn main() { takes_bool(x); issue5504(); + issue5697(); let _ = if gen_opt().is_some() { 1 @@ -117,3 +120,42 @@ fn issue5504() { if m!().is_some() {} while m!().is_some() {} } + +// None of these should be linted because none of the suggested methods +// are `const fn` without toggling a feature. +const fn issue5697() { + if let Ok(_) = Ok::(42) {} + + if let Err(_) = Err::(42) {} + + if let Some(_) = Some(42) {} + + if let None = None::<()> {} + + while let Ok(_) = Ok::(10) {} + + while let Err(_) = Ok::(10) {} + + while let Some(_) = Some(42) {} + + while let None = None::<()> {} + + match Ok::(42) { + Ok(_) => true, + Err(_) => false, + }; + + match Err::(42) { + Ok(_) => false, + Err(_) => true, + }; + match Some(42) { + Some(_) => true, + None => false, + }; + + match None::<()> { + Some(_) => false, + None => true, + }; +} diff --git a/src/tools/clippy/tests/ui/redundant_pattern_matching.rs b/src/tools/clippy/tests/ui/redundant_pattern_matching.rs index 51912dade0356..17de66f9ad0eb 100644 --- a/src/tools/clippy/tests/ui/redundant_pattern_matching.rs +++ b/src/tools/clippy/tests/ui/redundant_pattern_matching.rs @@ -1,5 +1,7 @@ // run-rustfix +#![feature(const_if_match)] +#![feature(const_loop)] #![warn(clippy::all)] #![warn(clippy::redundant_pattern_matching)] #![allow(clippy::unit_arg, unused_must_use, clippy::needless_bool, deprecated)] @@ -88,6 +90,7 @@ fn main() { takes_bool(x); issue5504(); + issue5697(); let _ = if let Some(_) = gen_opt() { 1 @@ -138,3 +141,42 @@ fn issue5504() { if let Some(_) = m!() {} while let Some(_) = m!() {} } + +// None of these should be linted because none of the suggested methods +// are `const fn` without toggling a feature. +const fn issue5697() { + if let Ok(_) = Ok::(42) {} + + if let Err(_) = Err::(42) {} + + if let Some(_) = Some(42) {} + + if let None = None::<()> {} + + while let Ok(_) = Ok::(10) {} + + while let Err(_) = Ok::(10) {} + + while let Some(_) = Some(42) {} + + while let None = None::<()> {} + + match Ok::(42) { + Ok(_) => true, + Err(_) => false, + }; + + match Err::(42) { + Ok(_) => false, + Err(_) => true, + }; + match Some(42) { + Some(_) => true, + None => false, + }; + + match None::<()> { + Some(_) => false, + None => true, + }; +} diff --git a/src/tools/clippy/tests/ui/redundant_pattern_matching.stderr b/src/tools/clippy/tests/ui/redundant_pattern_matching.stderr index b58deb7954efe..1b9a4b40a2f02 100644 --- a/src/tools/clippy/tests/ui/redundant_pattern_matching.stderr +++ b/src/tools/clippy/tests/ui/redundant_pattern_matching.stderr @@ -1,5 +1,5 @@ error: redundant pattern matching, consider using `is_ok()` - --> $DIR/redundant_pattern_matching.rs:8:12 + --> $DIR/redundant_pattern_matching.rs:10:12 | LL | if let Ok(_) = Ok::(42) {} | -------^^^^^--------------------- help: try this: `if Ok::(42).is_ok()` @@ -7,67 +7,67 @@ LL | if let Ok(_) = Ok::(42) {} = note: `-D clippy::redundant-pattern-matching` implied by `-D warnings` error: redundant pattern matching, consider using `is_err()` - --> $DIR/redundant_pattern_matching.rs:10:12 + --> $DIR/redundant_pattern_matching.rs:12:12 | LL | if let Err(_) = Err::(42) {} | -------^^^^^^---------------------- help: try this: `if Err::(42).is_err()` error: redundant pattern matching, consider using `is_none()` - --> $DIR/redundant_pattern_matching.rs:12:12 + --> $DIR/redundant_pattern_matching.rs:14:12 | LL | if let None = None::<()> {} | -------^^^^------------- help: try this: `if None::<()>.is_none()` error: redundant pattern matching, consider using `is_some()` - --> $DIR/redundant_pattern_matching.rs:14:12 + --> $DIR/redundant_pattern_matching.rs:16:12 | LL | if let Some(_) = Some(42) {} | -------^^^^^^^----------- help: try this: `if Some(42).is_some()` error: redundant pattern matching, consider using `is_some()` - --> $DIR/redundant_pattern_matching.rs:16:12 + --> $DIR/redundant_pattern_matching.rs:18:12 | LL | if let Some(_) = Some(42) { | -------^^^^^^^----------- help: try this: `if Some(42).is_some()` error: redundant pattern matching, consider using `is_some()` - --> $DIR/redundant_pattern_matching.rs:22:15 + --> $DIR/redundant_pattern_matching.rs:24:15 | LL | while let Some(_) = Some(42) {} | ----------^^^^^^^----------- help: try this: `while Some(42).is_some()` error: redundant pattern matching, consider using `is_none()` - --> $DIR/redundant_pattern_matching.rs:24:15 + --> $DIR/redundant_pattern_matching.rs:26:15 | LL | while let None = Some(42) {} | ----------^^^^----------- help: try this: `while Some(42).is_none()` error: redundant pattern matching, consider using `is_none()` - --> $DIR/redundant_pattern_matching.rs:26:15 + --> $DIR/redundant_pattern_matching.rs:28:15 | LL | while let None = None::<()> {} | ----------^^^^------------- help: try this: `while None::<()>.is_none()` error: redundant pattern matching, consider using `is_ok()` - --> $DIR/redundant_pattern_matching.rs:28:15 + --> $DIR/redundant_pattern_matching.rs:30:15 | LL | while let Ok(_) = Ok::(10) {} | ----------^^^^^--------------------- help: try this: `while Ok::(10).is_ok()` error: redundant pattern matching, consider using `is_err()` - --> $DIR/redundant_pattern_matching.rs:30:15 + --> $DIR/redundant_pattern_matching.rs:32:15 | LL | while let Err(_) = Ok::(10) {} | ----------^^^^^^--------------------- help: try this: `while Ok::(10).is_err()` error: redundant pattern matching, consider using `is_some()` - --> $DIR/redundant_pattern_matching.rs:33:15 + --> $DIR/redundant_pattern_matching.rs:35:15 | LL | while let Some(_) = v.pop() { | ----------^^^^^^^---------- help: try this: `while v.pop().is_some()` error: redundant pattern matching, consider using `is_ok()` - --> $DIR/redundant_pattern_matching.rs:49:5 + --> $DIR/redundant_pattern_matching.rs:51:5 | LL | / match Ok::(42) { LL | | Ok(_) => true, @@ -76,7 +76,7 @@ LL | | }; | |_____^ help: try this: `Ok::(42).is_ok()` error: redundant pattern matching, consider using `is_err()` - --> $DIR/redundant_pattern_matching.rs:54:5 + --> $DIR/redundant_pattern_matching.rs:56:5 | LL | / match Ok::(42) { LL | | Ok(_) => false, @@ -85,7 +85,7 @@ LL | | }; | |_____^ help: try this: `Ok::(42).is_err()` error: redundant pattern matching, consider using `is_err()` - --> $DIR/redundant_pattern_matching.rs:59:5 + --> $DIR/redundant_pattern_matching.rs:61:5 | LL | / match Err::(42) { LL | | Ok(_) => false, @@ -94,7 +94,7 @@ LL | | }; | |_____^ help: try this: `Err::(42).is_err()` error: redundant pattern matching, consider using `is_ok()` - --> $DIR/redundant_pattern_matching.rs:64:5 + --> $DIR/redundant_pattern_matching.rs:66:5 | LL | / match Err::(42) { LL | | Ok(_) => true, @@ -103,7 +103,7 @@ LL | | }; | |_____^ help: try this: `Err::(42).is_ok()` error: redundant pattern matching, consider using `is_some()` - --> $DIR/redundant_pattern_matching.rs:69:5 + --> $DIR/redundant_pattern_matching.rs:71:5 | LL | / match Some(42) { LL | | Some(_) => true, @@ -112,7 +112,7 @@ LL | | }; | |_____^ help: try this: `Some(42).is_some()` error: redundant pattern matching, consider using `is_none()` - --> $DIR/redundant_pattern_matching.rs:74:5 + --> $DIR/redundant_pattern_matching.rs:76:5 | LL | / match None::<()> { LL | | Some(_) => false, @@ -121,7 +121,7 @@ LL | | }; | |_____^ help: try this: `None::<()>.is_none()` error: redundant pattern matching, consider using `is_none()` - --> $DIR/redundant_pattern_matching.rs:79:13 + --> $DIR/redundant_pattern_matching.rs:81:13 | LL | let _ = match None::<()> { | _____________^ @@ -131,61 +131,61 @@ LL | | }; | |_____^ help: try this: `None::<()>.is_none()` error: redundant pattern matching, consider using `is_ok()` - --> $DIR/redundant_pattern_matching.rs:84:20 + --> $DIR/redundant_pattern_matching.rs:86:20 | LL | let _ = if let Ok(_) = Ok::(4) { true } else { false }; | -------^^^^^--------------------- help: try this: `if Ok::(4).is_ok()` error: redundant pattern matching, consider using `is_some()` - --> $DIR/redundant_pattern_matching.rs:87:20 + --> $DIR/redundant_pattern_matching.rs:89:20 | LL | let x = if let Some(_) = opt { true } else { false }; | -------^^^^^^^------ help: try this: `if opt.is_some()` error: redundant pattern matching, consider using `is_some()` - --> $DIR/redundant_pattern_matching.rs:92:20 + --> $DIR/redundant_pattern_matching.rs:95:20 | LL | let _ = if let Some(_) = gen_opt() { | -------^^^^^^^------------ help: try this: `if gen_opt().is_some()` error: redundant pattern matching, consider using `is_none()` - --> $DIR/redundant_pattern_matching.rs:94:19 + --> $DIR/redundant_pattern_matching.rs:97:19 | LL | } else if let None = gen_opt() { | -------^^^^------------ help: try this: `if gen_opt().is_none()` error: redundant pattern matching, consider using `is_ok()` - --> $DIR/redundant_pattern_matching.rs:96:19 + --> $DIR/redundant_pattern_matching.rs:99:19 | LL | } else if let Ok(_) = gen_res() { | -------^^^^^------------ help: try this: `if gen_res().is_ok()` error: redundant pattern matching, consider using `is_err()` - --> $DIR/redundant_pattern_matching.rs:98:19 + --> $DIR/redundant_pattern_matching.rs:101:19 | LL | } else if let Err(_) = gen_res() { | -------^^^^^^------------ help: try this: `if gen_res().is_err()` error: redundant pattern matching, consider using `is_some()` - --> $DIR/redundant_pattern_matching.rs:131:19 + --> $DIR/redundant_pattern_matching.rs:134:19 | LL | while let Some(_) = r#try!(result_opt()) {} | ----------^^^^^^^----------------------- help: try this: `while r#try!(result_opt()).is_some()` error: redundant pattern matching, consider using `is_some()` - --> $DIR/redundant_pattern_matching.rs:132:16 + --> $DIR/redundant_pattern_matching.rs:135:16 | LL | if let Some(_) = r#try!(result_opt()) {} | -------^^^^^^^----------------------- help: try this: `if r#try!(result_opt()).is_some()` error: redundant pattern matching, consider using `is_some()` - --> $DIR/redundant_pattern_matching.rs:138:12 + --> $DIR/redundant_pattern_matching.rs:141:12 | LL | if let Some(_) = m!() {} | -------^^^^^^^------- help: try this: `if m!().is_some()` error: redundant pattern matching, consider using `is_some()` - --> $DIR/redundant_pattern_matching.rs:139:15 + --> $DIR/redundant_pattern_matching.rs:142:15 | LL | while let Some(_) = m!() {} | ----------^^^^^^^------- help: try this: `while m!().is_some()` diff --git a/src/tools/clippy/tests/ui/redundant_pattern_matching_const_result.fixed b/src/tools/clippy/tests/ui/redundant_pattern_matching_const_result.fixed new file mode 100644 index 0000000000000..c8bc5458067d3 --- /dev/null +++ b/src/tools/clippy/tests/ui/redundant_pattern_matching_const_result.fixed @@ -0,0 +1,46 @@ +// run-rustfix + +#![feature(const_if_match)] +#![feature(const_loop)] +#![feature(const_result)] +#![warn(clippy::redundant_pattern_matching)] +#![allow(unused)] + +// Test that results are linted with the feature enabled. + +const fn issue_5697() { + if Ok::(42).is_ok() {} + + if Err::(42).is_err() {} + + while Ok::(10).is_ok() {} + + while Ok::(10).is_err() {} + + Ok::(42).is_ok(); + + Err::(42).is_err(); + + // These should not be linted until `const_option` is implemented. + // See https://github.com/rust-lang/rust/issues/67441 + + if let Some(_) = Some(42) {} + + if let None = None::<()> {} + + while let Some(_) = Some(42) {} + + while let None = None::<()> {} + + match Some(42) { + Some(_) => true, + None => false, + }; + + match None::<()> { + Some(_) => false, + None => true, + }; +} + +fn main() {} diff --git a/src/tools/clippy/tests/ui/redundant_pattern_matching_const_result.rs b/src/tools/clippy/tests/ui/redundant_pattern_matching_const_result.rs new file mode 100644 index 0000000000000..75f37ec15c622 --- /dev/null +++ b/src/tools/clippy/tests/ui/redundant_pattern_matching_const_result.rs @@ -0,0 +1,52 @@ +// run-rustfix + +#![feature(const_if_match)] +#![feature(const_loop)] +#![feature(const_result)] +#![warn(clippy::redundant_pattern_matching)] +#![allow(unused)] + +// Test that results are linted with the feature enabled. + +const fn issue_5697() { + if let Ok(_) = Ok::(42) {} + + if let Err(_) = Err::(42) {} + + while let Ok(_) = Ok::(10) {} + + while let Err(_) = Ok::(10) {} + + match Ok::(42) { + Ok(_) => true, + Err(_) => false, + }; + + match Err::(42) { + Ok(_) => false, + Err(_) => true, + }; + + // These should not be linted until `const_option` is implemented. + // See https://github.com/rust-lang/rust/issues/67441 + + if let Some(_) = Some(42) {} + + if let None = None::<()> {} + + while let Some(_) = Some(42) {} + + while let None = None::<()> {} + + match Some(42) { + Some(_) => true, + None => false, + }; + + match None::<()> { + Some(_) => false, + None => true, + }; +} + +fn main() {} diff --git a/src/tools/clippy/tests/ui/redundant_pattern_matching_const_result.stderr b/src/tools/clippy/tests/ui/redundant_pattern_matching_const_result.stderr new file mode 100644 index 0000000000000..c32292f0eee8b --- /dev/null +++ b/src/tools/clippy/tests/ui/redundant_pattern_matching_const_result.stderr @@ -0,0 +1,46 @@ +error: redundant pattern matching, consider using `is_ok()` + --> $DIR/redundant_pattern_matching_const_result.rs:12:12 + | +LL | if let Ok(_) = Ok::(42) {} + | -------^^^^^--------------------- help: try this: `if Ok::(42).is_ok()` + | + = note: `-D clippy::redundant-pattern-matching` implied by `-D warnings` + +error: redundant pattern matching, consider using `is_err()` + --> $DIR/redundant_pattern_matching_const_result.rs:14:12 + | +LL | if let Err(_) = Err::(42) {} + | -------^^^^^^---------------------- help: try this: `if Err::(42).is_err()` + +error: redundant pattern matching, consider using `is_ok()` + --> $DIR/redundant_pattern_matching_const_result.rs:16:15 + | +LL | while let Ok(_) = Ok::(10) {} + | ----------^^^^^--------------------- help: try this: `while Ok::(10).is_ok()` + +error: redundant pattern matching, consider using `is_err()` + --> $DIR/redundant_pattern_matching_const_result.rs:18:15 + | +LL | while let Err(_) = Ok::(10) {} + | ----------^^^^^^--------------------- help: try this: `while Ok::(10).is_err()` + +error: redundant pattern matching, consider using `is_ok()` + --> $DIR/redundant_pattern_matching_const_result.rs:20:5 + | +LL | / match Ok::(42) { +LL | | Ok(_) => true, +LL | | Err(_) => false, +LL | | }; + | |_____^ help: try this: `Ok::(42).is_ok()` + +error: redundant pattern matching, consider using `is_err()` + --> $DIR/redundant_pattern_matching_const_result.rs:25:5 + | +LL | / match Err::(42) { +LL | | Ok(_) => false, +LL | | Err(_) => true, +LL | | }; + | |_____^ help: try this: `Err::(42).is_err()` + +error: aborting due to 6 previous errors + diff --git a/src/tools/clippy/tests/ui/repl_uninit.rs b/src/tools/clippy/tests/ui/repl_uninit.rs index 346972b7bb4e0..ad5b8e4857d17 100644 --- a/src/tools/clippy/tests/ui/repl_uninit.rs +++ b/src/tools/clippy/tests/ui/repl_uninit.rs @@ -17,6 +17,12 @@ fn main() { std::mem::forget(mem::replace(&mut v, new_v)); } + unsafe { + let taken_v = mem::replace(&mut v, mem::MaybeUninit::uninit().assume_init()); + let new_v = might_panic(taken_v); + std::mem::forget(mem::replace(&mut v, new_v)); + } + unsafe { let taken_v = mem::replace(&mut v, mem::zeroed()); let new_v = might_panic(taken_v); diff --git a/src/tools/clippy/tests/ui/repl_uninit.stderr b/src/tools/clippy/tests/ui/repl_uninit.stderr index c1f55d7601e5c..09468eeaea4bf 100644 --- a/src/tools/clippy/tests/ui/repl_uninit.stderr +++ b/src/tools/clippy/tests/ui/repl_uninit.stderr @@ -2,26 +2,29 @@ error: replacing with `mem::uninitialized()` --> $DIR/repl_uninit.rs:15:23 | LL | let taken_v = mem::replace(&mut v, mem::uninitialized()); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using: `std::ptr::read(&mut v)` | = note: `-D clippy::mem-replace-with-uninit` implied by `-D warnings` - = help: consider using the `take_mut` crate instead -error: replacing with `mem::zeroed()` +error: replacing with `mem::MaybeUninit::uninit().assume_init()` --> $DIR/repl_uninit.rs:21:23 | +LL | let taken_v = mem::replace(&mut v, mem::MaybeUninit::uninit().assume_init()); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using: `std::ptr::read(&mut v)` + +error: replacing with `mem::zeroed()` + --> $DIR/repl_uninit.rs:27:23 + | LL | let taken_v = mem::replace(&mut v, mem::zeroed()); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | = help: consider using a default value or the `take_mut` crate instead error: replacing with `mem::uninitialized()` - --> $DIR/repl_uninit.rs:33:28 + --> $DIR/repl_uninit.rs:39:28 | LL | let taken_u = unsafe { mem::replace(uref, mem::uninitialized()) }; - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ - | - = help: consider using the `take_mut` crate instead + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using: `std::ptr::read(uref)` -error: aborting due to 3 previous errors +error: aborting due to 4 previous errors