diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 2a85197068f5..bf681a49b98d 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -145,13 +145,28 @@ using that version of Rust. You can use [rustup-toolchain-install-master][rtim] to do that: -``` +```bash cargo install rustup-toolchain-install-master rustup-toolchain-install-master -n master --force rustup override set master cargo test ``` +After fixing the build failure on this repository, we can submit a pull request +to [`rust-lang/rust`] to fix the toolstate. + +To submit a pull request, you should follow these steps: + +```bash +# Assuming you already cloned the rust-lang/rust repo and you're in the correct directory +git submodule update --remote src/tools/clippy +cargo update -p clippy +git add -u +git commit -m "Update Clippy" +./x.py test -i --stage 1 src/tools/clippy # This is optional and should succeed anyway +# Open a PR in rust-lang/rust +``` + ## Issue and PR triage Clippy is following the [Rust triage procedure][triage] for issues and pull @@ -211,3 +226,4 @@ or the [MIT](http://opensource.org/licenses/MIT) license. [homu]: https://github.com/servo/homu [homu_instructions]: https://buildbot2.rust-lang.org/homu/ [homu_queue]: https://buildbot2.rust-lang.org/homu/queue/clippy +[`rust-lang/rust`]: https://github.com/rust-lang/rust diff --git a/clippy_lints/src/bytecount.rs b/clippy_lints/src/bytecount.rs index ab9cf9512617..1e1e4317e34e 100644 --- a/clippy_lints/src/bytecount.rs +++ b/clippy_lints/src/bytecount.rs @@ -47,8 +47,8 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for ByteCount { then { let body = cx.tcx.hir().body(body_id); if_chain! { - if body.arguments.len() == 1; - if let Some(argname) = get_pat_name(&body.arguments[0].pat); + if body.params.len() == 1; + if let Some(argname) = get_pat_name(&body.params[0].pat); if let ExprKind::Binary(ref op, ref l, ref r) = body.value.node; if op.node == BinOpKind::Eq; if match_type(cx, diff --git a/clippy_lints/src/escape.rs b/clippy_lints/src/escape.rs index c37efbd6e49d..e174cdfb8603 100644 --- a/clippy_lints/src/escape.rs +++ b/clippy_lints/src/escape.rs @@ -108,7 +108,7 @@ fn is_argument(map: &hir::map::Map<'_>, id: HirId) -> bool { } match map.find(map.get_parent_node(id)) { - Some(Node::Arg(_)) => true, + Some(Node::Param(_)) => true, _ => false, } } diff --git a/clippy_lints/src/eta_reduction.rs b/clippy_lints/src/eta_reduction.rs index e4fcd271bbcb..485a29ed3d89 100644 --- a/clippy_lints/src/eta_reduction.rs +++ b/clippy_lints/src/eta_reduction.rs @@ -202,7 +202,10 @@ fn get_type_name(cx: &LateContext<'_, '_>, ty: Ty<'_>) -> String { } } -fn compare_inputs(closure_inputs: &mut dyn Iterator, call_args: &mut dyn Iterator) -> bool { +fn compare_inputs( + closure_inputs: &mut dyn Iterator, + call_args: &mut dyn Iterator, +) -> bool { for (closure_input, function_arg) in closure_inputs.zip(call_args) { if let PatKind::Binding(_, _, ident, _) = closure_input.pat.node { // XXXManishearth Should I be checking the binding mode here? diff --git a/clippy_lints/src/functions.rs b/clippy_lints/src/functions.rs index 9772a603ba03..e009d28db685 100644 --- a/clippy_lints/src/functions.rs +++ b/clippy_lints/src/functions.rs @@ -280,7 +280,7 @@ impl<'a, 'tcx> Functions { } } -fn raw_ptr_arg(arg: &hir::Arg, ty: &hir::Ty) -> Option { +fn raw_ptr_arg(arg: &hir::Param, ty: &hir::Ty) -> Option { if let (&hir::PatKind::Binding(_, id, _, _), &hir::TyKind::Ptr(_)) = (&arg.pat.node, &ty.node) { Some(id) } else { diff --git a/clippy_lints/src/inherent_to_string.rs b/clippy_lints/src/inherent_to_string.rs index eb29a6d436ab..26b9657589f3 100644 --- a/clippy_lints/src/inherent_to_string.rs +++ b/clippy_lints/src/inherent_to_string.rs @@ -104,6 +104,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for InherentToString { if impl_item.ident.name.as_str() == "to_string"; let decl = &signature.decl; if decl.implicit_self.has_implicit_self(); + if decl.inputs.len() == 1; // Check if return type is String if match_type(cx, return_ty(cx, impl_item.hir_id), &paths::STRING); diff --git a/clippy_lints/src/loops.rs b/clippy_lints/src/loops.rs index b1da7483a600..0ad7c26f8492 100644 --- a/clippy_lints/src/loops.rs +++ b/clippy_lints/src/loops.rs @@ -312,6 +312,14 @@ declare_clippy_lint! { /// for i in 0..v.len() { foo(v[i]); } /// for i in 0..v.len() { bar(i, v[i]); } /// ``` + /// Could be written as + /// ```rust + /// # let v = vec![1]; + /// # fn foo(bar: usize) {} + /// # fn bar(bar: usize, baz: usize) {} + /// for item in &v { foo(*item); } + /// for (i, item) in v.iter().enumerate() { bar(i, *item); } + /// ``` pub EXPLICIT_COUNTER_LOOP, complexity, "for-looping with an explicit counter when `_.enumerate()` would do" diff --git a/clippy_lints/src/map_clone.rs b/clippy_lints/src/map_clone.rs index 09c1f4b3c979..5c44346aa6df 100644 --- a/clippy_lints/src/map_clone.rs +++ b/clippy_lints/src/map_clone.rs @@ -57,7 +57,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for MapClone { let closure_body = cx.tcx.hir().body(body_id); let closure_expr = remove_blocks(&closure_body.value); then { - match closure_body.arguments[0].pat.node { + match closure_body.params[0].pat.node { hir::PatKind::Ref(ref inner, _) => if let hir::PatKind::Binding( hir::BindingAnnotation::Unannotated, .., name, None ) = inner.node { diff --git a/clippy_lints/src/map_unit_fn.rs b/clippy_lints/src/map_unit_fn.rs index 7d46478a6fd3..0df14c2664ae 100644 --- a/clippy_lints/src/map_unit_fn.rs +++ b/clippy_lints/src/map_unit_fn.rs @@ -161,7 +161,10 @@ fn reduce_unit_expression<'a>(cx: &LateContext<'_, '_>, expr: &'a hir::Expr) -> } } -fn unit_closure<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, expr: &'a hir::Expr) -> Option<(&'tcx hir::Arg, &'a hir::Expr)> { +fn unit_closure<'a, 'tcx>( + cx: &LateContext<'a, 'tcx>, + expr: &'a hir::Expr, +) -> Option<(&'tcx hir::Param, &'a hir::Expr)> { if let hir::ExprKind::Closure(_, ref decl, inner_expr_id, _, _) = expr.node { let body = cx.tcx.hir().body(inner_expr_id); let body_expr = &body.value; diff --git a/clippy_lints/src/methods/mod.rs b/clippy_lints/src/methods/mod.rs index f7544e095b8b..81a8e69220c1 100644 --- a/clippy_lints/src/methods/mod.rs +++ b/clippy_lints/src/methods/mod.rs @@ -302,6 +302,11 @@ declare_clippy_lint! { /// # let vec = vec![1]; /// vec.iter().filter(|x| **x == 0).next(); /// ``` + /// Could be written as + /// ```rust + /// # let vec = vec![1]; + /// vec.iter().find(|x| **x == 0); + /// ``` pub FILTER_NEXT, complexity, "using `filter(p).next()`, which is more succinctly expressed as `.find(p)`" @@ -425,6 +430,11 @@ declare_clippy_lint! { /// # let vec = vec![1]; /// vec.iter().find(|x| **x == 0).is_some(); /// ``` + /// Could be written as + /// ```rust + /// # let vec = vec![1]; + /// vec.iter().any(|x| *x == 0); + /// ``` pub SEARCH_IS_SOME, complexity, "using an iterator search followed by `is_some()`, which is more succinctly expressed as a call to `any()`" @@ -442,7 +452,12 @@ declare_clippy_lint! { /// **Example:** /// ```rust /// let name = "foo"; - /// name.chars().next() == Some('_'); + /// if name.chars().next() == Some('_') {}; + /// ``` + /// Could be written as + /// ```rust + /// let name = "foo"; + /// if name.starts_with('_') {}; /// ``` pub CHARS_NEXT_CMP, complexity, @@ -889,6 +904,10 @@ declare_clippy_lint! { /// ```rust /// let _ = [1, 2, 3].into_iter().map(|x| *x).collect::>(); /// ``` + /// Could be written as: + /// ```rust + /// let _ = [1, 2, 3].iter().map(|x| *x).collect::>(); + /// ``` pub INTO_ITER_ON_ARRAY, correctness, "using `.into_iter()` on an array" @@ -1713,8 +1732,8 @@ fn lint_unnecessary_fold(cx: &LateContext<'_, '_>, expr: &hir::Expr, fold_args: if bin_op.node == op; // Extract the names of the two arguments to the closure - if let Some(first_arg_ident) = get_arg_name(&closure_body.arguments[0].pat); - if let Some(second_arg_ident) = get_arg_name(&closure_body.arguments[1].pat); + if let Some(first_arg_ident) = get_arg_name(&closure_body.params[0].pat); + if let Some(second_arg_ident) = get_arg_name(&closure_body.params[1].pat); if match_var(&*left_expr, first_arg_ident); if replacement_has_args || match_var(&*right_expr, second_arg_ident); @@ -2326,7 +2345,7 @@ fn lint_flat_map_identity<'a, 'tcx>( if let hir::ExprKind::Closure(_, _, body_id, _, _) = arg_node; let body = cx.tcx.hir().body(*body_id); - if let hir::PatKind::Binding(_, _, binding_ident, _) = body.arguments[0].pat.node; + if let hir::PatKind::Binding(_, _, binding_ident, _) = body.params[0].pat.node; if let hir::ExprKind::Path(hir::QPath::Resolved(_, ref path)) = body.value.node; if path.segments.len() == 1; @@ -2371,7 +2390,7 @@ fn lint_search_is_some<'a, 'tcx>( if search_method == "find"; if let hir::ExprKind::Closure(_, _, body_id, ..) = search_args[1].node; let closure_body = cx.tcx.hir().body(body_id); - if let Some(closure_arg) = closure_body.arguments.get(0); + if let Some(closure_arg) = closure_body.params.get(0); if let hir::PatKind::Ref(..) = closure_arg.pat.node; then { Some(search_snippet.replacen('&', "", 1)) @@ -2781,7 +2800,10 @@ impl SelfKind { hir::Mutability::MutMutable => &paths::ASMUT_TRAIT, }; - let trait_def_id = get_trait_def_id(cx, trait_path).expect("trait def id not found"); + let trait_def_id = match get_trait_def_id(cx, trait_path) { + Some(did) => did, + None => return false, + }; implements_trait(cx, ty, trait_def_id, &[parent_ty.into()]) } diff --git a/clippy_lints/src/methods/unnecessary_filter_map.rs b/clippy_lints/src/methods/unnecessary_filter_map.rs index a28e9b1e3079..1d562566fdf8 100644 --- a/clippy_lints/src/methods/unnecessary_filter_map.rs +++ b/clippy_lints/src/methods/unnecessary_filter_map.rs @@ -17,7 +17,7 @@ pub(super) fn lint(cx: &LateContext<'_, '_>, expr: &hir::Expr, args: &[hir::Expr if let hir::ExprKind::Closure(_, _, body_id, ..) = args[1].node { let body = cx.tcx.hir().body(body_id); - let arg_id = body.arguments[0].pat.hir_id; + let arg_id = body.params[0].pat.hir_id; let mutates_arg = match mutated_variables(&body.value, cx) { Some(used_mutably) => used_mutably.contains(&arg_id), None => true, diff --git a/clippy_lints/src/misc.rs b/clippy_lints/src/misc.rs index 362eaea3c1f8..b9c05bb2bd33 100644 --- a/clippy_lints/src/misc.rs +++ b/clippy_lints/src/misc.rs @@ -105,6 +105,12 @@ declare_clippy_lint! { /// # let y = String::from("foo"); /// if x.to_owned() == y {} /// ``` + /// Could be written as + /// ```rust + /// # let x = "foo"; + /// # let y = String::from("foo"); + /// if x == y {} + /// ``` pub CMP_OWNED, perf, "creating owned instances for comparing with others, e.g., `x == \"foo\".to_string()`" diff --git a/clippy_lints/src/misc_early.rs b/clippy_lints/src/misc_early.rs index c8ac26044ebc..6ff2a4045797 100644 --- a/clippy_lints/src/misc_early.rs +++ b/clippy_lints/src/misc_early.rs @@ -6,7 +6,6 @@ use rustc::lint::{in_external_macro, EarlyContext, EarlyLintPass, LintArray, Lin use rustc::{declare_lint_pass, declare_tool_lint}; use rustc_data_structures::fx::FxHashMap; use rustc_errors::Applicability; -use std::char; use syntax::ast::*; use syntax::source_map::Span; use syntax::visit::{walk_expr, FnKind, Visitor}; @@ -391,92 +390,93 @@ impl EarlyLintPass for MiscEarlyLints { impl MiscEarlyLints { fn check_lit(self, cx: &EarlyContext<'_>, lit: &Lit) { - if_chain! { - if let LitKind::Int(value, ..) = lit.node; - if let Some(src) = snippet_opt(cx, lit.span); - if let Some(firstch) = src.chars().next(); - if char::to_digit(firstch, 10).is_some(); - then { - let mut prev = '\0'; - for (idx, ch) in src.chars().enumerate() { - if ch == 'i' || ch == 'u' { - if prev != '_' { - span_lint_and_sugg( - cx, - UNSEPARATED_LITERAL_SUFFIX, - lit.span, - "integer type suffix should be separated by an underscore", - "add an underscore", - format!("{}_{}", &src[0..idx], &src[idx..]), - Applicability::MachineApplicable, - ); - } - break; - } - prev = ch; + // The `line!()` macro is compiler built-in and a special case for these lints. + let lit_snip = match snippet_opt(cx, lit.span) { + Some(snip) => { + if snip.contains('!') { + return; } - if src.starts_with("0x") { - let mut seen = (false, false); - for ch in src.chars() { - match ch { - 'a' ..= 'f' => seen.0 = true, - 'A' ..= 'F' => seen.1 = true, - 'i' | 'u' => break, // start of suffix already - _ => () - } + snip + }, + _ => return, + }; + + if let LitKind::Int(value, lit_int_type) = lit.node { + let suffix = match lit_int_type { + LitIntType::Signed(ty) => ty.ty_to_string(), + LitIntType::Unsigned(ty) => ty.ty_to_string(), + LitIntType::Unsuffixed => "", + }; + + let maybe_last_sep_idx = lit_snip.len() - suffix.len() - 1; + // Do not lint when literal is unsuffixed. + if !suffix.is_empty() && lit_snip.as_bytes()[maybe_last_sep_idx] != b'_' { + span_lint_and_sugg( + cx, + UNSEPARATED_LITERAL_SUFFIX, + lit.span, + "integer type suffix should be separated by an underscore", + "add an underscore", + format!("{}_{}", &lit_snip[..=maybe_last_sep_idx], suffix), + Applicability::MachineApplicable, + ); + } + + if lit_snip.starts_with("0x") { + let mut seen = (false, false); + for ch in lit_snip.as_bytes()[2..=maybe_last_sep_idx].iter() { + match ch { + b'a'..=b'f' => seen.0 = true, + b'A'..=b'F' => seen.1 = true, + _ => {}, } if seen.0 && seen.1 { - span_lint(cx, MIXED_CASE_HEX_LITERALS, lit.span, - "inconsistent casing in hexadecimal literal"); + span_lint( + cx, + MIXED_CASE_HEX_LITERALS, + lit.span, + "inconsistent casing in hexadecimal literal", + ); + break; } - } else if src.starts_with("0b") || src.starts_with("0o") { - /* nothing to do */ - } else if value != 0 && src.starts_with('0') { - span_lint_and_then(cx, - ZERO_PREFIXED_LITERAL, - lit.span, - "this is a decimal constant", - |db| { + } + } else if lit_snip.starts_with("0b") || lit_snip.starts_with("0o") { + /* nothing to do */ + } else if value != 0 && lit_snip.starts_with('0') { + span_lint_and_then( + cx, + ZERO_PREFIXED_LITERAL, + lit.span, + "this is a decimal constant", + |db| { db.span_suggestion( lit.span, - "if you mean to use a decimal constant, remove the `0` to remove confusion", - src.trim_start_matches(|c| c == '_' || c == '0').to_string(), + "if you mean to use a decimal constant, remove the `0` to avoid confusion", + lit_snip.trim_start_matches(|c| c == '_' || c == '0').to_string(), Applicability::MaybeIncorrect, ); db.span_suggestion( lit.span, "if you mean to use an octal constant, use `0o`", - format!("0o{}", src.trim_start_matches(|c| c == '_' || c == '0')), + format!("0o{}", lit_snip.trim_start_matches(|c| c == '_' || c == '0')), Applicability::MaybeIncorrect, ); - }); - } + }, + ); } - } - if_chain! { - if let LitKind::Float(..) = lit.node; - if let Some(src) = snippet_opt(cx, lit.span); - if let Some(firstch) = src.chars().next(); - if char::to_digit(firstch, 10).is_some(); - then { - let mut prev = '\0'; - for (idx, ch) in src.chars().enumerate() { - if ch == 'f' { - if prev != '_' { - span_lint_and_sugg( - cx, - UNSEPARATED_LITERAL_SUFFIX, - lit.span, - "float type suffix should be separated by an underscore", - "add an underscore", - format!("{}_{}", &src[0..idx], &src[idx..]), - Applicability::MachineApplicable, - ); - } - break; - } - prev = ch; - } + } else if let LitKind::Float(_, float_ty) = lit.node { + let suffix = float_ty.ty_to_string(); + let maybe_last_sep_idx = lit_snip.len() - suffix.len() - 1; + if lit_snip.as_bytes()[maybe_last_sep_idx] != b'_' { + span_lint_and_sugg( + cx, + UNSEPARATED_LITERAL_SUFFIX, + lit.span, + "float type suffix should be separated by an underscore", + "add an underscore", + format!("{}_{}", &lit_snip[..=maybe_last_sep_idx], suffix), + Applicability::MachineApplicable, + ); } } } diff --git a/clippy_lints/src/missing_doc.rs b/clippy_lints/src/missing_doc.rs index 6c575fd94a87..33d1fce4e2a6 100644 --- a/clippy_lints/src/missing_doc.rs +++ b/clippy_lints/src/missing_doc.rs @@ -196,7 +196,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for MissingDoc { } } - fn check_variant(&mut self, cx: &LateContext<'a, 'tcx>, v: &'tcx hir::Variant, _: &hir::Generics) { + fn check_variant(&mut self, cx: &LateContext<'a, 'tcx>, v: &'tcx hir::Variant) { self.check_missing_docs_attrs(cx, &v.attrs, v.span, "a variant"); } } diff --git a/clippy_lints/src/needless_bool.rs b/clippy_lints/src/needless_bool.rs index 7d28b32c3299..b761457f64ce 100644 --- a/clippy_lints/src/needless_bool.rs +++ b/clippy_lints/src/needless_bool.rs @@ -31,6 +31,10 @@ declare_clippy_lint! { /// true /// } /// ``` + /// Could be written as + /// ```rust,ignore + /// !x + /// ``` pub NEEDLESS_BOOL, complexity, "if-statements with plain booleans in the then- and else-clause, e.g., `if p { true } else { false }`" diff --git a/clippy_lints/src/needless_pass_by_value.rs b/clippy_lints/src/needless_pass_by_value.rs index f09fb22f82ad..d3ca5cb539a1 100644 --- a/clippy_lints/src/needless_pass_by_value.rs +++ b/clippy_lints/src/needless_pass_by_value.rs @@ -152,7 +152,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for NeedlessPassByValue { let fn_sig = cx.tcx.fn_sig(fn_def_id); let fn_sig = cx.tcx.erase_late_bound_regions(&fn_sig); - for (idx, ((input, &ty), arg)) in decl.inputs.iter().zip(fn_sig.inputs()).zip(&body.arguments).enumerate() { + for (idx, ((input, &ty), arg)) in decl.inputs.iter().zip(fn_sig.inputs()).zip(&body.params).enumerate() { // All spans generated from a proc-macro invocation are the same... if span == input.span { return; diff --git a/clippy_lints/src/ranges.rs b/clippy_lints/src/ranges.rs index a4ce129c95ea..804f86a9a964 100644 --- a/clippy_lints/src/ranges.rs +++ b/clippy_lints/src/ranges.rs @@ -43,6 +43,11 @@ declare_clippy_lint! { /// # let x = vec![1]; /// x.iter().zip(0..x.len()); /// ``` + /// Could be written as + /// ```rust + /// # let x = vec![1]; + /// x.iter().enumerate(); + /// ``` pub RANGE_ZIP_WITH_LEN, complexity, "zipping iterator with a range when `enumerate()` would do" @@ -64,6 +69,10 @@ declare_clippy_lint! { /// ```rust,ignore /// for x..(y+1) { .. } /// ``` + /// Could be written as + /// ```rust,ignore + /// for x..=y { .. } + /// ``` pub RANGE_PLUS_ONE, complexity, "`x..(y+1)` reads better as `x..=y`" @@ -82,6 +91,10 @@ declare_clippy_lint! { /// ```rust,ignore /// for x..=(y-1) { .. } /// ``` + /// Could be written as + /// ```rust,ignore + /// for x..y { .. } + /// ``` pub RANGE_MINUS_ONE, complexity, "`x..=(y-1)` reads better as `x..y`" diff --git a/clippy_lints/src/returns.rs b/clippy_lints/src/returns.rs index 3cc053a0ebf6..e864518ee598 100644 --- a/clippy_lints/src/returns.rs +++ b/clippy_lints/src/returns.rs @@ -277,7 +277,7 @@ impl EarlyLintPass for Return { if_chain! { if let Some(ref stmt) = block.stmts.last(); if let ast::StmtKind::Expr(ref expr) = stmt.node; - if is_unit_expr(expr) && !expr.span.from_expansion(); + if is_unit_expr(expr) && !stmt.span.from_expansion(); then { let sp = expr.span; span_lint_and_then(cx, UNUSED_UNIT, sp, "unneeded unit expression", |db| { diff --git a/clippy_lints/src/swap.rs b/clippy_lints/src/swap.rs index 11ab5b876282..80c9a33c06a8 100644 --- a/clippy_lints/src/swap.rs +++ b/clippy_lints/src/swap.rs @@ -52,6 +52,12 @@ declare_clippy_lint! { /// a = b; /// b = a; /// ``` + /// Could be written as: + /// ```rust + /// # let mut a = 1; + /// # let mut b = 2; + /// std::mem::swap(&mut a, &mut b); + /// ``` pub ALMOST_SWAPPED, correctness, "`foo = bar; bar = foo` sequence" diff --git a/clippy_lints/src/types.rs b/clippy_lints/src/types.rs index 24eba166c7b1..28d337d3cd67 100644 --- a/clippy_lints/src/types.rs +++ b/clippy_lints/src/types.rs @@ -2056,7 +2056,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for ImplicitHasher { continue; } let generics_suggestion_span = generics.span.substitute_dummy({ - let pos = snippet_opt(cx, item.span.until(body.arguments[0].pat.span)) + let pos = snippet_opt(cx, item.span.until(body.params[0].pat.span)) .and_then(|snip| { let i = snip.find("fn")?; Some(item.span.lo() + BytePos((i + (&snip[i..]).find('(')?) as u32)) diff --git a/clippy_lints/src/utils/author.rs b/clippy_lints/src/utils/author.rs index e987b447a268..87208dd4beb7 100644 --- a/clippy_lints/src/utils/author.rs +++ b/clippy_lints/src/utils/author.rs @@ -90,12 +90,12 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Author { done(); } - fn check_variant(&mut self, cx: &LateContext<'a, 'tcx>, var: &'tcx hir::Variant, generics: &hir::Generics) { + fn check_variant(&mut self, cx: &LateContext<'a, 'tcx>, var: &'tcx hir::Variant) { if !has_attr(cx.sess(), &var.attrs) { return; } prelude(); - PrintVisitor::new("var").visit_variant(var, generics, hir::DUMMY_HIR_ID); + PrintVisitor::new("var").visit_variant(var, &hir::Generics::empty(), hir::DUMMY_HIR_ID); done(); } diff --git a/clippy_lints/src/utils/mod.rs b/clippy_lints/src/utils/mod.rs index 2d772d77ed10..8fb45899653c 100644 --- a/clippy_lints/src/utils/mod.rs +++ b/clippy_lints/src/utils/mod.rs @@ -261,6 +261,7 @@ pub fn path_to_res(cx: &LateContext<'_, '_>, path: &[&str]) -> Option<(def::Res) } /// 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 { let res = match path_to_res(cx, path) { Some(res) => res, @@ -268,7 +269,8 @@ pub fn get_trait_def_id(cx: &LateContext<'_, '_>, path: &[&str]) -> Option Some(trait_id), + Res::Def(DefKind::Trait, trait_id) | Res::Def(DefKind::TraitAlias, trait_id) => Some(trait_id), + Res::Err => unreachable!("this trait resolution is impossible: {:?}", &path), _ => None, } } @@ -833,7 +835,7 @@ pub fn remove_blocks(expr: &Expr) -> &Expr { } } -pub fn is_self(slf: &Arg) -> bool { +pub fn is_self(slf: &Param) -> bool { if let PatKind::Binding(.., name, _) = slf.pat.node { name.name == kw::SelfLower } else { @@ -853,8 +855,8 @@ pub fn is_self_ty(slf: &hir::Ty) -> bool { false } -pub fn iter_input_pats<'tcx>(decl: &FnDecl, body: &'tcx Body) -> impl Iterator { - (0..decl.inputs.len()).map(move |i| &body.arguments[i]) +pub fn iter_input_pats<'tcx>(decl: &FnDecl, body: &'tcx Body) -> impl Iterator { + (0..decl.inputs.len()).map(move |i| &body.params[i]) } /// Checks if a given expression is a match expression expanded from the `?` diff --git a/clippy_lints/src/utils/ptr.rs b/clippy_lints/src/utils/ptr.rs index e378ef0c0a90..be7bd0d21f69 100644 --- a/clippy_lints/src/utils/ptr.rs +++ b/clippy_lints/src/utils/ptr.rs @@ -13,7 +13,7 @@ pub fn get_spans( replacements: &[(&'static str, &'static str)], ) -> Option)>> { if let Some(body) = opt_body_id.map(|id| cx.tcx.hir().body(id)) { - get_binding_name(&body.arguments[idx]).map_or_else( + get_binding_name(&body.params[idx]).map_or_else( || Some(vec![]), |name| extract_clone_suggestions(cx, name, replacements, body), ) @@ -80,6 +80,6 @@ impl<'a, 'tcx> Visitor<'tcx> for PtrCloneVisitor<'a, 'tcx> { } } -fn get_binding_name(arg: &Arg) -> Option { +fn get_binding_name(arg: &Param) -> Option { get_pat_name(&arg.pat) } diff --git a/doc/adding_lints.md b/doc/adding_lints.md index c9816911a828..1fb26c66d9fb 100644 --- a/doc/adding_lints.md +++ b/doc/adding_lints.md @@ -86,6 +86,8 @@ test. That allows us to check if the output is turning into what we want. Once we are satisfied with the output, we need to run `tests/ui/update-all-references.sh` to update the `.stderr` file for our lint. +Please note that, we should run `TESTNAME=ui/foo_functions cargo uitest` +every time before running `tests/ui/update-all-references.sh`. Running `TESTNAME=ui/foo_functions cargo uitest` should pass then. When we commit our lint, we need to commit the generated `.stderr` files, too. diff --git a/tests/ui/author/for_loop.stderr b/tests/ui/author/for_loop.stderr deleted file mode 100644 index e69de29bb2d1..000000000000 diff --git a/tests/ui/author/if.stderr b/tests/ui/author/if.stderr deleted file mode 100644 index e69de29bb2d1..000000000000 diff --git a/tests/ui/cmp_owned/with_suggestion.fixed b/tests/ui/cmp_owned/with_suggestion.fixed new file mode 100644 index 000000000000..05fb96339e33 --- /dev/null +++ b/tests/ui/cmp_owned/with_suggestion.fixed @@ -0,0 +1,72 @@ +// run-rustfix + +#[warn(clippy::cmp_owned)] +#[allow(clippy::unnecessary_operation, clippy::no_effect, unused_must_use, clippy::eq_op)] +fn main() { + fn with_to_string(x: &str) { + x != "foo"; + + "foo" != x; + } + + let x = "oh"; + + with_to_string(x); + + x != "foo"; + + x != "foo"; + + 42.to_string() == "42"; + + Foo == Foo; + + "abc".chars().filter(|c| *c != 'X'); + + "abc".chars().filter(|c| *c != 'X'); +} + +struct Foo; + +impl PartialEq for Foo { + // Allow this here, because it emits the lint + // without a suggestion. This is tested in + // `tests/ui/cmp_owned/without_suggestion.rs` + #[allow(clippy::cmp_owned)] + fn eq(&self, other: &Self) -> bool { + self.to_owned() == *other + } +} + +impl ToOwned for Foo { + type Owned = Bar; + fn to_owned(&self) -> Bar { + Bar + } +} + +#[derive(PartialEq)] +struct Bar; + +impl PartialEq for Bar { + fn eq(&self, _: &Foo) -> bool { + true + } +} + +impl std::borrow::Borrow for Bar { + fn borrow(&self) -> &Foo { + static FOO: Foo = Foo; + &FOO + } +} + +#[derive(PartialEq)] +struct Baz; + +impl ToOwned for Baz { + type Owned = Baz; + fn to_owned(&self) -> Baz { + Baz + } +} diff --git a/tests/ui/cmp_owned.rs b/tests/ui/cmp_owned/with_suggestion.rs similarity index 77% rename from tests/ui/cmp_owned.rs rename to tests/ui/cmp_owned/with_suggestion.rs index a5f92b30bc2b..0a02825ed82f 100644 --- a/tests/ui/cmp_owned.rs +++ b/tests/ui/cmp_owned/with_suggestion.rs @@ -1,5 +1,7 @@ +// run-rustfix + #[warn(clippy::cmp_owned)] -#[allow(clippy::unnecessary_operation)] +#[allow(clippy::unnecessary_operation, clippy::no_effect, unused_must_use, clippy::eq_op)] fn main() { fn with_to_string(x: &str) { x != "foo".to_string(); @@ -22,21 +24,15 @@ fn main() { "abc".chars().filter(|c| c.to_owned() != 'X'); "abc".chars().filter(|c| *c != 'X'); - - let x = &Baz; - let y = &Baz; - - y.to_owned() == *x; - - let x = &&Baz; - let y = &Baz; - - y.to_owned() == **x; } struct Foo; impl PartialEq for Foo { + // Allow this here, because it emits the lint + // without a suggestion. This is tested in + // `tests/ui/cmp_owned/without_suggestion.rs` + #[allow(clippy::cmp_owned)] fn eq(&self, other: &Self) -> bool { self.to_owned() == *other } diff --git a/tests/ui/cmp_owned.stderr b/tests/ui/cmp_owned/with_suggestion.stderr similarity index 54% rename from tests/ui/cmp_owned.stderr rename to tests/ui/cmp_owned/with_suggestion.stderr index 9be749f8d040..2f333e6ea8ec 100644 --- a/tests/ui/cmp_owned.stderr +++ b/tests/ui/cmp_owned/with_suggestion.stderr @@ -1,5 +1,5 @@ error: this creates an owned instance just for comparison - --> $DIR/cmp_owned.rs:5:14 + --> $DIR/with_suggestion.rs:7:14 | LL | x != "foo".to_string(); | ^^^^^^^^^^^^^^^^^ help: try: `"foo"` @@ -7,52 +7,34 @@ LL | x != "foo".to_string(); = note: `-D clippy::cmp-owned` implied by `-D warnings` error: this creates an owned instance just for comparison - --> $DIR/cmp_owned.rs:7:9 + --> $DIR/with_suggestion.rs:9:9 | LL | "foo".to_string() != x; | ^^^^^^^^^^^^^^^^^ help: try: `"foo"` error: this creates an owned instance just for comparison - --> $DIR/cmp_owned.rs:14:10 + --> $DIR/with_suggestion.rs:16:10 | LL | x != "foo".to_owned(); | ^^^^^^^^^^^^^^^^ help: try: `"foo"` error: this creates an owned instance just for comparison - --> $DIR/cmp_owned.rs:16:10 + --> $DIR/with_suggestion.rs:18:10 | LL | x != String::from("foo"); | ^^^^^^^^^^^^^^^^^^^ help: try: `"foo"` error: this creates an owned instance just for comparison - --> $DIR/cmp_owned.rs:20:5 + --> $DIR/with_suggestion.rs:22:5 | LL | Foo.to_owned() == Foo; | ^^^^^^^^^^^^^^ help: try: `Foo` error: this creates an owned instance just for comparison - --> $DIR/cmp_owned.rs:22:30 + --> $DIR/with_suggestion.rs:24:30 | LL | "abc".chars().filter(|c| c.to_owned() != 'X'); | ^^^^^^^^^^^^ help: try: `*c` -error: this creates an owned instance just for comparison - --> $DIR/cmp_owned.rs:29:5 - | -LL | y.to_owned() == *x; - | ^^^^^^^^^^^^^^^^^^ try implementing the comparison without allocating - -error: this creates an owned instance just for comparison - --> $DIR/cmp_owned.rs:34:5 - | -LL | y.to_owned() == **x; - | ^^^^^^^^^^^^^^^^^^^ try implementing the comparison without allocating - -error: this creates an owned instance just for comparison - --> $DIR/cmp_owned.rs:41:9 - | -LL | self.to_owned() == *other - | ^^^^^^^^^^^^^^^^^^^^^^^^^ try implementing the comparison without allocating - -error: aborting due to 9 previous errors +error: aborting due to 6 previous errors diff --git a/tests/ui/cmp_owned/without_suggestion.rs b/tests/ui/cmp_owned/without_suggestion.rs new file mode 100644 index 000000000000..9ab8795474c6 --- /dev/null +++ b/tests/ui/cmp_owned/without_suggestion.rs @@ -0,0 +1,52 @@ +#[allow(clippy::unnecessary_operation)] + +fn main() { + let x = &Baz; + let y = &Baz; + y.to_owned() == *x; + + let x = &&Baz; + let y = &Baz; + y.to_owned() == **x; +} + +struct Foo; + +impl PartialEq for Foo { + fn eq(&self, other: &Self) -> bool { + self.to_owned() == *other + } +} + +impl ToOwned for Foo { + type Owned = Bar; + fn to_owned(&self) -> Bar { + Bar + } +} + +#[derive(PartialEq)] +struct Baz; + +impl ToOwned for Baz { + type Owned = Baz; + fn to_owned(&self) -> Baz { + Baz + } +} + +#[derive(PartialEq)] +struct Bar; + +impl PartialEq for Bar { + fn eq(&self, _: &Foo) -> bool { + true + } +} + +impl std::borrow::Borrow for Bar { + fn borrow(&self) -> &Foo { + static FOO: Foo = Foo; + &FOO + } +} diff --git a/tests/ui/cmp_owned/without_suggestion.stderr b/tests/ui/cmp_owned/without_suggestion.stderr new file mode 100644 index 000000000000..6e8a5ad2a17b --- /dev/null +++ b/tests/ui/cmp_owned/without_suggestion.stderr @@ -0,0 +1,22 @@ +error: this creates an owned instance just for comparison + --> $DIR/without_suggestion.rs:6:5 + | +LL | y.to_owned() == *x; + | ^^^^^^^^^^^^^^^^^^ try implementing the comparison without allocating + | + = note: `-D clippy::cmp-owned` implied by `-D warnings` + +error: this creates an owned instance just for comparison + --> $DIR/without_suggestion.rs:10:5 + | +LL | y.to_owned() == **x; + | ^^^^^^^^^^^^^^^^^^^ try implementing the comparison without allocating + +error: this creates an owned instance just for comparison + --> $DIR/without_suggestion.rs:17:9 + | +LL | self.to_owned() == *other + | ^^^^^^^^^^^^^^^^^^^^^^^^^ try implementing the comparison without allocating + +error: aborting due to 3 previous errors + diff --git a/tests/ui/def_id_nocore.rs b/tests/ui/def_id_nocore.rs new file mode 100644 index 000000000000..2a948d60b108 --- /dev/null +++ b/tests/ui/def_id_nocore.rs @@ -0,0 +1,29 @@ +// ignore-windows +// ignore-macos + +#![feature(no_core, lang_items, start)] +#![no_core] + +#[link(name = "c")] +extern "C" {} + +#[lang = "sized"] +pub trait Sized {} +#[lang = "copy"] +pub trait Copy {} +#[lang = "freeze"] +pub unsafe trait Freeze {} + +#[lang = "start"] +#[start] +fn start(_argc: isize, _argv: *const *const u8) -> isize { + 0 +} + +pub struct A; + +impl A { + pub fn as_ref(self) -> &'static str { + "A" + } +} diff --git a/tests/ui/def_id_nocore.stderr b/tests/ui/def_id_nocore.stderr new file mode 100644 index 000000000000..ed87a50547d1 --- /dev/null +++ b/tests/ui/def_id_nocore.stderr @@ -0,0 +1,10 @@ +error: methods called `as_*` usually take self by reference or self by mutable reference; consider choosing a less ambiguous name + --> $DIR/def_id_nocore.rs:26:19 + | +LL | pub fn as_ref(self) -> &'static str { + | ^^^^ + | + = note: `-D clippy::wrong-self-convention` implied by `-D warnings` + +error: aborting due to previous error + diff --git a/tests/ui/default_trait_access.stdout b/tests/ui/default_trait_access.stdout deleted file mode 100644 index e69de29bb2d1..000000000000 diff --git a/tests/ui/duration_subsec.stdout b/tests/ui/duration_subsec.stdout deleted file mode 100644 index e69de29bb2d1..000000000000 diff --git a/tests/ui/inherent_to_string.rs b/tests/ui/inherent_to_string.rs index fc21b5cbc3f1..e6cf337d1bb1 100644 --- a/tests/ui/inherent_to_string.rs +++ b/tests/ui/inherent_to_string.rs @@ -1,5 +1,6 @@ #![warn(clippy::inherent_to_string)] #![deny(clippy::inherent_to_string_shadow_display)] +#![allow(clippy::many_single_char_names)] use std::fmt; @@ -12,6 +13,7 @@ struct B; struct C; struct D; struct E; +struct F; impl A { // Should be detected; emit warning @@ -64,6 +66,13 @@ impl E { } } +impl F { + // Should not be detected, as it does not match the function signature + fn to_string(&self, _i: i32) -> String { + "F.to_string()".to_string() + } +} + fn main() { let a = A; a.to_string(); @@ -81,4 +90,7 @@ fn main() { d.to_string(); E::to_string(); + + let f = F; + f.to_string(1); } diff --git a/tests/ui/inherent_to_string.stderr b/tests/ui/inherent_to_string.stderr index 5252a168830a..76d1bb873ebe 100644 --- a/tests/ui/inherent_to_string.stderr +++ b/tests/ui/inherent_to_string.stderr @@ -1,5 +1,5 @@ error: implementation of inherent method `to_string(&self) -> String` for type `A` - --> $DIR/inherent_to_string.rs:18:5 + --> $DIR/inherent_to_string.rs:20:5 | LL | / fn to_string(&self) -> String { LL | | "A.to_string()".to_string() @@ -10,7 +10,7 @@ LL | | } = help: implement trait `Display` for type `A` instead error: type `C` implements inherent method `to_string(&self) -> String` which shadows the implementation of `Display` - --> $DIR/inherent_to_string.rs:42:5 + --> $DIR/inherent_to_string.rs:44:5 | LL | / fn to_string(&self) -> String { LL | | "C.to_string()".to_string() diff --git a/tests/ui/literals.stderr b/tests/ui/literals.stderr index 4f3d430c4d99..33321440d831 100644 --- a/tests/ui/literals.stderr +++ b/tests/ui/literals.stderr @@ -25,7 +25,7 @@ LL | let fail_multi_zero = 000_123usize; | ^^^^^^^^^^^^ | = note: `-D clippy::zero-prefixed-literal` implied by `-D warnings` -help: if you mean to use a decimal constant, remove the `0` to remove confusion +help: if you mean to use a decimal constant, remove the `0` to avoid confusion | LL | let fail_multi_zero = 123usize; | ^^^^^^^^ @@ -39,7 +39,7 @@ error: this is a decimal constant | LL | let fail8 = 0123; | ^^^^ -help: if you mean to use a decimal constant, remove the `0` to remove confusion +help: if you mean to use a decimal constant, remove the `0` to avoid confusion | LL | let fail8 = 123; | ^^^ diff --git a/tests/ui/missing_const_for_fn/cant_be_const.stderr b/tests/ui/missing_const_for_fn/cant_be_const.stderr deleted file mode 100644 index e69de29bb2d1..000000000000 diff --git a/tests/ui/or_fun_call.fixed b/tests/ui/or_fun_call.fixed new file mode 100644 index 000000000000..c572e0f2f451 --- /dev/null +++ b/tests/ui/or_fun_call.fixed @@ -0,0 +1,102 @@ +// run-rustfix + +#![warn(clippy::or_fun_call)] +#![allow(dead_code)] + +use std::collections::BTreeMap; +use std::collections::HashMap; +use std::time::Duration; + +/// Checks implementation of the `OR_FUN_CALL` lint. +fn or_fun_call() { + struct Foo; + + impl Foo { + fn new() -> Foo { + Foo + } + } + + enum Enum { + A(i32), + } + + fn make() -> T { + unimplemented!(); + } + + let with_enum = Some(Enum::A(1)); + with_enum.unwrap_or(Enum::A(5)); + + let with_const_fn = Some(Duration::from_secs(1)); + with_const_fn.unwrap_or(Duration::from_secs(5)); + + let with_constructor = Some(vec![1]); + with_constructor.unwrap_or_else(make); + + let with_new = Some(vec![1]); + with_new.unwrap_or_default(); + + let with_const_args = Some(vec![1]); + with_const_args.unwrap_or_else(|| Vec::with_capacity(12)); + + let with_err: Result<_, ()> = Ok(vec![1]); + with_err.unwrap_or_else(|_| make()); + + let with_err_args: Result<_, ()> = Ok(vec![1]); + with_err_args.unwrap_or_else(|_| Vec::with_capacity(12)); + + let with_default_trait = Some(1); + with_default_trait.unwrap_or_default(); + + let with_default_type = Some(1); + with_default_type.unwrap_or_default(); + + let with_vec = Some(vec![1]); + with_vec.unwrap_or_else(|| vec![]); + + // FIXME #944: ~|SUGGESTION with_vec.unwrap_or_else(|| vec![]); + + let without_default = Some(Foo); + without_default.unwrap_or_else(Foo::new); + + let mut map = HashMap::::new(); + map.entry(42).or_insert_with(String::new); + + let mut btree = BTreeMap::::new(); + btree.entry(42).or_insert_with(String::new); + + let stringy = Some(String::from("")); + let _ = stringy.unwrap_or_else(|| "".to_owned()); + + let opt = Some(1); + let hello = "Hello"; + let _ = opt.ok_or_else(|| format!("{} world.", hello)); +} + +struct Foo(u8); +struct Bar(String, Duration); +#[rustfmt::skip] +fn test_or_with_ctors() { + let opt = Some(1); + let opt_opt = Some(Some(1)); + // we also test for const promotion, this makes sure we don't hit that + let two = 2; + + let _ = opt_opt.unwrap_or(Some(2)); + let _ = opt_opt.unwrap_or(Some(two)); + let _ = opt.ok_or(Some(2)); + let _ = opt.ok_or(Some(two)); + let _ = opt.ok_or(Foo(2)); + let _ = opt.ok_or(Foo(two)); + let _ = opt.or(Some(2)); + let _ = opt.or(Some(two)); + + let _ = Some("a".to_string()).or_else(|| Some("b".to_string())); + + let b = "b".to_string(); + let _ = Some(Bar("a".to_string(), Duration::from_secs(1))) + .or(Some(Bar(b, Duration::from_secs(2)))); +} + +fn main() {} diff --git a/tests/ui/or_fun_call.rs b/tests/ui/or_fun_call.rs index f339bae8ac6d..3c94542774b1 100644 --- a/tests/ui/or_fun_call.rs +++ b/tests/ui/or_fun_call.rs @@ -1,4 +1,7 @@ +// run-rustfix + #![warn(clippy::or_fun_call)] +#![allow(dead_code)] use std::collections::BTreeMap; use std::collections::HashMap; diff --git a/tests/ui/or_fun_call.stderr b/tests/ui/or_fun_call.stderr index f6e5d202e0c1..7d60cba38313 100644 --- a/tests/ui/or_fun_call.stderr +++ b/tests/ui/or_fun_call.stderr @@ -1,5 +1,5 @@ error: use of `unwrap_or` followed by a function call - --> $DIR/or_fun_call.rs:32:22 + --> $DIR/or_fun_call.rs:35:22 | LL | with_constructor.unwrap_or(make()); | ^^^^^^^^^^^^^^^^^ help: try this: `unwrap_or_else(make)` @@ -7,79 +7,79 @@ LL | with_constructor.unwrap_or(make()); = note: `-D clippy::or-fun-call` implied by `-D warnings` error: use of `unwrap_or` followed by a call to `new` - --> $DIR/or_fun_call.rs:35:5 + --> $DIR/or_fun_call.rs:38:5 | LL | with_new.unwrap_or(Vec::new()); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `with_new.unwrap_or_default()` error: use of `unwrap_or` followed by a function call - --> $DIR/or_fun_call.rs:38:21 + --> $DIR/or_fun_call.rs:41:21 | LL | with_const_args.unwrap_or(Vec::with_capacity(12)); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `unwrap_or_else(|| Vec::with_capacity(12))` error: use of `unwrap_or` followed by a function call - --> $DIR/or_fun_call.rs:41:14 + --> $DIR/or_fun_call.rs:44:14 | LL | with_err.unwrap_or(make()); | ^^^^^^^^^^^^^^^^^ help: try this: `unwrap_or_else(|_| make())` error: use of `unwrap_or` followed by a function call - --> $DIR/or_fun_call.rs:44:19 + --> $DIR/or_fun_call.rs:47:19 | LL | with_err_args.unwrap_or(Vec::with_capacity(12)); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `unwrap_or_else(|_| Vec::with_capacity(12))` error: use of `unwrap_or` followed by a call to `default` - --> $DIR/or_fun_call.rs:47:5 + --> $DIR/or_fun_call.rs:50:5 | LL | with_default_trait.unwrap_or(Default::default()); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `with_default_trait.unwrap_or_default()` error: use of `unwrap_or` followed by a call to `default` - --> $DIR/or_fun_call.rs:50:5 + --> $DIR/or_fun_call.rs:53:5 | LL | with_default_type.unwrap_or(u64::default()); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `with_default_type.unwrap_or_default()` error: use of `unwrap_or` followed by a function call - --> $DIR/or_fun_call.rs:53:14 + --> $DIR/or_fun_call.rs:56:14 | LL | with_vec.unwrap_or(vec![]); | ^^^^^^^^^^^^^^^^^ help: try this: `unwrap_or_else(|| vec![])` error: use of `unwrap_or` followed by a function call - --> $DIR/or_fun_call.rs:58:21 + --> $DIR/or_fun_call.rs:61:21 | LL | without_default.unwrap_or(Foo::new()); | ^^^^^^^^^^^^^^^^^^^^^ help: try this: `unwrap_or_else(Foo::new)` error: use of `or_insert` followed by a function call - --> $DIR/or_fun_call.rs:61:19 + --> $DIR/or_fun_call.rs:64:19 | LL | map.entry(42).or_insert(String::new()); | ^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `or_insert_with(String::new)` error: use of `or_insert` followed by a function call - --> $DIR/or_fun_call.rs:64:21 + --> $DIR/or_fun_call.rs:67:21 | LL | btree.entry(42).or_insert(String::new()); | ^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `or_insert_with(String::new)` error: use of `unwrap_or` followed by a function call - --> $DIR/or_fun_call.rs:67:21 + --> $DIR/or_fun_call.rs:70:21 | LL | let _ = stringy.unwrap_or("".to_owned()); | ^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `unwrap_or_else(|| "".to_owned())` error: use of `ok_or` followed by a function call - --> $DIR/or_fun_call.rs:71:17 + --> $DIR/or_fun_call.rs:74:17 | LL | let _ = opt.ok_or(format!("{} world.", hello)); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `ok_or_else(|| format!("{} world.", hello))` error: use of `or` followed by a function call - --> $DIR/or_fun_call.rs:92:35 + --> $DIR/or_fun_call.rs:95:35 | LL | let _ = Some("a".to_string()).or(Some("b".to_string())); | ^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `or_else(|| Some("b".to_string()))` diff --git a/tests/ui/proc_macro.rs b/tests/ui/proc_macro.rs old mode 100755 new mode 100644 diff --git a/tests/ui/range_plus_minus_one.fixed b/tests/ui/range_plus_minus_one.fixed new file mode 100644 index 000000000000..badfb5baf3c1 --- /dev/null +++ b/tests/ui/range_plus_minus_one.fixed @@ -0,0 +1,37 @@ +// run-rustfix + +#![allow(unused_parens)] + +fn f() -> usize { + 42 +} + +#[warn(clippy::range_plus_one)] +fn main() { + for _ in 0..2 {} + for _ in 0..=2 {} + + for _ in 0..=3 {} + for _ in 0..=3 + 1 {} + + for _ in 0..=5 {} + for _ in 0..=1 + 5 {} + + for _ in 1..=1 {} + for _ in 1..=1 + 1 {} + + for _ in 0..13 + 13 {} + for _ in 0..=13 - 7 {} + + for _ in 0..=f() {} + for _ in 0..=(1 + f()) {} + + let _ = ..11 - 1; + let _ = ..11; + let _ = ..11; + let _ = (1..=11); + let _ = ((f() + 1)..=f()); + + let mut vec: Vec<()> = std::vec::Vec::new(); + vec.drain(..); +} diff --git a/tests/ui/range_plus_minus_one.rs b/tests/ui/range_plus_minus_one.rs index 54aec853d3ba..c4facd2c23d9 100644 --- a/tests/ui/range_plus_minus_one.rs +++ b/tests/ui/range_plus_minus_one.rs @@ -1,3 +1,7 @@ +// run-rustfix + +#![allow(unused_parens)] + fn f() -> usize { 42 } diff --git a/tests/ui/range_plus_minus_one.stderr b/tests/ui/range_plus_minus_one.stderr index 9ebc22e16253..8318f6b25963 100644 --- a/tests/ui/range_plus_minus_one.stderr +++ b/tests/ui/range_plus_minus_one.stderr @@ -1,5 +1,5 @@ error: an inclusive range would be more readable - --> $DIR/range_plus_minus_one.rs:10:14 + --> $DIR/range_plus_minus_one.rs:14:14 | LL | for _ in 0..3 + 1 {} | ^^^^^^^^ help: use: `0..=3` @@ -7,25 +7,25 @@ LL | for _ in 0..3 + 1 {} = note: `-D clippy::range-plus-one` implied by `-D warnings` error: an inclusive range would be more readable - --> $DIR/range_plus_minus_one.rs:13:14 + --> $DIR/range_plus_minus_one.rs:17:14 | LL | for _ in 0..1 + 5 {} | ^^^^^^^^ help: use: `0..=5` error: an inclusive range would be more readable - --> $DIR/range_plus_minus_one.rs:16:14 + --> $DIR/range_plus_minus_one.rs:20:14 | LL | for _ in 1..1 + 1 {} | ^^^^^^^^ help: use: `1..=1` error: an inclusive range would be more readable - --> $DIR/range_plus_minus_one.rs:22:14 + --> $DIR/range_plus_minus_one.rs:26:14 | LL | for _ in 0..(1 + f()) {} | ^^^^^^^^^^^^ help: use: `0..=f()` error: an exclusive range would be more readable - --> $DIR/range_plus_minus_one.rs:26:13 + --> $DIR/range_plus_minus_one.rs:30:13 | LL | let _ = ..=11 - 1; | ^^^^^^^^^ help: use: `..11` @@ -33,19 +33,19 @@ LL | let _ = ..=11 - 1; = note: `-D clippy::range-minus-one` implied by `-D warnings` error: an exclusive range would be more readable - --> $DIR/range_plus_minus_one.rs:27:13 + --> $DIR/range_plus_minus_one.rs:31:13 | LL | let _ = ..=(11 - 1); | ^^^^^^^^^^^ help: use: `..11` error: an inclusive range would be more readable - --> $DIR/range_plus_minus_one.rs:28:13 + --> $DIR/range_plus_minus_one.rs:32:13 | LL | let _ = (1..11 + 1); | ^^^^^^^^^^^ help: use: `(1..=11)` error: an inclusive range would be more readable - --> $DIR/range_plus_minus_one.rs:29:13 + --> $DIR/range_plus_minus_one.rs:33:13 | LL | let _ = (f() + 1)..(f() + 1); | ^^^^^^^^^^^^^^^^^^^^ help: use: `((f() + 1)..=f())` diff --git a/tests/ui/short_circuit_statement.fixed b/tests/ui/short_circuit_statement.fixed new file mode 100644 index 000000000000..af0a397bd1af --- /dev/null +++ b/tests/ui/short_circuit_statement.fixed @@ -0,0 +1,18 @@ +// run-rustfix + +#![warn(clippy::short_circuit_statement)] +#![allow(clippy::nonminimal_bool)] + +fn main() { + if f() { g(); } + if !f() { g(); } + if !(1 == 2) { g(); } +} + +fn f() -> bool { + true +} + +fn g() -> bool { + false +} diff --git a/tests/ui/short_circuit_statement.rs b/tests/ui/short_circuit_statement.rs index 84e736fe080f..73a55bf1f5e2 100644 --- a/tests/ui/short_circuit_statement.rs +++ b/tests/ui/short_circuit_statement.rs @@ -1,4 +1,7 @@ +// run-rustfix + #![warn(clippy::short_circuit_statement)] +#![allow(clippy::nonminimal_bool)] fn main() { f() && g(); diff --git a/tests/ui/short_circuit_statement.stderr b/tests/ui/short_circuit_statement.stderr index a526766f6989..0a3f60c3d132 100644 --- a/tests/ui/short_circuit_statement.stderr +++ b/tests/ui/short_circuit_statement.stderr @@ -1,5 +1,5 @@ error: boolean short circuit operator in statement may be clearer using an explicit test - --> $DIR/short_circuit_statement.rs:4:5 + --> $DIR/short_circuit_statement.rs:7:5 | LL | f() && g(); | ^^^^^^^^^^^ help: replace it with: `if f() { g(); }` @@ -7,13 +7,13 @@ LL | f() && g(); = note: `-D clippy::short-circuit-statement` implied by `-D warnings` error: boolean short circuit operator in statement may be clearer using an explicit test - --> $DIR/short_circuit_statement.rs:5:5 + --> $DIR/short_circuit_statement.rs:8:5 | LL | f() || g(); | ^^^^^^^^^^^ help: replace it with: `if !f() { g(); }` error: boolean short circuit operator in statement may be clearer using an explicit test - --> $DIR/short_circuit_statement.rs:6:5 + --> $DIR/short_circuit_statement.rs:9:5 | LL | 1 == 2 || g(); | ^^^^^^^^^^^^^^ help: replace it with: `if !(1 == 2) { g(); }` diff --git a/tests/ui/slow_vector_initialization.stdout b/tests/ui/slow_vector_initialization.stdout deleted file mode 100644 index e69de29bb2d1..000000000000 diff --git a/tests/ui/ty_fn_sig.stderr b/tests/ui/ty_fn_sig.stderr deleted file mode 100644 index e69de29bb2d1..000000000000 diff --git a/tests/ui/unseparated_prefix_literals.fixed b/tests/ui/unseparated_prefix_literals.fixed index 1948b18d1639..26bc47257b79 100644 --- a/tests/ui/unseparated_prefix_literals.fixed +++ b/tests/ui/unseparated_prefix_literals.fixed @@ -3,6 +3,12 @@ #![warn(clippy::unseparated_literal_suffix)] #![allow(dead_code)] +macro_rules! lit_from_macro { + () => { + 42_usize + }; +} + fn main() { let _ok1 = 1234_i32; let _ok2 = 1234_isize; @@ -17,4 +23,12 @@ fn main() { let _okf2 = 1_f32; let _failf1 = 1.5_f32; let _failf2 = 1_f32; + + // Test for macro + let _ = lit_from_macro!(); + + // Counter example + let _ = line!(); + // Because `assert!` contains `line!()` macro. + assert_eq!(4897_u32, 32223); } diff --git a/tests/ui/unseparated_prefix_literals.rs b/tests/ui/unseparated_prefix_literals.rs index d70b1cf29f53..d710ccd1be2c 100644 --- a/tests/ui/unseparated_prefix_literals.rs +++ b/tests/ui/unseparated_prefix_literals.rs @@ -3,6 +3,12 @@ #![warn(clippy::unseparated_literal_suffix)] #![allow(dead_code)] +macro_rules! lit_from_macro { + () => { + 42usize + }; +} + fn main() { let _ok1 = 1234_i32; let _ok2 = 1234_isize; @@ -17,4 +23,12 @@ fn main() { let _okf2 = 1_f32; let _failf1 = 1.5f32; let _failf2 = 1f32; + + // Test for macro + let _ = lit_from_macro!(); + + // Counter example + let _ = line!(); + // Because `assert!` contains `line!()` macro. + assert_eq!(4897u32, 32223); } diff --git a/tests/ui/unseparated_prefix_literals.stderr b/tests/ui/unseparated_prefix_literals.stderr index 2b8121db3fc0..85f1881949eb 100644 --- a/tests/ui/unseparated_prefix_literals.stderr +++ b/tests/ui/unseparated_prefix_literals.stderr @@ -1,5 +1,5 @@ error: integer type suffix should be separated by an underscore - --> $DIR/unseparated_prefix_literals.rs:10:18 + --> $DIR/unseparated_prefix_literals.rs:16:18 | LL | let _fail1 = 1234i32; | ^^^^^^^ help: add an underscore: `1234_i32` @@ -7,40 +7,55 @@ LL | let _fail1 = 1234i32; = note: `-D clippy::unseparated-literal-suffix` implied by `-D warnings` error: integer type suffix should be separated by an underscore - --> $DIR/unseparated_prefix_literals.rs:11:18 + --> $DIR/unseparated_prefix_literals.rs:17:18 | LL | let _fail2 = 1234u32; | ^^^^^^^ help: add an underscore: `1234_u32` error: integer type suffix should be separated by an underscore - --> $DIR/unseparated_prefix_literals.rs:12:18 + --> $DIR/unseparated_prefix_literals.rs:18:18 | LL | let _fail3 = 1234isize; | ^^^^^^^^^ help: add an underscore: `1234_isize` error: integer type suffix should be separated by an underscore - --> $DIR/unseparated_prefix_literals.rs:13:18 + --> $DIR/unseparated_prefix_literals.rs:19:18 | LL | let _fail4 = 1234usize; | ^^^^^^^^^ help: add an underscore: `1234_usize` error: integer type suffix should be separated by an underscore - --> $DIR/unseparated_prefix_literals.rs:14:18 + --> $DIR/unseparated_prefix_literals.rs:20:18 | LL | let _fail5 = 0x123isize; | ^^^^^^^^^^ help: add an underscore: `0x123_isize` error: float type suffix should be separated by an underscore - --> $DIR/unseparated_prefix_literals.rs:18:19 + --> $DIR/unseparated_prefix_literals.rs:24:19 | LL | let _failf1 = 1.5f32; | ^^^^^^ help: add an underscore: `1.5_f32` error: float type suffix should be separated by an underscore - --> $DIR/unseparated_prefix_literals.rs:19:19 + --> $DIR/unseparated_prefix_literals.rs:25:19 | LL | let _failf2 = 1f32; | ^^^^ help: add an underscore: `1_f32` -error: aborting due to 7 previous errors +error: integer type suffix should be separated by an underscore + --> $DIR/unseparated_prefix_literals.rs:8:9 + | +LL | 42usize + | ^^^^^^^ help: add an underscore: `42_usize` +... +LL | let _ = lit_from_macro!(); + | ----------------- in this macro invocation + +error: integer type suffix should be separated by an underscore + --> $DIR/unseparated_prefix_literals.rs:33:16 + | +LL | assert_eq!(4897u32, 32223); + | ^^^^^^^ help: add an underscore: `4897_u32` + +error: aborting due to 9 previous errors diff --git a/tests/ui/unused_unit.fixed b/tests/ui/unused_unit.fixed index 3c9e91a19f9a..17c1a5de5973 100644 --- a/tests/ui/unused_unit.fixed +++ b/tests/ui/unused_unit.fixed @@ -10,6 +10,7 @@ #![rustfmt::skip] #![deny(clippy::unused_unit)] +#![allow(dead_code)] struct Unitter; impl Unitter { @@ -42,3 +43,16 @@ fn main() { } return; } + +// https://github.com/rust-lang/rust-clippy/issues/4076 +fn foo() { + macro_rules! foo { + (recv($r:expr) -> $res:pat => $body:expr) => { + $body + } + } + + foo! { + recv(rx) -> _x => () + } +} diff --git a/tests/ui/unused_unit.rs b/tests/ui/unused_unit.rs index 1acd427be1ee..e04c52573375 100644 --- a/tests/ui/unused_unit.rs +++ b/tests/ui/unused_unit.rs @@ -10,6 +10,7 @@ #![rustfmt::skip] #![deny(clippy::unused_unit)] +#![allow(dead_code)] struct Unitter; impl Unitter { @@ -43,3 +44,16 @@ fn main() { } return(); } + +// https://github.com/rust-lang/rust-clippy/issues/4076 +fn foo() { + macro_rules! foo { + (recv($r:expr) -> $res:pat => $body:expr) => { + $body + } + } + + foo! { + recv(rx) -> _x => () + } +} diff --git a/tests/ui/unused_unit.stderr b/tests/ui/unused_unit.stderr index c33a220b98c0..6ef6dc4f5d6c 100644 --- a/tests/ui/unused_unit.stderr +++ b/tests/ui/unused_unit.stderr @@ -1,5 +1,5 @@ error: unneeded unit return type - --> $DIR/unused_unit.rs:18:59 + --> $DIR/unused_unit.rs:19:59 | LL | pub fn get_unit (), G>(&self, f: F, _g: G) -> | ___________________________________________________________^ @@ -13,37 +13,37 @@ LL | #![deny(clippy::unused_unit)] | ^^^^^^^^^^^^^^^^^^^ error: unneeded unit return type - --> $DIR/unused_unit.rs:28:19 + --> $DIR/unused_unit.rs:29:19 | LL | fn into(self) -> () { | ^^^^^ help: remove the `-> ()` error: unneeded unit expression - --> $DIR/unused_unit.rs:29:9 + --> $DIR/unused_unit.rs:30:9 | LL | () | ^^ help: remove the final `()` error: unneeded unit return type - --> $DIR/unused_unit.rs:33:18 + --> $DIR/unused_unit.rs:34:18 | LL | fn return_unit() -> () { () } | ^^^^^ help: remove the `-> ()` error: unneeded unit expression - --> $DIR/unused_unit.rs:33:26 + --> $DIR/unused_unit.rs:34:26 | LL | fn return_unit() -> () { () } | ^^ help: remove the final `()` error: unneeded `()` - --> $DIR/unused_unit.rs:42:14 + --> $DIR/unused_unit.rs:43:14 | LL | break(); | ^^ help: remove the `()` error: unneeded `()` - --> $DIR/unused_unit.rs:44:11 + --> $DIR/unused_unit.rs:45:11 | LL | return(); | ^^ help: remove the `()` diff --git a/tests/ui/unwrap_or.stdout b/tests/ui/unwrap_or.stdout deleted file mode 100644 index e69de29bb2d1..000000000000 diff --git a/util/fetch_prs_between.sh b/util/fetch_prs_between.sh index 36b4785ac87e..bea5a021aefd 100755 --- a/util/fetch_prs_between.sh +++ b/util/fetch_prs_between.sh @@ -11,7 +11,7 @@ last=$2 IFS=' ' -for pr in $(git log --oneline --grep "Merge #" --grep "Merge pull request" --grep "Auto merge of" "$first...$last" | sort -rn | uniq); do +for pr in $(git log --oneline --grep "Merge #" --grep "Merge pull request" --grep "Auto merge of" --grep "Rollup merge of" "$first...$last" | sort -rn | uniq); do id=$(echo $pr | rg -o '#[0-9]{3,5}' | cut -c 2-) commit=$(echo $pr | cut -d' ' -f 1) message=$(git --no-pager show --pretty=medium $commit)