diff --git a/Cargo.lock b/Cargo.lock index eb3f294e5cec7..8db8a56eaa8ec 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -180,7 +180,7 @@ dependencies = [ "merge", "num_cpus", "once_cell", - "opener 0.5.0", + "opener", "pretty_assertions", "serde", "serde_json", @@ -290,7 +290,7 @@ dependencies = [ "log", "memchr", "num_cpus", - "opener 0.5.0", + "opener", "openssl", "percent-encoding 2.1.0", "pretty_env_logger", @@ -553,7 +553,7 @@ dependencies = [ [[package]] name = "clippy" -version = "0.1.54" +version = "0.1.55" dependencies = [ "cargo_metadata 0.12.0", "clippy_lints", @@ -578,7 +578,7 @@ dependencies = [ "bytecount", "clap", "itertools 0.9.0", - "opener 0.4.1", + "opener", "regex", "shell-escape", "walkdir", @@ -586,7 +586,7 @@ dependencies = [ [[package]] name = "clippy_lints" -version = "0.1.54" +version = "0.1.55" dependencies = [ "cargo_metadata 0.12.0", "clippy_utils", @@ -601,12 +601,13 @@ dependencies = [ "serde_json", "toml", "unicode-normalization", + "unicode-script", "url 2.2.2", ] [[package]] name = "clippy_utils" -version = "0.1.54" +version = "0.1.55" dependencies = [ "if_chain", "itertools 0.9.0", @@ -2413,15 +2414,6 @@ dependencies = [ "winapi 0.3.9", ] -[[package]] -name = "opener" -version = "0.4.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "13117407ca9d0caf3a0e74f97b490a7e64c0ae3aa90a8b7085544d0c37b6f3ae" -dependencies = [ - "winapi 0.3.9", -] - [[package]] name = "opener" version = "0.5.0" @@ -5497,9 +5489,9 @@ dependencies = [ [[package]] name = "unicode-script" -version = "0.5.2" +version = "0.5.3" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "79bf4d5fc96546fdb73f9827097810bbda93b11a6770ff3a54e1f445d4135787" +checksum = "098ec66172ce21cd55f8bcc786ee209dd20e04eff70acfca30cb79924d173ae9" [[package]] name = "unicode-security" diff --git a/src/tools/clippy/.github/workflows/clippy.yml b/src/tools/clippy/.github/workflows/clippy.yml index 32103f59d8b0e..d856c55a41a4b 100644 --- a/src/tools/clippy/.github/workflows/clippy.yml +++ b/src/tools/clippy/.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/src/tools/clippy/.github/workflows/clippy_bors.yml b/src/tools/clippy/.github/workflows/clippy_bors.yml index f27fee87dc165..146b6fccd0c76 100644 --- a/src/tools/clippy/.github/workflows/clippy_bors.yml +++ b/src/tools/clippy/.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/src/tools/clippy/.github/workflows/remark.yml b/src/tools/clippy/.github/workflows/remark.yml index 4f25a86b2e4df..77efdec1e50db 100644 --- a/src/tools/clippy/.github/workflows/remark.yml +++ b/src/tools/clippy/.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/src/tools/clippy/.remarkrc b/src/tools/clippy/.remarkrc index 0ede7ac75cb6f..04b82b8cc581f 100644 --- a/src/tools/clippy/.remarkrc +++ b/src/tools/clippy/.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/src/tools/clippy/CHANGELOG.md b/src/tools/clippy/CHANGELOG.md index 41af8e190ddf3..f3a8070323879 100644 --- a/src/tools/clippy/CHANGELOG.md +++ b/src/tools/clippy/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 + +- [`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 `#[ = ""]` 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 [`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/src/tools/clippy/CONTRIBUTING.md b/src/tools/clippy/CONTRIBUTING.md index 7d7b7c811738e..4273fda4e640d 100644 --- a/src/tools/clippy/CONTRIBUTING.md +++ b/src/tools/clippy/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 ` where `` is a path to the rustc repo +Run `cargo dev setup intellij --repo-path ` where `` 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/src/tools/clippy/Cargo.toml b/src/tools/clippy/Cargo.toml index b003b15a11d75..9b5d9b2adf3b2 100644 --- a/src/tools/clippy/Cargo.toml +++ b/src/tools/clippy/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/src/tools/clippy/README.md b/src/tools/clippy/README.md index bd322cc80702a..e1c968273cdf6 100644 --- a/src/tools/clippy/README.md +++ b/src/tools/clippy/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/src/tools/clippy/clippy_dev/Cargo.toml b/src/tools/clippy/clippy_dev/Cargo.toml index b1844e29b3273..5c6c106e0e638 100644 --- a/src/tools/clippy/clippy_dev/Cargo.toml +++ b/src/tools/clippy/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/src/tools/clippy/clippy_dev/src/fmt.rs b/src/tools/clippy/clippy_dev/src/fmt.rs index 1517cdc941962..c81eb40d52f35 100644 --- a/src/tools/clippy/clippy_dev/src/fmt.rs +++ b/src/tools/clippy/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/src/tools/clippy/clippy_dev/src/ide_setup.rs b/src/tools/clippy/clippy_dev/src/ide_setup.rs deleted file mode 100644 index defb1133e44e6..0000000000000 --- a/src/tools/clippy/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/src/tools/clippy/clippy_dev/src/lib.rs b/src/tools/clippy/clippy_dev/src/lib.rs index 69f42aca8b690..72bdaf8d59282 100644 --- a/src/tools/clippy/clippy_dev/src/lib.rs +++ b/src/tools/clippy/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/src/tools/clippy/clippy_dev/src/main.rs b/src/tools/clippy/clippy_dev/src/main.rs index 7040c257c831b..ff324ff6ee6ff 100644 --- a/src/tools/clippy/clippy_dev/src/main.rs +++ b/src/tools/clippy/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/src/tools/clippy/clippy_dev/src/setup/git_hook.rs b/src/tools/clippy/clippy_dev/src/setup/git_hook.rs new file mode 100644 index 0000000000000..3fbb77d59235c --- /dev/null +++ b/src/tools/clippy/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/src/tools/clippy/clippy_dev/src/setup/intellij.rs b/src/tools/clippy/clippy_dev/src/setup/intellij.rs new file mode 100644 index 0000000000000..bf741e6d1217f --- /dev/null +++ b/src/tools/clippy/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 { + 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 { + 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/src/tools/clippy/clippy_dev/src/setup/mod.rs b/src/tools/clippy/clippy_dev/src/setup/mod.rs new file mode 100644 index 0000000000000..a1e4dd103b88b --- /dev/null +++ b/src/tools/clippy/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/src/tools/clippy/clippy_dev/src/setup/vscode.rs b/src/tools/clippy/clippy_dev/src/setup/vscode.rs new file mode 100644 index 0000000000000..d59001b2c66af --- /dev/null +++ b/src/tools/clippy/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/src/tools/clippy/clippy_dev/src/update_lints.rs b/src/tools/clippy/clippy_dev/src/update_lints.rs index edf6c5f57a497..db467c26f1546 100644 --- a/src/tools/clippy/clippy_dev/src/update_lints.rs +++ b/src/tools/clippy/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/src/tools/clippy/clippy_lints/Cargo.toml b/src/tools/clippy/clippy_lints/Cargo.toml index 48f2972ec58d1..42cf7547f5194 100644 --- a/src/tools/clippy/clippy_lints/Cargo.toml +++ b/src/tools/clippy/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/src/tools/clippy/clippy_lints/src/assign_ops.rs b/src/tools/clippy/clippy_lints/src/assign_ops.rs index bc6eec0051a41..a8c527fe2e353 100644 --- a/src/tools/clippy/clippy_lints/src/assign_ops.rs +++ b/src/tools/clippy/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/src/tools/clippy/clippy_lints/src/attrs.rs b/src/tools/clippy/clippy_lints/src/attrs.rs index 932cd58bf6259..f272ed010a1b0 100644 --- a/src/tools/clippy/clippy_lints/src/attrs.rs +++ b/src/tools/clippy/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/src/tools/clippy/clippy_lints/src/blacklisted_name.rs b/src/tools/clippy/clippy_lints/src/blacklisted_name.rs index b26ef33e05698..8eb94f3c28e44 100644 --- a/src/tools/clippy/clippy_lints/src/blacklisted_name.rs +++ b/src/tools/clippy/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, + test_modules_deep: u32, } impl BlacklistedName { pub fn new(blacklist: FxHashSet) -> 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/src/tools/clippy/clippy_lints/src/bytecount.rs b/src/tools/clippy/clippy_lints/src/bytecount.rs index 877ae002d36e9..4f7ffdcdfb499 100644 --- a/src/tools/clippy/clippy_lints/src/bytecount.rs +++ b/src/tools/clippy/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 { - 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/src/tools/clippy/clippy_lints/src/collapsible_match.rs b/src/tools/clippy/clippy_lints/src/collapsible_match.rs index ab22578abd674..a6c3a5b0e83c4 100644 --- a/src/tools/clippy/clippy_lints/src/collapsible_match.rs +++ b/src/tools/clippy/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 { .. } - 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/src/tools/clippy/clippy_lints/src/default.rs b/src/tools/clippy/clippy_lints/src/default.rs index 7a53d390bb45f..947479db8f5d7 100644 --- a/src/tools/clippy/clippy_lints/src/default.rs +++ b/src/tools/clippy/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/src/tools/clippy/clippy_lints/src/default_numeric_fallback.rs b/src/tools/clippy/clippy_lints/src/default_numeric_fallback.rs index 759f7d4062d44..a125376bffa9f 100644 --- a/src/tools/clippy/clippy_lints/src/default_numeric_fallback.rs +++ b/src/tools/clippy/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/src/tools/clippy/clippy_lints/src/deprecated_lints.rs b/src/tools/clippy/clippy_lints/src/deprecated_lints.rs index 04f3d77464f98..2933fbc93418a 100644 --- a/src/tools/clippy/clippy_lints/src/deprecated_lints.rs +++ b/src/tools/clippy/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/src/tools/clippy/clippy_lints/src/derive.rs b/src/tools/clippy/clippy_lints/src/derive.rs index 729941f345a92..3ac20fd9849ee 100644 --- a/src/tools/clippy/clippy_lints/src/derive.rs +++ b/src/tools/clippy/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/src/tools/clippy/clippy_lints/src/disallowed_method.rs b/src/tools/clippy/clippy_lints/src/disallowed_method.rs index ded0a0fff54b2..aa1a609afedc0 100644 --- a/src/tools/clippy/clippy_lints/src/disallowed_method.rs +++ b/src/tools/clippy/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>, + def_ids: FxHashSet<(DefId, Vec)>, } impl DisallowedMethod { @@ -61,6 +55,7 @@ impl DisallowedMethod { .iter() .map(|s| s.split("::").map(|seg| Symbol::intern(seg)).collect::>()) .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::>(); + if let Res::Def(_, id) = clippy_utils::path_to_res(cx, &segs.iter().map(String::as_str).collect::>()) + { + 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/src/tools/clippy/clippy_lints/src/disallowed_script_idents.rs b/src/tools/clippy/clippy_lints/src/disallowed_script_idents.rs new file mode 100644 index 0000000000000..12c525634c51d --- /dev/null +++ b/src/tools/clippy/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