petitparser / dart-petitparser

Dynamic parser combinators in Dart.
https://pub.dartlang.org/packages/petitparser
MIT License
457 stars 48 forks source link

.flatten() ignores mapped values. #105

Closed gmpassos closed 3 years ago

gmpassos commented 3 years ago

If flatten() is added at the end of this parser, it will ignore any mapped value:

  Parser<String> stringContentDoubleQuotedLexicalTokenEscaped() => (char('\\') &
              (char('n').map((_) => '\n') |
                  char('r').map((_) => '\r') |
                  char('"').map((_) => '"') |
                  char("'").map((_) => "'") |
                  char('t').map((_) => '\t') |
                  char('b').map((_) => '\b') |
                  char('\\').map((_) => '\\')))
          .map((v) {
        return v[1] as String;
      }).flatten();

If this is the real behavior of flatten (FlattenParser), this should be documented, since any check of map call will be CPU intensive.

My suggestion is to keep the current behavior, due to performance, and just add a documentation description. Or add an assertion mode for the grammar, enabling additional checks, useful for unit tests.

renggli commented 3 years ago

Yes, this is the intended behavior of the flatten() parser. I've tried to make the documentation a bit more clear. Let me know if there is a way to improve it further?

Regarding a linter or checked mode for grammars that sounds like a great idea. I'd like to keep this issue open for that idea. Note that there is already some tooling that removes unnecessary parsers and optimizes the grammar: https://github.com/petitparser/dart-petitparser/blob/main/petitparser/lib/src/reflection/optimize.dart.

gmpassos commented 3 years ago

Myabe add at the end of the line #7:

... (any sub-call of map(), cast() and pick(n) will be ignored).