Repository: samuelcolvin/pydantic
Pull request #2250 (2021-01-10 17:13:39)

fix(decorator): detect when a duplicate argument is passed in
<!-- Thank you for your contribution! -->
<!-- Unless your change is trivial, please create an issue to discuss the change before creating a PR -->
<!-- See https://pydantic-docs.helpmanual.io/contributing/ for help on Contributing -->

## Change Summary

Currently, it is possible to override a positional argument with a keyword argument using the decorator.

In native python, this raises a TypeError.

Detect this usecase, to prevent keywords overriding positional parameters.


## Related issue number

<!-- Are there any issues opened that will be resolved by merging this change? -->

Closes: #2249

## Checklist

* [x] Unit tests for the changes exist
* [x] Tests pass on CI and coverage remains at 100%
* [x] Documentation reflects the changes where applicable
* [x] `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)


codecov[bot]: # [Codecov](https://codecov.io/gh/samuelcolvin/pydantic/pull/2250?src=pr&el=h1) Report
> Merging [#2250](https://codecov.io/gh/samuelcolvin/pydantic/pull/2250?src=pr&el=desc) (bf4774b) into [master](https://codecov.io/gh/samuelcolvin/pydantic/commit/13a5c7d676167b415080de5e6e6a74bea095b239?el=desc) (13a5c7d) will **decrease** coverage by `0.42%`.
> The diff coverage is `100.00%`.


```diff
@@             Coverage Diff             @@
##            master    #2250      +/-   ##
===========================================
- Coverage   100.00%   99.57%   -0.43%     
===========================================
  Files           21       21              
  Lines         4199     4211      +12     
  Branches       854      856       +2     
