linliu-code opened a new issue, #683: URL: https://github.com/apache/hudi-rs/issues/683
## Problem `hudi-core` has 27 items with no production caller. That on its own would be ordinary cleanup, but the reason it went unnoticed is the part worth fixing: **the compiler already detects every one of them, and 27 `allow(dead_code)` attributes switch the detection off.** CI runs `cargo clippy -p hudi-core --lib -- -D warnings` (`.github/workflows/ci.yml`), which promotes `dead_code` to an error. Because `cargo check --lib` compiles without test targets, anything reachable only from `#[cfg(test)]` code is reported as dead, which is exactly the "no production caller" question. The attributes suppress that, so the gate passes while the condition it exists to catch is present. This matters beyond tidiness. The same shape has produced real defects here recently: a five-gate log-block scan whose completed/inflight gate was fully unit-tested and never armed in production (#678), and a header-only log-read tier that existed in full with no caller outside the memory bench (#682). In both cases the code was correct and unreachable, and the tests passed because they built their own inputs. Unreachable code is not free: it reads as live to the next person changing nearby code. ## Reproduction ```bash # strip every `#[allow(dead_code)]` / `#![allow(dead_code)]` under crates/core/src, then: cargo check -p hudi-core --lib 2>&1 | grep -cE "never used|never read|never constructed" # => 27 ``` Four of the attributes are module-wide `#![allow(dead_code)]`, which disables the check for a whole file rather than one item. ## The 27 Line numbers drift; regenerate with the command above rather than trusting these. | Location | Item | |---|---| | `file_group/log_file/content.rs` | field `hudi_configs` | | `file_group/reader_v2/buffered_record_converter.rs` | method `convert` | | `file_group/reader_v2/delete_context.rs` | `from_reader_schema` | | `file_group/reader_v2/engine.rs` | field `iterator_mode` | | `file_group/reader_v2/engine.rs` | `builder`, `open`, `set_output_converter`, `set_buffered_record_converter`, `read_stats`, `valid_block_instants` | | `file_group/reader_v2/engine.rs` | struct `HoodieFileGroupReaderBuilder` never constructed, plus its methods | | `file_group/reader_v2/gaps.rs` | variants `VersionOne`, `Both` | | `file_group/reader_v2/log_record_reader.rs` | field `instant_times_included` | | `file_group/reader_v2/log_record_reader.rs` | fields `force_full_scan`, `allow_inflight_instants` | | `file_group/reader_v2/log_record_reader.rs` | multiple methods | | `file_group/reader_v2/merge_iterator.rs` | `new_eager_from_vec`, `current_in_memory_bytes` | | `file_group/reader_v2/merged_log_record_reader.rs` | field `num_merged_records_in_log` | | `file_group/reader_v2/merged_log_record_reader.rs` | `scan`, `scan_with_skip`, `with_instant_range`, plus more methods | | `file_group/reader_v2/reader_context.rs` | `partition_path`, `empty` | | `file_group/reader_v2/record_context.rs` | fields `populate_meta_fields`, `partition_path` | | `file_group/reader_v2/record_context.rs` | `delete_batch_to_keys`, `get_schema_from_buffer_record` | | `file_group/reader_v2/row_serde.rs` | function `seal` | | `file_group/reader_v2/schema_handler.rs` | `with_data_schema_json`, `with_requested_schema_json`, `schema_for_updates`, `set_schema_for_updates` | | `hfile/block.rs` | fields `prev_block_offset`, `checksum_type`, `on_disk_data_size_with_header`; methods `iter`, `data`; struct `DataBlockIterator` | | `hfile/trailer.rs` | multiple fields | | `timeline/loader.rs` | method `storage` | | `timeline/view.rs` | field `start_timestamp` | Several are plainly deliberate parity scaffolding mirroring Hudi's Java reader, so **unused does not mean delete**. `allow_inflight_instants`, for instance, is Java's escape hatch for the completed/inflight check that #678 wires up. ## Proposed work The deletions are not the deliverable; re-arming the detector is. 1. Triage each item into **genuinely dead** (delete) or **deliberately dormant** (keep). 2. Replace every kept item's `#[allow(dead_code)]` with `#[expect(dead_code, reason = "...")]`. MSRV here is 1.91.1, so `expect` is available, and it inverts the failure mode: it errors when the item *becomes* used, and it forces a written justification. Silent suppression becomes a maintained claim, and "inert by design" stops being visually identical to "inert by accident". 3. Ban module-wide `#![allow(dead_code)]`; require per-item with a reason. After that the existing CI leg enforces this by itself, with no new tooling. ## Known limitation This detector only sees crate-internal items. `pub` items in `pub` modules are never flagged, because they are reachable through the public API. That blind spot is exactly why the orphaned log-read tier in #682 needed a manual cross-crate grep to find. A complete sweep needs a second pass over the public surface. -- 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]
