Add NoneRequired error on optional fields

[NicolasPllr1]

Change Summary

This changes the nullable validator. When the inner validation fails, it now adds a NoneRequired error-line.

The first commit updates the nullable validator, while follow-up commits update test data. These test data become more verbose especially with recursive models. This is similar to what happens with union types when the validation fails, you get lots of error-lines.

I am not sure this is the 'correct' thing to do with optional types, but it's the simplest way I found to address pydantic#8852 and is inspired by the proposed solution there.

Note: maybe documentation could be updated following such a change. I decided to wait for feedback before proceeding further!

Example

The goal was to have this behavior on optional fields:

from typing import Optional
from pydantic import BaseModel

class User(BaseModel):
    age: Optional[int] # or an explicit union, e.g. int | None

User(age="hello")

pydantic_core._pydantic_core.ValidationError: 2 validation errors for User
age
  Input should be a valid integer, unable to parse string as an integer [type=int_parsing, input_value='hello', input_type=str]
    For further information visit https://errors.pydantic.dev/2.12/v/int_parsing
age
  Input should be None [type=none_required, input_value='hello', input_type=str]
    For further information visit https://errors.pydantic.dev/2.12/v/none_required

With this change, the errors one gets from an optional type - i.e. a type where None is an accepted value - look like the errors one gets when working with union types.

For example, if we go int | bool instead of int | None:

class User(BaseModel):
    # age: Optional[int]  # or an explicit union, e.g. int | None
    age: bool | int

User(age="hello")

pydantic_core._pydantic_core.ValidationError: 2 validation errors for User
age.bool
  Input should be a valid boolean, unable to interpret input [type=bool_parsing, input_value='hello', input_type=str]
    For further information visit https://errors.pydantic.dev/2.12/v/bool_parsing
age.int
  Input should be a valid integer, unable to parse string as an integer [type=int_parsing, input_value='hello', input_type=str]
    For further information visit https://errors.pydantic.dev/2.12/v/int_parsing

Both errors - with the union type and with the optional type - have the exact same shape now.

Related issue number

• A clearer ValidationError should be provided when an Optional type hint is used pydantic#8852

Checklist

• Unit tests for the changes exist (in the sense that existing unit tests cover nullable fields and I updated them)
• Documentation reflects the changes where applicable
• Pydantic tests pass with this pydantic-core (except for expected changes)
• My PR is ready to review, please add a comment including the phrase "please review" to assign reviewers

Selected Reviewer: @Viicos

[codecov]

Codecov Report

❌ Patch coverage is 0% with 7 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
src/validators/nullable.rs	0.00%	7 Missing ⚠️

📢 Thoughts on this report? Let us know!

[codspeed-hq]

CodSpeed Performance Report

Merging #1791 will not alter performance

_{Comparing NicolasPllr1:add-none-required-error-on-optional-fields (91b130b) with main (0cd11fe)}

Summary

✅ 163 untouched

[NicolasPllr1]

please review

[samuelcolvin]

Thanks for the contribution.

Surely this is going to be a breaking change for lots of people?

I think if we were to accept this, it would need to be behind a config flag, and opt in.

[NicolasPllr1]

Thanks for the quick reply!

Surely this is going to be a breaking change for lots of people?

I did not think of that ... If their program relies on the specific errors emitted / the number of errors, then yes it's a breaking change.

I think if we were to accept this, it would need to be behind a config flag, and opt in.

Happy to work on that if you think this makes sense / had value.

[davidhewitt]

As per the versioning policy https://docs.pydantic.dev/latest/version-policy/#pydantic-v2 I think adding additional error detail is permitted without considering it breaking.

I'd be quite up for having this. Consider e.g. a three-member union:

from pydantic import BaseModel

class User(BaseModel):
    age: int | float | None

user = User(age='abc')

ValidationError: 2 validation errors for User
age.int
  Input should be a valid integer, unable to parse string as an integer [type=int_parsing, input_value='abc', input_type=str]
    For further information visit https://errors.pydantic.dev/2.12/v/int_parsing
age.float
  Input should be a valid number, unable to parse string as a number [type=float_parsing, input_value='abc', input_type=str]
    For further information visit https://errors.pydantic.dev/2.12/v/float_parsing

It feels like we should have an age.None member there!

[Viicos]

I'd also be up to have this included without consider it breaking. Alternatively this could make it for V3, but I'm not sure introducing an opt-in mechanism under V2 is worth it (I don't think users will ever enable it, it's a welcomed improvement but not a necessity for end users).

[NicolasPllr1]

Is there something I can/should do or is it now fully on your hands ?

Thanks anyways for the work and answers you gave!

[davidhewitt]

I think we need to convince @samuelcolvin that this is fine to merge, and then we can ship it :)

[samuelcolvin]

We discussed this at length on our call today.

The decision is to go ahead with it, but test this feature against FastAPI and the list of other companies that we test with new versions of pydantic. If they all pass, or getting them to pass is trivial, we release it as a bug fix. If many of them fail or fixing them is non-trivial, we should defer this to v3.

[Viicos]

I ran the third-party tests for this branch in pydantic/pydantic#12313, and got 11 failing tests (16 in reality, but 5 of them are from another change awaiting test fixes in FastAPI). Two other third party tests are failing but because they seem to be broken currently.

However, quite some Pydantic tests are broken due to this. Granted, we do a lot of assertions on errors, but considering the short timeline before 2.12, I'd say we either defer this to the next release or v3. @NicolasPllr1 I can make sure to have this PR conflict free over time.