cmd/cover: exclude commented-out code from coverage instrumentation

Add logic to exclude purely commented lines from coverage instrumentation.

When instrumenting Go code for coverage, the cover tool now identifies
and excludes lines that contain only comments from coverage blocks.
This prevents commented-out code from being reported as "uncovered"
in coverage reports, which can be misleading.

The implementation adds a splitBlockByComments function that parses
source code character by character to identify segments containing
executable code versus segments containing only comments. The
addCounters function now uses this to create coverage counters only
for segments that contain actual executable code.

The parser correctly handles:
- Single-line comments (//)
- Multi-line comments (/* */)
- String literals containing comment-like sequences
- Raw string literals with fake comments
- Mixed lines with both code and comments

This improves the accuracy of coverage reports by ensuring that
commented-out code, TODOs, and documentation comments don't inflate
the count of uncovered lines.

Fixes #22545

Change-Id: Ib428e6569011abb5f315387e81547147a2dadd2b
GitHub-Last-Rev: 915058146bb5f929f08d63ee191edebd51b2ab56
GitHub-Pull-Request: golang/go#76692
Reviewed-on: https://go-review.googlesource.com/c/go/+/726800
Reviewed-by: Alan Donovan <adonovan@google.com>
Auto-Submit: Alan Donovan <adonovan@google.com>
Reviewed-by: Nicholas Husin <husin@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>

5 files changed, 506 insertions, 14 deletions

diff --git a/src/cmd/cover/cover.go b/src/cmd/cover/cover.go
index 9207fa0e8b..578a978f4e 100644
--- a/src/cmd/cover/cover.go
+++ b/src/cmd/cover/cover.go
@@ -13,6 +13,7 @@ import (
 	"fmt"
 	"go/ast"
 	"go/parser"
+	"go/scanner"
 	"go/token"
 	"internal/coverage"
 	"internal/coverage/encodemeta"
@@ -266,6 +267,142 @@ type File struct {
 	pkg     *Package
 }
 
+// Range represents a contiguous range of executable code within a basic block.
+type Range struct {
+	pos token.Pos
+	end token.Pos
+}
+
+// codeRanges analyzes a block range and returns the sub-ranges that contain
+// executable code, excluding comment-only and blank lines.
+// If no executable code is found, it returns a single zero-width range at
+// start, so that callers always get at least one range (required by pkgcfg
+// mode, which needs a counter unit for every function body).
+func (f *File) codeRanges(start, end token.Pos) []Range {
+	var (
+		startOffset = f.offset(start)
+		endOffset   = f.offset(end)
+		src         = f.content[startOffset:endOffset]
+		origFile    = f.fset.File(start)
+	)
+
+	// Create a temporary File for scanning this block.
+	// We use a separate file because we're scanning a slice of the
+	// original source, so positions in scanFile are relative to the
+	// block start, not the original file.
+	scanFile := token.NewFileSet().AddFile("", -1, len(src))
+
+	var s scanner.Scanner
+	s.Init(scanFile, src, nil, 0)
+
+	// Build ranges in a single pass through the token stream.
+	// We track the last line known to contain code (prevEndLine).
+	// When the next token appears on a line beyond prevEndLine+1,
+	// a gap (comment or blank lines) has been detected: close the
+	// current range and start a new one. Using the token's position
+	// directly (rather than the line start) ensures counter insertion
+	// lands after any closing "*/" on that line.
+	var ranges []Range
+	var codeStart token.Pos // start of current code range (in origFile)
+	prevEndLine := 0        // last line with code; 0 means no code yet
+
+	for {
+		pos, tok, lit := s.Scan()
+		if tok == token.EOF {
+			break
+		}
+
+		// Skip braces and automatic semicolons: braces are block
+		// delimiters, not executable code. The Go spec
+		// (https://go.dev/ref/spec#Semicolons) requires the scanner
+		// to insert semicolons (with lit == "\n") after }, ), ], etc.
+		// These are always on lines already marked by real tokens,
+		// except for lone "}" lines. Skipping both prevents a lone
+		// "}" from being treated as a separate code range, which
+		// would cause counter insertion after return statements.
+		if tok == token.LBRACE || tok == token.RBRACE {
+			continue
+		}
+		if tok == token.SEMICOLON && lit == "\n" {
+			continue
+		}
+
+		// Use PositionFor with adjusted=false to ignore //line directives.
+		startLine := scanFile.PositionFor(pos, false).Line
+		endLine := startLine
+		if tok == token.STRING {
+			// Only string literals can span multiple lines.
+			// TODO(adonovan): simplify when https://go.dev/issue/74958 is resolved.
+			endLine = scanFile.PositionFor(pos+token.Pos(len(lit)), false).Line
+		}
+
+		if prevEndLine == 0 {
+			// First code token — start the first range.
+			codeStart = origFile.Pos(startOffset + scanFile.Offset(pos))
+		} else if startLine > prevEndLine+1 {
+			// Gap detected — close previous range, start new one.
+			codeEnd := origFile.Pos(startOffset + scanFile.Offset(scanFile.LineStart(prevEndLine+1)))
+			ranges = append(ranges, Range{pos: codeStart, end: codeEnd})
+			codeStart = origFile.Pos(startOffset + scanFile.Offset(pos))
+		}
+
+		if endLine > prevEndLine {
+			prevEndLine = endLine
+		}
+	}
+
+	// Close any open code range at the end.
+	if prevEndLine > 0 {
+		if prevEndLine < scanFile.LineCount() {
+			// There are non-code lines after the last code line
+			// (e.g., a lone "}"). Close at the next line's start.
+			codeEnd := origFile.Pos(startOffset + scanFile.Offset(scanFile.LineStart(prevEndLine+1)))
+			ranges = append(ranges, Range{pos: codeStart, end: codeEnd})
+		} else {
+			ranges = append(ranges, Range{pos: codeStart, end: end})
+		}
+	}
+
+	// If no code was found, return a zero-width range so that callers
+	// still get a counter (needed for pkgcfg function registration)
+	// but the range doesn't visually cover any source lines.
+	if len(ranges) == 0 {
+		return []Range{{pos: start, end: start}}
+	}
+
+	return ranges
+}
+
+// insideStatement reports whether pos falls strictly inside
+// (not at the start of) any statement in stmts.
+func insideStatement(pos token.Pos, stmts []ast.Stmt) bool {
+	// Binary search for the first statement starting at or after pos.
+	i, _ := slices.BinarySearchFunc(stmts, pos, func(s ast.Stmt, p token.Pos) int {
+		return cmp.Compare(s.Pos(), p)
+	})
+	// Check if pos falls inside the preceding statement.
+	return i > 0 && pos < stmts[i-1].End()
+}
+
+// mergeRangesWithinStatements merges consecutive ranges when a later range's
+// start position falls strictly inside a statement. This prevents counter
+// insertion inside multi-line statements such as const (...) blocks.
+func mergeRangesWithinStatements(ranges []Range, stmts []ast.Stmt) []Range {
+	if len(ranges) <= 1 {
+		return ranges
+	}
+	merged := []Range{ranges[0]}
+	for _, r := range ranges[1:] {
+		if insideStatement(r.pos, stmts) {
+			// Extend previous range to cover this one.
+			merged[len(merged)-1].end = r.end
+		} else {
+			merged = append(merged, r)
+		}
+	}
+	return merged
+}
+
 // findText finds text in the original source, starting at pos.
 // It correctly skips over comments and assumes it need not
 // handle quoted strings.
@@ -720,8 +857,9 @@ func (f *File) newCounter(start, end token.Pos, numStmt int) string {
 			panic("internal error: counter var unset")
 		}
 		stmt = counterStmt(f, fmt.Sprintf("%s[%d]", f.fn.counterVar, slot))
-		stpos := f.fset.Position(start)
-		enpos := f.fset.Position(end)
+		// Physical positions, ignoring //line directives.
+		stpos := f.position(start)
+		enpos := f.position(end)
 		stpos, enpos = dedup(stpos, enpos)
 		unit := coverage.CoverableUnit{
 			StLine:  uint32(stpos.Line),
@@ -755,7 +893,8 @@ func (f *File) addCounters(pos, insertPos, blockEnd token.Pos, list []ast.Stmt,
 	// Special case: make sure we add a counter to an empty block. Can't do this below
 	// or we will add a counter to an empty statement list after, say, a return statement.
 	if len(list) == 0 {
-		f.edit.Insert(f.offset(insertPos), f.newCounter(insertPos, blockEnd, 0)+";")
+		r := f.codeRanges(insertPos, blockEnd)[0]
+		f.edit.Insert(f.offset(r.pos), f.newCounter(r.pos, r.end, 0)+";")
 		return
 	}
 	// Make a copy of the list, as we may mutate it and should leave the
@@ -805,7 +944,16 @@ func (f *File) addCounters(pos, insertPos, blockEnd token.Pos, list []ast.Stmt,
 			end = blockEnd
 		}
 		if pos != end { // Can have no source to cover if e.g. blocks abut.
-			f.edit.Insert(f.offset(insertPos), f.newCounter(pos, end, last)+";")
+			// Create counters only for executable code ranges.
+			// Merge back ranges that fall inside a statement to avoid
+			// inserting counters inside multi-line constructs (e.g. const blocks).
+			for i, r := range mergeRangesWithinStatements(f.codeRanges(pos, end), list[:last]) {
+				insertOffset := f.offset(r.pos)
+				if i == 0 {
+					insertOffset = f.offset(insertPos)
+				}
+				f.edit.Insert(insertOffset, f.newCounter(r.pos, r.end, last)+";")
+			}
 		}
 		list = list[last:]
 		if len(list) == 0 {
@@ -976,9 +1124,14 @@ type block1 struct {
 	index int
 }
 
+// position returns the Position for pos, ignoring //line directives.
+func (f *File) position(pos token.Pos) token.Position {
+	return f.fset.PositionFor(pos, false)
+}
+
 // offset translates a token position into a 0-indexed byte offset.
 func (f *File) offset(pos token.Pos) int {
-	return f.fset.Position(pos).Offset
+	return f.position(pos).Offset
 }
 
 // addVariables adds to the end of the file the declarations to set up the counter and position variables.
@@ -1020,8 +1173,9 @@ func (f *File) addVariables(w io.Writer) {
 	// - 32-bit ending line number
 	// - (16 bit ending column number << 16) | (16-bit starting column number).
 	for i, block := range f.blocks {
-		start := f.fset.Position(block.startByte)
-		end := f.fset.Position(block.endByte)
+		// Physical positions, ignoring //line directives.
+		start := f.position(block.startByte)
+		end := f.position(block.endByte)
 
 		start, end = dedup(start, end)
 
diff --git a/src/cmd/cover/cover_test.go b/src/cmd/cover/cover_test.go
index a19e0d0924..504c263c9c 100644
--- a/src/cmd/cover/cover_test.go
+++ b/src/cmd/cover/cover_test.go
@@ -8,6 +8,7 @@ import (
 	"bufio"
 	"bytes"
 	cmdcover "cmd/cover"
+	"cmp"
 	"flag"
 	"fmt"
 	"go/ast"
@@ -19,6 +20,8 @@ import (
 	"os/exec"
 	"path/filepath"
 	"regexp"
+	"slices"
+	"strconv"
 	"strings"
 	"sync"
 	"testing"
@@ -647,3 +650,217 @@ func TestAlignment(t *testing.T) {
 	cmd := testenv.Command(t, testenv.GoToolPath(t), "test", "-cover", filepath.Join(testdata, "align.go"), filepath.Join(testdata, "align_test.go"))
 	run(cmd, t)
 }
+
+// lineRange represents a coverage range as a pair of line numbers (1-indexed, inclusive).
+type lineRange struct {
+	start, end int
+}
+
+// parseBrackets strips bracket markers (U+00AB «, U+00BB ») from src,
+// returning the cleaned Go source and a list of expected coverage ranges
+// as line number pairs (1-indexed, inclusive).
+func parseBrackets(src []byte) ([]byte, []lineRange) {
+	const (
+		open  = "\u00ab" // «
+		close = "\u00bb" // »
+	)
+	var (
+		cleaned []byte
+		ranges  []lineRange
+		stack   []int // stack of open marker byte positions in cleaned
+	)
+	i := 0
+	for i < len(src) {
+		if bytes.HasPrefix(src[i:], []byte(open)) {
+			stack = append(stack, len(cleaned))
+			i += len(open)
+		} else if bytes.HasPrefix(src[i:], []byte(close)) {
+			if len(stack) == 0 {
+				panic("unmatched close bracket at offset " + strconv.Itoa(i))
+			}
+			startPos := stack[len(stack)-1]
+			stack = stack[:len(stack)-1]
+			endPos := len(cleaned)
+			// Convert byte positions to line numbers.
+			startLine := 1 + bytes.Count(cleaned[:startPos], []byte{'\n'})
+			endLine := 1 + bytes.Count(cleaned[:endPos], []byte{'\n'})
+			// If endPos is at the start of a line (after \n), the range
+			// actually ended on the previous line.
+			if endPos > 0 && cleaned[endPos-1] == '\n' {
+				endLine--
+			}
+			if endLine < startLine {
+				endLine = startLine
+			}
+			ranges = append(ranges, lineRange{startLine, endLine})
+			i += len(close)
+		} else {
+			cleaned = append(cleaned, src[i])
+			i++
+		}
+	}
+	if len(stack) != 0 {
+		panic("unmatched open bracket(s)")
+	}
+	return cleaned, ranges
+}
+
+// coverRanges runs the cover tool on src and returns the coverage ranges
+// as line number pairs (1-indexed, inclusive).
+func coverRanges(t *testing.T, src []byte) []lineRange {
+	t.Helper()
+	tmpdir := t.TempDir()
+	srcPath := filepath.Join(tmpdir, "test.go")
+	if err := os.WriteFile(srcPath, src, 0666); err != nil {
+		t.Fatalf("writing test file: %v", err)
+	}
+
+	cmd := testenv.Command(t, testcover(t), "-mode=set", srcPath)
+	out, err := cmd.Output()
+	if err != nil {
+		t.Fatalf("cover failed: %v\nOutput: %s", err, out)
+	}
+
+	outStr := string(out)
+	// Skip the //line directive that cover adds at the top.
+	if _, after, ok := strings.Cut(outStr, "\n"); ok {
+		outStr = after
+	}
+
+	// Parse the coverage struct's Pos array to extract ranges.
+	// Format: startLine, endLine, (endCol<<16)|startCol, // [index]
+	re := regexp.MustCompile(`(\d+), (\d+), (0x[0-9a-f]+), // \[\d+\]`)
+	matches := re.FindAllStringSubmatch(outStr, -1)
+
+	var result []lineRange
+	for _, m := range matches {
+		startLine, _ := strconv.Atoi(m[1])
+		endLine, _ := strconv.Atoi(m[2])
+		var packed int
+		fmt.Sscanf(m[3], "0x%x", &packed)
+		endCol := (packed >> 16) & 0xFFFF
+		startCol := packed & 0xFFFF
+		// Skip zero-width ranges (comment-only or empty blocks).
+		if startLine == endLine && startCol == endCol {
+			continue
+		}
+		// The range [start, end) is half-open. When endCol==1, the
+		// range reaches the beginning of endLine but includes no
+		// content on it, so the last covered line is endLine-1.
+		if endCol == 1 && endLine > startLine {
+			endLine--
+		}
+		result = append(result, lineRange{startLine, endLine})
+	}
+	return result
+}
+
+// compareRanges is a test helper that compares got and want line ranges.
+// Both slices are sorted before comparison since the cover tool's output
+// order (AST walk order) may differ from source order (marker order).
+func compareRanges(t *testing.T, src []byte, got, want []lineRange) {
+	t.Helper()
+	lines := strings.Split(string(src), "\n")
+	snippet := func(r lineRange) string {
+		start, end := r.start-1, r.end
+		if start < 0 {
+			start = 0
+		}
+		if end > len(lines) {
+			end = len(lines)
+		}
+		s := strings.Join(lines[start:end], "\n")
+		if len(s) > 120 {
+			s = s[:117] + "..."
+		}
+		return s
+	}
+
+	sortRanges := func(rs []lineRange) []lineRange {
+		s := append([]lineRange(nil), rs...)
+		slices.SortFunc(s, func(a, b lineRange) int {
+			if c := cmp.Compare(a.start, b.start); c != 0 {
+				return c
+			}
+			return cmp.Compare(a.end, b.end)
+		})
+		return s
+	}
+
+	gotSorted := sortRanges(got)
+	wantSorted := sortRanges(want)
+
+	if len(gotSorted) != len(wantSorted) {
+		t.Errorf("got %d ranges, want %d", len(gotSorted), len(wantSorted))
+		for i, r := range gotSorted {
+			t.Logf("  got[%d]: lines %d-%d %q", i, r.start, r.end, snippet(r))
+		}
+		for i, r := range wantSorted {
+			t.Logf("  want[%d]: lines %d-%d %q", i, r.start, r.end, snippet(r))
+		}
+		return
+	}
+	for i := range wantSorted {
+		if gotSorted[i] != wantSorted[i] {
+			t.Errorf("range %d: got lines %d-%d %q, want lines %d-%d %q",
+				i, gotSorted[i].start, gotSorted[i].end, snippet(gotSorted[i]),
+				wantSorted[i].start, wantSorted[i].end, snippet(wantSorted[i]))
+		}
+	}
+}
+
+// TestCommentedOutCodeExclusion tests that comment-only and blank lines
+// are excluded from coverage ranges using a bracket-marker testdata file.
+func TestCommentedOutCodeExclusion(t *testing.T) {
+	testenv.MustHaveGoBuild(t)
+
+	markedSrc, err := os.ReadFile(filepath.Join(testdata, "ranges", "ranges.go"))
+	if err != nil {
+		t.Fatal(err)
+	}
+	src, want := parseBrackets(markedSrc)
+	got := coverRanges(t, src)
+	compareRanges(t, src, got, want)
+}
+
+// TestLineDirective verifies that //line directives don't affect coverage
+// line number calculations (we use physical lines, not remapped ones).
+func TestLineDirective(t *testing.T) {
+	testenv.MustHaveGoBuild(t)
+
+	// Source with //line directive that would remap lines if not handled correctly.
+	testSrc := `package main
+
+import "fmt"
+
+func main() {
+	//line other.go:100
+«	x := 1
+»	// comment that should be excluded
+«	y := 2
+»	//line other.go:200
+«	fmt.Println(x, y)
+»}`
+
+	src, want := parseBrackets([]byte(testSrc))
+	got := coverRanges(t, src)
+	compareRanges(t, src, got, want)
+}
+
+// TestCommentExclusionBasic is a simple test verifying that a comment splits
+// a single block into two separate coverage ranges.
+func TestCommentExclusionBasic(t *testing.T) {
+	testenv.MustHaveGoBuild(t)
+
+	testSrc := `package main
+
+func main() {
+«	x := 1
+»	// this comment should split the block
+«	y := 2
+»}`
+
+	src, want := parseBrackets([]byte(testSrc))
+	got := coverRanges(t, src)
+	compareRanges(t, src, got, want)
+}
diff --git a/src/cmd/cover/testdata/html/html.golden b/src/cmd/cover/testdata/html/html.golden
index 84377d1e20..8cacf6ade1 100644
--- a/src/cmd/cover/testdata/html/html.golden
+++ b/src/cmd/cover/testdata/html/html.golden
@@ -1,6 +1,6 @@
 // START f
-func f() <span class="cov8" title="1">{
-	ch := make(chan int)
+func f() {
+	<span class="cov8" title="1">ch := make(chan int)
 	select </span>{
 	case &lt;-ch:<span class="cov0" title="0"></span>
 	default:<span class="cov8" title="1"></span>
@@ -9,10 +9,10 @@ func f() <span class="cov8" title="1">{
 
 // END f
 // START g
-func g() <span class="cov8" title="1">{
-	if false </span><span class="cov0" title="0">{
-		fmt.Printf("Hello")
-	}</span>
+func g() {
+	<span class="cov8" title="1">if false </span>{
+		<span class="cov0" title="0">fmt.Printf("Hello")
+</span>	}
 }
 
 // END g
diff --git a/src/cmd/cover/testdata/main.go b/src/cmd/cover/testdata/main.go
index be74b4aa65..009de789f0 100644
--- a/src/cmd/cover/testdata/main.go
+++ b/src/cmd/cover/testdata/main.go
@@ -94,12 +94,16 @@ func count(line uint32) (uint32, int) {
 	// and we don't want that brace to steal the count for the condition on the "if".
 	// Therefore we test for a perfect (lo==line && hi==line) match, but if we can't
 	// find that we take the first imperfect match.
+	// When multiple perfect matches exist (e.g., an outer block counter and an inner
+	// function literal counter on the same line), prefer the last one: it corresponds
+	// to the innermost scope, which is the most specific counter for that line.
 	index := -1
 	indexLo := uint32(1e9)
+	perfectIndex := -1
 	for i := range coverTest.Count {
 		lo, hi := coverTest.Pos[3*i], coverTest.Pos[3*i+1]
 		if lo == line && line == hi {
-			return coverTest.Count[i], i
+			perfectIndex = i
 		}
 		// Choose the earliest match (the counters are in unpredictable order).
 		if lo <= line && line <= hi && indexLo > lo {
@@ -107,6 +111,9 @@ func count(line uint32) (uint32, int) {
 			indexLo = lo
 		}
 	}
+	if perfectIndex >= 0 {
+		return coverTest.Count[perfectIndex], perfectIndex
+	}
 	if index == -1 {
 		fmt.Fprintln(os.Stderr, "cover_test: no counter for line", line)
 		PASS = false
diff --git a/src/cmd/cover/testdata/ranges/ranges.go b/src/cmd/cover/testdata/ranges/ranges.go
new file mode 100644
index 0000000000..17b42e6423
--- /dev/null
+++ b/src/cmd/cover/testdata/ranges/ranges.go
@@ -0,0 +1,114 @@
+package main
+
+import "fmt"
+
+func main() {
+«	fmt.Println("Start")
+	x := 42
+»
+	// This is a regular comment
+«	y := 0
+	fmt.Println("After comment")
+»
+	// Multiple comment lines
+	// fmt.Println("commented code")
+	// TODO: implement this later
+
+«	fmt.Println("After multiple comments")
+»
+	/* block comment */
+
+«	fmt.Println("After block comment")
+	z := 0
+»
+	«if x > 0 {»
+«		y = x * 2
+»	} else {
+«		y = x - 2
+»	}
+
+«	z = 5
+»
+	/* Multiline block
+	comment spanning
+	several lines */
+
+«	z1 := 0
+»
+«	z1 = 1 /* inline comment
+»	spanning lines
+«	end */ z1 = 2
+»
+«	z1 = 3; /* // */ z1 = 4
+»
+«	z1 = 5 /* //
+»	//
+«	// */ z1 = 6
+»
+	/*
+«	*/ z1 = 7 /*
+»	*/
+
+«	z1 = 8/*
+»	*/ /* comment
+«	*/z1 = 9
+»
+«	/* before */ z1 = 10
+	/* before */ z1 = 10 /* after */
+	             z1 = 10 /* after */
+»
+«	fmt.Printf("Result: %d\n", z)
+	fmt.Printf("Result: %d\n", z1)
+»
+«	s := `This is a multi-line raw string
+	// fake comment on line 2
+	/* and fake comment on line 3 */
+	and other`
+»
+«	s = `another multiline string
+	` // another trap
+»
+«	fmt.Printf("%s", s)
+»
+	// More comments to exclude
+	// for i := 0; i < 10; i++ {
+	//   fmt.Printf("Loop %d", i)
+	// }
+
+«	fmt.Printf("Result: %d\n", y)»
+	// end comment
+}
+
+func empty() {
+
+}
+
+func singleBlock() {
+«	fmt.Printf("ResultSomething")
+»}
+
+func justComment() {
+	// comment
+}
+
+func justMultilineComment() {
+	/* comment
+	again
+	until here */
+}
+
+func constBlock() {
+«	const (
+		A = 1
+
+		B = 2
+	)
+	fmt.Printf("A=%d B=%d", A, B)
+»}
+
+func compositeLit() {
+«	m := map[string]int{
+		"a": 1,
+»	}
+«	fmt.Println(m)
+»}