chore: remove stale #[allow(dead_code)] from expr visitors and spec

Part of #2559.

- expr/visitors: gate strict_metrics_evaluator and strict_projection
  behind cfg(test) — they are only exercised by their own unit tests
  until delete/overwrite support starts using them — and drop the
  per-item allows.
- spec/manifest/writer: add_delete_entry and add_existing_entry are
  only called from unit tests, so mark them cfg(test) instead of
  allow(dead_code).
- spec/snapshot: remove the unused Snapshot::log() helper.
- spec/values/decimal_utils: decimal_new is a test-only helper, so
  mark it cfg(test).

Co-authored-by: Isaac

Author: huan233usc

crates/iceberg/src/expr/visitors/mod.rs

@@ -24,5 +24,8 @@ pub(crate) mod page_index_evaluator;
 pub(crate) mod predicate_visitor;
 pub(crate) mod rewrite_not;
 pub(crate) mod row_group_metrics_evaluator;
+// TODO: Remove the `cfg(test)` once delete/overwrite support starts using the strict visitors.
+#[cfg(test)]
 pub(crate) mod strict_metrics_evaluator;
+#[cfg(test)]
 pub(crate) mod strict_projection;

crates/iceberg/src/expr/visitors/strict_metrics_evaluator.rs

@@ -22,12 +22,9 @@ use crate::expr::{BoundPredicate, BoundReference};
 use crate::spec::{DataFile, Datum};
 use crate::{Error, ErrorKind, Result};
 
-#[allow(dead_code)]
 const ROWS_MUST_MATCH: Result<bool> = Ok(true);
-#[allow(dead_code)]
 const ROWS_MIGHT_NOT_MATCH: Result<bool> = Ok(false);
 
-#[allow(dead_code)]
 /// Evaluates an `Expression` on a `DataFile` to test whether all rows in the file match.
 ///  
 /// This evaluation is strict: it returns true if all rows in a file must match the expression.
@@ -41,7 +38,6 @@ pub(crate) struct StrictMetricsEvaluator<'a> {
 }
 
 impl<'a> StrictMetricsEvaluator<'a> {
-    #[allow(dead_code)]
     fn new(data_file: &'a DataFile) -> Self {
         StrictMetricsEvaluator { data_file }
     }
@@ -50,7 +46,6 @@ impl<'a> StrictMetricsEvaluator<'a> {
     /// provided [`DataFile`]'s metrics. Used by [`TableScan`] to
     /// see if this `DataFile` contains data that could match
     /// the scan's filter.
-    #[allow(dead_code)]
     pub(crate) fn eval(filter: &'a BoundPredicate, data_file: &'a DataFile) -> crate::Result<bool> {
         if data_file.record_count == 0 {
             return ROWS_MUST_MATCH;

crates/iceberg/src/expr/visitors/strict_projection.rs

@@ -24,15 +24,11 @@ use crate::expr::{BoundPredicate, BoundReference, Predicate};
 use crate::spec::{Datum, PartitionField, PartitionSpecRef};
 use crate::{Error, ErrorKind};
 
-// # TODO
-// Remove this after delete support
-#[allow(dead_code)]
 pub(crate) struct StrictProjection {
     partition_spec: PartitionSpecRef,
     cached_parts: HashMap<i32, Vec<PartitionField>>,
 }
 
-#[allow(dead_code)]
 impl StrictProjection {
     pub(crate) fn new(partition_spec: PartitionSpecRef) -> Self {
         Self {

crates/iceberg/src/spec/manifest/writer.rs

@@ -336,10 +336,7 @@ impl ManifestWriter {
     /// Add a delete manifest entry. This method will update following status of the entry:
     /// - Update the entry status to `Deleted`
     /// - Set the snapshot id to the current snapshot id
-    ///
-    /// # TODO
-    /// Remove this allow later
-    #[allow(dead_code)]
+    #[cfg(test)]
     pub(crate) fn add_delete_entry(&mut self, mut entry: ManifestEntry) -> Result<()> {
         self.check_data_file(&entry.data_file)?;
         entry.status = ManifestStatus::Deleted;
@@ -371,10 +368,7 @@ impl ManifestWriter {
 
     /// Add an existing manifest entry. This method will update following status of the entry:
     /// - Update the entry status to `Existing`
-    ///
-    /// # TODO
-    /// Remove this allow later
-    #[allow(dead_code)]
+    #[cfg(test)]
     pub(crate) fn add_existing_entry(&mut self, mut entry: ManifestEntry) -> Result<()> {
         self.check_data_file(&entry.data_file)?;
         entry.status = ManifestStatus::Existing;

crates/iceberg/src/spec/snapshot.rs

@@ -25,7 +25,6 @@ use chrono::{DateTime, Utc};
 use serde::{Deserialize, Serialize};
 use typed_builder::TypedBuilder;
 
-use super::table_metadata::SnapshotLog;
 use crate::error::{Result, timestamp_ms_to_utc};
 use crate::spec::{SchemaId, SchemaRef, TableMetadata};
 use crate::{Error, ErrorKind};
@@ -193,14 +192,6 @@ impl Snapshot {
         }
     }
 
-    #[allow(dead_code)]
-    pub(crate) fn log(&self) -> SnapshotLog {
-        SnapshotLog {
-            timestamp_ms: self.timestamp_ms,
-            snapshot_id: self.snapshot_id,
-        }
-    }
-
     /// The row-id of the first newly added row in this snapshot. All rows added in this snapshot will
     /// have a row-id assigned to them greater than this value. All rows with a row-id less than this
     /// value were created in a snapshot that was added to the table (but not necessarily committed to

crates/iceberg/src/spec/values/decimal_utils.rs

@@ -83,7 +83,7 @@ pub fn try_decimal_from_i128_with_scale(mantissa: i128, scale: u32) -> Result<De
 /// Create a D128 from i64 mantissa and scale.
 ///
 /// This is equivalent to rust_decimal's `Decimal::new`.
-#[allow(dead_code)]
+#[cfg(test)]
 pub fn decimal_new(mantissa: i64, scale: u32) -> Decimal {
     decimal_from_i128_with_scale(mantissa as i128, scale)
 }