Skip to content

Commit

Permalink
cucumber-expressions: Fix suggestions with in-word parameters (#661)
Browse files Browse the repository at this point in the history
* Add tests describing issue #657

* Force ParameterTypeMatcher to match full words

* Enable choosing it custom ParameterType must match a full word or not

 - default is true
 - default parameters do not force matching a word (to be discussed)
 - updated some tests so they keep passing

* Take into account punctuation when suggesting parameters

* Java: Don't suggest parameter unless at beginning/end or surrounded by space/punctuation

Also: Remove an obsolete test, update other tests and improve float/double matching

* Remove "force_full_word" parameter when creating ParameterType

Also use \p{P} instead of [[:punct]] to have the same implementation across multiple languages
Update tests so they still reflect expected behavior even with word matching

* Port tests from cucumber-expressions/ruby to reflect issue #657

* Only propose full words as parameters in cucumber-expressions/javascript

Also adapt tests so they reflect those changes

* Fixed rspec organization so the test name make sense

* Port tests from cucumber-expressions/ruby to cucumber-expressions/go to reflect issue #657

* Do not suggest in-word parameters in cucumber-expressions/go

Also update test to reflect the changes

* Apply @luke-hill review for a more ruby-ish code

* Undo changes unrelated to matching words

* Extract groupMatchesFullWord method for clarity
  • Loading branch information
vincent-psarga authored and aslakhellesoy committed Aug 4, 2019
1 parent 9b51d46 commit 388003d
Show file tree
Hide file tree
Showing 10 changed files with 423 additions and 110 deletions.
128 changes: 118 additions & 10 deletions cucumber-expressions/go/cucumber_expression_generator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ func TestCucumberExpressionGeneratory(t *testing.T) {
parameterTypeRegistry := NewParameterTypeRegistry()
parameterType1, err := NewParameterType(
"type1",
[]*regexp.Regexp{regexp.MustCompile("cd")},
[]*regexp.Regexp{regexp.MustCompile("c d")},
"type1",
nil,
true,
Expand All @@ -119,7 +119,7 @@ func TestCucumberExpressionGeneratory(t *testing.T) {
require.NoError(t, parameterTypeRegistry.DefineParameterType(parameterType1))
parameterType2, err := NewParameterType(
"type2",
[]*regexp.Regexp{regexp.MustCompile("bc")},
[]*regexp.Regexp{regexp.MustCompile("b c")},
"type2",
nil,
true,
Expand All @@ -131,9 +131,9 @@ func TestCucumberExpressionGeneratory(t *testing.T) {
assertExpressionWithParameterTypeRegistry(
t,
parameterTypeRegistry,
"a{type2}defg",
"a {type2} d e f g",
[]string{"type2"},
"abcdefg",
"a b c d e f g",
)
})

Expand Down Expand Up @@ -204,7 +204,7 @@ func TestCucumberExpressionGeneratory(t *testing.T) {
require.NoError(t, parameterTypeRegistry.DefineParameterType(optionalFlightParameterType))
optionalHotelParameterType, err := NewParameterType(
"optional-hotel",
[]*regexp.Regexp{regexp.MustCompile("(1st hotel)?")},
[]*regexp.Regexp{regexp.MustCompile("(1 hotel)?")},
"optional-hotel",
nil,
true,
Expand All @@ -213,18 +213,18 @@ func TestCucumberExpressionGeneratory(t *testing.T) {
require.NoError(t, err)
require.NoError(t, parameterTypeRegistry.DefineParameterType(optionalHotelParameterType))
generator := NewCucumberExpressionGenerator(parameterTypeRegistry)
generatedExpression := generator.GenerateExpressions("I reach Stage4: 1st flight-1st hotel")[0]
// While you would expect this to be `I reach Stage{int}: {optional-flight}-{optional-hotel}`
generatedExpression := generator.GenerateExpressions("I reach Stage 4: 1st flight -1 hotel")[0]
// While you would expect this to be `I reach Stage {int}: {optional-flight} -{optional-hotel}`
// the `-1` causes {int} to match just before {optional-hotel}.
require.Equal(t, generatedExpression.Source(), "I reach Stage{int}: {optional-flight}{int}st hotel")
require.Equal(t, generatedExpression.Source(), "I reach Stage {int}: {optional-flight} {int} hotel")
})

t.Run("generates at most 256 expressions", func(t *testing.T) {
parameterTypeRegistry := NewParameterTypeRegistry()
for i := 1; i <= 4; i++ {
myType, err := NewParameterType(
"my-type-"+string(i),
[]*regexp.Regexp{regexp.MustCompile("[a-z]")},
[]*regexp.Regexp{regexp.MustCompile("([a-z] )*?[a-z]")},
"string",
nil,
true,
Expand All @@ -236,7 +236,7 @@ func TestCucumberExpressionGeneratory(t *testing.T) {

generator := NewCucumberExpressionGenerator(parameterTypeRegistry)
// This would otherwise generate 4^11=419430 expressions and consume just shy of 1.5GB.
generatedExpressions := generator.GenerateExpressions("a simple step")
generatedExpressions := generator.GenerateExpressions("a s i m p l e s t e p")
require.Equal(t, len(generatedExpressions), 256)
})

Expand Down Expand Up @@ -269,6 +269,114 @@ func TestCucumberExpressionGeneratory(t *testing.T) {
require.Equal(t, generatedExpressions[0].Source(), "{exactly-one} {zero-or-more} {zero-or-more}")
require.Equal(t, generatedExpressions[1].Source(), "{zero-or-more} {zero-or-more} {zero-or-more}")
})

t.Run("does not suggest parameter when match is at the beginning of a word", func(t *testing.T) {
parameterTypeRegistry := NewParameterTypeRegistry()
direction, err := NewParameterType(
"direction",
[]*regexp.Regexp{regexp.MustCompile("(up|down)")},
"string",
nil,
true,
false,
)
require.NoError(t, err)
require.NoError(t, parameterTypeRegistry.DefineParameterType(direction))

generator := NewCucumberExpressionGenerator(parameterTypeRegistry)
generatedExpressions := generator.GenerateExpressions("I download a picture")

require.Equal(t, len(generatedExpressions), 1)
require.Equal(t, generatedExpressions[0].Source(), "I download a picture")
})

t.Run("does not suggest parameter when match is inside a word", func(t *testing.T) {
parameterTypeRegistry := NewParameterTypeRegistry()
direction, err := NewParameterType(
"direction",
[]*regexp.Regexp{regexp.MustCompile("(up|down)")},
"string",
nil,
true,
false,
)
require.NoError(t, err)
require.NoError(t, parameterTypeRegistry.DefineParameterType(direction))

generator := NewCucumberExpressionGenerator(parameterTypeRegistry)
generatedExpressions := generator.GenerateExpressions("I watch the muppet show")

require.Equal(t, len(generatedExpressions), 1)
require.Equal(t, generatedExpressions[0].Source(), "I watch the muppet show")
})

t.Run("does not suggest parameter when match is at the end of a word", func(t *testing.T) {
parameterTypeRegistry := NewParameterTypeRegistry()
direction, err := NewParameterType(
"direction",
[]*regexp.Regexp{regexp.MustCompile("(up|down)")},
"string",
nil,
true,
false,
)
require.NoError(t, err)
require.NoError(t, parameterTypeRegistry.DefineParameterType(direction))

generator := NewCucumberExpressionGenerator(parameterTypeRegistry)
generatedExpressions := generator.GenerateExpressions("I create a group")

require.Equal(t, len(generatedExpressions), 1)
require.Equal(t, generatedExpressions[0].Source(), "I create a group")
})

t.Run("does suggest parameter when match is a full word", func(t *testing.T) {
parameterTypeRegistry := NewParameterTypeRegistry()
direction, err := NewParameterType(
"direction",
[]*regexp.Regexp{regexp.MustCompile("(up|down)")},
"string",
nil,
true,
false,
)
require.NoError(t, err)
require.NoError(t, parameterTypeRegistry.DefineParameterType(direction))

generator := NewCucumberExpressionGenerator(parameterTypeRegistry)

generatedExpressions1 := generator.GenerateExpressions("I go down the road")
require.Equal(t, len(generatedExpressions1), 1)
require.Equal(t, generatedExpressions1[0].Source(), "I go {direction} the road")

generatedExpressions2 := generator.GenerateExpressions("up the hill, the road goes down")
require.Equal(t, len(generatedExpressions2), 1)
require.Equal(t, generatedExpressions2[0].Source(), "{direction} the hill, the road goes {direction}")
})

t.Run("does suggest parameter when match is wrapped around punctuation characters", func(t *testing.T) {
parameterTypeRegistry := NewParameterTypeRegistry()
direction, err := NewParameterType(
"direction",
[]*regexp.Regexp{regexp.MustCompile("(up|down)")},
"string",
nil,
true,
false,
)
require.NoError(t, err)
require.NoError(t, parameterTypeRegistry.DefineParameterType(direction))

generator := NewCucumberExpressionGenerator(parameterTypeRegistry)

generatedExpressions1 := generator.GenerateExpressions("direction is:down")
require.Equal(t, len(generatedExpressions1), 1)
require.Equal(t, generatedExpressions1[0].Source(), "direction is:{direction}")

generatedExpressions2 := generator.GenerateExpressions("direction is down.")
require.Equal(t, len(generatedExpressions2), 1)
require.Equal(t, generatedExpressions2[0].Source(), "direction is {direction}.")
})
}

func assertExpression(t *testing.T, expectedExpression string, expectedArgumentNames []string, text string) {
Expand Down
23 changes: 22 additions & 1 deletion cucumber-expressions/go/parameter_type_matcher.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,13 +39,34 @@ func (p *ParameterTypeMatcher) AdvanceTo(newMatchPosition int) *ParameterTypeMat
}

func (p *ParameterTypeMatcher) Find() bool {
return p.match != nil && p.Group() != ""
return p.match != nil && p.FullWord() && p.Group() != ""
}

func (p *ParameterTypeMatcher) FullWord() bool {
return p.MatchStartWord() && p.MatchEndWord()
}

func (p *ParameterTypeMatcher) MatchStartWord() bool {
return p.Start() == 0 || p.CharacterIsWordDelimiter(p.Start()-1)
}

func (p *ParameterTypeMatcher) MatchEndWord() bool {
return p.End() == len(p.text) || p.CharacterIsWordDelimiter(p.End())
}

func (p *ParameterTypeMatcher) CharacterIsWordDelimiter(index int) bool {
matched, _ := regexp.MatchString(`\s|\p{P}`, p.text[index:index+1])
return matched
}

func (p *ParameterTypeMatcher) Start() int {
return p.matchPosition + p.match[0]
}

func (p *ParameterTypeMatcher) End() int {
return p.Start() + len(p.Group())
}

func (p *ParameterTypeMatcher) Group() string {
return p.text[p.matchPosition:][p.match[0]:p.match[1]]
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ private static List<ParameterTypeMatcher> createParameterTypeMatchers(ParameterT
for (String captureGroupRegexp : captureGroupRegexps) {
Pattern regexp = Pattern.compile("(" + captureGroupRegexp + ")");
Matcher matcher = regexp.matcher(text);
result.add(new ParameterTypeMatcher(parameterType, matcher, text.length()));
result.add(new ParameterTypeMatcher(parameterType, matcher, text));
}
return result;
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,30 +1,50 @@
package io.cucumber.cucumberexpressions;

import java.util.regex.Matcher;
import java.util.regex.Pattern;

final class ParameterTypeMatcher implements Comparable<ParameterTypeMatcher> {
private final ParameterType<?> parameterType;
private final Matcher matcher;
private final int textLength;
private final String text;

ParameterTypeMatcher(ParameterType<?> parameterType, Matcher matcher, int textLength) {
ParameterTypeMatcher(ParameterType<?> parameterType, Matcher matcher, String text) {
this.parameterType = parameterType;
this.matcher = matcher;
this.textLength = textLength;
this.text = text;
}

private static boolean isWhitespaceOrPunctuation(char c) {
return Pattern.matches("[\\s\\p{P}]", new String(new char[]{c}));
}

boolean advanceToAndFind(int newMatchPos) {
// Unlike js, ruby and go, the matcher is stateful
// so we can't use the immutable semantics.
matcher.region(newMatchPos, textLength);
matcher.region(newMatchPos, text.length());
while (matcher.find()) {
if (!group().isEmpty()) {
if (!group().isEmpty() && groupMatchesFullWord()) {
return true;
}
}
return false;
}

private boolean groupMatchesFullWord() {
if (matcher.start() > 0) {
char before = text.charAt(matcher.start() - 1);
if (!isWhitespaceOrPunctuation(before)) {
return false;
}
}

if (matcher.end() < text.length()) {
char after = text.charAt(matcher.end());
return isWhitespaceOrPunctuation(after);
}
return true;
}

int start() {
return matcher.start();
}
Expand Down
Loading

0 comments on commit 388003d

Please sign in to comment.