Repository: python-attrs/attrs

Problem statement:
clean division between "value" and "object"
Objects, as in object-oriented programming, are side-effecty, mutable state, whose methods ought to represent I/O.

Values, as in functional programming, are immutable data, whose methods ought to represent computation.

Python does not have as good a division between these very different sorts of beasts as it should, but it does have one critical distinction: the `__hash__` method.

`characteristic` has this issue which crops up occasionally where you end up with objects that you can't put into a dictionary as a key, because one of its attributes is a dictionary.  Sometimes people expect to be able to do this because `characteristic` makes so many other things easy, and just expect dictionaries to suddenly be immutable; sometimes people expect hash-by-identity.

I propose that attr provide a way to specifically create two types of objects with a distinct interface; one that creates a "value" and one that creates an "object", so that users can see issues around mutability far earlier in the process.

So, for example, the "object" type would:
- not provide `__hash__` by default, provide an `__eq__` that does structural equality, and `__gt__`/`__lt__` that just raise exceptions
- if asked, provide an identity-based `__hash__`, but then also switch to an identity-based `__eq__`

and the 'value" type would
- call `hash()` on all of its arguments at construction time so it would fail immediately if it contained a mutable type
- fail immediately at class-definition time if any validator is mutable
- provide immutable descriptors for all its attributes



Hints:
How would that look API-wise?  Would something like `@attr.object` or `@attr.value` be presets for `@attr.s`?

I'm not sure.  I'm trying to decide what I think the default for `attr.s` is; there's a strong case in my mind for both default-to-mutable or default-to-immutable.  `attr.s(mutable=True)`?

The default should be what most people (and I!) would expect it to do.  We’re still on Python and don’t have free COW structures etc.  Therefore any immutability gimmickry ought to be opt-in (but in place and simple to use).

I think you're right; Python programmers are going to expect mutability by default, trying to turn that off would just be an ideological statement, not useful functionality.  So how about just having a `__hash__` that raises an exception which points you at `attr.value`, and `attr.s` is `object`?

Hi, I found this ticket because I was looking for an `immutable` argument to `@attr.s` like there was with characteristic :)

