emilk opened a new pull request, #24466: URL: https://github.com/apache/datafusion/pull/24466
## Which issue does this PR close? - Part of #18467. ## Rationale for this change Stacked on #24465, which turns on `clippy::pedantic` as a group and opts out of every lint that still fires, each with its hit count. This PR pays down eight of those opt-outs: cheap ones where the fix is mechanical and the lint is worth keeping on afterwards. Contains the two commits from #24465, so review this one after that. ## What changes are included in this PR? One commit per lint, each removing its `"allow"` line from `Cargo.toml` and fixing every site. | Lint | Sites | Fix | | ---------------------------------------------------------------------------------------------------------- | ----: | ------------------------------------------------------- | | [`collapsible_else_if`](https://rust-lang.github.io/rust-clippy/master/index.html#collapsible_else_if) | 1 | flatten `else { if .. }` into `else if` | | [`range_plus_one`](https://rust-lang.github.io/rust-clippy/master/index.html#range_plus_one) | 12 | `a..=b` instead of `a..b + 1` | | [`stable_sort_primitive`](https://rust-lang.github.io/rust-clippy/master/index.html#stable_sort_primitive) | 14 | `sort_unstable` where stability cannot be observed | | [`comparison_chain`](https://rust-lang.github.io/rust-clippy/master/index.html#comparison_chain) | 3 | `match Ord::cmp` instead of an `if`/`else if` chain | | [`should_panic_without_expect`](https://rust-lang.github.io/rust-clippy/master/index.html#should_panic_without_expect) | 1 | name the expected panic message | | [`manual_assert_eq`](https://rust-lang.github.io/rust-clippy/master/index.html#manual_assert_eq) | 25 | `assert_eq!`/`assert_ne!` so failures print both values | | [`needless_for_each`](https://rust-lang.github.io/rust-clippy/master/index.html#needless_for_each) | 47 | `for` loops instead of `for_each` | | [`manual_let_else`](https://rust-lang.github.io/rust-clippy/master/index.html#manual_let_else) | 106 | `let .. else` instead of a diverging `match` | Two notes for reviewers: - `needless_for_each` has 47 sites, not the 44 the opt-out recorded. Three live in `#[cfg(not(feature = "force_hash_collisions"))]` branches of `hash_utils`, which `--all-features` compiles away, so our lint command never saw them. - Three `manual_let_else` rewrites in the Substrait consumer needed an explicit `<[Expr; N]>::try_from(args)`; as `args.try_into()` the target array type is no longer inferable from the `let` pattern. ## Are these changes tested? `cargo clippy --workspace --all-targets --all-features` reports no warnings, and the extended test suite passes. The changes are mechanical and behavior-preserving, so no new tests. ## Are there any user-facing changes? No. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
