Address reviewer comment

Author: def-

src/repr/src/adt/date.rs

@@ -54,9 +54,9 @@ impl RustType<ProtoDate> for Date {
     }
 
     fn from_proto(proto: ProtoDate) -> Result<Self, TryFromProtoError> {
-        // Go through `from_pg_epoch` so out-of-range days are rejected
-        // here — pushing them into a `Row` succeeds but `read_datum` would
-        // later panic when reconstructing the date.
+        // Go through `from_pg_epoch` so out-of-range days are rejected here.
+        // Pushing them into a `Row` succeeds, but `read_datum` would later
+        // panic when reconstructing the date.
         Date::from_pg_epoch(proto.days)
             .map_err(|err| TryFromProtoError::InvalidFieldError(err.to_string()))
     }

src/repr/src/adt/numeric.rs

@@ -402,9 +402,9 @@ pub fn twos_complement_be_to_numeric_inner<D: Dec<N>, const N: usize>(
     if input.is_empty() {
         // An empty byte string is not a valid two's-complement integer. Our own
         // encoder never emits one (zero is the single byte `0x00`), so this only
-        // arises from untrusted input — e.g. an Avro `decimal` field whose
-        // unscaled value was encoded as zero-length `bytes`. Reject it rather
-        // than indexing `input[0]` below and panicking.
+        // arises from untrusted input. One example is an Avro `decimal` field
+        // whose unscaled value was encoded as zero-length `bytes`. Reject it
+        // rather than indexing `input[0]` below and panicking.
         bail!("cannot parse a numeric value from an empty byte string");
     }
     let is_neg = if (input[0] & 0x80) != 0 {

src/repr/src/adt/timestamp.rs

@@ -899,7 +899,7 @@ impl RustType<ProtoNaiveDateTime> for CheckedTimestamp<NaiveDateTime> {
 
     fn from_proto(proto: ProtoNaiveDateTime) -> Result<Self, TryFromProtoError> {
         // Go through `from_timestamplike` so out-of-range values are
-        // rejected here — pushing them into a `Row` succeeds but
+        // rejected here. Pushing them into a `Row` succeeds, but
         // `read_datum` would panic when reconstructing the timestamp.
         CheckedTimestamp::from_timestamplike(NaiveDateTime::from_proto(proto)?)
             .map_err(|err| TryFromProtoError::InvalidFieldError(err.to_string()))
@@ -1119,8 +1119,8 @@ mod test {
     fn test_round_to_precision_leap_second_off_minute() {
         // Regression: parsing a `:60` literal can leave chrono's leap-second
         // representation (sub-second >= 1s) on a second other than `:59` (after
-        // time-zone math). Rounding such a value — as the string->timestamp cast
-        // does — must not panic. `truncate_microseconds`/`truncate_milliseconds`
+        // time-zone math). Rounding such a value, as the string->timestamp cast
+        // does, must not panic. `truncate_microseconds`/`truncate_milliseconds`
         // previously rebuilt the time with `from_hms_{micro,milli}_opt`, whose
         // leap sub-second range is only valid at `:59`, and unwrapped the `None`.
         let leap = NaiveDate::from_ymd_opt(3, 3, 17)

src/repr/src/relation.rs

@@ -1035,8 +1035,8 @@ impl RustType<ProtoRelationDesc> for RelationDesc {
             Box::new(itertools::repeat_n(val, proto.names.len()))
         } else {
             // Reject mismatched lengths explicitly rather than panicking via
-            // `zip_eq` below — this branch is reachable from untrusted proto
-            // bytes.
+            // `zip_eq` below, since this branch is reachable from untrusted
+            // proto bytes.
             if proto.names.len() != proto.metadata.len() {
                 return Err(TryFromProtoError::InvalidFieldError(format!(
                     "ProtoRelationDesc: names ({}) and metadata ({}) length mismatch",

src/repr/src/row/encode.rs

@@ -2126,7 +2126,9 @@ impl RowPacker<'_> {
                     // Map keys must be unique and strictly ascending; iterating a
                     // map that violates this trips a debug_assert. A crafted
                     // proto can, so reject it as a decode error here instead.
-                    if let Some(prev) = prev_key && e.key.as_str() <= prev {
+                    if let Some(prev) = prev_key
+                        && e.key.as_str() <= prev
+                    {
                         return Err(format!(
                             "dict keys must be unique and in ascending order, \
                              but {:?} came after {:?}",
@@ -2148,9 +2150,9 @@ impl RowPacker<'_> {
                 // represented as variants of ProtoDatumOther.
                 //
                 // `decPackedToNumber` (called via `Decimal::from_packed_bcd`)
-                // doesn't bounds-check its input and segfaults on empty bcd —
-                // reachable from untrusted proto bytes, so we reject before
-                // descending into the FFI.
+                // doesn't bounds-check its input and segfaults on empty bcd.
+                // That is reachable from untrusted proto bytes, so we reject
+                // before descending into the FFI.
                 if x.bcd.is_empty() {
                     return Err("ProtoNumeric.bcd is empty".to_string());
                 }
@@ -2170,9 +2172,10 @@ impl RowPacker<'_> {
                             upper,
                         } = &**inner;
 
-                        // Range bounds must not be `Datum::Null` — `push_range_with`
-                        // panics on that invariant. Reject untrusted proto bytes
-                        // that would push a `Null` bound before calling it.
+                        // Range bounds must not be `Datum::Null`, because
+                        // `push_range_with` panics on that invariant. Reject
+                        // untrusted proto bytes that would push a `Null` bound
+                        // before calling it.
                         let is_null_proto = |d: &ProtoDatum| {
                             matches!(
                                 d.datum_type,
@@ -2282,7 +2285,7 @@ mod tests {
     #[mz_ore::test]
     fn proto_row_invalid_range_is_error() {
         // A ProtoRow with a range whose bounds have inconsistent datum kinds
-        // (or a null/extra bound) must decode to an error, not panic — the range
+        // (or a null/extra bound) must decode to an error, not panic. The range
         // packer used to `assert!` these invariants. Regression for the
         // row_proto_roundtrip cargo-fuzz finding.
         use prost::Message;
@@ -2301,7 +2304,7 @@ mod tests {
     #[mz_ore::test]
     fn proto_row_unordered_dict_keys_is_error() {
         // A ProtoRow with a dict whose keys are duplicated or not in ascending
-        // order must decode to an error, not panic — iterating such a map trips
+        // order must decode to an error, not panic. Iterating such a map trips
         // a debug_assert. Regression for the row_proto_roundtrip cargo-fuzz
         // finding.
         use prost::Message;

src/repr/src/strconv.rs

@@ -733,9 +733,9 @@ where
 pub fn parse_uuid(s: &str) -> Result<Uuid, ParseError> {
     let trimmed = s.trim();
     // The `uuid` crate panics while constructing a parse error for some short,
-    // brace-wrapped inputs (e.g. `{}`, `{\0}`) — it mis-slices the input. Reject
-    // anything that can't be a UUID before handing it to the crate: the shortest
-    // valid form is 32 hex digits and every form is ASCII.
+    // brace-wrapped inputs (e.g. `{}`, `{\0}`) because it mis-slices the input.
+    // Reject anything that can't be a UUID before handing it to the crate. The
+    // shortest valid form is 32 hex digits and every form is ASCII.
     if trimmed.len() < 32 || !trimmed.is_ascii() {
         return Err(ParseError::invalid_input_syntax("uuid", s));
     }

src/storage-types/src/sources/casts.rs

@@ -820,8 +820,8 @@ mod tests {
         fn error_uuid() {
             // `parse_uuid` rejects anything too short to be a UUID before the
             // `uuid` crate runs (the crate panics building an error for some
-            // short inputs), so the error carries no crate-specific detail —
-            // matching Postgres, whose message for `'bad'::uuid` has none.
+            // short inputs), so the error carries no crate-specific detail.
+            // This matches Postgres, whose message for `'bad'::uuid` has none.
             assert_eq!(
                 eval_cast_err(CastFunc::CastStringToUuid, "bad"),
                 parse_err("uuid", "bad"),