Skip to content

Commit

Permalink
[presubmit] ICU syntax check: allow =2 and =3 cases
Browse files Browse the repository at this point in the history
Manually add =2 and =3 numeric cases to the allowed syntax of the ICU
syntax presubmit check.

While a generic solution that recognizes all possible numeric cases
automatically would also fix this for the =4 case and above, this
solution here is simpler and seem sufficient, given that the vast
majority of strings use up to =3 (at this point there is a single =4
case in the codebase, and none above).

Fixed: 41495103
Change-Id: Ib945c640f94d139d90a0d06e67dff5099b50d788
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5237281
Reviewed-by: Dominic Battre <[email protected]>
Reviewed-by: Nicolas MacBeth <[email protected]>
Reviewed-by: Rainhard Findling <[email protected]>
Commit-Queue: Rainhard Findling <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1256306}
  • Loading branch information
Rainhard Findling authored and Chromium LUCI CQ committed Feb 5, 2024
1 parent 4d91927 commit 3cde3ef
Show file tree
Hide file tree
Showing 2 changed files with 46 additions and 4 deletions.
4 changes: 2 additions & 2 deletions PRESUBMIT.py
Original file line number Diff line number Diff line change
Expand Up @@ -6282,10 +6282,10 @@ def _ValidateIcuSyntax(text, level, signatures):
"""
valid_types = {
'plural': (frozenset(
['=0', '=1', 'zero', 'one', 'two', 'few', 'many',
['=0', '=1', '=2', '=3', 'zero', 'one', 'two', 'few', 'many',
'other']), frozenset(['=1', 'other'])),
'selectordinal': (frozenset(
['=0', '=1', 'zero', 'one', 'two', 'few', 'many',
['=0', '=1', '=2', '=3', 'zero', 'one', 'two', 'few', 'many',
'other']), frozenset(['one', 'other'])),
'select': (frozenset(), frozenset(['other'])),
}
Expand Down
46 changes: 44 additions & 2 deletions PRESUBMIT_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -3389,6 +3389,23 @@ class StringTest(unittest.TestCase):
</release>
</grit>
""".splitlines()
# A grd file with multiple ICU syntax messages without syntax errors.
NEW_GRD_CONTENTS_ICU_SYNTAX_OK3 = """<?xml version="1.0" encoding="UTF-8"?>
<grit latest_public_release="1" current_release="1">
<release seq="1">
<messages>
<message name="IDS_TEST1">
{NUM, plural,
=0 {New test text for numeric zero}
=1 {Different test text for numeric one}
=2 {New test text for numeric two}
=3 {New test text for numeric three}
other {Different test text for plural with {NUM} as number}}
</message>
</messages>
</release>
</grit>
""".splitlines()
# A grd file with one ICU syntax message with syntax errors (misses a comma).
NEW_GRD_CONTENTS_ICU_SYNTAX_ERROR = """<?xml version="1.0" encoding="UTF-8"?>
<grit latest_public_release="1" current_release="1">
Expand Down Expand Up @@ -3481,9 +3498,22 @@ class StringTest(unittest.TestCase):
'other {Different test text for plural with {NUM} as number}}',
'</message>',
'</grit-part>')
# A grdp file with multiple ICU syntax messages without syntax errors.
NEW_GRDP_CONTENTS_ICU_SYNTAX_OK3 = (
'<?xml version="1.0" encoding="utf-8"?>',
'<grit-part>',
'<message name="IDS_PART_TEST1">',
'{NUM, plural,',
'=0 {New test text for numeric zero}',
'=1 {Different test text for numeric one}',
'=2 {New test text for numeric two}',
'=3 {New test text for numeric three}',
'other {Different test text for plural with {NUM} as number}}',
'</message>',
'</grit-part>')

# A grdp file with one ICU syntax message with syntax errors (superfluent
# whitespace).
# A grdp file with one ICU syntax message with syntax errors (superfluous
# space).
NEW_GRDP_CONTENTS_ICU_SYNTAX_ERROR = (
'<?xml version="1.0" encoding="utf-8"?>',
'<grit-part>',
Expand Down Expand Up @@ -3889,6 +3919,18 @@ def testIcuSyntax(self):
if e.message == self.ICU_SYNTAX_ERROR_MESSAGE]
self.assertEqual(0, len(icu_errors))

# Valid changes in ICU syntax. Should not raise an error.
input_api = self.makeInputApi([
MockAffectedFile('test.grd', self.NEW_GRD_CONTENTS_ICU_SYNTAX_OK3,
self.NEW_GRD_CONTENTS_ICU_SYNTAX_OK1, action='M'),
MockAffectedFile('part.grdp', self.NEW_GRDP_CONTENTS_ICU_SYNTAX_OK3,
self.NEW_GRDP_CONTENTS_ICU_SYNTAX_OK1, action='M')])
results = PRESUBMIT.CheckStrings(input_api, MockOutputApi())
# We expect no ICU syntax errors.
icu_errors = [e for e in results
if e.message == self.ICU_SYNTAX_ERROR_MESSAGE]
self.assertEqual(0, len(icu_errors))

# Add invalid ICU syntax strings. Should raise two errors.
input_api = self.makeInputApi([
MockAffectedFile('test.grd', self.NEW_GRD_CONTENTS_ICU_SYNTAX_ERROR,
Expand Down

0 comments on commit 3cde3ef

Please sign in to comment.