[
https://issues.apache.org/jira/browse/OPENNLP-1926?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Kristian Rickert updated OPENNLP-1926:
--------------------------------------
Description:
OpenNLP's processing path is RegEx-free by policy; a RegEx step that is the
class's purpose (for example {{{}RegexNameFinder{}}}) is the documented
exception (OPENNLP-1925). An audit of main found the production classes below
touching {{{}java.util.regex{}}}, and the work turned up a second group:
{{{}String.split{}}}, {{{}matches{}}}, {{{}replaceAll{}}}, and {{replaceFirst}}
calls that compile a Pattern on each call without an import. This epic replaces
each incidental usage with cursor or code-point loops and ends with a
checkstyle guard so it stays that way. Classes whose job IS pattern matching
are exempt and listed for completeness.
h3. Tickets, in merge order
Each branch is stacked on the one before it, so the pull requests merge in this
order and OPENNLP-1935 merges last.
||Ticket||Scope||PR||
|OPENNLP-1928|part 1: trivial batch, plus the shared {{StringUtil}} helpers
{{{}isAsciiWhitespace{}}}, {{{}splitOnAsciiWhitespace{}}},
{{{}containsAsciiUpperCase{}}}, {{containsAsciiDigit}}|apache/opennlp#1275|
|OPENNLP-1930|part 2: Arvores Deitadas markup parsing|apache/opennlp#1276|
|OPENNLP-1931|part 3: JSON vocabulary and id2label scrape in
opennlp-dl|apache/opennlp#1277|
|OPENNLP-1932|part 4: wildcard matching in the model
resolver|apache/opennlp#1278|
|OPENNLP-1933|part 5: per-call String regex splits and
replacements|apache/opennlp#1279|
|OPENNLP-1929|bug: {{BasicContextGenerator}} splits on its separator as a
regular expression|apache/opennlp#1280|
|OPENNLP-1934|part 6: tokenizer alphanumeric pattern evaluated as a character
set|apache/opennlp#1281|
|OPENNLP-1935|part 7: checkstyle guard and the exempt list in
{{checkstyle-suppressions.xml}}|apache/opennlp#1282|
h3. In scope: incidental usage to replace
||Class||Module||Regex usage||Ticket||
|{{{}DefaultLemmatizerContextGenerator{}}},
{{DefaultPOSContextGenerator}}|opennlp-runtime|{{[A-Z]}} / {{[0-9]}} presence
checks|OPENNLP-1928|
|{{FeatureGeneratorUtil}}|opennlp-runtime|{{^[A-ZÄÖÜ]\.$}}|OPENNLP-1928|
|{{TokenPatternFeatureGenerator}}|opennlp-runtime|{{[^a-zA-Z]}}
strip|OPENNLP-1928|
|{{{}BioCodec{}}}, {{NameFinderME}}|opennlp-runtime|{{(.{+})-\w{+}}} outcome
split|OPENNLP-1928|
|{{BrownCluster}}|opennlp-runtime|split on tab|OPENNLP-1928|
|{{lang.en.TokenSampleStream}}|opennlp-runtime|{{split("}}
{{{}s+"){}}}, {{[A-Za-z0-9]}} check|OPENNLP-1928|
|{{ConlluStream}}|opennlp-formats|{{{}split("-"){}}},
{{text_([a-z]{2,3})}}|OPENNLP-1928|
|{{EmojiCharSequenceNormalizer}}|opennlp-runtime|surrogate-range
replace|OPENNLP-1928|
|{{ParserTool}}|opennlp-cli|parenthesis spacing|OPENNLP-1928|
|{{DownloadUtil}}|opennlp-runtime|{{split("}}
{{{}s"){}}}, HTML link scrape|OPENNLP-1928|
|{{{}ADNameSampleStream{}}}, {{{}ADPOSSampleStream{}}},
{{{}ADSentenceSampleStream{}}}, {{ADSentenceStream}}|opennlp-formats|AD corpus
markup, whitespace, metadata, and punctuation patterns|OPENNLP-1928 (trivial
patterns), OPENNLP-1930 (node, leaf, tag, and metadata patterns)|
|{{{}AbstractDL{}}}, {{DocumentCategorizerConfig}}|opennlp-dl|ad-hoc JSON
scrape|OPENNLP-1931|
|{{{}AbstractClassPathModelFinder{}}}, {{{}DirectoryModelFinder{}}},
{{SimpleClassPathModelFinder}}|opennlp-model-resolver|wildcard-to-regex glob
matching, class path split|OPENNLP-1932|
|{{{}SimpleEventStreamBuilder{}}}, {{{}RealValueFileEventStream{}}},
{{RealBasicEventStream}}|opennlp-ml-commons, opennlp-ml-maxent|{{split("}}
{{s+")}}|OPENNLP-1933|
|{{LeipzigLanguageSampleStream}}|opennlp-formats|{{matches("[a-z]+")}}|OPENNLP-1933|
|{{{}MascNamedEntityParser{}}}, {{{}MascPennTagParser{}}},
{{MascWordParser}}|opennlp-formats|{{replaceFirst}} of id prefixes|OPENNLP-1933|
|{{MorfologikDictionaryBuilder}}|opennlp-morfologik|anchored {{replaceAll}} of
the metadata suffix|OPENNLP-1933|
|{{FrequencyDictionaryLoader}}|opennlp-spellcheck|{{[\t ]+}} column
split|OPENNLP-1933|
|{{BasicContextGenerator}}|opennlp-ml-maxent|{{split(separator)}} with a
caller-supplied separator; documented as a character, evaluated as a
pattern|OPENNLP-1929|
|{{{}TokenizerME{}}}, {{TokSpanEventStream}}|opennlp-runtime|per-token match of
the alphanumeric pattern|OPENNLP-1934: built-in {{^[...]+$}} defaults run as a
character set lookup, other patterns still use the engine|
Not in scope: single-character literal splits such as {{split(",")}} or
{{split("\t")}} take the JDK fast path and compile no Pattern.
h3. Exempt: RegEx as the class's contract (no change)
||Class||Module||Why exempt||
|{{{}RegexNameFinder{}}}, {{RegexNameFinderFactory}}|opennlp-runtime|the class
IS a regex step (OPENNLP-1925 note added)|
|{{AncoraSpanishHeadRules}}|opennlp-runtime|head rules are regex strings
defined by the treebank spec|
|{{Parse}}|opennlp-api|Penn Treebank markup grammar|
|{{NameSample}}|opennlp-api|name-sample markup grammar|
|{{{}TokenizerFactory{}}}, {{lang.Factory}}|opennlp-runtime|the alphanumeric
pattern is written to the model manifest as a regex string and read back with
{{{}Pattern.compile{}}}, so the {{Pattern}} type is part of the model contract
(settled in OPENNLP-1934; the hot path no longer uses the engine for the
shipped defaults)|
|{{SpellCheckingCharSequenceNormalizer}}|opennlp-spellcheck|the URL heuristic
is an alternation of schemes, an email shape, and a TLD list|
The exempt list is enforced: OPENNLP-1935 adds the checkstyle checks
{{NoRegexImport}} and {{NoRegexStringCall}} for production sources, with these
classes as the documented suppressions.
h3. Rules for replacement
* Behavior must be identical; existing tests stay green, and any predicate
rewrite gets tests for both the accept and reject side, including empty input,
leading and trailing separators, non-ASCII spaces and digits, and
supplementary-plane characters. Where practical the old pattern is kept in the
test as the oracle.
* Where a rewrite exposes a behavior difference, it is a bug: the failing test
is committed before the fix.
* Replacements use cursor and code-point loops only; whitespace goes through
{{StringUtil.isWhitespace}} or {{StringUtil.isAsciiWhitespace}} (the {{\s}}
set), never {{{}Character.isWhitespace{}}}; case mapping goes through
{{{}StringUtil{}}}.
* Shared scans live in {{StringUtil}} when more than one module needs them, in
a package-private helper otherwise.
* One commit per class or cohesive group so each fix can be cherry-picked onto
its own ticket branch.
One note on the {{text_([a-z]{2,3})}} cell: the braces are escaped so JIRA does
not read them as a macro, same as in the 1928 description. If the preview shows
the backslashes, the
{noformat}
{noformat}
fallback from earlier applies.
was:
OpenNLP's processing path is RegEx-free by policy; a genuine RegEx step (for
example {{RegexNameFinder}}) is the documented exception (OPENNLP-1925). An
audit of main found the following production classes touching
{{java.util.regex}}. This epic covers replacing every incidental usage with
cursor or code-point loops. Classes whose job IS pattern matching are exempt
and listed here only for completeness.
h3. In scope: incidental usage to replace
|| Class || Module || Regex usage || Effort ||
| {{DefaultLemmatizerContextGenerator}} | opennlp-runtime | {{[A-Z]}} /
{{[0-9]}} presence checks | trivial: code-point loop |
| {{DefaultPOSContextGenerator}} | opennlp-runtime | {{[A-Z]}} / {{[0-9]}}
presence checks | trivial: code-point loop |
| {{FeatureGeneratorUtil}} | opennlp-runtime | {{^[A-ZÄÖÜ]\.$}} | trivial:
length-2 check |
| {{TokenPatternFeatureGenerator}} | opennlp-runtime | {{[^a-zA-Z]}} strip |
trivial: code-point loop |
| {{BioCodec}} | opennlp-runtime | {{(.+)-\w+}} outcome split | trivial:
lastIndexOf('-') |
| {{NameFinderME}} | opennlp-runtime | {{(.+)-\w+}} outcome split | trivial:
lastIndexOf('-') |
| {{BrownCluster}} | opennlp-runtime | split on tab | trivial: cursor split |
| {{TokenSampleStream}} | opennlp-runtime | {{split("\\s+")}} | trivial: cursor
split |
| {{ConlluStream}} | opennlp-formats | {{split("-")}}, {{text_([a-z]{2,3})}} |
trivial: startsWith / substring |
| {{EmojiCharSequenceNormalizer}} | opennlp-runtime | surrogate-range replace |
trivial: code-point loop |
| {{ParserTool}} | opennlp-cli | parenthesis spacing | moderate: single cursor
scan |
| {{DownloadUtil}} | opennlp-runtime | {{split("\\s")}}, HTML link scrape |
moderate: cursor + tag scan |
| {{AbstractDL}} | opennlp-dl | ad-hoc JSON field scrape | moderate: cursor
parse |
| {{DocumentCategorizerConfig}} | opennlp-dl | ad-hoc JSON map scrape |
moderate: cursor parse |
| {{ADNameSampleStream}}, {{ADPOSSampleStream}}, {{ADSentenceSampleStream}},
{{ADSentenceStream}} | opennlp-formats | AD corpus markup parsing | moderate:
cursor parse |
| {{AbstractClassPathModelFinder}}, {{DirectoryModelFinder}},
{{SimpleClassPathModelFinder}} | opennlp-model-resolver | wildcard-to-regex
glob matching | moderate: hand-rolled glob |
h3. Exempt: genuine RegEx steps (no change)
|| Class || Module || Why exempt ||
| {{RegexNameFinder}}, {{RegexNameFinderFactory}} | opennlp-runtime | the class
IS a regex step (OPENNLP-1925 note added) |
| {{AncoraSpanishHeadRules}} | opennlp-runtime | head rules are regex strings
defined by the treebank spec |
| {{Parse}} | opennlp-api | Penn Treebank markup grammar |
| {{NameSample}} | opennlp-api | name-sample markup grammar |
Still determining:
|| Class || Module || Investigating ||
| {{TokenizerFactory}}, {{TokenizerME}}, {{TokSpanEventStream}},
{{lang.Factory}}, {{lang.en.TokenSampleStream}} | opennlp-runtime |
alphanumeric pattern comes from model training parameters, part of the API
contract |
h3. Rules for replacement
* Behavior must be identical; existing tests stay green, and any predicate
rewrite gets pinning tests for both the accept and reject side.
* Replacements use cursor and code-point loops only; case mapping goes through
{{StringUtil}}.
* One commit per class or cohesive group so each fix can be cherry-picked onto
its own ticket branch.
> Replace incidental RegEx usage in production code with cursor and code-point
> loops
> ----------------------------------------------------------------------------------
>
> Key: OPENNLP-1926
> URL: https://issues.apache.org/jira/browse/OPENNLP-1926
> Project: OpenNLP
> Issue Type: Epic
> Reporter: Kristian Rickert
> Assignee: Kristian Rickert
> Priority: Major
>
> OpenNLP's processing path is RegEx-free by policy; a RegEx step that is the
> class's purpose (for example {{{}RegexNameFinder{}}}) is the documented
> exception (OPENNLP-1925). An audit of main found the production classes below
> touching {{{}java.util.regex{}}}, and the work turned up a second group:
> {{{}String.split{}}}, {{{}matches{}}}, {{{}replaceAll{}}}, and
> {{replaceFirst}} calls that compile a Pattern on each call without an import.
> This epic replaces each incidental usage with cursor or code-point loops and
> ends with a checkstyle guard so it stays that way. Classes whose job IS
> pattern matching are exempt and listed for completeness.
> h3. Tickets, in merge order
> Each branch is stacked on the one before it, so the pull requests merge in
> this order and OPENNLP-1935 merges last.
> ||Ticket||Scope||PR||
> |OPENNLP-1928|part 1: trivial batch, plus the shared {{StringUtil}} helpers
> {{{}isAsciiWhitespace{}}}, {{{}splitOnAsciiWhitespace{}}},
> {{{}containsAsciiUpperCase{}}}, {{containsAsciiDigit}}|apache/opennlp#1275|
> |OPENNLP-1930|part 2: Arvores Deitadas markup parsing|apache/opennlp#1276|
> |OPENNLP-1931|part 3: JSON vocabulary and id2label scrape in
> opennlp-dl|apache/opennlp#1277|
> |OPENNLP-1932|part 4: wildcard matching in the model
> resolver|apache/opennlp#1278|
> |OPENNLP-1933|part 5: per-call String regex splits and
> replacements|apache/opennlp#1279|
> |OPENNLP-1929|bug: {{BasicContextGenerator}} splits on its separator as a
> regular expression|apache/opennlp#1280|
> |OPENNLP-1934|part 6: tokenizer alphanumeric pattern evaluated as a character
> set|apache/opennlp#1281|
> |OPENNLP-1935|part 7: checkstyle guard and the exempt list in
> {{checkstyle-suppressions.xml}}|apache/opennlp#1282|
> h3. In scope: incidental usage to replace
> ||Class||Module||Regex usage||Ticket||
> |{{{}DefaultLemmatizerContextGenerator{}}},
> {{DefaultPOSContextGenerator}}|opennlp-runtime|{{[A-Z]}} / {{[0-9]}} presence
> checks|OPENNLP-1928|
> |{{FeatureGeneratorUtil}}|opennlp-runtime|{{^[A-ZÄÖÜ]\.$}}|OPENNLP-1928|
> |{{TokenPatternFeatureGenerator}}|opennlp-runtime|{{[^a-zA-Z]}}
> strip|OPENNLP-1928|
> |{{{}BioCodec{}}}, {{NameFinderME}}|opennlp-runtime|{{(.{+})-\w{+}}} outcome
> split|OPENNLP-1928|
> |{{BrownCluster}}|opennlp-runtime|split on tab|OPENNLP-1928|
> |{{lang.en.TokenSampleStream}}|opennlp-runtime|{{split("}}
> {{{}s+"){}}}, {{[A-Za-z0-9]}} check|OPENNLP-1928|
> |{{ConlluStream}}|opennlp-formats|{{{}split("-"){}}},
> {{text_([a-z]{2,3})}}|OPENNLP-1928|
> |{{EmojiCharSequenceNormalizer}}|opennlp-runtime|surrogate-range
> replace|OPENNLP-1928|
> |{{ParserTool}}|opennlp-cli|parenthesis spacing|OPENNLP-1928|
> |{{DownloadUtil}}|opennlp-runtime|{{split("}}
> {{{}s"){}}}, HTML link scrape|OPENNLP-1928|
> |{{{}ADNameSampleStream{}}}, {{{}ADPOSSampleStream{}}},
> {{{}ADSentenceSampleStream{}}}, {{ADSentenceStream}}|opennlp-formats|AD
> corpus markup, whitespace, metadata, and punctuation patterns|OPENNLP-1928
> (trivial patterns), OPENNLP-1930 (node, leaf, tag, and metadata patterns)|
> |{{{}AbstractDL{}}}, {{DocumentCategorizerConfig}}|opennlp-dl|ad-hoc JSON
> scrape|OPENNLP-1931|
> |{{{}AbstractClassPathModelFinder{}}}, {{{}DirectoryModelFinder{}}},
> {{SimpleClassPathModelFinder}}|opennlp-model-resolver|wildcard-to-regex glob
> matching, class path split|OPENNLP-1932|
> |{{{}SimpleEventStreamBuilder{}}}, {{{}RealValueFileEventStream{}}},
> {{RealBasicEventStream}}|opennlp-ml-commons, opennlp-ml-maxent|{{split("}}
> {{s+")}}|OPENNLP-1933|
> |{{LeipzigLanguageSampleStream}}|opennlp-formats|{{matches("[a-z]+")}}|OPENNLP-1933|
> |{{{}MascNamedEntityParser{}}}, {{{}MascPennTagParser{}}},
> {{MascWordParser}}|opennlp-formats|{{replaceFirst}} of id
> prefixes|OPENNLP-1933|
> |{{MorfologikDictionaryBuilder}}|opennlp-morfologik|anchored {{replaceAll}}
> of the metadata suffix|OPENNLP-1933|
> |{{FrequencyDictionaryLoader}}|opennlp-spellcheck|{{[\t ]+}} column
> split|OPENNLP-1933|
> |{{BasicContextGenerator}}|opennlp-ml-maxent|{{split(separator)}} with a
> caller-supplied separator; documented as a character, evaluated as a
> pattern|OPENNLP-1929|
> |{{{}TokenizerME{}}}, {{TokSpanEventStream}}|opennlp-runtime|per-token match
> of the alphanumeric pattern|OPENNLP-1934: built-in {{^[...]+$}} defaults run
> as a character set lookup, other patterns still use the engine|
> Not in scope: single-character literal splits such as {{split(",")}} or
> {{split("\t")}} take the JDK fast path and compile no Pattern.
> h3. Exempt: RegEx as the class's contract (no change)
> ||Class||Module||Why exempt||
> |{{{}RegexNameFinder{}}}, {{RegexNameFinderFactory}}|opennlp-runtime|the
> class IS a regex step (OPENNLP-1925 note added)|
> |{{AncoraSpanishHeadRules}}|opennlp-runtime|head rules are regex strings
> defined by the treebank spec|
> |{{Parse}}|opennlp-api|Penn Treebank markup grammar|
> |{{NameSample}}|opennlp-api|name-sample markup grammar|
> |{{{}TokenizerFactory{}}}, {{lang.Factory}}|opennlp-runtime|the alphanumeric
> pattern is written to the model manifest as a regex string and read back with
> {{{}Pattern.compile{}}}, so the {{Pattern}} type is part of the model
> contract (settled in OPENNLP-1934; the hot path no longer uses the engine for
> the shipped defaults)|
> |{{SpellCheckingCharSequenceNormalizer}}|opennlp-spellcheck|the URL heuristic
> is an alternation of schemes, an email shape, and a TLD list|
> The exempt list is enforced: OPENNLP-1935 adds the checkstyle checks
> {{NoRegexImport}} and {{NoRegexStringCall}} for production sources, with
> these classes as the documented suppressions.
> h3. Rules for replacement
> * Behavior must be identical; existing tests stay green, and any predicate
> rewrite gets tests for both the accept and reject side, including empty
> input, leading and trailing separators, non-ASCII spaces and digits, and
> supplementary-plane characters. Where practical the old pattern is kept in
> the test as the oracle.
> * Where a rewrite exposes a behavior difference, it is a bug: the failing
> test is committed before the fix.
> * Replacements use cursor and code-point loops only; whitespace goes through
> {{StringUtil.isWhitespace}} or {{StringUtil.isAsciiWhitespace}} (the {{\s}}
> set), never {{{}Character.isWhitespace{}}}; case mapping goes through
> {{{}StringUtil{}}}.
> * Shared scans live in {{StringUtil}} when more than one module needs them,
> in a package-private helper otherwise.
> * One commit per class or cohesive group so each fix can be cherry-picked
> onto its own ticket branch.
> One note on the {{text_([a-z]{2,3})}} cell: the braces are escaped so JIRA
> does not read them as a macro, same as in the 1928 description. If the
> preview shows the backslashes, the
> {noformat}
> {noformat}
> fallback from earlier applies.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
