Add Rule keyword support

[unkind]

Hi,

PR provides the Rule keyword support (https://cucumber.io/docs/gherkin/reference/#rule).

[ciaranmcnulty]

I really want this feature, looking forward to reviewing it

[unkind]

This one still doesn't work: 'complex_background.feature' => 'Rule keyword not supported',.

It turns out Scenario and Scenario Outline have the same node type in Cucumber:

Feature: Complex background

  ...

  Rule: My Rule
    Scenario: with examples
      Given the <value> minimalism

      Examples:
      | value |
      | 1     |
      | 2     |

[ciaranmcnulty]

It's really good that you've used the cucumber tests to guide things - your comment about the Scenario Outline difference is captured in #153

I've left a few comments, the ones marked [BC] would allow us to do this as a minor release, I'm not sure if that's the right approach or not yet

[ciaranmcnulty]

[BC] We should define a new interface that extends KeywordsInterface and adds this method

[ciaranmcnulty]

[BC] we should add this as the last parameter and make it optional (with a default)

[ciaranmcnulty]

I have a slight concern that we lose the ordering of scenarios+rules here by having them as separate arguments. (The main concern is the impact on Behat's formatters).

Perhaps we should be passing an array of Scenarios+Rules? Needs some thought

[ciaranmcnulty]

[BC] We need to consider how a consumer will know this method is available. Maybe we need an interface to hint it? I slightly prefer

if ($featureNode instanceof FeatureRulesNode) {

vs

if(method_exists($featureNode, 'getRules')){

[stof]

Well, if a consumer does not want to have to check for existence, the easiest way is to bump their min version of behat/gherkin.

To me, there is no need for an interface (which would not be allowed to be removed without a BC break, and there is no right way to trigger deprecations for instanceof checks relying on deprecated interfaces, only static analysis can do it)

[ciaranmcnulty]

I guess this should be resolved before merge

[ciaranmcnulty]

@pamil @stof we need to consider how Behat will adapt to this change. It'll impact tagging, running and a lot more.

1. We could do it in a minor version of behat/gherkin with BC layers, then Behat can do some conditional logic to see if the parser supports rules

   • Quite a 'safe' option
   • Could enable a situation where a user has a Rule-aware Gherkin but not a Rule-aware Behat and therefore rules aren't a parse error, but are ignored by the runner

2. We could do it in a major, expand Behat's supported versions, and have similar conditional logic

   • Guarantees Behat understands rules if they're parsed
   • Probably messier conditional logic in Behat if we didn't put in a BC layer

3. We could do it in a major and bump Behat's dependency overnight

   • Guarantees Behat understands rules if they're parsed
   • No BC issues
   • Could conflict on codebases that use e.g. Codeception+Behat (does that happen?)

[unkind]

I found inconsistency with https://github.com/cucumber/common:

Feature: Some rules

  Background:
    Given fb

  Rule: A
    The rule A description

    Background:
      Given ab

    Example: Example A
      Given a

That parser doesn't remove indentation of description:

"description":"    The rule A description"

We can keep indentation data in FeatureNode, RuleNode, etc. and provide 2 methods: getDescription() and getRawDescription().

[unkind]

#245 is just better way for this.