From 9e687f3b962dab86a5580410a46d1eb0ae5f8601 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dawid=20Ma=C5=82ecki?= Date: Fri, 3 Apr 2026 06:46:30 -0700 Subject: [PATCH] Detecting references to exluded symbols Summary: Adds `--warn-excluded-refs` flag to the C++ API parser which produces warnings whenever the excluded symbol is still referenced in the snapshot. By definition, if the private symbol is referenced in the public API then it must be public. Hence, every reference to it should also be excluded. This flag helps to verify if there are such cases. Changelog: [Internal] Differential Revision: D99417803 --- scripts/cxx-api/parser/__init__.py | 13 +- scripts/cxx-api/parser/__main__.py | 26 ++ scripts/cxx-api/parser/main.py | 231 ++++++++++++++- scripts/cxx-api/parser/snapshot.py | 1 + .../tests/test_excluded_symbol_references.py | 280 ++++++++++++++++++ 5 files changed, 548 insertions(+), 3 deletions(-) create mode 100644 scripts/cxx-api/tests/test_excluded_symbol_references.py diff --git a/scripts/cxx-api/parser/__init__.py b/scripts/cxx-api/parser/__init__.py index 9dade47a0352..ea62cfecc880 100644 --- a/scripts/cxx-api/parser/__init__.py +++ b/scripts/cxx-api/parser/__init__.py @@ -3,7 +3,16 @@ # This source code is licensed under the MIT license found in the # LICENSE file in the root directory of this source tree. -from .main import build_snapshot +from .main import ( + build_snapshot, + ExcludedSymbolReference, + find_excluded_symbol_references, +) from .path_utils import get_repo_root -__all__ = ["build_snapshot", "get_repo_root"] +__all__ = [ + "build_snapshot", + "ExcludedSymbolReference", + "find_excluded_symbol_references", + "get_repo_root", +] diff --git a/scripts/cxx-api/parser/__main__.py b/scripts/cxx-api/parser/__main__.py index 574b54d1bfa1..ca4d1198b1df 100644 --- a/scripts/cxx-api/parser/__main__.py +++ b/scripts/cxx-api/parser/__main__.py @@ -96,6 +96,7 @@ def build_snapshot_for_view( input_filter: str = None, work_dir: str | None = None, exclude_symbols: list[str] | None = None, + warn_excluded_refs: bool = False, ) -> str: if verbose: print(f"[{api_view}] Generating API view") @@ -130,6 +131,23 @@ def build_snapshot_for_view( snapshot = build_snapshot( os.path.join(work_dir, "xml"), exclude_symbols=exclude_symbols ) + + if warn_excluded_refs and snapshot.excluded_symbol_references: + _YELLOW = "\033[33m" + _RESET = "\033[0m" + refs = snapshot.excluded_symbol_references + print( + f"{_YELLOW}[{api_view}] WARNING: Found {len(refs)} reference(s) " + f"to excluded symbols:{_RESET}", + file=sys.stderr, + ) + for ref in refs: + print( + f"{_YELLOW} • {ref.scope}: {ref.context} '{ref.symbol}' " + f"matches excluded pattern '{ref.pattern}'{_RESET}", + file=sys.stderr, + ) + snapshot_string = snapshot.to_string() output_file = os.path.join(output_dir, f"{api_view}Cxx.api") @@ -151,6 +169,7 @@ def build_snapshots( view_filter: str | None = None, is_test: bool = False, keep_xml: bool = False, + warn_excluded_refs: bool = False, ) -> None: if not is_test: configs_to_build = [ @@ -195,6 +214,7 @@ def build_snapshots( input_filter=input_filter if config.input_filter else None, work_dir=work_dir, exclude_symbols=config.exclude_symbols, + warn_excluded_refs=warn_excluded_refs, ) futures[future] = config.snapshot_name @@ -285,6 +305,11 @@ def main(): action="store_true", help="Keep the generated Doxygen XML files next to the .api output in a xml/ directory", ) + parser.add_argument( + "--warn-excluded-refs", + action="store_true", + help="Warn when non-excluded symbols reference types matching exclude_symbols patterns", + ) args = parser.parse_args() verbose = not args.validate @@ -343,6 +368,7 @@ def main(): view_filter=args.view, is_test=args.test, keep_xml=args.xml, + warn_excluded_refs=args.warn_excluded_refs, ) if args.validate: diff --git a/scripts/cxx-api/parser/main.py b/scripts/cxx-api/parser/main.py index d5f629d7aa2f..47e88cb9bee1 100644 --- a/scripts/cxx-api/parser/main.py +++ b/scripts/cxx-api/parser/main.py @@ -10,6 +10,7 @@ from __future__ import annotations import os +from dataclasses import dataclass from doxmlparser import compound, index @@ -24,8 +25,38 @@ get_typedef_member, get_variable_member, ) +from .member import ( + FriendMember, + FunctionMember, + PropertyMember, + TypedefMember, + VariableMember, +) +from .scope import Scope, StructLikeScopeKind +from .scope.extendable import Extendable from .snapshot import Snapshot -from .utils import has_scope_resolution_outside_angles, parse_qualified_path +from .utils import ( + format_parsed_type, + has_scope_resolution_outside_angles, + parse_qualified_path, +) + + +@dataclass +class ExcludedSymbolReference: + """A reference to an excluded symbol found in the API snapshot.""" + + symbol: str + """The full text containing the reference (e.g., the type string).""" + + pattern: str + """The exclude_symbols pattern that matched.""" + + scope: str + """The qualified name of the scope containing the reference.""" + + context: str + """Description of where the reference appears (e.g., 'base class', 'return type').""" def _should_exclude_symbol(name: str, exclude_symbols: list[str]) -> bool: @@ -170,6 +201,199 @@ def _handle_class_compound(snapshot, compound_object): ) +def _check_text_for_excluded_patterns( + text: str, + scope_name: str, + context: str, + exclude_symbols: list[str], + results: list[ExcludedSymbolReference], +) -> None: + """Append an ExcludedSymbolReference for each pattern found in *text*.""" + for pattern in exclude_symbols: + if pattern in text: + results.append( + ExcludedSymbolReference( + symbol=text, + pattern=pattern, + scope=scope_name, + context=context, + ) + ) + + +def _check_arguments_for_excluded_patterns( + arguments: list, + scope_name: str, + context_prefix: str, + exclude_symbols: list[str], + results: list[ExcludedSymbolReference], +) -> None: + """Check every argument's type string for excluded patterns.""" + for arg in arguments: + # Argument is a tuple: (qualifiers, type, name, default_value) + arg_type = arg[1] + if arg_type: + _check_text_for_excluded_patterns( + arg_type, + scope_name, + f"{context_prefix} parameter type", + exclude_symbols, + results, + ) + + +def _check_member_for_excluded_patterns( + member, + scope_name: str, + exclude_symbols: list[str], + results: list[ExcludedSymbolReference], +) -> None: + """Check a single member for type references matching excluded patterns.""" + member_name = f"{scope_name}::{member.name}" + + if isinstance(member, FunctionMember): + if member.type: + _check_text_for_excluded_patterns( + member.type, + member_name, + "return type", + exclude_symbols, + results, + ) + _check_arguments_for_excluded_patterns( + member.arguments, + member_name, + "function", + exclude_symbols, + results, + ) + + elif isinstance(member, VariableMember): + type_str = format_parsed_type(member._parsed_type) + if type_str: + _check_text_for_excluded_patterns( + type_str, + member_name, + "variable type", + exclude_symbols, + results, + ) + _check_arguments_for_excluded_patterns( + member._fp_arguments, + member_name, + "function pointer", + exclude_symbols, + results, + ) + + elif isinstance(member, TypedefMember): + value = member.get_value() + if value: + _check_text_for_excluded_patterns( + value, + member_name, + "typedef target type", + exclude_symbols, + results, + ) + _check_arguments_for_excluded_patterns( + member._fp_arguments, + member_name, + "function pointer", + exclude_symbols, + results, + ) + + elif isinstance(member, PropertyMember): + if member.type: + _check_text_for_excluded_patterns( + member.type, + member_name, + "property type", + exclude_symbols, + results, + ) + + elif isinstance(member, FriendMember): + _check_text_for_excluded_patterns( + member.name, + member_name, + "friend declaration", + exclude_symbols, + results, + ) + + if member.specialization_args: + for arg in member.specialization_args: + _check_text_for_excluded_patterns( + arg, + member_name, + "member specialization argument", + exclude_symbols, + results, + ) + + +def _walk_scope_for_excluded_patterns( + scope: Scope, + exclude_symbols: list[str], + results: list[ExcludedSymbolReference], +) -> None: + """Recursively walk a scope tree checking for excluded pattern references.""" + scope_name = scope.get_qualified_name() or "(root)" + + # Check base classes (StructLikeScopeKind, ProtocolScopeKind, InterfaceScopeKind) + if isinstance(scope.kind, Extendable): + for base in scope.kind.base_classes: + _check_text_for_excluded_patterns( + base.name, + scope_name, + "base class", + exclude_symbols, + results, + ) + + # Check specialization args + if isinstance(scope.kind, StructLikeScopeKind) and scope.kind.specialization_args: + for arg in scope.kind.specialization_args: + _check_text_for_excluded_patterns( + arg, + scope_name, + "specialization argument", + exclude_symbols, + results, + ) + + for member in scope.get_members(): + _check_member_for_excluded_patterns( + member, scope_name, exclude_symbols, results + ) + + for inner in scope.inner_scopes.values(): + _walk_scope_for_excluded_patterns(inner, exclude_symbols, results) + + +def find_excluded_symbol_references( + snapshot: Snapshot, + exclude_symbols: list[str], +) -> list[ExcludedSymbolReference]: + """ + Walk the snapshot scope tree after it has been finalized and find + references to excluded symbols in type strings, base classes, and + other type references. + + This detects cases where a non-excluded symbol references an excluded + symbol (e.g., a class inherits from an excluded base, a function returns + an excluded type, etc.). + """ + if not exclude_symbols: + return [] + + results: list[ExcludedSymbolReference] = [] + _walk_scope_for_excluded_patterns(snapshot.root_scope, exclude_symbols, results) + return results + + def build_snapshot(xml_dir: str, exclude_symbols: list[str] | None = None) -> Snapshot: """ Reads the Doxygen XML output and builds a snapshot of the C++ API. @@ -218,4 +442,9 @@ def build_snapshot(xml_dir: str, exclude_symbols: list[str] | None = None) -> Sn print(f"Unknown compound kind: {kind}") snapshot.finish() + + snapshot.excluded_symbol_references = find_excluded_symbol_references( + snapshot, exclude_symbols + ) + return snapshot diff --git a/scripts/cxx-api/parser/snapshot.py b/scripts/cxx-api/parser/snapshot.py index eca55a0bcb29..edde0192e53f 100644 --- a/scripts/cxx-api/parser/snapshot.py +++ b/scripts/cxx-api/parser/snapshot.py @@ -21,6 +21,7 @@ class Snapshot: def __init__(self) -> None: self.root_scope: Scope = Scope(NamespaceScopeKind()) + self.excluded_symbol_references: list = [] def ensure_scope(self, scope_path: list[str]) -> Scope: """ diff --git a/scripts/cxx-api/tests/test_excluded_symbol_references.py b/scripts/cxx-api/tests/test_excluded_symbol_references.py new file mode 100644 index 000000000000..15cfefc1c54e --- /dev/null +++ b/scripts/cxx-api/tests/test_excluded_symbol_references.py @@ -0,0 +1,280 @@ +# 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. + +from __future__ import annotations + +import unittest + +from ..parser.main import find_excluded_symbol_references +from ..parser.member import ( + FriendMember, + FunctionMember, + PropertyMember, + TypedefMember, + VariableMember, +) +from ..parser.scope import Scope, StructLikeScopeKind +from ..parser.scope.extendable import Extendable +from ..parser.snapshot import Snapshot + + +def _make_snapshot_with_class( + class_name: str = "facebook::react::Foo", +) -> tuple[Snapshot, Scope]: + """Create a snapshot with a single struct and return both.""" + snapshot = Snapshot() + snapshot.create_or_get_namespace("facebook") + snapshot.create_or_get_namespace("facebook::react") + scope = snapshot.create_struct_like(class_name, StructLikeScopeKind.Type.STRUCT) + return snapshot, scope + + +class TestFindExcludedSymbolReferencesEmpty(unittest.TestCase): + def test_empty_exclude_symbols_returns_empty(self) -> None: + snapshot = Snapshot() + refs = find_excluded_symbol_references(snapshot, []) + self.assertEqual(refs, []) + + def test_no_references_returns_empty(self) -> None: + snapshot, scope = _make_snapshot_with_class() + scope.add_member( + FunctionMember( + name="doStuff", + type="int", + visibility="public", + arg_string="()", + is_virtual=False, + is_pure_virtual=False, + is_static=False, + ) + ) + snapshot.finish() + refs = find_excluded_symbol_references(snapshot, ["Experimental"]) + self.assertEqual(refs, []) + + +class TestFindExcludedSymbolReferencesBaseClass(unittest.TestCase): + def test_base_class_reference_detected(self) -> None: + snapshot, scope = _make_snapshot_with_class() + scope.kind.add_base( + Extendable.Base( + name="ExperimentalBase", + protection="public", + virtual=False, + refid="", + ) + ) + snapshot.finish() + refs = find_excluded_symbol_references(snapshot, ["Experimental"]) + self.assertEqual(len(refs), 1) + self.assertEqual(refs[0].symbol, "ExperimentalBase") + self.assertEqual(refs[0].pattern, "Experimental") + self.assertEqual(refs[0].context, "base class") + + def test_base_class_no_match(self) -> None: + snapshot, scope = _make_snapshot_with_class() + scope.kind.add_base( + Extendable.Base( + name="RegularBase", + protection="public", + virtual=False, + refid="", + ) + ) + snapshot.finish() + refs = find_excluded_symbol_references(snapshot, ["Experimental"]) + self.assertEqual(refs, []) + + +class TestFindExcludedSymbolReferencesFunctionMember(unittest.TestCase): + def test_return_type_reference_detected(self) -> None: + snapshot, scope = _make_snapshot_with_class() + scope.add_member( + FunctionMember( + name="getModule", + type="ExperimentalModule", + visibility="public", + arg_string="()", + is_virtual=False, + is_pure_virtual=False, + is_static=False, + ) + ) + snapshot.finish() + refs = find_excluded_symbol_references(snapshot, ["Experimental"]) + self.assertEqual(len(refs), 1) + self.assertEqual(refs[0].symbol, "ExperimentalModule") + self.assertEqual(refs[0].context, "return type") + + def test_parameter_type_reference_detected(self) -> None: + snapshot, scope = _make_snapshot_with_class() + scope.add_member( + FunctionMember( + name="setModule", + type="void", + visibility="public", + arg_string="(ExperimentalModule module)", + is_virtual=False, + is_pure_virtual=False, + is_static=False, + ) + ) + snapshot.finish() + refs = find_excluded_symbol_references(snapshot, ["Experimental"]) + self.assertEqual(len(refs), 1) + self.assertEqual(refs[0].context, "function parameter type") + + def test_no_match_in_function(self) -> None: + snapshot, scope = _make_snapshot_with_class() + scope.add_member( + FunctionMember( + name="doExperimentalStuff", + type="int", + visibility="public", + arg_string="(int x)", + is_virtual=False, + is_pure_virtual=False, + is_static=False, + ) + ) + snapshot.finish() + refs = find_excluded_symbol_references(snapshot, ["Experimental"]) + self.assertEqual(refs, []) + + +class TestFindExcludedSymbolReferencesVariableMember(unittest.TestCase): + def test_variable_type_reference_detected(self) -> None: + snapshot, scope = _make_snapshot_with_class() + scope.add_member( + VariableMember( + name="module", + type="ExperimentalModule", + visibility="public", + is_const=False, + is_static=False, + is_constexpr=False, + is_mutable=False, + value=None, + definition="ExperimentalModule module", + ) + ) + snapshot.finish() + refs = find_excluded_symbol_references(snapshot, ["Experimental"]) + self.assertEqual(len(refs), 1) + self.assertEqual(refs[0].context, "variable type") + + +class TestFindExcludedSymbolReferencesTypedefMember(unittest.TestCase): + def test_typedef_target_type_reference_detected(self) -> None: + snapshot, scope = _make_snapshot_with_class() + scope.add_member( + TypedefMember( + name="ModuleAlias", + type="ExperimentalModule", + argstring=None, + visibility="public", + keyword="using", + ) + ) + snapshot.finish() + refs = find_excluded_symbol_references(snapshot, ["Experimental"]) + self.assertEqual(len(refs), 1) + self.assertEqual(refs[0].context, "typedef target type") + + +class TestFindExcludedSymbolReferencesFriendMember(unittest.TestCase): + def test_friend_declaration_detected(self) -> None: + snapshot, scope = _make_snapshot_with_class() + scope.add_member(FriendMember(name="ExperimentalHelper")) + snapshot.finish() + refs = find_excluded_symbol_references(snapshot, ["Experimental"]) + self.assertEqual(len(refs), 1) + self.assertEqual(refs[0].context, "friend declaration") + + +class TestFindExcludedSymbolReferencesPropertyMember(unittest.TestCase): + def test_property_type_reference_detected(self) -> None: + snapshot, scope = _make_snapshot_with_class() + scope.add_member( + PropertyMember( + name="module", + type="ExperimentalModule *", + visibility="public", + is_static=False, + accessor=None, + is_readable=True, + is_writable=True, + ) + ) + snapshot.finish() + refs = find_excluded_symbol_references(snapshot, ["Experimental"]) + self.assertEqual(len(refs), 1) + self.assertEqual(refs[0].context, "property type") + + +class TestFindExcludedSymbolReferencesSpecializationArgs(unittest.TestCase): + def test_scope_specialization_arg_detected(self) -> None: + snapshot = Snapshot() + snapshot.create_or_get_namespace("facebook") + snapshot.create_or_get_namespace("facebook::react") + scope = snapshot.create_struct_like( + "facebook::react::Container", + StructLikeScopeKind.Type.STRUCT, + ) + scope.add_member( + FunctionMember( + name="get", + type="int", + visibility="public", + arg_string="()", + is_virtual=False, + is_pure_virtual=False, + is_static=False, + ) + ) + snapshot.finish() + refs = find_excluded_symbol_references(snapshot, ["Experimental"]) + self.assertEqual(len(refs), 1) + self.assertEqual(refs[0].context, "specialization argument") + + +class TestFindExcludedSymbolReferencesMultiplePatterns(unittest.TestCase): + def test_multiple_patterns_detected(self) -> None: + snapshot, scope = _make_snapshot_with_class() + scope.add_member( + FunctionMember( + name="getModule", + type="ExperimentalModule", + visibility="public", + arg_string="(FantomArg arg)", + is_virtual=False, + is_pure_virtual=False, + is_static=False, + ) + ) + snapshot.finish() + refs = find_excluded_symbol_references(snapshot, ["Experimental", "Fantom"]) + self.assertEqual(len(refs), 2) + patterns = {r.pattern for r in refs} + self.assertIn("Experimental", patterns) + self.assertIn("Fantom", patterns) + + def test_same_text_matches_multiple_patterns(self) -> None: + snapshot, scope = _make_snapshot_with_class() + scope.add_member( + FunctionMember( + name="get", + type="ExperimentalFantomModule", + visibility="public", + arg_string="()", + is_virtual=False, + is_pure_virtual=False, + is_static=False, + ) + ) + snapshot.finish() + refs = find_excluded_symbol_references(snapshot, ["Experimental", "Fantom"]) + self.assertEqual(len(refs), 2) + self.assertTrue(all(r.symbol == "ExperimentalFantomModule" for r in refs))