ALSO: on the subject of "value types", I would like to point out my new library [sumtypes](https://pypi.python.org/pypi/sumtypes) (honestly I didn't come here looking for a place to advertise, but it seems relevant). `immutable` would be great to have for it too (and I probably _would_ make it the default in that library).

I’m reluctant to implement immutable myself because it adds another method I have to highly invasively muck with (`__setattr__`).

_But_ `attrs` [is extensible](https://attrs.readthedocs.org/en/stable/extending.html), so feel free to implement it yourself!

@hynek I guess there's two separate aspects of immutability: making sure declared attributes are immutable, and making sure new attributes can't be dynamically assigned to. 

Neither of these actually require defining `__setattr__`. The former can be accomplished with the descriptor protocol, and the latter can be accomplished with `__slots__`. 

I assumed Attribute must have already implemented the descriptor protocol, but I see now I'm wrong, and, for example, mutating an attribute post-instantiation will allow setting it to a value that a validator wouldn't accept, because validators are not run on mutation. So maybe it would be best if Attribute instances do provide the descriptor protocol, both to support validating mutation as well as implementing immutability if it's requested?

I kinda want to recommend setting up a performance test suite at this point, because while it seems to me like using the descriptor protocol _ought_ to be zero-cost on PyPy and "fast enough" on CPython, it also seems like it would be worth knowing that for sure.

(Although I also assumed that attrs would validate on mutation and this is a slightly unpleasant surprise.  Hmm.)

attrs had validation on mutation once but there were so many loopholes that I decided to take it out because the performance hit for _everyone_ simply wasn’t worth it.

I’m pretty sure that using descriptors is measurably slower.  I’ll happily be proven wrong with a benchmark. :)

What do you mean "so many loopholes"?  Other than `__dict__.__setitem__` and `object.__setattr__` what else is there?

@glyph well, there's also other methods on `__dict__` than `__setitem__`, so generalize that to "`__dict__` mutation"

Personally I think it's worth doing validation with the descriptor protocol even though someone could bypass with `__dict__` mutation. However! It would also be possible to make naive dict mutation ineffective, since Python always consults a descriptor before a `__dict__` entry. It might have other downsides, but you could have the descriptors store their actual data under a different name in the instance dict, so at least someone would really know they're bypassing validation when they're assigning something to `__dict__['__attrs_x']`.

But... yeah, that might be a bit too surprising/weird for people who do want to dig down into the representation. I mean, Pickling would still work just fine, but maybe there are some useful things that would break?

Also, as a follow-up to my previous mention of `__slots__`, I've hit a snag there: apparently the way `__slots__` works is by creating descriptors on the class. Which means you can't use your _own_ descriptors (or even non-descriptors -- you can't even use class variables to declare defaults for instances, for example). The result of trying to add `__slots__` to a `@attrib.s`-using class is this:

```
>>> @attr.s
... class Foo(object):
...  __slots__ = ('x', 'y')
...  x = attr.ib()
...  y = attr.ib()
...
>>> Foo(1, 2)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "<attrs generated init a7439b12cdcdd2d603c04767f9a798adb8a0d944>", line 2, in __init__
AttributeError: 'Foo' object attribute 'x' is read-only
```

There may be some clever trick I'm missing to still use `__slots__` (which actually would be nice to use on CPython if we could...), but I'm thinking @hynek is right and the only way to implement immutability would be by defining a `__setattr__` :-(

Oh, never mind. There is a way to use both slots and descriptors:

http://stackoverflow.com/questions/4912499/using-python-descriptors-with-slots

It's just a matter of having the user's descriptor be named different from the slot.

most obvious loophole:  `instance.attribute.do_something_that_mutates_instance()`.

I don’t want to sound like an ass but adding features that are invasive but only interesting to a small fraction of people made `characteristic` what it is so I’m much more conservative this time and rather made it extensible than implementing everything ppl shout at me.

I’m open to descriptor-based solutions that have no negative impact on performance though.

Mutability doesn't seem like a particularly obscure or minority concern :).

That said, I'm not sure what you're objecting to.  Doing validation on attributes by default?  Doing validation on attributes at all?  Or the actual topic of this bug, `@attr.value`, which would not be on by default anyway?

Echo chamber. :)

I’m objecting the tangent of validating on assignment.  I don’t object immutability but I feel it should be a separate decorator.  Maybe `@attr.value`? :)

OK good. Let's have another issue to discuss that.

For the case of `@attr.value` specifically, we could just override `__setattr__` to raise `AttributeError` after initialization. Does that seem sufficient?

You mentioned not wanting to do it yourself, but nothing _except_ `value` would need to mess with it.

Just to be clear: my main issue is not being lazy but wanting to keep attrs clean. :)  Moving settattr magic into a separate decorator seems a fair compromise to me.

Cool, thank you, I asked the question very awkwardly but that was exactly the form of answer I wanted :).

See also #50 ?


Patch:
diff --git a/src/attr/__init__.py b/src/attr/__init__.py
--- a/src/attr/__init__.py
+++ b/src/attr/__init__.py
@@ -19,6 +19,7 @@
     get_run_validators,
     set_run_validators,
 )
+from . import exceptions
 from . import filters
 from . import validators
 
@@ -49,6 +50,7 @@
     "attrib",
     "attributes",
     "attrs",
+    "exceptions",
     "fields",
     "filters",
     "get_run_validators",
diff --git a/src/attr/_make.py b/src/attr/_make.py
--- a/src/attr/_make.py
+++ b/src/attr/_make.py
@@ -3,8 +3,9 @@
 import hashlib
 import linecache
 
-from ._compat import exec_, iteritems, isclass, iterkeys
 from . import _config
