Skip to content
This repository

Cucumber is generating Ruby step definition snippets that cause Ruby warnings. #328

Closed
martco opened this Issue October 12, 2012 · 10 comments

3 participants

martco Oleg Sukhodolsky Matt Wynne
martco

Steps to Recreate

  1. If I save a file called addition.feature that has the following line inside of it:

    Given I have entered 50 into the calculator
    
  2. And at the command line I run:

    cucumber features/addition.feature
    
  3. Cucumber will tell me that I haven’t implemented the step defintion, and it provides me with a snippet to move me along.

    You can implement step definitions for undefined steps with these snippets:
    
    Given /^I have entered (.*) into the calculator$/ do |n|
      pending # express the regexp above with the code you wish you had         
    end
    

    This is really useful

  4. However, when I check the ruby syntax of features/addition.feature with warnings on:

    ruby -cw features/addition.feature
    

    I get the following warning:

    features/addition.feature:1: warning: ambiguous first argument; put parentheses or even spaces
    
  5. And whenever I save a file that has many Cucumber-generated snippets inside of it, I see this in my vim Quickfix window (which is checking syntax with warnings on):

    This is alarming

Who Cares

One response to this might be, “Who cares? Not my problem, if you don’t want to see the warnings, turn off verbose mode. Verbose mode is broken.”

I might agree with that while writing closed-source stuff, but I disagree when considering open source software. As @mislav writes (emphasis mine):

If you’re a developer of open source code you should regularly check that your code doesn’t generate ruby warnings. If you accomplish this, you allow users of your code to use it in their projects with the verbose mode on and not get warned by potential threats in your codebase.

Addressing the Issue

The ruby warning is happening because the first / of the Cucumber-generated snippet could signify either a division or a regular expression. I’ve tested a fix that wraps the regular expression in parentheses and the warning is gone. This could happen in-line or perhaps could be activated by a setting. I’ve got a pull request lined-up, but wanted to check here first for initial feedback or thoughts.

Thanks. I think the snippet-generation is really useful, and I would love to continue using it with, at the very least, an option to generate ruby-safe snippets.

Matt Wynne
Owner

So two options here:

1) put brackets around the regexp

Given(/foo/) do
  # whatever
end

2) use another way of marking up the regexp

Given %r{foo} do
  # whatever
end

3) who cares

I care. I think I like option 1 best. What do other people think?

Matt Wynne
Owner

Oh wait that's three options. :)

martco

I like 1 because:

  1. I've been using it locally and it works fine
  2. It's what the ruby warning suggests
  3. It has the least impact on the code
martco

but actually, now that I consider it further, i think the %r { ... } literal is a better choice than regex with parens:

%r literal Regex with parens
%r{Lorem ipsum} (/^Lorem ipsum$/)

It seems more legible and should still address the issue.

Matt Wynne
Owner

Are those two examples the same? Are the anchors not required with the %r{} form?

martco

They're not the same. I tested earlier and was going to mention that. I tried using %r{} and was getting ambiguous mismatch errors. I think the first option is best.

martco

To better answer your question, I didn't use the anchors when testing. So right now that would lead me to believe that they are necessary.

martco

@mattwynne i opened a pull request here

Matt Wynne
Owner
Oleg Sukhodolsky
Collaborator

Fixed by PR #331

Oleg Sukhodolsky os97673 closed this February 18, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.