alternate approach for lookup key logic

sydney-runkle · sydney-runkle · commit 3576b50ced36 · 2025-02-21T10:43:18.000-05:00
diff --git a/src/lookup_key.rs b/src/lookup_key.rs
@@ -578,30 +578,6 @@ fn py_get_attrs<'py>(obj: &Bound<'py, PyAny>, attr_name: &Py<PyString>) -> PyRes
     }
 }
 
-pub fn get_lookup_key(
-    py: Python,
-    validation_alias: Option<&Py<PyAny>>,
-    validate_by_name: bool,
-    validate_by_alias: bool,
-    field_name: &str,
-) -> PyResult<LookupKey> {
-    match validation_alias {
-        Some(va) => {
-            let va_bound = va.into_bound_py_any(py)?;
-            let lookup_key = match (validate_by_name, validate_by_alias) {
-                (true, true) => LookupKey::from_py(py, &va_bound, Some(field_name))?,
-                (true, false) => LookupKey::from_string(py, field_name),
-                (false, true) => LookupKey::from_py(py, &va_bound, None)?,
-                (false, false) => {
-                    return py_schema_err!("`validate_by_name` and `validate_by_alias` cannot both be set to `False`.")
-                }
-            };
-            Ok(lookup_key)
-        }
-        None => Ok(LookupKey::from_string(py, field_name)),
-    }
-}
-
 #[derive(Debug)]
 #[allow(clippy::struct_field_names)]
 pub struct LookupKeyCollection {
diff --git a/src/validators/arguments.rs b/src/validators/arguments.rs
@@ -11,7 +11,7 @@ use crate::build_tools::py_schema_err;
 use crate::build_tools::{schema_or_config_same, ExtraBehavior};
 use crate::errors::{ErrorTypeDefaults, ValError, ValLineError, ValResult};
 use crate::input::{Arguments, BorrowInput, Input, KeywordArgs, PositionalArgs, ValidationMatch};
-use crate::lookup_key::get_lookup_key;
+use crate::lookup_key::LookupKeyCollection;
 use crate::tools::SchemaDict;
 
 use super::validation_state::ValidationState;
@@ -44,7 +44,7 @@ struct Parameter {
     name: String,
     kwarg_key: Option<Py<PyString>>,
     validator: CombinedValidator,
-    alias: Option<Py<PyAny>>,
+    lookup_key_collection: LookupKeyCollection,
     mode: String,
 }
 
@@ -126,12 +126,16 @@ impl BuildValidator for ArgumentsValidator {
             } else if has_default {
                 had_default_arg = true;
             }
+
+            let validation_alias = arg.get_item(intern!(py, "alias"))?;
+            let lookup_key_collection = LookupKeyCollection::new(py, validation_alias, name.as_str())?;
+
             parameters.push(Parameter {
                 positional,
                 name,
                 kwarg_key,
                 validator,
-                alias: arg.get_item(intern!(py, "alias"))?.map(std::convert::Into::into),
+                lookup_key_collection,
                 mode: mode.to_string(),
             });
         }
