Repository: samuelcolvin/pydantic (python)

Issue:
Some recursive models do not require `update_forward_refs` and silently behave incorrectly
# Bug

Output of `python -c "import pydantic.utils; print(pydantic.utils.version_info())"`:
```
             pydantic version: 1.4
            pydantic compiled: False
                 install path: /Users/dmontague/Programming/oss/pydantic/pydantic
               python version: 3.7.4 (default, Oct 17 2019, 23:32:33)  [Clang 11.0.0 (clang-1100.0.33.8)]
                     platform: Darwin-19.0.0-x86_64-i386-64bit
     optional deps. installed: ['typing-extensions', 'email-validator', 'devtools']
```

This test passes on master, but it should not:

```py
from typing import Optional, Tuple

from pydantic import BaseModel


def test_parse_nested_tuple():
    class NestedTuple(BaseModel):
        x: Tuple[int, Optional["NestedTuple"]]

    obj = NestedTuple.parse_obj({'x': ('1', {'x': ('2', {'x': ('3', None)})})})
    assert obj.dict() == {'x': (1, {'x': ('2', {'x': ('3', None)})})}
    # Note above: the 2 and 3 did not get parsed into ints !!

    NestedTuple.update_forward_refs()
    obj = NestedTuple.parse_obj({'x': ('1', {'x': ('2', {'x': ('3', None)})})})
    assert obj.dict() == {'x': (1, {'x': (2, {'x': (3, None)})})}
```

Presumably we should either get an error at some point prior to the `NestedRootTuple.update_forward_refs()` call, or the two calls to `obj.dict()` should generate the same output.

I'm guessing this is related to the field being `Optional`, but I'm not sure. I discovered it while working on #1200 .

Pull request:
fix: force need of `update_forward_refs` in recursive models
## Change Summary
Currently in nested models, we only check if the type of the field is `ForwardRef` to raise an error.
But we could have `ForwardRef` inside a `Union`, a `Tuple` or anything else...
By not catching this we get a wrong behaviour

## Related issue number
closes #1201

<!-- Are there any issues opened that will be resolved by merging this change? -->

## Checklist

