avm2: Small XML code cleanup

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).

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)]

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>(

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,

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(());