inspect.signature: Use '/' to separate positional-only parameters from

the rest in Signature.__str__. #20356

4 files changed, 63 insertions, 42 deletions

diff --git a/Doc/library/inspect.rst b/Doc/library/inspect.rst
index 0a3a571..91d8831 100644
--- a/Doc/library/inspect.rst
+++ b/Doc/library/inspect.rst
@@ -510,9 +510,8 @@ function.
 
    .. attribute:: Parameter.name
 
-      The name of the parameter as a string.  Must be a valid python identifier
-      name (with the exception of ``POSITIONAL_ONLY`` parameters, which can have
-      it set to ``None``).
+      The name of the parameter as a string.  The name must be a valid
+      Python identifier.
 
    .. attribute:: Parameter.default
 
@@ -596,6 +595,10 @@ function.
          >>> str(param.replace(default=Parameter.empty, annotation='spam'))
          "foo:'spam'"
 
+    .. versionchanged:: 3.4
+        In Python 3.3 Parameter objects were allowed to have ``name`` set
+        to ``None`` if their ``kind`` was set to ``POSITIONAL_ONLY``.
+        This is no longer permitted.
 
 .. class:: BoundArguments
 
diff --git a/Doc/whatsnew/3.4.rst b/Doc/whatsnew/3.4.rst
index 1d88965..4718339 100644
--- a/Doc/whatsnew/3.4.rst
+++ b/Doc/whatsnew/3.4.rst
@@ -1488,6 +1488,9 @@ removed:
 * Support for loading the deprecated ``TYPE_INT64`` has been removed from
   :mod:`marshal`.  (Contributed by Dan Riti in :issue:`15480`.)
 
+* :class:`inspect.Signature`: positional-only parameters are now required
+  to have a valid name.
+
 
 Code Cleanups
 -------------
diff --git a/Lib/inspect.py b/Lib/inspect.py
index 3599e02..15584c1 100644
--- a/Lib/inspect.py
+++ b/Lib/inspect.py
@@ -1629,17 +1629,16 @@ class Parameter:
         self._default = default
         self._annotation = annotation
 
-        if name is None:
-            if kind != _POSITIONAL_ONLY:
-                raise ValueError("None is not a valid name for a "
-                                 "non-positional-only parameter")
-            self._name = name
-        else:
-            name = str(name)
-            if kind != _POSITIONAL_ONLY and not name.isidentifier():
-                msg = '{!r} is not a valid parameter name'.format(name)
-                raise ValueError(msg)
-            self._name = name
+        if name is _empty:
+            raise ValueError('name is a required attribute for Parameter')
+
+        if not isinstance(name, str):
+            raise TypeError("name must be a str, not a {!r}".format(name))
+
+        if not name.isidentifier():
+            raise ValueError('{!r} is not a valid parameter name'.format(name))
+
+        self._name = name
 
         self._partial_kwarg = _partial_kwarg
 
@@ -1683,12 +1682,7 @@ class Parameter:
 
     def __str__(self):
         kind = self.kind
-
         formatted = self._name
-        if kind == _POSITIONAL_ONLY:
-            if formatted is None:
-                formatted = ''
-            formatted = '<{}>'.format(formatted)
 
         # Add annotation and default value
         if self._annotation is not _empty:
@@ -1858,21 +1852,19 @@ class Signature:
 
                 for idx, param in enumerate(parameters):
                     kind = param.kind
+                    name = param.name
+
                     if kind < top_kind:
                         msg = 'wrong parameter order: {} before {}'
-                        msg = msg.format(top_kind, param.kind)
+                        msg = msg.format(top_kind, kind)
                         raise ValueError(msg)
                     else:
                         top_kind = kind
 
-                    name = param.name
-                    if name is None:
-                        name = str(idx)
-                        param = param.replace(name=name)
-
                     if name in params:
                         msg = 'duplicate parameter name: {!r}'.format(name)
                         raise ValueError(msg)
+
                     params[name] = param
             else:
                 params = OrderedDict(((param.name, param)
@@ -2292,11 +2284,21 @@ class Signature:
 
     def __str__(self):
         result = []
+        render_pos_only_separator = False
         render_kw_only_separator = True
-        for idx, param in enumerate(self.parameters.values()):
+        for param in self.parameters.values():
             formatted = str(param)
 
             kind = param.kind
+
+            if kind == _POSITIONAL_ONLY:
+                render_pos_only_separator = True
+            elif render_pos_only_separator:
+                # It's not a positional-only parameter, and the flag
+                # is set to 'True' (there were pos-only params before.)
+                result.append('/')
+                render_pos_only_separator = False
+
             if kind == _VAR_POSITIONAL:
                 # OK, we have an '*args'-like parameter, so we won't need
                 # a '*' to separate keyword-only arguments
@@ -2312,6 +2314,11 @@ class Signature:
 
             result.append(formatted)
 
+        if render_pos_only_separator:
+            # There were only positional-only parameters, hence the
+            # flag was not reset to 'False'
+            result.append('/')
+
         rendered = '({})'.format(', '.join(result))
 
         if self.return_annotation is not _empty:
diff --git a/Lib/test/test_inspect.py b/Lib/test/test_inspect.py
index ec04c85..484e0dc 100644
--- a/Lib/test/test_inspect.py
+++ b/Lib/test/test_inspect.py
@@ -2122,6 +2122,7 @@ class TestSignatureObject(unittest.TestCase):
 
     def test_signature_str_positional_only(self):
         P = inspect.Parameter
+        S = inspect.Signature
 
         def test(a_po, *, b, **kwargs):
             return a_po, kwargs
@@ -2132,14 +2133,20 @@ class TestSignatureObject(unittest.TestCase):
         test.__signature__ = sig.replace(parameters=new_params)
 
         self.assertEqual(str(inspect.signature(test)),
-                         '(<a_po>, *, b, **kwargs)')
+                         '(a_po, /, *, b, **kwargs)')
 
-        sig = inspect.signature(test)
-        new_params = list(sig.parameters.values())
-        new_params[0] = new_params[0].replace(name=None)
-        test.__signature__ = sig.replace(parameters=new_params)
-        self.assertEqual(str(inspect.signature(test)),
-                         '(<0>, *, b, **kwargs)')
+        self.assertEqual(str(S(parameters=[P('foo', P.POSITIONAL_ONLY)])),
+                         '(foo, /)')
+
+        self.assertEqual(str(S(parameters=[
+                                P('foo', P.POSITIONAL_ONLY),
+                                P('bar', P.VAR_KEYWORD)])),
+                         '(foo, /, **bar)')
+
+        self.assertEqual(str(S(parameters=[
+                                P('foo', P.POSITIONAL_ONLY),
+                                P('bar', P.VAR_POSITIONAL)])),
+                         '(foo, /, *bar)')
 
     def test_signature_replace_anno(self):
         def test() -> 42:
@@ -2178,10 +2185,13 @@ class TestParameterObject(unittest.TestCase):
         with self.assertRaisesRegex(ValueError, 'not a valid parameter name'):
             inspect.Parameter('1', kind=inspect.Parameter.VAR_KEYWORD)
 
-        with self.assertRaisesRegex(ValueError,
-                                     'non-positional-only parameter'):
+        with self.assertRaisesRegex(TypeError, 'name must be a str'):
             inspect.Parameter(None, kind=inspect.Parameter.VAR_KEYWORD)
 
+        with self.assertRaisesRegex(ValueError,
+                                    'is not a valid parameter name'):
+            inspect.Parameter('$', kind=inspect.Parameter.VAR_KEYWORD)
+
         with self.assertRaisesRegex(ValueError, 'cannot have default values'):
             inspect.Parameter('a', default=42,
                               kind=inspect.Parameter.VAR_KEYWORD)
@@ -2230,7 +2240,8 @@ class TestParameterObject(unittest.TestCase):
         self.assertEqual(p2.name, 'bar')
         self.assertNotEqual(p2, p)
 
-        with self.assertRaisesRegex(ValueError, 'not a valid parameter name'):
+        with self.assertRaisesRegex(ValueError,
+                                    'name is a required attribute'):
             p2 = p2.replace(name=p2.empty)
 
         p2 = p2.replace(name='foo', default=None)
@@ -2252,14 +2263,11 @@ class TestParameterObject(unittest.TestCase):
         self.assertEqual(p2, p)
 
     def test_signature_parameter_positional_only(self):
-        p = inspect.Parameter(None, kind=inspect.Parameter.POSITIONAL_ONLY)
-        self.assertEqual(str(p), '<>')
-
-        p = p.replace(name='1')
-        self.assertEqual(str(p), '<1>')
+        with self.assertRaisesRegex(TypeError, 'name must be a str'):
+            inspect.Parameter(None, kind=inspect.Parameter.POSITIONAL_ONLY)
 
     def test_signature_parameter_immutability(self):
-        p = inspect.Parameter(None, kind=inspect.Parameter.POSITIONAL_ONLY)
+        p = inspect.Parameter('spam', kind=inspect.Parameter.KEYWORD_ONLY)
 
         with self.assertRaises(AttributeError):
             p.foo = 'bar'