diff --git a/Cargo.lock b/Cargo.lock index 8af442c8f4b..8acb8ef83e5 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -418,6 +418,17 @@ dependencies = [ "serde_json", ] +[[package]] +name = "cargo_metadata" +version = "0.12.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "d5a5f7b42f606b7f23674f6f4d877628350682bc40687d3fae65679a58d55345" +dependencies = [ + "semver 0.11.0", + "serde", + "serde_json", +] + [[package]] name = "cargotest2" version = "0.1.0" @@ -530,15 +541,14 @@ dependencies = [ name = "clippy" version = "0.0.212" dependencies = [ - "cargo_metadata 0.11.1", + "cargo_metadata 0.12.0", "clippy-mini-macro-test", "clippy_lints", "compiletest_rs", "derive-new", - "lazy_static", "rustc-workspace-hack", "rustc_tools_util 0.2.0", - "semver 0.10.0", + "semver 0.11.0", "serde", "tempfile", "tester", @@ -552,14 +562,14 @@ version = "0.2.0" name = "clippy_lints" version = "0.0.212" dependencies = [ - "cargo_metadata 0.11.1", + "cargo_metadata 0.12.0", "if_chain", "itertools 0.9.0", "pulldown-cmark 0.8.0", "quine-mc_cluskey", "quote", "regex-syntax", - "semver 0.10.0", + "semver 0.11.0", "serde", "smallvec 1.4.2", "syn", @@ -4373,7 +4383,7 @@ version = "0.9.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "1d7eb9ef2c18661902cc47e535f9bc51b78acd254da71d375c2f6720d9a40403" dependencies = [ - "semver-parser", + "semver-parser 0.7.0", "serde", ] @@ -4383,7 +4393,17 @@ version = "0.10.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "394cec28fa623e00903caf7ba4fa6fb9a0e260280bb8cdbbba029611108a0190" dependencies = [ - "semver-parser", + "semver-parser 0.7.0", + "serde", +] + +[[package]] +name = "semver" +version = "0.11.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f301af10236f6df4160f7c3f04eec6dbc70ace82d23326abad5edee88801c6b6" +dependencies = [ + "semver-parser 0.10.1", "serde", ] @@ -4393,6 +4413,15 @@ version = "0.7.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "388a1df253eca08550bef6c72392cfe7c30914bf41df5269b68cbd6ff8f570a3" +[[package]] +name = "semver-parser" +version = "0.10.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "42ef146c2ad5e5f4b037cd6ce2ebb775401729b19a82040c1beac9d36c7d1428" +dependencies = [ + "pest", +] + [[package]] name = "serde" version = "1.0.115" @@ -4424,9 +4453,9 @@ dependencies = [ [[package]] name = "serde_json" -version = "1.0.57" +version = "1.0.59" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "164eacbdb13512ec2745fb09d51fd5b22b0d65ed294a1dcf7285a360c80a675c" +checksum = "dcac07dbffa1c65e7f816ab9eba78eb142c6d44410f4eeba1e26e4f5dfa56b95" dependencies = [ "itoa", "ryu", diff --git a/src/tools/clippy/.github/PULL_REQUEST_TEMPLATE.md b/src/tools/clippy/.github/PULL_REQUEST_TEMPLATE.md index 137a7363094..6c92e10522c 100644 --- a/src/tools/clippy/.github/PULL_REQUEST_TEMPLATE.md +++ b/src/tools/clippy/.github/PULL_REQUEST_TEMPLATE.md @@ -12,12 +12,12 @@ your PR is merged. If you added a new lint, here's a checklist for things that will be checked during review or continuous integration. -- [ ] Followed [lint naming conventions][lint_naming] -- [ ] Added passing UI tests (including committed `.stderr` file) -- [ ] `cargo test` passes locally -- [ ] Executed `cargo dev update_lints` -- [ ] Added lint documentation -- [ ] Run `cargo dev fmt` +- \[ ] Followed [lint naming conventions][lint_naming] +- \[ ] Added passing UI tests (including committed `.stderr` file) +- \[ ] `cargo test` passes locally +- \[ ] Executed `cargo dev update_lints` +- \[ ] Added lint documentation +- \[ ] Run `cargo dev fmt` [lint_naming]: https://rust-lang.github.io/rfcs/0344-conventions-galore.html#lints diff --git a/src/tools/clippy/.github/workflows/clippy.yml b/src/tools/clippy/.github/workflows/clippy.yml index 99e371631b1..cf4aa39e49b 100644 --- a/src/tools/clippy/.github/workflows/clippy.yml +++ b/src/tools/clippy/.github/workflows/clippy.yml @@ -36,14 +36,14 @@ jobs: github_token: "${{ secrets.github_token }}" - name: rust-toolchain - uses: actions-rs/toolchain@v1.0.3 + uses: actions-rs/toolchain@v1.0.6 with: toolchain: nightly target: x86_64-unknown-linux-gnu profile: minimal - name: Checkout - uses: actions/checkout@v2.0.0 + uses: actions/checkout@v2.3.3 - name: Run cargo update run: cargo update @@ -63,7 +63,7 @@ jobs: - name: Set LD_LIBRARY_PATH (Linux) run: | SYSROOT=$(rustc --print sysroot) - echo "::set-env name=LD_LIBRARY_PATH::${SYSROOT}/lib${LD_LIBRARY_PATH+:${LD_LIBRARY_PATH}}" + echo "LD_LIBRARY_PATH=${SYSROOT}/lib${LD_LIBRARY_PATH+:${LD_LIBRARY_PATH}}" >> $GITHUB_ENV - name: Build run: cargo build --features deny-warnings diff --git a/src/tools/clippy/.github/workflows/clippy_bors.yml b/src/tools/clippy/.github/workflows/clippy_bors.yml index fd0cd7a1890..7509d90c6c2 100644 --- a/src/tools/clippy/.github/workflows/clippy_bors.yml +++ b/src/tools/clippy/.github/workflows/clippy_bors.yml @@ -11,6 +11,10 @@ env: CARGO_TARGET_DIR: '${{ github.workspace }}/target' NO_FMT_TEST: 1 +defaults: + run: + shell: bash + jobs: changelog: runs-on: ubuntu-latest @@ -20,7 +24,7 @@ jobs: with: github_token: "${{ secrets.github_token }}" - name: Checkout - uses: actions/checkout@v2.0.0 + uses: actions/checkout@v2.3.3 with: ref: ${{ github.ref }} @@ -81,14 +85,14 @@ jobs: if: matrix.host == 'i686-unknown-linux-gnu' - name: rust-toolchain - uses: actions-rs/toolchain@v1.0.3 + uses: actions-rs/toolchain@v1.0.6 with: toolchain: nightly target: ${{ matrix.host }} profile: minimal - name: Checkout - uses: actions/checkout@v2.0.0 + uses: actions/checkout@v2.3.3 - name: Run cargo update run: cargo update @@ -105,14 +109,13 @@ jobs: run: bash setup-toolchain.sh env: HOST_TOOLCHAIN: ${{ matrix.host }} - shell: bash # Run - name: Set LD_LIBRARY_PATH (Linux) if: runner.os == 'Linux' run: | SYSROOT=$(rustc --print sysroot) - echo "::set-env name=LD_LIBRARY_PATH::${SYSROOT}/lib${LD_LIBRARY_PATH+:${LD_LIBRARY_PATH}}" + echo "LD_LIBRARY_PATH=${SYSROOT}/lib${LD_LIBRARY_PATH+:${LD_LIBRARY_PATH}}" >> $GITHUB_ENV - name: Link rustc dylib (MacOS) if: runner.os == 'macOS' run: | @@ -122,41 +125,33 @@ jobs: - name: Set PATH (Windows) if: runner.os == 'Windows' run: | - $sysroot = rustc --print sysroot - $env:PATH += ';' + $sysroot + '\bin' - echo "::set-env name=PATH::$env:PATH" + SYSROOT=$(rustc --print sysroot) + echo "$SYSROOT/bin" >> $GITHUB_PATH - name: Build run: cargo build --features deny-warnings - shell: bash - name: Test run: cargo test --features deny-warnings - shell: bash - name: Test clippy_lints run: cargo test --features deny-warnings - shell: bash working-directory: clippy_lints - name: Test rustc_tools_util run: cargo test --features deny-warnings - shell: bash working-directory: rustc_tools_util - name: Test clippy_dev run: cargo test --features deny-warnings - shell: bash working-directory: clippy_dev - name: Test cargo-clippy run: ../target/debug/cargo-clippy - shell: bash working-directory: clippy_workspace_tests - name: Test clippy-driver run: bash .github/driver.sh - shell: bash env: OS: ${{ runner.os }} @@ -165,7 +160,7 @@ jobs: run: | cargo +nightly install cargo-cache --no-default-features --features ci-autoclean cargo-cache cargo cache - shell: bash + integration_build: needs: changelog runs-on: ubuntu-latest @@ -177,14 +172,14 @@ jobs: github_token: "${{ secrets.github_token }}" - name: rust-toolchain - uses: actions-rs/toolchain@v1.0.3 + uses: actions-rs/toolchain@v1.0.6 with: toolchain: nightly target: x86_64-unknown-linux-gnu profile: minimal - name: Checkout - uses: actions/checkout@v2.0.0 + uses: actions/checkout@v2.3.3 - name: Run cargo update run: cargo update @@ -258,14 +253,14 @@ jobs: github_token: "${{ secrets.github_token }}" - name: rust-toolchain - uses: actions-rs/toolchain@v1.0.3 + uses: actions-rs/toolchain@v1.0.6 with: toolchain: nightly target: x86_64-unknown-linux-gnu profile: minimal - name: Checkout - uses: actions/checkout@v2.0.0 + uses: actions/checkout@v2.3.3 - name: Run cargo update run: cargo update diff --git a/src/tools/clippy/.github/workflows/clippy_dev.yml b/src/tools/clippy/.github/workflows/clippy_dev.yml index ec3b43c2f43..5ee157cf23b 100644 --- a/src/tools/clippy/.github/workflows/clippy_dev.yml +++ b/src/tools/clippy/.github/workflows/clippy_dev.yml @@ -23,7 +23,7 @@ jobs: steps: # Setup - name: rust-toolchain - uses: actions-rs/toolchain@v1.0.3 + uses: actions-rs/toolchain@v1.0.6 with: toolchain: nightly target: x86_64-unknown-linux-gnu @@ -31,7 +31,7 @@ jobs: components: rustfmt - name: Checkout - uses: actions/checkout@v2.0.0 + uses: actions/checkout@v2.3.3 # Run - name: Build diff --git a/src/tools/clippy/.github/workflows/deploy.yml b/src/tools/clippy/.github/workflows/deploy.yml index f542f9b02c1..15aeaf907dc 100644 --- a/src/tools/clippy/.github/workflows/deploy.yml +++ b/src/tools/clippy/.github/workflows/deploy.yml @@ -21,10 +21,10 @@ jobs: steps: # Setup - name: Checkout - uses: actions/checkout@v2.0.0 + uses: actions/checkout@v2.3.3 - name: Checkout - uses: actions/checkout@v2.0.0 + uses: actions/checkout@v2.3.3 with: ref: ${{ env.TARGET_BRANCH }} path: 'out' @@ -34,10 +34,10 @@ jobs: if: startswith(github.ref, 'refs/tags/') run: | TAG=$(basename ${{ github.ref }}) - echo "::set-env name=TAG_NAME::$TAG" + echo "TAG_NAME=$TAG" >> $GITHUB_ENV - name: Set beta to true if: github.ref == 'refs/heads/beta' - run: echo "::set-env name=BETA::true" + run: echo "BETA=true" >> $GITHUB_ENV - name: Use scripts and templates from master branch run: | diff --git a/src/tools/clippy/.github/workflows/remark.yml b/src/tools/clippy/.github/workflows/remark.yml index cc175e8bf24..4f25a86b2e4 100644 --- a/src/tools/clippy/.github/workflows/remark.yml +++ b/src/tools/clippy/.github/workflows/remark.yml @@ -16,10 +16,10 @@ jobs: steps: # Setup - name: Checkout - uses: actions/checkout@v2.0.0 + uses: actions/checkout@v2.3.3 - name: Setup Node.js - uses: actions/setup-node@v1.1.0 + uses: actions/setup-node@v1.4.4 - name: Install remark run: npm install remark-cli remark-lint remark-lint-maximum-line-length remark-preset-lint-recommended diff --git a/src/tools/clippy/CHANGELOG.md b/src/tools/clippy/CHANGELOG.md index 0bd13320dc9..d82f970b8bf 100644 --- a/src/tools/clippy/CHANGELOG.md +++ b/src/tools/clippy/CHANGELOG.md @@ -1796,6 +1796,7 @@ Released 2018-09-13 [`manual_saturating_arithmetic`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_saturating_arithmetic [`manual_strip`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_strip [`manual_swap`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_swap +[`manual_unwrap_or`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_unwrap_or [`many_single_char_names`]: https://rust-lang.github.io/rust-clippy/master/index.html#many_single_char_names [`map_clone`]: https://rust-lang.github.io/rust-clippy/master/index.html#map_clone [`map_entry`]: https://rust-lang.github.io/rust-clippy/master/index.html#map_entry @@ -1892,6 +1893,7 @@ Released 2018-09-13 [`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 [`ptr_arg`]: https://rust-lang.github.io/rust-clippy/master/index.html#ptr_arg +[`ptr_eq`]: https://rust-lang.github.io/rust-clippy/master/index.html#ptr_eq [`ptr_offset_with_cast`]: https://rust-lang.github.io/rust-clippy/master/index.html#ptr_offset_with_cast [`pub_enum_variant_names`]: https://rust-lang.github.io/rust-clippy/master/index.html#pub_enum_variant_names [`question_mark`]: https://rust-lang.github.io/rust-clippy/master/index.html#question_mark @@ -1917,6 +1919,7 @@ Released 2018-09-13 [`rest_pat_in_fully_bound_structs`]: https://rust-lang.github.io/rust-clippy/master/index.html#rest_pat_in_fully_bound_structs [`result_map_or_into_option`]: https://rust-lang.github.io/rust-clippy/master/index.html#result_map_or_into_option [`result_map_unit_fn`]: https://rust-lang.github.io/rust-clippy/master/index.html#result_map_unit_fn +[`result_unit_err`]: https://rust-lang.github.io/rust-clippy/master/index.html#result_unit_err [`reversed_empty_ranges`]: https://rust-lang.github.io/rust-clippy/master/index.html#reversed_empty_ranges [`same_functions_in_if_condition`]: https://rust-lang.github.io/rust-clippy/master/index.html#same_functions_in_if_condition [`same_item_push`]: https://rust-lang.github.io/rust-clippy/master/index.html#same_item_push diff --git a/src/tools/clippy/CONTRIBUTING.md b/src/tools/clippy/CONTRIBUTING.md index 100c9edb367..6494695606c 100644 --- a/src/tools/clippy/CONTRIBUTING.md +++ b/src/tools/clippy/CONTRIBUTING.md @@ -316,8 +316,8 @@ If you have @bors permissions, you can find an overview of the available commands [here][homu_instructions]. [triage]: https://forge.rust-lang.org/release/triage-procedure.html -[l-crash]: https://github.com/rust-lang/rust-clippy/labels/L-crash%20%3Aboom%3A -[l-bug]: https://github.com/rust-lang/rust-clippy/labels/L-bug%20%3Abeetle%3A +[l-crash]: https://github.com/rust-lang/rust-clippy/labels/L-crash +[l-bug]: https://github.com/rust-lang/rust-clippy/labels/L-bug [homu]: https://github.com/rust-lang/homu [homu_instructions]: https://buildbot2.rust-lang.org/homu/ [homu_queue]: https://buildbot2.rust-lang.org/homu/queue/clippy diff --git a/src/tools/clippy/Cargo.toml b/src/tools/clippy/Cargo.toml index c7a3099b8ab..1ddcd18598d 100644 --- a/src/tools/clippy/Cargo.toml +++ b/src/tools/clippy/Cargo.toml @@ -31,16 +31,14 @@ path = "src/driver.rs" # begin automatic update clippy_lints = { version = "0.0.212", path = "clippy_lints" } # end automatic update -semver = "0.10" +semver = "0.11" rustc_tools_util = { version = "0.2.0", path = "rustc_tools_util"} tempfile = { version = "3.1.0", optional = true } -lazy_static = "1.0" [dev-dependencies] -cargo_metadata = "0.11.1" +cargo_metadata = "0.12" compiletest_rs = { version = "0.5.0", features = ["tmp"] } tester = "0.7" -lazy_static = "1.0" clippy-mini-macro-test = { version = "0.2", path = "mini-macro" } serde = { version = "1.0", features = ["derive"] } derive-new = "0.5" diff --git a/src/tools/clippy/README.md b/src/tools/clippy/README.md index 62a8be0abf2..e1b3c84d691 100644 --- a/src/tools/clippy/README.md +++ b/src/tools/clippy/README.md @@ -169,12 +169,33 @@ You can add options to your code to `allow`/`warn`/`deny` Clippy lints: Note: `deny` produces errors instead of warnings. -If you do not want to include your lint levels in your code, you can globally enable/disable lints by passing extra -flags to Clippy during the run: `cargo clippy -- -A clippy::lint_name` will run Clippy with `lint_name` disabled and -`cargo clippy -- -W clippy::lint_name` will run it with that enabled. This also works with lint groups. For example you -can run Clippy with warnings for all lints enabled: `cargo clippy -- -W clippy::pedantic` +If you do not want to include your lint levels in your code, you can globally enable/disable lints +by passing extra flags to Clippy during the run: + +To disable `lint_name`, run + +```terminal +cargo clippy -- -A clippy::lint_name +``` + +And to enable `lint_name`, run + +```terminal +cargo clippy -- -W clippy::lint_name +``` + +This also works with lint groups. For example you +can run Clippy with warnings for all lints enabled: +```terminal +cargo clippy -- -W clippy::pedantic +``` + If you care only about a single lint, you can allow all others and then explicitly reenable -the lint(s) you are interested in: `cargo clippy -- -Aclippy::all -Wclippy::useless_format -Wclippy::...` +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`. ## Contributing diff --git a/src/tools/clippy/clippy_dev/src/update_lints.rs b/src/tools/clippy/clippy_dev/src/update_lints.rs index a9a70929942..556b67e0b37 100644 --- a/src/tools/clippy/clippy_dev/src/update_lints.rs +++ b/src/tools/clippy/clippy_dev/src/update_lints.rs @@ -29,7 +29,7 @@ pub fn run(update_mode: UpdateMode) { false, update_mode == UpdateMode::Change, || { - format!("pub static ref ALL_LINTS: Vec = vec!{:#?};", sorted_usable_lints) + format!("vec!{:#?}", sorted_usable_lints) .lines() .map(ToString::to_string) .collect::>() diff --git a/src/tools/clippy/clippy_lints/Cargo.toml b/src/tools/clippy/clippy_lints/Cargo.toml index fcf817b82c8..d9471d25197 100644 --- a/src/tools/clippy/clippy_lints/Cargo.toml +++ b/src/tools/clippy/clippy_lints/Cargo.toml @@ -17,7 +17,7 @@ keywords = ["clippy", "lint", "plugin"] edition = "2018" [dependencies] -cargo_metadata = "0.11.1" +cargo_metadata = "0.12" if_chain = "1.0.0" itertools = "0.9" pulldown-cmark = { version = "0.8", default-features = false } @@ -27,7 +27,7 @@ serde = { version = "1.0", features = ["derive"] } smallvec = { version = "1", features = ["union"] } toml = "0.5.3" unicode-normalization = "0.1" -semver = "0.10.0" +semver = "0.11" # NOTE: cargo requires serde feat in its url dep # see url = { version = "2.1.0", features = ["serde"] } diff --git a/src/tools/clippy/clippy_lints/src/consts.rs b/src/tools/clippy/clippy_lints/src/consts.rs index 062c9bd2d9e..c5e33b288a9 100644 --- a/src/tools/clippy/clippy_lints/src/consts.rs +++ b/src/tools/clippy/clippy_lints/src/consts.rs @@ -40,6 +40,8 @@ pub enum Constant { Tuple(Vec), /// A raw pointer. RawPtr(u128), + /// A reference + Ref(Box), /// A literal with syntax error. Err(Symbol), } @@ -66,6 +68,7 @@ impl PartialEq for Constant { (&Self::Bool(l), &Self::Bool(r)) => l == r, (&Self::Vec(ref l), &Self::Vec(ref r)) | (&Self::Tuple(ref l), &Self::Tuple(ref r)) => l == r, (&Self::Repeat(ref lv, ref ls), &Self::Repeat(ref rv, ref rs)) => ls == rs && lv == rv, + (&Self::Ref(ref lb), &Self::Ref(ref rb)) => *lb == *rb, // TODO: are there inter-type equalities? _ => false, } @@ -110,6 +113,9 @@ impl Hash for Constant { Self::RawPtr(u) => { u.hash(state); }, + Self::Ref(ref r) => { + r.hash(state); + }, Self::Err(ref s) => { s.hash(state); }, @@ -144,6 +150,7 @@ impl Constant { x => x, } }, + (&Self::Ref(ref lb), &Self::Ref(ref rb)) => Self::partial_cmp(tcx, cmp_type, lb, rb), // TODO: are there any useful inter-type orderings? _ => None, } @@ -239,7 +246,7 @@ impl<'a, 'tcx> ConstEvalLateContext<'a, 'tcx> { ExprKind::Unary(op, ref operand) => self.expr(operand).and_then(|o| match op { UnOp::UnNot => self.constant_not(&o, self.typeck_results.expr_ty(e)), UnOp::UnNeg => self.constant_negate(&o, self.typeck_results.expr_ty(e)), - UnOp::UnDeref => Some(o), + UnOp::UnDeref => Some(if let Constant::Ref(r) = o { *r } else { o }), }), ExprKind::Binary(op, ref left, ref right) => self.binop(op, left, right), ExprKind::Call(ref callee, ref args) => { @@ -269,6 +276,7 @@ impl<'a, 'tcx> ConstEvalLateContext<'a, 'tcx> { } }, ExprKind::Index(ref arr, ref index) => self.index(arr, index), + ExprKind::AddrOf(_, _, ref inner) => self.expr(inner).map(|r| Constant::Ref(Box::new(r))), // TODO: add other expressions. _ => None, } diff --git a/src/tools/clippy/clippy_lints/src/copies.rs b/src/tools/clippy/clippy_lints/src/copies.rs index 10a64769585..6c969c3ead0 100644 --- a/src/tools/clippy/clippy_lints/src/copies.rs +++ b/src/tools/clippy/clippy_lints/src/copies.rs @@ -1,4 +1,4 @@ -use crate::utils::{eq_expr_value, SpanlessEq, SpanlessHash}; +use crate::utils::{eq_expr_value, in_macro, SpanlessEq, SpanlessHash}; use crate::utils::{get_parent_expr, higher, if_sequence, snippet, span_lint_and_note, span_lint_and_then}; use rustc_data_structures::fx::FxHashMap; use rustc_hir::{Arm, Block, Expr, ExprKind, MatchSource, Pat, PatKind}; @@ -220,6 +220,10 @@ fn lint_same_fns_in_if_cond(cx: &LateContext<'_>, conds: &[&Expr<'_>]) { }; let eq: &dyn Fn(&&Expr<'_>, &&Expr<'_>) -> bool = &|&lhs, &rhs| -> bool { + // Do not lint if any expr originates from a macro + if in_macro(lhs.span) || in_macro(rhs.span) { + return false; + } // Do not spawn warning if `IFS_SAME_COND` already produced it. if eq_expr_value(cx, lhs, rhs) { return false; diff --git a/src/tools/clippy/clippy_lints/src/doc.rs b/src/tools/clippy/clippy_lints/src/doc.rs index 62bb70af06e..07f604cf714 100644 --- a/src/tools/clippy/clippy_lints/src/doc.rs +++ b/src/tools/clippy/clippy_lints/src/doc.rs @@ -32,6 +32,11 @@ declare_clippy_lint! { /// **Known problems:** Lots of bad docs won’t be fixed, what the lint checks /// for is limited, and there are still false positives. /// + /// In addition, when writing documentation comments, including `[]` brackets + /// inside a link text would trip the parser. Therfore, documenting link with + /// `[`SmallVec<[T; INLINE_CAPACITY]>`]` and then [`SmallVec<[T; INLINE_CAPACITY]>`]: SmallVec + /// would fail. + /// /// **Examples:** /// ```rust /// /// Do something with the foo_bar parameter. See also @@ -39,6 +44,14 @@ declare_clippy_lint! { /// // ^ `foo_bar` and `that::other::module::foo` should be ticked. /// fn doit(foo_bar: usize) {} /// ``` + /// + /// ```rust + /// // Link text with `[]` brackets should be written as following: + /// /// Consume the array and return the inner + /// /// [`SmallVec<[T; INLINE_CAPACITY]>`][SmallVec]. + /// /// [SmallVec]: SmallVec + /// fn main() {} + /// ``` pub DOC_MARKDOWN, pedantic, "presence of `_`, `::` or camel-case outside backticks in documentation" diff --git a/src/tools/clippy/clippy_lints/src/eq_op.rs b/src/tools/clippy/clippy_lints/src/eq_op.rs index e16ec783fab..3201adbf9a0 100644 --- a/src/tools/clippy/clippy_lints/src/eq_op.rs +++ b/src/tools/clippy/clippy_lints/src/eq_op.rs @@ -1,8 +1,10 @@ use crate::utils::{ - eq_expr_value, implements_trait, in_macro, is_copy, multispan_sugg, snippet, span_lint, span_lint_and_then, + eq_expr_value, higher, implements_trait, in_macro, is_copy, is_expn_of, multispan_sugg, snippet, span_lint, + span_lint_and_then, }; +use if_chain::if_chain; use rustc_errors::Applicability; -use rustc_hir::{BinOp, BinOpKind, BorrowKind, Expr, ExprKind}; +use rustc_hir::{BinOp, BinOpKind, BorrowKind, Expr, ExprKind, StmtKind}; use rustc_lint::{LateContext, LateLintPass}; use rustc_session::{declare_lint_pass, declare_tool_lint}; @@ -23,6 +25,12 @@ declare_clippy_lint! { /// # let x = 1; /// if x + 1 == x + 1 {} /// ``` + /// or + /// ```rust + /// # let a = 3; + /// # let b = 4; + /// assert_eq!(a, a); + /// ``` pub EQ_OP, correctness, "equal operands on both sides of a comparison or bitwise combination (e.g., `x == x`)" @@ -52,9 +60,34 @@ declare_clippy_lint! { declare_lint_pass!(EqOp => [EQ_OP, OP_REF]); +const ASSERT_MACRO_NAMES: [&str; 4] = ["assert_eq", "assert_ne", "debug_assert_eq", "debug_assert_ne"]; + impl<'tcx> LateLintPass<'tcx> for EqOp { #[allow(clippy::similar_names, clippy::too_many_lines)] fn check_expr(&mut self, cx: &LateContext<'tcx>, e: &'tcx Expr<'_>) { + if let ExprKind::Block(ref block, _) = e.kind { + for stmt in block.stmts { + for amn in &ASSERT_MACRO_NAMES { + if_chain! { + if is_expn_of(stmt.span, amn).is_some(); + if let StmtKind::Semi(ref matchexpr) = stmt.kind; + if let Some(macro_args) = higher::extract_assert_macro_args(matchexpr); + if macro_args.len() == 2; + let (lhs, rhs) = (macro_args[0], macro_args[1]); + if eq_expr_value(cx, lhs, rhs); + + then { + span_lint( + cx, + EQ_OP, + lhs.span.to(rhs.span), + &format!("identical args used in this `{}!` macro call", amn), + ); + } + } + } + } + } if let ExprKind::Binary(op, ref left, ref right) = e.kind { if e.span.from_expansion() { return; diff --git a/src/tools/clippy/clippy_lints/src/format.rs b/src/tools/clippy/clippy_lints/src/format.rs index d6541010bca..26da058598e 100644 --- a/src/tools/clippy/clippy_lints/src/format.rs +++ b/src/tools/clippy/clippy_lints/src/format.rs @@ -1,6 +1,6 @@ use crate::utils::paths; use crate::utils::{ - is_expn_of, is_type_diagnostic_item, last_path_segment, match_def_path, match_function_call, snippet, + is_expn_of, is_type_diagnostic_item, last_path_segment, match_def_path, match_function_call, snippet, snippet_opt, span_lint_and_then, }; use if_chain::if_chain; @@ -132,7 +132,11 @@ fn on_new_v1<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) -> Option [], }` if tup.is_empty() { - return Some(format!("{:?}.to_string()", s.as_str())); + if let Some(s_src) = snippet_opt(cx, lit.span) { + // Simulate macro expansion, converting {{ and }} to { and }. + let s_expand = s_src.replace("{{", "{").replace("}}", "}"); + return Some(format!("{}.to_string()", s_expand)) + } } else if s.as_str().is_empty() { return on_argumentv1_new(cx, &tup[0], arms); } diff --git a/src/tools/clippy/clippy_lints/src/functions.rs b/src/tools/clippy/clippy_lints/src/functions.rs index 50b39cf4ea7..fd45a6da61c 100644 --- a/src/tools/clippy/clippy_lints/src/functions.rs +++ b/src/tools/clippy/clippy_lints/src/functions.rs @@ -1,8 +1,9 @@ use crate::utils::{ - attr_by_name, attrs::is_proc_macro, is_must_use_ty, is_trait_impl_item, iter_input_pats, match_def_path, - must_use_attr, qpath_res, return_ty, snippet, snippet_opt, span_lint, span_lint_and_help, span_lint_and_then, - trait_ref_of_method, type_is_unsafe_function, + attr_by_name, attrs::is_proc_macro, is_must_use_ty, is_trait_impl_item, is_type_diagnostic_item, iter_input_pats, + last_path_segment, match_def_path, must_use_attr, qpath_res, return_ty, snippet, snippet_opt, span_lint, + span_lint_and_help, span_lint_and_then, trait_ref_of_method, type_is_unsafe_function, }; +use if_chain::if_chain; use rustc_ast::ast::Attribute; use rustc_data_structures::fx::FxHashSet; use rustc_errors::Applicability; @@ -16,6 +17,7 @@ use rustc_middle::ty::{self, Ty}; use rustc_session::{declare_tool_lint, impl_lint_pass}; use rustc_span::source_map::Span; use rustc_target::spec::abi::Abi; +use rustc_typeck::hir_ty_to_ty; declare_clippy_lint! { /// **What it does:** Checks for functions with too many parameters. @@ -169,6 +171,52 @@ declare_clippy_lint! { "function or method that could take a `#[must_use]` attribute" } +declare_clippy_lint! { + /// **What it does:** Checks for public functions that return a `Result` + /// with an `Err` type of `()`. It suggests using a custom type that + /// implements [`std::error::Error`]. + /// + /// **Why is this bad?** Unit does not implement `Error` and carries no + /// further information about what went wrong. + /// + /// **Known problems:** Of course, this lint assumes that `Result` is used + /// for a fallible operation (which is after all the intended use). However + /// code may opt to (mis)use it as a basic two-variant-enum. In that case, + /// the suggestion is misguided, and the code should use a custom enum + /// instead. + /// + /// **Examples:** + /// ```rust + /// pub fn read_u8() -> Result { Err(()) } + /// ``` + /// should become + /// ```rust,should_panic + /// use std::fmt; + /// + /// #[derive(Debug)] + /// pub struct EndOfStream; + /// + /// impl fmt::Display for EndOfStream { + /// fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + /// write!(f, "End of Stream") + /// } + /// } + /// + /// impl std::error::Error for EndOfStream { } + /// + /// pub fn read_u8() -> Result { Err(EndOfStream) } + ///# fn main() { + ///# read_u8().unwrap(); + ///# } + /// ``` + /// + /// Note that there are crates that simplify creating the error type, e.g. + /// [`thiserror`](https://docs.rs/thiserror). + pub RESULT_UNIT_ERR, + style, + "public function returning `Result` with an `Err` type of `()`" +} + #[derive(Copy, Clone)] pub struct Functions { threshold: u64, @@ -188,6 +236,7 @@ impl_lint_pass!(Functions => [ MUST_USE_UNIT, DOUBLE_MUST_USE, MUST_USE_CANDIDATE, + RESULT_UNIT_ERR, ]); impl<'tcx> LateLintPass<'tcx> for Functions { @@ -233,15 +282,16 @@ impl<'tcx> LateLintPass<'tcx> for Functions { fn check_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx hir::Item<'_>) { let attr = must_use_attr(&item.attrs); if let hir::ItemKind::Fn(ref sig, ref _generics, ref body_id) = item.kind { + let is_public = cx.access_levels.is_exported(item.hir_id); + let fn_header_span = item.span.with_hi(sig.decl.output.span().hi()); + if is_public { + check_result_unit_err(cx, &sig.decl, item.span, fn_header_span); + } if let Some(attr) = attr { - let fn_header_span = item.span.with_hi(sig.decl.output.span().hi()); check_needless_must_use(cx, &sig.decl, item.hir_id, item.span, fn_header_span, attr); return; } - if cx.access_levels.is_exported(item.hir_id) - && !is_proc_macro(cx.sess(), &item.attrs) - && attr_by_name(&item.attrs, "no_mangle").is_none() - { + if is_public && !is_proc_macro(cx.sess(), &item.attrs) && attr_by_name(&item.attrs, "no_mangle").is_none() { check_must_use_candidate( cx, &sig.decl, @@ -257,11 +307,15 @@ impl<'tcx> LateLintPass<'tcx> for Functions { fn check_impl_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx hir::ImplItem<'_>) { if let hir::ImplItemKind::Fn(ref sig, ref body_id) = item.kind { + let is_public = cx.access_levels.is_exported(item.hir_id); + let fn_header_span = item.span.with_hi(sig.decl.output.span().hi()); + if is_public && trait_ref_of_method(cx, item.hir_id).is_none() { + check_result_unit_err(cx, &sig.decl, item.span, fn_header_span); + } let attr = must_use_attr(&item.attrs); if let Some(attr) = attr { - let fn_header_span = item.span.with_hi(sig.decl.output.span().hi()); check_needless_must_use(cx, &sig.decl, item.hir_id, item.span, fn_header_span, attr); - } else if cx.access_levels.is_exported(item.hir_id) + } else if is_public && !is_proc_macro(cx.sess(), &item.attrs) && trait_ref_of_method(cx, item.hir_id).is_none() { @@ -284,18 +338,21 @@ impl<'tcx> LateLintPass<'tcx> for Functions { if sig.header.abi == Abi::Rust { self.check_arg_number(cx, &sig.decl, item.span.with_hi(sig.decl.output.span().hi())); } + let is_public = cx.access_levels.is_exported(item.hir_id); + let fn_header_span = item.span.with_hi(sig.decl.output.span().hi()); + if is_public { + check_result_unit_err(cx, &sig.decl, item.span, fn_header_span); + } let attr = must_use_attr(&item.attrs); if let Some(attr) = attr { - let fn_header_span = item.span.with_hi(sig.decl.output.span().hi()); check_needless_must_use(cx, &sig.decl, item.hir_id, item.span, fn_header_span, attr); } if let hir::TraitFn::Provided(eid) = *eid { let body = cx.tcx.hir().body(eid); Self::check_raw_ptr(cx, sig.header.unsafety, &sig.decl, body, item.hir_id); - if attr.is_none() && cx.access_levels.is_exported(item.hir_id) && !is_proc_macro(cx.sess(), &item.attrs) - { + if attr.is_none() && is_public && !is_proc_macro(cx.sess(), &item.attrs) { check_must_use_candidate( cx, &sig.decl, @@ -411,6 +468,29 @@ impl<'tcx> Functions { } } +fn check_result_unit_err(cx: &LateContext<'_>, decl: &hir::FnDecl<'_>, item_span: Span, fn_header_span: Span) { + if_chain! { + if !in_external_macro(cx.sess(), item_span); + if let hir::FnRetTy::Return(ref ty) = decl.output; + if let hir::TyKind::Path(ref qpath) = ty.kind; + if is_type_diagnostic_item(cx, hir_ty_to_ty(cx.tcx, ty), sym!(result_type)); + if let Some(ref args) = last_path_segment(qpath).args; + if let [_, hir::GenericArg::Type(ref err_ty)] = args.args; + if let hir::TyKind::Tup(t) = err_ty.kind; + if t.is_empty(); + then { + span_lint_and_help( + cx, + RESULT_UNIT_ERR, + fn_header_span, + "this returns a `Result<_, ()>", + None, + "use a custom Error type instead", + ); + } + } +} + fn check_needless_must_use( cx: &LateContext<'_>, decl: &hir::FnDecl<'_>, diff --git a/src/tools/clippy/clippy_lints/src/lib.rs b/src/tools/clippy/clippy_lints/src/lib.rs index 93b5d9e178c..d4d2f92a6a6 100644 --- a/src/tools/clippy/clippy_lints/src/lib.rs +++ b/src/tools/clippy/clippy_lints/src/lib.rs @@ -234,6 +234,7 @@ mod main_recursion; mod manual_async_fn; mod manual_non_exhaustive; mod manual_strip; +mod manual_unwrap_or; mod map_clone; mod map_err_ignore; mod map_identity; @@ -281,6 +282,7 @@ mod path_buf_push_overwrite; mod pattern_type_mismatch; mod precedence; mod ptr; +mod ptr_eq; mod ptr_offset_with_cast; mod question_mark; mod ranges; @@ -581,6 +583,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: &functions::MUST_USE_CANDIDATE, &functions::MUST_USE_UNIT, &functions::NOT_UNSAFE_PTR_ARG_DEREF, + &functions::RESULT_UNIT_ERR, &functions::TOO_MANY_ARGUMENTS, &functions::TOO_MANY_LINES, &future_not_send::FUTURE_NOT_SEND, @@ -638,6 +641,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: &manual_async_fn::MANUAL_ASYNC_FN, &manual_non_exhaustive::MANUAL_NON_EXHAUSTIVE, &manual_strip::MANUAL_STRIP, + &manual_unwrap_or::MANUAL_UNWRAP_OR, &map_clone::MAP_CLONE, &map_err_ignore::MAP_ERR_IGNORE, &map_identity::MAP_IDENTITY, @@ -778,6 +782,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: &ptr::CMP_NULL, &ptr::MUT_FROM_REF, &ptr::PTR_ARG, + &ptr_eq::PTR_EQ, &ptr_offset_with_cast::PTR_OFFSET_WITH_CAST, &question_mark::QUESTION_MARK, &ranges::RANGE_MINUS_ONE, @@ -916,6 +921,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: let verbose_bit_mask_threshold = conf.verbose_bit_mask_threshold; store.register_late_pass(move || box bit_mask::BitMask::new(verbose_bit_mask_threshold)); store.register_late_pass(|| box ptr::Ptr); + store.register_late_pass(|| box ptr_eq::PtrEq); store.register_late_pass(|| box needless_bool::NeedlessBool); store.register_late_pass(|| box needless_bool::BoolComparison); store.register_late_pass(|| box approx_const::ApproxConstant); @@ -1122,6 +1128,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: store.register_late_pass(|| box repeat_once::RepeatOnce); store.register_late_pass(|| box unwrap_in_result::UnwrapInResult); store.register_late_pass(|| box self_assignment::SelfAssignment); + store.register_late_pass(|| box manual_unwrap_or::ManualUnwrapOr); store.register_late_pass(|| box float_equality_without_abs::FloatEqualityWithoutAbs); store.register_late_pass(|| box async_yields_async::AsyncYieldsAsync); store.register_late_pass(|| box manual_strip::ManualStrip); @@ -1324,6 +1331,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: LintId::of(&functions::DOUBLE_MUST_USE), LintId::of(&functions::MUST_USE_UNIT), LintId::of(&functions::NOT_UNSAFE_PTR_ARG_DEREF), + LintId::of(&functions::RESULT_UNIT_ERR), LintId::of(&functions::TOO_MANY_ARGUMENTS), LintId::of(&get_last_with_len::GET_LAST_WITH_LEN), LintId::of(&identity_op::IDENTITY_OP), @@ -1362,6 +1370,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: LintId::of(&manual_async_fn::MANUAL_ASYNC_FN), LintId::of(&manual_non_exhaustive::MANUAL_NON_EXHAUSTIVE), LintId::of(&manual_strip::MANUAL_STRIP), + LintId::of(&manual_unwrap_or::MANUAL_UNWRAP_OR), LintId::of(&map_clone::MAP_CLONE), LintId::of(&map_identity::MAP_IDENTITY), LintId::of(&map_unit_fn::OPTION_MAP_UNIT_FN), @@ -1457,6 +1466,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: LintId::of(&ptr::CMP_NULL), LintId::of(&ptr::MUT_FROM_REF), LintId::of(&ptr::PTR_ARG), + LintId::of(&ptr_eq::PTR_EQ), LintId::of(&ptr_offset_with_cast::PTR_OFFSET_WITH_CAST), LintId::of(&question_mark::QUESTION_MARK), LintId::of(&ranges::RANGE_ZIP_WITH_LEN), @@ -1554,6 +1564,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: LintId::of(&formatting::SUSPICIOUS_UNARY_OP_FORMATTING), LintId::of(&functions::DOUBLE_MUST_USE), LintId::of(&functions::MUST_USE_UNIT), + LintId::of(&functions::RESULT_UNIT_ERR), LintId::of(&if_let_some_result::IF_LET_SOME_RESULT), LintId::of(&inherent_to_string::INHERENT_TO_STRING), LintId::of(&len_zero::LEN_WITHOUT_IS_EMPTY), @@ -1611,6 +1622,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: LintId::of(&panic_unimplemented::PANIC_PARAMS), LintId::of(&ptr::CMP_NULL), LintId::of(&ptr::PTR_ARG), + LintId::of(&ptr_eq::PTR_EQ), LintId::of(&question_mark::QUESTION_MARK), LintId::of(&redundant_field_names::REDUNDANT_FIELD_NAMES), LintId::of(&redundant_static_lifetimes::REDUNDANT_STATIC_LIFETIMES), @@ -1654,6 +1666,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: LintId::of(&loops::MUT_RANGE_BOUND), LintId::of(&loops::WHILE_LET_LOOP), LintId::of(&manual_strip::MANUAL_STRIP), + LintId::of(&manual_unwrap_or::MANUAL_UNWRAP_OR), LintId::of(&map_identity::MAP_IDENTITY), LintId::of(&map_unit_fn::OPTION_MAP_UNIT_FN), LintId::of(&map_unit_fn::RESULT_MAP_UNIT_FN), diff --git a/src/tools/clippy/clippy_lints/src/loops.rs b/src/tools/clippy/clippy_lints/src/loops.rs index 4fdcaca8f60..63d7e3176b1 100644 --- a/src/tools/clippy/clippy_lints/src/loops.rs +++ b/src/tools/clippy/clippy_lints/src/loops.rs @@ -5,9 +5,8 @@ use crate::utils::usage::{is_unused, mutated_variables}; use crate::utils::{ contains_name, get_enclosing_block, get_parent_expr, get_trait_def_id, has_iter_method, higher, implements_trait, is_integer_const, is_no_std_crate, is_refutable, is_type_diagnostic_item, last_path_segment, match_trait_method, - match_type, match_var, multispan_sugg, qpath_res, snippet, snippet_opt, snippet_with_applicability, - snippet_with_macro_callsite, span_lint, span_lint_and_help, span_lint_and_sugg, span_lint_and_then, sugg, - SpanlessEq, + match_type, match_var, multispan_sugg, qpath_res, snippet, snippet_with_applicability, snippet_with_macro_callsite, + span_lint, span_lint_and_help, span_lint_and_sugg, span_lint_and_then, sugg, SpanlessEq, }; use if_chain::if_chain; use rustc_ast::ast; @@ -770,15 +769,28 @@ fn check_for_loop<'tcx>( body: &'tcx Expr<'_>, expr: &'tcx Expr<'_>, ) { - check_for_loop_range(cx, pat, arg, body, expr); + let is_manual_memcpy_triggered = detect_manual_memcpy(cx, pat, arg, body, expr); + if !is_manual_memcpy_triggered { + check_for_loop_range(cx, pat, arg, body, expr); + check_for_loop_explicit_counter(cx, pat, arg, body, expr); + } check_for_loop_arg(cx, pat, arg, expr); - check_for_loop_explicit_counter(cx, pat, arg, body, expr); check_for_loop_over_map_kv(cx, pat, arg, body, expr); check_for_mut_range_bound(cx, arg, body); - detect_manual_memcpy(cx, pat, arg, body, expr); detect_same_item_push(cx, pat, arg, body, expr); } +// this function assumes the given expression is a `for` loop. +fn get_span_of_entire_for_loop(expr: &Expr<'_>) -> Span { + // for some reason this is the only way to get the `Span` + // of the entire `for` loop + if let ExprKind::Match(_, arms, _) = &expr.kind { + arms[0].body.span + } else { + unreachable!() + } +} + fn same_var<'tcx>(cx: &LateContext<'tcx>, expr: &Expr<'_>, var: HirId) -> bool { if_chain! { if let ExprKind::Path(qpath) = &expr.kind; @@ -794,36 +806,131 @@ fn same_var<'tcx>(cx: &LateContext<'tcx>, expr: &Expr<'_>, var: HirId) -> bool { } } +/// a wrapper of `Sugg`. Besides what `Sugg` do, this removes unnecessary `0`; +/// and also, it avoids subtracting a variable from the same one by replacing it with `0`. +/// it exists for the convenience of the overloaded operators while normal functions can do the +/// same. +#[derive(Clone)] +struct MinifyingSugg<'a>(Sugg<'a>); + +impl<'a> MinifyingSugg<'a> { + fn as_str(&self) -> &str { + let Sugg::NonParen(s) | Sugg::MaybeParen(s) | Sugg::BinOp(_, s) = &self.0; + s.as_ref() + } + + fn into_sugg(self) -> Sugg<'a> { + self.0 + } +} + +impl<'a> From> for MinifyingSugg<'a> { + fn from(sugg: Sugg<'a>) -> Self { + Self(sugg) + } +} + +impl std::ops::Add for &MinifyingSugg<'static> { + type Output = MinifyingSugg<'static>; + fn add(self, rhs: &MinifyingSugg<'static>) -> MinifyingSugg<'static> { + match (self.as_str(), rhs.as_str()) { + ("0", _) => rhs.clone(), + (_, "0") => self.clone(), + (_, _) => (&self.0 + &rhs.0).into(), + } + } +} + +impl std::ops::Sub for &MinifyingSugg<'static> { + type Output = MinifyingSugg<'static>; + fn sub(self, rhs: &MinifyingSugg<'static>) -> MinifyingSugg<'static> { + match (self.as_str(), rhs.as_str()) { + (_, "0") => self.clone(), + ("0", _) => (-rhs.0.clone()).into(), + (x, y) if x == y => sugg::ZERO.into(), + (_, _) => (&self.0 - &rhs.0).into(), + } + } +} + +impl std::ops::Add<&MinifyingSugg<'static>> for MinifyingSugg<'static> { + type Output = MinifyingSugg<'static>; + fn add(self, rhs: &MinifyingSugg<'static>) -> MinifyingSugg<'static> { + match (self.as_str(), rhs.as_str()) { + ("0", _) => rhs.clone(), + (_, "0") => self, + (_, _) => (self.0 + &rhs.0).into(), + } + } +} + +impl std::ops::Sub<&MinifyingSugg<'static>> for MinifyingSugg<'static> { + type Output = MinifyingSugg<'static>; + fn sub(self, rhs: &MinifyingSugg<'static>) -> MinifyingSugg<'static> { + match (self.as_str(), rhs.as_str()) { + (_, "0") => self, + ("0", _) => (-rhs.0.clone()).into(), + (x, y) if x == y => sugg::ZERO.into(), + (_, _) => (self.0 - &rhs.0).into(), + } + } +} + +/// a wrapper around `MinifyingSugg`, which carries a operator like currying +/// so that the suggested code become more efficient (e.g. `foo + -bar` `foo - bar`). +struct Offset { + value: MinifyingSugg<'static>, + sign: OffsetSign, +} + #[derive(Clone, Copy)] enum OffsetSign { Positive, Negative, } -struct Offset { - value: String, - sign: OffsetSign, -} - impl Offset { - fn negative(value: String) -> Self { + fn negative(value: Sugg<'static>) -> Self { Self { - value, + value: value.into(), sign: OffsetSign::Negative, } } - fn positive(value: String) -> Self { + fn positive(value: Sugg<'static>) -> Self { Self { - value, + value: value.into(), sign: OffsetSign::Positive, } } + + fn empty() -> Self { + Self::positive(sugg::ZERO) + } } -struct FixedOffsetVar<'hir> { - var: &'hir Expr<'hir>, - offset: Offset, +fn apply_offset(lhs: &MinifyingSugg<'static>, rhs: &Offset) -> MinifyingSugg<'static> { + match rhs.sign { + OffsetSign::Positive => lhs + &rhs.value, + OffsetSign::Negative => lhs - &rhs.value, + } +} + +#[derive(Debug, Clone, Copy)] +enum StartKind<'hir> { + Range, + Counter { initializer: &'hir Expr<'hir> }, +} + +struct IndexExpr<'hir> { + base: &'hir Expr<'hir>, + idx: StartKind<'hir>, + idx_offset: Offset, +} + +struct Start<'hir> { + id: HirId, + kind: StartKind<'hir>, } fn is_slice_like<'tcx>(cx: &LateContext<'tcx>, ty: Ty<'_>) -> bool { @@ -846,14 +953,28 @@ fn fetch_cloned_expr<'tcx>(expr: &'tcx Expr<'tcx>) -> &'tcx Expr<'tcx> { } } -fn get_offset<'tcx>(cx: &LateContext<'tcx>, idx: &Expr<'_>, var: HirId) -> Option { - fn extract_offset<'tcx>(cx: &LateContext<'tcx>, e: &Expr<'_>, var: HirId) -> Option { +fn get_details_from_idx<'tcx>( + cx: &LateContext<'tcx>, + idx: &Expr<'_>, + starts: &[Start<'tcx>], +) -> Option<(StartKind<'tcx>, Offset)> { + fn get_start<'tcx>(cx: &LateContext<'tcx>, e: &Expr<'_>, starts: &[Start<'tcx>]) -> Option> { + starts.iter().find_map(|start| { + if same_var(cx, e, start.id) { + Some(start.kind) + } else { + None + } + }) + } + + fn get_offset<'tcx>(cx: &LateContext<'tcx>, e: &Expr<'_>, starts: &[Start<'tcx>]) -> Option> { match &e.kind { ExprKind::Lit(l) => match l.node { - ast::LitKind::Int(x, _ty) => Some(x.to_string()), + ast::LitKind::Int(x, _ty) => Some(Sugg::NonParen(x.to_string().into())), _ => None, }, - ExprKind::Path(..) if !same_var(cx, e, var) => Some(snippet_opt(cx, e.span).unwrap_or_else(|| "??".into())), + ExprKind::Path(..) if get_start(cx, e, starts).is_none() => Some(Sugg::hir(cx, e, "???")), _ => None, } } @@ -861,55 +982,89 @@ fn get_offset<'tcx>(cx: &LateContext<'tcx>, idx: &Expr<'_>, var: HirId) -> Optio match idx.kind { ExprKind::Binary(op, lhs, rhs) => match op.node { BinOpKind::Add => { - let offset_opt = if same_var(cx, lhs, var) { - extract_offset(cx, rhs, var) - } else if same_var(cx, rhs, var) { - extract_offset(cx, lhs, var) - } else { - None - }; + let offset_opt = get_start(cx, lhs, starts) + .and_then(|s| get_offset(cx, rhs, starts).map(|o| (s, o))) + .or_else(|| get_start(cx, rhs, starts).and_then(|s| get_offset(cx, lhs, starts).map(|o| (s, o)))); - offset_opt.map(Offset::positive) + offset_opt.map(|(s, o)| (s, Offset::positive(o))) + }, + BinOpKind::Sub => { + get_start(cx, lhs, starts).and_then(|s| get_offset(cx, rhs, starts).map(|o| (s, Offset::negative(o)))) }, - BinOpKind::Sub if same_var(cx, lhs, var) => extract_offset(cx, rhs, var).map(Offset::negative), _ => None, }, - ExprKind::Path(..) if same_var(cx, idx, var) => Some(Offset::positive("0".into())), + ExprKind::Path(..) => get_start(cx, idx, starts).map(|s| (s, Offset::empty())), _ => None, } } -fn get_assignments<'tcx>(body: &'tcx Expr<'tcx>) -> impl Iterator, &'tcx Expr<'tcx>)>> { - fn get_assignment<'tcx>(e: &'tcx Expr<'tcx>) -> Option<(&'tcx Expr<'tcx>, &'tcx Expr<'tcx>)> { - if let ExprKind::Assign(lhs, rhs, _) = e.kind { - Some((lhs, rhs)) - } else { - None - } - } - - // This is one of few ways to return different iterators - // derived from: https://stackoverflow.com/questions/29760668/conditionally-iterate-over-one-of-several-possible-iterators/52064434#52064434 - let mut iter_a = None; - let mut iter_b = None; - - if let ExprKind::Block(b, _) = body.kind { - let Block { stmts, expr, .. } = *b; - - iter_a = stmts - .iter() - .filter_map(|stmt| match stmt.kind { - StmtKind::Local(..) | StmtKind::Item(..) => None, - StmtKind::Expr(e) | StmtKind::Semi(e) => Some(e), - }) - .chain(expr.into_iter()) - .map(get_assignment) - .into() +fn get_assignment<'tcx>(e: &'tcx Expr<'tcx>) -> Option<(&'tcx Expr<'tcx>, &'tcx Expr<'tcx>)> { + if let ExprKind::Assign(lhs, rhs, _) = e.kind { + Some((lhs, rhs)) } else { - iter_b = Some(get_assignment(body)) + None } +} - iter_a.into_iter().flatten().chain(iter_b.into_iter()) +/// Get assignments from the given block. +/// The returned iterator yields `None` if no assignment expressions are there, +/// filtering out the increments of the given whitelisted loop counters; +/// because its job is to make sure there's nothing other than assignments and the increments. +fn get_assignments<'a: 'c, 'tcx: 'c, 'c>( + cx: &'a LateContext<'tcx>, + Block { stmts, expr, .. }: &'tcx Block<'tcx>, + loop_counters: &'c [Start<'tcx>], +) -> impl Iterator, &'tcx Expr<'tcx>)>> + 'c { + // As the `filter` and `map` below do different things, I think putting together + // just increases complexity. (cc #3188 and #4193) + #[allow(clippy::filter_map)] + stmts + .iter() + .filter_map(move |stmt| match stmt.kind { + StmtKind::Local(..) | StmtKind::Item(..) => None, + StmtKind::Expr(e) | StmtKind::Semi(e) => Some(e), + }) + .chain((*expr).into_iter()) + .filter(move |e| { + if let ExprKind::AssignOp(_, place, _) = e.kind { + !loop_counters + .iter() + // skip the first item which should be `StartKind::Range` + // this makes it possible to use the slice with `StartKind::Range` in the same iterator loop. + .skip(1) + .any(|counter| same_var(cx, place, counter.id)) + } else { + true + } + }) + .map(get_assignment) +} + +fn get_loop_counters<'a, 'tcx>( + cx: &'a LateContext<'tcx>, + body: &'tcx Block<'tcx>, + expr: &'tcx Expr<'_>, +) -> Option> + 'a> { + // Look for variables that are incremented once per loop iteration. + let mut increment_visitor = IncrementVisitor::new(cx); + walk_block(&mut increment_visitor, body); + + // For each candidate, check the parent block to see if + // it's initialized to zero at the start of the loop. + get_enclosing_block(&cx, expr.hir_id).and_then(|block| { + increment_visitor + .into_results() + .filter_map(move |var_id| { + let mut initialize_visitor = InitializeVisitor::new(cx, expr, var_id); + walk_block(&mut initialize_visitor, block); + + initialize_visitor.get_result().map(|(_, initializer)| Start { + id: var_id, + kind: StartKind::Counter { initializer }, + }) + }) + .into() + }) } fn build_manual_memcpy_suggestion<'tcx>( @@ -917,80 +1072,97 @@ fn build_manual_memcpy_suggestion<'tcx>( start: &Expr<'_>, end: &Expr<'_>, limits: ast::RangeLimits, - dst_var: FixedOffsetVar<'_>, - src_var: FixedOffsetVar<'_>, + dst: &IndexExpr<'_>, + src: &IndexExpr<'_>, ) -> String { - fn print_sum(arg1: &str, arg2: &Offset) -> String { - match (arg1, &arg2.value[..], arg2.sign) { - ("0", "0", _) => "0".into(), - ("0", x, OffsetSign::Positive) | (x, "0", _) => x.into(), - ("0", x, OffsetSign::Negative) => format!("-{}", x), - (x, y, OffsetSign::Positive) => format!("({} + {})", x, y), - (x, y, OffsetSign::Negative) => { - if x == y { - "0".into() - } else { - format!("({} - {})", x, y) - } - }, - } - } - - fn print_offset(start_str: &str, inline_offset: &Offset) -> String { - let offset = print_sum(start_str, inline_offset); + fn print_offset(offset: MinifyingSugg<'static>) -> MinifyingSugg<'static> { if offset.as_str() == "0" { - "".into() + sugg::EMPTY.into() } else { offset } } - let print_limit = |end: &Expr<'_>, offset: Offset, var: &Expr<'_>| { + let print_limit = |end: &Expr<'_>, end_str: &str, base: &Expr<'_>, sugg: MinifyingSugg<'static>| { if_chain! { if let ExprKind::MethodCall(method, _, len_args, _) = end.kind; if method.ident.name == sym!(len); if len_args.len() == 1; if let Some(arg) = len_args.get(0); - if var_def_id(cx, arg) == var_def_id(cx, var); + if var_def_id(cx, arg) == var_def_id(cx, base); then { - match offset.sign { - OffsetSign::Negative => format!("({} - {})", snippet(cx, end.span, ".len()"), offset.value), - OffsetSign::Positive => "".into(), + if sugg.as_str() == end_str { + sugg::EMPTY.into() + } else { + sugg } } else { - let end_str = match limits { + match limits { ast::RangeLimits::Closed => { - let end = sugg::Sugg::hir(cx, end, ""); - format!("{}", end + sugg::ONE) + sugg + &sugg::ONE.into() }, - ast::RangeLimits::HalfOpen => format!("{}", snippet(cx, end.span, "..")), - }; - - print_sum(&end_str, &offset) + ast::RangeLimits::HalfOpen => sugg, + } } } }; - let start_str = snippet(cx, start.span, "").to_string(); - let dst_offset = print_offset(&start_str, &dst_var.offset); - let dst_limit = print_limit(end, dst_var.offset, dst_var.var); - let src_offset = print_offset(&start_str, &src_var.offset); - let src_limit = print_limit(end, src_var.offset, src_var.var); + let start_str = Sugg::hir(cx, start, "").into(); + let end_str: MinifyingSugg<'_> = Sugg::hir(cx, end, "").into(); - let dst_var_name = snippet_opt(cx, dst_var.var.span).unwrap_or_else(|| "???".into()); - let src_var_name = snippet_opt(cx, src_var.var.span).unwrap_or_else(|| "???".into()); + let print_offset_and_limit = |idx_expr: &IndexExpr<'_>| match idx_expr.idx { + StartKind::Range => ( + print_offset(apply_offset(&start_str, &idx_expr.idx_offset)).into_sugg(), + print_limit( + end, + end_str.as_str(), + idx_expr.base, + apply_offset(&end_str, &idx_expr.idx_offset), + ) + .into_sugg(), + ), + StartKind::Counter { initializer } => { + let counter_start = Sugg::hir(cx, initializer, "").into(); + ( + print_offset(apply_offset(&counter_start, &idx_expr.idx_offset)).into_sugg(), + print_limit( + end, + end_str.as_str(), + idx_expr.base, + apply_offset(&end_str, &idx_expr.idx_offset) + &counter_start - &start_str, + ) + .into_sugg(), + ) + }, + }; - let dst = if dst_offset == "" && dst_limit == "" { - dst_var_name + let (dst_offset, dst_limit) = print_offset_and_limit(&dst); + let (src_offset, src_limit) = print_offset_and_limit(&src); + + let dst_base_str = snippet(cx, dst.base.span, "???"); + let src_base_str = snippet(cx, src.base.span, "???"); + + let dst = if dst_offset == sugg::EMPTY && dst_limit == sugg::EMPTY { + dst_base_str } else { - format!("{}[{}..{}]", dst_var_name, dst_offset, dst_limit) + format!( + "{}[{}..{}]", + dst_base_str, + dst_offset.maybe_par(), + dst_limit.maybe_par() + ) + .into() }; format!( - "{}.clone_from_slice(&{}[{}..{}])", - dst, src_var_name, src_offset, src_limit + "{}.clone_from_slice(&{}[{}..{}]);", + dst, + src_base_str, + src_offset.maybe_par(), + src_limit.maybe_par() ) } + /// Checks for for loops that sequentially copy items from one slice-like /// object to another. fn detect_manual_memcpy<'tcx>( @@ -999,7 +1171,7 @@ fn detect_manual_memcpy<'tcx>( arg: &'tcx Expr<'_>, body: &'tcx Expr<'_>, expr: &'tcx Expr<'_>, -) { +) -> bool { if let Some(higher::Range { start: Some(start), end: Some(end), @@ -1008,32 +1180,53 @@ fn detect_manual_memcpy<'tcx>( { // the var must be a single name if let PatKind::Binding(_, canonical_id, _, _) = pat.kind { - // The only statements in the for loops can be indexed assignments from - // indexed retrievals. - let big_sugg = get_assignments(body) + let mut starts = vec![Start { + id: canonical_id, + kind: StartKind::Range, + }]; + + // This is one of few ways to return different iterators + // derived from: https://stackoverflow.com/questions/29760668/conditionally-iterate-over-one-of-several-possible-iterators/52064434#52064434 + let mut iter_a = None; + let mut iter_b = None; + + if let ExprKind::Block(block, _) = body.kind { + if let Some(loop_counters) = get_loop_counters(cx, block, expr) { + starts.extend(loop_counters); + } + iter_a = Some(get_assignments(cx, block, &starts)); + } else { + iter_b = Some(get_assignment(body)); + } + + let assignments = iter_a.into_iter().flatten().chain(iter_b.into_iter()); + + let big_sugg = assignments + // The only statements in the for loops can be indexed assignments from + // indexed retrievals (except increments of loop counters). .map(|o| { o.and_then(|(lhs, rhs)| { let rhs = fetch_cloned_expr(rhs); if_chain! { - if let ExprKind::Index(seqexpr_left, idx_left) = lhs.kind; - if let ExprKind::Index(seqexpr_right, idx_right) = rhs.kind; - if is_slice_like(cx, cx.typeck_results().expr_ty(seqexpr_left)) - && is_slice_like(cx, cx.typeck_results().expr_ty(seqexpr_right)); - if let Some(offset_left) = get_offset(cx, &idx_left, canonical_id); - if let Some(offset_right) = get_offset(cx, &idx_right, canonical_id); + if let ExprKind::Index(base_left, idx_left) = lhs.kind; + if let ExprKind::Index(base_right, idx_right) = rhs.kind; + if is_slice_like(cx, cx.typeck_results().expr_ty(base_left)) + && is_slice_like(cx, cx.typeck_results().expr_ty(base_right)); + if let Some((start_left, offset_left)) = get_details_from_idx(cx, &idx_left, &starts); + if let Some((start_right, offset_right)) = get_details_from_idx(cx, &idx_right, &starts); // Source and destination must be different - if var_def_id(cx, seqexpr_left) != var_def_id(cx, seqexpr_right); + if var_def_id(cx, base_left) != var_def_id(cx, base_right); then { - Some((FixedOffsetVar { var: seqexpr_left, offset: offset_left }, - FixedOffsetVar { var: seqexpr_right, offset: offset_right })) + Some((IndexExpr { base: base_left, idx: start_left, idx_offset: offset_left }, + IndexExpr { base: base_right, idx: start_right, idx_offset: offset_right })) } else { None } } }) }) - .map(|o| o.map(|(dst, src)| build_manual_memcpy_suggestion(cx, start, end, limits, dst, src))) + .map(|o| o.map(|(dst, src)| build_manual_memcpy_suggestion(cx, start, end, limits, &dst, &src))) .collect::>>() .filter(|v| !v.is_empty()) .map(|v| v.join("\n ")); @@ -1042,15 +1235,17 @@ fn detect_manual_memcpy<'tcx>( span_lint_and_sugg( cx, MANUAL_MEMCPY, - expr.span, + get_span_of_entire_for_loop(expr), "it looks like you're manually copying between slices", "try replacing the loop by", big_sugg, Applicability::Unspecified, ); + return true; } } } + false } // Scans the body of the for loop and determines whether lint should be given @@ -1533,6 +1728,9 @@ fn check_arg_type(cx: &LateContext<'_>, pat: &Pat<'_>, arg: &Expr<'_>) { } } +// To trigger the EXPLICIT_COUNTER_LOOP lint, a variable must be +// incremented exactly once in the loop body, and initialized to zero +// at the start of the loop. fn check_for_loop_explicit_counter<'tcx>( cx: &LateContext<'tcx>, pat: &'tcx Pat<'_>, @@ -1541,40 +1739,23 @@ fn check_for_loop_explicit_counter<'tcx>( expr: &'tcx Expr<'_>, ) { // Look for variables that are incremented once per loop iteration. - let mut visitor = IncrementVisitor { - cx, - states: FxHashMap::default(), - depth: 0, - done: false, - }; - walk_expr(&mut visitor, body); + let mut increment_visitor = IncrementVisitor::new(cx); + walk_expr(&mut increment_visitor, body); // For each candidate, check the parent block to see if // it's initialized to zero at the start of the loop. if let Some(block) = get_enclosing_block(&cx, expr.hir_id) { - for (id, _) in visitor.states.iter().filter(|&(_, v)| *v == VarState::IncrOnce) { - let mut visitor2 = InitializeVisitor { - cx, - end_expr: expr, - var_id: *id, - state: VarState::IncrOnce, - name: None, - depth: 0, - past_loop: false, - }; - walk_block(&mut visitor2, block); + for id in increment_visitor.into_results() { + let mut initialize_visitor = InitializeVisitor::new(cx, expr, id); + walk_block(&mut initialize_visitor, block); - if visitor2.state == VarState::Warn { - if let Some(name) = visitor2.name { + if_chain! { + if let Some((name, initializer)) = initialize_visitor.get_result(); + if is_integer_const(cx, initializer, 0); + then { let mut applicability = Applicability::MachineApplicable; - // for some reason this is the only way to get the `Span` - // of the entire `for` loop - let for_span = if let ExprKind::Match(_, arms, _) = &expr.kind { - arms[0].body.span - } else { - unreachable!() - }; + let for_span = get_span_of_entire_for_loop(expr); span_lint_and_sugg( cx, @@ -2127,26 +2308,42 @@ fn is_simple_break_expr(expr: &Expr<'_>) -> bool { } } -// To trigger the EXPLICIT_COUNTER_LOOP lint, a variable must be -// incremented exactly once in the loop body, and initialized to zero -// at the start of the loop. #[derive(Debug, PartialEq)] -enum VarState { +enum IncrementVisitorVarState { Initial, // Not examined yet IncrOnce, // Incremented exactly once, may be a loop counter - Declared, // Declared but not (yet) initialized to zero - Warn, DontWarn, } /// Scan a for loop for variables that are incremented exactly once and not used after that. struct IncrementVisitor<'a, 'tcx> { - cx: &'a LateContext<'tcx>, // context reference - states: FxHashMap, // incremented variables - depth: u32, // depth of conditional expressions + cx: &'a LateContext<'tcx>, // context reference + states: FxHashMap, // incremented variables + depth: u32, // depth of conditional expressions done: bool, } +impl<'a, 'tcx> IncrementVisitor<'a, 'tcx> { + fn new(cx: &'a LateContext<'tcx>) -> Self { + Self { + cx, + states: FxHashMap::default(), + depth: 0, + done: false, + } + } + + fn into_results(self) -> impl Iterator { + self.states.into_iter().filter_map(|(id, state)| { + if state == IncrementVisitorVarState::IncrOnce { + Some(id) + } else { + None + } + }) + } +} + impl<'a, 'tcx> Visitor<'tcx> for IncrementVisitor<'a, 'tcx> { type Map = Map<'tcx>; @@ -2158,85 +2355,118 @@ impl<'a, 'tcx> Visitor<'tcx> for IncrementVisitor<'a, 'tcx> { // If node is a variable if let Some(def_id) = var_def_id(self.cx, expr) { if let Some(parent) = get_parent_expr(self.cx, expr) { - let state = self.states.entry(def_id).or_insert(VarState::Initial); - if *state == VarState::IncrOnce { - *state = VarState::DontWarn; + let state = self.states.entry(def_id).or_insert(IncrementVisitorVarState::Initial); + if *state == IncrementVisitorVarState::IncrOnce { + *state = IncrementVisitorVarState::DontWarn; return; } match parent.kind { ExprKind::AssignOp(op, ref lhs, ref rhs) => { if lhs.hir_id == expr.hir_id { - if op.node == BinOpKind::Add && is_integer_const(self.cx, rhs, 1) { - *state = match *state { - VarState::Initial if self.depth == 0 => VarState::IncrOnce, - _ => VarState::DontWarn, - }; + *state = if op.node == BinOpKind::Add + && is_integer_const(self.cx, rhs, 1) + && *state == IncrementVisitorVarState::Initial + && self.depth == 0 + { + IncrementVisitorVarState::IncrOnce } else { - // Assigned some other value - *state = VarState::DontWarn; - } + // Assigned some other value or assigned multiple times + IncrementVisitorVarState::DontWarn + }; } }, - ExprKind::Assign(ref lhs, _, _) if lhs.hir_id == expr.hir_id => *state = VarState::DontWarn, + ExprKind::Assign(ref lhs, _, _) if lhs.hir_id == expr.hir_id => { + *state = IncrementVisitorVarState::DontWarn + }, ExprKind::AddrOf(BorrowKind::Ref, mutability, _) if mutability == Mutability::Mut => { - *state = VarState::DontWarn + *state = IncrementVisitorVarState::DontWarn }, _ => (), } } + + walk_expr(self, expr); } else if is_loop(expr) || is_conditional(expr) { self.depth += 1; walk_expr(self, expr); self.depth -= 1; - return; } else if let ExprKind::Continue(_) = expr.kind { self.done = true; - return; + } else { + walk_expr(self, expr); } - walk_expr(self, expr); } fn nested_visit_map(&mut self) -> NestedVisitorMap { NestedVisitorMap::None } } -/// Checks whether a variable is initialized to zero at the start of a loop. +enum InitializeVisitorState<'hir> { + Initial, // Not examined yet + Declared(Symbol), // Declared but not (yet) initialized + Initialized { + name: Symbol, + initializer: &'hir Expr<'hir>, + }, + DontWarn, +} + +/// Checks whether a variable is initialized at the start of a loop and not modified +/// and used after the loop. struct InitializeVisitor<'a, 'tcx> { cx: &'a LateContext<'tcx>, // context reference end_expr: &'tcx Expr<'tcx>, // the for loop. Stop scanning here. var_id: HirId, - state: VarState, - name: Option, + state: InitializeVisitorState<'tcx>, depth: u32, // depth of conditional expressions past_loop: bool, } +impl<'a, 'tcx> InitializeVisitor<'a, 'tcx> { + fn new(cx: &'a LateContext<'tcx>, end_expr: &'tcx Expr<'tcx>, var_id: HirId) -> Self { + Self { + cx, + end_expr, + var_id, + state: InitializeVisitorState::Initial, + depth: 0, + past_loop: false, + } + } + + fn get_result(&self) -> Option<(Symbol, &'tcx Expr<'tcx>)> { + if let InitializeVisitorState::Initialized { name, initializer } = self.state { + Some((name, initializer)) + } else { + None + } + } +} + impl<'a, 'tcx> Visitor<'tcx> for InitializeVisitor<'a, 'tcx> { type Map = Map<'tcx>; fn visit_stmt(&mut self, stmt: &'tcx Stmt<'_>) { // Look for declarations of the variable - if let StmtKind::Local(ref local) = stmt.kind { - if local.pat.hir_id == self.var_id { - if let PatKind::Binding(.., ident, _) = local.pat.kind { - self.name = Some(ident.name); - - self.state = local.init.as_ref().map_or(VarState::Declared, |init| { - if is_integer_const(&self.cx, init, 0) { - VarState::Warn - } else { - VarState::Declared - } - }) - } + if_chain! { + if let StmtKind::Local(ref local) = stmt.kind; + if local.pat.hir_id == self.var_id; + if let PatKind::Binding(.., ident, _) = local.pat.kind; + then { + self.state = local.init.map_or(InitializeVisitorState::Declared(ident.name), |init| { + InitializeVisitorState::Initialized { + initializer: init, + name: ident.name, + } + }) } } walk_stmt(self, stmt); } fn visit_expr(&mut self, expr: &'tcx Expr<'_>) { - if self.state == VarState::DontWarn { + if matches!(self.state, InitializeVisitorState::DontWarn) { return; } if expr.hir_id == self.end_expr.hir_id { @@ -2245,45 +2475,51 @@ impl<'a, 'tcx> Visitor<'tcx> for InitializeVisitor<'a, 'tcx> { } // No need to visit expressions before the variable is // declared - if self.state == VarState::IncrOnce { + if matches!(self.state, InitializeVisitorState::Initial) { return; } // If node is the desired variable, see how it's used if var_def_id(self.cx, expr) == Some(self.var_id) { + if self.past_loop { + self.state = InitializeVisitorState::DontWarn; + return; + } + if let Some(parent) = get_parent_expr(self.cx, expr) { match parent.kind { ExprKind::AssignOp(_, ref lhs, _) if lhs.hir_id == expr.hir_id => { - self.state = VarState::DontWarn; + self.state = InitializeVisitorState::DontWarn; }, ExprKind::Assign(ref lhs, ref rhs, _) if lhs.hir_id == expr.hir_id => { - self.state = if is_integer_const(&self.cx, rhs, 0) && self.depth == 0 { - VarState::Warn - } else { - VarState::DontWarn + self.state = if_chain! { + if self.depth == 0; + if let InitializeVisitorState::Declared(name) + | InitializeVisitorState::Initialized { name, ..} = self.state; + then { + InitializeVisitorState::Initialized { initializer: rhs, name } + } else { + InitializeVisitorState::DontWarn + } } }, ExprKind::AddrOf(BorrowKind::Ref, mutability, _) if mutability == Mutability::Mut => { - self.state = VarState::DontWarn + self.state = InitializeVisitorState::DontWarn }, _ => (), } } - if self.past_loop { - self.state = VarState::DontWarn; - return; - } + walk_expr(self, expr); } else if !self.past_loop && is_loop(expr) { - self.state = VarState::DontWarn; - return; + self.state = InitializeVisitorState::DontWarn; } else if is_conditional(expr) { self.depth += 1; walk_expr(self, expr); self.depth -= 1; - return; + } else { + walk_expr(self, expr); } - walk_expr(self, expr); } fn nested_visit_map(&mut self) -> NestedVisitorMap { diff --git a/src/tools/clippy/clippy_lints/src/manual_unwrap_or.rs b/src/tools/clippy/clippy_lints/src/manual_unwrap_or.rs new file mode 100644 index 00000000000..ddb8cc25077 --- /dev/null +++ b/src/tools/clippy/clippy_lints/src/manual_unwrap_or.rs @@ -0,0 +1,104 @@ +use crate::consts::constant_simple; +use crate::utils; +use if_chain::if_chain; +use rustc_errors::Applicability; +use rustc_hir::{def, Arm, Expr, ExprKind, PatKind, QPath}; +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}; + +declare_clippy_lint! { + /// **What it does:** + /// Finds patterns that reimplement `Option::unwrap_or`. + /// + /// **Why is this bad?** + /// Concise code helps focusing on behavior instead of boilerplate. + /// + /// **Known problems:** None. + /// + /// **Example:** + /// ```rust + /// let foo: Option = None; + /// match foo { + /// Some(v) => v, + /// None => 1, + /// }; + /// ``` + /// + /// Use instead: + /// ```rust + /// let foo: Option = None; + /// foo.unwrap_or(1); + /// ``` + pub MANUAL_UNWRAP_OR, + complexity, + "finds patterns that can be encoded more concisely with `Option::unwrap_or`" +} + +declare_lint_pass!(ManualUnwrapOr => [MANUAL_UNWRAP_OR]); + +impl LateLintPass<'_> for ManualUnwrapOr { + fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) { + if in_external_macro(cx.sess(), expr.span) { + return; + } + lint_option_unwrap_or_case(cx, expr); + } +} + +fn lint_option_unwrap_or_case<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) { + fn applicable_none_arm<'a>(arms: &'a [Arm<'a>]) -> Option<&'a Arm<'a>> { + if_chain! { + if arms.len() == 2; + if arms.iter().all(|arm| arm.guard.is_none()); + if let Some((idx, none_arm)) = arms.iter().enumerate().find(|(_, arm)| + if let PatKind::Path(ref qpath) = arm.pat.kind { + utils::match_qpath(qpath, &utils::paths::OPTION_NONE) + } else { + false + } + ); + let some_arm = &arms[1 - idx]; + if let PatKind::TupleStruct(ref some_qpath, &[some_binding], _) = some_arm.pat.kind; + if utils::match_qpath(some_qpath, &utils::paths::OPTION_SOME); + if let PatKind::Binding(_, binding_hir_id, ..) = some_binding.kind; + if let ExprKind::Path(QPath::Resolved(_, body_path)) = some_arm.body.kind; + if let def::Res::Local(body_path_hir_id) = body_path.res; + if body_path_hir_id == binding_hir_id; + if !utils::usage::contains_return_break_continue_macro(none_arm.body); + then { + Some(none_arm) + } else { + None + } + } + } + + if_chain! { + if let ExprKind::Match(scrutinee, match_arms, _) = expr.kind; + let ty = cx.typeck_results().expr_ty(scrutinee); + if utils::is_type_diagnostic_item(cx, ty, sym!(option_type)); + if let Some(none_arm) = applicable_none_arm(match_arms); + if let Some(scrutinee_snippet) = utils::snippet_opt(cx, scrutinee.span); + if let Some(none_body_snippet) = utils::snippet_opt(cx, none_arm.body.span); + if let Some(indent) = utils::indent_of(cx, expr.span); + if constant_simple(cx, cx.typeck_results(), none_arm.body).is_some(); + then { + let reindented_none_body = + utils::reindent_multiline(none_body_snippet.into(), true, Some(indent)); + utils::span_lint_and_sugg( + cx, + MANUAL_UNWRAP_OR, expr.span, + "this pattern reimplements `Option::unwrap_or`", + "replace with", + format!( + "{}.unwrap_or({})", + scrutinee_snippet, + reindented_none_body, + ), + Applicability::MachineApplicable, + ); + } + } +} diff --git a/src/tools/clippy/clippy_lints/src/mut_key.rs b/src/tools/clippy/clippy_lints/src/mut_key.rs index 8a2dbdc50ea..4525b12689f 100644 --- a/src/tools/clippy/clippy_lints/src/mut_key.rs +++ b/src/tools/clippy/clippy_lints/src/mut_key.rs @@ -1,6 +1,7 @@ use crate::utils::{match_def_path, paths, span_lint, trait_ref_of_method}; use rustc_hir as hir; use rustc_lint::{LateContext, LateLintPass}; +use rustc_middle::ty::TypeFoldable; use rustc_middle::ty::{Adt, Array, RawPtr, Ref, Slice, Tuple, Ty, TypeAndMut}; use rustc_session::{declare_lint_pass, declare_tool_lint}; use rustc_span::source_map::Span; @@ -120,7 +121,11 @@ fn is_mutable_type<'tcx>(cx: &LateContext<'tcx>, ty: Ty<'tcx>, span: Span) -> bo size.try_eval_usize(cx.tcx, cx.param_env).map_or(true, |u| u != 0) && is_mutable_type(cx, inner_ty, span) }, Tuple(..) => ty.tuple_fields().any(|ty| is_mutable_type(cx, ty, span)), - Adt(..) => cx.tcx.layout_of(cx.param_env.and(ty)).is_ok() && !ty.is_freeze(cx.tcx.at(span), cx.param_env), + Adt(..) => { + cx.tcx.layout_of(cx.param_env.and(ty)).is_ok() + && !ty.has_escaping_bound_vars() + && !ty.is_freeze(cx.tcx.at(span), cx.param_env) + }, _ => false, } } diff --git a/src/tools/clippy/clippy_lints/src/mutable_debug_assertion.rs b/src/tools/clippy/clippy_lints/src/mutable_debug_assertion.rs index cc635c2a202..76417aa7ed0 100644 --- a/src/tools/clippy/clippy_lints/src/mutable_debug_assertion.rs +++ b/src/tools/clippy/clippy_lints/src/mutable_debug_assertion.rs @@ -1,7 +1,6 @@ -use crate::utils::{is_direct_expn_of, span_lint}; -use if_chain::if_chain; +use crate::utils::{higher, is_direct_expn_of, span_lint}; use rustc_hir::intravisit::{walk_expr, NestedVisitorMap, Visitor}; -use rustc_hir::{BorrowKind, Expr, ExprKind, MatchSource, Mutability, StmtKind, UnOp}; +use rustc_hir::{BorrowKind, Expr, ExprKind, MatchSource, Mutability}; use rustc_lint::{LateContext, LateLintPass}; use rustc_middle::hir::map::Map; use rustc_middle::ty; @@ -39,66 +38,23 @@ impl<'tcx> LateLintPass<'tcx> for DebugAssertWithMutCall { fn check_expr(&mut self, cx: &LateContext<'tcx>, e: &'tcx Expr<'_>) { for dmn in &DEBUG_MACRO_NAMES { if is_direct_expn_of(e.span, dmn).is_some() { - if let Some(span) = extract_call(cx, e) { - span_lint( - cx, - DEBUG_ASSERT_WITH_MUT_CALL, - span, - &format!("do not call a function with mutable arguments inside of `{}!`", dmn), - ); - } - } - } - } -} - -//HACK(hellow554): remove this when #4694 is implemented -fn extract_call<'tcx>(cx: &LateContext<'tcx>, e: &'tcx Expr<'_>) -> Option { - if_chain! { - if let ExprKind::Block(ref block, _) = e.kind; - if block.stmts.len() == 1; - if let StmtKind::Semi(ref matchexpr) = block.stmts[0].kind; - then { - // debug_assert - if_chain! { - if let ExprKind::Match(ref ifclause, _, _) = matchexpr.kind; - if let ExprKind::DropTemps(ref droptmp) = ifclause.kind; - if let ExprKind::Unary(UnOp::UnNot, ref condition) = droptmp.kind; - then { - let mut visitor = MutArgVisitor::new(cx); - visitor.visit_expr(condition); - return visitor.expr_span(); - } - } - - // debug_assert_{eq,ne} - if_chain! { - if let ExprKind::Block(ref matchblock, _) = matchexpr.kind; - if let Some(ref matchheader) = matchblock.expr; - if let ExprKind::Match(ref headerexpr, _, _) = matchheader.kind; - if let ExprKind::Tup(ref conditions) = headerexpr.kind; - if conditions.len() == 2; - then { - if let ExprKind::AddrOf(BorrowKind::Ref, _, ref lhs) = conditions[0].kind { + if let Some(macro_args) = higher::extract_assert_macro_args(e) { + for arg in macro_args { let mut visitor = MutArgVisitor::new(cx); - visitor.visit_expr(lhs); + visitor.visit_expr(arg); if let Some(span) = visitor.expr_span() { - return Some(span); - } - } - if let ExprKind::AddrOf(BorrowKind::Ref, _, ref rhs) = conditions[1].kind { - let mut visitor = MutArgVisitor::new(cx); - visitor.visit_expr(rhs); - if let Some(span) = visitor.expr_span() { - return Some(span); + span_lint( + cx, + DEBUG_ASSERT_WITH_MUT_CALL, + span, + &format!("do not call a function with mutable arguments inside of `{}!`", dmn), + ); } } } } } } - - None } struct MutArgVisitor<'a, 'tcx> { diff --git a/src/tools/clippy/clippy_lints/src/option_if_let_else.rs b/src/tools/clippy/clippy_lints/src/option_if_let_else.rs index 4a3eb9c983a..eb7624b25a3 100644 --- a/src/tools/clippy/clippy_lints/src/option_if_let_else.rs +++ b/src/tools/clippy/clippy_lints/src/option_if_let_else.rs @@ -5,22 +5,20 @@ use crate::utils::{is_type_diagnostic_item, paths, span_lint_and_sugg}; use if_chain::if_chain; use rustc_errors::Applicability; -use rustc_hir::intravisit::{NestedVisitorMap, Visitor}; use rustc_hir::{Arm, BindingAnnotation, Block, Expr, ExprKind, MatchSource, Mutability, PatKind, UnOp}; use rustc_lint::{LateContext, LateLintPass}; -use rustc_middle::hir::map::Map; use rustc_session::{declare_lint_pass, declare_tool_lint}; declare_clippy_lint! { /// **What it does:** - /// Lints usage of `if let Some(v) = ... { y } else { x }` which is more + /// Lints usage of `if let Some(v) = ... { y } else { x }` which is more /// idiomatically done with `Option::map_or` (if the else bit is a pure /// expression) or `Option::map_or_else` (if the else bit is an impure - /// expresion). + /// expression). /// /// **Why is this bad?** /// Using the dedicated functions of the Option type is clearer and - /// more concise than an if let expression. + /// more concise than an `if let` expression. /// /// **Known problems:** /// This lint uses a deliberately conservative metric for checking @@ -84,53 +82,6 @@ struct OptionIfLetElseOccurence { wrap_braces: bool, } -struct ReturnBreakContinueMacroVisitor { - seen_return_break_continue: bool, -} - -impl ReturnBreakContinueMacroVisitor { - fn new() -> ReturnBreakContinueMacroVisitor { - ReturnBreakContinueMacroVisitor { - seen_return_break_continue: false, - } - } -} - -impl<'tcx> Visitor<'tcx> for ReturnBreakContinueMacroVisitor { - type Map = Map<'tcx>; - fn nested_visit_map(&mut self) -> NestedVisitorMap { - NestedVisitorMap::None - } - - fn visit_expr(&mut self, ex: &'tcx Expr<'tcx>) { - if self.seen_return_break_continue { - // No need to look farther if we've already seen one of them - return; - } - match &ex.kind { - ExprKind::Ret(..) | ExprKind::Break(..) | ExprKind::Continue(..) => { - self.seen_return_break_continue = true; - }, - // Something special could be done here to handle while or for loop - // desugaring, as this will detect a break if there's a while loop - // or a for loop inside the expression. - _ => { - if utils::in_macro(ex.span) { - self.seen_return_break_continue = true; - } else { - rustc_hir::intravisit::walk_expr(self, ex); - } - }, - } - } -} - -fn contains_return_break_continue_macro(expression: &Expr<'_>) -> bool { - let mut recursive_visitor = ReturnBreakContinueMacroVisitor::new(); - recursive_visitor.visit_expr(expression); - recursive_visitor.seen_return_break_continue -} - /// Extracts the body of a given arm. If the arm contains only an expression, /// then it returns the expression. Otherwise, it returns the entire block fn extract_body_from_arm<'a>(arm: &'a Arm<'a>) -> Option<&'a Expr<'a>> { @@ -208,8 +159,8 @@ fn detect_option_if_let_else<'tcx>( if let PatKind::TupleStruct(struct_qpath, &[inner_pat], _) = &arms[0].pat.kind; if utils::match_qpath(struct_qpath, &paths::OPTION_SOME); if let PatKind::Binding(bind_annotation, _, id, _) = &inner_pat.kind; - if !contains_return_break_continue_macro(arms[0].body); - if !contains_return_break_continue_macro(arms[1].body); + if !utils::usage::contains_return_break_continue_macro(arms[0].body); + if !utils::usage::contains_return_break_continue_macro(arms[1].body); then { let capture_mut = if bind_annotation == &BindingAnnotation::Mutable { "mut " } else { "" }; let some_body = extract_body_from_arm(&arms[0])?; diff --git a/src/tools/clippy/clippy_lints/src/ptr_eq.rs b/src/tools/clippy/clippy_lints/src/ptr_eq.rs new file mode 100644 index 00000000000..3be792ce5e4 --- /dev/null +++ b/src/tools/clippy/clippy_lints/src/ptr_eq.rs @@ -0,0 +1,96 @@ +use crate::utils; +use if_chain::if_chain; +use rustc_errors::Applicability; +use rustc_hir::{BinOpKind, Expr, ExprKind}; +use rustc_lint::{LateContext, LateLintPass}; +use rustc_session::{declare_lint_pass, declare_tool_lint}; + +declare_clippy_lint! { + /// **What it does:** Use `std::ptr::eq` when applicable + /// + /// **Why is this bad?** `ptr::eq` can be used to compare `&T` references + /// (which coerce to `*const T` implicitly) by their address rather than + /// comparing the values they point to. + /// + /// **Known problems:** None. + /// + /// **Example:** + /// + /// ```rust + /// let a = &[1, 2, 3]; + /// let b = &[1, 2, 3]; + /// + /// assert!(a as *const _ as usize == b as *const _ as usize); + /// ``` + /// Use instead: + /// ```rust + /// let a = &[1, 2, 3]; + /// let b = &[1, 2, 3]; + /// + /// assert!(std::ptr::eq(a, b)); + /// ``` + pub PTR_EQ, + style, + "use `std::ptr::eq` when comparing raw pointers" +} + +declare_lint_pass!(PtrEq => [PTR_EQ]); + +static LINT_MSG: &str = "use `std::ptr::eq` when comparing raw pointers"; + +impl LateLintPass<'_> for PtrEq { + fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) { + if utils::in_macro(expr.span) { + return; + } + + if let ExprKind::Binary(ref op, ref left, ref right) = expr.kind { + if BinOpKind::Eq == op.node { + let (left, right) = match (expr_as_cast_to_usize(cx, left), expr_as_cast_to_usize(cx, right)) { + (Some(lhs), Some(rhs)) => (lhs, rhs), + _ => (&**left, &**right), + }; + + if_chain! { + if let Some(left_var) = expr_as_cast_to_raw_pointer(cx, left); + if let Some(right_var) = expr_as_cast_to_raw_pointer(cx, right); + if let Some(left_snip) = utils::snippet_opt(cx, left_var.span); + if let Some(right_snip) = utils::snippet_opt(cx, right_var.span); + then { + utils::span_lint_and_sugg( + cx, + PTR_EQ, + expr.span, + LINT_MSG, + "try", + format!("std::ptr::eq({}, {})", left_snip, right_snip), + Applicability::MachineApplicable, + ); + } + } + } + } + } +} + +// If the given expression is a cast to an usize, return the lhs of the cast +// E.g., `foo as *const _ as usize` returns `foo as *const _`. +fn expr_as_cast_to_usize<'tcx>(cx: &LateContext<'tcx>, cast_expr: &'tcx Expr<'_>) -> Option<&'tcx Expr<'tcx>> { + if cx.typeck_results().expr_ty(cast_expr) == cx.tcx.types.usize { + if let ExprKind::Cast(ref expr, _) = cast_expr.kind { + return Some(expr); + } + } + None +} + +// If the given expression is a cast to a `*const` pointer, return the lhs of the cast +// E.g., `foo as *const _` returns `foo`. +fn expr_as_cast_to_raw_pointer<'tcx>(cx: &LateContext<'tcx>, cast_expr: &'tcx Expr<'_>) -> Option<&'tcx Expr<'tcx>> { + if cx.typeck_results().expr_ty(cast_expr).is_unsafe_ptr() { + if let ExprKind::Cast(ref expr, _) = cast_expr.kind { + return Some(expr); + } + } + None +} diff --git a/src/tools/clippy/clippy_lints/src/transmute.rs b/src/tools/clippy/clippy_lints/src/transmute.rs index c75adb62f25..47c650ac27d 100644 --- a/src/tools/clippy/clippy_lints/src/transmute.rs +++ b/src/tools/clippy/clippy_lints/src/transmute.rs @@ -98,7 +98,11 @@ declare_clippy_lint! { /// /// **Why is this bad?** This can always be rewritten with `&` and `*`. /// - /// **Known problems:** None. + /// **Known problems:** + /// - `mem::transmute` in statics and constants is stable from Rust 1.46.0, + /// while dereferencing raw pointer is not stable yet. + /// If you need to do this in those places, + /// you would have to use `transmute` instead. /// /// **Example:** /// ```rust,ignore diff --git a/src/tools/clippy/clippy_lints/src/trivially_copy_pass_by_ref.rs b/src/tools/clippy/clippy_lints/src/trivially_copy_pass_by_ref.rs index d92eb86fb2e..e90ea0fc200 100644 --- a/src/tools/clippy/clippy_lints/src/trivially_copy_pass_by_ref.rs +++ b/src/tools/clippy/clippy_lints/src/trivially_copy_pass_by_ref.rs @@ -12,8 +12,8 @@ use rustc_middle::ty; use rustc_session::{declare_tool_lint, impl_lint_pass}; use rustc_span::Span; use rustc_target::abi::LayoutOf; -use rustc_target::spec::Target; use rustc_target::spec::abi::Abi; +use rustc_target::spec::Target; declare_clippy_lint! { /// **What it does:** Checks for functions taking arguments by reference, where diff --git a/src/tools/clippy/clippy_lints/src/types.rs b/src/tools/clippy/clippy_lints/src/types.rs index 5e83b6c81ec..9a948af8bfc 100644 --- a/src/tools/clippy/clippy_lints/src/types.rs +++ b/src/tools/clippy/clippy_lints/src/types.rs @@ -17,6 +17,7 @@ use rustc_hir::{ use rustc_lint::{LateContext, LateLintPass, LintContext}; use rustc_middle::hir::map::Map; use rustc_middle::lint::in_external_macro; +use rustc_middle::ty::TypeFoldable; use rustc_middle::ty::{self, InferTy, Ty, TyCtxt, TyS, TypeckResults}; use rustc_session::{declare_lint_pass, declare_tool_lint, impl_lint_pass}; use rustc_span::hygiene::{ExpnKind, MacroKind}; @@ -541,6 +542,7 @@ impl Types { _ => None, }); let ty_ty = hir_ty_to_ty(cx.tcx, boxed_ty); + if !ty_ty.has_escaping_bound_vars(); if ty_ty.is_sized(cx.tcx.at(ty.span), cx.param_env); if let Ok(ty_ty_size) = cx.layout_of(ty_ty).map(|l| l.size.bytes()); if ty_ty_size <= self.vec_box_size_threshold; diff --git a/src/tools/clippy/clippy_lints/src/utils/eager_or_lazy.rs b/src/tools/clippy/clippy_lints/src/utils/eager_or_lazy.rs index 27e9567740d..4ceea13df37 100644 --- a/src/tools/clippy/clippy_lints/src/utils/eager_or_lazy.rs +++ b/src/tools/clippy/clippy_lints/src/utils/eager_or_lazy.rs @@ -82,7 +82,7 @@ fn identify_some_pure_patterns(expr: &Expr<'_>) -> bool { /// Identify some potentially computationally expensive patterns. /// This function is named so to stress that its implementation is non-exhaustive. /// It returns FNs and FPs. -fn identify_some_potentially_expensive_patterns<'a, 'tcx>(cx: &'a LateContext<'tcx>, expr: &'tcx Expr<'tcx>) -> bool { +fn identify_some_potentially_expensive_patterns<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) -> bool { // Searches an expression for method calls or function calls that aren't ctors struct FunCallFinder<'a, 'tcx> { cx: &'a LateContext<'tcx>, diff --git a/src/tools/clippy/clippy_lints/src/utils/higher.rs b/src/tools/clippy/clippy_lints/src/utils/higher.rs index 8563b469a30..6d7c5058b4f 100644 --- a/src/tools/clippy/clippy_lints/src/utils/higher.rs +++ b/src/tools/clippy/clippy_lints/src/utils/higher.rs @@ -7,6 +7,7 @@ use crate::utils::{is_expn_of, match_def_path, paths}; use if_chain::if_chain; use rustc_ast::ast; use rustc_hir as hir; +use rustc_hir::{BorrowKind, Expr, ExprKind, StmtKind, UnOp}; use rustc_lint::LateContext; /// Converts a hir binary operator to the corresponding `ast` type. @@ -241,3 +242,56 @@ pub fn vec_macro<'e>(cx: &LateContext<'_>, expr: &'e hir::Expr<'_>) -> Option(e: &'tcx Expr<'tcx>) -> Option>> { + /// Try to match the AST for a pattern that contains a match, for example when two args are + /// compared + fn ast_matchblock(matchblock_expr: &'tcx Expr<'tcx>) -> Option>> { + if_chain! { + if let ExprKind::Match(ref headerexpr, _, _) = &matchblock_expr.kind; + if let ExprKind::Tup([lhs, rhs]) = &headerexpr.kind; + if let ExprKind::AddrOf(BorrowKind::Ref, _, lhs) = lhs.kind; + if let ExprKind::AddrOf(BorrowKind::Ref, _, rhs) = rhs.kind; + then { + return Some(vec![lhs, rhs]); + } + } + None + } + + if let ExprKind::Block(ref block, _) = e.kind { + if block.stmts.len() == 1 { + if let StmtKind::Semi(ref matchexpr) = block.stmts[0].kind { + // macros with unique arg: `{debug_}assert!` (e.g., `debug_assert!(some_condition)`) + if_chain! { + if let ExprKind::Match(ref ifclause, _, _) = matchexpr.kind; + if let ExprKind::DropTemps(ref droptmp) = ifclause.kind; + if let ExprKind::Unary(UnOp::UnNot, condition) = droptmp.kind; + then { + return Some(vec![condition]); + } + } + + // debug macros with two args: `debug_assert_{ne, eq}` (e.g., `assert_ne!(a, b)`) + if_chain! { + if let ExprKind::Block(ref matchblock,_) = matchexpr.kind; + if let Some(ref matchblock_expr) = matchblock.expr; + then { + return ast_matchblock(matchblock_expr); + } + } + } + } else if let Some(matchblock_expr) = block.expr { + // macros with two args: `assert_{ne, eq}` (e.g., `assert_ne!(a, b)`) + return ast_matchblock(&matchblock_expr); + } + } + None +} diff --git a/src/tools/clippy/clippy_lints/src/utils/mod.rs b/src/tools/clippy/clippy_lints/src/utils/mod.rs index 790ac4f7dd8..a9d26d48b12 100644 --- a/src/tools/clippy/clippy_lints/src/utils/mod.rs +++ b/src/tools/clippy/clippy_lints/src/utils/mod.rs @@ -708,7 +708,7 @@ fn reindent_multiline_inner(s: &str, ignore_first: bool, indent: Option, } /// Gets the parent expression, if any –- this is useful to constrain a lint. -pub fn get_parent_expr<'c>(cx: &'c LateContext<'_>, e: &Expr<'_>) -> Option<&'c Expr<'c>> { +pub fn get_parent_expr<'tcx>(cx: &LateContext<'tcx>, e: &Expr<'_>) -> Option<&'tcx Expr<'tcx>> { let map = &cx.tcx.hir(); let hir_id = e.hir_id; let parent_id = map.get_parent_node(hir_id); diff --git a/src/tools/clippy/clippy_lints/src/utils/sugg.rs b/src/tools/clippy/clippy_lints/src/utils/sugg.rs index a2a1d109c9a..625120b880e 100644 --- a/src/tools/clippy/clippy_lints/src/utils/sugg.rs +++ b/src/tools/clippy/clippy_lints/src/utils/sugg.rs @@ -13,8 +13,10 @@ use rustc_span::{BytePos, Pos}; use std::borrow::Cow; use std::convert::TryInto; use std::fmt::Display; +use std::ops::{Add, Neg, Not, Sub}; /// A helper type to build suggestion correctly handling parenthesis. +#[derive(Clone, PartialEq)] pub enum Sugg<'a> { /// An expression that never needs parenthesis such as `1337` or `[0; 42]`. NonParen(Cow<'a, str>), @@ -25,8 +27,12 @@ pub enum Sugg<'a> { BinOp(AssocOp, Cow<'a, str>), } +/// Literal constant `0`, for convenience. +pub const ZERO: Sugg<'static> = Sugg::NonParen(Cow::Borrowed("0")); /// Literal constant `1`, for convenience. pub const ONE: Sugg<'static> = Sugg::NonParen(Cow::Borrowed("1")); +/// a constant represents an empty string, for convenience. +pub const EMPTY: Sugg<'static> = Sugg::NonParen(Cow::Borrowed("")); impl Display for Sugg<'_> { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> Result<(), std::fmt::Error> { @@ -269,21 +275,60 @@ impl<'a> Sugg<'a> { } } -impl<'a, 'b> std::ops::Add> for Sugg<'a> { +// Copied from the rust standart library, and then edited +macro_rules! forward_binop_impls_to_ref { + (impl $imp:ident, $method:ident for $t:ty, type Output = $o:ty) => { + impl $imp<$t> for &$t { + type Output = $o; + + fn $method(self, other: $t) -> $o { + $imp::$method(self, &other) + } + } + + impl $imp<&$t> for $t { + type Output = $o; + + fn $method(self, other: &$t) -> $o { + $imp::$method(&self, other) + } + } + + impl $imp for $t { + type Output = $o; + + fn $method(self, other: $t) -> $o { + $imp::$method(&self, &other) + } + } + }; +} + +impl Add for &Sugg<'_> { type Output = Sugg<'static>; - fn add(self, rhs: Sugg<'b>) -> Sugg<'static> { - make_binop(ast::BinOpKind::Add, &self, &rhs) + fn add(self, rhs: &Sugg<'_>) -> Sugg<'static> { + make_binop(ast::BinOpKind::Add, self, rhs) } } -impl<'a, 'b> std::ops::Sub> for Sugg<'a> { +impl Sub for &Sugg<'_> { type Output = Sugg<'static>; - fn sub(self, rhs: Sugg<'b>) -> Sugg<'static> { - make_binop(ast::BinOpKind::Sub, &self, &rhs) + fn sub(self, rhs: &Sugg<'_>) -> Sugg<'static> { + make_binop(ast::BinOpKind::Sub, self, rhs) } } -impl<'a> std::ops::Not for Sugg<'a> { +forward_binop_impls_to_ref!(impl Add, add for Sugg<'_>, type Output = Sugg<'static>); +forward_binop_impls_to_ref!(impl Sub, sub for Sugg<'_>, type Output = Sugg<'static>); + +impl Neg for Sugg<'_> { + type Output = Sugg<'static>; + fn neg(self) -> Sugg<'static> { + make_unop("-", self) + } +} + +impl Not for Sugg<'_> { type Output = Sugg<'static>; fn not(self) -> Sugg<'static> { make_unop("!", self) diff --git a/src/tools/clippy/clippy_lints/src/utils/usage.rs b/src/tools/clippy/clippy_lints/src/utils/usage.rs index ea1dc3be29b..2fd6046ebcf 100644 --- a/src/tools/clippy/clippy_lints/src/utils/usage.rs +++ b/src/tools/clippy/clippy_lints/src/utils/usage.rs @@ -1,10 +1,11 @@ +use crate::utils; use crate::utils::match_var; use rustc_data_structures::fx::FxHashSet; use rustc_hir as hir; use rustc_hir::def::Res; use rustc_hir::intravisit; use rustc_hir::intravisit::{walk_expr, NestedVisitorMap, Visitor}; -use rustc_hir::{Expr, HirId, Path}; +use rustc_hir::{Expr, ExprKind, HirId, Path}; use rustc_infer::infer::TyCtxtInferExt; use rustc_lint::LateContext; use rustc_middle::hir::map::Map; @@ -174,3 +175,50 @@ impl<'a, 'tcx> intravisit::Visitor<'tcx> for BindingUsageFinder<'a, 'tcx> { intravisit::NestedVisitorMap::OnlyBodies(self.cx.tcx.hir()) } } + +struct ReturnBreakContinueMacroVisitor { + seen_return_break_continue: bool, +} + +impl ReturnBreakContinueMacroVisitor { + fn new() -> ReturnBreakContinueMacroVisitor { + ReturnBreakContinueMacroVisitor { + seen_return_break_continue: false, + } + } +} + +impl<'tcx> Visitor<'tcx> for ReturnBreakContinueMacroVisitor { + type Map = Map<'tcx>; + fn nested_visit_map(&mut self) -> NestedVisitorMap { + NestedVisitorMap::None + } + + fn visit_expr(&mut self, ex: &'tcx Expr<'tcx>) { + if self.seen_return_break_continue { + // No need to look farther if we've already seen one of them + return; + } + match &ex.kind { + ExprKind::Ret(..) | ExprKind::Break(..) | ExprKind::Continue(..) => { + self.seen_return_break_continue = true; + }, + // Something special could be done here to handle while or for loop + // desugaring, as this will detect a break if there's a while loop + // or a for loop inside the expression. + _ => { + if utils::in_macro(ex.span) { + self.seen_return_break_continue = true; + } else { + rustc_hir::intravisit::walk_expr(self, ex); + } + }, + } + } +} + +pub fn contains_return_break_continue_macro(expression: &Expr<'_>) -> bool { + let mut recursive_visitor = ReturnBreakContinueMacroVisitor::new(); + recursive_visitor.visit_expr(expression); + recursive_visitor.seen_return_break_continue +} diff --git a/src/tools/clippy/doc/adding_lints.md b/src/tools/clippy/doc/adding_lints.md index 2869c3bf7d4..2572833b8de 100644 --- a/src/tools/clippy/doc/adding_lints.md +++ b/src/tools/clippy/doc/adding_lints.md @@ -104,7 +104,8 @@ every time before running `tests/ui/update-all-references.sh`. Running `TESTNAME=foo_functions cargo uitest` should pass then. When we commit our lint, we need to commit the generated `.stderr` files, too. In general, you should only commit files changed by `tests/ui/update-all-references.sh` for the -specific lint you are creating/editing. +specific lint you are creating/editing. Note that if the generated files are +empty, they should be removed. ### Cargo lints @@ -224,6 +225,17 @@ automate everything. We will have to register our lint pass manually in the 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 +`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 +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 +actually run, which in turn affects the order that any emitted lints are output +in. + [declare_clippy_lint]: https://github.com/rust-lang/rust-clippy/blob/557f6848bd5b7183f55c1e1522a326e9e1df6030/clippy_lints/src/lib.rs#L60 [example_lint_page]: https://rust-lang.github.io/rust-clippy/master/index.html#redundant_closure [lint_naming]: https://rust-lang.github.io/rfcs/0344-conventions-galore.html#lints @@ -453,12 +465,12 @@ Before submitting your PR make sure you followed all of the basic requirements: -- [ ] Followed [lint naming conventions][lint_naming] -- [ ] Added passing UI tests (including committed `.stderr` file) -- [ ] `cargo test` passes locally -- [ ] Executed `cargo dev update_lints` -- [ ] Added lint documentation -- [ ] Run `cargo dev fmt` +- \[ ] Followed [lint naming conventions][lint_naming] +- \[ ] Added passing UI tests (including committed `.stderr` file) +- \[ ] `cargo test` passes locally +- \[ ] Executed `cargo dev update_lints` +- \[ ] Added lint documentation +- \[ ] Run `cargo dev fmt` ## Cheatsheet diff --git a/src/tools/clippy/doc/backport.md b/src/tools/clippy/doc/backport.md index 259696658ea..15f3d1f0806 100644 --- a/src/tools/clippy/doc/backport.md +++ b/src/tools/clippy/doc/backport.md @@ -5,7 +5,7 @@ Backports in Clippy are rare and should be approved by the Clippy team. For example, a backport is done, if a crucial ICE was fixed or a lint is broken to a point, that it has to be disabled, before landing on stable. -Backports are done to the `beta` release of Clippy. Backports to stable Clippy +Backports are done to the `beta` branch of Clippy. Backports to stable Clippy releases basically don't exist, since this would require a Rust point release, which is almost never justifiable for a Clippy fix. @@ -18,7 +18,31 @@ Backports are done on the beta branch of the Clippy repository. # Assuming the current directory corresponds to the Clippy repository $ git checkout beta $ git checkout -b backport -$ git cherry-pick # `` is the commit hash of the commit, that should be backported +$ git cherry-pick # `` is the commit hash of the commit(s), that should be backported +$ git push origin backport +``` + +Now you should test that the backport passes all the tests in the Rust +repository. You can do this with: + +```bash +# Assuming the current directory corresponds to the Rust repository +$ git checkout beta +$ git subtree pull -p src/tools/clippy https://github.com//rust-clippy backport +$ ./x.py test src/tools/clippy +``` + +Should the test fail, you can fix Clippy directly in the Rust repository. This +has to be first applied to the Clippy beta branch and then again synced to the +Rust repository, though. The easiest way to do this is: + +```bash +# In the Rust repository +$ git diff --patch --relative=src/tools/clippy > clippy.patch +# In the Clippy repository +$ git apply /path/to/clippy.patch +$ git add -u +$ git commit -m "Fix rustup fallout" $ git push origin backport ``` @@ -29,22 +53,19 @@ After this, you can open a PR to the `beta` branch of the Clippy repository. This step must be done, **after** the PR of the previous step was merged. -After the backport landed in the Clippy repository, also the Clippy version on -the Rust `beta` branch has to be updated. +After the backport landed in the Clippy repository, the branch has to be synced +back to the beta branch of the Rust repository. ```bash # Assuming the current directory corresponds to the Rust repository $ git checkout beta $ git checkout -b clippy_backport -$ pushd src/tools/clippy -$ git fetch -$ git checkout beta -$ popd -$ git add src/tools/clippy -§ git commit -m "Update Clippy" +$ git subtree pull -p src/tools/clippy https://github.com/rust-lang/rust-clippy beta $ git push origin clippy_backport ``` -After this you can open a PR to the `beta` branch of the Rust repository. In -this PR you should tag the Clippy team member, that agreed to the backport or -the `@rust-lang/clippy` team. Make sure to add `[beta]` to the title of the PR. +Make sure to test the backport in the Rust repository before opening a PR. This +is done with `./x.py test src/tools/clippy`. If that passes all tests, open a PR +to the `beta` branch of the Rust repository. In this PR you should tag the +Clippy team member, that agreed to the backport or the `@rust-lang/clippy` team. +Make sure to add `[beta]` to the title of the PR. diff --git a/src/tools/clippy/doc/basics.md b/src/tools/clippy/doc/basics.md index 38959e2331b..f25edb793e2 100644 --- a/src/tools/clippy/doc/basics.md +++ b/src/tools/clippy/doc/basics.md @@ -46,7 +46,7 @@ this toolchain, you can just use the `setup-toolchain.sh` script or use `rustup-toolchain-install-master`: ```bash -sh setup-toolchain.sh +bash setup-toolchain.sh # OR cargo install rustup-toolchain-install-master # For better IDE integration also add `-c rustfmt -c rust-src` (optional) diff --git a/src/tools/clippy/doc/common_tools_writing_lints.md b/src/tools/clippy/doc/common_tools_writing_lints.md index 53c3d084dbc..d56079a4ab7 100644 --- a/src/tools/clippy/doc/common_tools_writing_lints.md +++ b/src/tools/clippy/doc/common_tools_writing_lints.md @@ -45,11 +45,13 @@ Similarly in [`TypeckResults`][TypeckResults] methods, you have the [`pat_ty()`] to retrieve a type from a pattern. Two noticeable items here: -- `cx` is the lint context [`LateContext`][LateContext]. - The two most useful data structures in this context are `tcx` and `tables`, - allowing us to jump to type definitions and other compilation stages such as HIR. -- `tables` is [`TypeckResults`][TypeckResults] and is created by type checking step, - it includes useful information such as types of expressions, ways to resolve methods and so on. +- `cx` is the lint context [`LateContext`][LateContext]. The two most useful + data structures in this context are `tcx` and the `TypeckResults` returned by + `LateContext::typeck_results`, allowing us to jump to type definitions and + other compilation stages such as HIR. +- `typeck_results`'s return value is [`TypeckResults`][TypeckResults] and is + created by type checking step, it includes useful information such as types + of expressions, ways to resolve methods and so on. # Checking if an expr is calling a specific method diff --git a/src/tools/clippy/doc/release.md b/src/tools/clippy/doc/release.md index 391952ea6b1..eaa6a9af277 100644 --- a/src/tools/clippy/doc/release.md +++ b/src/tools/clippy/doc/release.md @@ -68,7 +68,7 @@ be updated. ```bash # Assuming the current directory corresponds to the Clippy repository $ git checkout beta -$ git rebase $BETA_SHA +$ git reset --hard $BETA_SHA $ git push upstream beta ``` diff --git a/src/tools/clippy/src/driver.rs b/src/tools/clippy/src/driver.rs index 805828eece1..cc71cc66b92 100644 --- a/src/tools/clippy/src/driver.rs +++ b/src/tools/clippy/src/driver.rs @@ -1,4 +1,5 @@ #![feature(rustc_private)] +#![feature(once_cell)] #![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)] @@ -17,9 +18,9 @@ use rustc_interface::interface; use rustc_middle::ty::TyCtxt; use rustc_tools_util::VersionInfo; -use lazy_static::lazy_static; use std::borrow::Cow; use std::env; +use std::lazy::SyncLazy; use std::ops::Deref; use std::panic; use std::path::{Path, PathBuf}; @@ -230,13 +231,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"; -lazy_static! { - static ref ICE_HOOK: Box) + Sync + Send + 'static> = { - 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 @@ -295,7 +294,7 @@ fn toolchain_path(home: Option, toolchain: Option) -> Option = env::args().collect(); diff --git a/src/tools/clippy/src/lintlist/mod.rs b/src/tools/clippy/src/lintlist/mod.rs index ce3d0efab3a..6301d623a2b 100644 --- a/src/tools/clippy/src/lintlist/mod.rs +++ b/src/tools/clippy/src/lintlist/mod.rs @@ -1,15 +1,16 @@ -//! This file is managed by `cargo dev update_lints`. Do not edit. +//! This file is managed by `cargo dev update_lints`. Do not edit or format this file. -use lazy_static::lazy_static; +use std::lazy::SyncLazy; pub mod lint; pub use lint::Level; pub use lint::Lint; pub use lint::LINT_LEVELS; -lazy_static! { +#[rustfmt::skip] +pub static ALL_LINTS: SyncLazy> = SyncLazy::new(|| { // begin lint list, do not remove this comment, it’s used in `update_lints` -pub static ref ALL_LINTS: Vec = vec![ +vec![ Lint { name: "absurd_extreme_comparisons", group: "correctness", @@ -1179,6 +1180,13 @@ pub static ref ALL_LINTS: Vec = vec![ deprecation: None, module: "swap", }, + Lint { + name: "manual_unwrap_or", + group: "complexity", + desc: "finds patterns that can be encoded more concisely with `Option::unwrap_or`", + deprecation: None, + module: "manual_unwrap_or", + }, Lint { name: "many_single_char_names", group: "style", @@ -1844,6 +1852,13 @@ pub static ref ALL_LINTS: Vec = vec![ deprecation: None, module: "ptr", }, + Lint { + name: "ptr_eq", + group: "style", + desc: "use `std::ptr::eq` when comparing raw pointers", + deprecation: None, + module: "ptr_eq", + }, Lint { name: "ptr_offset_with_cast", group: "complexity", @@ -1998,6 +2013,13 @@ pub static ref ALL_LINTS: Vec = vec![ deprecation: None, module: "map_unit_fn", }, + Lint { + name: "result_unit_err", + group: "style", + desc: "public function returning `Result` with an `Err` type of `()`", + deprecation: None, + module: "functions", + }, Lint { name: "reversed_empty_ranges", group: "correctness", @@ -2817,6 +2839,6 @@ pub static ref ALL_LINTS: Vec = vec![ deprecation: None, module: "methods", }, -]; +] // end lint list, do not remove this comment, it’s used in `update_lints` -} +}); diff --git a/src/tools/clippy/tests/ui-cargo/update-references.sh b/src/tools/clippy/tests/ui-cargo/update-references.sh index 50d42678734..2ab51168bca 100755 --- a/src/tools/clippy/tests/ui-cargo/update-references.sh +++ b/src/tools/clippy/tests/ui-cargo/update-references.sh @@ -29,10 +29,18 @@ while [[ "$1" != "" ]]; do ! (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-toml/update-references.sh b/src/tools/clippy/tests/ui-toml/update-references.sh index 50d42678734..2ab51168bca 100755 --- a/src/tools/clippy/tests/ui-toml/update-references.sh +++ b/src/tools/clippy/tests/ui-toml/update-references.sh @@ -29,10 +29,18 @@ while [[ "$1" != "" ]]; do ! (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/auxiliary/proc_macro_derive.rs b/src/tools/clippy/tests/ui/auxiliary/proc_macro_derive.rs index cd5a5ae0aa7..7c4e4a14551 100644 --- a/src/tools/clippy/tests/ui/auxiliary/proc_macro_derive.rs +++ b/src/tools/clippy/tests/ui/auxiliary/proc_macro_derive.rs @@ -4,6 +4,7 @@ #![crate_type = "proc-macro"] #![feature(repr128, proc_macro_quote)] #![allow(incomplete_features)] +#![allow(clippy::eq_op)] extern crate proc_macro; diff --git a/src/tools/clippy/tests/ui/crashes/ice-6139.rs b/src/tools/clippy/tests/ui/crashes/ice-6139.rs new file mode 100644 index 00000000000..f3966e47f5e --- /dev/null +++ b/src/tools/clippy/tests/ui/crashes/ice-6139.rs @@ -0,0 +1,7 @@ +trait T<'a> {} + +fn foo(_: Vec>>) {} + +fn main() { + foo(vec![]); +} diff --git a/src/tools/clippy/tests/ui/crashes/ice-6153.rs b/src/tools/clippy/tests/ui/crashes/ice-6153.rs new file mode 100644 index 00000000000..9f73f39f10d --- /dev/null +++ b/src/tools/clippy/tests/ui/crashes/ice-6153.rs @@ -0,0 +1,9 @@ +pub struct S<'a, 'e>(&'a str, &'e str); + +pub type T<'a, 'e> = std::collections::HashMap, ()>; + +impl<'e, 'a: 'e> S<'a, 'e> { + pub fn foo(_a: &str, _b: &str, _map: &T) {} +} + +fn main() {} diff --git a/src/tools/clippy/tests/ui/doc_errors.rs b/src/tools/clippy/tests/ui/doc_errors.rs index 445fc8d31d7..f47b81a450e 100644 --- a/src/tools/clippy/tests/ui/doc_errors.rs +++ b/src/tools/clippy/tests/ui/doc_errors.rs @@ -1,5 +1,6 @@ // edition:2018 #![warn(clippy::missing_errors_doc)] +#![allow(clippy::result_unit_err)] use std::io; diff --git a/src/tools/clippy/tests/ui/doc_errors.stderr b/src/tools/clippy/tests/ui/doc_errors.stderr index f44d6693d30..c7b616e2897 100644 --- a/src/tools/clippy/tests/ui/doc_errors.stderr +++ b/src/tools/clippy/tests/ui/doc_errors.stderr @@ -1,5 +1,5 @@ error: docs for function returning `Result` missing `# Errors` section - --> $DIR/doc_errors.rs:6:1 + --> $DIR/doc_errors.rs:7:1 | LL | / pub fn pub_fn_missing_errors_header() -> Result<(), ()> { LL | | unimplemented!(); @@ -9,7 +9,7 @@ LL | | } = note: `-D clippy::missing-errors-doc` implied by `-D warnings` error: docs for function returning `Result` missing `# Errors` section - --> $DIR/doc_errors.rs:10:1 + --> $DIR/doc_errors.rs:11:1 | LL | / pub async fn async_pub_fn_missing_errors_header() -> Result<(), ()> { LL | | unimplemented!(); @@ -17,7 +17,7 @@ LL | | } | |_^ error: docs for function returning `Result` missing `# Errors` section - --> $DIR/doc_errors.rs:15:1 + --> $DIR/doc_errors.rs:16:1 | LL | / pub fn pub_fn_returning_io_result() -> io::Result<()> { LL | | unimplemented!(); @@ -25,7 +25,7 @@ LL | | } | |_^ error: docs for function returning `Result` missing `# Errors` section - --> $DIR/doc_errors.rs:20:1 + --> $DIR/doc_errors.rs:21:1 | LL | / pub async fn async_pub_fn_returning_io_result() -> io::Result<()> { LL | | unimplemented!(); @@ -33,7 +33,7 @@ LL | | } | |_^ error: docs for function returning `Result` missing `# Errors` section - --> $DIR/doc_errors.rs:50:5 + --> $DIR/doc_errors.rs:51:5 | LL | / pub fn pub_method_missing_errors_header() -> Result<(), ()> { LL | | unimplemented!(); @@ -41,7 +41,7 @@ LL | | } | |_____^ error: docs for function returning `Result` missing `# Errors` section - --> $DIR/doc_errors.rs:55:5 + --> $DIR/doc_errors.rs:56:5 | LL | / pub async fn async_pub_method_missing_errors_header() -> Result<(), ()> { LL | | unimplemented!(); @@ -49,7 +49,7 @@ LL | | } | |_____^ error: docs for function returning `Result` missing `# Errors` section - --> $DIR/doc_errors.rs:84:5 + --> $DIR/doc_errors.rs:85:5 | LL | fn trait_method_missing_errors_header() -> Result<(), ()>; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ diff --git a/src/tools/clippy/tests/ui/double_must_use.rs b/src/tools/clippy/tests/ui/double_must_use.rs index a48e675e4ea..05e087b08bc 100644 --- a/src/tools/clippy/tests/ui/double_must_use.rs +++ b/src/tools/clippy/tests/ui/double_must_use.rs @@ -1,4 +1,5 @@ #![warn(clippy::double_must_use)] +#![allow(clippy::result_unit_err)] #[must_use] pub fn must_use_result() -> Result<(), ()> { diff --git a/src/tools/clippy/tests/ui/double_must_use.stderr b/src/tools/clippy/tests/ui/double_must_use.stderr index bc37785294f..8290ece1cad 100644 --- a/src/tools/clippy/tests/ui/double_must_use.stderr +++ b/src/tools/clippy/tests/ui/double_must_use.stderr @@ -1,5 +1,5 @@ error: this function has an empty `#[must_use]` attribute, but returns a type already marked as `#[must_use]` - --> $DIR/double_must_use.rs:4:1 + --> $DIR/double_must_use.rs:5:1 | LL | pub fn must_use_result() -> Result<(), ()> { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -8,7 +8,7 @@ LL | pub fn must_use_result() -> Result<(), ()> { = help: either add some descriptive text or remove the attribute error: this function has an empty `#[must_use]` attribute, but returns a type already marked as `#[must_use]` - --> $DIR/double_must_use.rs:9:1 + --> $DIR/double_must_use.rs:10:1 | LL | pub fn must_use_tuple() -> (Result<(), ()>, u8) { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -16,7 +16,7 @@ LL | pub fn must_use_tuple() -> (Result<(), ()>, u8) { = help: either add some descriptive text or remove the attribute error: this function has an empty `#[must_use]` attribute, but returns a type already marked as `#[must_use]` - --> $DIR/double_must_use.rs:14:1 + --> $DIR/double_must_use.rs:15:1 | LL | pub fn must_use_array() -> [Result<(), ()>; 1] { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ diff --git a/src/tools/clippy/tests/ui/double_parens.rs b/src/tools/clippy/tests/ui/double_parens.rs index 9c7590c7dd6..ff1dc76ab63 100644 --- a/src/tools/clippy/tests/ui/double_parens.rs +++ b/src/tools/clippy/tests/ui/double_parens.rs @@ -1,5 +1,5 @@ #![warn(clippy::double_parens)] -#![allow(dead_code)] +#![allow(dead_code, clippy::eq_op)] #![feature(custom_inner_attributes)] #![rustfmt::skip] diff --git a/src/tools/clippy/tests/ui/eq_op_macros.rs b/src/tools/clippy/tests/ui/eq_op_macros.rs new file mode 100644 index 00000000000..6b5b31a1a2e --- /dev/null +++ b/src/tools/clippy/tests/ui/eq_op_macros.rs @@ -0,0 +1,56 @@ +#![warn(clippy::eq_op)] + +// lint also in macro definition +macro_rules! assert_in_macro_def { + () => { + let a = 42; + assert_eq!(a, a); + assert_ne!(a, a); + debug_assert_eq!(a, a); + debug_assert_ne!(a, a); + }; +} + +// lint identical args in assert-like macro invocations (see #3574) +fn main() { + assert_in_macro_def!(); + + let a = 1; + let b = 2; + + // lint identical args in `assert_eq!` + assert_eq!(a, a); + assert_eq!(a + 1, a + 1); + // ok + assert_eq!(a, b); + assert_eq!(a, a + 1); + assert_eq!(a + 1, b + 1); + + // lint identical args in `assert_ne!` + assert_ne!(a, a); + assert_ne!(a + 1, a + 1); + // ok + assert_ne!(a, b); + assert_ne!(a, a + 1); + assert_ne!(a + 1, b + 1); + + // lint identical args in `debug_assert_eq!` + debug_assert_eq!(a, a); + debug_assert_eq!(a + 1, a + 1); + // ok + debug_assert_eq!(a, b); + debug_assert_eq!(a, a + 1); + debug_assert_eq!(a + 1, b + 1); + + // lint identical args in `debug_assert_ne!` + debug_assert_ne!(a, a); + debug_assert_ne!(a + 1, a + 1); + // ok + debug_assert_ne!(a, b); + debug_assert_ne!(a, a + 1); + debug_assert_ne!(a + 1, b + 1); + + let my_vec = vec![1; 5]; + let mut my_iter = my_vec.iter(); + assert_ne!(my_iter.next(), my_iter.next()); +} diff --git a/src/tools/clippy/tests/ui/eq_op_macros.stderr b/src/tools/clippy/tests/ui/eq_op_macros.stderr new file mode 100644 index 00000000000..fb9378108b9 --- /dev/null +++ b/src/tools/clippy/tests/ui/eq_op_macros.stderr @@ -0,0 +1,95 @@ +error: identical args used in this `assert_eq!` macro call + --> $DIR/eq_op_macros.rs:7:20 + | +LL | assert_eq!(a, a); + | ^^^^ +... +LL | assert_in_macro_def!(); + | ----------------------- in this macro invocation + | + = note: `-D clippy::eq-op` implied by `-D warnings` + = note: this error originates in a macro (in Nightly builds, run with -Z macro-backtrace for more info) + +error: identical args used in this `assert_ne!` macro call + --> $DIR/eq_op_macros.rs:8:20 + | +LL | assert_ne!(a, a); + | ^^^^ +... +LL | assert_in_macro_def!(); + | ----------------------- in this macro invocation + | + = note: this error originates in a macro (in Nightly builds, run with -Z macro-backtrace for more info) + +error: identical args used in this `assert_eq!` macro call + --> $DIR/eq_op_macros.rs:22:16 + | +LL | assert_eq!(a, a); + | ^^^^ + +error: identical args used in this `assert_eq!` macro call + --> $DIR/eq_op_macros.rs:23:16 + | +LL | assert_eq!(a + 1, a + 1); + | ^^^^^^^^^^^^ + +error: identical args used in this `assert_ne!` macro call + --> $DIR/eq_op_macros.rs:30:16 + | +LL | assert_ne!(a, a); + | ^^^^ + +error: identical args used in this `assert_ne!` macro call + --> $DIR/eq_op_macros.rs:31:16 + | +LL | assert_ne!(a + 1, a + 1); + | ^^^^^^^^^^^^ + +error: identical args used in this `debug_assert_eq!` macro call + --> $DIR/eq_op_macros.rs:9:26 + | +LL | debug_assert_eq!(a, a); + | ^^^^ +... +LL | assert_in_macro_def!(); + | ----------------------- in this macro invocation + | + = note: this error originates in a macro (in Nightly builds, run with -Z macro-backtrace for more info) + +error: identical args used in this `debug_assert_ne!` macro call + --> $DIR/eq_op_macros.rs:10:26 + | +LL | debug_assert_ne!(a, a); + | ^^^^ +... +LL | assert_in_macro_def!(); + | ----------------------- in this macro invocation + | + = note: this error originates in a macro (in Nightly builds, run with -Z macro-backtrace for more info) + +error: identical args used in this `debug_assert_eq!` macro call + --> $DIR/eq_op_macros.rs:38:22 + | +LL | debug_assert_eq!(a, a); + | ^^^^ + +error: identical args used in this `debug_assert_eq!` macro call + --> $DIR/eq_op_macros.rs:39:22 + | +LL | debug_assert_eq!(a + 1, a + 1); + | ^^^^^^^^^^^^ + +error: identical args used in this `debug_assert_ne!` macro call + --> $DIR/eq_op_macros.rs:46:22 + | +LL | debug_assert_ne!(a, a); + | ^^^^ + +error: identical args used in this `debug_assert_ne!` macro call + --> $DIR/eq_op_macros.rs:47:22 + | +LL | debug_assert_ne!(a + 1, a + 1); + | ^^^^^^^^^^^^ + +error: aborting due to 12 previous errors + diff --git a/src/tools/clippy/tests/ui/float_cmp.rs b/src/tools/clippy/tests/ui/float_cmp.rs index 9fa0e5f5c07..586784b73e6 100644 --- a/src/tools/clippy/tests/ui/float_cmp.rs +++ b/src/tools/clippy/tests/ui/float_cmp.rs @@ -2,6 +2,7 @@ #![allow( unused, clippy::no_effect, + clippy::op_ref, clippy::unnecessary_operation, clippy::cast_lossless, clippy::many_single_char_names @@ -116,4 +117,8 @@ fn main() { 1.23f64.signum() != x64.signum(); 1.23f64.signum() != -(x64.signum()); 1.23f64.signum() != 3.21f64.signum(); + + // the comparison should also look through references + &0.0 == &ZERO; + &&&&0.0 == &&&&ZERO; } diff --git a/src/tools/clippy/tests/ui/float_cmp.stderr b/src/tools/clippy/tests/ui/float_cmp.stderr index f7c380fc915..bb4051c4662 100644 --- a/src/tools/clippy/tests/ui/float_cmp.stderr +++ b/src/tools/clippy/tests/ui/float_cmp.stderr @@ -1,5 +1,5 @@ error: strict comparison of `f32` or `f64` - --> $DIR/float_cmp.rs:65:5 + --> $DIR/float_cmp.rs:66:5 | LL | ONE as f64 != 2.0; | ^^^^^^^^^^^^^^^^^ help: consider comparing them within some margin of error: `(ONE as f64 - 2.0).abs() > error_margin` @@ -8,7 +8,7 @@ LL | ONE as f64 != 2.0; = note: `f32::EPSILON` and `f64::EPSILON` are available for the `error_margin` error: strict comparison of `f32` or `f64` - --> $DIR/float_cmp.rs:70:5 + --> $DIR/float_cmp.rs:71:5 | LL | x == 1.0; | ^^^^^^^^ help: consider comparing them within some margin of error: `(x - 1.0).abs() < error_margin` @@ -16,7 +16,7 @@ LL | x == 1.0; = note: `f32::EPSILON` and `f64::EPSILON` are available for the `error_margin` error: strict comparison of `f32` or `f64` - --> $DIR/float_cmp.rs:73:5 + --> $DIR/float_cmp.rs:74:5 | LL | twice(x) != twice(ONE as f64); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider comparing them within some margin of error: `(twice(x) - twice(ONE as f64)).abs() > error_margin` @@ -24,7 +24,7 @@ LL | twice(x) != twice(ONE as f64); = note: `f32::EPSILON` and `f64::EPSILON` are available for the `error_margin` error: strict comparison of `f32` or `f64` - --> $DIR/float_cmp.rs:93:5 + --> $DIR/float_cmp.rs:94:5 | LL | NON_ZERO_ARRAY[i] == NON_ZERO_ARRAY[j]; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider comparing them within some margin of error: `(NON_ZERO_ARRAY[i] - NON_ZERO_ARRAY[j]).abs() < error_margin` @@ -32,7 +32,7 @@ LL | NON_ZERO_ARRAY[i] == NON_ZERO_ARRAY[j]; = note: `f32::EPSILON` and `f64::EPSILON` are available for the `error_margin` error: strict comparison of `f32` or `f64` arrays - --> $DIR/float_cmp.rs:98:5 + --> $DIR/float_cmp.rs:99:5 | LL | a1 == a2; | ^^^^^^^^ @@ -40,7 +40,7 @@ LL | a1 == a2; = note: `f32::EPSILON` and `f64::EPSILON` are available for the `error_margin` error: strict comparison of `f32` or `f64` - --> $DIR/float_cmp.rs:99:5 + --> $DIR/float_cmp.rs:100:5 | LL | a1[0] == a2[0]; | ^^^^^^^^^^^^^^ help: consider comparing them within some margin of error: `(a1[0] - a2[0]).abs() < error_margin` diff --git a/src/tools/clippy/tests/ui/format.fixed b/src/tools/clippy/tests/ui/format.fixed index 30651476999..740a22a07d7 100644 --- a/src/tools/clippy/tests/ui/format.fixed +++ b/src/tools/clippy/tests/ui/format.fixed @@ -13,7 +13,8 @@ fn main() { "foo".to_string(); "{}".to_string(); "{} abc {}".to_string(); - "foo {}\n\" bar".to_string(); + r##"foo {} +" bar"##.to_string(); "foo".to_string(); format!("{:?}", "foo"); // Don't warn about `Debug`. diff --git a/src/tools/clippy/tests/ui/format.stderr b/src/tools/clippy/tests/ui/format.stderr index 9734492154e..96df7f37f77 100644 --- a/src/tools/clippy/tests/ui/format.stderr +++ b/src/tools/clippy/tests/ui/format.stderr @@ -25,7 +25,13 @@ LL | / format!( LL | | r##"foo {{}} LL | | " bar"## LL | | ); - | |______^ help: consider using `.to_string()`: `"foo {}/n/" bar".to_string();` + | |______^ + | +help: consider using `.to_string()` + | +LL | r##"foo {} +LL | " bar"##.to_string(); + | error: useless use of `format!` --> $DIR/format.rs:21:5 diff --git a/src/tools/clippy/tests/ui/manual_memcpy.stderr b/src/tools/clippy/tests/ui/manual_memcpy.stderr deleted file mode 100644 index bad84a58900..00000000000 --- a/src/tools/clippy/tests/ui/manual_memcpy.stderr +++ /dev/null @@ -1,88 +0,0 @@ -error: it looks like you're manually copying between slices - --> $DIR/manual_memcpy.rs:7:14 - | -LL | for i in 0..src.len() { - | ^^^^^^^^^^^^ help: try replacing the loop by: `dst[..src.len()].clone_from_slice(&src[..])` - | - = note: `-D clippy::manual-memcpy` implied by `-D warnings` - -error: it looks like you're manually copying between slices - --> $DIR/manual_memcpy.rs:12:14 - | -LL | for i in 0..src.len() { - | ^^^^^^^^^^^^ help: try replacing the loop by: `dst[10..(src.len() + 10)].clone_from_slice(&src[..])` - -error: it looks like you're manually copying between slices - --> $DIR/manual_memcpy.rs:17:14 - | -LL | for i in 0..src.len() { - | ^^^^^^^^^^^^ help: try replacing the loop by: `dst[..src.len()].clone_from_slice(&src[10..])` - -error: it looks like you're manually copying between slices - --> $DIR/manual_memcpy.rs:22:14 - | -LL | for i in 11..src.len() { - | ^^^^^^^^^^^^^ help: try replacing the loop by: `dst[11..src.len()].clone_from_slice(&src[(11 - 10)..(src.len() - 10)])` - -error: it looks like you're manually copying between slices - --> $DIR/manual_memcpy.rs:27:14 - | -LL | for i in 0..dst.len() { - | ^^^^^^^^^^^^ help: try replacing the loop by: `dst.clone_from_slice(&src[..dst.len()])` - -error: it looks like you're manually copying between slices - --> $DIR/manual_memcpy.rs:40:14 - | -LL | for i in 10..256 { - | ^^^^^^^ - | -help: try replacing the loop by - | -LL | for i in dst[10..256].clone_from_slice(&src[(10 - 5)..(256 - 5)]) -LL | dst2[(10 + 500)..(256 + 500)].clone_from_slice(&src[10..256]) { - | - -error: it looks like you're manually copying between slices - --> $DIR/manual_memcpy.rs:52:14 - | -LL | for i in 10..LOOP_OFFSET { - | ^^^^^^^^^^^^^^^ help: try replacing the loop by: `dst[(10 + LOOP_OFFSET)..(LOOP_OFFSET + LOOP_OFFSET)].clone_from_slice(&src[(10 - some_var)..(LOOP_OFFSET - some_var)])` - -error: it looks like you're manually copying between slices - --> $DIR/manual_memcpy.rs:65:14 - | -LL | for i in 0..src_vec.len() { - | ^^^^^^^^^^^^^^^^ help: try replacing the loop by: `dst_vec[..src_vec.len()].clone_from_slice(&src_vec[..])` - -error: it looks like you're manually copying between slices - --> $DIR/manual_memcpy.rs:94:14 - | -LL | for i in from..from + src.len() { - | ^^^^^^^^^^^^^^^^^^^^^^ help: try replacing the loop by: `dst[from..from + src.len()].clone_from_slice(&src[..(from + src.len() - from)])` - -error: it looks like you're manually copying between slices - --> $DIR/manual_memcpy.rs:98:14 - | -LL | for i in from..from + 3 { - | ^^^^^^^^^^^^^^ help: try replacing the loop by: `dst[from..from + 3].clone_from_slice(&src[..(from + 3 - from)])` - -error: it looks like you're manually copying between slices - --> $DIR/manual_memcpy.rs:103:14 - | -LL | for i in 0..5 { - | ^^^^ help: try replacing the loop by: `dst[..5].clone_from_slice(&src[..5])` - -error: it looks like you're manually copying between slices - --> $DIR/manual_memcpy.rs:108:14 - | -LL | for i in 0..0 { - | ^^^^ help: try replacing the loop by: `dst[..0].clone_from_slice(&src[..0])` - -error: it looks like you're manually copying between slices - --> $DIR/manual_memcpy.rs:120:14 - | -LL | for i in 0..src.len() { - | ^^^^^^^^^^^^ help: try replacing the loop by: `dst[..src.len()].clone_from_slice(&src[..])` - -error: aborting due to 13 previous errors - diff --git a/src/tools/clippy/tests/ui/manual_memcpy/with_loop_counters.rs b/src/tools/clippy/tests/ui/manual_memcpy/with_loop_counters.rs new file mode 100644 index 00000000000..ba388a05a28 --- /dev/null +++ b/src/tools/clippy/tests/ui/manual_memcpy/with_loop_counters.rs @@ -0,0 +1,88 @@ +#![warn(clippy::needless_range_loop, clippy::manual_memcpy)] + +pub fn manual_copy_with_counters(src: &[i32], dst: &mut [i32], dst2: &mut [i32]) { + let mut count = 0; + for i in 3..src.len() { + dst[i] = src[count]; + count += 1; + } + + let mut count = 0; + for i in 3..src.len() { + dst[count] = src[i]; + count += 1; + } + + let mut count = 3; + for i in 0..src.len() { + dst[count] = src[i]; + count += 1; + } + + let mut count = 3; + for i in 0..src.len() { + dst[i] = src[count]; + count += 1; + } + + let mut count = 0; + for i in 3..(3 + src.len()) { + dst[i] = src[count]; + count += 1; + } + + let mut count = 3; + for i in 5..src.len() { + dst[i] = src[count - 2]; + count += 1; + } + + let mut count = 2; + for i in 0..dst.len() { + dst[i] = src[count]; + count += 1; + } + + let mut count = 5; + for i in 3..10 { + dst[i] = src[count]; + count += 1; + } + + let mut count = 3; + let mut count2 = 30; + for i in 0..src.len() { + dst[count] = src[i]; + dst2[count2] = src[i]; + count += 1; + count2 += 1; + } + + // make sure parentheses are added properly to bitwise operators, which have lower precedence than + // arithmetric ones + let mut count = 0 << 1; + for i in 0..1 << 1 { + dst[count] = src[i + 2]; + count += 1; + } + + // make sure incrementing expressions without semicolons at the end of loops are handled correctly. + let mut count = 0; + for i in 3..src.len() { + dst[i] = src[count]; + count += 1 + } + + // make sure ones where the increment is not at the end of the loop. + // As a possible enhancement, one could adjust the offset in the suggestion according to + // the position. For example, if the increment is at the top of the loop; + // treating the loop counter as if it were initialized 1 greater than the original value. + let mut count = 0; + #[allow(clippy::needless_range_loop)] + for i in 0..src.len() { + count += 1; + dst[i] = src[count]; + } +} + +fn main() {} diff --git a/src/tools/clippy/tests/ui/manual_memcpy/with_loop_counters.stderr b/src/tools/clippy/tests/ui/manual_memcpy/with_loop_counters.stderr new file mode 100644 index 00000000000..2547b19f5d1 --- /dev/null +++ b/src/tools/clippy/tests/ui/manual_memcpy/with_loop_counters.stderr @@ -0,0 +1,111 @@ +error: it looks like you're manually copying between slices + --> $DIR/with_loop_counters.rs:5:5 + | +LL | / for i in 3..src.len() { +LL | | dst[i] = src[count]; +LL | | count += 1; +LL | | } + | |_____^ help: try replacing the loop by: `dst[3..src.len()].clone_from_slice(&src[..(src.len() - 3)]);` + | + = note: `-D clippy::manual-memcpy` implied by `-D warnings` + +error: it looks like you're manually copying between slices + --> $DIR/with_loop_counters.rs:11:5 + | +LL | / for i in 3..src.len() { +LL | | dst[count] = src[i]; +LL | | count += 1; +LL | | } + | |_____^ help: try replacing the loop by: `dst[..(src.len() - 3)].clone_from_slice(&src[3..]);` + +error: it looks like you're manually copying between slices + --> $DIR/with_loop_counters.rs:17:5 + | +LL | / for i in 0..src.len() { +LL | | dst[count] = src[i]; +LL | | count += 1; +LL | | } + | |_____^ help: try replacing the loop by: `dst[3..(src.len() + 3)].clone_from_slice(&src[..]);` + +error: it looks like you're manually copying between slices + --> $DIR/with_loop_counters.rs:23:5 + | +LL | / for i in 0..src.len() { +LL | | dst[i] = src[count]; +LL | | count += 1; +LL | | } + | |_____^ help: try replacing the loop by: `dst[..src.len()].clone_from_slice(&src[3..(src.len() + 3)]);` + +error: it looks like you're manually copying between slices + --> $DIR/with_loop_counters.rs:29:5 + | +LL | / for i in 3..(3 + src.len()) { +LL | | dst[i] = src[count]; +LL | | count += 1; +LL | | } + | |_____^ help: try replacing the loop by: `dst[3..((3 + src.len()))].clone_from_slice(&src[..((3 + src.len()) - 3)]);` + +error: it looks like you're manually copying between slices + --> $DIR/with_loop_counters.rs:35:5 + | +LL | / for i in 5..src.len() { +LL | | dst[i] = src[count - 2]; +LL | | count += 1; +LL | | } + | |_____^ help: try replacing the loop by: `dst[5..src.len()].clone_from_slice(&src[(3 - 2)..((src.len() - 2) + 3 - 5)]);` + +error: it looks like you're manually copying between slices + --> $DIR/with_loop_counters.rs:41:5 + | +LL | / for i in 0..dst.len() { +LL | | dst[i] = src[count]; +LL | | count += 1; +LL | | } + | |_____^ help: try replacing the loop by: `dst.clone_from_slice(&src[2..(dst.len() + 2)]);` + +error: it looks like you're manually copying between slices + --> $DIR/with_loop_counters.rs:47:5 + | +LL | / for i in 3..10 { +LL | | dst[i] = src[count]; +LL | | count += 1; +LL | | } + | |_____^ help: try replacing the loop by: `dst[3..10].clone_from_slice(&src[5..(10 + 5 - 3)]);` + +error: it looks like you're manually copying between slices + --> $DIR/with_loop_counters.rs:54:5 + | +LL | / for i in 0..src.len() { +LL | | dst[count] = src[i]; +LL | | dst2[count2] = src[i]; +LL | | count += 1; +LL | | count2 += 1; +LL | | } + | |_____^ + | +help: try replacing the loop by + | +LL | dst[3..(src.len() + 3)].clone_from_slice(&src[..]); +LL | dst2[30..(src.len() + 30)].clone_from_slice(&src[..]); + | + +error: it looks like you're manually copying between slices + --> $DIR/with_loop_counters.rs:64:5 + | +LL | / for i in 0..1 << 1 { +LL | | dst[count] = src[i + 2]; +LL | | count += 1; +LL | | } + | |_____^ help: try replacing the loop by: `dst[(0 << 1)..((1 << 1) + (0 << 1))].clone_from_slice(&src[2..((1 << 1) + 2)]);` + +error: it looks like you're manually copying between slices + --> $DIR/with_loop_counters.rs:71:5 + | +LL | / for i in 3..src.len() { +LL | | dst[i] = src[count]; +LL | | count += 1 +LL | | } + | |_____^ help: try replacing the loop by: `dst[3..src.len()].clone_from_slice(&src[..(src.len() - 3)]);` + +error: aborting due to 11 previous errors + diff --git a/src/tools/clippy/tests/ui/manual_memcpy.rs b/src/tools/clippy/tests/ui/manual_memcpy/without_loop_counters.rs similarity index 100% rename from src/tools/clippy/tests/ui/manual_memcpy.rs rename to src/tools/clippy/tests/ui/manual_memcpy/without_loop_counters.rs diff --git a/src/tools/clippy/tests/ui/manual_memcpy/without_loop_counters.stderr b/src/tools/clippy/tests/ui/manual_memcpy/without_loop_counters.stderr new file mode 100644 index 00000000000..54b966f6e54 --- /dev/null +++ b/src/tools/clippy/tests/ui/manual_memcpy/without_loop_counters.stderr @@ -0,0 +1,115 @@ +error: it looks like you're manually copying between slices + --> $DIR/without_loop_counters.rs:7:5 + | +LL | / for i in 0..src.len() { +LL | | dst[i] = src[i]; +LL | | } + | |_____^ help: try replacing the loop by: `dst[..src.len()].clone_from_slice(&src[..]);` + | + = note: `-D clippy::manual-memcpy` implied by `-D warnings` + +error: it looks like you're manually copying between slices + --> $DIR/without_loop_counters.rs:12:5 + | +LL | / for i in 0..src.len() { +LL | | dst[i + 10] = src[i]; +LL | | } + | |_____^ help: try replacing the loop by: `dst[10..(src.len() + 10)].clone_from_slice(&src[..]);` + +error: it looks like you're manually copying between slices + --> $DIR/without_loop_counters.rs:17:5 + | +LL | / for i in 0..src.len() { +LL | | dst[i] = src[i + 10]; +LL | | } + | |_____^ help: try replacing the loop by: `dst[..src.len()].clone_from_slice(&src[10..(src.len() + 10)]);` + +error: it looks like you're manually copying between slices + --> $DIR/without_loop_counters.rs:22:5 + | +LL | / for i in 11..src.len() { +LL | | dst[i] = src[i - 10]; +LL | | } + | |_____^ help: try replacing the loop by: `dst[11..src.len()].clone_from_slice(&src[(11 - 10)..(src.len() - 10)]);` + +error: it looks like you're manually copying between slices + --> $DIR/without_loop_counters.rs:27:5 + | +LL | / for i in 0..dst.len() { +LL | | dst[i] = src[i]; +LL | | } + | |_____^ help: try replacing the loop by: `dst.clone_from_slice(&src[..dst.len()]);` + +error: it looks like you're manually copying between slices + --> $DIR/without_loop_counters.rs:40:5 + | +LL | / for i in 10..256 { +LL | | dst[i] = src[i - 5]; +LL | | dst2[i + 500] = src[i] +LL | | } + | |_____^ + | +help: try replacing the loop by + | +LL | dst[10..256].clone_from_slice(&src[(10 - 5)..(256 - 5)]); +LL | dst2[(10 + 500)..(256 + 500)].clone_from_slice(&src[10..256]); + | + +error: it looks like you're manually copying between slices + --> $DIR/without_loop_counters.rs:52:5 + | +LL | / for i in 10..LOOP_OFFSET { +LL | | dst[i + LOOP_OFFSET] = src[i - some_var]; +LL | | } + | |_____^ help: try replacing the loop by: `dst[(10 + LOOP_OFFSET)..(LOOP_OFFSET + LOOP_OFFSET)].clone_from_slice(&src[(10 - some_var)..(LOOP_OFFSET - some_var)]);` + +error: it looks like you're manually copying between slices + --> $DIR/without_loop_counters.rs:65:5 + | +LL | / for i in 0..src_vec.len() { +LL | | dst_vec[i] = src_vec[i]; +LL | | } + | |_____^ help: try replacing the loop by: `dst_vec[..src_vec.len()].clone_from_slice(&src_vec[..]);` + +error: it looks like you're manually copying between slices + --> $DIR/without_loop_counters.rs:94:5 + | +LL | / for i in from..from + src.len() { +LL | | dst[i] = src[i - from]; +LL | | } + | |_____^ help: try replacing the loop by: `dst[from..(from + src.len())].clone_from_slice(&src[..(from + src.len() - from)]);` + +error: it looks like you're manually copying between slices + --> $DIR/without_loop_counters.rs:98:5 + | +LL | / for i in from..from + 3 { +LL | | dst[i] = src[i - from]; +LL | | } + | |_____^ help: try replacing the loop by: `dst[from..(from + 3)].clone_from_slice(&src[..(from + 3 - from)]);` + +error: it looks like you're manually copying between slices + --> $DIR/without_loop_counters.rs:103:5 + | +LL | / for i in 0..5 { +LL | | dst[i - 0] = src[i]; +LL | | } + | |_____^ help: try replacing the loop by: `dst[..5].clone_from_slice(&src[..5]);` + +error: it looks like you're manually copying between slices + --> $DIR/without_loop_counters.rs:108:5 + | +LL | / for i in 0..0 { +LL | | dst[i] = src[i]; +LL | | } + | |_____^ help: try replacing the loop by: `dst[..0].clone_from_slice(&src[..0]);` + +error: it looks like you're manually copying between slices + --> $DIR/without_loop_counters.rs:120:5 + | +LL | / for i in 0..src.len() { +LL | | dst[i] = src[i].clone(); +LL | | } + | |_____^ help: try replacing the loop by: `dst[..src.len()].clone_from_slice(&src[..]);` + +error: aborting due to 13 previous errors + diff --git a/src/tools/clippy/tests/ui/manual_unwrap_or.fixed b/src/tools/clippy/tests/ui/manual_unwrap_or.fixed new file mode 100644 index 00000000000..a8736f1e6ef --- /dev/null +++ b/src/tools/clippy/tests/ui/manual_unwrap_or.fixed @@ -0,0 +1,68 @@ +// run-rustfix +#![allow(dead_code)] + +fn unwrap_or() { + // int case + Some(1).unwrap_or(42); + + // int case reversed + Some(1).unwrap_or(42); + + // richer none expr + Some(1).unwrap_or(1 + 42); + + // multiline case + #[rustfmt::skip] + Some(1).unwrap_or({ + 42 + 42 + + 42 + 42 + 42 + + 42 + 42 + 42 + }); + + // string case + Some("Bob").unwrap_or("Alice"); + + // don't lint + match Some(1) { + Some(i) => i + 2, + None => 42, + }; + match Some(1) { + Some(i) => i, + None => return, + }; + for j in 0..4 { + match Some(j) { + Some(i) => i, + None => continue, + }; + match Some(j) { + Some(i) => i, + None => break, + }; + } + + // cases where the none arm isn't a constant expression + // are not linted due to potential ownership issues + + // ownership issue example, don't lint + struct NonCopyable; + let mut option: Option = None; + match option { + Some(x) => x, + None => { + option = Some(NonCopyable); + // some more code ... + option.unwrap() + }, + }; + + // ownership issue example, don't lint + let option: Option<&str> = None; + match option { + Some(s) => s, + None => &format!("{} {}!", "hello", "world"), + }; +} + +fn main() {} diff --git a/src/tools/clippy/tests/ui/manual_unwrap_or.rs b/src/tools/clippy/tests/ui/manual_unwrap_or.rs new file mode 100644 index 00000000000..bede8cffc32 --- /dev/null +++ b/src/tools/clippy/tests/ui/manual_unwrap_or.rs @@ -0,0 +1,83 @@ +// run-rustfix +#![allow(dead_code)] + +fn unwrap_or() { + // int case + match Some(1) { + Some(i) => i, + None => 42, + }; + + // int case reversed + match Some(1) { + None => 42, + Some(i) => i, + }; + + // richer none expr + match Some(1) { + Some(i) => i, + None => 1 + 42, + }; + + // multiline case + #[rustfmt::skip] + match Some(1) { + Some(i) => i, + None => { + 42 + 42 + + 42 + 42 + 42 + + 42 + 42 + 42 + } + }; + + // string case + match Some("Bob") { + Some(i) => i, + None => "Alice", + }; + + // don't lint + match Some(1) { + Some(i) => i + 2, + None => 42, + }; + match Some(1) { + Some(i) => i, + None => return, + }; + for j in 0..4 { + match Some(j) { + Some(i) => i, + None => continue, + }; + match Some(j) { + Some(i) => i, + None => break, + }; + } + + // cases where the none arm isn't a constant expression + // are not linted due to potential ownership issues + + // ownership issue example, don't lint + struct NonCopyable; + let mut option: Option = None; + match option { + Some(x) => x, + None => { + option = Some(NonCopyable); + // some more code ... + option.unwrap() + }, + }; + + // ownership issue example, don't lint + let option: Option<&str> = None; + match option { + Some(s) => s, + None => &format!("{} {}!", "hello", "world"), + }; +} + +fn main() {} diff --git a/src/tools/clippy/tests/ui/manual_unwrap_or.stderr b/src/tools/clippy/tests/ui/manual_unwrap_or.stderr new file mode 100644 index 00000000000..674f2952635 --- /dev/null +++ b/src/tools/clippy/tests/ui/manual_unwrap_or.stderr @@ -0,0 +1,61 @@ +error: this pattern reimplements `Option::unwrap_or` + --> $DIR/manual_unwrap_or.rs:6:5 + | +LL | / match Some(1) { +LL | | Some(i) => i, +LL | | None => 42, +LL | | }; + | |_____^ help: replace with: `Some(1).unwrap_or(42)` + | + = note: `-D clippy::manual-unwrap-or` implied by `-D warnings` + +error: this pattern reimplements `Option::unwrap_or` + --> $DIR/manual_unwrap_or.rs:12:5 + | +LL | / match Some(1) { +LL | | None => 42, +LL | | Some(i) => i, +LL | | }; + | |_____^ help: replace with: `Some(1).unwrap_or(42)` + +error: this pattern reimplements `Option::unwrap_or` + --> $DIR/manual_unwrap_or.rs:18:5 + | +LL | / match Some(1) { +LL | | Some(i) => i, +LL | | None => 1 + 42, +LL | | }; + | |_____^ help: replace with: `Some(1).unwrap_or(1 + 42)` + +error: this pattern reimplements `Option::unwrap_or` + --> $DIR/manual_unwrap_or.rs:25:5 + | +LL | / match Some(1) { +LL | | Some(i) => i, +LL | | None => { +LL | | 42 + 42 +... | +LL | | } +LL | | }; + | |_____^ + | +help: replace with + | +LL | Some(1).unwrap_or({ +LL | 42 + 42 +LL | + 42 + 42 + 42 +LL | + 42 + 42 + 42 +LL | }); + | + +error: this pattern reimplements `Option::unwrap_or` + --> $DIR/manual_unwrap_or.rs:35:5 + | +LL | / match Some("Bob") { +LL | | Some(i) => i, +LL | | None => "Alice", +LL | | }; + | |_____^ help: replace with: `Some("Bob").unwrap_or("Alice")` + +error: aborting due to 5 previous errors + diff --git a/src/tools/clippy/tests/ui/ptr_eq.fixed b/src/tools/clippy/tests/ui/ptr_eq.fixed new file mode 100644 index 00000000000..209081e6e80 --- /dev/null +++ b/src/tools/clippy/tests/ui/ptr_eq.fixed @@ -0,0 +1,38 @@ +// run-rustfix +#![warn(clippy::ptr_eq)] + +macro_rules! mac { + ($a:expr, $b:expr) => { + $a as *const _ as usize == $b as *const _ as usize + }; +} + +macro_rules! another_mac { + ($a:expr, $b:expr) => { + $a as *const _ == $b as *const _ + }; +} + +fn main() { + let a = &[1, 2, 3]; + let b = &[1, 2, 3]; + + let _ = std::ptr::eq(a, b); + let _ = std::ptr::eq(a, b); + let _ = a.as_ptr() == b as *const _; + let _ = a.as_ptr() == b.as_ptr(); + + // Do not lint + + let _ = mac!(a, b); + let _ = another_mac!(a, b); + + let a = &mut [1, 2, 3]; + let b = &mut [1, 2, 3]; + + let _ = a.as_mut_ptr() == b as *mut [i32] as *mut _; + let _ = a.as_mut_ptr() == b.as_mut_ptr(); + + let _ = a == b; + let _ = core::ptr::eq(a, b); +} diff --git a/src/tools/clippy/tests/ui/ptr_eq.rs b/src/tools/clippy/tests/ui/ptr_eq.rs new file mode 100644 index 00000000000..69162870807 --- /dev/null +++ b/src/tools/clippy/tests/ui/ptr_eq.rs @@ -0,0 +1,38 @@ +// run-rustfix +#![warn(clippy::ptr_eq)] + +macro_rules! mac { + ($a:expr, $b:expr) => { + $a as *const _ as usize == $b as *const _ as usize + }; +} + +macro_rules! another_mac { + ($a:expr, $b:expr) => { + $a as *const _ == $b as *const _ + }; +} + +fn main() { + let a = &[1, 2, 3]; + let b = &[1, 2, 3]; + + let _ = a as *const _ as usize == b as *const _ as usize; + let _ = a as *const _ == b as *const _; + let _ = a.as_ptr() == b as *const _; + let _ = a.as_ptr() == b.as_ptr(); + + // Do not lint + + let _ = mac!(a, b); + let _ = another_mac!(a, b); + + let a = &mut [1, 2, 3]; + let b = &mut [1, 2, 3]; + + let _ = a.as_mut_ptr() == b as *mut [i32] as *mut _; + let _ = a.as_mut_ptr() == b.as_mut_ptr(); + + let _ = a == b; + let _ = core::ptr::eq(a, b); +} diff --git a/src/tools/clippy/tests/ui/ptr_eq.stderr b/src/tools/clippy/tests/ui/ptr_eq.stderr new file mode 100644 index 00000000000..45d8c60382b --- /dev/null +++ b/src/tools/clippy/tests/ui/ptr_eq.stderr @@ -0,0 +1,16 @@ +error: use `std::ptr::eq` when comparing raw pointers + --> $DIR/ptr_eq.rs:20:13 + | +LL | let _ = a as *const _ as usize == b as *const _ as usize; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `std::ptr::eq(a, b)` + | + = note: `-D clippy::ptr-eq` implied by `-D warnings` + +error: use `std::ptr::eq` when comparing raw pointers + --> $DIR/ptr_eq.rs:21:13 + | +LL | let _ = a as *const _ == b as *const _; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `std::ptr::eq(a, b)` + +error: aborting due to 2 previous errors + diff --git a/src/tools/clippy/tests/ui/result_unit_error.rs b/src/tools/clippy/tests/ui/result_unit_error.rs new file mode 100644 index 00000000000..a66f581b215 --- /dev/null +++ b/src/tools/clippy/tests/ui/result_unit_error.rs @@ -0,0 +1,38 @@ +#[warn(clippy::result_unit_err)] +#[allow(unused)] + +pub fn returns_unit_error() -> Result { + Err(()) +} + +fn private_unit_errors() -> Result { + Err(()) +} + +pub trait HasUnitError { + fn get_that_error(&self) -> Result; + + fn get_this_one_too(&self) -> Result { + Err(()) + } +} + +impl HasUnitError for () { + fn get_that_error(&self) -> Result { + Ok(true) + } +} + +trait PrivateUnitError { + fn no_problem(&self) -> Result; +} + +pub struct UnitErrorHolder; + +impl UnitErrorHolder { + pub fn unit_error(&self) -> Result { + Ok(0) + } +} + +fn main() {} diff --git a/src/tools/clippy/tests/ui/result_unit_error.stderr b/src/tools/clippy/tests/ui/result_unit_error.stderr new file mode 100644 index 00000000000..b8230032491 --- /dev/null +++ b/src/tools/clippy/tests/ui/result_unit_error.stderr @@ -0,0 +1,35 @@ +error: this returns a `Result<_, ()> + --> $DIR/result_unit_error.rs:4:1 + | +LL | pub fn returns_unit_error() -> Result { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: `-D clippy::result-unit-err` implied by `-D warnings` + = help: use a custom Error type instead + +error: this returns a `Result<_, ()> + --> $DIR/result_unit_error.rs:13:5 + | +LL | fn get_that_error(&self) -> Result; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = help: use a custom Error type instead + +error: this returns a `Result<_, ()> + --> $DIR/result_unit_error.rs:15:5 + | +LL | fn get_this_one_too(&self) -> Result { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = help: use a custom Error type instead + +error: this returns a `Result<_, ()> + --> $DIR/result_unit_error.rs:33:5 + | +LL | pub fn unit_error(&self) -> Result { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = help: use a custom Error type instead + +error: aborting due to 4 previous errors + diff --git a/src/tools/clippy/tests/ui/same_functions_in_if_condition.rs b/src/tools/clippy/tests/ui/same_functions_in_if_condition.rs index 686867cf5c6..7f28f025790 100644 --- a/src/tools/clippy/tests/ui/same_functions_in_if_condition.rs +++ b/src/tools/clippy/tests/ui/same_functions_in_if_condition.rs @@ -77,4 +77,14 @@ fn ifs_same_cond_fn() { } } -fn main() {} +fn main() { + // macro as condition (see #6168) + let os = if cfg!(target_os = "macos") { + "macos" + } else if cfg!(target_os = "windows") { + "windows" + } else { + "linux" + }; + println!("{}", os); +} diff --git a/src/tools/clippy/tests/ui/shadow.rs b/src/tools/clippy/tests/ui/shadow.rs index bd91ae4e934..e366c75335c 100644 --- a/src/tools/clippy/tests/ui/shadow.rs +++ b/src/tools/clippy/tests/ui/shadow.rs @@ -8,6 +8,7 @@ #![allow( unused_parens, unused_variables, + clippy::manual_unwrap_or, clippy::missing_docs_in_private_items, clippy::single_match )] diff --git a/src/tools/clippy/tests/ui/shadow.stderr b/src/tools/clippy/tests/ui/shadow.stderr index 8a831375b41..7c1ad2949e9 100644 --- a/src/tools/clippy/tests/ui/shadow.stderr +++ b/src/tools/clippy/tests/ui/shadow.stderr @@ -1,135 +1,135 @@ error: `x` is shadowed by itself in `&mut x` - --> $DIR/shadow.rs:26:5 + --> $DIR/shadow.rs:27:5 | LL | let x = &mut x; | ^^^^^^^^^^^^^^^ | = note: `-D clippy::shadow-same` implied by `-D warnings` note: previous binding is here - --> $DIR/shadow.rs:25:13 + --> $DIR/shadow.rs:26:13 | LL | let mut x = 1; | ^ error: `x` is shadowed by itself in `{ x }` - --> $DIR/shadow.rs:27:5 - | -LL | let x = { x }; - | ^^^^^^^^^^^^^^ - | -note: previous binding is here - --> $DIR/shadow.rs:26:9 - | -LL | let x = &mut x; - | ^ - -error: `x` is shadowed by itself in `(&*x)` --> $DIR/shadow.rs:28:5 | -LL | let x = (&*x); +LL | let x = { x }; | ^^^^^^^^^^^^^^ | note: previous binding is here --> $DIR/shadow.rs:27:9 | +LL | let x = &mut x; + | ^ + +error: `x` is shadowed by itself in `(&*x)` + --> $DIR/shadow.rs:29:5 + | +LL | let x = (&*x); + | ^^^^^^^^^^^^^^ + | +note: previous binding is here + --> $DIR/shadow.rs:28:9 + | LL | let x = { x }; | ^ error: `x` is shadowed by `{ *x + 1 }` which reuses the original value - --> $DIR/shadow.rs:29:9 + --> $DIR/shadow.rs:30:9 | LL | let x = { *x + 1 }; | ^ | = note: `-D clippy::shadow-reuse` implied by `-D warnings` note: initialization happens here - --> $DIR/shadow.rs:29:13 + --> $DIR/shadow.rs:30:13 | LL | let x = { *x + 1 }; | ^^^^^^^^^^ note: previous binding is here - --> $DIR/shadow.rs:28:9 + --> $DIR/shadow.rs:29:9 | LL | let x = (&*x); | ^ error: `x` is shadowed by `id(x)` which reuses the original value - --> $DIR/shadow.rs:30:9 - | -LL | let x = id(x); - | ^ - | -note: initialization happens here - --> $DIR/shadow.rs:30:13 - | -LL | let x = id(x); - | ^^^^^ -note: previous binding is here - --> $DIR/shadow.rs:29:9 - | -LL | let x = { *x + 1 }; - | ^ - -error: `x` is shadowed by `(1, x)` which reuses the original value --> $DIR/shadow.rs:31:9 | -LL | let x = (1, x); +LL | let x = id(x); | ^ | note: initialization happens here --> $DIR/shadow.rs:31:13 | -LL | let x = (1, x); - | ^^^^^^ +LL | let x = id(x); + | ^^^^^ note: previous binding is here --> $DIR/shadow.rs:30:9 | -LL | let x = id(x); +LL | let x = { *x + 1 }; | ^ -error: `x` is shadowed by `first(x)` which reuses the original value +error: `x` is shadowed by `(1, x)` which reuses the original value --> $DIR/shadow.rs:32:9 | -LL | let x = first(x); +LL | let x = (1, x); | ^ | note: initialization happens here --> $DIR/shadow.rs:32:13 | +LL | let x = (1, x); + | ^^^^^^ +note: previous binding is here + --> $DIR/shadow.rs:31:9 + | +LL | let x = id(x); + | ^ + +error: `x` is shadowed by `first(x)` which reuses the original value + --> $DIR/shadow.rs:33:9 + | +LL | let x = first(x); + | ^ + | +note: initialization happens here + --> $DIR/shadow.rs:33:13 + | LL | let x = first(x); | ^^^^^^^^ note: previous binding is here - --> $DIR/shadow.rs:31:9 + --> $DIR/shadow.rs:32:9 | LL | let x = (1, x); | ^ error: `x` is being shadowed - --> $DIR/shadow.rs:34:9 + --> $DIR/shadow.rs:35:9 | LL | let x = y; | ^ | = note: `-D clippy::shadow-unrelated` implied by `-D warnings` note: initialization happens here - --> $DIR/shadow.rs:34:13 + --> $DIR/shadow.rs:35:13 | LL | let x = y; | ^ note: previous binding is here - --> $DIR/shadow.rs:32:9 + --> $DIR/shadow.rs:33:9 | LL | let x = first(x); | ^ error: `x` shadows a previous declaration - --> $DIR/shadow.rs:36:5 + --> $DIR/shadow.rs:37:5 | LL | let x; | ^^^^^^ | note: previous binding is here - --> $DIR/shadow.rs:34:9 + --> $DIR/shadow.rs:35:9 | LL | let x = y; | ^ diff --git a/src/tools/clippy/tests/ui/update-references.sh b/src/tools/clippy/tests/ui/update-references.sh index 2c13c327d79..e16ed600ef8 100755 --- a/src/tools/clippy/tests/ui/update-references.sh +++ b/src/tools/clippy/tests/ui/update-references.sh @@ -30,15 +30,27 @@ while [[ "$1" != "" ]]; do ! (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/used_underscore_binding.rs b/src/tools/clippy/tests/ui/used_underscore_binding.rs index 8e0243c49aa..d8bda7e8f48 100644 --- a/src/tools/clippy/tests/ui/used_underscore_binding.rs +++ b/src/tools/clippy/tests/ui/used_underscore_binding.rs @@ -3,7 +3,7 @@ #![feature(rustc_private)] #![warn(clippy::all)] -#![allow(clippy::blacklisted_name)] +#![allow(clippy::blacklisted_name, clippy::eq_op)] #![warn(clippy::used_underscore_binding)] #[macro_use] diff --git a/src/tools/rustc-workspace-hack/Cargo.toml b/src/tools/rustc-workspace-hack/Cargo.toml index 11d61606ff5..213cb07fddf 100644 --- a/src/tools/rustc-workspace-hack/Cargo.toml +++ b/src/tools/rustc-workspace-hack/Cargo.toml @@ -66,7 +66,7 @@ crossbeam-utils = { version = "0.7.2", features = ["nightly"] } proc-macro2 = { version = "1", features = ["default"] } quote = { version = "1", features = ["default"] } serde = { version = "1.0.82", features = ['derive'] } -serde_json = { version = "1.0.31", features = ["raw_value"] } +serde_json = { version = "1.0.31", features = ["raw_value", "unbounded_depth"] } smallvec-0_6 = { package = "smallvec", version = "0.6", features = ['union', 'may_dangle'] } smallvec = { version = "1.0", features = ['union', 'may_dangle'] } syn = { version = "1", features = ['fold', 'full', 'extra-traits', 'visit', 'visit-mut'] }