Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

format_signatures: Fix whitespace issues and add tests #381

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion noxfile.py
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down
104 changes: 98 additions & 6 deletions sphinx_immaterial/apidoc/format_signatures.py
Original file line number Diff line number Diff line change
Expand Up @@ -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[:]
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would this be better?

Suggested change
del new_node[:]
new_node.children.clear()

I think the current change would delete children and attributes, no?

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
Expand Down Expand Up @@ -185,6 +188,36 @@ 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]:
first_name_text_node: Optional[docutils.nodes.Text] = None
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
):
first_name_text_node = text_node
break
if first_name_text_node is not None:
break
if first_name_text_node is not None:
break
return first_name_text_node
Comment on lines +197 to +218
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why all the breaks? Seems like a simple return makes sense.

Suggested change
first_name_text_node: Optional[docutils.nodes.Text] = None
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
):
first_name_text_node = text_node
break
if first_name_text_node is not None:
break
if first_name_text_node is not None:
break
return first_name_text_node
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]
Expand Down Expand Up @@ -244,12 +277,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<ptrdiff_t, N>
# tensorstore::GetStaticOrDynamicExtent(span<X, N>);
#
# 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
Expand Down
64 changes: 64 additions & 0 deletions tests/format_signatures_test.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
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 <typename T, \
typename U = void, \
int AnotherParameter = 42> \
requires std::is_const_v<T> \
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<ptrdiff_t, N> tensorstore::GetStaticOrDynamicExtent(span<X, N>);",
"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")
snapshot.assert_match(node.pformat(), f"{identifier}_pformat.txt")
2 changes: 2 additions & 0 deletions tests/requirements.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
void foo(int something, int something_else, bool third_param,
bool fourth_param, int fifth_param);
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
template <typename T, typename U = void, int AnotherParameter = 42>
requires std::is_const_v<T>
const MyType
LongFunctionSignatureExample(const MyType bar, uint8_t *arr,
unsigned int len = DEFAULT_LENGTH,
bool baz = false);
Original file line number Diff line number Diff line change
@@ -0,0 +1,165 @@
<desc_signature _toc_name="LongFunctionSignatureExample()" _toc_parts="('LongFunctionSignatureExample',)" classes="sig sig-object highlight cpp" ids="_CPPv4I00_iEIQNSt10is_const_vI1TEEE28LongFunctionSignatureExampleK6MyTypeK6MyTypeP7uint8_tjb" is_multiline="False" multi_line_parameter_list="False" signature_format_id="353e40c43b4168678988580cfda2596c">
<desc_sig_keyword classes="k">
template

<desc_sig_punctuation classes="p">
<
<desc_cpp_template_param>
<desc_sig_keyword classes="k">
typename
<desc_sig_space classes="w">

<desc_name classes="sig-name descname sig-name-nonprimary" xml:space="preserve">
<reference classes="desctype n" internal="True" refid="_CPPv4I00_iEIQNSt10is_const_vI1TEEE28LongFunctionSignatureExampleK6MyTypeK6MyTypeP7uint8_tjb" reftitle="LongFunctionSignatureExample::T (C++ type template parameter)">
<desc_sig_name classes="n">
T
<desc_sig_punctuation classes="p">
,
<desc_sig_space classes="w">

<desc_cpp_template_param>
<desc_sig_keyword classes="k">
typename
<desc_sig_space classes="w">

<desc_name classes="sig-name descname sig-name-nonprimary" xml:space="preserve">
<reference classes="desctype n" internal="True" refid="_CPPv4I00_iEIQNSt10is_const_vI1TEEE28LongFunctionSignatureExampleK6MyTypeK6MyTypeP7uint8_tjb" reftitle="LongFunctionSignatureExample::U (C++ type template parameter)">
<desc_sig_name classes="n">
U
<desc_sig_space classes="w">

<desc_sig_punctuation classes="p">
=
<desc_sig_space classes="w">

<desc_sig_keyword_type classes="kt">
void
<desc_sig_punctuation classes="p">
,
<desc_sig_space classes="w">

<desc_cpp_template_param>
<desc_sig_keyword_type classes="kt">
int
<desc_sig_space classes="w">

