-
Notifications
You must be signed in to change notification settings - Fork 73
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Ad hoc relaxation of the integrity checker for plural strings
Only the CLDR 'other' form must have the exact same number of placeholders. For other forms, we allow one placeholder to be removed. Note that placeholders are stored in a set, so duplicate placeholders will count as one. This only target printflike check, so mainly targeting Android, and some gettext. This is pretty adhoc anyway
- Loading branch information
Showing
4 changed files
with
158 additions
and
22 deletions.
There are no files selected for viewing
41 changes: 41 additions & 0 deletions
41
.../mojito/service/assetintegritychecker/integritychecker/PluralIntegrityCheckerRelaxer.java
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,41 @@ | ||
| package com.box.l10n.mojito.service.assetintegritychecker.integritychecker; | ||
|
|
||
| import java.util.Set; | ||
| import org.springframework.stereotype.Component; | ||
|
|
||
| @Component | ||
| public class PluralIntegrityCheckerRelaxer { | ||
|
|
||
| /** | ||
| * This is very ad hoc! | ||
| * | ||
| * <p>There are cases where the plural form doesn’t retain the number. Right now, those are | ||
| * wrongly marked as rejected. Ideally, we should clearly identify which placeholder might be | ||
| * missing from the string, but that’s another level of complexity. Also, use the class name to | ||
| * target certain integrity checks to keep this hack constrained. | ||
| */ | ||
| public boolean shouldRelaxIntegrityCheck( | ||
| String source, String target, String pluralForm, TextUnitIntegrityChecker textUnitChecker) { | ||
|
|
||
| boolean shouldRelax = false; | ||
|
|
||
| if (pluralForm != null && !"other".equals(pluralForm)) { | ||
| if (textUnitChecker instanceof PrintfLikeIntegrityChecker | ||
| || textUnitChecker instanceof PrintfLikeIgnorePercentageAfterBracketsIntegrityChecker | ||
| || textUnitChecker instanceof PrintfLikeVariableTypeIntegrityChecker | ||
| || textUnitChecker instanceof SimplePrintfLikeIntegrityChecker) { | ||
|
|
||
| RegexIntegrityChecker regexIntegrityChecker = (RegexIntegrityChecker) textUnitChecker; | ||
|
|
||
| Set<String> sourcePlaceholders = regexIntegrityChecker.getPlaceholders(source); | ||
| Set<String> targetPlaceholders = regexIntegrityChecker.getPlaceholders(target); | ||
|
|
||
| if (sourcePlaceholders.size() - targetPlaceholders.size() <= 1) { | ||
| shouldRelax = true; | ||
| } | ||
| } | ||
| } | ||
|
|
||
| return shouldRelax; | ||
| } | ||
| } |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
64 changes: 64 additions & 0 deletions
64
...ito/service/assetintegritychecker/integritychecker/PluralIntegrityCheckerRelaxerTest.java
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,64 @@ | ||
| package com.box.l10n.mojito.service.assetintegritychecker.integritychecker; | ||
|
|
||
| import static org.junit.jupiter.api.Assertions.assertFalse; | ||
| import static org.junit.jupiter.api.Assertions.assertTrue; | ||
|
|
||
| import org.junit.Test; | ||
|
|
||
| public class PluralIntegrityCheckerRelaxerTest { | ||
|
|
||
| @Test | ||
| public void testShouldRelaxIntegrityCheck_whenPluralFormIsOther() { | ||
| PluralIntegrityCheckerRelaxer relaxer = new PluralIntegrityCheckerRelaxer(); | ||
| boolean result = | ||
| relaxer.shouldRelaxIntegrityCheck( | ||
| "source %d", "target", "other", new PrintfLikeIntegrityChecker()); | ||
|
|
||
| assertFalse(result, "Integrity check should not be relaxed for plural form 'other'."); | ||
| } | ||
|
|
||
| @Test | ||
| public void testShouldRelaxIntegrityCheck_whenPlaceholdersDifferByOne_One() { | ||
| PluralIntegrityCheckerRelaxer relaxer = new PluralIntegrityCheckerRelaxer(); | ||
| boolean result = | ||
| relaxer.shouldRelaxIntegrityCheck( | ||
| "source %d", "target", "one", new PrintfLikeIntegrityChecker()); | ||
|
|
||
| assertTrue( | ||
| result, "Integrity check should be relaxed when placeholders differ by one for 'one' form"); | ||
| } | ||
|
|
||
| @Test | ||
| public void testShouldRelaxIntegrityCheck_whenPlaceholdersDifferByOne_One_DuplicatePlaceholder() { | ||
| PluralIntegrityCheckerRelaxer relaxer = new PluralIntegrityCheckerRelaxer(); | ||
| boolean result = | ||
| relaxer.shouldRelaxIntegrityCheck( | ||
| "source %d %d", "target", "one", new PrintfLikeIntegrityChecker()); | ||
|
|
||
| assertTrue( | ||
| result, | ||
| "Integrity check should be relaxed when placeholders differ by one for 'one' form (as placeholders are stored in a set, ignoring duplicates)"); | ||
| } | ||
|
|
||
| @Test | ||
| public void testShouldRelaxIntegrityCheck_whenPlaceholdersDifferByTwo_One() { | ||
| PluralIntegrityCheckerRelaxer relaxer = new PluralIntegrityCheckerRelaxer(); | ||
| boolean result = | ||
| relaxer.shouldRelaxIntegrityCheck( | ||
| "source %1d %2d", "target", "one", new PrintfLikeIntegrityChecker()); | ||
|
|
||
| assertFalse( | ||
| result, | ||
| "Integrity check should not be relaxed when placeholders differ by two for 'one' form"); | ||
| } | ||
|
|
||
| @Test | ||
| public void testShouldRelaxIntegrityCheck_withUnsupportedChecker() { | ||
| PluralIntegrityCheckerRelaxer relaxer = new PluralIntegrityCheckerRelaxer(); | ||
|
|
||
| boolean result = | ||
| relaxer.shouldRelaxIntegrityCheck("source %d", "target", "one", new URLIntegrityChecker()); | ||
|
|
||
| assertFalse(result, "Integrity check should not be relaxed for unsupported checker types."); | ||
| } | ||
| } |