From b7fc905fceecacd660e658ef3bc5c7ace276e2c5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matthias=20Kr=C3=BCger?= Date: Thu, 12 Mar 2020 21:47:47 +0100 Subject: [PATCH] submodules: update clippy from 329923ed to 8485d40a Changes: ```` Rustup to rust-lang/rust#69674 Use visit_place Check for mutation Only fires on temporaries Extend `redundant_clone` to the case that cloned value is not consumed add CR feedback Improve documentation Use `edition:2018` flag more widely Update tests/ui/needless_doc_main.rs Move links to the end of each chapter on adding_lints Move links to the end of each chapter on CONTRIBUTING Clean-up adding_lints.md Clean-up CONTRIBUTING.md needless_doc_main: only check rust code Use `node_type_opt` over `node_type` Fix doc Fix ICE with trivial_bounds feature clippy_lints: readme: don't mention crates.io since it is no longer used to publish clippy. update rust-lang.github.io to rustc-dev-guide.rust-lang.org Improve placeholder in map_unit_fn Fix match single binding when in a let stmt Improve error messages for {option,result}_map_unit_fn Mention the setup instructions in CONTRIBUTING redundant_pattern: take binding (ref, ref mut) into account in suggestion. check_pat: delay creation of the "normal" vec until we reach the branch where is is actually needed deps: bump itertools 0.8 -> 0.9 add lint on File::read_to_string and File::read_to_end transition rustc-guide to rustc-dev-guide Rename macro_use_import -> macro_use_imports warn on macro_use attr Fix deploy script for tag deploys ```` Fixes #69957 --- .github/deploy.sh | 14 +- CHANGELOG.md | 2 + CONTRIBUTING.md | 159 +++++++++--------- README.md | 2 +- clippy_lints/Cargo.toml | 2 +- clippy_lints/README.md | 4 +- clippy_lints/src/attrs.rs | 4 +- clippy_lints/src/doc.rs | 24 ++- clippy_lints/src/functions.rs | 2 +- clippy_lints/src/implicit_return.rs | 8 +- clippy_lints/src/inline_fn_without_body.rs | 2 +- clippy_lints/src/lib.rs | 9 + clippy_lints/src/lifetimes.rs | 2 +- clippy_lints/src/macro_use.rs | 53 ++++++ clippy_lints/src/map_unit_fn.rs | 6 +- clippy_lints/src/matches.rs | 54 ++++-- clippy_lints/src/methods/mod.rs | 2 +- clippy_lints/src/misc_early.rs | 39 +++-- clippy_lints/src/missing_const_for_fn.rs | 7 +- clippy_lints/src/missing_doc.rs | 2 +- clippy_lints/src/missing_inline.rs | 2 +- clippy_lints/src/mut_key.rs | 2 +- clippy_lints/src/ptr.rs | 2 +- clippy_lints/src/redundant_clone.rs | 118 ++++++++----- clippy_lints/src/shadow.rs | 12 +- .../src/trivially_copy_pass_by_ref.rs | 2 +- clippy_lints/src/types.rs | 4 +- clippy_lints/src/utils/mod.rs | 23 ++- clippy_lints/src/utils/paths.rs | 1 + clippy_lints/src/verbose_file_reads.rs | 83 +++++++++ doc/adding_lints.md | 150 +++++++++-------- src/lintlist/mod.rs | 16 +- tests/ui/crashes/shadow.rs | 6 + tests/ui/crashes/trivial_bounds.rs | 13 ++ tests/ui/doc_errors.rs | 2 +- tests/ui/format.fixed | 2 +- tests/ui/format.rs | 2 +- tests/ui/issue_4266.rs | 2 +- tests/ui/macro_use_imports.rs | 11 ++ tests/ui/macro_use_imports.stderr | 10 ++ tests/ui/match_single_binding.fixed | 9 +- tests/ui/match_single_binding.rs | 10 +- tests/ui/match_single_binding.stderr | 34 ++-- tests/ui/methods.rs | 2 +- tests/ui/needless_doc_main.rs | 36 +++- tests/ui/needless_doc_main.stderr | 14 +- tests/ui/option_map_unit_fn_fixable.fixed | 8 +- tests/ui/option_map_unit_fn_fixable.rs | 8 +- tests/ui/option_map_unit_fn_fixable.stderr | 80 +++++---- tests/ui/patterns.fixed | 16 ++ tests/ui/patterns.rs | 16 ++ tests/ui/patterns.stderr | 14 +- tests/ui/redundant_clone.fixed | 24 +++ tests/ui/redundant_clone.rs | 24 +++ tests/ui/redundant_clone.stderr | 30 +++- tests/ui/result_map_unit_fn_fixable.stderr | 34 ++-- tests/ui/single_component_path_imports.fixed | 2 +- tests/ui/single_component_path_imports.rs | 2 +- tests/ui/use_self.fixed | 2 +- tests/ui/use_self.rs | 2 +- tests/ui/verbose_file_reads.rs | 28 +++ tests/ui/verbose_file_reads.stderr | 19 +++ 62 files changed, 928 insertions(+), 346 deletions(-) create mode 100644 clippy_lints/src/macro_use.rs create mode 100644 clippy_lints/src/verbose_file_reads.rs create mode 100644 tests/ui/crashes/shadow.rs create mode 100644 tests/ui/crashes/trivial_bounds.rs create mode 100644 tests/ui/macro_use_imports.rs create mode 100644 tests/ui/macro_use_imports.stderr create mode 100644 tests/ui/verbose_file_reads.rs create mode 100644 tests/ui/verbose_file_reads.stderr diff --git a/.github/deploy.sh b/.github/deploy.sh index b1f572d2b455..9ef9678ee92d 100644 --- a/.github/deploy.sh +++ b/.github/deploy.sh @@ -33,7 +33,17 @@ if git diff --exit-code --quiet; then exit 0 fi -git add . -git commit -m "Automatic deploy to GitHub Pages: ${SHA}" +if [[ -n $TAG_NAME ]]; then + # Add the new dir + git add $TAG_NAME + # Update the symlink + git add stable + # Update versions file + git add versions.json + git commit -m "Add documentation for ${TAG_NAME} release: ${SHA}" +else + git add . + git commit -m "Automatic deploy to GitHub Pages: ${SHA}" +fi git push "$SSH_REPO" "$TARGET_BRANCH" diff --git a/CHANGELOG.md b/CHANGELOG.md index 4d8e24943897..32cbbe801017 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1209,6 +1209,7 @@ Released 2018-09-13 [`linkedlist`]: https://rust-lang.github.io/rust-clippy/master/index.html#linkedlist [`logic_bug`]: https://rust-lang.github.io/rust-clippy/master/index.html#logic_bug [`lossy_float_literal`]: https://rust-lang.github.io/rust-clippy/master/index.html#lossy_float_literal +[`macro_use_imports`]: https://rust-lang.github.io/rust-clippy/master/index.html#macro_use_imports [`main_recursion`]: https://rust-lang.github.io/rust-clippy/master/index.html#main_recursion [`manual_memcpy`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_memcpy [`manual_saturating_arithmetic`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_saturating_arithmetic @@ -1416,6 +1417,7 @@ Released 2018-09-13 [`useless_vec`]: https://rust-lang.github.io/rust-clippy/master/index.html#useless_vec [`vec_box`]: https://rust-lang.github.io/rust-clippy/master/index.html#vec_box [`verbose_bit_mask`]: https://rust-lang.github.io/rust-clippy/master/index.html#verbose_bit_mask +[`verbose_file_reads`]: https://rust-lang.github.io/rust-clippy/master/index.html#verbose_file_reads [`while_immutable_condition`]: https://rust-lang.github.io/rust-clippy/master/index.html#while_immutable_condition [`while_let_loop`]: https://rust-lang.github.io/rust-clippy/master/index.html#while_let_loop [`while_let_on_iterator`]: https://rust-lang.github.io/rust-clippy/master/index.html#while_let_on_iterator diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 4777f2fabeb2..b1f9be44b73d 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -8,10 +8,9 @@ something. We appreciate any sort of contributions, and don't want a wall of rul Clippy welcomes contributions from everyone. There are many ways to contribute to Clippy and the following document explains how you can contribute and how to get started. If you have any questions about contributing or need help with -anything, feel free to ask questions on issues or visit the `#clippy` IRC channel on `irc.mozilla.org` or meet us in -`#clippy` on [Discord](https://discord.gg/rust-lang). +anything, feel free to ask questions on issues or visit the `#clippy` on [Discord]. -All contributors are expected to follow the [Rust Code of Conduct](http://www.rust-lang.org/conduct.html). +All contributors are expected to follow the [Rust Code of Conduct]. * [Getting started](#getting-started) * [Finding something to fix/improve](#finding-something-to-fiximprove) @@ -22,70 +21,81 @@ All contributors are expected to follow the [Rust Code of Conduct](http://www.ru * [Bors and Homu](#bors-and-homu) * [Contributions](#contributions) +[Discord]: https://discord.gg/rust-lang +[Rust Code of Conduct]: https://www.rust-lang.org/policies/code-of-conduct + ## Getting started High level approach: 1. Find something to fix/improve 2. Change code (likely some file in `clippy_lints/src/`) -3. Run `cargo test` in the root directory and wiggle code until it passes -4. Open a PR (also can be done between 2. and 3. if you run into problems) +3. Follow the instructions in the [docs for writing lints](doc/adding_lints.md) such as running the `setup-toolchain.sh` script +4. Run `cargo test` in the root directory and wiggle code until it passes +5. Open a PR (also can be done after 2. if you run into problems) ### Finding something to fix/improve All issues on Clippy are mentored, if you want help with a bug just ask @Manishearth, @llogiq, @mcarton or @oli-obk. -Some issues are easier than others. The [`good first issue`](https://github.com/rust-lang/rust-clippy/labels/good%20first%20issue) -label can be used to find the easy issues. If you want to work on an issue, please leave a comment -so that we can assign it to you! +Some issues are easier than others. The [`good first issue`] label can be used to find the easy issues. +If you want to work on an issue, please leave a comment so that we can assign it to you! -There are also some abandoned PRs, marked with -[`S-inactive-closed`](https://github.com/rust-lang/rust-clippy/pulls?q=is%3Aclosed+label%3AS-inactive-closed). +There are also some abandoned PRs, marked with [`S-inactive-closed`]. Pretty often these PRs are nearly completed and just need some extra steps (formatting, addressing review comments, ...) to be merged. If you want to complete such a PR, please leave a comment in the PR and open a new one based on it. -Issues marked [`T-AST`](https://github.com/rust-lang/rust-clippy/labels/T-AST) involve simple -matching of the syntax tree structure, and are generally easier than -[`T-middle`](https://github.com/rust-lang/rust-clippy/labels/T-middle) issues, which involve types +Issues marked [`T-AST`] involve simple matching of the syntax tree structure, +and are generally easier than [`T-middle`] issues, which involve types and resolved paths. -[`T-AST`](https://github.com/rust-lang/rust-clippy/labels/T-AST) issues will generally need you to match against a -predefined syntax structure. To figure out how this syntax structure is encoded in the AST, it is recommended to run -`rustc -Z ast-json` on an example of the structure and compare with the [nodes in the AST -docs](https://doc.rust-lang.org/nightly/nightly-rustc/syntax/ast). Usually the lint will end up to be a nested series of -matches and ifs, [like -so](https://github.com/rust-lang/rust-clippy/blob/de5ccdfab68a5e37689f3c950ed1532ba9d652a0/src/misc.rs#L34). +[`T-AST`] issues will generally need you to match against a predefined syntax structure. +To figure out how this syntax structure is encoded in the AST, it is recommended to run +`rustc -Z ast-json` on an example of the structure and compare with the [nodes in the AST docs]. +Usually the lint will end up to be a nested series of matches and ifs, [like so][deep-nesting]. +But we can make it nest-less by using [if_chain] macro, [like this][nest-less]. -[`E-medium`](https://github.com/rust-lang/rust-clippy/labels/E-medium) issues are generally -pretty easy too, though it's recommended you work on an E-easy issue first. They are mostly classified -as `E-medium`, since they might be somewhat involved code wise, but not difficult per-se. +[`E-medium`] issues are generally pretty easy too, though it's recommended you work on an E-easy issue first. +They are mostly classified as [`E-medium`], since they might be somewhat involved code wise, +but not difficult per-se. -[`T-middle`](https://github.com/rust-lang/rust-clippy/labels/T-middle) issues can -be more involved and require verifying types. The -[`ty`](https://doc.rust-lang.org/nightly/nightly-rustc/rustc/ty) module contains a +[`T-middle`] issues can be more involved and require verifying types. The [`ty`] module contains a lot of methods that are useful, though one of the most useful would be `expr_ty` (gives the type of an AST expression). `match_def_path()` in Clippy's `utils` module can also be useful. +[`good first issue`]: https://github.com/rust-lang/rust-clippy/labels/good%20first%20issue +[`S-inactive-closed`]: https://github.com/rust-lang/rust-clippy/pulls?q=is%3Aclosed+label%3AS-inactive-closed +[`T-AST`]: https://github.com/rust-lang/rust-clippy/labels/T-AST +[`T-middle`]: https://github.com/rust-lang/rust-clippy/labels/T-middle +[`E-medium`]: https://github.com/rust-lang/rust-clippy/labels/E-medium +[`ty`]: https://doc.rust-lang.org/nightly/nightly-rustc/rustc/ty +[nodes in the AST docs]: https://doc.rust-lang.org/nightly/nightly-rustc/rustc_ast/ast/ +[deep-nesting]: https://github.com/rust-lang/rust-clippy/blob/557f6848bd5b7183f55c1e1522a326e9e1df6030/clippy_lints/src/mem_forget.rs#L29-L43 +[if_chain]: https://docs.rs/if_chain/*/if_chain +[nest-less]: https://github.com/rust-lang/rust-clippy/blob/557f6848bd5b7183f55c1e1522a326e9e1df6030/clippy_lints/src/bit_mask.rs#L124-L150 + ## Writing code -Have a look at the [docs for writing lints](doc/adding_lints.md) for more details. [Llogiq's blog post on -lints](https://llogiq.github.io/2015/06/04/workflows.html) is also a nice primer to lint-writing, though it does get -into advanced stuff and may be a bit outdated. +Have a look at the [docs for writing lints][adding_lints] for more details. [Llogiq's blog post on lints] +is also a nice primer to lint-writing, though it does get into advanced stuff and may be a bit outdated. If you want to add a new lint or change existing ones apart from bugfixing, it's also a good idea to give the [stability guarantees][rfc_stability] and [lint categories][rfc_lint_cats] sections of the [Clippy 1.0 RFC][clippy_rfc] a quick read. -## How Clippy works +[adding_lints]: https://github.com/rust-lang/rust-clippy/blob/master/doc/adding_lints.md +[Llogiq's blog post on lints]: https://llogiq.github.io/2015/06/04/workflows.html +[clippy_rfc]: https://github.com/rust-lang/rfcs/blob/master/text/2476-clippy-uno.md +[rfc_stability]: https://github.com/rust-lang/rfcs/blob/master/text/2476-clippy-uno.md#stability-guarantees +[rfc_lint_cats]: https://github.com/rust-lang/rfcs/blob/master/text/2476-clippy-uno.md#lint-audit-and-categories -Clippy is a [rustc compiler plugin][compiler_plugin]. The main entry point is at [`src/lib.rs`][main_entry]. In there, -the lint registration is delegated to the [`clippy_lints`][lint_crate] crate. +## How Clippy works -[`clippy_lints/src/lib.rs`][lint_crate_entry] imports all the different lint modules and registers them with the rustc -plugin registry. For example, the [`else_if_without_else`][else_if_without_else] lint is registered like this: +[`clippy_lints/src/lib.rs`][lint_crate_entry] imports all the different lint modules and registers in the [`LintStore`]. +For example, the [`else_if_without_else`][else_if_without_else] lint is registered like this: ```rust // ./clippy_lints/src/lib.rs @@ -94,25 +104,24 @@ plugin registry. For example, the [`else_if_without_else`][else_if_without_else] pub mod else_if_without_else; // ... -pub fn register_plugins(reg: &mut rustc_driver::plugin::Registry) { +pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: &Conf) { // ... - reg.register_early_lint_pass(box else_if_without_else::ElseIfWithoutElse); + store.register_early_pass(|| box else_if_without_else::ElseIfWithoutElse); // ... - reg.register_lint_group("clippy::restriction", vec![ + store.register_group(true, "clippy::restriction", Some("clippy_restriction"), vec![ // ... - else_if_without_else::ELSE_IF_WITHOUT_ELSE, + LintId::of(&else_if_without_else::ELSE_IF_WITHOUT_ELSE), // ... ]); } ``` -The [`plugin::PluginRegistry`][plugin_registry] provides two methods to register lints: -[register_early_lint_pass][reg_early_lint_pass] and [register_late_lint_pass][reg_late_lint_pass]. Both take an object +The [`rustc_lint::LintStore`][`LintStore`] provides two methods to register lints: +[register_early_pass][reg_early_pass] and [register_late_pass][reg_late_pass]. Both take an object that implements an [`EarlyLintPass`][early_lint_pass] or [`LateLintPass`][late_lint_pass] respectively. This is done in every single lint. It's worth noting that the majority of `clippy_lints/src/lib.rs` is autogenerated by `cargo dev -update_lints` and you don't have to add anything by hand. When you are writing your own lint, you can use that script to -save you some time. +update_lints`. When you are writing your own lint, you can use that script to save you some time. ```rust // ./clippy_lints/src/else_if_without_else.rs @@ -134,8 +143,16 @@ The difference between `EarlyLintPass` and `LateLintPass` is that the methods of AST information. The methods of the `LateLintPass` trait are executed after type checking and contain type information via the `LateContext` parameter. -That's why the `else_if_without_else` example uses the `register_early_lint_pass` function. Because the [actual lint -logic][else_if_without_else] does not depend on any type information. +That's why the `else_if_without_else` example uses the `register_early_pass` function. Because the +[actual lint logic][else_if_without_else] does not depend on any type information. + +[lint_crate_entry]: https://github.com/rust-lang/rust-clippy/blob/master/clippy_lints/src/lib.rs +[else_if_without_else]: https://github.com/rust-lang/rust-clippy/blob/4253aa7137cb7378acc96133c787e49a345c2b3c/clippy_lints/src/else_if_without_else.rs +[`LintStore`]: https://doc.rust-lang.org/nightly/nightly-rustc/rustc_lint/struct.LintStore.html +[reg_early_pass]: https://doc.rust-lang.org/nightly/nightly-rustc/rustc_lint/struct.LintStore.html#method.register_early_pass +[reg_late_pass]: https://doc.rust-lang.org/nightly/nightly-rustc/rustc_lint/struct.LintStore.html#method.register_late_pass +[early_lint_pass]: https://doc.rust-lang.org/nightly/nightly-rustc/rustc_lint/trait.EarlyLintPass.html +[late_lint_pass]: https://doc.rust-lang.org/nightly/nightly-rustc/rustc_lint/trait.LateLintPass.html ## Fixing build failures caused by Rust @@ -144,8 +161,9 @@ the times we have to adapt to the changes and only very rarely there's an actual caused by Rust updates, can be a good way to learn about Rust internals. In order to find out why Clippy does not work properly with a new Rust commit, you can use the [rust-toolstate commit -history][toolstate_commit_history]. You will then have to look for the last commit that contains `test-pass -> -build-fail` or `test-pass` -> `test-fail` for the `clippy-driver` component. [Here][toolstate_commit] is an example. +history][toolstate_commit_history]. You will then have to look for the last commit that contains +`test-pass -> build-fail` or `test-pass -> test-fail` for the `clippy-driver` component. +[Here][toolstate_commit] is an example. The commit message contains a link to the PR. The PRs are usually small enough to discover the breaking API change and if they are bigger, they likely include some discussion that may help you to fix Clippy. @@ -157,14 +175,8 @@ If you decide to make Clippy work again with a Rust commit that breaks it, you probably want to install the latest Rust from master locally and run Clippy using that version of Rust. -You can use [rustup-toolchain-install-master][rtim] to do that: - -```bash -cargo install rustup-toolchain-install-master -rustup-toolchain-install-master --force -n master -c rustc-dev -rustup override set master -cargo test -``` +You can set up the master toolchain by running `./setup-toolchain.sh`. That script will install +[rustup-toolchain-install-master][rtim] and master toolchain, then run `rustup override set master`. After fixing the build failure on this repository, we can submit a pull request to [`rust-lang/rust`] to fix the toolstate. @@ -181,6 +193,12 @@ git commit -m "Update Clippy" # Open a PR in rust-lang/rust ``` +[rustup_component_history]: https://rust-lang.github.io/rustup-components-history +[toolstate_commit_history]: https://github.com/rust-lang-nursery/rust-toolstate/commits/master +[toolstate_commit]: https://github.com/rust-lang-nursery/rust-toolstate/commit/aad74d8294e198a7cf8ac81a91aebb7f3bbcf727 +[rtim]: https://github.com/kennytm/rustup-toolchain-install-master +[`rust-lang/rust`]: https://github.com/rust-lang/rust + ## Issue and PR triage Clippy is following the [Rust triage procedure][triage] for issues and pull @@ -205,6 +223,12 @@ You can find the Clippy bors queue [here][homu_queue]. If you have @bors permissions, you can find an overview of the available commands [here][homu_instructions]. +[triage]: https://forge.rust-lang.org/triage-procedure.html +[l-crash]: https://github.com/rust-lang/rust-clippy/labels/L-crash%20%3Aboom%3A +[l-bug]: https://github.com/rust-lang/rust-clippy/labels/L-bug%20%3Abeetle%3A +[homu]: https://github.com/rust-lang/homu +[homu_instructions]: https://buildbot2.rust-lang.org/homu/ +[homu_queue]: https://buildbot2.rust-lang.org/homu/queue/clippy ## Contributions @@ -212,32 +236,9 @@ Contributions to Clippy should be made in the form of GitHub pull requests. Each be reviewed by a core contributor (someone with permission to land patches) and either landed in the main tree or given feedback for changes that would be required. -All code in this repository is under the [Apache-2.0](http://www.apache.org/licenses/LICENSE-2.0>) -or the [MIT](http://opensource.org/licenses/MIT) license. +All code in this repository is under the [Apache-2.0] or the [MIT] license. -[main_entry]: https://github.com/rust-lang/rust-clippy/blob/c5b39a5917ffc0f1349b6e414fa3b874fdcf8429/src/lib.rs#L14 -[lint_crate]: https://github.com/rust-lang/rust-clippy/tree/c5b39a5917ffc0f1349b6e414fa3b874fdcf8429/clippy_lints/src -[lint_crate_entry]: https://github.com/rust-lang/rust-clippy/blob/c5b39a5917ffc0f1349b6e414fa3b874fdcf8429/clippy_lints/src/lib.rs -[else_if_without_else]: https://github.com/rust-lang/rust-clippy/blob/c5b39a5917ffc0f1349b6e414fa3b874fdcf8429/clippy_lints/src/else_if_without_else.rs -[compiler_plugin]: https://doc.rust-lang.org/unstable-book/language-features/plugin.html#lint-plugins -[plugin_registry]: https://doc.rust-lang.org/nightly/nightly-rustc/rustc_plugin_impl/registry/struct.Registry.html -[reg_early_lint_pass]: https://doc.rust-lang.org/nightly/nightly-rustc/rustc_plugin_impl/registry/struct.Registry.html#method.register_early_lint_pass -[reg_late_lint_pass]: https://doc.rust-lang.org/nightly/nightly-rustc/rustc_plugin_impl/registry/struct.Registry.html#method.register_late_lint_pass -[early_lint_pass]: https://doc.rust-lang.org/nightly/nightly-rustc/rustc/lint/trait.EarlyLintPass.html -[late_lint_pass]: https://doc.rust-lang.org/nightly/nightly-rustc/rustc/lint/trait.LateLintPass.html -[toolstate_commit_history]: https://github.com/rust-lang-nursery/rust-toolstate/commits/master -[toolstate_commit]: https://github.com/rust-lang-nursery/rust-toolstate/commit/6ce0459f6bfa7c528ae1886492a3e0b5ef0ee547 -[rtim]: https://github.com/kennytm/rustup-toolchain-install-master -[rustup_component_history]: https://mexus.github.io/rustup-components-history -[clippy_rfc]: https://github.com/rust-lang/rfcs/blob/master/text/2476-clippy-uno.md -[rfc_stability]: https://github.com/rust-lang/rfcs/blob/master/text/2476-clippy-uno.md#stability-guarantees -[rfc_lint_cats]: https://github.com/rust-lang/rfcs/blob/master/text/2476-clippy-uno.md#lint-audit-and-categories -[triage]: https://forge.rust-lang.org/triage-procedure.html -[l-crash]: https://github.com/rust-lang/rust-clippy/labels/L-crash%20%3Aboom%3A -[l-bug]: https://github.com/rust-lang/rust-clippy/labels/L-bug%20%3Abeetle%3A -[homu]: https://github.com/servo/homu -[homu_instructions]: https://buildbot2.rust-lang.org/homu/ -[homu_queue]: https://buildbot2.rust-lang.org/homu/queue/clippy -[`rust-lang/rust`]: https://github.com/rust-lang/rust +[Apache-2.0]: https://www.apache.org/licenses/LICENSE-2.0 +[MIT]: https://opensource.org/licenses/MIT diff --git a/README.md b/README.md index 6915b1bde025..2181c296e9b2 100644 --- a/README.md +++ b/README.md @@ -5,7 +5,7 @@ A collection of lints to catch common mistakes and improve your [Rust](https://github.com/rust-lang/rust) code. -[There are 359 lints included in this crate!](https://rust-lang.github.io/rust-clippy/master/index.html) +[There are 361 lints included in this crate!](https://rust-lang.github.io/rust-clippy/master/index.html) We have a bunch of lint categories to allow you to choose how much Clippy is supposed to ~~annoy~~ help you: diff --git a/clippy_lints/Cargo.toml b/clippy_lints/Cargo.toml index ad3b077ec4b9..99eebcdd3d59 100644 --- a/clippy_lints/Cargo.toml +++ b/clippy_lints/Cargo.toml @@ -19,7 +19,7 @@ edition = "2018" [dependencies] cargo_metadata = "0.9.0" if_chain = "1.0.0" -itertools = "0.8" +itertools = "0.9" lazy_static = "1.0.2" matches = "0.1.7" pulldown-cmark = { version = "0.7", default-features = false } diff --git a/clippy_lints/README.md b/clippy_lints/README.md index 2724aa6a0523..513583b7e349 100644 --- a/clippy_lints/README.md +++ b/clippy_lints/README.md @@ -1,3 +1 @@ -This crate contains Clippy lints. For the main crate, check -[*crates.io*](https://crates.io/crates/clippy) or -[GitHub](https://github.com/rust-lang/rust-clippy). +This crate contains Clippy lints. For the main crate, check [GitHub](https://github.com/rust-lang/rust-clippy). diff --git a/clippy_lints/src/attrs.rs b/clippy_lints/src/attrs.rs index 7d5e1d881912..833739f1bd3c 100644 --- a/clippy_lints/src/attrs.rs +++ b/clippy_lints/src/attrs.rs @@ -379,8 +379,8 @@ fn is_relevant_impl(cx: &LateContext<'_, '_>, item: &ImplItem<'_>) -> bool { fn is_relevant_trait(cx: &LateContext<'_, '_>, item: &TraitItem<'_>) -> bool { match item.kind { - TraitItemKind::Method(_, TraitMethod::Required(_)) => true, - TraitItemKind::Method(_, TraitMethod::Provided(eid)) => { + TraitItemKind::Fn(_, TraitMethod::Required(_)) => true, + TraitItemKind::Fn(_, TraitMethod::Provided(eid)) => { is_relevant_expr(cx, cx.tcx.body_tables(eid), &cx.tcx.hir().body(eid).value) }, _ => false, diff --git a/clippy_lints/src/doc.rs b/clippy_lints/src/doc.rs index 35578ae68a46..006d732703ec 100644 --- a/clippy_lints/src/doc.rs +++ b/clippy_lints/src/doc.rs @@ -179,7 +179,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for DocMarkdown { fn check_trait_item(&mut self, cx: &LateContext<'a, 'tcx>, item: &'tcx hir::TraitItem<'_>) { let headers = check_attrs(cx, &self.valid_idents, &item.attrs); - if let hir::TraitItemKind::Method(ref sig, ..) = item.kind { + if let hir::TraitItemKind::Fn(ref sig, ..) = item.kind { if !in_external_macro(cx.tcx.sess, item.span) { lint_for_missing_headers(cx, item.hir_id, item.span, sig, headers, None); } @@ -367,6 +367,8 @@ fn check_attrs<'a>(cx: &LateContext<'_, '_>, valid_idents: &FxHashSet, a check_doc(cx, valid_idents, events, &spans) } +const RUST_CODE: &[&str] = &["rust", "no_run", "should_panic", "compile_fail", "edition2018"]; + fn check_doc<'a, Events: Iterator, Range)>>( cx: &LateContext<'_, '_>, valid_idents: &FxHashSet, @@ -374,6 +376,7 @@ fn check_doc<'a, Events: Iterator, Range 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, }; @@ -386,11 +389,20 @@ fn check_doc<'a, Events: Iterator, Range in_code = true, - End(CodeBlock(_)) => in_code = false, + Start(CodeBlock(ref kind)) => { + in_code = true; + if let CodeBlockKind::Fenced(lang) = kind { + is_rust = + lang.is_empty() || !lang.contains("ignore") && lang.split(',').any(|i| RUST_CODE.contains(&i)); + } + }, + End(CodeBlock(_)) => { + in_code = false; + is_rust = false; + }, Start(Link(_, url, _)) => in_link = Some(url), End(Link(..)) => in_link = None, Start(Heading(_)) => in_heading = true, @@ -413,7 +425,9 @@ fn check_doc<'a, Events: Iterator, Range LateLintPass<'a, 'tcx> for Functions { } fn check_trait_item(&mut self, cx: &LateContext<'a, 'tcx>, item: &'tcx hir::TraitItem<'_>) { - if let hir::TraitItemKind::Method(ref sig, ref eid) = item.kind { + if let hir::TraitItemKind::Fn(ref sig, ref eid) = item.kind { // don't lint extern functions decls, it's not their fault if sig.header.abi == Abi::Rust { self.check_arg_number(cx, &sig.decl, item.span.with_hi(sig.decl.output.span().hi())); diff --git a/clippy_lints/src/implicit_return.rs b/clippy_lints/src/implicit_return.rs index b0d80aa0d539..d968a928c33b 100644 --- a/clippy_lints/src/implicit_return.rs +++ b/clippy_lints/src/implicit_return.rs @@ -1,5 +1,5 @@ use crate::utils::{ - match_def_path, + fn_has_unsatisfiable_preds, match_def_path, paths::{BEGIN_PANIC, BEGIN_PANIC_FMT}, snippet_opt, span_lint_and_then, }; @@ -133,6 +133,12 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for ImplicitReturn { _: HirId, ) { let def_id = cx.tcx.hir().body_owner_def_id(body.id()); + + // Building MIR for `fn`s with unsatisfiable preds results in ICE. + if fn_has_unsatisfiable_preds(cx, def_id) { + return; + } + let mir = cx.tcx.optimized_mir(def_id); // checking return type through MIR, HIR is not able to determine inferred closure return types diff --git a/clippy_lints/src/inline_fn_without_body.rs b/clippy_lints/src/inline_fn_without_body.rs index de16a4f1ef82..32032c4da885 100644 --- a/clippy_lints/src/inline_fn_without_body.rs +++ b/clippy_lints/src/inline_fn_without_body.rs @@ -32,7 +32,7 @@ declare_lint_pass!(InlineFnWithoutBody => [INLINE_FN_WITHOUT_BODY]); impl<'a, 'tcx> LateLintPass<'a, 'tcx> for InlineFnWithoutBody { fn check_trait_item(&mut self, cx: &LateContext<'a, 'tcx>, item: &'tcx TraitItem<'_>) { - if let TraitItemKind::Method(_, TraitMethod::Required(_)) = item.kind { + if let TraitItemKind::Fn(_, TraitMethod::Required(_)) = item.kind { check_attrs(cx, item.ident.name, &item.attrs); } } diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 327cc56cafa5..1eac69678a64 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -234,6 +234,7 @@ pub mod let_underscore; pub mod lifetimes; pub mod literal_representation; pub mod loops; +pub mod macro_use; pub mod main_recursion; pub mod map_clone; pub mod map_unit_fn; @@ -309,6 +310,7 @@ pub mod unused_self; pub mod unwrap; pub mod use_self; pub mod vec; +pub mod verbose_file_reads; pub mod wildcard_dependencies; pub mod wildcard_imports; pub mod write; @@ -599,6 +601,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: &loops::WHILE_IMMUTABLE_CONDITION, &loops::WHILE_LET_LOOP, &loops::WHILE_LET_ON_ITERATOR, + ¯o_use::MACRO_USE_IMPORTS, &main_recursion::MAIN_RECURSION, &map_clone::MAP_CLONE, &map_unit_fn::OPTION_MAP_UNIT_FN, @@ -813,6 +816,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: &unwrap::UNNECESSARY_UNWRAP, &use_self::USE_SELF, &vec::USELESS_VEC, + &verbose_file_reads::VERBOSE_FILE_READS, &wildcard_dependencies::WILDCARD_DEPENDENCIES, &wildcard_imports::ENUM_GLOB_USE, &wildcard_imports::WILDCARD_IMPORTS, @@ -1012,6 +1016,8 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: store.register_early_pass(move || box excessive_bools::ExcessiveBools::new(max_struct_bools, max_fn_params_bools)); store.register_early_pass(|| box option_env_unwrap::OptionEnvUnwrap); store.register_late_pass(|| box wildcard_imports::WildcardImports); + store.register_early_pass(|| box macro_use::MacroUseImports); + store.register_late_pass(|| box verbose_file_reads::VerboseFileReads); store.register_group(true, "clippy::restriction", Some("clippy_restriction"), vec![ LintId::of(&arithmetic::FLOAT_ARITHMETIC), @@ -1079,6 +1085,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: LintId::of(&literal_representation::LARGE_DIGIT_GROUPS), LintId::of(&loops::EXPLICIT_INTO_ITER_LOOP), LintId::of(&loops::EXPLICIT_ITER_LOOP), + LintId::of(¯o_use::MACRO_USE_IMPORTS), LintId::of(&matches::SINGLE_MATCH_ELSE), LintId::of(&methods::FILTER_MAP), LintId::of(&methods::FILTER_MAP_NEXT), @@ -1368,6 +1375,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: LintId::of(&unwrap::PANICKING_UNWRAP), LintId::of(&unwrap::UNNECESSARY_UNWRAP), LintId::of(&vec::USELESS_VEC), + LintId::of(&verbose_file_reads::VERBOSE_FILE_READS), LintId::of(&write::PRINTLN_EMPTY_STRING), LintId::of(&write::PRINT_LITERAL), LintId::of(&write::PRINT_WITH_NEWLINE), @@ -1551,6 +1559,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: LintId::of(&types::UNNECESSARY_CAST), LintId::of(&types::VEC_BOX), LintId::of(&unwrap::UNNECESSARY_UNWRAP), + LintId::of(&verbose_file_reads::VERBOSE_FILE_READS), LintId::of(&zero_div_zero::ZERO_DIVIDED_BY_ZERO), ]); diff --git a/clippy_lints/src/lifetimes.rs b/clippy_lints/src/lifetimes.rs index a120e3a7517f..3b687bd54a1c 100644 --- a/clippy_lints/src/lifetimes.rs +++ b/clippy_lints/src/lifetimes.rs @@ -100,7 +100,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Lifetimes { } fn check_trait_item(&mut self, cx: &LateContext<'a, 'tcx>, item: &'tcx TraitItem<'_>) { - if let TraitItemKind::Method(ref sig, ref body) = item.kind { + if let TraitItemKind::Fn(ref sig, ref body) = item.kind { let body = match *body { TraitMethod::Required(_) => None, TraitMethod::Provided(id) => Some(id), diff --git a/clippy_lints/src/macro_use.rs b/clippy_lints/src/macro_use.rs new file mode 100644 index 000000000000..b1345d0b751a --- /dev/null +++ b/clippy_lints/src/macro_use.rs @@ -0,0 +1,53 @@ +use crate::utils::{snippet, span_lint_and_sugg}; +use if_chain::if_chain; +use rustc_ast::ast; +use rustc_errors::Applicability; +use rustc_lint::{EarlyContext, EarlyLintPass}; +use rustc_session::{declare_lint_pass, declare_tool_lint}; +use rustc_span::edition::Edition; + +declare_clippy_lint! { + /// **What it does:** Checks for `#[macro_use] use...`. + /// + /// **Why is this bad?** Since the Rust 2018 edition you can import + /// macro's directly, this is considered idiomatic. + /// + /// **Known problems:** This lint does not generate an auto-applicable suggestion. + /// + /// **Example:** + /// ```rust + /// #[macro_use] + /// use lazy_static; + /// ``` + pub MACRO_USE_IMPORTS, + pedantic, + "#[macro_use] is no longer needed" +} + +declare_lint_pass!(MacroUseImports => [MACRO_USE_IMPORTS]); + +impl EarlyLintPass for MacroUseImports { + fn check_item(&mut self, ecx: &EarlyContext<'_>, item: &ast::Item) { + if_chain! { + if ecx.sess.opts.edition == Edition::Edition2018; + if let ast::ItemKind::Use(use_tree) = &item.kind; + if let Some(mac_attr) = item + .attrs + .iter() + .find(|attr| attr.ident().map(|s| s.to_string()) == Some("macro_use".to_string())); + then { + let msg = "`macro_use` attributes are no longer needed in the Rust 2018 edition"; + let help = format!("use {}::", snippet(ecx, use_tree.span, "_")); + span_lint_and_sugg( + ecx, + MACRO_USE_IMPORTS, + mac_attr.span, + msg, + "remove the attribute and import the macro directly, try", + help, + Applicability::HasPlaceholders, + ); + } + } + } +} diff --git a/clippy_lints/src/map_unit_fn.rs b/clippy_lints/src/map_unit_fn.rs index 8bc08c342dc9..d7ea022d8886 100644 --- a/clippy_lints/src/map_unit_fn.rs +++ b/clippy_lints/src/map_unit_fn.rs @@ -186,19 +186,19 @@ fn unit_closure<'a, 'tcx>( /// `x.field` => `x_field` /// `y` => `_y` /// -/// Anything else will return `_`. +/// Anything else will return `a`. fn let_binding_name(cx: &LateContext<'_, '_>, var_arg: &hir::Expr<'_>) -> String { match &var_arg.kind { hir::ExprKind::Field(_, _) => snippet(cx, var_arg.span, "_").replace(".", "_"), hir::ExprKind::Path(_) => format!("_{}", snippet(cx, var_arg.span, "")), - _ => "_".to_string(), + _ => "a".to_string(), } } #[must_use] fn suggestion_msg(function_type: &str, map_type: &str) -> String { format!( - "called `map(f)` on an `{0}` value where `f` is a unit {1}", + "called `map(f)` on an `{0}` value where `f` is a {1} that returns the unit type", map_type, function_type ) } diff --git a/clippy_lints/src/matches.rs b/clippy_lints/src/matches.rs index 9668c5d83499..b0aae5e30e6e 100644 --- a/clippy_lints/src/matches.rs +++ b/clippy_lints/src/matches.rs @@ -14,8 +14,8 @@ use rustc_ast::ast::LitKind; use rustc_errors::Applicability; use rustc_hir::def::CtorKind; use rustc_hir::{ - print, Arm, BindingAnnotation, Block, BorrowKind, Expr, ExprKind, Local, MatchSource, Mutability, Pat, PatKind, - QPath, RangeEnd, + print, Arm, BindingAnnotation, Block, BorrowKind, Expr, ExprKind, Local, MatchSource, Mutability, Node, Pat, + PatKind, QPath, RangeEnd, }; use rustc_lint::{LateContext, LateLintPass, LintContext}; use rustc_session::{declare_tool_lint, impl_lint_pass}; @@ -882,7 +882,7 @@ fn check_wild_in_or_pats(cx: &LateContext<'_, '_>, arms: &[Arm<'_>]) { } } -fn check_match_single_binding(cx: &LateContext<'_, '_>, ex: &Expr<'_>, arms: &[Arm<'_>], expr: &Expr<'_>) { +fn check_match_single_binding<'a>(cx: &LateContext<'_, 'a>, ex: &Expr<'a>, arms: &[Arm<'_>], expr: &Expr<'_>) { if in_macro(expr.span) || arms.len() != 1 || is_refutable(cx, arms[0].pat) { return; } @@ -914,19 +914,38 @@ fn check_match_single_binding(cx: &LateContext<'_, '_>, ex: &Expr<'_>, arms: &[A let mut applicability = Applicability::MaybeIncorrect; match arms[0].pat.kind { PatKind::Binding(..) | PatKind::Tuple(_, _) | PatKind::Struct(..) => { + // If this match is in a local (`let`) stmt + let (target_span, sugg) = if let Some(parent_let_node) = opt_parent_let(cx, ex) { + ( + parent_let_node.span, + format!( + "let {} = {};\n{}let {} = {};", + snippet_with_applicability(cx, bind_names, "..", &mut applicability), + snippet_with_applicability(cx, matched_vars, "..", &mut applicability), + " ".repeat(indent_of(cx, expr.span).unwrap_or(0)), + snippet_with_applicability(cx, parent_let_node.pat.span, "..", &mut applicability), + snippet_body + ), + ) + } else { + ( + expr.span, + format!( + "let {} = {};\n{}{}", + snippet_with_applicability(cx, bind_names, "..", &mut applicability), + snippet_with_applicability(cx, matched_vars, "..", &mut applicability), + " ".repeat(indent_of(cx, expr.span).unwrap_or(0)), + snippet_body + ), + ) + }; span_lint_and_sugg( cx, MATCH_SINGLE_BINDING, - expr.span, + target_span, "this match could be written as a `let` statement", "consider using `let` statement", - format!( - "let {} = {};\n{}{}", - snippet_with_applicability(cx, bind_names, "..", &mut applicability), - snippet_with_applicability(cx, matched_vars, "..", &mut applicability), - " ".repeat(indent_of(cx, expr.span).unwrap_or(0)), - snippet_body, - ), + sugg, applicability, ); }, @@ -945,6 +964,19 @@ fn check_match_single_binding(cx: &LateContext<'_, '_>, ex: &Expr<'_>, arms: &[A } } +/// Returns true if the `ex` match expression is in a local (`let`) statement +fn opt_parent_let<'a>(cx: &LateContext<'_, 'a>, ex: &Expr<'a>) -> Option<&'a Local<'a>> { + if_chain! { + let map = &cx.tcx.hir(); + if let Some(Node::Expr(parent_arm_expr)) = map.find(map.get_parent_node(ex.hir_id)); + if let Some(Node::Local(parent_let_expr)) = map.find(map.get_parent_node(parent_arm_expr.hir_id)); + then { + return Some(parent_let_expr); + } + } + None +} + /// Gets all arms that are unbounded `PatRange`s. fn all_ranges<'a, 'tcx>( cx: &LateContext<'a, 'tcx>, diff --git a/clippy_lints/src/methods/mod.rs b/clippy_lints/src/methods/mod.rs index eef510540901..19d9a31aaf41 100644 --- a/clippy_lints/src/methods/mod.rs +++ b/clippy_lints/src/methods/mod.rs @@ -1738,7 +1738,7 @@ fn lint_expect_fun_call( if let hir::ExprKind::Path(ref p) = fun.kind { match cx.tables.qpath_res(p, fun.hir_id) { hir::def::Res::Def(hir::def::DefKind::Fn, def_id) - | hir::def::Res::Def(hir::def::DefKind::Method, def_id) => matches!( + | hir::def::Res::Def(hir::def::DefKind::AssocFn, def_id) => matches!( cx.tcx.fn_sig(def_id).output().skip_binder().kind, ty::Ref(ty::ReStatic, ..) ), diff --git a/clippy_lints/src/misc_early.rs b/clippy_lints/src/misc_early.rs index 60b672766231..718ec1c00bb4 100644 --- a/clippy_lints/src/misc_early.rs +++ b/clippy_lints/src/misc_early.rs @@ -5,8 +5,8 @@ use crate::utils::{ use if_chain::if_chain; use rustc::lint::in_external_macro; use rustc_ast::ast::{ - Block, Expr, ExprKind, GenericParamKind, Generics, Lit, LitFloatType, LitIntType, LitKind, NodeId, Pat, PatKind, - StmtKind, UnOp, + BindingMode, Block, Expr, ExprKind, GenericParamKind, Generics, Lit, LitFloatType, LitIntType, LitKind, Mutability, + NodeId, Pat, PatKind, StmtKind, UnOp, }; use rustc_ast::visit::{walk_expr, FnKind, Visitor}; use rustc_data_structures::fx::FxHashMap; @@ -318,18 +318,6 @@ impl EarlyLintPass for MiscEarlyLints { return; } if wilds > 0 { - let mut normal = vec![]; - - for field in pfields { - match field.pat.kind { - PatKind::Wild => {}, - _ => { - if let Ok(n) = cx.sess().source_map().span_to_snippet(field.span) { - normal.push(n); - } - }, - } - } for field in pfields { if let PatKind::Wild = field.pat.kind { wilds -= 1; @@ -341,6 +329,19 @@ impl EarlyLintPass for MiscEarlyLints { "You matched a field with a wildcard pattern. Consider using `..` instead", ); } else { + let mut normal = vec![]; + + for field in pfields { + match field.pat.kind { + PatKind::Wild => {}, + _ => { + if let Ok(n) = cx.sess().source_map().span_to_snippet(field.span) { + normal.push(n); + } + }, + } + } + span_lint_and_help( cx, UNNEEDED_FIELD_PATTERN, @@ -355,7 +356,13 @@ impl EarlyLintPass for MiscEarlyLints { } } - if let PatKind::Ident(_, ident, Some(ref right)) = pat.kind { + if let PatKind::Ident(left, ident, Some(ref right)) = pat.kind { + let left_binding = match left { + BindingMode::ByRef(Mutability::Mut) => "ref mut ", + BindingMode::ByRef(Mutability::Not) => "ref ", + _ => "", + }; + if let PatKind::Wild = right.kind { span_lint_and_sugg( cx, @@ -366,7 +373,7 @@ impl EarlyLintPass for MiscEarlyLints { ident.name, ident.name, ), "try", - format!("{}", ident.name), + format!("{}{}", left_binding, ident.name), Applicability::MachineApplicable, ); } diff --git a/clippy_lints/src/missing_const_for_fn.rs b/clippy_lints/src/missing_const_for_fn.rs index e829baa22688..a22d8b4cf9eb 100644 --- a/clippy_lints/src/missing_const_for_fn.rs +++ b/clippy_lints/src/missing_const_for_fn.rs @@ -1,4 +1,4 @@ -use crate::utils::{has_drop, is_entrypoint_fn, span_lint, trait_ref_of_method}; +use crate::utils::{fn_has_unsatisfiable_preds, has_drop, is_entrypoint_fn, span_lint, trait_ref_of_method}; use rustc::lint::in_external_macro; use rustc_hir as hir; use rustc_hir::intravisit::FnKind; @@ -88,6 +88,11 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for MissingConstForFn { return; } + // Building MIR for `fn`s with unsatisfiable preds results in ICE. + if fn_has_unsatisfiable_preds(cx, def_id) { + return; + } + // Perform some preliminary checks that rule out constness on the Clippy side. This way we // can skip the actual const check and return early. match kind { diff --git a/clippy_lints/src/missing_doc.rs b/clippy_lints/src/missing_doc.rs index 5e32172d78a1..57bec0a65b37 100644 --- a/clippy_lints/src/missing_doc.rs +++ b/clippy_lints/src/missing_doc.rs @@ -164,7 +164,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for MissingDoc { fn check_trait_item(&mut self, cx: &LateContext<'a, 'tcx>, trait_item: &'tcx hir::TraitItem<'_>) { let desc = match trait_item.kind { hir::TraitItemKind::Const(..) => "an associated constant", - hir::TraitItemKind::Method(..) => "a trait method", + hir::TraitItemKind::Fn(..) => "a trait method", hir::TraitItemKind::Type(..) => "an associated type", }; diff --git a/clippy_lints/src/missing_inline.rs b/clippy_lints/src/missing_inline.rs index b736c58879ba..fad4d6d0fe2e 100644 --- a/clippy_lints/src/missing_inline.rs +++ b/clippy_lints/src/missing_inline.rs @@ -100,7 +100,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for MissingInline { let tit_ = cx.tcx.hir().trait_item(tit.id); match tit_.kind { hir::TraitItemKind::Const(..) | hir::TraitItemKind::Type(..) => {}, - hir::TraitItemKind::Method(..) => { + hir::TraitItemKind::Fn(..) => { if tit.defaultness.has_value() { // trait method with default body needs inline in case // an impl is not provided diff --git a/clippy_lints/src/mut_key.rs b/clippy_lints/src/mut_key.rs index d48c4c419aef..61f514a740a5 100644 --- a/clippy_lints/src/mut_key.rs +++ b/clippy_lints/src/mut_key.rs @@ -67,7 +67,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for MutableKeyType { } fn check_trait_item(&mut self, cx: &LateContext<'a, 'tcx>, item: &'tcx hir::TraitItem<'tcx>) { - if let hir::TraitItemKind::Method(ref sig, ..) = item.kind { + if let hir::TraitItemKind::Fn(ref sig, ..) = item.kind { check_sig(cx, item.hir_id, &sig.decl); } } diff --git a/clippy_lints/src/ptr.rs b/clippy_lints/src/ptr.rs index c1aee94aa421..a6d62406aa76 100644 --- a/clippy_lints/src/ptr.rs +++ b/clippy_lints/src/ptr.rs @@ -121,7 +121,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Ptr { } fn check_trait_item(&mut self, cx: &LateContext<'a, 'tcx>, item: &'tcx TraitItem<'_>) { - if let TraitItemKind::Method(ref sig, ref trait_method) = item.kind { + if let TraitItemKind::Fn(ref sig, ref trait_method) = item.kind { let body_id = if let TraitMethod::Provided(b) = *trait_method { Some(b) } else { diff --git a/clippy_lints/src/redundant_clone.rs b/clippy_lints/src/redundant_clone.rs index ddb0c7484348..b63bb371c4f3 100644 --- a/clippy_lints/src/redundant_clone.rs +++ b/clippy_lints/src/redundant_clone.rs @@ -1,12 +1,12 @@ use crate::utils::{ - has_drop, is_copy, match_def_path, match_type, paths, snippet_opt, span_lint_hir, span_lint_hir_and_then, - walk_ptrs_ty_depth, + fn_has_unsatisfiable_preds, has_drop, is_copy, match_def_path, match_type, paths, snippet_opt, span_lint_hir, + span_lint_hir_and_then, walk_ptrs_ty_depth, }; use if_chain::if_chain; use matches::matches; use rustc::mir::{ self, traversal, - visit::{MutatingUseContext, PlaceContext, Visitor as _}, + visit::{MutatingUseContext, NonMutatingUseContext, PlaceContext, Visitor as _}, }; use rustc::ty::{self, fold::TypeVisitor, Ty}; use rustc_data_structures::{fx::FxHashMap, transitive_relation::TransitiveRelation}; @@ -79,6 +79,12 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for RedundantClone { _: HirId, ) { let def_id = cx.tcx.hir().body_owner_def_id(body.id()); + + // Building MIR for `fn`s with unsatisfiable preds results in ICE. + if fn_has_unsatisfiable_preds(cx, def_id) { + return; + } + let mir = cx.tcx.optimized_mir(def_id); let mir_read_only = mir.unwrap_read_only(); @@ -104,7 +110,8 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for RedundantClone { continue; } - let (fn_def_id, arg, arg_ty, _) = unwrap_or_continue!(is_call_with_ref_arg(cx, mir, &terminator.kind)); + let (fn_def_id, arg, arg_ty, clone_ret) = + unwrap_or_continue!(is_call_with_ref_arg(cx, mir, &terminator.kind)); let from_borrow = match_def_path(cx, fn_def_id, &paths::CLONE_TRAIT_METHOD) || match_def_path(cx, fn_def_id, &paths::TO_OWNED_METHOD) @@ -126,8 +133,8 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for RedundantClone { statement_index: bbdata.statements.len(), }; - // Cloned local - let local = if from_borrow { + // `Local` to be cloned, and a local of `clone` call's destination + let (local, ret_local) = if from_borrow { // `res = clone(arg)` can be turned into `res = move arg;` // if `arg` is the only borrow of `cloned` at this point. @@ -135,7 +142,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for RedundantClone { continue; } - cloned + (cloned, clone_ret) } else { // `arg` is a reference as it is `.deref()`ed in the previous block. // Look into the predecessor block and find out the source of deref. @@ -147,15 +154,15 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for RedundantClone { let pred_terminator = mir[ps[0]].terminator(); // receiver of the `deref()` call - let pred_arg = if_chain! { - if let Some((pred_fn_def_id, pred_arg, pred_arg_ty, Some(res))) = + let (pred_arg, deref_clone_ret) = if_chain! { + if let Some((pred_fn_def_id, pred_arg, pred_arg_ty, res)) = is_call_with_ref_arg(cx, mir, &pred_terminator.kind); - if res.local == cloned; + if res == cloned; if match_def_path(cx, pred_fn_def_id, &paths::DEREF_TRAIT_METHOD); if match_type(cx, pred_arg_ty, &paths::PATH_BUF) || match_type(cx, pred_arg_ty, &paths::OS_STRING); then { - pred_arg + (pred_arg, res) } else { continue; } @@ -182,25 +189,35 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for RedundantClone { continue; } - local + (local, deref_clone_ret) }; - // `local` cannot be moved out if it is used later - let used_later = traversal::ReversePostorder::new(&mir, bb).skip(1).any(|(tbb, tdata)| { - // Give up on loops - if tdata.terminator().successors().any(|s| *s == bb) { - return true; - } + let is_temp = mir_read_only.local_kind(ret_local) == mir::LocalKind::Temp; + + // 1. `local` can be moved out if it is not used later. + // 2. If `ret_local` is a temporary and is neither consumed nor mutated, we can remove this `clone` + // call anyway. + let (used, consumed_or_mutated) = traversal::ReversePostorder::new(&mir, bb).skip(1).fold( + (false, !is_temp), + |(used, consumed), (tbb, tdata)| { + // Short-circuit + if (used && consumed) || + // Give up on loops + tdata.terminator().successors().any(|s| *s == bb) + { + return (true, true); + } - let mut vis = LocalUseVisitor { - local, - used_other_than_drop: false, - }; - vis.visit_basic_block_data(tbb, tdata); - vis.used_other_than_drop - }); + let mut vis = LocalUseVisitor { + used: (local, false), + consumed_or_mutated: (ret_local, false), + }; + vis.visit_basic_block_data(tbb, tdata); + (used || vis.used.1, consumed || vis.consumed_or_mutated.1) + }, + ); - if !used_later { + if !used || !consumed_or_mutated { let span = terminator.source_info.span; let scope = terminator.source_info.scope; let node = mir.source_scopes[scope] @@ -234,10 +251,17 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for RedundantClone { String::new(), app, ); - db.span_note( - span.with_hi(span.lo() + BytePos(u32::try_from(dot).unwrap())), - "this value is dropped without further use", - ); + if used { + db.span_note( + span, + "cloned value is neither consumed nor mutated", + ); + } else { + db.span_note( + span.with_hi(span.lo() + BytePos(u32::try_from(dot).unwrap())), + "this value is dropped without further use", + ); + } }); } else { span_lint_hir(cx, REDUNDANT_CLONE, node, span, "redundant clone"); @@ -253,7 +277,7 @@ fn is_call_with_ref_arg<'tcx>( cx: &LateContext<'_, 'tcx>, mir: &'tcx mir::Body<'tcx>, kind: &'tcx mir::TerminatorKind<'tcx>, -) -> Option<(def_id::DefId, mir::Local, Ty<'tcx>, Option<&'tcx mir::Place<'tcx>>)> { +) -> Option<(def_id::DefId, mir::Local, Ty<'tcx>, mir::Local)> { if_chain! { if let mir::TerminatorKind::Call { func, args, destination, .. } = kind; if args.len() == 1; @@ -262,7 +286,7 @@ fn is_call_with_ref_arg<'tcx>( if let (inner_ty, 1) = walk_ptrs_ty_depth(args[0].ty(&*mir, cx.tcx)); if !is_copy(cx, inner_ty); then { - Some((def_id, *local, inner_ty, destination.as_ref().map(|(dest, _)| dest))) + Some((def_id, *local, inner_ty, destination.as_ref().map(|(dest, _)| dest)?.as_local()?)) } else { None } @@ -331,8 +355,8 @@ fn base_local_and_movability<'tcx>( } struct LocalUseVisitor { - local: mir::Local, - used_other_than_drop: bool, + used: (mir::Local, bool), + consumed_or_mutated: (mir::Local, bool), } impl<'tcx> mir::visit::Visitor<'tcx> for LocalUseVisitor { @@ -340,11 +364,6 @@ impl<'tcx> mir::visit::Visitor<'tcx> for LocalUseVisitor { let statements = &data.statements; for (statement_index, statement) in statements.iter().enumerate() { self.visit_statement(statement, mir::Location { block, statement_index }); - - // Once flagged, skip remaining statements - if self.used_other_than_drop { - return; - } } self.visit_terminator( @@ -356,14 +375,23 @@ impl<'tcx> mir::visit::Visitor<'tcx> for LocalUseVisitor { ); } - fn visit_local(&mut self, local: &mir::Local, ctx: PlaceContext, _: mir::Location) { - match ctx { - PlaceContext::MutatingUse(MutatingUseContext::Drop) | PlaceContext::NonUse(_) => return, - _ => {}, + fn visit_place(&mut self, place: &mir::Place<'tcx>, ctx: PlaceContext, _: mir::Location) { + let local = place.local; + + if local == self.used.0 + && !matches!(ctx, PlaceContext::MutatingUse(MutatingUseContext::Drop) | PlaceContext::NonUse(_)) + { + self.used.1 = true; } - if *local == self.local { - self.used_other_than_drop = true; + if local == self.consumed_or_mutated.0 { + match ctx { + PlaceContext::NonMutatingUse(NonMutatingUseContext::Move) + | PlaceContext::MutatingUse(MutatingUseContext::Borrow) => { + self.consumed_or_mutated.1 = true; + }, + _ => {}, + } } } } diff --git a/clippy_lints/src/shadow.rs b/clippy_lints/src/shadow.rs index 0dc2705550b9..0ada65ec7855 100644 --- a/clippy_lints/src/shadow.rs +++ b/clippy_lints/src/shadow.rs @@ -154,10 +154,14 @@ fn check_local<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, local: &'tcx Local<'_>, bin } fn is_binding(cx: &LateContext<'_, '_>, pat_id: HirId) -> bool { - let var_ty = cx.tables.node_type(pat_id); - match var_ty.kind { - ty::Adt(..) => false, - _ => true, + let var_ty = cx.tables.node_type_opt(pat_id); + if let Some(var_ty) = var_ty { + match var_ty.kind { + ty::Adt(..) => false, + _ => true, + } + } else { + false } } diff --git a/clippy_lints/src/trivially_copy_pass_by_ref.rs b/clippy_lints/src/trivially_copy_pass_by_ref.rs index e00bc2e090ae..7e9a94f14fef 100644 --- a/clippy_lints/src/trivially_copy_pass_by_ref.rs +++ b/clippy_lints/src/trivially_copy_pass_by_ref.rs @@ -132,7 +132,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for TriviallyCopyPassByRef { return; } - if let hir::TraitItemKind::Method(method_sig, _) = &item.kind { + if let hir::TraitItemKind::Fn(method_sig, _) = &item.kind { self.check_poly_fn(cx, item.hir_id, &*method_sig.decl, None); } } diff --git a/clippy_lints/src/types.rs b/clippy_lints/src/types.rs index e1685c97a065..fbd3f37c5643 100644 --- a/clippy_lints/src/types.rs +++ b/clippy_lints/src/types.rs @@ -204,7 +204,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Types { fn check_trait_item(&mut self, cx: &LateContext<'_, '_>, item: &TraitItem<'_>) { match item.kind { TraitItemKind::Const(ref ty, _) | TraitItemKind::Type(_, Some(ref ty)) => self.check_ty(cx, ty, false), - TraitItemKind::Method(ref sig, _) => self.check_fn_decl(cx, &sig.decl), + TraitItemKind::Fn(ref sig, _) => self.check_fn_decl(cx, &sig.decl), _ => (), } } @@ -1457,7 +1457,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for TypeComplexity { fn check_trait_item(&mut self, cx: &LateContext<'a, 'tcx>, item: &'tcx TraitItem<'_>) { match item.kind { TraitItemKind::Const(ref ty, _) | TraitItemKind::Type(_, Some(ref ty)) => self.check_type(cx, ty), - TraitItemKind::Method(FnSig { ref decl, .. }, TraitMethod::Required(_)) => self.check_fndecl(cx, decl), + TraitItemKind::Fn(FnSig { ref decl, .. }, TraitMethod::Required(_)) => self.check_fndecl(cx, decl), // methods with default impl are covered by check_fn _ => (), } diff --git a/clippy_lints/src/utils/mod.rs b/clippy_lints/src/utils/mod.rs index dc8775b43b19..9f7cf4007657 100644 --- a/clippy_lints/src/utils/mod.rs +++ b/clippy_lints/src/utils/mod.rs @@ -32,7 +32,7 @@ use rustc::ty::{ self, layout::{self, IntegerExt}, subst::GenericArg, - Binder, Ty, TyCtxt, + Binder, Ty, TyCtxt, TypeFoldable, }; use rustc_ast::ast::{self, Attribute, LitKind}; use rustc_attr as attr; @@ -1377,6 +1377,27 @@ pub fn is_trait_impl_item(cx: &LateContext<'_, '_>, hir_id: HirId) -> bool { } } +/// Check if it's even possible to satisfy the `where` clause for the item. +/// +/// `trivial_bounds` feature allows functions with unsatisfiable bounds, for example: +/// +/// ```ignore +/// fn foo() where i32: Iterator { +/// for _ in 2i32 {} +/// } +/// ``` +pub fn fn_has_unsatisfiable_preds(cx: &LateContext<'_, '_>, did: DefId) -> bool { + use rustc_infer::traits; + let predicates = cx + .tcx + .predicates_of(did) + .predicates + .iter() + .filter_map(|(p, _)| if p.is_global() { Some(*p) } else { None }) + .collect(); + !traits::normalize_and_test_predicates(cx.tcx, traits::elaborate_predicates(cx.tcx, predicates).collect()) +} + #[cfg(test)] mod test { use super::{trim_multiline, without_block_comments}; diff --git a/clippy_lints/src/utils/paths.rs b/clippy_lints/src/utils/paths.rs index 5ec04d965c86..6cb1f694fd5e 100644 --- a/clippy_lints/src/utils/paths.rs +++ b/clippy_lints/src/utils/paths.rs @@ -31,6 +31,7 @@ pub const DROP_TRAIT: [&str; 4] = ["core", "ops", "drop", "Drop"]; pub const DURATION: [&str; 3] = ["core", "time", "Duration"]; pub const EARLY_CONTEXT: [&str; 4] = ["rustc", "lint", "context", "EarlyContext"]; pub const EXIT: [&str; 3] = ["std", "process", "exit"]; +pub const FILE: [&str; 3] = ["std", "fs", "File"]; pub const FILE_TYPE: [&str; 3] = ["std", "fs", "FileType"]; pub const FMT_ARGUMENTS_NEW_V1: [&str; 4] = ["core", "fmt", "Arguments", "new_v1"]; pub const FMT_ARGUMENTS_NEW_V1_FORMATTED: [&str; 4] = ["core", "fmt", "Arguments", "new_v1_formatted"]; diff --git a/clippy_lints/src/verbose_file_reads.rs b/clippy_lints/src/verbose_file_reads.rs new file mode 100644 index 000000000000..37885317c58e --- /dev/null +++ b/clippy_lints/src/verbose_file_reads.rs @@ -0,0 +1,83 @@ +use crate::utils::{match_type, paths, span_lint_and_help}; +use if_chain::if_chain; +use rustc_hir::{Expr, ExprKind, QPath}; +use rustc_lint::{LateContext, LateLintPass}; +use rustc_session::{declare_lint_pass, declare_tool_lint}; + +declare_clippy_lint! { + /// **What it does:** Checks for use of File::read_to_end and File::read_to_string. + /// + /// **Why is this bad?** `fs::{read, read_to_string}` provide the same functionality when `buf` is empty with fewer imports and no intermediate values. + /// See also: [fs::read docs](https://doc.rust-lang.org/std/fs/fn.read.html), [fs::read_to_string docs](https://doc.rust-lang.org/std/fs/fn.read_to_string.html) + /// **Known problems:** None. + /// + /// **Example:** + /// + /// ```rust,no_run + /// # use std::io::Read; + /// # use std::fs::File; + /// let mut f = File::open("foo.txt").unwrap(); + /// let mut bytes = Vec::new(); + /// f.read_to_end(&mut bytes).unwrap(); + /// ``` + /// Can be written more concisely as + /// ```rust,no_run + /// # use std::fs; + /// let mut bytes = fs::read("foo.txt").unwrap(); + /// ``` + pub VERBOSE_FILE_READS, + complexity, + "use of `File::read_to_end` or `File::read_to_string`" +} + +declare_lint_pass!(VerboseFileReads => [VERBOSE_FILE_READS]); + +impl<'a, 'tcx> LateLintPass<'a, 'tcx> for VerboseFileReads { + fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr<'tcx>) { + if is_file_read_to_end(cx, expr) { + span_lint_and_help( + cx, + VERBOSE_FILE_READS, + expr.span, + "use of `File::read_to_end`", + "consider using `fs::read` instead", + ); + } else if is_file_read_to_string(cx, expr) { + span_lint_and_help( + cx, + VERBOSE_FILE_READS, + expr.span, + "use of `File::read_to_string`", + "consider using `fs::read_to_string` instead", + ) + } + } +} + +fn is_file_read_to_end<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr<'tcx>) -> bool { + if_chain! { + if let ExprKind::MethodCall(method_name, _, exprs) = expr.kind; + if method_name.ident.as_str() == "read_to_end"; + if let ExprKind::Path(QPath::Resolved(None, _)) = &exprs[0].kind; + let ty = cx.tables.expr_ty(&exprs[0]); + if match_type(cx, ty, &paths::FILE); + then { + return true + } + } + false +} + +fn is_file_read_to_string<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr<'tcx>) -> bool { + if_chain! { + if let ExprKind::MethodCall(method_name, _, exprs) = expr.kind; + if method_name.ident.as_str() == "read_to_string"; + if let ExprKind::Path(QPath::Resolved(None, _)) = &exprs[0].kind; + let ty = cx.tables.expr_ty(&exprs[0]); + if match_type(cx, ty, &paths::FILE); + then { + return true + } + } + false +} diff --git a/doc/adding_lints.md b/doc/adding_lints.md index 7e1263ebf22f..1d78a27820a6 100644 --- a/doc/adding_lints.md +++ b/doc/adding_lints.md @@ -1,4 +1,4 @@ -## Adding a new lint +# Adding a new lint You are probably here because you want to add a new lint to Clippy. If this is the first time you're contributing to Clippy, this document guides you through @@ -25,14 +25,14 @@ because that's clearly a non-descriptive name. - [PR Checklist](#pr-checklist) - [Cheatsheet](#cheatsheet) -### Setup +## Setup When working on Clippy, you will need the current git master version of rustc, which can change rapidly. Make sure you're working near rust-clippy's master, and use the `setup-toolchain.sh` script to configure the appropriate toolchain for the Clippy directory. -### Getting Started +## Getting Started There is a bit of boilerplate code that needs to be set up when creating a new lint. Fortunately, you can use the clippy dev tools to handle this for you. We @@ -45,7 +45,7 @@ two files: `tests/ui/foo_functions.rs` and `clippy_lints/src/foo_functions.rs`, as well as run `cargo dev update_lints` to register the new lint. Next, we'll open up these files and add our lint! -### Testing +## Testing Let's write some tests first that we can execute while we iterate on our lint. @@ -88,12 +88,10 @@ fn main() { let a = A; a.foo(); } - ``` -Now we can run the test with `TESTNAME=foo_functions cargo uitest`. -Currently this test will fail. If you go through the output you will see that we -are told that `clippy::foo_functions` is an unknown lint, which is expected. +Now we can run the test with `TESTNAME=foo_functions cargo uitest`, +currently this test is meaningless though. While we are working on implementing our lint, we can keep running the UI test. That allows us to check if the output is turning into what we want. @@ -105,24 +103,26 @@ every time before running `tests/ui/update-all-references.sh`. Running `TESTNAME=foo_functions cargo uitest` should pass then. When we commit our lint, we need to commit the generated `.stderr` files, too. -### Rustfix tests +## Rustfix tests If the lint you are working on is making use of structured suggestions, the test file should include a `// run-rustfix` comment at the top. This will -additionally run [rustfix](https://github.com/rust-lang-nursery/rustfix) for -that test. Rustfix will apply the suggestions from the lint to the code of the -test file and compare that to the contents of a `.fixed` file. +additionally run [rustfix] for that test. Rustfix will apply the suggestions +from the lint to the code of the test file and compare that to the contents of +a `.fixed` file. Use `tests/ui/update-all-references.sh` to automatically generate the `.fixed` file after running the tests. -### Edition 2018 tests +[rustfix]: https://github.com/rust-lang/rustfix + +## Edition 2018 tests Some features require the 2018 edition to work (e.g. `async_await`), but compile-test tests run on the 2015 edition by default. To change this behavior -add `// compile-flags: --edition 2018` at the top of the test file. +add `// edition:2018` at the top of the test file (note that it's space-sensitive). -### Testing manually +## Testing manually Manually testing against an example file can be useful if you have added some `println!`s and the test suite output becomes unreadable. To try Clippy with @@ -131,7 +131,7 @@ clippy-driver -- -L ./target/debug input.rs` from the working copy root. With tests in place, let's have a look at implementing our lint now. -### Lint declaration +## Lint declaration Let's start by opening the new file created in the `clippy_lints` crate at `clippy_lints/src/foo_functions.rs`. That's the crate where all the @@ -140,7 +140,7 @@ lint code is. This file has already imported some initial things we will need: ```rust use rustc_lint::{EarlyLintPass, EarlyContext}; use rustc_session::{declare_lint_pass, declare_tool_lint}; -use syntax::ast::*; +use rustc_ast::ast::*; ``` The next step is to update the lint declaration. Lints are declared using the @@ -167,12 +167,12 @@ declare_clippy_lint! { ``` * The section of lines prefixed with `///` constitutes the lint documentation - section. This is the default documentation style and will be displayed at - https://rust-lang.github.io/rust-clippy/master/index.html. -* `FOO_FUNCTIONS` is the name of our lint. Be sure to follow the [lint naming - guidelines][lint_naming] here when naming your lint. In short, the name should - state the thing that is being checked for and read well when used with - `allow`/`warn`/`deny`. + section. This is the default documentation style and will be displayed + [like this][example_lint_page]. +* `FOO_FUNCTIONS` is the name of our lint. Be sure to follow the + [lint naming guidelines][lint_naming] here when naming your lint. + In short, the name should state the thing that is being checked for and + read well when used with `allow`/`warn`/`deny`. * `pedantic` sets the lint level to `Allow`. The exact mapping can be found [here][category_level_mapping] * The last part should be a text that explains what exactly is wrong with the @@ -199,14 +199,15 @@ automate everything. We will have to register our lint pass manually in the `register_plugins` function in `clippy_lints/src/lib.rs`: ```rust -store.register_early_pass(box foo_functions::FooFunctions); +store.register_early_pass(|| box foo_functions::FooFunctions); ``` -This should fix the `unknown clippy lint: clippy::foo_functions` error that we -saw when we executed our tests the first time. The next decision we have to make -is which lint pass our lint is going to need. +[declare_clippy_lint]: https://github.com/rust-lang/rust-clippy/blob/557f6848bd5b7183f55c1e1522a326e9e1df6030/clippy_lints/src/lib.rs#L60 +[example_lint_page]: https://rust-lang.github.io/rust-clippy/master/index.html#redundant_closure +[lint_naming]: https://rust-lang.github.io/rfcs/0344-conventions-galore.html#lints +[category_level_mapping]: https://github.com/rust-lang/rust-clippy/blob/557f6848bd5b7183f55c1e1522a326e9e1df6030/clippy_lints/src/lib.rs#L110 -### Lint passes +## Lint passes Writing a lint that only checks for the name of a function means that we only have to deal with the AST and don't have to deal with the type system at all. @@ -224,7 +225,10 @@ Since we don't need type information for checking the function name, we used `--pass=early` when running the new lint automation and all the imports were added accordingly. -### Emitting a lint +[early_lint_pass]: https://doc.rust-lang.org/nightly/nightly-rustc/rustc_lint/trait.EarlyLintPass.html +[late_lint_pass]: https://doc.rust-lang.org/nightly/nightly-rustc/rustc_lint/trait.LateLintPass.html + +## Emitting a lint With UI tests and the lint declaration in place, we can start working on the implementation of the lint logic. @@ -233,7 +237,7 @@ Let's start by implementing the `EarlyLintPass` for our `FooFunctions`: ```rust impl EarlyLintPass for FooFunctions { - fn check_fn(&mut self, cx: &EarlyContext<'_>, fn_kind: FnKind<'_>, _: &FnDecl, span: Span, _: NodeId) { + fn check_fn(&mut self, cx: &EarlyContext<'_>, fn_kind: FnKind<'_>, span: Span, _: NodeId) { // TODO: Emit lint here } } @@ -255,7 +259,7 @@ automatically. This is how it looks: ```rust impl EarlyLintPass for FooFunctions { - fn check_fn(&mut self, cx: &EarlyContext<'_>, _: FnKind<'_>, _: &FnDecl, span: Span, _: NodeId) { + fn check_fn(&mut self, cx: &EarlyContext<'_>, fn_kind: FnKind<'_>, span: Span, _: NodeId) { span_lint_and_help( cx, FOO_FUNCTIONS, @@ -269,20 +273,23 @@ impl EarlyLintPass for FooFunctions { Running our UI test should now produce output that contains the lint message. -### Adding the lint logic +[check_fn]: https://doc.rust-lang.org/nightly/nightly-rustc/rustc_lint/trait.EarlyLintPass.html#method.check_fn +[diagnostics]: https://github.com/rust-lang/rust-clippy/blob/master/clippy_lints/src/utils/diagnostics.rs + +## Adding the lint logic Writing the logic for your lint will most likely be different from our example, so this section is kept rather short. Using the [`check_fn`][check_fn] method gives us access to [`FnKind`][fn_kind] -that has two relevant variants for us `FnKind::ItemFn` and `FnKind::Method`. -Both provide access to the name of the function/method via an [`Ident`][ident]. +that has the [`FnKind::Fn`] variant. It provides access to the name of the +function/method via an [`Ident`][ident]. With that we can expand our `check_fn` method to: ```rust impl EarlyLintPass for FooFunctions { - fn check_fn(&mut self, cx: &EarlyContext<'_>, fn_kind: FnKind<'_>, _: &FnDecl, span: Span, _: NodeId) { + fn check_fn(&mut self, cx: &EarlyContext<'_>, fn_kind: FnKind<'_>, span: Span, _: NodeId) { if is_foo_fn(fn_kind) { span_lint_and_help( cx, @@ -307,9 +314,11 @@ In our example, `is_foo_fn` looks like: fn is_foo_fn(fn_kind: FnKind<'_>) -> bool { match fn_kind { - FnKind::ItemFn(ident, ..) | FnKind::Method(ident, ..) => { - ident.name == "foo" - }, + FnKind::Fn(_, ident, ..) => { + // check if `fn` name is `foo` + ident.name.as_str() == "foo" + } + // ignore closures FnKind::Closure(..) => false } } @@ -325,13 +334,18 @@ implementation is not violating any Clippy lints itself. That should be it for the lint implementation. Running `cargo test` should now pass. -### Author lint +[fn_kind]: https://doc.rust-lang.org/nightly/nightly-rustc/rustc_ast/visit/enum.FnKind.html +[`FnKind::Fn`]: https://doc.rust-lang.org/nightly/nightly-rustc/rustc_ast/visit/enum.FnKind.html#variant.Fn +[ident]: https://doc.rust-lang.org/nightly/nightly-rustc/rustc_span/symbol/struct.Ident.html + +## Author lint If you have trouble implementing your lint, there is also the internal `author` lint to generate Clippy code that detects the offending pattern. It does not work for all of the Rust syntax, but can give a good starting point. -The quickest way to use it, is the [Rust playground: play.rust-lang.org][Play]. +The quickest way to use it, is the +[Rust playground: play.rust-lang.org][author_example]. Put the code you want to lint into the editor and add the `#[clippy::author]` attribute above the item. Then run Clippy via `Tools -> Clippy` and you should see the generated code in the output below. @@ -341,7 +355,9 @@ see the generated code in the output below. If the command was executed successfully, you can copy the code over to where you are implementing your lint. -### Documentation +[author_example]: https://play.rust-lang.org/?version=nightly&mode=debug&edition=2018&gist=9a12cb60e5c6ad4e3003ac6d5e63cf55 + +## Documentation The final thing before submitting our PR is to add some documentation to our lint declaration. @@ -374,11 +390,13 @@ declare_clippy_lint! { Once your lint is merged, this documentation will show up in the [lint list][lint_list]. -### Running rustfmt +[lint_list]: https://rust-lang.github.io/rust-clippy/master/index.html + +## Running rustfmt -[Rustfmt](https://github.com/rust-lang/rustfmt) is a tool for formatting Rust -code according to style guidelines. Your code has to be formatted by `rustfmt` -before a PR can be merged. Clippy uses nightly `rustfmt` in the CI. +[Rustfmt] is a tool for formatting Rust code according to style guidelines. +Your code has to be formatted by `rustfmt` before a PR can be merged. +Clippy uses nightly `rustfmt` in the CI. It can be installed via `rustup`: @@ -389,13 +407,17 @@ rustup component add rustfmt --toolchain=nightly Use `cargo dev fmt` to format the whole codebase. Make sure that `rustfmt` is installed for the nightly toolchain. -### Debugging +[Rustfmt]: https://github.com/rust-lang/rustfmt + +## Debugging -If you want to debug parts of your lint implementation, you can use the `dbg!` +If you want to debug parts of your lint implementation, you can use the [`dbg!`] macro anywhere in your code. Running the tests should then include the debug output in the `stdout` part. -### PR Checklist +[`dbg!`]: https://doc.rust-lang.org/std/macro.dbg.html + +## PR Checklist Before submitting your PR make sure you followed all of the basic requirements: @@ -408,7 +430,7 @@ Before submitting your PR make sure you followed all of the basic requirements: - [ ] Added lint documentation - [ ] Run `cargo dev fmt` -### Cheatsheet +## Cheatsheet Here are some pointers to things you are likely going to need for every lint: @@ -419,45 +441,33 @@ Here are some pointers to things you are likely going to need for every lint: * [`from_expansion`][from_expansion] and [`in_external_macro`][in_external_macro] * [`Span`][span] * [`Applicability`][applicability] -* [The rustc guide][rustc_guide] explains a lot of internal compiler concepts +* [The rustc-dev-guide][rustc-dev-guide] explains a lot of internal compiler concepts * [The nightly rustc docs][nightly_docs] which has been linked to throughout this guide For `EarlyLintPass` lints: * [`EarlyLintPass`][early_lint_pass] -* [`syntax::ast`][ast] +* [`rustc_ast::ast`][ast] For `LateLintPass` lints: * [`LateLintPass`][late_lint_pass] * [`Ty::TyKind`][ty] - While most of Clippy's lint utils are documented, most of rustc's internals lack documentation currently. This is unfortunate, but in most cases you can probably get away with copying things from existing similar lints. If you are stuck, -don't hesitate to ask on Discord, IRC or in the issue/PR. +don't hesitate to ask on [Discord] or in the issue/PR. -[lint_list]: https://rust-lang.github.io/rust-clippy/master/index.html -[lint_naming]: https://rust-lang.github.io/rfcs/0344-conventions-galore.html#lints -[category_level_mapping]: https://github.com/rust-lang/rust-clippy/blob/bd23cb89ec0ea63403a17d3fc5e50c88e38dd54f/clippy_lints/src/lib.rs#L43 -[declare_clippy_lint]: https://github.com/rust-lang/rust-clippy/blob/a71acac1da7eaf667ab90a1d65d10e5cc4b80191/clippy_lints/src/lib.rs#L39 -[check_fn]: https://doc.rust-lang.org/nightly/nightly-rustc/rustc/lint/trait.EarlyLintPass.html#method.check_fn -[early_lint_pass]: https://doc.rust-lang.org/nightly/nightly-rustc/rustc/lint/trait.EarlyLintPass.html -[late_lint_pass]: https://doc.rust-lang.org/nightly/nightly-rustc/rustc/lint/trait.LateLintPass.html -[fn_kind]: https://doc.rust-lang.org/nightly/nightly-rustc/syntax/visit/enum.FnKind.html -[diagnostics]: https://github.com/rust-lang/rust-clippy/blob/master/clippy_lints/src/utils/diagnostics.rs [utils]: https://github.com/rust-lang/rust-clippy/blob/master/clippy_lints/src/utils/mod.rs -[ident]: https://doc.rust-lang.org/nightly/nightly-rustc/syntax/source_map/symbol/struct.Ident.html -[span]: https://doc.rust-lang.org/nightly/nightly-rustc/rustc_span/struct.Span.html -[applicability]: https://doc.rust-lang.org/nightly/nightly-rustc/rustc_errors/enum.Applicability.html [if_chain]: https://docs.rs/if_chain/*/if_chain/ -[ty]: https://doc.rust-lang.org/nightly/nightly-rustc/rustc/ty/sty/index.html -[ast]: https://doc.rust-lang.org/nightly/nightly-rustc/syntax/ast/index.html [from_expansion]: https://doc.rust-lang.org/nightly/nightly-rustc/rustc_span/struct.Span.html#method.from_expansion [in_external_macro]: https://doc.rust-lang.org/nightly/nightly-rustc/rustc/lint/fn.in_external_macro.html -[play]: https://play.rust-lang.org -[author_example]: https://play.rust-lang.org/?version=stable&mode=debug&edition=2018&gist=f093b986e80ad62f3b67a1f24f5e66e2 -[rustc_guide]: https://rust-lang.github.io/rustc-guide/ +[span]: https://doc.rust-lang.org/nightly/nightly-rustc/rustc_span/struct.Span.html +[applicability]: https://doc.rust-lang.org/nightly/nightly-rustc/rustc_errors/enum.Applicability.html +[rustc-dev-guide]: https://rustc-dev-guide.rust-lang.org/ [nightly_docs]: https://doc.rust-lang.org/nightly/nightly-rustc/rustc/ +[ast]: https://doc.rust-lang.org/nightly/nightly-rustc/rustc_ast/ast/index.html +[ty]: https://doc.rust-lang.org/nightly/nightly-rustc/rustc/ty/sty/index.html +[Discord]: https://discord.gg/rust-lang diff --git a/src/lintlist/mod.rs b/src/lintlist/mod.rs index 2b93e4279f0d..fd948953ea23 100644 --- a/src/lintlist/mod.rs +++ b/src/lintlist/mod.rs @@ -6,7 +6,7 @@ pub use lint::Lint; pub use lint::LINT_LEVELS; // begin lint list, do not remove this comment, it’s used in `update_lints` -pub const ALL_LINTS: [Lint; 359] = [ +pub const ALL_LINTS: [Lint; 361] = [ Lint { name: "absurd_extreme_comparisons", group: "correctness", @@ -1015,6 +1015,13 @@ pub const ALL_LINTS: [Lint; 359] = [ deprecation: None, module: "float_literal", }, + Lint { + name: "macro_use_imports", + group: "pedantic", + desc: "#[macro_use] is no longer needed", + deprecation: None, + module: "macro_use", + }, Lint { name: "main_recursion", group: "style", @@ -2394,6 +2401,13 @@ pub const ALL_LINTS: [Lint; 359] = [ deprecation: None, module: "bit_mask", }, + Lint { + name: "verbose_file_reads", + group: "complexity", + desc: "use of `File::read_to_end` or `File::read_to_string`", + deprecation: None, + module: "verbose_file_reads", + }, Lint { name: "while_immutable_condition", group: "correctness", diff --git a/tests/ui/crashes/shadow.rs b/tests/ui/crashes/shadow.rs new file mode 100644 index 000000000000..843e8ef64dcd --- /dev/null +++ b/tests/ui/crashes/shadow.rs @@ -0,0 +1,6 @@ +fn main() { + let x: [i32; { + let u = 2; + 4 + }] = [2; { 4 }]; +} diff --git a/tests/ui/crashes/trivial_bounds.rs b/tests/ui/crashes/trivial_bounds.rs new file mode 100644 index 000000000000..2bb95c18a391 --- /dev/null +++ b/tests/ui/crashes/trivial_bounds.rs @@ -0,0 +1,13 @@ +// run-pass + +#![feature(trivial_bounds)] +#![allow(unused, trivial_bounds)] + +fn test_trivial_bounds() +where + i32: Iterator, +{ + for _ in 2i32 {} +} + +fn main() {} diff --git a/tests/ui/doc_errors.rs b/tests/ui/doc_errors.rs index 1401a658e037..445fc8d31d77 100644 --- a/tests/ui/doc_errors.rs +++ b/tests/ui/doc_errors.rs @@ -1,4 +1,4 @@ -// compile-flags: --edition 2018 +// edition:2018 #![warn(clippy::missing_errors_doc)] use std::io; diff --git a/tests/ui/format.fixed b/tests/ui/format.fixed index 6e100230a3ad..306514769990 100644 --- a/tests/ui/format.fixed +++ b/tests/ui/format.fixed @@ -1,6 +1,6 @@ // run-rustfix -#![allow(clippy::print_literal)] +#![allow(clippy::print_literal, clippy::redundant_clone)] #![warn(clippy::useless_format)] struct Foo(pub String); diff --git a/tests/ui/format.rs b/tests/ui/format.rs index 1fae6603ac09..b604d79cca37 100644 --- a/tests/ui/format.rs +++ b/tests/ui/format.rs @@ -1,6 +1,6 @@ // run-rustfix -#![allow(clippy::print_literal)] +#![allow(clippy::print_literal, clippy::redundant_clone)] #![warn(clippy::useless_format)] struct Foo(pub String); diff --git a/tests/ui/issue_4266.rs b/tests/ui/issue_4266.rs index 1538abb1d776..8a9d5a3d1d56 100644 --- a/tests/ui/issue_4266.rs +++ b/tests/ui/issue_4266.rs @@ -1,4 +1,4 @@ -// compile-flags: --edition 2018 +// edition:2018 #![allow(dead_code)] async fn sink1<'a>(_: &'a str) {} // lint diff --git a/tests/ui/macro_use_imports.rs b/tests/ui/macro_use_imports.rs new file mode 100644 index 000000000000..60c64ee8146e --- /dev/null +++ b/tests/ui/macro_use_imports.rs @@ -0,0 +1,11 @@ +// edition:2018 +#![warn(clippy::macro_use_imports)] + +use std::collections::HashMap; +#[macro_use] +use std::prelude; + +fn main() { + let _ = HashMap::::new(); + println!(); +} diff --git a/tests/ui/macro_use_imports.stderr b/tests/ui/macro_use_imports.stderr new file mode 100644 index 000000000000..b5e3dbec5727 --- /dev/null +++ b/tests/ui/macro_use_imports.stderr @@ -0,0 +1,10 @@ +error: `macro_use` attributes are no longer needed in the Rust 2018 edition + --> $DIR/macro_use_imports.rs:5:1 + | +LL | #[macro_use] + | ^^^^^^^^^^^^ help: remove the attribute and import the macro directly, try: `use std::prelude::` + | + = note: `-D clippy::macro-use-imports` implied by `-D warnings` + +error: aborting due to previous error + diff --git a/tests/ui/match_single_binding.fixed b/tests/ui/match_single_binding.fixed index 932bd6783a15..bc2346a8dbf0 100644 --- a/tests/ui/match_single_binding.fixed +++ b/tests/ui/match_single_binding.fixed @@ -1,13 +1,17 @@ // run-rustfix #![warn(clippy::match_single_binding)] -#![allow(clippy::many_single_char_names, clippy::toplevel_ref_arg)] +#![allow(unused_variables, clippy::many_single_char_names, clippy::toplevel_ref_arg)] struct Point { x: i32, y: i32, } +fn coords() -> Point { + Point { x: 1, y: 2 } +} + fn main() { let a = 1; let b = 2; @@ -60,4 +64,7 @@ fn main() { let mut x = 5; let ref mut mr = x; println!("Got a mutable reference to {}", mr); + // Lint + let Point { x, y } = coords(); + let product = x * y; } diff --git a/tests/ui/match_single_binding.rs b/tests/ui/match_single_binding.rs index 55b0b09a0088..0517b3bbfbf9 100644 --- a/tests/ui/match_single_binding.rs +++ b/tests/ui/match_single_binding.rs @@ -1,13 +1,17 @@ // run-rustfix #![warn(clippy::match_single_binding)] -#![allow(clippy::many_single_char_names, clippy::toplevel_ref_arg)] +#![allow(unused_variables, clippy::many_single_char_names, clippy::toplevel_ref_arg)] struct Point { x: i32, y: i32, } +fn coords() -> Point { + Point { x: 1, y: 2 } +} + fn main() { let a = 1; let b = 2; @@ -72,4 +76,8 @@ fn main() { match x { ref mut mr => println!("Got a mutable reference to {}", mr), } + // Lint + let product = match coords() { + Point { x, y } => x * y, + }; } diff --git a/tests/ui/match_single_binding.stderr b/tests/ui/match_single_binding.stderr index 471aec780d23..05ba9e5f7f3c 100644 --- a/tests/ui/match_single_binding.stderr +++ b/tests/ui/match_single_binding.stderr @@ -1,5 +1,5 @@ error: this match could be written as a `let` statement - --> $DIR/match_single_binding.rs:16:5 + --> $DIR/match_single_binding.rs:20:5 | LL | / match (a, b, c) { LL | | (x, y, z) => { @@ -18,7 +18,7 @@ LL | } | error: this match could be written as a `let` statement - --> $DIR/match_single_binding.rs:22:5 + --> $DIR/match_single_binding.rs:26:5 | LL | / match (a, b, c) { LL | | (x, y, z) => println!("{} {} {}", x, y, z), @@ -32,7 +32,7 @@ LL | println!("{} {} {}", x, y, z); | error: this match could be replaced by its body itself - --> $DIR/match_single_binding.rs:37:5 + --> $DIR/match_single_binding.rs:41:5 | LL | / match a { LL | | _ => println!("whatever"), @@ -40,7 +40,7 @@ LL | | } | |_____^ help: consider using the match body instead: `println!("whatever");` error: this match could be replaced by its body itself - --> $DIR/match_single_binding.rs:41:5 + --> $DIR/match_single_binding.rs:45:5 | LL | / match a { LL | | _ => { @@ -59,7 +59,7 @@ LL | } | error: this match could be replaced by its body itself - --> $DIR/match_single_binding.rs:48:5 + --> $DIR/match_single_binding.rs:52:5 | LL | / match a { LL | | _ => { @@ -81,7 +81,7 @@ LL | } | error: this match could be written as a `let` statement - --> $DIR/match_single_binding.rs:58:5 + --> $DIR/match_single_binding.rs:62:5 | LL | / match p { LL | | Point { x, y } => println!("Coords: ({}, {})", x, y), @@ -95,7 +95,7 @@ LL | println!("Coords: ({}, {})", x, y); | error: this match could be written as a `let` statement - --> $DIR/match_single_binding.rs:62:5 + --> $DIR/match_single_binding.rs:66:5 | LL | / match p { LL | | Point { x: x1, y: y1 } => println!("Coords: ({}, {})", x1, y1), @@ -109,7 +109,7 @@ LL | println!("Coords: ({}, {})", x1, y1); | error: this match could be written as a `let` statement - --> $DIR/match_single_binding.rs:67:5 + --> $DIR/match_single_binding.rs:71:5 | LL | / match x { LL | | ref r => println!("Got a reference to {}", r), @@ -123,7 +123,7 @@ LL | println!("Got a reference to {}", r); | error: this match could be written as a `let` statement - --> $DIR/match_single_binding.rs:72:5 + --> $DIR/match_single_binding.rs:76:5 | LL | / match x { LL | | ref mut mr => println!("Got a mutable reference to {}", mr), @@ -136,5 +136,19 @@ LL | let ref mut mr = x; LL | println!("Got a mutable reference to {}", mr); | -error: aborting due to 9 previous errors +error: this match could be written as a `let` statement + --> $DIR/match_single_binding.rs:80:5 + | +LL | / let product = match coords() { +LL | | Point { x, y } => x * y, +LL | | }; + | |______^ + | +help: consider using `let` statement + | +LL | let Point { x, y } = coords(); +LL | let product = x * y; + | + +error: aborting due to 10 previous errors diff --git a/tests/ui/methods.rs b/tests/ui/methods.rs index bc4484358297..2af33b263405 100644 --- a/tests/ui/methods.rs +++ b/tests/ui/methods.rs @@ -1,5 +1,5 @@ // aux-build:option_helpers.rs -// compile-flags: --edition 2018 +// edition:2018 #![warn(clippy::all, clippy::pedantic)] #![allow( diff --git a/tests/ui/needless_doc_main.rs b/tests/ui/needless_doc_main.rs index 813d2606153b..682d7b3c4ceb 100644 --- a/tests/ui/needless_doc_main.rs +++ b/tests/ui/needless_doc_main.rs @@ -8,7 +8,23 @@ /// unimplemented!(); /// } /// ``` -fn bad_doctest() {} +/// +/// This should, too. +/// +/// ```rust +/// fn main() { +/// unimplemented!(); +/// } +/// ``` +/// +/// This one too. +/// +/// ```no_run +/// fn main() { +/// unimplemented!(); +/// } +/// ``` +fn bad_doctests() {} /// # Examples /// @@ -34,9 +50,25 @@ fn bad_doctest() {} /// assert_eq(1u8, test::black_box(1)); /// } /// ``` +/// +/// We should not lint ignored examples: +/// +/// ```rust,ignore +/// fn main() { +/// unimplemented!(); +/// } +/// ``` +/// +/// Or even non-rust examples: +/// +/// ```text +/// fn main() { +/// is what starts the program +/// } +/// ``` fn no_false_positives() {} fn main() { - bad_doctest(); + bad_doctests(); no_false_positives(); } diff --git a/tests/ui/needless_doc_main.stderr b/tests/ui/needless_doc_main.stderr index 22d145aad585..65d40ee6832f 100644 --- a/tests/ui/needless_doc_main.stderr +++ b/tests/ui/needless_doc_main.stderr @@ -6,5 +6,17 @@ LL | /// fn main() { | = note: `-D clippy::needless-doctest-main` implied by `-D warnings` -error: aborting due to previous error +error: needless `fn main` in doctest + --> $DIR/needless_doc_main.rs:15:4 + | +LL | /// fn main() { + | ^^^^^^^^^^^^ + +error: needless `fn main` in doctest + --> $DIR/needless_doc_main.rs:23:4 + | +LL | /// fn main() { + | ^^^^^^^^^^^^ + +error: aborting due to 3 previous errors diff --git a/tests/ui/option_map_unit_fn_fixable.fixed b/tests/ui/option_map_unit_fn_fixable.fixed index ad153e4fc194..9a0da404cb6d 100644 --- a/tests/ui/option_map_unit_fn_fixable.fixed +++ b/tests/ui/option_map_unit_fn_fixable.fixed @@ -13,6 +13,10 @@ fn plus_one(value: usize) -> usize { value + 1 } +fn option() -> Option { + Some(10) +} + struct HasOption { field: Option, } @@ -73,6 +77,8 @@ fn option_map_unit_fn() { if let Some(value) = x.field { plus_one(value + captured); } - if let Some(ref value) = x.field { do_nothing(value + captured) }} + if let Some(ref value) = x.field { do_nothing(value + captured) } + + if let Some(a) = option() { do_nothing(a) }} fn main() {} diff --git a/tests/ui/option_map_unit_fn_fixable.rs b/tests/ui/option_map_unit_fn_fixable.rs index 6926498341ac..58041b62df35 100644 --- a/tests/ui/option_map_unit_fn_fixable.rs +++ b/tests/ui/option_map_unit_fn_fixable.rs @@ -13,6 +13,10 @@ fn plus_one(value: usize) -> usize { value + 1 } +fn option() -> Option { + Some(10) +} + struct HasOption { field: Option, } @@ -73,6 +77,8 @@ fn option_map_unit_fn() { x.field.map(|value| { { plus_one(value + captured); } }); - x.field.map(|ref value| { do_nothing(value + captured) });} + x.field.map(|ref value| { do_nothing(value + captured) }); + + option().map(do_nothing);} fn main() {} diff --git a/tests/ui/option_map_unit_fn_fixable.stderr b/tests/ui/option_map_unit_fn_fixable.stderr index f993e1931d59..1312c70b6d59 100644 --- a/tests/ui/option_map_unit_fn_fixable.stderr +++ b/tests/ui/option_map_unit_fn_fixable.stderr @@ -1,5 +1,5 @@ -error: called `map(f)` on an `Option` value where `f` is a unit function - --> $DIR/option_map_unit_fn_fixable.rs:34:5 +error: called `map(f)` on an `Option` value where `f` is a function that returns the unit type + --> $DIR/option_map_unit_fn_fixable.rs:38:5 | LL | x.field.map(do_nothing); | ^^^^^^^^^^^^^^^^^^^^^^^- @@ -8,133 +8,141 @@ LL | x.field.map(do_nothing); | = note: `-D clippy::option-map-unit-fn` implied by `-D warnings` -error: called `map(f)` on an `Option` value where `f` is a unit function - --> $DIR/option_map_unit_fn_fixable.rs:36:5 +error: called `map(f)` on an `Option` value where `f` is a function that returns the unit type + --> $DIR/option_map_unit_fn_fixable.rs:40:5 | LL | x.field.map(do_nothing); | ^^^^^^^^^^^^^^^^^^^^^^^- | | | help: try this: `if let Some(x_field) = x.field { do_nothing(x_field) }` -error: called `map(f)` on an `Option` value where `f` is a unit function - --> $DIR/option_map_unit_fn_fixable.rs:38:5 +error: called `map(f)` on an `Option` value where `f` is a function that returns the unit type + --> $DIR/option_map_unit_fn_fixable.rs:42:5 | LL | x.field.map(diverge); | ^^^^^^^^^^^^^^^^^^^^- | | | help: try this: `if let Some(x_field) = x.field { diverge(x_field) }` -error: called `map(f)` on an `Option` value where `f` is a unit closure - --> $DIR/option_map_unit_fn_fixable.rs:44:5 +error: called `map(f)` on an `Option` value where `f` is a closure that returns the unit type + --> $DIR/option_map_unit_fn_fixable.rs:48:5 | LL | x.field.map(|value| x.do_option_nothing(value + captured)); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^- | | | help: try this: `if let Some(value) = x.field { x.do_option_nothing(value + captured) }` -error: called `map(f)` on an `Option` value where `f` is a unit closure - --> $DIR/option_map_unit_fn_fixable.rs:46:5 +error: called `map(f)` on an `Option` value where `f` is a closure that returns the unit type + --> $DIR/option_map_unit_fn_fixable.rs:50:5 | LL | x.field.map(|value| { x.do_option_plus_one(value + captured); }); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^- | | | help: try this: `if let Some(value) = x.field { x.do_option_plus_one(value + captured); }` -error: called `map(f)` on an `Option` value where `f` is a unit closure - --> $DIR/option_map_unit_fn_fixable.rs:49:5 +error: called `map(f)` on an `Option` value where `f` is a closure that returns the unit type + --> $DIR/option_map_unit_fn_fixable.rs:53:5 | LL | x.field.map(|value| do_nothing(value + captured)); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^- | | | help: try this: `if let Some(value) = x.field { do_nothing(value + captured) }` -error: called `map(f)` on an `Option` value where `f` is a unit closure - --> $DIR/option_map_unit_fn_fixable.rs:51:5 +error: called `map(f)` on an `Option` value where `f` is a closure that returns the unit type + --> $DIR/option_map_unit_fn_fixable.rs:55:5 | LL | x.field.map(|value| { do_nothing(value + captured) }); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^- | | | help: try this: `if let Some(value) = x.field { do_nothing(value + captured) }` -error: called `map(f)` on an `Option` value where `f` is a unit closure - --> $DIR/option_map_unit_fn_fixable.rs:53:5 +error: called `map(f)` on an `Option` value where `f` is a closure that returns the unit type + --> $DIR/option_map_unit_fn_fixable.rs:57:5 | LL | x.field.map(|value| { do_nothing(value + captured); }); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^- | | | help: try this: `if let Some(value) = x.field { do_nothing(value + captured); }` -error: called `map(f)` on an `Option` value where `f` is a unit closure - --> $DIR/option_map_unit_fn_fixable.rs:55:5 +error: called `map(f)` on an `Option` value where `f` is a closure that returns the unit type + --> $DIR/option_map_unit_fn_fixable.rs:59:5 | LL | x.field.map(|value| { { do_nothing(value + captured); } }); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^- | | | help: try this: `if let Some(value) = x.field { do_nothing(value + captured); }` -error: called `map(f)` on an `Option` value where `f` is a unit closure - --> $DIR/option_map_unit_fn_fixable.rs:58:5 +error: called `map(f)` on an `Option` value where `f` is a closure that returns the unit type + --> $DIR/option_map_unit_fn_fixable.rs:62:5 | LL | x.field.map(|value| diverge(value + captured)); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^- | | | help: try this: `if let Some(value) = x.field { diverge(value + captured) }` -error: called `map(f)` on an `Option` value where `f` is a unit closure - --> $DIR/option_map_unit_fn_fixable.rs:60:5 +error: called `map(f)` on an `Option` value where `f` is a closure that returns the unit type + --> $DIR/option_map_unit_fn_fixable.rs:64:5 | LL | x.field.map(|value| { diverge(value + captured) }); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^- | | | help: try this: `if let Some(value) = x.field { diverge(value + captured) }` -error: called `map(f)` on an `Option` value where `f` is a unit closure - --> $DIR/option_map_unit_fn_fixable.rs:62:5 +error: called `map(f)` on an `Option` value where `f` is a closure that returns the unit type + --> $DIR/option_map_unit_fn_fixable.rs:66:5 | LL | x.field.map(|value| { diverge(value + captured); }); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^- | | | help: try this: `if let Some(value) = x.field { diverge(value + captured); }` -error: called `map(f)` on an `Option` value where `f` is a unit closure - --> $DIR/option_map_unit_fn_fixable.rs:64:5 +error: called `map(f)` on an `Option` value where `f` is a closure that returns the unit type + --> $DIR/option_map_unit_fn_fixable.rs:68:5 | LL | x.field.map(|value| { { diverge(value + captured); } }); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^- | | | help: try this: `if let Some(value) = x.field { diverge(value + captured); }` -error: called `map(f)` on an `Option` value where `f` is a unit closure - --> $DIR/option_map_unit_fn_fixable.rs:69:5 +error: called `map(f)` on an `Option` value where `f` is a closure that returns the unit type + --> $DIR/option_map_unit_fn_fixable.rs:73:5 | LL | x.field.map(|value| { let y = plus_one(value + captured); }); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^- | | | help: try this: `if let Some(value) = x.field { let y = plus_one(value + captured); }` -error: called `map(f)` on an `Option` value where `f` is a unit closure - --> $DIR/option_map_unit_fn_fixable.rs:71:5 +error: called `map(f)` on an `Option` value where `f` is a closure that returns the unit type + --> $DIR/option_map_unit_fn_fixable.rs:75:5 | LL | x.field.map(|value| { plus_one(value + captured); }); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^- | | | help: try this: `if let Some(value) = x.field { plus_one(value + captured); }` -error: called `map(f)` on an `Option` value where `f` is a unit closure - --> $DIR/option_map_unit_fn_fixable.rs:73:5 +error: called `map(f)` on an `Option` value where `f` is a closure that returns the unit type + --> $DIR/option_map_unit_fn_fixable.rs:77:5 | LL | x.field.map(|value| { { plus_one(value + captured); } }); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^- | | | help: try this: `if let Some(value) = x.field { plus_one(value + captured); }` -error: called `map(f)` on an `Option` value where `f` is a unit closure - --> $DIR/option_map_unit_fn_fixable.rs:76:5 +error: called `map(f)` on an `Option` value where `f` is a closure that returns the unit type + --> $DIR/option_map_unit_fn_fixable.rs:80:5 | -LL | x.field.map(|ref value| { do_nothing(value + captured) });} +LL | x.field.map(|ref value| { do_nothing(value + captured) }); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^- | | | help: try this: `if let Some(ref value) = x.field { do_nothing(value + captured) }` -error: aborting due to 17 previous errors +error: called `map(f)` on an `Option` value where `f` is a function that returns the unit type + --> $DIR/option_map_unit_fn_fixable.rs:82:5 + | +LL | option().map(do_nothing);} + | ^^^^^^^^^^^^^^^^^^^^^^^^- + | | + | help: try this: `if let Some(a) = option() { do_nothing(a) }` + +error: aborting due to 18 previous errors diff --git a/tests/ui/patterns.fixed b/tests/ui/patterns.fixed index dfe27e193b97..f22388154499 100644 --- a/tests/ui/patterns.fixed +++ b/tests/ui/patterns.fixed @@ -17,4 +17,20 @@ fn main() { [x, inside @ .., y] => (), // no error [..] => (), } + + let mut mutv = vec![1, 2, 3]; + + // required "ref" left out in suggestion: #5271 + match mutv { + ref mut x => { + x.push(4); + println!("vec: {:?}", x); + }, + ref y if y == &vec![0] => (), + } + + match mutv { + ref x => println!("vec: {:?}", x), + ref y if y == &vec![0] => (), + } } diff --git a/tests/ui/patterns.rs b/tests/ui/patterns.rs index bd202fc04205..5848ecd38d98 100644 --- a/tests/ui/patterns.rs +++ b/tests/ui/patterns.rs @@ -17,4 +17,20 @@ fn main() { [x, inside @ .., y] => (), // no error [..] => (), } + + let mut mutv = vec![1, 2, 3]; + + // required "ref" left out in suggestion: #5271 + match mutv { + ref mut x @ _ => { + x.push(4); + println!("vec: {:?}", x); + }, + ref y if y == &vec![0] => (), + } + + match mutv { + ref x @ _ => println!("vec: {:?}", x), + ref y if y == &vec![0] => (), + } } diff --git a/tests/ui/patterns.stderr b/tests/ui/patterns.stderr index f25e71e872b2..af067580688b 100644 --- a/tests/ui/patterns.stderr +++ b/tests/ui/patterns.stderr @@ -6,5 +6,17 @@ LL | y @ _ => (), | = note: `-D clippy::redundant-pattern` implied by `-D warnings` -error: aborting due to previous error +error: the `x @ _` pattern can be written as just `x` + --> $DIR/patterns.rs:25:9 + | +LL | ref mut x @ _ => { + | ^^^^^^^^^^^^^ help: try: `ref mut x` + +error: the `x @ _` pattern can be written as just `x` + --> $DIR/patterns.rs:33:9 + | +LL | ref x @ _ => println!("vec: {:?}", x), + | ^^^^^^^^^ help: try: `ref x` + +error: aborting due to 3 previous errors diff --git a/tests/ui/redundant_clone.fixed b/tests/ui/redundant_clone.fixed index 84931f66fa8d..54815603c6de 100644 --- a/tests/ui/redundant_clone.fixed +++ b/tests/ui/redundant_clone.fixed @@ -50,6 +50,7 @@ fn main() { cannot_double_move(Alpha); cannot_move_from_type_with_drop(); borrower_propagation(); + not_consumed(); } #[derive(Clone)] @@ -136,3 +137,26 @@ fn borrower_propagation() { let _f = f.clone(); // ok } } + +fn not_consumed() { + let x = std::path::PathBuf::from("home"); + let y = x.join("matthias"); + // join() creates a new owned PathBuf, does not take a &mut to x variable, thus the .clone() is + // redundant. (It also does not consume the PathBuf) + + println!("x: {:?}, y: {:?}", x, y); + + let mut s = String::new(); + s.clone().push_str("foo"); // OK, removing this `clone()` will change the behavior. + s.push_str("bar"); + assert_eq!(s, "bar"); + + let t = Some(s); + // OK + if let Some(x) = t.clone() { + println!("{}", x); + } + if let Some(x) = t { + println!("{}", x); + } +} diff --git a/tests/ui/redundant_clone.rs b/tests/ui/redundant_clone.rs index 9ea2de9a3daa..a9b31183adc8 100644 --- a/tests/ui/redundant_clone.rs +++ b/tests/ui/redundant_clone.rs @@ -50,6 +50,7 @@ fn main() { cannot_double_move(Alpha); cannot_move_from_type_with_drop(); borrower_propagation(); + not_consumed(); } #[derive(Clone)] @@ -136,3 +137,26 @@ fn borrower_propagation() { let _f = f.clone(); // ok } } + +fn not_consumed() { + let x = std::path::PathBuf::from("home"); + let y = x.clone().join("matthias"); + // join() creates a new owned PathBuf, does not take a &mut to x variable, thus the .clone() is + // redundant. (It also does not consume the PathBuf) + + println!("x: {:?}, y: {:?}", x, y); + + let mut s = String::new(); + s.clone().push_str("foo"); // OK, removing this `clone()` will change the behavior. + s.push_str("bar"); + assert_eq!(s, "bar"); + + let t = Some(s); + // OK + if let Some(x) = t.clone() { + println!("{}", x); + } + if let Some(x) = t { + println!("{}", x); + } +} diff --git a/tests/ui/redundant_clone.stderr b/tests/ui/redundant_clone.stderr index 0f185cda0197..9c27812b9cdc 100644 --- a/tests/ui/redundant_clone.stderr +++ b/tests/ui/redundant_clone.stderr @@ -108,52 +108,64 @@ LL | let _t = tup.0.clone(); | ^^^^^ error: redundant clone - --> $DIR/redundant_clone.rs:59:22 + --> $DIR/redundant_clone.rs:60:22 | LL | (a.clone(), a.clone()) | ^^^^^^^^ help: remove this | note: this value is dropped without further use - --> $DIR/redundant_clone.rs:59:21 + --> $DIR/redundant_clone.rs:60:21 | LL | (a.clone(), a.clone()) | ^ error: redundant clone - --> $DIR/redundant_clone.rs:119:15 + --> $DIR/redundant_clone.rs:120:15 | LL | let _s = s.clone(); | ^^^^^^^^ help: remove this | note: this value is dropped without further use - --> $DIR/redundant_clone.rs:119:14 + --> $DIR/redundant_clone.rs:120:14 | LL | let _s = s.clone(); | ^ error: redundant clone - --> $DIR/redundant_clone.rs:120:15 + --> $DIR/redundant_clone.rs:121:15 | LL | let _t = t.clone(); | ^^^^^^^^ help: remove this | note: this value is dropped without further use - --> $DIR/redundant_clone.rs:120:14 + --> $DIR/redundant_clone.rs:121:14 | LL | let _t = t.clone(); | ^ error: redundant clone - --> $DIR/redundant_clone.rs:130:19 + --> $DIR/redundant_clone.rs:131:19 | LL | let _f = f.clone(); | ^^^^^^^^ help: remove this | note: this value is dropped without further use - --> $DIR/redundant_clone.rs:130:18 + --> $DIR/redundant_clone.rs:131:18 | LL | let _f = f.clone(); | ^ -error: aborting due to 13 previous errors +error: redundant clone + --> $DIR/redundant_clone.rs:143:14 + | +LL | let y = x.clone().join("matthias"); + | ^^^^^^^^ help: remove this + | +note: cloned value is neither consumed nor mutated + --> $DIR/redundant_clone.rs:143:13 + | +LL | let y = x.clone().join("matthias"); + | ^^^^^^^^^ + +error: aborting due to 14 previous errors diff --git a/tests/ui/result_map_unit_fn_fixable.stderr b/tests/ui/result_map_unit_fn_fixable.stderr index 33be39e34f15..467e00263cd3 100644 --- a/tests/ui/result_map_unit_fn_fixable.stderr +++ b/tests/ui/result_map_unit_fn_fixable.stderr @@ -1,4 +1,4 @@ -error: called `map(f)` on an `Result` value where `f` is a unit function +error: called `map(f)` on an `Result` value where `f` is a function that returns the unit type --> $DIR/result_map_unit_fn_fixable.rs:35:5 | LL | x.field.map(do_nothing); @@ -8,7 +8,7 @@ LL | x.field.map(do_nothing); | = note: `-D clippy::result-map-unit-fn` implied by `-D warnings` -error: called `map(f)` on an `Result` value where `f` is a unit function +error: called `map(f)` on an `Result` value where `f` is a function that returns the unit type --> $DIR/result_map_unit_fn_fixable.rs:37:5 | LL | x.field.map(do_nothing); @@ -16,7 +16,7 @@ LL | x.field.map(do_nothing); | | | help: try this: `if let Ok(x_field) = x.field { do_nothing(x_field) }` -error: called `map(f)` on an `Result` value where `f` is a unit function +error: called `map(f)` on an `Result` value where `f` is a function that returns the unit type --> $DIR/result_map_unit_fn_fixable.rs:39:5 | LL | x.field.map(diverge); @@ -24,7 +24,7 @@ LL | x.field.map(diverge); | | | help: try this: `if let Ok(x_field) = x.field { diverge(x_field) }` -error: called `map(f)` on an `Result` value where `f` is a unit closure +error: called `map(f)` on an `Result` value where `f` is a closure that returns the unit type --> $DIR/result_map_unit_fn_fixable.rs:45:5 | LL | x.field.map(|value| x.do_result_nothing(value + captured)); @@ -32,7 +32,7 @@ LL | x.field.map(|value| x.do_result_nothing(value + captured)); | | | help: try this: `if let Ok(value) = x.field { x.do_result_nothing(value + captured) }` -error: called `map(f)` on an `Result` value where `f` is a unit closure +error: called `map(f)` on an `Result` value where `f` is a closure that returns the unit type --> $DIR/result_map_unit_fn_fixable.rs:47:5 | LL | x.field.map(|value| { x.do_result_plus_one(value + captured); }); @@ -40,7 +40,7 @@ LL | x.field.map(|value| { x.do_result_plus_one(value + captured); }); | | | help: try this: `if let Ok(value) = x.field { x.do_result_plus_one(value + captured); }` -error: called `map(f)` on an `Result` value where `f` is a unit closure +error: called `map(f)` on an `Result` value where `f` is a closure that returns the unit type --> $DIR/result_map_unit_fn_fixable.rs:50:5 | LL | x.field.map(|value| do_nothing(value + captured)); @@ -48,7 +48,7 @@ LL | x.field.map(|value| do_nothing(value + captured)); | | | help: try this: `if let Ok(value) = x.field { do_nothing(value + captured) }` -error: called `map(f)` on an `Result` value where `f` is a unit closure +error: called `map(f)` on an `Result` value where `f` is a closure that returns the unit type --> $DIR/result_map_unit_fn_fixable.rs:52:5 | LL | x.field.map(|value| { do_nothing(value + captured) }); @@ -56,7 +56,7 @@ LL | x.field.map(|value| { do_nothing(value + captured) }); | | | help: try this: `if let Ok(value) = x.field { do_nothing(value + captured) }` -error: called `map(f)` on an `Result` value where `f` is a unit closure +error: called `map(f)` on an `Result` value where `f` is a closure that returns the unit type --> $DIR/result_map_unit_fn_fixable.rs:54:5 | LL | x.field.map(|value| { do_nothing(value + captured); }); @@ -64,7 +64,7 @@ LL | x.field.map(|value| { do_nothing(value + captured); }); | | | help: try this: `if let Ok(value) = x.field { do_nothing(value + captured); }` -error: called `map(f)` on an `Result` value where `f` is a unit closure +error: called `map(f)` on an `Result` value where `f` is a closure that returns the unit type --> $DIR/result_map_unit_fn_fixable.rs:56:5 | LL | x.field.map(|value| { { do_nothing(value + captured); } }); @@ -72,7 +72,7 @@ LL | x.field.map(|value| { { do_nothing(value + captured); } }); | | | help: try this: `if let Ok(value) = x.field { do_nothing(value + captured); }` -error: called `map(f)` on an `Result` value where `f` is a unit closure +error: called `map(f)` on an `Result` value where `f` is a closure that returns the unit type --> $DIR/result_map_unit_fn_fixable.rs:59:5 | LL | x.field.map(|value| diverge(value + captured)); @@ -80,7 +80,7 @@ LL | x.field.map(|value| diverge(value + captured)); | | | help: try this: `if let Ok(value) = x.field { diverge(value + captured) }` -error: called `map(f)` on an `Result` value where `f` is a unit closure +error: called `map(f)` on an `Result` value where `f` is a closure that returns the unit type --> $DIR/result_map_unit_fn_fixable.rs:61:5 | LL | x.field.map(|value| { diverge(value + captured) }); @@ -88,7 +88,7 @@ LL | x.field.map(|value| { diverge(value + captured) }); | | | help: try this: `if let Ok(value) = x.field { diverge(value + captured) }` -error: called `map(f)` on an `Result` value where `f` is a unit closure +error: called `map(f)` on an `Result` value where `f` is a closure that returns the unit type --> $DIR/result_map_unit_fn_fixable.rs:63:5 | LL | x.field.map(|value| { diverge(value + captured); }); @@ -96,7 +96,7 @@ LL | x.field.map(|value| { diverge(value + captured); }); | | | help: try this: `if let Ok(value) = x.field { diverge(value + captured); }` -error: called `map(f)` on an `Result` value where `f` is a unit closure +error: called `map(f)` on an `Result` value where `f` is a closure that returns the unit type --> $DIR/result_map_unit_fn_fixable.rs:65:5 | LL | x.field.map(|value| { { diverge(value + captured); } }); @@ -104,7 +104,7 @@ LL | x.field.map(|value| { { diverge(value + captured); } }); | | | help: try this: `if let Ok(value) = x.field { diverge(value + captured); }` -error: called `map(f)` on an `Result` value where `f` is a unit closure +error: called `map(f)` on an `Result` value where `f` is a closure that returns the unit type --> $DIR/result_map_unit_fn_fixable.rs:70:5 | LL | x.field.map(|value| { let y = plus_one(value + captured); }); @@ -112,7 +112,7 @@ LL | x.field.map(|value| { let y = plus_one(value + captured); }); | | | help: try this: `if let Ok(value) = x.field { let y = plus_one(value + captured); }` -error: called `map(f)` on an `Result` value where `f` is a unit closure +error: called `map(f)` on an `Result` value where `f` is a closure that returns the unit type --> $DIR/result_map_unit_fn_fixable.rs:72:5 | LL | x.field.map(|value| { plus_one(value + captured); }); @@ -120,7 +120,7 @@ LL | x.field.map(|value| { plus_one(value + captured); }); | | | help: try this: `if let Ok(value) = x.field { plus_one(value + captured); }` -error: called `map(f)` on an `Result` value where `f` is a unit closure +error: called `map(f)` on an `Result` value where `f` is a closure that returns the unit type --> $DIR/result_map_unit_fn_fixable.rs:74:5 | LL | x.field.map(|value| { { plus_one(value + captured); } }); @@ -128,7 +128,7 @@ LL | x.field.map(|value| { { plus_one(value + captured); } }); | | | help: try this: `if let Ok(value) = x.field { plus_one(value + captured); }` -error: called `map(f)` on an `Result` value where `f` is a unit closure +error: called `map(f)` on an `Result` value where `f` is a closure that returns the unit type --> $DIR/result_map_unit_fn_fixable.rs:77:5 | LL | x.field.map(|ref value| { do_nothing(value + captured) }); diff --git a/tests/ui/single_component_path_imports.fixed b/tests/ui/single_component_path_imports.fixed index 9095069b0939..a7a8499b58f0 100644 --- a/tests/ui/single_component_path_imports.fixed +++ b/tests/ui/single_component_path_imports.fixed @@ -1,5 +1,5 @@ // run-rustfix -// compile-flags: --edition 2018 +// edition:2018 #![warn(clippy::single_component_path_imports)] #![allow(unused_imports)] diff --git a/tests/ui/single_component_path_imports.rs b/tests/ui/single_component_path_imports.rs index 1ebb3ee59b8b..9a427e90ad3d 100644 --- a/tests/ui/single_component_path_imports.rs +++ b/tests/ui/single_component_path_imports.rs @@ -1,5 +1,5 @@ // run-rustfix -// compile-flags: --edition 2018 +// edition:2018 #![warn(clippy::single_component_path_imports)] #![allow(unused_imports)] diff --git a/tests/ui/use_self.fixed b/tests/ui/use_self.fixed index 802de31b783f..ebb3aa28daf3 100644 --- a/tests/ui/use_self.fixed +++ b/tests/ui/use_self.fixed @@ -1,5 +1,5 @@ // run-rustfix -// compile-flags: --edition 2018 +// edition:2018 #![warn(clippy::use_self)] #![allow(dead_code)] diff --git a/tests/ui/use_self.rs b/tests/ui/use_self.rs index 605c4f8c41fd..8a182192ab34 100644 --- a/tests/ui/use_self.rs +++ b/tests/ui/use_self.rs @@ -1,5 +1,5 @@ // run-rustfix -// compile-flags: --edition 2018 +// edition:2018 #![warn(clippy::use_self)] #![allow(dead_code)] diff --git a/tests/ui/verbose_file_reads.rs b/tests/ui/verbose_file_reads.rs new file mode 100644 index 000000000000..e0065e05ade6 --- /dev/null +++ b/tests/ui/verbose_file_reads.rs @@ -0,0 +1,28 @@ +#![warn(clippy::verbose_file_reads)] +use std::env::temp_dir; +use std::fs::File; +use std::io::Read; + +struct Struct; +// To make sure we only warn on File::{read_to_end, read_to_string} calls +impl Struct { + pub fn read_to_end(&self) {} + + pub fn read_to_string(&self) {} +} + +fn main() -> std::io::Result<()> { + let path = "foo.txt"; + // Lint shouldn't catch this + let s = Struct; + s.read_to_end(); + s.read_to_string(); + // Should catch this + let mut f = File::open(&path)?; + let mut buffer = Vec::new(); + f.read_to_end(&mut buffer)?; + // ...and this + let mut string_buffer = String::new(); + f.read_to_string(&mut string_buffer)?; + Ok(()) +} diff --git a/tests/ui/verbose_file_reads.stderr b/tests/ui/verbose_file_reads.stderr new file mode 100644 index 000000000000..550b6ab679f1 --- /dev/null +++ b/tests/ui/verbose_file_reads.stderr @@ -0,0 +1,19 @@ +error: use of `File::read_to_end` + --> $DIR/verbose_file_reads.rs:23:5 + | +LL | f.read_to_end(&mut buffer)?; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: `-D clippy::verbose-file-reads` implied by `-D warnings` + = help: consider using `fs::read` instead + +error: use of `File::read_to_string` + --> $DIR/verbose_file_reads.rs:26:5 + | +LL | f.read_to_string(&mut string_buffer)?; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = help: consider using `fs::read_to_string` instead + +error: aborting due to 2 previous errors +