Tested and documented undefined inheritance

[mristin]

If the base class does not inherit from DBC or uses DBCMeta as the meta class, inheritance of invariants will be broken.

In this patch, we tested and documented this unexpected behavior to warn the users about this foot-gun.

[verinov]

Looks like it wouldn't be hard to check this at runtime. For example, after this line there could be a check like assert "__invariants__" in cls.__dict__, or assert not any(getattr(c, "__invariants__", None) is invariants for c in cls.mro()[1:]) (with an informative message, of course).

[coveralls]

coverage: 92.929%. remained the same
when pulling 032efaf on mristin/Test-and-document-that-undefined-behavior-without-DBC
into 625e797 on master.

[mristin]

Looks like it wouldn't be hard to check this at runtime. For example, after this line there could be a check like assert "__invariants__" in cls.__dict__, or assert not any(getattr(c, "__invariants__", None) is invariants for c in cls.mro()[1:]) (with an informative message, of course).

Can you please clarify a bit?

Note that the check:

icontract/icontract/_decorators.py

Line 478 in 625e797

if not hasattr(cls, "__invariants__"):

is just making sure that there was no bug in the implementation. That is, that all the runtime types are as expected.

[verinov]

When DBC is used, each class will have its own cls.__invariants__, in other words there will be no aliasing.
Without DBC, nothing ensures that, and getattr(cls, "__invariants__") will happily resolve to the class attribute found in the base class. Two example asserts I wrote are checking if the "invariants" attribute of cls was set on cls itself, and that it wasn't only set on one of its base classes.
If it was a method, I would say that we are checking whether a derived classes overrides a method in its base class. Except here we are talking about attributes.

Maybe it's also possible to copy the list every time, as in setattr(cls, "__invariants__", [*getattr(cls, "__invariants__"), self._invariant]) instead of invariants.append(self._invariant). This should fix the aliasing issue even for non-DBC classes. I don't know if it is important to keep getattr(cls, "__invariants__") list being the same python object though.

[mristin]

@verinov thanks for the clarifications! I looked into this for some time. After quite some experiments, I decided to leave it at the documentation. Namely, the Liskov substitution principle would be broken without the inheritance from DBC or setting DBCMeta as a meta class for pre-conditions.

I could always make the copy for invariants, postconditions and snapshots, but that would mask the error, and make the code of the library very messy and pretty unmaintainable.