diff --git a/cmd/deadcode/deadcode.go b/cmd/deadcode/deadcode.go index e0fce428d08..e0b4de32387 100644 --- a/cmd/deadcode/deadcode.go +++ b/cmd/deadcode/deadcode.go @@ -260,7 +260,7 @@ func main() { edges = append(edges, jsonEdge{ Initial: cond(len(edges) == 0, prettyName(edge.Caller.Func, true), ""), Kind: cond(isStaticCall(edge), "static", "dynamic"), - Position: toJSONPosition(prog.Fset.Position(edge.Site.Pos())), + Position: toJSONPosition(prog.Fset.Position(edge.Pos())), Callee: prettyName(edge.Callee.Func, true), }) } diff --git a/cmd/deadcode/testdata/issue67915.txt b/cmd/deadcode/testdata/issue67915.txt new file mode 100644 index 00000000000..c896e45940c --- /dev/null +++ b/cmd/deadcode/testdata/issue67915.txt @@ -0,0 +1,37 @@ +# Test of -whylive with reflective call +# (regression test for golang/go#67915). + +# The live function is reached via reflection: + + deadcode example.com + want "unreachable func: dead" +!want "unreachable func: live" + +# Reflective calls have Edge.Site=nil, which formerly led to a crash +# when -whylive would compute its position. Now it has NoPos. + + deadcode -whylive=example.com.live example.com + want " example.com.main" + want " static@L0006 --> reflect.Value.Call" + want "dynamic@L0000 --> example.com.live" + +-- go.mod -- +module example.com +go 1.18 + +-- main.go -- +package main + +import "reflect" + +func main() { + reflect.ValueOf(live).Call(nil) +} + +func live() { + println("hello") +} + +func dead() { + println("goodbye") +} diff --git a/go.mod b/go.mod index 12e1b033bc3..40c3d4751cd 100644 --- a/go.mod +++ b/go.mod @@ -5,10 +5,10 @@ go 1.19 // => default GODEBUG has gotypesalias=0 require ( github.com/google/go-cmp v0.6.0 github.com/yuin/goldmark v1.4.13 - golang.org/x/mod v0.18.0 - golang.org/x/net v0.26.0 + golang.org/x/mod v0.19.0 + golang.org/x/net v0.27.0 golang.org/x/sync v0.7.0 golang.org/x/telemetry v0.0.0-20240521205824-bda55230c457 ) -require golang.org/x/sys v0.21.0 // indirect +require golang.org/x/sys v0.22.0 // indirect diff --git a/go.sum b/go.sum index 7a313b1630d..91180267608 100644 --- a/go.sum +++ b/go.sum @@ -2,13 +2,13 @@ github.com/google/go-cmp v0.6.0 h1:ofyhxvXcZhMsU5ulbFiLKl/XBFqE1GSq7atu8tAmTRI= github.com/google/go-cmp v0.6.0/go.mod h1:17dUlkBOakJ0+DkrSSNjCkIjxS6bF9zb3elmeNGIjoY= github.com/yuin/goldmark v1.4.13 h1:fVcFKWvrslecOb/tg+Cc05dkeYx540o0FuFt3nUVDoE= github.com/yuin/goldmark v1.4.13/go.mod h1:6yULJ656Px+3vBD8DxQVa3kxgyrAnzto9xy5taEt/CY= -golang.org/x/mod v0.18.0 h1:5+9lSbEzPSdWkH32vYPBwEpX8KwDbM52Ud9xBUvNlb0= -golang.org/x/mod v0.18.0/go.mod h1:hTbmBsO62+eylJbnUtE2MGJUyE7QWk4xUqPFrRgJ+7c= -golang.org/x/net v0.26.0 h1:soB7SVo0PWrY4vPW/+ay0jKDNScG2X9wFeYlXIvJsOQ= -golang.org/x/net v0.26.0/go.mod h1:5YKkiSynbBIh3p6iOc/vibscux0x38BZDkn8sCUPxHE= +golang.org/x/mod v0.19.0 h1:fEdghXQSo20giMthA7cd28ZC+jts4amQ3YMXiP5oMQ8= +golang.org/x/mod v0.19.0/go.mod h1:hTbmBsO62+eylJbnUtE2MGJUyE7QWk4xUqPFrRgJ+7c= +golang.org/x/net v0.27.0 h1:5K3Njcw06/l2y9vpGCSdcxWOYHOUk3dVNGDXN+FvAys= +golang.org/x/net v0.27.0/go.mod h1:dDi0PyhWNoiUOrAS8uXv/vnScO4wnHQO4mj9fn/RytE= golang.org/x/sync v0.7.0 h1:YsImfSBoP9QPYL0xyKJPq0gcaJdG3rInoqxTWbfQu9M= golang.org/x/sync v0.7.0/go.mod h1:Czt+wKu1gCyEFDUtn0jG5QVvpJ6rzVqr5aXyt9drQfk= -golang.org/x/sys v0.21.0 h1:rF+pYz3DAGSQAxAu1CbC7catZg4ebC4UIeIhKxBZvws= -golang.org/x/sys v0.21.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA= +golang.org/x/sys v0.22.0 h1:RI27ohtqKCnwULzJLqkv897zojh5/DwS/ENaMzUOaWI= +golang.org/x/sys v0.22.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA= golang.org/x/telemetry v0.0.0-20240521205824-bda55230c457 h1:zf5N6UOrA487eEFacMePxjXAJctxKmyjKUsjA11Uzuk= golang.org/x/telemetry v0.0.0-20240521205824-bda55230c457/go.mod h1:pRgIJT+bRLFKnoM1ldnzKoxTIn14Yxz928LQRYYgIN0= diff --git a/go/analysis/analysistest/analysistest.go b/go/analysis/analysistest/analysistest.go index 368cb1bd2d9..c1b2dd4fa1b 100644 --- a/go/analysis/analysistest/analysistest.go +++ b/go/analysis/analysistest/analysistest.go @@ -114,9 +114,9 @@ type Testing interface { // into nonconflicting parts. // // Conflicts of the second kind can be avoided by giving the -// alternative fixes different names (SuggestedFix.Message) and using -// a multi-section .txtar file with a named section for each -// alternative fix. +// alternative fixes different names (SuggestedFix.Message) and +// defining the .golden file as a multi-section txtar file with a +// named section for each alternative fix, as shown above. // // Analyzers that compute fixes from a textual diff of the // before/after file contents (instead of directly from syntax tree @@ -126,6 +126,16 @@ type Testing interface { // sufficient separation of the statements in the test input so that // the computed diffs do not overlap. If that fails, break the test // into smaller parts. +// +// TODO(adonovan): the behavior of RunWithSuggestedFixes as documented +// above is impractical for tests that report multiple diagnostics and +// offer multiple alternative fixes for the same diagnostic, and it is +// inconsistent with the interpretation of multiple diagnostics +// described at Diagnostic.SuggestedFixes. +// We need to rethink the analyzer testing API to better support such +// cases. In the meantime, users of RunWithSuggestedFixes testing +// analyzers that offer alternative fixes are advised to put each fix +// in a separate .go file in the testdata. func RunWithSuggestedFixes(t Testing, dir string, a *analysis.Analyzer, patterns ...string) []*Result { r := Run(t, dir, a, patterns...) diff --git a/go/analysis/diagnostic.go b/go/analysis/diagnostic.go index 4eb90599808..ee083a2d686 100644 --- a/go/analysis/diagnostic.go +++ b/go/analysis/diagnostic.go @@ -33,8 +33,17 @@ type Diagnostic struct { URL string // SuggestedFixes is an optional list of fixes to address the - // problem described by the diagnostic, each one representing + // problem described by the diagnostic. Each one represents // an alternative strategy; at most one may be applied. + // + // Fixes for different diagnostics should be treated as + // independent changes to the same baseline file state, + // analogous to a set of git commits all with the same parent. + // Combining fixes requires resolving any conflicts that + // arise, analogous to a git merge. + // Any conflicts that remain may be dealt with, depending on + // the tool, by discarding fixes, consulting the user, or + // aborting the operation. SuggestedFixes []SuggestedFix // Related contains optional secondary positions and messages @@ -58,8 +67,10 @@ type RelatedInformation struct { // // The TextEdits must not overlap, nor contain edits for other packages. type SuggestedFix struct { - // A description for this suggested fix to be shown to a user deciding - // whether to accept it. + // A verb phrase describing the fix, to be shown to + // a user trying to decide whether to accept it. + // + // Example: "Remove the surplus argument" Message string TextEdits []TextEdit } diff --git a/go/analysis/internal/checker/checker.go b/go/analysis/internal/checker/checker.go index 52f0e55aaae..5a7146576a9 100644 --- a/go/analysis/internal/checker/checker.go +++ b/go/analysis/internal/checker/checker.go @@ -11,7 +11,6 @@ package checker import ( "bytes" "encoding/gob" - "errors" "flag" "fmt" "go/format" @@ -134,16 +133,21 @@ func Run(args []string, analyzers []*analysis.Analyzer) (exitcode int) { allSyntax := needFacts(analyzers) initial, err := load(args, allSyntax) if err != nil { - if _, ok := err.(typeParseError); !ok { - // Fail when some of the errors are not - // related to parsing nor typing. - log.Print(err) - return 1 - } - // TODO: filter analyzers based on RunDespiteError? + log.Print(err) + return 1 } - // Run the analysis. + pkgsExitCode := 0 + // Print package errors regardless of RunDespiteErrors. + // Do not exit if there are errors, yet. + if n := packages.PrintErrors(initial); n > 0 { + pkgsExitCode = 1 + } + + // Run the analyzers. On each package with (transitive) + // errors, we run only the subset of analyzers that are + // marked (and whose transitive requirements are also + // marked) with RunDespiteErrors. roots := analyze(initial, analyzers) // Apply fixes. @@ -155,18 +159,18 @@ func Run(args []string, analyzers []*analysis.Analyzer) (exitcode int) { } } - // Print the results. - return printDiagnostics(roots) -} - -// typeParseError represents a package load error -// that is related to typing and parsing. -type typeParseError struct { - error + // Print the results. If !RunDespiteErrors and there + // are errors in the packages, this will have 0 exit + // code. Otherwise, we prefer to return exit code + // indicating diagnostics. + if diagExitCode := printDiagnostics(roots); diagExitCode != 0 { + return diagExitCode // there were diagnostics + } + return pkgsExitCode // package errors but no diagnostics } -// load loads the initial packages. If all loading issues are related to -// typing and parsing, the returned error is of type typeParseError. +// load loads the initial packages. Returns only top-level loading +// errors. Does not consider errors in packages. func load(patterns []string, allSyntax bool) ([]*packages.Package, error) { mode := packages.LoadSyntax if allSyntax { @@ -178,44 +182,12 @@ func load(patterns []string, allSyntax bool) ([]*packages.Package, error) { Tests: IncludeTests, } initial, err := packages.Load(&conf, patterns...) - if err == nil { - if len(initial) == 0 { - err = fmt.Errorf("%s matched no packages", strings.Join(patterns, " ")) - } else { - err = loadingError(initial) - } + if err == nil && len(initial) == 0 { + err = fmt.Errorf("%s matched no packages", strings.Join(patterns, " ")) } return initial, err } -// loadingError checks for issues during the loading of initial -// packages. Returns nil if there are no issues. Returns error -// of type typeParseError if all errors, including those in -// dependencies, are related to typing or parsing. Otherwise, -// a plain error is returned with an appropriate message. -func loadingError(initial []*packages.Package) error { - var err error - if n := packages.PrintErrors(initial); n > 1 { - err = fmt.Errorf("%d errors during loading", n) - } else if n == 1 { - err = errors.New("error during loading") - } else { - // no errors - return nil - } - all := true - packages.Visit(initial, nil, func(pkg *packages.Package) { - for _, err := range pkg.Errors { - typeOrParse := err.Kind == packages.TypeError || err.Kind == packages.ParseError - all = all && typeOrParse - } - }) - if all { - return typeParseError{err} - } - return err -} - // TestAnalyzer applies an analyzer to a set of packages (and their // dependencies if necessary) and returns the results. // The analyzer must be valid according to [analysis.Validate]. diff --git a/go/analysis/internal/checker/checker_test.go b/go/analysis/internal/checker/checker_test.go index 69ea944d888..46c74d72328 100644 --- a/go/analysis/internal/checker/checker_test.go +++ b/go/analysis/internal/checker/checker_test.go @@ -68,10 +68,11 @@ func Foo() { } var renameAnalyzer = &analysis.Analyzer{ - Name: "rename", - Requires: []*analysis.Analyzer{inspect.Analyzer}, - Run: run, - Doc: "renames symbols named bar to baz", + Name: "rename", + Requires: []*analysis.Analyzer{inspect.Analyzer}, + Run: run, + Doc: "renames symbols named bar to baz", + RunDespiteErrors: true, } var otherAnalyzer = &analysis.Analyzer{ // like analyzer but with a different Name. @@ -140,13 +141,28 @@ func TestRunDespiteErrors(t *testing.T) { func Foo(s string) int { return s + 1 } -`} +`, + "cperr/test.go": `package copyerr + +import "sync" + +func bar() { } // for renameAnalyzer + +type T struct{ mu sync.Mutex } +type T1 struct{ t *T } + +func NewT1() *T1 { return &T1{T} } +`, + } testdata, cleanup, err := analysistest.WriteFiles(files) if err != nil { t.Fatal(err) } - path := filepath.Join(testdata, "src/rderr/test.go") + defer cleanup() + + rderrFile := "file=" + filepath.Join(testdata, "src/rderr/test.go") + cperrFile := "file=" + filepath.Join(testdata, "src/cperr/test.go") // A no-op analyzer that should finish regardless of // parse or type errors in the code. @@ -159,8 +175,8 @@ func Foo(s string) int { RunDespiteErrors: true, } - // A no-op analyzer that should finish regardless of - // parse or type errors in the code. + // A no-op analyzer, with facts, that should finish + // regardless of parse or type errors in the code. noopWithFact := &analysis.Analyzer{ Name: "noopfact", Requires: []*analysis.Analyzer{inspect.Analyzer}, @@ -178,30 +194,34 @@ func Foo(s string) int { code int }{ // parse/type errors - {name: "skip-error", pattern: []string{"file=" + path}, analyzers: []*analysis.Analyzer{renameAnalyzer}, code: 1}, + {name: "skip-error", pattern: []string{rderrFile}, analyzers: []*analysis.Analyzer{renameAnalyzer}, code: 1}, // RunDespiteErrors allows a driver to run an Analyzer even after parse/type errors. // // The noop analyzer doesn't use facts, so the driver loads only the root // package from source. For the rest, it asks 'go list' for export data, // which fails because the compiler encounters the type error. Since the // errors come from 'go list', the driver doesn't run the analyzer. - {name: "despite-error", pattern: []string{"file=" + path}, analyzers: []*analysis.Analyzer{noop}, code: 1}, + {name: "despite-error", pattern: []string{rderrFile}, analyzers: []*analysis.Analyzer{noop}, code: 1}, // The noopfact analyzer does use facts, so the driver loads source for // all dependencies, does type checking itself, recognizes the error as a // type error, and runs the analyzer. - {name: "despite-error-fact", pattern: []string{"file=" + path}, analyzers: []*analysis.Analyzer{noopWithFact}, code: 0}, + {name: "despite-error-fact", pattern: []string{rderrFile}, analyzers: []*analysis.Analyzer{noopWithFact}, code: 1}, // combination of parse/type errors and no errors - {name: "despite-error-and-no-error", pattern: []string{"file=" + path, "sort"}, analyzers: []*analysis.Analyzer{renameAnalyzer, noop}, code: 1}, + {name: "despite-error-and-no-error", pattern: []string{rderrFile, "sort"}, analyzers: []*analysis.Analyzer{renameAnalyzer, noop}, code: 1}, // non-existing package error {name: "no-package", pattern: []string{"xyz"}, analyzers: []*analysis.Analyzer{renameAnalyzer}, code: 1}, {name: "no-package-despite-error", pattern: []string{"abc"}, analyzers: []*analysis.Analyzer{noop}, code: 1}, {name: "no-multi-package-despite-error", pattern: []string{"xyz", "abc"}, analyzers: []*analysis.Analyzer{noop}, code: 1}, // combination of type/parsing and different errors - {name: "different-errors", pattern: []string{"file=" + path, "xyz"}, analyzers: []*analysis.Analyzer{renameAnalyzer, noop}, code: 1}, + {name: "different-errors", pattern: []string{rderrFile, "xyz"}, analyzers: []*analysis.Analyzer{renameAnalyzer, noop}, code: 1}, // non existing dir error {name: "no-match-dir", pattern: []string{"file=non/existing/dir"}, analyzers: []*analysis.Analyzer{renameAnalyzer, noop}, code: 1}, // no errors {name: "no-errors", pattern: []string{"sort"}, analyzers: []*analysis.Analyzer{renameAnalyzer, noop}, code: 0}, + // duplicate list error with no findings + {name: "list-error", pattern: []string{cperrFile}, analyzers: []*analysis.Analyzer{noop}, code: 1}, + // duplicate list errors with findings (issue #67790) + {name: "list-error-findings", pattern: []string{cperrFile}, analyzers: []*analysis.Analyzer{renameAnalyzer}, code: 3}, } { if test.name == "despite-error" && testenv.Go1Point() < 20 { // The behavior in the comment on the despite-error test only occurs for Go 1.20+. @@ -211,8 +231,6 @@ func Foo(s string) int { t.Errorf("got incorrect exit code %d for test %s; want %d", got, test.name, test.code) } } - - defer cleanup() } type EmptyFact struct{} diff --git a/go/analysis/passes/asmdecl/asmdecl.go b/go/analysis/passes/asmdecl/asmdecl.go index 3417232ce35..c9ba1a375d3 100644 --- a/go/analysis/passes/asmdecl/asmdecl.go +++ b/go/analysis/passes/asmdecl/asmdecl.go @@ -96,7 +96,7 @@ var ( asmArchRISCV64 = asmArch{name: "riscv64", bigEndian: false, stack: "SP", lr: true, retRegs: []string{"X10", "F10"}} asmArchS390X = asmArch{name: "s390x", bigEndian: true, stack: "R15", lr: true} asmArchWasm = asmArch{name: "wasm", bigEndian: false, stack: "SP", lr: false} - asmArchLoong64 = asmArch{name: "loong64", bigEndian: false, stack: "R3", lr: true} + asmArchLoong64 = asmArch{name: "loong64", bigEndian: false, stack: "R3", lr: true, retRegs: []string{"R4", "F0"}} arches = []*asmArch{ &asmArch386, diff --git a/go/analysis/passes/buildtag/buildtag.go b/go/analysis/passes/buildtag/buildtag.go index 51ba2a91e5b..5b4cf9d9edc 100644 --- a/go/analysis/passes/buildtag/buildtag.go +++ b/go/analysis/passes/buildtag/buildtag.go @@ -2,9 +2,6 @@ // Use of this source code is governed by a BSD-style // license that can be found in the LICENSE file. -//go:build go1.16 -// +build go1.16 - // Package buildtag defines an Analyzer that checks build tags. package buildtag diff --git a/go/analysis/passes/buildtag/buildtag_old.go b/go/analysis/passes/buildtag/buildtag_old.go deleted file mode 100644 index 19ef6b9bce4..00000000000 --- a/go/analysis/passes/buildtag/buildtag_old.go +++ /dev/null @@ -1,174 +0,0 @@ -// Copyright 2013 The Go Authors. All rights reserved. -// Use of this source code is governed by a BSD-style -// license that can be found in the LICENSE file. - -// TODO(rsc): Delete this file once Go 1.17 comes out and we can retire Go 1.15 support. - -//go:build !go1.16 -// +build !go1.16 - -// Package buildtag defines an Analyzer that checks build tags. -package buildtag - -import ( - "bytes" - "fmt" - "go/ast" - "go/parser" - "strings" - "unicode" - - "golang.org/x/tools/go/analysis" - "golang.org/x/tools/go/analysis/passes/internal/analysisutil" -) - -const Doc = "check // +build directives" - -var Analyzer = &analysis.Analyzer{ - Name: "buildtag", - Doc: Doc, - Run: runBuildTag, -} - -func runBuildTag(pass *analysis.Pass) (interface{}, error) { - for _, f := range pass.Files { - checkGoFile(pass, f) - } - for _, name := range pass.OtherFiles { - if err := checkOtherFile(pass, name); err != nil { - return nil, err - } - } - for _, name := range pass.IgnoredFiles { - if strings.HasSuffix(name, ".go") { - f, err := parser.ParseFile(pass.Fset, name, nil, parser.ParseComments) - if err != nil { - // Not valid Go source code - not our job to diagnose, so ignore. - return nil, nil - } - checkGoFile(pass, f) - } else { - if err := checkOtherFile(pass, name); err != nil { - return nil, err - } - } - } - return nil, nil -} - -func checkGoFile(pass *analysis.Pass, f *ast.File) { - pastCutoff := false - for _, group := range f.Comments { - // A +build comment is ignored after or adjoining the package declaration. - if group.End()+1 >= f.Package { - pastCutoff = true - } - - // "+build" is ignored within or after a /*...*/ comment. - if !strings.HasPrefix(group.List[0].Text, "//") { - pastCutoff = true - continue - } - - // Check each line of a //-comment. - for _, c := range group.List { - if !strings.Contains(c.Text, "+build") { - continue - } - if err := checkLine(c.Text, pastCutoff); err != nil { - pass.Reportf(c.Pos(), "%s", err) - } - } - } -} - -func checkOtherFile(pass *analysis.Pass, filename string) error { - content, tf, err := analysisutil.ReadFile(pass, filename) - if err != nil { - return err - } - - // We must look at the raw lines, as build tags may appear in non-Go - // files such as assembly files. - lines := bytes.SplitAfter(content, nl) - - // Determine cutpoint where +build comments are no longer valid. - // They are valid in leading // comments in the file followed by - // a blank line. - // - // This must be done as a separate pass because of the - // requirement that the comment be followed by a blank line. - var cutoff int - for i, line := range lines { - line = bytes.TrimSpace(line) - if !bytes.HasPrefix(line, slashSlash) { - if len(line) > 0 { - break - } - cutoff = i - } - } - - for i, line := range lines { - line = bytes.TrimSpace(line) - if !bytes.HasPrefix(line, slashSlash) { - continue - } - if !bytes.Contains(line, []byte("+build")) { - continue - } - if err := checkLine(string(line), i >= cutoff); err != nil { - pass.Reportf(analysisutil.LineStart(tf, i+1), "%s", err) - continue - } - } - return nil -} - -// checkLine checks a line that starts with "//" and contains "+build". -func checkLine(line string, pastCutoff bool) error { - line = strings.TrimPrefix(line, "//") - line = strings.TrimSpace(line) - - if strings.HasPrefix(line, "+build") { - fields := strings.Fields(line) - if fields[0] != "+build" { - // Comment is something like +buildasdf not +build. - return fmt.Errorf("possible malformed +build comment") - } - if pastCutoff { - return fmt.Errorf("+build comment must appear before package clause and be followed by a blank line") - } - if err := checkArguments(fields); err != nil { - return err - } - } else { - // Comment with +build but not at beginning. - if !pastCutoff { - return fmt.Errorf("possible malformed +build comment") - } - } - return nil -} - -func checkArguments(fields []string) error { - for _, arg := range fields[1:] { - for _, elem := range strings.Split(arg, ",") { - if strings.HasPrefix(elem, "!!") { - return fmt.Errorf("invalid double negative in build constraint: %s", arg) - } - elem = strings.TrimPrefix(elem, "!") - for _, c := range elem { - if !unicode.IsLetter(c) && !unicode.IsDigit(c) && c != '_' && c != '.' { - return fmt.Errorf("invalid non-alphanumeric build constraint: %s", arg) - } - } - } - } - return nil -} - -var ( - nl = []byte("\n") - slashSlash = []byte("//") -) diff --git a/go/analysis/passes/buildtag/buildtag_test.go b/go/analysis/passes/buildtag/buildtag_test.go index 163e8e30da3..110343ceac6 100644 --- a/go/analysis/passes/buildtag/buildtag_test.go +++ b/go/analysis/passes/buildtag/buildtag_test.go @@ -5,8 +5,6 @@ package buildtag_test import ( - "runtime" - "strings" "testing" "golang.org/x/tools/go/analysis" @@ -15,9 +13,6 @@ import ( ) func Test(t *testing.T) { - if strings.HasPrefix(runtime.Version(), "go1.") && runtime.Version() < "go1.16" { - t.Skipf("skipping on %v", runtime.Version()) - } analyzer := *buildtag.Analyzer analyzer.Run = func(pass *analysis.Pass) (interface{}, error) { defer func() { diff --git a/go/analysis/passes/fieldalignment/fieldalignment.go b/go/analysis/passes/fieldalignment/fieldalignment.go index 012e2ecd0c9..8af717b4c6c 100644 --- a/go/analysis/passes/fieldalignment/fieldalignment.go +++ b/go/analysis/passes/fieldalignment/fieldalignment.go @@ -46,6 +46,15 @@ Be aware that the most compact order is not always the most efficient. In rare cases it may cause two variables each updated by its own goroutine to occupy the same CPU cache line, inducing a form of memory contention known as "false sharing" that slows down both goroutines. + +Unlike most analyzers, which report likely mistakes, the diagnostics +produced by fieldanalyzer very rarely indicate a significant problem, +so the analyzer is not included in typical suites such as vet or +gopls. Use this standalone command to run it on your code: + + $ go install golang.org/x/tools/go/analysis/passes/fieldalignment/cmd/fieldalignment@latest + $ go fieldalignment [packages] + ` var Analyzer = &analysis.Analyzer{ diff --git a/go/analysis/passes/stringintconv/string.go b/go/analysis/passes/stringintconv/string.go index c77182daef6..a3afbf696e1 100644 --- a/go/analysis/passes/stringintconv/string.go +++ b/go/analysis/passes/stringintconv/string.go @@ -16,6 +16,7 @@ import ( "golang.org/x/tools/go/analysis/passes/internal/analysisutil" "golang.org/x/tools/go/ast/inspector" "golang.org/x/tools/internal/aliases" + "golang.org/x/tools/internal/analysisinternal" "golang.org/x/tools/internal/typeparams" ) @@ -73,9 +74,15 @@ func typeName(t types.Type) string { func run(pass *analysis.Pass) (interface{}, error) { inspect := pass.ResultOf[inspect.Analyzer].(*inspector.Inspector) nodeFilter := []ast.Node{ + (*ast.File)(nil), (*ast.CallExpr)(nil), } + var file *ast.File inspect.Preorder(nodeFilter, func(n ast.Node) { + if n, ok := n.(*ast.File); ok { + file = n + return + } call := n.(*ast.CallExpr) if len(call.Args) != 1 { @@ -167,27 +174,72 @@ func run(pass *analysis.Pass) (interface{}, error) { diag := analysis.Diagnostic{ Pos: n.Pos(), - Message: fmt.Sprintf("conversion from %s to %s yields a string of one rune, not a string of digits (did you mean fmt.Sprint(x)?)", source, target), + Message: fmt.Sprintf("conversion from %s to %s yields a string of one rune, not a string of digits", source, target), + } + addFix := func(message string, edits []analysis.TextEdit) { + diag.SuggestedFixes = append(diag.SuggestedFixes, analysis.SuggestedFix{ + Message: message, + TextEdits: edits, + }) } + // Fix 1: use fmt.Sprint(x) + // + // Prefer fmt.Sprint over strconv.Itoa, FormatInt, + // or FormatUint, as it works for any type. + // Add an import of "fmt" as needed. + // + // Unless the type is exactly string, we must retain the conversion. + // + // Do not offer this fix if type parameters are involved, + // as there are too many combinations and subtleties. + // Consider x = rune | int16 | []byte: in all cases, + // string(x) is legal, but the appropriate diagnostic + // and fix differs. Similarly, don't offer the fix if + // the type has methods, as some {String,GoString,Format} + // may change the behavior of fmt.Sprint. + if len(ttypes) == 1 && len(vtypes) == 1 && types.NewMethodSet(V0).Len() == 0 { + fmtName, importEdits := analysisinternal.AddImport(pass.TypesInfo, file, arg.Pos(), "fmt", "fmt") + if types.Identical(T0, types.Typ[types.String]) { + // string(x) -> fmt.Sprint(x) + addFix("Format the number as a decimal", append(importEdits, + analysis.TextEdit{ + Pos: call.Fun.Pos(), + End: call.Fun.End(), + NewText: []byte(fmtName + ".Sprint"), + }), + ) + } else { + // mystring(x) -> mystring(fmt.Sprint(x)) + addFix("Format the number as a decimal", append(importEdits, + analysis.TextEdit{ + Pos: call.Lparen + 1, + End: call.Lparen + 1, + NewText: []byte(fmtName + ".Sprint("), + }, + analysis.TextEdit{ + Pos: call.Rparen, + End: call.Rparen, + NewText: []byte(")"), + }), + ) + } + } + + // Fix 2: use string(rune(x)) if convertibleToRune { - diag.SuggestedFixes = []analysis.SuggestedFix{ + addFix("Convert a single rune to a string", []analysis.TextEdit{ { - Message: "Did you mean to convert a rune to a string?", - TextEdits: []analysis.TextEdit{ - { - Pos: arg.Pos(), - End: arg.Pos(), - NewText: []byte("rune("), - }, - { - Pos: arg.End(), - End: arg.End(), - NewText: []byte(")"), - }, - }, + Pos: arg.Pos(), + End: arg.Pos(), + NewText: []byte("rune("), }, - } + { + Pos: arg.End(), + End: arg.End(), + NewText: []byte(")"), + }, + }) } pass.Report(diag) }) diff --git a/go/analysis/passes/stringintconv/string_test.go b/go/analysis/passes/stringintconv/string_test.go index 0b1f99bf862..a86e7fedebf 100644 --- a/go/analysis/passes/stringintconv/string_test.go +++ b/go/analysis/passes/stringintconv/string_test.go @@ -13,5 +13,6 @@ import ( func Test(t *testing.T) { testdata := analysistest.TestData() - analysistest.RunWithSuggestedFixes(t, testdata, stringintconv.Analyzer, "a", "typeparams") + analysistest.Run(t, testdata, stringintconv.Analyzer, "a", "typeparams") + analysistest.RunWithSuggestedFixes(t, testdata, stringintconv.Analyzer, "fix") } diff --git a/go/analysis/passes/stringintconv/testdata/src/a/a.go b/go/analysis/passes/stringintconv/testdata/src/a/a.go index 236626260fa..c1a9fc26a7f 100644 --- a/go/analysis/passes/stringintconv/testdata/src/a/a.go +++ b/go/analysis/passes/stringintconv/testdata/src/a/a.go @@ -25,12 +25,13 @@ func StringTest() { o struct{ x int } ) const p = 0 - _ = string(i) // want `^conversion from int to string yields a string of one rune, not a string of digits \(did you mean fmt\.Sprint\(x\)\?\)$` + // First time only, assert the complete message: + _ = string(i) // want `^conversion from int to string yields a string of one rune, not a string of digits$` _ = string(j) _ = string(k) - _ = string(p) // want `^conversion from untyped int to string yields a string of one rune, not a string of digits \(did you mean fmt\.Sprint\(x\)\?\)$` - _ = A(l) // want `^conversion from C \(int\) to A \(string\) yields a string of one rune, not a string of digits \(did you mean fmt\.Sprint\(x\)\?\)$` - _ = B(m) // want `^conversion from (uintptr|D \(uintptr\)) to B \(string\) yields a string of one rune, not a string of digits \(did you mean fmt\.Sprint\(x\)\?\)$` - _ = string(n[1]) // want `^conversion from int to string yields a string of one rune, not a string of digits \(did you mean fmt\.Sprint\(x\)\?\)$` - _ = string(o.x) // want `^conversion from int to string yields a string of one rune, not a string of digits \(did you mean fmt\.Sprint\(x\)\?\)$` + _ = string(p) // want `...from untyped int to string...` + _ = A(l) // want `...from C \(int\) to A \(string\)...` + _ = B(m) // want `...from (uintptr|D \(uintptr\)) to B \(string\)...` + _ = string(n[1]) // want `...from int to string...` + _ = string(o.x) // want `...from int to string...` } diff --git a/go/analysis/passes/stringintconv/testdata/src/a/a.go.golden b/go/analysis/passes/stringintconv/testdata/src/a/a.go.golden deleted file mode 100644 index bccc7a43b0b..00000000000 --- a/go/analysis/passes/stringintconv/testdata/src/a/a.go.golden +++ /dev/null @@ -1,36 +0,0 @@ -// Copyright 2020 The Go Authors. All rights reserved. -// Use of this source code is governed by a BSD-style -// license that can be found in the LICENSE file. - -// This file contains tests for the stringintconv checker. - -package a - -type A string - -type B = string - -type C int - -type D = uintptr - -func StringTest() { - var ( - i int - j rune - k byte - l C - m D - n = []int{0, 1, 2} - o struct{ x int } - ) - const p = 0 - _ = string(rune(i)) // want `^conversion from int to string yields a string of one rune, not a string of digits \(did you mean fmt\.Sprint\(x\)\?\)$` - _ = string(j) - _ = string(k) - _ = string(rune(p)) // want `^conversion from untyped int to string yields a string of one rune, not a string of digits \(did you mean fmt\.Sprint\(x\)\?\)$` - _ = A(rune(l)) // want `^conversion from C \(int\) to A \(string\) yields a string of one rune, not a string of digits \(did you mean fmt\.Sprint\(x\)\?\)$` - _ = B(rune(m)) // want `^conversion from (uintptr|D \(uintptr\)) to B \(string\) yields a string of one rune, not a string of digits \(did you mean fmt\.Sprint\(x\)\?\)$` - _ = string(rune(n[1])) // want `^conversion from int to string yields a string of one rune, not a string of digits \(did you mean fmt\.Sprint\(x\)\?\)$` - _ = string(rune(o.x)) // want `^conversion from int to string yields a string of one rune, not a string of digits \(did you mean fmt\.Sprint\(x\)\?\)$` -} diff --git a/go/analysis/passes/stringintconv/testdata/src/fix/fix.go b/go/analysis/passes/stringintconv/testdata/src/fix/fix.go new file mode 100644 index 00000000000..50f718e2db3 --- /dev/null +++ b/go/analysis/passes/stringintconv/testdata/src/fix/fix.go @@ -0,0 +1,5 @@ +package fix + +func _(x uint64) { + println(string(x)) // want `conversion from uint64 to string yields...` +} diff --git a/go/analysis/passes/stringintconv/testdata/src/fix/fix.go.golden b/go/analysis/passes/stringintconv/testdata/src/fix/fix.go.golden new file mode 100644 index 00000000000..66ecdf0e2fb --- /dev/null +++ b/go/analysis/passes/stringintconv/testdata/src/fix/fix.go.golden @@ -0,0 +1,16 @@ +-- Format the number as a decimal -- +package fix + +import "fmt" + +func _(x uint64) { + println(fmt.Sprint(x)) // want `conversion from uint64 to string yields...` +} + +-- Convert a single rune to a string -- +package fix + +func _(x uint64) { + println(string(rune(x))) // want `conversion from uint64 to string yields...` +} + diff --git a/go/analysis/passes/stringintconv/testdata/src/fix/fixnamed.go b/go/analysis/passes/stringintconv/testdata/src/fix/fixnamed.go new file mode 100644 index 00000000000..f77d5e171b9 --- /dev/null +++ b/go/analysis/passes/stringintconv/testdata/src/fix/fixnamed.go @@ -0,0 +1,7 @@ +package fix + +type mystring string + +func _(x int16) mystring { + return mystring(x) // want `conversion from int16 to mystring \(string\)...` +} diff --git a/go/analysis/passes/stringintconv/testdata/src/fix/fixnamed.go.golden b/go/analysis/passes/stringintconv/testdata/src/fix/fixnamed.go.golden new file mode 100644 index 00000000000..e3a80a0185b --- /dev/null +++ b/go/analysis/passes/stringintconv/testdata/src/fix/fixnamed.go.golden @@ -0,0 +1,19 @@ +-- Format the number as a decimal -- +package fix + +import "fmt" + +type mystring string + +func _(x int16) mystring { + return mystring(fmt.Sprint(x)) // want `conversion from int16 to mystring \(string\)...` +} + +-- Convert a single rune to a string -- +package fix + +type mystring string + +func _(x int16) mystring { + return mystring(rune(x)) // want `conversion from int16 to mystring \(string\)...` +} diff --git a/go/analysis/passes/stringintconv/testdata/src/typeparams/typeparams.go b/go/analysis/passes/stringintconv/testdata/src/typeparams/typeparams.go index b50aa3f4424..7a218e2924a 100644 --- a/go/analysis/passes/stringintconv/testdata/src/typeparams/typeparams.go +++ b/go/analysis/passes/stringintconv/testdata/src/typeparams/typeparams.go @@ -22,16 +22,16 @@ func _[AllString ~string, MaybeString ~string | ~int, NotString ~int | byte, Nam ) const p = 0 - _ = MaybeString(i) // want `conversion from int to string .in MaybeString. yields a string of one rune, not a string of digits .did you mean fmt\.Sprint.x.\?.` + _ = MaybeString(i) // want `conversion from int to string .in MaybeString. yields a string of one rune, not a string of digits` _ = MaybeString(r) _ = MaybeString(b) - _ = MaybeString(I) // want `conversion from Int .int. to string .in MaybeString. yields a string of one rune, not a string of digits .did you mean fmt\.Sprint.x.\?.` - _ = MaybeString(U) // want `conversion from uintptr to string .in MaybeString. yields a string of one rune, not a string of digits .did you mean fmt\.Sprint.x.\?.` + _ = MaybeString(I) // want `conversion from Int .int. to string .in MaybeString. yields a string of one rune, not a string of digits` + _ = MaybeString(U) // want `conversion from uintptr to string .in MaybeString. yields a string of one rune, not a string of digits` // Type parameters are never constant types, so arguments are always // converted to their default type (int versus untyped int, in this case) - _ = MaybeString(p) // want `conversion from int to string .in MaybeString. yields a string of one rune, not a string of digits .did you mean fmt\.Sprint.x.\?.` + _ = MaybeString(p) // want `conversion from int to string .in MaybeString. yields a string of one rune, not a string of digits` // ...even if the type parameter is only strings. - _ = AllString(p) // want `conversion from int to string .in AllString. yields a string of one rune, not a string of digits .did you mean fmt\.Sprint.x.\?.` + _ = AllString(p) // want `conversion from int to string .in AllString. yields a string of one rune, not a string of digits` _ = NotString(i) _ = NotString(r) @@ -40,10 +40,10 @@ func _[AllString ~string, MaybeString ~string | ~int, NotString ~int | byte, Nam _ = NotString(U) _ = NotString(p) - _ = NamedString(i) // want `conversion from int to String .string, in NamedString. yields a string of one rune, not a string of digits .did you mean fmt\.Sprint.x.\?.` - _ = string(M) // want `conversion from int .in MaybeString. to string yields a string of one rune, not a string of digits .did you mean fmt\.Sprint.x.\?.` + _ = NamedString(i) // want `conversion from int to String .string, in NamedString. yields a string of one rune, not a string of digits` + _ = string(M) // want `conversion from int .in MaybeString. to string yields a string of one rune, not a string of digits` // Note that M is not convertible to rune. - _ = MaybeString(M) // want `conversion from int .in MaybeString. to string .in MaybeString. yields a string of one rune, not a string of digits .did you mean fmt\.Sprint.x.\?.` + _ = MaybeString(M) // want `conversion from int .in MaybeString. to string .in MaybeString. yields a string of one rune, not a string of digits` _ = NotString(N) // ok } diff --git a/go/analysis/passes/stringintconv/testdata/src/typeparams/typeparams.go.golden b/go/analysis/passes/stringintconv/testdata/src/typeparams/typeparams.go.golden deleted file mode 100644 index 8a78530243d..00000000000 --- a/go/analysis/passes/stringintconv/testdata/src/typeparams/typeparams.go.golden +++ /dev/null @@ -1,49 +0,0 @@ -// Copyright 2021 The Go Authors. All rights reserved. -// Use of this source code is governed by a BSD-style -// license that can be found in the LICENSE file. - -package typeparams - -type ( - Int int - Uintptr = uintptr - String string -) - -func _[AllString ~string, MaybeString ~string | ~int, NotString ~int | byte, NamedString String | Int]() { - var ( - i int - r rune - b byte - I Int - U uintptr - M MaybeString - N NotString - ) - const p = 0 - - _ = MaybeString(rune(i)) // want `conversion from int to string .in MaybeString. yields a string of one rune, not a string of digits .did you mean fmt\.Sprint.x.\?.` - _ = MaybeString(r) - _ = MaybeString(b) - _ = MaybeString(rune(I)) // want `conversion from Int .int. to string .in MaybeString. yields a string of one rune, not a string of digits .did you mean fmt\.Sprint.x.\?.` - _ = MaybeString(rune(U)) // want `conversion from uintptr to string .in MaybeString. yields a string of one rune, not a string of digits .did you mean fmt\.Sprint.x.\?.` - // Type parameters are never constant types, so arguments are always - // converted to their default type (int versus untyped int, in this case) - _ = MaybeString(rune(p)) // want `conversion from int to string .in MaybeString. yields a string of one rune, not a string of digits .did you mean fmt\.Sprint.x.\?.` - // ...even if the type parameter is only strings. - _ = AllString(rune(p)) // want `conversion from int to string .in AllString. yields a string of one rune, not a string of digits .did you mean fmt\.Sprint.x.\?.` - - _ = NotString(i) - _ = NotString(r) - _ = NotString(b) - _ = NotString(I) - _ = NotString(U) - _ = NotString(p) - - _ = NamedString(rune(i)) // want `conversion from int to String .string, in NamedString. yields a string of one rune, not a string of digits .did you mean fmt\.Sprint.x.\?.` - _ = string(M) // want `conversion from int .in MaybeString. to string yields a string of one rune, not a string of digits .did you mean fmt\.Sprint.x.\?.` - - // Note that M is not convertible to rune. - _ = MaybeString(M) // want `conversion from int .in MaybeString. to string .in MaybeString. yields a string of one rune, not a string of digits .did you mean fmt\.Sprint.x.\?.` - _ = NotString(N) // ok -} diff --git a/go/ast/astutil/enclosing.go b/go/ast/astutil/enclosing.go index 2c4c4e23289..6e34df46130 100644 --- a/go/ast/astutil/enclosing.go +++ b/go/ast/astutil/enclosing.go @@ -106,8 +106,21 @@ func PathEnclosingInterval(root *ast.File, start, end token.Pos) (path []ast.Nod // Does augmented child strictly contain [start, end)? if augPos <= start && end <= augEnd { - _, isToken := child.(tokenNode) - return isToken || visit(child) + if is[tokenNode](child) { + return true + } + + // childrenOf elides the FuncType node beneath FuncDecl. + // Add it back here for TypeParams, Params, Results, + // all FieldLists). But we don't add it back for the "func" token + // even though it is is the tree at FuncDecl.Type.Func. + if decl, ok := node.(*ast.FuncDecl); ok { + if fields, ok := child.(*ast.FieldList); ok && fields != decl.Recv { + path = append(path, decl.Type) + } + } + + return visit(child) } // Does [start, end) overlap multiple children? @@ -313,6 +326,8 @@ func childrenOf(n ast.Node) []ast.Node { // // As a workaround, we inline the case for FuncType // here and order things correctly. + // We also need to insert the elided FuncType just + // before the 'visit' recursion. // children = nil // discard ast.Walk(FuncDecl) info subtrees children = append(children, tok(n.Type.Func, len("func"))) @@ -632,3 +647,8 @@ func NodeDescription(n ast.Node) string { } panic(fmt.Sprintf("unexpected node type: %T", n)) } + +func is[T any](x any) bool { + _, ok := x.(T) + return ok +} diff --git a/go/ast/astutil/enclosing_test.go b/go/ast/astutil/enclosing_test.go index 1f9d06ce1f8..09483c4c8f4 100644 --- a/go/ast/astutil/enclosing_test.go +++ b/go/ast/astutil/enclosing_test.go @@ -72,6 +72,8 @@ func g[A any, P interface{ctype1| ~ctype2}](a1 A, p1 P) {} type PT[T constraint] struct{ t T } +func (r recv) method(p param) {} + var v GT[targ1] var h = g[ targ2, targ3] @@ -204,14 +206,16 @@ func TestPathEnclosingInterval_Paths(t *testing.T) { "[Ident File],true"}, {"f() // NB", "[CallExpr ExprStmt BlockStmt FuncDecl File],true"}, - {" any", "[Ident Field FieldList FuncDecl File],true"}, - {"|", "[BinaryExpr Field FieldList InterfaceType Field FieldList FuncDecl File],true"}, + {" any", "[Ident Field FieldList FuncType FuncDecl File],true"}, + {"|", "[BinaryExpr Field FieldList InterfaceType Field FieldList FuncType FuncDecl File],true"}, {"ctype2", - "[Ident UnaryExpr BinaryExpr Field FieldList InterfaceType Field FieldList FuncDecl File],true"}, - {"a1", "[Ident Field FieldList FuncDecl File],true"}, + "[Ident UnaryExpr BinaryExpr Field FieldList InterfaceType Field FieldList FuncType FuncDecl File],true"}, + {"a1", "[Ident Field FieldList FuncType FuncDecl File],true"}, {"PT[T constraint]", "[TypeSpec GenDecl File],false"}, {"[T constraint]", "[FieldList TypeSpec GenDecl File],true"}, {"targ2", "[Ident IndexListExpr ValueSpec GenDecl File],true"}, + {"p param", "[Field FieldList FuncType FuncDecl File],true"}, // FuncType is present for FuncDecl.Params (etc) + {"r recv", "[Field FieldList FuncDecl File],true"}, // no FuncType for FuncDecl.Recv } for _, test := range tests { f, start, end := findInterval(t, new(token.FileSet), input, test.substr) diff --git a/go/ast/astutil/util.go b/go/ast/astutil/util.go index 919d5305ab4..6bdcf70ac27 100644 --- a/go/ast/astutil/util.go +++ b/go/ast/astutil/util.go @@ -7,6 +7,7 @@ package astutil import "go/ast" // Unparen returns e with any enclosing parentheses stripped. +// TODO(adonovan): use go1.22's ast.Unparen. func Unparen(e ast.Expr) ast.Expr { for { p, ok := e.(*ast.ParenExpr) diff --git a/go/callgraph/vta/helpers_test.go b/go/callgraph/vta/helpers_test.go index 1a539e69b63..64e26f1a479 100644 --- a/go/callgraph/vta/helpers_test.go +++ b/go/callgraph/vta/helpers_test.go @@ -99,7 +99,7 @@ func callGraphStr(g *callgraph.Graph) []string { for f, n := range g.Nodes { c := make(map[string][]string) for _, edge := range n.Out { - cs := edge.Site.String() + cs := edge.Site.String() // TODO(adonovan): handle Site=nil gracefully c[cs] = append(c[cs], funcName(edge.Callee.Func)) } diff --git a/go/internal/packagesdriver/sizes.go b/go/internal/packagesdriver/sizes.go deleted file mode 100644 index c6e7c0d442f..00000000000 --- a/go/internal/packagesdriver/sizes.go +++ /dev/null @@ -1,54 +0,0 @@ -// Copyright 2018 The Go Authors. All rights reserved. -// Use of this source code is governed by a BSD-style -// license that can be found in the LICENSE file. - -// Package packagesdriver fetches type sizes for go/packages and go/analysis. -package packagesdriver - -import ( - "context" - "fmt" - "strings" - - "golang.org/x/tools/internal/gocommand" -) - -// TODO(adonovan): move back into go/packages. -func GetSizesForArgsGolist(ctx context.Context, inv gocommand.Invocation, gocmdRunner *gocommand.Runner) (string, string, error) { - inv.Verb = "list" - inv.Args = []string{"-f", "{{context.GOARCH}} {{context.Compiler}}", "--", "unsafe"} - stdout, stderr, friendlyErr, rawErr := gocmdRunner.RunRaw(ctx, inv) - var goarch, compiler string - if rawErr != nil { - rawErrMsg := rawErr.Error() - if strings.Contains(rawErrMsg, "cannot find main module") || - strings.Contains(rawErrMsg, "go.mod file not found") { - // User's running outside of a module. - // All bets are off. Get GOARCH and guess compiler is gc. - // TODO(matloob): Is this a problem in practice? - inv.Verb = "env" - inv.Args = []string{"GOARCH"} - envout, enverr := gocmdRunner.Run(ctx, inv) - if enverr != nil { - return "", "", enverr - } - goarch = strings.TrimSpace(envout.String()) - compiler = "gc" - } else if friendlyErr != nil { - return "", "", friendlyErr - } else { - // This should be unreachable, but be defensive - // in case RunRaw's error results are inconsistent. - return "", "", rawErr - } - } else { - fields := strings.Fields(stdout.String()) - if len(fields) < 2 { - return "", "", fmt.Errorf("could not parse GOARCH and Go compiler in format \" \":\nstdout: <<%s>>\nstderr: <<%s>>", - stdout.String(), stderr.String()) - } - goarch = fields[0] - compiler = fields[1] - } - return compiler, goarch, nil -} diff --git a/go/loader/loader_test.go b/go/loader/loader_test.go index 1e0b16e7fc3..4729ba34559 100644 --- a/go/loader/loader_test.go +++ b/go/loader/loader_test.go @@ -118,6 +118,10 @@ func TestLoad_NoInitialPackages(t *testing.T) { } func TestLoad_MissingInitialPackage(t *testing.T) { + if runtime.GOOS == "wasip1" { + t.Skip("Skipping due to golang/go#64725: fails with EBADF errors") + } + var conf loader.Config conf.Import("nosuchpkg") conf.Import("errors") diff --git a/go/packages/golist.go b/go/packages/golist.go index d9be410aa1a..1a3a5b44f5c 100644 --- a/go/packages/golist.go +++ b/go/packages/golist.go @@ -21,7 +21,6 @@ import ( "sync" "unicode" - "golang.org/x/tools/go/internal/packagesdriver" "golang.org/x/tools/internal/gocommand" "golang.org/x/tools/internal/packagesinternal" ) @@ -149,7 +148,7 @@ func goListDriver(cfg *Config, patterns ...string) (_ *DriverResponse, err error if cfg.Mode&NeedTypesSizes != 0 || cfg.Mode&NeedTypes != 0 { errCh := make(chan error) go func() { - compiler, arch, err := packagesdriver.GetSizesForArgsGolist(ctx, state.cfgInvocation(), cfg.gocmdRunner) + compiler, arch, err := getSizesForArgs(ctx, state.cfgInvocation(), cfg.gocmdRunner) response.dr.Compiler = compiler response.dr.Arch = arch errCh <- err @@ -1024,3 +1023,44 @@ func cmdDebugStr(cmd *exec.Cmd) string { } return fmt.Sprintf("GOROOT=%v GOPATH=%v GO111MODULE=%v GOPROXY=%v PWD=%v %v", env["GOROOT"], env["GOPATH"], env["GO111MODULE"], env["GOPROXY"], env["PWD"], strings.Join(args, " ")) } + +// getSizesForArgs queries 'go list' for the appropriate +// Compiler and GOARCH arguments to pass to [types.SizesFor]. +func getSizesForArgs(ctx context.Context, inv gocommand.Invocation, gocmdRunner *gocommand.Runner) (string, string, error) { + inv.Verb = "list" + inv.Args = []string{"-f", "{{context.GOARCH}} {{context.Compiler}}", "--", "unsafe"} + stdout, stderr, friendlyErr, rawErr := gocmdRunner.RunRaw(ctx, inv) + var goarch, compiler string + if rawErr != nil { + rawErrMsg := rawErr.Error() + if strings.Contains(rawErrMsg, "cannot find main module") || + strings.Contains(rawErrMsg, "go.mod file not found") { + // User's running outside of a module. + // All bets are off. Get GOARCH and guess compiler is gc. + // TODO(matloob): Is this a problem in practice? + inv.Verb = "env" + inv.Args = []string{"GOARCH"} + envout, enverr := gocmdRunner.Run(ctx, inv) + if enverr != nil { + return "", "", enverr + } + goarch = strings.TrimSpace(envout.String()) + compiler = "gc" + } else if friendlyErr != nil { + return "", "", friendlyErr + } else { + // This should be unreachable, but be defensive + // in case RunRaw's error results are inconsistent. + return "", "", rawErr + } + } else { + fields := strings.Fields(stdout.String()) + if len(fields) < 2 { + return "", "", fmt.Errorf("could not parse GOARCH and Go compiler in format \" \":\nstdout: <<%s>>\nstderr: <<%s>>", + stdout.String(), stderr.String()) + } + goarch = fields[0] + compiler = fields[1] + } + return compiler, goarch, nil +} diff --git a/go/ssa/builder.go b/go/ssa/builder.go index 8b7a814232d..55943e45d24 100644 --- a/go/ssa/builder.go +++ b/go/ssa/builder.go @@ -127,10 +127,48 @@ var ( // builder holds state associated with the package currently being built. // Its methods contain all the logic for AST-to-SSA conversion. +// +// All Functions belong to the same Program. +// +// builders are not thread-safe. type builder struct { - // Invariant: 0 <= rtypes <= finished <= created.Len() - created *creator // functions created during building - finished int // Invariant: create[i].built holds for i in [0,finished) + fns []*Function // Functions that have finished their CREATE phases. + + finished int // finished is the length of the prefix of fns containing built functions. + + // The task of building shared functions within the builder. + // Shared functions are ones the the builder may either create or lookup. + // These may be built by other builders in parallel. + // The task is done when the builder has finished iterating, and it + // waits for all shared functions to finish building. + // nil implies there are no hared functions to wait on. + buildshared *task +} + +// shared is done when the builder has built all of the +// enqueued functions to a fixed-point. +func (b *builder) shared() *task { + if b.buildshared == nil { // lazily-initialize + b.buildshared = &task{done: make(chan unit)} + } + return b.buildshared +} + +// enqueue fn to be built by the builder. +func (b *builder) enqueue(fn *Function) { + b.fns = append(b.fns, fn) +} + +// waitForSharedFunction indicates that the builder should wait until +// the potentially shared function fn has finished building. +// +// This should include any functions that may be built by other +// builders. +func (b *builder) waitForSharedFunction(fn *Function) { + if fn.buildshared != nil { // maybe need to wait? + s := b.shared() + s.addEdge(fn.buildshared) + } } // cond emits to fn code to evaluate boolean condition e and jump @@ -779,7 +817,7 @@ func (b *builder) expr0(fn *Function, e ast.Expr, tv types.TypeAndValue) Value { callee := v.(*Function) // (func) if callee.typeparams.Len() > 0 { targs := fn.subst.types(instanceArgs(fn.info, e)) - callee = callee.instance(targs, b.created) + callee = callee.instance(targs, b) } return callee } @@ -800,7 +838,8 @@ func (b *builder) expr0(fn *Function, e ast.Expr, tv types.TypeAndValue) Value { case types.MethodExpr: // (*T).f or T.f, the method f from the method-set of type T. // The result is a "thunk". - thunk := createThunk(fn.Prog, sel, b.created) + thunk := createThunk(fn.Prog, sel) + b.enqueue(thunk) return emitConv(fn, thunk, fn.typ(tv.Type)) case types.MethodVal: @@ -842,8 +881,11 @@ func (b *builder) expr0(fn *Function, e ast.Expr, tv types.TypeAndValue) Value { // obj is generic. obj = fn.Prog.canon.instantiateMethod(obj, fn.subst.types(targs), fn.Prog.ctxt) } + bound := createBound(fn.Prog, obj) + b.enqueue(bound) + c := &MakeClosure{ - Fn: createBound(fn.Prog, obj, b.created), + Fn: bound, Bindings: []Value{v}, } c.setPos(e.Sel.Pos()) @@ -976,7 +1018,7 @@ func (b *builder) setCallFunc(fn *Function, e *ast.CallExpr, c *CallCommon) { c.Method = obj } else { // "Call"-mode call. - c.Value = fn.Prog.objectMethod(obj, b.created) + c.Value = fn.Prog.objectMethod(obj, b) c.Args = append(c.Args, v) } return @@ -2684,7 +2726,7 @@ start: // The "intrinsics" new/make/len/cap are forbidden here. // panic is treated like an ordinary function call. deferstack := emitLoad(fn, fn.lookup(fn.deferstack, false)) - v := Defer{pos: s.Defer, _DeferStack: deferstack} + v := Defer{pos: s.Defer, DeferStack: deferstack} b.setCall(fn, s.Call, &v.Call) fn.emit(&v) @@ -2838,11 +2880,16 @@ type buildFunc = func(*builder, *Function) // iterate causes all created but unbuilt functions to be built. As // this may create new methods, the process is iterated until it // converges. +// +// Waits for any dependencies to finish building. func (b *builder) iterate() { - for ; b.finished < b.created.Len(); b.finished++ { - fn := b.created.At(b.finished) + for ; b.finished < len(b.fns); b.finished++ { + fn := b.fns[b.finished] b.buildFunction(fn) } + + b.buildshared.markDone() + b.buildshared.wait() } // buildFunction builds SSA code for the body of function fn. Idempotent. @@ -3084,7 +3131,7 @@ func (prog *Program) Build() { p.Build() } else { wg.Add(1) - cpuLimit <- struct{}{} // acquire a token + cpuLimit <- unit{} // acquire a token go func(p *Package) { p.Build() wg.Done() @@ -3096,7 +3143,7 @@ func (prog *Program) Build() { } // cpuLimit is a counting semaphore to limit CPU parallelism. -var cpuLimit = make(chan struct{}, runtime.GOMAXPROCS(0)) +var cpuLimit = make(chan unit, runtime.GOMAXPROCS(0)) // Build builds SSA code for all functions and vars in package p. // @@ -3117,7 +3164,7 @@ func (p *Package) build() { defer logStack("build %s", p)() } - b := builder{created: &p.created} + b := builder{fns: p.created} b.iterate() // We no longer need transient information: ASTs or go/types deductions. diff --git a/go/ssa/builder_test.go b/go/ssa/builder_test.go index 062a221dbfd..ed1d84feeb9 100644 --- a/go/ssa/builder_test.go +++ b/go/ssa/builder_test.go @@ -20,6 +20,7 @@ import ( "strings" "testing" + "golang.org/x/sync/errgroup" "golang.org/x/tools/go/analysis/analysistest" "golang.org/x/tools/go/buildutil" "golang.org/x/tools/go/loader" @@ -1186,3 +1187,76 @@ func TestLabels(t *testing.T) { pkg.Build() } } + +func TestFixedBugs(t *testing.T) { + for _, name := range []string{ + "issue66783a", + "issue66783b", + } { + + t.Run(name, func(t *testing.T) { + base := name + ".go" + path := filepath.Join(analysistest.TestData(), "fixedbugs", base) + fset := token.NewFileSet() + f, err := parser.ParseFile(fset, path, nil, parser.ParseComments) + if err != nil { + t.Fatal(err) + } + files := []*ast.File{f} + pkg := types.NewPackage(name, name) + mode := ssa.SanityCheckFunctions | ssa.InstantiateGenerics + // mode |= ssa.PrintFunctions // debug mode + if _, _, err := ssautil.BuildPackage(&types.Config{}, fset, pkg, files, mode); err != nil { + t.Error(err) + } + }) + } +} + +func TestIssue67079(t *testing.T) { + // This test reproduced a race in the SSA builder nearly 100% of the time. + + // Load the package. + const src = `package p; type T int; func (T) f() {}; var _ = (*T).f` + conf := loader.Config{Fset: token.NewFileSet()} + f, err := parser.ParseFile(conf.Fset, "p.go", src, 0) + if err != nil { + t.Fatal(err) + } + conf.CreateFromFiles("p", f) + iprog, err := conf.Load() + if err != nil { + t.Fatal(err) + } + pkg := iprog.Created[0].Pkg + + // Create and build SSA program. + prog := ssautil.CreateProgram(iprog, ssa.BuilderMode(0)) + prog.Build() + + var g errgroup.Group + + // Access bodies of all functions. + g.Go(func() error { + for fn := range ssautil.AllFunctions(prog) { + for _, b := range fn.Blocks { + for _, instr := range b.Instrs { + if call, ok := instr.(*ssa.Call); ok { + call.Common().StaticCallee() // access call.Value + } + } + } + } + return nil + }) + + // Force building of wrappers. + g.Go(func() error { + ptrT := types.NewPointer(pkg.Scope().Lookup("T").Type()) + ptrTf := types.NewMethodSet(ptrT).At(0) // (*T).f symbol + prog.MethodValue(ptrTf) + return nil + }) + + g.Wait() // ignore error +} diff --git a/go/ssa/create.go b/go/ssa/create.go index f4dab2decdd..423bce87182 100644 --- a/go/ssa/create.go +++ b/go/ssa/create.go @@ -96,8 +96,9 @@ func memberFromObject(pkg *Package, obj types.Object, syntax ast.Node, goversion pkg.ninit++ name = fmt.Sprintf("init#%d", pkg.ninit) } - fn := createFunction(pkg.Prog, obj, name, syntax, pkg.info, goversion, &pkg.created) + fn := createFunction(pkg.Prog, obj, name, syntax, pkg.info, goversion) fn.Pkg = pkg + pkg.created = append(pkg.created, fn) pkg.objects[obj] = fn if name != "_" && sig.Recv() == nil { pkg.Members[name] = fn // package-level function @@ -111,7 +112,7 @@ func memberFromObject(pkg *Package, obj types.Object, syntax ast.Node, goversion // createFunction creates a function or method. It supports both // CreatePackage (with or without syntax) and the on-demand creation // of methods in non-created packages based on their types.Func. -func createFunction(prog *Program, obj *types.Func, name string, syntax ast.Node, info *types.Info, goversion string, cr *creator) *Function { +func createFunction(prog *Program, obj *types.Func, name string, syntax ast.Node, info *types.Info, goversion string) *Function { sig := obj.Type().(*types.Signature) // Collect type parameters. @@ -143,7 +144,6 @@ func createFunction(prog *Program, obj *types.Func, name string, syntax ast.Node if tparams.Len() > 0 { fn.generic = new(generic) } - cr.Add(fn) return fn } @@ -184,19 +184,6 @@ func membersFromDecl(pkg *Package, decl ast.Decl, goversion string) { } } -// creator tracks functions that have finished their CREATE phases. -// -// All Functions belong to the same Program. May have differing packages. -// -// creators are not thread-safe. -type creator []*Function - -func (c *creator) Add(fn *Function) { - *c = append(*c, fn) -} -func (c *creator) At(i int) *Function { return (*c)[i] } -func (c *creator) Len() int { return len(*c) } - // CreatePackage creates and returns an SSA Package from the // specified type-checked, error-free file ASTs, and populates its // Members mapping. @@ -238,7 +225,7 @@ func (prog *Program) CreatePackage(pkg *types.Package, files []*ast.File, info * goversion: "", // See Package.build for details. } p.Members[p.init.name] = p.init - p.created.Add(p.init) + p.created = append(p.created, p.init) // Allocate all package members: vars, funcs, consts and types. if len(files) > 0 { diff --git a/go/ssa/instantiate.go b/go/ssa/instantiate.go index e5e7162a8a2..2512f32976c 100644 --- a/go/ssa/instantiate.go +++ b/go/ssa/instantiate.go @@ -23,7 +23,7 @@ type generic struct { // Any created instance is added to cr. // // Acquires fn.generic.instancesMu. -func (fn *Function) instance(targs []types.Type, cr *creator) *Function { +func (fn *Function) instance(targs []types.Type, b *builder) *Function { key := fn.Prog.canon.List(targs) gen := fn.generic @@ -32,20 +32,24 @@ func (fn *Function) instance(targs []types.Type, cr *creator) *Function { defer gen.instancesMu.Unlock() inst, ok := gen.instances[key] if !ok { - inst = createInstance(fn, targs, cr) + inst = createInstance(fn, targs) + inst.buildshared = b.shared() + b.enqueue(inst) + if gen.instances == nil { gen.instances = make(map[*typeList]*Function) } gen.instances[key] = inst + } else { + b.waitForSharedFunction(inst) } return inst } // createInstance returns the instantiation of generic function fn using targs. -// If the instantiation is created, this is added to cr. // // Requires fn.generic.instancesMu. -func createInstance(fn *Function, targs []types.Type, cr *creator) *Function { +func createInstance(fn *Function, targs []types.Type) *Function { prog := fn.Prog // Compute signature. @@ -78,8 +82,7 @@ func createInstance(fn *Function, targs []types.Type, cr *creator) *Function { if prog.mode&InstantiateGenerics != 0 && !prog.isParameterized(targs...) { synthetic = fmt.Sprintf("instance of %s", fn.Name()) if fn.syntax != nil { - scope := obj.Origin().Scope() - subst = makeSubster(prog.ctxt, scope, fn.typeparams, targs, false) + subst = makeSubster(prog.ctxt, obj, fn.typeparams, targs, false) build = (*builder).buildFromSyntax } else { build = (*builder).buildParamsOnly @@ -90,7 +93,7 @@ func createInstance(fn *Function, targs []types.Type, cr *creator) *Function { } /* generic instance or instantiation wrapper */ - instance := &Function{ + return &Function{ name: fmt.Sprintf("%s%s", fn.Name(), targs), // may not be unique object: obj, Signature: sig, @@ -107,8 +110,6 @@ func createInstance(fn *Function, targs []types.Type, cr *creator) *Function { typeargs: targs, subst: subst, } - cr.Add(instance) - return instance } // isParameterized reports whether any of the specified types contains diff --git a/go/ssa/instantiate_test.go b/go/ssa/instantiate_test.go index 476848d2205..25f78492874 100644 --- a/go/ssa/instantiate_test.go +++ b/go/ssa/instantiate_test.go @@ -96,11 +96,11 @@ func LoadPointer(addr *unsafe.Pointer) (val unsafe.Pointer) meth := prog.FuncValue(obj) - var cr creator + b := &builder{} intSliceTyp := types.NewSlice(types.Typ[types.Int]) - instance := meth.instance([]types.Type{intSliceTyp}, &cr) - if len(cr) != 1 { - t.Errorf("Expected first instance to create a function. got %d created functions", len(cr)) + instance := meth.instance([]types.Type{intSliceTyp}, b) + if len(b.fns) != 1 { + t.Errorf("Expected first instance to create a function. got %d created functions", len(b.fns)) } if instance.Origin() != meth { t.Errorf("Expected Origin of %s to be %s. got %s", instance, meth, instance.Origin()) @@ -114,13 +114,13 @@ func LoadPointer(addr *unsafe.Pointer) (val unsafe.Pointer) } // A second request with an identical type returns the same Function. - second := meth.instance([]types.Type{types.NewSlice(types.Typ[types.Int])}, &cr) - if second != instance || len(cr) != 1 { + second := meth.instance([]types.Type{types.NewSlice(types.Typ[types.Int])}, b) + if second != instance || len(b.fns) != 1 { t.Error("Expected second identical instantiation to not create a function") } // Add a second instance. - inst2 := meth.instance([]types.Type{types.NewSlice(types.Typ[types.Uint])}, &cr) + inst2 := meth.instance([]types.Type{types.NewSlice(types.Typ[types.Uint])}, b) instances = allInstances(meth) // Note: instance.Name() < inst2.Name() @@ -134,7 +134,6 @@ func LoadPointer(addr *unsafe.Pointer) (val unsafe.Pointer) // TODO(adonovan): tests should not rely on unexported functions. // build and sanity check manually created instance. - var b builder b.buildFunction(instance) var buf bytes.Buffer if !sanityCheck(instance, &buf) { diff --git a/go/ssa/interp/interp.go b/go/ssa/interp/interp.go index acd0cca2bf5..3ba78fbd89e 100644 --- a/go/ssa/interp/interp.go +++ b/go/ssa/interp/interp.go @@ -264,7 +264,7 @@ func visitInstr(fr *frame, instr ssa.Instruction) continuation { case *ssa.Defer: fn, args := prepareCall(fr, &instr.Call) defers := &fr.defers - if into := fr.get(deferStack(instr)); into != nil { + if into := fr.get(instr.DeferStack); into != nil { defers = into.(**deferred) } *defers = &deferred{ @@ -723,8 +723,3 @@ func Interpret(mainpkg *ssa.Package, mode Mode, sizes types.Sizes, filename stri } return } - -// TODO(taking): Hack while proposal #66601 is being finalized. -// -//go:linkname deferStack golang.org/x/tools/go/ssa.deferStack -func deferStack(i *ssa.Defer) ssa.Value diff --git a/go/ssa/interp/interp_test.go b/go/ssa/interp/interp_test.go index 2cd7ee98502..2ad6a9a0982 100644 --- a/go/ssa/interp/interp_test.go +++ b/go/ssa/interp/interp_test.go @@ -134,6 +134,7 @@ var testdataTests = []string{ "fixedbugs/issue55115.go", "fixedbugs/issue52835.go", "fixedbugs/issue55086.go", + "fixedbugs/issue66783.go", "typeassert.go", "zeros.go", } diff --git a/go/ssa/interp/testdata/fixedbugs/issue66783.go b/go/ssa/interp/testdata/fixedbugs/issue66783.go new file mode 100644 index 00000000000..e49e86dcc36 --- /dev/null +++ b/go/ssa/interp/testdata/fixedbugs/issue66783.go @@ -0,0 +1,54 @@ +package main + +import "fmt" + +func Fn[N any]() (any, any, any) { + // Very recursive type to exercise substitution. + type t[x any, ignored *N] struct { + f x + g N + nx *t[x, *N] + nn *t[N, *N] + } + n := t[N, *N]{} + s := t[string, *N]{} + i := t[int, *N]{} + return n, s, i +} + +func main() { + + sn, ss, si := Fn[string]() + in, is, ii := Fn[int]() + + for i, t := range []struct { + x, y any + want bool + }{ + {sn, ss, true}, // main.t[string;string,*string] == main.t[string;string,*string] + {sn, si, false}, // main.t[string;string,*string] != main.t[string;int,*string] + {sn, in, false}, // main.t[string;string,*string] != main.t[int;int,*int] + {sn, is, false}, // main.t[string;string,*string] != main.t[int;string,*int] + {sn, ii, false}, // main.t[string;string,*string] != main.t[int;int,*int] + + {ss, si, false}, // main.t[string;string,*string] != main.t[string;int,*string] + {ss, in, false}, // main.t[string;string,*string] != main.t[int;int,*int] + {ss, is, false}, // main.t[string;string,*string] != main.t[int;string,*int] + {ss, ii, false}, // main.t[string;string,*string] != main.t[int;int,*int] + + {si, in, false}, // main.t[string;int,*string] != main.t[int;int,*int] + {si, is, false}, // main.t[string;int,*string] != main.t[int;string,*int] + {si, ii, false}, // main.t[string;int,*string] != main.t[int;int,*int] + + {in, is, false}, // main.t[int;int,*int] != main.t[int;string,*int] + {in, ii, true}, // main.t[int;int,*int] == main.t[int;int,*int] + + {is, ii, false}, // main.t[int;string,*int] != main.t[int;int,*int] + } { + x, y, want := t.x, t.y, t.want + if got := x == y; got != want { + msg := fmt.Sprintf("(case %d) %T == %T. got %v. wanted %v", i, x, y, got, want) + panic(msg) + } + } +} diff --git a/go/ssa/lift.go b/go/ssa/lift.go index 49e148d716a..aada3dc3227 100644 --- a/go/ssa/lift.go +++ b/go/ssa/lift.go @@ -176,8 +176,12 @@ func lift(fn *Function) { // While we're here, we also eliminate 'rundefers' // instructions and ssa:deferstack() in functions that contain no - // 'defer' instructions. Eliminate ssa:deferstack() if it does not - // escape. + // 'defer' instructions. For now, we also eliminate + // 's = ssa:deferstack()' calls if s doesn't escape, replacing s + // with nil in Defer{DeferStack: s}. This has the same meaning, + // but allows eliminating the intrinsic function `ssa:deferstack()` + // (unless it is needed due to range-over-func instances). This gives + // ssa users more time to support range-over-func. usesDefer := false deferstackAlloc, deferstackCall := deferstackPreamble(fn) eliminateDeferStack := deferstackAlloc != nil && !deferstackAlloc.Heap @@ -205,12 +209,12 @@ func lift(fn *Function) { case *Defer: usesDefer = true if eliminateDeferStack { - // Clear _DeferStack and remove references to loads - if instr._DeferStack != nil { - if refs := instr._DeferStack.Referrers(); refs != nil { + // Clear DeferStack and remove references to loads + if instr.DeferStack != nil { + if refs := instr.DeferStack.Referrers(); refs != nil { *refs = removeInstr(*refs, instr) } - instr._DeferStack = nil + instr.DeferStack = nil } } case *RunDefers: diff --git a/go/ssa/methods.go b/go/ssa/methods.go index 58bd45b8146..b9560183a95 100644 --- a/go/ssa/methods.go +++ b/go/ssa/methods.go @@ -40,7 +40,7 @@ func (prog *Program) MethodValue(sel *types.Selection) *Function { defer logStack("MethodValue %s %v", T, sel)() } - var cr creator + var b builder m := func() *Function { prog.methodsMu.Lock() @@ -61,20 +61,23 @@ func (prog *Program) MethodValue(sel *types.Selection) *Function { needsPromotion := len(sel.Index()) > 1 needsIndirection := !isPointer(recvType(obj)) && isPointer(T) if needsPromotion || needsIndirection { - fn = createWrapper(prog, toSelection(sel), &cr) + fn = createWrapper(prog, toSelection(sel)) + fn.buildshared = b.shared() + b.enqueue(fn) } else { - fn = prog.objectMethod(obj, &cr) + fn = prog.objectMethod(obj, &b) } if fn.Signature.Recv() == nil { panic(fn) } mset.mapping[id] = fn + } else { + b.waitForSharedFunction(fn) } return fn }() - b := builder{created: &cr} b.iterate() return m @@ -88,7 +91,7 @@ func (prog *Program) MethodValue(sel *types.Selection) *Function { // objectMethod panics if the function is not a method. // // Acquires prog.objectMethodsMu. -func (prog *Program) objectMethod(obj *types.Func, cr *creator) *Function { +func (prog *Program) objectMethod(obj *types.Func, b *builder) *Function { sig := obj.Type().(*types.Signature) if sig.Recv() == nil { panic("not a method: " + obj.String()) @@ -101,10 +104,10 @@ func (prog *Program) objectMethod(obj *types.Func, cr *creator) *Function { // Instantiation of generic? if originObj := obj.Origin(); originObj != obj { - origin := prog.objectMethod(originObj, cr) + origin := prog.objectMethod(originObj, b) assert(origin.typeparams.Len() > 0, "origin is not generic") targs := receiverTypeArgs(obj) - return origin.instance(targs, cr) + return origin.instance(targs, b) } // Consult/update cache of methods created from types.Func. @@ -112,13 +115,17 @@ func (prog *Program) objectMethod(obj *types.Func, cr *creator) *Function { defer prog.objectMethodsMu.Unlock() fn, ok := prog.objectMethods[obj] if !ok { - fn = createFunction(prog, obj, obj.Name(), nil, nil, "", cr) + fn = createFunction(prog, obj, obj.Name(), nil, nil, "") fn.Synthetic = "from type information (on demand)" + fn.buildshared = b.shared() + b.enqueue(fn) if prog.objectMethods == nil { prog.objectMethods = make(map[*types.Func]*Function) } prog.objectMethods[obj] = fn + } else { + b.waitForSharedFunction(fn) } return fn } diff --git a/go/ssa/print.go b/go/ssa/print.go index 40c06862946..c890d7ee531 100644 --- a/go/ssa/print.go +++ b/go/ssa/print.go @@ -356,8 +356,8 @@ func (s *Send) String() string { func (s *Defer) String() string { prefix := "defer " - if s._DeferStack != nil { - prefix += "[" + relName(s._DeferStack, s) + "] " + if s.DeferStack != nil { + prefix += "[" + relName(s.DeferStack, s) + "] " } c := printCall(&s.Call, prefix, s) return c diff --git a/go/ssa/sanity.go b/go/ssa/sanity.go index d635c15a3b0..285cba04a9f 100644 --- a/go/ssa/sanity.go +++ b/go/ssa/sanity.go @@ -21,7 +21,7 @@ type sanity struct { reporter io.Writer fn *Function block *BasicBlock - instrs map[Instruction]struct{} + instrs map[Instruction]unit insane bool } @@ -461,10 +461,10 @@ func (s *sanity) checkFunction(fn *Function) bool { } } // Build the set of valid referrers. - s.instrs = make(map[Instruction]struct{}) + s.instrs = make(map[Instruction]unit) for _, b := range fn.Blocks { for _, instr := range b.Instrs { - s.instrs[instr] = struct{}{} + s.instrs[instr] = unit{} } } for i, p := range fn.Params { diff --git a/go/ssa/ssa.go b/go/ssa/ssa.go index 59474a9d3db..1231afd9e0c 100644 --- a/go/ssa/ssa.go +++ b/go/ssa/ssa.go @@ -69,7 +69,7 @@ type Package struct { ninit int32 // number of init functions info *types.Info // package type information files []*ast.File // package ASTs - created creator // members created as a result of building this package (includes declared functions, wrappers) + created []*Function // members created as a result of building this package (includes declared functions, wrappers) initVersion map[ast.Expr]string // goversion to use for each global var init expr } @@ -348,6 +348,8 @@ type Function struct { Pkg *Package // enclosing package; nil for shared funcs (wrappers and error.Error) Prog *Program // enclosing program + buildshared *task // wait for a shared function to be done building (may be nil if <=1 builder ever needs to wait) + // These fields are populated only when the function body is built: Params []*Parameter // function parameters; for methods, includes receiver @@ -1255,10 +1257,10 @@ type Go struct { // The Defer instruction pushes the specified call onto a stack of // functions to be called by a RunDefers instruction or by a panic. // -// If _DeferStack != nil, it indicates the defer list that the defer is +// If DeferStack != nil, it indicates the defer list that the defer is // added to. Defer list values come from the Builtin function // ssa:deferstack. Calls to ssa:deferstack() produces the defer stack -// of the current function frame. _DeferStack allows for deferring into an +// of the current function frame. DeferStack allows for deferring into an // alternative function stack than the current function. // // See CallCommon for generic function call documentation. @@ -1272,11 +1274,9 @@ type Go struct { // defer invoke t5.Println(...t6) type Defer struct { anInstruction - Call CallCommon - _DeferStack Value // stack (from ssa:deferstack() intrinsic) onto which this function is pushed - pos token.Pos - - // TODO: Exporting _DeferStack and possibly making _DeferStack != nil awaits proposal https://github.com/golang/go/issues/66601. + Call CallCommon + DeferStack Value // stack of deferred functions (from ssa:deferstack() intrinsic) onto which this function is pushed + pos token.Pos } // The Send instruction sends X on channel Chan. @@ -1718,7 +1718,7 @@ func (s *Call) Operands(rands []*Value) []*Value { } func (s *Defer) Operands(rands []*Value) []*Value { - return append(s.Call.Operands(rands), &s._DeferStack) + return append(s.Call.Operands(rands), &s.DeferStack) } func (v *ChangeInterface) Operands(rands []*Value) []*Value { @@ -1869,7 +1869,3 @@ func (v *Const) Operands(rands []*Value) []*Value { return rands } func (v *Function) Operands(rands []*Value) []*Value { return rands } func (v *Global) Operands(rands []*Value) []*Value { return rands } func (v *Parameter) Operands(rands []*Value) []*Value { return rands } - -// Exposed to interp using the linkname hack -// TODO(taking): Remove some form of https://go.dev/issue/66601 is accepted. -func deferStack(i *Defer) Value { return i._DeferStack } diff --git a/go/ssa/stdlib_test.go b/go/ssa/stdlib_test.go index d294fe6b085..03c88510840 100644 --- a/go/ssa/stdlib_test.go +++ b/go/ssa/stdlib_test.go @@ -50,6 +50,16 @@ func TestNetHTTP(t *testing.T) { testLoad(t, 120, "net/http") } +// TestCycles loads two standard libraries that depend on the same +// generic instantiations. +// internal/trace/testtrace and net/http both depend on +// slices.Contains[[]string string] and slices.Index[[]string string] +// This can under some schedules create a cycle of dependencies +// where both need to wait on the other to finish building. +func TestCycles(t *testing.T) { + testLoad(t, 120, "net/http", "internal/trace/testtrace") +} + func testLoad(t *testing.T, minPkgs int, patterns ...string) { // Note: most of the commentary below applies to TestStdlib. diff --git a/go/ssa/subst.go b/go/ssa/subst.go index 6490db8fb26..75d887d7e52 100644 --- a/go/ssa/subst.go +++ b/go/ssa/subst.go @@ -7,66 +7,73 @@ package ssa import ( "go/types" + "golang.org/x/tools/go/types/typeutil" "golang.org/x/tools/internal/aliases" ) -// Type substituter for a fixed set of replacement types. +// subster defines a type substitution operation of a set of type parameters +// to type parameter free replacement types. Substitution is done within +// the context of a package-level function instantiation. *Named types +// declared in the function are unique to the instantiation. // -// A nil *subster is an valid, empty substitution map. It always acts as +// For example, given a parameterized function F +// +// func F[S, T any]() any { +// type X struct{ s S; next *X } +// var p *X +// return p +// } +// +// calling the instantiation F[string, int]() returns an interface +// value (*X[string,int], nil) where the underlying value of +// X[string,int] is a struct{s string; next *X[string,int]}. +// +// A nil *subster is a valid, empty substitution map. It always acts as // the identity function. This allows for treating parameterized and // non-parameterized functions identically while compiling to ssa. // // Not concurrency-safe. +// +// Note: Some may find it helpful to think through some of the most +// complex substitution cases using lambda calculus inspired notation. +// subst.typ() solves evaluating a type expression E +// within the body of a function Fn[m] with the type parameters m +// once we have applied the type arguments N. +// We can succinctly write this as a function application: +// +// ((λm. E) N) +// +// go/types does not provide this interface directly. +// So what subster provides is a type substitution operation +// +// E[m:=N] type subster struct { replacements map[*types.TypeParam]types.Type // values should contain no type params cache map[types.Type]types.Type // cache of subst results - ctxt *types.Context // cache for instantiation - scope *types.Scope // *types.Named declared within this scope can be substituted (optional) - debug bool // perform extra debugging checks + origin *types.Func // types.Objects declared within this origin function are unique within this context + ctxt *types.Context // speeds up repeated instantiations + uniqueness typeutil.Map // determines the uniqueness of the instantiations within the function // TODO(taking): consider adding Pos - // TODO(zpavlinovic): replacements can contain type params - // when generating instances inside of a generic function body. } // Returns a subster that replaces tparams[i] with targs[i]. Uses ctxt as a cache. // targs should not contain any types in tparams. -// scope is the (optional) lexical block of the generic function for which we are substituting. -func makeSubster(ctxt *types.Context, scope *types.Scope, tparams *types.TypeParamList, targs []types.Type, debug bool) *subster { +// fn is the generic function for which we are substituting. +func makeSubster(ctxt *types.Context, fn *types.Func, tparams *types.TypeParamList, targs []types.Type, debug bool) *subster { assert(tparams.Len() == len(targs), "makeSubster argument count must match") subst := &subster{ replacements: make(map[*types.TypeParam]types.Type, tparams.Len()), cache: make(map[types.Type]types.Type), + origin: fn.Origin(), ctxt: ctxt, - scope: scope, - debug: debug, } for i := 0; i < tparams.Len(); i++ { subst.replacements[tparams.At(i)] = targs[i] } - if subst.debug { - subst.wellFormed() - } return subst } -// wellFormed asserts that subst was properly initialized. -func (subst *subster) wellFormed() { - if subst == nil { - return - } - // Check that all of the type params do not appear in the arguments. - s := make(map[types.Type]bool, len(subst.replacements)) - for tparam := range subst.replacements { - s[tparam] = true - } - for _, r := range subst.replacements { - if reaches(r, s) { - panic(subst) - } - } -} - // typ returns the type of t with the type parameter tparams[i] substituted // for the type targs[i] where subst was created using tparams and targs. func (subst *subster) typ(t types.Type) (res types.Type) { @@ -82,9 +89,10 @@ func (subst *subster) typ(t types.Type) (res types.Type) { switch t := t.(type) { case *types.TypeParam: - r := subst.replacements[t] - assert(r != nil, "type param without replacement encountered") - return r + if r := subst.replacements[t]; r != nil { + return r + } + return t case *types.Basic: return t @@ -194,7 +202,7 @@ func (subst *subster) struct_(t *types.Struct) *types.Struct { return t } -// varlist reutrns subst(in[i]) or return nils if subst(v[i]) == v[i] for all i. +// varlist returns subst(in[i]) or return nils if subst(v[i]) == v[i] for all i. func (subst *subster) varlist(in varlist) []*types.Var { var out []*types.Var // nil => no updates for i, n := 0, in.Len(); i < n; i++ { @@ -322,71 +330,146 @@ func (subst *subster) alias(t *aliases.Alias) types.Type { } func (subst *subster) named(t *types.Named) types.Type { - // A named type may be: - // (1) ordinary named type (non-local scope, no type parameters, no type arguments), - // (2) locally scoped type, - // (3) generic (type parameters but no type arguments), or - // (4) instantiated (type parameters and type arguments). - tparams := t.TypeParams() - if tparams.Len() == 0 { - if subst.scope != nil && !subst.scope.Contains(t.Obj().Pos()) { - // Outside the current function scope? - return t // case (1) ordinary + // A Named type is a user defined type. + // Ignoring generics, Named types are canonical: they are identical if + // and only if they have the same defining symbol. + // Generics complicate things, both if the type definition itself is + // parameterized, and if the type is defined within the scope of a + // parameterized function. In this case, two named types are identical if + // and only if their identifying symbols are identical, and all type + // arguments bindings in scope of the named type definition (including the + // type parameters of the definition itself) are equivalent. + // + // Notably: + // 1. For type definition type T[P1 any] struct{}, T[A] and T[B] are identical + // only if A and B are identical. + // 2. Inside the generic func Fn[m any]() any { type T struct{}; return T{} }, + // the result of Fn[A] and Fn[B] have identical type if and only if A and + // B are identical. + // 3. Both 1 and 2 could apply, such as in + // func F[m any]() any { type T[x any] struct{}; return T{} } + // + // A subster replaces type parameters within a function scope, and therefore must + // also replace free type parameters in the definitions of local types. + // + // Note: There are some detailed notes sprinkled throughout that borrow from + // lambda calculus notation. These contain some over simplifying math. + // + // LC: One way to think about subster is that it is a way of evaluating + // ((λm. E) N) as E[m:=N]. + // Each Named type t has an object *TypeName within a scope S that binds an + // underlying type expression U. U can refer to symbols within S (+ S's ancestors). + // Let x = t.TypeParams() and A = t.TypeArgs(). + // Each Named type t is then either: + // U where len(x) == 0 && len(A) == 0 + // λx. U where len(x) != 0 && len(A) == 0 + // ((λx. U) A) where len(x) == len(A) + // In each case, we will evaluate t[m:=N]. + tparams := t.TypeParams() // x + targs := t.TypeArgs() // A + + if !declaredWithin(t.Obj(), subst.origin) { + // t is declared outside of Fn[m]. + // + // In this case, we can skip substituting t.Underlying(). + // The underlying type cannot refer to the type parameters. + // + // LC: Let free(E) be the set of free type parameters in an expression E. + // Then whenever m ∉ free(E), then E = E[m:=N]. + // t ∉ Scope(fn) so therefore m ∉ free(U) and m ∩ x = ∅. + if targs.Len() == 0 { + // t has no type arguments. So it does not need to be instantiated. + // + // This is the normal case in real Go code, where t is not parameterized, + // declared at some package scope, and m is a TypeParam from a parameterized + // function F[m] or method. + // + // LC: m ∉ free(A) lets us conclude m ∉ free(t). So t=t[m:=N]. + return t } - // case (2) locally scoped type. - // Create a new named type to represent this instantiation. - // We assume that local types of distinct instantiations of a - // generic function are distinct, even if they don't refer to - // type parameters, but the spec is unclear; see golang/go#58573. + // t is declared outside of Fn[m] and has type arguments. + // The type arguments may contain type parameters m so + // substitute the type arguments, and instantiate the substituted + // type arguments. + // + // LC: Evaluate this as ((λx. U) A') where A' = A[m := N]. + newTArgs := subst.typelist(targs) + return subst.instantiate(t.Origin(), newTArgs) + } + + // t is declared within Fn[m]. + + if targs.Len() == 0 { // no type arguments? + assert(t == t.Origin(), "local parameterized type abstraction must be an origin type") + + // t has no type arguments. + // The underlying type of t may contain the function's type parameters, + // replace these, and create a new type. // // Subtle: We short circuit substitution and use a newly created type in - // subst, i.e. cache[t]=n, to pre-emptively replace t with n in recursive - // types during traversal. This both breaks infinite cycles and allows for - // constructing types with the replacement applied in subst.typ(under). + // subst, i.e. cache[t]=fresh, to preemptively replace t with fresh + // in recursive types during traversal. This both breaks infinite cycles + // and allows for constructing types with the replacement applied in + // subst.typ(U). // - // Example: - // func foo[T any]() { - // type linkedlist struct { - // next *linkedlist - // val T - // } - // } + // A new copy of the Named and Typename (and constraints) per function + // instantiation matches the semantics of Go, which treats all function + // instantiations F[N] as having distinct local types. // - // When the field `next *linkedlist` is visited during subst.typ(under), - // we want the substituted type for the field `next` to be `*n`. - n := types.NewNamed(t.Obj(), nil, nil) - subst.cache[t] = n - subst.cache[n] = n - n.SetUnderlying(subst.typ(t.Underlying())) - return n - } - targs := t.TypeArgs() - - // insts are arguments to instantiate using. - insts := make([]types.Type, tparams.Len()) - - // case (3) generic ==> targs.Len() == 0 - // Instantiating a generic with no type arguments should be unreachable. - // Please report a bug if you encounter this. - assert(targs.Len() != 0, "substition into a generic Named type is currently unsupported") - - // case (4) instantiated. - // Substitute into the type arguments and instantiate the replacements/ - // Example: - // type N[A any] func() A - // func Foo[T](g N[T]) {} - // To instantiate Foo[string], one goes through {T->string}. To get the type of g - // one subsitutes T with string in {N with typeargs == {T} and typeparams == {A} } - // to get {N with TypeArgs == {string} and typeparams == {A} }. - assert(targs.Len() == tparams.Len(), "typeargs.Len() must match typeparams.Len() if present") - for i, n := 0, targs.Len(); i < n; i++ { - inst := subst.typ(targs.At(i)) // TODO(generic): Check with rfindley for mutual recursion - insts[i] = inst + // LC: x.Len()=0 can be thought of as a special case of λx. U. + // LC: Evaluate (λx. U)[m:=N] as (λx'. U') where U'=U[x:=x',m:=N]. + tname := t.Obj() + obj := types.NewTypeName(tname.Pos(), tname.Pkg(), tname.Name(), nil) + fresh := types.NewNamed(obj, nil, nil) + var newTParams []*types.TypeParam + for i := 0; i < tparams.Len(); i++ { + cur := tparams.At(i) + cobj := cur.Obj() + cname := types.NewTypeName(cobj.Pos(), cobj.Pkg(), cobj.Name(), nil) + ntp := types.NewTypeParam(cname, nil) + subst.cache[cur] = ntp + newTParams = append(newTParams, ntp) + } + fresh.SetTypeParams(newTParams) + subst.cache[t] = fresh + subst.cache[fresh] = fresh + fresh.SetUnderlying(subst.typ(t.Underlying())) + // Substitute into all of the constraints after they are created. + for i, ntp := range newTParams { + bound := tparams.At(i).Constraint() + ntp.SetConstraint(subst.typ(bound)) + } + return fresh } - r, err := types.Instantiate(subst.ctxt, t.Origin(), insts, false) + + // t is defined within Fn[m] and t has type arguments (an instantiation). + // We reduce this to the two cases above: + // (1) substitute the function's type parameters into t.Origin(). + // (2) substitute t's type arguments A and instantiate the updated t.Origin() with these. + // + // LC: Evaluate ((λx. U) A)[m:=N] as (t' A') where t' = (λx. U)[m:=N] and A'=A [m:=N] + subOrigin := subst.typ(t.Origin()) + subTArgs := subst.typelist(targs) + return subst.instantiate(subOrigin, subTArgs) +} + +func (subst *subster) instantiate(orig types.Type, targs []types.Type) types.Type { + i, err := types.Instantiate(subst.ctxt, orig, targs, false) assert(err == nil, "failed to Instantiate Named type") - return r + if c, _ := subst.uniqueness.At(i).(types.Type); c != nil { + return c.(types.Type) + } + subst.uniqueness.Set(i, i) + return i +} + +func (subst *subster) typelist(l *types.TypeList) []types.Type { + res := make([]types.Type, l.Len()) + for i := 0; i < l.Len(); i++ { + res[i] = subst.typ(l.At(i)) + } + return res } func (subst *subster) signature(t *types.Signature) types.Type { diff --git a/go/ssa/subst_test.go b/go/ssa/subst_test.go index 6652b1a8e97..3c126faac36 100644 --- a/go/ssa/subst_test.go +++ b/go/ssa/subst_test.go @@ -16,6 +16,10 @@ func TestSubst(t *testing.T) { const source = ` package P +func within(){ + // Pretend that the instantiation happens within this function. +} + type t0 int func (t0) f() type t1 interface{ f() } @@ -55,6 +59,11 @@ var _ L[int] = Fn0[L[int]](nil) t.Fatal(err) } + within, _ := pkg.Scope().Lookup("within").(*types.Func) + if within == nil { + t.Fatal("Failed to find the function within()") + } + for _, test := range []struct { expr string // type expression of Named parameterized type args []string // type expressions of args for named @@ -94,7 +103,7 @@ var _ L[int] = Fn0[L[int]](nil) T := tv.Type.(*types.Named) - subst := makeSubster(types.NewContext(), nil, T.TypeParams(), targs, true) + subst := makeSubster(types.NewContext(), within, T.TypeParams(), targs, true) sub := subst.typ(T.Underlying()) if got := sub.String(); got != test.want { t.Errorf("subst{%v->%v}.typ(%s) = %v, want %v", test.expr, test.args, T.Underlying(), got, test.want) diff --git a/go/ssa/task.go b/go/ssa/task.go new file mode 100644 index 00000000000..50249852665 --- /dev/null +++ b/go/ssa/task.go @@ -0,0 +1,103 @@ +// Copyright 2024 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +package ssa + +import ( + "sync/atomic" +) + +// Each task has two states: it is initially "active", +// and transitions to "done". +// +// tasks form a directed graph. An edge from x to y (with y in x.edges) +// indicates that the task x waits on the task y to be done. +// Cycles are permitted. +// +// Calling x.wait() blocks the calling goroutine until task x, +// and all the tasks transitively reachable from x are done. +// +// The nil *task is always considered done. +type task struct { + done chan unit // close when the task is done. + edges map[*task]unit // set of predecessors of this task. + transitive atomic.Bool // true once it is known all predecessors are done. +} + +func (x *task) isTransitivelyDone() bool { return x == nil || x.transitive.Load() } + +// addEdge creates an edge from x to y, indicating that +// x.wait() will not return before y is done. +// All calls to x.addEdge(...) should happen before x.markDone(). +func (x *task) addEdge(y *task) { + if x == y || y.isTransitivelyDone() { + return // no work remaining + } + + // heuristic done check + select { + case <-x.done: + panic("cannot add an edge to a done task") + default: + } + + if x.edges == nil { + x.edges = make(map[*task]unit) + } + x.edges[y] = unit{} +} + +// markDone changes the task's state to markDone. +func (x *task) markDone() { + if x != nil { + close(x.done) + } +} + +// wait blocks until x and all the tasks it can reach through edges are done. +func (x *task) wait() { + if x.isTransitivelyDone() { + return // already known to be done. Skip allocations. + } + + // Use BFS to wait on u.done to be closed, for all u transitively + // reachable from x via edges. + // + // This work can be repeated by multiple workers doing wait(). + // + // Note: Tarjan's SCC algorithm is able to mark SCCs as transitively done + // as soon as the SCC has been visited. This is theoretically faster, but is + // a more complex algorithm. Until we have evidence, we need the more complex + // algorithm, the simpler algorithm BFS is implemented. + // + // In Go 1.23, ssa/TestStdlib reaches <=3 *tasks per wait() in most schedules + // On some schedules, there is a cycle building net/http and internal/trace/testtrace + // due to slices functions. + work := []*task{x} + enqueued := map[*task]unit{x: {}} + for i := 0; i < len(work); i++ { + u := work[i] + if u.isTransitivelyDone() { // already transitively done + work[i] = nil + continue + } + <-u.done // wait for u to be marked done. + + for v := range u.edges { + if _, ok := enqueued[v]; !ok { + enqueued[v] = unit{} + work = append(work, v) + } + } + } + + // work is transitively closed over dependencies. + // u in work is done (or transitively done and skipped). + // u is transitively done. + for _, u := range work { + if u != nil { + x.transitive.Store(true) + } + } +} diff --git a/go/ssa/testdata/fixedbugs/issue66783a.go b/go/ssa/testdata/fixedbugs/issue66783a.go new file mode 100644 index 00000000000..d4cf0f5153d --- /dev/null +++ b/go/ssa/testdata/fixedbugs/issue66783a.go @@ -0,0 +1,24 @@ +//go:build ignore +// +build ignore + +package issue66783a + +type S[T any] struct { + a T +} + +func (s S[T]) M() { + type A S[T] + type B[U any] A + _ = B[rune](s) +} + +// M[int] + +// panic: in (issue66783a.S[int]).M[int]: +// cannot convert term *t0 (issue66783a.S[int] [within struct{a int}]) +// to type issue66783a.B[rune] [within struct{a T}] [recovered] + +func M() { + S[int]{}.M() +} diff --git a/go/ssa/testdata/fixedbugs/issue66783b.go b/go/ssa/testdata/fixedbugs/issue66783b.go new file mode 100644 index 00000000000..50a2d303be8 --- /dev/null +++ b/go/ssa/testdata/fixedbugs/issue66783b.go @@ -0,0 +1,22 @@ +//go:build ignore +// +build ignore + +package issue66783b + +type I1[T any] interface { + M(T) +} + +type I2[T any] I1[T] + +func foo[T any](i I2[T]) { + _ = i.M +} + +type S[T any] struct{} + +func (s S[T]) M(t T) {} + +func M2() { + foo[int](I2[int](S[int]{})) +} diff --git a/go/ssa/util.go b/go/ssa/util.go index ed3e993489d..549c9c819ea 100644 --- a/go/ssa/util.go +++ b/go/ssa/util.go @@ -22,6 +22,8 @@ import ( "golang.org/x/tools/internal/typesinternal" ) +type unit struct{} + //// Sanity checking utilities // assert panics with the mesage msg if p is false. @@ -149,6 +151,26 @@ func isUntyped(typ types.Type) bool { return ok && b.Info()&types.IsUntyped != 0 } +// declaredWithin reports whether an object is declared within a function. +// +// obj must not be a method or a field. +func declaredWithin(obj types.Object, fn *types.Func) bool { + if obj.Pos() != token.NoPos { + return fn.Scope().Contains(obj.Pos()) // trust the positions if they exist. + } + if fn.Pkg() != obj.Pkg() { + return false // fast path for different packages + } + + // Traverse Parent() scopes for fn.Scope(). + for p := obj.Parent(); p != nil; p = p.Parent() { + if p == fn.Scope() { + return true + } + } + return false +} + // logStack prints the formatted "start" message to stderr and // returns a closure that prints the corresponding "end" message. // Call using 'defer logStack(...)()' to show builder stack on panic. diff --git a/go/ssa/wrappers.go b/go/ssa/wrappers.go index b25c4c78979..d09b4f250ee 100644 --- a/go/ssa/wrappers.go +++ b/go/ssa/wrappers.go @@ -42,7 +42,7 @@ import ( // - optional implicit field selections // - meth.Obj() may denote a concrete or an interface method // - the result may be a thunk or a wrapper. -func createWrapper(prog *Program, sel *selection, cr *creator) *Function { +func createWrapper(prog *Program, sel *selection) *Function { obj := sel.obj.(*types.Func) // the declared function sig := sel.typ.(*types.Signature) // type of this wrapper @@ -63,7 +63,7 @@ func createWrapper(prog *Program, sel *selection, cr *creator) *Function { defer logStack("create %s to (%s)", description, recv.Type())() } /* method wrapper */ - fn := &Function{ + return &Function{ name: name, method: sel, object: obj, @@ -77,8 +77,6 @@ func createWrapper(prog *Program, sel *selection, cr *creator) *Function { info: nil, goversion: "", } - cr.Add(fn) - return fn } // buildWrapper builds fn.Body for a method wrapper. @@ -141,7 +139,7 @@ func (b *builder) buildWrapper(fn *Function) { if !isPointer(r) { v = emitLoad(fn, v) } - c.Call.Value = fn.Prog.objectMethod(fn.object, b.created) + c.Call.Value = fn.Prog.objectMethod(fn.object, b) c.Call.Args = append(c.Call.Args, v) } else { c.Call.Method = fn.object @@ -188,7 +186,7 @@ func createParams(fn *Function, start int) { // Unlike createWrapper, createBound need perform no indirection or field // selections because that can be done before the closure is // constructed. -func createBound(prog *Program, obj *types.Func, cr *creator) *Function { +func createBound(prog *Program, obj *types.Func) *Function { description := fmt.Sprintf("bound method wrapper for %s", obj) if prog.mode&LogSource != 0 { defer logStack("%s", description)() @@ -208,7 +206,6 @@ func createBound(prog *Program, obj *types.Func, cr *creator) *Function { goversion: "", } fn.FreeVars = []*FreeVar{{name: "recv", typ: recvType(obj), parent: fn}} // (cyclic) - cr.Add(fn) return fn } @@ -220,7 +217,7 @@ func (b *builder) buildBound(fn *Function) { recv := fn.FreeVars[0] if !types.IsInterface(recvType(fn.object)) { // concrete - c.Call.Value = fn.Prog.objectMethod(fn.object, b.created) + c.Call.Value = fn.Prog.objectMethod(fn.object, b) c.Call.Args = []Value{recv} } else { c.Call.Method = fn.object @@ -251,12 +248,12 @@ func (b *builder) buildBound(fn *Function) { // f is a synthetic wrapper defined as if by: // // f := func(t T) { return t.meth() } -func createThunk(prog *Program, sel *selection, cr *creator) *Function { +func createThunk(prog *Program, sel *selection) *Function { if sel.kind != types.MethodExpr { panic(sel) } - fn := createWrapper(prog, sel, cr) + fn := createWrapper(prog, sel) if fn.Signature.Recv() != nil { panic(fn) // unexpected receiver } diff --git a/go/types/objectpath/objectpath.go b/go/types/objectpath/objectpath.go index a2386c347a2..d648c3d071b 100644 --- a/go/types/objectpath/objectpath.go +++ b/go/types/objectpath/objectpath.go @@ -51,7 +51,7 @@ type Path string // // PO package->object Package.Scope.Lookup // OT object->type Object.Type -// TT type->type Type.{Elem,Key,Params,Results,Underlying} [EKPRU] +// TT type->type Type.{Elem,Key,{,{,Recv}Type}Params,Results,Underlying} [EKPRUTrC] // TO type->object Type.{At,Field,Method,Obj} [AFMO] // // All valid paths start with a package and end at an object @@ -63,8 +63,8 @@ type Path string // - The only PO operator is Package.Scope.Lookup, which requires an identifier. // - The only OT operator is Object.Type, // which we encode as '.' because dot cannot appear in an identifier. -// - The TT operators are encoded as [EKPRUTC]; -// one of these (TypeParam) requires an integer operand, +// - The TT operators are encoded as [EKPRUTrC]; +// two of these ({,Recv}TypeParams) require an integer operand, // which is encoded as a string of decimal digits. // - The TO operators are encoded as [AFMO]; // three of these (At,Field,Method) require an integer operand, @@ -98,19 +98,20 @@ const ( opType = '.' // .Type() (Object) // type->type operators - opElem = 'E' // .Elem() (Pointer, Slice, Array, Chan, Map) - opKey = 'K' // .Key() (Map) - opParams = 'P' // .Params() (Signature) - opResults = 'R' // .Results() (Signature) - opUnderlying = 'U' // .Underlying() (Named) - opTypeParam = 'T' // .TypeParams.At(i) (Named, Signature) - opConstraint = 'C' // .Constraint() (TypeParam) + opElem = 'E' // .Elem() (Pointer, Slice, Array, Chan, Map) + opKey = 'K' // .Key() (Map) + opParams = 'P' // .Params() (Signature) + opResults = 'R' // .Results() (Signature) + opUnderlying = 'U' // .Underlying() (Named) + opTypeParam = 'T' // .TypeParams.At(i) (Named, Signature) + opRecvTypeParam = 'r' // .RecvTypeParams.At(i) (Signature) + opConstraint = 'C' // .Constraint() (TypeParam) // type->object operators - opAt = 'A' // .At(i) (Tuple) - opField = 'F' // .Field(i) (Struct) - opMethod = 'M' // .Method(i) (Named or Interface; not Struct: "promoted" names are ignored) - opObj = 'O' // .Obj() (Named, TypeParam) + opAt = 'A' // .At(i) (Tuple) + opField = 'F' // .Field(i) (Struct) + opMethod = 'M' // .Method(i) (Named or Interface; not Struct: "promoted" names are ignored) + opObj = 'O' // .Obj() (Named, TypeParam) ) // For is equivalent to new(Encoder).For(obj). @@ -286,7 +287,7 @@ func (enc *Encoder) For(obj types.Object) (Path, error) { } } else { if named, _ := T.(*types.Named); named != nil { - if r := findTypeParam(obj, named.TypeParams(), path, nil); r != nil { + if r := findTypeParam(obj, named.TypeParams(), path, opTypeParam, nil); r != nil { // generic named type return Path(r), nil } @@ -462,7 +463,10 @@ func find(obj types.Object, T types.Type, path []byte, seen map[*types.TypeName] } return find(obj, T.Elem(), append(path, opElem), seen) case *types.Signature: - if r := findTypeParam(obj, T.TypeParams(), path, seen); r != nil { + if r := findTypeParam(obj, T.RecvTypeParams(), path, opRecvTypeParam, nil); r != nil { + return r + } + if r := findTypeParam(obj, T.TypeParams(), path, opTypeParam, seen); r != nil { return r } if r := find(obj, T.Params(), append(path, opParams), seen); r != nil { @@ -525,10 +529,10 @@ func find(obj types.Object, T types.Type, path []byte, seen map[*types.TypeName] panic(T) } -func findTypeParam(obj types.Object, list *types.TypeParamList, path []byte, seen map[*types.TypeName]bool) []byte { +func findTypeParam(obj types.Object, list *types.TypeParamList, path []byte, op byte, seen map[*types.TypeName]bool) []byte { for i := 0; i < list.Len(); i++ { tparam := list.At(i) - path2 := appendOpArg(path, opTypeParam, i) + path2 := appendOpArg(path, op, i) if r := find(obj, tparam, path2, seen); r != nil { return r } @@ -580,10 +584,10 @@ func Object(pkg *types.Package, p Path) (types.Object, error) { code := suffix[0] suffix = suffix[1:] - // Codes [AFM] have an integer operand. + // Codes [AFMTr] have an integer operand. var index int switch code { - case opAt, opField, opMethod, opTypeParam: + case opAt, opField, opMethod, opTypeParam, opRecvTypeParam: rest := strings.TrimLeft(suffix, "0123456789") numerals := suffix[:len(suffix)-len(rest)] suffix = rest @@ -664,6 +668,17 @@ func Object(pkg *types.Package, p Path) (types.Object, error) { } t = tparams.At(index) + case opRecvTypeParam: + sig, ok := t.(*types.Signature) // Signature + if !ok { + return nil, fmt.Errorf("cannot apply %q to %s (got %T, want signature)", code, t, t) + } + rtparams := sig.RecvTypeParams() + if n := rtparams.Len(); index >= n { + return nil, fmt.Errorf("tuple index %d out of range [0-%d)", index, n) + } + t = rtparams.At(index) + case opConstraint: tparam, ok := t.(*types.TypeParam) if !ok { @@ -725,6 +740,10 @@ func Object(pkg *types.Package, p Path) (types.Object, error) { } } + if obj == nil { + panic(p) // path does not end in an object-valued operator + } + if obj.Pkg() != pkg { return nil, fmt.Errorf("path denotes %s, which belongs to a different package", obj) } diff --git a/go/types/objectpath/objectpath_go118_test.go b/go/types/objectpath/objectpath_go118_test.go index f061fd8d695..0effe3b9b2b 100644 --- a/go/types/objectpath/objectpath_go118_test.go +++ b/go/types/objectpath/objectpath_go118_test.go @@ -39,6 +39,8 @@ func F[FP0 any, FP1 interface{ M() }](FP0, FP1) {} {"b", "T", "type b.T[TP0 any, TP1 interface{M0(); M1()}] struct{}", ""}, {"b", "T.O", "type b.T[TP0 any, TP1 interface{M0(); M1()}] struct{}", ""}, {"b", "T.M0", "func (b.T[RP0, RP1]).M()", ""}, + {"b", "T.M0.r1O", "type parameter RP1 interface{M0(); M1()}", ""}, + {"b", "T.M0.r1CM1", "func (interface).M1()", ""}, {"b", "T.T0O", "type parameter TP0 any", ""}, {"b", "T.T1O", "type parameter TP1 interface{M0(); M1()}", ""}, {"b", "T.T1CM0", "func (interface).M0()", ""}, diff --git a/gopls/README.md b/gopls/README.md index 0b5f4ade769..6602e0c27a7 100644 --- a/gopls/README.md +++ b/gopls/README.md @@ -3,7 +3,9 @@ [![PkgGoDev](https://pkg.go.dev/badge/golang.org/x/tools/gopls)](https://pkg.go.dev/golang.org/x/tools/gopls) `gopls` (pronounced "Go please") is the official Go [language server] developed -by the Go team. It provides IDE features to any [LSP]-compatible editor. +by the Go team. +It provides a wide variety of [IDE features](doc/features/README.md) +to any [LSP]-compatible editor. @@ -11,11 +13,16 @@ You should not need to interact with `gopls` directly--it will be automatically integrated into your editor. The specific features and settings vary slightly by editor, so we recommend that you proceed to the [documentation for your editor](#editors) below. +Also, the gopls documentation for each feature describes whether it is +supported in each client editor. ## Editors To get started with `gopls`, install an LSP plugin in your editor of choice. +TODO: ensure that each editor has a local page (and move these to doc/clients/$EDITOR.md). +TODO: also, be more consistent about editor (e.g. Emacs) vs. client (e.g. eglot). + * [VS Code](https://github.com/golang/vscode-go/blob/master/README.md) * [Vim / Neovim](doc/vim.md) * [Emacs](doc/emacs.md) @@ -80,16 +87,30 @@ that it officially supports only the two most recent major Go releases. Until August 2024, the Go team will also maintain best-effort support for the last 4 major Go releases, as described in [issue #39146](https://go.dev/issues/39146). -Starting with the release of Go 1.23.0 and gopls@v0.17.0 in August 2024, the -gopls build will depend on the latest version of Go. However, due to the -[forward compatibility](https://go.dev/blog/toolchain) support added to the -`go` command in Go 1.21, as long as Go 1.21 or later are used to install gopls, -the toolchain upgrade will be handled automatically, just like any other -dependency. Gopls will continue to support integrating with the two most recent -major Go releases of the `go` command, per the Go Release Policy. See -[issue #65917](https://go.dev/issue/65917) for more details. - -Maintaining support for legacy versions of Go caused +When using gopls, there are three versions to be aware of: +1. The _gopls build go version_: the version of Go used to build gopls. +2. The _go command version_: the version of the go list command executed by + gopls to load information about your workspace. +3. The _language version_: the version in the go directive of the current + file's enclosing go.mod file, which determines the file's Go language + semantics. + +Starting with the release of Go 1.23.0 and gopls@v0.17.0 in August 2024, we +will only support the most recent Go version as the _gopls build go version_. +However, due to the [forward compatibility](https://go.dev/blog/toolchain) +support added in Go 1.21, as long as Go 1.21 or later are used to install +gopls, any necessary toolchain upgrade will be handled automatically, just like +any other dependency. + +Additionally, starting with gopls@v0.17.0, the _go command version_ will narrow +from 4 versions to 3. This is more consistent with the Go Release Policy. + +Gopls supports **all** Go versions as its _language version_, by providing +compiler errors based on the language version and filtering available standard +library symbols based on the standard library APIs available at that Go +version. + +Maintaining support for building gopls with legacy versions of Go caused [significant friction](https://go.dev/issue/50825) for gopls maintainers and held back other improvements. If you are unable to install a supported version of Go on your system, you can still install an older version of gopls. The @@ -123,8 +144,9 @@ If you are having issues with `gopls`, please follow the steps described in the ## Additional information -* [Features](doc/features.md) +* [Index of features](doc/features/README.md) * [Command-line interface](doc/command-line.md) +* [Configuration settings](doc/settings.md) * [Advanced topics](doc/advanced.md) * [Contributing to `gopls`](doc/contributing.md) * [Integrating `gopls` with an editor](doc/design/integrating.md) diff --git a/gopls/doc/advanced.md b/gopls/doc/advanced.md index 7159626306d..4c5e6015fd7 100644 --- a/gopls/doc/advanced.md +++ b/gopls/doc/advanced.md @@ -1,4 +1,4 @@ -# Advanced topics +# Gopls: Advanced topics This documentation is for advanced `gopls` users, who may want to test unreleased versions or try out special features. @@ -9,7 +9,7 @@ To get a specific version of `gopls` (for example, to test a prerelease version), run: ```sh -GO111MODULE=on go install golang.org/x/tools/gopls@vX.Y.Z +$ go install golang.org/x/tools/gopls@vX.Y.Z ``` Where `vX.Y.Z` is the desired version. @@ -54,27 +54,4 @@ Note that you must work inside the `GOROOT/src` subdirectory, as the `go` command does not recognize `go.work` files in a parent of `GOROOT/src` (https://go.dev/issue/59429). -## Working with generic code - -Gopls has support for editing generic Go code. To enable this support, you need -to **install gopls using Go 1.18 or later**. The easiest way to do this is by -[installing Go 1.18+](https://go.dev/dl) and then using this Go version to -install gopls: - -``` -$ go install golang.org/x/tools/gopls@latest -``` - -It is strongly recommended that you install the latest version of `gopls`, or -the latest **unstable** version as [described above](#installing-unreleased-versions). - -The `gopls` built with these instructions understands generic code. See the -[generics tutorial](https://go.dev/doc/tutorial/generics) for more information -on how to use generics in Go! - -### Known issues - - * [`staticcheck`](https://github.com/golang/tools/blob/master/gopls/doc/settings.md#staticcheck) - on generic code is not supported yet. - [Go project]: https://go.googlesource.com/go diff --git a/gopls/doc/analyzers.md b/gopls/doc/analyzers.md index b590120985e..45db766719f 100644 --- a/gopls/doc/analyzers.md +++ b/gopls/doc/analyzers.md @@ -1,4 +1,4 @@ -# Analyzers +# Gopls: Analyzers @@ -15,19 +15,20 @@ before you run your tests, or even before you save your files. This document describes the suite of analyzers available in gopls, which aggregates analyzers from a variety of sources: -- all the usual bug-finding analyzers from the `go vet` suite; -- a number of analyzers with more substantial dependencies that prevent them from being used in `go vet`; -- analyzers that augment compilation errors by suggesting quick fixes to common mistakes; and -- a handful of analyzers that suggest possible style improvements. +- all the usual bug-finding analyzers from the `go vet` suite (e.g. `printf`; see [`go tool vet help`](https://pkg.go.dev/cmd/vet) for the complete list); +- a number of analyzers with more substantial dependencies that prevent them from being used in `go vet` (e.g. `nilness`); +- analyzers that augment compilation errors by suggesting quick fixes to common mistakes (e.g. `fillreturns`); and +- a handful of analyzers that suggest possible style improvements (e.g. `simplifyrange`). -More details about how to enable and disable analyzers can be found -[here](settings.md#analyses). +To enable or disable analyzers, use the [analyses](settings.md#analyses) setting. In addition, gopls includes the [`staticcheck` suite](https://staticcheck.dev/docs/checks), though these analyzers are off by default. Use the [`staticcheck`](settings.md#staticcheck`) setting to enable them, and consult staticcheck's documentation for analyzer details. + + @@ -257,40 +258,6 @@ Default: on. Package documentation: [errorsas](https://pkg.go.dev/golang.org/x/tools/go/analysis/passes/errorsas) - -## `fieldalignment`: find structs that would use less memory if their fields were sorted - - -This analyzer find structs that can be rearranged to use less memory, and provides -a suggested edit with the most compact order. - -Note that there are two different diagnostics reported. One checks struct size, -and the other reports "pointer bytes" used. Pointer bytes is how many bytes of the -object that the garbage collector has to potentially scan for pointers, for example: - - struct { uint32; string } - -have 16 pointer bytes because the garbage collector has to scan up through the string's -inner pointer. - - struct { string; *uint32 } - -has 24 pointer bytes because it has to scan further through the *uint32. - - struct { string; uint32 } - -has 8 because it can stop immediately after the string pointer. - -Be aware that the most compact order is not always the most efficient. -In rare cases it may cause two variables each updated by its own goroutine -to occupy the same CPU cache line, inducing a form of memory contention -known as "false sharing" that slows down both goroutines. - - -Default: off. Enable by setting `"analyses": {"fieldalignment": true}`. - -Package documentation: [fieldalignment](https://pkg.go.dev/golang.org/x/tools/go/analysis/passes/fieldalignment) - ## `fillreturns`: suggest fixes for errors due to an incorrect number of return values diff --git a/gopls/doc/assets/assets.go b/gopls/doc/assets/assets.go new file mode 100644 index 00000000000..6fef4a886a1 --- /dev/null +++ b/gopls/doc/assets/assets.go @@ -0,0 +1,3 @@ +// Package assets is an empty package to appease "go test ./...", +// as run by our CI builders, which doesn't like an empty module. +package assets diff --git a/gopls/doc/assets/browse-assembly.png b/gopls/doc/assets/browse-assembly.png new file mode 100644 index 00000000000..93ae8d215e7 Binary files /dev/null and b/gopls/doc/assets/browse-assembly.png differ diff --git a/gopls/doc/assets/browse-free-symbols.png b/gopls/doc/assets/browse-free-symbols.png new file mode 100644 index 00000000000..c521f52dcc7 Binary files /dev/null and b/gopls/doc/assets/browse-free-symbols.png differ diff --git a/gopls/doc/assets/browse-pkg-doc.png b/gopls/doc/assets/browse-pkg-doc.png new file mode 100644 index 00000000000..32db6167c2b Binary files /dev/null and b/gopls/doc/assets/browse-pkg-doc.png differ diff --git a/gopls/doc/assets/code-action-doc.png b/gopls/doc/assets/code-action-doc.png new file mode 100644 index 00000000000..2f2e6e476b8 Binary files /dev/null and b/gopls/doc/assets/code-action-doc.png differ diff --git a/gopls/doc/assets/convert-string-interpreted.png b/gopls/doc/assets/convert-string-interpreted.png new file mode 100644 index 00000000000..6bb7f2a9b35 Binary files /dev/null and b/gopls/doc/assets/convert-string-interpreted.png differ diff --git a/gopls/doc/assets/convert-string-raw.png b/gopls/doc/assets/convert-string-raw.png new file mode 100644 index 00000000000..24dea626eb1 Binary files /dev/null and b/gopls/doc/assets/convert-string-raw.png differ diff --git a/gopls/doc/assets/diagnostic-analysis.png b/gopls/doc/assets/diagnostic-analysis.png new file mode 100644 index 00000000000..5a934d0d6e6 Binary files /dev/null and b/gopls/doc/assets/diagnostic-analysis.png differ diff --git a/gopls/doc/assets/diagnostic-typeerror.png b/gopls/doc/assets/diagnostic-typeerror.png new file mode 100644 index 00000000000..8f78228893c Binary files /dev/null and b/gopls/doc/assets/diagnostic-typeerror.png differ diff --git a/gopls/doc/assets/document-highlight.png b/gopls/doc/assets/document-highlight.png new file mode 100644 index 00000000000..ded4564c027 Binary files /dev/null and b/gopls/doc/assets/document-highlight.png differ diff --git a/gopls/doc/assets/documentlink.png b/gopls/doc/assets/documentlink.png new file mode 100644 index 00000000000..8bc5e3d05e6 Binary files /dev/null and b/gopls/doc/assets/documentlink.png differ diff --git a/gopls/doc/assets/extract-function-after.png b/gopls/doc/assets/extract-function-after.png new file mode 100644 index 00000000000..4599a827a0e Binary files /dev/null and b/gopls/doc/assets/extract-function-after.png differ diff --git a/gopls/doc/assets/extract-function-before.png b/gopls/doc/assets/extract-function-before.png new file mode 100644 index 00000000000..9c2590b41c3 Binary files /dev/null and b/gopls/doc/assets/extract-function-before.png differ diff --git a/gopls/doc/assets/extract-to-new-file-after.png b/gopls/doc/assets/extract-to-new-file-after.png new file mode 100644 index 00000000000..3f0aa856091 Binary files /dev/null and b/gopls/doc/assets/extract-to-new-file-after.png differ diff --git a/gopls/doc/assets/extract-to-new-file-before.png b/gopls/doc/assets/extract-to-new-file-before.png new file mode 100644 index 00000000000..9c05ceb9db1 Binary files /dev/null and b/gopls/doc/assets/extract-to-new-file-before.png differ diff --git a/gopls/doc/assets/extract-var-after.png b/gopls/doc/assets/extract-var-after.png new file mode 100644 index 00000000000..db558d6736a Binary files /dev/null and b/gopls/doc/assets/extract-var-after.png differ diff --git a/gopls/doc/assets/extract-var-before.png b/gopls/doc/assets/extract-var-before.png new file mode 100644 index 00000000000..356a242db3c Binary files /dev/null and b/gopls/doc/assets/extract-var-before.png differ diff --git a/gopls/doc/assets/fill-struct-after.png b/gopls/doc/assets/fill-struct-after.png new file mode 100644 index 00000000000..61662287e10 Binary files /dev/null and b/gopls/doc/assets/fill-struct-after.png differ diff --git a/gopls/doc/assets/fill-struct-before.png b/gopls/doc/assets/fill-struct-before.png new file mode 100644 index 00000000000..fd544921a6d Binary files /dev/null and b/gopls/doc/assets/fill-struct-before.png differ diff --git a/gopls/doc/assets/fill-switch-after.png b/gopls/doc/assets/fill-switch-after.png new file mode 100644 index 00000000000..33d1bd34c4a Binary files /dev/null and b/gopls/doc/assets/fill-switch-after.png differ diff --git a/gopls/doc/assets/fill-switch-before.png b/gopls/doc/assets/fill-switch-before.png new file mode 100644 index 00000000000..f25af03b9c8 Binary files /dev/null and b/gopls/doc/assets/fill-switch-before.png differ diff --git a/gopls/doc/assets/fill-switch-enum-after.png b/gopls/doc/assets/fill-switch-enum-after.png new file mode 100644 index 00000000000..564be177976 Binary files /dev/null and b/gopls/doc/assets/fill-switch-enum-after.png differ diff --git a/gopls/doc/assets/fill-switch-enum-before.png b/gopls/doc/assets/fill-switch-enum-before.png new file mode 100644 index 00000000000..85150347fb0 Binary files /dev/null and b/gopls/doc/assets/fill-switch-enum-before.png differ diff --git a/gopls/doc/assets/foldingrange.png b/gopls/doc/assets/foldingrange.png new file mode 100644 index 00000000000..19e7645b266 Binary files /dev/null and b/gopls/doc/assets/foldingrange.png differ diff --git a/gopls/doc/assets/go.mod b/gopls/doc/assets/go.mod new file mode 100644 index 00000000000..73f49695726 --- /dev/null +++ b/gopls/doc/assets/go.mod @@ -0,0 +1,7 @@ +// This module contains no Go code, but serves to carve out a hole in +// its parent module to avoid bloating it with large image files that +// would otherwise be dowloaded by "go install golang.org/x/tools/gopls@latest". + +module golang.org/x/tools/gopls/doc/assets + +go 1.19 diff --git a/gopls/doc/assets/hover-basic.png b/gopls/doc/assets/hover-basic.png new file mode 100644 index 00000000000..687ff71c162 Binary files /dev/null and b/gopls/doc/assets/hover-basic.png differ diff --git a/gopls/doc/assets/hover-doclink.png b/gopls/doc/assets/hover-doclink.png new file mode 100644 index 00000000000..dcee92b2d98 Binary files /dev/null and b/gopls/doc/assets/hover-doclink.png differ diff --git a/gopls/doc/assets/hover-embed.png b/gopls/doc/assets/hover-embed.png new file mode 100644 index 00000000000..4d877a283da Binary files /dev/null and b/gopls/doc/assets/hover-embed.png differ diff --git a/gopls/doc/assets/hover-field-tag.png b/gopls/doc/assets/hover-field-tag.png new file mode 100644 index 00000000000..f36640c0317 Binary files /dev/null and b/gopls/doc/assets/hover-field-tag.png differ diff --git a/gopls/doc/assets/hover-linkname.png b/gopls/doc/assets/hover-linkname.png new file mode 100644 index 00000000000..c547d52f7a4 Binary files /dev/null and b/gopls/doc/assets/hover-linkname.png differ diff --git a/gopls/doc/assets/hover-size-field.png b/gopls/doc/assets/hover-size-field.png new file mode 100644 index 00000000000..090d0ff17a9 Binary files /dev/null and b/gopls/doc/assets/hover-size-field.png differ diff --git a/gopls/doc/assets/hover-size-struct.png b/gopls/doc/assets/hover-size-struct.png new file mode 100644 index 00000000000..4af9a33ec04 Binary files /dev/null and b/gopls/doc/assets/hover-size-struct.png differ diff --git a/gopls/doc/assets/hover-size-wasteful.png b/gopls/doc/assets/hover-size-wasteful.png new file mode 100644 index 00000000000..6d907fb446c Binary files /dev/null and b/gopls/doc/assets/hover-size-wasteful.png differ diff --git a/gopls/doc/assets/inlayhint-parameternames.png b/gopls/doc/assets/inlayhint-parameternames.png new file mode 100644 index 00000000000..83d934e1ca6 Binary files /dev/null and b/gopls/doc/assets/inlayhint-parameternames.png differ diff --git a/gopls/doc/assets/invert-if-after.png b/gopls/doc/assets/invert-if-after.png new file mode 100644 index 00000000000..d66dc8e92f7 Binary files /dev/null and b/gopls/doc/assets/invert-if-after.png differ diff --git a/gopls/doc/assets/invert-if-before.png b/gopls/doc/assets/invert-if-before.png new file mode 100644 index 00000000000..48581d2f3d8 Binary files /dev/null and b/gopls/doc/assets/invert-if-before.png differ diff --git a/gopls/doc/assets/outgoingcalls.png b/gopls/doc/assets/outgoingcalls.png new file mode 100644 index 00000000000..00ca4b1a50a Binary files /dev/null and b/gopls/doc/assets/outgoingcalls.png differ diff --git a/gopls/doc/assets/remove-unusedparam-after.png b/gopls/doc/assets/remove-unusedparam-after.png new file mode 100644 index 00000000000..04193fdcb18 Binary files /dev/null and b/gopls/doc/assets/remove-unusedparam-after.png differ diff --git a/gopls/doc/assets/remove-unusedparam-before.png b/gopls/doc/assets/remove-unusedparam-before.png new file mode 100644 index 00000000000..4e49c4294fb Binary files /dev/null and b/gopls/doc/assets/remove-unusedparam-before.png differ diff --git a/gopls/doc/assets/rename-conflict.png b/gopls/doc/assets/rename-conflict.png new file mode 100644 index 00000000000..105a6ee15de Binary files /dev/null and b/gopls/doc/assets/rename-conflict.png differ diff --git a/gopls/doc/assets/signature-help.png b/gopls/doc/assets/signature-help.png new file mode 100644 index 00000000000..ca537787475 Binary files /dev/null and b/gopls/doc/assets/signature-help.png differ diff --git a/gopls/doc/codelenses.md b/gopls/doc/codelenses.md index 378a3db1732..71425ce09c6 100644 --- a/gopls/doc/codelenses.md +++ b/gopls/doc/codelenses.md @@ -1,4 +1,4 @@ -# Code Lenses +# Gopls: Code lenses A "code lens" is a command associated with a range of a source file. The VS Code manual describes code lenses as @@ -14,9 +14,16 @@ They can be enabled and disabled using the [`codelenses`](settings.md#codelenses) setting. Their features are subject to change. +Client support: +- **VS Code**: Code Lenses appear as small text links above a line of source code. +- **Emacs + eglot**: Not supported, but prototype exists at https://github.joaotavora/eglot/pull/71. +- **Vim + coc.nvim**: ?? +- **CLI**: `gopls codelens`. For example, `gopls codelens -exec file.go:123 "run test"` runs the test at the specified line. + + -## ⬤ `gc_details`: Toggle display of Go compiler optimization decisions +## `gc_details`: Toggle display of Go compiler optimization decisions This codelens source causes the `package` declaration of @@ -40,7 +47,7 @@ Default: off File type: Go -## ⬤ `generate`: Run `go generate` +## `generate`: Run `go generate` This codelens source annotates any `//go:generate` comments @@ -55,7 +62,7 @@ Default: on File type: Go -## ⬤ `regenerate_cgo`: Re-generate cgo declarations +## `regenerate_cgo`: Re-generate cgo declarations This codelens source annotates an `import "C"` declaration @@ -71,7 +78,7 @@ Default: on File type: Go -## ⬤ `test`: Run tests and benchmarks +## `test`: Run tests and benchmarks This codelens source annotates each `Test` and `Benchmark` @@ -90,7 +97,7 @@ Default: off File type: Go -## ⬤ `run_govulncheck`: Run govulncheck +## `run_govulncheck`: Run govulncheck This codelens source annotates the `module` directive in a @@ -107,7 +114,7 @@ Default: off File type: go.mod -## ⬤ `tidy`: Tidy go.mod file +## `tidy`: Tidy go.mod file This codelens source annotates the `module` directive in a @@ -120,7 +127,7 @@ Default: on File type: go.mod -## ⬤ `upgrade_dependency`: Update dependencies +## `upgrade_dependency`: Update dependencies This codelens source annotates the `module` directive in a @@ -135,7 +142,7 @@ Default: on File type: go.mod -## ⬤ `vendor`: Update vendor directory +## `vendor`: Update vendor directory This codelens source annotates the `module` directive in a diff --git a/gopls/doc/command-line.md b/gopls/doc/command-line.md index 465905873c1..4f825e21b89 100644 --- a/gopls/doc/command-line.md +++ b/gopls/doc/command-line.md @@ -1,15 +1,35 @@ -# Command line +# Gopls: Command-line interface -**Note: The `gopls` command-line is still experimental and subject to change at any point.** +The `gopls` command provides a number of subcommands that expose much +of the server's functionality. However, the interface is currently +**experimental** and **subject to change at any point.** +It is not efficient, complete, flexible, or officially supported. -`gopls` exposes some (but not all) features on the command-line. This can be useful for debugging `gopls` itself. +Its primary use is as a debugging aid. +For example, this command reports the location of references to the +symbol at the specified file/line/column: - +``` +$ gopls references ./gopls/main.go:35:8 +Log: Loading packages... +Info: Finished loading packages. +/home/gopher/xtools/go/packages/gopackages/main.go:27:7-11 +/home/gopher/xtools/gopls/internal/cmd/integration_test.go:1062:7-11 +/home/gopher/xtools/gopls/internal/test/integration/bench/bench_test.go:59:8-12 +/home/gopher/xtools/gopls/internal/test/integration/regtest.go:140:8-12 +/home/gopher/xtools/gopls/main.go:35:7-11 +``` -Learn about available commands and flags by running `gopls help`. - -Much of the functionality of `gopls` is available through a command line interface. +See golang/go#63693 for a discussion of its future. -There are two main reasons for this. The first is that we do not want users to rely on separate command line tools when they wish to do some task outside of an editor. The second is that the CLI assists in debugging. It is easier to reproduce behavior via single command. +Learn about available commands and flags by running `gopls help`. -It is not a goal of `gopls` to be a high performance command line tool. Its command line is intended for single file/package user interaction speeds, not bulk processing. +Positions within files are specified as `file.go:line:column` triples, +where the line and column start at 1, and columns are measured in +bytes of the UTF-8 encoding. +Alternatively, positions may be specified by the byte offset within +the UTF-8 encoding of the file, starting from zero, for example +`file.go:#1234`. +(When working in non-ASCII files, beware that your editor may report a +position's offset within its file using a different measure such as +UTF-16 codes, Unicode code points, or graphemes). diff --git a/gopls/doc/commands.md b/gopls/doc/commands.md index c1c5bead121..8d0c870747e 100644 --- a/gopls/doc/commands.md +++ b/gopls/doc/commands.md @@ -1,10 +1,24 @@ -# Commands +# Gopls: Commands -This document describes the LSP-level commands supported by `gopls`. They cannot be invoked directly by users, and all the details are subject to change, so nobody should rely on this information. +The LSP's `workspace/executeCommand` RPC is an extension mechanism +that allows clients to invoke ad hoc commands offered by servers. +This document describes the commands supported by `gopls`. + +Most commands provide the implementations of features advertised +through documented LSP mechanisms such as +[Code Lenses](settings.md#code-lenses) and +[Code Actions](features/transformation.md#code-actions). + +They are not intended to be invoked directly by clients, +and typically editors do not make them directly accessible. + +We document them for completeness, but these interfaces +are not stable and may change without notice. +TODO(rfindley): unpublish and remove this page. -### **Add a dependency** -Identifier: `gopls.add_dependency` + +## `gopls.add_dependency`: **Add a dependency** Adds a dependency to the go.mod file for a module. @@ -21,8 +35,8 @@ Args: } ``` -### **Add an import** -Identifier: `gopls.add_import` + +## `gopls.add_import`: **Add an import** Ask the server to add an import path to a given Go file. The method will call applyEdit on the client so that clients don't have to apply the edit @@ -41,8 +55,8 @@ Args: } ``` -### **Update the given telemetry counters** -Identifier: `gopls.add_telemetry_counters` + +## `gopls.add_telemetry_counters`: **Update the given telemetry counters** Gopls will prepend "fwd/" to all the counters updated using this command to avoid conflicts with other counters gopls collects. @@ -57,8 +71,8 @@ Args: } ``` -### **Apply a fix** -Identifier: `gopls.apply_fix` + +## `gopls.apply_fix`: **Apply a fix** Applies a fix to a region of source code. @@ -144,8 +158,23 @@ Result: } ``` -### **Perform a "change signature" refactoring** -Identifier: `gopls.change_signature` + +## `gopls.assembly`: **Browse assembly listing of current function in a browser.** + +This command opens a web-based disassembly listing of the +specified function symbol (plus any nested lambdas and defers). +The machine architecture is determined by the view. + +Args: + +``` +string, +string, +string +``` + + +## `gopls.change_signature`: **Perform a "change signature" refactoring** This command is experimental, currently only supporting parameter removal. Its signature will certainly change in the future (pun intended). @@ -217,8 +246,8 @@ Result: } ``` -### **Check for upgrades** -Identifier: `gopls.check_upgrades` + +## `gopls.check_upgrades`: **Check for upgrades** Checks for module upgrades. @@ -233,8 +262,8 @@ Args: } ``` -### **Cause server to publish diagnostics for the specified files.** -Identifier: `gopls.diagnose_files` + +## `gopls.diagnose_files`: **Cause server to publish diagnostics for the specified files.** This command is needed by the 'gopls {check,fix}' CLI subcommands. @@ -246,8 +275,8 @@ Args: } ``` -### **View package documentation.** -Identifier: `gopls.doc` + +## `gopls.doc`: **Browse package documentation.** Opens the Go package documentation page for the current package in a browser. @@ -270,8 +299,8 @@ Args: } ``` -### **Run go mod edit -go=version** -Identifier: `gopls.edit_go_directive` + +## `gopls.edit_go_directive`: **Run go mod edit -go=version** Runs `go mod edit -go=version` for a module. @@ -286,8 +315,31 @@ Args: } ``` -### **Get known vulncheck result** -Identifier: `gopls.fetch_vulncheck_result` + +## `gopls.extract_to_new_file`: **Move selected declarations to a new file** + +Used by the code action of the same name. + +Args: + +``` +{ + "uri": string, + "range": { + "start": { + "line": uint32, + "character": uint32, + }, + "end": { + "line": uint32, + "character": uint32, + }, + }, +} +``` + + +## `gopls.fetch_vulncheck_result`: **Get known vulncheck result** Fetch the result of latest vulnerability check (`govulncheck`). @@ -306,8 +358,8 @@ Result: map[golang.org/x/tools/gopls/internal/protocol.DocumentURI]*golang.org/x/tools/gopls/internal/vulncheck.Result ``` -### **report free symbols referenced by the selection.** -Identifier: `gopls.free_symbols` + +## `gopls.free_symbols`: **Browse free symbols referenced by the selection in a browser.** This command is a query over a selected range of Go source code. It reports the set of "free" symbols of the @@ -322,21 +374,22 @@ Args: ``` string, { - // The range's start position. - "start": { - "line": uint32, - "character": uint32, - }, - // The range's end position. - "end": { - "line": uint32, - "character": uint32, + "uri": string, + "range": { + "start": { + "line": uint32, + "character": uint32, + }, + "end": { + "line": uint32, + "character": uint32, + }, }, } ``` -### **Toggle gc_details** -Identifier: `gopls.gc_details` + +## `gopls.gc_details`: **Toggle gc_details** Toggle the calculation of gc annotations. @@ -346,8 +399,8 @@ Args: string ``` -### **Run go generate** -Identifier: `gopls.generate` + +## `gopls.generate`: **Run go generate** Runs `go generate` for a given directory. @@ -362,8 +415,8 @@ Args: } ``` -### **'go get' a package** -Identifier: `gopls.go_get_package` + +## `gopls.go_get_package`: **'go get' a package** Runs `go get` to fetch a package. @@ -379,8 +432,8 @@ Args: } ``` -### **List imports of a file and its package** -Identifier: `gopls.list_imports` + +## `gopls.list_imports`: **List imports of a file and its package** Retrieve a list of imports in the given Go file, and the package it belongs to. @@ -410,8 +463,8 @@ Result: } ``` -### **List known packages** -Identifier: `gopls.list_known_packages` + +## `gopls.list_known_packages`: **List known packages** Retrieve a list of packages that are importable from the given URI. @@ -437,15 +490,15 @@ Result: } ``` -### **Prompt user to enable telemetry** -Identifier: `gopls.maybe_prompt_for_telemetry` + +## `gopls.maybe_prompt_for_telemetry`: **Prompt user to enable telemetry** Checks for the right conditions, and then prompts the user to ask if they want to enable Go telemetry uploading. If the user responds 'Yes', the telemetry mode is set to "on". -### **Fetch memory statistics** -Identifier: `gopls.mem_stats` + +## `gopls.mem_stats`: **Fetch memory statistics** Call runtime.GC multiple times and return memory statistics as reported by runtime.MemStats. @@ -462,8 +515,8 @@ Result: } ``` -### **Regenerate cgo** -Identifier: `gopls.regenerate_cgo` + +## `gopls.regenerate_cgo`: **Regenerate cgo** Regenerates cgo definitions. @@ -476,8 +529,8 @@ Args: } ``` -### **Remove a dependency** -Identifier: `gopls.remove_dependency` + +## `gopls.remove_dependency`: **Remove a dependency** Removes a dependency from the go.mod file of a module. @@ -496,8 +549,8 @@ Args: } ``` -### **Reset go.mod diagnostics** -Identifier: `gopls.reset_go_mod_diagnostics` + +## `gopls.reset_go_mod_diagnostics`: **Reset go.mod diagnostics** Reset diagnostics in the go.mod file of a module. @@ -514,8 +567,8 @@ Args: } ``` -### **Run `go work [args...]`, and apply the resulting go.work** -Identifier: `gopls.run_go_work_command` + +## `gopls.run_go_work_command`: **Run `go work [args...]`, and apply the resulting go.work** edits to the current go.work file @@ -529,11 +582,13 @@ Args: } ``` -### **Run vulncheck** -Identifier: `gopls.run_govulncheck` + +## `gopls.run_govulncheck`: **Run vulncheck** Run vulnerability check (`govulncheck`). +This command is asynchronous; clients must wait for the 'end' progress notification. + Args: ``` @@ -555,11 +610,13 @@ Result: } ``` -### **Run test(s)** -Identifier: `gopls.run_tests` + +## `gopls.run_tests`: **Run test(s)** Runs `go test` for a specific set of test or benchmark functions. +This command is asynchronous; clients must wait for the 'end' progress notification. + Args: ``` @@ -573,8 +630,13 @@ Args: } ``` -### **Start the gopls debug server** -Identifier: `gopls.start_debugging` + +## `gopls.scan_imports`: **force a sychronous scan of the imports cache.** + +This command is intended for use by gopls tests only. + + +## `gopls.start_debugging`: **Start the gopls debug server** Start the gopls debug server if it isn't running, and return the debug address. @@ -618,8 +680,8 @@ Result: } ``` -### **Start capturing a profile of gopls' execution** -Identifier: `gopls.start_profile` + +## `gopls.start_profile`: **Start capturing a profile of gopls' execution** Start a new pprof profile. Before using the resulting file, profiling must be stopped with a corresponding call to StopProfile. @@ -639,8 +701,8 @@ Result: struct{} ``` -### **Stop an ongoing profile** -Identifier: `gopls.stop_profile` + +## `gopls.stop_profile`: **Stop an ongoing profile** This command is intended for internal use only, by the gopls benchmark runner. @@ -660,11 +722,18 @@ Result: } ``` -### **Run test(s) (legacy)** -Identifier: `gopls.test` + +## `gopls.test`: **Run test(s) (legacy)** Runs `go test` for a specific set of test or benchmark functions. +This command is asynchronous; wait for the 'end' progress notification. + +This command is an alias for RunTests; the only difference +is the form of the parameters. + +TODO(adonovan): eliminate it. + Args: ``` @@ -673,8 +742,8 @@ string, []string ``` -### **Run go mod tidy** -Identifier: `gopls.tidy` + +## `gopls.tidy`: **Run go mod tidy** Runs `go mod tidy` for a module. @@ -687,8 +756,8 @@ Args: } ``` -### **Toggle gc_details** -Identifier: `gopls.toggle_gc_details` + +## `gopls.toggle_gc_details`: **Toggle gc_details** Toggle the calculation of gc annotations. @@ -701,8 +770,8 @@ Args: } ``` -### **Update go.sum** -Identifier: `gopls.update_go_sum` + +## `gopls.update_go_sum`: **Update go.sum** Updates the go.sum file for a module. @@ -715,8 +784,8 @@ Args: } ``` -### **Upgrade a dependency** -Identifier: `gopls.upgrade_dependency` + +## `gopls.upgrade_dependency`: **Upgrade a dependency** Upgrades a dependency in the go.mod file for a module. @@ -733,8 +802,8 @@ Args: } ``` -### **Run go mod vendor** -Identifier: `gopls.vendor` + +## `gopls.vendor`: **Run go mod vendor** Runs `go mod vendor` for a module. @@ -747,8 +816,8 @@ Args: } ``` -### **List current Views on the server.** -Identifier: `gopls.views` + +## `gopls.views`: **List current Views on the server.** This command is intended for use by gopls tests only. @@ -764,8 +833,8 @@ Result: } ``` -### **Fetch workspace statistics** -Identifier: `gopls.workspace_stats` + +## `gopls.workspace_stats`: **Fetch workspace statistics** Query statistics about workspace builds, modules, packages, and files. diff --git a/gopls/doc/contributing.md b/gopls/doc/contributing.md index a2f987b63c9..007c5793073 100644 --- a/gopls/doc/contributing.md +++ b/gopls/doc/contributing.md @@ -1,4 +1,4 @@ -# Documentation for contributors +# Gopls: Documentation for contributors This documentation augments the general documentation for contributing to the x/tools repository, described at the [repository root](../../CONTRIBUTING.md). @@ -165,3 +165,17 @@ telemetry.--> [issue-gopls]: https://github.com/golang/go/issues?utf8=%E2%9C%93&q=is%3Aissue+is%3Aopen+label%3Agopls "gopls issues" [issue-wanted]: https://github.com/golang/go/issues?utf8=✓&q=is%3Aissue+is%3Aopen+label%3Agopls+label%3A"help+wanted" "help wanted" + +## Documentation + +Each CL that adds or changes a feature should include, in addition to +a test that exercises the new behavior: + +- a **release note** that briefly explains the change, and +- **comprehensive documentation** in the [index of features](features/README.md). + +The release note should go in the file named for the forthcoming +release, for example [release/v0.16.0.md](release/v0.16.0.md). (Create +the file if your feature is the first to be added after a release.) + + diff --git a/gopls/doc/daemon.md b/gopls/doc/daemon.md index 86356daf236..0844bc062e7 100644 --- a/gopls/doc/daemon.md +++ b/gopls/doc/daemon.md @@ -1,4 +1,4 @@ -# Running gopls as a daemon +# Gopls: Running as a daemon **Note: this feature is new. If you encounter bugs, please [file an issue](troubleshooting.md#file-an-issue).** diff --git a/gopls/doc/emacs.md b/gopls/doc/emacs.md index 8a54cf19d0a..3b6ee80d05a 100644 --- a/gopls/doc/emacs.md +++ b/gopls/doc/emacs.md @@ -1,4 +1,4 @@ -# Emacs +# Gopls: Using Emacs ## Installing `gopls` @@ -111,11 +111,14 @@ project root. ;; Optional: install eglot-format-buffer as a save hook. ;; The depth of -10 places this before eglot's willSave notification, ;; so that that notification reports the actual contents that will be saved. -(defun eglot-format-buffer-on-save () +(defun eglot-format-buffer-before-save () (add-hook 'before-save-hook #'eglot-format-buffer -10 t)) -(add-hook 'go-mode-hook #'eglot-format-buffer-on-save) +(add-hook 'go-mode-hook #'eglot-format-buffer-before-save) ``` +Use `M-x eglot-upgrade-eglot` to upgrade to the latest version of +Eglot. + ### Configuring `gopls` via Eglot See [settings] for information about available gopls settings. diff --git a/gopls/doc/features.md b/gopls/doc/features.md deleted file mode 100644 index 70a734eadc3..00000000000 --- a/gopls/doc/features.md +++ /dev/null @@ -1,55 +0,0 @@ -# Features - -This document describes some of the features supported by `gopls`. It is -currently under construction, so, for a comprehensive list, see the -[Language Server Protocol](https://microsoft.github.io/language-server-protocol/). - -## Special features - -Here, only special features outside of the LSP are described. - -### Symbol Queries - -Gopls supports some extended syntax for `workspace/symbol` requests, when using -the `fuzzy` symbol matcher (the default). Inspired by the popular fuzzy matcher -[FZF](https://github.com/junegunn/fzf), the following special characters are -supported within symbol queries: - -| Character | Usage | Match | -| --------- | --------- | ------------ | -| `'` | `'abc` | exact | -| `^` | `^printf` | exact prefix | -| `$` | `printf$` | exact suffix | - -## Template Files - -Gopls provides some support for Go template files, that is, files that -are parsed by `text/template` or `html/template`. -Gopls recognizes template files based on their file extension, which may be -configured by the -[`templateExtensions`](https://github.com/golang/tools/blob/master/gopls/doc/settings.md#templateextensions) setting. -Making this list empty turns off template support. - -In template files, template support works inside -the default `{{` delimiters. (Go template parsing -allows the user to specify other delimiters, but -gopls does not know how to do that.) - -Gopls template support includes the following features: -+ **Diagnostics**: if template parsing returns an error, -it is presented as a diagnostic. (Missing functions do not produce errors.) -+ **Syntax Highlighting**: syntax highlighting is provided for template files. -+ **Definitions**: gopls provides jump-to-definition inside templates, though it does not understand scoping (all templates are considered to be in one global scope). -+ **References**: gopls provides find-references, with the same scoping limitation as definitions. -+ **Completions**: gopls will attempt to suggest completions inside templates. - -### Configuring your editor - -In addition to configuring `templateExtensions`, you may need to configure your -editor or LSP client to activate `gopls` for template files. For example, in -`VS Code` you will need to configure both -[`files.associations`](https://code.visualstudio.com/docs/languages/identifiers) -and `build.templateExtensions` (the gopls setting). - - - diff --git a/gopls/doc/features/README.md b/gopls/doc/features/README.md new file mode 100644 index 00000000000..fc559fd9ab3 --- /dev/null +++ b/gopls/doc/features/README.md @@ -0,0 +1,61 @@ +# Gopls: Index of features + +This page provides an index of all supported features of gopls that +are accessible through the [language server protocol](https://microsoft.github.io/language-server-protocol/) (LSP). +It is intended for: +- **users of gopls** learning its capabilities so that they get the most out of their editor; +- **editor maintainers** adding or improving Go support in an LSP-capable editor; and +- **contributors to gopls** trying to understand how it works. + +In an ideal world, Go users would not need to know that gopls or even +LSP exists, as their LSP-enabled editors would implement every facet +of the protocol and expose each feature in a natural and discoverable +way. In reality, editors vary widely in their support for LSP, so +unfortunately these documents necessarily involve many details of the +protocol. + +We also list [settings](../settings.md) that affect each feature. + +Most features are illustrated with reference to VS Code, but we will +briefly mention whether each feature is supported in other popular +clients, and if so, how to find it. We welcome contributions, edits, +and updates from users of any editor. + +Contributors should [update this documentation](../contributing.md#documentation) +when making significant changes to existing features or when adding new ones. + +- [Passive](passive.md): features that are always on and require no special action + - [Hover](passive.md#hover): information about the symbol under the cursor + - [Signature Help](passive.md#signature-help): type information about the enclosing function call + - [Document Highlight](passive.md#document-highlight): highlight identifiers referring to the same symbol + - [Inlay Hint](passive.md#inlay-hint): show implicit names of struct fields and parameter names + - [Semantic Tokens](passive.md#semantic-tokens): report syntax information used by editors to color the text + - [Folding Range](passive.md#folding-range): report text regions that can be "folded" (expanded/collapsed) in an editor + - [Document Link](passive.md#document-link): extracts URLs from doc comments, strings in current file so client can linkify +- [Diagnostics](diagnostics.md): compile errors and static analysis findings +- [Navigation](navigation.md): navigation of cross-references, types, and symbols + - [Definition](navigation.md#definition): go to definition of selected symbol + - [Type Definition](navigation.md#type-definition): go to definition of type of selected symbol + - [References](navigation.md#references): list references to selected symbol + - [Implementation](navigation.md#implementation): show "implements" relationships of selected type + - [Document Symbol](passive.md#document-symbol): outline of symbols defined in current file + - [Symbol](navigation.md#symbol): fuzzy search for symbol by name + - [Selection Range](navigation.md#selection-range): select enclosing unit of syntax + - [Call Hierarchy](navigation.md#call-hierarchy): show outgoing/incoming calls to the current function +- [Completion](completion.md): context-aware completion of identifiers, statements +- [Code transformation](transformation.md): fixes and refactorings + - [Formatting](transformation.md#formatting): format the source code + - [Rename](transformation.md#rename): rename a symbol or package + - [Organize imports](transformation.md#organize-imports): organize the import declaration + - [Extract](transformation.md#extract): extract selection to a new file/function/variable + - [Inline](transformation.md#inline): inline a call to a function or method + - [Miscellaneous rewrites](transformation.md#miscellaneous-rewrites): various Go-specific refactorings +- [Web-based queries](web.md): commands that open a browser page + - [Package documentation](web.md#doc): browse documentation for current Go package + - [Free symbols](web.md#freesymbols): show symbols used by a selected block of code + - [Assembly](web.md#assembly): show listing of assembly code for selected function +- Support for non-Go files: + - [Template files](templates.md): files parsed by `text/template` and `html/template` + - [go.mod and go.work files](modfiles.md): Go module and workspace manifests +- [Command-line interface](../command-line.md): CLI for debugging and scripting (unstable) +- [Non-standard commands](../commands.md): gopls-specific RPC protocol extensions (unstable) diff --git a/gopls/doc/features/completion.md b/gopls/doc/features/completion.md new file mode 100644 index 00000000000..46991aab05e --- /dev/null +++ b/gopls/doc/features/completion.md @@ -0,0 +1,3 @@ +# Gopls: Completion + +TODO(golang/go#62022): document diff --git a/gopls/doc/features/diagnostics.md b/gopls/doc/features/diagnostics.md new file mode 100644 index 00000000000..374c5f97962 --- /dev/null +++ b/gopls/doc/features/diagnostics.md @@ -0,0 +1,171 @@ +# Gopls: Diagnostics + +Gopls continuously annotates all your open files of source code with a +variety of diagnostics. Every time you edit a file or make a +configuration change, gopls asynchronously recomputes these +diagnostics and sends them to the client using the LSP +[`publishDiagnostics`](https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification#textDocument_publishDiagnostics) +notification, giving you real-time feedback that reduces the cost of +common mistakes. + +Diagnostics come from two main sources: compilation errors and analysis findings. + +- **Compilation errors** are those that you would obtain from running `go + build`. Gopls doesn't actually run the compiler; that would be too + slow. Instead it runs `go list` (when needed) to compute the + metadata of the compilation, then processes those packages in a similar + manner to the compiler front-end: reading, scanning, and parsing the + source files, then type-checking them. Each of these steps can + produce errors that gopls will surface as a diagnostic. + + The `source` field of the LSP `Diagnostic` record indicates where + the diagnostic came from: those with source `"go list"` come from + the `go list` command, and those with source `"compiler"` come from + gopls' parsing or type checking phases, which are similar to those + used in the Go compiler. + + ![A diagnostic due to a type error](../assets/diagnostic-typeerror.png) + + The example above shows a `string + int` addition, causes the type + checker to report a `MismatchedTypes` error. The diagnostic contains + a link to the documentation about this class of type error. + +- **Analysis findings** come from the [**Go analysis + framework**](https://golang.org/x/tools/go/analysis), the system + used by `go vet` to apply a variety of additional static checks to + your Go code. The best-known example is the [`printf` + analyzer](https://pkg.go.dev/golang.org/x/tools/go/analysis/passes/printf), + which reports calls to [`fmt.Printf`](https://pkg.go.dev/fmt#Printf) + where the format "verb" doesn't match the argument, such as + `fmt.Printf("%d", "three")`. + + Gopls provides dozens of analyzers aggregated from a variety of + suites. See [Analyzers](../analyzers.md) for the complete list. The + `source` field of each diagnostic produced by an analyzer records + the name of the analyzer that produced it. + + ![A diagnostic due to an analysis finding](../assets/diagnostic-analysis.png) + + The example above shows a `printf` formatting mistake. The diagnostic contains + a link to the documentation for the `printf` analyzer. + +## Recomputation of diagnostics + +Diagnostics are automatically recomputed each time the source files +are edited. + +Compilation errors in open files are updated after a very short delay +(tens of milliseconds) after each file change, potentially after every keystroke. +This ensures rapid feedback of syntax and type errors while editing. + +Compilation and analysis diagnostics for the whole workspace are much +more expensive to compute, so they are usually recomputed after a +short idle period (around 1s) following an edit. + +The [`diagnosticsDelay`](../settings.md#diagnosticsDelay) setting determines +this period. +Alternatively, diagnostics may be triggered only after an edited file +is saved, using the +[`diagnosticsTrigger`](../settings.md#diagnosticsTrigger) setting. + +Gopls does not currently support "pull-based" diagnostics, which are +computed synchronously when requested by the client; see golang/go#53275. + + +## Quick fixes + +Each analyzer diagnostic may suggest one or more alternative +ways to fix the problem by editing the code. +For example, when a `return` statement has too few operands, +the [`fillreturns`](../analyzers.md#fillreturns) analyzer +suggests a fix that heuristically fills in the missing ones +with suitable values. Applying the fix eliminates the compilation error. + +![An analyzer diagnostic with two alternative fixes](../assets/remove-unusedparam-before.png) + +The screenshot above shows VS Code's Quick Fix menu for an "unused +parameter" analysis diagnostic with two alternative fixes. +(See [Remove unused parameter](transformation.md#remove-unused-parameter) for more detail.) + +Suggested fixes that are indisputably safe are [code +actions](transformation.md#code-actions) whose kind is +`"source.fixAll"`. Many client editors have a shortcut to apply all +such fixes. + +TODO(adonovan): audit all the analyzers to ensure that their +documentation is up-to-date w.r.t. any fixes they suggest. + +Settings: + +- The [`diagnosticsDelay`](../settings.md#diagnosticsDelay) setting determines + the idle period after an edit before diagnostics are recomputed. +- The [`diagnosticsTriggerr`](../settings.md#diagnosticsTrigger) setting determines + what events cause recomputation of diagnostics. +- The [`linkTarget`](../settings.md#linkTarget) setting specifies + the base URI for Go package links in the Diagnostic.CodeDescription field. + +Client support: +- **VS Code**: Diagnostics appear as a squiggly underline. + Hovering reveals the details, along with any suggested fixes. +- **Emacs + eglot**: Diagnostics appear as a squiggly underline. + Hovering reveals the details. Use `M-x eglot-code-action-quickfix` + to apply available fixes; it will prompt if there are more than one. +- **Vim + coc.nvim**: ?? +- **CLI**: `gopls check file.go` + + diff --git a/gopls/doc/features/modfiles.md b/gopls/doc/features/modfiles.md new file mode 100644 index 00000000000..775be987ade --- /dev/null +++ b/gopls/doc/features/modfiles.md @@ -0,0 +1,9 @@ +# Gopls: Support for go.mod and go.work files + +TODO: document these features for go.{mod,work} files: +- hover +- vulncheck +- add dependency +- update dependency +- diagnostics + diff --git a/gopls/doc/features/navigation.md b/gopls/doc/features/navigation.md new file mode 100644 index 00000000000..973507dfc51 --- /dev/null +++ b/gopls/doc/features/navigation.md @@ -0,0 +1,251 @@ +# Gopls: Navigation features + +This page documents gopls features for navigating your source code. + +## Definition + +The LSP [`textDocument/definition`](https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#textDocument_definition) +request returns the location of the declaration of the symbol under the cursor. +Most editors provide a command to navigate directly to that location. + +A definition query also works in these unexpected places: + +- On an **import path**, it returns the list of locations, of + each package declaration in the files of the imported package. +- On a **package declaration**, it returns the location of + the package declaration that provides the documentation of that package. +- On a symbol in a **[`go:linkname` directive](https://pkg.go.dev/cmd/compile)**, + it returns the location of that symbol's declaration. +- On a **[doc link](https://tip.golang.org/doc/comment#doclinks)**, it returns + (like [`hover`](passive.md#hover)) the location of the linked symbol. +- On a file name in a **[`go:embed` directive](https://pkg.go.dev/embed)**, + it returns the location of the embedded file. + + + +Client support: +- **VS Code**: Use [Go to Definition](https://code.visualstudio.com/docs/editor/editingevolved#_go-to-definition) (`F12` or `⌘`-click). + If the cursor is already at the declaration, the request is instead interpreted as "Go to References". +- **Emacs + eglot**: use [`M-x xref-find-definitions`](https://www.gnu.org/software/emacs/manual/html_node/emacs/Xref.html). +- **Vim + coc.nvim**: ?? +- **CLI**: `gopls definition file.go:#offset` + +## References + +The LSP [`textDocument/references`](https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#textDocument_references) +request returns the locations of all identifiers that refers to the symbol under the cursor. + +The references algorithm handles various parts of syntax as follows: + +- The references to a **symbol** report all uses of that symbol. + In the case of exported symbols this may include locations in other packages. +- The references to a **package declaration** are all the + direct imports of the package, along with all the other package + declarations in the same package. +- It is an error to request the references to a **built-in symbol** + such as `int` or `append`, + as they are presumed too numerous to be of interest. +- The references to an **interface method** include references to + concrete types that implement the interface. Similarly, the + references to a **method of a concrete type** include references to + corresponding interface methods. +- An **embedded field** `T` in a struct type such as `struct{T}` is + unique in Go in that it is both a reference (to a type) and a + definition (of a field). + The `references` operation reports only the references to it [as a field](golang/go#63521). + To find references to the type, jump to the type declararation first. + +Be aware that a references query returns information only about the +build configuration used to analyze the selected file, so if you ask +for the references to a symbol defined in `foo_windows.go`, the result +will never include the file `bar_linux.go`, even if that file refers +to a symbol of the same name; see golang/go#65755. + +Clients can request that the the declaration be included among the +references; most do. + +Client support: +- **VS Code**: Use [`Go to References`](https://code.visualstudio.com/docs/editor/editingevolved#_peek) to quickly "peek" at the references, + or `Find all References` to open the references panel. +- **Emacs + eglot**: Via [`xref` package](https://www.gnu.org/software/emacs/manual/html_node/emacs/Xref.html): use `M-x xref-find-references`. +- **Vim + coc.nvim**: ?? +- **CLI**: `gopls references file.go:#offset` + +## Implementation + +The LSP +[`textDocument/implementation`](https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#textDocument_implementation) +request queries the "implements" relation between interfaces and concrete types: + +- When invoked on a reference to an **interface type**, it returns the + location of the declaration of each type that implements + the interface. +- When invoked on a **concrete type**, + it returns the locations of the matching interface types. +- When invoked on an **interface method**, it returns the corresponding + methods of the types that satisfy the interface. +- When invoked on a **concrete method**, + it returns the locations of the matching interface methods. + +Only non-trivial interfaces are considered; no implementations are +reported for type `any`. + +Within the same package, all matching types/methods are reported. +However, across packages, only exported package-level types and their +methods are reported, so local types (whether interfaces, or struct +types with methods due to embedding) may be missing from the results. + + +Generic types are currently not fully supported; see golang/go#59224. + +Client support: +- **VS Code**: Use [Go to Implementations](https://code.visualstudio.com/docs/editor/editingevolved#_go-to-implementation) (`⌘F12`). +- **Emacs + eglot**: Use `M-x eglot-find-implementation`. +- **Vim + coc.nvim**: ?? +- **CLI**: `gopls implementation file.go:#offset` + + +## Type Definition + +The LSP +[`textDocument/typeDefinition`](https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#textDocument_typeDefinition) +request returns the location of the type of the selected symbol. + +For example, if the selection is the name `buf` of a local variable of +type `*bytes.Buffer`, a `typeDefinition` query will return the +location of the type `bytes.Buffer`. +Clients typically navigate to that location. + +Type constructors such as pointer, array, slice, channel, and map are +stripped off the selected type in the search for a named type. For +example, if x is of type `chan []*T`, the reported type definition +will be that of `T`. +Similarly, if the symbol's type is a function with one "interesting" +(named, non-`error`) result type, the function's result type is used. + +Gopls currently requires that a `typeDefinition` query be applied to a +symbol, not to an arbitrary expression; see golang/go#67890 for +potential extensions of this functionality. + + +Client support: +- **VS Code**: Use [Go to Type Definition](https://code.visualstudio.com/docs/editor/editingevolved#_go-to-implementation). +- **Emacs + eglot**: Use `M-x eglot-find-typeDefinition`. +- **Vim + coc.nvim**: ?? +- **CLI**: not supported + +## Document Symbol + +The `textDocument/documentSymbol` LSP query reports the list of +top-level declarations in this file. Clients may use this information +to present an overview of the file, and an index for faster navigation. + +Gopls responds with the +[`DocumentSymbol`](https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification#documentSymbol) +type if the client indicates +[`hierarchicalDocumentSymbolSupport`](https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification#documentSymbolClientCapabilities); +otherwise it returns a +[`SymbolInformation`](https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification#symbolInformation). + +Client support: +- **VS Code**: Use the [Outline view](https://code.visualstudio.com/docs/getstarted/userinterface#_outline-view) for navigation. +- **Emacs + eglot**: Use [`M-x imenu`](https://www.gnu.org/software/emacs/manual/html_node/emacs/Imenu.html#Imenu) to jump to a symbol. +- **Vim + coc.nvim**: ?? +- **CLI**: `gopls links file.go` + + +## Symbol + +The +[`workspace/symbol`](https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification#workspace_symbol) +LSP query searches an index of all the symbols in the workspace. + +The default symbol matching algorithm (`fastFuzzy`), inspired by the +popular fuzzy matcher [FZF](https://github.com/junegunn/fzf), attempts +a variety of inexact matches to correct for misspellings in your +query. For example, it considers `DocSym` a match for `DocumentSymbol`. + + + +TODO: screenshot + +Settings: +- The [`symbolMatcher`](../settings.md#symbolMatcher) setting controls the algorithm used for symbol matching. +- The [`symbolStyle`](../settings.md#symbolStyle) setting controls how symbols are qualified in symbol responses. +- The [`symbolScope`](../settings.md#symbolScope) setting determines the scope of the query. +- The [`directoryFilters`](../settings.md#directoryFilters) setting specifies directories to be excluded from the search. + +Client support: +- **VS Code**: Use ⌘T to open [Go to Symbol](https://code.visualstudio.com/docs/editor/editingevolved#_go-to-symbol) with workspace scope. (Alternatively, use Ctrl-Shift-O, and add a `@` prefix to search within the file or a `#` prefix to search throughout the workspace.) +- **Emacs + eglot**: Use [`M-x xref-find-apropos`](https://www.gnu.org/software/emacs/manual/html_node/emacs/Looking-Up-Identifiers.html) to show symbols that match a search term. +- **Vim + coc.nvim**: ?? +- **CLI**: `gopls links file.go` + + +## Selection Range + +The +[`textDocument/selectionRange`](https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification#textDocument_selectionRange) +LSP query returns information about the lexical extent of each piece +of syntax enclosing the current selection. +Clients may use it to provide an operation to expand the selection +to successively larger expressions. + +Client support: +- **VSCode**: Use `⌘⇧^→` to expand the selection or `⌘⇧^←` to contract it again; watch this [video](https://www.youtube.com/watch?v=dO4SGAMl7uQ). +- **Emacs + eglot**: Not standard. Use `M-x eglot-expand-selection` defined in [this configuration snippet](https://github.com/joaotavora/eglot/discussions/1220#discussioncomment-9321061). +- **Vim + coc.nvim**: ?? +- **CLI**: not supported + +## Call Hierarchy + +The LSP CallHierarchy mechanism consists of three queries that +together enable clients to present a hierarchical view of a portion of +the static call graph: + +- [`textDocument/prepareCallHierarchy`](https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification#textDocument_prepareCallHierarchy) returns a list of [items](https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification#callHierarchyItem) for a given position, each representing a named function or method enclosing the position; +- [`callHierarchyItem/incomingCalls`](https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification#callHierarchy_incomingCalls) returns the set of call sites that call the selected item; and +- [`callHierarchy/outgoingCalls`](https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification#callHierarchy_incomingCalls) returns the set of functions called by the selected item. + +Invoke the command while selecting the name in a function declaration. + +Dynamic calls are not included, because it is not analytically +practical to detect them. So, beware that the results may not be +exhaustive, and perform a [References](#references) query if necessary. + +The hierarchy does not consider a nested function distinct from its +enclosing named function. (Without the ability to detect dynamic +calls, it would make little sense do so.) + +The screenshot below shows the outgoing call tree rooted at `f`. The +tree has been expanded to show a path from `f` to the `String` method +of `fmt.Stringer` through the guts of `fmt.Sprint:` + + + +Caveats: +- In some cases dynamic function calls are (erroneously) included in + the output; see golang/go#68153. + +Client support: +- **VS Code**: `Show Call Hierarchy` menu item (`⌥⇧H`) opens [Call hierarchy view](https://code.visualstudio.com/docs/cpp/cpp-ide#_call-hierarchy) (note: docs refer to C++ but the idea is the same for Go). +- **Emacs + eglot**: Not standard; install with `(package-vc-install "https://github.com/dolmens/eglot-hierarchy")`. Use `M-x eglot-hierarchy-call-hierarchy` to show the direct incoming calls to the selected function; use a prefix argument (`C-u`) to show the direct outgoing calls. There is no way to expand the tree. +- **CLI**: `gopls call_hierarchy file.go:#offset` shows outgoing and incoming calls. diff --git a/gopls/doc/features/passive.md b/gopls/doc/features/passive.md new file mode 100644 index 00000000000..ec10f509742 --- /dev/null +++ b/gopls/doc/features/passive.md @@ -0,0 +1,271 @@ +# Gopls: Passive features + +This page documents the fundamental LSP features of gopls that may be +described as "passive", since many editors use them to continuously +provide information about your source files without requiring any +special action. + +See also [Code Lenses](../codelenses.md), some of which annotate your +source code with additional information and may thus also be +considered passive features. + + +## Hover + +The LSP [`textDocument/hover`](https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#textDocument_hover) +query returns description of the code currently under the cursor, such +as its name, kind, type, value (for a constant), abbreviated +declaration (for a type), its doc comment, if any, and a link to the +symbol's documentation on `pkg.go.dev`. The client may request either +plain text or Markdown. + + + +Depending on the selection, it may include additional information. +For example, hovering over a type shows its declared methods, +plus any methods promoted from embedded fields. + +**Doc links**: A doc comment may refer to another symbol using square +brackets, for example `[fmt.Printf]`. Hovering over one of these +[doc links](https://go.dev/doc/comment#doclinks) reveals +information about the referenced symbol. + + + +**Struct size/offset info**: for declarations of struct types, +hovering over the name reveals the struct's size in bytes: + + + +And hovering over each field name shows the size and offset of that field: + + + +This information may be useful when optimizing the layout of your data +structures, or when reading assembly files or stack traces that refer +to each field by its cryptic byte offset. + +In addition, Hover reports the percentage of wasted space due to +suboptimal ordering of struct fields, if this figure is 20% or higher: + + + +In the struct above, alignment rules require each of the two boolean +fields (1 byte) to occupy a complete word (8 bytes), leading to (7 + +7) / (3 * 8) = 58% waste. +Placing the two booleans together would save a word. +(In most structures clarity is more important than compactness, so you +should reorder fields to save space only in data structures that have +been shown by profiling to be very frequently allocated.) + +**Embed directives**: hovering over the file name pattern in +[`//go:embed` directive](https://pkg.go.dev/embed), for example +`*.html`, reveals the list of file names to which the wildcard +expands. + + + + +**Linkname directives**: a [`//go:linkname` directive](https://pkg.go.dev/cmd/compile#hdr-Compiler_Directives) creates a linker-level alias for another symbol. +Hovering over the directive shows information about the other symbol. + + + +The hover information for symbols from the standard library added +after Go 1.0 states the Go release that added the symbol. + +Settings: +- The [`hoverKind`](../settings.md#hoverKind) setting controls the verbosity of documentation. +- The [`linkTarget`](../settings.md#linkTarget) setting specifies + the base URI for Go package links + +Caveats: +- It is an unfortunate limitation of the LSP that a `Hover` request + currently includes only a position but not a selection, as this + means it is impossible to request information about the type and + methods of, say, the `f(x)` portion of the larger expression + `f(x).y`. Please upvote microsoft/language-server-protocol#1466 if + you would like to see this addressed. + +Client support: +- **VS Code**: enabled by default. Displays rendered Markdown in a panel near the cursor. +- **Emacs + eglot**: enabled by default. Displays a one-line summary in the echo area. +- **Vim + coc.nvim**: ?? +- **CLI**: `gopls definition file.go:#start-#end` includes information from a Hover query. + + +## Signature Help + +The LSP [`textDocument/signatureHelp`](https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#textDocument_signatureHelp) +query returns information about the innermost function call enclosing +the cursor or selection, including the signature of the function and +the names, types, and documentation of each parameter. + +Clients may provide this information to help remind the user of the +purpose of each parameter and their order, while reading or editing a +function call. + + + +Client support: +- **VS Code**: enabled by default. + Also known as "[parameter hints](https://code.visualstudio.com/api/references/vscode-api#SignatureHelpProvider)" in the [IntelliSense settings](https://code.visualstudio.com/docs/editor/intellisense#_settings). + Displays signature and doc comment alongside Hover information. +- **Emacs + eglot**: enabled by default. Displays signature in the echo area. +- **Vim + coc.nvim**: ?? +- **CLI**: `gopls signature file.go:#start-#end` + + +## Document Highlight + +The LSP [`textDocument/documentHighlight`](https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#textDocument_documentHighlight) +query reports a set of source ranges that should be highlighted based +on the current cursor position or selection, to emphasize the +relationship between them. + +Each of the following parts of syntax forms a set so that if you +select any one member, gopls will highlight the complete set: + +- each identifier that refers to the same symbol (as in the screenshot below); +- a named result variable and all its corresponding operands of `return` statements; +- the `for`, `break`, and `continue` tokens of the same loop; +- the `switch` and `break` tokens of the same switch statement; +- the `func` keyword of a function and all of its `return` statements. + +More than one of these rules may be activated by the same selection, +for example, an identifier that is also a return operand. + + + +Client support: +- **VS Code**: enabled by default. Triggered by cursor motion, or single click. + (Note: double clicking activates a simple syntax-oblivious textual match.) +- **Emacs + eglot**: enabled by default. Triggered by cursor motion or selection. +- **Vim + coc.nvim**: ?? +- **CLI**: `gopls signature file.go:#start-#end` + + +## Inlay Hint + +The LSP [`textDocument/inlayHint`](https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#textDocument_inlayHint) +query returns a set of annotations to be spliced into the current file +that reveal implicit information. + + + +Examples: + +- In a function call `f(1, 2)`, gopls will provide hints for the + names of the parameters (`parameterNames`), as in the screenshot above. +- In a call to a generic function, hints provide the type arguments + (`functionTypeParameters`). +- In an assignment `x, y = 1, 2`, hints provide the types of the + variables (`assignVariableTypes`). +- In a struct literal such as `Point2D{1, 2}`, hints provide the field + names (`compositeLiteralFields`). +- In a nested composite literal `T{{...}}`, a hint provides the type of + the inner literal, `{...}` (`compositeLiteralTypes`). +- In a `for k, v := range x {}` loop, hints provide the types of the + variables k and v (`rangeVariableTypes`). +- For a constant expression (perhaps using `iota`), a hint provides + its computed value (`constantValues`). + +See [Inlay hints](../inlayHints.md) for a complete list with examples. + +TODO: Do we really need that separate doc? We could put all the + information here with much less fuss. It changes so rarely that a + culture of "update the tests and docs in every CL" should be sufficient. + IIUC, VS Code needs only the api-json representation. + +Settings: +- The [`hints`](../settings.md#hints) setting indicates the desired set of hints. + To reduce distractions, its default value is empty. + To enable hints, add one or more of the identifiers above to the hints + map. For example: + ```json5 + "hints": {"parameterNames": true} + ``` + +Client support: +- **VS Code**: in addition to the `hints` configuration value, VS Code provides a graphical + configuration menu ("Preferences: Open Settings (UI)" the search for "Go Inlay Hints") + for each supported kind of inlay hint. +- **Emacs + eglot**: disabled by default. Needs `M-x eglot-inlay-hints-mode` plus the configuration [described here](https://www.reddit.com/r/emacs/comments/11bqzvk/emacs29_and_eglot_inlay_hints/) +- **Vim + coc.nvim**: ?? +- **CLI**: not supported + +## Semantic Tokens + +The LSP [`textDocument/semanticTokens`](https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#textDocument_semanticTokens) +query reports information about all the tokens in the current file, or +a portion of it. +The client may use this information to provide syntax highlighting +that conveys semantic distinctions between, for example, functions and +types, constants and variables, or library functions and built-ins. +The client specifies the sets of types and modifiers they are interested in. + +Settings: +- The [`semanticTokens`](../settings.md#semanticTokens) setting determines whether + gopls responds to semantic token requests. This option allows users to disable + semantic tokens even when their client provides no client-side control over the + feature. Because gopls' semantic-tokens algorithm depends on type checking, + which adds a tangible latency, this feature is currently disabled by default + to avoid any delay in syntax highlighting; see golang/go#45313, golang/go#47465. +- The experimental [`noSemanticString`](../settings.md#noSemanticString) and + [`noSemanticNumber`](../settings.md#noSemanticNumber) settings cause the server + to exclude the `string` and `number` kinds from the response, as some clients + may do a more colorful job highlighting these tokens; see golang/go#45753. + +Client Support: +- **VS Code**: See [Semantic Highlighting Guide](https://code.visualstudio.com/api/language-extensions/semantic-highlight-guide). +- **Emacs + eglot**: Not supported; see joaotavora/eglot#615. +- **Vim + coc.nvim**: ?? +- **CLI**: `gopls semtok file.go` + +For internal details of gopls' implementation of semantic tokens, +see [semantic tokens](../semantictokens.md). + +## Folding Range + +The LSP [`textDocument/foldingRange`](https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#textDocument_foldingRange) +query reports the list of regions in the current file that may be +independently collapsed or expanded. For example, it may be convenient +to collapse large comments or functions when studying some code so +that more of it fits in a single screen. + + + +The protocol [allows](https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#foldingRangeClientCapabilities) clients to indicate whether they prefer +fine-grained ranges such as matched pairs of brackets, or only ranges +consisting of complete lines. + +Client support: +- **VS Code**: displayed in left margin. Toggle the chevrons (`∨` and `>`) to collapse or expand. +- **Emacs + eglot**: not supported. +- **Vim + coc.nvim**: ?? +- **CLI**: `gopls folding_ranges file.go` + +## Document Link + +The LSP [`textDocument/documentLink`](https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#textDocument_documentLink) +query uses heuristics to extracts URLs from doc comments and string +literals in the current file so that client can present them as +clickable links. + + + +In addition to explicit URLs, gopls also turns string literals in +import declarations into links to the pkg.go.dev documentation for the +imported package. + +Settings: +- The [`importShortcut`](../settings.md#importShortcut) setting determines + what kind of link is returned for an `import` declaration. +- The [`linkTarget`](../settings.md#linkTarget) setting specifies + the base URI for Go package links. + +Client support: +- **VS Code**: Hovering over a link displays a "Follow link (cmd+click)" popup. +- **Emacs + eglot**: not currently used. +- **Vim + coc.nvim**: ?? +- **CLI**: `gopls links file.go` diff --git a/gopls/doc/features/templates.md b/gopls/doc/features/templates.md new file mode 100644 index 00000000000..a71a2ea181c --- /dev/null +++ b/gopls/doc/features/templates.md @@ -0,0 +1,49 @@ +# Gopls: Support for template files + +Gopls provides some support for Go template files, that is, files that +are parsed by [`text/template`](https://pkg.go.dev/text/template) or +[`html/template`](https://pkg.go.dev/html/template). + +## Enabling template support + +Gopls recognizes template files based on their file extension, which +may be configured by the +[`templateExtensions`](../settings.md#templateExtensions) setting. If +this list is empty, template support is disabled. (This is the default +value, since Go templates don't have a canonical file extension.) + +Additional configuration may be necessary to ensure that your client +chooses the correct language kind when opening template files. +Gopls recogizes both `"tmpl"` and `"gotmpl"` for template files. +For example, in `VS Code` you will also need to add an +entry to the +[`files.associations`](https://code.visualstudio.com/docs/languages/identifiers) +mapping: +```json +"files.associations": { + ".mytemplate": "gotmpl" +}, +``` + + +## Features +In template files, template support works inside +the default `{{` delimiters. (Go template parsing +allows the user to specify other delimiters, but +gopls does not know how to do that.) + +Gopls template support includes the following features: ++ **Diagnostics**: if template parsing returns an error, +it is presented as a diagnostic. (Missing functions do not produce errors.) ++ **Syntax Highlighting**: syntax highlighting is provided for template files. ++ **Definitions**: gopls provides jump-to-definition inside templates, though it does not understand scoping (all templates are considered to be in one global scope). ++ **References**: gopls provides find-references, with the same scoping limitation as definitions. ++ **Completions**: gopls will attempt to suggest completions inside templates. + +TODO: also ++ Hover ++ SemanticTokens ++ Symbol search ++ DocumentHighlight + + diff --git a/gopls/doc/features/transformation.md b/gopls/doc/features/transformation.md new file mode 100644 index 00000000000..14de653f346 --- /dev/null +++ b/gopls/doc/features/transformation.md @@ -0,0 +1,656 @@ +# Gopls: Code transformation features + +This document describes gopls' features for code transformation, which +include a range of behavior-preserving changes (refactorings, +formatting, simplifications), code repair (fixes), and editing support +(filling in struct literals, switch statements). + +Code transformations are not a single category in the LSP: +- a few, such as Formatting and Rename, are primary operations in the + protocol; +- some are [commands](../commands.md) exposed through [Code + Lenses](../codelenses.md) (though none of these are transformations of Go syntax); + +- most are defined as *code actions*. + +## Code Actions + +A **code action** is an action associated with a portion of the file. +Each time the selection changes, a typical client makes a +[`textDocument/codeAction`](https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#textDocument_codeAction) +request for the set of available actions, then updates its UI +elements (menus, icons, tooltips) to reflect them. +The VS Code manual describes code actions as +"[Quick fixes + Refactorings](https://code.visualstudio.com/docs/editor/refactoring#_code-actions-quick-fixes-and-refactorings)". + +A `codeAction` request delivers the menu, so to speak, but it does +not order the meal. When an action is chosen, one of two things happens. +In trivial cases, the action itself contains an edit that the +client can directly apply to the file. +But in most cases, the action contains a [command](../commands.md) +to be sent back to the server for its side effects, +just as with the command associated with a Code Lens. +This allows the work of computing the patch to be done lazily, only +when actually needed. (Most aren't.) +The server may then compute the edit and send the client a +[`workspace/applyEdit`](https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#workspace_applyEdit) +request to patch the files. +Not all code actions' commands have an `applyEdit` side effect: some +may change the state of the server, for example, to toggle a +variable, or cause the server to send other requests to the client, +such as as a `showDocument` request to open a report in a web browser. + +The main difference between code lenses and code actions is this: +- `codeLens` requests commands for the entire file. + Each command specifies its applicable source range, + and typically appears as an annotation on that source range. +- `codeAction` requests commands only for a particular range: the current selection. + All the commands are presented together in a menu at that location. + +Each action has a _kind_, +which is a hierarchical identifier such as `refactor.inline`. +Clients may filter actions based on their kind. +For example, VS Code has two menus, "Refactor..." and "Source action...", +each populated by different kinds of code actions (`refactor.*` and `source.*`), +plus a lightbulb icon that triggers a menu of "quick fixes" (of kind `quickfix.*`), +which are commands deemed unambiguously safe to apply. + + + +Many transformations are computed by [analyzers](../analyzers.md) +that, in the course of reporting a diagnostic about a problem, +also suggest a fix. +A `codeActions` request will return any fixes accompanying diagnostics +for the current selection. + + +Caveats: +- Many of gopls code transformations are limited by Go's syntax tree + representation, which currently records comments not in the tree, + but in a side table; consequently, transformations such as Extract + and Inline are prone to losing comments. This is issue + golang/go#20744, and it is a priority for us to fix in 2024. + +- Generated files, as identified by the conventional + [DO NOT EDIT](https://go.dev/s/generatedcode) comment, + are not offered code actions for transformations. + + +Client support for code actions: + +- **VS Code**: Depending on their kind, code actions are found in + the "Refactor..." menu (`^⇧R`), + the "Source action..." menu, + the 💡 (light bulb) icon's menu, or + the "Quick fix" (`⌘.`) menu. +- **Emacs + eglot**: Code actions are invisible. + Use `M-x eglot-code-actions` to select one from those that are + available (if there are multiple) and execute it. + Some action kinds have filtering shortcuts, + e.g. [`M-x eglot-code-action-{inline,extract,rewrite}`](https://joaotavora.github.io/eglot/#index-M_002dx-eglot_002dcode_002daction_002dinline). +- **CLI**: `gopls fix -a file.go:#123-#456 kinds...` executes code actions of the specified + kinds (e.g. `refactor.inline`) on the selected range, specified using zero-based byte offsets. + +## Formatting + +The LSP +[`textDocument/formatting`](https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#textDocument_formatting) +request returns edits that format a file. +Gopls applies Go's canonical formatting algorithm, +[`go fmt`](https://pkg.go.dev/cmd/gofmt). +LSP formatting options are ignored. + +Most clients are configured to format files and organize imports +whenever a file is saved. + +Settings: +- The [`gofumpt`](../settings.md#gofumpt) setting causes gopls to use an + alternative formatter, [`github.com/mvdan/gofumpt`](https://pkg.go.dev/mvdan.cc/gofumpt). + +Client support: +- **VS Code**: Formats on save by default. Use `Format document` menu item (`⌥⇧F`) to invoke manually. +- **Emacs + eglot**: Use `M-x eglot-format-buffer` to format. Attach it to `before-save-hook` to format on save. For formatting combined with organize-imports, many users take the legacy approach of setting `"goimports"` as their `gofmt-command` using [go-mode](https://github.com/dominikh/go-mode.el), and adding `gofmt-before-save` to `before-save-hook`. An LSP-based solution requires code such as https://github.com/joaotavora/eglot/discussions/1409. +- **CLI**: `gopls format file.go` + +## Organize imports + +A `codeActions` request in a file whose imports are not organized will +return an action of the standard kind `source.organizeImports`. +Its command has the effect of organizing the imports: +deleting existing imports that are duplicate or unused, +adding new ones for undefined symbols, +and sorting them into the conventional order. + +The addition of new imports is based on heuristics that depends on +your workspace and the contents of your GOMODCACHE directory; it may +sometimes make surprising choices. + +Many editors automatically organize imports and format the code before +saving any edited file. + +Some users dislike the automatic removal of imports that are +unreferenced because, for example, the sole line that refers to the +import is temporarily commented out for debugging; see golang/go#54362. + +Settings: + +- The [`local`](../settings.md#local) setting is a comma-separated list of + prefixes of import paths that are "local" to the current file and + should appear after standard and third-party packages in the sort order. + +Client support: +- **VS Code**: automatically invokes `source.organizeImports` before save. + To disable it, use the snippet below, and invoke the "Organize Imports" command manually as needed. + ``` + "[go]": { + "editor.codeActionsOnSave": { "source.organizeImports": false } + } + ``` +- **Emacs + eglot**: Use `M-x eglot-code-action-organize-imports` to invoke manually. + Many users of [go-mode](https://github.com/dominikh/go-mode.el) use these lines to + organize imports and reformat each modified file before saving it, but this + approach is based on the legacy + [`goimports`](https://pkg.go.dev/golang.org/x/tools/cmd/goimports) tool, not gopls: + ```lisp + (setq gofmt-command "goimports") + (add-hook 'before-save-hook 'gofmt-before-save) + ``` +- **CLI**: `gopls fix -a file.go:#offset source.organizeImports` + + +## Rename + +The LSP +[`textDocument/rename`](https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#textDocument_rename) +request renames a symbol. + +Renaming is a two-stage process. The first step, a +[`prepareRename`](https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#textDocument_prepareRename) query, returns the current +name of the identifier under the cursor (if indeed there is one). +The client then displays a dialog prompting the user to choose a new +name by editing the old one. The second step, `rename` proper, applies +the changes. (This simple dialog support is unique among LSP +refactoring operations; see microsoft/language-server-protocol#1164.) + +Gopls' renaming algorithm takes great care to detect situations in +which renaming might introduce a compilation error. +For example, changing a name may cause a symbol to become "shadowed", +so that some existing references are no longer in scope. Gopls will +report an error, stating the pair of symbols and the shadowed reference: + + + +As another example, consider renaming a method of a concrete type. +Renaming may cause the type to no longer satisfy the same interfaces +as before, which could cause the program to fail to compile. +To avoid this, gopls inspects each conversion (explicit or implicit) +from the affected type to an interface type, and checks whether it +would remain valid after the renaming. If not, it aborts the renaming +with an error. + +If you intend to rename both the original method and the corresponding +methods of any matching interface types (as well as any methods of +types matching them in turn), you can indicate this by invoking the +rename operation on the interface method. + +Similarly, gopls will report an error if you rename a field of a +struct that happens to be an "anonymous" field that embeds a type, +since that would require a larger renaming involving the type as well. +If that is what you intend, you can again indicate this by +invoking the rename operation on the type. + +Renaming should never introduce a compilation error, but it may +introduce dynamic errors. For example, in a method renaming, if there +is no direct conversion of the affected type to the interface type, +but there is an intermediate conversion to `interface{}` followed by a +type assertion to the interface type, then gopls may proceed to rename +the method, causing the type assertion to fail at run time. +Similar problems may arise with packages that use reflection, such as +`encoding/json` or `text/template`. There is no substitute for good +judgment and testing. + +Some tips for best results: +- There is currently no special support for renaming all receivers of + a family of methods at once, so you will need to rename one receiver + one at a time (golang/go#41892). +- The safety checks performed by the Rename algorithm require type + information. If the program is grossly malformed, there may be + insufficient information for it to run (golang/go#41870), + and renaming cannot generally be used to fix a type error (golang/go#41851). + When refactoring, we recommend working in small steps, repairing any + problems as you go, so that as much as possible of the program + compiles at each step. +- Sometimes it may be desirable for a renaming operation to change the + reference structure of the program, for example to intentionally + combine two variables x and y by renaming y to x. + The renaming tool is too strict to help in this case (golang/go#41852). + + + +For the gory details of gopls' rename algorithm, you may be interested +in the latter half of this 2015 GothamGo talk: +[Using go/types for Code Comprehension and Refactoring Tools](https://www.youtube.com/watch?v=p_cz7AxVdfg). + +Client support: +- **VS Code**: Use "[Rename symbol](https://code.visualstudio.com/docs/editor/editingevolved#_rename-symbol)" menu item (`F2`). +- **Emacs + eglot**: Use `M-x eglot-rename`, or `M-x go-rename` from [go-mode](https://github.com/dominikh/go-mode.el). +- **Vim + coc.nvim**: Use the `coc-rename` command. +- **CLI**: `gopls rename file.go:#offset newname` + + + +## Extract function/method/variable + +The `refactor.extract` family of code actions all return commands that +replace the selected expression or statements with a reference to a +newly created declaration that contains the selected code: + + + +- **Extract function** replaces one or more complete statements by a + call to a new function named `newFunction` whose body contains the + statements. The selection must enclose fewer statements than the + entire body of the existing function. + + ![Before extracting a function](../assets/extract-function-before.png) + ![After extracting a function](../assets/extract-function-after.png) + +- **Extract method** is a variant of "Extract function" offered when + the selected statements belong to a method. The newly created function + will be a method of the same receiver type. + +- **Extract variable** replaces an expression by a reference to a new + local variable named `x` initialized by the expression: + + ![Before extracting a var](../assets/extract-var-before.png) + ![After extracting a var](../assets/extract-var-after.png) + +If the default name for the new declaration is already in use, gopls +generates a fresh name. + +Extraction is a challenging problem requiring consideration of a +variety of aspects such as identifier scope and shadowing, control +flow such as `break`/`continue` in a loop or `return` in a +function, cardinality of variables, and even subtle issues of style. +In each case, the tool will try to update the extracted statements +as needed to avoid build breakage or behavior changes. +Unfortunately, gopls' Extract algorithms are considerably less +rigorous than the Rename and Inline operations, and we are aware of a +number of cases where it falls short, including: + +- https://github.com/golang/go/issues/66289 +- https://github.com/golang/go/issues/65944 +- https://github.com/golang/go/issues/64821 +- https://github.com/golang/go/issues/63394 +- https://github.com/golang/go/issues/61496 +- https://github.com/golang/go/issues/50851 + +The following Extract features are planned for 2024 but not yet supported: + +- **Extract constant** is a variant of "Extract variable" to be + offered when the expression is constant; see golang/go#37170. +- **Extract parameter struct** will replace two or more parameters of a + function by a struct type with one field per parameter; see golang/go#65552. + + +- **Extract interface for type** will create a declaration of an + interface type with all the methods of the selected concrete type; + see golang/go#65721 and golang/go#46665. + + + +## Extract declarations to new file + +(Available from gopls/v0.17.0) + +If you select one or more top-level declarations, gopls will offer an +"Extract declarations to new file" code action that moves the selected +declarations into a new file whose name is based on the first declared +symbol. +Gopls also offers this code action when the selection is just the +first token of the declaration, such as `func` or `type`. +Import declarations are created as needed. + +![Before: select the declarations to move](../assets/extract-to-new-file-before.png) +![After: the new file is based on the first symbol name](../assets/extract-to-new-file-after.png) + + + +## Inline call to function + +For a `codeActions` request where the selection is (or is within) a +call of a function or method, gopls will return a command of kind +`refactor.inline`, whose effect is to inline the function call. + +The screenshots below show a call to `sum` before and after inlining: + +![Before: select Refactor... Inline call to sum](../inline-before.png) +![After: the call has been replaced by the sum logic](../inline-after.png) + +Inlining replaces the call expression by a copy of the function body, +with parameters replaced by arguments. +Inlining is useful for a number of reasons. +Perhaps you want to eliminate a call to a deprecated +function such as `ioutil.ReadFile` by replacing it with a call to the +newer `os.ReadFile`; inlining will do that for you. +Or perhaps you want to copy and modify an existing function in some +way; inlining can provide a starting point. +The inlining logic also provides a building block for +other refactorings, such as "change signature". + +Not every call can be inlined. +Of course, the tool needs to know which function is being called, so +you can't inline a dynamic call through a function value or interface +method; but static calls to methods are fine. +Nor can you inline a call if the callee is declared in another package +and refers to non-exported parts of that package, or to [internal +packages](https://go.dev/doc/go1.4#internalpackages) that are +inaccessible to the caller. +Calls to generic functions are not yet supported +(golang/go#63352), though we plan to fix that. + +When inlining is possible, it's critical that the tool preserve +the original behavior of the program. +We don't want refactoring to break the build, or, worse, to introduce +subtle latent bugs. +This is especially important when inlining tools are used to perform +automated clean-ups in large code bases; +we must be able to trust the tool. +Our inliner is very careful not to make guesses or unsound +assumptions about the behavior of the code. +However, that does mean it sometimes produces a change that differs +from what someone with expert knowledge of the same code might have +written by hand. + +In the most difficult cases, especially with complex control flow, it +may not be safe to eliminate the function call at all. +For example, the behavior of a `defer` statement is intimately tied to +its enclosing function call, and `defer` is the only control +construct that can be used to handle panics, so it cannot be reduced +into simpler constructs. +So, for example, given a function f defined as: + +```go +func f(s string) { + defer fmt.Println("goodbye") + fmt.Println(s) +} +``` +a call `f("hello")` will be inlined to: +```go + func() { + defer fmt.Println("goodbye") + fmt.Println("hello") + }() +``` +Although the parameter was eliminated, the function call remains. + +An inliner is a bit like an optimizing compiler. +A compiler is considered "correct" if it doesn't change the meaning of +the program in translation from source language to target language. +An _optimizing_ compiler exploits the particulars of the input to +generate better code, where "better" usually means more efficient. +As users report inputs that cause the compiler to emit suboptimal +code, the compiler is improved to recognize more cases, or more rules, +and more exceptions to rules---but this process has no end. +Inlining is similar, except that "better" code means tidier code. +The most conservative translation provides a simple but (hopefully!) +correct foundation, on top of which endless rules, and exceptions to +rules, can embellish and improve the quality of the output. + +Here are some of the technical challenges involved in sound inlining: + +- **Effects:** When replacing a parameter by its argument expression, + we must be careful not to change the effects of the call. For + example, if we call a function `func twice(x int) int { return x + x }` + with `twice(g())`, we do not want to see `g() + g()`, which would + cause g's effects to occur twice, and potentially each call might + return a different value. All effects must occur the same number of + times, and in the same order. This requires analyzing both the + arguments and the callee function to determine whether they are + "pure", whether they read variables, or whether (and when) they + update them too. The inliner will introduce a declaration such as + `var x int = g()` when it cannot prove that it is safe to substitute + the argument throughout. + +- **Constants:** If inlining always replaced a parameter by its argument + when the value is constant, some programs would no longer build + because checks previously done at run time would happen at compile time. + For example `func index(s string, i int) byte { return s[i] }` + is a valid function, but if inlining were to replace the call `index("abc", 3)` + by the expression `"abc"[3]`, the compiler will report that the + index `3` is out of bounds for the string `"abc"`. + The inliner will prevent substitution of parameters by problematic + constant arguments, again introducing a `var` declaration instead. + +- **Referential integrity:** When a parameter variable is replaced by + its argument expression, we must ensure that any names in the + argument expression continue to refer to the same thing---not to a + different declaration in the callee function body that happens to + use the same name! The inliner must replace local references such as + `Printf` by qualified references such as `fmt.Printf`, and add an + import of package `fmt` as needed. + +- **Implicit conversions:** When passing an argument to a function, it + is implicitly converted to the parameter type. + If we eliminate the parameter variable, we don't want to + lose the conversion as it may be important. + For example, in `func f(x any) { y := x; fmt.Printf("%T", &y) }` the + type of variable y is `any`, so the program prints `"*interface{}"`. + But if inlining the call `f(1)` were to produce the statement `y := + 1`, then the type of y would have changed to `int`, which could + cause a compile error or, as in this case, a bug, as the program + now prints `"*int"`. When the inliner substitutes a parameter variable + by its argument value, it may need to introduce explicit conversions + of each value to the original parameter type, such as `y := any(1)`. + +- **Last reference:** When an argument expression has no effects + and its corresponding parameter is never used, the expression + may be eliminated. However, if the expression contains the last + reference to a local variable at the caller, this may cause a compile + error because the variable is now unused! So the inliner must be + cautious about eliminating references to local variables. + +This is just a taste of the problem domain. If you're curious, the +documentation for [golang.org/x/tools/internal/refactor/inline](https://pkg.go.dev/golang.org/x/tools/internal/refactor/inline) has +more detail. All of this is to say, it's a complex problem, and we aim +for correctness first of all. We've already implemented a number of +important "tidiness optimizations" and we expect more to follow. + +## Miscellaneous rewrites + +This section covers a number of transformations that are accessible as +code actions of kind `refactor.rewrite`. + + + +### Remove unused parameter + +The [`unusedparams` analyzer](../analyzers.md#unusedparams) reports a +diagnostic for each parameter that is not used within the function body. +For example: +```go +func f(x, y int) { // "unused parameter: x" + fmt.Println(y) +} +``` + +It does _not_ report diagnostics for address-taken functions, which +may need all their parameters, even unused ones, in order to conform +to a particular function signature. +Nor does it report diagnostics for exported functions, +which may be address-taken by another package. +(A function is _address-taken_ if it is used other than in call position, `f(...)`.) + +In addition to the diagnostic, it suggests two possible fixes: + +1. rename the parameter to `_` to emphasize that it is unreferenced (an immediate edit); or +2. delete the parameter altogether, using a `ChangeSignature` command, updating all callers. + +Fix \#2 uses the same machinery as "Inline function call" (see above) +to ensure that the behavior of all existing calls is preserved, even +when the argument expression for the deleted parameter has side +effects, as in the example below. + +![The parameter x is unused](../assets/remove-unusedparam-before.png) +![The parameter x has been deleted](../assets/remove-unusedparam-after.png) + +Observe that in the first call, the argument `chargeCreditCard()` was +not deleted because of potential side effects, whereas in the second +call, the argument 2, a constant, was safely deleted. + +### Convert string literal between raw and interpreted + +When the selection is a string literal, gopls offers a code action +to convert the string between raw form (`` `abc` ``) and interpreted +form (`"abc"`): + +![Convert to interpreted](../assets/convert-string-interpreted.png) +![Convert to raw](../assets/convert-string-raw.png) + +Apply the code action a second time to revert back to the original +form. + +### Invert 'if' condition + +When the selection is within an `if`/`else` statement that is not +followed by `else if`, gopls offers a code action to invert the +statement, negating the condition and swapping the `if` and and `else` +blocks. + +![Before "Invert if condition"](../assets/invert-if-before.png) +![After "Invert if condition"](../assets/invert-if-after.png) + + + +### Split elements into separate lines + +When the selection is within a bracketed list such as: + +- the **elements** of a composite literal, `[]T{a, b, c}`, +- the **arguments** of a function call, `f(a, b, c)`, +- the **groups of parameters** of a function signature, `func(a, b, c int, d, e bool)`, or +- its **groups of results**, `func() (x, y string, z rune)`, + +gopls will offer the "Split [items] into separate lines" code +action, which would transform the forms above into these forms: + +```go +[]T{ + a, + b, + c, +} + +f( + a, + b, + c, +) + +func( + a, b, c int, + d, e bool, +) + +func() ( + x, y string, + z rune, +) +``` +Observe that in the last two cases, each +[group](https://pkg.go.dev/go/ast#Field) of parameters or results is +treated as a single item. + +The opposite code action, "Join [items] into one line", undoes the operation. +Neither action is offered if the list is already full split or joined, +respectively, or trivial (fewer than two items). + +These code actions are not offered for lists containing `//`-style +comments, which run to the end of the line. + + + +### Fill struct literal + +When the cursor is within a struct literal `S{}`, gopls offers the +"Fill S" code action, which populates each missing field of the +literal that is accessible. + +It uses the following heuristic to choose the value assigned to each +field: it finds candidate variables, constants, and functions that are +assignable to the field, and picks the one whose name is the closest +match to the field name. +If there are none, it uses the zero value (such as `0`, `""`, or +`nil`) of the field's type. + +In the example below, a +[`slog.HandlerOptions`](https://pkg.go.dev/golang.org/x/exp/slog#HandlerOptions) +struct literal is filled in using two local variables (`level` and +`add`) and a function (`replace`): + +![Before "Fill slog.HandlerOptions"](../assets/fill-struct-before.png) +![After "Fill slog.HandlerOptions"](../assets/fill-struct-after.png) + +Caveats: + +- This code action requires type information for the struct type, so + if it is defined in another package that is not yet imported, you + may need to "organize imports" first, for example by saving the + file. +- Candidate declarations are sought only in the current file, and only + above the current point. Symbols declared beneath the current point, + or in other files in the package, are not considered; see + golang/go#68224. + +### Fill switch + +When the cursor is within a switch statement whose operand type is an +_enum_ (a finite set of named constants), or within a type switch, +gopls offers the "Add cases for T" code action, which populates the +switch statement by adding a case for each accessible named constant +of the enum type, or, for a type switch, by adding a case for each +accessible named non-interface type that implements the interface. +Only missing cases are added. + +The screenshots below show a type switch whose operand has the +[`net.Addr`](https://pkg.go.dev/net#Addr) interface type. The code +action adds one case per concrete network address type, plus a default +case that panics with an informative message if an unexpected operand +is encountered. + +![Before "Add cases for Addr"](../assets/fill-switch-before.png) +![After "Add cases for Addr"](../assets/fill-switch-after.png) + +And these screenshots illustrate the code action adding cases for each +value of the +[`html.TokenType`](https://pkg.go.dev/golang.org/x/net/html#TokenType) +enum type, which represents the various types of token from +which HTML documents are composed: + +![Before "Add cases for Addr"](../assets/fill-switch-enum-before.png) +![After "Add cases for Addr"](../assets/fill-switch-enum-after.png) diff --git a/gopls/doc/features/web.md b/gopls/doc/features/web.md new file mode 100644 index 00000000000..4940ee199da --- /dev/null +++ b/gopls/doc/features/web.md @@ -0,0 +1,152 @@ +# Gopls: Web-based features + +The LSP +[`window.showDocument`](https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#window_showDocument) request +allows the server to instruct the client to open a file in the editor +or a web page in a browser. It is the basis for a number of gopls +features that report information about your program through a web +interface. + +We recognize that a web interface is not ideal for everyone: some +users prefer a full-screen editor layout and dislike switching +windows; others may work in a text-only terminal without a window +system, perhaps over remote ssh or on the Linux console. +Unfortunately, the LSP lacks several natural kinds of extensibility, +including the ability for servers to define: + +- queries that [generalize a References + query](https://github.com/microsoft/language-server-protocol/issues/1911), + displaying results using similar UI elements; +- commands that [produce a stream of + text](https://github.com/joaotavora/eglot/discussions/1402), like a + typical shell command or compiler, that the client can redirect to + the editor's usual terminal-like UI element; or +- refactoring operations that, like Rename, [prompt the + user](https://github.com/microsoft/language-server-protocol/issues/1164) + for additional information. + +The web-based UI can help fill these gaps until such time as the LSP +provides standard ways of implementing these features. + +Gopls' web server listens on a `localhost` port. For security, all its +endpoints include a random string that serves as an authentication +token. The client, provided authenticated URLs by the server, will be +able to access your source code, but arbitrary processes running on +your machine will not. +Restarting the gopls process causes this secret to change, rendering +all existing previous URLs invalid; existing pages will display a banner +indicating that they have become disconnected. + +TODO: combine the web server and the debug server; see golang/go#68229. + +Gopls supports two-way communication between the web browser and the +client editor. All of the web-based reports contain links to +declarations in your source code. Clicking on one of these links +causes gopls to send a `showDocument` request to your editor to open +the relevant source file at the appropriate line. This works even when +your source code has been modified but not saved. +(VS Code users: please upvote microsoft/vscode#208093 if you would +like your editor to raise its window when handling this event.) + + +## View package documentation + +In any Go source file, a code action request returns a command to +"View package documentation". This command opens a browser window +showing the documentation for the current Go package, presented using +a similar design to https://pkg.go.dev. + +This allows you to preview the documentation for your packages, even +internal ones that may be unpublished externally. Reloading the page +updates the documentation to reflect your changes. It is not necessary +to save modified Go source files. + + + +Clicking on the link for a package-level symbol or method, which in +`pkg.go.dev` would ordinarily take you to a source-code viewer such as +GitHub or Google Code Search, causes your editor to navigate to the +relevant source file and line. + +Client support: +- **VS Code**: Use the "Source Action... > View package documentation" menu. +- **Emacs + eglot**: Use `M-x go-browse-doc` in [go-mode](https://github.com/dominikh/go-mode.el). +- **Vim + coc.nvim**: ?? + + + +## View free symbols + +When studying code, either to understand it or to evaluate a different +organization or factoring, it is common to need to know what the +"inputs" are to a given chunk of code, either because you are +considering extracting it into its own function and want to know what +parameters it would take, or just to understand how one piece of a long +function relates to the preceding pieces. + +If you select a chunk of code, and apply the "[View free +symbols](../commands.md#gopls.free_symbols)" command, your editor will +open a browser displaying a report on the free symbols of the +selection. A symbol is "free" if it is referenced from within the +selection but defined outside of it. In essence, these are the inputs +to the selected chunk. + + + +The report classifies the symbols into imported, local, and +package-level symbols. The imported symbols are grouped by package, +and link to the documentation for the package, as described above. +Each of the remaining symbols is presented as a link that causes your +editor to navigate to its declaration. + +TODO: explain dotted paths. + +Client support: +- **VS Code**: Use the "Source Action... > View free symbols" menu. +- **Emacs + eglot**: Use `M-x go-browse-freesymbols` in [go-mode](https://github.com/dominikh/go-mode.el). +- **Vim + coc.nvim**: ?? + + + +## View assembly + +When you're optimizing the performance of your code or investigating +an unexpected crash, it may sometimes be helpful to inspect the +assembly code produced by the compiler for a given Go function. + +If you position the cursor or selection within a function f, a code +action will offer the "[View assembly for +f](../commands.md#gopls.assembly)" command. This opens a web-based +listing of the assembly for the function, plus any functions nested +within it. + +Each time you edit your source and reload the page, the current +package is recompiled and the listing is updated. It is not necessary +to save your modified files. + +The compiler's target architecture is the same as the one gopls uses +when analyzing the file: typically, this is your machine's GOARCH, but +when viewing a file with a build tag, such as one named `foo_amd64.go` +or containing the comment `//go:build amd64`, the tags determine the +architecture. + +Each instruction is displayed with a link that causes your editor to +navigate to the source line responsible for the instruction, according +to the debug information. + + + +The example above shows the arm64 assembly listing of +[`time.NewTimer`](https://pkg.go.dev/time#NewTimer). +Observe that the indicated instruction links to a source location +inside a different function, `syncTimer`, because the compiler +inlined the call from `NewTimer` to `syncTimer`. + +Viewing assembly is not yet supported for generic functions, package +initializers (`func init`), or functions in test packages. +(Contributions welcome!) + +Client support: +- **VS Code**: Use the "Source Action... > View GOARCH assembly for f" menu. +- **Emacs + eglot**: Use `M-x go-browse-assembly` in [go-mode](https://github.com/dominikh/go-mode.el). +- **Vim + coc.nvim**: ?? diff --git a/gopls/doc/generate/generate.go b/gopls/doc/generate/generate.go index f49a787888a..524dc178d55 100644 --- a/gopls/doc/generate/generate.go +++ b/gopls/doc/generate/generate.go @@ -7,12 +7,14 @@ // gopls/doc/settings.md -- from linking gopls/internal/settings.DefaultOptions // gopls/doc/commands.md -- from loading gopls/internal/protocol/command // gopls/doc/analyzers.md -- from linking gopls/internal/settings.DefaultAnalyzers -// gopls/doc/inlayHints.md -- from linking gopls/internal/golang.AllInlayHints +// gopls/doc/inlayHints.md -- from loading gopls/internal/settings.InlayHint // gopls/internal/doc/api.json -- all of the above in a single value, for 'gopls api-json' // // Run it with this command: // -// $ cd gopls/doc && go generate +// $ cd gopls/internal/doc && go generate +// +// TODO(adonovan): move this package to gopls/internal/doc/generate. package main import ( @@ -39,8 +41,6 @@ import ( "golang.org/x/tools/gopls/internal/doc" "golang.org/x/tools/gopls/internal/golang" "golang.org/x/tools/gopls/internal/mod" - "golang.org/x/tools/gopls/internal/protocol" - "golang.org/x/tools/gopls/internal/protocol/command" "golang.org/x/tools/gopls/internal/protocol/command/commandmeta" "golang.org/x/tools/gopls/internal/settings" "golang.org/x/tools/gopls/internal/util/maps" @@ -137,23 +137,12 @@ func loadAPI() (*doc.API, error) { &packages.Config{ Mode: packages.NeedTypes | packages.NeedTypesInfo | packages.NeedSyntax | packages.NeedDeps, }, - "golang.org/x/tools/gopls/internal/settings", // for settings - "golang.org/x/tools/gopls/internal/protocol", // for lenses + "golang.org/x/tools/gopls/internal/settings", ) if err != nil { return nil, err } - // TODO(adonovan): document at packages.Load that the result - // order does not match the pattern order. - var protocolPkg, settingsPkg *packages.Package - for _, pkg := range pkgs { - switch pkg.Types.Name() { - case "settings": - settingsPkg = pkg - case "protocol": - protocolPkg = pkg - } - } + settingsPkg := pkgs[0] defaults := settings.DefaultOptions() api := &doc.API{ @@ -165,16 +154,15 @@ func loadAPI() (*doc.API, error) { if err != nil { return nil, err } - api.Lenses, err = loadLenses(protocolPkg, defaults.Codelenses) + api.Lenses, err = loadLenses(settingsPkg, defaults.Codelenses) if err != nil { return nil, err } - - // Transform the internal command name to the external command name. - for _, c := range api.Commands { - c.Command = command.ID(c.Command) + api.Hints, err = loadHints(settingsPkg) + if err != nil { + return nil, err } - api.Hints = loadHints(golang.AllInlayHints) + for _, category := range []reflect.Value{ reflect.ValueOf(defaults.UserOptions), } { @@ -187,14 +175,13 @@ func loadAPI() (*doc.API, error) { if err != nil { return nil, err } - catName := strings.TrimSuffix(category.Type().Name(), "Options") - api.Options[catName] = opts - // Hardcode the expected values for the analyses and code lenses - // settings, since their keys are not enums. + // Edge case for "analyses": populate its enum keys from + // the analyzer list, since its map keys are strings, not enums. + // Also, set its EnumKeys.ValueType for historical reasons. for _, opt := range opts { - switch opt.Name { - case "analyses": + if opt.Name == "analyses" { + opt.EnumKeys.ValueType = "bool" for _, a := range api.Analyzers { opt.EnumKeys.Keys = append(opt.EnumKeys.Keys, doc.EnumKey{ Name: fmt.Sprintf("%q", a.Name), @@ -202,33 +189,11 @@ func loadAPI() (*doc.API, error) { Default: strconv.FormatBool(a.Default), }) } - case "codelenses": - // Hack: Lenses don't set default values, and we don't want to - // pass in the list of expected lenses to loadOptions. Instead, - // format the defaults using reflection here. The hackiest part - // is reversing lowercasing of the field name. - reflectField := category.FieldByName(upperFirst(opt.Name)) - for _, l := range api.Lenses { - def, err := formatDefaultFromEnumBoolMap(reflectField, l.Lens) - if err != nil { - return nil, err - } - opt.EnumKeys.Keys = append(opt.EnumKeys.Keys, doc.EnumKey{ - Name: fmt.Sprintf("%q", l.Lens), - Doc: l.Doc, - Default: def, - }) - } - case "hints": - for _, a := range api.Hints { - opt.EnumKeys.Keys = append(opt.EnumKeys.Keys, doc.EnumKey{ - Name: fmt.Sprintf("%q", a.Name), - Doc: a.Doc, - Default: strconv.FormatBool(a.Default), - }) - } } } + + catName := strings.TrimSuffix(category.Type().Name(), "Options") + api.Options[catName] = opts } return api, nil } @@ -241,7 +206,7 @@ func loadOptions(category reflect.Value, optsType types.Object, pkg *packages.Pa return nil, err } - enums, err := loadEnums(pkg) + enums, err := loadEnums(pkg) // TODO(adonovan): do this only once at toplevel. if err != nil { return nil, err } @@ -287,24 +252,42 @@ func loadOptions(category reflect.Value, optsType types.Object, pkg *packages.Pa return nil, err } + // Derive the doc-and-api.json type from the Go field type. + // + // In principle, we should use JSON nomenclature here + // (number, array, object, etc; see #68057), but in + // practice we use the Go type string ([]T, map[K]V, + // etc) with only one tweak: enumeration types are + // replaced by "enum", including when they appear as + // map keys. + // + // Notable edge cases: + // - any (e.g. in linksInHover) is really a sum of false | true | "internal". + // - time.Duration is really a string with a particular syntax. typ := typesField.Type().String() if _, ok := enums[typesField.Type()]; ok { typ = "enum" } name := lowerFirst(typesField.Name()) + // enum-keyed maps var enumKeys doc.EnumKeys if m, ok := typesField.Type().Underlying().(*types.Map); ok { - e, ok := enums[m.Key()] - if ok { - typ = strings.Replace(typ, m.Key().String(), m.Key().Underlying().String(), 1) - } - keys, err := collectEnumKeys(name, m, reflectField, e) - if err != nil { - return nil, err - } - if keys != nil { - enumKeys = *keys + if values, ok := enums[m.Key()]; ok { + // Update type name: "map[CodeLensSource]T" -> "map[enum]T" + // hack: assumes key substring is unique! + typ = strings.Replace(typ, m.Key().String(), "enum", 1) + + enumKeys.ValueType = m.Elem().String() // e.g. bool + + // For map[enum]T fields, gather the set of valid + // EnumKeys (from type information). If T=bool, also + // record the default value (from reflection). + keys, err := collectEnumKeys(m, reflectField, values) + if err != nil { + return nil, err + } + enumKeys.Keys = keys } } @@ -331,7 +314,7 @@ func loadOptions(category reflect.Value, optsType types.Object, pkg *packages.Pa // loadEnums returns a description of gopls' settings enum types based on static analysis. func loadEnums(pkg *packages.Package) (map[types.Type][]doc.EnumValue, error) { - enums := map[types.Type][]doc.EnumValue{} + enums := make(map[types.Type][]doc.EnumValue) for _, name := range pkg.Types.Scope().Names() { obj := pkg.Types.Scope().Lookup(name) cnst, ok := obj.(*types.Const) @@ -352,23 +335,25 @@ func loadEnums(pkg *packages.Package) (map[types.Type][]doc.EnumValue, error) { } enums[obj.Type()] = append(enums[obj.Type()], v) } + + // linksInHover is a one-off edge case (true | false | "gopls") + // that doesn't warrant a general solution (e.g. struct tag). + enums[pkg.Types.Scope().Lookup("LinksInHoverEnum").Type()] = []doc.EnumValue{ + {Value: "false", Doc: "false: do not show links"}, + {Value: "true", Doc: "true: show links to the `linkTarget` domain"}, + {Value: `"gopls"`, Doc: "`\"gopls\"`: show links to gopls' internal documentation viewer"}, + } + return enums, nil } -func collectEnumKeys(name string, m *types.Map, reflectField reflect.Value, enumValues []doc.EnumValue) (*doc.EnumKeys, error) { - // Make sure the value type gets set for analyses and codelenses - // too. - if len(enumValues) == 0 && !hardcodedEnumKeys(name) { - return nil, nil - } - keys := &doc.EnumKeys{ - ValueType: m.Elem().String(), - } +func collectEnumKeys(m *types.Map, reflectField reflect.Value, enumValues []doc.EnumValue) ([]doc.EnumKey, error) { // We can get default values for enum -> bool maps. var isEnumBoolMap bool if basic, ok := m.Elem().Underlying().(*types.Basic); ok && basic.Kind() == types.Bool { isEnumBoolMap = true } + var keys []doc.EnumKey for _, v := range enumValues { var def string if isEnumBoolMap { @@ -378,7 +363,7 @@ func collectEnumKeys(name string, m *types.Map, reflectField reflect.Value, enum return nil, err } } - keys.Keys = append(keys.Keys, doc.EnumKey{ + keys = append(keys, doc.EnumKey{ Name: v.Value, Doc: v.Doc, Default: def, @@ -534,19 +519,19 @@ func structDoc(fields []*commandmeta.Field, level int) string { return b.String() } -// loadLenses combines the syntactic comments from the protocol +// loadLenses combines the syntactic comments from the settings // package with the default values from settings.DefaultOptions(), and // returns a list of Code Lens descriptors. -func loadLenses(protocolPkg *packages.Package, defaults map[protocol.CodeLensSource]bool) ([]*doc.Lens, error) { +func loadLenses(settingsPkg *packages.Package, defaults map[settings.CodeLensSource]bool) ([]*doc.Lens, error) { // Find the CodeLensSource enums among the files of the protocol package. // Map each enum value to its doc comment. enumDoc := make(map[string]string) - for _, f := range protocolPkg.Syntax { + for _, f := range settingsPkg.Syntax { for _, decl := range f.Decls { if decl, ok := decl.(*ast.GenDecl); ok && decl.Tok == token.CONST { for _, spec := range decl.Specs { spec := spec.(*ast.ValueSpec) - posn := safetoken.StartPosition(protocolPkg.Fset, spec.Pos()) + posn := safetoken.StartPosition(settingsPkg.Fset, spec.Pos()) if id, ok := spec.Type.(*ast.Ident); ok && id.Name == "CodeLensSource" { if len(spec.Names) != 1 || len(spec.Values) != 1 { return nil, fmt.Errorf("%s: declare one CodeLensSource per line", posn) @@ -571,7 +556,7 @@ func loadLenses(protocolPkg *packages.Package, defaults map[protocol.CodeLensSou // Build list of Lens descriptors. var lenses []*doc.Lens - addAll := func(sources map[protocol.CodeLensSource]cache.CodeLensSourceFunc, fileType string) error { + addAll := func(sources map[settings.CodeLensSource]cache.CodeLensSourceFunc, fileType string) error { slice := maps.Keys(sources) sort.Slice(slice, func(i, j int) bool { return slice[i] < slice[j] }) for _, source := range slice { @@ -614,21 +599,22 @@ func loadAnalyzers(m map[string]*settings.Analyzer) []*doc.Analyzer { return json } -func loadHints(m map[string]*golang.Hint) []*doc.Hint { - var sorted []string - for _, h := range m { - sorted = append(sorted, h.Name) +// loadHints derives and returns the inlay hints metadata from the settings.InlayHint type. +func loadHints(settingsPkg *packages.Package) ([]*doc.Hint, error) { + enums, err := loadEnums(settingsPkg) // TODO(adonovan): call loadEnums exactly once + if err != nil { + return nil, err } - sort.Strings(sorted) - var json []*doc.Hint - for _, name := range sorted { - h := m[name] - json = append(json, &doc.Hint{ - Name: h.Name, - Doc: h.Doc, + inlayHint := settingsPkg.Types.Scope().Lookup("InlayHint").Type() + var hints []*doc.Hint + for _, enumVal := range enums[inlayHint] { + name, _ := strconv.Unquote(enumVal.Value) + hints = append(hints, &doc.Hint{ + Name: name, + Doc: enumVal.Doc, }) } - return json + return hints, nil } func lowerFirst(x string) string { @@ -707,20 +693,10 @@ func rewriteSettings(prevContent []byte, api *doc.API) ([]byte, error) { fmt.Fprintf(&buf, "\n", opt.Name) // heading - // (The blob helps the reader see the start of each item, - // which is otherwise hard to discern in GitHub markdown.) - // - // TODO(adonovan): We should display not the Go type (e.g. - // `time.Duration`, `map[Enum]bool`) for each setting, - // but its JSON type, since that's the actual interface. - // We need a better way to derive accurate JSON type descriptions - // from Go types. eg. "a string parsed as if by - // `time.Duration.Parse`". (`time.Duration` is an integer, not - // a string!) // // We do not display the undocumented dotted-path alias // (h.title + "." + opt.Name) used by VS Code only. - fmt.Fprintf(&buf, "### ⬤ `%s` *%v*\n\n", opt.Name, opt.Type) + fmt.Fprintf(&buf, "### `%s %s`\n\n", opt.Name, opt.Type) // status switch opt.Status { @@ -739,10 +715,6 @@ func rewriteSettings(prevContent []byte, api *doc.API) ([]byte, error) { buf.WriteString(opt.Doc) // enums - // - // TODO(adonovan): `CodeLensSource` should be treated as an enum, - // but loadEnums considers only the `settings` package, - // not `protocol`. write := func(name, doc string) { if doc != "" { unbroken := parBreakRE.ReplaceAllString(doc, "\\\n") @@ -752,12 +724,14 @@ func rewriteSettings(prevContent []byte, api *doc.API) ([]byte, error) { } } if len(opt.EnumValues) > 0 && opt.Type == "enum" { + // enum as top-level type constructor buf.WriteString("\nMust be one of:\n\n") for _, val := range opt.EnumValues { write(val.Value, val.Doc) } } else if len(opt.EnumKeys.Keys) > 0 && shouldShowEnumKeysInSettings(opt.Name) { - buf.WriteString("\nCan contain any of:\n\n") + // enum as map key (currently just "annotations") + buf.WriteString("\nEach enum must be one of:\n\n") for _, val := range opt.EnumKeys.Keys { write(val.Name, val.Doc) } @@ -779,7 +753,9 @@ func rewriteSettings(prevContent []byte, api *doc.API) ([]byte, error) { var parBreakRE = regexp.MustCompile("\n{2,}") func shouldShowEnumKeysInSettings(name string) bool { - // These fields have too many possible options to print. + // These fields have too many possible options, + // or too voluminous documentation, to render as enums. + // Instead they each get their own page in the manual. return !(name == "analyses" || name == "codelenses" || name == "hints") } @@ -837,10 +813,6 @@ func collectGroups(opts []*doc.Option) []optionsGroup { return groups } -func hardcodedEnumKeys(name string) bool { - return name == "analyses" || name == "codelenses" -} - func capitalize(s string) string { return string(unicode.ToUpper(rune(s[0]))) + s[1:] } @@ -848,7 +820,7 @@ func capitalize(s string) string { func rewriteCodeLenses(prevContent []byte, api *doc.API) ([]byte, error) { var buf bytes.Buffer for _, lens := range api.Lenses { - fmt.Fprintf(&buf, "## ⬤ `%s`: %s\n\n", lens.Lens, lens.Title) + fmt.Fprintf(&buf, "## `%s`: %s\n\n", lens.Lens, lens.Title) fmt.Fprintf(&buf, "%s\n\n", lens.Doc) fmt.Fprintf(&buf, "Default: %v\n\n", onOff(lens.Default)) fmt.Fprintf(&buf, "File type: %s\n\n", lens.FileType) @@ -859,7 +831,10 @@ func rewriteCodeLenses(prevContent []byte, api *doc.API) ([]byte, error) { func rewriteCommands(prevContent []byte, api *doc.API) ([]byte, error) { var buf bytes.Buffer for _, command := range api.Commands { - fmt.Fprintf(&buf, "### **%v**\nIdentifier: `%v`\n\n%v\n\n", command.Title, command.Command, command.Doc) + // Emit HTML anchor as GitHub markdown doesn't support + // "# Heading {#anchor}" syntax. + fmt.Fprintf(&buf, "\n", command.Command) + fmt.Fprintf(&buf, "## `%s`: **%s**\n\n%v\n\n", command.Command, command.Title, command.Doc) if command.ArgDoc != "" { fmt.Fprintf(&buf, "Args:\n\n```\n%s\n```\n\n", command.ArgDoc) } diff --git a/gopls/doc/helix.md b/gopls/doc/helix.md index 83f923de923..209ffdaaa81 100644 --- a/gopls/doc/helix.md +++ b/gopls/doc/helix.md @@ -1,4 +1,4 @@ -# Helix +# Gopls: Using Helix Configuring `gopls` to work with Helix is rather straightforward. Install `gopls`, and then add it to the `PATH` variable. If it is in the `PATH` variable, Helix will be able to detect it automatically. diff --git a/gopls/doc/inlayHints.md b/gopls/doc/inlayHints.md index 2ae9a2828af..0e84d43f1da 100644 --- a/gopls/doc/inlayHints.md +++ b/gopls/doc/inlayHints.md @@ -1,29 +1,35 @@ -# Hints +# Gopls: Inlay hints -This document describes the inlay hints that `gopls` uses inside the editor. +Inlay hints are helpful annotations that the editor can optionally +display in-line in the source code, such as the names of parameters in +a function call. This document describes the inlay hints available +from `gopls`. + ## **assignVariableTypes** -Enable/disable inlay hints for variable types in assign statements: +`"assignVariableTypes"` controls inlay hints for variable types in assign statements: ```go i/* int*/, j/* int*/ := 0, len(r)-1 ``` + **Disabled by default. Enable it by setting `"hints": {"assignVariableTypes": true}`.** ## **compositeLiteralFields** -Enable/disable inlay hints for composite literal field names: +`"compositeLiteralFields"` inlay hints for composite literal field names: ```go {/*in: */"Hello, world", /*want: */"dlrow ,olleH"} ``` + **Disabled by default. Enable it by setting `"hints": {"compositeLiteralFields": true}`.** ## **compositeLiteralTypes** -Enable/disable inlay hints for composite literal types: +`"compositeLiteralTypes"` controls inlay hints for composite literal types: ```go for _, c := range []struct { in, want string @@ -32,11 +38,12 @@ Enable/disable inlay hints for composite literal types: } ``` + **Disabled by default. Enable it by setting `"hints": {"compositeLiteralTypes": true}`.** ## **constantValues** -Enable/disable inlay hints for constant values: +`"constantValues"` controls inlay hints for constant values: ```go const ( KindNone Kind = iota/* = 0*/ @@ -46,35 +53,39 @@ Enable/disable inlay hints for constant values: ) ``` + **Disabled by default. Enable it by setting `"hints": {"constantValues": true}`.** ## **functionTypeParameters** -Enable/disable inlay hints for implicit type parameters on generic functions: +`"functionTypeParameters"` inlay hints for implicit type parameters on generic functions: ```go myFoo/*[int, string]*/(1, "hello") ``` + **Disabled by default. Enable it by setting `"hints": {"functionTypeParameters": true}`.** ## **parameterNames** -Enable/disable inlay hints for parameter names: +`"parameterNames"` controls inlay hints for parameter names: ```go parseInt(/* str: */ "123", /* radix: */ 8) ``` + **Disabled by default. Enable it by setting `"hints": {"parameterNames": true}`.** ## **rangeVariableTypes** -Enable/disable inlay hints for variable types in range statements: +`"rangeVariableTypes"` controls inlay hints for variable types in range statements: ```go for k/* int*/, v/* string*/ := range []string{} { fmt.Println(k, v) } ``` + **Disabled by default. Enable it by setting `"hints": {"rangeVariableTypes": true}`.** diff --git a/gopls/doc/refactor-inline.md b/gopls/doc/refactor-inline.md index dd857f874dd..cdddeb29e6e 100644 --- a/gopls/doc/refactor-inline.md +++ b/gopls/doc/refactor-inline.md @@ -1,4 +1,6 @@ + + Gopls v0.14 supports a new refactoring operation: inlining of function calls. diff --git a/gopls/doc/release/v0.16.0.md b/gopls/doc/release/v0.16.0.md index dc004c44e85..1bcb5ec3d06 100644 --- a/gopls/doc/release/v0.16.0.md +++ b/gopls/doc/release/v0.16.0.md @@ -1,64 +1,121 @@ -gopls/v0.16.0 +# gopls/v0.16.0 + ``` -go install golang.org/x/tools/gopls@v0.16.0 +go install golang.org/x/tools/gopls@v0.16.0-pre.2 ``` -## Configuration Changes - -- The experimental "allowImplicitNetworkAccess" setting is deprecated (but not - yet removed). Please comment on https://go.dev/issue/66861 if you use this +This release includes several features and bug fixes, and is the first +version of gopls to support Go 1.23. To install it, run: + +## New support policy; end of support for Go 1.19 and Go 1.20 + +**TL;DR: We are narrowing gopls' support window, but this is unlikely to +affect you as long as you use at least Go 1.21 to build gopls. This doesn't +affect gopls' support for the code you are writing.** + +This is the last release of gopls that may be built with Go 1.19 or Go 1.20, +and also the last to support integrating with go command versions 1.19 and +1.20. If built or used with either of these Go versions, it will display +a message advising the user to upgrade. + +When using gopls, there are three versions to be aware of: +1. The _gopls build go version_: the version of Go used to build gopls. +2. The _go command version_: the version of the go list command executed by + gopls to load information about your workspace. +3. The _language version_: the version in the go directive of the current + file's enclosing go.mod file, which determines the file's Go language + semantics. + +This gopls release, v0.16.0, is the final release to support Go 1.19 and Go +1.20 as the _gopls build go version_ or _go command version_. There is no +change to gopls' support for all _language versions_--in fact this support has +somewhat improved with the addition of the `stdversion` analyzer (see below). + +Starting with gopls@v0.17.0, which will be released after Go 1.23.0 is released +in August, gopls will only support the latest version of Go as the +_gopls build go version_. +However, thanks to the [forward compatibility](https://go.dev/blog/toolchain) +added to Go 1.21, any necessary toolchain upgrade should be handled +automatically for users of Go 1.21 or later, just like any other dependency. +Additionally, we are reducing our _go command version_ support window from +4 versions to 3. Note that this means if you have at least Go 1.21 installed on +your system, you should still be able to `go install` and use gopls@v0.17.0. + +We have no plans to ever change our _language version_ support: we expect that +gopls will always support developing programs that target _any_ Go version. + +By focusing on building gopls with the latest Go version, we can significantly +reduce our maintenance burden and help improve the stability of future gopls +releases. See the newly updated +[support policy](https://github.com/golang/tools/tree/master/gopls#support-policy) +for details. Please comment on golang/go#65917 if +you have concerns about this change. + +## Configuration changes + +- The experimental `allowImplicitNetworkAccess` setting is deprecated (but not + yet removed). Please comment on golang/go#66861 if you use this setting and would be impacted by its removal. ## New features +### Go 1.23 support + +This version of gopls is the first to support the new language features of Go 1.23, +including +[range-over-func](https://go.dev/wiki/RangefuncExperiment) iterators +and support for the +[`godebug` directive](https://go.dev/ref/mod#go-mod-file-godebug) +in go.mod files. + ### Integrated documentation viewer -Gopls now offers a "View package documentation" code action that opens -a local web page displaying the generated documentation for the -current Go package in a form similar to https://pkg.go.dev. -The page will be initially scrolled to the documentation for the -declaration containing the cursor. +Gopls now offers a "Browse documentation" code action that opens a +local web page displaying the generated documentation for Go packages +and symbols in a form similar to https://pkg.go.dev. +The package or symbol is chosen based on the current selection. + Use this feature to preview the marked-up documentation as you prepare API changes, or to read the documentation for locally edited packages, even ones that have not yet been saved. Reload the page after an edit to see updated documentation. -TODO: demo in VS Code. + -Clicking on the source-code link associated with a declaration will +As in `pkg.go.dev`, the heading for each symbol contains a link to the +source code of its declaration. In `pkg.go.dev`, these links would refer +to a source code page on a site such as GitHub or Google Code Search. +However, in gopls' internal viewer, clicking on one of these links will cause your editor to navigate to the declaration. +(This feature requires that your LSP client honors the `showDocument` downcall.) -TODO: demo of source linking. + Editor support: - -- VS Code: use the `Source action > View package documentation` menu item. +- VS Code: use the "Source action > Browse documentation for func fmt.Println" menu item. Note: source links navigate the editor but don't yet raise the window yet. - Please upvote https://github.com/microsoft/vscode/issues/208093 and - https://github.com/microsoft/vscode/issues/207634 (temporarily closed). - -- Emacs: requires eglot v1.17. You may find this `go-doc` function a - useful shortcut: + Please upvote microsoft/vscode#208093 and microsoft/vscode#207634 (temporarily closed). +- Emacs: requires eglot v1.17. Use `M-x go-browse-doc` from github.com/dominikh/go-mode.el. -```lisp -(eglot--code-action eglot-code-action-doc "source.doc") +The `linksInHover` setting now supports a new value, `"gopls"`, +that causes documentation links in the the Markdown output +of the Hover operation to link to gopls' internal doc viewer. -(defalias 'go-doc #'eglot-code-action-doc - "View documentation for the current Go package.") -``` -- TODO: test in vim, neovim, sublime, helix. +### Browse free symbols -### Free symbols - -Gopls offers another web-based code action, "Show free symbols", +Gopls offers another web-based code action, "Browse free symbols", which displays the free symbols referenced by the selected code. A symbol is "free" if it is referenced within the selection but -declared outside of it. The free symbols that are variables are, in -effect, the set of parameters that would be needed if the block were -extracted into its own function in the same package. +declared outside of it. The free symbols that are variables are +approximately the set of parameters that would be needed if the block +were extracted into its own function. Even when you don't intend to extract a block into a new function, this information can help you to tell at a glance what names a block @@ -68,25 +125,46 @@ Each dotted path of identifiers (such as `file.Name.Pos`) is reported as a separate item, so that you can see which parts of a complex type are actually needed. -Viewing the free symbols of the body of a function may reveal that +The free symbols of the body of a function may reveal that only a small part (a single field of a struct, say) of one of the function's parameters is used, allowing you to simplify and generalize the function by choosing a different type for that parameter. -- TODO screenshot + + +Editor support: +- VS Code: use the `Source action > Browse free symbols` menu item. +- Emacs: requires eglot v1.17. Use `M-x go-browse-freesymbols` from github.com/dominikh/go-mode.el. -- VS Code: use the `Source action > View free symbols` menu item. +### Browse assembly -- Emacs: requires eglot v1.17. You may find this `go-doc` function a - useful shortcut: +Gopls offers a third web-based code action, "Browse assembly for f", +which displays an assembly listing of the declaration of the function +f enclosing the selected code, plus any nested functions such as +function literals or deferred calls. -```lisp -(eglot--code-action eglot-code-action-freesymbols "source.freesymbols") +Gopls invokes the compiler to generate the report; +reloading the page updates the report. -(defalias 'go-freesymbols #'eglot-code-action-freesymbols - "View free symbols referred to by the current selection.") -``` -TODO(dominikh/go-mode.el#436): add both of these to go-mode.el. +The machine architecture is determined by the build +configuration that gopls selects for the current file. +This is usually the same as your machine's GOARCH unless you are +working in a file with `go:build` tags for a different architecture. + + + +Gopls cannot yet display assembly for generic functions: +generic functions are not fully compiled until they are instantiated, +but any function declaration enclosing the selection cannot be an +instantiated generic function. + + +Editor support: +- VS Code: use the "Source action > Browse assembly for f" menu item. +- Emacs: requires eglot v1.17. Use `M-x go-browse-assembly` from github.com/dominikh/go-mode.el. ### `unusedwrite` analyzer @@ -117,6 +195,37 @@ func (s S) set(x int) { } ``` +### `stdversion` analyzer + +The new +[`stdversion`](https://pkg.go.dev/golang.org/x/tools/go/analysis/passes/stdversion) +analyzer warns about the use of too-new standard library symbols based on the +version of the `go` directive in your `go.mod` file. This improves our support +for older _language versions_ (see above), even when gopls is built with +a recent Go version. + +Consider the go.mod file and Go file below. +The declaration of `var `alias refers to a type, `types.Alias`, +introduced in go1.22, but the file belongs to a module that requires +only go1.21, so the analyzer reports a diagnostic: + +``` +module example.com +go 1.21 +``` + +```go +package p + +import "go/types" + +var alias types.Alias // types.Alias requires go1.22 or later (module is go1.21) +``` + +When an individual file is build-tagged for a release of Go other than +than module's version, the analyzer will apply appropriate checks for +the file's version. + ### Two more vet analyzers The [framepointer](https://pkg.go.dev/golang.org/x/tools/go/analysis/passes/framepointer) @@ -128,39 +237,46 @@ Henceforth, gopls will always include any analyzers run by vet. ### Hover shows size/offset info, and struct tags -TODO: consolidate release notes related to Hover improvements. - Hovering over the identifier that declares a type or struct field now -displays the size information for the type, and the offset information -for the field. In addition, it reports the percentage of wasted space -due to suboptimal ordering of struct fields, if this figure is 20% or -higher. This information may be helpful when making space -optimizations to your data structures, or when reading assembly code. +displays the size information for the type: -TODO: example hover image. + -Hovering over a field with struct tags now also includes those tags. +and the offset information for the field: -TODO: example hover image + -### Hover and definition on doc links +In addition, it reports the percentage of wasted space due to +suboptimal ordering of struct fields, if this figure is 20% or higher: -Go 1.19 added support for [links in doc -comments](https://go.dev/doc/comment#links), allowing the -documentation for one symbol to reference another: + -TODO: turn the code below into a VS Code screenshot of hover. +In the struct above, alignment rules require each of the two boolean +fields (1 byte) to occupy a complete word (8 bytes), leading to (7 + +7) / (3 * 8) = 58% waste. +Placing the two booleans together would save a word. -```go -// Logf logs a message formatted in the manner of [fmt.Printf]. -func Logf(format string, args ...any) -``` +This information may be helpful when making space optimizations to +your data structures, or when reading assembly code. + +Also, hovering over a reference to a field with a struct tag now also +display the tag: + + + +### Hover and "Go to Definition" work on symbols in doc comments -Gopls's Hover and Definition operations now treat these links just +Go 1.19 added support for [doc links](https://go.dev/doc/comment#links), +allowing the doc comment for one symbol to reference another. + +Gopls' Hover and Definition operations now treat these links just like identifiers, so hovering over one will display information about -the symbol, and "Go to definition" will navigate to its declaration. -Thanks to @rogeryk for contributing this feature. +the symbol: + + +Similarly, "Go to definition" will navigate to its declaration. +Thanks to @rogeryk for contributing this feature. ## Bugs fixed diff --git a/gopls/doc/release/v0.17.0.md b/gopls/doc/release/v0.17.0.md new file mode 100644 index 00000000000..65b835d6737 --- /dev/null +++ b/gopls/doc/release/v0.17.0.md @@ -0,0 +1,30 @@ + + +# Configuration Changes + +The `fieldalignment` analyzer, previously disabled by default, has +been removed: it is redundant with the hover size/offset information +displayed by v0.16.0 and its diagnostics were confusing. + + +# New features + +## Extract declarations to new file +Gopls now offers another code action, "Extract declarations to new file", +which moves selected code sections to a newly created file within the +same package. The created filename is chosen as the first {function, type, +const, var} name encountered. In addition, import declarations are added or +removed as needed. + +The user can invoke this code action by selecting a function name, the keywords +`func`, `const`, `var`, `type`, or by placing the caret on them without selecting, +or by selecting a whole declaration or multiple declrations. + +In order to avoid ambiguity and surprise about what to extract, some kinds +of paritial selection of a declration cannot invoke this code action. + +## Standard library version information in Hover + +Hovering over a standard library symbol now displays information about the first +Go release containing the symbol. For example, hovering over `errors.As` shows +"Added in go1.13". diff --git a/gopls/doc/releases.md b/gopls/doc/releases.md index befb92c3966..c4220e41116 100644 --- a/gopls/doc/releases.md +++ b/gopls/doc/releases.md @@ -1,4 +1,4 @@ -# Gopls release policy +# Gopls: Release policy Gopls releases follow [semver](http://semver.org), with major changes and new features introduced only in new minor versions (i.e. versions of the form diff --git a/gopls/doc/semantictokens.md b/gopls/doc/semantictokens.md index a1e140d29ec..761d94a02d1 100644 --- a/gopls/doc/semantictokens.md +++ b/gopls/doc/semantictokens.md @@ -1,4 +1,7 @@ -# Semantic Tokens +# Gopls: Semantic Tokens + +TODO(adonovan): this doc is internal, not for end users. +Move it closer to the code in golang or protocol/semtok. The [LSP](https://microsoft.github.io/language-server-protocol/specifications/specification-3-17/#textDocument_semanticTokens) specifies semantic tokens as a way of telling clients about language-specific diff --git a/gopls/doc/settings.md b/gopls/doc/settings.md index a3c5bb5ddeb..d64be6370ef 100644 --- a/gopls/doc/settings.md +++ b/gopls/doc/settings.md @@ -1,8 +1,8 @@ -# Settings +# Gopls: Settings This document describes gopls' configuration settings. -Gopls settings are defined by an JSON object whose valid fields are +Gopls settings are defined by a JSON object whose valid fields are described below. These fields are gopls-specific, and generic LSP clients have no knowledge of them. @@ -17,9 +17,7 @@ Some clients also permit settings to be configured differently for each workspace folder. Any settings that are experimental or for debugging purposes are -marked as such. To enable all experimental features, use -**allExperiments: `true`**. You will still be able to independently -override specific experimental features. +marked as such. - + + -
Gopls server has terminated. Page is inactive.

