From 2ed5fb9bbc0bb7128d9eff25ad764d8219decb09 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nicol=C3=A1s=20Hatcher?= Date: Wed, 25 Dec 2024 20:00:56 +0100 Subject: [PATCH] FIX: Adds some validation and tests --- base/src/model.rs | 17 +- .../src/test/user_model/test_defined_names.rs | 159 ++++++++++++++++++ 2 files changed, 175 insertions(+), 1 deletion(-) diff --git a/base/src/model.rs b/base/src/model.rs index 3152a82..a334ebb 100644 --- a/base/src/model.rs +++ b/base/src/model.rs @@ -16,7 +16,7 @@ use crate::{ }, token::{get_error_by_name, Error, OpCompare, OpProduct, OpSum, OpUnary}, types::*, - utils::{self, is_valid_column_number, is_valid_row}, + utils::{self, is_valid_column_number, is_valid_identifier, is_valid_row}, }, formatter::{ format::{format_number, parse_formatted_number}, @@ -2039,6 +2039,9 @@ impl Model { scope: Option, formula: &str, ) -> Result<(), String> { + if !is_valid_identifier(name) { + return Err("Invalid defined name".to_string()); + }; let name_upper = name.to_uppercase(); let defined_names = &self.workbook.defined_names; let sheet_id = match scope { @@ -2093,7 +2096,19 @@ impl Model { new_scope: Option, new_formula: &str, ) -> Result<(), String> { + if !is_valid_identifier(new_name) { + return Err("Invalid defined name".to_string()); + }; let name_upper = name.to_uppercase(); + let new_name_upper = new_name.to_uppercase(); + + if name_upper != new_name_upper || scope != new_scope { + for key in self.parsed_defined_names.keys() { + if key.1.to_uppercase() == new_name_upper && key.0 == new_scope { + return Err("Defined name already exists".to_string()); + } + } + } let defined_names = &self.workbook.defined_names; let sheet_id = match scope { Some(index) => Some(self.workbook.worksheet(index)?.sheet_id), diff --git a/base/src/test/user_model/test_defined_names.rs b/base/src/test/user_model/test_defined_names.rs index 5b0dadb..88700e7 100644 --- a/base/src/test/user_model/test_defined_names.rs +++ b/base/src/test/user_model/test_defined_names.rs @@ -165,3 +165,162 @@ fn rename_defined_name() { Ok(r#"=CONCATENATE(newName," world!")"#.to_string()) ); } + +#[test] +fn rename_defined_name_operations() { + let mut model = UserModel::new_empty("model", "en", "UTC").unwrap(); + model.set_user_input(0, 1, 1, "42").unwrap(); + model.set_user_input(0, 1, 2, "123").unwrap(); + + model + .new_defined_name("answer", None, "Sheet1!$A$1") + .unwrap(); + + model + .set_user_input(0, 2, 1, "=IF(answer<2, answer*2, answer^2)") + .unwrap(); + + model + .set_user_input(0, 3, 1, "=badDunction(-answer)") + .unwrap(); + + model.new_sheet().unwrap(); + model.set_user_input(1, 1, 1, "78").unwrap(); + model + .new_defined_name("answer", Some(1), "Sheet1!$A$1") + .unwrap(); + + model.set_user_input(1, 3, 1, "=answer").unwrap(); + + model + .update_defined_name("answer", None, "respuesta", None, "Sheet1!$A$1") + .unwrap(); + + assert_eq!( + model.get_cell_content(0, 2, 1), + Ok("=IF(respuesta<2,respuesta*2,respuesta^2)".to_string()) + ); + + assert_eq!( + model.get_cell_content(0, 3, 1), + Ok("=badDunction(-respuesta)".to_string()) + ); + + // A defined name with the same name but different scope + assert_eq!(model.get_cell_content(1, 3, 1), Ok("=answer".to_string())); +} + +#[test] +fn rename_defined_name_string_operations() { + let mut model = UserModel::new_empty("model", "en", "UTC").unwrap(); + model.set_user_input(0, 1, 1, "Hello").unwrap(); + model.set_user_input(0, 1, 2, "World").unwrap(); + + model + .new_defined_name("hello", None, "Sheet1!$A$1") + .unwrap(); + + model + .new_defined_name("world", None, "Sheet1!$B$1") + .unwrap(); + + model.set_user_input(0, 2, 1, "=hello&world").unwrap(); + + model + .update_defined_name("hello", None, "HolaS", None, "Sheet1!$A$1") + .unwrap(); + + assert_eq!( + model.get_cell_content(0, 2, 1), + Ok("=HolaS&world".to_string()) + ); +} + +#[test] +fn invalid_names() { + let mut model = UserModel::new_empty("model", "en", "UTC").unwrap(); + model.set_user_input(0, 1, 1, "Hello").unwrap(); + model + .new_defined_name("MyName", None, "Sheet1!$A$1") + .unwrap(); + + // spaces + assert_eq!( + model.new_defined_name("A real", None, "Sheet1!$A$1"), + Err("Invalid defined name".to_string()) + ); + + // Starts with number + assert_eq!( + model.new_defined_name("2real", None, "Sheet1!$A$1"), + Err("Invalid defined name".to_string()) + ); + + // Updating also fails + assert_eq!( + model.update_defined_name("MyName", None, "My Name", None, "Sheet1!$A$1"), + Err("Invalid defined name".to_string()) + ); +} + +#[test] +fn already_existing() { + let mut model = UserModel::new_empty("model", "en", "UTC").unwrap(); + + model + .new_defined_name("MyName", None, "Sheet1!$A$1") + .unwrap(); + model + .new_defined_name("Another", None, "Sheet1!$A$1") + .unwrap(); + + // Can't create a new name with the same name + assert_eq!( + model.new_defined_name("MyName", None, "Sheet1!$A$2"), + Err("Defined name already exists".to_string()) + ); + + // Can't update one into an existing + assert_eq!( + model.update_defined_name("Another", None, "MyName", None, "Sheet1!$A$1"), + Err("Defined name already exists".to_string()) + ); +} + +#[test] +fn invalid_sheet() { + let mut model = UserModel::new_empty("model", "en", "UTC").unwrap(); + model.set_user_input(0, 1, 1, "Hello").unwrap(); + model + .new_defined_name("MyName", None, "Sheet1!$A$1") + .unwrap(); + + assert_eq!( + model.new_defined_name("Mything", Some(2), "Sheet1!$A$1"), + Err("Invalid sheet index".to_string()) + ); + + assert_eq!( + model.update_defined_name("MyName", None, "MyName", Some(2), "Sheet1!$A$1"), + Err("Invalid sheet index".to_string()) + ); + + assert_eq!( + model.update_defined_name("MyName", Some(9), "YourName", None, "Sheet1!$A$1"), + Err("Invalid sheet index".to_string()) + ); +} + +#[test] +fn invalid_formula() { + let mut model = UserModel::new_empty("model", "en", "UTC").unwrap(); + model.set_user_input(0, 1, 1, "Hello").unwrap(); + model.new_defined_name("MyName", None, "A1").unwrap(); + + model.set_user_input(0, 1, 2, "=MyName").unwrap(); + + assert_eq!( + model.get_formatted_cell_value(0, 1, 2), + Ok("#NAME?".to_string()) + ); +}