Replace string eval

[cupakromer]

First pass at replacing all uses of string eval per #267

I've tested on 1.8.7-p374, 1.9.3-p392, and 2.0.0-p247.

One potential issue is that I made the decision to use Module#class_exec which was only added in 1.8.7 and 1.9. I did this for two reasons:

1. It only supports blocks (not possible to pass strings)
2. It provides the ability to directly pass instance variables into the evaluation block, instead of having to exploit the block closure with local variables:

# Using class_exec
@klass.class_exec(@method_name) do |method_name|
  define_method(method_name) do
    # ...
  end
end

# Using class_eval
method_name = @method_name
@klass.class_eval do
  define_method(method_name) do
    # ...
  end
end

This will break 1.8.6 support. Let me know what you think.

[coveralls]

Coverage increased (+0%) when pulling d3ef19c on cupakromer:replace-string-eval into 6ef6f11 on rspec:master.

[fables-tales]

@cupakromer thanks for this PR! I don't get why this raise_error got more specific in something related to changing evals to define_method type situations. What's going on here?

[cupakromer]

There was some interpolation going on in https://github.com/rspec/rspec-mocks/pull/340/files#L4R16. Without the more specific spec, I wasn't sure if my change to the method_name interpolation was correct.

[JonRowe]

Given the drop of the .inspect call I'm ok with this change to the spec

[myronmarston]

First pass at replacing all uses of string eval per #267

I've tested on 1.8.7-p374, 1.9.3-p392, and 2.0.0-p247.

One potential issue is that I made the decision to use Module#class_exec which was only added in 1.8.7 and 1.9. I did this for two reasons:

It only supports blocks (not possible to pass strings)
It provides the ability to directly pass instance variables into the evaluation block, instead of having to exploit the block closure with local variables:

Using class_exec

@klass.class_exec(@method_name) do |method_name|
define_method(method_name) do
# ...
end
end

Using class_eval

method_name = @method_name
@klass.class_eval do
define_method(method_name) do
# ...
end
end
This will break 1.8.6 support. Let me know what you think.

Breaking 1.8.6 support is fine in master. We're dropping support in RSpec 3.0.

As for the class_exec idiom you used here...it's not one I'm used to seeing. It looks odd to me at first; I tend to use the class_eval idiom you listed instead. I commented on the changes before reading your summary and seeing you explained this. I think after reading your explanation, I'm fine keeping the class_exec.

[coveralls]

Coverage increased (+0%) when pulling 7569e3b on cupakromer:replace-string-eval into 6ef6f11 on rspec:master.

[JonRowe]

(Sorry repeating from line comment in case it gets lost)

We've already dropped support for 1.8.6 and even 2.14 doesn't work 100% with it so I don't think it's an issue with respect to this. I'm leaning towards preferring _exec given that by being explicitly we're limiting the amount of leakage than occur from using the closures.

[JonRowe]

Is the to_sym necessary here?

[cupakromer]

👍 Nope, your'e right. I now see it was necessary in the original due to the string interpolation (symbols are converted to strings), so the colon needed to be added back. Will remove.

[JonRowe]

Good work, I thought I'd caught most of these on my first pass through removing evals, guess not :)

❤️ ❤️ ❤️

[coveralls]

Coverage remained the same when pulling 63fd8c4 on cupakromer:replace-string-eval into 6ef6f11 on rspec:master.