Extended PO parsing logic with validation rules (resolves #75)

[vkhytskyi-allegro]

The aim of this PR is to introduce validation rules for PO parsing logic. Those rules check that:

• there is exactly zero or one msgid_plural definition per translation entry; a Multiple msgid_plural error error gets thrown otherwise.
• there are no duplicate entries with exact msgid values; a Duplicate msgid error error gets thrown otherwise.
• the number of plural forms matches exactly the number from nplurals defined in Plural-Forms header for entries that have plural forms; a Plural forms range error error gets thrown otherwise.
• the number of msgstr matches exacty the one (if msgid_plural is not defined) or the number from nplurals (if msgid_plural is defined); a Translation string range error error gets thrown otherwise.

[smhg]

@vkhytskyi-allegro Can you please also add usage information to the README? That helps me to understand the changes too.

[vkhytskyi-allegro]

@smhg The aim of this PR is to introduce validation rules for PO parsing logic. Those rules check that:

• there is exactly zero or one msgid_plural definition per translation entry; a Multiple msgid_plural error error gets thrown otherwise.
• there are no duplicate entries with exact msgid values; a Duplicate msgid error error gets thrown otherwise.
• the number of plural forms matches exactly the number from nplurals defined in Plural-Forms header for entries that have plural forms; a Plural forms range error error gets thrown otherwise.
• the number of msgstr matches exacty the one (if msgid_plural is not defined) or the number from nplurals (if msgid_plural is defined); a Translation string range error error gets thrown otherwise.

Since the idea of the PR is to enable those rules permanently, I am not sure whether there is anything to add to README. Please correct me if I am wrong.

[vkhytskyi-allegro]

Hey @smhg 👋🏼 Is there a chance you could take a look at my last reply? Thanks!

[smhg]

Thank you for following up on this @vkhytskyi-allegro. I did not find the time yet to move this forward. I will try to add questions when possible. These are mainly meant for me to get insights into this PR. Don't interpret them as any criticism. My responsibility is to make sure the impact these changes have on other (current and future) users are sensible.

[smhg]

I haven't yet looked at the actual _validateToken method but shouldn't we put validation behind a flag? While we could release this as a new major version with it being enabled by default, users will likely still want to turn this off. @vkhytskyi-allegro @KubaZ @szczepanekpawel Does that sound reasonable?

[vkhytskyi-allegro]

I will try to add questions when possible. These are mainly meant for me to get insights into this PR. Don't interpret them as any criticism. My responsibility is to make sure the impact these changes have on other (current and future) users are sensible.

Sure, I absolutely understand that and open to make changes in the implementation the way you see fit 🙂

[vkhytskyi-allegro]

@smhg / @KubaZ / @szczepanekpawel the PR is ready for another round of review 😉

Changes since the last time:

• Moved validation logic behind a flag
• Updated README explaining what validation does and how to turn in on
• Added more unit tests
• Reverted ESLint configuration change
• Improved JSDoc

[smhg]

@vkhytskyi-allegro I've left feedback on stream/PoParserTransform but I'd apply the same logic to parse/Parser. If we can eliminate the need for both normalize methods, I think the PR is ready to merge.

[vkhytskyi-allegro]

@smhg Sorry for impatience but I'd like to follow up on last comments I left (https://github.com/smhg/gettext-parser/pull/76#discussion_r1136781289 & https://github.com/smhg/gettext-parser/pull/76#discussion_r1136834716) and ask whether you had a chance to take a look at them? Thanks!

[smhg]

@vkhytskyi-allegro I will find some time during the next few days to get back to you. You are obviously right about these lines containing options magic. I didn't know about those (I've adopted this project). I currently prefer to clean that up (major version bump) to enable the behavior I described earlier. Feel free to move ahead or wait for more feedback (if applicable).

[smhg]

@vkhytskyi-allegro Thank you for your ongoing work! I'd like to simplify things further. Since this will be a major release, it makes no sense to still support defaultCharset directly (outside of options.defaultCharset). Secondly, I think Parser and PoParserTransform should handle their options in exactly the same way. Thinking about future options it will just become a longer and longer parameter list otherwise. Third, the Readme would need to be adapted to reflect the new arguments. Do you still feel like moving on with this or do you prefer I handle these changes?

[vkhytskyi-allegro]

@smhg Thank you for getting back to me. I am open to continue working on the PR till it's ready to be merged. I've just pushed changes that should address your latest comments.

[smhg]

Thank you for your contributions @vkhytskyi-allegro! I'll release this soon.

[KubaZ]

Hey @smhg 👋 Do You plan to release this change in near future?

[smhg]

@KubaZ waiting for another PR, I lost track of this release. I'm sorry. It's now released as 7.0.0.

[KubaZ]

No worries, thanks for quick response and for the release! 🚀