Skip to content

Commit f67d75f

Browse files
committed
src/goCover: ignore bogus-looking line/column data
Go's line directive can cause the coverage data to include bogus line/column info. The extension naively assumed the coverage data position info is always valid and line/column numbers are 1-based sane numbers. Of course, that's not always true any more. The example in #2453 generated the following coverage data. mode: set blah/blah.go:3.16,5.2 1 1 blah/blah.go:7.16,9.2 1 1 blah/blah.go:13.16,98989.0 2 1 blah/blah.go:98992.0,98993.0 2 1 blah/blah.go:98989.0,98991.0 1 0 The column values were 0. The extension blindly converted them to 0-based positions by subtracting 1. This illegal range eventually made the vscode to throw an exception eventually. I think the go test -cover shouldn't use the bogus line/col info in coverage profile, but that is another issue. It does not make sense to map such bogus numbers, so, this CL filters them out. For golang/go#41222 Fixes #2453 Change-Id: I31a28fecae0a01406e0dc32c48dca1fb4f7b6f0c Reviewed-on: https://go-review.googlesource.com/c/vscode-go/+/447957 Run-TryBot: Hyang-Ah Hana Kim <[email protected]> TryBot-Result: kokoro <[email protected]> Reviewed-by: Peter Weinberger <[email protected]> Reviewed-by: Than McIntosh <[email protected]>
1 parent 91fe72d commit f67d75f

File tree

3 files changed

+24
-7
lines changed

3 files changed

+24
-7
lines changed

src/goCover.ts

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -261,13 +261,25 @@ export function applyCodeCoverageToAllEditors(coverProfilePath: string, dir?: st
261261

262262
// and fill in coveragePath
263263
const coverage = coveragePath.get(parse[1]) || emptyCoverageData();
264+
// When line directive is used this information is artificial and
265+
// the source code file can be non-existent or wrong (go.dev/issues/41222).
266+
// There is no perfect way to guess whether the line/col in coverage profile
267+
// is bogus. At least, we know that 0 or negative values are not true line/col.
268+
const startLine = parseInt(parse[2], 10);
269+
const startCol = parseInt(parse[3], 10);
270+
const endLine = parseInt(parse[4], 10);
271+
const endCol = parseInt(parse[5], 10);
272+
if (startLine < 1 || startCol < 1 || endLine < 1 || endCol < 1) {
273+
return;
274+
}
264275
const range = new vscode.Range(
265276
// Convert lines and columns to 0-based
266-
parseInt(parse[2], 10) - 1,
267-
parseInt(parse[3], 10) - 1,
268-
parseInt(parse[4], 10) - 1,
269-
parseInt(parse[5], 10) - 1
277+
startLine - 1,
278+
startCol - 1,
279+
endLine - 1,
280+
endCol - 1
270281
);
282+
271283
const counts = parseInt(parse[7], 10);
272284
// If is Covered (CoverCount > 0)
273285
if (counts > 0) {

test/integration/coverage.test.ts

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,9 +9,7 @@
99
import assert from 'assert';
1010
import { applyCodeCoverageToAllEditors, coverageFilesForTest, initForTest } from '../../src/goCover';
1111
import { updateGoVarsFromConfig } from '../../src/goInstallTools';
12-
import fs = require('fs-extra');
1312
import path = require('path');
14-
import sinon = require('sinon');
1513
import vscode = require('vscode');
1614

1715
// The ideal test would check that each open editor containing a file with coverage
@@ -39,6 +37,11 @@ suite('Coverage for tests', function () {
3937
const files = Object.keys(coverageFilesForTest());
4038
const aDotGo = files.includes(path.join(fixtureSourcePath, 'a', 'a.go'));
4139
const bDotGo = files.includes(path.join(fixtureSourcePath, 'b', 'b.go'));
42-
assert.equal(aDotGo && bDotGo, true, `seen a.go:${aDotGo}, seen b.go:${bDotGo}\n${files}\n`);
40+
// Coverage data (cover.out) contains a couple of bogus data with file name blah.go. They shouldn't appear.
41+
const blahDotGo = files.includes(path.join(fixtureSourcePath, 'b', 'blah.go'));
42+
assert(
43+
aDotGo && bDotGo && !blahDotGo,
44+
`!seen a.go:${aDotGo} or !seen b.go:${bDotGo} or seen blah.go:${blahDotGo}: ${files}\n`
45+
);
4346
});
4447
});

test/testdata/coverage/cover.out

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
11
mode: set
22
github.com/microsoft/vscode-go/gofixtures/coveragetest/a/a.go:19.71,22.25 3 1
33
github.com/microsoft/vscode-go/gofixtures/coveragetest/b/b.go:35.2,35.14 1 1
4+
github.com/microsoft/vscode-go/gofixtures/coveragetest/b/blah.go:13.16,98989.0 2 1
5+
github.com/microsoft/vscode-go/gofixtures/coveragetest/b/blah.go:98992.0,98993.0 -2 1

0 commit comments

Comments
 (0)