diff --git a/Cargo.lock b/Cargo.lock index 8385f1a18e5cd..1a16789d15c45 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -555,7 +555,7 @@ dependencies = [ [[package]] name = "clippy" -version = "0.1.57" +version = "0.1.58" dependencies = [ "cargo_metadata 0.12.0", "clippy_lints", @@ -582,6 +582,7 @@ version = "0.0.1" dependencies = [ "bytecount", "clap", + "indoc", "itertools 0.10.1", "opener", "regex", @@ -591,7 +592,7 @@ dependencies = [ [[package]] name = "clippy_lints" -version = "0.1.57" +version = "0.1.58" dependencies = [ "cargo_metadata 0.12.0", "clippy_utils", @@ -612,7 +613,7 @@ dependencies = [ [[package]] name = "clippy_utils" -version = "0.1.57" +version = "0.1.58" dependencies = [ "if_chain", "rustc-semver", @@ -1665,6 +1666,15 @@ dependencies = [ "serde", ] +[[package]] +name = "indoc" +version = "1.0.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e5a75aeaaef0ce18b58056d306c27b07436fbb34b8816c53094b76dd81803136" +dependencies = [ + "unindent", +] + [[package]] name = "installer" version = "0.0.0" @@ -5593,6 +5603,12 @@ dependencies = [ "diff", ] +[[package]] +name = "unindent" +version = "0.1.7" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f14ee04d9415b52b3aeab06258a3f07093182b88ba0f9b8d203f211a7a7d41c7" + [[package]] name = "unstable-book-gen" version = "0.1.0" diff --git a/src/tools/clippy/CHANGELOG.md b/src/tools/clippy/CHANGELOG.md index e700e5f0d736e..3b4c687209e11 100644 --- a/src/tools/clippy/CHANGELOG.md +++ b/src/tools/clippy/CHANGELOG.md @@ -2730,11 +2730,13 @@ Released 2018-09-13 [`fn_address_comparisons`]: https://rust-lang.github.io/rust-clippy/master/index.html#fn_address_comparisons [`fn_params_excessive_bools`]: https://rust-lang.github.io/rust-clippy/master/index.html#fn_params_excessive_bools [`fn_to_numeric_cast`]: https://rust-lang.github.io/rust-clippy/master/index.html#fn_to_numeric_cast +[`fn_to_numeric_cast_any`]: https://rust-lang.github.io/rust-clippy/master/index.html#fn_to_numeric_cast_any [`fn_to_numeric_cast_with_truncation`]: https://rust-lang.github.io/rust-clippy/master/index.html#fn_to_numeric_cast_with_truncation [`for_kv_map`]: https://rust-lang.github.io/rust-clippy/master/index.html#for_kv_map [`for_loops_over_fallibles`]: https://rust-lang.github.io/rust-clippy/master/index.html#for_loops_over_fallibles [`forget_copy`]: https://rust-lang.github.io/rust-clippy/master/index.html#forget_copy [`forget_ref`]: https://rust-lang.github.io/rust-clippy/master/index.html#forget_ref +[`format_in_format_args`]: https://rust-lang.github.io/rust-clippy/master/index.html#format_in_format_args [`from_iter_instead_of_collect`]: https://rust-lang.github.io/rust-clippy/master/index.html#from_iter_instead_of_collect [`from_over_into`]: https://rust-lang.github.io/rust-clippy/master/index.html#from_over_into [`from_str_radix_10`]: https://rust-lang.github.io/rust-clippy/master/index.html#from_str_radix_10 @@ -2836,6 +2838,7 @@ Released 2018-09-13 [`match_result_ok`]: https://rust-lang.github.io/rust-clippy/master/index.html#match_result_ok [`match_same_arms`]: https://rust-lang.github.io/rust-clippy/master/index.html#match_same_arms [`match_single_binding`]: https://rust-lang.github.io/rust-clippy/master/index.html#match_single_binding +[`match_str_case_mismatch`]: https://rust-lang.github.io/rust-clippy/master/index.html#match_str_case_mismatch [`match_wild_err_arm`]: https://rust-lang.github.io/rust-clippy/master/index.html#match_wild_err_arm [`match_wildcard_for_single_variants`]: https://rust-lang.github.io/rust-clippy/master/index.html#match_wildcard_for_single_variants [`maybe_infinite_iter`]: https://rust-lang.github.io/rust-clippy/master/index.html#maybe_infinite_iter @@ -2896,6 +2899,7 @@ Released 2018-09-13 [`new_ret_no_self`]: https://rust-lang.github.io/rust-clippy/master/index.html#new_ret_no_self [`new_without_default`]: https://rust-lang.github.io/rust-clippy/master/index.html#new_without_default [`no_effect`]: https://rust-lang.github.io/rust-clippy/master/index.html#no_effect +[`no_effect_underscore_binding`]: https://rust-lang.github.io/rust-clippy/master/index.html#no_effect_underscore_binding [`non_ascii_literal`]: https://rust-lang.github.io/rust-clippy/master/index.html#non_ascii_literal [`non_octal_unix_permissions`]: https://rust-lang.github.io/rust-clippy/master/index.html#non_octal_unix_permissions [`non_send_fields_in_send_ty`]: https://rust-lang.github.io/rust-clippy/master/index.html#non_send_fields_in_send_ty @@ -3012,16 +3016,19 @@ Released 2018-09-13 [`temporary_assignment`]: https://rust-lang.github.io/rust-clippy/master/index.html#temporary_assignment [`to_digit_is_some`]: https://rust-lang.github.io/rust-clippy/master/index.html#to_digit_is_some [`to_string_in_display`]: https://rust-lang.github.io/rust-clippy/master/index.html#to_string_in_display +[`to_string_in_format_args`]: https://rust-lang.github.io/rust-clippy/master/index.html#to_string_in_format_args [`todo`]: https://rust-lang.github.io/rust-clippy/master/index.html#todo [`too_many_arguments`]: https://rust-lang.github.io/rust-clippy/master/index.html#too_many_arguments [`too_many_lines`]: https://rust-lang.github.io/rust-clippy/master/index.html#too_many_lines [`toplevel_ref_arg`]: https://rust-lang.github.io/rust-clippy/master/index.html#toplevel_ref_arg +[`trailing_empty_array`]: https://rust-lang.github.io/rust-clippy/master/index.html#trailing_empty_array [`trait_duplication_in_bounds`]: https://rust-lang.github.io/rust-clippy/master/index.html#trait_duplication_in_bounds [`transmute_bytes_to_str`]: https://rust-lang.github.io/rust-clippy/master/index.html#transmute_bytes_to_str [`transmute_float_to_int`]: https://rust-lang.github.io/rust-clippy/master/index.html#transmute_float_to_int [`transmute_int_to_bool`]: https://rust-lang.github.io/rust-clippy/master/index.html#transmute_int_to_bool [`transmute_int_to_char`]: https://rust-lang.github.io/rust-clippy/master/index.html#transmute_int_to_char [`transmute_int_to_float`]: https://rust-lang.github.io/rust-clippy/master/index.html#transmute_int_to_float +[`transmute_num_to_bytes`]: https://rust-lang.github.io/rust-clippy/master/index.html#transmute_num_to_bytes [`transmute_ptr_to_ptr`]: https://rust-lang.github.io/rust-clippy/master/index.html#transmute_ptr_to_ptr [`transmute_ptr_to_ref`]: https://rust-lang.github.io/rust-clippy/master/index.html#transmute_ptr_to_ref [`transmutes_expressible_as_ptr_casts`]: https://rust-lang.github.io/rust-clippy/master/index.html#transmutes_expressible_as_ptr_casts @@ -3031,10 +3038,12 @@ Released 2018-09-13 [`try_err`]: https://rust-lang.github.io/rust-clippy/master/index.html#try_err [`type_complexity`]: https://rust-lang.github.io/rust-clippy/master/index.html#type_complexity [`type_repetition_in_bounds`]: https://rust-lang.github.io/rust-clippy/master/index.html#type_repetition_in_bounds +[`undocumented_unsafe_blocks`]: https://rust-lang.github.io/rust-clippy/master/index.html#undocumented_unsafe_blocks [`undropped_manually_drops`]: https://rust-lang.github.io/rust-clippy/master/index.html#undropped_manually_drops [`unicode_not_nfc`]: https://rust-lang.github.io/rust-clippy/master/index.html#unicode_not_nfc [`unimplemented`]: https://rust-lang.github.io/rust-clippy/master/index.html#unimplemented [`uninit_assumed_init`]: https://rust-lang.github.io/rust-clippy/master/index.html#uninit_assumed_init +[`uninit_vec`]: https://rust-lang.github.io/rust-clippy/master/index.html#uninit_vec [`unit_arg`]: https://rust-lang.github.io/rust-clippy/master/index.html#unit_arg [`unit_cmp`]: https://rust-lang.github.io/rust-clippy/master/index.html#unit_cmp [`unit_return_expecting_ord`]: https://rust-lang.github.io/rust-clippy/master/index.html#unit_return_expecting_ord diff --git a/src/tools/clippy/Cargo.toml b/src/tools/clippy/Cargo.toml index ba3ed3053ac74..ed7fb1440139f 100644 --- a/src/tools/clippy/Cargo.toml +++ b/src/tools/clippy/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "clippy" -version = "0.1.57" +version = "0.1.58" description = "A bunch of helpful lints to avoid common pitfalls in Rust" repository = "https://github.com/rust-lang/rust-clippy" readme = "README.md" diff --git a/src/tools/clippy/clippy_dev/Cargo.toml b/src/tools/clippy/clippy_dev/Cargo.toml index 4a13a45240976..affb283017c8c 100644 --- a/src/tools/clippy/clippy_dev/Cargo.toml +++ b/src/tools/clippy/clippy_dev/Cargo.toml @@ -6,6 +6,7 @@ edition = "2021" [dependencies] bytecount = "0.6" clap = "2.33" +indoc = "1.0" itertools = "0.10" opener = "0.5" regex = "1.5" diff --git a/src/tools/clippy/clippy_dev/src/main.rs b/src/tools/clippy/clippy_dev/src/main.rs index 8fdeba9842af3..b5c04efce3bc9 100644 --- a/src/tools/clippy/clippy_dev/src/main.rs +++ b/src/tools/clippy/clippy_dev/src/main.rs @@ -28,6 +28,7 @@ fn main() { matches.value_of("pass"), matches.value_of("name"), matches.value_of("category"), + matches.is_present("msrv"), ) { Ok(_) => update_lints::run(update_lints::UpdateMode::Change), Err(e) => eprintln!("Unable to create lint: {}", e), @@ -147,6 +148,11 @@ fn get_clap_config<'a>() -> ArgMatches<'a> { "internal_warn", ]) .takes_value(true), + ) + .arg( + Arg::with_name("msrv") + .long("msrv") + .help("Add MSRV config code to the lint"), ), ) .subcommand( diff --git a/src/tools/clippy/clippy_dev/src/new_lint.rs b/src/tools/clippy/clippy_dev/src/new_lint.rs index 3a81aaba6de04..25320907bb492 100644 --- a/src/tools/clippy/clippy_dev/src/new_lint.rs +++ b/src/tools/clippy/clippy_dev/src/new_lint.rs @@ -1,4 +1,5 @@ use crate::clippy_project_root; +use indoc::indoc; use std::fs::{self, OpenOptions}; use std::io::prelude::*; use std::io::{self, ErrorKind}; @@ -32,7 +33,7 @@ impl Context for io::Result { /// # Errors /// /// This function errors out if the files couldn't be created or written to. -pub fn create(pass: Option<&str>, lint_name: Option<&str>, category: Option<&str>) -> io::Result<()> { +pub fn create(pass: Option<&str>, lint_name: Option<&str>, category: Option<&str>, msrv: bool) -> io::Result<()> { let lint = LintData { pass: pass.expect("`pass` argument is validated by clap"), name: lint_name.expect("`name` argument is validated by clap"), @@ -40,29 +41,12 @@ pub fn create(pass: Option<&str>, lint_name: Option<&str>, category: Option<&str project_root: clippy_project_root(), }; - create_lint(&lint).context("Unable to create lint implementation")?; + create_lint(&lint, msrv).context("Unable to create lint implementation")?; create_test(&lint).context("Unable to create a test for the new lint") } -fn create_lint(lint: &LintData<'_>) -> io::Result<()> { - let (pass_type, pass_lifetimes, pass_import, context_import) = match lint.pass { - "early" => ("EarlyLintPass", "", "use rustc_ast::ast::*;", "EarlyContext"), - "late" => ("LateLintPass", "<'_>", "use rustc_hir::*;", "LateContext"), - _ => { - unreachable!("`pass_type` should only ever be `early` or `late`!"); - }, - }; - - let camel_case_name = to_camel_case(lint.name); - let lint_contents = get_lint_file_contents( - pass_type, - pass_lifetimes, - lint.name, - &camel_case_name, - lint.category, - pass_import, - context_import, - ); +fn create_lint(lint: &LintData<'_>, enable_msrv: bool) -> io::Result<()> { + let lint_contents = get_lint_file_contents(lint, enable_msrv); let lint_path = format!("clippy_lints/src/{}.rs", lint.name); write_file(lint.project_root.join(&lint_path), lint_contents.as_bytes()) @@ -122,12 +106,13 @@ fn to_camel_case(name: &str) -> String { fn get_test_file_contents(lint_name: &str, header_commands: Option<&str>) -> String { let mut contents = format!( - "#![warn(clippy::{})] + indoc! {" + #![warn(clippy::{})] -fn main() {{ - // test code goes here -}} -", + fn main() {{ + // test code goes here + }} + "}, lint_name ); @@ -140,64 +125,143 @@ fn main() {{ fn get_manifest_contents(lint_name: &str, hint: &str) -> String { format!( - r#" -# {} + indoc! {r#" + # {} -[package] -name = "{}" -version = "0.1.0" -publish = false + [package] + name = "{}" + version = "0.1.0" + publish = false -[workspace] -"#, + [workspace] + "#}, hint, lint_name ) } -fn get_lint_file_contents( - pass_type: &str, - pass_lifetimes: &str, - lint_name: &str, - camel_case_name: &str, - category: &str, - pass_import: &str, - context_import: &str, -) -> String { - format!( - "use rustc_lint::{{{type}, {context_import}}}; -use rustc_session::{{declare_lint_pass, declare_tool_lint}}; -{pass_import} - -declare_clippy_lint! {{ - /// ### What it does - /// - /// ### Why is this bad? - /// - /// ### Example - /// ```rust - /// // example code where clippy issues a warning - /// ``` - /// Use instead: - /// ```rust - /// // example code which does not raise clippy warning - /// ``` - pub {name_upper}, - {category}, - \"default lint description\" -}} - -declare_lint_pass!({name_camel} => [{name_upper}]); - -impl {type}{lifetimes} for {name_camel} {{}} -", - type=pass_type, - lifetimes=pass_lifetimes, - name_upper=lint_name.to_uppercase(), - name_camel=camel_case_name, - category=category, - pass_import=pass_import, - context_import=context_import - ) +fn get_lint_file_contents(lint: &LintData<'_>, enable_msrv: bool) -> String { + let mut result = String::new(); + + let (pass_type, pass_lifetimes, pass_import, context_import) = match lint.pass { + "early" => ("EarlyLintPass", "", "use rustc_ast::ast::*;", "EarlyContext"), + "late" => ("LateLintPass", "<'_>", "use rustc_hir::*;", "LateContext"), + _ => { + unreachable!("`pass_type` should only ever be `early` or `late`!"); + }, + }; + + let lint_name = lint.name; + let pass_name = lint.pass; + let category = lint.category; + let name_camel = to_camel_case(lint.name); + let name_upper = lint_name.to_uppercase(); + + result.push_str(&if enable_msrv { + format!( + indoc! {" + use clippy_utils::msrvs; + {pass_import} + use rustc_lint::{{{context_import}, {pass_type}, LintContext}}; + use rustc_semver::RustcVersion; + use rustc_session::{{declare_tool_lint, impl_lint_pass}}; + + "}, + pass_type = pass_type, + pass_import = pass_import, + context_import = context_import, + ) + } else { + format!( + indoc! {" + {pass_import} + use rustc_lint::{{{context_import}, {pass_type}}}; + use rustc_session::{{declare_lint_pass, declare_tool_lint}}; + + "}, + pass_import = pass_import, + pass_type = pass_type, + context_import = context_import + ) + }); + + result.push_str(&format!( + indoc! {" + declare_clippy_lint! {{ + /// ### What it does + /// + /// ### Why is this bad? + /// + /// ### Example + /// ```rust + /// // example code where clippy issues a warning + /// ``` + /// Use instead: + /// ```rust + /// // example code which does not raise clippy warning + /// ``` + pub {name_upper}, + {category}, + \"default lint description\" + }} + "}, + name_upper = name_upper, + category = category, + )); + + result.push_str(&if enable_msrv { + format!( + indoc! {" + pub struct {name_camel} {{ + msrv: Option, + }} + + impl {name_camel} {{ + #[must_use] + pub fn new(msrv: Option) -> Self {{ + Self {{ msrv }} + }} + }} + + impl_lint_pass!({name_camel} => [{name_upper}]); + + impl {pass_type}{pass_lifetimes} for {name_camel} {{ + extract_msrv_attr!({context_import}); + }} + + // TODO: Register the lint pass in `clippy_lints/src/lib.rs`, + // e.g. store.register_{pass_name}_pass(move || Box::new({module_name}::{name_camel}::new(msrv))); + // TODO: Add MSRV level to `clippy_utils/src/msrvs.rs` if needed. + // TODO: Add MSRV test to `tests/ui/min_rust_version_attr.rs`. + // TODO: Update msrv config comment in `clippy_lints/src/utils/conf.rs` + "}, + pass_type = pass_type, + pass_lifetimes = pass_lifetimes, + pass_name = pass_name, + name_upper = name_upper, + name_camel = name_camel, + module_name = lint_name, + context_import = context_import, + ) + } else { + format!( + indoc! {" + declare_lint_pass!({name_camel} => [{name_upper}]); + + impl {pass_type}{pass_lifetimes} for {name_camel} {{}} + // + // TODO: Register the lint pass in `clippy_lints/src/lib.rs`, + // e.g. store.register_{pass_name}_pass(|| Box::new({module_name}::{name_camel})); + "}, + pass_type = pass_type, + pass_lifetimes = pass_lifetimes, + pass_name = pass_name, + name_upper = name_upper, + name_camel = name_camel, + module_name = lint_name, + ) + }); + + result } #[test] diff --git a/src/tools/clippy/clippy_dev/src/update_lints.rs b/src/tools/clippy/clippy_dev/src/update_lints.rs index 10d859988f6f2..23f58bc4915f9 100644 --- a/src/tools/clippy/clippy_dev/src/update_lints.rs +++ b/src/tools/clippy/clippy_dev/src/update_lints.rs @@ -619,8 +619,8 @@ mod tests { Lint::new("should_assert_eq2", "group2", "abc", None, "module_name"), ]; let expected = vec![ - format!("[`should_assert_eq`]: {}#should_assert_eq", DOCS_LINK.to_string()), - format!("[`should_assert_eq2`]: {}#should_assert_eq2", DOCS_LINK.to_string()), + format!("[`should_assert_eq`]: {}#should_assert_eq", DOCS_LINK), + format!("[`should_assert_eq2`]: {}#should_assert_eq2", DOCS_LINK), ]; assert_eq!(expected, gen_changelog_lint_list(lints.iter())); } diff --git a/src/tools/clippy/clippy_lints/Cargo.toml b/src/tools/clippy/clippy_lints/Cargo.toml index 7900dc6d04141..aaf9ac83d4900 100644 --- a/src/tools/clippy/clippy_lints/Cargo.toml +++ b/src/tools/clippy/clippy_lints/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "clippy_lints" -version = "0.1.57" +version = "0.1.58" description = "A bunch of helpful lints to avoid common pitfalls in Rust" repository = "https://github.com/rust-lang/rust-clippy" readme = "README.md" diff --git a/src/tools/clippy/clippy_lints/src/casts/fn_to_numeric_cast_any.rs b/src/tools/clippy/clippy_lints/src/casts/fn_to_numeric_cast_any.rs new file mode 100644 index 0000000000000..03621887a34a6 --- /dev/null +++ b/src/tools/clippy/clippy_lints/src/casts/fn_to_numeric_cast_any.rs @@ -0,0 +1,34 @@ +use clippy_utils::diagnostics::span_lint_and_sugg; +use clippy_utils::source::snippet_with_applicability; +use rustc_errors::Applicability; +use rustc_hir::Expr; +use rustc_lint::LateContext; +use rustc_middle::ty::{self, Ty}; + +use super::FN_TO_NUMERIC_CAST_ANY; + +pub(super) fn check(cx: &LateContext<'_>, expr: &Expr<'_>, cast_expr: &Expr<'_>, cast_from: Ty<'_>, cast_to: Ty<'_>) { + // We allow casts from any function type to any function type. + match cast_to.kind() { + ty::FnDef(..) | ty::FnPtr(..) => return, + _ => { /* continue to checks */ }, + } + + match cast_from.kind() { + ty::FnDef(..) | ty::FnPtr(_) => { + let mut applicability = Applicability::MaybeIncorrect; + let from_snippet = snippet_with_applicability(cx, cast_expr.span, "..", &mut applicability); + + span_lint_and_sugg( + cx, + FN_TO_NUMERIC_CAST_ANY, + expr.span, + &format!("casting function pointer `{}` to `{}`", from_snippet, cast_to), + "did you mean to invoke the function?", + format!("{}() as {}", from_snippet, cast_to), + applicability, + ); + }, + _ => {}, + } +} diff --git a/src/tools/clippy/clippy_lints/src/casts/mod.rs b/src/tools/clippy/clippy_lints/src/casts/mod.rs index 27e1bea799353..f0800c6a6f18f 100644 --- a/src/tools/clippy/clippy_lints/src/casts/mod.rs +++ b/src/tools/clippy/clippy_lints/src/casts/mod.rs @@ -7,6 +7,7 @@ mod cast_ref_to_mut; mod cast_sign_loss; mod char_lit_as_u8; mod fn_to_numeric_cast; +mod fn_to_numeric_cast_any; mod fn_to_numeric_cast_with_truncation; mod ptr_as_ptr; mod unnecessary_cast; @@ -251,6 +252,42 @@ declare_clippy_lint! { "casting a function pointer to a numeric type not wide enough to store the address" } +declare_clippy_lint! { + /// ### What it does + /// Checks for casts of a function pointer to any integer type. + /// + /// ### Why is this bad? + /// Casting a function pointer to an integer can have surprising results and can occur + /// accidentally if parantheses are omitted from a function call. If you aren't doing anything + /// low-level with function pointers then you can opt-out of casting functions to integers in + /// order to avoid mistakes. Alternatively, you can use this lint to audit all uses of function + /// pointer casts in your code. + /// + /// ### Example + /// ```rust + /// // Bad: fn1 is cast as `usize` + /// fn fn1() -> u16 { + /// 1 + /// }; + /// let _ = fn1 as usize; + /// + /// // Good: maybe you intended to call the function? + /// fn fn2() -> u16 { + /// 1 + /// }; + /// let _ = fn2() as usize; + /// + /// // Good: maybe you intended to cast it to a function type? + /// fn fn3() -> u16 { + /// 1 + /// } + /// let _ = fn3 as fn() -> u16; + /// ``` + pub FN_TO_NUMERIC_CAST_ANY, + restriction, + "casting a function pointer to any integer type" +} + declare_clippy_lint! { /// ### What it does /// Checks for casts of `&T` to `&mut T` anywhere in the code. @@ -360,6 +397,7 @@ impl_lint_pass!(Casts => [ CAST_REF_TO_MUT, CAST_PTR_ALIGNMENT, UNNECESSARY_CAST, + FN_TO_NUMERIC_CAST_ANY, FN_TO_NUMERIC_CAST, FN_TO_NUMERIC_CAST_WITH_TRUNCATION, CHAR_LIT_AS_U8, @@ -385,6 +423,7 @@ impl<'tcx> LateLintPass<'tcx> for Casts { return; } + fn_to_numeric_cast_any::check(cx, expr, cast_expr, cast_from, cast_to); fn_to_numeric_cast::check(cx, expr, cast_expr, cast_from, cast_to); fn_to_numeric_cast_with_truncation::check(cx, expr, cast_expr, cast_from, cast_to); if cast_from.is_numeric() && cast_to.is_numeric() && !in_external_macro(cx.sess(), expr.span) { diff --git a/src/tools/clippy/clippy_lints/src/copies.rs b/src/tools/clippy/clippy_lints/src/copies.rs index b7385dcfbca19..8abf10c0d1c2d 100644 --- a/src/tools/clippy/clippy_lints/src/copies.rs +++ b/src/tools/clippy/clippy_lints/src/copies.rs @@ -459,12 +459,10 @@ fn emit_branches_sharing_code_lint( } else { sm.stmt_span(block.stmts[block.stmts.len() - end_stmts].span, block.span) }; - let moved_end = block - .expr - .map_or_else( - || sm.stmt_span(block.stmts[block.stmts.len() - 1].span, block.span), - |expr| expr.span.source_callsite(), - ); + let moved_end = block.expr.map_or_else( + || sm.stmt_span(block.stmts[block.stmts.len() - 1].span, block.span), + |expr| expr.span.source_callsite(), + ); let moved_span = moved_start.to(moved_end); let moved_snipped = reindent_multiline(snippet(cx, moved_span, "_"), true, None); diff --git a/src/tools/clippy/clippy_lints/src/default.rs b/src/tools/clippy/clippy_lints/src/default.rs index db8f2171348f7..cde27d3ad2a0c 100644 --- a/src/tools/clippy/clippy_lints/src/default.rs +++ b/src/tools/clippy/clippy_lints/src/default.rs @@ -1,5 +1,6 @@ use clippy_utils::diagnostics::{span_lint_and_note, span_lint_and_sugg}; use clippy_utils::source::snippet_with_macro_callsite; +use clippy_utils::ty::{has_drop, is_copy}; use clippy_utils::{any_parent_is_automatically_derived, contains_name, in_macro, match_def_path, paths}; use if_chain::if_chain; use rustc_data_structures::fx::FxHashSet; @@ -139,6 +140,13 @@ impl LateLintPass<'_> for Default { .fields .iter() .all(|field| field.vis.is_accessible_from(module_did, cx.tcx)); + let all_fields_are_copy = variant + .fields + .iter() + .all(|field| { + is_copy(cx, cx.tcx.type_of(field.did)) + }); + if !has_drop(cx, binding_type) || all_fields_are_copy; then { (local, variant, ident.name, binding_type, expr.span) } else { diff --git a/src/tools/clippy/clippy_lints/src/disallowed_type.rs b/src/tools/clippy/clippy_lints/src/disallowed_type.rs index 87124f093a86d..48f781516f422 100644 --- a/src/tools/clippy/clippy_lints/src/disallowed_type.rs +++ b/src/tools/clippy/clippy_lints/src/disallowed_type.rs @@ -1,12 +1,14 @@ -use clippy_utils::diagnostics::span_lint; +use clippy_utils::diagnostics::span_lint_and_then; -use rustc_data_structures::fx::FxHashSet; +use rustc_data_structures::fx::FxHashMap; use rustc_hir::{ def::Res, def_id::DefId, Item, ItemKind, PolyTraitRef, PrimTy, TraitBoundModifier, Ty, TyKind, UseKind, }; use rustc_lint::{LateContext, LateLintPass}; use rustc_session::{declare_tool_lint, impl_lint_pass}; -use rustc_span::{Span, Symbol}; +use rustc_span::Span; + +use crate::utils::conf; declare_clippy_lint! { /// ### What it does @@ -19,7 +21,15 @@ declare_clippy_lint! { /// An example clippy.toml configuration: /// ```toml /// # clippy.toml - /// disallowed-types = ["std::collections::BTreeMap"] + /// disallowed-types = [ + /// # Can use a string as the path of the disallowed type. + /// "std::collections::BTreeMap", + /// # Can also use an inline table with a `path` key. + /// { path = "std::net::TcpListener" }, + /// # When using an inline table, can add a `reason` for why the type + /// # is disallowed. + /// { path = "std::net::Ipv4Addr", reason = "no IPv4 allowed" }, + /// ] /// ``` /// /// ```rust,ignore @@ -38,33 +48,30 @@ declare_clippy_lint! { } #[derive(Clone, Debug)] pub struct DisallowedType { - disallowed: FxHashSet>, - def_ids: FxHashSet, - prim_tys: FxHashSet, + conf_disallowed: Vec, + def_ids: FxHashMap>, + prim_tys: FxHashMap>, } impl DisallowedType { - pub fn new(disallowed: &FxHashSet) -> Self { + pub fn new(conf_disallowed: Vec) -> Self { Self { - disallowed: disallowed - .iter() - .map(|s| s.split("::").map(Symbol::intern).collect::>()) - .collect(), - def_ids: FxHashSet::default(), - prim_tys: FxHashSet::default(), + conf_disallowed, + def_ids: FxHashMap::default(), + prim_tys: FxHashMap::default(), } } fn check_res_emit(&self, cx: &LateContext<'_>, res: &Res, span: Span) { match res { Res::Def(_, did) => { - if self.def_ids.contains(did) { - emit(cx, &cx.tcx.def_path_str(*did), span); + if let Some(reason) = self.def_ids.get(did) { + emit(cx, &cx.tcx.def_path_str(*did), span, reason.as_deref()); } }, Res::PrimTy(prim) => { - if self.prim_tys.contains(prim) { - emit(cx, prim.name_str(), span); + if let Some(reason) = self.prim_tys.get(prim) { + emit(cx, prim.name_str(), span, reason.as_deref()); } }, _ => {}, @@ -76,14 +83,21 @@ impl_lint_pass!(DisallowedType => [DISALLOWED_TYPE]); impl<'tcx> LateLintPass<'tcx> for DisallowedType { fn check_crate(&mut self, cx: &LateContext<'_>) { - for path in &self.disallowed { - let segs = path.iter().map(ToString::to_string).collect::>(); - match clippy_utils::path_to_res(cx, &segs.iter().map(String::as_str).collect::>()) { + for conf in &self.conf_disallowed { + let (path, reason) = match conf { + conf::DisallowedType::Simple(path) => (path, None), + conf::DisallowedType::WithReason { path, reason } => ( + path, + reason.as_ref().map(|reason| format!("{} (from clippy.toml)", reason)), + ), + }; + let segs: Vec<_> = path.split("::").collect(); + match clippy_utils::path_to_res(cx, &segs) { Res::Def(_, id) => { - self.def_ids.insert(id); + self.def_ids.insert(id, reason); }, Res::PrimTy(ty) => { - self.prim_tys.insert(ty); + self.prim_tys.insert(ty, reason); }, _ => {}, } @@ -107,11 +121,16 @@ impl<'tcx> LateLintPass<'tcx> for DisallowedType { } } -fn emit(cx: &LateContext<'_>, name: &str, span: Span) { - span_lint( +fn emit(cx: &LateContext<'_>, name: &str, span: Span, reason: Option<&str>) { + span_lint_and_then( cx, DISALLOWED_TYPE, span, &format!("`{}` is not allowed according to config", name), + |diag| { + if let Some(reason) = reason { + diag.note(reason); + } + }, ); } diff --git a/src/tools/clippy/clippy_lints/src/doc.rs b/src/tools/clippy/clippy_lints/src/doc.rs index 9840affbf6fd8..5511c3ea9b688 100644 --- a/src/tools/clippy/clippy_lints/src/doc.rs +++ b/src/tools/clippy/clippy_lints/src/doc.rs @@ -1,3 +1,4 @@ +use clippy_utils::attrs::is_doc_hidden; use clippy_utils::diagnostics::{span_lint, span_lint_and_help, span_lint_and_note}; use clippy_utils::source::first_line_of_span; use clippy_utils::ty::{implements_trait, is_type_diagnostic_item}; @@ -297,6 +298,17 @@ fn lint_for_missing_headers<'tcx>( if !cx.access_levels.is_exported(def_id) { return; // Private functions do not require doc comments } + + // do not lint if any parent has `#[doc(hidden)]` attribute (#7347) + if cx + .tcx + .hir() + .parent_iter(cx.tcx.hir().local_def_id_to_hir_id(def_id)) + .any(|(id, _node)| is_doc_hidden(cx.tcx.hir().attrs(id))) + { + return; + } + if !headers.safety && sig.header.unsafety == hir::Unsafety::Unsafe { span_lint( cx, diff --git a/src/tools/clippy/clippy_lints/src/eq_op.rs b/src/tools/clippy/clippy_lints/src/eq_op.rs index 51d5094e8c998..655560afd4250 100644 --- a/src/tools/clippy/clippy_lints/src/eq_op.rs +++ b/src/tools/clippy/clippy_lints/src/eq_op.rs @@ -1,7 +1,9 @@ use clippy_utils::diagnostics::{multispan_sugg, span_lint, span_lint_and_then}; use clippy_utils::source::snippet; use clippy_utils::ty::{implements_trait, is_copy}; -use clippy_utils::{ast_utils::is_useless_with_eq_exprs, eq_expr_value, higher, in_macro, is_expn_of}; +use clippy_utils::{ + ast_utils::is_useless_with_eq_exprs, eq_expr_value, higher, in_macro, is_expn_of, is_in_test_function, +}; use if_chain::if_chain; use rustc_errors::Applicability; use rustc_hir::{BinOpKind, BorrowKind, Expr, ExprKind, StmtKind}; @@ -81,7 +83,7 @@ impl<'tcx> LateLintPass<'tcx> for EqOp { if macro_args.len() == 2; let (lhs, rhs) = (macro_args[0], macro_args[1]); if eq_expr_value(cx, lhs, rhs); - + if !is_in_test_function(cx.tcx, e.hir_id); then { span_lint( cx, @@ -108,7 +110,10 @@ impl<'tcx> LateLintPass<'tcx> for EqOp { if macro_with_not_op(&left.kind) || macro_with_not_op(&right.kind) { return; } - if is_useless_with_eq_exprs(op.node.into()) && eq_expr_value(cx, left, right) { + if is_useless_with_eq_exprs(op.node.into()) + && eq_expr_value(cx, left, right) + && !is_in_test_function(cx.tcx, e.hir_id) + { span_lint( cx, EQ_OP, diff --git a/src/tools/clippy/clippy_lints/src/equatable_if_let.rs b/src/tools/clippy/clippy_lints/src/equatable_if_let.rs index 0c6ba91c9430b..e8b1d6f6edaaa 100644 --- a/src/tools/clippy/clippy_lints/src/equatable_if_let.rs +++ b/src/tools/clippy/clippy_lints/src/equatable_if_let.rs @@ -1,5 +1,5 @@ use clippy_utils::diagnostics::span_lint_and_sugg; -use clippy_utils::source::snippet_with_applicability; +use clippy_utils::source::snippet_with_context; use clippy_utils::ty::implements_trait; use if_chain::if_chain; use rustc_errors::Applicability; @@ -77,9 +77,9 @@ impl<'tcx> LateLintPass<'tcx> for PatternEquality { let pat_str = match pat.kind { PatKind::Struct(..) => format!( "({})", - snippet_with_applicability(cx, pat.span, "..", &mut applicability), + snippet_with_context(cx, pat.span, expr.span.ctxt(), "..", &mut applicability).0, ), - _ => snippet_with_applicability(cx, pat.span, "..", &mut applicability).to_string(), + _ => snippet_with_context(cx, pat.span, expr.span.ctxt(), "..", &mut applicability).0.to_string(), }; span_lint_and_sugg( cx, @@ -89,7 +89,7 @@ impl<'tcx> LateLintPass<'tcx> for PatternEquality { "try", format!( "{} == {}", - snippet_with_applicability(cx, exp.span, "..", &mut applicability), + snippet_with_context(cx, exp.span, expr.span.ctxt(), "..", &mut applicability).0, pat_str, ), applicability, diff --git a/src/tools/clippy/clippy_lints/src/format.rs b/src/tools/clippy/clippy_lints/src/format.rs index 37d9ea3bdc117..c22f9d0e17032 100644 --- a/src/tools/clippy/clippy_lints/src/format.rs +++ b/src/tools/clippy/clippy_lints/src/format.rs @@ -1,11 +1,10 @@ use clippy_utils::diagnostics::span_lint_and_sugg; use clippy_utils::higher::FormatExpn; -use clippy_utils::last_path_segment; use clippy_utils::source::{snippet_opt, snippet_with_applicability}; use clippy_utils::sugg::Sugg; use if_chain::if_chain; use rustc_errors::Applicability; -use rustc_hir::{BorrowKind, Expr, ExprKind, QPath}; +use rustc_hir::{Expr, ExprKind}; use rustc_lint::{LateContext, LateLintPass}; use rustc_middle::ty; use rustc_session::{declare_lint_pass, declare_tool_lint}; @@ -69,8 +68,8 @@ impl<'tcx> LateLintPass<'tcx> for UselessFormat { ty::Str => true, _ => false, }; - if format_args.args.iter().all(is_display_arg); - if format_args.fmt_expr.map_or(true, check_unformatted); + if let Some(args) = format_args.args(); + if args.iter().all(|arg| arg.is_display() && !arg.has_string_formatting()); then { let is_new_string = match value.kind { ExprKind::Binary(..) => true, @@ -101,47 +100,3 @@ fn span_useless_format(cx: &LateContext<'_>, span: Span, sugg: String, applicabi applicability, ); } - -fn is_display_arg(expr: &Expr<'_>) -> bool { - if_chain! { - if let ExprKind::Call(_, [_, fmt]) = expr.kind; - if let ExprKind::Path(QPath::Resolved(_, path)) = fmt.kind; - if let [.., t, _] = path.segments; - if t.ident.name == sym::Display; - then { true } else { false } - } -} - -/// Checks if the expression matches -/// ```rust,ignore -/// &[_ { -/// format: _ { -/// width: _::Implied, -/// precision: _::Implied, -/// ... -/// }, -/// ..., -/// }] -/// ``` -fn check_unformatted(expr: &Expr<'_>) -> bool { - if_chain! { - if let ExprKind::AddrOf(BorrowKind::Ref, _, expr) = expr.kind; - if let ExprKind::Array([expr]) = expr.kind; - // struct `core::fmt::rt::v1::Argument` - if let ExprKind::Struct(_, fields, _) = expr.kind; - if let Some(format_field) = fields.iter().find(|f| f.ident.name == sym::format); - // struct `core::fmt::rt::v1::FormatSpec` - if let ExprKind::Struct(_, fields, _) = format_field.expr.kind; - if let Some(precision_field) = fields.iter().find(|f| f.ident.name == sym::precision); - if let ExprKind::Path(ref precision_path) = precision_field.expr.kind; - if last_path_segment(precision_path).ident.name == sym::Implied; - if let Some(width_field) = fields.iter().find(|f| f.ident.name == sym::width); - if let ExprKind::Path(ref width_qpath) = width_field.expr.kind; - if last_path_segment(width_qpath).ident.name == sym::Implied; - then { - return true; - } - } - - false -} diff --git a/src/tools/clippy/clippy_lints/src/format_args.rs b/src/tools/clippy/clippy_lints/src/format_args.rs new file mode 100644 index 0000000000000..8b27442aa9465 --- /dev/null +++ b/src/tools/clippy/clippy_lints/src/format_args.rs @@ -0,0 +1,223 @@ +use clippy_utils::diagnostics::{span_lint_and_sugg, span_lint_and_then}; +use clippy_utils::higher::{FormatArgsArg, FormatArgsExpn, FormatExpn}; +use clippy_utils::source::snippet_opt; +use clippy_utils::ty::implements_trait; +use clippy_utils::{is_diag_trait_item, match_def_path, paths}; +use if_chain::if_chain; +use rustc_errors::Applicability; +use rustc_hir::{Expr, ExprKind}; +use rustc_lint::{LateContext, LateLintPass}; +use rustc_middle::ty::adjustment::{Adjust, Adjustment}; +use rustc_middle::ty::Ty; +use rustc_session::{declare_lint_pass, declare_tool_lint}; +use rustc_span::{sym, BytePos, ExpnData, ExpnKind, Span, Symbol}; + +declare_clippy_lint! { + /// ### What it does + /// Detects `format!` within the arguments of another macro that does + /// formatting such as `format!` itself, `write!` or `println!`. Suggests + /// inlining the `format!` call. + /// + /// ### Why is this bad? + /// The recommended code is both shorter and avoids a temporary allocation. + /// + /// ### Example + /// ```rust + /// # use std::panic::Location; + /// println!("error: {}", format!("something failed at {}", Location::caller())); + /// ``` + /// Use instead: + /// ```rust + /// # use std::panic::Location; + /// println!("error: something failed at {}", Location::caller()); + /// ``` + pub FORMAT_IN_FORMAT_ARGS, + perf, + "`format!` used in a macro that does formatting" +} + +declare_clippy_lint! { + /// ### What it does + /// Checks for [`ToString::to_string`](https://doc.rust-lang.org/std/string/trait.ToString.html#tymethod.to_string) + /// applied to a type that implements [`Display`](https://doc.rust-lang.org/std/fmt/trait.Display.html) + /// in a macro that does formatting. + /// + /// ### Why is this bad? + /// Since the type implements `Display`, the use of `to_string` is + /// unnecessary. + /// + /// ### Example + /// ```rust + /// # use std::panic::Location; + /// println!("error: something failed at {}", Location::caller().to_string()); + /// ``` + /// Use instead: + /// ```rust + /// # use std::panic::Location; + /// println!("error: something failed at {}", Location::caller()); + /// ``` + pub TO_STRING_IN_FORMAT_ARGS, + perf, + "`to_string` applied to a type that implements `Display` in format args" +} + +declare_lint_pass!(FormatArgs => [FORMAT_IN_FORMAT_ARGS, TO_STRING_IN_FORMAT_ARGS]); + +const FORMAT_MACRO_PATHS: &[&[&str]] = &[ + &paths::FORMAT_ARGS_MACRO, + &paths::ASSERT_EQ_MACRO, + &paths::ASSERT_MACRO, + &paths::ASSERT_NE_MACRO, + &paths::EPRINT_MACRO, + &paths::EPRINTLN_MACRO, + &paths::PRINT_MACRO, + &paths::PRINTLN_MACRO, + &paths::WRITE_MACRO, + &paths::WRITELN_MACRO, +]; + +const FORMAT_MACRO_DIAG_ITEMS: &[Symbol] = &[sym::format_macro, sym::std_panic_macro]; + +impl<'tcx> LateLintPass<'tcx> for FormatArgs { + fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) { + if_chain! { + if let Some(format_args) = FormatArgsExpn::parse(expr); + let expr_expn_data = expr.span.ctxt().outer_expn_data(); + let outermost_expn_data = outermost_expn_data(expr_expn_data); + if let Some(macro_def_id) = outermost_expn_data.macro_def_id; + if FORMAT_MACRO_PATHS + .iter() + .any(|path| match_def_path(cx, macro_def_id, path)) + || FORMAT_MACRO_DIAG_ITEMS + .iter() + .any(|diag_item| cx.tcx.is_diagnostic_item(*diag_item, macro_def_id)); + if let ExpnKind::Macro(_, name) = outermost_expn_data.kind; + if let Some(args) = format_args.args(); + then { + for (i, arg) in args.iter().enumerate() { + if !arg.is_display() { + continue; + } + if arg.has_string_formatting() { + continue; + } + if is_aliased(&args, i) { + continue; + } + check_format_in_format_args(cx, outermost_expn_data.call_site, name, arg); + check_to_string_in_format_args(cx, name, arg); + } + } + } + } +} + +fn outermost_expn_data(expn_data: ExpnData) -> ExpnData { + if expn_data.call_site.from_expansion() { + outermost_expn_data(expn_data.call_site.ctxt().outer_expn_data()) + } else { + expn_data + } +} + +fn check_format_in_format_args(cx: &LateContext<'_>, call_site: Span, name: Symbol, arg: &FormatArgsArg<'_>) { + if_chain! { + if FormatExpn::parse(arg.value).is_some(); + if !arg.value.span.ctxt().outer_expn_data().call_site.from_expansion(); + then { + span_lint_and_then( + cx, + FORMAT_IN_FORMAT_ARGS, + trim_semicolon(cx, call_site), + &format!("`format!` in `{}!` args", name), + |diag| { + diag.help(&format!( + "combine the `format!(..)` arguments with the outer `{}!(..)` call", + name + )); + diag.help("or consider changing `format!` to `format_args!`"); + }, + ); + } + } +} + +fn check_to_string_in_format_args<'tcx>(cx: &LateContext<'tcx>, name: Symbol, arg: &FormatArgsArg<'tcx>) { + let value = arg.value; + if_chain! { + if !value.span.from_expansion(); + if let ExprKind::MethodCall(_, _, [receiver], _) = value.kind; + if let Some(method_def_id) = cx.typeck_results().type_dependent_def_id(value.hir_id); + if is_diag_trait_item(cx, method_def_id, sym::ToString); + let receiver_ty = cx.typeck_results().expr_ty(receiver); + if let Some(display_trait_id) = cx.tcx.get_diagnostic_item(sym::Display); + if let Some(receiver_snippet) = snippet_opt(cx, receiver.span); + then { + let (n_needed_derefs, target) = count_needed_derefs( + receiver_ty, + cx.typeck_results().expr_adjustments(receiver).iter(), + ); + if implements_trait(cx, target, display_trait_id, &[]) { + if n_needed_derefs == 0 { + span_lint_and_sugg( + cx, + TO_STRING_IN_FORMAT_ARGS, + value.span.with_lo(receiver.span.hi()), + &format!("`to_string` applied to a type that implements `Display` in `{}!` args", name), + "remove this", + String::new(), + Applicability::MachineApplicable, + ); + } else { + span_lint_and_sugg( + cx, + TO_STRING_IN_FORMAT_ARGS, + value.span, + &format!("`to_string` applied to a type that implements `Display` in `{}!` args", name), + "use this", + format!("{:*>width$}{}", "", receiver_snippet, width = n_needed_derefs), + Applicability::MachineApplicable, + ); + } + } + } + } +} + +// Returns true if `args[i]` "refers to" or "is referred to by" another argument. +fn is_aliased(args: &[FormatArgsArg<'_>], i: usize) -> bool { + let value = args[i].value; + args.iter() + .enumerate() + .any(|(j, arg)| i != j && std::ptr::eq(value, arg.value)) +} + +fn trim_semicolon(cx: &LateContext<'_>, span: Span) -> Span { + snippet_opt(cx, span).map_or(span, |snippet| { + let snippet = snippet.trim_end_matches(';'); + span.with_hi(span.lo() + BytePos(u32::try_from(snippet.len()).unwrap())) + }) +} + +fn count_needed_derefs<'tcx, I>(mut ty: Ty<'tcx>, mut iter: I) -> (usize, Ty<'tcx>) +where + I: Iterator>, +{ + let mut n_total = 0; + let mut n_needed = 0; + loop { + if let Some(Adjustment { + kind: Adjust::Deref(overloaded_deref), + target, + }) = iter.next() + { + n_total += 1; + if overloaded_deref.is_some() { + n_needed = n_total; + } + ty = target; + } else { + return (n_needed, ty); + } + } +} diff --git a/src/tools/clippy/clippy_lints/src/functions/mod.rs b/src/tools/clippy/clippy_lints/src/functions/mod.rs index 04fc5887e8e8b..d7c5ec9ba355b 100644 --- a/src/tools/clippy/clippy_lints/src/functions/mod.rs +++ b/src/tools/clippy/clippy_lints/src/functions/mod.rs @@ -91,11 +91,9 @@ declare_clippy_lint! { declare_clippy_lint! { /// ### What it does - /// Checks for a [`#[must_use]`] attribute on + /// Checks for a `#[must_use]` attribute on /// unit-returning functions and methods. /// - /// [`#[must_use]`]: https://doc.rust-lang.org/reference/attributes/diagnostics.html#the-must_use-attribute - /// /// ### Why is this bad? /// Unit values are useless. The attribute is likely /// a remnant of a refactoring that removed the return type. @@ -112,12 +110,10 @@ declare_clippy_lint! { declare_clippy_lint! { /// ### What it does - /// Checks for a [`#[must_use]`] attribute without + /// Checks for a `#[must_use]` attribute without /// further information on functions and methods that return a type already /// marked as `#[must_use]`. /// - /// [`#[must_use]`]: https://doc.rust-lang.org/reference/attributes/diagnostics.html#the-must_use-attribute - /// /// ### Why is this bad? /// The attribute isn't needed. Not using the result /// will already be reported. Alternatively, one can add some text to the @@ -138,11 +134,9 @@ declare_clippy_lint! { declare_clippy_lint! { /// ### What it does /// Checks for public functions that have no - /// [`#[must_use]`] attribute, but return something not already marked + /// `#[must_use]` attribute, but return something not already marked /// must-use, have no mutable arg and mutate no statics. /// - /// [`#[must_use]`]: https://doc.rust-lang.org/reference/attributes/diagnostics.html#the-must_use-attribute - /// /// ### Why is this bad? /// Not bad at all, this lint just shows places where /// you could add the attribute. diff --git a/src/tools/clippy/clippy_lints/src/identity_op.rs b/src/tools/clippy/clippy_lints/src/identity_op.rs index 73bdd67ff5d25..414f465c49414 100644 --- a/src/tools/clippy/clippy_lints/src/identity_op.rs +++ b/src/tools/clippy/clippy_lints/src/identity_op.rs @@ -66,7 +66,6 @@ fn is_allowed(cx: &LateContext<'_>, cmp: BinOp, left: &Expr<'_>, right: &Expr<'_ && constant_simple(cx, cx.typeck_results(), left) == Some(Constant::Int(1)) } -#[allow(clippy::cast_possible_wrap)] fn check(cx: &LateContext<'_>, e: &Expr<'_>, m: i8, span: Span, arg: Span) { if let Some(Constant::Int(v)) = constant_simple(cx, cx.typeck_results(), e) { let check = match *cx.typeck_results().expr_ty(e).kind() { diff --git a/src/tools/clippy/clippy_lints/src/if_then_panic.rs b/src/tools/clippy/clippy_lints/src/if_then_panic.rs index 10bca59e6d06a..e8cea5529e889 100644 --- a/src/tools/clippy/clippy_lints/src/if_then_panic.rs +++ b/src/tools/clippy/clippy_lints/src/if_then_panic.rs @@ -78,10 +78,10 @@ impl LateLintPass<'_> for IfThenPanic { if let Expr{kind: ExprKind::Unary(UnOp::Not, not_expr), ..} = e { sugg::Sugg::hir_with_applicability(cx, not_expr, "..", &mut applicability).maybe_par().to_string() } else { - format!("!{}", sugg::Sugg::hir_with_applicability(cx, e, "..", &mut applicability).maybe_par().to_string()) + format!("!{}", sugg::Sugg::hir_with_applicability(cx, e, "..", &mut applicability).maybe_par()) } } else { - format!("!{}", sugg::Sugg::hir_with_applicability(cx, cond, "..", &mut applicability).maybe_par().to_string()) + format!("!{}", sugg::Sugg::hir_with_applicability(cx, cond, "..", &mut applicability).maybe_par()) }; span_lint_and_sugg( diff --git a/src/tools/clippy/clippy_lints/src/implicit_saturating_sub.rs b/src/tools/clippy/clippy_lints/src/implicit_saturating_sub.rs index 79d4d7ddcbced..a4f60ded3a6e0 100644 --- a/src/tools/clippy/clippy_lints/src/implicit_saturating_sub.rs +++ b/src/tools/clippy/clippy_lints/src/implicit_saturating_sub.rs @@ -43,7 +43,7 @@ impl<'tcx> LateLintPass<'tcx> for ImplicitSaturatingSub { return; } if_chain! { - if let Some(higher::If { cond, then, .. }) = higher::If::hir(expr); + if let Some(higher::If { cond, then, r#else: None }) = higher::If::hir(expr); // Check if the conditional expression is a binary operation if let ExprKind::Binary(ref cond_op, cond_left, cond_right) = cond.kind; diff --git a/src/tools/clippy/clippy_lints/src/inherent_to_string.rs b/src/tools/clippy/clippy_lints/src/inherent_to_string.rs index 3c40ca50a0981..61dd0eb4af7ed 100644 --- a/src/tools/clippy/clippy_lints/src/inherent_to_string.rs +++ b/src/tools/clippy/clippy_lints/src/inherent_to_string.rs @@ -138,10 +138,10 @@ fn show_lint(cx: &LateContext<'_>, item: &ImplItem<'_>) { item.span, &format!( "type `{}` implements inherent method `to_string(&self) -> String` which shadows the implementation of `Display`", - self_type.to_string() + self_type ), None, - &format!("remove the inherent method from type `{}`", self_type.to_string()), + &format!("remove the inherent method from type `{}`", self_type), ); } else { span_lint_and_help( @@ -150,10 +150,10 @@ fn show_lint(cx: &LateContext<'_>, item: &ImplItem<'_>) { item.span, &format!( "implementation of inherent method `to_string(&self) -> String` for type `{}`", - self_type.to_string() + self_type ), None, - &format!("implement trait `Display` for type `{}` instead", self_type.to_string()), + &format!("implement trait `Display` for type `{}` instead", self_type), ); } } diff --git a/src/tools/clippy/clippy_lints/src/let_underscore.rs b/src/tools/clippy/clippy_lints/src/let_underscore.rs index 89146b4dd2c9b..9efd7aba7e83b 100644 --- a/src/tools/clippy/clippy_lints/src/let_underscore.rs +++ b/src/tools/clippy/clippy_lints/src/let_underscore.rs @@ -10,12 +10,11 @@ use rustc_session::{declare_lint_pass, declare_tool_lint}; declare_clippy_lint! { /// ### What it does - /// Checks for `let _ = ` - /// where expr is #[must_use] + /// Checks for `let _ = ` where expr is `#[must_use]` /// /// ### Why is this bad? - /// It's better to explicitly - /// handle the value of a #[must_use] expr + /// It's better to explicitly handle the value of a `#[must_use]` + /// expr /// /// ### Example /// ```rust diff --git a/src/tools/clippy/clippy_lints/src/lib.register_all.rs b/src/tools/clippy/clippy_lints/src/lib.register_all.rs index 6a3ee35b41a4b..c949ee23ecc7a 100644 --- a/src/tools/clippy/clippy_lints/src/lib.register_all.rs +++ b/src/tools/clippy/clippy_lints/src/lib.register_all.rs @@ -60,6 +60,8 @@ store.register_group(true, "clippy::all", Some("clippy_all"), vec![ LintId::of(float_equality_without_abs::FLOAT_EQUALITY_WITHOUT_ABS), LintId::of(float_literal::EXCESSIVE_PRECISION), LintId::of(format::USELESS_FORMAT), + LintId::of(format_args::FORMAT_IN_FORMAT_ARGS), + LintId::of(format_args::TO_STRING_IN_FORMAT_ARGS), LintId::of(formatting::POSSIBLE_MISSING_COMMA), LintId::of(formatting::SUSPICIOUS_ASSIGNMENT_FORMATTING), LintId::of(formatting::SUSPICIOUS_ELSE_FORMATTING), @@ -118,6 +120,7 @@ store.register_group(true, "clippy::all", Some("clippy_all"), vec![ LintId::of(map_unit_fn::OPTION_MAP_UNIT_FN), LintId::of(map_unit_fn::RESULT_MAP_UNIT_FN), LintId::of(match_result_ok::MATCH_RESULT_OK), + LintId::of(match_str_case_mismatch::MATCH_STR_CASE_MISMATCH), LintId::of(matches::INFALLIBLE_DESTRUCTURING_MATCH), LintId::of(matches::MATCH_AS_REF), LintId::of(matches::MATCH_LIKE_MATCHES_MACRO), @@ -265,6 +268,7 @@ store.register_group(true, "clippy::all", Some("clippy_all"), vec![ LintId::of(transmute::TRANSMUTE_INT_TO_BOOL), LintId::of(transmute::TRANSMUTE_INT_TO_CHAR), LintId::of(transmute::TRANSMUTE_INT_TO_FLOAT), + LintId::of(transmute::TRANSMUTE_NUM_TO_BYTES), LintId::of(transmute::TRANSMUTE_PTR_TO_REF), LintId::of(transmute::UNSOUND_COLLECTION_TRANSMUTE), LintId::of(transmute::WRONG_TRANSMUTE), @@ -277,6 +281,7 @@ store.register_group(true, "clippy::all", Some("clippy_all"), vec![ LintId::of(types::VEC_BOX), LintId::of(undropped_manually_drops::UNDROPPED_MANUALLY_DROPS), LintId::of(unicode::INVISIBLE_CHARACTERS), + LintId::of(uninit_vec::UNINIT_VEC), LintId::of(unit_return_expecting_ord::UNIT_RETURN_EXPECTING_ORD), LintId::of(unit_types::UNIT_ARG), LintId::of(unit_types::UNIT_CMP), diff --git a/src/tools/clippy/clippy_lints/src/lib.register_complexity.rs b/src/tools/clippy/clippy_lints/src/lib.register_complexity.rs index 64b82fc0faac8..c51341bdf0c23 100644 --- a/src/tools/clippy/clippy_lints/src/lib.register_complexity.rs +++ b/src/tools/clippy/clippy_lints/src/lib.register_complexity.rs @@ -82,6 +82,7 @@ store.register_group(true, "clippy::complexity", Some("clippy_complexity"), vec! LintId::of(transmute::TRANSMUTE_INT_TO_BOOL), LintId::of(transmute::TRANSMUTE_INT_TO_CHAR), LintId::of(transmute::TRANSMUTE_INT_TO_FLOAT), + LintId::of(transmute::TRANSMUTE_NUM_TO_BYTES), LintId::of(transmute::TRANSMUTE_PTR_TO_REF), LintId::of(types::BORROWED_BOX), LintId::of(types::TYPE_COMPLEXITY), diff --git a/src/tools/clippy/clippy_lints/src/lib.register_correctness.rs b/src/tools/clippy/clippy_lints/src/lib.register_correctness.rs index bbe47a0e772f7..ff56a6081fb57 100644 --- a/src/tools/clippy/clippy_lints/src/lib.register_correctness.rs +++ b/src/tools/clippy/clippy_lints/src/lib.register_correctness.rs @@ -36,6 +36,7 @@ store.register_group(true, "clippy::correctness", Some("clippy_correctness"), ve LintId::of(loops::ITER_NEXT_LOOP), LintId::of(loops::NEVER_LOOP), LintId::of(loops::WHILE_IMMUTABLE_CONDITION), + LintId::of(match_str_case_mismatch::MATCH_STR_CASE_MISMATCH), LintId::of(mem_replace::MEM_REPLACE_WITH_UNINIT), LintId::of(methods::CLONE_DOUBLE_REF), LintId::of(methods::ITERATOR_STEP_BY_ZERO), @@ -62,6 +63,7 @@ store.register_group(true, "clippy::correctness", Some("clippy_correctness"), ve LintId::of(transmuting_null::TRANSMUTING_NULL), LintId::of(undropped_manually_drops::UNDROPPED_MANUALLY_DROPS), LintId::of(unicode::INVISIBLE_CHARACTERS), + LintId::of(uninit_vec::UNINIT_VEC), LintId::of(unit_return_expecting_ord::UNIT_RETURN_EXPECTING_ORD), LintId::of(unit_types::UNIT_CMP), LintId::of(unnamed_address::FN_ADDRESS_COMPARISONS), diff --git a/src/tools/clippy/clippy_lints/src/lib.register_lints.rs b/src/tools/clippy/clippy_lints/src/lib.register_lints.rs index b0be3b653664c..e8dd3708c8ed4 100644 --- a/src/tools/clippy/clippy_lints/src/lib.register_lints.rs +++ b/src/tools/clippy/clippy_lints/src/lib.register_lints.rs @@ -67,6 +67,7 @@ store.register_lints(&[ casts::CAST_SIGN_LOSS, casts::CHAR_LIT_AS_U8, casts::FN_TO_NUMERIC_CAST, + casts::FN_TO_NUMERIC_CAST_ANY, casts::FN_TO_NUMERIC_CAST_WITH_TRUNCATION, casts::PTR_AS_PTR, casts::UNNECESSARY_CAST, @@ -138,6 +139,8 @@ store.register_lints(&[ floating_point_arithmetic::IMPRECISE_FLOPS, floating_point_arithmetic::SUBOPTIMAL_FLOPS, format::USELESS_FORMAT, + format_args::FORMAT_IN_FORMAT_ARGS, + format_args::TO_STRING_IN_FORMAT_ARGS, formatting::POSSIBLE_MISSING_COMMA, formatting::SUSPICIOUS_ASSIGNMENT_FORMATTING, formatting::SUSPICIOUS_ELSE_FORMATTING, @@ -225,6 +228,7 @@ store.register_lints(&[ map_unit_fn::RESULT_MAP_UNIT_FN, match_on_vec_items::MATCH_ON_VEC_ITEMS, match_result_ok::MATCH_RESULT_OK, + match_str_case_mismatch::MATCH_STR_CASE_MISMATCH, matches::INFALLIBLE_DESTRUCTURING_MATCH, matches::MATCH_AS_REF, matches::MATCH_BOOL, @@ -359,6 +363,7 @@ store.register_lints(&[ neg_multiply::NEG_MULTIPLY, new_without_default::NEW_WITHOUT_DEFAULT, no_effect::NO_EFFECT, + no_effect::NO_EFFECT_UNDERSCORE_BINDING, no_effect::UNNECESSARY_OPERATION, non_copy_const::BORROW_INTERIOR_MUTABLE_CONST, non_copy_const::DECLARE_INTERIOR_MUTABLE_CONST, @@ -438,6 +443,7 @@ store.register_lints(&[ temporary_assignment::TEMPORARY_ASSIGNMENT, to_digit_is_some::TO_DIGIT_IS_SOME, to_string_in_display::TO_STRING_IN_DISPLAY, + trailing_empty_array::TRAILING_EMPTY_ARRAY, trait_bounds::TRAIT_DUPLICATION_IN_BOUNDS, trait_bounds::TYPE_REPETITION_IN_BOUNDS, transmute::CROSSPOINTER_TRANSMUTE, @@ -447,6 +453,7 @@ store.register_lints(&[ transmute::TRANSMUTE_INT_TO_BOOL, transmute::TRANSMUTE_INT_TO_CHAR, transmute::TRANSMUTE_INT_TO_FLOAT, + transmute::TRANSMUTE_NUM_TO_BYTES, transmute::TRANSMUTE_PTR_TO_PTR, transmute::TRANSMUTE_PTR_TO_REF, transmute::UNSOUND_COLLECTION_TRANSMUTE, @@ -463,10 +470,12 @@ store.register_lints(&[ types::REDUNDANT_ALLOCATION, types::TYPE_COMPLEXITY, types::VEC_BOX, + undocumented_unsafe_blocks::UNDOCUMENTED_UNSAFE_BLOCKS, undropped_manually_drops::UNDROPPED_MANUALLY_DROPS, unicode::INVISIBLE_CHARACTERS, unicode::NON_ASCII_LITERAL, unicode::UNICODE_NOT_NFC, + uninit_vec::UNINIT_VEC, unit_return_expecting_ord::UNIT_RETURN_EXPECTING_ORD, unit_types::LET_UNIT_VALUE, unit_types::UNIT_ARG, diff --git a/src/tools/clippy/clippy_lints/src/lib.register_nursery.rs b/src/tools/clippy/clippy_lints/src/lib.register_nursery.rs index 96e0b421094d6..1e54482a8dafd 100644 --- a/src/tools/clippy/clippy_lints/src/lib.register_nursery.rs +++ b/src/tools/clippy/clippy_lints/src/lib.register_nursery.rs @@ -25,6 +25,7 @@ store.register_group(true, "clippy::nursery", Some("clippy_nursery"), vec![ LintId::of(regex::TRIVIAL_REGEX), LintId::of(strings::STRING_LIT_AS_BYTES), LintId::of(suspicious_operation_groupings::SUSPICIOUS_OPERATION_GROUPINGS), + LintId::of(trailing_empty_array::TRAILING_EMPTY_ARRAY), LintId::of(transmute::USELESS_TRANSMUTE), LintId::of(use_self::USE_SELF), ]) diff --git a/src/tools/clippy/clippy_lints/src/lib.register_pedantic.rs b/src/tools/clippy/clippy_lints/src/lib.register_pedantic.rs index 6533b94e82bd5..268349d284811 100644 --- a/src/tools/clippy/clippy_lints/src/lib.register_pedantic.rs +++ b/src/tools/clippy/clippy_lints/src/lib.register_pedantic.rs @@ -72,6 +72,7 @@ store.register_group(true, "clippy::pedantic", Some("clippy_pedantic"), vec![ LintId::of(needless_continue::NEEDLESS_CONTINUE), LintId::of(needless_for_each::NEEDLESS_FOR_EACH), LintId::of(needless_pass_by_value::NEEDLESS_PASS_BY_VALUE), + LintId::of(no_effect::NO_EFFECT_UNDERSCORE_BINDING), LintId::of(non_expressive_names::MANY_SINGLE_CHAR_NAMES), LintId::of(non_expressive_names::SIMILAR_NAMES), LintId::of(pass_by_ref_or_value::LARGE_TYPES_PASSED_BY_VALUE), diff --git a/src/tools/clippy/clippy_lints/src/lib.register_perf.rs b/src/tools/clippy/clippy_lints/src/lib.register_perf.rs index 5432345760bc3..a0d5cf9418e0b 100644 --- a/src/tools/clippy/clippy_lints/src/lib.register_perf.rs +++ b/src/tools/clippy/clippy_lints/src/lib.register_perf.rs @@ -5,6 +5,8 @@ store.register_group(true, "clippy::perf", Some("clippy_perf"), vec![ LintId::of(entry::MAP_ENTRY), LintId::of(escape::BOXED_LOCAL), + LintId::of(format_args::FORMAT_IN_FORMAT_ARGS), + LintId::of(format_args::TO_STRING_IN_FORMAT_ARGS), LintId::of(large_const_arrays::LARGE_CONST_ARRAYS), LintId::of(large_enum_variant::LARGE_ENUM_VARIANT), LintId::of(loops::MANUAL_MEMCPY), diff --git a/src/tools/clippy/clippy_lints/src/lib.register_restriction.rs b/src/tools/clippy/clippy_lints/src/lib.register_restriction.rs index 4463dea5fcb84..3d68a6e900958 100644 --- a/src/tools/clippy/clippy_lints/src/lib.register_restriction.rs +++ b/src/tools/clippy/clippy_lints/src/lib.register_restriction.rs @@ -8,6 +8,7 @@ store.register_group(true, "clippy::restriction", Some("clippy_restriction"), ve LintId::of(as_conversions::AS_CONVERSIONS), LintId::of(asm_syntax::INLINE_ASM_X86_ATT_SYNTAX), LintId::of(asm_syntax::INLINE_ASM_X86_INTEL_SYNTAX), + LintId::of(casts::FN_TO_NUMERIC_CAST_ANY), LintId::of(create_dir::CREATE_DIR), LintId::of(dbg_macro::DBG_MACRO), LintId::of(default_numeric_fallback::DEFAULT_NUMERIC_FALLBACK), @@ -56,6 +57,7 @@ store.register_group(true, "clippy::restriction", Some("clippy_restriction"), ve LintId::of(strings::STR_TO_STRING), LintId::of(types::RC_BUFFER), LintId::of(types::RC_MUTEX), + LintId::of(undocumented_unsafe_blocks::UNDOCUMENTED_UNSAFE_BLOCKS), LintId::of(unnecessary_self_imports::UNNECESSARY_SELF_IMPORTS), LintId::of(unwrap_in_result::UNWRAP_IN_RESULT), LintId::of(verbose_file_reads::VERBOSE_FILE_READS), diff --git a/src/tools/clippy/clippy_lints/src/lib.rs b/src/tools/clippy/clippy_lints/src/lib.rs index 5534f9c94f367..ed7e827702395 100644 --- a/src/tools/clippy/clippy_lints/src/lib.rs +++ b/src/tools/clippy/clippy_lints/src/lib.rs @@ -218,6 +218,7 @@ mod float_equality_without_abs; mod float_literal; mod floating_point_arithmetic; mod format; +mod format_args; mod formatting; mod from_over_into; mod from_str_radix_10; @@ -265,6 +266,7 @@ mod map_err_ignore; mod map_unit_fn; mod match_on_vec_items; mod match_result_ok; +mod match_str_case_mismatch; mod matches; mod mem_forget; mod mem_replace; @@ -352,13 +354,16 @@ mod tabs_in_doc_comments; mod temporary_assignment; mod to_digit_is_some; mod to_string_in_display; +mod trailing_empty_array; mod trait_bounds; mod transmute; mod transmuting_null; mod try_err; mod types; +mod undocumented_unsafe_blocks; mod undropped_manually_drops; mod unicode; +mod uninit_vec; mod unit_return_expecting_ord; mod unit_types; mod unnamed_address; @@ -516,6 +521,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: store.register_late_pass(|| Box::new(blocks_in_if_conditions::BlocksInIfConditions)); store.register_late_pass(|| Box::new(collapsible_match::CollapsibleMatch)); store.register_late_pass(|| Box::new(unicode::Unicode)); + store.register_late_pass(|| Box::new(uninit_vec::UninitVec)); store.register_late_pass(|| Box::new(unit_return_expecting_ord::UnitReturnExpectingOrd)); store.register_late_pass(|| Box::new(strings::StringAdd)); store.register_late_pass(|| Box::new(implicit_return::ImplicitReturn)); @@ -754,8 +760,8 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: store.register_late_pass(|| Box::new(bool_assert_comparison::BoolAssertComparison)); store.register_early_pass(move || Box::new(module_style::ModStyle)); store.register_late_pass(|| Box::new(unused_async::UnusedAsync)); - let disallowed_types = conf.disallowed_types.iter().cloned().collect::>(); - store.register_late_pass(move || Box::new(disallowed_type::DisallowedType::new(&disallowed_types))); + let disallowed_types = conf.disallowed_types.clone(); + store.register_late_pass(move || Box::new(disallowed_type::DisallowedType::new(disallowed_types.clone()))); let import_renames = conf.enforced_import_renames.clone(); store.register_late_pass(move || Box::new(missing_enforced_import_rename::ImportRename::new(import_renames.clone()))); let scripts = conf.allowed_scripts.clone(); @@ -767,6 +773,11 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: store.register_late_pass(move || Box::new(if_then_panic::IfThenPanic)); let enable_raw_pointer_heuristic_for_send = conf.enable_raw_pointer_heuristic_for_send; store.register_late_pass(move || Box::new(non_send_fields_in_send_ty::NonSendFieldInSendTy::new(enable_raw_pointer_heuristic_for_send))); + store.register_late_pass(move || Box::new(undocumented_unsafe_blocks::UndocumentedUnsafeBlocks::default())); + store.register_late_pass(|| Box::new(match_str_case_mismatch::MatchStrCaseMismatch)); + store.register_late_pass(move || Box::new(format_args::FormatArgs)); + store.register_late_pass(|| Box::new(trailing_empty_array::TrailingEmptyArray)); + } #[rustfmt::skip] diff --git a/src/tools/clippy/clippy_lints/src/match_result_ok.rs b/src/tools/clippy/clippy_lints/src/match_result_ok.rs index 3db1f0421ea70..ecf6ad316a461 100644 --- a/src/tools/clippy/clippy_lints/src/match_result_ok.rs +++ b/src/tools/clippy/clippy_lints/src/match_result_ok.rs @@ -35,7 +35,7 @@ declare_clippy_lint! { /// } /// /// if let Ok(value) = iter.next() { - /// vec.push_value) + /// vec.push(value) /// } /// ``` pub MATCH_RESULT_OK, diff --git a/src/tools/clippy/clippy_lints/src/match_str_case_mismatch.rs b/src/tools/clippy/clippy_lints/src/match_str_case_mismatch.rs new file mode 100644 index 0000000000000..a83f38e3d516e --- /dev/null +++ b/src/tools/clippy/clippy_lints/src/match_str_case_mismatch.rs @@ -0,0 +1,171 @@ +use clippy_utils::diagnostics::span_lint_and_sugg; +use clippy_utils::ty::is_type_diagnostic_item; +use rustc_ast::ast::LitKind; +use rustc_errors::Applicability; +use rustc_hir::intravisit::{walk_expr, NestedVisitorMap, Visitor}; +use rustc_hir::{Arm, Expr, ExprKind, MatchSource, PatKind}; +use rustc_lint::{LateContext, LateLintPass}; +use rustc_middle::hir::map::Map; +use rustc_middle::lint::in_external_macro; +use rustc_middle::ty; +use rustc_session::{declare_lint_pass, declare_tool_lint}; +use rustc_span::symbol::SymbolStr; +use rustc_span::{sym, Span}; + +declare_clippy_lint! { + /// ### What it does + /// Checks for `match` expressions modifying the case of a string with non-compliant arms + /// + /// ### Why is this bad? + /// The arm is unreachable, which is likely a mistake + /// + /// ### Example + /// ```rust + /// # let text = "Foo"; + /// + /// match &*text.to_ascii_lowercase() { + /// "foo" => {}, + /// "Bar" => {}, + /// _ => {}, + /// } + /// ``` + /// Use instead: + /// ```rust + /// # let text = "Foo"; + /// + /// match &*text.to_ascii_lowercase() { + /// "foo" => {}, + /// "bar" => {}, + /// _ => {}, + /// } + /// ``` + pub MATCH_STR_CASE_MISMATCH, + correctness, + "creation of a case altering match expression with non-compliant arms" +} + +declare_lint_pass!(MatchStrCaseMismatch => [MATCH_STR_CASE_MISMATCH]); + +#[derive(Debug)] +enum CaseMethod { + LowerCase, + AsciiLowerCase, + UpperCase, + AsciiUppercase, +} + +impl LateLintPass<'_> for MatchStrCaseMismatch { + fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) { + if_chain! { + if !in_external_macro(cx.tcx.sess, expr.span); + if let ExprKind::Match(match_expr, arms, MatchSource::Normal) = expr.kind; + if let ty::Ref(_, ty, _) = cx.typeck_results().expr_ty(match_expr).kind(); + if let ty::Str = ty.kind(); + then { + let mut visitor = MatchExprVisitor { + cx, + case_method: None, + }; + + visitor.visit_expr(match_expr); + + if let Some(case_method) = visitor.case_method { + if let Some((bad_case_span, bad_case_str)) = verify_case(&case_method, arms) { + lint(cx, &case_method, bad_case_span, &bad_case_str); + } + } + } + } + } +} + +struct MatchExprVisitor<'a, 'tcx> { + cx: &'a LateContext<'tcx>, + case_method: Option, +} + +impl<'a, 'tcx> Visitor<'tcx> for MatchExprVisitor<'a, 'tcx> { + type Map = Map<'tcx>; + + fn nested_visit_map(&mut self) -> NestedVisitorMap { + NestedVisitorMap::None + } + + fn visit_expr(&mut self, ex: &'tcx Expr<'_>) { + match ex.kind { + ExprKind::MethodCall(segment, _, [receiver], _) + if self.case_altered(&*segment.ident.as_str(), receiver) => {}, + _ => walk_expr(self, ex), + } + } +} + +impl<'a, 'tcx> MatchExprVisitor<'a, 'tcx> { + fn case_altered(&mut self, segment_ident: &str, receiver: &Expr<'_>) -> bool { + if let Some(case_method) = get_case_method(segment_ident) { + let ty = self.cx.typeck_results().expr_ty(receiver).peel_refs(); + + if is_type_diagnostic_item(self.cx, ty, sym::String) || ty.kind() == &ty::Str { + self.case_method = Some(case_method); + return true; + } + } + + false + } +} + +fn get_case_method(segment_ident_str: &str) -> Option { + match segment_ident_str { + "to_lowercase" => Some(CaseMethod::LowerCase), + "to_ascii_lowercase" => Some(CaseMethod::AsciiLowerCase), + "to_uppercase" => Some(CaseMethod::UpperCase), + "to_ascii_uppercase" => Some(CaseMethod::AsciiUppercase), + _ => None, + } +} + +fn verify_case<'a>(case_method: &'a CaseMethod, arms: &'a [Arm<'_>]) -> Option<(Span, SymbolStr)> { + let case_check = match case_method { + CaseMethod::LowerCase => |input: &str| -> bool { input.chars().all(char::is_lowercase) }, + CaseMethod::AsciiLowerCase => |input: &str| -> bool { input.chars().all(|c| matches!(c, 'a'..='z')) }, + CaseMethod::UpperCase => |input: &str| -> bool { input.chars().all(char::is_uppercase) }, + CaseMethod::AsciiUppercase => |input: &str| -> bool { input.chars().all(|c| matches!(c, 'A'..='Z')) }, + }; + + for arm in arms { + if_chain! { + if let PatKind::Lit(Expr { + kind: ExprKind::Lit(lit), + .. + }) = arm.pat.kind; + if let LitKind::Str(symbol, _) = lit.node; + let input = symbol.as_str(); + if !case_check(&input); + then { + return Some((lit.span, input)); + } + } + } + + None +} + +fn lint(cx: &LateContext<'_>, case_method: &CaseMethod, bad_case_span: Span, bad_case_str: &str) { + let (method_str, suggestion) = match case_method { + CaseMethod::LowerCase => ("to_lower_case", bad_case_str.to_lowercase()), + CaseMethod::AsciiLowerCase => ("to_ascii_lowercase", bad_case_str.to_ascii_lowercase()), + CaseMethod::UpperCase => ("to_uppercase", bad_case_str.to_uppercase()), + CaseMethod::AsciiUppercase => ("to_ascii_uppercase", bad_case_str.to_ascii_uppercase()), + }; + + span_lint_and_sugg( + cx, + MATCH_STR_CASE_MISMATCH, + bad_case_span, + "this `match` arm has a differing case than its expression", + &*format!("consider changing the case of this arm to respect `{}`", method_str), + format!("\"{}\"", suggestion), + Applicability::MachineApplicable, + ); +} diff --git a/src/tools/clippy/clippy_lints/src/matches.rs b/src/tools/clippy/clippy_lints/src/matches.rs index 56d4163a6b343..b643fba5d3288 100644 --- a/src/tools/clippy/clippy_lints/src/matches.rs +++ b/src/tools/clippy/clippy_lints/src/matches.rs @@ -1187,7 +1187,7 @@ where 'b: 'a, I: Clone + Iterator>, { - if !has_only_ref_pats(pats.clone()) { + if !has_multiple_ref_pats(pats.clone()) { return; } @@ -1693,12 +1693,12 @@ fn is_ref_some_arm(cx: &LateContext<'_>, arm: &Arm<'_>) -> Option(pats: I) -> bool +fn has_multiple_ref_pats<'a, 'b, I>(pats: I) -> bool where 'b: 'a, I: Iterator>, { - let mut at_least_one_is_true = false; + let mut ref_count = 0; for opt in pats.map(|pat| match pat.kind { PatKind::Ref(..) => Some(true), // &-patterns PatKind::Wild => Some(false), // an "anything" wildcard is also fine @@ -1706,13 +1706,13 @@ where }) { if let Some(inner) = opt { if inner { - at_least_one_is_true = true; + ref_count += 1; } } else { return false; } } - at_least_one_is_true + ref_count > 1 } pub fn overlapping(ranges: &[SpannedRange]) -> Option<(&SpannedRange, &SpannedRange)> diff --git a/src/tools/clippy/clippy_lints/src/methods/mod.rs b/src/tools/clippy/clippy_lints/src/methods/mod.rs index b26d11c0d6b0d..26c29fbb289cb 100644 --- a/src/tools/clippy/clippy_lints/src/methods/mod.rs +++ b/src/tools/clippy/clippy_lints/src/methods/mod.rs @@ -1777,14 +1777,13 @@ declare_clippy_lint! { } declare_clippy_lint! { - /// **What it does:** Checks for usages of `str::splitn(2, _)` - /// - /// **Why is this bad?** `split_once` is both clearer in intent and slightly more efficient. - /// - /// **Known problems:** None. + /// ### What it does + /// Checks for usages of `str::splitn(2, _)` /// - /// **Example:** + /// ### Why is this bad? + /// `split_once` is both clearer in intent and slightly more efficient. /// + /// ### Example /// ```rust,ignore /// // Bad /// let (key, value) = _.splitn(2, '=').next_tuple()?; diff --git a/src/tools/clippy/clippy_lints/src/methods/uninit_assumed_init.rs b/src/tools/clippy/clippy_lints/src/methods/uninit_assumed_init.rs index 1a5894e48d14c..ce89189bce977 100644 --- a/src/tools/clippy/clippy_lints/src/methods/uninit_assumed_init.rs +++ b/src/tools/clippy/clippy_lints/src/methods/uninit_assumed_init.rs @@ -1,9 +1,8 @@ use clippy_utils::diagnostics::span_lint; -use clippy_utils::{is_expr_path_def_path, match_def_path, paths}; +use clippy_utils::{is_expr_path_def_path, paths, ty::is_uninit_value_valid_for_ty}; use if_chain::if_chain; use rustc_hir as hir; use rustc_lint::LateContext; -use rustc_middle::ty::{self, Ty}; use super::UNINIT_ASSUMED_INIT; @@ -13,7 +12,7 @@ pub(super) fn check(cx: &LateContext<'_>, expr: &hir::Expr<'_>, recv: &hir::Expr if let hir::ExprKind::Call(callee, args) = recv.kind; if args.is_empty(); if is_expr_path_def_path(cx, callee, &paths::MEM_MAYBEUNINIT_UNINIT); - if !is_maybe_uninit_ty_valid(cx, cx.typeck_results().expr_ty_adjusted(expr)); + if !is_uninit_value_valid_for_ty(cx, cx.typeck_results().expr_ty_adjusted(expr)); then { span_lint( cx, @@ -24,12 +23,3 @@ pub(super) fn check(cx: &LateContext<'_>, expr: &hir::Expr<'_>, recv: &hir::Expr } } } - -fn is_maybe_uninit_ty_valid(cx: &LateContext<'_>, ty: Ty<'_>) -> bool { - match ty.kind() { - ty::Array(component, _) => is_maybe_uninit_ty_valid(cx, component), - ty::Tuple(types) => types.types().all(|ty| is_maybe_uninit_ty_valid(cx, ty)), - ty::Adt(adt, _) => match_def_path(cx, adt.did, &paths::MEM_MAYBEUNINIT), - _ => false, - } -} diff --git a/src/tools/clippy/clippy_lints/src/missing_inline.rs b/src/tools/clippy/clippy_lints/src/missing_inline.rs index 667cdd8302528..b593c747498e3 100644 --- a/src/tools/clippy/clippy_lints/src/missing_inline.rs +++ b/src/tools/clippy/clippy_lints/src/missing_inline.rs @@ -8,7 +8,7 @@ use rustc_span::sym; declare_clippy_lint! { /// ### What it does - /// it lints if an exported function, method, trait method with default impl, + /// It lints if an exported function, method, trait method with default impl, /// or trait method impl is not `#[inline]`. /// /// ### Why is this bad? diff --git a/src/tools/clippy/clippy_lints/src/mut_mut.rs b/src/tools/clippy/clippy_lints/src/mut_mut.rs index 610152a217f1e..7c4cac29ba8e8 100644 --- a/src/tools/clippy/clippy_lints/src/mut_mut.rs +++ b/src/tools/clippy/clippy_lints/src/mut_mut.rs @@ -82,6 +82,10 @@ impl<'a, 'tcx> intravisit::Visitor<'tcx> for MutVisitor<'a, 'tcx> { } fn visit_ty(&mut self, ty: &'tcx hir::Ty<'_>) { + if in_external_macro(self.cx.sess(), ty.span) { + return; + } + if let hir::TyKind::Rptr( _, hir::MutTy { diff --git a/src/tools/clippy/clippy_lints/src/no_effect.rs b/src/tools/clippy/clippy_lints/src/no_effect.rs index c5a5cde4b110f..6dae8f320436f 100644 --- a/src/tools/clippy/clippy_lints/src/no_effect.rs +++ b/src/tools/clippy/clippy_lints/src/no_effect.rs @@ -1,9 +1,10 @@ use clippy_utils::diagnostics::{span_lint_hir, span_lint_hir_and_then}; +use clippy_utils::is_lint_allowed; use clippy_utils::source::snippet_opt; use clippy_utils::ty::has_drop; use rustc_errors::Applicability; use rustc_hir::def::{DefKind, Res}; -use rustc_hir::{is_range_literal, BinOpKind, BlockCheckMode, Expr, ExprKind, Stmt, StmtKind, UnsafeSource}; +use rustc_hir::{is_range_literal, BinOpKind, BlockCheckMode, Expr, ExprKind, PatKind, Stmt, StmtKind, UnsafeSource}; use rustc_lint::{LateContext, LateLintPass}; use rustc_session::{declare_lint_pass, declare_tool_lint}; use std::ops::Deref; @@ -13,7 +14,7 @@ declare_clippy_lint! { /// Checks for statements which have no effect. /// /// ### Why is this bad? - /// Similar to dead code, these statements are actually + /// Unlike dead code, these statements are actually /// executed. However, as they have no effect, all they do is make the code less /// readable. /// @@ -26,6 +27,28 @@ declare_clippy_lint! { "statements with no effect" } +declare_clippy_lint! { + /// ### What it does + /// Checks for binding to underscore prefixed variable without side-effects. + /// + /// ### Why is this bad? + /// Unlike dead code, these bindings are actually + /// executed. However, as they have no effect and shouldn't be used further on, all they + /// do is make the code less readable. + /// + /// ### Known problems + /// Further usage of this variable is not checked, which can lead to false positives if it is + /// used later in the code. + /// + /// ### Example + /// ```rust,ignore + /// let _i_serve_no_purpose = 1; + /// ``` + pub NO_EFFECT_UNDERSCORE_BINDING, + pedantic, + "binding to `_` prefixed variable with no side-effect" +} + declare_clippy_lint! { /// ### What it does /// Checks for expression statements that can be reduced to a @@ -44,6 +67,46 @@ declare_clippy_lint! { "outer expressions with no effect" } +declare_lint_pass!(NoEffect => [NO_EFFECT, UNNECESSARY_OPERATION, NO_EFFECT_UNDERSCORE_BINDING]); + +impl<'tcx> LateLintPass<'tcx> for NoEffect { + fn check_stmt(&mut self, cx: &LateContext<'tcx>, stmt: &'tcx Stmt<'_>) { + if check_no_effect(cx, stmt) { + return; + } + check_unnecessary_operation(cx, stmt); + } +} + +fn check_no_effect(cx: &LateContext<'tcx>, stmt: &'tcx Stmt<'_>) -> bool { + if let StmtKind::Semi(expr) = stmt.kind { + if has_no_effect(cx, expr) { + span_lint_hir(cx, NO_EFFECT, expr.hir_id, stmt.span, "statement with no effect"); + return true; + } + } else if let StmtKind::Local(local) = stmt.kind { + if_chain! { + if !is_lint_allowed(cx, NO_EFFECT_UNDERSCORE_BINDING, local.hir_id); + if let Some(init) = local.init; + if !local.pat.span.from_expansion(); + if has_no_effect(cx, init); + if let PatKind::Binding(_, _, ident, _) = local.pat.kind; + if ident.name.to_ident_string().starts_with('_'); + then { + span_lint_hir( + cx, + NO_EFFECT_UNDERSCORE_BINDING, + init.hir_id, + stmt.span, + "binding to `_` prefixed variable with no side-effect" + ); + return true; + } + } + } + false +} + fn has_no_effect(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool { if expr.span.from_expansion() { return false; @@ -88,71 +151,59 @@ fn has_no_effect(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool { } } -declare_lint_pass!(NoEffect => [NO_EFFECT, UNNECESSARY_OPERATION]); - -impl<'tcx> LateLintPass<'tcx> for NoEffect { - fn check_stmt(&mut self, cx: &LateContext<'tcx>, stmt: &'tcx Stmt<'_>) { - if let StmtKind::Semi(expr) = stmt.kind { - if has_no_effect(cx, expr) { - span_lint_hir(cx, NO_EFFECT, expr.hir_id, stmt.span, "statement with no effect"); - } else if let Some(reduced) = reduce_expression(cx, expr) { - for e in &reduced { - if e.span.from_expansion() { - return; - } - } - if let ExprKind::Index(..) = &expr.kind { - let snippet; - if_chain! { - if let Some(arr) = snippet_opt(cx, reduced[0].span); - if let Some(func) = snippet_opt(cx, reduced[1].span); - then { - snippet = format!("assert!({}.len() > {});", &arr, &func); - } else { - return; - } - } - span_lint_hir_and_then( - cx, - UNNECESSARY_OPERATION, - expr.hir_id, - stmt.span, - "unnecessary operation", - |diag| { - diag.span_suggestion( - stmt.span, - "statement can be written as", - snippet, - Applicability::MaybeIncorrect, - ); - }, - ); +fn check_unnecessary_operation(cx: &LateContext<'tcx>, stmt: &'tcx Stmt<'_>) { + if_chain! { + if let StmtKind::Semi(expr) = stmt.kind; + if let Some(reduced) = reduce_expression(cx, expr); + if !&reduced.iter().any(|e| e.span.from_expansion()); + then { + if let ExprKind::Index(..) = &expr.kind { + let snippet; + if let (Some(arr), Some(func)) = (snippet_opt(cx, reduced[0].span), snippet_opt(cx, reduced[1].span)) { + snippet = format!("assert!({}.len() > {});", &arr, &func); } else { - let mut snippet = String::new(); - for e in reduced { - if let Some(snip) = snippet_opt(cx, e.span) { - snippet.push_str(&snip); - snippet.push(';'); - } else { - return; - } + return; + } + span_lint_hir_and_then( + cx, + UNNECESSARY_OPERATION, + expr.hir_id, + stmt.span, + "unnecessary operation", + |diag| { + diag.span_suggestion( + stmt.span, + "statement can be written as", + snippet, + Applicability::MaybeIncorrect, + ); + }, + ); + } else { + let mut snippet = String::new(); + for e in reduced { + if let Some(snip) = snippet_opt(cx, e.span) { + snippet.push_str(&snip); + snippet.push(';'); + } else { + return; } - span_lint_hir_and_then( - cx, - UNNECESSARY_OPERATION, - expr.hir_id, - stmt.span, - "unnecessary operation", - |diag| { - diag.span_suggestion( - stmt.span, - "statement can be reduced to", - snippet, - Applicability::MachineApplicable, - ); - }, - ); } + span_lint_hir_and_then( + cx, + UNNECESSARY_OPERATION, + expr.hir_id, + stmt.span, + "unnecessary operation", + |diag| { + diag.span_suggestion( + stmt.span, + "statement can be reduced to", + snippet, + Applicability::MachineApplicable, + ); + }, + ); } } } diff --git a/src/tools/clippy/clippy_lints/src/ptr.rs b/src/tools/clippy/clippy_lints/src/ptr.rs index d180d6f922710..92a4801a8468a 100644 --- a/src/tools/clippy/clippy_lints/src/ptr.rs +++ b/src/tools/clippy/clippy_lints/src/ptr.rs @@ -139,6 +139,7 @@ declare_clippy_lint! { /// unsafe { std::slice::from_raw_parts(ptr::null(), 0); } /// ``` /// + /// ```ignore /// // Good /// unsafe { std::slice::from_raw_parts(NonNull::dangling().as_ptr(), 0); } /// ``` diff --git a/src/tools/clippy/clippy_lints/src/question_mark.rs b/src/tools/clippy/clippy_lints/src/question_mark.rs index aa6d254e7a544..4d616e26bfc1d 100644 --- a/src/tools/clippy/clippy_lints/src/question_mark.rs +++ b/src/tools/clippy/clippy_lints/src/question_mark.rs @@ -4,10 +4,10 @@ use clippy_utils::is_lang_ctor; use clippy_utils::source::snippet_with_applicability; use clippy_utils::sugg::Sugg; use clippy_utils::ty::is_type_diagnostic_item; -use clippy_utils::{eq_expr_value, path_to_local_id}; +use clippy_utils::{eq_expr_value, path_to_local, path_to_local_id}; use if_chain::if_chain; use rustc_errors::Applicability; -use rustc_hir::LangItem::{OptionNone, OptionSome}; +use rustc_hir::LangItem::{OptionNone, OptionSome, ResultOk}; use rustc_hir::{BindingAnnotation, Block, Expr, ExprKind, PatKind, StmtKind}; use rustc_lint::{LateContext, LateLintPass}; use rustc_session::{declare_lint_pass, declare_tool_lint}; @@ -48,16 +48,20 @@ impl QuestionMark { /// } /// ``` /// + /// ```ignore + /// if result.is_err() { + /// return result; + /// } + /// ``` + /// /// If it matches, it will suggest to use the question mark operator instead - fn check_is_none_and_early_return_none(cx: &LateContext<'_>, expr: &Expr<'_>) { + fn check_is_none_or_err_and_early_return(cx: &LateContext<'_>, expr: &Expr<'_>) { if_chain! { if let Some(higher::If { cond, then, r#else }) = higher::If::hir(expr); if let ExprKind::MethodCall(segment, _, args, _) = &cond.kind; - if segment.ident.name == sym!(is_none); - if Self::expression_returns_none(cx, then); if let Some(subject) = args.get(0); - if Self::is_option(cx, subject); - + if (Self::option_check_and_early_return(cx, subject, then) && segment.ident.name == sym!(is_none)) || + (Self::result_check_and_early_return(cx, subject, then) && segment.ident.name == sym!(is_err)); then { let mut applicability = Applicability::MachineApplicable; let receiver_str = &Sugg::hir_with_applicability(cx, subject, "..", &mut applicability); @@ -95,31 +99,24 @@ impl QuestionMark { } } - fn check_if_let_some_and_early_return_none(cx: &LateContext<'_>, expr: &Expr<'_>) { + fn check_if_let_some_or_err_and_early_return(cx: &LateContext<'_>, expr: &Expr<'_>) { if_chain! { if let Some(higher::IfLet { let_pat, let_expr, if_then, if_else: Some(if_else) }) = higher::IfLet::hir(cx, expr); - if Self::is_option(cx, let_expr); - if let PatKind::TupleStruct(ref path1, fields, None) = let_pat.kind; - if is_lang_ctor(cx, path1, OptionSome); + if (Self::option_check_and_early_return(cx, let_expr, if_else) && is_lang_ctor(cx, path1, OptionSome)) || + (Self::result_check_and_early_return(cx, let_expr, if_else) && is_lang_ctor(cx, path1, ResultOk)); + if let PatKind::Binding(annot, bind_id, _, _) = fields[0].kind; let by_ref = matches!(annot, BindingAnnotation::Ref | BindingAnnotation::RefMut); - if let ExprKind::Block(block, None) = if_then.kind; if block.stmts.is_empty(); if let Some(trailing_expr) = &block.expr; if path_to_local_id(trailing_expr, bind_id); - - if Self::expression_returns_none(cx, if_else); then { let mut applicability = Applicability::MachineApplicable; let receiver_str = snippet_with_applicability(cx, let_expr.span, "..", &mut applicability); - let replacement = format!( - "{}{}?", - receiver_str, - if by_ref { ".as_ref()" } else { "" }, - ); + let replacement = format!("{}{}?", receiver_str, if by_ref { ".as_ref()" } else { "" },); span_lint_and_sugg( cx, @@ -134,6 +131,14 @@ impl QuestionMark { } } + fn result_check_and_early_return(cx: &LateContext<'_>, expr: &Expr<'_>, nested_expr: &Expr<'_>) -> bool { + Self::is_result(cx, expr) && Self::expression_returns_unmodified_err(cx, nested_expr, expr) + } + + fn option_check_and_early_return(cx: &LateContext<'_>, expr: &Expr<'_>, nested_expr: &Expr<'_>) -> bool { + Self::is_option(cx, expr) && Self::expression_returns_none(cx, nested_expr) + } + fn moves_by_default(cx: &LateContext<'_>, expression: &Expr<'_>) -> bool { let expr_ty = cx.typeck_results().expr_ty(expression); @@ -146,6 +151,12 @@ impl QuestionMark { is_type_diagnostic_item(cx, expr_ty, sym::Option) } + fn is_result(cx: &LateContext<'_>, expression: &Expr<'_>) -> bool { + let expr_ty = cx.typeck_results().expr_ty(expression); + + is_type_diagnostic_item(cx, expr_ty, sym::Result) + } + fn expression_returns_none(cx: &LateContext<'_>, expression: &Expr<'_>) -> bool { match expression.kind { ExprKind::Block(block, _) => { @@ -161,6 +172,27 @@ impl QuestionMark { } } + fn expression_returns_unmodified_err( + cx: &LateContext<'_>, + expression: &Expr<'_>, + origin_hir_id: &Expr<'_>, + ) -> bool { + match expression.kind { + ExprKind::Block(block, _) => { + if let Some(return_expression) = Self::return_expression(block) { + return Self::expression_returns_unmodified_err(cx, return_expression, origin_hir_id); + } + + false + }, + ExprKind::Ret(Some(expr)) | ExprKind::Call(expr, _) => { + Self::expression_returns_unmodified_err(cx, expr, origin_hir_id) + }, + ExprKind::Path(_) => path_to_local(expression) == path_to_local(origin_hir_id), + _ => false, + } + } + fn return_expression<'tcx>(block: &Block<'tcx>) -> Option<&'tcx Expr<'tcx>> { // Check if last expression is a return statement. Then, return the expression if_chain! { @@ -189,7 +221,7 @@ impl QuestionMark { impl<'tcx> LateLintPass<'tcx> for QuestionMark { fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) { - Self::check_is_none_and_early_return_none(cx, expr); - Self::check_if_let_some_and_early_return_none(cx, expr); + Self::check_is_none_or_err_and_early_return(cx, expr); + Self::check_if_let_some_or_err_and_early_return(cx, expr); } } diff --git a/src/tools/clippy/clippy_lints/src/semicolon_if_nothing_returned.rs b/src/tools/clippy/clippy_lints/src/semicolon_if_nothing_returned.rs index 6966230156cfa..c0e4914efe0bd 100644 --- a/src/tools/clippy/clippy_lints/src/semicolon_if_nothing_returned.rs +++ b/src/tools/clippy/clippy_lints/src/semicolon_if_nothing_returned.rs @@ -1,7 +1,7 @@ use crate::rustc_lint::LintContext; use clippy_utils::diagnostics::span_lint_and_sugg; use clippy_utils::source::snippet_with_macro_callsite; -use clippy_utils::{in_macro, sugg}; +use clippy_utils::sugg; use if_chain::if_chain; use rustc_errors::Applicability; use rustc_hir::{Block, ExprKind}; @@ -39,7 +39,7 @@ declare_lint_pass!(SemicolonIfNothingReturned => [SEMICOLON_IF_NOTHING_RETURNED] impl LateLintPass<'_> for SemicolonIfNothingReturned { fn check_block(&mut self, cx: &LateContext<'tcx>, block: &'tcx Block<'tcx>) { if_chain! { - if !in_macro(block.span); + if !block.span.from_expansion(); if let Some(expr) = block.expr; let t_expr = cx.typeck_results().expr_ty(expr); if t_expr.is_unit(); diff --git a/src/tools/clippy/clippy_lints/src/shadow.rs b/src/tools/clippy/clippy_lints/src/shadow.rs index 2ca7c18800ee2..64841f33cc385 100644 --- a/src/tools/clippy/clippy_lints/src/shadow.rs +++ b/src/tools/clippy/clippy_lints/src/shadow.rs @@ -162,11 +162,7 @@ fn lint_shadow(cx: &LateContext<'_>, pat: &Pat<'_>, shadowed: HirId, span: Span) (SHADOW_SAME, msg) }, Some(expr) if is_local_used(cx, expr, shadowed) => { - let msg = format!( - "`{}` is shadowed by `{}` which reuses the original value", - snippet(cx, pat.span, "_"), - snippet(cx, expr.span, "..") - ); + let msg = format!("`{}` is shadowed", snippet(cx, pat.span, "_")); (SHADOW_REUSE, msg) }, _ => { diff --git a/src/tools/clippy/clippy_lints/src/suspicious_operation_groupings.rs b/src/tools/clippy/clippy_lints/src/suspicious_operation_groupings.rs index 44d5ff0b63ad5..201aa06782405 100644 --- a/src/tools/clippy/clippy_lints/src/suspicious_operation_groupings.rs +++ b/src/tools/clippy/clippy_lints/src/suspicious_operation_groupings.rs @@ -678,7 +678,7 @@ fn suggestion_with_swapped_ident( Some(format!( "{}{}{}", snippet_with_applicability(cx, expr.span.with_hi(current_ident.span.lo()), "..", applicability), - new_ident.to_string(), + new_ident, snippet_with_applicability(cx, expr.span.with_lo(current_ident.span.hi()), "..", applicability), )) }) diff --git a/src/tools/clippy/clippy_lints/src/trailing_empty_array.rs b/src/tools/clippy/clippy_lints/src/trailing_empty_array.rs new file mode 100644 index 0000000000000..c216a1f81ea54 --- /dev/null +++ b/src/tools/clippy/clippy_lints/src/trailing_empty_array.rs @@ -0,0 +1,77 @@ +use clippy_utils::diagnostics::span_lint_and_help; +use rustc_hir::{HirId, Item, ItemKind}; +use rustc_lint::{LateContext, LateLintPass}; +use rustc_middle::ty::Const; +use rustc_session::{declare_lint_pass, declare_tool_lint}; +use rustc_span::sym; + +declare_clippy_lint! { + /// ### What it does + /// Displays a warning when a struct with a trailing zero-sized array is declared without a `repr` attribute. + /// + /// ### Why is this bad? + /// Zero-sized arrays aren't very useful in Rust itself, so such a struct is likely being created to pass to C code or in some other situation where control over memory layout matters (for example, in conjuction with manual allocation to make it easy to compute the offset of the array). Either way, `#[repr(C)]` (or another `repr` attribute) is needed. + /// + /// ### Example + /// ```rust + /// struct RarelyUseful { + /// some_field: u32, + /// last: [u32; 0], + /// } + /// ``` + /// + /// Use instead: + /// ```rust + /// #[repr(C)] + /// struct MoreOftenUseful { + /// some_field: usize, + /// last: [u32; 0], + /// } + /// ``` + pub TRAILING_EMPTY_ARRAY, + nursery, + "struct with a trailing zero-sized array but without `#[repr(C)]` or another `repr` attribute" +} +declare_lint_pass!(TrailingEmptyArray => [TRAILING_EMPTY_ARRAY]); + +impl<'tcx> LateLintPass<'tcx> for TrailingEmptyArray { + fn check_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx Item<'tcx>) { + if is_struct_with_trailing_zero_sized_array(cx, item) && !has_repr_attr(cx, item.hir_id()) { + span_lint_and_help( + cx, + TRAILING_EMPTY_ARRAY, + item.span, + "trailing zero-sized array in a struct which is not marked with a `repr` attribute", + None, + &format!( + "consider annotating `{}` with `#[repr(C)]` or another `repr` attribute", + cx.tcx.def_path_str(item.def_id.to_def_id()) + ), + ); + } + } +} + +fn is_struct_with_trailing_zero_sized_array(cx: &LateContext<'tcx>, item: &'tcx Item<'tcx>) -> bool { + if_chain! { + // First check if last field is an array + if let ItemKind::Struct(data, _) = &item.kind; + if let Some(last_field) = data.fields().last(); + if let rustc_hir::TyKind::Array(_, length) = last_field.ty.kind; + + // Then check if that that array zero-sized + let length_ldid = cx.tcx.hir().local_def_id(length.hir_id); + let length = Const::from_anon_const(cx.tcx, length_ldid); + let length = length.try_eval_usize(cx.tcx, cx.param_env); + if let Some(length) = length; + then { + length == 0 + } else { + false + } + } +} + +fn has_repr_attr(cx: &LateContext<'_>, hir_id: HirId) -> bool { + cx.tcx.hir().attrs(hir_id).iter().any(|attr| attr.has_name(sym::repr)) +} diff --git a/src/tools/clippy/clippy_lints/src/transmute/mod.rs b/src/tools/clippy/clippy_lints/src/transmute/mod.rs index 33ec9c331ce56..e6acf1a94c929 100644 --- a/src/tools/clippy/clippy_lints/src/transmute/mod.rs +++ b/src/tools/clippy/clippy_lints/src/transmute/mod.rs @@ -3,6 +3,7 @@ mod transmute_float_to_int; mod transmute_int_to_bool; mod transmute_int_to_char; mod transmute_int_to_float; +mod transmute_num_to_bytes; mod transmute_ptr_to_ptr; mod transmute_ptr_to_ref; mod transmute_ref_to_ref; @@ -261,6 +262,28 @@ declare_clippy_lint! { "transmutes from a float to an integer" } +declare_clippy_lint! { + /// ### What it does + /// Checks for transmutes from a number to an array of `u8` + /// + /// ### Why this is bad? + /// Transmutes are dangerous and error-prone, whereas `to_ne_bytes` + /// is intuitive and safe. + /// + /// ### Example + /// ```rust + /// unsafe { + /// let x: [u8; 8] = std::mem::transmute(1i64); + /// } + /// + /// // should be + /// let x: [u8; 8] = 0i64.to_ne_bytes(); + /// ``` + pub TRANSMUTE_NUM_TO_BYTES, + complexity, + "transmutes from a number to an array of `u8`" +} + declare_clippy_lint! { /// ### What it does /// Checks for transmutes from a pointer to a pointer, or @@ -330,6 +353,7 @@ declare_lint_pass!(Transmute => [ TRANSMUTE_INT_TO_BOOL, TRANSMUTE_INT_TO_FLOAT, TRANSMUTE_FLOAT_TO_INT, + TRANSMUTE_NUM_TO_BYTES, UNSOUND_COLLECTION_TRANSMUTE, TRANSMUTES_EXPRESSIBLE_AS_PTR_CASTS, ]); @@ -365,6 +389,7 @@ impl<'tcx> LateLintPass<'tcx> for Transmute { linted |= transmute_int_to_bool::check(cx, e, from_ty, to_ty, args); linted |= transmute_int_to_float::check(cx, e, from_ty, to_ty, args, const_context); linted |= transmute_float_to_int::check(cx, e, from_ty, to_ty, args, const_context); + linted |= transmute_num_to_bytes::check(cx, e, from_ty, to_ty, args, const_context); linted |= unsound_collection_transmute::check(cx, e, from_ty, to_ty); if !linted { diff --git a/src/tools/clippy/clippy_lints/src/transmute/transmute_int_to_char.rs b/src/tools/clippy/clippy_lints/src/transmute/transmute_int_to_char.rs index 8f884e6a4a17b..e83d2e06b9a8d 100644 --- a/src/tools/clippy/clippy_lints/src/transmute/transmute_int_to_char.rs +++ b/src/tools/clippy/clippy_lints/src/transmute/transmute_int_to_char.rs @@ -33,7 +33,7 @@ pub(super) fn check<'tcx>( diag.span_suggestion( e.span, "consider using", - format!("std::char::from_u32({}).unwrap()", arg.to_string()), + format!("std::char::from_u32({}).unwrap()", arg), Applicability::Unspecified, ); }, diff --git a/src/tools/clippy/clippy_lints/src/transmute/transmute_int_to_float.rs b/src/tools/clippy/clippy_lints/src/transmute/transmute_int_to_float.rs index 2b6a4cff81eb5..05eee380d6f40 100644 --- a/src/tools/clippy/clippy_lints/src/transmute/transmute_int_to_float.rs +++ b/src/tools/clippy/clippy_lints/src/transmute/transmute_int_to_float.rs @@ -36,7 +36,7 @@ pub(super) fn check<'tcx>( diag.span_suggestion( e.span, "consider using", - format!("{}::from_bits({})", to_ty, arg.to_string()), + format!("{}::from_bits({})", to_ty, arg), Applicability::Unspecified, ); }, diff --git a/src/tools/clippy/clippy_lints/src/transmute/transmute_num_to_bytes.rs b/src/tools/clippy/clippy_lints/src/transmute/transmute_num_to_bytes.rs new file mode 100644 index 0000000000000..5ba58a7649401 --- /dev/null +++ b/src/tools/clippy/clippy_lints/src/transmute/transmute_num_to_bytes.rs @@ -0,0 +1,49 @@ +use super::TRANSMUTE_NUM_TO_BYTES; +use clippy_utils::diagnostics::span_lint_and_then; +use clippy_utils::sugg; +use rustc_errors::Applicability; +use rustc_hir::Expr; +use rustc_lint::LateContext; +use rustc_middle::ty::{self, Ty, UintTy}; + +/// Checks for `transmute_int_to_float` lint. +/// Returns `true` if it's triggered, otherwise returns `false`. +pub(super) fn check<'tcx>( + cx: &LateContext<'tcx>, + e: &'tcx Expr<'_>, + from_ty: Ty<'tcx>, + to_ty: Ty<'tcx>, + args: &'tcx [Expr<'_>], + const_context: bool, +) -> bool { + match (&from_ty.kind(), &to_ty.kind()) { + (ty::Int(_) | ty::Uint(_) | ty::Float(_), ty::Array(arr_ty, _)) => { + if !matches!(arr_ty.kind(), ty::Uint(UintTy::U8)) { + return false; + } + if matches!(from_ty.kind(), ty::Float(_)) && const_context { + // TODO: Remove when const_float_bits_conv is stabilized + // rust#72447 + return false; + } + + span_lint_and_then( + cx, + TRANSMUTE_NUM_TO_BYTES, + e.span, + &format!("transmute from a `{}` to a `{}`", from_ty, to_ty), + |diag| { + let arg = sugg::Sugg::hir(cx, &args[0], ".."); + diag.span_suggestion( + e.span, + "consider using `to_ne_bytes()`", + format!("{}.to_ne_bytes()", arg), + Applicability::Unspecified, + ); + }, + ); + true + }, + _ => false, + } +} diff --git a/src/tools/clippy/clippy_lints/src/undocumented_unsafe_blocks.rs b/src/tools/clippy/clippy_lints/src/undocumented_unsafe_blocks.rs new file mode 100644 index 0000000000000..e08e4d03c7efe --- /dev/null +++ b/src/tools/clippy/clippy_lints/src/undocumented_unsafe_blocks.rs @@ -0,0 +1,225 @@ +use clippy_utils::diagnostics::{span_lint_and_help, span_lint_and_sugg}; +use clippy_utils::source::{indent_of, reindent_multiline, snippet}; +use clippy_utils::{in_macro, is_lint_allowed}; +use rustc_errors::Applicability; +use rustc_hir::intravisit::{walk_expr, NestedVisitorMap, Visitor}; +use rustc_hir::{Block, BlockCheckMode, Expr, ExprKind, HirId, Local, UnsafeSource}; +use rustc_lexer::TokenKind; +use rustc_lint::{LateContext, LateLintPass}; +use rustc_middle::hir::map::Map; +use rustc_middle::lint::in_external_macro; +use rustc_middle::ty::TyCtxt; +use rustc_session::{declare_tool_lint, impl_lint_pass}; +use rustc_span::{BytePos, Span}; +use std::borrow::Cow; + +declare_clippy_lint! { + /// ### What it does + /// Checks for `unsafe` blocks without a `// Safety: ` comment + /// explaining why the unsafe operations performed inside + /// the block are safe. + /// + /// ### Why is this bad? + /// Undocumented unsafe blocks can make it difficult to + /// read and maintain code, as well as uncover unsoundness + /// and bugs. + /// + /// ### Example + /// ```rust + /// use std::ptr::NonNull; + /// let a = &mut 42; + /// + /// let ptr = unsafe { NonNull::new_unchecked(a) }; + /// ``` + /// Use instead: + /// ```rust + /// use std::ptr::NonNull; + /// let a = &mut 42; + /// + /// // Safety: references are guaranteed to be non-null. + /// let ptr = unsafe { NonNull::new_unchecked(a) }; + /// ``` + pub UNDOCUMENTED_UNSAFE_BLOCKS, + restriction, + "creating an unsafe block without explaining why it is safe" +} + +impl_lint_pass!(UndocumentedUnsafeBlocks => [UNDOCUMENTED_UNSAFE_BLOCKS]); + +#[derive(Default)] +pub struct UndocumentedUnsafeBlocks { + pub local_level: u32, + pub local_span: Option, + // The local was already checked for an overall safety comment + // There is no need to continue checking the blocks in the local + pub local_checked: bool, + // Since we can only check the blocks from expanded macros + // We have to omit the suggestion due to the actual definition + // Not being available to us + pub macro_expansion: bool, +} + +impl LateLintPass<'_> for UndocumentedUnsafeBlocks { + fn check_block(&mut self, cx: &LateContext<'_>, block: &'_ Block<'_>) { + if_chain! { + if !self.local_checked; + if !is_lint_allowed(cx, UNDOCUMENTED_UNSAFE_BLOCKS, block.hir_id); + if !in_external_macro(cx.tcx.sess, block.span); + if let BlockCheckMode::UnsafeBlock(UnsafeSource::UserProvided) = block.rules; + if let Some(enclosing_scope_hir_id) = cx.tcx.hir().get_enclosing_scope(block.hir_id); + if self.block_has_safety_comment(cx.tcx, enclosing_scope_hir_id, block.span) == Some(false); + then { + let mut span = block.span; + + if let Some(local_span) = self.local_span { + span = local_span; + + let result = self.block_has_safety_comment(cx.tcx, enclosing_scope_hir_id, span); + + if result.unwrap_or(true) { + self.local_checked = true; + return; + } + } + + self.lint(cx, span); + } + } + } + + fn check_local(&mut self, cx: &LateContext<'_>, local: &'_ Local<'_>) { + if_chain! { + if !is_lint_allowed(cx, UNDOCUMENTED_UNSAFE_BLOCKS, local.hir_id); + if !in_external_macro(cx.tcx.sess, local.span); + if let Some(init) = local.init; + then { + self.visit_expr(init); + + if self.local_level > 0 { + self.local_span = Some(local.span); + } + } + } + } + + fn check_block_post(&mut self, _: &LateContext<'_>, _: &'_ Block<'_>) { + self.local_level = self.local_level.saturating_sub(1); + + if self.local_level == 0 { + self.local_checked = false; + self.local_span = None; + } + } +} + +impl<'hir> Visitor<'hir> for UndocumentedUnsafeBlocks { + type Map = Map<'hir>; + + fn nested_visit_map(&mut self) -> NestedVisitorMap { + NestedVisitorMap::None + } + + fn visit_expr(&mut self, ex: &'v Expr<'v>) { + match ex.kind { + ExprKind::Block(_, _) => self.local_level = self.local_level.saturating_add(1), + _ => walk_expr(self, ex), + } + } +} + +impl UndocumentedUnsafeBlocks { + fn block_has_safety_comment(&mut self, tcx: TyCtxt<'_>, enclosing_hir_id: HirId, block_span: Span) -> Option { + let map = tcx.hir(); + let source_map = tcx.sess.source_map(); + + let enclosing_scope_span = map.opt_span(enclosing_hir_id)?; + + let between_span = if in_macro(block_span) { + self.macro_expansion = true; + enclosing_scope_span.with_hi(block_span.hi()) + } else { + self.macro_expansion = false; + enclosing_scope_span.to(block_span) + }; + + let file_name = source_map.span_to_filename(between_span); + let source_file = source_map.get_source_file(&file_name)?; + + let lex_start = (between_span.lo().0 + 1) as usize; + let src_str = source_file.src.as_ref()?[lex_start..between_span.hi().0 as usize].to_string(); + + let mut pos = 0; + let mut comment = false; + + for token in rustc_lexer::tokenize(&src_str) { + match token.kind { + TokenKind::LineComment { doc_style: None } + | TokenKind::BlockComment { + doc_style: None, + terminated: true, + } => { + let comment_str = src_str[pos + 2..pos + token.len].to_ascii_uppercase(); + + if comment_str.contains("SAFETY:") { + comment = true; + } + }, + // We need to add all whitespace to `pos` before checking the comment's line number + TokenKind::Whitespace => {}, + _ => { + if comment { + // Get the line number of the "comment" (really wherever the trailing whitespace ended) + let comment_line_num = source_file + .lookup_file_pos_with_col_display(BytePos((lex_start + pos).try_into().unwrap())) + .0; + // Find the block/local's line number + let block_line_num = tcx.sess.source_map().lookup_char_pos(block_span.lo()).line; + + // Check the comment is immediately followed by the block/local + if block_line_num == comment_line_num + 1 || block_line_num == comment_line_num { + return Some(true); + } + + comment = false; + } + }, + } + + pos += token.len; + } + + Some(false) + } + + fn lint(&self, cx: &LateContext<'_>, mut span: Span) { + let source_map = cx.tcx.sess.source_map(); + + if source_map.is_multiline(span) { + span = source_map.span_until_char(span, '\n'); + } + + if self.macro_expansion { + span_lint_and_help( + cx, + UNDOCUMENTED_UNSAFE_BLOCKS, + span, + "unsafe block in macro expansion missing a safety comment", + None, + "consider adding a safety comment in the macro definition", + ); + } else { + let block_indent = indent_of(cx, span); + let suggestion = format!("// Safety: ...\n{}", snippet(cx, span, "..")); + + span_lint_and_sugg( + cx, + UNDOCUMENTED_UNSAFE_BLOCKS, + span, + "unsafe block missing a safety comment", + "consider adding a safety comment", + reindent_multiline(Cow::Borrowed(&suggestion), true, block_indent).to_string(), + Applicability::HasPlaceholders, + ); + } + } +} diff --git a/src/tools/clippy/clippy_lints/src/uninit_vec.rs b/src/tools/clippy/clippy_lints/src/uninit_vec.rs new file mode 100644 index 0000000000000..f3e8b6881058f --- /dev/null +++ b/src/tools/clippy/clippy_lints/src/uninit_vec.rs @@ -0,0 +1,223 @@ +use clippy_utils::diagnostics::{span_lint, span_lint_and_then}; +use clippy_utils::higher::{get_vec_init_kind, VecInitKind}; +use clippy_utils::ty::{is_type_diagnostic_item, is_uninit_value_valid_for_ty}; +use clippy_utils::{is_lint_allowed, path_to_local_id, peel_hir_expr_while, SpanlessEq}; +use rustc_hir::{Block, Expr, ExprKind, HirId, PatKind, PathSegment, Stmt, StmtKind}; +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::{sym, Span}; + +// TODO: add `ReadBuf` (RFC 2930) in "How to fix" once it is available in std +declare_clippy_lint! { + /// ### What it does + /// Checks for `set_len()` call that creates `Vec` with uninitialized elements. + /// This is commonly caused by calling `set_len()` right after allocating or + /// reserving a buffer with `new()`, `default()`, `with_capacity()`, or `reserve()`. + /// + /// ### Why is this bad? + /// It creates a `Vec` with uninitialized data, which leads to + /// undefined behavior with most safe operations. Notably, uninitialized + /// `Vec` must not be used with generic `Read`. + /// + /// Moreover, calling `set_len()` on a `Vec` created with `new()` or `default()` + /// creates out-of-bound values that lead to heap memory corruption when used. + /// + /// ### Known Problems + /// This lint only checks directly adjacent statements. + /// + /// ### Example + /// ```rust,ignore + /// let mut vec: Vec = Vec::with_capacity(1000); + /// unsafe { vec.set_len(1000); } + /// reader.read(&mut vec); // undefined behavior! + /// ``` + /// + /// ### How to fix? + /// 1. Use an initialized buffer: + /// ```rust,ignore + /// let mut vec: Vec = vec![0; 1000]; + /// reader.read(&mut vec); + /// ``` + /// 2. Wrap the content in `MaybeUninit`: + /// ```rust,ignore + /// let mut vec: Vec> = Vec::with_capacity(1000); + /// vec.set_len(1000); // `MaybeUninit` can be uninitialized + /// ``` + /// 3. If you are on nightly, `Vec::spare_capacity_mut()` is available: + /// ```rust,ignore + /// let mut vec: Vec = Vec::with_capacity(1000); + /// let remaining = vec.spare_capacity_mut(); // `&mut [MaybeUninit]` + /// // perform initialization with `remaining` + /// vec.set_len(...); // Safe to call `set_len()` on initialized part + /// ``` + pub UNINIT_VEC, + correctness, + "Vec with uninitialized data" +} + +declare_lint_pass!(UninitVec => [UNINIT_VEC]); + +// FIXME: update to a visitor-based implementation. +// Threads: https://github.com/rust-lang/rust-clippy/pull/7682#discussion_r710998368 +impl<'tcx> LateLintPass<'tcx> for UninitVec { + fn check_block(&mut self, cx: &LateContext<'tcx>, block: &'tcx Block<'_>) { + if !in_external_macro(cx.tcx.sess, block.span) { + for w in block.stmts.windows(2) { + if let StmtKind::Expr(expr) | StmtKind::Semi(expr) = w[1].kind { + handle_uninit_vec_pair(cx, &w[0], expr); + } + } + + if let (Some(stmt), Some(expr)) = (block.stmts.last(), block.expr) { + handle_uninit_vec_pair(cx, stmt, expr); + } + } + } +} + +fn handle_uninit_vec_pair( + cx: &LateContext<'tcx>, + maybe_init_or_reserve: &'tcx Stmt<'tcx>, + maybe_set_len: &'tcx Expr<'tcx>, +) { + if_chain! { + if let Some(vec) = extract_init_or_reserve_target(cx, maybe_init_or_reserve); + if let Some((set_len_self, call_span)) = extract_set_len_self(cx, maybe_set_len); + if vec.location.eq_expr(cx, set_len_self); + if let ty::Ref(_, vec_ty, _) = cx.typeck_results().expr_ty_adjusted(set_len_self).kind(); + if let ty::Adt(_, substs) = vec_ty.kind(); + // `#[allow(...)]` attribute can be set on enclosing unsafe block of `set_len()` + if !is_lint_allowed(cx, UNINIT_VEC, maybe_set_len.hir_id); + then { + if vec.has_capacity() { + // with_capacity / reserve -> set_len + + // Check T of Vec + if !is_uninit_value_valid_for_ty(cx, substs.type_at(0)) { + // FIXME: #7698, false positive of the internal lints + #[allow(clippy::collapsible_span_lint_calls)] + span_lint_and_then( + cx, + UNINIT_VEC, + vec![call_span, maybe_init_or_reserve.span], + "calling `set_len()` immediately after reserving a buffer creates uninitialized values", + |diag| { + diag.help("initialize the buffer or wrap the content in `MaybeUninit`"); + }, + ); + } + } else { + // new / default -> set_len + span_lint( + cx, + UNINIT_VEC, + vec![call_span, maybe_init_or_reserve.span], + "calling `set_len()` on empty `Vec` creates out-of-bound values", + ); + } + } + } +} + +/// The target `Vec` that is initialized or reserved +#[derive(Clone, Copy)] +struct TargetVec<'tcx> { + location: VecLocation<'tcx>, + /// `None` if `reserve()` + init_kind: Option, +} + +impl TargetVec<'_> { + pub fn has_capacity(self) -> bool { + !matches!(self.init_kind, Some(VecInitKind::New | VecInitKind::Default)) + } +} + +#[derive(Clone, Copy)] +enum VecLocation<'tcx> { + Local(HirId), + Expr(&'tcx Expr<'tcx>), +} + +impl<'tcx> VecLocation<'tcx> { + pub fn eq_expr(self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) -> bool { + match self { + VecLocation::Local(hir_id) => path_to_local_id(expr, hir_id), + VecLocation::Expr(self_expr) => SpanlessEq::new(cx).eq_expr(self_expr, expr), + } + } +} + +/// Finds the target location where the result of `Vec` initialization is stored +/// or `self` expression for `Vec::reserve()`. +fn extract_init_or_reserve_target<'tcx>(cx: &LateContext<'tcx>, stmt: &'tcx Stmt<'tcx>) -> Option> { + match stmt.kind { + StmtKind::Local(local) => { + if_chain! { + if let Some(init_expr) = local.init; + if let PatKind::Binding(_, hir_id, _, None) = local.pat.kind; + if let Some(init_kind) = get_vec_init_kind(cx, init_expr); + then { + return Some(TargetVec { + location: VecLocation::Local(hir_id), + init_kind: Some(init_kind), + }) + } + } + }, + StmtKind::Expr(expr) | StmtKind::Semi(expr) => match expr.kind { + ExprKind::Assign(lhs, rhs, _span) => { + if let Some(init_kind) = get_vec_init_kind(cx, rhs) { + return Some(TargetVec { + location: VecLocation::Expr(lhs), + init_kind: Some(init_kind), + }); + } + }, + ExprKind::MethodCall(path, _, [self_expr, _], _) if is_reserve(cx, path, self_expr) => { + return Some(TargetVec { + location: VecLocation::Expr(self_expr), + init_kind: None, + }); + }, + _ => (), + }, + StmtKind::Item(_) => (), + } + None +} + +fn is_reserve(cx: &LateContext<'_>, path: &PathSegment<'_>, self_expr: &Expr<'_>) -> bool { + is_type_diagnostic_item(cx, cx.typeck_results().expr_ty(self_expr).peel_refs(), sym::Vec) + && path.ident.name.as_str() == "reserve" +} + +/// Returns self if the expression is `Vec::set_len()` +fn extract_set_len_self(cx: &LateContext<'_>, expr: &'tcx Expr<'_>) -> Option<(&'tcx Expr<'tcx>, Span)> { + // peel unsafe blocks in `unsafe { vec.set_len() }` + 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) { + (None, Some(expr)) => Some(expr), + (Some(StmtKind::Expr(expr) | StmtKind::Semi(expr)), _) => Some(expr), + _ => None, + } + } else { + None + } + }); + match expr.kind { + ExprKind::MethodCall(path, _, [self_expr, _], _) => { + let self_type = cx.typeck_results().expr_ty(self_expr).peel_refs(); + if is_type_diagnostic_item(cx, self_type, sym::Vec) && path.ident.name.as_str() == "set_len" { + Some((self_expr, expr.span)) + } else { + None + } + }, + _ => None, + } +} diff --git a/src/tools/clippy/clippy_lints/src/unnecessary_sort_by.rs b/src/tools/clippy/clippy_lints/src/unnecessary_sort_by.rs index dd74bf367f3a5..26b56e0f2f316 100644 --- a/src/tools/clippy/clippy_lints/src/unnecessary_sort_by.rs +++ b/src/tools/clippy/clippy_lints/src/unnecessary_sort_by.rs @@ -1,6 +1,6 @@ use clippy_utils::diagnostics::span_lint_and_sugg; use clippy_utils::sugg::Sugg; -use clippy_utils::ty::is_type_diagnostic_item; +use clippy_utils::ty::{implements_trait, is_type_diagnostic_item}; use if_chain::if_chain; use rustc_errors::Applicability; use rustc_hir::{Expr, ExprKind, Mutability, Param, Pat, PatKind, Path, PathSegment, QPath}; @@ -193,10 +193,15 @@ fn detect_lint(cx: &LateContext<'_>, expr: &Expr<'_>) -> Option { let vec_name = Sugg::hir(cx, &args[0], "..").to_string(); let unstable = name == "sort_unstable_by"; + if_chain! { if let ExprKind::Path(QPath::Resolved(_, Path { segments: [PathSegment { ident: left_name, .. }], .. - })) = &left_expr.kind { - if left_name == left_ident { + })) = &left_expr.kind; + if left_name == left_ident; + if cx.tcx.get_diagnostic_item(sym::Ord).map_or(false, |id| { + implements_trait(cx, cx.typeck_results().expr_ty(left_expr), id, &[]) + }); + then { return Some(LintTrigger::Sort(SortDetection { vec_name, unstable })); } } diff --git a/src/tools/clippy/clippy_lints/src/utils/conf.rs b/src/tools/clippy/clippy_lints/src/utils/conf.rs index 6cbada4c1505b..d05c52122d5ee 100644 --- a/src/tools/clippy/clippy_lints/src/utils/conf.rs +++ b/src/tools/clippy/clippy_lints/src/utils/conf.rs @@ -23,6 +23,14 @@ pub enum DisallowedMethod { WithReason { path: String, reason: Option }, } +/// A single disallowed type, used by the `DISALLOWED_TYPE` lint. +#[derive(Clone, Debug, Deserialize)] +#[serde(untagged)] +pub enum DisallowedType { + Simple(String), + WithReason { path: String, reason: Option }, +} + /// Conf with parse errors #[derive(Default)] pub struct TryConf { @@ -255,7 +263,7 @@ define_Conf! { /// Lint: DISALLOWED_TYPE. /// /// The list of disallowed types, written as fully qualified paths. - (disallowed_types: Vec = Vec::new()), + (disallowed_types: Vec = Vec::new()), /// Lint: UNREADABLE_LITERAL. /// /// Should the fraction of a decimal be linted to include separators. diff --git a/src/tools/clippy/clippy_lints/src/utils/internal_lints.rs b/src/tools/clippy/clippy_lints/src/utils/internal_lints.rs index 9f9edbf258ac2..824ec53ab9c75 100644 --- a/src/tools/clippy/clippy_lints/src/utils/internal_lints.rs +++ b/src/tools/clippy/clippy_lints/src/utils/internal_lints.rs @@ -770,8 +770,7 @@ impl<'tcx> LateLintPass<'tcx> for MatchTypeOnDiagItem { let segments: Vec<&str> = segments.iter().map(|sym| &**sym).collect(); if let Some(ty_did) = path_to_res(cx, &segments[..]).opt_def_id(); // Check if the matched type is a diagnostic item - let diag_items = cx.tcx.diagnostic_items(ty_did.krate); - if let Some(item_name) = diag_items.iter().find_map(|(k, v)| if *v == ty_did { Some(k) } else { None }); + if let Some(item_name) = cx.tcx.get_diagnostic_name(ty_did); then { // TODO: check paths constants from external crates. let cx_snippet = snippet(cx, context.span, "_"); diff --git a/src/tools/clippy/clippy_lints/src/vec_init_then_push.rs b/src/tools/clippy/clippy_lints/src/vec_init_then_push.rs index d8e241d72af48..b92b6ca4f4380 100644 --- a/src/tools/clippy/clippy_lints/src/vec_init_then_push.rs +++ b/src/tools/clippy/clippy_lints/src/vec_init_then_push.rs @@ -1,16 +1,14 @@ use clippy_utils::diagnostics::span_lint_and_sugg; +use clippy_utils::higher::{get_vec_init_kind, VecInitKind}; use clippy_utils::source::snippet; -use clippy_utils::ty::is_type_diagnostic_item; -use clippy_utils::{match_def_path, path_to_local, path_to_local_id, paths}; +use clippy_utils::{path_to_local, path_to_local_id}; use if_chain::if_chain; -use rustc_ast::ast::LitKind; use rustc_errors::Applicability; -use rustc_hir::{BindingAnnotation, Block, Expr, ExprKind, HirId, Local, PatKind, QPath, Stmt, StmtKind}; +use rustc_hir::{BindingAnnotation, Block, Expr, ExprKind, HirId, Local, PatKind, Stmt, StmtKind}; use rustc_lint::{LateContext, LateLintPass, LintContext}; use rustc_middle::lint::in_external_macro; use rustc_session::{declare_tool_lint, impl_lint_pass}; -use rustc_span::{symbol::sym, Span}; -use std::convert::TryInto; +use rustc_span::Span; declare_clippy_lint! { /// ### What it does @@ -41,11 +39,6 @@ pub struct VecInitThenPush { searcher: Option, } -#[derive(Clone, Copy)] -enum VecInitKind { - New, - WithCapacity(u64), -} struct VecPushSearcher { local_id: HirId, init: VecInitKind, @@ -58,7 +51,8 @@ impl VecPushSearcher { fn display_err(&self, cx: &LateContext<'_>) { match self.init { _ if self.found == 0 => return, - VecInitKind::WithCapacity(x) if x > self.found => return, + VecInitKind::WithLiteralCapacity(x) if x > self.found => return, + VecInitKind::WithExprCapacity(_) => return, _ => (), }; @@ -152,37 +146,3 @@ impl LateLintPass<'_> for VecInitThenPush { } } } - -fn get_vec_init_kind<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) -> Option { - if let ExprKind::Call(func, args) = expr.kind { - match func.kind { - ExprKind::Path(QPath::TypeRelative(ty, name)) - if is_type_diagnostic_item(cx, cx.typeck_results().node_type(ty.hir_id), sym::Vec) => - { - if name.ident.name == sym::new { - return Some(VecInitKind::New); - } else if name.ident.name.as_str() == "with_capacity" { - return args.get(0).and_then(|arg| { - if_chain! { - if let ExprKind::Lit(lit) = &arg.kind; - if let LitKind::Int(num, _) = lit.node; - then { - Some(VecInitKind::WithCapacity(num.try_into().ok()?)) - } else { - None - } - } - }); - } - } - ExprKind::Path(QPath::Resolved(_, path)) - if match_def_path(cx, path.res.opt_def_id()?, &paths::DEFAULT_TRAIT_METHOD) - && is_type_diagnostic_item(cx, cx.typeck_results().expr_ty(expr), sym::Vec) => - { - return Some(VecInitKind::New); - } - _ => (), - } - } - None -} diff --git a/src/tools/clippy/clippy_utils/Cargo.toml b/src/tools/clippy/clippy_utils/Cargo.toml index e7fca3ae5d401..d99a3d9359e1f 100644 --- a/src/tools/clippy/clippy_utils/Cargo.toml +++ b/src/tools/clippy/clippy_utils/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "clippy_utils" -version = "0.1.57" +version = "0.1.58" edition = "2021" publish = false diff --git a/src/tools/clippy/clippy_utils/src/higher.rs b/src/tools/clippy/clippy_utils/src/higher.rs index 74cf323720cbb..60c4cb361aa6c 100644 --- a/src/tools/clippy/clippy_utils/src/higher.rs +++ b/src/tools/clippy/clippy_utils/src/higher.rs @@ -2,13 +2,16 @@ #![deny(clippy::missing_docs_in_private_items)] -use crate::{is_expn_of, match_def_path, paths}; +use crate::ty::is_type_diagnostic_item; +use crate::{is_expn_of, last_path_segment, match_def_path, paths}; use if_chain::if_chain; use rustc_ast::ast::{self, LitKind}; use rustc_hir as hir; -use rustc_hir::{Arm, Block, BorrowKind, Expr, ExprKind, LoopSource, MatchSource, Node, Pat, StmtKind, UnOp}; +use rustc_hir::{ + Arm, Block, BorrowKind, Expr, ExprKind, HirId, LoopSource, MatchSource, Node, Pat, QPath, StmtKind, UnOp, +}; use rustc_lint::LateContext; -use rustc_span::{sym, ExpnKind, Span, Symbol}; +use rustc_span::{sym, symbol, ExpnKind, Span, Symbol}; /// The essential nodes of a desugared for loop as well as the entire span: /// `for pat in arg { body }` becomes `(pat, arg, body)`. Return `(pat, arg, body, span)`. @@ -569,6 +572,106 @@ impl FormatArgsExpn<'tcx> { } } } + + /// Returns a vector of `FormatArgsArg`. + pub fn args(&self) -> Option>> { + if let Some(expr) = self.fmt_expr { + if_chain! { + if let ExprKind::AddrOf(BorrowKind::Ref, _, expr) = expr.kind; + if let ExprKind::Array(exprs) = expr.kind; + then { + exprs.iter().map(|fmt| { + if_chain! { + // struct `core::fmt::rt::v1::Argument` + if let ExprKind::Struct(_, fields, _) = fmt.kind; + if let Some(position_field) = fields.iter().find(|f| f.ident.name == sym::position); + if let ExprKind::Lit(lit) = &position_field.expr.kind; + if let LitKind::Int(position, _) = lit.node; + then { + let i = usize::try_from(position).unwrap(); + Some(FormatArgsArg { value: self.value_args[i], arg: &self.args[i], fmt: Some(fmt) }) + } else { + None + } + } + }).collect() + } else { + None + } + } + } else { + Some( + self.value_args + .iter() + .zip(self.args.iter()) + .map(|(value, arg)| FormatArgsArg { value, arg, fmt: None }) + .collect(), + ) + } + } +} + +/// Type representing a `FormatArgsExpn`'s format arguments +pub struct FormatArgsArg<'tcx> { + /// An element of `value_args` according to `position` + pub value: &'tcx Expr<'tcx>, + /// An element of `args` according to `position` + pub arg: &'tcx Expr<'tcx>, + /// An element of `fmt_expn` + pub fmt: Option<&'tcx Expr<'tcx>>, +} + +impl<'tcx> FormatArgsArg<'tcx> { + /// Returns true if any formatting parameters are used that would have an effect on strings, + /// like `{:+2}` instead of just `{}`. + pub fn has_string_formatting(&self) -> bool { + self.fmt.map_or(false, |fmt| { + // `!` because these conditions check that `self` is unformatted. + !if_chain! { + // struct `core::fmt::rt::v1::Argument` + if let ExprKind::Struct(_, fields, _) = fmt.kind; + if let Some(format_field) = fields.iter().find(|f| f.ident.name == sym::format); + // struct `core::fmt::rt::v1::FormatSpec` + if let ExprKind::Struct(_, subfields, _) = format_field.expr.kind; + let mut precision_found = false; + let mut width_found = false; + if subfields.iter().all(|field| { + match field.ident.name { + sym::precision => { + precision_found = true; + if let ExprKind::Path(ref precision_path) = field.expr.kind { + last_path_segment(precision_path).ident.name == sym::Implied + } else { + false + } + } + sym::width => { + width_found = true; + if let ExprKind::Path(ref width_qpath) = field.expr.kind { + last_path_segment(width_qpath).ident.name == sym::Implied + } else { + false + } + } + _ => true, + } + }); + if precision_found && width_found; + then { true } else { false } + } + }) + } + + /// Returns true if the argument is formatted using `Display::fmt`. + pub fn is_display(&self) -> bool { + if_chain! { + if let ExprKind::Call(_, [_, format_field]) = self.arg.kind; + if let ExprKind::Path(QPath::Resolved(_, path)) = format_field.kind; + if let [.., t, _] = path.segments; + if t.ident.name == sym::Display; + then { true } else { false } + } + } } /// Checks if a `let` statement is from a `for` loop desugaring. @@ -631,3 +734,51 @@ impl PanicExpn<'tcx> { } } } + +/// A parsed `Vec` initialization expression +#[derive(Clone, Copy)] +pub enum VecInitKind { + /// `Vec::new()` + New, + /// `Vec::default()` or `Default::default()` + Default, + /// `Vec::with_capacity(123)` + WithLiteralCapacity(u64), + /// `Vec::with_capacity(slice.len())` + WithExprCapacity(HirId), +} + +/// Checks if given expression is an initialization of `Vec` and returns its kind. +pub fn get_vec_init_kind<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) -> Option { + if let ExprKind::Call(func, args) = expr.kind { + match func.kind { + ExprKind::Path(QPath::TypeRelative(ty, name)) + if is_type_diagnostic_item(cx, cx.typeck_results().node_type(ty.hir_id), sym::Vec) => + { + if name.ident.name == sym::new { + return Some(VecInitKind::New); + } 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)?; + if_chain! { + if let ExprKind::Lit(lit) = &arg.kind; + if let LitKind::Int(num, _) = lit.node; + then { + return Some(VecInitKind::WithLiteralCapacity(num.try_into().ok()?)) + } + } + return Some(VecInitKind::WithExprCapacity(arg.hir_id)); + } + } + ExprKind::Path(QPath::Resolved(_, path)) + if match_def_path(cx, path.res.opt_def_id()?, &paths::DEFAULT_TRAIT_METHOD) + && is_type_diagnostic_item(cx, cx.typeck_results().expr_ty(expr), sym::Vec) => + { + return Some(VecInitKind::Default); + } + _ => (), + } + } + None +} diff --git a/src/tools/clippy/clippy_utils/src/lib.rs b/src/tools/clippy/clippy_utils/src/lib.rs index 8e94d16a33a0e..9bc380ca6caa6 100644 --- a/src/tools/clippy/clippy_utils/src/lib.rs +++ b/src/tools/clippy/clippy_utils/src/lib.rs @@ -69,11 +69,13 @@ use rustc_hir::def::{DefKind, Res}; use rustc_hir::def_id::DefId; use rustc_hir::hir_id::{HirIdMap, HirIdSet}; use rustc_hir::intravisit::{self, walk_expr, ErasedMap, FnKind, NestedVisitorMap, Visitor}; +use rustc_hir::itemlikevisit::ItemLikeVisitor; use rustc_hir::LangItem::{OptionNone, ResultErr, ResultOk}; use rustc_hir::{ - def, Arm, BindingAnnotation, Block, Body, Constness, Destination, Expr, ExprKind, FnDecl, GenericArgs, HirId, Impl, - ImplItem, ImplItemKind, IsAsync, Item, ItemKind, LangItem, Local, MatchSource, Mutability, Node, Param, Pat, - PatKind, Path, PathSegment, PrimTy, QPath, Stmt, StmtKind, TraitItem, TraitItemKind, TraitRef, TyKind, UnOp, + def, Arm, BindingAnnotation, Block, Body, Constness, Destination, Expr, ExprKind, FnDecl, ForeignItem, GenericArgs, + HirId, Impl, ImplItem, ImplItemKind, IsAsync, Item, ItemKind, LangItem, Local, MatchSource, Mutability, Node, + Param, Pat, PatKind, Path, PathSegment, PrimTy, QPath, Stmt, StmtKind, TraitItem, TraitItemKind, TraitRef, TyKind, + UnOp, }; use rustc_lint::{LateContext, Level, Lint, LintContext}; use rustc_middle::hir::exports::Export; @@ -251,11 +253,7 @@ pub fn is_lang_ctor(cx: &LateContext<'_>, qpath: &QPath<'_>, lang_item: LangItem /// Returns `true` if this `span` was expanded by any macro. #[must_use] pub fn in_macro(span: Span) -> bool { - if span.from_expansion() { - !matches!(span.ctxt().outer_expn_data().kind, ExpnKind::Desugaring(..)) - } else { - false - } + span.from_expansion() && !matches!(span.ctxt().outer_expn_data().kind, ExpnKind::Desugaring(..)) } pub fn is_unit_expr(expr: &Expr<'_>) -> bool { @@ -1285,10 +1283,9 @@ pub fn is_integer_const(cx: &LateContext<'_>, e: &Expr<'_>, value: u128) -> bool } let enclosing_body = cx.tcx.hir().local_def_id(cx.tcx.hir().enclosing_body_owner(e.hir_id)); if let Some((Constant::Int(v), _)) = constant(cx, cx.tcx.typeck(enclosing_body), e) { - value == v - } else { - false + return value == v; } + false } /// Checks whether the given expression is a constant literal of the given value. @@ -1315,7 +1312,7 @@ pub fn is_adjusted(cx: &LateContext<'_>, e: &Expr<'_>) -> bool { /// Returns the pre-expansion span if is this comes from an expansion of the /// macro `name`. -/// See also `is_direct_expn_of`. +/// See also [`is_direct_expn_of`]. #[must_use] pub fn is_expn_of(mut span: Span, name: &str) -> Option { loop { @@ -1338,13 +1335,13 @@ pub fn is_expn_of(mut span: Span, name: &str) -> Option { /// Returns the pre-expansion span if the span directly comes from an expansion /// of the macro `name`. -/// The difference with `is_expn_of` is that in -/// ```rust,ignore +/// The difference with [`is_expn_of`] is that in +/// ```rust +/// # macro_rules! foo { ($e:tt) => { $e } }; macro_rules! bar { ($e:expr) => { $e } } /// foo!(bar!(42)); /// ``` /// `42` is considered expanded from `foo!` and `bar!` by `is_expn_of` but only -/// `bar!` by -/// `is_direct_expn_of`. +/// from `bar!` by `is_direct_expn_of`. #[must_use] pub fn is_direct_expn_of(span: Span, name: &str) -> Option { if span.from_expansion() { @@ -1467,11 +1464,9 @@ pub fn is_self(slf: &Param<'_>) -> bool { } pub fn is_self_ty(slf: &hir::Ty<'_>) -> bool { - if_chain! { - if let TyKind::Path(QPath::Resolved(None, path)) = slf.kind; - if let Res::SelfTy(..) = path.res; - then { - return true + if let TyKind::Path(QPath::Resolved(None, path)) = slf.kind { + if let Res::SelfTy(..) = path.res { + return true; } } false @@ -2062,27 +2057,80 @@ macro_rules! unwrap_cargo_metadata { } pub fn is_hir_ty_cfg_dependant(cx: &LateContext<'_>, ty: &hir::Ty<'_>) -> bool { - if_chain! { - if let TyKind::Path(QPath::Resolved(_, path)) = ty.kind; - if let Res::Def(_, def_id) = path.res; - then { - cx.tcx.has_attr(def_id, sym::cfg) || cx.tcx.has_attr(def_id, sym::cfg_attr) - } else { - false + if let TyKind::Path(QPath::Resolved(_, path)) = ty.kind { + if let Res::Def(_, def_id) = path.res { + return cx.tcx.has_attr(def_id, sym::cfg) || cx.tcx.has_attr(def_id, sym::cfg_attr); } } + false } -/// Checks whether item either has `test` attribute applied, or -/// is a module with `test` in its name. -pub fn is_test_module_or_function(tcx: TyCtxt<'_>, item: &Item<'_>) -> bool { - if let Some(def_id) = tcx.hir().opt_local_def_id(item.hir_id()) { - if tcx.has_attr(def_id.to_def_id(), sym::test) { - return true; +struct VisitConstTestStruct<'tcx> { + tcx: TyCtxt<'tcx>, + names: Vec, + found: bool, +} +impl<'hir> ItemLikeVisitor<'hir> for VisitConstTestStruct<'hir> { + fn visit_item(&mut self, item: &Item<'_>) { + if let ItemKind::Const(ty, _body) = item.kind { + if let TyKind::Path(QPath::Resolved(_, path)) = ty.kind { + // We could also check for the type name `test::TestDescAndFn` + // and the `#[rustc_test_marker]` attribute? + if let Res::Def(DefKind::Struct, _) = path.res { + let has_test_marker = self + .tcx + .hir() + .attrs(item.hir_id()) + .iter() + .any(|a| a.has_name(sym::rustc_test_marker)); + if has_test_marker && self.names.contains(&item.ident.name) { + self.found = true; + } + } + } } } + fn visit_trait_item(&mut self, _: &TraitItem<'_>) {} + fn visit_impl_item(&mut self, _: &ImplItem<'_>) {} + fn visit_foreign_item(&mut self, _: &ForeignItem<'_>) {} +} + +/// Checks if the function containing the given `HirId` is a `#[test]` function +/// +/// Note: If you use this function, please add a `#[test]` case in `tests/ui_test`. +pub fn is_in_test_function(tcx: TyCtxt<'_>, id: hir::HirId) -> bool { + let names: Vec<_> = tcx + .hir() + .parent_iter(id) + // Since you can nest functions we need to collect all until we leave + // function scope + .filter_map(|(_id, node)| { + if let Node::Item(item) = node { + if let ItemKind::Fn(_, _, _) = item.kind { + return Some(item.ident.name); + } + } + None + }) + .collect(); + let parent_mod = tcx.parent_module(id); + let mut vis = VisitConstTestStruct { + tcx, + names, + found: false, + }; + tcx.hir().visit_item_likes_in_module(parent_mod, &mut vis); + vis.found +} - matches!(item.kind, ItemKind::Mod(..)) && item.ident.name.as_str().contains("test") +/// Checks whether item either has `test` attribute appelied, or +/// is a module with `test` in its name. +/// +/// Note: If you use this function, please add a `#[test]` case in `tests/ui_test`. +pub fn is_test_module_or_function(tcx: TyCtxt<'_>, item: &Item<'_>) -> bool { + is_in_test_function(tcx, item.hir_id()) + || matches!(item.kind, ItemKind::Mod(..)) + && item.ident.name.as_str().split('_').any(|a| a == "test" || a == "tests") } macro_rules! op_utils { diff --git a/src/tools/clippy/clippy_utils/src/paths.rs b/src/tools/clippy/clippy_utils/src/paths.rs index 81aff585ded1b..501b08a47f161 100644 --- a/src/tools/clippy/clippy_utils/src/paths.rs +++ b/src/tools/clippy/clippy_utils/src/paths.rs @@ -17,6 +17,12 @@ pub const APPLICABILITY_VALUES: [[&str; 3]; 4] = [ #[cfg(feature = "metadata-collector-lint")] pub const DIAGNOSTIC_BUILDER: [&str; 3] = ["rustc_errors", "diagnostic_builder", "DiagnosticBuilder"]; pub const ARC_PTR_EQ: [&str; 4] = ["alloc", "sync", "Arc", "ptr_eq"]; +#[allow(clippy::invalid_paths)] // `check_path` does not seem to work for macros +pub const ASSERT_EQ_MACRO: [&str; 3] = ["core", "macros", "assert_eq"]; +#[allow(clippy::invalid_paths)] // `check_path` does not seem to work for macros +pub const ASSERT_MACRO: [&str; 4] = ["core", "macros", "builtin", "assert"]; +#[allow(clippy::invalid_paths)] // `check_path` does not seem to work for macros +pub const ASSERT_NE_MACRO: [&str; 3] = ["core", "macros", "assert_ne"]; pub const ASMUT_TRAIT: [&str; 3] = ["core", "convert", "AsMut"]; pub const ASREF_TRAIT: [&str; 3] = ["core", "convert", "AsRef"]; pub(super) const BEGIN_PANIC: [&str; 3] = ["std", "panicking", "begin_panic"]; @@ -41,11 +47,17 @@ pub const DROP: [&str; 3] = ["core", "mem", "drop"]; pub const DURATION: [&str; 3] = ["core", "time", "Duration"]; #[cfg(feature = "internal-lints")] pub const EARLY_CONTEXT: [&str; 2] = ["rustc_lint", "EarlyContext"]; +#[allow(clippy::invalid_paths)] // `check_path` does not seem to work for macros +pub const EPRINT_MACRO: [&str; 3] = ["std", "macros", "eprint"]; +#[allow(clippy::invalid_paths)] // `check_path` does not seem to work for macros +pub const EPRINTLN_MACRO: [&str; 3] = ["std", "macros", "eprintln"]; pub const EXIT: [&str; 3] = ["std", "process", "exit"]; pub const F32_EPSILON: [&str; 4] = ["core", "f32", "", "EPSILON"]; pub const F64_EPSILON: [&str; 4] = ["core", "f64", "", "EPSILON"]; pub const FILE: [&str; 3] = ["std", "fs", "File"]; pub const FILE_TYPE: [&str; 3] = ["std", "fs", "FileType"]; +#[allow(clippy::invalid_paths)] // `check_path` does not seem to work for macros +pub const FORMAT_ARGS_MACRO: [&str; 4] = ["core", "macros", "builtin", "format_args"]; pub const FROM_FROM: [&str; 4] = ["core", "convert", "From", "from"]; pub const FROM_ITERATOR: [&str; 5] = ["core", "iter", "traits", "collect", "FromIterator"]; pub const FROM_ITERATOR_METHOD: [&str; 6] = ["core", "iter", "traits", "collect", "FromIterator", "from_iter"]; @@ -108,6 +120,10 @@ pub const PERMISSIONS_FROM_MODE: [&str; 6] = ["std", "os", "unix", "fs", "Permis pub const POLL: [&str; 4] = ["core", "task", "poll", "Poll"]; pub const POLL_PENDING: [&str; 5] = ["core", "task", "poll", "Poll", "Pending"]; pub const POLL_READY: [&str; 5] = ["core", "task", "poll", "Poll", "Ready"]; +#[allow(clippy::invalid_paths)] // `check_path` does not seem to work for macros +pub const PRINT_MACRO: [&str; 3] = ["std", "macros", "print"]; +#[allow(clippy::invalid_paths)] // `check_path` does not seem to work for macros +pub const PRINTLN_MACRO: [&str; 3] = ["std", "macros", "println"]; pub const PTR_COPY: [&str; 3] = ["core", "intrinsics", "copy"]; pub const PTR_COPY_NONOVERLAPPING: [&str; 3] = ["core", "intrinsics", "copy_nonoverlapping"]; pub const PTR_EQ: [&str; 3] = ["core", "ptr", "eq"]; @@ -184,3 +200,7 @@ pub const VEC_NEW: [&str; 4] = ["alloc", "vec", "Vec", "new"]; pub const VEC_RESIZE: [&str; 4] = ["alloc", "vec", "Vec", "resize"]; pub const WEAK_ARC: [&str; 3] = ["alloc", "sync", "Weak"]; pub const WEAK_RC: [&str; 3] = ["alloc", "rc", "Weak"]; +#[allow(clippy::invalid_paths)] // `check_path` does not seem to work for macros +pub const WRITE_MACRO: [&str; 3] = ["core", "macros", "write"]; +#[allow(clippy::invalid_paths)] // `check_path` does not seem to work for macros +pub const WRITELN_MACRO: [&str; 3] = ["core", "macros", "writeln"]; diff --git a/src/tools/clippy/clippy_utils/src/ty.rs b/src/tools/clippy/clippy_utils/src/ty.rs index 6ebe1a0028a31..ca64ac7de3eea 100644 --- a/src/tools/clippy/clippy_utils/src/ty.rs +++ b/src/tools/clippy/clippy_utils/src/ty.rs @@ -367,3 +367,13 @@ pub fn same_type_and_consts(a: Ty<'tcx>, b: Ty<'tcx>) -> bool { _ => a == b, } } + +/// Checks if a given type looks safe to be uninitialized. +pub fn is_uninit_value_valid_for_ty(cx: &LateContext<'_>, ty: Ty<'_>) -> bool { + match ty.kind() { + ty::Array(component, _) => is_uninit_value_valid_for_ty(cx, component), + ty::Tuple(types) => types.types().all(|ty| is_uninit_value_valid_for_ty(cx, ty)), + ty::Adt(adt, _) => cx.tcx.lang_items().maybe_uninit() == Some(adt.did), + _ => false, + } +} diff --git a/src/tools/clippy/doc/basics.md b/src/tools/clippy/doc/basics.md index ff2e0417435bf..57a90a924ec3c 100644 --- a/src/tools/clippy/doc/basics.md +++ b/src/tools/clippy/doc/basics.md @@ -96,6 +96,7 @@ cargo dev setup git-hook # (experimental) Setup Clippy to work with IntelliJ-Rust cargo dev setup intellij ``` +More about intellij command usage and reasons [here](../CONTRIBUTING.md#intellij-rust) ## lintcheck `cargo lintcheck` will build and run clippy on a fixed set of crates and generate a log of the results. diff --git a/src/tools/clippy/rust-toolchain b/src/tools/clippy/rust-toolchain index f98819303e682..67eaf286004f9 100644 --- a/src/tools/clippy/rust-toolchain +++ b/src/tools/clippy/rust-toolchain @@ -1,3 +1,3 @@ [toolchain] -channel = "nightly-2021-10-07" +channel = "nightly-2021-10-21" components = ["llvm-tools-preview", "rustc-dev", "rust-src"] diff --git a/src/tools/clippy/tests/compile-test.rs b/src/tools/clippy/tests/compile-test.rs index e8b1640c8693e..c15835ef29956 100644 --- a/src/tools/clippy/tests/compile-test.rs +++ b/src/tools/clippy/tests/compile-test.rs @@ -149,6 +149,19 @@ fn run_ui(cfg: &mut compiletest::Config) { compiletest::run_tests(cfg); } +fn run_ui_test(cfg: &mut compiletest::Config) { + cfg.mode = TestMode::Ui; + cfg.src_base = Path::new("tests").join("ui_test"); + let _g = VarGuard::set("CARGO_MANIFEST_DIR", std::fs::canonicalize("tests").unwrap()); + let rustcflags = cfg.target_rustcflags.get_or_insert_with(Default::default); + let len = rustcflags.len(); + rustcflags.push_str(" --test"); + compiletest::run_tests(cfg); + if let Some(ref mut flags) = &mut cfg.target_rustcflags { + flags.truncate(len); + } +} + fn run_internal_tests(cfg: &mut compiletest::Config) { // only run internal tests with the internal-tests feature if !RUN_INTERNAL_TESTS { @@ -312,6 +325,7 @@ fn compile_test() { prepare_env(); let mut config = default_config(); run_ui(&mut config); + run_ui_test(&mut config); run_ui_toml(&mut config); run_ui_cargo(&mut config); run_internal_tests(&mut config); diff --git a/src/tools/clippy/tests/ui-internal/unnecessary_symbol_str.stderr b/src/tools/clippy/tests/ui-internal/unnecessary_symbol_str.stderr index 8e04d447fbcaa..12e05eaa7a09a 100644 --- a/src/tools/clippy/tests/ui-internal/unnecessary_symbol_str.stderr +++ b/src/tools/clippy/tests/ui-internal/unnecessary_symbol_str.stderr @@ -27,12 +27,13 @@ error: unnecessary `Symbol` to string conversion --> $DIR/unnecessary_symbol_str.rs:14:5 | LL | &*Ident::empty().as_str() == "clippy"; - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `Ident::empty().name == rustc_span::sym::clippy` + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `Ident::empty().name == rustc_span::sym::clippy` error: unnecessary `Symbol` to string conversion --> $DIR/unnecessary_symbol_str.rs:15:5 | LL | "clippy" == Ident::empty().to_string(); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `rustc_span::sym::clippy == Ident::empty().name` + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `rustc_span::sym::clippy == Ident::empty().name` error: aborting due to 5 previous errors + diff --git a/src/tools/clippy/tests/ui-toml/toml_disallowed_type/clippy.toml b/src/tools/clippy/tests/ui-toml/toml_disallowed_type/clippy.toml index dac4446703b0f..6cb9e2ef95467 100644 --- a/src/tools/clippy/tests/ui-toml/toml_disallowed_type/clippy.toml +++ b/src/tools/clippy/tests/ui-toml/toml_disallowed_type/clippy.toml @@ -7,5 +7,9 @@ disallowed-types = [ "std::time::Instant", "std::io::Read", "std::primitive::usize", - "bool" + "bool", + # can give path and reason with an inline table + { path = "std::net::Ipv4Addr", reason = "no IPv4 allowed" }, + # can use an inline table but omit reason + { path = "std::net::TcpListener" }, ] diff --git a/src/tools/clippy/tests/ui-toml/toml_disallowed_type/conf_disallowed_type.rs b/src/tools/clippy/tests/ui-toml/toml_disallowed_type/conf_disallowed_type.rs index 0871a3073abd3..410f076505511 100644 --- a/src/tools/clippy/tests/ui-toml/toml_disallowed_type/conf_disallowed_type.rs +++ b/src/tools/clippy/tests/ui-toml/toml_disallowed_type/conf_disallowed_type.rs @@ -25,6 +25,10 @@ struct GenArg([u8; U]); static BAD: foo::atomic::AtomicPtr<()> = foo::atomic::AtomicPtr::new(std::ptr::null_mut()); +fn ip(_: std::net::Ipv4Addr) {} + +fn listener(_: std::net::TcpListener) {} + #[allow(clippy::diverging_sub_expression)] fn main() { let _: std::collections::HashMap<(), ()> = std::collections::HashMap::new(); diff --git a/src/tools/clippy/tests/ui-toml/toml_disallowed_type/conf_disallowed_type.stderr b/src/tools/clippy/tests/ui-toml/toml_disallowed_type/conf_disallowed_type.stderr index 90ce7db2cc4e6..08a400a83675b 100644 --- a/src/tools/clippy/tests/ui-toml/toml_disallowed_type/conf_disallowed_type.stderr +++ b/src/tools/clippy/tests/ui-toml/toml_disallowed_type/conf_disallowed_type.stderr @@ -60,59 +60,73 @@ error: `usize` is not allowed according to config LL | struct GenArg([u8; U]); | ^^^^^ +error: `std::net::Ipv4Addr` is not allowed according to config + --> $DIR/conf_disallowed_type.rs:28:10 + | +LL | fn ip(_: std::net::Ipv4Addr) {} + | ^^^^^^^^^^^^^^^^^^ + | + = note: no IPv4 allowed (from clippy.toml) + +error: `std::net::TcpListener` is not allowed according to config + --> $DIR/conf_disallowed_type.rs:30:16 + | +LL | fn listener(_: std::net::TcpListener) {} + | ^^^^^^^^^^^^^^^^^^^^^ + error: `std::collections::HashMap` is not allowed according to config - --> $DIR/conf_disallowed_type.rs:30:48 + --> $DIR/conf_disallowed_type.rs:34:48 | LL | let _: std::collections::HashMap<(), ()> = std::collections::HashMap::new(); | ^^^^^^^^^^^^^^^^^^^^^^^^^ error: `std::collections::HashMap` is not allowed according to config - --> $DIR/conf_disallowed_type.rs:30:12 + --> $DIR/conf_disallowed_type.rs:34:12 | LL | let _: std::collections::HashMap<(), ()> = std::collections::HashMap::new(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: `std::time::Instant` is not allowed according to config - --> $DIR/conf_disallowed_type.rs:31:13 + --> $DIR/conf_disallowed_type.rs:35:13 | LL | let _ = Sneaky::now(); | ^^^^^^ error: `std::sync::atomic::AtomicU32` is not allowed according to config - --> $DIR/conf_disallowed_type.rs:32:13 + --> $DIR/conf_disallowed_type.rs:36:13 | LL | let _ = foo::atomic::AtomicU32::new(0); | ^^^^^^^^^^^^^^^^^^^^^^ error: `std::sync::atomic::AtomicU32` is not allowed according to config - --> $DIR/conf_disallowed_type.rs:33:17 + --> $DIR/conf_disallowed_type.rs:37:17 | LL | static FOO: std::sync::atomic::AtomicU32 = foo::atomic::AtomicU32::new(1); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: `std::sync::atomic::AtomicU32` is not allowed according to config - --> $DIR/conf_disallowed_type.rs:33:48 + --> $DIR/conf_disallowed_type.rs:37:48 | LL | static FOO: std::sync::atomic::AtomicU32 = foo::atomic::AtomicU32::new(1); | ^^^^^^^^^^^^^^^^^^^^^^ error: `syn::TypePath` is not allowed according to config - --> $DIR/conf_disallowed_type.rs:34:43 + --> $DIR/conf_disallowed_type.rs:38:43 | LL | let _: std::collections::BTreeMap<(), syn::TypePath> = Default::default(); | ^^^^^^^^^^^^^ error: `syn::Ident` is not allowed according to config - --> $DIR/conf_disallowed_type.rs:35:13 + --> $DIR/conf_disallowed_type.rs:39:13 | LL | let _ = syn::Ident::new("", todo!()); | ^^^^^^^^^^ error: `usize` is not allowed according to config - --> $DIR/conf_disallowed_type.rs:37:12 + --> $DIR/conf_disallowed_type.rs:41:12 | LL | let _: usize = 64_usize; | ^^^^^ -error: aborting due to 19 previous errors +error: aborting due to 21 previous errors diff --git a/src/tools/clippy/tests/ui/auxiliary/macro_rules.rs b/src/tools/clippy/tests/ui/auxiliary/macro_rules.rs index 170955e726cc5..0251fada9e85a 100644 --- a/src/tools/clippy/tests/ui/auxiliary/macro_rules.rs +++ b/src/tools/clippy/tests/ui/auxiliary/macro_rules.rs @@ -113,3 +113,10 @@ macro_rules! default_numeric_fallback { let x = 22; }; } + +#[macro_export] +macro_rules! mut_mut { + () => { + let mut_mut_ty: &mut &mut u32 = &mut &mut 1u32; + }; +} diff --git a/src/tools/clippy/tests/ui/cfg_attr_rustfmt.fixed b/src/tools/clippy/tests/ui/cfg_attr_rustfmt.fixed index 4e583a25b94c2..061a4ab9b2ef8 100644 --- a/src/tools/clippy/tests/ui/cfg_attr_rustfmt.fixed +++ b/src/tools/clippy/tests/ui/cfg_attr_rustfmt.fixed @@ -1,7 +1,7 @@ // run-rustfix #![feature(stmt_expr_attributes)] -#![allow(unused, clippy::no_effect)] +#![allow(unused, clippy::no_effect, clippy::unnecessary_operation)] #![warn(clippy::deprecated_cfg_attr)] // This doesn't get linted, see known problems diff --git a/src/tools/clippy/tests/ui/cfg_attr_rustfmt.rs b/src/tools/clippy/tests/ui/cfg_attr_rustfmt.rs index 9c0fcf6fb454c..035169fab85be 100644 --- a/src/tools/clippy/tests/ui/cfg_attr_rustfmt.rs +++ b/src/tools/clippy/tests/ui/cfg_attr_rustfmt.rs @@ -1,7 +1,7 @@ // run-rustfix #![feature(stmt_expr_attributes)] -#![allow(unused, clippy::no_effect)] +#![allow(unused, clippy::no_effect, clippy::unnecessary_operation)] #![warn(clippy::deprecated_cfg_attr)] // This doesn't get linted, see known problems diff --git a/src/tools/clippy/tests/ui/doc_unsafe.rs b/src/tools/clippy/tests/ui/doc_unsafe.rs index 8f823f1672ba2..03bb30f9083ac 100644 --- a/src/tools/clippy/tests/ui/doc_unsafe.rs +++ b/src/tools/clippy/tests/ui/doc_unsafe.rs @@ -115,3 +115,13 @@ fn main() { drive(); } } + +// do not lint if any parent has `#[doc(hidden)]` attribute +// see #7347 +#[doc(hidden)] +pub mod __macro { + pub struct T; + impl T { + pub unsafe fn f() {} + } +} diff --git a/src/tools/clippy/tests/ui/equatable_if_let.fixed b/src/tools/clippy/tests/ui/equatable_if_let.fixed index ba72cc237b4a5..88918d9671e42 100644 --- a/src/tools/clippy/tests/ui/equatable_if_let.fixed +++ b/src/tools/clippy/tests/ui/equatable_if_let.fixed @@ -66,4 +66,13 @@ fn main() { if g == NotStructuralEq::A {} if let Some(NotPartialEq::A) = Some(f) {} if Some(g) == Some(NotStructuralEq::A) {} + + macro_rules! m1 { + (x) => { + "abc" + }; + } + if "abc" == m1!(x) { + println!("OK"); + } } diff --git a/src/tools/clippy/tests/ui/equatable_if_let.rs b/src/tools/clippy/tests/ui/equatable_if_let.rs index 12526ca193db6..9a7ab75ef450f 100644 --- a/src/tools/clippy/tests/ui/equatable_if_let.rs +++ b/src/tools/clippy/tests/ui/equatable_if_let.rs @@ -66,4 +66,13 @@ fn main() { if let NotStructuralEq::A = g {} if let Some(NotPartialEq::A) = Some(f) {} if let Some(NotStructuralEq::A) = Some(g) {} + + macro_rules! m1 { + (x) => { + "abc" + }; + } + if let m1!(x) = "abc" { + println!("OK"); + } } diff --git a/src/tools/clippy/tests/ui/equatable_if_let.stderr b/src/tools/clippy/tests/ui/equatable_if_let.stderr index 79ef919384df2..760ff88f448f0 100644 --- a/src/tools/clippy/tests/ui/equatable_if_let.stderr +++ b/src/tools/clippy/tests/ui/equatable_if_let.stderr @@ -60,5 +60,11 @@ error: this pattern matching can be expressed using equality LL | if let Some(NotStructuralEq::A) = Some(g) {} | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `Some(g) == Some(NotStructuralEq::A)` -error: aborting due to 10 previous errors +error: this pattern matching can be expressed using equality + --> $DIR/equatable_if_let.rs:75:8 + | +LL | if let m1!(x) = "abc" { + | ^^^^^^^^^^^^^^^^^^ help: try: `"abc" == m1!(x)` + +error: aborting due to 11 previous errors diff --git a/src/tools/clippy/tests/ui/expect_fun_call.fixed b/src/tools/clippy/tests/ui/expect_fun_call.fixed index a756d1cf50659..cf923a6a5940c 100644 --- a/src/tools/clippy/tests/ui/expect_fun_call.fixed +++ b/src/tools/clippy/tests/ui/expect_fun_call.fixed @@ -1,6 +1,7 @@ // run-rustfix #![warn(clippy::expect_fun_call)] +#![allow(clippy::to_string_in_format_args)] /// Checks implementation of the `EXPECT_FUN_CALL` lint diff --git a/src/tools/clippy/tests/ui/expect_fun_call.rs b/src/tools/clippy/tests/ui/expect_fun_call.rs index 60bbaa89d4282..e6f252259df70 100644 --- a/src/tools/clippy/tests/ui/expect_fun_call.rs +++ b/src/tools/clippy/tests/ui/expect_fun_call.rs @@ -1,6 +1,7 @@ // run-rustfix #![warn(clippy::expect_fun_call)] +#![allow(clippy::to_string_in_format_args)] /// Checks implementation of the `EXPECT_FUN_CALL` lint diff --git a/src/tools/clippy/tests/ui/expect_fun_call.stderr b/src/tools/clippy/tests/ui/expect_fun_call.stderr index 6dc796f5cee37..ac48a06671cd2 100644 --- a/src/tools/clippy/tests/ui/expect_fun_call.stderr +++ b/src/tools/clippy/tests/ui/expect_fun_call.stderr @@ -1,5 +1,5 @@ error: use of `expect` followed by a function call - --> $DIR/expect_fun_call.rs:28:26 + --> $DIR/expect_fun_call.rs:29:26 | LL | with_none_and_format.expect(&format!("Error {}: fake error", error_code)); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `unwrap_or_else(|| panic!("Error {}: fake error", error_code))` @@ -7,67 +7,67 @@ LL | with_none_and_format.expect(&format!("Error {}: fake error", error_code = note: `-D clippy::expect-fun-call` implied by `-D warnings` error: use of `expect` followed by a function call - --> $DIR/expect_fun_call.rs:31:26 + --> $DIR/expect_fun_call.rs:32:26 | LL | with_none_and_as_str.expect(format!("Error {}: fake error", error_code).as_str()); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `unwrap_or_else(|| panic!("Error {}: fake error", error_code))` error: use of `expect` followed by a function call - --> $DIR/expect_fun_call.rs:41:25 + --> $DIR/expect_fun_call.rs:42:25 | LL | with_err_and_format.expect(&format!("Error {}: fake error", error_code)); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `unwrap_or_else(|_| panic!("Error {}: fake error", error_code))` error: use of `expect` followed by a function call - --> $DIR/expect_fun_call.rs:44:25 + --> $DIR/expect_fun_call.rs:45:25 | LL | with_err_and_as_str.expect(format!("Error {}: fake error", error_code).as_str()); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `unwrap_or_else(|_| panic!("Error {}: fake error", error_code))` error: use of `expect` followed by a function call - --> $DIR/expect_fun_call.rs:56:17 + --> $DIR/expect_fun_call.rs:57:17 | LL | Some("foo").expect(format!("{} {}", 1, 2).as_ref()); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `unwrap_or_else(|| panic!("{} {}", 1, 2))` error: use of `expect` followed by a function call - --> $DIR/expect_fun_call.rs:77:21 + --> $DIR/expect_fun_call.rs:78:21 | LL | Some("foo").expect(&get_string()); | ^^^^^^^^^^^^^^^^^^^^^ help: try this: `unwrap_or_else(|| { panic!("{}", get_string()) })` error: use of `expect` followed by a function call - --> $DIR/expect_fun_call.rs:78:21 + --> $DIR/expect_fun_call.rs:79:21 | LL | Some("foo").expect(get_string().as_ref()); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `unwrap_or_else(|| { panic!("{}", get_string()) })` error: use of `expect` followed by a function call - --> $DIR/expect_fun_call.rs:79:21 + --> $DIR/expect_fun_call.rs:80:21 | LL | Some("foo").expect(get_string().as_str()); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `unwrap_or_else(|| { panic!("{}", get_string()) })` error: use of `expect` followed by a function call - --> $DIR/expect_fun_call.rs:81:21 + --> $DIR/expect_fun_call.rs:82:21 | LL | Some("foo").expect(get_static_str()); | ^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `unwrap_or_else(|| { panic!("{}", get_static_str()) })` error: use of `expect` followed by a function call - --> $DIR/expect_fun_call.rs:82:21 + --> $DIR/expect_fun_call.rs:83:21 | LL | Some("foo").expect(get_non_static_str(&0)); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `unwrap_or_else(|| { panic!("{}", get_non_static_str(&0).to_string()) })` error: use of `expect` followed by a function call - --> $DIR/expect_fun_call.rs:86:16 + --> $DIR/expect_fun_call.rs:87:16 | LL | Some(true).expect(&format!("key {}, {}", 1, 2)); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `unwrap_or_else(|| panic!("key {}, {}", 1, 2))` error: use of `expect` followed by a function call - --> $DIR/expect_fun_call.rs:92:17 + --> $DIR/expect_fun_call.rs:93:17 | LL | opt_ref.expect(&format!("{:?}", opt_ref)); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `unwrap_or_else(|| panic!("{:?}", opt_ref))` diff --git a/src/tools/clippy/tests/ui/field_reassign_with_default.rs b/src/tools/clippy/tests/ui/field_reassign_with_default.rs index 787053fb00064..7367910eaa126 100644 --- a/src/tools/clippy/tests/ui/field_reassign_with_default.rs +++ b/src/tools/clippy/tests/ui/field_reassign_with_default.rs @@ -183,3 +183,67 @@ struct WrapperMulti { i: T, j: U, } + +mod issue6312 { + use std::sync::atomic::AtomicBool; + use std::sync::Arc; + + // do not lint: type implements `Drop` but not all fields are `Copy` + #[derive(Clone, Default)] + pub struct ImplDropNotAllCopy { + name: String, + delay_data_sync: Arc, + } + + impl Drop for ImplDropNotAllCopy { + fn drop(&mut self) { + self.close() + } + } + + impl ImplDropNotAllCopy { + fn new(name: &str) -> Self { + let mut f = ImplDropNotAllCopy::default(); + f.name = name.to_owned(); + f + } + fn close(&self) {} + } + + // lint: type implements `Drop` and all fields are `Copy` + #[derive(Clone, Default)] + pub struct ImplDropAllCopy { + name: usize, + delay_data_sync: bool, + } + + impl Drop for ImplDropAllCopy { + fn drop(&mut self) { + self.close() + } + } + + impl ImplDropAllCopy { + fn new(name: &str) -> Self { + let mut f = ImplDropAllCopy::default(); + f.name = name.len(); + f + } + fn close(&self) {} + } + + // lint: type does not implement `Drop` though all fields are `Copy` + #[derive(Clone, Default)] + pub struct NoDropAllCopy { + name: usize, + delay_data_sync: bool, + } + + impl NoDropAllCopy { + fn new(name: &str) -> Self { + let mut f = NoDropAllCopy::default(); + f.name = name.len(); + f + } + } +} diff --git a/src/tools/clippy/tests/ui/field_reassign_with_default.stderr b/src/tools/clippy/tests/ui/field_reassign_with_default.stderr index b56db08ec8a78..3ce4b91a54869 100644 --- a/src/tools/clippy/tests/ui/field_reassign_with_default.stderr +++ b/src/tools/clippy/tests/ui/field_reassign_with_default.stderr @@ -107,5 +107,29 @@ note: consider initializing the variable with `WrapperMulti:: { i: 42, LL | let mut a: WrapperMulti = Default::default(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ -error: aborting due to 9 previous errors +error: field assignment outside of initializer for an instance created with Default::default() + --> $DIR/field_reassign_with_default.rs:229:13 + | +LL | f.name = name.len(); + | ^^^^^^^^^^^^^^^^^^^^ + | +note: consider initializing the variable with `issue6312::ImplDropAllCopy { name: name.len(), ..Default::default() }` and removing relevant reassignments + --> $DIR/field_reassign_with_default.rs:228:13 + | +LL | let mut f = ImplDropAllCopy::default(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: field assignment outside of initializer for an instance created with Default::default() + --> $DIR/field_reassign_with_default.rs:245:13 + | +LL | f.name = name.len(); + | ^^^^^^^^^^^^^^^^^^^^ + | +note: consider initializing the variable with `issue6312::NoDropAllCopy { name: name.len(), ..Default::default() }` and removing relevant reassignments + --> $DIR/field_reassign_with_default.rs:244:13 + | +LL | let mut f = NoDropAllCopy::default(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: aborting due to 11 previous errors diff --git a/src/tools/clippy/tests/ui/fn_to_numeric_cast_any.rs b/src/tools/clippy/tests/ui/fn_to_numeric_cast_any.rs new file mode 100644 index 0000000000000..46704683926b2 --- /dev/null +++ b/src/tools/clippy/tests/ui/fn_to_numeric_cast_any.rs @@ -0,0 +1,76 @@ +#![warn(clippy::fn_to_numeric_cast_any)] +#![allow(clippy::fn_to_numeric_cast, clippy::fn_to_numeric_cast_with_truncation)] + +fn foo() -> u8 { + 0 +} + +fn generic_foo(x: T) -> T { + x +} + +trait Trait { + fn static_method() -> u32 { + 2 + } +} + +struct Struct; + +impl Trait for Struct {} + +fn fn_pointer_to_integer() { + let _ = foo as i8; + let _ = foo as i16; + let _ = foo as i32; + let _ = foo as i64; + let _ = foo as i128; + let _ = foo as isize; + + let _ = foo as u8; + let _ = foo as u16; + let _ = foo as u32; + let _ = foo as u64; + let _ = foo as u128; + let _ = foo as usize; +} + +fn static_method_to_integer() { + let _ = Struct::static_method as usize; +} + +fn fn_with_fn_arg(f: fn(i32) -> u32) -> usize { + f as usize +} + +fn fn_with_generic_static_trait_method() -> usize { + T::static_method as usize +} + +fn closure_to_fn_to_integer() { + let clos = |x| x * 2_u32; + + let _ = (clos as fn(u32) -> u32) as usize; +} + +fn fn_to_raw_ptr() { + let _ = foo as *const (); +} + +fn cast_fn_to_self() { + // Casting to the same function pointer type should be permitted. + let _ = foo as fn() -> u8; +} + +fn cast_generic_to_concrete() { + // Casting to a more concrete function pointer type should be permitted. + let _ = generic_foo as fn(usize) -> usize; +} + +fn cast_closure_to_fn() { + // Casting a closure to a function pointer should be permitted. + let id = |x| x; + let _ = id as fn(usize) -> usize; +} + +fn main() {} diff --git a/src/tools/clippy/tests/ui/fn_to_numeric_cast_any.stderr b/src/tools/clippy/tests/ui/fn_to_numeric_cast_any.stderr new file mode 100644 index 0000000000000..a6c4a77672f86 --- /dev/null +++ b/src/tools/clippy/tests/ui/fn_to_numeric_cast_any.stderr @@ -0,0 +1,106 @@ +error: casting function pointer `foo` to `i8` + --> $DIR/fn_to_numeric_cast_any.rs:23:13 + | +LL | let _ = foo as i8; + | ^^^^^^^^^ help: did you mean to invoke the function?: `foo() as i8` + | + = note: `-D clippy::fn-to-numeric-cast-any` implied by `-D warnings` + +error: casting function pointer `foo` to `i16` + --> $DIR/fn_to_numeric_cast_any.rs:24:13 + | +LL | let _ = foo as i16; + | ^^^^^^^^^^ help: did you mean to invoke the function?: `foo() as i16` + +error: casting function pointer `foo` to `i32` + --> $DIR/fn_to_numeric_cast_any.rs:25:13 + | +LL | let _ = foo as i32; + | ^^^^^^^^^^ help: did you mean to invoke the function?: `foo() as i32` + +error: casting function pointer `foo` to `i64` + --> $DIR/fn_to_numeric_cast_any.rs:26:13 + | +LL | let _ = foo as i64; + | ^^^^^^^^^^ help: did you mean to invoke the function?: `foo() as i64` + +error: casting function pointer `foo` to `i128` + --> $DIR/fn_to_numeric_cast_any.rs:27:13 + | +LL | let _ = foo as i128; + | ^^^^^^^^^^^ help: did you mean to invoke the function?: `foo() as i128` + +error: casting function pointer `foo` to `isize` + --> $DIR/fn_to_numeric_cast_any.rs:28:13 + | +LL | let _ = foo as isize; + | ^^^^^^^^^^^^ help: did you mean to invoke the function?: `foo() as isize` + +error: casting function pointer `foo` to `u8` + --> $DIR/fn_to_numeric_cast_any.rs:30:13 + | +LL | let _ = foo as u8; + | ^^^^^^^^^ help: did you mean to invoke the function?: `foo() as u8` + +error: casting function pointer `foo` to `u16` + --> $DIR/fn_to_numeric_cast_any.rs:31:13 + | +LL | let _ = foo as u16; + | ^^^^^^^^^^ help: did you mean to invoke the function?: `foo() as u16` + +error: casting function pointer `foo` to `u32` + --> $DIR/fn_to_numeric_cast_any.rs:32:13 + | +LL | let _ = foo as u32; + | ^^^^^^^^^^ help: did you mean to invoke the function?: `foo() as u32` + +error: casting function pointer `foo` to `u64` + --> $DIR/fn_to_numeric_cast_any.rs:33:13 + | +LL | let _ = foo as u64; + | ^^^^^^^^^^ help: did you mean to invoke the function?: `foo() as u64` + +error: casting function pointer `foo` to `u128` + --> $DIR/fn_to_numeric_cast_any.rs:34:13 + | +LL | let _ = foo as u128; + | ^^^^^^^^^^^ help: did you mean to invoke the function?: `foo() as u128` + +error: casting function pointer `foo` to `usize` + --> $DIR/fn_to_numeric_cast_any.rs:35:13 + | +LL | let _ = foo as usize; + | ^^^^^^^^^^^^ help: did you mean to invoke the function?: `foo() as usize` + +error: casting function pointer `Struct::static_method` to `usize` + --> $DIR/fn_to_numeric_cast_any.rs:39:13 + | +LL | let _ = Struct::static_method as usize; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: did you mean to invoke the function?: `Struct::static_method() as usize` + +error: casting function pointer `f` to `usize` + --> $DIR/fn_to_numeric_cast_any.rs:43:5 + | +LL | f as usize + | ^^^^^^^^^^ help: did you mean to invoke the function?: `f() as usize` + +error: casting function pointer `T::static_method` to `usize` + --> $DIR/fn_to_numeric_cast_any.rs:47:5 + | +LL | T::static_method as usize + | ^^^^^^^^^^^^^^^^^^^^^^^^^ help: did you mean to invoke the function?: `T::static_method() as usize` + +error: casting function pointer `(clos as fn(u32) -> u32)` to `usize` + --> $DIR/fn_to_numeric_cast_any.rs:53:13 + | +LL | let _ = (clos as fn(u32) -> u32) as usize; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: did you mean to invoke the function?: `(clos as fn(u32) -> u32)() as usize` + +error: casting function pointer `foo` to `*const ()` + --> $DIR/fn_to_numeric_cast_any.rs:57:13 + | +LL | let _ = foo as *const (); + | ^^^^^^^^^^^^^^^^ help: did you mean to invoke the function?: `foo() as *const ()` + +error: aborting due to 17 previous errors + diff --git a/src/tools/clippy/tests/ui/format.fixed b/src/tools/clippy/tests/ui/format.fixed index 5dd64140e8116..73fc750511c78 100644 --- a/src/tools/clippy/tests/ui/format.fixed +++ b/src/tools/clippy/tests/ui/format.fixed @@ -1,6 +1,6 @@ // run-rustfix -#![allow(clippy::print_literal, clippy::redundant_clone)] +#![allow(clippy::print_literal, clippy::redundant_clone, clippy::to_string_in_format_args)] #![warn(clippy::useless_format)] struct Foo(pub String); diff --git a/src/tools/clippy/tests/ui/format.rs b/src/tools/clippy/tests/ui/format.rs index 4599fb5207ea8..2f4595650cbf3 100644 --- a/src/tools/clippy/tests/ui/format.rs +++ b/src/tools/clippy/tests/ui/format.rs @@ -1,6 +1,6 @@ // run-rustfix -#![allow(clippy::print_literal, clippy::redundant_clone)] +#![allow(clippy::print_literal, clippy::redundant_clone, clippy::to_string_in_format_args)] #![warn(clippy::useless_format)] struct Foo(pub String); diff --git a/src/tools/clippy/tests/ui/format_args.fixed b/src/tools/clippy/tests/ui/format_args.fixed new file mode 100644 index 0000000000000..8376566c4d62d --- /dev/null +++ b/src/tools/clippy/tests/ui/format_args.fixed @@ -0,0 +1,105 @@ +// run-rustfix + +#![allow(unreachable_code)] +#![allow(unused_macros)] +#![allow(unused_variables)] +#![allow(clippy::assertions_on_constants)] +#![allow(clippy::eq_op)] +#![warn(clippy::to_string_in_format_args)] + +use std::io::{stdout, Write}; +use std::ops::Deref; +use std::panic::Location; + +struct Somewhere; + +impl ToString for Somewhere { + fn to_string(&self) -> String { + String::from("somewhere") + } +} + +struct X(u32); + +impl Deref for X { + type Target = u32; + + fn deref(&self) -> &u32 { + &self.0 + } +} + +struct Y<'a>(&'a X); + +impl<'a> Deref for Y<'a> { + type Target = &'a X; + + fn deref(&self) -> &Self::Target { + &self.0 + } +} + +struct Z(u32); + +impl Deref for Z { + type Target = u32; + + fn deref(&self) -> &u32 { + &self.0 + } +} + +impl std::fmt::Display for Z { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + write!(f, "Z") + } +} + +macro_rules! my_macro { + () => { + // here be dragons, do not enter (or lint) + println!("error: something failed at {}", Location::caller().to_string()); + }; +} + +macro_rules! my_other_macro { + () => { + Location::caller().to_string() + }; +} + +fn main() { + let x = &X(1); + let x_ref = &x; + + let _ = format!("error: something failed at {}", Location::caller()); + let _ = write!( + stdout(), + "error: something failed at {}", + Location::caller() + ); + let _ = writeln!( + stdout(), + "error: something failed at {}", + Location::caller() + ); + print!("error: something failed at {}", Location::caller()); + println!("error: something failed at {}", Location::caller()); + eprint!("error: something failed at {}", Location::caller()); + eprintln!("error: something failed at {}", Location::caller()); + let _ = format_args!("error: something failed at {}", Location::caller()); + assert!(true, "error: something failed at {}", Location::caller()); + assert_eq!(0, 0, "error: something failed at {}", Location::caller()); + assert_ne!(0, 0, "error: something failed at {}", Location::caller()); + panic!("error: something failed at {}", Location::caller()); + println!("{}", *X(1)); + println!("{}", ***Y(&X(1))); + println!("{}", Z(1)); + println!("{}", **x); + println!("{}", ***x_ref); + + println!("error: something failed at {}", Somewhere.to_string()); + println!("{} and again {0}", x.to_string()); + my_macro!(); + println!("error: something failed at {}", my_other_macro!()); +} diff --git a/src/tools/clippy/tests/ui/format_args.rs b/src/tools/clippy/tests/ui/format_args.rs new file mode 100644 index 0000000000000..164cc07066dc3 --- /dev/null +++ b/src/tools/clippy/tests/ui/format_args.rs @@ -0,0 +1,105 @@ +// run-rustfix + +#![allow(unreachable_code)] +#![allow(unused_macros)] +#![allow(unused_variables)] +#![allow(clippy::assertions_on_constants)] +#![allow(clippy::eq_op)] +#![warn(clippy::to_string_in_format_args)] + +use std::io::{stdout, Write}; +use std::ops::Deref; +use std::panic::Location; + +struct Somewhere; + +impl ToString for Somewhere { + fn to_string(&self) -> String { + String::from("somewhere") + } +} + +struct X(u32); + +impl Deref for X { + type Target = u32; + + fn deref(&self) -> &u32 { + &self.0 + } +} + +struct Y<'a>(&'a X); + +impl<'a> Deref for Y<'a> { + type Target = &'a X; + + fn deref(&self) -> &Self::Target { + &self.0 + } +} + +struct Z(u32); + +impl Deref for Z { + type Target = u32; + + fn deref(&self) -> &u32 { + &self.0 + } +} + +impl std::fmt::Display for Z { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + write!(f, "Z") + } +} + +macro_rules! my_macro { + () => { + // here be dragons, do not enter (or lint) + println!("error: something failed at {}", Location::caller().to_string()); + }; +} + +macro_rules! my_other_macro { + () => { + Location::caller().to_string() + }; +} + +fn main() { + let x = &X(1); + let x_ref = &x; + + let _ = format!("error: something failed at {}", Location::caller().to_string()); + let _ = write!( + stdout(), + "error: something failed at {}", + Location::caller().to_string() + ); + let _ = writeln!( + stdout(), + "error: something failed at {}", + Location::caller().to_string() + ); + print!("error: something failed at {}", Location::caller().to_string()); + println!("error: something failed at {}", Location::caller().to_string()); + eprint!("error: something failed at {}", Location::caller().to_string()); + eprintln!("error: something failed at {}", Location::caller().to_string()); + let _ = format_args!("error: something failed at {}", Location::caller().to_string()); + assert!(true, "error: something failed at {}", Location::caller().to_string()); + assert_eq!(0, 0, "error: something failed at {}", Location::caller().to_string()); + assert_ne!(0, 0, "error: something failed at {}", Location::caller().to_string()); + panic!("error: something failed at {}", Location::caller().to_string()); + println!("{}", X(1).to_string()); + println!("{}", Y(&X(1)).to_string()); + println!("{}", Z(1).to_string()); + println!("{}", x.to_string()); + println!("{}", x_ref.to_string()); + + println!("error: something failed at {}", Somewhere.to_string()); + println!("{} and again {0}", x.to_string()); + my_macro!(); + println!("error: something failed at {}", my_other_macro!()); +} diff --git a/src/tools/clippy/tests/ui/format_args.stderr b/src/tools/clippy/tests/ui/format_args.stderr new file mode 100644 index 0000000000000..9cfc97edeafb8 --- /dev/null +++ b/src/tools/clippy/tests/ui/format_args.stderr @@ -0,0 +1,106 @@ +error: `to_string` applied to a type that implements `Display` in `format!` args + --> $DIR/format_args.rs:75:72 + | +LL | let _ = format!("error: something failed at {}", Location::caller().to_string()); + | ^^^^^^^^^^^^ help: remove this + | + = note: `-D clippy::to-string-in-format-args` implied by `-D warnings` + +error: `to_string` applied to a type that implements `Display` in `write!` args + --> $DIR/format_args.rs:79:27 + | +LL | Location::caller().to_string() + | ^^^^^^^^^^^^ help: remove this + +error: `to_string` applied to a type that implements `Display` in `writeln!` args + --> $DIR/format_args.rs:84:27 + | +LL | Location::caller().to_string() + | ^^^^^^^^^^^^ help: remove this + +error: `to_string` applied to a type that implements `Display` in `print!` args + --> $DIR/format_args.rs:86:63 + | +LL | print!("error: something failed at {}", Location::caller().to_string()); + | ^^^^^^^^^^^^ help: remove this + +error: `to_string` applied to a type that implements `Display` in `println!` args + --> $DIR/format_args.rs:87:65 + | +LL | println!("error: something failed at {}", Location::caller().to_string()); + | ^^^^^^^^^^^^ help: remove this + +error: `to_string` applied to a type that implements `Display` in `eprint!` args + --> $DIR/format_args.rs:88:64 + | +LL | eprint!("error: something failed at {}", Location::caller().to_string()); + | ^^^^^^^^^^^^ help: remove this + +error: `to_string` applied to a type that implements `Display` in `eprintln!` args + --> $DIR/format_args.rs:89:66 + | +LL | eprintln!("error: something failed at {}", Location::caller().to_string()); + | ^^^^^^^^^^^^ help: remove this + +error: `to_string` applied to a type that implements `Display` in `format_args!` args + --> $DIR/format_args.rs:90:77 + | +LL | let _ = format_args!("error: something failed at {}", Location::caller().to_string()); + | ^^^^^^^^^^^^ help: remove this + +error: `to_string` applied to a type that implements `Display` in `assert!` args + --> $DIR/format_args.rs:91:70 + | +LL | assert!(true, "error: something failed at {}", Location::caller().to_string()); + | ^^^^^^^^^^^^ help: remove this + +error: `to_string` applied to a type that implements `Display` in `assert_eq!` args + --> $DIR/format_args.rs:92:73 + | +LL | assert_eq!(0, 0, "error: something failed at {}", Location::caller().to_string()); + | ^^^^^^^^^^^^ help: remove this + +error: `to_string` applied to a type that implements `Display` in `assert_ne!` args + --> $DIR/format_args.rs:93:73 + | +LL | assert_ne!(0, 0, "error: something failed at {}", Location::caller().to_string()); + | ^^^^^^^^^^^^ help: remove this + +error: `to_string` applied to a type that implements `Display` in `panic!` args + --> $DIR/format_args.rs:94:63 + | +LL | panic!("error: something failed at {}", Location::caller().to_string()); + | ^^^^^^^^^^^^ help: remove this + +error: `to_string` applied to a type that implements `Display` in `println!` args + --> $DIR/format_args.rs:95:20 + | +LL | println!("{}", X(1).to_string()); + | ^^^^^^^^^^^^^^^^ help: use this: `*X(1)` + +error: `to_string` applied to a type that implements `Display` in `println!` args + --> $DIR/format_args.rs:96:20 + | +LL | println!("{}", Y(&X(1)).to_string()); + | ^^^^^^^^^^^^^^^^^^^^ help: use this: `***Y(&X(1))` + +error: `to_string` applied to a type that implements `Display` in `println!` args + --> $DIR/format_args.rs:97:24 + | +LL | println!("{}", Z(1).to_string()); + | ^^^^^^^^^^^^ help: remove this + +error: `to_string` applied to a type that implements `Display` in `println!` args + --> $DIR/format_args.rs:98:20 + | +LL | println!("{}", x.to_string()); + | ^^^^^^^^^^^^^ help: use this: `**x` + +error: `to_string` applied to a type that implements `Display` in `println!` args + --> $DIR/format_args.rs:99:20 + | +LL | println!("{}", x_ref.to_string()); + | ^^^^^^^^^^^^^^^^^ help: use this: `***x_ref` + +error: aborting due to 17 previous errors + diff --git a/src/tools/clippy/tests/ui/format_args_unfixable.rs b/src/tools/clippy/tests/ui/format_args_unfixable.rs new file mode 100644 index 0000000000000..a8c06c2bde664 --- /dev/null +++ b/src/tools/clippy/tests/ui/format_args_unfixable.rs @@ -0,0 +1,60 @@ +#![allow(clippy::assertions_on_constants)] +#![allow(clippy::eq_op)] +#![warn(clippy::format_in_format_args)] +#![warn(clippy::to_string_in_format_args)] + +use std::io::{stdout, Error, ErrorKind, Write}; +use std::ops::Deref; +use std::panic::Location; + +macro_rules! my_macro { + () => { + // here be dragons, do not enter (or lint) + println!("error: {}", format!("something failed at {}", Location::caller())); + }; +} + +macro_rules! my_other_macro { + () => { + format!("something failed at {}", Location::caller()) + }; +} + +fn main() { + let error = Error::new(ErrorKind::Other, "bad thing"); + let x = 'x'; + + println!("error: {}", format!("something failed at {}", Location::caller())); + println!("{}: {}", error, format!("something failed at {}", Location::caller())); + println!("{:?}: {}", error, format!("something failed at {}", Location::caller())); + println!("{{}}: {}", format!("something failed at {}", Location::caller())); + println!(r#"error: "{}""#, format!("something failed at {}", Location::caller())); + println!("error: {}", format!(r#"something failed at "{}""#, Location::caller())); + println!("error: {}", format!("something failed at {} {0}", Location::caller())); + let _ = format!("error: {}", format!("something failed at {}", Location::caller())); + let _ = write!( + stdout(), + "error: {}", + format!("something failed at {}", Location::caller()) + ); + let _ = writeln!( + stdout(), + "error: {}", + format!("something failed at {}", Location::caller()) + ); + print!("error: {}", format!("something failed at {}", Location::caller())); + eprint!("error: {}", format!("something failed at {}", Location::caller())); + eprintln!("error: {}", format!("something failed at {}", Location::caller())); + let _ = format_args!("error: {}", format!("something failed at {}", Location::caller())); + assert!(true, "error: {}", format!("something failed at {}", Location::caller())); + assert_eq!(0, 0, "error: {}", format!("something failed at {}", Location::caller())); + assert_ne!(0, 0, "error: {}", format!("something failed at {}", Location::caller())); + panic!("error: {}", format!("something failed at {}", Location::caller())); + + println!("error: {}", format_args!("something failed at {}", Location::caller())); + println!("error: {:>70}", format!("something failed at {}", Location::caller())); + println!("error: {} {0}", format!("something failed at {}", Location::caller())); + println!("{} and again {0}", format!("hi {}", x)); + my_macro!(); + println!("error: {}", my_other_macro!()); +} diff --git a/src/tools/clippy/tests/ui/format_args_unfixable.stderr b/src/tools/clippy/tests/ui/format_args_unfixable.stderr new file mode 100644 index 0000000000000..4476218ad58e9 --- /dev/null +++ b/src/tools/clippy/tests/ui/format_args_unfixable.stderr @@ -0,0 +1,175 @@ +error: `format!` in `println!` args + --> $DIR/format_args_unfixable.rs:27:5 + | +LL | println!("error: {}", format!("something failed at {}", Location::caller())); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: `-D clippy::format-in-format-args` implied by `-D warnings` + = help: combine the `format!(..)` arguments with the outer `println!(..)` call + = help: or consider changing `format!` to `format_args!` + +error: `format!` in `println!` args + --> $DIR/format_args_unfixable.rs:28:5 + | +LL | println!("{}: {}", error, format!("something failed at {}", Location::caller())); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = help: combine the `format!(..)` arguments with the outer `println!(..)` call + = help: or consider changing `format!` to `format_args!` + +error: `format!` in `println!` args + --> $DIR/format_args_unfixable.rs:29:5 + | +LL | println!("{:?}: {}", error, format!("something failed at {}", Location::caller())); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = help: combine the `format!(..)` arguments with the outer `println!(..)` call + = help: or consider changing `format!` to `format_args!` + +error: `format!` in `println!` args + --> $DIR/format_args_unfixable.rs:30:5 + | +LL | println!("{{}}: {}", format!("something failed at {}", Location::caller())); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = help: combine the `format!(..)` arguments with the outer `println!(..)` call + = help: or consider changing `format!` to `format_args!` + +error: `format!` in `println!` args + --> $DIR/format_args_unfixable.rs:31:5 + | +LL | println!(r#"error: "{}""#, format!("something failed at {}", Location::caller())); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = help: combine the `format!(..)` arguments with the outer `println!(..)` call + = help: or consider changing `format!` to `format_args!` + +error: `format!` in `println!` args + --> $DIR/format_args_unfixable.rs:32:5 + | +LL | println!("error: {}", format!(r#"something failed at "{}""#, Location::caller())); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = help: combine the `format!(..)` arguments with the outer `println!(..)` call + = help: or consider changing `format!` to `format_args!` + +error: `format!` in `println!` args + --> $DIR/format_args_unfixable.rs:33:5 + | +LL | println!("error: {}", format!("something failed at {} {0}", Location::caller())); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = help: combine the `format!(..)` arguments with the outer `println!(..)` call + = help: or consider changing `format!` to `format_args!` + +error: `format!` in `format!` args + --> $DIR/format_args_unfixable.rs:34:13 + | +LL | let _ = format!("error: {}", format!("something failed at {}", Location::caller())); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = help: combine the `format!(..)` arguments with the outer `format!(..)` call + = help: or consider changing `format!` to `format_args!` + +error: `format!` in `write!` args + --> $DIR/format_args_unfixable.rs:35:13 + | +LL | let _ = write!( + | _____________^ +LL | | stdout(), +LL | | "error: {}", +LL | | format!("something failed at {}", Location::caller()) +LL | | ); + | |_____^ + | + = help: combine the `format!(..)` arguments with the outer `write!(..)` call + = help: or consider changing `format!` to `format_args!` + +error: `format!` in `writeln!` args + --> $DIR/format_args_unfixable.rs:40:13 + | +LL | let _ = writeln!( + | _____________^ +LL | | stdout(), +LL | | "error: {}", +LL | | format!("something failed at {}", Location::caller()) +LL | | ); + | |_____^ + | + = help: combine the `format!(..)` arguments with the outer `writeln!(..)` call + = help: or consider changing `format!` to `format_args!` + +error: `format!` in `print!` args + --> $DIR/format_args_unfixable.rs:45:5 + | +LL | print!("error: {}", format!("something failed at {}", Location::caller())); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = help: combine the `format!(..)` arguments with the outer `print!(..)` call + = help: or consider changing `format!` to `format_args!` + +error: `format!` in `eprint!` args + --> $DIR/format_args_unfixable.rs:46:5 + | +LL | eprint!("error: {}", format!("something failed at {}", Location::caller())); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = help: combine the `format!(..)` arguments with the outer `eprint!(..)` call + = help: or consider changing `format!` to `format_args!` + +error: `format!` in `eprintln!` args + --> $DIR/format_args_unfixable.rs:47:5 + | +LL | eprintln!("error: {}", format!("something failed at {}", Location::caller())); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = help: combine the `format!(..)` arguments with the outer `eprintln!(..)` call + = help: or consider changing `format!` to `format_args!` + +error: `format!` in `format_args!` args + --> $DIR/format_args_unfixable.rs:48:13 + | +LL | let _ = format_args!("error: {}", format!("something failed at {}", Location::caller())); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = help: combine the `format!(..)` arguments with the outer `format_args!(..)` call + = help: or consider changing `format!` to `format_args!` + +error: `format!` in `assert!` args + --> $DIR/format_args_unfixable.rs:49:5 + | +LL | assert!(true, "error: {}", format!("something failed at {}", Location::caller())); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = help: combine the `format!(..)` arguments with the outer `assert!(..)` call + = help: or consider changing `format!` to `format_args!` + +error: `format!` in `assert_eq!` args + --> $DIR/format_args_unfixable.rs:50:5 + | +LL | assert_eq!(0, 0, "error: {}", format!("something failed at {}", Location::caller())); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = help: combine the `format!(..)` arguments with the outer `assert_eq!(..)` call + = help: or consider changing `format!` to `format_args!` + +error: `format!` in `assert_ne!` args + --> $DIR/format_args_unfixable.rs:51:5 + | +LL | assert_ne!(0, 0, "error: {}", format!("something failed at {}", Location::caller())); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = help: combine the `format!(..)` arguments with the outer `assert_ne!(..)` call + = help: or consider changing `format!` to `format_args!` + +error: `format!` in `panic!` args + --> $DIR/format_args_unfixable.rs:52:5 + | +LL | panic!("error: {}", format!("something failed at {}", Location::caller())); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = help: combine the `format!(..)` arguments with the outer `panic!(..)` call + = help: or consider changing `format!` to `format_args!` + +error: aborting due to 18 previous errors + diff --git a/src/tools/clippy/tests/ui/implicit_saturating_sub.fixed b/src/tools/clippy/tests/ui/implicit_saturating_sub.fixed index 859765d08a70b..e6f57e9267eac 100644 --- a/src/tools/clippy/tests/ui/implicit_saturating_sub.fixed +++ b/src/tools/clippy/tests/ui/implicit_saturating_sub.fixed @@ -157,4 +157,12 @@ fn main() { if i_64 != 0 { i_64 -= 1; } + + // issue #7831 + // No Lint + if u_32 > 0 { + u_32 -= 1; + } else { + println!("side effect"); + } } diff --git a/src/tools/clippy/tests/ui/implicit_saturating_sub.rs b/src/tools/clippy/tests/ui/implicit_saturating_sub.rs index 2f32a7b157821..8bb28d149c628 100644 --- a/src/tools/clippy/tests/ui/implicit_saturating_sub.rs +++ b/src/tools/clippy/tests/ui/implicit_saturating_sub.rs @@ -203,4 +203,12 @@ fn main() { if i_64 != 0 { i_64 -= 1; } + + // issue #7831 + // No Lint + if u_32 > 0 { + u_32 -= 1; + } else { + println!("side effect"); + } } diff --git a/src/tools/clippy/tests/ui/match_expr_like_matches_macro.stderr b/src/tools/clippy/tests/ui/match_expr_like_matches_macro.stderr index 366ef36c367bf..d7cedf9f9f159 100644 --- a/src/tools/clippy/tests/ui/match_expr_like_matches_macro.stderr +++ b/src/tools/clippy/tests/ui/match_expr_like_matches_macro.stderr @@ -110,23 +110,6 @@ LL | | _ => false, LL | | }; | |_________^ help: try this: `matches!(&val, &Some(ref _a))` -error: you don't need to add `&` to both the expression and the patterns - --> $DIR/match_expr_like_matches_macro.rs:166:20 - | -LL | let _res = match &val { - | ____________________^ -LL | | &Some(ref _a) => true, -LL | | _ => false, -LL | | }; - | |_________^ - | - = note: `-D clippy::match-ref-pats` implied by `-D warnings` -help: try - | -LL ~ let _res = match val { -LL ~ Some(ref _a) => true, - | - error: match expression looks like `matches!` macro --> $DIR/match_expr_like_matches_macro.rs:178:20 | @@ -137,21 +120,5 @@ LL | | _ => false, LL | | }; | |_________^ help: try this: `matches!(&val, &Some(ref _a))` -error: you don't need to add `&` to both the expression and the patterns - --> $DIR/match_expr_like_matches_macro.rs:178:20 - | -LL | let _res = match &val { - | ____________________^ -LL | | &Some(ref _a) => true, -LL | | _ => false, -LL | | }; - | |_________^ - | -help: try - | -LL ~ let _res = match val { -LL ~ Some(ref _a) => true, - | - -error: aborting due to 14 previous errors +error: aborting due to 12 previous errors diff --git a/src/tools/clippy/tests/ui/match_overlapping_arm.rs b/src/tools/clippy/tests/ui/match_overlapping_arm.rs index 846d665d1d864..ff91c4498ec62 100644 --- a/src/tools/clippy/tests/ui/match_overlapping_arm.rs +++ b/src/tools/clippy/tests/ui/match_overlapping_arm.rs @@ -10,98 +10,95 @@ fn overlapping() { const FOO: u64 = 2; match 42 { - 0..=10 => println!("0 ... 10"), - 0..=11 => println!("0 ... 11"), + 0..=10 => println!("0..=10"), + 0..=11 => println!("0..=11"), _ => (), } match 42 { - 0..=5 => println!("0 ... 5"), - 6..=7 => println!("6 ... 7"), - FOO..=11 => println!("0 ... 11"), + 0..=5 => println!("0..=5"), + 6..=7 => println!("6..=7"), + FOO..=11 => println!("FOO..=11"), _ => (), } match 42 { 2 => println!("2"), - 0..=5 => println!("0 ... 5"), + 0..=5 => println!("0..=5"), _ => (), } match 42 { 2 => println!("2"), - 0..=2 => println!("0 ... 2"), + 0..=2 => println!("0..=2"), _ => (), } match 42 { - 0..=10 => println!("0 ... 10"), - 11..=50 => println!("11 ... 50"), + 0..=10 => println!("0..=10"), + 11..=50 => println!("11..=50"), _ => (), } match 42 { 2 => println!("2"), - 0..2 => println!("0 .. 2"), + 0..2 => println!("0..2"), _ => (), } match 42 { - 0..10 => println!("0 .. 10"), - 10..50 => println!("10 .. 50"), + 0..10 => println!("0..10"), + 10..50 => println!("10..50"), _ => (), } match 42 { - 0..11 => println!("0 .. 11"), - 0..=11 => println!("0 ... 11"), + 0..11 => println!("0..11"), + 0..=11 => println!("0..=11"), _ => (), } match 42 { - 5..7 => println!("5 .. 7"), - 0..10 => println!("0 .. 10"), + 5..7 => println!("5..7"), + 0..10 => println!("0..10"), _ => (), } match 42 { - 5..10 => println!("5 .. 10"), - 0..=10 => println!("0 ... 10"), + 5..10 => println!("5..10"), + 0..=10 => println!("0..=10"), _ => (), } match 42 { - 0..14 => println!("0 .. 14"), - 5..10 => println!("5 .. 10"), + 0..14 => println!("0..14"), + 5..10 => println!("5..10"), _ => (), } match 42 { - 5..14 => println!("5 .. 14"), - 0..=10 => println!("0 ... 10"), + 5..14 => println!("5..14"), + 0..=10 => println!("0..=10"), _ => (), } match 42 { - 0..7 => println!("0 .. 7"), - 0..=10 => println!("0 ... 10"), + 0..7 => println!("0..7"), + 0..=10 => println!("0..=10"), _ => (), } - /* - // FIXME(JohnTitor): uncomment this once rustfmt knows half-open patterns match 42 { - 0.. => println!("0 .. 42"), - 3.. => println!("3 .. 42"), + 3.. => println!("3.."), + 0.. => println!("0.."), _ => (), } match 42 { - ..=23 => println!("0 ... 23"), - ..26 => println!("0 .. 26"), + ..=23 => println!("..=23"), + ..26 => println!("..26"), _ => (), } - */ if let None = Some(42) { // nothing diff --git a/src/tools/clippy/tests/ui/match_overlapping_arm.stderr b/src/tools/clippy/tests/ui/match_overlapping_arm.stderr index 359fa49f51be7..c2b3f173c2b80 100644 --- a/src/tools/clippy/tests/ui/match_overlapping_arm.stderr +++ b/src/tools/clippy/tests/ui/match_overlapping_arm.stderr @@ -1,63 +1,75 @@ error: some ranges overlap --> $DIR/match_overlapping_arm.rs:13:9 | -LL | 0..=10 => println!("0 ... 10"), +LL | 0..=10 => println!("0..=10"), | ^^^^^^ | = note: `-D clippy::match-overlapping-arm` implied by `-D warnings` note: overlaps with this --> $DIR/match_overlapping_arm.rs:14:9 | -LL | 0..=11 => println!("0 ... 11"), +LL | 0..=11 => println!("0..=11"), | ^^^^^^ error: some ranges overlap --> $DIR/match_overlapping_arm.rs:19:9 | -LL | 0..=5 => println!("0 ... 5"), +LL | 0..=5 => println!("0..=5"), | ^^^^^ | note: overlaps with this --> $DIR/match_overlapping_arm.rs:21:9 | -LL | FOO..=11 => println!("0 ... 11"), +LL | FOO..=11 => println!("FOO..=11"), | ^^^^^^^^ error: some ranges overlap --> $DIR/match_overlapping_arm.rs:56:9 | -LL | 0..11 => println!("0 .. 11"), +LL | 0..11 => println!("0..11"), | ^^^^^ | note: overlaps with this --> $DIR/match_overlapping_arm.rs:57:9 | -LL | 0..=11 => println!("0 ... 11"), +LL | 0..=11 => println!("0..=11"), | ^^^^^^ error: some ranges overlap --> $DIR/match_overlapping_arm.rs:81:9 | -LL | 0..=10 => println!("0 ... 10"), +LL | 0..=10 => println!("0..=10"), | ^^^^^^ | note: overlaps with this --> $DIR/match_overlapping_arm.rs:80:9 | -LL | 5..14 => println!("5 .. 14"), +LL | 5..14 => println!("5..14"), | ^^^^^ error: some ranges overlap --> $DIR/match_overlapping_arm.rs:86:9 | -LL | 0..7 => println!("0 .. 7"), +LL | 0..7 => println!("0..7"), | ^^^^ | note: overlaps with this --> $DIR/match_overlapping_arm.rs:87:9 | -LL | 0..=10 => println!("0 ... 10"), +LL | 0..=10 => println!("0..=10"), | ^^^^^^ -error: aborting due to 5 previous errors +error: some ranges overlap + --> $DIR/match_overlapping_arm.rs:98:9 + | +LL | ..=23 => println!("..=23"), + | ^^^^^ + | +note: overlaps with this + --> $DIR/match_overlapping_arm.rs:99:9 + | +LL | ..26 => println!("..26"), + | ^^^^ + +error: aborting due to 6 previous errors diff --git a/src/tools/clippy/tests/ui/match_ref_pats.rs b/src/tools/clippy/tests/ui/match_ref_pats.rs index 6cbb4d32b0d71..50246486bb6fc 100644 --- a/src/tools/clippy/tests/ui/match_ref_pats.rs +++ b/src/tools/clippy/tests/ui/match_ref_pats.rs @@ -72,4 +72,46 @@ mod ice_3719 { } } +mod issue_7740 { + macro_rules! foobar_variant( + ($idx:expr) => (FooBar::get($idx).unwrap()) + ); + + enum FooBar { + Foo, + Bar, + FooBar, + BarFoo, + } + + impl FooBar { + fn get(idx: u8) -> Option<&'static Self> { + match idx { + 0 => Some(&FooBar::Foo), + 1 => Some(&FooBar::Bar), + 2 => Some(&FooBar::FooBar), + 3 => Some(&FooBar::BarFoo), + _ => None, + } + } + } + + fn issue_7740() { + // Issue #7740 + match foobar_variant!(0) { + &FooBar::Foo => println!("Foo"), + &FooBar::Bar => println!("Bar"), + &FooBar::FooBar => println!("FooBar"), + _ => println!("Wild"), + } + + // This shouldn't trigger + if let &FooBar::BarFoo = foobar_variant!(3) { + println!("BarFoo"); + } else { + println!("Wild"); + } + } +} + fn main() {} diff --git a/src/tools/clippy/tests/ui/match_ref_pats.stderr b/src/tools/clippy/tests/ui/match_ref_pats.stderr index 072aff445e97f..901820077e20e 100644 --- a/src/tools/clippy/tests/ui/match_ref_pats.stderr +++ b/src/tools/clippy/tests/ui/match_ref_pats.stderr @@ -15,21 +15,6 @@ LL ~ Some(v) => println!("{:?}", v), LL ~ None => println!("none"), | -error: you don't need to add `&` to all patterns - --> $DIR/match_ref_pats.rs:18:5 - | -LL | / match tup { -LL | | &(v, 1) => println!("{}", v), -LL | | _ => println!("none"), -LL | | } - | |_____^ - | -help: instead of prefixing all patterns with `&`, you can dereference the expression - | -LL ~ match *tup { -LL ~ (v, 1) => println!("{}", v), - | - error: you don't need to add `&` to both the expression and the patterns --> $DIR/match_ref_pats.rs:24:5 | @@ -54,52 +39,30 @@ LL | if let &None = a { | = note: `-D clippy::redundant-pattern-matching` implied by `-D warnings` -error: you don't need to add `&` to all patterns - --> $DIR/match_ref_pats.rs:36:5 - | -LL | / if let &None = a { -LL | | println!("none"); -LL | | } - | |_____^ - | -help: instead of prefixing all patterns with `&`, you can dereference the expression - | -LL | if let None = *a { - | ~~~~ ~~ - error: redundant pattern matching, consider using `is_none()` --> $DIR/match_ref_pats.rs:41:12 | LL | if let &None = &b { | -------^^^^^----- help: try this: `if b.is_none()` -error: you don't need to add `&` to both the expression and the patterns - --> $DIR/match_ref_pats.rs:41:5 - | -LL | / if let &None = &b { -LL | | println!("none"); -LL | | } - | |_____^ - | -help: try - | -LL | if let None = b { - | ~~~~ ~ - error: you don't need to add `&` to all patterns - --> $DIR/match_ref_pats.rs:68:9 + --> $DIR/match_ref_pats.rs:101:9 | -LL | / match foo_variant!(0) { -LL | | &Foo::A => println!("A"), +LL | / match foobar_variant!(0) { +LL | | &FooBar::Foo => println!("Foo"), +LL | | &FooBar::Bar => println!("Bar"), +LL | | &FooBar::FooBar => println!("FooBar"), LL | | _ => println!("Wild"), LL | | } | |_________^ | help: instead of prefixing all patterns with `&`, you can dereference the expression | -LL ~ match *foo_variant!(0) { -LL ~ Foo::A => println!("A"), +LL ~ match *foobar_variant!(0) { +LL ~ FooBar::Foo => println!("Foo"), +LL ~ FooBar::Bar => println!("Bar"), +LL ~ FooBar::FooBar => println!("FooBar"), | -error: aborting due to 8 previous errors +error: aborting due to 5 previous errors diff --git a/src/tools/clippy/tests/ui/match_str_case_mismatch.rs b/src/tools/clippy/tests/ui/match_str_case_mismatch.rs new file mode 100644 index 0000000000000..208a4bba3d23c --- /dev/null +++ b/src/tools/clippy/tests/ui/match_str_case_mismatch.rs @@ -0,0 +1,98 @@ +#![warn(clippy::match_str_case_mismatch)] + +// Valid + +fn as_str_match() { + let var = "BAR"; + + match var.to_ascii_lowercase().as_str() { + "foo" => {}, + "bar" => {}, + _ => {}, + } +} + +fn addrof_unary_match() { + let var = "BAR"; + + match &*var.to_ascii_lowercase() { + "foo" => {}, + "bar" => {}, + _ => {}, + } +} + +fn alternating_chain() { + let var = "BAR"; + + match &*var + .to_ascii_lowercase() + .to_uppercase() + .to_lowercase() + .to_ascii_uppercase() + { + "FOO" => {}, + "BAR" => {}, + _ => {}, + } +} + +fn unrelated_method() { + struct Item { + a: String, + } + + impl Item { + #[allow(clippy::wrong_self_convention)] + fn to_lowercase(self) -> String { + self.a + } + } + + let item = Item { a: String::from("BAR") }; + + match &*item.to_lowercase() { + "FOO" => {}, + "BAR" => {}, + _ => {}, + } +} + +// Invalid + +fn as_str_match_mismatch() { + let var = "BAR"; + + match var.to_ascii_lowercase().as_str() { + "foo" => {}, + "Bar" => {}, + _ => {}, + } +} + +fn addrof_unary_match_mismatch() { + let var = "BAR"; + + match &*var.to_ascii_lowercase() { + "foo" => {}, + "Bar" => {}, + _ => {}, + } +} + +fn alternating_chain_mismatch() { + let var = "BAR"; + + match &*var + .to_ascii_lowercase() + .to_uppercase() + .to_lowercase() + .to_ascii_uppercase() + { + "FOO" => {}, + "bAR" => {}, + _ => {}, + } +} + +fn main() {} diff --git a/src/tools/clippy/tests/ui/match_str_case_mismatch.stderr b/src/tools/clippy/tests/ui/match_str_case_mismatch.stderr new file mode 100644 index 0000000000000..fa023477a9c33 --- /dev/null +++ b/src/tools/clippy/tests/ui/match_str_case_mismatch.stderr @@ -0,0 +1,36 @@ +error: this `match` arm has a differing case than its expression + --> $DIR/match_str_case_mismatch.rs:68:9 + | +LL | "Bar" => {}, + | ^^^^^ + | + = note: `-D clippy::match-str-case-mismatch` implied by `-D warnings` +help: consider changing the case of this arm to respect `to_ascii_lowercase` + | +LL | "bar" => {}, + | ~~~~~ + +error: this `match` arm has a differing case than its expression + --> $DIR/match_str_case_mismatch.rs:78:9 + | +LL | "Bar" => {}, + | ^^^^^ + | +help: consider changing the case of this arm to respect `to_ascii_lowercase` + | +LL | "bar" => {}, + | ~~~~~ + +error: this `match` arm has a differing case than its expression + --> $DIR/match_str_case_mismatch.rs:93:9 + | +LL | "bAR" => {}, + | ^^^^^ + | +help: consider changing the case of this arm to respect `to_ascii_uppercase` + | +LL | "BAR" => {}, + | ~~~~~ + +error: aborting due to 3 previous errors + diff --git a/src/tools/clippy/tests/ui/mut_mut.rs b/src/tools/clippy/tests/ui/mut_mut.rs index 8965cef66dedd..be854d9418332 100644 --- a/src/tools/clippy/tests/ui/mut_mut.rs +++ b/src/tools/clippy/tests/ui/mut_mut.rs @@ -1,6 +1,11 @@ +// aux-build:macro_rules.rs + #![allow(unused, clippy::no_effect, clippy::unnecessary_operation)] #![warn(clippy::mut_mut)] +#[macro_use] +extern crate macro_rules; + fn fun(x: &mut &mut u32) -> bool { **x > 0 } @@ -47,3 +52,8 @@ fn issue939() { println!(":{}", arg); } } + +fn issue6922() { + // do not lint from an external macro + mut_mut!(); +} diff --git a/src/tools/clippy/tests/ui/mut_mut.stderr b/src/tools/clippy/tests/ui/mut_mut.stderr index 0fed6953cb85c..6820a85aa5433 100644 --- a/src/tools/clippy/tests/ui/mut_mut.stderr +++ b/src/tools/clippy/tests/ui/mut_mut.stderr @@ -1,5 +1,5 @@ error: generally you want to avoid `&mut &mut _` if possible - --> $DIR/mut_mut.rs:4:11 + --> $DIR/mut_mut.rs:9:11 | LL | fn fun(x: &mut &mut u32) -> bool { | ^^^^^^^^^^^^^ @@ -7,13 +7,13 @@ LL | fn fun(x: &mut &mut u32) -> bool { = note: `-D clippy::mut-mut` implied by `-D warnings` error: generally you want to avoid `&mut &mut _` if possible - --> $DIR/mut_mut.rs:20:17 + --> $DIR/mut_mut.rs:25:17 | LL | let mut x = &mut &mut 1u32; | ^^^^^^^^^^^^^^ error: generally you want to avoid `&mut &mut _` if possible - --> $DIR/mut_mut.rs:14:9 + --> $DIR/mut_mut.rs:19:9 | LL | &mut $p | ^^^^^^^ @@ -24,37 +24,37 @@ LL | let mut z = mut_ptr!(&mut 3u32); = note: this error originates in the macro `mut_ptr` (in Nightly builds, run with -Z macro-backtrace for more info) error: this expression mutably borrows a mutable reference. Consider reborrowing - --> $DIR/mut_mut.rs:22:21 + --> $DIR/mut_mut.rs:27:21 | LL | let mut y = &mut x; | ^^^^^^ error: generally you want to avoid `&mut &mut _` if possible - --> $DIR/mut_mut.rs:26:32 + --> $DIR/mut_mut.rs:31:32 | LL | let y: &mut &mut u32 = &mut &mut 2; | ^^^^^^^^^^^ error: generally you want to avoid `&mut &mut _` if possible - --> $DIR/mut_mut.rs:26:16 + --> $DIR/mut_mut.rs:31:16 | LL | let y: &mut &mut u32 = &mut &mut 2; | ^^^^^^^^^^^^^ error: generally you want to avoid `&mut &mut _` if possible - --> $DIR/mut_mut.rs:31:37 + --> $DIR/mut_mut.rs:36:37 | LL | let y: &mut &mut &mut u32 = &mut &mut &mut 2; | ^^^^^^^^^^^^^^^^ error: generally you want to avoid `&mut &mut _` if possible - --> $DIR/mut_mut.rs:31:16 + --> $DIR/mut_mut.rs:36:16 | LL | let y: &mut &mut &mut u32 = &mut &mut &mut 2; | ^^^^^^^^^^^^^^^^^^ error: generally you want to avoid `&mut &mut _` if possible - --> $DIR/mut_mut.rs:31:21 + --> $DIR/mut_mut.rs:36:21 | LL | let y: &mut &mut &mut u32 = &mut &mut &mut 2; | ^^^^^^^^^^^^^ diff --git a/src/tools/clippy/tests/ui/no_effect.rs b/src/tools/clippy/tests/ui/no_effect.rs index 7ec845adfaacf..7bcc4cad0d363 100644 --- a/src/tools/clippy/tests/ui/no_effect.rs +++ b/src/tools/clippy/tests/ui/no_effect.rs @@ -1,5 +1,5 @@ #![feature(box_syntax)] -#![warn(clippy::no_effect)] +#![warn(clippy::no_effect_underscore_binding)] #![allow(dead_code)] #![allow(path_statements)] #![allow(clippy::deref_addrof)] @@ -90,6 +90,10 @@ fn main() { || x += 5; let s: String = "foo".into(); FooString { s: s }; + let _unused = 1; + let _penguin = || println!("Some helpful closure"); + let _duck = Struct { field: 0 }; + let _cat = [2, 4, 6, 8][2]; #[allow(clippy::no_effect)] 0; @@ -97,6 +101,8 @@ fn main() { // Do not warn get_number(); unsafe { unsafe_fn() }; + let _used = get_struct(); + let _x = vec![1]; DropUnit; DropStruct { field: 0 }; DropTuple(0); diff --git a/src/tools/clippy/tests/ui/no_effect.stderr b/src/tools/clippy/tests/ui/no_effect.stderr index 6b24675ac2d42..a5dbc9fef455a 100644 --- a/src/tools/clippy/tests/ui/no_effect.stderr +++ b/src/tools/clippy/tests/ui/no_effect.stderr @@ -156,5 +156,31 @@ error: statement with no effect LL | FooString { s: s }; | ^^^^^^^^^^^^^^^^^^^ -error: aborting due to 26 previous errors +error: binding to `_` prefixed variable with no side-effect + --> $DIR/no_effect.rs:93:5 + | +LL | let _unused = 1; + | ^^^^^^^^^^^^^^^^ + | + = note: `-D clippy::no-effect-underscore-binding` implied by `-D warnings` + +error: binding to `_` prefixed variable with no side-effect + --> $DIR/no_effect.rs:94:5 + | +LL | let _penguin = || println!("Some helpful closure"); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: binding to `_` prefixed variable with no side-effect + --> $DIR/no_effect.rs:95:5 + | +LL | let _duck = Struct { field: 0 }; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: binding to `_` prefixed variable with no side-effect + --> $DIR/no_effect.rs:96:5 + | +LL | let _cat = [2, 4, 6, 8][2]; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: aborting due to 30 previous errors diff --git a/src/tools/clippy/tests/ui/option_if_let_else.fixed b/src/tools/clippy/tests/ui/option_if_let_else.fixed index a3ebe5d070384..4077f1920a383 100644 --- a/src/tools/clippy/tests/ui/option_if_let_else.fixed +++ b/src/tools/clippy/tests/ui/option_if_let_else.fixed @@ -1,8 +1,7 @@ // edition:2018 // run-rustfix #![warn(clippy::option_if_let_else)] -#![allow(clippy::redundant_closure)] -#![allow(clippy::ref_option_ref, clippy::equatable_if_let)] +#![allow(clippy::redundant_closure, clippy::ref_option_ref, clippy::equatable_if_let)] fn bad1(string: Option<&str>) -> (bool, &str) { string.map_or((false, "hello"), |x| (true, x)) diff --git a/src/tools/clippy/tests/ui/option_if_let_else.rs b/src/tools/clippy/tests/ui/option_if_let_else.rs index b11df3db60f57..2f414e129d5a7 100644 --- a/src/tools/clippy/tests/ui/option_if_let_else.rs +++ b/src/tools/clippy/tests/ui/option_if_let_else.rs @@ -1,8 +1,7 @@ // edition:2018 // run-rustfix #![warn(clippy::option_if_let_else)] -#![allow(clippy::redundant_closure)] -#![allow(clippy::ref_option_ref, clippy::equatable_if_let)] +#![allow(clippy::redundant_closure, clippy::ref_option_ref, clippy::equatable_if_let)] fn bad1(string: Option<&str>) -> (bool, &str) { if let Some(x) = string { diff --git a/src/tools/clippy/tests/ui/option_if_let_else.stderr b/src/tools/clippy/tests/ui/option_if_let_else.stderr index ed748ee8b39e4..803d941c36df8 100644 --- a/src/tools/clippy/tests/ui/option_if_let_else.stderr +++ b/src/tools/clippy/tests/ui/option_if_let_else.stderr @@ -1,5 +1,5 @@ error: use Option::map_or instead of an if let/else - --> $DIR/option_if_let_else.rs:8:5 + --> $DIR/option_if_let_else.rs:7:5 | LL | / if let Some(x) = string { LL | | (true, x) @@ -11,19 +11,19 @@ LL | | } = note: `-D clippy::option-if-let-else` implied by `-D warnings` error: use Option::map_or instead of an if let/else - --> $DIR/option_if_let_else.rs:26:13 + --> $DIR/option_if_let_else.rs:25:13 | LL | let _ = if let Some(s) = *string { s.len() } else { 0 }; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `string.map_or(0, |s| s.len())` error: use Option::map_or instead of an if let/else - --> $DIR/option_if_let_else.rs:27:13 + --> $DIR/option_if_let_else.rs:26:13 | LL | let _ = if let Some(s) = &num { s } else { &0 }; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `num.as_ref().map_or(&0, |s| s)` error: use Option::map_or instead of an if let/else - --> $DIR/option_if_let_else.rs:28:13 + --> $DIR/option_if_let_else.rs:27:13 | LL | let _ = if let Some(s) = &mut num { | _____________^ @@ -43,13 +43,13 @@ LL ~ }); | error: use Option::map_or instead of an if let/else - --> $DIR/option_if_let_else.rs:34:13 + --> $DIR/option_if_let_else.rs:33:13 | LL | let _ = if let Some(ref s) = num { s } else { &0 }; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `num.as_ref().map_or(&0, |s| s)` error: use Option::map_or instead of an if let/else - --> $DIR/option_if_let_else.rs:35:13 + --> $DIR/option_if_let_else.rs:34:13 | LL | let _ = if let Some(mut s) = num { | _____________^ @@ -69,7 +69,7 @@ LL ~ }); | error: use Option::map_or instead of an if let/else - --> $DIR/option_if_let_else.rs:41:13 + --> $DIR/option_if_let_else.rs:40:13 | LL | let _ = if let Some(ref mut s) = num { | _____________^ @@ -89,7 +89,7 @@ LL ~ }); | error: use Option::map_or instead of an if let/else - --> $DIR/option_if_let_else.rs:50:5 + --> $DIR/option_if_let_else.rs:49:5 | LL | / if let Some(x) = arg { LL | | let y = x * x; @@ -108,7 +108,7 @@ LL + }) | error: use Option::map_or_else instead of an if let/else - --> $DIR/option_if_let_else.rs:63:13 + --> $DIR/option_if_let_else.rs:62:13 | LL | let _ = if let Some(x) = arg { | _____________^ @@ -120,7 +120,7 @@ LL | | }; | |_____^ help: try: `arg.map_or_else(|| side_effect(), |x| x)` error: use Option::map_or_else instead of an if let/else - --> $DIR/option_if_let_else.rs:72:13 + --> $DIR/option_if_let_else.rs:71:13 | LL | let _ = if let Some(x) = arg { | _____________^ @@ -143,13 +143,13 @@ LL ~ }, |x| x * x * x * x); | error: use Option::map_or instead of an if let/else - --> $DIR/option_if_let_else.rs:101:13 + --> $DIR/option_if_let_else.rs:100:13 | LL | let _ = if let Some(x) = optional { x + 2 } else { 5 }; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `optional.map_or(5, |x| x + 2)` error: use Option::map_or instead of an if let/else - --> $DIR/option_if_let_else.rs:110:13 + --> $DIR/option_if_let_else.rs:109:13 | LL | let _ = if let Some(x) = Some(0) { | _____________^ @@ -171,13 +171,13 @@ LL ~ }); | error: use Option::map_or_else instead of an if let/else - --> $DIR/option_if_let_else.rs:138:13 + --> $DIR/option_if_let_else.rs:137:13 | LL | let _ = if let Some(x) = Some(0) { s.len() + x } else { s.len() }; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `Some(0).map_or_else(|| s.len(), |x| s.len() + x)` error: use Option::map_or instead of an if let/else - --> $DIR/option_if_let_else.rs:142:13 + --> $DIR/option_if_let_else.rs:141:13 | LL | let _ = if let Some(x) = Some(0) { | _____________^ diff --git a/src/tools/clippy/tests/ui/question_mark.fixed b/src/tools/clippy/tests/ui/question_mark.fixed index 0b5746cb52270..ccb2e5a302e91 100644 --- a/src/tools/clippy/tests/ui/question_mark.fixed +++ b/src/tools/clippy/tests/ui/question_mark.fixed @@ -104,6 +104,21 @@ fn func() -> Option { Some(0) } +fn result_func(x: Result) -> Result { + let _ = x?; + + x?; + + // No warning + let y = if let Ok(x) = x { + x + } else { + return Err("some error"); + }; + + Ok(y) +} + fn main() { some_func(Some(42)); some_func(None); @@ -123,4 +138,6 @@ fn main() { returns_something_similar_to_option(so); func(); + + let _ = result_func(Ok(42)); } diff --git a/src/tools/clippy/tests/ui/question_mark.rs b/src/tools/clippy/tests/ui/question_mark.rs index 0f0825c933467..ca3722371f524 100644 --- a/src/tools/clippy/tests/ui/question_mark.rs +++ b/src/tools/clippy/tests/ui/question_mark.rs @@ -134,6 +134,23 @@ fn func() -> Option { Some(0) } +fn result_func(x: Result) -> Result { + let _ = if let Ok(x) = x { x } else { return x }; + + if x.is_err() { + return x; + } + + // No warning + let y = if let Ok(x) = x { + x + } else { + return Err("some error"); + }; + + Ok(y) +} + fn main() { some_func(Some(42)); some_func(None); @@ -153,4 +170,6 @@ fn main() { returns_something_similar_to_option(so); func(); + + let _ = result_func(Ok(42)); } diff --git a/src/tools/clippy/tests/ui/question_mark.stderr b/src/tools/clippy/tests/ui/question_mark.stderr index 6f330cfa385dd..161588cb73cba 100644 --- a/src/tools/clippy/tests/ui/question_mark.stderr +++ b/src/tools/clippy/tests/ui/question_mark.stderr @@ -100,5 +100,19 @@ LL | | return None; LL | | } | |_____^ help: replace it with: `f()?;` -error: aborting due to 11 previous errors +error: this if-let-else may be rewritten with the `?` operator + --> $DIR/question_mark.rs:138:13 + | +LL | let _ = if let Ok(x) = x { x } else { return x }; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace it with: `x?` + +error: this block may be rewritten with the `?` operator + --> $DIR/question_mark.rs:140:5 + | +LL | / if x.is_err() { +LL | | return x; +LL | | } + | |_____^ help: replace it with: `x?;` + +error: aborting due to 13 previous errors diff --git a/src/tools/clippy/tests/ui/semicolon_if_nothing_returned.rs b/src/tools/clippy/tests/ui/semicolon_if_nothing_returned.rs index 9644a23296831..7a45f1b18d4af 100644 --- a/src/tools/clippy/tests/ui/semicolon_if_nothing_returned.rs +++ b/src/tools/clippy/tests/ui/semicolon_if_nothing_returned.rs @@ -98,3 +98,15 @@ fn unsafe_checks() { let mut s = MaybeUninit::::uninit(); let _d = || unsafe { ptr::drop_in_place(s.as_mut_ptr()) }; } + +// Issue #7768 +#[rustfmt::skip] +fn macro_with_semicolon() { + macro_rules! repro { + () => { + while false { + } + }; + } + repro!(); +} diff --git a/src/tools/clippy/tests/ui/shadow.rs b/src/tools/clippy/tests/ui/shadow.rs index 02e838456d0b5..55caef59f7f68 100644 --- a/src/tools/clippy/tests/ui/shadow.rs +++ b/src/tools/clippy/tests/ui/shadow.rs @@ -17,6 +17,11 @@ fn shadow_reuse() -> Option<()> { let x = foo(x); let x = || x; let x = Some(1).map(|_| x)?; + let y = 1; + let y = match y { + 1 => 2, + _ => 3, + }; None } diff --git a/src/tools/clippy/tests/ui/shadow.stderr b/src/tools/clippy/tests/ui/shadow.stderr index 8b60e072c9342..feed6e1ba8b86 100644 --- a/src/tools/clippy/tests/ui/shadow.stderr +++ b/src/tools/clippy/tests/ui/shadow.stderr @@ -47,7 +47,7 @@ note: previous binding is here LL | let x = &mut x; | ^ -error: `x` is shadowed by `x.0` which reuses the original value +error: `x` is shadowed --> $DIR/shadow.rs:13:9 | LL | let x = x.0; @@ -60,7 +60,7 @@ note: previous binding is here LL | let x = ([[0]], ()); | ^ -error: `x` is shadowed by `x[0]` which reuses the original value +error: `x` is shadowed --> $DIR/shadow.rs:14:9 | LL | let x = x[0]; @@ -72,7 +72,7 @@ note: previous binding is here LL | let x = x.0; | ^ -error: `x` is shadowed by `x` which reuses the original value +error: `x` is shadowed --> $DIR/shadow.rs:15:10 | LL | let [x] = x; @@ -84,7 +84,7 @@ note: previous binding is here LL | let x = x[0]; | ^ -error: `x` is shadowed by `Some(x)` which reuses the original value +error: `x` is shadowed --> $DIR/shadow.rs:16:9 | LL | let x = Some(x); @@ -96,7 +96,7 @@ note: previous binding is here LL | let [x] = x; | ^ -error: `x` is shadowed by `foo(x)` which reuses the original value +error: `x` is shadowed --> $DIR/shadow.rs:17:9 | LL | let x = foo(x); @@ -108,7 +108,7 @@ note: previous binding is here LL | let x = Some(x); | ^ -error: `x` is shadowed by `|| x` which reuses the original value +error: `x` is shadowed --> $DIR/shadow.rs:18:9 | LL | let x = || x; @@ -120,7 +120,7 @@ note: previous binding is here LL | let x = foo(x); | ^ -error: `x` is shadowed by `Some(1).map(|_| x)?` which reuses the original value +error: `x` is shadowed --> $DIR/shadow.rs:19:9 | LL | let x = Some(1).map(|_| x)?; @@ -132,102 +132,114 @@ note: previous binding is here LL | let x = || x; | ^ +error: `y` is shadowed + --> $DIR/shadow.rs:21:9 + | +LL | let y = match y { + | ^ + | +note: previous binding is here + --> $DIR/shadow.rs:20:9 + | +LL | let y = 1; + | ^ + error: `x` shadows a previous, unrelated binding - --> $DIR/shadow.rs:25:9 + --> $DIR/shadow.rs:30:9 | LL | let x = 2; | ^ | = note: `-D clippy::shadow-unrelated` implied by `-D warnings` note: previous binding is here - --> $DIR/shadow.rs:24:9 + --> $DIR/shadow.rs:29:9 | LL | let x = 1; | ^ error: `x` shadows a previous, unrelated binding - --> $DIR/shadow.rs:30:13 + --> $DIR/shadow.rs:35:13 | LL | let x = 1; | ^ | note: previous binding is here - --> $DIR/shadow.rs:29:10 + --> $DIR/shadow.rs:34:10 | LL | fn f(x: u32) { | ^ error: `x` shadows a previous, unrelated binding - --> $DIR/shadow.rs:35:14 + --> $DIR/shadow.rs:40:14 | LL | Some(x) => { | ^ | note: previous binding is here - --> $DIR/shadow.rs:32:9 + --> $DIR/shadow.rs:37:9 | LL | let x = 1; | ^ error: `x` shadows a previous, unrelated binding - --> $DIR/shadow.rs:36:17 + --> $DIR/shadow.rs:41:17 | LL | let x = 1; | ^ | note: previous binding is here - --> $DIR/shadow.rs:35:14 + --> $DIR/shadow.rs:40:14 | LL | Some(x) => { | ^ error: `x` shadows a previous, unrelated binding - --> $DIR/shadow.rs:40:17 + --> $DIR/shadow.rs:45:17 | LL | if let Some(x) = Some(1) {} | ^ | note: previous binding is here - --> $DIR/shadow.rs:32:9 + --> $DIR/shadow.rs:37:9 | LL | let x = 1; | ^ error: `x` shadows a previous, unrelated binding - --> $DIR/shadow.rs:41:20 + --> $DIR/shadow.rs:46:20 | LL | while let Some(x) = Some(1) {} | ^ | note: previous binding is here - --> $DIR/shadow.rs:32:9 + --> $DIR/shadow.rs:37:9 | LL | let x = 1; | ^ error: `x` shadows a previous, unrelated binding - --> $DIR/shadow.rs:42:15 + --> $DIR/shadow.rs:47:15 | LL | let _ = |[x]: [u32; 1]| { | ^ | note: previous binding is here - --> $DIR/shadow.rs:32:9 + --> $DIR/shadow.rs:37:9 | LL | let x = 1; | ^ error: `x` shadows a previous, unrelated binding - --> $DIR/shadow.rs:43:13 + --> $DIR/shadow.rs:48:13 | LL | let x = 1; | ^ | note: previous binding is here - --> $DIR/shadow.rs:42:15 + --> $DIR/shadow.rs:47:15 | LL | let _ = |[x]: [u32; 1]| { | ^ -error: aborting due to 19 previous errors +error: aborting due to 20 previous errors diff --git a/src/tools/clippy/tests/ui/to_string_in_display.rs b/src/tools/clippy/tests/ui/to_string_in_display.rs index eb8105c6b6da0..3ccdcd1117b5a 100644 --- a/src/tools/clippy/tests/ui/to_string_in_display.rs +++ b/src/tools/clippy/tests/ui/to_string_in_display.rs @@ -1,5 +1,5 @@ #![warn(clippy::to_string_in_display)] -#![allow(clippy::inherent_to_string_shadow_display)] +#![allow(clippy::inherent_to_string_shadow_display, clippy::to_string_in_format_args)] use std::fmt; diff --git a/src/tools/clippy/tests/ui/trailing_empty_array.rs b/src/tools/clippy/tests/ui/trailing_empty_array.rs new file mode 100644 index 0000000000000..501c9eb7651f8 --- /dev/null +++ b/src/tools/clippy/tests/ui/trailing_empty_array.rs @@ -0,0 +1,186 @@ +#![warn(clippy::trailing_empty_array)] +#![feature(const_generics_defaults)] + +// Do lint: + +struct RarelyUseful { + field: i32, + last: [usize; 0], +} + +struct OnlyField { + first_and_last: [usize; 0], +} + +struct GenericArrayType { + field: i32, + last: [T; 0], +} + +#[must_use] +struct OnlyAnotherAttribute { + field: i32, + last: [usize; 0], +} + +#[derive(Debug)] +struct OnlyADeriveAttribute { + field: i32, + last: [usize; 0], +} + +const ZERO: usize = 0; +struct ZeroSizedWithConst { + field: i32, + last: [usize; ZERO], +} + +#[allow(clippy::eq_op)] +const fn compute_zero() -> usize { + (4 + 6) - (2 * 5) +} +struct ZeroSizedWithConstFunction { + field: i32, + last: [usize; compute_zero()], +} + +const fn compute_zero_from_arg(x: usize) -> usize { + x - 1 +} +struct ZeroSizedWithConstFunction2 { + field: i32, + last: [usize; compute_zero_from_arg(1)], +} + +struct ZeroSizedArrayWrapper([usize; 0]); + +struct TupleStruct(i32, [usize; 0]); + +struct LotsOfFields { + f1: u32, + f2: u32, + f3: u32, + f4: u32, + f5: u32, + f6: u32, + f7: u32, + f8: u32, + f9: u32, + f10: u32, + f11: u32, + f12: u32, + f13: u32, + f14: u32, + f15: u32, + f16: u32, + last: [usize; 0], +} + +// Don't lint + +#[repr(C)] +struct GoodReason { + field: i32, + last: [usize; 0], +} + +#[repr(C)] +struct OnlyFieldWithReprC { + first_and_last: [usize; 0], +} + +struct NonZeroSizedArray { + field: i32, + last: [usize; 1], +} + +struct NotLastField { + f1: u32, + zero_sized: [usize; 0], + last: i32, +} + +const ONE: usize = 1; +struct NonZeroSizedWithConst { + field: i32, + last: [usize; ONE], +} + +#[derive(Debug)] +#[repr(C)] +struct AlsoADeriveAttribute { + field: i32, + last: [usize; 0], +} + +#[must_use] +#[repr(C)] +struct AlsoAnotherAttribute { + field: i32, + last: [usize; 0], +} + +#[repr(packed)] +struct ReprPacked { + field: i32, + last: [usize; 0], +} + +#[repr(C, packed)] +struct ReprCPacked { + field: i32, + last: [usize; 0], +} + +#[repr(align(64))] +struct ReprAlign { + field: i32, + last: [usize; 0], +} +#[repr(C, align(64))] +struct ReprCAlign { + field: i32, + last: [usize; 0], +} + +// NOTE: because of https://doc.rust-lang.org/stable/reference/type-layout.html#primitive-representation-of-enums-with-fields and I'm not sure when in the compilation pipeline that would happen +#[repr(C)] +enum DontLintAnonymousStructsFromDesuraging { + A(u32), + B(f32, [u64; 0]), + C { x: u32, y: [u64; 0] }, +} + +#[repr(C)] +struct TupleStructReprC(i32, [usize; 0]); + +type NamedTuple = (i32, [usize; 0]); + +#[rustfmt::skip] // [rustfmt#4995](https://github.com/rust-lang/rustfmt/issues/4995) +struct ConstParamZeroDefault { + field: i32, + last: [usize; N], +} + +struct ConstParamNoDefault { + field: i32, + last: [usize; N], +} + +#[rustfmt::skip] +struct ConstParamNonZeroDefault { + field: i32, + last: [usize; N], +} + +struct TwoGenericParams { + field: i32, + last: [T; N], +} + +type A = ConstParamZeroDefault; +type B = ConstParamZeroDefault<0>; +type C = ConstParamNoDefault<0>; +type D = ConstParamNonZeroDefault<0>; + +fn main() {} diff --git a/src/tools/clippy/tests/ui/trailing_empty_array.stderr b/src/tools/clippy/tests/ui/trailing_empty_array.stderr new file mode 100644 index 0000000000000..d88aa0504b537 --- /dev/null +++ b/src/tools/clippy/tests/ui/trailing_empty_array.stderr @@ -0,0 +1,120 @@ +error: trailing zero-sized array in a struct which is not marked with a `repr` attribute + --> $DIR/trailing_empty_array.rs:6:1 + | +LL | / struct RarelyUseful { +LL | | field: i32, +LL | | last: [usize; 0], +LL | | } + | |_^ + | + = note: `-D clippy::trailing-empty-array` implied by `-D warnings` + = help: consider annotating `RarelyUseful` with `#[repr(C)]` or another `repr` attribute + +error: trailing zero-sized array in a struct which is not marked with a `repr` attribute + --> $DIR/trailing_empty_array.rs:11:1 + | +LL | / struct OnlyField { +LL | | first_and_last: [usize; 0], +LL | | } + | |_^ + | + = help: consider annotating `OnlyField` with `#[repr(C)]` or another `repr` attribute + +error: trailing zero-sized array in a struct which is not marked with a `repr` attribute + --> $DIR/trailing_empty_array.rs:15:1 + | +LL | / struct GenericArrayType { +LL | | field: i32, +LL | | last: [T; 0], +LL | | } + | |_^ + | + = help: consider annotating `GenericArrayType` with `#[repr(C)]` or another `repr` attribute + +error: trailing zero-sized array in a struct which is not marked with a `repr` attribute + --> $DIR/trailing_empty_array.rs:21:1 + | +LL | / struct OnlyAnotherAttribute { +LL | | field: i32, +LL | | last: [usize; 0], +LL | | } + | |_^ + | + = help: consider annotating `OnlyAnotherAttribute` with `#[repr(C)]` or another `repr` attribute + +error: trailing zero-sized array in a struct which is not marked with a `repr` attribute + --> $DIR/trailing_empty_array.rs:27:1 + | +LL | / struct OnlyADeriveAttribute { +LL | | field: i32, +LL | | last: [usize; 0], +LL | | } + | |_^ + | + = help: consider annotating `OnlyADeriveAttribute` with `#[repr(C)]` or another `repr` attribute + +error: trailing zero-sized array in a struct which is not marked with a `repr` attribute + --> $DIR/trailing_empty_array.rs:33:1 + | +LL | / struct ZeroSizedWithConst { +LL | | field: i32, +LL | | last: [usize; ZERO], +LL | | } + | |_^ + | + = help: consider annotating `ZeroSizedWithConst` with `#[repr(C)]` or another `repr` attribute + +error: trailing zero-sized array in a struct which is not marked with a `repr` attribute + --> $DIR/trailing_empty_array.rs:42:1 + | +LL | / struct ZeroSizedWithConstFunction { +LL | | field: i32, +LL | | last: [usize; compute_zero()], +LL | | } + | |_^ + | + = help: consider annotating `ZeroSizedWithConstFunction` with `#[repr(C)]` or another `repr` attribute + +error: trailing zero-sized array in a struct which is not marked with a `repr` attribute + --> $DIR/trailing_empty_array.rs:50:1 + | +LL | / struct ZeroSizedWithConstFunction2 { +LL | | field: i32, +LL | | last: [usize; compute_zero_from_arg(1)], +LL | | } + | |_^ + | + = help: consider annotating `ZeroSizedWithConstFunction2` with `#[repr(C)]` or another `repr` attribute + +error: trailing zero-sized array in a struct which is not marked with a `repr` attribute + --> $DIR/trailing_empty_array.rs:55:1 + | +LL | struct ZeroSizedArrayWrapper([usize; 0]); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = help: consider annotating `ZeroSizedArrayWrapper` with `#[repr(C)]` or another `repr` attribute + +error: trailing zero-sized array in a struct which is not marked with a `repr` attribute + --> $DIR/trailing_empty_array.rs:57:1 + | +LL | struct TupleStruct(i32, [usize; 0]); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = help: consider annotating `TupleStruct` with `#[repr(C)]` or another `repr` attribute + +error: trailing zero-sized array in a struct which is not marked with a `repr` attribute + --> $DIR/trailing_empty_array.rs:59:1 + | +LL | / struct LotsOfFields { +LL | | f1: u32, +LL | | f2: u32, +LL | | f3: u32, +... | +LL | | last: [usize; 0], +LL | | } + | |_^ + | + = help: consider annotating `LotsOfFields` with `#[repr(C)]` or another `repr` attribute + +error: aborting due to 11 previous errors + diff --git a/src/tools/clippy/tests/ui/transmute.rs b/src/tools/clippy/tests/ui/transmute.rs index bce4c81b78aa1..6a7037d8f3826 100644 --- a/src/tools/clippy/tests/ui/transmute.rs +++ b/src/tools/clippy/tests/ui/transmute.rs @@ -103,6 +103,33 @@ mod int_to_float { } } +mod num_to_bytes { + fn test() { + unsafe { + let _: [u8; 1] = std::mem::transmute(0u8); + let _: [u8; 4] = std::mem::transmute(0u32); + let _: [u8; 16] = std::mem::transmute(0u128); + let _: [u8; 1] = std::mem::transmute(0i8); + let _: [u8; 4] = std::mem::transmute(0i32); + let _: [u8; 16] = std::mem::transmute(0i128); + let _: [u8; 4] = std::mem::transmute(0.0f32); + let _: [u8; 8] = std::mem::transmute(0.0f64); + } + } + const fn test_const() { + unsafe { + let _: [u8; 1] = std::mem::transmute(0u8); + let _: [u8; 4] = std::mem::transmute(0u32); + let _: [u8; 16] = std::mem::transmute(0u128); + let _: [u8; 1] = std::mem::transmute(0i8); + let _: [u8; 4] = std::mem::transmute(0i32); + let _: [u8; 16] = std::mem::transmute(0i128); + let _: [u8; 4] = std::mem::transmute(0.0f32); + let _: [u8; 8] = std::mem::transmute(0.0f64); + } + } +} + fn bytes_to_str(b: &[u8], mb: &mut [u8]) { let _: &str = unsafe { std::mem::transmute(b) }; let _: &mut str = unsafe { std::mem::transmute(mb) }; diff --git a/src/tools/clippy/tests/ui/transmute.stderr b/src/tools/clippy/tests/ui/transmute.stderr index e31accb982af3..86537153e3228 100644 --- a/src/tools/clippy/tests/ui/transmute.stderr +++ b/src/tools/clippy/tests/ui/transmute.stderr @@ -140,8 +140,94 @@ error: transmute from a `i64` to a `f64` LL | let _: f64 = unsafe { std::mem::transmute(0_i64) }; | ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using: `f64::from_bits(0_i64 as u64)` +error: transmute from a `u8` to a `[u8; 1]` + --> $DIR/transmute.rs:109:30 + | +LL | let _: [u8; 1] = std::mem::transmute(0u8); + | ^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using `to_ne_bytes()`: `0u8.to_ne_bytes()` + | + = note: `-D clippy::transmute-num-to-bytes` implied by `-D warnings` + +error: transmute from a `u32` to a `[u8; 4]` + --> $DIR/transmute.rs:110:30 + | +LL | let _: [u8; 4] = std::mem::transmute(0u32); + | ^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using `to_ne_bytes()`: `0u32.to_ne_bytes()` + +error: transmute from a `u128` to a `[u8; 16]` + --> $DIR/transmute.rs:111:31 + | +LL | let _: [u8; 16] = std::mem::transmute(0u128); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using `to_ne_bytes()`: `0u128.to_ne_bytes()` + +error: transmute from a `i8` to a `[u8; 1]` + --> $DIR/transmute.rs:112:30 + | +LL | let _: [u8; 1] = std::mem::transmute(0i8); + | ^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using `to_ne_bytes()`: `0i8.to_ne_bytes()` + +error: transmute from a `i32` to a `[u8; 4]` + --> $DIR/transmute.rs:113:30 + | +LL | let _: [u8; 4] = std::mem::transmute(0i32); + | ^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using `to_ne_bytes()`: `0i32.to_ne_bytes()` + +error: transmute from a `i128` to a `[u8; 16]` + --> $DIR/transmute.rs:114:31 + | +LL | let _: [u8; 16] = std::mem::transmute(0i128); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using `to_ne_bytes()`: `0i128.to_ne_bytes()` + +error: transmute from a `f32` to a `[u8; 4]` + --> $DIR/transmute.rs:115:30 + | +LL | let _: [u8; 4] = std::mem::transmute(0.0f32); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using `to_ne_bytes()`: `0.0f32.to_ne_bytes()` + +error: transmute from a `f64` to a `[u8; 8]` + --> $DIR/transmute.rs:116:30 + | +LL | let _: [u8; 8] = std::mem::transmute(0.0f64); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using `to_ne_bytes()`: `0.0f64.to_ne_bytes()` + +error: transmute from a `u8` to a `[u8; 1]` + --> $DIR/transmute.rs:121:30 + | +LL | let _: [u8; 1] = std::mem::transmute(0u8); + | ^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using `to_ne_bytes()`: `0u8.to_ne_bytes()` + +error: transmute from a `u32` to a `[u8; 4]` + --> $DIR/transmute.rs:122:30 + | +LL | let _: [u8; 4] = std::mem::transmute(0u32); + | ^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using `to_ne_bytes()`: `0u32.to_ne_bytes()` + +error: transmute from a `u128` to a `[u8; 16]` + --> $DIR/transmute.rs:123:31 + | +LL | let _: [u8; 16] = std::mem::transmute(0u128); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using `to_ne_bytes()`: `0u128.to_ne_bytes()` + +error: transmute from a `i8` to a `[u8; 1]` + --> $DIR/transmute.rs:124:30 + | +LL | let _: [u8; 1] = std::mem::transmute(0i8); + | ^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using `to_ne_bytes()`: `0i8.to_ne_bytes()` + +error: transmute from a `i32` to a `[u8; 4]` + --> $DIR/transmute.rs:125:30 + | +LL | let _: [u8; 4] = std::mem::transmute(0i32); + | ^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using `to_ne_bytes()`: `0i32.to_ne_bytes()` + +error: transmute from a `i128` to a `[u8; 16]` + --> $DIR/transmute.rs:126:31 + | +LL | let _: [u8; 16] = std::mem::transmute(0i128); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using `to_ne_bytes()`: `0i128.to_ne_bytes()` + error: transmute from a `&[u8]` to a `&str` - --> $DIR/transmute.rs:107:28 + --> $DIR/transmute.rs:134:28 | LL | let _: &str = unsafe { std::mem::transmute(b) }; | ^^^^^^^^^^^^^^^^^^^^^^ help: consider using: `std::str::from_utf8(b).unwrap()` @@ -149,10 +235,10 @@ LL | let _: &str = unsafe { std::mem::transmute(b) }; = note: `-D clippy::transmute-bytes-to-str` implied by `-D warnings` error: transmute from a `&mut [u8]` to a `&mut str` - --> $DIR/transmute.rs:108:32 + --> $DIR/transmute.rs:135:32 | LL | let _: &mut str = unsafe { std::mem::transmute(mb) }; | ^^^^^^^^^^^^^^^^^^^^^^^ help: consider using: `std::str::from_utf8_mut(mb).unwrap()` -error: aborting due to 24 previous errors +error: aborting due to 38 previous errors diff --git a/src/tools/clippy/tests/ui/undocumented_unsafe_blocks.rs b/src/tools/clippy/tests/ui/undocumented_unsafe_blocks.rs new file mode 100644 index 0000000000000..52577323a5837 --- /dev/null +++ b/src/tools/clippy/tests/ui/undocumented_unsafe_blocks.rs @@ -0,0 +1,287 @@ +#![warn(clippy::undocumented_unsafe_blocks)] + +// Valid comments + +fn nested_local() { + let _ = { + let _ = { + // Safety: + let _ = unsafe {}; + }; + }; +} + +fn deep_nest() { + let _ = { + let _ = { + // Safety: + let _ = unsafe {}; + + // Safety: + unsafe {}; + + let _ = { + let _ = { + let _ = { + let _ = { + let _ = { + // Safety: + let _ = unsafe {}; + + // Safety: + unsafe {}; + }; + }; + }; + + // Safety: + unsafe {}; + }; + }; + }; + + // Safety: + unsafe {}; + }; + + // Safety: + unsafe {}; +} + +fn local_tuple_expression() { + // Safety: + let _ = (42, unsafe {}); +} + +fn line_comment() { + // Safety: + unsafe {} +} + +fn line_comment_newlines() { + // Safety: + + unsafe {} +} + +fn line_comment_empty() { + // Safety: + // + // + // + unsafe {} +} + +fn line_comment_with_extras() { + // This is a description + // Safety: + unsafe {} +} + +fn block_comment() { + /* Safety: */ + unsafe {} +} + +fn block_comment_newlines() { + /* Safety: */ + + unsafe {} +} + +#[rustfmt::skip] +fn inline_block_comment() { + /* Safety: */unsafe {} +} + +fn block_comment_with_extras() { + /* This is a description + * Safety: + */ + unsafe {} +} + +fn block_comment_terminator_same_line() { + /* This is a description + * Safety: */ + unsafe {} +} + +fn buried_safety() { + // Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor + // incididunt ut labore et dolore magna aliqua. Ut enim ad minim veniam, quis nostrud exercitation + // ullamco laboris nisi ut aliquip ex ea commodo consequat. Duis aute irure dolor in + // reprehenderit in voluptate velit esse cillum dolore eu fugiat nulla pariatur. Excepteur sint + // occaecat cupidatat non proident, sunt in culpa qui officia deserunt mollit anim id est + // laborum. Safety: + // Tellus elementum sagittis vitae et leo duis ut diam quam. Sit amet nulla facilisi + // morbi tempus iaculis urna. Amet luctus venenatis lectus magna. At quis risus sed vulputate odio + // ut. Luctus venenatis lectus magna fringilla urna. Tortor id aliquet lectus proin nibh nisl + // condimentum id venenatis. Vulputate dignissim suspendisse in est ante in nibh mauris cursus. + unsafe {} +} + +fn safety_with_prepended_text() { + // This is a test. Safety: + unsafe {} +} + +fn local_line_comment() { + // Safety: + let _ = unsafe {}; +} + +fn local_block_comment() { + /* Safety: */ + let _ = unsafe {}; +} + +fn comment_array() { + // Safety: + let _ = [unsafe { 14 }, unsafe { 15 }, 42, unsafe { 16 }]; +} + +fn comment_tuple() { + // Safety: + let _ = (42, unsafe {}, "test", unsafe {}); +} + +fn comment_unary() { + // Safety: + let _ = *unsafe { &42 }; +} + +#[allow(clippy::match_single_binding)] +fn comment_match() { + // Safety: + let _ = match unsafe {} { + _ => {}, + }; +} + +fn comment_addr_of() { + // Safety: + let _ = &unsafe {}; +} + +fn comment_repeat() { + // Safety: + let _ = [unsafe {}; 5]; +} + +fn comment_macro_call() { + macro_rules! t { + ($b:expr) => { + $b + }; + } + + t!( + // Safety: + unsafe {} + ); +} + +fn comment_macro_def() { + macro_rules! t { + () => { + // Safety: + unsafe {} + }; + } + + t!(); +} + +fn non_ascii_comment() { + // ॐ᧻໒ Safety: ௵∰ + unsafe {}; +} + +fn local_commented_block() { + let _ = + // Safety: + unsafe {}; +} + +fn local_nest() { + // Safety: + let _ = [(42, unsafe {}, unsafe {}), (52, unsafe {}, unsafe {})]; +} + +// Invalid comments + +fn no_comment() { + unsafe {} +} + +fn no_comment_array() { + let _ = [unsafe { 14 }, unsafe { 15 }, 42, unsafe { 16 }]; +} + +fn no_comment_tuple() { + let _ = (42, unsafe {}, "test", unsafe {}); +} + +fn no_comment_unary() { + let _ = *unsafe { &42 }; +} + +#[allow(clippy::match_single_binding)] +fn no_comment_match() { + let _ = match unsafe {} { + _ => {}, + }; +} + +fn no_comment_addr_of() { + let _ = &unsafe {}; +} + +fn no_comment_repeat() { + let _ = [unsafe {}; 5]; +} + +fn local_no_comment() { + let _ = unsafe {}; +} + +fn no_comment_macro_call() { + macro_rules! t { + ($b:expr) => { + $b + }; + } + + t!(unsafe {}); +} + +fn no_comment_macro_def() { + macro_rules! t { + () => { + unsafe {} + }; + } + + t!(); +} + +fn trailing_comment() { + unsafe {} // Safety: +} + +fn internal_comment() { + unsafe { + // Safety: + } +} + +fn interference() { + // Safety + + let _ = 42; + + unsafe {}; +} + +fn main() {} diff --git a/src/tools/clippy/tests/ui/undocumented_unsafe_blocks.stderr b/src/tools/clippy/tests/ui/undocumented_unsafe_blocks.stderr new file mode 100644 index 0000000000000..613e9ffca4565 --- /dev/null +++ b/src/tools/clippy/tests/ui/undocumented_unsafe_blocks.stderr @@ -0,0 +1,159 @@ +error: unsafe block missing a safety comment + --> $DIR/undocumented_unsafe_blocks.rs:215:5 + | +LL | unsafe {} + | ^^^^^^^^^ + | + = note: `-D clippy::undocumented-unsafe-blocks` implied by `-D warnings` +help: consider adding a safety comment + | +LL ~ // Safety: ... +LL + unsafe {} + | + +error: unsafe block missing a safety comment + --> $DIR/undocumented_unsafe_blocks.rs:219:5 + | +LL | let _ = [unsafe { 14 }, unsafe { 15 }, 42, unsafe { 16 }]; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: consider adding a safety comment + | +LL ~ // Safety: ... +LL + let _ = [unsafe { 14 }, unsafe { 15 }, 42, unsafe { 16 }]; + | + +error: unsafe block missing a safety comment + --> $DIR/undocumented_unsafe_blocks.rs:223:5 + | +LL | let _ = (42, unsafe {}, "test", unsafe {}); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: consider adding a safety comment + | +LL ~ // Safety: ... +LL + let _ = (42, unsafe {}, "test", unsafe {}); + | + +error: unsafe block missing a safety comment + --> $DIR/undocumented_unsafe_blocks.rs:227:5 + | +LL | let _ = *unsafe { &42 }; + | ^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: consider adding a safety comment + | +LL ~ // Safety: ... +LL + let _ = *unsafe { &42 }; + | + +error: unsafe block missing a safety comment + --> $DIR/undocumented_unsafe_blocks.rs:232:5 + | +LL | let _ = match unsafe {} { + | ^^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: consider adding a safety comment + | +LL ~ // Safety: ... +LL + let _ = match unsafe {} { + | + +error: unsafe block missing a safety comment + --> $DIR/undocumented_unsafe_blocks.rs:238:5 + | +LL | let _ = &unsafe {}; + | ^^^^^^^^^^^^^^^^^^^ + | +help: consider adding a safety comment + | +LL ~ // Safety: ... +LL + let _ = &unsafe {}; + | + +error: unsafe block missing a safety comment + --> $DIR/undocumented_unsafe_blocks.rs:242:5 + | +LL | let _ = [unsafe {}; 5]; + | ^^^^^^^^^^^^^^^^^^^^^^^ + | +help: consider adding a safety comment + | +LL ~ // Safety: ... +LL + let _ = [unsafe {}; 5]; + | + +error: unsafe block missing a safety comment + --> $DIR/undocumented_unsafe_blocks.rs:246:5 + | +LL | let _ = unsafe {}; + | ^^^^^^^^^^^^^^^^^^ + | +help: consider adding a safety comment + | +LL ~ // Safety: ... +LL + let _ = unsafe {}; + | + +error: unsafe block missing a safety comment + --> $DIR/undocumented_unsafe_blocks.rs:256:8 + | +LL | t!(unsafe {}); + | ^^^^^^^^^ + | +help: consider adding a safety comment + | +LL ~ t!(// Safety: ... +LL ~ unsafe {}); + | + +error: unsafe block in macro expansion missing a safety comment + --> $DIR/undocumented_unsafe_blocks.rs:262:13 + | +LL | unsafe {} + | ^^^^^^^^^ +... +LL | t!(); + | ---- in this macro invocation + | + = help: consider adding a safety comment in the macro definition + = note: this error originates in the macro `t` (in Nightly builds, run with -Z macro-backtrace for more info) + +error: unsafe block missing a safety comment + --> $DIR/undocumented_unsafe_blocks.rs:270:5 + | +LL | unsafe {} // Safety: + | ^^^^^^^^^ + | +help: consider adding a safety comment + | +LL ~ // Safety: ... +LL ~ unsafe {} // Safety: + | + +error: unsafe block missing a safety comment + --> $DIR/undocumented_unsafe_blocks.rs:274:5 + | +LL | unsafe { + | ^^^^^^^^ + | +help: consider adding a safety comment + | +LL ~ // Safety: ... +LL + unsafe { + | + +error: unsafe block missing a safety comment + --> $DIR/undocumented_unsafe_blocks.rs:284:5 + | +LL | unsafe {}; + | ^^^^^^^^^ + | +help: consider adding a safety comment + | +LL ~ // Safety: ... +LL ~ unsafe {}; + | + +error: aborting due to 13 previous errors + diff --git a/src/tools/clippy/tests/ui/uninit_vec.rs b/src/tools/clippy/tests/ui/uninit_vec.rs new file mode 100644 index 0000000000000..dc150cf28f2cc --- /dev/null +++ b/src/tools/clippy/tests/ui/uninit_vec.rs @@ -0,0 +1,94 @@ +#![warn(clippy::uninit_vec)] + +use std::mem::MaybeUninit; + +#[derive(Default)] +struct MyVec { + vec: Vec, +} + +fn main() { + // with_capacity() -> set_len() should be detected + let mut vec: Vec = Vec::with_capacity(1000); + unsafe { + vec.set_len(200); + } + + // reserve() -> set_len() should be detected + vec.reserve(1000); + unsafe { + vec.set_len(200); + } + + // new() -> set_len() should be detected + let mut vec: Vec = Vec::new(); + unsafe { + vec.set_len(200); + } + + // default() -> set_len() should be detected + let mut vec: Vec = Default::default(); + unsafe { + vec.set_len(200); + } + + let mut vec: Vec = Vec::default(); + unsafe { + vec.set_len(200); + } + + // test when both calls are enclosed in the same unsafe block + unsafe { + let mut vec: Vec = Vec::with_capacity(1000); + vec.set_len(200); + + vec.reserve(1000); + vec.set_len(200); + } + + let mut vec: Vec = Vec::with_capacity(1000); + unsafe { + // test the case where there are other statements in the following unsafe block + vec.set_len(200); + assert!(vec.len() == 200); + } + + // handle vec stored in the field of a struct + let mut my_vec = MyVec::default(); + my_vec.vec.reserve(1000); + unsafe { + my_vec.vec.set_len(200); + } + + my_vec.vec = Vec::with_capacity(1000); + unsafe { + my_vec.vec.set_len(200); + } + + // Test `#[allow(...)]` attributes on inner unsafe block (shouldn't trigger) + let mut vec: Vec = Vec::with_capacity(1000); + #[allow(clippy::uninit_vec)] + unsafe { + vec.set_len(200); + } + + // MaybeUninit-wrapped types should not be detected + unsafe { + let mut vec: Vec> = Vec::with_capacity(1000); + vec.set_len(200); + + let mut vec: Vec<(MaybeUninit, MaybeUninit)> = Vec::with_capacity(1000); + vec.set_len(200); + + let mut vec: Vec<(MaybeUninit, [MaybeUninit; 2])> = Vec::with_capacity(1000); + vec.set_len(200); + } + + // known false negative + let mut vec1: Vec = Vec::with_capacity(1000); + let mut vec2: Vec = Vec::with_capacity(1000); + unsafe { + vec1.set_len(200); + vec2.set_len(200); + } +} diff --git a/src/tools/clippy/tests/ui/uninit_vec.stderr b/src/tools/clippy/tests/ui/uninit_vec.stderr new file mode 100644 index 0000000000000..520bfb26b62e1 --- /dev/null +++ b/src/tools/clippy/tests/ui/uninit_vec.stderr @@ -0,0 +1,105 @@ +error: calling `set_len()` immediately after reserving a buffer creates uninitialized values + --> $DIR/uninit_vec.rs:12:5 + | +LL | let mut vec: Vec = Vec::with_capacity(1000); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +LL | unsafe { +LL | vec.set_len(200); + | ^^^^^^^^^^^^^^^^ + | + = note: `-D clippy::uninit-vec` implied by `-D warnings` + = help: initialize the buffer or wrap the content in `MaybeUninit` + +error: calling `set_len()` immediately after reserving a buffer creates uninitialized values + --> $DIR/uninit_vec.rs:18:5 + | +LL | vec.reserve(1000); + | ^^^^^^^^^^^^^^^^^^ +LL | unsafe { +LL | vec.set_len(200); + | ^^^^^^^^^^^^^^^^ + | + = help: initialize the buffer or wrap the content in `MaybeUninit` + +error: calling `set_len()` on empty `Vec` creates out-of-bound values + --> $DIR/uninit_vec.rs:24:5 + | +LL | let mut vec: Vec = Vec::new(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +LL | unsafe { +LL | vec.set_len(200); + | ^^^^^^^^^^^^^^^^ + +error: calling `set_len()` on empty `Vec` creates out-of-bound values + --> $DIR/uninit_vec.rs:30:5 + | +LL | let mut vec: Vec = Default::default(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +LL | unsafe { +LL | vec.set_len(200); + | ^^^^^^^^^^^^^^^^ + +error: calling `set_len()` on empty `Vec` creates out-of-bound values + --> $DIR/uninit_vec.rs:35:5 + | +LL | let mut vec: Vec = Vec::default(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +LL | unsafe { +LL | vec.set_len(200); + | ^^^^^^^^^^^^^^^^ + +error: calling `set_len()` immediately after reserving a buffer creates uninitialized values + --> $DIR/uninit_vec.rs:49:5 + | +LL | let mut vec: Vec = Vec::with_capacity(1000); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +... +LL | vec.set_len(200); + | ^^^^^^^^^^^^^^^^ + | + = help: initialize the buffer or wrap the content in `MaybeUninit` + +error: calling `set_len()` immediately after reserving a buffer creates uninitialized values + --> $DIR/uninit_vec.rs:58:5 + | +LL | my_vec.vec.reserve(1000); + | ^^^^^^^^^^^^^^^^^^^^^^^^^ +LL | unsafe { +LL | my_vec.vec.set_len(200); + | ^^^^^^^^^^^^^^^^^^^^^^^ + | + = help: initialize the buffer or wrap the content in `MaybeUninit` + +error: calling `set_len()` immediately after reserving a buffer creates uninitialized values + --> $DIR/uninit_vec.rs:63:5 + | +LL | my_vec.vec = Vec::with_capacity(1000); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +LL | unsafe { +LL | my_vec.vec.set_len(200); + | ^^^^^^^^^^^^^^^^^^^^^^^ + | + = help: initialize the buffer or wrap the content in `MaybeUninit` + +error: calling `set_len()` immediately after reserving a buffer creates uninitialized values + --> $DIR/uninit_vec.rs:42:9 + | +LL | let mut vec: Vec = Vec::with_capacity(1000); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +LL | vec.set_len(200); + | ^^^^^^^^^^^^^^^^ + | + = help: initialize the buffer or wrap the content in `MaybeUninit` + +error: calling `set_len()` immediately after reserving a buffer creates uninitialized values + --> $DIR/uninit_vec.rs:45:9 + | +LL | vec.reserve(1000); + | ^^^^^^^^^^^^^^^^^^ +LL | vec.set_len(200); + | ^^^^^^^^^^^^^^^^ + | + = help: initialize the buffer or wrap the content in `MaybeUninit` + +error: aborting due to 10 previous errors + diff --git a/src/tools/clippy/tests/ui/unnecessary_sort_by.fixed b/src/tools/clippy/tests/ui/unnecessary_sort_by.fixed index b45b27d8f23b1..d806d620b176a 100644 --- a/src/tools/clippy/tests/ui/unnecessary_sort_by.fixed +++ b/src/tools/clippy/tests/ui/unnecessary_sort_by.fixed @@ -2,6 +2,7 @@ #![allow(clippy::stable_sort_primitive)] +use std::cell::Ref; use std::cmp::Reverse; fn unnecessary_sort_by() { @@ -33,6 +34,10 @@ fn unnecessary_sort_by() { // `Reverse(b)` would borrow in the following cases, don't lint vec.sort_by(|a, b| b.cmp(a)); vec.sort_unstable_by(|a, b| b.cmp(a)); + + // No warning if element does not implement `Ord` + let mut vec: Vec> = Vec::new(); + vec.sort_unstable_by(|a, b| a.cmp(b)); } // Do not suggest returning a reference to the closure parameter of `Vec::sort_by_key` diff --git a/src/tools/clippy/tests/ui/unnecessary_sort_by.rs b/src/tools/clippy/tests/ui/unnecessary_sort_by.rs index be2abe7f7014d..6ee9c3af455df 100644 --- a/src/tools/clippy/tests/ui/unnecessary_sort_by.rs +++ b/src/tools/clippy/tests/ui/unnecessary_sort_by.rs @@ -2,6 +2,7 @@ #![allow(clippy::stable_sort_primitive)] +use std::cell::Ref; use std::cmp::Reverse; fn unnecessary_sort_by() { @@ -33,6 +34,10 @@ fn unnecessary_sort_by() { // `Reverse(b)` would borrow in the following cases, don't lint vec.sort_by(|a, b| b.cmp(a)); vec.sort_unstable_by(|a, b| b.cmp(a)); + + // No warning if element does not implement `Ord` + let mut vec: Vec> = Vec::new(); + vec.sort_unstable_by(|a, b| a.cmp(b)); } // Do not suggest returning a reference to the closure parameter of `Vec::sort_by_key` diff --git a/src/tools/clippy/tests/ui/unnecessary_sort_by.stderr b/src/tools/clippy/tests/ui/unnecessary_sort_by.stderr index 50607933e18f7..ca9641e880316 100644 --- a/src/tools/clippy/tests/ui/unnecessary_sort_by.stderr +++ b/src/tools/clippy/tests/ui/unnecessary_sort_by.stderr @@ -1,5 +1,5 @@ error: use Vec::sort here instead - --> $DIR/unnecessary_sort_by.rs:14:5 + --> $DIR/unnecessary_sort_by.rs:15:5 | LL | vec.sort_by(|a, b| a.cmp(b)); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `vec.sort()` @@ -7,67 +7,67 @@ LL | vec.sort_by(|a, b| a.cmp(b)); = note: `-D clippy::unnecessary-sort-by` implied by `-D warnings` error: use Vec::sort here instead - --> $DIR/unnecessary_sort_by.rs:15:5 + --> $DIR/unnecessary_sort_by.rs:16:5 | LL | vec.sort_unstable_by(|a, b| a.cmp(b)); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `vec.sort_unstable()` error: use Vec::sort_by_key here instead - --> $DIR/unnecessary_sort_by.rs:16:5 + --> $DIR/unnecessary_sort_by.rs:17:5 | LL | vec.sort_by(|a, b| (a + 5).abs().cmp(&(b + 5).abs())); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `vec.sort_by_key(|a| (a + 5).abs())` error: use Vec::sort_by_key here instead - --> $DIR/unnecessary_sort_by.rs:17:5 + --> $DIR/unnecessary_sort_by.rs:18:5 | LL | vec.sort_unstable_by(|a, b| id(-a).cmp(&id(-b))); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `vec.sort_unstable_by_key(|a| id(-a))` error: use Vec::sort_by_key here instead - --> $DIR/unnecessary_sort_by.rs:20:5 + --> $DIR/unnecessary_sort_by.rs:21:5 | LL | vec.sort_by(|a, b| (b + 5).abs().cmp(&(a + 5).abs())); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `vec.sort_by_key(|b| Reverse((b + 5).abs()))` error: use Vec::sort_by_key here instead - --> $DIR/unnecessary_sort_by.rs:21:5 + --> $DIR/unnecessary_sort_by.rs:22:5 | LL | vec.sort_unstable_by(|a, b| id(-b).cmp(&id(-a))); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `vec.sort_unstable_by_key(|b| Reverse(id(-b)))` error: use Vec::sort_by_key here instead - --> $DIR/unnecessary_sort_by.rs:31:5 + --> $DIR/unnecessary_sort_by.rs:32:5 | LL | vec.sort_by(|a, b| (***a).abs().cmp(&(***b).abs())); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `vec.sort_by_key(|a| (***a).abs())` error: use Vec::sort_by_key here instead - --> $DIR/unnecessary_sort_by.rs:32:5 + --> $DIR/unnecessary_sort_by.rs:33:5 | LL | vec.sort_unstable_by(|a, b| (***a).abs().cmp(&(***b).abs())); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `vec.sort_unstable_by_key(|a| (***a).abs())` error: use Vec::sort_by_key here instead - --> $DIR/unnecessary_sort_by.rs:88:9 + --> $DIR/unnecessary_sort_by.rs:93:9 | LL | args.sort_by(|a, b| a.name().cmp(&b.name())); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `args.sort_by_key(|a| a.name())` error: use Vec::sort_by_key here instead - --> $DIR/unnecessary_sort_by.rs:89:9 + --> $DIR/unnecessary_sort_by.rs:94:9 | LL | args.sort_unstable_by(|a, b| a.name().cmp(&b.name())); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `args.sort_unstable_by_key(|a| a.name())` error: use Vec::sort_by_key here instead - --> $DIR/unnecessary_sort_by.rs:91:9 + --> $DIR/unnecessary_sort_by.rs:96:9 | LL | args.sort_by(|a, b| b.name().cmp(&a.name())); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `args.sort_by_key(|b| Reverse(b.name()))` error: use Vec::sort_by_key here instead - --> $DIR/unnecessary_sort_by.rs:92:9 + --> $DIR/unnecessary_sort_by.rs:97:9 | LL | args.sort_unstable_by(|a, b| b.name().cmp(&a.name())); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `args.sort_unstable_by_key(|b| Reverse(b.name()))` diff --git a/src/tools/clippy/tests/ui/wildcard_imports.fixed b/src/tools/clippy/tests/ui/wildcard_imports.fixed index ee9c9045fff55..98bc1e80731ff 100644 --- a/src/tools/clippy/tests/ui/wildcard_imports.fixed +++ b/src/tools/clippy/tests/ui/wildcard_imports.fixed @@ -230,4 +230,12 @@ mod super_imports { let _ = foofoo(); } } + + mod attestation_should_be_replaced { + use super::foofoo; + + fn with_explicit() { + let _ = foofoo(); + } + } } diff --git a/src/tools/clippy/tests/ui/wildcard_imports.rs b/src/tools/clippy/tests/ui/wildcard_imports.rs index efaa8f9ef6641..4ef61f9245b58 100644 --- a/src/tools/clippy/tests/ui/wildcard_imports.rs +++ b/src/tools/clippy/tests/ui/wildcard_imports.rs @@ -231,4 +231,12 @@ mod super_imports { let _ = foofoo(); } } + + mod attestation_should_be_replaced { + use super::*; + + fn with_explicit() { + let _ = foofoo(); + } + } } diff --git a/src/tools/clippy/tests/ui/wildcard_imports.stderr b/src/tools/clippy/tests/ui/wildcard_imports.stderr index 66267dd27b84f..d7af0c046e886 100644 --- a/src/tools/clippy/tests/ui/wildcard_imports.stderr +++ b/src/tools/clippy/tests/ui/wildcard_imports.stderr @@ -122,5 +122,11 @@ error: usage of wildcard import LL | use super::super::super_imports::*; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `super::super::super_imports::foofoo` -error: aborting due to 20 previous errors +error: usage of wildcard import + --> $DIR/wildcard_imports.rs:236:13 + | +LL | use super::*; + | ^^^^^^^^ help: try: `super::foofoo` + +error: aborting due to 21 previous errors diff --git a/src/tools/clippy/tests/ui_test/eq_op.rs b/src/tools/clippy/tests/ui_test/eq_op.rs new file mode 100644 index 0000000000000..f2f5f1e588ed4 --- /dev/null +++ b/src/tools/clippy/tests/ui_test/eq_op.rs @@ -0,0 +1,15 @@ +#[warn(clippy::eq_op)] +#[test] +fn eq_op_shouldnt_trigger_in_tests() { + let a = 1; + let result = a + 1 == 1 + a; + assert!(result); +} + +#[test] +fn eq_op_macros_shouldnt_trigger_in_tests() { + let a = 1; + let b = 2; + assert_eq!(a, a); + assert_eq!(a + b, b + a); +}