Address Codacy findings in the API checker scripts
Put multi-line docstring summaries on their own line, as a single sentence followed by a blank line (pydocstyle D205, D209, D213, D415). Annotate the subprocess import and calls with nosec: they only run fixed argument lists, never through a shell (Bandit B404, B603, B607). Do the same for the three broad except clauses in extract_api.py, which deliberately fall through to the next libclang candidate or skip an unresolvable alias (Bandit B110, B112). Signed-off-by: Niels Lohmann <mail@nlohmann.me>
This commit is contained in:
5 files changed
+77
-41
No files matched your search
@@ -1,5 +1,6 @@
|
|||||||
#!/usr/bin/env python3
|
#!/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:
|
Consumes an API snapshot from extract_api.py and checks:
|
||||||
1. Every public callable/type-tier entry has an @sa comment (with exceptions)
|
1. Every public callable/type-tier entry has an @sa comment (with exceptions)
|
||||||
@@ -11,7 +12,8 @@ import argparse
|
|||||||
import json
|
import json
|
||||||
import os
|
import os
|
||||||
import re
|
import re
|
||||||
import subprocess
|
# subprocess is only called with fixed argument lists, never through a shell.
|
||||||
|
import subprocess # nosec B404
|
||||||
import sys
|
import sys
|
||||||
from urllib.parse import unquote
|
from urllib.parse import unquote
|
||||||
|
|
||||||
@@ -26,7 +28,7 @@ STL_EXEMPT = {'value_type', 'reference', 'const_reference', 'pointer', 'const_po
|
|||||||
def get_repo_root():
|
def get_repo_root():
|
||||||
"""Find the repository root via git, so this script works regardless of invoking CWD."""
|
"""Find the repository root via git, so this script works regardless of invoking CWD."""
|
||||||
try:
|
try:
|
||||||
result = subprocess.run(
|
result = subprocess.run( # nosec B603 B607
|
||||||
['git', 'rev-parse', '--show-toplevel'],
|
['git', 'rev-parse', '--show-toplevel'],
|
||||||
capture_output=True, text=True, check=True, timeout=10
|
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:
|
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.
|
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'.
|
'api/basic_json/operator_ltlt.md' redirects to the real file at 'api/operator_ltlt.md'.
|
||||||
|
|||||||
@@ -1,5 +1,6 @@
|
|||||||
#!/usr/bin/env python3
|
#!/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
|
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
|
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:
|
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
|
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
|
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:
|
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:
|
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
|
the library only checks whether they're defined, it never #defines them itself. Requiring a
|
||||||
|
|||||||
@@ -1,5 +1,6 @@
|
|||||||
#!/usr/bin/env python3
|
#!/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:
|
A "ref" for --old/--new is resolved in this order:
|
||||||
1. A stored, committed historical record at tools/api_checker/history/<ref>.json, if one exists
|
1. A stored, committed historical record at tools/api_checker/history/<ref>.json, if one exists
|
||||||
@@ -24,7 +25,8 @@ refinements verified, by testing against real historical releases -- not by insp
|
|||||||
import argparse
|
import argparse
|
||||||
import json
|
import json
|
||||||
import os
|
import os
|
||||||
import subprocess
|
# subprocess is only called with fixed argument lists, never through a shell.
|
||||||
|
import subprocess # nosec B404
|
||||||
import sys
|
import sys
|
||||||
import tempfile
|
import tempfile
|
||||||
from collections import defaultdict
|
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:
|
def resolve_commit_sha(ref: str) -> str | None:
|
||||||
"""Resolve a ref to its full commit sha, or None if that fails."""
|
"""Resolve a ref to its full commit sha, or None if that fails."""
|
||||||
try:
|
try:
|
||||||
result = subprocess.run(
|
result = subprocess.run( # nosec B603 B607
|
||||||
['git', 'rev-parse', ref],
|
['git', 'rev-parse', ref],
|
||||||
capture_output=True, text=True, check=True, timeout=10
|
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',
|
def extract_surface_for_ref(ref: str, header: str = 'include/nlohmann/json.hpp',
|
||||||
include: str = 'include') -> dict:
|
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
|
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.
|
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.
|
these; snapshot_release.py uses them as a historical record's provenance.
|
||||||
"""
|
"""
|
||||||
with tempfile.TemporaryDirectory(prefix='api_checker_') as tmpdir:
|
with tempfile.TemporaryDirectory(prefix='api_checker_') as tmpdir:
|
||||||
archive = subprocess.run(
|
archive = subprocess.run( # nosec B603 B607
|
||||||
['git', 'archive', ref, '--', 'include'],
|
['git', 'archive', ref, '--', 'include'],
|
||||||
capture_output=True, check=True
|
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_include = os.path.join(tmpdir, include)
|
||||||
ref_header = os.path.join(tmpdir, header)
|
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)
|
sys.exit(1)
|
||||||
|
|
||||||
surface_output = os.path.join(tmpdir, 'surface.json')
|
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'),
|
[sys.executable, os.path.join(SCRIPT_DIR, 'extract_api.py'),
|
||||||
'--header', ref_header,
|
'--header', ref_header,
|
||||||
'--include', ref_include,
|
'--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:
|
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
|
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
|
extract_api.py's identity_key() joins into one opaque string internally, exposed here as
|
||||||
|
|||||||
@@ -1,5 +1,6 @@
|
|||||||
#!/usr/bin/env python3
|
#!/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,
|
This tool derives the public API from C++ semantics (class templates, access specifiers,
|
||||||
namespace scoping) independently of documentation status. The extracted surface is the
|
namespace scoping) independently of documentation status. The extracted surface is the
|
||||||
@@ -23,7 +24,8 @@ import argparse
|
|||||||
import json
|
import json
|
||||||
import os
|
import os
|
||||||
import re
|
import re
|
||||||
import subprocess
|
# subprocess is only called with fixed argument lists, never through a shell.
|
||||||
|
import subprocess # nosec B404
|
||||||
import sys
|
import sys
|
||||||
|
|
||||||
try:
|
try:
|
||||||
@@ -69,10 +71,13 @@ def _read_file_cached(path: str) -> str:
|
|||||||
|
|
||||||
|
|
||||||
def get_signature_text(cursor) -> 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
|
Return the normalized source text of a declaration's own signature.
|
||||||
the function body ('{') or at the terminating ';'/'= default;'/'= 0;'. Never includes the
|
|
||||||
body, so implementation-only changes don't affect identity.
|
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():
|
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
|
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:
|
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
|
CONSTRUCTOR/DESTRUCTOR cursors, FUNCTION_TEMPLATE cursors that are themselves templated
|
||||||
constructors (e.g. `template<typename CompatibleType> basic_json(CompatibleType&& val)`,
|
constructors (e.g. `template<typename CompatibleType> basic_json(CompatibleType&& val)`,
|
||||||
which libclang represents as FUNCTION_TEMPLATE, not CONSTRUCTOR), or CONVERSION_FUNCTION
|
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:
|
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
|
Return a stable, overload-disambiguating identity key for a cursor.
|
||||||
tried and found broken:
|
|
||||||
|
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
|
1. {scope, name, kind, params} from cursor.get_arguments() alone: silently collided for
|
||||||
overload sets differentiated only by constness, ref-qualifiers, or SFINAE constraints
|
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():
|
def get_repo_root():
|
||||||
"""Find the repository root via git, so location paths are deterministic regardless of invoking CWD."""
|
"""Find the repository root via git, so location paths are deterministic regardless of invoking CWD."""
|
||||||
try:
|
try:
|
||||||
result = subprocess.run(
|
result = subprocess.run( # nosec B603 B607
|
||||||
['git', 'rev-parse', '--show-toplevel'],
|
['git', 'rev-parse', '--show-toplevel'],
|
||||||
capture_output=True, text=True, check=True, timeout=10
|
capture_output=True, text=True, check=True, timeout=10
|
||||||
)
|
)
|
||||||
@@ -278,7 +288,7 @@ def cursor_location(cursor) -> str:
|
|||||||
def get_system_includes(compiler='clang++'):
|
def get_system_includes(compiler='clang++'):
|
||||||
"""Discover system include paths by parsing clang++ -E -x c++ -v /dev/null output."""
|
"""Discover system include paths by parsing clang++ -E -x c++ -v /dev/null output."""
|
||||||
try:
|
try:
|
||||||
result = subprocess.run(
|
result = subprocess.run( # nosec B603
|
||||||
[compiler, '-E', '-x', 'c++', '-v', '/dev/null'],
|
[compiler, '-E', '-x', 'c++', '-v', '/dev/null'],
|
||||||
capture_output=True, text=True, check=True, timeout=10
|
capture_output=True, text=True, check=True, timeout=10
|
||||||
)
|
)
|
||||||
@@ -320,7 +330,8 @@ def setup_libclang():
|
|||||||
try:
|
try:
|
||||||
cindex.conf.set_library_file(path)
|
cindex.conf.set_library_file(path)
|
||||||
return True
|
return True
|
||||||
except Exception:
|
except Exception: # nosec B112
|
||||||
|
# libclang rejected this candidate; try the next one.
|
||||||
continue
|
continue
|
||||||
|
|
||||||
try:
|
try:
|
||||||
@@ -328,7 +339,8 @@ def setup_libclang():
|
|||||||
if filename:
|
if filename:
|
||||||
cindex.conf.set_library_file(filename)
|
cindex.conf.set_library_file(filename)
|
||||||
return True
|
return True
|
||||||
except Exception:
|
except Exception: # nosec B110
|
||||||
|
# No usable default library; the caller reports the failure.
|
||||||
pass
|
pass
|
||||||
|
|
||||||
return False
|
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):
|
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):
|
if cursor.kind not in (cindex.CursorKind.CLASS_TEMPLATE, cindex.CursorKind.CLASS_DECL, cindex.CursorKind.STRUCT_DECL):
|
||||||
return
|
return
|
||||||
if cursor.spelling not in public_classes or not cursor.is_definition():
|
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)
|
doc_url = extract_sa_url(underlying_decl.raw_comment)
|
||||||
if underlying_decl.location.file:
|
if underlying_decl.location.file:
|
||||||
underlying_location = cursor_location(underlying_decl)
|
underlying_location = cursor_location(underlying_decl)
|
||||||
except Exception:
|
except Exception: # nosec B110
|
||||||
|
# Alias without a resolvable declaration: no @sa to follow.
|
||||||
pass
|
pass
|
||||||
|
|
||||||
key = identity_key(child, scope)
|
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):
|
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,
|
Write the minimal, location/doc-independent API surface.
|
||||||
pretty_signature) -- no opaque joined key, no location, no doc_url.
|
|
||||||
|
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
|
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
|
tools/api_checker/README.md and POLICY.md): 'signature'/'identity_name' are the same values
|
||||||
|
|||||||
@@ -1,5 +1,6 @@
|
|||||||
#!/usr/bin/env python3
|
#!/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:
|
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/<tag>.json, commit
|
once a release is tagged, run this once to capture tools/api_checker/history/<tag>.json, commit
|
||||||
@@ -13,7 +14,8 @@ import argparse
|
|||||||
import datetime
|
import datetime
|
||||||
import json
|
import json
|
||||||
import os
|
import os
|
||||||
import subprocess
|
# subprocess is only called with fixed argument lists, never through a shell.
|
||||||
|
import subprocess # nosec B404
|
||||||
import sys
|
import sys
|
||||||
|
|
||||||
SCRIPT_DIR = os.path.dirname(os.path.abspath(__file__))
|
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:
|
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)."""
|
List every v3.* git tag, sorted by dotted version.
|
||||||
result = subprocess.run(['git', 'tag', '--list', 'v3.*'], capture_output=True, text=True, check=True)
|
|
||||||
|
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()]
|
tags = [t for t in result.stdout.splitlines() if t.strip()]
|
||||||
|
|
||||||
def version_key(tag):
|
def version_key(tag):
|
||||||
|
|||||||
Reference in new issue
Block a user