diff options
51 files changed, 735 insertions, 127 deletions
diff --git a/CHANGELOG.md b/CHANGELOG.md index 10111be596a..35864e85673 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4200,6 +4200,7 @@ Released 2018-09-13 [`same_item_push`]: https://rust-lang.github.io/rust-clippy/master/index.html#same_item_push [`same_name_method`]: https://rust-lang.github.io/rust-clippy/master/index.html#same_name_method [`search_is_some`]: https://rust-lang.github.io/rust-clippy/master/index.html#search_is_some +[`seek_from_current`]: https://rust-lang.github.io/rust-clippy/master/index.html#seek_from_current [`seek_to_start_instead_of_rewind`]: https://rust-lang.github.io/rust-clippy/master/index.html#seek_to_start_instead_of_rewind [`self_assignment`]: https://rust-lang.github.io/rust-clippy/master/index.html#self_assignment [`self_named_constructors`]: https://rust-lang.github.io/rust-clippy/master/index.html#self_named_constructors diff --git a/README.md b/README.md index a8a6b86d2a1..f74de7de42b 100644 --- a/README.md +++ b/README.md @@ -1,6 +1,6 @@ # Clippy -[](https://github.com/rust-lang/rust-clippy/actions?query=workflow%3A%22Clippy+Test%22+event%3Apush+branch%3Aauto) +[/badge.svg?branch=auto&event=push)](https://github.com/rust-lang/rust-clippy/actions?query=workflow%3A%22Clippy+Test+(bors)%22+event%3Apush+branch%3Aauto) [](#license) A collection of lints to catch common mistakes and improve your [Rust](https://github.com/rust-lang/rust) code. @@ -204,12 +204,6 @@ lints can be configured and the meaning of the variables. > > `clippy.toml` or `.clippy.toml` cannot be used to allow/deny lints. -> **Note** -> -> Configuration changes will not apply for code that has already been compiled and cached under `./target/`; -> for example, adding a new string to `doc-valid-idents` may still result in Clippy flagging that string. To be sure -> that any configuration changes are applied, you may want to run `cargo clean` and re-compile your crate from scratch. - To deactivate the “for further information visit *lint-link*” message you can define the `CLIPPY_DISABLE_DOCS_LINKS` environment variable. diff --git a/clippy_lints/src/attrs.rs b/clippy_lints/src/attrs.rs index bed9cf565f3..235ab4977ce 100644 --- a/clippy_lints/src/attrs.rs +++ b/clippy_lints/src/attrs.rs @@ -11,14 +11,14 @@ use rustc_errors::Applicability; use rustc_hir::{ Block, Expr, ExprKind, ImplItem, ImplItemKind, Item, ItemKind, StmtKind, TraitFn, TraitItem, TraitItemKind, }; -use rustc_lint::{EarlyContext, EarlyLintPass, LateContext, LateLintPass, LintContext}; +use rustc_lint::{EarlyContext, EarlyLintPass, LateContext, LateLintPass, Level, LintContext}; use rustc_middle::lint::in_external_macro; use rustc_middle::ty; use rustc_semver::RustcVersion; use rustc_session::{declare_lint_pass, declare_tool_lint, impl_lint_pass}; use rustc_span::source_map::Span; -use rustc_span::sym; use rustc_span::symbol::Symbol; +use rustc_span::{sym, DUMMY_SP}; use semver::Version; static UNIX_SYSTEMS: &[&str] = &[ @@ -303,6 +303,26 @@ declare_lint_pass!(Attributes => [ ]); impl<'tcx> LateLintPass<'tcx> for Attributes { + fn check_crate(&mut self, cx: &LateContext<'tcx>) { + for (name, level) in &cx.sess().opts.lint_opts { + if name == "clippy::restriction" && *level > Level::Allow { + span_lint_and_then( + cx, + BLANKET_CLIPPY_RESTRICTION_LINTS, + DUMMY_SP, + "`clippy::restriction` is not meant to be enabled as a group", + |diag| { + diag.note(format!( + "because of the command line `--{} clippy::restriction`", + level.as_str() + )); + diag.help("enable the restriction lints you need individually"); + }, + ); + } + } + } + fn check_attribute(&mut self, cx: &LateContext<'tcx>, attr: &'tcx Attribute) { if let Some(items) = &attr.meta_item_list() { if let Some(ident) = attr.ident() { @@ -441,9 +461,9 @@ fn check_clippy_lint_names(cx: &LateContext<'_>, name: Symbol, items: &[NestedMe cx, BLANKET_CLIPPY_RESTRICTION_LINTS, lint.span(), - "restriction lints are not meant to be all enabled", + "`clippy::restriction` is not meant to be enabled as a group", None, - "try enabling only the lints you really need", + "enable the restriction lints you need individually", ); } } diff --git a/clippy_lints/src/bool_to_int_with_if.rs b/clippy_lints/src/bool_to_int_with_if.rs index 001d74c2605..40ded405669 100644 --- a/clippy_lints/src/bool_to_int_with_if.rs +++ b/clippy_lints/src/bool_to_int_with_if.rs @@ -1,9 +1,10 @@ +use clippy_utils::higher::If; use rustc_ast::LitKind; use rustc_hir::{Block, ExprKind}; use rustc_lint::{LateContext, LateLintPass}; use rustc_session::{declare_lint_pass, declare_tool_lint}; -use clippy_utils::{diagnostics::span_lint_and_then, is_else_clause, is_integer_literal, sugg::Sugg}; +use clippy_utils::{diagnostics::span_lint_and_then, in_constant, is_else_clause, is_integer_literal, sugg::Sugg}; use rustc_errors::Applicability; declare_clippy_lint! { @@ -44,17 +45,17 @@ declare_clippy_lint! { declare_lint_pass!(BoolToIntWithIf => [BOOL_TO_INT_WITH_IF]); impl<'tcx> LateLintPass<'tcx> for BoolToIntWithIf { - fn check_expr(&mut self, ctx: &LateContext<'tcx>, expr: &'tcx rustc_hir::Expr<'tcx>) { - if !expr.span.from_expansion() { - check_if_else(ctx, expr); + fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx rustc_hir::Expr<'tcx>) { + if !expr.span.from_expansion() && !in_constant(cx, expr.hir_id) { + check_if_else(cx, expr); } } } -fn check_if_else<'tcx>(ctx: &LateContext<'tcx>, expr: &'tcx rustc_hir::Expr<'tcx>) { - if let ExprKind::If(check, then, Some(else_)) = expr.kind +fn check_if_else<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx rustc_hir::Expr<'tcx>) { + if let Some(If { cond, then, r#else: Some(r#else) }) = If::hir(expr) && let Some(then_lit) = int_literal(then) - && let Some(else_lit) = int_literal(else_) + && let Some(else_lit) = int_literal(r#else) { let inverted = if is_integer_literal(then_lit, 1) && is_integer_literal(else_lit, 0) { false @@ -66,17 +67,17 @@ fn check_if_else<'tcx>(ctx: &LateContext<'tcx>, expr: &'tcx rustc_hir::Expr<'tcx }; let mut applicability = Applicability::MachineApplicable; let snippet = { - let mut sugg = Sugg::hir_with_applicability(ctx, check, "..", &mut applicability); + let mut sugg = Sugg::hir_with_applicability(cx, cond, "..", &mut applicability); if inverted { sugg = !sugg; } sugg }; - let ty = ctx.typeck_results().expr_ty(then_lit); // then and else must be of same type + let ty = cx.typeck_results().expr_ty(then_lit); // then and else must be of same type let suggestion = { - let wrap_in_curly = is_else_clause(ctx.tcx, expr); + let wrap_in_curly = is_else_clause(cx.tcx, expr); let mut s = Sugg::NonParen(format!("{ty}::from({snippet})").into()); if wrap_in_curly { s = s.blockify(); @@ -87,7 +88,7 @@ fn check_if_else<'tcx>(ctx: &LateContext<'tcx>, expr: &'tcx rustc_hir::Expr<'tcx let into_snippet = snippet.clone().maybe_par(); let as_snippet = snippet.as_ty(ty); - span_lint_and_then(ctx, + span_lint_and_then(cx, BOOL_TO_INT_WITH_IF, expr.span, "boolean to int conversion using if", diff --git a/clippy_lints/src/declared_lints.rs b/clippy_lints/src/declared_lints.rs index 35f86e8aa78..f4fc2bd3330 100644 --- a/clippy_lints/src/declared_lints.rs +++ b/clippy_lints/src/declared_lints.rs @@ -363,6 +363,7 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[ crate::methods::REPEAT_ONCE_INFO, crate::methods::RESULT_MAP_OR_INTO_OPTION_INFO, crate::methods::SEARCH_IS_SOME_INFO, + crate::methods::SEEK_FROM_CURRENT_INFO, crate::methods::SEEK_TO_START_INSTEAD_OF_REWIND_INFO, crate::methods::SHOULD_IMPLEMENT_TRAIT_INFO, crate::methods::SINGLE_CHAR_ADD_STR_INFO, diff --git a/clippy_lints/src/dereference.rs b/clippy_lints/src/dereference.rs index 7b5c10db4e1..4d2f5eea105 100644 --- a/clippy_lints/src/dereference.rs +++ b/clippy_lints/src/dereference.rs @@ -9,6 +9,7 @@ use clippy_utils::{ }; use rustc_ast::util::parser::{PREC_POSTFIX, PREC_PREFIX}; use rustc_data_structures::fx::FxIndexMap; +use rustc_data_structures::graph::iterate::{CycleDetector, TriColorDepthFirstSearch}; use rustc_errors::Applicability; use rustc_hir::intravisit::{walk_ty, Visitor}; use rustc_hir::{ @@ -1049,7 +1050,7 @@ fn ty_contains_infer(ty: &hir::Ty<'_>) -> bool { // If the conditions are met, returns `Some(Position::ImplArg(..))`; otherwise, returns `None`. // The "is copyable" condition is to avoid the case where removing the `&` means `e` would have to // be moved, but it cannot be. -#[expect(clippy::too_many_arguments)] +#[expect(clippy::too_many_arguments, clippy::too_many_lines)] fn needless_borrow_impl_arg_position<'tcx>( cx: &LateContext<'tcx>, possible_borrowers: &mut Vec<(LocalDefId, PossibleBorrowerMap<'tcx, 'tcx>)>, @@ -1092,7 +1093,7 @@ fn needless_borrow_impl_arg_position<'tcx>( .iter() .filter_map(|predicate| { if let PredicateKind::Trait(trait_predicate) = predicate.kind().skip_binder() - && trait_predicate.trait_ref.self_ty() == param_ty.to_ty(cx.tcx) + && trait_predicate.self_ty() == param_ty.to_ty(cx.tcx) { Some(trait_predicate.trait_ref.def_id) } else { @@ -1111,6 +1112,16 @@ fn needless_borrow_impl_arg_position<'tcx>( return Position::Other(precedence); } + // See: + // - https://github.com/rust-lang/rust-clippy/pull/9674#issuecomment-1289294201 + // - https://github.com/rust-lang/rust-clippy/pull/9674#issuecomment-1292225232 + if projection_predicates + .iter() + .any(|projection_predicate| is_mixed_projection_predicate(cx, callee_def_id, projection_predicate)) + { + return Position::Other(precedence); + } + // `substs_with_referent_ty` can be constructed outside of `check_referent` because the same // elements are modified each time `check_referent` is called. let mut substs_with_referent_ty = substs_with_expr_ty.to_vec(); @@ -1190,8 +1201,39 @@ fn has_ref_mut_self_method(cx: &LateContext<'_>, trait_def_id: DefId) -> bool { }) } -fn referent_used_exactly_once<'tcx>( +fn is_mixed_projection_predicate<'tcx>( cx: &LateContext<'tcx>, + callee_def_id: DefId, + projection_predicate: &ProjectionPredicate<'tcx>, +) -> bool { + let generics = cx.tcx.generics_of(callee_def_id); + // The predicate requires the projected type to equal a type parameter from the parent context. + if let Some(term_ty) = projection_predicate.term.ty() + && let ty::Param(term_param_ty) = term_ty.kind() + && (term_param_ty.index as usize) < generics.parent_count + { + // The inner-most self type is a type parameter from the current function. + let mut projection_ty = projection_predicate.projection_ty; + loop { + match projection_ty.self_ty().kind() { + ty::Projection(inner_projection_ty) => { + projection_ty = *inner_projection_ty; + } + ty::Param(param_ty) => { + return (param_ty.index as usize) >= generics.parent_count; + } + _ => { + return false; + } + } + } + } else { + false + } +} + +fn referent_used_exactly_once<'a, 'tcx>( + cx: &'a LateContext<'tcx>, possible_borrowers: &mut Vec<(LocalDefId, PossibleBorrowerMap<'tcx, 'tcx>)>, reference: &Expr<'tcx>, ) -> bool { @@ -1201,6 +1243,8 @@ fn referent_used_exactly_once<'tcx>( && let Some(statement) = mir.basic_blocks[location.block].statements.get(location.statement_index) && let StatementKind::Assign(box (_, Rvalue::Ref(_, _, place))) = statement.kind && !place.has_deref() + // Ensure not in a loop (https://github.com/rust-lang/rust-clippy/issues/9710) + && TriColorDepthFirstSearch::new(&mir.basic_blocks).run_from(location.block, &mut CycleDetector).is_none() { let body_owner_local_def_id = cx.tcx.hir().enclosing_body_owner(reference.hir_id); if possible_borrowers diff --git a/clippy_lints/src/format_args.rs b/clippy_lints/src/format_args.rs index 4c4a1e06cd4..a73453bf027 100644 --- a/clippy_lints/src/format_args.rs +++ b/clippy_lints/src/format_args.rs @@ -115,7 +115,7 @@ declare_clippy_lint! { /// nothing will be suggested, e.g. `println!("{0}={1}", var, 1+2)`. #[clippy::version = "1.65.0"] pub UNINLINED_FORMAT_ARGS, - style, + pedantic, "using non-inlined variables in `format!` calls" } diff --git a/clippy_lints/src/matches/significant_drop_in_scrutinee.rs b/clippy_lints/src/matches/significant_drop_in_scrutinee.rs index 85269e533a0..93e4160c69b 100644 --- a/clippy_lints/src/matches/significant_drop_in_scrutinee.rs +++ b/clippy_lints/src/matches/significant_drop_in_scrutinee.rs @@ -226,7 +226,7 @@ impl<'a, 'tcx> SigDropHelper<'a, 'tcx> { /// This will try to set the current suggestion (so it can be moved into the suggestions vec /// later). If `allow_move_and_clone` is false, the suggestion *won't* be set -- this gives us /// an opportunity to look for another type in the chain that will be trivially copyable. - /// However, if we are at the the end of the chain, we want to accept whatever is there. (The + /// However, if we are at the end of the chain, we want to accept whatever is there. (The /// suggestion won't actually be output, but the diagnostic message will be output, so the user /// can determine the best way to handle the lint.) fn try_setting_current_suggestion(&mut self, expr: &'tcx Expr<'_>, allow_move_and_clone: bool) { diff --git a/clippy_lints/src/methods/mod.rs b/clippy_lints/src/methods/mod.rs index 4fd1e3e54ae..6665329d114 100644 --- a/clippy_lints/src/methods/mod.rs +++ b/clippy_lints/src/methods/mod.rs @@ -69,6 +69,7 @@ mod path_buf_push_overwrite; mod range_zip_with_len; mod repeat_once; mod search_is_some; +mod seek_from_current; mod seek_to_start_instead_of_rewind; mod single_char_add_str; mod single_char_insert_string; @@ -102,7 +103,7 @@ mod zst_offset; use bind_instead_of_map::BindInsteadOfMap; use clippy_utils::consts::{constant, Constant}; use clippy_utils::diagnostics::{span_lint, span_lint_and_help}; -use clippy_utils::ty::{contains_adt_constructor, implements_trait, is_copy, is_type_diagnostic_item}; +use clippy_utils::ty::{contains_ty_adt_constructor_opaque, implements_trait, is_copy, is_type_diagnostic_item}; use clippy_utils::{contains_return, is_trait_method, iter_input_pats, meets_msrv, msrvs, return_ty}; use if_chain::if_chain; use rustc_hir as hir; @@ -3070,6 +3071,49 @@ declare_clippy_lint! { declare_clippy_lint! { /// ### What it does /// + /// Checks an argument of `seek` method of `Seek` trait + /// and if it start seek from `SeekFrom::Current(0)`, suggests `stream_position` instead. + /// + /// ### Why is this bad? + /// + /// Readability. Use dedicated method. + /// + /// ### Example + /// + /// ```rust + /// use std::fs::File; + /// use std::io::{self, Write, Seek, SeekFrom}; + /// + /// fn main() -> io::Result<()> { + /// let mut f = File::create("foo.txt")?; + /// f.write_all(b"Hello")?; + /// eprintln!("Written {} bytes", f.seek(SeekFrom::Current(0))?); + /// + /// Ok(()) + /// } + /// ``` + /// Use instead: + /// ```rust + /// use std::fs::File; + /// use std::io::{self, Write, Seek, SeekFrom}; + /// + /// fn main() -> io::Result<()> { + /// let mut f = File::create("foo.txt")?; + /// f.write_all(b"Hello")?; + /// eprintln!("Written {} bytes", f.stream_position()?); + /// + /// Ok(()) + /// } + /// ``` + #[clippy::version = "1.66.0"] + pub SEEK_FROM_CURRENT, + complexity, + "use dedicated method for seek from current position" +} + +declare_clippy_lint! { + /// ### What it does + /// /// Checks for jumps to the start of a stream that implements `Seek` /// and uses the `seek` method providing `Start` as parameter. /// @@ -3222,6 +3266,7 @@ impl_lint_pass!(Methods => [ VEC_RESIZE_TO_ZERO, VERBOSE_FILE_READS, ITER_KV_MAP, + SEEK_FROM_CURRENT, SEEK_TO_START_INSTEAD_OF_REWIND, ]); @@ -3349,36 +3394,10 @@ impl<'tcx> LateLintPass<'tcx> for Methods { if let hir::ImplItemKind::Fn(_, _) = impl_item.kind { let ret_ty = return_ty(cx, impl_item.hir_id()); - // walk the return type and check for Self (this does not check associated types) - if let Some(self_adt) = self_ty.ty_adt_def() { - if contains_adt_constructor(ret_ty, self_adt) { - return; - } - } else if ret_ty.contains(self_ty) { + if contains_ty_adt_constructor_opaque(cx, ret_ty, self_ty) { return; } - // if return type is impl trait, check the associated types - if let ty::Opaque(def_id, _) = *ret_ty.kind() { - // one of the associated types must be Self - for &(predicate, _span) in cx.tcx.explicit_item_bounds(def_id) { - if let ty::PredicateKind::Projection(projection_predicate) = predicate.kind().skip_binder() { - let assoc_ty = match projection_predicate.term.unpack() { - ty::TermKind::Ty(ty) => ty, - ty::TermKind::Const(_c) => continue, - }; - // walk the associated type and check for Self - if let Some(self_adt) = self_ty.ty_adt_def() { - if contains_adt_constructor(assoc_ty, self_adt) { - return; - } - } else if assoc_ty.contains(self_ty) { - return; - } - } - } - } - if name == "new" && ret_ty != self_ty { span_lint( cx, @@ -3638,6 +3657,9 @@ impl Methods { vec_resize_to_zero::check(cx, expr, count_arg, default_arg, span); }, ("seek", [arg]) => { + if meets_msrv(self.msrv, msrvs::SEEK_FROM_CURRENT) { + seek_from_current::check(cx, expr, recv, arg); + } if meets_msrv(self.msrv, msrvs::SEEK_REWIND) { seek_to_start_instead_of_rewind::check(cx, expr, recv, arg, span); } diff --git a/clippy_lints/src/methods/seek_from_current.rs b/clippy_lints/src/methods/seek_from_current.rs new file mode 100644 index 00000000000..361a3082f94 --- /dev/null +++ b/clippy_lints/src/methods/seek_from_current.rs @@ -0,0 +1,48 @@ +use rustc_ast::ast::{LitIntType, LitKind}; +use rustc_errors::Applicability; +use rustc_hir::{Expr, ExprKind}; +use rustc_lint::LateContext; + +use clippy_utils::{ + diagnostics::span_lint_and_sugg, get_trait_def_id, match_def_path, paths, source::snippet_with_applicability, + ty::implements_trait, +}; + +use super::SEEK_FROM_CURRENT; + +pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>, recv: &'tcx Expr<'_>, arg: &'tcx Expr<'_>) { + let ty = cx.typeck_results().expr_ty(recv); + + if let Some(def_id) = get_trait_def_id(cx, &paths::STD_IO_SEEK) { + if implements_trait(cx, ty, def_id, &[]) && arg_is_seek_from_current(cx, arg) { + let mut applicability = Applicability::MachineApplicable; + let snip = snippet_with_applicability(cx, recv.span, "..", &mut applicability); + + span_lint_and_sugg( + cx, + SEEK_FROM_CURRENT, + expr.span, + "using `SeekFrom::Current` to start from current position", + "replace with", + format!("{snip}.stream_position()"), + applicability, + ); + } + } +} + +fn arg_is_seek_from_current<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) -> bool { + if let ExprKind::Call(f, args) = expr.kind && + let ExprKind::Path(ref path) = f.kind && + let Some(def_id) = cx.qpath_res(path, f.hir_id).opt_def_id() && + match_def_path(cx, def_id, &paths::STD_IO_SEEK_FROM_CURRENT) { + // check if argument of `SeekFrom::Current` is `0` + if args.len() == 1 && + let ExprKind::Lit(ref lit) = args[0].kind && + let LitKind::Int(0, LitIntType::Unsuffixed) = lit.node { + return true + } + } + + false +} diff --git a/clippy_lints/src/methods/string_extend_chars.rs b/clippy_lints/src/methods/string_extend_chars.rs index 6974260f70d..0e93543fd7e 100644 --- a/clippy_lints/src/methods/string_extend_chars.rs +++ b/clippy_lints/src/methods/string_extend_chars.rs @@ -19,7 +19,11 @@ pub(super) fn check(cx: &LateContext<'_>, expr: &hir::Expr<'_>, recv: &hir::Expr let target = &arglists[0].0; let self_ty = cx.typeck_results().expr_ty(target).peel_refs(); let ref_str = if *self_ty.kind() == ty::Str { - "" + if matches!(target.kind, hir::ExprKind::Index(..)) { + "&" + } else { + "" + } } else if is_type_diagnostic_item(cx, self_ty, sym::String) { "&" } else { diff --git a/clippy_lints/src/option_if_let_else.rs b/clippy_lints/src/option_if_let_else.rs index 4eb42da1fed..472f52380bb 100644 --- a/clippy_lints/src/option_if_let_else.rs +++ b/clippy_lints/src/option_if_let_else.rs @@ -213,11 +213,14 @@ fn try_convert_match<'tcx>( cx: &LateContext<'tcx>, arms: &[Arm<'tcx>], ) -> Option<(&'tcx Pat<'tcx>, &'tcx Expr<'tcx>, &'tcx Expr<'tcx>)> { - if arms.len() == 2 { - return if is_none_or_err_arm(cx, &arms[1]) { - Some((arms[0].pat, arms[0].body, arms[1].body)) - } else if is_none_or_err_arm(cx, &arms[0]) { - Some((arms[1].pat, arms[1].body, arms[0].body)) + if let [first_arm, second_arm] = arms + && first_arm.guard.is_none() + && second_arm.guard.is_none() + { + return if is_none_or_err_arm(cx, second_arm) { + Some((first_arm.pat, first_arm.body, second_arm.body)) + } else if is_none_or_err_arm(cx, first_arm) { + Some((second_arm.pat, second_arm.body, first_arm.body)) } else { None }; diff --git a/clippy_lints/src/question_mark.rs b/clippy_lints/src/question_mark.rs index 328371fd602..5644c2f72db 100644 --- a/clippy_lints/src/question_mark.rs +++ b/clippy_lints/src/question_mark.rs @@ -189,6 +189,7 @@ fn is_early_return(smbl: Symbol, cx: &LateContext<'_>, if_block: &IfBlockType<'_ && expr_return_none_or_err(smbl, cx, if_else.unwrap(), let_expr, Some(let_pat_sym))) || is_res_lang_ctor(cx, res, ResultErr) && expr_return_none_or_err(smbl, cx, if_then, let_expr, Some(let_pat_sym)) + && if_else.is_none() }, _ => false, } diff --git a/clippy_lints/src/undocumented_unsafe_blocks.rs b/clippy_lints/src/undocumented_unsafe_blocks.rs index d2e675a783e..e8f15a44473 100644 --- a/clippy_lints/src/undocumented_unsafe_blocks.rs +++ b/clippy_lints/src/undocumented_unsafe_blocks.rs @@ -68,7 +68,8 @@ impl LateLintPass<'_> for UndocumentedUnsafeBlocks { && !in_external_macro(cx.tcx.sess, block.span) && !is_lint_allowed(cx, UNDOCUMENTED_UNSAFE_BLOCKS, block.hir_id) && !is_unsafe_from_proc_macro(cx, block.span) - && !block_has_safety_comment(cx, block) + && !block_has_safety_comment(cx, block.span) + && !block_parents_have_safety_comment(cx, block.hir_id) { let source_map = cx.tcx.sess.source_map(); let span = if source_map.is_multiline(block.span) { @@ -126,8 +127,41 @@ fn is_unsafe_from_proc_macro(cx: &LateContext<'_>, span: Span) -> bool { .map_or(true, |src| !src.starts_with("unsafe")) } +// Checks if any parent {expression, statement, block, local, const, static} +// has a safety comment +fn block_parents_have_safety_comment(cx: &LateContext<'_>, id: hir::HirId) -> bool { + if let Some(node) = get_parent_node(cx.tcx, id) { + return match node { + Node::Expr(expr) => !is_branchy(expr) && span_in_body_has_safety_comment(cx, expr.span), + Node::Stmt(hir::Stmt { + kind: + hir::StmtKind::Local(hir::Local { span, .. }) + | hir::StmtKind::Expr(hir::Expr { span, .. }) + | hir::StmtKind::Semi(hir::Expr { span, .. }), + .. + }) + | Node::Local(hir::Local { span, .. }) + | Node::Item(hir::Item { + kind: hir::ItemKind::Const(..) | ItemKind::Static(..), + span, + .. + }) => span_in_body_has_safety_comment(cx, *span), + _ => false, + }; + } + false +} + +/// Checks if an expression is "branchy", e.g. loop, match/if/etc. +fn is_branchy(expr: &hir::Expr<'_>) -> bool { + matches!( + expr.kind, + hir::ExprKind::If(..) | hir::ExprKind::Loop(..) | hir::ExprKind::Match(..) + ) +} + /// Checks if the lines immediately preceding the block contain a safety comment. -fn block_has_safety_comment(cx: &LateContext<'_>, block: &hir::Block<'_>) -> bool { +fn block_has_safety_comment(cx: &LateContext<'_>, span: Span) -> bool { // This intentionally ignores text before the start of a function so something like: // ``` // // SAFETY: reason @@ -136,7 +170,7 @@ fn block_has_safety_comment(cx: &LateContext<'_>, block: &hir::Block<'_>) -> boo // won't work. This is to avoid dealing with where such a comment should be place relative to // attributes and doc comments. - span_from_macro_expansion_has_safety_comment(cx, block.span) || span_in_body_has_safety_comment(cx, block.span) + span_from_macro_expansion_has_safety_comment(cx, span) || span_in_body_has_safety_comment(cx, span) } /// Checks if the lines immediately preceding the item contain a safety comment. diff --git a/clippy_lints/src/use_self.rs b/clippy_lints/src/use_self.rs index acf27c34e94..464ffdc0a2a 100644 --- a/clippy_lints/src/use_self.rs +++ b/clippy_lints/src/use_self.rs @@ -30,7 +30,6 @@ declare_clippy_lint! { /// /// ### Known problems /// - Unaddressed false negative in fn bodies of trait implementations - /// - False positive with associated types in traits (#4140) /// /// ### Example /// ```rust @@ -235,24 +234,13 @@ impl<'tcx> LateLintPass<'tcx> for UseSelf { then {} else { return; } } match expr.kind { - ExprKind::Struct(QPath::Resolved(_, path), ..) => match path.res { - Res::SelfTyParam { .. } | Res::SelfTyAlias { .. } => (), - Res::Def(DefKind::Variant, _) => lint_path_to_variant(cx, path), - _ => span_lint(cx, path.span), - }, - // tuple struct instantiation (`Foo(arg)` or `Enum::Foo(arg)`) + ExprKind::Struct(QPath::Resolved(_, path), ..) => check_path(cx, path), ExprKind::Call(fun, _) => { if let ExprKind::Path(QPath::Resolved(_, path)) = fun.kind { - if let Res::Def(DefKind::Ctor(ctor_of, _), ..) = path.res { - match ctor_of { - CtorOf::Variant => lint_path_to_variant(cx, path), - CtorOf::Struct => span_lint(cx, path.span), - } - } + check_path(cx, path); } }, - // unit enum variants (`Enum::A`) - ExprKind::Path(QPath::Resolved(_, path)) => lint_path_to_variant(cx, path), + ExprKind::Path(QPath::Resolved(_, path)) => check_path(cx, path), _ => (), } } @@ -268,15 +256,7 @@ impl<'tcx> LateLintPass<'tcx> for UseSelf { | PatKind::Struct(QPath::Resolved(_, path), _, _) = pat.kind; if cx.typeck_results().pat_ty(pat) == cx.tcx.type_of(impl_id); then { - match path.res { - Res::Def(DefKind::Ctor(ctor_of, _), ..) => match ctor_of { - CtorOf::Variant => lint_path_to_variant(cx, path), - CtorOf::Struct => span_lint(cx, path.span), - }, - Res::Def(DefKind::Variant, ..) => lint_path_to_variant(cx, path), - Res::Def(DefKind::Struct, ..) => span_lint(cx, path.span), - _ => () - } + check_path(cx, path); } } } @@ -314,6 +294,16 @@ fn span_lint(cx: &LateContext<'_>, span: Span) { ); } +fn check_path(cx: &LateContext<'_>, path: &Path<'_>) { + match path.res { + Res::Def(DefKind::Ctor(CtorOf::Variant, _) | DefKind::Variant, ..) => { + lint_path_to_variant(cx, path); + }, + Res::Def(DefKind::Ctor(CtorOf::Struct, _) | DefKind::Struct, ..) => span_lint(cx, path.span), + _ => (), + } +} + fn lint_path_to_variant(cx: &LateContext<'_>, path: &Path<'_>) { if let [.., self_seg, _variant] = path.segments { let span = path diff --git a/clippy_utils/src/msrvs.rs b/clippy_utils/src/msrvs.rs index 04b504d044d..9a672e2ddfd 100644 --- a/clippy_utils/src/msrvs.rs +++ b/clippy_utils/src/msrvs.rs @@ -17,7 +17,7 @@ msrv_aliases! { 1,58,0 { FORMAT_ARGS_CAPTURE } 1,53,0 { OR_PATTERNS, MANUAL_BITS, BTREE_MAP_RETAIN, BTREE_SET_RETAIN, ARRAY_INTO_ITERATOR } 1,52,0 { STR_SPLIT_ONCE, REM_EUCLID_CONST } - 1,51,0 { BORROW_AS_PTR, UNSIGNED_ABS } + 1,51,0 { BORROW_AS_PTR, SEEK_FROM_CURRENT, UNSIGNED_ABS } 1,50,0 { BOOL_THEN, CLAMP } 1,47,0 { TAU } 1,46,0 { CONST_IF_MATCH } diff --git a/clippy_utils/src/paths.rs b/clippy_utils/src/paths.rs index e37c7e34c0c..6c09c146082 100644 --- a/clippy_utils/src/paths.rs +++ b/clippy_utils/src/paths.rs @@ -116,6 +116,7 @@ pub const STDOUT: [&str; 4] = ["std", "io", "stdio", "stdout"]; pub const CONVERT_IDENTITY: [&str; 3] = ["core", "convert", "identity"]; pub const STD_FS_CREATE_DIR: [&str; 3] = ["std", "fs", "create_dir"]; pub const STD_IO_SEEK: [&str; 3] = ["std", "io", "Seek"]; +pub const STD_IO_SEEK_FROM_CURRENT: [&str; 4] = ["std", "io", "SeekFrom", "Current"]; pub const STD_IO_SEEKFROM_START: [&str; 4] = ["std", "io", "SeekFrom", "Start"]; pub const STRING_AS_MUT_STR: [&str; 4] = ["alloc", "string", "String", "as_mut_str"]; pub const STRING_AS_STR: [&str; 4] = ["alloc", "string", "String", "as_str"]; diff --git a/clippy_utils/src/ty.rs b/clippy_utils/src/ty.rs index a15daec7c3c..2dd8c826ab3 100644 --- a/clippy_utils/src/ty.rs +++ b/clippy_utils/src/ty.rs @@ -59,6 +59,58 @@ pub fn contains_adt_constructor<'tcx>(ty: Ty<'tcx>, adt: AdtDef<'tcx>) -> bool { }) } +/// Walks into `ty` and returns `true` if any inner type is an instance of the given type, or adt +/// constructor of the same type. +/// +/// This method also recurses into opaque type predicates, so call it with `impl Trait<U>` and `U` +/// will also return `true`. +pub fn contains_ty_adt_constructor_opaque<'tcx>(cx: &LateContext<'tcx>, ty: Ty<'tcx>, needle: Ty<'tcx>) -> bool { + ty.walk().any(|inner| match inner.unpack() { + GenericArgKind::Type(inner_ty) => { + if inner_ty == needle { + return true; + } + + if inner_ty.ty_adt_def() == needle.ty_adt_def() { + return true; + } + + if let ty::Opaque(def_id, _) = *inner_ty.kind() { + for &(predicate, _span) in cx.tcx.explicit_item_bounds(def_id) { + match predicate.kind().skip_binder() { + // For `impl Trait<U>`, it will register a predicate of `T: Trait<U>`, so we go through + // and check substituions to find `U`. + ty::PredicateKind::Trait(trait_predicate) => { + if trait_predicate + .trait_ref + .substs + .types() + .skip(1) // Skip the implicit `Self` generic parameter + .any(|ty| contains_ty_adt_constructor_opaque(cx, ty, needle)) + { + return true; + } + }, + // For `impl Trait<Assoc=U>`, it will register a predicate of `<T as Trait>::Assoc = U`, + // so we check the term for `U`. + ty::PredicateKind::Projection(projection_predicate) => { + if let ty::TermKind::Ty(ty) = projection_predicate.term.unpack() { + if contains_ty_adt_constructor_opaque(cx, ty, needle) { + return true; + } + }; + }, + _ => (), + } + } + } + + false + }, + GenericArgKind::Lifetime(_) | GenericArgKind::Const(_) => false, + }) +} + /// Resolves `<T as Iterator>::Item` for `T` /// Do not invoke without first verifying that the type implements `Iterator` pub fn get_iterator_item_ty<'tcx>(cx: &LateContext<'tcx>, ty: Ty<'tcx>) -> Option<Ty<'tcx>> { @@ -711,7 +763,7 @@ impl core::ops::Add<u32> for EnumValue { } } -/// Attempts to read the given constant as though it were an an enum value. +/// Attempts to read the given constant as though it were an enum value. #[expect(clippy::cast_possible_truncation, clippy::cast_possible_wrap)] pub fn read_explicit_enum_value(tcx: TyCtxt<'_>, id: DefId) -> Option<EnumValue> { if let Ok(ConstValue::Scalar(Scalar::Int(value))) = tcx.const_eval_poly(id) { diff --git a/tests/ui-cargo/cargo_rust_version/fail_both_diff/src/main.stderr b/tests/ui-cargo/cargo_rust_version/fail_both_diff/src/main.stderr index 9a7d802dc6d..163f8bb35e7 100644 --- a/tests/ui-cargo/cargo_rust_version/fail_both_diff/src/main.stderr +++ b/tests/ui-cargo/cargo_rust_version/fail_both_diff/src/main.stderr @@ -12,5 +12,11 @@ note: the lint level is defined here LL | #![deny(clippy::use_self)] | ^^^^^^^^^^^^^^^^ -error: aborting due to previous error; 1 warning emitted +error: unnecessary structure name repetition + --> $DIR/main.rs:7:9 + | +LL | Foo + | ^^^ help: use the applicable keyword: `Self` + +error: aborting due to 2 previous errors; 1 warning emitted diff --git a/tests/ui-cargo/cargo_rust_version/fail_both_same/src/main.stderr b/tests/ui-cargo/cargo_rust_version/fail_both_same/src/main.stderr index a280e1bacdf..259d39b1252 100644 --- a/tests/ui-cargo/cargo_rust_version/fail_both_same/src/main.stderr +++ b/tests/ui-cargo/cargo_rust_version/fail_both_same/src/main.stderr @@ -10,5 +10,11 @@ note: the lint level is defined here LL | #![deny(clippy::use_self)] | ^^^^^^^^^^^^^^^^ -error: aborting due to previous error +error: unnecessary structure name repetition + --> $DIR/main.rs:7:9 + | +LL | Foo + | ^^^ help: use the applicable keyword: `Self` + +error: aborting due to 2 previous errors diff --git a/tests/ui-cargo/cargo_rust_version/fail_cargo/src/main.stderr b/tests/ui-cargo/cargo_rust_version/fail_cargo/src/main.stderr index a280e1bacdf..259d39b1252 100644 --- a/tests/ui-cargo/cargo_rust_version/fail_cargo/src/main.stderr +++ b/tests/ui-cargo/cargo_rust_version/fail_cargo/src/main.stderr @@ -10,5 +10,11 @@ note: the lint level is defined here LL | #![deny(clippy::use_self)] | ^^^^^^^^^^^^^^^^ -error: aborting due to previous error +error: unnecessary structure name repetition + --> $DIR/main.rs:7:9 + | +LL | Foo + | ^^^ help: use the applicable keyword: `Self` + +error: aborting due to 2 previous errors diff --git a/tests/ui-cargo/cargo_rust_version/fail_clippy/src/main.stderr b/tests/ui-cargo/cargo_rust_version/fail_clippy/src/main.stderr index a280e1bacdf..259d39b1252 100644 --- a/tests/ui-cargo/cargo_rust_version/fail_clippy/src/main.stderr +++ b/tests/ui-cargo/cargo_rust_version/fail_clippy/src/main.stderr @@ -10,5 +10,11 @@ note: the lint level is defined here LL | #![deny(clippy::use_self)] | ^^^^^^^^^^^^^^^^ -error: aborting due to previous error +error: unnecessary structure name repetition + --> $DIR/main.rs:7:9 + | +LL | Foo + | ^^^ help: use the applicable keyword: `Self` + +error: aborting due to 2 previous errors diff --git a/tests/ui-cargo/cargo_rust_version/fail_file_attr/src/main.stderr b/tests/ui-cargo/cargo_rust_version/fail_file_attr/src/main.stderr index 88f6e00922b..97e6c3d5a55 100644 --- a/tests/ui-cargo/cargo_rust_version/fail_file_attr/src/main.stderr +++ b/tests/ui-cargo/cargo_rust_version/fail_file_attr/src/main.stderr @@ -10,5 +10,11 @@ note: the lint level is defined here LL | #![deny(clippy::use_self)] | ^^^^^^^^^^^^^^^^ -error: aborting due to previous error +error: unnecessary structure name repetition + --> $DIR/main.rs:12:9 + | +LL | Foo + | ^^^ help: use the applicable keyword: `Self` + +error: aborting due to 2 previous errors diff --git a/tests/ui/blanket_clippy_restriction_lints.rs b/tests/ui/blanket_clippy_restriction_lints.rs index d055f17526b..554745368bc 100644 --- a/tests/ui/blanket_clippy_restriction_lints.rs +++ b/tests/ui/blanket_clippy_restriction_lints.rs @@ -1,3 +1,5 @@ +// compile-flags: -W clippy::restriction + #![warn(clippy::blanket_clippy_restriction_lints)] //! Test that the whole restriction group is not enabled diff --git a/tests/ui/blanket_clippy_restriction_lints.stderr b/tests/ui/blanket_clippy_restriction_lints.stderr index e83eb4d605a..2bf89ab69a4 100644 --- a/tests/ui/blanket_clippy_restriction_lints.stderr +++ b/tests/ui/blanket_clippy_restriction_lints.stderr @@ -1,27 +1,32 @@ -error: restriction lints are not meant to be all enabled - --> $DIR/blanket_clippy_restriction_lints.rs:4:9 +error: `clippy::restriction` is not meant to be enabled as a group + | + = note: because of the command line `--warn clippy::restriction` + = help: enable the restriction lints you need individually + = note: `-D clippy::blanket-clippy-restriction-lints` implied by `-D warnings` + +error: `clippy::restriction` is not meant to be enabled as a group + --> $DIR/blanket_clippy_restriction_lints.rs:6:9 | LL | #![warn(clippy::restriction)] | ^^^^^^^^^^^^^^^^^^^ | - = help: try enabling only the lints you really need - = note: `-D clippy::blanket-clippy-restriction-lints` implied by `-D warnings` + = help: enable the restriction lints you need individually -error: restriction lints are not meant to be all enabled - --> $DIR/blanket_clippy_restriction_lints.rs:5:9 +error: `clippy::restriction` is not meant to be enabled as a group + --> $DIR/blanket_clippy_restriction_lints.rs:7:9 | LL | #![deny(clippy::restriction)] | ^^^^^^^^^^^^^^^^^^^ | - = help: try enabling only the lints you really need + = help: enable the restriction lints you need individually -error: restriction lints are not meant to be all enabled - --> $DIR/blanket_clippy_restriction_lints.rs:6:11 +error: `clippy::restriction` is not meant to be enabled as a group + --> $DIR/blanket_clippy_restriction_lints.rs:8:11 | LL | #![forbid(clippy::restriction)] | ^^^^^^^^^^^^^^^^^^^ | - = help: try enabling only the lints you really need + = help: enable the restriction lints you need individually -error: aborting due to 3 previous errors +error: aborting due to 4 previous errors diff --git a/tests/ui/bool_to_int_with_if.fixed b/tests/ui/bool_to_int_with_if.fixed index 2c8339cdd7f..37d3e3286a4 100644 --- a/tests/ui/bool_to_int_with_if.fixed +++ b/tests/ui/bool_to_int_with_if.fixed @@ -1,5 +1,6 @@ // run-rustfix +#![feature(let_chains)] #![warn(clippy::bool_to_int_with_if)] #![allow(unused, dead_code, clippy::unnecessary_operation, clippy::no_effect)] @@ -76,6 +77,8 @@ fn main() { 123 }; + pub const SHOULD_NOT_LINT: usize = if true { 1 } else { 0 }; + some_fn(a); } @@ -89,3 +92,22 @@ fn side_effect() {} fn cond(a: bool, b: bool) -> bool { a || b } + +enum Enum { + A, + B, +} + +fn if_let(a: Enum, b: Enum) { + if let Enum::A = a { + 1 + } else { + 0 + }; + + if let Enum::A = a && let Enum::B = b { + 1 + } else { + 0 + }; +} diff --git a/tests/ui/bool_to_int_with_if.rs b/tests/ui/bool_to_int_with_if.rs index 5d9496f0177..ebdf86fd185 100644 --- a/tests/ui/bool_to_int_with_if.rs +++ b/tests/ui/bool_to_int_with_if.rs @@ -1,5 +1,6 @@ // run-rustfix +#![feature(let_chains)] #![warn(clippy::bool_to_int_with_if)] #![allow(unused, dead_code, clippy::unnecessary_operation, clippy::no_effect)] @@ -108,6 +109,8 @@ fn main() { 123 }; + pub const SHOULD_NOT_LINT: usize = if true { 1 } else { 0 }; + some_fn(a); } @@ -121,3 +124,22 @@ fn side_effect() {} fn cond(a: bool, b: bool) -> bool { a || b } + +enum Enum { + A, + B, +} + +fn if_let(a: Enum, b: Enum) { + if let Enum::A = a { + 1 + } else { + 0 + }; + + if let Enum::A = a && let Enum::B = b { + 1 + } else { + 0 + }; +} diff --git a/tests/ui/bool_to_int_with_if.stderr b/tests/ui/bool_to_int_with_if.stderr index 4cb5531bef6..5cfb75cc0df 100644 --- a/tests/ui/bool_to_int_with_if.stderr +++ b/tests/ui/bool_to_int_with_if.stderr @@ -1,5 +1,5 @@ error: boolean to int conversion using if - --> $DIR/bool_to_int_with_if.rs:15:5 + --> $DIR/bool_to_int_with_if.rs:16:5 | LL | / if a { LL | | 1 @@ -12,7 +12,7 @@ LL | | }; = note: `-D clippy::bool-to-int-with-if` implied by `-D warnings` error: boolean to int conversion using if - --> $DIR/bool_to_int_with_if.rs:20:5 + --> $DIR/bool_to_int_with_if.rs:21:5 | LL | / if a { LL | | 0 @@ -24,7 +24,7 @@ LL | | }; = note: `!a as i32` or `(!a).into()` can also be valid options error: boolean to int conversion using if - --> $DIR/bool_to_int_with_if.rs:25:5 + --> $DIR/bool_to_int_with_if.rs:26:5 | LL | / if !a { LL | | 1 @@ -36,7 +36,7 @@ LL | | }; = note: `!a as i32` or `(!a).into()` can also be valid options error: boolean to int conversion using if - --> $DIR/bool_to_int_with_if.rs:30:5 + --> $DIR/bool_to_int_with_if.rs:31:5 | LL | / if a || b { LL | | 1 @@ -48,7 +48,7 @@ LL | | }; = note: `(a || b) as i32` or `(a || b).into()` can also be valid options error: boolean to int conversion using if - --> $DIR/bool_to_int_with_if.rs:35:5 + --> $DIR/bool_to_int_with_if.rs:36:5 | LL | / if cond(a, b) { LL | | 1 @@ -60,7 +60,7 @@ LL | | }; = note: `cond(a, b) as i32` or `cond(a, b).into()` can also be valid options error: boolean to int conversion using if - --> $DIR/bool_to_int_with_if.rs:40:5 + --> $DIR/bool_to_int_with_if.rs:41:5 | LL | / if x + y < 4 { LL | | 1 @@ -72,7 +72,7 @@ LL | | }; = note: `(x + y < 4) as i32` or `(x + y < 4).into()` can also be valid options error: boolean to int conversion using if - --> $DIR/bool_to_int_with_if.rs:49:12 + --> $DIR/bool_to_int_with_if.rs:50:12 | LL | } else if b { | ____________^ @@ -85,7 +85,7 @@ LL | | }; = note: `b as i32` or `b.into()` can also be valid options error: boolean to int conversion using if - --> $DIR/bool_to_int_with_if.rs:58:12 + --> $DIR/bool_to_int_with_if.rs:59:12 | LL | } else if b { | ____________^ @@ -98,7 +98,7 @@ LL | | }; = note: `!b as i32` or `(!b).into()` can also be valid options error: boolean to int conversion using if - --> $DIR/bool_to_int_with_if.rs:116:5 + --> $DIR/bool_to_int_with_if.rs:119:5 | LL | if a { 1 } else { 0 } | ^^^^^^^^^^^^^^^^^^^^^ help: replace with from: `u8::from(a)` diff --git a/tests/ui/manual_flatten.rs b/tests/ui/manual_flatten.rs index 96cd87c0e19..552213a7ff2 100644 --- a/tests/ui/manual_flatten.rs +++ b/tests/ui/manual_flatten.rs @@ -10,7 +10,7 @@ fn main() { } } - // Test for loop over implicitly implicitly adjusted `Iterator` with `if let` statement + // Test for loop over implicitly adjusted `Iterator` with `if let` statement let y: Vec<Result<i32, i32>> = vec![]; for n in y.clone() { if let Ok(n) = n { diff --git a/tests/ui/mut_range_bound.rs b/tests/ui/mut_range_bound.rs index e1ae1ef9282..7fdeb27ed98 100644 --- a/tests/ui/mut_range_bound.rs +++ b/tests/ui/mut_range_bound.rs @@ -76,7 +76,7 @@ fn mut_range_bound_no_immediate_break() { let mut n = 3; for i in n..10 { if n == 4 { - n = 1; // FIXME: warning because is is not immediately followed by break + n = 1; // FIXME: warning because it is not immediately followed by break let _ = 2; break; } diff --git a/tests/ui/mut_range_bound.stderr b/tests/ui/mut_range_bound.stderr index e0c8dced382..b679b7a0aaf 100644 --- a/tests/ui/mut_range_bound.stderr +++ b/tests/ui/mut_range_bound.stderr @@ -50,7 +50,7 @@ LL | m = 2; // warning because it is not immediately followed by break error: attempt to mutate range bound within loop --> $DIR/mut_range_bound.rs:79:13 | -LL | n = 1; // FIXME: warning because is is not immediately followed by break +LL | n = 1; // FIXME: warning because it is not immediately followed by break | ^ | = note: the range of the loop is unchanged diff --git a/tests/ui/needless_borrow.fixed b/tests/ui/needless_borrow.fixed index 340e89d2db1..9931fab04eb 100644 --- a/tests/ui/needless_borrow.fixed +++ b/tests/ui/needless_borrow.fixed @@ -385,3 +385,38 @@ mod used_more_than_once { fn use_x(_: impl AsRef<str>) {} fn use_x_again(_: impl AsRef<str>) {} } + +// https://github.com/rust-lang/rust-clippy/issues/9111#issuecomment-1277114280 +#[allow(dead_code)] +mod issue_9111 { + struct A; + + impl Extend<u8> for A { + fn extend<T: IntoIterator<Item = u8>>(&mut self, _: T) { + unimplemented!() + } + } + + impl<'a> Extend<&'a u8> for A { + fn extend<T: IntoIterator<Item = &'a u8>>(&mut self, _: T) { + unimplemented!() + } + } + + fn main() { + let mut a = A; + a.extend(&[]); // vs a.extend([]); + } +} + +#[allow(dead_code)] +mod issue_9710 { + fn main() { + let string = String::new(); + for _i in 0..10 { + f(&string); + } + } + + fn f<T: AsRef<str>>(_: T) {} +} diff --git a/tests/ui/needless_borrow.rs b/tests/ui/needless_borrow.rs index c93711ac8e2..4460f16d191 100644 --- a/tests/ui/needless_borrow.rs +++ b/tests/ui/needless_borrow.rs @@ -385,3 +385,38 @@ mod used_more_than_once { fn use_x(_: impl AsRef<str>) {} fn use_x_again(_: impl AsRef<str>) {} } + +// https://github.com/rust-lang/rust-clippy/issues/9111#issuecomment-1277114280 +#[allow(dead_code)] +mod issue_9111 { + struct A; + + impl Extend<u8> for A { + fn extend<T: IntoIterator<Item = u8>>(&mut self, _: T) { + unimplemented!() + } + } + + impl<'a> Extend<&'a u8> for A { + fn extend<T: IntoIterator<Item = &'a u8>>(&mut self, _: T) { + unimplemented!() + } + } + + fn main() { + let mut a = A; + a.extend(&[]); // vs a.extend([]); + } +} + +#[allow(dead_code)] +mod issue_9710 { + fn main() { + let string = String::new(); + for _i in 0..10 { + f(&string); + } + } + + fn f<T: AsRef<str>>(_: T) {} +} diff --git a/tests/ui/new_ret_no_self.rs b/tests/ui/new_ret_no_self.rs index 2f315ffe298..f69982d63a8 100644 --- a/tests/ui/new_ret_no_self.rs +++ b/tests/ui/new_ret_no_self.rs @@ -350,3 +350,53 @@ impl RetOtherSelf<T> { RetOtherSelf(RetOtherSelfWrapper(t)) } } + +mod issue7344 { + struct RetImplTraitSelf<T>(T); + + impl<T> RetImplTraitSelf<T> { + // should not trigger lint + fn new(t: T) -> impl Into<Self> { + Self(t) + } + } + + struct RetImplTraitNoSelf<T>(T); + + impl<T> RetImplTraitNoSelf<T> { + // should trigger lint + fn new(t: T) -> impl Into<i32> { + 1 + } + } + + trait Trait2<T, U> {} + impl<T, U> Trait2<T, U> for () {} + + struct RetImplTraitSelf2<T>(T); + + impl<T> RetImplTraitSelf2<T> { + // should not trigger lint + fn new(t: T) -> impl Trait2<(), Self> { + unimplemented!() + } + } + + struct RetImplTraitNoSelf2<T>(T); + + impl<T> RetImplTraitNoSelf2<T> { + // should trigger lint + fn new(t: T) -> impl Trait2<(), i32> { + unimplemented!() + } + } + + struct RetImplTraitSelfAdt<'a>(&'a str); + + impl<'a> RetImplTraitSelfAdt<'a> { + // should not trigger lint + fn new<'b: 'a>(s: &'b str) -> impl Into<RetImplTraitSelfAdt<'b>> { + RetImplTraitSelfAdt(s) + } + } +} diff --git a/tests/ui/new_ret_no_self.stderr b/tests/ui/new_ret_no_self.stderr index 8217bc6187f..bc13be47927 100644 --- a/tests/ui/new_ret_no_self.stderr +++ b/tests/ui/new_ret_no_self.stderr @@ -76,5 +76,21 @@ LL | | unimplemented!(); LL | | } | |_________^ -error: aborting due to 10 previous errors +error: methods called `new` usually return `Self` + --> $DIR/new_ret_no_self.rs:368:9 + | +LL | / fn new(t: T) -> impl Into<i32> { +LL | | 1 +LL | | } + | |_________^ + +error: methods called `new` usually return `Self` + --> $DIR/new_ret_no_self.rs:389:9 + | +LL | / fn new(t: T) -> impl Trait2<(), i32> { +LL | | unimplemented!() +LL | | } + | |_________^ + +error: aborting due to 12 previous errors diff --git a/tests/ui/option_if_let_else.fixed b/tests/ui/option_if_let_else.fixed index f15ac551bb3..0456005dce4 100644 --- a/tests/ui/option_if_let_else.fixed +++ b/tests/ui/option_if_let_else.fixed @@ -189,3 +189,12 @@ fn main() { let _ = res.map_or(1, |a| a + 1); let _ = res.map_or(5, |a| a + 1); } + +#[allow(dead_code)] +fn issue9742() -> Option<&'static str> { + // should not lint because of guards + match Some("foo ") { + Some(name) if name.starts_with("foo") => Some(name.trim()), + _ => None, + } +} diff --git a/tests/ui/option_if_let_else.rs b/tests/ui/option_if_let_else.rs index 9eeaea12d3b..23b148752cb 100644 --- a/tests/ui/option_if_let_else.rs +++ b/tests/ui/option_if_let_else.rs @@ -230,3 +230,12 @@ fn main() { }; let _ = if let Ok(a) = res { a + 1 } else { 5 }; } + +#[allow(dead_code)] +fn issue9742() -> Option<&'static str> { + // should not lint because of guards + match Some("foo ") { + Some(name) if name.starts_with("foo") => Some(name.trim()), + _ => None, + } +} diff --git a/tests/ui/question_mark.fixed b/tests/ui/question_mark.fixed index 993389232cc..5c49d46da72 100644 --- a/tests/ui/question_mark.fixed +++ b/tests/ui/question_mark.fixed @@ -134,6 +134,9 @@ fn result_func(x: Result<i32, i32>) -> Result<i32, i32> { return func_returning_result(); } + // no warning + let _ = if let Err(e) = x { Err(e) } else { Ok(0) }; + Ok(y) } diff --git a/tests/ui/question_mark.rs b/tests/ui/question_mark.rs index 9ae0d88829a..d057df6a9b3 100644 --- a/tests/ui/question_mark.rs +++ b/tests/ui/question_mark.rs @@ -166,6 +166,9 @@ fn result_func(x: Result<i32, i32>) -> Result<i32, i32> { return func_returning_result(); } + // no warning + let _ = if let Err(e) = x { Err(e) } else { Ok(0) }; + Ok(y) } diff --git a/tests/ui/question_mark.stderr b/tests/ui/question_mark.stderr index 1b6cd524b2f..23172d7e535 100644 --- a/tests/ui/question_mark.stderr +++ b/tests/ui/question_mark.stderr @@ -115,7 +115,7 @@ LL | | } | |_____^ help: replace it with: `x?;` error: this block may be rewritten with the `?` operator - --> $DIR/question_mark.rs:193:5 + --> $DIR/question_mark.rs:196:5 | LL | / if let Err(err) = func_returning_result() { LL | | return Err(err); @@ -123,7 +123,7 @@ LL | | } | |_____^ help: replace it with: `func_returning_result()?;` error: this block may be rewritten with the `?` operator - --> $DIR/question_mark.rs:200:5 + --> $DIR/question_mark.rs:203:5 | LL | / if let Err(err) = func_returning_result() { LL | | return Err(err); diff --git a/tests/ui/seek_from_current.fixed b/tests/ui/seek_from_current.fixed new file mode 100644 index 00000000000..4b5303324bc --- /dev/null +++ b/tests/ui/seek_from_current.fixed @@ -0,0 +1,26 @@ +// run-rustfix +#![warn(clippy::seek_from_current)] +#![feature(custom_inner_attributes)] + +use std::fs::File; +use std::io::{self, Seek, SeekFrom, Write}; + +fn _msrv_1_50() -> io::Result<()> { + #![clippy::msrv = "1.50"] + let mut f = File::create("foo.txt")?; + f.write_all(b"Hi!")?; + f.seek(SeekFrom::Current(0))?; + f.seek(SeekFrom::Current(1))?; + Ok(()) +} + +fn _msrv_1_51() -> io::Result<()> { + #![clippy::msrv = "1.51"] + let mut f = File::create("foo.txt")?; + f.write_all(b"Hi!")?; + f.stream_position()?; + f.seek(SeekFrom::Current(1))?; + Ok(()) +} + +fn main() {} diff --git a/tests/ui/seek_from_current.rs b/tests/ui/seek_from_current.rs new file mode 100644 index 00000000000..f93639261a1 --- /dev/null +++ b/tests/ui/seek_from_current.rs @@ -0,0 +1,26 @@ +// run-rustfix +#![warn(clippy::seek_from_current)] +#![feature(custom_inner_attributes)] + +use std::fs::File; +use std::io::{self, Seek, SeekFrom, Write}; + +fn _msrv_1_50() -> io::Result<()> { + #![clippy::msrv = "1.50"] + let mut f = File::create("foo.txt")?; + f.write_all(b"Hi!")?; + f.seek(SeekFrom::Current(0))?; + f.seek(SeekFrom::Current(1))?; + Ok(()) +} + +fn _msrv_1_51() -> io::Result<()> { + #![clippy::msrv = "1.51"] + let mut f = File::create("foo.txt")?; + f.write_all(b"Hi!")?; + f.seek(SeekFrom::Current(0))?; + f.seek(SeekFrom::Current(1))?; + Ok(()) +} + +fn main() {} diff --git a/tests/ui/seek_from_current.stderr b/tests/ui/seek_from_current.stderr new file mode 100644 index 00000000000..db1125b53cd --- /dev/null +++ b/tests/ui/seek_from_current.stderr @@ -0,0 +1,10 @@ +error: using `SeekFrom::Current` to start from current position + --> $DIR/seek_from_current.rs:21:5 + | +LL | f.seek(SeekFrom::Current(0))?; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace with: `f.stream_position()` + | + = note: `-D clippy::seek-from-current` implied by `-D warnings` + +error: aborting due to previous error + diff --git a/tests/ui/string_extend.fixed b/tests/ui/string_extend.fixed index 1883a9f8325..d200d7310fc 100644 --- a/tests/ui/string_extend.fixed +++ b/tests/ui/string_extend.fixed @@ -29,4 +29,7 @@ fn main() { let f = HasChars; s.extend(f.chars()); + + // issue #9735 + s.push_str(&abc[0..2]); } diff --git a/tests/ui/string_extend.rs b/tests/ui/string_extend.rs index 07d0baa1be6..0dd96a3b210 100644 --- a/tests/ui/string_extend.rs +++ b/tests/ui/string_extend.rs @@ -29,4 +29,7 @@ fn main() { let f = HasChars; s.extend(f.chars()); + + // issue #9735 + s.extend(abc[0..2].chars()); } diff --git a/tests/ui/string_extend.stderr b/tests/ui/string_extend.stderr index 6af8c9e1662..b35c77fd961 100644 --- a/tests/ui/string_extend.stderr +++ b/tests/ui/string_extend.stderr @@ -18,5 +18,11 @@ error: calling `.extend(_.chars())` LL | s.extend(def.chars()); | ^^^^^^^^^^^^^^^^^^^^^ help: try this: `s.push_str(&def)` -error: aborting due to 3 previous errors +error: calling `.extend(_.chars())` + --> $DIR/string_extend.rs:34:5 + | +LL | s.extend(abc[0..2].chars()); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `s.push_str(&abc[0..2])` + +error: aborting due to 4 previous errors diff --git a/tests/ui/undocumented_unsafe_blocks.rs b/tests/ui/undocumented_unsafe_blocks.rs index 08aee433215..cbc6768033e 100644 --- a/tests/ui/undocumented_unsafe_blocks.rs +++ b/tests/ui/undocumented_unsafe_blocks.rs @@ -490,4 +490,23 @@ unsafe impl CrateRoot for () {} // SAFETY: ok unsafe impl CrateRoot for (i32) {} +fn issue_9142() { + // SAFETY: ok + let _ = + // we need this comment to avoid rustfmt putting + // it all on one line + unsafe {}; + + // SAFETY: this is more than one level away, so it should warn + let _ = { + if unsafe { true } { + todo!(); + } else { + let bar = unsafe {}; + todo!(); + bar + } + }; +} + fn main() {} diff --git a/tests/ui/undocumented_unsafe_blocks.stderr b/tests/ui/undocumented_unsafe_blocks.stderr index 2c466ff5c73..ba4de9806d1 100644 --- a/tests/ui/undocumented_unsafe_blocks.stderr +++ b/tests/ui/undocumented_unsafe_blocks.stderr @@ -263,5 +263,29 @@ LL | unsafe impl CrateRoot for () {} | = help: consider adding a safety comment on the preceding line -error: aborting due to 31 previous errors +error: unsafe block missing a safety comment + --> $DIR/undocumented_unsafe_blocks.rs:498:9 + | +LL | unsafe {}; + | ^^^^^^^^^ + | + = help: consider adding a safety comment on the preceding line + +error: unsafe block missing a safety comment + --> $DIR/undocumented_unsafe_blocks.rs:502:12 + | +LL | if unsafe { true } { + | ^^^^^^^^^^^^^^^ + | + = help: consider adding a safety comment on the preceding line + +error: unsafe block missing a safety comment + --> $DIR/undocumented_unsafe_blocks.rs:505:23 + | +LL | let bar = unsafe {}; + | ^^^^^^^^^ + | + = help: consider adding a safety comment on the preceding line + +error: aborting due to 34 previous errors diff --git a/tests/ui/use_self_trait.fixed b/tests/ui/use_self_trait.fixed index 12adf1e41a6..4e779308d02 100644 --- a/tests/ui/use_self_trait.fixed +++ b/tests/ui/use_self_trait.fixed @@ -47,8 +47,7 @@ impl Mul for Bad { impl Clone for Bad { fn clone(&self) -> Self { - // FIXME: applicable here - Bad + Self } } @@ -138,4 +137,16 @@ mod impl_in_macro { parse_ip_impl!(Foo); // Should not lint } +mod full_path_replacement { + trait Error { + fn custom<T: std::fmt::Display>(_msg: T) -> Self; + } + + impl Error for std::fmt::Error { + fn custom<T: std::fmt::Display>(_msg: T) -> Self { + Self // Should lint + } + } +} + fn main() {} diff --git a/tests/ui/use_self_trait.rs b/tests/ui/use_self_trait.rs index 49dbcddc1d8..325dc73b21e 100644 --- a/tests/ui/use_self_trait.rs +++ b/tests/ui/use_self_trait.rs @@ -47,7 +47,6 @@ impl Mul for Bad { impl Clone for Bad { fn clone(&self) -> Self { - // FIXME: applicable here Bad } } @@ -138,4 +137,16 @@ mod impl_in_macro { parse_ip_impl!(Foo); // Should not lint } +mod full_path_replacement { + trait Error { + fn custom<T: std::fmt::Display>(_msg: T) -> Self; + } + + impl Error for std::fmt::Error { + fn custom<T: std::fmt::Display>(_msg: T) -> Self { + std::fmt::Error // Should lint + } + } +} + fn main() {} diff --git a/tests/ui/use_self_trait.stderr b/tests/ui/use_self_trait.stderr index 55af3ff2a93..090729b9c3d 100644 --- a/tests/ui/use_self_trait.stderr +++ b/tests/ui/use_self_trait.stderr @@ -84,5 +84,17 @@ error: unnecessary structure name repetition LL | fn mul(self, rhs: Bad) -> Bad { | ^^^ help: use the applicable keyword: `Self` -error: aborting due to 14 previous errors +error: unnecessary structure name repetition + --> $DIR/use_self_trait.rs:50:9 + | +LL | Bad + | ^^^ help: use the applicable keyword: `Self` + +error: unnecessary structure name repetition + --> $DIR/use_self_trait.rs:147:13 + | +LL | std::fmt::Error // Should lint + | ^^^^^^^^^^^^^^^ help: use the applicable keyword: `Self` + +error: aborting due to 16 previous errors |