1.7.16 is out this week and I have no time to look at this. @donv since you elevated this I just bumped it to 1.7.17, but I don't know when I can look at this.

This is a nontrivial thing to fix. It would require us to modify the interpreter (and maybe the parser) to emit events for every newline. Currently I believe many (most? all?) newlines that have no actual code get collapsed into single NewlineNode instances. I'll take a look at the AST and see how bad it is.

Yeah, it's as I feared. Source lines that don't have any actual code on them are dropped by the parser.

~/projects/jruby-1.7 $ cat blah.rb
p 1
# comment

p 2

~/projects/jruby-1.7 $ ast blah.rb 
AST:
RootNode 0
  BlockNode 0
    NewlineNode 0
      FCallOneArgNode:p 0
        ArrayNode 0
          FixnumNode 0
, null
    NewlineNode 3
      FCallOneArgNode:p 3
        ArrayNode 3
          FixnumNode 3
, null

What happens if they are not dropped?

Everything runs slower (AST interp needs to traverse every node to execute)...

...but I have been thinking about this and there are two possible solutions (both which can be done at same time):

1. Only strip out contiguous newline nodes if they have the same line number

This will also be slower since we will be interp'ing multiple extra newline nodes but it will be a mix. I might try this one first to look for impact then move on to two.

1. Do not strip out newline nodes (or only strip out line in break script engine #1) if --profile option is specified.

I am not sure but don't we need an option like --profile for stuff to work? I will look at option 1 and if I cannot see much change in interping these extra newlines we might go with that.

I like option 1. :-)

I think you mean --debug for full trace events to work. I believe that is necessary to get accurate coverage numbers, because the JIT doesn't emit all the line number events. It probably will in 9k though.

ok there is a larger problem and this bug is not so simple to fix. I am adding some notes for the next attempt (perhaps we may only do this for 9k too).

In RubyYaccLexer. I can add comments and newlines which never are destined to be real lines to be marked as needing coverage data. For newline_node in ParserSupport I can add logic to make sure I don't strip out a relevent newlinenode. There are two other places where newlines are important for 1.7 but since they are not stripped out I can also add those elements to mark themselves important for coverage. So marking is simple.

whitespace-only lines and comment lines never actually emit anything to the parser. The lexer does not think they are important enough to add them to the AST. Having implemented a really really janky system for saving non-essential syntax in the jruby-parser project I am going to say we will never add that system to jruby proper. So what to do?

I have not seen how mri copes with this so perhaps they do something which will give an aha moment.
In 9k, I think if debug is on I can emit raw missing linenumber instrs during IRBuild with the scope boundaries helping to emit coverage properly. So I will add to 9k and probably retarget back to 1.7 if reading MRI I learn a simple way of doing this (after all MRI does not emit comments and blank lines either -- of course mri1.9 is instr-based like 9k).

Hi @enebo ! Did you learn anything new from MRI on this subject? Fixing for JRuby 9k only is OK for us, BTW.

No I have not learned anything more on this. It is weird though that MRI works and we don't since they also do not add this extra info to the AST. Their lexer is completely different so they may be recording something we don't or can't.

Hi, has there been any progress here? I see the milestone has been modified to 9.0.0.0.

Big thanks!

Ooops, the milestone was actually removed... so I guess I shouldn't expect this in 9.0.0.0, right?

Maybe this can be looked at for JRuby 9.1.0.0 ?

Ok I looked a few more minutes on this. So one new piece of data is that TracePoint events DO NOT have anything to do with how simple_cov keeps track of this as:

file_name = File.expand_path('dummy.rb')
File.write(file_name, "# Comment\n\n")
puts "Line count: #{File.readlines(file_name).size}"

trace = TracePoint.new() do |tp|
  puts "LINE: #{tp.path}:#{tp.lineno} - #{tp.event}" if tp.path =~ /#{file_name}/
end

trace.enable
require file_name.chomp('.rb')

will return no events at all for the empty file. If you add a call or some actual code into the file then you see events. So it is probably doing something else like perhaps using SCRIPT_LINES or possibly something with loaded features to reparse the file somehow? I guess looking at simple_cov support would be next.

heh...coverage:

file_name = File.expand_path('dummy.rb')
File.write(file_name, "# Comment\n\nhash\n\n")
puts "Line count: #{File.readlines(file_name).size}"

require 'coverage'

Coverage.start
require file_name.chomp('.rb')
p Coverage.result

So it appears if we have code we create the result array. This result array is nearly right, but there seems to be two related issues:

if I have code with no code and only empty lines (e.g. comments or whitespace) like '# c\n\n\n' then JRuby will return nothing at all. Like the file does not exist.

If we have code but have trailing empty lines then the hash entry is made but the trailing lines are never added to the list.

@donv is going to look at adding some specs for this in ruby/spec. The commit explains the base issue and how it was addressed.