<desc_name classes="sig-name descname sig-name-nonprimary" xml:space="preserve">
<reference classes="n" internal="True" refid="_CPPv4I00_iEIQNSt10is_const_vI1TEEE28LongFunctionSignatureExampleK6MyTypeK6MyTypeP7uint8_tjb" reftitle="LongFunctionSignatureExample::AnotherParameter (C++ non-type template parameter)">
<desc_sig_name classes="n">
AnotherParameter
<desc_sig_space classes="w">

<desc_sig_punctuation classes="p">
=
<desc_sig_space classes="w">

<desc_sig_literal_number classes="m">
42
<desc_sig_punctuation classes="p">
>


<desc_cpp_requires_clause>
<desc_sig_keyword classes="k">
requires
<desc_sig_space classes="w">

<desc_sig_name classes="n">
std
<desc_sig_punctuation classes="p">
::
<desc_sig_name classes="n">
is_const_v
<desc_sig_punctuation classes="p">
<
<reference classes="desctype" internal="True" refid="_CPPv4I00_iEIQNSt10is_const_vI1TEEE28LongFunctionSignatureExampleK6MyTypeK6MyTypeP7uint8_tjb" reftitle="LongFunctionSignatureExample::T (C++ type template parameter)">
<desc_sig_name classes="n">
T
<desc_sig_punctuation classes="p">
>

<desc_sig_keyword classes="k">
const
<desc_sig_space classes="w">

<desc_sig_name classes="n">
MyType
<desc_sig_space classes="w">

<desc_name classes="sig-name descname" xml:space="preserve">
<desc_sig_name classes="n">
LongFunctionSignatureExample
<desc_sig_punctuation classes="p">
(
<inline>
<desc_sig_keyword classes="k">
const
<desc_sig_space classes="w">

<desc_sig_name classes="n">
MyType
<desc_sig_space classes="w">

<reference classes="n sig-param" internal="True" refid="_CPPv4I00_iEIQNSt10is_const_vI1TEEE28LongFunctionSignatureExampleK6MyTypeK6MyTypeP7uint8_tjb" reftitle="LongFunctionSignatureExample::bar (C++ function parameter)">
<desc_sig_name classes="n sig-param">
bar
<desc_sig_punctuation classes="p">
,

<inline>
<desc_sig_name classes="n">
uint8_t
<desc_sig_space classes="w">

<desc_sig_punctuation classes="p">
*
<reference classes="n sig-param" internal="True" refid="_CPPv4I00_iEIQNSt10is_const_vI1TEEE28LongFunctionSignatureExampleK6MyTypeK6MyTypeP7uint8_tjb" reftitle="LongFunctionSignatureExample::arr (C++ function parameter)">
<desc_sig_name classes="n sig-param">
arr
<desc_sig_punctuation classes="p">
,


<inline>
<desc_sig_keyword_type classes="kt">
unsigned
<desc_sig_space classes="w">

<desc_sig_keyword_type classes="kt">
int
<desc_sig_space classes="w">

<reference classes="n sig-param" internal="True" refid="_CPPv4I00_iEIQNSt10is_const_vI1TEEE28LongFunctionSignatureExampleK6MyTypeK6MyTypeP7uint8_tjb" reftitle="LongFunctionSignatureExample::len (C++ function parameter)">
<desc_sig_name classes="n sig-param">
len
<desc_sig_space classes="w">

<desc_sig_punctuation classes="p">
=
<desc_sig_space classes="w">

<desc_sig_name classes="n">
DEFAULT_LENGTH
<desc_sig_punctuation classes="p">
,


<inline>
<desc_sig_keyword_type classes="kt">
bool
<desc_sig_space classes="w">

<reference classes="n sig-param" internal="True" refid="_CPPv4I00_iEIQNSt10is_const_vI1TEEE28LongFunctionSignatureExampleK6MyTypeK6MyTypeP7uint8_tjb" reftitle="LongFunctionSignatureExample::baz (C++ function parameter)">
<desc_sig_name classes="n sig-param">
baz
<desc_sig_space classes="w">

<desc_sig_punctuation classes="p">
=
<desc_sig_space classes="w">

<desc_sig_keyword classes="k">
false
<desc_sig_punctuation classes="p">
)
;
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
std::integral_constant<ptrdiff_t, N>
tensorstore::GetStaticOrDynamicExtent(span<X, N>);
Loading
Loading