diff --git a/.github/workflows/clippy.yml b/.github/workflows/clippy.yml index 32103f59d8b..d856c55a41a 100644 --- a/.github/workflows/clippy.yml +++ b/.github/workflows/clippy.yml @@ -71,7 +71,7 @@ jobs: working-directory: clippy_workspace_tests - name: Test cargo-clippy --fix - run: ../target/debug/cargo-clippy clippy --fix -Zunstable-options + run: ../target/debug/cargo-clippy clippy --fix working-directory: clippy_workspace_tests - name: Test clippy-driver diff --git a/.github/workflows/clippy_bors.yml b/.github/workflows/clippy_bors.yml index f27fee87dc1..146b6fccd0c 100644 --- a/.github/workflows/clippy_bors.yml +++ b/.github/workflows/clippy_bors.yml @@ -90,11 +90,6 @@ jobs: - name: Checkout uses: actions/checkout@v2.3.3 - # FIXME: should not be necessary once 1.24.2 is the default version on the windows runner - - name: Update rustup - run: rustup self update - if: runner.os == 'Windows' - - name: Install toolchain run: rustup show active-toolchain @@ -139,7 +134,7 @@ jobs: working-directory: clippy_workspace_tests - name: Test cargo-clippy --fix - run: ../target/debug/cargo-clippy clippy --fix -Zunstable-options + run: ../target/debug/cargo-clippy clippy --fix working-directory: clippy_workspace_tests - name: Test clippy-driver diff --git a/.github/workflows/remark.yml b/.github/workflows/remark.yml index 4f25a86b2e4..77efdec1e50 100644 --- a/.github/workflows/remark.yml +++ b/.github/workflows/remark.yml @@ -22,7 +22,7 @@ jobs: uses: actions/setup-node@v1.4.4 - name: Install remark - run: npm install remark-cli remark-lint remark-lint-maximum-line-length remark-preset-lint-recommended + run: npm install remark-cli remark-lint remark-lint-maximum-line-length remark-preset-lint-recommended remark-gfm # Run - name: Check *.md files diff --git a/.remarkrc b/.remarkrc index 0ede7ac75cb..04b82b8cc58 100644 --- a/.remarkrc +++ b/.remarkrc @@ -1,6 +1,7 @@ { "plugins": [ "remark-preset-lint-recommended", + "remark-gfm", ["remark-lint-list-item-indent", false], ["remark-lint-no-literal-urls", false], ["remark-lint-no-shortcut-reference-link", false], diff --git a/CHANGELOG.md b/CHANGELOG.md index 41af8e190dd..f3a80703238 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,11 +6,139 @@ document. ## Unreleased / In Rust Nightly -[7c7683c...master](https://github.com/rust-lang/rust-clippy/compare/7c7683c...master) +[3ae8faf...master](https://github.com/rust-lang/rust-clippy/compare/3ae8faf...master) + +## Rust 1.54 + +Current beta, release 2021-07-29 + +[7c7683c...3ae8faf](https://github.com/rust-lang/rust-clippy/compare/7c7683c...3ae8faf) + +### New Lints + +- [`ref_binding_to_reference`] + [#7105](https://github.com/rust-lang/rust-clippy/pull/7105) +- [`needless_bitwise_bool`] + [#7133](https://github.com/rust-lang/rust-clippy/pull/7133) +- [`unused_async`] [#7225](https://github.com/rust-lang/rust-clippy/pull/7225) +- [`manual_str_repeat`] + [#7265](https://github.com/rust-lang/rust-clippy/pull/7265) +- [`suspicious_splitn`] + [#7292](https://github.com/rust-lang/rust-clippy/pull/7292) + +### Moves and Deprecations + +- Deprecate `pub_enum_variant_names` and `wrong_pub_self_convention` in favor of + the new `avoid_breaking_exported_api` config option (see + [Enhancements](#1-54-enhancements)) + [#7187](https://github.com/rust-lang/rust-clippy/pull/7187) +- Move [`inconsistent_struct_constructor`] to `pedantic` + [#7193](https://github.com/rust-lang/rust-clippy/pull/7193) +- Move [`needless_borrow`] to `style` (now warn-by-default) + [#7254](https://github.com/rust-lang/rust-clippy/pull/7254) +- Move [`suspicious_operation_groupings`] to `nursery` + [#7266](https://github.com/rust-lang/rust-clippy/pull/7266) +- Move [`semicolon_if_nothing_returned`] to `pedantic` + [#7268](https://github.com/rust-lang/rust-clippy/pull/7268) + +### Enhancements <a name="1-54-enhancements"></a> + +- [`while_let_on_iterator`]: Now also lints in nested loops + [#6966](https://github.com/rust-lang/rust-clippy/pull/6966) +- [`single_char_pattern`]: Now also lints on `strip_prefix` and `strip_suffix` + [#7156](https://github.com/rust-lang/rust-clippy/pull/7156) +- [`needless_collect`]: Now also lints on assignments with type annotations + [#7163](https://github.com/rust-lang/rust-clippy/pull/7163) +- [`if_then_some_else_none`]: Now works with the MSRV config + [#7177](https://github.com/rust-lang/rust-clippy/pull/7177) +- Add `avoid_breaking_exported_api` config option for the lints + [`enum_variant_names`], [`large_types_passed_by_value`], + [`trivially_copy_pass_by_ref`], [`unnecessary_wraps`], + [`upper_case_acronyms`], and [`wrong_self_convention`]. We recommend to set + this configuration option to `false` before a major release (1.0/2.0/...) to + clean up the API [#7187](https://github.com/rust-lang/rust-clippy/pull/7187) +- [`needless_collect`]: Now lints on even more data structures + [#7188](https://github.com/rust-lang/rust-clippy/pull/7188) +- [`missing_docs_in_private_items`]: No longer sees `#[<name> = "<value>"]` like + attributes as sufficient documentation + [#7281](https://github.com/rust-lang/rust-clippy/pull/7281) +- [`needless_collect`], [`short_circuit_statement`], [`unnecessary_operation`]: + Now work as expected when used with `allow` + [#7282](https://github.com/rust-lang/rust-clippy/pull/7282) + +### False Positive Fixes + +- [`implicit_return`]: Now takes all diverging functions in account to avoid + false positives [#6951](https://github.com/rust-lang/rust-clippy/pull/6951) +- [`while_let_on_iterator`]: No longer lints when the iterator is a struct field + and the struct is used in the loop + [#6966](https://github.com/rust-lang/rust-clippy/pull/6966) +- [`multiple_inherent_impl`]: No longer lints with generic arguments + [#7089](https://github.com/rust-lang/rust-clippy/pull/7089) +- [`comparison_chain`]: No longer lints in a `const` context + [#7118](https://github.com/rust-lang/rust-clippy/pull/7118) +- [`while_immutable_condition`]: Fix false positive where mutation in the loop + variable wasn't picked up + [#7144](https://github.com/rust-lang/rust-clippy/pull/7144) +- [`default_trait_access`]: No longer lints in macros + [#7150](https://github.com/rust-lang/rust-clippy/pull/7150) +- [`needless_question_mark`]: No longer lints when the inner value is implicitly + dereferenced [#7165](https://github.com/rust-lang/rust-clippy/pull/7165) +- [`unused_unit`]: No longer lints when multiple macro contexts are involved + [#7167](https://github.com/rust-lang/rust-clippy/pull/7167) +- [`eval_order_dependence`]: Fix false positive in async context + [#7174](https://github.com/rust-lang/rust-clippy/pull/7174) +- [`unnecessary_filter_map`]: No longer lints if the `filter_map` changes the + type [#7175](https://github.com/rust-lang/rust-clippy/pull/7175) +- [`wrong_self_convention`]: No longer lints in trait implementations of + non-`Copy` types [#7182](https://github.com/rust-lang/rust-clippy/pull/7182) +- [`suboptimal_flops`]: No longer lints on `powi(2)` + [#7201](https://github.com/rust-lang/rust-clippy/pull/7201) +- [`wrong_self_convention`]: No longer lints if there is no implicit `self` + [#7215](https://github.com/rust-lang/rust-clippy/pull/7215) +- [`option_if_let_else`]: No longer lints on `else if let` pattern + [#7216](https://github.com/rust-lang/rust-clippy/pull/7216) +- [`use_self`], [`useless_conversion`]: Fix false positives when generic + arguments are involved + [#7223](https://github.com/rust-lang/rust-clippy/pull/7223) +- [`manual_unwrap_or`]: Fix false positive with deref coercion + [#7233](https://github.com/rust-lang/rust-clippy/pull/7233) +- [`similar_names`]: No longer lints on `wparam`/`lparam` + [#7255](https://github.com/rust-lang/rust-clippy/pull/7255) +- [`redundant_closure`]: No longer lints on using the `vec![]` macro in a + closure [#7263](https://github.com/rust-lang/rust-clippy/pull/7263) + +### Suggestion Fixes/Improvements + +- [`implicit_return`] + [#6951](https://github.com/rust-lang/rust-clippy/pull/6951) + - Fix suggestion for async functions + - Improve suggestion with macros + - Suggest to change `break` to `return` when appropriate +- [`while_let_on_iterator`]: Now suggests `&mut iter` when necessary + [#6966](https://github.com/rust-lang/rust-clippy/pull/6966) +- [`match_single_binding`]: Improve suggestion when match scrutinee has side + effects [#7095](https://github.com/rust-lang/rust-clippy/pull/7095) +- [`needless_borrow`]: Now suggests to also change usage sites as needed + [#7105](https://github.com/rust-lang/rust-clippy/pull/7105) +- [`write_with_newline`]: Improve suggestion when only `\n` is written to the + buffer [#7183](https://github.com/rust-lang/rust-clippy/pull/7183) +- [`from_iter_instead_of_collect`]: The suggestion is now auto applicable also + when a `<_ as Trait>::_` is involved + [#7264](https://github.com/rust-lang/rust-clippy/pull/7264) +- [`not_unsafe_ptr_arg_deref`]: Improved error message + [#7294](https://github.com/rust-lang/rust-clippy/pull/7294) + +### ICE Fixes + +- Fix ICE when running Clippy on `libstd` + [#7140](https://github.com/rust-lang/rust-clippy/pull/7140) +- [`implicit_return`] + [#7242](https://github.com/rust-lang/rust-clippy/pull/7242) ## Rust 1.53 -Current beta, release 2021-06-17 +Current stable, released 2021-06-17 [6ed6f1e...7c7683c](https://github.com/rust-lang/rust-clippy/compare/6ed6f1e...7c7683c) @@ -194,7 +322,7 @@ Current beta, release 2021-06-17 ## Rust 1.52 -Current stable, released 2021-05-06 +Released 2021-05-06 [3e41797...6ed6f1e](https://github.com/rust-lang/rust-clippy/compare/3e41797...6ed6f1e) @@ -2295,6 +2423,7 @@ Released 2018-09-13 <!-- begin autogenerated links to lint list --> [`absurd_extreme_comparisons`]: https://rust-lang.github.io/rust-clippy/master/index.html#absurd_extreme_comparisons [`almost_swapped`]: https://rust-lang.github.io/rust-clippy/master/index.html#almost_swapped +[`append_instead_of_extend`]: https://rust-lang.github.io/rust-clippy/master/index.html#append_instead_of_extend [`approx_constant`]: https://rust-lang.github.io/rust-clippy/master/index.html#approx_constant [`as_conversions`]: https://rust-lang.github.io/rust-clippy/master/index.html#as_conversions [`assertions_on_constants`]: https://rust-lang.github.io/rust-clippy/master/index.html#assertions_on_constants @@ -2358,6 +2487,8 @@ Released 2018-09-13 [`derive_hash_xor_eq`]: https://rust-lang.github.io/rust-clippy/master/index.html#derive_hash_xor_eq [`derive_ord_xor_partial_ord`]: https://rust-lang.github.io/rust-clippy/master/index.html#derive_ord_xor_partial_ord [`disallowed_method`]: https://rust-lang.github.io/rust-clippy/master/index.html#disallowed_method +[`disallowed_script_idents`]: https://rust-lang.github.io/rust-clippy/master/index.html#disallowed_script_idents +[`disallowed_type`]: https://rust-lang.github.io/rust-clippy/master/index.html#disallowed_type [`diverging_sub_expression`]: https://rust-lang.github.io/rust-clippy/master/index.html#diverging_sub_expression [`doc_markdown`]: https://rust-lang.github.io/rust-clippy/master/index.html#doc_markdown [`double_comparisons`]: https://rust-lang.github.io/rust-clippy/master/index.html#double_comparisons @@ -2527,6 +2658,7 @@ Released 2018-09-13 [`misrefactored_assign_op`]: https://rust-lang.github.io/rust-clippy/master/index.html#misrefactored_assign_op [`missing_const_for_fn`]: https://rust-lang.github.io/rust-clippy/master/index.html#missing_const_for_fn [`missing_docs_in_private_items`]: https://rust-lang.github.io/rust-clippy/master/index.html#missing_docs_in_private_items +[`missing_enforced_import_renames`]: https://rust-lang.github.io/rust-clippy/master/index.html#missing_enforced_import_renames [`missing_errors_doc`]: https://rust-lang.github.io/rust-clippy/master/index.html#missing_errors_doc [`missing_inline_in_public_items`]: https://rust-lang.github.io/rust-clippy/master/index.html#missing_inline_in_public_items [`missing_panics_doc`]: https://rust-lang.github.io/rust-clippy/master/index.html#missing_panics_doc @@ -2574,6 +2706,7 @@ Released 2018-09-13 [`non_octal_unix_permissions`]: https://rust-lang.github.io/rust-clippy/master/index.html#non_octal_unix_permissions [`nonminimal_bool`]: https://rust-lang.github.io/rust-clippy/master/index.html#nonminimal_bool [`nonsensical_open_options`]: https://rust-lang.github.io/rust-clippy/master/index.html#nonsensical_open_options +[`nonstandard_macro_braces`]: https://rust-lang.github.io/rust-clippy/master/index.html#nonstandard_macro_braces [`not_unsafe_ptr_arg_deref`]: https://rust-lang.github.io/rust-clippy/master/index.html#not_unsafe_ptr_arg_deref [`ok_expect`]: https://rust-lang.github.io/rust-clippy/master/index.html#ok_expect [`op_ref`]: https://rust-lang.github.io/rust-clippy/master/index.html#op_ref diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 7d7b7c81173..4273fda4e64 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -115,7 +115,7 @@ To work around this, you need to have a copy of the [rustc-repo][rustc_repo] ava `git clone https://github.com/rust-lang/rust/`. Then you can run a `cargo dev` command to automatically make Clippy use the rustc-repo via path-dependencies which `IntelliJ Rust` will be able to understand. -Run `cargo dev ide_setup --repo-path <repo-path>` where `<repo-path>` is a path to the rustc repo +Run `cargo dev setup intellij --repo-path <repo-path>` where `<repo-path>` is a path to the rustc repo you just cloned. The command will add path-dependencies pointing towards rustc-crates inside the rustc repo to Clippys `Cargo.toml`s and should allow `IntelliJ Rust` to understand most of the types that Clippy uses. diff --git a/Cargo.toml b/Cargo.toml index b003b15a11d..9b5d9b2adf3 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "clippy" -version = "0.1.54" +version = "0.1.55" authors = ["The Rust Clippy Developers"] description = "A bunch of helpful lints to avoid common pitfalls in Rust" repository = "https://github.com/rust-lang/rust-clippy" diff --git a/README.md b/README.md index bd322cc8070..e1c968273cd 100644 --- a/README.md +++ b/README.md @@ -10,16 +10,17 @@ A collection of lints to catch common mistakes and improve your [Rust](https://g Lints are divided into categories, each with a default [lint level](https://doc.rust-lang.org/rustc/lints/levels.html). You can choose how much Clippy is supposed to ~~annoy~~ help you by changing the lint level by category. -| Category | Description | Default level | -| --------------------- | ----------------------------------------------------------------------- | ------------- | -| `clippy::all` | all lints that are on by default (correctness, style, complexity, perf) | **warn/deny** | -| `clippy::correctness` | code that is outright wrong or very useless | **deny** | -| `clippy::style` | code that should be written in a more idiomatic way | **warn** | -| `clippy::complexity` | code that does something simple but in a complex way | **warn** | -| `clippy::perf` | code that can be written to run faster | **warn** | -| `clippy::pedantic` | lints which are rather strict or might have false positives | allow | -| `clippy::nursery` | new lints that are still under development | allow | -| `clippy::cargo` | lints for the cargo manifest | allow | +| Category | Description | Default level | +| --------------------- | ----------------------------------------------------------------------------------- | ------------- | +| `clippy::all` | all lints that are on by default (correctness, suspicious, style, complexity, perf) | **warn/deny** | +| `clippy::correctness` | code that is outright wrong or useless | **deny** | +| `clippy::suspicious` | code that is most likely wrong or useless | **warn** | +| `clippy::style` | code that should be written in a more idiomatic way | **warn** | +| `clippy::complexity` | code that does something simple but in a complex way | **warn** | +| `clippy::perf` | code that can be written to run faster | **warn** | +| `clippy::pedantic` | lints which are rather strict or might have false positives | allow | +| `clippy::nursery` | new lints that are still under development | allow | +| `clippy::cargo` | lints for the cargo manifest | allow | More to come, please [file an issue](https://github.com/rust-lang/rust-clippy/issues) if you have ideas! @@ -75,11 +76,10 @@ cargo clippy #### Automatically applying Clippy suggestions -Clippy can automatically apply some lint suggestions. -Note that this is still experimental and only supported on the nightly channel: +Clippy can automatically apply some lint suggestions, just like the compiler. ```terminal -cargo clippy --fix -Z unstable-options +cargo clippy --fix ``` #### Workspaces diff --git a/clippy_dev/Cargo.toml b/clippy_dev/Cargo.toml index b1844e29b32..5c6c106e0e6 100644 --- a/clippy_dev/Cargo.toml +++ b/clippy_dev/Cargo.toml @@ -8,7 +8,7 @@ edition = "2018" bytecount = "0.6" clap = "2.33" itertools = "0.9" -opener = "0.4" +opener = "0.5" regex = "1" shell-escape = "0.1" walkdir = "2" diff --git a/clippy_dev/src/fmt.rs b/clippy_dev/src/fmt.rs index 1517cdc9419..c81eb40d52f 100644 --- a/clippy_dev/src/fmt.rs +++ b/clippy_dev/src/fmt.rs @@ -60,11 +60,7 @@ pub fn run(check: bool, verbose: bool) { let entry = entry?; let path = entry.path(); - if path.extension() != Some("rs".as_ref()) - || entry.file_name() == "ice-3891.rs" - // Avoid rustfmt bug rust-lang/rustfmt#1873 - || cfg!(windows) && entry.file_name() == "implicit_hasher.rs" - { + if path.extension() != Some("rs".as_ref()) || entry.file_name() == "ice-3891.rs" { continue; } @@ -90,7 +86,7 @@ pub fn run(check: bool, verbose: bool) { }, CliError::RaSetupActive => { eprintln!( - "error: a local rustc repo is enabled as path dependency via `cargo dev ide_setup`. + "error: a local rustc repo is enabled as path dependency via `cargo dev setup intellij`. Not formatting because that would format the local repo as well! Please revert the changes to Cargo.tomls first." ); diff --git a/clippy_dev/src/ide_setup.rs b/clippy_dev/src/ide_setup.rs deleted file mode 100644 index defb1133e44..00000000000 --- a/clippy_dev/src/ide_setup.rs +++ /dev/null @@ -1,103 +0,0 @@ -use std::fs; -use std::fs::File; -use std::io::prelude::*; -use std::path::{Path, PathBuf}; - -// This module takes an absolute path to a rustc repo and alters the dependencies to point towards -// the respective rustc subcrates instead of using extern crate xyz. -// This allows rust analyzer to analyze rustc internals and show proper information inside clippy -// code. See https://github.com/rust-analyzer/rust-analyzer/issues/3517 and https://github.com/rust-lang/rust-clippy/issues/5514 for details - -/// # Panics -/// -/// Panics if `rustc_path` does not lead to a rustc repo or the files could not be read -pub fn run(rustc_path: Option<&str>) { - // we can unwrap here because the arg is required by clap - let rustc_path = PathBuf::from(rustc_path.unwrap()) - .canonicalize() - .expect("failed to get the absolute repo path"); - assert!(rustc_path.is_dir(), "path is not a directory"); - let rustc_source_basedir = rustc_path.join("compiler"); - assert!( - rustc_source_basedir.is_dir(), - "are you sure the path leads to a rustc repo?" - ); - - let clippy_root_manifest = fs::read_to_string("Cargo.toml").expect("failed to read ./Cargo.toml"); - let clippy_root_lib_rs = fs::read_to_string("src/driver.rs").expect("failed to read ./src/driver.rs"); - inject_deps_into_manifest( - &rustc_source_basedir, - "Cargo.toml", - &clippy_root_manifest, - &clippy_root_lib_rs, - ) - .expect("Failed to inject deps into ./Cargo.toml"); - - let clippy_lints_manifest = - fs::read_to_string("clippy_lints/Cargo.toml").expect("failed to read ./clippy_lints/Cargo.toml"); - let clippy_lints_lib_rs = - fs::read_to_string("clippy_lints/src/lib.rs").expect("failed to read ./clippy_lints/src/lib.rs"); - inject_deps_into_manifest( - &rustc_source_basedir, - "clippy_lints/Cargo.toml", - &clippy_lints_manifest, - &clippy_lints_lib_rs, - ) - .expect("Failed to inject deps into ./clippy_lints/Cargo.toml"); -} - -fn inject_deps_into_manifest( - rustc_source_dir: &Path, - manifest_path: &str, - cargo_toml: &str, - lib_rs: &str, -) -> std::io::Result<()> { - // do not inject deps if we have aleady done so - if cargo_toml.contains("[target.'cfg(NOT_A_PLATFORM)'.dependencies]") { - eprintln!( - "cargo dev ide_setup: warning: deps already found inside {}, doing nothing.", - manifest_path - ); - return Ok(()); - } - - let extern_crates = lib_rs - .lines() - // get the deps - .filter(|line| line.starts_with("extern crate")) - // we have something like "extern crate foo;", we only care about the "foo" - // ↓ ↓ - // extern crate rustc_middle; - .map(|s| &s[13..(s.len() - 1)]); - - let new_deps = extern_crates.map(|dep| { - // format the dependencies that are going to be put inside the Cargo.toml - format!( - "{dep} = {{ path = \"{source_path}/{dep}\" }}\n", - dep = dep, - source_path = rustc_source_dir.display() - ) - }); - - // format a new [dependencies]-block with the new deps we need to inject - let mut all_deps = String::from("[target.'cfg(NOT_A_PLATFORM)'.dependencies]\n"); - new_deps.for_each(|dep_line| { - all_deps.push_str(&dep_line); - }); - all_deps.push_str("\n[dependencies]\n"); - - // replace "[dependencies]" with - // [dependencies] - // dep1 = { path = ... } - // dep2 = { path = ... } - // etc - let new_manifest = cargo_toml.replacen("[dependencies]\n", &all_deps, 1); - - // println!("{}", new_manifest); - let mut file = File::create(manifest_path)?; - file.write_all(new_manifest.as_bytes())?; - - println!("Dependency paths injected: {}", manifest_path); - - Ok(()) -} diff --git a/clippy_dev/src/lib.rs b/clippy_dev/src/lib.rs index 69f42aca8b6..72bdaf8d592 100644 --- a/clippy_dev/src/lib.rs +++ b/clippy_dev/src/lib.rs @@ -14,9 +14,9 @@ use walkdir::WalkDir; pub mod bless; pub mod fmt; -pub mod ide_setup; pub mod new_lint; pub mod serve; +pub mod setup; pub mod stderr_length_check; pub mod update_lints; diff --git a/clippy_dev/src/main.rs b/clippy_dev/src/main.rs index 7040c257c83..ff324ff6ee6 100644 --- a/clippy_dev/src/main.rs +++ b/clippy_dev/src/main.rs @@ -2,8 +2,8 @@ // warn on lints, that are included in `rust-lang/rust`s bootstrap #![warn(rust_2018_idioms, unused_lifetimes)] -use clap::{App, Arg, ArgMatches, SubCommand}; -use clippy_dev::{bless, fmt, ide_setup, new_lint, serve, stderr_length_check, update_lints}; +use clap::{App, AppSettings, Arg, ArgMatches, SubCommand}; +use clippy_dev::{bless, fmt, new_lint, serve, setup, stderr_length_check, update_lints}; fn main() { let matches = get_clap_config(); @@ -36,7 +36,22 @@ fn main() { ("limit_stderr_length", _) => { stderr_length_check::check(); }, - ("ide_setup", Some(matches)) => ide_setup::run(matches.value_of("rustc-repo-path")), + ("setup", Some(sub_command)) => match sub_command.subcommand() { + ("intellij", Some(matches)) => setup::intellij::setup_rustc_src( + matches + .value_of("rustc-repo-path") + .expect("this field is mandatory and therefore always valid"), + ), + ("git-hook", Some(matches)) => setup::git_hook::install_hook(matches.is_present("force-override")), + ("vscode-tasks", Some(matches)) => setup::vscode::install_tasks(matches.is_present("force-override")), + _ => {}, + }, + ("remove", Some(sub_command)) => match sub_command.subcommand() { + ("git-hook", Some(_)) => setup::git_hook::remove_hook(), + ("intellij", Some(_)) => setup::intellij::remove_rustc_src(), + ("vscode-tasks", Some(_)) => setup::vscode::remove_tasks(), + _ => {}, + }, ("serve", Some(matches)) => { let port = matches.value_of("port").unwrap().parse().unwrap(); let lint = matches.value_of("lint"); @@ -48,6 +63,7 @@ fn main() { fn get_clap_config<'a>() -> ArgMatches<'a> { App::new("Clippy developer tooling") + .setting(AppSettings::ArgRequiredElseHelp) .subcommand( SubCommand::with_name("bless") .about("bless the test output changes") @@ -123,6 +139,7 @@ fn get_clap_config<'a>() -> ArgMatches<'a> { .possible_values(&[ "style", "correctness", + "suspicious", "complexity", "perf", "pedantic", @@ -140,16 +157,54 @@ fn get_clap_config<'a>() -> ArgMatches<'a> { .about("Ensures that stderr files do not grow longer than a certain amount of lines."), ) .subcommand( - SubCommand::with_name("ide_setup") - .about("Alter dependencies so Intellij Rust can find rustc internals") - .arg( - Arg::with_name("rustc-repo-path") - .long("repo-path") - .short("r") - .help("The path to a rustc repo that will be used for setting the dependencies") - .takes_value(true) - .value_name("path") - .required(true), + SubCommand::with_name("setup") + .about("Support for setting up your personal development environment") + .setting(AppSettings::ArgRequiredElseHelp) + .subcommand( + SubCommand::with_name("intellij") + .about("Alter dependencies so Intellij Rust can find rustc internals") + .arg( + Arg::with_name("rustc-repo-path") + .long("repo-path") + .short("r") + .help("The path to a rustc repo that will be used for setting the dependencies") + .takes_value(true) + .value_name("path") + .required(true), + ), + ) + .subcommand( + SubCommand::with_name("git-hook") + .about("Add a pre-commit git hook that formats your code to make it look pretty") + .arg( + Arg::with_name("force-override") + .long("force-override") + .short("f") + .help("Forces the override of an existing git pre-commit hook") + .required(false), + ), + ) + .subcommand( + SubCommand::with_name("vscode-tasks") + .about("Add several tasks to vscode for formatting, validation and testing") + .arg( + Arg::with_name("force-override") + .long("force-override") + .short("f") + .help("Forces the override of existing vscode tasks") + .required(false), + ), + ), + ) + .subcommand( + SubCommand::with_name("remove") + .about("Support for undoing changes done by the setup command") + .setting(AppSettings::ArgRequiredElseHelp) + .subcommand(SubCommand::with_name("git-hook").about("Remove any existing pre-commit git hook")) + .subcommand(SubCommand::with_name("vscode-tasks").about("Remove any existing vscode tasks")) + .subcommand( + SubCommand::with_name("intellij") + .about("Removes rustc source paths added via `cargo dev setup intellij`"), ), ) .subcommand( diff --git a/clippy_dev/src/setup/git_hook.rs b/clippy_dev/src/setup/git_hook.rs new file mode 100644 index 00000000000..3fbb77d5923 --- /dev/null +++ b/clippy_dev/src/setup/git_hook.rs @@ -0,0 +1,85 @@ +use std::fs; +use std::path::Path; + +use super::verify_inside_clippy_dir; + +/// Rusts setup uses `git rev-parse --git-common-dir` to get the root directory of the repo. +/// I've decided against this for the sake of simplicity and to make sure that it doesn't install +/// the hook if `clippy_dev` would be used in the rust tree. The hook also references this tool +/// for formatting and should therefor only be used in a normal clone of clippy +const REPO_GIT_DIR: &str = ".git"; +const HOOK_SOURCE_FILE: &str = "util/etc/pre-commit.sh"; +const HOOK_TARGET_FILE: &str = ".git/hooks/pre-commit"; + +pub fn install_hook(force_override: bool) { + if !check_precondition(force_override) { + return; + } + + // So a little bit of a funny story. Git on unix requires the pre-commit file + // to have the `execute` permission to be set. The Rust functions for modifying + // these flags doesn't seem to work when executed with normal user permissions. + // + // However, there is a little hack that is also being used by Rust itself in their + // setup script. Git saves the `execute` flag when syncing files. This means + // that we can check in a file with execution permissions and the sync it to create + // a file with the flag set. We then copy this file here. The copy function will also + // include the `execute` permission. + match fs::copy(HOOK_SOURCE_FILE, HOOK_TARGET_FILE) { + Ok(_) => { + println!("info: the hook can be removed with `cargo dev remove git-hook`"); + println!("git hook successfully installed"); + }, + Err(err) => eprintln!( + "error: unable to copy `{}` to `{}` ({})", + HOOK_SOURCE_FILE, HOOK_TARGET_FILE, err + ), + } +} + +fn check_precondition(force_override: bool) -> bool { + if !verify_inside_clippy_dir() { + return false; + } + + // Make sure that we can find the git repository + let git_path = Path::new(REPO_GIT_DIR); + if !git_path.exists() || !git_path.is_dir() { + eprintln!("error: clippy_dev was unable to find the `.git` directory"); + return false; + } + + // Make sure that we don't override an existing hook by accident + let path = Path::new(HOOK_TARGET_FILE); + if path.exists() { + if force_override { + return delete_git_hook_file(path); + } + + eprintln!("error: there is already a pre-commit hook installed"); + println!("info: use the `--force-override` flag to override the existing hook"); + return false; + } + + true +} + +pub fn remove_hook() { + let path = Path::new(HOOK_TARGET_FILE); + if path.exists() { + if delete_git_hook_file(path) { + println!("git hook successfully removed"); + } + } else { + println!("no pre-commit hook was found"); + } +} + +fn delete_git_hook_file(path: &Path) -> bool { + if let Err(err) = fs::remove_file(path) { + eprintln!("error: unable to delete existing pre-commit git hook ({})", err); + false + } else { + true + } +} diff --git a/clippy_dev/src/setup/intellij.rs b/clippy_dev/src/setup/intellij.rs new file mode 100644 index 00000000000..bf741e6d121 --- /dev/null +++ b/clippy_dev/src/setup/intellij.rs @@ -0,0 +1,223 @@ +use std::fs; +use std::fs::File; +use std::io::prelude::*; +use std::path::{Path, PathBuf}; + +// This module takes an absolute path to a rustc repo and alters the dependencies to point towards +// the respective rustc subcrates instead of using extern crate xyz. +// This allows IntelliJ to analyze rustc internals and show proper information inside Clippy +// code. See https://github.com/rust-lang/rust-clippy/issues/5514 for details + +const RUSTC_PATH_SECTION: &str = "[target.'cfg(NOT_A_PLATFORM)'.dependencies]"; +const DEPENDENCIES_SECTION: &str = "[dependencies]"; + +const CLIPPY_PROJECTS: &[ClippyProjectInfo] = &[ + ClippyProjectInfo::new("root", "Cargo.toml", "src/driver.rs"), + ClippyProjectInfo::new("clippy_lints", "clippy_lints/Cargo.toml", "clippy_lints/src/lib.rs"), + ClippyProjectInfo::new("clippy_utils", "clippy_utils/Cargo.toml", "clippy_utils/src/lib.rs"), +]; + +/// Used to store clippy project information to later inject the dependency into. +struct ClippyProjectInfo { + /// Only used to display information to the user + name: &'static str, + cargo_file: &'static str, + lib_rs_file: &'static str, +} + +impl ClippyProjectInfo { + const fn new(name: &'static str, cargo_file: &'static str, lib_rs_file: &'static str) -> Self { + Self { + name, + cargo_file, + lib_rs_file, + } + } +} + +pub fn setup_rustc_src(rustc_path: &str) { + let rustc_source_dir = match check_and_get_rustc_dir(rustc_path) { + Ok(path) => path, + Err(_) => return, + }; + + for project in CLIPPY_PROJECTS { + if inject_deps_into_project(&rustc_source_dir, project).is_err() { + return; + } + } + + println!("info: the source paths can be removed again with `cargo dev remove intellij`"); +} + +fn check_and_get_rustc_dir(rustc_path: &str) -> Result<PathBuf, ()> { + let mut path = PathBuf::from(rustc_path); + + if path.is_relative() { + match path.canonicalize() { + Ok(absolute_path) => { + println!("info: the rustc path was resolved to: `{}`", absolute_path.display()); + path = absolute_path; + }, + Err(err) => { + eprintln!("error: unable to get the absolute path of rustc ({})", err); + return Err(()); + }, + }; + } + + let path = path.join("compiler"); + println!("info: looking for compiler sources at: {}", path.display()); + + if !path.exists() { + eprintln!("error: the given path does not exist"); + return Err(()); + } + + if !path.is_dir() { + eprintln!("error: the given path is not a directory"); + return Err(()); + } + + Ok(path) +} + +fn inject_deps_into_project(rustc_source_dir: &Path, project: &ClippyProjectInfo) -> Result<(), ()> { + let cargo_content = read_project_file(project.cargo_file)?; + let lib_content = read_project_file(project.lib_rs_file)?; + + if inject_deps_into_manifest(rustc_source_dir, project.cargo_file, &cargo_content, &lib_content).is_err() { + eprintln!( + "error: unable to inject dependencies into {} with the Cargo file {}", + project.name, project.cargo_file + ); + Err(()) + } else { + Ok(()) + } +} + +/// `clippy_dev` expects to be executed in the root directory of Clippy. This function +/// loads the given file or returns an error. Having it in this extra function ensures +/// that the error message looks nice. +fn read_project_file(file_path: &str) -> Result<String, ()> { + let path = Path::new(file_path); + if !path.exists() { + eprintln!("error: unable to find the file `{}`", file_path); + return Err(()); + } + + match fs::read_to_string(path) { + Ok(content) => Ok(content), + Err(err) => { + eprintln!("error: the file `{}` could not be read ({})", file_path, err); + Err(()) + }, + } +} + +fn inject_deps_into_manifest( + rustc_source_dir: &Path, + manifest_path: &str, + cargo_toml: &str, + lib_rs: &str, +) -> std::io::Result<()> { + // do not inject deps if we have already done so + if cargo_toml.contains(RUSTC_PATH_SECTION) { + eprintln!( + "warn: dependencies are already setup inside {}, skipping file", + manifest_path + ); + return Ok(()); + } + + let extern_crates = lib_rs + .lines() + // only take dependencies starting with `rustc_` + .filter(|line| line.starts_with("extern crate rustc_")) + // we have something like "extern crate foo;", we only care about the "foo" + // extern crate rustc_middle; + // ^^^^^^^^^^^^ + .map(|s| &s[13..(s.len() - 1)]); + + let new_deps = extern_crates.map(|dep| { + // format the dependencies that are going to be put inside the Cargo.toml + format!( + "{dep} = {{ path = \"{source_path}/{dep}\" }}\n", + dep = dep, + source_path = rustc_source_dir.display() + ) + }); + + // format a new [dependencies]-block with the new deps we need to inject + let mut all_deps = String::from("[target.'cfg(NOT_A_PLATFORM)'.dependencies]\n"); + new_deps.for_each(|dep_line| { + all_deps.push_str(&dep_line); + }); + all_deps.push_str("\n[dependencies]\n"); + + // replace "[dependencies]" with + // [dependencies] + // dep1 = { path = ... } + // dep2 = { path = ... } + // etc + let new_manifest = cargo_toml.replacen("[dependencies]\n", &all_deps, 1); + + // println!("{}", new_manifest); + let mut file = File::create(manifest_path)?; + file.write_all(new_manifest.as_bytes())?; + + println!("info: successfully setup dependencies inside {}", manifest_path); + + Ok(()) +} + +pub fn remove_rustc_src() { + for project in CLIPPY_PROJECTS { + remove_rustc_src_from_project(project); + } +} + +fn remove_rustc_src_from_project(project: &ClippyProjectInfo) -> bool { + let mut cargo_content = if let Ok(content) = read_project_file(project.cargo_file) { + content + } else { + return false; + }; + let section_start = if let Some(section_start) = cargo_content.find(RUSTC_PATH_SECTION) { + section_start + } else { + println!( + "info: dependencies could not be found in `{}` for {}, skipping file", + project.cargo_file, project.name + ); + return true; + }; + + let end_point = if let Some(end_point) = cargo_content.find(DEPENDENCIES_SECTION) { + end_point + } else { + eprintln!( + "error: the end of the rustc dependencies section could not be found in `{}`", + project.cargo_file + ); + return false; + }; + + cargo_content.replace_range(section_start..end_point, ""); + + match File::create(project.cargo_file) { + Ok(mut file) => { + file.write_all(cargo_content.as_bytes()).unwrap(); + println!("info: successfully removed dependencies inside {}", project.cargo_file); + true + }, + Err(err) => { + eprintln!( + "error: unable to open file `{}` to remove rustc dependencies for {} ({})", + project.cargo_file, project.name, err + ); + false + }, + } +} diff --git a/clippy_dev/src/setup/mod.rs b/clippy_dev/src/setup/mod.rs new file mode 100644 index 00000000000..a1e4dd103b8 --- /dev/null +++ b/clippy_dev/src/setup/mod.rs @@ -0,0 +1,23 @@ +pub mod git_hook; +pub mod intellij; +pub mod vscode; + +use std::path::Path; + +const CLIPPY_DEV_DIR: &str = "clippy_dev"; + +/// This function verifies that the tool is being executed in the clippy directory. +/// This is useful to ensure that setups only modify Clippys resources. The verification +/// is done by checking that `clippy_dev` is a sub directory of the current directory. +/// +/// It will print an error message and return `false` if the directory could not be +/// verified. +fn verify_inside_clippy_dir() -> bool { + let path = Path::new(CLIPPY_DEV_DIR); + if path.exists() && path.is_dir() { + true + } else { + eprintln!("error: unable to verify that the working directory is clippys directory"); + false + } +} diff --git a/clippy_dev/src/setup/vscode.rs b/clippy_dev/src/setup/vscode.rs new file mode 100644 index 00000000000..d59001b2c66 --- /dev/null +++ b/clippy_dev/src/setup/vscode.rs @@ -0,0 +1,104 @@ +use std::fs; +use std::path::Path; + +use super::verify_inside_clippy_dir; + +const VSCODE_DIR: &str = ".vscode"; +const TASK_SOURCE_FILE: &str = "util/etc/vscode-tasks.json"; +const TASK_TARGET_FILE: &str = ".vscode/tasks.json"; + +pub fn install_tasks(force_override: bool) { + if !check_install_precondition(force_override) { + return; + } + + match fs::copy(TASK_SOURCE_FILE, TASK_TARGET_FILE) { + Ok(_) => { + println!("info: the task file can be removed with `cargo dev remove vscode-tasks`"); + println!("vscode tasks successfully installed"); + }, + Err(err) => eprintln!( + "error: unable to copy `{}` to `{}` ({})", + TASK_SOURCE_FILE, TASK_TARGET_FILE, err + ), + } +} + +fn check_install_precondition(force_override: bool) -> bool { + if !verify_inside_clippy_dir() { + return false; + } + + let vs_dir_path = Path::new(VSCODE_DIR); + if vs_dir_path.exists() { + // verify the target will be valid + if !vs_dir_path.is_dir() { + eprintln!("error: the `.vscode` path exists but seems to be a file"); + return false; + } + + // make sure that we don't override any existing tasks by accident + let path = Path::new(TASK_TARGET_FILE); + if path.exists() { + if force_override { + return delete_vs_task_file(path); + } + + eprintln!( + "error: there is already a `task.json` file inside the `{}` directory", + VSCODE_DIR + ); + println!("info: use the `--force-override` flag to override the existing `task.json` file"); + return false; + } + } else { + match fs::create_dir(vs_dir_path) { + Ok(_) => { + println!("info: created `{}` directory for clippy", VSCODE_DIR); + }, + Err(err) => { + eprintln!( + "error: the task target directory `{}` could not be created ({})", + VSCODE_DIR, err + ); + }, + } + } + + true +} + +pub fn remove_tasks() { + let path = Path::new(TASK_TARGET_FILE); + if path.exists() { + if delete_vs_task_file(path) { + try_delete_vs_directory_if_empty(); + println!("vscode tasks successfully removed"); + } + } else { + println!("no vscode tasks were found"); + } +} + +fn delete_vs_task_file(path: &Path) -> bool { + if let Err(err) = fs::remove_file(path) { + eprintln!("error: unable to delete the existing `tasks.json` file ({})", err); + return false; + } + + true +} + +/// This function will try to delete the `.vscode` directory if it's empty. +/// It may fail silently. +fn try_delete_vs_directory_if_empty() { + let path = Path::new(VSCODE_DIR); + if path.read_dir().map_or(false, |mut iter| iter.next().is_none()) { + // The directory is empty. We just try to delete it but allow a silence + // fail as an empty `.vscode` directory is still valid + let _silence_result = fs::remove_dir(path); + } else { + // The directory is not empty or could not be read. Either way don't take + // any further actions + } +} diff --git a/clippy_dev/src/update_lints.rs b/clippy_dev/src/update_lints.rs index edf6c5f57a4..db467c26f15 100644 --- a/clippy_dev/src/update_lints.rs +++ b/clippy_dev/src/update_lints.rs @@ -92,7 +92,10 @@ pub fn run(update_mode: UpdateMode) { || { // clippy::all should only include the following lint groups: let all_group_lints = usable_lints.iter().filter(|l| { - l.group == "correctness" || l.group == "style" || l.group == "complexity" || l.group == "perf" + matches!( + &*l.group, + "correctness" | "suspicious" | "style" | "complexity" | "perf" + ) }); gen_lint_group_list(all_group_lints) diff --git a/clippy_lints/Cargo.toml b/clippy_lints/Cargo.toml index 48f2972ec58..42cf7547f51 100644 --- a/clippy_lints/Cargo.toml +++ b/clippy_lints/Cargo.toml @@ -1,7 +1,7 @@ [package] name = "clippy_lints" # begin automatic update -version = "0.1.54" +version = "0.1.55" # end automatic update authors = ["The Rust Clippy Developers"] description = "A bunch of helpful lints to avoid common pitfalls in Rust" @@ -23,6 +23,7 @@ serde = { version = "1.0", features = ["derive"] } serde_json = { version = "1.0", optional = true } toml = "0.5.3" unicode-normalization = "0.1" +unicode-script = { version = "0.5.3", default-features = false } semver = "0.11" rustc-semver = "1.1.0" # NOTE: cargo requires serde feat in its url dep diff --git a/clippy_lints/src/assign_ops.rs b/clippy_lints/src/assign_ops.rs index bc6eec0051a..a8c527fe2e3 100644 --- a/clippy_lints/src/assign_ops.rs +++ b/clippy_lints/src/assign_ops.rs @@ -55,7 +55,7 @@ declare_clippy_lint! { /// a += a + b; /// ``` pub MISREFACTORED_ASSIGN_OP, - complexity, + suspicious, "having a variable on both sides of an assign op" } diff --git a/clippy_lints/src/attrs.rs b/clippy_lints/src/attrs.rs index 932cd58bf62..f272ed010a1 100644 --- a/clippy_lints/src/attrs.rs +++ b/clippy_lints/src/attrs.rs @@ -173,7 +173,7 @@ declare_clippy_lint! { /// #![deny(clippy::as_conversions)] /// ``` pub BLANKET_CLIPPY_RESTRICTION_LINTS, - style, + suspicious, "enabling the complete restriction group" } diff --git a/clippy_lints/src/blacklisted_name.rs b/clippy_lints/src/blacklisted_name.rs index b26ef33e056..8eb94f3c28e 100644 --- a/clippy_lints/src/blacklisted_name.rs +++ b/clippy_lints/src/blacklisted_name.rs @@ -1,6 +1,6 @@ -use clippy_utils::diagnostics::span_lint; +use clippy_utils::{diagnostics::span_lint, is_test_module_or_function}; use rustc_data_structures::fx::FxHashSet; -use rustc_hir::{Pat, PatKind}; +use rustc_hir::{Item, Pat, PatKind}; use rustc_lint::{LateContext, LateLintPass}; use rustc_session::{declare_tool_lint, impl_lint_pass}; @@ -25,18 +25,37 @@ declare_clippy_lint! { #[derive(Clone, Debug)] pub struct BlacklistedName { blacklist: FxHashSet<String>, + test_modules_deep: u32, } impl BlacklistedName { pub fn new(blacklist: FxHashSet<String>) -> Self { - Self { blacklist } + Self { + blacklist, + test_modules_deep: 0, + } + } + + fn in_test_module(&self) -> bool { + self.test_modules_deep != 0 } } impl_lint_pass!(BlacklistedName => [BLACKLISTED_NAME]); impl<'tcx> LateLintPass<'tcx> for BlacklistedName { + fn check_item(&mut self, cx: &LateContext<'_>, item: &Item<'_>) { + if is_test_module_or_function(cx.tcx, item) { + self.test_modules_deep = self.test_modules_deep.saturating_add(1); + } + } + fn check_pat(&mut self, cx: &LateContext<'tcx>, pat: &'tcx Pat<'_>) { + // Check whether we are under the `test` attribute. + if self.in_test_module() { + return; + } + if let PatKind::Binding(.., ident, _) = pat.kind { if self.blacklist.contains(&ident.name.to_string()) { span_lint( @@ -48,4 +67,10 @@ impl<'tcx> LateLintPass<'tcx> for BlacklistedName { } } } + + fn check_item_post(&mut self, cx: &LateContext<'_>, item: &Item<'_>) { + if is_test_module_or_function(cx.tcx, item) { + self.test_modules_deep = self.test_modules_deep.saturating_sub(1); + } + } } diff --git a/clippy_lints/src/bytecount.rs b/clippy_lints/src/bytecount.rs index 877ae002d36..4f7ffdcdfb4 100644 --- a/clippy_lints/src/bytecount.rs +++ b/clippy_lints/src/bytecount.rs @@ -1,15 +1,15 @@ use clippy_utils::diagnostics::span_lint_and_sugg; use clippy_utils::source::snippet_with_applicability; use clippy_utils::ty::match_type; -use clippy_utils::{contains_name, get_pat_name, paths, single_segment_path}; +use clippy_utils::visitors::LocalUsedVisitor; +use clippy_utils::{path_to_local_id, paths, peel_ref_operators, remove_blocks, strip_pat_refs}; use if_chain::if_chain; use rustc_errors::Applicability; -use rustc_hir::{BinOpKind, BorrowKind, Expr, ExprKind, UnOp}; +use rustc_hir::{BinOpKind, Expr, ExprKind, PatKind}; use rustc_lint::{LateContext, LateLintPass}; use rustc_middle::ty::{self, UintTy}; use rustc_session::{declare_lint_pass, declare_tool_lint}; use rustc_span::sym; -use rustc_span::Symbol; declare_clippy_lint! { /// **What it does:** Checks for naive byte counts @@ -38,42 +38,43 @@ declare_lint_pass!(ByteCount => [NAIVE_BYTECOUNT]); impl<'tcx> LateLintPass<'tcx> for ByteCount { fn check_expr(&mut self, cx: &LateContext<'_>, expr: &Expr<'_>) { if_chain! { - if let ExprKind::MethodCall(count, _, count_args, _) = expr.kind; + if let ExprKind::MethodCall(count, _, [count_recv], _) = expr.kind; if count.ident.name == sym!(count); - if count_args.len() == 1; - if let ExprKind::MethodCall(filter, _, filter_args, _) = count_args[0].kind; + if let ExprKind::MethodCall(filter, _, [filter_recv, filter_arg], _) = count_recv.kind; if filter.ident.name == sym!(filter); - if filter_args.len() == 2; - if let ExprKind::Closure(_, _, body_id, _, _) = filter_args[1].kind; + if let ExprKind::Closure(_, _, body_id, _, _) = filter_arg.kind; let body = cx.tcx.hir().body(body_id); - if body.params.len() == 1; - if let Some(argname) = get_pat_name(body.params[0].pat); + if let [param] = body.params; + if let PatKind::Binding(_, arg_id, _, _) = strip_pat_refs(param.pat).kind; if let ExprKind::Binary(ref op, l, r) = body.value.kind; if op.node == BinOpKind::Eq; if match_type(cx, - cx.typeck_results().expr_ty(&filter_args[0]).peel_refs(), + cx.typeck_results().expr_ty(filter_recv).peel_refs(), &paths::SLICE_ITER); + let operand_is_arg = |expr| { + let expr = peel_ref_operators(cx, remove_blocks(expr)); + path_to_local_id(expr, arg_id) + }; + let needle = if operand_is_arg(l) { + r + } else if operand_is_arg(r) { + l + } else { + return; + }; + if ty::Uint(UintTy::U8) == *cx.typeck_results().expr_ty(needle).peel_refs().kind(); + if !LocalUsedVisitor::new(cx, arg_id).check_expr(needle); then { - let needle = match get_path_name(l) { - Some(name) if check_arg(name, argname, r) => r, - _ => match get_path_name(r) { - Some(name) if check_arg(name, argname, l) => l, - _ => { return; } - } - }; - if ty::Uint(UintTy::U8) != *cx.typeck_results().expr_ty(needle).peel_refs().kind() { - return; - } let haystack = if let ExprKind::MethodCall(path, _, args, _) = - filter_args[0].kind { + filter_recv.kind { let p = path.ident.name; if (p == sym::iter || p == sym!(iter_mut)) && args.len() == 1 { &args[0] } else { - &filter_args[0] + &filter_recv } } else { - &filter_args[0] + &filter_recv }; let mut applicability = Applicability::MaybeIncorrect; span_lint_and_sugg( @@ -91,24 +92,3 @@ impl<'tcx> LateLintPass<'tcx> for ByteCount { }; } } - -fn check_arg(name: Symbol, arg: Symbol, needle: &Expr<'_>) -> bool { - name == arg && !contains_name(name, needle) -} - -fn get_path_name(expr: &Expr<'_>) -> Option<Symbol> { - match expr.kind { - ExprKind::Box(e) | ExprKind::AddrOf(BorrowKind::Ref, _, e) | ExprKind::Unary(UnOp::Deref, e) => { - get_path_name(e) - }, - ExprKind::Block(b, _) => { - if b.stmts.is_empty() { - b.expr.as_ref().and_then(|p| get_path_name(p)) - } else { - None - } - }, - ExprKind::Path(ref qpath) => single_segment_path(qpath).map(|ps| ps.ident.name), - _ => None, - } -} diff --git a/clippy_lints/src/collapsible_match.rs b/clippy_lints/src/collapsible_match.rs index ab22578abd6..a6c3a5b0e83 100644 --- a/clippy_lints/src/collapsible_match.rs +++ b/clippy_lints/src/collapsible_match.rs @@ -1,11 +1,10 @@ use clippy_utils::diagnostics::span_lint_and_then; use clippy_utils::visitors::LocalUsedVisitor; -use clippy_utils::{is_lang_ctor, path_to_local, SpanlessEq}; +use clippy_utils::{is_lang_ctor, path_to_local, peel_ref_operators, SpanlessEq}; use if_chain::if_chain; use rustc_hir::LangItem::OptionNone; -use rustc_hir::{Arm, Expr, ExprKind, Guard, HirId, Pat, PatKind, StmtKind, UnOp}; +use rustc_hir::{Arm, Expr, ExprKind, Guard, HirId, Pat, PatKind, StmtKind}; use rustc_lint::{LateContext, LateLintPass}; -use rustc_middle::ty::TypeckResults; use rustc_session::{declare_lint_pass, declare_tool_lint}; use rustc_span::{MultiSpan, Span}; @@ -73,7 +72,7 @@ fn check_arm<'tcx>(arm: &Arm<'tcx>, wild_outer_arm: &Arm<'tcx>, cx: &LateContext if arms_inner.iter().all(|arm| arm.guard.is_none()); // match expression must be a local binding // match <local> { .. } - if let Some(binding_id) = path_to_local(strip_ref_operators(expr_in, cx.typeck_results())); + if let Some(binding_id) = path_to_local(peel_ref_operators(cx, expr_in)); // one of the branches must be "wild-like" if let Some(wild_inner_arm_idx) = arms_inner.iter().rposition(|arm_inner| arm_is_wild_like(cx, arm_inner)); let (wild_inner_arm, non_wild_inner_arm) = @@ -163,16 +162,3 @@ fn pat_contains_or(pat: &Pat<'_>) -> bool { }); result } - -/// Removes `AddrOf` operators (`&`) or deref operators (`*`), but only if a reference type is -/// dereferenced. An overloaded deref such as `Vec` to slice would not be removed. -fn strip_ref_operators<'hir>(mut expr: &'hir Expr<'hir>, typeck_results: &TypeckResults<'_>) -> &'hir Expr<'hir> { - loop { - match expr.kind { - ExprKind::AddrOf(_, _, e) => expr = e, - ExprKind::Unary(UnOp::Deref, e) if typeck_results.expr_ty(e).is_ref() => expr = e, - _ => break, - } - } - expr -} diff --git a/clippy_lints/src/default.rs b/clippy_lints/src/default.rs index 7a53d390bb4..947479db8f5 100644 --- a/clippy_lints/src/default.rs +++ b/clippy_lints/src/default.rs @@ -7,7 +7,6 @@ use rustc_errors::Applicability; use rustc_hir::def::Res; use rustc_hir::{Block, Expr, ExprKind, PatKind, QPath, Stmt, StmtKind}; use rustc_lint::{LateContext, LateLintPass}; -use rustc_middle::lint::in_external_macro; use rustc_middle::ty; use rustc_session::{declare_tool_lint, impl_lint_pass}; use rustc_span::symbol::{Ident, Symbol}; @@ -122,7 +121,7 @@ impl LateLintPass<'_> for Default { if let StmtKind::Local(local) = stmt.kind; if let Some(expr) = local.init; if !any_parent_is_automatically_derived(cx.tcx, expr.hir_id); - if !in_external_macro(cx.tcx.sess, expr.span); + if !in_macro(expr.span); // only take bindings to identifiers if let PatKind::Binding(_, binding_id, ident, _) = local.pat.kind; // only when assigning `... = Default::default()` diff --git a/clippy_lints/src/default_numeric_fallback.rs b/clippy_lints/src/default_numeric_fallback.rs index 759f7d4062d..a125376bffa 100644 --- a/clippy_lints/src/default_numeric_fallback.rs +++ b/clippy_lints/src/default_numeric_fallback.rs @@ -7,9 +7,10 @@ use rustc_hir::{ intravisit::{walk_expr, walk_stmt, NestedVisitorMap, Visitor}, Body, Expr, ExprKind, HirId, Lit, Stmt, StmtKind, }; -use rustc_lint::{LateContext, LateLintPass}; +use rustc_lint::{LateContext, LateLintPass, LintContext}; use rustc_middle::{ hir::map::Map, + lint::in_external_macro, ty::{self, FloatTy, IntTy, PolyFnSig, Ty}, }; use rustc_session::{declare_lint_pass, declare_tool_lint}; @@ -73,6 +74,7 @@ impl<'a, 'tcx> NumericFallbackVisitor<'a, 'tcx> { /// Check whether a passed literal has potential to cause fallback or not. fn check_lit(&self, lit: &Lit, lit_ty: Ty<'tcx>) { if_chain! { + if !in_external_macro(self.cx.sess(), lit.span); if let Some(ty_bound) = self.ty_bounds.last(); if matches!(lit.node, LitKind::Int(_, LitIntType::Unsuffixed) | LitKind::Float(_, LitFloatType::Unsuffixed)); diff --git a/clippy_lints/src/deprecated_lints.rs b/clippy_lints/src/deprecated_lints.rs index 04f3d77464f..2933fbc9341 100644 --- a/clippy_lints/src/deprecated_lints.rs +++ b/clippy_lints/src/deprecated_lints.rs @@ -149,7 +149,7 @@ declare_deprecated_lint! { /// enables the `enum_variant_names` lint for public items. /// ``` pub PUB_ENUM_VARIANT_NAMES, - "set the `avoid_breaking_exported_api` config option to `false` to enable the `enum_variant_names` lint for public items" + "set the `avoid-breaking-exported-api` config option to `false` to enable the `enum_variant_names` lint for public items" } declare_deprecated_lint! { @@ -158,5 +158,5 @@ declare_deprecated_lint! { /// **Deprecation reason:** The `avoid_breaking_exported_api` config option was added, which /// enables the `wrong_self_conversion` lint for public items. pub WRONG_PUB_SELF_CONVENTION, - "set the `avoid_breaking_exported_api` config option to `false` to enable the `wrong_self_convention` lint for public items" + "set the `avoid-breaking-exported-api` config option to `false` to enable the `wrong_self_convention` lint for public items" } diff --git a/clippy_lints/src/derive.rs b/clippy_lints/src/derive.rs index 729941f345a..3ac20fd9849 100644 --- a/clippy_lints/src/derive.rs +++ b/clippy_lints/src/derive.rs @@ -410,11 +410,8 @@ impl<'tcx> Visitor<'tcx> for UnsafeVisitor<'_, 'tcx> { } if let ExprKind::Block(block, _) = expr.kind { - match block.rules { - BlockCheckMode::UnsafeBlock(UnsafeSource::UserProvided) => { - self.has_unsafe = true; - }, - _ => {}, + if let BlockCheckMode::UnsafeBlock(UnsafeSource::UserProvided) = block.rules { + self.has_unsafe = true; } } diff --git a/clippy_lints/src/disallowed_method.rs b/clippy_lints/src/disallowed_method.rs index ded0a0fff54..aa1a609afed 100644 --- a/clippy_lints/src/disallowed_method.rs +++ b/clippy_lints/src/disallowed_method.rs @@ -2,7 +2,7 @@ use clippy_utils::diagnostics::span_lint; use clippy_utils::fn_def_id; use rustc_data_structures::fx::FxHashSet; -use rustc_hir::Expr; +use rustc_hir::{def::Res, def_id::DefId, Crate, Expr}; use rustc_lint::{LateContext, LateLintPass}; use rustc_session::{declare_tool_lint, impl_lint_pass}; use rustc_span::Symbol; @@ -13,21 +13,14 @@ declare_clippy_lint! { /// **Why is this bad?** Some methods are undesirable in certain contexts, /// and it's beneficial to lint for them as needed. /// - /// **Known problems:** Currently, you must write each function as a - /// fully-qualified path. This lint doesn't support aliases or reexported - /// names; be aware that many types in `std` are actually reexports. - /// - /// For example, if you want to disallow `Duration::as_secs`, your clippy.toml - /// configuration would look like - /// `disallowed-methods = ["core::time::Duration::as_secs"]` and not - /// `disallowed-methods = ["std::time::Duration::as_secs"]` as you might expect. + /// **Known problems:** None. /// /// **Example:** /// /// An example clippy.toml configuration: /// ```toml /// # clippy.toml - /// disallowed-methods = ["alloc::vec::Vec::leak", "std::time::Instant::now"] + /// disallowed-methods = ["std::vec::Vec::leak", "std::time::Instant::now"] /// ``` /// /// ```rust,ignore @@ -52,6 +45,7 @@ declare_clippy_lint! { #[derive(Clone, Debug)] pub struct DisallowedMethod { disallowed: FxHashSet<Vec<Symbol>>, + def_ids: FxHashSet<(DefId, Vec<Symbol>)>, } impl DisallowedMethod { @@ -61,6 +55,7 @@ impl DisallowedMethod { .iter() .map(|s| s.split("::").map(|seg| Symbol::intern(seg)).collect::<Vec<_>>()) .collect(), + def_ids: FxHashSet::default(), } } } @@ -68,10 +63,20 @@ impl DisallowedMethod { impl_lint_pass!(DisallowedMethod => [DISALLOWED_METHOD]); impl<'tcx> LateLintPass<'tcx> for DisallowedMethod { + fn check_crate(&mut self, cx: &LateContext<'_>, _: &Crate<'_>) { + for path in &self.disallowed { + let segs = path.iter().map(ToString::to_string).collect::<Vec<_>>(); + if let Res::Def(_, id) = clippy_utils::path_to_res(cx, &segs.iter().map(String::as_str).collect::<Vec<_>>()) + { + self.def_ids.insert((id, path.clone())); + } + } + } + fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) { if let Some(def_id) = fn_def_id(cx, expr) { - let func_path = cx.get_def_path(def_id); - if self.disallowed.contains(&func_path) { + if self.def_ids.iter().any(|(id, _)| def_id == *id) { + let func_path = cx.get_def_path(def_id); let func_path_string = func_path .into_iter() .map(Symbol::to_ident_string) diff --git a/clippy_lints/src/disallowed_script_idents.rs b/clippy_lints/src/disallowed_script_idents.rs new file mode 100644 index 00000000000..12c525634c5 --- /dev/null +++ b/clippy_lints/src/disallowed_script_idents.rs @@ -0,0 +1,112 @@ +use clippy_utils::diagnostics::span_lint; +use rustc_ast::ast; +use rustc_data_structures::fx::FxHashSet; +use rustc_lint::{EarlyContext, EarlyLintPass, Level}; +use rustc_session::{declare_tool_lint, impl_lint_pass}; +use unicode_script::{Script, UnicodeScript}; + +declare_clippy_lint! { + /// **What it does:** Checks for usage of unicode scripts other than those explicitly allowed + /// by the lint config. + /// + /// This lint doesn't take into account non-text scripts such as `Unknown` and `Linear_A`. + /// It also ignores the `Common` script type. + /// While configuring, be sure to use official script name [aliases] from + /// [the list of supported scripts][supported_scripts]. + /// + /// See also: [`non_ascii_idents`]. + /// + /// [aliases]: http://www.unicode.org/reports/tr24/tr24-31.html#Script_Value_Aliases + /// [supported_scripts]: https://www.unicode.org/iso15924/iso15924-codes.html + /// + /// **Why is this bad?** It may be not desired to have many different scripts for + /// identifiers in the codebase. + /// + /// Note that if you only want to allow plain English, you might want to use + /// built-in [`non_ascii_idents`] lint instead. + /// + /// [`non_ascii_idents`]: https://doc.rust-lang.org/rustc/lints/listing/allowed-by-default.html#non-ascii-idents + /// + /// **Known problems:** None. + /// + /// **Example:** + /// ```rust + /// // Assuming that `clippy.toml` contains the following line: + /// // allowed-locales = ["Latin", "Cyrillic"] + /// let counter = 10; // OK, latin is allowed. + /// let счётчик = 10; // OK, cyrillic is allowed. + /// let zähler = 10; // OK, it's still latin. + /// let カウンタ = 10; // Will spawn the lint. + /// ``` + pub DISALLOWED_SCRIPT_IDENTS, + restriction, + "usage of non-allowed Unicode scripts" +} + +#[derive(Clone, Debug)] +pub struct DisallowedScriptIdents { + whitelist: FxHashSet<Script>, +} + +impl DisallowedScriptIdents { + pub fn new(whitelist: &[String]) -> Self { + let whitelist = whitelist + .iter() + .map(String::as_str) + .filter_map(Script::from_full_name) + .collect(); + Self { whitelist } + } +} + +impl_lint_pass!(DisallowedScriptIdents => [DISALLOWED_SCRIPT_IDENTS]); + +impl EarlyLintPass for DisallowedScriptIdents { + fn check_crate(&mut self, cx: &EarlyContext<'_>, _: &ast::Crate) { + // Implementation is heavily inspired by the implementation of [`non_ascii_idents`] lint: + // https://github.com/rust-lang/rust/blob/master/compiler/rustc_lint/src/non_ascii_idents.rs + + let check_disallowed_script_idents = cx.builder.lint_level(DISALLOWED_SCRIPT_IDENTS).0 != Level::Allow; + if !check_disallowed_script_idents { + return; + } + + let symbols = cx.sess.parse_sess.symbol_gallery.symbols.lock(); + // Sort by `Span` so that error messages make sense with respect to the + // order of identifier locations in the code. + let mut symbols: Vec<_> = symbols.iter().collect(); + symbols.sort_unstable_by_key(|k| k.1); + + for (symbol, &span) in &symbols { + // Note: `symbol.as_str()` is an expensive operation, thus should not be called + // more than once for a single symbol. + let symbol_str = symbol.as_str(); + if symbol_str.is_ascii() { + continue; + } + + for c in symbol_str.chars() { + // We want to iterate through all the scripts associated with this character + // and check whether at least of one scripts is in the whitelist. + let forbidden_script = c + .script_extension() + .iter() + .find(|script| !self.whitelist.contains(script)); + if let Some(script) = forbidden_script { + span_lint( + cx, + DISALLOWED_SCRIPT_IDENTS, + span, + &format!( + "identifier `{}` has a Unicode script that is not allowed by configuration: {}", + symbol_str, + script.full_name() + ), + ); + // We don't want to spawn warning multiple times over a single identifier. + break; + } + } + } + } +} diff --git a/clippy_lints/src/disallowed_type.rs b/clippy_lints/src/disallowed_type.rs new file mode 100644 index 00000000000..e4a88c6324e --- /dev/null +++ b/clippy_lints/src/disallowed_type.rs @@ -0,0 +1,126 @@ +use clippy_utils::diagnostics::span_lint; + +use rustc_data_structures::fx::FxHashSet; +use rustc_hir::{ + def::Res, def_id::DefId, Crate, Item, ItemKind, PolyTraitRef, TraitBoundModifier, Ty, TyKind, UseKind, +}; +use rustc_lint::{LateContext, LateLintPass}; +use rustc_session::{declare_tool_lint, impl_lint_pass}; +use rustc_span::{Span, Symbol}; + +declare_clippy_lint! { + /// **What it does:** Denies the configured types in clippy.toml. + /// + /// **Why is this bad?** Some types are undesirable in certain contexts. + /// + /// **Known problems:** None. + /// + /// N.B. There is no way to ban primitive types. + /// + /// **Example:** + /// + /// An example clippy.toml configuration: + /// ```toml + /// # clippy.toml + /// disallowed-methods = ["std::collections::BTreeMap"] + /// ``` + /// + /// ```rust,ignore + /// use std::collections::BTreeMap; + /// // or its use + /// let x = std::collections::BTreeMap::new(); + /// ``` + /// Use instead: + /// ```rust,ignore + /// // A similar type that is allowed by the config + /// use std::collections::HashMap; + /// ``` + pub DISALLOWED_TYPE, + nursery, + "use of a disallowed type" +} +#[derive(Clone, Debug)] +pub struct DisallowedType { + disallowed: FxHashSet<Vec<Symbol>>, + def_ids: FxHashSet<(DefId, Vec<Symbol>)>, +} + +impl DisallowedType { + pub fn new(disallowed: &FxHashSet<String>) -> Self { + Self { + disallowed: disallowed + .iter() + .map(|s| s.split("::").map(|seg| Symbol::intern(seg)).collect::<Vec<_>>()) + .collect(), + def_ids: FxHashSet::default(), + } + } +} + +impl_lint_pass!(DisallowedType => [DISALLOWED_TYPE]); + +impl<'tcx> LateLintPass<'tcx> for DisallowedType { + fn check_crate(&mut self, cx: &LateContext<'_>, _: &Crate<'_>) { + for path in &self.disallowed { + let segs = path.iter().map(ToString::to_string).collect::<Vec<_>>(); + if let Res::Def(_, id) = clippy_utils::path_to_res(cx, &segs.iter().map(String::as_str).collect::<Vec<_>>()) + { + self.def_ids.insert((id, path.clone())); + } + } + } + + fn check_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx Item<'tcx>) { + if_chain! { + if let ItemKind::Use(path, UseKind::Single) = &item.kind; + if let Res::Def(_, did) = path.res; + if let Some((_, name)) = self.def_ids.iter().find(|(id, _)| *id == did); + then { + emit(cx, name, item.span,); + } + } + } + + fn check_ty(&mut self, cx: &LateContext<'tcx>, ty: &'tcx Ty<'tcx>) { + if_chain! { + if let TyKind::Path(path) = &ty.kind; + if let Some(did) = cx.qpath_res(path, ty.hir_id).opt_def_id(); + if let Some((_, name)) = self.def_ids.iter().find(|(id, _)| *id == did); + then { + emit(cx, name, path.span()); + } + } + } + + fn check_poly_trait_ref(&mut self, cx: &LateContext<'tcx>, poly: &'tcx PolyTraitRef<'tcx>, _: TraitBoundModifier) { + if_chain! { + if let Res::Def(_, did) = poly.trait_ref.path.res; + if let Some((_, name)) = self.def_ids.iter().find(|(id, _)| *id == did); + then { + emit(cx, name, poly.trait_ref.path.span); + } + } + } + + // TODO: if non primitive const generics are a thing + // fn check_generic_arg(&mut self, cx: &LateContext<'tcx>, arg: &'tcx GenericArg<'tcx>) { + // match arg { + // GenericArg::Const(c) => {}, + // } + // } + // fn check_generic_param(&mut self, cx: &LateContext<'tcx>, param: &'tcx GenericParam<'tcx>) { + // match param.kind { + // GenericParamKind::Const { .. } => {}, + // } + // } +} + +fn emit(cx: &LateContext<'_>, name: &[Symbol], span: Span) { + let name = name.iter().map(|s| s.to_ident_string()).collect::<Vec<_>>().join("::"); + span_lint( + cx, + DISALLOWED_TYPE, + span, + &format!("`{}` is not allowed according to config", name), + ); +} diff --git a/clippy_lints/src/doc.rs b/clippy_lints/src/doc.rs index e67ec4e06c5..4e164d33a05 100644 --- a/clippy_lints/src/doc.rs +++ b/clippy_lints/src/doc.rs @@ -1,4 +1,5 @@ -use clippy_utils::diagnostics::{span_lint, span_lint_and_note}; +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}; use clippy_utils::{is_entrypoint_fn, is_expn_of, match_panic_def_id, method_chain_args, return_ty}; use if_chain::if_chain; @@ -37,7 +38,8 @@ declare_clippy_lint! { /// consider that. /// /// **Known problems:** Lots of bad docs won’t be fixed, what the lint checks - /// for is limited, and there are still false positives. + /// for is limited, and there are still false positives. HTML elements and their + /// content are not linted. /// /// In addition, when writing documentation comments, including `[]` brackets /// inside a link text would trip the parser. Therfore, documenting link with @@ -469,11 +471,11 @@ fn check_doc<'a, Events: Iterator<Item = (pulldown_cmark::Event<'a>, Range<usize spans: &[(usize, Span)], ) -> DocHeaders { // true if a safety header was found - use pulldown_cmark::CodeBlockKind; use pulldown_cmark::Event::{ Code, End, FootnoteReference, HardBreak, Html, Rule, SoftBreak, Start, TaskListMarker, Text, }; - use pulldown_cmark::Tag::{CodeBlock, Heading, Link}; + use pulldown_cmark::Tag::{CodeBlock, Heading, Item, Link, Paragraph}; + use pulldown_cmark::{CodeBlockKind, CowStr}; let mut headers = DocHeaders { safety: false, @@ -485,6 +487,9 @@ fn check_doc<'a, Events: Iterator<Item = (pulldown_cmark::Event<'a>, Range<usize let mut in_heading = false; let mut is_rust = false; let mut edition = None; + let mut ticks_unbalanced = false; + let mut text_to_check: Vec<(CowStr<'_>, Span)> = Vec::new(); + let mut paragraph_span = spans.get(0).expect("function isn't called if doc comment is empty").1; for (event, range) in events { match event { Start(CodeBlock(ref kind)) => { @@ -510,13 +515,42 @@ fn check_doc<'a, Events: Iterator<Item = (pulldown_cmark::Event<'a>, Range<usize }, Start(Link(_, url, _)) => in_link = Some(url), End(Link(..)) => in_link = None, - Start(Heading(_)) => in_heading = true, - End(Heading(_)) => in_heading = false, + Start(Heading(_) | Paragraph | Item) => { + if let Start(Heading(_)) = event { + in_heading = true; + } + ticks_unbalanced = false; + let (_, span) = get_current_span(spans, range.start); + paragraph_span = first_line_of_span(cx, span); + }, + End(Heading(_) | Paragraph | Item) => { + if let End(Heading(_)) = event { + in_heading = false; + } + if ticks_unbalanced { + span_lint_and_help( + cx, + DOC_MARKDOWN, + paragraph_span, + "backticks are unbalanced", + None, + "a backtick may be missing a pair", + ); + } else { + for (text, span) in text_to_check { + check_text(cx, valid_idents, &text, span); + } + } + text_to_check = Vec::new(); + }, Start(_tag) | End(_tag) => (), // We don't care about other tags Html(_html) => (), // HTML is weird, just ignore it SoftBreak | HardBreak | TaskListMarker(_) | Code(_) | Rule => (), FootnoteReference(text) | Text(text) => { - if Some(&text) == in_link.as_ref() { + let (begin, span) = get_current_span(spans, range.start); + paragraph_span = paragraph_span.with_hi(span.hi()); + ticks_unbalanced |= text.contains('`'); + if Some(&text) == in_link.as_ref() || ticks_unbalanced { // Probably a link of the form `<http://example.com>` // Which are represented as a link to "http://example.com" with // text "http://example.com" by pulldown-cmark @@ -525,11 +559,6 @@ fn check_doc<'a, Events: Iterator<Item = (pulldown_cmark::Event<'a>, Range<usize headers.safety |= in_heading && text.trim() == "Safety"; headers.errors |= in_heading && text.trim() == "Errors"; headers.panics |= in_heading && text.trim() == "Panics"; - let index = match spans.binary_search_by(|c| c.0.cmp(&range.start)) { - Ok(o) => o, - Err(e) => e - 1, - }; - let (begin, span) = spans[index]; if in_code { if is_rust { let edition = edition.unwrap_or_else(|| cx.tcx.sess.edition()); @@ -538,8 +567,7 @@ fn check_doc<'a, Events: Iterator<Item = (pulldown_cmark::Event<'a>, Range<usize } else { // Adjust for the beginning of the current `Event` let span = span.with_lo(span.lo() + BytePos::from_usize(range.start - begin)); - - check_text(cx, valid_idents, &text, span); + text_to_check.push((text, span)); } }, } @@ -547,6 +575,14 @@ fn check_doc<'a, Events: Iterator<Item = (pulldown_cmark::Event<'a>, Range<usize headers } +fn get_current_span(spans: &[(usize, Span)], idx: usize) -> (usize, Span) { + let index = match spans.binary_search_by(|c| c.0.cmp(&idx)) { + Ok(o) => o, + Err(e) => e - 1, + }; + spans[index] +} + fn check_code(cx: &LateContext<'_>, text: &str, edition: Edition, span: Span) { fn has_needless_main(code: &str, edition: Edition) -> bool { rustc_driver::catch_fatal_errors(|| { diff --git a/clippy_lints/src/eval_order_dependence.rs b/clippy_lints/src/eval_order_dependence.rs index 5fdf5bc9e9d..03a8b40df55 100644 --- a/clippy_lints/src/eval_order_dependence.rs +++ b/clippy_lints/src/eval_order_dependence.rs @@ -38,7 +38,7 @@ declare_clippy_lint! { /// let a = tmp + x; /// ``` pub EVAL_ORDER_DEPENDENCE, - complexity, + suspicious, "whether a variable read occurs before a write depends on sub-expression evaluation order" } diff --git a/clippy_lints/src/float_equality_without_abs.rs b/clippy_lints/src/float_equality_without_abs.rs index b5ebe5f90ba..1e503cc795c 100644 --- a/clippy_lints/src/float_equality_without_abs.rs +++ b/clippy_lints/src/float_equality_without_abs.rs @@ -36,7 +36,7 @@ declare_clippy_lint! { /// } /// ``` pub FLOAT_EQUALITY_WITHOUT_ABS, - correctness, + suspicious, "float equality check without `.abs()`" } diff --git a/clippy_lints/src/formatting.rs b/clippy_lints/src/formatting.rs index 3bd6a09d365..8aefb8d46f6 100644 --- a/clippy_lints/src/formatting.rs +++ b/clippy_lints/src/formatting.rs @@ -22,7 +22,7 @@ declare_clippy_lint! { /// a =- 42; // confusing, should it be `a -= 42` or `a = -42`? /// ``` pub SUSPICIOUS_ASSIGNMENT_FORMATTING, - style, + suspicious, "suspicious formatting of `*=`, `-=` or `!=`" } @@ -44,7 +44,7 @@ declare_clippy_lint! { /// } /// ``` pub SUSPICIOUS_UNARY_OP_FORMATTING, - style, + suspicious, "suspicious formatting of unary `-` or `!` on the RHS of a BinOp" } @@ -80,7 +80,7 @@ declare_clippy_lint! { /// } /// ``` pub SUSPICIOUS_ELSE_FORMATTING, - style, + suspicious, "suspicious formatting of `else`" } diff --git a/clippy_lints/src/get_last_with_len.rs b/clippy_lints/src/get_last_with_len.rs index 3707e792177..8e45fdfecc4 100644 --- a/clippy_lints/src/get_last_with_len.rs +++ b/clippy_lints/src/get_last_with_len.rs @@ -74,7 +74,7 @@ impl<'tcx> LateLintPass<'tcx> for GetLastWithLen { // LHS of subtraction is "x.len()" if let ExprKind::MethodCall(arg_lhs_path, _, lhs_args, _) = &lhs.kind; - if arg_lhs_path.ident.name == sym!(len); + if arg_lhs_path.ident.name == sym::len; if let Some(arg_lhs_struct) = lhs_args.get(0); // The two vectors referenced (x in x.get(...) and in x.len()) diff --git a/clippy_lints/src/if_let_mutex.rs b/clippy_lints/src/if_let_mutex.rs index f661f7ede82..5403d76ea30 100644 --- a/clippy_lints/src/if_let_mutex.rs +++ b/clippy_lints/src/if_let_mutex.rs @@ -83,7 +83,7 @@ impl<'tcx> LateLintPass<'tcx> for IfLetMutex { } } -/// Checks if `Mutex::lock` is called in the `if let _ = expr. +/// Checks if `Mutex::lock` is called in the `if let` expr. pub struct OppVisitor<'a, 'tcx> { mutex_lock_called: bool, found_mutex: Option<&'tcx Expr<'tcx>>, diff --git a/clippy_lints/src/len_zero.rs b/clippy_lints/src/len_zero.rs index 583514b22f9..d69187f6746 100644 --- a/clippy_lints/src/len_zero.rs +++ b/clippy_lints/src/len_zero.rs @@ -128,7 +128,7 @@ impl<'tcx> LateLintPass<'tcx> for LenZero { fn check_impl_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx ImplItem<'_>) { if_chain! { - if item.ident.as_str() == "len"; + if item.ident.name == sym::len; if let ImplItemKind::Fn(sig, _) = &item.kind; if sig.decl.implicit_self.has_implicit_self(); if cx.access_levels.is_exported(item.hir_id()); @@ -189,8 +189,8 @@ impl<'tcx> LateLintPass<'tcx> for LenZero { } fn check_trait_items(cx: &LateContext<'_>, visited_trait: &Item<'_>, trait_items: &[TraitItemRef]) { - fn is_named_self(cx: &LateContext<'_>, item: &TraitItemRef, name: &str) -> bool { - item.ident.name.as_str() == name + fn is_named_self(cx: &LateContext<'_>, item: &TraitItemRef, name: Symbol) -> bool { + item.ident.name == name && if let AssocItemKind::Fn { has_self } = item.kind { has_self && { cx.tcx.fn_sig(item.id.def_id).inputs().skip_binder().len() == 1 } } else { @@ -207,7 +207,9 @@ fn check_trait_items(cx: &LateContext<'_>, visited_trait: &Item<'_>, trait_items } } - if cx.access_levels.is_exported(visited_trait.hir_id()) && trait_items.iter().any(|i| is_named_self(cx, i, "len")) { + if cx.access_levels.is_exported(visited_trait.hir_id()) + && trait_items.iter().any(|i| is_named_self(cx, i, sym::len)) + { let mut current_and_super_traits = DefIdSet::default(); fill_trait_set(visited_trait.def_id.to_def_id(), &mut current_and_super_traits, cx); @@ -401,7 +403,7 @@ fn check_len( return; } - if method_name.as_str() == "len" && args.len() == 1 && has_is_empty(cx, &args[0]) { + if method_name == sym::len && args.len() == 1 && has_is_empty(cx, &args[0]) { let mut applicability = Applicability::MachineApplicable; span_lint_and_sugg( cx, diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index e7dd3952b3a..e0325738466 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -60,9 +60,9 @@ use rustc_session::Session; /// 4. The `description` that contains a short explanation on what's wrong with code where the /// lint is triggered. /// -/// Currently the categories `style`, `correctness`, `complexity` and `perf` are enabled by default. -/// As said in the README.md of this repository, if the lint level mapping changes, please update -/// README.md. +/// Currently the categories `style`, `correctness`, `suspicious`, `complexity` and `perf` are +/// enabled by default. As said in the README.md of this repository, if the lint level mapping +/// changes, please update README.md. /// /// # Example /// @@ -106,6 +106,11 @@ macro_rules! declare_clippy_lint { $(#[$attr])* pub clippy::$name, Deny, $description, report_in_external_macro: true } }; + { $(#[$attr:meta])* pub $name:tt, suspicious, $description:tt } => { + declare_tool_lint! { + $(#[$attr])* pub clippy::$name, Warn, $description, report_in_external_macro: true + } + }; { $(#[$attr:meta])* pub $name:tt, complexity, $description:tt } => { declare_tool_lint! { $(#[$attr])* pub clippy::$name, Warn, $description, report_in_external_macro: true @@ -187,6 +192,8 @@ mod default_numeric_fallback; mod dereference; mod derive; mod disallowed_method; +mod disallowed_script_idents; +mod disallowed_type; mod doc; mod double_comparison; mod double_parens; @@ -254,7 +261,6 @@ mod manual_strip; mod manual_unwrap_or; mod map_clone; mod map_err_ignore; -mod map_identity; mod map_unit_fn; mod match_on_vec_items; mod matches; @@ -267,6 +273,7 @@ mod misc; mod misc_early; mod missing_const_for_fn; mod missing_doc; +mod missing_enforced_import_rename; mod missing_inline; mod modulo_arithmetic; mod multiple_crate_versions; @@ -293,6 +300,7 @@ mod no_effect; mod non_copy_const; mod non_expressive_names; mod non_octal_unix_permissions; +mod nonstandard_macro_braces; mod open_options; mod option_env_unwrap; mod option_if_let_else; @@ -483,11 +491,11 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: ); store.register_removed( "clippy::pub_enum_variant_names", - "set the `avoid_breaking_exported_api` config option to `false` to enable the `enum_variant_names` lint for public items", + "set the `avoid-breaking-exported-api` config option to `false` to enable the `enum_variant_names` lint for public items", ); store.register_removed( "clippy::wrong_pub_self_convention", - "set the `avoid_breaking_exported_api` config option to `false` to enable the `wrong_self_convention` lint for public items", + "set the `avoid-breaking-exported-api` config option to `false` to enable the `wrong_self_convention` lint for public items", ); // end deprecated lints, do not remove this comment, it’s used in `update_lints` @@ -583,6 +591,8 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: derive::EXPL_IMPL_CLONE_ON_COPY, derive::UNSAFE_DERIVE_DESERIALIZE, disallowed_method::DISALLOWED_METHOD, + disallowed_script_idents::DISALLOWED_SCRIPT_IDENTS, + disallowed_type::DISALLOWED_TYPE, doc::DOC_MARKDOWN, doc::MISSING_ERRORS_DOC, doc::MISSING_PANICS_DOC, @@ -705,7 +715,6 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: manual_unwrap_or::MANUAL_UNWRAP_OR, map_clone::MAP_CLONE, map_err_ignore::MAP_ERR_IGNORE, - map_identity::MAP_IDENTITY, map_unit_fn::OPTION_MAP_UNIT_FN, map_unit_fn::RESULT_MAP_UNIT_FN, match_on_vec_items::MATCH_ON_VEC_ITEMS, @@ -730,6 +739,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: mem_replace::MEM_REPLACE_OPTION_WITH_NONE, mem_replace::MEM_REPLACE_WITH_DEFAULT, mem_replace::MEM_REPLACE_WITH_UNINIT, + methods::APPEND_INSTEAD_OF_EXTEND, methods::BIND_INSTEAD_OF_MAP, methods::BYTES_NTH, methods::CHARS_LAST_CMP, @@ -765,6 +775,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: methods::MANUAL_STR_REPEAT, methods::MAP_COLLECT_RESULT_UNIT, methods::MAP_FLATTEN, + methods::MAP_IDENTITY, methods::MAP_UNWRAP_OR, methods::NEW_RET_NO_SELF, methods::OK_EXPECT, @@ -810,6 +821,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: misc_early::ZERO_PREFIXED_LITERAL, missing_const_for_fn::MISSING_CONST_FOR_FN, missing_doc::MISSING_DOCS_IN_PRIVATE_ITEMS, + missing_enforced_import_rename::MISSING_ENFORCED_IMPORT_RENAMES, missing_inline::MISSING_INLINE_IN_PUBLIC_ITEMS, modulo_arithmetic::MODULO_ARITHMETIC, multiple_crate_versions::MULTIPLE_CRATE_VERSIONS, @@ -843,6 +855,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: non_expressive_names::MANY_SINGLE_CHAR_NAMES, non_expressive_names::SIMILAR_NAMES, non_octal_unix_permissions::NON_OCTAL_UNIX_PERMISSIONS, + nonstandard_macro_braces::NONSTANDARD_MACRO_BRACES, open_options::NONSENSICAL_OPEN_OPTIONS, option_env_unwrap::OPTION_ENV_UNWRAP, option_if_let_else::OPTION_IF_LET_ELSE, @@ -989,6 +1002,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: LintId::of(create_dir::CREATE_DIR), LintId::of(dbg_macro::DBG_MACRO), LintId::of(default_numeric_fallback::DEFAULT_NUMERIC_FALLBACK), + LintId::of(disallowed_script_idents::DISALLOWED_SCRIPT_IDENTS), LintId::of(else_if_without_else::ELSE_IF_WITHOUT_ELSE), LintId::of(exhaustive_items::EXHAUSTIVE_ENUMS), LintId::of(exhaustive_items::EXHAUSTIVE_STRUCTS), @@ -1013,6 +1027,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: LintId::of(misc::FLOAT_CMP_CONST), LintId::of(misc_early::UNNEEDED_FIELD_PATTERN), LintId::of(missing_doc::MISSING_DOCS_IN_PRIVATE_ITEMS), + LintId::of(missing_enforced_import_rename::MISSING_ENFORCED_IMPORT_RENAMES), LintId::of(missing_inline::MISSING_INLINE_IN_PUBLIC_ITEMS), LintId::of(modulo_arithmetic::MODULO_ARITHMETIC), LintId::of(panic_in_result_fn::PANIC_IN_RESULT_FN), @@ -1090,6 +1105,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: LintId::of(methods::CLONED_INSTEAD_OF_COPIED), LintId::of(methods::FILTER_MAP_NEXT), LintId::of(methods::FLAT_MAP_OPTION), + LintId::of(methods::FROM_ITER_INSTEAD_OF_COLLECT), LintId::of(methods::IMPLICIT_CLONE), LintId::of(methods::INEFFICIENT_TO_STRING), LintId::of(methods::MAP_FLATTEN), @@ -1260,7 +1276,6 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: LintId::of(manual_strip::MANUAL_STRIP), LintId::of(manual_unwrap_or::MANUAL_UNWRAP_OR), LintId::of(map_clone::MAP_CLONE), - LintId::of(map_identity::MAP_IDENTITY), LintId::of(map_unit_fn::OPTION_MAP_UNIT_FN), LintId::of(map_unit_fn::RESULT_MAP_UNIT_FN), LintId::of(matches::INFALLIBLE_DESTRUCTURING_MATCH), @@ -1276,6 +1291,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: LintId::of(mem_replace::MEM_REPLACE_OPTION_WITH_NONE), LintId::of(mem_replace::MEM_REPLACE_WITH_DEFAULT), LintId::of(mem_replace::MEM_REPLACE_WITH_UNINIT), + LintId::of(methods::APPEND_INSTEAD_OF_EXTEND), LintId::of(methods::BIND_INSTEAD_OF_MAP), LintId::of(methods::BYTES_NTH), LintId::of(methods::CHARS_LAST_CMP), @@ -1286,7 +1302,6 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: LintId::of(methods::FILTER_MAP_IDENTITY), LintId::of(methods::FILTER_NEXT), LintId::of(methods::FLAT_MAP_IDENTITY), - LintId::of(methods::FROM_ITER_INSTEAD_OF_COLLECT), LintId::of(methods::INSPECT_FOR_EACH), LintId::of(methods::INTO_ITER_ON_REF), LintId::of(methods::ITERATOR_STEP_BY_ZERO), @@ -1301,6 +1316,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: LintId::of(methods::MANUAL_SATURATING_ARITHMETIC), LintId::of(methods::MANUAL_STR_REPEAT), LintId::of(methods::MAP_COLLECT_RESULT_UNIT), + LintId::of(methods::MAP_IDENTITY), LintId::of(methods::NEW_RET_NO_SELF), LintId::of(methods::OK_EXPECT), LintId::of(methods::OPTION_AS_REF_DEREF), @@ -1359,6 +1375,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: LintId::of(non_expressive_names::JUST_UNDERSCORES_AND_DIGITS), LintId::of(non_expressive_names::MANY_SINGLE_CHAR_NAMES), LintId::of(non_octal_unix_permissions::NON_OCTAL_UNIX_PERMISSIONS), + LintId::of(nonstandard_macro_braces::NONSTANDARD_MACRO_BRACES), LintId::of(open_options::NONSENSICAL_OPEN_OPTIONS), LintId::of(option_env_unwrap::OPTION_ENV_UNWRAP), LintId::of(overflow_check_conditional::OVERFLOW_CHECK_CONDITIONAL), @@ -1447,7 +1464,6 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: store.register_group(true, "clippy::style", Some("clippy_style"), vec![ LintId::of(assertions_on_constants::ASSERTIONS_ON_CONSTANTS), LintId::of(assign_ops::ASSIGN_OP_PATTERN), - LintId::of(attrs::BLANKET_CLIPPY_RESTRICTION_LINTS), LintId::of(blacklisted_name::BLACKLISTED_NAME), LintId::of(blocks_in_if_conditions::BLOCKS_IN_IF_CONDITIONS), LintId::of(bool_assert_comparison::BOOL_ASSERT_COMPARISON), @@ -1465,9 +1481,6 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: LintId::of(eq_op::OP_REF), LintId::of(eta_reduction::REDUNDANT_CLOSURE), LintId::of(float_literal::EXCESSIVE_PRECISION), - LintId::of(formatting::SUSPICIOUS_ASSIGNMENT_FORMATTING), - LintId::of(formatting::SUSPICIOUS_ELSE_FORMATTING), - LintId::of(formatting::SUSPICIOUS_UNARY_OP_FORMATTING), LintId::of(from_over_into::FROM_OVER_INTO), LintId::of(from_str_radix_10::FROM_STR_RADIX_10), LintId::of(functions::DOUBLE_MUST_USE), @@ -1480,7 +1493,6 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: LintId::of(len_zero::LEN_ZERO), LintId::of(literal_representation::INCONSISTENT_DIGIT_GROUPING), LintId::of(literal_representation::UNUSUAL_BYTE_GROUPINGS), - LintId::of(loops::EMPTY_LOOP), LintId::of(loops::FOR_KV_MAP), LintId::of(loops::NEEDLESS_RANGE_LOOP), LintId::of(loops::SAME_ITEM_PUSH), @@ -1501,7 +1513,6 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: LintId::of(methods::BYTES_NTH), LintId::of(methods::CHARS_LAST_CMP), LintId::of(methods::CHARS_NEXT_CMP), - LintId::of(methods::FROM_ITER_INSTEAD_OF_COLLECT), LintId::of(methods::INTO_ITER_ON_REF), LintId::of(methods::ITER_CLONED_COLLECT), LintId::of(methods::ITER_NEXT_SLICE), @@ -1535,6 +1546,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: LintId::of(non_copy_const::DECLARE_INTERIOR_MUTABLE_CONST), LintId::of(non_expressive_names::JUST_UNDERSCORES_AND_DIGITS), LintId::of(non_expressive_names::MANY_SINGLE_CHAR_NAMES), + LintId::of(nonstandard_macro_braces::NONSTANDARD_MACRO_BRACES), LintId::of(ptr::CMP_NULL), LintId::of(ptr::PTR_ARG), LintId::of(ptr_eq::PTR_EQ), @@ -1560,7 +1572,6 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: ]); store.register_group(true, "clippy::complexity", Some("clippy_complexity"), vec![ - LintId::of(assign_ops::MISREFACTORED_ASSIGN_OP), LintId::of(attrs::DEPRECATED_CFG_ATTR), LintId::of(booleans::NONMINIMAL_BOOL), LintId::of(casts::CHAR_LIT_AS_U8), @@ -1570,7 +1581,6 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: LintId::of(double_parens::DOUBLE_PARENS), LintId::of(duration_subsec::DURATION_SUBSEC), LintId::of(eval_order_dependence::DIVERGING_SUB_EXPRESSION), - LintId::of(eval_order_dependence::EVAL_ORDER_DEPENDENCE), LintId::of(explicit_write::EXPLICIT_WRITE), LintId::of(format::USELESS_FORMAT), LintId::of(functions::TOO_MANY_ARGUMENTS), @@ -1581,12 +1591,10 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: LintId::of(lifetimes::NEEDLESS_LIFETIMES), LintId::of(loops::EXPLICIT_COUNTER_LOOP), LintId::of(loops::MANUAL_FLATTEN), - LintId::of(loops::MUT_RANGE_BOUND), LintId::of(loops::SINGLE_ELEMENT_LOOP), LintId::of(loops::WHILE_LET_LOOP), LintId::of(manual_strip::MANUAL_STRIP), LintId::of(manual_unwrap_or::MANUAL_UNWRAP_OR), - LintId::of(map_identity::MAP_IDENTITY), LintId::of(map_unit_fn::OPTION_MAP_UNIT_FN), LintId::of(map_unit_fn::RESULT_MAP_UNIT_FN), LintId::of(matches::MATCH_AS_REF), @@ -1601,11 +1609,11 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: LintId::of(methods::ITER_COUNT), LintId::of(methods::MANUAL_FILTER_MAP), LintId::of(methods::MANUAL_FIND_MAP), + LintId::of(methods::MAP_IDENTITY), LintId::of(methods::OPTION_AS_REF_DEREF), LintId::of(methods::OPTION_FILTER_MAP), LintId::of(methods::SEARCH_IS_SOME), LintId::of(methods::SKIP_WHILE_NEXT), - LintId::of(methods::SUSPICIOUS_MAP), LintId::of(methods::UNNECESSARY_FILTER_MAP), LintId::of(methods::USELESS_ASREF), LintId::of(misc::SHORT_CIRCUIT_STATEMENT), @@ -1674,7 +1682,6 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: LintId::of(enum_clike::ENUM_CLIKE_UNPORTABLE_VARIANT), LintId::of(eq_op::EQ_OP), LintId::of(erasing_op::ERASING_OP), - LintId::of(float_equality_without_abs::FLOAT_EQUALITY_WITHOUT_ABS), LintId::of(formatting::POSSIBLE_MISSING_COMMA), LintId::of(functions::NOT_UNSAFE_PTR_ARG_DEREF), LintId::of(if_let_mutex::IF_LET_MUTEX), @@ -1684,7 +1691,6 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: LintId::of(inline_fn_without_body::INLINE_FN_WITHOUT_BODY), LintId::of(let_underscore::LET_UNDERSCORE_LOCK), LintId::of(literal_representation::MISTYPED_LITERAL_SUFFIXES), - LintId::of(loops::FOR_LOOPS_OVER_FALLIBLES), LintId::of(loops::ITER_NEXT_LOOP), LintId::of(loops::NEVER_LOOP), LintId::of(loops::WHILE_IMMUTABLE_CONDITION), @@ -1699,7 +1705,6 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: LintId::of(misc::CMP_NAN), LintId::of(misc::FLOAT_CMP), LintId::of(misc::MODULO_ONE), - LintId::of(mut_key::MUTABLE_KEY_TYPE), LintId::of(non_octal_unix_permissions::NON_OCTAL_UNIX_PERMISSIONS), LintId::of(open_options::NONSENSICAL_OPEN_OPTIONS), LintId::of(option_env_unwrap::OPTION_ENV_UNWRAP), @@ -1710,8 +1715,6 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: LintId::of(self_assignment::SELF_ASSIGNMENT), LintId::of(serde_api::SERDE_API_MISUSE), LintId::of(size_of_in_element_count::SIZE_OF_IN_ELEMENT_COUNT), - LintId::of(suspicious_trait_impl::SUSPICIOUS_ARITHMETIC_IMPL), - LintId::of(suspicious_trait_impl::SUSPICIOUS_OP_ASSIGN_IMPL), LintId::of(swap::ALMOST_SWAPPED), LintId::of(to_string_in_display::TO_STRING_IN_DISPLAY), LintId::of(transmute::UNSOUND_COLLECTION_TRANSMUTE), @@ -1728,6 +1731,23 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: LintId::of(vec_resize_to_zero::VEC_RESIZE_TO_ZERO), ]); + store.register_group(true, "clippy::suspicious", None, vec![ + LintId::of(assign_ops::MISREFACTORED_ASSIGN_OP), + LintId::of(attrs::BLANKET_CLIPPY_RESTRICTION_LINTS), + LintId::of(eval_order_dependence::EVAL_ORDER_DEPENDENCE), + LintId::of(float_equality_without_abs::FLOAT_EQUALITY_WITHOUT_ABS), + LintId::of(formatting::SUSPICIOUS_ASSIGNMENT_FORMATTING), + LintId::of(formatting::SUSPICIOUS_ELSE_FORMATTING), + LintId::of(formatting::SUSPICIOUS_UNARY_OP_FORMATTING), + LintId::of(loops::EMPTY_LOOP), + LintId::of(loops::FOR_LOOPS_OVER_FALLIBLES), + LintId::of(loops::MUT_RANGE_BOUND), + LintId::of(methods::SUSPICIOUS_MAP), + LintId::of(mut_key::MUTABLE_KEY_TYPE), + LintId::of(suspicious_trait_impl::SUSPICIOUS_ARITHMETIC_IMPL), + LintId::of(suspicious_trait_impl::SUSPICIOUS_OP_ASSIGN_IMPL), + ]); + store.register_group(true, "clippy::perf", Some("clippy_perf"), vec![ LintId::of(entry::MAP_ENTRY), LintId::of(escape::BOXED_LOCAL), @@ -1735,6 +1755,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: LintId::of(large_enum_variant::LARGE_ENUM_VARIANT), LintId::of(loops::MANUAL_MEMCPY), LintId::of(loops::NEEDLESS_COLLECT), + LintId::of(methods::APPEND_INSTEAD_OF_EXTEND), LintId::of(methods::EXPECT_FUN_CALL), LintId::of(methods::ITER_NTH), LintId::of(methods::MANUAL_STR_REPEAT), @@ -1761,6 +1782,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: LintId::of(attrs::EMPTY_LINE_AFTER_OUTER_ATTR), LintId::of(cognitive_complexity::COGNITIVE_COMPLEXITY), LintId::of(disallowed_method::DISALLOWED_METHOD), + LintId::of(disallowed_type::DISALLOWED_TYPE), LintId::of(fallible_impl_from::FALLIBLE_IMPL_FROM), LintId::of(floating_point_arithmetic::IMPRECISE_FLOPS), LintId::of(floating_point_arithmetic::SUBOPTIMAL_FLOPS), @@ -2038,8 +2060,9 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: store.register_early_pass(move || box non_expressive_names::NonExpressiveNames { single_char_binding_names_threshold, }); + let macro_matcher = conf.standard_macro_braces.iter().cloned().collect::<FxHashSet<_>>(); + store.register_early_pass(move || box nonstandard_macro_braces::MacroBraces::new(¯o_matcher)); store.register_late_pass(|| box macro_use::MacroUseImports::default()); - store.register_late_pass(|| box map_identity::MapIdentity); store.register_late_pass(|| box pattern_type_mismatch::PatternTypeMismatch); store.register_late_pass(|| box stable_sort_primitive::StableSortPrimitive); store.register_late_pass(|| box repeat_once::RepeatOnce); @@ -2066,7 +2089,12 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: store.register_late_pass(move || box if_then_some_else_none::IfThenSomeElseNone::new(msrv)); store.register_early_pass(|| box bool_assert_comparison::BoolAssertComparison); store.register_late_pass(|| box unused_async::UnusedAsync); - + let disallowed_types = conf.disallowed_types.iter().cloned().collect::<FxHashSet<_>>(); + store.register_late_pass(move || box disallowed_type::DisallowedType::new(&disallowed_types)); + let import_renames = conf.enforced_import_renames.clone(); + store.register_late_pass(move || box missing_enforced_import_rename::ImportRename::new(import_renames.clone())); + let scripts = conf.allowed_scripts.clone(); + store.register_early_pass(move || box disallowed_script_idents::DisallowedScriptIdents::new(&scripts)); } #[rustfmt::skip] diff --git a/clippy_lints/src/loops/manual_memcpy.rs b/clippy_lints/src/loops/manual_memcpy.rs index c91fe88757e..a98e2dc1372 100644 --- a/clippy_lints/src/loops/manual_memcpy.rs +++ b/clippy_lints/src/loops/manual_memcpy.rs @@ -118,7 +118,7 @@ fn build_manual_memcpy_suggestion<'tcx>( let print_limit = |end: &Expr<'_>, end_str: &str, base: &Expr<'_>, sugg: MinifyingSugg<'static>| { if_chain! { if let ExprKind::MethodCall(method, _, len_args, _) = end.kind; - if method.ident.name == sym!(len); + if method.ident.name == sym::len; if len_args.len() == 1; if let Some(arg) = len_args.get(0); if path_to_local(arg) == path_to_local(base); diff --git a/clippy_lints/src/loops/mod.rs b/clippy_lints/src/loops/mod.rs index a4bc3e6bd10..56a123b69c6 100644 --- a/clippy_lints/src/loops/mod.rs +++ b/clippy_lints/src/loops/mod.rs @@ -199,7 +199,7 @@ declare_clippy_lint! { /// } /// ``` pub FOR_LOOPS_OVER_FALLIBLES, - correctness, + suspicious, "for-looping over an `Option` or a `Result`, which is more clearly expressed as an `if let`" } @@ -313,7 +313,7 @@ declare_clippy_lint! { /// loop {} /// ``` pub EMPTY_LOOP, - style, + suspicious, "empty `loop {}`, which should block or sleep" } @@ -401,7 +401,7 @@ declare_clippy_lint! { /// } /// ``` pub MUT_RANGE_BOUND, - complexity, + suspicious, "for loop over a range where one of the bounds is a mutable variable" } diff --git a/clippy_lints/src/loops/needless_collect.rs b/clippy_lints/src/loops/needless_collect.rs index eb82c9c27c3..51d7def137e 100644 --- a/clippy_lints/src/loops/needless_collect.rs +++ b/clippy_lints/src/loops/needless_collect.rs @@ -7,10 +7,10 @@ use clippy_utils::{is_trait_method, path_to_local_id}; use if_chain::if_chain; use rustc_errors::Applicability; use rustc_hir::intravisit::{walk_block, walk_expr, NestedVisitorMap, Visitor}; -use rustc_hir::{Block, Expr, ExprKind, GenericArg, GenericArgs, HirId, Local, Pat, PatKind, QPath, StmtKind, Ty}; +use rustc_hir::{Block, Expr, ExprKind, HirId, PatKind, StmtKind}; use rustc_lint::LateContext; use rustc_middle::hir::map::Map; -use rustc_span::symbol::{sym, Ident}; +use rustc_span::sym; use rustc_span::{MultiSpan, Span}; const NEEDLESS_COLLECT_MSG: &str = "avoid using `collect()` when not needed"; @@ -24,10 +24,8 @@ fn check_needless_collect_direct_usage<'tcx>(expr: &'tcx Expr<'_>, cx: &LateCont if let ExprKind::MethodCall(method, _, args, _) = expr.kind; if let ExprKind::MethodCall(chain_method, method0_span, _, _) = args[0].kind; if chain_method.ident.name == sym!(collect) && is_trait_method(cx, &args[0], sym::Iterator); - if let Some(generic_args) = chain_method.args; - if let Some(GenericArg::Type(ref ty)) = generic_args.args.get(0); - if let Some(ty) = cx.typeck_results().node_type_opt(ty.hir_id); then { + let ty = cx.typeck_results().expr_ty(&args[0]); let mut applicability = Applicability::MachineApplicable; let is_empty_sugg = "next().is_none()".to_string(); let method_name = &*method.ident.name.as_str(); @@ -72,40 +70,25 @@ fn check_needless_collect_direct_usage<'tcx>(expr: &'tcx Expr<'_>, cx: &LateCont } fn check_needless_collect_indirect_usage<'tcx>(expr: &'tcx Expr<'_>, cx: &LateContext<'tcx>) { - fn get_hir_id<'tcx>(ty: Option<&Ty<'tcx>>, method_args: Option<&GenericArgs<'tcx>>) -> Option<HirId> { - if let Some(ty) = ty { - return Some(ty.hir_id); - } - - if let Some(generic_args) = method_args { - if let Some(GenericArg::Type(ref ty)) = generic_args.args.get(0) { - return Some(ty.hir_id); - } - } - - None - } if let ExprKind::Block(block, _) = expr.kind { for stmt in block.stmts { if_chain! { - if let StmtKind::Local( - Local { pat: Pat { hir_id: pat_id, kind: PatKind::Binding(_, _, ident, .. ), .. }, - init: Some(init_expr), ty, .. } - ) = stmt.kind; + if let StmtKind::Local(local) = stmt.kind; + if let PatKind::Binding(_, id, ..) = local.pat.kind; + if let Some(init_expr) = local.init; if let ExprKind::MethodCall(method_name, collect_span, &[ref iter_source], ..) = init_expr.kind; if method_name.ident.name == sym!(collect) && is_trait_method(cx, init_expr, sym::Iterator); - if let Some(hir_id) = get_hir_id(*ty, method_name.args); - if let Some(ty) = cx.typeck_results().node_type_opt(hir_id); + let ty = cx.typeck_results().expr_ty(init_expr); if is_type_diagnostic_item(cx, ty, sym::vec_type) || is_type_diagnostic_item(cx, ty, sym::vecdeque_type) || is_type_diagnostic_item(cx, ty, sym::BinaryHeap) || is_type_diagnostic_item(cx, ty, sym::LinkedList); - if let Some(iter_calls) = detect_iter_and_into_iters(block, *ident); + if let Some(iter_calls) = detect_iter_and_into_iters(block, id); if let [iter_call] = &*iter_calls; then { let mut used_count_visitor = UsedCountVisitor { cx, - id: *pat_id, + id, count: 0, }; walk_block(&mut used_count_visitor, block); @@ -187,48 +170,40 @@ enum IterFunctionKind { struct IterFunctionVisitor { uses: Vec<IterFunction>, seen_other: bool, - target: Ident, + target: HirId, } impl<'tcx> Visitor<'tcx> for IterFunctionVisitor { fn visit_expr(&mut self, expr: &'tcx Expr<'tcx>) { // Check function calls on our collection - if_chain! { - if let ExprKind::MethodCall(method_name, _, args, _) = &expr.kind; - if let Some(Expr { kind: ExprKind::Path(QPath::Resolved(_, path)), .. }) = args.get(0); - if let &[name] = &path.segments; - if name.ident == self.target; - then { - let len = sym!(len); - let is_empty = sym!(is_empty); - let contains = sym!(contains); - match method_name.ident.name { - sym::into_iter => self.uses.push( - IterFunction { func: IterFunctionKind::IntoIter, span: expr.span } - ), - name if name == len => self.uses.push( - IterFunction { func: IterFunctionKind::Len, span: expr.span } - ), - name if name == is_empty => self.uses.push( - IterFunction { func: IterFunctionKind::IsEmpty, span: expr.span } - ), - name if name == contains => self.uses.push( - IterFunction { func: IterFunctionKind::Contains(args[1].span), span: expr.span } - ), + if let ExprKind::MethodCall(method_name, _, [recv, args @ ..], _) = &expr.kind { + if path_to_local_id(recv, self.target) { + match &*method_name.ident.name.as_str() { + "into_iter" => self.uses.push(IterFunction { + func: IterFunctionKind::IntoIter, + span: expr.span, + }), + "len" => self.uses.push(IterFunction { + func: IterFunctionKind::Len, + span: expr.span, + }), + "is_empty" => self.uses.push(IterFunction { + func: IterFunctionKind::IsEmpty, + span: expr.span, + }), + "contains" => self.uses.push(IterFunction { + func: IterFunctionKind::Contains(args[0].span), + span: expr.span, + }), _ => self.seen_other = true, } - return + return; } } // Check if the collection is used for anything else - if_chain! { - if let Expr { kind: ExprKind::Path(QPath::Resolved(_, path)), .. } = expr; - if let &[name] = &path.segments; - if name.ident == self.target; - then { - self.seen_other = true; - } else { - walk_expr(self, expr); - } + if path_to_local_id(expr, self.target) { + self.seen_other = true; + } else { + walk_expr(self, expr); } } @@ -262,10 +237,10 @@ impl<'a, 'tcx> Visitor<'tcx> for UsedCountVisitor<'a, 'tcx> { /// Detect the occurrences of calls to `iter` or `into_iter` for the /// given identifier -fn detect_iter_and_into_iters<'tcx>(block: &'tcx Block<'tcx>, identifier: Ident) -> Option<Vec<IterFunction>> { +fn detect_iter_and_into_iters<'tcx>(block: &'tcx Block<'tcx>, id: HirId) -> Option<Vec<IterFunction>> { let mut visitor = IterFunctionVisitor { uses: Vec::new(), - target: identifier, + target: id, seen_other: false, }; visitor.visit_block(block); diff --git a/clippy_lints/src/loops/needless_range_loop.rs b/clippy_lints/src/loops/needless_range_loop.rs index 3065bcc3e6c..3810d0dcc05 100644 --- a/clippy_lints/src/loops/needless_range_loop.rs +++ b/clippy_lints/src/loops/needless_range_loop.rs @@ -192,7 +192,7 @@ fn is_len_call(expr: &Expr<'_>, var: Symbol) -> bool { if_chain! { if let ExprKind::MethodCall(method, _, len_args, _) = expr.kind; if len_args.len() == 1; - if method.ident.name == sym!(len); + if method.ident.name == sym::len; if let ExprKind::Path(QPath::Resolved(_, path)) = len_args[0].kind; if path.segments.len() == 1; if path.segments[0].ident.name == var; diff --git a/clippy_lints/src/loops/while_let_on_iterator.rs b/clippy_lints/src/loops/while_let_on_iterator.rs index 63560047578..d57588716a5 100644 --- a/clippy_lints/src/loops/while_let_on_iterator.rs +++ b/clippy_lints/src/loops/while_let_on_iterator.rs @@ -1,7 +1,9 @@ use super::WHILE_LET_ON_ITERATOR; use clippy_utils::diagnostics::span_lint_and_sugg; use clippy_utils::source::snippet_with_applicability; -use clippy_utils::{get_enclosing_loop, is_refutable, is_trait_method, match_def_path, paths, visitors::is_res_used}; +use clippy_utils::{ + get_enclosing_loop_or_closure, is_refutable, is_trait_method, match_def_path, paths, visitors::is_res_used, +}; use if_chain::if_chain; use rustc_errors::Applicability; use rustc_hir::intravisit::{walk_expr, ErasedMap, NestedVisitorMap, Visitor}; @@ -315,9 +317,10 @@ fn needs_mutable_borrow(cx: &LateContext<'tcx>, iter_expr: &IterExpr, loop_expr: } } - if let Some(e) = get_enclosing_loop(cx.tcx, loop_expr) { - // The iterator expression will be used on the next iteration unless it is declared within the outer - // loop. + if let Some(e) = get_enclosing_loop_or_closure(cx.tcx, loop_expr) { + // The iterator expression will be used on the next iteration (for loops), or on the next call (for + // closures) unless it is declared within the enclosing expression. TODO: Check for closures + // used where an `FnOnce` type is expected. let local_id = match iter_expr.path { Res::Local(id) => id, _ => return true, diff --git a/clippy_lints/src/manual_map.rs b/clippy_lints/src/manual_map.rs index 0b873534f2c..97e4a983f32 100644 --- a/clippy_lints/src/manual_map.rs +++ b/clippy_lints/src/manual_map.rs @@ -3,19 +3,17 @@ use clippy_utils::diagnostics::span_lint_and_sugg; use clippy_utils::source::{snippet_with_applicability, snippet_with_context}; use clippy_utils::ty::{is_type_diagnostic_item, peel_mid_ty_refs_is_mutable}; use clippy_utils::{ - can_move_expr_to_closure, in_constant, is_allowed, is_else_clause, is_lang_ctor, match_var, peel_hir_expr_refs, + can_move_expr_to_closure, in_constant, is_allowed, is_else_clause, is_lang_ctor, path_to_local_id, + peel_hir_expr_refs, }; use rustc_ast::util::parser::PREC_POSTFIX; use rustc_errors::Applicability; use rustc_hir::LangItem::{OptionNone, OptionSome}; -use rustc_hir::{Arm, BindingAnnotation, Block, Expr, ExprKind, MatchSource, Mutability, Pat, PatKind}; +use rustc_hir::{Arm, BindingAnnotation, Block, Expr, ExprKind, HirId, MatchSource, Mutability, Pat, PatKind}; use rustc_lint::{LateContext, LateLintPass, LintContext}; use rustc_middle::lint::in_external_macro; use rustc_session::{declare_lint_pass, declare_tool_lint}; -use rustc_span::{ - symbol::{sym, Ident}, - SyntaxContext, -}; +use rustc_span::{sym, SyntaxContext}; declare_clippy_lint! { /// **What it does:** Checks for usages of `match` which could be implemented using `map` @@ -141,13 +139,13 @@ impl LateLintPass<'_> for ManualMap { scrutinee_str.into() }; - let body_str = if let PatKind::Binding(annotation, _, some_binding, None) = some_pat.kind { - match can_pass_as_func(cx, some_binding, some_expr) { + let body_str = if let PatKind::Binding(annotation, id, some_binding, None) = some_pat.kind { + match can_pass_as_func(cx, id, some_expr) { Some(func) if func.span.ctxt() == some_expr.span.ctxt() => { snippet_with_applicability(cx, func.span, "..", &mut app).into_owned() }, _ => { - if match_var(some_expr, some_binding.name) + if path_to_local_id(some_expr, id) && !is_allowed(cx, MATCH_AS_REF, expr.hir_id) && binding_ref.is_some() { @@ -199,10 +197,10 @@ impl LateLintPass<'_> for ManualMap { // Checks whether the expression could be passed as a function, or whether a closure is needed. // Returns the function to be passed to `map` if it exists. -fn can_pass_as_func(cx: &LateContext<'tcx>, binding: Ident, expr: &'tcx Expr<'_>) -> Option<&'tcx Expr<'tcx>> { +fn can_pass_as_func(cx: &LateContext<'tcx>, binding: HirId, expr: &'tcx Expr<'_>) -> Option<&'tcx Expr<'tcx>> { match expr.kind { ExprKind::Call(func, [arg]) - if match_var(arg, binding.name) && cx.typeck_results().expr_adjustments(arg).is_empty() => + if path_to_local_id(arg, binding) && cx.typeck_results().expr_adjustments(arg).is_empty() => { Some(func) }, diff --git a/clippy_lints/src/map_identity.rs b/clippy_lints/src/map_identity.rs deleted file mode 100644 index 41cda23510e..00000000000 --- a/clippy_lints/src/map_identity.rs +++ /dev/null @@ -1,126 +0,0 @@ -use clippy_utils::diagnostics::span_lint_and_sugg; -use clippy_utils::ty::is_type_diagnostic_item; -use clippy_utils::{is_adjusted, is_qpath_def_path, is_trait_method, match_var, paths, remove_blocks}; -use if_chain::if_chain; -use rustc_errors::Applicability; -use rustc_hir::{Body, Expr, ExprKind, Pat, PatKind, QPath, StmtKind}; -use rustc_lint::{LateContext, LateLintPass}; -use rustc_session::{declare_lint_pass, declare_tool_lint}; -use rustc_span::sym; - -declare_clippy_lint! { - /// **What it does:** Checks for instances of `map(f)` where `f` is the identity function. - /// - /// **Why is this bad?** It can be written more concisely without the call to `map`. - /// - /// **Known problems:** None. - /// - /// **Example:** - /// - /// ```rust - /// let x = [1, 2, 3]; - /// let y: Vec<_> = x.iter().map(|x| x).map(|x| 2*x).collect(); - /// ``` - /// Use instead: - /// ```rust - /// let x = [1, 2, 3]; - /// let y: Vec<_> = x.iter().map(|x| 2*x).collect(); - /// ``` - pub MAP_IDENTITY, - complexity, - "using iterator.map(|x| x)" -} - -declare_lint_pass!(MapIdentity => [MAP_IDENTITY]); - -impl<'tcx> LateLintPass<'tcx> for MapIdentity { - fn check_expr(&mut self, cx: &LateContext<'_>, expr: &Expr<'_>) { - if expr.span.from_expansion() { - return; - } - - if_chain! { - if let Some([caller, func]) = get_map_argument(cx, expr); - if is_expr_identity_function(cx, func); - then { - span_lint_and_sugg( - cx, - MAP_IDENTITY, - expr.span.trim_start(caller.span).unwrap(), - "unnecessary map of the identity function", - "remove the call to `map`", - String::new(), - Applicability::MachineApplicable - ) - } - } - } -} - -/// Returns the arguments passed into map() if the expression is a method call to -/// map(). Otherwise, returns None. -fn get_map_argument<'a>(cx: &LateContext<'_>, expr: &'a Expr<'a>) -> Option<&'a [Expr<'a>]> { - if_chain! { - if let ExprKind::MethodCall(method, _, args, _) = expr.kind; - if args.len() == 2 && method.ident.name == sym::map; - let caller_ty = cx.typeck_results().expr_ty(&args[0]); - if is_trait_method(cx, expr, sym::Iterator) - || is_type_diagnostic_item(cx, caller_ty, sym::result_type) - || is_type_diagnostic_item(cx, caller_ty, sym::option_type); - then { - Some(args) - } else { - None - } - } -} - -/// Checks if an expression represents the identity function -/// Only examines closures and `std::convert::identity` -fn is_expr_identity_function(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool { - match expr.kind { - ExprKind::Closure(_, _, body_id, _, _) => is_body_identity_function(cx, cx.tcx.hir().body(body_id)), - ExprKind::Path(ref path) => is_qpath_def_path(cx, path, expr.hir_id, &paths::CONVERT_IDENTITY), - _ => false, - } -} - -/// Checks if a function's body represents the identity function -/// Looks for bodies of the form `|x| x`, `|x| return x`, `|x| { return x }` or `|x| { -/// return x; }` -fn is_body_identity_function(cx: &LateContext<'_>, func: &Body<'_>) -> bool { - let params = func.params; - let body = remove_blocks(&func.value); - - // if there's less/more than one parameter, then it is not the identity function - if params.len() != 1 { - return false; - } - - match body.kind { - ExprKind::Path(QPath::Resolved(None, _)) => match_expr_param(cx, body, params[0].pat), - ExprKind::Ret(Some(ret_val)) => match_expr_param(cx, ret_val, params[0].pat), - ExprKind::Block(block, _) => { - if_chain! { - if block.stmts.len() == 1; - if let StmtKind::Semi(expr) | StmtKind::Expr(expr) = block.stmts[0].kind; - if let ExprKind::Ret(Some(ret_val)) = expr.kind; - then { - match_expr_param(cx, ret_val, params[0].pat) - } else { - false - } - } - }, - _ => false, - } -} - -/// Returns true iff an expression returns the same thing as a parameter's pattern -fn match_expr_param(cx: &LateContext<'_>, expr: &Expr<'_>, pat: &Pat<'_>) -> bool { - if let PatKind::Binding(_, _, ident, _) = pat.kind { - match_var(expr, ident.name) && !(cx.typeck_results().hir_owner == expr.hir_id.owner && is_adjusted(cx, expr)) - } else { - false - } -} diff --git a/clippy_lints/src/matches.rs b/clippy_lints/src/matches.rs index cd3e3b97928..f1e3492c4ec 100644 --- a/clippy_lints/src/matches.rs +++ b/clippy_lints/src/matches.rs @@ -992,9 +992,9 @@ impl CommonPrefixSearcher<'a> { } } -fn is_doc_hidden(cx: &LateContext<'_>, variant_def: &VariantDef) -> bool { +fn is_hidden(cx: &LateContext<'_>, variant_def: &VariantDef) -> bool { let attrs = cx.tcx.get_attrs(variant_def.def_id); - clippy_utils::attrs::is_doc_hidden(attrs) + clippy_utils::attrs::is_doc_hidden(attrs) || clippy_utils::attrs::is_unstable(attrs) } #[allow(clippy::too_many_lines)] @@ -1033,7 +1033,8 @@ fn check_wild_enum_match(cx: &LateContext<'_>, ex: &Expr<'_>, arms: &[Arm<'_>]) // Accumulate the variants which should be put in place of the wildcard because they're not // already covered. - let mut missing_variants: Vec<_> = adt_def.variants.iter().collect(); + let has_hidden = adt_def.variants.iter().any(|x| is_hidden(cx, x)); + let mut missing_variants: Vec<_> = adt_def.variants.iter().filter(|x| !is_hidden(cx, x)).collect(); let mut path_prefix = CommonPrefixSearcher::None; for arm in arms { @@ -1118,7 +1119,7 @@ fn check_wild_enum_match(cx: &LateContext<'_>, ex: &Expr<'_>, arms: &[Arm<'_>]) match missing_variants.as_slice() { [] => (), - [x] if !adt_def.is_variant_list_non_exhaustive() && !is_doc_hidden(cx, x) => span_lint_and_sugg( + [x] if !adt_def.is_variant_list_non_exhaustive() && !has_hidden => span_lint_and_sugg( cx, MATCH_WILDCARD_FOR_SINGLE_VARIANTS, wildcard_span, @@ -1129,7 +1130,7 @@ fn check_wild_enum_match(cx: &LateContext<'_>, ex: &Expr<'_>, arms: &[Arm<'_>]) ), variants => { let mut suggestions: Vec<_> = variants.iter().copied().map(format_suggestion).collect(); - let message = if adt_def.is_variant_list_non_exhaustive() { + let message = if adt_def.is_variant_list_non_exhaustive() || has_hidden { suggestions.push("_".into()); "wildcard matches known variants and will also match future added variants" } else { @@ -2266,7 +2267,8 @@ fn lint_match_arms<'tcx>(cx: &LateContext<'tcx>, expr: &Expr<'_>) { ), ); } else { - diag.span_help(i.pat.span, &format!("consider refactoring into `{} | {}`", lhs, rhs)); + diag.span_help(i.pat.span, &format!("consider refactoring into `{} | {}`", lhs, rhs,)) + .help("...or consider changing the match arm bodies"); } }, ); diff --git a/clippy_lints/src/methods/append_instead_of_extend.rs b/clippy_lints/src/methods/append_instead_of_extend.rs new file mode 100644 index 00000000000..e39a5a1efd1 --- /dev/null +++ b/clippy_lints/src/methods/append_instead_of_extend.rs @@ -0,0 +1,41 @@ +use clippy_utils::diagnostics::span_lint_and_sugg; +use clippy_utils::source::snippet_with_applicability; +use clippy_utils::ty::{is_type_diagnostic_item, is_type_lang_item}; +use if_chain::if_chain; +use rustc_errors::Applicability; +use rustc_hir::{Expr, ExprKind, LangItem}; +use rustc_lint::LateContext; +use rustc_span::symbol::sym; + +use super::APPEND_INSTEAD_OF_EXTEND; + +pub(super) fn check(cx: &LateContext<'_>, expr: &Expr<'_>, recv: &Expr<'_>, arg: &Expr<'_>) { + let ty = cx.typeck_results().expr_ty(recv).peel_refs(); + if_chain! { + if is_type_diagnostic_item(cx, ty, sym::vec_type); + //check source object + if let ExprKind::MethodCall(src_method, _, [drain_vec, drain_arg], _) = &arg.kind; + if src_method.ident.as_str() == "drain"; + if let src_ty = cx.typeck_results().expr_ty(drain_vec).peel_refs(); + if is_type_diagnostic_item(cx, src_ty, sym::vec_type); + //check drain range + if let src_ty_range = cx.typeck_results().expr_ty(drain_arg).peel_refs(); + if is_type_lang_item(cx, src_ty_range, LangItem::RangeFull); + then { + let mut applicability = Applicability::MachineApplicable; + span_lint_and_sugg( + cx, + APPEND_INSTEAD_OF_EXTEND, + expr.span, + "use of `extend` instead of `append` for adding the full range of a second vector", + "try this", + format!( + "{}.append(&mut {})", + snippet_with_applicability(cx, recv.span, "..", &mut applicability), + snippet_with_applicability(cx, drain_vec.span, "..", &mut applicability) + ), + applicability, + ); + } + } +} diff --git a/clippy_lints/src/methods/filter_map_identity.rs b/clippy_lints/src/methods/filter_map_identity.rs index 403fe8d3546..d1b5e945dfd 100644 --- a/clippy_lints/src/methods/filter_map_identity.rs +++ b/clippy_lints/src/methods/filter_map_identity.rs @@ -1,6 +1,5 @@ use clippy_utils::diagnostics::span_lint_and_sugg; -use clippy_utils::{is_expr_path_def_path, is_trait_method, path_to_local_id, paths}; -use if_chain::if_chain; +use clippy_utils::{is_expr_identity_function, is_trait_method}; use rustc_errors::Applicability; use rustc_hir as hir; use rustc_lint::LateContext; @@ -9,32 +8,15 @@ use rustc_span::{source_map::Span, sym}; use super::FILTER_MAP_IDENTITY; pub(super) fn check(cx: &LateContext<'_>, expr: &hir::Expr<'_>, filter_map_arg: &hir::Expr<'_>, filter_map_span: Span) { - if is_trait_method(cx, expr, sym::Iterator) { - let apply_lint = |message: &str| { - span_lint_and_sugg( - cx, - FILTER_MAP_IDENTITY, - filter_map_span.with_hi(expr.span.hi()), - message, - "try", - "flatten()".to_string(), - Applicability::MachineApplicable, - ); - }; - - if_chain! { - if let hir::ExprKind::Closure(_, _, body_id, _, _) = filter_map_arg.kind; - let body = cx.tcx.hir().body(body_id); - - if let hir::PatKind::Binding(_, binding_id, ..) = body.params[0].pat.kind; - if path_to_local_id(&body.value, binding_id); - then { - apply_lint("called `filter_map(|x| x)` on an `Iterator`"); - } - } - - if is_expr_path_def_path(cx, filter_map_arg, &paths::CONVERT_IDENTITY) { - apply_lint("called `filter_map(std::convert::identity)` on an `Iterator`"); - } + if is_trait_method(cx, expr, sym::Iterator) && is_expr_identity_function(cx, filter_map_arg) { + span_lint_and_sugg( + cx, + FILTER_MAP_IDENTITY, + filter_map_span.with_hi(expr.span.hi()), + "use of `filter_map` with an identity function", + "try", + "flatten()".to_string(), + Applicability::MachineApplicable, + ); } } diff --git a/clippy_lints/src/methods/flat_map_identity.rs b/clippy_lints/src/methods/flat_map_identity.rs index 25f8434cb94..6f911d79d0b 100644 --- a/clippy_lints/src/methods/flat_map_identity.rs +++ b/clippy_lints/src/methods/flat_map_identity.rs @@ -1,6 +1,5 @@ use clippy_utils::diagnostics::span_lint_and_sugg; -use clippy_utils::{is_expr_path_def_path, is_trait_method, paths}; -use if_chain::if_chain; +use clippy_utils::{is_expr_identity_function, is_trait_method}; use rustc_errors::Applicability; use rustc_hir as hir; use rustc_lint::LateContext; @@ -15,36 +14,15 @@ pub(super) fn check<'tcx>( flat_map_arg: &'tcx hir::Expr<'_>, flat_map_span: Span, ) { - if is_trait_method(cx, expr, sym::Iterator) { - let apply_lint = |message: &str| { - span_lint_and_sugg( - cx, - FLAT_MAP_IDENTITY, - flat_map_span.with_hi(expr.span.hi()), - message, - "try", - "flatten()".to_string(), - Applicability::MachineApplicable, - ); - }; - - if_chain! { - if let hir::ExprKind::Closure(_, _, body_id, _, _) = flat_map_arg.kind; - let body = cx.tcx.hir().body(body_id); - - if let hir::PatKind::Binding(_, _, binding_ident, _) = body.params[0].pat.kind; - if let hir::ExprKind::Path(hir::QPath::Resolved(_, path)) = body.value.kind; - - if path.segments.len() == 1; - if path.segments[0].ident.name == binding_ident.name; - - then { - apply_lint("called `flat_map(|x| x)` on an `Iterator`"); - } - } - - if is_expr_path_def_path(cx, flat_map_arg, &paths::CONVERT_IDENTITY) { - apply_lint("called `flat_map(std::convert::identity)` on an `Iterator`"); - } + if is_trait_method(cx, expr, sym::Iterator) && is_expr_identity_function(cx, flat_map_arg) { + span_lint_and_sugg( + cx, + FLAT_MAP_IDENTITY, + flat_map_span.with_hi(expr.span.hi()), + "use of `flat_map` with an identity function", + "try", + "flatten()".to_string(), + Applicability::MachineApplicable, + ); } } diff --git a/clippy_lints/src/methods/map_identity.rs b/clippy_lints/src/methods/map_identity.rs new file mode 100644 index 00000000000..538a12566e3 --- /dev/null +++ b/clippy_lints/src/methods/map_identity.rs @@ -0,0 +1,38 @@ +use clippy_utils::diagnostics::span_lint_and_sugg; +use clippy_utils::ty::is_type_diagnostic_item; +use clippy_utils::{is_expr_identity_function, is_trait_method}; +use rustc_errors::Applicability; +use rustc_hir as hir; +use rustc_lint::LateContext; +use rustc_span::{source_map::Span, sym}; + +use super::MAP_IDENTITY; + +pub(super) fn check( + cx: &LateContext<'_>, + expr: &hir::Expr<'_>, + caller: &hir::Expr<'_>, + map_arg: &hir::Expr<'_>, + _map_span: Span, +) { + let caller_ty = cx.typeck_results().expr_ty(caller); + + if_chain! { + if is_trait_method(cx, expr, sym::Iterator) + || is_type_diagnostic_item(cx, caller_ty, sym::result_type) + || is_type_diagnostic_item(cx, caller_ty, sym::option_type); + if is_expr_identity_function(cx, map_arg); + if let Some(sugg_span) = expr.span.trim_start(caller.span); + then { + span_lint_and_sugg( + cx, + MAP_IDENTITY, + sugg_span, + "unnecessary map of the identity function", + "remove the call to `map`", + String::new(), + Applicability::MachineApplicable, + ) + } + } +} diff --git a/clippy_lints/src/methods/mod.rs b/clippy_lints/src/methods/mod.rs index c8ae972f18c..283fcf281df 100644 --- a/clippy_lints/src/methods/mod.rs +++ b/clippy_lints/src/methods/mod.rs @@ -1,3 +1,4 @@ +mod append_instead_of_extend; mod bind_instead_of_map; mod bytes_nth; mod chars_cmp; @@ -35,6 +36,7 @@ mod manual_saturating_arithmetic; mod manual_str_repeat; mod map_collect_result_unit; mod map_flatten; +mod map_identity; mod map_unwrap_or; mod ok_expect; mod option_as_ref_deref; @@ -1031,6 +1033,30 @@ declare_clippy_lint! { "using `.get().unwrap()` or `.get_mut().unwrap()` when using `[]` would work instead" } +declare_clippy_lint! { + /// **What it does:** Checks for occurrences where one vector gets extended instead of append + /// + /// **Why is this bad?** Using `append` instead of `extend` is more concise and faster + /// + /// **Known problems:** None. + /// + /// **Example:** + /// + /// ```rust + /// let mut a = vec![1, 2, 3]; + /// let mut b = vec![4, 5, 6]; + /// + /// // Bad + /// a.extend(b.drain(..)); + /// + /// // Good + /// a.append(&mut b); + /// ``` + pub APPEND_INSTEAD_OF_EXTEND, + perf, + "using vec.append(&mut vec) to move the full range of a vecor to another" +} + declare_clippy_lint! { /// **What it does:** Checks for the use of `.extend(s.chars())` where s is a /// `&str` or `String`. @@ -1222,7 +1248,7 @@ declare_clippy_lint! { /// let _ = (0..3).map(|x| x + 2).count(); /// ``` pub SUSPICIOUS_MAP, - complexity, + suspicious, "suspicious usage of map" } @@ -1504,7 +1530,7 @@ declare_clippy_lint! { /// assert_eq!(v, vec![5, 5, 5, 5, 5]); /// ``` pub FROM_ITER_INSTEAD_OF_COLLECT, - style, + pedantic, "use `.collect()` instead of `::from_iter()`" } @@ -1561,6 +1587,29 @@ declare_clippy_lint! { "call to `filter_map` where `flatten` is sufficient" } +declare_clippy_lint! { + /// **What it does:** Checks for instances of `map(f)` where `f` is the identity function. + /// + /// **Why is this bad?** It can be written more concisely without the call to `map`. + /// + /// **Known problems:** None. + /// + /// **Example:** + /// + /// ```rust + /// let x = [1, 2, 3]; + /// let y: Vec<_> = x.iter().map(|x| x).map(|x| 2*x).collect(); + /// ``` + /// Use instead: + /// ```rust + /// let x = [1, 2, 3]; + /// let y: Vec<_> = x.iter().map(|x| 2*x).collect(); + /// ``` + pub MAP_IDENTITY, + complexity, + "using iterator.map(|x| x)" +} + declare_clippy_lint! { /// **What it does:** Checks for the use of `.bytes().nth()`. /// @@ -1728,6 +1777,7 @@ impl_lint_pass!(Methods => [ FILTER_NEXT, SKIP_WHILE_NEXT, FILTER_MAP_IDENTITY, + MAP_IDENTITY, MANUAL_FILTER_MAP, MANUAL_FIND_MAP, OPTION_FILTER_MAP, @@ -1760,7 +1810,8 @@ impl_lint_pass!(Methods => [ INSPECT_FOR_EACH, IMPLICIT_CLONE, SUSPICIOUS_SPLITN, - MANUAL_STR_REPEAT + MANUAL_STR_REPEAT, + APPEND_INSTEAD_OF_EXTEND ]); /// Extracts a method call name, args, and `Span` of the method name. @@ -1985,7 +2036,7 @@ impl<'tcx> LateLintPass<'tcx> for Methods { fn check_methods<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>, msrv: Option<&RustcVersion>) { if let Some((name, [recv, args @ ..], span)) = method_call!(expr) { match (name, args) { - ("add" | "offset" | "sub" | "wrapping_offset" | "wrapping_add" | "wrapping_sub", [recv, _]) => { + ("add" | "offset" | "sub" | "wrapping_offset" | "wrapping_add" | "wrapping_sub", [_arg]) => { zst_offset::check(cx, expr, recv); }, ("and_then", [arg]) => { @@ -2022,7 +2073,10 @@ fn check_methods<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>, msrv: Optio Some(("ok", [recv], _)) => ok_expect::check(cx, expr, recv), _ => expect_used::check(cx, expr, recv), }, - ("extend", [arg]) => string_extend_chars::check(cx, expr, recv, arg), + ("extend", [arg]) => { + string_extend_chars::check(cx, expr, recv, arg); + append_instead_of_extend::check(cx, expr, recv, arg); + }, ("filter_map", [arg]) => { unnecessary_filter_map::check(cx, expr, arg); filter_map_identity::check(cx, expr, arg, span); @@ -2058,6 +2112,7 @@ fn check_methods<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>, msrv: Optio _ => {}, } } + map_identity::check(cx, expr, recv, m_arg, span); }, ("map_or", [def, map]) => option_map_or_none::check(cx, expr, recv, def, map), ("next", []) => { diff --git a/clippy_lints/src/methods/or_fun_call.rs b/clippy_lints/src/methods/or_fun_call.rs index 800172f4cf3..073c5570a88 100644 --- a/clippy_lints/src/methods/or_fun_call.rs +++ b/clippy_lints/src/methods/or_fun_call.rs @@ -87,7 +87,7 @@ pub(super) fn check<'tcx>( ]; if let hir::ExprKind::MethodCall(path, _, args, _) = &arg.kind { - if path.ident.as_str() == "len" { + if path.ident.name == sym::len { let ty = cx.typeck_results().expr_ty(&args[0]).peel_refs(); match ty.kind() { diff --git a/clippy_lints/src/missing_enforced_import_rename.rs b/clippy_lints/src/missing_enforced_import_rename.rs new file mode 100644 index 00000000000..59565350f72 --- /dev/null +++ b/clippy_lints/src/missing_enforced_import_rename.rs @@ -0,0 +1,102 @@ +use clippy_utils::{diagnostics::span_lint_and_sugg, source::snippet_opt}; + +use rustc_data_structures::fx::FxHashMap; +use rustc_errors::Applicability; +use rustc_hir::{def::Res, def_id::DefId, Crate, Item, ItemKind, UseKind}; +use rustc_lint::{LateContext, LateLintPass, LintContext}; +use rustc_session::{declare_tool_lint, impl_lint_pass}; +use rustc_span::Symbol; + +use crate::utils::conf::Rename; + +declare_clippy_lint! { + /// **What it does:** Checks for imports that do not rename the item as specified + /// in the `enforce-import-renames` config option. + /// + /// **Why is this bad?** Consistency is important, if a project has defined import + /// renames they should be followed. More practically, some item names are too + /// vague outside of their defining scope this can enforce a more meaningful naming. + /// + /// **Known problems:** None + /// + /// **Example:** + /// + /// An example clippy.toml configuration: + /// ```toml + /// # clippy.toml + /// enforced-import-renames = [ { path = "serde_json::Value", rename = "JsonValue" }] + /// ``` + /// + /// ```rust,ignore + /// use serde_json::Value; + /// ``` + /// Use instead: + /// ```rust,ignore + /// use serde_json::Value as JsonValue; + /// ``` + pub MISSING_ENFORCED_IMPORT_RENAMES, + restriction, + "enforce import renames" +} + +pub struct ImportRename { + conf_renames: Vec<Rename>, + renames: FxHashMap<DefId, Symbol>, +} + +impl ImportRename { + pub fn new(conf_renames: Vec<Rename>) -> Self { + Self { + conf_renames, + renames: FxHashMap::default(), + } + } +} + +impl_lint_pass!(ImportRename => [MISSING_ENFORCED_IMPORT_RENAMES]); + +impl LateLintPass<'_> for ImportRename { + fn check_crate(&mut self, cx: &LateContext<'_>, _: &Crate<'_>) { + for Rename { path, rename } in &self.conf_renames { + if let Res::Def(_, id) = clippy_utils::path_to_res(cx, &path.split("::").collect::<Vec<_>>()) { + self.renames.insert(id, Symbol::intern(rename)); + } + } + } + + fn check_item(&mut self, cx: &LateContext<'_>, item: &Item<'_>) { + if_chain! { + if let ItemKind::Use(path, UseKind::Single) = &item.kind; + if let Res::Def(_, id) = path.res; + if let Some(name) = self.renames.get(&id); + // Remove semicolon since it is not present for nested imports + let span_without_semi = cx.sess().source_map().span_until_char(item.span, ';'); + if let Some(snip) = snippet_opt(cx, span_without_semi); + if let Some(import) = match snip.split_once(" as ") { + None => Some(snip.as_str()), + Some((import, rename)) => { + if rename.trim() == &*name.as_str() { + None + } else { + Some(import.trim()) + } + }, + }; + then { + span_lint_and_sugg( + cx, + MISSING_ENFORCED_IMPORT_RENAMES, + span_without_semi, + "this import should be renamed", + "try", + format!( + "{} as {}", + import, + name, + ), + Applicability::MachineApplicable, + ); + } + } + } +} diff --git a/clippy_lints/src/mut_key.rs b/clippy_lints/src/mut_key.rs index 1786d5805d7..6c87136e5e1 100644 --- a/clippy_lints/src/mut_key.rs +++ b/clippy_lints/src/mut_key.rs @@ -50,7 +50,7 @@ declare_clippy_lint! { /// } /// ``` pub MUTABLE_KEY_TYPE, - correctness, + suspicious, "Check for mutable `Map`/`Set` key type" } diff --git a/clippy_lints/src/no_effect.rs b/clippy_lints/src/no_effect.rs index b2206a82208..910b0536092 100644 --- a/clippy_lints/src/no_effect.rs +++ b/clippy_lints/src/no_effect.rs @@ -167,7 +167,7 @@ fn reduce_expression<'a>(cx: &LateContext<'_>, expr: &'a Expr<'a>) -> Option<Vec BlockCheckMode::UnsafeBlock(UnsafeSource::UserProvided) => None, BlockCheckMode::DefaultBlock => Some(vec![&**e]), // in case of compiler-inserted signaling blocks - _ => reduce_expression(cx, e), + BlockCheckMode::UnsafeBlock(_) => reduce_expression(cx, e), } }) } else { diff --git a/clippy_lints/src/nonstandard_macro_braces.rs b/clippy_lints/src/nonstandard_macro_braces.rs new file mode 100644 index 00000000000..1adad5be6dd --- /dev/null +++ b/clippy_lints/src/nonstandard_macro_braces.rs @@ -0,0 +1,276 @@ +use std::{ + fmt, + hash::{Hash, Hasher}, +}; + +use clippy_utils::{diagnostics::span_lint_and_help, in_macro, is_direct_expn_of, source::snippet_opt}; +use if_chain::if_chain; +use rustc_ast::ast; +use rustc_data_structures::fx::{FxHashMap, FxHashSet}; +use rustc_lint::{EarlyContext, EarlyLintPass}; +use rustc_session::{declare_tool_lint, impl_lint_pass}; +use rustc_span::Span; +use serde::{de, Deserialize}; + +declare_clippy_lint! { + /// **What it does:** Checks that common macros are used with consistent bracing. + /// + /// **Why is this bad?** This is mostly a consistency lint although using () or [] + /// doesn't give you a semicolon in item position, which can be unexpected. + /// + /// **Known problems:** + /// None + /// + /// **Example:** + /// + /// ```rust + /// vec!{1, 2, 3}; + /// ``` + /// Use instead: + /// ```rust + /// vec![1, 2, 3]; + /// ``` + pub NONSTANDARD_MACRO_BRACES, + style, + "check consistent use of braces in macro" +} + +const BRACES: &[(&str, &str)] = &[("(", ")"), ("{", "}"), ("[", "]")]; + +/// The (name, (open brace, close brace), source snippet) +type MacroInfo<'a> = (&'a str, &'a (String, String), String); + +#[derive(Clone, Debug, Default)] +pub struct MacroBraces { + macro_braces: FxHashMap<String, (String, String)>, + done: FxHashSet<Span>, +} + +impl MacroBraces { + pub fn new(conf: &FxHashSet<MacroMatcher>) -> Self { + let macro_braces = macro_braces(conf.clone()); + Self { + macro_braces, + done: FxHashSet::default(), + } + } +} + +impl_lint_pass!(MacroBraces => [NONSTANDARD_MACRO_BRACES]); + +impl EarlyLintPass for MacroBraces { + fn check_item(&mut self, cx: &EarlyContext<'_>, item: &ast::Item) { + if let Some((name, braces, snip)) = is_offending_macro(cx, item.span, self) { + let span = item.span.ctxt().outer_expn_data().call_site; + emit_help(cx, snip, braces, name, span); + self.done.insert(span); + } + } + + fn check_stmt(&mut self, cx: &EarlyContext<'_>, stmt: &ast::Stmt) { + if let Some((name, braces, snip)) = is_offending_macro(cx, stmt.span, self) { + let span = stmt.span.ctxt().outer_expn_data().call_site; + emit_help(cx, snip, braces, name, span); + self.done.insert(span); + } + } + + fn check_expr(&mut self, cx: &EarlyContext<'_>, expr: &ast::Expr) { + if let Some((name, braces, snip)) = is_offending_macro(cx, expr.span, self) { + let span = expr.span.ctxt().outer_expn_data().call_site; + emit_help(cx, snip, braces, name, span); + self.done.insert(span); + } + } + + fn check_ty(&mut self, cx: &EarlyContext<'_>, ty: &ast::Ty) { + if let Some((name, braces, snip)) = is_offending_macro(cx, ty.span, self) { + let span = ty.span.ctxt().outer_expn_data().call_site; + emit_help(cx, snip, braces, name, span); + self.done.insert(span); + } + } +} + +fn is_offending_macro<'a>(cx: &EarlyContext<'_>, span: Span, this: &'a MacroBraces) -> Option<MacroInfo<'a>> { + if_chain! { + if in_macro(span); + if let Some((name, braces)) = find_matching_macro(span, &this.macro_braces); + if let Some(snip) = snippet_opt(cx, span.ctxt().outer_expn_data().call_site); + let c = snip.replace(" ", ""); // make formatting consistent + if !c.starts_with(&format!("{}!{}", name, braces.0)); + if !this.done.contains(&span.ctxt().outer_expn_data().call_site); + then { + Some((name, braces, snip)) + } else { + None + } + } +} + +fn emit_help(cx: &EarlyContext<'_>, snip: String, braces: &(String, String), name: &str, span: Span) { + let with_space = &format!("! {}", braces.0); + let without_space = &format!("!{}", braces.0); + let mut help = snip; + for b in BRACES.iter().filter(|b| b.0 != braces.0) { + help = help.replace(b.0, &braces.0).replace(b.1, &braces.1); + // Only `{` traditionally has space before the brace + if braces.0 != "{" && help.contains(with_space) { + help = help.replace(with_space, without_space); + } else if braces.0 == "{" && help.contains(without_space) { + help = help.replace(without_space, with_space); + } + } + span_lint_and_help( + cx, + NONSTANDARD_MACRO_BRACES, + span, + &format!("use of irregular braces for `{}!` macro", name), + Some(span), + &format!("consider writing `{}`", help), + ); +} + +fn find_matching_macro( + span: Span, + braces: &FxHashMap<String, (String, String)>, +) -> Option<(&String, &(String, String))> { + braces + .iter() + .find(|(macro_name, _)| is_direct_expn_of(span, macro_name).is_some()) +} + +fn macro_braces(conf: FxHashSet<MacroMatcher>) -> FxHashMap<String, (String, String)> { + let mut braces = vec![ + macro_matcher!( + name: "print", + braces: ("(", ")"), + ), + macro_matcher!( + name: "println", + braces: ("(", ")"), + ), + macro_matcher!( + name: "eprint", + braces: ("(", ")"), + ), + macro_matcher!( + name: "eprintln", + braces: ("(", ")"), + ), + macro_matcher!( + name: "write", + braces: ("(", ")"), + ), + macro_matcher!( + name: "writeln", + braces: ("(", ")"), + ), + macro_matcher!( + name: "format", + braces: ("(", ")"), + ), + macro_matcher!( + name: "format_args", + braces: ("(", ")"), + ), + macro_matcher!( + name: "vec", + braces: ("[", "]"), + ), + ] + .into_iter() + .collect::<FxHashMap<_, _>>(); + // We want users items to override any existing items + for it in conf { + braces.insert(it.name, it.braces); + } + braces +} + +macro_rules! macro_matcher { + (name: $name:expr, braces: ($open:expr, $close:expr) $(,)?) => { + ($name.to_owned(), ($open.to_owned(), $close.to_owned())) + }; +} +pub(crate) use macro_matcher; + +#[derive(Clone, Debug)] +pub struct MacroMatcher { + name: String, + braces: (String, String), +} + +impl Hash for MacroMatcher { + fn hash<H: Hasher>(&self, state: &mut H) { + self.name.hash(state); + } +} + +impl PartialEq for MacroMatcher { + fn eq(&self, other: &Self) -> bool { + self.name == other.name + } +} +impl Eq for MacroMatcher {} + +impl<'de> Deserialize<'de> for MacroMatcher { + fn deserialize<D>(deser: D) -> Result<Self, D::Error> + where + D: de::Deserializer<'de>, + { + #[derive(Deserialize)] + #[serde(field_identifier, rename_all = "lowercase")] + enum Field { + Name, + Brace, + } + struct MacVisitor; + impl<'de> de::Visitor<'de> for MacVisitor { + type Value = MacroMatcher; + + fn expecting(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result { + formatter.write_str("struct MacroMatcher") + } + + fn visit_map<V>(self, mut map: V) -> Result<Self::Value, V::Error> + where + V: de::MapAccess<'de>, + { + let mut name = None; + let mut brace: Option<&str> = None; + while let Some(key) = map.next_key()? { + match key { + Field::Name => { + if name.is_some() { + return Err(de::Error::duplicate_field("name")); + } + name = Some(map.next_value()?); + }, + Field::Brace => { + if brace.is_some() { + return Err(de::Error::duplicate_field("brace")); + } + brace = Some(map.next_value()?); + }, + } + } + let name = name.ok_or_else(|| de::Error::missing_field("name"))?; + let brace = brace.ok_or_else(|| de::Error::missing_field("brace"))?; + Ok(MacroMatcher { + name, + braces: BRACES + .iter() + .find(|b| b.0 == brace) + .map(|(o, c)| ((*o).to_owned(), (*c).to_owned())) + .ok_or_else(|| { + de::Error::custom(&format!("expected one of `(`, `{{`, `[` found `{}`", brace)) + })?, + }) + } + } + + const FIELDS: &[&str] = &["name", "brace"]; + deser.deserialize_struct("MacroMatcher", FIELDS, MacVisitor) + } +} diff --git a/clippy_lints/src/ranges.rs b/clippy_lints/src/ranges.rs index ae5f0627fd6..b41c478c266 100644 --- a/clippy_lints/src/ranges.rs +++ b/clippy_lints/src/ranges.rs @@ -329,7 +329,7 @@ fn check_range_zip_with_len(cx: &LateContext<'_>, path: &PathSegment<'_>, args: if is_integer_const(cx, start, 0); // `.len()` call if let ExprKind::MethodCall(len_path, _, len_args, _) = end.kind; - if len_path.ident.name == sym!(len) && len_args.len() == 1; + if len_path.ident.name == sym::len && len_args.len() == 1; // `.iter()` and `.len()` called on same `Path` if let ExprKind::Path(QPath::Resolved(_, iter_path)) = iter_args[0].kind; if let ExprKind::Path(QPath::Resolved(_, len_path)) = len_args[0].kind; diff --git a/clippy_lints/src/redundant_clone.rs b/clippy_lints/src/redundant_clone.rs index 7dafce60d5e..119ec21520f 100644 --- a/clippy_lints/src/redundant_clone.rs +++ b/clippy_lints/src/redundant_clone.rs @@ -132,7 +132,7 @@ impl<'tcx> LateLintPass<'tcx> for RedundantClone { } } - // `{ cloned = &arg; clone(move cloned); }` or `{ cloned = &arg; to_path_buf(cloned); }` + // `{ arg = &cloned; clone(move arg); }` or `{ arg = &cloned; to_path_buf(arg); }` let (cloned, cannot_move_out) = unwrap_or_continue!(find_stmt_assigns_to(cx, mir, arg, from_borrow, bb)); let loc = mir::Location { diff --git a/clippy_lints/src/semicolon_if_nothing_returned.rs b/clippy_lints/src/semicolon_if_nothing_returned.rs index 16e4d73851f..da3e30af35c 100644 --- a/clippy_lints/src/semicolon_if_nothing_returned.rs +++ b/clippy_lints/src/semicolon_if_nothing_returned.rs @@ -1,3 +1,4 @@ +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}; @@ -45,6 +46,7 @@ impl LateLintPass<'_> for SemicolonIfNothingReturned { if t_expr.is_unit(); if let snippet = snippet_with_macro_callsite(cx, expr.span, "}"); if !snippet.ends_with('}'); + if cx.sess().source_map().is_multiline(block.span); then { // filter out the desugared `for` loop if let ExprKind::DropTemps(..) = &expr.kind { diff --git a/clippy_lints/src/suspicious_trait_impl.rs b/clippy_lints/src/suspicious_trait_impl.rs index 512abde11a6..2203ab57b10 100644 --- a/clippy_lints/src/suspicious_trait_impl.rs +++ b/clippy_lints/src/suspicious_trait_impl.rs @@ -26,7 +26,7 @@ declare_clippy_lint! { /// } /// ``` pub SUSPICIOUS_ARITHMETIC_IMPL, - correctness, + suspicious, "suspicious use of operators in impl of arithmetic trait" } @@ -47,7 +47,7 @@ declare_clippy_lint! { /// } /// ``` pub SUSPICIOUS_OP_ASSIGN_IMPL, - correctness, + suspicious, "suspicious use of operators in impl of OpAssign trait" } diff --git a/clippy_lints/src/unnested_or_patterns.rs b/clippy_lints/src/unnested_or_patterns.rs index 1b3c457b01a..07a4e294049 100644 --- a/clippy_lints/src/unnested_or_patterns.rs +++ b/clippy_lints/src/unnested_or_patterns.rs @@ -1,6 +1,6 @@ #![allow(clippy::wildcard_imports, clippy::enum_glob_use)] -use clippy_utils::ast_utils::{eq_field_pat, eq_id, eq_pat, eq_path, eq_maybe_qself}; +use clippy_utils::ast_utils::{eq_field_pat, eq_id, eq_maybe_qself, eq_pat, eq_path}; use clippy_utils::diagnostics::span_lint_and_then; use clippy_utils::{meets_msrv, msrvs, over}; use rustc_ast::mut_visit::*; @@ -277,7 +277,8 @@ fn transform_with_focus_on_idx(alternatives: &mut Vec<P<Pat>>, focus_idx: usize) ps1, start, alternatives, |k, ps1, idx| matches!( k, - TupleStruct(qself2, path2, ps2) if eq_maybe_qself(qself1, qself2) && eq_path(path1, path2) && eq_pre_post(ps1, ps2, idx) + TupleStruct(qself2, path2, ps2) + if eq_maybe_qself(qself1, qself2) && eq_path(path1, path2) && eq_pre_post(ps1, ps2, idx) ), |k| always_pat!(k, TupleStruct(_, _, ps) => ps), ), diff --git a/clippy_lints/src/use_self.rs b/clippy_lints/src/use_self.rs index 254b104bdef..906ac10f461 100644 --- a/clippy_lints/src/use_self.rs +++ b/clippy_lints/src/use_self.rs @@ -1,23 +1,22 @@ use clippy_utils::diagnostics::span_lint_and_sugg; -use clippy_utils::source::snippet_opt; use clippy_utils::ty::same_type_and_consts; use clippy_utils::{in_macro, meets_msrv, msrvs}; use if_chain::if_chain; +use rustc_data_structures::fx::FxHashSet; use rustc_errors::Applicability; use rustc_hir::{ self as hir, - def::{self, DefKind}, + def::{CtorOf, DefKind, Res}, def_id::LocalDefId, intravisit::{walk_ty, NestedVisitorMap, Visitor}, - Expr, ExprKind, FnRetTy, FnSig, GenericArg, HirId, Impl, ImplItemKind, Item, ItemKind, Node, Path, PathSegment, - QPath, TyKind, + Expr, ExprKind, FnRetTy, FnSig, GenericArg, HirId, Impl, ImplItemKind, Item, ItemKind, Path, QPath, TyKind, }; use rustc_lint::{LateContext, LateLintPass, LintContext}; use rustc_middle::hir::map::Map; -use rustc_middle::ty::{AssocKind, Ty}; +use rustc_middle::ty::AssocKind; use rustc_semver::RustcVersion; use rustc_session::{declare_tool_lint, impl_lint_pass}; -use rustc_span::{BytePos, Span}; +use rustc_span::Span; use rustc_typeck::hir_ty_to_ty; declare_clippy_lint! { @@ -75,10 +74,9 @@ impl UseSelf { #[derive(Debug)] enum StackItem { Check { - hir_id: HirId, - impl_trait_ref_def_id: Option<LocalDefId>, - types_to_skip: Vec<HirId>, - types_to_lint: Vec<HirId>, + impl_id: LocalDefId, + in_body: u32, + types_to_skip: FxHashSet<HirId>, }, NoCheck, } @@ -88,60 +86,41 @@ impl_lint_pass!(UseSelf => [USE_SELF]); const SEGMENTS_MSG: &str = "segments should be composed of at least 1 element"; impl<'tcx> LateLintPass<'tcx> for UseSelf { - fn check_item(&mut self, cx: &LateContext<'_>, item: &Item<'_>) { + fn check_item(&mut self, _cx: &LateContext<'_>, item: &Item<'_>) { + if !is_item_interesting(item) { + // This does two things: + // 1) Reduce needless churn on `self.stack` + // 2) Don't push `StackItem::NoCheck` when entering `ItemKind::OpaqueTy`, + // in order to lint `foo() -> impl <..>` + return; + } // We push the self types of `impl`s on a stack here. Only the top type on the stack is // relevant for linting, since this is the self type of the `impl` we're currently in. To // avoid linting on nested items, we push `StackItem::NoCheck` on the stack to signal, that // we're in an `impl` or nested item, that we don't want to lint - // - // NB: If you push something on the stack in this method, remember to also pop it in the - // `check_item_post` method. - match &item.kind { - ItemKind::Impl(Impl { - self_ty: hir_self_ty, - of_trait, - .. - }) => { - let should_check = if let TyKind::Path(QPath::Resolved(_, item_path)) = hir_self_ty.kind { - let parameters = &item_path.segments.last().expect(SEGMENTS_MSG).args; - parameters.as_ref().map_or(true, |params| { - !params.parenthesized && !params.args.iter().any(|arg| matches!(arg, GenericArg::Lifetime(_))) - }) - } else { - false - }; - let impl_trait_ref_def_id = of_trait.as_ref().map(|_| cx.tcx.hir().local_def_id(item.hir_id())); - if should_check { - self.stack.push(StackItem::Check { - hir_id: hir_self_ty.hir_id, - impl_trait_ref_def_id, - types_to_lint: Vec::new(), - types_to_skip: Vec::new(), - }); - } else { - self.stack.push(StackItem::NoCheck); + let stack_item = if_chain! { + if let ItemKind::Impl(Impl { self_ty, .. }) = item.kind; + if let TyKind::Path(QPath::Resolved(_, item_path)) = self_ty.kind; + let parameters = &item_path.segments.last().expect(SEGMENTS_MSG).args; + if parameters.as_ref().map_or(true, |params| { + !params.parenthesized && !params.args.iter().any(|arg| matches!(arg, GenericArg::Lifetime(_))) + }); + then { + StackItem::Check { + impl_id: item.def_id, + in_body: 0, + types_to_skip: std::iter::once(self_ty.hir_id).collect(), } - }, - ItemKind::Static(..) - | ItemKind::Const(..) - | ItemKind::Fn(..) - | ItemKind::Enum(..) - | ItemKind::Struct(..) - | ItemKind::Union(..) - | ItemKind::Trait(..) => { - self.stack.push(StackItem::NoCheck); - }, - _ => (), - } + } else { + StackItem::NoCheck + } + }; + self.stack.push(stack_item); } fn check_item_post(&mut self, _: &LateContext<'_>, item: &Item<'_>) { - use ItemKind::{Const, Enum, Fn, Impl, Static, Struct, Trait, Union}; - match item.kind { - Impl { .. } | Static(..) | Const(..) | Fn(..) | Enum(..) | Struct(..) | Union(..) | Trait(..) => { - self.stack.pop(); - }, - _ => (), + if is_item_interesting(item) { + self.stack.pop(); } } @@ -151,11 +130,11 @@ impl<'tcx> LateLintPass<'tcx> for UseSelf { if_chain! { if let ImplItemKind::Fn(FnSig { decl, .. }, ..) = impl_item.kind; if let Some(&mut StackItem::Check { - impl_trait_ref_def_id: Some(def_id), + impl_id, ref mut types_to_skip, .. }) = self.stack.last_mut(); - if let Some(impl_trait_ref) = cx.tcx.impl_trait_ref(def_id); + if let Some(impl_trait_ref) = cx.tcx.impl_trait_ref(impl_id); then { // `self_ty` is the semantic self type of `impl <trait> for <type>`. This cannot be // `Self`. @@ -203,142 +182,76 @@ impl<'tcx> LateLintPass<'tcx> for UseSelf { } } - fn check_body(&mut self, cx: &LateContext<'tcx>, body: &'tcx hir::Body<'_>) { + fn check_body(&mut self, _: &LateContext<'_>, _: &hir::Body<'_>) { // `hir_ty_to_ty` cannot be called in `Body`s or it will panic (sometimes). But in bodies // we can use `cx.typeck_results.node_type(..)` to get the `ty::Ty` from a `hir::Ty`. // However the `node_type()` method can *only* be called in bodies. - // - // This method implementation determines which types should get linted in a `Body` and - // which shouldn't, with a visitor. We could directly lint in the visitor, but then we - // could only allow this lint on item scope. And we would have to check if those types are - // already dealt with in `check_ty` anyway. - if let Some(StackItem::Check { - hir_id, - types_to_lint, - types_to_skip, - .. - }) = self.stack.last_mut() - { - let self_ty = ty_from_hir_id(cx, *hir_id); + if let Some(&mut StackItem::Check { ref mut in_body, .. }) = self.stack.last_mut() { + *in_body = in_body.saturating_add(1); + } + } - let mut visitor = LintTyCollector { - cx, - self_ty, - types_to_lint: vec![], - types_to_skip: vec![], - }; - visitor.visit_expr(&body.value); - types_to_lint.extend(visitor.types_to_lint); - types_to_skip.extend(visitor.types_to_skip); + fn check_body_post(&mut self, _: &LateContext<'_>, _: &hir::Body<'_>) { + if let Some(&mut StackItem::Check { ref mut in_body, .. }) = self.stack.last_mut() { + *in_body = in_body.saturating_sub(1); } } fn check_ty(&mut self, cx: &LateContext<'_>, hir_ty: &hir::Ty<'_>) { - if in_macro(hir_ty.span) - || in_impl(cx, hir_ty) - || !meets_msrv(self.msrv.as_ref(), &msrvs::TYPE_ALIAS_ENUM_VARIANTS) - { - return; - } - - let lint_dependend_on_expr_kind = if let Some(StackItem::Check { - hir_id, - types_to_lint, - types_to_skip, - .. - }) = self.stack.last() - { - if types_to_skip.contains(&hir_ty.hir_id) { - false - } else if types_to_lint.contains(&hir_ty.hir_id) { - true + if_chain! { + if !in_macro(hir_ty.span); + if meets_msrv(self.msrv.as_ref(), &msrvs::TYPE_ALIAS_ENUM_VARIANTS); + if let Some(&StackItem::Check { + impl_id, + in_body, + ref types_to_skip, + }) = self.stack.last(); + if let TyKind::Path(QPath::Resolved(_, path)) = hir_ty.kind; + if !matches!(path.res, Res::SelfTy(..) | Res::Def(DefKind::TyParam, _)); + if !types_to_skip.contains(&hir_ty.hir_id); + let ty = if in_body > 0 { + cx.typeck_results().node_type(hir_ty.hir_id) } else { - let self_ty = ty_from_hir_id(cx, *hir_id); - should_lint_ty(hir_ty, hir_ty_to_ty(cx.tcx, hir_ty), self_ty) - } - } else { - false - }; - - if lint_dependend_on_expr_kind { - // FIXME: this span manipulation should not be necessary - // @flip1995 found an ast lowering issue in - // https://github.com/rust-lang/rust/blob/master/src/librustc_ast_lowering/path.rs#l142-l162 + hir_ty_to_ty(cx.tcx, hir_ty) + }; + if same_type_and_consts(ty, cx.tcx.type_of(impl_id)); let hir = cx.tcx.hir(); let id = hir.get_parent_node(hir_ty.hir_id); - - if !hir.opt_span(id).map_or(false, in_macro) { - match hir.find(id) { - Some(Node::Expr(Expr { - kind: ExprKind::Path(QPath::TypeRelative(_, segment)), - .. - })) => span_lint_until_last_segment(cx, hir_ty.span, segment), - _ => span_lint(cx, hir_ty.span), - } + if !hir.opt_span(id).map_or(false, in_macro); + then { + span_lint(cx, hir_ty.span); } } } fn check_expr(&mut self, cx: &LateContext<'_>, expr: &Expr<'_>) { - fn expr_ty_matches(cx: &LateContext<'_>, expr: &Expr<'_>, self_ty: Ty<'_>) -> bool { - let def_id = expr.hir_id.owner; - if cx.tcx.has_typeck_results(def_id) { - cx.tcx.typeck(def_id).expr_ty_opt(expr) == Some(self_ty) - } else { - false - } + if_chain! { + if !in_macro(expr.span); + if meets_msrv(self.msrv.as_ref(), &msrvs::TYPE_ALIAS_ENUM_VARIANTS); + if let Some(&StackItem::Check { impl_id, .. }) = self.stack.last(); + if cx.typeck_results().expr_ty(expr) == cx.tcx.type_of(impl_id); + then {} else { return; } } - - if in_macro(expr.span) || !meets_msrv(self.msrv.as_ref(), &msrvs::TYPE_ALIAS_ENUM_VARIANTS) { - return; - } - - if let Some(StackItem::Check { hir_id, .. }) = self.stack.last() { - let self_ty = ty_from_hir_id(cx, *hir_id); - - match &expr.kind { - ExprKind::Struct(QPath::Resolved(_, path), ..) => { - if expr_ty_matches(cx, expr, self_ty) { - match path.res { - def::Res::SelfTy(..) => (), - def::Res::Def(DefKind::Variant, _) => span_lint_on_path_until_last_segment(cx, path), - _ => { - span_lint(cx, path.span); - }, + match expr.kind { + ExprKind::Struct(QPath::Resolved(_, path), ..) => match path.res { + Res::SelfTy(..) => (), + Res::Def(DefKind::Variant, _) => lint_path_to_variant(cx, path), + _ => span_lint(cx, path.span), + }, + // tuple struct instantiation (`Foo(arg)` or `Enum::Foo(arg)`) + ExprKind::Call(fun, _) => { + if let ExprKind::Path(QPath::Resolved(_, path)) = fun.kind { + if let Res::Def(DefKind::Ctor(ctor_of, _), ..) = path.res { + match ctor_of { + CtorOf::Variant => lint_path_to_variant(cx, path), + CtorOf::Struct => span_lint(cx, path.span), } } - }, - // tuple struct instantiation (`Foo(arg)` or `Enum::Foo(arg)`) - ExprKind::Call(fun, _) => { - if let Expr { - kind: ExprKind::Path(ref qpath), - .. - } = fun - { - if expr_ty_matches(cx, expr, self_ty) { - let res = cx.qpath_res(qpath, fun.hir_id); - - if let def::Res::Def(DefKind::Ctor(ctor_of, _), ..) = res { - match ctor_of { - def::CtorOf::Variant => { - span_lint_on_qpath_resolved(cx, qpath, true); - }, - def::CtorOf::Struct => { - span_lint_on_qpath_resolved(cx, qpath, false); - }, - } - } - } - } - }, - // unit enum variants (`Enum::A`) - ExprKind::Path(qpath) => { - if expr_ty_matches(cx, expr, self_ty) { - span_lint_on_qpath_resolved(cx, qpath, true); - } - }, - _ => (), - } + } + }, + // unit enum variants (`Enum::A`) + ExprKind::Path(QPath::Resolved(_, path)) => lint_path_to_variant(cx, path), + _ => (), } } @@ -364,35 +277,6 @@ impl<'tcx> Visitor<'tcx> for SkipTyCollector { } } -struct LintTyCollector<'a, 'tcx> { - cx: &'a LateContext<'tcx>, - self_ty: Ty<'tcx>, - types_to_lint: Vec<HirId>, - types_to_skip: Vec<HirId>, -} - -impl<'a, 'tcx> Visitor<'tcx> for LintTyCollector<'a, 'tcx> { - type Map = Map<'tcx>; - - fn visit_ty(&mut self, hir_ty: &'tcx hir::Ty<'_>) { - if_chain! { - if let Some(ty) = self.cx.typeck_results().node_type_opt(hir_ty.hir_id); - if should_lint_ty(hir_ty, ty, self.self_ty); - then { - self.types_to_lint.push(hir_ty.hir_id); - } else { - self.types_to_skip.push(hir_ty.hir_id); - } - } - - walk_ty(self, hir_ty); - } - - fn nested_visit_map(&mut self) -> NestedVisitorMap<Self::Map> { - NestedVisitorMap::None - } -} - fn span_lint(cx: &LateContext<'_>, span: Span) { span_lint_and_sugg( cx, @@ -405,66 +289,19 @@ fn span_lint(cx: &LateContext<'_>, span: Span) { ); } -#[allow(clippy::cast_possible_truncation)] -fn span_lint_until_last_segment(cx: &LateContext<'_>, span: Span, segment: &PathSegment<'_>) { - let sp = span.with_hi(segment.ident.span.lo()); - // remove the trailing :: - let span_without_last_segment = match snippet_opt(cx, sp) { - Some(snippet) => match snippet.rfind("::") { - Some(bidx) => sp.with_hi(sp.lo() + BytePos(bidx as u32)), - None => sp, - }, - None => sp, - }; - span_lint(cx, span_without_last_segment); -} - -fn span_lint_on_path_until_last_segment(cx: &LateContext<'_>, path: &Path<'_>) { - if path.segments.len() > 1 { - span_lint_until_last_segment(cx, path.span, path.segments.last().unwrap()); +fn lint_path_to_variant(cx: &LateContext<'_>, path: &Path<'_>) { + if let [.., self_seg, _variant] = path.segments { + let span = path + .span + .with_hi(self_seg.args().span_ext().unwrap_or(self_seg.ident.span).hi()); + span_lint(cx, span); } } -fn span_lint_on_qpath_resolved(cx: &LateContext<'_>, qpath: &QPath<'_>, until_last_segment: bool) { - if let QPath::Resolved(_, path) = qpath { - if until_last_segment { - span_lint_on_path_until_last_segment(cx, path); - } else { - span_lint(cx, path.span); - } - } -} - -fn ty_from_hir_id<'tcx>(cx: &LateContext<'tcx>, hir_id: HirId) -> Ty<'tcx> { - if let Some(Node::Ty(hir_ty)) = cx.tcx.hir().find(hir_id) { - hir_ty_to_ty(cx.tcx, hir_ty) - } else { - unreachable!("This function should only be called with `HirId`s that are for sure `Node::Ty`") - } -} - -fn in_impl(cx: &LateContext<'tcx>, hir_ty: &hir::Ty<'_>) -> bool { - let map = cx.tcx.hir(); - let parent = map.get_parent_node(hir_ty.hir_id); - if_chain! { - if let Some(Node::Item(item)) = map.find(parent); - if let ItemKind::Impl { .. } = item.kind; - then { - true - } else { - false - } - } -} - -fn should_lint_ty(hir_ty: &hir::Ty<'_>, ty: Ty<'_>, self_ty: Ty<'_>) -> bool { - if_chain! { - if same_type_and_consts(ty, self_ty); - if let TyKind::Path(QPath::Resolved(_, path)) = hir_ty.kind; - then { - !matches!(path.res, def::Res::SelfTy(..)) - } else { - false - } - } +fn is_item_interesting(item: &Item<'_>) -> bool { + use rustc_hir::ItemKind::{Const, Enum, Fn, Impl, Static, Struct, Trait, Union}; + matches!( + item.kind, + Impl { .. } | Static(..) | Const(..) | Fn(..) | Enum(..) | Struct(..) | Union(..) | Trait(..) + ) } diff --git a/clippy_lints/src/useless_conversion.rs b/clippy_lints/src/useless_conversion.rs index 2be99fb761b..c97f7e1626e 100644 --- a/clippy_lints/src/useless_conversion.rs +++ b/clippy_lints/src/useless_conversion.rs @@ -104,7 +104,7 @@ impl<'tcx> LateLintPass<'tcx> for UselessConversion { } } if_chain! { - if match_trait_method(cx, e, &paths::TRY_INTO_TRAIT) && &*name.ident.as_str() == "try_into"; + if match_trait_method(cx, e, &paths::TRY_INTO_TRAIT) && name.ident.name == sym::try_into; let a = cx.typeck_results().expr_ty(e); let b = cx.typeck_results().expr_ty(&args[0]); if is_type_diagnostic_item(cx, a, sym::result_type); diff --git a/clippy_lints/src/utils/conf.rs b/clippy_lints/src/utils/conf.rs index 0e33ae740d9..44d3d456342 100644 --- a/clippy_lints/src/utils/conf.rs +++ b/clippy_lints/src/utils/conf.rs @@ -8,6 +8,13 @@ use std::error::Error; use std::path::{Path, PathBuf}; use std::{env, fmt, fs, io}; +/// Holds information used by `MISSING_ENFORCED_IMPORT_RENAMES` lint. +#[derive(Clone, Debug, Deserialize)] +pub struct Rename { + pub path: String, + pub rename: String, +} + /// Conf with parse errors #[derive(Default)] pub struct TryConf { @@ -24,6 +31,9 @@ impl TryConf { } } +/// Note that the configuration parsing currently doesn't support documentation that will +/// that spans over several lines. This will be possible with the new implementation +/// See (rust-clippy#7172) macro_rules! define_Conf { ($( #[doc = $doc:literal] @@ -149,7 +159,7 @@ define_Conf! { "WebGL", "TensorFlow", "TrueType", - "iOS", "macOS", + "iOS", "macOS", "FreeBSD", "TeX", "LaTeX", "BibTeX", "BibLaTeX", "MinGW", "CamelCase", @@ -182,20 +192,28 @@ define_Conf! { (vec_box_size_threshold: u64 = 4096), /// Lint: TYPE_REPETITION_IN_BOUNDS. The maximum number of bounds a trait can have to be linted (max_trait_bounds: u64 = 3), - /// Lint: STRUCT_EXCESSIVE_BOOLS. The maximum number of bools a struct can have + /// Lint: STRUCT_EXCESSIVE_BOOLS. The maximum number of bool fields a struct can have (max_struct_bools: u64 = 3), - /// Lint: FN_PARAMS_EXCESSIVE_BOOLS. The maximum number of bools function parameters can have + /// Lint: FN_PARAMS_EXCESSIVE_BOOLS. The maximum number of bool parameters a function can have (max_fn_params_bools: u64 = 3), /// Lint: WILDCARD_IMPORTS. Whether to allow certain wildcard imports (prelude, super in tests). (warn_on_all_wildcard_imports: bool = false), /// Lint: DISALLOWED_METHOD. The list of disallowed methods, written as fully qualified paths. (disallowed_methods: Vec<String> = Vec::new()), + /// Lint: DISALLOWED_TYPE. The list of disallowed types, written as fully qualified paths. + (disallowed_types: Vec<String> = Vec::new()), /// Lint: UNREADABLE_LITERAL. Should the fraction of a decimal be linted to include separators. (unreadable_literal_lint_fractions: bool = true), /// Lint: UPPER_CASE_ACRONYMS. Enables verbose mode. Triggers if there is more than one uppercase char next to each other (upper_case_acronyms_aggressive: bool = false), /// Lint: _CARGO_COMMON_METADATA. For internal testing only, ignores the current `publish` settings in the Cargo manifest. (cargo_ignore_publish: bool = false), + /// Lint: NONSTANDARD_MACRO_BRACES. Enforce the named macros always use the braces specified. <br> A `MacroMatcher` can be added like so `{ name = "macro_name", brace = "(" }`. If the macro is could be used with a full path two `MacroMatcher`s have to be added one with the full path `crate_name::macro_name` and one with just the macro name. + (standard_macro_braces: Vec<crate::nonstandard_macro_braces::MacroMatcher> = Vec::new()), + /// Lint: MISSING_ENFORCED_IMPORT_RENAMES. The list of imports to always rename, a fully qualified path followed by the rename. + (enforced_import_renames: Vec<crate::utils::conf::Rename> = Vec::new()), + /// Lint: RESTRICTED_SCRIPTS. The list of unicode scripts allowed to be used in the scope. + (allowed_scripts: Vec<String> = vec!["Latin".to_string()]), } /// Search for the configuration file. diff --git a/clippy_lints/src/utils/internal_lints/metadata_collector.rs b/clippy_lints/src/utils/internal_lints/metadata_collector.rs index 46af03663b8..e877af09e28 100644 --- a/clippy_lints/src/utils/internal_lints/metadata_collector.rs +++ b/clippy_lints/src/utils/internal_lints/metadata_collector.rs @@ -9,6 +9,7 @@ //! a simple mistake) use if_chain::if_chain; +use rustc_ast as ast; use rustc_data_structures::fx::FxHashMap; use rustc_hir::{ self as hir, def::DefKind, intravisit, intravisit::Visitor, ExprKind, Item, ItemKind, Mutability, QPath, @@ -46,8 +47,9 @@ const DEPRECATED_LINT_GROUP_STR: &str = "deprecated"; const DEPRECATED_LINT_LEVEL: &str = "none"; /// This array holds Clippy's lint groups with their corresponding default lint level. The /// lint level for deprecated lints is set in `DEPRECATED_LINT_LEVEL`. -const DEFAULT_LINT_LEVELS: [(&str, &str); 8] = [ +const DEFAULT_LINT_LEVELS: &[(&str, &str)] = &[ ("correctness", "deny"), + ("suspicious", "warn"), ("restriction", "allow"), ("style", "warn"), ("pedantic", "allow"), @@ -485,16 +487,32 @@ fn extract_attr_docs_or_lint(cx: &LateContext<'_>, item: &Item<'_>) -> Option<St /// /// Would result in `Hello world!\n=^.^=\n` fn extract_attr_docs(cx: &LateContext<'_>, item: &Item<'_>) -> Option<String> { - cx.tcx - .hir() - .attrs(item.hir_id()) - .iter() - .filter_map(|x| x.doc_str().map(|sym| sym.as_str().to_string())) - .reduce(|mut acc, sym| { - acc.push_str(&sym); - acc.push('\n'); - acc - }) + let attrs = cx.tcx.hir().attrs(item.hir_id()); + let mut lines = attrs.iter().filter_map(ast::Attribute::doc_str); + let mut docs = String::from(&*lines.next()?.as_str()); + let mut in_code_block = false; + for line in lines { + docs.push('\n'); + let line = line.as_str(); + let line = &*line; + if let Some(info) = line.trim_start().strip_prefix("```") { + in_code_block = !in_code_block; + if in_code_block { + let lang = info + .trim() + .split(',') + // remove rustdoc directives + .find(|&s| !matches!(s, "" | "ignore" | "no_run" | "should_panic")) + // if no language is present, fill in "rust" + .unwrap_or("rust"); + docs.push_str("```"); + docs.push_str(lang); + continue; + } + } + docs.push_str(line); + } + Some(docs) } fn get_lint_group_and_level_or_lint( diff --git a/clippy_lints/src/wildcard_imports.rs b/clippy_lints/src/wildcard_imports.rs index 51c1117d206..520586b3a1f 100644 --- a/clippy_lints/src/wildcard_imports.rs +++ b/clippy_lints/src/wildcard_imports.rs @@ -1,6 +1,6 @@ use clippy_utils::diagnostics::span_lint_and_sugg; -use clippy_utils::in_macro; use clippy_utils::source::{snippet, snippet_with_applicability}; +use clippy_utils::{in_macro, is_test_module_or_function}; use if_chain::if_chain; use rustc_errors::Applicability; use rustc_hir::{ @@ -106,7 +106,7 @@ impl_lint_pass!(WildcardImports => [ENUM_GLOB_USE, WILDCARD_IMPORTS]); impl LateLintPass<'_> for WildcardImports { fn check_item(&mut self, cx: &LateContext<'_>, item: &Item<'_>) { - if is_test_module_or_function(item) { + if is_test_module_or_function(cx.tcx, item) { self.test_modules_deep = self.test_modules_deep.saturating_add(1); } if item.vis.node.is_pub() || item.vis.node.is_pub_restricted() { @@ -183,8 +183,8 @@ impl LateLintPass<'_> for WildcardImports { } } - fn check_item_post(&mut self, _: &LateContext<'_>, item: &Item<'_>) { - if is_test_module_or_function(item) { + fn check_item_post(&mut self, cx: &LateContext<'_>, item: &Item<'_>) { + if is_test_module_or_function(cx.tcx, item) { self.test_modules_deep = self.test_modules_deep.saturating_sub(1); } } @@ -208,7 +208,3 @@ fn is_prelude_import(segments: &[PathSegment<'_>]) -> bool { fn is_super_only_import(segments: &[PathSegment<'_>]) -> bool { segments.len() == 1 && segments[0].ident.name == kw::Super } - -fn is_test_module_or_function(item: &Item<'_>) -> bool { - matches!(item.kind, ItemKind::Mod(..)) && item.ident.name.as_str().contains("test") -} diff --git a/clippy_utils/Cargo.toml b/clippy_utils/Cargo.toml index 93ed3b18400..6ede9011208 100644 --- a/clippy_utils/Cargo.toml +++ b/clippy_utils/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "clippy_utils" -version = "0.1.54" +version = "0.1.55" authors = ["The Rust Clippy Developers"] edition = "2018" publish = false diff --git a/clippy_utils/src/ast_utils.rs b/clippy_utils/src/ast_utils.rs index 90c034bd02a..30c2260d15c 100644 --- a/clippy_utils/src/ast_utils.rs +++ b/clippy_utils/src/ast_utils.rs @@ -47,9 +47,14 @@ pub fn eq_pat(l: &Pat, r: &Pat) -> bool { | (Ref(l, Mutability::Mut), Ref(r, Mutability::Mut)) => eq_pat(l, r), (Tuple(l), Tuple(r)) | (Slice(l), Slice(r)) => over(l, r, |l, r| eq_pat(l, r)), (Path(lq, lp), Path(rq, rp)) => both(lq, rq, |l, r| eq_qself(l, r)) && eq_path(lp, rp), - (TupleStruct(lqself, lp, lfs), TupleStruct(rqself, rp, rfs)) => eq_maybe_qself(lqself, rqself) && eq_path(lp, rp) && over(lfs, rfs, |l, r| eq_pat(l, r)), + (TupleStruct(lqself, lp, lfs), TupleStruct(rqself, rp, rfs)) => { + eq_maybe_qself(lqself, rqself) && eq_path(lp, rp) && over(lfs, rfs, |l, r| eq_pat(l, r)) + }, (Struct(lqself, lp, lfs, lr), Struct(rqself, rp, rfs, rr)) => { - lr == rr && eq_maybe_qself(lqself, rqself) &&eq_path(lp, rp) && unordered_over(lfs, rfs, |lf, rf| eq_field_pat(lf, rf)) + lr == rr + && eq_maybe_qself(lqself, rqself) + && eq_path(lp, rp) + && unordered_over(lfs, rfs, |lf, rf| eq_field_pat(lf, rf)) }, (Or(ls), Or(rs)) => unordered_over(ls, rs, |l, r| eq_pat(l, r)), (MacCall(l), MacCall(r)) => eq_mac_call(l, r), @@ -82,7 +87,7 @@ pub fn eq_maybe_qself(l: &Option<QSelf>, r: &Option<QSelf>) -> bool { match (l, r) { (Some(l), Some(r)) => eq_qself(l, r), (None, None) => true, - _ => false + _ => false, } } diff --git a/clippy_utils/src/attrs.rs b/clippy_utils/src/attrs.rs index 0318c483959..c19b558cd8c 100644 --- a/clippy_utils/src/attrs.rs +++ b/clippy_utils/src/attrs.rs @@ -157,3 +157,8 @@ pub fn is_doc_hidden(attrs: &[ast::Attribute]) -> bool { .filter_map(ast::Attribute::meta_item_list) .any(|l| attr::list_contains_name(&l, sym::hidden)) } + +/// Return true if the attributes contain `#[unstable]` +pub fn is_unstable(attrs: &[ast::Attribute]) -> bool { + attrs.iter().any(|attr| attr.has_name(sym::unstable)) +} diff --git a/clippy_utils/src/consts.rs b/clippy_utils/src/consts.rs index 0d7fdeeb920..15c27d1a996 100644 --- a/clippy_utils/src/consts.rs +++ b/clippy_utils/src/consts.rs @@ -1,6 +1,6 @@ #![allow(clippy::float_cmp)] -use crate::{clip, sext, unsext}; +use crate::{clip, is_direct_expn_of, sext, unsext}; use if_chain::if_chain; use rustc_ast::ast::{self, LitFloatType, LitKind}; use rustc_data_structures::sync::Lrc; @@ -230,7 +230,13 @@ impl<'a, 'tcx> ConstEvalLateContext<'a, 'tcx> { match e.kind { ExprKind::Path(ref qpath) => self.fetch_path(qpath, e.hir_id, self.typeck_results.expr_ty(e)), ExprKind::Block(block, _) => self.block(block), - ExprKind::Lit(ref lit) => Some(lit_to_constant(&lit.node, self.typeck_results.expr_ty_opt(e))), + ExprKind::Lit(ref lit) => { + if is_direct_expn_of(e.span, "cfg").is_some() { + None + } else { + Some(lit_to_constant(&lit.node, self.typeck_results.expr_ty_opt(e))) + } + }, ExprKind::Array(vec) => self.multi(vec).map(Constant::Vec), ExprKind::Tup(tup) => self.multi(tup).map(Constant::Tuple), ExprKind::Repeat(value, _) => { diff --git a/clippy_utils/src/lib.rs b/clippy_utils/src/lib.rs index 9dcfa66c4ef..217a1f4dded 100644 --- a/clippy_utils/src/lib.rs +++ b/clippy_utils/src/lib.rs @@ -72,7 +72,7 @@ use rustc_hir::LangItem::{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, Node, Param, Pat, PatKind, Path, - PathSegment, QPath, Stmt, StmtKind, TraitItem, TraitItemKind, TraitRef, TyKind, + PathSegment, QPath, Stmt, StmtKind, TraitItem, TraitItemKind, TraitRef, TyKind, UnOp, }; use rustc_lint::{LateContext, Level, Lint, LintContext}; use rustc_middle::hir::exports::Export; @@ -326,16 +326,6 @@ pub fn is_trait_method(cx: &LateContext<'_>, expr: &Expr<'_>, diag_item: Symbol) .map_or(false, |did| is_diag_trait_item(cx, did, diag_item)) } -/// Checks if an expression references a variable of the given name. -pub fn match_var(expr: &Expr<'_>, var: Symbol) -> bool { - if let ExprKind::Path(QPath::Resolved(None, path)) = expr.kind { - if let [p] = path.segments { - return p.ident.name == var; - } - } - false -} - pub fn last_path_segment<'tcx>(path: &QPath<'tcx>) -> &'tcx PathSegment<'tcx> { match *path { QPath::Resolved(_, path) => path.segments.last().expect("A path must have at least one segment"), @@ -707,16 +697,6 @@ pub fn get_item_name(cx: &LateContext<'_>, expr: &Expr<'_>) -> Option<Symbol> { } } -/// Gets the name of a `Pat`, if any. -pub fn get_pat_name(pat: &Pat<'_>) -> Option<Symbol> { - match pat.kind { - PatKind::Binding(.., ref spname, _) => Some(spname.name), - PatKind::Path(ref qpath) => single_segment_path(qpath).map(|ps| ps.ident.name), - PatKind::Box(p) | PatKind::Ref(p, _) => get_pat_name(&*p), - _ => None, - } -} - pub struct ContainsName { pub name: Symbol, pub result: bool, @@ -861,14 +841,16 @@ pub fn get_enclosing_block<'tcx>(cx: &LateContext<'tcx>, hir_id: HirId) -> Optio }) } -/// Gets the loop enclosing the given expression, if any. -pub fn get_enclosing_loop(tcx: TyCtxt<'tcx>, expr: &Expr<'_>) -> Option<&'tcx Expr<'tcx>> { +/// Gets the loop or closure enclosing the given expression, if any. +pub fn get_enclosing_loop_or_closure(tcx: TyCtxt<'tcx>, expr: &Expr<'_>) -> Option<&'tcx Expr<'tcx>> { let map = tcx.hir(); for (_, node) in map.parent_iter(expr.hir_id) { match node { Node::Expr( - e @ Expr { - kind: ExprKind::Loop(..), + e + @ + Expr { + kind: ExprKind::Loop(..) | ExprKind::Closure(..), .. }, ) => return Some(e), @@ -1399,6 +1381,55 @@ pub fn is_must_use_func_call(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool { did.map_or(false, |did| must_use_attr(cx.tcx.get_attrs(did)).is_some()) } +/// Checks if an expression represents the identity function +/// Only examines closures and `std::convert::identity` +pub fn is_expr_identity_function(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool { + /// Checks if a function's body represents the identity function. Looks for bodies of the form: + /// * `|x| x` + /// * `|x| return x` + /// * `|x| { return x }` + /// * `|x| { return x; }` + fn is_body_identity_function(cx: &LateContext<'_>, func: &Body<'_>) -> bool { + let id = if_chain! { + if let [param] = func.params; + if let PatKind::Binding(_, id, _, _) = param.pat.kind; + then { + id + } else { + return false; + } + }; + + let mut expr = &func.value; + loop { + match expr.kind { + #[rustfmt::skip] + ExprKind::Block(&Block { stmts: [], expr: Some(e), .. }, _, ) + | ExprKind::Ret(Some(e)) => expr = e, + #[rustfmt::skip] + ExprKind::Block(&Block { stmts: [stmt], expr: None, .. }, _) => { + if_chain! { + if let StmtKind::Semi(e) | StmtKind::Expr(e) = stmt.kind; + if let ExprKind::Ret(Some(ret_val)) = e.kind; + then { + expr = ret_val; + } else { + return false; + } + } + }, + _ => return path_to_local_id(expr, id) && cx.typeck_results().expr_adjustments(expr).is_empty(), + } + } + } + + match expr.kind { + ExprKind::Closure(_, _, body_id, _, _) => is_body_identity_function(cx, cx.tcx.hir().body(body_id)), + ExprKind::Path(ref path) => is_qpath_def_path(cx, path, expr.hir_id, &paths::CONVERT_IDENTITY), + _ => false, + } +} + /// Gets the node where an expression is either used, or it's type is unified with another branch. pub fn get_expr_use_or_unification_node(tcx: TyCtxt<'tcx>, expr: &Expr<'_>) -> Option<Node<'tcx>> { let map = tcx.hir(); @@ -1654,6 +1685,19 @@ pub fn peel_hir_expr_refs(expr: &'a Expr<'a>) -> (&'a Expr<'a>, usize) { (e, count) } +/// Removes `AddrOf` operators (`&`) or deref operators (`*`), but only if a reference type is +/// dereferenced. An overloaded deref such as `Vec` to slice would not be removed. +pub fn peel_ref_operators<'hir>(cx: &LateContext<'_>, mut expr: &'hir Expr<'hir>) -> &'hir Expr<'hir> { + loop { + match expr.kind { + ExprKind::AddrOf(_, _, e) => expr = e, + ExprKind::Unary(UnOp::Deref, e) if cx.typeck_results().expr_ty(e).is_ref() => expr = e, + _ => break, + } + } + expr +} + #[macro_export] macro_rules! unwrap_cargo_metadata { ($cx: ident, $lint: ident, $deps: expr) => {{ @@ -1683,3 +1727,15 @@ pub fn is_hir_ty_cfg_dependant(cx: &LateContext<'_>, ty: &hir::Ty<'_>) -> bool { } } } + +/// 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; + } + } + + matches!(item.kind, ItemKind::Mod(..)) && item.ident.name.as_str().contains("test") +} diff --git a/clippy_utils/src/ptr.rs b/clippy_utils/src/ptr.rs index 791688cd194..8adb6915952 100644 --- a/clippy_utils/src/ptr.rs +++ b/clippy_utils/src/ptr.rs @@ -1,10 +1,10 @@ use crate::source::snippet; -use crate::{get_pat_name, match_var}; +use crate::{path_to_local_id, strip_pat_refs}; use rustc_hir::intravisit::{walk_expr, NestedVisitorMap, Visitor}; -use rustc_hir::{Body, BodyId, Expr, ExprKind, Param}; +use rustc_hir::{Body, BodyId, Expr, ExprKind, HirId, PatKind}; use rustc_lint::LateContext; use rustc_middle::hir::map::Map; -use rustc_span::{Span, Symbol}; +use rustc_span::Span; use std::borrow::Cow; pub fn get_spans( @@ -14,10 +14,11 @@ pub fn get_spans( replacements: &[(&'static str, &'static str)], ) -> Option<Vec<(Span, Cow<'static, str>)>> { if let Some(body) = opt_body_id.map(|id| cx.tcx.hir().body(id)) { - get_binding_name(&body.params[idx]).map_or_else( - || Some(vec![]), - |name| extract_clone_suggestions(cx, name, replacements, body), - ) + if let PatKind::Binding(_, binding_id, _, _) = strip_pat_refs(body.params[idx].pat).kind { + extract_clone_suggestions(cx, binding_id, replacements, body) + } else { + Some(vec![]) + } } else { Some(vec![]) } @@ -25,13 +26,13 @@ pub fn get_spans( fn extract_clone_suggestions<'tcx>( cx: &LateContext<'tcx>, - name: Symbol, + id: HirId, replace: &[(&'static str, &'static str)], body: &'tcx Body<'_>, ) -> Option<Vec<(Span, Cow<'static, str>)>> { let mut visitor = PtrCloneVisitor { cx, - name, + id, replace, spans: vec![], abort: false, @@ -42,7 +43,7 @@ fn extract_clone_suggestions<'tcx>( struct PtrCloneVisitor<'a, 'tcx> { cx: &'a LateContext<'tcx>, - name: Symbol, + id: HirId, replace: &'a [(&'static str, &'static str)], spans: Vec<(Span, Cow<'static, str>)>, abort: bool, @@ -55,16 +56,15 @@ impl<'a, 'tcx> Visitor<'tcx> for PtrCloneVisitor<'a, 'tcx> { if self.abort { return; } - if let ExprKind::MethodCall(seg, _, args, _) = expr.kind { - if args.len() == 1 && match_var(&args[0], self.name) { + if let ExprKind::MethodCall(seg, _, [recv], _) = expr.kind { + if path_to_local_id(recv, self.id) { if seg.ident.name.as_str() == "capacity" { self.abort = true; return; } for &(fn_name, suffix) in self.replace { if seg.ident.name.as_str() == fn_name { - self.spans - .push((expr.span, snippet(self.cx, args[0].span, "_") + suffix)); + self.spans.push((expr.span, snippet(self.cx, recv.span, "_") + suffix)); return; } } @@ -77,7 +77,3 @@ impl<'a, 'tcx> Visitor<'tcx> for PtrCloneVisitor<'a, 'tcx> { NestedVisitorMap::None } } - -fn get_binding_name(arg: &Param<'_>) -> Option<Symbol> { - get_pat_name(arg.pat) -} diff --git a/doc/basics.md b/doc/basics.md index ed3a2fff83f..e98354358af 100644 --- a/doc/basics.md +++ b/doc/basics.md @@ -90,8 +90,10 @@ cargo dev fmt cargo dev update_lints # create a new lint and register it cargo dev new_lint +# automatically formatting all code before each commit +cargo dev setup git-hook # (experimental) Setup Clippy to work with IntelliJ-Rust -cargo dev ide_setup +cargo dev setup intellij ``` ## lintcheck diff --git a/doc/common_tools_writing_lints.md b/doc/common_tools_writing_lints.md index abac1227b4f..0a85f650011 100644 --- a/doc/common_tools_writing_lints.md +++ b/doc/common_tools_writing_lints.md @@ -6,7 +6,7 @@ You may need following tooltips to catch up with common operations. - [Retrieving the type of an expression](#retrieving-the-type-of-an-expression) - [Checking if an expression is calling a specific method](#checking-if-an-expr-is-calling-a-specific-method) - [Checking if a type implements a specific trait](#checking-if-a-type-implements-a-specific-trait) - - [Checking if a type defines a method](#checking-if-a-type-defines-a-method) + - [Checking if a type defines a specific method](#checking-if-a-type-defines-a-specific-method) - [Dealing with macros](#dealing-with-macros) Useful Rustc dev guide links: diff --git a/doc/release.md b/doc/release.md index e0af9bf0625..afe3033c288 100644 --- a/doc/release.md +++ b/doc/release.md @@ -101,6 +101,21 @@ After this, the release should be available on the Clippy [release page]. [release page]: https://github.com/rust-lang/rust-clippy/releases +## Update the `stable` branch + +At this step you should have already checked out the commit of the `rust-1.XX.0` +tag. Updating the stable branch from here is as easy as: + +```bash +# Assuming the current directory corresponds to the Clippy repository and the +# commit of the just created rust-1.XX.0 tag is checked out. +$ git push upstream rust-1.XX.0:stable # `upstream` is the `rust-lang/rust-clippy` remote +``` + +_NOTE: Usually there are no stable backports for Clippy, so this update should +be possible without force pushing or anything like this. If there should have +happened a stable backport, make sure to re-merge those changes just as with the +`beta` branch._ ## Update `CHANGELOG.md` diff --git a/lintcheck/README.md b/lintcheck/README.md index a61070d8a80..8c169506e53 100644 --- a/lintcheck/README.md +++ b/lintcheck/README.md @@ -69,7 +69,7 @@ is checked. is explicitly specified in the options. ### Fix mode -You can run `./lintcheck/target/debug/lintcheck --fix` which will run Clippy with `-Zunstable-options --fix` and +You can run `./lintcheck/target/debug/lintcheck --fix` which will run Clippy with `--fix` and print a warning if Clippys suggestions fail to apply (if the resulting code does not build). This lets us spot bad suggestions or false positives automatically in some cases. diff --git a/lintcheck/src/main.rs b/lintcheck/src/main.rs index f6a75595c98..7d2f3173fb0 100644 --- a/lintcheck/src/main.rs +++ b/lintcheck/src/main.rs @@ -260,14 +260,7 @@ impl Crate { let shared_target_dir = clippy_project_root().join("target/lintcheck/shared_target_dir"); let mut args = if fix { - vec![ - "-Zunstable-options", - "--fix", - "-Zunstable-options", - "--allow-no-vcs", - "--", - "--cap-lints=warn", - ] + vec!["--fix", "--allow-no-vcs", "--", "--cap-lints=warn"] } else { vec!["--", "--message-format=json", "--", "--cap-lints=warn"] }; diff --git a/rust-toolchain b/rust-toolchain index e3863c46288..2d3c65c1d39 100644 --- a/rust-toolchain +++ b/rust-toolchain @@ -1,3 +1,3 @@ [toolchain] -channel = "nightly-2021-06-03" +channel = "nightly-2021-07-01" components = ["llvm-tools-preview", "rustc-dev", "rust-src"] diff --git a/src/main.rs b/src/main.rs index 7bb80b1196e..6bd4123ddeb 100644 --- a/src/main.rs +++ b/src/main.rs @@ -70,7 +70,6 @@ impl ClippyCmd { I: Iterator<Item = String>, { let mut cargo_subcommand = "check"; - let mut unstable_options = false; let mut args = vec![]; for arg in old_args.by_ref() { @@ -80,18 +79,12 @@ impl ClippyCmd { continue; }, "--" => break, - // Cover -Zunstable-options and -Z unstable-options - s if s.ends_with("unstable-options") => unstable_options = true, _ => {}, } args.push(arg); } - if cargo_subcommand == "fix" && !unstable_options { - panic!("Usage of `--fix` requires `-Z unstable-options`"); - } - let mut clippy_args: Vec<String> = old_args.collect(); if cargo_subcommand == "fix" && !clippy_args.iter().any(|arg| arg == "--no-deps") { clippy_args.push("--no-deps".into()); @@ -176,34 +169,23 @@ mod tests { use super::ClippyCmd; #[test] - #[should_panic] - fn fix_without_unstable() { + fn fix() { let args = "cargo clippy --fix".split_whitespace().map(ToString::to_string); - ClippyCmd::new(args); - } - - #[test] - fn fix_unstable() { - let args = "cargo clippy --fix -Zunstable-options" - .split_whitespace() - .map(ToString::to_string); let cmd = ClippyCmd::new(args); assert_eq!("fix", cmd.cargo_subcommand); - assert!(cmd.args.iter().any(|arg| arg.ends_with("unstable-options"))); + assert!(!cmd.args.iter().any(|arg| arg.ends_with("unstable-options"))); } #[test] fn fix_implies_no_deps() { - let args = "cargo clippy --fix -Zunstable-options" - .split_whitespace() - .map(ToString::to_string); + let args = "cargo clippy --fix".split_whitespace().map(ToString::to_string); let cmd = ClippyCmd::new(args); assert!(cmd.clippy_args.iter().any(|arg| arg == "--no-deps")); } #[test] fn no_deps_not_duplicated_with_fix() { - let args = "cargo clippy --fix -Zunstable-options -- --no-deps" + let args = "cargo clippy --fix -- --no-deps" .split_whitespace() .map(ToString::to_string); let cmd = ClippyCmd::new(args); diff --git a/tests/compile-test.rs b/tests/compile-test.rs index 7d266a36bb6..caa19e39ccd 100644 --- a/tests/compile-test.rs +++ b/tests/compile-test.rs @@ -48,7 +48,24 @@ fn third_party_crates() -> String { && name.rsplit('.').next().map(|ext| ext.eq_ignore_ascii_case("rlib")) == Some(true) { if let Some(old) = crates.insert(dep, path.clone()) { - panic!("Found multiple rlibs for crate `{}`: `{:?}` and `{:?}", dep, old, path); + // Check which action should be done in order to remove compiled deps. + // If pre-installed version of compiler is used, `cargo clean` will do. + // Otherwise (for bootstrapped compiler), the dependencies directory + // must be removed manually. + let suggested_action = if std::env::var_os("RUSTC_BOOTSTRAP").is_some() { + "remove the stageN-tools directory" + } else { + "run `cargo clean`" + }; + + panic!( + "\n---------------------------------------------------\n\n \ + Found multiple rlibs for crate `{}`: `{:?}` and `{:?}`.\n \ + Probably, you need to {} before running tests again.\n \ + \nFor details on that error see https://github.com/rust-lang/rust-clippy/issues/7343 \ + \n---------------------------------------------------\n", + dep, old, path, suggested_action + ); } break; } diff --git a/tests/ui-toml/missing_enforced_import_rename/clippy.toml b/tests/ui-toml/missing_enforced_import_rename/clippy.toml new file mode 100644 index 00000000000..05ba822874d --- /dev/null +++ b/tests/ui-toml/missing_enforced_import_rename/clippy.toml @@ -0,0 +1,10 @@ +enforced-import-renames = [ + { path = "std::option::Option", rename = "Maybe" }, + { path = "std::process::Child", rename = "Kid" }, + { path = "std::process::exit", rename = "goodbye" }, + { path = "std::collections::BTreeMap", rename = "Map" }, + { path = "std::clone", rename = "foo" }, + { path = "std::thread::sleep", rename = "thread_sleep" }, + { path = "std::any::type_name", rename = "ident" }, + { path = "std::sync::Mutex", rename = "StdMutie" } +] diff --git a/tests/ui-toml/missing_enforced_import_rename/conf_missing_enforced_import_rename.rs b/tests/ui-toml/missing_enforced_import_rename/conf_missing_enforced_import_rename.rs new file mode 100644 index 00000000000..f60058c8628 --- /dev/null +++ b/tests/ui-toml/missing_enforced_import_rename/conf_missing_enforced_import_rename.rs @@ -0,0 +1,16 @@ +#![warn(clippy::missing_enforced_import_renames)] + +use std::alloc as colla; +use std::option::Option as Maybe; +use std::process::{exit as wrong_exit, Child as Kid}; +use std::thread::sleep; +#[rustfmt::skip] +use std::{ + any::{type_name, Any}, + clone, + sync :: Mutex, +}; + +fn main() { + use std::collections::BTreeMap as OopsWrongRename; +} diff --git a/tests/ui-toml/missing_enforced_import_rename/conf_missing_enforced_import_rename.stderr b/tests/ui-toml/missing_enforced_import_rename/conf_missing_enforced_import_rename.stderr new file mode 100644 index 00000000000..45de8fdffef --- /dev/null +++ b/tests/ui-toml/missing_enforced_import_rename/conf_missing_enforced_import_rename.stderr @@ -0,0 +1,40 @@ +error: this import should be renamed + --> $DIR/conf_missing_enforced_import_rename.rs:5:20 + | +LL | use std::process::{exit as wrong_exit, Child as Kid}; + | ^^^^^^^^^^^^^^^^^^ help: try: `exit as goodbye` + | + = note: `-D clippy::missing-enforced-import-renames` implied by `-D warnings` + +error: this import should be renamed + --> $DIR/conf_missing_enforced_import_rename.rs:6:1 + | +LL | use std::thread::sleep; + | ^^^^^^^^^^^^^^^^^^^^^^ help: try: `use std::thread::sleep as thread_sleep` + +error: this import should be renamed + --> $DIR/conf_missing_enforced_import_rename.rs:9:11 + | +LL | any::{type_name, Any}, + | ^^^^^^^^^ help: try: `type_name as ident` + +error: this import should be renamed + --> $DIR/conf_missing_enforced_import_rename.rs:10:5 + | +LL | clone, + | ^^^^^ help: try: `clone as foo` + +error: this import should be renamed + --> $DIR/conf_missing_enforced_import_rename.rs:11:5 + | +LL | sync :: Mutex, + | ^^^^^^^^^^^^^ help: try: `sync :: Mutex as StdMutie` + +error: this import should be renamed + --> $DIR/conf_missing_enforced_import_rename.rs:15:5 + | +LL | use std::collections::BTreeMap as OopsWrongRename; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `use std::collections::BTreeMap as Map` + +error: aborting due to 6 previous errors + diff --git a/tests/ui-toml/nonstandard_macro_braces/clippy.toml b/tests/ui-toml/nonstandard_macro_braces/clippy.toml new file mode 100644 index 00000000000..bced8948a02 --- /dev/null +++ b/tests/ui-toml/nonstandard_macro_braces/clippy.toml @@ -0,0 +1,6 @@ +standard-macro-braces = [ + { name = "quote", brace = "{" }, + { name = "quote::quote", brace = "{" }, + { name = "eprint", brace = "[" }, + { name = "type_pos", brace = "[" }, +] diff --git a/tests/ui-toml/nonstandard_macro_braces/conf_nonstandard_macro_braces.rs b/tests/ui-toml/nonstandard_macro_braces/conf_nonstandard_macro_braces.rs new file mode 100644 index 00000000000..4ae6864cbb0 --- /dev/null +++ b/tests/ui-toml/nonstandard_macro_braces/conf_nonstandard_macro_braces.rs @@ -0,0 +1,44 @@ +// #![warn(clippy::nonstandard_macro_braces)] + +extern crate quote; + +use quote::quote; + +#[rustfmt::skip] +macro_rules! test { + () => { + vec!{0, 0, 0} + }; +} + +#[rustfmt::skip] +macro_rules! test2 { + ($($arg:tt)*) => { + format_args!($($arg)*) + }; +} + +macro_rules! type_pos { + ($what:ty) => { + Vec<$what> + }; +} + +#[rustfmt::skip] +fn main() { + let _ = vec! {1, 2, 3}; + let _ = format!["ugh {} stop being such a good compiler", "hello"]; + let _ = quote!(let x = 1;); + let _ = quote::quote!(match match match); + let _ = test!(); + let _ = vec![1,2,3]; + + let _ = quote::quote! {true || false}; + let _ = vec! [0 ,0 ,0]; + let _ = format!("fds{}fds", 10); + let _ = test2!["{}{}{}", 1, 2, 3]; + + let _: type_pos!(usize) = vec![]; + + eprint!("test if user config overrides defaults"); +} diff --git a/tests/ui-toml/nonstandard_macro_braces/conf_nonstandard_macro_braces.stderr b/tests/ui-toml/nonstandard_macro_braces/conf_nonstandard_macro_braces.stderr new file mode 100644 index 00000000000..7bcd1829524 --- /dev/null +++ b/tests/ui-toml/nonstandard_macro_braces/conf_nonstandard_macro_braces.stderr @@ -0,0 +1,94 @@ +error: use of irregular braces for `vec!` macro + --> $DIR/conf_nonstandard_macro_braces.rs:29:13 + | +LL | let _ = vec! {1, 2, 3}; + | ^^^^^^^^^^^^^^ + | + = note: `-D clippy::nonstandard-macro-braces` implied by `-D warnings` +help: consider writing `vec![1, 2, 3]` + --> $DIR/conf_nonstandard_macro_braces.rs:29:13 + | +LL | let _ = vec! {1, 2, 3}; + | ^^^^^^^^^^^^^^ + +error: use of irregular braces for `format!` macro + --> $DIR/conf_nonstandard_macro_braces.rs:30:13 + | +LL | let _ = format!["ugh {} stop being such a good compiler", "hello"]; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: consider writing `format!("ugh () stop being such a good compiler", "hello")` + --> $DIR/conf_nonstandard_macro_braces.rs:30:13 + | +LL | let _ = format!["ugh {} stop being such a good compiler", "hello"]; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: use of irregular braces for `quote!` macro + --> $DIR/conf_nonstandard_macro_braces.rs:31:13 + | +LL | let _ = quote!(let x = 1;); + | ^^^^^^^^^^^^^^^^^^ + | +help: consider writing `quote! {let x = 1;}` + --> $DIR/conf_nonstandard_macro_braces.rs:31:13 + | +LL | let _ = quote!(let x = 1;); + | ^^^^^^^^^^^^^^^^^^ + +error: use of irregular braces for `quote::quote!` macro + --> $DIR/conf_nonstandard_macro_braces.rs:32:13 + | +LL | let _ = quote::quote!(match match match); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: consider writing `quote::quote! {match match match}` + --> $DIR/conf_nonstandard_macro_braces.rs:32:13 + | +LL | let _ = quote::quote!(match match match); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: use of irregular braces for `vec!` macro + --> $DIR/conf_nonstandard_macro_braces.rs:10:9 + | +LL | vec!{0, 0, 0} + | ^^^^^^^^^^^^^ +... +LL | let _ = test!(); + | ------- in this macro invocation + | +help: consider writing `vec![0, 0, 0]` + --> $DIR/conf_nonstandard_macro_braces.rs:10:9 + | +LL | vec!{0, 0, 0} + | ^^^^^^^^^^^^^ +... +LL | let _ = test!(); + | ------- in this macro invocation + = note: this error originates in the macro `test` (in Nightly builds, run with -Z macro-backtrace for more info) + +error: use of irregular braces for `type_pos!` macro + --> $DIR/conf_nonstandard_macro_braces.rs:41:12 + | +LL | let _: type_pos!(usize) = vec![]; + | ^^^^^^^^^^^^^^^^ + | +help: consider writing `type_pos![usize]` + --> $DIR/conf_nonstandard_macro_braces.rs:41:12 + | +LL | let _: type_pos!(usize) = vec![]; + | ^^^^^^^^^^^^^^^^ + +error: use of irregular braces for `eprint!` macro + --> $DIR/conf_nonstandard_macro_braces.rs:43:5 + | +LL | eprint!("test if user config overrides defaults"); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: consider writing `eprint!["test if user config overrides defaults"];` + --> $DIR/conf_nonstandard_macro_braces.rs:43:5 + | +LL | eprint!("test if user config overrides defaults"); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: aborting due to 7 previous errors + diff --git a/tests/ui-toml/toml_disallowed_method/clippy.toml b/tests/ui-toml/toml_disallowed_method/clippy.toml index c0df3b6e8af..a3245da6825 100644 --- a/tests/ui-toml/toml_disallowed_method/clippy.toml +++ b/tests/ui-toml/toml_disallowed_method/clippy.toml @@ -1 +1,5 @@ -disallowed-methods = ["core::iter::traits::iterator::Iterator::sum", "regex::re_unicode::Regex::is_match", "regex::re_unicode::Regex::new"] +disallowed-methods = [ + "std::iter::Iterator::sum", + "regex::Regex::is_match", + "regex::Regex::new" +] diff --git a/tests/ui-toml/toml_disallowed_type/clippy.toml b/tests/ui-toml/toml_disallowed_type/clippy.toml new file mode 100644 index 00000000000..2eff854c22c --- /dev/null +++ b/tests/ui-toml/toml_disallowed_type/clippy.toml @@ -0,0 +1,9 @@ +disallowed-types = [ + "std::collections::HashMap", + "std::sync::atomic::AtomicU32", + "syn::TypePath", + "proc_macro2::Ident", + "std::thread::Thread", + "std::time::Instant", + "std::io::Read", +] diff --git a/tests/ui-toml/toml_disallowed_type/conf_disallowed_type.rs b/tests/ui-toml/toml_disallowed_type/conf_disallowed_type.rs new file mode 100644 index 00000000000..567afb5aec1 --- /dev/null +++ b/tests/ui-toml/toml_disallowed_type/conf_disallowed_type.rs @@ -0,0 +1,35 @@ +#![warn(clippy::disallowed_type)] + +extern crate quote; +extern crate syn; + +use std::sync as foo; +use std::sync::atomic::AtomicU32; +use std::time::Instant as Sneaky; + +struct HashMap; + +fn bad_return_type() -> fn() -> Sneaky { + todo!() +} + +fn bad_arg_type(_: impl Fn(Sneaky) -> foo::atomic::AtomicU32) { + todo!() +} + +fn trait_obj(_: &dyn std::io::Read) { + todo!() +} + +static BAD: foo::atomic::AtomicPtr<()> = foo::atomic::AtomicPtr::new(std::ptr::null_mut()); + +#[allow(clippy::diverging_sub_expression)] +fn main() { + let _: std::collections::HashMap<(), ()> = std::collections::HashMap::new(); + let _ = Sneaky::now(); + let _ = foo::atomic::AtomicU32::new(0); + static FOO: std::sync::atomic::AtomicU32 = foo::atomic::AtomicU32::new(1); + let _: std::collections::BTreeMap<(), syn::TypePath> = Default::default(); + let _ = syn::Ident::new("", todo!()); + let _ = HashMap; +} diff --git a/tests/ui-toml/toml_disallowed_type/conf_disallowed_type.stderr b/tests/ui-toml/toml_disallowed_type/conf_disallowed_type.stderr new file mode 100644 index 00000000000..4e6fd91fba1 --- /dev/null +++ b/tests/ui-toml/toml_disallowed_type/conf_disallowed_type.stderr @@ -0,0 +1,88 @@ +error: `std::sync::atomic::AtomicU32` is not allowed according to config + --> $DIR/conf_disallowed_type.rs:7:1 + | +LL | use std::sync::atomic::AtomicU32; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: `-D clippy::disallowed-type` implied by `-D warnings` + +error: `std::time::Instant` is not allowed according to config + --> $DIR/conf_disallowed_type.rs:8:1 + | +LL | use std::time::Instant as Sneaky; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: `std::time::Instant` is not allowed according to config + --> $DIR/conf_disallowed_type.rs:12:33 + | +LL | fn bad_return_type() -> fn() -> Sneaky { + | ^^^^^^ + +error: `std::time::Instant` is not allowed according to config + --> $DIR/conf_disallowed_type.rs:16:28 + | +LL | fn bad_arg_type(_: impl Fn(Sneaky) -> foo::atomic::AtomicU32) { + | ^^^^^^ + +error: `std::sync::atomic::AtomicU32` is not allowed according to config + --> $DIR/conf_disallowed_type.rs:16:39 + | +LL | fn bad_arg_type(_: impl Fn(Sneaky) -> foo::atomic::AtomicU32) { + | ^^^^^^^^^^^^^^^^^^^^^^ + +error: `std::io::Read` is not allowed according to config + --> $DIR/conf_disallowed_type.rs:20:22 + | +LL | fn trait_obj(_: &dyn std::io::Read) { + | ^^^^^^^^^^^^^ + +error: `std::collections::HashMap` is not allowed according to config + --> $DIR/conf_disallowed_type.rs:28: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:28: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:29:13 + | +LL | let _ = Sneaky::now(); + | ^^^^^^ + +error: `std::sync::atomic::AtomicU32` is not allowed according to config + --> $DIR/conf_disallowed_type.rs:30:13 + | +LL | let _ = foo::atomic::AtomicU32::new(0); + | ^^^^^^^^^^^^^^^^^^^^^^ + +error: `std::sync::atomic::AtomicU32` is not allowed according to config + --> $DIR/conf_disallowed_type.rs:31: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:31: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:32:43 + | +LL | let _: std::collections::BTreeMap<(), syn::TypePath> = Default::default(); + | ^^^^^^^^^^^^^ + +error: `proc_macro2::Ident` is not allowed according to config + --> $DIR/conf_disallowed_type.rs:33:13 + | +LL | let _ = syn::Ident::new("", todo!()); + | ^^^^^^^^^^ + +error: aborting due to 14 previous errors + diff --git a/tests/ui-toml/toml_unknown_key/conf_unknown_key.stderr b/tests/ui-toml/toml_unknown_key/conf_unknown_key.stderr index a7be00426c4..e0029ebeb27 100644 --- a/tests/ui-toml/toml_unknown_key/conf_unknown_key.stderr +++ b/tests/ui-toml/toml_unknown_key/conf_unknown_key.stderr @@ -1,4 +1,4 @@ -error: error reading Clippy's configuration file `$DIR/clippy.toml`: unknown field `foobar`, expected one of `avoid-breaking-exported-api`, `msrv`, `blacklisted-names`, `cognitive-complexity-threshold`, `cyclomatic-complexity-threshold`, `doc-valid-idents`, `too-many-arguments-threshold`, `type-complexity-threshold`, `single-char-binding-names-threshold`, `too-large-for-stack`, `enum-variant-name-threshold`, `enum-variant-size-threshold`, `verbose-bit-mask-threshold`, `literal-representation-threshold`, `trivial-copy-size-limit`, `pass-by-value-size-limit`, `too-many-lines-threshold`, `array-size-threshold`, `vec-box-size-threshold`, `max-trait-bounds`, `max-struct-bools`, `max-fn-params-bools`, `warn-on-all-wildcard-imports`, `disallowed-methods`, `unreadable-literal-lint-fractions`, `upper-case-acronyms-aggressive`, `cargo-ignore-publish`, `third-party` at line 5 column 1 +error: error reading Clippy's configuration file `$DIR/clippy.toml`: unknown field `foobar`, expected one of `avoid-breaking-exported-api`, `msrv`, `blacklisted-names`, `cognitive-complexity-threshold`, `cyclomatic-complexity-threshold`, `doc-valid-idents`, `too-many-arguments-threshold`, `type-complexity-threshold`, `single-char-binding-names-threshold`, `too-large-for-stack`, `enum-variant-name-threshold`, `enum-variant-size-threshold`, `verbose-bit-mask-threshold`, `literal-representation-threshold`, `trivial-copy-size-limit`, `pass-by-value-size-limit`, `too-many-lines-threshold`, `array-size-threshold`, `vec-box-size-threshold`, `max-trait-bounds`, `max-struct-bools`, `max-fn-params-bools`, `warn-on-all-wildcard-imports`, `disallowed-methods`, `disallowed-types`, `unreadable-literal-lint-fractions`, `upper-case-acronyms-aggressive`, `cargo-ignore-publish`, `standard-macro-braces`, `enforced-import-renames`, `allowed-scripts`, `third-party` at line 5 column 1 error: aborting due to previous error diff --git a/tests/ui/append_instead_of_extend.fixed b/tests/ui/append_instead_of_extend.fixed new file mode 100644 index 00000000000..283358333cd --- /dev/null +++ b/tests/ui/append_instead_of_extend.fixed @@ -0,0 +1,55 @@ +// run-rustfix +#![warn(clippy::append_instead_of_extend)] +use std::collections::BinaryHeap; +fn main() { + //gets linted + let mut vec1 = vec![0u8; 1024]; + let mut vec2: std::vec::Vec<u8> = Vec::new(); + + vec2.append(&mut vec1); + + let mut vec3 = vec![0u8; 1024]; + let mut vec4: std::vec::Vec<u8> = Vec::new(); + + vec4.append(&mut vec3); + + let mut vec11: std::vec::Vec<u8> = Vec::new(); + + vec11.append(&mut return_vector()); + + //won't get linted it dosen't move the entire content of a vec into another + let mut test1 = vec![0u8, 10]; + let mut test2: std::vec::Vec<u8> = Vec::new(); + + test2.extend(test1.drain(4..10)); + + let mut vec3 = vec![0u8; 104]; + let mut vec7: std::vec::Vec<u8> = Vec::new(); + + vec3.append(&mut vec7); + + let mut vec5 = vec![0u8; 1024]; + let mut vec6: std::vec::Vec<u8> = Vec::new(); + + vec5.extend(vec6.drain(..4)); + + let mut vec9: std::vec::Vec<u8> = Vec::new(); + + return_vector().append(&mut vec9); + + //won't get linted because it is not a vec + + let mut heap = BinaryHeap::from(vec![1, 3]); + let mut heap2 = BinaryHeap::from(vec![]); + heap2.extend(heap.drain()) +} + +fn return_vector() -> Vec<u8> { + let mut new_vector = vec![]; + + for i in 1..10 { + new_vector.push(i) + } + + new_vector +} diff --git a/tests/ui/append_instead_of_extend.rs b/tests/ui/append_instead_of_extend.rs new file mode 100644 index 00000000000..abde5cdac5c --- /dev/null +++ b/tests/ui/append_instead_of_extend.rs @@ -0,0 +1,55 @@ +// run-rustfix +#![warn(clippy::append_instead_of_extend)] +use std::collections::BinaryHeap; +fn main() { + //gets linted + let mut vec1 = vec![0u8; 1024]; + let mut vec2: std::vec::Vec<u8> = Vec::new(); + + vec2.extend(vec1.drain(..)); + + let mut vec3 = vec![0u8; 1024]; + let mut vec4: std::vec::Vec<u8> = Vec::new(); + + vec4.extend(vec3.drain(..)); + + let mut vec11: std::vec::Vec<u8> = Vec::new(); + + vec11.extend(return_vector().drain(..)); + + //won't get linted it dosen't move the entire content of a vec into another + let mut test1 = vec![0u8, 10]; + let mut test2: std::vec::Vec<u8> = Vec::new(); + + test2.extend(test1.drain(4..10)); + + let mut vec3 = vec![0u8; 104]; + let mut vec7: std::vec::Vec<u8> = Vec::new(); + + vec3.append(&mut vec7); + + let mut vec5 = vec![0u8; 1024]; + let mut vec6: std::vec::Vec<u8> = Vec::new(); + + vec5.extend(vec6.drain(..4)); + + let mut vec9: std::vec::Vec<u8> = Vec::new(); + + return_vector().append(&mut vec9); + + //won't get linted because it is not a vec + + let mut heap = BinaryHeap::from(vec![1, 3]); + let mut heap2 = BinaryHeap::from(vec![]); + heap2.extend(heap.drain()) +} + +fn return_vector() -> Vec<u8> { + let mut new_vector = vec![]; + + for i in 1..10 { + new_vector.push(i) + } + + new_vector +} diff --git a/tests/ui/append_instead_of_extend.stderr b/tests/ui/append_instead_of_extend.stderr new file mode 100644 index 00000000000..9d309d981de --- /dev/null +++ b/tests/ui/append_instead_of_extend.stderr @@ -0,0 +1,22 @@ +error: use of `extend` instead of `append` for adding the full range of a second vector + --> $DIR/append_instead_of_extend.rs:9:5 + | +LL | vec2.extend(vec1.drain(..)); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `vec2.append(&mut vec1)` + | + = note: `-D clippy::append-instead-of-extend` implied by `-D warnings` + +error: use of `extend` instead of `append` for adding the full range of a second vector + --> $DIR/append_instead_of_extend.rs:14:5 + | +LL | vec4.extend(vec3.drain(..)); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `vec4.append(&mut vec3)` + +error: use of `extend` instead of `append` for adding the full range of a second vector + --> $DIR/append_instead_of_extend.rs:18:5 + | +LL | vec11.extend(return_vector().drain(..)); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `vec11.append(&mut return_vector())` + +error: aborting due to 3 previous errors + diff --git a/tests/ui/assertions_on_constants.rs b/tests/ui/assertions_on_constants.rs index e989de65404..6617ca183a8 100644 --- a/tests/ui/assertions_on_constants.rs +++ b/tests/ui/assertions_on_constants.rs @@ -28,4 +28,7 @@ fn main() { debug_assert!(false); // #3948 assert_const!(3); assert_const!(-1); + + // Don't lint on this: + assert!(cfg!(feature = "hey") || cfg!(not(feature = "asdf"))); } diff --git a/tests/ui/auxiliary/macro_rules.rs b/tests/ui/auxiliary/macro_rules.rs index d4470d3f407..170955e726c 100644 --- a/tests/ui/auxiliary/macro_rules.rs +++ b/tests/ui/auxiliary/macro_rules.rs @@ -106,3 +106,10 @@ macro_rules! field_reassign_with_default { } }; } + +#[macro_export] +macro_rules! default_numeric_fallback { + () => { + let x = 22; + }; +} diff --git a/tests/ui/auxiliary/non-exhaustive-enum.rs b/tests/ui/auxiliary/non-exhaustive-enum.rs new file mode 100644 index 00000000000..420232f9f8d --- /dev/null +++ b/tests/ui/auxiliary/non-exhaustive-enum.rs @@ -0,0 +1,8 @@ +// Stripped down version of the ErrorKind enum of std +#[non_exhaustive] +pub enum ErrorKind { + NotFound, + PermissionDenied, + #[doc(hidden)] + Uncategorized, +} diff --git a/tests/ui/blacklisted_name.rs b/tests/ui/blacklisted_name.rs index cb15bdd2f1b..57d7139fef5 100644 --- a/tests/ui/blacklisted_name.rs +++ b/tests/ui/blacklisted_name.rs @@ -43,3 +43,15 @@ fn issue_1647_ref_mut() { let ref mut baz = 0; if let Some(ref mut quux) = Some(42) {} } + +mod tests { + fn issue_7305() { + // `blackisted_name` lint should not be triggered inside of the test code. + let foo = 0; + + // Check that even in nested functions warning is still not triggere. + fn nested() { + let foo = 0; + } + } +} diff --git a/tests/ui/default_numeric_fallback.rs b/tests/ui/default_numeric_fallback.rs index 43468872db0..c0625fd1b75 100644 --- a/tests/ui/default_numeric_fallback.rs +++ b/tests/ui/default_numeric_fallback.rs @@ -1,3 +1,5 @@ +// aux-build:macro_rules.rs + #![warn(clippy::default_numeric_fallback)] #![allow(unused)] #![allow(clippy::never_loop)] @@ -5,6 +7,9 @@ #![allow(clippy::unnecessary_operation)] #![allow(clippy::branches_sharing_code)] +#[macro_use] +extern crate macro_rules; + mod basic_expr { fn test() { // Should lint unsuffixed literals typed `i32`. @@ -133,4 +138,22 @@ mod method_calls { } } +mod in_macro { + macro_rules! internal_macro { + () => { + let x = 22; + }; + } + + // Should lint in internal macro. + fn internal() { + internal_macro!(); + } + + // Should NOT lint in external macro. + fn external() { + default_numeric_fallback!(); + } +} + fn main() {} diff --git a/tests/ui/default_numeric_fallback.stderr b/tests/ui/default_numeric_fallback.stderr index d1c4c8203dd..5862cd936ac 100644 --- a/tests/ui/default_numeric_fallback.stderr +++ b/tests/ui/default_numeric_fallback.stderr @@ -1,5 +1,5 @@ error: default numeric fallback might occur - --> $DIR/default_numeric_fallback.rs:11:17 + --> $DIR/default_numeric_fallback.rs:16:17 | LL | let x = 22; | ^^ help: consider adding suffix: `22_i32` @@ -7,142 +7,153 @@ LL | let x = 22; = note: `-D clippy::default-numeric-fallback` implied by `-D warnings` error: default numeric fallback might occur - --> $DIR/default_numeric_fallback.rs:12:18 + --> $DIR/default_numeric_fallback.rs:17:18 | LL | let x = [1, 2, 3]; | ^ help: consider adding suffix: `1_i32` error: default numeric fallback might occur - --> $DIR/default_numeric_fallback.rs:12:21 + --> $DIR/default_numeric_fallback.rs:17:21 | LL | let x = [1, 2, 3]; | ^ help: consider adding suffix: `2_i32` error: default numeric fallback might occur - --> $DIR/default_numeric_fallback.rs:12:24 + --> $DIR/default_numeric_fallback.rs:17:24 | LL | let x = [1, 2, 3]; | ^ help: consider adding suffix: `3_i32` error: default numeric fallback might occur - --> $DIR/default_numeric_fallback.rs:13:28 + --> $DIR/default_numeric_fallback.rs:18:28 | LL | let x = if true { (1, 2) } else { (3, 4) }; | ^ help: consider adding suffix: `1_i32` error: default numeric fallback might occur - --> $DIR/default_numeric_fallback.rs:13:31 + --> $DIR/default_numeric_fallback.rs:18:31 | LL | let x = if true { (1, 2) } else { (3, 4) }; | ^ help: consider adding suffix: `2_i32` error: default numeric fallback might occur - --> $DIR/default_numeric_fallback.rs:13:44 + --> $DIR/default_numeric_fallback.rs:18:44 | LL | let x = if true { (1, 2) } else { (3, 4) }; | ^ help: consider adding suffix: `3_i32` error: default numeric fallback might occur - --> $DIR/default_numeric_fallback.rs:13:47 + --> $DIR/default_numeric_fallback.rs:18:47 | LL | let x = if true { (1, 2) } else { (3, 4) }; | ^ help: consider adding suffix: `4_i32` error: default numeric fallback might occur - --> $DIR/default_numeric_fallback.rs:14:23 + --> $DIR/default_numeric_fallback.rs:19:23 | LL | let x = match 1 { | ^ help: consider adding suffix: `1_i32` error: default numeric fallback might occur - --> $DIR/default_numeric_fallback.rs:15:13 + --> $DIR/default_numeric_fallback.rs:20:13 | LL | 1 => 1, | ^ help: consider adding suffix: `1_i32` error: default numeric fallback might occur - --> $DIR/default_numeric_fallback.rs:15:18 + --> $DIR/default_numeric_fallback.rs:20:18 | LL | 1 => 1, | ^ help: consider adding suffix: `1_i32` error: default numeric fallback might occur - --> $DIR/default_numeric_fallback.rs:16:18 + --> $DIR/default_numeric_fallback.rs:21:18 | LL | _ => 2, | ^ help: consider adding suffix: `2_i32` error: default numeric fallback might occur - --> $DIR/default_numeric_fallback.rs:20:17 + --> $DIR/default_numeric_fallback.rs:25:17 | LL | let x = 0.12; | ^^^^ help: consider adding suffix: `0.12_f64` error: default numeric fallback might occur - --> $DIR/default_numeric_fallback.rs:38:21 + --> $DIR/default_numeric_fallback.rs:43:21 | LL | let y = 1; | ^ help: consider adding suffix: `1_i32` error: default numeric fallback might occur - --> $DIR/default_numeric_fallback.rs:46:21 + --> $DIR/default_numeric_fallback.rs:51:21 | LL | let y = 1; | ^ help: consider adding suffix: `1_i32` error: default numeric fallback might occur - --> $DIR/default_numeric_fallback.rs:52:21 + --> $DIR/default_numeric_fallback.rs:57:21 | LL | let y = 1; | ^ help: consider adding suffix: `1_i32` error: default numeric fallback might occur - --> $DIR/default_numeric_fallback.rs:64:9 + --> $DIR/default_numeric_fallback.rs:69:9 | LL | 1 | ^ help: consider adding suffix: `1_i32` error: default numeric fallback might occur - --> $DIR/default_numeric_fallback.rs:70:27 + --> $DIR/default_numeric_fallback.rs:75:27 | LL | let f = || -> _ { 1 }; | ^ help: consider adding suffix: `1_i32` error: default numeric fallback might occur - --> $DIR/default_numeric_fallback.rs:74:29 + --> $DIR/default_numeric_fallback.rs:79:29 | LL | let f = || -> i32 { 1 }; | ^ help: consider adding suffix: `1_i32` error: default numeric fallback might occur - --> $DIR/default_numeric_fallback.rs:88:21 + --> $DIR/default_numeric_fallback.rs:93:21 | LL | generic_arg(1); | ^ help: consider adding suffix: `1_i32` error: default numeric fallback might occur - --> $DIR/default_numeric_fallback.rs:91:32 + --> $DIR/default_numeric_fallback.rs:96:32 | LL | let x: _ = generic_arg(1); | ^ help: consider adding suffix: `1_i32` error: default numeric fallback might occur - --> $DIR/default_numeric_fallback.rs:109:28 + --> $DIR/default_numeric_fallback.rs:114:28 | LL | GenericStruct { x: 1 }; | ^ help: consider adding suffix: `1_i32` error: default numeric fallback might occur - --> $DIR/default_numeric_fallback.rs:112:36 + --> $DIR/default_numeric_fallback.rs:117:36 | LL | let _ = GenericStruct { x: 1 }; | ^ help: consider adding suffix: `1_i32` error: default numeric fallback might occur - --> $DIR/default_numeric_fallback.rs:132:23 + --> $DIR/default_numeric_fallback.rs:137:23 | LL | s.generic_arg(1); | ^ help: consider adding suffix: `1_i32` -error: aborting due to 24 previous errors +error: default numeric fallback might occur + --> $DIR/default_numeric_fallback.rs:144:21 + | +LL | let x = 22; + | ^^ help: consider adding suffix: `22_i32` +... +LL | internal_macro!(); + | ------------------ in this macro invocation + | + = note: this error originates in the macro `internal_macro` (in Nightly builds, run with -Z macro-backtrace for more info) + +error: aborting due to 25 previous errors diff --git a/tests/ui/deprecated.stderr b/tests/ui/deprecated.stderr index 03c9f438891..0af6b500115 100644 --- a/tests/ui/deprecated.stderr +++ b/tests/ui/deprecated.stderr @@ -84,13 +84,13 @@ error: lint `clippy::filter_map` has been removed: this lint has been replaced b LL | #[warn(clippy::filter_map)] | ^^^^^^^^^^^^^^^^^^ -error: lint `clippy::pub_enum_variant_names` has been removed: set the `avoid_breaking_exported_api` config option to `false` to enable the `enum_variant_names` lint for public items +error: lint `clippy::pub_enum_variant_names` has been removed: set the `avoid-breaking-exported-api` config option to `false` to enable the `enum_variant_names` lint for public items --> $DIR/deprecated.rs:15:8 | LL | #[warn(clippy::pub_enum_variant_names)] | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ -error: lint `clippy::wrong_pub_self_convention` has been removed: set the `avoid_breaking_exported_api` config option to `false` to enable the `wrong_self_convention` lint for public items +error: lint `clippy::wrong_pub_self_convention` has been removed: set the `avoid-breaking-exported-api` config option to `false` to enable the `wrong_self_convention` lint for public items --> $DIR/deprecated.rs:16:8 | LL | #[warn(clippy::wrong_pub_self_convention)] diff --git a/tests/ui/disallowed_script_idents.rs b/tests/ui/disallowed_script_idents.rs new file mode 100644 index 00000000000..cfdda35971f --- /dev/null +++ b/tests/ui/disallowed_script_idents.rs @@ -0,0 +1,10 @@ +#![deny(clippy::disallowed_script_idents)] +#![allow(dead_code)] + +fn main() { + let counter = 10; // OK, latin is allowed. + let zähler = 10; // OK, it's still latin. + + let счётчик = 10; // Cyrillic is not allowed by default. + let カウンタ = 10; // Same for japanese. +} diff --git a/tests/ui/disallowed_script_idents.stderr b/tests/ui/disallowed_script_idents.stderr new file mode 100644 index 00000000000..cc84dc1d43c --- /dev/null +++ b/tests/ui/disallowed_script_idents.stderr @@ -0,0 +1,20 @@ +error: identifier `счётчик` has a Unicode script that is not allowed by configuration: Cyrillic + --> $DIR/disallowed_script_idents.rs:8:9 + | +LL | let счётчик = 10; // Cyrillic is not allowed by default. + | ^^^^^^^ + | +note: the lint level is defined here + --> $DIR/disallowed_script_idents.rs:1:9 + | +LL | #![deny(clippy::disallowed_script_idents)] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: identifier `カウンタ` has a Unicode script that is not allowed by configuration: Katakana + --> $DIR/disallowed_script_idents.rs:9:9 + | +LL | let カウンタ = 10; // Same for japanese. + | ^^^^^^^^ + +error: aborting due to 2 previous errors + diff --git a/tests/ui/doc.rs b/tests/ui/doc/doc.rs similarity index 99% rename from tests/ui/doc.rs rename to tests/ui/doc/doc.rs index c946a047f1b..8afef6b23d4 100644 --- a/tests/ui/doc.rs +++ b/tests/ui/doc/doc.rs @@ -64,7 +64,7 @@ fn test_units() { /// WebGL /// TensorFlow /// TrueType -/// iOS macOS +/// iOS macOS FreeBSD /// TeX LaTeX BibTeX BibLaTeX /// MinGW /// CamelCase (see also #2395) diff --git a/tests/ui/doc.stderr b/tests/ui/doc/doc.stderr similarity index 100% rename from tests/ui/doc.stderr rename to tests/ui/doc/doc.stderr diff --git a/tests/ui/doc/unbalanced_ticks.rs b/tests/ui/doc/unbalanced_ticks.rs new file mode 100644 index 00000000000..78e87bc6906 --- /dev/null +++ b/tests/ui/doc/unbalanced_ticks.rs @@ -0,0 +1,36 @@ +//! This file tests for the `DOC_MARKDOWN` lint, specifically cases +//! where ticks are unbalanced (see issue #6753). + +#![allow(dead_code)] +#![warn(clippy::doc_markdown)] + +/// This is a doc comment with `unbalanced_tick marks and several words that +/// should be `encompassed_by` tick marks because they `contain_underscores`. +/// Because of the initial `unbalanced_tick` pair, the error message is +/// very `confusing_and_misleading`. +fn main() {} + +/// This paragraph has `unbalanced_tick marks and should stop_linting. +/// +/// This paragraph is fine and should_be linted normally. +/// +/// Double unbalanced backtick from ``here to here` should lint. +/// +/// Double balanced back ticks ``start end`` is fine. +fn multiple_paragraphs() {} + +/// ``` +/// // Unbalanced tick mark in code block shouldn't warn: +/// ` +/// ``` +fn in_code_block() {} + +/// # `Fine` +/// +/// ## not_fine +/// +/// ### `unbalanced +/// +/// - This `item has unbalanced tick marks +/// - This item needs backticks_here +fn other_markdown() {} diff --git a/tests/ui/doc/unbalanced_ticks.stderr b/tests/ui/doc/unbalanced_ticks.stderr new file mode 100644 index 00000000000..45ca34e2a8c --- /dev/null +++ b/tests/ui/doc/unbalanced_ticks.stderr @@ -0,0 +1,64 @@ +error: backticks are unbalanced + --> $DIR/unbalanced_ticks.rs:7:1 + | +LL | / /// This is a doc comment with `unbalanced_tick marks and several words that +LL | | /// should be `encompassed_by` tick marks because they `contain_underscores`. +LL | | /// Because of the initial `unbalanced_tick` pair, the error message is +LL | | /// very `confusing_and_misleading`. + | |____________________________________^ + | + = note: `-D clippy::doc-markdown` implied by `-D warnings` + = help: a backtick may be missing a pair + +error: backticks are unbalanced + --> $DIR/unbalanced_ticks.rs:13:1 + | +LL | /// This paragraph has `unbalanced_tick marks and should stop_linting. + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = help: a backtick may be missing a pair + +error: you should put `should_be` between ticks in the documentation + --> $DIR/unbalanced_ticks.rs:15:32 + | +LL | /// This paragraph is fine and should_be linted normally. + | ^^^^^^^^^ + +error: backticks are unbalanced + --> $DIR/unbalanced_ticks.rs:17:1 + | +LL | /// Double unbalanced backtick from ``here to here` should lint. + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = help: a backtick may be missing a pair + +error: you should put `not_fine` between ticks in the documentation + --> $DIR/unbalanced_ticks.rs:30:8 + | +LL | /// ## not_fine + | ^^^^^^^^ + +error: backticks are unbalanced + --> $DIR/unbalanced_ticks.rs:32:1 + | +LL | /// ### `unbalanced + | ^^^^^^^^^^^^^^^^^^^ + | + = help: a backtick may be missing a pair + +error: backticks are unbalanced + --> $DIR/unbalanced_ticks.rs:34:1 + | +LL | /// - This `item has unbalanced tick marks + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = help: a backtick may be missing a pair + +error: you should put `backticks_here` between ticks in the documentation + --> $DIR/unbalanced_ticks.rs:35:23 + | +LL | /// - This item needs backticks_here + | ^^^^^^^^^^^^^^ + +error: aborting due to 8 previous errors + diff --git a/tests/ui/field_reassign_with_default.rs b/tests/ui/field_reassign_with_default.rs index 1368c5d7984..787053fb000 100644 --- a/tests/ui/field_reassign_with_default.rs +++ b/tests/ui/field_reassign_with_default.rs @@ -29,6 +29,21 @@ struct C { i: Vec<i32>, j: i64, } + +#[derive(Default)] +struct D { + a: Option<i32>, + b: Option<i32>, +} + +macro_rules! m { + ($key:ident: $value:tt) => {{ + let mut data = $crate::D::default(); + data.$key = Some($value); + data + }}; +} + /// Implements .next() that returns a different number each time. struct SideEffect(i32); @@ -143,6 +158,11 @@ fn main() { let mut a: WrapperMulti<i32, i64> = Default::default(); a.i = 42; + + // Don't lint in macros + m! { + a: 42 + }; } mod m { diff --git a/tests/ui/field_reassign_with_default.stderr b/tests/ui/field_reassign_with_default.stderr index dd7c0360bb1..b56db08ec8a 100644 --- a/tests/ui/field_reassign_with_default.stderr +++ b/tests/ui/field_reassign_with_default.stderr @@ -1,108 +1,108 @@ error: field assignment outside of initializer for an instance created with Default::default() - --> $DIR/field_reassign_with_default.rs:48:5 + --> $DIR/field_reassign_with_default.rs:63:5 | LL | a.i = 42; | ^^^^^^^^^ | = note: `-D clippy::field-reassign-with-default` implied by `-D warnings` note: consider initializing the variable with `main::A { i: 42, ..Default::default() }` and removing relevant reassignments - --> $DIR/field_reassign_with_default.rs:47:5 + --> $DIR/field_reassign_with_default.rs:62:5 | LL | let mut a: A = Default::default(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: field assignment outside of initializer for an instance created with Default::default() - --> $DIR/field_reassign_with_default.rs:88:5 + --> $DIR/field_reassign_with_default.rs:103:5 | LL | a.j = 43; | ^^^^^^^^^ | note: consider initializing the variable with `main::A { j: 43, i: 42 }` and removing relevant reassignments - --> $DIR/field_reassign_with_default.rs:87:5 + --> $DIR/field_reassign_with_default.rs:102:5 | LL | let mut a: A = Default::default(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: field assignment outside of initializer for an instance created with Default::default() - --> $DIR/field_reassign_with_default.rs:93:5 + --> $DIR/field_reassign_with_default.rs:108:5 | LL | a.i = 42; | ^^^^^^^^^ | note: consider initializing the variable with `main::A { i: 42, j: 44 }` and removing relevant reassignments - --> $DIR/field_reassign_with_default.rs:92:5 + --> $DIR/field_reassign_with_default.rs:107:5 | LL | let mut a: A = Default::default(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: field assignment outside of initializer for an instance created with Default::default() - --> $DIR/field_reassign_with_default.rs:99:5 + --> $DIR/field_reassign_with_default.rs:114:5 | LL | a.i = 42; | ^^^^^^^^^ | note: consider initializing the variable with `main::A { i: 42, ..Default::default() }` and removing relevant reassignments - --> $DIR/field_reassign_with_default.rs:98:5 + --> $DIR/field_reassign_with_default.rs:113:5 | LL | let mut a = A::default(); | ^^^^^^^^^^^^^^^^^^^^^^^^^ error: field assignment outside of initializer for an instance created with Default::default() - --> $DIR/field_reassign_with_default.rs:109:5 + --> $DIR/field_reassign_with_default.rs:124:5 | LL | a.i = Default::default(); | ^^^^^^^^^^^^^^^^^^^^^^^^^ | note: consider initializing the variable with `main::A { i: Default::default(), ..Default::default() }` and removing relevant reassignments - --> $DIR/field_reassign_with_default.rs:108:5 + --> $DIR/field_reassign_with_default.rs:123:5 | LL | let mut a: A = Default::default(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: field assignment outside of initializer for an instance created with Default::default() - --> $DIR/field_reassign_with_default.rs:113:5 + --> $DIR/field_reassign_with_default.rs:128:5 | LL | a.i = Default::default(); | ^^^^^^^^^^^^^^^^^^^^^^^^^ | note: consider initializing the variable with `main::A { i: Default::default(), j: 45 }` and removing relevant reassignments - --> $DIR/field_reassign_with_default.rs:112:5 + --> $DIR/field_reassign_with_default.rs:127:5 | LL | let mut a: A = Default::default(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: field assignment outside of initializer for an instance created with Default::default() - --> $DIR/field_reassign_with_default.rs:135:5 + --> $DIR/field_reassign_with_default.rs:150:5 | LL | a.i = vec![1]; | ^^^^^^^^^^^^^^ | note: consider initializing the variable with `C { i: vec![1], ..Default::default() }` and removing relevant reassignments - --> $DIR/field_reassign_with_default.rs:134:5 + --> $DIR/field_reassign_with_default.rs:149:5 | LL | let mut a: C = C::default(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: field assignment outside of initializer for an instance created with Default::default() - --> $DIR/field_reassign_with_default.rs:142:5 + --> $DIR/field_reassign_with_default.rs:157:5 | LL | a.i = true; | ^^^^^^^^^^^ | note: consider initializing the variable with `Wrapper::<bool> { i: true }` and removing relevant reassignments - --> $DIR/field_reassign_with_default.rs:141:5 + --> $DIR/field_reassign_with_default.rs:156:5 | LL | let mut a: Wrapper<bool> = Default::default(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: field assignment outside of initializer for an instance created with Default::default() - --> $DIR/field_reassign_with_default.rs:145:5 + --> $DIR/field_reassign_with_default.rs:160:5 | LL | a.i = 42; | ^^^^^^^^^ | note: consider initializing the variable with `WrapperMulti::<i32, i64> { i: 42, ..Default::default() }` and removing relevant reassignments - --> $DIR/field_reassign_with_default.rs:144:5 + --> $DIR/field_reassign_with_default.rs:159:5 | LL | let mut a: WrapperMulti<i32, i64> = Default::default(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ diff --git a/tests/ui/filter_map_identity.fixed b/tests/ui/filter_map_identity.fixed index 23ce28d8e9b..a5860aa49b3 100644 --- a/tests/ui/filter_map_identity.fixed +++ b/tests/ui/filter_map_identity.fixed @@ -1,6 +1,6 @@ // run-rustfix -#![allow(unused_imports)] +#![allow(unused_imports, clippy::needless_return)] #![warn(clippy::filter_map_identity)] fn main() { @@ -13,4 +13,7 @@ fn main() { use std::convert::identity; let iterator = vec![Some(1), None, Some(2)].into_iter(); let _ = iterator.flatten(); + + let iterator = vec![Some(1), None, Some(2)].into_iter(); + let _ = iterator.flatten(); } diff --git a/tests/ui/filter_map_identity.rs b/tests/ui/filter_map_identity.rs index e698df13eea..7e998b9cdf7 100644 --- a/tests/ui/filter_map_identity.rs +++ b/tests/ui/filter_map_identity.rs @@ -1,6 +1,6 @@ // run-rustfix -#![allow(unused_imports)] +#![allow(unused_imports, clippy::needless_return)] #![warn(clippy::filter_map_identity)] fn main() { @@ -13,4 +13,7 @@ fn main() { use std::convert::identity; let iterator = vec![Some(1), None, Some(2)].into_iter(); let _ = iterator.filter_map(identity); + + let iterator = vec![Some(1), None, Some(2)].into_iter(); + let _ = iterator.filter_map(|x| return x); } diff --git a/tests/ui/filter_map_identity.stderr b/tests/ui/filter_map_identity.stderr index 596a6320608..43c9fdca4fb 100644 --- a/tests/ui/filter_map_identity.stderr +++ b/tests/ui/filter_map_identity.stderr @@ -1,4 +1,4 @@ -error: called `filter_map(|x| x)` on an `Iterator` +error: use of `filter_map` with an identity function --> $DIR/filter_map_identity.rs:8:22 | LL | let _ = iterator.filter_map(|x| x); @@ -6,17 +6,23 @@ LL | let _ = iterator.filter_map(|x| x); | = note: `-D clippy::filter-map-identity` implied by `-D warnings` -error: called `filter_map(std::convert::identity)` on an `Iterator` +error: use of `filter_map` with an identity function --> $DIR/filter_map_identity.rs:11:22 | LL | let _ = iterator.filter_map(std::convert::identity); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `flatten()` -error: called `filter_map(std::convert::identity)` on an `Iterator` +error: use of `filter_map` with an identity function --> $DIR/filter_map_identity.rs:15:22 | LL | let _ = iterator.filter_map(identity); | ^^^^^^^^^^^^^^^^^^^^ help: try: `flatten()` -error: aborting due to 3 previous errors +error: use of `filter_map` with an identity function + --> $DIR/filter_map_identity.rs:18:22 + | +LL | let _ = iterator.filter_map(|x| return x); + | ^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `flatten()` + +error: aborting due to 4 previous errors diff --git a/tests/ui/flat_map_identity.fixed b/tests/ui/flat_map_identity.fixed index dfe3bd47e13..1f4b880ef5b 100644 --- a/tests/ui/flat_map_identity.fixed +++ b/tests/ui/flat_map_identity.fixed @@ -1,6 +1,6 @@ // run-rustfix -#![allow(unused_imports)] +#![allow(unused_imports, clippy::needless_return)] #![warn(clippy::flat_map_identity)] use std::convert; @@ -11,4 +11,7 @@ fn main() { let iterator = [[0, 1], [2, 3], [4, 5]].iter(); let _ = iterator.flatten(); + + let iterator = [[0, 1], [2, 3], [4, 5]].iter(); + let _ = iterator.flatten(); } diff --git a/tests/ui/flat_map_identity.rs b/tests/ui/flat_map_identity.rs index 393b9569255..de14a06d4e6 100644 --- a/tests/ui/flat_map_identity.rs +++ b/tests/ui/flat_map_identity.rs @@ -1,6 +1,6 @@ // run-rustfix -#![allow(unused_imports)] +#![allow(unused_imports, clippy::needless_return)] #![warn(clippy::flat_map_identity)] use std::convert; @@ -11,4 +11,7 @@ fn main() { let iterator = [[0, 1], [2, 3], [4, 5]].iter(); let _ = iterator.flat_map(convert::identity); + + let iterator = [[0, 1], [2, 3], [4, 5]].iter(); + let _ = iterator.flat_map(|x| return x); } diff --git a/tests/ui/flat_map_identity.stderr b/tests/ui/flat_map_identity.stderr index e4686ae5a54..e776c9fdf51 100644 --- a/tests/ui/flat_map_identity.stderr +++ b/tests/ui/flat_map_identity.stderr @@ -1,4 +1,4 @@ -error: called `flat_map(|x| x)` on an `Iterator` +error: use of `flat_map` with an identity function --> $DIR/flat_map_identity.rs:10:22 | LL | let _ = iterator.flat_map(|x| x); @@ -6,11 +6,17 @@ LL | let _ = iterator.flat_map(|x| x); | = note: `-D clippy::flat-map-identity` implied by `-D warnings` -error: called `flat_map(std::convert::identity)` on an `Iterator` +error: use of `flat_map` with an identity function --> $DIR/flat_map_identity.rs:13:22 | LL | let _ = iterator.flat_map(convert::identity); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `flatten()` -error: aborting due to 2 previous errors +error: use of `flat_map` with an identity function + --> $DIR/flat_map_identity.rs:16:22 + | +LL | let _ = iterator.flat_map(|x| return x); + | ^^^^^^^^^^^^^^^^^^^^^^ help: try: `flatten()` + +error: aborting due to 3 previous errors diff --git a/tests/ui/match_same_arms.stderr b/tests/ui/match_same_arms.stderr index 0549886a1e8..e48451acfe8 100644 --- a/tests/ui/match_same_arms.stderr +++ b/tests/ui/match_same_arms.stderr @@ -32,6 +32,7 @@ help: consider refactoring into `(1, .., 3) | (.., 3)` | LL | (1, .., 3) => 42, | ^^^^^^^^^^ + = help: ...or consider changing the match arm bodies error: this `match` has identical arm bodies --> $DIR/match_same_arms.rs:24:15 @@ -49,6 +50,7 @@ help: consider refactoring into `42 | 51` | LL | 42 => 1, | ^^ + = help: ...or consider changing the match arm bodies error: this `match` has identical arm bodies --> $DIR/match_same_arms.rs:26:15 @@ -66,6 +68,7 @@ help: consider refactoring into `41 | 52` | LL | 41 => 2, | ^^ + = help: ...or consider changing the match arm bodies error: this `match` has identical arm bodies --> $DIR/match_same_arms.rs:32:14 @@ -83,6 +86,7 @@ help: consider refactoring into `1 | 2` | LL | 1 => 2, | ^ + = help: ...or consider changing the match arm bodies error: this `match` has identical arm bodies --> $DIR/match_same_arms.rs:33:14 @@ -100,6 +104,7 @@ help: consider refactoring into `1 | 3` | LL | 1 => 2, | ^ + = help: ...or consider changing the match arm bodies error: this `match` has identical arm bodies --> $DIR/match_same_arms.rs:33:14 @@ -117,6 +122,7 @@ help: consider refactoring into `2 | 3` | LL | 2 => 2, //~ ERROR 2nd matched arms have same body | ^ + = help: ...or consider changing the match arm bodies error: this `match` has identical arm bodies --> $DIR/match_same_arms.rs:50:55 @@ -134,6 +140,7 @@ help: consider refactoring into `CommandInfo::BuiltIn { name, .. } | CommandInfo | LL | CommandInfo::BuiltIn { name, .. } => name.to_string(), | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + = help: ...or consider changing the match arm bodies error: aborting due to 8 previous errors diff --git a/tests/ui/match_same_arms2.stderr b/tests/ui/match_same_arms2.stderr index 430021a0f7f..e1ed12f9370 100644 --- a/tests/ui/match_same_arms2.stderr +++ b/tests/ui/match_same_arms2.stderr @@ -53,6 +53,7 @@ help: consider refactoring into `42 | 51` | LL | 42 => foo(), | ^^ + = help: ...or consider changing the match arm bodies error: this `match` has identical arm bodies --> $DIR/match_same_arms2.rs:40:17 @@ -70,6 +71,7 @@ help: consider refactoring into `Some(_) | None` | LL | Some(_) => 24, | ^^^^^^^ + = help: ...or consider changing the match arm bodies error: this `match` has identical arm bodies --> $DIR/match_same_arms2.rs:62:28 @@ -87,6 +89,7 @@ help: consider refactoring into `(Some(a), None) | (None, Some(a))` | LL | (Some(a), None) => bar(a), | ^^^^^^^^^^^^^^^ + = help: ...or consider changing the match arm bodies error: this `match` has identical arm bodies --> $DIR/match_same_arms2.rs:68:26 @@ -104,6 +107,7 @@ help: consider refactoring into `(Some(a), ..) | (.., Some(a))` | LL | (Some(a), ..) => bar(a), | ^^^^^^^^^^^^^ + = help: ...or consider changing the match arm bodies error: this `match` has identical arm bodies --> $DIR/match_same_arms2.rs:102:29 @@ -121,6 +125,7 @@ help: consider refactoring into `(Ok(x), Some(_)) | (Ok(_), Some(x))` | LL | (Ok(x), Some(_)) => println!("ok {}", x), | ^^^^^^^^^^^^^^^^ + = help: ...or consider changing the match arm bodies = note: this error originates in the macro `println` (in Nightly builds, run with -Z macro-backtrace for more info) error: this `match` has identical arm bodies @@ -139,6 +144,7 @@ help: consider refactoring into `Ok(3) | Ok(_)` | LL | Ok(3) => println!("ok"), | ^^^^^ + = help: ...or consider changing the match arm bodies = note: this error originates in the macro `println` (in Nightly builds, run with -Z macro-backtrace for more info) error: this `match` has identical arm bodies @@ -163,6 +169,7 @@ help: consider refactoring into `0 | 1` | LL | 0 => { | ^ + = help: ...or consider changing the match arm bodies error: match expression looks like `matches!` macro --> $DIR/match_same_arms2.rs:162:16 diff --git a/tests/ui/match_wildcard_for_single_variants.fixed b/tests/ui/match_wildcard_for_single_variants.fixed index 31b743f7a18..e675c183ea7 100644 --- a/tests/ui/match_wildcard_for_single_variants.fixed +++ b/tests/ui/match_wildcard_for_single_variants.fixed @@ -115,9 +115,16 @@ fn main() { pub enum Enum { A, B, + C, #[doc(hidden)] __Private, } + match Enum::A { + Enum::A => (), + Enum::B => (), + Enum::C => (), + _ => (), + } match Enum::A { Enum::A => (), Enum::B => (), diff --git a/tests/ui/match_wildcard_for_single_variants.rs b/tests/ui/match_wildcard_for_single_variants.rs index d19e6ab7eb2..38c3ffc00c7 100644 --- a/tests/ui/match_wildcard_for_single_variants.rs +++ b/tests/ui/match_wildcard_for_single_variants.rs @@ -115,9 +115,16 @@ fn main() { pub enum Enum { A, B, + C, #[doc(hidden)] __Private, } + match Enum::A { + Enum::A => (), + Enum::B => (), + Enum::C => (), + _ => (), + } match Enum::A { Enum::A => (), Enum::B => (), diff --git a/tests/ui/mut_key.stderr b/tests/ui/mut_key.stderr index 8d6a259c7e3..a8460b06ca6 100644 --- a/tests/ui/mut_key.stderr +++ b/tests/ui/mut_key.stderr @@ -4,7 +4,7 @@ error: mutable key type LL | fn should_not_take_this_arg(m: &mut HashMap<Key, usize>, _n: usize) -> HashSet<Key> { | ^^^^^^^^^^^^^^^^^^^^^^^^ | - = note: `#[deny(clippy::mutable_key_type)]` on by default + = note: `-D clippy::mutable-key-type` implied by `-D warnings` error: mutable key type --> $DIR/mut_key.rs:27:72 diff --git a/tests/ui/semicolon_if_nothing_returned.rs b/tests/ui/semicolon_if_nothing_returned.rs index 0abe2cca267..79ba7402f1f 100644 --- a/tests/ui/semicolon_if_nothing_returned.rs +++ b/tests/ui/semicolon_if_nothing_returned.rs @@ -17,6 +17,24 @@ fn basic101(x: i32) { y = x + 1 } +#[rustfmt::skip] +fn closure_error() { + let _d = || { + hello() + }; +} + +#[rustfmt::skip] +fn unsafe_checks_error() { + use std::mem::MaybeUninit; + use std::ptr; + + let mut s = MaybeUninit::<String>::uninit(); + let _d = || unsafe { + ptr::drop_in_place(s.as_mut_ptr()) + }; +} + // this is fine fn print_sum(a: i32, b: i32) { println!("{}", a + b); @@ -53,3 +71,29 @@ fn loop_test(x: i32) { println!("{}", ext); } } + +fn closure() { + let _d = || hello(); +} + +#[rustfmt::skip] +fn closure_block() { + let _d = || { hello() }; +} + +unsafe fn some_unsafe_op() {} +unsafe fn some_other_unsafe_fn() {} + +fn do_something() { + unsafe { some_unsafe_op() }; + + unsafe { some_other_unsafe_fn() }; +} + +fn unsafe_checks() { + use std::mem::MaybeUninit; + use std::ptr; + + let mut s = MaybeUninit::<String>::uninit(); + let _d = || unsafe { ptr::drop_in_place(s.as_mut_ptr()) }; +} diff --git a/tests/ui/semicolon_if_nothing_returned.stderr b/tests/ui/semicolon_if_nothing_returned.stderr index b73f8967538..e88ebe2ad35 100644 --- a/tests/ui/semicolon_if_nothing_returned.stderr +++ b/tests/ui/semicolon_if_nothing_returned.stderr @@ -18,5 +18,17 @@ error: consider adding a `;` to the last statement for consistent formatting LL | y = x + 1 | ^^^^^^^^^ help: add a `;` here: `y = x + 1;` -error: aborting due to 3 previous errors +error: consider adding a `;` to the last statement for consistent formatting + --> $DIR/semicolon_if_nothing_returned.rs:23:9 + | +LL | hello() + | ^^^^^^^ help: add a `;` here: `hello();` + +error: consider adding a `;` to the last statement for consistent formatting + --> $DIR/semicolon_if_nothing_returned.rs:34:9 + | +LL | ptr::drop_in_place(s.as_mut_ptr()) + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: add a `;` here: `ptr::drop_in_place(s.as_mut_ptr());` + +error: aborting due to 5 previous errors diff --git a/tests/ui/suspicious_arithmetic_impl.stderr b/tests/ui/suspicious_arithmetic_impl.stderr index 388fc740082..63fc9ecb79a 100644 --- a/tests/ui/suspicious_arithmetic_impl.stderr +++ b/tests/ui/suspicious_arithmetic_impl.stderr @@ -12,7 +12,7 @@ error: suspicious use of binary operator in `AddAssign` impl LL | *self = *self - other; | ^ | - = note: `#[deny(clippy::suspicious_op_assign_impl)]` on by default + = note: `-D clippy::suspicious-op-assign-impl` implied by `-D warnings` error: suspicious use of binary operator in `MulAssign` impl --> $DIR/suspicious_arithmetic_impl.rs:32:16 diff --git a/tests/ui/unnecessary_cast_fixable.fixed b/tests/ui/unnecessary_cast_fixable.fixed index 7fbce58a82f..bda0f2c47cd 100644 --- a/tests/ui/unnecessary_cast_fixable.fixed +++ b/tests/ui/unnecessary_cast_fixable.fixed @@ -1,7 +1,7 @@ // run-rustfix #![warn(clippy::unnecessary_cast)] -#![allow(clippy::no_effect, clippy::unnecessary_operation)] +#![allow(clippy::no_effect, clippy::unnecessary_operation, clippy::nonstandard_macro_braces)] fn main() { // casting integer literal to float is unnecessary diff --git a/tests/ui/unnecessary_cast_fixable.rs b/tests/ui/unnecessary_cast_fixable.rs index a71363ea4d2..f7a4f2a5988 100644 --- a/tests/ui/unnecessary_cast_fixable.rs +++ b/tests/ui/unnecessary_cast_fixable.rs @@ -1,7 +1,7 @@ // run-rustfix #![warn(clippy::unnecessary_cast)] -#![allow(clippy::no_effect, clippy::unnecessary_operation)] +#![allow(clippy::no_effect, clippy::unnecessary_operation, clippy::nonstandard_macro_braces)] fn main() { // casting integer literal to float is unnecessary diff --git a/tests/ui/use_self.fixed b/tests/ui/use_self.fixed index 631da6fe066..e2c28542efc 100644 --- a/tests/ui/use_self.fixed +++ b/tests/ui/use_self.fixed @@ -492,3 +492,26 @@ mod issue7206 { } } } + +mod self_is_ty_param { + trait Trait { + type Type; + type Hi; + + fn test(); + } + + impl<I> Trait for I + where + I: Iterator, + I::Item: Trait, // changing this to Self would require <Self as Iterator> + { + type Type = I; + type Hi = I::Item; + + fn test() { + let _: I::Item; + let _: I; // this could lint, but is questionable + } + } +} diff --git a/tests/ui/use_self.rs b/tests/ui/use_self.rs index 7a10d755faa..3cd99b9f5cd 100644 --- a/tests/ui/use_self.rs +++ b/tests/ui/use_self.rs @@ -279,7 +279,7 @@ mod generics { impl<T> Foo<T> { // `Self` is applicable here fn foo(value: T) -> Foo<T> { - Foo { value } + Foo::<T> { value } } // `Cannot` use `Self` as a return type as the generic types are different @@ -492,3 +492,26 @@ mod issue7206 { } } } + +mod self_is_ty_param { + trait Trait { + type Type; + type Hi; + + fn test(); + } + + impl<I> Trait for I + where + I: Iterator, + I::Item: Trait, // changing this to Self would require <Self as Iterator> + { + type Type = I; + type Hi = I::Item; + + fn test() { + let _: I::Item; + let _: I; // this could lint, but is questionable + } + } +} diff --git a/tests/ui/use_self.stderr b/tests/ui/use_self.stderr index cf6222c9b45..6ac26c9e5a9 100644 --- a/tests/ui/use_self.stderr +++ b/tests/ui/use_self.stderr @@ -153,8 +153,8 @@ LL | fn foo(value: T) -> Foo<T> { error: unnecessary structure name repetition --> $DIR/use_self.rs:282:13 | -LL | Foo { value } - | ^^^ help: use the applicable keyword: `Self` +LL | Foo::<T> { value } + | ^^^^^^^^ help: use the applicable keyword: `Self` error: unnecessary structure name repetition --> $DIR/use_self.rs:454:13 diff --git a/tests/ui/vec.fixed b/tests/ui/vec.fixed index da35f2e5c1b..318f9c2dceb 100644 --- a/tests/ui/vec.fixed +++ b/tests/ui/vec.fixed @@ -1,5 +1,5 @@ // run-rustfix - +#![allow(clippy::nonstandard_macro_braces)] #![warn(clippy::useless_vec)] #[derive(Debug)] diff --git a/tests/ui/vec.rs b/tests/ui/vec.rs index e9ed83e5c5a..d7673ce3e64 100644 --- a/tests/ui/vec.rs +++ b/tests/ui/vec.rs @@ -1,5 +1,5 @@ // run-rustfix - +#![allow(clippy::nonstandard_macro_braces)] #![warn(clippy::useless_vec)] #[derive(Debug)] diff --git a/tests/ui/while_let_on_iterator.fixed b/tests/ui/while_let_on_iterator.fixed index c3e2cf0c4a4..52e80ceee83 100644 --- a/tests/ui/while_let_on_iterator.fixed +++ b/tests/ui/while_let_on_iterator.fixed @@ -320,6 +320,20 @@ fn issue1924() { println!("iterator field {}", it.1); } +fn issue7249() { + let mut it = 0..10; + let mut x = || { + // Needs &mut, the closure can be called multiple times + for x in &mut it { + if x % 2 == 0 { + break; + } + } + }; + x(); + x(); +} + fn main() { let mut it = 0..20; for _ in it { diff --git a/tests/ui/while_let_on_iterator.rs b/tests/ui/while_let_on_iterator.rs index 1717006a449..5078a3c9028 100644 --- a/tests/ui/while_let_on_iterator.rs +++ b/tests/ui/while_let_on_iterator.rs @@ -320,6 +320,20 @@ fn issue1924() { println!("iterator field {}", it.1); } +fn issue7249() { + let mut it = 0..10; + let mut x = || { + // Needs &mut, the closure can be called multiple times + while let Some(x) = it.next() { + if x % 2 == 0 { + break; + } + } + }; + x(); + x(); +} + fn main() { let mut it = 0..20; while let Some(..) = it.next() { diff --git a/tests/ui/while_let_on_iterator.stderr b/tests/ui/while_let_on_iterator.stderr index eff559bef7e..cb0afeae15e 100644 --- a/tests/ui/while_let_on_iterator.stderr +++ b/tests/ui/while_let_on_iterator.stderr @@ -105,10 +105,16 @@ LL | while let Some(n) = it.next() { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `for n in &mut it` error: this loop could be written as a `for` loop - --> $DIR/while_let_on_iterator.rs:325:5 + --> $DIR/while_let_on_iterator.rs:327:9 + | +LL | while let Some(x) = it.next() { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `for x in &mut it` + +error: this loop could be written as a `for` loop + --> $DIR/while_let_on_iterator.rs:339:5 | LL | while let Some(..) = it.next() { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `for _ in it` -error: aborting due to 18 previous errors +error: aborting due to 19 previous errors diff --git a/tests/ui/wildcard_enum_match_arm.fixed b/tests/ui/wildcard_enum_match_arm.fixed index 5ad27bb1450..3ee4ab48ac8 100644 --- a/tests/ui/wildcard_enum_match_arm.fixed +++ b/tests/ui/wildcard_enum_match_arm.fixed @@ -1,4 +1,5 @@ // run-rustfix +// aux-build:non-exhaustive-enum.rs #![deny(clippy::wildcard_enum_match_arm)] #![allow( @@ -11,7 +12,9 @@ clippy::diverging_sub_expression )] -use std::io::ErrorKind; +extern crate non_exhaustive_enum; + +use non_exhaustive_enum::ErrorKind; #[derive(Clone, Copy, Debug, Eq, PartialEq)] enum Color { @@ -77,29 +80,25 @@ fn main() { let error_kind = ErrorKind::NotFound; match error_kind { ErrorKind::NotFound => {}, - ErrorKind::PermissionDenied | ErrorKind::ConnectionRefused | ErrorKind::ConnectionReset | ErrorKind::ConnectionAborted | ErrorKind::NotConnected | ErrorKind::AddrInUse | ErrorKind::AddrNotAvailable | ErrorKind::BrokenPipe | ErrorKind::AlreadyExists | ErrorKind::WouldBlock | ErrorKind::InvalidInput | ErrorKind::InvalidData | ErrorKind::TimedOut | ErrorKind::WriteZero | ErrorKind::Interrupted | ErrorKind::Other | ErrorKind::UnexpectedEof | ErrorKind::Unsupported | ErrorKind::OutOfMemory | _ => {}, + ErrorKind::PermissionDenied | _ => {}, } match error_kind { ErrorKind::NotFound => {}, ErrorKind::PermissionDenied => {}, - ErrorKind::ConnectionRefused => {}, - ErrorKind::ConnectionReset => {}, - ErrorKind::ConnectionAborted => {}, - ErrorKind::NotConnected => {}, - ErrorKind::AddrInUse => {}, - ErrorKind::AddrNotAvailable => {}, - ErrorKind::BrokenPipe => {}, - ErrorKind::AlreadyExists => {}, - ErrorKind::WouldBlock => {}, - ErrorKind::InvalidInput => {}, - ErrorKind::InvalidData => {}, - ErrorKind::TimedOut => {}, - ErrorKind::WriteZero => {}, - ErrorKind::Interrupted => {}, - ErrorKind::Other => {}, - ErrorKind::UnexpectedEof => {}, - ErrorKind::Unsupported => {}, - ErrorKind::OutOfMemory => {}, _ => {}, } + + { + #![allow(clippy::manual_non_exhaustive)] + pub enum Enum { + A, + B, + #[doc(hidden)] + __Private, + } + match Enum::A { + Enum::A => (), + Enum::B | _ => (), + } + } } diff --git a/tests/ui/wildcard_enum_match_arm.rs b/tests/ui/wildcard_enum_match_arm.rs index adca0738bba..46886550453 100644 --- a/tests/ui/wildcard_enum_match_arm.rs +++ b/tests/ui/wildcard_enum_match_arm.rs @@ -1,4 +1,5 @@ // run-rustfix +// aux-build:non-exhaustive-enum.rs #![deny(clippy::wildcard_enum_match_arm)] #![allow( @@ -11,7 +12,9 @@ clippy::diverging_sub_expression )] -use std::io::ErrorKind; +extern crate non_exhaustive_enum; + +use non_exhaustive_enum::ErrorKind; #[derive(Clone, Copy, Debug, Eq, PartialEq)] enum Color { @@ -82,24 +85,20 @@ fn main() { match error_kind { ErrorKind::NotFound => {}, ErrorKind::PermissionDenied => {}, - ErrorKind::ConnectionRefused => {}, - ErrorKind::ConnectionReset => {}, - ErrorKind::ConnectionAborted => {}, - ErrorKind::NotConnected => {}, - ErrorKind::AddrInUse => {}, - ErrorKind::AddrNotAvailable => {}, - ErrorKind::BrokenPipe => {}, - ErrorKind::AlreadyExists => {}, - ErrorKind::WouldBlock => {}, - ErrorKind::InvalidInput => {}, - ErrorKind::InvalidData => {}, - ErrorKind::TimedOut => {}, - ErrorKind::WriteZero => {}, - ErrorKind::Interrupted => {}, - ErrorKind::Other => {}, - ErrorKind::UnexpectedEof => {}, - ErrorKind::Unsupported => {}, - ErrorKind::OutOfMemory => {}, _ => {}, } + + { + #![allow(clippy::manual_non_exhaustive)] + pub enum Enum { + A, + B, + #[doc(hidden)] + __Private, + } + match Enum::A { + Enum::A => (), + _ => (), + } + } } diff --git a/tests/ui/wildcard_enum_match_arm.stderr b/tests/ui/wildcard_enum_match_arm.stderr index 73f6a4a80c9..d63f2090353 100644 --- a/tests/ui/wildcard_enum_match_arm.stderr +++ b/tests/ui/wildcard_enum_match_arm.stderr @@ -1,38 +1,44 @@ error: wildcard match will also match any future added variants - --> $DIR/wildcard_enum_match_arm.rs:39:9 + --> $DIR/wildcard_enum_match_arm.rs:42:9 | LL | _ => eprintln!("Not red"), | ^ help: try this: `Color::Green | Color::Blue | Color::Rgb(..) | Color::Cyan` | note: the lint level is defined here - --> $DIR/wildcard_enum_match_arm.rs:3:9 + --> $DIR/wildcard_enum_match_arm.rs:4:9 | LL | #![deny(clippy::wildcard_enum_match_arm)] | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: wildcard match will also match any future added variants - --> $DIR/wildcard_enum_match_arm.rs:43:9 + --> $DIR/wildcard_enum_match_arm.rs:46:9 | LL | _not_red => eprintln!("Not red"), | ^^^^^^^^ help: try this: `_not_red @ Color::Green | _not_red @ Color::Blue | _not_red @ Color::Rgb(..) | _not_red @ Color::Cyan` error: wildcard match will also match any future added variants - --> $DIR/wildcard_enum_match_arm.rs:47:9 + --> $DIR/wildcard_enum_match_arm.rs:50:9 | LL | not_red => format!("{:?}", not_red), | ^^^^^^^ help: try this: `not_red @ Color::Green | not_red @ Color::Blue | not_red @ Color::Rgb(..) | not_red @ Color::Cyan` error: wildcard match will also match any future added variants - --> $DIR/wildcard_enum_match_arm.rs:63:9 + --> $DIR/wildcard_enum_match_arm.rs:66:9 | LL | _ => "No red", | ^ help: try this: `Color::Red | Color::Green | Color::Blue | Color::Rgb(..) | Color::Cyan` error: wildcard matches known variants and will also match future added variants - --> $DIR/wildcard_enum_match_arm.rs:80:9 + --> $DIR/wildcard_enum_match_arm.rs:83:9 | LL | _ => {}, - | ^ help: try this: `ErrorKind::PermissionDenied | ErrorKind::ConnectionRefused | ErrorKind::ConnectionReset | ErrorKind::ConnectionAborted | ErrorKind::NotConnected | ErrorKind::AddrInUse | ErrorKind::AddrNotAvailable | ErrorKind::BrokenPipe | ErrorKind::AlreadyExists | ErrorKind::WouldBlock | ErrorKind::InvalidInput | ErrorKind::InvalidData | ErrorKind::TimedOut | ErrorKind::WriteZero | ErrorKind::Interrupted | ErrorKind::Other | ErrorKind::UnexpectedEof | ErrorKind::Unsupported | ErrorKind::OutOfMemory | _` + | ^ help: try this: `ErrorKind::PermissionDenied | _` -error: aborting due to 5 previous errors +error: wildcard matches known variants and will also match future added variants + --> $DIR/wildcard_enum_match_arm.rs:101:13 + | +LL | _ => (), + | ^ help: try this: `Enum::B | _` + +error: aborting due to 6 previous errors diff --git a/tests/ui/zero_offset.rs b/tests/ui/zero_offset.rs index 2de904376ad..6c190a4c86c 100644 --- a/tests/ui/zero_offset.rs +++ b/tests/ui/zero_offset.rs @@ -1,12 +1,18 @@ fn main() { unsafe { - let x = &() as *const (); - x.offset(0); - x.wrapping_add(0); - x.sub(0); - x.wrapping_sub(0); + let m = &mut () as *mut (); + m.offset(0); + m.wrapping_add(0); + m.sub(0); + m.wrapping_sub(0); - let y = &1 as *const u8; - y.offset(0); + let c = &() as *const (); + c.offset(0); + c.wrapping_add(0); + c.sub(0); + c.wrapping_sub(0); + + let sized = &1 as *const i32; + sized.offset(0); } } diff --git a/tests/ui/zero_offset.stderr b/tests/ui/zero_offset.stderr index cfcd7de2b3d..b12c8e9a73c 100644 --- a/tests/ui/zero_offset.stderr +++ b/tests/ui/zero_offset.stderr @@ -1,9 +1,52 @@ -error[E0606]: casting `&i32` as `*const u8` is invalid - --> $DIR/zero_offset.rs:9:17 +error: offset calculation on zero-sized value + --> $DIR/zero_offset.rs:4:9 | -LL | let y = &1 as *const u8; - | ^^^^^^^^^^^^^^^ +LL | m.offset(0); + | ^^^^^^^^^^^ + | + = note: `#[deny(clippy::zst_offset)]` on by default -error: aborting due to previous error +error: offset calculation on zero-sized value + --> $DIR/zero_offset.rs:5:9 + | +LL | m.wrapping_add(0); + | ^^^^^^^^^^^^^^^^^ + +error: offset calculation on zero-sized value + --> $DIR/zero_offset.rs:6:9 + | +LL | m.sub(0); + | ^^^^^^^^ + +error: offset calculation on zero-sized value + --> $DIR/zero_offset.rs:7:9 + | +LL | m.wrapping_sub(0); + | ^^^^^^^^^^^^^^^^^ + +error: offset calculation on zero-sized value + --> $DIR/zero_offset.rs:10:9 + | +LL | c.offset(0); + | ^^^^^^^^^^^ + +error: offset calculation on zero-sized value + --> $DIR/zero_offset.rs:11:9 + | +LL | c.wrapping_add(0); + | ^^^^^^^^^^^^^^^^^ + +error: offset calculation on zero-sized value + --> $DIR/zero_offset.rs:12:9 + | +LL | c.sub(0); + | ^^^^^^^^ + +error: offset calculation on zero-sized value + --> $DIR/zero_offset.rs:13:9 + | +LL | c.wrapping_sub(0); + | ^^^^^^^^^^^^^^^^^ + +error: aborting due to 8 previous errors -For more information about this error, try `rustc --explain E0606`. diff --git a/util/etc/pre-commit.sh b/util/etc/pre-commit.sh new file mode 100755 index 00000000000..528f8953b25 --- /dev/null +++ b/util/etc/pre-commit.sh @@ -0,0 +1,21 @@ +#!/bin/sh + +# hide output +set -e + +# Update lints +cargo dev update_lints +git add clippy_lints/src/lib.rs + +# Formatting: +# Git will not automatically add the formatted code to the staged changes once +# fmt was executed. This collects all staged files rs files that are currently staged. +# They will later be added back. +# +# This was proudly stolen and adjusted from here: +# https://medium.com/@harshitbangar/automatic-code-formatting-with-git-66c3c5c26798 +files=$( (git diff --cached --name-only --diff-filter=ACMR | grep -Ei "\.rs$") || true) +if [ ! -z "${files}" ]; then + cargo dev fmt + git add $(echo "$files" | paste -s -d " " -) +fi diff --git a/util/etc/vscode-tasks.json b/util/etc/vscode-tasks.json new file mode 100644 index 00000000000..a4bb26b9f90 --- /dev/null +++ b/util/etc/vscode-tasks.json @@ -0,0 +1,57 @@ +{ + "version": "2.0.0", + "tasks": [ + { + "label": "cargo check", + "type": "shell", + "command": "cargo check", + "problemMatcher": [], + "group": { + "kind": "build", + "isDefault": true, + }, + }, + { + "label": "cargo dev fmt", + "type": "shell", + "command": "cargo dev fmt", + "problemMatcher": [], + "group": "none", + }, + { + "label": "cargo uitest", + "type": "shell", + "command": "cargo uitest", + "options": { + "env": { + "RUST_BACKTRACE": "1", + // This task will usually execute all UI tests inside `tests/ui` you can + // optionally uncomment the line below and only run a specific test. + // + // See: https://github.com/rust-lang/rust-clippy/blob/master/doc/adding_lints.md#testing + // + // "TESTNAME": "<TODO>", + }, + }, + "problemMatcher": [], + "group": { + "kind": "test", + "isDefault": true, + } + }, + { + "label": "cargo test", + "type": "shell", + "command": "cargo test", + "problemMatcher": [], + "group": "test", + }, + { + "label": "cargo dev bless", + "type": "shell", + "command": "cargo dev bless", + "problemMatcher": [], + "group": "none", + }, + ], +} diff --git a/util/lintlib.py b/util/lintlib.py index 5707cf0ce0f..9cefb2dbb19 100644 --- a/util/lintlib.py +++ b/util/lintlib.py @@ -12,13 +12,14 @@ Config = collections.namedtuple('Config', 'name ty doc default') lintname_re = re.compile(r'''pub\s+([A-Z_][A-Z_0-9]*)''') group_re = re.compile(r'''\s*([a-z_][a-z_0-9]+)''') -conf_re = re.compile(r'''define_Conf! {\n([^}]*)\n}''', re.MULTILINE) +conf_re = re.compile(r'''define_Conf! {\n((?!\n})[\s\S])*\n}''', re.MULTILINE) confvar_re = re.compile( r'''/// Lint: ([\w,\s]+)\. (.*)\n\s*\(([^:]+):\s*([^\s=]+)\s*=\s*([^\.\)]+).*\),''', re.MULTILINE) comment_re = re.compile(r'''\s*/// ?(.*)''') lint_levels = { "correctness": 'Deny', + "suspicious": 'Warn', "style": 'Warn', "complexity": 'Warn', "perf": 'Warn', @@ -91,7 +92,7 @@ def parse_configs(path): contents = fp.read() match = re.search(conf_re, contents) - confvars = re.findall(confvar_re, match.group(1)) + confvars = re.findall(confvar_re, match.group(0)) for (lints, doc, name, ty, default) in confvars: for lint in lints.split(','):