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

IsFloat validator should return a standard error message for empty strings #110

Merged

Conversation

froschdesign
Copy link
Member

Q A
Documentation no
Bugfix yes
BC Break no
New Feature no
RFC no
QA no

Description

The following code throws an AssertionError:

$validator = new Laminas\I18n\Validator\IsFloat();
var_dump($validator->isValid(''));
Fatal error: Uncaught AssertionError: assert($lastStringGroup !== '') in /src/Validator/IsFloat.php:242

// No strrpos() in wrappers yet. ICU 4.x doesn't have grouping size for
// everything. ICU 52 has 3 for ALL locales.
$groupSize = $formatter->getAttribute(NumberFormatter::GROUPING_SIZE) ?: 3;
assert(is_int($groupSize));
$lastStringGroup = $this->wrapper->strlen($value) > $groupSize ?
$this->wrapper->substr($value, 0 - $groupSize) :
$value;
assert(is_string($lastStringGroup));
assert($lastStringGroup !== '');

These lines were introduced with #104: 5062ef1

In previous versions this was not a problem:

$validator = new Laminas\I18n\Validator\IsFloat();
var_dump($validator->isValid('')); // false
var_dump($validator->getMessages()); ['notFloat' => 'The input does not appear to be a float']

@froschdesign froschdesign added the Bug Something isn't working label Nov 7, 2023
@gsteel gsteel added this to the 2.24.1 milestone Nov 7, 2023
@gsteel
Copy link
Member

gsteel commented Nov 7, 2023

@froschdesign
Look good to you? I can't request your review because you opened the patch!

@gsteel gsteel assigned froschdesign and unassigned gsteel Nov 7, 2023
@froschdesign froschdesign changed the title Adds unit test for isFloat validator that demonstrates an error with empty strings IsFloat validator should return a standard error message for empty strings Nov 7, 2023
@froschdesign froschdesign merged commit dafb5ed into laminas:2.24.x Nov 8, 2023
11 checks passed
@froschdesign froschdesign deleted the hotfix/isfloat-emptystring branch November 8, 2023 08:56
@froschdesign
Copy link
Member Author

@gsteel
Thank you for your help! 👍🏻

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants