Address code review

Let _format_astring() fall through to the generic TypeError for a bool
value instead of a special-case message (suggested by David Murray),
and move the substitution note back into the body rather than the
versionadded directive.

Co-authored-by: R. David Murray <rdmurray@bitdance.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

Author: 3 people

Doc/library/imaplib.rst

@@ -268,14 +268,14 @@ The placeholders are:
 
 ``??`` stands for a literal ``?``.
 
+Substitution is only performed when *params* is given;
+if no *params* are given, an argument containing a literal ``?`` is unchanged.
 The *params* keyword is accepted by :meth:`~IMAP4.search`,
 :meth:`~IMAP4.fetch`, :meth:`~IMAP4.sort`, :meth:`~IMAP4.thread` and
 :meth:`~IMAP4.uid`.
 
 .. versionadded:: next
    The *params* keyword argument.
-   Substitution is only performed when *params* is given,
-   so an existing call that contains a literal ``?`` is unaffected.
 
 An :class:`IMAP4` instance has the following methods:
 

Lib/imaplib.py

@@ -185,9 +185,7 @@ def _format_sequence_set(arg):
 def _format_astring(value):
     if isinstance(value, (list, tuple)):
         return '(' + ' '.join(map(_format_astring, value)) + ')'
-    if isinstance(value, bool):
-        raise TypeError('a boolean is not a valid IMAP4 string')
-    if isinstance(value, int):
+    if not isinstance(value, bool) and isinstance(value, int):
         return str(value)
     if isinstance(value, (bytes, bytearray)):
         value = str(value, 'ascii')
@@ -204,6 +202,8 @@ def _format_astring(value):
 
 def _format_flags(value):
     if isinstance(value, (list, tuple)):
+        # A nested sequence is not part of the API; it produces invalid
+        # syntax that is rejected by the server.
         return '(' + ' '.join(map(_format_flags, value)) + ')'
     if isinstance(value, (bytes, bytearray)):
         value = str(value, 'ascii')

Lib/test/test_imaplib.py

@@ -184,21 +184,21 @@ def test_astring(self):
         self.assertEqual(m._astring(b'INBOX'), b'INBOX')
         # Names with protocol-sensitive characters are quoted.
         self.assertEqual(m._astring('New folder'), b'"New folder"')
-        self.assertEqual(m._astring('a"b'), b'"a\\"b"')
-        self.assertEqual(m._astring('a\\b'), b'"a\\\\b"')
+        self.assertEqual(m._astring('a"b'), rb'"a\"b"')
+        self.assertEqual(m._astring(r'a\b'), rb'"a\\b"')
         self.assertEqual(m._astring(''), b'""')
         self.assertEqual(m._astring('*'), b'"*"')
         # A well-formed quoted string is passed through unchanged.
         self.assertEqual(m._astring('"New folder"'), b'"New folder"')
         self.assertEqual(m._astring('""'), b'""')
         # Including a lenient (non-RFC) backslash escape, which the server
         # may accept.
-        self.assertEqual(m._astring('"a\\b"'), b'"a\\b"')
+        self.assertEqual(m._astring(r'"a\b"'), rb'"a\b"')
         # A string that only looks quoted but is not a single token is
         # quoted as data, closing the argument injection vector.
         self.assertEqual(m._astring('"a" SELECT evil "'),
-                         b'"\\"a\\" SELECT evil \\""')
-        self.assertEqual(m._astring('"'), b'"\\""')
+                         rb'"\"a\" SELECT evil \""')
+        self.assertEqual(m._astring('"'), rb'"\""')
         # Non-ASCII names are only allowed in a quoted string or a
         # literal, never in an atom (RFC 6855).
         m._encoding = 'utf-8'
@@ -344,6 +344,11 @@ def test_substitute_errors(self):
         self.assertRaises(ValueError, sub, '?', ['a\r\nb'])  # CR/LF not inline
         self.assertRaises(TypeError, sub, '?', [True])     # bool is not a string
         self.assertRaises(TypeError, sub, '?', [1.5])      # float is not a string
+        self.assertRaises(TypeError, sub, '?s', [['a']])   # not a message number
+        self.assertRaises(TypeError, sub, '?s', [[1.5]])   # not a message number
+        self.assertRaises(TypeError, sub, '?s', [[(1, 'a')]])  # not a message number
+        self.assertRaises(ValueError, sub, '?s', [[(1,)]])       # not a range pair
+        self.assertRaises(ValueError, sub, '?s', [[(1, 2, 3)]])  # not a range pair
 
 
 if ssl: