Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: only allow safe-precision integers to pass parsedInteger #744

Merged
merged 2 commits into from
May 4, 2023
Merged

feat: only allow safe-precision integers to pass parsedInteger #744

merged 2 commits into from
May 4, 2023

Conversation

thetayloredman
Copy link
Contributor

Fixes #743

This commit adds an additional check to the parsedInteger validator, using Number.isSafeInteger() to require that any parsed integer has maximum precision (cannot overflow). This better represents what the average user would want, but may fail on some valid inputs. An unsafeParsedInteger validator should be considered in the future.

Due to many upcoming changes to error scopes, 'string' was used as the base type and the regex check was made in the narrow. The error message when a non-string is passed has changed slightly, just saying "must be a string" instead of "must be a well-formed integer string." This is a very minor change and can be dismissed.

BREAKING CHANGE: Any unsafe integers that use the parsedInteger validator will now fail the validation.

Fixes #743

This commit adds an additional check to the `parsedInteger` validator,
using `Number.isSafeInteger()` to require that any parsed integer has
maximum precision (cannot overflow). This better represents what the
average user would want, but may fail on some valid inputs. An
`unsafeParsedInteger` validator should be considered in the future.

Due to many upcoming changes to error scopes, 'string' was used as the
base type and the regex check was made in the narrow. The error message
when a non-string is passed has changed slightly, just saying "must be a
string" instead of "must be a well-formed integer string." This is a
very minor change and can be dismissed.

BREAKING CHANGE: Any unsafe integers that use the `parsedInteger`
validator will now fail the validation.

Signed-Off-By: LogN <[email protected]>
@thetayloredman thetayloredman changed the title feat!: only allow safe-precesion integers to pass parsedInteger feat!: only allow safe-precision integers to pass parsedInteger May 4, 2023
@thetayloredman
Copy link
Contributor Author

test failure not caused by this pr

Copy link
Member

@ssalbdivad ssalbdivad left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great overall and I appreciate your thoroughness in detailing the motivation for the changes!

If we can add that last case to the test we can merge it right away ⛵

dev/test/keywords.test.ts Show resolved Hide resolved
@ssalbdivad
Copy link
Member

Honestly I'm not going to worry about the docgen related failure on Windows, I'll look into that when I'm getting the perf branch merged 👍

@ssalbdivad ssalbdivad changed the title feat!: only allow safe-precision integers to pass parsedInteger feat: only allow safe-precision integers to pass parsedInteger May 4, 2023
@ssalbdivad ssalbdivad merged commit a4387ec into arktypeio:main May 4, 2023
ahrjarrett pushed a commit to ahrjarrett/arktype-fork that referenced this pull request May 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done (merged or closed)
Development

Successfully merging this pull request may close these issues.

Furter limitations on parsedInteger
2 participants