php-gettext / Gettext

PHP library to collect and manipulate gettext (.po, .mo, .php, .json, etc)
MIT License
690 stars 135 forks source link

fix plural forms validation #157

Closed soukicz closed 6 years ago

soukicz commented 7 years ago

My previous PR https://github.com/oscarotero/Gettext/pull/156 missed some characters in plural forms validation

oscarotero commented 7 years ago

Maybe an easier solution is just check whether the plural form has any letter different than n? Somethin like this (not tested):

if (preg_match('/[A-Ma-mO-Zo-z]/', str_replace('return ', '', $code)) {
    throw new \InvalidArgumentException('Invalid Plural form: ' . $code);
}

Just an idea, what do you think?

soukicz commented 7 years ago

It would be also better to check it in setter before variable substitution. I will updated it.

oscarotero commented 7 years ago

We can also test the regexp with all possible formulas to be sure that doesn't fail with any valid plural formula.

$languages = Gettext\Languages\Language::getAll();

foreach ($languages as $language) {
    //test $language->formula
}
soukicz commented 6 years ago

I have simplified validation and added testing for predefined plural forms.

oscarotero commented 6 years ago

Great. Thank you 👍