Include the underlying reason in invalid --changed-before/-within date errors

CHANGELOG.md

@@ -13,6 +13,7 @@
 - Fire the "search pattern contains a path separator" diagnostic for any pattern containing `/`, not just patterns that happen to name an existing directory. Preserves the legacy Windows behaviour that also flags native `\` separators when the pattern resolves to a real directory. See #1873.
 - Also fire the "search pattern contains a path separator" diagnostic for `--and` patterns, not only the primary positional pattern. `--and` patterns are matched against the file name just like the primary pattern, so a path separator in them silently returned zero results. See #1873.
 - Fix bug where passing "-" as a directory argument didn't actually search that directory, see #849 (@Sean-Kenneth-Doherty).
+- Include the underlying parser error in the "not a valid date or duration" message for `--changed-before`/`--changed-within`, e.g. to explain that a given day is out of range for a calendar date, see #2053.
 
 # 10.4.2
 

src/filter/time.rs

@@ -1,3 +1,4 @@
+use anyhow::Result;
 use jiff::{Span, Timestamp, Zoned, civil::DateTime, tz::TimeZone};
 
 use std::time::{Duration, SystemTime, UNIX_EPOCH};
@@ -26,31 +27,46 @@ fn now() -> Zoned {
 }
 
 impl TimeFilter {
-    fn from_str(s: &str) -> Option<SystemTime> {
+    /// Parses a duration/timestamp/date/`@`-prefixed unix-timestamp string.
+    ///
+    /// On failure, returns the underlying parse error from the `DateTime`
+    /// parser (the calendar-date format, and the most common source of
+    /// confusing failures, e.g. `2025-11-31` which is not a valid calendar
+    /// date) instead of silently discarding it, so callers can tell the
+    /// user *why* their input was rejected.
+    fn from_str(s: &str) -> Result<SystemTime> {
         if let Ok(span) = s.parse::<Span>() {
-            let datetime = now().checked_sub(span).ok()?;
-            Some(datetime.into())
-        } else if let Ok(timestamp) = s.parse::<Timestamp>() {
-            Some(timestamp.into())
-        } else if let Ok(datetime) = s.parse::<DateTime>() {
-            Some(
-                TimeZone::system()
-                    .to_ambiguous_zoned(datetime)
-                    .later()
-                    .ok()?
-                    .into(),
-            )
-        } else {
-            let timestamp_secs: u64 = s.strip_prefix('@')?.parse().ok()?;
-            Some(UNIX_EPOCH + Duration::from_secs(timestamp_secs))
+            let datetime = now().checked_sub(span)?;
+            return Ok(datetime.into());
+        }
+        if let Ok(timestamp) = s.parse::<Timestamp>() {
+            return Ok(timestamp.into());
+        }
+        match s.parse::<DateTime>() {
+            Ok(datetime) => Ok(TimeZone::system()
+                .to_ambiguous_zoned(datetime)
+                .later()?
+                .into()),
+            Err(datetime_err) => {
+                if let Some(timestamp_secs) = s.strip_prefix('@')
+                    && let Ok(timestamp_secs) = timestamp_secs.parse()
+                {
+                    return Ok(UNIX_EPOCH + Duration::from_secs(timestamp_secs));
+                }
+                // None of the supported formats matched. The `DateTime`
+                // parser gives the most useful reason for calendar-date-
+                // shaped input (the common case for this kind of mistake),
+                // so surface that instead of a generic message.
+                Err(datetime_err.into())
+            }
         }
     }
 
-    pub fn before(s: &str) -> Option<TimeFilter> {
+    pub fn before(s: &str) -> Result<TimeFilter> {
         TimeFilter::from_str(s).map(TimeFilter::Before)
     }
 
-    pub fn after(s: &str) -> Option<TimeFilter> {
+    pub fn after(s: &str) -> Result<TimeFilter> {
         TimeFilter::from_str(s).map(TimeFilter::After)
     }
 
@@ -177,7 +193,7 @@ mod tests {
         let t1m_ago = ref_time - Duration::from_secs(60);
         let t1s_later = ref_time + Duration::from_secs(1);
         // Timestamp only supported via '@' prefix
-        assert!(TimeFilter::before(&ref_timestamp.to_string()).is_none());
+        assert!(TimeFilter::before(&ref_timestamp.to_string()).is_err());
         assert!(
             TimeFilter::before(&format!("@{ref_timestamp}"))
                 .unwrap()
@@ -199,4 +215,86 @@ mod tests {
                 .applies_to(&t1s_later)
         );
     }
+
+    /// Length of the longest contiguous substring shared by `a` and `b`.
+    ///
+    /// Used to check that two error messages share substantial content
+    /// without hardcoding what that content actually says.
+    fn longest_common_substring_len(a: &str, b: &str) -> usize {
+        let a: Vec<char> = a.chars().collect();
+        let b: Vec<char> = b.chars().collect();
+        let mut prev = vec![0usize; b.len() + 1];
+        let mut best = 0;
+        for i in 1..=a.len() {
+            let mut cur = vec![0usize; b.len() + 1];
+            for j in 1..=b.len() {
+                if a[i - 1] == b[j - 1] {
+                    cur[j] = prev[j - 1] + 1;
+                    best = best.max(cur[j]);
+                }
+            }
+            prev = cur;
+        }
+        best
+    }
+
+    #[test]
+    fn invalid_calendar_date_error_includes_inner_reason() {
+        // The error must surface *why* parsing failed, not merely echo the
+        // raw input back under a generic wrapper. We deliberately don't
+        // assert on jiff's exact wording (e.g. that it mentions "day"),
+        // since that would break on an unrelated jiff version bump. See
+        // https://github.com/sharkdp/fd/issues/2053
+        //
+        // A naive check that two different invalid inputs produce two
+        // different messages is gameable: a regression that dropped the
+        // real parse reason but still echoed the input (e.g.
+        // `format!("could not parse '{s}' as a date")`) would still make
+        // the two messages differ, purely because the inputs differ, while
+        // never actually surfacing the reason.
+        //
+        // Instead, compare error text for inputs that are invalid for the
+        // *same* underlying reason against error text for an input that's
+        // invalid for a *different* reason:
+        // - "2025-11-31" and "2019-06-31" both fail because day 31 doesn't
+        //   exist in a 30-day month (November / June), despite the raw
+        //   strings barely overlapping (different year and month).
+        // - "2025-13-01" fails for an unrelated reason (month 13 doesn't
+        //   exist).
+        // A message that actually carries the reason will make the first
+        // pair share a large chunk of text that the third message doesn't
+        // share. A message that's just the input echoed into a fixed
+        // template would make all three overlap by roughly the same
+        // (small) amount, since the only shared content would be the fixed
+        // wrapper text.
+        for filter in [TimeFilter::before, TimeFilter::after] {
+            let same_reason_a = filter("2025-11-31").unwrap_err().to_string();
+            let same_reason_b = filter("2019-06-31").unwrap_err().to_string();
+            let different_reason = filter("2025-13-01").unwrap_err().to_string();
+
+            assert!(!same_reason_a.is_empty());
+            assert_ne!(
+                same_reason_a, different_reason,
+                "distinct invalid inputs should surface distinct underlying reasons, not a shared generic message"
+            );
+
+            let same_reason_overlap = longest_common_substring_len(&same_reason_a, &same_reason_b);
+            let different_reason_overlap =
+                longest_common_substring_len(&same_reason_a, &different_reason);
+            assert!(
+                same_reason_overlap >= 20,
+                "two dates invalid for the same reason should share substantial error text \
+                 (got only {same_reason_overlap} shared characters between {same_reason_a:?} \
+                 and {same_reason_b:?})"
+            );
+            assert!(
+                same_reason_overlap > different_reason_overlap + 10,
+                "shared text between same-reason errors ({same_reason_overlap} chars) should \
+                 clearly exceed shared text between different-reason errors \
+                 ({different_reason_overlap} chars): {same_reason_a:?} vs {same_reason_b:?} vs \
+                 {different_reason:?}; a smaller gap suggests the message may just be echoing \
+                 the input rather than surfacing the actual reason"
+            );
+        }
+    }
 }

src/main.rs

@@ -496,23 +496,27 @@ fn determine_ls_command(colored_output: bool) -> Result<Vec<&'static str>> {
 fn extract_time_constraints(opts: &Opts) -> Result<Vec<TimeFilter>> {
     let mut time_constraints: Vec<TimeFilter> = Vec::new();
     if let Some(ref t) = opts.changed_within {
-        if let Some(f) = TimeFilter::after(t) {
-            time_constraints.push(f);
-        } else {
-            return Err(anyhow!(
-                "'{}' is not a valid date or duration. See 'fd --help'.",
-                t
-            ));
+        match TimeFilter::after(t) {
+            Ok(f) => time_constraints.push(f),
+            Err(e) => {
+                return Err(anyhow!(
+                    "'{}' is not a valid date or duration: {}. See 'fd --help'.",
+                    t,
+                    e
+                ));
+            }
         }
     }
     if let Some(ref t) = opts.changed_before {
-        if let Some(f) = TimeFilter::before(t) {
-            time_constraints.push(f);
-        } else {
-            return Err(anyhow!(
-                "'{}' is not a valid date or duration. See 'fd --help'.",
-                t
-            ));
+        match TimeFilter::before(t) {
+            Ok(f) => time_constraints.push(f),
+            Err(e) => {
+                return Err(anyhow!(
+                    "'{}' is not a valid date or duration: {}. See 'fd --help'.",
+                    t,
+                    e
+                ));
+            }
         }
     }
     Ok(time_constraints)