@@ -193,7 +197,7 @@ impl Validator for ArgumentsValidator {
         let mut output_args: Vec<PyObject> = Vec::with_capacity(self.positional_params_count);
         let output_kwargs = PyDict::new(py);
         let mut errors: Vec<ValLineError> = Vec::new();
-        let mut used_kwargs: AHashSet<String> = AHashSet::with_capacity(self.parameters.len());
+        let mut used_kwargs: AHashSet<&str> = AHashSet::with_capacity(self.parameters.len());
 
         let validate_by_alias = state.validate_by_alias_or(self.validate_by_alias);
         let validate_by_name = state.validate_by_name_or(self.validate_by_name);
@@ -209,19 +213,17 @@ impl Validator for ArgumentsValidator {
             let mut kw_value = None;
             let mut kw_lookup_key = None;
             if parameter.mode == "keyword_only" || parameter.mode == "positional_or_keyword" {
-                kw_lookup_key = Some(get_lookup_key(
-                    py,
-                    parameter.alias.as_ref(),
-                    validate_by_name,
-                    validate_by_alias,
-                    &parameter.name,
-                )?);
+                kw_lookup_key = Some(
+                    parameter
+                        .lookup_key_collection
+                        .select(validate_by_alias, validate_by_name)?,
+                );
             }
 
             if let Some(kwargs) = args.kwargs() {
-                if let Some(ref lookup_key) = kw_lookup_key {
+                if let Some(lookup_key) = kw_lookup_key {
                     if let Some((lookup_path, value)) = kwargs.get_item(lookup_key)? {
-                        used_kwargs.insert(lookup_path.first_key().to_string());
+                        used_kwargs.insert(lookup_path.first_key());
                         kw_value = Some((lookup_path, value));
                     }
                 }
diff --git a/src/validators/dataclass.rs b/src/validators/dataclass.rs
@@ -12,7 +12,7 @@ use crate::errors::{ErrorType, ErrorTypeDefaults, ValError, ValLineError, ValRes
 use crate::input::{
     input_as_python_instance, Arguments, BorrowInput, Input, InputType, KeywordArgs, PositionalArgs, ValidationMatch,
 };
-use crate::lookup_key::get_lookup_key;
+use crate::lookup_key::LookupKeyCollection;
 use crate::tools::SchemaDict;
 use crate::validators::function::convert_err;
 
@@ -27,7 +27,7 @@ struct Field {
     py_name: Py<PyString>,
     init: bool,
     init_only: bool,
-    alias: Option<Py<PyAny>>,
+    lookup_key_collection: LookupKeyCollection,
     validator: CombinedValidator,
     frozen: bool,
 }
@@ -93,13 +93,14 @@ impl BuildValidator for DataclassArgsValidator {
                 positional_count += 1;
             }
 
+            let validation_alias = field.get_item(intern!(py, "validation_alias"))?;
+            let lookup_key_collection = LookupKeyCollection::new(py, validation_alias, name.as_str())?;
+
             fields.push(Field {
                 kw_only,
                 name,
                 py_name: py_name.into(),
-                alias: field
-                    .get_item(intern!(py, "validation_alias"))?
-                    .map(std::convert::Into::into),
+                lookup_key_collection,
                 validator,
                 init: field.get_as(intern!(py, "init"))?.unwrap_or(true),
                 init_only: field.get_as(intern!(py, "init_only"))?.unwrap_or(false),
@@ -151,7 +152,7 @@ impl Validator for DataclassArgsValidator {
         let mut init_only_args = self.init_only_count.map(Vec::with_capacity);
 
         let mut errors: Vec<ValLineError> = Vec::new();
-        let mut used_keys: AHashSet<String> = AHashSet::with_capacity(self.fields.len());
+        let mut used_keys: AHashSet<&str> = AHashSet::with_capacity(self.fields.len());
 
         let state = &mut state.rebind_extra(|extra| extra.data = Some(output_dict.clone()));
 
@@ -200,17 +201,14 @@ impl Validator for DataclassArgsValidator {
                 }
             }
 
-            let lookup_key = get_lookup_key(
-                py,
-                field.alias.as_ref(),
-                validate_by_name,
-                validate_by_alias,
-                &field.name,
-            )?;
+            let lookup_key = field
+                .lookup_key_collection
+                .select(validate_by_alias, validate_by_name)?;
+
             let mut kw_value = None;
             if let Some(kwargs) = args.kwargs() {
-                if let Some((lookup_path, value)) = kwargs.get_item(&lookup_key)? {
-                    used_keys.insert(lookup_path.first_key().to_string());
+                if let Some((lookup_path, value)) = kwargs.get_item(lookup_key)? {
+                    used_keys.insert(lookup_path.first_key());
                     kw_value = Some((lookup_path, value));
                 }
             }
diff --git a/src/validators/model_fields.rs b/src/validators/model_fields.rs
@@ -93,6 +93,13 @@ impl BuildValidator for ModelFieldsValidator {
             });
         }
 
+        let validate_by_alias = config.get_as(intern!(py, "validate_by_alias"))?.unwrap_or(true);
+        let validate_by_name = config.get_as(intern!(py, "validate_by_name"))?.unwrap_or(false);
+
+        if !validate_by_alias && !validate_by_name {
+            return py_schema_err!("`validate_by_name` and `validate_by_alias` cannot both be set to `False`.");
+        }
+
         Ok(Self {
             fields,
             model_name,
diff --git a/src/validators/typed_dict.rs b/src/validators/typed_dict.rs
@@ -10,7 +10,7 @@ use crate::input::BorrowInput;
 use crate::input::ConsumeIterator;
 use crate::input::ValidationMatch;
 use crate::input::{Input, ValidatedDict};
-use crate::lookup_key::get_lookup_key;
+use crate::lookup_key::LookupKeyCollection;
 use crate::tools::SchemaDict;
 use ahash::AHashSet;
 use jiter::PartialMode;
@@ -20,7 +20,7 @@ use super::{build_validator, BuildValidator, CombinedValidator, DefinitionsBuild
 #[derive(Debug)]
 struct TypedDictField {
     name: String,
-    alias: Option<Py<PyAny>>,
+    lookup_key_collection: LookupKeyCollection,
     name_py: Py<PyString>,
     required: bool,
     validator: CombinedValidator,
@@ -109,11 +109,12 @@ impl BuildValidator for TypedDictValidator {
                 }
             }
 
+            let validation_alias = field_info.get_item(intern!(py, "validation_alias"))?;
+            let lookup_key_collection = LookupKeyCollection::new(py, validation_alias, field_name)?;
+
             fields.push(TypedDictField {
                 name: field_name.to_string(),
-                alias: field_info
-                    .get_item(intern!(py, "validation_alias"))?
-                    .map(std::convert::Into::into),
+                lookup_key_collection,
                 name_py: field_name_py.into(),
                 validator,
                 required,
@@ -162,7 +163,7 @@ impl Validator for TypedDictValidator {
 
         // we only care about which keys have been used if we're iterating over the object for extra after
         // the first pass
-        let mut used_keys: Option<AHashSet<String>> =
+        let mut used_keys: Option<AHashSet<&str>> =
             if self.extra_behavior == ExtraBehavior::Ignore || dict.is_py_get_attr() {
                 None
             } else {
@@ -175,14 +176,10 @@ impl Validator for TypedDictValidator {
             let mut fields_set_count: usize = 0;
 
             for field in &self.fields {
-                let lookup_key = get_lookup_key(
-                    py,
-                    field.alias.as_ref(),
-                    validate_by_name,
-                    validate_by_alias,
-                    &field.name,
-                )?;
-                let op_key_value = match dict.get_item(&lookup_key) {
+                let lookup_key = field
+                    .lookup_key_collection
+                    .select(validate_by_alias, validate_by_name)?;
+                let op_key_value = match dict.get_item(lookup_key) {
                     Ok(v) => v,
                     Err(ValError::LineErrors(line_errors)) => {
                         let field_loc: LocItem = field.name.clone().into();
@@ -199,7 +196,7 @@ impl Validator for TypedDictValidator {
                     if let Some(ref mut used_keys) = used_keys {
                         // key is "used" whether or not validation passes, since we want to skip this key in
                         // extra logic either way
-                        used_keys.insert(lookup_path.first_key().to_string());
+                        used_keys.insert(lookup_path.first_key());
                     }
                     let is_last_partial = if let Some(ref last_key) = partial_last_key {
                         let first_key_loc: LocItem = lookup_path.first_key().into();
@@ -265,7 +262,7 @@ impl Validator for TypedDictValidator {
         if let Some(used_keys) = used_keys {
             struct ValidateExtras<'a, 's, 'py> {
                 py: Python<'py>,
-                used_keys: AHashSet<String>,
+                used_keys: AHashSet<&'a str>,
                 errors: &'a mut Vec<ValLineError>,
                 extras_validator: Option<&'a CombinedValidator>,
                 output_dict: &'a Bound<'py, PyDict>,
diff --git a/tests/validators/test_arguments.py b/tests/validators/test_arguments.py
@@ -928,19 +928,6 @@ def test_only_allow_alias(py_and_json) -> None:
         assert v.validate_test(ArgsKwargs((), {'a': 'hello'}))
 
 
-def test_invalid_config_raises() -> None:
-    with pytest.raises(SchemaError, match='`validate_by_name` and `validate_by_alias` cannot both be set to `False`.'):
-        SchemaValidator(
-            core_schema.arguments_schema(
-                [
-                    core_schema.arguments_parameter(name='a', schema=core_schema.str_schema(), alias='FieldA'),
-                ],
-                validate_by_name=False,
-                validate_by_alias=False,
-            )
-        )
-
-
 def validate(config=None):
     def decorator(function):
         parameters = signature(function).parameters
diff --git a/tests/validators/test_dataclasses.py b/tests/validators/test_dataclasses.py
@@ -8,7 +8,7 @@
 import pytest
 from dirty_equals import IsListOrTuple, IsStr
 
-from pydantic_core import ArgsKwargs, SchemaError, SchemaValidator, ValidationError, core_schema
+from pydantic_core import ArgsKwargs, SchemaValidator, ValidationError, core_schema
 
 from ..conftest import Err, PyAndJson, assert_gc
 
@@ -1774,22 +1774,3 @@ def test_only_allow_alias(py_and_json) -> None:
     assert v.validate_test({'FieldA': 'hello'}) == BasicDataclass(a='hello')
     with pytest.raises(ValidationError, match=r'FieldA\n +Field required \[type=missing,'):
         assert v.validate_test({'a': 'hello'})
-
-
-def test_invalid_config_raises() -> None:
-    with pytest.raises(SchemaError, match='`validate_by_name` and `validate_by_alias` cannot both be set to `False`.'):
-        SchemaValidator(
-            core_schema.dataclass_schema(
-                BasicDataclass,
-                core_schema.dataclass_args_schema(
-                    'BasicDataclass',
-                    [
-                        core_schema.dataclass_field(
-                            name='a', schema=core_schema.str_schema(), validation_alias='FieldA'
-                        ),
-                    ],
-                ),
-                ['a'],
-                config=core_schema.CoreConfig(validate_by_name=False, validate_by_alias=False),
-            )
-        )
diff --git a/tests/validators/test_model_fields.py b/tests/validators/test_model_fields.py
@@ -543,17 +543,6 @@ def test_only_allow_alias(py_and_json) -> None:
         assert v.validate_test({'field_a': '123'})
 
 
-def test_invalid_config_raises() -> None:
-    with pytest.raises(SchemaError, match='`validate_by_name` and `validate_by_alias` cannot both be set to `False`.'):
-        SchemaValidator(
-            {
-                'type': 'model-fields',
-                'fields': {'field_a': {'validation_alias': 'FieldA', 'type': 'model-field', 'schema': {'type': 'int'}}},
-            },
-            config=CoreConfig(validate_by_name=False, validate_by_alias=False),
-        )
-
-
 @pytest.mark.parametrize(
     'input_value,expected',
     [
diff --git a/tests/validators/test_typed_dict.py b/tests/validators/test_typed_dict.py
@@ -436,19 +436,6 @@ def test_only_allow_alias(py_and_json) -> None:
         assert v.validate_test({'field_a': '123'})
 
 
-def test_invalid_config_raises() -> None:
-    with pytest.raises(SchemaError, match='`validate_by_name` and `validate_by_alias` cannot both be set to `False`.'):
-        SchemaValidator(
-            {
-                'type': 'typed-dict',
-                'fields': {
-                    'field_a': {'validation_alias': 'FieldA', 'type': 'typed-dict-field', 'schema': {'type': 'int'}}
-                },
-                'config': {'validate_by_name': False, 'validate_by_alias': False},
-            }
-        )
-
-
 @pytest.mark.parametrize(
     'input_value,expected',
     [
