have conversion does not work properly with Enumerator instances

[myronmarston]

I had a part of a spec like:

expect(some_enumerator).to have(2).things

...which converted to:

expect(some_enumerator.size).to eq(2)

...which failed with:

     NoMethodError:
       undefined method `size' for #<Enumerator:0x007f8a41825758>

To fix it, I changed size to count. Would be nice if transpec would handle this automatically if possible (not sure how much work that would be).

[yujinakayama]

It seems Enumerator#size is available since Ruby 2.0. Actually Transpec investigates what methods the subject responds to in dynamic analysis. I guess you ran Transpec on Ruby 2.0 or 2.1, and after the conversion you ran the spec on Ruby 1.8 or 1.9.

[myronmarston]

This is the same root cause as #44: I passed -s to skip the dynamic analysis, so transpec had no way to know it was an enumerator.

Still, given that Enumerator has a count method on all ruby versions, and (as far as I know), all other common collection classes have a count method as well, do you think it makes sense for it to use count when not doing dynamic analysis since I believe that is the most commonly supported method? Another case where count is better than size is with an activerecord relation: count will just run the count query, where as I think size or length will basically be all.size or all.length, causing it to fetch all records first, which will perform much worse.

[yujinakayama]

do you think it makes sense for it to use count when not doing dynamic analysis since I believe that is the most commonly supported method?

Sounds good to me.

Or now I'm wondering if Transpec should warn (and maybe skip the conversion) when it does not have enough runtime information for some conversions, rather than converting aggressively as it currently does.

[myronmarston]

Or now I'm wondering Transpec should warn (and maybe skip the conversion) when it does not have enough runtime information for some conversions, rather than converting aggressively as Transpec currently does.

Hmm. I can see an argument for that, but it seems like usually if it converts wrongly you wind up with a failure rather than a false positive (although maybe that's just luck). I think skipping the conversion (unless it absolutely knows it can't convert properly) would be a little annoying -- it's still the user's responsibility to review the diff and they have source control to manage reversions and what not, but if there were a large number of conversions transpec could do that were correct but it wasn't sure about them and it skipped them, that would be frustrating to the user.

What would be nice is if transpec's output gave you a list of things (file and line numbers) it was less sure about that it recommends you pay special attention to when you review.

[yujinakayama]

but if there were a large number of conversions transpec could do that were correct but it wasn't sure about them and it skipped them, that would be frustrating to the user.

What would be nice is if transpec's output gave you a list of things (file and line numbers) it was less sure about that it recommends you pay special attention to when you review.

It makes sense.

By the way, now I remember the reason why I chose size rather than count as the default method. It's because String#count is not a method that returns a collection size, and also have(n).items matcher uses size as the default method. So I think I'm going to keep size the default and get users to pay attention to the unsure conversions by outputting warnings. Anyway, there's no perfect way to handle have(n).items matcher without dynamic analysis.

[myronmarston]

It's because String#count is not a method that returns a collection size

Ah, I didn't think about String#count. That said, String isn't a "normal" enumerable, since you need to either decide to look at its bytes or its chars and the count of these two can (and generally is, unless it's an ASCII string in ASCII or UTF-8 encoding) be different.

Anyhow, I'm fine keeping size as the default. Thanks!