From 6028505f8c49a56b54e04b02819c59f2642d2920 Mon Sep 17 00:00:00 2001
From: sleepycatcoding <131554884+sleepycatcoding@users.noreply.github.com>
Date: Fri, 15 Sep 2023 01:23:35 +0300
Subject: [PATCH] avm2: Small XML code cleanup

---
 core/src/avm2/e4x.rs               | 10 +++++-----
 core/src/avm2/e4x/is_xml_name.rs   | 22 +++++++---------------
 core/src/avm2/globals/toplevel.rs  |  8 +++++++-
 core/src/avm2/globals/xml.rs       |  2 +-
 core/src/avm2/object/xml_object.rs |  7 ++++---
 5 files changed, 24 insertions(+), 25 deletions(-)

diff --git a/core/src/avm2/e4x.rs b/core/src/avm2/e4x.rs
index 95b1e5a1bd61..907b90e9e65c 100644
--- a/core/src/avm2/e4x.rs
+++ b/core/src/avm2/e4x.rs
@@ -246,11 +246,11 @@ impl<'gc> E4XNode<'gc> {
 
     /// Returns the amount of children in this node if this node is of Element kind, otherwise returns [None].
     pub fn length(&self) -> Option<usize> {
-        let E4XNodeKind::Element { children, .. } = &*self.kind() else {
-            return None;
-        };
-
-        Some(children.len())
+        if let E4XNodeKind::Element { children, .. } = &*self.kind() {
+            Some(children.len())
+        } else {
+            None
+        }
     }
 
     /// Removes all matching children matching provided name, returns the first child removed along with its index (if any).
diff --git a/core/src/avm2/e4x/is_xml_name.rs b/core/src/avm2/e4x/is_xml_name.rs
index 25d8f33a9246..a3c12acb81c8 100644
--- a/core/src/avm2/e4x/is_xml_name.rs
+++ b/core/src/avm2/e4x/is_xml_name.rs
@@ -1,7 +1,7 @@
 // NOTE: Regex to match character groups: '\[#(.{5})-#(.{5})\]', substitution '(0$1, 0$2),\n'
 //       and regex to match single characters: '#(.{5})', substitution '(0$1, 0$1), // single\n'
 
-use crate::avm2::{Activation, Error, Value};
+use crate::string::AvmString;
 
 // https://www.w3.org/TR/2004/REC-xml-20040204/#NT-Letter
 static LETTER_TABLE: &[(u32, u32)] = &[
@@ -411,30 +411,22 @@ fn is_extender(c: char) -> bool {
 }
 
 // Based on https://github.com/adobe/avmplus/blob/858d034a3bd3a54d9b70909386435cf4aec81d21/core/AvmCore.cpp#L3478
-pub fn is_xml_name<'gc>(
-    activation: &mut Activation<'_, 'gc>,
-    name: Value<'gc>,
-) -> Result<bool, Error<'gc>> {
-    if matches!(name, Value::Undefined | Value::Null) {
-        return Ok(false);
-    }
-
-    let name = name.coerce_to_string(activation)?;
+pub fn is_xml_name(name: AvmString<'_>) -> bool {
     if name.is_empty() {
-        return Ok(false);
+        return false;
     }
 
     let mut name = name.chars();
     let first_char = match name.next().expect("At least one character should exist") {
         Ok(c) => c,
-        Err(_) => return Ok(false),
+        Err(_) => return false,
     };
 
     if !is_letter(first_char) && first_char != '_' {
-        return Ok(false);
+        return false;
     }
 
-    Ok(name.map(|x| x.ok()).all(|x| {
+    name.map(|x| x.ok()).all(|x| {
         if let Some(x) = x {
             is_digit(x)
                 || is_letter(x)
@@ -446,7 +438,7 @@ pub fn is_xml_name<'gc>(
         } else {
             false
         }
-    }))
+    })
 }
 
 #[cfg(test)]
diff --git a/core/src/avm2/globals/toplevel.rs b/core/src/avm2/globals/toplevel.rs
index bcd06c70c892..0ef90049a2a9 100644
--- a/core/src/avm2/globals/toplevel.rs
+++ b/core/src/avm2/globals/toplevel.rs
@@ -241,7 +241,13 @@ pub fn is_xml_name<'gc>(
     args: &[Value<'gc>],
 ) -> Result<Value<'gc>, Error<'gc>> {
     let name = args.get(0).unwrap_or(&Value::Undefined);
-    Ok(crate::avm2::e4x::is_xml_name(activation, *name)?.into())
+    if matches!(name, Value::Undefined | Value::Null) {
+        return Ok(false.into());
+    }
+
+    let name = name.coerce_to_string(activation)?;
+
+    Ok(crate::avm2::e4x::is_xml_name(name).into())
 }
 
 pub fn escape<'gc>(
diff --git a/core/src/avm2/globals/xml.rs b/core/src/avm2/globals/xml.rs
index d067a321478f..e834325f832c 100644
--- a/core/src/avm2/globals/xml.rs
+++ b/core/src/avm2/globals/xml.rs
@@ -114,7 +114,7 @@ pub fn set_name<'gc>(
         new_name.coerce_to_string(activation)?
     };
 
-    let is_name_valid = crate::avm2::e4x::is_xml_name(activation, new_name.into())?;
+    let is_name_valid = crate::avm2::e4x::is_xml_name(new_name);
     if !is_name_valid {
         return Err(Error::AvmError(type_error(
             activation,
diff --git a/core/src/avm2/object/xml_object.rs b/core/src/avm2/object/xml_object.rs
index e39ace370ed7..95398f15cd6e 100644
--- a/core/src/avm2/object/xml_object.rs
+++ b/core/src/avm2/object/xml_object.rs
@@ -361,9 +361,10 @@ impl<'gc> TObject<'gc> for XmlObject<'gc> {
         }
 
         // 7. Let isValidName be the result of calling the function isXMLName (section 13.1.2.1) with argument n
-        let is_valid_name = name.local_name().map_or(Ok(false), |x| {
-            crate::avm2::e4x::is_xml_name(activation, x.into())
-        })?;
+        let is_valid_name = name
+            .local_name()
+            .map(crate::avm2::e4x::is_xml_name)
+            .unwrap_or(false);
         // 8. If isValidName is false and n.localName is not equal to the string "*", return
         if !is_valid_name && !name.is_any_name() {
             return Ok(());