Fix: no trailing comma in multi-line signatures by default

- C and C++ domains will never feature a trailing comma as it'd correspond to incorrect syntax.
- Python and JS get a new config value to optionally disable the trailing comma

sphinx/addnodes.py

@@ -243,7 +243,8 @@ class desc_parameterlist(nodes.Part, nodes.Inline, nodes.FixedTextElement):
 
     As default the parameter list is written in line with the rest of the signature.
     Set ``multi_line_parameter_list = True`` to describe a multi-line parameter list.
-    In that case each parameter will then be written on its own, indented line.
+    In that case each parameter will then be written on its own, indented line. A
+    trailing comma will be added on the last line if ``trailing_comma`` is set to True.
     """
 
     child_text_separator = ', '
@@ -257,7 +258,8 @@ class desc_type_parameter_list(nodes.Part, nodes.Inline, nodes.FixedTextElement)
 
     As default the type parameters list is written in line with the rest of the signature.
     Set ``multi_line_parameter_list = True`` to describe a multi-line type parameters list.
-    In that case each type parameter will then be written on its own, indented line.
+    In that case each type parameter will then be written on its own, indented line. A
+    trailing comma will be added on the last line if ``trailing_comma`` is set to True.
     """
 
     child_text_separator = ', '

sphinx/domains/javascript.py

@@ -108,6 +108,8 @@ def handle_signature(self, sig: str, signode: desc_signature) -> tuple[str, str]
             and (len(sig) > max_len > 0)
         )
 
+        trailing_comma = self.env.config.javascript_trailing_comma_in_multi_line_signatures
+
         display_prefix = self.get_display_prefix()
         if display_prefix:
             signode += addnodes.desc_annotation('', '', *display_prefix)
@@ -128,7 +130,12 @@ def handle_signature(self, sig: str, signode: desc_signature) -> tuple[str, str]
             if not arglist:
                 signode += addnodes.desc_parameterlist()
             else:
-                _pseudo_parse_arglist(signode, arglist, multi_line_parameter_list)
+                _pseudo_parse_arglist(
+                    signode,
+                    arglist,
+                    multi_line_parameter_list,
+                    trailing_comma,
+                )
         return fullname, prefix
 
     def _object_hierarchy_parts(self, sig_node: desc_signature) -> tuple[str, ...]:
@@ -505,6 +512,9 @@ def setup(app: Sphinx) -> ExtensionMetadata:
     app.add_config_value(
         'javascript_maximum_signature_line_length', None, 'env', {int, type(None)},
     )
