OPENNLP-1875: Align whitespace handling with the Unicode White_Space property

[krickert]

UCD-backs the whitespace predicates outside the OPENNLP-1850 engine (OPENNLP-1852 consistency item; generalizes the OPENNLP-205 correction). Every StringUtil.isWhitespace, Character.isWhitespace, Character.isSpaceChar, and \s-split site outside the 1850 engine was audited and classified:

• Migrated to the UCD White_Space set: both rule-based tokenizers, Span.trim, the sentence detector's abbreviation guard, the stopword and spellcheck CLIs, SymSpell.lookupCompound, the spellcheck PER_TOKEN normalizer, and the UIMA NumberUtil.
• Frozen with documented rationale and permanent pins: trained-model feature generation and the BERT reference definition (model stability).
• Kept JVM/ASCII semantics with per-site justification comments: format parsers, where the format fixes the definition.
• StringUtil.isWhitespace itself stays frozen as the documented legacy predicate, since its remaining callers are exactly the frozen sites; user-text code is directed to UnicodeWhitespace.

Method is characterize-then-refactor: the first commit pins pre-change behavior at NBSP, figure and narrow space, U+2028/U+2029, NEL, and U+001C..U+001F for every touched site, so the migration commits flip only the deliberate deltas.

This is a behavior change; the release-note text is in the JIRA issue.

Verification: opennlp-runtime 1313/0, plus green suites in api, formats, cli, tools, uima, and spellcheck.

https://issues.apache.org/jira/browse/OPENNLP-1875

[rzo1]

Hi @krickert - as mentioned on Slack I currently dont have the time for a manual review but I just let Fable do a comprehensive review on this PR. Here is the result:

• opennlp-api/src/main/java/opennlp/tools/tokenize/WhitespaceTokenizer.java: Sample parsers that split training/annotation lines via WhitespaceTokenizer.INSTANCE (NameSample.parse, POSSample, TokenSample, brat/ontonotes/doccat streams) silently inherit the Unicode White_Space migration. Existing corpora containing U+001C..U+001F or U+0085 now produce different samples (and shifted NameSample token indices). These indirect format-parsing sites fall outside the PR's audit of direct call sites and received neither a freeze, a justification comment, nor a characterization pin, unlike the analogous direct sites (ConlluTokenSampleStream, SgmlParser, AD streams) that were frozen for exactly this reason.
• opennlp-core/opennlp-runtime/src/test/java/opennlp/tools/tokenize/WhitespaceTokenizerTest.java: The interaction of keepNewLines=true with U+0085 (newly whitespace in this PR) is untested and undocumented in both WhitespaceTokenizer and SimpleTokenizer. The private isLineSeparator guard matches only \n and \r, so U+0085/U+2028/U+2029 now split tokens but are silently dropped instead of being emitted as newline tokens; every new test sets setKeepNewLines(false), so this asymmetry is neither pinned by a test nor mentioned in the updated javadoc. Users relying on keepNewLines to reconstruct line structure get silent data loss for NEL/LS/PS input.
• opennlp-extensions/opennlp-spellcheck/src/main/java/opennlp/spellcheck/symspell/SymSpell.java: lookupCompound now splits terms on Unicode White_Space but computes the final reported edit distance against input.trim(), which strips only chars <= U+0020. Leading/trailing NBSP/NEL runs inflate the reported distance (an NBSP-only input yields distance 1 instead of 0, unlike an ASCII-space-only input). The trim on the distance path should use the same Unicode set as the split.
• opennlp-extensions/opennlp-spellcheck/src/main/java/opennlp/spellcheck/cmdline/CorrectTextTool.java: In listSuggestions the blank-line guard line.isBlank() still uses JVM whitespace while tokenization uses the Unicode set, so an NBSP-only line skips the newline branch and then tokenizes to zero tokens: it disappears from the output entirely, whereas an ASCII-blank line emits an empty line. Guard with the same Unicode whitespace definition or emit a newline when tokenize() returns empty.
• opennlp-core/opennlp-runtime/src/main/java/opennlp/tools/sentdetect/SentenceDetectorME.java and opennlp-extensions/opennlp-uima/src/main/java/opennlp/uima/normalizer/NumberUtil.java: The whitespace-semantics change is not reflected in the published javadoc of the affected API surface. isAcceptableBreak is protected and designed for override but documents the change only in inline comments; NumberUtil.parse now strips all Unicode White_Space (including NBSP/NNBSP grouping separators) but only the private helper mentions it. Both should get the same "Since 3.0 ... Unicode White_Space" note the other migrated public sites received.
• opennlp-core/opennlp-runtime/src/test/java/opennlp/tools/tokenize/SimpleTokenizerTest.java: The private cp(int codePoint) helper is duplicated across eight test files touched by this PR (four within opennlp-runtime alone). Consider a shared test utility, or simply Character.toString(int), which makes the helper unnecessary.

