[Cucumber-JVM] Parameter type priority and mixing regular/cucumber-expression step-definitions

[Shannon Cole]

Hello Cucumber folks,

I'm trying to understand glue code resolution in the presence of step

definitions using Cucumber Expressions and Regular Expressions.

io.cucumber.cucumberexpressions.RegularExpression implements a step

definition lookup which prioritises matching custom parameter type

regexes over the type hint taken from the type of the method

arguments. This seems backwards. It means that we can't define

parameter types which use the same regex.

The argument can be made that there should not be more than one

parameter type with the same regex; but it's actually a very common

use-case. For example: there are many ways to convert quoted strings

to data types. It should be possible to define all of these ways using

a parameter type annotation like:

    @ParameterType("\"([^\"]+)\"")

It seems arbitrary that whichever one is defined first and happens to

occupy `parameterTypesByRegexp` in `ParameterTypeRegistry` will then

commandeer every regular-expression-step-definition which wants to

accept a quoted string.

I'm guessing that this functionality makes sense for JVM languages

which have untyped glue definitions; but it seems to me that the type

hint, if present, should definitely take precedence over a regex which

just happens to match.

Are you open to a PR which adjusts the behaviour?

Finally, thanks for a great tool! We are getting a lot of mileage out

of Cucumber.

Shannon

PS: The posting rules link is dead.

https://cucumber.io/support/posting-rules

[MP Korstanje]

Heya Shannon,

you've guessed correctly. Cucumber expressions were developed with weakly typed languages in mind (javascript, ruby, ect).

I've bolted on anonymous parameter types which will try to convert arbitrary strings to arbitrary types using the type hints.

So I guess it's time to go all the way now and properly support types in parameter types.

>  Are you open to a PR which adjusts the behaviour?

If we can preserve backwards compatibility, then definitely.

You can find the source code here: https://github.com/cucumber/cucumber/tree/master/cucumber-expressions

The build process of the Cucumber repo is quite complicated. It has to support polyglot components.

So for now I would recommend cloning the project and opening the cucumber-expressions/java directory in your IDE.

Cheers,
Rien

[Shannon Cole]

Hi Rien,

I'm unsure whether what I have in mind is backwards compatible.

I found some discussion here:

https://github.com/cucumber/cucumber/issues/658

> So we now have a library that tries to guess the types based on the

> regex you use instead of the other way around.

I think this is the crucial point. From a Java perspective, where the

step definition already has the types in the method signature, it

doesn't make sense to use a type derived from a regexp match. If there

is a mismatch, we'll immediately get an exception when the glue is

invoked.

(Which seems to be the same issue as #658.)

The change I have in mind is to always use the type hint and never

allow a regexp match to override the type hint, this would cascade to

removing `useRegexpMatchAsStrongTypeHint` as it would no longer be

used. As far as I can tell, this makes sense for Java. I can't speak

for the other implementations. I guess that for Ruby, there are no

type hints?

I have pushed some changes here:

https://github.com/cucumber/cucumber/compare/master...scolej:type-hint-precedence

Does this look backwards compatible?

(Also on that branch is a minor performance tweak which may be of interest.)

Have a good weekend!

Shannon

[M.P. Korstanje]

Hey Shannon,

That doesn't look backwards compatible at a glance. But please create a PR so we can discuss it with context at hand.

Currently you create an anonymous parameter type when a type hint is available. This anonymous parameter type delegates to the default transformer.

However if I understood you correctly you've suggested that based on the type hint you wanted to look up a parameter type by its type rather then regex.

You may want to start over and start by creating a test case that registers two parameter types for some quoted strings with the same regex but different types and work your way through from there.

The resolution could be something like

1. Look up a transformer by type hint

2. Fall back to current algorithm.

-Rien

PS; Please separate out the performance improvements into a sperate mr. I feel this PR will be complex enough as it is already.

--
Posting rules: https://cucumber.io/support/posting-rules
---
You received this message because you are subscribed to a topic in the Google Groups "Cukes" group.
To unsubscribe from this topic, visit https://groups.google.com/d/topic/cukes/3LrwR2yndW8/unsubscribe.
To unsubscribe from this group and all its topics, send an email to cukes+un...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/cukes/e154ab49-e540-4898-bacb-7a4d177ba96f%40googlegroups.com.

[Shannon Cole]

Hi Rien,

Sorry for the long delay. I've tried to get a better understanding and

pushed some new changes. These should be more minimal and be

backwards-compatible. Please see:

https://github.com/cucumber/cucumber/pull/918

Also, for anyone else following along, the minor performance stuff

ended up in a PR here:

https://github.com/cucumber/cucumber/pull/906

Cheers,

Shannon