sparkline: add tests for error handling

src/chart.rs

@@ -7562,6 +7562,17 @@ impl ChartRange {
         )
     }
 
+    // Convert the row/col range into a range error string.
+    pub(crate) fn error_range(&self) -> String {
+        utility::chart_error_range(
+            &self.sheet_name,
+            self.first_row,
+            self.first_col,
+            self.last_row,
+            self.last_col,
+        )
+    }
+
     // Unique key to identify/find the range of values to build the cache.
     pub(crate) fn key(&self) -> (String, RowNum, ColNum, RowNum, ColNum) {
         (
@@ -7578,14 +7589,22 @@ impl ChartRange {
         !self.sheet_name.is_empty()
     }
 
-    // Get the number of data points in the range.
+    // Get the number of X or Y data points in the range.
     pub(crate) fn number_of_points(&self) -> usize {
         let row_range = (self.last_row - self.first_row + 1) as usize;
         let col_range = (self.last_col - self.first_col + 1) as usize;
 
         std::cmp::max(row_range, col_range)
     }
 
+    // Get the number of X and Y data points in the range.
+    pub(crate) fn number_of_range_points(&self) -> (usize, usize) {
+        let row_range = (self.last_row - self.first_row + 1) as usize;
+        let col_range = (self.last_col - self.first_col + 1) as usize;
+
+        (row_range, col_range)
+    }
+
     // Set the start point in a 2D range. This is used to start incremental
     // ranges, see below.
     pub(crate) fn set_baseline(&mut self, row_order: bool) {
@@ -7610,7 +7629,7 @@ impl ChartRange {
 
     // Check that the row/column values in the range are valid.
     pub(crate) fn validate(&self) -> Result<(), XlsxError> {
-        let range = self.formula_abs();
+        let range = self.error_range();
 
         let error_message = format!("Sheet name error for range: '{range}'");
         utility::validate_sheetname(&self.sheet_name, &error_message)?;
@@ -7642,6 +7661,11 @@ impl ChartRange {
         Ok(())
     }
 
+    // Check that the range is 1D.
+    pub(crate) fn is_1d(&self) -> bool {
+        self.last_row - self.first_row == 0 || self.last_col - self.first_col == 0
+    }
+
     /// Add data to the `ChartRange` cache.
     ///
     /// This method is only used to populate the chart data caches in test code.

src/error.rs

@@ -87,6 +87,10 @@ pub enum XlsxError {
     /// chart is configured incorrectly.
     ChartError(String),
 
+    /// A general error that is raised when a sparkline parameter is incorrect
+    /// or a sparkline is configured incorrectly.
+    SparklineError(String),
+
     /// A general error when one of the parameters supplied to a
     /// [`ExcelDateTime`](crate::ExcelDateTime) method is outside Excel's
     /// allowable ranges.
@@ -260,6 +264,10 @@ impl fmt::Display for XlsxError {
                 write!(f, "Chart error: '{error}'.")
             }
 
+            XlsxError::SparklineError(error) => {
+                write!(f, "Sparkline error: '{error}'.")
+            }
+
             XlsxError::DateTimeRangeError(error) => {
                 write!(f, "Date range error: '{error}'")
             }

src/sparkline/tests.rs

@@ -17,87 +17,40 @@ mod sparkline_tests {
     use pretty_assertions::assert_eq;
 
     #[test]
-    fn sparkline02_1() -> Result<(), XlsxError> {
+    fn sparkline_errors() {
         let mut worksheet = Worksheet::new();
-        worksheet.set_selected(true);
 
-        worksheet.write(0, 0, -2)?;
-        worksheet.write(0, 1, 2)?;
-        worksheet.write(0, 2, 3)?;
-        worksheet.write(0, 3, -1)?;
-        worksheet.write(0, 4, 0)?;
-
-        let sparkline = Sparkline::new().set_range(("Sheet1", 0, 0, 0, 4));
+        let sparkline = Sparkline::new();
+        let result = worksheet.add_sparkline(0, 5, &sparkline);
+        assert!(matches!(result, Err(XlsxError::SparklineError(_))));
 
-        worksheet.add_sparkline(0, 5, &sparkline)?;
+        let sparkline = Sparkline::new().set_range(("Sheet1", 0, 0, 1, 4));
+        let result = worksheet.add_sparkline(0, 5, &sparkline);
+        assert!(matches!(result, Err(XlsxError::SparklineError(_))));
 
-        worksheet.assemble_xml_file();
+        let sparkline = Sparkline::new().set_range(("Sheet1", 9, 9, 0, 4));
+        let result = worksheet.add_sparkline(0, 5, &sparkline);
+        assert!(matches!(result, Err(XlsxError::ChartError(_))));
 
-        let got = worksheet.writer.read_to_str();
-        let got = xml_to_vec(got);
+        let sparkline = Sparkline::new();
+        let result = worksheet.add_sparkline_group(0, 5, 0, 5, &sparkline);
+        assert!(matches!(result, Err(XlsxError::SparklineError(_))));
 
-        let expected = xml_to_vec(
-            r#"
-            <?xml version="1.0" encoding="UTF-8" standalone="yes"?>
-            <worksheet xmlns="http://schemas.openxmlformats.org/spreadsheetml/2006/main" xmlns:r="http://schemas.openxmlformats.org/officeDocument/2006/relationships" xmlns:mc="http://schemas.openxmlformats.org/markup-compatibility/2006" xmlns:x14ac="http://schemas.microsoft.com/office/spreadsheetml/2009/9/ac" mc:Ignorable="x14ac">
-              <dimension ref="A1:E1"/>
-              <sheetViews>
-                <sheetView tabSelected="1" workbookViewId="0"/>
-              </sheetViews>
-              <sheetFormatPr defaultRowHeight="15" x14ac:dyDescent="0.25"/>
-              <sheetData>
-                <row r="1" spans="1:5" x14ac:dyDescent="0.25">
-                  <c r="A1">
-                    <v>-2</v>
-                  </c>
-                  <c r="B1">
-                    <v>2</v>
-                  </c>
-                  <c r="C1">
-                    <v>3</v>
-                  </c>
-                  <c r="D1">
-                    <v>-1</v>
-                  </c>
-                  <c r="E1">
-                    <v>0</v>
-                  </c>
-                </row>
-              </sheetData>
-              <pageMargins left="0.7" right="0.7" top="0.75" bottom="0.75" header="0.3" footer="0.3"/>
-              <extLst>
-                <ext xmlns:x14="http://schemas.microsoft.com/office/spreadsheetml/2009/9/main" uri="{05C60535-1F16-4fd2-B633-F4F36F0B64E0}">
-                  <x14:sparklineGroups xmlns:xm="http://schemas.microsoft.com/office/excel/2006/main">
-                    <x14:sparklineGroup displayEmptyCellsAs="gap">
-                      <x14:colorSeries theme="4" tint="-0.499984740745262"/>
-                      <x14:colorNegative theme="5"/>
-                      <x14:colorAxis rgb="FF000000"/>
-                      <x14:colorMarkers theme="4" tint="-0.499984740745262"/>
-                      <x14:colorFirst theme="4" tint="0.39997558519241921"/>
-                      <x14:colorLast theme="4" tint="0.39997558519241921"/>
-                      <x14:colorHigh theme="4"/>
-                      <x14:colorLow theme="4"/>
-                      <x14:sparklines>
-                        <x14:sparkline>
-                          <xm:f>Sheet1!A1:E1</xm:f>
-                          <xm:sqref>F1</xm:sqref>
-                        </x14:sparkline>
-                      </x14:sparklines>
-                    </x14:sparklineGroup>
-                  </x14:sparklineGroups>
-                </ext>
-              </extLst>
-            </worksheet>
-            "#,
-        );
+        let sparkline = Sparkline::new().set_range(("Sheet1", 0, 0, 0, 5));
+        let result = worksheet.add_sparkline_group(0, 5, 0, 5, &sparkline);
+        assert!(matches!(result, Err(XlsxError::SparklineError(_))));
 
-        assert_eq!(expected, got);
+        let sparkline = Sparkline::new().set_range(("Sheet1", 9, 9, 0, 5));
+        let result = worksheet.add_sparkline_group(0, 5, 0, 5, &sparkline);
+        assert!(matches!(result, Err(XlsxError::ChartError(_))));
 
-        Ok(())
+        let sparkline = Sparkline::new().set_range(("Sheet1", 0, 0, 4, 5));
+        let result = worksheet.add_sparkline_group(0, 5, 0, 5, &sparkline);
+        assert!(matches!(result, Err(XlsxError::SparklineError(_))));
     }
 
     #[test]
-    fn sparkline02_2() -> Result<(), XlsxError> {
+    fn sparkline02_1() -> Result<(), XlsxError> {
         let mut worksheet = Worksheet::new();
         worksheet.set_selected(true);
 
@@ -109,7 +62,7 @@ mod sparkline_tests {
 
         let sparkline = Sparkline::new().set_range(("Sheet1", 0, 0, 0, 4));
 
-        worksheet.add_sparkline_group(0, 5, 0, 5, &sparkline)?;
+        worksheet.add_sparkline(0, 5, &sparkline)?;
 
         worksheet.assemble_xml_file();
 

src/utility.rs

@@ -408,6 +408,26 @@ pub(crate) fn chart_range_abs(
     }
 }
 
+// Convert zero indexed row and col cell references to a range and tuple string
+// suitable for an error message.
+pub(crate) fn chart_error_range(
+    sheet_name: &str,
+    first_row: RowNum,
+    first_col: ColNum,
+    last_row: RowNum,
+    last_col: ColNum,
+) -> String {
+    let sheet_name = quote_sheetname(sheet_name);
+    let range1 = row_col_to_cell(first_row, first_col);
+    let range2 = row_col_to_cell(last_row, last_col);
+
+    if range1 == range2 {
+        format!("{sheet_name}!{range1}/({first_row}, {first_col})")
+    } else {
+        format!("{sheet_name}!{range1}:{range2}/({first_row}, {first_col}, {last_row}, {last_col})")
+    }
+}
+
 // Create a quoted version of a worksheet name. Excel single quotes worksheet
 // names that contain spaces and some other characters.
 pub(crate) fn quote_sheetname(sheetname: &str) -> String {

src/worksheet.rs

@@ -5394,9 +5394,20 @@ impl Worksheet {
     ///
     /// # Errors
     ///
+    /// * [`XlsxError::SparklineError`] - An error that is raised when there is
+    ///   an parameter error with the sparkline.
+    /// * [`XlsxError::ChartError`] - An error that is raised when there is an
+    ///   parameter error with the data range for the sparkline.
     /// * [`XlsxError::RowColumnLimitError`] - Row or column exceeds Excel's
     ///   worksheet limits.
-    /// * TODO - add sparkline errors.
+    /// * [`XlsxError::SheetnameCannotBeBlank`] - Worksheet name in chart range
+    ///   cannot be blank.
+    /// * [`XlsxError::SheetnameLengthExceeded`] - Worksheet name in chart range
+    ///   exceeds Excel's limit of 31 characters.
+    /// * [`XlsxError::SheetnameContainsInvalidCharacter`] - Worksheet name in
+    ///   chart range cannot contain invalid characters: `[ ] : * ? / \`
+    /// * [`XlsxError::SheetnameStartsOrEndsWithApostrophe`] - Worksheet name in
+    ///   chart range cannot start or end with an apostrophe.
     ///
     /// # Examples
     ///
@@ -5448,14 +5459,22 @@ impl Worksheet {
 
         // Check that the sparkline has a range.
         if !sparkline.data_range.has_data() {
-            return Err(XlsxError::ParameterError(
+            return Err(XlsxError::SparklineError(
                 "Sparkline data range not set".to_string(),
             ));
         }
 
         // Check that the sparkline range is valid.
         sparkline.data_range.validate()?;
 
+        // Check that the sparkline range is 1D.
+        if !sparkline.data_range.is_1d() {
+            let range = sparkline.data_range.error_range();
+            return Err(XlsxError::SparklineError(format!(
+                "Sparkline data range '{range}' must be a 1D range"
+            )));
+        }
+
         // Clone the sparkline and set a data range.
         let mut sparkline = sparkline.clone();
         sparkline.add_cell_range(row, col);
@@ -5500,15 +5519,27 @@ impl Worksheet {
     ///
     /// # Errors
     ///
+    /// # Errors
+    ///
+    /// * [`XlsxError::SparklineError`] - An error that is raised when there is
+    ///   an parameter error with the sparkline.
+    /// * [`XlsxError::ChartError`] - An error that is raised when there is an
+    ///   parameter error with the data range for the sparkline.
     /// * [`XlsxError::RowColumnLimitError`] - Row or column exceeds Excel's
     ///   worksheet limits.
-    /// * [`XlsxError::RowColumnOrderError`] - First row larger than the last
-    ///   row.
-    /// * TODO - add sparkline errors.
+    /// * [`XlsxError::SheetnameCannotBeBlank`] - Worksheet name in chart range
+    ///   cannot be blank.
+    /// * [`XlsxError::SheetnameLengthExceeded`] - Worksheet name in chart range
+    ///   exceeds Excel's limit of 31 characters.
+    /// * [`XlsxError::SheetnameContainsInvalidCharacter`] - Worksheet name in
+    ///   chart range cannot contain invalid characters: `[ ] : * ? / \`
+    /// * [`XlsxError::SheetnameStartsOrEndsWithApostrophe`] - Worksheet name in
+    ///   chart range cannot start or end with an apostrophe.
     ///
     /// # Examples
     ///
-    /// The following example demonstrates adding a sparkline group to a worksheet.
+    /// The following example demonstrates adding a sparkline group to a
+    /// worksheet.
     ///
     /// ```
     /// # // This code is available in examples/doc_worksheet_add_sparkline_group.rs
@@ -5545,7 +5576,8 @@ impl Worksheet {
     ///
     /// Output file:
     ///
-    /// <img src="https://rustxlsxwriter.github.io/images/worksheet_add_sparkline_group.png">
+    /// <img
+    /// src="https://rustxlsxwriter.github.io/images/worksheet_add_sparkline_group.png">
     ///
     pub fn add_sparkline_group(
         &mut self,
@@ -5567,23 +5599,38 @@ impl Worksheet {
             return Err(XlsxError::RowColumnOrderError);
         }
 
-        // Check that the cell location is a 1D/scalar range.
-        if last_row - first_row > 0 && last_col - first_col > 0 {
-            return Err(XlsxError::ParameterError(
-                format!("Sparkline group location `({first_row}, {first_col}, {last_row}, {last_col})` must be a 1D/scalar range"),
-            ));
-        }
-
         // Check that the sparkline has a range.
         if !sparkline.data_range.has_data() {
-            return Err(XlsxError::ParameterError(
+            return Err(XlsxError::SparklineError(
                 "Sparkline data range not set".to_string(),
             ));
         }
 
         // Check that the sparkline range is valid.
         sparkline.data_range.validate()?;
 
+        // Check that the sparkline range is 2D.
+        if sparkline.data_range.is_1d() {
+            let range = sparkline.data_range.error_range();
+            return Err(XlsxError::SparklineError(format!(
+                "Sparkline data range '{range}' must be a 2D range"
+            )));
+        }
+
+        // Check that the group data range matches 1 dimension of the sparkline
+        // data range.
+        let row_range = (last_row - first_row + 1) as usize;
+        let col_range = (last_col - first_col + 1) as usize;
+        let num_cells = std::cmp::max(row_range, col_range);
+        let (num_rows, num_cols) = sparkline.data_range.number_of_range_points();
+        if num_cells != num_rows && num_cells != num_cols {
+            let cell_range = format!("({first_row}, {first_col}, {last_row}, {last_col})");
+            let sparkline_range = sparkline.data_range.error_range();
+            return Err(XlsxError::SparklineError(format!(
+                "Sparkline group range '{cell_range}' doesn't match dimensions of data range '{sparkline_range}'"
+            )));
+        }
+
         // Clone the sparkline and set a data range.
         let mut sparkline = sparkline.clone();
         sparkline.add_group_range(first_row, first_col, last_row, last_col);