Run the snapshot comparison on CI (#56207)

.gitignore

@@ -183,5 +183,5 @@ __pycache__/
 # Doxygen XML output used for C++ API tracking
 /packages/react-native/**/api/xml
 /packages/react-native/**/api/codegen
-/packages/react-native/**/.doxygen.config.generated
+/packages/react-native/**/.doxygen.config.*.generated
 /scripts/cxx-api/codegen

scripts/cxx-api/parser/__main__.py

@@ -35,8 +35,12 @@ def run_command(
     """Run a subprocess command with consistent error handling."""
     result = subprocess.run(cmd, **kwargs)
     if result.returncode != 0:
-        if verbose:
-            print(f"{label} finished with error: {result.stderr}")
+        stderr_output = result.stderr or ""
+        if isinstance(stderr_output, bytes):
+            stderr_output = stderr_output.decode("utf-8", errors="replace")
+        print(f"{label} failed (exit code {result.returncode})", file=sys.stderr)
+        if stderr_output:
+            print(stderr_output, file=sys.stderr)
         raise RuntimeError(
             f"{label} finished with error (exit code {result.returncode})"
         )
@@ -53,9 +57,11 @@ def build_codegen(
 ) -> str:
     react_native_dir = os.path.join(get_react_native_dir(), "packages", "react-native")
 
+    node_bin = os.environ.get("NODE_BIN", "node")
+
     run_command(
         [
-            "node",
+            node_bin,
             "./scripts/generate-codegen-artifacts.js",
             "--path",
             "./",
@@ -193,22 +199,20 @@ def build_snapshots(
                     failed_views = ", ".join(name for name, _ in errors)
                     raise RuntimeError(f"Failed to generate snapshots: {failed_views}")
     else:
-        with tempfile.TemporaryDirectory(prefix="cxx-api-test-") as work_dir:
-            snapshot = build_snapshot_for_view(
-                api_view="Test",
-                react_native_dir=react_native_dir,
-                include_directories=[],
-                exclude_patterns=[],
-                definitions={},
-                output_dir=output_dir,
-                codegen_platform=None,
-                verbose=verbose,
-                input_filter=input_filter,
-                work_dir=work_dir,
-            )
-
-            if verbose:
-                print(snapshot)
+        snapshot = build_snapshot_for_view(
+            api_view="Test",
+            react_native_dir=react_native_dir,
+            include_directories=[],
+            exclude_patterns=[],
+            definitions={},
+            output_dir=output_dir,
+            codegen_platform=None,
+            verbose=verbose,
+            input_filter=input_filter,
+        )
+
+        if verbose:
+            print(snapshot)
 
 
 def get_default_snapshot_dir() -> str:
@@ -229,6 +233,11 @@ def main():
         action="store_true",
         help="Generate snapshots to a temp directory and compare against committed ones",
     )
+    parser.add_argument(
+        "--check-output",
+        type=str,
+        help="File path to write check results to (used with --check)",
+    )
     parser.add_argument(
         "--snapshot-dir",
         type=str,
@@ -304,7 +313,11 @@ def main():
         if args.check:
             snapshot_dir = args.snapshot_dir or get_default_snapshot_dir()
 
-            if not check_snapshots(snapshot_output_dir, snapshot_dir):
+            if not check_snapshots(
+                snapshot_output_dir,
+                snapshot_dir,
+                output_file=args.check_output,
+            ):
                 sys.exit(1)
 
             print("All snapshot checks passed")

scripts/cxx-api/parser/builders.py

@@ -41,7 +41,12 @@
     resolve_linked_text_name,
     split_specialization,
 )
-from .utils.argument_parsing import _find_matching_angle, _split_arguments
+from .utils.argument_parsing import (
+    _find_matching_angle,
+    _split_arguments,
+    format_parsed_type,
+    parse_type_with_argstrings,
+)
 
 
 @dataclass
@@ -161,8 +166,14 @@ def get_base_classes(
 ) -> list:
     """
     Get the base classes of a compound object.
+
+    Deduplicates base classes by name. Doxygen can emit duplicate
+    ``basecompoundref`` entries when a class inherits constructors via
+    ``using Base::Base;`` — the using-declaration is incorrectly reported
+    as an additional base class reference.
     """
     base_classes = []
+    seen_names: set[str] = set()
     if compound_object.basecompoundref:
         for base in compound_object.basecompoundref:
             # base is a compoundRefType with:
@@ -179,6 +190,10 @@ def get_base_classes(
                 # Ignore private base classes
                 continue
 
+            if base_name in seen_names:
+                continue
+            seen_names.add(base_name)
+
             base_classes.append(
                 base_class(
                     base_name,
@@ -307,6 +322,16 @@ def get_doxygen_params(
             if param.get_type()
             else ""
         )
+
+        # Doxygen may incorrectly cross-reference parameter names inside
+        # inline function pointer types to member variables of the enclosing
+        # class, producing qualified paths like "const void*
+        # ns::Class::data" instead of "const void* data".  Re-parse the
+        # type through parse_type_with_argstrings which delegates to
+        # _parse_single_argument — that already strips "::" from names.
+        segments = parse_type_with_argstrings(param_type)
+        if len(segments) > 1:
+            param_type = format_parsed_type(segments)
         param_name = param.declname or param.defname or None
         param_default = (
             resolve_linked_text_name(param.defval)[0].strip() if param.defval else None
@@ -333,6 +358,8 @@ def get_doxygen_params(
                     + param_array
                 )
                 param_name = None
+            elif param_name:
+                param_name += param_array
             else:
                 param_type += param_array
 
@@ -350,6 +377,29 @@ def get_doxygen_params(
                     param_type[:insert_pos] + param_name + param_type[insert_pos:]
                 )
                 param_name = None
+        else:
+            # Doxygen bug: for pointer-to-member-function params with
+            # ref-qualifiers (& or &&), Doxygen incorrectly embeds the
+            # parameter name in the type string between cv-qualifiers
+            # and the ref-qualifier, and omits <declname> entirely:
+            #   <type>R(ns::*)() const asFoo &amp;</type>
+            # Detect this pattern and reconstruct the correct type:
+            #   R(ns::*asFoo)() const &
+            m = re.search(
+                r"(\([^)]*::\*\))"  # group 1: ptr-to-member declarator
+                r"(.+?)"  # group 2: param list + cv-qualifiers
+                r"\s+([a-zA-Z_]\w*)"  # group 3: misplaced identifier
+                r"\s*(&{1,2})\s*$",  # group 4: ref-qualifier
+                param_type,
+            )
+            if m:
+                param_type = (
+                    param_type[: m.end(1) - 1]  # up to ')' of (ns::*)
+                    + m.group(3)  # insert extracted name
+                    + param_type[m.end(1) - 1 : m.end(2)]  # ')' + params + cv-quals
+                    + " "
+                    + m.group(4)  # ref-qualifier
+                )
 
         qualifiers, core_type = extract_qualifiers(param_type)
         arguments.append((qualifiers, core_type, param_name, param_default))

scripts/cxx-api/parser/member/function_member.py

@@ -65,6 +65,26 @@ def close(self, scope: Scope):
         self.type = qualify_type_str(self.type, scope)
         self.arguments = qualify_arguments(self.arguments, scope)
         self._qualify_specialization_args(scope)
+        self._qualify_conversion_operator_type(scope)
+
+    def _qualify_conversion_operator_type(self, scope: Scope) -> None:
+        """Qualify the type portion of conversion operator names.
+
+        Conversion operators like ``operator jsi::Array`` contain a type
+        reference that may need namespace qualification (e.g., to
+        ``operator facebook::jsi::Array``).
+        """
+        prefix = "operator "
+        if not self.name.startswith(prefix):
+            return
+
+        type_part = self.name[len(prefix) :]
+        if not type_part or not (type_part[0].isalpha() or type_part[0] == "_"):
+            return
+
+        qualified = qualify_type_str(type_part, scope)
+        if qualified != type_part:
+            self.name = prefix + qualified
 
     def to_string(
         self,

scripts/cxx-api/parser/member/variable_member.py

@@ -111,6 +111,8 @@ def to_string(
                 result += f"{qualified_type} (*{name})({formatted_args})"
         else:
             result += f"{format_parsed_type(self._parsed_type)} {name}"
+            if self.argstring:
+                result += self.argstring
 
         if STORE_INITIALIZERS_IN_SNAPSHOT and self.value is not None:
             if self.is_brace_initializer:

scripts/cxx-api/parser/scope/base_scope_kind.py

@@ -34,7 +34,12 @@ def _format_scope_body(self, scope: Scope, member_suffix: str = "") -> str:
         stringified_members = [
             member.to_string(2) + member_suffix for member in scope.get_members()
         ]
+
         stringified_members = natsorted(stringified_members)
+        # Deduplicate members that produce identical signatures (e.g.
+        # constructors inherited from multiple bases).
+        stringified_members = list(dict.fromkeys(stringified_members))
+
         result = "{"
         if stringified_members:
             result += "\n" + "\n".join(stringified_members)

scripts/cxx-api/parser/scope/namespace_scope_kind.py

@@ -35,6 +35,8 @@ def to_string(self, scope: Scope) -> str:
         result = []
         for kind in MemberKind:
             sorted_group = natsorted(groups[kind])
+            # Deduplicate members with identical signatures.
+            sorted_group = list(dict.fromkeys(sorted_group))
             result.extend(sorted_group)
 
         return "\n".join(result)

scripts/cxx-api/parser/snapshot_diff.py

@@ -9,25 +9,40 @@
 
 import difflib
 import os
+import sys
 
 
-def check_snapshots(generated_dir: str, committed_dir: str) -> bool:
+def check_snapshots(
+    generated_dir: str, committed_dir: str, output_file: str | None = None
+) -> bool:
     """Compare generated snapshots against committed ones.
 
     Returns True if check passes (snapshots match or no committed snapshots).
     Returns False if snapshots differ.
+
+    If output_file is provided, writes comparison results to that file
+    instead of stdout.
     """
+    out = open(output_file, "w") if output_file else sys.stdout
+    try:
+        return _check_snapshots_impl(generated_dir, committed_dir, out)
+    finally:
+        if output_file:
+            out.close()
+
+
+def _check_snapshots_impl(generated_dir: str, committed_dir: str, out) -> bool:
     if not os.path.isdir(committed_dir):
-        print(f"No committed snapshots directory found at: {committed_dir}")
-        print("Skipping comparison (no baseline to compare against)")
+        print(f"No committed snapshots directory found at: {committed_dir}", file=out)
+        print("Skipping comparison (no baseline to compare against)", file=out)
         return True
 
     committed_files = sorted(f for f in os.listdir(committed_dir) if f.endswith(".api"))
     generated_files = sorted(f for f in os.listdir(generated_dir) if f.endswith(".api"))
 
     if not committed_files:
-        print("No committed snapshot files found")
-        print("Skipping comparison (no baseline to compare against)")
+        print("No committed snapshot files found", file=out)
+        print("Skipping comparison (no baseline to compare against)", file=out)
         return True
 
     committed_set = set(committed_files)
@@ -40,13 +55,14 @@ def check_snapshots(generated_dir: str, committed_dir: str) -> bool:
 
         if filename not in generated_set:
             print(
-                f"FAIL: {filename} exists in committed snapshots but was not generated"
+                f"FAIL: {filename} exists in committed snapshots but was not generated",
+                file=out,
             )
             all_passed = False
             continue
 
         if filename not in committed_set:
-            print(f"OK: {filename} generated (no committed baseline)")
+            print(f"OK: {filename} generated (no committed baseline)", file=out)
             continue
 
         with open(committed_path) as f:
@@ -55,9 +71,9 @@ def check_snapshots(generated_dir: str, committed_dir: str) -> bool:
             generated_content = f.read()
 
         if committed_content == generated_content:
-            print(f"OK: {filename} matches committed snapshot")
+            print(f"OK: {filename} matches committed snapshot", file=out)
         else:
-            print(f"FAIL: {filename} differs from committed snapshot")
+            print(f"FAIL: {filename} differs from committed snapshot", file=out)
             diff = "\n".join(
                 difflib.unified_diff(
                     committed_content.splitlines(),
@@ -67,7 +83,7 @@ def check_snapshots(generated_dir: str, committed_dir: str) -> bool:
                     lineterm="",
                 )
             )
-            print(diff)
+            print(diff, file=out)
             all_passed = False
 
     return all_passed

scripts/cxx-api/parser/utils/argument_parsing.py

@@ -530,6 +530,14 @@ def parse_type_with_argstrings(
                 i = close + 1
                 continue
 
+            # Complex declarator starting with * or &, e.g. *(*fp)(int)
+            # in "int(*(*fp)(int))(double)".  Argument lists never start
+            # with pointer/reference characters.
+            if stripped and stripped[0] in ("*", "&"):
+                current_text.append(type_str[i : close + 1])
+                i = close + 1
+                continue
+
             # Try to parse as a function argument list
             args: list[Argument] = []
             if stripped:

scripts/cxx-api/tests/snapshots/should_deduplicate_base_classes_with_using_constructor/snapshot.api

@@ -0,0 +1,12 @@
+struct test::Converter<int> : public test::ConverterBase<int> {
+  public void doSomething();
+}
+
+template <typename T>
+struct test::Converter {
+}
+
+template <typename T>
+struct test::ConverterBase {
+  public ConverterBase() = default;
+}

scripts/cxx-api/tests/snapshots/should_deduplicate_base_classes_with_using_constructor/test.h

@@ -0,0 +1,26 @@
+/*
+ * Copyright (c) Meta Platforms, Inc. and affiliates.
+ *
+ * This source code is licensed under the MIT license found in the
+ * LICENSE file in the root directory of this source tree.
+ */
+
+#pragma once
+
+namespace test {
+
+template <typename T>
+struct ConverterBase {
+  ConverterBase() = default;
+};
+
+template <typename T>
+struct Converter {};
+
+template <>
+struct Converter<int> : public ConverterBase<int> {
+  using ConverterBase<int>::ConverterBase;
+  void doSomething();
+};
+
+} // namespace test

scripts/cxx-api/tests/snapshots/should_handle_array_syntax_correctly/snapshot.api

@@ -0,0 +1,3 @@
+void test::arrayParam(const char name[]);
+void test::arrayParamSized(int values[10]);
+void test::arrayParamUnnamed(const char[]);