sirtimbly/IronCalc
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

UPDATE: Adds name validation and exposes it in wasm

Browse Source 
We do a trick I am not proud of. Because all of our errors are Strings,
we don't have a way to separate a name error from an index error,
for instance. What I do in prepend the error with a string that indicates
where it comes from.
This commit is contained in:
Nicolás Hatcher
2025-11-17 19:57:18 +01:00
committed by Nicolás Hatcher Andrés
parent 3db094c956
commit 1391f196b5
7 changed files with 108 additions and 49 deletions
Download Patch File Download Diff File Expand all files Collapse all files

30
base/src/expressions/utils/mod.rs
View File

@@ -211,15 +211,19 @@ pub fn parse_reference_a1(r: &str) -> Option<ParsedReference> {
pub fn is_valid_identifier(name: &str) -> bool {
// https://support.microsoft.com/en-us/office/names-in-formulas-fc2935f9-115d-4bef-a370-3aa8bb4c91f1
// https://github.com/MartinTrummer/excel-names/
// NOTE: We are being much more restrictive than Excel.
// In particular we do not support non ascii characters.
let upper = name.to_ascii_uppercase();
let bytes = upper.as_bytes();
let len = bytes.len();
// length of chars
let len = upper.chars().count();
let mut chars = upper.chars();
if len > 255 || len == 0 {
return false;
}
let first = bytes[0] as char;
let first = match chars.next() {
Some(ch) => ch,
None => return false,
};
// The first character of a name must be a letter, an underscore character (_), or a backslash (\).
if !(first.is_ascii_alphabetic() || first == '_' || first == '\\') {
return false;
@@ -237,20 +241,10 @@ pub fn is_valid_identifier(name: &str) -> bool {
if parse_reference_r1c1(name).is_some() {
return false;
}
let mut i = 1;
while i < len {
let ch = bytes[i] as char;
match ch {
'a'..='z' => {}
'A'..='Z' => {}
'0'..='9' => {}
'_' => {}
'.' => {}
_ => {
return false;
}
for ch in chars {
if !(ch.is_alphanumeric() || ch == '_' || ch == '.') {
return false;
}
i += 1;
}
true

2
base/src/expressions/utils/test.rs
View File

@@ -196,6 +196,7 @@ fn test_names() {
assert!(is_valid_identifier("_."));
assert!(is_valid_identifier("_1"));
assert!(is_valid_identifier("\\."));
assert!(is_valid_identifier("truñe"));
// invalid
assert!(!is_valid_identifier("true"));
@@ -209,7 +210,6 @@ fn test_names() {
assert!(!is_valid_identifier("1true"));
assert!(!is_valid_identifier("test€"));
assert!(!is_valid_identifier("truñe"));
assert!(!is_valid_identifier("tr&ue"));
assert!(!is_valid_identifier("LOG10"));

76
base/src/model.rs
View File

@@ -2068,21 +2068,7 @@ impl Model {
scope: Option<u32>,
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 {
Some(index) => Some(self.workbook.worksheet(index)?.sheet_id),
None => None,
};
// if the defined name already exist return error
for df in defined_names {
if df.name.to_uppercase() == name_upper && df.sheet_id == sheet_id {
return Err("Defined name already exists".to_string());
}
}
let sheet_id = self.is_valid_defined_name(name, scope, formula)?;
self.workbook.defined_names.push(DefinedName {
name: name.to_string(),
formula: formula.to_string(),
@@ -2093,6 +2079,48 @@ impl Model {
Ok(())
}
/// Validates if a defined name can be created
pub fn is_valid_defined_name(
&self,
name: &str,
scope: Option<u32>,
formula: &str,
) -> Result<Option<u32>, String> {
if !is_valid_identifier(name) {
return Err("Name: Invalid defined name".to_string());
}
let name_upper = name.to_uppercase();
let defined_names = &self.workbook.defined_names;
let sheet_id = match scope {
Some(index) => match self.workbook.worksheet(index) {
Ok(ws) => Some(ws.sheet_id),
Err(_) => return Err("Scope: Invalid sheet index".to_string()),
},
None => None,
};
// if the defined name already exist return error
for df in defined_names {
if df.name.to_uppercase() == name_upper && df.sheet_id == sheet_id {
return Err("Name: Defined name already exists".to_string());
}
}
// Make sure the formula is valid
match common::ParsedReference::parse_reference_formula(
None,
formula,
&self.locale,
|name| self.get_sheet_index_by_name(name),
) {
Ok(_) => {}
Err(_) => {
return Err("Formula: Invalid defined name formula".to_string());
}
};
Ok(sheet_id)
}
/// Delete defined name of name and scope
pub fn delete_defined_name(&mut self, name: &str, scope: Option<u32>) -> Result<(), String> {
let name_upper = name.to_uppercase();
@@ -2126,7 +2154,7 @@ impl Model {
new_formula: &str,
) -> Result<(), String> {
if !is_valid_identifier(new_name) {
return Err("Invalid defined name".to_string());
return Err("Name: Invalid defined name".to_string());
};
let name_upper = name.to_uppercase();
let new_name_upper = new_name.to_uppercase();
@@ -2134,18 +2162,28 @@ impl Model {
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());
return Err("Name: 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),
Some(index) => Some(
self.workbook
.worksheet(index)
.map_err(|_| "Scope: Invalid sheet index")?
.sheet_id,
),
None => None,
};
let new_sheet_id = match new_scope {
Some(index) => Some(self.workbook.worksheet(index)?.sheet_id),
Some(index) => Some(
self.workbook
.worksheet(index)
.map_err(|_| "Scope: Invalid sheet index")?
.sheet_id,
),
None => None,
};

18
base/src/test/user_model/test_defined_names.rs
View File

@@ -254,19 +254,19 @@ fn invalid_names() {
// spaces
assert_eq!(
model.new_defined_name("A real", None, "Sheet1!$A$1"),
Err("Invalid defined name".to_string())
Err("Name: 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())
Err("Name: 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())
Err("Name: Invalid defined name".to_string())
);
}
@@ -284,13 +284,13 @@ fn already_existing() {
// 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())
Err("Name: 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())
Err("Name: Defined name already exists".to_string())
);
}
@@ -304,17 +304,17 @@ fn invalid_sheet() {
assert_eq!(
model.new_defined_name("Mything", Some(2), "Sheet1!$A$1"),
Err("Invalid sheet index".to_string())
Err("Scope: 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())
Err("Scope: 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())
Err("General: Failed to get old name".to_string())
);
}
@@ -322,7 +322,7 @@ fn invalid_sheet() {
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();
assert!(model.new_defined_name("MyName", None, "A1").is_err());
model.set_user_input(0, 1, 2, "=MyName").unwrap();

15
base/src/user_model/common.rs
View File

@@ -2001,7 +2001,10 @@ impl UserModel {
new_scope: Option<u32>,
new_formula: &str,
) -> Result<(), String> {
let old_formula = self.model.get_defined_name_formula(name, scope)?;
let old_formula = self
.model
.get_defined_name_formula(name, scope)
.map_err(|_| "General: Failed to get old name")?;
let diff_list = vec![Diff::UpdateDefinedName {
name: name.to_string(),
scope,
@@ -2017,6 +2020,16 @@ impl UserModel {
Ok(())
}
/// validates a new defined name
pub fn is_valid_defined_name(
&self,
name: &str,
scope: Option<u32>,
formula: &str,
) -> Result<Option<u32>, String> {
self.model.is_valid_defined_name(name, scope, formula)
}
// **** Private methods ****** //
pub(crate) fn push_diff_list(&mut self, diff_list: DiffList) {

13
bindings/wasm/src/lib.rs
View File

@@ -775,4 +775,17 @@ impl Model {
.get_first_non_empty_in_row_after_column(sheet, row, column)
.map_err(to_js_error)
}
#[wasm_bindgen(js_name = "isValidDefinedName")]
pub fn is_valid_defined_name(
&self,
name: &str,
scope: Option<u32>,
formula: &str,
) -> Result<(), JsError> {
match self.model.is_valid_defined_name(name, scope, formula) {
Ok(_) => Ok(()),
Err(e) => Err(to_js_error(e.to_string())),
}
}
}

3
webapp/IronCalc/src/components/RightDrawer/NamedRanges/EditNamedRange.tsx
View File

@@ -83,7 +83,7 @@ function EditNamedRange({
} else {
// Check for duplicates only if format is valid
const scopeIndex = worksheets.findIndex((s) => s.name === scope);
const newScope = scopeIndex >= 0 ? scopeIndex : null;
const newScope = scopeIndex >= 0 ? scopeIndex : undefined;
const existing = definedNameList.find(
(dn) =>
dn.name === trimmed &&
@@ -99,6 +99,7 @@ function EditNamedRange({
}
setNameError(error);
setFormulaError("");
}, [name, scope, definedNameList, editingDefinedName, worksheets]);
const hasAnyError = nameError !== "" || formulaError !== "";