Skip to content

Remove warning on overlapping character classes#175

Open
jneen wants to merge 1 commit intok-takata:masterfrom
jneen:remove-warning-on-overlapping-char-classes
Open

Remove warning on overlapping character classes#175
jneen wants to merge 1 commit intok-takata:masterfrom
jneen:remove-warning-on-overlapping-char-classes

Conversation

@jneen
Copy link
Copy Markdown

@jneen jneen commented Mar 13, 2026

Re https://bugs.ruby-lang.org/issues/21870

Rationale

In https://bugs.ruby-lang.org/issues/1831, warnings were added for duplicate characters in a character class, specifically to prevent cases like /[:alpha:]/. However, this was implemented in a much more general fashion, and as of the version of Onigmo in Ruby 4, classes such as \p{Word} and \p{S} now overlap, at ZWJ and ZWNJ specifically.

This has resulted in expressions like /[\p{Word}\p{S}]/ emitting an unconditional warning, despite the use of these partially-overlapping classes being unlikely to be a mistake. Duplicate ranges do not seem to incur any performance penalty, as the ranges are simplified at regexp compile time.

Proposal

This pull request is the simplest possible solution to the problem - it simply does not check unicode ranges for overlap. This preserves the original intent of the warning:

$ ./ruby -we 'p %r/[:alpha:]/'
-e:1: warning: character class has duplicated range: /[:alpha:]/

$ ./ruby -we 'p %r/[aa]/`
-e:1: warning: character class has duplicated range: /[aa]/

$ ./ruby -we 'p %r/[\p{S}\p{S}]/'
-e:1: warning: character class has duplicated range: /[\p{S}\p{S}]/

while allowing character classes to be used freely:

$ ./ruby -we 'p %r/[\p{Word}\p{S}]/'
(no warning)

Though I can't necessarily explain why, these also do seem to warn properly with this change, though I had initially expected them not to:

$ ./ruby -we 'p(/[\p{Word}\p{Alpha}]/)'
-e:1: warning: character class has duplicated range: /[\p{Word}\p{Alpha}]/

$ ./ruby -we 'p(/[a-fb-g]/)'
-e:1: warning: character class has duplicated range: /[a-fb-g]/

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant