Reimplement message id changes including on the C extension.

6 files changed, 204 insertions, 85 deletions

diff --git a/CHANGES.rst b/CHANGES.rst
index 29a7005..1721aa0 100644
--- a/CHANGES.rst
+++ b/CHANGES.rst
@@ -4,13 +4,16 @@ Changes
 4.3 (unreleased)
 ----------------
 
-- Nothing changed yet.
+- Add attributes to support pluralization on a Message and update the
+  MessageFactory accordingly.
 
 
 4.2 (2018-10-05)
 ----------------
 
-- Fix the possibility of a rare crash in the C extension when deallocating items. See `#7 <https://github.com/zopefoundation/zope.i18nmessageid/issues/7>`_.
+- Fix the possibility of a rare crash in the C extension when
+  deallocating items. See `issue 7
+  <https://github.com/zopefoundation/zope.i18nmessageid/issues/7>`_.
 
 - Drop support for Python 3.3.
 
diff --git a/docs/narr.rst b/docs/narr.rst
index e48c53a..5982880 100644
--- a/docs/narr.rst
+++ b/docs/narr.rst
@@ -54,7 +54,7 @@ exports an already-created factory for that domain:
   >>> foo = _z_('foo')
   >>> foo.domain
   'zope'
-  
+
 
 Example Usage
 -------------
@@ -135,14 +135,17 @@ Last but not least, messages are reduceable for pickling:
   >>> args == (u'robot-message',
   ...          'futurama',
   ...          u'${name} is a robot.',
-  ...          {u'name': u'Bender'})
+  ...          {u'name': u'Bender'},
+  ...          None,
+  ...          None,
+  ...          None)
   True
 
   >>> fembot = Message(u'fembot')
   >>> callable, args = fembot.__reduce__()
   >>> callable is Message
   True
-  >>> args == (u'fembot', None, None, None)
+  >>> args == (u'fembot', None, None, None, None, None, None)
   True
 
 Pickling and unpickling works, which means we can store message IDs in
diff --git a/setup.py b/setup.py
index bc45913..749bd0e 100644
--- a/setup.py
+++ b/setup.py
@@ -58,7 +58,8 @@ if not is_pypy and not is_jython:
 
 
 def read(*rnames):
-    return open(os.path.join(os.path.dirname(__file__), *rnames)).read()
+    with open(os.path.join(os.path.dirname(__file__), *rnames)) as stream:
+        return stream.read()
 
 
 class optional_build_ext(build_ext):
