From 49ae2d89155d303d4e5825c7762eb8644374aca6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nicol=C3=A1s=20Hatcher?= Date: Sat, 16 Nov 2024 14:18:12 +0100 Subject: [PATCH] FIX: Forbid unwrap, expect and panic in the base code --- Makefile | 2 +- base/src/diffs.rs | 2 + base/src/expressions/lexer/test/test_util.rs | 2 + base/src/expressions/lexer/util.rs | 2 + base/src/expressions/parser/mod.rs | 30 +++++-- base/src/expressions/parser/test.rs | 2 + base/src/formatter/dates.rs | 1 + base/src/functions/financial.rs | 80 ++++++++++++++----- base/src/functions/mathematical.rs | 64 +++++++++------ base/src/functions/mod.rs | 1 + base/src/functions/statistical.rs | 47 +++++++---- base/src/functions/subtotal.rs | 39 +++++++-- base/src/functions/text.rs | 32 +++++--- base/src/functions/xlookup.rs | 32 +++++--- base/src/language/mod.rs | 1 + base/src/locale/mod.rs | 1 + base/src/model.rs | 5 ++ base/src/new_empty.rs | 1 + base/src/test/test_error_propagation.rs | 1 + base/src/test/test_metadata.rs | 2 + .../test/test_set_functions_error_handling.rs | 2 + base/src/test/user_model/test_diff_queue.rs | 2 + base/src/test/user_model/test_paste_csv.rs | 2 + base/src/user_model/common.rs | 14 +++- base/src/utils.rs | 3 + base/src/worksheet.rs | 4 +- bindings/python/Cargo.toml | 3 - bindings/python/src/lib.rs | 2 + bindings/wasm/src/lib.rs | 22 +++-- xlsx/examples/hello_calc.rs | 8 +- xlsx/src/bin/documentation.rs | 3 + xlsx/src/bin/test.rs | 3 + xlsx/src/bin/xlsx_2_icalc.rs | 3 + xlsx/src/compare.rs | 2 + xlsx/src/export/mod.rs | 2 + xlsx/src/export/workbook.rs | 1 + xlsx/src/export/worksheets.rs | 3 + xlsx/src/import/colors.rs | 2 + xlsx/src/import/shared_strings.rs | 1 + xlsx/src/import/tables.rs | 28 +++++-- xlsx/src/import/util.rs | 2 + xlsx/src/import/worksheets.rs | 7 +- xlsx/tests/test.rs | 3 + 43 files changed, 341 insertions(+), 128 deletions(-) diff --git a/Makefile b/Makefile index 93d00fc..be4c03d 100644 --- a/Makefile +++ b/Makefile @@ -1,7 +1,7 @@ .PHONY: lint lint: cargo fmt -- --check - cargo clippy --all-targets --all-features + cargo clippy --all-targets --all-features -- -W clippy::unwrap_used -W clippy::expect_used -W clippy::panic -D warnings cd webapp && npm install && npm run check .PHONY: format diff --git a/base/src/diffs.rs b/base/src/diffs.rs index dbdfdc7..e02c115 100644 --- a/base/src/diffs.rs +++ b/base/src/diffs.rs @@ -26,6 +26,7 @@ pub struct SetCellValue { } impl Model { + #[allow(clippy::expect_used)] pub(crate) fn shift_cell_formula( &mut self, sheet: u32, @@ -57,6 +58,7 @@ impl Model { } } + #[allow(clippy::expect_used)] pub fn forward_references( &mut self, source_area: &Area, diff --git a/base/src/expressions/lexer/test/test_util.rs b/base/src/expressions/lexer/test/test_util.rs index fff5985..51a0726 100644 --- a/base/src/expressions/lexer/test/test_util.rs +++ b/base/src/expressions/lexer/test/test_util.rs @@ -1,3 +1,5 @@ +#![allow(clippy::expect_used)] + use crate::expressions::{ lexer::util::get_tokens, token::{OpCompare, OpSum, TokenType}, diff --git a/base/src/expressions/lexer/util.rs b/base/src/expressions/lexer/util.rs index d1f4de6..9eb5913 100644 --- a/base/src/expressions/lexer/util.rs +++ b/base/src/expressions/lexer/util.rs @@ -52,7 +52,9 @@ pub fn get_tokens(formula: &str) -> Vec { let mut lexer = Lexer::new( formula, LexerMode::A1, + #[allow(clippy::expect_used)] get_locale("en").expect(""), + #[allow(clippy::expect_used)] get_language("en").expect(""), ); let mut start = lexer.get_position(); diff --git a/base/src/expressions/parser/mod.rs b/base/src/expressions/parser/mod.rs index afeec68..5f10755 100644 --- a/base/src/expressions/parser/mod.rs +++ b/base/src/expressions/parser/mod.rs @@ -63,7 +63,9 @@ pub(crate) fn parse_range(formula: &str) -> Result<(i32, i32, i32, i32), String> let mut lexer = lexer::Lexer::new( formula, lexer::LexerMode::A1, + #[allow(clippy::expect_used)] get_locale("en").expect(""), + #[allow(clippy::expect_used)] get_language("en").expect(""), ); if let TokenType::Range { @@ -202,7 +204,9 @@ impl Parser { let lexer = lexer::Lexer::new( "", lexer::LexerMode::A1, + #[allow(clippy::expect_used)] get_locale("en").expect(""), + #[allow(clippy::expect_used)] get_language("en").expect(""), ); Parser { @@ -675,14 +679,23 @@ impl Parser { } }; // table-name => table - let table = self.tables.get(&table_name).unwrap_or_else(|| { - panic!( - "Table not found: '{table_name}' at '{}!{}{}'", - context.sheet, - number_to_column(context.column).expect(""), - context.row - ) - }); + let table = match self.tables.get(&table_name) { + Some(t) => t, + None => { + let message = format!( + "Table not found: '{table_name}' at '{}!{}{}'", + context.sheet, + number_to_column(context.column) + .unwrap_or(format!("{}", context.column)), + context.row + ); + return Node::ParseErrorKind { + formula: self.lexer.get_formula(), + position: 0, + message, + }; + } + }; let table_sheet_index = match self.get_sheet_index_by_name(&table.sheet_name) { Some(i) => i, None => { @@ -701,6 +714,7 @@ impl Parser { }; // context must be with tables.reference + #[allow(clippy::expect_used)] let (column_start, mut row_start, column_end, mut row_end) = parse_range(&table.reference).expect("Failed parsing range"); diff --git a/base/src/expressions/parser/test.rs b/base/src/expressions/parser/test.rs index 3d8369a..aff3df4 100644 --- a/base/src/expressions/parser/test.rs +++ b/base/src/expressions/parser/test.rs @@ -1,3 +1,5 @@ +#![allow(clippy::panic)] + use std::collections::HashMap; use crate::expressions::lexer::LexerMode; diff --git a/base/src/formatter/dates.rs b/base/src/formatter/dates.rs index af1a6a2..0715281 100644 --- a/base/src/formatter/dates.rs +++ b/base/src/formatter/dates.rs @@ -5,6 +5,7 @@ use chrono::NaiveDate; use crate::constants::EXCEL_DATE_BASE; pub fn from_excel_date(days: i64) -> NaiveDate { + #[allow(clippy::expect_used)] let dt = NaiveDate::from_ymd_opt(1900, 1, 1).expect("problem with chrono::NaiveDate"); dt + Duration::days(days - 2) } diff --git a/base/src/functions/financial.rs b/base/src/functions/financial.rs index 59fe2a8..797905a 100644 --- a/base/src/functions/financial.rs +++ b/base/src/functions/financial.rs @@ -220,7 +220,13 @@ impl Model { row2 = self .workbook .worksheet(left.sheet) - .expect("Sheet expected during evaluation.") + .map_err(|_| { + CalcResult::new_error( + Error::ERROR, + *cell, + format!("Invalid worksheet index: '{}'", left.sheet), + ) + })? .dimension() .max_row; } @@ -228,7 +234,13 @@ impl Model { column2 = self .workbook .worksheet(left.sheet) - .expect("Sheet expected during evaluation.") + .map_err(|_| { + CalcResult::new_error( + Error::ERROR, + *cell, + format!("Invalid worksheet index: '{}'", left.sheet), + ) + })? .dimension() .max_column; } @@ -283,7 +295,13 @@ impl Model { row2 = self .workbook .worksheet(left.sheet) - .expect("Sheet expected during evaluation.") + .map_err(|_| { + CalcResult::new_error( + Error::ERROR, + *cell, + format!("Invalid worksheet index: '{}'", left.sheet), + ) + })? .dimension() .max_row; } @@ -291,7 +309,13 @@ impl Model { column2 = self .workbook .worksheet(left.sheet) - .expect("Sheet expected during evaluation.") + .map_err(|_| { + CalcResult::new_error( + Error::ERROR, + *cell, + format!("Invalid worksheet index: '{}'", left.sheet), + ) + })? .dimension() .max_column; } @@ -359,7 +383,13 @@ impl Model { row2 = self .workbook .worksheet(left.sheet) - .expect("Sheet expected during evaluation.") + .map_err(|_| { + CalcResult::new_error( + Error::ERROR, + *cell, + format!("Invalid worksheet index: '{}'", left.sheet), + ) + })? .dimension() .max_row; } @@ -367,7 +397,13 @@ impl Model { column2 = self .workbook .worksheet(left.sheet) - .expect("Sheet expected during evaluation.") + .map_err(|_| { + CalcResult::new_error( + Error::ERROR, + *cell, + format!("Invalid worksheet index: '{}'", left.sheet), + ) + })? .dimension() .max_column; } @@ -862,20 +898,28 @@ impl Model { let column1 = left.column; let mut column2 = right.column; if row1 == 1 && row2 == LAST_ROW { - row2 = self - .workbook - .worksheet(left.sheet) - .expect("Sheet expected during evaluation.") - .dimension() - .max_row; + row2 = match self.workbook.worksheet(left.sheet) { + Ok(s) => s.dimension().max_row, + Err(_) => { + return CalcResult::new_error( + Error::ERROR, + cell, + format!("Invalid worksheet index: '{}'", left.sheet), + ); + } + }; } if column1 == 1 && column2 == LAST_COLUMN { - column2 = self - .workbook - .worksheet(left.sheet) - .expect("Sheet expected during evaluation.") - .dimension() - .max_column; + column2 = match self.workbook.worksheet(left.sheet) { + Ok(s) => s.dimension().max_column, + Err(_) => { + return CalcResult::new_error( + Error::ERROR, + cell, + format!("Invalid worksheet index: '{}'", left.sheet), + ); + } + }; } for row in row1..row2 + 1 { for column in column1..(column2 + 1) { diff --git a/base/src/functions/mathematical.rs b/base/src/functions/mathematical.rs index 936fd94..12402c2 100644 --- a/base/src/functions/mathematical.rs +++ b/base/src/functions/mathematical.rs @@ -128,20 +128,28 @@ impl Model { let column1 = left.column; let mut column2 = right.column; if row1 == 1 && row2 == LAST_ROW { - row2 = self - .workbook - .worksheet(left.sheet) - .expect("Sheet expected during evaluation.") - .dimension() - .max_row; + row2 = match self.workbook.worksheet(left.sheet) { + Ok(s) => s.dimension().max_row, + Err(_) => { + return CalcResult::new_error( + Error::ERROR, + cell, + format!("Invalid worksheet index: '{}'", left.sheet), + ); + } + }; } if column1 == 1 && column2 == LAST_COLUMN { - column2 = self - .workbook - .worksheet(left.sheet) - .expect("Sheet expected during evaluation.") - .dimension() - .max_column; + column2 = match self.workbook.worksheet(left.sheet) { + Ok(s) => s.dimension().max_column, + Err(_) => { + return CalcResult::new_error( + Error::ERROR, + cell, + format!("Invalid worksheet index: '{}'", left.sheet), + ); + } + }; } for row in row1..row2 + 1 { for column in column1..(column2 + 1) { @@ -195,20 +203,28 @@ impl Model { let column1 = left.column; let mut column2 = right.column; if row1 == 1 && row2 == LAST_ROW { - row2 = self - .workbook - .worksheet(left.sheet) - .expect("Sheet expected during evaluation.") - .dimension() - .max_row; + row2 = match self.workbook.worksheet(left.sheet) { + Ok(s) => s.dimension().max_row, + Err(_) => { + return CalcResult::new_error( + Error::ERROR, + cell, + format!("Invalid worksheet index: '{}'", left.sheet), + ); + } + }; } if column1 == 1 && column2 == LAST_COLUMN { - column2 = self - .workbook - .worksheet(left.sheet) - .expect("Sheet expected during evaluation.") - .dimension() - .max_column; + column2 = match self.workbook.worksheet(left.sheet) { + Ok(s) => s.dimension().max_column, + Err(_) => { + return CalcResult::new_error( + Error::ERROR, + cell, + format!("Invalid worksheet index: '{}'", left.sheet), + ); + } + }; } for row in row1..row2 + 1 { for column in column1..(column2 + 1) { diff --git a/base/src/functions/mod.rs b/base/src/functions/mod.rs index b870ec9..f637288 100644 --- a/base/src/functions/mod.rs +++ b/base/src/functions/mod.rs @@ -1148,6 +1148,7 @@ impl Model { #[cfg(test)] mod tests { + #![allow(clippy::unwrap_used)] use std::{ fs::File, io::{BufRead, BufReader}, diff --git a/base/src/functions/statistical.rs b/base/src/functions/statistical.rs index 637b9bd..2c1523c 100644 --- a/base/src/functions/statistical.rs +++ b/base/src/functions/statistical.rs @@ -381,11 +381,16 @@ impl Model { let right_row = first_range.right.row; let right_column = first_range.right.column; - let dimension = self - .workbook - .worksheet(first_range.left.sheet) - .expect("Sheet expected during evaluation.") - .dimension(); + let dimension = match self.workbook.worksheet(first_range.left.sheet) { + Ok(s) => s.dimension(), + Err(_) => { + return CalcResult::new_error( + Error::ERROR, + cell, + format!("Invalid worksheet index: '{}'", first_range.left.sheet), + ) + } + }; let max_row = dimension.max_row; let max_column = dimension.max_column; @@ -526,20 +531,28 @@ impl Model { let mut right_column = sum_range.right.column; if left_row == 1 && right_row == LAST_ROW { - right_row = self - .workbook - .worksheet(sum_range.left.sheet) - .expect("Sheet expected during evaluation.") - .dimension() - .max_row; + right_row = match self.workbook.worksheet(sum_range.left.sheet) { + Ok(s) => s.dimension().max_row, + Err(_) => { + return Err(CalcResult::new_error( + Error::ERROR, + cell, + format!("Invalid worksheet index: '{}'", sum_range.left.sheet), + )); + } + }; } if left_column == 1 && right_column == LAST_COLUMN { - right_column = self - .workbook - .worksheet(sum_range.left.sheet) - .expect("Sheet expected during evaluation.") - .dimension() - .max_column; + right_column = match self.workbook.worksheet(sum_range.left.sheet) { + Ok(s) => s.dimension().max_column, + Err(_) => { + return Err(CalcResult::new_error( + Error::ERROR, + cell, + format!("Invalid worksheet index: '{}'", sum_range.left.sheet), + )); + } + }; } for row in left_row..right_row + 1 { diff --git a/base/src/functions/subtotal.rs b/base/src/functions/subtotal.rs index 598b269..d0f9b86 100644 --- a/base/src/functions/subtotal.rs +++ b/base/src/functions/subtotal.rs @@ -53,8 +53,13 @@ impl Model { false } - fn cell_hidden_status(&self, sheet_index: u32, row: i32, column: i32) -> CellTableStatus { - let worksheet = self.workbook.worksheet(sheet_index).expect(""); + fn cell_hidden_status( + &self, + sheet_index: u32, + row: i32, + column: i32, + ) -> Result { + let worksheet = self.workbook.worksheet(sheet_index)?; let mut hidden = false; for row_style in &worksheet.rows { if row_style.r == row { @@ -63,13 +68,13 @@ impl Model { } } if !hidden { - return CellTableStatus::Normal; + return Ok(CellTableStatus::Normal); } // The row is hidden we need to know if the table has filters if self.get_table_for_cell(sheet_index, row, column) { - CellTableStatus::Filtered + Ok(CellTableStatus::Filtered) } else { - CellTableStatus::Hidden + Ok(CellTableStatus::Hidden) } } @@ -143,7 +148,11 @@ impl Model { let column2 = right.column; for row in row1..=row2 { - let cell_status = self.cell_hidden_status(left.sheet, row, column1); + let cell_status = self + .cell_hidden_status(left.sheet, row, column1) + .map_err(|message| { + CalcResult::new_error(Error::ERROR, cell, message) + })?; if cell_status == CellTableStatus::Filtered { continue; } @@ -380,7 +389,14 @@ impl Model { let column2 = right.column; for row in row1..=row2 { - let cell_status = self.cell_hidden_status(left.sheet, row, column1); + let cell_status = match self + .cell_hidden_status(left.sheet, row, column1) + { + Ok(s) => s, + Err(message) => { + return CalcResult::new_error(Error::ERROR, cell, message); + } + }; if cell_status == CellTableStatus::Filtered { continue; } @@ -449,7 +465,14 @@ impl Model { let column2 = right.column; for row in row1..=row2 { - let cell_status = self.cell_hidden_status(left.sheet, row, column1); + let cell_status = match self + .cell_hidden_status(left.sheet, row, column1) + { + Ok(s) => s, + Err(message) => { + return CalcResult::new_error(Error::ERROR, cell, message); + } + }; if cell_status == CellTableStatus::Filtered { continue; } diff --git a/base/src/functions/text.rs b/base/src/functions/text.rs index a72b8ee..3a48767 100644 --- a/base/src/functions/text.rs +++ b/base/src/functions/text.rs @@ -888,20 +888,28 @@ impl Model { let column1 = left.column; let mut column2 = right.column; if row1 == 1 && row2 == LAST_ROW { - row2 = self - .workbook - .worksheet(left.sheet) - .expect("Sheet expected during evaluation.") - .dimension() - .max_row; + row2 = match self.workbook.worksheet(left.sheet) { + Ok(s) => s.dimension().max_row, + Err(_) => { + return CalcResult::new_error( + Error::ERROR, + cell, + format!("Invalid worksheet index: '{}'", left.sheet), + ); + } + }; } if column1 == 1 && column2 == LAST_COLUMN { - column2 = self - .workbook - .worksheet(left.sheet) - .expect("Sheet expected during evaluation.") - .dimension() - .max_column; + column2 = match self.workbook.worksheet(left.sheet) { + Ok(s) => s.dimension().max_column, + Err(_) => { + return CalcResult::new_error( + Error::ERROR, + cell, + format!("Invalid worksheet index: '{}'", left.sheet), + ); + } + }; } for row in row1..row2 + 1 { for column in column1..(column2 + 1) { diff --git a/base/src/functions/xlookup.rs b/base/src/functions/xlookup.rs index 3a915b0..016d97a 100644 --- a/base/src/functions/xlookup.rs +++ b/base/src/functions/xlookup.rs @@ -251,20 +251,28 @@ impl Model { let column1 = left.column; if row1 == 1 && row2 == LAST_ROW { - row2 = self - .workbook - .worksheet(left.sheet) - .expect("Sheet expected during evaluation.") - .dimension() - .max_row; + row2 = match self.workbook.worksheet(left.sheet) { + Ok(s) => s.dimension().max_row, + Err(_) => { + return CalcResult::new_error( + Error::ERROR, + cell, + format!("Invalid worksheet index: '{}'", left.sheet), + ); + } + }; } if column1 == 1 && column2 == LAST_COLUMN { - column2 = self - .workbook - .worksheet(left.sheet) - .expect("Sheet expected during evaluation.") - .dimension() - .max_column; + column2 = match self.workbook.worksheet(left.sheet) { + Ok(s) => s.dimension().max_column, + Err(_) => { + return CalcResult::new_error( + Error::ERROR, + cell, + format!("Invalid worksheet index: '{}'", left.sheet), + ); + } + }; } let left = CellReferenceIndex { sheet: left.sheet, diff --git a/base/src/language/mod.rs b/base/src/language/mod.rs index 4667a3f..ef362c4 100644 --- a/base/src/language/mod.rs +++ b/base/src/language/mod.rs @@ -31,6 +31,7 @@ pub struct Language { pub errors: Errors, } +#[allow(clippy::expect_used)] static LANGUAGES: Lazy> = Lazy::new(|| { bitcode::decode(include_bytes!("language.bin")).expect("Failed parsing language file") }); diff --git a/base/src/locale/mod.rs b/base/src/locale/mod.rs index bb46f00..3d7ba3c 100644 --- a/base/src/locale/mod.rs +++ b/base/src/locale/mod.rs @@ -65,6 +65,7 @@ pub struct DecimalFormats { pub standard: String, } +#[allow(clippy::expect_used)] static LOCALES: Lazy> = Lazy::new(|| bitcode::decode(include_bytes!("locales.bin")).expect("Failed parsing locale")); diff --git a/base/src/model.rs b/base/src/model.rs index a64b86e..422b068 100644 --- a/base/src/model.rs +++ b/base/src/model.rs @@ -41,6 +41,7 @@ pub use crate::mock_time::get_milliseconds_since_epoch; /// * Or mocked for tests #[cfg(not(test))] #[cfg(not(target_arch = "wasm32"))] +#[allow(clippy::expect_used)] pub fn get_milliseconds_since_epoch() -> i64 { use std::time::{SystemTime, UNIX_EPOCH}; SystemTime::now() @@ -529,6 +530,7 @@ impl Model { } } + #[allow(clippy::expect_used)] fn cell_reference_to_string( &self, cell_reference: &CellReferenceIndex, @@ -544,6 +546,7 @@ impl Model { /// Sets `result` in the cell given by `sheet` sheet index, row and column /// Note that will panic if the cell does not exist /// It will do nothing if the cell does not have a formula + #[allow(clippy::expect_used)] fn set_cell_value(&mut self, cell_reference: CellReferenceIndex, result: &CalcResult) { let CellReferenceIndex { sheet, column, row } = cell_reference; let cell = &self.workbook.worksheets[sheet as usize].sheet_data[&row][&column]; @@ -875,6 +878,7 @@ impl Model { .map_err(|_| format!("Invalid timezone: {}", workbook.settings.tz))?; // FIXME: Add support for display languages + #[allow(clippy::expect_used)] let language = get_language("en").expect("").clone(); let mut shared_strings = HashMap::new(); for (index, s) in workbook.shared_strings.iter().enumerate() { @@ -1986,6 +1990,7 @@ impl Model { #[cfg(test)] mod tests { + #![allow(clippy::expect_used)] use super::CellReferenceIndex as CellReference; use crate::{test::util::new_empty_model, types::Cell}; diff --git a/base/src/new_empty.rs b/base/src/new_empty.rs index fc4ba11..a3da916 100644 --- a/base/src/new_empty.rs +++ b/base/src/new_empty.rs @@ -392,6 +392,7 @@ impl Model { let cells = HashMap::new(); // FIXME: Add support for display languages + #[allow(clippy::expect_used)] let language = get_language("en").expect("").clone(); let mut model = Model { diff --git a/base/src/test/test_error_propagation.rs b/base/src/test/test_error_propagation.rs index fed0a95..5f40085 100644 --- a/base/src/test/test_error_propagation.rs +++ b/base/src/test/test_error_propagation.rs @@ -1,4 +1,5 @@ #![allow(clippy::unwrap_used)] +#![allow(clippy::panic)] use crate::test::util::new_empty_model; use crate::types::Cell; diff --git a/base/src/test/test_metadata.rs b/base/src/test/test_metadata.rs index c1d8742..4938a50 100644 --- a/base/src/test/test_metadata.rs +++ b/base/src/test/test_metadata.rs @@ -1,3 +1,5 @@ +#![allow(clippy::unwrap_used)] + use crate::test::util::new_empty_model; #[test] diff --git a/base/src/test/test_set_functions_error_handling.rs b/base/src/test/test_set_functions_error_handling.rs index 7aeb521..f1f0ab7 100644 --- a/base/src/test/test_set_functions_error_handling.rs +++ b/base/src/test/test_set_functions_error_handling.rs @@ -1,3 +1,5 @@ +#![allow(clippy::unwrap_used)] + use crate::{expressions::token, test::util::new_empty_model, types::Cell}; #[test] diff --git a/base/src/test/user_model/test_diff_queue.rs b/base/src/test/user_model/test_diff_queue.rs index 01c3de4..32e489c 100644 --- a/base/src/test/user_model/test_diff_queue.rs +++ b/base/src/test/user_model/test_diff_queue.rs @@ -1,3 +1,5 @@ +#![allow(clippy::unwrap_used)] + use crate::{ constants::{DEFAULT_COLUMN_WIDTH, DEFAULT_ROW_HEIGHT}, test::util::new_empty_model, diff --git a/base/src/test/user_model/test_paste_csv.rs b/base/src/test/user_model/test_paste_csv.rs index 5c56f29..88a9132 100644 --- a/base/src/test/user_model/test_paste_csv.rs +++ b/base/src/test/user_model/test_paste_csv.rs @@ -1,3 +1,5 @@ +#![allow(clippy::unwrap_used)] + use crate::{expressions::types::Area, UserModel}; #[test] diff --git a/base/src/user_model/common.rs b/base/src/user_model/common.rs index 3ed8d4e..bc841a6 100644 --- a/base/src/user_model/common.rs +++ b/base/src/user_model/common.rs @@ -571,7 +571,10 @@ impl UserModel { break; } } - let data = worksheet.sheet_data.get(&row).unwrap().clone(); + let data = match worksheet.sheet_data.get(&row) { + Some(s) => s.clone(), + None => return Err(format!("Row number '{row}' is not valid.")), + }; let old_data = Box::new(RowData { row: row_data, data, @@ -1369,11 +1372,16 @@ impl UserModel { ); text_row.push(text); } - wtr.write_record(text_row).unwrap(); + wtr.write_record(text_row) + .map_err(|e| format!("Error while processing csv: {}", e))?; data.insert(row, data_row); } - let csv = String::from_utf8(wtr.into_inner().unwrap()).unwrap(); + let csv = String::from_utf8( + wtr.into_inner() + .map_err(|e| format!("Processing error: '{}'", e))?, + ) + .map_err(|e| format!("Error converting from utf8: '{}'", e))?; Ok(Clipboard { csv, diff --git a/base/src/utils.rs b/base/src/utils.rs index b52c71c..5f15aae 100644 --- a/base/src/utils.rs +++ b/base/src/utils.rs @@ -34,6 +34,7 @@ impl ParsedReference { locale: &Locale, get_sheet_index_by_name: F, ) -> Result { + #[allow(clippy::expect_used)] let language = get_language("en").expect(""); let mut lexer = Lexer::new(reference, LexerMode::A1, locale, language); @@ -151,6 +152,8 @@ pub(crate) fn is_valid_hex_color(color: &str) -> bool { #[cfg(test)] mod tests { + #![allow(clippy::expect_used)] + use super::*; use crate::language::get_language; use crate::locale::{get_locale, Locale}; diff --git a/base/src/worksheet.rs b/base/src/worksheet.rs index d5fcbe5..027eafe 100644 --- a/base/src/worksheet.rs +++ b/base/src/worksheet.rs @@ -254,7 +254,7 @@ impl Worksheet { /// Changes the height of a row. /// * If the row does not a have a style we add it. /// * If it has we modify the height and make sure it is applied. - /// + /// /// Fails if column index is outside allowed range. pub fn set_row_height(&mut self, row: i32, height: f64) -> Result<(), String> { if !is_valid_row(row) { @@ -283,7 +283,7 @@ impl Worksheet { /// Changes the width of a column. /// * If the column does not a have a width we simply add it /// * If it has, it might be part of a range and we ned to split the range. - /// + /// /// Fails if column index is outside allowed range. pub fn set_column_width(&mut self, column: i32, width: f64) -> Result<(), String> { self.set_column_width_and_style(column, width, None) diff --git a/bindings/python/Cargo.toml b/bindings/python/Cargo.toml index 68107a2..020a2f7 100644 --- a/bindings/python/Cargo.toml +++ b/bindings/python/Cargo.toml @@ -19,6 +19,3 @@ pyo3 = { version = "0.22.3", features = ["extension-module"] } [features] extension-module = ["pyo3/extension-module"] default = ["extension-module"] - -[tool.maturin] -features = ["pyo3/extension-module"] \ No newline at end of file diff --git a/bindings/python/src/lib.rs b/bindings/python/src/lib.rs index 9ac4717..5c99aa9 100644 --- a/bindings/python/src/lib.rs +++ b/bindings/python/src/lib.rs @@ -208,6 +208,7 @@ impl PyModel { .map_err(|e| WorkbookError::new_err(e.to_string())) } + #[allow(clippy::panic)] pub fn test_panic(&self) -> PyResult<()> { panic!("This function panics for testing panic handling"); } @@ -240,6 +241,7 @@ pub fn create(name: &str, locale: &str, tz: &str) -> PyResult { } #[pyfunction] +#[allow(clippy::panic)] pub fn test_panic() { panic!("This function panics for testing panic handling"); } diff --git a/bindings/wasm/src/lib.rs b/bindings/wasm/src/lib.rs index c1fd0c1..d333573 100644 --- a/bindings/wasm/src/lib.rs +++ b/bindings/wasm/src/lib.rs @@ -274,15 +274,18 @@ impl Model { row: i32, column: i32, ) -> Result { - self.model + let style = self + .model .get_cell_style(sheet, row, column) - .map_err(to_js_error) - .map(|x| serde_wasm_bindgen::to_value(&x).unwrap()) + .map_err(to_js_error)?; + + serde_wasm_bindgen::to_value(&style).map_err(|e| to_js_error(e.to_string())) } #[wasm_bindgen(js_name = "onPasteStyles")] pub fn on_paste_styles(&mut self, styles: JsValue) -> Result<(), JsError> { - let styles: &Vec> = &serde_wasm_bindgen::from_value(styles).unwrap(); + let styles: &Vec> = + &serde_wasm_bindgen::from_value(styles).map_err(|e| to_js_error(e.to_string()))?; self.model.on_paste_styles(styles).map_err(to_js_error) } @@ -304,7 +307,10 @@ impl Model { ) } + // I don't _think_ serializing to JsValue can't fail + // FIXME: Remove this clippy directive #[wasm_bindgen(js_name = "getWorksheetsProperties")] + #[allow(clippy::unwrap_used)] pub fn get_worksheets_properties(&self) -> JsValue { serde_wasm_bindgen::to_value(&self.model.get_worksheets_properties()).unwrap() } @@ -320,7 +326,10 @@ impl Model { vec![sheet as i32, row, column] } + // I don't _think_ serializing to JsValue can't fail + // FIXME: Remove this clippy directive #[wasm_bindgen(js_name = "getSelectedView")] + #[allow(clippy::unwrap_used)] pub fn get_selected_view(&self) -> JsValue { serde_wasm_bindgen::to_value(&self.model.get_selected_view()).unwrap() } @@ -503,8 +512,9 @@ impl Model { let data = self .model .copy_to_clipboard() - .map_err(|e| to_js_error(e.to_string())); - data.map(|x| serde_wasm_bindgen::to_value(&x).unwrap()) + .map_err(|e| to_js_error(e.to_string()))?; + + serde_wasm_bindgen::to_value(&data).map_err(|e| to_js_error(e.to_string())) } #[wasm_bindgen(js_name = "pasteFromClipboard")] diff --git a/xlsx/examples/hello_calc.rs b/xlsx/examples/hello_calc.rs index 7dcabeb..085d2c8 100644 --- a/xlsx/examples/hello_calc.rs +++ b/xlsx/examples/hello_calc.rs @@ -9,17 +9,15 @@ fn main() -> Result<(), Box> { for row in 1..100 { for column in 1..100 { let value = row * column; - model - .set_user_input(0, row, column, format!("{}", value)) - .unwrap(); + model.set_user_input(0, row, column, format!("{}", value))?; } } // Adds a new sheet model.add_sheet("Calculation")?; // column 100 is CV - let last_column = number_to_column(100).unwrap(); + let last_column = number_to_column(100).ok_or("Invalid column number")?; let formula = format!("=SUM(Sheet1!A1:{}100)", last_column); - model.set_user_input(1, 1, 1, formula).unwrap(); + model.set_user_input(1, 1, 1, formula)?; // evaluates model.evaluate(); diff --git a/xlsx/src/bin/documentation.rs b/xlsx/src/bin/documentation.rs index 6e93e44..0ba4b7f 100644 --- a/xlsx/src/bin/documentation.rs +++ b/xlsx/src/bin/documentation.rs @@ -1,3 +1,6 @@ +#![allow(clippy::panic)] +#![allow(clippy::expect_used)] + //! Produces documentation of all the implemented IronCalc functions //! and saves the result to functions.md //! diff --git a/xlsx/src/bin/test.rs b/xlsx/src/bin/test.rs index c7703cc..3dc2ef5 100644 --- a/xlsx/src/bin/test.rs +++ b/xlsx/src/bin/test.rs @@ -1,3 +1,6 @@ +#![allow(clippy::unwrap_used)] +#![allow(clippy::panic)] + //! Tests an Excel xlsx file. //! Returns a list of differences in json format. //! Saves an IronCalc version diff --git a/xlsx/src/bin/xlsx_2_icalc.rs b/xlsx/src/bin/xlsx_2_icalc.rs index e778d2a..a712c14 100644 --- a/xlsx/src/bin/xlsx_2_icalc.rs +++ b/xlsx/src/bin/xlsx_2_icalc.rs @@ -1,3 +1,6 @@ +#![allow(clippy::unwrap_used)] +#![allow(clippy::panic)] + //! Tests an Excel xlsx file. //! Returns a list of differences in json format. //! Saves an IronCalc version diff --git a/xlsx/src/compare.rs b/xlsx/src/compare.rs index 2b1a47e..09798a1 100644 --- a/xlsx/src/compare.rs +++ b/xlsx/src/compare.rs @@ -1,3 +1,5 @@ +#![allow(clippy::unwrap_used)] + use std::path::Path; use ironcalc_base::cell::CellValue; diff --git a/xlsx/src/export/mod.rs b/xlsx/src/export/mod.rs index 7e73e29..00c3ba4 100644 --- a/xlsx/src/export/mod.rs +++ b/xlsx/src/export/mod.rs @@ -1,3 +1,5 @@ +#![allow(clippy::unwrap_used)] + mod _rels; mod doc_props; mod escape; diff --git a/xlsx/src/export/workbook.rs b/xlsx/src/export/workbook.rs index b8d5105..795b83c 100644 --- a/xlsx/src/export/workbook.rs +++ b/xlsx/src/export/workbook.rs @@ -1,3 +1,4 @@ +#![allow(clippy::unwrap_used)] //! //! A workbook is composed of workbook-level properties and a collection of 1 or more sheets. diff --git a/xlsx/src/export/worksheets.rs b/xlsx/src/export/worksheets.rs index 06c5c6f..b3f2cbc 100644 --- a/xlsx/src/export/worksheets.rs +++ b/xlsx/src/export/worksheets.rs @@ -1,3 +1,6 @@ +#![allow(clippy::unwrap_used)] +#![allow(clippy::panic)] + //! # A note on shared formulas //! Although both Excel and IronCalc uses shared formulas they are used in a slightly different way that cannot be mapped 1-1 //! In IronCalc _all_ formulas are shared and there is a list of shared formulas much like there is a list of shared strings. diff --git a/xlsx/src/import/colors.rs b/xlsx/src/import/colors.rs index b6cfd34..2a97cdc 100644 --- a/xlsx/src/import/colors.rs +++ b/xlsx/src/import/colors.rs @@ -1,3 +1,5 @@ +#![allow(clippy::unwrap_used)] + use core::cmp::max; use core::cmp::min; diff --git a/xlsx/src/import/shared_strings.rs b/xlsx/src/import/shared_strings.rs index cd6ccec..2f5a69a 100644 --- a/xlsx/src/import/shared_strings.rs +++ b/xlsx/src/import/shared_strings.rs @@ -38,6 +38,7 @@ fn read_shared_strings_from_string(text: &str) -> Result, XlsxError> #[cfg(test)] mod tests { + #![allow(clippy::unwrap_used)] use super::*; #[test] diff --git a/xlsx/src/import/tables.rs b/xlsx/src/import/tables.rs index acc274f..72ca0c5 100644 --- a/xlsx/src/import/tables.rs +++ b/xlsx/src/import/tables.rs @@ -41,29 +41,35 @@ pub(crate) fn load_table( // They also need to be different from any defined name let name = table .attribute("name") - .expect("Missing table name") + .ok_or_else(|| XlsxError::Xml("Corrupt XML structure: missing table name".to_string()))? .to_string(); let display_name = table .attribute("name") - .expect("Missing table display name") + .ok_or_else(|| { + XlsxError::Xml("Corrupt XML structure: missing table display name".to_string()) + })? .to_string(); // Range of the table, including the totals if any and headers. let reference = table .attribute("ref") - .expect("Missing table ref") + .ok_or_else(|| XlsxError::Xml("Corrupt XML structure: missing table ref".to_string()))? .to_string(); // Either 0 or 1, indicates if the table has a formula for totals at the bottom of the table let totals_row_count = match table.attribute("totalsRowCount") { - Some(s) => s.parse::().expect("Invalid totalsRowCount"), + Some(s) => s.parse::().map_err(|_| { + XlsxError::Xml("Corrupt XML structure: Invalid totalsRowCount".to_string()) + })?, None => 0, }; // Either 0 or 1, indicates if the table has headers at the top of the table let header_row_count = match table.attribute("headerRowCount") { - Some(s) => s.parse::().expect("Invalid headerRowCount"), + Some(s) => s.parse::().map_err(|_| { + XlsxError::Xml("Corrupt XML structure: Invalid headerRowCount".to_string()) + })?, None => 1, }; @@ -125,9 +131,15 @@ pub(crate) fn load_table( .collect::>(); let mut columns = Vec::new(); for table_column in table_column { - let column_name = table_column.attribute("name").expect("Missing column name"); - let id = table_column.attribute("id").expect("Missing column id"); - let id = id.parse::().expect("Invalid id"); + let column_name = table_column.attribute("name").ok_or_else(|| { + XlsxError::Xml("Corrupt XML structure: missing column name".to_string()) + })?; + let id = table_column.attribute("id").ok_or_else(|| { + XlsxError::Xml("Corrupt XML structure: missing column id".to_string()) + })?; + let id = id + .parse::() + .map_err(|_| XlsxError::Xml("Corrupt XML structure: invalid id".to_string()))?; // style index of the header row of the table let header_row_dxf_id = if let Some(index_str) = table_column.attribute("headerRowDxfId") { diff --git a/xlsx/src/import/util.rs b/xlsx/src/import/util.rs index f186b3a..c9626e4 100644 --- a/xlsx/src/import/util.rs +++ b/xlsx/src/import/util.rs @@ -1,3 +1,5 @@ +#![allow(clippy::unwrap_used)] + use colors::{get_indexed_color, get_themed_color}; use roxmltree::{ExpandedName, Node}; diff --git a/xlsx/src/import/worksheets.rs b/xlsx/src/import/worksheets.rs index e529842..3d14b44 100644 --- a/xlsx/src/import/worksheets.rs +++ b/xlsx/src/import/worksheets.rs @@ -1,3 +1,5 @@ +#![allow(clippy::unwrap_used)] + use std::{collections::HashMap, io::Read, num::ParseIntError}; use ironcalc_base::{ @@ -1037,7 +1039,10 @@ pub(super) fn load_sheets( name: sheet_name.to_string(), id: sheet.sheet_id, state: state.clone(), - comments: comments.get(rel_id).expect("").to_vec(), + comments: comments + .get(rel_id) + .ok_or_else(|| XlsxError::Xml("Corrupt XML structure".to_string()))? + .to_vec(), }; let (s, is_selected) = load_sheet(archive, &path, settings, worksheets, tables, shared_strings)?; diff --git a/xlsx/tests/test.rs b/xlsx/tests/test.rs index 904ef04..7b18790 100644 --- a/xlsx/tests/test.rs +++ b/xlsx/tests/test.rs @@ -1,3 +1,6 @@ +#![allow(clippy::unwrap_used)] +#![allow(clippy::panic)] + use std::io::Read; use std::{env, fs, io}; use uuid::Uuid;