cucumber-expressions: regexp uniqueness constraint in transforms is unnecessary #132

Open
jbpros opened this Issue Mar 11, 2017 · 2 comments

Projects

None yet

2 participants

@jbpros
Member
jbpros commented Mar 11, 2017

Summary

There is no need to enforce uniqueness of regular expressions between custom parameter types.

Current Behavior

When using cucumber.js-2.0.0-rc.8, the following code:

defineParameterType({
  regexp: /[A-Z]\w+/,
  transformer: name => name,
  typeName: 'name'
})

defineParameterType({
  regexp: /[A-Z]\w+/,
  transformer: name => world.fetchPerson(name),
  typeName: 'person'
})

Triggers the following error:

Error: There is already a parameter with regexp [A-Z]\w+
    at set (/Users/jbpros/Projects/cucumber-pro/node_modules/cucumber-expressions/dist/parameter_type_registry.js:132:45)

Desired Behavior

These two transforms are expected to return related things: the first one returns the name of a person, the second one returns an instance of the person, based on their name. Therefore, they both need to use the same regular expression.

I would expect the code above to not throw any error, the constraint on regular expression uniqueness between parameter types to not be enforced. Two reasons for that:

  1. The types are always explicit in step definition expressions, therefore I think there is no ambiguity possible when identifying types and their transform functions.
  2. Enforcing regular expressions uniqueness is not robust anyway. I was able to circumvent this uniqueness constraint by using two different regular expressions exposing the exact same rules:
  • /[A-Z]\w+/
  • /[A-Z]\w{1,}/

Possible Solution

I think we can safely remove this check on regular expression uniqueness as long as type names are uniquely identified.

@aslakhellesoy
Member

I agree.

The Cucumber Expressions library is also responsible for producing expressions for undefined steps.

In doing that, it will suggest I have a {color} ball for an undefined step I have a blue ball, if there is a parameter type registered that matches blue.

What should it suggest if there are multiple parameter types that match?

@jbpros
Member
jbpros commented Mar 11, 2017

I literally realised this 10 minutes ago while playing with step defs 😃 les grands esprits se rencontrent.

I'm not sure what we should do. We could suggest alternative expressions. e.g. in cucumber.js's output:

(...)
Undefined. Implement with one of the following snippets:

       Given('I have a {color} ball', async function (color) {
         return 'pending'
       })

       Given('I have a {cssColor} ball', async function (cssColor) {
         return 'pending'
       })

It could potentially lead to an explosion of alternatives, if there are several params that have multiple matching types.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment