Optimize string handling in header-translator a bit

Author: madsmtm

Cargo.lock

@@ -22,7 +22,6 @@ tracing = { version = "0.1.37", default-features = false, features = ["std"] }
 tracing-subscriber = { version = "0.3.16", features = ["fmt", "env-filter"] }
 tracing-tree = "0.4.1"
 proc-macro2 = "1.0.66"
-syn = { version = "2.0", features = ["parsing"] }
 heck = "0.5"
 semver = { version = "1.0", features = ["serde"] }
 lenient_semver_parser = "0.4"

crates/header-translator/Cargo.toml

@@ -487,11 +487,11 @@ impl LibraryConfig {
         let allowed_in = self.class_data.values();
         for data in all.clone().filter(filter_ptr(allowed_in)) {
             assert_eq!(data.derives, Default::default());
-            assert_eq!(data.definition_skipped, Default::default());
+            assert_eq!(data.definition_skipped, bool::default());
             assert_eq!(data.categories, Default::default());
             assert_eq!(data.counterpart, Default::default());
             assert_eq!(data.skipped_protocols, Default::default());
-            assert_eq!(data.main_thread_only, Default::default());
+            assert_eq!(data.main_thread_only, bool::default());
             assert_eq!(data.bridged_to, Default::default());
         }
 
@@ -507,7 +507,7 @@ impl LibraryConfig {
 
         let allowed_in = self.fns.values();
         for data in all.clone().filter(filter_ptr(allowed_in)) {
-            assert_eq!(data.no_implementor, Default::default());
+            assert_eq!(data.no_implementor, bool::default());
             assert_eq!(data.implementor, Default::default());
             assert_eq!(data.arguments, Default::default());
             assert_eq!(data.return_, Default::default());

crates/header-translator/src/config.rs

@@ -95,14 +95,5 @@ fn immediate_children<'tu>(
     });
 }
 
-pub(crate) fn to_snake_case(input: impl AsRef<str>) -> String {
-    let input = input.as_ref();
-    if input == "_" {
-        String::from("_")
-    } else {
-        heck::ToSnakeCase::to_snake_case(input)
-    }
-}
-
 /// The version of the framework crates.
 pub const VERSION: &str = "0.3.2";

crates/header-translator/src/lib.rs

@@ -10,7 +10,9 @@ use crate::context::Context;
 use crate::display_helper::FormatterFn;
 use crate::documentation::Documentation;
 use crate::id::{cfg_gate_ln, ItemTree};
-use crate::name_translation::{self, is_likely_bounds_affecting};
+use crate::name_translation::{
+    self, handle_keyword, is_likely_bounds_affecting, param_name, to_snake_case,
+};
 use crate::objc2_utils::in_selector_family;
 use crate::rust_type::{MethodArgumentQualifier, SafetyProperty, Ty};
 use crate::stmt::parse_param_children;
@@ -636,7 +638,7 @@ impl Method {
                         "`{arg_name}` might not be bounds-checked"
                     )));
                 }
-                safety.merge(arg_ty.safety_in_method_argument(&crate::to_snake_case(arg_name)))
+                safety.merge(arg_ty.safety_in_method_argument(&to_snake_case(arg_name)))
             })
             .merge(result_type.safety_in_fn_return());
 
@@ -907,7 +909,7 @@ impl Method {
                     is_class,
                 );
 
