Skip to content

Commit bcbf637

Browse files
ViicosViicos
committed
Feedback
1 parent b6a2ca1 commit bcbf637

File tree

5 files changed

+285
-27
lines changed

5 files changed

+285
-27
lines changed

‎python/pydantic_core/core_schema.py‎

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3551,9 +3551,7 @@ class ArgumentsV3Schema(TypedDict, total=False):
35513551
arguments_schema: Required[list[ArgumentsV3Parameter]]
35523552
validate_by_name: bool
35533553
validate_by_alias: bool
3554-
var_args_schema: CoreSchema
3555-
var_kwargs_mode: VarKwargsMode
3556-
var_kwargs_schema: CoreSchema
3554+
extra_behavior: Literal['forbid', 'ignore'] # 'allow' doesn't make sense here.
35573555
ref: str
35583556
metadata: dict[str, Any]
35593557
serialization: SerSchema
@@ -3564,6 +3562,7 @@ def arguments_v3_schema(
35643562
*,
35653563
validate_by_name: bool | None = None,
35663564
validate_by_alias: bool | None = None,
3565+
extra_behavior: Literal['forbid', 'ignore'] | None = None,
35673566
ref: str | None = None,
35683567
metadata: dict[str, Any] | None = None,
35693568
serialization: SerSchema | None = None,
@@ -3585,10 +3584,14 @@ def arguments_v3_schema(
35853584
assert v.validate_python({'a': 'hi', 'kwargs': {'b': True}}) == (('hi',), {'b': True})
35863585
```
35873586
3587+
This schema is currently not used by other Pydantic components. In V3, it will most likely
3588+
become the default arguments schema for the `'call'` schema.
3589+
35883590
Args:
35893591
arguments: The arguments to use for the arguments schema.
35903592
validate_by_name: Whether to populate by the parameter names, defaults to `False`.
35913593
validate_by_alias: Whether to populate by the parameter aliases, defaults to `True`.
3594+
extra_behavior: The extra behavior to use.
35923595
ref: optional unique identifier of the schema, used to reference the schema in other places.
35933596
metadata: Any other information you want to include with the schema, not used by pydantic-core.
35943597
serialization: Custom serialization schema.
@@ -3598,6 +3601,7 @@ def arguments_v3_schema(
35983601
arguments_schema=arguments,
35993602
validate_by_name=validate_by_name,
36003603
validate_by_alias=validate_by_alias,
3604+
extra_behavior=extra_behavior,
36013605
ref=ref,
36023606
metadata=metadata,
36033607
serialization=serialization,

‎src/validators/arguments_v3.rs‎

Lines changed: 131 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,9 @@ use pyo3::IntoPyObjectExt;
99

1010
use crate::build_tools::py_schema_err;
1111
use crate::build_tools::{schema_or_config_same, ExtraBehavior};
12+
use crate::errors::LocItem;
1213
use crate::errors::{ErrorTypeDefaults, ValError, ValLineError, ValResult};
14+
use crate::input::ConsumeIterator;
1315
use crate::input::{
1416
Arguments, BorrowInput, Input, KeywordArgs, PositionalArgs, ValidatedDict, ValidatedTuple, ValidationMatch,
1517
};
@@ -86,13 +88,22 @@ impl BuildValidator for ArgumentsV3Validator {
8688
let mut parameters: Vec<Parameter> = Vec::with_capacity(arguments_schema.len());
8789

8890
let mut had_default_arg = false;
91+
let mut had_positional_or_keyword = false;
92+
let mut had_var_args = false;
8993
let mut had_keyword_only = false;
94+
let mut had_var_kwargs = false;
95+
96+
let mut names: AHashSet<String> = AHashSet::with_capacity(arguments_schema.len());
9097

9198
for arg in arguments_schema.iter() {
9299
let arg = arg.downcast::<PyDict>()?;
93100

94101
let py_name: Bound<PyString> = arg.get_as_req(intern!(py, "name"))?;
95102
let name = py_name.to_string();
103+
if !names.insert(name.clone()) {
104+
return py_schema_err!("Duplicate parameter '{}'", name);
105+
}
106+
96107
let py_mode = arg.get_as::<Bound<'_, PyString>>(intern!(py, "mode"))?;
97108
let py_mode = py_mode
98109
.as_ref()
@@ -102,8 +113,51 @@ impl BuildValidator for ArgumentsV3Validator {
102113

103114
let mode = ParameterMode::from_str(py_mode)?;
104115

105-
if mode == ParameterMode::KeywordOnly {
106-
had_keyword_only = true;
116+
match mode {
117+
ParameterMode::PositionalOnly => {
118+
if had_positional_or_keyword || had_var_args || had_keyword_only || had_var_kwargs {
119+
return py_schema_err!(
120+
"Positional only parameter '{}' cannot follow other parameter kinds",
121+
name
122+
);
123+
}
124+
}
125+
ParameterMode::PositionalOrKeyword => {
126+
if had_var_args || had_keyword_only || had_var_kwargs {
127+
return py_schema_err!(
128+
"Positional or keyword parameter '{}' cannot follow variadic or keyword only parameters",
129+
name
130+
);
131+
}
132+
had_positional_or_keyword = true;
133+
}
134+
ParameterMode::VarArgs => {
135+
if had_var_args {
136+
return py_schema_err!("Duplicate variadic positional parameter '{}'", name);
137+
}
138+
if had_keyword_only || had_var_kwargs {
139+
return py_schema_err!(
140+
"Variadic positional parameter '{}' cannot follow variadic or keyword only parameters",
141+
name
142+
);
143+
}
144+
had_var_args = true;
145+
}
146+
ParameterMode::KeywordOnly => {
147+
if had_var_kwargs {
148+
return py_schema_err!(
149+
"Keyword only parameter '{}' cannot follow variadic keyword only parameter",
150+
name
151+
);
152+
}
153+
had_keyword_only = true;
154+
}
155+
ParameterMode::VarKwargsUniform | ParameterMode::VarKwargsUnpackedTypedDict => {
156+
if had_var_kwargs {
157+
return py_schema_err!("Duplicate variadic keyword parameter '{}'", name);
158+
}
159+
had_var_kwargs = true;
160+
}
107161
}
108162

109163
let schema = arg.get_as_req(intern!(py, "schema"))?;
@@ -188,12 +242,26 @@ impl ArgumentsV3Validator {
188242
let validate_by_alias = state.validate_by_alias_or(self.validate_by_alias);
189243
let validate_by_name = state.validate_by_name_or(self.validate_by_name);
190244

245+
// Keep track of used keys for extra behavior:
246+
let mut used_keys: Option<AHashSet<&str>> = if self.extra == ExtraBehavior::Ignore || mapping.is_py_get_attr() {
247+
None
248+
} else {
249+
Some(AHashSet::with_capacity(self.parameters.len()))
250+
};
251+
191252
for parameter in &self.parameters {
192253
let lookup_key = parameter
193254
.lookup_key_collection
194255
.select(validate_by_alias, validate_by_name)?;
256+
195257
// A value is present in the mapping:
196258
if let Some((lookup_path, dict_value)) = mapping.get_item(lookup_key)? {
259+
if let Some(ref mut used_keys) = used_keys {
260+
// key is "used" whether or not validation passes, since we want to skip this key in
261+
// extra logic either way
262+
used_keys.insert(lookup_path.first_key());
263+
}
264+
197265
match parameter.mode {
198266
ParameterMode::PositionalOnly | ParameterMode::PositionalOrKeyword => {
199267
match parameter.validator.validate(py, dict_value.borrow_input(), state) {
@@ -368,6 +436,66 @@ impl ArgumentsV3Validator {
368436
}
369437
}
370438

439+
if let Some(used_keys) = used_keys {
440+
struct ValidateExtra<'a> {
441+
used_keys: AHashSet<&'a str>,
442+
errors: &'a mut Vec<ValLineError>,
443+
extra_behavior: ExtraBehavior,
444+
}
445+
446+
impl<'py, Key, Value> ConsumeIterator<ValResult<(Key, Value)>> for ValidateExtra<'_>
447+
where
448+
Key: BorrowInput<'py> + Clone + Into<LocItem>,
449+
Value: BorrowInput<'py>,
450+
{
451+
type Output = ValResult<()>;
452+
fn consume_iterator(self, iterator: impl Iterator<Item = ValResult<(Key, Value)>>) -> Self::Output {
453+
for item_result in iterator {
454+
let (raw_key, value) = item_result?;
455+
let either_str = match raw_key
456+
.borrow_input()
457+
.validate_str(true, false)
458+
.map(ValidationMatch::into_inner)
459+
{
460+
Ok(k) => k,
461+
Err(ValError::LineErrors(line_errors)) => {
462+
for err in line_errors {
463+
self.errors.push(
464+
err.with_outer_location(raw_key.clone())
465+
.with_type(ErrorTypeDefaults::InvalidKey),
466+
);
467+
}
468+
continue;
469+
}
470+
Err(err) => return Err(err),
471+
};
472+
let cow = either_str.as_cow()?;
473+
if self.used_keys.contains(cow.as_ref()) {
474+
continue;
475+
}
476+
477+
let value = value.borrow_input();
478+
479+
if self.extra_behavior == ExtraBehavior::Forbid {
480+
self.errors.push(ValLineError::new_with_loc(
481+
ErrorTypeDefaults::ExtraForbidden,
482+
value,
483+
raw_key.clone(),
484+
));
485+
}
486+
}
487+
488+
Ok(())
489+
}
490+
}
491+
492+
mapping.iterate(ValidateExtra {
493+
used_keys,
494+
errors: &mut errors,
495+
extra_behavior: self.extra,
496+
})??;
497+
}
498+
371499
if !errors.is_empty() {
372500
Err(ValError::LineErrors(errors))
373501
} else {
@@ -397,7 +525,7 @@ impl ArgumentsV3Validator {
397525
let validate_by_alias = state.validate_by_alias_or(self.validate_by_alias);
398526
let validate_by_name = state.validate_by_name_or(self.validate_by_name);
399527

400-
// go through non variadic arguments, getting the value from args or kwargs and validating it
528+
// go through non variadic parameters, getting the value from args or kwargs and validating it
401529
for (index, parameter) in self.parameters.iter().filter(|p| !p.is_variadic()).enumerate() {
402530
let lookup_key = parameter
403531
.lookup_key_collection

‎tests/validators/arguments_v3/test_build_errors.py‎

Lines changed: 87 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,87 @@
1+
import pytest
2+
3+
from pydantic_core import SchemaError, SchemaValidator
4+
from pydantic_core import core_schema as cs
5+
6+
7+
def test_build_non_default_follows_default() -> None:
8+
with pytest.raises(SchemaError, match="Required parameter 'b' follows parameter with default"):
9+
SchemaValidator(
10+
schema=cs.arguments_v3_schema(
11+
[
12+
cs.arguments_v3_parameter(
13+
name='a',
14+
schema=cs.with_default_schema(schema=cs.int_schema(), default_factory=lambda: 42),
15+
mode='positional_or_keyword',
16+
),
17+
cs.arguments_v3_parameter(name='b', schema=cs.int_schema(), mode='positional_or_keyword'),
18+
]
19+
)
20+
)
21+
22+
23+
def test_duplicate_parameter_name() -> None:
24+
with pytest.raises(SchemaError, match="Duplicate parameter 'test'"):
25+
SchemaValidator(
26+
schema=cs.arguments_v3_schema(
27+
[
28+
cs.arguments_v3_parameter(name='test', schema=cs.int_schema()),
29+
cs.arguments_v3_parameter(name='a', schema=cs.int_schema()),
30+
cs.arguments_v3_parameter(name='test', schema=cs.int_schema()),
31+
]
32+
)
33+
)
34+
35+
36+
def test_invalid_positional_only_parameter_position() -> None:
37+
with pytest.raises(SchemaError, match="Positional only parameter 'test' cannot follow other parameter kinds"):
38+
SchemaValidator(
39+
schema=cs.arguments_v3_schema(
40+
[
41+
cs.arguments_v3_parameter(name='a', schema=cs.int_schema(), mode='var_args'),
42+
cs.arguments_v3_parameter(name='test', schema=cs.int_schema(), mode='positional_only'),
43+
]
44+
)
45+
)
46+
47+
48+
def test_invalid_positional_or_keyword_parameter_position() -> None:
49+
with pytest.raises(
50+
SchemaError, match="Positional or keyword parameter 'test' cannot follow variadic or keyword only parameters"
51+
):
52+
SchemaValidator(
53+
schema=cs.arguments_v3_schema(
54+
[
55+
cs.arguments_v3_parameter(name='a', schema=cs.int_schema(), mode='var_args'),
56+
cs.arguments_v3_parameter(name='test', schema=cs.int_schema(), mode='positional_or_keyword'),
57+
]
58+
)
59+
)
60+
61+
62+
def test_invalid_var_args_parameter_position() -> None:
63+
with pytest.raises(
64+
SchemaError, match="Variadic positional parameter 'test' cannot follow variadic or keyword only parameters"
65+
):
66+
SchemaValidator(
67+
schema=cs.arguments_v3_schema(
68+
[
69+
cs.arguments_v3_parameter(name='a', schema=cs.int_schema(), mode='keyword_only'),
70+
cs.arguments_v3_parameter(name='test', schema=cs.int_schema(), mode='var_args'),
71+
]
72+
)
73+
)
74+
75+
76+
def test_invalid_keyword_only_parameter_position() -> None:
77+
with pytest.raises(
78+
SchemaError, match="Keyword only parameter 'test' cannot follow variadic keyword only parameter"
79+
):
80+
SchemaValidator(
81+
schema=cs.arguments_v3_schema(
82+
[
83+
cs.arguments_v3_parameter(name='a', schema=cs.int_schema(), mode='var_kwargs_uniform'),
84+
cs.arguments_v3_parameter(name='test', schema=cs.int_schema(), mode='keyword_only'),
85+
]
86+
)
87+
)

‎tests/validators/arguments_v3/test_extra.py‎

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,57 @@
1+
import pytest
2+
3+
from pydantic_core import ArgsKwargs, ValidationError
4+
from pydantic_core import core_schema as cs
5+
6+
from ...conftest import PyAndJson
7+
8+
9+
@pytest.mark.parametrize(
10+
['input_value', 'err_type'],
11+
(
12+
[ArgsKwargs((), {'a': 1, 'b': 2, 'c': 3}), 'unexpected_keyword_argument'],
13+
[ArgsKwargs((), {'a': 1, 'c': 3, 'extra': 'value'}), 'unexpected_keyword_argument'],
14+
[{'a': 1, 'b': 2, 'c': 3}, 'extra_forbidden'],
15+
[{'a': 1, 'c': 3, 'extra': 'value'}, 'extra_forbidden'],
16+
),
17+
)
18+
def test_extra_forbid(py_and_json: PyAndJson, input_value, err_type) -> None:
19+
v = py_and_json(
20+
cs.arguments_v3_schema(
21+
[
22+
cs.arguments_v3_parameter(name='a', schema=cs.int_schema()),
23+
cs.arguments_v3_parameter(name='b', schema=cs.int_schema(), alias='c'),
24+
],
25+
extra_behavior='forbid',
26+
),
27+
)
28+
29+
with pytest.raises(ValidationError) as exc_info:
30+
v.validate_test(input_value)
31+
32+
error = exc_info.value.errors()[0]
33+
34+
assert error['type'] == err_type
35+
36+
37+
@pytest.mark.parametrize(
38+
'input_value',
39+
[
40+
ArgsKwargs((), {'a': 1, 'b': 2, 'c': 3}),
41+
ArgsKwargs((), {'a': 1, 'c': 3, 'extra': 'value'}),
42+
{'a': 1, 'b': 2, 'c': 3},
43+
{'a': 1, 'c': 3, 'extra': 'value'},
44+
],
45+
)
46+
def test_extra_ignore(py_and_json: PyAndJson, input_value) -> None:
47+
v = py_and_json(
48+
cs.arguments_v3_schema(
49+
[
50+
cs.arguments_v3_parameter(name='a', schema=cs.int_schema(), mode='keyword_only'),
51+
cs.arguments_v3_parameter(name='b', schema=cs.int_schema(), alias='c', mode='keyword_only'),
52+
],
53+
extra_behavior='ignore',
54+
),
55+
)
56+
57+
assert v.validate_test(input_value) == ((), {'a': 1, 'b': 3})

0 commit comments

Comments
 (0)