Skip to content

Allow validated fields to work alongside defaulted ones - #707

Open
wbernoudy wants to merge 1 commit into
getml:mainfrom
wbernoudy:fix/default-fields-alongside-validated
Open

Allow validated fields to work alongside defaulted ones#707
wbernoudy wants to merge 1 commit into
getml:mainfrom
wbernoudy:fix/default-fields-alongside-validated

Conversation

@wbernoudy

Copy link
Copy Markdown

Fixes #700

Like outlined in my comment in that issue, this works by giving the constructor of rfl::Validator an "escape-hatch" using std::source_location. If the constructor sees that the caller is specifically the method rfl::parsing::Parser::read_struct_with_default(), it will skip validation, allowing the construction of the user's struct with all defaults.

This is definitely a hack and there may be dangers I am unaware of with this, but I thought I would make a PR to see if it was possible (and how bad it was!).

This is also explicitly requiring std::source_location; though compiler support for this is required by the library, I see there is some fallback code if that is not available. I am happy to re-work this PR a bit to allow a fallback (or at least not break compilation).

@gemini-code-assist

Copy link
Copy Markdown

Caution

The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased.

@liuzicheng1987

Copy link
Copy Markdown
Contributor

@wbernoudy there are multiple build failures in the test pipeline, could you take a look?

Also, what if people use the DefaultIfMissing processor? What if they wrap their validator in DefaultValue? In that case, the validator would no longer guarantee that the condition it sets always holds.

Also, it introduces brittleness to the code - by moving around the code a bit, you could inadvertently invalidate all of the validators.

To be honest, I am also a bit uncomfortable with the solution here...I understand that we use source_location in other places, but we do so in a very controlled manner and it is absolutely essential to the functionality of the library. In this case, it really solves a very minor problem (all you have to do is assign a default value to the field that does fulfill the validator's condition and then there's no problem), so I am unsure whether this is the right way forward and might create more problems in the future than it solves.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Reading JSON into a struct with a DefaultVal field and a size validator field always raises a size validation error

2 participants