@@ -135,7 +136,7 @@ setup(
     packages=find_packages('src'),
     package_dir={'': 'src'},
     namespace_packages=['zope'],
-    install_requires=['setuptools'],
+    install_requires=['setuptools', 'six'],
     include_package_data=True,
     test_suite='zope.i18nmessageid.tests.test_suite',
     zip_safe=False,
diff --git a/src/zope/i18nmessageid/_zope_i18nmessageid_message.c b/src/zope/i18nmessageid/_zope_i18nmessageid_message.c
index 2fd30ea..4481d65 100644
--- a/src/zope/i18nmessageid/_zope_i18nmessageid_message.c
+++ b/src/zope/i18nmessageid/_zope_i18nmessageid_message.c
@@ -63,6 +63,9 @@ typedef struct {
   PyObject *domain;
   PyObject *default_;
   PyObject *mapping;
+  PyObject *value_plural;
+  PyObject *default_plural;
+  PyObject *number;
 } Message;
 
 static PyTypeObject MessageType;
@@ -70,56 +73,89 @@ static PyTypeObject MessageType;
 static PyObject *
 Message_new(PyTypeObject *type, PyObject *args, PyObject *kwds)
 {
-  static char *kwlist[] = {"value", "domain", "default", "mapping", NULL};
+  static char *kwlist[] = {"value", "domain", "default", "mapping",
+                           "msgid_plural", "default_plural", "number", NULL};
   PyObject *value, *domain=NULL, *default_=NULL, *mapping=NULL, *s;
+  PyObject *value_plural=NULL, *default_plural=NULL, *number=NULL;
   Message *self;
 
-  if (!PyArg_ParseTupleAndKeywords(args, kwds, "O|OOO", kwlist, 
-                                   &value, &domain, &default_, &mapping))
-    return NULL; 
+  if (!PyArg_ParseTupleAndKeywords(args, kwds, "O|OOOOOO", kwlist,
+                                   &value, &domain, &default_, &mapping,
+                                   &value_plural, &default_plural, &number))
+    return NULL;
+
+  if (number != NULL && Py_None != number) {
+#if PY_MAJOR_VERSION >= 3
+    if (!(PyLong_Check(number) || PyFloat_Check(number))) {
+#else
+    if (!(PyLong_Check(number) || PyInt_Check(number) || PyFloat_Check(number))) {
+#endif
+      PyErr_SetString(PyExc_TypeError,
+                      "`number` should be an integer or a float");
+      return NULL;
+    }
+  }
 
   args = Py_BuildValue("(O)", value);
   if (args == NULL)
     return NULL;
 
-  s = PyUnicode_Type.tp_new(type, args, NULL); 
+  s = PyUnicode_Type.tp_new(type, args, NULL);
   Py_DECREF(args);
   if (s == NULL)
     return NULL;
 
-  if (! PyObject_TypeCheck(s, &MessageType))
-    {
-      PyErr_SetString(PyExc_TypeError, 
-                      "unicode.__new__ didn't return a Message");
-      Py_DECREF(s);
-      return NULL;
-    }
+  if (!PyObject_TypeCheck(s, &MessageType)) {
+    PyErr_SetString(PyExc_TypeError, "unicode.__new__ didn't return a Message");
+    Py_DECREF(s);
+    return NULL;
+  }
 
   self = (Message*)s;
 
-  if (PyObject_TypeCheck(value, &MessageType))
-    {
-      self->domain = ((Message *)value)->domain;
-      self->default_ = ((Message *)value)->default_;
-      self->mapping = ((Message *)value)->mapping;
-    }
-  else
-    {
-      self->domain = self->default_ = self->mapping = NULL;
-    }
+  if (PyObject_TypeCheck(value, &MessageType)) {
+    /* value is a Message so we copy it and use it as base */
+    self->domain = ((Message *)value)->domain;
+    self->default_ = ((Message *)value)->default_;
+    self->mapping = ((Message *)value)->mapping;
+    self->value_plural = ((Message *)value)->value_plural;
+    self->default_plural = ((Message *)value)->default_plural;
+    self->number = ((Message *)value)->number;
+  }
+  else {
+    self->domain = NULL;
+    self->default_ = NULL;
+    self->mapping = NULL;
+    self->value_plural = NULL;
+    self->default_plural = NULL;
+    self->number = NULL;
+  }
 
   if (domain != NULL)
     self->domain = domain;
- 
+
   if (default_ != NULL)
     self->default_ = default_;
 
   if (mapping != NULL)
     self->mapping = mapping;
 
+  if (value_plural != NULL)
+    self->value_plural = value_plural;
+
+  if (default_plural != NULL)
+    self->default_plural = default_plural;
+
+  if (number != NULL) {
+    self->number = number;
+  }
+
   Py_XINCREF(self->mapping);
   Py_XINCREF(self->default_);
   Py_XINCREF(self->domain);
+  Py_XINCREF(self->value_plural);
+  Py_XINCREF(self->default_plural);
+  Py_XINCREF(self->number);
 
   return (PyObject *)self;
 }
@@ -132,6 +168,9 @@ static PyMemberDef Message_members[] = {
   { "domain", T_OBJECT, offsetof(Message, domain), READONLY },
   { "default", T_OBJECT, offsetof(Message, default_), READONLY },
   { "mapping", T_OBJECT, offsetof(Message, mapping), READONLY },
+  { "msgid_plural", T_OBJECT, offsetof(Message, value_plural), READONLY },
+  { "default_plural", T_OBJECT, offsetof(Message, default_plural), READONLY },
+  { "number", T_OBJECT, offsetof(Message, number), READONLY },
   {NULL}	/* Sentinel */
 };
 
@@ -141,6 +180,9 @@ Message_traverse(Message *self, visitproc visit, void *arg)
   Py_VISIT(self->domain);
   Py_VISIT(self->default_);
   Py_VISIT(self->mapping);
+  Py_VISIT(self->value_plural);
+  Py_VISIT(self->default_plural);
+  Py_VISIT(self->number);
   return 0;
 }
 
@@ -150,6 +192,9 @@ Message_clear(Message *self)
   Py_CLEAR(self->domain);
   Py_CLEAR(self->default_);
   Py_CLEAR(self->mapping);
+  Py_CLEAR(self->value_plural);
+  Py_CLEAR(self->default_plural);
+  Py_CLEAR(self->number);
   return 0;
 }
 
