emilk opened a new pull request, #24848: URL: https://github.com/apache/datafusion/pull/24848
## Which issue does this PR close? - Part of #18467. ## Rationale for this change Continuing the work in: - #24466 - #24566 - #24837 Turn on more `clippy::pedantic` lints from the opt-out list in `Cargo.toml`. ## What changes are included in this PR? One commit per lint, each removing its `"allow"` line from `Cargo.toml` and fixing every site. Review one commit at a time! Let me know if you disagree with any and I'll revert it. | Lint | Sites | Fix | | ------------------------------------------------------------------------------------------------------------------------------ | ----: | ------------------------------------------------------------------------------ | | [`manual_string_new`](https://rust-lang.github.io/rust-clippy/master/index.html#manual_string_new) | 82 | `String::new()` instead of `"".to_string()` / `"".to_owned()` / `"".into()` | | [`from_iter_instead_of_collect`](https://rust-lang.github.io/rust-clippy/master/index.html#from_iter_instead_of_collect) | 56 | `iter.collect::<T>()` instead of `T::from_iter(iter)`, so it reads in evaluation order | | [`ignored_unit_patterns`](https://rust-lang.github.io/rust-clippy/master/index.html#ignored_unit_patterns) | 46 | `Ok(())` instead of `Ok(_)`, so the pattern stops matching if a payload is ever added | | [`redundant_else`](https://rust-lang.github.io/rust-clippy/master/index.html#redundant_else) | 48 | dropped `else` after a diverging branch | | [`unnested_or_patterns`](https://rust-lang.github.io/rust-clippy/master/index.html#unnested_or_patterns) | 68 | `Time32(Microsecond \| Nanosecond)` instead of repeating the prefix | | [`match_bool`](https://rust-lang.github.io/rust-clippy/master/index.html#match_bool) | 46 | `if`/`else` instead of `match` on a bool | | [`used_underscore_items`](https://rust-lang.github.io/rust-clippy/master/index.html#used_underscore_items) | 28 | dropped the leading `_` from eleven private helpers | | [`missing_fields_in_debug`](https://rust-lang.github.io/rust-clippy/master/index.html#missing_fields_in_debug) | 28 | `finish_non_exhaustive()` on manual `Debug` impls that skip a field | Two things worth a closer look: - **`used_underscore_items` renames a public trait method.** Two helpers needed a new name because the un-prefixed one was taken: `_date_trunc_coarse` -> `truncate_datetime_fields` (private), and `ReadOptions::_get_resolved_schema` -> `infer_schema_if_needed`, since the trait's required method is already called `get_resolved_schema`. `ReadOptions` is public, so this is an API change. - **`missing_fields_in_debug` spares `Column`.** Its `Debug` output is embedded verbatim in user-facing error messages (`Physical plan does not support logical expression ...`), so `finish_non_exhaustive()` would put a bare `..` in front of users and it broke six `.slt` expectations. That impl keeps `finish()` under an `#[expect]` with a reason; the other 27 are converted. For `redundant_else`, where both branches diverged the fix is `return if X { A } else { B };` rather than two sequential `return`s. ## What is the testing strategy for this PR? `cargo clippy --workspace --all-targets --all-features -- -D warnings` reports no warnings, and the extended test suite passes (69 test binaries). The changes are mechanical and behavior-preserving, so no new tests. ## Are there any user-facing changes? Yes, one: `ReadOptions::_get_resolved_schema` is renamed to `infer_schema_if_needed`. See above. -- 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]