Human review will follow.

[krickert]

Thanks. Addressed in 9fd57a8, one pushback:

• Indirect sample-parser sites: each now carries the same per-site justification comment as the frozen direct call sites, and characterization tests pin the new behavior where it is deliberate: a NameSample, POSSample, and TokenSample line separated by U+0085 parses identically to its ASCII-space equivalent, token indices and spans included. The brat, OntoNotes, and doccat streams got the comment treatment; their separators are format-fixed ASCII in practice, and the comments say so.
• keepNewLines and NEL/LS/PS: fixed, not just documented. The line separator set under keepNewLines is now the ASCII pair plus U+0085, U+2028, and U+2029 in both WhitespaceTokenizer and SimpleTokenizer, so those code points are emitted as newline tokens instead of silently dropped; the SimpleTokenizer path also anchors the emitted span at the separator's own position. Javadoc states the emitted set, tests pin it with keepNewLines enabled.
• SymSpell compound distance: the distance path now trims with the same Unicode White_Space set the split uses; an NBSP-only input reports distance 0 like an ASCII-space-only input, tested.
• CorrectTextTool blank guard: a whitespace-only line now emits an empty output line instead of disappearing; guarded with the same Unicode definition as tokenization, tested through the command line tool.
• SentenceDetectorME.isAcceptableBreak and NumberUtil.parse: both carry the since-3.0 Unicode White_Space javadoc note the other migrated public sites received.
• cp(int) duplication: kept as-is. The three-line helper names the intent at call sites full of hex literals, and consolidating across modules would need a shared test artifact for a helper that small; the duplication predates this PR across the suite and is better swept in one test-wide cleanup than piecemeal here.

[rzo1]

A) Please reduce the inline commentary.

Nearly every touched call site carries a multi-line narrative comment ("Sample parsing deliberately shares…", "Resource-format parsing: …", "Format parsing: the training data is…", the restated whitespace-delta notes in SentenceDetectorME, SymSpell, NumberUtil, etc.). These explain the migration decision, not the code.

IMHO that rationale belongs in the JIRA issue / this PR description / the release notes, where it's already captured. In the code it's noise that will go stale and doesn't help the next reader. (This can be taken verbatim to all other open PRs as well)

Concretely:

• Drop the per-call-site justification comments entirely (format parsers, sample parsers, CLI tools, isLineSeparator, the repeated "since 3.0…" delta enumerations at call sites).
• Keep, but condense to 1–3 lines, the notes that document a real invariant or user-visible behavior change: the frozen-predicate rationale on StringUtil.isWhitespace and the two context generators, the BERT-reference pin in BertNormalization, and a short "whitespace is the Unicode White_Space set since 3.0" note in the javadoc of WhitespaceTokenizer, SimpleTokenizer and Span#trim. One sentence plus a pointer to OPENNLP-1875 is enough; the full U+xxxx enumerations don't need to live in javadoc.
• The comment explaining the SimpleTokenizer span-position fix in keepNewLines mode can stay (it explains a non-obvious bug fix), but one line suffices.

B) Please expose the new predicate via StringUtil and use that at the migrated call sites.

I'd like to avoid splitting the whitespace predicates across two entry points: people reach for StringUtil by habit, and having the legacy and the standard predicate side by side (with distinct names) makes the difference discoverable. UnicodeWhitespace should stay as the implementation/reference-data home; just add a delegating facade:

/**
 * Unicode {@code White_Space} membership; delegates to
 * {@link opennlp.tools.util.normalizer.UnicodeWhitespace#isWhitespace(int)}.
 */
public static boolean isUnicodeWhitespace(int codePoint) {
  return UnicodeWhitespace.isWhitespace(code
}          

(plus a char overload to mirror the existing pair), and switch the migrated call sites (WhitespaceTokenizer, SimpleTokenizer, Span#trim, spellcheck, UIMAdeWhitespace.isWhitespace(…) toStringUtil.isUnicodeWhitespace(…). The CharClass.whitespace() usage in SentenceDetectorME is fine as is.

C) I think the issue and PR would benefit from a clearer title / heading

The PR title (and the issue title) is hard to understand; something like

• Align whitespace handling with the Unicode White_Space property
• Migrate whitespace checks on user text to Unicode White_Space

would be better understandable and cleaner in release notes.