Fix case where incorrect number of ranges reported

[jcavar]

When defining a step with optional capturing group at the end, incorrect number of ranges is reported. We need to check if range is valid before assigning group.

I've added tests similar to others, but I think we should maybe have unit test suite that cover parsing logic? It would make it easier to add tests and cover more edge cases.

[kerrmarin]

This is covered in the readme: https://github.com/net-a-porter-mobile/XCTest-Gherkin#ambiguous-steps with the currently preferred approach

[jcavar]

Hmm, I think this is not the same case. What this PR fixes is one step definition that matches multiple steps. From what I understand from README, it is multiple step definitions that match single step?

[jcavar]

Additionally, If you try to run test without this fix, the code crashes. That is I believe not expected behaviour even in example from README.

[ilyapuchka]

@jcavar I think what @kerrmarin means by referring this section of readme is that rather than using optional groups the same can be achieved with following two steps:

step("^I verify (.+) password$") {
  ...
}

step("I verify (.+) password against (.+) username") {
  ...
}

This will avoid crash and ambiguity of steps. At the same time I agree that crashing is not nice from framework POV and the fix does not change any behaviour and fixes this case, so I'm 👍 for it. At the same time I would encourage using two different steps as it makes it more clear for the reader and makes steps closer to single responsibility principle.

[deanWombourne]

@jcavar I've just released this as version 0.19.2, thanks for your help :)