[RaphaelFakhri]

Fixes #1305

Parser.meta() treats any ? that follows a RepeatMatcher as the reluctant modifier of the repetition and returns the repetition unchanged. A group whose body compiles to a single RepeatMatcher, such as (\d{2}), takes the same path, so (\d{2})? loses its ? and the group becomes required. ^v(\d{2})?$ does not match v.

A + after {n,m} is not consumed either, so the possessive a{2}+ is parsed as (a{2})+, and ^a{2}+$ matches aaaa.

Changes

• In Parser.repeat(), consume an optional ? or + modifier directly after the closing }. RepeatMatcher already matches greedily without backtracking, so both modifiers keep the current matching behavior for the repetition itself.
• Remove the RepeatMatcher special case from meta(), so a ? after a group always makes the group optional.
• Add repeatInOptionalGroup and repeatModifiers to AbstractPatternMatcherTest. They compare the results with java.util.regex.Pattern for all of the matcher test variants.

Testing

./gradlew :spectator-api:test --tests 'com.netflix.spectator.impl.matcher.*'

• Without the fix: 629 tests, 10 failures (both new tests in PatternMatcherTest, CaseInsensitivePatternMatcherTest, ExpandOrPatternMatcherTest, Re2PatternMatcherTest, and Re2CaseInsensitivePatternMatcherTest), for example ^v(\d{2})?$ should match v and ^a{2}+$ shouldn't match aaaa.
• With the fix: 629 tests, 0 failures.
• ./gradlew :spectator-api:build (checkstyle, PMD, SpotBugs, license, tests on JDK 8, 17, and 25), ./gradlew license, and ./gradlew :spectator-reg-atlas:test pass.

[brharrington]

Thanks for the fix. While reviewing, we found that {n,} was being parsed as {n}, and with this change x{n,}+ would have become exactly x{n}, so patterns like ^\d{2,}+$ that matched before would stop matching. Rather than going back and forth, I pushed a few commits on top of your branch:

• Merge of main: brings in the recent matcher fixes (Fix SQL pattern for prefix ending in literal % #1308–Fix optimizer for content after end anchor #1311).
• Fix open ended repeat and modifier parsing:
  • {n,} is now parsed as at least n, built the same way as +/* (n repetitions followed by zero or more), so it can still give characters back to what follows. {n,}+ is possessive.
  • The ?/+ modifier after } is handled in repeat() for every repeat form, the same approach as your change. Your tests are kept, and the cases from our review are merged into repeatModifiers.
  • ++ no longer requires two matches.
  • Malformed repetitions (a{2,3, a{2,3,4}, a { with nothing before it) now raise clear errors.
  • toPatternExpr checks the RE2 1000-repeat limit for nested repeats.
• Flatten repetition of unbounded repetition:
  • The optimizer rewrites shapes like (a{2,}){2} to a{4,} and (a+)?b to a*b. The matcher doesn't backtrack into a repeat, but these forms match the same strings and stay consistent with java.util.regex.
  • RepeatMatcher.rewrite wasn't recursing, so optimizer rules and ignoreCase were skipped inside repeats. For example, (?i)(ab|cd){2,} didn't match ABCD. That is fixed too.

./gradlew :spectator-api:build and :spectator-reg-atlas:test pass locally. Let me know if anything looks off.