From 388003d6d71e2e5547399133d31b00c76e071be2 Mon Sep 17 00:00:00 2001 From: Vincent Date: Mon, 5 Aug 2019 01:29:45 +0200 Subject: [PATCH] cucumber-expressions: Fix suggestions with in-word parameters (#661) * 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 --- .../go/cucumber_expression_generator_test.go | 128 ++++++++++++++-- .../go/parameter_type_matcher.go | 23 ++- .../CucumberExpressionGenerator.java | 2 +- .../ParameterTypeMatcher.java | 30 +++- .../CucumberExpressionGeneratorTest.java | 143 +++++++++--------- .../javascript/src/ParameterTypeMatcher.ts | 15 +- .../test/CucumberExpressionGeneratorTest.ts | 106 +++++++++++-- .../cucumber_expression_generator.rb | 2 +- .../parameter_type_matcher.rb | 18 ++- .../cucumber_expression_generator_spec.rb | 66 +++++++- 10 files changed, 423 insertions(+), 110 deletions(-) diff --git a/cucumber-expressions/go/cucumber_expression_generator_test.go b/cucumber-expressions/go/cucumber_expression_generator_test.go index 5622045fec..b3ce708c31 100644 --- a/cucumber-expressions/go/cucumber_expression_generator_test.go +++ b/cucumber-expressions/go/cucumber_expression_generator_test.go @@ -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, @@ -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, @@ -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", ) }) @@ -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, @@ -213,10 +213,10 @@ 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) { @@ -224,7 +224,7 @@ func TestCucumberExpressionGeneratory(t *testing.T) { 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, @@ -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) }) @@ -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) { diff --git a/cucumber-expressions/go/parameter_type_matcher.go b/cucumber-expressions/go/parameter_type_matcher.go index 6a89cfa9dc..051bd2572c 100644 --- a/cucumber-expressions/go/parameter_type_matcher.go +++ b/cucumber-expressions/go/parameter_type_matcher.go @@ -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]] } diff --git a/cucumber-expressions/java/src/main/java/io/cucumber/cucumberexpressions/CucumberExpressionGenerator.java b/cucumber-expressions/java/src/main/java/io/cucumber/cucumberexpressions/CucumberExpressionGenerator.java index ec7ed8b322..fc141218ec 100644 --- a/cucumber-expressions/java/src/main/java/io/cucumber/cucumberexpressions/CucumberExpressionGenerator.java +++ b/cucumber-expressions/java/src/main/java/io/cucumber/cucumberexpressions/CucumberExpressionGenerator.java @@ -114,7 +114,7 @@ private static List 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; } diff --git a/cucumber-expressions/java/src/main/java/io/cucumber/cucumberexpressions/ParameterTypeMatcher.java b/cucumber-expressions/java/src/main/java/io/cucumber/cucumberexpressions/ParameterTypeMatcher.java index 426e0a3123..2c9af22a26 100644 --- a/cucumber-expressions/java/src/main/java/io/cucumber/cucumberexpressions/ParameterTypeMatcher.java +++ b/cucumber-expressions/java/src/main/java/io/cucumber/cucumberexpressions/ParameterTypeMatcher.java @@ -1,30 +1,50 @@ package io.cucumber.cucumberexpressions; import java.util.regex.Matcher; +import java.util.regex.Pattern; final class ParameterTypeMatcher implements Comparable { 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(); } diff --git a/cucumber-expressions/java/src/test/java/io/cucumber/cucumberexpressions/CucumberExpressionGeneratorTest.java b/cucumber-expressions/java/src/test/java/io/cucumber/cucumberexpressions/CucumberExpressionGeneratorTest.java index 4c0f0c38a3..8b4e6be37b 100644 --- a/cucumber-expressions/java/src/test/java/io/cucumber/cucumberexpressions/CucumberExpressionGeneratorTest.java +++ b/cucumber-expressions/java/src/test/java/io/cucumber/cucumberexpressions/CucumberExpressionGeneratorTest.java @@ -110,70 +110,102 @@ public Currency transform(String arg) { "I have a EUR account and a GBP account"); } + @Test + public void does_not_suggest_parameter_type_when_surrounded_by_alphanum() { + parameterTypeRegistry.defineParameterType(new ParameterType<>( + "direction", + "(up|down)", + String.class, + new Transformer() { + @Override + public String transform(String arg) { + return arg; + } + }, + true, + false + )); + assertExpression( + "I like muppets", Collections.emptyList(), + "I like muppets"); + } + + @Test + public void does_suggest_parameter_type_when_surrounded_by_space() { + parameterTypeRegistry.defineParameterType(new ParameterType<>( + "direction", + "(up|down)", + String.class, + new Transformer() { + @Override + public String transform(String arg) { + return arg; + } + }, + true, + false + )); + assertExpression( + "it went {direction} and {direction}", asList("direction", "direction2"), + "it went up and down"); + } + @Test public void prefers_leftmost_match_when_there_is_overlap() { parameterTypeRegistry.defineParameterType(new ParameterType<>( - "currency", - "cd", - Currency.class, - new Transformer() { + "right", + "c d", + String.class, + new Transformer() { @Override - public Currency transform(String arg) { - return Currency.getInstance(arg); + public String transform(String arg) { + return arg; } } )); parameterTypeRegistry.defineParameterType(new ParameterType<>( - "date", - "bc", - Date.class, - new Transformer() { + "left", + "b c", + String.class, + new Transformer() { @Override - public Date transform(String arg) { - try { - return df.parse(arg); - } catch (ParseException e) { - throw new RuntimeException(e); - } + public String transform(String arg) { + return arg; } } )); assertExpression( - "a{date}defg", singletonList("date"), - "abcdefg"); + "a {left} d e f g", singletonList("left"), + "a b c d e f g"); } @Test public void prefers_widest_match_when_pos_is_same() { parameterTypeRegistry.defineParameterType(new ParameterType<>( - "currency", - "cd", - Currency.class, - new Transformer() { + "airport", + "[A-Z]{3}", + String.class, + new Transformer() { @Override - public Currency transform(String arg) { - return Currency.getInstance(arg); + public String transform(String arg) { + return arg; } } )); parameterTypeRegistry.defineParameterType(new ParameterType<>( - "date", - "cde", - Date.class, - new Transformer() { + "leg", + "[A-Z]{3}-[A-Z]{3}", + String.class, + new Transformer() { @Override - public Date transform(String arg) { - try { - return df.parse(arg); - } catch (ParseException e) { - throw new RuntimeException(e); - } + public String transform(String arg) { + return arg; } } )); assertExpression( - "ab{date}fg", singletonList("date"), - "abcdefg"); + "leg {leg}", singletonList("leg"), + "leg LHR-CDG"); } @Test @@ -234,43 +266,6 @@ public void exposes_transforms_in_generated_expression() { assertEquals(Double.class, generatedExpression.getParameterTypes().get(1).getType()); } - @Test - public void matches_parameter_types_with_optional_capture_groups() { - ParameterType optionalFlight = new ParameterType<>( - "optional-flight", - "(1st flight)?", - String.class, - new Transformer() { - @Override - public String transform(String arg) { - return arg; - } - }, - true, - false - ); - ParameterType optionalHotel = new ParameterType<>( - "optional-hotel", - "(1st hotel)?", - String.class, - new Transformer() { - @Override - public String transform(String arg) { - return arg; - } - }, - true, - false - ); - - parameterTypeRegistry.defineParameterType(optionalFlight); - parameterTypeRegistry.defineParameterType(optionalHotel); - // 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}. - List generatedExpressions = generator.generateExpressions("I reach Stage4: 1st flight-1st hotel"); - assertEquals("I reach Stage{int}: {optional-flight}{int}st hotel", generatedExpressions.get(0).getSource()); - } - @Test public void generates_at_most_256_expressions() { for (int i = 0; i < 4; i++) { @@ -291,7 +286,7 @@ public String transform(String arg) { } // This would otherwise generate 4^11=419430 expressions and consume just shy of 1.5GB. - assertEquals(256, generator.generateExpressions("a simple step").size()); + assertEquals(256, generator.generateExpressions("a b c d e f g h i j k").size()); } @Test diff --git a/cucumber-expressions/javascript/src/ParameterTypeMatcher.ts b/cucumber-expressions/javascript/src/ParameterTypeMatcher.ts index c0a78c9bcb..fca7f9c348 100644 --- a/cucumber-expressions/javascript/src/ParameterTypeMatcher.ts +++ b/cucumber-expressions/javascript/src/ParameterTypeMatcher.ts @@ -40,13 +40,26 @@ export default class ParameterTypeMatcher { } get find() { - return this.match && this.group !== '' + return this.match && this.group !== '' && this.full_word } get start() { return this.matchPosition + this.match.index } + get full_word() { + return this.match_start_word && this.match_end_word + } + + get match_start_word() { + return this.start == 0 || this.text[this.start - 1].match(/\s|\p{P}/u) + } + + get match_end_word() { + const next_character_index = this.start + this.group.length + return next_character_index === this.text.length || this.text[next_character_index].match(/\s|\p{P}/u) + } + get group() { return this.match[0] } diff --git a/cucumber-expressions/javascript/test/CucumberExpressionGeneratorTest.ts b/cucumber-expressions/javascript/test/CucumberExpressionGeneratorTest.ts index 900df93b3e..ade4a1374b 100644 --- a/cucumber-expressions/javascript/test/CucumberExpressionGeneratorTest.ts +++ b/cucumber-expressions/javascript/test/CucumberExpressionGeneratorTest.ts @@ -132,7 +132,7 @@ describe('CucumberExpressionGenerator', () => { parameterTypeRegistry.defineParameterType( new ParameterType( 'currency', - /cd/, + /c d/, Currency, s => new Currency(s), true, @@ -140,10 +140,10 @@ describe('CucumberExpressionGenerator', () => { ) ) parameterTypeRegistry.defineParameterType( - new ParameterType('date', /bc/, Date, s => new Date(s), true, false) + new ParameterType('date', /b c/, Date, s => new Date(s), true, false) ) - assertExpression('a{date}defg', ['date'], 'abcdefg') + assertExpression('a {date} d e f g', ['date'], 'a b c d e f g') }) // TODO: prefers widest match @@ -201,7 +201,7 @@ describe('CucumberExpressionGenerator', () => { parameterTypeRegistry.defineParameterType( new ParameterType( 'optional-hotel', - /(1st hotel)?/, + /(1 hotel)?/, null, s => s, true, @@ -210,24 +210,24 @@ describe('CucumberExpressionGenerator', () => { ) const expression = generator.generateExpressions( - 'I reach Stage4: 1st flight-1st hotel' + '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 + // 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}. assert.strictEqual( expression.source, - 'I reach Stage{int}: {optional-flight}{int}st hotel' + 'I reach Stage {int}: {optional-flight} {int} hotel' ) }) it('generates at most 256 expressions', () => { for (let i = 0; i < 4; i++) { parameterTypeRegistry.defineParameterType( - new ParameterType('my-type-' + i, /[a-z]/, null, s => s, true, false) + new ParameterType('my-type-' + i, /([a-z] )*?[a-z]/, null, s => s, true, false) ) } // This would otherwise generate 4^11=419430 expressions and consume just shy of 1.5GB. - const expressions = generator.generateExpressions('a simple step') + const expressions = generator.generateExpressions('a s i m p l e s t e p') assert.strictEqual(expressions.length, 256) }) @@ -250,4 +250,92 @@ describe('CucumberExpressionGenerator', () => { '{zero-or-more} {zero-or-more} {zero-or-more}' ) }) + + it('does not suggest parameter included at the beginning of a word', () => { + parameterTypeRegistry.defineParameterType( + new ParameterType('direction', /(up|down)/, null, s => s, true, false) + ) + + const expressions = generator.generateExpressions('I download a picture') + assert.strictEqual(expressions.length, 1) + assert.notEqual( + expressions[0].source, + 'I {direction}load a picture' + ) + assert.strictEqual( + expressions[0].source, + 'I download a picture' + ) + }) + + it('does not suggest parameter included inside a word', () => { + parameterTypeRegistry.defineParameterType( + new ParameterType('direction', /(up|down)/, null, s => s, true, false) + ) + + const expressions = generator.generateExpressions('I watch the muppet show') + assert.strictEqual(expressions.length, 1) + assert.notEqual( + expressions[0].source, + 'I watch the m{direction}pet show' + ) + assert.strictEqual( + expressions[0].source, + 'I watch the muppet show' + ) + }) + + it('does not suggest parameter at the end of a word', () => { + parameterTypeRegistry.defineParameterType( + new ParameterType('direction', /(up|down)/, null, s => s, true, false) + ) + + const expressions = generator.generateExpressions('I create a group') + assert.strictEqual(expressions.length, 1) + assert.notEqual( + expressions[0].source, + 'I create a gro{direction}' + ) + assert.strictEqual( + expressions[0].source, + 'I create a group' + ) + }) + + it('does suggest parameter that are a full word', () => { + parameterTypeRegistry.defineParameterType( + new ParameterType('direction', /(up|down)/, null, s => s, true, false) + ) + + assert.strictEqual( + generator.generateExpressions("When I go down the road")[0].source, + "When I go {direction} the road" + ) + + assert.strictEqual( + generator.generateExpressions("When I walk up the hill")[0].source, + "When I walk {direction} the hill" + ) + + assert.strictEqual( + generator.generateExpressions("up the hill, the road goes down")[0].source, + "{direction} the hill, the road goes {direction}" + ) + }) + + it('does not consider punctuation as being part of a word', () => { + parameterTypeRegistry.defineParameterType( + new ParameterType('direction', /(up|down)/, null, s => s, true, false) + ) + + assert.strictEqual( + generator.generateExpressions("direction is:down")[0].source, + "direction is:{direction}" + ) + + assert.strictEqual( + generator.generateExpressions("direction is down.")[0].source, + "direction is {direction}." + ) + }) }) diff --git a/cucumber-expressions/ruby/lib/cucumber/cucumber_expressions/cucumber_expression_generator.rb b/cucumber-expressions/ruby/lib/cucumber/cucumber_expressions/cucumber_expression_generator.rb index 4c5b9823f2..b161584fe0 100644 --- a/cucumber-expressions/ruby/lib/cucumber/cucumber_expressions/cucumber_expression_generator.rb +++ b/cucumber-expressions/ruby/lib/cucumber/cucumber_expressions/cucumber_expression_generator.rb @@ -89,7 +89,7 @@ def create_parameter_type_matchers2(parameter_type, text) regexps = parameter_type.regexps regexps.each do |regexp| regexp = Regexp.new("(#{regexp})") - result.push(ParameterTypeMatcher.new(parameter_type, regexp, text)) + result.push(ParameterTypeMatcher.new(parameter_type, regexp, text, 0)) end result end diff --git a/cucumber-expressions/ruby/lib/cucumber/cucumber_expressions/parameter_type_matcher.rb b/cucumber-expressions/ruby/lib/cucumber/cucumber_expressions/parameter_type_matcher.rb index cb63ea4de1..5322ba781d 100644 --- a/cucumber-expressions/ruby/lib/cucumber/cucumber_expressions/parameter_type_matcher.rb +++ b/cucumber-expressions/ruby/lib/cucumber/cucumber_expressions/parameter_type_matcher.rb @@ -11,7 +11,7 @@ def initialize(parameter_type, regexp, text, match_position=0) def advance_to(new_match_position) (new_match_position...@text.length).each {|advancedPos| matcher = self.class.new(parameter_type, @regexp, @text, advancedPos) - if matcher.find + if matcher.find && matcher.full_word? return matcher end } @@ -23,6 +23,10 @@ def find !@match.nil? && !group.empty? end + def full_word? + space_before_match_or_sentence_start? && space_after_match_or_sentence_end? + end + def start @match.begin(0) end @@ -38,6 +42,18 @@ def <=>(other) return length_comparison if length_comparison != 0 0 end + + private + + def space_before_match_or_sentence_start? + match_begin = @match.begin(0) + match_begin == 0 || @text[match_begin - 1].match(/\s|\p{P}/) + end + + def space_after_match_or_sentence_end? + match_end = @match.end(0) + match_end == @text.length || @text[match_end].match(/\s|\p{P}/) + end end end end diff --git a/cucumber-expressions/ruby/spec/cucumber/cucumber_expressions/cucumber_expression_generator_spec.rb b/cucumber-expressions/ruby/spec/cucumber/cucumber_expressions/cucumber_expression_generator_spec.rb index e66cab3f5b..1865391c83 100644 --- a/cucumber-expressions/ruby/spec/cucumber/cucumber_expressions/cucumber_expression_generator_spec.rb +++ b/cucumber-expressions/ruby/spec/cucumber/cucumber_expressions/cucumber_expression_generator_spec.rb @@ -109,32 +109,32 @@ class Currency )) @parameter_type_registry.define_parameter_type(ParameterType.new( 'optional-hotel', - /(1st hotel)?/, + /(1 hotel)?/, String, lambda {|s| s}, true, false )) - expression = @generator.generate_expressions("I reach Stage4: 1st flight-1st hotel")[0] - # While you would expect this to be `I reach Stage{int}: {optional-flight}-{optional-hotel}` + expression = @generator.generate_expressions("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}. - expect(expression.source).to eq("I reach Stage{int}: {optional-flight}{int}st hotel") + expect(expression.source).to eq("I reach Stage {int}: {optional-flight} {int} hotel") end it "generates at most 256 expressions" do for i in 0..3 @parameter_type_registry.define_parameter_type(ParameterType.new( "my-type-#{i}", - /[a-z]/, + /([a-z] )*?[a-z]/, String, lambda {|s| s}, true, false )) end - # This would otherwise generate 4^11=419430 expressions and consume just shy of 1.5GB. - expressions = @generator.generate_expressions("a simple step") + # This would otherwise generate 4^11=4194300 expressions and consume just shy of 1.5GB. + expressions = @generator.generate_expressions("a s i m p l e s t e p") expect(expressions.length).to eq(256) end @@ -162,6 +162,58 @@ class Currency expect(expressions[1].source).to eq("{zero-or-more} {zero-or-more} {zero-or-more}") end + context "does not suggest parameter when match is" do + before do + @parameter_type_registry.define_parameter_type(ParameterType.new( + 'direction', + /(up|down)/, + String, + lambda {|s| s}, + true, + false + )) + end + + it "at the beginning of a word" do + expect(@generator.generate_expression("When I download a picture").source).not_to eq("When I {direction}load a picture") + expect(@generator.generate_expression("When I download a picture").source).to eq("When I download a picture") + end + + it "inside a word" do + expect(@generator.generate_expression("When I watch the muppet show").source).not_to eq("When I watch the m{direction}pet show") + expect(@generator.generate_expression("When I watch the muppet show").source).to eq("When I watch the muppet show") + end + + it "at the end of a word" do + expect(@generator.generate_expression("When I create a group").source).not_to eq("When I create a gro{direction}") + expect(@generator.generate_expression("When I create a group").source).to eq("When I create a group") + end + end + + context "does suggest parameter when match is" do + before do + @parameter_type_registry.define_parameter_type(ParameterType.new( + 'direction', + /(up|down)/, + String, + lambda {|s| s}, + true, + false + )) + end + + it "a full word" do + expect(@generator.generate_expression("When I go down the road").source).to eq("When I go {direction} the road") + expect(@generator.generate_expression("When I walk up the hill").source).to eq("When I walk {direction} the hill") + expect(@generator.generate_expression("up the hill, the road goes down").source).to eq("{direction} the hill, the road goes {direction}") + end + + it 'wrapped around punctuation characters' do + expect(@generator.generate_expression("When direction is:down").source).to eq("When direction is:{direction}") + expect(@generator.generate_expression("Then direction is down.").source).to eq("Then direction is {direction}.") + end + end + def assert_expression(expected_expression, expected_argument_names, text) generated_expression = @generator.generate_expression(text) expect(generated_expression.parameter_names).to eq(expected_argument_names)