+from ._compat import exec_, iteritems, isclass, iterkeys
+from .exceptions import FrozenInstanceError
 
 
 class _Nothing(object):
@@ -145,8 +146,16 @@ def _transform_attrs(cls, these):
             had_default = True
 
 
+def _frozen_setattrs(self, name, value):
+    """
+    Attached to frozen classes as __setattr__.
+    """
+    raise FrozenInstanceError()
+
+
 def attributes(maybe_cls=None, these=None, repr_ns=None,
-               repr=True, cmp=True, hash=True, init=True, slots=False):
+               repr=True, cmp=True, hash=True, init=True,
+               slots=False, frozen=False):
     """
     A class decorator that adds `dunder
     <https://wiki.python.org/moin/DunderAlias>`_\ -methods according to the
@@ -161,33 +170,42 @@ def attributes(maybe_cls=None, these=None, repr_ns=None,
         If *these* is not `None`, the class body is *ignored*.
     :type these: :class:`dict` of :class:`str` to :func:`attr.ib`
 
-    :param repr_ns: When using nested classes, there's no way in Python 2 to
-        automatically detect that.  Therefore it's possible to set the
+    :param str repr_ns: When using nested classes, there's no way in Python 2
+        to automatically detect that.  Therefore it's possible to set the
         namespace explicitly for a more meaningful ``repr`` output.
-
-    :param repr: Create a ``__repr__`` method with a human readable
+    :param bool repr: Create a ``__repr__`` method with a human readable
         represantation of ``attrs`` attributes..
-    :type repr: bool
-
-    :param cmp: Create ``__eq__``, ``__ne__``, ``__lt__``, ``__le__``,
+    :param bool cmp: Create ``__eq__``, ``__ne__``, ``__lt__``, ``__le__``,
         ``__gt__``, and ``__ge__`` methods that compare the class as if it were
         a tuple of its ``attrs`` attributes.  But the attributes are *only*
         compared, if the type of both classes is *identical*!
-    :type cmp: bool
+    :param bool hash: Create a ``__hash__`` method that returns the
+        :func:`hash` of a tuple of all ``attrs`` attribute values.
+    :param bool init: Create a ``__init__`` method that initialiazes the
+        ``attrs`` attributes.  Leading underscores are stripped for the
+        argument name.
+    :param bool slots: Create a slots_-style class that's more
+        memory-efficient.  See :ref:`slots` for further ramifications.
+    :param bool frozen: Make instances immutable after initialization.  If
+        someone attempts to modify a frozen instance,
+        :exc:`attr.exceptions.FrozenInstanceError` is raised.
+
+        Please note:
 
-    :param hash: Create a ``__hash__`` method that returns the :func:`hash` of
-        a tuple of all ``attrs`` attribute values.
-    :type hash: bool
+            1. This is achieved by installing a custom ``__setattr__`` method
+               on your class so you can't implement an own one.
 
-    :param init: Create a ``__init__`` method that initialiazes the ``attrs``
-        attributes.  Leading underscores are stripped for the argument name.
-    :type init: bool
+            2. True immutability is impossible in Python.
 
-    :param slots: Create a slots_-style class that's more memory-efficient.
-        See :ref:`slots` for further ramifications.
-    :type slots: bool
+            3. This *does* have a minor a runtime performance impact when
+               initializing new instances.  In other words: ``__init__`` is
+               slightly slower with ``frozen=True``.
 
-    .. _slots: https://docs.python.org/3.5/reference/datamodel.html#slots
+        ..  _slots: https://docs.python.org/3.5/reference/datamodel.html#slots
+
+        ..  versionadded:: 16.0.0 *slots*
+
+        ..  versionadded:: 16.1.0 *frozen*
     """
     def wrap(cls):
         if getattr(cls, "__class__", None) is None:
@@ -209,8 +227,10 @@ def wrap(cls):
         if hash is True:
             cls = _add_hash(cls)
         if init is True:
