Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cmd/cover: //line directives frequently handled incorrectly #41222

Open
pjweinb opened this issue Sep 4, 2020 · 2 comments
Open

cmd/cover: //line directives frequently handled incorrectly #41222

pjweinb opened this issue Sep 4, 2020 · 2 comments
Labels
help wanted NeedsInvestigation
Milestone

Comments

@pjweinb
Copy link

@pjweinb pjweinb commented Sep 4, 2020

(This combines #35781, #33690, #25280, and golang/vscode-go#603)

Source files that contain //line directives confuse the coverage tool, prducing coverage output that is unhelpful or confusing to users.

For instance, a short main.go (import path foo.gak) with first lines

//
package main

produces a coverage file:

mode: set
foo.gak/main.go:6.10,8.26 2 1
foo.gak/main.go:12.2,12.19 1 1
foo.gak/main.go:8.26,11.3 2 1
foo.gak/main.go:12.19,13.21 1 0

If the first line is changed to

//line con.go:8

the coverage file changes to:

mode: set
foo.gak/main.go:12.0,14.0 2 1
foo.gak/main.go:18.0,18.0 1 1
foo.gak/main.go:14.0,17.0 2 1
foo.gak/main.go:18.0,19.0 1 0

This file does not refer to con.go, and the ranges are wrong (or, at best, imprecise) for that file, and completely wrong for main.go.

A minimal improvement would be for the coverage tool to change user's //line directives to bare // comments, so that the coverage data corresponds to the file presented to the tool. [Getting results that point to files mentioned in //line directives would require changing the format of the tables in the generated code, at least. Even then the results might be peculiar, for instance if the generated file were run through go fmt.]

@andybons
Copy link
Member

@andybons andybons commented Sep 8, 2020

@golang/tools-team

@andybons andybons added the NeedsInvestigation label Sep 8, 2020
@andybons andybons added this to the Unplanned milestone Sep 8, 2020
@ianlancetaylor ianlancetaylor removed this from the Unplanned milestone Oct 14, 2020
@ianlancetaylor ianlancetaylor added this to the Backlog milestone Oct 14, 2020
@brianpursley
Copy link

@brianpursley brianpursley commented Nov 2, 2020

I was looking into this a little bit and here is some more background information...

According to the Go Programming Language Reference, line directives can be of //line or /*line ...*/ format:

In order to be recognized as a line directive, the comment must start with //line or /*line followed by a space, and must contain at least one colon. The //line form must start at the beginning of a line. A line directive specifies the source position for the character immediately following the comment as having come from the specified file, line and column: For a //line comment, this is the first character of the next line, and for a /*line comment this is the character position immediately following the closing */.

The //line format wouldn't be too hard to replace because it has to be at the start of the line, but /*line ...*/ is more complicated because it can be anywhere on a line. However unlikely, it could be inside a string, so it is not just a simple replace... replacing it if it is inside a string, would change the program behavior.

I suppose one approach would be to just fix it for //line type directives (by replacing those lines with //) and leave /*line ...*/ as-is, and it would be a known limitation of cover.

WDYT?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted NeedsInvestigation
Projects
None yet
Development

No branches or pull requests

4 participants