OPENNLP-1955: Replace literal String.split calls with a regex-free StringUtil.split - #1302
Conversation
…tics
Add split(CharSequence, char) and split(CharSequence, char, int) to
opennlp.tools.util.StringUtil, byte-identical to String.split with a
literal single-character pattern: leading and middle empty fields kept,
trailing empty fields removed at limit 0 and kept at every other limit,
empty input yielding a one-element array containing the empty string,
and the String.split limit contract for positive, zero, and negative
limits. The scan compares characters, so metacharacter separators are
treated as literals and supplementary code points pass through
untouched. Null input throws IllegalArgumentException, matching
splitOnUnicodeWhitespace.
Test-first: StringUtilSplitTest does not compile against main without
the new methods (red: cannot find symbol: method split) and passes with
them (green: 41 tests).
Fuzz evidence, default JDK, seed 1955, scratch dir outside the repo:
200,000 random inputs over both overloads, 400,000 comparisons against
String.split, separators {comma, space, tab, newline, dash, underscore,
semicolon, colon, hash, dollar, dot, pipe} where hash, dollar, and pipe
are regex metacharacters proving literal treatment, strings of length
0-40 with empty, leading, trailing, and repeated separators plus
supplementary code points, limits {0, 1, 2, 3, 5, 7, -1}:
totalComparisons=400000, mismatches=0.
…il.split Replace the literal-separator String.split call in the allowed-prefix parser with StringUtil.split(prop, ','). Byte-identical: the property is a comma-separated list, the parser trims and blanks-filters every field, so the limit-0 trailing-empty behavior of String.split is preserved exactly. Part of removing the remaining literal String.split calls from production code (OPENNLP-1926 removed the java.util.regex imports but missed the calls that never import the package).
…plit
Replace the literal-separator String.split calls in the CLI tools with
the regex-free StringUtil.split, which is byte-identical for a single
character separator: the doccat and namefind tools split their comma
separated name type and feature generator parameters, and
NGramLanguageModelTool splits input lines on a single space. The six
first-field category extractions in FineGrainedReportListener (two
GroupedMatrixLabelComparator sites and four GroupedLabelComparator
sites, each guarded by contains("-")) become an indexOf and substring
walk, which returns the text before the first dash with no array
allocation, exactly what split("-")[0] produced.
…ngUtil.split Replace the literal-separator String.split calls in the corpus format parsers with the regex-free StringUtil.split, byte-identical for a single character separator, preserving the limit-0 trailing-empty behavior the tab- and space-separated formats rely on. Covered: the Conll, Evalita, GermEval, BioNLP2004, and ConllXP name and POS sample streams (line.split on tab or space), the brat annotation and name sample streams (semicolon, colon, and comma separated values), the AD contraction lookup (underscore), the CoNLL-U word line (tab), the MASC sentence anchors (space), the NKJP corresp pointer list (comma), and the OntoNotes document-to-line stream (newline). Each replacement keeps the exact receiver, separator, and limit the parser used before.
….split Replace the literal-separator String.split calls with the regex-free StringUtil.split, byte-identical for a single character separator and keeping the limit-0 trailing-empty behavior. Covered: the chunker and lemmatizer sample streams and the lemmatizer dictionary (tab, then hash for multiple lemmas), the doccat factory feature generator list (comma), the POS dictionary tag list (space), the word cluster dictionary (space), the Glove vector loader (space), and the emoji annotation, emoji emoticon, and full case fold table loaders. The three table loaders split on their single-character constants; the constants become char so the bounded splits keep their exact limits (5, 6, and the default) with no string pattern left in the class. Pinning tests where coverage of the replaced parsing was thin: DictionaryLemmatizerTest gains a multi-lemma entry check and POSDictionaryTest gains a multi-tag dictionary entry check.
…tringUtil.split Replace the literal-separator String.split calls that parse the name type mapping parameter (comma-separated entries, colon-separated name type pairs) with the regex-free StringUtil.split, byte-identical for a single character separator. A malformed entry still logs the same warning and is skipped.
|
Open item: merge order. Only |
|
Open item: four production files outside the ticket's list still call |
|
Open item: MASC consistency. This PR leaves |
mawiesne
left a comment
There was a problem hiding this comment.
Thx for the PR - it's clean and reads fine.
Based on main. Goes first in the de-regex series: the other open PRs do not depend on it today, but #1279 and #1282 build on the utility it adds (see the comments below).
String.splitwith a one-character pattern compiles no regular expression on the JDK fast path, so this is a policy and consistency change, not a performance one. The epic removed thejava.util.regeximports, and the audit keyed on the import missed the literal-separatorString.splitcalls, which do not import the package.The utility
StringUtil.split(CharSequence, char)andStringUtil.split(CharSequence, char, int limit)in opennlp-api give the result ofString.splitwith the same one-character pattern: leading and middle empty fields kept, trailing empty fields removed at limit 0 and kept at any other limit, empty input gives one empty field, and theString.splitlimit contract for positive, zero and negative limits. The scan compares chars, so a metacharacter such as.,$or|is a literal. Null input and a surrogate separator throwIllegalArgumentException; a surrogate separator is rejected because the char scan would split a supplementary code point between its two chars, while the JDK pattern matches code points.The call sites
The 33 files listed in the ticket, in five commits by module:
ExtensionLoader(opennlp-api), eight cmdline tools, 13 corpus format streams, ten runtime parsers and loaders, and the UIMA name finder mapping. Each site keeps its receiver, separator and limit; the emoji and case fold table loaders keep their bounded splits at 5 and 6 with the separator constants aschar. The six first-field extractions inFineGrainedReportListenerare anindexOfandsubstringinstead of a split and an array.No file outside the 33 is touched.
matches,replaceAllandreplaceFirstdo not occur in them.Tests
StringUtilSplitTest, 46 tests: pinned cases for both overloads (empty input, one separator, leading, trailing and repeated separators, metacharacter separators, supplementary code points, each limit kind), a check that the two-argument overload equals limit 0, anyCharSequenceas input, null, and the surrogate rule. A seeded differential test compares both overloads withString.spliton the quoted separator over 20,000 random inputs built from 12 separators, ASCII and supplementary code points, and the limits 0, 1, 2, 3, 5, 7 and -1. The test class does not compile without the new methods, which is the red side.DictionaryLemmatizerTestandPOSDictionaryTesthave a new multi-lemma and a multi-tag entry case where the replaced parsing had no direct coverage. Both pass before and after the change, as expected for a refactor.Fuzz run outside the repository during authoring: 400,000 comparisons with
String.splitover both overloads, 0 mismatches.opennlp-api 426, opennlp-runtime 2682, opennlp-formats 357, opennlp-cli 38 and opennlp-uima 52 tests with checkstyle and forbiddenapis,
-Dopennlp.forkCount=1.Manual
No change: the utility is internal to the parsers and the corpus formats read the same.
Before merge
OPENNLP-1955