Skip to content

Commit f44f50e

Browse files
committed
gopls/internal/lsp/source: implementation: report builtin 'error'
This change causes the implementation query to report the (fake) location in builtin.go of the the built-in error type, if the query type satisfies that interface. Similarly, the error.Error method. Also, a regtest. Fixes golang/go#59527 Change-Id: I61b179c33c5dfa2c5933f6cae79e7245f83292f2 Reviewed-on: https://go-review.googlesource.com/c/tools/+/483535 Run-TryBot: Alan Donovan <[email protected]> TryBot-Result: Gopher Robot <[email protected]> Reviewed-by: Robert Findley <[email protected]> gopls-CI: kokoro <[email protected]>
1 parent 787e720 commit f44f50e

File tree

3 files changed

+101
-9
lines changed

3 files changed

+101
-9
lines changed

gopls/internal/lsp/regtest/marker.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -289,6 +289,9 @@ var update = flag.Bool("update", false, "if set, update test data during marker
289289
// - parallelize/optimize test execution
290290
// - reorganize regtest packages (and rename to just 'test'?)
291291
// - Rename the files .txtar.
292+
// - Provide some means by which locations in the standard library
293+
// (or builtin.go) can be named, so that, for example, we can we
294+
// can assert that MyError implements the built-in error type.
292295
//
293296
// Existing marker tests (in ../testdata) to port:
294297
// - CallHierarchy

gopls/internal/lsp/source/implementation.go

Lines changed: 29 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,6 @@ import (
3030
//
3131
// TODO(adonovan):
3232
// - Audit to ensure robustness in face of type errors.
33-
// - Support 'error' and 'error.Error', which were also lacking from the old implementation.
3433
// - Eliminate false positives due to 'tricky' cases of the global algorithm.
3534
// - Ensure we have test coverage of:
3635
// type aliases
@@ -388,9 +387,38 @@ func localImplementations(ctx context.Context, snapshot Snapshot, pkg Package, q
388387
locs = append(locs, loc)
389388
}
390389

390+
// Special case: for types that satisfy error, report builtin.go (see #59527).
391+
if types.Implements(queryType, errorInterfaceType) {
392+
loc, err := errorLocation(ctx, snapshot)
393+
if err != nil {
394+
return nil, err
395+
}
396+
locs = append(locs, loc)
397+
}
398+
391399
return locs, nil
392400
}
393401

402+
var errorInterfaceType = types.Universe.Lookup("error").Type().Underlying().(*types.Interface)
403+
404+
// errorLocation returns the location of the 'error' type in builtin.go.
405+
func errorLocation(ctx context.Context, snapshot Snapshot) (protocol.Location, error) {
406+
pgf, err := snapshot.BuiltinFile(ctx)
407+
if err != nil {
408+
return protocol.Location{}, err
409+
}
410+
for _, decl := range pgf.File.Decls {
411+
if decl, ok := decl.(*ast.GenDecl); ok {
412+
for _, spec := range decl.Specs {
413+
if spec, ok := spec.(*ast.TypeSpec); ok && spec.Name.Name == "error" {
414+
return pgf.NodeLocation(spec.Name)
415+
}
416+
}
417+
}
418+
}
419+
return protocol.Location{}, fmt.Errorf("built-in error type not found")
420+
}
421+
394422
// concreteImplementsIntf returns true if a is an interface type implemented by
395423
// concrete type b, or vice versa.
396424
func concreteImplementsIntf(a, b types.Type) bool {

gopls/internal/regtest/misc/references_test.go

Lines changed: 69 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,8 @@ package misc
77
import (
88
"fmt"
99
"os"
10+
"path/filepath"
11+
"reflect"
1012
"sort"
1113
"strings"
1214
"testing"
@@ -315,11 +317,11 @@ func _() {
315317
// - inside the foo.mod/bar [foo.mod/bar.test] test variant package
316318
// - from the foo.mod/bar_test [foo.mod/bar.test] x_test package
317319
// - from the foo.mod/foo package
318-
{"Blah", []string{"bar/bar.go", "bar/bar_test.go", "bar/bar_x_test.go", "foo/foo.go"}},
320+
{"Blah", []string{"bar/bar.go:3", "bar/bar_test.go:7", "bar/bar_x_test.go:12", "foo/foo.go:12"}},
319321

320322
// Foo is referenced in bar_x_test.go via the intermediate test variant
321323
// foo.mod/foo [foo.mod/bar.test].
322-
{"Foo", []string{"bar/bar_x_test.go", "foo/foo.go"}},
324+
{"Foo", []string{"bar/bar_x_test.go:13", "foo/foo.go:5"}},
323325
}
324326

325327
for _, test := range refTests {
@@ -339,11 +341,11 @@ func _() {
339341
// InterfaceM is implemented both in foo.mod/bar [foo.mod/bar.test] (which
340342
// doesn't import foo), and in foo.mod/bar_test [foo.mod/bar.test], which
341343
// imports the test variant of foo.
342-
{"InterfaceM", []string{"bar/bar_test.go", "bar/bar_x_test.go"}},
344+
{"InterfaceM", []string{"bar/bar_test.go:3", "bar/bar_x_test.go:8"}},
343345

344346
// A search within the ordinary package to should find implementations
345347
// (Fer) within the augmented test package.
346-
{"InterfaceF", []string{"foo/foo_test.go"}},
348+
{"InterfaceF", []string{"foo/foo_test.go:3"}},
347349
}
348350

349351
for _, test := range implTests {
@@ -503,19 +505,78 @@ func F() {} // declaration
503505
env.OpenFile("a/a.go")
504506
refLoc := env.RegexpSearch("a/a.go", "F")
505507
got := fileLocations(env, env.References(refLoc))
506-
want := []string{"a/a.go", "b/b.go", "lib/lib.go"}
508+
want := []string{"a/a.go:5", "b/b.go:5", "lib/lib.go:3"}
507509
if diff := cmp.Diff(want, got); diff != "" {
508510
t.Errorf("incorrect References (-want +got):\n%s", diff)
509511
}
510512
})
511513
}
512514

513-
// fileLocations returns a new sorted array of the relative
514-
// file name of each location. Duplicates are not removed.
515+
// Test an 'implementation' query on a type that implements 'error'.
516+
// (Unfortunately builtin locations cannot be expressed using @loc
517+
// in the marker test framework.)
518+
func TestImplementationsOfError(t *testing.T) {
519+
const src = `
520+
-- go.mod --
521+
module example.com
522+
go 1.12
523+
524+
-- a.go --
525+
package a
526+
527+
type Error2 interface {
528+
Error() string
529+
}
530+
531+
type MyError int
532+
func (MyError) Error() string { return "" }
533+
534+
type MyErrorPtr int
535+
func (*MyErrorPtr) Error() string { return "" }
536+
`
537+
Run(t, src, func(t *testing.T, env *Env) {
538+
env.OpenFile("a.go")
539+
540+
for _, test := range []struct {
541+
re string
542+
want []string
543+
}{
544+
// error type
545+
{"Error2", []string{"a.go:10", "a.go:7", "std:builtin/builtin.go"}},
546+
{"MyError", []string{"a.go:3", "std:builtin/builtin.go"}},
547+
{"MyErrorPtr", []string{"a.go:3", "std:builtin/builtin.go"}},
548+
// error.Error method
549+
{"(Error).. string", []string{"a.go:11", "a.go:8", "std:builtin/builtin.go"}},
550+
{"MyError. (Error)", []string{"a.go:4", "std:builtin/builtin.go"}},
551+
{"MyErrorPtr. (Error)", []string{"a.go:4", "std:builtin/builtin.go"}},
552+
} {
553+
matchLoc := env.RegexpSearch("a.go", test.re)
554+
impls := env.Implementations(matchLoc)
555+
got := fileLocations(env, impls)
556+
if !reflect.DeepEqual(got, test.want) {
557+
t.Errorf("Implementations(%q) = %q, want %q",
558+
test.re, got, test.want)
559+
}
560+
}
561+
})
562+
}
563+
564+
// fileLocations returns a new sorted array of the
565+
// relative file name and line number of each location.
566+
// Duplicates are not removed.
567+
// Standard library filenames are abstracted for robustness.
515568
func fileLocations(env *regtest.Env, locs []protocol.Location) []string {
516569
got := make([]string, 0, len(locs))
517570
for _, loc := range locs {
518-
got = append(got, env.Sandbox.Workdir.URIToPath(loc.URI))
571+
path := env.Sandbox.Workdir.URIToPath(loc.URI) // (slashified)
572+
if i := strings.Index(path, "/src/"); i >= 0 && filepath.IsAbs(path) {
573+
// Absolute path with "src" segment: assume it's in GOROOT.
574+
// Strip directory and don't add line/column since they are fragile.
575+
path = "std:" + path[i+len("/src/"):]
576+
} else {
577+
path = fmt.Sprintf("%s:%d", path, loc.Range.Start.Line+1)
578+
}
579+
got = append(got, path)
519580
}
520581
sort.Strings(got)
521582
return got

0 commit comments

Comments
 (0)