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

fix: remove traditional validation rule param types (1/2) #8078

Merged

Conversation

kenjis
Copy link
Member

@kenjis kenjis commented Oct 23, 2023

Description
Fixes #6489

To add declare(strict_types=1). See #8072
There is no guarantee that a string will be passed, so declaring strict types may result in a TypeError.

Checklist:

  • Securely signed commits
  • Component(s) with PHPDoc blocks, only if necessary or adds value
  • Unit testing, with >80% coverage
  • User guide updated
  • Conforms to style guide

@kenjis kenjis added refactor Pull requests that refactor code breaking change Pull requests that may break existing functionalities 4.5 labels Oct 23, 2023
@kenjis kenjis force-pushed the fix-traditional-validation-rule-param-types branch from b0ff3b4 to 690d996 Compare October 23, 2023 02:39
@kenjis kenjis changed the title refactor: remove traditional validation rule param types fix: remove traditional validation rule param types Oct 23, 2023
@kenjis kenjis added bug Verified issues on the current code behavior or pull requests that will fix them docs needed Pull requests needing documentation write-ups and/or revisions. and removed refactor Pull requests that refactor code labels Oct 23, 2023
Copy link
Member

@MGatner MGatner left a comment

Choose a reason for hiding this comment

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

If the intent is to support anything that would have been juggled to a string then shouldn't our @param tags be scalar|null instead? And maybe also Stringable?

@kenjis
Copy link
Member Author

kenjis commented Oct 26, 2023

I don't see much point in thinking too much about @param.

The traditional validation rules assumed strings. They were originally designed to validate POST data, so the values are all strings. null is for when the key of the array data to validate is missing.

I don't want the traditional rules to be used and would like to remove them, but I am not sure if apps migrated from older CIs will not have any problems when switching to the strict rules..

@kenjis kenjis force-pushed the fix-traditional-validation-rule-param-types branch from 690d996 to 776b9cf Compare October 27, 2023 04:21
@kenjis kenjis removed the docs needed Pull requests needing documentation write-ups and/or revisions. label Oct 27, 2023
@kenjis
Copy link
Member Author

kenjis commented Oct 27, 2023

Added docs.

@MGatner
Copy link
Member

MGatner commented Oct 27, 2023

Fair enough. This is basically a "pre-deprecation" change required so we can enact strict types.

@kenjis kenjis merged commit 76edbf4 into codeigniter4:4.5 Oct 27, 2023
46 checks passed
@kenjis kenjis deleted the fix-traditional-validation-rule-param-types branch October 27, 2023 21:33
@kenjis kenjis changed the title fix: remove traditional validation rule param types fix: remove traditional validation rule param types (1/2) Oct 30, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
4.5 breaking change Pull requests that may break existing functionalities bug Verified issues on the current code behavior or pull requests that will fix them
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants