diff --git a/tools/api_checker/check_docs.py b/tools/api_checker/check_docs.py index 759fb71d9..84ec96d2d 100755 --- a/tools/api_checker/check_docs.py +++ b/tools/api_checker/check_docs.py @@ -1,5 +1,6 @@ #!/usr/bin/env python3 -"""Verify that public API entries have documentation links. +""" +Verify that public API entries have documentation links. Consumes an API snapshot from extract_api.py and checks: 1. Every public callable/type-tier entry has an @sa comment (with exceptions) @@ -11,7 +12,8 @@ import argparse import json import os import re -import subprocess +# subprocess is only called with fixed argument lists, never through a shell. +import subprocess # nosec B404 import sys from urllib.parse import unquote @@ -26,7 +28,7 @@ STL_EXEMPT = {'value_type', 'reference', 'const_reference', 'pointer', 'const_po def get_repo_root(): """Find the repository root via git, so this script works regardless of invoking CWD.""" try: - result = subprocess.run( + result = subprocess.run( # nosec B603 B607 ['git', 'rev-parse', '--show-toplevel'], capture_output=True, text=True, check=True, timeout=10 ) @@ -40,7 +42,8 @@ MKDOCS_YML = os.path.join(REPO_ROOT, 'docs', 'mkdocs', 'mkdocs.yml') def load_redirect_map() -> dict: - """Parse the redirect_maps block of docs/mkdocs/mkdocs.yml: {old_relative_path: new_relative_path}. + """ + Parse the redirect_maps block of docs/mkdocs/mkdocs.yml: {old_relative_path: new_relative_path}. mkdocs' redirect plugin lets a doc page move without breaking existing @sa URLs -- e.g. 'api/basic_json/operator_ltlt.md' redirects to the real file at 'api/operator_ltlt.md'. diff --git a/tools/api_checker/check_macros.py b/tools/api_checker/check_macros.py index da3b22cd2..0657270fb 100755 --- a/tools/api_checker/check_macros.py +++ b/tools/api_checker/check_macros.py @@ -1,5 +1,6 @@ #!/usr/bin/env python3 -"""Advisory-only cross-check between documented macros and their #define sites. +""" +Advisory-only cross-check between documented macros and their #define sites. Macros have no C++ access-specifier concept, so the AST-based public/private test that extract_api.py uses for classes doesn't transfer -- see tools/api_checker/POLICY.md's "Known @@ -29,7 +30,8 @@ def report(location: str, description: str): def extract_macro_names(md_path: str) -> list: - """Extract macro name(s) from a doc page's H1 heading. + """ + Extract macro name(s) from a doc page's H1 heading. Most pages use a single-line markdown heading ('# JSON_ASSERT'). A few document a family of related macros under one page using a multi-line HTML heading listing comma-separated names @@ -62,8 +64,10 @@ def extract_macro_names(md_path: str) -> list: def macro_is_referenced(macro_name: str, include_dir: str) -> bool: - """Check whether macro_name is defined OR referenced (#ifdef/#ifndef/defined()) anywhere - under include_dir. + """ + Check whether macro_name is defined or referenced anywhere under include_dir. + + A reference is any #ifdef, #ifndef, or defined() check. Some documented macros (e.g. JSON_NOEXCEPTION, JSON_THROW_USER) are user-supplied overrides: the library only checks whether they're defined, it never #defines them itself. Requiring a diff --git a/tools/api_checker/diff_api.py b/tools/api_checker/diff_api.py index dbce9932e..a61628152 100755 --- a/tools/api_checker/diff_api.py +++ b/tools/api_checker/diff_api.py @@ -1,5 +1,6 @@ #!/usr/bin/env python3 -"""Diff the public API surface between two refs to flag breaking vs. feature changes. +""" +Diff the public API surface between two refs to flag breaking vs. feature changes. A "ref" for --old/--new is resolved in this order: 1. A stored, committed historical record at tools/api_checker/history/.json, if one exists @@ -24,7 +25,8 @@ refinements verified, by testing against real historical releases -- not by insp import argparse import json import os -import subprocess +# subprocess is only called with fixed argument lists, never through a shell. +import subprocess # nosec B404 import sys import tempfile from collections import defaultdict @@ -43,7 +45,7 @@ from extract_api import SURFACE_FORMAT_VERSION # noqa: E402 def resolve_commit_sha(ref: str) -> str | None: """Resolve a ref to its full commit sha, or None if that fails.""" try: - result = subprocess.run( + result = subprocess.run( # nosec B603 B607 ['git', 'rev-parse', ref], capture_output=True, text=True, check=True, timeout=10 ) @@ -54,7 +56,8 @@ def resolve_commit_sha(ref: str) -> str | None: def extract_surface_for_ref(ref: str, header: str = 'include/nlohmann/json.hpp', include: str = 'include') -> dict: - """Check out the full include/ tree at `ref` into a temp dir and extract its API surface. + """ + Check out the full include/ tree at `ref` into a temp dir and extract its API surface. A single-file checkout of json.hpp is not enough: json.hpp #includes dozens of other headers under nlohmann/detail/ that must exist at the same ref for parsing to succeed. @@ -64,11 +67,11 @@ def extract_surface_for_ref(ref: str, header: str = 'include/nlohmann/json.hpp', these; snapshot_release.py uses them as a historical record's provenance. """ with tempfile.TemporaryDirectory(prefix='api_checker_') as tmpdir: - archive = subprocess.run( + archive = subprocess.run( # nosec B603 B607 ['git', 'archive', ref, '--', 'include'], capture_output=True, check=True ) - subprocess.run(['tar', '-x', '-C', tmpdir], input=archive.stdout, check=True) + subprocess.run(['tar', '-x', '-C', tmpdir], input=archive.stdout, check=True) # nosec B603 B607 ref_include = os.path.join(tmpdir, include) ref_header = os.path.join(tmpdir, header) @@ -77,7 +80,7 @@ def extract_surface_for_ref(ref: str, header: str = 'include/nlohmann/json.hpp', sys.exit(1) surface_output = os.path.join(tmpdir, 'surface.json') - result = subprocess.run( + result = subprocess.run( # nosec B603 [sys.executable, os.path.join(SCRIPT_DIR, 'extract_api.py'), '--header', ref_header, '--include', ref_include, @@ -127,7 +130,8 @@ def resolve_surface(ref: str | None, explicit_file: str | None, header: str, inc def build_identity_dict(public_api: list) -> dict: - """Group a surface's public_api list by identity for diffing. + """ + Group a surface's public_api list by identity for diffing. Identity is (scope, identity_name, kind, signature) -- the same four components extract_api.py's identity_key() joins into one opaque string internally, exposed here as diff --git a/tools/api_checker/extract_api.py b/tools/api_checker/extract_api.py index d076679ba..edef1126c 100755 --- a/tools/api_checker/extract_api.py +++ b/tools/api_checker/extract_api.py @@ -1,5 +1,6 @@ #!/usr/bin/env python3 -"""Extract the public API surface of nlohmann/json using libclang AST. +""" +Extract the public API surface of nlohmann/json using libclang AST. This tool derives the public API from C++ semantics (class templates, access specifiers, namespace scoping) independently of documentation status. The extracted surface is the @@ -23,7 +24,8 @@ import argparse import json import os import re -import subprocess +# subprocess is only called with fixed argument lists, never through a shell. +import subprocess # nosec B404 import sys try: @@ -69,10 +71,13 @@ def _read_file_cached(path: str) -> str: def get_signature_text(cursor) -> str: - """The normalized source text of a declaration's own signature -- template header, return - type, name, full parameter list, and trailing cv/ref/noexcept qualifiers -- stopping before - the function body ('{') or at the terminating ';'/'= default;'/'= 0;'. Never includes the - body, so implementation-only changes don't affect identity. + """ + Return the normalized source text of a declaration's own signature. + + The signature covers the template header, return type, name, full parameter list, and + trailing cv/ref/noexcept qualifiers, stopping before the function body ('{') or at the + terminating ';'/'= default;'/'= 0;'. Never includes the body, so implementation-only changes + don't affect identity. Reads raw source text via cursor.extent's byte offsets rather than cursor.get_tokens(): the latter was found to silently return zero tokens whenever a cursor's extent starts @@ -150,7 +155,10 @@ def get_signature_text(cursor) -> str: def get_identity_name(cursor, scope: str) -> str: - """The name component of a cursor's identity -- usually cursor.spelling, but not for + """ + Return the name component of a cursor's identity. + + This is usually cursor.spelling, but not for CONSTRUCTOR/DESTRUCTOR cursors, FUNCTION_TEMPLATE cursors that are themselves templated constructors (e.g. `template basic_json(CompatibleType&& val)`, which libclang represents as FUNCTION_TEMPLATE, not CONSTRUCTOR), or CONVERSION_FUNCTION @@ -203,9 +211,11 @@ def get_identity_name(cursor, scope: str) -> str: def identity_key(cursor, scope: str) -> str: - """A stable, overload-disambiguating key, used internally during extraction to prevent two - distinct entries from silently colliding in the in-memory api_dict. Two prior approaches were - tried and found broken: + """ + Return a stable, overload-disambiguating identity key for a cursor. + + The key is used internally during extraction to prevent two distinct entries from silently + colliding in the in-memory api_dict. Two prior approaches were tried and found broken: 1. {scope, name, kind, params} from cursor.get_arguments() alone: silently collided for overload sets differentiated only by constness, ref-qualifiers, or SFINAE constraints @@ -246,7 +256,7 @@ def identity_key(cursor, scope: str) -> str: def get_repo_root(): """Find the repository root via git, so location paths are deterministic regardless of invoking CWD.""" try: - result = subprocess.run( + result = subprocess.run( # nosec B603 B607 ['git', 'rev-parse', '--show-toplevel'], capture_output=True, text=True, check=True, timeout=10 ) @@ -278,7 +288,7 @@ def cursor_location(cursor) -> str: def get_system_includes(compiler='clang++'): """Discover system include paths by parsing clang++ -E -x c++ -v /dev/null output.""" try: - result = subprocess.run( + result = subprocess.run( # nosec B603 [compiler, '-E', '-x', 'c++', '-v', '/dev/null'], capture_output=True, text=True, check=True, timeout=10 ) @@ -320,7 +330,8 @@ def setup_libclang(): try: cindex.conf.set_library_file(path) return True - except Exception: + except Exception: # nosec B112 + # libclang rejected this candidate; try the next one. continue try: @@ -328,7 +339,8 @@ def setup_libclang(): if filename: cindex.conf.set_library_file(filename) return True - except Exception: + except Exception: # nosec B110 + # No usable default library; the caller reports the failure. pass return False @@ -358,8 +370,12 @@ def get_qualified_name(cursor) -> str: def walk_class_template(cursor, public_classes: set, api_dict: dict, documented_non_public: list): - """Walk a class template's members: extract public callable/type-tier entities, and flag - any non-public member that surprisingly carries a real @sa URL (a genuine documentation leak).""" + """ + Walk a class template's members. + + Extract public callable/type-tier entities, and flag any non-public member that surprisingly + carries an @sa URL into the documentation site (a genuine documentation leak). + """ if cursor.kind not in (cindex.CursorKind.CLASS_TEMPLATE, cindex.CursorKind.CLASS_DECL, cindex.CursorKind.STRUCT_DECL): return if cursor.spelling not in public_classes or not cursor.is_definition(): @@ -423,7 +439,8 @@ def walk_class_template(cursor, public_classes: set, api_dict: dict, documented_ doc_url = extract_sa_url(underlying_decl.raw_comment) if underlying_decl.location.file: underlying_location = cursor_location(underlying_decl) - except Exception: + except Exception: # nosec B110 + # Alias without a resolvable declaration: no @sa to follow. pass key = identity_key(child, scope) @@ -486,9 +503,12 @@ def walk_ast(cursor, public_classes: set, api_dict: dict, documented_non_public: def write_surface(api_dict: dict, output_path: str, extracted_from: str, extra_meta: dict | None = None): - """Write the minimal, location/doc-independent API surface: a format_version-tagged, sorted - list of self-describing records (scope, kind, name, identity_name, tier, signature, - pretty_signature) -- no opaque joined key, no location, no doc_url. + """ + Write the minimal, location/doc-independent API surface. + + The surface is a format_version-tagged, sorted list of self-describing records (scope, kind, + name, identity_name, tier, signature, pretty_signature) -- no opaque joined key, no location, + no doc_url. This is the file meant to be committed and diffed release-to-release (see tools/api_checker/README.md and POLICY.md): 'signature'/'identity_name' are the same values diff --git a/tools/api_checker/snapshot_release.py b/tools/api_checker/snapshot_release.py index c8d95f002..417a4b8a7 100755 --- a/tools/api_checker/snapshot_release.py +++ b/tools/api_checker/snapshot_release.py @@ -1,5 +1,6 @@ #!/usr/bin/env python3 -"""Capture immutable, per-release API surface records into tools/api_checker/history/. +""" +Capture immutable, per-release API surface records into tools/api_checker/history/. These are the durable, committed counterpart to diff_api.py's live git-archive-and-extract path: once a release is tagged, run this once to capture tools/api_checker/history/.json, commit @@ -13,7 +14,8 @@ import argparse import datetime import json import os -import subprocess +# subprocess is only called with fixed argument lists, never through a shell. +import subprocess # nosec B404 import sys SCRIPT_DIR = os.path.dirname(os.path.abspath(__file__)) @@ -24,9 +26,12 @@ from diff_api import extract_surface_for_ref # noqa: E402 def discover_v3_tags() -> list: - """List every v3.* git tag, sorted by dotted version (not lexicographically -- v3.9.0 must - sort before v3.10.0).""" - result = subprocess.run(['git', 'tag', '--list', 'v3.*'], capture_output=True, text=True, check=True) + """ + List every v3.* git tag, sorted by dotted version. + + Sorting is numeric, not lexicographic: v3.9.0 must sort before v3.10.0. + """ + result = subprocess.run(['git', 'tag', '--list', 'v3.*'], capture_output=True, text=True, check=True) # nosec B603 B607 tags = [t for t in result.stdout.splitlines() if t.strip()] def version_key(tag):