From 690032c81177a13974191eeebd36c0fc27e11cb1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nicol=C3=A1s=20Hatcher?= Date: Thu, 26 Dec 2024 00:02:28 +0100 Subject: [PATCH] FIX: Remove optional context in parser The context was optional because I thought that paring an RC formula did not need context. You at least need the sheet in which you are parsing For instance toknow if a defined name is local --- base/src/expressions/parser/mod.rs | 378 ++++++++---------- .../expressions/parser/tests/test_general.rs | 46 +-- .../parser/tests/test_issue_155.rs | 8 +- .../parser/tests/test_move_formula.rs | 43 +- .../expressions/parser/tests/test_ranges.rs | 10 +- .../parser/tests/test_stringify.rs | 10 +- .../expressions/parser/tests/test_tables.rs | 6 +- base/src/model.rs | 16 +- base/src/new_empty.rs | 10 +- xlsx/src/import/worksheets.rs | 2 +- 10 files changed, 246 insertions(+), 283 deletions(-) diff --git a/base/src/expressions/parser/mod.rs b/base/src/expressions/parser/mod.rs index 3233c2e..fd24551 100644 --- a/base/src/expressions/parser/mod.rs +++ b/base/src/expressions/parser/mod.rs @@ -190,7 +190,7 @@ pub struct Parser { lexer: lexer::Lexer, worksheets: Vec, defined_names: Vec<(String, Option)>, - context: Option, + context: CellReferenceRC, tables: HashMap, } @@ -208,11 +208,16 @@ impl Parser { #[allow(clippy::expect_used)] get_language("en").expect(""), ); + let context = CellReferenceRC { + sheet: worksheets.first().map_or("", |v| v).to_string(), + column: 1, + row: 1, + }; Parser { lexer, worksheets, defined_names, - context: None, + context, tables, } } @@ -229,9 +234,9 @@ impl Parser { self.defined_names = defined_names; } - pub fn parse(&mut self, formula: &str, context: &Option) -> Node { + pub fn parse(&mut self, formula: &str, context: &CellReferenceRC) -> Node { self.lexer.set_formula(formula); - self.context.clone_from(context); + self.context = context.clone(); self.parse_expr() } @@ -477,16 +482,7 @@ impl Parser { absolute_column, absolute_row, } => { - let context = match &self.context { - Some(c) => c, - None => { - return Node::ParseErrorKind { - formula: self.lexer.get_formula(), - position: self.lexer.get_position() as usize, - message: "Expected context for the reference".to_string(), - } - } - }; + let context = &self.context; let sheet_index = match &sheet { Some(name) => self.get_sheet_index_by_name(name), None => self.get_sheet_index_by_name(&context.sheet), @@ -521,16 +517,7 @@ impl Parser { } } TokenType::Range { sheet, left, right } => { - let context = match &self.context { - Some(c) => c, - None => { - return Node::ParseErrorKind { - formula: self.lexer.get_formula(), - position: self.lexer.get_position() as usize, - message: "Expected context for the reference".to_string(), - } - } - }; + let context = &self.context; let sheet_index = match &sheet { Some(name) => self.get_sheet_index_by_name(name), None => self.get_sheet_index_by_name(&context.sheet), @@ -619,16 +606,7 @@ impl Parser { } return Node::InvalidFunctionKind { name, args }; } - let context = match &self.context { - Some(c) => c, - None => { - return Node::ParseErrorKind { - formula: self.lexer.get_formula(), - position: self.lexer.get_position() as usize, - message: "Expected context for the reference".to_string(), - } - } - }; + let context = &self.context; let context_sheet_index = match self.get_sheet_index_by_name(&context.sheet) { Some(i) => i, @@ -723,187 +701,177 @@ impl Parser { // We will try to convert to a normal reference // table_name[column_name] => cell1:cell2 // table_name[[#This Row], [column_name]:[column_name]] => cell1:cell2 - if let Some(context) = &self.context { - let context_sheet_index = match self.get_sheet_index_by_name(&context.sheet) { - Some(i) => i, - None => { - return Node::ParseErrorKind { - formula: self.lexer.get_formula(), - position: 0, - message: "sheet not found".to_string(), - }; - } - }; - // table-name => table - 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 => { - return Node::ParseErrorKind { - formula: self.lexer.get_formula(), - position: 0, - message: "sheet not found".to_string(), - }; - } - }; + let context = &self.context; + let context_sheet_index = match self.get_sheet_index_by_name(&context.sheet) { + Some(i) => i, + None => { + return Node::ParseErrorKind { + formula: self.lexer.get_formula(), + position: 0, + message: "sheet not found".to_string(), + }; + } + }; + // table-name => table + 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 => { + return Node::ParseErrorKind { + formula: self.lexer.get_formula(), + position: 0, + message: "sheet not found".to_string(), + }; + } + }; - let sheet_name = if table_sheet_index == context_sheet_index { - None - } else { - Some(table.sheet_name.clone()) - }; + let sheet_name = if table_sheet_index == context_sheet_index { + None + } else { + Some(table.sheet_name.clone()) + }; - // 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"); + // 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"); - let totals_row_count = table.totals_row_count as i32; - let header_row_count = table.header_row_count as i32; - row_end -= totals_row_count; + let totals_row_count = table.totals_row_count as i32; + let header_row_count = table.header_row_count as i32; + row_end -= totals_row_count; - match specifier { - Some(token::TableSpecifier::ThisRow) => { - row_start = context.row; - row_end = context.row; - } - Some(token::TableSpecifier::Totals) => { - if totals_row_count != 0 { - row_start = row_end + 1; - row_end = row_start; - } else { - // Table1[#Totals] is #REF! if Table1 does not have totals - return Node::ErrorKind(token::Error::REF); - } - } - Some(token::TableSpecifier::Headers) => { + match specifier { + Some(token::TableSpecifier::ThisRow) => { + row_start = context.row; + row_end = context.row; + } + Some(token::TableSpecifier::Totals) => { + if totals_row_count != 0 { + row_start = row_end + 1; row_end = row_start; - } - Some(token::TableSpecifier::Data) => { - row_start += header_row_count; - } - Some(token::TableSpecifier::All) => { - if totals_row_count != 0 { - row_end += 1; - } - } - None => { - // skip the headers - row_start += header_row_count; + } else { + // Table1[#Totals] is #REF! if Table1 does not have totals + return Node::ErrorKind(token::Error::REF); } } - match table_reference { - None => { - return Node::RangeKind { - sheet_name, - sheet_index: table_sheet_index, - absolute_row1: true, - absolute_column1: true, - row1: row_start, - column1: column_start, - absolute_row2: true, - absolute_column2: true, - row2: row_end, - column2: column_end, - }; - } - Some(TableReference::ColumnReference(s)) => { - let column_index = match get_table_column_by_name(&s, table) { - Some(s) => s + column_start, - None => { - return Node::ParseErrorKind { - formula: self.lexer.get_formula(), - position: self.lexer.get_position() as usize, - message: format!( - "Expecting column: {s} in table {table_name}" - ), - }; - } - }; - if row_start == row_end { - return Node::ReferenceKind { - sheet_name, - sheet_index: table_sheet_index, - absolute_row: true, - absolute_column: true, - row: row_start, - column: column_index, - }; - } - return Node::RangeKind { - sheet_name, - sheet_index: table_sheet_index, - absolute_row1: true, - absolute_column1: true, - row1: row_start, - column1: column_index, - absolute_row2: true, - absolute_column2: true, - row2: row_end, - column2: column_index, - }; - } - Some(TableReference::RangeReference((left, right))) => { - let left_column_index = match get_table_column_by_name(&left, table) { - Some(f) => f + column_start, - None => { - return Node::ParseErrorKind { - formula: self.lexer.get_formula(), - position: self.lexer.get_position() as usize, - message: format!( - "Expecting column: {left} in table {table_name}" - ), - }; - } - }; - - let right_column_index = match get_table_column_by_name(&right, table) { - Some(f) => f + column_start, - None => { - return Node::ParseErrorKind { - formula: self.lexer.get_formula(), - position: self.lexer.get_position() as usize, - message: format!( - "Expecting column: {right} in table {table_name}" - ), - }; - } - }; - return Node::RangeKind { - sheet_name, - sheet_index: table_sheet_index, - absolute_row1: true, - absolute_column1: true, - row1: row_start, - column1: left_column_index, - absolute_row2: true, - absolute_column2: true, - row2: row_end, - column2: right_column_index, - }; + Some(token::TableSpecifier::Headers) => { + row_end = row_start; + } + Some(token::TableSpecifier::Data) => { + row_start += header_row_count; + } + Some(token::TableSpecifier::All) => { + if totals_row_count != 0 { + row_end += 1; } } + None => { + // skip the headers + row_start += header_row_count; + } } - Node::ParseErrorKind { - formula: self.lexer.get_formula(), - position: 0, - message: "Structured references not supported in R1C1 mode".to_string(), + match table_reference { + None => Node::RangeKind { + sheet_name, + sheet_index: table_sheet_index, + absolute_row1: true, + absolute_column1: true, + row1: row_start, + column1: column_start, + absolute_row2: true, + absolute_column2: true, + row2: row_end, + column2: column_end, + }, + Some(TableReference::ColumnReference(s)) => { + let column_index = match get_table_column_by_name(&s, table) { + Some(s) => s + column_start, + None => { + return Node::ParseErrorKind { + formula: self.lexer.get_formula(), + position: self.lexer.get_position() as usize, + message: format!("Expecting column: {s} in table {table_name}"), + }; + } + }; + if row_start == row_end { + return Node::ReferenceKind { + sheet_name, + sheet_index: table_sheet_index, + absolute_row: true, + absolute_column: true, + row: row_start, + column: column_index, + }; + } + Node::RangeKind { + sheet_name, + sheet_index: table_sheet_index, + absolute_row1: true, + absolute_column1: true, + row1: row_start, + column1: column_index, + absolute_row2: true, + absolute_column2: true, + row2: row_end, + column2: column_index, + } + } + Some(TableReference::RangeReference((left, right))) => { + let left_column_index = match get_table_column_by_name(&left, table) { + Some(f) => f + column_start, + None => { + return Node::ParseErrorKind { + formula: self.lexer.get_formula(), + position: self.lexer.get_position() as usize, + message: format!( + "Expecting column: {left} in table {table_name}" + ), + }; + } + }; + + let right_column_index = match get_table_column_by_name(&right, table) { + Some(f) => f + column_start, + None => { + return Node::ParseErrorKind { + formula: self.lexer.get_formula(), + position: self.lexer.get_position() as usize, + message: format!( + "Expecting column: {right} in table {table_name}" + ), + }; + } + }; + Node::RangeKind { + sheet_name, + sheet_index: table_sheet_index, + absolute_row1: true, + absolute_column1: true, + row1: row_start, + column1: left_column_index, + absolute_row2: true, + absolute_column2: true, + row2: row_end, + column2: right_column_index, + } + } } } } diff --git a/base/src/expressions/parser/tests/test_general.rs b/base/src/expressions/parser/tests/test_general.rs index 7c7c842..6690892 100644 --- a/base/src/expressions/parser/tests/test_general.rs +++ b/base/src/expressions/parser/tests/test_general.rs @@ -25,7 +25,7 @@ fn test_parser_reference() { row: 1, column: 1, }; - let t = parser.parse("A2", &Some(cell_reference)); + let t = parser.parse("A2", &cell_reference); assert_eq!(to_rc_format(&t), "R[1]C[0]"); } @@ -40,7 +40,7 @@ fn test_parser_absolute_column() { row: 1, column: 1, }; - let t = parser.parse("$A1", &Some(cell_reference)); + let t = parser.parse("$A1", &cell_reference); assert_eq!(to_rc_format(&t), "R[0]C1"); } @@ -55,7 +55,7 @@ fn test_parser_absolute_row_col() { row: 1, column: 1, }; - let t = parser.parse("$C$5", &Some(cell_reference)); + let t = parser.parse("$C$5", &cell_reference); assert_eq!(to_rc_format(&t), "R5C3"); } @@ -70,7 +70,7 @@ fn test_parser_absolute_row_col_1() { row: 1, column: 1, }; - let t = parser.parse("$A$1", &Some(cell_reference)); + let t = parser.parse("$A$1", &cell_reference); assert_eq!(to_rc_format(&t), "R1C1"); } @@ -86,7 +86,7 @@ fn test_parser_simple_formula() { column: 1, }; - let t = parser.parse("C3+Sheet2!D4", &Some(cell_reference)); + let t = parser.parse("C3+Sheet2!D4", &cell_reference); assert_eq!(to_rc_format(&t), "R[2]C[2]+Sheet2!R[3]C[3]"); } @@ -102,7 +102,7 @@ fn test_parser_boolean() { column: 1, }; - let t = parser.parse("true", &Some(cell_reference)); + let t = parser.parse("true", &cell_reference); assert_eq!(to_rc_format(&t), "TRUE"); } @@ -117,7 +117,7 @@ fn test_parser_bad_formula() { row: 1, column: 1, }; - let t = parser.parse("#Value", &Some(cell_reference)); + let t = parser.parse("#Value", &cell_reference); match &t { Node::ParseErrorKind { formula, @@ -146,7 +146,7 @@ fn test_parser_bad_formula_1() { row: 1, column: 1, }; - let t = parser.parse("<5", &Some(cell_reference)); + let t = parser.parse("<5", &cell_reference); match &t { Node::ParseErrorKind { formula, @@ -175,7 +175,7 @@ fn test_parser_bad_formula_2() { row: 1, column: 1, }; - let t = parser.parse("*5", &Some(cell_reference)); + let t = parser.parse("*5", &cell_reference); match &t { Node::ParseErrorKind { formula, @@ -204,7 +204,7 @@ fn test_parser_bad_formula_3() { row: 1, column: 1, }; - let t = parser.parse("SUM(#VALVE!)", &Some(cell_reference)); + let t = parser.parse("SUM(#VALVE!)", &cell_reference); match &t { Node::ParseErrorKind { formula, @@ -259,11 +259,11 @@ fn test_parser_formulas() { for formula in formulas { let t = parser.parse( formula.initial, - &Some(CellReferenceRC { + &CellReferenceRC { sheet: "Sheet1".to_string(), row: 1, column: 1, - }), + }, ); assert_eq!(to_rc_format(&t), formula.expected); assert_eq!(to_string(&t, &cell_reference), formula.initial); @@ -324,11 +324,11 @@ fn test_parser_r1c1_formulas() { for formula in formulas { let t = parser.parse( formula.initial, - &Some(CellReferenceRC { + &CellReferenceRC { sheet: "Sheet1".to_string(), row: 1, column: 1, - }), + }, ); assert_eq!(to_string(&t, &cell_reference), formula.expected); assert_eq!(to_rc_format(&t), formula.initial); @@ -347,7 +347,7 @@ fn test_parser_quotes() { column: 1, }; - let t = parser.parse("C3+'Second Sheet'!D4", &Some(cell_reference)); + let t = parser.parse("C3+'Second Sheet'!D4", &cell_reference); assert_eq!(to_rc_format(&t), "R[2]C[2]+'Second Sheet'!R[3]C[3]"); } @@ -363,7 +363,7 @@ fn test_parser_escape_quotes() { column: 1, }; - let t = parser.parse("C3+'Second ''2'' Sheet'!D4", &Some(cell_reference)); + let t = parser.parse("C3+'Second ''2'' Sheet'!D4", &cell_reference); assert_eq!(to_rc_format(&t), "R[2]C[2]+'Second ''2'' Sheet'!R[3]C[3]"); } @@ -379,7 +379,7 @@ fn test_parser_parenthesis() { column: 1, }; - let t = parser.parse("(C3=\"Yes\")*5", &Some(cell_reference)); + let t = parser.parse("(C3=\"Yes\")*5", &cell_reference); assert_eq!(to_rc_format(&t), "(R[2]C[2]=\"Yes\")*5"); } @@ -395,7 +395,7 @@ fn test_parser_excel_xlfn() { column: 1, }; - let t = parser.parse("_xlfn.CONCAT(C3)", &Some(cell_reference)); + let t = parser.parse("_xlfn.CONCAT(C3)", &cell_reference); assert_eq!(to_rc_format(&t), "CONCAT(R[2]C[2])"); } @@ -409,7 +409,7 @@ fn test_to_string_displaced() { let worksheets = vec!["Sheet1".to_string()]; let mut parser = Parser::new(worksheets, vec![], HashMap::new()); - let node = parser.parse("C3", &Some(context.clone())); + let node = parser.parse("C3", context); let displace_data = DisplaceData::Column { sheet: 0, column: 1, @@ -429,7 +429,7 @@ fn test_to_string_displaced_full_ranges() { let worksheets = vec!["Sheet1".to_string()]; let mut parser = Parser::new(worksheets, vec![], HashMap::new()); - let node = parser.parse("SUM(3:3)", &Some(context.clone())); + let node = parser.parse("SUM(3:3)", context); let displace_data = DisplaceData::Column { sheet: 0, column: 1, @@ -440,7 +440,7 @@ fn test_to_string_displaced_full_ranges() { "SUM(3:3)".to_string() ); - let node = parser.parse("SUM(D:D)", &Some(context.clone())); + let node = parser.parse("SUM(D:D)", context); let displace_data = DisplaceData::Row { sheet: 0, row: 3, @@ -462,7 +462,7 @@ fn test_to_string_displaced_too_low() { let worksheets = vec!["Sheet1".to_string()]; let mut parser = Parser::new(worksheets, vec![], HashMap::new()); - let node = parser.parse("C3", &Some(context.clone())); + let node = parser.parse("C3", context); let displace_data = DisplaceData::Column { sheet: 0, column: 1, @@ -482,7 +482,7 @@ fn test_to_string_displaced_too_high() { let worksheets = vec!["Sheet1".to_string()]; let mut parser = Parser::new(worksheets, vec![], HashMap::new()); - let node = parser.parse("C3", &Some(context.clone())); + let node = parser.parse("C3", context); let displace_data = DisplaceData::Column { sheet: 0, column: 1, diff --git a/base/src/expressions/parser/tests/test_issue_155.rs b/base/src/expressions/parser/tests/test_issue_155.rs index 8d26346..6e8f7ba 100644 --- a/base/src/expressions/parser/tests/test_issue_155.rs +++ b/base/src/expressions/parser/tests/test_issue_155.rs @@ -17,7 +17,7 @@ fn issue_155_parser() { row: 2, column: 2, }; - let t = parser.parse("A$1:A2", &Some(cell_reference.clone())); + let t = parser.parse("A$1:A2", &cell_reference); assert_eq!(to_string(&t, &cell_reference), "A$1:A2"); } @@ -32,7 +32,7 @@ fn issue_155_parser_case_2() { row: 20, column: 20, }; - let t = parser.parse("C$1:D2", &Some(cell_reference.clone())); + let t = parser.parse("C$1:D2", &cell_reference); assert_eq!(to_string(&t, &cell_reference), "C$1:D2"); } @@ -48,7 +48,7 @@ fn issue_155_parser_only_row() { column: 20, }; // This is tricky, I am not sure what to do in these cases - let t = parser.parse("A$2:B1", &Some(cell_reference.clone())); + let t = parser.parse("A$2:B1", &cell_reference); assert_eq!(to_string(&t, &cell_reference), "A1:B$2"); } @@ -64,6 +64,6 @@ fn issue_155_parser_only_column() { column: 20, }; // This is tricky, I am not sure what to do in these cases - let t = parser.parse("D1:$A3", &Some(cell_reference.clone())); + let t = parser.parse("D1:$A3", &cell_reference); assert_eq!(to_string(&t, &cell_reference), "$A1:D3"); } diff --git a/base/src/expressions/parser/tests/test_move_formula.rs b/base/src/expressions/parser/tests/test_move_formula.rs index 5aa43ad..37fde78 100644 --- a/base/src/expressions/parser/tests/test_move_formula.rs +++ b/base/src/expressions/parser/tests/test_move_formula.rs @@ -27,7 +27,7 @@ fn test_move_formula() { }; // formula AB31 will not change - let node = parser.parse("AB31", &Some(context.clone())); + let node = parser.parse("AB31", context); let t = move_formula( &node, &MoveContext { @@ -43,7 +43,7 @@ fn test_move_formula() { assert_eq!(t, "AB31"); // formula $AB$31 will not change - let node = parser.parse("AB31", &Some(context.clone())); + let node = parser.parse("AB31", context); let t = move_formula( &node, &MoveContext { @@ -59,7 +59,7 @@ fn test_move_formula() { assert_eq!(t, "AB31"); // but formula D5 will change to N15 (N = D + 10) - let node = parser.parse("D5", &Some(context.clone())); + let node = parser.parse("D5", context); let t = move_formula( &node, &MoveContext { @@ -75,7 +75,7 @@ fn test_move_formula() { assert_eq!(t, "N15"); // Also formula $D$5 will change to N15 (N = D + 10) - let node = parser.parse("$D$5", &Some(context.clone())); + let node = parser.parse("$D$5", context); let t = move_formula( &node, &MoveContext { @@ -113,7 +113,7 @@ fn test_move_formula_context_offset() { height: 5, }; - let node = parser.parse("-X9+C2%", &Some(context.clone())); + let node = parser.parse("-X9+C2%", context); let t = move_formula( &node, &MoveContext { @@ -152,7 +152,7 @@ fn test_move_formula_area_limits() { }; // Outside of the area. Not moved - let node = parser.parse("B2+B3+C1+G6+H5", &Some(context.clone())); + let node = parser.parse("B2+B3+C1+G6+H5", context); let t = move_formula( &node, &MoveContext { @@ -168,7 +168,7 @@ fn test_move_formula_area_limits() { assert_eq!(t, "B2+B3+C1+G6+H5"); // In the area. Moved - let node = parser.parse("C2+F4+F5+F6", &Some(context.clone())); + let node = parser.parse("C2+F4+F5+F6", context); let t = move_formula( &node, &MoveContext { @@ -205,7 +205,7 @@ fn test_move_formula_ranges() { height: 5, }; // Ranges inside the area are fully displaced (absolute or not) - let node = parser.parse("SUM(C2:F5)", &Some(context.clone())); + let node = parser.parse("SUM(C2:F5)", context); let t = move_formula( &node, &MoveContext { @@ -220,7 +220,7 @@ fn test_move_formula_ranges() { ); assert_eq!(t, "SUM(M12:P15)"); - let node = parser.parse("SUM($C$2:$F$5)", &Some(context.clone())); + let node = parser.parse("SUM($C$2:$F$5)", context); let t = move_formula( &node, &MoveContext { @@ -236,7 +236,7 @@ fn test_move_formula_ranges() { assert_eq!(t, "SUM($M$12:$P$15)"); // Ranges completely outside of the area are not touched - let node = parser.parse("SUM(A1:B3)", &Some(context.clone())); + let node = parser.parse("SUM(A1:B3)", context); let t = move_formula( &node, &MoveContext { @@ -251,7 +251,7 @@ fn test_move_formula_ranges() { ); assert_eq!(t, "SUM(A1:B3)"); - let node = parser.parse("SUM($A$1:$B$3)", &Some(context.clone())); + let node = parser.parse("SUM($A$1:$B$3)", context); let t = move_formula( &node, &MoveContext { @@ -267,7 +267,7 @@ fn test_move_formula_ranges() { assert_eq!(t, "SUM($A$1:$B$3)"); // Ranges that overlap with the area are also NOT displaced - let node = parser.parse("SUM(A1:F5)", &Some(context.clone())); + let node = parser.parse("SUM(A1:F5)", context); let t = move_formula( &node, &MoveContext { @@ -283,7 +283,7 @@ fn test_move_formula_ranges() { assert_eq!(t, "SUM(A1:F5)"); // Ranges that contain the area are also NOT displaced - let node = parser.parse("SUM(A1:X50)", &Some(context.clone())); + let node = parser.parse("SUM(A1:X50)", context); let t = move_formula( &node, &MoveContext { @@ -321,7 +321,7 @@ fn test_move_formula_wrong_reference() { let mut parser = Parser::new(worksheets, vec![], HashMap::new()); // Wrong formulas will NOT be displaced - let node = parser.parse("Sheet3!AB31", &Some(context.clone())); + let node = parser.parse("Sheet3!AB31", context); let t = move_formula( &node, &MoveContext { @@ -335,7 +335,7 @@ fn test_move_formula_wrong_reference() { }, ); assert_eq!(t, "Sheet3!AB31"); - let node = parser.parse("Sheet3!$X$9", &Some(context.clone())); + let node = parser.parse("Sheet3!$X$9", context); let t = move_formula( &node, &MoveContext { @@ -350,7 +350,7 @@ fn test_move_formula_wrong_reference() { ); assert_eq!(t, "Sheet3!$X$9"); - let node = parser.parse("SUM(Sheet3!D2:D3)", &Some(context.clone())); + let node = parser.parse("SUM(Sheet3!D2:D3)", context); let t = move_formula( &node, &MoveContext { @@ -387,7 +387,7 @@ fn test_move_formula_misc() { width: 4, height: 5, }; - let node = parser.parse("X9^C2-F4*H2", &Some(context.clone())); + let node = parser.parse("X9^C2-F4*H2", context); let t = move_formula( &node, &MoveContext { @@ -402,7 +402,7 @@ fn test_move_formula_misc() { ); assert_eq!(t, "X9^M12-P14*H2"); - let node = parser.parse("F5*(-D5)*SUM(A1, X9, $D$5)", &Some(context.clone())); + let node = parser.parse("F5*(-D5)*SUM(A1, X9, $D$5)", context); let t = move_formula( &node, &MoveContext { @@ -417,7 +417,7 @@ fn test_move_formula_misc() { ); assert_eq!(t, "P15*(-N15)*SUM(A1,X9,$N$15)"); - let node = parser.parse("IF(F5 < -D5, X9 & F5, FALSE)", &Some(context.clone())); + let node = parser.parse("IF(F5 < -D5, X9 & F5, FALSE)", context); let t = move_formula( &node, &MoveContext { @@ -457,10 +457,7 @@ fn test_move_formula_another_sheet() { }; // Formula AB31 and JJ3:JJ4 refers to original Sheet1!AB31 and Sheet1!JJ3:JJ4 - let node = parser.parse( - "AB31*SUM(JJ3:JJ4)+SUM(Sheet2!C2:F6)*SUM(C2:F6)", - &Some(context.clone()), - ); + let node = parser.parse("AB31*SUM(JJ3:JJ4)+SUM(Sheet2!C2:F6)*SUM(C2:F6)", context); let t = move_formula( &node, &MoveContext { diff --git a/base/src/expressions/parser/tests/test_ranges.rs b/base/src/expressions/parser/tests/test_ranges.rs index 3f05ab3..ec6766c 100644 --- a/base/src/expressions/parser/tests/test_ranges.rs +++ b/base/src/expressions/parser/tests/test_ranges.rs @@ -52,11 +52,11 @@ fn test_parser_formulas_with_full_ranges() { for formula in &formulas { let t = parser.parse( formula.formula_a1, - &Some(CellReferenceRC { + &CellReferenceRC { sheet: "Sheet1".to_string(), row: 1, column: 1, - }), + }, ); assert_eq!(to_rc_format(&t), formula.formula_r1c1); assert_eq!(to_string(&t, &cell_reference), formula.formula_a1); @@ -67,11 +67,11 @@ fn test_parser_formulas_with_full_ranges() { for formula in &formulas { let t = parser.parse( formula.formula_r1c1, - &Some(CellReferenceRC { + &CellReferenceRC { sheet: "Sheet1".to_string(), row: 1, column: 1, - }), + }, ); assert_eq!(to_rc_format(&t), formula.formula_r1c1); assert_eq!(to_string(&t, &cell_reference), formula.formula_a1); @@ -93,7 +93,7 @@ fn test_range_inverse_order() { // D4:C2 => C2:D4 let t = parser.parse( "SUM(D4:C2)*SUM(Sheet2!D4:C20)*SUM($C$20:D4)", - &Some(cell_reference.clone()), + &cell_reference, ); assert_eq!( to_string(&t, &cell_reference), diff --git a/base/src/expressions/parser/tests/test_stringify.rs b/base/src/expressions/parser/tests/test_stringify.rs index dbdd53b..36dd3dc 100644 --- a/base/src/expressions/parser/tests/test_stringify.rs +++ b/base/src/expressions/parser/tests/test_stringify.rs @@ -17,18 +17,18 @@ fn exp_order() { row: 1, column: 1, }; - let t = parser.parse("(1 + 2)^3 + 4", &Some(cell_reference.clone())); + let t = parser.parse("(1 + 2)^3 + 4", &cell_reference); assert_eq!(to_string(&t, &cell_reference), "(1+2)^3+4"); - let t = parser.parse("(C5 + 3)^R4", &Some(cell_reference.clone())); + let t = parser.parse("(C5 + 3)^R4", &cell_reference); assert_eq!(to_string(&t, &cell_reference), "(C5+3)^R4"); - let t = parser.parse("(C5 + 3)^(R4*6)", &Some(cell_reference.clone())); + let t = parser.parse("(C5 + 3)^(R4*6)", &cell_reference); assert_eq!(to_string(&t, &cell_reference), "(C5+3)^(R4*6)"); - let t = parser.parse("(C5)^(R4)", &Some(cell_reference.clone())); + let t = parser.parse("(C5)^(R4)", &cell_reference); assert_eq!(to_string(&t, &cell_reference), "C5^R4"); - let t = parser.parse("(5)^(4)", &Some(cell_reference.clone())); + let t = parser.parse("(5)^(4)", &cell_reference); assert_eq!(to_string(&t, &cell_reference), "5^4"); } diff --git a/base/src/expressions/parser/tests/test_tables.rs b/base/src/expressions/parser/tests/test_tables.rs index a31aaaf..3addf98 100644 --- a/base/src/expressions/parser/tests/test_tables.rs +++ b/base/src/expressions/parser/tests/test_tables.rs @@ -72,7 +72,7 @@ fn simple_table() { }; let formula = "SUM(tblIncome[[#This Row],[Jan]:[Dec]])"; - let t = parser.parse(formula, &Some(cell_reference.clone())); + let t = parser.parse(formula, &cell_reference); assert_eq!(to_string(&t, &cell_reference), "SUM($A$2:$E$2)"); // Cell A3 @@ -82,7 +82,7 @@ fn simple_table() { column: 1, }; let formula = "SUBTOTAL(109, tblIncome[Jan])"; - let t = parser.parse(formula, &Some(cell_reference.clone())); + let t = parser.parse(formula, &cell_reference); assert_eq!(to_string(&t, &cell_reference), "SUBTOTAL(109,$A$2:$A$3)"); // Cell A3 in 'Second Sheet' @@ -92,7 +92,7 @@ fn simple_table() { column: 1, }; let formula = "SUBTOTAL(109, tblIncome[Jan])"; - let t = parser.parse(formula, &Some(cell_reference.clone())); + let t = parser.parse(formula, &cell_reference); assert_eq!( to_string(&t, &cell_reference), "SUBTOTAL(109,'Sheet One'!$A$2:$A$3)" diff --git a/base/src/model.rs b/base/src/model.rs index 4bd36ec..8f228d1 100644 --- a/base/src/model.rs +++ b/base/src/model.rs @@ -1040,7 +1040,7 @@ impl Model { column: source.column, }; let formula_str = move_formula( - &self.parser.parse(formula, &Some(cell_reference)), + &self.parser.parse(formula, &cell_reference), &MoveContext { source_sheet_name: &source_sheet_name, row: source.row, @@ -1148,7 +1148,7 @@ impl Model { row: source.row, column: source.column, }; - let formula = &self.parser.parse(formula_str, &Some(cell_reference)); + let formula = &self.parser.parse(formula_str, &cell_reference); let cell_reference = CellReferenceRC { sheet: target_sheet_name, row: target.row, @@ -1524,13 +1524,11 @@ impl Model { column, }; let shared_formulas = &mut worksheet.shared_formulas; - let mut parsed_formula = self.parser.parse(formula, &Some(cell_reference.clone())); + let mut parsed_formula = self.parser.parse(formula, &cell_reference); // If the formula fails to parse try adding a parenthesis // SUM(A1:A3 => SUM(A1:A3) if let Node::ParseErrorKind { .. } = parsed_formula { - let new_parsed_formula = self - .parser - .parse(&format!("{})", formula), &Some(cell_reference)); + let new_parsed_formula = self.parser.parse(&format!("{})", formula), &cell_reference); match new_parsed_formula { Node::ParseErrorKind { .. } => {} _ => parsed_formula = new_parsed_formula, @@ -2140,14 +2138,14 @@ impl Model { self.parser.set_lexer_mode(LexerMode::R1C1); let worksheets = &mut self.workbook.worksheets; for worksheet in worksheets { - let cell_reference = &Some(CellReferenceRC { + let cell_reference = CellReferenceRC { sheet: worksheet.get_name(), row: 1, column: 1, - }); + }; let mut formulas = Vec::new(); for formula in &worksheet.shared_formulas { - let mut t = self.parser.parse(formula, cell_reference); + let mut t = self.parser.parse(formula, &cell_reference); rename_defined_name_in_node(&mut t, name, scope, new_name); formulas.push(to_rc_format(&t)); } diff --git a/base/src/new_empty.rs b/base/src/new_empty.rs index 454821b..5399e23 100644 --- a/base/src/new_empty.rs +++ b/base/src/new_empty.rs @@ -85,14 +85,14 @@ impl Model { let worksheets = &self.workbook.worksheets; for worksheet in worksheets { let shared_formulas = &worksheet.shared_formulas; - let cell_reference = &Some(CellReferenceRC { + let cell_reference = CellReferenceRC { sheet: worksheet.get_name(), row: 1, column: 1, - }); + }; let mut parse_formula = Vec::new(); for formula in shared_formulas { - let t = self.parser.parse(formula, cell_reference); + let t = self.parser.parse(formula, &cell_reference); parse_formula.push(t); } self.parsed_formulas.push(parse_formula); @@ -268,11 +268,11 @@ impl Model { // We use iter because the default would be a mut_iter and we don't need a mutable reference let worksheets = &mut self.workbook.worksheets; for worksheet in worksheets { - let cell_reference = &Some(CellReferenceRC { + let cell_reference = &CellReferenceRC { sheet: worksheet.get_name(), row: 1, column: 1, - }); + }; let mut formulas = Vec::new(); for formula in &worksheet.shared_formulas { let mut t = self.parser.parse(formula, cell_reference); diff --git a/xlsx/src/import/worksheets.rs b/xlsx/src/import/worksheets.rs index 836f233..1c450b5 100644 --- a/xlsx/src/import/worksheets.rs +++ b/xlsx/src/import/worksheets.rs @@ -309,7 +309,7 @@ fn from_a1_to_rc( let mut parser = Parser::new(worksheets.to_owned(), defined_names, tables); let cell_reference = parse_reference(&context).map_err(|error| XlsxError::Xml(error.to_string()))?; - let t = parser.parse(&formula, &Some(cell_reference)); + let t = parser.parse(&formula, &cell_reference); Ok(to_rc_format(&t)) }