Fix :: Signature generation with extra: allow never uses a field na…

…me (#1420)

* fix :: signature generation does not use field names

fix #1418

* fix :: use field.outer_type_ when generating signature

`extra_data: Dict[str, str]` would generate `extra_data: str` in the signature

* tweak to keep signature kwargs name

Co-authored-by: Samuel Colvin <s@muelcolvin.com>

changes/1418-prettywood.md

@@ -0,0 +1 @@
+Signature generation with `extra: allow` never uses a field name

pydantic/utils.py

@@ -174,13 +174,31 @@ def generate_model_signature(
 
             # TODO: replace annotation with actual expected types once #1055 solved
             kwargs = {'default': field.default} if not field.required else {}
-            merged_params[param_name] = Parameter(param_name, Parameter.KEYWORD_ONLY, annotation=field.type_, **kwargs)
+            merged_params[param_name] = Parameter(
+                param_name, Parameter.KEYWORD_ONLY, annotation=field.outer_type_, **kwargs
+            )
 
     if config.extra is config.extra.allow:
         use_var_kw = True
 
     if var_kw and use_var_kw:
-        merged_params[var_kw.name] = var_kw
+        # Make sure the parameter for extra kwargs
+        # does not have the same name as a field
+        default_model_signature = [
+            ('__pydantic_self__', Parameter.POSITIONAL_OR_KEYWORD),
+            ('data', Parameter.VAR_KEYWORD),
+        ]
+        if [(p.name, p.kind) for p in present_params] == default_model_signature:
+            # if this is the standard model signature, use extra_data as the extra args name
+            var_kw_name = 'extra_data'
+        else:
+            # else start from var_kw
+            var_kw_name = var_kw.name
+
+        # generate a name that's definitely unique
+        while var_kw_name in fields:
+            var_kw_name += '_'
+        merged_params[var_kw_name] = var_kw.replace(name=var_kw_name)
 
     return Signature(parameters=list(merged_params.values()), return_annotation=None)
 

tests/test_model_signature.py

@@ -71,7 +71,7 @@ def test_invalid_identifiers_signature():
     )
     assert _equals(str(signature(model)), '(*, valid_identifier: int = 123, yeah: int = 0) -> None')
     model = create_model('Model', **{'123 invalid identifier!': 123, '!': Field(0, alias='yeah')})
-    assert _equals(str(signature(model)), '(*, yeah: int = 0, **data: Any) -> None')
+    assert _equals(str(signature(model)), '(*, yeah: int = 0, **extra_data: Any) -> None')
 
 
 def test_use_field_name():
@@ -82,3 +82,47 @@ class Config:
             allow_population_by_field_name = True
 
     assert _equals(str(signature(Foo)), '(*, foo: str) -> None')
+
+
+def test_extra_allow_no_conflict():
+    class Model(BaseModel):
+        spam: str
+
+        class Config:
+            extra = Extra.allow
+
+    assert _equals(str(signature(Model)), '(*, spam: str, **extra_data: Any) -> None')
+
+
+def test_extra_allow_conflict():
+    class Model(BaseModel):
+        extra_data: str
+
+        class Config:
+            extra = Extra.allow
+
+    assert _equals(str(signature(Model)), '(*, extra_data: str, **extra_data_: Any) -> None')
+
+
+def test_extra_allow_conflict_twice():
+    class Model(BaseModel):
+        extra_data: str
+        extra_data_: str
+
+        class Config:
+            extra = Extra.allow
+
+    assert _equals(str(signature(Model)), '(*, extra_data: str, extra_data_: str, **extra_data__: Any) -> None')
+
+
+def test_extra_allow_conflict_custom_signature():
+    class Model(BaseModel):
+        extra_data: int
+
+        def __init__(self, extra_data: int = 1, **foobar: Any):
+            super().__init__(extra_data=extra_data, **foobar)
+
+        class Config:
+            extra = Extra.allow
+
+    assert _equals(str(signature(Model)), '(extra_data: int = 1, **foobar: Any) -> None')