From 3c5d985af74b03d4fa3a7048284c36562cbe325f Mon Sep 17 00:00:00 2001 From: Jeremy Maitin-Shepard Date: Thu, 3 Oct 2024 22:54:23 -0700 Subject: [PATCH] format_signatures: Fix whitespace issues and add tests (#381) * format_signatures: Fix whitespace issues and add tests * Update sphinx_immaterial/apidoc/format_signatures.py Co-authored-by: Brendan <2bndy5@gmail.com> * Exclude pformat snapshots since they aren't consistent across Sphinx/docutils versions --------- Co-authored-by: Brendan <2bndy5@gmail.com> --- noxfile.py | 2 +- sphinx_immaterial/apidoc/format_signatures.py | 98 +++++++++++++++++-- tests/format_signatures_test.py | 63 ++++++++++++ tests/requirements.txt | 2 + .../cpp_function_astext.txt | 2 + .../cpp_function_long_astext.txt | 6 ++ .../cpp_function_long_return_type_astext.txt | 2 + .../py_function_astext.txt | 9 ++ 8 files changed, 177 insertions(+), 7 deletions(-) create mode 100644 tests/format_signatures_test.py create mode 100644 tests/snapshots/format_signatures_test/test_format_signatures/cpp_function_astext.txt create mode 100644 tests/snapshots/format_signatures_test/test_format_signatures/cpp_function_long_astext.txt create mode 100644 tests/snapshots/format_signatures_test/test_format_signatures/cpp_function_long_return_type_astext.txt create mode 100644 tests/snapshots/format_signatures_test/test_format_signatures/py_function_astext.txt diff --git a/noxfile.py b/noxfile.py index a77906a0..3736f034 100644 --- a/noxfile.py +++ b/noxfile.py @@ -136,7 +136,7 @@ def check_eof(session: nox.Session): def check_trailing_space(session: nox.Session): """Ensure no trailing whitespace.""" session.install("-r", "requirements/dev-pre_commit_hooks.txt") - all_files = get_file_list("*") + all_files = get_file_list("*", exclude=re.compile("tests/snapshots/.*")) # error output is super long and unhelpful; we only need the stdout in case of error ret = session.run( "trailing-whitespace-fixer", *all_files, silent=True, success_codes=[0, 1] diff --git a/sphinx_immaterial/apidoc/format_signatures.py b/sphinx_immaterial/apidoc/format_signatures.py index e0ddc49c..e4fb5345 100644 --- a/sphinx_immaterial/apidoc/format_signatures.py +++ b/sphinx_immaterial/apidoc/format_signatures.py @@ -151,6 +151,9 @@ def _transform_node(node): if isinstance(node, docutils.nodes.Text): return replacements.get(id(node)) or [] new_node = node.copy() + # Ensure the new node does not contain any children. `desc_sig_space`, + # for example, adds a default child text node with content " ". + del new_node[:] new_node.extend(_transform_node_list(node.children, _transform_node)) if len(new_node.children) == 0: # A pending_xref with no children is an error and indicates a @@ -185,6 +188,30 @@ class FormatInputComponent(NamedTuple): """ +_INDENTED_LINE_PATTERN = re.compile(r".*(?:^|\n)([ \t]+)$", re.DOTALL) + + +def _find_first_primary_entity_name_text_node( + adjusted_nodes: list[docutils.nodes.Node], +) -> Optional[docutils.nodes.Text]: + for adjusted_node in adjusted_nodes: + node: docutils.nodes.Element + for node in adjusted_node.findall( + lambda node: ( + isinstance(node, sphinx.addnodes.desc_addname) + or ( + isinstance(node, sphinx.addnodes.desc_name) + and "sig-name-nonprimary" not in node["classes"] + ) + ) + ): + for text_node in cast(docutils.nodes.Element, node).findall( + docutils.nodes.Text + ): + return text_node + return None + + class FormatApplier: components: list[FormatInputComponent] adjusted_nodes: list[docutils.nodes.Node] @@ -244,12 +271,71 @@ def apply( replacements: dict[int, list[docutils.nodes.Node]] = collections.defaultdict( list ) - for tag, i1, i2, j1, j2 in difflib.SequenceMatcher( - " \t\n".__contains__, - formatted_input, - formatted_output, - autojunk=False, - ).get_opcodes(): + + def _get_opcodes(): + return difflib.SequenceMatcher( + " \t\n".__contains__, + formatted_input, + formatted_output, + autojunk=False, + ).get_opcodes() + + # Align `formatted_input` to `formatted_output`. + opcodes = _get_opcodes() + + # Find the first text node of the entity name. + # + # Sometimes the formatter may indent the primary entity name, e.g.: + # + # std::integral_constant + # tensorstore::GetStaticOrDynamicExtent(span); + # + # where the primary entity name is `tensorstore::GetStaticOrDynamicExtent`. + # + # This is not desirable for API documentation, and therefore is stripped + # out. + # + # To check for and remove such indentation, first locate the Text node + # corresponding to the start of the primary entity name. + first_name_text_node = _find_first_primary_entity_name_text_node( + self.adjusted_nodes + ) + + # Then determine the offests into `formatted_input` and + # `formatted_output` corresponding to `first_name_text_node`. + first_name_text_node_input_offset = 0 + for component in components: + if component.node is first_name_text_node: + break + first_name_text_node_input_offset += len(component.orig_text) + + # Determine output offset of `first_name_text_node_input_offset`. + first_name_text_node_output_offset = 0 + for tag, i1, i2, j1, j2 in opcodes: + if i2 >= first_name_text_node_input_offset: + if tag == "equal": + first_name_text_node_output_offset = j1 + ( + first_name_text_node_input_offset - i1 + ) + else: + first_name_text_node_output_offset = j1 + break + + # Check if `first_name_text_node` is indented on a new line. + if ( + m := _INDENTED_LINE_PATTERN.fullmatch( + formatted_output, 0, first_name_text_node_output_offset + ) + ) is not None: + # Strip leading whitespace, and recompute opcodes. + formatted_output = ( + formatted_output[: m.start(1)] + + formatted_output[first_name_text_node_output_offset:] + ) + opcodes = _get_opcodes() + + # Compute the replacement text nodes for each component. + for tag, i1, i2, j1, j2 in opcodes: while True: if i1 != i2 and i1 == component_end_offset: component_start_offset = component_end_offset diff --git a/tests/format_signatures_test.py b/tests/format_signatures_test.py new file mode 100644 index 00000000..83e97075 --- /dev/null +++ b/tests/format_signatures_test.py @@ -0,0 +1,63 @@ +import sphinx.addnodes + + +TEST_SIGNATURES = { + "cpp_function": "cpp:function:: void foo(int something, int something_else, bool third_param, bool fourth_param, int fifth_param)", + "cpp_function_long": r"cpp:function:: template \ + requires std::is_const_v \ + const MyType LongFunctionSignatureExample(\ + const MyType bar, \ + uint8_t* arr, \ + unsigned int len = DEFAULT_LENGTH, \ + bool baz = false)", + "cpp_function_long_return_type": r"cpp:function:: std::integral_constant tensorstore::GetStaticOrDynamicExtent(span);", + "py_function": r"py:function:: some_module.method_name( \ + some_parameter_with_a_long_name: \ + collections.abc.MutableMapping[\ + tuple[str, float, numbers.Real], \ + dict[int, tuple[list[frozenset[int]]]]], \ + ) -> collections.abc.MutableMapping[\ + tuple[str, float, numbers.Real], \ + dict[int, tuple[list[frozenset[int]]]]]", +} + + +def test_format_signatures(immaterial_make_app, snapshot): + app = immaterial_make_app( + extra_conf=""" +extensions.append("sphinx_immaterial.apidoc.format_signatures") +object_description_options = [ + ("cpp:.*", dict(clang_format_style={"BasedOnStyle": "LLVM"})), + ("py:.*", dict(black_format_style={})), +] + """, + files={ + "index.rst": "\n\n".join( + f""" +.. {directive} + + Synopsis goes here. +""" + for directive in TEST_SIGNATURES.values() + ) + }, + ) + + app.build() + + assert not app._warning.getvalue() + + doc = app.env.get_and_resolve_doctree("index", app.builder) + + formatted_signatures = { + identifier: signature + for identifier, signature in zip( + TEST_SIGNATURES.keys(), + doc.findall(condition=sphinx.addnodes.desc_signature), + ) + } + for identifier in TEST_SIGNATURES.keys(): + node = formatted_signatures[identifier] + snapshot.assert_match(node.astext(), f"{identifier}_astext.txt") diff --git a/tests/requirements.txt b/tests/requirements.txt index f3ba9860..37b4a690 100644 --- a/tests/requirements.txt +++ b/tests/requirements.txt @@ -10,3 +10,5 @@ tests/issue_134/sphinx-immaterial-pybind11-issue-134 -r ../requirements/json.txt -r ../requirements/jsonschema_validation.txt -r ../requirements/keys.txt +-r ../requirements/black.txt +-r ../requirements/clang-format.txt diff --git a/tests/snapshots/format_signatures_test/test_format_signatures/cpp_function_astext.txt b/tests/snapshots/format_signatures_test/test_format_signatures/cpp_function_astext.txt new file mode 100644 index 00000000..4b0694ca --- /dev/null +++ b/tests/snapshots/format_signatures_test/test_format_signatures/cpp_function_astext.txt @@ -0,0 +1,2 @@ +void foo(int something, int something_else, bool third_param, + bool fourth_param, int fifth_param); \ No newline at end of file diff --git a/tests/snapshots/format_signatures_test/test_format_signatures/cpp_function_long_astext.txt b/tests/snapshots/format_signatures_test/test_format_signatures/cpp_function_long_astext.txt new file mode 100644 index 00000000..3970e6c7 --- /dev/null +++ b/tests/snapshots/format_signatures_test/test_format_signatures/cpp_function_long_astext.txt @@ -0,0 +1,6 @@ +template + requires std::is_const_v +const MyType +LongFunctionSignatureExample(const MyType bar, uint8_t *arr, + unsigned int len = DEFAULT_LENGTH, + bool baz = false); \ No newline at end of file diff --git a/tests/snapshots/format_signatures_test/test_format_signatures/cpp_function_long_return_type_astext.txt b/tests/snapshots/format_signatures_test/test_format_signatures/cpp_function_long_return_type_astext.txt new file mode 100644 index 00000000..843ba369 --- /dev/null +++ b/tests/snapshots/format_signatures_test/test_format_signatures/cpp_function_long_return_type_astext.txt @@ -0,0 +1,2 @@ +std::integral_constant +tensorstore::GetStaticOrDynamicExtent(span); diff --git a/tests/snapshots/format_signatures_test/test_format_signatures/py_function_astext.txt b/tests/snapshots/format_signatures_test/test_format_signatures/py_function_astext.txt new file mode 100644 index 00000000..9bafd6b7 --- /dev/null +++ b/tests/snapshots/format_signatures_test/test_format_signatures/py_function_astext.txt @@ -0,0 +1,9 @@ +some_module.method_name( + some_parameter_with_a_long_name: collections.abc.MutableMapping[ + tuple[str, float, numbers.Real], + dict[int, tuple[list[frozenset[int]]]], + ] +) -> collections.abc.MutableMapping[ + tuple[str, float, numbers.Real], + dict[int, tuple[list[frozenset[int]]]], +] \ No newline at end of file