From 03de7aa5b1ed3094cdbaea685826ab8ca06271d1 Mon Sep 17 00:00:00 2001 From: Elliott Hughes Date: Thu, 22 Apr 2010 13:38:42 -0700 Subject: [PATCH] java.text.RuleBasedCollator fixes. Add expectations for broken harmony tests, add our own equivalent (but correct) tets, and fix the bug turned up by the correct tests: the icu4jni RuleBasedCollator was using toString to convert a CharacterIterator to a String, resulting in iteration over the result of Object.toString (the class name and identity hash code) rather than the characters of interest. Also shut javac up about non-ASCII characters in Locale.java. Bug: 2608742 Bug: 2608750 Change-Id: I2171789058c8116eacd7e5815bd483f0bc07c69b --- .../com/ibm/icu4jni/text/RuleBasedCollator.java | 13 ++++- libcore/luni/src/main/java/java/util/Locale.java | 4 +- .../luni/src/test/java/java/text/CollatorTest.java | 61 ++++++++++++++++++++++ .../src/main/java/java/text/RuleBasedCollator.java | 11 ++-- libcore/tools/runner/expectations/brokentests.txt | 17 ++++++ 5 files changed, 97 insertions(+), 9 deletions(-) diff --git a/libcore/icu/src/main/java/com/ibm/icu4jni/text/RuleBasedCollator.java b/libcore/icu/src/main/java/com/ibm/icu4jni/text/RuleBasedCollator.java index a87c97840..3135daa97 100644 --- a/libcore/icu/src/main/java/com/ibm/icu4jni/text/RuleBasedCollator.java +++ b/libcore/icu/src/main/java/com/ibm/icu4jni/text/RuleBasedCollator.java @@ -516,8 +516,17 @@ public final class RuleBasedCollator extends Collator { return result; } - public CollationElementIterator getCollationElementIterator(CharacterIterator source) { - return getCollationElementIterator(source.toString()); + public CollationElementIterator getCollationElementIterator(CharacterIterator it) { + // We only implement the String-based API, so build a string from the iterator. + return getCollationElementIterator(characterIteratorToString(it)); + } + + private String characterIteratorToString(CharacterIterator it) { + StringBuilder result = new StringBuilder(); + for (char ch = it.current(); ch != CharacterIterator.DONE; ch = it.next()) { + result.append(ch); + } + return result.toString(); } /** diff --git a/libcore/luni/src/main/java/java/util/Locale.java b/libcore/luni/src/main/java/java/util/Locale.java index 7d03eac6b..c6535c1a1 100644 --- a/libcore/luni/src/main/java/java/util/Locale.java +++ b/libcore/luni/src/main/java/java/util/Locale.java @@ -392,8 +392,8 @@ public final class Locale implements Cloneable, Serializable { * to {@code locale}. The exact output form depends on whether this locale * corresponds to a specific language, country and variant, such as: * {@code English}, {@code English (United States)}, {@code English (United - * States,Computer)}, {@code anglais (États-Unis)}, {@code anglais - * (États-Unis,informatique)}. + * States,Computer)}, {@code anglais (États-Unis)}, {@code anglais + * (États-Unis,informatique)}. */ public String getDisplayName(Locale locale) { int count = 0; diff --git a/libcore/luni/src/test/java/java/text/CollatorTest.java b/libcore/luni/src/test/java/java/text/CollatorTest.java index 48c0eb1f5..5cded0a37 100644 --- a/libcore/luni/src/test/java/java/text/CollatorTest.java +++ b/libcore/luni/src/test/java/java/text/CollatorTest.java @@ -82,4 +82,65 @@ public class CollatorTest extends junit.framework.TestCase { assertTrue("Error: \u00e0\u0325 should equal to a\u0325\u0300 with decomposition", myCollator.compare("\u00e0\u0325", "a\u0325\u0300") == 0); } + + public void testEqualsObject() throws ParseException { + String rule = "< a < b < c < d < e"; + RuleBasedCollator coll = new RuleBasedCollator(rule); + + assertEquals(Collator.TERTIARY, coll.getStrength()); + // This is a harmony test, but it assumes that RuleBasedCollators default to + // NO_DECOMPOSITION, which isn't true on Android. + // assertEquals(Collator.NO_DECOMPOSITION, coll.getDecomposition()); + RuleBasedCollator other = new RuleBasedCollator(rule); + assertTrue(coll.equals(other)); + + coll.setStrength(Collator.PRIMARY); + assertFalse(coll.equals(other)); + + coll.setStrength(Collator.TERTIARY); + coll.setDecomposition(Collator.CANONICAL_DECOMPOSITION); + other.setDecomposition(Collator.NO_DECOMPOSITION); // See comment above. + assertFalse(coll.equals(other)); + } + + public void test_Harmony_1352() throws Exception { + // Regression test for HARMONY-1352, that doesn't get run in the harmony test suite because + // of an earlier failure. + try { + new RuleBasedCollator("< a< b< c< d").getCollationElementIterator((CharacterIterator) null); + fail("NullPointerException expected"); + } catch (NullPointerException expected) { + } + } + + private void assertCollationElementIterator(CollationElementIterator it, Integer... offsets) { + for (int offset : offsets) { + assertEquals(offset, it.getOffset()); + it.next(); + } + assertEquals(CollationElementIterator.NULLORDER, it.next()); + } + + private void assertGetCollationElementIteratorString(Locale l, String s, Integer... offsets) { + RuleBasedCollator coll = (RuleBasedCollator) Collator.getInstance(l); + assertCollationElementIterator(coll.getCollationElementIterator(s), offsets); + } + + private void assertGetCollationElementIteratorCharacterIterator(Locale l, String s, Integer... offsets) { + RuleBasedCollator coll = (RuleBasedCollator) Collator.getInstance(l); + CharacterIterator it = new StringCharacterIterator(s); + assertCollationElementIterator(coll.getCollationElementIterator(it), offsets); + } + + public void testGetCollationElementIteratorString() throws Exception { + assertGetCollationElementIteratorString(new Locale("es", "", "TRADITIONAL"), "cha", 0, 2, 3); + assertGetCollationElementIteratorString(new Locale("es", "", ""), "cha", 0, 1, 2, 3); + assertGetCollationElementIteratorString(new Locale("de", "DE", ""), "\u00e6b", 0, 1, 1, 2); + } + + public void testGetCollationElementIteratorCharacterIterator() throws Exception { + assertGetCollationElementIteratorCharacterIterator(new Locale("es", "", "TRADITIONAL"), "cha", 0, 2, 3); + assertGetCollationElementIteratorCharacterIterator(new Locale("es", "", ""), "cha", 0, 1, 2, 3); + assertGetCollationElementIteratorCharacterIterator(new Locale("de", "DE", ""), "\u00e6b", 0, 1, 1, 2); + } } diff --git a/libcore/text/src/main/java/java/text/RuleBasedCollator.java b/libcore/text/src/main/java/java/text/RuleBasedCollator.java index 77492b5d4..0955f8948 100644 --- a/libcore/text/src/main/java/java/text/RuleBasedCollator.java +++ b/libcore/text/src/main/java/java/text/RuleBasedCollator.java @@ -269,15 +269,14 @@ public class RuleBasedCollator extends Collator { * the result of a former {@link #getRules()} call. *

* Note that the {@code rules} are actually interpreted as a delta to the - * standard Unicode Collation Algorithm (UCA). Hence, an empty {@code rules} - * string results in the default UCA rules being applied. This differs + * standard Unicode Collation Algorithm (UCA). This differs * slightly from other implementations which work with full {@code rules} * specifications and may result in different behavior. * * @param rules * the collation rules. * @throws NullPointerException - * if {@code rules} is {@code null}. + * if {@code rules == null}. * @throws ParseException * if {@code rules} contains rules with invalid collation rule * syntax. @@ -286,6 +285,9 @@ public class RuleBasedCollator extends Collator { if (rules == null) { throw new NullPointerException(); } + if (rules.isEmpty()) { + throw new ParseException("empty rules", 0); + } try { this.icuColl = new com.ibm.icu4jni.text.RuleBasedCollator(rules); this.icuColl.setDecomposition(com.ibm.icu4jni.text.Collator.CANONICAL_DECOMPOSITION); @@ -310,8 +312,7 @@ public class RuleBasedCollator extends Collator { * the source character iterator. * @return a {@code CollationElementIterator} for {@code source}. */ - public CollationElementIterator getCollationElementIterator( - CharacterIterator source) { + public CollationElementIterator getCollationElementIterator(CharacterIterator source) { if (source == null) { throw new NullPointerException(); } diff --git a/libcore/tools/runner/expectations/brokentests.txt b/libcore/tools/runner/expectations/brokentests.txt index 2d98c23ec..75624ee8e 100644 --- a/libcore/tools/runner/expectations/brokentests.txt +++ b/libcore/tools/runner/expectations/brokentests.txt @@ -711,3 +711,20 @@ result EXEC_FAILED pattern .*GMT-07:00.* +# These harmony tests are broken. The RI doesn't ship with es__TRADITIONAL, so +# they have incorrect expectations. +# http://b/2608750 + +test org.apache.harmony.text.tests.java.text.RuleBasedCollatorTest#testGetCollationElementIteratorCharacterIterator +result EXEC_FAILED +pattern .*expected:<1> but was:<2>.* + +test org.apache.harmony.text.tests.java.text.RuleBasedCollatorTest#testGetCollationElementIteratorString +result EXEC_FAILED +pattern .*expected:<1> but was:<2>.* + +# This test fails because on Android, RuleBasedCollators default to +# CANONICAL_DECOMPOSITION, not NO_DECOMPOSITION. +test org.apache.harmony.text.tests.java.text.RuleBasedCollatorTest#testEqualsObject +result EXEC_FAILED +pattern .*expected:<0> but was:<1>.* -- 2.11.0