===========================================
- Hits          4199     4193       -6     
- Misses           0       13      +13     
- Partials         0        5       +5     
```


| [Impacted Files](https://codecov.io/gh/samuelcolvin/pydantic/pull/2250?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [pydantic/decorator.py](https://codecov.io/gh/samuelcolvin/pydantic/pull/2250/diff?src=pr&el=tree#diff-cHlkYW50aWMvZGVjb3JhdG9yLnB5) | `100.00% <100.00%> (ø)` | |
| [pydantic/version.py](https://codecov.io/gh/samuelcolvin/pydantic/pull/2250/diff?src=pr&el=tree#diff-cHlkYW50aWMvdmVyc2lvbi5weQ==) | `88.23% <0.00%> (-11.77%)` | :arrow_down: |
| [pydantic/typing.py](https://codecov.io/gh/samuelcolvin/pydantic/pull/2250/diff?src=pr&el=tree#diff-cHlkYW50aWMvdHlwaW5nLnB5) | `92.90% <0.00%> (-7.10%)` | :arrow_down: |
| [pydantic/env\_settings.py](https://codecov.io/gh/samuelcolvin/pydantic/pull/2250/diff?src=pr&el=tree#diff-cHlkYW50aWMvZW52X3NldHRpbmdzLnB5) | `97.93% <0.00%> (-2.07%)` | :arrow_down: |
| [pydantic/networks.py](https://codecov.io/gh/samuelcolvin/pydantic/pull/2250/diff?src=pr&el=tree#diff-cHlkYW50aWMvbmV0d29ya3MucHk=) | `99.26% <0.00%> (-0.74%)` | :arrow_down: |
| [pydantic/utils.py](https://codecov.io/gh/samuelcolvin/pydantic/pull/2250/diff?src=pr&el=tree#diff-cHlkYW50aWMvdXRpbHMucHk=) | `99.30% <0.00%> (-0.70%)` | :arrow_down: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/samuelcolvin/pydantic/pull/2250?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/samuelcolvin/pydantic/pull/2250?src=pr&el=footer). Last update [13a5c7d...bf4774b](https://codecov.io/gh/samuelcolvin/pydantic/pull/2250?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).


Patch:
diff --git a/changes/2249-cybojenix.md b/changes/2249-cybojenix.md
new file mode 100644
index 000000000..ad7021f59
--- /dev/null
+++ b/changes/2249-cybojenix.md
@@ -0,0 +1,2 @@
+Prevent overriding positional arguments with keyword arguments in
+`validate_arguments`, as per behaviour with native functions.
\ No newline at end of file
diff --git a/docs/examples/validation_decorator_parameter_types.py b/docs/examples/validation_decorator_parameter_types.py
index 0f80133b5..9d0b59148 100644
--- a/docs/examples/validation_decorator_parameter_types.py
+++ b/docs/examples/validation_decorator_parameter_types.py
@@ -65,4 +65,4 @@ def armageddon(
 
 
 print(armageddon(1, 2, e=3))
-print(armageddon(1, 2, 3, 4, 5, 6, c=7, e=8, f=9, g=10, spam=11))
+print(armageddon(1, 2, 3, 4, 5, 6, e=8, f=9, g=10, spam=11))
diff --git a/pydantic/decorator.py b/pydantic/decorator.py
index 85d04c610..383fa2fef 100644
--- a/pydantic/decorator.py
+++ b/pydantic/decorator.py
@@ -66,6 +66,7 @@ def validate_arguments(func: Optional['AnyCallableT'] = None, *, config: 'Config
 ALT_V_ARGS = 'v__args'
 ALT_V_KWARGS = 'v__kwargs'
 V_POSITIONAL_ONLY_NAME = 'v__positional_only'
+V_DUPLICATE_KWARGS = 'v__duplicate_kwargs'
 
 
 class ValidatedFunction:
@@ -74,10 +75,10 @@ class ValidatedFunction:
 
         parameters: Mapping[str, Parameter] = signature(function).parameters
 
-        if parameters.keys() & {ALT_V_ARGS, ALT_V_KWARGS, V_POSITIONAL_ONLY_NAME}:
+        if parameters.keys() & {ALT_V_ARGS, ALT_V_KWARGS, V_POSITIONAL_ONLY_NAME, V_DUPLICATE_KWARGS}:
             raise ConfigError(
-                f'"{ALT_V_ARGS}", "{ALT_V_KWARGS}" and "{V_POSITIONAL_ONLY_NAME}" are not permitted as argument '
-                f'names when using the "{validate_arguments.__name__}" decorator'
+                f'"{ALT_V_ARGS}", "{ALT_V_KWARGS}", "{V_POSITIONAL_ONLY_NAME}" and "{V_DUPLICATE_KWARGS}" '
+                f'are not permitted as argument names when using the "{validate_arguments.__name__}" decorator'
             )
 
         self.raw_function = function
@@ -105,6 +106,7 @@ class ValidatedFunction:
             elif p.kind == Parameter.POSITIONAL_OR_KEYWORD:
                 self.arg_mapping[i] = name
                 fields[name] = annotation, default
+                fields[V_DUPLICATE_KWARGS] = List[str], None
             elif p.kind == Parameter.KEYWORD_ONLY:
                 fields[name] = annotation, default
             elif p.kind == Parameter.VAR_POSITIONAL:
@@ -161,11 +163,14 @@ class ValidatedFunction:
 
         var_kwargs = {}
         wrong_positional_args = []
+        duplicate_kwargs = []
         non_var_fields = set(self.model.__fields__) - {self.v_args_name, self.v_kwargs_name}
         for k, v in kwargs.items():
             if k in non_var_fields:
                 if k in self.positional_only_args:
                     wrong_positional_args.append(k)
+                if k in values:
+                    duplicate_kwargs.append(k)
                 values[k] = v
             else:
                 var_kwargs[k] = v
@@ -174,6 +179,8 @@ class ValidatedFunction:
             values[self.v_kwargs_name] = var_kwargs
         if wrong_positional_args:
             values[V_POSITIONAL_ONLY_NAME] = wrong_positional_args
+        if duplicate_kwargs:
+            values[V_DUPLICATE_KWARGS] = duplicate_kwargs
         return values
 
     def execute(self, m: BaseModel) -> Any:
@@ -246,6 +253,12 @@ class ValidatedFunction:
                 keys = ', '.join(map(repr, v))
                 raise TypeError(f'positional-only argument{plural} passed as keyword argument{plural}: {keys}')
 
+            @validator(V_DUPLICATE_KWARGS, check_fields=False, allow_reuse=True)
+            def check_duplicate_kwargs(cls, v: List[str]) -> None:
+                plural = '' if len(v) == 1 else 's'
+                keys = ', '.join(map(repr, v))
+                raise TypeError(f'multiple values for argument{plural}: {keys}')
+
             class Config(CustomConfig):
                 extra = Extra.forbid
 


Test patch:
diff --git a/tests/test_decorator.py b/tests/test_decorator.py
index fa668d5f5..83d159c10 100644
--- a/tests/test_decorator.py
+++ b/tests/test_decorator.py
@@ -52,6 +52,20 @@ def test_args():
         {'loc': ('kwargs',), 'msg': "unexpected keyword argument: 'apple'", 'type': 'type_error'}
     ]
 
+    with pytest.raises(ValidationError) as exc_info:
+        foo(1, 2, a=3)
+
+    assert exc_info.value.errors() == [
+        {'loc': ('v__duplicate_kwargs',), 'msg': "multiple values for argument: 'a'", 'type': 'type_error'}
+    ]
+
+    with pytest.raises(ValidationError) as exc_info:
+        foo(1, 2, a=3, b=4)
+
+    assert exc_info.value.errors() == [
+        {'loc': ('v__duplicate_kwargs',), 'msg': "multiple values for arguments: 'a', 'b'", 'type': 'type_error'}
+    ]
+
 
 def test_wrap():
     @validate_arguments
@@ -68,7 +82,7 @@ def test_wrap():
     assert foo_bar.vd.arg_mapping == {0: 'a', 1: 'b'}
     assert foo_bar.vd.positional_only_args == set()
     assert issubclass(foo_bar.model, BaseModel)
-    assert foo_bar.model.__fields__.keys() == {'a', 'b', 'args', 'kwargs'}
+    assert foo_bar.model.__fields__.keys() == {'a', 'b', 'args', 'kwargs', 'v__duplicate_kwargs'}
     assert foo_bar.model.__name__ == 'FooBar'
     assert foo_bar.model.schema()['title'] == 'FooBar'
     # signature is slightly different on 3.6
@@ -168,7 +182,7 @@ def test_args_name():
     def foo(args: int, kwargs: int):
         return f'args={args!r}, kwargs={kwargs!r}'
 
-    assert foo.model.__fields__.keys() == {'args', 'kwargs', 'v__args', 'v__kwargs'}
+    assert foo.model.__fields__.keys() == {'args', 'kwargs', 'v__args', 'v__kwargs', 'v__duplicate_kwargs'}
     assert foo(1, 2) == 'args=1, kwargs=2'
 
     with pytest.raises(ValidationError) as exc_info:
@@ -191,10 +205,36 @@ def test_args_name():
 
 
 def test_v_args():
-    with pytest.raises(ConfigError, match='"v__args", "v__kwargs" and "v__positional_only" are not permitted'):
+    with pytest.raises(
+        ConfigError, match='"v__args", "v__kwargs", "v__positional_only" and "v__duplicate_kwargs" are not permitted'
+    ):
+
+        @validate_arguments
+        def foo1(v__args: int):
+            pass
+
+    with pytest.raises(
+        ConfigError, match='"v__args", "v__kwargs", "v__positional_only" and "v__duplicate_kwargs" are not permitted'
+    ):
+
+        @validate_arguments
+        def foo2(v__kwargs: int):
+            pass
+
+    with pytest.raises(
+        ConfigError, match='"v__args", "v__kwargs", "v__positional_only" and "v__duplicate_kwargs" are not permitted'
+    ):
+
+        @validate_arguments
+        def foo3(v__positional_only: int):
+            pass
+
+    with pytest.raises(
+        ConfigError, match='"v__args", "v__kwargs", "v__positional_only" and "v__duplicate_kwargs" are not permitted'
+    ):
 
         @validate_arguments
-        def foo(v__args: int):
+        def foo4(v__duplicate_kwargs: int):
             pass