Repository: FactoryBoy/factory_boy

Issue:
Maybe: Inconsistent phases TypeError masked by __repr__ AttributeError 
#### Description
The TypeError check in `declarations.Maybe.__init__` is masked by an AttributeError because the TypeError error message uses `__repr__` which in turn uses `self` attributes that haven't yet be set in `__init__`.

#### To Reproduce
Noticed when upgrading from `2.8.1` to `2.12.0` on a factory that used Traits and had a PostGeneration hook which resulted in a Maybe object with a yes declaration evaluating to a List object (`attributes` enum) and a no declaration evaluating to a PostGeneration object (`post_instance` enum).

To reproduce: Create a `Maybe` object with yes/no declarations with differing enums. 

##### Model / Factory code
Minimal example

```python
In [1]: from factory.declarations import LazyAttribute, Maybe, PostGenerationDeclaration

In [2]: Maybe(None, LazyAttribute(None), PostGenerationDeclaration())

AttributeError                            Traceback (most recent call last)
----> 1 Maybe(None, LazyAttribute(None), PostGenerationDeclaration())

~/venv/lib/python3.6/site-packages/factory/declarations.py in __init__(self, decider, yes_declaration, no_declaration)
    456
    457         if len(used_phases) > 1:
--> 458             raise TypeError("Inconsistent phases for %r: %r" % (self, phases))
    459
    460         self.FACTORY_BUILDER_PHASE = used_phases.pop() if used_phases else enums.BuilderPhase.ATTRIBUTE_RESOLUTION

~/venv/lib/python3.6/site-packages/factory/declarations.py in __repr__(self)
    504
    505     def __repr__(self):
--> 506         return 'Maybe(%r, yes=%r, no=%r)' % (self.decider, self.yes, self.no)
    507
    508

AttributeError: 'Maybe' object has no attribute 'decider'
```

##### The issue
The TypeError error message uses `self` but the `__repr__` uses attributes that only get set on `self` at the end of `__init__`. This raises an AttributeError which masks the actual TypeError.

If the attributes were set ahead of the check on https://github.com/FactoryBoy/factory_boy/blob/master/factory/declarations.py#L454 then the expected exception would be raised:

```python
In [1]: from factory.declarations import LazyAttribute, Maybe, PostGenerationDeclaration

In [2]: Maybe(None, LazyAttribute(None), PostGenerationDeclaration())

TypeError                                 Traceback (most recent call last)
/vagrant/.konchrc in <module>
----> 1 Maybe(None, LazyAttribute(None), PostGenerationDeclaration())

~/venv/lib/python3.6/site-packages/factory/declarations.py in __init__(self, decider, yes_declaration, no_declaration)
    460
    461         if len(used_phases) > 1:
--> 462             raise TypeError("Inconsistent phases for %r: %r" % (self, phases))
    463
    464         self.FACTORY_BUILDER_PHASE = used_phases.pop() if used_phases else enums.BuilderPhase.ATTRIBUTE_RESOLUTION

TypeError: Inconsistent phases for Maybe(None, yes=<factory.declarations.LazyAttribute object at 0x7f0ec15d4d30>, no=<factory.declarations.PostGenerationDeclaration object at 0x7f0ec15d4160>): {'yes_declaration': 'attributes', 'no_declaration': 'post_instance'}
```

#### Notes

Check was introduced in 33d27c219044

Discussion:
francoisfreitag: I think both commits can be squashed.

Patch:
diff --git a/factory/declarations.py b/factory/declarations.py
index bdb44ab..18c1fd5 100644
--- a/factory/declarations.py
+++ b/factory/declarations.py
@@ -444,6 +444,15 @@ SKIP = Skip()
 class Maybe(BaseDeclaration):
     def __init__(self, decider, yes_declaration=SKIP, no_declaration=SKIP):
         super(Maybe, self).__init__()
+
+        if enums.get_builder_phase(decider) is None:
+            # No builder phase => flat value
+            decider = SelfAttribute(decider, default=None)
+
+        self.decider = decider
+        self.yes = yes_declaration
+        self.no = no_declaration
+
         phases = {
             'yes_declaration': enums.get_builder_phase(yes_declaration),
             'no_declaration': enums.get_builder_phase(no_declaration),
@@ -455,14 +464,6 @@ class Maybe(BaseDeclaration):
 
         self.FACTORY_BUILDER_PHASE = used_phases.pop() if used_phases else enums.BuilderPhase.ATTRIBUTE_RESOLUTION
 
-        if enums.get_builder_phase(decider) is None:
-            # No builder phase => flat value
-            decider = SelfAttribute(decider, default=None)
-
-        self.decider = decider
-        self.yes = yes_declaration
-        self.no = no_declaration
-
     def call(self, instance, step, context):
         decider_phase = enums.get_builder_phase(self.decider)
         if decider_phase == enums.BuilderPhase.ATTRIBUTE_RESOLUTION:


Test patch:
diff --git a/tests/test_declarations.py b/tests/test_declarations.py
index 889080a..2e6474d 100644
--- a/tests/test_declarations.py
+++ b/tests/test_declarations.py
@@ -43,6 +43,21 @@ class DigTestCase(unittest.TestCase):
         self.assertEqual(42, declarations.deepgetattr(obj, 'a.b.c.n.x', 42))
 
 
+class MaybeTestCase(unittest.TestCase):
+    @classmethod
+    def setUpClass(cls):
+        # remove after dropping python 2
+        import sys
+        if int(sys.version[0]) == 2:
+            cls.assertRaisesRegex = cls.assertRaisesRegexp
+
+    def test_init(self):
+        declarations.Maybe('foo', 1, 2)
+
+        with self.assertRaisesRegex(TypeError, 'Inconsistent phases'):
+            declarations.Maybe('foo', declarations.LazyAttribute(None), declarations.PostGenerationDeclaration())
+
+
 class SelfAttributeTestCase(unittest.TestCase):
     def test_standard(self):
         a = declarations.SelfAttribute('foo.bar.baz')