* [ ] Unit tests for the changes exist
* [ ] Tests pass on CI and coverage remains at 100%
* [ ] Documentation reflects the changes where applicable
* [ ] `changes/<pull request or issue id>-<github username>.md` file added describing change
  (see [changes/README.md](https://github.com/samuelcolvin/pydantic/blob/master/changes/README.md) for details)


Patch:
diff --git a/changes/1201-PrettyWood.md b/changes/1201-PrettyWood.md
new file mode 100644
index 000000000..d22cd864c
--- /dev/null
+++ b/changes/1201-PrettyWood.md
@@ -0,0 +1,1 @@
+Fix: some recursive models did not require `update_forward_refs` and silently behaved incorrectly
\ No newline at end of file
diff --git a/docs/usage/exporting_models.md b/docs/usage/exporting_models.md
index 8478457b8..26a6b0fa4 100644
--- a/docs/usage/exporting_models.md
+++ b/docs/usage/exporting_models.md
@@ -7,8 +7,8 @@ This is the primary way of converting a model to a dictionary. Sub-models will b
 
 Arguments:
 
-* `include`: fields to include in the returned dictionary; see [below](#advanced-include-exclude)
-* `exclude`: fields to exclude from the returned dictionary; see [below](#advanced-include-exclude)
+* `include`: fields to include in the returned dictionary; see [below](#advanced-include-and-exclude)
+* `exclude`: fields to exclude from the returned dictionary; see [below](#advanced-include-and-exclude)
 * `by_alias`: whether field aliases should be used as keys in the returned dictionary; default `False`
 * `exclude_unset`: whether fields which were not explicitly set when creating the model should
   be excluded from the returned dictionary; default `False`.
@@ -44,8 +44,8 @@ _(This script is complete, it should run "as is")_
 
 Arguments:
 
-* `include`: fields to include in the returned dictionary; see [below](#advanced-include-exclude)
-* `exclude`: fields to exclude from the returned dictionary; see [below](#advanced-include-exclude)
+* `include`: fields to include in the returned dictionary; see [below](#advanced-include-and-exclude)
+* `exclude`: fields to exclude from the returned dictionary; see [below](#advanced-include-and-exclude)
 * `update`: a dictionary of values to change when creating the copied model
 * `deep`: whether to make a deep copy of the new model; default `False`
 
@@ -64,8 +64,8 @@ only the value for the `__root__` key is serialised)
 
 Arguments:
 
-* `include`: fields to include in the returned dictionary; see [below](#advanced-include-exclude)
-* `exclude`: fields to exclude from the returned dictionary; see [below](#advanced-include-exclude)
+* `include`: fields to include in the returned dictionary; see [below](#advanced-include-and-exclude)
+* `exclude`: fields to exclude from the returned dictionary; see [below](#advanced-include-and-exclude)
 * `by_alias`: whether field aliases should be used as keys in the returned dictionary; default `False`
 * `exclude_unset`: whether fields which were not set when creating the model and have their default values should
   be excluded from the returned dictionary; default `False`.
diff --git a/pydantic/fields.py b/pydantic/fields.py
index 4a0469f97..ebe2d56ef 100644
--- a/pydantic/fields.py
+++ b/pydantic/fields.py
@@ -25,7 +25,7 @@ from typing import (
 from . import errors as errors_
 from .class_validators import Validator, make_generic_validator, prep_validators
 from .error_wrappers import ErrorWrapper
-from .errors import NoneIsNotAllowedError
+from .errors import ConfigError, NoneIsNotAllowedError
 from .types import Json, JsonWrapper
 from .typing import (
     NONE_TYPES,
@@ -565,6 +565,13 @@ class ModelField(Representation):
         self, v: Any, values: Dict[str, Any], *, loc: 'LocStr', cls: Optional['ModelOrDc'] = None
     ) -> 'ValidateReturn':
 
+        if self.type_.__class__ is ForwardRef:
+            assert cls is not None
+            raise ConfigError(
+                f'field "{self.name}" not yet prepared so type is still a ForwardRef, '
+                f'you might need to call {cls.__name__}.update_forward_refs().'
+            )
+
         errors: Optional['ErrorList']
         if self.pre_validators:
             v, errors = self._apply_validators(v, values, loc, cls, self.pre_validators)
diff --git a/pydantic/main.py b/pydantic/main.py
index 10a3ee889..786ee2296 100644
--- a/pydantic/main.py
+++ b/pydantic/main.py
@@ -34,15 +34,7 @@ from .json import custom_pydantic_encoder, pydantic_encoder
 from .parse import Protocol, load_file, load_str_bytes
 from .schema import default_ref_template, model_schema
 from .types import PyObject, StrBytes
-from .typing import (
-    AnyCallable,
-    ForwardRef,
-    get_args,
-    get_origin,
-    is_classvar,
-    resolve_annotations,
-    update_field_forward_refs,
-)
+from .typing import AnyCallable, get_args, get_origin, is_classvar, resolve_annotations, update_field_forward_refs
 from .utils import (
     ROOT_KEY,
     ClassAttribute,
@@ -968,12 +960,6 @@ def validate_model(  # noqa: C901 (ignore complexity)
             return {}, set(), ValidationError([ErrorWrapper(exc, loc=ROOT_KEY)], cls_)
 
     for name, field in model.__fields__.items():
-        if field.type_.__class__ == ForwardRef:
-            raise ConfigError(
-                f'field "{field.name}" not yet prepared so type is still a ForwardRef, '
-                f'you might need to call {cls_.__name__}.update_forward_refs().'
-            )
-
         value = input_data.get(field.alias, _missing)
         using_name = False
         if value is _missing and config.allow_population_by_field_name and field.alt_alias:


Test patch:
diff --git a/tests/test_forward_ref.py b/tests/test_forward_ref.py
index f762eb11c..459c384dc 100644
--- a/tests/test_forward_ref.py
+++ b/tests/test_forward_ref.py
@@ -1,8 +1,9 @@
 import sys
+from typing import Optional, Tuple
 
 import pytest
 
-from pydantic import ConfigError, ValidationError
+from pydantic import BaseModel, ConfigError, ValidationError
 
 skip_pre_37 = pytest.mark.skipif(sys.version_info < (3, 7), reason='testing >= 3.7 behaviour only')
 
@@ -477,3 +478,19 @@ def test_forward_ref_with_create_model(create_module):
         Main = pydantic.create_model('Main', sub=('Sub', ...), __module__=__name__)
         instance = Main(sub={})
         assert instance.sub.dict() == {'foo': 'bar'}
+
+
+def test_nested_forward_ref():
+    class NestedTuple(BaseModel):
+        x: Tuple[int, Optional['NestedTuple']]  # noqa: F821
+
+    with pytest.raises(ConfigError) as exc_info:
+        NestedTuple.parse_obj({'x': ('1', {'x': ('2', {'x': ('3', None)})})})
+    assert str(exc_info.value) == (
+        'field "x_1" not yet prepared so type is still a ForwardRef, '
+        'you might need to call NestedTuple.update_forward_refs().'
+    )
+
+    NestedTuple.update_forward_refs()
+    obj = NestedTuple.parse_obj({'x': ('1', {'x': ('2', {'x': ('3', None)})})})
+    assert obj.dict() == {'x': (1, {'x': (2, {'x': (3, None)})})}