Extract compiler

[aslakhellesoy]

🤔 What's changed?

Compilation of a Cucumber Expression AST to a RegExp is extracted to two separate classes:

• AbstractCompiler
• RegExpCompiler

⚡️ What's your motivation?

In order to fix cucumber/language-service#16 we need to be able to generate a Gherkin step text from a Cucumber Expression. For example, we should be able to generate I have 0 cukes from the Cucumber Expression I have {int} cukes.

This can be done by subclassing AbstractCompiler and generating steps instead of regexps.

🏷️ What kind of change is this?

• 🏦 Refactoring/debt/DX (improvement to code design, tooling, documentation etc. without changing behaviour)

♻️ Anything particular you want feedback on?

I don't think this is necessary to port to the other languages. Being able to extend the compiler in this way is only needed by the language server.

[aurelien-reeves]

Maybe we could also export RegExpCompiler? That may be useful sometime?

[aslakhellesoy]

It's only used internally, so I'd rather not. By exposing as little as possible we minimise the API surface to maintain.

[aurelien-reeves]

Well done!💪

I guess at the moment, tests relies on former CucumberExpressionTest ones?

Would it worth the cost to refactor the tests in the same way the code has been refactored?

[aslakhellesoy]

Well done!💪

I guess at the moment, tests relies on former CucumberExpressionTest ones?

Would it worth the cost to refactor the tests in the same way the code has been refactored?

I don't think it's needed - it's a bit tricky to do since CucumberExpressionTest uses shared test data, and I don't want to port this to the other languages.

[mpkorstanje]

This doesn't look like a well thought out addition to the public API. Though It's a bit hard to provide constructive criticism, there are no examples of how this would be used.

I don't believe the compiler contains anything that could be reused for this use case. It recursively invokes compile on all nodes of the AST. This is something that could be duplicated relatively easily in another project and would instead depend on the AST rather then an adhoc defined abstract compiler.

Perhaps the language service needs valid ASTs? If so then it might be worth splitting of the validations from the compiler. But again hard to say.

[aslakhellesoy]

Fair point. The thing I wanted to reuse from the compiler is the traversal and validation, but I'm happy to reimplement the traversal in the language service. The validation isn't needed - it's already done when the CucumberExpression is created.

The only thing I need is to expose the AST. I've expressed some reservations against this in #51 (comment), but it looks like #41 might take a while, so I think we should do it anyway.

What do you think about an updated PR where I just expose the AST?

[mpkorstanje]

I don't expect #41 to happen anytime soon. I'm less opposed to exposing the AST then you. But it might need some prettying.

[aslakhellesoy]

I'm not worried about exposing the AST anymore since 41 has been quiet.

What kind of prettying did you have in mind for the AST?

[aslakhellesoy]

Superseded by #109