Exceptions thrown when running with Coverage enabled

Great investigation!

I had not considered the case of a file with no lines; it should indeed initialize to at least an empty array, so future array dereferences don't cause NPE.

The other one is definitely more confusing, for the reasons you state: I wouldn't expect the parser to produce negative line numbers.

I'll have a look at your patch. Without knowing why the parser produces negative line numbers, masking the issue might be the simplest fix. It's possible that bad line numbers are being fed to ERB processing somehow, or those negative line numbers might actually be expected. It may be that we should not be doing coverage processing at all for eval-related parses that don't have an explicit file, but I (or someone) would have to research to see if that's the case.

Here's a similar patch that ignores negative line numbers and always adds at least an empty Integer[] for coverage lines. I'm not certain this is the right approach, but it would fix the issues you saw.

https://gist.github.com/3507988

Let me know what you think of this.

WRT to the empty file case, I think always inserting an empty array is right; MRI appears to add an array of length == file line count whether there's > 0 lines or not.

The other case...I'm not sure.

Thanks Charles. I've tested your patch and it works for my app, and I agree that looks like the way to go.

I've been wandering through the parser code, but haven't made a huge amount of progress. It seems that the negative line numbers are stemming from ERB calling eval() on the code it produces from .erb files, but I haven't made it much further than that. I'll have another look tomorrow and see if I can dig anything else up.

Thanks again for the quick fix

Mark

Obviously by "tomorrow" I meant "right now"... starting from the parser and working backwards, I think the negative line numbers can be explained by this bit from tilt-1.3.3/lib/tilt/template.rb:

    # JRuby doesn't allow Object#instance_eval to yield to the block it's
    # closed over. This is by design and (ostensibly) something that will
    # change in MRI, though no current MRI version tested (1.8.6 - 1.9.2)
    # exhibits the behavior. More info here:
    #
    # http://jira.codehaus.org/browse/JRUBY-2599
    #
    # We redefine evaluate_source to work around this issues.
    if defined?(RUBY_ENGINE) && RUBY_ENGINE == 'jruby'
      undef evaluate_source
      def evaluate_source(scope, locals, &block)
        source, offset = precompiled(locals)
        file, lineno = eval_file, (line - offset)

        scope.instance_eval { Kernel::eval(source, binding, file, lineno) }
      end
    end

That 'lineno' is -5 when it fails on my local instance. I'll do some digging around tomorrow (seriously) to see what's going on there, but at least it suggests JRuby can just ignore the bogus line numbers.

The bug referenced in that tilt comment appears to have been fixed some time ago. I wonder if it was just left in place because nobody knew it had been fixed? I will file an issue for them to reevaluate the hack.

In any case, I'll go with my patch for now. Thanks for the extra legwork!

FYI, it appears the JRuby hack was removed from tilt master, at least:

commit 570f1408d741674687df7e3ffd41d58d3e7a3a69
Author: Ryan Tomayko <rtomayko@gmail.com>
Date:   Mon Sep 19 04:12:55 2011 -0700

    remove evaluate_source method completely
    
    This should no longer be used anywhere. It's nice to remove the
    JRuby hack as well.

Aha! Brilliant. Many thanks for all the work