-            cls = _add_init(cls)
-        if slots:
+            cls = _add_init(cls, frozen)
+        if frozen is True:
+            cls.__setattr__ = _frozen_setattrs
+        if slots is True:
             cls_dict = dict(cls.__dict__)
             cls_dict["__slots__"] = tuple(ca_list)
             for ca_name in ca_list:
@@ -367,7 +387,10 @@ def repr_(self):
     return cls
 
 
-def _add_init(cls):
+def _add_init(cls, frozen):
+    """
+    Add a __init__ method to *cls*.  If *frozen* is True, make it immutable.
+    """
     attrs = [a for a in cls.__attrs_attrs__
              if a.init or a.default is not NOTHING]
 
@@ -378,14 +401,21 @@ def _add_init(cls):
         sha1.hexdigest()
     )
 
-    script = _attrs_to_script(attrs)
+    script = _attrs_to_script(attrs, frozen)
     locs = {}
     bytecode = compile(script, unique_filename, "exec")
     attr_dict = dict((a.name, a) for a in attrs)
-    exec_(bytecode, {"NOTHING": NOTHING,
-                     "attr_dict": attr_dict,
-                     "validate": validate,
-                     "_convert": _convert}, locs)
+    globs = {
+        "NOTHING": NOTHING,
+        "attr_dict": attr_dict,
+        "validate": validate,
+        "_convert": _convert
+    }
+    if frozen is True:
+        # Save the lookup overhead in __init__ if we need to circumvent
+        # immutability.
+        globs["_cached_setattr"] = object.__setattr__
+    exec_(bytecode, globs, locs)
     init = locs["__init__"]
 
     # In order of debuggers like PDB being able to step through the code,
@@ -450,11 +480,31 @@ def _convert(inst):
             setattr(inst, a.name, a.convert(getattr(inst, a.name)))
 
 
-def _attrs_to_script(attrs):
+def _attrs_to_script(attrs, frozen):
     """
     Return a valid Python script of an initializer for *attrs*.
+
+    If *frozen* is True, we cannot set the attributes directly so we use
+    a cached ``object.__setattr__``.
     """
     lines = []
+    if frozen is True:
+        lines.append(
+            "_setattr = _cached_setattr.__get__(self, self.__class__)"
+        )
+
+        def fmt_setter(attr_name, value):
+            return "_setattr('%(attr_name)s', %(value)s)" % {
+                "attr_name": attr_name,
+                "value": value,
+            }
+    else:
+        def fmt_setter(attr_name, value):
+            return "self.%(attr_name)s = %(value)s" % {
+                "attr_name": attr_name,
+                "value": value,
+            }
+
     args = []
     has_validator = False
     has_convert = False
@@ -467,14 +517,16 @@ def _attrs_to_script(attrs):
         arg_name = a.name.lstrip("_")
         if a.init is False:
             if isinstance(a.default, Factory):