@@ -168,11 +213,14 @@ Message_reduce(Message *self)
   value = PyObject_CallFunctionObjArgs((PyObject *)&PyUnicode_Type, self, NULL);
   if (value == NULL)
     return NULL;
-  result = Py_BuildValue("(O(OOOO))", Py_TYPE(&(self->base)),
+  result = Py_BuildValue("(O(OOOOOOO))", Py_TYPE(&(self->base)),
 			 value,
 			 self->domain ? self->domain : Py_None,
 			 self->default_ ? self->default_ : Py_None,
-			 self->mapping ? self->mapping : Py_None);
+			 self->mapping ? self->mapping : Py_None,
+			 self->value_plural ? self->value_plural : Py_None,
+			 self->default_plural ? self->default_plural : Py_None,
+			 self->number ? self->number : Py_None);
   Py_DECREF(value);
   return result;
 }
@@ -184,7 +232,7 @@ static PyMethodDef Message_methods[] = {
 };
 
 
-static char MessageType__doc__[] = 
+static char MessageType__doc__[] =
 "Message\n"
 "\n"
 "This is a string used as a message.  It has a domain attribute that is\n"
@@ -216,7 +264,7 @@ MessageType = {
         /* tp_setattro       */ (setattrofunc)0,
         /* tp_as_buffer      */ 0,
         /* tp_flags          */ Py_TPFLAGS_DEFAULT
-				| Py_TPFLAGS_BASETYPE 
+                            | Py_TPFLAGS_BASETYPE
                           	| Py_TPFLAGS_HAVE_GC,
 	/* tp_doc            */ MessageType__doc__,
         /* tp_traverse       */ (traverseproc)Message_traverse,
@@ -252,7 +300,7 @@ static struct PyMethodDef _zope_i18nmessageid_message_methods[] = {
 static char _zope_i18nmessageid_message_module_name[] =
 "_zope_i18nmessageid_message";
 
-static char _zope_i18nmessageid_message_module_documentation[] = 
+static char _zope_i18nmessageid_message_module_documentation[] =
 "I18n Messages";
 
 #if PY_MAJOR_VERSION >= 3
@@ -285,7 +333,7 @@ PyMODINIT_FUNC
   MessageType.tp_base = &PyUnicode_Type;
   if (PyType_Ready(&MessageType) < 0)
     return MOD_ERROR_VAL;
-        
+
   /* Create the module and add the functions */
 #if PY_MAJOR_VERSION >= 3
   m = PyModule_Create(&moduledef);
@@ -294,10 +342,10 @@ PyMODINIT_FUNC
                      _zope_i18nmessageid_message_methods,
                      _zope_i18nmessageid_message_module_documentation);
 #endif
-   
+
   if (m == NULL)
     return MOD_ERROR_VAL;
-       
+
   /* Add types: */
   if (PyModule_AddObject(m, "Message", (PyObject *)&MessageType) < 0)
     return MOD_ERROR_VAL;
diff --git a/src/zope/i18nmessageid/message.py b/src/zope/i18nmessageid/message.py
index 0e62408..bed7a37 100644
--- a/src/zope/i18nmessageid/message.py
+++ b/src/zope/i18nmessageid/message.py
@@ -15,13 +15,10 @@
 """
 __docformat__ = "reStructuredText"
 
-try:
-    unicode
-except NameError: #pragma NO COVER Python3
-    unicode = str
+import six
 
 
-class Message(unicode):
+class Message(six.text_type):
     """Message (Python implementation)
 
     This is a string used as a message.  It has a domain attribute that is
@@ -31,18 +28,37 @@ class Message(unicode):
     message id itself implicitly serves as the default text.
     """
 
-    __slots__ = ('domain', 'default', 'mapping', '_readonly')
+    __slots__ = (
+        'domain', 'default', 'mapping', '_readonly',
+        'msgid_plural', 'default_plural', 'number')
 
-    def __new__(cls, ustr, domain=None, default=None, mapping=None):
-        self = unicode.__new__(cls, ustr)
+    def __new__(cls, ustr, domain=None, default=None, mapping=None,
+                msgid_plural=None, default_plural=None, number=None):
+        self = six.text_type.__new__(cls, ustr)
         if isinstance(ustr, self.__class__):
             domain = ustr.domain and ustr.domain[:] or domain
             default = ustr.default and ustr.default[:] or default
             mapping = ustr.mapping and ustr.mapping.copy() or mapping
-            ustr = unicode(ustr)
+            msgid_plural = (
+                ustr.msgid_plural and ustr.msgid_plural[:] or msgid_plural)
+            default_plural = (
+                ustr.default_plural and ustr.default_plural[:]
+                or default_plural)
+            number = ustr.number is not None and ustr.number or number
+            ustr = six.text_type(ustr)
+
         self.domain = domain
         self.default = default
         self.mapping = mapping
+        self.msgid_plural = msgid_plural
+        self.default_plural = default_plural
+
+        if number is not None and not isinstance(
+                number, six.integer_types + (float,)):
+            # Number must be an integer
+            raise TypeError('`number` should be an integer or a float')
+
+        self.number = number
         self._readonly = True
         return self
 
@@ -54,27 +70,34 @@ class Message(unicode):
         if getattr(self, '_readonly', False):
             raise TypeError('readonly attribute')
         else:
-            return unicode.__setattr__(self, key, value)
+            return six.text_type.__setattr__(self, key, value)
 
     def __getstate__(self):
-        return unicode(self), self.domain, self.default, self.mapping
+        return (
+            six.text_type(self), self.domain, self.default, self.mapping,
+            self.msgid_plural, self.default_plural, self.number)
 
     def __reduce__(self):
         return self.__class__, self.__getstate__()
 
+
 # Name the fallback Python implementation to make it easier to test.
 pyMessage = Message
 
+
 try:
     from ._zope_i18nmessageid_message import Message
-except ImportError: # pragma: no cover
+except ImportError:  # pragma: no cover
     pass
 
+
 class MessageFactory(object):
     """Factory for creating i18n messages."""
 
     def __init__(self, domain):
         self._domain = domain
 
-    def __call__(self, ustr, default=None, mapping=None):
-        return Message(ustr, self._domain, default, mapping)
+    def __call__(self, ustr, default=None, mapping=None,
+                 msgid_plural=None, default_plural=None, number=None):
+        return Message(ustr, self._domain, default, mapping,
+                       msgid_plural, default_plural, number)
diff --git a/src/zope/i18nmessageid/tests.py b/src/zope/i18nmessageid/tests.py
index 0e8f6b4..810a3ff 100644
--- a/src/zope/i18nmessageid/tests.py
+++ b/src/zope/i18nmessageid/tests.py
@@ -15,12 +15,12 @@
 """
 import sys
 import unittest
-
 from zope.i18nmessageid import message as messageid
 
+
 class PyMessageTests(unittest.TestCase):
 
-    _TEST_REAOONLY = True
+    _TEST_READONLY = True
 
     def _getTargetClass(self):
         return messageid.pyMessage
@@ -34,91 +34,126 @@ class PyMessageTests(unittest.TestCase):
         self.assertEqual(message.domain, None)
         self.assertEqual(message.default, None)
         self.assertEqual(message.mapping, None)
-        if self._TEST_REAOONLY:
+        self.assertEqual(message.msgid_plural, None)
+        self.assertEqual(message.default_plural, None)
+        self.assertEqual(message.number, None)
+        if self._TEST_READONLY:
             self.assertTrue(message._readonly)
 
     def test_ctor_explicit(self):
         mapping = {'key': 'value'}
-        message = self._makeOne('testing', 'domain', 'default', mapping)
+        message = self._makeOne(
+            'testing', 'domain', 'default', mapping,
+            msgid_plural='testings', default_plural="defaults", number=2)
         self.assertEqual(message, 'testing')
         self.assertEqual(message.domain, 'domain')
         self.assertEqual(message.default, 'default')
         self.assertEqual(message.mapping, mapping)
-        if self._TEST_REAOONLY:
+        self.assertEqual(message.msgid_plural, 'testings')
+        self.assertEqual(message.default_plural, 'defaults')
+        self.assertEqual(message.number, 2)
+        if self._TEST_READONLY:
             self.assertTrue(message._readonly)
 
     def test_ctor_copy(self):
         mapping = {'key': 'value'}
-        source = self._makeOne('testing', 'domain', 'default', mapping)
+        source = self._makeOne(
+            'testing', 'domain', 'default', mapping,
+            msgid_plural='testings', default_plural="defaults", number=2)
         message = self._makeOne(source)
         self.assertEqual(message, 'testing')
         self.assertEqual(message.domain, 'domain')
         self.assertEqual(message.default, 'default')
         self.assertEqual(message.mapping, mapping)
-        if self._TEST_REAOONLY:
+        self.assertEqual(message.msgid_plural, 'testings')
+        self.assertEqual(message.default_plural, 'defaults')
+        self.assertEqual(message.number, 2)
+        if self._TEST_READONLY:
             self.assertTrue(message._readonly)
 
     def test_ctor_copy_w_overrides(self):
         mapping = {'key': 'value'}
         source = self._makeOne('testing')
-        message = self._makeOne(source, 'domain', 'default', mapping)
+        message = self._makeOne(
+            source, 'domain', 'default', mapping,
+            msgid_plural='testings', default_plural="defaults", number=2)
         self.assertEqual(message, 'testing')
         self.assertEqual(message.domain, 'domain')
         self.assertEqual(message.default, 'default')
         self.assertEqual(message.mapping, mapping)
-        if self._TEST_REAOONLY:
+        self.assertEqual(message.msgid_plural, 'testings')
+        self.assertEqual(message.default_plural, 'defaults')
+        self.assertEqual(message.number, 2)
+        if self._TEST_READONLY:
             self.assertTrue(message._readonly)
 
     def test_domain_immutable(self):
         message = self._makeOne('testing')
-        def _try():
+        with self.assertRaises((TypeError, AttributeError)):
             message.domain = 'domain'
-        # C version raises AttributeError, Python version TypeError
-        self.assertRaises((TypeError, AttributeError), _try)
 
     def test_default_immutable(self):
         message = self._makeOne('testing')
-        def _try():
+        with self.assertRaises((TypeError, AttributeError)):
             message.default = 'default'
-        # C version raises AttributeError, Python version TypeError
-        self.assertRaises((TypeError, AttributeError), _try)
 
     def test_mapping_immutable(self):
         mapping = {'key': 'value'}
         message = self._makeOne('testing')
-        def _try():
+        with self.assertRaises((TypeError, AttributeError)):
             message.mapping = mapping
-        # C version raises AttributeError, Python version TypeError
-        self.assertRaises((TypeError, AttributeError), _try)
+
+    def test_msgid_plural_immutable(self):
+        message = self._makeOne('testing')
+        with self.assertRaises((TypeError, AttributeError)):
+            message.msgid_plural = 'bar'
+
+    def test_default_plural_immutable(self):
+        message = self._makeOne('testing')
+        with self.assertRaises((TypeError, AttributeError)):
+            message.default_plural = 'bar'
+
+    def test_number_immutable(self):
+        message = self._makeOne('testing')
+        with self.assertRaises((TypeError, AttributeError)):
+            message.number = 23
 
     def test_unknown_immutable(self):
         message = self._makeOne('testing')
-        def _try():
+        with self.assertRaises((TypeError, AttributeError)):
             message.unknown = 'unknown'
-        # C version raises AttributeError, Python version TypeError
-        self.assertRaises((TypeError, AttributeError), _try)
 
     def test___reduce__(self):
         mapping = {'key': 'value'}
         source = self._makeOne('testing')
-        message = self._makeOne(source, 'domain', 'default', mapping)
+        message = self._makeOne(
+            source, 'domain', 'default', mapping,
+            msgid_plural='testings', default_plural="defaults", number=2)
         klass, state = message.__reduce__()
         self.assertTrue(klass is self._getTargetClass())
-        self.assertEqual(state, ('testing', 'domain', 'default', mapping))
+        self.assertEqual(
+            state,
+            ('testing', 'domain', 'default', {'key': 'value'},
+             'testings', 'defaults', 2))
 
     def test_non_unicode_default(self):
         message = self._makeOne(u'str', default=123)
         self.assertEqual(message.default, 123)
 
-@unittest.skipIf(messageid.Message is messageid.pyMessage,
-                 "Duplicate tests")
+    def test_non_numeric_number(self):
+        with self.assertRaises((TypeError, AttributeError)):
+            self._makeOne(u'str', default=123, number="one")
+
+
+@unittest.skipIf(messageid.Message is messageid.pyMessage, "Duplicate tests")
 class MessageTests(PyMessageTests):
 
-    _TEST_REAOONLY = False
+    _TEST_READONLY = False
 
     def _getTargetClass(self):
         return messageid.Message
 
+
 @unittest.skipIf('java' in sys.platform or hasattr(sys, 'pypy_version_info'),
                  "We don't expect the C implementation here")
 class OptimizationTests(unittest.TestCase):
@@ -126,35 +161,41 @@ class OptimizationTests(unittest.TestCase):
     def test_optimizations_available(self):
         self.assertIsNot(messageid.Message, messageid.pyMessage)
 
+
 class MessageFactoryTests(unittest.TestCase):
 
     def _getTargetClass(self):
-        from zope.i18nmessageid.message import MessageFactory
-        return MessageFactory
+        return messageid.MessageFactory
 
     def _makeOne(self, *args, **kw):
         return self._getTargetClass()(*args, **kw)
 
     def test___call___defaults(self):
-        from zope.i18nmessageid.message import Message
         factory = self._makeOne('domain')
         message = factory('testing')
-        self.assertTrue(isinstance(message, Message))
+        self.assertTrue(isinstance(message, messageid.Message))
         self.assertEqual(message, 'testing')
         self.assertEqual(message.domain, 'domain')
         self.assertEqual(message.default, None)
         self.assertEqual(message.mapping, None)
+        self.assertEqual(message.msgid_plural, None)
+        self.assertEqual(message.default_plural, None)
+        self.assertEqual(message.number, None)
 
     def test___call___explicit(self):
-        from zope.i18nmessageid.message import Message
         mapping = {'key': 'value'}
         factory = self._makeOne('domain')
-        message = factory('testing', 'default', mapping)
-        self.assertTrue(isinstance(message, Message))
+        message = factory(
+            'testing', 'default', mapping,
+            msgid_plural='testings', default_plural="defaults", number=2)
+        self.assertTrue(isinstance(message, messageid.Message))
         self.assertEqual(message, 'testing')
         self.assertEqual(message.domain, 'domain')
         self.assertEqual(message.default, 'default')
         self.assertEqual(message.mapping, mapping)
+        self.assertEqual(message.msgid_plural, 'testings')
+        self.assertEqual(message.default_plural, 'defaults')
+        self.assertEqual(message.number, 2)
 
 
 def test_suite():