diff --git a/src/tools/clippy/.github/workflows/clippy.yml b/src/tools/clippy/.github/workflows/clippy.yml index cf4aa39e49b90..530e60001f726 100644 --- a/src/tools/clippy/.github/workflows/clippy.yml +++ b/src/tools/clippy/.github/workflows/clippy.yml @@ -35,29 +35,11 @@ jobs: with: github_token: "${{ secrets.github_token }}" - - name: rust-toolchain - uses: actions-rs/toolchain@v1.0.6 - with: - toolchain: nightly - target: x86_64-unknown-linux-gnu - profile: minimal - - name: Checkout uses: actions/checkout@v2.3.3 - - name: Run cargo update - run: cargo update - - - name: Cache cargo dir - uses: actions/cache@v2 - with: - path: ~/.cargo - key: ${{ runner.os }}-x86_64-unknown-linux-gnu-${{ hashFiles('Cargo.lock') }} - restore-keys: | - ${{ runner.os }}-x86_64-unknown-linux-gnu - - - name: Master Toolchain Setup - run: bash setup-toolchain.sh + - name: Install toolchain + run: rustup show active-toolchain # Run - name: Set LD_LIBRARY_PATH (Linux) @@ -66,13 +48,13 @@ jobs: echo "LD_LIBRARY_PATH=${SYSROOT}/lib${LD_LIBRARY_PATH+:${LD_LIBRARY_PATH}}" >> $GITHUB_ENV - name: Build - run: cargo build --features deny-warnings + run: cargo build --features deny-warnings,internal-lints - name: Test - run: cargo test --features deny-warnings + run: cargo test --features deny-warnings,internal-lints - name: Test clippy_lints - run: cargo test --features deny-warnings + run: cargo test --features deny-warnings,internal-lints working-directory: clippy_lints - name: Test rustc_tools_util @@ -98,9 +80,3 @@ jobs: cargo dev new_lint --name new_late_pass --pass late cargo check git reset --hard HEAD - - # Cleanup - - name: Run cargo-cache --autoclean - run: | - cargo +nightly install cargo-cache --no-default-features --features ci-autoclean cargo-cache - cargo cache diff --git a/src/tools/clippy/.github/workflows/clippy_bors.yml b/src/tools/clippy/.github/workflows/clippy_bors.yml index 784463fe0df9e..5d846eb64c78e 100644 --- a/src/tools/clippy/.github/workflows/clippy_bors.yml +++ b/src/tools/clippy/.github/workflows/clippy_bors.yml @@ -23,6 +23,7 @@ jobs: - uses: rust-lang/simpleinfra/github-actions/cancel-outdated-builds@master with: github_token: "${{ secrets.github_token }}" + - name: Checkout uses: actions/checkout@v2.3.3 with: @@ -84,31 +85,11 @@ jobs: sudo apt-get install gcc-multilib libssl-dev:i386 libgit2-dev:i386 if: matrix.host == 'i686-unknown-linux-gnu' - - name: rust-toolchain - uses: actions-rs/toolchain@v1.0.6 - with: - toolchain: nightly - target: ${{ matrix.host }} - profile: minimal - - name: Checkout uses: actions/checkout@v2.3.3 - - name: Run cargo update - run: cargo update - - - name: Cache cargo dir - uses: actions/cache@v2 - with: - path: ~/.cargo - key: ${{ runner.os }}-${{ matrix.host }}-${{ hashFiles('Cargo.lock') }} - restore-keys: | - ${{ runner.os }}-${{ matrix.host }} - - - name: Master Toolchain Setup - run: bash setup-toolchain.sh - env: - HOST_TOOLCHAIN: ${{ matrix.host }} + - name: Install toolchain + run: rustup show active-toolchain # Run - name: Set LD_LIBRARY_PATH (Linux) @@ -128,13 +109,13 @@ jobs: SYSROOT=$(rustc --print sysroot) echo "$SYSROOT/bin" >> $GITHUB_PATH - - name: Build with internal lints + - name: Build run: cargo build --features deny-warnings,internal-lints - - name: Test with internal lints + - name: Test run: cargo test --features deny-warnings,internal-lints - - name: Test clippy_lints with internal lints + - name: Test clippy_lints run: cargo test --features deny-warnings,internal-lints working-directory: clippy_lints @@ -155,12 +136,6 @@ jobs: env: OS: ${{ runner.os }} - # Cleanup - - name: Run cargo-cache --autoclean - run: | - cargo +nightly install cargo-cache --no-default-features --features ci-autoclean cargo-cache - cargo cache - integration_build: needs: changelog runs-on: ubuntu-latest @@ -171,29 +146,11 @@ jobs: with: github_token: "${{ secrets.github_token }}" - - name: rust-toolchain - uses: actions-rs/toolchain@v1.0.6 - with: - toolchain: nightly - target: x86_64-unknown-linux-gnu - profile: minimal - - name: Checkout uses: actions/checkout@v2.3.3 - - name: Run cargo update - run: cargo update - - - name: Cache cargo dir - uses: actions/cache@v2 - with: - path: ~/.cargo - key: ${{ runner.os }}-x86_64-unknown-linux-gnu-${{ hashFiles('Cargo.lock') }} - restore-keys: | - ${{ runner.os }}-x86_64-unknown-linux-gnu - - - name: Master Toolchain Setup - run: bash setup-toolchain.sh + - name: Install toolchain + run: rustup show active-toolchain # Run - name: Build Integration Test @@ -214,11 +171,6 @@ jobs: name: target path: target - # Cleanup - - name: Run cargo-cache --autoclean - run: | - cargo +nightly install cargo-cache --no-default-features --features ci-autoclean cargo-cache - cargo cache integration: needs: integration_build strategy: @@ -252,29 +204,11 @@ jobs: with: github_token: "${{ secrets.github_token }}" - - name: rust-toolchain - uses: actions-rs/toolchain@v1.0.6 - with: - toolchain: nightly - target: x86_64-unknown-linux-gnu - profile: minimal - - name: Checkout uses: actions/checkout@v2.3.3 - - name: Run cargo update - run: cargo update - - - name: Cache cargo dir - uses: actions/cache@v2 - with: - path: ~/.cargo - key: ${{ runner.os }}-x86_64-unknown-linux-gnu-${{ hashFiles('Cargo.lock') }} - restore-keys: | - ${{ runner.os }}-x86_64-unknown-linux-gnu - - - name: Master Toolchain Setup - run: bash setup-toolchain.sh + - name: Install toolchain + run: rustup show active-toolchain # Download - name: Download target dir @@ -288,16 +222,11 @@ jobs: # Run - name: Test ${{ matrix.integration }} - run: $CARGO_TARGET_DIR/debug/integration + run: | + RUSTUP_TOOLCHAIN="$(rustup show active-toolchain | grep -o -E "nightly-[0-9]{4}-[0-9]{2}-[0-9]{2}")" \ + $CARGO_TARGET_DIR/debug/integration env: INTEGRATION: ${{ matrix.integration }} - RUSTUP_TOOLCHAIN: master - - # Cleanup - - name: Run cargo-cache --autoclean - run: | - cargo +nightly install cargo-cache --no-default-features --features ci-autoclean cargo-cache - cargo cache # These jobs doesn't actually test anything, but they're only used to tell # bors the build completed, as there is no practical way to detect when a diff --git a/src/tools/clippy/.github/workflows/clippy_dev.yml b/src/tools/clippy/.github/workflows/clippy_dev.yml index 5ee157cf23b86..95da775b7bc36 100644 --- a/src/tools/clippy/.github/workflows/clippy_dev.yml +++ b/src/tools/clippy/.github/workflows/clippy_dev.yml @@ -22,6 +22,12 @@ jobs: steps: # Setup + - name: Checkout + uses: actions/checkout@v2.3.3 + + - name: remove toolchain file + run: rm rust-toolchain + - name: rust-toolchain uses: actions-rs/toolchain@v1.0.6 with: @@ -29,9 +35,7 @@ jobs: target: x86_64-unknown-linux-gnu profile: minimal components: rustfmt - - - name: Checkout - uses: actions/checkout@v2.3.3 + default: true # Run - name: Build diff --git a/src/tools/clippy/CHANGELOG.md b/src/tools/clippy/CHANGELOG.md index c7e02aaf4e188..af3b1c1db2aed 100644 --- a/src/tools/clippy/CHANGELOG.md +++ b/src/tools/clippy/CHANGELOG.md @@ -2006,6 +2006,7 @@ Released 2018-09-13 [`possible_missing_comma`]: https://rust-lang.github.io/rust-clippy/master/index.html#possible_missing_comma [`precedence`]: https://rust-lang.github.io/rust-clippy/master/index.html#precedence [`print_literal`]: https://rust-lang.github.io/rust-clippy/master/index.html#print_literal +[`print_stderr`]: https://rust-lang.github.io/rust-clippy/master/index.html#print_stderr [`print_stdout`]: https://rust-lang.github.io/rust-clippy/master/index.html#print_stdout [`print_with_newline`]: https://rust-lang.github.io/rust-clippy/master/index.html#print_with_newline [`println_empty_string`]: https://rust-lang.github.io/rust-clippy/master/index.html#println_empty_string @@ -2024,6 +2025,7 @@ Released 2018-09-13 [`redundant_closure`]: https://rust-lang.github.io/rust-clippy/master/index.html#redundant_closure [`redundant_closure_call`]: https://rust-lang.github.io/rust-clippy/master/index.html#redundant_closure_call [`redundant_closure_for_method_calls`]: https://rust-lang.github.io/rust-clippy/master/index.html#redundant_closure_for_method_calls +[`redundant_else`]: https://rust-lang.github.io/rust-clippy/master/index.html#redundant_else [`redundant_field_names`]: https://rust-lang.github.io/rust-clippy/master/index.html#redundant_field_names [`redundant_pattern`]: https://rust-lang.github.io/rust-clippy/master/index.html#redundant_pattern [`redundant_pattern_matching`]: https://rust-lang.github.io/rust-clippy/master/index.html#redundant_pattern_matching @@ -2169,5 +2171,6 @@ Released 2018-09-13 [`zero_divided_by_zero`]: https://rust-lang.github.io/rust-clippy/master/index.html#zero_divided_by_zero [`zero_prefixed_literal`]: https://rust-lang.github.io/rust-clippy/master/index.html#zero_prefixed_literal [`zero_ptr`]: https://rust-lang.github.io/rust-clippy/master/index.html#zero_ptr +[`zero_sized_map_values`]: https://rust-lang.github.io/rust-clippy/master/index.html#zero_sized_map_values [`zst_offset`]: https://rust-lang.github.io/rust-clippy/master/index.html#zst_offset diff --git a/src/tools/clippy/CONTRIBUTING.md b/src/tools/clippy/CONTRIBUTING.md index f8c26e2d456d7..4cfeaa153a01b 100644 --- a/src/tools/clippy/CONTRIBUTING.md +++ b/src/tools/clippy/CONTRIBUTING.md @@ -19,10 +19,10 @@ All contributors are expected to follow the [Rust Code of Conduct]. - [Writing code](#writing-code) - [Getting code-completion for rustc internals to work](#getting-code-completion-for-rustc-internals-to-work) - [How Clippy works](#how-clippy-works) - - [Fixing build failures caused by Rust](#fixing-build-failures-caused-by-rust) + - [Syncing changes between Clippy and `rust-lang/rust`](#syncing-changes-between-clippy-and-rust-langrust) - [Patching git-subtree to work with big repos](#patching-git-subtree-to-work-with-big-repos) - - [Performing the sync](#performing-the-sync) - - [Syncing back changes in Clippy to [`rust-lang/rust`]](#syncing-back-changes-in-clippy-to-rust-langrust) + - [Performing the sync from `rust-lang/rust` to Clippy](#performing-the-sync-from-rust-langrust-to-clippy) + - [Performing the sync from Clippy to `rust-lang/rust`](#performing-the-sync-from-clippy-to-rust-langrust) - [Defining remotes](#defining-remotes) - [Issue and PR triage](#issue-and-pr-triage) - [Bors and Homu](#bors-and-homu) @@ -49,7 +49,7 @@ first read the [Basics docs](doc/basics.md).** All issues on Clippy are mentored, if you want help with a bug just ask @Manishearth, @flip1995, @phansch or @yaahc. -Some issues are easier than others. The [`good first issue`] label can be used to find the easy issues. +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`]. @@ -68,16 +68,16 @@ To figure out how this syntax structure is encoded in the AST, it is recommended 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`] issues are generally pretty easy too, though it's recommended you work on an [`good first issue`] +[`E-medium`] issues are generally pretty easy too, though it's recommended you work on an [`good-first-issue`] first. Sometimes they are only somewhat involved code wise, but not difficult per-se. -Note that [`E-medium`] issues may require some knowledge of Clippy internals or some -debugging to find the actual problem behind the issue. +Note that [`E-medium`] issues may require some knowledge of Clippy internals or some +debugging to find the actual problem behind the issue. [`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 +[`good-first-issue`]: https://github.com/rust-lang/rust-clippy/labels/good-first-issue [`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 @@ -111,7 +111,7 @@ To work around this, you need to have a copy of the [rustc-repo][rustc_repo] ava `git clone https://github.com/rust-lang/rust/`. Then you can run a `cargo dev` command to automatically make Clippy use the rustc-repo via path-dependencies which rust-analyzer will be able to understand. -Run `cargo dev ra-setup --repo-path ` where `` is an absolute path to the rustc repo +Run `cargo dev ra_setup --repo-path ` where `` is an absolute path to the rustc repo you just cloned. The command will add path-dependencies pointing towards rustc-crates inside the rustc repo to Clippys `Cargo.toml`s and should allow rust-analyzer to understand most of the types that Clippy uses. @@ -182,18 +182,26 @@ That's why the `else_if_without_else` example uses the `register_early_pass` fun [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 +## Syncing changes between Clippy and [`rust-lang/rust`] -Clippy currently gets built with `rustc` of the `rust-lang/rust` `master` -branch. Most of the times we have to adapt to the changes and only very rarely -there's an actual bug in Rust. +Clippy currently gets built with a pinned nightly version. -If you decide to make Clippy work again with a Rust commit that breaks it, you -have to sync the `rust-lang/rust-clippy` repository with the `subtree` copy of -Clippy in the `rust-lang/rust` repository. +In the `rust-lang/rust` repository, where rustc resides, there's a copy of Clippy +that compiler hackers modify from time to time to adapt to changes in the unstable +API of the compiler. -For general information about `subtree`s in the Rust repository see [Rust's -`CONTRIBUTING.md`][subtree]. +We need to sync these changes back to this repository periodically, and the changes +made to this repository in the meantime also need to be synced to the `rust-lang/rust` repository. + +To avoid flooding the `rust-lang/rust` PR queue, this two-way sync process is done +in a bi-weekly basis if there's no urgent changes. This is done starting on the day of +the Rust stable release and then every other week. That way we guarantee that we keep +this repo up to date with the latest compiler API, and every feature in Clippy is available +for 2 weeks in nightly, before it can get to beta. For reference, the first sync +following this cadence was performed the 2020-08-27. + +This process is described in detail in the following sections. For general information +about `subtree`s in the Rust repository see [Rust's `CONTRIBUTING.md`][subtree]. ### Patching git-subtree to work with big repos @@ -222,13 +230,14 @@ This shell has a hardcoded recursion limit set to 1000. In order to make this pr you need to force the script to run `bash` instead. You can do this by editing the first line of the `git-subtree` script and changing `sh` to `bash`. -### Performing the sync +### Performing the sync from [`rust-lang/rust`] to Clippy Here is a TL;DR version of the sync process (all of the following commands have to be run inside the `rust` directory): -1. Clone the [`rust-lang/rust`] repository -2. Sync the changes to the rust-copy of Clippy to your Clippy fork: +1. Clone the [`rust-lang/rust`] repository or make sure it is up to date. +2. Checkout the commit from the latest available nightly. You can get it using `rustup check`. +3. Sync the changes to the rust-copy of Clippy to your Clippy fork: ```bash # Make sure to change `your-github-name` to your github name in the following command git subtree push -P src/tools/clippy git@github.com:your-github-name/rust-clippy sync-from-rust @@ -246,17 +255,11 @@ to be run inside the `rust` directory): git checkout sync-from-rust git merge upstream/master ``` -3. Open a PR to `rust-lang/rust-clippy` and wait for it to get merged (to +4. Open a PR to `rust-lang/rust-clippy` and wait for it to get merged (to accelerate the process ping the `@rust-lang/clippy` team in your PR and/or ~~annoy~~ ask them in the [Zulip] stream.) - -### Syncing back changes in Clippy to [`rust-lang/rust`] -To avoid flooding the [`rust-lang/rust`] PR queue, changes in Clippy's repo are synced back -in a bi-weekly basis if there's no urgent changes. This is done starting on the day of -the Rust stable release and then every other week. That way we guarantee that -every feature in Clippy is available for 2 weeks in nightly, before it can get to beta. -For reference, the first sync following this cadence was performed the 2020-08-27. +### Performing the sync from Clippy to [`rust-lang/rust`] All of the following commands have to be run inside the `rust` directory. diff --git a/src/tools/clippy/Cargo.toml b/src/tools/clippy/Cargo.toml index a765390c6032d..7f9d22e594b9c 100644 --- a/src/tools/clippy/Cargo.toml +++ b/src/tools/clippy/Cargo.toml @@ -20,7 +20,6 @@ publish = false [[bin]] name = "cargo-clippy" -test = false path = "src/main.rs" [[bin]] diff --git a/src/tools/clippy/README.md b/src/tools/clippy/README.md index fddf0614a0b85..dc931963726b2 100644 --- a/src/tools/clippy/README.md +++ b/src/tools/clippy/README.md @@ -82,6 +82,22 @@ Note that this is still experimental and only supported on the nightly channel: cargo clippy --fix -Z unstable-options ``` +#### Workspaces + +All the usual workspace options should work with Clippy. For example the following command +will run Clippy on the `example` crate: + +```terminal +cargo clippy -p example +``` + +As with `cargo check`, this includes dependencies that are members of the workspace, like path dependencies. +If you want to run Clippy **only** on the given crate, use the `--no-deps` option like this: + +```terminal +cargo clippy -p example -- --no-deps +``` + ### Running Clippy from the command line without installing it To have cargo compile your crate with Clippy without Clippy installation @@ -192,7 +208,6 @@ the lint(s) you are interested in: ```terminal cargo clippy -- -A clippy::all -W clippy::useless_format -W clippy::... ``` -Note that if you've run clippy before, this may only take effect after you've modified a file or ran `cargo clean`. ### Specifying the minimum supported Rust version diff --git a/src/tools/clippy/clippy_dev/src/bless.rs b/src/tools/clippy/clippy_dev/src/bless.rs new file mode 100644 index 0000000000000..645098e4cfcd2 --- /dev/null +++ b/src/tools/clippy/clippy_dev/src/bless.rs @@ -0,0 +1,74 @@ +//! `bless` updates the reference files in the repo with changed output files +//! from the last test run. + +use std::env; +use std::ffi::OsStr; +use std::fs; +use std::lazy::SyncLazy; +use std::path::PathBuf; +use walkdir::WalkDir; + +use crate::clippy_project_root; + +// NOTE: this is duplicated with tests/cargo/mod.rs What to do? +pub static CARGO_TARGET_DIR: SyncLazy = SyncLazy::new(|| match env::var_os("CARGO_TARGET_DIR") { + Some(v) => v.into(), + None => env::current_dir().unwrap().join("target"), +}); + +pub fn bless() { + let test_dirs = [ + clippy_project_root().join("tests").join("ui"), + clippy_project_root().join("tests").join("ui-toml"), + clippy_project_root().join("tests").join("ui-cargo"), + ]; + for test_dir in &test_dirs { + WalkDir::new(test_dir) + .into_iter() + .filter_map(Result::ok) + .filter(|f| f.path().extension() == Some(OsStr::new("rs"))) + .for_each(|f| { + update_reference_file(f.path().with_extension("stdout")); + update_reference_file(f.path().with_extension("stderr")); + update_reference_file(f.path().with_extension("fixed")); + }); + } +} + +fn update_reference_file(reference_file_path: PathBuf) { + let test_output_path = build_dir().join(PathBuf::from(reference_file_path.file_name().unwrap())); + let relative_reference_file_path = reference_file_path.strip_prefix(clippy_project_root()).unwrap(); + + // If compiletest did not write any changes during the test run, + // we don't have to update anything + if !test_output_path.exists() { + return; + } + + let test_output_file = fs::read(&test_output_path).expect("Unable to read test output file"); + let reference_file = fs::read(&reference_file_path).unwrap_or_default(); + + if test_output_file != reference_file { + // If a test run caused an output file to change, update the reference file + println!("updating {}", &relative_reference_file_path.display()); + fs::copy(test_output_path, &reference_file_path).expect("Could not update reference file"); + + // We need to re-read the file now because it was potentially updated from copying + let reference_file = fs::read(&reference_file_path).unwrap_or_default(); + + if reference_file.is_empty() { + // If we copied over an empty output file, we remove the now empty reference file + println!("removing {}", &relative_reference_file_path.display()); + fs::remove_file(reference_file_path).expect("Could not remove reference file"); + } + } +} + +fn build_dir() -> PathBuf { + let profile = env::var("PROFILE").unwrap_or_else(|_| "debug".to_string()); + let mut path = PathBuf::new(); + path.push(CARGO_TARGET_DIR.clone()); + path.push(profile); + path.push("test_build_base"); + path +} diff --git a/src/tools/clippy/clippy_dev/src/fmt.rs b/src/tools/clippy/clippy_dev/src/fmt.rs index 6ae3f58c1f2ad..6b528d219df27 100644 --- a/src/tools/clippy/clippy_dev/src/fmt.rs +++ b/src/tools/clippy/clippy_dev/src/fmt.rs @@ -1,9 +1,9 @@ use crate::clippy_project_root; use shell_escape::escape; use std::ffi::OsStr; -use std::io; use std::path::Path; use std::process::{self, Command}; +use std::{fs, io}; use walkdir::WalkDir; #[derive(Debug)] @@ -12,6 +12,7 @@ pub enum CliError { IoError(io::Error), RustfmtNotInstalled, WalkDirError(walkdir::Error), + RaSetupActive, } impl From for CliError { @@ -31,12 +32,23 @@ struct FmtContext { verbose: bool, } +// the "main" function of cargo dev fmt pub fn run(check: bool, verbose: bool) { fn try_run(context: &FmtContext) -> Result { let mut success = true; let project_root = clippy_project_root(); + // if we added a local rustc repo as path dependency to clippy for rust analyzer, we do NOT want to + // format because rustfmt would also format the entire rustc repo as it is a local + // dependency + if fs::read_to_string(project_root.join("Cargo.toml")) + .expect("Failed to read clippy Cargo.toml") + .contains(&"[target.'cfg(NOT_A_PLATFORM)'.dependencies]") + { + return Err(CliError::RaSetupActive); + } + rustfmt_test(context)?; success &= cargo_fmt(context, project_root.as_path())?; @@ -75,6 +87,13 @@ pub fn run(check: bool, verbose: bool) { CliError::WalkDirError(err) => { eprintln!("error: {}", err); }, + CliError::RaSetupActive => { + eprintln!( + "error: a local rustc repo is enabled as path dependency via `cargo dev ra_setup`. +Not formatting because that would format the local repo as well! +Please revert the changes to Cargo.tomls first." + ); + }, } } diff --git a/src/tools/clippy/clippy_dev/src/lib.rs b/src/tools/clippy/clippy_dev/src/lib.rs index f51c45e9eb593..17cc08ee10fea 100644 --- a/src/tools/clippy/clippy_dev/src/lib.rs +++ b/src/tools/clippy/clippy_dev/src/lib.rs @@ -10,6 +10,7 @@ use std::lazy::SyncLazy; use std::path::{Path, PathBuf}; use walkdir::WalkDir; +pub mod bless; pub mod fmt; pub mod new_lint; pub mod ra_setup; diff --git a/src/tools/clippy/clippy_dev/src/main.rs b/src/tools/clippy/clippy_dev/src/main.rs index 7a8cbd5251da9..4fdae38e3ab7a 100644 --- a/src/tools/clippy/clippy_dev/src/main.rs +++ b/src/tools/clippy/clippy_dev/src/main.rs @@ -1,10 +1,53 @@ #![cfg_attr(feature = "deny-warnings", deny(warnings))] -use clap::{App, Arg, SubCommand}; -use clippy_dev::{fmt, new_lint, ra_setup, serve, stderr_length_check, update_lints}; +use clap::{App, Arg, ArgMatches, SubCommand}; +use clippy_dev::{bless, fmt, new_lint, ra_setup, serve, stderr_length_check, update_lints}; fn main() { - let matches = App::new("Clippy developer tooling") + let matches = get_clap_config(); + + match matches.subcommand() { + ("bless", Some(_)) => { + bless::bless(); + }, + ("fmt", Some(matches)) => { + fmt::run(matches.is_present("check"), matches.is_present("verbose")); + }, + ("update_lints", Some(matches)) => { + if matches.is_present("print-only") { + update_lints::print_lints(); + } else if matches.is_present("check") { + update_lints::run(update_lints::UpdateMode::Check); + } else { + update_lints::run(update_lints::UpdateMode::Change); + } + }, + ("new_lint", Some(matches)) => { + match new_lint::create( + matches.value_of("pass"), + matches.value_of("name"), + matches.value_of("category"), + ) { + Ok(_) => update_lints::run(update_lints::UpdateMode::Change), + Err(e) => eprintln!("Unable to create lint: {}", e), + } + }, + ("limit_stderr_length", _) => { + stderr_length_check::check(); + }, + ("ra_setup", Some(matches)) => ra_setup::run(matches.value_of("rustc-repo-path")), + ("serve", Some(matches)) => { + let port = matches.value_of("port").unwrap().parse().unwrap(); + let lint = matches.value_of("lint"); + serve::run(port, lint); + }, + _ => {}, + } +} + +fn get_clap_config<'a>() -> ArgMatches<'a> { + App::new("Clippy developer tooling") + .subcommand(SubCommand::with_name("bless").about("bless the test output changes")) .subcommand( SubCommand::with_name("fmt") .about("Run rustfmt on all projects and tests") @@ -25,16 +68,16 @@ fn main() { .about("Updates lint registration and information from the source code") .long_about( "Makes sure that:\n \ - * the lint count in README.md is correct\n \ - * the changelog contains markdown link references at the bottom\n \ - * all lint groups include the correct lints\n \ - * lint modules in `clippy_lints/*` are visible in `src/lib.rs` via `pub mod`\n \ - * all lints are registered in the lint store", + * the lint count in README.md is correct\n \ + * the changelog contains markdown link references at the bottom\n \ + * all lint groups include the correct lints\n \ + * lint modules in `clippy_lints/*` are visible in `src/lifb.rs` via `pub mod`\n \ + * all lints are registered in the lint store", ) .arg(Arg::with_name("print-only").long("print-only").help( "Print a table of lints to STDOUT. \ - This does not include deprecated and internal lints. \ - (Does not modify any files)", + This does not include deprecated and internal lints. \ + (Does not modify any files)", )) .arg( Arg::with_name("check") @@ -88,7 +131,7 @@ fn main() { .about("Ensures that stderr files do not grow longer than a certain amount of lines."), ) .subcommand( - SubCommand::with_name("ra-setup") + SubCommand::with_name("ra_setup") .about("Alter dependencies so rust-analyzer can find rustc internals") .arg( Arg::with_name("rustc-repo-path") @@ -113,40 +156,5 @@ fn main() { ) .arg(Arg::with_name("lint").help("Which lint's page to load initially (optional)")), ) - .get_matches(); - - match matches.subcommand() { - ("fmt", Some(matches)) => { - fmt::run(matches.is_present("check"), matches.is_present("verbose")); - }, - ("update_lints", Some(matches)) => { - if matches.is_present("print-only") { - update_lints::print_lints(); - } else if matches.is_present("check") { - update_lints::run(update_lints::UpdateMode::Check); - } else { - update_lints::run(update_lints::UpdateMode::Change); - } - }, - ("new_lint", Some(matches)) => { - match new_lint::create( - matches.value_of("pass"), - matches.value_of("name"), - matches.value_of("category"), - ) { - Ok(_) => update_lints::run(update_lints::UpdateMode::Change), - Err(e) => eprintln!("Unable to create lint: {}", e), - } - }, - ("limit_stderr_length", _) => { - stderr_length_check::check(); - }, - ("ra-setup", Some(matches)) => ra_setup::run(matches.value_of("rustc-repo-path")), - ("serve", Some(matches)) => { - let port = matches.value_of("port").unwrap().parse().unwrap(); - let lint = matches.value_of("lint"); - serve::run(port, lint); - }, - _ => {}, - } + .get_matches() } diff --git a/src/tools/clippy/clippy_dev/src/ra_setup.rs b/src/tools/clippy/clippy_dev/src/ra_setup.rs index 9d9e836cc08a2..40bf4a9505a88 100644 --- a/src/tools/clippy/clippy_dev/src/ra_setup.rs +++ b/src/tools/clippy/clippy_dev/src/ra_setup.rs @@ -52,7 +52,7 @@ fn inject_deps_into_manifest( // do not inject deps if we have aleady done so if cargo_toml.contains("[target.'cfg(NOT_A_PLATFORM)'.dependencies]") { eprintln!( - "cargo dev ra-setup: warning: deps already found inside {}, doing nothing.", + "cargo dev ra_setup: warning: deps already found inside {}, doing nothing.", manifest_path ); return Ok(()); diff --git a/src/tools/clippy/clippy_lints/src/await_holding_invalid.rs b/src/tools/clippy/clippy_lints/src/await_holding_invalid.rs index cfade9cbc469a..f136aa4572a87 100644 --- a/src/tools/clippy/clippy_lints/src/await_holding_invalid.rs +++ b/src/tools/clippy/clippy_lints/src/await_holding_invalid.rs @@ -99,7 +99,11 @@ impl LateLintPass<'_> for AwaitHolding { }; let def_id = cx.tcx.hir().body_owner_def_id(body_id); let typeck_results = cx.tcx.typeck(def_id); - check_interior_types(cx, &typeck_results.generator_interior_types.as_ref().skip_binder(), body.value.span); + check_interior_types( + cx, + &typeck_results.generator_interior_types.as_ref().skip_binder(), + body.value.span, + ); } } } diff --git a/src/tools/clippy/clippy_lints/src/checked_conversions.rs b/src/tools/clippy/clippy_lints/src/checked_conversions.rs index 28c1a54d2c5a6..54bc69e058bc7 100644 --- a/src/tools/clippy/clippy_lints/src/checked_conversions.rs +++ b/src/tools/clippy/clippy_lints/src/checked_conversions.rs @@ -6,9 +6,12 @@ use rustc_errors::Applicability; use rustc_hir::{BinOp, BinOpKind, Expr, ExprKind, QPath, TyKind}; use rustc_lint::{LateContext, LateLintPass, LintContext}; use rustc_middle::lint::in_external_macro; -use rustc_session::{declare_lint_pass, declare_tool_lint}; +use rustc_semver::RustcVersion; +use rustc_session::{declare_tool_lint, impl_lint_pass}; -use crate::utils::{snippet_with_applicability, span_lint_and_sugg, SpanlessEq}; +use crate::utils::{meets_msrv, snippet_with_applicability, span_lint_and_sugg, SpanlessEq}; + +const CHECKED_CONVERSIONS_MSRV: RustcVersion = RustcVersion::new(1, 34, 0); declare_clippy_lint! { /// **What it does:** Checks for explicit bounds checking when casting. @@ -39,10 +42,25 @@ declare_clippy_lint! { "`try_from` could replace manual bounds checking when casting" } -declare_lint_pass!(CheckedConversions => [CHECKED_CONVERSIONS]); +pub struct CheckedConversions { + msrv: Option, +} + +impl CheckedConversions { + #[must_use] + pub fn new(msrv: Option) -> Self { + Self { msrv } + } +} + +impl_lint_pass!(CheckedConversions => [CHECKED_CONVERSIONS]); impl<'tcx> LateLintPass<'tcx> for CheckedConversions { fn check_expr(&mut self, cx: &LateContext<'_>, item: &Expr<'_>) { + if !meets_msrv(self.msrv.as_ref(), &CHECKED_CONVERSIONS_MSRV) { + return; + } + let result = if_chain! { if !in_external_macro(cx.sess(), item.span); if let ExprKind::Binary(op, ref left, ref right) = &item.kind; @@ -74,6 +92,8 @@ impl<'tcx> LateLintPass<'tcx> for CheckedConversions { } } } + + extract_msrv_attr!(LateContext); } /// Searches for a single check from unsigned to _ is done diff --git a/src/tools/clippy/clippy_lints/src/create_dir.rs b/src/tools/clippy/clippy_lints/src/create_dir.rs index 4002fb655a5eb..200b6a565cc43 100644 --- a/src/tools/clippy/clippy_lints/src/create_dir.rs +++ b/src/tools/clippy/clippy_lints/src/create_dir.rs @@ -8,7 +8,7 @@ use rustc_session::{declare_lint_pass, declare_tool_lint}; declare_clippy_lint! { /// **What it does:** Checks usage of `std::fs::create_dir` and suggest using `std::fs::create_dir_all` instead. /// - /// **Why is this bad?** Sometimes `std::fs::crate_dir` is mistakenly chosen over `std::fs::create_dir_all`. + /// **Why is this bad?** Sometimes `std::fs::create_dir` is mistakenly chosen over `std::fs::create_dir_all`. /// /// **Known problems:** None. /// diff --git a/src/tools/clippy/clippy_lints/src/doc.rs b/src/tools/clippy/clippy_lints/src/doc.rs index edecba57e44f0..aba6553279590 100644 --- a/src/tools/clippy/clippy_lints/src/doc.rs +++ b/src/tools/clippy/clippy_lints/src/doc.rs @@ -14,6 +14,7 @@ use rustc_middle::ty; use rustc_parse::maybe_new_parser_from_source_str; use rustc_session::parse::ParseSess; use rustc_session::{declare_tool_lint, impl_lint_pass}; +use rustc_span::edition::Edition; use rustc_span::source_map::{BytePos, FilePathMapping, MultiSpan, SourceMap, Span}; use rustc_span::{sym, FileName, Pos}; use std::io; @@ -377,7 +378,7 @@ fn check_attrs<'a>(cx: &LateContext<'_>, valid_idents: &FxHashSet, attrs check_doc(cx, valid_idents, events, &spans) } -const RUST_CODE: &[&str] = &["rust", "no_run", "should_panic", "compile_fail", "edition2018"]; +const RUST_CODE: &[&str] = &["rust", "no_run", "should_panic", "compile_fail"]; fn check_doc<'a, Events: Iterator, Range)>>( cx: &LateContext<'_>, @@ -400,13 +401,24 @@ fn check_doc<'a, Events: Iterator, Range { 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)); + for item in lang.split(',') { + if item == "ignore" { + is_rust = false; + break; + } + if let Some(stripped) = item.strip_prefix("edition") { + is_rust = true; + edition = stripped.parse::().ok(); + } else if item.is_empty() || RUST_CODE.contains(&item) { + is_rust = true; + } + } } }, End(CodeBlock(_)) => { @@ -436,7 +448,8 @@ fn check_doc<'a, Events: Iterator, Range, Range, text: &str, span: Span) { - fn has_needless_main(code: &str) -> bool { - let filename = FileName::anon_source_code(code); - - let sm = Lrc::new(SourceMap::new(FilePathMapping::empty())); - let emitter = EmitterWriter::new(box io::sink(), None, false, false, false, None, false); - let handler = Handler::with_emitter(false, None, box emitter); - let sess = ParseSess::with_span_handler(handler, sm); - - let mut parser = match maybe_new_parser_from_source_str(&sess, filename, code.into()) { - Ok(p) => p, - Err(errs) => { - for mut err in errs { - err.cancel(); - } - return false; - }, - }; - - let mut relevant_main_found = false; - loop { - match parser.parse_item() { - Ok(Some(item)) => match &item.kind { - // Tests with one of these items are ignored - ItemKind::Static(..) - | ItemKind::Const(..) - | ItemKind::ExternCrate(..) - | ItemKind::ForeignMod(..) => return false, - // We found a main function ... - ItemKind::Fn(_, sig, _, Some(block)) if item.ident.name == sym::main => { - let is_async = matches!(sig.header.asyncness, Async::Yes{..}); - let returns_nothing = match &sig.decl.output { - FnRetTy::Default(..) => true, - FnRetTy::Ty(ty) if ty.kind.is_unit() => true, - _ => false, - }; - - if returns_nothing && !is_async && !block.stmts.is_empty() { - // This main function should be linted, but only if there are no other functions - relevant_main_found = true; - } else { - // This main function should not be linted, we're done - return false; +fn check_code(cx: &LateContext<'_>, text: &str, edition: Edition, span: Span) { + fn has_needless_main(code: &str, edition: Edition) -> bool { + rustc_driver::catch_fatal_errors(|| { + rustc_span::with_session_globals(edition, || { + let filename = FileName::anon_source_code(code); + + let sm = Lrc::new(SourceMap::new(FilePathMapping::empty())); + let emitter = EmitterWriter::new(box io::sink(), None, false, false, false, None, false); + let handler = Handler::with_emitter(false, None, box emitter); + let sess = ParseSess::with_span_handler(handler, sm); + + let mut parser = match maybe_new_parser_from_source_str(&sess, filename, code.into()) { + Ok(p) => p, + Err(errs) => { + for mut err in errs { + err.cancel(); } + return false; }, - // Another function was found; this case is ignored too - ItemKind::Fn(..) => return false, - _ => {}, - }, - Ok(None) => break, - Err(mut e) => { - e.cancel(); - return false; - }, - } - } + }; + + let mut relevant_main_found = false; + loop { + match parser.parse_item() { + Ok(Some(item)) => match &item.kind { + // Tests with one of these items are ignored + ItemKind::Static(..) + | ItemKind::Const(..) + | ItemKind::ExternCrate(..) + | ItemKind::ForeignMod(..) => return false, + // We found a main function ... + ItemKind::Fn(_, sig, _, Some(block)) if item.ident.name == sym::main => { + let is_async = matches!(sig.header.asyncness, Async::Yes { .. }); + let returns_nothing = match &sig.decl.output { + FnRetTy::Default(..) => true, + FnRetTy::Ty(ty) if ty.kind.is_unit() => true, + _ => false, + }; + + if returns_nothing && !is_async && !block.stmts.is_empty() { + // This main function should be linted, but only if there are no other functions + relevant_main_found = true; + } else { + // This main function should not be linted, we're done + return false; + } + }, + // Another function was found; this case is ignored too + ItemKind::Fn(..) => return false, + _ => {}, + }, + Ok(None) => break, + Err(mut e) => { + e.cancel(); + return false; + }, + } + } - relevant_main_found + relevant_main_found + }) + }) + .ok() + .unwrap_or_default() } - if has_needless_main(text) { + if has_needless_main(text, edition) { span_lint(cx, NEEDLESS_DOCTEST_MAIN, span, "needless `fn main` in doctest"); } } diff --git a/src/tools/clippy/clippy_lints/src/functions.rs b/src/tools/clippy/clippy_lints/src/functions.rs index 8b58d1f26013a..fd93548b55c6d 100644 --- a/src/tools/clippy/clippy_lints/src/functions.rs +++ b/src/tools/clippy/clippy_lints/src/functions.rs @@ -405,13 +405,10 @@ impl<'tcx> Functions { break; } if in_comment { - match line.find("*/") { - Some(i) => { - line = &line[i + 2..]; - in_comment = false; - continue; - }, - None => break, + if let Some(i) = line.find("*/") { + line = &line[i + 2..]; + in_comment = false; + continue; } } else { let multi_idx = line.find("/*").unwrap_or_else(|| line.len()); @@ -423,8 +420,8 @@ impl<'tcx> Functions { in_comment = true; continue; } - break; } + break; } if code_in_line { line_count += 1; diff --git a/src/tools/clippy/clippy_lints/src/len_zero.rs b/src/tools/clippy/clippy_lints/src/len_zero.rs index 8842901d90b83..6fe5335109045 100644 --- a/src/tools/clippy/clippy_lints/src/len_zero.rs +++ b/src/tools/clippy/clippy_lints/src/len_zero.rs @@ -222,9 +222,8 @@ fn check_impl_items(cx: &LateContext<'_>, item: &Item<'_>, impl_items: &[ImplIte let is_empty = if let Some(is_empty) = impl_items.iter().find(|i| is_named_self(cx, i, "is_empty")) { if cx.access_levels.is_exported(is_empty.id.hir_id) { return; - } else { - "a private" } + "a private" } else { "no corresponding" }; diff --git a/src/tools/clippy/clippy_lints/src/lib.rs b/src/tools/clippy/clippy_lints/src/lib.rs index 2b99ed570b148..02ba422a2f5b8 100644 --- a/src/tools/clippy/clippy_lints/src/lib.rs +++ b/src/tools/clippy/clippy_lints/src/lib.rs @@ -27,6 +27,7 @@ extern crate rustc_ast; extern crate rustc_ast_pretty; extern crate rustc_attr; extern crate rustc_data_structures; +extern crate rustc_driver; extern crate rustc_errors; extern crate rustc_hir; extern crate rustc_hir_pretty; @@ -294,6 +295,7 @@ mod question_mark; mod ranges; mod redundant_clone; mod redundant_closure_call; +mod redundant_else; mod redundant_field_names; mod redundant_pub_crate; mod redundant_static_lifetimes; @@ -344,6 +346,7 @@ mod wildcard_dependencies; mod wildcard_imports; mod write; mod zero_div_zero; +mod zero_sized_map_values; // end lints modules, do not remove this comment, it’s used in `update_lints` pub use crate::utils::conf::Conf; @@ -509,6 +512,8 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: #[cfg(feature = "internal-lints")] &utils::internal_lints::DEFAULT_LINT, #[cfg(feature = "internal-lints")] + &utils::internal_lints::INTERNING_DEFINED_SYMBOL, + #[cfg(feature = "internal-lints")] &utils::internal_lints::INVALID_PATHS, #[cfg(feature = "internal-lints")] &utils::internal_lints::LINT_WITHOUT_LINT_PASS, @@ -831,6 +836,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: &ranges::REVERSED_EMPTY_RANGES, &redundant_clone::REDUNDANT_CLONE, &redundant_closure_call::REDUNDANT_CLOSURE_CALL, + &redundant_else::REDUNDANT_ELSE, &redundant_field_names::REDUNDANT_FIELD_NAMES, &redundant_pub_crate::REDUNDANT_PUB_CRATE, &redundant_static_lifetimes::REDUNDANT_STATIC_LIFETIMES, @@ -934,6 +940,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: &wildcard_imports::WILDCARD_IMPORTS, &write::PRINTLN_EMPTY_STRING, &write::PRINT_LITERAL, + &write::PRINT_STDERR, &write::PRINT_STDOUT, &write::PRINT_WITH_NEWLINE, &write::USE_DEBUG, @@ -941,6 +948,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: &write::WRITE_LITERAL, &write::WRITE_WITH_NEWLINE, &zero_div_zero::ZERO_DIVIDED_BY_ZERO, + &zero_sized_map_values::ZERO_SIZED_MAP_VALUES, ]); // end register lints, do not remove this comment, it’s used in `update_lints` @@ -953,6 +961,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: store.register_late_pass(|| box utils::internal_lints::CollapsibleCalls); store.register_late_pass(|| box utils::internal_lints::CompilerLintFunctions::new()); store.register_late_pass(|| box utils::internal_lints::InvalidPaths); + store.register_late_pass(|| box utils::internal_lints::InterningDefinedSymbol::default()); store.register_late_pass(|| box utils::internal_lints::LintWithoutLintPass::default()); store.register_late_pass(|| box utils::internal_lints::MatchTypeOnDiagItem); store.register_late_pass(|| box utils::internal_lints::OuterExpnDataPass); @@ -1000,6 +1009,14 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: store.register_late_pass(move || box matches::Matches::new(msrv)); store.register_early_pass(move || box manual_non_exhaustive::ManualNonExhaustive::new(msrv)); store.register_late_pass(move || box manual_strip::ManualStrip::new(msrv)); + store.register_early_pass(move || box redundant_static_lifetimes::RedundantStaticLifetimes::new(msrv)); + store.register_early_pass(move || box redundant_field_names::RedundantFieldNames::new(msrv)); + store.register_late_pass(move || box checked_conversions::CheckedConversions::new(msrv)); + store.register_late_pass(move || box mem_replace::MemReplace::new(msrv)); + store.register_late_pass(move || box ranges::Ranges::new(msrv)); + store.register_late_pass(move || box use_self::UseSelf::new(msrv)); + store.register_late_pass(move || box missing_const_for_fn::MissingConstForFn::new(msrv)); + store.register_late_pass(|| box size_of_in_element_count::SizeOfInElementCount); store.register_late_pass(|| box map_clone::MapClone); store.register_late_pass(|| box map_err_ignore::MapErrIgnore); @@ -1010,7 +1027,6 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: store.register_late_pass(|| box main_recursion::MainRecursion::default()); store.register_late_pass(|| box lifetimes::Lifetimes); store.register_late_pass(|| box entry::HashMapPass); - store.register_late_pass(|| box ranges::Ranges); store.register_late_pass(|| box types::Casts); let type_complexity_threshold = conf.type_complexity_threshold; store.register_late_pass(move || box types::TypeComplexity::new(type_complexity_threshold)); @@ -1055,7 +1071,6 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: store.register_late_pass(|| box neg_multiply::NegMultiply); store.register_late_pass(|| box mem_discriminant::MemDiscriminant); store.register_late_pass(|| box mem_forget::MemForget); - store.register_late_pass(|| box mem_replace::MemReplace); store.register_late_pass(|| box arithmetic::Arithmetic::default()); store.register_late_pass(|| box assign_ops::AssignOps); store.register_late_pass(|| box let_if_seq::LetIfSeq); @@ -1077,7 +1092,6 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: store.register_late_pass(move || box pass_by_ref_or_value); store.register_late_pass(|| box ref_option_ref::RefOptionRef); store.register_late_pass(|| box try_err::TryErr); - store.register_late_pass(|| box use_self::UseSelf); store.register_late_pass(|| box bytecount::ByteCount); store.register_late_pass(|| box infinite_iter::InfiniteIter); store.register_late_pass(|| box inline_fn_without_body::InlineFnWithoutBody); @@ -1103,10 +1117,8 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: store.register_late_pass(|| box unnecessary_wraps::UnnecessaryWraps); store.register_late_pass(|| box types::RefToMut); store.register_late_pass(|| box assertions_on_constants::AssertionsOnConstants); - store.register_late_pass(|| box missing_const_for_fn::MissingConstForFn); store.register_late_pass(|| box transmuting_null::TransmutingNull); store.register_late_pass(|| box path_buf_push_overwrite::PathBufPushOverwrite); - store.register_late_pass(|| box checked_conversions::CheckedConversions); store.register_late_pass(|| box integer_division::IntegerDivision); store.register_late_pass(|| box inherent_to_string::InherentToString); let max_trait_bounds = conf.max_trait_bounds; @@ -1132,9 +1144,9 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: store.register_early_pass(|| box items_after_statements::ItemsAfterStatements); store.register_early_pass(|| box precedence::Precedence); store.register_early_pass(|| box needless_continue::NeedlessContinue); + store.register_early_pass(|| box redundant_else::RedundantElse); store.register_late_pass(|| box create_dir::CreateDir); store.register_early_pass(|| box needless_arbitrary_self_type::NeedlessArbitrarySelfType); - store.register_early_pass(|| box redundant_static_lifetimes::RedundantStaticLifetimes); store.register_late_pass(|| box cargo_common_metadata::CargoCommonMetadata); store.register_late_pass(|| box multiple_crate_versions::MultipleCrateVersions); store.register_late_pass(|| box wildcard_dependencies::WildcardDependencies); @@ -1174,7 +1186,6 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: store.register_late_pass(|| box mut_mutex_lock::MutMutexLock); store.register_late_pass(|| box match_on_vec_items::MatchOnVecItems); store.register_late_pass(|| box manual_async_fn::ManualAsyncFn); - store.register_early_pass(|| box redundant_field_names::RedundantFieldNames); store.register_late_pass(|| box vec_resize_to_zero::VecResizeToZero); store.register_late_pass(|| box panic_in_result_fn::PanicInResultFn); let single_char_binding_names_threshold = conf.single_char_binding_names_threshold; @@ -1200,6 +1211,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: store.register_late_pass(|| box undropped_manually_drops::UndroppedManuallyDrops); store.register_late_pass(|| box strings::StrToString); store.register_late_pass(|| box strings::StringToString); + store.register_late_pass(|| box zero_sized_map_values::ZeroSizedMapValues); store.register_group(true, "clippy::restriction", Some("clippy_restriction"), vec![ LintId::of(&arithmetic::FLOAT_ARITHMETIC), @@ -1247,6 +1259,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: LintId::of(&types::RC_BUFFER), LintId::of(&unwrap_in_result::UNWRAP_IN_RESULT), LintId::of(&verbose_file_reads::VERBOSE_FILE_READS), + LintId::of(&write::PRINT_STDERR), LintId::of(&write::PRINT_STDOUT), LintId::of(&write::USE_DEBUG), ]); @@ -1308,6 +1321,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: LintId::of(&pass_by_ref_or_value::TRIVIALLY_COPY_PASS_BY_REF), LintId::of(&ranges::RANGE_MINUS_ONE), LintId::of(&ranges::RANGE_PLUS_ONE), + LintId::of(&redundant_else::REDUNDANT_ELSE), LintId::of(&ref_option_ref::REF_OPTION_REF), LintId::of(&shadow::SHADOW_UNRELATED), LintId::of(&strings::STRING_ADD_ASSIGN), @@ -1330,6 +1344,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: LintId::of(&unused_self::UNUSED_SELF), LintId::of(&wildcard_imports::ENUM_GLOB_USE), LintId::of(&wildcard_imports::WILDCARD_IMPORTS), + LintId::of(&zero_sized_map_values::ZERO_SIZED_MAP_VALUES), ]); #[cfg(feature = "internal-lints")] @@ -1338,6 +1353,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: LintId::of(&utils::internal_lints::COLLAPSIBLE_SPAN_LINT_CALLS), LintId::of(&utils::internal_lints::COMPILER_LINT_FUNCTIONS), LintId::of(&utils::internal_lints::DEFAULT_LINT), + LintId::of(&utils::internal_lints::INTERNING_DEFINED_SYMBOL), LintId::of(&utils::internal_lints::INVALID_PATHS), LintId::of(&utils::internal_lints::LINT_WITHOUT_LINT_PASS), LintId::of(&utils::internal_lints::MATCH_TYPE_ON_DIAGNOSTIC_ITEM), diff --git a/src/tools/clippy/clippy_lints/src/manual_ok_or.rs b/src/tools/clippy/clippy_lints/src/manual_ok_or.rs index c99d2e35b94a6..b97d97ea1a5ef 100644 --- a/src/tools/clippy/clippy_lints/src/manual_ok_or.rs +++ b/src/tools/clippy/clippy_lints/src/manual_ok_or.rs @@ -8,6 +8,7 @@ use rustc_lint::LintContext; use rustc_lint::{LateContext, LateLintPass}; use rustc_middle::lint::in_external_macro; use rustc_session::{declare_lint_pass, declare_tool_lint}; +use rustc_span::symbol::sym; declare_clippy_lint! { /// **What it does:** @@ -51,7 +52,7 @@ impl LateLintPass<'_> for ManualOkOr { if args.len() == 3; let method_receiver = &args[0]; let ty = cx.typeck_results().expr_ty(method_receiver); - if is_type_diagnostic_item(cx, ty, sym!(option_type)); + if is_type_diagnostic_item(cx, ty, sym::option_type); let or_expr = &args[1]; if is_ok_wrapping(cx, &args[2]); if let ExprKind::Call(Expr { kind: ExprKind::Path(err_path), .. }, &[ref err_arg]) = or_expr.kind; diff --git a/src/tools/clippy/clippy_lints/src/matches.rs b/src/tools/clippy/clippy_lints/src/matches.rs index 274d20cfa8005..04b35835c6b8e 100644 --- a/src/tools/clippy/clippy_lints/src/matches.rs +++ b/src/tools/clippy/clippy_lints/src/matches.rs @@ -4,8 +4,8 @@ use crate::utils::usage::is_unused; use crate::utils::{ expr_block, get_arg_name, get_parent_expr, in_macro, indent_of, is_allowed, is_expn_of, is_refutable, is_type_diagnostic_item, is_wild, match_qpath, match_type, match_var, meets_msrv, multispan_sugg, remove_blocks, - snippet, snippet_block, snippet_with_applicability, span_lint_and_help, span_lint_and_note, span_lint_and_sugg, - span_lint_and_then, + snippet, snippet_block, snippet_opt, snippet_with_applicability, span_lint_and_help, span_lint_and_note, + span_lint_and_sugg, span_lint_and_then, }; use crate::utils::{paths, search_same, SpanlessEq, SpanlessHash}; use if_chain::if_chain; @@ -689,10 +689,9 @@ fn check_single_match(cx: &LateContext<'_>, ex: &Expr<'_>, arms: &[Arm<'_>], exp if stmts.len() == 1 && block_expr.is_none() || stmts.is_empty() && block_expr.is_some() { // single statement/expr "else" block, don't lint return; - } else { - // block with 2+ statements or 1 expr and 1+ statement - Some(els) } + // block with 2+ statements or 1 expr and 1+ statement + Some(els) } else { // not a block, don't lint return; @@ -1238,6 +1237,24 @@ fn check_match_single_binding<'a>(cx: &LateContext<'a>, ex: &Expr<'a>, arms: &[A if in_macro(expr.span) || arms.len() != 1 || is_refutable(cx, arms[0].pat) { return; } + + // HACK: + // This is a hack to deal with arms that are excluded by macros like `#[cfg]`. It is only used here + // to prevent false positives as there is currently no better way to detect if code was excluded by + // a macro. See PR #6435 + if_chain! { + if let Some(match_snippet) = snippet_opt(cx, expr.span); + if let Some(arm_snippet) = snippet_opt(cx, arms[0].span); + if let Some(ex_snippet) = snippet_opt(cx, ex.span); + let rest_snippet = match_snippet.replace(&arm_snippet, "").replace(&ex_snippet, ""); + if rest_snippet.contains("=>"); + then { + // The code it self contains another thick arrow "=>" + // -> Either another arm or a comment + return; + } + } + let matched_vars = ex.span; let bind_names = arms[0].pat.span; let match_body = remove_blocks(&arms[0].body); diff --git a/src/tools/clippy/clippy_lints/src/mem_replace.rs b/src/tools/clippy/clippy_lints/src/mem_replace.rs index bb0acecc5a92d..19087b0207714 100644 --- a/src/tools/clippy/clippy_lints/src/mem_replace.rs +++ b/src/tools/clippy/clippy_lints/src/mem_replace.rs @@ -1,13 +1,14 @@ use crate::utils::{ - in_macro, match_def_path, match_qpath, paths, snippet, snippet_with_applicability, span_lint_and_help, + in_macro, match_def_path, match_qpath, meets_msrv, paths, snippet, snippet_with_applicability, span_lint_and_help, span_lint_and_sugg, span_lint_and_then, }; use if_chain::if_chain; use rustc_errors::Applicability; use rustc_hir::{BorrowKind, Expr, ExprKind, Mutability, QPath}; -use rustc_lint::{LateContext, LateLintPass}; +use rustc_lint::{LateContext, LateLintPass, LintContext}; use rustc_middle::lint::in_external_macro; -use rustc_session::{declare_lint_pass, declare_tool_lint}; +use rustc_semver::RustcVersion; +use rustc_session::{declare_tool_lint, impl_lint_pass}; use rustc_span::source_map::Span; use rustc_span::symbol::sym; @@ -94,7 +95,7 @@ declare_clippy_lint! { "replacing a value of type `T` with `T::default()` instead of using `std::mem::take`" } -declare_lint_pass!(MemReplace => +impl_lint_pass!(MemReplace => [MEM_REPLACE_OPTION_WITH_NONE, MEM_REPLACE_WITH_UNINIT, MEM_REPLACE_WITH_DEFAULT]); fn check_replace_option_with_none(cx: &LateContext<'_>, src: &Expr<'_>, dest: &Expr<'_>, expr_span: Span) { @@ -224,6 +225,19 @@ fn check_replace_with_default(cx: &LateContext<'_>, src: &Expr<'_>, dest: &Expr< } } +const MEM_REPLACE_WITH_DEFAULT_MSRV: RustcVersion = RustcVersion::new(1, 40, 0); + +pub struct MemReplace { + msrv: Option, +} + +impl MemReplace { + #[must_use] + pub fn new(msrv: Option) -> Self { + Self { msrv } + } +} + impl<'tcx> LateLintPass<'tcx> for MemReplace { fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) { if_chain! { @@ -236,8 +250,11 @@ impl<'tcx> LateLintPass<'tcx> for MemReplace { then { check_replace_option_with_none(cx, src, dest, expr.span); check_replace_with_uninit(cx, src, dest, expr.span); - check_replace_with_default(cx, src, dest, expr.span); + if meets_msrv(self.msrv.as_ref(), &MEM_REPLACE_WITH_DEFAULT_MSRV) { + check_replace_with_default(cx, src, dest, expr.span); + } } } } + extract_msrv_attr!(LateContext); } diff --git a/src/tools/clippy/clippy_lints/src/methods/mod.rs b/src/tools/clippy/clippy_lints/src/methods/mod.rs index 8002c27a5e91b..e99fe1b97ff64 100644 --- a/src/tools/clippy/clippy_lints/src/methods/mod.rs +++ b/src/tools/clippy/clippy_lints/src/methods/mod.rs @@ -22,6 +22,7 @@ use rustc_semver::RustcVersion; use rustc_session::{declare_tool_lint, impl_lint_pass}; use rustc_span::source_map::Span; use rustc_span::symbol::{sym, SymbolStr}; +use rustc_typeck::hir_ty_to_ty; use crate::consts::{constant, Constant}; use crate::utils::eager_or_lazy::is_lazyness_candidate; @@ -1487,7 +1488,7 @@ impl<'tcx> LateLintPass<'tcx> for Methods { ["expect", ..] => lint_expect(cx, expr, arg_lists[0]), ["unwrap_or", "map"] => option_map_unwrap_or::lint(cx, expr, arg_lists[1], arg_lists[0], method_spans[1]), ["unwrap_or_else", "map"] => { - if !lint_map_unwrap_or_else(cx, expr, arg_lists[1], arg_lists[0]) { + if !lint_map_unwrap_or_else(cx, expr, arg_lists[1], arg_lists[0], self.msrv.as_ref()) { unnecessary_lazy_eval::lint(cx, expr, arg_lists[0], "unwrap_or"); } }, @@ -1509,7 +1510,7 @@ impl<'tcx> LateLintPass<'tcx> for Methods { ["next", "iter"] => lint_iter_next(cx, expr, arg_lists[1]), ["map", "filter"] => lint_filter_map(cx, expr, arg_lists[1], arg_lists[0]), ["map", "filter_map"] => lint_filter_map_map(cx, expr, arg_lists[1], arg_lists[0]), - ["next", "filter_map"] => lint_filter_map_next(cx, expr, arg_lists[1]), + ["next", "filter_map"] => lint_filter_map_next(cx, expr, arg_lists[1], self.msrv.as_ref()), ["map", "find"] => lint_find_map(cx, expr, arg_lists[1], arg_lists[0]), ["flat_map", "filter"] => lint_filter_flat_map(cx, expr, arg_lists[1], arg_lists[0]), ["flat_map", "filter_map"] => lint_filter_map_flat_map(cx, expr, arg_lists[1], arg_lists[0]), @@ -1568,7 +1569,7 @@ impl<'tcx> LateLintPass<'tcx> for Methods { lint_expect_fun_call(cx, expr, *method_span, &method_call.ident.as_str(), args); let self_ty = cx.typeck_results().expr_ty_adjusted(&args[0]); - if args.len() == 1 && method_call.ident.name == sym!(clone) { + if args.len() == 1 && method_call.ident.name == sym::clone { lint_clone_on_copy(cx, expr, &args[0], self_ty); lint_clone_on_ref_ptr(cx, expr, &args[0]); } @@ -1592,7 +1593,7 @@ impl<'tcx> LateLintPass<'tcx> for Methods { } } }, - ty::Ref(..) if method_call.ident.name == sym!(into_iter) => { + ty::Ref(..) if method_call.ident.name == sym::into_iter => { lint_into_iter(cx, expr, self_ty, *method_span); }, _ => (), @@ -1623,10 +1624,15 @@ impl<'tcx> LateLintPass<'tcx> for Methods { let item = cx.tcx.hir().expect_item(parent); let def_id = cx.tcx.hir().local_def_id(item.hir_id); let self_ty = cx.tcx.type_of(def_id); + + // if this impl block implements a trait, lint in trait definition instead + if let hir::ItemKind::Impl { of_trait: Some(_), .. } = item.kind { + return; + } + if_chain! { if let hir::ImplItemKind::Fn(ref sig, id) = impl_item.kind; if let Some(first_arg) = iter_input_pats(&sig.decl, cx.tcx.hir().body(id)).next(); - if let hir::ItemKind::Impl{ of_trait: None, .. } = item.kind; let method_def_id = cx.tcx.hir().local_def_id(impl_item.hir_id); let method_sig = cx.tcx.fn_sig(method_def_id); @@ -1668,40 +1674,17 @@ impl<'tcx> LateLintPass<'tcx> for Methods { } } - if let Some((ref conv, self_kinds)) = &CONVENTIONS - .iter() - .find(|(ref conv, _)| conv.check(&name)) - { - if !self_kinds.iter().any(|k| k.matches(cx, self_ty, first_arg_ty)) { - let lint = if item.vis.node.is_pub() { - WRONG_PUB_SELF_CONVENTION - } else { - WRONG_SELF_CONVENTION - }; - - span_lint( - cx, - lint, - first_arg.pat.span, - &format!("methods called `{}` usually take {}; consider choosing a less ambiguous name", - conv, - &self_kinds - .iter() - .map(|k| k.description()) - .collect::>() - .join(" or ") - ), - ); - } - } + lint_wrong_self_convention( + cx, + &name, + item.vis.node.is_pub(), + self_ty, + first_arg_ty, + first_arg.pat.span + ); } } - // if this impl block implements a trait, lint in trait definition instead - if let hir::ItemKind::Impl { of_trait: Some(_), .. } = item.kind { - return; - } - if let hir::ImplItemKind::Fn(_, _) = impl_item.kind { let ret_ty = return_ty(cx, impl_item.hir_id); @@ -1735,8 +1718,23 @@ impl<'tcx> LateLintPass<'tcx> for Methods { } fn check_trait_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx TraitItem<'_>) { + if in_external_macro(cx.tcx.sess, item.span) { + return; + } + + if_chain! { + if let TraitItemKind::Fn(ref sig, _) = item.kind; + if let Some(first_arg_ty) = sig.decl.inputs.iter().next(); + let first_arg_span = first_arg_ty.span; + let first_arg_ty = hir_ty_to_ty(cx.tcx, first_arg_ty); + let self_ty = TraitRef::identity(cx.tcx, item.hir_id.owner.to_def_id()).self_ty(); + + then { + lint_wrong_self_convention(cx, &item.ident.name.as_str(), false, self_ty, first_arg_ty, first_arg_span); + } + } + if_chain! { - if !in_external_macro(cx.tcx.sess, item.span); if item.ident.name == sym::new; if let TraitItemKind::Fn(_, _) = item.kind; let ret_ty = return_ty(cx, item.hir_id); @@ -1757,6 +1755,39 @@ impl<'tcx> LateLintPass<'tcx> for Methods { extract_msrv_attr!(LateContext); } +fn lint_wrong_self_convention<'tcx>( + cx: &LateContext<'tcx>, + item_name: &str, + is_pub: bool, + self_ty: &'tcx TyS<'tcx>, + first_arg_ty: &'tcx TyS<'tcx>, + first_arg_span: Span, +) { + let lint = if is_pub { + WRONG_PUB_SELF_CONVENTION + } else { + WRONG_SELF_CONVENTION + }; + if let Some((ref conv, self_kinds)) = &CONVENTIONS.iter().find(|(ref conv, _)| conv.check(item_name)) { + if !self_kinds.iter().any(|k| k.matches(cx, self_ty, first_arg_ty)) { + span_lint( + cx, + lint, + first_arg_span, + &format!( + "methods called `{}` usually take {}; consider choosing a less ambiguous name", + conv, + &self_kinds + .iter() + .map(|k| k.description()) + .collect::>() + .join(" or ") + ), + ); + } + } +} + /// Checks for the `OR_FUN_CALL` lint. #[allow(clippy::too_many_lines)] fn lint_or_fun_call<'tcx>( @@ -2100,8 +2131,11 @@ fn lint_clone_on_copy(cx: &LateContext<'_>, expr: &hir::Expr<'_>, arg: &hir::Exp cx, CLONE_DOUBLE_REF, expr.span, - "using `clone` on a double-reference; \ - this will copy the reference instead of cloning the inner type", + &format!( + "using `clone` on a double-reference; \ + this will copy the reference of type `{}` instead of cloning the inner type", + ty + ), |diag| { if let Some(snip) = sugg::Sugg::hir_opt(cx, arg) { let mut ty = innermost; @@ -2174,11 +2208,17 @@ fn lint_clone_on_copy(cx: &LateContext<'_>, expr: &hir::Expr<'_>, arg: &hir::Exp } else { snip = None; } - span_lint_and_then(cx, CLONE_ON_COPY, expr.span, "using `clone` on a `Copy` type", |diag| { - if let Some((text, snip)) = snip { - diag.span_suggestion(expr.span, text, snip, Applicability::MachineApplicable); - } - }); + span_lint_and_then( + cx, + CLONE_ON_COPY, + expr.span, + &format!("using `clone` on type `{}` which implements the `Copy` trait", ty), + |diag| { + if let Some((text, snip)) = snip { + diag.span_suggestion(expr.span, text, snip, Applicability::MachineApplicable); + } + }, + ); } } @@ -2638,9 +2678,9 @@ fn lint_unwrap(cx: &LateContext<'_>, expr: &hir::Expr<'_>, unwrap_args: &[hir::E fn lint_expect(cx: &LateContext<'_>, expr: &hir::Expr<'_>, expect_args: &[hir::Expr<'_>]) { let obj_ty = cx.typeck_results().expr_ty(&expect_args[0]).peel_refs(); - let mess = if is_type_diagnostic_item(cx, obj_ty, sym!(option_type)) { + let mess = if is_type_diagnostic_item(cx, obj_ty, sym::option_type) { Some((EXPECT_USED, "an Option", "None")) - } else if is_type_diagnostic_item(cx, obj_ty, sym!(result_type)) { + } else if is_type_diagnostic_item(cx, obj_ty, sym::result_type) { Some((EXPECT_USED, "a Result", "Err")) } else { None @@ -2733,6 +2773,8 @@ fn lint_map_flatten<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx hir::Expr<'_>, map } } +const MAP_UNWRAP_OR_MSRV: RustcVersion = RustcVersion::new(1, 41, 0); + /// lint use of `map().unwrap_or_else()` for `Option`s and `Result`s /// Return true if lint triggered fn lint_map_unwrap_or_else<'tcx>( @@ -2740,7 +2782,11 @@ fn lint_map_unwrap_or_else<'tcx>( expr: &'tcx hir::Expr<'_>, map_args: &'tcx [hir::Expr<'_>], unwrap_args: &'tcx [hir::Expr<'_>], + msrv: Option<&RustcVersion>, ) -> bool { + if !meets_msrv(msrv, &MAP_UNWRAP_OR_MSRV) { + return false; + } // lint if the caller of `map()` is an `Option` let is_option = is_type_diagnostic_item(cx, cx.typeck_results().expr_ty(&map_args[0]), sym::option_type); let is_result = is_type_diagnostic_item(cx, cx.typeck_results().expr_ty(&map_args[0]), sym::result_type); @@ -2923,9 +2969,20 @@ fn lint_filter_map<'tcx>( } } +const FILTER_MAP_NEXT_MSRV: RustcVersion = RustcVersion::new(1, 30, 0); + /// lint use of `filter_map().next()` for `Iterators` -fn lint_filter_map_next<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx hir::Expr<'_>, filter_args: &'tcx [hir::Expr<'_>]) { +fn lint_filter_map_next<'tcx>( + cx: &LateContext<'tcx>, + expr: &'tcx hir::Expr<'_>, + filter_args: &'tcx [hir::Expr<'_>], + msrv: Option<&RustcVersion>, +) { if match_trait_method(cx, expr, &paths::ITERATOR) { + if !meets_msrv(msrv, &FILTER_MAP_NEXT_MSRV) { + return; + } + let msg = "called `filter_map(..).next()` on an `Iterator`. This is more succinctly expressed by calling \ `.find_map(..)` instead."; let filter_snippet = snippet(cx, filter_args[1].span, ".."); @@ -3116,7 +3173,7 @@ fn lint_search_is_some<'tcx>( else if search_method == "find" { let is_string_or_str_slice = |e| { let self_ty = cx.typeck_results().expr_ty(e).peel_refs(); - if is_type_diagnostic_item(cx, self_ty, sym!(string_type)) { + if is_type_diagnostic_item(cx, self_ty, sym::string_type) { true } else { *self_ty.kind() == ty::Str diff --git a/src/tools/clippy/clippy_lints/src/methods/unnecessary_filter_map.rs b/src/tools/clippy/clippy_lints/src/methods/unnecessary_filter_map.rs index 75e123eb5939d..d082a88cd2db5 100644 --- a/src/tools/clippy/clippy_lints/src/methods/unnecessary_filter_map.rs +++ b/src/tools/clippy/clippy_lints/src/methods/unnecessary_filter_map.rs @@ -69,10 +69,9 @@ fn check_expression<'tcx>(cx: &LateContext<'tcx>, arg_id: hir::HirId, expr: &'tc } } return (true, false); - } else { - // We don't know. It might do anything. - return (true, true); } + // We don't know. It might do anything. + return (true, true); } } (true, true) diff --git a/src/tools/clippy/clippy_lints/src/missing_const_for_fn.rs b/src/tools/clippy/clippy_lints/src/missing_const_for_fn.rs index 25245b3dbf08e..6ebeaced62a33 100644 --- a/src/tools/clippy/clippy_lints/src/missing_const_for_fn.rs +++ b/src/tools/clippy/clippy_lints/src/missing_const_for_fn.rs @@ -1,14 +1,19 @@ use crate::utils::qualify_min_const_fn::is_min_const_fn; -use crate::utils::{fn_has_unsatisfiable_preds, has_drop, is_entrypoint_fn, span_lint, trait_ref_of_method}; +use crate::utils::{ + fn_has_unsatisfiable_preds, has_drop, is_entrypoint_fn, meets_msrv, span_lint, trait_ref_of_method, +}; use rustc_hir as hir; use rustc_hir::intravisit::FnKind; use rustc_hir::{Body, Constness, FnDecl, GenericParamKind, HirId}; -use rustc_lint::{LateContext, LateLintPass}; +use rustc_lint::{LateContext, LateLintPass, LintContext}; use rustc_middle::lint::in_external_macro; -use rustc_session::{declare_lint_pass, declare_tool_lint}; +use rustc_semver::RustcVersion; +use rustc_session::{declare_tool_lint, impl_lint_pass}; use rustc_span::Span; use rustc_typeck::hir_ty_to_ty; +const MISSING_CONST_FOR_FN_MSRV: RustcVersion = RustcVersion::new(1, 37, 0); + declare_clippy_lint! { /// **What it does:** /// @@ -69,7 +74,18 @@ declare_clippy_lint! { "Lint functions definitions that could be made `const fn`" } -declare_lint_pass!(MissingConstForFn => [MISSING_CONST_FOR_FN]); +impl_lint_pass!(MissingConstForFn => [MISSING_CONST_FOR_FN]); + +pub struct MissingConstForFn { + msrv: Option, +} + +impl MissingConstForFn { + #[must_use] + pub fn new(msrv: Option) -> Self { + Self { msrv } + } +} impl<'tcx> LateLintPass<'tcx> for MissingConstForFn { fn check_fn( @@ -81,6 +97,10 @@ impl<'tcx> LateLintPass<'tcx> for MissingConstForFn { span: Span, hir_id: HirId, ) { + if !meets_msrv(self.msrv.as_ref(), &MISSING_CONST_FOR_FN_MSRV) { + return; + } + let def_id = cx.tcx.hir().local_def_id(hir_id); if in_external_macro(cx.tcx.sess, span) || is_entrypoint_fn(cx, def_id.to_def_id()) { @@ -99,7 +119,7 @@ impl<'tcx> LateLintPass<'tcx> for MissingConstForFn { let has_const_generic_params = generics .params .iter() - .any(|param| matches!(param.kind, GenericParamKind::Const{ .. })); + .any(|param| matches!(param.kind, GenericParamKind::Const { .. })); if already_const(header) || has_const_generic_params { return; @@ -126,6 +146,7 @@ impl<'tcx> LateLintPass<'tcx> for MissingConstForFn { span_lint(cx, MISSING_CONST_FOR_FN, span, "this could be a `const fn`"); } } + extract_msrv_attr!(LateContext); } /// Returns true if any of the method parameters is a type that implements `Drop`. The method diff --git a/src/tools/clippy/clippy_lints/src/missing_doc.rs b/src/tools/clippy/clippy_lints/src/missing_doc.rs index 4678f6872f37c..27f1074a0dd8a 100644 --- a/src/tools/clippy/clippy_lints/src/missing_doc.rs +++ b/src/tools/clippy/clippy_lints/src/missing_doc.rs @@ -2,7 +2,7 @@ // *rustc*'s // [`missing_doc`]. // -// [`missing_doc`]: https://github.com/rust-lang/rust/blob/d6d05904697d89099b55da3331155392f1db9c00/src/librustc_lint/builtin.rs#L246 +// [`missing_doc`]: https://github.com/rust-lang/rust/blob/cf9cf7c923eb01146971429044f216a3ca905e06/compiler/rustc_lint/src/builtin.rs#L415 // use crate::utils::span_lint; @@ -70,7 +70,14 @@ impl MissingDoc { } } - fn check_missing_docs_attrs(&self, cx: &LateContext<'_>, attrs: &[ast::Attribute], sp: Span, desc: &'static str) { + fn check_missing_docs_attrs( + &self, + cx: &LateContext<'_>, + attrs: &[ast::Attribute], + sp: Span, + article: &'static str, + desc: &'static str, + ) { // If we're building a test harness, then warning about // documentation is probably not really relevant right now. if cx.sess().opts.test { @@ -94,7 +101,7 @@ impl MissingDoc { cx, MISSING_DOCS_IN_PRIVATE_ITEMS, sp, - &format!("missing documentation for {}", desc), + &format!("missing documentation for {} {}", article, desc), ); } } @@ -120,13 +127,11 @@ impl<'tcx> LateLintPass<'tcx> for MissingDoc { } fn check_crate(&mut self, cx: &LateContext<'tcx>, krate: &'tcx hir::Crate<'_>) { - self.check_missing_docs_attrs(cx, &krate.item.attrs, krate.item.span, "crate"); + self.check_missing_docs_attrs(cx, &krate.item.attrs, krate.item.span, "the", "crate"); } fn check_item(&mut self, cx: &LateContext<'tcx>, it: &'tcx hir::Item<'_>) { - let desc = match it.kind { - hir::ItemKind::Const(..) => "a constant", - hir::ItemKind::Enum(..) => "an enum", + match it.kind { hir::ItemKind::Fn(..) => { // ignore main() if it.ident.name == sym::main { @@ -136,16 +141,17 @@ impl<'tcx> LateLintPass<'tcx> for MissingDoc { return; } } - "a function" }, - hir::ItemKind::Mod(..) => "a module", - hir::ItemKind::Static(..) => "a static", - hir::ItemKind::Struct(..) => "a struct", - hir::ItemKind::Trait(..) => "a trait", - hir::ItemKind::TraitAlias(..) => "a trait alias", - hir::ItemKind::TyAlias(..) => "a type alias", - hir::ItemKind::Union(..) => "a union", - hir::ItemKind::OpaqueTy(..) => "an existential type", + hir::ItemKind::Const(..) + | hir::ItemKind::Enum(..) + | hir::ItemKind::Mod(..) + | hir::ItemKind::Static(..) + | hir::ItemKind::Struct(..) + | hir::ItemKind::Trait(..) + | hir::ItemKind::TraitAlias(..) + | hir::ItemKind::TyAlias(..) + | hir::ItemKind::Union(..) + | hir::ItemKind::OpaqueTy(..) => {}, hir::ItemKind::ExternCrate(..) | hir::ItemKind::ForeignMod { .. } | hir::ItemKind::GlobalAsm(..) @@ -153,17 +159,17 @@ impl<'tcx> LateLintPass<'tcx> for MissingDoc { | hir::ItemKind::Use(..) => return, }; - self.check_missing_docs_attrs(cx, &it.attrs, it.span, desc); + let def_id = cx.tcx.hir().local_def_id(it.hir_id); + let (article, desc) = cx.tcx.article_and_description(def_id.to_def_id()); + + self.check_missing_docs_attrs(cx, &it.attrs, it.span, article, desc); } fn check_trait_item(&mut self, cx: &LateContext<'tcx>, trait_item: &'tcx hir::TraitItem<'_>) { - let desc = match trait_item.kind { - hir::TraitItemKind::Const(..) => "an associated constant", - hir::TraitItemKind::Fn(..) => "a trait method", - hir::TraitItemKind::Type(..) => "an associated type", - }; + let def_id = cx.tcx.hir().local_def_id(trait_item.hir_id); + let (article, desc) = cx.tcx.article_and_description(def_id.to_def_id()); - self.check_missing_docs_attrs(cx, &trait_item.attrs, trait_item.span, desc); + self.check_missing_docs_attrs(cx, &trait_item.attrs, trait_item.span, article, desc); } fn check_impl_item(&mut self, cx: &LateContext<'tcx>, impl_item: &'tcx hir::ImplItem<'_>) { @@ -178,21 +184,17 @@ impl<'tcx> LateLintPass<'tcx> for MissingDoc { }, } - let desc = match impl_item.kind { - hir::ImplItemKind::Const(..) => "an associated constant", - hir::ImplItemKind::Fn(..) => "a method", - hir::ImplItemKind::TyAlias(_) => "an associated type", - }; - self.check_missing_docs_attrs(cx, &impl_item.attrs, impl_item.span, desc); + let (article, desc) = cx.tcx.article_and_description(def_id.to_def_id()); + self.check_missing_docs_attrs(cx, &impl_item.attrs, impl_item.span, article, desc); } fn check_struct_field(&mut self, cx: &LateContext<'tcx>, sf: &'tcx hir::StructField<'_>) { if !sf.is_positional() { - self.check_missing_docs_attrs(cx, &sf.attrs, sf.span, "a struct field"); + self.check_missing_docs_attrs(cx, &sf.attrs, sf.span, "a", "struct field"); } } fn check_variant(&mut self, cx: &LateContext<'tcx>, v: &'tcx hir::Variant<'_>) { - self.check_missing_docs_attrs(cx, &v.attrs, v.span, "a variant"); + self.check_missing_docs_attrs(cx, &v.attrs, v.span, "a", "variant"); } } diff --git a/src/tools/clippy/clippy_lints/src/needless_borrow.rs b/src/tools/clippy/clippy_lints/src/needless_borrow.rs index 405c21d608d97..bff53eb8ccada 100644 --- a/src/tools/clippy/clippy_lints/src/needless_borrow.rs +++ b/src/tools/clippy/clippy_lints/src/needless_borrow.rs @@ -47,7 +47,7 @@ impl<'tcx> LateLintPass<'tcx> for NeedlessBorrow { return; } if let ExprKind::AddrOf(BorrowKind::Ref, Mutability::Not, ref inner) = e.kind { - if let ty::Ref(..) = cx.typeck_results().expr_ty(inner).kind() { + if let ty::Ref(_, ty, _) = cx.typeck_results().expr_ty(inner).kind() { for adj3 in cx.typeck_results().expr_adjustments(e).windows(3) { if let [Adjustment { kind: Adjust::Deref(_), .. @@ -62,8 +62,11 @@ impl<'tcx> LateLintPass<'tcx> for NeedlessBorrow { cx, NEEDLESS_BORROW, e.span, - "this expression borrows a reference that is immediately dereferenced \ + &format!( + "this expression borrows a reference (`&{}`) that is immediately dereferenced \ by the compiler", + ty + ), |diag| { if let Some(snippet) = snippet_opt(cx, inner.span) { diag.span_suggestion( diff --git a/src/tools/clippy/clippy_lints/src/needless_pass_by_value.rs b/src/tools/clippy/clippy_lints/src/needless_pass_by_value.rs index 532c0266946b3..5043b7b1fc3c1 100644 --- a/src/tools/clippy/clippy_lints/src/needless_pass_by_value.rs +++ b/src/tools/clippy/clippy_lints/src/needless_pass_by_value.rs @@ -90,9 +90,10 @@ impl<'tcx> LateLintPass<'tcx> for NeedlessPassByValue { // Exclude non-inherent impls if let Some(Node::Item(item)) = cx.tcx.hir().find(cx.tcx.hir().get_parent_node(hir_id)) { - if matches!(item.kind, ItemKind::Impl{ of_trait: Some(_), .. } | - ItemKind::Trait(..)) - { + if matches!( + item.kind, + ItemKind::Impl { of_trait: Some(_), .. } | ItemKind::Trait(..) + ) { return; } } diff --git a/src/tools/clippy/clippy_lints/src/needless_update.rs b/src/tools/clippy/clippy_lints/src/needless_update.rs index 98e9078094a22..41cf541ecf5ef 100644 --- a/src/tools/clippy/clippy_lints/src/needless_update.rs +++ b/src/tools/clippy/clippy_lints/src/needless_update.rs @@ -8,6 +8,9 @@ declare_clippy_lint! { /// **What it does:** Checks for needlessly including a base struct on update /// when all fields are changed anyway. /// + /// This lint is not applied to structs marked with + /// [non_exhaustive](https://doc.rust-lang.org/reference/attributes/type_system.html). + /// /// **Why is this bad?** This will cost resources (because the base has to be /// somewhere), and make the code less readable. /// @@ -49,7 +52,9 @@ impl<'tcx> LateLintPass<'tcx> for NeedlessUpdate { if let ExprKind::Struct(_, ref fields, Some(ref base)) = expr.kind { let ty = cx.typeck_results().expr_ty(expr); if let ty::Adt(def, _) = ty.kind() { - if fields.len() == def.non_enum_variant().fields.len() { + if fields.len() == def.non_enum_variant().fields.len() + && !def.variants[0_usize.into()].is_field_list_non_exhaustive() + { span_lint( cx, NEEDLESS_UPDATE, diff --git a/src/tools/clippy/clippy_lints/src/non_expressive_names.rs b/src/tools/clippy/clippy_lints/src/non_expressive_names.rs index 5b42b61fcde90..446426b3e611f 100644 --- a/src/tools/clippy/clippy_lints/src/non_expressive_names.rs +++ b/src/tools/clippy/clippy_lints/src/non_expressive_names.rs @@ -409,11 +409,10 @@ fn levenstein_not_1(a_name: &str, b_name: &str) -> bool { if let Some(b2) = b_chars.next() { // check if there's just one character inserted return a != b2 || a_chars.ne(b_chars); - } else { - // tuple - // ntuple - return true; } + // tuple + // ntuple + return true; } // for item in items true diff --git a/src/tools/clippy/clippy_lints/src/panic_in_result_fn.rs b/src/tools/clippy/clippy_lints/src/panic_in_result_fn.rs index 72dfccc1089e9..37e2b50def17a 100644 --- a/src/tools/clippy/clippy_lints/src/panic_in_result_fn.rs +++ b/src/tools/clippy/clippy_lints/src/panic_in_result_fn.rs @@ -1,18 +1,16 @@ -use crate::utils::{is_expn_of, is_type_diagnostic_item, return_ty, span_lint_and_then}; +use crate::utils::{find_macro_calls, is_type_diagnostic_item, return_ty, span_lint_and_then}; use rustc_hir as hir; -use rustc_hir::intravisit::{self, FnKind, NestedVisitorMap, Visitor}; -use rustc_hir::Expr; +use rustc_hir::intravisit::FnKind; use rustc_lint::{LateContext, LateLintPass}; -use rustc_middle::hir::map::Map; use rustc_session::{declare_lint_pass, declare_tool_lint}; use rustc_span::{sym, Span}; declare_clippy_lint! { - /// **What it does:** Checks for usage of `panic!`, `unimplemented!`, `todo!` or `unreachable!` in a function of type result. + /// **What it does:** Checks for usage of `panic!`, `unimplemented!`, `todo!`, `unreachable!` or assertions in a function of type result. /// - /// **Why is this bad?** For some codebases, it is desirable for functions of type result to return an error instead of crashing. Hence unimplemented, panic and unreachable should be avoided. + /// **Why is this bad?** For some codebases, it is desirable for functions of type result to return an error instead of crashing. Hence panicking macros should be avoided. /// - /// **Known problems:** None. + /// **Known problems:** Functions called from a function returning a `Result` may invoke a panicking macro. This is not checked. /// /// **Example:** /// @@ -22,9 +20,15 @@ declare_clippy_lint! { /// panic!("error"); /// } /// ``` + /// Use instead: + /// ```rust + /// fn result_without_panic() -> Result { + /// Err(String::from("error")) + /// } + /// ``` pub PANIC_IN_RESULT_FN, restriction, - "functions of type `Result<..>` that contain `panic!()`, `todo!()` or `unreachable()` or `unimplemented()` " + "functions of type `Result<..>` that contain `panic!()`, `todo!()`, `unreachable()`, `unimplemented()` or assertion" } declare_lint_pass!(PanicInResultFn => [PANIC_IN_RESULT_FN]); @@ -47,43 +51,33 @@ impl<'tcx> LateLintPass<'tcx> for PanicInResultFn { } } -struct FindPanicUnimplementedUnreachable { - result: Vec, -} - -impl<'tcx> Visitor<'tcx> for FindPanicUnimplementedUnreachable { - type Map = Map<'tcx>; - - fn visit_expr(&mut self, expr: &'tcx Expr<'_>) { - if ["unimplemented", "unreachable", "panic", "todo"] - .iter() - .any(|fun| is_expn_of(expr.span, fun).is_some()) - { - self.result.push(expr.span); - } - // and check sub-expressions - intravisit::walk_expr(self, expr); - } - - fn nested_visit_map(&mut self) -> NestedVisitorMap { - NestedVisitorMap::None - } -} - fn lint_impl_body<'tcx>(cx: &LateContext<'tcx>, impl_span: Span, body: &'tcx hir::Body<'tcx>) { - let mut panics = FindPanicUnimplementedUnreachable { result: Vec::new() }; - panics.visit_expr(&body.value); - if !panics.result.is_empty() { + let panics = find_macro_calls( + &[ + "unimplemented", + "unreachable", + "panic", + "todo", + "assert", + "assert_eq", + "assert_ne", + "debug_assert", + "debug_assert_eq", + "debug_assert_ne", + ], + body, + ); + if !panics.is_empty() { span_lint_and_then( cx, PANIC_IN_RESULT_FN, impl_span, - "used `unimplemented!()`, `unreachable!()`, `todo!()` or `panic!()` in a function that returns `Result`", + "used `unimplemented!()`, `unreachable!()`, `todo!()`, `panic!()` or assertion in a function that returns `Result`", move |diag| { diag.help( - "`unimplemented!()`, `unreachable!()`, `todo!()` or `panic!()` should not be used in a function that returns `Result` as `Result` is expected to return an error instead of crashing", + "`unimplemented!()`, `unreachable!()`, `todo!()`, `panic!()` or assertions should not be used in a function that returns `Result` as `Result` is expected to return an error instead of crashing", ); - diag.span_note(panics.result, "return Err() instead of panicking"); + diag.span_note(panics, "return Err() instead of panicking"); }, ); } diff --git a/src/tools/clippy/clippy_lints/src/pass_by_ref_or_value.rs b/src/tools/clippy/clippy_lints/src/pass_by_ref_or_value.rs index f03facc235e28..6a17d654ac943 100644 --- a/src/tools/clippy/clippy_lints/src/pass_by_ref_or_value.rs +++ b/src/tools/clippy/clippy_lints/src/pass_by_ref_or_value.rs @@ -244,9 +244,10 @@ impl<'tcx> LateLintPass<'tcx> for PassByRefOrValue { // Exclude non-inherent impls if let Some(Node::Item(item)) = cx.tcx.hir().find(cx.tcx.hir().get_parent_node(hir_id)) { - if matches!(item.kind, ItemKind::Impl{ of_trait: Some(_), .. } | - ItemKind::Trait(..)) - { + if matches!( + item.kind, + ItemKind::Impl { of_trait: Some(_), .. } | ItemKind::Trait(..) + ) { return; } } diff --git a/src/tools/clippy/clippy_lints/src/ranges.rs b/src/tools/clippy/clippy_lints/src/ranges.rs index 4b514bbd42ca7..3e454eecd970e 100644 --- a/src/tools/clippy/clippy_lints/src/ranges.rs +++ b/src/tools/clippy/clippy_lints/src/ranges.rs @@ -3,9 +3,10 @@ use if_chain::if_chain; use rustc_ast::ast::RangeLimits; use rustc_errors::Applicability; use rustc_hir::{BinOpKind, Expr, ExprKind, PathSegment, QPath}; -use rustc_lint::{LateContext, LateLintPass}; +use rustc_lint::{LateContext, LateLintPass, LintContext}; use rustc_middle::ty; -use rustc_session::{declare_lint_pass, declare_tool_lint}; +use rustc_semver::RustcVersion; +use rustc_session::{declare_tool_lint, impl_lint_pass}; use rustc_span::source_map::{Span, Spanned}; use rustc_span::sym; use rustc_span::symbol::Ident; @@ -13,8 +14,8 @@ use std::cmp::Ordering; use crate::utils::sugg::Sugg; use crate::utils::{ - get_parent_expr, is_integer_const, single_segment_path, snippet, snippet_opt, snippet_with_applicability, - span_lint, span_lint_and_sugg, span_lint_and_then, + get_parent_expr, in_constant, is_integer_const, meets_msrv, single_segment_path, snippet, snippet_opt, + snippet_with_applicability, span_lint, span_lint_and_sugg, span_lint_and_then, }; use crate::utils::{higher, SpanlessEq}; @@ -160,7 +161,20 @@ declare_clippy_lint! { "manually reimplementing {`Range`, `RangeInclusive`}`::contains`" } -declare_lint_pass!(Ranges => [ +const MANUAL_RANGE_CONTAINS_MSRV: RustcVersion = RustcVersion::new(1, 35, 0); + +pub struct Ranges { + msrv: Option, +} + +impl Ranges { + #[must_use] + pub fn new(msrv: Option) -> Self { + Self { msrv } + } +} + +impl_lint_pass!(Ranges => [ RANGE_ZIP_WITH_LEN, RANGE_PLUS_ONE, RANGE_MINUS_ONE, @@ -175,7 +189,9 @@ impl<'tcx> LateLintPass<'tcx> for Ranges { check_range_zip_with_len(cx, path, args, expr.span); }, ExprKind::Binary(ref op, ref l, ref r) => { - check_possible_range_contains(cx, op.node, l, r, expr.span); + if meets_msrv(self.msrv.as_ref(), &MANUAL_RANGE_CONTAINS_MSRV) { + check_possible_range_contains(cx, op.node, l, r, expr); + } }, _ => {}, } @@ -184,9 +200,15 @@ impl<'tcx> LateLintPass<'tcx> for Ranges { check_inclusive_range_minus_one(cx, expr); check_reversed_empty_range(cx, expr); } + extract_msrv_attr!(LateContext); } -fn check_possible_range_contains(cx: &LateContext<'_>, op: BinOpKind, l: &Expr<'_>, r: &Expr<'_>, span: Span) { +fn check_possible_range_contains(cx: &LateContext<'_>, op: BinOpKind, l: &Expr<'_>, r: &Expr<'_>, expr: &Expr<'_>) { + if in_constant(cx, expr.hir_id) { + return; + } + + let span = expr.span; let combine_and = match op { BinOpKind::And | BinOpKind::BitAnd => true, BinOpKind::Or | BinOpKind::BitOr => false, diff --git a/src/tools/clippy/clippy_lints/src/redundant_clone.rs b/src/tools/clippy/clippy_lints/src/redundant_clone.rs index f0e507105a6a0..06adbb523d706 100644 --- a/src/tools/clippy/clippy_lints/src/redundant_clone.rs +++ b/src/tools/clippy/clippy_lints/src/redundant_clone.rs @@ -390,7 +390,10 @@ impl<'tcx> mir::visit::Visitor<'tcx> for LocalUseVisitor { let local = place.local; if local == self.used.0 - && !matches!(ctx, PlaceContext::MutatingUse(MutatingUseContext::Drop) | PlaceContext::NonUse(_)) + && !matches!( + ctx, + PlaceContext::MutatingUse(MutatingUseContext::Drop) | PlaceContext::NonUse(_) + ) { self.used.1 = true; } diff --git a/src/tools/clippy/clippy_lints/src/redundant_else.rs b/src/tools/clippy/clippy_lints/src/redundant_else.rs new file mode 100644 index 0000000000000..3d585cd27a3d0 --- /dev/null +++ b/src/tools/clippy/clippy_lints/src/redundant_else.rs @@ -0,0 +1,135 @@ +use crate::utils::span_lint_and_help; +use rustc_ast::ast::{Block, Expr, ExprKind, Stmt, StmtKind}; +use rustc_ast::visit::{walk_expr, Visitor}; +use rustc_lint::{EarlyContext, EarlyLintPass}; +use rustc_middle::lint::in_external_macro; +use rustc_session::{declare_lint_pass, declare_tool_lint}; + +declare_clippy_lint! { + /// **What it does:** Checks for `else` blocks that can be removed without changing semantics. + /// + /// **Why is this bad?** The `else` block adds unnecessary indentation and verbosity. + /// + /// **Known problems:** Some may prefer to keep the `else` block for clarity. + /// + /// **Example:** + /// + /// ```rust + /// fn my_func(count: u32) { + /// if count == 0 { + /// print!("Nothing to do"); + /// return; + /// } else { + /// print!("Moving on..."); + /// } + /// } + /// ``` + /// Use instead: + /// ```rust + /// fn my_func(count: u32) { + /// if count == 0 { + /// print!("Nothing to do"); + /// return; + /// } + /// print!("Moving on..."); + /// } + /// ``` + pub REDUNDANT_ELSE, + pedantic, + "`else` branch that can be removed without changing semantics" +} + +declare_lint_pass!(RedundantElse => [REDUNDANT_ELSE]); + +impl EarlyLintPass for RedundantElse { + fn check_stmt(&mut self, cx: &EarlyContext<'_>, stmt: &Stmt) { + if in_external_macro(cx.sess, stmt.span) { + return; + } + // Only look at expressions that are a whole statement + let expr: &Expr = match &stmt.kind { + StmtKind::Expr(expr) | StmtKind::Semi(expr) => expr, + _ => return, + }; + // if else + let (mut then, mut els): (&Block, &Expr) = match &expr.kind { + ExprKind::If(_, then, Some(els)) => (then, els), + _ => return, + }; + loop { + if !BreakVisitor::default().check_block(then) { + // then block does not always break + return; + } + match &els.kind { + // else if else + ExprKind::If(_, next_then, Some(next_els)) => { + then = next_then; + els = next_els; + continue; + }, + // else if without else + ExprKind::If(..) => return, + // done + _ => break, + } + } + span_lint_and_help( + cx, + REDUNDANT_ELSE, + els.span, + "redundant else block", + None, + "remove the `else` block and move the contents out", + ); + } +} + +/// Call `check` functions to check if an expression always breaks control flow +#[derive(Default)] +struct BreakVisitor { + is_break: bool, +} + +impl<'ast> Visitor<'ast> for BreakVisitor { + fn visit_block(&mut self, block: &'ast Block) { + self.is_break = match block.stmts.as_slice() { + [.., last] => self.check_stmt(last), + _ => false, + }; + } + + fn visit_expr(&mut self, expr: &'ast Expr) { + self.is_break = match expr.kind { + ExprKind::Break(..) | ExprKind::Continue(..) | ExprKind::Ret(..) => true, + ExprKind::Match(_, ref arms) => arms.iter().all(|arm| self.check_expr(&arm.body)), + ExprKind::If(_, ref then, Some(ref els)) => self.check_block(then) && self.check_expr(els), + ExprKind::If(_, _, None) + // ignore loops for simplicity + | ExprKind::While(..) | ExprKind::ForLoop(..) | ExprKind::Loop(..) => false, + _ => { + walk_expr(self, expr); + return; + }, + }; + } +} + +impl BreakVisitor { + fn check(&mut self, item: T, visit: fn(&mut Self, T)) -> bool { + visit(self, item); + std::mem::replace(&mut self.is_break, false) + } + + fn check_block(&mut self, block: &Block) -> bool { + self.check(block, Self::visit_block) + } + + fn check_expr(&mut self, expr: &Expr) -> bool { + self.check(expr, Self::visit_expr) + } + + fn check_stmt(&mut self, stmt: &Stmt) -> bool { + self.check(stmt, Self::visit_stmt) + } +} diff --git a/src/tools/clippy/clippy_lints/src/redundant_field_names.rs b/src/tools/clippy/clippy_lints/src/redundant_field_names.rs index 2a81170e49e75..38dcf7a192c82 100644 --- a/src/tools/clippy/clippy_lints/src/redundant_field_names.rs +++ b/src/tools/clippy/clippy_lints/src/redundant_field_names.rs @@ -1,9 +1,12 @@ -use crate::utils::span_lint_and_sugg; +use crate::utils::{meets_msrv, span_lint_and_sugg}; use rustc_ast::ast::{Expr, ExprKind}; use rustc_errors::Applicability; use rustc_lint::{EarlyContext, EarlyLintPass}; use rustc_middle::lint::in_external_macro; -use rustc_session::{declare_lint_pass, declare_tool_lint}; +use rustc_semver::RustcVersion; +use rustc_session::{declare_tool_lint, impl_lint_pass}; + +const REDUNDANT_FIELD_NAMES_MSRV: RustcVersion = RustcVersion::new(1, 17, 0); declare_clippy_lint! { /// **What it does:** Checks for fields in struct literals where shorthands @@ -33,10 +36,25 @@ declare_clippy_lint! { "checks for fields in struct literals where shorthands could be used" } -declare_lint_pass!(RedundantFieldNames => [REDUNDANT_FIELD_NAMES]); +pub struct RedundantFieldNames { + msrv: Option, +} + +impl RedundantFieldNames { + #[must_use] + pub fn new(msrv: Option) -> Self { + Self { msrv } + } +} + +impl_lint_pass!(RedundantFieldNames => [REDUNDANT_FIELD_NAMES]); impl EarlyLintPass for RedundantFieldNames { fn check_expr(&mut self, cx: &EarlyContext<'_>, expr: &Expr) { + if !meets_msrv(self.msrv.as_ref(), &REDUNDANT_FIELD_NAMES_MSRV) { + return; + } + if in_external_macro(cx.sess, expr.span) { return; } @@ -64,4 +82,5 @@ impl EarlyLintPass for RedundantFieldNames { } } } + extract_msrv_attr!(EarlyContext); } diff --git a/src/tools/clippy/clippy_lints/src/redundant_static_lifetimes.rs b/src/tools/clippy/clippy_lints/src/redundant_static_lifetimes.rs index 7bbcc67aa2ddf..fcfa3c12755af 100644 --- a/src/tools/clippy/clippy_lints/src/redundant_static_lifetimes.rs +++ b/src/tools/clippy/clippy_lints/src/redundant_static_lifetimes.rs @@ -1,8 +1,11 @@ -use crate::utils::{snippet, span_lint_and_then}; +use crate::utils::{meets_msrv, snippet, span_lint_and_then}; use rustc_ast::ast::{Item, ItemKind, Ty, TyKind}; use rustc_errors::Applicability; use rustc_lint::{EarlyContext, EarlyLintPass}; -use rustc_session::{declare_lint_pass, declare_tool_lint}; +use rustc_semver::RustcVersion; +use rustc_session::{declare_tool_lint, impl_lint_pass}; + +const REDUNDANT_STATIC_LIFETIMES_MSRV: RustcVersion = RustcVersion::new(1, 17, 0); declare_clippy_lint! { /// **What it does:** Checks for constants and statics with an explicit `'static` lifetime. @@ -29,7 +32,18 @@ declare_clippy_lint! { "Using explicit `'static` lifetime for constants or statics when elision rules would allow omitting them." } -declare_lint_pass!(RedundantStaticLifetimes => [REDUNDANT_STATIC_LIFETIMES]); +pub struct RedundantStaticLifetimes { + msrv: Option, +} + +impl RedundantStaticLifetimes { + #[must_use] + pub fn new(msrv: Option) -> Self { + Self { msrv } + } +} + +impl_lint_pass!(RedundantStaticLifetimes => [REDUNDANT_STATIC_LIFETIMES]); impl RedundantStaticLifetimes { // Recursively visit types @@ -84,6 +98,10 @@ impl RedundantStaticLifetimes { impl EarlyLintPass for RedundantStaticLifetimes { fn check_item(&mut self, cx: &EarlyContext<'_>, item: &Item) { + if !meets_msrv(self.msrv.as_ref(), &REDUNDANT_STATIC_LIFETIMES_MSRV) { + return; + } + if !item.span.from_expansion() { if let ItemKind::Const(_, ref var_type, _) = item.kind { self.visit_type(var_type, cx, "constants have by default a `'static` lifetime"); @@ -96,4 +114,6 @@ impl EarlyLintPass for RedundantStaticLifetimes { } } } + + extract_msrv_attr!(EarlyContext); } diff --git a/src/tools/clippy/clippy_lints/src/ref_option_ref.rs b/src/tools/clippy/clippy_lints/src/ref_option_ref.rs index a914a77d48b4e..803ebada54b79 100644 --- a/src/tools/clippy/clippy_lints/src/ref_option_ref.rs +++ b/src/tools/clippy/clippy_lints/src/ref_option_ref.rs @@ -2,6 +2,7 @@ use crate::utils::{last_path_segment, snippet, span_lint_and_sugg}; use rustc_hir::{GenericArg, Mutability, Ty, TyKind}; use rustc_lint::{LateContext, LateLintPass}; use rustc_session::{declare_lint_pass, declare_tool_lint}; +use rustc_span::symbol::sym; use if_chain::if_chain; use rustc_errors::Applicability; @@ -41,7 +42,7 @@ impl<'tcx> LateLintPass<'tcx> for RefOptionRef { if let Some(res) = last.res; if let Some(def_id) = res.opt_def_id(); - if cx.tcx.is_diagnostic_item(sym!(option_type), def_id); + if cx.tcx.is_diagnostic_item(sym::option_type, def_id); if let Some(ref params) = last_path_segment(qpath).args ; if !params.parenthesized; if let Some(inner_ty) = params.args.iter().find_map(|arg| match arg { diff --git a/src/tools/clippy/clippy_lints/src/strings.rs b/src/tools/clippy/clippy_lints/src/strings.rs index 77e790733789c..31dd5965473d3 100644 --- a/src/tools/clippy/clippy_lints/src/strings.rs +++ b/src/tools/clippy/clippy_lints/src/strings.rs @@ -372,7 +372,7 @@ impl LateLintPass<'_> for StringToString { if let ExprKind::MethodCall(path, _, args, _) = &expr.kind; if path.ident.name == sym!(to_string); let ty = cx.typeck_results().expr_ty(&args[0]); - if is_type_diagnostic_item(cx, ty, sym!(string_type)); + if is_type_diagnostic_item(cx, ty, sym::string_type); then { span_lint_and_help( cx, diff --git a/src/tools/clippy/clippy_lints/src/types.rs b/src/tools/clippy/clippy_lints/src/types.rs index 74ba53e6a9a02..fd74783335d57 100644 --- a/src/tools/clippy/clippy_lints/src/types.rs +++ b/src/tools/clippy/clippy_lints/src/types.rs @@ -1104,7 +1104,9 @@ fn is_empty_block(expr: &Expr<'_>) -> bool { expr.kind, ExprKind::Block( Block { - stmts: &[], expr: None, .. + stmts: &[], + expr: None, + .. }, _, ) diff --git a/src/tools/clippy/clippy_lints/src/unnecessary_wraps.rs b/src/tools/clippy/clippy_lints/src/unnecessary_wraps.rs index 5d801511a0b18..5b9a80f92db69 100644 --- a/src/tools/clippy/clippy_lints/src/unnecessary_wraps.rs +++ b/src/tools/clippy/clippy_lints/src/unnecessary_wraps.rs @@ -9,6 +9,7 @@ use rustc_hir::{Body, ExprKind, FnDecl, HirId, ItemKind, Node}; use rustc_lint::{LateContext, LateLintPass}; use rustc_middle::ty::subst::GenericArgKind; use rustc_session::{declare_lint_pass, declare_tool_lint}; +use rustc_span::symbol::sym; use rustc_span::Span; declare_clippy_lint! { @@ -74,14 +75,17 @@ impl<'tcx> LateLintPass<'tcx> for UnnecessaryWraps { } if let Some(Node::Item(item)) = cx.tcx.hir().find(cx.tcx.hir().get_parent_node(hir_id)) { - if matches!(item.kind, ItemKind::Impl{ of_trait: Some(_), ..} | ItemKind::Trait(..)) { + if matches!( + item.kind, + ItemKind::Impl { of_trait: Some(_), .. } | ItemKind::Trait(..) + ) { return; } } - let (return_type, path) = if is_type_diagnostic_item(cx, return_ty(cx, hir_id), sym!(option_type)) { + let (return_type, path) = if is_type_diagnostic_item(cx, return_ty(cx, hir_id), sym::option_type) { ("Option", &paths::OPTION_SOME) - } else if is_type_diagnostic_item(cx, return_ty(cx, hir_id), sym!(result_type)) { + } else if is_type_diagnostic_item(cx, return_ty(cx, hir_id), sym::result_type) { ("Result", &paths::RESULT_OK) } else { return; diff --git a/src/tools/clippy/clippy_lints/src/use_self.rs b/src/tools/clippy/clippy_lints/src/use_self.rs index 5ac4797680bc1..3b23f885e08d9 100644 --- a/src/tools/clippy/clippy_lints/src/use_self.rs +++ b/src/tools/clippy/clippy_lints/src/use_self.rs @@ -12,11 +12,12 @@ use rustc_middle::hir::map::Map; use rustc_middle::lint::in_external_macro; use rustc_middle::ty; use rustc_middle::ty::{DefIdTree, Ty}; -use rustc_session::{declare_lint_pass, declare_tool_lint}; +use rustc_semver::RustcVersion; +use rustc_session::{declare_tool_lint, impl_lint_pass}; use rustc_span::symbol::kw; use rustc_typeck::hir_ty_to_ty; -use crate::utils::{differing_macro_contexts, span_lint_and_sugg}; +use crate::utils::{differing_macro_contexts, meets_msrv, span_lint_and_sugg}; declare_clippy_lint! { /// **What it does:** Checks for unnecessary repetition of structure name when a @@ -53,7 +54,7 @@ declare_clippy_lint! { "unnecessary structure name repetition whereas `Self` is applicable" } -declare_lint_pass!(UseSelf => [USE_SELF]); +impl_lint_pass!(UseSelf => [USE_SELF]); const SEGMENTS_MSG: &str = "segments should be composed of at least 1 element"; @@ -157,8 +158,25 @@ fn check_trait_method_impl_decl<'tcx>( } } +const USE_SELF_MSRV: RustcVersion = RustcVersion::new(1, 37, 0); + +pub struct UseSelf { + msrv: Option, +} + +impl UseSelf { + #[must_use] + pub fn new(msrv: Option) -> Self { + Self { msrv } + } +} + impl<'tcx> LateLintPass<'tcx> for UseSelf { fn check_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx Item<'_>) { + if !meets_msrv(self.msrv.as_ref(), &USE_SELF_MSRV) { + return; + } + if in_external_macro(cx.sess(), item.span) { return; } @@ -204,6 +222,7 @@ impl<'tcx> LateLintPass<'tcx> for UseSelf { } } } + extract_msrv_attr!(LateContext); } struct UseSelfVisitor<'a, 'tcx> { diff --git a/src/tools/clippy/clippy_lints/src/utils/ast_utils.rs b/src/tools/clippy/clippy_lints/src/utils/ast_utils.rs index 31b4e25411bd3..f0267e4c79289 100644 --- a/src/tools/clippy/clippy_lints/src/utils/ast_utils.rs +++ b/src/tools/clippy/clippy_lints/src/utils/ast_utils.rs @@ -408,7 +408,10 @@ pub fn eq_use_tree_kind(l: &UseTreeKind, r: &UseTreeKind) -> bool { } pub fn eq_defaultness(l: Defaultness, r: Defaultness) -> bool { - matches!((l, r), (Defaultness::Final, Defaultness::Final) | (Defaultness::Default(_), Defaultness::Default(_))) + matches!( + (l, r), + (Defaultness::Final, Defaultness::Final) | (Defaultness::Default(_), Defaultness::Default(_)) + ) } pub fn eq_vis(l: &Visibility, r: &Visibility) -> bool { diff --git a/src/tools/clippy/clippy_lints/src/utils/conf.rs b/src/tools/clippy/clippy_lints/src/utils/conf.rs index 6403ff6dad185..32d7840a451c6 100644 --- a/src/tools/clippy/clippy_lints/src/utils/conf.rs +++ b/src/tools/clippy/clippy_lints/src/utils/conf.rs @@ -106,7 +106,7 @@ macro_rules! define_Conf { pub use self::helpers::Conf; define_Conf! { - /// Lint: MANUAL_NON_EXHAUSTIVE, MANUAL_STRIP, OPTION_AS_REF_DEREF, MATCH_LIKE_MATCHES_MACRO. The minimum rust version that the project supports + /// Lint: REDUNDANT_FIELD_NAMES, REDUNDANT_STATIC_LIFETIMES, FILTER_MAP_NEXT, CHECKED_CONVERSIONS, MANUAL_RANGE_CONTAINS, USE_SELF, MEM_REPLACE_WITH_DEFAULT, MANUAL_NON_EXHAUSTIVE, OPTION_AS_REF_DEREF, MAP_UNWRAP_OR, MATCH_LIKE_MATCHES_MACRO, MANUAL_STRIP, MISSING_CONST_FOR_FN. The minimum rust version that the project supports (msrv, "msrv": Option, None), /// Lint: BLACKLISTED_NAME. The list of blacklisted names to lint about. NB: `bar` is not here since it has legitimate uses (blacklisted_names, "blacklisted_names": Vec, ["foo", "baz", "quux"].iter().map(ToString::to_string).collect()), diff --git a/src/tools/clippy/clippy_lints/src/utils/internal_lints.rs b/src/tools/clippy/clippy_lints/src/utils/internal_lints.rs index 8b59a9541a736..9ba39f73ee884 100644 --- a/src/tools/clippy/clippy_lints/src/utils/internal_lints.rs +++ b/src/tools/clippy/clippy_lints/src/utils/internal_lints.rs @@ -15,6 +15,7 @@ use rustc_hir::intravisit::{NestedVisitorMap, Visitor}; use rustc_hir::{Crate, Expr, ExprKind, HirId, Item, MutTy, Mutability, Node, Path, StmtKind, Ty, TyKind}; use rustc_lint::{EarlyContext, EarlyLintPass, LateContext, LateLintPass}; use rustc_middle::hir::map::Map; +use rustc_middle::mir::interpret::ConstValue; use rustc_middle::ty; use rustc_session::{declare_lint_pass, declare_tool_lint, impl_lint_pass}; use rustc_span::source_map::{Span, Spanned}; @@ -247,6 +248,30 @@ declare_clippy_lint! { "invalid path" } +declare_clippy_lint! { + /// **What it does:** + /// Checks for interning symbols that have already been pre-interned and defined as constants. + /// + /// **Why is this bad?** + /// It's faster and easier to use the symbol constant. + /// + /// **Known problems:** None. + /// + /// **Example:** + /// Bad: + /// ```rust,ignore + /// let _ = sym!(f32); + /// ``` + /// + /// Good: + /// ```rust,ignore + /// let _ = sym::f32; + /// ``` + pub INTERNING_DEFINED_SYMBOL, + internal, + "interning a symbol that is pre-interned and defined as a constant" +} + declare_lint_pass!(ClippyLintsInternal => [CLIPPY_LINTS_INTERNAL]); impl EarlyLintPass for ClippyLintsInternal { @@ -840,3 +865,56 @@ impl<'tcx> LateLintPass<'tcx> for InvalidPaths { } } } + +#[derive(Default)] +pub struct InterningDefinedSymbol { + // Maps the symbol value to the constant name. + symbol_map: FxHashMap, +} + +impl_lint_pass!(InterningDefinedSymbol => [INTERNING_DEFINED_SYMBOL]); + +impl<'tcx> LateLintPass<'tcx> for InterningDefinedSymbol { + fn check_crate(&mut self, cx: &LateContext<'_>, _: &Crate<'_>) { + if !self.symbol_map.is_empty() { + return; + } + + if let Some(Res::Def(_, def_id)) = path_to_res(cx, &paths::SYM_MODULE) { + for item in cx.tcx.item_children(def_id).iter() { + if_chain! { + if let Res::Def(DefKind::Const, item_def_id) = item.res; + let ty = cx.tcx.type_of(item_def_id); + if match_type(cx, ty, &paths::SYMBOL); + if let Ok(ConstValue::Scalar(value)) = cx.tcx.const_eval_poly(item_def_id); + if let Ok(value) = value.to_u32(); + then { + self.symbol_map.insert(value, item.ident.to_string()); + } + } + } + } + } + + fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) { + if_chain! { + if let ExprKind::Call(func, [arg]) = &expr.kind; + if let ty::FnDef(def_id, _) = cx.typeck_results().expr_ty(func).kind(); + if match_def_path(cx, *def_id, &paths::SYMBOL_INTERN); + if let Some(Constant::Str(arg)) = constant_simple(cx, cx.typeck_results(), arg); + let value = Symbol::intern(&arg).as_u32(); + if let Some(symbol_const) = self.symbol_map.get(&value); + then { + span_lint_and_sugg( + cx, + INTERNING_DEFINED_SYMBOL, + is_expn_of(expr.span, "sym").unwrap_or(expr.span), + "interning a defined symbol", + "try", + format!("rustc_span::symbol::sym::{}", symbol_const), + Applicability::MachineApplicable, + ); + } + } + } +} diff --git a/src/tools/clippy/clippy_lints/src/utils/mod.rs b/src/tools/clippy/clippy_lints/src/utils/mod.rs index 0deaee3a944a0..424856090f261 100644 --- a/src/tools/clippy/clippy_lints/src/utils/mod.rs +++ b/src/tools/clippy/clippy_lints/src/utils/mod.rs @@ -41,7 +41,7 @@ use rustc_errors::Applicability; use rustc_hir as hir; use rustc_hir::def::{DefKind, Res}; use rustc_hir::def_id::{DefId, CRATE_DEF_INDEX, LOCAL_CRATE}; -use rustc_hir::intravisit::{NestedVisitorMap, Visitor}; +use rustc_hir::intravisit::{self, NestedVisitorMap, Visitor}; use rustc_hir::Node; use rustc_hir::{ def, Arm, Block, Body, Constness, Crate, Expr, ExprKind, FnDecl, HirId, ImplItem, ImplItemKind, Item, ItemKind, @@ -603,6 +603,37 @@ pub fn contains_return(expr: &hir::Expr<'_>) -> bool { visitor.found } +struct FindMacroCalls<'a, 'b> { + names: &'a [&'b str], + result: Vec, +} + +impl<'a, 'b, 'tcx> Visitor<'tcx> for FindMacroCalls<'a, 'b> { + type Map = Map<'tcx>; + + fn visit_expr(&mut self, expr: &'tcx Expr<'_>) { + if self.names.iter().any(|fun| is_expn_of(expr.span, fun).is_some()) { + self.result.push(expr.span); + } + // and check sub-expressions + intravisit::walk_expr(self, expr); + } + + fn nested_visit_map(&mut self) -> NestedVisitorMap { + NestedVisitorMap::None + } +} + +/// Finds calls of the specified macros in a function body. +pub fn find_macro_calls(names: &[&str], body: &Body<'_>) -> Vec { + let mut fmc = FindMacroCalls { + names, + result: Vec::new(), + }; + fmc.visit_expr(&body.value); + fmc.result +} + /// Converts a span to a code snippet if available, otherwise use default. /// /// This is useful if you want to provide suggestions for your lint or more generally, if you want @@ -1500,7 +1531,7 @@ pub fn is_no_std_crate(krate: &Crate<'_>) -> bool { /// ``` pub fn is_trait_impl_item(cx: &LateContext<'_>, hir_id: HirId) -> bool { if let Some(Node::Item(item)) = cx.tcx.hir().find(cx.tcx.hir().get_parent_node(hir_id)) { - matches!(item.kind, ItemKind::Impl{ of_trait: Some(_), .. }) + matches!(item.kind, ItemKind::Impl { of_trait: Some(_), .. }) } else { false } diff --git a/src/tools/clippy/clippy_lints/src/utils/paths.rs b/src/tools/clippy/clippy_lints/src/utils/paths.rs index 6fdc7b4587f04..2080a49a11cd6 100644 --- a/src/tools/clippy/clippy_lints/src/utils/paths.rs +++ b/src/tools/clippy/clippy_lints/src/utils/paths.rs @@ -146,6 +146,12 @@ pub const STR_FROM_UTF8: [&str; 4] = ["core", "str", "converts", "from_utf8"]; pub const STR_LEN: [&str; 4] = ["core", "str", "", "len"]; pub const STR_STARTS_WITH: [&str; 4] = ["core", "str", "", "starts_with"]; #[cfg(feature = "internal-lints")] +pub const SYMBOL: [&str; 3] = ["rustc_span", "symbol", "Symbol"]; +#[cfg(feature = "internal-lints")] +pub const SYMBOL_INTERN: [&str; 4] = ["rustc_span", "symbol", "Symbol", "intern"]; +#[cfg(feature = "internal-lints")] +pub const SYM_MODULE: [&str; 3] = ["rustc_span", "symbol", "sym"]; +#[cfg(feature = "internal-lints")] pub const SYNTAX_CONTEXT: [&str; 3] = ["rustc_span", "hygiene", "SyntaxContext"]; pub const TO_OWNED: [&str; 3] = ["alloc", "borrow", "ToOwned"]; pub const TO_OWNED_METHOD: [&str; 4] = ["alloc", "borrow", "ToOwned", "to_owned"]; diff --git a/src/tools/clippy/clippy_lints/src/utils/usage.rs b/src/tools/clippy/clippy_lints/src/utils/usage.rs index a7d0ea6ccfbb2..fc0db7f64ec95 100644 --- a/src/tools/clippy/clippy_lints/src/utils/usage.rs +++ b/src/tools/clippy/clippy_lints/src/utils/usage.rs @@ -116,20 +116,27 @@ pub struct ParamBindingIdCollector { } impl<'tcx> ParamBindingIdCollector { fn collect_binding_hir_ids(body: &'tcx hir::Body<'tcx>) -> Vec { - let mut finder = ParamBindingIdCollector { - binding_hir_ids: Vec::new(), - }; - finder.visit_body(body); - finder.binding_hir_ids + let mut hir_ids: Vec = Vec::new(); + for param in body.params.iter() { + let mut finder = ParamBindingIdCollector { + binding_hir_ids: Vec::new(), + }; + finder.visit_param(param); + for hir_id in &finder.binding_hir_ids { + hir_ids.push(*hir_id); + } + } + hir_ids } } impl<'tcx> intravisit::Visitor<'tcx> for ParamBindingIdCollector { type Map = Map<'tcx>; - fn visit_param(&mut self, param: &'tcx hir::Param<'tcx>) { - if let hir::PatKind::Binding(_, hir_id, ..) = param.pat.kind { + fn visit_pat(&mut self, pat: &'tcx hir::Pat<'tcx>) { + if let hir::PatKind::Binding(_, hir_id, ..) = pat.kind { self.binding_hir_ids.push(hir_id); } + intravisit::walk_pat(self, pat); } fn nested_visit_map(&mut self) -> intravisit::NestedVisitorMap { diff --git a/src/tools/clippy/clippy_lints/src/write.rs b/src/tools/clippy/clippy_lints/src/write.rs index ff414f748ef97..337f7a229b906 100644 --- a/src/tools/clippy/clippy_lints/src/write.rs +++ b/src/tools/clippy/clippy_lints/src/write.rs @@ -75,6 +75,24 @@ declare_clippy_lint! { "printing on stdout" } +declare_clippy_lint! { + /// **What it does:** Checks for printing on *stderr*. The purpose of this lint + /// is to catch debugging remnants. + /// + /// **Why is this bad?** People often print on *stderr* while debugging an + /// application and might forget to remove those prints afterward. + /// + /// **Known problems:** Only catches `eprint!` and `eprintln!` calls. + /// + /// **Example:** + /// ```rust + /// eprintln!("Hello world!"); + /// ``` + pub PRINT_STDERR, + restriction, + "printing on stderr" +} + declare_clippy_lint! { /// **What it does:** Checks for use of `Debug` formatting. The purpose of this /// lint is to catch debugging remnants. @@ -201,6 +219,7 @@ impl_lint_pass!(Write => [ PRINT_WITH_NEWLINE, PRINTLN_EMPTY_STRING, PRINT_STDOUT, + PRINT_STDERR, USE_DEBUG, PRINT_LITERAL, WRITE_WITH_NEWLINE, @@ -243,47 +262,22 @@ impl EarlyLintPass for Write { .map_or(false, |crate_name| crate_name == "build_script_build") } - if mac.path == sym!(println) { - if !is_build_script(cx) { - span_lint(cx, PRINT_STDOUT, mac.span(), "use of `println!`"); - } - if let (Some(fmt_str), _) = self.check_tts(cx, mac.args.inner_tokens(), false) { - if fmt_str.symbol == Symbol::intern("") { - span_lint_and_sugg( - cx, - PRINTLN_EMPTY_STRING, - mac.span(), - "using `println!(\"\")`", - "replace it with", - "println!()".to_string(), - Applicability::MachineApplicable, - ); - } - } - } else if mac.path == sym!(print) { + if mac.path == sym!(print) { if !is_build_script(cx) { span_lint(cx, PRINT_STDOUT, mac.span(), "use of `print!`"); } - if let (Some(fmt_str), _) = self.check_tts(cx, mac.args.inner_tokens(), false) { - if check_newlines(&fmt_str) { - span_lint_and_then( - cx, - PRINT_WITH_NEWLINE, - mac.span(), - "using `print!()` with a format string that ends in a single newline", - |err| { - err.multipart_suggestion( - "use `println!` instead", - vec![ - (mac.path.span, String::from("println")), - (newline_span(&fmt_str), String::new()), - ], - Applicability::MachineApplicable, - ); - }, - ); - } + self.lint_print_with_newline(cx, mac); + } else if mac.path == sym!(println) { + if !is_build_script(cx) { + span_lint(cx, PRINT_STDOUT, mac.span(), "use of `println!`"); } + self.lint_println_empty_string(cx, mac); + } else if mac.path == sym!(eprint) { + span_lint(cx, PRINT_STDERR, mac.span(), "use of `eprint!`"); + self.lint_print_with_newline(cx, mac); + } else if mac.path == sym!(eprintln) { + span_lint(cx, PRINT_STDERR, mac.span(), "use of `eprintln!`"); + self.lint_println_empty_string(cx, mac); } else if mac.path == sym!(write) { if let (Some(fmt_str), _) = self.check_tts(cx, mac.args.inner_tokens(), true) { if check_newlines(&fmt_str) { @@ -487,6 +481,45 @@ impl Write { } } } + + fn lint_println_empty_string(&self, cx: &EarlyContext<'_>, mac: &MacCall) { + if let (Some(fmt_str), _) = self.check_tts(cx, mac.args.inner_tokens(), false) { + if fmt_str.symbol == Symbol::intern("") { + let name = mac.path.segments[0].ident.name; + span_lint_and_sugg( + cx, + PRINTLN_EMPTY_STRING, + mac.span(), + &format!("using `{}!(\"\")`", name), + "replace it with", + format!("{}!()", name), + Applicability::MachineApplicable, + ); + } + } + } + + fn lint_print_with_newline(&self, cx: &EarlyContext<'_>, mac: &MacCall) { + if let (Some(fmt_str), _) = self.check_tts(cx, mac.args.inner_tokens(), false) { + if check_newlines(&fmt_str) { + let name = mac.path.segments[0].ident.name; + let suggested = format!("{}ln", name); + span_lint_and_then( + cx, + PRINT_WITH_NEWLINE, + mac.span(), + &format!("using `{}!()` with a format string that ends in a single newline", name), + |err| { + err.multipart_suggestion( + &format!("use `{}!` instead", suggested), + vec![(mac.path.span, suggested), (newline_span(&fmt_str), String::new())], + Applicability::MachineApplicable, + ); + }, + ); + } + } + } } /// Checks if the format string contains a single newline that terminates it. diff --git a/src/tools/clippy/clippy_lints/src/zero_sized_map_values.rs b/src/tools/clippy/clippy_lints/src/zero_sized_map_values.rs new file mode 100644 index 0000000000000..1d5fa8d06a996 --- /dev/null +++ b/src/tools/clippy/clippy_lints/src/zero_sized_map_values.rs @@ -0,0 +1,82 @@ +use if_chain::if_chain; +use rustc_hir::{self as hir, HirId, ItemKind, Node}; +use rustc_lint::{LateContext, LateLintPass}; +use rustc_middle::ty::{Adt, Ty}; +use rustc_session::{declare_lint_pass, declare_tool_lint}; +use rustc_target::abi::LayoutOf as _; +use rustc_typeck::hir_ty_to_ty; + +use crate::utils::{is_type_diagnostic_item, match_type, paths, span_lint_and_help}; + +declare_clippy_lint! { + /// **What it does:** Checks for maps with zero-sized value types anywhere in the code. + /// + /// **Why is this bad?** Since there is only a single value for a zero-sized type, a map + /// containing zero sized values is effectively a set. Using a set in that case improves + /// readability and communicates intent more clearly. + /// + /// **Known problems:** + /// * A zero-sized type cannot be recovered later if it contains private fields. + /// * This lints the signature of public items + /// + /// **Example:** + /// + /// ```rust + /// # use std::collections::HashMap; + /// fn unique_words(text: &str) -> HashMap<&str, ()> { + /// todo!(); + /// } + /// ``` + /// Use instead: + /// ```rust + /// # use std::collections::HashSet; + /// fn unique_words(text: &str) -> HashSet<&str> { + /// todo!(); + /// } + /// ``` + pub ZERO_SIZED_MAP_VALUES, + pedantic, + "usage of map with zero-sized value type" +} + +declare_lint_pass!(ZeroSizedMapValues => [ZERO_SIZED_MAP_VALUES]); + +impl LateLintPass<'_> for ZeroSizedMapValues { + fn check_ty(&mut self, cx: &LateContext<'_>, hir_ty: &hir::Ty<'_>) { + if_chain! { + if !hir_ty.span.from_expansion(); + if !in_trait_impl(cx, hir_ty.hir_id); + let ty = ty_from_hir_ty(cx, hir_ty); + if is_type_diagnostic_item(cx, ty, sym!(hashmap_type)) || match_type(cx, ty, &paths::BTREEMAP); + if let Adt(_, ref substs) = ty.kind(); + let ty = substs.type_at(1); + if let Ok(layout) = cx.layout_of(ty); + if layout.is_zst(); + then { + span_lint_and_help(cx, ZERO_SIZED_MAP_VALUES, hir_ty.span, "map with zero-sized value type", None, "consider using a set instead"); + } + } + } +} + +fn in_trait_impl(cx: &LateContext<'_>, hir_id: HirId) -> bool { + let parent_id = cx.tcx.hir().get_parent_item(hir_id); + if let Some(Node::Item(item)) = cx.tcx.hir().find(cx.tcx.hir().get_parent_item(parent_id)) { + if let ItemKind::Impl { of_trait: Some(_), .. } = item.kind { + return true; + } + } + false +} + +fn ty_from_hir_ty<'tcx>(cx: &LateContext<'tcx>, hir_ty: &hir::Ty<'_>) -> Ty<'tcx> { + cx.maybe_typeck_results() + .and_then(|results| { + if results.hir_owner == hir_ty.hir_id.owner { + results.node_type_opt(hir_ty.hir_id) + } else { + None + } + }) + .unwrap_or_else(|| hir_ty_to_ty(cx.tcx, hir_ty)) +} diff --git a/src/tools/clippy/clippy_workspace_tests/path_dep/Cargo.toml b/src/tools/clippy/clippy_workspace_tests/path_dep/Cargo.toml new file mode 100644 index 0000000000000..85a91cd2decd5 --- /dev/null +++ b/src/tools/clippy/clippy_workspace_tests/path_dep/Cargo.toml @@ -0,0 +1,3 @@ +[package] +name = "path_dep" +version = "0.1.0" diff --git a/src/tools/clippy/clippy_workspace_tests/path_dep/src/lib.rs b/src/tools/clippy/clippy_workspace_tests/path_dep/src/lib.rs new file mode 100644 index 0000000000000..35ce524f2b10c --- /dev/null +++ b/src/tools/clippy/clippy_workspace_tests/path_dep/src/lib.rs @@ -0,0 +1,6 @@ +#![deny(clippy::empty_loop)] + +#[cfg(feature = "primary_package_test")] +pub fn lint_me() { + loop {} +} diff --git a/src/tools/clippy/clippy_workspace_tests/subcrate/Cargo.toml b/src/tools/clippy/clippy_workspace_tests/subcrate/Cargo.toml index 83ea5868160bb..45362c11b856e 100644 --- a/src/tools/clippy/clippy_workspace_tests/subcrate/Cargo.toml +++ b/src/tools/clippy/clippy_workspace_tests/subcrate/Cargo.toml @@ -1,3 +1,6 @@ [package] name = "subcrate" version = "0.1.0" + +[dependencies] +path_dep = { path = "../path_dep" } diff --git a/src/tools/clippy/doc/adding_lints.md b/src/tools/clippy/doc/adding_lints.md index b1dacfc9c6d28..60dfdb76650a1 100644 --- a/src/tools/clippy/doc/adding_lints.md +++ b/src/tools/clippy/doc/adding_lints.md @@ -98,12 +98,12 @@ 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. Once we are satisfied with the output, we need to run -`tests/ui/update-all-references.sh` to update the `.stderr` file for our lint. +`cargo dev bless` to update the `.stderr` file for our lint. Please note that, we should run `TESTNAME=foo_functions cargo uitest` -every time before running `tests/ui/update-all-references.sh`. +every time before running `cargo dev bless`. Running `TESTNAME=foo_functions cargo uitest` should pass then. When we commit our lint, we need to commit the generated `.stderr` files, too. In general, you -should only commit files changed by `tests/ui/update-all-references.sh` for the +should only commit files changed by `cargo dev bless` for the specific lint you are creating/editing. Note that if the generated files are empty, they should be removed. @@ -122,8 +122,7 @@ we will find by default two new crates, each with its manifest file: If you need more cases, you can copy one of those crates (under `foo_categories`) and rename it. The process of generating the `.stderr` file is the same, and prepending the `TESTNAME` -variable to `cargo uitest` works too, but the script to update the references -is in another path: `tests/ui-cargo/update-all-references.sh`. +variable to `cargo uitest` works too. ## Rustfix tests @@ -133,7 +132,7 @@ 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 +Use `cargo dev bless` to automatically generate the `.fixed` file after running the tests. [rustfix]: https://github.com/rust-lang/rustfix @@ -226,13 +225,13 @@ store.register_early_pass(|| box foo_functions::FooFunctions); ``` As one may expect, there is a corresponding `register_late_pass` method -available as well. Without a call to one of `register_early_pass` or +available as well. Without a call to one of `register_early_pass` or `register_late_pass`, the lint pass in question will not be run. -One reason that `cargo dev` does not automate this step is that multiple lints +One reason that `cargo dev` does not automate this step is that multiple lints can use the same lint pass, so registering the lint pass may already be done when adding a new lint. Another reason that this step is not automated is that -the order that the passes are registered determines the order the passes +the order that the passes are registered determines the order the passes actually run, which in turn affects the order that any emitted lints are output in. @@ -368,7 +367,7 @@ fn is_foo_fn(fn_kind: FnKind<'_>) -> bool { Now we should also run the full test suite with `cargo test`. At this point running `cargo test` should produce the expected output. Remember to run -`tests/ui/update-all-references.sh` to update the `.stderr` file. +`cargo dev bless` to update the `.stderr` file. `cargo test` (as opposed to `cargo uitest`) will also ensure that our lint implementation is not violating any Clippy lints itself. @@ -380,6 +379,57 @@ pass. [`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 +## Specifying the lint's minimum supported Rust version (msrv) + +Projects supporting older versions of Rust would need to disable a lint if it targets features +present in later versions. Support for this can be added by specifying an msrv in your lint like so, + +```rust +const MANUAL_STRIP_MSRV: RustcVersion = RustcVersion::new(1, 45, 0); +``` + +The project's msrv will also have to be an attribute in the lint so you'll have to add a struct +and constructor for your lint. The project's msrv needs to be passed when the lint is registered +in `lib.rs` + +```rust +pub struct ManualStrip { + msrv: Option, +} + +impl ManualStrip { + #[must_use] + pub fn new(msrv: Option) -> Self { + Self { msrv } + } +} +``` + +The project's msrv can then be matched against the lint's msrv in the LintPass using the `meets_msrv` utility +function. + +``` rust +if !meets_msrv(self.msrv.as_ref(), &MANUAL_STRIP_MSRV) { + return; +} +``` + +The project's msrv can also be specified as an inner attribute, which overrides the value from +`clippy.toml`. This can be accounted for using the `extract_msrv_attr!(LintContext)` macro and passing +LateContext/EarlyContext. + +```rust +impl<'tcx> LateLintPass<'tcx> for ManualStrip { + fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) { + ... + } + extract_msrv_attr!(LateContext); +} +``` + +Once the msrv is added to the lint, a relevant test case should be added to `tests/ui/min_rust_version_attr.rs` +which verifies that the lint isn't emitted if the project's msrv is lower. + ## Author lint If you have trouble implementing your lint, there is also the internal `author` diff --git a/src/tools/clippy/doc/basics.md b/src/tools/clippy/doc/basics.md index f25edb793e26d..954474a17aa8e 100644 --- a/src/tools/clippy/doc/basics.md +++ b/src/tools/clippy/doc/basics.md @@ -1,16 +1,14 @@ # Basics for hacking on Clippy This document explains the basics for hacking on Clippy. Besides others, this -includes how to set-up the development environment, how to build and how to test -Clippy. For a more in depth description on the codebase take a look at [Adding -Lints] or [Common Tools]. +includes how to build and test Clippy. For a more in depth description on +the codebase take a look at [Adding Lints] or [Common Tools]. [Adding Lints]: https://github.com/rust-lang/rust-clippy/blob/master/doc/adding_lints.md [Common Tools]: https://github.com/rust-lang/rust-clippy/blob/master/doc/common_tools_writing_lints.md - [Basics for hacking on Clippy](#basics-for-hacking-on-clippy) - - [Get the code](#get-the-code) - - [Setup](#setup) + - [Get the Code](#get-the-code) - [Building and Testing](#building-and-testing) - [`cargo dev`](#cargo-dev) - [PR](#pr) @@ -38,29 +36,9 @@ git rebase upstream/master git push ``` -## Setup - -Next we need to setup the toolchain to compile Clippy. Since Clippy heavily -relies on compiler internals it is build with the latest rustc master. To get -this toolchain, you can just use the `setup-toolchain.sh` script or use -`rustup-toolchain-install-master`: - -```bash -bash setup-toolchain.sh -# OR -cargo install rustup-toolchain-install-master -# For better IDE integration also add `-c rustfmt -c rust-src` (optional) -rustup-toolchain-install-master -f -n master -c rustc-dev -c llvm-tools -rustup override set master -``` - -_Note:_ Sometimes you may get compiler errors when building Clippy, even if you -didn't change anything. Normally those will be fixed by a maintainer in a few hours. - ## Building and Testing -Once the `master` toolchain is installed, you can build and test Clippy like -every other Rust project: +You can build and test Clippy like every other Rust project: ```bash cargo build # builds Clippy @@ -83,7 +61,7 @@ If the output of a [UI test] differs from the expected output, you can update th reference file with: ```bash -sh tests/ui/update-all-references.sh +cargo dev bless ``` For example, this is necessary, if you fix a typo in an error message of a lint @@ -109,7 +87,7 @@ cargo dev update_lints # create a new lint and register it cargo dev new_lint # (experimental) Setup Clippy to work with rust-analyzer -cargo dev ra-setup +cargo dev ra_setup ``` ## PR diff --git a/src/tools/clippy/rust-toolchain b/src/tools/clippy/rust-toolchain index bf867e0ae5b6c..d2e84132f4ed9 100644 --- a/src/tools/clippy/rust-toolchain +++ b/src/tools/clippy/rust-toolchain @@ -1 +1,3 @@ -nightly +[toolchain] +channel = "nightly-2020-12-20" +components = ["llvm-tools-preview", "rustc-dev", "rust-src", "rustfmt"] diff --git a/src/tools/clippy/setup-toolchain.sh b/src/tools/clippy/setup-toolchain.sh deleted file mode 100755 index 191ea4315a6b5..0000000000000 --- a/src/tools/clippy/setup-toolchain.sh +++ /dev/null @@ -1,36 +0,0 @@ -#!/usr/bin/env bash -# Set up the appropriate rustc toolchain - -set -e - -cd "$(dirname "$0")" - -RTIM_PATH=$(command -v rustup-toolchain-install-master) || INSTALLED=false -CARGO_HOME=${CARGO_HOME:-$HOME/.cargo} - -# Check if RTIM is not installed or installed in other locations not in ~/.cargo/bin -if [[ "$INSTALLED" == false || "$RTIM_PATH" == $CARGO_HOME/bin/rustup-toolchain-install-master ]]; then - cargo +nightly install rustup-toolchain-install-master -else - VERSION=$(rustup-toolchain-install-master -V | grep -o "[0-9.]*") - REMOTE=$(cargo +nightly search rustup-toolchain-install-master | grep -o "[0-9.]*") - echo "info: skipping updating rustup-toolchain-install-master at $RTIM_PATH" - echo " current version : $VERSION" - echo " remote version : $REMOTE" -fi - -RUST_COMMIT=$(git ls-remote https://github.com/rust-lang/rust master | awk '{print $1}') - -if rustc +master -Vv 2>/dev/null | grep -q "$RUST_COMMIT"; then - echo "info: master toolchain is up-to-date" - exit 0 -fi - -if [[ -n "$HOST_TOOLCHAIN" ]]; then - TOOLCHAIN=('--host' "$HOST_TOOLCHAIN") -else - TOOLCHAIN=() -fi - -rustup-toolchain-install-master -f -n master "${TOOLCHAIN[@]}" -c rustc-dev -c llvm-tools -- "$RUST_COMMIT" -rustup override set master diff --git a/src/tools/clippy/src/driver.rs b/src/tools/clippy/src/driver.rs index 87dd19c5d4d05..40f1b802e60e6 100644 --- a/src/tools/clippy/src/driver.rs +++ b/src/tools/clippy/src/driver.rs @@ -1,5 +1,6 @@ #![feature(rustc_private)] #![feature(once_cell)] +#![feature(bool_to_option)] #![cfg_attr(feature = "deny-warnings", deny(warnings))] // warn on lints, that are included in `rust-lang/rust`s bootstrap #![warn(rust_2018_idioms, unused_lifetimes)] @@ -19,6 +20,7 @@ use rustc_tools_util::VersionInfo; use std::borrow::Cow; use std::env; +use std::iter; use std::lazy::SyncLazy; use std::ops::Deref; use std::panic; @@ -41,26 +43,12 @@ fn arg_value<'a, T: Deref>( match arg.next().or_else(|| args.next()) { Some(v) if pred(v) => return Some(v), - _ => {} + _ => {}, } } None } -#[test] -fn test_arg_value() { - let args = &["--bar=bar", "--foobar", "123", "--foo"]; - - assert_eq!(arg_value(&[] as &[&str], "--foobar", |_| true), None); - assert_eq!(arg_value(args, "--bar", |_| false), None); - assert_eq!(arg_value(args, "--bar", |_| true), Some("bar")); - assert_eq!(arg_value(args, "--bar", |p| p == "bar"), Some("bar")); - assert_eq!(arg_value(args, "--bar", |p| p == "foo"), None); - assert_eq!(arg_value(args, "--foobar", |p| p == "foo"), None); - assert_eq!(arg_value(args, "--foobar", |p| p == "123"), Some("123")); - assert_eq!(arg_value(args, "--foo", |_| true), None); -} - struct DefaultCallbacks; impl rustc_driver::Callbacks for DefaultCallbacks {} @@ -121,12 +109,11 @@ You can use tool lints to allow or deny lints from your code, eg.: const BUG_REPORT_URL: &str = "https://github.com/rust-lang/rust-clippy/issues/new"; -static ICE_HOOK: SyncLazy) + Sync + Send + 'static>> = - SyncLazy::new(|| { - let hook = panic::take_hook(); - panic::set_hook(Box::new(|info| report_clippy_ice(info, BUG_REPORT_URL))); - hook - }); +static ICE_HOOK: SyncLazy) + Sync + Send + 'static>> = SyncLazy::new(|| { + let hook = panic::take_hook(); + panic::set_hook(Box::new(|info| report_clippy_ice(info, BUG_REPORT_URL))); + hook +}); fn report_clippy_ice(info: &panic::PanicInfo<'_>, bug_report_url: &str) { // Invoke our ICE handler, which prints the actual panic message and optionally a backtrace @@ -183,6 +170,29 @@ fn toolchain_path(home: Option, toolchain: Option) -> Option(args: &mut Vec, clippy_args: I) +where + T: AsRef, + U: AsRef + ?Sized + 'a, + I: Iterator + Clone, +{ + let args_iter = clippy_args.map(AsRef::as_ref); + let args_count = args_iter.clone().count(); + + if args_count > 0 { + if let Some(start) = args.windows(args_count).enumerate().find_map(|(current, window)| { + window + .iter() + .map(AsRef::as_ref) + .eq(args_iter.clone()) + .then_some(current) + }) { + args.drain(start..start + args_count); + } + } +} + +#[allow(clippy::too_many_lines)] pub fn main() { rustc_driver::init_rustc_env_logger(); SyncLazy::force(&ICE_HOOK); @@ -258,17 +268,14 @@ pub fn main() { // Setting RUSTC_WRAPPER causes Cargo to pass 'rustc' as the first argument. // We're invoking the compiler programmatically, so we ignore this/ - let wrapper_mode = - orig_args.get(1).map(Path::new).and_then(Path::file_stem) == Some("rustc".as_ref()); + let wrapper_mode = orig_args.get(1).map(Path::new).and_then(Path::file_stem) == Some("rustc".as_ref()); if wrapper_mode { // we still want to be able to invoke it normally though orig_args.remove(1); } - if !wrapper_mode - && (orig_args.iter().any(|a| a == "--help" || a == "-h") || orig_args.len() == 1) - { + if !wrapper_mode && (orig_args.iter().any(|a| a == "--help" || a == "-h") || orig_args.len() == 1) { display_help(); exit(0); } @@ -281,25 +288,88 @@ pub fn main() { args.extend(vec!["--sysroot".into(), sys_root]); }; - // this check ensures that dependencies are built but not linted and the final - // crate is linted but not built - let clippy_enabled = env::var("CLIPPY_TESTS").map_or(false, |val| val == "true") - || arg_value(&orig_args, "--cap-lints", |val| val == "allow").is_none(); + let clippy_args = env::var("CLIPPY_ARGS").unwrap_or_default(); + let clippy_args = clippy_args.split_whitespace(); + let no_deps = clippy_args.clone().any(|flag| flag == "--no-deps"); + + // We enable Clippy if one of the following conditions is met + // - IF Clippy is run on its test suite OR + // - IF Clippy is run on the main crate, not on deps (`!cap_lints_allow`) THEN + // - IF `--no-deps` is not set (`!no_deps`) OR + // - IF `--no-deps` is set and Clippy is run on the specified primary package + let clippy_tests_set = env::var("CLIPPY_TESTS").map_or(false, |val| val == "true"); + let cap_lints_allow = arg_value(&orig_args, "--cap-lints", |val| val == "allow").is_some(); + let in_primary_package = env::var("CARGO_PRIMARY_PACKAGE").is_ok(); + let clippy_enabled = clippy_tests_set || (!cap_lints_allow && (!no_deps || in_primary_package)); if clippy_enabled { + remove_clippy_args(&mut args, iter::once("--no-deps")); args.extend(vec!["--cfg".into(), r#"feature="cargo-clippy""#.into()]); - if let Ok(extra_args) = env::var("CLIPPY_ARGS") { - args.extend( - extra_args - .split("__CLIPPY_HACKERY__") - .filter_map(|s| if s.is_empty() { None } else { Some(s.to_string()) }), - ); - } + } else { + // Remove all flags passed through RUSTFLAGS if Clippy is not enabled. + remove_clippy_args(&mut args, clippy_args); } + let mut clippy = ClippyCallbacks; let mut default = DefaultCallbacks; let callbacks: &mut (dyn rustc_driver::Callbacks + Send) = if clippy_enabled { &mut clippy } else { &mut default }; + rustc_driver::RunCompiler::new(&args, callbacks).run() })) } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_arg_value() { + let args = &["--bar=bar", "--foobar", "123", "--foo"]; + + assert_eq!(arg_value(&[] as &[&str], "--foobar", |_| true), None); + assert_eq!(arg_value(args, "--bar", |_| false), None); + assert_eq!(arg_value(args, "--bar", |_| true), Some("bar")); + assert_eq!(arg_value(args, "--bar", |p| p == "bar"), Some("bar")); + assert_eq!(arg_value(args, "--bar", |p| p == "foo"), None); + assert_eq!(arg_value(args, "--foobar", |p| p == "foo"), None); + assert_eq!(arg_value(args, "--foobar", |p| p == "123"), Some("123")); + assert_eq!(arg_value(args, "--foo", |_| true), None); + } + + #[test] + fn removes_clippy_args_from_start() { + let mut args = vec!["-D", "clippy::await_holding_lock", "--cfg", r#"feature="some_feat""#]; + let clippy_args = ["-D", "clippy::await_holding_lock"].iter(); + + remove_clippy_args(&mut args, clippy_args); + assert_eq!(args, &["--cfg", r#"feature="some_feat""#]); + } + + #[test] + fn removes_clippy_args_from_end() { + let mut args = vec!["-Zui-testing", "-A", "clippy::empty_loop", "--no-deps"]; + let clippy_args = ["-A", "clippy::empty_loop", "--no-deps"].iter(); + + remove_clippy_args(&mut args, clippy_args); + assert_eq!(args, &["-Zui-testing"]); + } + + #[test] + fn removes_clippy_args_from_middle() { + let mut args = vec!["-Zui-testing", "-W", "clippy::filter_map", "-L", "serde"]; + let clippy_args = ["-W", "clippy::filter_map"].iter(); + + remove_clippy_args(&mut args, clippy_args); + assert_eq!(args, &["-Zui-testing", "-L", "serde"]); + } + + #[test] + fn no_clippy_args_to_remove() { + let mut args = vec!["-Zui-testing", "-L", "serde"]; + let clippy_args: [&str; 0] = []; + + remove_clippy_args(&mut args, clippy_args.iter()); + assert_eq!(args, &["-Zui-testing", "-L", "serde"]); + } +} diff --git a/src/tools/clippy/src/main.rs b/src/tools/clippy/src/main.rs index 6739a4cf2245e..1c0e04689a9fe 100644 --- a/src/tools/clippy/src/main.rs +++ b/src/tools/clippy/src/main.rs @@ -1,3 +1,5 @@ +#![feature(bool_to_option)] +#![feature(command_access)] #![cfg_attr(feature = "deny-warnings", deny(warnings))] // warn on lints, that are included in `rust-lang/rust`s bootstrap #![warn(rust_2018_idioms, unused_lifetimes)] @@ -62,7 +64,7 @@ struct ClippyCmd { unstable_options: bool, cargo_subcommand: &'static str, args: Vec, - clippy_args: String, + clippy_args: Option, } impl ClippyCmd { @@ -99,13 +101,17 @@ impl ClippyCmd { args.insert(0, "+nightly".to_string()); } - let clippy_args: String = old_args.map(|arg| format!("{}__CLIPPY_HACKERY__", arg)).collect(); + let mut clippy_args = old_args.collect::>().join(" "); + if cargo_subcommand == "fix" && !clippy_args.contains("--no-deps") { + clippy_args = format!("{} --no-deps", clippy_args); + } + let has_args = !clippy_args.is_empty(); ClippyCmd { unstable_options, cargo_subcommand, args, - clippy_args, + clippy_args: has_args.then_some(clippy_args), } } @@ -145,15 +151,24 @@ impl ClippyCmd { .map(|p| ("CARGO_TARGET_DIR", p)) } - fn into_std_cmd(self) -> Command { + fn into_std_cmd(self, rustflags: Option) -> Command { let mut cmd = Command::new("cargo"); cmd.env(self.path_env(), Self::path()) .envs(ClippyCmd::target_dir()) - .env("CLIPPY_ARGS", self.clippy_args) .arg(self.cargo_subcommand) .args(&self.args); + // HACK: pass Clippy args to the driver *also* through RUSTFLAGS. + // This guarantees that new builds will be triggered when Clippy flags change. + if let Some(clippy_args) = self.clippy_args { + cmd.env( + "RUSTFLAGS", + rustflags.map_or(clippy_args.clone(), |flags| format!("{} {}", clippy_args, flags)), + ); + cmd.env("CLIPPY_ARGS", clippy_args); + } + cmd } } @@ -164,7 +179,7 @@ where { let cmd = ClippyCmd::new(old_args); - let mut cmd = cmd.into_std_cmd(); + let mut cmd = cmd.into_std_cmd(env::var("RUSTFLAGS").ok()); let exit_status = cmd .spawn() @@ -182,6 +197,7 @@ where #[cfg(test)] mod tests { use super::ClippyCmd; + use std::ffi::OsStr; #[test] #[should_panic] @@ -196,15 +212,37 @@ mod tests { .split_whitespace() .map(ToString::to_string); let cmd = ClippyCmd::new(args); + assert_eq!("fix", cmd.cargo_subcommand); assert_eq!("RUSTC_WORKSPACE_WRAPPER", cmd.path_env()); assert!(cmd.args.iter().any(|arg| arg.ends_with("unstable-options"))); } + #[test] + fn fix_implies_no_deps() { + let args = "cargo clippy --fix -Zunstable-options" + .split_whitespace() + .map(ToString::to_string); + let cmd = ClippyCmd::new(args); + + assert!(cmd.clippy_args.unwrap().contains("--no-deps")); + } + + #[test] + fn no_deps_not_duplicated_with_fix() { + let args = "cargo clippy --fix -Zunstable-options -- --no-deps" + .split_whitespace() + .map(ToString::to_string); + let cmd = ClippyCmd::new(args); + + assert_eq!(1, cmd.clippy_args.unwrap().matches("--no-deps").count()); + } + #[test] fn check() { let args = "cargo clippy".split_whitespace().map(ToString::to_string); let cmd = ClippyCmd::new(args); + assert_eq!("check", cmd.cargo_subcommand); assert_eq!("RUSTC_WRAPPER", cmd.path_env()); } @@ -215,7 +253,63 @@ mod tests { .split_whitespace() .map(ToString::to_string); let cmd = ClippyCmd::new(args); + assert_eq!("check", cmd.cargo_subcommand); assert_eq!("RUSTC_WORKSPACE_WRAPPER", cmd.path_env()); } + + #[test] + fn clippy_args_into_rustflags() { + let args = "cargo clippy -- -W clippy::as_conversions" + .split_whitespace() + .map(ToString::to_string); + let cmd = ClippyCmd::new(args); + + let rustflags = None; + let cmd = cmd.into_std_cmd(rustflags); + + assert!(cmd + .get_envs() + .any(|(key, val)| key == "RUSTFLAGS" && val == Some(OsStr::new("-W clippy::as_conversions")))); + } + + #[test] + fn clippy_args_respect_existing_rustflags() { + let args = "cargo clippy -- -D clippy::await_holding_lock" + .split_whitespace() + .map(ToString::to_string); + let cmd = ClippyCmd::new(args); + + let rustflags = Some(r#"--cfg feature="some_feat""#.into()); + let cmd = cmd.into_std_cmd(rustflags); + + assert!(cmd.get_envs().any(|(key, val)| key == "RUSTFLAGS" + && val == Some(OsStr::new(r#"-D clippy::await_holding_lock --cfg feature="some_feat""#)))); + } + + #[test] + fn no_env_change_if_no_clippy_args() { + let args = "cargo clippy".split_whitespace().map(ToString::to_string); + let cmd = ClippyCmd::new(args); + + let rustflags = Some(r#"--cfg feature="some_feat""#.into()); + let cmd = cmd.into_std_cmd(rustflags); + + assert!(!cmd + .get_envs() + .any(|(key, _)| key == "RUSTFLAGS" || key == "CLIPPY_ARGS")); + } + + #[test] + fn no_env_change_if_no_clippy_args_nor_rustflags() { + let args = "cargo clippy".split_whitespace().map(ToString::to_string); + let cmd = ClippyCmd::new(args); + + let rustflags = None; + let cmd = cmd.into_std_cmd(rustflags); + + assert!(!cmd + .get_envs() + .any(|(key, _)| key == "RUSTFLAGS" || key == "CLIPPY_ARGS")) + } } diff --git a/src/tools/clippy/tests/dogfood.rs b/src/tools/clippy/tests/dogfood.rs index a6163a83d7687..fda1413868e82 100644 --- a/src/tools/clippy/tests/dogfood.rs +++ b/src/tools/clippy/tests/dogfood.rs @@ -3,7 +3,7 @@ #![feature(once_cell)] use std::lazy::SyncLazy; -use std::path::PathBuf; +use std::path::{Path, PathBuf}; use std::process::Command; mod cargo; @@ -23,7 +23,7 @@ fn dogfood_clippy() { .current_dir(root_dir) .env("CLIPPY_DOGFOOD", "1") .env("CARGO_INCREMENTAL", "0") - .arg("clippy-preview") + .arg("clippy") .arg("--all-targets") .arg("--all-features") .arg("--") @@ -47,12 +47,77 @@ fn dogfood_clippy() { #[test] fn dogfood_subprojects() { + fn test_no_deps_ignores_path_deps_in_workspaces() { + fn clean(cwd: &Path, target_dir: &Path) { + Command::new("cargo") + .current_dir(cwd) + .env("CARGO_TARGET_DIR", target_dir) + .arg("clean") + .args(&["-p", "subcrate"]) + .args(&["-p", "path_dep"]) + .output() + .unwrap(); + } + + if cargo::is_rustc_test_suite() { + return; + } + let root = PathBuf::from(env!("CARGO_MANIFEST_DIR")); + let target_dir = root.join("target").join("dogfood"); + let cwd = root.join("clippy_workspace_tests"); + + // Make sure we start with a clean state + clean(&cwd, &target_dir); + + // `path_dep` is a path dependency of `subcrate` that would trigger a denied lint. + // Make sure that with the `--no-deps` argument Clippy does not run on `path_dep`. + let output = Command::new(&*CLIPPY_PATH) + .current_dir(&cwd) + .env("CLIPPY_DOGFOOD", "1") + .env("CARGO_INCREMENTAL", "0") + .arg("clippy") + .args(&["-p", "subcrate"]) + .arg("--") + .arg("--no-deps") + .arg("-Cdebuginfo=0") // disable debuginfo to generate less data in the target dir + .args(&["--cfg", r#"feature="primary_package_test""#]) + .output() + .unwrap(); + println!("status: {}", output.status); + println!("stdout: {}", String::from_utf8_lossy(&output.stdout)); + println!("stderr: {}", String::from_utf8_lossy(&output.stderr)); + + assert!(output.status.success()); + + // Make sure we start with a clean state + clean(&cwd, &target_dir); + + // Test that without the `--no-deps` argument, `path_dep` is linted. + let output = Command::new(&*CLIPPY_PATH) + .current_dir(&cwd) + .env("CLIPPY_DOGFOOD", "1") + .env("CARGO_INCREMENTAL", "0") + .arg("clippy") + .args(&["-p", "subcrate"]) + .arg("--") + .arg("-Cdebuginfo=0") // disable debuginfo to generate less data in the target dir + .args(&["--cfg", r#"feature="primary_package_test""#]) + .output() + .unwrap(); + println!("status: {}", output.status); + println!("stdout: {}", String::from_utf8_lossy(&output.stdout)); + println!("stderr: {}", String::from_utf8_lossy(&output.stderr)); + + assert!(!output.status.success()); + } + // run clippy on remaining subprojects and fail the test if lint warnings are reported if cargo::is_rustc_test_suite() { return; } let root_dir = PathBuf::from(env!("CARGO_MANIFEST_DIR")); + // NOTE: `path_dep` crate is omitted on purpose here for d in &[ "clippy_workspace_tests", "clippy_workspace_tests/src", @@ -78,4 +143,8 @@ fn dogfood_subprojects() { assert!(output.status.success()); } + + // NOTE: Since tests run in parallel we can't run cargo commands on the same workspace at the + // same time, so we test this immediately after the dogfood for workspaces. + test_no_deps_ignores_path_deps_in_workspaces(); } diff --git a/src/tools/clippy/tests/integration.rs b/src/tools/clippy/tests/integration.rs index a78273ce0da41..1718089e8bd27 100644 --- a/src/tools/clippy/tests/integration.rs +++ b/src/tools/clippy/tests/integration.rs @@ -72,6 +72,8 @@ fn integration_test() { panic!("incompatible crate versions"); } else if stderr.contains("failed to run `rustc` to learn about target-specific information") { panic!("couldn't find librustc_driver, consider setting `LD_LIBRARY_PATH`"); + } else if stderr.contains("toolchain") && stderr.contains("is not installed") { + panic!("missing required toolchain"); } match output.status.code() { diff --git a/src/tools/clippy/tests/ui-cargo/update-all-references.sh b/src/tools/clippy/tests/ui-cargo/update-all-references.sh index 7028b251ea030..4391499a1e1f8 100755 --- a/src/tools/clippy/tests/ui-cargo/update-all-references.sh +++ b/src/tools/clippy/tests/ui-cargo/update-all-references.sh @@ -1,18 +1,3 @@ #!/bin/bash -# -# A script to update the references for all tests. The idea is that -# you do a run, which will generate files in the build directory -# containing the (normalized) actual output of the compiler. You then -# run this script, which will copy those files over. If you find -# yourself manually editing a foo.stderr file, you're doing it wrong. -# -# See all `update-references.sh`, if you just want to update a single test. -if [[ "$1" == "--help" || "$1" == "-h" ]]; then - echo "usage: $0" -fi - -BUILD_DIR=$PWD/target/debug/test_build_base -MY_DIR=$(dirname "$0") -cd "$MY_DIR" || exit -find . -name '*.rs' -exec ./update-references.sh "$BUILD_DIR" {} + +echo "Please use 'cargo dev bless' instead." diff --git a/src/tools/clippy/tests/ui-cargo/update-references.sh b/src/tools/clippy/tests/ui-cargo/update-references.sh deleted file mode 100755 index 2ab51168bcaa6..0000000000000 --- a/src/tools/clippy/tests/ui-cargo/update-references.sh +++ /dev/null @@ -1,46 +0,0 @@ -#!/bin/bash - -# A script to update the references for particular tests. The idea is -# that you do a run, which will generate files in the build directory -# containing the (normalized) actual output of the compiler. This -# script will then copy that output and replace the "expected output" -# files. You can then commit the changes. -# -# If you find yourself manually editing a foo.stderr file, you're -# doing it wrong. - -if [[ "$1" == "--help" || "$1" == "-h" || "$1" == "" || "$2" == "" ]]; then - echo "usage: $0 " - echo "" - echo "For example:" - echo " $0 ../../../build/x86_64-apple-darwin/test/ui *.rs */*.rs" -fi - -MYDIR=$(dirname "$0") - -BUILD_DIR="$1" -shift - -while [[ "$1" != "" ]]; do - STDERR_NAME="${1/%.rs/.stderr}" - STDOUT_NAME="${1/%.rs/.stdout}" - shift - if [[ -f "$BUILD_DIR"/"$STDOUT_NAME" ]] && \ - ! (cmp -s -- "$BUILD_DIR"/"$STDOUT_NAME" "$MYDIR"/"$STDOUT_NAME"); then - echo updating "$MYDIR"/"$STDOUT_NAME" - cp "$BUILD_DIR"/"$STDOUT_NAME" "$MYDIR"/"$STDOUT_NAME" - if [[ ! -s "$MYDIR"/"$STDOUT_NAME" ]]; then - echo removing "$MYDIR"/"$STDOUT_NAME" - rm "$MYDIR"/"$STDOUT_NAME" - fi - fi - if [[ -f "$BUILD_DIR"/"$STDERR_NAME" ]] && \ - ! (cmp -s -- "$BUILD_DIR"/"$STDERR_NAME" "$MYDIR"/"$STDERR_NAME"); then - echo updating "$MYDIR"/"$STDERR_NAME" - cp "$BUILD_DIR"/"$STDERR_NAME" "$MYDIR"/"$STDERR_NAME" - if [[ ! -s "$MYDIR"/"$STDERR_NAME" ]]; then - echo removing "$MYDIR"/"$STDERR_NAME" - rm "$MYDIR"/"$STDERR_NAME" - fi - fi -done diff --git a/src/tools/clippy/tests/ui-internal/interning_defined_symbol.fixed b/src/tools/clippy/tests/ui-internal/interning_defined_symbol.fixed new file mode 100644 index 0000000000000..c6b84d2ef650b --- /dev/null +++ b/src/tools/clippy/tests/ui-internal/interning_defined_symbol.fixed @@ -0,0 +1,33 @@ +// run-rustfix +#![deny(clippy::internal)] +#![feature(rustc_private)] + +extern crate rustc_span; + +use rustc_span::symbol::Symbol; + +macro_rules! sym { + ($tt:tt) => { + rustc_span::symbol::Symbol::intern(stringify!($tt)) + }; +} + +fn main() { + // Direct use of Symbol::intern + let _ = rustc_span::symbol::sym::f32; + + // Using a sym macro + let _ = rustc_span::symbol::sym::f32; + + // Correct suggestion when symbol isn't stringified constant name + let _ = rustc_span::symbol::sym::proc_dash_macro; + + // Interning a symbol that is not defined + let _ = Symbol::intern("xyz123"); + let _ = sym!(xyz123); + + // Using a different `intern` function + let _ = intern("f32"); +} + +fn intern(_: &str) {} diff --git a/src/tools/clippy/tests/ui-internal/interning_defined_symbol.rs b/src/tools/clippy/tests/ui-internal/interning_defined_symbol.rs new file mode 100644 index 0000000000000..9ec82d4ad0bae --- /dev/null +++ b/src/tools/clippy/tests/ui-internal/interning_defined_symbol.rs @@ -0,0 +1,33 @@ +// run-rustfix +#![deny(clippy::internal)] +#![feature(rustc_private)] + +extern crate rustc_span; + +use rustc_span::symbol::Symbol; + +macro_rules! sym { + ($tt:tt) => { + rustc_span::symbol::Symbol::intern(stringify!($tt)) + }; +} + +fn main() { + // Direct use of Symbol::intern + let _ = Symbol::intern("f32"); + + // Using a sym macro + let _ = sym!(f32); + + // Correct suggestion when symbol isn't stringified constant name + let _ = Symbol::intern("proc-macro"); + + // Interning a symbol that is not defined + let _ = Symbol::intern("xyz123"); + let _ = sym!(xyz123); + + // Using a different `intern` function + let _ = intern("f32"); +} + +fn intern(_: &str) {} diff --git a/src/tools/clippy/tests/ui-internal/interning_defined_symbol.stderr b/src/tools/clippy/tests/ui-internal/interning_defined_symbol.stderr new file mode 100644 index 0000000000000..74b906c8a5797 --- /dev/null +++ b/src/tools/clippy/tests/ui-internal/interning_defined_symbol.stderr @@ -0,0 +1,27 @@ +error: interning a defined symbol + --> $DIR/interning_defined_symbol.rs:17:13 + | +LL | let _ = Symbol::intern("f32"); + | ^^^^^^^^^^^^^^^^^^^^^ help: try: `rustc_span::symbol::sym::f32` + | +note: the lint level is defined here + --> $DIR/interning_defined_symbol.rs:2:9 + | +LL | #![deny(clippy::internal)] + | ^^^^^^^^^^^^^^^^ + = note: `#[deny(clippy::interning_defined_symbol)]` implied by `#[deny(clippy::internal)]` + +error: interning a defined symbol + --> $DIR/interning_defined_symbol.rs:20:13 + | +LL | let _ = sym!(f32); + | ^^^^^^^^^ help: try: `rustc_span::symbol::sym::f32` + +error: interning a defined symbol + --> $DIR/interning_defined_symbol.rs:23:13 + | +LL | let _ = Symbol::intern("proc-macro"); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `rustc_span::symbol::sym::proc_dash_macro` + +error: aborting due to 3 previous errors + diff --git a/src/tools/clippy/tests/ui-toml/update-all-references.sh b/src/tools/clippy/tests/ui-toml/update-all-references.sh index 7028b251ea030..4391499a1e1f8 100755 --- a/src/tools/clippy/tests/ui-toml/update-all-references.sh +++ b/src/tools/clippy/tests/ui-toml/update-all-references.sh @@ -1,18 +1,3 @@ #!/bin/bash -# -# A script to update the references for all tests. The idea is that -# you do a run, which will generate files in the build directory -# containing the (normalized) actual output of the compiler. You then -# run this script, which will copy those files over. If you find -# yourself manually editing a foo.stderr file, you're doing it wrong. -# -# See all `update-references.sh`, if you just want to update a single test. -if [[ "$1" == "--help" || "$1" == "-h" ]]; then - echo "usage: $0" -fi - -BUILD_DIR=$PWD/target/debug/test_build_base -MY_DIR=$(dirname "$0") -cd "$MY_DIR" || exit -find . -name '*.rs' -exec ./update-references.sh "$BUILD_DIR" {} + +echo "Please use 'cargo dev bless' instead." diff --git a/src/tools/clippy/tests/ui-toml/update-references.sh b/src/tools/clippy/tests/ui-toml/update-references.sh deleted file mode 100755 index 2ab51168bcaa6..0000000000000 --- a/src/tools/clippy/tests/ui-toml/update-references.sh +++ /dev/null @@ -1,46 +0,0 @@ -#!/bin/bash - -# A script to update the references for particular tests. The idea is -# that you do a run, which will generate files in the build directory -# containing the (normalized) actual output of the compiler. This -# script will then copy that output and replace the "expected output" -# files. You can then commit the changes. -# -# If you find yourself manually editing a foo.stderr file, you're -# doing it wrong. - -if [[ "$1" == "--help" || "$1" == "-h" || "$1" == "" || "$2" == "" ]]; then - echo "usage: $0 " - echo "" - echo "For example:" - echo " $0 ../../../build/x86_64-apple-darwin/test/ui *.rs */*.rs" -fi - -MYDIR=$(dirname "$0") - -BUILD_DIR="$1" -shift - -while [[ "$1" != "" ]]; do - STDERR_NAME="${1/%.rs/.stderr}" - STDOUT_NAME="${1/%.rs/.stdout}" - shift - if [[ -f "$BUILD_DIR"/"$STDOUT_NAME" ]] && \ - ! (cmp -s -- "$BUILD_DIR"/"$STDOUT_NAME" "$MYDIR"/"$STDOUT_NAME"); then - echo updating "$MYDIR"/"$STDOUT_NAME" - cp "$BUILD_DIR"/"$STDOUT_NAME" "$MYDIR"/"$STDOUT_NAME" - if [[ ! -s "$MYDIR"/"$STDOUT_NAME" ]]; then - echo removing "$MYDIR"/"$STDOUT_NAME" - rm "$MYDIR"/"$STDOUT_NAME" - fi - fi - if [[ -f "$BUILD_DIR"/"$STDERR_NAME" ]] && \ - ! (cmp -s -- "$BUILD_DIR"/"$STDERR_NAME" "$MYDIR"/"$STDERR_NAME"); then - echo updating "$MYDIR"/"$STDERR_NAME" - cp "$BUILD_DIR"/"$STDERR_NAME" "$MYDIR"/"$STDERR_NAME" - if [[ ! -s "$MYDIR"/"$STDERR_NAME" ]]; then - echo removing "$MYDIR"/"$STDERR_NAME" - rm "$MYDIR"/"$STDERR_NAME" - fi - fi -done diff --git a/src/tools/clippy/tests/ui/clone_on_copy.stderr b/src/tools/clippy/tests/ui/clone_on_copy.stderr index ec2faf4ab40d2..14a700886a7bc 100644 --- a/src/tools/clippy/tests/ui/clone_on_copy.stderr +++ b/src/tools/clippy/tests/ui/clone_on_copy.stderr @@ -1,4 +1,4 @@ -error: using `clone` on a `Copy` type +error: using `clone` on type `i32` which implements the `Copy` trait --> $DIR/clone_on_copy.rs:22:5 | LL | 42.clone(); @@ -6,25 +6,25 @@ LL | 42.clone(); | = note: `-D clippy::clone-on-copy` implied by `-D warnings` -error: using `clone` on a `Copy` type +error: using `clone` on type `i32` which implements the `Copy` trait --> $DIR/clone_on_copy.rs:26:5 | LL | (&42).clone(); | ^^^^^^^^^^^^^ help: try dereferencing it: `*(&42)` -error: using `clone` on a `Copy` type +error: using `clone` on type `i32` which implements the `Copy` trait --> $DIR/clone_on_copy.rs:29:5 | LL | rc.borrow().clone(); | ^^^^^^^^^^^^^^^^^^^ help: try dereferencing it: `*rc.borrow()` -error: using `clone` on a `Copy` type +error: using `clone` on type `char` which implements the `Copy` trait --> $DIR/clone_on_copy.rs:35:14 | LL | is_ascii('z'.clone()); | ^^^^^^^^^^^ help: try removing the `clone` call: `'z'` -error: using `clone` on a `Copy` type +error: using `clone` on type `i32` which implements the `Copy` trait --> $DIR/clone_on_copy.rs:39:14 | LL | vec.push(42.clone()); diff --git a/src/tools/clippy/tests/ui/eprint_with_newline.rs b/src/tools/clippy/tests/ui/eprint_with_newline.rs new file mode 100644 index 0000000000000..8df32649ad948 --- /dev/null +++ b/src/tools/clippy/tests/ui/eprint_with_newline.rs @@ -0,0 +1,49 @@ +#![allow(clippy::print_literal)] +#![warn(clippy::print_with_newline)] + +fn main() { + eprint!("Hello\n"); + eprint!("Hello {}\n", "world"); + eprint!("Hello {} {}\n", "world", "#2"); + eprint!("{}\n", 1265); + eprint!("\n"); + + // these are all fine + eprint!(""); + eprint!("Hello"); + eprintln!("Hello"); + eprintln!("Hello\n"); + eprintln!("Hello {}\n", "world"); + eprint!("Issue\n{}", 1265); + eprint!("{}", 1265); + eprint!("\n{}", 1275); + eprint!("\n\n"); + eprint!("like eof\n\n"); + eprint!("Hello {} {}\n\n", "world", "#2"); + eprintln!("\ndon't\nwarn\nfor\nmultiple\nnewlines\n"); // #3126 + eprintln!("\nbla\n\n"); // #3126 + + // Escaping + eprint!("\\n"); // #3514 + eprint!("\\\n"); // should fail + eprint!("\\\\n"); + + // Raw strings + eprint!(r"\n"); // #3778 + + // Literal newlines should also fail + eprint!( + " +" + ); + eprint!( + r" +" + ); + + // Don't warn on CRLF (#4208) + eprint!("\r\n"); + eprint!("foo\r\n"); + eprint!("\\r\n"); //~ ERROR + eprint!("foo\rbar\n") // ~ ERROR +} diff --git a/src/tools/clippy/tests/ui/eprint_with_newline.stderr b/src/tools/clippy/tests/ui/eprint_with_newline.stderr new file mode 100644 index 0000000000000..31811d1d92a04 --- /dev/null +++ b/src/tools/clippy/tests/ui/eprint_with_newline.stderr @@ -0,0 +1,121 @@ +error: using `eprint!()` with a format string that ends in a single newline + --> $DIR/eprint_with_newline.rs:5:5 + | +LL | eprint!("Hello/n"); + | ^^^^^^^^^^^^^^^^^^ + | + = note: `-D clippy::print-with-newline` implied by `-D warnings` +help: use `eprintln!` instead + | +LL | eprintln!("Hello"); + | ^^^^^^^^ -- + +error: using `eprint!()` with a format string that ends in a single newline + --> $DIR/eprint_with_newline.rs:6:5 + | +LL | eprint!("Hello {}/n", "world"); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: use `eprintln!` instead + | +LL | eprintln!("Hello {}", "world"); + | ^^^^^^^^ -- + +error: using `eprint!()` with a format string that ends in a single newline + --> $DIR/eprint_with_newline.rs:7:5 + | +LL | eprint!("Hello {} {}/n", "world", "#2"); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: use `eprintln!` instead + | +LL | eprintln!("Hello {} {}", "world", "#2"); + | ^^^^^^^^ -- + +error: using `eprint!()` with a format string that ends in a single newline + --> $DIR/eprint_with_newline.rs:8:5 + | +LL | eprint!("{}/n", 1265); + | ^^^^^^^^^^^^^^^^^^^^^ + | +help: use `eprintln!` instead + | +LL | eprintln!("{}", 1265); + | ^^^^^^^^ -- + +error: using `eprint!()` with a format string that ends in a single newline + --> $DIR/eprint_with_newline.rs:9:5 + | +LL | eprint!("/n"); + | ^^^^^^^^^^^^^ + | +help: use `eprintln!` instead + | +LL | eprintln!(); + | ^^^^^^^^ -- + +error: using `eprint!()` with a format string that ends in a single newline + --> $DIR/eprint_with_newline.rs:28:5 + | +LL | eprint!("//n"); // should fail + | ^^^^^^^^^^^^^^^ + | +help: use `eprintln!` instead + | +LL | eprintln!("/"); // should fail + | ^^^^^^^^ -- + +error: using `eprint!()` with a format string that ends in a single newline + --> $DIR/eprint_with_newline.rs:35:5 + | +LL | / eprint!( +LL | | " +LL | | " +LL | | ); + | |_____^ + | +help: use `eprintln!` instead + | +LL | eprintln!( +LL | "" + | + +error: using `eprint!()` with a format string that ends in a single newline + --> $DIR/eprint_with_newline.rs:39:5 + | +LL | / eprint!( +LL | | r" +LL | | " +LL | | ); + | |_____^ + | +help: use `eprintln!` instead + | +LL | eprintln!( +LL | r"" + | + +error: using `eprint!()` with a format string that ends in a single newline + --> $DIR/eprint_with_newline.rs:47:5 + | +LL | eprint!("/r/n"); //~ ERROR + | ^^^^^^^^^^^^^^^^ + | +help: use `eprintln!` instead + | +LL | eprintln!("/r"); //~ ERROR + | ^^^^^^^^ -- + +error: using `eprint!()` with a format string that ends in a single newline + --> $DIR/eprint_with_newline.rs:48:5 + | +LL | eprint!("foo/rbar/n") // ~ ERROR + | ^^^^^^^^^^^^^^^^^^^^^ + | +help: use `eprintln!` instead + | +LL | eprintln!("foo/rbar") // ~ ERROR + | ^^^^^^^^ -- + +error: aborting due to 10 previous errors + diff --git a/src/tools/clippy/tests/ui/eta.stderr b/src/tools/clippy/tests/ui/eta.stderr index c4713ca8083dd..16aa1b07733d5 100644 --- a/src/tools/clippy/tests/ui/eta.stderr +++ b/src/tools/clippy/tests/ui/eta.stderr @@ -12,7 +12,7 @@ error: redundant closure found LL | meta(|a| foo(a)); | ^^^^^^^^^^ help: remove closure as shown: `foo` -error: this expression borrows a reference that is immediately dereferenced by the compiler +error: this expression borrows a reference (`&u8`) that is immediately dereferenced by the compiler --> $DIR/eta.rs:24:21 | LL | all(&[1, 2, 3], &&2, |x, y| below(x, y)); //is adjusted diff --git a/src/tools/clippy/tests/ui/formatting.rs b/src/tools/clippy/tests/ui/formatting.rs index f54b3f2bfe28a..0d14807ff1cfc 100644 --- a/src/tools/clippy/tests/ui/formatting.rs +++ b/src/tools/clippy/tests/ui/formatting.rs @@ -10,91 +10,6 @@ fn foo() -> bool { #[rustfmt::skip] fn main() { - // weird `else` formatting: - if foo() { - } { - } - - if foo() { - } if foo() { - } - - let _ = { // if as the last expression - let _ = 0; - - if foo() { - } if foo() { - } - else { - } - }; - - let _ = { // if in the middle of a block - if foo() { - } if foo() { - } - else { - } - - let _ = 0; - }; - - if foo() { - } else - { - } - - if foo() { - } - else - { - } - - if foo() { - } else - if foo() { // the span of the above error should continue here - } - - if foo() { - } - else - if foo() { // the span of the above error should continue here - } - - // those are ok: - if foo() { - } - { - } - - if foo() { - } else { - } - - if foo() { - } - else { - } - - if foo() { - } - if foo() { - } - - if foo() { - } else if foo() { - } - - if foo() { - } - else if foo() { - } - - if foo() { - } - else if - foo() {} - // weird op_eq formatting: let mut a = 42; a =- 35; @@ -146,7 +61,7 @@ fn main() { // don't lint if the indentation suggests not to let _ = &[ - 1 + 2, 3 + 1 + 2, 3 - 4, 5 ]; // lint if it doesn't diff --git a/src/tools/clippy/tests/ui/formatting.stderr b/src/tools/clippy/tests/ui/formatting.stderr index e2095cc125bb7..bde434c7e2e78 100644 --- a/src/tools/clippy/tests/ui/formatting.stderr +++ b/src/tools/clippy/tests/ui/formatting.stderr @@ -1,80 +1,5 @@ -error: this looks like an `else {..}` but the `else` is missing - --> $DIR/formatting.rs:15:6 - | -LL | } { - | ^ - | - = note: `-D clippy::suspicious-else-formatting` implied by `-D warnings` - = note: to remove this lint, add the missing `else` or add a new line before the next block - -error: this looks like an `else if` but the `else` is missing - --> $DIR/formatting.rs:19:6 - | -LL | } if foo() { - | ^ - | - = note: to remove this lint, add the missing `else` or add a new line before the second `if` - -error: this looks like an `else if` but the `else` is missing - --> $DIR/formatting.rs:26:10 - | -LL | } if foo() { - | ^ - | - = note: to remove this lint, add the missing `else` or add a new line before the second `if` - -error: this looks like an `else if` but the `else` is missing - --> $DIR/formatting.rs:34:10 - | -LL | } if foo() { - | ^ - | - = note: to remove this lint, add the missing `else` or add a new line before the second `if` - -error: this is an `else {..}` but the formatting might hide it - --> $DIR/formatting.rs:43:6 - | -LL | } else - | ______^ -LL | | { - | |____^ - | - = note: to remove this lint, remove the `else` or remove the new line between `else` and `{..}` - -error: this is an `else {..}` but the formatting might hide it - --> $DIR/formatting.rs:48:6 - | -LL | } - | ______^ -LL | | else -LL | | { - | |____^ - | - = note: to remove this lint, remove the `else` or remove the new line between `else` and `{..}` - -error: this is an `else if` but the formatting might hide it - --> $DIR/formatting.rs:54:6 - | -LL | } else - | ______^ -LL | | if foo() { // the span of the above error should continue here - | |____^ - | - = note: to remove this lint, remove the `else` or remove the new line between `else` and `if` - -error: this is an `else if` but the formatting might hide it - --> $DIR/formatting.rs:59:6 - | -LL | } - | ______^ -LL | | else -LL | | if foo() { // the span of the above error should continue here - | |____^ - | - = note: to remove this lint, remove the `else` or remove the new line between `else` and `if` - error: this looks like you are trying to use `.. -= ..`, but you really are doing `.. = (- ..)` - --> $DIR/formatting.rs:100:6 + --> $DIR/formatting.rs:15:6 | LL | a =- 35; | ^^^^ @@ -83,7 +8,7 @@ LL | a =- 35; = note: to remove this lint, use either `-=` or `= -` error: this looks like you are trying to use `.. *= ..`, but you really are doing `.. = (* ..)` - --> $DIR/formatting.rs:101:6 + --> $DIR/formatting.rs:16:6 | LL | a =* &191; | ^^^^ @@ -91,7 +16,7 @@ LL | a =* &191; = note: to remove this lint, use either `*=` or `= *` error: this looks like you are trying to use `.. != ..`, but you really are doing `.. = (! ..)` - --> $DIR/formatting.rs:104:6 + --> $DIR/formatting.rs:19:6 | LL | b =! false; | ^^^^ @@ -99,7 +24,7 @@ LL | b =! false; = note: to remove this lint, use either `!=` or `= !` error: possibly missing a comma here - --> $DIR/formatting.rs:113:19 + --> $DIR/formatting.rs:28:19 | LL | -1, -2, -3 // <= no comma here | ^ @@ -108,7 +33,7 @@ LL | -1, -2, -3 // <= no comma here = note: to remove this lint, add a comma or write the expr in a single line error: possibly missing a comma here - --> $DIR/formatting.rs:117:19 + --> $DIR/formatting.rs:32:19 | LL | -1, -2, -3 // <= no comma here | ^ @@ -116,12 +41,12 @@ LL | -1, -2, -3 // <= no comma here = note: to remove this lint, add a comma or write the expr in a single line error: possibly missing a comma here - --> $DIR/formatting.rs:154:11 + --> $DIR/formatting.rs:69:11 | LL | -1 | ^ | = note: to remove this lint, add a comma or write the expr in a single line -error: aborting due to 14 previous errors +error: aborting due to 6 previous errors diff --git a/src/tools/clippy/tests/ui/match_single_binding.fixed b/src/tools/clippy/tests/ui/match_single_binding.fixed index f3627902eec98..526e94b10bd0e 100644 --- a/src/tools/clippy/tests/ui/match_single_binding.fixed +++ b/src/tools/clippy/tests/ui/match_single_binding.fixed @@ -87,4 +87,32 @@ fn main() { unwrapped }) .collect::>(); + // Ok + let x = 1; + match x { + #[cfg(disabled_feature)] + 0 => println!("Disabled branch"), + _ => println!("Enabled branch"), + } + // Lint + let x = 1; + let y = 1; + println!("Single branch"); + // Ok + let x = 1; + let y = 1; + match match y { + 0 => 1, + _ => 2, + } { + #[cfg(disabled_feature)] + 0 => println!("Array index start"), + _ => println!("Not an array index start"), + } + // False negative + let x = 1; + match x { + // => + _ => println!("Not an array index start"), + } } diff --git a/src/tools/clippy/tests/ui/match_single_binding.rs b/src/tools/clippy/tests/ui/match_single_binding.rs index 8c182148ae184..6a2ca7c5e9340 100644 --- a/src/tools/clippy/tests/ui/match_single_binding.rs +++ b/src/tools/clippy/tests/ui/match_single_binding.rs @@ -99,4 +99,37 @@ fn main() { unwrapped => unwrapped, }) .collect::>(); + // Ok + let x = 1; + match x { + #[cfg(disabled_feature)] + 0 => println!("Disabled branch"), + _ => println!("Enabled branch"), + } + // Lint + let x = 1; + let y = 1; + match match y { + 0 => 1, + _ => 2, + } { + _ => println!("Single branch"), + } + // Ok + let x = 1; + let y = 1; + match match y { + 0 => 1, + _ => 2, + } { + #[cfg(disabled_feature)] + 0 => println!("Array index start"), + _ => println!("Not an array index start"), + } + // False negative + let x = 1; + match x { + // => + _ => println!("Not an array index start"), + } } diff --git a/src/tools/clippy/tests/ui/match_single_binding.stderr b/src/tools/clippy/tests/ui/match_single_binding.stderr index 795c8c3e24d7e..cbbf5d29c0247 100644 --- a/src/tools/clippy/tests/ui/match_single_binding.stderr +++ b/src/tools/clippy/tests/ui/match_single_binding.stderr @@ -167,5 +167,16 @@ LL | unwrapped LL | }) | -error: aborting due to 11 previous errors +error: this match could be replaced by its body itself + --> $DIR/match_single_binding.rs:112:5 + | +LL | / match match y { +LL | | 0 => 1, +LL | | _ => 2, +LL | | } { +LL | | _ => println!("Single branch"), +LL | | } + | |_____^ help: consider using the match body instead: `println!("Single branch");` + +error: aborting due to 12 previous errors diff --git a/src/tools/clippy/tests/ui/min_rust_version_attr.rs b/src/tools/clippy/tests/ui/min_rust_version_attr.rs index 1026cc40d3b0e..3848bca320759 100644 --- a/src/tools/clippy/tests/ui/min_rust_version_attr.rs +++ b/src/tools/clippy/tests/ui/min_rust_version_attr.rs @@ -2,7 +2,7 @@ #![feature(custom_inner_attributes)] #![clippy::msrv = "1.0.0"] -use std::ops::Deref; +use std::ops::{Deref, RangeFrom}; fn option_as_ref_deref() { let mut opt = Some(String::from("123")); @@ -42,12 +42,94 @@ pub fn manual_strip_msrv() { } } +pub fn redundant_fieldnames() { + let start = 0; + let _ = RangeFrom { start: start }; +} + +pub fn redundant_static_lifetime() { + const VAR_ONE: &'static str = "Test constant #1"; +} + +pub fn checked_conversion() { + let value: i64 = 42; + let _ = value <= (u32::max_value() as i64) && value >= 0; + let _ = value <= (u32::MAX as i64) && value >= 0; +} + +pub fn filter_map_next() { + let a = ["1", "lol", "3", "NaN", "5"]; + + #[rustfmt::skip] + let _: Option = vec![1, 2, 3, 4, 5, 6] + .into_iter() + .filter_map(|x| { + if x == 2 { + Some(x * 2) + } else { + None + } + }) + .next(); +} + +#[allow(clippy::no_effect)] +#[allow(clippy::short_circuit_statement)] +#[allow(clippy::unnecessary_operation)] +pub fn manual_range_contains() { + let x = 5; + x >= 8 && x < 12; +} + +pub fn use_self() { + struct Foo {} + + impl Foo { + fn new() -> Foo { + Foo {} + } + fn test() -> Foo { + Foo::new() + } + } +} + +fn replace_with_default() { + let mut s = String::from("foo"); + let _ = std::mem::replace(&mut s, String::default()); +} + +fn map_unwrap_or() { + let opt = Some(1); + + // Check for `option.map(_).unwrap_or(_)` use. + // Single line case. + let _ = opt + .map(|x| x + 1) + // Should lint even though this call is on a separate line. + .unwrap_or(0); +} + +// Could be const +fn missing_const_for_fn() -> i32 { + 1 +} + fn main() { + filter_map_next(); + checked_conversion(); + redundant_fieldnames(); + redundant_static_lifetime(); option_as_ref_deref(); match_like_matches(); match_same_arms(); match_same_arms2(); manual_strip_msrv(); + manual_range_contains(); + use_self(); + replace_with_default(); + map_unwrap_or(); + missing_const_for_fn(); } mod meets_msrv { diff --git a/src/tools/clippy/tests/ui/min_rust_version_attr.stderr b/src/tools/clippy/tests/ui/min_rust_version_attr.stderr index 3e1af046e7a27..348052631049b 100644 --- a/src/tools/clippy/tests/ui/min_rust_version_attr.stderr +++ b/src/tools/clippy/tests/ui/min_rust_version_attr.stderr @@ -1,12 +1,12 @@ error: stripping a prefix manually - --> $DIR/min_rust_version_attr.rs:60:24 + --> $DIR/min_rust_version_attr.rs:142:24 | LL | assert_eq!(s["hello, ".len()..].to_uppercase(), "WORLD!"); | ^^^^^^^^^^^^^^^^^^^^ | = note: `-D clippy::manual-strip` implied by `-D warnings` note: the prefix was tested here - --> $DIR/min_rust_version_attr.rs:59:9 + --> $DIR/min_rust_version_attr.rs:141:9 | LL | if s.starts_with("hello, ") { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -17,13 +17,13 @@ LL | assert_eq!(.to_uppercase(), "WORLD!"); | error: stripping a prefix manually - --> $DIR/min_rust_version_attr.rs:72:24 + --> $DIR/min_rust_version_attr.rs:154:24 | LL | assert_eq!(s["hello, ".len()..].to_uppercase(), "WORLD!"); | ^^^^^^^^^^^^^^^^^^^^ | note: the prefix was tested here - --> $DIR/min_rust_version_attr.rs:71:9 + --> $DIR/min_rust_version_attr.rs:153:9 | LL | if s.starts_with("hello, ") { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ diff --git a/src/tools/clippy/tests/ui/missing-doc-crate-missing.stderr b/src/tools/clippy/tests/ui/missing-doc-crate-missing.stderr index da46f9886366c..d56c5cc4c3ae2 100644 --- a/src/tools/clippy/tests/ui/missing-doc-crate-missing.stderr +++ b/src/tools/clippy/tests/ui/missing-doc-crate-missing.stderr @@ -1,4 +1,4 @@ -error: missing documentation for crate +error: missing documentation for the crate --> $DIR/missing-doc-crate-missing.rs:1:1 | LL | / #![warn(clippy::missing_docs_in_private_items)] diff --git a/src/tools/clippy/tests/ui/missing-doc-impl.stderr b/src/tools/clippy/tests/ui/missing-doc-impl.stderr index 9656a39abceb5..7e10404ca005e 100644 --- a/src/tools/clippy/tests/ui/missing-doc-impl.stderr +++ b/src/tools/clippy/tests/ui/missing-doc-impl.stderr @@ -51,13 +51,13 @@ LL | | fn foo_with_impl(&self) {} LL | | } | |_^ -error: missing documentation for a trait method +error: missing documentation for an associated function --> $DIR/missing-doc-impl.rs:39:5 | LL | fn foo(&self); | ^^^^^^^^^^^^^^ -error: missing documentation for a trait method +error: missing documentation for an associated function --> $DIR/missing-doc-impl.rs:40:5 | LL | fn foo_with_impl(&self) {} @@ -75,25 +75,25 @@ error: missing documentation for an associated type LL | type AssociatedTypeDef = Self; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ -error: missing documentation for a method +error: missing documentation for an associated function --> $DIR/missing-doc-impl.rs:62:5 | LL | pub fn foo() {} | ^^^^^^^^^^^^^^^ -error: missing documentation for a method +error: missing documentation for an associated function --> $DIR/missing-doc-impl.rs:63:5 | LL | fn bar() {} | ^^^^^^^^^^^ -error: missing documentation for a method +error: missing documentation for an associated function --> $DIR/missing-doc-impl.rs:67:5 | LL | pub fn foo() {} | ^^^^^^^^^^^^^^^ -error: missing documentation for a method +error: missing documentation for an associated function --> $DIR/missing-doc-impl.rs:70:5 | LL | fn foo2() {} diff --git a/src/tools/clippy/tests/ui/needless_borrow.stderr b/src/tools/clippy/tests/ui/needless_borrow.stderr index 0bfeda7914db7..bea4b41b803d0 100644 --- a/src/tools/clippy/tests/ui/needless_borrow.stderr +++ b/src/tools/clippy/tests/ui/needless_borrow.stderr @@ -1,4 +1,4 @@ -error: this expression borrows a reference that is immediately dereferenced by the compiler +error: this expression borrows a reference (`&i32`) that is immediately dereferenced by the compiler --> $DIR/needless_borrow.rs:14:15 | LL | let c = x(&&a); @@ -12,7 +12,7 @@ error: this pattern creates a reference to a reference LL | if let Some(ref cake) = Some(&5) {} | ^^^^^^^^ help: change this to: `cake` -error: this expression borrows a reference that is immediately dereferenced by the compiler +error: this expression borrows a reference (`&i32`) that is immediately dereferenced by the compiler --> $DIR/needless_borrow.rs:28:15 | LL | 46 => &&a, diff --git a/src/tools/clippy/tests/ui/needless_doc_main.rs b/src/tools/clippy/tests/ui/needless_doc_main.rs index 883683e08a2aa..83e9bbaa3af43 100644 --- a/src/tools/clippy/tests/ui/needless_doc_main.rs +++ b/src/tools/clippy/tests/ui/needless_doc_main.rs @@ -10,7 +10,7 @@ /// ``` /// /// With an explicit return type it should lint too -/// ``` +/// ```edition2015 /// fn main() -> () { /// unimplemented!(); /// } @@ -39,7 +39,7 @@ fn bad_doctests() {} /// ``` /// /// This shouldn't lint either, because main is async: -/// ``` +/// ```edition2018 /// async fn main() { /// assert_eq!(42, ANSWER); /// } @@ -128,6 +128,12 @@ fn bad_doctests() {} /// ``` fn no_false_positives() {} +/// Yields a parse error when interpreted as rust code: +/// ``` +/// r#"hi" +/// ``` +fn issue_6022() {} + fn main() { bad_doctests(); no_false_positives(); diff --git a/src/tools/clippy/tests/ui/needless_update.rs b/src/tools/clippy/tests/ui/needless_update.rs index bfa005a19f910..b93ff048a62f2 100644 --- a/src/tools/clippy/tests/ui/needless_update.rs +++ b/src/tools/clippy/tests/ui/needless_update.rs @@ -6,9 +6,20 @@ struct S { pub b: i32, } +#[non_exhaustive] +struct T { + pub x: i32, + pub y: i32, +} + fn main() { let base = S { a: 0, b: 0 }; S { ..base }; // no error S { a: 1, ..base }; // no error S { a: 1, b: 1, ..base }; + + let base = T { x: 0, y: 0 }; + T { ..base }; // no error + T { x: 1, ..base }; // no error + T { x: 1, y: 1, ..base }; // no error } diff --git a/src/tools/clippy/tests/ui/needless_update.stderr b/src/tools/clippy/tests/ui/needless_update.stderr index 133c834880dd9..b154b3b306ddc 100644 --- a/src/tools/clippy/tests/ui/needless_update.stderr +++ b/src/tools/clippy/tests/ui/needless_update.stderr @@ -1,5 +1,5 @@ error: struct update has no effect, all the fields in the struct have already been specified - --> $DIR/needless_update.rs:13:23 + --> $DIR/needless_update.rs:19:23 | LL | S { a: 1, b: 1, ..base }; | ^^^^ diff --git a/src/tools/clippy/tests/ui/panic_in_result_fn.stderr b/src/tools/clippy/tests/ui/panic_in_result_fn.stderr index ca73ac5a41114..eb744b0c198fa 100644 --- a/src/tools/clippy/tests/ui/panic_in_result_fn.stderr +++ b/src/tools/clippy/tests/ui/panic_in_result_fn.stderr @@ -1,4 +1,4 @@ -error: used `unimplemented!()`, `unreachable!()`, `todo!()` or `panic!()` in a function that returns `Result` +error: used `unimplemented!()`, `unreachable!()`, `todo!()`, `panic!()` or assertion in a function that returns `Result` --> $DIR/panic_in_result_fn.rs:7:5 | LL | / fn result_with_panic() -> Result // should emit lint @@ -8,7 +8,7 @@ LL | | } | |_____^ | = note: `-D clippy::panic-in-result-fn` implied by `-D warnings` - = help: `unimplemented!()`, `unreachable!()`, `todo!()` or `panic!()` should not be used in a function that returns `Result` as `Result` is expected to return an error instead of crashing + = help: `unimplemented!()`, `unreachable!()`, `todo!()`, `panic!()` or assertions should not be used in a function that returns `Result` as `Result` is expected to return an error instead of crashing note: return Err() instead of panicking --> $DIR/panic_in_result_fn.rs:9:9 | @@ -16,7 +16,7 @@ LL | panic!("error"); | ^^^^^^^^^^^^^^^^ = note: this error originates in a macro (in Nightly builds, run with -Z macro-backtrace for more info) -error: used `unimplemented!()`, `unreachable!()`, `todo!()` or `panic!()` in a function that returns `Result` +error: used `unimplemented!()`, `unreachable!()`, `todo!()`, `panic!()` or assertion in a function that returns `Result` --> $DIR/panic_in_result_fn.rs:12:5 | LL | / fn result_with_unimplemented() -> Result // should emit lint @@ -25,7 +25,7 @@ LL | | unimplemented!(); LL | | } | |_____^ | - = help: `unimplemented!()`, `unreachable!()`, `todo!()` or `panic!()` should not be used in a function that returns `Result` as `Result` is expected to return an error instead of crashing + = help: `unimplemented!()`, `unreachable!()`, `todo!()`, `panic!()` or assertions should not be used in a function that returns `Result` as `Result` is expected to return an error instead of crashing note: return Err() instead of panicking --> $DIR/panic_in_result_fn.rs:14:9 | @@ -33,7 +33,7 @@ LL | unimplemented!(); | ^^^^^^^^^^^^^^^^^ = note: this error originates in a macro (in Nightly builds, run with -Z macro-backtrace for more info) -error: used `unimplemented!()`, `unreachable!()`, `todo!()` or `panic!()` in a function that returns `Result` +error: used `unimplemented!()`, `unreachable!()`, `todo!()`, `panic!()` or assertion in a function that returns `Result` --> $DIR/panic_in_result_fn.rs:17:5 | LL | / fn result_with_unreachable() -> Result // should emit lint @@ -42,7 +42,7 @@ LL | | unreachable!(); LL | | } | |_____^ | - = help: `unimplemented!()`, `unreachable!()`, `todo!()` or `panic!()` should not be used in a function that returns `Result` as `Result` is expected to return an error instead of crashing + = help: `unimplemented!()`, `unreachable!()`, `todo!()`, `panic!()` or assertions should not be used in a function that returns `Result` as `Result` is expected to return an error instead of crashing note: return Err() instead of panicking --> $DIR/panic_in_result_fn.rs:19:9 | @@ -50,7 +50,7 @@ LL | unreachable!(); | ^^^^^^^^^^^^^^^ = note: this error originates in a macro (in Nightly builds, run with -Z macro-backtrace for more info) -error: used `unimplemented!()`, `unreachable!()`, `todo!()` or `panic!()` in a function that returns `Result` +error: used `unimplemented!()`, `unreachable!()`, `todo!()`, `panic!()` or assertion in a function that returns `Result` --> $DIR/panic_in_result_fn.rs:22:5 | LL | / fn result_with_todo() -> Result // should emit lint @@ -59,7 +59,7 @@ LL | | todo!("Finish this"); LL | | } | |_____^ | - = help: `unimplemented!()`, `unreachable!()`, `todo!()` or `panic!()` should not be used in a function that returns `Result` as `Result` is expected to return an error instead of crashing + = help: `unimplemented!()`, `unreachable!()`, `todo!()`, `panic!()` or assertions should not be used in a function that returns `Result` as `Result` is expected to return an error instead of crashing note: return Err() instead of panicking --> $DIR/panic_in_result_fn.rs:24:9 | @@ -67,7 +67,7 @@ LL | todo!("Finish this"); | ^^^^^^^^^^^^^^^^^^^^^ = note: this error originates in a macro (in Nightly builds, run with -Z macro-backtrace for more info) -error: used `unimplemented!()`, `unreachable!()`, `todo!()` or `panic!()` in a function that returns `Result` +error: used `unimplemented!()`, `unreachable!()`, `todo!()`, `panic!()` or assertion in a function that returns `Result` --> $DIR/panic_in_result_fn.rs:53:1 | LL | / fn function_result_with_panic() -> Result // should emit lint @@ -76,7 +76,7 @@ LL | | panic!("error"); LL | | } | |_^ | - = help: `unimplemented!()`, `unreachable!()`, `todo!()` or `panic!()` should not be used in a function that returns `Result` as `Result` is expected to return an error instead of crashing + = help: `unimplemented!()`, `unreachable!()`, `todo!()`, `panic!()` or assertions should not be used in a function that returns `Result` as `Result` is expected to return an error instead of crashing note: return Err() instead of panicking --> $DIR/panic_in_result_fn.rs:55:5 | @@ -84,7 +84,7 @@ LL | panic!("error"); | ^^^^^^^^^^^^^^^^ = note: this error originates in a macro (in Nightly builds, run with -Z macro-backtrace for more info) -error: used `unimplemented!()`, `unreachable!()`, `todo!()` or `panic!()` in a function that returns `Result` +error: used `unimplemented!()`, `unreachable!()`, `todo!()`, `panic!()` or assertion in a function that returns `Result` --> $DIR/panic_in_result_fn.rs:68:1 | LL | / fn main() -> Result<(), String> { @@ -93,7 +93,7 @@ LL | | Ok(()) LL | | } | |_^ | - = help: `unimplemented!()`, `unreachable!()`, `todo!()` or `panic!()` should not be used in a function that returns `Result` as `Result` is expected to return an error instead of crashing + = help: `unimplemented!()`, `unreachable!()`, `todo!()`, `panic!()` or assertions should not be used in a function that returns `Result` as `Result` is expected to return an error instead of crashing note: return Err() instead of panicking --> $DIR/panic_in_result_fn.rs:69:5 | diff --git a/src/tools/clippy/tests/ui/panic_in_result_fn_assertions.rs b/src/tools/clippy/tests/ui/panic_in_result_fn_assertions.rs new file mode 100644 index 0000000000000..ffdf8288adc70 --- /dev/null +++ b/src/tools/clippy/tests/ui/panic_in_result_fn_assertions.rs @@ -0,0 +1,48 @@ +#![warn(clippy::panic_in_result_fn)] +#![allow(clippy::unnecessary_wraps)] + +struct A; + +impl A { + fn result_with_assert_with_message(x: i32) -> Result // should emit lint + { + assert!(x == 5, "wrong argument"); + Ok(true) + } + + fn result_with_assert_eq(x: i32) -> Result // should emit lint + { + assert_eq!(x, 5); + Ok(true) + } + + fn result_with_assert_ne(x: i32) -> Result // should emit lint + { + assert_ne!(x, 1); + Ok(true) + } + + fn other_with_assert_with_message(x: i32) // should not emit lint + { + assert!(x == 5, "wrong argument"); + } + + fn other_with_assert_eq(x: i32) // should not emit lint + { + assert_eq!(x, 5); + } + + fn other_with_assert_ne(x: i32) // should not emit lint + { + assert_ne!(x, 1); + } + + fn result_without_banned_functions() -> Result // should not emit lint + { + let assert = "assert!"; + println!("No {}", assert); + Ok(true) + } +} + +fn main() {} diff --git a/src/tools/clippy/tests/ui/panic_in_result_fn_assertions.stderr b/src/tools/clippy/tests/ui/panic_in_result_fn_assertions.stderr new file mode 100644 index 0000000000000..86f61ad718a9c --- /dev/null +++ b/src/tools/clippy/tests/ui/panic_in_result_fn_assertions.stderr @@ -0,0 +1,57 @@ +error: used `unimplemented!()`, `unreachable!()`, `todo!()`, `panic!()` or assertion in a function that returns `Result` + --> $DIR/panic_in_result_fn_assertions.rs:7:5 + | +LL | / fn result_with_assert_with_message(x: i32) -> Result // should emit lint +LL | | { +LL | | assert!(x == 5, "wrong argument"); +LL | | Ok(true) +LL | | } + | |_____^ + | + = note: `-D clippy::panic-in-result-fn` implied by `-D warnings` + = help: `unimplemented!()`, `unreachable!()`, `todo!()`, `panic!()` or assertions should not be used in a function that returns `Result` as `Result` is expected to return an error instead of crashing +note: return Err() instead of panicking + --> $DIR/panic_in_result_fn_assertions.rs:9:9 + | +LL | assert!(x == 5, "wrong argument"); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + = note: this error originates in a macro (in Nightly builds, run with -Z macro-backtrace for more info) + +error: used `unimplemented!()`, `unreachable!()`, `todo!()`, `panic!()` or assertion in a function that returns `Result` + --> $DIR/panic_in_result_fn_assertions.rs:13:5 + | +LL | / fn result_with_assert_eq(x: i32) -> Result // should emit lint +LL | | { +LL | | assert_eq!(x, 5); +LL | | Ok(true) +LL | | } + | |_____^ + | + = help: `unimplemented!()`, `unreachable!()`, `todo!()`, `panic!()` or assertions should not be used in a function that returns `Result` as `Result` is expected to return an error instead of crashing +note: return Err() instead of panicking + --> $DIR/panic_in_result_fn_assertions.rs:15:9 + | +LL | assert_eq!(x, 5); + | ^^^^^^^^^^^^^^^^^ + = note: this error originates in a macro (in Nightly builds, run with -Z macro-backtrace for more info) + +error: used `unimplemented!()`, `unreachable!()`, `todo!()`, `panic!()` or assertion in a function that returns `Result` + --> $DIR/panic_in_result_fn_assertions.rs:19:5 + | +LL | / fn result_with_assert_ne(x: i32) -> Result // should emit lint +LL | | { +LL | | assert_ne!(x, 1); +LL | | Ok(true) +LL | | } + | |_____^ + | + = help: `unimplemented!()`, `unreachable!()`, `todo!()`, `panic!()` or assertions should not be used in a function that returns `Result` as `Result` is expected to return an error instead of crashing +note: return Err() instead of panicking + --> $DIR/panic_in_result_fn_assertions.rs:21:9 + | +LL | assert_ne!(x, 1); + | ^^^^^^^^^^^^^^^^^ + = note: this error originates in a macro (in Nightly builds, run with -Z macro-backtrace for more info) + +error: aborting due to 3 previous errors + diff --git a/src/tools/clippy/tests/ui/panic_in_result_fn_debug_assertions.rs b/src/tools/clippy/tests/ui/panic_in_result_fn_debug_assertions.rs new file mode 100644 index 0000000000000..b60c79f97c865 --- /dev/null +++ b/src/tools/clippy/tests/ui/panic_in_result_fn_debug_assertions.rs @@ -0,0 +1,48 @@ +#![warn(clippy::panic_in_result_fn)] +#![allow(clippy::unnecessary_wraps)] + +struct A; + +impl A { + fn result_with_debug_assert_with_message(x: i32) -> Result // should emit lint + { + debug_assert!(x == 5, "wrong argument"); + Ok(true) + } + + fn result_with_debug_assert_eq(x: i32) -> Result // should emit lint + { + debug_assert_eq!(x, 5); + Ok(true) + } + + fn result_with_debug_assert_ne(x: i32) -> Result // should emit lint + { + debug_assert_ne!(x, 1); + Ok(true) + } + + fn other_with_debug_assert_with_message(x: i32) // should not emit lint + { + debug_assert!(x == 5, "wrong argument"); + } + + fn other_with_debug_assert_eq(x: i32) // should not emit lint + { + debug_assert_eq!(x, 5); + } + + fn other_with_debug_assert_ne(x: i32) // should not emit lint + { + debug_assert_ne!(x, 1); + } + + fn result_without_banned_functions() -> Result // should not emit lint + { + let debug_assert = "debug_assert!"; + println!("No {}", debug_assert); + Ok(true) + } +} + +fn main() {} diff --git a/src/tools/clippy/tests/ui/panic_in_result_fn_debug_assertions.stderr b/src/tools/clippy/tests/ui/panic_in_result_fn_debug_assertions.stderr new file mode 100644 index 0000000000000..ec18e89698c56 --- /dev/null +++ b/src/tools/clippy/tests/ui/panic_in_result_fn_debug_assertions.stderr @@ -0,0 +1,57 @@ +error: used `unimplemented!()`, `unreachable!()`, `todo!()`, `panic!()` or assertion in a function that returns `Result` + --> $DIR/panic_in_result_fn_debug_assertions.rs:7:5 + | +LL | / fn result_with_debug_assert_with_message(x: i32) -> Result // should emit lint +LL | | { +LL | | debug_assert!(x == 5, "wrong argument"); +LL | | Ok(true) +LL | | } + | |_____^ + | + = note: `-D clippy::panic-in-result-fn` implied by `-D warnings` + = help: `unimplemented!()`, `unreachable!()`, `todo!()`, `panic!()` or assertions should not be used in a function that returns `Result` as `Result` is expected to return an error instead of crashing +note: return Err() instead of panicking + --> $DIR/panic_in_result_fn_debug_assertions.rs:9:9 + | +LL | debug_assert!(x == 5, "wrong argument"); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + = note: this error originates in a macro (in Nightly builds, run with -Z macro-backtrace for more info) + +error: used `unimplemented!()`, `unreachable!()`, `todo!()`, `panic!()` or assertion in a function that returns `Result` + --> $DIR/panic_in_result_fn_debug_assertions.rs:13:5 + | +LL | / fn result_with_debug_assert_eq(x: i32) -> Result // should emit lint +LL | | { +LL | | debug_assert_eq!(x, 5); +LL | | Ok(true) +LL | | } + | |_____^ + | + = help: `unimplemented!()`, `unreachable!()`, `todo!()`, `panic!()` or assertions should not be used in a function that returns `Result` as `Result` is expected to return an error instead of crashing +note: return Err() instead of panicking + --> $DIR/panic_in_result_fn_debug_assertions.rs:15:9 + | +LL | debug_assert_eq!(x, 5); + | ^^^^^^^^^^^^^^^^^^^^^^^ + = note: this error originates in a macro (in Nightly builds, run with -Z macro-backtrace for more info) + +error: used `unimplemented!()`, `unreachable!()`, `todo!()`, `panic!()` or assertion in a function that returns `Result` + --> $DIR/panic_in_result_fn_debug_assertions.rs:19:5 + | +LL | / fn result_with_debug_assert_ne(x: i32) -> Result // should emit lint +LL | | { +LL | | debug_assert_ne!(x, 1); +LL | | Ok(true) +LL | | } + | |_____^ + | + = help: `unimplemented!()`, `unreachable!()`, `todo!()`, `panic!()` or assertions should not be used in a function that returns `Result` as `Result` is expected to return an error instead of crashing +note: return Err() instead of panicking + --> $DIR/panic_in_result_fn_debug_assertions.rs:21:9 + | +LL | debug_assert_ne!(x, 1); + | ^^^^^^^^^^^^^^^^^^^^^^^ + = note: this error originates in a macro (in Nightly builds, run with -Z macro-backtrace for more info) + +error: aborting due to 3 previous errors + diff --git a/src/tools/clippy/tests/ui/print_stderr.rs b/src/tools/clippy/tests/ui/print_stderr.rs new file mode 100644 index 0000000000000..fa07e74a7be47 --- /dev/null +++ b/src/tools/clippy/tests/ui/print_stderr.rs @@ -0,0 +1,8 @@ +#![warn(clippy::print_stderr)] + +fn main() { + eprintln!("Hello"); + println!("This should not do anything"); + eprint!("World"); + print!("Nor should this"); +} diff --git a/src/tools/clippy/tests/ui/print_stderr.stderr b/src/tools/clippy/tests/ui/print_stderr.stderr new file mode 100644 index 0000000000000..5af735af65769 --- /dev/null +++ b/src/tools/clippy/tests/ui/print_stderr.stderr @@ -0,0 +1,16 @@ +error: use of `eprintln!` + --> $DIR/print_stderr.rs:4:5 + | +LL | eprintln!("Hello"); + | ^^^^^^^^^^^^^^^^^^ + | + = note: `-D clippy::print-stderr` implied by `-D warnings` + +error: use of `eprint!` + --> $DIR/print_stderr.rs:6:5 + | +LL | eprint!("World"); + | ^^^^^^^^^^^^^^^^ + +error: aborting due to 2 previous errors + diff --git a/src/tools/clippy/tests/ui/println_empty_string.fixed b/src/tools/clippy/tests/ui/println_empty_string.fixed index 2b889b62ea991..9760680927a63 100644 --- a/src/tools/clippy/tests/ui/println_empty_string.fixed +++ b/src/tools/clippy/tests/ui/println_empty_string.fixed @@ -8,4 +8,11 @@ fn main() { match "a" { _ => println!(), } + + eprintln!(); + eprintln!(); + + match "a" { + _ => eprintln!(), + } } diff --git a/src/tools/clippy/tests/ui/println_empty_string.rs b/src/tools/clippy/tests/ui/println_empty_string.rs index 890f5f6847603..80fdb3e6e2105 100644 --- a/src/tools/clippy/tests/ui/println_empty_string.rs +++ b/src/tools/clippy/tests/ui/println_empty_string.rs @@ -8,4 +8,11 @@ fn main() { match "a" { _ => println!(""), } + + eprintln!(); + eprintln!(""); + + match "a" { + _ => eprintln!(""), + } } diff --git a/src/tools/clippy/tests/ui/println_empty_string.stderr b/src/tools/clippy/tests/ui/println_empty_string.stderr index 23112b8816893..17fe4ea74790c 100644 --- a/src/tools/clippy/tests/ui/println_empty_string.stderr +++ b/src/tools/clippy/tests/ui/println_empty_string.stderr @@ -12,5 +12,17 @@ error: using `println!("")` LL | _ => println!(""), | ^^^^^^^^^^^^ help: replace it with: `println!()` -error: aborting due to 2 previous errors +error: using `eprintln!("")` + --> $DIR/println_empty_string.rs:13:5 + | +LL | eprintln!(""); + | ^^^^^^^^^^^^^ help: replace it with: `eprintln!()` + +error: using `eprintln!("")` + --> $DIR/println_empty_string.rs:16:14 + | +LL | _ => eprintln!(""), + | ^^^^^^^^^^^^^ help: replace it with: `eprintln!()` + +error: aborting due to 4 previous errors diff --git a/src/tools/clippy/tests/ui/range_contains.fixed b/src/tools/clippy/tests/ui/range_contains.fixed index 048874a7f8294..47c974e614b91 100644 --- a/src/tools/clippy/tests/ui/range_contains.fixed +++ b/src/tools/clippy/tests/ui/range_contains.fixed @@ -44,3 +44,8 @@ fn main() { (0. ..1.).contains(&y); !(0. ..=1.).contains(&y); } + +// Fix #6373 +pub const fn in_range(a: i32) -> bool { + 3 <= a && a <= 20 +} diff --git a/src/tools/clippy/tests/ui/range_contains.rs b/src/tools/clippy/tests/ui/range_contains.rs index 60ad259f404d8..835deced5e4cb 100644 --- a/src/tools/clippy/tests/ui/range_contains.rs +++ b/src/tools/clippy/tests/ui/range_contains.rs @@ -44,3 +44,8 @@ fn main() { y >= 0. && y < 1.; y < 0. || y > 1.; } + +// Fix #6373 +pub const fn in_range(a: i32) -> bool { + 3 <= a && a <= 20 +} diff --git a/src/tools/clippy/tests/ui/redundant_else.rs b/src/tools/clippy/tests/ui/redundant_else.rs new file mode 100644 index 0000000000000..737c8a9f8db43 --- /dev/null +++ b/src/tools/clippy/tests/ui/redundant_else.rs @@ -0,0 +1,154 @@ +#![warn(clippy::redundant_else)] +#![allow(clippy::needless_return)] + +fn main() { + loop { + // break + if foo() { + println!("Love your neighbor;"); + break; + } else { + println!("yet don't pull down your hedge."); + } + // continue + if foo() { + println!("He that lies down with Dogs,"); + continue; + } else { + println!("shall rise up with fleas."); + } + // match block + if foo() { + match foo() { + 1 => break, + _ => return, + } + } else { + println!("You may delay, but time will not."); + } + } + // else if + if foo() { + return; + } else if foo() { + return; + } else { + println!("A fat kitchen makes a lean will."); + } + // let binding outside of block + let _ = { + if foo() { + return; + } else { + 1 + } + }; + // else if with let binding outside of block + let _ = { + if foo() { + return; + } else if foo() { + return; + } else { + 2 + } + }; + // inside if let + let _ = if let Some(1) = foo() { + let _ = 1; + if foo() { + return; + } else { + 1 + } + } else { + 1 + }; + + // + // non-lint cases + // + + // sanity check + if foo() { + let _ = 1; + } else { + println!("Who is wise? He that learns from every one."); + } + // else if without else + if foo() { + return; + } else if foo() { + foo() + }; + // nested if return + if foo() { + if foo() { + return; + } + } else { + foo() + }; + // match with non-breaking branch + if foo() { + match foo() { + 1 => foo(), + _ => return, + } + } else { + println!("Three may keep a secret, if two of them are dead."); + } + // let binding + let _ = if foo() { + return; + } else { + 1 + }; + // assign + let a; + a = if foo() { + return; + } else { + 1 + }; + // assign-op + a += if foo() { + return; + } else { + 1 + }; + // if return else if else + if foo() { + return; + } else if foo() { + 1 + } else { + 2 + }; + // if else if return else + if foo() { + 1 + } else if foo() { + return; + } else { + 2 + }; + // else if with let binding + let _ = if foo() { + return; + } else if foo() { + return; + } else { + 2 + }; + // inside function call + Box::new(if foo() { + return; + } else { + 1 + }); +} + +fn foo() -> T { + unimplemented!("I'm not Santa Claus") +} diff --git a/src/tools/clippy/tests/ui/redundant_else.stderr b/src/tools/clippy/tests/ui/redundant_else.stderr new file mode 100644 index 0000000000000..9000cdc814b1f --- /dev/null +++ b/src/tools/clippy/tests/ui/redundant_else.stderr @@ -0,0 +1,80 @@ +error: redundant else block + --> $DIR/redundant_else.rs:10:16 + | +LL | } else { + | ________________^ +LL | | println!("yet don't pull down your hedge."); +LL | | } + | |_________^ + | + = note: `-D clippy::redundant-else` implied by `-D warnings` + = help: remove the `else` block and move the contents out + +error: redundant else block + --> $DIR/redundant_else.rs:17:16 + | +LL | } else { + | ________________^ +LL | | println!("shall rise up with fleas."); +LL | | } + | |_________^ + | + = help: remove the `else` block and move the contents out + +error: redundant else block + --> $DIR/redundant_else.rs:26:16 + | +LL | } else { + | ________________^ +LL | | println!("You may delay, but time will not."); +LL | | } + | |_________^ + | + = help: remove the `else` block and move the contents out + +error: redundant else block + --> $DIR/redundant_else.rs:35:12 + | +LL | } else { + | ____________^ +LL | | println!("A fat kitchen makes a lean will."); +LL | | } + | |_____^ + | + = help: remove the `else` block and move the contents out + +error: redundant else block + --> $DIR/redundant_else.rs:42:16 + | +LL | } else { + | ________________^ +LL | | 1 +LL | | } + | |_________^ + | + = help: remove the `else` block and move the contents out + +error: redundant else block + --> $DIR/redundant_else.rs:52:16 + | +LL | } else { + | ________________^ +LL | | 2 +LL | | } + | |_________^ + | + = help: remove the `else` block and move the contents out + +error: redundant else block + --> $DIR/redundant_else.rs:61:16 + | +LL | } else { + | ________________^ +LL | | 1 +LL | | } + | |_________^ + | + = help: remove the `else` block and move the contents out + +error: aborting due to 7 previous errors + diff --git a/src/tools/clippy/tests/ui/suspicious_else_formatting.rs b/src/tools/clippy/tests/ui/suspicious_else_formatting.rs new file mode 100644 index 0000000000000..226010ec6df3a --- /dev/null +++ b/src/tools/clippy/tests/ui/suspicious_else_formatting.rs @@ -0,0 +1,79 @@ +#![warn(clippy::suspicious_else_formatting)] + +fn foo() -> bool { + true +} + +#[rustfmt::skip] +fn main() { + // weird `else` formatting: + if foo() { + } { + } + + if foo() { + } if foo() { + } + + let _ = { // if as the last expression + let _ = 0; + + if foo() { + } if foo() { + } + else { + } + }; + + let _ = { // if in the middle of a block + if foo() { + } if foo() { + } + else { + } + + let _ = 0; + }; + + if foo() { + } else + { + } + + if foo() { + } + else + { + } + + if foo() { + } else + if foo() { // the span of the above error should continue here + } + + if foo() { + } + else + if foo() { // the span of the above error should continue here + } + + // those are ok: + if foo() { + } + { + } + + if foo() { + } else { + } + + if foo() { + } + else { + } + + if foo() { + } + if foo() { + } +} diff --git a/src/tools/clippy/tests/ui/suspicious_else_formatting.stderr b/src/tools/clippy/tests/ui/suspicious_else_formatting.stderr new file mode 100644 index 0000000000000..bbc036d376fef --- /dev/null +++ b/src/tools/clippy/tests/ui/suspicious_else_formatting.stderr @@ -0,0 +1,77 @@ +error: this looks like an `else {..}` but the `else` is missing + --> $DIR/suspicious_else_formatting.rs:11:6 + | +LL | } { + | ^ + | + = note: `-D clippy::suspicious-else-formatting` implied by `-D warnings` + = note: to remove this lint, add the missing `else` or add a new line before the next block + +error: this looks like an `else if` but the `else` is missing + --> $DIR/suspicious_else_formatting.rs:15:6 + | +LL | } if foo() { + | ^ + | + = note: to remove this lint, add the missing `else` or add a new line before the second `if` + +error: this looks like an `else if` but the `else` is missing + --> $DIR/suspicious_else_formatting.rs:22:10 + | +LL | } if foo() { + | ^ + | + = note: to remove this lint, add the missing `else` or add a new line before the second `if` + +error: this looks like an `else if` but the `else` is missing + --> $DIR/suspicious_else_formatting.rs:30:10 + | +LL | } if foo() { + | ^ + | + = note: to remove this lint, add the missing `else` or add a new line before the second `if` + +error: this is an `else {..}` but the formatting might hide it + --> $DIR/suspicious_else_formatting.rs:39:6 + | +LL | } else + | ______^ +LL | | { + | |____^ + | + = note: to remove this lint, remove the `else` or remove the new line between `else` and `{..}` + +error: this is an `else {..}` but the formatting might hide it + --> $DIR/suspicious_else_formatting.rs:44:6 + | +LL | } + | ______^ +LL | | else +LL | | { + | |____^ + | + = note: to remove this lint, remove the `else` or remove the new line between `else` and `{..}` + +error: this is an `else if` but the formatting might hide it + --> $DIR/suspicious_else_formatting.rs:50:6 + | +LL | } else + | ______^ +LL | | if foo() { // the span of the above error should continue here + | |____^ + | + = note: to remove this lint, remove the `else` or remove the new line between `else` and `if` + +error: this is an `else if` but the formatting might hide it + --> $DIR/suspicious_else_formatting.rs:55:6 + | +LL | } + | ______^ +LL | | else +LL | | if foo() { // the span of the above error should continue here + | |____^ + | + = note: to remove this lint, remove the `else` or remove the new line between `else` and `if` + +error: aborting due to 8 previous errors + diff --git a/src/tools/clippy/tests/ui/unnecessary_clone.stderr b/src/tools/clippy/tests/ui/unnecessary_clone.stderr index 5ffa6c4fd0616..9df1ae5686730 100644 --- a/src/tools/clippy/tests/ui/unnecessary_clone.stderr +++ b/src/tools/clippy/tests/ui/unnecessary_clone.stderr @@ -30,7 +30,7 @@ error: using `.clone()` on a ref-counted pointer LL | let _: Arc = x.clone(); | ^^^^^^^^^ help: try this: `Arc::::clone(&x)` -error: using `clone` on a `Copy` type +error: using `clone` on type `T` which implements the `Copy` trait --> $DIR/unnecessary_clone.rs:40:5 | LL | t.clone(); @@ -38,13 +38,13 @@ LL | t.clone(); | = note: `-D clippy::clone-on-copy` implied by `-D warnings` -error: using `clone` on a `Copy` type +error: using `clone` on type `std::option::Option` which implements the `Copy` trait --> $DIR/unnecessary_clone.rs:42:5 | LL | Some(t).clone(); | ^^^^^^^^^^^^^^^ help: try removing the `clone` call: `Some(t)` -error: using `clone` on a double-reference; this will copy the reference instead of cloning the inner type +error: using `clone` on a double-reference; this will copy the reference of type `&std::vec::Vec` instead of cloning the inner type --> $DIR/unnecessary_clone.rs:48:22 | LL | let z: &Vec<_> = y.clone(); @@ -60,13 +60,13 @@ help: or try being explicit if you are sure, that you want to clone a reference LL | let z: &Vec<_> = <&std::vec::Vec>::clone(y); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ -error: using `clone` on a `Copy` type +error: using `clone` on type `many_derefs::E` which implements the `Copy` trait --> $DIR/unnecessary_clone.rs:84:20 | LL | let _: E = a.clone(); | ^^^^^^^^^ help: try dereferencing it: `*****a` -error: using `clone` on a double-reference; this will copy the reference instead of cloning the inner type +error: using `clone` on a double-reference; this will copy the reference of type `&[u8]` instead of cloning the inner type --> $DIR/unnecessary_clone.rs:89:22 | LL | let _ = &mut encoded.clone(); @@ -81,7 +81,7 @@ help: or try being explicit if you are sure, that you want to clone a reference LL | let _ = &mut <&[u8]>::clone(encoded); | ^^^^^^^^^^^^^^^^^^^^^^^ -error: using `clone` on a double-reference; this will copy the reference instead of cloning the inner type +error: using `clone` on a double-reference; this will copy the reference of type `&[u8]` instead of cloning the inner type --> $DIR/unnecessary_clone.rs:90:18 | LL | let _ = &encoded.clone(); diff --git a/src/tools/clippy/tests/ui/unnecessary_lazy_eval_unfixable.rs b/src/tools/clippy/tests/ui/unnecessary_lazy_eval_unfixable.rs index 2e923bc97a2ef..b05dd143bfd74 100644 --- a/src/tools/clippy/tests/ui/unnecessary_lazy_eval_unfixable.rs +++ b/src/tools/clippy/tests/ui/unnecessary_lazy_eval_unfixable.rs @@ -15,4 +15,8 @@ fn main() { } let _ = Ok(1).unwrap_or_else(|e::E| 2); let _ = Ok(1).unwrap_or_else(|SomeStruct { .. }| 2); + + // Fix #6343 + let arr = [(Some(1),)]; + Some(&0).and_then(|&i| arr[i].0); } diff --git a/src/tools/clippy/tests/ui/update-all-references.sh b/src/tools/clippy/tests/ui/update-all-references.sh index 30ba9188db43d..4391499a1e1f8 100755 --- a/src/tools/clippy/tests/ui/update-all-references.sh +++ b/src/tools/clippy/tests/ui/update-all-references.sh @@ -1,21 +1,3 @@ #!/bin/bash -# A script to update the references for all tests. The idea is that -# you do a run, which will generate files in the build directory -# containing the (normalized) actual output of the compiler. You then -# run this script, which will copy those files over. If you find -# yourself manually editing a foo.stderr file, you're doing it wrong. -# -# See all `update-references.sh`, if you just want to update a single test. - -if [[ "$1" == "--help" || "$1" == "-h" ]]; then - echo "usage: $0" -fi - -CARGO_TARGET_DIR=${CARGO_TARGET_DIR:-$PWD/target} -PROFILE=${PROFILE:-debug} -BUILD_DIR=${CARGO_TARGET_DIR}/${PROFILE}/test_build_base - -MY_DIR=$(dirname "$0") -cd "$MY_DIR" || exit -find . -name '*.rs' -exec ./update-references.sh "$BUILD_DIR" {} + +echo "Please use 'cargo dev bless' instead." diff --git a/src/tools/clippy/tests/ui/update-references.sh b/src/tools/clippy/tests/ui/update-references.sh deleted file mode 100755 index e16ed600ef814..0000000000000 --- a/src/tools/clippy/tests/ui/update-references.sh +++ /dev/null @@ -1,56 +0,0 @@ -#!/bin/bash - -# A script to update the references for particular tests. The idea is -# that you do a run, which will generate files in the build directory -# containing the (normalized) actual output of the compiler. This -# script will then copy that output and replace the "expected output" -# files. You can then commit the changes. -# -# If you find yourself manually editing a `foo.stderr` file, you're -# doing it wrong. - -if [[ "$1" == "--help" || "$1" == "-h" || "$1" == "" || "$2" == "" ]]; then - echo "usage: $0 " - echo "" - echo "For example:" - echo " $0 ../../../build/x86_64-apple-darwin/test/ui *.rs */*.rs" -fi - -MYDIR=$(dirname "$0") - -BUILD_DIR="$1" -shift - -while [[ "$1" != "" ]]; do - STDERR_NAME="${1/%.rs/.stderr}" - STDOUT_NAME="${1/%.rs/.stdout}" - FIXED_NAME="${1/%.rs/.fixed}" - shift - if [[ -f "$BUILD_DIR"/"$STDOUT_NAME" ]] && \ - ! (cmp -s -- "$BUILD_DIR"/"$STDOUT_NAME" "$MYDIR"/"$STDOUT_NAME"); then - echo updating "$MYDIR"/"$STDOUT_NAME" - cp "$BUILD_DIR"/"$STDOUT_NAME" "$MYDIR"/"$STDOUT_NAME" - if [[ ! -s "$MYDIR"/"$STDOUT_NAME" ]]; then - echo removing "$MYDIR"/"$STDOUT_NAME" - rm "$MYDIR"/"$STDOUT_NAME" - fi - fi - if [[ -f "$BUILD_DIR"/"$STDERR_NAME" ]] && \ - ! (cmp -s -- "$BUILD_DIR"/"$STDERR_NAME" "$MYDIR"/"$STDERR_NAME"); then - echo updating "$MYDIR"/"$STDERR_NAME" - cp "$BUILD_DIR"/"$STDERR_NAME" "$MYDIR"/"$STDERR_NAME" - if [[ ! -s "$MYDIR"/"$STDERR_NAME" ]]; then - echo removing "$MYDIR"/"$STDERR_NAME" - rm "$MYDIR"/"$STDERR_NAME" - fi - fi - if [[ -f "$BUILD_DIR"/"$FIXED_NAME" ]] && \ - ! (cmp -s -- "$BUILD_DIR"/"$FIXED_NAME" "$MYDIR"/"$FIXED_NAME"); then - echo updating "$MYDIR"/"$FIXED_NAME" - cp "$BUILD_DIR"/"$FIXED_NAME" "$MYDIR"/"$FIXED_NAME" - if [[ ! -s "$MYDIR"/"$FIXED_NAME" ]]; then - echo removing "$MYDIR"/"$FIXED_NAME" - rm "$MYDIR"/"$FIXED_NAME" - fi - fi -done diff --git a/src/tools/clippy/tests/ui/use_self.fixed b/src/tools/clippy/tests/ui/use_self.fixed index ebb3aa28daf3d..d6a890014e681 100644 --- a/src/tools/clippy/tests/ui/use_self.fixed +++ b/src/tools/clippy/tests/ui/use_self.fixed @@ -71,6 +71,7 @@ mod lifetimes { mod issue2894 { trait IntoBytes { + #[allow(clippy::wrong_self_convention)] fn into_bytes(&self) -> Vec; } diff --git a/src/tools/clippy/tests/ui/use_self.rs b/src/tools/clippy/tests/ui/use_self.rs index 8a182192ab34d..b04d9ce75b2a5 100644 --- a/src/tools/clippy/tests/ui/use_self.rs +++ b/src/tools/clippy/tests/ui/use_self.rs @@ -71,6 +71,7 @@ mod lifetimes { mod issue2894 { trait IntoBytes { + #[allow(clippy::wrong_self_convention)] fn into_bytes(&self) -> Vec; } diff --git a/src/tools/clippy/tests/ui/use_self.stderr b/src/tools/clippy/tests/ui/use_self.stderr index b33928597c145..80e1bfc75e80a 100644 --- a/src/tools/clippy/tests/ui/use_self.stderr +++ b/src/tools/clippy/tests/ui/use_self.stderr @@ -37,19 +37,19 @@ LL | Foo::new() | ^^^ help: use the applicable keyword: `Self` error: unnecessary structure name repetition - --> $DIR/use_self.rs:89:56 + --> $DIR/use_self.rs:90:56 | LL | fn bad(foos: &[Self]) -> impl Iterator { | ^^^ help: use the applicable keyword: `Self` error: unnecessary structure name repetition - --> $DIR/use_self.rs:104:13 + --> $DIR/use_self.rs:105:13 | LL | TS(0) | ^^ help: use the applicable keyword: `Self` error: unnecessary structure name repetition - --> $DIR/use_self.rs:112:25 + --> $DIR/use_self.rs:113:25 | LL | fn new() -> Foo { | ^^^ help: use the applicable keyword: `Self` @@ -60,7 +60,7 @@ LL | use_self_expand!(); // Should lint in local macros = note: this error originates in a macro (in Nightly builds, run with -Z macro-backtrace for more info) error: unnecessary structure name repetition - --> $DIR/use_self.rs:113:17 + --> $DIR/use_self.rs:114:17 | LL | Foo {} | ^^^ help: use the applicable keyword: `Self` @@ -71,91 +71,91 @@ LL | use_self_expand!(); // Should lint in local macros = note: this error originates in a macro (in Nightly builds, run with -Z macro-backtrace for more info) error: unnecessary structure name repetition - --> $DIR/use_self.rs:148:21 + --> $DIR/use_self.rs:149:21 | LL | fn baz() -> Foo { | ^^^ help: use the applicable keyword: `Self` error: unnecessary structure name repetition - --> $DIR/use_self.rs:149:13 + --> $DIR/use_self.rs:150:13 | LL | Foo {} | ^^^ help: use the applicable keyword: `Self` error: unnecessary structure name repetition - --> $DIR/use_self.rs:136:29 + --> $DIR/use_self.rs:137:29 | LL | fn bar() -> Bar { | ^^^ help: use the applicable keyword: `Self` error: unnecessary structure name repetition - --> $DIR/use_self.rs:137:21 + --> $DIR/use_self.rs:138:21 | LL | Bar { foo: Foo {} } | ^^^ help: use the applicable keyword: `Self` error: unnecessary structure name repetition - --> $DIR/use_self.rs:166:21 + --> $DIR/use_self.rs:167:21 | LL | let _ = Enum::B(42); | ^^^^ help: use the applicable keyword: `Self` error: unnecessary structure name repetition - --> $DIR/use_self.rs:167:21 + --> $DIR/use_self.rs:168:21 | LL | let _ = Enum::C { field: true }; | ^^^^ help: use the applicable keyword: `Self` error: unnecessary structure name repetition - --> $DIR/use_self.rs:168:21 + --> $DIR/use_self.rs:169:21 | LL | let _ = Enum::A; | ^^^^ help: use the applicable keyword: `Self` error: unnecessary structure name repetition - --> $DIR/use_self.rs:199:13 + --> $DIR/use_self.rs:200:13 | LL | nested::A::fun_1(); | ^^^^^^^^^ help: use the applicable keyword: `Self` error: unnecessary structure name repetition - --> $DIR/use_self.rs:200:13 + --> $DIR/use_self.rs:201:13 | LL | nested::A::A; | ^^^^^^^^^ help: use the applicable keyword: `Self` error: unnecessary structure name repetition - --> $DIR/use_self.rs:202:13 + --> $DIR/use_self.rs:203:13 | LL | nested::A {}; | ^^^^^^^^^ help: use the applicable keyword: `Self` error: unnecessary structure name repetition - --> $DIR/use_self.rs:221:13 + --> $DIR/use_self.rs:222:13 | LL | TestStruct::from_something() | ^^^^^^^^^^ help: use the applicable keyword: `Self` error: unnecessary structure name repetition - --> $DIR/use_self.rs:235:25 + --> $DIR/use_self.rs:236:25 | LL | async fn g() -> S { | ^ help: use the applicable keyword: `Self` error: unnecessary structure name repetition - --> $DIR/use_self.rs:236:13 + --> $DIR/use_self.rs:237:13 | LL | S {} | ^ help: use the applicable keyword: `Self` error: unnecessary structure name repetition - --> $DIR/use_self.rs:240:16 + --> $DIR/use_self.rs:241:16 | LL | &p[S::A..S::B] | ^ help: use the applicable keyword: `Self` error: unnecessary structure name repetition - --> $DIR/use_self.rs:240:22 + --> $DIR/use_self.rs:241:22 | LL | &p[S::A..S::B] | ^ help: use the applicable keyword: `Self` diff --git a/src/tools/clippy/tests/ui/wrong_self_convention.rs b/src/tools/clippy/tests/ui/wrong_self_convention.rs index f44305d7e4838..5282eba74fd18 100644 --- a/src/tools/clippy/tests/ui/wrong_self_convention.rs +++ b/src/tools/clippy/tests/ui/wrong_self_convention.rs @@ -88,3 +88,52 @@ mod issue4037 { } } } + +// Lint also in trait definition (see #6307) +mod issue6307 { + trait T: Sized { + fn as_i32(self) {} + fn as_u32(&self) {} + fn into_i32(&self) {} + fn into_u32(self) {} + fn is_i32(self) {} + fn is_u32(&self) {} + fn to_i32(self) {} + fn to_u32(&self) {} + fn from_i32(self) {} + // check whether the lint can be allowed at the function level + #[allow(clippy::wrong_self_convention)] + fn from_cake(self) {} + + // test for false positives + fn as_(self) {} + fn into_(&self) {} + fn is_(self) {} + fn to_(self) {} + fn from_(self) {} + fn to_mut(&mut self) {} + } + + trait U { + fn as_i32(self); + fn as_u32(&self); + fn into_i32(&self); + fn into_u32(self); + fn is_i32(self); + fn is_u32(&self); + fn to_i32(self); + fn to_u32(&self); + fn from_i32(self); + // check whether the lint can be allowed at the function level + #[allow(clippy::wrong_self_convention)] + fn from_cake(self); + + // test for false positives + fn as_(self); + fn into_(&self); + fn is_(self); + fn to_(self); + fn from_(self); + fn to_mut(&mut self); + } +} diff --git a/src/tools/clippy/tests/ui/wrong_self_convention.stderr b/src/tools/clippy/tests/ui/wrong_self_convention.stderr index ef3ad73ebc7c1..86467eb0fc737 100644 --- a/src/tools/clippy/tests/ui/wrong_self_convention.stderr +++ b/src/tools/clippy/tests/ui/wrong_self_convention.stderr @@ -72,5 +72,65 @@ error: methods called `from_*` usually take no self; consider choosing a less am LL | pub fn from_i64(self) {} | ^^^^ -error: aborting due to 12 previous errors +error: methods called `as_*` usually take self by reference or self by mutable reference; consider choosing a less ambiguous name + --> $DIR/wrong_self_convention.rs:95:19 + | +LL | fn as_i32(self) {} + | ^^^^ + +error: methods called `into_*` usually take self by value; consider choosing a less ambiguous name + --> $DIR/wrong_self_convention.rs:97:21 + | +LL | fn into_i32(&self) {} + | ^^^^^ + +error: methods called `is_*` usually take self by reference or no self; consider choosing a less ambiguous name + --> $DIR/wrong_self_convention.rs:99:19 + | +LL | fn is_i32(self) {} + | ^^^^ + +error: methods called `to_*` usually take self by reference; consider choosing a less ambiguous name + --> $DIR/wrong_self_convention.rs:101:19 + | +LL | fn to_i32(self) {} + | ^^^^ + +error: methods called `from_*` usually take no self; consider choosing a less ambiguous name + --> $DIR/wrong_self_convention.rs:103:21 + | +LL | fn from_i32(self) {} + | ^^^^ + +error: methods called `as_*` usually take self by reference or self by mutable reference; consider choosing a less ambiguous name + --> $DIR/wrong_self_convention.rs:118:19 + | +LL | fn as_i32(self); + | ^^^^ + +error: methods called `into_*` usually take self by value; consider choosing a less ambiguous name + --> $DIR/wrong_self_convention.rs:120:21 + | +LL | fn into_i32(&self); + | ^^^^^ + +error: methods called `is_*` usually take self by reference or no self; consider choosing a less ambiguous name + --> $DIR/wrong_self_convention.rs:122:19 + | +LL | fn is_i32(self); + | ^^^^ + +error: methods called `to_*` usually take self by reference; consider choosing a less ambiguous name + --> $DIR/wrong_self_convention.rs:124:19 + | +LL | fn to_i32(self); + | ^^^^ + +error: methods called `from_*` usually take no self; consider choosing a less ambiguous name + --> $DIR/wrong_self_convention.rs:126:21 + | +LL | fn from_i32(self); + | ^^^^ + +error: aborting due to 22 previous errors diff --git a/src/tools/clippy/tests/ui/zero_sized_btreemap_values.rs b/src/tools/clippy/tests/ui/zero_sized_btreemap_values.rs new file mode 100644 index 0000000000000..5cd254787d834 --- /dev/null +++ b/src/tools/clippy/tests/ui/zero_sized_btreemap_values.rs @@ -0,0 +1,68 @@ +#![warn(clippy::zero_sized_map_values)] +use std::collections::BTreeMap; + +const CONST_OK: Option> = None; +const CONST_NOT_OK: Option> = None; + +static STATIC_OK: Option> = None; +static STATIC_NOT_OK: Option> = None; + +type OkMap = BTreeMap; +type NotOkMap = BTreeMap; + +enum TestEnum { + Ok(BTreeMap), + NotOk(BTreeMap), +} + +struct Test { + ok: BTreeMap, + not_ok: BTreeMap, + + also_not_ok: Vec>, +} + +trait TestTrait { + type Output; + + fn produce_output() -> Self::Output; + + fn weird_map(&self, map: BTreeMap); +} + +impl Test { + fn ok(&self) -> BTreeMap { + todo!() + } + + fn not_ok(&self) -> BTreeMap { + todo!() + } +} + +impl TestTrait for Test { + type Output = BTreeMap; + + fn produce_output() -> Self::Output { + todo!(); + } + + fn weird_map(&self, map: BTreeMap) { + todo!(); + } +} + +fn test(map: BTreeMap, key: &str) -> BTreeMap { + todo!(); +} + +fn test2(map: BTreeMap, key: &str) -> BTreeMap { + todo!(); +} + +fn main() { + let _: BTreeMap = BTreeMap::new(); + let _: BTreeMap = BTreeMap::new(); + + let _: BTreeMap<_, _> = std::iter::empty::<(String, ())>().collect(); +} diff --git a/src/tools/clippy/tests/ui/zero_sized_btreemap_values.stderr b/src/tools/clippy/tests/ui/zero_sized_btreemap_values.stderr new file mode 100644 index 0000000000000..334d921a9af3f --- /dev/null +++ b/src/tools/clippy/tests/ui/zero_sized_btreemap_values.stderr @@ -0,0 +1,107 @@ +error: map with zero-sized value type + --> $DIR/zero_sized_btreemap_values.rs:5:28 + | +LL | const CONST_NOT_OK: Option> = None; + | ^^^^^^^^^^^^^^^^^^^^ + | + = note: `-D clippy::zero-sized-map-values` implied by `-D warnings` + = help: consider using a set instead + +error: map with zero-sized value type + --> $DIR/zero_sized_btreemap_values.rs:8:30 + | +LL | static STATIC_NOT_OK: Option> = None; + | ^^^^^^^^^^^^^^^^^^^^ + | + = help: consider using a set instead + +error: map with zero-sized value type + --> $DIR/zero_sized_btreemap_values.rs:11:17 + | +LL | type NotOkMap = BTreeMap; + | ^^^^^^^^^^^^^^^^^^^^ + | + = help: consider using a set instead + +error: map with zero-sized value type + --> $DIR/zero_sized_btreemap_values.rs:15:11 + | +LL | NotOk(BTreeMap), + | ^^^^^^^^^^^^^^^^^^^^ + | + = help: consider using a set instead + +error: map with zero-sized value type + --> $DIR/zero_sized_btreemap_values.rs:20:13 + | +LL | not_ok: BTreeMap, + | ^^^^^^^^^^^^^^^^^^^^ + | + = help: consider using a set instead + +error: map with zero-sized value type + --> $DIR/zero_sized_btreemap_values.rs:22:22 + | +LL | also_not_ok: Vec>, + | ^^^^^^^^^^^^^^^^^^^ + | + = help: consider using a set instead + +error: map with zero-sized value type + --> $DIR/zero_sized_btreemap_values.rs:30:30 + | +LL | fn weird_map(&self, map: BTreeMap); + | ^^^^^^^^^^^^^^^^^^^ + | + = help: consider using a set instead + +error: map with zero-sized value type + --> $DIR/zero_sized_btreemap_values.rs:38:25 + | +LL | fn not_ok(&self) -> BTreeMap { + | ^^^^^^^^^^^^^^^^^^^^ + | + = help: consider using a set instead + +error: map with zero-sized value type + --> $DIR/zero_sized_btreemap_values.rs:55:14 + | +LL | fn test(map: BTreeMap, key: &str) -> BTreeMap { + | ^^^^^^^^^^^^^^^^^^^^ + | + = help: consider using a set instead + +error: map with zero-sized value type + --> $DIR/zero_sized_btreemap_values.rs:55:50 + | +LL | fn test(map: BTreeMap, key: &str) -> BTreeMap { + | ^^^^^^^^^^^^^^^^^^^^ + | + = help: consider using a set instead + +error: map with zero-sized value type + --> $DIR/zero_sized_btreemap_values.rs:64:35 + | +LL | let _: BTreeMap = BTreeMap::new(); + | ^^^^^^^^^^^^^ + | + = help: consider using a set instead + +error: map with zero-sized value type + --> $DIR/zero_sized_btreemap_values.rs:64:12 + | +LL | let _: BTreeMap = BTreeMap::new(); + | ^^^^^^^^^^^^^^^^^^^^ + | + = help: consider using a set instead + +error: map with zero-sized value type + --> $DIR/zero_sized_btreemap_values.rs:67:12 + | +LL | let _: BTreeMap<_, _> = std::iter::empty::<(String, ())>().collect(); + | ^^^^^^^^^^^^^^ + | + = help: consider using a set instead + +error: aborting due to 13 previous errors + diff --git a/src/tools/clippy/tests/ui/zero_sized_hashmap_values.rs b/src/tools/clippy/tests/ui/zero_sized_hashmap_values.rs new file mode 100644 index 0000000000000..a1608d863fb52 --- /dev/null +++ b/src/tools/clippy/tests/ui/zero_sized_hashmap_values.rs @@ -0,0 +1,68 @@ +#![warn(clippy::zero_sized_map_values)] +use std::collections::HashMap; + +const CONST_OK: Option> = None; +const CONST_NOT_OK: Option> = None; + +static STATIC_OK: Option> = None; +static STATIC_NOT_OK: Option> = None; + +type OkMap = HashMap; +type NotOkMap = HashMap; + +enum TestEnum { + Ok(HashMap), + NotOk(HashMap), +} + +struct Test { + ok: HashMap, + not_ok: HashMap, + + also_not_ok: Vec>, +} + +trait TestTrait { + type Output; + + fn produce_output() -> Self::Output; + + fn weird_map(&self, map: HashMap); +} + +impl Test { + fn ok(&self) -> HashMap { + todo!() + } + + fn not_ok(&self) -> HashMap { + todo!() + } +} + +impl TestTrait for Test { + type Output = HashMap; + + fn produce_output() -> Self::Output { + todo!(); + } + + fn weird_map(&self, map: HashMap) { + todo!(); + } +} + +fn test(map: HashMap, key: &str) -> HashMap { + todo!(); +} + +fn test2(map: HashMap, key: &str) -> HashMap { + todo!(); +} + +fn main() { + let _: HashMap = HashMap::new(); + let _: HashMap = HashMap::new(); + + let _: HashMap<_, _> = std::iter::empty::<(String, ())>().collect(); +} diff --git a/src/tools/clippy/tests/ui/zero_sized_hashmap_values.stderr b/src/tools/clippy/tests/ui/zero_sized_hashmap_values.stderr new file mode 100644 index 0000000000000..43987b3d01d16 --- /dev/null +++ b/src/tools/clippy/tests/ui/zero_sized_hashmap_values.stderr @@ -0,0 +1,107 @@ +error: map with zero-sized value type + --> $DIR/zero_sized_hashmap_values.rs:5:28 + | +LL | const CONST_NOT_OK: Option> = None; + | ^^^^^^^^^^^^^^^^^^^ + | + = note: `-D clippy::zero-sized-map-values` implied by `-D warnings` + = help: consider using a set instead + +error: map with zero-sized value type + --> $DIR/zero_sized_hashmap_values.rs:8:30 + | +LL | static STATIC_NOT_OK: Option> = None; + | ^^^^^^^^^^^^^^^^^^^ + | + = help: consider using a set instead + +error: map with zero-sized value type + --> $DIR/zero_sized_hashmap_values.rs:11:17 + | +LL | type NotOkMap = HashMap; + | ^^^^^^^^^^^^^^^^^^^ + | + = help: consider using a set instead + +error: map with zero-sized value type + --> $DIR/zero_sized_hashmap_values.rs:15:11 + | +LL | NotOk(HashMap), + | ^^^^^^^^^^^^^^^^^^^ + | + = help: consider using a set instead + +error: map with zero-sized value type + --> $DIR/zero_sized_hashmap_values.rs:20:13 + | +LL | not_ok: HashMap, + | ^^^^^^^^^^^^^^^^^^^ + | + = help: consider using a set instead + +error: map with zero-sized value type + --> $DIR/zero_sized_hashmap_values.rs:22:22 + | +LL | also_not_ok: Vec>, + | ^^^^^^^^^^^^^^^^^^ + | + = help: consider using a set instead + +error: map with zero-sized value type + --> $DIR/zero_sized_hashmap_values.rs:30:30 + | +LL | fn weird_map(&self, map: HashMap); + | ^^^^^^^^^^^^^^^^^^ + | + = help: consider using a set instead + +error: map with zero-sized value type + --> $DIR/zero_sized_hashmap_values.rs:38:25 + | +LL | fn not_ok(&self) -> HashMap { + | ^^^^^^^^^^^^^^^^^^^ + | + = help: consider using a set instead + +error: map with zero-sized value type + --> $DIR/zero_sized_hashmap_values.rs:55:14 + | +LL | fn test(map: HashMap, key: &str) -> HashMap { + | ^^^^^^^^^^^^^^^^^^^ + | + = help: consider using a set instead + +error: map with zero-sized value type + --> $DIR/zero_sized_hashmap_values.rs:55:49 + | +LL | fn test(map: HashMap, key: &str) -> HashMap { + | ^^^^^^^^^^^^^^^^^^^ + | + = help: consider using a set instead + +error: map with zero-sized value type + --> $DIR/zero_sized_hashmap_values.rs:64:34 + | +LL | let _: HashMap = HashMap::new(); + | ^^^^^^^^^^^^ + | + = help: consider using a set instead + +error: map with zero-sized value type + --> $DIR/zero_sized_hashmap_values.rs:64:12 + | +LL | let _: HashMap = HashMap::new(); + | ^^^^^^^^^^^^^^^^^^^ + | + = help: consider using a set instead + +error: map with zero-sized value type + --> $DIR/zero_sized_hashmap_values.rs:67:12 + | +LL | let _: HashMap<_, _> = std::iter::empty::<(String, ())>().collect(); + | ^^^^^^^^^^^^^ + | + = help: consider using a set instead + +error: aborting due to 13 previous errors + diff --git a/src/tools/clippy/triagebot.toml b/src/tools/clippy/triagebot.toml index b7b20b40e68a5..b9549be3a8b63 100644 --- a/src/tools/clippy/triagebot.toml +++ b/src/tools/clippy/triagebot.toml @@ -1,7 +1,7 @@ [relabel] allow-unauthenticated = [ "A-*", "C-*", "E-*", "L-*", "M-*", "O-*", "P-*", "S-*", "T-*", - "good first issue" + "good-first-issue" ] [assign] diff --git a/src/tools/clippy/util/gh-pages/index.html b/src/tools/clippy/util/gh-pages/index.html index e11f2eeba3b32..428708136cb65 100644 --- a/src/tools/clippy/util/gh-pages/index.html +++ b/src/tools/clippy/util/gh-pages/index.html @@ -89,7 +89,7 @@

ALL the Clippy Lints

+ ng-repeat="lint in data | filter:byLevels | filter:byGroups | filter:bySearch | orderBy:'id' track by lint.id">

@@ -215,6 +215,46 @@

return $scope.groups[lint.group]; }; + $scope.bySearch = function (lint, index, array) { + let search_str = $scope.search; + // It can be `null` I haven't missed this value + if (search_str == null || search_str.length == 0) { + return true; + } + search_str = search_str.toLowerCase(); + + // Search by id + let id_search = search_str.trim().replace(/(\-| )/g, "_"); + if (lint.id.includes(id_search)) { + return true; + } + + // Search the description + // The use of `for`-loops instead of `foreach` enables us to return early + let search_lint = (lint, therm) => { + for (const field in lint.docs) { + // Continue if it's not a property + if (!lint.docs.hasOwnProperty(field)) { + continue; + } + + // Return if not found + if (lint.docs[field].toLowerCase().includes(therm)) { + return true; + } + } + return false; + }; + let therms = search_str.split(" "); + for (index = 0; index < therms.length; index++) { + if (!search_lint(lint, therms[index])) { + return false; + } + } + + return true; + } + // Get data $scope.open = {}; $scope.loading = true;