-                lines.append("""\
-self.{attr_name} = attr_dict["{attr_name}"].default.factory()""".format(
-                    attr_name=attr_name,
+                lines.append(fmt_setter(
+                    attr_name,
+                    "attr_dict['{attr_name}'].default.factory()"
+                    .format(attr_name=attr_name)
                 ))
             else:
-                lines.append("""\
-self.{attr_name} = attr_dict["{attr_name}"].default""".format(
-                    attr_name=attr_name,
+                lines.append(fmt_setter(
+                    attr_name,
+                    "attr_dict['{attr_name}'].default"
+                    .format(attr_name=attr_name)
                 ))
         elif a.default is not NOTHING and not isinstance(a.default, Factory):
             args.append(
@@ -483,26 +535,21 @@ def _attrs_to_script(attrs):
                     attr_name=attr_name,
                 )
             )
-            lines.append("self.{attr_name} = {arg_name}".format(
-                arg_name=arg_name,
-                attr_name=attr_name,
-            ))
+            lines.append(fmt_setter(attr_name, arg_name))
         elif a.default is not NOTHING and isinstance(a.default, Factory):
             args.append("{arg_name}=NOTHING".format(arg_name=arg_name))
-            lines.extend("""\
-if {arg_name} is not NOTHING:
-    self.{attr_name} = {arg_name}
-else:
-    self.{attr_name} = attr_dict["{attr_name}"].default.factory()"""
-                         .format(attr_name=attr_name,
-                                 arg_name=arg_name)
-                         .split("\n"))
+            lines.append("if {arg_name} is not NOTHING:"
+                         .format(arg_name=arg_name))
+            lines.append("    " + fmt_setter(attr_name, arg_name))
+            lines.append("else:")
+            lines.append("    " + fmt_setter(
+                attr_name,
+                "attr_dict['{attr_name}'].default.factory()"
+                .format(attr_name=attr_name)
+            ))
         else:
             args.append(arg_name)
-            lines.append("self.{attr_name} = {arg_name}".format(
-                attr_name=attr_name,
-                arg_name=arg_name,
-            ))
+            lines.append(fmt_setter(attr_name, arg_name))
 
     if has_convert:
         lines.append("_convert(self)")
@@ -511,10 +558,10 @@ def _attrs_to_script(attrs):
 
     return """\
 def __init__(self, {args}):
-    {setters}
+    {lines}
 """.format(
         args=", ".join(args),
-        setters="\n    ".join(lines) if lines else "pass",
+        lines="\n    ".join(lines) if lines else "pass",
     )
 
 
@@ -544,7 +591,7 @@ def __init__(self, **kw):
                     raise TypeError("Missing argument '{arg}'.".format(arg=a))
 
     def __setattr__(self, name, value):
-        raise AttributeError("can't set attribute")  # To mirror namedtuple.
+        raise FrozenInstanceError()
 
     @classmethod
     def from_counting_attr(cls, name, ca):
diff --git a/src/attr/exceptions.py b/src/attr/exceptions.py
new file mode 100644
--- /dev/null
+++ b/src/attr/exceptions.py
@@ -0,0 +1,12 @@
+from __future__ import absolute_import, division, print_function
+
+
+class FrozenInstanceError(AttributeError):
+    """
+    A frozen/immutable instance has been attempted to be modified.
+
+    It mirrors the behavior of ``namedtuples`` by using the same error message
+    and subclassing :exc:`AttributeError``.
+    """
+    msg = "can't set attribute"
+    args = [msg]


Test patch:
diff --git a/tests/test_dark_magic.py b/tests/test_dark_magic.py
--- a/tests/test_dark_magic.py
+++ b/tests/test_dark_magic.py
@@ -8,6 +8,7 @@
 
 from attr._compat import TYPE
 from attr._make import Attribute, NOTHING
+from attr.exceptions import FrozenInstanceError
 
 
 @attr.s
@@ -62,6 +63,11 @@ class SubSlots(SuperSlots):
     y = attr.ib()
 
 
+@attr.s(frozen=True, slots=True)
+class Frozen(object):
+    x = attr.ib()
+
+
 class TestDarkMagic(object):
     """
     Integration tests.
@@ -114,12 +120,12 @@ class C3(object):
 
         assert "C3(_x=1)" == repr(C3(x=1))
 
-    @given(booleans())
-    def test_programmatic(self, slots):
+    @given(booleans(), booleans())
+    def test_programmatic(self, slots, frozen):
         """
         `attr.make_class` works.
         """
-        PC = attr.make_class("PC", ["a", "b"], slots=slots)
+        PC = attr.make_class("PC", ["a", "b"], slots=slots, frozen=frozen)
         assert (
             Attribute(name="a", default=NOTHING, validator=None,
                       repr=True, cmp=True, hash=True, init=True),
@@ -155,3 +161,19 @@ class Sub2(base):
         i = Sub2(x=obj)
         assert i.x is i.meth() is obj
         assert "Sub2(x={obj})".format(obj=obj) == repr(i)
+
+    @pytest.mark.parametrize("frozen_class", [
+        Frozen,  # has slots=True
+        attr.make_class("FrozenToo", ["x"], slots=False, frozen=True),
+    ])
+    def test_frozen_instance(self, frozen_class):
+        """
+        Frozen instances can't be modified (easily).
+        """
+        frozen = frozen_class(1)
+
+        with pytest.raises(FrozenInstanceError) as e:
+            frozen.x = 2
+
+        assert e.value.args[0] == "can't set attribute"
+        assert 1 == frozen.x
diff --git a/tests/test_dunders.py b/tests/test_dunders.py
--- a/tests/test_dunders.py
+++ b/tests/test_dunders.py
@@ -35,7 +35,7 @@
 class InitC(object):
     __attrs_attrs__ = [simple_attr("a"), simple_attr("b")]
 
-InitC = _add_init(InitC)
+InitC = _add_init(InitC, False)
 
 
 class TestAddCmp(object):
@@ -219,12 +219,13 @@ class TestAddInit(object):
     """
     Tests for `_add_init`.
     """
-    @given(booleans())
-    def test_init(self, slots):
+    @given(booleans(), booleans())
+    def test_init(self, slots, frozen):
         """
         If `init` is False, ignore that attribute.
         """
-        C = make_class("C", {"a": attr(init=False), "b": attr()}, slots=slots)
+        C = make_class("C", {"a": attr(init=False), "b": attr()},
+                       slots=slots, frozen=frozen)
         with pytest.raises(TypeError) as e:
             C(a=1, b=2)
 
@@ -233,8 +234,8 @@ def test_init(self, slots):
             e.value.args[0]
         )
 
-    @given(booleans())
-    def test_no_init_default(self, slots):
+    @given(booleans(), booleans())
+    def test_no_init_default(self, slots, frozen):
         """
         If `init` is False but a Factory is specified, don't allow passing that
         argument but initialize it anyway.
@@ -243,7 +244,7 @@ def test_no_init_default(self, slots):
             "_a": attr(init=False, default=42),
             "_b": attr(init=False, default=Factory(list)),
             "c": attr()
-        }, slots=slots)
+        }, slots=slots, frozen=frozen)
         with pytest.raises(TypeError):
             C(a=1, c=2)
         with pytest.raises(TypeError):
