diff --git a/CHANGELOG.md b/CHANGELOG.md index fef25ad8635ae..26c9877dbffb6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5463,6 +5463,7 @@ Released 2018-09-13 [`string_to_string`]: https://rust-lang.github.io/rust-clippy/master/index.html#string_to_string [`strlen_on_c_strings`]: https://rust-lang.github.io/rust-clippy/master/index.html#strlen_on_c_strings [`struct_excessive_bools`]: https://rust-lang.github.io/rust-clippy/master/index.html#struct_excessive_bools +[`struct_field_names`]: https://rust-lang.github.io/rust-clippy/master/index.html#struct_field_names [`stutter`]: https://rust-lang.github.io/rust-clippy/master/index.html#stutter [`suboptimal_flops`]: https://rust-lang.github.io/rust-clippy/master/index.html#suboptimal_flops [`suspicious_arithmetic_impl`]: https://rust-lang.github.io/rust-clippy/master/index.html#suspicious_arithmetic_impl @@ -5625,6 +5626,7 @@ Released 2018-09-13 [`single-char-binding-names-threshold`]: https://doc.rust-lang.org/clippy/lint_configuration.html#single-char-binding-names-threshold [`too-large-for-stack`]: https://doc.rust-lang.org/clippy/lint_configuration.html#too-large-for-stack [`enum-variant-name-threshold`]: https://doc.rust-lang.org/clippy/lint_configuration.html#enum-variant-name-threshold +[`struct-field-name-threshold`]: https://doc.rust-lang.org/clippy/lint_configuration.html#struct-field-name-threshold [`enum-variant-size-threshold`]: https://doc.rust-lang.org/clippy/lint_configuration.html#enum-variant-size-threshold [`verbose-bit-mask-threshold`]: https://doc.rust-lang.org/clippy/lint_configuration.html#verbose-bit-mask-threshold [`literal-representation-threshold`]: https://doc.rust-lang.org/clippy/lint_configuration.html#literal-representation-threshold diff --git a/Cargo.toml b/Cargo.toml index cbcb42dad79b2..2c50addfd7df1 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -29,7 +29,7 @@ color-print = "0.3.4" # Sync version with Cargo anstream = "0.5.0" [dev-dependencies] -ui_test = "0.20" +ui_test = "0.21.2" tester = "0.9" regex = "1.5" toml = "0.7.3" diff --git a/book/src/development/adding_lints.md b/book/src/development/adding_lints.md index e001197842bdc..55c0e105b307a 100644 --- a/book/src/development/adding_lints.md +++ b/book/src/development/adding_lints.md @@ -30,6 +30,7 @@ because that's clearly a non-descriptive name. - [Documentation](#documentation) - [Running rustfmt](#running-rustfmt) - [Debugging](#debugging) + - [Conflicting lints](#conflicting-lints) - [PR Checklist](#pr-checklist) - [Adding configuration to a lint](#adding-configuration-to-a-lint) - [Cheat Sheet](#cheat-sheet) @@ -612,6 +613,24 @@ output in the `stdout` part. [`dbg!`]: https://doc.rust-lang.org/std/macro.dbg.html +## Conflicting lints + +There are several lints that deal with the same pattern but suggest different approaches. In other words, some lints +may suggest modifications that go in the opposite direction to what some other lints already propose for the same +code, creating conflicting diagnostics. + +When you are creating a lint that ends up in this scenario, the following tips should be encouraged to guide +classification: + +* The only case where they should be in the same category is if that category is `restriction`. For example, +`semicolon_inside_block` and `semicolon_outside_block`. +* For all the other cases, they should be in different categories with different levels of allowance. For example, +`implicit_return` (restriction, allow) and `needless_return` (style, warn). + +For lints that are in different categories, it is also recommended that at least one of them should be in the +`restriction` category. The reason for this is that the `restriction` group is the only group where we don't +recommend to enable the entire set, but cherry pick lints out of. + ## PR Checklist Before submitting your PR make sure you followed all the basic requirements: diff --git a/book/src/lint_configuration.md b/book/src/lint_configuration.md index 2c958ccbbc246..c7eeed1795466 100644 --- a/book/src/lint_configuration.md +++ b/book/src/lint_configuration.md @@ -100,7 +100,7 @@ Suppress lints whenever the suggested change would cause breakage for other crat ## `msrv` The minimum rust version that the project supports -**Default Value:** `None` (`Option`) +**Default Value:** `Msrv { stack: [] }` (`crate::Msrv`) --- **Affected lints:** @@ -273,6 +273,16 @@ The minimum number of enum variants for the lints about variant names to trigger * [`enum_variant_names`](https://rust-lang.github.io/rust-clippy/master/index.html#enum_variant_names) +## `struct-field-name-threshold` +The minimum number of struct fields for the lints about field names to trigger + +**Default Value:** `3` (`u64`) + +--- +**Affected lints:** +* [`struct_variant_names`](https://rust-lang.github.io/rust-clippy/master/index.html#struct_variant_names) + + ## `enum-variant-size-threshold` The maximum size of an enum's variant to avoid box suggestion diff --git a/clippy_lints/src/async_yields_async.rs b/clippy_lints/src/async_yields_async.rs index 96a90d599abaa..56f56fff1e78a 100644 --- a/clippy_lints/src/async_yields_async.rs +++ b/clippy_lints/src/async_yields_async.rs @@ -2,7 +2,7 @@ use clippy_utils::diagnostics::span_lint_hir_and_then; use clippy_utils::source::snippet; use clippy_utils::ty::implements_trait; use rustc_errors::Applicability; -use rustc_hir::{AsyncCoroutineKind, Body, BodyId, ExprKind, CoroutineKind, QPath}; +use rustc_hir::{AsyncCoroutineKind, Body, BodyId, CoroutineKind, ExprKind, QPath}; use rustc_lint::{LateContext, LateLintPass}; use rustc_session::{declare_lint_pass, declare_tool_lint}; diff --git a/clippy_lints/src/await_holding_invalid.rs b/clippy_lints/src/await_holding_invalid.rs index 60f9bbd7458b3..ae8618dcaa062 100644 --- a/clippy_lints/src/await_holding_invalid.rs +++ b/clippy_lints/src/await_holding_invalid.rs @@ -287,5 +287,8 @@ fn is_mutex_guard(cx: &LateContext<'_>, def_id: DefId) -> bool { } fn is_refcell_ref(cx: &LateContext<'_>, def_id: DefId) -> bool { - matches!(cx.tcx.get_diagnostic_name(def_id), Some(sym::RefCellRef | sym::RefCellRefMut)) + matches!( + cx.tcx.get_diagnostic_name(def_id), + Some(sym::RefCellRef | sym::RefCellRefMut) + ) } diff --git a/clippy_lints/src/box_default.rs b/clippy_lints/src/box_default.rs index 3f1ff66b8cf60..cc9bd727937b1 100644 --- a/clippy_lints/src/box_default.rs +++ b/clippy_lints/src/box_default.rs @@ -3,8 +3,9 @@ use clippy_utils::macros::macro_backtrace; use clippy_utils::ty::expr_sig; use clippy_utils::{get_parent_node, is_default_equivalent, path_def_id}; use rustc_errors::Applicability; +use rustc_hir::def::Res; use rustc_hir::intravisit::{walk_ty, Visitor}; -use rustc_hir::{def::Res, Block, Expr, ExprKind, Local, Node, QPath, TyKind}; +use rustc_hir::{Block, Expr, ExprKind, Local, Node, QPath, TyKind}; use rustc_lint::{LateContext, LateLintPass, LintContext}; use rustc_middle::lint::in_external_macro; use rustc_middle::ty::print::with_forced_trimmed_paths; diff --git a/clippy_lints/src/declared_lints.rs b/clippy_lints/src/declared_lints.rs index 481c44031cf72..77438b27f9009 100644 --- a/clippy_lints/src/declared_lints.rs +++ b/clippy_lints/src/declared_lints.rs @@ -154,9 +154,6 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[ crate::endian_bytes::LITTLE_ENDIAN_BYTES_INFO, crate::entry::MAP_ENTRY_INFO, crate::enum_clike::ENUM_CLIKE_UNPORTABLE_VARIANT_INFO, - crate::enum_variants::ENUM_VARIANT_NAMES_INFO, - crate::enum_variants::MODULE_INCEPTION_INFO, - crate::enum_variants::MODULE_NAME_REPETITIONS_INFO, crate::equatable_if_let::EQUATABLE_IF_LET_INFO, crate::error_impl_error::ERROR_IMPL_ERROR_INFO, crate::escape::BOXED_LOCAL_INFO, @@ -226,6 +223,10 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[ crate::instant_subtraction::UNCHECKED_DURATION_SUBTRACTION_INFO, crate::int_plus_one::INT_PLUS_ONE_INFO, crate::invalid_upcast_comparisons::INVALID_UPCAST_COMPARISONS_INFO, + crate::item_name_repetitions::ENUM_VARIANT_NAMES_INFO, + crate::item_name_repetitions::MODULE_INCEPTION_INFO, + crate::item_name_repetitions::MODULE_NAME_REPETITIONS_INFO, + crate::item_name_repetitions::STRUCT_FIELD_NAMES_INFO, crate::items_after_statements::ITEMS_AFTER_STATEMENTS_INFO, crate::items_after_test_module::ITEMS_AFTER_TEST_MODULE_INFO, crate::iter_not_returning_iterator::ITER_NOT_RETURNING_ITERATOR_INFO, diff --git a/clippy_lints/src/default_instead_of_iter_empty.rs b/clippy_lints/src/default_instead_of_iter_empty.rs index 3d6d257e386b5..2d11fa6b647dc 100644 --- a/clippy_lints/src/default_instead_of_iter_empty.rs +++ b/clippy_lints/src/default_instead_of_iter_empty.rs @@ -1,6 +1,6 @@ use clippy_utils::diagnostics::span_lint_and_sugg; -use clippy_utils::source::snippet_with_context; use clippy_utils::last_path_segment; +use clippy_utils::source::snippet_with_context; use rustc_errors::Applicability; use rustc_hir::{def, Expr, ExprKind, GenericArg, QPath, TyKind}; use rustc_lint::{LateContext, LateLintPass}; diff --git a/clippy_lints/src/exit.rs b/clippy_lints/src/exit.rs index 5de79133c7dcc..5ecd0ffadf362 100644 --- a/clippy_lints/src/exit.rs +++ b/clippy_lints/src/exit.rs @@ -1,5 +1,5 @@ use clippy_utils::diagnostics::span_lint; -use clippy_utils::{is_entrypoint_fn}; +use clippy_utils::is_entrypoint_fn; use if_chain::if_chain; use rustc_hir::{Expr, ExprKind, Item, ItemKind, Node}; use rustc_lint::{LateContext, LateLintPass}; diff --git a/clippy_lints/src/from_raw_with_void_ptr.rs b/clippy_lints/src/from_raw_with_void_ptr.rs index d82ea6d2fc80e..617c96b4fcbc9 100644 --- a/clippy_lints/src/from_raw_with_void_ptr.rs +++ b/clippy_lints/src/from_raw_with_void_ptr.rs @@ -1,6 +1,6 @@ use clippy_utils::diagnostics::span_lint_and_help; -use clippy_utils::ty::is_c_void; use clippy_utils::path_def_id; +use clippy_utils::ty::is_c_void; use rustc_hir::def_id::DefId; use rustc_hir::{Expr, ExprKind, QPath}; use rustc_lint::{LateContext, LateLintPass}; diff --git a/clippy_lints/src/functions/impl_trait_in_params.rs b/clippy_lints/src/functions/impl_trait_in_params.rs index 597fca8888516..ee66c841ed25c 100644 --- a/clippy_lints/src/functions/impl_trait_in_params.rs +++ b/clippy_lints/src/functions/impl_trait_in_params.rs @@ -1,50 +1,104 @@ use clippy_utils::diagnostics::span_lint_and_then; use clippy_utils::is_in_test_function; +use rustc_hir as hir; use rustc_hir::intravisit::FnKind; -use rustc_hir::{Body, HirId}; +use rustc_hir::{Body, GenericParam, Generics, HirId, ImplItem, ImplItemKind, TraitItem, TraitItemKind}; use rustc_lint::LateContext; -use rustc_span::Span; +use rustc_span::symbol::Ident; +use rustc_span::{BytePos, Span}; use super::IMPL_TRAIT_IN_PARAMS; +fn report( + cx: &LateContext<'_>, + param: &GenericParam<'_>, + ident: &Ident, + generics: &Generics<'_>, + first_param_span: Span, +) { + // No generics with nested generics, and no generics like FnMut(x) + span_lint_and_then( + cx, + IMPL_TRAIT_IN_PARAMS, + param.span, + "`impl Trait` used as a function parameter", + |diag| { + if let Some(gen_span) = generics.span_for_param_suggestion() { + // If there's already a generic param with the same bound, do not lint **this** suggestion. + diag.span_suggestion_with_style( + gen_span, + "add a type parameter", + format!(", {{ /* Generic name */ }}: {}", ¶m.name.ident().as_str()[5..]), + rustc_errors::Applicability::HasPlaceholders, + rustc_errors::SuggestionStyle::ShowAlways, + ); + } else { + diag.span_suggestion_with_style( + Span::new( + first_param_span.lo() - rustc_span::BytePos(1), + ident.span.hi(), + ident.span.ctxt(), + ident.span.parent(), + ), + "add a type parameter", + format!("<{{ /* Generic name */ }}: {}>", ¶m.name.ident().as_str()[5..]), + rustc_errors::Applicability::HasPlaceholders, + rustc_errors::SuggestionStyle::ShowAlways, + ); + } + }, + ); +} + pub(super) fn check_fn<'tcx>(cx: &LateContext<'_>, kind: &'tcx FnKind<'_>, body: &'tcx Body<'_>, hir_id: HirId) { - if cx.tcx.visibility(cx.tcx.hir().body_owner_def_id(body.id())).is_public() && !is_in_test_function(cx.tcx, hir_id) - { - if let FnKind::ItemFn(ident, generics, _) = kind { + if_chain! { + if let FnKind::ItemFn(ident, generics, _) = kind; + if cx.tcx.visibility(cx.tcx.hir().body_owner_def_id(body.id())).is_public(); + if !is_in_test_function(cx.tcx, hir_id); + then { for param in generics.params { if param.is_impl_trait() { - // No generics with nested generics, and no generics like FnMut(x) - span_lint_and_then( - cx, - IMPL_TRAIT_IN_PARAMS, - param.span, - "'`impl Trait` used as a function parameter'", - |diag| { - if let Some(gen_span) = generics.span_for_param_suggestion() { - diag.span_suggestion_with_style( - gen_span, - "add a type parameter", - format!(", {{ /* Generic name */ }}: {}", ¶m.name.ident().as_str()[5..]), - rustc_errors::Applicability::HasPlaceholders, - rustc_errors::SuggestionStyle::ShowAlways, - ); - } else { - diag.span_suggestion_with_style( - Span::new( - body.params[0].span.lo() - rustc_span::BytePos(1), - ident.span.hi(), - ident.span.ctxt(), - ident.span.parent(), - ), - "add a type parameter", - format!("<{{ /* Generic name */ }}: {}>", ¶m.name.ident().as_str()[5..]), - rustc_errors::Applicability::HasPlaceholders, - rustc_errors::SuggestionStyle::ShowAlways, - ); - } - }, - ); + report(cx, param, ident, generics, body.params[0].span); + }; + } + } + } +} + +pub(super) fn check_impl_item(cx: &LateContext<'_>, impl_item: &ImplItem<'_>) { + if_chain! { + if let ImplItemKind::Fn(_, body_id) = impl_item.kind; + if let hir::Node::Item(item) = cx.tcx.hir().get_parent(impl_item.hir_id()); + if let hir::ItemKind::Impl(impl_) = item.kind; + if let hir::Impl { of_trait, .. } = *impl_; + if of_trait.is_none(); + let body = cx.tcx.hir().body(body_id); + if cx.tcx.visibility(cx.tcx.hir().body_owner_def_id(body.id())).is_public(); + if !is_in_test_function(cx.tcx, impl_item.hir_id()); + then { + for param in impl_item.generics.params { + if param.is_impl_trait() { + report(cx, param, &impl_item.ident, impl_item.generics, body.params[0].span); + } + } + } + } +} + +pub(super) fn check_trait_item(cx: &LateContext<'_>, trait_item: &TraitItem<'_>, avoid_breaking_exported_api: bool) { + if_chain! { + if !avoid_breaking_exported_api; + if let TraitItemKind::Fn(_, _) = trait_item.kind; + if let hir::Node::Item(item) = cx.tcx.hir().get_parent(trait_item.hir_id()); + // ^^ (Will always be a trait) + if !item.vis_span.is_empty(); // Is public + if !is_in_test_function(cx.tcx, trait_item.hir_id()); + then { + for param in trait_item.generics.params { + if param.is_impl_trait() { + let sp = trait_item.ident.span.with_hi(trait_item.ident.span.hi() + BytePos(1)); + report(cx, param, &trait_item.ident, trait_item.generics, sp.shrink_to_hi()); } } } diff --git a/clippy_lints/src/functions/mod.rs b/clippy_lints/src/functions/mod.rs index ee10334c67f13..716908483e9db 100644 --- a/clippy_lints/src/functions/mod.rs +++ b/clippy_lints/src/functions/mod.rs @@ -360,18 +360,26 @@ declare_clippy_lint! { } #[derive(Copy, Clone)] +#[allow(clippy::struct_field_names)] pub struct Functions { too_many_arguments_threshold: u64, too_many_lines_threshold: u64, large_error_threshold: u64, + avoid_breaking_exported_api: bool, } impl Functions { - pub fn new(too_many_arguments_threshold: u64, too_many_lines_threshold: u64, large_error_threshold: u64) -> Self { + pub fn new( + too_many_arguments_threshold: u64, + too_many_lines_threshold: u64, + large_error_threshold: u64, + avoid_breaking_exported_api: bool, + ) -> Self { Self { too_many_arguments_threshold, too_many_lines_threshold, large_error_threshold, + avoid_breaking_exported_api, } } } @@ -415,6 +423,7 @@ impl<'tcx> LateLintPass<'tcx> for Functions { fn check_impl_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx hir::ImplItem<'_>) { must_use::check_impl_item(cx, item); result::check_impl_item(cx, item, self.large_error_threshold); + impl_trait_in_params::check_impl_item(cx, item); } fn check_trait_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx hir::TraitItem<'_>) { @@ -422,5 +431,6 @@ impl<'tcx> LateLintPass<'tcx> for Functions { not_unsafe_ptr_arg_deref::check_trait_item(cx, item); must_use::check_trait_item(cx, item); result::check_trait_item(cx, item, self.large_error_threshold); + impl_trait_in_params::check_trait_item(cx, item, self.avoid_breaking_exported_api); } } diff --git a/clippy_lints/src/enum_variants.rs b/clippy_lints/src/item_name_repetitions.rs similarity index 60% rename from clippy_lints/src/enum_variants.rs rename to clippy_lints/src/item_name_repetitions.rs index e332f681b6dbd..8b4984da3dd1d 100644 --- a/clippy_lints/src/enum_variants.rs +++ b/clippy_lints/src/item_name_repetitions.rs @@ -1,9 +1,10 @@ //! lint on enum variants that are prefixed or suffixed by the same characters use clippy_utils::diagnostics::{span_lint, span_lint_and_help, span_lint_hir}; +use clippy_utils::macros::span_is_local; use clippy_utils::source::is_present_in_source; -use clippy_utils::str_utils::{camel_case_split, count_match_end, count_match_start}; -use rustc_hir::{EnumDef, Item, ItemKind, OwnerId, Variant}; +use clippy_utils::str_utils::{camel_case_split, count_match_end, count_match_start, to_camel_case, to_snake_case}; +use rustc_hir::{EnumDef, FieldDef, Item, ItemKind, OwnerId, Variant, VariantData}; use rustc_lint::{LateContext, LateLintPass}; use rustc_session::{declare_tool_lint, impl_lint_pass}; use rustc_span::source_map::Span; @@ -103,32 +104,184 @@ declare_clippy_lint! { style, "modules that have the same name as their parent module" } +declare_clippy_lint! { + /// ### What it does + /// Detects struct fields that are prefixed or suffixed + /// by the same characters or the name of the struct itself. + /// + /// ### Why is this bad? + /// Information common to all struct fields is better represented in the struct name. + /// + /// ### Limitations + /// Characters with no casing will be considered when comparing prefixes/suffixes + /// This applies to numbers and non-ascii characters without casing + /// e.g. `foo1` and `foo2` is considered to have different prefixes + /// (the prefixes are `foo1` and `foo2` respectively), as also `bar螃`, `bar蟹` + /// + /// ### Example + /// ```rust + /// struct Cake { + /// cake_sugar: u8, + /// cake_flour: u8, + /// cake_eggs: u8 + /// } + /// ``` + /// Use instead: + /// ```rust + /// struct Cake { + /// sugar: u8, + /// flour: u8, + /// eggs: u8 + /// } + /// ``` + #[clippy::version = "1.75.0"] + pub STRUCT_FIELD_NAMES, + pedantic, + "structs where all fields share a prefix/postfix or contain the name of the struct" +} -pub struct EnumVariantNames { +pub struct ItemNameRepetitions { modules: Vec<(Symbol, String, OwnerId)>, - threshold: u64, + enum_threshold: u64, + struct_threshold: u64, avoid_breaking_exported_api: bool, allow_private_module_inception: bool, } -impl EnumVariantNames { +impl ItemNameRepetitions { #[must_use] - pub fn new(threshold: u64, avoid_breaking_exported_api: bool, allow_private_module_inception: bool) -> Self { + pub fn new( + enum_threshold: u64, + struct_threshold: u64, + avoid_breaking_exported_api: bool, + allow_private_module_inception: bool, + ) -> Self { Self { modules: Vec::new(), - threshold, + enum_threshold, + struct_threshold, avoid_breaking_exported_api, allow_private_module_inception, } } } -impl_lint_pass!(EnumVariantNames => [ +impl_lint_pass!(ItemNameRepetitions => [ ENUM_VARIANT_NAMES, + STRUCT_FIELD_NAMES, MODULE_NAME_REPETITIONS, MODULE_INCEPTION ]); +#[must_use] +fn have_no_extra_prefix(prefixes: &[&str]) -> bool { + prefixes.iter().all(|p| p == &"" || p == &"_") +} + +fn check_fields(cx: &LateContext<'_>, threshold: u64, item: &Item<'_>, fields: &[FieldDef<'_>]) { + if (fields.len() as u64) < threshold { + return; + } + + check_struct_name_repetition(cx, item, fields); + + // if the SyntaxContext of the identifiers of the fields and struct differ dont lint them. + // this prevents linting in macros in which the location of the field identifier names differ + if !fields.iter().all(|field| item.ident.span.eq_ctxt(field.ident.span)) { + return; + } + + let mut pre: Vec<&str> = match fields.first() { + Some(first_field) => first_field.ident.name.as_str().split('_').collect(), + None => return, + }; + let mut post = pre.clone(); + post.reverse(); + for field in fields { + let field_split: Vec<&str> = field.ident.name.as_str().split('_').collect(); + if field_split.len() == 1 { + return; + } + + pre = pre + .into_iter() + .zip(field_split.iter()) + .take_while(|(a, b)| &a == b) + .map(|e| e.0) + .collect(); + post = post + .into_iter() + .zip(field_split.iter().rev()) + .take_while(|(a, b)| &a == b) + .map(|e| e.0) + .collect(); + } + let prefix = pre.join("_"); + post.reverse(); + let postfix = match post.last() { + Some(&"") => post.join("_") + "_", + Some(_) | None => post.join("_"), + }; + if fields.len() > 1 { + let (what, value) = match ( + prefix.is_empty() || prefix.chars().all(|c| c == '_'), + postfix.is_empty(), + ) { + (true, true) => return, + (false, _) => ("pre", prefix), + (true, false) => ("post", postfix), + }; + span_lint_and_help( + cx, + STRUCT_FIELD_NAMES, + item.span, + &format!("all fields have the same {what}fix: `{value}`"), + None, + &format!("remove the {what}fixes"), + ); + } +} + +fn check_struct_name_repetition(cx: &LateContext<'_>, item: &Item<'_>, fields: &[FieldDef<'_>]) { + let snake_name = to_snake_case(item.ident.name.as_str()); + let item_name_words: Vec<&str> = snake_name.split('_').collect(); + for field in fields { + if field.ident.span.eq_ctxt(item.ident.span) { + //consider linting only if the field identifier has the same SyntaxContext as the item(struct) + let field_words: Vec<&str> = field.ident.name.as_str().split('_').collect(); + if field_words.len() >= item_name_words.len() { + // if the field name is shorter than the struct name it cannot contain it + if field_words.iter().zip(item_name_words.iter()).all(|(a, b)| a == b) { + span_lint_hir( + cx, + STRUCT_FIELD_NAMES, + field.hir_id, + field.span, + "field name starts with the struct's name", + ); + } + if field_words.len() > item_name_words.len() { + // lint only if the end is not covered by the start + if field_words + .iter() + .rev() + .zip(item_name_words.iter().rev()) + .all(|(a, b)| a == b) + { + span_lint_hir( + cx, + STRUCT_FIELD_NAMES, + field.hir_id, + field.span, + "field name ends with the struct's name", + ); + } + } + } + } + } +} + fn check_enum_start(cx: &LateContext<'_>, item_name: &str, variant: &Variant<'_>) { let name = variant.ident.name.as_str(); let item_name_chars = item_name.chars().count(); @@ -218,35 +371,7 @@ fn check_variant(cx: &LateContext<'_>, threshold: u64, def: &EnumDef<'_>, item_n ); } -#[must_use] -fn have_no_extra_prefix(prefixes: &[&str]) -> bool { - prefixes.iter().all(|p| p == &"" || p == &"_") -} - -#[must_use] -fn to_camel_case(item_name: &str) -> String { - let mut s = String::new(); - let mut up = true; - for c in item_name.chars() { - if c.is_uppercase() { - // we only turn snake case text into CamelCase - return item_name.to_string(); - } - if c == '_' { - up = true; - continue; - } - if up { - up = false; - s.extend(c.to_uppercase()); - } else { - s.push(c); - } - } - s -} - -impl LateLintPass<'_> for EnumVariantNames { +impl LateLintPass<'_> for ItemNameRepetitions { fn check_item_post(&mut self, _cx: &LateContext<'_>, _item: &Item<'_>) { let last = self.modules.pop(); assert!(last.is_some()); @@ -303,9 +428,15 @@ impl LateLintPass<'_> for EnumVariantNames { } } } - if let ItemKind::Enum(ref def, _) = item.kind { - if !(self.avoid_breaking_exported_api && cx.effective_visibilities.is_exported(item.owner_id.def_id)) { - check_variant(cx, self.threshold, def, item_name, item.span); + if !(self.avoid_breaking_exported_api && cx.effective_visibilities.is_exported(item.owner_id.def_id)) + && span_is_local(item.span) + { + match item.kind { + ItemKind::Enum(def, _) => check_variant(cx, self.enum_threshold, &def, item_name, item.span), + ItemKind::Struct(VariantData::Struct(fields, _), _) => { + check_fields(cx, self.struct_threshold, item, fields); + }, + _ => (), } } self.modules.push((item.ident.name, item_camel, item.owner_id)); diff --git a/clippy_lints/src/iter_without_into_iter.rs b/clippy_lints/src/iter_without_into_iter.rs index 43e1b92c9b9a7..0ee291a4e9da9 100644 --- a/clippy_lints/src/iter_without_into_iter.rs +++ b/clippy_lints/src/iter_without_into_iter.rs @@ -9,6 +9,7 @@ use rustc_lint::{LateContext, LateLintPass}; use rustc_middle::ty::{self, Ty}; use rustc_session::{declare_lint_pass, declare_tool_lint}; use rustc_span::{sym, Symbol}; +use std::iter; declare_clippy_lint! { /// ### What it does @@ -52,12 +53,13 @@ declare_clippy_lint! { declare_clippy_lint! { /// ### What it does /// This is the opposite of the `iter_without_into_iter` lint. - /// It looks for `IntoIterator for (&|&mut) Type` implementations without an inherent `iter` or `iter_mut` method. + /// It looks for `IntoIterator for (&|&mut) Type` implementations without an inherent `iter` or `iter_mut` method + /// on the type or on any of the types in its `Deref` chain. /// /// ### Why is this bad? /// It's not bad, but having them is idiomatic and allows the type to be used in iterator chains - /// by just calling `.iter()`, instead of the more awkward `<&Type>::into_iter` or `(&val).iter()` syntax - /// in case of ambiguity with another `Intoiterator` impl. + /// by just calling `.iter()`, instead of the more awkward `<&Type>::into_iter` or `(&val).into_iter()` syntax + /// in case of ambiguity with another `IntoIterator` impl. /// /// ### Example /// ```rust @@ -102,7 +104,20 @@ fn is_nameable_in_impl_trait(ty: &rustc_hir::Ty<'_>) -> bool { !matches!(ty.kind, TyKind::OpaqueDef(..)) } -fn type_has_inherent_method(cx: &LateContext<'_>, ty: Ty<'_>, method_name: Symbol) -> bool { +/// Returns the deref chain of a type, starting with the type itself. +fn deref_chain<'cx, 'tcx>(cx: &'cx LateContext<'tcx>, ty: Ty<'tcx>) -> impl Iterator> + 'cx { + iter::successors(Some(ty), |&ty| { + if let Some(deref_did) = cx.tcx.lang_items().deref_trait() + && implements_trait(cx, ty, deref_did, &[]) + { + make_normalized_projection(cx.tcx, cx.param_env, deref_did, sym::Target, [ty]) + } else { + None + } + }) +} + +fn adt_has_inherent_method(cx: &LateContext<'_>, ty: Ty<'_>, method_name: Symbol) -> bool { if let Some(ty_did) = ty.ty_adt_def().map(ty::AdtDef::did) { cx.tcx.inherent_impls(ty_did).iter().any(|&did| { cx.tcx @@ -127,7 +142,11 @@ impl LateLintPass<'_> for IterWithoutIntoIter { Mutability::Mut => sym::iter_mut, Mutability::Not => sym::iter, } - && !type_has_inherent_method(cx, ty, expected_method_name) + && !deref_chain(cx, ty) + .any(|ty| { + // We can't check inherent impls for slices, but we know that they have an `iter(_mut)` method + ty.peel_refs().is_slice() || adt_has_inherent_method(cx, ty, expected_method_name) + }) && let Some(iter_assoc_span) = imp.items.iter().find_map(|item| { if item.ident.name == sym!(IntoIter) { Some(cx.tcx.hir().impl_item(item.id).expect_type().span) diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 0f35ec2766574..a70a38ee08bff 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -50,9 +50,6 @@ extern crate clippy_utils; #[macro_use] extern crate declare_clippy_lint; -use std::io; -use std::path::PathBuf; - use clippy_utils::msrvs::Msrv; use rustc_data_structures::fx::FxHashSet; use rustc_lint::{Lint, LintId}; @@ -121,7 +118,6 @@ mod empty_structs_with_brackets; mod endian_bytes; mod entry; mod enum_clike; -mod enum_variants; mod equatable_if_let; mod error_impl_error; mod escape; @@ -166,6 +162,7 @@ mod inline_fn_without_body; mod instant_subtraction; mod int_plus_one; mod invalid_upcast_comparisons; +mod item_name_repetitions; mod items_after_statements; mod items_after_test_module; mod iter_not_returning_iterator; @@ -362,7 +359,6 @@ mod zero_sized_map_values; // end lints modules, do not remove this comment, it’s used in `update_lints` use crate::utils::conf::metadata::get_configuration_metadata; -use crate::utils::conf::TryConf; pub use crate::utils::conf::{lookup_conf_file, Conf}; use crate::utils::FindAll; @@ -374,65 +370,13 @@ use crate::utils::FindAll; /// level (i.e `#![cfg_attr(...)]`) will still be expanded even when using a pre-expansion pass. /// /// Used in `./src/driver.rs`. -pub fn register_pre_expansion_lints(store: &mut rustc_lint::LintStore, sess: &Session, conf: &Conf) { +pub fn register_pre_expansion_lints(store: &mut rustc_lint::LintStore, conf: &'static Conf) { // NOTE: Do not add any more pre-expansion passes. These should be removed eventually. - let msrv = Msrv::read(&conf.msrv, sess); - let msrv = move || msrv.clone(); + let msrv = || conf.msrv.clone(); store.register_pre_expansion_pass(move || Box::new(attrs::EarlyAttributes { msrv: msrv() })); } -#[doc(hidden)] -pub fn read_conf(sess: &Session, path: &io::Result<(Option, Vec)>) -> Conf { - if let Ok((_, warnings)) = path { - for warning in warnings { - sess.warn(warning.clone()); - } - } - let file_name = match path { - Ok((Some(path), _)) => path, - Ok((None, _)) => return Conf::default(), - Err(error) => { - sess.err(format!("error finding Clippy's configuration file: {error}")); - return Conf::default(); - }, - }; - - let TryConf { conf, errors, warnings } = utils::conf::read(sess, file_name); - // all conf errors are non-fatal, we just use the default conf in case of error - for error in errors { - if let Some(span) = error.span { - sess.span_err( - span, - format!("error reading Clippy's configuration file: {}", error.message), - ); - } else { - sess.err(format!( - "error reading Clippy's configuration file `{}`: {}", - file_name.display(), - error.message - )); - } - } - - for warning in warnings { - if let Some(span) = warning.span { - sess.span_warn( - span, - format!("error reading Clippy's configuration file: {}", warning.message), - ); - } else { - sess.warn(format!( - "error reading Clippy's configuration file `{}`: {}", - file_name.display(), - warning.message - )); - } - } - - conf -} - #[derive(Default)] struct RegistrationGroups { all: Vec, @@ -558,7 +502,7 @@ fn register_categories(store: &mut rustc_lint::LintStore) { /// /// Used in `./src/driver.rs`. #[expect(clippy::too_many_lines)] -pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: &Conf) { +pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: &'static Conf) { register_removed_non_tool_lints(store); register_categories(store); @@ -660,8 +604,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: store.register_late_pass(|_| Box::new(non_octal_unix_permissions::NonOctalUnixPermissions)); store.register_early_pass(|| Box::new(unnecessary_self_imports::UnnecessarySelfImports)); - let msrv = Msrv::read(&conf.msrv, sess); - let msrv = move || msrv.clone(); + let msrv = || conf.msrv.clone(); let avoid_breaking_exported_api = conf.avoid_breaking_exported_api; let allow_expect_in_tests = conf.allow_expect_in_tests; let allow_unwrap_in_tests = conf.allow_unwrap_in_tests; @@ -762,6 +705,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: too_many_arguments_threshold, too_many_lines_threshold, large_error_threshold, + avoid_breaking_exported_api, )) }); let doc_valid_idents = conf.doc_valid_idents.iter().cloned().collect::>(); @@ -806,7 +750,8 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: suppress_restriction_lint_in_const, )) }); - store.register_late_pass(|_| Box::new(non_copy_const::NonCopyConst)); + let ignore_interior_mutability = conf.ignore_interior_mutability.clone(); + store.register_late_pass(move |_| Box::new(non_copy_const::NonCopyConst::new(ignore_interior_mutability.clone()))); store.register_late_pass(|_| Box::new(ptr_offset_with_cast::PtrOffsetWithCast)); store.register_late_pass(|_| Box::new(redundant_clone::RedundantClone)); store.register_late_pass(|_| Box::new(slow_vector_initialization::SlowVectorInit)); @@ -851,10 +796,12 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: )) }); let enum_variant_name_threshold = conf.enum_variant_name_threshold; + let struct_field_name_threshold = conf.struct_field_name_threshold; let allow_private_module_inception = conf.allow_private_module_inception; store.register_late_pass(move |_| { - Box::new(enum_variants::EnumVariantNames::new( + Box::new(item_name_repetitions::ItemNameRepetitions::new( enum_variant_name_threshold, + struct_field_name_threshold, avoid_breaking_exported_api, allow_private_module_inception, )) diff --git a/clippy_lints/src/loops/same_item_push.rs b/clippy_lints/src/loops/same_item_push.rs index f7b3b2358a0bd..5fffb27cda2f1 100644 --- a/clippy_lints/src/loops/same_item_push.rs +++ b/clippy_lints/src/loops/same_item_push.rs @@ -185,7 +185,7 @@ fn get_vec_push<'tcx>( if let StmtKind::Semi(semi_stmt) = &stmt.kind; if let ExprKind::MethodCall(path, self_expr, args, _) = &semi_stmt.kind; // Figure out the parameters for the method call - if let Some(pushed_item) = args.get(0); + if let Some(pushed_item) = args.first(); // Check that the method being called is push() on a Vec if is_type_diagnostic_item(cx, cx.typeck_results().expr_ty(self_expr), sym::Vec); if path.ident.name.as_str() == "push"; diff --git a/clippy_lints/src/manual_async_fn.rs b/clippy_lints/src/manual_async_fn.rs index 914ceca2995d4..5a87e75722d01 100644 --- a/clippy_lints/src/manual_async_fn.rs +++ b/clippy_lints/src/manual_async_fn.rs @@ -4,7 +4,7 @@ use if_chain::if_chain; use rustc_errors::Applicability; use rustc_hir::intravisit::FnKind; use rustc_hir::{ - AsyncCoroutineKind, Block, Body, Closure, Expr, ExprKind, FnDecl, FnRetTy, CoroutineKind, GenericArg, GenericBound, + AsyncCoroutineKind, Block, Body, Closure, CoroutineKind, Expr, ExprKind, FnDecl, FnRetTy, GenericArg, GenericBound, ImplItem, Item, ItemKind, LifetimeName, Node, Term, TraitRef, Ty, TyKind, TypeBindingKind, }; use rustc_lint::{LateContext, LateLintPass}; diff --git a/clippy_lints/src/manual_bits.rs b/clippy_lints/src/manual_bits.rs index 6c7c57ba1d6b6..552c57d5e0253 100644 --- a/clippy_lints/src/manual_bits.rs +++ b/clippy_lints/src/manual_bits.rs @@ -103,9 +103,9 @@ fn get_size_of_ty<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) -> Option< if let ExprKind::Path(ref count_func_qpath) = count_func.kind; if let QPath::Resolved(_, count_func_path) = count_func_qpath; - if let Some(segment_zero) = count_func_path.segments.get(0); + if let Some(segment_zero) = count_func_path.segments.first(); if let Some(args) = segment_zero.args; - if let Some(GenericArg::Type(real_ty)) = args.args.get(0); + if let Some(GenericArg::Type(real_ty)) = args.args.first(); if let Some(def_id) = cx.qpath_res(count_func_qpath, count_func.hir_id).opt_def_id(); if cx.tcx.is_diagnostic_item(sym::mem_size_of, def_id); diff --git a/clippy_lints/src/manual_is_ascii_check.rs b/clippy_lints/src/manual_is_ascii_check.rs index f264424470da1..9da20a28fba50 100644 --- a/clippy_lints/src/manual_is_ascii_check.rs +++ b/clippy_lints/src/manual_is_ascii_check.rs @@ -15,12 +15,13 @@ use rustc_span::{sym, Span}; declare_clippy_lint! { /// ### What it does /// Suggests to use dedicated built-in methods, - /// `is_ascii_(lowercase|uppercase|digit)` for checking on corresponding ascii range + /// `is_ascii_(lowercase|uppercase|digit|hexdigit)` for checking on corresponding + /// ascii range /// /// ### Why is this bad? /// Using the built-in functions is more readable and makes it /// clear that it's not a specific subset of characters, but all - /// ASCII (lowercase|uppercase|digit) characters. + /// ASCII (lowercase|uppercase|digit|hexdigit) characters. /// ### Example /// ```rust /// fn main() { @@ -28,6 +29,7 @@ declare_clippy_lint! { /// assert!(matches!(b'X', b'A'..=b'Z')); /// assert!(matches!('2', '0'..='9')); /// assert!(matches!('x', 'A'..='Z' | 'a'..='z')); + /// assert!(matches!('C', '0'..='9' | 'a'..='f' | 'A'..='F')); /// /// ('0'..='9').contains(&'0'); /// ('a'..='z').contains(&'a'); @@ -41,6 +43,7 @@ declare_clippy_lint! { /// assert!(b'X'.is_ascii_uppercase()); /// assert!('2'.is_ascii_digit()); /// assert!('x'.is_ascii_alphabetic()); + /// assert!('C'.is_ascii_hexdigit()); /// /// '0'.is_ascii_digit(); /// 'a'.is_ascii_lowercase(); @@ -75,6 +78,12 @@ enum CharRange { FullChar, /// '0..=9' Digit, + /// 'a..=f' + LowerHexLetter, + /// 'A..=F' + UpperHexLetter, + /// '0..=9' | 'a..=f' | 'A..=F' + HexDigit, Otherwise, } @@ -116,7 +125,8 @@ fn check_is_ascii(cx: &LateContext<'_>, span: Span, recv: &Expr<'_>, range: &Cha CharRange::LowerChar => Some("is_ascii_lowercase"), CharRange::FullChar => Some("is_ascii_alphabetic"), CharRange::Digit => Some("is_ascii_digit"), - CharRange::Otherwise => None, + CharRange::HexDigit => Some("is_ascii_hexdigit"), + CharRange::Otherwise | CharRange::LowerHexLetter | CharRange::UpperHexLetter => None, } { let default_snip = ".."; let mut app = Applicability::MachineApplicable; @@ -141,6 +151,12 @@ fn check_pat(pat_kind: &PatKind<'_>) -> CharRange { if ranges.len() == 2 && ranges.contains(&CharRange::UpperChar) && ranges.contains(&CharRange::LowerChar) { CharRange::FullChar + } else if ranges.len() == 3 + && ranges.contains(&CharRange::Digit) + && ranges.contains(&CharRange::LowerHexLetter) + && ranges.contains(&CharRange::UpperHexLetter) + { + CharRange::HexDigit } else { CharRange::Otherwise } @@ -156,6 +172,8 @@ fn check_range(start: &Expr<'_>, end: &Expr<'_>) -> CharRange { match (&start_lit.node, &end_lit.node) { (Char('a'), Char('z')) | (Byte(b'a'), Byte(b'z')) => CharRange::LowerChar, (Char('A'), Char('Z')) | (Byte(b'A'), Byte(b'Z')) => CharRange::UpperChar, + (Char('a'), Char('f')) | (Byte(b'a'), Byte(b'f')) => CharRange::LowerHexLetter, + (Char('A'), Char('F')) | (Byte(b'A'), Byte(b'F')) => CharRange::UpperHexLetter, (Char('0'), Char('9')) | (Byte(b'0'), Byte(b'9')) => CharRange::Digit, _ => CharRange::Otherwise, } diff --git a/clippy_lints/src/matches/mod.rs b/clippy_lints/src/matches/mod.rs index a23000e5fe15c..b5ab94b3a2f8e 100644 --- a/clippy_lints/src/matches/mod.rs +++ b/clippy_lints/src/matches/mod.rs @@ -967,7 +967,6 @@ declare_clippy_lint! { "checks for unnecessary guards in match expressions" } -#[derive(Default)] pub struct Matches { msrv: Msrv, infallible_destructuring_match_linted: bool, @@ -978,7 +977,7 @@ impl Matches { pub fn new(msrv: Msrv) -> Self { Self { msrv, - ..Matches::default() + infallible_destructuring_match_linted: false, } } } diff --git a/clippy_lints/src/methods/filter_map_identity.rs b/clippy_lints/src/methods/filter_map_identity.rs index 3337b250c0e79..20878f1e4dfe6 100644 --- a/clippy_lints/src/methods/filter_map_identity.rs +++ b/clippy_lints/src/methods/filter_map_identity.rs @@ -1,5 +1,5 @@ use clippy_utils::diagnostics::span_lint_and_sugg; -use clippy_utils::{is_expr_identity_function, is_trait_method}; +use clippy_utils::{is_expr_untyped_identity_function, is_trait_method}; use rustc_errors::Applicability; use rustc_hir as hir; use rustc_lint::LateContext; @@ -9,7 +9,7 @@ use rustc_span::sym; use super::FILTER_MAP_IDENTITY; pub(super) fn check(cx: &LateContext<'_>, expr: &hir::Expr<'_>, filter_map_arg: &hir::Expr<'_>, filter_map_span: Span) { - if is_trait_method(cx, expr, sym::Iterator) && is_expr_identity_function(cx, filter_map_arg) { + if is_trait_method(cx, expr, sym::Iterator) && is_expr_untyped_identity_function(cx, filter_map_arg) { span_lint_and_sugg( cx, FILTER_MAP_IDENTITY, diff --git a/clippy_lints/src/methods/flat_map_identity.rs b/clippy_lints/src/methods/flat_map_identity.rs index 84a21de0ac860..8849a4f49420d 100644 --- a/clippy_lints/src/methods/flat_map_identity.rs +++ b/clippy_lints/src/methods/flat_map_identity.rs @@ -1,5 +1,5 @@ use clippy_utils::diagnostics::span_lint_and_sugg; -use clippy_utils::{is_expr_identity_function, is_trait_method}; +use clippy_utils::{is_expr_untyped_identity_function, is_trait_method}; use rustc_errors::Applicability; use rustc_hir as hir; use rustc_lint::LateContext; @@ -15,7 +15,7 @@ pub(super) fn check<'tcx>( flat_map_arg: &'tcx hir::Expr<'_>, flat_map_span: Span, ) { - if is_trait_method(cx, expr, sym::Iterator) && is_expr_identity_function(cx, flat_map_arg) { + if is_trait_method(cx, expr, sym::Iterator) && is_expr_untyped_identity_function(cx, flat_map_arg) { span_lint_and_sugg( cx, FLAT_MAP_IDENTITY, diff --git a/clippy_lints/src/methods/get_first.rs b/clippy_lints/src/methods/get_first.rs index ee063adac64af..e7b4564c651ef 100644 --- a/clippy_lints/src/methods/get_first.rs +++ b/clippy_lints/src/methods/get_first.rs @@ -1,5 +1,4 @@ use clippy_utils::diagnostics::span_lint_and_sugg; -use clippy_utils::is_slice_of_primitives; use clippy_utils::source::snippet_with_applicability; use if_chain::if_chain; use rustc_ast::LitKind; @@ -20,7 +19,6 @@ pub(super) fn check<'tcx>( if let Some(method_id) = cx.typeck_results().type_dependent_def_id(expr.hir_id); if let Some(impl_id) = cx.tcx.impl_of_method(method_id); if cx.tcx.type_of(impl_id).instantiate_identity().is_slice(); - if let Some(_) = is_slice_of_primitives(cx, recv); if let hir::ExprKind::Lit(Spanned { node: LitKind::Int(0, _), .. }) = arg.kind; then { let mut app = Applicability::MachineApplicable; diff --git a/clippy_lints/src/methods/map_identity.rs b/clippy_lints/src/methods/map_identity.rs index 7be1ce483f63f..57581363cfa0c 100644 --- a/clippy_lints/src/methods/map_identity.rs +++ b/clippy_lints/src/methods/map_identity.rs @@ -1,6 +1,6 @@ use clippy_utils::diagnostics::span_lint_and_sugg; use clippy_utils::ty::is_type_diagnostic_item; -use clippy_utils::{is_expr_identity_function, is_trait_method}; +use clippy_utils::{is_expr_untyped_identity_function, is_trait_method}; use rustc_errors::Applicability; use rustc_hir as hir; use rustc_lint::LateContext; @@ -23,7 +23,7 @@ pub(super) fn check( if is_trait_method(cx, expr, sym::Iterator) || is_type_diagnostic_item(cx, caller_ty, sym::Result) || is_type_diagnostic_item(cx, caller_ty, sym::Option); - if is_expr_identity_function(cx, map_arg); + if is_expr_untyped_identity_function(cx, map_arg); if let Some(sugg_span) = expr.span.trim_start(caller.span); then { span_lint_and_sugg( diff --git a/clippy_lints/src/methods/search_is_some.rs b/clippy_lints/src/methods/search_is_some.rs index afdb8ce94ac43..04ddaaa2f4617 100644 --- a/clippy_lints/src/methods/search_is_some.rs +++ b/clippy_lints/src/methods/search_is_some.rs @@ -39,7 +39,7 @@ pub(super) fn check<'tcx>( if search_method == "find"; if let hir::ExprKind::Closure(&hir::Closure { body, .. }) = search_arg.kind; let closure_body = cx.tcx.hir().body(body); - if let Some(closure_arg) = closure_body.params.get(0); + if let Some(closure_arg) = closure_body.params.first(); then { if let hir::PatKind::Ref(..) = closure_arg.pat.kind { Some(search_snippet.replacen('&', "", 1)) diff --git a/clippy_lints/src/methods/unnecessary_lazy_eval.rs b/clippy_lints/src/methods/unnecessary_lazy_eval.rs index 47e2e744112c5..4a651396f14d0 100644 --- a/clippy_lints/src/methods/unnecessary_lazy_eval.rs +++ b/clippy_lints/src/methods/unnecessary_lazy_eval.rs @@ -2,6 +2,7 @@ use clippy_utils::diagnostics::span_lint_and_then; use clippy_utils::source::snippet; use clippy_utils::ty::is_type_diagnostic_item; use clippy_utils::{eager_or_lazy, is_from_proc_macro, usage}; +use hir::FnRetTy; use rustc_errors::Applicability; use rustc_hir as hir; use rustc_lint::LateContext; @@ -27,7 +28,7 @@ pub(super) fn check<'tcx>( let is_bool = cx.typeck_results().expr_ty(recv).is_bool(); if is_option || is_result || is_bool { - if let hir::ExprKind::Closure(&hir::Closure { body, .. }) = arg.kind { + if let hir::ExprKind::Closure(&hir::Closure { body, fn_decl, .. }) = arg.kind { let body = cx.tcx.hir().body(body); let body_expr = &body.value; @@ -48,7 +49,14 @@ pub(super) fn check<'tcx>( .iter() // bindings are checked to be unused above .all(|param| matches!(param.pat.kind, hir::PatKind::Binding(..) | hir::PatKind::Wild)) - { + && matches!( + fn_decl.output, + FnRetTy::DefaultReturn(_) + | FnRetTy::Return(hir::Ty { + kind: hir::TyKind::Infer, + .. + }) + ) { Applicability::MachineApplicable } else { // replacing the lambda may break type inference diff --git a/clippy_lints/src/missing_doc.rs b/clippy_lints/src/missing_doc.rs index f2773cad400cc..0629dee4f722f 100644 --- a/clippy_lints/src/missing_doc.rs +++ b/clippy_lints/src/missing_doc.rs @@ -67,7 +67,7 @@ impl MissingDoc { if_chain! { if let Some(meta) = meta; if let MetaItemKind::List(list) = meta.kind; - if let Some(meta) = list.get(0); + if let Some(meta) = list.first(); if let Some(name) = meta.ident(); then { name.name == sym::include diff --git a/clippy_lints/src/missing_enforced_import_rename.rs b/clippy_lints/src/missing_enforced_import_rename.rs index 96d83e114a410..fc088710e4295 100644 --- a/clippy_lints/src/missing_enforced_import_rename.rs +++ b/clippy_lints/src/missing_enforced_import_rename.rs @@ -17,6 +17,9 @@ declare_clippy_lint! { /// Checks for imports that do not rename the item as specified /// in the `enforce-import-renames` config option. /// + /// Note: Even though this lint is warn-by-default, it will only trigger if + /// import renames are defined in the clippy.toml file. + /// /// ### Why is this bad? /// Consistency is important, if a project has defined import /// renames they should be followed. More practically, some item names are too @@ -38,7 +41,7 @@ declare_clippy_lint! { /// ``` #[clippy::version = "1.55.0"] pub MISSING_ENFORCED_IMPORT_RENAMES, - restriction, + style, "enforce import renames" } diff --git a/clippy_lints/src/missing_fields_in_debug.rs b/clippy_lints/src/missing_fields_in_debug.rs index f0b865be8042d..d205237a5915c 100644 --- a/clippy_lints/src/missing_fields_in_debug.rs +++ b/clippy_lints/src/missing_fields_in_debug.rs @@ -1,9 +1,9 @@ use std::ops::ControlFlow; use clippy_utils::diagnostics::span_lint_and_then; +use clippy_utils::is_path_lang_item; use clippy_utils::ty::is_type_diagnostic_item; use clippy_utils::visitors::{for_each_expr, Visitable}; -use clippy_utils::is_path_lang_item; use rustc_ast::LitKind; use rustc_data_structures::fx::FxHashSet; use rustc_hir::def::{DefKind, Res}; diff --git a/clippy_lints/src/multiple_unsafe_ops_per_block.rs b/clippy_lints/src/multiple_unsafe_ops_per_block.rs index fe35126aab216..2c42a7a3676a1 100644 --- a/clippy_lints/src/multiple_unsafe_ops_per_block.rs +++ b/clippy_lints/src/multiple_unsafe_ops_per_block.rs @@ -9,7 +9,7 @@ use rustc_lint::{LateContext, LateLintPass}; use rustc_middle::lint::in_external_macro; use rustc_middle::ty; use rustc_session::{declare_lint_pass, declare_tool_lint}; -use rustc_span::Span; +use rustc_span::{DesugaringKind, Span}; declare_clippy_lint! { /// ### What it does @@ -64,7 +64,10 @@ declare_lint_pass!(MultipleUnsafeOpsPerBlock => [MULTIPLE_UNSAFE_OPS_PER_BLOCK]) impl<'tcx> LateLintPass<'tcx> for MultipleUnsafeOpsPerBlock { fn check_block(&mut self, cx: &LateContext<'tcx>, block: &'tcx hir::Block<'_>) { - if !matches!(block.rules, BlockCheckMode::UnsafeBlock(_)) || in_external_macro(cx.tcx.sess, block.span) { + if !matches!(block.rules, BlockCheckMode::UnsafeBlock(_)) + || in_external_macro(cx.tcx.sess, block.span) + || block.span.is_desugaring(DesugaringKind::Await) + { return; } let mut unsafe_ops = vec![]; diff --git a/clippy_lints/src/needless_continue.rs b/clippy_lints/src/needless_continue.rs index 38a75034cd314..377cbef7b99e3 100644 --- a/clippy_lints/src/needless_continue.rs +++ b/clippy_lints/src/needless_continue.rs @@ -189,7 +189,7 @@ fn needless_continue_in_else(else_expr: &ast::Expr, label: Option<&ast::Label>) } fn is_first_block_stmt_continue(block: &ast::Block, label: Option<&ast::Label>) -> bool { - block.stmts.get(0).map_or(false, |stmt| match stmt.kind { + block.stmts.first().map_or(false, |stmt| match stmt.kind { ast::StmtKind::Semi(ref e) | ast::StmtKind::Expr(ref e) => { if let ast::ExprKind::Continue(ref l) = e.kind { compare_labels(label, l.as_ref()) @@ -434,7 +434,7 @@ fn erode_from_back(s: &str) -> String { } fn span_of_first_expr_in_block(block: &ast::Block) -> Option { - block.stmts.get(0).map(|stmt| stmt.span) + block.stmts.first().map(|stmt| stmt.span) } #[cfg(test)] diff --git a/clippy_lints/src/needless_pass_by_ref_mut.rs b/clippy_lints/src/needless_pass_by_ref_mut.rs index 212d6234bdb3c..1d5874f6feacb 100644 --- a/clippy_lints/src/needless_pass_by_ref_mut.rs +++ b/clippy_lints/src/needless_pass_by_ref_mut.rs @@ -7,7 +7,8 @@ use rustc_data_structures::fx::{FxHashSet, FxIndexMap}; use rustc_errors::Applicability; use rustc_hir::intravisit::{walk_qpath, FnKind, Visitor}; use rustc_hir::{ - Body, Closure, Expr, ExprKind, FnDecl, HirId, HirIdMap, HirIdSet, Impl, ItemKind, Mutability, Node, PatKind, QPath, + BlockCheckMode, Body, Closure, Expr, ExprKind, FnDecl, HirId, HirIdMap, HirIdSet, Impl, ItemKind, Mutability, Node, + PatKind, QPath, }; use rustc_hir_typeck::expr_use_visitor as euv; use rustc_infer::infer::{InferCtxt, TyCtxtInferExt}; @@ -139,13 +140,23 @@ impl<'tcx> LateLintPass<'tcx> for NeedlessPassByRefMut<'tcx> { let hir_id = cx.tcx.hir().local_def_id_to_hir_id(fn_def_id); let is_async = match kind { FnKind::ItemFn(.., header) => { + if header.is_unsafe() { + // We don't check unsafe functions. + return; + } let attrs = cx.tcx.hir().attrs(hir_id); if header.abi != Abi::Rust || requires_exact_signature(attrs) { return; } header.is_async() }, - FnKind::Method(.., sig) => sig.header.is_async(), + FnKind::Method(.., sig) => { + if sig.header.is_unsafe() { + // We don't check unsafe functions. + return; + } + sig.header.is_async() + }, FnKind::Closure => return, }; @@ -186,20 +197,21 @@ impl<'tcx> LateLintPass<'tcx> for NeedlessPassByRefMut<'tcx> { }; let infcx = cx.tcx.infer_ctxt().build(); euv::ExprUseVisitor::new(&mut ctx, &infcx, fn_def_id, cx.param_env, cx.typeck_results()).consume_body(body); - if is_async { - let mut checked_closures = FxHashSet::default(); - - // We retrieve all the closures declared in the async function because they will - // not be found by `euv::Delegate`. - let mut closures: FxHashSet = FxHashSet::default(); - for_each_expr_with_closures(cx, body, |expr| { - if let ExprKind::Closure(closure) = expr.kind { - closures.insert(closure.def_id); - } - ControlFlow::<()>::Continue(()) - }); - check_closures(&mut ctx, cx, &infcx, &mut checked_closures, closures); + let mut checked_closures = FxHashSet::default(); + + // We retrieve all the closures declared in the function because they will not be found + // by `euv::Delegate`. + let mut closures: FxHashSet = FxHashSet::default(); + for_each_expr_with_closures(cx, body, |expr| { + if let ExprKind::Closure(closure) = expr.kind { + closures.insert(closure.def_id); + } + ControlFlow::<()>::Continue(()) + }); + check_closures(&mut ctx, cx, &infcx, &mut checked_closures, closures); + + if is_async { while !ctx.async_closures.is_empty() { let async_closures = ctx.async_closures.clone(); ctx.async_closures.clear(); @@ -304,10 +316,27 @@ impl<'tcx> MutablyUsedVariablesCtxt<'tcx> { } self.aliases.insert(alias, target); } + + // The goal here is to find if the current scope is unsafe or not. It stops when it finds + // a function or an unsafe block. + fn is_in_unsafe_block(&self, item: HirId) -> bool { + let hir = self.tcx.hir(); + for (parent, node) in hir.parent_iter(item) { + if let Some(fn_sig) = hir.fn_sig_by_hir_id(parent) { + return fn_sig.header.is_unsafe(); + } else if let Node::Block(block) = node { + if matches!(block.rules, BlockCheckMode::UnsafeBlock(_)) { + return true; + } + } + } + false + } } impl<'tcx> euv::Delegate<'tcx> for MutablyUsedVariablesCtxt<'tcx> { - fn consume(&mut self, cmt: &euv::PlaceWithHirId<'tcx>, _id: HirId) { + #[allow(clippy::if_same_then_else)] + fn consume(&mut self, cmt: &euv::PlaceWithHirId<'tcx>, id: HirId) { if let euv::Place { base: euv::PlaceBase::Local(vid) @@ -327,13 +356,18 @@ impl<'tcx> euv::Delegate<'tcx> for MutablyUsedVariablesCtxt<'tcx> { && matches!(base_ty.ref_mutability(), Some(Mutability::Mut)) { self.add_mutably_used_var(*vid); + } else if self.is_in_unsafe_block(id) { + // If we are in an unsafe block, any operation on this variable must not be warned + // upon! + self.add_mutably_used_var(*vid); } self.prev_bind = None; self.prev_move_to_closure.remove(vid); } } - fn borrow(&mut self, cmt: &euv::PlaceWithHirId<'tcx>, _id: HirId, borrow: ty::BorrowKind) { + #[allow(clippy::if_same_then_else)] + fn borrow(&mut self, cmt: &euv::PlaceWithHirId<'tcx>, id: HirId, borrow: ty::BorrowKind) { self.prev_bind = None; if let euv::Place { base: @@ -355,6 +389,10 @@ impl<'tcx> euv::Delegate<'tcx> for MutablyUsedVariablesCtxt<'tcx> { || (borrow == ty::BorrowKind::UniqueImmBorrow && base_ty.ref_mutability() == Some(Mutability::Mut)) { self.add_mutably_used_var(*vid); + } else if self.is_in_unsafe_block(id) { + // If we are in an unsafe block, any operation on this variable must not be warned + // upon! + self.add_mutably_used_var(*vid); } } else if borrow == ty::ImmBorrow { // If there is an `async block`, it'll contain a call to a closure which we need to @@ -397,7 +435,21 @@ impl<'tcx> euv::Delegate<'tcx> for MutablyUsedVariablesCtxt<'tcx> { } } - fn copy(&mut self, _cmt: &euv::PlaceWithHirId<'tcx>, _id: HirId) { + fn copy(&mut self, cmt: &euv::PlaceWithHirId<'tcx>, id: HirId) { + if let euv::Place { + base: + euv::PlaceBase::Local(vid) + | euv::PlaceBase::Upvar(UpvarId { + var_path: UpvarPath { hir_id: vid }, + .. + }), + .. + } = &cmt.place + { + if self.is_in_unsafe_block(id) { + self.add_mutably_used_var(*vid); + } + } self.prev_bind = None; } @@ -427,8 +479,22 @@ impl<'tcx> euv::Delegate<'tcx> for MutablyUsedVariablesCtxt<'tcx> { } } - fn bind(&mut self, _cmt: &euv::PlaceWithHirId<'tcx>, id: HirId) { + fn bind(&mut self, cmt: &euv::PlaceWithHirId<'tcx>, id: HirId) { self.prev_bind = Some(id); + if let euv::Place { + base: + euv::PlaceBase::Local(vid) + | euv::PlaceBase::Upvar(UpvarId { + var_path: UpvarPath { hir_id: vid }, + .. + }), + .. + } = &cmt.place + { + if self.is_in_unsafe_block(id) { + self.add_mutably_used_var(*vid); + } + } } } diff --git a/clippy_lints/src/needless_pass_by_value.rs b/clippy_lints/src/needless_pass_by_value.rs index 6d7df2eac6291..f3c0d616473f3 100644 --- a/clippy_lints/src/needless_pass_by_value.rs +++ b/clippy_lints/src/needless_pass_by_value.rs @@ -1,10 +1,10 @@ use clippy_utils::diagnostics::{multispan_sugg, span_lint_and_then}; +use clippy_utils::is_self; use clippy_utils::ptr::get_spans; use clippy_utils::source::{snippet, snippet_opt}; use clippy_utils::ty::{ implements_trait, implements_trait_with_env_from_iter, is_copy, is_type_diagnostic_item, is_type_lang_item, }; -use clippy_utils::is_self; use if_chain::if_chain; use rustc_ast::ast::Attribute; use rustc_errors::{Applicability, Diagnostic}; diff --git a/clippy_lints/src/needless_question_mark.rs b/clippy_lints/src/needless_question_mark.rs index d267b3de7f25a..ed279a3813d70 100644 --- a/clippy_lints/src/needless_question_mark.rs +++ b/clippy_lints/src/needless_question_mark.rs @@ -4,7 +4,7 @@ use clippy_utils::source::snippet; use if_chain::if_chain; use rustc_errors::Applicability; use rustc_hir::def::{DefKind, Res}; -use rustc_hir::{AsyncCoroutineKind, Block, Body, Expr, ExprKind, CoroutineKind, LangItem, MatchSource, QPath}; +use rustc_hir::{AsyncCoroutineKind, Block, Body, CoroutineKind, Expr, ExprKind, LangItem, MatchSource, QPath}; use rustc_lint::{LateContext, LateLintPass}; use rustc_session::{declare_lint_pass, declare_tool_lint}; diff --git a/clippy_lints/src/non_copy_const.rs b/clippy_lints/src/non_copy_const.rs index 2b4e3260c56b1..613afa46a9126 100644 --- a/clippy_lints/src/non_copy_const.rs +++ b/clippy_lints/src/non_copy_const.rs @@ -5,9 +5,10 @@ use std::ptr; use clippy_utils::diagnostics::span_lint_and_then; -use clippy_utils::in_constant; use clippy_utils::macros::macro_backtrace; +use clippy_utils::{def_path_def_ids, in_constant}; use if_chain::if_chain; +use rustc_data_structures::fx::FxHashSet; use rustc_hir::def::{DefKind, Res}; use rustc_hir::def_id::DefId; use rustc_hir::{ @@ -15,9 +16,10 @@ use rustc_hir::{ }; use rustc_lint::{LateContext, LateLintPass, Lint}; use rustc_middle::mir::interpret::{ErrorHandled, EvalToValTreeResult, GlobalId}; +use rustc_middle::query::Key; use rustc_middle::ty::adjustment::Adjust; use rustc_middle::ty::{self, Ty, TyCtxt}; -use rustc_session::{declare_lint_pass, declare_tool_lint}; +use rustc_session::{declare_tool_lint, impl_lint_pass}; use rustc_span::{sym, InnerSpan, Span}; use rustc_target::abi::VariantIdx; @@ -126,128 +128,6 @@ declare_clippy_lint! { "referencing `const` with interior mutability" } -fn is_unfrozen<'tcx>(cx: &LateContext<'tcx>, ty: Ty<'tcx>) -> bool { - // Ignore types whose layout is unknown since `is_freeze` reports every generic types as `!Freeze`, - // making it indistinguishable from `UnsafeCell`. i.e. it isn't a tool to prove a type is - // 'unfrozen'. However, this code causes a false negative in which - // a type contains a layout-unknown type, but also an unsafe cell like `const CELL: Cell`. - // Yet, it's better than `ty.has_type_flags(TypeFlags::HAS_TY_PARAM | TypeFlags::HAS_PROJECTION)` - // since it works when a pointer indirection involves (`Cell<*const T>`). - // Making up a `ParamEnv` where every generic params and assoc types are `Freeze`is another option; - // but I'm not sure whether it's a decent way, if possible. - cx.tcx.layout_of(cx.param_env.and(ty)).is_ok() && !ty.is_freeze(cx.tcx, cx.param_env) -} - -fn is_value_unfrozen_raw<'tcx>( - cx: &LateContext<'tcx>, - result: Result>, ErrorHandled>, - ty: Ty<'tcx>, -) -> bool { - fn inner<'tcx>(cx: &LateContext<'tcx>, val: ty::ValTree<'tcx>, ty: Ty<'tcx>) -> bool { - match *ty.kind() { - // the fact that we have to dig into every structs to search enums - // leads us to the point checking `UnsafeCell` directly is the only option. - ty::Adt(ty_def, ..) if ty_def.is_unsafe_cell() => true, - // As of 2022-09-08 miri doesn't track which union field is active so there's no safe way to check the - // contained value. - ty::Adt(def, ..) if def.is_union() => false, - ty::Array(ty, _) => val.unwrap_branch().iter().any(|field| inner(cx, *field, ty)), - ty::Adt(def, _) if def.is_union() => false, - ty::Adt(def, args) if def.is_enum() => { - let (&variant_index, fields) = val.unwrap_branch().split_first().unwrap(); - let variant_index = VariantIdx::from_u32(variant_index.unwrap_leaf().try_to_u32().ok().unwrap()); - fields - .iter() - .copied() - .zip( - def.variants()[variant_index] - .fields - .iter() - .map(|field| field.ty(cx.tcx, args)), - ) - .any(|(field, ty)| inner(cx, field, ty)) - }, - ty::Adt(def, args) => val - .unwrap_branch() - .iter() - .zip(def.non_enum_variant().fields.iter().map(|field| field.ty(cx.tcx, args))) - .any(|(field, ty)| inner(cx, *field, ty)), - ty::Tuple(tys) => val - .unwrap_branch() - .iter() - .zip(tys) - .any(|(field, ty)| inner(cx, *field, ty)), - _ => false, - } - } - result.map_or_else( - |err| { - // Consider `TooGeneric` cases as being unfrozen. - // This causes a false positive where an assoc const whose type is unfrozen - // have a value that is a frozen variant with a generic param (an example is - // `declare_interior_mutable_const::enums::BothOfCellAndGeneric::GENERIC_VARIANT`). - // However, it prevents a number of false negatives that is, I think, important: - // 1. assoc consts in trait defs referring to consts of themselves (an example is - // `declare_interior_mutable_const::traits::ConcreteTypes::ANOTHER_ATOMIC`). - // 2. a path expr referring to assoc consts whose type is doesn't have any frozen variants in trait - // defs (i.e. without substitute for `Self`). (e.g. borrowing - // `borrow_interior_mutable_const::trait::ConcreteTypes::ATOMIC`) - // 3. similar to the false positive above; but the value is an unfrozen variant, or the type has no - // enums. (An example is - // `declare_interior_mutable_const::enums::BothOfCellAndGeneric::UNFROZEN_VARIANT` and - // `declare_interior_mutable_const::enums::BothOfCellAndGeneric::NO_ENUM`). - // One might be able to prevent these FNs correctly, and replace this with `false`; - // e.g. implementing `has_frozen_variant` described above, and not running this function - // when the type doesn't have any frozen variants would be the 'correct' way for the 2nd - // case (that actually removes another suboptimal behavior (I won't say 'false positive') where, - // similar to 2., but with the a frozen variant) (e.g. borrowing - // `borrow_interior_mutable_const::enums::AssocConsts::TO_BE_FROZEN_VARIANT`). - // I chose this way because unfrozen enums as assoc consts are rare (or, hopefully, none). - matches!(err, ErrorHandled::TooGeneric(..)) - }, - |val| val.map_or(true, |val| inner(cx, val, ty)), - ) -} - -fn is_value_unfrozen_poly<'tcx>(cx: &LateContext<'tcx>, body_id: BodyId, ty: Ty<'tcx>) -> bool { - let def_id = body_id.hir_id.owner.to_def_id(); - let args = ty::GenericArgs::identity_for_item(cx.tcx, def_id); - let instance = ty::Instance::new(def_id, args); - let cid = rustc_middle::mir::interpret::GlobalId { - instance, - promoted: None, - }; - let param_env = cx.tcx.param_env(def_id).with_reveal_all_normalized(cx.tcx); - let result = cx.tcx.const_eval_global_id_for_typeck(param_env, cid, None); - is_value_unfrozen_raw(cx, result, ty) -} - -fn is_value_unfrozen_expr<'tcx>(cx: &LateContext<'tcx>, hir_id: HirId, def_id: DefId, ty: Ty<'tcx>) -> bool { - let args = cx.typeck_results().node_args(hir_id); - - let result = const_eval_resolve(cx.tcx, cx.param_env, ty::UnevaluatedConst::new(def_id, args), None); - is_value_unfrozen_raw(cx, result, ty) -} - -pub fn const_eval_resolve<'tcx>( - tcx: TyCtxt<'tcx>, - param_env: ty::ParamEnv<'tcx>, - ct: ty::UnevaluatedConst<'tcx>, - span: Option, -) -> EvalToValTreeResult<'tcx> { - match ty::Instance::resolve(tcx, param_env, ct.def, ct.args) { - Ok(Some(instance)) => { - let cid = GlobalId { - instance, - promoted: None, - }; - tcx.const_eval_global_id_for_typeck(param_env, cid, span) - }, - Ok(None) => Err(ErrorHandled::TooGeneric(span.unwrap_or(rustc_span::DUMMY_SP))), - Err(err) => Err(ErrorHandled::Reported(err.into(), span.unwrap_or(rustc_span::DUMMY_SP))), - } -} - #[derive(Copy, Clone)] enum Source { Item { item: Span }, @@ -292,13 +172,178 @@ fn lint(cx: &LateContext<'_>, source: Source) { }); } -declare_lint_pass!(NonCopyConst => [DECLARE_INTERIOR_MUTABLE_CONST, BORROW_INTERIOR_MUTABLE_CONST]); +#[derive(Clone)] +pub struct NonCopyConst { + ignore_interior_mutability: Vec, + ignore_mut_def_ids: FxHashSet, +} + +impl_lint_pass!(NonCopyConst => [DECLARE_INTERIOR_MUTABLE_CONST, BORROW_INTERIOR_MUTABLE_CONST]); + +impl NonCopyConst { + pub fn new(ignore_interior_mutability: Vec) -> Self { + Self { + ignore_interior_mutability, + ignore_mut_def_ids: FxHashSet::default(), + } + } + + fn is_ty_ignored(&self, ty: Ty<'_>) -> bool { + matches!(ty.ty_adt_id(), Some(adt_id) if self.ignore_mut_def_ids.contains(&adt_id)) + } + + fn is_unfrozen<'tcx>(cx: &LateContext<'tcx>, ty: Ty<'tcx>) -> bool { + // Ignore types whose layout is unknown since `is_freeze` reports every generic types as `!Freeze`, + // making it indistinguishable from `UnsafeCell`. i.e. it isn't a tool to prove a type is + // 'unfrozen'. However, this code causes a false negative in which + // a type contains a layout-unknown type, but also an unsafe cell like `const CELL: Cell`. + // Yet, it's better than `ty.has_type_flags(TypeFlags::HAS_TY_PARAM | TypeFlags::HAS_PROJECTION)` + // since it works when a pointer indirection involves (`Cell<*const T>`). + // Making up a `ParamEnv` where every generic params and assoc types are `Freeze`is another option; + // but I'm not sure whether it's a decent way, if possible. + cx.tcx.layout_of(cx.param_env.and(ty)).is_ok() && !ty.is_freeze(cx.tcx, cx.param_env) + } + + fn is_value_unfrozen_raw_inner<'tcx>(&self, cx: &LateContext<'tcx>, val: ty::ValTree<'tcx>, ty: Ty<'tcx>) -> bool { + if self.is_ty_ignored(ty) { + return false; + } + match *ty.kind() { + // the fact that we have to dig into every structs to search enums + // leads us to the point checking `UnsafeCell` directly is the only option. + ty::Adt(ty_def, ..) if ty_def.is_unsafe_cell() => true, + // As of 2022-09-08 miri doesn't track which union field is active so there's no safe way to check the + // contained value. + ty::Adt(def, ..) if def.is_union() => false, + ty::Array(ty, _) => val + .unwrap_branch() + .iter() + .any(|field| self.is_value_unfrozen_raw_inner(cx, *field, ty)), + ty::Adt(def, _) if def.is_union() => false, + ty::Adt(def, args) if def.is_enum() => { + let (&variant_index, fields) = val.unwrap_branch().split_first().unwrap(); + let variant_index = VariantIdx::from_u32(variant_index.unwrap_leaf().try_to_u32().ok().unwrap()); + fields + .iter() + .copied() + .zip( + def.variants()[variant_index] + .fields + .iter() + .map(|field| field.ty(cx.tcx, args)), + ) + .any(|(field, ty)| self.is_value_unfrozen_raw_inner(cx, field, ty)) + }, + ty::Adt(def, args) => val + .unwrap_branch() + .iter() + .zip(def.non_enum_variant().fields.iter().map(|field| field.ty(cx.tcx, args))) + .any(|(field, ty)| self.is_value_unfrozen_raw_inner(cx, *field, ty)), + ty::Tuple(tys) => val + .unwrap_branch() + .iter() + .zip(tys) + .any(|(field, ty)| self.is_value_unfrozen_raw_inner(cx, *field, ty)), + _ => false, + } + } + + fn is_value_unfrozen_raw<'tcx>( + &self, + cx: &LateContext<'tcx>, + result: Result>, ErrorHandled>, + ty: Ty<'tcx>, + ) -> bool { + result.map_or_else( + |err| { + // Consider `TooGeneric` cases as being unfrozen. + // This causes a false positive where an assoc const whose type is unfrozen + // have a value that is a frozen variant with a generic param (an example is + // `declare_interior_mutable_const::enums::BothOfCellAndGeneric::GENERIC_VARIANT`). + // However, it prevents a number of false negatives that is, I think, important: + // 1. assoc consts in trait defs referring to consts of themselves (an example is + // `declare_interior_mutable_const::traits::ConcreteTypes::ANOTHER_ATOMIC`). + // 2. a path expr referring to assoc consts whose type is doesn't have any frozen variants in trait + // defs (i.e. without substitute for `Self`). (e.g. borrowing + // `borrow_interior_mutable_const::trait::ConcreteTypes::ATOMIC`) + // 3. similar to the false positive above; but the value is an unfrozen variant, or the type has no + // enums. (An example is + // `declare_interior_mutable_const::enums::BothOfCellAndGeneric::UNFROZEN_VARIANT` and + // `declare_interior_mutable_const::enums::BothOfCellAndGeneric::NO_ENUM`). + // One might be able to prevent these FNs correctly, and replace this with `false`; + // e.g. implementing `has_frozen_variant` described above, and not running this function + // when the type doesn't have any frozen variants would be the 'correct' way for the 2nd + // case (that actually removes another suboptimal behavior (I won't say 'false positive') where, + // similar to 2., but with the a frozen variant) (e.g. borrowing + // `borrow_interior_mutable_const::enums::AssocConsts::TO_BE_FROZEN_VARIANT`). + // I chose this way because unfrozen enums as assoc consts are rare (or, hopefully, none). + matches!(err, ErrorHandled::TooGeneric(..)) + }, + |val| val.map_or(true, |val| self.is_value_unfrozen_raw_inner(cx, val, ty)), + ) + } + + fn is_value_unfrozen_poly<'tcx>(&self, cx: &LateContext<'tcx>, body_id: BodyId, ty: Ty<'tcx>) -> bool { + let def_id = body_id.hir_id.owner.to_def_id(); + let args = ty::GenericArgs::identity_for_item(cx.tcx, def_id); + let instance = ty::Instance::new(def_id, args); + let cid = rustc_middle::mir::interpret::GlobalId { + instance, + promoted: None, + }; + let param_env = cx.tcx.param_env(def_id).with_reveal_all_normalized(cx.tcx); + let result = cx.tcx.const_eval_global_id_for_typeck(param_env, cid, None); + self.is_value_unfrozen_raw(cx, result, ty) + } + + fn is_value_unfrozen_expr<'tcx>(&self, cx: &LateContext<'tcx>, hir_id: HirId, def_id: DefId, ty: Ty<'tcx>) -> bool { + let args = cx.typeck_results().node_args(hir_id); + + let result = Self::const_eval_resolve(cx.tcx, cx.param_env, ty::UnevaluatedConst::new(def_id, args), None); + self.is_value_unfrozen_raw(cx, result, ty) + } + + pub fn const_eval_resolve<'tcx>( + tcx: TyCtxt<'tcx>, + param_env: ty::ParamEnv<'tcx>, + ct: ty::UnevaluatedConst<'tcx>, + span: Option, + ) -> EvalToValTreeResult<'tcx> { + match ty::Instance::resolve(tcx, param_env, ct.def, ct.args) { + Ok(Some(instance)) => { + let cid = GlobalId { + instance, + promoted: None, + }; + tcx.const_eval_global_id_for_typeck(param_env, cid, span) + }, + Ok(None) => Err(ErrorHandled::TooGeneric(span.unwrap_or(rustc_span::DUMMY_SP))), + Err(err) => Err(ErrorHandled::Reported(err.into(), span.unwrap_or(rustc_span::DUMMY_SP))), + } + } +} impl<'tcx> LateLintPass<'tcx> for NonCopyConst { + fn check_crate(&mut self, cx: &LateContext<'tcx>) { + self.ignore_mut_def_ids.clear(); + let mut path = Vec::new(); + for ty in &self.ignore_interior_mutability { + path.extend(ty.split("::")); + for id in def_path_def_ids(cx, &path[..]) { + self.ignore_mut_def_ids.insert(id); + } + path.clear(); + } + } + fn check_item(&mut self, cx: &LateContext<'tcx>, it: &'tcx Item<'_>) { if let ItemKind::Const(.., body_id) = it.kind { let ty = cx.tcx.type_of(it.owner_id).instantiate_identity(); - if !ignored_macro(cx, it) && is_unfrozen(cx, ty) && is_value_unfrozen_poly(cx, body_id, ty) { + if !ignored_macro(cx, it) + && !self.is_ty_ignored(ty) + && Self::is_unfrozen(cx, ty) + && self.is_value_unfrozen_poly(cx, body_id, ty) + { lint(cx, Source::Item { item: it.span }); } } @@ -311,7 +356,7 @@ impl<'tcx> LateLintPass<'tcx> for NonCopyConst { // Normalize assoc types because ones originated from generic params // bounded other traits could have their bound. let normalized = cx.tcx.normalize_erasing_regions(cx.param_env, ty); - if is_unfrozen(cx, normalized) + if !self.is_ty_ignored(ty) && Self::is_unfrozen(cx, normalized) // When there's no default value, lint it only according to its type; // in other words, lint consts whose value *could* be unfrozen, not definitely is. // This feels inconsistent with how the lint treats generic types, @@ -324,7 +369,7 @@ impl<'tcx> LateLintPass<'tcx> for NonCopyConst { // i.e. having an enum doesn't necessary mean a type has a frozen variant. // And, implementing it isn't a trivial task; it'll probably end up // re-implementing the trait predicate evaluation specific to `Freeze`. - && body_id_opt.map_or(true, |body_id| is_value_unfrozen_poly(cx, body_id, normalized)) + && body_id_opt.map_or(true, |body_id| self.is_value_unfrozen_poly(cx, body_id, normalized)) { lint(cx, Source::Assoc { item: trait_item.span }); } @@ -367,8 +412,8 @@ impl<'tcx> LateLintPass<'tcx> for NonCopyConst { // e.g. `layout_of(...).is_err() || has_frozen_variant(...);` let ty = cx.tcx.type_of(impl_item.owner_id).instantiate_identity(); let normalized = cx.tcx.normalize_erasing_regions(cx.param_env, ty); - if is_unfrozen(cx, normalized); - if is_value_unfrozen_poly(cx, *body_id, normalized); + if !self.is_ty_ignored(ty) && Self::is_unfrozen(cx, normalized); + if self.is_value_unfrozen_poly(cx, *body_id, normalized); then { lint( cx, @@ -384,7 +429,10 @@ impl<'tcx> LateLintPass<'tcx> for NonCopyConst { // Normalize assoc types originated from generic params. let normalized = cx.tcx.normalize_erasing_regions(cx.param_env, ty); - if is_unfrozen(cx, ty) && is_value_unfrozen_poly(cx, *body_id, normalized) { + if !self.is_ty_ignored(ty) + && Self::is_unfrozen(cx, ty) + && self.is_value_unfrozen_poly(cx, *body_id, normalized) + { lint(cx, Source::Assoc { item: impl_item.span }); } }, @@ -478,7 +526,10 @@ impl<'tcx> LateLintPass<'tcx> for NonCopyConst { cx.typeck_results().expr_ty(dereferenced_expr) }; - if is_unfrozen(cx, ty) && is_value_unfrozen_expr(cx, expr.hir_id, item_def_id, ty) { + if !self.is_ty_ignored(ty) + && Self::is_unfrozen(cx, ty) + && self.is_value_unfrozen_expr(cx, expr.hir_id, item_def_id, ty) + { lint(cx, Source::Expr { expr: expr.span }); } } diff --git a/clippy_lints/src/non_send_fields_in_send_ty.rs b/clippy_lints/src/non_send_fields_in_send_ty.rs index d388dfc08f3b7..62ef48c8a90cc 100644 --- a/clippy_lints/src/non_send_fields_in_send_ty.rs +++ b/clippy_lints/src/non_send_fields_in_send_ty.rs @@ -1,7 +1,7 @@ use clippy_utils::diagnostics::span_lint_and_then; +use clippy_utils::is_lint_allowed; use clippy_utils::source::snippet; use clippy_utils::ty::{implements_trait, is_copy}; -use clippy_utils::is_lint_allowed; use rustc_ast::ImplPolarity; use rustc_hir::def_id::DefId; use rustc_hir::{FieldDef, Item, ItemKind, Node}; diff --git a/clippy_lints/src/only_used_in_recursion.rs b/clippy_lints/src/only_used_in_recursion.rs index 3dc652f9dc025..f0f8d510c7e24 100644 --- a/clippy_lints/src/only_used_in_recursion.rs +++ b/clippy_lints/src/only_used_in_recursion.rs @@ -134,6 +134,7 @@ impl Usage { /// The parameters being checked by the lint, indexed by both the parameter's `HirId` and the /// `DefId` of the function paired with the parameter's index. #[derive(Default)] +#[allow(clippy::struct_field_names)] struct Params { params: Vec, by_id: HirIdMap, diff --git a/clippy_lints/src/operators/cmp_owned.rs b/clippy_lints/src/operators/cmp_owned.rs index 136642d69dcaa..ea8ed28ba62ba 100644 --- a/clippy_lints/src/operators/cmp_owned.rs +++ b/clippy_lints/src/operators/cmp_owned.rs @@ -1,7 +1,7 @@ use clippy_utils::diagnostics::span_lint_and_then; +use clippy_utils::path_def_id; use clippy_utils::source::snippet; use clippy_utils::ty::{implements_trait, is_copy}; -use clippy_utils::path_def_id; use rustc_errors::Applicability; use rustc_hir::{BinOpKind, Expr, ExprKind, UnOp}; use rustc_lint::LateContext; diff --git a/clippy_lints/src/rc_clone_in_vec_init.rs b/clippy_lints/src/rc_clone_in_vec_init.rs index 1a127c2bcf68e..bd0ca66b7618a 100644 --- a/clippy_lints/src/rc_clone_in_vec_init.rs +++ b/clippy_lints/src/rc_clone_in_vec_init.rs @@ -1,8 +1,8 @@ use clippy_utils::diagnostics::span_lint_and_then; use clippy_utils::higher::VecArgs; +use clippy_utils::last_path_segment; use clippy_utils::macros::root_macro_call_first_node; use clippy_utils::source::{indent_of, snippet}; -use clippy_utils::last_path_segment; use rustc_errors::Applicability; use rustc_hir::{Expr, ExprKind, QPath, TyKind}; use rustc_lint::{LateContext, LateLintPass}; diff --git a/clippy_lints/src/redundant_async_block.rs b/clippy_lints/src/redundant_async_block.rs index b8f787e1f1fbd..2e895d5f23696 100644 --- a/clippy_lints/src/redundant_async_block.rs +++ b/clippy_lints/src/redundant_async_block.rs @@ -5,7 +5,7 @@ use clippy_utils::peel_blocks; use clippy_utils::source::{snippet, walk_span_to_context}; use clippy_utils::visitors::for_each_expr; use rustc_errors::Applicability; -use rustc_hir::{AsyncCoroutineKind, Closure, Expr, ExprKind, CoroutineKind, MatchSource}; +use rustc_hir::{AsyncCoroutineKind, Closure, CoroutineKind, Expr, ExprKind, MatchSource}; use rustc_lint::{LateContext, LateLintPass}; use rustc_middle::lint::in_external_macro; use rustc_middle::ty::UpvarCapture; diff --git a/clippy_lints/src/redundant_locals.rs b/clippy_lints/src/redundant_locals.rs index 197742b5dd41d..a1a0e8f35208f 100644 --- a/clippy_lints/src/redundant_locals.rs +++ b/clippy_lints/src/redundant_locals.rs @@ -62,7 +62,7 @@ impl<'tcx> LateLintPass<'tcx> for RedundantLocals { if let Res::Local(binding_id) = cx.qpath_res(&qpath, expr.hir_id); if let Node::Pat(binding_pat) = cx.tcx.hir().get(binding_id); // the previous binding has the same mutability - if find_binding(binding_pat, ident).unwrap().1 == mutability; + if find_binding(binding_pat, ident).is_some_and(|bind| bind.1 == mutability); // the local does not change the effect of assignments to the binding. see #11290 if !affects_assignments(cx, mutability, binding_id, local.hir_id); // the local does not affect the code's drop behavior diff --git a/clippy_lints/src/slow_vector_initialization.rs b/clippy_lints/src/slow_vector_initialization.rs index 9db18c2976c22..2278e41be3758 100644 --- a/clippy_lints/src/slow_vector_initialization.rs +++ b/clippy_lints/src/slow_vector_initialization.rs @@ -335,7 +335,7 @@ impl<'a, 'tcx> Visitor<'tcx> for VectorInitializationVisitor<'a, 'tcx> { fn visit_block(&mut self, block: &'tcx Block<'_>) { if self.initialization_found { - if let Some(s) = block.stmts.get(0) { + if let Some(s) = block.stmts.first() { self.visit_stmt(s); } diff --git a/clippy_lints/src/swap_ptr_to_ref.rs b/clippy_lints/src/swap_ptr_to_ref.rs index 3685432a25399..4bfbe3bf37e7d 100644 --- a/clippy_lints/src/swap_ptr_to_ref.rs +++ b/clippy_lints/src/swap_ptr_to_ref.rs @@ -1,6 +1,6 @@ use clippy_utils::diagnostics::span_lint_and_then; -use clippy_utils::source::snippet_with_context; use clippy_utils::path_def_id; +use clippy_utils::source::snippet_with_context; use rustc_errors::Applicability; use rustc_hir::{BorrowKind, Expr, ExprKind, Mutability, UnOp}; use rustc_lint::{LateContext, LateLintPass}; diff --git a/clippy_lints/src/types/mod.rs b/clippy_lints/src/types/mod.rs index 71a4b3fba1b59..788678a63b76d 100644 --- a/clippy_lints/src/types/mod.rs +++ b/clippy_lints/src/types/mod.rs @@ -578,7 +578,7 @@ impl Types { } } -#[allow(clippy::struct_excessive_bools)] +#[allow(clippy::struct_excessive_bools, clippy::struct_field_names)] #[derive(Clone, Copy, Default)] struct CheckTyContext { is_in_trait_impl: bool, diff --git a/clippy_lints/src/uninit_vec.rs b/clippy_lints/src/uninit_vec.rs index 6756df8e716ca..72569e10f0581 100644 --- a/clippy_lints/src/uninit_vec.rs +++ b/clippy_lints/src/uninit_vec.rs @@ -201,7 +201,7 @@ fn extract_set_len_self<'tcx>(cx: &LateContext<'_>, expr: &'tcx Expr<'_>) -> Opt let expr = peel_hir_expr_while(expr, |e| { if let ExprKind::Block(block, _) = e.kind { // Extract the first statement/expression - match (block.stmts.get(0).map(|stmt| &stmt.kind), block.expr) { + match (block.stmts.first().map(|stmt| &stmt.kind), block.expr) { (None, Some(expr)) => Some(expr), (Some(StmtKind::Expr(expr) | StmtKind::Semi(expr)), _) => Some(expr), _ => None, diff --git a/clippy_lints/src/unnecessary_map_on_constructor.rs b/clippy_lints/src/unnecessary_map_on_constructor.rs index 5aa057580e9d9..894de0d85c1ef 100644 --- a/clippy_lints/src/unnecessary_map_on_constructor.rs +++ b/clippy_lints/src/unnecessary_map_on_constructor.rs @@ -40,7 +40,7 @@ impl<'tcx> LateLintPass<'tcx> for UnnecessaryMapOnConstructor { let (constructor_path, constructor_item) = if let hir::ExprKind::Call(constructor, constructor_args) = recv.kind && let hir::ExprKind::Path(constructor_path) = constructor.kind - && let Some(arg) = constructor_args.get(0) + && let Some(arg) = constructor_args.first() { if constructor.span.from_expansion() || arg.span.from_expansion() { return; @@ -66,7 +66,7 @@ impl<'tcx> LateLintPass<'tcx> for UnnecessaryMapOnConstructor { _ => return, } - if let Some(map_arg) = args.get(0) + if let Some(map_arg) = args.first() && let hir::ExprKind::Path(fun) = map_arg.kind { if map_arg.span.from_expansion() { diff --git a/clippy_lints/src/use_self.rs b/clippy_lints/src/use_self.rs index 50231d930d0e1..f10ed4b3d4193 100644 --- a/clippy_lints/src/use_self.rs +++ b/clippy_lints/src/use_self.rs @@ -54,7 +54,6 @@ declare_clippy_lint! { "unnecessary structure name repetition whereas `Self` is applicable" } -#[derive(Default)] pub struct UseSelf { msrv: Msrv, stack: Vec, @@ -65,7 +64,7 @@ impl UseSelf { pub fn new(msrv: Msrv) -> Self { Self { msrv, - ..Self::default() + stack: Vec::new(), } } } diff --git a/clippy_lints/src/utils/author.rs b/clippy_lints/src/utils/author.rs index f02c33cc6743e..aecb0c6dbfa10 100644 --- a/clippy_lints/src/utils/author.rs +++ b/clippy_lints/src/utils/author.rs @@ -268,8 +268,8 @@ impl<'a, 'tcx> PrintVisitor<'a, 'tcx> { fn qpath(&self, qpath: &Binding<&QPath<'_>>) { if let QPath::LangItem(lang_item, ..) = *qpath.value { chain!(self, "matches!({qpath}, QPath::LangItem(LangItem::{lang_item:?}, _))"); - } else { - chain!(self, "match_qpath({qpath}, &[{}])", path_to_string(qpath.value)); + } else if let Ok(path) = path_to_string(qpath.value) { + chain!(self, "match_qpath({qpath}, &[{}])", path); } } @@ -738,8 +738,8 @@ fn has_attr(cx: &LateContext<'_>, hir_id: hir::HirId) -> bool { get_attr(cx.sess(), attrs, "author").count() > 0 } -fn path_to_string(path: &QPath<'_>) -> String { - fn inner(s: &mut String, path: &QPath<'_>) { +fn path_to_string(path: &QPath<'_>) -> Result { + fn inner(s: &mut String, path: &QPath<'_>) -> Result<(), ()> { match *path { QPath::Resolved(_, path) => { for (i, segment) in path.segments.iter().enumerate() { @@ -751,16 +751,18 @@ fn path_to_string(path: &QPath<'_>) -> String { }, QPath::TypeRelative(ty, segment) => match &ty.kind { hir::TyKind::Path(inner_path) => { - inner(s, inner_path); + inner(s, inner_path)?; *s += ", "; write!(s, "{:?}", segment.ident.as_str()).unwrap(); }, other => write!(s, "/* unimplemented: {other:?}*/").unwrap(), }, - QPath::LangItem(..) => panic!("path_to_string: called for lang item qpath"), + QPath::LangItem(..) => return Err(()), } + + Ok(()) } let mut s = String::new(); - inner(&mut s, path); - s + inner(&mut s, path)?; + Ok(s) } diff --git a/clippy_lints/src/utils/conf.rs b/clippy_lints/src/utils/conf.rs index 23da1de77303e..8829f188fe797 100644 --- a/clippy_lints/src/utils/conf.rs +++ b/clippy_lints/src/utils/conf.rs @@ -8,8 +8,9 @@ use serde::de::{Deserializer, IgnoredAny, IntoDeserializer, MapAccess, Visitor}; use serde::Deserialize; use std::fmt::{Debug, Display, Formatter}; use std::ops::Range; -use std::path::{Path, PathBuf}; +use std::path::PathBuf; use std::str::FromStr; +use std::sync::OnceLock; use std::{cmp, env, fmt, fs, io}; #[rustfmt::skip] @@ -78,62 +79,35 @@ pub struct TryConf { impl TryConf { fn from_toml_error(file: &SourceFile, error: &toml::de::Error) -> Self { - ConfError::from_toml(file, error).into() - } -} - -impl From for TryConf { - fn from(value: ConfError) -> Self { Self { conf: Conf::default(), - errors: vec![value], + errors: vec![ConfError::from_toml(file, error)], warnings: vec![], } } } -impl From for TryConf { - fn from(value: io::Error) -> Self { - ConfError::from(value).into() - } -} - #[derive(Debug)] pub struct ConfError { pub message: String, - pub span: Option, + pub span: Span, } impl ConfError { fn from_toml(file: &SourceFile, error: &toml::de::Error) -> Self { - if let Some(span) = error.span() { - Self::spanned(file, error.message(), span) - } else { - Self { - message: error.message().to_string(), - span: None, - } - } + let span = error.span().unwrap_or(0..file.source_len.0 as usize); + Self::spanned(file, error.message(), span) } fn spanned(file: &SourceFile, message: impl Into, span: Range) -> Self { Self { message: message.into(), - span: Some(Span::new( + span: Span::new( file.start_pos + BytePos::from_usize(span.start), file.start_pos + BytePos::from_usize(span.end), SyntaxContext::root(), None, - )), - } - } -} - -impl From for ConfError { - fn from(value: io::Error) -> Self { - Self { - message: value.to_string(), - span: None, + ), } } } @@ -297,7 +271,7 @@ define_Conf! { /// Lint: MANUAL_SPLIT_ONCE, MANUAL_STR_REPEAT, CLONED_INSTEAD_OF_COPIED, REDUNDANT_FIELD_NAMES, OPTION_MAP_UNWRAP_OR, REDUNDANT_STATIC_LIFETIMES, FILTER_MAP_NEXT, CHECKED_CONVERSIONS, MANUAL_RANGE_CONTAINS, USE_SELF, MEM_REPLACE_WITH_DEFAULT, MANUAL_NON_EXHAUSTIVE, OPTION_AS_REF_DEREF, MAP_UNWRAP_OR, MATCH_LIKE_MATCHES_MACRO, MANUAL_STRIP, MISSING_CONST_FOR_FN, UNNESTED_OR_PATTERNS, FROM_OVER_INTO, PTR_AS_PTR, IF_THEN_SOME_ELSE_NONE, APPROX_CONSTANT, DEPRECATED_CFG_ATTR, INDEX_REFUTABLE_SLICE, MAP_CLONE, BORROW_AS_PTR, MANUAL_BITS, ERR_EXPECT, CAST_ABS_TO_UNSIGNED, UNINLINED_FORMAT_ARGS, MANUAL_CLAMP, MANUAL_LET_ELSE, UNCHECKED_DURATION_SUBTRACTION, COLLAPSIBLE_STR_REPLACE, SEEK_FROM_CURRENT, SEEK_REWIND, UNNECESSARY_LAZY_EVALUATIONS, TRANSMUTE_PTR_TO_REF, ALMOST_COMPLETE_RANGE, NEEDLESS_BORROW, DERIVABLE_IMPLS, MANUAL_IS_ASCII_CHECK, MANUAL_REM_EUCLID, MANUAL_RETAIN, TYPE_REPETITION_IN_BOUNDS, TUPLE_ARRAY_CONVERSIONS, MANUAL_TRY_FOLD, MANUAL_HASH_ONE. /// /// The minimum rust version that the project supports - (msrv: Option = None), + (msrv: crate::Msrv = crate::Msrv::empty()), /// DEPRECATED LINT: BLACKLISTED_NAME. /// /// Use the Disallowed Names lint instead @@ -360,6 +334,10 @@ define_Conf! { /// /// The minimum number of enum variants for the lints about variant names to trigger (enum_variant_name_threshold: u64 = 3), + /// Lint: STRUCT_VARIANT_NAMES. + /// + /// The minimum number of struct fields for the lints about field names to trigger + (struct_field_name_threshold: u64 = 3), /// Lint: LARGE_ENUM_VARIANT. /// /// The maximum size of an enum's variant to avoid box suggestion @@ -641,15 +619,8 @@ pub fn lookup_conf_file() -> io::Result<(Option, Vec)> { } } -/// Read the `toml` configuration file. -/// -/// In case of error, the function tries to continue as much as possible. -pub fn read(sess: &Session, path: &Path) -> TryConf { - let file = match sess.source_map().load_file(path) { - Err(e) => return e.into(), - Ok(file) => file, - }; - match toml::de::Deserializer::new(file.src.as_ref().unwrap()).deserialize_map(ConfVisitor(&file)) { +fn deserialize(file: &SourceFile) -> TryConf { + match toml::de::Deserializer::new(file.src.as_ref().unwrap()).deserialize_map(ConfVisitor(file)) { Ok(mut conf) => { extend_vec_if_indicator_present(&mut conf.conf.doc_valid_idents, DEFAULT_DOC_VALID_IDENTS); extend_vec_if_indicator_present(&mut conf.conf.disallowed_names, DEFAULT_DISALLOWED_NAMES); @@ -662,7 +633,7 @@ pub fn read(sess: &Session, path: &Path) -> TryConf { conf }, - Err(e) => TryConf::from_toml_error(&file, &e), + Err(e) => TryConf::from_toml_error(file, &e), } } @@ -672,6 +643,60 @@ fn extend_vec_if_indicator_present(vec: &mut Vec, default: &[&str]) { } } +impl Conf { + pub fn read(sess: &Session, path: &io::Result<(Option, Vec)>) -> &'static Conf { + static CONF: OnceLock = OnceLock::new(); + CONF.get_or_init(|| Conf::read_inner(sess, path)) + } + + fn read_inner(sess: &Session, path: &io::Result<(Option, Vec)>) -> Conf { + match path { + Ok((_, warnings)) => { + for warning in warnings { + sess.warn(warning.clone()); + } + }, + Err(error) => { + sess.err(format!("error finding Clippy's configuration file: {error}")); + }, + } + + let TryConf { + mut conf, + errors, + warnings, + } = match path { + Ok((Some(path), _)) => match sess.source_map().load_file(path) { + Ok(file) => deserialize(&file), + Err(error) => { + sess.err(format!("failed to read `{}`: {error}", path.display())); + TryConf::default() + }, + }, + _ => TryConf::default(), + }; + + conf.msrv.read_cargo(sess); + + // all conf errors are non-fatal, we just use the default conf in case of error + for error in errors { + sess.span_err( + error.span, + format!("error reading Clippy's configuration file: {}", error.message), + ); + } + + for warning in warnings { + sess.span_warn( + warning.span, + format!("error reading Clippy's configuration file: {}", warning.message), + ); + } + + conf + } +} + const SEPARATOR_WIDTH: usize = 4; #[derive(Debug)] diff --git a/clippy_lints/src/utils/internal_lints/if_chain_style.rs b/clippy_lints/src/utils/internal_lints/if_chain_style.rs index fe2f12fe833b5..8cdd5ea890371 100644 --- a/clippy_lints/src/utils/internal_lints/if_chain_style.rs +++ b/clippy_lints/src/utils/internal_lints/if_chain_style.rs @@ -30,7 +30,7 @@ impl<'tcx> LateLintPass<'tcx> for IfChainStyle { if_chain_local_span(cx, local, if_chain_span), "`let` expression should be above the `if_chain!`", ); - } else if local.span.ctxt() == block.span.ctxt() && is_if_chain_then(after, block.expr, if_chain_span) { + } else if local.span.eq_ctxt(block.span) && is_if_chain_then(after, block.expr, if_chain_span) { span_lint( cx, IF_CHAIN_STYLE, diff --git a/clippy_lints/src/utils/internal_lints/interning_defined_symbol.rs b/clippy_lints/src/utils/internal_lints/interning_defined_symbol.rs index 82f9d4e41e882..fc9afe5ca8b96 100644 --- a/clippy_lints/src/utils/internal_lints/interning_defined_symbol.rs +++ b/clippy_lints/src/utils/internal_lints/interning_defined_symbol.rs @@ -13,6 +13,7 @@ use rustc_lint::{LateContext, LateLintPass}; use rustc_middle::mir::ConstValue; use rustc_middle::ty; use rustc_session::{declare_tool_lint, impl_lint_pass}; +use rustc_span::sym; use rustc_span::symbol::Symbol; use std::borrow::Cow; @@ -160,12 +161,8 @@ impl<'tcx> LateLintPass<'tcx> for InterningDefinedSymbol { impl InterningDefinedSymbol { fn symbol_str_expr<'tcx>(&self, expr: &'tcx Expr<'tcx>, cx: &LateContext<'tcx>) -> Option> { - static IDENT_STR_PATHS: &[&[&str]] = &[&paths::IDENT_AS_STR, &paths::TO_STRING_METHOD]; - static SYMBOL_STR_PATHS: &[&[&str]] = &[ - &paths::SYMBOL_AS_STR, - &paths::SYMBOL_TO_IDENT_STRING, - &paths::TO_STRING_METHOD, - ]; + static IDENT_STR_PATHS: &[&[&str]] = &[&paths::IDENT_AS_STR]; + static SYMBOL_STR_PATHS: &[&[&str]] = &[&paths::SYMBOL_AS_STR, &paths::SYMBOL_TO_IDENT_STRING]; let call = if_chain! { if let ExprKind::AddrOf(_, _, e) = expr.kind; if let ExprKind::Unary(UnOp::Deref, e) = e.kind; @@ -186,9 +183,19 @@ impl InterningDefinedSymbol { }; // ...which converts it to a string let paths = if is_ident { IDENT_STR_PATHS } else { SYMBOL_STR_PATHS }; - if let Some(path) = paths.iter().find(|path| match_def_path(cx, did, path)); + if let Some(is_to_owned) = paths + .iter() + .find_map(|path| if match_def_path(cx, did, path) { + Some(path == &paths::SYMBOL_TO_IDENT_STRING) + } else { + None + }) + .or_else(|| if cx.tcx.is_diagnostic_item(sym::to_string_method, did) { + Some(true) + } else { + None + }); then { - let is_to_owned = path.last().unwrap().ends_with("string"); return Some(SymbolStrExpr::Expr { item, is_ident, diff --git a/clippy_utils/Cargo.toml b/clippy_utils/Cargo.toml index 8522493f67b3a..90091ca927aea 100644 --- a/clippy_utils/Cargo.toml +++ b/clippy_utils/Cargo.toml @@ -9,6 +9,7 @@ arrayvec = { version = "0.7", default-features = false } if_chain = "1.0" itertools = "0.10.1" rustc-semver = "1.1" +serde = { version = "1.0" } [features] deny-warnings = [] diff --git a/clippy_utils/src/consts.rs b/clippy_utils/src/consts.rs index 0bae7056c4fb8..79c04c7c7f4a3 100644 --- a/clippy_utils/src/consts.rs +++ b/clippy_utils/src/consts.rs @@ -504,7 +504,7 @@ impl<'a, 'tcx> ConstEvalLateContext<'a, 'tcx> { }, (Some(Constant::Vec(vec)), _) => { if !vec.is_empty() && vec.iter().all(|x| *x == vec[0]) { - match vec.get(0) { + match vec.first() { Some(Constant::F32(x)) => Some(Constant::F32(*x)), Some(Constant::F64(x)) => Some(Constant::F64(*x)), _ => None, diff --git a/clippy_utils/src/higher.rs b/clippy_utils/src/higher.rs index 741f9f54883d6..edea4b3667fed 100644 --- a/clippy_utils/src/higher.rs +++ b/clippy_utils/src/higher.rs @@ -449,7 +449,7 @@ pub fn get_vec_init_kind<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) - } else if name.ident.name == symbol::kw::Default { return Some(VecInitKind::Default); } else if name.ident.name.as_str() == "with_capacity" { - let arg = args.get(0)?; + let arg = args.first()?; return match constant_simple(cx, cx.typeck_results(), arg) { Some(Constant::Int(num)) => Some(VecInitKind::WithConstCapacity(num)), _ => Some(VecInitKind::WithExprCapacity(arg.hir_id)), diff --git a/clippy_utils/src/lib.rs b/clippy_utils/src/lib.rs index c2c97259d38bf..93b3702282288 100644 --- a/clippy_utils/src/lib.rs +++ b/clippy_utils/src/lib.rs @@ -2027,48 +2027,88 @@ pub fn is_must_use_func_call(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool { did.map_or(false, |did| cx.tcx.has_attr(did, sym::must_use)) } -/// Checks if an expression represents the identity function -/// Only examines closures and `std::convert::identity` -pub fn is_expr_identity_function(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool { - /// Checks if a function's body represents the identity function. Looks for bodies of the form: - /// * `|x| x` - /// * `|x| return x` - /// * `|x| { return x }` - /// * `|x| { return x; }` - fn is_body_identity_function(cx: &LateContext<'_>, func: &Body<'_>) -> bool { - let id = if_chain! { - if let [param] = func.params; - if let PatKind::Binding(_, id, _, _) = param.pat.kind; - then { - id - } else { - return false; - } - }; +/// Checks if a function's body represents the identity function. Looks for bodies of the form: +/// * `|x| x` +/// * `|x| return x` +/// * `|x| { return x }` +/// * `|x| { return x; }` +/// +/// Consider calling [`is_expr_untyped_identity_function`] or [`is_expr_identity_function`] instead. +fn is_body_identity_function(cx: &LateContext<'_>, func: &Body<'_>) -> bool { + let id = if_chain! { + if let [param] = func.params; + if let PatKind::Binding(_, id, _, _) = param.pat.kind; + then { + id + } else { + return false; + } + }; - let mut expr = func.value; - loop { - match expr.kind { - #[rustfmt::skip] - ExprKind::Block(&Block { stmts: [], expr: Some(e), .. }, _, ) - | ExprKind::Ret(Some(e)) => expr = e, - #[rustfmt::skip] - ExprKind::Block(&Block { stmts: [stmt], expr: None, .. }, _) => { - if_chain! { - if let StmtKind::Semi(e) | StmtKind::Expr(e) = stmt.kind; - if let ExprKind::Ret(Some(ret_val)) = e.kind; - then { - expr = ret_val; - } else { - return false; - } - } + let mut expr = func.value; + loop { + match expr.kind { + ExprKind::Block( + &Block { + stmts: [], + expr: Some(e), + .. }, - _ => return path_to_local_id(expr, id) && cx.typeck_results().expr_adjustments(expr).is_empty(), - } + _, + ) + | ExprKind::Ret(Some(e)) => expr = e, + ExprKind::Block( + &Block { + stmts: [stmt], + expr: None, + .. + }, + _, + ) => { + if_chain! { + if let StmtKind::Semi(e) | StmtKind::Expr(e) = stmt.kind; + if let ExprKind::Ret(Some(ret_val)) = e.kind; + then { + expr = ret_val; + } else { + return false; + } + } + }, + _ => return path_to_local_id(expr, id) && cx.typeck_results().expr_adjustments(expr).is_empty(), } } +} +/// This is the same as [`is_expr_identity_function`], but does not consider closures +/// with type annotations for its bindings (or similar) as identity functions: +/// * `|x: u8| x` +/// * `std::convert::identity::` +pub fn is_expr_untyped_identity_function(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool { + match expr.kind { + ExprKind::Closure(&Closure { body, fn_decl, .. }) + if fn_decl.inputs.iter().all(|ty| matches!(ty.kind, TyKind::Infer)) => + { + is_body_identity_function(cx, cx.tcx.hir().body(body)) + }, + ExprKind::Path(QPath::Resolved(_, path)) + if path.segments.iter().all(|seg| seg.infer_args) + && let Some(did) = path.res.opt_def_id() => { + cx.tcx.is_diagnostic_item(sym::convert_identity, did) + }, + _ => false, + } +} + +/// Checks if an expression represents the identity function +/// Only examines closures and `std::convert::identity` +/// +/// NOTE: If you want to use this function to find out if a closure is unnecessary, you likely want +/// to call [`is_expr_untyped_identity_function`] instead, which makes sure that the closure doesn't +/// have type annotations. This is important because removing a closure with bindings can +/// remove type information that helped type inference before, which can then lead to compile +/// errors. +pub fn is_expr_identity_function(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool { match expr.kind { ExprKind::Closure(&Closure { body, .. }) => is_body_identity_function(cx, cx.tcx.hir().body(body)), _ => path_def_id(cx, expr).map_or(false, |id| cx.tcx.is_diagnostic_item(sym::convert_identity, id)), diff --git a/clippy_utils/src/msrvs.rs b/clippy_utils/src/msrvs.rs index df839c2106f14..c6a48874e09bc 100644 --- a/clippy_utils/src/msrvs.rs +++ b/clippy_utils/src/msrvs.rs @@ -1,9 +1,7 @@ -use std::sync::OnceLock; - use rustc_ast::Attribute; use rustc_semver::RustcVersion; use rustc_session::Session; -use rustc_span::Span; +use serde::Deserialize; use crate::attrs::get_unique_attr; @@ -53,65 +51,45 @@ msrv_aliases! { 1,15,0 { MAYBE_BOUND_IN_WHERE } } -fn parse_msrv(msrv: &str, sess: Option<&Session>, span: Option) -> Option { - if let Ok(version) = RustcVersion::parse(msrv) { - return Some(version); - } else if let Some(sess) = sess { - if let Some(span) = span { - sess.span_err(span, format!("`{msrv}` is not a valid Rust version")); - } - } - None -} - /// Tracks the current MSRV from `clippy.toml`, `Cargo.toml` or set via `#[clippy::msrv]` -#[derive(Debug, Clone, Default)] +#[derive(Debug, Clone)] pub struct Msrv { stack: Vec, } +impl<'de> Deserialize<'de> for Msrv { + fn deserialize(deserializer: D) -> Result + where + D: serde::Deserializer<'de>, + { + let v = String::deserialize(deserializer)?; + RustcVersion::parse(&v) + .map(|v| Msrv { stack: vec![v] }) + .map_err(|_| serde::de::Error::custom("not a valid Rust version")) + } +} + impl Msrv { - fn new(initial: Option) -> Self { - Self { - stack: Vec::from_iter(initial), - } + pub fn empty() -> Msrv { + Msrv { stack: Vec::new() } } - fn read_inner(conf_msrv: &Option, sess: &Session) -> Self { + pub fn read_cargo(&mut self, sess: &Session) { let cargo_msrv = std::env::var("CARGO_PKG_RUST_VERSION") .ok() - .and_then(|v| parse_msrv(&v, None, None)); - let clippy_msrv = conf_msrv.as_ref().and_then(|s| { - parse_msrv(s, None, None).or_else(|| { - sess.err(format!( - "error reading Clippy's configuration file. `{s}` is not a valid Rust version" - )); - None - }) - }); - - // if both files have an msrv, let's compare them and emit a warning if they differ - if let Some(cargo_msrv) = cargo_msrv - && let Some(clippy_msrv) = clippy_msrv - && clippy_msrv != cargo_msrv - { - sess.warn(format!( - "the MSRV in `clippy.toml` and `Cargo.toml` differ; using `{clippy_msrv}` from `clippy.toml`" - )); + .and_then(|v| RustcVersion::parse(&v).ok()); + + match (self.current(), cargo_msrv) { + (None, Some(cargo_msrv)) => self.stack = vec![cargo_msrv], + (Some(clippy_msrv), Some(cargo_msrv)) => { + if clippy_msrv != cargo_msrv { + sess.warn(format!( + "the MSRV in `clippy.toml` and `Cargo.toml` differ; using `{clippy_msrv}` from `clippy.toml`" + )); + } + }, + _ => {}, } - - Self::new(clippy_msrv.or(cargo_msrv)) - } - - /// Set the initial MSRV from the Clippy config file or from Cargo due to the `rust-version` - /// field in `Cargo.toml` - /// - /// Returns a `&'static Msrv` as `Copy` types are more easily passed to the - /// `register_{late,early}_pass` callbacks - pub fn read(conf_msrv: &Option, sess: &Session) -> &'static Self { - static PARSED: OnceLock = OnceLock::new(); - - PARSED.get_or_init(|| Self::read_inner(conf_msrv, sess)) } pub fn current(&self) -> Option { @@ -125,10 +103,14 @@ impl Msrv { fn parse_attr(sess: &Session, attrs: &[Attribute]) -> Option { if let Some(msrv_attr) = get_unique_attr(sess, attrs, "msrv") { if let Some(msrv) = msrv_attr.value_str() { - return parse_msrv(&msrv.to_string(), Some(sess), Some(msrv_attr.span)); - } + if let Ok(version) = RustcVersion::parse(msrv.as_str()) { + return Some(version); + } - sess.span_err(msrv_attr.span, "bad clippy attribute"); + sess.span_err(msrv_attr.span, format!("`{msrv}` is not a valid Rust version")); + } else { + sess.span_err(msrv_attr.span, "bad clippy attribute"); + } } None diff --git a/clippy_utils/src/paths.rs b/clippy_utils/src/paths.rs index 1f2bb16f459fc..4a20399e3649c 100644 --- a/clippy_utils/src/paths.rs +++ b/clippy_utils/src/paths.rs @@ -98,7 +98,6 @@ pub const SYMBOL_TO_IDENT_STRING: [&str; 4] = ["rustc_span", "symbol", "Symbol", pub const SYM_MODULE: [&str; 3] = ["rustc_span", "symbol", "sym"]; #[cfg(feature = "internal")] pub const SYNTAX_CONTEXT: [&str; 3] = ["rustc_span", "hygiene", "SyntaxContext"]; -pub const TO_STRING_METHOD: [&str; 4] = ["alloc", "string", "ToString", "to_string"]; #[expect(clippy::invalid_paths)] // internal lints do not know about all external crates pub const TOKIO_IO_ASYNCREADEXT: [&str; 5] = ["tokio", "io", "util", "async_read_ext", "AsyncReadExt"]; #[expect(clippy::invalid_paths)] // internal lints do not know about all external crates diff --git a/clippy_utils/src/str_utils.rs b/clippy_utils/src/str_utils.rs index 03a9d3c25fd98..69c25b427e1c8 100644 --- a/clippy_utils/src/str_utils.rs +++ b/clippy_utils/src/str_utils.rs @@ -236,6 +236,59 @@ pub fn count_match_end(str1: &str, str2: &str) -> StrCount { }) } +/// Returns a `snake_case` version of the input +/// ``` +/// use clippy_utils::str_utils::to_snake_case; +/// assert_eq!(to_snake_case("AbcDef"), "abc_def"); +/// assert_eq!(to_snake_case("ABCD"), "a_b_c_d"); +/// assert_eq!(to_snake_case("AbcDD"), "abc_d_d"); +/// assert_eq!(to_snake_case("Abc1DD"), "abc1_d_d"); +/// ``` +pub fn to_snake_case(name: &str) -> String { + let mut s = String::new(); + for (i, c) in name.chars().enumerate() { + if c.is_uppercase() { + // characters without capitalization are considered lowercase + if i != 0 { + s.push('_'); + } + s.extend(c.to_lowercase()); + } else { + s.push(c); + } + } + s +} +/// Returns a `CamelCase` version of the input +/// ``` +/// use clippy_utils::str_utils::to_camel_case; +/// assert_eq!(to_camel_case("abc_def"), "AbcDef"); +/// assert_eq!(to_camel_case("a_b_c_d"), "ABCD"); +/// assert_eq!(to_camel_case("abc_d_d"), "AbcDD"); +/// assert_eq!(to_camel_case("abc1_d_d"), "Abc1DD"); +/// ``` +pub fn to_camel_case(item_name: &str) -> String { + let mut s = String::new(); + let mut up = true; + for c in item_name.chars() { + if c.is_uppercase() { + // we only turn snake case text into CamelCase + return item_name.to_string(); + } + if c == '_' { + up = true; + continue; + } + if up { + up = false; + s.extend(c.to_uppercase()); + } else { + s.push(c); + } + } + s +} + #[cfg(test)] mod test { use super::*; diff --git a/rust-toolchain b/rust-toolchain index fe2c77ab47f00..7c5b5e97a5c71 100644 --- a/rust-toolchain +++ b/rust-toolchain @@ -1,3 +1,3 @@ [toolchain] -channel = "nightly-2023-10-06" +channel = "nightly-2023-10-21" components = ["cargo", "llvm-tools", "rust-src", "rust-std", "rustc", "rustc-dev", "rustfmt"] diff --git a/src/driver.rs b/src/driver.rs index d47767faed9ed..0e81419f1fa3d 100644 --- a/src/driver.rs +++ b/src/driver.rs @@ -147,9 +147,9 @@ impl rustc_driver::Callbacks for ClippyCallbacks { (previous)(sess, lint_store); } - let conf = clippy_lints::read_conf(sess, &conf_path); - clippy_lints::register_plugins(lint_store, sess, &conf); - clippy_lints::register_pre_expansion_lints(lint_store, sess, &conf); + let conf = clippy_lints::Conf::read(sess, &conf_path); + clippy_lints::register_plugins(lint_store, sess, conf); + clippy_lints::register_pre_expansion_lints(lint_store, conf); clippy_lints::register_renamed(lint_store); })); diff --git a/tests/compile-test.rs b/tests/compile-test.rs index f340cf5938a7d..1494c7d317962 100644 --- a/tests/compile-test.rs +++ b/tests/compile-test.rs @@ -105,27 +105,20 @@ static EXTERN_FLAGS: LazyLock> = LazyLock::new(|| { // whether to run internal tests or not const RUN_INTERNAL_TESTS: bool = cfg!(feature = "internal"); -fn canonicalize(path: impl AsRef) -> PathBuf { - let path = path.as_ref(); - fs::create_dir_all(path).unwrap(); - fs::canonicalize(path).unwrap_or_else(|err| panic!("{} cannot be canonicalized: {err}", path.display())) -} - fn base_config(test_dir: &str) -> (Config, Args) { let mut args = Args::test().unwrap(); args.bless |= var_os("RUSTC_BLESS").is_some_and(|v| v != "0"); + let target_dir = PathBuf::from(var_os("CARGO_TARGET_DIR").unwrap_or_else(|| "target".into())); let mut config = Config { mode: Mode::Yolo { rustfix: ui_test::RustfixMode::Everything, }, - stderr_filters: vec![(Match::PathBackslash, b"/")], - stdout_filters: vec![], filter_files: env::var("TESTNAME") .map(|filters| filters.split(',').map(str::to_string).collect()) .unwrap_or_default(), target: None, - out_dir: canonicalize(var_os("CARGO_TARGET_DIR").unwrap_or_else(|| "target".into())).join("ui_test"), + out_dir: target_dir.join("ui_test"), ..Config::rustc(Path::new("tests").join(test_dir)) }; config.with_args(&args, /* bless by default */ false); @@ -168,19 +161,13 @@ fn run_ui() { config .program .envs - .push(("CLIPPY_CONF_DIR".into(), Some(canonicalize("tests").into()))); - - let quiet = args.quiet; + .push(("CLIPPY_CONF_DIR".into(), Some("tests".into()))); ui_test::run_tests_generic( vec![config], ui_test::default_file_filter, ui_test::default_per_file_config, - if quiet { - status_emitter::Text::quiet() - } else { - status_emitter::Text::verbose() - }, + status_emitter::Text::from(args.format), ) .unwrap(); } @@ -194,17 +181,12 @@ fn run_internal_tests() { if let OutputConflictHandling::Error(err) = &mut config.output_conflict_handling { *err = "cargo uitest --features internal -- -- --bless".into(); } - let quiet = args.quiet; ui_test::run_tests_generic( vec![config], ui_test::default_file_filter, ui_test::default_per_file_config, - if quiet { - status_emitter::Text::quiet() - } else { - status_emitter::Text::verbose() - }, + status_emitter::Text::from(args.format), ) .unwrap(); } @@ -212,22 +194,9 @@ fn run_internal_tests() { fn run_ui_toml() { let (mut config, args) = base_config("ui-toml"); - config.stderr_filters = vec![ - ( - Match::Exact( - canonicalize("tests") - .parent() - .unwrap() - .to_string_lossy() - .as_bytes() - .to_vec(), - ), - b"$DIR", - ), - (Match::Exact(b"\\".to_vec()), b"/"), - ]; - - let quiet = args.quiet; + config + .stderr_filters + .push((Match::from(env::current_dir().unwrap().as_path()), b"$DIR")); ui_test::run_tests_generic( vec![config], @@ -238,11 +207,7 @@ fn run_ui_toml() { .envs .push(("CLIPPY_CONF_DIR".into(), Some(path.parent().unwrap().into()))); }, - if quiet { - status_emitter::Text::quiet() - } else { - status_emitter::Text::verbose() - }, + status_emitter::Text::from(args.format), ) .unwrap(); } @@ -270,22 +235,9 @@ fn run_ui_cargo() { }); config.edition = None; - config.stderr_filters = vec![ - ( - Match::Exact( - canonicalize("tests") - .parent() - .unwrap() - .to_string_lossy() - .as_bytes() - .to_vec(), - ), - b"$DIR", - ), - (Match::Exact(b"\\".to_vec()), b"/"), - ]; - - let quiet = args.quiet; + config + .stderr_filters + .push((Match::from(env::current_dir().unwrap().as_path()), b"$DIR")); let ignored_32bit = |path: &Path| { // FIXME: for some reason the modules are linted in a different order for this test @@ -297,20 +249,8 @@ fn run_ui_cargo() { |path, config| { path.ends_with("Cargo.toml") && ui_test::default_any_file_filter(path, config) && !ignored_32bit(path) }, - |config, path, _file_contents| { - config.out_dir = canonicalize( - std::env::current_dir() - .unwrap() - .join("target") - .join("ui_test_cargo/") - .join(path.parent().unwrap()), - ); - }, - if quiet { - status_emitter::Text::quiet() - } else { - status_emitter::Text::verbose() - }, + |_config, _path, _file_contents| {}, + status_emitter::Text::from(args.format), ) .unwrap(); } diff --git a/tests/ui-internal/unnecessary_def_path_hardcoded_path.rs b/tests/ui-internal/unnecessary_def_path_hardcoded_path.rs index 60be297881341..f6abb3cc3d713 100644 --- a/tests/ui-internal/unnecessary_def_path_hardcoded_path.rs +++ b/tests/ui-internal/unnecessary_def_path_hardcoded_path.rs @@ -12,5 +12,5 @@ fn main() { const DEREF_TRAIT_METHOD: [&str; 5] = ["core", "ops", "deref", "Deref", "deref"]; // Don't lint, not a diagnostic or language item - const OPS_MOD: [&str; 5] = ["core", "ops"]; + const OPS_MOD: [&str; 2] = ["core", "ops"]; } diff --git a/tests/ui-toml/borrow_interior_mutable_const/clippy.toml b/tests/ui-toml/borrow_interior_mutable_const/clippy.toml new file mode 100644 index 0000000000000..34a1036e891d8 --- /dev/null +++ b/tests/ui-toml/borrow_interior_mutable_const/clippy.toml @@ -0,0 +1 @@ +ignore-interior-mutability = ["borrow_interior_mutable_const_ignore::Counted"] \ No newline at end of file diff --git a/tests/ui-toml/borrow_interior_mutable_const/ignore.rs b/tests/ui-toml/borrow_interior_mutable_const/ignore.rs new file mode 100644 index 0000000000000..79c7cef6ce1b9 --- /dev/null +++ b/tests/ui-toml/borrow_interior_mutable_const/ignore.rs @@ -0,0 +1,37 @@ +//@compile-flags: --crate-name borrow_interior_mutable_const_ignore + +#![warn(clippy::borrow_interior_mutable_const)] +#![allow(clippy::declare_interior_mutable_const)] + +use core::cell::Cell; +use std::cmp::{Eq, PartialEq}; +use std::collections::{HashMap, HashSet}; +use std::hash::{Hash, Hasher}; +use std::ops::Deref; +use std::sync::atomic::{AtomicUsize, Ordering}; + +struct Counted { + count: AtomicUsize, + val: T, +} + +impl Counted { + const fn new(val: T) -> Self { + Self { + count: AtomicUsize::new(0), + val, + } + } +} + +enum OptionalCell { + Unfrozen(Counted), + Frozen, +} + +const UNFROZEN_VARIANT: OptionalCell = OptionalCell::Unfrozen(Counted::new(true)); +const FROZEN_VARIANT: OptionalCell = OptionalCell::Frozen; + +fn main() { + let _ = &UNFROZEN_VARIANT; +} diff --git a/tests/ui-toml/declare_interior_mutable_const/clippy.toml b/tests/ui-toml/declare_interior_mutable_const/clippy.toml new file mode 100644 index 0000000000000..71d13212e2a87 --- /dev/null +++ b/tests/ui-toml/declare_interior_mutable_const/clippy.toml @@ -0,0 +1 @@ +ignore-interior-mutability = ["declare_interior_mutable_const_ignore::Counted"] \ No newline at end of file diff --git a/tests/ui-toml/declare_interior_mutable_const/ignore.rs b/tests/ui-toml/declare_interior_mutable_const/ignore.rs new file mode 100644 index 0000000000000..6385cf4f852fc --- /dev/null +++ b/tests/ui-toml/declare_interior_mutable_const/ignore.rs @@ -0,0 +1,46 @@ +//@compile-flags: --crate-name declare_interior_mutable_const_ignore + +#![warn(clippy::declare_interior_mutable_const)] +#![allow(clippy::borrow_interior_mutable_const)] + +use core::cell::Cell; +use std::cmp::{Eq, PartialEq}; +use std::collections::{HashMap, HashSet}; +use std::hash::{Hash, Hasher}; +use std::ops::Deref; +use std::sync::atomic::{AtomicUsize, Ordering}; + +struct Counted { + count: AtomicUsize, + val: T, +} + +impl Counted { + const fn new(val: T) -> Self { + Self { + count: AtomicUsize::new(0), + val, + } + } +} + +enum OptionalCell { + Unfrozen(Counted), + Frozen, +} + +const UNFROZEN_VARIANT: OptionalCell = OptionalCell::Unfrozen(Counted::new(true)); +const FROZEN_VARIANT: OptionalCell = OptionalCell::Frozen; + +const fn unfrozen_variant() -> OptionalCell { + OptionalCell::Unfrozen(Counted::new(true)) +} + +const fn frozen_variant() -> OptionalCell { + OptionalCell::Frozen +} + +const UNFROZEN_VARIANT_FROM_FN: OptionalCell = unfrozen_variant(); +const FROZEN_VARIANT_FROM_FN: OptionalCell = frozen_variant(); + +fn main() {} diff --git a/tests/ui-toml/enum_variant_names/enum_variant_names.rs b/tests/ui-toml/enum_variant_names/enum_variant_names.rs deleted file mode 100644 index 8f4e178ccfe17..0000000000000 --- a/tests/ui-toml/enum_variant_names/enum_variant_names.rs +++ /dev/null @@ -1,16 +0,0 @@ -enum Foo { - AFoo, - BFoo, - CFoo, - DFoo, -} -enum Foo2 { - //~^ ERROR: all variants have the same postfix - AFoo, - BFoo, - CFoo, - DFoo, - EFoo, -} - -fn main() {} diff --git a/tests/ui-toml/enum_variant_names/enum_variant_names.stderr b/tests/ui-toml/enum_variant_names/enum_variant_names.stderr deleted file mode 100644 index 11039b1db4877..0000000000000 --- a/tests/ui-toml/enum_variant_names/enum_variant_names.stderr +++ /dev/null @@ -1,18 +0,0 @@ -error: all variants have the same postfix: `Foo` - --> $DIR/enum_variant_names.rs:7:1 - | -LL | / enum Foo2 { -LL | | -LL | | AFoo, -LL | | BFoo, -... | -LL | | EFoo, -LL | | } - | |_^ - | - = help: remove the postfixes and use full paths to the variants instead of glob imports - = note: `-D clippy::enum-variant-names` implied by `-D warnings` - = help: to override `-D warnings` add `#[allow(clippy::enum_variant_names)]` - -error: aborting due to previous error - diff --git a/tests/ui-toml/impl_trait_in_params/clippy.toml b/tests/ui-toml/impl_trait_in_params/clippy.toml new file mode 100644 index 0000000000000..87e1f235741dd --- /dev/null +++ b/tests/ui-toml/impl_trait_in_params/clippy.toml @@ -0,0 +1 @@ +avoid-breaking-exported-api = false \ No newline at end of file diff --git a/tests/ui-toml/impl_trait_in_params/impl_trait_in_params.rs b/tests/ui-toml/impl_trait_in_params/impl_trait_in_params.rs new file mode 100644 index 0000000000000..08fc7edf1c861 --- /dev/null +++ b/tests/ui-toml/impl_trait_in_params/impl_trait_in_params.rs @@ -0,0 +1,16 @@ +//! As avoid-breaking-exported-api is `false`, nothing here should lint +#![warn(clippy::impl_trait_in_params)] +#![no_main] +//@no-rustfix + +pub trait Trait {} + +trait Private { + fn t(_: impl Trait); + fn tt(_: T); +} + +pub trait Public { + fn t(_: impl Trait); //~ ERROR: `impl Trait` used as a function parameter + fn tt(_: T); +} diff --git a/tests/ui-toml/impl_trait_in_params/impl_trait_in_params.stderr b/tests/ui-toml/impl_trait_in_params/impl_trait_in_params.stderr new file mode 100644 index 0000000000000..80c4f5ed4b0c0 --- /dev/null +++ b/tests/ui-toml/impl_trait_in_params/impl_trait_in_params.stderr @@ -0,0 +1,15 @@ +error: `impl Trait` used as a function parameter + --> $DIR/impl_trait_in_params.rs:14:13 + | +LL | fn t(_: impl Trait); + | ^^^^^^^^^^ + | + = note: `-D clippy::impl-trait-in-params` implied by `-D warnings` + = help: to override `-D warnings` add `#[allow(clippy::impl_trait_in_params)]` +help: add a type parameter + | +LL | fn t<{ /* Generic name */ }: Trait>(_: impl Trait); + | +++++++++++++++++++++++++++++++ + +error: aborting due to previous error + diff --git a/tests/ui-toml/invalid_min_rust_version/invalid_min_rust_version.rs b/tests/ui-toml/invalid_min_rust_version/invalid_min_rust_version.rs index 03fa719975b68..85e2fb8c797f8 100644 --- a/tests/ui-toml/invalid_min_rust_version/invalid_min_rust_version.rs +++ b/tests/ui-toml/invalid_min_rust_version/invalid_min_rust_version.rs @@ -1,4 +1,4 @@ -//@error-in-other-file: `invalid.version` is not a valid Rust version +//@error-in-other-file: not a valid Rust version #![allow(clippy::redundant_clone)] diff --git a/tests/ui-toml/invalid_min_rust_version/invalid_min_rust_version.stderr b/tests/ui-toml/invalid_min_rust_version/invalid_min_rust_version.stderr index e9d8fd2e0f525..f127c2408f905 100644 --- a/tests/ui-toml/invalid_min_rust_version/invalid_min_rust_version.stderr +++ b/tests/ui-toml/invalid_min_rust_version/invalid_min_rust_version.stderr @@ -1,4 +1,8 @@ -error: error reading Clippy's configuration file. `invalid.version` is not a valid Rust version +error: error reading Clippy's configuration file: not a valid Rust version + --> $DIR/$DIR/clippy.toml:1:8 + | +LL | msrv = "invalid.version" + | ^^^^^^^^^^^^^^^^^ error: aborting due to previous error diff --git a/tests/ui-toml/enum_variants_threshold0/clippy.toml b/tests/ui-toml/item_name_repetitions/threshold0/clippy.toml similarity index 50% rename from tests/ui-toml/enum_variants_threshold0/clippy.toml rename to tests/ui-toml/item_name_repetitions/threshold0/clippy.toml index f85aade6ae87d..d41edbaf7fa2e 100644 --- a/tests/ui-toml/enum_variants_threshold0/clippy.toml +++ b/tests/ui-toml/item_name_repetitions/threshold0/clippy.toml @@ -1 +1,2 @@ +struct-field-name-threshold = 0 enum-variant-name-threshold = 0 diff --git a/tests/ui-toml/enum_variants_threshold0/enum_variants_name_threshold.rs b/tests/ui-toml/item_name_repetitions/threshold0/item_name_repetitions.rs similarity index 65% rename from tests/ui-toml/enum_variants_threshold0/enum_variants_name_threshold.rs rename to tests/ui-toml/item_name_repetitions/threshold0/item_name_repetitions.rs index 6918d7528c160..b633dcbd19e44 100644 --- a/tests/ui-toml/enum_variants_threshold0/enum_variants_name_threshold.rs +++ b/tests/ui-toml/item_name_repetitions/threshold0/item_name_repetitions.rs @@ -1,3 +1,5 @@ +struct Data {} + enum Actions {} fn main() {} diff --git a/tests/ui-toml/enum_variant_names/clippy.toml b/tests/ui-toml/item_name_repetitions/threshold5/clippy.toml similarity index 50% rename from tests/ui-toml/enum_variant_names/clippy.toml rename to tests/ui-toml/item_name_repetitions/threshold5/clippy.toml index 0ad7a97994849..028a62790792f 100644 --- a/tests/ui-toml/enum_variant_names/clippy.toml +++ b/tests/ui-toml/item_name_repetitions/threshold5/clippy.toml @@ -1 +1,2 @@ enum-variant-name-threshold = 5 +struct-field-name-threshold = 5 diff --git a/tests/ui-toml/item_name_repetitions/threshold5/item_name_repetitions.rs b/tests/ui-toml/item_name_repetitions/threshold5/item_name_repetitions.rs new file mode 100644 index 0000000000000..d437311691d49 --- /dev/null +++ b/tests/ui-toml/item_name_repetitions/threshold5/item_name_repetitions.rs @@ -0,0 +1,32 @@ +#![warn(clippy::struct_field_names)] + +struct Data { + a_data: u8, + b_data: u8, + c_data: u8, + d_data: u8, +} +struct Data2 { + //~^ ERROR: all fields have the same postfix + a_data: u8, + b_data: u8, + c_data: u8, + d_data: u8, + e_data: u8, +} +enum Foo { + AFoo, + BFoo, + CFoo, + DFoo, +} +enum Foo2 { + //~^ ERROR: all variants have the same postfix + AFoo, + BFoo, + CFoo, + DFoo, + EFoo, +} + +fn main() {} diff --git a/tests/ui-toml/item_name_repetitions/threshold5/item_name_repetitions.stderr b/tests/ui-toml/item_name_repetitions/threshold5/item_name_repetitions.stderr new file mode 100644 index 0000000000000..33802c44bf9a3 --- /dev/null +++ b/tests/ui-toml/item_name_repetitions/threshold5/item_name_repetitions.stderr @@ -0,0 +1,34 @@ +error: all fields have the same postfix: `data` + --> $DIR/item_name_repetitions.rs:9:1 + | +LL | / struct Data2 { +LL | | +LL | | a_data: u8, +LL | | b_data: u8, +... | +LL | | e_data: u8, +LL | | } + | |_^ + | + = help: remove the postfixes + = note: `-D clippy::struct-field-names` implied by `-D warnings` + = help: to override `-D warnings` add `#[allow(clippy::struct_field_names)]` + +error: all variants have the same postfix: `Foo` + --> $DIR/item_name_repetitions.rs:23:1 + | +LL | / enum Foo2 { +LL | | +LL | | AFoo, +LL | | BFoo, +... | +LL | | EFoo, +LL | | } + | |_^ + | + = help: remove the postfixes and use full paths to the variants instead of glob imports + = note: `-D clippy::enum-variant-names` implied by `-D warnings` + = help: to override `-D warnings` add `#[allow(clippy::enum_variant_names)]` + +error: aborting due to 2 previous errors + diff --git a/tests/ui-toml/pub_crate_missing_docs/pub_crate_missing_doc.rs b/tests/ui-toml/pub_crate_missing_docs/pub_crate_missing_doc.rs index 830d71f61dd50..cd53f5044599e 100644 --- a/tests/ui-toml/pub_crate_missing_docs/pub_crate_missing_doc.rs +++ b/tests/ui-toml/pub_crate_missing_docs/pub_crate_missing_doc.rs @@ -1,5 +1,6 @@ //! this is crate #![allow(missing_docs)] +#![allow(clippy::struct_field_names)] #![warn(clippy::missing_docs_in_private_items)] /// this is mod diff --git a/tests/ui-toml/pub_crate_missing_docs/pub_crate_missing_doc.stderr b/tests/ui-toml/pub_crate_missing_docs/pub_crate_missing_doc.stderr index 1ecdabbc03e6d..2cf20b4604910 100644 --- a/tests/ui-toml/pub_crate_missing_docs/pub_crate_missing_doc.stderr +++ b/tests/ui-toml/pub_crate_missing_docs/pub_crate_missing_doc.stderr @@ -1,5 +1,5 @@ error: missing documentation for a function - --> $DIR/pub_crate_missing_doc.rs:12:5 + --> $DIR/pub_crate_missing_doc.rs:13:5 | LL | pub(crate) fn crate_no_docs() {} | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -8,25 +8,25 @@ LL | pub(crate) fn crate_no_docs() {} = help: to override `-D warnings` add `#[allow(clippy::missing_docs_in_private_items)]` error: missing documentation for a function - --> $DIR/pub_crate_missing_doc.rs:15:5 + --> $DIR/pub_crate_missing_doc.rs:16:5 | LL | pub(super) fn super_no_docs() {} | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: missing documentation for a function - --> $DIR/pub_crate_missing_doc.rs:23:9 + --> $DIR/pub_crate_missing_doc.rs:24:9 | LL | pub(crate) fn sub_crate_no_docs() {} | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: missing documentation for a struct field - --> $DIR/pub_crate_missing_doc.rs:33:9 + --> $DIR/pub_crate_missing_doc.rs:34:9 | LL | pub(crate) crate_field_no_docs: (), | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: missing documentation for a struct - --> $DIR/pub_crate_missing_doc.rs:39:5 + --> $DIR/pub_crate_missing_doc.rs:40:5 | LL | / pub(crate) struct CrateStructNoDocs { LL | | /// some docs @@ -38,13 +38,13 @@ LL | | } | |_____^ error: missing documentation for a struct field - --> $DIR/pub_crate_missing_doc.rs:42:9 + --> $DIR/pub_crate_missing_doc.rs:43:9 | LL | pub(crate) crate_field_no_docs: (), | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: missing documentation for a type alias - --> $DIR/pub_crate_missing_doc.rs:51:1 + --> $DIR/pub_crate_missing_doc.rs:52:1 | LL | type CrateTypedefNoDocs = String; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ diff --git a/tests/ui-toml/toml_unknown_key/conf_unknown_key.stderr b/tests/ui-toml/toml_unknown_key/conf_unknown_key.stderr index 4bed5c149f517..2f9eaa5178c49 100644 --- a/tests/ui-toml/toml_unknown_key/conf_unknown_key.stderr +++ b/tests/ui-toml/toml_unknown_key/conf_unknown_key.stderr @@ -53,6 +53,7 @@ error: error reading Clippy's configuration file: unknown field `foobar`, expect single-char-binding-names-threshold stack-size-threshold standard-macro-braces + struct-field-name-threshold suppress-restriction-lint-in-const third-party too-large-for-stack @@ -126,6 +127,7 @@ error: error reading Clippy's configuration file: unknown field `barfoo`, expect single-char-binding-names-threshold stack-size-threshold standard-macro-braces + struct-field-name-threshold suppress-restriction-lint-in-const third-party too-large-for-stack diff --git a/tests/ui/author/macro_in_closure.rs b/tests/ui/author/macro_in_closure.rs new file mode 100644 index 0000000000000..444e6a12165d2 --- /dev/null +++ b/tests/ui/author/macro_in_closure.rs @@ -0,0 +1,5 @@ +fn main() { + #[clippy::author] + let print_text = |x| println!("{}", x); + print_text("hello"); +} diff --git a/tests/ui/author/macro_in_closure.stdout b/tests/ui/author/macro_in_closure.stdout new file mode 100644 index 0000000000000..9ab71986f40f4 --- /dev/null +++ b/tests/ui/author/macro_in_closure.stdout @@ -0,0 +1,39 @@ +if let StmtKind::Local(local) = stmt.kind + && let Some(init) = local.init + && let ExprKind::Closure(CaptureBy::Ref, fn_decl, body_id, _, None) = init.kind + && let FnRetTy::DefaultReturn(_) = fn_decl.output + && expr = &cx.tcx.hir().body(body_id).value + && let ExprKind::Block(block, None) = expr.kind + && block.stmts.len() == 1 + && let StmtKind::Semi(e) = block.stmts[0].kind + && let ExprKind::Call(func, args) = e.kind + && let ExprKind::Path(ref qpath) = func.kind + && match_qpath(qpath, &["$crate", "io", "_print"]) + && args.len() == 1 + && let ExprKind::Call(func1, args1) = args[0].kind + && let ExprKind::Path(ref qpath1) = func1.kind + && args1.len() == 2 + && let ExprKind::AddrOf(BorrowKind::Ref, Mutability::Not, inner) = args1[0].kind + && let ExprKind::Array(elements) = inner.kind + && elements.len() == 2 + && let ExprKind::Lit(ref lit) = elements[0].kind + && let LitKind::Str(s, _) = lit.node + && s.as_str() == "" + && let ExprKind::Lit(ref lit1) = elements[1].kind + && let LitKind::Str(s1, _) = lit1.node + && s1.as_str() == "\n" + && let ExprKind::AddrOf(BorrowKind::Ref, Mutability::Not, inner1) = args1[1].kind + && let ExprKind::Array(elements1) = inner1.kind + && elements1.len() == 1 + && let ExprKind::Call(func2, args2) = elements1[0].kind + && let ExprKind::Path(ref qpath2) = func2.kind + && args2.len() == 1 + && let ExprKind::AddrOf(BorrowKind::Ref, Mutability::Not, inner2) = args2[0].kind + && let ExprKind::Path(ref qpath3) = inner2.kind + && match_qpath(qpath3, &["x"]) + && block.expr.is_none() + && let PatKind::Binding(BindingAnnotation::NONE, _, name, None) = local.pat.kind + && name.as_str() == "print_text" +{ + // report your lint here +} diff --git a/tests/ui/author/macro_in_loop.rs b/tests/ui/author/macro_in_loop.rs new file mode 100644 index 0000000000000..8a520501f8dd4 --- /dev/null +++ b/tests/ui/author/macro_in_loop.rs @@ -0,0 +1,8 @@ +#![feature(stmt_expr_attributes)] + +fn main() { + #[clippy::author] + for i in 0..1 { + println!("{}", i); + } +} diff --git a/tests/ui/author/macro_in_loop.stdout b/tests/ui/author/macro_in_loop.stdout new file mode 100644 index 0000000000000..bd054b6abc438 --- /dev/null +++ b/tests/ui/author/macro_in_loop.stdout @@ -0,0 +1,48 @@ +if let Some(higher::ForLoop { pat: pat, arg: arg, body: body, .. }) = higher::ForLoop::hir(expr) + && let PatKind::Binding(BindingAnnotation::NONE, _, name, None) = pat.kind + && name.as_str() == "i" + && let ExprKind::Struct(qpath, fields, None) = arg.kind + && matches!(qpath, QPath::LangItem(LangItem::Range, _)) + && fields.len() == 2 + && fields[0].ident.as_str() == "start" + && let ExprKind::Lit(ref lit) = fields[0].expr.kind + && let LitKind::Int(0, LitIntType::Unsuffixed) = lit.node + && fields[1].ident.as_str() == "end" + && let ExprKind::Lit(ref lit1) = fields[1].expr.kind + && let LitKind::Int(1, LitIntType::Unsuffixed) = lit1.node + && let ExprKind::Block(block, None) = body.kind + && block.stmts.len() == 1 + && let StmtKind::Semi(e) = block.stmts[0].kind + && let ExprKind::Block(block1, None) = e.kind + && block1.stmts.len() == 1 + && let StmtKind::Semi(e1) = block1.stmts[0].kind + && let ExprKind::Call(func, args) = e1.kind + && let ExprKind::Path(ref qpath1) = func.kind + && match_qpath(qpath1, &["$crate", "io", "_print"]) + && args.len() == 1 + && let ExprKind::Call(func1, args1) = args[0].kind + && let ExprKind::Path(ref qpath2) = func1.kind + && args1.len() == 2 + && let ExprKind::AddrOf(BorrowKind::Ref, Mutability::Not, inner) = args1[0].kind + && let ExprKind::Array(elements) = inner.kind + && elements.len() == 2 + && let ExprKind::Lit(ref lit2) = elements[0].kind + && let LitKind::Str(s, _) = lit2.node + && s.as_str() == "" + && let ExprKind::Lit(ref lit3) = elements[1].kind + && let LitKind::Str(s1, _) = lit3.node + && s1.as_str() == "\n" + && let ExprKind::AddrOf(BorrowKind::Ref, Mutability::Not, inner1) = args1[1].kind + && let ExprKind::Array(elements1) = inner1.kind + && elements1.len() == 1 + && let ExprKind::Call(func2, args2) = elements1[0].kind + && let ExprKind::Path(ref qpath3) = func2.kind + && args2.len() == 1 + && let ExprKind::AddrOf(BorrowKind::Ref, Mutability::Not, inner2) = args2[0].kind + && let ExprKind::Path(ref qpath4) = inner2.kind + && match_qpath(qpath4, &["i"]) + && block1.expr.is_none() + && block.expr.is_none() +{ + // report your lint here +} diff --git a/tests/ui/get_first.fixed b/tests/ui/get_first.fixed index b1a597fc4dd5b..a7cdd2a93baca 100644 --- a/tests/ui/get_first.fixed +++ b/tests/ui/get_first.fixed @@ -14,17 +14,20 @@ impl Bar { fn main() { let x = vec![2, 3, 5]; - let _ = x.first(); // Use x.first() + let _ = x.first(); + //~^ ERROR: accessing first element with `x.get(0)` let _ = x.get(1); let _ = x[0]; let y = [2, 3, 5]; - let _ = y.first(); // Use y.first() + let _ = y.first(); + //~^ ERROR: accessing first element with `y.get(0)` let _ = y.get(1); let _ = y[0]; let z = &[2, 3, 5]; - let _ = z.first(); // Use z.first() + let _ = z.first(); + //~^ ERROR: accessing first element with `z.get(0)` let _ = z.get(1); let _ = z[0]; @@ -37,4 +40,8 @@ fn main() { let bar = Bar { arr: [0, 1, 2] }; let _ = bar.get(0); // Do not lint, because Bar is struct. + + let non_primitives = [vec![1, 2], vec![3, 4]]; + let _ = non_primitives.first(); + //~^ ERROR: accessing first element with `non_primitives.get(0)` } diff --git a/tests/ui/get_first.rs b/tests/ui/get_first.rs index e27ee4be8c087..cca743c4bf5e0 100644 --- a/tests/ui/get_first.rs +++ b/tests/ui/get_first.rs @@ -14,17 +14,20 @@ impl Bar { fn main() { let x = vec![2, 3, 5]; - let _ = x.get(0); // Use x.first() + let _ = x.get(0); + //~^ ERROR: accessing first element with `x.get(0)` let _ = x.get(1); let _ = x[0]; let y = [2, 3, 5]; - let _ = y.get(0); // Use y.first() + let _ = y.get(0); + //~^ ERROR: accessing first element with `y.get(0)` let _ = y.get(1); let _ = y[0]; let z = &[2, 3, 5]; - let _ = z.get(0); // Use z.first() + let _ = z.get(0); + //~^ ERROR: accessing first element with `z.get(0)` let _ = z.get(1); let _ = z[0]; @@ -37,4 +40,8 @@ fn main() { let bar = Bar { arr: [0, 1, 2] }; let _ = bar.get(0); // Do not lint, because Bar is struct. + + let non_primitives = [vec![1, 2], vec![3, 4]]; + let _ = non_primitives.get(0); + //~^ ERROR: accessing first element with `non_primitives.get(0)` } diff --git a/tests/ui/get_first.stderr b/tests/ui/get_first.stderr index 56b4c29a31353..8ee66e33cc813 100644 --- a/tests/ui/get_first.stderr +++ b/tests/ui/get_first.stderr @@ -1,23 +1,29 @@ error: accessing first element with `x.get(0)` --> $DIR/get_first.rs:17:13 | -LL | let _ = x.get(0); // Use x.first() +LL | let _ = x.get(0); | ^^^^^^^^ help: try: `x.first()` | = note: `-D clippy::get-first` implied by `-D warnings` = help: to override `-D warnings` add `#[allow(clippy::get_first)]` error: accessing first element with `y.get(0)` - --> $DIR/get_first.rs:22:13 + --> $DIR/get_first.rs:23:13 | -LL | let _ = y.get(0); // Use y.first() +LL | let _ = y.get(0); | ^^^^^^^^ help: try: `y.first()` error: accessing first element with `z.get(0)` - --> $DIR/get_first.rs:27:13 + --> $DIR/get_first.rs:29:13 | -LL | let _ = z.get(0); // Use z.first() +LL | let _ = z.get(0); | ^^^^^^^^ help: try: `z.first()` -error: aborting due to 3 previous errors +error: accessing first element with `non_primitives.get(0)` + --> $DIR/get_first.rs:45:13 + | +LL | let _ = non_primitives.get(0); + | ^^^^^^^^^^^^^^^^^^^^^ help: try: `non_primitives.first()` + +error: aborting due to 4 previous errors diff --git a/tests/ui/impl_trait_in_params.rs b/tests/ui/impl_trait_in_params.rs index b652e4a4abe2e..a6251a370d1d0 100644 --- a/tests/ui/impl_trait_in_params.rs +++ b/tests/ui/impl_trait_in_params.rs @@ -1,20 +1,47 @@ #![allow(unused)] #![warn(clippy::impl_trait_in_params)] + //@no-rustfix pub trait Trait {} pub trait AnotherTrait {} // Should warn pub fn a(_: impl Trait) {} -//~^ ERROR: '`impl Trait` used as a function parameter' -//~| NOTE: `-D clippy::impl-trait-in-params` implied by `-D warnings` +//~^ ERROR: `impl Trait` used as a function parameter pub fn c(_: C, _: impl Trait) {} -//~^ ERROR: '`impl Trait` used as a function parameter' -fn d(_: impl AnotherTrait) {} +//~^ ERROR: `impl Trait` used as a function parameter // Shouldn't warn pub fn b(_: B) {} fn e>(_: T) {} +fn d(_: impl AnotherTrait) {} + +//------ IMPLS + +pub trait Public { + // See test in ui-toml for a case where avoid-breaking-exported-api is set to false + fn t(_: impl Trait); + fn tt(_: T) {} +} + +trait Private { + // This shouldn't lint + fn t(_: impl Trait); + fn tt(_: T) {} +} + +struct S; +impl S { + pub fn h(_: impl Trait) {} //~ ERROR: `impl Trait` used as a function parameter + fn i(_: impl Trait) {} + pub fn j(_: J) {} + pub fn k>(_: K, _: impl AnotherTrait) {} //~ ERROR: `impl Trait` used as a function parameter +} + +// Trying with traits +impl Public for S { + fn t(_: impl Trait) {} +} fn main() {} diff --git a/tests/ui/impl_trait_in_params.stderr b/tests/ui/impl_trait_in_params.stderr index 36b4f27e9a456..0ae7a3672d190 100644 --- a/tests/ui/impl_trait_in_params.stderr +++ b/tests/ui/impl_trait_in_params.stderr @@ -1,5 +1,5 @@ -error: '`impl Trait` used as a function parameter' - --> $DIR/impl_trait_in_params.rs:8:13 +error: `impl Trait` used as a function parameter + --> $DIR/impl_trait_in_params.rs:9:13 | LL | pub fn a(_: impl Trait) {} | ^^^^^^^^^^ @@ -11,7 +11,7 @@ help: add a type parameter LL | pub fn a<{ /* Generic name */ }: Trait>(_: impl Trait) {} | +++++++++++++++++++++++++++++++ -error: '`impl Trait` used as a function parameter' +error: `impl Trait` used as a function parameter --> $DIR/impl_trait_in_params.rs:11:29 | LL | pub fn c(_: C, _: impl Trait) {} @@ -22,5 +22,27 @@ help: add a type parameter LL | pub fn c(_: C, _: impl Trait) {} | +++++++++++++++++++++++++++++++ -error: aborting due to 2 previous errors +error: `impl Trait` used as a function parameter + --> $DIR/impl_trait_in_params.rs:36:17 + | +LL | pub fn h(_: impl Trait) {} + | ^^^^^^^^^^ + | +help: add a type parameter + | +LL | pub fn h<{ /* Generic name */ }: Trait>(_: impl Trait) {} + | +++++++++++++++++++++++++++++++ + +error: `impl Trait` used as a function parameter + --> $DIR/impl_trait_in_params.rs:39:45 + | +LL | pub fn k>(_: K, _: impl AnotherTrait) {} + | ^^^^^^^^^^^^^^^^^^^^^^ + | +help: add a type parameter + | +LL | pub fn k, { /* Generic name */ }: AnotherTrait>(_: K, _: impl AnotherTrait) {} + | +++++++++++++++++++++++++++++++++++++++++++ + +error: aborting due to 4 previous errors diff --git a/tests/ui/into_iter_without_iter.rs b/tests/ui/into_iter_without_iter.rs index 6be3bb8abdddc..e6ff821a8ad04 100644 --- a/tests/ui/into_iter_without_iter.rs +++ b/tests/ui/into_iter_without_iter.rs @@ -122,3 +122,37 @@ fn main() { } } } + +fn issue11635() { + // A little more involved than the original repro in the issue, but this tests that it correctly + // works for more than one deref step + + use std::ops::Deref; + + pub struct Thing(Vec); + pub struct Thing2(Thing); + + impl Deref for Thing { + type Target = [u8]; + + fn deref(&self) -> &Self::Target { + &self.0 + } + } + + impl Deref for Thing2 { + type Target = Thing; + fn deref(&self) -> &Self::Target { + &self.0 + } + } + + impl<'a> IntoIterator for &'a Thing2 { + type Item = &'a u8; + type IntoIter = <&'a [u8] as IntoIterator>::IntoIter; + + fn into_iter(self) -> Self::IntoIter { + self.0.iter() + } + } +} diff --git a/tests/ui/manual_filter_map.fixed b/tests/ui/manual_filter_map.fixed index 4de45e39b1004..a44c46c145c6c 100644 --- a/tests/ui/manual_filter_map.fixed +++ b/tests/ui/manual_filter_map.fixed @@ -2,6 +2,7 @@ #![warn(clippy::manual_filter_map)] #![allow(clippy::redundant_closure)] // FIXME suggestion may have redundant closure #![allow(clippy::useless_vec)] +#![allow(clippy::struct_field_names)] fn main() { // is_some(), unwrap() diff --git a/tests/ui/manual_filter_map.rs b/tests/ui/manual_filter_map.rs index 22f316f90b6a1..e72d0c4305b34 100644 --- a/tests/ui/manual_filter_map.rs +++ b/tests/ui/manual_filter_map.rs @@ -2,6 +2,7 @@ #![warn(clippy::manual_filter_map)] #![allow(clippy::redundant_closure)] // FIXME suggestion may have redundant closure #![allow(clippy::useless_vec)] +#![allow(clippy::struct_field_names)] fn main() { // is_some(), unwrap() diff --git a/tests/ui/manual_filter_map.stderr b/tests/ui/manual_filter_map.stderr index 0bfc1f5c7450b..cf64bb25951a3 100644 --- a/tests/ui/manual_filter_map.stderr +++ b/tests/ui/manual_filter_map.stderr @@ -1,11 +1,11 @@ error: `filter(..).map(..)` can be simplified as `filter_map(..)` - --> $DIR/manual_filter_map.rs:8:19 + --> $DIR/manual_filter_map.rs:9:19 | LL | let _ = (0..).filter(|n| to_opt(*n).is_some()).map(|a| to_opt(a).unwrap()); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `filter_map(|a| to_opt(a))` | note: the suggestion might change the behavior of the program when merging `filter` and `map`, because this expression potentially contains side effects and will only execute once - --> $DIR/manual_filter_map.rs:8:30 + --> $DIR/manual_filter_map.rs:9:30 | LL | let _ = (0..).filter(|n| to_opt(*n).is_some()).map(|a| to_opt(a).unwrap()); | ^^^^^^^^^^ @@ -13,31 +13,31 @@ LL | let _ = (0..).filter(|n| to_opt(*n).is_some()).map(|a| to_opt(a).unwrap = help: to override `-D warnings` add `#[allow(clippy::manual_filter_map)]` error: `filter(..).map(..)` can be simplified as `filter_map(..)` - --> $DIR/manual_filter_map.rs:11:19 + --> $DIR/manual_filter_map.rs:12:19 | LL | let _ = (0..).filter(|&n| to_opt(n).is_some()).map(|a| to_opt(a).expect("hi")); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `filter_map(|a| to_opt(a))` | note: the suggestion might change the behavior of the program when merging `filter` and `map`, because this expression potentially contains side effects and will only execute once - --> $DIR/manual_filter_map.rs:11:31 + --> $DIR/manual_filter_map.rs:12:31 | LL | let _ = (0..).filter(|&n| to_opt(n).is_some()).map(|a| to_opt(a).expect("hi")); | ^^^^^^^^^ error: `filter(..).map(..)` can be simplified as `filter_map(..)` - --> $DIR/manual_filter_map.rs:14:19 + --> $DIR/manual_filter_map.rs:15:19 | LL | let _ = (0..).filter(|&n| to_res(n).is_ok()).map(|a| to_res(a).unwrap_or(1)); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `filter_map(|a| to_res(a).ok())` | note: the suggestion might change the behavior of the program when merging `filter` and `map`, because this expression potentially contains side effects and will only execute once - --> $DIR/manual_filter_map.rs:14:31 + --> $DIR/manual_filter_map.rs:15:31 | LL | let _ = (0..).filter(|&n| to_res(n).is_ok()).map(|a| to_res(a).unwrap_or(1)); | ^^^^^^^^^ error: `filter(..).map(..)` can be simplified as `filter_map(..)` - --> $DIR/manual_filter_map.rs:17:10 + --> $DIR/manual_filter_map.rs:18:10 | LL | .filter(|&x| to_ref(to_opt(x)).is_some()) | __________^ @@ -45,13 +45,13 @@ LL | | .map(|y| to_ref(to_opt(y)).unwrap()); | |____________________________________________^ help: try: `filter_map(|y| *to_ref(to_opt(y)))` | note: the suggestion might change the behavior of the program when merging `filter` and `map`, because this expression potentially contains side effects and will only execute once - --> $DIR/manual_filter_map.rs:17:22 + --> $DIR/manual_filter_map.rs:18:22 | LL | .filter(|&x| to_ref(to_opt(x)).is_some()) | ^^^^^^^^^^^^^^^^^ error: `filter(..).map(..)` can be simplified as `filter_map(..)` - --> $DIR/manual_filter_map.rs:20:10 + --> $DIR/manual_filter_map.rs:21:10 | LL | .filter(|x| to_ref(to_opt(*x)).is_some()) | __________^ @@ -59,13 +59,13 @@ LL | | .map(|y| to_ref(to_opt(y)).unwrap()); | |____________________________________________^ help: try: `filter_map(|y| *to_ref(to_opt(y)))` | note: the suggestion might change the behavior of the program when merging `filter` and `map`, because this expression potentially contains side effects and will only execute once - --> $DIR/manual_filter_map.rs:20:21 + --> $DIR/manual_filter_map.rs:21:21 | LL | .filter(|x| to_ref(to_opt(*x)).is_some()) | ^^^^^^^^^^^^^^^^^^ error: `filter(..).map(..)` can be simplified as `filter_map(..)` - --> $DIR/manual_filter_map.rs:24:10 + --> $DIR/manual_filter_map.rs:25:10 | LL | .filter(|&x| to_ref(to_res(x)).is_ok()) | __________^ @@ -73,13 +73,13 @@ LL | | .map(|y| to_ref(to_res(y)).unwrap()); | |____________________________________________^ help: try: `filter_map(|y| to_ref(to_res(y)).ok())` | note: the suggestion might change the behavior of the program when merging `filter` and `map`, because this expression potentially contains side effects and will only execute once - --> $DIR/manual_filter_map.rs:24:22 + --> $DIR/manual_filter_map.rs:25:22 | LL | .filter(|&x| to_ref(to_res(x)).is_ok()) | ^^^^^^^^^^^^^^^^^ error: `filter(..).map(..)` can be simplified as `filter_map(..)` - --> $DIR/manual_filter_map.rs:27:10 + --> $DIR/manual_filter_map.rs:28:10 | LL | .filter(|x| to_ref(to_res(*x)).is_ok()) | __________^ @@ -87,13 +87,13 @@ LL | | .map(|y| to_ref(to_res(y)).unwrap()); | |____________________________________________^ help: try: `filter_map(|y| to_ref(to_res(y)).ok())` | note: the suggestion might change the behavior of the program when merging `filter` and `map`, because this expression potentially contains side effects and will only execute once - --> $DIR/manual_filter_map.rs:27:21 + --> $DIR/manual_filter_map.rs:28:21 | LL | .filter(|x| to_ref(to_res(*x)).is_ok()) | ^^^^^^^^^^^^^^^^^^ error: `find(..).map(..)` can be simplified as `find_map(..)` - --> $DIR/manual_filter_map.rs:33:27 + --> $DIR/manual_filter_map.rs:34:27 | LL | iter::>().find(|x| x.is_some()).map(|x| x.cloned().unwrap()); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `find_map(|x| x.cloned())` @@ -102,79 +102,79 @@ LL | iter::>().find(|x| x.is_some()).map(|x| x.cloned().unwrap() = help: to override `-D warnings` add `#[allow(clippy::manual_find_map)]` error: `find(..).map(..)` can be simplified as `find_map(..)` - --> $DIR/manual_filter_map.rs:34:28 + --> $DIR/manual_filter_map.rs:35:28 | LL | iter::<&Option<&u8>>().find(|x| x.is_some()).map(|x| x.cloned().unwrap()); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `find_map(|x| x.cloned())` error: `find(..).map(..)` can be simplified as `find_map(..)` - --> $DIR/manual_filter_map.rs:35:31 + --> $DIR/manual_filter_map.rs:36:31 | LL | iter::<&Option>().find(|x| x.is_some()).map(|x| x.as_deref().unwrap()); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `find_map(|x| x.as_deref())` error: `find(..).map(..)` can be simplified as `find_map(..)` - --> $DIR/manual_filter_map.rs:36:31 + --> $DIR/manual_filter_map.rs:37:31 | LL | iter::>().find(|&x| to_ref(x).is_some()).map(|y| to_ref(y).cloned().unwrap()); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `find_map(|y| to_ref(y).cloned())` | note: the suggestion might change the behavior of the program when merging `filter` and `map`, because this expression potentially contains side effects and will only execute once - --> $DIR/manual_filter_map.rs:36:41 + --> $DIR/manual_filter_map.rs:37:41 | LL | iter::>().find(|&x| to_ref(x).is_some()).map(|y| to_ref(y).cloned().unwrap()); | ^^^^^^^^^ error: `find(..).map(..)` can be simplified as `find_map(..)` - --> $DIR/manual_filter_map.rs:38:30 + --> $DIR/manual_filter_map.rs:39:30 | LL | iter::>().find(|x| x.is_ok()).map(|x| x.unwrap()); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `find_map(|x| x.ok())` error: `find(..).map(..)` can be simplified as `find_map(..)` - --> $DIR/manual_filter_map.rs:39:31 + --> $DIR/manual_filter_map.rs:40:31 | LL | iter::<&Result>().find(|x| x.is_ok()).map(|x| x.unwrap()); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `find_map(|x| x.ok())` error: `find(..).map(..)` can be simplified as `find_map(..)` - --> $DIR/manual_filter_map.rs:40:32 + --> $DIR/manual_filter_map.rs:41:32 | LL | iter::<&&Result>().find(|x| x.is_ok()).map(|x| x.unwrap()); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `find_map(|x| x.ok())` error: `find(..).map(..)` can be simplified as `find_map(..)` - --> $DIR/manual_filter_map.rs:41:31 + --> $DIR/manual_filter_map.rs:42:31 | LL | iter::>().find(|x| x.is_ok()).map(|x| x.cloned().unwrap()); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `find_map(|x| x.cloned().ok())` error: `find(..).map(..)` can be simplified as `find_map(..)` - --> $DIR/manual_filter_map.rs:42:32 + --> $DIR/manual_filter_map.rs:43:32 | LL | iter::<&Result<&u8, ()>>().find(|x| x.is_ok()).map(|x| x.cloned().unwrap()); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `find_map(|x| x.cloned().ok())` error: `find(..).map(..)` can be simplified as `find_map(..)` - --> $DIR/manual_filter_map.rs:43:35 + --> $DIR/manual_filter_map.rs:44:35 | LL | iter::<&Result>().find(|x| x.is_ok()).map(|x| x.as_deref().unwrap()); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `find_map(|x| x.as_deref().ok())` error: `find(..).map(..)` can be simplified as `find_map(..)` - --> $DIR/manual_filter_map.rs:44:35 + --> $DIR/manual_filter_map.rs:45:35 | LL | iter::>().find(|&x| to_ref(x).is_ok()).map(|y| to_ref(y).cloned().unwrap()); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `find_map(|y| to_ref(y).cloned().ok())` | note: the suggestion might change the behavior of the program when merging `filter` and `map`, because this expression potentially contains side effects and will only execute once - --> $DIR/manual_filter_map.rs:44:45 + --> $DIR/manual_filter_map.rs:45:45 | LL | iter::>().find(|&x| to_ref(x).is_ok()).map(|y| to_ref(y).cloned().unwrap()); | ^^^^^^^^^ error: `filter(..).map(..)` can be simplified as `filter_map(..)` - --> $DIR/manual_filter_map.rs:92:10 + --> $DIR/manual_filter_map.rs:93:10 | LL | .filter(|f| f.option_field.is_some()) | __________^ @@ -182,7 +182,7 @@ LL | | .map(|f| f.option_field.clone().unwrap()); | |_________________________________________________^ help: try: `filter_map(|f| f.option_field.clone())` error: `filter(..).map(..)` can be simplified as `filter_map(..)` - --> $DIR/manual_filter_map.rs:97:10 + --> $DIR/manual_filter_map.rs:98:10 | LL | .filter(|f| f.ref_field.is_some()) | __________^ @@ -190,7 +190,7 @@ LL | | .map(|f| f.ref_field.cloned().unwrap()); | |_______________________________________________^ help: try: `filter_map(|f| f.ref_field.cloned())` error: `filter(..).map(..)` can be simplified as `filter_map(..)` - --> $DIR/manual_filter_map.rs:102:10 + --> $DIR/manual_filter_map.rs:103:10 | LL | .filter(|f| f.ref_field.is_some()) | __________^ @@ -198,7 +198,7 @@ LL | | .map(|f| f.ref_field.copied().unwrap()); | |_______________________________________________^ help: try: `filter_map(|f| f.ref_field.copied())` error: `filter(..).map(..)` can be simplified as `filter_map(..)` - --> $DIR/manual_filter_map.rs:107:10 + --> $DIR/manual_filter_map.rs:108:10 | LL | .filter(|f| f.result_field.is_ok()) | __________^ @@ -206,7 +206,7 @@ LL | | .map(|f| f.result_field.clone().unwrap()); | |_________________________________________________^ help: try: `filter_map(|f| f.result_field.clone().ok())` error: `filter(..).map(..)` can be simplified as `filter_map(..)` - --> $DIR/manual_filter_map.rs:112:10 + --> $DIR/manual_filter_map.rs:113:10 | LL | .filter(|f| f.result_field.is_ok()) | __________^ @@ -214,7 +214,7 @@ LL | | .map(|f| f.result_field.as_ref().unwrap()); | |__________________________________________________^ help: try: `filter_map(|f| f.result_field.as_ref().ok())` error: `filter(..).map(..)` can be simplified as `filter_map(..)` - --> $DIR/manual_filter_map.rs:117:10 + --> $DIR/manual_filter_map.rs:118:10 | LL | .filter(|f| f.result_field.is_ok()) | __________^ @@ -222,7 +222,7 @@ LL | | .map(|f| f.result_field.as_deref().unwrap()); | |____________________________________________________^ help: try: `filter_map(|f| f.result_field.as_deref().ok())` error: `filter(..).map(..)` can be simplified as `filter_map(..)` - --> $DIR/manual_filter_map.rs:122:10 + --> $DIR/manual_filter_map.rs:123:10 | LL | .filter(|f| f.result_field.is_ok()) | __________^ @@ -230,7 +230,7 @@ LL | | .map(|f| f.result_field.as_mut().unwrap()); | |__________________________________________________^ help: try: `filter_map(|f| f.result_field.as_mut().ok())` error: `filter(..).map(..)` can be simplified as `filter_map(..)` - --> $DIR/manual_filter_map.rs:127:10 + --> $DIR/manual_filter_map.rs:128:10 | LL | .filter(|f| f.result_field.is_ok()) | __________^ @@ -238,7 +238,7 @@ LL | | .map(|f| f.result_field.as_deref_mut().unwrap()); | |________________________________________________________^ help: try: `filter_map(|f| f.result_field.as_deref_mut().ok())` error: `filter(..).map(..)` can be simplified as `filter_map(..)` - --> $DIR/manual_filter_map.rs:132:10 + --> $DIR/manual_filter_map.rs:133:10 | LL | .filter(|f| f.result_field.is_ok()) | __________^ @@ -246,7 +246,7 @@ LL | | .map(|f| f.result_field.to_owned().unwrap()); | |____________________________________________________^ help: try: `filter_map(|f| f.result_field.to_owned().ok())` error: `filter(..).map(..)` can be simplified as `filter_map(..)` - --> $DIR/manual_filter_map.rs:145:27 + --> $DIR/manual_filter_map.rs:146:27 | LL | let _x = iter.clone().filter(|x| matches!(x, Enum::A(_))).map(|x| match x { | ___________________________^ @@ -256,7 +256,7 @@ LL | | }); | |______^ help: try: `filter_map(|x| match x { Enum::A(s) => Some(s), _ => None })` error: `filter(..).map(..)` can be simplified as `filter_map(..)` - --> $DIR/manual_filter_map.rs:155:10 + --> $DIR/manual_filter_map.rs:156:10 | LL | .filter(|x| matches!(x, Enum::A(_))) | __________^ diff --git a/tests/ui/manual_find_map.fixed b/tests/ui/manual_find_map.fixed index 0e92d25e68f1c..2d9a356b9bcda 100644 --- a/tests/ui/manual_find_map.fixed +++ b/tests/ui/manual_find_map.fixed @@ -2,6 +2,7 @@ #![warn(clippy::manual_find_map)] #![allow(clippy::redundant_closure)] // FIXME suggestion may have redundant closure #![allow(clippy::useless_vec)] +#![allow(clippy::struct_field_names)] fn main() { // is_some(), unwrap() diff --git a/tests/ui/manual_find_map.rs b/tests/ui/manual_find_map.rs index b2568c823ebcf..7c5cc136695a7 100644 --- a/tests/ui/manual_find_map.rs +++ b/tests/ui/manual_find_map.rs @@ -2,6 +2,7 @@ #![warn(clippy::manual_find_map)] #![allow(clippy::redundant_closure)] // FIXME suggestion may have redundant closure #![allow(clippy::useless_vec)] +#![allow(clippy::struct_field_names)] fn main() { // is_some(), unwrap() diff --git a/tests/ui/manual_find_map.stderr b/tests/ui/manual_find_map.stderr index 0dc9ae1df035f..0526382323d36 100644 --- a/tests/ui/manual_find_map.stderr +++ b/tests/ui/manual_find_map.stderr @@ -1,11 +1,11 @@ error: `find(..).map(..)` can be simplified as `find_map(..)` - --> $DIR/manual_find_map.rs:8:19 + --> $DIR/manual_find_map.rs:9:19 | LL | let _ = (0..).find(|n| to_opt(*n).is_some()).map(|a| to_opt(a).unwrap()); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `find_map(|a| to_opt(a))` | note: the suggestion might change the behavior of the program when merging `filter` and `map`, because this expression potentially contains side effects and will only execute once - --> $DIR/manual_find_map.rs:8:28 + --> $DIR/manual_find_map.rs:9:28 | LL | let _ = (0..).find(|n| to_opt(*n).is_some()).map(|a| to_opt(a).unwrap()); | ^^^^^^^^^^ @@ -13,31 +13,31 @@ LL | let _ = (0..).find(|n| to_opt(*n).is_some()).map(|a| to_opt(a).unwrap() = help: to override `-D warnings` add `#[allow(clippy::manual_find_map)]` error: `find(..).map(..)` can be simplified as `find_map(..)` - --> $DIR/manual_find_map.rs:11:19 + --> $DIR/manual_find_map.rs:12:19 | LL | let _ = (0..).find(|&n| to_opt(n).is_some()).map(|a| to_opt(a).expect("hi")); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `find_map(|a| to_opt(a))` | note: the suggestion might change the behavior of the program when merging `filter` and `map`, because this expression potentially contains side effects and will only execute once - --> $DIR/manual_find_map.rs:11:29 + --> $DIR/manual_find_map.rs:12:29 | LL | let _ = (0..).find(|&n| to_opt(n).is_some()).map(|a| to_opt(a).expect("hi")); | ^^^^^^^^^ error: `find(..).map(..)` can be simplified as `find_map(..)` - --> $DIR/manual_find_map.rs:14:19 + --> $DIR/manual_find_map.rs:15:19 | LL | let _ = (0..).find(|&n| to_res(n).is_ok()).map(|a| to_res(a).unwrap_or(1)); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `find_map(|a| to_res(a).ok())` | note: the suggestion might change the behavior of the program when merging `filter` and `map`, because this expression potentially contains side effects and will only execute once - --> $DIR/manual_find_map.rs:14:29 + --> $DIR/manual_find_map.rs:15:29 | LL | let _ = (0..).find(|&n| to_res(n).is_ok()).map(|a| to_res(a).unwrap_or(1)); | ^^^^^^^^^ error: `find(..).map(..)` can be simplified as `find_map(..)` - --> $DIR/manual_find_map.rs:17:10 + --> $DIR/manual_find_map.rs:18:10 | LL | .find(|&x| to_ref(to_opt(x)).is_some()) | __________^ @@ -45,13 +45,13 @@ LL | | .map(|y| to_ref(to_opt(y)).unwrap()); | |____________________________________________^ help: try: `find_map(|y| *to_ref(to_opt(y)))` | note: the suggestion might change the behavior of the program when merging `filter` and `map`, because this expression potentially contains side effects and will only execute once - --> $DIR/manual_find_map.rs:17:20 + --> $DIR/manual_find_map.rs:18:20 | LL | .find(|&x| to_ref(to_opt(x)).is_some()) | ^^^^^^^^^^^^^^^^^ error: `find(..).map(..)` can be simplified as `find_map(..)` - --> $DIR/manual_find_map.rs:20:10 + --> $DIR/manual_find_map.rs:21:10 | LL | .find(|x| to_ref(to_opt(*x)).is_some()) | __________^ @@ -59,13 +59,13 @@ LL | | .map(|y| to_ref(to_opt(y)).unwrap()); | |____________________________________________^ help: try: `find_map(|y| *to_ref(to_opt(y)))` | note: the suggestion might change the behavior of the program when merging `filter` and `map`, because this expression potentially contains side effects and will only execute once - --> $DIR/manual_find_map.rs:20:19 + --> $DIR/manual_find_map.rs:21:19 | LL | .find(|x| to_ref(to_opt(*x)).is_some()) | ^^^^^^^^^^^^^^^^^^ error: `find(..).map(..)` can be simplified as `find_map(..)` - --> $DIR/manual_find_map.rs:24:10 + --> $DIR/manual_find_map.rs:25:10 | LL | .find(|&x| to_ref(to_res(x)).is_ok()) | __________^ @@ -73,13 +73,13 @@ LL | | .map(|y| to_ref(to_res(y)).unwrap()); | |____________________________________________^ help: try: `find_map(|y| to_ref(to_res(y)).ok())` | note: the suggestion might change the behavior of the program when merging `filter` and `map`, because this expression potentially contains side effects and will only execute once - --> $DIR/manual_find_map.rs:24:20 + --> $DIR/manual_find_map.rs:25:20 | LL | .find(|&x| to_ref(to_res(x)).is_ok()) | ^^^^^^^^^^^^^^^^^ error: `find(..).map(..)` can be simplified as `find_map(..)` - --> $DIR/manual_find_map.rs:27:10 + --> $DIR/manual_find_map.rs:28:10 | LL | .find(|x| to_ref(to_res(*x)).is_ok()) | __________^ @@ -87,109 +87,109 @@ LL | | .map(|y| to_ref(to_res(y)).unwrap()); | |____________________________________________^ help: try: `find_map(|y| to_ref(to_res(y)).ok())` | note: the suggestion might change the behavior of the program when merging `filter` and `map`, because this expression potentially contains side effects and will only execute once - --> $DIR/manual_find_map.rs:27:19 + --> $DIR/manual_find_map.rs:28:19 | LL | .find(|x| to_ref(to_res(*x)).is_ok()) | ^^^^^^^^^^^^^^^^^^ error: `find(..).map(..)` can be simplified as `find_map(..)` - --> $DIR/manual_find_map.rs:33:26 + --> $DIR/manual_find_map.rs:34:26 | LL | iter::>().find(|x| x.is_some()).map(|x| x.unwrap()); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `find_map(|x| x)` error: `find(..).map(..)` can be simplified as `find_map(..)` - --> $DIR/manual_find_map.rs:34:27 + --> $DIR/manual_find_map.rs:35:27 | LL | iter::<&Option>().find(|x| x.is_some()).map(|x| x.unwrap()); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `find_map(|x| *x)` error: `find(..).map(..)` can be simplified as `find_map(..)` - --> $DIR/manual_find_map.rs:35:28 + --> $DIR/manual_find_map.rs:36:28 | LL | iter::<&&Option>().find(|x| x.is_some()).map(|x| x.unwrap()); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `find_map(|x| **x)` error: `find(..).map(..)` can be simplified as `find_map(..)` - --> $DIR/manual_find_map.rs:36:27 + --> $DIR/manual_find_map.rs:37:27 | LL | iter::>().find(|x| x.is_some()).map(|x| x.cloned().unwrap()); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `find_map(|x| x.cloned())` error: `find(..).map(..)` can be simplified as `find_map(..)` - --> $DIR/manual_find_map.rs:37:28 + --> $DIR/manual_find_map.rs:38:28 | LL | iter::<&Option<&u8>>().find(|x| x.is_some()).map(|x| x.cloned().unwrap()); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `find_map(|x| x.cloned())` error: `find(..).map(..)` can be simplified as `find_map(..)` - --> $DIR/manual_find_map.rs:38:31 + --> $DIR/manual_find_map.rs:39:31 | LL | iter::<&Option>().find(|x| x.is_some()).map(|x| x.as_deref().unwrap()); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `find_map(|x| x.as_deref())` error: `find(..).map(..)` can be simplified as `find_map(..)` - --> $DIR/manual_find_map.rs:39:31 + --> $DIR/manual_find_map.rs:40:31 | LL | iter::>().find(|&x| to_ref(x).is_some()).map(|y| to_ref(y).cloned().unwrap()); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `find_map(|y| to_ref(y).cloned())` | note: the suggestion might change the behavior of the program when merging `filter` and `map`, because this expression potentially contains side effects and will only execute once - --> $DIR/manual_find_map.rs:39:41 + --> $DIR/manual_find_map.rs:40:41 | LL | iter::>().find(|&x| to_ref(x).is_some()).map(|y| to_ref(y).cloned().unwrap()); | ^^^^^^^^^ error: `find(..).map(..)` can be simplified as `find_map(..)` - --> $DIR/manual_find_map.rs:41:30 + --> $DIR/manual_find_map.rs:42:30 | LL | iter::>().find(|x| x.is_ok()).map(|x| x.unwrap()); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `find_map(|x| x.ok())` error: `find(..).map(..)` can be simplified as `find_map(..)` - --> $DIR/manual_find_map.rs:42:31 + --> $DIR/manual_find_map.rs:43:31 | LL | iter::<&Result>().find(|x| x.is_ok()).map(|x| x.unwrap()); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `find_map(|x| x.ok())` error: `find(..).map(..)` can be simplified as `find_map(..)` - --> $DIR/manual_find_map.rs:43:32 + --> $DIR/manual_find_map.rs:44:32 | LL | iter::<&&Result>().find(|x| x.is_ok()).map(|x| x.unwrap()); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `find_map(|x| x.ok())` error: `find(..).map(..)` can be simplified as `find_map(..)` - --> $DIR/manual_find_map.rs:44:31 + --> $DIR/manual_find_map.rs:45:31 | LL | iter::>().find(|x| x.is_ok()).map(|x| x.cloned().unwrap()); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `find_map(|x| x.cloned().ok())` error: `find(..).map(..)` can be simplified as `find_map(..)` - --> $DIR/manual_find_map.rs:45:32 + --> $DIR/manual_find_map.rs:46:32 | LL | iter::<&Result<&u8, ()>>().find(|x| x.is_ok()).map(|x| x.cloned().unwrap()); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `find_map(|x| x.cloned().ok())` error: `find(..).map(..)` can be simplified as `find_map(..)` - --> $DIR/manual_find_map.rs:46:35 + --> $DIR/manual_find_map.rs:47:35 | LL | iter::<&Result>().find(|x| x.is_ok()).map(|x| x.as_deref().unwrap()); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `find_map(|x| x.as_deref().ok())` error: `find(..).map(..)` can be simplified as `find_map(..)` - --> $DIR/manual_find_map.rs:47:35 + --> $DIR/manual_find_map.rs:48:35 | LL | iter::>().find(|&x| to_ref(x).is_ok()).map(|y| to_ref(y).cloned().unwrap()); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `find_map(|y| to_ref(y).cloned().ok())` | note: the suggestion might change the behavior of the program when merging `filter` and `map`, because this expression potentially contains side effects and will only execute once - --> $DIR/manual_find_map.rs:47:45 + --> $DIR/manual_find_map.rs:48:45 | LL | iter::>().find(|&x| to_ref(x).is_ok()).map(|y| to_ref(y).cloned().unwrap()); | ^^^^^^^^^ error: `find(..).map(..)` can be simplified as `find_map(..)` - --> $DIR/manual_find_map.rs:95:10 + --> $DIR/manual_find_map.rs:96:10 | LL | .find(|f| f.option_field.is_some()) | __________^ @@ -197,7 +197,7 @@ LL | | .map(|f| f.option_field.clone().unwrap()); | |_________________________________________________^ help: try: `find_map(|f| f.option_field.clone())` error: `find(..).map(..)` can be simplified as `find_map(..)` - --> $DIR/manual_find_map.rs:100:10 + --> $DIR/manual_find_map.rs:101:10 | LL | .find(|f| f.ref_field.is_some()) | __________^ @@ -205,7 +205,7 @@ LL | | .map(|f| f.ref_field.cloned().unwrap()); | |_______________________________________________^ help: try: `find_map(|f| f.ref_field.cloned())` error: `find(..).map(..)` can be simplified as `find_map(..)` - --> $DIR/manual_find_map.rs:105:10 + --> $DIR/manual_find_map.rs:106:10 | LL | .find(|f| f.ref_field.is_some()) | __________^ @@ -213,7 +213,7 @@ LL | | .map(|f| f.ref_field.copied().unwrap()); | |_______________________________________________^ help: try: `find_map(|f| f.ref_field.copied())` error: `find(..).map(..)` can be simplified as `find_map(..)` - --> $DIR/manual_find_map.rs:110:10 + --> $DIR/manual_find_map.rs:111:10 | LL | .find(|f| f.result_field.is_ok()) | __________^ @@ -221,7 +221,7 @@ LL | | .map(|f| f.result_field.clone().unwrap()); | |_________________________________________________^ help: try: `find_map(|f| f.result_field.clone().ok())` error: `find(..).map(..)` can be simplified as `find_map(..)` - --> $DIR/manual_find_map.rs:115:10 + --> $DIR/manual_find_map.rs:116:10 | LL | .find(|f| f.result_field.is_ok()) | __________^ @@ -229,7 +229,7 @@ LL | | .map(|f| f.result_field.as_ref().unwrap()); | |__________________________________________________^ help: try: `find_map(|f| f.result_field.as_ref().ok())` error: `find(..).map(..)` can be simplified as `find_map(..)` - --> $DIR/manual_find_map.rs:120:10 + --> $DIR/manual_find_map.rs:121:10 | LL | .find(|f| f.result_field.is_ok()) | __________^ @@ -237,7 +237,7 @@ LL | | .map(|f| f.result_field.as_deref().unwrap()); | |____________________________________________________^ help: try: `find_map(|f| f.result_field.as_deref().ok())` error: `find(..).map(..)` can be simplified as `find_map(..)` - --> $DIR/manual_find_map.rs:125:10 + --> $DIR/manual_find_map.rs:126:10 | LL | .find(|f| f.result_field.is_ok()) | __________^ @@ -245,7 +245,7 @@ LL | | .map(|f| f.result_field.as_mut().unwrap()); | |__________________________________________________^ help: try: `find_map(|f| f.result_field.as_mut().ok())` error: `find(..).map(..)` can be simplified as `find_map(..)` - --> $DIR/manual_find_map.rs:130:10 + --> $DIR/manual_find_map.rs:131:10 | LL | .find(|f| f.result_field.is_ok()) | __________^ @@ -253,7 +253,7 @@ LL | | .map(|f| f.result_field.as_deref_mut().unwrap()); | |________________________________________________________^ help: try: `find_map(|f| f.result_field.as_deref_mut().ok())` error: `find(..).map(..)` can be simplified as `find_map(..)` - --> $DIR/manual_find_map.rs:135:10 + --> $DIR/manual_find_map.rs:136:10 | LL | .find(|f| f.result_field.is_ok()) | __________^ diff --git a/tests/ui/manual_is_ascii_check.fixed b/tests/ui/manual_is_ascii_check.fixed index 5be2dd280b821..9c4bd335ad8ba 100644 --- a/tests/ui/manual_is_ascii_check.fixed +++ b/tests/ui/manual_is_ascii_check.fixed @@ -33,6 +33,7 @@ fn msrv_1_23() { assert!(matches!(b'1', b'0'..=b'9')); assert!(matches!('X', 'A'..='Z')); assert!(matches!('x', 'A'..='Z' | 'a'..='z')); + assert!(matches!('x', '0'..='9' | 'a'..='f' | 'A'..='F')); } #[clippy::msrv = "1.24"] @@ -40,14 +41,17 @@ fn msrv_1_24() { assert!(b'1'.is_ascii_digit()); assert!('X'.is_ascii_uppercase()); assert!('x'.is_ascii_alphabetic()); + assert!('x'.is_ascii_hexdigit()); } #[clippy::msrv = "1.46"] fn msrv_1_46() { const FOO: bool = matches!('x', '0'..='9'); + const BAR: bool = matches!('x', '0'..='9' | 'a'..='f' | 'A'..='F'); } #[clippy::msrv = "1.47"] fn msrv_1_47() { const FOO: bool = 'x'.is_ascii_digit(); + const BAR: bool = 'x'.is_ascii_hexdigit(); } diff --git a/tests/ui/manual_is_ascii_check.rs b/tests/ui/manual_is_ascii_check.rs index f9249e22a026d..785943cd24d2c 100644 --- a/tests/ui/manual_is_ascii_check.rs +++ b/tests/ui/manual_is_ascii_check.rs @@ -33,6 +33,7 @@ fn msrv_1_23() { assert!(matches!(b'1', b'0'..=b'9')); assert!(matches!('X', 'A'..='Z')); assert!(matches!('x', 'A'..='Z' | 'a'..='z')); + assert!(matches!('x', '0'..='9' | 'a'..='f' | 'A'..='F')); } #[clippy::msrv = "1.24"] @@ -40,14 +41,17 @@ fn msrv_1_24() { assert!(matches!(b'1', b'0'..=b'9')); assert!(matches!('X', 'A'..='Z')); assert!(matches!('x', 'A'..='Z' | 'a'..='z')); + assert!(matches!('x', '0'..='9' | 'a'..='f' | 'A'..='F')); } #[clippy::msrv = "1.46"] fn msrv_1_46() { const FOO: bool = matches!('x', '0'..='9'); + const BAR: bool = matches!('x', '0'..='9' | 'a'..='f' | 'A'..='F'); } #[clippy::msrv = "1.47"] fn msrv_1_47() { const FOO: bool = matches!('x', '0'..='9'); + const BAR: bool = matches!('x', '0'..='9' | 'a'..='f' | 'A'..='F'); } diff --git a/tests/ui/manual_is_ascii_check.stderr b/tests/ui/manual_is_ascii_check.stderr index e0fb46e59fa14..f69522c5ff84c 100644 --- a/tests/ui/manual_is_ascii_check.stderr +++ b/tests/ui/manual_is_ascii_check.stderr @@ -98,28 +98,40 @@ LL | ('A'..='Z').contains(cool_letter); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `cool_letter.is_ascii_uppercase()` error: manual check for common ascii range - --> $DIR/manual_is_ascii_check.rs:40:13 + --> $DIR/manual_is_ascii_check.rs:41:13 | LL | assert!(matches!(b'1', b'0'..=b'9')); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `b'1'.is_ascii_digit()` error: manual check for common ascii range - --> $DIR/manual_is_ascii_check.rs:41:13 + --> $DIR/manual_is_ascii_check.rs:42:13 | LL | assert!(matches!('X', 'A'..='Z')); | ^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `'X'.is_ascii_uppercase()` error: manual check for common ascii range - --> $DIR/manual_is_ascii_check.rs:42:13 + --> $DIR/manual_is_ascii_check.rs:43:13 | LL | assert!(matches!('x', 'A'..='Z' | 'a'..='z')); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `'x'.is_ascii_alphabetic()` error: manual check for common ascii range - --> $DIR/manual_is_ascii_check.rs:52:23 + --> $DIR/manual_is_ascii_check.rs:44:13 + | +LL | assert!(matches!('x', '0'..='9' | 'a'..='f' | 'A'..='F')); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `'x'.is_ascii_hexdigit()` + +error: manual check for common ascii range + --> $DIR/manual_is_ascii_check.rs:55:23 | LL | const FOO: bool = matches!('x', '0'..='9'); | ^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `'x'.is_ascii_digit()` -error: aborting due to 20 previous errors +error: manual check for common ascii range + --> $DIR/manual_is_ascii_check.rs:56:23 + | +LL | const BAR: bool = matches!('x', '0'..='9' | 'a'..='f' | 'A'..='F'); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `'x'.is_ascii_hexdigit()` + +error: aborting due to 22 previous errors diff --git a/tests/ui/map_identity.fixed b/tests/ui/map_identity.fixed index cc40b1620587c..e756d9b593542 100644 --- a/tests/ui/map_identity.fixed +++ b/tests/ui/map_identity.fixed @@ -17,6 +17,9 @@ fn main() { }); let _: Result = Ok(1); let _: Result = Ok(1).map_err(|a: u32| a * 42); + // : u32 guides type inference + let _ = Ok(1).map_err(|a: u32| a); + let _ = Ok(1).map_err(std::convert::identity::); } fn not_identity(x: &u16) -> u16 { diff --git a/tests/ui/map_identity.rs b/tests/ui/map_identity.rs index 97a91aea6dc4e..74cbaade40549 100644 --- a/tests/ui/map_identity.rs +++ b/tests/ui/map_identity.rs @@ -19,6 +19,9 @@ fn main() { }); let _: Result = Ok(1).map_err(|a| a); let _: Result = Ok(1).map_err(|a: u32| a * 42); + // : u32 guides type inference + let _ = Ok(1).map_err(|a: u32| a); + let _ = Ok(1).map_err(std::convert::identity::); } fn not_identity(x: &u16) -> u16 { diff --git a/tests/ui/min_ident_chars.rs b/tests/ui/min_ident_chars.rs index 030863ca0d304..f99c35d5c57c4 100644 --- a/tests/ui/min_ident_chars.rs +++ b/tests/ui/min_ident_chars.rs @@ -1,5 +1,6 @@ //@aux-build:proc_macros.rs #![allow(irrefutable_let_patterns, nonstandard_style, unused)] +#![allow(clippy::struct_field_names)] #![warn(clippy::min_ident_chars)] extern crate proc_macros; diff --git a/tests/ui/min_ident_chars.stderr b/tests/ui/min_ident_chars.stderr index 253636cf91da1..e4181157ea23b 100644 --- a/tests/ui/min_ident_chars.stderr +++ b/tests/ui/min_ident_chars.stderr @@ -1,5 +1,5 @@ error: this ident consists of a single char - --> $DIR/min_ident_chars.rs:8:8 + --> $DIR/min_ident_chars.rs:9:8 | LL | struct A { | ^ @@ -8,169 +8,169 @@ LL | struct A { = help: to override `-D warnings` add `#[allow(clippy::min_ident_chars)]` error: this ident consists of a single char - --> $DIR/min_ident_chars.rs:9:5 + --> $DIR/min_ident_chars.rs:10:5 | LL | a: u32, | ^ error: this ident consists of a single char - --> $DIR/min_ident_chars.rs:11:5 + --> $DIR/min_ident_chars.rs:12:5 | LL | A: u32, | ^ error: this ident consists of a single char - --> $DIR/min_ident_chars.rs:12:5 + --> $DIR/min_ident_chars.rs:13:5 | LL | I: u32, | ^ error: this ident consists of a single char - --> $DIR/min_ident_chars.rs:15:8 + --> $DIR/min_ident_chars.rs:16:8 | LL | struct B(u32); | ^ error: this ident consists of a single char - --> $DIR/min_ident_chars.rs:17:8 + --> $DIR/min_ident_chars.rs:18:8 | LL | struct O { | ^ error: this ident consists of a single char - --> $DIR/min_ident_chars.rs:18:5 + --> $DIR/min_ident_chars.rs:19:5 | LL | o: u32, | ^ error: this ident consists of a single char - --> $DIR/min_ident_chars.rs:23:6 + --> $DIR/min_ident_chars.rs:24:6 | LL | enum C { | ^ error: this ident consists of a single char - --> $DIR/min_ident_chars.rs:24:5 + --> $DIR/min_ident_chars.rs:25:5 | LL | D, | ^ error: this ident consists of a single char - --> $DIR/min_ident_chars.rs:25:5 + --> $DIR/min_ident_chars.rs:26:5 | LL | E, | ^ error: this ident consists of a single char - --> $DIR/min_ident_chars.rs:26:5 + --> $DIR/min_ident_chars.rs:27:5 | LL | F, | ^ error: this ident consists of a single char - --> $DIR/min_ident_chars.rs:50:9 + --> $DIR/min_ident_chars.rs:51:9 | LL | let h = 1; | ^ error: this ident consists of a single char - --> $DIR/min_ident_chars.rs:51:9 + --> $DIR/min_ident_chars.rs:52:9 | LL | let e = 2; | ^ error: this ident consists of a single char - --> $DIR/min_ident_chars.rs:52:9 + --> $DIR/min_ident_chars.rs:53:9 | LL | let l = 3; | ^ error: this ident consists of a single char - --> $DIR/min_ident_chars.rs:53:9 + --> $DIR/min_ident_chars.rs:54:9 | LL | let l = 4; | ^ error: this ident consists of a single char - --> $DIR/min_ident_chars.rs:54:9 + --> $DIR/min_ident_chars.rs:55:9 | LL | let o = 6; | ^ error: this ident consists of a single char - --> $DIR/min_ident_chars.rs:58:10 + --> $DIR/min_ident_chars.rs:59:10 | LL | let (h, o, w) = (1, 2, 3); | ^ error: this ident consists of a single char - --> $DIR/min_ident_chars.rs:58:13 + --> $DIR/min_ident_chars.rs:59:13 | LL | let (h, o, w) = (1, 2, 3); | ^ error: this ident consists of a single char - --> $DIR/min_ident_chars.rs:59:10 + --> $DIR/min_ident_chars.rs:60:10 | LL | for (a, (r, e)) in (0..1000).enumerate().enumerate() {} | ^ error: this ident consists of a single char - --> $DIR/min_ident_chars.rs:59:14 + --> $DIR/min_ident_chars.rs:60:14 | LL | for (a, (r, e)) in (0..1000).enumerate().enumerate() {} | ^ error: this ident consists of a single char - --> $DIR/min_ident_chars.rs:59:17 + --> $DIR/min_ident_chars.rs:60:17 | LL | for (a, (r, e)) in (0..1000).enumerate().enumerate() {} | ^ error: this ident consists of a single char - --> $DIR/min_ident_chars.rs:61:16 + --> $DIR/min_ident_chars.rs:62:16 | LL | while let (d, o, _i, n, g) = (true, true, false, false, true) {} | ^ error: this ident consists of a single char - --> $DIR/min_ident_chars.rs:61:19 + --> $DIR/min_ident_chars.rs:62:19 | LL | while let (d, o, _i, n, g) = (true, true, false, false, true) {} | ^ error: this ident consists of a single char - --> $DIR/min_ident_chars.rs:61:29 + --> $DIR/min_ident_chars.rs:62:29 | LL | while let (d, o, _i, n, g) = (true, true, false, false, true) {} | ^ error: this ident consists of a single char - --> $DIR/min_ident_chars.rs:65:9 + --> $DIR/min_ident_chars.rs:66:9 | LL | let o = 1; | ^ error: this ident consists of a single char - --> $DIR/min_ident_chars.rs:66:9 + --> $DIR/min_ident_chars.rs:67:9 | LL | let o = O { o }; | ^ error: this ident consists of a single char - --> $DIR/min_ident_chars.rs:80:4 + --> $DIR/min_ident_chars.rs:81:4 | LL | fn b() {} | ^ error: this ident consists of a single char - --> $DIR/min_ident_chars.rs:81:21 + --> $DIR/min_ident_chars.rs:82:21 | LL | fn wrong_pythagoras(a: f32, b: f32) -> f32 { | ^ error: this ident consists of a single char - --> $DIR/min_ident_chars.rs:81:29 + --> $DIR/min_ident_chars.rs:82:29 | LL | fn wrong_pythagoras(a: f32, b: f32) -> f32 { | ^ diff --git a/tests/ui/misnamed_getters.fixed b/tests/ui/misnamed_getters.fixed index 2a7a2067ee083..70af604b21448 100644 --- a/tests/ui/misnamed_getters.fixed +++ b/tests/ui/misnamed_getters.fixed @@ -1,4 +1,5 @@ #![allow(unused)] +#![allow(clippy::struct_field_names)] #![warn(clippy::misnamed_getters)] struct A { diff --git a/tests/ui/misnamed_getters.rs b/tests/ui/misnamed_getters.rs index 56ddc46c4d4a7..23c3e7bc5cf7e 100644 --- a/tests/ui/misnamed_getters.rs +++ b/tests/ui/misnamed_getters.rs @@ -1,4 +1,5 @@ #![allow(unused)] +#![allow(clippy::struct_field_names)] #![warn(clippy::misnamed_getters)] struct A { diff --git a/tests/ui/misnamed_getters.stderr b/tests/ui/misnamed_getters.stderr index aadec65490836..120a3f3112ef1 100644 --- a/tests/ui/misnamed_getters.stderr +++ b/tests/ui/misnamed_getters.stderr @@ -1,5 +1,5 @@ error: getter function appears to return the wrong field - --> $DIR/misnamed_getters.rs:11:5 + --> $DIR/misnamed_getters.rs:12:5 | LL | / fn a(&self) -> &u8 { LL | | @@ -13,7 +13,7 @@ LL | | } = help: to override `-D warnings` add `#[allow(clippy::misnamed_getters)]` error: getter function appears to return the wrong field - --> $DIR/misnamed_getters.rs:16:5 + --> $DIR/misnamed_getters.rs:17:5 | LL | / fn a_mut(&mut self) -> &mut u8 { LL | | @@ -23,7 +23,7 @@ LL | | } | |_____^ error: getter function appears to return the wrong field - --> $DIR/misnamed_getters.rs:21:5 + --> $DIR/misnamed_getters.rs:22:5 | LL | / fn b(self) -> u8 { LL | | @@ -33,7 +33,7 @@ LL | | } | |_____^ error: getter function appears to return the wrong field - --> $DIR/misnamed_getters.rs:26:5 + --> $DIR/misnamed_getters.rs:27:5 | LL | / fn b_mut(&mut self) -> &mut u8 { LL | | @@ -43,7 +43,7 @@ LL | | } | |_____^ error: getter function appears to return the wrong field - --> $DIR/misnamed_getters.rs:31:5 + --> $DIR/misnamed_getters.rs:32:5 | LL | / fn c(&self) -> &u8 { LL | | @@ -53,7 +53,7 @@ LL | | } | |_____^ error: getter function appears to return the wrong field - --> $DIR/misnamed_getters.rs:36:5 + --> $DIR/misnamed_getters.rs:37:5 | LL | / fn c_mut(&mut self) -> &mut u8 { LL | | @@ -63,7 +63,7 @@ LL | | } | |_____^ error: getter function appears to return the wrong field - --> $DIR/misnamed_getters.rs:48:5 + --> $DIR/misnamed_getters.rs:49:5 | LL | / unsafe fn a(&self) -> &u8 { LL | | @@ -73,7 +73,7 @@ LL | | } | |_____^ error: getter function appears to return the wrong field - --> $DIR/misnamed_getters.rs:52:5 + --> $DIR/misnamed_getters.rs:53:5 | LL | / unsafe fn a_mut(&mut self) -> &mut u8 { LL | | @@ -83,7 +83,7 @@ LL | | } | |_____^ error: getter function appears to return the wrong field - --> $DIR/misnamed_getters.rs:57:5 + --> $DIR/misnamed_getters.rs:58:5 | LL | / unsafe fn b(self) -> u8 { LL | | @@ -93,7 +93,7 @@ LL | | } | |_____^ error: getter function appears to return the wrong field - --> $DIR/misnamed_getters.rs:62:5 + --> $DIR/misnamed_getters.rs:63:5 | LL | / unsafe fn b_mut(&mut self) -> &mut u8 { LL | | @@ -103,7 +103,7 @@ LL | | } | |_____^ error: getter function appears to return the wrong field - --> $DIR/misnamed_getters.rs:75:5 + --> $DIR/misnamed_getters.rs:76:5 | LL | / unsafe fn a_unchecked(&self) -> &u8 { LL | | @@ -113,7 +113,7 @@ LL | | } | |_____^ error: getter function appears to return the wrong field - --> $DIR/misnamed_getters.rs:79:5 + --> $DIR/misnamed_getters.rs:80:5 | LL | / unsafe fn a_unchecked_mut(&mut self) -> &mut u8 { LL | | @@ -123,7 +123,7 @@ LL | | } | |_____^ error: getter function appears to return the wrong field - --> $DIR/misnamed_getters.rs:84:5 + --> $DIR/misnamed_getters.rs:85:5 | LL | / unsafe fn b_unchecked(self) -> u8 { LL | | @@ -133,7 +133,7 @@ LL | | } | |_____^ error: getter function appears to return the wrong field - --> $DIR/misnamed_getters.rs:89:5 + --> $DIR/misnamed_getters.rs:90:5 | LL | / unsafe fn b_unchecked_mut(&mut self) -> &mut u8 { LL | | @@ -143,7 +143,7 @@ LL | | } | |_____^ error: getter function appears to return the wrong field - --> $DIR/misnamed_getters.rs:122:5 + --> $DIR/misnamed_getters.rs:123:5 | LL | / fn a(&self) -> &u8 { LL | | @@ -153,7 +153,7 @@ LL | | } | |_____^ error: getter function appears to return the wrong field - --> $DIR/misnamed_getters.rs:126:5 + --> $DIR/misnamed_getters.rs:127:5 | LL | / fn a_mut(&mut self) -> &mut u8 { LL | | @@ -163,7 +163,7 @@ LL | | } | |_____^ error: getter function appears to return the wrong field - --> $DIR/misnamed_getters.rs:131:5 + --> $DIR/misnamed_getters.rs:132:5 | LL | / fn d(&self) -> &u8 { LL | | @@ -173,7 +173,7 @@ LL | | } | |_____^ error: getter function appears to return the wrong field - --> $DIR/misnamed_getters.rs:135:5 + --> $DIR/misnamed_getters.rs:136:5 | LL | / fn d_mut(&mut self) -> &mut u8 { LL | | diff --git a/tests/ui/multiple_unsafe_ops_per_block.rs b/tests/ui/multiple_unsafe_ops_per_block.rs index 4ef6f0ca92f2d..8afb4df20af4b 100644 --- a/tests/ui/multiple_unsafe_ops_per_block.rs +++ b/tests/ui/multiple_unsafe_ops_per_block.rs @@ -147,4 +147,11 @@ fn _field_fn_ptr(x: unsafe fn()) { } } +// await expands to an unsafe block with several operations, but this is fine.: #11312 +async fn await_desugaring_silent() { + async fn helper() {} + + helper().await; +} + fn main() {} diff --git a/tests/ui/needless_bool/fixable.fixed b/tests/ui/needless_bool/fixable.fixed index c9ea831f8b2a3..3059de8f89c49 100644 --- a/tests/ui/needless_bool/fixable.fixed +++ b/tests/ui/needless_bool/fixable.fixed @@ -7,7 +7,8 @@ clippy::equatable_if_let, clippy::needless_if, clippy::needless_return, - clippy::self_named_constructors + clippy::self_named_constructors, + clippy::struct_field_names )] use std::cell::Cell; diff --git a/tests/ui/needless_bool/fixable.rs b/tests/ui/needless_bool/fixable.rs index b83d9c3f2096a..b2cbe86e2235b 100644 --- a/tests/ui/needless_bool/fixable.rs +++ b/tests/ui/needless_bool/fixable.rs @@ -7,7 +7,8 @@ clippy::equatable_if_let, clippy::needless_if, clippy::needless_return, - clippy::self_named_constructors + clippy::self_named_constructors, + clippy::struct_field_names )] use std::cell::Cell; diff --git a/tests/ui/needless_bool/fixable.stderr b/tests/ui/needless_bool/fixable.stderr index 2b189c898512d..72b0670c95bae 100644 --- a/tests/ui/needless_bool/fixable.stderr +++ b/tests/ui/needless_bool/fixable.stderr @@ -1,5 +1,5 @@ error: this if-then-else expression returns a bool literal - --> $DIR/fixable.rs:40:5 + --> $DIR/fixable.rs:41:5 | LL | / if x { LL | | true @@ -12,7 +12,7 @@ LL | | }; = help: to override `-D warnings` add `#[allow(clippy::needless_bool)]` error: this if-then-else expression returns a bool literal - --> $DIR/fixable.rs:45:5 + --> $DIR/fixable.rs:46:5 | LL | / if x { LL | | false @@ -22,7 +22,7 @@ LL | | }; | |_____^ help: you can reduce it to: `!x` error: this if-then-else expression returns a bool literal - --> $DIR/fixable.rs:50:5 + --> $DIR/fixable.rs:51:5 | LL | / if x && y { LL | | false @@ -32,7 +32,7 @@ LL | | }; | |_____^ help: you can reduce it to: `!(x && y)` error: this if-then-else expression returns a bool literal - --> $DIR/fixable.rs:58:5 + --> $DIR/fixable.rs:59:5 | LL | / if a == b { LL | | false @@ -42,7 +42,7 @@ LL | | }; | |_____^ help: you can reduce it to: `a != b` error: this if-then-else expression returns a bool literal - --> $DIR/fixable.rs:63:5 + --> $DIR/fixable.rs:64:5 | LL | / if a != b { LL | | false @@ -52,7 +52,7 @@ LL | | }; | |_____^ help: you can reduce it to: `a == b` error: this if-then-else expression returns a bool literal - --> $DIR/fixable.rs:68:5 + --> $DIR/fixable.rs:69:5 | LL | / if a < b { LL | | false @@ -62,7 +62,7 @@ LL | | }; | |_____^ help: you can reduce it to: `a >= b` error: this if-then-else expression returns a bool literal - --> $DIR/fixable.rs:73:5 + --> $DIR/fixable.rs:74:5 | LL | / if a <= b { LL | | false @@ -72,7 +72,7 @@ LL | | }; | |_____^ help: you can reduce it to: `a > b` error: this if-then-else expression returns a bool literal - --> $DIR/fixable.rs:78:5 + --> $DIR/fixable.rs:79:5 | LL | / if a > b { LL | | false @@ -82,7 +82,7 @@ LL | | }; | |_____^ help: you can reduce it to: `a <= b` error: this if-then-else expression returns a bool literal - --> $DIR/fixable.rs:83:5 + --> $DIR/fixable.rs:84:5 | LL | / if a >= b { LL | | false @@ -92,7 +92,7 @@ LL | | }; | |_____^ help: you can reduce it to: `a < b` error: this if-then-else expression returns a bool literal - --> $DIR/fixable.rs:111:5 + --> $DIR/fixable.rs:112:5 | LL | / if x { LL | | return true; @@ -102,7 +102,7 @@ LL | | }; | |_____^ help: you can reduce it to: `return x` error: this if-then-else expression returns a bool literal - --> $DIR/fixable.rs:119:5 + --> $DIR/fixable.rs:120:5 | LL | / if x { LL | | return false; @@ -112,7 +112,7 @@ LL | | }; | |_____^ help: you can reduce it to: `return !x` error: this if-then-else expression returns a bool literal - --> $DIR/fixable.rs:127:5 + --> $DIR/fixable.rs:128:5 | LL | / if x && y { LL | | return true; @@ -122,7 +122,7 @@ LL | | }; | |_____^ help: you can reduce it to: `return x && y` error: this if-then-else expression returns a bool literal - --> $DIR/fixable.rs:135:5 + --> $DIR/fixable.rs:136:5 | LL | / if x && y { LL | | return false; @@ -132,7 +132,7 @@ LL | | }; | |_____^ help: you can reduce it to: `return !(x && y)` error: equality checks against true are unnecessary - --> $DIR/fixable.rs:143:8 + --> $DIR/fixable.rs:144:8 | LL | if x == true {}; | ^^^^^^^^^ help: try simplifying it as shown: `x` @@ -141,25 +141,25 @@ LL | if x == true {}; = help: to override `-D warnings` add `#[allow(clippy::bool_comparison)]` error: equality checks against false can be replaced by a negation - --> $DIR/fixable.rs:147:8 + --> $DIR/fixable.rs:148:8 | LL | if x == false {}; | ^^^^^^^^^^ help: try simplifying it as shown: `!x` error: equality checks against true are unnecessary - --> $DIR/fixable.rs:157:8 + --> $DIR/fixable.rs:158:8 | LL | if x == true {}; | ^^^^^^^^^ help: try simplifying it as shown: `x` error: equality checks against false can be replaced by a negation - --> $DIR/fixable.rs:158:8 + --> $DIR/fixable.rs:159:8 | LL | if x == false {}; | ^^^^^^^^^^ help: try simplifying it as shown: `!x` error: this if-then-else expression returns a bool literal - --> $DIR/fixable.rs:167:12 + --> $DIR/fixable.rs:168:12 | LL | } else if returns_bool() { | ____________^ @@ -170,7 +170,7 @@ LL | | }; | |_____^ help: you can reduce it to: `{ !returns_bool() }` error: this if-then-else expression returns a bool literal - --> $DIR/fixable.rs:180:5 + --> $DIR/fixable.rs:181:5 | LL | / if unsafe { no(4) } & 1 != 0 { LL | | true @@ -180,13 +180,13 @@ LL | | }; | |_____^ help: you can reduce it to: `(unsafe { no(4) } & 1 != 0)` error: this if-then-else expression returns a bool literal - --> $DIR/fixable.rs:185:30 + --> $DIR/fixable.rs:186:30 | LL | let _brackets_unneeded = if unsafe { no(4) } & 1 != 0 { true } else { false }; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: you can reduce it to: `unsafe { no(4) } & 1 != 0` error: this if-then-else expression returns a bool literal - --> $DIR/fixable.rs:188:9 + --> $DIR/fixable.rs:189:9 | LL | if unsafe { no(4) } & 1 != 0 { true } else { false } | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: you can reduce it to: `(unsafe { no(4) } & 1 != 0)` diff --git a/tests/ui/needless_pass_by_ref_mut.rs b/tests/ui/needless_pass_by_ref_mut.rs index 39d76f9990022..ea5e74c4c0067 100644 --- a/tests/ui/needless_pass_by_ref_mut.rs +++ b/tests/ui/needless_pass_by_ref_mut.rs @@ -270,6 +270,38 @@ pub async fn closure4(n: &mut usize) { })(); } +// Should not warn. +async fn _f(v: &mut Vec<()>) { + let x = || v.pop(); + _ = || || x; +} + +struct Data { + value: T, +} +// Unsafe functions should not warn. +unsafe fn get_mut_unchecked(ptr: &mut NonNull>) -> &mut T { + &mut (*ptr.as_ptr()).value +} +// Unsafe blocks should not warn. +fn get_mut_unchecked2(ptr: &mut NonNull>) -> &mut T { + unsafe { &mut (*ptr.as_ptr()).value } +} + +fn set_true(b: &mut bool) { + *b = true; +} + +// Should not warn. +fn true_setter(b: &mut bool) -> impl FnOnce() + '_ { + move || set_true(b) +} + +// Should not warn. +fn filter_copy(predicate: &mut impl FnMut(T) -> bool) -> impl FnMut(&T) -> bool + '_ { + move |&item| predicate(item) +} + fn main() { let mut u = 0; let mut v = vec![0]; diff --git a/tests/ui/redundant_locals.rs b/tests/ui/redundant_locals.rs index e81db300f15a2..182d067a5e9fa 100644 --- a/tests/ui/redundant_locals.rs +++ b/tests/ui/redundant_locals.rs @@ -117,6 +117,14 @@ fn macros() { let x = 1; let x = x; } + + let x = 10; + macro_rules! rebind_outer_macro { + ($x:ident) => { + let x = x; + }; + } + rebind_outer_macro!(y); } struct WithDrop(usize); diff --git a/tests/ui/redundant_locals.stderr b/tests/ui/redundant_locals.stderr index d794a87fe7df6..30ab4aa2ea915 100644 --- a/tests/ui/redundant_locals.stderr +++ b/tests/ui/redundant_locals.stderr @@ -157,13 +157,13 @@ LL | let x = 1; | ^ error: redundant redefinition of a binding `a` - --> $DIR/redundant_locals.rs:144:5 + --> $DIR/redundant_locals.rs:152:5 | LL | let a = a; | ^^^^^^^^^^ | help: `a` is initially defined here - --> $DIR/redundant_locals.rs:142:9 + --> $DIR/redundant_locals.rs:150:9 | LL | let a = WithoutDrop(1); | ^ diff --git a/tests/ui/rest_pat_in_fully_bound_structs.rs b/tests/ui/rest_pat_in_fully_bound_structs.rs index e25609f756017..51fe346d09264 100644 --- a/tests/ui/rest_pat_in_fully_bound_structs.rs +++ b/tests/ui/rest_pat_in_fully_bound_structs.rs @@ -1,4 +1,5 @@ #![warn(clippy::rest_pat_in_fully_bound_structs)] +#![allow(clippy::struct_field_names)] struct A { a: i32, diff --git a/tests/ui/rest_pat_in_fully_bound_structs.stderr b/tests/ui/rest_pat_in_fully_bound_structs.stderr index 2c221b4dbb2cc..a62f1d0b65f3b 100644 --- a/tests/ui/rest_pat_in_fully_bound_structs.stderr +++ b/tests/ui/rest_pat_in_fully_bound_structs.stderr @@ -1,5 +1,5 @@ error: unnecessary use of `..` pattern in struct binding. All fields were already bound - --> $DIR/rest_pat_in_fully_bound_structs.rs:22:9 + --> $DIR/rest_pat_in_fully_bound_structs.rs:23:9 | LL | A { a: 5, b: 42, c: "", .. } => {}, // Lint | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -9,7 +9,7 @@ LL | A { a: 5, b: 42, c: "", .. } => {}, // Lint = help: to override `-D warnings` add `#[allow(clippy::rest_pat_in_fully_bound_structs)]` error: unnecessary use of `..` pattern in struct binding. All fields were already bound - --> $DIR/rest_pat_in_fully_bound_structs.rs:24:9 + --> $DIR/rest_pat_in_fully_bound_structs.rs:25:9 | LL | A { a: 0, b: 0, c: "", .. } => {}, // Lint | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -17,7 +17,7 @@ LL | A { a: 0, b: 0, c: "", .. } => {}, // Lint = help: consider removing `..` from this binding error: unnecessary use of `..` pattern in struct binding. All fields were already bound - --> $DIR/rest_pat_in_fully_bound_structs.rs:31:9 + --> $DIR/rest_pat_in_fully_bound_structs.rs:32:9 | LL | A { a: 0, b: 0, c: "", .. } => {}, // Lint | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ diff --git a/tests/ui/struct_fields.rs b/tests/ui/struct_fields.rs new file mode 100644 index 0000000000000..8b1a1446e3c85 --- /dev/null +++ b/tests/ui/struct_fields.rs @@ -0,0 +1,331 @@ +//@aux-build:proc_macros.rs + +#![warn(clippy::struct_field_names)] +#![allow(unused)] + +#[macro_use] +extern crate proc_macros; + +struct Data1 { + field_data1: u8, + //~^ ERROR: field name ends with the struct's name + another: u8, + foo: u8, + bar: u8, +} + +struct Data2 { + another: u8, + foo: u8, + data2_field: u8, + //~^ ERROR: field name starts with the struct's name + bar: u8, +} + +struct StructData { + //~^ ERROR: all fields have the same postfix: `data` + movable_data: u8, + fixed_data: u8, + invisible_data: u8, +} + +struct DataStruct { + //~^ ERROR: all fields have the same prefix: `data` + data_movable: u8, + data_fixed: u8, + data_invisible: u8, +} + +struct DoublePrefix { + //~^ ERROR: all fields have the same prefix: `some_data` + some_data_a: bool, + some_data_b: bool, + some_data_c: bool, +} + +struct DoublePostfix { + //~^ ERROR: all fields have the same postfix: `some_data` + a_some_data: bool, + b_some_data: bool, + c_some_data: bool, +} + +#[allow(non_snake_case)] +struct NotSnakeCase { + //~^ ERROR: all fields have the same postfix: `someData` + a_someData: bool, + b_someData: bool, + c_someData: bool, +} +#[allow(non_snake_case)] +struct NotSnakeCase2 { + //~^ ERROR: all fields have the same prefix: `someData` + someData_c: bool, + someData_b: bool, + someData_a_b: bool, +} + +// no error, threshold is 3 fiels by default +struct Fooo { + foo: u8, + bar: u8, +} + +struct NonCaps { + //~^ ERROR: all fields have the same prefix: `prefix` + prefix_的: u8, + prefix_tea: u8, + prefix_cake: u8, +} + +// should not lint +#[allow(clippy::struct_field_names)] +pub mod allowed { + pub struct PubAllowed { + some_this: u8, + some_that: u8, + some_other_what: u8, + } +} + +// should not lint +struct SomeData { + foo: u8, + bar: bool, + path: u8, + answer: u8, +} + +// should not lint +pub struct NetworkLayer { + layer1: Vec, + layer2: Vec, + layer3: Vec, + layer4: Vec, +} + +//should not lint +struct North { + normal: u8, + no_left: u8, + no_right: u8, +} + +mod issue8324_from_enum_variant_names { + // 8324: enum_variant_names warns even if removing the suffix would leave an empty string + struct Phase { + pre_lookup: u8, + lookup: u8, + post_lookup: u8, + } +} + +mod issue9018_from_enum_variant_names { + struct DoLint { + //~^ ERROR: all fields have the same prefix: `_type` + _type_create: u8, + _type_read: u8, + _type_update: u8, + _type_destroy: u8, + } + + struct DoLint2 { + //~^ ERROR: all fields have the same prefix: `__type` + __type_create: u8, + __type_read: u8, + __type_update: u8, + __type_destroy: u8, + } + + struct DoLint3 { + //~^ ERROR: all fields have the same prefix: `___type` + ___type_create: u8, + ___type_read: u8, + ___type_update: u8, + ___type_destroy: u8, + } + + struct DoLint4 { + //~^ ERROR: all fields have the same postfix: `_` + create_: u8, + read_: u8, + update_: u8, + destroy_: u8, + } + + struct DoLint5 { + //~^ ERROR: all fields have the same postfix: `__` + create__: u8, + read__: u8, + update__: u8, + destroy__: u8, + } + + struct DoLint6 { + //~^ ERROR: all fields have the same postfix: `___` + create___: u8, + read___: u8, + update___: u8, + destroy___: u8, + } + + struct DoLintToo { + //~^ ERROR: all fields have the same postfix: `type` + _create_type: u8, + _update_type: u8, + _delete_type: u8, + } + + struct DoNotLint { + _foo: u8, + _bar: u8, + _baz: u8, + } + + struct DoNotLint2 { + __foo: u8, + __bar: u8, + __baz: u8, + } +} + +mod allow_attributes_on_fields { + struct Struct { + #[allow(clippy::struct_field_names)] + struct_starts_with: u8, + #[allow(clippy::struct_field_names)] + ends_with_struct: u8, + foo: u8, + } +} + +// food field should not lint +struct Foo { + food: i32, + a: i32, + b: i32, +} + +struct Proxy { + proxy: i32, + //~^ ERROR: field name starts with the struct's name + unrelated1: bool, + unrelated2: bool, +} + +// should not lint +pub struct RegexT { + __buffer: i32, + __allocated: i32, + __used: i32, +} + +mod macro_tests { + macro_rules! mk_struct { + () => { + struct MacroStruct { + some_a: i32, + some_b: i32, + some_c: i32, + } + }; + } + mk_struct!(); + //~^ ERROR: all fields have the same prefix: `some` + + macro_rules! mk_struct2 { + () => { + struct Macrobaz { + macrobaz_a: i32, + some_b: i32, + some_c: i32, + } + }; + } + mk_struct2!(); + //~^ ERROR: field name starts with the struct's name + + macro_rules! mk_struct_with_names { + ($struct_name:ident, $field:ident) => { + struct $struct_name { + $field: i32, + other_something: i32, + other_field: i32, + } + }; + } + // expands to `struct Foo { foo: i32, ... }` + mk_struct_with_names!(Foo, foo); + //~^ ERROR: field name starts with the struct's name + + // expands to a struct with all fields starting with `other` but should not + // be linted because some fields come from the macro definition and the other from the input + mk_struct_with_names!(Some, other_data); + + // should not lint when names come from different places + macro_rules! mk_struct_with_field_name { + ($field_name:ident) => { + struct Baz { + one: i32, + two: i32, + $field_name: i32, + } + }; + } + mk_struct_with_field_name!(baz_three); + + // should not lint when names come from different places + macro_rules! mk_struct_with_field_name { + ($field_name:ident) => { + struct Bazilisk { + baz_one: i32, + baz_two: i32, + $field_name: i32, + } + }; + } + mk_struct_with_field_name!(baz_three); + + macro_rules! mk_struct_full_def { + ($struct_name:ident, $field1:ident, $field2:ident, $field3:ident) => { + struct $struct_name { + $field1: i32, + $field2: i32, + $field3: i32, + } + }; + } + mk_struct_full_def!(PrefixData, some_data, some_meta, some_other); + //~^ ERROR: all fields have the same prefix: `some` +} + +// should not lint on external code +external! { + struct DataExternal { + field_data1: u8, + another: u8, + foo: u8, + bar: u8, + } + + struct NotSnakeCaseExternal { + someData_c: bool, + someData_b: bool, + someData_a_b: bool, + } + + struct DoublePrefixExternal { + some_data_a: bool, + some_data_b: bool, + some_data_c: bool, + } + + struct StructDataExternal { + movable_data: u8, + fixed_data: u8, + invisible_data: u8, + } + +} + +fn main() {} diff --git a/tests/ui/struct_fields.stderr b/tests/ui/struct_fields.stderr new file mode 100644 index 0000000000000..4ca57715b1856 --- /dev/null +++ b/tests/ui/struct_fields.stderr @@ -0,0 +1,265 @@ +error: field name ends with the struct's name + --> $DIR/struct_fields.rs:10:5 + | +LL | field_data1: u8, + | ^^^^^^^^^^^^^^^ + | + = note: `-D clippy::struct-field-names` implied by `-D warnings` + = help: to override `-D warnings` add `#[allow(clippy::struct_field_names)]` + +error: field name starts with the struct's name + --> $DIR/struct_fields.rs:20:5 + | +LL | data2_field: u8, + | ^^^^^^^^^^^^^^^ + +error: all fields have the same postfix: `data` + --> $DIR/struct_fields.rs:25:1 + | +LL | / struct StructData { +LL | | +LL | | movable_data: u8, +LL | | fixed_data: u8, +LL | | invisible_data: u8, +LL | | } + | |_^ + | + = help: remove the postfixes + +error: all fields have the same prefix: `data` + --> $DIR/struct_fields.rs:32:1 + | +LL | / struct DataStruct { +LL | | +LL | | data_movable: u8, +LL | | data_fixed: u8, +LL | | data_invisible: u8, +LL | | } + | |_^ + | + = help: remove the prefixes + +error: all fields have the same prefix: `some_data` + --> $DIR/struct_fields.rs:39:1 + | +LL | / struct DoublePrefix { +LL | | +LL | | some_data_a: bool, +LL | | some_data_b: bool, +LL | | some_data_c: bool, +LL | | } + | |_^ + | + = help: remove the prefixes + +error: all fields have the same postfix: `some_data` + --> $DIR/struct_fields.rs:46:1 + | +LL | / struct DoublePostfix { +LL | | +LL | | a_some_data: bool, +LL | | b_some_data: bool, +LL | | c_some_data: bool, +LL | | } + | |_^ + | + = help: remove the postfixes + +error: all fields have the same postfix: `someData` + --> $DIR/struct_fields.rs:54:1 + | +LL | / struct NotSnakeCase { +LL | | +LL | | a_someData: bool, +LL | | b_someData: bool, +LL | | c_someData: bool, +LL | | } + | |_^ + | + = help: remove the postfixes + +error: all fields have the same prefix: `someData` + --> $DIR/struct_fields.rs:61:1 + | +LL | / struct NotSnakeCase2 { +LL | | +LL | | someData_c: bool, +LL | | someData_b: bool, +LL | | someData_a_b: bool, +LL | | } + | |_^ + | + = help: remove the prefixes + +error: all fields have the same prefix: `prefix` + --> $DIR/struct_fields.rs:74:1 + | +LL | / struct NonCaps { +LL | | +LL | | prefix_的: u8, +LL | | prefix_tea: u8, +LL | | prefix_cake: u8, +LL | | } + | |_^ + | + = help: remove the prefixes + +error: all fields have the same prefix: `_type` + --> $DIR/struct_fields.rs:124:5 + | +LL | / struct DoLint { +LL | | +LL | | _type_create: u8, +LL | | _type_read: u8, +LL | | _type_update: u8, +LL | | _type_destroy: u8, +LL | | } + | |_____^ + | + = help: remove the prefixes + +error: all fields have the same prefix: `__type` + --> $DIR/struct_fields.rs:132:5 + | +LL | / struct DoLint2 { +LL | | +LL | | __type_create: u8, +LL | | __type_read: u8, +LL | | __type_update: u8, +LL | | __type_destroy: u8, +LL | | } + | |_____^ + | + = help: remove the prefixes + +error: all fields have the same prefix: `___type` + --> $DIR/struct_fields.rs:140:5 + | +LL | / struct DoLint3 { +LL | | +LL | | ___type_create: u8, +LL | | ___type_read: u8, +LL | | ___type_update: u8, +LL | | ___type_destroy: u8, +LL | | } + | |_____^ + | + = help: remove the prefixes + +error: all fields have the same postfix: `_` + --> $DIR/struct_fields.rs:148:5 + | +LL | / struct DoLint4 { +LL | | +LL | | create_: u8, +LL | | read_: u8, +LL | | update_: u8, +LL | | destroy_: u8, +LL | | } + | |_____^ + | + = help: remove the postfixes + +error: all fields have the same postfix: `__` + --> $DIR/struct_fields.rs:156:5 + | +LL | / struct DoLint5 { +LL | | +LL | | create__: u8, +LL | | read__: u8, +LL | | update__: u8, +LL | | destroy__: u8, +LL | | } + | |_____^ + | + = help: remove the postfixes + +error: all fields have the same postfix: `___` + --> $DIR/struct_fields.rs:164:5 + | +LL | / struct DoLint6 { +LL | | +LL | | create___: u8, +LL | | read___: u8, +LL | | update___: u8, +LL | | destroy___: u8, +LL | | } + | |_____^ + | + = help: remove the postfixes + +error: all fields have the same postfix: `type` + --> $DIR/struct_fields.rs:172:5 + | +LL | / struct DoLintToo { +LL | | +LL | | _create_type: u8, +LL | | _update_type: u8, +LL | | _delete_type: u8, +LL | | } + | |_____^ + | + = help: remove the postfixes + +error: field name starts with the struct's name + --> $DIR/struct_fields.rs:210:5 + | +LL | proxy: i32, + | ^^^^^^^^^^ + +error: all fields have the same prefix: `some` + --> $DIR/struct_fields.rs:226:13 + | +LL | / struct MacroStruct { +LL | | some_a: i32, +LL | | some_b: i32, +LL | | some_c: i32, +LL | | } + | |_____________^ +... +LL | mk_struct!(); + | ------------ in this macro invocation + | + = help: remove the prefixes + = note: this error originates in the macro `mk_struct` (in Nightly builds, run with -Z macro-backtrace for more info) + +error: field name starts with the struct's name + --> $DIR/struct_fields.rs:239:17 + | +LL | macrobaz_a: i32, + | ^^^^^^^^^^^^^^^ +... +LL | mk_struct2!(); + | ------------- in this macro invocation + | + = note: this error originates in the macro `mk_struct2` (in Nightly builds, run with -Z macro-backtrace for more info) + +error: field name starts with the struct's name + --> $DIR/struct_fields.rs:251:17 + | +LL | $field: i32, + | ^^^^^^^^^^^ +... +LL | mk_struct_with_names!(Foo, foo); + | ------------------------------- in this macro invocation + | + = note: this error originates in the macro `mk_struct_with_names` (in Nightly builds, run with -Z macro-backtrace for more info) + +error: all fields have the same prefix: `some` + --> $DIR/struct_fields.rs:291:13 + | +LL | / struct $struct_name { +LL | | $field1: i32, +LL | | $field2: i32, +LL | | $field3: i32, +LL | | } + | |_____________^ +... +LL | mk_struct_full_def!(PrefixData, some_data, some_meta, some_other); + | ----------------------------------------------------------------- in this macro invocation + | + = help: remove the prefixes + = note: this error originates in the macro `mk_struct_full_def` (in Nightly builds, run with -Z macro-backtrace for more info) + +error: aborting due to 21 previous errors + diff --git a/tests/ui/unnecessary_lazy_eval.fixed b/tests/ui/unnecessary_lazy_eval.fixed index 304e7b7fd7f6d..4778eaefdbdbb 100644 --- a/tests/ui/unnecessary_lazy_eval.fixed +++ b/tests/ui/unnecessary_lazy_eval.fixed @@ -5,6 +5,7 @@ #![allow(clippy::map_identity)] #![allow(clippy::needless_borrow)] #![allow(clippy::unnecessary_literal_unwrap)] +#![allow(clippy::unit_arg)] use std::ops::Deref; @@ -76,6 +77,8 @@ fn main() { let _ = opt.ok_or(2); let _ = nested_tuple_opt.unwrap_or(Some((1, 2))); let _ = cond.then_some(astronomers_pi); + let _ = true.then_some({}); + let _ = true.then_some({}); // Should lint - Builtin deref let r = &1; diff --git a/tests/ui/unnecessary_lazy_eval.rs b/tests/ui/unnecessary_lazy_eval.rs index ddfa6bb3ef615..d4b7fd31b1b0f 100644 --- a/tests/ui/unnecessary_lazy_eval.rs +++ b/tests/ui/unnecessary_lazy_eval.rs @@ -5,6 +5,7 @@ #![allow(clippy::map_identity)] #![allow(clippy::needless_borrow)] #![allow(clippy::unnecessary_literal_unwrap)] +#![allow(clippy::unit_arg)] use std::ops::Deref; @@ -76,6 +77,8 @@ fn main() { let _ = opt.ok_or_else(|| 2); let _ = nested_tuple_opt.unwrap_or_else(|| Some((1, 2))); let _ = cond.then(|| astronomers_pi); + let _ = true.then(|| -> _ {}); + let _ = true.then(|| {}); // Should lint - Builtin deref let r = &1; diff --git a/tests/ui/unnecessary_lazy_eval.stderr b/tests/ui/unnecessary_lazy_eval.stderr index 4f1ca3748728f..1b0db4759bbaa 100644 --- a/tests/ui/unnecessary_lazy_eval.stderr +++ b/tests/ui/unnecessary_lazy_eval.stderr @@ -1,5 +1,5 @@ error: unnecessary closure used to substitute value for `Option::None` - --> $DIR/unnecessary_lazy_eval.rs:68:13 + --> $DIR/unnecessary_lazy_eval.rs:69:13 | LL | let _ = opt.unwrap_or_else(|| 2); | ^^^^-------------------- @@ -10,7 +10,7 @@ LL | let _ = opt.unwrap_or_else(|| 2); = help: to override `-D warnings` add `#[allow(clippy::unnecessary_lazy_evaluations)]` error: unnecessary closure used to substitute value for `Option::None` - --> $DIR/unnecessary_lazy_eval.rs:69:13 + --> $DIR/unnecessary_lazy_eval.rs:70:13 | LL | let _ = opt.unwrap_or_else(|| astronomers_pi); | ^^^^--------------------------------- @@ -18,7 +18,7 @@ LL | let _ = opt.unwrap_or_else(|| astronomers_pi); | help: use `unwrap_or(..)` instead: `unwrap_or(astronomers_pi)` error: unnecessary closure used to substitute value for `Option::None` - --> $DIR/unnecessary_lazy_eval.rs:70:13 + --> $DIR/unnecessary_lazy_eval.rs:71:13 | LL | let _ = opt.unwrap_or_else(|| ext_str.some_field); | ^^^^------------------------------------- @@ -26,7 +26,7 @@ LL | let _ = opt.unwrap_or_else(|| ext_str.some_field); | help: use `unwrap_or(..)` instead: `unwrap_or(ext_str.some_field)` error: unnecessary closure used to substitute value for `Option::None` - --> $DIR/unnecessary_lazy_eval.rs:72:13 + --> $DIR/unnecessary_lazy_eval.rs:73:13 | LL | let _ = opt.and_then(|_| ext_opt); | ^^^^--------------------- @@ -34,7 +34,7 @@ LL | let _ = opt.and_then(|_| ext_opt); | help: use `and(..)` instead: `and(ext_opt)` error: unnecessary closure used to substitute value for `Option::None` - --> $DIR/unnecessary_lazy_eval.rs:73:13 + --> $DIR/unnecessary_lazy_eval.rs:74:13 | LL | let _ = opt.or_else(|| ext_opt); | ^^^^------------------- @@ -42,7 +42,7 @@ LL | let _ = opt.or_else(|| ext_opt); | help: use `or(..)` instead: `or(ext_opt)` error: unnecessary closure used to substitute value for `Option::None` - --> $DIR/unnecessary_lazy_eval.rs:74:13 + --> $DIR/unnecessary_lazy_eval.rs:75:13 | LL | let _ = opt.or_else(|| None); | ^^^^---------------- @@ -50,7 +50,7 @@ LL | let _ = opt.or_else(|| None); | help: use `or(..)` instead: `or(None)` error: unnecessary closure used to substitute value for `Option::None` - --> $DIR/unnecessary_lazy_eval.rs:75:13 + --> $DIR/unnecessary_lazy_eval.rs:76:13 | LL | let _ = opt.get_or_insert_with(|| 2); | ^^^^------------------------ @@ -58,7 +58,7 @@ LL | let _ = opt.get_or_insert_with(|| 2); | help: use `get_or_insert(..)` instead: `get_or_insert(2)` error: unnecessary closure used to substitute value for `Option::None` - --> $DIR/unnecessary_lazy_eval.rs:76:13 + --> $DIR/unnecessary_lazy_eval.rs:77:13 | LL | let _ = opt.ok_or_else(|| 2); | ^^^^---------------- @@ -66,7 +66,7 @@ LL | let _ = opt.ok_or_else(|| 2); | help: use `ok_or(..)` instead: `ok_or(2)` error: unnecessary closure used to substitute value for `Option::None` - --> $DIR/unnecessary_lazy_eval.rs:77:13 + --> $DIR/unnecessary_lazy_eval.rs:78:13 | LL | let _ = nested_tuple_opt.unwrap_or_else(|| Some((1, 2))); | ^^^^^^^^^^^^^^^^^------------------------------- @@ -74,15 +74,31 @@ LL | let _ = nested_tuple_opt.unwrap_or_else(|| Some((1, 2))); | help: use `unwrap_or(..)` instead: `unwrap_or(Some((1, 2)))` error: unnecessary closure used with `bool::then` - --> $DIR/unnecessary_lazy_eval.rs:78:13 + --> $DIR/unnecessary_lazy_eval.rs:79:13 | LL | let _ = cond.then(|| astronomers_pi); | ^^^^^----------------------- | | | help: use `then_some(..)` instead: `then_some(astronomers_pi)` +error: unnecessary closure used with `bool::then` + --> $DIR/unnecessary_lazy_eval.rs:80:13 + | +LL | let _ = true.then(|| -> _ {}); + | ^^^^^---------------- + | | + | help: use `then_some(..)` instead: `then_some({})` + +error: unnecessary closure used with `bool::then` + --> $DIR/unnecessary_lazy_eval.rs:81:13 + | +LL | let _ = true.then(|| {}); + | ^^^^^----------- + | | + | help: use `then_some(..)` instead: `then_some({})` + error: unnecessary closure used to substitute value for `Option::None` - --> $DIR/unnecessary_lazy_eval.rs:82:13 + --> $DIR/unnecessary_lazy_eval.rs:85:13 | LL | let _ = Some(1).unwrap_or_else(|| *r); | ^^^^^^^^--------------------- @@ -90,7 +106,7 @@ LL | let _ = Some(1).unwrap_or_else(|| *r); | help: use `unwrap_or(..)` instead: `unwrap_or(*r)` error: unnecessary closure used to substitute value for `Option::None` - --> $DIR/unnecessary_lazy_eval.rs:84:13 + --> $DIR/unnecessary_lazy_eval.rs:87:13 | LL | let _ = Some(1).unwrap_or_else(|| *b); | ^^^^^^^^--------------------- @@ -98,7 +114,7 @@ LL | let _ = Some(1).unwrap_or_else(|| *b); | help: use `unwrap_or(..)` instead: `unwrap_or(*b)` error: unnecessary closure used to substitute value for `Option::None` - --> $DIR/unnecessary_lazy_eval.rs:86:13 + --> $DIR/unnecessary_lazy_eval.rs:89:13 | LL | let _ = Some(1).as_ref().unwrap_or_else(|| &r); | ^^^^^^^^^^^^^^^^^--------------------- @@ -106,7 +122,7 @@ LL | let _ = Some(1).as_ref().unwrap_or_else(|| &r); | help: use `unwrap_or(..)` instead: `unwrap_or(&r)` error: unnecessary closure used to substitute value for `Option::None` - --> $DIR/unnecessary_lazy_eval.rs:87:13 + --> $DIR/unnecessary_lazy_eval.rs:90:13 | LL | let _ = Some(1).as_ref().unwrap_or_else(|| &b); | ^^^^^^^^^^^^^^^^^--------------------- @@ -114,7 +130,7 @@ LL | let _ = Some(1).as_ref().unwrap_or_else(|| &b); | help: use `unwrap_or(..)` instead: `unwrap_or(&b)` error: unnecessary closure used to substitute value for `Option::None` - --> $DIR/unnecessary_lazy_eval.rs:90:13 + --> $DIR/unnecessary_lazy_eval.rs:93:13 | LL | let _ = Some(10).unwrap_or_else(|| 2); | ^^^^^^^^^-------------------- @@ -122,7 +138,7 @@ LL | let _ = Some(10).unwrap_or_else(|| 2); | help: use `unwrap_or(..)` instead: `unwrap_or(2)` error: unnecessary closure used to substitute value for `Option::None` - --> $DIR/unnecessary_lazy_eval.rs:91:13 + --> $DIR/unnecessary_lazy_eval.rs:94:13 | LL | let _ = Some(10).and_then(|_| ext_opt); | ^^^^^^^^^--------------------- @@ -130,7 +146,7 @@ LL | let _ = Some(10).and_then(|_| ext_opt); | help: use `and(..)` instead: `and(ext_opt)` error: unnecessary closure used to substitute value for `Option::None` - --> $DIR/unnecessary_lazy_eval.rs:92:28 + --> $DIR/unnecessary_lazy_eval.rs:95:28 | LL | let _: Option = None.or_else(|| ext_opt); | ^^^^^------------------- @@ -138,7 +154,7 @@ LL | let _: Option = None.or_else(|| ext_opt); | help: use `or(..)` instead: `or(ext_opt)` error: unnecessary closure used to substitute value for `Option::None` - --> $DIR/unnecessary_lazy_eval.rs:93:13 + --> $DIR/unnecessary_lazy_eval.rs:96:13 | LL | let _ = None.get_or_insert_with(|| 2); | ^^^^^------------------------ @@ -146,7 +162,7 @@ LL | let _ = None.get_or_insert_with(|| 2); | help: use `get_or_insert(..)` instead: `get_or_insert(2)` error: unnecessary closure used to substitute value for `Option::None` - --> $DIR/unnecessary_lazy_eval.rs:94:35 + --> $DIR/unnecessary_lazy_eval.rs:97:35 | LL | let _: Result = None.ok_or_else(|| 2); | ^^^^^---------------- @@ -154,7 +170,7 @@ LL | let _: Result = None.ok_or_else(|| 2); | help: use `ok_or(..)` instead: `ok_or(2)` error: unnecessary closure used to substitute value for `Option::None` - --> $DIR/unnecessary_lazy_eval.rs:95:28 + --> $DIR/unnecessary_lazy_eval.rs:98:28 | LL | let _: Option = None.or_else(|| None); | ^^^^^---------------- @@ -162,7 +178,7 @@ LL | let _: Option = None.or_else(|| None); | help: use `or(..)` instead: `or(None)` error: unnecessary closure used to substitute value for `Option::None` - --> $DIR/unnecessary_lazy_eval.rs:98:13 + --> $DIR/unnecessary_lazy_eval.rs:101:13 | LL | let _ = deep.0.unwrap_or_else(|| 2); | ^^^^^^^-------------------- @@ -170,7 +186,7 @@ LL | let _ = deep.0.unwrap_or_else(|| 2); | help: use `unwrap_or(..)` instead: `unwrap_or(2)` error: unnecessary closure used to substitute value for `Option::None` - --> $DIR/unnecessary_lazy_eval.rs:99:13 + --> $DIR/unnecessary_lazy_eval.rs:102:13 | LL | let _ = deep.0.and_then(|_| ext_opt); | ^^^^^^^--------------------- @@ -178,7 +194,7 @@ LL | let _ = deep.0.and_then(|_| ext_opt); | help: use `and(..)` instead: `and(ext_opt)` error: unnecessary closure used to substitute value for `Option::None` - --> $DIR/unnecessary_lazy_eval.rs:100:13 + --> $DIR/unnecessary_lazy_eval.rs:103:13 | LL | let _ = deep.0.or_else(|| None); | ^^^^^^^---------------- @@ -186,7 +202,7 @@ LL | let _ = deep.0.or_else(|| None); | help: use `or(..)` instead: `or(None)` error: unnecessary closure used to substitute value for `Option::None` - --> $DIR/unnecessary_lazy_eval.rs:101:13 + --> $DIR/unnecessary_lazy_eval.rs:104:13 | LL | let _ = deep.0.get_or_insert_with(|| 2); | ^^^^^^^------------------------ @@ -194,7 +210,7 @@ LL | let _ = deep.0.get_or_insert_with(|| 2); | help: use `get_or_insert(..)` instead: `get_or_insert(2)` error: unnecessary closure used to substitute value for `Option::None` - --> $DIR/unnecessary_lazy_eval.rs:102:13 + --> $DIR/unnecessary_lazy_eval.rs:105:13 | LL | let _ = deep.0.ok_or_else(|| 2); | ^^^^^^^---------------- @@ -202,7 +218,7 @@ LL | let _ = deep.0.ok_or_else(|| 2); | help: use `ok_or(..)` instead: `ok_or(2)` error: unnecessary closure used to substitute value for `Option::None` - --> $DIR/unnecessary_lazy_eval.rs:132:28 + --> $DIR/unnecessary_lazy_eval.rs:135:28 | LL | let _: Option = None.or_else(|| Some(3)); | ^^^^^------------------- @@ -210,7 +226,7 @@ LL | let _: Option = None.or_else(|| Some(3)); | help: use `or(..)` instead: `or(Some(3))` error: unnecessary closure used to substitute value for `Option::None` - --> $DIR/unnecessary_lazy_eval.rs:133:13 + --> $DIR/unnecessary_lazy_eval.rs:136:13 | LL | let _ = deep.0.or_else(|| Some(3)); | ^^^^^^^------------------- @@ -218,7 +234,7 @@ LL | let _ = deep.0.or_else(|| Some(3)); | help: use `or(..)` instead: `or(Some(3))` error: unnecessary closure used to substitute value for `Option::None` - --> $DIR/unnecessary_lazy_eval.rs:134:13 + --> $DIR/unnecessary_lazy_eval.rs:137:13 | LL | let _ = opt.or_else(|| Some(3)); | ^^^^------------------- @@ -226,7 +242,7 @@ LL | let _ = opt.or_else(|| Some(3)); | help: use `or(..)` instead: `or(Some(3))` error: unnecessary closure used to substitute value for `Result::Err` - --> $DIR/unnecessary_lazy_eval.rs:140:13 + --> $DIR/unnecessary_lazy_eval.rs:143:13 | LL | let _ = res2.unwrap_or_else(|_| 2); | ^^^^^--------------------- @@ -234,7 +250,7 @@ LL | let _ = res2.unwrap_or_else(|_| 2); | help: use `unwrap_or(..)` instead: `unwrap_or(2)` error: unnecessary closure used to substitute value for `Result::Err` - --> $DIR/unnecessary_lazy_eval.rs:141:13 + --> $DIR/unnecessary_lazy_eval.rs:144:13 | LL | let _ = res2.unwrap_or_else(|_| astronomers_pi); | ^^^^^---------------------------------- @@ -242,7 +258,7 @@ LL | let _ = res2.unwrap_or_else(|_| astronomers_pi); | help: use `unwrap_or(..)` instead: `unwrap_or(astronomers_pi)` error: unnecessary closure used to substitute value for `Result::Err` - --> $DIR/unnecessary_lazy_eval.rs:142:13 + --> $DIR/unnecessary_lazy_eval.rs:145:13 | LL | let _ = res2.unwrap_or_else(|_| ext_str.some_field); | ^^^^^-------------------------------------- @@ -250,7 +266,7 @@ LL | let _ = res2.unwrap_or_else(|_| ext_str.some_field); | help: use `unwrap_or(..)` instead: `unwrap_or(ext_str.some_field)` error: unnecessary closure used to substitute value for `Result::Err` - --> $DIR/unnecessary_lazy_eval.rs:164:35 + --> $DIR/unnecessary_lazy_eval.rs:167:35 | LL | let _: Result = res.and_then(|_| Err(2)); | ^^^^-------------------- @@ -258,7 +274,7 @@ LL | let _: Result = res.and_then(|_| Err(2)); | help: use `and(..)` instead: `and(Err(2))` error: unnecessary closure used to substitute value for `Result::Err` - --> $DIR/unnecessary_lazy_eval.rs:165:35 + --> $DIR/unnecessary_lazy_eval.rs:168:35 | LL | let _: Result = res.and_then(|_| Err(astronomers_pi)); | ^^^^--------------------------------- @@ -266,7 +282,7 @@ LL | let _: Result = res.and_then(|_| Err(astronomers_pi)); | help: use `and(..)` instead: `and(Err(astronomers_pi))` error: unnecessary closure used to substitute value for `Result::Err` - --> $DIR/unnecessary_lazy_eval.rs:166:35 + --> $DIR/unnecessary_lazy_eval.rs:169:35 | LL | let _: Result = res.and_then(|_| Err(ext_str.some_field)); | ^^^^------------------------------------- @@ -274,7 +290,7 @@ LL | let _: Result = res.and_then(|_| Err(ext_str.some_field)) | help: use `and(..)` instead: `and(Err(ext_str.some_field))` error: unnecessary closure used to substitute value for `Result::Err` - --> $DIR/unnecessary_lazy_eval.rs:168:35 + --> $DIR/unnecessary_lazy_eval.rs:171:35 | LL | let _: Result = res.or_else(|_| Ok(2)); | ^^^^------------------ @@ -282,7 +298,7 @@ LL | let _: Result = res.or_else(|_| Ok(2)); | help: use `or(..)` instead: `or(Ok(2))` error: unnecessary closure used to substitute value for `Result::Err` - --> $DIR/unnecessary_lazy_eval.rs:169:35 + --> $DIR/unnecessary_lazy_eval.rs:172:35 | LL | let _: Result = res.or_else(|_| Ok(astronomers_pi)); | ^^^^------------------------------- @@ -290,7 +306,7 @@ LL | let _: Result = res.or_else(|_| Ok(astronomers_pi)); | help: use `or(..)` instead: `or(Ok(astronomers_pi))` error: unnecessary closure used to substitute value for `Result::Err` - --> $DIR/unnecessary_lazy_eval.rs:170:35 + --> $DIR/unnecessary_lazy_eval.rs:173:35 | LL | let _: Result = res.or_else(|_| Ok(ext_str.some_field)); | ^^^^----------------------------------- @@ -298,7 +314,7 @@ LL | let _: Result = res.or_else(|_| Ok(ext_str.some_field)); | help: use `or(..)` instead: `or(Ok(ext_str.some_field))` error: unnecessary closure used to substitute value for `Result::Err` - --> $DIR/unnecessary_lazy_eval.rs:171:35 + --> $DIR/unnecessary_lazy_eval.rs:174:35 | LL | let _: Result = res. | ___________________________________^ @@ -312,5 +328,5 @@ LL | | or_else(|_| Ok(ext_str.some_field)); | | | help: use `or(..)` instead: `or(Ok(ext_str.some_field))` -error: aborting due to 38 previous errors +error: aborting due to 40 previous errors diff --git a/tests/ui/unnecessary_lazy_eval_unfixable.rs b/tests/ui/unnecessary_lazy_eval_unfixable.rs index 33685bfb738b0..412d4aaafb4a1 100644 --- a/tests/ui/unnecessary_lazy_eval_unfixable.rs +++ b/tests/ui/unnecessary_lazy_eval_unfixable.rs @@ -25,3 +25,8 @@ fn main() { let arr = [(Some(1),)]; Some(&0).and_then(|&i| arr[i].0); } + +fn issue11672() { + // Return type annotation helps type inference and removing it can break code + let _ = true.then(|| -> &[u8] { &[] }); +} diff --git a/tests/ui/unnecessary_lazy_eval_unfixable.stderr b/tests/ui/unnecessary_lazy_eval_unfixable.stderr index 27fa560d4d77c..95b02be91ca8a 100644 --- a/tests/ui/unnecessary_lazy_eval_unfixable.stderr +++ b/tests/ui/unnecessary_lazy_eval_unfixable.stderr @@ -25,5 +25,13 @@ LL | let _ = Ok(1).unwrap_or_else(|SomeStruct { .. }| 2); | | | help: use `unwrap_or(..)` instead: `unwrap_or(2)` -error: aborting due to 3 previous errors +error: unnecessary closure used with `bool::then` + --> $DIR/unnecessary_lazy_eval_unfixable.rs:31:13 + | +LL | let _ = true.then(|| -> &[u8] { &[] }); + | ^^^^^------------------------- + | | + | help: use `then_some(..)` instead: `then_some({ &[] })` + +error: aborting due to 4 previous errors diff --git a/triagebot.toml b/triagebot.toml index 6856bb0ab3759..419b3c30deb0a 100644 --- a/triagebot.toml +++ b/triagebot.toml @@ -11,6 +11,8 @@ allow-unauthenticated = [ # Have rustbot inform users about the *No Merge Policy* [no-merges] +exclude_titles = ["Rustup"] # exclude syncs from rust-lang/rust +labels = ["has-merge-commits", "S-waiting-on-author"] [autolabel."S-waiting-on-review"] new_pr = true