Throw an explicit error when there is nothing to repeat

Y-- commented 3 years ago

Hi!

Thanks for this cool library!

If you do not specify anything to "repeat", such as "*m", here is what we have currently:

 [Message]: Array is empty
   [Stack]: RuntimeError: unreachable
            at ~lib/array/Array<assembly/parser/node/Node>#pop (<anonymous>:wasm-function[100]:0x2504)
            at assembly/parser/parser/Parser#parseSequence (<anonymous>:wasm-function[133]:0x2d84)
            at assembly/parser/parser/Parser#toAST (<anonymous>:wasm-function[134]:0x2dff)
            at assembly/parser/parser/Parser.toAST (<anonymous>:wasm-function[135]:0x2e10)
            at assembly/regexp/RegExp#constructor (<anonymous>:wasm-function[250]:0x4b00)
            at start:assembly/__tests__/regex.spec~anonymous|7~anonymous|0 (<anonymous>:wasm-function[412]:0x7374)
            at node_modules/@as-pect/assembly/assembly/internal/call/__call (<anonymous>:wasm-function[416]:0x73f1)

This is the behavior in JS:

> new RegExp('*m')
Uncaught SyntaxError: Invalid regular expression: /*m/: Nothing to repeat

Couple notes with this PR:

it seems that the assertion on error message isn't actually used (i.e. if I remove the "if" the tests are still green)
as you can see, in JS they repeat the expression: I wasn't quite sure if we wanted to keep a reference on the input in the Parser's constructor and reproduce this behavior

Let me know what you think, happy to update the code in any way!

ColinEberhardt commented 3 years ago

Thanks for this bug-fix, much appreciated. Error handling is an area I haven't really explored in any great depth yet.

Regarding your comments:

it seems that the assertion on error message isn't actually used (i.e. if I remove the "if" the tests are still green)

Sounds like a bug in as-pect - worth raising an issue? https://github.com/jtenner/as-pect

as you can see, in JS they repeat the expression: I wasn't quite sure if we wanted to keep a reference on the input in the Parser's constructor and reproduce this behavior

I'm not concerned about matching this behaviour exactly, especially as it would add more complexity to the code.

github-actions[bot] commented 3 years ago

:tada: This PR is included in version 1.6.1 :tada:

The release is available on:

Your semantic-release bot :package::rocket:

Y-- commented 3 years ago

Perfect, thanks a lot! ~Will try to submit an issue or PR to as-pect then.~ edit: posted here: https://github.com/jtenner/as-pect/issues/332 And sounds perfectly reasonable to me regarding the message!

ColinEberhardt / assemblyscript-regex

Throw an explicit error when there is nothing to repeat #35