run `strip_whitespace` before length checks on strings

[samuelcolvin]

As per the zen of pydantic, we guarantee the output type, rather guarantee to check the output type.

Also check other such changes are done before checks on strings.

[huyta55]

Hey, I would like to work on this issue. Could you point me towards the directory that this issue is referring to? It would help me a ton in getting started.

[huyta55]

From what I understand so far, is the issue around here?

pydantic-core/src/validators/string.rs

Line 95 in 0e6a09b

if let Some(min_length) = self.min_length {

Is that what you meant before length checks? so I would run strip_whitespace before this?

[samuelcolvin]

Yes.

[huyta55]

gotcha tysm for confirming!

[huyta55]

Hi, so I am trying to solve the issue right now and I think what I did with the actual code in string.rs is correct (I moved the if self.strip_whitespace { str = str.trim(); } to before the min_length and max_length check starts. However, the change is now causing some tests to fail due to the order of strip_whitespace changing. I am trying to change the test to fit the new order logic right now, but I would like to know what the rules and processes are for this repo regarding branches, PRs, commits, etc. I tried to create my own branch to have my changes uploaded in case I needed help but I cannot find it, even though VS Code is detecting it with git. If you could give me some instructions regarding that it would be amazing.

[samuelcolvin]

This is just github and git, nothing special to this repo. Before contributing to other's repos, best if you get familiar with these tools first.

In this case, you need to:

• fork the repo (using the button in the top right on the home page of the repo)
• clone your fork using git
• change branch to a new branch named something like fix-strip_whitespace-ordering
• make the changes
• push your code to github
• go to the home page of this repo and you should be prompted to create a pull request using your branch

[huyta55]

Ok so I managed to create my own fork, branch in that fork, then I have pushed my changes to that branch on git. I fixed one of the test logic issue that arose due to me changing the order of strip_whitespace before the length checks, but I am not sure how to fix the broken tests that are checking to match with regex patterns.

The specific tests I referring to validates this: ({'strip_whitespace': True, 'pattern': r'\d+$'}, 'foobar 123 ', Err("String should match pattern '\\d+$'")),. This hypothetically should still work with the change in strip_whitespace order since 'foobar 123' will just become 'foobar123', which should still fail the pattern check. However, for some reason the regex is matching and not throwing the desired error.

I've been trying to see the value of &self.pattern before the pattern check by printing/logging self.pattern_str before the pattern check code: https://github.com/huyta55/pydantic-core/blob/b1602dae88e1e3a53755b440a07c618fe059ebc0/src/validators/string.rs#L109 but it would not print or log in pytest for some reason, even though the test is failing.

Do you have any idea as to why the test inputs would all the sudden be passing the regex check? or how to get the value of the pattern being compared to show up in the terminal when the test fails?

[samuelcolvin]

Pytest doesn't print output unless either the test fails or you add -s. Please read the docs or use Google.

The regex only has an end bound, not a start bound ^.

[huyta55]

#264
I created a PR for my proposed changes to solve this issue.

[PrettyWood]

closed by #264