+    app.add_config_value(
+        'javascript_trailing_comma_in_multi_line_signatures', True, 'env',
+    )
     return {
         'version': 'builtin',
         'env_version': 3,

sphinx/domains/python/__init__.py

@@ -935,6 +935,9 @@ def setup(app: Sphinx) -> ExtensionMetadata:
     app.add_config_value(
         'python_maximum_signature_line_length', None, 'env', {int, type(None)},
     )
+    app.add_config_value(
+        'python_trailing_comma_in_multi_line_signatures', True, 'env',
+    )
     app.add_config_value('python_display_short_literal_types', False, 'env')
     app.connect('object-description-transform', filter_meta_fields)
     app.connect('missing-reference', builtin_resolver, priority=900)

sphinx/domains/python/_annotations.py

@@ -365,10 +365,12 @@ def _pformat_token(self, tok: Token, native: bool = False) -> str:
 def _parse_type_list(
     tp_list: str, env: BuildEnvironment,
     multi_line_parameter_list: bool = False,
+    trailing_comma: bool = True,
 ) -> addnodes.desc_type_parameter_list:
     """Parse a list of type parameters according to PEP 695."""
     type_params = addnodes.desc_type_parameter_list(tp_list)
     type_params['multi_line_parameter_list'] = multi_line_parameter_list
+    type_params['trailing_comma'] = trailing_comma
     # formal parameter names are interpreted as type parameter names and
     # type annotations are interpreted as type parameter bound or constraints
     parser = _TypeParameterListParser(tp_list)
@@ -425,11 +427,14 @@ def _parse_type_list(
 
 
 def _parse_arglist(
-    arglist: str, env: BuildEnvironment, multi_line_parameter_list: bool = False,
+    arglist: str, env: BuildEnvironment,
+    multi_line_parameter_list: bool = False,
+    trailing_comma: bool = True,
 ) -> addnodes.desc_parameterlist:
     """Parse a list of arguments using AST parser"""
     params = addnodes.desc_parameterlist(arglist)
     params['multi_line_parameter_list'] = multi_line_parameter_list
+    params['trailing_comma'] = trailing_comma
     sig = signature_from_str('(%s)' % arglist)
     last_kind = None
     for param in sig.parameters.values():
@@ -478,7 +483,9 @@ def _parse_arglist(
 
 
 def _pseudo_parse_arglist(
-    signode: desc_signature, arglist: str, multi_line_parameter_list: bool = False,
+    signode: desc_signature, arglist: str,
+    multi_line_parameter_list: bool = False,
+    trailing_comma: bool = True,
 ) -> None:
     """"Parse" a list of arguments separated by commas.
 
@@ -488,6 +495,7 @@ def _pseudo_parse_arglist(
     """
     paramlist = addnodes.desc_parameterlist()
     paramlist['multi_line_parameter_list'] = multi_line_parameter_list
+    paramlist['trailing_comma'] = trailing_comma
     stack: list[Element] = [paramlist]
     try:
         for argument in arglist.split(','):

sphinx/domains/python/_object.py

@@ -258,6 +258,7 @@ def handle_signature(self, sig: str, signode: desc_signature) -> tuple[str, str]
             and (sig_len - (arglist_span[1] - arglist_span[0])) > max_len > 0
         )
 
+        trailing_comma = self.env.config.python_trailing_comma_in_multi_line_signatures
         sig_prefix = self.get_signature_prefix(sig)
         if sig_prefix:
             if type(sig_prefix) is str:
@@ -277,25 +278,33 @@ def handle_signature(self, sig: str, signode: desc_signature) -> tuple[str, str]
 
         if tp_list:
             try:
-                signode += _parse_type_list(tp_list, self.env, multi_line_type_parameter_list)
+                signode += _parse_type_list(
+                    tp_list, self.env, multi_line_type_parameter_list, trailing_comma
+                )
             except Exception as exc:
                 logger.warning("could not parse tp_list (%r): %s", tp_list, exc,
                                location=signode)
 
         if arglist:
             try:
-                signode += _parse_arglist(arglist, self.env, multi_line_parameter_list)
+                signode += _parse_arglist(
+                    arglist, self.env, multi_line_parameter_list, trailing_comma
+                )
             except SyntaxError:
                 # fallback to parse arglist original parser
                 # (this may happen if the argument list is incorrectly used
                 # as a list of bases when documenting a class)
                 # it supports to represent optional arguments (ex. "func(foo [, bar])")
-                _pseudo_parse_arglist(signode, arglist, multi_line_parameter_list)
+                _pseudo_parse_arglist(
+                    signode, arglist, multi_line_parameter_list, trailing_comma
+                )
             except (NotImplementedError, ValueError) as exc:
                 # duplicated parameter names raise ValueError and not a SyntaxError
                 logger.warning("could not parse arglist (%r): %s", arglist, exc,
                                location=signode)
-                _pseudo_parse_arglist(signode, arglist, multi_line_parameter_list)
+                _pseudo_parse_arglist(
+                    signode, arglist, multi_line_parameter_list, trailing_comma
+                )
         else:
             if self.needs_arglist():
                 # for callables, add an empty parameter list

sphinx/writers/html5.py

@@ -173,6 +173,7 @@ def _visit_sig_parameter_list(
         self.required_params_left = sum(self.list_is_required_param)
         self.param_separator = node.child_text_separator
         self.multi_line_parameter_list = node.get('multi_line_parameter_list', False)
+        self.trailing_comma = node.get('trailing_comma', False)
         if self.multi_line_parameter_list:
             self.body.append('\n\n')
             self.body.append(self.starttag(node, 'dl'))
@@ -230,7 +231,8 @@ def depart_desc_parameter(self, node: Element) -> None:
             )
             opt_param_left_at_level = self.params_left_at_level > 0
             if opt_param_left_at_level or is_required and (is_last_group or next_is_required):
-                self.body.append(self.param_separator)
+                if not is_last_group or opt_param_left_at_level or self.trailing_comma:
+                    self.body.append(self.param_separator)
                 self.body.append('</dd>\n')
 
         elif self.required_params_left:
@@ -272,19 +274,25 @@ def visit_desc_optional(self, node: Element) -> None:
 
     def depart_desc_optional(self, node: Element) -> None:
         self.optional_param_level -= 1
+        level = self.optional_param_level
         if self.multi_line_parameter_list:
-            # If it's the first time we go down one level, add the separator
-            # before the bracket.
-            if self.optional_param_level == self.max_optional_param_level - 1:
+            max_level = self.max_optional_param_level
+            is_last_group = self.param_group_index + 1 == len(self.list_is_required_param)
+            # If it's the first time we go down one level, add the separator before the
+            # bracket, except if this is the last parameter and the parameter list
+            # should not feature a trailing comma.
+            if level == max_level - 1 and (
+                not is_last_group or level > 0 or self.trailing_comma
+            ):
                 self.body.append(self.param_separator)
             self.body.append('<span class="optional">]</span>')
             # End the line if we have just closed the last bracket of this
             # optional parameter group.
-            if self.optional_param_level == 0:
+            if level == 0:
                 self.body.append('</dd>\n')
         else:
             self.body.append('<span class="optional">]</span>')
-        if self.optional_param_level == 0:
+        if level == 0:
             self.param_group_index += 1
 
     def visit_desc_annotation(self, node: Element) -> None:

sphinx/writers/latex.py

@@ -875,6 +875,7 @@ def _visit_sig_parameter_list(self, node: Element, parameter_group: type[Element
         self.required_params_left = sum(self.list_is_required_param)
         self.param_separator = r'\sphinxparamcomma '
         self.multi_line_parameter_list = node.get('multi_line_parameter_list', False)
+        self.trailing_comma = node.get('trailing_comma', False)
 
     def visit_desc_parameterlist(self, node: Element) -> None:
         if self.has_tp_list:
@@ -930,7 +931,9 @@ def _depart_sig_parameter(self, node: Element) -> None:
                 and self.list_is_required_param[self.param_group_index + 1]
             )
             opt_param_left_at_level = self.params_left_at_level > 0
-            if opt_param_left_at_level or is_required and (is_last_group or next_is_required):
+            if opt_param_left_at_level or is_required and (
+                next_is_required or self.trailing_comma
+            ):
                 self.body.append(self.param_separator)
 
         elif self.required_params_left:
@@ -970,13 +973,19 @@ def visit_desc_optional(self, node: Element) -> None:
 
     def depart_desc_optional(self, node: Element) -> None:
         self.optional_param_level -= 1
+        level = self.optional_param_level
         if self.multi_line_parameter_list:
+            max_level = self.max_optional_param_level
+            is_last_group = self.param_group_index + 1 == len(self.list_is_required_param)
             # If it's the first time we go down one level, add the separator before the
-            # bracket.
-            if self.optional_param_level == self.max_optional_param_level - 1:
+            # bracket, except if this is the last parameter and the parameter list
+            # should not feature a trailing comma.
+            if level == max_level - 1 and (
+                not is_last_group or level > 0 or self.trailing_comma
+            ):
                 self.body.append(self.param_separator)
         self.body.append('}')
-        if self.optional_param_level == 0:
+        if level == 0:
             self.param_group_index += 1
 
     def visit_desc_annotation(self, node: Element) -> None:

sphinx/writers/text.py

@@ -627,6 +627,7 @@ def _visit_sig_parameter_list(
         self.required_params_left = sum(self.list_is_required_param)
         self.param_separator = ', '
         self.multi_line_parameter_list = node.get('multi_line_parameter_list', False)
+        self.trailing_comma = node.get('trailing_comma', False)
         if self.multi_line_parameter_list:
             self.param_separator = self.param_separator.rstrip()
         self.context.append(sig_close_paren)
@@ -671,7 +672,8 @@ def visit_desc_parameter(self, node: Element) -> None:
             )
             opt_param_left_at_level = self.params_left_at_level > 0
             if opt_param_left_at_level or is_required and (is_last_group or next_is_required):
-                self.add_text(self.param_separator)
+                if not is_last_group or opt_param_left_at_level or self.trailing_comma:
+                    self.add_text(self.param_separator)
                 self.end_state(wrap=False, end=None)
 
         elif self.required_params_left:
@@ -711,20 +713,26 @@ def visit_desc_optional(self, node: Element) -> None:
 
     def depart_desc_optional(self, node: Element) -> None:
         self.optional_param_level -= 1
+        level = self.optional_param_level
         if self.multi_line_parameter_list:
+            max_level = self.max_optional_param_level
+            is_last_group = self.param_group_index + 1 == len(self.list_is_required_param)
             # If it's the first time we go down one level, add the separator before the
-            # bracket.
-            if self.optional_param_level == self.max_optional_param_level - 1:
+            # bracket, except if this is the last parameter and the parameter list
+            # should not feature a trailing comma.
+            if level == max_level - 1 and (
+                not is_last_group or level > 0 or self.trailing_comma
+            ):
                 self.add_text(self.param_separator)
             self.add_text(']')
             # End the line if we have just closed the last bracket of this group of
             # optional parameters.
-            if self.optional_param_level == 0:
+            if level == 0:
                 self.end_state(wrap=False, end=None)
 
         else:
             self.add_text(']')
-        if self.optional_param_level == 0:
+        if level == 0:
             self.param_group_index += 1
 
     def visit_desc_annotation(self, node: Element) -> None:

tests/test_builders/test_build_latex.py

@@ -2224,7 +2224,6 @@ def test_one_parameter_per_line(app):
     app.build(force_all=True)
     result = (app.outdir / 'projectnamenotset.tex').read_text(encoding='utf8')
 
-    # TODO: should these asserts check presence or absence of a final \sphinxparamcomma?
     # signature of 23 characters is too short to trigger one-param-per-line mark-up
     assert (
         '\\pysiglinewithargsret\n'
@@ -2237,6 +2236,7 @@ def test_one_parameter_per_line(app):
         '{\\sphinxbfcode{\\sphinxupquote{foo}}}\n'
         '{\\sphinxoptional{\\sphinxparam{' in result
     )
+    assert r'\sphinxparam{\DUrole{n}{f}}\sphinxparamcomma' in result
 
     # generic_arg[T]
     assert (
@@ -2293,6 +2293,21 @@ def test_one_parameter_per_line(app):
     )
 
 
+@pytest.mark.sphinx(
+    'latex',
+    testroot='domain-py-python_maximum_signature_line_length',
+    confoverrides={
+        'python_maximum_signature_line_length': 23,
+        'python_trailing_comma_in_multi_line_signatures': False,
+    },
+)
+def test_one_parameter_per_line_without_trailing_comma(app):
+    app.build(force_all=True)
+    result = (app.outdir / 'projectnamenotset.tex').read_text(encoding='utf8')
+
+    assert r'\sphinxparam{\DUrole{n}{f}}}}' in result
+
+
 @pytest.mark.sphinx('latex', testroot='markup-rubric')
 def test_latex_rubric(app):
     app.build()

tests/test_domains/test_domain_c.py

@@ -1370,7 +1370,7 @@ def test_domain_c_c_maximum_signature_line_length_in_html(app):
 <dd>\
 <span class="n"><span class="pre">str</span></span>\
 <span class="w"> </span>\
-<span class="n"><span class="pre">name</span></span>,\
+<span class="n"><span class="pre">name</span></span>\
 </dd>
 </dl>
 
@@ -1391,6 +1391,6 @@ def test_domain_c_c_maximum_signature_line_length_in_text(app):
     content = (app.outdir / 'index.txt').read_text(encoding='utf8')
     param_line_fmt = STDINDENT * ' ' + '{}\n'
 
-    expected_parameter_list_hello = '(\n{})'.format(param_line_fmt.format('str name,'))
+    expected_parameter_list_hello = '(\n{})'.format(param_line_fmt.format('str name'))
 
     assert expected_parameter_list_hello in content

tests/test_domains/test_domain_cpp.py

@@ -2422,7 +2422,7 @@ def test_domain_cpp_cpp_maximum_signature_line_length_in_html(app):
 <dd>\
 <span class="n"><span class="pre">str</span></span>\
 <span class="w"> </span>\
-<span class="n sig-param"><span class="pre">name</span></span>,\
+<span class="n sig-param"><span class="pre">name</span></span>\
 </dd>
 </dl>
 
@@ -2441,6 +2441,6 @@ def test_domain_cpp_cpp_maximum_signature_line_length_in_text(app):
     content = (app.outdir / 'index.txt').read_text(encoding='utf8')
     param_line_fmt = STDINDENT * ' ' + '{}\n'
 
-    expected_parameter_list_hello = '(\n{})'.format(param_line_fmt.format('str name,'))
+    expected_parameter_list_hello = '(\n{})'.format(param_line_fmt.format('str name'))
 
     assert expected_parameter_list_hello in content