-                let mut safety = ty.safety_in_fn_argument(&crate::to_snake_case(&name));
+                let mut safety = ty.safety_in_fn_argument(&to_snake_case(&name));
                 if kind == PropertyKind::UnsafeRetained && !ty.is_primitive_or_record() {
                     // We could _possibly_ allow these to be safe, but let's
                     // not for now, they interact weirdly with the getters
@@ -1245,7 +1247,7 @@ impl Method {
             if !self.safe {
                 write!(f, "unsafe ")?;
             }
-            write!(f, "fn {}(", handle_reserved(&self.fn_name))?;
+            write!(f, "fn {}(", handle_keyword(&self.fn_name))?;
 
             // Receiver
             if let MemoryManagement::RetainedInit = self.memory_management {
@@ -1258,7 +1260,7 @@ impl Method {
 
             // Arguments
             for (param, arg_ty) in arguments {
-                let param = handle_reserved(&crate::to_snake_case(param));
+                let param = param_name(param);
                 write!(f, "{param}: {}, ", arg_ty.method_argument())?;
             }
             if self.requires_main_thread_marker {
@@ -1286,28 +1288,3 @@ impl Method {
         })
     }
 }
-
-pub(crate) fn handle_reserved(name: &str) -> String {
-    // try to parse name as an identifier
-    if let Ok(ident) = syn::parse_str::<syn::Ident>(name) {
-        ident.to_string()
-    }
-    // try to parse as a raw identifier (NOTE: does not handle `self` or `super`)
-    else if let Ok(ident) = syn::parse_str::<syn::Ident>(&format!("r#{name}")) {
-        ident.to_string()
-    }
-    // translate whatever remains unchanged (needed for, e.g., `_`)
-    else if name == "self" {
-        "self_".into()
-    } else if name == "Self" {
-        "r#Self".into()
-    } else if name == "super" {
-        "super_".into()
-    } else if name == "_" {
-        // Hack: Assumes that there are not other fields / parameters with the
-        // name `_`.
-        "param1".into()
-    } else {
-        name.into()
-    }
-}

crates/header-translator/src/method.rs

@@ -5,7 +5,7 @@
 //! Kinda ugly and under-tested, may not work for all cases.
 #![allow(clippy::if_same_then_else)]
 
-use std::{collections::VecDeque, iter::FusedIterator};
+use std::{borrow::Cow, collections::VecDeque, iter::FusedIterator};
 
 use itertools::Itertools;
 
@@ -526,6 +526,108 @@ pub(crate) fn is_completion_handler_param_name(s: &str) -> bool {
     )
 }
 
+pub(crate) fn to_snake_case(mut input: &str) -> Cow<'_, str> {
+    if !input.contains(|c: char| c.is_ascii_uppercase()) {
+        // No uppercase chars -> assume already snake case
+        Cow::Borrowed(input)
+    } else {
+        // Preserve leading underscores.
+        let mut leading_underscores = 0;
+        while let Some(rest) = input.strip_prefix('_') {
+            leading_underscores += 1;
+            input = rest;
+        }
+        let mut result = heck::ToSnakeCase::to_snake_case(input);
+        for _ in 0..leading_underscores {
+            result.insert(0, '_');
+        }
+        Cow::Owned(result)
+    }
+}
+
+pub(crate) fn param_name(param: &str) -> Cow<'_, str> {
+    let param = to_snake_case(param);
+    if let Some(param) = keyword(&param) {
+        Cow::Borrowed(param)
+    } else {
+        param
+    }
+}
+
+/// Check if a given name is a reserved keyword.
+fn keyword(name: &str) -> Option<&'static str> {
+    // <https://doc.rust-lang.org/reference/keywords.html>
+    Some(match name {
+        // Strict keywords.
+        "_" => "param1", // HACK: Assumes that there are not other fields / parameters with the name `_`.
+        "as" => "r#as",
+        "async" => "r#async",
+        "await" => "r#await",
+        "break" => "r#break",
+        "const" => "r#const",
+        "continue" => "r#continue",
+        "crate" => "crate_", // Cannot be a raw identifier
+        "dyn" => "r#dyn",
+        "else" => "r#else",
+        "enum" => "r#enum",
+        "extern" => "r#extern",
+        "false" => "r#false",
+        "fn" => "r#fn",
+        "for" => "r#for",
+        "if" => "r#if",
+        "impl" => "r#impl",
+        "in" => "r#in",
+        "let" => "r#let",
+        "loop" => "r#loop",
+        "match" => "r#match",
+        "mod" => "r#mod",
+        "move" => "r#move",
+        "mut" => "r#mut",
+        "pub" => "r#pub",
+        "ref" => "r#ref",
+        "return" => "r#return",
+        "self" => "self_", // Cannot be a raw identifier
+        "Self" => "Self_", // Cannot be a raw identifier
+        "static" => "r#static",
+        "struct" => "r#struct",
+        "super" => "super_", // Cannot be a raw identifier
+        "trait" => "r#trait",
+        "true" => "r#true",
+        "type" => "r#type",
+        "unsafe" => "r#unsafe",
+        "use" => "r#use",
+        "where" => "r#where",
+        "while" => "r#while",
+
+        // Reserved keywords.
+        "abstract" => "r#abstract",
+        "become" => "r#become",
+        "box" => "r#box",
+        "do" => "r#do",
+        "final" => "r#final",
+        "gen" => "r#gen",
+        "macro" => "r#macro",
+        "override" => "r#override",
+        "priv" => "r#priv",
+        "try" => "r#try",
+        "typeof" => "r#typeof",
+        "unsized" => "r#unsized",
+        "virtual" => "r#virtual",
+        "yield" => "r#yield",
+
+        // Weak keywords. Not necessary to r#-prefix these.
+        // "macro_rules" => "r#macro_rules",
+        // "raw" => "r#raw",
+        // "safe" => "r#safe",
+        // "union" => "r#union",
+        _ => return None,
+    })
+}
+
+pub(crate) fn handle_keyword(name: &str) -> &str {
+    keyword(name).unwrap_or(name)
+}
+
 #[cfg(test)]
 mod tests {
     use super::*;
@@ -772,4 +874,13 @@ mod tests {
             "signData:withSecureElementPass:completion:"
         )));
     }