@@ -252,8 +253,8 @@ def test_no_init_default(self, slots):
         i = C(23)
         assert (42, [], 23) == (i._a, i._b, i.c)
 
-    @given(booleans())
-    def test_no_init_order(self, slots):
+    @given(booleans(), booleans())
+    def test_no_init_order(self, slots, frozen):
         """
         If an attribute is `init=False`, it's legal to come after a mandatory
         attribute.
@@ -261,7 +262,7 @@ def test_no_init_order(self, slots):
         make_class("C", {
             "a": attr(default=Factory(list)),
             "b": attr(init=False),
-        }, slots=slots)
+        }, slots=slots, frozen=frozen)
 
     def test_sets_attributes(self):
         """
@@ -282,7 +283,7 @@ class C(object):
                 simple_attr(name="c", default=None),
             ]
 
-        C = _add_init(C)
+        C = _add_init(C, False)
         i = C()
         assert 2 == i.a
         assert "hallo" == i.b
@@ -300,7 +301,7 @@ class C(object):
                 simple_attr(name="a", default=Factory(list)),
                 simple_attr(name="b", default=Factory(D)),
             ]
-        C = _add_init(C)
+        C = _add_init(C, False)
         i = C()
         assert [] == i.a
         assert isinstance(i.b, D)
@@ -363,7 +364,7 @@ def test_underscores(self):
         class C(object):
             __attrs_attrs__ = [simple_attr("_private")]
 
-        C = _add_init(C)
+        C = _add_init(C, False)
         i = C(private=42)
         assert 42 == i._private