diff options
| author | Michael Wright <mikerite@lavabit.com> | 2019-09-22 08:59:23 +0200 |
|---|---|---|
| committer | Michael Wright <mikerite@lavabit.com> | 2019-09-22 08:59:23 +0200 |
| commit | d04bf1511411121cdff4688544dd4d6afa53f7ae (patch) | |
| tree | 47abaa2b8f5ccabea6df5515578efaf7e42f9a87 | |
| parent | fed1709f46791c0b46b6460740d15f0f5b89d8d5 (diff) | |
| parent | d07d001b74f384e34d5f670c34e45bdc9a372ca2 (diff) | |
| download | rust-d04bf1511411121cdff4688544dd4d6afa53f7ae.tar.gz rust-d04bf1511411121cdff4688544dd4d6afa53f7ae.zip | |
Merge branch 'master' into unneeded_wildcard_pattern
62 files changed, 1246 insertions, 389 deletions
diff --git a/CHANGELOG.md b/CHANGELOG.md index eb710654caf..6d190286e0a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1050,12 +1050,14 @@ Released 2018-09-13 [`mem_discriminant_non_enum`]: https://rust-lang.github.io/rust-clippy/master/index.html#mem_discriminant_non_enum [`mem_forget`]: https://rust-lang.github.io/rust-clippy/master/index.html#mem_forget [`mem_replace_option_with_none`]: https://rust-lang.github.io/rust-clippy/master/index.html#mem_replace_option_with_none +[`mem_replace_with_uninit`]: https://rust-lang.github.io/rust-clippy/master/index.html#mem_replace_with_uninit [`min_max`]: https://rust-lang.github.io/rust-clippy/master/index.html#min_max [`misaligned_transmute`]: https://rust-lang.github.io/rust-clippy/master/index.html#misaligned_transmute [`misrefactored_assign_op`]: https://rust-lang.github.io/rust-clippy/master/index.html#misrefactored_assign_op [`missing_const_for_fn`]: https://rust-lang.github.io/rust-clippy/master/index.html#missing_const_for_fn [`missing_docs_in_private_items`]: https://rust-lang.github.io/rust-clippy/master/index.html#missing_docs_in_private_items [`missing_inline_in_public_items`]: https://rust-lang.github.io/rust-clippy/master/index.html#missing_inline_in_public_items +[`missing_safety_doc`]: https://rust-lang.github.io/rust-clippy/master/index.html#missing_safety_doc [`mistyped_literal_suffixes`]: https://rust-lang.github.io/rust-clippy/master/index.html#mistyped_literal_suffixes [`mixed_case_hex_literals`]: https://rust-lang.github.io/rust-clippy/master/index.html#mixed_case_hex_literals [`module_inception`]: https://rust-lang.github.io/rust-clippy/master/index.html#module_inception diff --git a/README.md b/README.md index 4541af9c844..915396b901c 100644 --- a/README.md +++ b/README.md @@ -6,7 +6,7 @@ A collection of lints to catch common mistakes and improve your [Rust](https://github.com/rust-lang/rust) code. -[There are 314 lints included in this crate!](https://rust-lang.github.io/rust-clippy/master/index.html) +[There are 316 lints included in this crate!](https://rust-lang.github.io/rust-clippy/master/index.html) We have a bunch of lint categories to allow you to choose how much Clippy is supposed to ~~annoy~~ help you: diff --git a/clippy_lints/Cargo.toml b/clippy_lints/Cargo.toml index 48133cc5b02..423e13e9dd5 100644 --- a/clippy_lints/Cargo.toml +++ b/clippy_lints/Cargo.toml @@ -27,7 +27,7 @@ semver = "0.9.0" serde = { version = "1.0", features = ["derive"] } toml = "0.5.3" unicode-normalization = "0.1" -pulldown-cmark = "0.5.3" +pulldown-cmark = "0.6.0" url = { version = "2.1.0", features = ["serde"] } # cargo requires serde feat in its url dep # see https://github.com/rust-lang/rust/pull/63587#issuecomment-522343864 if_chain = "1.0.0" diff --git a/clippy_lints/src/assign_ops.rs b/clippy_lints/src/assign_ops.rs index ec4c8f5faa3..a56d751b22d 100644 --- a/clippy_lints/src/assign_ops.rs +++ b/clippy_lints/src/assign_ops.rs @@ -216,13 +216,13 @@ fn lint_misrefactored_assign_op( long ), format!("{} {}= {}", snip_a, op.node.as_str(), snip_r), - Applicability::MachineApplicable, + Applicability::MaybeIncorrect, ); db.span_suggestion( expr.span, "or", long, - Applicability::MachineApplicable, // snippet + Applicability::MaybeIncorrect, // snippet ); } }, diff --git a/clippy_lints/src/doc.rs b/clippy_lints/src/doc.rs index 4950212a4b5..86d83bf9602 100644 --- a/clippy_lints/src/doc.rs +++ b/clippy_lints/src/doc.rs @@ -34,6 +34,40 @@ declare_clippy_lint! { "presence of `_`, `::` or camel-case outside backticks in documentation" } +declare_clippy_lint! { + /// **What it does:** Checks for the doc comments of publicly visible + /// unsafe functions and warns if there is no `# Safety` section. + /// + /// **Why is this bad?** Unsafe functions should document their safety + /// preconditions, so that users can be sure they are using them safely. + /// + /// **Known problems:** None. + /// + /// **Examples**: + /// ```rust + ///# type Universe = (); + /// /// This function should really be documented + /// pub unsafe fn start_apocalypse(u: &mut Universe) { + /// unimplemented!(); + /// } + /// ``` + /// + /// At least write a line about safety: + /// + /// ```rust + ///# type Universe = (); + /// /// # Safety + /// /// + /// /// This function should not be called before the horsemen are ready. + /// pub unsafe fn start_apocalypse(u: &mut Universe) { + /// unimplemented!(); + /// } + /// ``` + pub MISSING_SAFETY_DOC, + style, + "`pub unsafe fn` without `# Safety` docs" +} + #[allow(clippy::module_name_repetitions)] #[derive(Clone)] pub struct DocMarkdown { @@ -46,7 +80,7 @@ impl DocMarkdown { } } -impl_lint_pass!(DocMarkdown => [DOC_MARKDOWN]); +impl_lint_pass!(DocMarkdown => [DOC_MARKDOWN, MISSING_SAFETY_DOC]); impl EarlyLintPass for DocMarkdown { fn check_crate(&mut self, cx: &EarlyContext<'_>, krate: &ast::Crate) { @@ -54,7 +88,20 @@ impl EarlyLintPass for DocMarkdown { } fn check_item(&mut self, cx: &EarlyContext<'_>, item: &ast::Item) { - check_attrs(cx, &self.valid_idents, &item.attrs); + if check_attrs(cx, &self.valid_idents, &item.attrs) { + return; + } + // no safety header + if let ast::ItemKind::Fn(_, ref header, ..) = item.node { + if item.vis.node.is_pub() && header.unsafety == ast::Unsafety::Unsafe { + span_lint( + cx, + MISSING_SAFETY_DOC, + item.span, + "unsafe function's docs miss `# Safety` section", + ); + } + } } } @@ -115,7 +162,7 @@ pub fn strip_doc_comment_decoration(comment: &str, span: Span) -> (String, Vec<( panic!("not a doc-comment: {}", comment); } -pub fn check_attrs<'a>(cx: &EarlyContext<'_>, valid_idents: &FxHashSet<String>, attrs: &'a [ast::Attribute]) { +pub fn check_attrs<'a>(cx: &EarlyContext<'_>, valid_idents: &FxHashSet<String>, attrs: &'a [ast::Attribute]) -> bool { let mut doc = String::new(); let mut spans = vec![]; @@ -129,7 +176,7 @@ pub fn check_attrs<'a>(cx: &EarlyContext<'_>, valid_idents: &FxHashSet<String>, } } else if attr.check_name(sym!(doc)) { // ignore mix of sugared and non-sugared doc - return; + return true; // don't trigger the safety check } } @@ -140,26 +187,28 @@ pub fn check_attrs<'a>(cx: &EarlyContext<'_>, valid_idents: &FxHashSet<String>, current += offset_copy; } - if !doc.is_empty() { - let parser = pulldown_cmark::Parser::new(&doc).into_offset_iter(); - // Iterate over all `Events` and combine consecutive events into one - let events = parser.coalesce(|previous, current| { - use pulldown_cmark::Event::*; - - let previous_range = previous.1; - let current_range = current.1; - - match (previous.0, current.0) { - (Text(previous), Text(current)) => { - let mut previous = previous.to_string(); - previous.push_str(¤t); - Ok((Text(previous.into()), previous_range)) - }, - (previous, current) => Err(((previous, previous_range), (current, current_range))), - } - }); - check_doc(cx, valid_idents, events, &spans); + if doc.is_empty() { + return false; } + + let parser = pulldown_cmark::Parser::new(&doc).into_offset_iter(); + // Iterate over all `Events` and combine consecutive events into one + let events = parser.coalesce(|previous, current| { + use pulldown_cmark::Event::*; + + let previous_range = previous.1; + let current_range = current.1; + + match (previous.0, current.0) { + (Text(previous), Text(current)) => { + let mut previous = previous.to_string(); + previous.push_str(¤t); + Ok((Text(previous.into()), previous_range)) + }, + (previous, current) => Err(((previous, previous_range), (current, current_range))), + } + }); + check_doc(cx, valid_idents, events, &spans) } fn check_doc<'a, Events: Iterator<Item = (pulldown_cmark::Event<'a>, Range<usize>)>>( @@ -167,12 +216,15 @@ fn check_doc<'a, Events: Iterator<Item = (pulldown_cmark::Event<'a>, Range<usize valid_idents: &FxHashSet<String>, events: Events, spans: &[(usize, Span)], -) { +) -> bool { + // true if a safety header was found use pulldown_cmark::Event::*; use pulldown_cmark::Tag::*; + let mut safety_header = false; let mut in_code = false; let mut in_link = None; + let mut in_heading = false; for (event, range) in events { match event { @@ -180,9 +232,11 @@ fn check_doc<'a, Events: Iterator<Item = (pulldown_cmark::Event<'a>, Range<usize End(CodeBlock(_)) => in_code = false, Start(Link(_, url, _)) => in_link = Some(url), End(Link(..)) => in_link = None, - Start(_tag) | End(_tag) => (), // We don't care about other tags - Html(_html) | InlineHtml(_html) => (), // HTML is weird, just ignore it - SoftBreak | HardBreak | TaskListMarker(_) | Code(_) => (), + Start(Heading(_)) => in_heading = true, + End(Heading(_)) => in_heading = false, + Start(_tag) | End(_tag) => (), // We don't care about other tags + Html(_html) => (), // HTML is weird, just ignore it + SoftBreak | HardBreak | TaskListMarker(_) | Code(_) | Rule => (), FootnoteReference(text) | Text(text) => { if Some(&text) == in_link.as_ref() { // Probably a link of the form `<http://example.com>` @@ -190,7 +244,7 @@ fn check_doc<'a, Events: Iterator<Item = (pulldown_cmark::Event<'a>, Range<usize // text "http://example.com" by pulldown-cmark continue; } - + safety_header |= in_heading && text.trim() == "Safety"; if !in_code { let index = match spans.binary_search_by(|c| c.0.cmp(&range.start)) { Ok(o) => o, @@ -207,6 +261,7 @@ fn check_doc<'a, Events: Iterator<Item = (pulldown_cmark::Event<'a>, Range<usize }, } } + safety_header } fn check_text(cx: &EarlyContext<'_>, valid_idents: &FxHashSet<String>, text: &str, span: Span) { diff --git a/clippy_lints/src/drop_forget_ref.rs b/clippy_lints/src/drop_forget_ref.rs index a815eae9fbb..023fd569318 100644 --- a/clippy_lints/src/drop_forget_ref.rs +++ b/clippy_lints/src/drop_forget_ref.rs @@ -1,4 +1,4 @@ -use crate::utils::{is_copy, match_def_path, paths, span_note_and_lint}; +use crate::utils::{is_copy, match_def_path, paths, qpath_res, span_note_and_lint}; use if_chain::if_chain; use rustc::hir::*; use rustc::lint::{LateContext, LateLintPass, LintArray, LintPass}; @@ -114,7 +114,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for DropForgetRef { if let ExprKind::Call(ref path, ref args) = expr.node; if let ExprKind::Path(ref qpath) = path.node; if args.len() == 1; - if let Some(def_id) = cx.tables.qpath_res(qpath, path.hir_id).opt_def_id(); + if let Some(def_id) = qpath_res(cx, qpath, path.hir_id).opt_def_id(); then { let lint; let msg; diff --git a/clippy_lints/src/explicit_write.rs b/clippy_lints/src/explicit_write.rs index a2edb4855b6..477f0e76025 100644 --- a/clippy_lints/src/explicit_write.rs +++ b/clippy_lints/src/explicit_write.rs @@ -140,7 +140,8 @@ fn write_output_string(write_args: &HirVec<Expr>) -> Option<String> { if output_args.len() > 0; if let ExprKind::AddrOf(_, ref output_string_expr) = output_args[0].node; if let ExprKind::Array(ref string_exprs) = output_string_expr.node; - if string_exprs.len() > 0; + // we only want to provide an automatic suggestion for simple (non-format) strings + if string_exprs.len() == 1; if let ExprKind::Lit(ref lit) = string_exprs[0].node; if let LitKind::Str(ref write_output, _) = lit.node; then { diff --git a/clippy_lints/src/functions.rs b/clippy_lints/src/functions.rs index a609c74c172..b332148cbdd 100644 --- a/clippy_lints/src/functions.rs +++ b/clippy_lints/src/functions.rs @@ -1,6 +1,6 @@ use std::convert::TryFrom; -use crate::utils::{iter_input_pats, snippet, snippet_opt, span_lint, type_is_unsafe_function}; +use crate::utils::{iter_input_pats, qpath_res, snippet, snippet_opt, span_lint, type_is_unsafe_function}; use matches::matches; use rustc::hir; use rustc::hir::def::Res; @@ -318,7 +318,7 @@ impl<'a, 'tcx> hir::intravisit::Visitor<'tcx> for DerefVisitor<'a, 'tcx> { impl<'a, 'tcx> DerefVisitor<'a, 'tcx> { fn check_arg(&self, ptr: &hir::Expr) { if let hir::ExprKind::Path(ref qpath) = ptr.node { - if let Res::Local(id) = self.cx.tables.qpath_res(qpath, ptr.hir_id) { + if let Res::Local(id) = qpath_res(self.cx, qpath, ptr.hir_id) { if self.ptrs.contains(&id) { span_lint( self.cx, diff --git a/clippy_lints/src/let_if_seq.rs b/clippy_lints/src/let_if_seq.rs index 845e40c5edf..4b1b57b4808 100644 --- a/clippy_lints/src/let_if_seq.rs +++ b/clippy_lints/src/let_if_seq.rs @@ -1,4 +1,4 @@ -use crate::utils::{higher, snippet, span_lint_and_then}; +use crate::utils::{higher, qpath_res, snippet, span_lint_and_then}; use if_chain::if_chain; use rustc::hir; use rustc::hir::def::Res; @@ -145,7 +145,7 @@ impl<'a, 'tcx> hir::intravisit::Visitor<'tcx> for UsedVisitor<'a, 'tcx> { fn visit_expr(&mut self, expr: &'tcx hir::Expr) { if_chain! { if let hir::ExprKind::Path(ref qpath) = expr.node; - if let Res::Local(local_id) = self.cx.tables.qpath_res(qpath, expr.hir_id); + if let Res::Local(local_id) = qpath_res(self.cx, qpath, expr.hir_id); if self.id == local_id; then { self.used = true; @@ -170,7 +170,7 @@ fn check_assign<'a, 'tcx>( if let hir::StmtKind::Semi(ref expr) = expr.node; if let hir::ExprKind::Assign(ref var, ref value) = expr.node; if let hir::ExprKind::Path(ref qpath) = var.node; - if let Res::Local(local_id) = cx.tables.qpath_res(qpath, var.hir_id); + if let Res::Local(local_id) = qpath_res(cx, qpath, var.hir_id); if decl == local_id; then { let mut v = UsedVisitor { diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index de4d262bddc..2caa065b937 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -667,6 +667,7 @@ pub fn register_plugins(reg: &mut rustc_driver::plugin::Registry<'_>, conf: &Con shadow::SHADOW_UNRELATED, strings::STRING_ADD_ASSIGN, trait_bounds::TYPE_REPETITION_IN_BOUNDS, + types::CAST_LOSSLESS, types::CAST_POSSIBLE_TRUNCATION, types::CAST_POSSIBLE_WRAP, types::CAST_PRECISION_LOSS, @@ -708,6 +709,7 @@ pub fn register_plugins(reg: &mut rustc_driver::plugin::Registry<'_>, conf: &Con copies::IFS_SAME_COND, copies::IF_SAME_THEN_ELSE, derive::DERIVE_HASH_XOR_EQ, + doc::MISSING_SAFETY_DOC, double_comparison::DOUBLE_COMPARISONS, double_parens::DOUBLE_PARENS, drop_bounds::DROP_BOUNDS, @@ -781,6 +783,7 @@ pub fn register_plugins(reg: &mut rustc_driver::plugin::Registry<'_>, conf: &Con matches::SINGLE_MATCH, mem_discriminant::MEM_DISCRIMINANT_NON_ENUM, mem_replace::MEM_REPLACE_OPTION_WITH_NONE, + mem_replace::MEM_REPLACE_WITH_UNINIT, methods::CHARS_LAST_CMP, methods::CHARS_NEXT_CMP, methods::CLONE_DOUBLE_REF, @@ -891,7 +894,6 @@ pub fn register_plugins(reg: &mut rustc_driver::plugin::Registry<'_>, conf: &Con types::ABSURD_EXTREME_COMPARISONS, types::BORROWED_BOX, types::BOX_VEC, - types::CAST_LOSSLESS, types::CAST_PTR_ALIGNMENT, types::CAST_REF_TO_MUT, types::CHAR_LIT_AS_U8, @@ -930,6 +932,7 @@ pub fn register_plugins(reg: &mut rustc_driver::plugin::Registry<'_>, conf: &Con block_in_if_condition::BLOCK_IN_IF_CONDITION_EXPR, block_in_if_condition::BLOCK_IN_IF_CONDITION_STMT, collapsible_if::COLLAPSIBLE_IF, + doc::MISSING_SAFETY_DOC, enum_variants::ENUM_VARIANT_NAMES, enum_variants::MODULE_INCEPTION, eq_op::OP_REF, @@ -1074,7 +1077,6 @@ pub fn register_plugins(reg: &mut rustc_driver::plugin::Registry<'_>, conf: &Con transmute::TRANSMUTE_PTR_TO_REF, transmute::USELESS_TRANSMUTE, types::BORROWED_BOX, - types::CAST_LOSSLESS, types::CHAR_LIT_AS_U8, types::OPTION_OPTION, types::TYPE_COMPLEXITY, @@ -1118,6 +1120,7 @@ pub fn register_plugins(reg: &mut rustc_driver::plugin::Registry<'_>, conf: &Con loops::REVERSE_RANGE_LOOP, loops::WHILE_IMMUTABLE_CONDITION, mem_discriminant::MEM_DISCRIMINANT_NON_ENUM, + mem_replace::MEM_REPLACE_WITH_UNINIT, methods::CLONE_DOUBLE_REF, methods::INTO_ITER_ON_ARRAY, methods::TEMPORARY_CSTRING_AS_PTR, diff --git a/clippy_lints/src/loops.rs b/clippy_lints/src/loops.rs index 882d9889742..0032cfd1985 100644 --- a/clippy_lints/src/loops.rs +++ b/clippy_lints/src/loops.rs @@ -11,7 +11,7 @@ use rustc::{declare_lint_pass, declare_tool_lint}; // use rustc::middle::region::CodeExtent; use crate::consts::{constant, Constant}; use crate::utils::usage::mutated_variables; -use crate::utils::{is_type_diagnostic_item, sext, sugg}; +use crate::utils::{is_type_diagnostic_item, qpath_res, sext, sugg}; use rustc::middle::expr_use_visitor::*; use rustc::middle::mem_categorization::cmt_; use rustc::middle::mem_categorization::Categorization; @@ -754,7 +754,7 @@ fn same_var<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, expr: &Expr, var: HirId) -> bo if let ExprKind::Path(ref qpath) = expr.node; if let QPath::Resolved(None, ref path) = *qpath; if path.segments.len() == 1; - if let Res::Local(local_id) = cx.tables.qpath_res(qpath, expr.hir_id); + if let Res::Local(local_id) = qpath_res(cx, qpath, expr.hir_id); // our variable! if local_id == var; then { @@ -1272,7 +1272,7 @@ fn check_for_loop_reverse_range<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, arg: &'tcx let start_snippet = snippet(cx, start.span, "_"); let end_snippet = snippet(cx, end.span, "_"); let dots = if limits == ast::RangeLimits::Closed { - "..." + "..=" } else { ".." }; @@ -1618,7 +1618,7 @@ fn check_for_mutability(cx: &LateContext<'_, '_>, bound: &Expr) -> Option<HirId> if let ExprKind::Path(ref qpath) = bound.node; if let QPath::Resolved(None, _) = *qpath; then { - let res = cx.tables.qpath_res(qpath, bound.hir_id); + let res = qpath_res(cx, qpath, bound.hir_id); if let Res::Local(node_id) = res { let node_str = cx.tcx.hir().get(node_id); if_chain! { @@ -1762,7 +1762,7 @@ impl<'a, 'tcx> VarVisitor<'a, 'tcx> { if self.prefer_mutable { self.indexed_mut.insert(seqvar.segments[0].ident.name); } - let res = self.cx.tables.qpath_res(seqpath, seqexpr.hir_id); + let res = qpath_res(self.cx, seqpath, seqexpr.hir_id); match res { Res::Local(hir_id) => { let parent_id = self.cx.tcx.hir().get_parent_item(expr.hir_id); @@ -1824,7 +1824,7 @@ impl<'a, 'tcx> Visitor<'tcx> for VarVisitor<'a, 'tcx> { if let QPath::Resolved(None, ref path) = *qpath; if path.segments.len() == 1; then { - if let Res::Local(local_id) = self.cx.tables.qpath_res(qpath, expr.hir_id) { + if let Res::Local(local_id) = qpath_res(self.cx, qpath, expr.hir_id) { if local_id == self.var { self.nonindex = true; } else { @@ -2163,7 +2163,7 @@ impl<'a, 'tcx> Visitor<'tcx> for InitializeVisitor<'a, 'tcx> { fn var_def_id(cx: &LateContext<'_, '_>, expr: &Expr) -> Option<HirId> { if let ExprKind::Path(ref qpath) = expr.node { - let path_res = cx.tables.qpath_res(qpath, expr.hir_id); + let path_res = qpath_res(cx, qpath, expr.hir_id); if let Res::Local(node_id) = path_res { return Some(node_id); } @@ -2355,7 +2355,7 @@ impl<'a, 'tcx> VarCollectorVisitor<'a, 'tcx> { if_chain! { if let ExprKind::Path(ref qpath) = ex.node; if let QPath::Resolved(None, _) = *qpath; - let res = self.cx.tables.qpath_res(qpath, ex.hir_id); + let res = qpath_res(self.cx, qpath, ex.hir_id); then { match res { Res::Local(node_id) => { diff --git a/clippy_lints/src/mem_forget.rs b/clippy_lints/src/mem_forget.rs index 54b0a61937e..9d457f453e6 100644 --- a/clippy_lints/src/mem_forget.rs +++ b/clippy_lints/src/mem_forget.rs @@ -1,4 +1,4 @@ -use crate::utils::{match_def_path, paths, span_lint}; +use crate::utils::{match_def_path, paths, qpath_res, span_lint}; use rustc::hir::{Expr, ExprKind}; use rustc::lint::{LateContext, LateLintPass, LintArray, LintPass}; use rustc::{declare_lint_pass, declare_tool_lint}; @@ -29,7 +29,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for MemForget { fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, e: &'tcx Expr) { if let ExprKind::Call(ref path_expr, ref args) = e.node { if let ExprKind::Path(ref qpath) = path_expr.node { - if let Some(def_id) = cx.tables.qpath_res(qpath, path_expr.hir_id).opt_def_id() { + if let Some(def_id) = qpath_res(cx, qpath, path_expr.hir_id).opt_def_id() { if match_def_path(cx, def_id, &paths::MEM_FORGET) { let forgot_ty = cx.tables.expr_ty(&args[0]); diff --git a/clippy_lints/src/mem_replace.rs b/clippy_lints/src/mem_replace.rs index 7e83e836b85..3e1155806b9 100644 --- a/clippy_lints/src/mem_replace.rs +++ b/clippy_lints/src/mem_replace.rs @@ -1,4 +1,6 @@ -use crate::utils::{match_def_path, match_qpath, paths, snippet_with_applicability, span_lint_and_sugg}; +use crate::utils::{ + match_def_path, match_qpath, paths, snippet_with_applicability, span_help_and_lint, span_lint_and_sugg, +}; use if_chain::if_chain; use rustc::hir::{Expr, ExprKind, MutMutable, QPath}; use rustc::lint::{LateContext, LateLintPass, LintArray, LintPass}; @@ -32,7 +34,40 @@ declare_clippy_lint! { "replacing an `Option` with `None` instead of `take()`" } -declare_lint_pass!(MemReplace => [MEM_REPLACE_OPTION_WITH_NONE]); +declare_clippy_lint! { + /// **What it does:** Checks for `mem::replace(&mut _, mem::uninitialized())` + /// and `mem::replace(&mut _, mem::zeroed())`. + /// + /// **Why is this bad?** This will lead to undefined behavior even if the + /// value is overwritten later, because the uninitialized value may be + /// observed in the case of a panic. + /// + /// **Known problems:** None. + /// + /// **Example:** + /// + /// ``` + /// use std::mem; + ///# fn may_panic(v: Vec<i32>) -> Vec<i32> { v } + /// + /// #[allow(deprecated, invalid_value)] + /// fn myfunc (v: &mut Vec<i32>) { + /// let taken_v = unsafe { mem::replace(v, mem::uninitialized()) }; + /// let new_v = may_panic(taken_v); // undefined behavior on panic + /// mem::forget(mem::replace(v, new_v)); + /// } + /// ``` + /// + /// The [take_mut](https://docs.rs/take_mut) crate offers a sound solution, + /// at the cost of either lazily creating a replacement value or aborting + /// on panic, to ensure that the uninitialized value cannot be observed. + pub MEM_REPLACE_WITH_UNINIT, + correctness, + "`mem::replace(&mut _, mem::uninitialized())` or `mem::replace(&mut _, mem::zeroed())`" +} + +declare_lint_pass!(MemReplace => + [MEM_REPLACE_OPTION_WITH_NONE, MEM_REPLACE_WITH_UNINIT]); impl<'a, 'tcx> LateLintPass<'a, 'tcx> for MemReplace { fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr) { @@ -45,37 +80,66 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for MemReplace { if match_def_path(cx, def_id, &paths::MEM_REPLACE); // Check that second argument is `Option::None` - if let ExprKind::Path(ref replacement_qpath) = func_args[1].node; - if match_qpath(replacement_qpath, &paths::OPTION_NONE); - then { - // Since this is a late pass (already type-checked), - // and we already know that the second argument is an - // `Option`, we do not need to check the first - // argument's type. All that's left is to get - // replacee's path. - let replaced_path = match func_args[0].node { - ExprKind::AddrOf(MutMutable, ref replaced) => { - if let ExprKind::Path(QPath::Resolved(None, ref replaced_path)) = replaced.node { - replaced_path - } else { - return - } - }, - ExprKind::Path(QPath::Resolved(None, ref replaced_path)) => replaced_path, - _ => return, - }; + if let ExprKind::Path(ref replacement_qpath) = func_args[1].node { + if match_qpath(replacement_qpath, &paths::OPTION_NONE) { - let mut applicability = Applicability::MachineApplicable; - span_lint_and_sugg( - cx, - MEM_REPLACE_OPTION_WITH_NONE, - expr.span, - "replacing an `Option` with `None`", - "consider `Option::take()` instead", - format!("{}.take()", snippet_with_applicability(cx, replaced_path.span, "", &mut applicability)), - applicability, - ); + // Since this is a late pass (already type-checked), + // and we already know that the second argument is an + // `Option`, we do not need to check the first + // argument's type. All that's left is to get + // replacee's path. + let replaced_path = match func_args[0].node { + ExprKind::AddrOf(MutMutable, ref replaced) => { + if let ExprKind::Path(QPath::Resolved(None, ref replaced_path)) = replaced.node { + replaced_path + } else { + return + } + }, + ExprKind::Path(QPath::Resolved(None, ref replaced_path)) => replaced_path, + _ => return, + }; + + let mut applicability = Applicability::MachineApplicable; + span_lint_and_sugg( + cx, + MEM_REPLACE_OPTION_WITH_NONE, + expr.span, + "replacing an `Option` with `None`", + "consider `Option::take()` instead", + format!("{}.take()", snippet_with_applicability(cx, replaced_path.span, "", &mut applicability)), + applicability, + ); + } + } + if let ExprKind::Call(ref repl_func, ref repl_args) = func_args[1].node { + if_chain! { + if repl_args.is_empty(); + if let ExprKind::Path(ref repl_func_qpath) = repl_func.node; + if let Some(repl_def_id) = cx.tables.qpath_res(repl_func_qpath, repl_func.hir_id).opt_def_id(); + then { + if match_def_path(cx, repl_def_id, &paths::MEM_UNINITIALIZED) { + span_help_and_lint( + cx, + MEM_REPLACE_WITH_UNINIT, + expr.span, + "replacing with `mem::uninitialized()`", + "consider using the `take_mut` crate instead", + ); + } else if match_def_path(cx, repl_def_id, &paths::MEM_ZEROED) && + !cx.tables.expr_ty(&func_args[1]).is_primitive() { + span_help_and_lint( + cx, + MEM_REPLACE_WITH_UNINIT, + expr.span, + "replacing with `mem::zeroed()`", + "consider using a default value or the `take_mut` crate instead", + ); + } + } + } + } } } } diff --git a/clippy_lints/src/no_effect.rs b/clippy_lints/src/no_effect.rs index 22129a3abb9..571e664d4d4 100644 --- a/clippy_lints/src/no_effect.rs +++ b/clippy_lints/src/no_effect.rs @@ -1,4 +1,4 @@ -use crate::utils::{has_drop, snippet_opt, span_lint, span_lint_and_sugg}; +use crate::utils::{has_drop, qpath_res, snippet_opt, span_lint, span_lint_and_sugg}; use rustc::hir::def::{DefKind, Res}; use rustc::hir::{BinOpKind, BlockCheckMode, Expr, ExprKind, Stmt, StmtKind, UnsafeSource}; use rustc::lint::{LateContext, LateLintPass, LintArray, LintPass}; @@ -67,7 +67,7 @@ fn has_no_effect(cx: &LateContext<'_, '_>, expr: &Expr) -> bool { }, ExprKind::Call(ref callee, ref args) => { if let ExprKind::Path(ref qpath) = callee.node { - let res = cx.tables.qpath_res(qpath, callee.hir_id); + let res = qpath_res(cx, qpath, callee.hir_id); match res { Res::Def(DefKind::Struct, ..) | Res::Def(DefKind::Variant, ..) | Res::Def(DefKind::Ctor(..), _) => { !has_drop(cx, cx.tables.expr_ty(expr)) && args.iter().all(|arg| has_no_effect(cx, arg)) @@ -145,7 +145,7 @@ fn reduce_expression<'a>(cx: &LateContext<'_, '_>, expr: &'a Expr) -> Option<Vec }, ExprKind::Call(ref callee, ref args) => { if let ExprKind::Path(ref qpath) = callee.node { - let res = cx.tables.qpath_res(qpath, callee.hir_id); + let res = qpath_res(cx, qpath, callee.hir_id); match res { Res::Def(DefKind::Struct, ..) | Res::Def(DefKind::Variant, ..) | Res::Def(DefKind::Ctor(..), _) if !has_drop(cx, cx.tables.expr_ty(expr)) => diff --git a/clippy_lints/src/non_copy_const.rs b/clippy_lints/src/non_copy_const.rs index 56922c73110..992baa05e78 100644 --- a/clippy_lints/src/non_copy_const.rs +++ b/clippy_lints/src/non_copy_const.rs @@ -14,7 +14,7 @@ use rustc_errors::Applicability; use rustc_typeck::hir_ty_to_ty; use syntax_pos::{InnerSpan, Span, DUMMY_SP}; -use crate::utils::{in_constant, is_copy, span_lint_and_then}; +use crate::utils::{in_constant, is_copy, qpath_res, span_lint_and_then}; declare_clippy_lint! { /// **What it does:** Checks for declaration of `const` items which is interior @@ -195,7 +195,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for NonCopyConst { } // Make sure it is a const item. - match cx.tables.qpath_res(qpath, expr.hir_id) { + match qpath_res(cx, qpath, expr.hir_id) { Res::Def(DefKind::Const, _) | Res::Def(DefKind::AssocConst, _) => {}, _ => return, }; diff --git a/clippy_lints/src/redundant_clone.rs b/clippy_lints/src/redundant_clone.rs index 68bf1febc6d..d029267e034 100644 --- a/clippy_lints/src/redundant_clone.rs +++ b/clippy_lints/src/redundant_clone.rs @@ -252,13 +252,13 @@ fn find_stmt_assigns_to<'a, 'tcx: 'a>( stmts .rev() .find_map(|stmt| { - if let mir::StatementKind::Assign( + if let mir::StatementKind::Assign(box ( mir::Place { base: mir::PlaceBase::Local(local), .. }, v, - ) = &stmt.kind + )) = &stmt.kind { if *local == to { return Some(v); @@ -269,10 +269,10 @@ fn find_stmt_assigns_to<'a, 'tcx: 'a>( }) .and_then(|v| { if by_ref { - if let mir::Rvalue::Ref(_, _, ref place) = **v { + if let mir::Rvalue::Ref(_, _, ref place) = v { return base_local_and_movability(cx, mir, place); } - } else if let mir::Rvalue::Use(mir::Operand::Copy(ref place)) = **v { + } else if let mir::Rvalue::Use(mir::Operand::Copy(ref place)) = v { return base_local_and_movability(cx, mir, place); } None @@ -291,7 +291,6 @@ fn base_local_and_movability<'tcx>( use rustc::mir::Place; use rustc::mir::PlaceBase; use rustc::mir::PlaceRef; - use rustc::mir::Projection; // Dereference. You cannot move things out from a borrowed value. let mut deref = false; @@ -303,7 +302,7 @@ fn base_local_and_movability<'tcx>( mut projection, } = place.as_ref(); if let PlaceBase::Local(local) = place_base { - while let Some(box Projection { base, elem }) = projection { + while let [base @ .., elem] = projection { projection = base; deref = matches!(elem, mir::ProjectionElem::Deref); field = !field diff --git a/clippy_lints/src/types.rs b/clippy_lints/src/types.rs index 2e4128ccca6..5696c2be12a 100644 --- a/clippy_lints/src/types.rs +++ b/clippy_lints/src/types.rs @@ -24,7 +24,7 @@ use crate::consts::{constant, Constant}; use crate::utils::paths; use crate::utils::{ clip, comparisons, differing_macro_contexts, higher, in_constant, int_bits, last_path_segment, match_def_path, - match_path, multispan_sugg, same_tys, sext, snippet, snippet_opt, snippet_with_applicability, + match_path, multispan_sugg, qpath_res, same_tys, sext, snippet, snippet_opt, snippet_with_applicability, snippet_with_macro_callsite, span_help_and_lint, span_lint, span_lint_and_sugg, span_lint_and_then, unsext, }; @@ -218,7 +218,7 @@ fn match_type_parameter(cx: &LateContext<'_, '_>, qpath: &QPath, path: &[&str]) _ => None, }); if let TyKind::Path(ref qpath) = ty.node; - if let Some(did) = cx.tables.qpath_res(qpath, ty.hir_id).opt_def_id(); + if let Some(did) = qpath_res(cx, qpath, ty.hir_id).opt_def_id(); if match_def_path(cx, did, path); then { return true; @@ -240,7 +240,7 @@ fn check_ty(cx: &LateContext<'_, '_>, hir_ty: &hir::Ty, is_local: bool) { match hir_ty.node { TyKind::Path(ref qpath) if !is_local => { let hir_id = hir_ty.hir_id; - let res = cx.tables.qpath_res(qpath, hir_id); + let res = qpath_res(cx, qpath, hir_id); if let Some(def_id) = res.opt_def_id() { if Some(def_id) == cx.tcx.lang_items().owned_box() { if match_type_parameter(cx, qpath, &paths::VEC) { @@ -263,7 +263,7 @@ fn check_ty(cx: &LateContext<'_, '_>, hir_ty: &hir::Ty, is_local: bool) { }); // ty is now _ at this point if let TyKind::Path(ref ty_qpath) = ty.node; - let res = cx.tables.qpath_res(ty_qpath, ty.hir_id); + let res = qpath_res(cx, ty_qpath, ty.hir_id); if let Some(def_id) = res.opt_def_id(); if Some(def_id) == cx.tcx.lang_items().owned_box(); // At this point, we know ty is Box<T>, now get T @@ -369,7 +369,7 @@ fn check_ty_rptr(cx: &LateContext<'_, '_>, hir_ty: &hir::Ty, is_local: bool, lt: match mut_ty.ty.node { TyKind::Path(ref qpath) => { let hir_id = mut_ty.ty.hir_id; - let def = cx.tables.qpath_res(qpath, hir_id); + let def = qpath_res(cx, qpath, hir_id); if_chain! { if let Some(def_id) = def.opt_def_id(); if Some(def_id) == cx.tcx.lang_items().owned_box(); @@ -765,7 +765,7 @@ declare_clippy_lint! { /// } /// ``` pub CAST_LOSSLESS, - complexity, + pedantic, "casts using `as` that are known to be lossless, e.g., `x as u64` where `x: u8`" } diff --git a/clippy_lints/src/utils/mod.rs b/clippy_lints/src/utils/mod.rs index 41b472b2764..9165f8d74d7 100644 --- a/clippy_lints/src/utils/mod.rs +++ b/clippy_lints/src/utils/mod.rs @@ -273,6 +273,19 @@ pub fn path_to_res(cx: &LateContext<'_, '_>, path: &[&str]) -> Option<(def::Res) } } +pub fn qpath_res(cx: &LateContext<'_, '_>, qpath: &hir::QPath, id: hir::HirId) -> Res { + match qpath { + hir::QPath::Resolved(_, path) => path.res, + hir::QPath::TypeRelative(..) => { + if cx.tcx.has_typeck_tables(id.owner_def_id()) { + cx.tcx.typeck_tables_of(id.owner_def_id()).qpath_res(qpath, id) + } else { + Res::Err + } + }, + } +} + /// Convenience function to get the `DefId` of a trait by path. /// It could be a trait or trait alias. pub fn get_trait_def_id(cx: &LateContext<'_, '_>, path: &[&str]) -> Option<DefId> { diff --git a/clippy_lints/src/utils/paths.rs b/clippy_lints/src/utils/paths.rs index 9b88a0d3089..cda3d2024d2 100644 --- a/clippy_lints/src/utils/paths.rs +++ b/clippy_lints/src/utils/paths.rs @@ -52,6 +52,8 @@ pub const MEM_FORGET: [&str; 3] = ["core", "mem", "forget"]; pub const MEM_MAYBEUNINIT: [&str; 4] = ["core", "mem", "maybe_uninit", "MaybeUninit"]; pub const MEM_MAYBEUNINIT_UNINIT: [&str; 5] = ["core", "mem", "maybe_uninit", "MaybeUninit", "uninit"]; pub const MEM_REPLACE: [&str; 3] = ["core", "mem", "replace"]; +pub const MEM_UNINITIALIZED: [&str; 3] = ["core", "mem", "uninitialized"]; +pub const MEM_ZEROED: [&str; 3] = ["core", "mem", "zeroed"]; pub const MUTEX: [&str; 4] = ["std", "sync", "mutex", "Mutex"]; pub const OPEN_OPTIONS: [&str; 3] = ["std", "fs", "OpenOptions"]; pub const OPS_MODULE: [&str; 2] = ["core", "ops"]; diff --git a/doc/adding_lints.md b/doc/adding_lints.md index 20810f0cab8..af6c3e29701 100644 --- a/doc/adding_lints.md +++ b/doc/adding_lints.md @@ -7,20 +7,22 @@ creating an example lint from scratch. To get started, we will create a lint that detects functions called `foo`, because that's clearly a non-descriptive name. -* [Setup](#Setup) -* [Testing](#Testing) -* [Rustfix tests](#Rustfix-tests) -* [Edition 2018 tests](#Edition-2018-tests) -* [Lint declaration](#Lint-declaration) -* [Lint passes](#Lint-passes) -* [Emitting a lint](#Emitting-a-lint) -* [Adding the lint logic](#Adding-the-lint-logic) -* [Author lint](#Author-lint) -* [Documentation](#Documentation) -* [Running rustfmt](#Running-rustfmt) -* [Debugging](#Debugging) -* [PR Checklist](#PR-Checklist) -* [Cheatsheet](#Cheatsheet) +- [Adding a new lint](#adding-a-new-lint) + - [Setup](#setup) + - [Testing](#testing) + - [Rustfix tests](#rustfix-tests) + - [Edition 2018 tests](#edition-2018-tests) + - [Testing manually](#testing-manually) + - [Lint declaration](#lint-declaration) + - [Lint passes](#lint-passes) + - [Emitting a lint](#emitting-a-lint) + - [Adding the lint logic](#adding-the-lint-logic) + - [Author lint](#author-lint) + - [Documentation](#documentation) + - [Running rustfmt](#running-rustfmt) + - [Debugging](#debugging) + - [PR Checklist](#pr-checklist) + - [Cheatsheet](#cheatsheet) ### Setup @@ -309,7 +311,7 @@ If you have trouble implementing your lint, there is also the internal `author` lint to generate Clippy code that detects the offending pattern. It does not work for all of the Rust syntax, but can give a good starting point. -The quickest way to use it, is the [Rust playground][play].rust-lang.org). +The quickest way to use it, is the [Rust playground: play.rust-lang.org][Play]. Put the code you want to lint into the editor and add the `#[clippy::author]` attribute above the item. Then run Clippy via `Tools -> Clippy` and you should see the generated code in the output below. diff --git a/src/driver.rs b/src/driver.rs index 92f83f1a29e..359d2f8530c 100644 --- a/src/driver.rs +++ b/src/driver.rs @@ -247,8 +247,9 @@ You can use tool lints to allow or deny lints from your code, eg.: pub fn main() { rustc_driver::init_rustc_env_logger(); + rustc_driver::install_ice_hook(); exit( - rustc_driver::report_ices_to_stderr_if_any(move || { + rustc_driver::catch_fatal_errors(move || { use std::env; if std::env::args().any(|a| a == "--version" || a == "-V") { diff --git a/src/lintlist/mod.rs b/src/lintlist/mod.rs index 589ae6b3d35..e6bcbfadf4f 100644 --- a/src/lintlist/mod.rs +++ b/src/lintlist/mod.rs @@ -6,7 +6,7 @@ pub use lint::Lint; pub use lint::LINT_LEVELS; // begin lint list, do not remove this comment, it’s used in `update_lints` -pub const ALL_LINTS: [Lint; 314] = [ +pub const ALL_LINTS: [Lint; 316] = [ Lint { name: "absurd_extreme_comparisons", group: "correctness", @@ -121,7 +121,7 @@ pub const ALL_LINTS: [Lint; 314] = [ }, Lint { name: "cast_lossless", - group: "complexity", + group: "pedantic", desc: "casts using `as` that are known to be lossless, e.g., `x as u64` where `x: u8`", deprecation: None, module: "types", @@ -1044,6 +1044,13 @@ pub const ALL_LINTS: [Lint; 314] = [ module: "mem_replace", }, Lint { + name: "mem_replace_with_uninit", + group: "correctness", + desc: "`mem::replace(&mut _, mem::uninitialized())` or `mem::replace(&mut _, mem::zeroed())`", + deprecation: None, + module: "mem_replace", + }, + Lint { name: "min_max", group: "correctness", desc: "`min(_, max(_, _))` (or vice versa) with bounds clamping the result to a constant", @@ -1079,6 +1086,13 @@ pub const ALL_LINTS: [Lint; 314] = [ module: "missing_inline", }, Lint { + name: "missing_safety_doc", + group: "style", + desc: "`pub unsafe fn` without `# Safety` docs", + deprecation: None, + module: "doc", + }, + Lint { name: "mistyped_literal_suffixes", group: "correctness", desc: "mistyped literal suffix", @@ -1977,7 +1991,7 @@ pub const ALL_LINTS: [Lint; 314] = [ Lint { name: "unneeded_wildcard_pattern", group: "complexity", - desc: "tuple patterns with a wildcard pattern (`_`) is next to a rest pattern (`..`) pattern", + desc: "tuple patterns with a wildcard pattern (`_`) is next to a rest pattern (`..`)", deprecation: None, module: "misc_early", }, diff --git a/tests/dogfood.rs b/tests/dogfood.rs index 1f03ba3950a..944f3c2c013 100644 --- a/tests/dogfood.rs +++ b/tests/dogfood.rs @@ -1,17 +1,19 @@ #[test] -fn dogfood() { +fn dogfood_clippy() { + // run clippy on itself and fail the test if lint warnings are reported if option_env!("RUSTC_TEST_SUITE").is_some() || cfg!(windows) { return; } let root_dir = std::path::PathBuf::from(env!("CARGO_MANIFEST_DIR")); - let clippy_cmd = std::path::Path::new(&root_dir) + let clippy_binary = std::path::Path::new(&root_dir) .join("target") .join(env!("PROFILE")) .join("cargo-clippy"); - let output = std::process::Command::new(clippy_cmd) + let output = std::process::Command::new(clippy_binary) .current_dir(root_dir) .env("CLIPPY_DOGFOOD", "1") + .env("CARGO_INCREMENTAL", "0") .arg("clippy-preview") .arg("--all-targets") .arg("--all-features") @@ -19,6 +21,7 @@ fn dogfood() { .args(&["-D", "clippy::all"]) .args(&["-D", "clippy::internal"]) .args(&["-D", "clippy::pedantic"]) + .arg("-Cdebuginfo=0") // disable debuginfo to generate less data in the target dir .output() .unwrap(); println!("status: {}", output.status); @@ -29,12 +32,13 @@ fn dogfood() { } #[test] -fn dogfood_tests() { +fn dogfood_subprojects() { + // run clippy on remaining subprojects and fail the test if lint warnings are reported if option_env!("RUSTC_TEST_SUITE").is_some() || cfg!(windows) { return; } let root_dir = std::path::PathBuf::from(env!("CARGO_MANIFEST_DIR")); - let clippy_cmd = std::path::Path::new(&root_dir) + let clippy_binary = std::path::Path::new(&root_dir) .join("target") .join(env!("PROFILE")) .join("cargo-clippy"); @@ -47,13 +51,15 @@ fn dogfood_tests() { "clippy_dev", "rustc_tools_util", ] { - let output = std::process::Command::new(&clippy_cmd) + let output = std::process::Command::new(&clippy_binary) .current_dir(root_dir.join(d)) .env("CLIPPY_DOGFOOD", "1") + .env("CARGO_INCREMENTAL", "0") .arg("clippy") .arg("--") .args(&["-D", "clippy::all"]) .args(&["-D", "clippy::pedantic"]) + .arg("-Cdebuginfo=0") // disable debuginfo to generate less data in the target dir .output() .unwrap(); println!("status: {}", output.status); diff --git a/tests/ui/deref_addrof_double_trigger.rs b/tests/ui/deref_addrof_double_trigger.rs index e19af5b9087..4531943299c 100644 --- a/tests/ui/deref_addrof_double_trigger.rs +++ b/tests/ui/deref_addrof_double_trigger.rs @@ -1,5 +1,7 @@ +// This test can't work with run-rustfix because it needs two passes of test+fix + #[warn(clippy::deref_addrof)] -#[allow(unused_variables)] +#[allow(unused_variables, unused_mut)] fn main() { let a = 10; diff --git a/tests/ui/deref_addrof_double_trigger.stderr b/tests/ui/deref_addrof_double_trigger.stderr index 89284e1a8ed..2c55a4ed6ac 100644 --- a/tests/ui/deref_addrof_double_trigger.stderr +++ b/tests/ui/deref_addrof_double_trigger.stderr @@ -1,5 +1,5 @@ error: immediately dereferencing a reference - --> $DIR/deref_addrof_double_trigger.rs:8:14 + --> $DIR/deref_addrof_double_trigger.rs:10:14 | LL | let b = **&&a; | ^^^^ help: try this: `&a` @@ -7,13 +7,13 @@ LL | let b = **&&a; = note: `-D clippy::deref-addrof` implied by `-D warnings` error: immediately dereferencing a reference - --> $DIR/deref_addrof_double_trigger.rs:12:17 + --> $DIR/deref_addrof_double_trigger.rs:14:17 | LL | let y = *&mut x; | ^^^^^^^ help: try this: `x` error: immediately dereferencing a reference - --> $DIR/deref_addrof_double_trigger.rs:19:18 + --> $DIR/deref_addrof_double_trigger.rs:21:18 | LL | let y = **&mut &mut x; | ^^^^^^^^^^^^ help: try this: `&mut x` diff --git a/tests/ui/doc_unsafe.rs b/tests/ui/doc_unsafe.rs new file mode 100644 index 00000000000..7b26e86b40b --- /dev/null +++ b/tests/ui/doc_unsafe.rs @@ -0,0 +1,25 @@ +/// This is not sufficiently documented +pub unsafe fn destroy_the_planet() { + unimplemented!(); +} + +/// This one is +/// +/// # Safety +/// +/// This function shouldn't be called unless the horsemen are ready +pub unsafe fn apocalypse(universe: &mut ()) { + unimplemented!(); +} + +/// This is a private function, so docs aren't necessary +unsafe fn you_dont_see_me() { + unimplemented!(); +} + +fn main() { + you_dont_see_me(); + destroy_the_planet(); + let mut universe = (); + apocalypse(&mut universe); +} diff --git a/tests/ui/doc_unsafe.stderr b/tests/ui/doc_unsafe.stderr new file mode 100644 index 00000000000..d6d1cd2d4fa --- /dev/null +++ b/tests/ui/doc_unsafe.stderr @@ -0,0 +1,12 @@ +error: unsafe function's docs miss `# Safety` section + --> $DIR/doc_unsafe.rs:2:1 + | +LL | / pub unsafe fn destroy_the_planet() { +LL | | unimplemented!(); +LL | | } + | |_^ + | + = note: `-D clippy::missing-safety-doc` implied by `-D warnings` + +error: aborting due to previous error + diff --git a/tests/ui/eq_op.rs b/tests/ui/eq_op.rs index cc0935ddb79..c93f520bee2 100644 --- a/tests/ui/eq_op.rs +++ b/tests/ui/eq_op.rs @@ -2,7 +2,8 @@ #[warn(clippy::eq_op)] #[allow(clippy::identity_op, clippy::double_parens, clippy::many_single_char_names)] #[allow(clippy::no_effect, unused_variables, clippy::unnecessary_operation, clippy::short_circuit_statement)] -#[warn(clippy::nonminimal_bool)] +#[allow(clippy::nonminimal_bool)] +#[allow(unused)] fn main() { // simple values and comparisons 1 == 1; @@ -50,42 +51,6 @@ fn main() { 2*a.len() == 2*a.len(); // ok, functions a.pop() == a.pop(); // ok, functions - use std::ops::BitAnd; - struct X(i32); - impl BitAnd for X { - type Output = X; - fn bitand(self, rhs: X) -> X { - X(self.0 & rhs.0) - } - } - impl<'a> BitAnd<&'a X> for X { - type Output = X; - fn bitand(self, rhs: &'a X) -> X { - X(self.0 & rhs.0) - } - } - let x = X(1); - let y = X(2); - let z = x & &y; - - #[derive(Copy, Clone)] - struct Y(i32); - impl BitAnd for Y { - type Output = Y; - fn bitand(self, rhs: Y) -> Y { - Y(self.0 & rhs.0) - } - } - impl<'a> BitAnd<&'a Y> for Y { - type Output = Y; - fn bitand(self, rhs: &'a Y) -> Y { - Y(self.0 & rhs.0) - } - } - let x = Y(1); - let y = Y(2); - let z = x & &y; - check_ignore_macro(); // named constants diff --git a/tests/ui/eq_op.stderr b/tests/ui/eq_op.stderr index 2dabaf0d4db..e37c0c22907 100644 --- a/tests/ui/eq_op.stderr +++ b/tests/ui/eq_op.stderr @@ -1,43 +1,5 @@ -error: this boolean expression can be simplified - --> $DIR/eq_op.rs:35:5 - | -LL | true && true; - | ^^^^^^^^^^^^ help: try: `true` - | - = note: `-D clippy::nonminimal-bool` implied by `-D warnings` - -error: this boolean expression can be simplified - --> $DIR/eq_op.rs:37:5 - | -LL | true || true; - | ^^^^^^^^^^^^ help: try: `true` - -error: this boolean expression can be simplified - --> $DIR/eq_op.rs:43:5 - | -LL | a == b && b == a; - | ^^^^^^^^^^^^^^^^ help: try: `a == b` - -error: this boolean expression can be simplified - --> $DIR/eq_op.rs:44:5 - | -LL | a != b && b != a; - | ^^^^^^^^^^^^^^^^ help: try: `a != b` - -error: this boolean expression can be simplified - --> $DIR/eq_op.rs:45:5 - | -LL | a < b && b > a; - | ^^^^^^^^^^^^^^ help: try: `a < b` - -error: this boolean expression can be simplified - --> $DIR/eq_op.rs:46:5 - | -LL | a <= b && b >= a; - | ^^^^^^^^^^^^^^^^ help: try: `a <= b` - error: equal expressions as operands to `==` - --> $DIR/eq_op.rs:8:5 + --> $DIR/eq_op.rs:9:5 | LL | 1 == 1; | ^^^^^^ @@ -45,170 +7,160 @@ LL | 1 == 1; = note: `-D clippy::eq-op` implied by `-D warnings` error: equal expressions as operands to `==` - --> $DIR/eq_op.rs:9:5 + --> $DIR/eq_op.rs:10:5 | LL | "no" == "no"; | ^^^^^^^^^^^^ error: equal expressions as operands to `!=` - --> $DIR/eq_op.rs:11:5 + --> $DIR/eq_op.rs:12:5 | LL | false != false; | ^^^^^^^^^^^^^^ error: equal expressions as operands to `<` - --> $DIR/eq_op.rs:12:5 + --> $DIR/eq_op.rs:13:5 | LL | 1.5 < 1.5; | ^^^^^^^^^ error: equal expressions as operands to `>=` - --> $DIR/eq_op.rs:13:5 + --> $DIR/eq_op.rs:14:5 | LL | 1u64 >= 1u64; | ^^^^^^^^^^^^ error: equal expressions as operands to `&` - --> $DIR/eq_op.rs:16:5 + --> $DIR/eq_op.rs:17:5 | LL | (1 as u64) & (1 as u64); | ^^^^^^^^^^^^^^^^^^^^^^^ error: equal expressions as operands to `^` - --> $DIR/eq_op.rs:17:5 + --> $DIR/eq_op.rs:18:5 | LL | 1 ^ ((((((1)))))); | ^^^^^^^^^^^^^^^^^ error: equal expressions as operands to `<` - --> $DIR/eq_op.rs:20:5 + --> $DIR/eq_op.rs:21:5 | LL | (-(2) < -(2)); | ^^^^^^^^^^^^^ error: equal expressions as operands to `==` - --> $DIR/eq_op.rs:21:5 + --> $DIR/eq_op.rs:22:5 | LL | ((1 + 1) & (1 + 1) == (1 + 1) & (1 + 1)); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: equal expressions as operands to `&` - --> $DIR/eq_op.rs:21:6 + --> $DIR/eq_op.rs:22:6 | LL | ((1 + 1) & (1 + 1) == (1 + 1) & (1 + 1)); | ^^^^^^^^^^^^^^^^^ error: equal expressions as operands to `&` - --> $DIR/eq_op.rs:21:27 + --> $DIR/eq_op.rs:22:27 | LL | ((1 + 1) & (1 + 1) == (1 + 1) & (1 + 1)); | ^^^^^^^^^^^^^^^^^ error: equal expressions as operands to `==` - --> $DIR/eq_op.rs:22:5 + --> $DIR/eq_op.rs:23:5 | LL | (1 * 2) + (3 * 4) == 1 * 2 + 3 * 4; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: equal expressions as operands to `!=` - --> $DIR/eq_op.rs:25:5 + --> $DIR/eq_op.rs:26:5 | LL | ([1] != [1]); | ^^^^^^^^^^^^ error: equal expressions as operands to `!=` - --> $DIR/eq_op.rs:26:5 + --> $DIR/eq_op.rs:27:5 | LL | ((1, 2) != (1, 2)); | ^^^^^^^^^^^^^^^^^^ error: equal expressions as operands to `==` - --> $DIR/eq_op.rs:30:5 + --> $DIR/eq_op.rs:31:5 | LL | 1 + 1 == 2; | ^^^^^^^^^^ error: equal expressions as operands to `==` - --> $DIR/eq_op.rs:31:5 + --> $DIR/eq_op.rs:32:5 | LL | 1 - 1 == 0; | ^^^^^^^^^^ error: equal expressions as operands to `-` - --> $DIR/eq_op.rs:31:5 + --> $DIR/eq_op.rs:32:5 | LL | 1 - 1 == 0; | ^^^^^ error: equal expressions as operands to `-` - --> $DIR/eq_op.rs:33:5 + --> $DIR/eq_op.rs:34:5 | LL | 1 - 1; | ^^^^^ error: equal expressions as operands to `/` - --> $DIR/eq_op.rs:34:5 + --> $DIR/eq_op.rs:35:5 | LL | 1 / 1; | ^^^^^ error: equal expressions as operands to `&&` - --> $DIR/eq_op.rs:35:5 + --> $DIR/eq_op.rs:36:5 | LL | true && true; | ^^^^^^^^^^^^ error: equal expressions as operands to `||` - --> $DIR/eq_op.rs:37:5 + --> $DIR/eq_op.rs:38:5 | LL | true || true; | ^^^^^^^^^^^^ error: equal expressions as operands to `&&` - --> $DIR/eq_op.rs:43:5 + --> $DIR/eq_op.rs:44:5 | LL | a == b && b == a; | ^^^^^^^^^^^^^^^^ error: equal expressions as operands to `&&` - --> $DIR/eq_op.rs:44:5 + --> $DIR/eq_op.rs:45:5 | LL | a != b && b != a; | ^^^^^^^^^^^^^^^^ error: equal expressions as operands to `&&` - --> $DIR/eq_op.rs:45:5 + --> $DIR/eq_op.rs:46:5 | LL | a < b && b > a; | ^^^^^^^^^^^^^^ error: equal expressions as operands to `&&` - --> $DIR/eq_op.rs:46:5 + --> $DIR/eq_op.rs:47:5 | LL | a <= b && b >= a; | ^^^^^^^^^^^^^^^^ error: equal expressions as operands to `==` - --> $DIR/eq_op.rs:49:5 + --> $DIR/eq_op.rs:50:5 | LL | a == a; | ^^^^^^ -error: taken reference of right operand - --> $DIR/eq_op.rs:87:13 - | -LL | let z = x & &y; - | ^^^^-- - | | - | help: use the right value directly: `y` - | - = note: `-D clippy::op-ref` implied by `-D warnings` - error: equal expressions as operands to `/` - --> $DIR/eq_op.rs:95:20 + --> $DIR/eq_op.rs:60:20 | LL | const D: u32 = A / A; | ^^^^^ -error: aborting due to 34 previous errors +error: aborting due to 27 previous errors diff --git a/tests/ui/explicit_write_non_rustfix.rs b/tests/ui/explicit_write_non_rustfix.rs new file mode 100644 index 00000000000..f21e8ef935b --- /dev/null +++ b/tests/ui/explicit_write_non_rustfix.rs @@ -0,0 +1,8 @@ +#![allow(unused_imports, clippy::blacklisted_name)] +#![warn(clippy::explicit_write)] + +fn main() { + use std::io::Write; + let bar = "bar"; + writeln!(std::io::stderr(), "foo {}", bar).unwrap(); +} diff --git a/tests/ui/explicit_write_non_rustfix.stderr b/tests/ui/explicit_write_non_rustfix.stderr new file mode 100644 index 00000000000..77cadb99bb5 --- /dev/null +++ b/tests/ui/explicit_write_non_rustfix.stderr @@ -0,0 +1,10 @@ +error: use of `writeln!(stderr(), ...).unwrap()`. Consider using `eprintln!` instead + --> $DIR/explicit_write_non_rustfix.rs:7:5 + | +LL | writeln!(std::io::stderr(), "foo {}", bar).unwrap(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: `-D clippy::explicit-write` implied by `-D warnings` + +error: aborting due to previous error + diff --git a/tests/ui/for_loop.stdout b/tests/ui/for_loop.stdout deleted file mode 100644 index e69de29bb2d..00000000000 --- a/tests/ui/for_loop.stdout +++ /dev/null diff --git a/tests/ui/for_loop_fixable.fixed b/tests/ui/for_loop_fixable.fixed new file mode 100644 index 00000000000..3075638ef94 --- /dev/null +++ b/tests/ui/for_loop_fixable.fixed @@ -0,0 +1,304 @@ +// run-rustfix + +#![allow(dead_code, unused)] + +use std::collections::*; + +#[warn(clippy::all)] +struct Unrelated(Vec<u8>); +impl Unrelated { + fn next(&self) -> std::slice::Iter<u8> { + self.0.iter() + } + + fn iter(&self) -> std::slice::Iter<u8> { + self.0.iter() + } +} + +#[warn( + clippy::needless_range_loop, + clippy::explicit_iter_loop, + clippy::explicit_into_iter_loop, + clippy::iter_next_loop, + clippy::reverse_range_loop, + clippy::for_kv_map +)] +#[allow( + clippy::linkedlist, + clippy::shadow_unrelated, + clippy::unnecessary_mut_passed, + clippy::cognitive_complexity, + clippy::similar_names +)] +#[allow(clippy::many_single_char_names, unused_variables, clippy::into_iter_on_array)] +fn main() { + const MAX_LEN: usize = 42; + let mut vec = vec![1, 2, 3, 4]; + + for i in (0..10).rev() { + println!("{}", i); + } + + for i in (0..=10).rev() { + println!("{}", i); + } + + for i in (0..MAX_LEN).rev() { + println!("{}", i); + } + + for i in 5..=5 { + // not an error, this is the range with only one element “5” + println!("{}", i); + } + + for i in 0..10 { + // not an error, the start index is less than the end index + println!("{}", i); + } + + for i in -10..0 { + // not an error + println!("{}", i); + } + + for i in (10..0).map(|x| x * 2) { + // not an error, it can't be known what arbitrary methods do to a range + println!("{}", i); + } + + // testing that the empty range lint folds constants + for i in (5 + 4..10).rev() { + println!("{}", i); + } + + for i in ((3 - 1)..(5 + 2)).rev() { + println!("{}", i); + } + + for i in (2 * 2)..(2 * 3) { + // no error, 4..6 is fine + println!("{}", i); + } + + let x = 42; + for i in x..10 { + // no error, not constant-foldable + println!("{}", i); + } + + // See #601 + for i in 0..10 { + // no error, id_col does not exist outside the loop + let mut id_col = vec![0f64; 10]; + id_col[i] = 1f64; + } + + for _v in &vec {} + + for _v in &mut vec {} + + let out_vec = vec![1, 2, 3]; + for _v in out_vec {} + + let array = [1, 2, 3]; + for _v in &array {} + + for _v in &vec {} // these are fine + for _v in &mut vec {} // these are fine + + for _v in &[1, 2, 3] {} + + for _v in (&mut [1, 2, 3]).iter() {} // no error + + for _v in &[0; 32] {} + + for _v in [0; 33].iter() {} // no error + + let ll: LinkedList<()> = LinkedList::new(); + for _v in &ll {} + + let vd: VecDeque<()> = VecDeque::new(); + for _v in &vd {} + + let bh: BinaryHeap<()> = BinaryHeap::new(); + for _v in &bh {} + + let hm: HashMap<(), ()> = HashMap::new(); + for _v in &hm {} + + let bt: BTreeMap<(), ()> = BTreeMap::new(); + for _v in &bt {} + + let hs: HashSet<()> = HashSet::new(); + for _v in &hs {} + + let bs: BTreeSet<()> = BTreeSet::new(); + for _v in &bs {} + + let u = Unrelated(vec![]); + for _v in u.next() {} // no error + for _v in u.iter() {} // no error + + let mut out = vec![]; + vec.iter().cloned().map(|x| out.push(x)).collect::<Vec<_>>(); + let _y = vec.iter().cloned().map(|x| out.push(x)).collect::<Vec<_>>(); // this is fine + + // Loop with explicit counter variable + + // Potential false positives + let mut _index = 0; + _index = 1; + for _v in &vec { + _index += 1 + } + + let mut _index = 0; + _index += 1; + for _v in &vec { + _index += 1 + } + + let mut _index = 0; + if true { + _index = 1 + } + for _v in &vec { + _index += 1 + } + + let mut _index = 0; + let mut _index = 1; + for _v in &vec { + _index += 1 + } + + let mut _index = 0; + for _v in &vec { + _index += 1; + _index += 1 + } + + let mut _index = 0; + for _v in &vec { + _index *= 2; + _index += 1 + } + + let mut _index = 0; + for _v in &vec { + _index = 1; + _index += 1 + } + + let mut _index = 0; + + for _v in &vec { + let mut _index = 0; + _index += 1 + } + + let mut _index = 0; + for _v in &vec { + _index += 1; + _index = 0; + } + + let mut _index = 0; + for _v in &vec { + for _x in 0..1 { + _index += 1; + } + _index += 1 + } + + let mut _index = 0; + for x in &vec { + if *x == 1 { + _index += 1 + } + } + + let mut _index = 0; + if true { + _index = 1 + }; + for _v in &vec { + _index += 1 + } + + let mut _index = 1; + if false { + _index = 0 + }; + for _v in &vec { + _index += 1 + } + + let mut index = 0; + { + let mut _x = &mut index; + } + for _v in &vec { + _index += 1 + } + + let mut index = 0; + for _v in &vec { + index += 1 + } + println!("index: {}", index); + + fn f<T>(_: &T, _: &T) -> bool { + unimplemented!() + } + fn g<T>(_: &mut [T], _: usize, _: usize) { + unimplemented!() + } + for i in 1..vec.len() { + if f(&vec[i - 1], &vec[i]) { + g(&mut vec, i - 1, i); + } + } + + for mid in 1..vec.len() { + let (_, _) = vec.split_at(mid); + } +} + +fn partition<T: PartialOrd + Send>(v: &mut [T]) -> usize { + let pivot = v.len() - 1; + let mut i = 0; + for j in 0..pivot { + if v[j] <= v[pivot] { + v.swap(i, j); + i += 1; + } + } + v.swap(i, pivot); + i +} + +#[warn(clippy::needless_range_loop)] +pub fn manual_copy_same_destination(dst: &mut [i32], d: usize, s: usize) { + // Same source and destination - don't trigger lint + for i in 0..dst.len() { + dst[d + i] = dst[s + i]; + } +} + +mod issue_2496 { + pub trait Handle { + fn new_for_index(index: usize) -> Self; + fn index(&self) -> usize; + } + + pub fn test<H: Handle>() -> H { + for x in 0..5 { + let next_handle = H::new_for_index(x); + println!("{}", next_handle.index()); + } + unimplemented!() + } +} diff --git a/tests/ui/for_loop.rs b/tests/ui/for_loop_fixable.rs index 5d367a62fc9..2201596fd6a 100644 --- a/tests/ui/for_loop.rs +++ b/tests/ui/for_loop_fixable.rs @@ -1,8 +1,8 @@ -use std::collections::*; -use std::rc::Rc; +// run-rustfix + +#![allow(dead_code, unused)] -static STATIC: [usize; 4] = [0, 1, 8, 16]; -const CONST: [usize; 4] = [0, 1, 8, 16]; +use std::collections::*; #[warn(clippy::all)] struct Unrelated(Vec<u8>); @@ -48,10 +48,6 @@ fn main() { println!("{}", i); } - for i in 5..5 { - println!("{}", i); - } - for i in 5..=5 { // not an error, this is the range with only one element “5” println!("{}", i); @@ -81,10 +77,6 @@ fn main() { println!("{}", i); } - for i in (5 + 2)..(8 - 1) { - println!("{}", i); - } - for i in (2 * 2)..(2 * 3) { // no error, 4..6 is fine println!("{}", i); @@ -145,8 +137,6 @@ fn main() { let bs: BTreeSet<()> = BTreeSet::new(); for _v in bs.iter() {} - for _v in vec.iter().next() {} - let u = Unrelated(vec![]); for _v in u.next() {} // no error for _v in u.iter() {} // no error @@ -275,17 +265,8 @@ fn main() { for mid in 1..vec.len() { let (_, _) = vec.split_at(mid); } - - const ZERO: usize = 0; - - for i in ZERO..vec.len() { - if f(&vec[i], &vec[i]) { - panic!("at the disco"); - } - } } -#[allow(dead_code)] fn partition<T: PartialOrd + Send>(v: &mut [T]) -> usize { let pivot = v.len() - 1; let mut i = 0; diff --git a/tests/ui/for_loop.stderr b/tests/ui/for_loop_fixable.stderr index 0f84abf45ed..6d6fa3ac7af 100644 --- a/tests/ui/for_loop.stderr +++ b/tests/ui/for_loop_fixable.stderr @@ -1,5 +1,5 @@ error: this range is empty so this for loop will never run - --> $DIR/for_loop.rs:39:14 + --> $DIR/for_loop_fixable.rs:39:14 | LL | for i in 10..0 { | ^^^^^ @@ -11,17 +11,17 @@ LL | for i in (0..10).rev() { | ^^^^^^^^^^^^^ error: this range is empty so this for loop will never run - --> $DIR/for_loop.rs:43:14 + --> $DIR/for_loop_fixable.rs:43:14 | LL | for i in 10..=0 { | ^^^^^^ help: consider using the following if you are attempting to iterate over this range in reverse | -LL | for i in (0...10).rev() { +LL | for i in (0..=10).rev() { | ^^^^^^^^^^^^^^ error: this range is empty so this for loop will never run - --> $DIR/for_loop.rs:47:14 + --> $DIR/for_loop_fixable.rs:47:14 | LL | for i in MAX_LEN..0 { | ^^^^^^^^^^ @@ -31,13 +31,7 @@ LL | for i in (0..MAX_LEN).rev() { | ^^^^^^^^^^^^^^^^^^ error: this range is empty so this for loop will never run - --> $DIR/for_loop.rs:51:14 - | -LL | for i in 5..5 { - | ^^^^ - -error: this range is empty so this for loop will never run - --> $DIR/for_loop.rs:76:14 + --> $DIR/for_loop_fixable.rs:72:14 | LL | for i in 10..5 + 4 { | ^^^^^^^^^ @@ -47,7 +41,7 @@ LL | for i in (5 + 4..10).rev() { | ^^^^^^^^^^^^^^^^^ error: this range is empty so this for loop will never run - --> $DIR/for_loop.rs:80:14 + --> $DIR/for_loop_fixable.rs:76:14 | LL | for i in (5 + 2)..(3 - 1) { | ^^^^^^^^^^^^^^^^ @@ -56,14 +50,8 @@ help: consider using the following if you are attempting to iterate over this ra LL | for i in ((3 - 1)..(5 + 2)).rev() { | ^^^^^^^^^^^^^^^^^^^^^^^^ -error: this range is empty so this for loop will never run - --> $DIR/for_loop.rs:84:14 - | -LL | for i in (5 + 2)..(8 - 1) { - | ^^^^^^^^^^^^^^^^ - error: it is more concise to loop over references to containers instead of using explicit iteration methods - --> $DIR/for_loop.rs:106:15 + --> $DIR/for_loop_fixable.rs:98:15 | LL | for _v in vec.iter() {} | ^^^^^^^^^^ help: to write this more concisely, try: `&vec` @@ -71,13 +59,13 @@ LL | for _v in vec.iter() {} = note: `-D clippy::explicit-iter-loop` implied by `-D warnings` error: it is more concise to loop over references to containers instead of using explicit iteration methods - --> $DIR/for_loop.rs:108:15 + --> $DIR/for_loop_fixable.rs:100:15 | LL | for _v in vec.iter_mut() {} | ^^^^^^^^^^^^^^ help: to write this more concisely, try: `&mut vec` error: it is more concise to loop over containers instead of using explicit iteration methods` - --> $DIR/for_loop.rs:111:15 + --> $DIR/for_loop_fixable.rs:103:15 | LL | for _v in out_vec.into_iter() {} | ^^^^^^^^^^^^^^^^^^^ help: to write this more concisely, try: `out_vec` @@ -85,84 +73,64 @@ LL | for _v in out_vec.into_iter() {} = note: `-D clippy::explicit-into-iter-loop` implied by `-D warnings` error: it is more concise to loop over references to containers instead of using explicit iteration methods - --> $DIR/for_loop.rs:114:15 + --> $DIR/for_loop_fixable.rs:106:15 | LL | for _v in array.into_iter() {} | ^^^^^^^^^^^^^^^^^ help: to write this more concisely, try: `&array` error: it is more concise to loop over references to containers instead of using explicit iteration methods - --> $DIR/for_loop.rs:119:15 + --> $DIR/for_loop_fixable.rs:111:15 | LL | for _v in [1, 2, 3].iter() {} | ^^^^^^^^^^^^^^^^ help: to write this more concisely, try: `&[1, 2, 3]` error: it is more concise to loop over references to containers instead of using explicit iteration methods - --> $DIR/for_loop.rs:123:15 + --> $DIR/for_loop_fixable.rs:115:15 | LL | for _v in [0; 32].iter() {} | ^^^^^^^^^^^^^^ help: to write this more concisely, try: `&[0; 32]` error: it is more concise to loop over references to containers instead of using explicit iteration methods - --> $DIR/for_loop.rs:128:15 + --> $DIR/for_loop_fixable.rs:120:15 | LL | for _v in ll.iter() {} | ^^^^^^^^^ help: to write this more concisely, try: `&ll` error: it is more concise to loop over references to containers instead of using explicit iteration methods - --> $DIR/for_loop.rs:131:15 + --> $DIR/for_loop_fixable.rs:123:15 | LL | for _v in vd.iter() {} | ^^^^^^^^^ help: to write this more concisely, try: `&vd` error: it is more concise to loop over references to containers instead of using explicit iteration methods - --> $DIR/for_loop.rs:134:15 + --> $DIR/for_loop_fixable.rs:126:15 | LL | for _v in bh.iter() {} | ^^^^^^^^^ help: to write this more concisely, try: `&bh` error: it is more concise to loop over references to containers instead of using explicit iteration methods - --> $DIR/for_loop.rs:137:15 + --> $DIR/for_loop_fixable.rs:129:15 | LL | for _v in hm.iter() {} | ^^^^^^^^^ help: to write this more concisely, try: `&hm` error: it is more concise to loop over references to containers instead of using explicit iteration methods - --> $DIR/for_loop.rs:140:15 + --> $DIR/for_loop_fixable.rs:132:15 | LL | for _v in bt.iter() {} | ^^^^^^^^^ help: to write this more concisely, try: `&bt` error: it is more concise to loop over references to containers instead of using explicit iteration methods - --> $DIR/for_loop.rs:143:15 + --> $DIR/for_loop_fixable.rs:135:15 | LL | for _v in hs.iter() {} | ^^^^^^^^^ help: to write this more concisely, try: `&hs` error: it is more concise to loop over references to containers instead of using explicit iteration methods - --> $DIR/for_loop.rs:146:15 + --> $DIR/for_loop_fixable.rs:138:15 | LL | for _v in bs.iter() {} | ^^^^^^^^^ help: to write this more concisely, try: `&bs` -error: you are iterating over `Iterator::next()` which is an Option; this will compile but is probably not what you want - --> $DIR/for_loop.rs:148:15 - | -LL | for _v in vec.iter().next() {} - | ^^^^^^^^^^^^^^^^^ - | - = note: `-D clippy::iter-next-loop` implied by `-D warnings` - -error: the loop variable `i` is only used to index `vec`. - --> $DIR/for_loop.rs:281:14 - | -LL | for i in ZERO..vec.len() { - | ^^^^^^^^^^^^^^^ - | - = note: `-D clippy::needless-range-loop` implied by `-D warnings` -help: consider using an iterator - | -LL | for <item> in &vec { - | ^^^^^^ ^^^^ - -error: aborting due to 22 previous errors +error: aborting due to 18 previous errors diff --git a/tests/ui/for_loop_unfixable.rs b/tests/ui/for_loop_unfixable.rs new file mode 100644 index 00000000000..5d94647e0db --- /dev/null +++ b/tests/ui/for_loop_unfixable.rs @@ -0,0 +1,41 @@ +// Tests from for_loop.rs that don't have suggestions + +#[warn( + clippy::needless_range_loop, + clippy::explicit_iter_loop, + clippy::explicit_into_iter_loop, + clippy::iter_next_loop, + clippy::reverse_range_loop, + clippy::for_kv_map +)] +#[allow( + clippy::linkedlist, + clippy::shadow_unrelated, + clippy::unnecessary_mut_passed, + clippy::cognitive_complexity, + clippy::similar_names, + unused, + dead_code +)] +#[allow(clippy::many_single_char_names, unused_variables, clippy::into_iter_on_array)] +fn main() { + for i in 5..5 { + println!("{}", i); + } + + let vec = vec![1, 2, 3, 4]; + + for _v in vec.iter().next() {} + + for i in (5 + 2)..(8 - 1) { + println!("{}", i); + } + + const ZERO: usize = 0; + + for i in ZERO..vec.len() { + if f(&vec[i], &vec[i]) { + panic!("at the disco"); + } + } +} diff --git a/tests/ui/for_loop_unfixable.stderr b/tests/ui/for_loop_unfixable.stderr new file mode 100644 index 00000000000..e88bfffaae6 --- /dev/null +++ b/tests/ui/for_loop_unfixable.stderr @@ -0,0 +1,9 @@ +error[E0425]: cannot find function `f` in this scope + --> $DIR/for_loop_unfixable.rs:37:12 + | +LL | if f(&vec[i], &vec[i]) { + | ^ help: a local variable with a similar name exists: `i` + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0425`. diff --git a/tests/ui/ice-4545.rs b/tests/ui/ice-4545.rs new file mode 100644 index 00000000000..d9c9c2096d9 --- /dev/null +++ b/tests/ui/ice-4545.rs @@ -0,0 +1,14 @@ +fn repro() { + trait Foo { + type Bar; + } + + #[allow(dead_code)] + struct Baz<T: Foo> { + field: T::Bar, + } +} + +fn main() { + repro(); +} diff --git a/tests/ui/identity_conversion.fixed b/tests/ui/identity_conversion.fixed new file mode 100644 index 00000000000..dd3fc56e98b --- /dev/null +++ b/tests/ui/identity_conversion.fixed @@ -0,0 +1,58 @@ +// run-rustfix + +#![deny(clippy::identity_conversion)] + +fn test_generic<T: Copy>(val: T) -> T { + let _ = val; + val +} + +fn test_generic2<T: Copy + Into<i32> + Into<U>, U: From<T>>(val: T) { + // ok + let _: i32 = val.into(); + let _: U = val.into(); + let _ = U::from(val); +} + +fn test_questionmark() -> Result<(), ()> { + { + let _: i32 = 0i32; + Ok(Ok(())) + }??; + Ok(()) +} + +fn test_issue_3913() -> Result<(), std::io::Error> { + use std::fs; + use std::path::Path; + + let path = Path::new("."); + for _ in fs::read_dir(path)? {} + + Ok(()) +} + +fn main() { + test_generic(10i32); + test_generic2::<i32, i32>(10i32); + test_questionmark().unwrap(); + test_issue_3913().unwrap(); + + let _: String = "foo".into(); + let _: String = From::from("foo"); + let _ = String::from("foo"); + #[allow(clippy::identity_conversion)] + { + let _: String = "foo".into(); + let _ = String::from("foo"); + let _ = "".lines().into_iter(); + } + + let _: String = "foo".to_string(); + let _: String = "foo".to_string(); + let _ = "foo".to_string(); + let _ = format!("A: {:04}", 123); + let _ = "".lines(); + let _ = vec![1, 2, 3].into_iter(); + let _: String = format!("Hello {}", "world"); +} diff --git a/tests/ui/identity_conversion.rs b/tests/ui/identity_conversion.rs index 164f0a3d6e7..875ed7db373 100644 --- a/tests/ui/identity_conversion.rs +++ b/tests/ui/identity_conversion.rs @@ -1,3 +1,5 @@ +// run-rustfix + #![deny(clippy::identity_conversion)] fn test_generic<T: Copy>(val: T) -> T { diff --git a/tests/ui/identity_conversion.stderr b/tests/ui/identity_conversion.stderr index 663f00d2279..3cabe53bf2b 100644 --- a/tests/ui/identity_conversion.stderr +++ b/tests/ui/identity_conversion.stderr @@ -1,65 +1,65 @@ error: identical conversion - --> $DIR/identity_conversion.rs:4:13 + --> $DIR/identity_conversion.rs:6:13 | LL | let _ = T::from(val); | ^^^^^^^^^^^^ help: consider removing `T::from()`: `val` | note: lint level defined here - --> $DIR/identity_conversion.rs:1:9 + --> $DIR/identity_conversion.rs:3:9 | LL | #![deny(clippy::identity_conversion)] | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: identical conversion - --> $DIR/identity_conversion.rs:5:5 + --> $DIR/identity_conversion.rs:7:5 | LL | val.into() | ^^^^^^^^^^ help: consider removing `.into()`: `val` error: identical conversion - --> $DIR/identity_conversion.rs:17:22 + --> $DIR/identity_conversion.rs:19:22 | LL | let _: i32 = 0i32.into(); | ^^^^^^^^^^^ help: consider removing `.into()`: `0i32` error: identical conversion - --> $DIR/identity_conversion.rs:49:21 + --> $DIR/identity_conversion.rs:51:21 | LL | let _: String = "foo".to_string().into(); | ^^^^^^^^^^^^^^^^^^^^^^^^ help: consider removing `.into()`: `"foo".to_string()` error: identical conversion - --> $DIR/identity_conversion.rs:50:21 + --> $DIR/identity_conversion.rs:52:21 | LL | let _: String = From::from("foo".to_string()); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider removing `From::from()`: `"foo".to_string()` error: identical conversion - --> $DIR/identity_conversion.rs:51:13 + --> $DIR/identity_conversion.rs:53:13 | LL | let _ = String::from("foo".to_string()); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider removing `String::from()`: `"foo".to_string()` error: identical conversion - --> $DIR/identity_conversion.rs:52:13 + --> $DIR/identity_conversion.rs:54:13 | LL | let _ = String::from(format!("A: {:04}", 123)); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider removing `String::from()`: `format!("A: {:04}", 123)` error: identical conversion - --> $DIR/identity_conversion.rs:53:13 + --> $DIR/identity_conversion.rs:55:13 | LL | let _ = "".lines().into_iter(); | ^^^^^^^^^^^^^^^^^^^^^^ help: consider removing `.into_iter()`: `"".lines()` error: identical conversion - --> $DIR/identity_conversion.rs:54:13 + --> $DIR/identity_conversion.rs:56:13 | LL | let _ = vec![1, 2, 3].into_iter().into_iter(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider removing `.into_iter()`: `vec![1, 2, 3].into_iter()` error: identical conversion - --> $DIR/identity_conversion.rs:55:21 + --> $DIR/identity_conversion.rs:57:21 | LL | let _: String = format!("Hello {}", "world").into(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider removing `.into()`: `format!("Hello {}", "world")` diff --git a/tests/ui/implicit_return.fixed b/tests/ui/implicit_return.fixed new file mode 100644 index 00000000000..dd42f06664e --- /dev/null +++ b/tests/ui/implicit_return.fixed @@ -0,0 +1,102 @@ +// run-rustfix + +#![warn(clippy::implicit_return)] +#![allow(clippy::needless_return, unused)] + +fn test_end_of_fn() -> bool { + if true { + // no error! + return true; + } + + return true +} + +#[allow(clippy::needless_bool)] +fn test_if_block() -> bool { + if true { + return true + } else { + return false + } +} + +#[allow(clippy::match_bool)] +#[rustfmt::skip] +fn test_match(x: bool) -> bool { + match x { + true => return false, + false => { return true }, + } +} + +#[allow(clippy::match_bool, clippy::needless_return)] +fn test_match_with_unreachable(x: bool) -> bool { + match x { + true => return false, + false => unreachable!(), + } +} + +#[allow(clippy::never_loop)] +fn test_loop() -> bool { + loop { + return true; + } +} + +#[allow(clippy::never_loop)] +fn test_loop_with_block() -> bool { + loop { + { + return true; + } + } +} + +#[allow(clippy::never_loop)] +fn test_loop_with_nests() -> bool { + loop { + if true { + return true; + } else { + let _ = true; + } + } +} + +#[allow(clippy::redundant_pattern_matching)] +fn test_loop_with_if_let() -> bool { + loop { + if let Some(x) = Some(true) { + return x; + } + } +} + +fn test_closure() { + #[rustfmt::skip] + let _ = || { return true }; + let _ = || return true; +} + +fn test_panic() -> bool { + panic!() +} + +fn test_return_macro() -> String { + return format!("test {}", "test") +} + +fn main() { + let _ = test_end_of_fn(); + let _ = test_if_block(); + let _ = test_match(true); + let _ = test_match_with_unreachable(true); + let _ = test_loop(); + let _ = test_loop_with_block(); + let _ = test_loop_with_nests(); + let _ = test_loop_with_if_let(); + test_closure(); + let _ = test_return_macro(); +} diff --git a/tests/ui/implicit_return.rs b/tests/ui/implicit_return.rs index 47e0679c430..5abbf6a5583 100644 --- a/tests/ui/implicit_return.rs +++ b/tests/ui/implicit_return.rs @@ -1,4 +1,7 @@ +// run-rustfix + #![warn(clippy::implicit_return)] +#![allow(clippy::needless_return, unused)] fn test_end_of_fn() -> bool { if true { diff --git a/tests/ui/implicit_return.stderr b/tests/ui/implicit_return.stderr index 41b0873317e..21822344437 100644 --- a/tests/ui/implicit_return.stderr +++ b/tests/ui/implicit_return.stderr @@ -1,5 +1,5 @@ error: missing return statement - --> $DIR/implicit_return.rs:9:5 + --> $DIR/implicit_return.rs:12:5 | LL | true | ^^^^ help: add `return` as shown: `return true` @@ -7,61 +7,61 @@ LL | true = note: `-D clippy::implicit-return` implied by `-D warnings` error: missing return statement - --> $DIR/implicit_return.rs:15:9 + --> $DIR/implicit_return.rs:18:9 | LL | true | ^^^^ help: add `return` as shown: `return true` error: missing return statement - --> $DIR/implicit_return.rs:17:9 + --> $DIR/implicit_return.rs:20:9 | LL | false | ^^^^^ help: add `return` as shown: `return false` error: missing return statement - --> $DIR/implicit_return.rs:25:17 + --> $DIR/implicit_return.rs:28:17 | LL | true => false, | ^^^^^ help: add `return` as shown: `return false` error: missing return statement - --> $DIR/implicit_return.rs:26:20 + --> $DIR/implicit_return.rs:29:20 | LL | false => { true }, | ^^^^ help: add `return` as shown: `return true` error: missing return statement - --> $DIR/implicit_return.rs:41:9 + --> $DIR/implicit_return.rs:44:9 | LL | break true; | ^^^^^^^^^^ help: change `break` to `return` as shown: `return true` error: missing return statement - --> $DIR/implicit_return.rs:49:13 + --> $DIR/implicit_return.rs:52:13 | LL | break true; | ^^^^^^^^^^ help: change `break` to `return` as shown: `return true` error: missing return statement - --> $DIR/implicit_return.rs:58:13 + --> $DIR/implicit_return.rs:61:13 | LL | break true; | ^^^^^^^^^^ help: change `break` to `return` as shown: `return true` error: missing return statement - --> $DIR/implicit_return.rs:76:18 + --> $DIR/implicit_return.rs:79:18 | LL | let _ = || { true }; | ^^^^ help: add `return` as shown: `return true` error: missing return statement - --> $DIR/implicit_return.rs:77:16 + --> $DIR/implicit_return.rs:80:16 | LL | let _ = || true; | ^^^^ help: add `return` as shown: `return true` error: missing return statement - --> $DIR/implicit_return.rs:85:5 + --> $DIR/implicit_return.rs:88:5 | LL | format!("test {}", "test") | ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: add `return` as shown: `return format!("test {}", "test")` diff --git a/tests/ui/inline_fn_without_body.fixed b/tests/ui/inline_fn_without_body.fixed new file mode 100644 index 00000000000..fe21a71a42c --- /dev/null +++ b/tests/ui/inline_fn_without_body.fixed @@ -0,0 +1,17 @@ +// run-rustfix + +#![warn(clippy::inline_fn_without_body)] +#![allow(clippy::inline_always)] + +trait Foo { + fn default_inline(); + + fn always_inline(); + + fn never_inline(); + + #[inline] + fn has_body() {} +} + +fn main() {} diff --git a/tests/ui/inline_fn_without_body.rs b/tests/ui/inline_fn_without_body.rs index af81feaa374..50746989466 100644 --- a/tests/ui/inline_fn_without_body.rs +++ b/tests/ui/inline_fn_without_body.rs @@ -1,3 +1,5 @@ +// run-rustfix + #![warn(clippy::inline_fn_without_body)] #![allow(clippy::inline_always)] diff --git a/tests/ui/inline_fn_without_body.stderr b/tests/ui/inline_fn_without_body.stderr index 87d2da71280..32d35e209b0 100644 --- a/tests/ui/inline_fn_without_body.stderr +++ b/tests/ui/inline_fn_without_body.stderr @@ -1,5 +1,5 @@ error: use of `#[inline]` on trait method `default_inline` which has no body - --> $DIR/inline_fn_without_body.rs:5:5 + --> $DIR/inline_fn_without_body.rs:7:5 | LL | #[inline] | _____-^^^^^^^^ @@ -9,7 +9,7 @@ LL | | fn default_inline(); = note: `-D clippy::inline-fn-without-body` implied by `-D warnings` error: use of `#[inline]` on trait method `always_inline` which has no body - --> $DIR/inline_fn_without_body.rs:8:5 + --> $DIR/inline_fn_without_body.rs:10:5 | LL | #[inline(always)] | _____-^^^^^^^^^^^^^^^^ @@ -17,7 +17,7 @@ LL | | fn always_inline(); | |____- help: remove error: use of `#[inline]` on trait method `never_inline` which has no body - --> $DIR/inline_fn_without_body.rs:11:5 + --> $DIR/inline_fn_without_body.rs:13:5 | LL | #[inline(never)] | _____-^^^^^^^^^^^^^^^ diff --git a/tests/ui/non_copy_const.rs b/tests/ui/non_copy_const.rs index 46cbb3fee35..5a62957cdb4 100644 --- a/tests/ui/non_copy_const.rs +++ b/tests/ui/non_copy_const.rs @@ -1,4 +1,3 @@ -#![feature(const_string_new, const_vec_new)] #![allow(clippy::ref_in_deref, dead_code)] use std::borrow::Cow; diff --git a/tests/ui/non_copy_const.stderr b/tests/ui/non_copy_const.stderr index 634933eac61..2f325f9e3df 100644 --- a/tests/ui/non_copy_const.stderr +++ b/tests/ui/non_copy_const.stderr @@ -1,5 +1,5 @@ error: a const item should never be interior mutable - --> $DIR/non_copy_const.rs:10:1 + --> $DIR/non_copy_const.rs:9:1 | LL | const ATOMIC: AtomicUsize = AtomicUsize::new(5); //~ ERROR interior mutable | -----^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -9,7 +9,7 @@ LL | const ATOMIC: AtomicUsize = AtomicUsize::new(5); //~ ERROR interior mutable = note: `#[deny(clippy::declare_interior_mutable_const)]` on by default error: a const item should never be interior mutable - --> $DIR/non_copy_const.rs:11:1 + --> $DIR/non_copy_const.rs:10:1 | LL | const CELL: Cell<usize> = Cell::new(6); //~ ERROR interior mutable | -----^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -17,7 +17,7 @@ LL | const CELL: Cell<usize> = Cell::new(6); //~ ERROR interior mutable | help: make this a static item: `static` error: a const item should never be interior mutable - --> $DIR/non_copy_const.rs:12:1 + --> $DIR/non_copy_const.rs:11:1 | LL | const ATOMIC_TUPLE: ([AtomicUsize; 1], Vec<AtomicUsize>, u8) = ([ATOMIC], Vec::new(), 7); | -----^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -25,7 +25,7 @@ LL | const ATOMIC_TUPLE: ([AtomicUsize; 1], Vec<AtomicUsize>, u8) = ([ATOMIC], V | help: make this a static item: `static` error: a const item should never be interior mutable - --> $DIR/non_copy_const.rs:17:9 + --> $DIR/non_copy_const.rs:16:9 | LL | const $name: $ty = $e; | ^^^^^^^^^^^^^^^^^^^^^^ @@ -34,49 +34,49 @@ LL | declare_const!(_ONCE: Once = Once::new()); //~ ERROR interior mutable | ------------------------------------------ in this macro invocation error: a const item should never be interior mutable - --> $DIR/non_copy_const.rs:41:5 + --> $DIR/non_copy_const.rs:40:5 | LL | const ATOMIC: AtomicUsize; //~ ERROR interior mutable | ^^^^^^^^^^^^^^^^^^^^^^^^^^ error: a const item should never be interior mutable - --> $DIR/non_copy_const.rs:45:5 + --> $DIR/non_copy_const.rs:44:5 | LL | const INPUT: T; | ^^^^^^^^^^^^^^^ | help: consider requiring `T` to be `Copy` - --> $DIR/non_copy_const.rs:45:18 + --> $DIR/non_copy_const.rs:44:18 | LL | const INPUT: T; | ^ error: a const item should never be interior mutable - --> $DIR/non_copy_const.rs:48:5 + --> $DIR/non_copy_const.rs:47:5 | LL | const ASSOC: Self::NonCopyType; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | help: consider requiring `<Self as Trait<T>>::NonCopyType` to be `Copy` - --> $DIR/non_copy_const.rs:48:18 + --> $DIR/non_copy_const.rs:47:18 | LL | const ASSOC: Self::NonCopyType; | ^^^^^^^^^^^^^^^^^ error: a const item should never be interior mutable - --> $DIR/non_copy_const.rs:52:5 + --> $DIR/non_copy_const.rs:51:5 | LL | const AN_INPUT: T = Self::INPUT; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | help: consider requiring `T` to be `Copy` - --> $DIR/non_copy_const.rs:52:21 + --> $DIR/non_copy_const.rs:51:21 | LL | const AN_INPUT: T = Self::INPUT; | ^ error: a const item should never be interior mutable - --> $DIR/non_copy_const.rs:17:9 + --> $DIR/non_copy_const.rs:16:9 | LL | const $name: $ty = $e; | ^^^^^^^^^^^^^^^^^^^^^^ @@ -85,49 +85,49 @@ LL | declare_const!(ANOTHER_INPUT: T = Self::INPUT); //~ ERROR interior muta | ----------------------------------------------- in this macro invocation error: a const item should never be interior mutable - --> $DIR/non_copy_const.rs:61:5 + --> $DIR/non_copy_const.rs:60:5 | LL | const SELF_2: Self; | ^^^^^^^^^^^^^^^^^^^ | help: consider requiring `Self` to be `Copy` - --> $DIR/non_copy_const.rs:61:19 + --> $DIR/non_copy_const.rs:60:19 | LL | const SELF_2: Self; | ^^^^ error: a const item should never be interior mutable - --> $DIR/non_copy_const.rs:82:5 + --> $DIR/non_copy_const.rs:81:5 | LL | const ASSOC_3: AtomicUsize = AtomicUsize::new(14); //~ ERROR interior mutable | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: a const item should never be interior mutable - --> $DIR/non_copy_const.rs:85:5 + --> $DIR/non_copy_const.rs:84:5 | LL | const U_SELF: U = U::SELF_2; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | help: consider requiring `U` to be `Copy` - --> $DIR/non_copy_const.rs:85:19 + --> $DIR/non_copy_const.rs:84:19 | LL | const U_SELF: U = U::SELF_2; | ^ error: a const item should never be interior mutable - --> $DIR/non_copy_const.rs:88:5 + --> $DIR/non_copy_const.rs:87:5 | LL | const T_ASSOC: T::NonCopyType = T::ASSOC; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | help: consider requiring `<T as Trait<u32>>::NonCopyType` to be `Copy` - --> $DIR/non_copy_const.rs:88:20 + --> $DIR/non_copy_const.rs:87:20 | LL | const T_ASSOC: T::NonCopyType = T::ASSOC; | ^^^^^^^^^^^^^^ error: a const item with interior mutability should not be borrowed - --> $DIR/non_copy_const.rs:95:5 + --> $DIR/non_copy_const.rs:94:5 | LL | ATOMIC.store(1, Ordering::SeqCst); //~ ERROR interior mutability | ^^^^^^ @@ -136,7 +136,7 @@ LL | ATOMIC.store(1, Ordering::SeqCst); //~ ERROR interior mutability = help: assign this const to a local or static variable, and use the variable here error: a const item with interior mutability should not be borrowed - --> $DIR/non_copy_const.rs:96:16 + --> $DIR/non_copy_const.rs:95:16 | LL | assert_eq!(ATOMIC.load(Ordering::SeqCst), 5); //~ ERROR interior mutability | ^^^^^^ @@ -144,7 +144,7 @@ LL | assert_eq!(ATOMIC.load(Ordering::SeqCst), 5); //~ ERROR interior mutabi = help: assign this const to a local or static variable, and use the variable here error: a const item with interior mutability should not be borrowed - --> $DIR/non_copy_const.rs:99:22 + --> $DIR/non_copy_const.rs:98:22 | LL | let _once_ref = &ONCE_INIT; //~ ERROR interior mutability | ^^^^^^^^^ @@ -152,7 +152,7 @@ LL | let _once_ref = &ONCE_INIT; //~ ERROR interior mutability = help: assign this const to a local or static variable, and use the variable here error: a const item with interior mutability should not be borrowed - --> $DIR/non_copy_const.rs:100:25 + --> $DIR/non_copy_const.rs:99:25 | LL | let _once_ref_2 = &&ONCE_INIT; //~ ERROR interior mutability | ^^^^^^^^^ @@ -160,7 +160,7 @@ LL | let _once_ref_2 = &&ONCE_INIT; //~ ERROR interior mutability = help: assign this const to a local or static variable, and use the variable here error: a const item with interior mutability should not be borrowed - --> $DIR/non_copy_const.rs:101:27 + --> $DIR/non_copy_const.rs:100:27 | LL | let _once_ref_4 = &&&&ONCE_INIT; //~ ERROR interior mutability | ^^^^^^^^^ @@ -168,7 +168,7 @@ LL | let _once_ref_4 = &&&&ONCE_INIT; //~ ERROR interior mutability = help: assign this const to a local or static variable, and use the variable here error: a const item with interior mutability should not be borrowed - --> $DIR/non_copy_const.rs:102:26 + --> $DIR/non_copy_const.rs:101:26 | LL | let _once_mut = &mut ONCE_INIT; //~ ERROR interior mutability | ^^^^^^^^^ @@ -176,7 +176,7 @@ LL | let _once_mut = &mut ONCE_INIT; //~ ERROR interior mutability = help: assign this const to a local or static variable, and use the variable here error: a const item with interior mutability should not be borrowed - --> $DIR/non_copy_const.rs:113:14 + --> $DIR/non_copy_const.rs:112:14 | LL | let _ = &ATOMIC_TUPLE; //~ ERROR interior mutability | ^^^^^^^^^^^^ @@ -184,7 +184,7 @@ LL | let _ = &ATOMIC_TUPLE; //~ ERROR interior mutability = help: assign this const to a local or static variable, and use the variable here error: a const item with interior mutability should not be borrowed - --> $DIR/non_copy_const.rs:114:14 + --> $DIR/non_copy_const.rs:113:14 | LL | let _ = &ATOMIC_TUPLE.0; //~ ERROR interior mutability | ^^^^^^^^^^^^ @@ -192,7 +192,7 @@ LL | let _ = &ATOMIC_TUPLE.0; //~ ERROR interior mutability = help: assign this const to a local or static variable, and use the variable here error: a const item with interior mutability should not be borrowed - --> $DIR/non_copy_const.rs:115:19 + --> $DIR/non_copy_const.rs:114:19 | LL | let _ = &(&&&&ATOMIC_TUPLE).0; //~ ERROR interior mutability | ^^^^^^^^^^^^ @@ -200,7 +200,7 @@ LL | let _ = &(&&&&ATOMIC_TUPLE).0; //~ ERROR interior mutability = help: assign this const to a local or static variable, and use the variable here error: a const item with interior mutability should not be borrowed - --> $DIR/non_copy_const.rs:116:14 + --> $DIR/non_copy_const.rs:115:14 | LL | let _ = &ATOMIC_TUPLE.0[0]; //~ ERROR interior mutability | ^^^^^^^^^^^^ @@ -208,7 +208,7 @@ LL | let _ = &ATOMIC_TUPLE.0[0]; //~ ERROR interior mutability = help: assign this const to a local or static variable, and use the variable here error: a const item with interior mutability should not be borrowed - --> $DIR/non_copy_const.rs:117:13 + --> $DIR/non_copy_const.rs:116:13 | LL | let _ = ATOMIC_TUPLE.0[0].load(Ordering::SeqCst); //~ ERROR interior mutability | ^^^^^^^^^^^^ @@ -216,7 +216,7 @@ LL | let _ = ATOMIC_TUPLE.0[0].load(Ordering::SeqCst); //~ ERROR interior mu = help: assign this const to a local or static variable, and use the variable here error: a const item with interior mutability should not be borrowed - --> $DIR/non_copy_const.rs:123:13 + --> $DIR/non_copy_const.rs:122:13 | LL | let _ = ATOMIC_TUPLE.0[0]; //~ ERROR interior mutability | ^^^^^^^^^^^^ @@ -224,7 +224,7 @@ LL | let _ = ATOMIC_TUPLE.0[0]; //~ ERROR interior mutability = help: assign this const to a local or static variable, and use the variable here error: a const item with interior mutability should not be borrowed - --> $DIR/non_copy_const.rs:128:5 + --> $DIR/non_copy_const.rs:127:5 | LL | CELL.set(2); //~ ERROR interior mutability | ^^^^ @@ -232,7 +232,7 @@ LL | CELL.set(2); //~ ERROR interior mutability = help: assign this const to a local or static variable, and use the variable here error: a const item with interior mutability should not be borrowed - --> $DIR/non_copy_const.rs:129:16 + --> $DIR/non_copy_const.rs:128:16 | LL | assert_eq!(CELL.get(), 6); //~ ERROR interior mutability | ^^^^ @@ -240,7 +240,7 @@ LL | assert_eq!(CELL.get(), 6); //~ ERROR interior mutability = help: assign this const to a local or static variable, and use the variable here error: a const item with interior mutability should not be borrowed - --> $DIR/non_copy_const.rs:142:5 + --> $DIR/non_copy_const.rs:141:5 | LL | u64::ATOMIC.store(5, Ordering::SeqCst); //~ ERROR interior mutability | ^^^^^^^^^^^ @@ -248,7 +248,7 @@ LL | u64::ATOMIC.store(5, Ordering::SeqCst); //~ ERROR interior mutability = help: assign this const to a local or static variable, and use the variable here error: a const item with interior mutability should not be borrowed - --> $DIR/non_copy_const.rs:143:16 + --> $DIR/non_copy_const.rs:142:16 | LL | assert_eq!(u64::ATOMIC.load(Ordering::SeqCst), 9); //~ ERROR interior mutability | ^^^^^^^^^^^ diff --git a/tests/ui/op_ref.rs b/tests/ui/op_ref.rs index bf43deca12c..6605c967c8e 100644 --- a/tests/ui/op_ref.rs +++ b/tests/ui/op_ref.rs @@ -1,6 +1,8 @@ #![allow(unused_variables, clippy::blacklisted_name)] - +#![warn(clippy::op_ref)] +#![allow(clippy::many_single_char_names)] use std::collections::HashSet; +use std::ops::BitAnd; fn main() { let tracked_fds: HashSet<i32> = HashSet::new(); @@ -18,4 +20,39 @@ fn main() { if b < &a { println!("OK"); } + + struct X(i32); + impl BitAnd for X { + type Output = X; + fn bitand(self, rhs: X) -> X { + X(self.0 & rhs.0) + } + } + impl<'a> BitAnd<&'a X> for X { + type Output = X; + fn bitand(self, rhs: &'a X) -> X { + X(self.0 & rhs.0) + } + } + let x = X(1); + let y = X(2); + let z = x & &y; + + #[derive(Copy, Clone)] + struct Y(i32); + impl BitAnd for Y { + type Output = Y; + fn bitand(self, rhs: Y) -> Y { + Y(self.0 & rhs.0) + } + } + impl<'a> BitAnd<&'a Y> for Y { + type Output = Y; + fn bitand(self, rhs: &'a Y) -> Y { + Y(self.0 & rhs.0) + } + } + let x = Y(1); + let y = Y(2); + let z = x & &y; } diff --git a/tests/ui/op_ref.stderr b/tests/ui/op_ref.stderr index f5c5b970261..0f6a45f905c 100644 --- a/tests/ui/op_ref.stderr +++ b/tests/ui/op_ref.stderr @@ -1,5 +1,5 @@ error: needlessly taken reference of both operands - --> $DIR/op_ref.rs:10:15 + --> $DIR/op_ref.rs:12:15 | LL | let foo = &5 - &6; | ^^^^^^^ @@ -11,12 +11,20 @@ LL | let foo = 5 - 6; | ^ ^ error: taken reference of right operand - --> $DIR/op_ref.rs:18:8 + --> $DIR/op_ref.rs:20:8 | LL | if b < &a { | ^^^^-- | | | help: use the right value directly: `a` -error: aborting due to 2 previous errors +error: taken reference of right operand + --> $DIR/op_ref.rs:57:13 + | +LL | let z = x & &y; + | ^^^^-- + | | + | help: use the right value directly: `y` + +error: aborting due to 3 previous errors diff --git a/tests/ui/repl_uninit.rs b/tests/ui/repl_uninit.rs new file mode 100644 index 00000000000..346972b7bb4 --- /dev/null +++ b/tests/ui/repl_uninit.rs @@ -0,0 +1,35 @@ +#![allow(deprecated, invalid_value)] +#![warn(clippy::all)] + +use std::mem; + +fn might_panic<X>(x: X) -> X { + // in practice this would be a possibly-panicky operation + x +} + +fn main() { + let mut v = vec![0i32; 4]; + // the following is UB if `might_panic` panics + unsafe { + let taken_v = mem::replace(&mut v, mem::uninitialized()); + 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); + std::mem::forget(mem::replace(&mut v, new_v)); + } + + // this is silly but OK, because usize is a primitive type + let mut u: usize = 42; + let uref = &mut u; + let taken_u = unsafe { mem::replace(uref, mem::zeroed()) }; + *uref = taken_u + 1; + + // this is still not OK, because uninit + let taken_u = unsafe { mem::replace(uref, mem::uninitialized()) }; + *uref = taken_u + 1; +} diff --git a/tests/ui/repl_uninit.stderr b/tests/ui/repl_uninit.stderr new file mode 100644 index 00000000000..c1f55d7601e --- /dev/null +++ b/tests/ui/repl_uninit.stderr @@ -0,0 +1,27 @@ +error: replacing with `mem::uninitialized()` + --> $DIR/repl_uninit.rs:15:23 + | +LL | let taken_v = mem::replace(&mut v, mem::uninitialized()); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: `-D clippy::mem-replace-with-uninit` implied by `-D warnings` + = help: consider using the `take_mut` crate instead + +error: replacing with `mem::zeroed()` + --> $DIR/repl_uninit.rs:21: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 + | +LL | let taken_u = unsafe { mem::replace(uref, mem::uninitialized()) }; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = help: consider using the `take_mut` crate instead + +error: aborting due to 3 previous errors + diff --git a/tests/ui/types.fixed b/tests/ui/types.fixed index a71a9ec8124..b1622e45f3b 100644 --- a/tests/ui/types.fixed +++ b/tests/ui/types.fixed @@ -1,6 +1,7 @@ // run-rustfix #![allow(dead_code, unused_variables)] +#![warn(clippy::all, clippy::pedantic)] // should not warn on lossy casting in constant types // because not supported yet diff --git a/tests/ui/types.rs b/tests/ui/types.rs index 6f48080cedd..30463f9e2a1 100644 --- a/tests/ui/types.rs +++ b/tests/ui/types.rs @@ -1,6 +1,7 @@ // run-rustfix #![allow(dead_code, unused_variables)] +#![warn(clippy::all, clippy::pedantic)] // should not warn on lossy casting in constant types // because not supported yet diff --git a/tests/ui/types.stderr b/tests/ui/types.stderr index 3b4f57a7a43..daba766856d 100644 --- a/tests/ui/types.stderr +++ b/tests/ui/types.stderr @@ -1,5 +1,5 @@ error: casting i32 to i64 may become silently lossy if you later change the type - --> $DIR/types.rs:13:22 + --> $DIR/types.rs:14:22 | LL | let c_i64: i64 = c as i64; | ^^^^^^^^ help: try: `i64::from(c)` diff --git a/tests/ui/use_self.fixed b/tests/ui/use_self.fixed index 73666f2faae..90133788526 100644 --- a/tests/ui/use_self.fixed +++ b/tests/ui/use_self.fixed @@ -313,3 +313,22 @@ mod rustfix { } } } + +mod issue3567 { + struct TestStruct {} + impl TestStruct { + fn from_something() -> Self { + Self {} + } + } + + trait Test { + fn test() -> TestStruct; + } + + impl Test for TestStruct { + fn test() -> TestStruct { + Self::from_something() + } + } +} diff --git a/tests/ui/use_self.rs b/tests/ui/use_self.rs index 2e2b4f8b9d8..e6900b91534 100644 --- a/tests/ui/use_self.rs +++ b/tests/ui/use_self.rs @@ -313,3 +313,22 @@ mod rustfix { } } } + +mod issue3567 { + struct TestStruct {} + impl TestStruct { + fn from_something() -> Self { + Self {} + } + } + + trait Test { + fn test() -> TestStruct; + } + + impl Test for TestStruct { + fn test() -> TestStruct { + TestStruct::from_something() + } + } +} diff --git a/tests/ui/use_self.stderr b/tests/ui/use_self.stderr index 699e735137f..d1bfb0e230d 100644 --- a/tests/ui/use_self.stderr +++ b/tests/ui/use_self.stderr @@ -216,5 +216,11 @@ error: unnecessary structure name repetition LL | nested::A {}; | ^^^^^^^^^ help: use the applicable keyword: `Self` -error: aborting due to 35 previous errors +error: unnecessary structure name repetition + --> $DIR/use_self.rs:331:13 + | +LL | TestStruct::from_something() + | ^^^^^^^^^^ help: use the applicable keyword: `Self` + +error: aborting due to 36 previous errors diff --git a/tests/ui/wildcard_enum_match_arm.fixed b/tests/ui/wildcard_enum_match_arm.fixed new file mode 100644 index 00000000000..01c861282dc --- /dev/null +++ b/tests/ui/wildcard_enum_match_arm.fixed @@ -0,0 +1,65 @@ +// run-rustfix + +#![deny(clippy::wildcard_enum_match_arm)] +#![allow(unreachable_code, unused_variables)] + +#[derive(Clone, Copy, Debug, Eq, PartialEq)] +enum Color { + Red, + Green, + Blue, + Rgb(u8, u8, u8), + Cyan, +} + +impl Color { + fn is_monochrome(self) -> bool { + match self { + Color::Red | Color::Green | Color::Blue => true, + Color::Rgb(r, g, b) => r | g == 0 || r | b == 0 || g | b == 0, + Color::Cyan => false, + } + } +} + +fn main() { + let color = Color::Rgb(0, 0, 127); + match color { + Color::Red => println!("Red"), + Color::Green | Color::Blue | Color::Rgb(..) | Color::Cyan => eprintln!("Not red"), + }; + match color { + Color::Red => println!("Red"), + _not_red @ Color::Green | _not_red @ Color::Blue | _not_red @ Color::Rgb(..) | _not_red @ Color::Cyan => eprintln!("Not red"), + }; + let _str = match color { + Color::Red => "Red".to_owned(), + not_red @ Color::Green | not_red @ Color::Blue | not_red @ Color::Rgb(..) | not_red @ Color::Cyan => format!("{:?}", not_red), + }; + match color { + Color::Red => {}, + Color::Green => {}, + Color::Blue => {}, + Color::Cyan => {}, + c if c.is_monochrome() => {}, + Color::Rgb(_, _, _) => {}, + }; + let _str = match color { + Color::Red => "Red", + c @ Color::Green | c @ Color::Blue | c @ Color::Rgb(_, _, _) | c @ Color::Cyan => "Not red", + }; + match color { + Color::Rgb(r, _, _) if r > 0 => "Some red", + Color::Red | Color::Green | Color::Blue | Color::Rgb(..) | Color::Cyan => "No red", + }; + match color { + Color::Red | Color::Green | Color::Blue | Color::Cyan => {}, + Color::Rgb(..) => {}, + }; + let x: u8 = unimplemented!(); + match x { + 0 => {}, + 140 => {}, + _ => {}, + }; +} diff --git a/tests/ui/wildcard_enum_match_arm.rs b/tests/ui/wildcard_enum_match_arm.rs index 94d69d3c8a4..d33c68a6c7d 100644 --- a/tests/ui/wildcard_enum_match_arm.rs +++ b/tests/ui/wildcard_enum_match_arm.rs @@ -1,4 +1,7 @@ +// run-rustfix + #![deny(clippy::wildcard_enum_match_arm)] +#![allow(unreachable_code, unused_variables)] #[derive(Clone, Copy, Debug, Eq, PartialEq)] enum Color { diff --git a/tests/ui/wildcard_enum_match_arm.stderr b/tests/ui/wildcard_enum_match_arm.stderr index 999c1693301..df90bad15ce 100644 --- a/tests/ui/wildcard_enum_match_arm.stderr +++ b/tests/ui/wildcard_enum_match_arm.stderr @@ -1,29 +1,29 @@ error: wildcard match will miss any future added variants. - --> $DIR/wildcard_enum_match_arm.rs:26:9 + --> $DIR/wildcard_enum_match_arm.rs:29:9 | LL | _ => eprintln!("Not red"), | ^ help: try this: `Color::Green | Color::Blue | Color::Rgb(..) | Color::Cyan` | note: lint level defined here - --> $DIR/wildcard_enum_match_arm.rs:1:9 + --> $DIR/wildcard_enum_match_arm.rs:3:9 | LL | #![deny(clippy::wildcard_enum_match_arm)] | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: wildcard match will miss any future added variants. - --> $DIR/wildcard_enum_match_arm.rs:30:9 + --> $DIR/wildcard_enum_match_arm.rs:33:9 | LL | _not_red => eprintln!("Not red"), | ^^^^^^^^ help: try this: `_not_red @ Color::Green | _not_red @ Color::Blue | _not_red @ Color::Rgb(..) | _not_red @ Color::Cyan` error: wildcard match will miss any future added variants. - --> $DIR/wildcard_enum_match_arm.rs:34:9 + --> $DIR/wildcard_enum_match_arm.rs:37:9 | LL | not_red => format!("{:?}", not_red), | ^^^^^^^ help: try this: `not_red @ Color::Green | not_red @ Color::Blue | not_red @ Color::Rgb(..) | not_red @ Color::Cyan` error: wildcard match will miss any future added variants. - --> $DIR/wildcard_enum_match_arm.rs:50:9 + --> $DIR/wildcard_enum_match_arm.rs:53:9 | LL | _ => "No red", | ^ help: try this: `Color::Red | Color::Green | Color::Blue | Color::Rgb(..) | Color::Cyan` |