gopls/internal/cache: use 1 not 0 for missing line/col info

This change causes the parser of go list error messages to
fill in 1, not 0, for missing line/column information, as
those are the correct values for this particular representation
(1-based UTF-8).

Also, report a bug if we see non-positive arguments to LineCol8Position.

Plus, a regression test.

Fixes golang/go#67360

Change-Id: I87635b99c8b13056c4816b58106ec4a29a9ceb9e
Reviewed-on: https://go-review.googlesource.com/c/tools/+/585555
Reviewed-by: Robert Findley <[email protected]>
LUCI-TryBot-Result: Go LUCI <[email protected]>
Auto-Submit: Alan Donovan <[email protected]>

Author: adonovan

gopls/internal/cache/errors.go

@@ -433,13 +433,13 @@ func splitFileLineCol(s string) (file string, line, col8 int) {
 	// strip col ":%d"
 	s, n1 := stripColonDigits(s)
 	if n1 < 0 {
-		return s, 0, 0 // "filename"
+		return s, 1, 1 // "filename"
 	}
 
 	// strip line ":%d"
 	s, n2 := stripColonDigits(s)
 	if n2 < 0 {
-		return s, n1, 0 // "filename:line"
+		return s, n1, 1 // "filename:line"
 	}
 
 	return s, n2, n1 // "filename:line:col"

gopls/internal/cache/errors_test.go

@@ -19,8 +19,8 @@ func TestParseErrorMessage(t *testing.T) {
 		name             string
 		in               string
 		expectedFileName string
-		expectedLine     int
-		expectedColumn   int
+		expectedLine     int // (missing => 1)
+		expectedColumn   int // (missing => 1)
 	}{
 		{
 			name:             "from go list output",
@@ -34,7 +34,7 @@ func TestParseErrorMessage(t *testing.T) {
 			in:               "C:\\foo\\bar.go:13: message",
 			expectedFileName: "bar.go",
 			expectedLine:     13,
-			expectedColumn:   0,
+			expectedColumn:   1,
 		},
 	}
 

gopls/internal/protocol/mapper.go

@@ -71,6 +71,7 @@ import (
 	"sync"
 	"unicode/utf8"
 
+	"golang.org/x/tools/gopls/internal/util/bug"
 	"golang.org/x/tools/gopls/internal/util/safetoken"
 )
 
@@ -131,6 +132,14 @@ func (m *Mapper) initLines() {
 // LineCol8Position converts a valid line and UTF-8 column number,
 // both 1-based, to a protocol (UTF-16) position.
 func (m *Mapper) LineCol8Position(line, col8 int) (Position, error) {
+	// Report a bug for inputs that are invalid for any file content.
+	if line < 1 {
+		return Position{}, bug.Errorf("invalid 1-based line number: %d", line)
+	}
+	if col8 < 1 {
+		return Position{}, bug.Errorf("invalid 1-based column number: %d", col8)
+	}
+
 	m.initLines()
 	line0 := line - 1 // 0-based
 	if !(0 <= line0 && line0 < len(m.lineStart)) {

gopls/internal/test/marker/testdata/diagnostics/issue67360.txt

@@ -0,0 +1,16 @@
+Regression test for #67360.
+
+This file causes go list to report a "use of internal package
+cmd/internal/browser" error. (It is important that this be a real
+internal std package.) The line directive caused the position of the
+error to lack a column. A bug in the error parser filled in 0, not 1,
+for the missing information, and this is an invalid value in the
+1-based UTF-8 domain, leading to a panic.
+
+-- flags --
+-min_go=go1.21
+
+-- foo.go --
+//line foo.go:1
+package main                    //@ diag(re"package", re"internal package.*not allowed")
+import _ "cmd/internal/browser" //@ diag(re`"`, re"could not import")