+
+    #[test]
+    fn snake() {
+        assert_eq!(to_snake_case("FooBar"), "foo_bar");
+        assert_eq!(to_snake_case("foo_bar"), "foo_bar");
+        assert_eq!(to_snake_case("_"), "_");
+        assert_eq!(to_snake_case("___Foo"), "___foo");
+        assert_eq!(to_snake_case("fooBar"), "foo_bar");
+    }
 }

crates/header-translator/src/name_translation.rs

@@ -22,9 +22,11 @@ use crate::id::ItemIdentifier;
 use crate::id::ItemTree;
 use crate::id::Location;
 use crate::immediate_children;
-use crate::method::{handle_reserved, Method};
-use crate::name_translation::is_likely_bounds_affecting;
-use crate::name_translation::{enum_prefix, split_words};
+use crate::method::Method;
+use crate::name_translation::handle_keyword;
+use crate::name_translation::{
+    enum_prefix, is_likely_bounds_affecting, param_name, split_words, to_snake_case,
+};
 use crate::protocol::parse_direct_protocols;
 use crate::protocol::ProtocolRef;
 use crate::rust_type::PointeeTy;
@@ -2057,7 +2059,7 @@ impl Stmt {
                                 "`{arg_name}` might not be bounds-checked"
                             )));
                         }
-                        safety.merge(arg_ty.safety_in_fn_argument(&crate::to_snake_case(arg_name)))
+                        safety.merge(arg_ty.safety_in_fn_argument(&to_snake_case(arg_name)))
                     })
                     .merge(result_type.safety_in_fn_return());
 
@@ -3068,7 +3070,7 @@ impl Stmt {
                         } else {
                             write!(f, "pub ")?;
                         }
-                        let name = handle_reserved(name);
+                        let name = handle_keyword(name);
                         writeln!(f, "{name}: {},", ty.record())?;
                     }
                     writeln!(f, "}}")?;
@@ -3361,7 +3363,7 @@ impl Stmt {
                     write!(f, "// TODO: ")?;
                     write!(f, "pub fn {c_name}(")?;
                     for (param, arg_ty) in arguments {
-                        let param = handle_reserved(&crate::to_snake_case(param));
+                        let param = param_name(param);
                         write!(f, "{param}: {},", arg_ty.fn_argument_converted())?;
                     }
                     writeln!(
@@ -3408,7 +3410,7 @@ impl Stmt {
                     }
                     writeln!(f, "#[inline]")?;
                     let unsafe_ = if *safe { "" } else { "unsafe " };
-                    let fn_name = handle_reserved(&id.name);
+                    let fn_name = handle_keyword(&id.name);
                     // Wrappers have normal Rust ABI to unclutter docs.
                     write!(f, "{vis} {unsafe_}fn {fn_name}(")?;
 
@@ -3433,7 +3435,7 @@ impl Stmt {
                             continue;
                         }
 
-                        let param = handle_reserved(&crate::to_snake_case(param));
+                        let param = param_name(param);
                         write!(f, "{param}: {},", arg_ty.fn_argument_converted())?;
                     }
 
@@ -3467,7 +3469,7 @@ impl Stmt {
 
                     write!(f, "fn {c_name}(")?;
                     for (param, arg_ty) in arguments {
-                        let param = handle_reserved(&crate::to_snake_case(param));
+                        let param = param_name(param);
                         write!(f, "{param}: {},", arg_ty.fn_argument_unconverted())?;
                     }
                     writeln!(
@@ -3480,7 +3482,7 @@ impl Stmt {
 
                     // Perform argument conversion.
                     for (i, (param, ty)) in arguments.iter().enumerate() {
-                        let param = handle_reserved(&crate::to_snake_case(param));
+                        let param = param_name(param);
                         let param_or_self = if arg_is_self.unwrap_or(usize::MAX) == i {
                             "self"
                         } else {
@@ -3493,7 +3495,7 @@ impl Stmt {
                     let fn_call = FormatterFn(|f| {
                         write!(f, "unsafe {{ {c_name}(")?;
                         for (i, (param, ty)) in arguments.iter().enumerate() {
-                            let param = handle_reserved(&crate::to_snake_case(param));
+                            let param = param_name(param);
                             if arg_is_self.unwrap_or(usize::MAX) == i {
                                 let converter = ty.fn_argument_converter("self", &param);
                                 if converter.to_string().is_empty() {