Parser with callable syntax for group & optional

[antonkomarev]

@BenMorel I found it hard to read such code and control start/end manually:

return $parser ??= (new PatternParserBuilder())
    ->startGroup()
    ->appendCapturePattern('[Zz]', TimeZoneOffsetSign::NAME)
    ->appendOr()
    ->startGroup()
    ->appendCapturePattern('[\-\+]', TimeZoneOffsetSign::NAME)
    ->appendCapturePattern(TimeZoneOffsetHour::PATTERN, TimeZoneOffsetHour::NAME)
    ->startOptional()
    ->appendLiteral(':')
    ->appendCapturePattern(TimeZoneOffsetMinute::PATTERN, TimeZoneOffsetMinute::NAME)
    ->endOptional()
    ->startOptional()
    ->appendLiteral(':')
    ->appendCapturePattern(TimeZoneOffsetSecond::PATTERN, TimeZoneOffsetSecond::NAME)
    ->endOptional()
    ->endGroup()
    ->endGroup()
    ->toParser();

Maybe this syntax will be more readable and easier to maintain:

return $parser ??= (new PatternParserBuilder())
    ->group(
        fn(PatternParserBuilder $builder) => $builder
            ->appendCapturePattern('[Zz]', TimeZoneOffsetSign::NAME)
            ->appendOr()
            ->group(
                fn(PatternParserBuilder $builder) => $builder
                    ->appendCapturePattern('[\-\+]', TimeZoneOffsetSign::NAME)
                    ->appendCapturePattern(TimeZoneOffsetHour::PATTERN, TimeZoneOffsetHour::NAME)
                    ->optional(
                        fn(PatternParserBuilder $builder) => $builder
                            ->appendLiteral(':')
                            ->appendCapturePattern(TimeZoneOffsetMinute::PATTERN, TimeZoneOffsetMinute::NAME)
                    )
                    ->optional(
                        fn(PatternParserBuilder $builder) => $builder
                            ->appendLiteral(':')
                            ->appendCapturePattern(TimeZoneOffsetSecond::PATTERN, TimeZoneOffsetSecond::NAME)
                    )
            )

    )
    ->toParser();

[BenMorel]

Hi, this one should be indented as it's indeed barely readable. Once indented though, I feel like it's good enough for our purposes, see this one for example:

return $parser ??= (new PatternParserBuilder())
    ->append(self::localDate())
    ->appendLiteral('/')

    ->startGroup()
        ->startGroup()
            ->append(self::localDate())
        ->endGroup()
    ->appendOr()
        ->startGroup()
            ->startOptional()
                ->appendCapturePattern(MonthOfYear::PATTERN, MonthOfYear::NAME)
                ->appendLiteral('-')
            ->endOptional()
            ->appendCapturePattern(DayOfMonth::PATTERN, DayOfMonth::NAME)
        ->endGroup()
    ->endGroup()

    ->toParser();

I'm afraid a version full of callbacks may not be more readable.

[antonkomarev]

Yeah, I think the same, but currently PHP does not gives us other ways to make it less verbose.

But there is another important thing in my mind. It's reduce error possibilities, because you don't need to think about closing groups, it will be self closable.

But I suppose it will fail on build call, if any groups are not closed.

[BenMorel]

But I suppose it will fail on build call, if any groups are not closed.

Yes exactly, the PatternParserBuilder maintains a stack and throws on unclosed groups.

That being said, I agree that it may not be the most optimal implementation, but I think it's good enough for the job. At least until someone comes up with a better way!