ICU-22987 Return DONE for BreakIterator.preceding on negative index

See #3507

Author: echeran

icu4c/source/test/intltest/rbbitst.cpp

@@ -107,6 +107,7 @@ void RBBITest::runIndexedTest( int32_t index, UBool exec, const char* &name, cha
     TESTCASE_AUTO(TestGetDisplayName);
 #if !UCONFIG_NO_FILE_IO
     TESTCASE_AUTO(TestEndBehaviour);
+    TESTCASE_AUTO(TestPreceding_NegativeIndex);
     TESTCASE_AUTO(TestWordBreaks);
     TESTCASE_AUTO(TestWordBoundary);
     TESTCASE_AUTO(TestLineBreaks);
@@ -740,6 +741,34 @@ void RBBITest::executeTest(TestParams *t, UErrorCode &status) {
     }
 }
 
+void RBBITest::TestPreceding_NegativeIndex() {
+    UErrorCode status = U_ZERO_ERROR;
+    Locale loc = Locale::getRoot();
+    LocalPointer<BreakIterator> bi(BreakIterator::createWordInstance(loc, status));
+    UnicodeString source(u"The quick brown fox jumped over the lazy dog.");
+    bi->setText(source);
+
+    assertEquals("length of source string", 45, source.length());
+
+    struct TestCase {
+        const UnicodeString description;
+        const int32_t startIdx;
+        const int32_t expected;
+    } cases[] = {
+        {"negative index",                 -2, BreakIterator::DONE},
+        {"zero",                            0, BreakIterator::DONE},
+        {"one",                             1, 0},
+        {"middle",                         41, 40},
+        {"end",                            45, 44},
+        {"after the end", source.length() + 2, source.length()}
+    };
+
+    for (const auto& cas : cases) {
+        int32_t actual = bi->preceding(cas.startIdx);
+        assertEquals(cas.description, cas.expected, actual);
+    }
+}
+
 void RBBITest::TestExtended() {
      // The expectations in this test heavily depends on the Thai dictionary.
      // Therefore, we skip this test under the LSTM configuration.

icu4c/source/test/intltest/rbbitst.h

@@ -58,6 +58,7 @@ class RBBITest: public IntlTest {
 
     void TestExtended();
     void executeTest(TestParams *, UErrorCode &status);
+    void TestPreceding_NegativeIndex();
 
     void TestWordBreaks();
     void TestWordBoundary();

icu4j/main/core/src/main/java/com/ibm/icu/text/BreakIterator.java

@@ -347,6 +347,10 @@ public int preceding(int offset) {
         // NOTE:  This implementation is here solely because we can't add new
         // abstract methods to an existing class.  There is almost ALWAYS a
         // better, faster way to do this.
+
+        if (offset < 0) {
+            return DONE;
+        }
         int pos = following(offset);
         while (pos >= offset && pos != DONE)
             pos = previous();

icu4j/main/core/src/main/java/com/ibm/icu/text/RuleBasedBreakIterator.java

@@ -511,7 +511,7 @@ public int preceding(int offset) {
         if (fText == null || offset > fText.getEndIndex()) {
             return last();
         } else if (offset < fText.getBeginIndex()) {
-            return first();
+            return DONE;
         }
 
         // Move requested offset to a code point start. It might be between a lead and trail surrogate.

icu4j/main/core/src/test/java/com/ibm/icu/dev/test/rbbi/BreakIteratorTest.java

@@ -8,6 +8,9 @@
  */
 package com.ibm.icu.dev.test.rbbi;
 
+import static org.hamcrest.CoreMatchers.is;
+import static org.junit.Assert.assertThat;
+
 import java.text.StringCharacterIterator;
 import java.util.Locale;
 
@@ -107,6 +110,33 @@ public void debugLogln(String s) {
     // tests
     //=========================================================================
 
+    @Test
+    public void TestPreceding_NegativeIndex() {
+        String source = "The quick brown fox jumped over the lazy dog.";
+        BreakIterator iter = BreakIterator.getWordInstance();
+        iter.setText(source);
+
+        assertEquals("length of source string", 45, source.length());
+
+        Object[][] casesData = {
+            {"negative index",                 -2, BreakIterator.DONE},
+            {"zero",                            0, BreakIterator.DONE},
+            {"one",                             1, 0},
+            {"middle",                         41, 40},
+            {"end",                            45, 44},
+            {"after the end", source.length() + 2, source.length()}
+        };
+
+        for (Object[] caseDatum : casesData) {
+            String desc = (String) caseDatum[0];
+            int startIdx = (int) caseDatum[1];
+            int expected = (int) caseDatum[2];
+
+            int actual = iter.preceding(startIdx);
+
+            assertThat(desc, actual, is(expected));
+        }
+    }
 
     /*
      * @bug 4153072

icu4j/main/core/src/test/java/com/ibm/icu/dev/test/rbbi/RBBITest.java

@@ -324,14 +324,14 @@ public void TestPreceding() {
         rbbi.setText((CharacterIterator)null);
         if (rbbi.preceding(-1) != BreakIterator.DONE) {
             errln("RuleBasedBreakIterator.preceding(-1) was suppose to return "
-                    + "0 when the object has a fText of null.");
+                    + "DONE when the object has a fText of null.");
         }
 
         // Tests when "else if (offset < fText.getBeginIndex())" is true
         rbbi.setText("dummy");
-        if (rbbi.preceding(-1) != 0) {
+        if (rbbi.preceding(-1) != BreakIterator.DONE) {
             errln("RuleBasedBreakIterator.preceding(-1) was suppose to return "
-                    + "0 when the object has a fText of dummy.");
+                    + "DONE when the object has a fText of dummy.");
         }
     }