Free symbols

The selected code contains references to these free* symbols: @@ -210,7 +175,7 @@ function httpGET(url) { fmt.Fprintf(&buf, "

    \n") for _, imp := range model.Imported { fmt.Fprintf(&buf, "
  • import \"%s\" // for %s
    • \n", - pkgURL(imp.Path, ""), + web.PkgURL(viewID, imp.Path, ""), html.EscapeString(string(imp.Path)), strings.Join(imp.Symbols, ", ")) } @@ -219,28 +184,6 @@ function httpGET(url) { } buf.WriteString("
\n") - // sourceLink returns HTML for a link to open a file in the client editor. - // TODO(adonovan): factor with RenderPackageDoc. - sourceLink := func(text, url string) string { - // The /open URL returns nothing but has the side effect - // of causing the LSP client to open the requested file. - // So we use onclick to prevent the browser from navigating. - // We keep the href attribute as it causes the to render - // as a link: blue, underlined, with URL hover information. - return fmt.Sprintf(`%[2]s`, - html.EscapeString(url), text) - } - - // objHTML returns HTML for obj.Name(), possibly as a link. - // TODO(adonovan): factor with RenderPackageDoc. - objHTML := func(obj types.Object) string { - text := obj.Name() - if posn := safetoken.StartPosition(pkg.FileSet(), obj.Pos()); posn.IsValid() { - return sourceLink(text, posURL(posn.Filename, posn.Line, posn.Column)) - } - return text - } - // -- package and local symbols -- showSymbols := func(scope, title string, symbols []Symbol) { @@ -253,7 +196,7 @@ function httpGET(url) { if i > 0 { buf.WriteByte('.') } - buf.WriteString(objHTML(obj)) + buf.WriteString(objHTML(pkg.FileSet(), web, obj)) } fmt.Fprintf(&buf, " %s\n", html.EscapeString(sym.Type)) } @@ -310,7 +253,7 @@ function httpGET(url) { as a separate item, so that you can see which parts of a complex type are actually needed. - Viewing the free symbols referenced by the body of a function may + The free symbols referenced by the body of a function may reveal that only a small part (a single field of a struct, say) of one of the function's parameters is used, allowing you to simplify and generalize the function by choosing a different type for that @@ -452,3 +395,26 @@ func freeRefs(pkg *types.Package, info *types.Info, file *ast.File, start, end t ast.Inspect(path[0], visit) return free } + +// objHTML returns HTML for obj.Name(), possibly marked up as a link +// to the web server that, when visited, opens the declaration in the +// client editor. +func objHTML(fset *token.FileSet, web Web, obj types.Object) string { + text := obj.Name() + if posn := safetoken.StartPosition(fset, obj.Pos()); posn.IsValid() { + url := web.SrcURL(posn.Filename, posn.Line, posn.Column) + return sourceLink(text, url) + } + return text +} + +// sourceLink returns HTML for a link to open a file in the client editor. +func sourceLink(text, url string) string { + // The /src URL returns nothing but has the side effect + // of causing the LSP client to open the requested file. + // So we use onclick to prevent the browser from navigating. + // We keep the href attribute as it causes the to render + // as a link: blue, underlined, with URL hover information. + return fmt.Sprintf(`%[2]s`, + html.EscapeString(url), text) +} diff --git a/gopls/internal/golang/freesymbols_test.go b/gopls/internal/golang/freesymbols_test.go index dd6d947fbc2..9ad8ca3ee6e 100644 --- a/gopls/internal/golang/freesymbols_test.go +++ b/gopls/internal/golang/freesymbols_test.go @@ -21,8 +21,8 @@ import ( // TestFreeRefs is a unit test of the free-references algorithm. func TestFreeRefs(t *testing.T) { - if runtime.GOOS == "js" { - t.Skip("some test imports are unsupported on js") + if runtime.GOOS == "js" || runtime.GOARCH == "wasm" { + t.Skip("some test imports are unsupported on js or wasm") } for i, test := range []struct { diff --git a/gopls/internal/golang/hover.go b/gopls/internal/golang/hover.go index 1eb32d9e4b7..2edb8a99d34 100644 --- a/gopls/internal/golang/hover.go +++ b/gopls/internal/golang/hover.go @@ -40,6 +40,7 @@ import ( "golang.org/x/tools/gopls/internal/util/typesutil" "golang.org/x/tools/internal/aliases" "golang.org/x/tools/internal/event" + "golang.org/x/tools/internal/stdlib" "golang.org/x/tools/internal/tokeninternal" "golang.org/x/tools/internal/typeparams" "golang.org/x/tools/internal/typesinternal" @@ -70,12 +71,17 @@ type hoverJSON struct { // LinkPath is the pkg.go.dev link for the given symbol. // For example, the "go/ast" part of "pkg.go.dev/go/ast#Node". + // It may have a module version suffix "@v1.2.3". LinkPath string `json:"linkPath"` // LinkAnchor is the pkg.go.dev link anchor for the given symbol. // For example, the "Node" part of "pkg.go.dev/go/ast#Node". LinkAnchor string `json:"linkAnchor"` + // stdVersion is the Go release version at which this symbol became available. + // It is nil for non-std library. + stdVersion *stdlib.Version + // New fields go below, and are unexported. The existing // exported fields are underspecified and have already // constrained our movements too much. A detailed JSON @@ -98,7 +104,10 @@ type hoverJSON struct { } // Hover implements the "textDocument/hover" RPC for Go files. -func Hover(ctx context.Context, snapshot *cache.Snapshot, fh file.Handle, position protocol.Position) (*protocol.Hover, error) { +// It may return nil even on success. +// +// If pkgURL is non-nil, it should be used to generate doc links. +func Hover(ctx context.Context, snapshot *cache.Snapshot, fh file.Handle, position protocol.Position, pkgURL func(path PackagePath, fragment string) protocol.URI) (*protocol.Hover, error) { ctx, done := event.Start(ctx, "golang.Hover") defer done() @@ -109,7 +118,7 @@ func Hover(ctx context.Context, snapshot *cache.Snapshot, fh file.Handle, positi if h == nil { return nil, nil } - hover, err := formatHover(h, snapshot.Options()) + hover, err := formatHover(h, snapshot.Options(), pkgURL) if err != nil { return nil, err } @@ -125,7 +134,44 @@ func Hover(ctx context.Context, snapshot *cache.Snapshot, fh file.Handle, positi // hover computes hover information at the given position. If we do not support // hovering at the position, it returns _, nil, nil: an error is only returned // if the position is valid but we fail to compute hover information. +// +// TODO(adonovan): strength-reduce file.Handle to protocol.DocumentURI. func hover(ctx context.Context, snapshot *cache.Snapshot, fh file.Handle, pp protocol.Position) (protocol.Range, *hoverJSON, error) { + // Check for hover inside the builtin file before attempting type checking + // below. NarrowestPackageForFile may or may not succeed, depending on + // whether this is a GOROOT view, but even if it does succeed the resulting + // package will be command-line-arguments package. The user should get a + // hover for the builtin object, not the object type checked from the + // builtin.go. + if snapshot.IsBuiltin(fh.URI()) { + pgf, err := snapshot.BuiltinFile(ctx) + if err != nil { + return protocol.Range{}, nil, err + } + pos, err := pgf.PositionPos(pp) + if err != nil { + return protocol.Range{}, nil, err + } + path, _ := astutil.PathEnclosingInterval(pgf.File, pos, pos) + if id, ok := path[0].(*ast.Ident); ok { + rng, err := pgf.NodeRange(id) + if err != nil { + return protocol.Range{}, nil, err + } + var obj types.Object + if id.Name == "Error" { + obj = types.Universe.Lookup("error").Type().Underlying().(*types.Interface).Method(0) + } else { + obj = types.Universe.Lookup(id.Name) + } + if obj != nil { + h, err := hoverBuiltin(ctx, snapshot, obj) + return rng, h, err + } + } + return protocol.Range{}, nil, nil // no object to hover + } + pkg, pgf, err := NarrowestPackageForFile(ctx, snapshot, fh.URI()) if err != nil { return protocol.Range{}, nil, err @@ -196,7 +242,7 @@ func hover(ctx context.Context, snapshot *cache.Snapshot, fh file.Handle, pp pro if hoverRange == nil { hoverRange = &rng } - return *hoverRange, hoverJSON, nil + return *hoverRange, hoverJSON, nil // (hoverJSON may be nil) } } // Handle hovering over (non-import-path) literals. @@ -533,13 +579,13 @@ func hover(ctx context.Context, snapshot *cache.Snapshot, fh file.Handle, pp pro pkg := obj.Pkg() if recv != nil { linkName = fmt.Sprintf("(%s.%s).%s", pkg.Name(), recv.Name(), obj.Name()) - if obj.Exported() && recv.Exported() && pkg.Scope().Lookup(recv.Name()) == recv { + if obj.Exported() && recv.Exported() && isPackageLevel(recv) { linkPath = pkg.Path() anchor = fmt.Sprintf("%s.%s", recv.Name(), obj.Name()) } } else { linkName = fmt.Sprintf("%s.%s", pkg.Name(), obj.Name()) - if obj.Exported() && pkg.Scope().Lookup(obj.Name()) == obj { + if obj.Exported() && isPackageLevel(obj) { linkPath = pkg.Path() anchor = obj.Name() } @@ -554,6 +600,11 @@ func hover(ctx context.Context, snapshot *cache.Snapshot, fh file.Handle, pp pro linkPath = strings.Replace(linkPath, mod.Path, mod.Path+"@"+mod.Version, 1) } + var version *stdlib.Version + if symbol := StdSymbolOf(obj); symbol != nil { + version = &symbol.Version + } + return *hoverRange, &hoverJSON{ Synopsis: doc.Synopsis(docText), FullDocumentation: docText, @@ -565,6 +616,7 @@ func hover(ctx context.Context, snapshot *cache.Snapshot, fh file.Handle, pp pro typeDecl: typeDecl, methods: methods, promotedFields: fields, + stdVersion: version, }, nil } @@ -586,13 +638,16 @@ func hoverBuiltin(ctx context.Context, snapshot *cache.Snapshot, obj types.Objec }, nil } - pgf, node, err := builtinDecl(ctx, snapshot, obj) + pgf, ident, err := builtinDecl(ctx, snapshot, obj) if err != nil { return nil, err } - var comment *ast.CommentGroup - path, _ := astutil.PathEnclosingInterval(pgf.File, node.Pos(), node.End()) + var ( + comment *ast.CommentGroup + decl ast.Decl + ) + path, _ := astutil.PathEnclosingInterval(pgf.File, ident.Pos(), ident.Pos()) for _, n := range path { switch n := n.(type) { case *ast.GenDecl: @@ -600,17 +655,17 @@ func hoverBuiltin(ctx context.Context, snapshot *cache.Snapshot, obj types.Objec comment = n.Doc node2 := *n node2.Doc = nil - node = &node2 + decl = &node2 case *ast.FuncDecl: // Ditto. comment = n.Doc node2 := *n node2.Doc = nil - node = &node2 + decl = &node2 } } - signature := formatNodeFile(pgf.Tok, node) + signature := formatNodeFile(pgf.Tok, decl) // Replace fake types with their common equivalent. // TODO(rfindley): we should instead use obj.Type(), which would have the // *actual* types of the builtin call. @@ -1086,7 +1141,8 @@ func parseFull(ctx context.Context, snapshot *cache.Snapshot, fset *token.FileSe return pgf, fullPos, nil } -func formatHover(h *hoverJSON, options *settings.Options) (string, error) { +// If pkgURL is non-nil, it should be used to generate doc links. +func formatHover(h *hoverJSON, options *settings.Options, pkgURL func(path PackagePath, fragment string) protocol.URI) (string, error) { maybeMarkdown := func(s string) string { if s != "" && options.PreferredContentFormat == protocol.Markdown { s = fmt.Sprintf("```go\n%s\n```", strings.Trim(s, "\n")) @@ -1121,11 +1177,15 @@ func formatHover(h *hoverJSON, options *settings.Options) (string, error) { formatDoc(h, options), maybeMarkdown(h.promotedFields), maybeMarkdown(h.methods), - formatLink(h, options), + fmt.Sprintf("Added in %v", h.stdVersion), + formatLink(h, options, pkgURL), } if h.typeDecl != "" { parts[0] = "" // type: suppress redundant Signature } + if h.stdVersion == nil || *h.stdVersion == stdlib.Version(0) { + parts[5] = "" // suppress stdlib version if not applicable or initial version 1.0 + } parts = slices.Remove(parts, "") var b strings.Builder @@ -1146,18 +1206,102 @@ func formatHover(h *hoverJSON, options *settings.Options) (string, error) { } } -func formatLink(h *hoverJSON, options *settings.Options) string { - if !options.LinksInHover || options.LinkTarget == "" || h.LinkPath == "" { +// StdSymbolOf returns the std lib symbol information of the given obj. +// It returns nil if the input obj is not an exported standard library symbol. +func StdSymbolOf(obj types.Object) *stdlib.Symbol { + if !obj.Exported() || obj.Pkg() == nil { + return nil + } + + // Symbols that not defined in standard library should return early. + // TODO(hxjiang): The returned slices is binary searchable. + symbols := stdlib.PackageSymbols[obj.Pkg().Path()] + if symbols == nil { + return nil + } + + // Handle Function, Type, Const & Var. + if isPackageLevel(obj) { + for _, s := range symbols { + if s.Kind == stdlib.Method || s.Kind == stdlib.Field { + continue + } + if s.Name == obj.Name() { + return &s + } + } + return nil + } + + // Handle Method. + if fn, _ := obj.(*types.Func); fn != nil { + isPtr, named := typesinternal.ReceiverNamed(fn.Type().(*types.Signature).Recv()) + if isPackageLevel(named.Obj()) { + for _, s := range symbols { + if s.Kind != stdlib.Method { + continue + } + ptr, recv, name := s.SplitMethod() + if ptr == isPtr && recv == named.Obj().Name() && name == fn.Name() { + return &s + } + } + return nil + } + } + + // Handle Field. + if v, _ := obj.(*types.Var); v != nil && v.IsField() { + for _, s := range symbols { + if s.Kind != stdlib.Field { + continue + } + + typeName, fieldName := s.SplitField() + if fieldName != v.Name() { + continue + } + + typeObj := obj.Pkg().Scope().Lookup(typeName) + if typeObj == nil { + continue + } + + if fieldObj, _, _ := types.LookupFieldOrMethod(typeObj.Type(), true, obj.Pkg(), fieldName); obj == fieldObj { + return &s + } + } + return nil + } + + return nil +} + +// If pkgURL is non-nil, it should be used to generate doc links. +func formatLink(h *hoverJSON, options *settings.Options, pkgURL func(path PackagePath, fragment string) protocol.URI) string { + if options.LinksInHover == false || h.LinkPath == "" { return "" } - plainLink := cache.BuildLink(options.LinkTarget, h.LinkPath, h.LinkAnchor) + var url protocol.URI + var caption string + if pkgURL != nil { // LinksInHover == "gopls" + path, _, _ := strings.Cut(h.LinkPath, "@") // remove optional module version suffix + url = pkgURL(PackagePath(path), h.LinkAnchor) + caption = "in gopls doc viewer" + } else { + if options.LinkTarget == "" { + return "" + } + url = cache.BuildLink(options.LinkTarget, h.LinkPath, h.LinkAnchor) + caption = "on " + options.LinkTarget + } switch options.PreferredContentFormat { case protocol.Markdown: - return fmt.Sprintf("[`%s` on %s](%s)", h.SymbolName, options.LinkTarget, plainLink) + return fmt.Sprintf("[`%s` %s](%s)", h.SymbolName, caption, url) case protocol.PlainText: return "" default: - return plainLink + return url } } diff --git a/gopls/internal/golang/inlay_hint.go b/gopls/internal/golang/inlay_hint.go index 6cf19b6f3c9..6e2b7f40d33 100644 --- a/gopls/internal/golang/inlay_hint.go +++ b/gopls/internal/golang/inlay_hint.go @@ -16,75 +16,13 @@ import ( "golang.org/x/tools/gopls/internal/cache" "golang.org/x/tools/gopls/internal/file" "golang.org/x/tools/gopls/internal/protocol" + "golang.org/x/tools/gopls/internal/settings" "golang.org/x/tools/gopls/internal/util/typesutil" "golang.org/x/tools/internal/event" "golang.org/x/tools/internal/typeparams" "golang.org/x/tools/internal/typesinternal" ) -const ( - maxLabelLength = 28 -) - -type InlayHintFunc func(node ast.Node, m *protocol.Mapper, tf *token.File, info *types.Info, q *types.Qualifier) []protocol.InlayHint - -type Hint struct { - Name string - Doc string - Run InlayHintFunc -} - -const ( - ParameterNames = "parameterNames" - AssignVariableTypes = "assignVariableTypes" - ConstantValues = "constantValues" - RangeVariableTypes = "rangeVariableTypes" - CompositeLiteralTypes = "compositeLiteralTypes" - CompositeLiteralFieldNames = "compositeLiteralFields" - FunctionTypeParameters = "functionTypeParameters" -) - -// AllInlayHints describes the various inlay-hints options. -// -// It is the source from which gopls/doc/inlayHints.md is generated. -var AllInlayHints = map[string]*Hint{ - AssignVariableTypes: { - Name: AssignVariableTypes, - Doc: "Enable/disable inlay hints for variable types in assign statements:\n```go\n\ti/* int*/, j/* int*/ := 0, len(r)-1\n```", - Run: assignVariableTypes, - }, - ParameterNames: { - Name: ParameterNames, - Doc: "Enable/disable inlay hints for parameter names:\n```go\n\tparseInt(/* str: */ \"123\", /* radix: */ 8)\n```", - Run: parameterNames, - }, - ConstantValues: { - Name: ConstantValues, - Doc: "Enable/disable inlay hints for constant values:\n```go\n\tconst (\n\t\tKindNone Kind = iota/* = 0*/\n\t\tKindPrint/* = 1*/\n\t\tKindPrintf/* = 2*/\n\t\tKindErrorf/* = 3*/\n\t)\n```", - Run: constantValues, - }, - RangeVariableTypes: { - Name: RangeVariableTypes, - Doc: "Enable/disable inlay hints for variable types in range statements:\n```go\n\tfor k/* int*/, v/* string*/ := range []string{} {\n\t\tfmt.Println(k, v)\n\t}\n```", - Run: rangeVariableTypes, - }, - CompositeLiteralTypes: { - Name: CompositeLiteralTypes, - Doc: "Enable/disable inlay hints for composite literal types:\n```go\n\tfor _, c := range []struct {\n\t\tin, want string\n\t}{\n\t\t/*struct{ in string; want string }*/{\"Hello, world\", \"dlrow ,olleH\"},\n\t}\n```", - Run: compositeLiteralTypes, - }, - CompositeLiteralFieldNames: { - Name: CompositeLiteralFieldNames, - Doc: "Enable/disable inlay hints for composite literal field names:\n```go\n\t{/*in: */\"Hello, world\", /*want: */\"dlrow ,olleH\"}\n```", - Run: compositeLiteralFields, - }, - FunctionTypeParameters: { - Name: FunctionTypeParameters, - Doc: "Enable/disable inlay hints for implicit type parameters on generic functions:\n```go\n\tmyFoo/*[int, string]*/(1, \"hello\")\n```", - Run: funcTypeParams, - }, -} - func InlayHint(ctx context.Context, snapshot *cache.Snapshot, fh file.Handle, pRng protocol.Range) ([]protocol.InlayHint, error) { ctx, done := event.Start(ctx, "golang.InlayHint") defer done() @@ -96,13 +34,13 @@ func InlayHint(ctx context.Context, snapshot *cache.Snapshot, fh file.Handle, pR // Collect a list of the inlay hints that are enabled. inlayHintOptions := snapshot.Options().InlayHintOptions - var enabledHints []InlayHintFunc + var enabledHints []inlayHintFunc for hint, enabled := range inlayHintOptions.Hints { if !enabled { continue } - if h, ok := AllInlayHints[hint]; ok { - enabledHints = append(enabledHints, h.Run) + if fn, ok := allInlayHints[hint]; ok { + enabledHints = append(enabledHints, fn) } } if len(enabledHints) == 0 { @@ -137,6 +75,18 @@ func InlayHint(ctx context.Context, snapshot *cache.Snapshot, fh file.Handle, pR return hints, nil } +type inlayHintFunc func(node ast.Node, m *protocol.Mapper, tf *token.File, info *types.Info, q *types.Qualifier) []protocol.InlayHint + +var allInlayHints = map[settings.InlayHint]inlayHintFunc{ + settings.AssignVariableTypes: assignVariableTypes, + settings.ConstantValues: constantValues, + settings.ParameterNames: parameterNames, + settings.RangeVariableTypes: rangeVariableTypes, + settings.CompositeLiteralTypes: compositeLiteralTypes, + settings.CompositeLiteralFieldNames: compositeLiteralFields, + settings.FunctionTypeParameters: funcTypeParams, +} + func parameterNames(node ast.Node, m *protocol.Mapper, tf *token.File, info *types.Info, _ *types.Qualifier) []protocol.InlayHint { callExpr, ok := node.(*ast.CallExpr) if !ok { @@ -393,6 +343,7 @@ func compositeLiteralTypes(node ast.Node, m *protocol.Mapper, tf *token.File, in } func buildLabel(s string) []protocol.InlayHintLabelPart { + const maxLabelLength = 28 label := protocol.InlayHintLabelPart{ Value: s, } diff --git a/gopls/internal/golang/lines.go b/gopls/internal/golang/lines.go index 761fee9e12d..24239941a2c 100644 --- a/gopls/internal/golang/lines.go +++ b/gopls/internal/golang/lines.go @@ -166,39 +166,22 @@ func findSplitJoinTarget(fset *token.FileSet, file *ast.File, src []byte, start, path, _ := astutil.PathEnclosingInterval(file, start, end) for _, node := range path { switch node := node.(type) { - case *ast.FuncDecl: - // target struct method declarations. - // function (...) someMethod(a int, b int, c int) (d int, e, int) {} - params := node.Type.Params - if isCursorInside(params.Opening, params.Closing) { - return "parameters", params, params.Opening, params.Closing - } - - results := node.Type.Results - if results != nil && isCursorInside(results.Opening, results.Closing) { - return "return values", results, results.Opening, results.Closing - } case *ast.FuncType: - // target function signature args and result. - // type someFunc func (a int, b int, c int) (d int, e int) - params := node.Params - if isCursorInside(params.Opening, params.Closing) { - return "parameters", params, params.Opening, params.Closing + // params or results of func signature + // Note: + // - each ast.Field (e.g. "x, y, z int") is considered a single item. + // - splitting Params and Results lists is not usually good style. + if p := node.Params; isCursorInside(p.Opening, p.Closing) { + return "parameters", p, p.Opening, p.Closing } - - results := node.Results - if results != nil && isCursorInside(results.Opening, results.Closing) { - return "return values", results, results.Opening, results.Closing + if r := node.Results; r != nil && isCursorInside(r.Opening, r.Closing) { + return "results", r, r.Opening, r.Closing } - case *ast.CallExpr: - // target function calls. - // someFunction(a, b, c) + case *ast.CallExpr: // f(a, b, c) if isCursorInside(node.Lparen, node.Rparen) { - return "parameters", node, node.Lparen, node.Rparen + return "arguments", node, node.Lparen, node.Rparen } - case *ast.CompositeLit: - // target composite lit instantiation (structs, maps, arrays). - // A{b: 1, c: 2, d: 3} + case *ast.CompositeLit: // T{a, b, c} if isCursorInside(node.Lbrace, node.Rbrace) { return "elements", node, node.Lbrace, node.Rbrace } diff --git a/gopls/internal/golang/pkgdoc.go b/gopls/internal/golang/pkgdoc.go index ce0e5bc7ac4..8f6b636027d 100644 --- a/gopls/internal/golang/pkgdoc.go +++ b/gopls/internal/golang/pkgdoc.go @@ -17,7 +17,6 @@ package golang // - list promoted methods---we have type information! // - gather Example tests, following go/doc and pkgsite. // - add option for doc.AllDecls: show non-exported symbols too. -// - abbreviate long signatures by replacing parameters 4 onwards with "...". // - style the
  • bullets in the index as invisible. // - add push notifications such as didChange -> reload. // - there appears to be a maximum file size beyond which the @@ -25,9 +24,9 @@ package golang // - modify JS httpGET function to give a transient visual indication // when clicking a source link that the editor is being navigated // (in case it doesn't raise itself, like VS Code). -// - move this into a new package, golang/pkgdoc, and then +// - move this into a new package, golang/web, and then // split out the various helpers without fear of polluting -// the golang package namespace. +// the golang package namespace? // - show "Deprecated" chip when appropriate. import ( @@ -43,36 +42,235 @@ import ( "path/filepath" "strings" + "golang.org/x/tools/go/ast/astutil" "golang.org/x/tools/gopls/internal/cache" + "golang.org/x/tools/gopls/internal/cache/parsego" "golang.org/x/tools/gopls/internal/protocol" - "golang.org/x/tools/gopls/internal/util/astutil" + goplsastutil "golang.org/x/tools/gopls/internal/util/astutil" "golang.org/x/tools/gopls/internal/util/bug" "golang.org/x/tools/gopls/internal/util/safetoken" "golang.org/x/tools/gopls/internal/util/slices" "golang.org/x/tools/gopls/internal/util/typesutil" + "golang.org/x/tools/internal/stdlib" "golang.org/x/tools/internal/typesinternal" ) -// TODO(adonovan): factor these two functions into an interface. -type ( - // A PkgURLFunc forms URLs of package or symbol documentation. - PkgURLFunc = func(path PackagePath, fragment string) protocol.URI +// DocFragment finds the package and (optionally) symbol identified by +// the current selection, and returns the package path and the +// optional symbol URL fragment (e.g. "#Buffer.Len") for a symbol, +// along with a title for the code action. +// +// It is called once to offer the code action, and again when the +// command is executed. This is slightly inefficient but ensures that +// the title and package/symbol logic are consistent in all cases. +// +// It returns zeroes if there is nothing to see here (e.g. reference to a builtin). +func DocFragment(pkg *cache.Package, pgf *parsego.File, start, end token.Pos) (pkgpath PackagePath, fragment, title string) { + thing := thingAtPoint(pkg, pgf, start, end) + + makeTitle := func(kind string, imp *types.Package, name string) string { + title := "Browse documentation for " + kind + " " + if imp != nil && imp != pkg.Types() { + title += imp.Name() + "." + } + return title + name + } - // A PosURLFunc forms URLs that cause the editor to navigate to a position. - PosURLFunc = func(filename string, line, col8 int) protocol.URI -) + wholePackage := func(pkg *types.Package) (PackagePath, string, string) { + // External test packages don't have /pkg doc pages, + // so instead show the doc for the package under test. + // (This named-based heuristic is imperfect.) + if forTest := strings.TrimSuffix(pkg.Path(), "_test"); forTest != pkg.Path() { + return PackagePath(forTest), "", makeTitle("package", nil, filepath.Base(forTest)) + } + + return PackagePath(pkg.Path()), "", makeTitle("package", nil, pkg.Name()) + } + + // Conceptually, we check cases in the order: + // 1. symbol + // 2. package + // 3. enclosing + // but the logic of cases 1 and 3 are identical, hence the odd factoring. + + // Imported package? + if thing.pkg != nil && thing.symbol == nil { + return wholePackage(thing.pkg) + } + + // Symbol? + var sym types.Object + if thing.symbol != nil { + sym = thing.symbol // reference to a symbol + } else if thing.enclosing != nil { + sym = thing.enclosing // selection is within a declaration of a symbol + } + if sym == nil { + return wholePackage(pkg.Types()) // no symbol + } + + // Built-in (error.Error, append or unsafe). + // TODO(adonovan): handle builtins in /pkg viewer. + if sym.Pkg() == nil { + return "", "", "" // nothing to see here + } + pkgpath = PackagePath(sym.Pkg().Path()) + + // Unexported? Show enclosing type or package. + if !sym.Exported() { + // Unexported method of exported type? + if fn, ok := sym.(*types.Func); ok { + if recv := fn.Type().(*types.Signature).Recv(); recv != nil { + _, named := typesinternal.ReceiverNamed(recv) + if named != nil && named.Obj().Exported() { + sym = named.Obj() + goto below + } + } + } -// RenderPackageDoc formats the package documentation page. + return wholePackage(sym.Pkg()) + below: + } + + // Reference to symbol in external test package? + // Short-circuit: see comment in wholePackage. + if strings.HasSuffix(string(pkgpath), "_test") { + return wholePackage(pkg.Types()) + } + + // package-level symbol? + if isPackageLevel(sym) { + return pkgpath, sym.Name(), makeTitle(objectKind(sym), sym.Pkg(), sym.Name()) + } + + // Inv: sym is field or method, or local. + switch sym := sym.(type) { + case *types.Func: // => method + sig := sym.Type().(*types.Signature) + isPtr, named := typesinternal.ReceiverNamed(sig.Recv()) + if named != nil { + if !named.Obj().Exported() { + return wholePackage(sym.Pkg()) // exported method of unexported type + } + name := fmt.Sprintf("(%s%s).%s", + strings.Repeat("*", btoi(isPtr)), // for *T + named.Obj().Name(), + sym.Name()) + fragment := named.Obj().Name() + "." + sym.Name() + return pkgpath, fragment, makeTitle("method", sym.Pkg(), name) + } + + case *types.Var: + if sym.IsField() { + // TODO(adonovan): support fields. + // The Var symbol doesn't include the struct + // type, so we need to use the logic from + // Hover. (This isn't important for + // DocFragment as fields don't have fragments, + // but it matters to the grand unification of + // Hover/Definition/DocFragment. + } + } + + // Field, non-exported method, or local declaration: + // just show current package. + return wholePackage(pkg.Types()) +} + +// thing describes the package or symbol denoted by a selection. +// +// TODO(adonovan): Hover, Definition, and References all start by +// identifying the selected object. Let's achieve a better factoring +// of the common parts using this structure, including uniform +// treatment of doc links, linkname, and suchlike. +type thing struct { + // At most one of these fields is set. + // (The 'enclosing' field is a fallback for when neither + // of the first two is set.) + symbol types.Object // referenced symbol + pkg *types.Package // referenced package + enclosing types.Object // package-level symbol or method decl enclosing selection +} + +func thingAtPoint(pkg *cache.Package, pgf *parsego.File, start, end token.Pos) thing { + path, _ := astutil.PathEnclosingInterval(pgf.File, start, end) + + // In an import spec? + if len(path) >= 3 { // [...ImportSpec GenDecl File] + if spec, ok := path[len(path)-3].(*ast.ImportSpec); ok { + if pkgname, ok := typesutil.ImportedPkgName(pkg.TypesInfo(), spec); ok { + return thing{pkg: pkgname.Imported()} + } + } + } + + // Definition or reference to symbol? + var obj types.Object + if id, ok := path[0].(*ast.Ident); ok { + obj = pkg.TypesInfo().ObjectOf(id) + + // Treat use to PkgName like ImportSpec. + if pkgname, ok := obj.(*types.PkgName); ok { + return thing{pkg: pkgname.Imported()} + } + + } else if sel, ok := path[0].(*ast.SelectorExpr); ok { + // e.g. selection is "fmt.Println" or just a portion ("mt.Prin") + obj = pkg.TypesInfo().Uses[sel.Sel] + } + if obj != nil { + return thing{symbol: obj} + } + + // Find enclosing declaration. + if n := len(path); n > 1 { + switch decl := path[n-2].(type) { + case *ast.FuncDecl: + // method? + if fn := pkg.TypesInfo().Defs[decl.Name]; fn != nil { + return thing{enclosing: fn} + } + + case *ast.GenDecl: + // path=[... Spec? GenDecl File] + for _, spec := range decl.Specs { + if n > 2 && spec == path[n-3] { + var name *ast.Ident + switch spec := spec.(type) { + case *ast.ValueSpec: + // var, const: use first name + name = spec.Names[0] + case *ast.TypeSpec: + name = spec.Name + } + if name != nil { + return thing{enclosing: pkg.TypesInfo().Defs[name]} + } + break + } + } + } + } + + return thing{} // nothing to see here +} + +// Web is an abstraction of gopls' web server. +type Web interface { + // PkgURL forms URLs of package or symbol documentation. + PkgURL(viewID string, path PackagePath, fragment string) protocol.URI + + // SrcURL forms URLs that cause the editor to open a file at a specific position. + SrcURL(filename string, line, col8 int) protocol.URI +} + +// PackageDocHTML formats the package documentation page. // // The posURL function returns a URL that when visited, has the side // effect of causing gopls to direct the client editor to navigate to // the specified file/line/column position, in UTF-8 coordinates. -// -// The pkgURL function returns a URL for the documentation of the -// specified package and symbol. -// -// TODO(adonovan): "Render" is a client-side verb; rename to PackageDocHTML. -func RenderPackageDoc(pkg *cache.Package, posURL PosURLFunc, pkgURL PkgURLFunc) ([]byte, error) { +func PackageDocHTML(viewID string, pkg *cache.Package, web Web) ([]byte, error) { // We can't use doc.NewFromFiles (even with doc.PreserveAST // mode) as it calls ast.NewPackage which assumes that each // ast.File has an ast.Scope and resolves identifiers to @@ -145,7 +343,7 @@ func RenderPackageDoc(pkg *cache.Package, posURL PosURLFunc, pkgURL PkgURLFunc) if link.Recv != "" { fragment = link.Recv + "." + link.Name } - return pkgURL(path, fragment) + return web.PkgURL(viewID, path, fragment) } parser := docpkg.Parser() parser.LookupPackage = func(name string) (importPath string, ok bool) { @@ -204,39 +402,46 @@ func RenderPackageDoc(pkg *cache.Package, posURL PosURLFunc, pkgURL PkgURLFunc) - ` + title + ` - + +
    -
    Gopls server has terminated. Page is inactive.