Bug 550471 - Improve and deprecate TextUtilities.indexOf

Existing TextUtilities.indexOf got a quite ugly return value and some
strange behaviour in details (e.g. how it match empty strings). It is
also superseded by the new MultiStringMatcher.
The implementation is slow (especially for larger inputs) but must be
retained at least for some years. This change provides a faster and
fully compatible reimplementation based on MultiStringMatcher.

Change-Id: I54c471ddd29a6aeb111151116e392dc8b8f8033a
Signed-off-by: Paul Pazderski <paul-eclipse@ppazderski.de>

1 files changed, 38 insertions, 37 deletions

diff --git a/org.eclipse.text/src/org/eclipse/jface/text/TextUtilities.java b/org.eclipse.text/src/org/eclipse/jface/text/TextUtilities.java
index f778e7a9546..f3373ee711a 100644
--- a/org.eclipse.text/src/org/eclipse/jface/text/TextUtilities.java
+++ b/org.eclipse.text/src/org/eclipse/jface/text/TextUtilities.java
@@ -1,5 +1,5 @@
 /*******************************************************************************
- * Copyright (c) 2000, 2016 IBM Corporation and others.
+ * Copyright (c) 2000, 2019 IBM Corporation and others.
  *
  * This program and the accompanying materials
  * are made available under the terms of the Eclipse Public License 2.0
@@ -21,6 +21,7 @@ import java.util.List;
 import java.util.ListIterator;
 import java.util.Map;
 import java.util.Map.Entry;
+import java.util.Objects;
 import java.util.Set;
 
 import org.eclipse.core.runtime.Assert;
@@ -69,52 +70,51 @@ public class TextUtilities {
 	}
 
 	/**
-	 * Returns the starting position and the index of the first matching search string
-	 * in the given text that is greater than the given offset. If more than one search
-	 * string matches with the same starting position then the longest one is returned.
+	 * Returns the starting position and the index of the first matching search string in the given
+	 * text that is greater than the given offset. If more than one search string matches with the
+	 * same starting position then the longest one is returned.
 	 *
 	 * @param searchStrings the strings to search for
 	 * @param text the text to be searched
 	 * @param offset the offset at which to start the search
-	 * @return an <code>int[]</code> with two elements where the first is the starting offset, the second the index of the found
-	 * 		search string in the given <code>searchStrings</code> array, returns <code>[-1, -1]</code> if no match exists
+	 * @return an <code>int[]</code> with two elements where the first is the starting offset, the
+	 *         second the index of the found search string in the given <code>searchStrings</code>
+	 *         array, returns <code>[-1, -1]</code> if no match exists
+	 * @deprecated use {@link MultiStringMatcher#indexOf(CharSequence, int, String...)} instead.
+	 *             Notable differences:
+	 *             <ul>
+	 *             <li>new indexOf will tolerate <code>null</code> and empty search strings (old
+	 *             accepted empty but throw on <code>null</code>)</li>
+	 *             <li>new indexOf will <b>not</b> match empty string (old matched empty if nothing
+	 *             else matched)</li>
+	 *             </ul>
 	 */
+	@Deprecated
 	public static int[] indexOf(String[] searchStrings, String text, int offset) {
-
-		int[] result= { -1, -1 };
-		int zeroIndex= -1;
-
-		for (int i= 0; i < searchStrings.length; i++) {
-
-			int length= searchStrings[i].length();
-
-			if (length == 0) {
-				zeroIndex= i;
-				continue;
+		// For compatibility this will throw a NullPointerException like the old implementation
+		// (instead of an IllegalArgumentException what would be the result from MultiStringMatcher.indexOf)
+		// and mimic the strange result for empty search string match from the old method.
+		Objects.requireNonNull(searchStrings);
+		for (String searchString : searchStrings) {
+			Objects.requireNonNull(searchString);
+		}
+		final MultiStringMatcher.Match match= MultiStringMatcher.indexOf(text, offset, searchStrings);
+		if (match != null) {
+			for (int i= 0; i < searchStrings.length; i++) {
+				if (match.getText().equals(searchStrings[i])) {
+					return new int[] { match.getOffset(), i };
+				}
 			}
-
-			int index= text.indexOf(searchStrings[i], offset);
-			if (index >= 0) {
-
-				if (result[0] == -1) {
-					result[0]= index;
-					result[1]= i;
-				} else if (index < result[0]) {
-					result[0]= index;
-					result[1]= i;
-				} else if (index == result[0] && length > searchStrings[result[1]].length()) {
-					result[0]= index;
-					result[1]= i;
+		} else {
+			// no match must check for empty search strings and mimic old return value
+			// search reversed because we want the last empty search string
+			for (int i= searchStrings.length - 1; i >= 0; i--) {
+				if (searchStrings[i].length() == 0) {
+					return new int[] { 0, i };
 				}
 			}
 		}
-
-		if (zeroIndex > -1 && result[0] == -1) {
-			result[0]= 0;
-			result[1]= zeroIndex;
-		}
-
-		return result;
+		return new int[] { -1, -1 };
 	}
 
 	/**
@@ -495,6 +495,7 @@ public class TextUtilities {
 		try {
 			lineDelimiter= document.getLineDelimiter(0);
 		} catch (BadLocationException x) {
+			// usually impossible for the first line
 		}
 
 		if (lineDelimiter != null)