Fix: Define Steps produces incorrect cucumber expression for step definition (#28)

[clrudolphi]

🤔 What's changed?

Modified the set of character escapes conducted within the CucumberExpressionSkeletonProvider such that the default set of characters needing escaping are escaped (such as was done within the Regex skeleton provider.

⚡️ What's your motivation?

This addresses the problem identified in GH28 in which empty parentheses resulted in an improper cucumber expression.

🏷️ What kind of change is this?

• 🐛 Bug fix (non-breaking change which fixes a defect)

♻️ Anything particular you want feedback on?

@gasparnagy please confirm that these character escapes won't cause other problems with Cucumber expressions and they remain compliant with Cucumber syntax and semantics.

📋 Checklist:

• [X ] I've changed the behaviour of the code
  • [ X] I have added/updated tests to cover my changes.
• My change requires a change to the documentation.
  • I have updated the documentation accordingly.
• Users should know about my change
  • [X ] I have added an entry to the "[vNext]" section of the CHANGELOG, linking to this pull request & included my GitHub handle to the release contributors list.

---

This text was originally taken from the template of the Cucumber project, then edited by hand. You can modify the template here.

[clrudolphi]

@gasparnagy please review. My regex skills are not as good as yours.

[gasparnagy]

Thx! Please check my comments and the ref for the cucumber expression syntax specs.

[gasparnagy]

This code supposed to produce valid cucumber expressions and cucumber expressions are different from regex from the escaping perspective.

The cucumber expression escaping is defined at https://github.com/cucumber/cucumber-expressions?tab=readme-ov-file#escaping and it only allows escaping for: {, (, / and \.

Therefore we cannot use Regex.Escape(text) as it uses the regex escaping that is far more greedy than necessary. I think we would just need to fix the original code to match the specs: the .Replace("(", @"\") is clearly buggy, as it should be .Replace("(", @"\(") and the escaping for / is missing (so one more replace).

[clrudolphi]

Thanks for the reference.
Two follow-up questions then:

1. In the original code, lines 31 and 32 both contain a Replace(@"", @"\"). Is that intentional?
2. When escaping the forward slash ('/'), C# doesn't recognize @"/" as a valid escape sequence. I will attempt testing without escaping it.

[clrudolphi]

Re: question 2 above, in my experimentation, I see now that / is meant to convey alternatives and so the use of a / in an example text fed into our snippet provider should treat that as a literal and should be escaped.
Since C# doesn't recognize that as a proper escape sequence, should the snippet provider generate the string as a strictly literal string (with the prepended @ sign)? Such as:

        [Given(@"I have {int} cucumber\(s) in my \/ belly")]

[clrudolphi]

I think I see that number 1 and 2 are related. When I leave line 32 as originally written, the example I'm using in number 2 is generated as

        [Given("I have {int} cucumber\\(s) in my \\/ belly")]

which is handled gracefully by the C# compiler without the prepended @ sign.

[gasparnagy]

@clrudolphi I agree. Let's keep it this way. Switching over to @ strings (verbatim string) just complicates the things.

[gasparnagy]

The proper escaped version of I call the integer.ToString() method would be I call the integer.ToString\() method (the . should not be escaped). See above.

Maybe we can find a better example, so that it shows all escaped characters, like When I use (parenthesis), {curly braces} and/or \ backslash, where the escaped should be When I use \(parenthesis), \{curly braces} and\/or \\ backslash

[clrudolphi]

Is the Regex snippet provider behaving correctly as-is? It does escape the period, which if I'm not mistaken is required because it could otherwise be mistaken for a Regex pattern character. Correct?

[gasparnagy]

Yes, correct.

[gasparnagy]

Thx!