From cbda30f9510c284283f02c24aa4c1333711a1fe8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nicol=C3=A1s=20Hatcher?= Date: Wed, 1 Jan 2025 11:39:35 +0100 Subject: [PATCH] FIX: Use 1 as the first serial number corresponding to 1899-12-31 --- base/src/constants.rs | 7 +- base/src/formatter/dates.rs | 22 ++++- base/src/formatter/format.rs | 19 ++-- base/src/functions/date_and_time.rs | 134 +++++++++++++--------------- base/src/functions/financial.rs | 66 ++++++++------ base/src/test/test_date_and_time.rs | 9 +- 6 files changed, 131 insertions(+), 126 deletions(-) diff --git a/base/src/constants.rs b/base/src/constants.rs index 3ecc065..047b4b0 100644 --- a/base/src/constants.rs +++ b/base/src/constants.rs @@ -17,11 +17,8 @@ pub(crate) const LAST_ROW: i32 = 1_048_576; // The 2 days offset is because of Excel 1900 bug pub(crate) const EXCEL_DATE_BASE: i32 = 693_594; -// Excel can handle dates until the year 0000-01-01 -// However, it uses a different numbering scheme for dates -// that are before 1900-01-01. -// So for now we will simply not support dates before 1900-01-01. -pub(crate) const MINIMUM_DATE_SERIAL_NUMBER: i32 = 2; +// We do not support dates before 1899-12-31. +pub(crate) const MINIMUM_DATE_SERIAL_NUMBER: i32 = 1; // Excel can handle dates until the year 9999-12-31 // 2958465 is the number of days from 1900-01-01 to 9999-12-31 diff --git a/base/src/formatter/dates.rs b/base/src/formatter/dates.rs index eb72ad2..0959f06 100644 --- a/base/src/formatter/dates.rs +++ b/base/src/formatter/dates.rs @@ -18,10 +18,22 @@ fn is_date_within_range(date: NaiveDate) -> bool { && convert_to_serial_number(date) <= MAXIMUM_DATE_SERIAL_NUMBER } -pub fn from_excel_date(days: i64) -> NaiveDate { +pub fn from_excel_date(days: i64) -> Result { + if days < MINIMUM_DATE_SERIAL_NUMBER as i64 { + return Err(format!( + "Excel date must be greater than {}", + MINIMUM_DATE_SERIAL_NUMBER + )); + }; + if days > MAXIMUM_DATE_SERIAL_NUMBER as i64 { + return Err(format!( + "Excel date must be less than {}", + MAXIMUM_DATE_SERIAL_NUMBER + )); + }; #[allow(clippy::expect_used)] let dt = NaiveDate::from_ymd_opt(1900, 1, 1).expect("problem with chrono::NaiveDate"); - dt + Duration::days(days - 2) + Ok(dt + Duration::days(days - 2)) } pub fn date_to_serial_number(day: u32, month: u32, year: i32) -> Result { @@ -40,6 +52,10 @@ pub fn permissive_date_to_serial_number(day: i32, month: i32, year: i32) -> Resu // This function applies that same logic to dates. And does it in the most compatible way as // possible. + // Special case for the minimum date + if year == 1899 && month == 12 && day == 31 { + return Ok(MINIMUM_DATE_SERIAL_NUMBER); + } let Some(mut date) = NaiveDate::from_ymd_opt(year, 1, 1) else { return Err("Out of range parameters for date".to_string()); }; @@ -135,7 +151,7 @@ mod tests { Ok(MAXIMUM_DATE_SERIAL_NUMBER), ); assert_eq!( - permissive_date_to_serial_number(1, 1, 1900), + permissive_date_to_serial_number(31, 12, 1899), Ok(MINIMUM_DATE_SERIAL_NUMBER), ); } diff --git a/base/src/formatter/format.rs b/base/src/formatter/format.rs index 9076d33..6cbf495 100644 --- a/base/src/formatter/format.rs +++ b/base/src/formatter/format.rs @@ -154,15 +154,16 @@ pub fn format_number(value_original: f64, format: &str, locale: &Locale) -> Form ParsePart::Date(p) => { let tokens = &p.tokens; let mut text = "".to_string(); - if !(1.0..=2_958_465.0).contains(&value) { - // 2_958_465 is 31 December 9999 - return Formatted { - text: "#VALUE!".to_owned(), - color: None, - error: Some("Date negative or too long".to_owned()), - }; - } - let date = from_excel_date(value as i64); + let date = match from_excel_date(value as i64) { + Ok(d) => d, + Err(e) => { + return Formatted { + text: "#VALUE!".to_owned(), + color: None, + error: Some(e), + } + } + }; for token in tokens { match token { TextToken::Literal(c) => { diff --git a/base/src/functions/date_and_time.rs b/base/src/functions/date_and_time.rs index d93d938..8134b21 100644 --- a/base/src/functions/date_and_time.rs +++ b/base/src/functions/date_and_time.rs @@ -4,6 +4,7 @@ use chrono::Months; use chrono::Timelike; use crate::constants::MAXIMUM_DATE_SERIAL_NUMBER; +use crate::constants::MINIMUM_DATE_SERIAL_NUMBER; use crate::expressions::types::CellReferenceIndex; use crate::formatter::dates::date_to_serial_number; use crate::formatter::dates::permissive_date_to_serial_number; @@ -20,27 +21,19 @@ impl Model { return CalcResult::new_args_number_error(cell); } let serial_number = match self.get_number(&args[0], cell) { - Ok(c) => { - let t = c.floor() as i64; - if t < 0 { - return CalcResult::Error { - error: Error::NUM, - origin: cell, - message: "Function DAY parameter 1 value is negative. It should be positive or zero.".to_string(), - }; - } - t - } + Ok(c) => c.floor() as i64, Err(s) => return s, }; - if serial_number > MAXIMUM_DATE_SERIAL_NUMBER as i64 { - return CalcResult::Error { - error: Error::NUM, - origin: cell, - message: "Function DAY parameter 1 value is too large.".to_string(), - }; - } - let date = from_excel_date(serial_number); + let date = match from_excel_date(serial_number) { + Ok(date) => date, + Err(_) => { + return CalcResult::Error { + error: Error::NUM, + origin: cell, + message: "Out of range parameters for date".to_string(), + } + } + }; let day = date.day() as f64; CalcResult::Number(day) } @@ -51,27 +44,19 @@ impl Model { return CalcResult::new_args_number_error(cell); } let serial_number = match self.get_number(&args[0], cell) { - Ok(c) => { - let t = c.floor() as i64; - if t < 0 { - return CalcResult::Error { - error: Error::NUM, - origin: cell, - message: "Function MONTH parameter 1 value is negative. It should be positive or zero.".to_string(), - }; - } - t - } + Ok(c) => c.floor() as i64, Err(s) => return s, }; - if serial_number > MAXIMUM_DATE_SERIAL_NUMBER as i64 { - return CalcResult::Error { - error: Error::NUM, - origin: cell, - message: "Function DAY parameter 1 value is too large.".to_string(), - }; - } - let date = from_excel_date(serial_number); + let date = match from_excel_date(serial_number) { + Ok(date) => date, + Err(_) => { + return CalcResult::Error { + error: Error::NUM, + origin: cell, + message: "Out of range parameters for date".to_string(), + } + } + }; let month = date.month() as f64; CalcResult::Number(month) } @@ -95,6 +80,16 @@ impl Model { } Err(s) => return s, }; + let date = match from_excel_date(serial_number) { + Ok(date) => date, + Err(_) => { + return CalcResult::Error { + error: Error::NUM, + origin: cell, + message: "Out of range parameters for date".to_string(), + } + } + }; if serial_number > MAXIMUM_DATE_SERIAL_NUMBER as i64 { return CalcResult::Error { error: Error::NUM, @@ -114,9 +109,9 @@ impl Model { let months_abs = months.unsigned_abs(); let native_date = if months > 0 { - from_excel_date(serial_number) + Months::new(months_abs) + date + Months::new(months_abs) } else { - from_excel_date(serial_number) - Months::new(months_abs) + date - Months::new(months_abs) }; // Instead of calculating the end of month we compute the first day of the following month @@ -187,27 +182,19 @@ impl Model { return CalcResult::new_args_number_error(cell); } let serial_number = match self.get_number(&args[0], cell) { - Ok(c) => { - let t = c.floor() as i64; - if t < 0 { - return CalcResult::Error { - error: Error::NUM, - origin: cell, - message: "Function YEAR parameter 1 value is negative. It should be positive or zero.".to_string(), - }; - } - t - } + Ok(c) => c.floor() as i64, Err(s) => return s, }; - if serial_number > MAXIMUM_DATE_SERIAL_NUMBER as i64 { - return CalcResult::Error { - error: Error::NUM, - origin: cell, - message: "Function DAY parameter 1 value is too large.".to_string(), - }; - } - let date = from_excel_date(serial_number); + let date = match from_excel_date(serial_number) { + Ok(date) => date, + Err(_) => { + return CalcResult::Error { + error: Error::NUM, + origin: cell, + message: "Out of range parameters for date".to_string(), + } + } + }; let year = date.year() as f64; CalcResult::Number(year) } @@ -219,20 +206,19 @@ impl Model { return CalcResult::new_args_number_error(cell); } let serial_number = match self.get_number(&args[0], cell) { - Ok(c) => { - let t = c.floor() as i64; - if t < 0 { - return CalcResult::Error { - error: Error::NUM, - origin: cell, - message: "Parameter 1 value is negative. It should be positive or zero." - .to_string(), - }; - } - t - } + Ok(c) => c.floor() as i64, Err(s) => return s, }; + let date = match from_excel_date(serial_number) { + Ok(date) => date, + Err(_) => { + return CalcResult::Error { + error: Error::NUM, + origin: cell, + message: "Out of range parameters for date".to_string(), + } + } + }; let months = match self.get_number(&args[1], cell) { Ok(c) => { @@ -245,13 +231,13 @@ impl Model { let months_abs = months.unsigned_abs(); let native_date = if months > 0 { - from_excel_date(serial_number) + Months::new(months_abs) + date + Months::new(months_abs) } else { - from_excel_date(serial_number) - Months::new(months_abs) + date - Months::new(months_abs) }; let serial_number = native_date.num_days_from_ce() - EXCEL_DATE_BASE; - if serial_number < 0 { + if serial_number < MINIMUM_DATE_SERIAL_NUMBER { return CalcResult::Error { error: Error::NUM, origin: cell, diff --git a/base/src/functions/financial.rs b/base/src/functions/financial.rs index 0c84076..023212d 100644 --- a/base/src/functions/financial.rs +++ b/base/src/functions/financial.rs @@ -2,7 +2,7 @@ use chrono::Datelike; use crate::{ calc_result::CalcResult, - constants::{LAST_COLUMN, LAST_ROW}, + constants::{LAST_COLUMN, LAST_ROW, MAXIMUM_DATE_SERIAL_NUMBER, MINIMUM_DATE_SERIAL_NUMBER}, expressions::{parser::Node, token::Error, types::CellReferenceIndex}, formatter::dates::from_excel_date, model::Model, @@ -13,37 +13,38 @@ use super::financial_util::{compute_irr, compute_npv, compute_rate, compute_xirr // See: // https://github.com/apache/openoffice/blob/c014b5f2b55cff8d4b0c952d5c16d62ecde09ca1/main/scaddins/source/analysis/financial.cxx -// FIXME: Is this enough? -fn is_valid_date(date: f64) -> bool { - date > 0.0 -} - -fn is_less_than_one_year(start_date: i64, end_date: i64) -> bool { +fn is_less_than_one_year(start_date: i64, end_date: i64) -> Result { + let end = match from_excel_date(end_date) { + Ok(s) => s, + Err(s) => return Err(s), + }; + let start = match from_excel_date(start_date) { + Ok(s) => s, + Err(s) => return Err(s), + }; if end_date - start_date < 365 { - return true; + return Ok(true); } - let end = from_excel_date(end_date); - let start = from_excel_date(start_date); let end_year = end.year(); let start_year = start.year(); if end_year == start_year { - return true; + return Ok(true); } if end_year != start_year + 1 { - return false; + return Ok(false); } let start_month = start.month(); let end_month = end.month(); if end_month < start_month { - return true; + return Ok(true); } if end_month > start_month { - return false; + return Ok(false); } // we are one year later same month let start_day = start.day(); let end_day = end.day(); - end_day <= start_day + Ok(end_day <= start_day) } fn compute_payment( @@ -923,7 +924,9 @@ impl Model { } let first_date = dates[0]; for date in &dates { - if !is_valid_date(*date) { + if *date < MINIMUM_DATE_SERIAL_NUMBER as f64 + || *date > MAXIMUM_DATE_SERIAL_NUMBER as f64 + { // Excel docs claim that if any number in dates is not a valid date, // XNPV returns the #VALUE! error value, but it seems to return #VALUE! return CalcResult::new_error( @@ -989,7 +992,9 @@ impl Model { } let first_date = dates[0]; for date in &dates { - if !is_valid_date(*date) { + if *date < MINIMUM_DATE_SERIAL_NUMBER as f64 + || *date > MAXIMUM_DATE_SERIAL_NUMBER as f64 + { return CalcResult::new_error( Error::NUM, cell, @@ -1373,9 +1378,10 @@ impl Model { Ok(f) => f, Err(s) => return s, }; - if !is_valid_date(settlement) || !is_valid_date(maturity) { - return CalcResult::new_error(Error::NUM, cell, "Invalid date".to_string()); - } + let less_than_one_year = match is_less_than_one_year(settlement as i64, maturity as i64) { + Ok(f) => f, + Err(_) => return CalcResult::new_error(Error::NUM, cell, "Invalid date".to_string()), + }; if settlement > maturity { return CalcResult::new_error( Error::NUM, @@ -1383,7 +1389,7 @@ impl Model { "settlement should be <= maturity".to_string(), ); } - if !is_less_than_one_year(settlement as i64, maturity as i64) { + if !less_than_one_year { return CalcResult::new_error( Error::NUM, cell, @@ -1437,9 +1443,10 @@ impl Model { Ok(f) => f, Err(s) => return s, }; - if !is_valid_date(settlement) || !is_valid_date(maturity) { - return CalcResult::new_error(Error::NUM, cell, "Invalid date".to_string()); - } + let less_than_one_year = match is_less_than_one_year(settlement as i64, maturity as i64) { + Ok(f) => f, + Err(_) => return CalcResult::new_error(Error::NUM, cell, "Invalid date".to_string()), + }; if settlement > maturity { return CalcResult::new_error( Error::NUM, @@ -1447,7 +1454,7 @@ impl Model { "settlement should be <= maturity".to_string(), ); } - if !is_less_than_one_year(settlement as i64, maturity as i64) { + if !less_than_one_year { return CalcResult::new_error( Error::NUM, cell, @@ -1487,9 +1494,10 @@ impl Model { Ok(f) => f, Err(s) => return s, }; - if !is_valid_date(settlement) || !is_valid_date(maturity) { - return CalcResult::new_error(Error::NUM, cell, "Invalid date".to_string()); - } + let less_than_one_year = match is_less_than_one_year(settlement as i64, maturity as i64) { + Ok(f) => f, + Err(_) => return CalcResult::new_error(Error::NUM, cell, "Invalid date".to_string()), + }; if settlement > maturity { return CalcResult::new_error( Error::NUM, @@ -1497,7 +1505,7 @@ impl Model { "settlement should be <= maturity".to_string(), ); } - if !is_less_than_one_year(settlement as i64, maturity as i64) { + if !less_than_one_year { return CalcResult::new_error( Error::NUM, cell, diff --git a/base/src/test/test_date_and_time.rs b/base/src/test/test_date_and_time.rs index 7e6ba03..7216700 100644 --- a/base/src/test/test_date_and_time.rs +++ b/base/src/test/test_date_and_time.rs @@ -132,8 +132,7 @@ fn test_day_small_serial() { model.evaluate(); assert_eq!(model._get_text("A1"), *"#NUM!"); - // This agrees with Google Docs and disagrees with Excel - assert_eq!(model._get_text("A2"), *"30"); + assert_eq!(model._get_text("A2"), *"#NUM!"); // Excel thinks is Feb 29, 1900 assert_eq!(model._get_text("A3"), *"28"); @@ -153,8 +152,7 @@ fn test_month_small_serial() { model.evaluate(); assert_eq!(model._get_text("A1"), *"#NUM!"); - // This agrees with Google Docs and disagrees with Excel - assert_eq!(model._get_text("A2"), *"12"); + assert_eq!(model._get_text("A2"), *"#NUM!"); // We agree with Excel here (We are both in Feb) assert_eq!(model._get_text("A3"), *"2"); @@ -174,8 +172,7 @@ fn test_year_small_serial() { model.evaluate(); assert_eq!(model._get_text("A1"), *"#NUM!"); - // This agrees with Google Docs and disagrees with Excel - assert_eq!(model._get_text("A2"), *"1899"); + assert_eq!(model._get_text("A2"), *"#NUM!"); assert_eq!(model._get_text("A3"), *"1900");