Support nocov comment at the end of a line

[cyberdelia]

Support nocov comments at the end of a line to mark it as irrelevant, it's a bit less verbose than blocks to ignore some benign things, i.e:

case value
when String
  StringFilter
when Array
  ArrayFilter
else
  raise "Absurd case" # :nocov: 
end

[cyberdelia]

Fixed the build failure in #1069.

[cyberdelia]

Freshly rebased!

cc/ @PragTob

[will]

@amatsuda please consider this pull request

[sferik]

I have a bit of feedback but support this feature overall.

Somewhat related: as we approach a 1.0.0 release, I was considering changing the block syntax to be more explicit. Something like:

case coverage_criterion
when :line   then compute_line
when :branch then compute_branch
when :method then compute_method
else
  # coverage:disable
  raise "Unknown criterion: #{coverage_criterion}"
  # coverage:enable
end

If that was the block syntax, presumably we’d want the inline syntax to match:

case coverage_criterion
when :line   then compute_line
when :branch then compute_branch
when :method then compute_method
else
  raise "Unknown criterion: #{coverage_criterion}" # coverage:disable
end

This seems as good a place as any to debate making this change.

[sferik]

I would add a separate, focused test for interactions between inline markers and blocks, instead of making this (already complex) test more complex. There are also a few other edge cases worth testing (e.g. inline # :nocov: comment inside a block).

[PragTob]

@sferik I like the idea of beginning and end comments, makes diagnosis easier 👌

As for one line/in-line comments I can see the appeal but there's a lot of edge cases to consider (as Erik pointed out) and iirc the lines classifier has been a source of performance degradation in the past - those together make me wonder how much of this we need.

The other thing to consider here is how this plays with branch coverage/other coverage types in general.

[sferik]

The other thing to consider here is how this plays with branch coverage/other coverage types in general.

Yes. Since #1161 we also now support method coverage, in addition to line and branch coverage.

Perhaps we should have more granular disabling like:

# coverage:disable:line
# coverage:disable:branch
# coverage:disable:method
# coverage:disable (disables all three)

These should work in both block and inline form.

@PragTob What do you think?

PS: I realize we’re bikeshedding the heck out of this PR, which is more narrowly scoped, but—if we’re going to make API changes before the 1.0 release—I want to make sure we get this right.