Merge pull request #552 from google/underscores_in_inner_types

Underscores in inner types
google · Jun 2, 2021 · 2aa8a22 · 2aa8a22
2 parents 43d523c + a069fdc
commit 2aa8a22
Show file tree

Hide file tree

Showing 2 changed files with 32 additions and 10 deletions.
diff --git a/engine/src/conversion/analysis/name_check.rs b/engine/src/conversion/analysis/name_check.rs
@@ -22,7 +22,7 @@ use crate::{
         error_reporter::convert_item_apis,
         ConvertError,
     },
-    types::{validate_ident_ok_for_rust, QualifiedName},
+    types::{validate_ident_ok_for_cxx, QualifiedName},
 };
 
 use super::fun::FnAnalysis;
@@ -37,15 +37,11 @@ pub(crate) fn check_names(apis: Vec<Api<FnAnalysis>>) -> Vec<Api<FnAnalysis>> {
         | ApiDetail::Const { .. }
         | ApiDetail::Enum { .. }
         | ApiDetail::Struct { .. } => {
-            let cxx_name = api
-                .cpp_name
-                .as_ref()
-                .map(|s|
-                    QualifiedName::new_from_cpp_name(&s)
-                )
-                .unwrap_or_else(|| api.name.clone());
-            for seg in cxx_name.segment_iter() {
-                validate_ident_ok_for_rust(&seg)?;
+            validate_all_segments_ok_for_cxx(api.name.segment_iter())?;
+            if let Some(ref cpp_name) = api.cpp_name {
+                // The C++ name might itself be outer_type::inner_type and thus may
+                // have multiple segments.
+                validate_all_segments_ok_for_cxx(QualifiedName::new_from_cpp_name(cpp_name).segment_iter())?;
             }
             Ok(Some(api))
         }
@@ -85,6 +81,15 @@ pub(crate) fn check_names(apis: Vec<Api<FnAnalysis>>) -> Vec<Api<FnAnalysis>> {
     results
 }
 
+fn validate_all_segments_ok_for_cxx(
+    items: impl Iterator<Item = String>,
+) -> Result<(), ConvertError> {
+    for seg in items {
+        validate_ident_ok_for_cxx(&seg)?;
+    }
+    Ok(())
+}
+
 fn cxxbridge_name(api: &Api<FnAnalysis>) -> Option<Ident> {
     match api.detail {
         ApiDetail::Function { ref analysis, .. } => Some(analysis.cxxbridge_name.clone()),

diff --git a/engine/src/integration_tests.rs b/engine/src/integration_tests.rs
@@ -5213,6 +5213,23 @@ fn test_closure() {
     run_test("", hdr, rs, &["get_a"], &[]);
 }
 
+#[test]
+fn test_underscored_namespace_for_inner_type() {
+    let hdr = indoc! {"
+        namespace __foo {
+            struct daft {
+                struct bob {
+                    int a;
+                };
+                int a;
+            };
+        }
+        inline void bar(__foo::daft::bob) {}
+    "};
+    let rs = quote! {};
+    run_test("", hdr, rs, &["bar"], &[]);
+}
+
 #[test]
 fn test_blocklist_not_overly_broad() {
     // This is a regression test. We used to block anything that starts with "rust" or "std",