From 5e43887ecbb612d9b1e2c40788a48fec841f965e Mon Sep 17 00:00:00 2001 From: Alan Donovan Date: Tue, 4 Jun 2024 12:58:54 -0400 Subject: [PATCH 01/84] internal/stdlib: generate from api/next/*.txt (go1.23) too This CL causes the generator to treat the next/*.txt files as if they comprise the next (e.g. go1.23.txt) manifest, which doesn't exist until close to the release (specifically, between the start of the freeze and the first release candidate). Change-Id: I84158032484d5aa4b1d2d57c11209a24e6924b5e Reviewed-on: https://go-review.googlesource.com/c/tools/+/590355 Reviewed-by: Russ Cox LUCI-TryBot-Result: Go LUCI --- .../marker/testdata/completion/append.txt | 7 +- internal/stdlib/generate.go | 54 ++++++--- internal/stdlib/manifest.go | 111 ++++++++++++++++++ 3 files changed, 155 insertions(+), 17 deletions(-) diff --git a/gopls/internal/test/marker/testdata/completion/append.txt b/gopls/internal/test/marker/testdata/completion/append.txt index b84735bd111..96c09d7d428 100644 --- a/gopls/internal/test/marker/testdata/completion/append.txt +++ b/gopls/internal/test/marker/testdata/completion/append.txt @@ -1,7 +1,10 @@ This test checks behavior of completion within append expressions. +It requires go1.23 as the new "structs" package appears as a completion. + -- flags -- -ignore_extra_diags +-min_go=go1.23 -- go.mod -- module golang.org/lsptests/append @@ -52,5 +55,7 @@ func _() { package append func _() { - _ = append(a, struct) //@complete(")") + _ = append(a, struct) //@complete(")", structs) } + +//@item(structs, "structs", `"structs"`) diff --git a/internal/stdlib/generate.go b/internal/stdlib/generate.go index ff2691c8e60..d4964f60955 100644 --- a/internal/stdlib/generate.go +++ b/internal/stdlib/generate.go @@ -30,24 +30,12 @@ import ( ) func main() { - // Read and parse the GOROOT/api manifests. - symRE := regexp.MustCompile(`^pkg (\S+).*?, (var|func|type|const|method \([^)]*\)) ([A-Z]\w*)(.*)`) pkgs := make(map[string]map[string]symInfo) // package -> symbol -> info - for minor := 0; ; minor++ { - base := "go1.txt" - if minor > 0 { - base = fmt.Sprintf("go1.%d.txt", minor) - } - filename := filepath.Join(runtime.GOROOT(), "api", base) - data, err := os.ReadFile(filename) - if err != nil { - if errors.Is(err, fs.ErrNotExist) { - break // all caught up - } - log.Fatal(err) - } + symRE := regexp.MustCompile(`^pkg (\S+).*?, (var|func|type|const|method \([^)]*\)) ([\pL\p{Nd}_]+)(.*)`) - // parse + // parse parses symbols out of GOROOT/api/*.txt data, with the specified minor version. + // Errors are reported against filename. + parse := func(filename string, data []byte, minor int) { for linenum, line := range strings.Split(string(data), "\n") { if line == "" || strings.HasPrefix(line, "#") { continue @@ -103,6 +91,40 @@ func main() { } } + // Read and parse the GOROOT/api manifests. + for minor := 0; ; minor++ { + base := "go1.txt" + if minor > 0 { + base = fmt.Sprintf("go1.%d.txt", minor) + } + filename := filepath.Join(runtime.GOROOT(), "api", base) + data, err := os.ReadFile(filename) + if err != nil { + if errors.Is(err, fs.ErrNotExist) { + // All caught up. + // Synthesize one final file from any api/next/*.txt fragments. + // (They are consolidated into a go1.%d file some time between + // the freeze and the first release candidate.) + filenames, err := filepath.Glob(filepath.Join(runtime.GOROOT(), "api/next/*.txt")) + if err != nil { + log.Fatal(err) + } + var next bytes.Buffer + for _, filename := range filenames { + data, err := os.ReadFile(filename) + if err != nil { + log.Fatal(err) + } + next.Write(data) + } + parse(filename, next.Bytes(), minor) // (filename is a lie) + break + } + log.Fatal(err) + } + parse(filename, data, minor) + } + // The APIs of the syscall/js and unsafe packages need to be computed explicitly, // because they're not included in the GOROOT/api/go1.*.txt files at this time. pkgs["syscall/js"] = loadSymbols("syscall/js", "GOOS=js", "GOARCH=wasm") diff --git a/internal/stdlib/manifest.go b/internal/stdlib/manifest.go index fd6892075ee..a928acf29fa 100644 --- a/internal/stdlib/manifest.go +++ b/internal/stdlib/manifest.go @@ -23,6 +23,7 @@ var PackageSymbols = map[string][]Symbol{ {"ErrWriteAfterClose", Var, 0}, {"ErrWriteTooLong", Var, 0}, {"FileInfoHeader", Func, 1}, + {"FileInfoNames", Type, 23}, {"Format", Type, 10}, {"FormatGNU", Const, 10}, {"FormatPAX", Const, 10}, @@ -820,6 +821,7 @@ var PackageSymbols = map[string][]Symbol{ {"(*ConnectionState).ExportKeyingMaterial", Method, 11}, {"(*Dialer).Dial", Method, 15}, {"(*Dialer).DialContext", Method, 15}, + {"(*ECHRejectionError).Error", Method, 23}, {"(*QUICConn).Close", Method, 21}, {"(*QUICConn).ConnectionState", Method, 21}, {"(*QUICConn).HandleData", Method, 21}, @@ -827,6 +829,7 @@ var PackageSymbols = map[string][]Symbol{ {"(*QUICConn).SendSessionTicket", Method, 21}, {"(*QUICConn).SetTransportParameters", Method, 21}, {"(*QUICConn).Start", Method, 21}, + {"(*QUICConn).StoreSession", Method, 23}, {"(*SessionState).Bytes", Method, 21}, {"(AlertError).Error", Method, 21}, {"(ClientAuthType).String", Method, 15}, @@ -877,6 +880,8 @@ var PackageSymbols = map[string][]Symbol{ {"Config.ClientSessionCache", Field, 3}, {"Config.CurvePreferences", Field, 3}, {"Config.DynamicRecordSizingDisabled", Field, 7}, + {"Config.EncryptedClientHelloConfigList", Field, 23}, + {"Config.EncryptedClientHelloRejectionVerify", Field, 23}, {"Config.GetCertificate", Field, 4}, {"Config.GetClientCertificate", Field, 8}, {"Config.GetConfigForClient", Field, 8}, @@ -902,6 +907,7 @@ var PackageSymbols = map[string][]Symbol{ {"ConnectionState", Type, 0}, {"ConnectionState.CipherSuite", Field, 0}, {"ConnectionState.DidResume", Field, 1}, + {"ConnectionState.ECHAccepted", Field, 23}, {"ConnectionState.HandshakeComplete", Field, 0}, {"ConnectionState.NegotiatedProtocol", Field, 0}, {"ConnectionState.NegotiatedProtocolIsMutual", Field, 0}, @@ -925,6 +931,8 @@ var PackageSymbols = map[string][]Symbol{ {"ECDSAWithP384AndSHA384", Const, 8}, {"ECDSAWithP521AndSHA512", Const, 8}, {"ECDSAWithSHA1", Const, 10}, + {"ECHRejectionError", Type, 23}, + {"ECHRejectionError.RetryConfigList", Field, 23}, {"Ed25519", Const, 13}, {"InsecureCipherSuites", Func, 14}, {"Listen", Func, 0}, @@ -943,6 +951,7 @@ var PackageSymbols = map[string][]Symbol{ {"ParseSessionState", Func, 21}, {"QUICClient", Func, 21}, {"QUICConfig", Type, 21}, + {"QUICConfig.EnableStoreSessionEvent", Field, 23}, {"QUICConfig.TLSConfig", Field, 21}, {"QUICConn", Type, 21}, {"QUICEncryptionLevel", Type, 21}, @@ -954,16 +963,20 @@ var PackageSymbols = map[string][]Symbol{ {"QUICEvent.Data", Field, 21}, {"QUICEvent.Kind", Field, 21}, {"QUICEvent.Level", Field, 21}, + {"QUICEvent.SessionState", Field, 23}, {"QUICEvent.Suite", Field, 21}, {"QUICEventKind", Type, 21}, {"QUICHandshakeDone", Const, 21}, {"QUICNoEvent", Const, 21}, {"QUICRejectedEarlyData", Const, 21}, + {"QUICResumeSession", Const, 23}, {"QUICServer", Func, 21}, {"QUICSessionTicketOptions", Type, 21}, {"QUICSessionTicketOptions.EarlyData", Field, 21}, + {"QUICSessionTicketOptions.Extra", Field, 23}, {"QUICSetReadSecret", Const, 21}, {"QUICSetWriteSecret", Const, 21}, + {"QUICStoreSession", Const, 23}, {"QUICTransportParameters", Const, 21}, {"QUICTransportParametersRequired", Const, 21}, {"QUICWriteData", Const, 21}, @@ -1036,6 +1049,8 @@ var PackageSymbols = map[string][]Symbol{ {"(*Certificate).Verify", Method, 0}, {"(*Certificate).VerifyHostname", Method, 0}, {"(*CertificateRequest).CheckSignature", Method, 5}, + {"(*OID).UnmarshalBinary", Method, 23}, + {"(*OID).UnmarshalText", Method, 23}, {"(*RevocationList).CheckSignatureFrom", Method, 19}, {"(CertificateInvalidError).Error", Method, 0}, {"(ConstraintViolationError).Error", Method, 0}, @@ -1043,6 +1058,8 @@ var PackageSymbols = map[string][]Symbol{ {"(InsecureAlgorithmError).Error", Method, 6}, {"(OID).Equal", Method, 22}, {"(OID).EqualASN1OID", Method, 22}, + {"(OID).MarshalBinary", Method, 23}, + {"(OID).MarshalText", Method, 23}, {"(OID).String", Method, 22}, {"(PublicKeyAlgorithm).String", Method, 10}, {"(SignatureAlgorithm).String", Method, 6}, @@ -1196,6 +1213,7 @@ var PackageSymbols = map[string][]Symbol{ {"ParseCertificates", Func, 0}, {"ParseDERCRL", Func, 0}, {"ParseECPrivateKey", Func, 1}, + {"ParseOID", Func, 23}, {"ParsePKCS1PrivateKey", Func, 0}, {"ParsePKCS1PublicKey", Func, 10}, {"ParsePKCS8PrivateKey", Func, 0}, @@ -2541,6 +2559,7 @@ var PackageSymbols = map[string][]Symbol{ {"PT_NOTE", Const, 0}, {"PT_NULL", Const, 0}, {"PT_OPENBSD_BOOTDATA", Const, 16}, + {"PT_OPENBSD_NOBTCFI", Const, 23}, {"PT_OPENBSD_RANDOMIZE", Const, 16}, {"PT_OPENBSD_WXNEEDED", Const, 16}, {"PT_PAX_FLAGS", Const, 16}, @@ -3620,13 +3639,16 @@ var PackageSymbols = map[string][]Symbol{ {"STT_COMMON", Const, 0}, {"STT_FILE", Const, 0}, {"STT_FUNC", Const, 0}, + {"STT_GNU_IFUNC", Const, 23}, {"STT_HIOS", Const, 0}, {"STT_HIPROC", Const, 0}, {"STT_LOOS", Const, 0}, {"STT_LOPROC", Const, 0}, {"STT_NOTYPE", Const, 0}, {"STT_OBJECT", Const, 0}, + {"STT_RELC", Const, 23}, {"STT_SECTION", Const, 0}, + {"STT_SRELC", Const, 23}, {"STT_TLS", Const, 0}, {"STV_DEFAULT", Const, 0}, {"STV_HIDDEN", Const, 0}, @@ -4544,11 +4566,14 @@ var PackageSymbols = map[string][]Symbol{ {"URLEncoding", Var, 0}, }, "encoding/binary": { + {"Append", Func, 23}, {"AppendByteOrder", Type, 19}, {"AppendUvarint", Func, 19}, {"AppendVarint", Func, 19}, {"BigEndian", Var, 0}, {"ByteOrder", Type, 0}, + {"Decode", Func, 23}, + {"Encode", Func, 23}, {"LittleEndian", Var, 0}, {"MaxVarintLen16", Const, 0}, {"MaxVarintLen32", Const, 0}, @@ -5308,6 +5333,7 @@ var PackageSymbols = map[string][]Symbol{ {"ParenExpr.Rparen", Field, 0}, {"ParenExpr.X", Field, 0}, {"Pkg", Const, 0}, + {"Preorder", Func, 23}, {"Print", Func, 0}, {"RECV", Const, 0}, {"RangeStmt", Type, 0}, @@ -5898,7 +5924,12 @@ var PackageSymbols = map[string][]Symbol{ }, "go/types": { {"(*Alias).Obj", Method, 22}, + {"(*Alias).Origin", Method, 23}, + {"(*Alias).Rhs", Method, 23}, + {"(*Alias).SetTypeParams", Method, 23}, {"(*Alias).String", Method, 22}, + {"(*Alias).TypeArgs", Method, 23}, + {"(*Alias).TypeParams", Method, 23}, {"(*Alias).Underlying", Method, 22}, {"(*ArgumentError).Error", Method, 18}, {"(*ArgumentError).Unwrap", Method, 18}, @@ -5943,6 +5974,7 @@ var PackageSymbols = map[string][]Symbol{ {"(*Func).Pkg", Method, 5}, {"(*Func).Pos", Method, 5}, {"(*Func).Scope", Method, 5}, + {"(*Func).Signature", Method, 23}, {"(*Func).String", Method, 5}, {"(*Func).Type", Method, 5}, {"(*Info).ObjectOf", Method, 5}, @@ -6992,6 +7024,12 @@ var PackageSymbols = map[string][]Symbol{ {"TempFile", Func, 0}, {"WriteFile", Func, 0}, }, + "iter": { + {"Pull", Func, 23}, + {"Pull2", Func, 23}, + {"Seq", Type, 23}, + {"Seq2", Type, 23}, + }, "log": { {"(*Logger).Fatal", Method, 0}, {"(*Logger).Fatalf", Method, 0}, @@ -7222,11 +7260,16 @@ var PackageSymbols = map[string][]Symbol{ {"Writer", Type, 0}, }, "maps": { + {"All", Func, 23}, {"Clone", Func, 21}, + {"Collect", Func, 23}, {"Copy", Func, 21}, {"DeleteFunc", Func, 21}, {"Equal", Func, 21}, {"EqualFunc", Func, 21}, + {"Insert", Func, 23}, + {"Keys", Func, 23}, + {"Values", Func, 23}, }, "math": { {"Abs", Func, 0}, @@ -7617,6 +7660,7 @@ var PackageSymbols = map[string][]Symbol{ }, "math/rand/v2": { {"(*ChaCha8).MarshalBinary", Method, 22}, + {"(*ChaCha8).Read", Method, 23}, {"(*ChaCha8).Seed", Method, 22}, {"(*ChaCha8).Uint64", Method, 22}, {"(*ChaCha8).UnmarshalBinary", Method, 22}, @@ -7636,6 +7680,7 @@ var PackageSymbols = map[string][]Symbol{ {"(*Rand).NormFloat64", Method, 22}, {"(*Rand).Perm", Method, 22}, {"(*Rand).Shuffle", Method, 22}, + {"(*Rand).Uint", Method, 23}, {"(*Rand).Uint32", Method, 22}, {"(*Rand).Uint32N", Method, 22}, {"(*Rand).Uint64", Method, 22}, @@ -7663,6 +7708,7 @@ var PackageSymbols = map[string][]Symbol{ {"Rand", Type, 22}, {"Shuffle", Func, 22}, {"Source", Type, 22}, + {"Uint", Func, 23}, {"Uint32", Func, 22}, {"Uint32N", Func, 22}, {"Uint64", Func, 22}, @@ -7743,6 +7789,7 @@ var PackageSymbols = map[string][]Symbol{ {"(*DNSError).Error", Method, 0}, {"(*DNSError).Temporary", Method, 0}, {"(*DNSError).Timeout", Method, 0}, + {"(*DNSError).Unwrap", Method, 23}, {"(*Dialer).Dial", Method, 1}, {"(*Dialer).DialContext", Method, 7}, {"(*Dialer).MultipathTCP", Method, 21}, @@ -7809,6 +7856,7 @@ var PackageSymbols = map[string][]Symbol{ {"(*TCPConn).RemoteAddr", Method, 0}, {"(*TCPConn).SetDeadline", Method, 0}, {"(*TCPConn).SetKeepAlive", Method, 0}, + {"(*TCPConn).SetKeepAliveConfig", Method, 23}, {"(*TCPConn).SetKeepAlivePeriod", Method, 2}, {"(*TCPConn).SetLinger", Method, 0}, {"(*TCPConn).SetNoDelay", Method, 0}, @@ -7922,6 +7970,7 @@ var PackageSymbols = map[string][]Symbol{ {"DNSError.IsTimeout", Field, 0}, {"DNSError.Name", Field, 0}, {"DNSError.Server", Field, 0}, + {"DNSError.UnwrapErr", Field, 23}, {"DefaultResolver", Var, 8}, {"Dial", Func, 0}, {"DialIP", Func, 0}, @@ -7937,6 +7986,7 @@ var PackageSymbols = map[string][]Symbol{ {"Dialer.DualStack", Field, 2}, {"Dialer.FallbackDelay", Field, 5}, {"Dialer.KeepAlive", Field, 3}, + {"Dialer.KeepAliveConfig", Field, 23}, {"Dialer.LocalAddr", Field, 1}, {"Dialer.Resolver", Field, 8}, {"Dialer.Timeout", Field, 1}, @@ -7989,10 +8039,16 @@ var PackageSymbols = map[string][]Symbol{ {"Interfaces", Func, 0}, {"InvalidAddrError", Type, 0}, {"JoinHostPort", Func, 0}, + {"KeepAliveConfig", Type, 23}, + {"KeepAliveConfig.Count", Field, 23}, + {"KeepAliveConfig.Enable", Field, 23}, + {"KeepAliveConfig.Idle", Field, 23}, + {"KeepAliveConfig.Interval", Field, 23}, {"Listen", Func, 0}, {"ListenConfig", Type, 11}, {"ListenConfig.Control", Field, 11}, {"ListenConfig.KeepAlive", Field, 13}, + {"ListenConfig.KeepAliveConfig", Field, 23}, {"ListenIP", Func, 0}, {"ListenMulticastUDP", Func, 0}, {"ListenPacket", Func, 0}, @@ -8081,6 +8137,7 @@ var PackageSymbols = map[string][]Symbol{ {"(*Request).Context", Method, 7}, {"(*Request).Cookie", Method, 0}, {"(*Request).Cookies", Method, 0}, + {"(*Request).CookiesNamed", Method, 23}, {"(*Request).FormFile", Method, 0}, {"(*Request).FormValue", Method, 0}, {"(*Request).MultipartReader", Method, 0}, @@ -8148,7 +8205,9 @@ var PackageSymbols = map[string][]Symbol{ {"Cookie.HttpOnly", Field, 0}, {"Cookie.MaxAge", Field, 0}, {"Cookie.Name", Field, 0}, + {"Cookie.Partitioned", Field, 23}, {"Cookie.Path", Field, 0}, + {"Cookie.Quoted", Field, 23}, {"Cookie.Raw", Field, 0}, {"Cookie.RawExpires", Field, 0}, {"Cookie.SameSite", Field, 11}, @@ -8225,7 +8284,9 @@ var PackageSymbols = map[string][]Symbol{ {"NoBody", Var, 8}, {"NotFound", Func, 0}, {"NotFoundHandler", Func, 0}, + {"ParseCookie", Func, 23}, {"ParseHTTPVersion", Func, 0}, + {"ParseSetCookie", Func, 23}, {"ParseTime", Func, 1}, {"Post", Func, 0}, {"PostForm", Func, 0}, @@ -8252,6 +8313,7 @@ var PackageSymbols = map[string][]Symbol{ {"Request.Host", Field, 0}, {"Request.Method", Field, 0}, {"Request.MultipartForm", Field, 0}, + {"Request.Pattern", Field, 23}, {"Request.PostForm", Field, 1}, {"Request.Proto", Field, 0}, {"Request.ProtoMajor", Field, 0}, @@ -8453,6 +8515,7 @@ var PackageSymbols = map[string][]Symbol{ {"DefaultRemoteAddr", Const, 0}, {"NewRecorder", Func, 0}, {"NewRequest", Func, 7}, + {"NewRequestWithContext", Func, 23}, {"NewServer", Func, 0}, {"NewTLSServer", Func, 0}, {"NewUnstartedServer", Func, 0}, @@ -8917,6 +8980,7 @@ var PackageSymbols = map[string][]Symbol{ {"Chown", Func, 0}, {"Chtimes", Func, 0}, {"Clearenv", Func, 0}, + {"CopyFS", Func, 23}, {"Create", Func, 0}, {"CreateTemp", Func, 16}, {"DevNull", Const, 0}, @@ -9150,6 +9214,7 @@ var PackageSymbols = map[string][]Symbol{ {"IsLocal", Func, 20}, {"Join", Func, 0}, {"ListSeparator", Const, 0}, + {"Localize", Func, 23}, {"Match", Func, 0}, {"Rel", Func, 0}, {"Separator", Const, 0}, @@ -9232,6 +9297,8 @@ var PackageSymbols = map[string][]Symbol{ {"(Value).Pointer", Method, 0}, {"(Value).Recv", Method, 0}, {"(Value).Send", Method, 0}, + {"(Value).Seq", Method, 23}, + {"(Value).Seq2", Method, 23}, {"(Value).Set", Method, 0}, {"(Value).SetBool", Method, 0}, {"(Value).SetBytes", Method, 0}, @@ -9314,6 +9381,7 @@ var PackageSymbols = map[string][]Symbol{ {"SelectSend", Const, 1}, {"SendDir", Const, 0}, {"Slice", Const, 0}, + {"SliceAt", Func, 23}, {"SliceHeader", Type, 0}, {"SliceHeader.Cap", Field, 0}, {"SliceHeader.Data", Field, 0}, @@ -9655,6 +9723,7 @@ var PackageSymbols = map[string][]Symbol{ {"BuildSetting", Type, 18}, {"BuildSetting.Key", Field, 18}, {"BuildSetting.Value", Field, 18}, + {"CrashOptions", Type, 23}, {"FreeOSMemory", Func, 1}, {"GCStats", Type, 1}, {"GCStats.LastGC", Field, 1}, @@ -9672,6 +9741,7 @@ var PackageSymbols = map[string][]Symbol{ {"PrintStack", Func, 0}, {"ReadBuildInfo", Func, 12}, {"ReadGCStats", Func, 1}, + {"SetCrashOutput", Func, 23}, {"SetGCPercent", Func, 1}, {"SetMaxStack", Func, 2}, {"SetMaxThreads", Func, 2}, @@ -9742,10 +9812,15 @@ var PackageSymbols = map[string][]Symbol{ {"WithRegion", Func, 11}, }, "slices": { + {"All", Func, 23}, + {"AppendSeq", Func, 23}, + {"Backward", Func, 23}, {"BinarySearch", Func, 21}, {"BinarySearchFunc", Func, 21}, + {"Chunk", Func, 23}, {"Clip", Func, 21}, {"Clone", Func, 21}, + {"Collect", Func, 23}, {"Compact", Func, 21}, {"CompactFunc", Func, 21}, {"Compare", Func, 21}, @@ -9767,11 +9842,16 @@ var PackageSymbols = map[string][]Symbol{ {"MaxFunc", Func, 21}, {"Min", Func, 21}, {"MinFunc", Func, 21}, + {"Repeat", Func, 23}, {"Replace", Func, 21}, {"Reverse", Func, 21}, {"Sort", Func, 21}, {"SortFunc", Func, 21}, {"SortStableFunc", Func, 21}, + {"Sorted", Func, 23}, + {"SortedFunc", Func, 23}, + {"SortedStableFunc", Func, 23}, + {"Values", Func, 23}, }, "sort": { {"(Float64Slice).Len", Method, 0}, @@ -9936,10 +10016,14 @@ var PackageSymbols = map[string][]Symbol{ {"TrimSpace", Func, 0}, {"TrimSuffix", Func, 1}, }, + "structs": { + {"HostLayout", Type, 23}, + }, "sync": { {"(*Cond).Broadcast", Method, 0}, {"(*Cond).Signal", Method, 0}, {"(*Cond).Wait", Method, 0}, + {"(*Map).Clear", Method, 23}, {"(*Map).CompareAndDelete", Method, 20}, {"(*Map).CompareAndSwap", Method, 20}, {"(*Map).Delete", Method, 9}, @@ -9986,13 +10070,17 @@ var PackageSymbols = map[string][]Symbol{ {"(*Bool).Store", Method, 19}, {"(*Bool).Swap", Method, 19}, {"(*Int32).Add", Method, 19}, + {"(*Int32).And", Method, 23}, {"(*Int32).CompareAndSwap", Method, 19}, {"(*Int32).Load", Method, 19}, + {"(*Int32).Or", Method, 23}, {"(*Int32).Store", Method, 19}, {"(*Int32).Swap", Method, 19}, {"(*Int64).Add", Method, 19}, + {"(*Int64).And", Method, 23}, {"(*Int64).CompareAndSwap", Method, 19}, {"(*Int64).Load", Method, 19}, + {"(*Int64).Or", Method, 23}, {"(*Int64).Store", Method, 19}, {"(*Int64).Swap", Method, 19}, {"(*Pointer).CompareAndSwap", Method, 19}, @@ -10000,18 +10088,24 @@ var PackageSymbols = map[string][]Symbol{ {"(*Pointer).Store", Method, 19}, {"(*Pointer).Swap", Method, 19}, {"(*Uint32).Add", Method, 19}, + {"(*Uint32).And", Method, 23}, {"(*Uint32).CompareAndSwap", Method, 19}, {"(*Uint32).Load", Method, 19}, + {"(*Uint32).Or", Method, 23}, {"(*Uint32).Store", Method, 19}, {"(*Uint32).Swap", Method, 19}, {"(*Uint64).Add", Method, 19}, + {"(*Uint64).And", Method, 23}, {"(*Uint64).CompareAndSwap", Method, 19}, {"(*Uint64).Load", Method, 19}, + {"(*Uint64).Or", Method, 23}, {"(*Uint64).Store", Method, 19}, {"(*Uint64).Swap", Method, 19}, {"(*Uintptr).Add", Method, 19}, + {"(*Uintptr).And", Method, 23}, {"(*Uintptr).CompareAndSwap", Method, 19}, {"(*Uintptr).Load", Method, 19}, + {"(*Uintptr).Or", Method, 23}, {"(*Uintptr).Store", Method, 19}, {"(*Uintptr).Swap", Method, 19}, {"(*Value).CompareAndSwap", Method, 17}, @@ -10023,6 +10117,11 @@ var PackageSymbols = map[string][]Symbol{ {"AddUint32", Func, 0}, {"AddUint64", Func, 0}, {"AddUintptr", Func, 0}, + {"AndInt32", Func, 23}, + {"AndInt64", Func, 23}, + {"AndUint32", Func, 23}, + {"AndUint64", Func, 23}, + {"AndUintptr", Func, 23}, {"Bool", Type, 19}, {"CompareAndSwapInt32", Func, 0}, {"CompareAndSwapInt64", Func, 0}, @@ -10038,6 +10137,11 @@ var PackageSymbols = map[string][]Symbol{ {"LoadUint32", Func, 0}, {"LoadUint64", Func, 0}, {"LoadUintptr", Func, 0}, + {"OrInt32", Func, 23}, + {"OrInt64", Func, 23}, + {"OrUint32", Func, 23}, + {"OrUint64", Func, 23}, + {"OrUintptr", Func, 23}, {"Pointer", Type, 19}, {"StoreInt32", Func, 0}, {"StoreInt64", Func, 0}, @@ -16200,6 +16304,7 @@ var PackageSymbols = map[string][]Symbol{ {"WSAEACCES", Const, 2}, {"WSAECONNABORTED", Const, 9}, {"WSAECONNRESET", Const, 3}, + {"WSAENOPROTOOPT", Const, 23}, {"WSAEnumProtocols", Func, 2}, {"WSAID_CONNECTEX", Var, 1}, {"WSAIoctl", Func, 0}, @@ -17284,6 +17389,7 @@ var PackageSymbols = map[string][]Symbol{ {"Encode", Func, 0}, {"EncodeRune", Func, 0}, {"IsSurrogate", Func, 0}, + {"RuneLen", Func, 23}, }, "unicode/utf8": { {"AppendRune", Func, 18}, @@ -17306,6 +17412,11 @@ var PackageSymbols = map[string][]Symbol{ {"ValidRune", Func, 1}, {"ValidString", Func, 0}, }, + "unique": { + {"(Handle).Value", Method, 23}, + {"Handle", Type, 23}, + {"Make", Func, 23}, + }, "unsafe": { {"Add", Func, 0}, {"Alignof", Func, 0}, From 7522327277f7694ed5003a0f7a3d9409e73fa3da Mon Sep 17 00:00:00 2001 From: toad <530901331qq@gmail.com> Date: Fri, 17 May 2024 14:56:35 +0800 Subject: [PATCH 02/84] gopls/rename: Fix spurious package name conflicts. Fixes golang/go#67069 Change-Id: I537308c8941a5f2f2c6e10791e75b529574d170b Reviewed-on: https://go-review.googlesource.com/c/tools/+/586336 Reviewed-by: Robert Findley Reviewed-by: Alan Donovan LUCI-TryBot-Result: Go LUCI Auto-Submit: Alan Donovan --- gopls/internal/golang/rename_check.go | 25 ++++----- .../marker/testdata/rename/issue67069.txt | 54 +++++++++++++++++++ 2 files changed, 67 insertions(+), 12 deletions(-) create mode 100644 gopls/internal/test/marker/testdata/rename/issue67069.txt diff --git a/gopls/internal/golang/rename_check.go b/gopls/internal/golang/rename_check.go index e63b745c1ad..497f1a09ca2 100644 --- a/gopls/internal/golang/rename_check.go +++ b/gopls/internal/golang/rename_check.go @@ -167,18 +167,20 @@ func (r *renamer) checkInPackageBlock(from types.Object) { } } - // Check for conflicts between package block and all file blocks. - for _, f := range r.pkg.Syntax() { - fileScope := r.pkg.TypesInfo().Scopes[f] - b, prev := fileScope.LookupParent(r.to, token.NoPos) - if b == fileScope { - r.errorf(from.Pos(), "renaming this %s %q to %q would conflict", objectKind(from), from.Name(), r.to) - var prevPos token.Pos - if prev != nil { - prevPos = prev.Pos() + // In the declaring package, check for conflicts between the + // package block and all file blocks. + if from.Pkg() == r.pkg.Types() { + for _, f := range r.pkg.Syntax() { + fileScope := r.pkg.TypesInfo().Scopes[f] + if fileScope == nil { + continue // type error? (golang/go#40835) + } + b, prev := fileScope.LookupParent(r.to, token.NoPos) + if b == fileScope { + r.errorf(from.Pos(), "renaming this %s %q to %q would conflict", objectKind(from), from.Name(), r.to) + r.errorf(prev.Pos(), "\twith this %s", objectKind(prev)) + return // since checkInPackageBlock would report redundant errors } - r.errorf(prevPos, "\twith this %s", objectKind(prev)) - return // since checkInPackageBlock would report redundant errors } } @@ -436,7 +438,6 @@ func (r *renamer) checkLabel(label *types.Label) { // checkStructField checks that the field renaming will not cause // conflicts at its declaration, or ambiguity or changes to any selection. func (r *renamer) checkStructField(from *types.Var) { - // If this is the declaring package, check that the struct // declaration is free of field conflicts, and field/method // conflicts. diff --git a/gopls/internal/test/marker/testdata/rename/issue67069.txt b/gopls/internal/test/marker/testdata/rename/issue67069.txt new file mode 100644 index 00000000000..2656de16970 --- /dev/null +++ b/gopls/internal/test/marker/testdata/rename/issue67069.txt @@ -0,0 +1,54 @@ +This test verifies spurious pkgname conflicts. +Issue golang/go#67069. + +-- go.mod -- +module example +go 1.19 + +-- aa/a.go -- +package aa + +var cc int //@rename("cc", "aa", CToA) +const C = 0 +const D = 0 + +-- aa/a_test.go -- +package aa_test + +import "example/aa" + +var _ = aa.C //@rename("aa", "bb", AToB) +-- @CToA/aa/a.go -- +@@ -3 +3 @@ +-var cc int //@rename("cc", "aa", CToA) ++var aa int //@rename("cc", "aa", CToA) +-- @AToB/aa/a_test.go -- +@@ -3 +3 @@ +-import "example/aa" ++import bb "example/aa" +@@ -5 +5 @@ +-var _ = aa.C //@rename("aa", "bb", AToB) ++var _ = bb.C //@rename("aa", "bb", AToB) +-- bb/b.go -- +package bb + +import "example/aa" + +var _ = aa.C +var bb int //@renameerr("bb", "aa", errImportConflict) + +-- @errImportConflict -- +bb/b.go:6:5: renaming this var "bb" to "aa" would conflict +bb/b.go:3:8: with this imported package name +-- aa/a_internal_test.go -- +package aa + +var _ = D //@rename("D", "aa", DToA) +-- @DToA/aa/a_internal_test.go -- +@@ -3 +3 @@ +-var _ = D //@rename("D", "aa", DToA) ++var _ = aa //@rename("D", "aa", DToA) +-- @DToA/aa/a.go -- +@@ -5 +5 @@ +-const D = 0 ++const aa = 0 From 8bf61b85c2e3f01e1affa5893c1f3ad9bc5bc66e Mon Sep 17 00:00:00 2001 From: Rob Findley Date: Tue, 4 Jun 2024 21:30:49 +0000 Subject: [PATCH 03/84] gopls/internal/cache: fix module resolver cache refreshing An apparent bad merge in CL 561235 caused the critical component-- clearing the resolver for a new scan--to be dropped. Fix this, so that the imports state is actually refreshed asynchronously. It is surprising that this was not reported, though I see perhaps two related comments in survey results. Most likely adding a new import is infrequent enough that users were simply confused, or learned to restart gopls (alas). Also, add more instrumentation that would help debug golang/go#67289. For golang/go#67289 Change-Id: I50d70a470eb393caf9e0b41856997942372b216f Reviewed-on: https://go-review.googlesource.com/c/tools/+/590377 Auto-Submit: Robert Findley Reviewed-by: Alan Donovan LUCI-TryBot-Result: Go LUCI --- gopls/internal/cache/imports.go | 10 ++++++---- internal/gocommand/invoke.go | 17 ++++++++++++----- internal/imports/fix.go | 34 ++++++++++++++++----------------- internal/imports/mod.go | 8 ++------ 4 files changed, 36 insertions(+), 33 deletions(-) diff --git a/gopls/internal/cache/imports.go b/gopls/internal/cache/imports.go index dd58ef566ec..76a5855fe87 100644 --- a/gopls/internal/cache/imports.go +++ b/gopls/internal/cache/imports.go @@ -119,8 +119,7 @@ type importsState struct { // newImportsState constructs a new imports state for running goimports // functions via [runProcessEnvFunc]. // -// The returned state will automatically refresh itself following a call to -// runProcessEnvFunc. +// The returned state will automatically refresh itself following a delay. func newImportsState(backgroundCtx context.Context, modCache *sharedModCache, env *imports.ProcessEnv) *importsState { s := &importsState{ ctx: backgroundCtx, @@ -128,6 +127,7 @@ func newImportsState(backgroundCtx context.Context, modCache *sharedModCache, en processEnv: env, } s.refreshTimer = newRefreshTimer(s.refreshProcessEnv) + s.refreshTimer.schedule() return s } @@ -210,20 +210,22 @@ func (s *importsState) refreshProcessEnv() { resolver, err := s.processEnv.GetResolver() s.mu.Unlock() if err != nil { + event.Error(s.ctx, "failed to get import resolver", err) return } event.Log(s.ctx, "background imports cache refresh starting") + resolver2 := resolver.ClearForNewScan() // Prime the new resolver before updating the processEnv, so that gopls // doesn't wait on an unprimed cache. - if err := imports.PrimeCache(context.Background(), resolver); err == nil { + if err := imports.PrimeCache(context.Background(), resolver2); err == nil { event.Log(ctx, fmt.Sprintf("background refresh finished after %v", time.Since(start))) } else { event.Log(ctx, fmt.Sprintf("background refresh finished after %v", time.Since(start)), keys.Err.Of(err)) } s.mu.Lock() - s.processEnv.UpdateResolver(resolver) + s.processEnv.UpdateResolver(resolver2) s.mu.Unlock() } diff --git a/internal/gocommand/invoke.go b/internal/gocommand/invoke.go index af0ee6c614d..804d5921faa 100644 --- a/internal/gocommand/invoke.go +++ b/internal/gocommand/invoke.go @@ -200,12 +200,14 @@ func (i *Invocation) runWithFriendlyError(ctx context.Context, stdout, stderr io return } -func (i *Invocation) run(ctx context.Context, stdout, stderr io.Writer) error { - log := i.Logf - if log == nil { - log = func(string, ...interface{}) {} +// logf logs if i.Logf is non-nil. +func (i *Invocation) logf(format string, args ...any) { + if i.Logf != nil { + i.Logf(format, args...) } +} +func (i *Invocation) run(ctx context.Context, stdout, stderr io.Writer) error { goArgs := []string{i.Verb} appendModFile := func() { @@ -277,7 +279,12 @@ func (i *Invocation) run(ctx context.Context, stdout, stderr io.Writer) error { cmd.Dir = i.WorkingDir } - defer func(start time.Time) { log("%s for %v", time.Since(start), cmdDebugStr(cmd)) }(time.Now()) + debugStr := cmdDebugStr(cmd) + i.logf("starting %v", debugStr) + start := time.Now() + defer func() { + i.logf("%s for %v", time.Since(start), debugStr) + }() return runCmdContext(ctx, cmd) } diff --git a/internal/imports/fix.go b/internal/imports/fix.go index 4569313a089..882dd42b549 100644 --- a/internal/imports/fix.go +++ b/internal/imports/fix.go @@ -365,9 +365,7 @@ func (p *pass) load() ([]*ImportFix, bool) { if p.loadRealPackageNames { err := p.loadPackageNames(append(imports, p.candidates...)) if err != nil { - if p.env.Logf != nil { - p.env.Logf("loading package names: %v", err) - } + p.env.logf("loading package names: %v", err) return nil, false } } @@ -580,9 +578,7 @@ func getFixes(ctx context.Context, fset *token.FileSet, f *ast.File, filename st return nil, err } srcDir := filepath.Dir(abs) - if env.Logf != nil { - env.Logf("fixImports(filename=%q), abs=%q, srcDir=%q ...", filename, abs, srcDir) - } + env.logf("fixImports(filename=%q), abs=%q, srcDir=%q ...", filename, abs, srcDir) // First pass: looking only at f, and using the naive algorithm to // derive package names from import paths, see if the file is already @@ -1014,16 +1010,26 @@ func (e *ProcessEnv) GetResolver() (Resolver, error) { // already know the view type. if len(e.Env["GOMOD"]) == 0 && len(e.Env["GOWORK"]) == 0 { e.resolver = newGopathResolver(e) + e.logf("created gopath resolver") } else if r, err := newModuleResolver(e, e.ModCache); err != nil { e.resolverErr = err + e.logf("failed to create module resolver: %v", err) } else { e.resolver = Resolver(r) + e.logf("created module resolver") } } return e.resolver, e.resolverErr } +// logf logs if e.Logf is non-nil. +func (e *ProcessEnv) logf(format string, args ...any) { + if e.Logf != nil { + e.Logf(format, args...) + } +} + // buildContext returns the build.Context to use for matching files. // // TODO(rfindley): support dynamic GOOS, GOARCH here, when doing cross-platform @@ -1610,9 +1616,7 @@ func loadExportsFromFiles(ctx context.Context, env *ProcessEnv, dir string, incl fullFile := filepath.Join(dir, fi.Name()) f, err := parser.ParseFile(fset, fullFile, nil, 0) if err != nil { - if env.Logf != nil { - env.Logf("error parsing %v: %v", fullFile, err) - } + env.logf("error parsing %v: %v", fullFile, err) continue } if f.Name.Name == "documentation" { @@ -1648,9 +1652,7 @@ func loadExportsFromFiles(ctx context.Context, env *ProcessEnv, dir string, incl } sortSymbols(exports) - if env.Logf != nil { - env.Logf("loaded exports in dir %v (package %v): %v", dir, pkgName, exports) - } + env.logf("loaded exports in dir %v (package %v): %v", dir, pkgName, exports) return pkgName, exports, nil } @@ -1710,16 +1712,12 @@ func findImport(ctx context.Context, pass *pass, candidates []pkgDistance, pkgNa wg.Done() }() - if pass.env.Logf != nil { - pass.env.Logf("loading exports in dir %s (seeking package %s)", c.pkg.dir, pkgName) - } + pass.env.logf("loading exports in dir %s (seeking package %s)", c.pkg.dir, pkgName) // If we're an x_test, load the package under test's test variant. includeTest := strings.HasSuffix(pass.f.Name.Name, "_test") && c.pkg.dir == pass.srcDir _, exports, err := resolver.loadExports(ctx, c.pkg, includeTest) if err != nil { - if pass.env.Logf != nil { - pass.env.Logf("loading exports in dir %s (seeking package %s): %v", c.pkg.dir, pkgName, err) - } + pass.env.logf("loading exports in dir %s (seeking package %s): %v", c.pkg.dir, pkgName, err) resc <- nil return } diff --git a/internal/imports/mod.go b/internal/imports/mod.go index 82fe644a189..91221fda322 100644 --- a/internal/imports/mod.go +++ b/internal/imports/mod.go @@ -265,9 +265,7 @@ func (r *ModuleResolver) initAllMods() error { return err } if mod.Dir == "" { - if r.env.Logf != nil { - r.env.Logf("module %v has not been downloaded and will be ignored", mod.Path) - } + r.env.logf("module %v has not been downloaded and will be ignored", mod.Path) // Can't do anything with a module that's not downloaded. continue } @@ -766,9 +764,7 @@ func (r *ModuleResolver) scanDirForPackage(root gopathwalk.Root, dir string) dir } modPath, err := module.UnescapePath(filepath.ToSlash(matches[1])) if err != nil { - if r.env.Logf != nil { - r.env.Logf("decoding module cache path %q: %v", subdir, err) - } + r.env.logf("decoding module cache path %q: %v", subdir, err) return directoryPackageInfo{ status: directoryScanned, err: fmt.Errorf("decoding module cache path %q: %v", subdir, err), From d9366dd1461f1a0f558c18121450eef40919f94c Mon Sep 17 00:00:00 2001 From: Michael Stapelberg Date: Thu, 6 Jun 2024 14:58:27 +0200 Subject: [PATCH 04/84] internal/imports: restore fixImports extension point http://go.dev/cl/589975 incorrectly made fixImports a regular function because the extension was not visible in the tools repository. Google-internally, we do use this extension point. Change-Id: I7ff41e49f852e0f72671141a7a2baa272ec5a6ce Reviewed-on: https://go-review.googlesource.com/c/tools/+/591035 Reviewed-by: Robert Findley LUCI-TryBot-Result: Go LUCI Auto-Submit: Michael Stapelberg --- internal/imports/fix.go | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/internal/imports/fix.go b/internal/imports/fix.go index 882dd42b549..31f8d1a9933 100644 --- a/internal/imports/fix.go +++ b/internal/imports/fix.go @@ -561,7 +561,12 @@ func (p *pass) addCandidate(imp *ImportInfo, pkg *packageInfo) { // fixImports adds and removes imports from f so that all its references are // satisfied and there are no unused imports. -func fixImports(fset *token.FileSet, f *ast.File, filename string, env *ProcessEnv) error { +// +// This is declared as a variable rather than a function so goimports can +// easily be extended by adding a file with an init function. +var fixImports = fixImportsDefault + +func fixImportsDefault(fset *token.FileSet, f *ast.File, filename string, env *ProcessEnv) error { fixes, err := getFixes(context.Background(), fset, f, filename, env) if err != nil { return err From 1c73966ad4fd42f6a81b0cdbe4ef0e78b302283d Mon Sep 17 00:00:00 2001 From: Rob Findley Date: Thu, 6 Jun 2024 14:44:56 +0000 Subject: [PATCH 05/84] internal/imports: add a warning not to remove the fixImports seam CL 589975 accidentally removed an extensibility seam that was used internally at Google. Strengthen the comment to make it more apparent that this seam is necessary. Change-Id: I7bb402a717968eec76cda514cddb4268c67b7329 Reviewed-on: https://go-review.googlesource.com/c/tools/+/590816 Auto-Submit: Robert Findley LUCI-TryBot-Result: Go LUCI Reviewed-by: Alan Donovan --- internal/imports/fix.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/internal/imports/fix.go b/internal/imports/fix.go index 31f8d1a9933..1d03a7c9533 100644 --- a/internal/imports/fix.go +++ b/internal/imports/fix.go @@ -564,6 +564,8 @@ func (p *pass) addCandidate(imp *ImportInfo, pkg *packageInfo) { // // This is declared as a variable rather than a function so goimports can // easily be extended by adding a file with an init function. +// +// DO NOT REMOVE: used internally at Google. var fixImports = fixImportsDefault func fixImportsDefault(fset *token.FileSet, f *ast.File, filename string, env *ProcessEnv) error { From 5e0f1d8dc37426cdb8e3823f9605745ce7245bb1 Mon Sep 17 00:00:00 2001 From: "Hana (Hyang-Ah) Kim" Date: Wed, 5 Jun 2024 16:48:30 -0400 Subject: [PATCH 06/84] gopls/internal/server: fix regression in organize imports code action CL 587555 suppressed some code actions if the request was triggered automatically, but that was too invasive and suppressed useful code actions like quick fixes and organize imports. Maybe some refactoring code actions and source.* actions can be also useful enough to be presented in the light bulb menu. Partially revert CL 587555, by suppress only refactor.inline code action if the user's intention is not refactoring (e.g. cursor move around a function call). However, if the user selected a code area (function name), that is likely an action for refactoring, so continue to present refactor.inline in the light bulb menu. Fixes golang/go#67823 Fixes golang/go#65167 Change-Id: I2d4a3d92199e501103596d0aed78ece34760149f Reviewed-on: https://go-review.googlesource.com/c/tools/+/590935 Auto-Submit: Hyang-Ah Hana Kim Reviewed-by: Robert Findley LUCI-TryBot-Result: Go LUCI --- gopls/internal/golang/codeaction.go | 4 +- gopls/internal/server/code_action.go | 25 +++++------ .../internal/test/integration/fake/editor.go | 5 +++ .../test/integration/misc/codeactions_test.go | 45 +++++++++++++++++++ gopls/internal/test/integration/wrappers.go | 8 +++- 5 files changed, 69 insertions(+), 18 deletions(-) diff --git a/gopls/internal/golang/codeaction.go b/gopls/internal/golang/codeaction.go index 1350a423fe5..8e8158c59db 100644 --- a/gopls/internal/golang/codeaction.go +++ b/gopls/internal/golang/codeaction.go @@ -28,7 +28,7 @@ import ( // CodeActions returns all code actions (edits and other commands) // available for the selected range. -func CodeActions(ctx context.Context, snapshot *cache.Snapshot, fh file.Handle, rng protocol.Range, diagnostics []protocol.Diagnostic, want map[protocol.CodeActionKind]bool) (actions []protocol.CodeAction, _ error) { +func CodeActions(ctx context.Context, snapshot *cache.Snapshot, fh file.Handle, rng protocol.Range, diagnostics []protocol.Diagnostic, want map[protocol.CodeActionKind]bool, triggerKind protocol.CodeActionTriggerKind) (actions []protocol.CodeAction, _ error) { // Only compute quick fixes if there are any diagnostics to fix. wantQuickFixes := want[protocol.QuickFix] && len(diagnostics) > 0 @@ -138,7 +138,7 @@ func CodeActions(ctx context.Context, snapshot *cache.Snapshot, fh file.Handle, actions = append(actions, rewrites...) } - if want[protocol.RefactorInline] { + if want[protocol.RefactorInline] && (triggerKind != protocol.CodeActionAutomatic || rng.Start != rng.End) { rewrites, err := getInlineCodeActions(pkg, pgf, rng, snapshot.Options()) if err != nil { return nil, err diff --git a/gopls/internal/server/code_action.go b/gopls/internal/server/code_action.go index b5c15d331e0..e79ae68810f 100644 --- a/gopls/internal/server/code_action.go +++ b/gopls/internal/server/code_action.go @@ -113,25 +113,22 @@ func (s *server) CodeAction(ctx context.Context, params *protocol.CodeActionPara // // The diagnostics already have a UI presence (e.g. squiggly underline); // the associated action may additionally show (in VS Code) as a lightbulb. + // Note s.codeActionsMatchingDiagnostics returns only fixes + // detected during the analysis phase. golang.CodeActions computes + // extra changes that can address some diagnostics. actions, err := s.codeActionsMatchingDiagnostics(ctx, uri, snapshot, params.Context.Diagnostics, want) if err != nil { return nil, err } - - // non-diagnostic code actions (non-problematic) - // - // Don't report these for mere cursor motion (trigger=Automatic), only - // when the menu is opened, to avoid a distracting lightbulb in VS Code. - // (See protocol/codeactionkind.go for background.) - // - // Some clients (e.g. eglot) do not set TriggerKind at all. - if k := params.Context.TriggerKind; k == nil || *k != protocol.CodeActionAutomatic { - moreActions, err := golang.CodeActions(ctx, snapshot, fh, params.Range, params.Context.Diagnostics, want) - if err != nil { - return nil, err - } - actions = append(actions, moreActions...) + var triggerKind protocol.CodeActionTriggerKind + if k := params.Context.TriggerKind; k != nil { + triggerKind = *k + } + moreActions, err := golang.CodeActions(ctx, snapshot, fh, params.Range, params.Context.Diagnostics, want, triggerKind) + if err != nil { + return nil, err } + actions = append(actions, moreActions...) // Don't suggest fixes for generated files, since they are generally // not useful and some editors may apply them automatically on save. diff --git a/gopls/internal/test/integration/fake/editor.go b/gopls/internal/test/integration/fake/editor.go index 1269ee0542e..36354d53e6f 100644 --- a/gopls/internal/test/integration/fake/editor.go +++ b/gopls/internal/test/integration/fake/editor.go @@ -1559,6 +1559,10 @@ func (e *Editor) ChangeWorkspaceFolders(ctx context.Context, folders []string) e // CodeAction executes a codeAction request on the server. // If loc.Range is zero, the whole file is implied. func (e *Editor) CodeAction(ctx context.Context, loc protocol.Location, diagnostics []protocol.Diagnostic) ([]protocol.CodeAction, error) { + return e.CodeAction0(ctx, loc, diagnostics, nil) +} + +func (e *Editor) CodeAction0(ctx context.Context, loc protocol.Location, diagnostics []protocol.Diagnostic, triggerKind *protocol.CodeActionTriggerKind) ([]protocol.CodeAction, error) { if e.Server == nil { return nil, nil } @@ -1573,6 +1577,7 @@ func (e *Editor) CodeAction(ctx context.Context, loc protocol.Location, diagnost TextDocument: e.TextDocumentIdentifier(path), Context: protocol.CodeActionContext{ Diagnostics: diagnostics, + TriggerKind: triggerKind, }, Range: loc.Range, // may be zero } diff --git a/gopls/internal/test/integration/misc/codeactions_test.go b/gopls/internal/test/integration/misc/codeactions_test.go index 7c56a18c65d..a6c2a8bd360 100644 --- a/gopls/internal/test/integration/misc/codeactions_test.go +++ b/gopls/internal/test/integration/misc/codeactions_test.go @@ -5,11 +5,13 @@ package misc import ( + "fmt" "testing" "github.com/google/go-cmp/cmp" "golang.org/x/tools/gopls/internal/protocol" . "golang.org/x/tools/gopls/internal/test/integration" + "golang.org/x/tools/gopls/internal/util/slices" ) // This test exercises the filtering of code actions in generated files. @@ -70,3 +72,46 @@ func g() {} protocol.GoFreeSymbols) }) } + +// Test refactor.inline is not included in automatically triggered code action +// unless users want refactoring. +func TestVSCodeIssue65167(t *testing.T) { + const vim1 = `package main + +func main() { + Func() // range to be selected +} + +func Func() int { return 0 } +` + + Run(t, "", func(t *testing.T, env *Env) { + env.CreateBuffer("main.go", vim1) + for _, triggerKind := range []protocol.CodeActionTriggerKind{0, protocol.CodeActionInvoked, protocol.CodeActionAutomatic} { + triggerKindPtr := &triggerKind + if triggerKind == 0 { + triggerKindPtr = nil + } + t.Run(fmt.Sprintf("trigger=%v", triggerKind), func(t *testing.T) { + for _, selectedRange := range []bool{false, true} { + t.Run(fmt.Sprintf("range=%t", selectedRange), func(t *testing.T) { + pattern := env.RegexpSearch("main.go", "Func") + rng := pattern.Range + if !selectedRange { + // assume the cursor is placed at the beginning of `Func`, so end==start. + rng.End = rng.Start + } + loc := protocol.Location{URI: pattern.URI, Range: rng} + actions := env.CodeAction0("main.go", loc, nil, triggerKindPtr) + want := triggerKind != protocol.CodeActionAutomatic || selectedRange + if got := slices.ContainsFunc(actions, func(act protocol.CodeAction) bool { + return act.Kind == protocol.RefactorInline + }); got != want { + t.Errorf("got refactor.inline = %t, want %t", got, want) + } + }) + } + }) + } + }) +} diff --git a/gopls/internal/test/integration/wrappers.go b/gopls/internal/test/integration/wrappers.go index eb472275d25..55b99ea741e 100644 --- a/gopls/internal/test/integration/wrappers.go +++ b/gopls/internal/test/integration/wrappers.go @@ -536,9 +536,13 @@ func (e *Env) AcceptCompletion(loc protocol.Location, item protocol.CompletionIt // CodeAction calls textDocument/codeAction for the given path, and calls // t.Fatal if there are errors. func (e *Env) CodeAction(path string, diagnostics []protocol.Diagnostic) []protocol.CodeAction { - e.T.Helper() loc := protocol.Location{URI: e.Sandbox.Workdir.URI(path)} // no Range => whole file - actions, err := e.Editor.CodeAction(e.Ctx, loc, diagnostics) + return e.CodeAction0(path, loc, diagnostics, nil) +} + +func (e *Env) CodeAction0(path string, loc protocol.Location, diagnostics []protocol.Diagnostic, triggerKind *protocol.CodeActionTriggerKind) []protocol.CodeAction { + e.T.Helper() + actions, err := e.Editor.CodeAction0(e.Ctx, loc, diagnostics, triggerKind) if err != nil { e.T.Fatal(err) } From 6e3b208b986ce9d4f5368198b9efb406799f3695 Mon Sep 17 00:00:00 2001 From: Suzy Mueller Date: Wed, 5 Jun 2024 14:48:47 -0700 Subject: [PATCH 07/84] gopls/internal/test: add test case for parameter rename match import This adds a test case to verify that valid renamings can occur, when the new name of a parameter is identical to an imported package, even when referenced by a function paramater. The bug where the rename was failing is fixed when using go1.22. The new go version fills in a missing Pos for an object scope that allows LookupPos to correctly determine if the scope is relevant. For golang/go#57479 Change-Id: Ifebafb78483f174eac94cd1ec5ac68db9e88684f Reviewed-on: https://go-review.googlesource.com/c/tools/+/590976 LUCI-TryBot-Result: Go LUCI Reviewed-by: Alan Donovan --- .../marker/testdata/rename/issue57479.txt | 37 +++++++++++++++++++ 1 file changed, 37 insertions(+) create mode 100644 gopls/internal/test/marker/testdata/rename/issue57479.txt diff --git a/gopls/internal/test/marker/testdata/rename/issue57479.txt b/gopls/internal/test/marker/testdata/rename/issue57479.txt new file mode 100644 index 00000000000..78004591398 --- /dev/null +++ b/gopls/internal/test/marker/testdata/rename/issue57479.txt @@ -0,0 +1,37 @@ +Test renaming a parameter to the name of an imported package +referenced by one of the function parameters. + +See golang/go#57479 + +-- flags -- +-min_go=go1.22 + +-- go.mod -- +module golang.org/lsptests/rename + +go 1.18 +-- a/a.go -- +package a + +import ( + "fmt" + "math" +) + +func _(x fmt.Stringer) {} //@rename("x", "fmt", xToFmt) + +func _(x int, y fmt.Stringer) {} //@rename("x", "fmt", xyToFmt) + +func _(x [math.MaxInt]bool) {} //@rename("x", "math", xToMath) +-- @xToFmt/a/a.go -- +@@ -8 +8 @@ +-func _(x fmt.Stringer) {} //@rename("x", "fmt", xToFmt) ++func _(fmt fmt.Stringer) {} //@rename("x", "fmt", xToFmt) +-- @xToMath/a/a.go -- +@@ -12 +12 @@ +-func _(x [math.MaxInt]bool) {} //@rename("x", "math", xToMath) ++func _(math [math.MaxInt]bool) {} //@rename("x", "math", xToMath) +-- @xyToFmt/a/a.go -- +@@ -10 +10 @@ +-func _(x int, y fmt.Stringer) {} //@rename("x", "fmt", xyToFmt) ++func _(fmt int, y fmt.Stringer) {} //@rename("x", "fmt", xyToFmt) From c24ae10e7b5734decca2d647ddcf4e3f929f007d Mon Sep 17 00:00:00 2001 From: Alan Donovan Date: Wed, 5 Jun 2024 23:33:57 -0400 Subject: [PATCH 08/84] gopls/internal/cmd: cleanup progress handling This CL adds connection.registerProgressHandler so that each call to, say, executeCommand can temporarily install a handler for progress notifications bearing the token passed to executeCommand. However, the initial workspace load must be treated a little specially since it occurs during the initialize RPC, which happens before the newConnection constructor has returned. So, tracking the IWL is now a core feature of connection instead of something local to the 'stats' subcommand. Also, make the "asynchronous" property a declarative attribute of each command.Command so that both the server and the client can do the right thing--start a thread, wait for the 'end' notification--based on the metadata. Also: - use connection.executeCommand in all cases instead of direct call to ExecuteCommand. - merge Application.connect{,remote} and factor common logic. Change-Id: If7a000593ef4d4dc5658423a03c56b2f4f3a06ca Reviewed-on: https://go-review.googlesource.com/c/tools/+/591095 Reviewed-by: Robert Findley LUCI-TryBot-Result: Go LUCI --- gopls/doc/commands.md | 11 ++ gopls/internal/cmd/call_hierarchy.go | 2 +- gopls/internal/cmd/capabilities_test.go | 2 +- gopls/internal/cmd/check.go | 2 +- gopls/internal/cmd/cmd.go | 136 ++++++++++++------- gopls/internal/cmd/codelens.go | 7 +- gopls/internal/cmd/definition.go | 2 +- gopls/internal/cmd/execute.go | 58 +++----- gopls/internal/cmd/folding_range.go | 2 +- gopls/internal/cmd/format.go | 2 +- gopls/internal/cmd/highlight.go | 2 +- gopls/internal/cmd/implementation.go | 2 +- gopls/internal/cmd/imports.go | 2 +- gopls/internal/cmd/links.go | 2 +- gopls/internal/cmd/prepare_rename.go | 2 +- gopls/internal/cmd/references.go | 2 +- gopls/internal/cmd/rename.go | 2 +- gopls/internal/cmd/semantictokens.go | 2 +- gopls/internal/cmd/signature.go | 2 +- gopls/internal/cmd/stats.go | 40 +----- gopls/internal/cmd/suggested_fix.go | 7 +- gopls/internal/cmd/symbols.go | 2 +- gopls/internal/cmd/workspace_symbol.go | 2 +- gopls/internal/doc/api.json | 6 +- gopls/internal/protocol/command/interface.go | 13 +- gopls/internal/protocol/command/util.go | 17 +++ gopls/internal/server/command.go | 34 ++--- 27 files changed, 194 insertions(+), 169 deletions(-) diff --git a/gopls/doc/commands.md b/gopls/doc/commands.md index c1c5bead121..1e153100c91 100644 --- a/gopls/doc/commands.md +++ b/gopls/doc/commands.md @@ -534,6 +534,8 @@ Identifier: `gopls.run_govulncheck` Run vulnerability check (`govulncheck`). +This command is asynchronous; clients must wait for the 'end' progress notification. + Args: ``` @@ -560,6 +562,8 @@ Identifier: `gopls.run_tests` 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: ``` @@ -665,6 +669,13 @@ Identifier: `gopls.test` 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: ``` diff --git a/gopls/internal/cmd/call_hierarchy.go b/gopls/internal/cmd/call_hierarchy.go index 82c18d0d28f..0ac6956144e 100644 --- a/gopls/internal/cmd/call_hierarchy.go +++ b/gopls/internal/cmd/call_hierarchy.go @@ -39,7 +39,7 @@ func (c *callHierarchy) Run(ctx context.Context, args ...string) error { return tool.CommandLineErrorf("call_hierarchy expects 1 argument (position)") } - conn, err := c.app.connect(ctx, nil) + conn, err := c.app.connect(ctx) if err != nil { return err } diff --git a/gopls/internal/cmd/capabilities_test.go b/gopls/internal/cmd/capabilities_test.go index e043f68eb29..97eb49652d0 100644 --- a/gopls/internal/cmd/capabilities_test.go +++ b/gopls/internal/cmd/capabilities_test.go @@ -48,7 +48,7 @@ func TestCapabilities(t *testing.T) { // Send an initialize request to the server. ctx := context.Background() - client := newClient(app, nil) + client := newClient(app) options := settings.DefaultOptions(app.options) server := server.New(cache.NewSession(ctx, cache.New(nil)), client, options) result, err := server.Initialize(ctx, params) diff --git a/gopls/internal/cmd/check.go b/gopls/internal/cmd/check.go index 7a0d19f85d8..a859ed2c708 100644 --- a/gopls/internal/cmd/check.go +++ b/gopls/internal/cmd/check.go @@ -51,7 +51,7 @@ func (c *check) Run(ctx context.Context, args ...string) error { opts.RelatedInformationSupported = true } - conn, err := c.app.connect(ctx, nil) + conn, err := c.app.connect(ctx) if err != nil { return err } diff --git a/gopls/internal/cmd/cmd.go b/gopls/internal/cmd/cmd.go index ba3a0b1a74c..b3e5dbe0cb8 100644 --- a/gopls/internal/cmd/cmd.go +++ b/gopls/internal/cmd/cmd.go @@ -12,6 +12,7 @@ import ( "flag" "fmt" "log" + "math/rand" "os" "path/filepath" "reflect" @@ -309,40 +310,31 @@ var ( ) // connect creates and initializes a new in-process gopls session. -// -// If onProgress is set, it is called for each new progress notification. -func (app *Application) connect(ctx context.Context, onProgress func(*protocol.ProgressParams)) (*connection, error) { - switch { - case app.Remote == "": - client := newClient(app, onProgress) +func (app *Application) connect(ctx context.Context) (*connection, error) { + client := newClient(app) + var svr protocol.Server + if app.Remote == "" { + // local options := settings.DefaultOptions(app.options) - server := server.New(cache.NewSession(ctx, cache.New(nil)), client, options) - conn := newConnection(server, client) - if err := conn.initialize(protocol.WithClient(ctx, client), app.options); err != nil { + svr = server.New(cache.NewSession(ctx, cache.New(nil)), client, options) + ctx = protocol.WithClient(ctx, client) + + } else { + // remote + netConn, err := lsprpc.ConnectToRemote(ctx, app.Remote) + if err != nil { return nil, err } - return conn, nil - - default: - return app.connectRemote(ctx, app.Remote) + stream := jsonrpc2.NewHeaderStream(netConn) + jsonConn := jsonrpc2.NewConn(stream) + svr = protocol.ServerDispatcher(jsonConn) + ctx = protocol.WithClient(ctx, client) + jsonConn.Go(ctx, + protocol.Handlers( + protocol.ClientHandler(client, jsonrpc2.MethodNotFound))) } -} - -func (app *Application) connectRemote(ctx context.Context, remote string) (*connection, error) { - conn, err := lsprpc.ConnectToRemote(ctx, remote) - if err != nil { - return nil, err - } - stream := jsonrpc2.NewHeaderStream(conn) - cc := jsonrpc2.NewConn(stream) - server := protocol.ServerDispatcher(cc) - client := newClient(app, nil) - connection := newConnection(server, client) - ctx = protocol.WithClient(ctx, connection.client) - cc.Go(ctx, - protocol.Handlers( - protocol.ClientHandler(client, jsonrpc2.MethodNotFound))) - return connection, connection.initialize(ctx, app.options) + conn := newConnection(svr, client) + return conn, conn.initialize(ctx, app.options) } func (c *connection) initialize(ctx context.Context, options func(*settings.Options)) error { @@ -368,12 +360,7 @@ func (c *connection) initialize(ctx context.Context, options func(*settings.Opti params.Capabilities.TextDocument.SemanticTokens.Requests.Full = &protocol.Or_ClientSemanticTokensRequestOptions_full{Value: true} params.Capabilities.TextDocument.SemanticTokens.TokenTypes = protocol.SemanticTypes() params.Capabilities.TextDocument.SemanticTokens.TokenModifiers = protocol.SemanticModifiers() - - // If the subcommand has registered a progress handler, report the progress - // capability. - if c.client.onProgress != nil { - params.Capabilities.Window.WorkDoneProgress = true - } + params.Capabilities.Window.WorkDoneProgress = true params.InitializationOptions = map[string]interface{}{ "symbolMatcher": string(opts.SymbolMatcher), @@ -392,10 +379,35 @@ type connection struct { client *cmdClient } +// registerProgressHandler registers a handler for progress notifications. +// The caller must call unregister when the handler is no longer needed. +func (cli *cmdClient) registerProgressHandler(handler func(*protocol.ProgressParams)) (token protocol.ProgressToken, unregister func()) { + token = fmt.Sprintf("tok%d", rand.Uint64()) + + // register + cli.progressHandlersMu.Lock() + if cli.progressHandlers == nil { + cli.progressHandlers = make(map[protocol.ProgressToken]func(*protocol.ProgressParams)) + } + cli.progressHandlers[token] = handler + cli.progressHandlersMu.Unlock() + + unregister = func() { + cli.progressHandlersMu.Lock() + delete(cli.progressHandlers, token) + cli.progressHandlersMu.Unlock() + } + return token, unregister +} + // cmdClient defines the protocol.Client interface behavior of the gopls CLI tool. type cmdClient struct { - app *Application - onProgress func(*protocol.ProgressParams) + app *Application + + progressHandlersMu sync.Mutex + progressHandlers map[protocol.ProgressToken]func(*protocol.ProgressParams) + iwlToken protocol.ProgressToken + iwlDone chan struct{} filesMu sync.Mutex // guards files map files map[protocol.DocumentURI]*cmdFile @@ -409,11 +421,11 @@ type cmdFile struct { diagnostics []protocol.Diagnostic } -func newClient(app *Application, onProgress func(*protocol.ProgressParams)) *cmdClient { +func newClient(app *Application) *cmdClient { return &cmdClient{ - app: app, - onProgress: onProgress, - files: make(map[protocol.DocumentURI]*cmdFile), + app: app, + files: make(map[protocol.DocumentURI]*cmdFile), + iwlDone: make(chan struct{}), } } @@ -674,12 +686,43 @@ func (c *cmdClient) PublishDiagnostics(ctx context.Context, p *protocol.PublishD } func (c *cmdClient) Progress(_ context.Context, params *protocol.ProgressParams) error { - if c.onProgress != nil { - c.onProgress(params) + token, ok := params.Token.(string) + if !ok { + return fmt.Errorf("unexpected progress token: %[1]T %[1]v", params.Token) + } + + c.progressHandlersMu.Lock() + handler := c.progressHandlers[token] + c.progressHandlersMu.Unlock() + if handler == nil { + handler = c.defaultProgressHandler } + handler(params) return nil } +// defaultProgressHandler is the default handler of progress messages, +// used during the initialize request. +func (c *cmdClient) defaultProgressHandler(params *protocol.ProgressParams) { + switch v := params.Value.(type) { + case *protocol.WorkDoneProgressBegin: + if v.Title == server.DiagnosticWorkTitle(server.FromInitialWorkspaceLoad) { + c.progressHandlersMu.Lock() + c.iwlToken = params.Token + c.progressHandlersMu.Unlock() + } + + case *protocol.WorkDoneProgressEnd: + c.progressHandlersMu.Lock() + iwlToken := c.iwlToken + c.progressHandlersMu.Unlock() + + if params.Token == iwlToken { + close(c.iwlDone) + } + } +} + func (c *cmdClient) ShowDocument(ctx context.Context, params *protocol.ShowDocumentParams) (*protocol.ShowDocumentResult, error) { var success bool if params.External { @@ -781,10 +824,7 @@ func (c *connection) diagnoseFiles(ctx context.Context, files []protocol.Documen if err != nil { return err } - _, err = c.ExecuteCommand(ctx, &protocol.ExecuteCommandParams{ - Command: cmd.Command, - Arguments: cmd.Arguments, - }) + _, err = c.executeCommand(ctx, &cmd) return err } diff --git a/gopls/internal/cmd/codelens.go b/gopls/internal/cmd/codelens.go index a7017d8e3f1..b07f15429ce 100644 --- a/gopls/internal/cmd/codelens.go +++ b/gopls/internal/cmd/codelens.go @@ -83,10 +83,7 @@ func (r *codelens) Run(ctx context.Context, args ...string) error { opts.Codelenses[protocol.CodeLensTest] = true } - // TODO(adonovan): cleanup: factor progress with stats subcommand. - cmdDone, onProgress := commandProgress() - - conn, err := r.app.connect(ctx, onProgress) + conn, err := r.app.connect(ctx) if err != nil { return err } @@ -125,7 +122,7 @@ func (r *codelens) Run(ctx context.Context, args ...string) error { // -exec: run the first matching code lens. if r.Exec { - _, err := conn.executeCommand(ctx, cmdDone, lens.Command) + _, err := conn.executeCommand(ctx, lens.Command) return err } diff --git a/gopls/internal/cmd/definition.go b/gopls/internal/cmd/definition.go index e5e119b8da8..d9cd98554e3 100644 --- a/gopls/internal/cmd/definition.go +++ b/gopls/internal/cmd/definition.go @@ -73,7 +73,7 @@ func (d *definition) Run(ctx context.Context, args ...string) error { o.PreferredContentFormat = protocol.Markdown } } - conn, err := d.app.connect(ctx, nil) + conn, err := d.app.connect(ctx) if err != nil { return err } diff --git a/gopls/internal/cmd/execute.go b/gopls/internal/cmd/execute.go index 381c2a7aa95..485e6d03ca9 100644 --- a/gopls/internal/cmd/execute.go +++ b/gopls/internal/cmd/execute.go @@ -76,14 +76,13 @@ func (e *execute) Run(ctx context.Context, args ...string) error { e.app.editFlags = &e.EditFlags // in case command performs an edit - cmdDone, onProgress := commandProgress() - conn, err := e.app.connect(ctx, onProgress) + conn, err := e.app.connect(ctx) if err != nil { return err } defer conn.terminate(ctx) - res, err := conn.executeCommand(ctx, cmdDone, &protocol.Command{ + res, err := conn.executeCommand(ctx, &protocol.Command{ Command: cmd, Arguments: jsonArgs, }) @@ -100,55 +99,40 @@ func (e *execute) Run(ctx context.Context, args ...string) error { return nil } -// -- shared command helpers -- - -const cmdProgressToken = "cmd-progress" - -// TODO(adonovan): disentangle this from app.connect, and factor with -// conn.executeCommand used by codelens and execute. Seems like -// connection needs a way to register and unregister independent -// handlers, later than at connect time. -func commandProgress() (<-chan bool, func(p *protocol.ProgressParams)) { - cmdDone := make(chan bool, 1) - onProgress := func(p *protocol.ProgressParams) { - switch v := p.Value.(type) { +// executeCommand executes a protocol.Command, displaying progress +// messages and awaiting completion of asynchronous commands. +func (conn *connection) executeCommand(ctx context.Context, cmd *protocol.Command) (any, error) { + endStatus := make(chan string, 1) + token, unregister := conn.client.registerProgressHandler(func(params *protocol.ProgressParams) { + switch v := params.Value.(type) { case *protocol.WorkDoneProgressReport: - // TODO(adonovan): how can we segregate command's stdout and - // stderr so that structure is preserved? - fmt.Fprintln(os.Stderr, v.Message) + fmt.Fprintln(os.Stderr, v.Message) // combined std{out,err} case *protocol.WorkDoneProgressEnd: - if p.Token == cmdProgressToken { - // commandHandler.run sends message = canceled | failed | completed - cmdDone <- v.Message == server.CommandCompleted - } + endStatus <- v.Message // = canceled | failed | completed } - } - return cmdDone, onProgress -} + }) + defer unregister() -func (conn *connection) executeCommand(ctx context.Context, done <-chan bool, cmd *protocol.Command) (any, error) { res, err := conn.ExecuteCommand(ctx, &protocol.ExecuteCommandParams{ Command: cmd.Command, Arguments: cmd.Arguments, WorkDoneProgressParams: protocol.WorkDoneProgressParams{ - WorkDoneToken: cmdProgressToken, + WorkDoneToken: token, }, }) if err != nil { return nil, err } - // Wait for it to finish (by watching for a progress token). - // - // In theory this is only necessary for the two async - // commands (RunGovulncheck and RunTests), but the tests - // fail for Test as well (why?), and there is no cost to - // waiting in all cases. TODO(adonovan): investigate. - if success := <-done; !success { - // TODO(adonovan): suppress this message; - // the command's stderr should suffice. - return nil, fmt.Errorf("command failed") + // Some commands are asynchronous, so clients + // must wait for the "end" progress notification. + enum := command.Command(strings.TrimPrefix(cmd.Command, "gopls.")) + if enum.IsAsync() { + status := <-endStatus + if status != server.CommandCompleted { + return nil, fmt.Errorf("command %s", status) + } } return res, nil diff --git a/gopls/internal/cmd/folding_range.go b/gopls/internal/cmd/folding_range.go index 13f78c197a5..f48feee5b2c 100644 --- a/gopls/internal/cmd/folding_range.go +++ b/gopls/internal/cmd/folding_range.go @@ -36,7 +36,7 @@ func (r *foldingRanges) Run(ctx context.Context, args ...string) error { return tool.CommandLineErrorf("folding_ranges expects 1 argument (file)") } - conn, err := r.app.connect(ctx, nil) + conn, err := r.app.connect(ctx) if err != nil { return err } diff --git a/gopls/internal/cmd/format.go b/gopls/internal/cmd/format.go index 75982c9efba..eb68d73d527 100644 --- a/gopls/internal/cmd/format.go +++ b/gopls/internal/cmd/format.go @@ -42,7 +42,7 @@ func (c *format) Run(ctx context.Context, args ...string) error { return nil } c.app.editFlags = &c.EditFlags - conn, err := c.app.connect(ctx, nil) + conn, err := c.app.connect(ctx) if err != nil { return err } diff --git a/gopls/internal/cmd/highlight.go b/gopls/internal/cmd/highlight.go index 9c1488b30be..43af063f53f 100644 --- a/gopls/internal/cmd/highlight.go +++ b/gopls/internal/cmd/highlight.go @@ -38,7 +38,7 @@ func (r *highlight) Run(ctx context.Context, args ...string) error { return tool.CommandLineErrorf("highlight expects 1 argument (position)") } - conn, err := r.app.connect(ctx, nil) + conn, err := r.app.connect(ctx) if err != nil { return err } diff --git a/gopls/internal/cmd/implementation.go b/gopls/internal/cmd/implementation.go index fcfb63185b4..858026540ad 100644 --- a/gopls/internal/cmd/implementation.go +++ b/gopls/internal/cmd/implementation.go @@ -39,7 +39,7 @@ func (i *implementation) Run(ctx context.Context, args ...string) error { return tool.CommandLineErrorf("implementation expects 1 argument (position)") } - conn, err := i.app.connect(ctx, nil) + conn, err := i.app.connect(ctx) if err != nil { return err } diff --git a/gopls/internal/cmd/imports.go b/gopls/internal/cmd/imports.go index 12b49ef254d..c64c871e390 100644 --- a/gopls/internal/cmd/imports.go +++ b/gopls/internal/cmd/imports.go @@ -43,7 +43,7 @@ func (t *imports) Run(ctx context.Context, args ...string) error { return tool.CommandLineErrorf("imports expects 1 argument") } t.app.editFlags = &t.EditFlags - conn, err := t.app.connect(ctx, nil) + conn, err := t.app.connect(ctx) if err != nil { return err } diff --git a/gopls/internal/cmd/links.go b/gopls/internal/cmd/links.go index 0f1d671a503..3c14f4e6608 100644 --- a/gopls/internal/cmd/links.go +++ b/gopls/internal/cmd/links.go @@ -44,7 +44,7 @@ func (l *links) Run(ctx context.Context, args ...string) error { if len(args) != 1 { return tool.CommandLineErrorf("links expects 1 argument") } - conn, err := l.app.connect(ctx, nil) + conn, err := l.app.connect(ctx) if err != nil { return err } diff --git a/gopls/internal/cmd/prepare_rename.go b/gopls/internal/cmd/prepare_rename.go index c7901e6484d..3ff38356d55 100644 --- a/gopls/internal/cmd/prepare_rename.go +++ b/gopls/internal/cmd/prepare_rename.go @@ -43,7 +43,7 @@ func (r *prepareRename) Run(ctx context.Context, args ...string) error { return tool.CommandLineErrorf("prepare_rename expects 1 argument (file)") } - conn, err := r.app.connect(ctx, nil) + conn, err := r.app.connect(ctx) if err != nil { return err } diff --git a/gopls/internal/cmd/references.go b/gopls/internal/cmd/references.go index 3c294c71b14..1483bf12db0 100644 --- a/gopls/internal/cmd/references.go +++ b/gopls/internal/cmd/references.go @@ -43,7 +43,7 @@ func (r *references) Run(ctx context.Context, args ...string) error { return tool.CommandLineErrorf("references expects 1 argument (position)") } - conn, err := r.app.connect(ctx, nil) + conn, err := r.app.connect(ctx) if err != nil { return err } diff --git a/gopls/internal/cmd/rename.go b/gopls/internal/cmd/rename.go index 6d831681c19..e96850cd1c8 100644 --- a/gopls/internal/cmd/rename.go +++ b/gopls/internal/cmd/rename.go @@ -45,7 +45,7 @@ func (r *rename) Run(ctx context.Context, args ...string) error { return tool.CommandLineErrorf("rename expects 2 arguments (position, new name)") } r.app.editFlags = &r.EditFlags - conn, err := r.app.connect(ctx, nil) + conn, err := r.app.connect(ctx) if err != nil { return err } diff --git a/gopls/internal/cmd/semantictokens.go b/gopls/internal/cmd/semantictokens.go index 572962116e8..77e8a03939c 100644 --- a/gopls/internal/cmd/semantictokens.go +++ b/gopls/internal/cmd/semantictokens.go @@ -71,7 +71,7 @@ func (c *semtok) Run(ctx context.Context, args ...string) error { } opts.SemanticTokens = true } - conn, err := c.app.connect(ctx, nil) + conn, err := c.app.connect(ctx) if err != nil { return err } diff --git a/gopls/internal/cmd/signature.go b/gopls/internal/cmd/signature.go index cf976a64859..601cfaa13fa 100644 --- a/gopls/internal/cmd/signature.go +++ b/gopls/internal/cmd/signature.go @@ -38,7 +38,7 @@ func (r *signature) Run(ctx context.Context, args ...string) error { return tool.CommandLineErrorf("signature expects 1 argument (position)") } - conn, err := r.app.connect(ctx, nil) + conn, err := r.app.connect(ctx) if err != nil { return err } diff --git a/gopls/internal/cmd/stats.go b/gopls/internal/cmd/stats.go index 8da1a1a6ae8..a78880107e7 100644 --- a/gopls/internal/cmd/stats.go +++ b/gopls/internal/cmd/stats.go @@ -16,13 +16,11 @@ import ( "reflect" "runtime" "strings" - "sync" "time" "golang.org/x/tools/gopls/internal/filecache" "golang.org/x/tools/gopls/internal/protocol" "golang.org/x/tools/gopls/internal/protocol/command" - "golang.org/x/tools/gopls/internal/server" "golang.org/x/tools/gopls/internal/settings" bugpkg "golang.org/x/tools/gopls/internal/util/bug" versionpkg "golang.org/x/tools/gopls/internal/version" @@ -85,30 +83,6 @@ func (s *stats) Run(ctx context.Context, args ...string) error { } o.VerboseWorkDoneProgress = true } - var ( - iwlMu sync.Mutex - iwlToken protocol.ProgressToken - iwlDone = make(chan struct{}) - ) - - onProgress := func(p *protocol.ProgressParams) { - switch v := p.Value.(type) { - case *protocol.WorkDoneProgressBegin: - if v.Title == server.DiagnosticWorkTitle(server.FromInitialWorkspaceLoad) { - iwlMu.Lock() - iwlToken = p.Token - iwlMu.Unlock() - } - case *protocol.WorkDoneProgressEnd: - iwlMu.Lock() - tok := iwlToken - iwlMu.Unlock() - - if p.Token == tok { - close(iwlDone) - } - } - } // do executes a timed section of the stats command. do := func(name string, f func() error) (time.Duration, error) { @@ -123,25 +97,25 @@ func (s *stats) Run(ctx context.Context, args ...string) error { } var conn *connection - iwlDuration, err := do("Initializing workspace", func() error { - var err error - conn, err = s.app.connect(ctx, onProgress) + iwlDuration, err := do("Initializing workspace", func() (err error) { + conn, err = s.app.connect(ctx) if err != nil { return err } select { - case <-iwlDone: + case <-conn.client.iwlDone: case <-ctx.Done(): return ctx.Err() } return nil }) - stats.InitialWorkspaceLoadDuration = fmt.Sprint(iwlDuration) if err != nil { return err } defer conn.terminate(ctx) + stats.InitialWorkspaceLoadDuration = fmt.Sprint(iwlDuration) + // Gather bug reports produced by any process using // this executable and persisted in the cache. do("Gathering bug reports", func() error { @@ -153,7 +127,7 @@ func (s *stats) Run(ctx context.Context, args ...string) error { }) if _, err := do("Querying memstats", func() error { - memStats, err := conn.ExecuteCommand(ctx, &protocol.ExecuteCommandParams{ + memStats, err := conn.executeCommand(ctx, &protocol.Command{ Command: command.MemStats.ID(), }) if err != nil { @@ -166,7 +140,7 @@ func (s *stats) Run(ctx context.Context, args ...string) error { } if _, err := do("Querying workspace stats", func() error { - wsStats, err := conn.ExecuteCommand(ctx, &protocol.ExecuteCommandParams{ + wsStats, err := conn.executeCommand(ctx, &protocol.Command{ Command: command.WorkspaceStats.ID(), }) if err != nil { diff --git a/gopls/internal/cmd/suggested_fix.go b/gopls/internal/cmd/suggested_fix.go index e066460526d..a326c9225f0 100644 --- a/gopls/internal/cmd/suggested_fix.go +++ b/gopls/internal/cmd/suggested_fix.go @@ -75,7 +75,7 @@ func (s *suggestedFix) Run(ctx context.Context, args ...string) error { return tool.CommandLineErrorf("fix expects at least 1 argument") } s.app.editFlags = &s.EditFlags - conn, err := s.app.connect(ctx, nil) + conn, err := s.app.connect(ctx) if err != nil { return err } @@ -136,10 +136,7 @@ func (s *suggestedFix) Run(ctx context.Context, args ...string) error { // This may cause the server to make // an ApplyEdit downcall to the client. if a.Command != nil { - if _, err := conn.ExecuteCommand(ctx, &protocol.ExecuteCommandParams{ - Command: a.Command.Command, - Arguments: a.Command.Arguments, - }); err != nil { + if _, err := conn.executeCommand(ctx, a.Command); err != nil { return err } // The specification says that commands should diff --git a/gopls/internal/cmd/symbols.go b/gopls/internal/cmd/symbols.go index 249397d320f..663a08f4be1 100644 --- a/gopls/internal/cmd/symbols.go +++ b/gopls/internal/cmd/symbols.go @@ -36,7 +36,7 @@ func (r *symbols) Run(ctx context.Context, args ...string) error { return tool.CommandLineErrorf("symbols expects 1 argument (position)") } - conn, err := r.app.connect(ctx, nil) + conn, err := r.app.connect(ctx) if err != nil { return err } diff --git a/gopls/internal/cmd/workspace_symbol.go b/gopls/internal/cmd/workspace_symbol.go index 9fa7526a24d..aba33fa9d2a 100644 --- a/gopls/internal/cmd/workspace_symbol.go +++ b/gopls/internal/cmd/workspace_symbol.go @@ -59,7 +59,7 @@ func (r *workspaceSymbol) Run(ctx context.Context, args ...string) error { } } - conn, err := r.app.connect(ctx, nil) + conn, err := r.app.connect(ctx) if err != nil { return err } diff --git a/gopls/internal/doc/api.json b/gopls/internal/doc/api.json index bcb2610a897..e17038511a9 100644 --- a/gopls/internal/doc/api.json +++ b/gopls/internal/doc/api.json @@ -1089,14 +1089,14 @@ { "Command": "gopls.run_govulncheck", "Title": "Run vulncheck", - "Doc": "Run vulnerability check (`govulncheck`).", + "Doc": "Run vulnerability check (`govulncheck`).\n\nThis command is asynchronous; clients must wait for the 'end' progress notification.", "ArgDoc": "{\n\t// Any document in the directory from which govulncheck will run.\n\t\"URI\": string,\n\t// Package pattern. E.g. \"\", \".\", \"./...\".\n\t\"Pattern\": string,\n}", "ResultDoc": "{\n\t// Token holds the progress token for LSP workDone reporting of the vulncheck\n\t// invocation.\n\t\"Token\": interface{},\n}" }, { "Command": "gopls.run_tests", "Title": "Run test(s)", - "Doc": "Runs `go test` for a specific set of test or benchmark functions.", + "Doc": "Runs `go test` for a specific set of test or benchmark functions.\n\nThis command is asynchronous; clients must wait for the 'end' progress notification.", "ArgDoc": "{\n\t// The test file containing the tests to run.\n\t\"URI\": string,\n\t// Specific test names to run, e.g. TestFoo.\n\t\"Tests\": []string,\n\t// Specific benchmarks to run, e.g. BenchmarkFoo.\n\t\"Benchmarks\": []string,\n}", "ResultDoc": "" }, @@ -1124,7 +1124,7 @@ { "Command": "gopls.test", "Title": "Run test(s) (legacy)", - "Doc": "Runs `go test` for a specific set of test or benchmark functions.", + "Doc": "Runs `go test` for a specific set of test or benchmark functions.\n\nThis command is asynchronous; wait for the 'end' progress notification.\n\nThis command is an alias for RunTests; the only difference\nis the form of the parameters.\n\nTODO(adonovan): eliminate it.", "ArgDoc": "string,\n[]string,\n[]string", "ResultDoc": "" }, diff --git a/gopls/internal/protocol/command/interface.go b/gopls/internal/protocol/command/interface.go index 0f4402641c9..0934cf22dc4 100644 --- a/gopls/internal/protocol/command/interface.go +++ b/gopls/internal/protocol/command/interface.go @@ -49,13 +49,20 @@ type Interface interface { // 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. Test(context.Context, protocol.DocumentURI, []string, []string) error - // TODO: deprecate Test in favor of RunTests below. - // Test: 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. RunTests(context.Context, RunTestsArgs) error // Generate: Run go generate @@ -178,6 +185,8 @@ type Interface interface { // RunGovulncheck: Run vulncheck // // Run vulnerability check (`govulncheck`). + // + // This command is asynchronous; clients must wait for the 'end' progress notification. RunGovulncheck(context.Context, VulncheckArgs) (RunVulncheckResult, error) // FetchVulncheckResult: Get known vulncheck result diff --git a/gopls/internal/protocol/command/util.go b/gopls/internal/protocol/command/util.go index f49e0404ea0..0d9799f5dc0 100644 --- a/gopls/internal/protocol/command/util.go +++ b/gopls/internal/protocol/command/util.go @@ -9,6 +9,11 @@ import ( "fmt" ) +// TODO(adonovan): use the "gopls."-prefix forms everywhere. The +// existence of two forms is a nuisance. I think it should be a +// straightforward fix, as all public APIs and docs use that form +// already. + // ID returns the command name for use in the LSP. func ID(name string) string { return "gopls." + name @@ -16,6 +21,18 @@ func ID(name string) string { type Command string +// IsAsync reports whether the command is asynchronous: +// clients must wait for the "end" progress notification. +func (c Command) IsAsync() bool { + switch string(c) { + // TODO(adonovan): derive this list from interface.go somewhow. + // Unfortunately we can't even reference the enum from here... + case "run_tests", "run_govulncheck", "test": + return true + } + return false +} + // ID returns the command identifier to use in the executeCommand request. func (c Command) ID() string { return ID(string(c)) diff --git a/gopls/internal/server/command.go b/gopls/internal/server/command.go index f7bf1aadd6f..ad2eb9501d5 100644 --- a/gopls/internal/server/command.go +++ b/gopls/internal/server/command.go @@ -12,6 +12,7 @@ import ( "fmt" "go/ast" "io" + "log" "os" "path/filepath" "regexp" @@ -93,18 +94,8 @@ func (*commandHandler) AddTelemetryCounters(_ context.Context, args command.AddT // commandConfig configures common command set-up and execution. type commandConfig struct { - // TODO(adonovan): whether a command is synchronous or - // asynchronous is part of the server interface contract, not - // a mere implementation detail of the handler. - // Export a (command.Command).IsAsync() property so that - // clients can tell. (The tricky part is ensuring the handler - // remains consistent with the command.Command metadata, as at - // the point were we read the 'async' field below, we no - // longer know that command.Command.) - - async bool // whether to run the command asynchronously. Async commands can only return errors. requireSave bool // whether all files must be saved for the command to work - progress string // title to use for progress reporting. If empty, no progress will be reported. + progress string // title to use for progress reporting. If empty, no progress will be reported. Mandatory for async commands. forView string // view to resolve to a snapshot; incompatible with forURI forURI protocol.DocumentURI // URI to resolve to a snapshot. If unset, snapshot will be nil. } @@ -115,7 +106,7 @@ type commandConfig struct { type commandDeps struct { snapshot *cache.Snapshot // present if cfg.forURI was set fh file.Handle // present if cfg.forURI was set - work *progress.WorkDone // present cfg.progress was set + work *progress.WorkDone // present if cfg.progress was set } type commandFunc func(context.Context, commandDeps) error @@ -195,7 +186,13 @@ func (c *commandHandler) run(ctx context.Context, cfg commandConfig, run command } return err } - if cfg.async { + + enum := command.Command(strings.TrimPrefix(c.params.Command, "gopls.")) + if enum.IsAsync() { + if cfg.progress == "" { + log.Fatalf("asynchronous command %q does not enable progress reporting", + enum) + } go func() { if err := runcmd(); err != nil { showMessage(ctx, c.s.client, protocol.Error, err.Error()) @@ -491,6 +488,7 @@ func dropDependency(pm *cache.ParsedModule, modulePath string) ([]protocol.TextE return protocol.EditsFromDiffEdits(pm.Mapper, diff) } +// Test is an alias for RunTests (with splayed arguments). func (c *commandHandler) Test(ctx context.Context, uri protocol.DocumentURI, tests, benchmarks []string) error { return c.RunTests(ctx, command.RunTestsArgs{ URI: uri, @@ -583,9 +581,8 @@ func (c *commandHandler) Doc(ctx context.Context, loc protocol.Location) error { func (c *commandHandler) RunTests(ctx context.Context, args command.RunTestsArgs) error { return c.run(ctx, commandConfig{ - async: true, - progress: "Running go test", - requireSave: true, // go test honors overlays, but tests themselves cannot + progress: "Running go test", // (asynchronous) + requireSave: true, // go test honors overlays, but tests themselves cannot forURI: args.URI, }, func(ctx context.Context, deps commandDeps) error { return c.runTests(ctx, deps.snapshot, deps.work, args.URI, args.Tests, args.Benchmarks) @@ -1088,9 +1085,8 @@ func (c *commandHandler) RunGovulncheck(ctx context.Context, args command.Vulnch // synchronize the start of the run and return the token. tokenChan := make(chan protocol.ProgressToken, 1) err := c.run(ctx, commandConfig{ - async: true, // need to be async to be cancellable - progress: "govulncheck", - requireSave: true, // govulncheck cannot honor overlays + progress: "govulncheck", // (asynchronous) + requireSave: true, // govulncheck cannot honor overlays forURI: args.URI, }, func(ctx context.Context, deps commandDeps) error { tokenChan <- deps.work.Token() From fc82f4e5a0172290db048466b14783f620daeae4 Mon Sep 17 00:00:00 2001 From: Alan Donovan Date: Thu, 6 Jun 2024 14:27:30 -0400 Subject: [PATCH 09/84] gopls/internal/protocol/command: use gopls.foo form everywhere MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Before, the command.Command enum values (e.g. "run_tests") did not match the actual command strings used in the Command field of protocol.Command (e.g. "gopls.run_tests"). This change causes us to use the "gopls."-prefixed form everywhere, avoiding the need for various conversions, and the opportunity to forget to make them. Also - tidy up the commands.md markdown. - remove 2x TODOs about eliminating Command.Title: I think it works nicely in the documentation. - remove ⬤ blobs from markdown by popular demand. Change-Id: Ida5981ba834ea148fa0267319cf874b9a48c9289 Reviewed-on: https://go-review.googlesource.com/c/tools/+/591175 Auto-Submit: Alan Donovan LUCI-TryBot-Result: Go LUCI Reviewed-by: Robert Findley --- gopls/doc/codelenses.md | 16 +- gopls/doc/commands.md | 105 +++------ gopls/doc/generate/generate.go | 13 +- gopls/doc/settings.md | 70 +++--- gopls/internal/cmd/execute.go | 6 +- gopls/internal/cmd/remote.go | 2 +- gopls/internal/cmd/stats.go | 4 +- gopls/internal/doc/api.go | 2 +- gopls/internal/lsprpc/lsprpc.go | 2 +- .../internal/protocol/command/command_gen.go | 220 +++++++++--------- .../protocol/command/commandmeta/meta.go | 19 +- gopls/internal/protocol/command/gen/gen.go | 14 +- gopls/internal/protocol/command/interface.go | 5 +- gopls/internal/protocol/command/util.go | 21 +- gopls/internal/server/command.go | 3 +- gopls/internal/settings/default.go | 2 +- gopls/internal/telemetry/telemetry_test.go | 4 +- .../test/integration/bench/bench_test.go | 2 +- .../test/integration/bench/iwl_test.go | 4 +- .../integration/codelens/codelens_test.go | 2 +- .../integration/codelens/gcdetails_test.go | 2 +- .../test/integration/debug/debug_test.go | 2 +- .../internal/test/integration/fake/editor.go | 4 +- .../test/integration/misc/debugserver_test.go | 2 +- .../test/integration/misc/import_test.go | 2 +- .../test/integration/misc/vuln_test.go | 2 +- gopls/internal/test/integration/wrappers.go | 8 +- 27 files changed, 240 insertions(+), 298 deletions(-) diff --git a/gopls/doc/codelenses.md b/gopls/doc/codelenses.md index 378a3db1732..34776273e29 100644 --- a/gopls/doc/codelenses.md +++ b/gopls/doc/codelenses.md @@ -16,7 +16,7 @@ Their features are subject to change. -## ⬤ `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 +40,7 @@ Default: off File type: Go -## ⬤ `generate`: Run `go generate` +## `generate`: Run `go generate` This codelens source annotates any `//go:generate` comments @@ -55,7 +55,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 +71,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 +90,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 +107,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 +120,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 +135,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/commands.md b/gopls/doc/commands.md index 1e153100c91..69c3af86225 100644 --- a/gopls/doc/commands.md +++ b/gopls/doc/commands.md @@ -3,8 +3,7 @@ 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. -### **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 +20,7 @@ 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 +39,7 @@ 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 +54,7 @@ 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 +140,7 @@ Result: } ``` -### **Perform a "change signature" refactoring** -Identifier: `gopls.change_signature` +## `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 +212,7 @@ Result: } ``` -### **Check for upgrades** -Identifier: `gopls.check_upgrades` +## `gopls.check_upgrades`: **Check for upgrades** Checks for module upgrades. @@ -233,8 +227,7 @@ 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 +239,7 @@ Args: } ``` -### **View package documentation.** -Identifier: `gopls.doc` +## `gopls.doc`: **View package documentation.** Opens the Go package documentation page for the current package in a browser. @@ -270,8 +262,7 @@ 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 +277,7 @@ Args: } ``` -### **Get known vulncheck result** -Identifier: `gopls.fetch_vulncheck_result` +## `gopls.fetch_vulncheck_result`: **Get known vulncheck result** Fetch the result of latest vulnerability check (`govulncheck`). @@ -306,8 +296,7 @@ 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`: **report free symbols referenced by the selection.** This command is a query over a selected range of Go source code. It reports the set of "free" symbols of the @@ -335,8 +324,7 @@ string, } ``` -### **Toggle gc_details** -Identifier: `gopls.gc_details` +## `gopls.gc_details`: **Toggle gc_details** Toggle the calculation of gc annotations. @@ -346,8 +334,7 @@ Args: string ``` -### **Run go generate** -Identifier: `gopls.generate` +## `gopls.generate`: **Run go generate** Runs `go generate` for a given directory. @@ -362,8 +349,7 @@ 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 +365,7 @@ 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 +395,7 @@ 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 +421,13 @@ 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 +444,7 @@ Result: } ``` -### **Regenerate cgo** -Identifier: `gopls.regenerate_cgo` +## `gopls.regenerate_cgo`: **Regenerate cgo** Regenerates cgo definitions. @@ -476,8 +457,7 @@ 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 +476,7 @@ 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 +493,7 @@ 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,8 +507,7 @@ Args: } ``` -### **Run vulncheck** -Identifier: `gopls.run_govulncheck` +## `gopls.run_govulncheck`: **Run vulncheck** Run vulnerability check (`govulncheck`). @@ -557,8 +534,7 @@ 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. @@ -577,8 +553,7 @@ Args: } ``` -### **Start the gopls debug server** -Identifier: `gopls.start_debugging` +## `gopls.start_debugging`: **Start the gopls debug server** Start the gopls debug server if it isn't running, and return the debug address. @@ -622,8 +597,7 @@ 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. @@ -643,8 +617,7 @@ 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. @@ -664,8 +637,7 @@ 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. @@ -684,8 +656,7 @@ string, []string ``` -### **Run go mod tidy** -Identifier: `gopls.tidy` +## `gopls.tidy`: **Run go mod tidy** Runs `go mod tidy` for a module. @@ -698,8 +669,7 @@ Args: } ``` -### **Toggle gc_details** -Identifier: `gopls.toggle_gc_details` +## `gopls.toggle_gc_details`: **Toggle gc_details** Toggle the calculation of gc annotations. @@ -712,8 +682,7 @@ Args: } ``` -### **Update go.sum** -Identifier: `gopls.update_go_sum` +## `gopls.update_go_sum`: **Update go.sum** Updates the go.sum file for a module. @@ -726,8 +695,7 @@ 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. @@ -744,8 +712,7 @@ Args: } ``` -### **Run go mod vendor** -Identifier: `gopls.vendor` +## `gopls.vendor`: **Run go mod vendor** Runs `go mod vendor` for a module. @@ -758,8 +725,7 @@ 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. @@ -775,8 +741,7 @@ 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/generate/generate.go b/gopls/doc/generate/generate.go index f49a787888a..33735270a9c 100644 --- a/gopls/doc/generate/generate.go +++ b/gopls/doc/generate/generate.go @@ -40,7 +40,6 @@ import ( "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" @@ -170,10 +169,6 @@ func loadAPI() (*doc.API, error) { 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 = loadHints(golang.AllInlayHints) for _, category := range []reflect.Value{ reflect.ValueOf(defaults.UserOptions), @@ -707,8 +702,6 @@ 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, @@ -720,7 +713,7 @@ func rewriteSettings(prevContent []byte, api *doc.API) ([]byte, error) { // // 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` *%v*\n\n", opt.Name, opt.Type) // status switch opt.Status { @@ -848,7 +841,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 +852,7 @@ 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) + 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/settings.md b/gopls/doc/settings.md index a3c5bb5ddeb..83f6280a70a 100644 --- a/gopls/doc/settings.md +++ b/gopls/doc/settings.md @@ -48,7 +48,7 @@ All clients but VS Code should use the short form. ## Build -### ⬤ `buildFlags` *[]string* +### `buildFlags` *[]string* buildFlags is the set of flags passed on to the build system when invoked. It is applied to queries like `go list`, which is used when discovering files. @@ -57,14 +57,14 @@ The most common use is to set `-tags`. Default: `[]`. -### ⬤ `env` *map[string]string* +### `env` *map[string]string* env adds environment variables to external commands run by `gopls`, most notably `go list`. Default: `{}`. -### ⬤ `directoryFilters` *[]string* +### `directoryFilters` *[]string* directoryFilters can be used to exclude unwanted directories from the workspace. By default, all directories are included. Filters are an @@ -88,7 +88,7 @@ Include only project_a, but not node_modules inside it: `-`, `+project_a`, `-pro Default: `["-**/node_modules"]`. -### ⬤ `templateExtensions` *[]string* +### `templateExtensions` *[]string* templateExtensions gives the extensions of file names that are treateed as template files. (The extension @@ -97,7 +97,7 @@ is the part of the file name after the final dot.) Default: `[]`. -### ⬤ `memoryMode` *string* +### `memoryMode` *string* **This setting is experimental and may be deleted.** @@ -106,7 +106,7 @@ obsolete, no effect Default: `""`. -### ⬤ `expandWorkspaceToModule` *bool* +### `expandWorkspaceToModule` *bool* **This setting is experimental and may be deleted.** @@ -122,7 +122,7 @@ gopls has to do to keep your workspace up to date. Default: `true`. -### ⬤ `allowImplicitNetworkAccess` *bool* +### `allowImplicitNetworkAccess` *bool* **This setting is experimental and may be deleted.** @@ -133,7 +133,7 @@ be removed. Default: `false`. -### ⬤ `standaloneTags` *[]string* +### `standaloneTags` *[]string* standaloneTags specifies a set of build constraints that identify individual Go source files that make up the entire main package of an @@ -160,7 +160,7 @@ Default: `["ignore"]`. ## Formatting -### ⬤ `local` *string* +### `local` *string* local is the equivalent of the `goimports -local` flag, which puts imports beginning with this string after third-party packages. It should @@ -170,7 +170,7 @@ separately. Default: `""`. -### ⬤ `gofumpt` *bool* +### `gofumpt` *bool* gofumpt indicates if we should run gofumpt formatting. @@ -180,7 +180,7 @@ Default: `false`. ## UI -### ⬤ `codelenses` *map[golang.org/x/tools/gopls/internal/protocol.CodeLensSource]bool* +### `codelenses` *map[golang.org/x/tools/gopls/internal/protocol.CodeLensSource]bool* codelenses overrides the enabled/disabled state of each of gopls' sources of [Code Lenses](codelenses.md). @@ -201,7 +201,7 @@ Example Usage: Default: `{"gc_details":false,"generate":true,"regenerate_cgo":true,"run_govulncheck":false,"tidy":true,"upgrade_dependency":true,"vendor":true}`. -### ⬤ `semanticTokens` *bool* +### `semanticTokens` *bool* **This setting is experimental and may be deleted.** @@ -211,7 +211,7 @@ semantic tokens to the client. Default: `false`. -### ⬤ `noSemanticString` *bool* +### `noSemanticString` *bool* **This setting is experimental and may be deleted.** @@ -220,7 +220,7 @@ noSemanticString turns off the sending of the semantic token 'string' Default: `false`. -### ⬤ `noSemanticNumber` *bool* +### `noSemanticNumber` *bool* **This setting is experimental and may be deleted.** @@ -232,7 +232,7 @@ Default: `false`. ## Completion -### ⬤ `usePlaceholders` *bool* +### `usePlaceholders` *bool* placeholders enables placeholders for function parameters or struct fields in completion responses. @@ -240,7 +240,7 @@ fields in completion responses. Default: `false`. -### ⬤ `completionBudget` *time.Duration* +### `completionBudget` *time.Duration* **This setting is for debugging purposes only.** @@ -253,7 +253,7 @@ results. Zero means unlimited. Default: `"100ms"`. -### ⬤ `matcher` *enum* +### `matcher` *enum* **This is an advanced setting and should not be configured by most `gopls` users.** @@ -269,7 +269,7 @@ Must be one of: Default: `"Fuzzy"`. -### ⬤ `experimentalPostfixCompletions` *bool* +### `experimentalPostfixCompletions` *bool* **This setting is experimental and may be deleted.** @@ -279,7 +279,7 @@ such as "someSlice.sort!". Default: `true`. -### ⬤ `completeFunctionCalls` *bool* +### `completeFunctionCalls` *bool* completeFunctionCalls enables function call completion. @@ -293,7 +293,7 @@ Default: `true`. ## Diagnostic -### ⬤ `analyses` *map[string]bool* +### `analyses` *map[string]bool* analyses specify analyses that the user would like to enable or disable. A map of the names of analysis passes that should be enabled/disabled. @@ -314,7 +314,7 @@ Example Usage: Default: `{}`. -### ⬤ `staticcheck` *bool* +### `staticcheck` *bool* **This setting is experimental and may be deleted.** @@ -325,7 +325,7 @@ These analyses are documented on Default: `false`. -### ⬤ `annotations` *map[string]bool* +### `annotations` *map[string]bool* **This setting is experimental and may be deleted.** @@ -342,7 +342,7 @@ Can contain any of: Default: `{"bounds":true,"escape":true,"inline":true,"nil":true}`. -### ⬤ `vulncheck` *enum* +### `vulncheck` *enum* **This setting is experimental and may be deleted.** @@ -357,7 +357,7 @@ directly and indirectly used by the analyzed main module. Default: `"Off"`. -### ⬤ `diagnosticsDelay` *time.Duration* +### `diagnosticsDelay` *time.Duration* **This is an advanced setting and should not be configured by most `gopls` users.** @@ -371,7 +371,7 @@ This option must be set to a valid duration string, for example `"250ms"`. Default: `"1s"`. -### ⬤ `diagnosticsTrigger` *enum* +### `diagnosticsTrigger` *enum* **This setting is experimental and may be deleted.** @@ -386,7 +386,7 @@ or configuration change will still trigger diagnostics. Default: `"Edit"`. -### ⬤ `analysisProgressReporting` *bool* +### `analysisProgressReporting` *bool* analysisProgressReporting controls whether gopls sends progress notifications when construction of its index of analysis facts is taking a @@ -404,7 +404,7 @@ Default: `true`. ## Documentation -### ⬤ `hoverKind` *enum* +### `hoverKind` *enum* hoverKind controls the information that appears in the hover text. SingleLine and Structured are intended for use only by authors of editor plugins. @@ -423,7 +423,7 @@ This should only be used by clients that support this behavior. Default: `"FullDocumentation"`. -### ⬤ `linkTarget` *string* +### `linkTarget` *string* linkTarget controls where documentation links go. It might be one of: @@ -439,7 +439,7 @@ documentation links in hover. Default: `"pkg.go.dev"`. -### ⬤ `linksInHover` *bool* +### `linksInHover` *bool* linksInHover toggles the presence of links to documentation in hover. @@ -449,7 +449,7 @@ Default: `true`. ## Inlayhint -### ⬤ `hints` *map[string]bool* +### `hints` *map[string]bool* **This setting is experimental and may be deleted.** @@ -463,7 +463,7 @@ Default: `{}`. ## Navigation -### ⬤ `importShortcut` *enum* +### `importShortcut` *enum* importShortcut specifies whether import statements should link to documentation or go to definitions. @@ -477,7 +477,7 @@ Must be one of: Default: `"Both"`. -### ⬤ `symbolMatcher` *enum* +### `symbolMatcher` *enum* **This is an advanced setting and should not be configured by most `gopls` users.** @@ -493,7 +493,7 @@ Must be one of: Default: `"FastFuzzy"`. -### ⬤ `symbolStyle` *enum* +### `symbolStyle` *enum* **This is an advanced setting and should not be configured by most `gopls` users.** @@ -523,7 +523,7 @@ just "Foo.Field". Default: `"Dynamic"`. -### ⬤ `symbolScope` *enum* +### `symbolScope` *enum* symbolScope controls which packages are searched for workspace/symbol requests. When the scope is "workspace", gopls searches only workspace @@ -539,7 +539,7 @@ dependencies. Default: `"all"`. -### ⬤ `verboseOutput` *bool* +### `verboseOutput` *bool* **This setting is for debugging purposes only.** diff --git a/gopls/internal/cmd/execute.go b/gopls/internal/cmd/execute.go index 485e6d03ca9..0797c9f2420 100644 --- a/gopls/internal/cmd/execute.go +++ b/gopls/internal/cmd/execute.go @@ -11,7 +11,6 @@ import ( "fmt" "log" "os" - "strings" "golang.org/x/tools/gopls/internal/protocol" "golang.org/x/tools/gopls/internal/protocol/command" @@ -58,7 +57,7 @@ func (e *execute) Run(ctx context.Context, args ...string) error { return tool.CommandLineErrorf("execute requires a command name") } cmd := args[0] - if !slices.Contains(command.Commands, command.Command(strings.TrimPrefix(cmd, "gopls."))) { + if !slices.Contains(command.Commands, command.Command(cmd)) { return tool.CommandLineErrorf("unrecognized command: %s", cmd) } @@ -127,8 +126,7 @@ func (conn *connection) executeCommand(ctx context.Context, cmd *protocol.Comman // Some commands are asynchronous, so clients // must wait for the "end" progress notification. - enum := command.Command(strings.TrimPrefix(cmd.Command, "gopls.")) - if enum.IsAsync() { + if command.Command(cmd.Command).IsAsync() { status := <-endStatus if status != server.CommandCompleted { return nil, fmt.Errorf("command %s", status) diff --git a/gopls/internal/cmd/remote.go b/gopls/internal/cmd/remote.go index 8de4365fc9d..ae4aa55ab61 100644 --- a/gopls/internal/cmd/remote.go +++ b/gopls/internal/cmd/remote.go @@ -151,7 +151,7 @@ func (c *startDebugging) Run(ctx context.Context, args ...string) error { Addr: debugAddr, } var result command.DebuggingResult - if err := lsprpc.ExecuteCommand(ctx, remote, command.StartDebugging.ID(), debugArgs, &result); err != nil { + if err := lsprpc.ExecuteCommand(ctx, remote, command.StartDebugging.String(), debugArgs, &result); err != nil { return err } if len(result.URLs) == 0 { diff --git a/gopls/internal/cmd/stats.go b/gopls/internal/cmd/stats.go index a78880107e7..cc19a94fb84 100644 --- a/gopls/internal/cmd/stats.go +++ b/gopls/internal/cmd/stats.go @@ -128,7 +128,7 @@ func (s *stats) Run(ctx context.Context, args ...string) error { if _, err := do("Querying memstats", func() error { memStats, err := conn.executeCommand(ctx, &protocol.Command{ - Command: command.MemStats.ID(), + Command: command.MemStats.String(), }) if err != nil { return err @@ -141,7 +141,7 @@ func (s *stats) Run(ctx context.Context, args ...string) error { if _, err := do("Querying workspace stats", func() error { wsStats, err := conn.executeCommand(ctx, &protocol.Command{ - Command: command.WorkspaceStats.ID(), + Command: command.WorkspaceStats.String(), }) if err != nil { return err diff --git a/gopls/internal/doc/api.go b/gopls/internal/doc/api.go index 54bd9178b76..62e6b5b79e5 100644 --- a/gopls/internal/doc/api.go +++ b/gopls/internal/doc/api.go @@ -55,7 +55,7 @@ type EnumValue struct { } type Command struct { - Command string + Command string // e.g. "gopls.run_tests" Title string Doc string ArgDoc string diff --git a/gopls/internal/lsprpc/lsprpc.go b/gopls/internal/lsprpc/lsprpc.go index ceb47aa6c43..b77557c9a4b 100644 --- a/gopls/internal/lsprpc/lsprpc.go +++ b/gopls/internal/lsprpc/lsprpc.go @@ -290,7 +290,7 @@ func (f *forwarder) handler(handler jsonrpc2.Handler) jsonrpc2.Handler { case "workspace/executeCommand": var params protocol.ExecuteCommandParams if err := json.Unmarshal(r.Params(), ¶ms); err == nil { - if params.Command == command.StartDebugging.ID() { + if params.Command == command.StartDebugging.String() { var args command.DebuggingArgs if err := command.UnmarshalArgs(params.Arguments, &args); err == nil { reply = f.replyWithDebugAddress(ctx, reply, args) diff --git a/gopls/internal/protocol/command/command_gen.go b/gopls/internal/protocol/command/command_gen.go index fe4b7d3d1b9..5e267afdfe9 100644 --- a/gopls/internal/protocol/command/command_gen.go +++ b/gopls/internal/protocol/command/command_gen.go @@ -18,45 +18,47 @@ import ( "golang.org/x/tools/gopls/internal/protocol" ) -// Symbolic names for gopls commands, excluding "gopls." prefix. -// These commands may be requested by ExecuteCommand, CodeLens, -// CodeAction, and other LSP requests. +// Symbolic names for gopls commands, corresponding to methods of [Interface]. +// +// The string value is used in the Command field of protocol.Command. +// These commands may be obtained from a CodeLens or CodeAction request +// and executed by an ExecuteCommand request. const ( - AddDependency Command = "add_dependency" - AddImport Command = "add_import" - AddTelemetryCounters Command = "add_telemetry_counters" - ApplyFix Command = "apply_fix" - ChangeSignature Command = "change_signature" - CheckUpgrades Command = "check_upgrades" - DiagnoseFiles Command = "diagnose_files" - Doc Command = "doc" - EditGoDirective Command = "edit_go_directive" - FetchVulncheckResult Command = "fetch_vulncheck_result" - FreeSymbols Command = "free_symbols" - GCDetails Command = "gc_details" - Generate Command = "generate" - GoGetPackage Command = "go_get_package" - ListImports Command = "list_imports" - ListKnownPackages Command = "list_known_packages" - MaybePromptForTelemetry Command = "maybe_prompt_for_telemetry" - MemStats Command = "mem_stats" - RegenerateCgo Command = "regenerate_cgo" - RemoveDependency Command = "remove_dependency" - ResetGoModDiagnostics Command = "reset_go_mod_diagnostics" - RunGoWorkCommand Command = "run_go_work_command" - RunGovulncheck Command = "run_govulncheck" - RunTests Command = "run_tests" - StartDebugging Command = "start_debugging" - StartProfile Command = "start_profile" - StopProfile Command = "stop_profile" - Test Command = "test" - Tidy Command = "tidy" - ToggleGCDetails Command = "toggle_gc_details" - UpdateGoSum Command = "update_go_sum" - UpgradeDependency Command = "upgrade_dependency" - Vendor Command = "vendor" - Views Command = "views" - WorkspaceStats Command = "workspace_stats" + AddDependency Command = "gopls.add_dependency" + AddImport Command = "gopls.add_import" + AddTelemetryCounters Command = "gopls.add_telemetry_counters" + ApplyFix Command = "gopls.apply_fix" + ChangeSignature Command = "gopls.change_signature" + CheckUpgrades Command = "gopls.check_upgrades" + DiagnoseFiles Command = "gopls.diagnose_files" + Doc Command = "gopls.doc" + EditGoDirective Command = "gopls.edit_go_directive" + FetchVulncheckResult Command = "gopls.fetch_vulncheck_result" + FreeSymbols Command = "gopls.free_symbols" + GCDetails Command = "gopls.gc_details" + Generate Command = "gopls.generate" + GoGetPackage Command = "gopls.go_get_package" + ListImports Command = "gopls.list_imports" + ListKnownPackages Command = "gopls.list_known_packages" + MaybePromptForTelemetry Command = "gopls.maybe_prompt_for_telemetry" + MemStats Command = "gopls.mem_stats" + RegenerateCgo Command = "gopls.regenerate_cgo" + RemoveDependency Command = "gopls.remove_dependency" + ResetGoModDiagnostics Command = "gopls.reset_go_mod_diagnostics" + RunGoWorkCommand Command = "gopls.run_go_work_command" + RunGovulncheck Command = "gopls.run_govulncheck" + RunTests Command = "gopls.run_tests" + StartDebugging Command = "gopls.start_debugging" + StartProfile Command = "gopls.start_profile" + StopProfile Command = "gopls.stop_profile" + Test Command = "gopls.test" + Tidy Command = "gopls.tidy" + ToggleGCDetails Command = "gopls.toggle_gc_details" + UpdateGoSum Command = "gopls.update_go_sum" + UpgradeDependency Command = "gopls.upgrade_dependency" + Vendor Command = "gopls.vendor" + Views Command = "gopls.views" + WorkspaceStats Command = "gopls.workspace_stats" ) var Commands = []Command{ @@ -98,163 +100,163 @@ var Commands = []Command{ } func Dispatch(ctx context.Context, params *protocol.ExecuteCommandParams, s Interface) (interface{}, error) { - switch params.Command { - case "gopls.add_dependency": + switch Command(params.Command) { + case AddDependency: var a0 DependencyArgs if err := UnmarshalArgs(params.Arguments, &a0); err != nil { return nil, err } return nil, s.AddDependency(ctx, a0) - case "gopls.add_import": + case AddImport: var a0 AddImportArgs if err := UnmarshalArgs(params.Arguments, &a0); err != nil { return nil, err } return nil, s.AddImport(ctx, a0) - case "gopls.add_telemetry_counters": + case AddTelemetryCounters: var a0 AddTelemetryCountersArgs if err := UnmarshalArgs(params.Arguments, &a0); err != nil { return nil, err } return nil, s.AddTelemetryCounters(ctx, a0) - case "gopls.apply_fix": + case ApplyFix: var a0 ApplyFixArgs if err := UnmarshalArgs(params.Arguments, &a0); err != nil { return nil, err } return s.ApplyFix(ctx, a0) - case "gopls.change_signature": + case ChangeSignature: var a0 ChangeSignatureArgs if err := UnmarshalArgs(params.Arguments, &a0); err != nil { return nil, err } return s.ChangeSignature(ctx, a0) - case "gopls.check_upgrades": + case CheckUpgrades: var a0 CheckUpgradesArgs if err := UnmarshalArgs(params.Arguments, &a0); err != nil { return nil, err } return nil, s.CheckUpgrades(ctx, a0) - case "gopls.diagnose_files": + case DiagnoseFiles: var a0 DiagnoseFilesArgs if err := UnmarshalArgs(params.Arguments, &a0); err != nil { return nil, err } return nil, s.DiagnoseFiles(ctx, a0) - case "gopls.doc": + case Doc: var a0 protocol.Location if err := UnmarshalArgs(params.Arguments, &a0); err != nil { return nil, err } return nil, s.Doc(ctx, a0) - case "gopls.edit_go_directive": + case EditGoDirective: var a0 EditGoDirectiveArgs if err := UnmarshalArgs(params.Arguments, &a0); err != nil { return nil, err } return nil, s.EditGoDirective(ctx, a0) - case "gopls.fetch_vulncheck_result": + case FetchVulncheckResult: var a0 URIArg if err := UnmarshalArgs(params.Arguments, &a0); err != nil { return nil, err } return s.FetchVulncheckResult(ctx, a0) - case "gopls.free_symbols": + case FreeSymbols: var a0 protocol.DocumentURI var a1 protocol.Range if err := UnmarshalArgs(params.Arguments, &a0, &a1); err != nil { return nil, err } return nil, s.FreeSymbols(ctx, a0, a1) - case "gopls.gc_details": + case GCDetails: var a0 protocol.DocumentURI if err := UnmarshalArgs(params.Arguments, &a0); err != nil { return nil, err } return nil, s.GCDetails(ctx, a0) - case "gopls.generate": + case Generate: var a0 GenerateArgs if err := UnmarshalArgs(params.Arguments, &a0); err != nil { return nil, err } return nil, s.Generate(ctx, a0) - case "gopls.go_get_package": + case GoGetPackage: var a0 GoGetPackageArgs if err := UnmarshalArgs(params.Arguments, &a0); err != nil { return nil, err } return nil, s.GoGetPackage(ctx, a0) - case "gopls.list_imports": + case ListImports: var a0 URIArg if err := UnmarshalArgs(params.Arguments, &a0); err != nil { return nil, err } return s.ListImports(ctx, a0) - case "gopls.list_known_packages": + case ListKnownPackages: var a0 URIArg if err := UnmarshalArgs(params.Arguments, &a0); err != nil { return nil, err } return s.ListKnownPackages(ctx, a0) - case "gopls.maybe_prompt_for_telemetry": + case MaybePromptForTelemetry: return nil, s.MaybePromptForTelemetry(ctx) - case "gopls.mem_stats": + case MemStats: return s.MemStats(ctx) - case "gopls.regenerate_cgo": + case RegenerateCgo: var a0 URIArg if err := UnmarshalArgs(params.Arguments, &a0); err != nil { return nil, err } return nil, s.RegenerateCgo(ctx, a0) - case "gopls.remove_dependency": + case RemoveDependency: var a0 RemoveDependencyArgs if err := UnmarshalArgs(params.Arguments, &a0); err != nil { return nil, err } return nil, s.RemoveDependency(ctx, a0) - case "gopls.reset_go_mod_diagnostics": + case ResetGoModDiagnostics: var a0 ResetGoModDiagnosticsArgs if err := UnmarshalArgs(params.Arguments, &a0); err != nil { return nil, err } return nil, s.ResetGoModDiagnostics(ctx, a0) - case "gopls.run_go_work_command": + case RunGoWorkCommand: var a0 RunGoWorkArgs if err := UnmarshalArgs(params.Arguments, &a0); err != nil { return nil, err } return nil, s.RunGoWorkCommand(ctx, a0) - case "gopls.run_govulncheck": + case RunGovulncheck: var a0 VulncheckArgs if err := UnmarshalArgs(params.Arguments, &a0); err != nil { return nil, err } return s.RunGovulncheck(ctx, a0) - case "gopls.run_tests": + case RunTests: var a0 RunTestsArgs if err := UnmarshalArgs(params.Arguments, &a0); err != nil { return nil, err } return nil, s.RunTests(ctx, a0) - case "gopls.start_debugging": + case StartDebugging: var a0 DebuggingArgs if err := UnmarshalArgs(params.Arguments, &a0); err != nil { return nil, err } return s.StartDebugging(ctx, a0) - case "gopls.start_profile": + case StartProfile: var a0 StartProfileArgs if err := UnmarshalArgs(params.Arguments, &a0); err != nil { return nil, err } return s.StartProfile(ctx, a0) - case "gopls.stop_profile": + case StopProfile: var a0 StopProfileArgs if err := UnmarshalArgs(params.Arguments, &a0); err != nil { return nil, err } return s.StopProfile(ctx, a0) - case "gopls.test": + case Test: var a0 protocol.DocumentURI var a1 []string var a2 []string @@ -262,39 +264,39 @@ func Dispatch(ctx context.Context, params *protocol.ExecuteCommandParams, s Inte return nil, err } return nil, s.Test(ctx, a0, a1, a2) - case "gopls.tidy": + case Tidy: var a0 URIArgs if err := UnmarshalArgs(params.Arguments, &a0); err != nil { return nil, err } return nil, s.Tidy(ctx, a0) - case "gopls.toggle_gc_details": + case ToggleGCDetails: var a0 URIArg if err := UnmarshalArgs(params.Arguments, &a0); err != nil { return nil, err } return nil, s.ToggleGCDetails(ctx, a0) - case "gopls.update_go_sum": + case UpdateGoSum: var a0 URIArgs if err := UnmarshalArgs(params.Arguments, &a0); err != nil { return nil, err } return nil, s.UpdateGoSum(ctx, a0) - case "gopls.upgrade_dependency": + case UpgradeDependency: var a0 DependencyArgs if err := UnmarshalArgs(params.Arguments, &a0); err != nil { return nil, err } return nil, s.UpgradeDependency(ctx, a0) - case "gopls.vendor": + case Vendor: var a0 URIArg if err := UnmarshalArgs(params.Arguments, &a0); err != nil { return nil, err } return nil, s.Vendor(ctx, a0) - case "gopls.views": + case Views: return s.Views(ctx) - case "gopls.workspace_stats": + case WorkspaceStats: return s.WorkspaceStats(ctx) } return nil, fmt.Errorf("unsupported command %q", params.Command) @@ -307,7 +309,7 @@ func NewAddDependencyCommand(title string, a0 DependencyArgs) (protocol.Command, } return protocol.Command{ Title: title, - Command: "gopls.add_dependency", + Command: AddDependency.String(), Arguments: args, }, nil } @@ -319,7 +321,7 @@ func NewAddImportCommand(title string, a0 AddImportArgs) (protocol.Command, erro } return protocol.Command{ Title: title, - Command: "gopls.add_import", + Command: AddImport.String(), Arguments: args, }, nil } @@ -331,7 +333,7 @@ func NewAddTelemetryCountersCommand(title string, a0 AddTelemetryCountersArgs) ( } return protocol.Command{ Title: title, - Command: "gopls.add_telemetry_counters", + Command: AddTelemetryCounters.String(), Arguments: args, }, nil } @@ -343,7 +345,7 @@ func NewApplyFixCommand(title string, a0 ApplyFixArgs) (protocol.Command, error) } return protocol.Command{ Title: title, - Command: "gopls.apply_fix", + Command: ApplyFix.String(), Arguments: args, }, nil } @@ -355,7 +357,7 @@ func NewChangeSignatureCommand(title string, a0 ChangeSignatureArgs) (protocol.C } return protocol.Command{ Title: title, - Command: "gopls.change_signature", + Command: ChangeSignature.String(), Arguments: args, }, nil } @@ -367,7 +369,7 @@ func NewCheckUpgradesCommand(title string, a0 CheckUpgradesArgs) (protocol.Comma } return protocol.Command{ Title: title, - Command: "gopls.check_upgrades", + Command: CheckUpgrades.String(), Arguments: args, }, nil } @@ -379,7 +381,7 @@ func NewDiagnoseFilesCommand(title string, a0 DiagnoseFilesArgs) (protocol.Comma } return protocol.Command{ Title: title, - Command: "gopls.diagnose_files", + Command: DiagnoseFiles.String(), Arguments: args, }, nil } @@ -391,7 +393,7 @@ func NewDocCommand(title string, a0 protocol.Location) (protocol.Command, error) } return protocol.Command{ Title: title, - Command: "gopls.doc", + Command: Doc.String(), Arguments: args, }, nil } @@ -403,7 +405,7 @@ func NewEditGoDirectiveCommand(title string, a0 EditGoDirectiveArgs) (protocol.C } return protocol.Command{ Title: title, - Command: "gopls.edit_go_directive", + Command: EditGoDirective.String(), Arguments: args, }, nil } @@ -415,7 +417,7 @@ func NewFetchVulncheckResultCommand(title string, a0 URIArg) (protocol.Command, } return protocol.Command{ Title: title, - Command: "gopls.fetch_vulncheck_result", + Command: FetchVulncheckResult.String(), Arguments: args, }, nil } @@ -427,7 +429,7 @@ func NewFreeSymbolsCommand(title string, a0 protocol.DocumentURI, a1 protocol.Ra } return protocol.Command{ Title: title, - Command: "gopls.free_symbols", + Command: FreeSymbols.String(), Arguments: args, }, nil } @@ -439,7 +441,7 @@ func NewGCDetailsCommand(title string, a0 protocol.DocumentURI) (protocol.Comman } return protocol.Command{ Title: title, - Command: "gopls.gc_details", + Command: GCDetails.String(), Arguments: args, }, nil } @@ -451,7 +453,7 @@ func NewGenerateCommand(title string, a0 GenerateArgs) (protocol.Command, error) } return protocol.Command{ Title: title, - Command: "gopls.generate", + Command: Generate.String(), Arguments: args, }, nil } @@ -463,7 +465,7 @@ func NewGoGetPackageCommand(title string, a0 GoGetPackageArgs) (protocol.Command } return protocol.Command{ Title: title, - Command: "gopls.go_get_package", + Command: GoGetPackage.String(), Arguments: args, }, nil } @@ -475,7 +477,7 @@ func NewListImportsCommand(title string, a0 URIArg) (protocol.Command, error) { } return protocol.Command{ Title: title, - Command: "gopls.list_imports", + Command: ListImports.String(), Arguments: args, }, nil } @@ -487,7 +489,7 @@ func NewListKnownPackagesCommand(title string, a0 URIArg) (protocol.Command, err } return protocol.Command{ Title: title, - Command: "gopls.list_known_packages", + Command: ListKnownPackages.String(), Arguments: args, }, nil } @@ -499,7 +501,7 @@ func NewMaybePromptForTelemetryCommand(title string) (protocol.Command, error) { } return protocol.Command{ Title: title, - Command: "gopls.maybe_prompt_for_telemetry", + Command: MaybePromptForTelemetry.String(), Arguments: args, }, nil } @@ -511,7 +513,7 @@ func NewMemStatsCommand(title string) (protocol.Command, error) { } return protocol.Command{ Title: title, - Command: "gopls.mem_stats", + Command: MemStats.String(), Arguments: args, }, nil } @@ -523,7 +525,7 @@ func NewRegenerateCgoCommand(title string, a0 URIArg) (protocol.Command, error) } return protocol.Command{ Title: title, - Command: "gopls.regenerate_cgo", + Command: RegenerateCgo.String(), Arguments: args, }, nil } @@ -535,7 +537,7 @@ func NewRemoveDependencyCommand(title string, a0 RemoveDependencyArgs) (protocol } return protocol.Command{ Title: title, - Command: "gopls.remove_dependency", + Command: RemoveDependency.String(), Arguments: args, }, nil } @@ -547,7 +549,7 @@ func NewResetGoModDiagnosticsCommand(title string, a0 ResetGoModDiagnosticsArgs) } return protocol.Command{ Title: title, - Command: "gopls.reset_go_mod_diagnostics", + Command: ResetGoModDiagnostics.String(), Arguments: args, }, nil } @@ -559,7 +561,7 @@ func NewRunGoWorkCommandCommand(title string, a0 RunGoWorkArgs) (protocol.Comman } return protocol.Command{ Title: title, - Command: "gopls.run_go_work_command", + Command: RunGoWorkCommand.String(), Arguments: args, }, nil } @@ -571,7 +573,7 @@ func NewRunGovulncheckCommand(title string, a0 VulncheckArgs) (protocol.Command, } return protocol.Command{ Title: title, - Command: "gopls.run_govulncheck", + Command: RunGovulncheck.String(), Arguments: args, }, nil } @@ -583,7 +585,7 @@ func NewRunTestsCommand(title string, a0 RunTestsArgs) (protocol.Command, error) } return protocol.Command{ Title: title, - Command: "gopls.run_tests", + Command: RunTests.String(), Arguments: args, }, nil } @@ -595,7 +597,7 @@ func NewStartDebuggingCommand(title string, a0 DebuggingArgs) (protocol.Command, } return protocol.Command{ Title: title, - Command: "gopls.start_debugging", + Command: StartDebugging.String(), Arguments: args, }, nil } @@ -607,7 +609,7 @@ func NewStartProfileCommand(title string, a0 StartProfileArgs) (protocol.Command } return protocol.Command{ Title: title, - Command: "gopls.start_profile", + Command: StartProfile.String(), Arguments: args, }, nil } @@ -619,7 +621,7 @@ func NewStopProfileCommand(title string, a0 StopProfileArgs) (protocol.Command, } return protocol.Command{ Title: title, - Command: "gopls.stop_profile", + Command: StopProfile.String(), Arguments: args, }, nil } @@ -631,7 +633,7 @@ func NewTestCommand(title string, a0 protocol.DocumentURI, a1 []string, a2 []str } return protocol.Command{ Title: title, - Command: "gopls.test", + Command: Test.String(), Arguments: args, }, nil } @@ -643,7 +645,7 @@ func NewTidyCommand(title string, a0 URIArgs) (protocol.Command, error) { } return protocol.Command{ Title: title, - Command: "gopls.tidy", + Command: Tidy.String(), Arguments: args, }, nil } @@ -655,7 +657,7 @@ func NewToggleGCDetailsCommand(title string, a0 URIArg) (protocol.Command, error } return protocol.Command{ Title: title, - Command: "gopls.toggle_gc_details", + Command: ToggleGCDetails.String(), Arguments: args, }, nil } @@ -667,7 +669,7 @@ func NewUpdateGoSumCommand(title string, a0 URIArgs) (protocol.Command, error) { } return protocol.Command{ Title: title, - Command: "gopls.update_go_sum", + Command: UpdateGoSum.String(), Arguments: args, }, nil } @@ -679,7 +681,7 @@ func NewUpgradeDependencyCommand(title string, a0 DependencyArgs) (protocol.Comm } return protocol.Command{ Title: title, - Command: "gopls.upgrade_dependency", + Command: UpgradeDependency.String(), Arguments: args, }, nil } @@ -691,7 +693,7 @@ func NewVendorCommand(title string, a0 URIArg) (protocol.Command, error) { } return protocol.Command{ Title: title, - Command: "gopls.vendor", + Command: Vendor.String(), Arguments: args, }, nil } @@ -703,7 +705,7 @@ func NewViewsCommand(title string) (protocol.Command, error) { } return protocol.Command{ Title: title, - Command: "gopls.views", + Command: Views.String(), Arguments: args, }, nil } @@ -715,7 +717,7 @@ func NewWorkspaceStatsCommand(title string) (protocol.Command, error) { } return protocol.Command{ Title: title, - Command: "gopls.workspace_stats", + Command: WorkspaceStats.String(), Arguments: args, }, nil } diff --git a/gopls/internal/protocol/command/commandmeta/meta.go b/gopls/internal/protocol/command/commandmeta/meta.go index db66bb32e9e..d9a5d27c361 100644 --- a/gopls/internal/protocol/command/commandmeta/meta.go +++ b/gopls/internal/protocol/command/commandmeta/meta.go @@ -23,19 +23,14 @@ import ( // A Command describes a workspace/executeCommand extension command. type Command struct { - MethodName string - Name string - // TODO(rFindley): I think Title can actually be eliminated. In all cases - // where we use it, there is probably a more appropriate contextual title. - Title string - Doc string - Args []*Field - Result *Field + MethodName string // e.g. "RunTests" + Name string // e.g. "gopls.run_tests" + Title string + Doc string + Args []*Field + Result *Field } -// (used by the ../command/gen template) -func (c *Command) ID() string { return "gopls." + c.Name } - type Field struct { Name string Doc string @@ -208,7 +203,7 @@ func lspName(methodName string) string { for i := range words { words[i] = strings.ToLower(words[i]) } - return strings.Join(words, "_") + return "gopls." + strings.Join(words, "_") } // splitCamel splits s into words, according to camel-case word boundaries. diff --git a/gopls/internal/protocol/command/gen/gen.go b/gopls/internal/protocol/command/gen/gen.go index fadb12ae2ed..8ae50f611c7 100644 --- a/gopls/internal/protocol/command/gen/gen.go +++ b/gopls/internal/protocol/command/gen/gen.go @@ -35,9 +35,11 @@ import ( {{end}} ) -// Symbolic names for gopls commands, excluding "gopls." prefix. -// These commands may be requested by ExecuteCommand, CodeLens, -// CodeAction, and other LSP requests. +// Symbolic names for gopls commands, corresponding to methods of [Interface]. +// +// The string value is used in the Command field of protocol.Command. +// These commands may be obtained from a CodeLens or CodeAction request +// and executed by an ExecuteCommand request. const ( {{- range .Commands}} {{.MethodName}} Command = "{{.Name}}" @@ -51,9 +53,9 @@ var Commands = []Command { } func Dispatch(ctx context.Context, params *protocol.ExecuteCommandParams, s Interface) (interface{}, error) { - switch params.Command { + switch Command(params.Command) { {{- range .Commands}} - case "{{.ID}}": + case {{.MethodName}}: {{- if .Args -}} {{- range $i, $v := .Args}} var a{{$i}} {{typeString $v.Type}} @@ -76,7 +78,7 @@ func New{{.MethodName}}Command(title string, {{range $i, $v := .Args}}{{if $i}}, } return protocol.Command{ Title: title, - Command: "{{.ID}}", + Command: {{.MethodName}}.String(), Arguments: args, }, nil } diff --git a/gopls/internal/protocol/command/interface.go b/gopls/internal/protocol/command/interface.go index 0934cf22dc4..e8d6d7dbac9 100644 --- a/gopls/internal/protocol/command/interface.go +++ b/gopls/internal/protocol/command/interface.go @@ -29,9 +29,8 @@ import ( // 1. All method arguments must be JSON serializable. // 2. Methods must return either error or (T, error), where T is a // JSON serializable type. -// 3. The first line of the doc string is special. Everything after the colon -// is considered the command 'Title'. -// TODO(rFindley): reconsider this -- Title may be unnecessary. +// 3. The first line of the doc string is special. +// Everything after the colon is considered the command 'Title'. // // The doc comment on each method is eventually published at // https://github.com/golang/tools/blob/master/gopls/doc/commands.md, diff --git a/gopls/internal/protocol/command/util.go b/gopls/internal/protocol/command/util.go index 0d9799f5dc0..b403679fc9a 100644 --- a/gopls/internal/protocol/command/util.go +++ b/gopls/internal/protocol/command/util.go @@ -9,35 +9,24 @@ import ( "fmt" ) -// TODO(adonovan): use the "gopls."-prefix forms everywhere. The -// existence of two forms is a nuisance. I think it should be a -// straightforward fix, as all public APIs and docs use that form -// already. - -// ID returns the command name for use in the LSP. -func ID(name string) string { - return "gopls." + name -} - +// A Command identifies one of gopls' ad-hoc extension commands +// that may be invoked through LSP's executeCommand. type Command string +func (c Command) String() string { return string(c) } + // IsAsync reports whether the command is asynchronous: // clients must wait for the "end" progress notification. func (c Command) IsAsync() bool { switch string(c) { // TODO(adonovan): derive this list from interface.go somewhow. // Unfortunately we can't even reference the enum from here... - case "run_tests", "run_govulncheck", "test": + case "gopls.run_tests", "gopls.run_govulncheck", "gopls.test": return true } return false } -// ID returns the command identifier to use in the executeCommand request. -func (c Command) ID() string { - return ID(string(c)) -} - // MarshalArgs encodes the given arguments to json.RawMessages. This function // is used to construct arguments to a protocol.Command. // diff --git a/gopls/internal/server/command.go b/gopls/internal/server/command.go index ad2eb9501d5..0a64d1cd1be 100644 --- a/gopls/internal/server/command.go +++ b/gopls/internal/server/command.go @@ -187,8 +187,7 @@ func (c *commandHandler) run(ctx context.Context, cfg commandConfig, run command return err } - enum := command.Command(strings.TrimPrefix(c.params.Command, "gopls.")) - if enum.IsAsync() { + if enum := command.Command(c.params.Command); enum.IsAsync() { if cfg.progress == "" { log.Fatalf("asynchronous command %q does not enable progress reporting", enum) diff --git a/gopls/internal/settings/default.go b/gopls/internal/settings/default.go index 56dce7e2b40..e280c756b32 100644 --- a/gopls/internal/settings/default.go +++ b/gopls/internal/settings/default.go @@ -27,7 +27,7 @@ func DefaultOptions(overrides ...func(*Options)) *Options { optionsOnce.Do(func() { var commands []string for _, c := range command.Commands { - commands = append(commands, c.ID()) + commands = append(commands, c.String()) } defaultOptions = &Options{ ClientOptions: ClientOptions{ diff --git a/gopls/internal/telemetry/telemetry_test.go b/gopls/internal/telemetry/telemetry_test.go index d2eecdf762e..6c1944b2230 100644 --- a/gopls/internal/telemetry/telemetry_test.go +++ b/gopls/internal/telemetry/telemetry_test.go @@ -123,11 +123,11 @@ func addForwardedCounters(env *Env, names []string, values []int64) { } var res error env.ExecuteCommand(&protocol.ExecuteCommandParams{ - Command: command.AddTelemetryCounters.ID(), + Command: command.AddTelemetryCounters.String(), Arguments: args, }, &res) if res != nil { - env.T.Errorf("%v failed - %v", command.AddTelemetryCounters.ID(), res) + env.T.Errorf("%v failed - %v", command.AddTelemetryCounters, res) } } diff --git a/gopls/internal/test/integration/bench/bench_test.go b/gopls/internal/test/integration/bench/bench_test.go index a04c63d8de3..5de6804c03b 100644 --- a/gopls/internal/test/integration/bench/bench_test.go +++ b/gopls/internal/test/integration/bench/bench_test.go @@ -288,7 +288,7 @@ func (s *SidecarServer) Connect(ctx context.Context) jsonrpc2.Conn { // the profile is written to a temp file that is deleted after the cpu_seconds // metric has been computed. func startProfileIfSupported(b *testing.B, env *integration.Env, name string) func() { - if !env.Editor.HasCommand(command.StartProfile.ID()) { + if !env.Editor.HasCommand(command.StartProfile) { return nil } b.StopTimer() diff --git a/gopls/internal/test/integration/bench/iwl_test.go b/gopls/internal/test/integration/bench/iwl_test.go index 07a5d9070d0..ecf26f95463 100644 --- a/gopls/internal/test/integration/bench/iwl_test.go +++ b/gopls/internal/test/integration/bench/iwl_test.go @@ -57,10 +57,10 @@ func doIWL(b *testing.B, gopath string, repo *repo) { env.Await(InitialWorkspaceLoad) - if env.Editor.HasCommand(command.MemStats.ID()) { + if env.Editor.HasCommand(command.MemStats) { b.StopTimer() params := &protocol.ExecuteCommandParams{ - Command: command.MemStats.ID(), + Command: command.MemStats.String(), } var memstats command.MemStatsResult env.ExecuteCommand(params, &memstats) diff --git a/gopls/internal/test/integration/codelens/codelens_test.go b/gopls/internal/test/integration/codelens/codelens_test.go index 399b8d393f8..800856d3f7a 100644 --- a/gopls/internal/test/integration/codelens/codelens_test.go +++ b/gopls/internal/test/integration/codelens/codelens_test.go @@ -54,7 +54,7 @@ const ( }, { label: "generate disabled", - enabled: map[string]bool{string(command.Generate): false}, + enabled: map[string]bool{string(protocol.CodeLensGenerate): false}, wantCodeLens: false, }, } diff --git a/gopls/internal/test/integration/codelens/gcdetails_test.go b/gopls/internal/test/integration/codelens/gcdetails_test.go index 1ac3a8884ee..359a7804ec4 100644 --- a/gopls/internal/test/integration/codelens/gcdetails_test.go +++ b/gopls/internal/test/integration/codelens/gcdetails_test.go @@ -101,7 +101,7 @@ go 1.12 hasGCDetails := func() bool { lenses := env.CodeLens("p_test.go") // should not crash for _, lens := range lenses { - if lens.Command.Command == command.GCDetails.ID() { + if lens.Command.Command == command.GCDetails.String() { return true } } diff --git a/gopls/internal/test/integration/debug/debug_test.go b/gopls/internal/test/integration/debug/debug_test.go index 3b43de9db4c..1dccea43062 100644 --- a/gopls/internal/test/integration/debug/debug_test.go +++ b/gopls/internal/test/integration/debug/debug_test.go @@ -80,7 +80,7 @@ func startDebugging(ctx context.Context, server protocol.Server, args *command.D return nil, err } res0, err := server.ExecuteCommand(ctx, &protocol.ExecuteCommandParams{ - Command: command.StartDebugging.ID(), + Command: command.StartDebugging.String(), Arguments: rawArgs, }) if err != nil { diff --git a/gopls/internal/test/integration/fake/editor.go b/gopls/internal/test/integration/fake/editor.go index 36354d53e6f..f5d62b06aaf 100644 --- a/gopls/internal/test/integration/fake/editor.go +++ b/gopls/internal/test/integration/fake/editor.go @@ -391,9 +391,9 @@ func marshalUnmarshal[T any](v any) (T, error) { } // HasCommand reports whether the connected server supports the command with the given ID. -func (e *Editor) HasCommand(id string) bool { +func (e *Editor) HasCommand(cmd command.Command) bool { for _, command := range e.serverCapabilities.ExecuteCommandProvider.Commands { - if command == id { + if command == cmd.String() { return true } } diff --git a/gopls/internal/test/integration/misc/debugserver_test.go b/gopls/internal/test/integration/misc/debugserver_test.go index d1ce21bd47a..87f892f7443 100644 --- a/gopls/internal/test/integration/misc/debugserver_test.go +++ b/gopls/internal/test/integration/misc/debugserver_test.go @@ -23,7 +23,7 @@ func TestStartDebugging(t *testing.T) { t.Fatal(err) } params := &protocol.ExecuteCommandParams{ - Command: command.StartDebugging.ID(), + Command: command.StartDebugging.String(), Arguments: args, } var result command.DebuggingResult diff --git a/gopls/internal/test/integration/misc/import_test.go b/gopls/internal/test/integration/misc/import_test.go index 0df3f8dadec..af933b1639c 100644 --- a/gopls/internal/test/integration/misc/import_test.go +++ b/gopls/internal/test/integration/misc/import_test.go @@ -121,7 +121,7 @@ func TestFoo2(t *testing.T) {} } var result command.ListImportsResult env.ExecuteCommand(&protocol.ExecuteCommandParams{ - Command: command.ListImports.ID(), + Command: command.ListImports.String(), Arguments: cmd.Arguments, }, &result) if diff := cmp.Diff(tt.want, result); diff != "" { diff --git a/gopls/internal/test/integration/misc/vuln_test.go b/gopls/internal/test/integration/misc/vuln_test.go index f47d06ac7af..6ba1ec888eb 100644 --- a/gopls/internal/test/integration/misc/vuln_test.go +++ b/gopls/internal/test/integration/misc/vuln_test.go @@ -41,7 +41,7 @@ package foo } params := &protocol.ExecuteCommandParams{ - Command: command.RunGovulncheck.ID(), + Command: command.RunGovulncheck.String(), Arguments: cmd.Arguments, } diff --git a/gopls/internal/test/integration/wrappers.go b/gopls/internal/test/integration/wrappers.go index 55b99ea741e..0982f8a11dc 100644 --- a/gopls/internal/test/integration/wrappers.go +++ b/gopls/internal/test/integration/wrappers.go @@ -355,13 +355,13 @@ func (e *Env) ExecuteCodeLensCommand(path string, cmd command.Command, result in var lens protocol.CodeLens var found bool for _, l := range lenses { - if l.Command.Command == cmd.ID() { + if l.Command.Command == cmd.String() { lens = l found = true } } if !found { - e.T.Fatalf("found no command with the ID %s", cmd.ID()) + e.T.Fatalf("found no command with the ID %s", cmd) } e.ExecuteCommand(&protocol.ExecuteCommandParams{ Command: lens.Command.Command, @@ -421,7 +421,7 @@ func (e *Env) StartProfile() (stop func() string) { e.T.Fatal(err) } params := &protocol.ExecuteCommandParams{ - Command: command.StartProfile.ID(), + Command: command.StartProfile.String(), Arguments: args, } var result command.StartProfileResult @@ -433,7 +433,7 @@ func (e *Env) StartProfile() (stop func() string) { e.T.Fatal(err) } stopParams := &protocol.ExecuteCommandParams{ - Command: command.StopProfile.ID(), + Command: command.StopProfile.String(), Arguments: stopArgs, } var result command.StopProfileResult From a35b054a189a7451ce4fb8067f293285b3f02500 Mon Sep 17 00:00:00 2001 From: Alan Donovan Date: Thu, 6 Jun 2024 16:48:10 -0400 Subject: [PATCH 10/84] gopls/internal/test/integration: style tweaks to CodeAction Details: - Pass TriggerKind by value, not pointer. Define nil and zero as equivalent, always. Name the zero value: UnknownTrigger. Omit "kind" suffix in var names for brevity. - Add Workdir.EntireFile(path string) Location helper. - Env.CodeAction renamed CodeActionForFile. Env.CodeAction0 unsuffixed. Editor.CodeAction deleted. Editor.CodeAction0 unsuffixed. - Restore lost commentary. (Sorry, I was late to the code review of CL 590935.) Change-Id: I3fbe3e4e7567366b0742dcec44dc50539b9e9621 Reviewed-on: https://go-review.googlesource.com/c/tools/+/591176 Reviewed-by: Hyang-Ah Hana Kim Auto-Submit: Alan Donovan LUCI-TryBot-Result: Go LUCI Reviewed-by: Robert Findley --- gopls/internal/golang/codeaction.go | 15 ++++++++---- gopls/internal/protocol/codeactionkind.go | 5 ++++ gopls/internal/server/code_action.go | 12 ++++++---- .../test/integration/bench/codeaction_test.go | 8 +++---- .../internal/test/integration/fake/editor.go | 12 ++++------ .../internal/test/integration/fake/workdir.go | 5 ++++ .../test/integration/misc/codeactions_test.go | 24 +++++++++---------- .../test/integration/misc/extract_test.go | 2 +- .../test/integration/misc/fix_test.go | 4 ++-- .../test/integration/misc/imports_test.go | 4 ++-- .../test/integration/misc/vuln_test.go | 8 +++---- .../test/integration/misc/webserver_test.go | 4 ++-- gopls/internal/test/integration/wrappers.go | 19 ++++++++------- 13 files changed, 69 insertions(+), 53 deletions(-) diff --git a/gopls/internal/golang/codeaction.go b/gopls/internal/golang/codeaction.go index 8e8158c59db..22088222b38 100644 --- a/gopls/internal/golang/codeaction.go +++ b/gopls/internal/golang/codeaction.go @@ -26,9 +26,14 @@ import ( "golang.org/x/tools/internal/imports" ) -// CodeActions returns all code actions (edits and other commands) -// available for the selected range. -func CodeActions(ctx context.Context, snapshot *cache.Snapshot, fh file.Handle, rng protocol.Range, diagnostics []protocol.Diagnostic, want map[protocol.CodeActionKind]bool, triggerKind protocol.CodeActionTriggerKind) (actions []protocol.CodeAction, _ error) { +// CodeActions returns all wanted code actions (edits and other +// commands) available for the selected range. +// +// Depending on how the request was triggered, fewer actions may be +// offered, e.g. to avoid UI distractions after mere cursor motion. +// +// See ../protocol/codeactionkind.go for some code action theory. +func CodeActions(ctx context.Context, snapshot *cache.Snapshot, fh file.Handle, rng protocol.Range, diagnostics []protocol.Diagnostic, want map[protocol.CodeActionKind]bool, trigger protocol.CodeActionTriggerKind) (actions []protocol.CodeAction, _ error) { // Only compute quick fixes if there are any diagnostics to fix. wantQuickFixes := want[protocol.QuickFix] && len(diagnostics) > 0 @@ -138,7 +143,9 @@ func CodeActions(ctx context.Context, snapshot *cache.Snapshot, fh file.Handle, actions = append(actions, rewrites...) } - if want[protocol.RefactorInline] && (triggerKind != protocol.CodeActionAutomatic || rng.Start != rng.End) { + // To avoid distraction (e.g. VS Code lightbulb), offer "inline" + // only after a selection or explicit menu operation. + if want[protocol.RefactorInline] && (trigger != protocol.CodeActionAutomatic || rng.Start != rng.End) { rewrites, err := getInlineCodeActions(pkg, pgf, rng, snapshot.Options()) if err != nil { return nil, err diff --git a/gopls/internal/protocol/codeactionkind.go b/gopls/internal/protocol/codeactionkind.go index 8acba0e1bab..f3d953fbcac 100644 --- a/gopls/internal/protocol/codeactionkind.go +++ b/gopls/internal/protocol/codeactionkind.go @@ -79,6 +79,11 @@ const ( GoFreeSymbols CodeActionKind = "source.freesymbols" ) +// CodeActionUnknownTrigger indicates that the trigger for a +// CodeAction request is unknown. A missing +// CodeActionContext.TriggerKind should be treated as equivalent. +const CodeActionUnknownTrigger CodeActionTriggerKind = 0 + // A CodeLensSource identifies an (algorithmic) source of code lenses. type CodeLensSource string diff --git a/gopls/internal/server/code_action.go b/gopls/internal/server/code_action.go index e79ae68810f..45f230e5061 100644 --- a/gopls/internal/server/code_action.go +++ b/gopls/internal/server/code_action.go @@ -109,7 +109,7 @@ func (s *server) CodeAction(ctx context.Context, params *protocol.CodeActionPara return actions, nil case file.Go: - // diagnostic-associated code actions (problematic code) + // diagnostic-bundled code actions // // The diagnostics already have a UI presence (e.g. squiggly underline); // the associated action may additionally show (in VS Code) as a lightbulb. @@ -120,11 +120,13 @@ func (s *server) CodeAction(ctx context.Context, params *protocol.CodeActionPara if err != nil { return nil, err } - var triggerKind protocol.CodeActionTriggerKind - if k := params.Context.TriggerKind; k != nil { - triggerKind = *k + + // computed code actions (may include quickfixes from diagnostics) + trigger := protocol.CodeActionUnknownTrigger + if k := params.Context.TriggerKind; k != nil { // (some clients omit it) + trigger = *k } - moreActions, err := golang.CodeActions(ctx, snapshot, fh, params.Range, params.Context.Diagnostics, want, triggerKind) + moreActions, err := golang.CodeActions(ctx, snapshot, fh, params.Range, params.Context.Diagnostics, want, trigger) if err != nil { return nil, err } diff --git a/gopls/internal/test/integration/bench/codeaction_test.go b/gopls/internal/test/integration/bench/codeaction_test.go index fe89500da82..679f2d4cf3d 100644 --- a/gopls/internal/test/integration/bench/codeaction_test.go +++ b/gopls/internal/test/integration/bench/codeaction_test.go @@ -20,7 +20,7 @@ func BenchmarkCodeAction(b *testing.B) { defer closeBuffer(b, env, test.file) env.AfterChange() - env.CodeAction(test.file, nil) // pre-warm + env.CodeActionForFile(test.file, nil) // pre-warm b.ResetTimer() @@ -29,7 +29,7 @@ func BenchmarkCodeAction(b *testing.B) { } for i := 0; i < b.N; i++ { - env.CodeAction(test.file, nil) + env.CodeActionForFile(test.file, nil) } }) } @@ -44,7 +44,7 @@ func BenchmarkCodeActionFollowingEdit(b *testing.B) { env.EditBuffer(test.file, protocol.TextEdit{NewText: "// __TEST_PLACEHOLDER_0__\n"}) env.AfterChange() - env.CodeAction(test.file, nil) // pre-warm + env.CodeActionForFile(test.file, nil) // pre-warm b.ResetTimer() @@ -62,7 +62,7 @@ func BenchmarkCodeActionFollowingEdit(b *testing.B) { // Increment the placeholder text, to ensure cache misses. NewText: fmt.Sprintf("// __TEST_PLACEHOLDER_%d__\n", edits), }) - env.CodeAction(test.file, nil) + env.CodeActionForFile(test.file, nil) } }) } diff --git a/gopls/internal/test/integration/fake/editor.go b/gopls/internal/test/integration/fake/editor.go index f5d62b06aaf..487a255da3d 100644 --- a/gopls/internal/test/integration/fake/editor.go +++ b/gopls/internal/test/integration/fake/editor.go @@ -933,7 +933,7 @@ func (e *Editor) Symbol(ctx context.Context, query string) ([]protocol.SymbolInf // OrganizeImports requests and performs the source.organizeImports codeAction. func (e *Editor) OrganizeImports(ctx context.Context, path string) error { - loc := protocol.Location{URI: e.sandbox.Workdir.URI(path)} // zero Range => whole file + loc := e.sandbox.Workdir.EntireFile(path) _, err := e.applyCodeActions(ctx, loc, nil, protocol.SourceOrganizeImports) return err } @@ -1558,11 +1558,9 @@ func (e *Editor) ChangeWorkspaceFolders(ctx context.Context, folders []string) e // CodeAction executes a codeAction request on the server. // If loc.Range is zero, the whole file is implied. -func (e *Editor) CodeAction(ctx context.Context, loc protocol.Location, diagnostics []protocol.Diagnostic) ([]protocol.CodeAction, error) { - return e.CodeAction0(ctx, loc, diagnostics, nil) -} - -func (e *Editor) CodeAction0(ctx context.Context, loc protocol.Location, diagnostics []protocol.Diagnostic, triggerKind *protocol.CodeActionTriggerKind) ([]protocol.CodeAction, error) { +// To reduce distraction, the trigger action (unknown, automatic, invoked) +// may affect what actions are offered. +func (e *Editor) CodeAction(ctx context.Context, loc protocol.Location, diagnostics []protocol.Diagnostic, trigger protocol.CodeActionTriggerKind) ([]protocol.CodeAction, error) { if e.Server == nil { return nil, nil } @@ -1577,7 +1575,7 @@ func (e *Editor) CodeAction0(ctx context.Context, loc protocol.Location, diagnos TextDocument: e.TextDocumentIdentifier(path), Context: protocol.CodeActionContext{ Diagnostics: diagnostics, - TriggerKind: triggerKind, + TriggerKind: &trigger, }, Range: loc.Range, // may be zero } diff --git a/gopls/internal/test/integration/fake/workdir.go b/gopls/internal/test/integration/fake/workdir.go index 4d21554d4a8..977bf5458c5 100644 --- a/gopls/internal/test/integration/fake/workdir.go +++ b/gopls/internal/test/integration/fake/workdir.go @@ -149,6 +149,11 @@ func (w *Workdir) URIToPath(uri protocol.DocumentURI) string { return w.RelPath(uri.Path()) } +// EntireFile returns the entire extent of the file named by the workdir-relative path. +func (w *Workdir) EntireFile(path string) protocol.Location { + return protocol.Location{URI: w.URI(path)} +} + // ReadFile reads a text file specified by a workdir-relative path. func (w *Workdir) ReadFile(path string) ([]byte, error) { backoff := 1 * time.Millisecond diff --git a/gopls/internal/test/integration/misc/codeactions_test.go b/gopls/internal/test/integration/misc/codeactions_test.go index a6c2a8bd360..1a169c6896f 100644 --- a/gopls/internal/test/integration/misc/codeactions_test.go +++ b/gopls/internal/test/integration/misc/codeactions_test.go @@ -40,7 +40,7 @@ func g() {} check := func(filename string, wantKind ...protocol.CodeActionKind) { env.OpenFile(filename) loc := env.RegexpSearch(filename, `g\(\)`) - actions, err := env.Editor.CodeAction(env.Ctx, loc, nil) + actions, err := env.Editor.CodeAction(env.Ctx, loc, nil, protocol.CodeActionUnknownTrigger) if err != nil { t.Fatal(err) } @@ -87,23 +87,21 @@ func Func() int { return 0 } Run(t, "", func(t *testing.T, env *Env) { env.CreateBuffer("main.go", vim1) - for _, triggerKind := range []protocol.CodeActionTriggerKind{0, protocol.CodeActionInvoked, protocol.CodeActionAutomatic} { - triggerKindPtr := &triggerKind - if triggerKind == 0 { - triggerKindPtr = nil - } - t.Run(fmt.Sprintf("trigger=%v", triggerKind), func(t *testing.T) { + for _, trigger := range []protocol.CodeActionTriggerKind{ + protocol.CodeActionUnknownTrigger, + protocol.CodeActionInvoked, + protocol.CodeActionAutomatic, + } { + t.Run(fmt.Sprintf("trigger=%v", trigger), func(t *testing.T) { for _, selectedRange := range []bool{false, true} { t.Run(fmt.Sprintf("range=%t", selectedRange), func(t *testing.T) { - pattern := env.RegexpSearch("main.go", "Func") - rng := pattern.Range + loc := env.RegexpSearch("main.go", "Func") if !selectedRange { // assume the cursor is placed at the beginning of `Func`, so end==start. - rng.End = rng.Start + loc.Range.End = loc.Range.Start } - loc := protocol.Location{URI: pattern.URI, Range: rng} - actions := env.CodeAction0("main.go", loc, nil, triggerKindPtr) - want := triggerKind != protocol.CodeActionAutomatic || selectedRange + actions := env.CodeAction(loc, nil, trigger) + want := trigger != protocol.CodeActionAutomatic || selectedRange if got := slices.ContainsFunc(actions, func(act protocol.CodeAction) bool { return act.Kind == protocol.RefactorInline }); got != want { diff --git a/gopls/internal/test/integration/misc/extract_test.go b/gopls/internal/test/integration/misc/extract_test.go index 86afb45a49a..ec13856361e 100644 --- a/gopls/internal/test/integration/misc/extract_test.go +++ b/gopls/internal/test/integration/misc/extract_test.go @@ -30,7 +30,7 @@ func Foo() int { Run(t, files, func(t *testing.T, env *Env) { env.OpenFile("main.go") loc := env.RegexpSearch("main.go", `a := 5\n.*return a`) - actions, err := env.Editor.CodeAction(env.Ctx, loc, nil) + actions, err := env.Editor.CodeAction(env.Ctx, loc, nil, protocol.CodeActionUnknownTrigger) if err != nil { t.Fatal(err) } diff --git a/gopls/internal/test/integration/misc/fix_test.go b/gopls/internal/test/integration/misc/fix_test.go index b3d86e1a080..acf896a9adb 100644 --- a/gopls/internal/test/integration/misc/fix_test.go +++ b/gopls/internal/test/integration/misc/fix_test.go @@ -115,7 +115,7 @@ func Foo() error { ReadDiagnostics("main.go", &d), ) var quickFixes []*protocol.CodeAction - for _, act := range env.CodeAction("main.go", d.Diagnostics) { + for _, act := range env.CodeActionForFile("main.go", d.Diagnostics) { if act.Kind == protocol.QuickFix { act := act // remove in go1.22 quickFixes = append(quickFixes, &act) @@ -152,7 +152,7 @@ func _() { ` Run(t, files, func(t *testing.T, env *Env) { env.OpenFile("external.go") - _, err := env.Editor.CodeAction(env.Ctx, env.RegexpSearch("external.go", "z"), nil) + _, err := env.Editor.CodeAction(env.Ctx, env.RegexpSearch("external.go", "z"), nil, protocol.CodeActionUnknownTrigger) if err != nil { t.Fatal(err) } diff --git a/gopls/internal/test/integration/misc/imports_test.go b/gopls/internal/test/integration/misc/imports_test.go index 0e85a96572e..ebc1c0d50d2 100644 --- a/gopls/internal/test/integration/misc/imports_test.go +++ b/gopls/internal/test/integration/misc/imports_test.go @@ -149,7 +149,7 @@ func main() { env.OrganizeImports("main.go") // Assert no quick fixes. - for _, act := range env.CodeAction("main.go", nil) { + for _, act := range env.CodeActionForFile("main.go", nil) { if act.Kind == protocol.QuickFix { t.Errorf("unexpected quick fix action: %#v", act) } @@ -187,7 +187,7 @@ func main() { env.OrganizeImports("main.go") // Assert no quick fixes. - for _, act := range env.CodeAction("main.go", nil) { + for _, act := range env.CodeActionForFile("main.go", nil) { if act.Kind == protocol.QuickFix { t.Errorf("unexpected quick-fix action: %#v", act) } diff --git a/gopls/internal/test/integration/misc/vuln_test.go b/gopls/internal/test/integration/misc/vuln_test.go index 6ba1ec888eb..1ed54a7bbe8 100644 --- a/gopls/internal/test/integration/misc/vuln_test.go +++ b/gopls/internal/test/integration/misc/vuln_test.go @@ -519,7 +519,7 @@ func TestRunVulncheckPackageDiagnostics(t *testing.T) { for pattern, want := range wantVulncheckDiagnostics { modPathDiagnostics := testVulnDiagnostics(t, env, pattern, want, gotDiagnostics) - gotActions := env.CodeAction("go.mod", modPathDiagnostics) + gotActions := env.CodeActionForFile("go.mod", modPathDiagnostics) if diff := diffCodeActions(gotActions, want.codeActions); diff != "" { t.Errorf("code actions for %q do not match, got %v, want %v\n%v\n", pattern, gotActions, want.codeActions, diff) continue @@ -716,7 +716,7 @@ func TestRunVulncheckWarning(t *testing.T) { modPathDiagnostics := testVulnDiagnostics(t, env, mod, want, gotDiagnostics) // Check that the actions we get when including all diagnostics at a location return the same result - gotActions := env.CodeAction("go.mod", modPathDiagnostics) + gotActions := env.CodeActionForFile("go.mod", modPathDiagnostics) if diff := diffCodeActions(gotActions, want.codeActions); diff != "" { t.Errorf("code actions for %q do not match, expected %v, got %v\n%v\n", mod, want.codeActions, gotActions, diff) continue @@ -839,7 +839,7 @@ func TestGovulncheckInfo(t *testing.T) { for mod, want := range wantDiagnostics { modPathDiagnostics := testVulnDiagnostics(t, env, mod, want, gotDiagnostics) // Check that the actions we get when including all diagnostics at a location return the same result - gotActions := env.CodeAction("go.mod", modPathDiagnostics) + gotActions := env.CodeActionForFile("go.mod", modPathDiagnostics) allActions = append(allActions, gotActions...) if diff := diffCodeActions(gotActions, want.codeActions); diff != "" { t.Errorf("code actions for %q do not match, expected %v, got %v\n%v\n", mod, want.codeActions, gotActions, diff) @@ -889,7 +889,7 @@ func testVulnDiagnostics(t *testing.T, env *Env, pattern string, want vulnDiagEx t.Errorf("incorrect (severity, source) for %q, want (%s, %s) got (%s, %s)\n", w.msg, w.severity, w.source, diag.Severity, diag.Source) } // Check expected code actions appear. - gotActions := env.CodeAction("go.mod", []protocol.Diagnostic{*diag}) + gotActions := env.CodeActionForFile("go.mod", []protocol.Diagnostic{*diag}) if diff := diffCodeActions(gotActions, w.codeActions); diff != "" { t.Errorf("code actions for %q do not match, want %v, got %v\n%v\n", w.msg, w.codeActions, gotActions, diff) continue diff --git a/gopls/internal/test/integration/misc/webserver_test.go b/gopls/internal/test/integration/misc/webserver_test.go index c4af449ec59..851cc68e4c6 100644 --- a/gopls/internal/test/integration/misc/webserver_test.go +++ b/gopls/internal/test/integration/misc/webserver_test.go @@ -198,7 +198,7 @@ func viewPkgDoc(t *testing.T, env *Env, filename string) protocol.URI { // Invoke the "View package documentation" code // action to start the server. var docAction *protocol.CodeAction - actions := env.CodeAction(filename, nil) + actions := env.CodeActionForFile(filename, nil) for _, act := range actions { if act.Title == "View package documentation" { docAction = &act @@ -254,7 +254,7 @@ func f(buf bytes.Buffer, greeting string) { // Invoke the "Show free symbols" code // action to start the server. loc := env.RegexpSearch("a/a.go", "«((?:.|\n)*)»") - actions, err := env.Editor.CodeAction(env.Ctx, loc, nil) + actions, err := env.Editor.CodeAction(env.Ctx, loc, nil, protocol.CodeActionUnknownTrigger) if err != nil { t.Fatalf("CodeAction: %v", err) } diff --git a/gopls/internal/test/integration/wrappers.go b/gopls/internal/test/integration/wrappers.go index 0982f8a11dc..8c5e35e93c0 100644 --- a/gopls/internal/test/integration/wrappers.go +++ b/gopls/internal/test/integration/wrappers.go @@ -207,7 +207,7 @@ func (e *Env) OrganizeImports(name string) { // ApplyQuickFixes processes the quickfix codeAction, calling t.Fatal on any error. func (e *Env) ApplyQuickFixes(path string, diagnostics []protocol.Diagnostic) { e.T.Helper() - loc := protocol.Location{URI: e.Sandbox.Workdir.URI(path)} // zero Range => whole file + loc := e.Sandbox.Workdir.EntireFile(path) if err := e.Editor.ApplyQuickFixes(e.Ctx, loc, diagnostics); err != nil { e.T.Fatal(err) } @@ -224,7 +224,7 @@ func (e *Env) ApplyCodeAction(action protocol.CodeAction) { // GetQuickFixes returns the available quick fix code actions. func (e *Env) GetQuickFixes(path string, diagnostics []protocol.Diagnostic) []protocol.CodeAction { e.T.Helper() - loc := protocol.Location{URI: e.Sandbox.Workdir.URI(path)} // zero Range => whole file + loc := e.Sandbox.Workdir.EntireFile(path) actions, err := e.Editor.GetQuickFixes(e.Ctx, loc, diagnostics) if err != nil { e.T.Fatal(err) @@ -533,16 +533,17 @@ func (e *Env) AcceptCompletion(loc protocol.Location, item protocol.CompletionIt } } -// CodeAction calls textDocument/codeAction for the given path, and calls -// t.Fatal if there are errors. -func (e *Env) CodeAction(path string, diagnostics []protocol.Diagnostic) []protocol.CodeAction { - loc := protocol.Location{URI: e.Sandbox.Workdir.URI(path)} // no Range => whole file - return e.CodeAction0(path, loc, diagnostics, nil) +// CodeActionForFile calls textDocument/codeAction for the entire +// file, and calls t.Fatal if there were errors. +func (e *Env) CodeActionForFile(path string, diagnostics []protocol.Diagnostic) []protocol.CodeAction { + return e.CodeAction(e.Sandbox.Workdir.EntireFile(path), diagnostics, protocol.CodeActionUnknownTrigger) } -func (e *Env) CodeAction0(path string, loc protocol.Location, diagnostics []protocol.Diagnostic, triggerKind *protocol.CodeActionTriggerKind) []protocol.CodeAction { +// CodeAction calls textDocument/codeAction for a selection, +// and calls t.Fatal if there were errors. +func (e *Env) CodeAction(loc protocol.Location, diagnostics []protocol.Diagnostic, trigger protocol.CodeActionTriggerKind) []protocol.CodeAction { e.T.Helper() - actions, err := e.Editor.CodeAction0(e.Ctx, loc, diagnostics, triggerKind) + actions, err := e.Editor.CodeAction(e.Ctx, loc, diagnostics, trigger) if err != nil { e.T.Fatal(err) } From 15d13c3f3fdb6da7b69ddc58ec99a212244c2884 Mon Sep 17 00:00:00 2001 From: Alan Donovan Date: Fri, 24 May 2024 23:09:17 -0400 Subject: [PATCH 11/84] gopls/internal/golang: add "Show assembly of f" code action This CL adds a new code action to display the Go assembly code produced for the enclosing function, and all its nested functions. Each instruction is marked up with a link that causes cursor navigation in the editor, using the magic of LSP's showDocument. Each page load causes a recompile. (A cold build may take a few seconds in a large project.) The architecture is determined by the view: for most files it will be the default GOARCH, but for arch-tagged files it will be the appropriate one. + Test + Release note Fixes golang/go#67478 Change-Id: I51285215e9b27c510076c64eeb7b7ae3899f8a59 Reviewed-on: https://go-review.googlesource.com/c/tools/+/588395 Reviewed-by: Robert Findley Auto-Submit: Alan Donovan LUCI-TryBot-Result: Go LUCI --- gopls/doc/commands.md | 14 ++ gopls/doc/release/v0.16.0.md | 25 +++ gopls/internal/doc/api.json | 7 + gopls/internal/golang/assembly.go | 166 ++++++++++++++++++ gopls/internal/golang/codeaction.go | 92 +++++++++- gopls/internal/protocol/codeactionkind.go | 3 +- .../internal/protocol/command/command_gen.go | 22 +++ gopls/internal/protocol/command/interface.go | 7 + gopls/internal/server/code_action.go | 5 +- gopls/internal/server/command.go | 15 +- gopls/internal/server/server.go | 64 ++++++- gopls/internal/settings/default.go | 1 + .../test/integration/misc/codeactions_test.go | 10 +- .../test/integration/misc/webserver_test.go | 77 ++++++++ 14 files changed, 499 insertions(+), 9 deletions(-) create mode 100644 gopls/internal/golang/assembly.go diff --git a/gopls/doc/commands.md b/gopls/doc/commands.md index 69c3af86225..357e3497de2 100644 --- a/gopls/doc/commands.md +++ b/gopls/doc/commands.md @@ -140,6 +140,20 @@ Result: } ``` +## `gopls.assembly`: **Show disassembly of current function.** + +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. diff --git a/gopls/doc/release/v0.16.0.md b/gopls/doc/release/v0.16.0.md index dc004c44e85..ab54ae2fb84 100644 --- a/gopls/doc/release/v0.16.0.md +++ b/gopls/doc/release/v0.16.0.md @@ -88,6 +88,31 @@ the function by choosing a different type for that parameter. ``` TODO(dominikh/go-mode.el#436): add both of these to go-mode.el. +### Show assembly + +Gopls offers a third web-based code action, "Show assembly for f", +which displays an assembly listing of the function declaration +enclosing the selected code, plus any nested functions (function +literals, deferred calls). +It invokes the compiler to generate the report. +Reloading the page will update the report. + +The machine architecture is determined by the "view" or 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. + +- TODO screenshot + +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. + + ### `unusedwrite` analyzer The new diff --git a/gopls/internal/doc/api.json b/gopls/internal/doc/api.json index e17038511a9..555c56ac8a6 100644 --- a/gopls/internal/doc/api.json +++ b/gopls/internal/doc/api.json @@ -960,6 +960,13 @@ "ArgDoc": "{\n\t// The name of the fix to apply.\n\t//\n\t// For fixes suggested by analyzers, this is a string constant\n\t// advertised by the analyzer that matches the Category of\n\t// the analysis.Diagnostic with a SuggestedFix containing no edits.\n\t//\n\t// For fixes suggested by code actions, this is a string agreed\n\t// upon by the code action and golang.ApplyFix.\n\t\"Fix\": string,\n\t// The file URI for the document to fix.\n\t\"URI\": string,\n\t// The document range to scan for fixes.\n\t\"Range\": {\n\t\t\"start\": {\n\t\t\t\"line\": uint32,\n\t\t\t\"character\": uint32,\n\t\t},\n\t\t\"end\": {\n\t\t\t\"line\": uint32,\n\t\t\t\"character\": uint32,\n\t\t},\n\t},\n\t// Whether to resolve and return the edits.\n\t\"ResolveEdits\": bool,\n}", "ResultDoc": "{\n\t// Holds changes to existing resources.\n\t\"changes\": map[golang.org/x/tools/gopls/internal/protocol.DocumentURI][]golang.org/x/tools/gopls/internal/protocol.TextEdit,\n\t// Depending on the client capability `workspace.workspaceEdit.resourceOperations` document changes\n\t// are either an array of `TextDocumentEdit`s to express changes to n different text documents\n\t// where each text document edit addresses a specific version of a text document. Or it can contain\n\t// above `TextDocumentEdit`s mixed with create, rename and delete file / folder operations.\n\t//\n\t// Whether a client supports versioned document edits is expressed via\n\t// `workspace.workspaceEdit.documentChanges` client capability.\n\t//\n\t// If a client neither supports `documentChanges` nor `workspace.workspaceEdit.resourceOperations` then\n\t// only plain `TextEdit`s using the `changes` property are supported.\n\t\"documentChanges\": []{\n\t\t\"TextDocumentEdit\": {\n\t\t\t\"textDocument\": { ... },\n\t\t\t\"edits\": { ... },\n\t\t},\n\t\t\"CreateFile\": {\n\t\t\t\"kind\": string,\n\t\t\t\"uri\": string,\n\t\t\t\"options\": { ... },\n\t\t\t\"ResourceOperation\": { ... },\n\t\t},\n\t\t\"RenameFile\": {\n\t\t\t\"kind\": string,\n\t\t\t\"oldUri\": string,\n\t\t\t\"newUri\": string,\n\t\t\t\"options\": { ... },\n\t\t\t\"ResourceOperation\": { ... },\n\t\t},\n\t\t\"DeleteFile\": {\n\t\t\t\"kind\": string,\n\t\t\t\"uri\": string,\n\t\t\t\"options\": { ... },\n\t\t\t\"ResourceOperation\": { ... },\n\t\t},\n\t},\n\t// A map of change annotations that can be referenced in `AnnotatedTextEdit`s or create, rename and\n\t// delete file / folder operations.\n\t//\n\t// Whether clients honor this property depends on the client capability `workspace.changeAnnotationSupport`.\n\t//\n\t// @since 3.16.0\n\t\"changeAnnotations\": map[string]golang.org/x/tools/gopls/internal/protocol.ChangeAnnotation,\n}" }, + { + "Command": "gopls.assembly", + "Title": "Show disassembly of current function.", + "Doc": "This command opens a web-based disassembly listing of the\nspecified function symbol (plus any nested lambdas and defers).\nThe machine architecture is determined by the view.", + "ArgDoc": "string,\nstring,\nstring", + "ResultDoc": "" + }, { "Command": "gopls.change_signature", "Title": "Perform a \"change signature\" refactoring", diff --git a/gopls/internal/golang/assembly.go b/gopls/internal/golang/assembly.go new file mode 100644 index 00000000000..cbb951288a6 --- /dev/null +++ b/gopls/internal/golang/assembly.go @@ -0,0 +1,166 @@ +// 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 golang + +// This file produces the "Show GOARCH assembly of f" HTML report. +// +// See also: +// - ./codeaction.go - computes the symbol and offers the CodeAction command. +// - ../server/command.go - handles the command by opening a web page. +// - ../server/server.go - handles the HTTP request and calls this function. + +import ( + "bytes" + "context" + "fmt" + "html" + "path/filepath" + "regexp" + "strconv" + "strings" + + "golang.org/x/tools/gopls/internal/cache" + "golang.org/x/tools/internal/gocommand" +) + +// AssemblyHTML returns an HTML document containing an assembly listing of the selected function. +// +// TODO(adonovan): +// - display a "Compiling..." message as a cold build can be slow. +// - cross-link jumps and block labels, like github.com/aclements/objbrowse. +func AssemblyHTML(ctx context.Context, snapshot *cache.Snapshot, pkg *cache.Package, symbol string, posURL PosURLFunc) ([]byte, error) { + // Compile the package with -S, and capture its stderr stream. + inv, cleanupInvocation, err := snapshot.GoCommandInvocation(false, &gocommand.Invocation{ + Verb: "build", + Args: []string{"-gcflags=-S", "."}, + WorkingDir: filepath.Dir(pkg.Metadata().CompiledGoFiles[0].Path()), + }) + if err != nil { + return nil, err // e.g. failed to write overlays (rare) + } + defer cleanupInvocation() + _, stderr, err, _ := snapshot.View().GoCommandRunner().RunRaw(ctx, *inv) + if err != nil { + return nil, err // e.g. won't compile + } + content := stderr.String() + + escape := html.EscapeString + + // Produce the report. + // TODO(adonovan): factor with RenderPkgDoc, FreeSymbolsHTML + title := fmt.Sprintf("%s assembly for %s", + escape(snapshot.View().GOARCH()), + escape(symbol)) + var buf bytes.Buffer + buf.WriteString(` + + + + + ` + escape(title) + ` + + + +
Gopls server has terminated. Page is inactive.
+

` + title + `

+

+ A Quick Guide to Go's Assembler +

+

+ Experimental. Contributions welcome! +

+

+ Click on a source line marker L1234 to navigate your editor there. + (Beware: #207634) +

+

+ Reload the page to recompile. +

+
+`)
+
+	// sourceLink returns HTML for a link to open a file in the client editor.
+	// TODO(adonovan): factor with two other copies.
+	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`,
+			escape(url), text)
+	}
+
+	// insnRx matches an assembly instruction line.
+	// Submatch groups are: (offset-hex-dec, file-line-column, instruction).
+	insnRx := regexp.MustCompile(`^(\s+0x[0-9a-f ]+)\(([^)]*)\)\s+(.*)$`)
+
+	// Parse the functions of interest out of the listing.
+	// Each function is of the form:
+	//
+	//     symbol STEXT k=v...
+	//         0x0000 00000 (/file.go:123) NOP...
+	//         ...
+	//
+	// Allow matches of symbol, symbol.func1, symbol.deferwrap, etc.
+	on := false
+	for _, line := range strings.Split(content, "\n") {
+		// start of function symbol?
+		if strings.Contains(line, " STEXT ") {
+			on = strings.HasPrefix(line, symbol) &&
+				(line[len(symbol)] == ' ' || line[len(symbol)] == '.')
+		}
+		if !on {
+			continue // within uninteresting symbol
+		}
+
+		// In lines of the form
+		//   "\t0x0000 00000 (/file.go:123) NOP..."
+		// replace the "(/file.go:123)" portion with an "L0123" source link.
+		// Skip filenames of the form "".
+		if parts := insnRx.FindStringSubmatch(line); parts != nil {
+			link := "     " // if unknown
+			if file, linenum, ok := cutLast(parts[2], ":"); ok && !strings.HasPrefix(file, "<") {
+				if linenum, err := strconv.Atoi(linenum); err == nil {
+					text := fmt.Sprintf("L%04d", linenum)
+					link = sourceLink(text, posURL(file, linenum, 1))
+				}
+			}
+			fmt.Fprintf(&buf, "%s\t%s\t%s", escape(parts[1]), link, escape(parts[3]))
+		} else {
+			buf.WriteString(escape(line))
+		}
+		buf.WriteByte('\n')
+	}
+	return buf.Bytes(), nil
+}
+
+// cutLast is the "last" analogue of [strings.Cut].
+func cutLast(s, sep string) (before, after string, ok bool) {
+	if i := strings.LastIndex(s, sep); i >= 0 {
+		return s[:i], s[i+len(sep):], true
+	}
+	return s, "", false
+}
diff --git a/gopls/internal/golang/codeaction.go b/gopls/internal/golang/codeaction.go
index 22088222b38..07f777eefdc 100644
--- a/gopls/internal/golang/codeaction.go
+++ b/gopls/internal/golang/codeaction.go
@@ -9,8 +9,10 @@ import (
 	"encoding/json"
 	"fmt"
 	"go/ast"
+	"go/types"
 	"strings"
 
+	"golang.org/x/tools/go/ast/astutil"
 	"golang.org/x/tools/gopls/internal/analysis/fillstruct"
 	"golang.org/x/tools/gopls/internal/analysis/fillswitch"
 	"golang.org/x/tools/gopls/internal/cache"
@@ -24,6 +26,7 @@ import (
 	"golang.org/x/tools/gopls/internal/util/slices"
 	"golang.org/x/tools/internal/event"
 	"golang.org/x/tools/internal/imports"
+	"golang.org/x/tools/internal/typesinternal"
 )
 
 // CodeActions returns all wanted code actions (edits and other
@@ -118,7 +121,7 @@ func CodeActions(ctx context.Context, snapshot *cache.Snapshot, fh file.Handle,
 			if err != nil {
 				return nil, err
 			}
-			// For implementation, see commandHandler.showFreeSymbols.
+			// For implementation, see commandHandler.FreeSymbols.
 			actions = append(actions, protocol.CodeAction{
 				Title:   cmd.Title,
 				Kind:    protocol.GoFreeSymbols,
@@ -130,6 +133,7 @@ func CodeActions(ctx context.Context, snapshot *cache.Snapshot, fh file.Handle,
 	// Code actions requiring type information.
 	if want[protocol.RefactorRewrite] ||
 		want[protocol.RefactorInline] ||
+		want[protocol.GoAssembly] ||
 		want[protocol.GoTest] {
 		pkg, pgf, err := NarrowestPackageForFile(ctx, snapshot, fh.URI())
 		if err != nil {
@@ -160,6 +164,14 @@ func CodeActions(ctx context.Context, snapshot *cache.Snapshot, fh file.Handle,
 			}
 			actions = append(actions, fixes...)
 		}
+
+		if want[protocol.GoAssembly] {
+			fixes, err := getGoAssemblyAction(snapshot.View(), pkg, pgf, rng)
+			if err != nil {
+				return nil, err
+			}
+			actions = append(actions, fixes...)
+		}
 	}
 	return actions, nil
 }
@@ -521,3 +533,81 @@ func getGoTestCodeActions(pkg *cache.Package, pgf *parsego.File, rng protocol.Ra
 		Command: &cmd,
 	}}, nil
 }
+
+// getGoAssemblyAction returns any "Show assembly for f" code actions for the selection.
+func getGoAssemblyAction(view *cache.View, pkg *cache.Package, pgf *parsego.File, rng protocol.Range) ([]protocol.CodeAction, error) {
+	start, end, err := pgf.RangePos(rng)
+	if err != nil {
+		return nil, err
+	}
+
+	// Find the enclosing toplevel function or method,
+	// and compute its symbol name (e.g. "pkgpath.(T).method").
+	// The report will show this method and all its nested
+	// functions (FuncLit, defers, etc).
+	//
+	// TODO(adonovan): this is no good for generics, since they
+	// will always be uninstantiated when they enclose the cursor.
+	// Instead, we need to query the func symbol under the cursor,
+	// rather than the enclosing function. It may be an explicitly
+	// or implicitly instantiated generic, and it may be defined
+	// in another package, though we would still need to compile
+	// the current package to see its assembly. The challenge,
+	// however, is that computing the linker name for a generic
+	// symbol is quite tricky. Talk with the compiler team for
+	// ideas.
+	//
+	// TODO(adonovan): think about a smoother UX for jumping
+	// directly to (say) a lambda of interest.
+	// Perhaps we could scroll to STEXT for the innermost
+	// enclosing nested function?
+	var actions []protocol.CodeAction
+	path, _ := astutil.PathEnclosingInterval(pgf.File, start, end)
+	if len(path) >= 2 { // [... FuncDecl File]
+		if decl, ok := path[len(path)-2].(*ast.FuncDecl); ok {
+			if fn, ok := pkg.TypesInfo().Defs[decl.Name].(*types.Func); ok {
+				sig := fn.Type().(*types.Signature)
+
+				// Compute the linker symbol of the enclosing function.
+				var sym strings.Builder
+				if fn.Pkg().Name() == "main" {
+					sym.WriteString("main")
+				} else {
+					sym.WriteString(fn.Pkg().Path())
+				}
+				sym.WriteString(".")
+				if sig.Recv() != nil {
+					if isPtr, named := typesinternal.ReceiverNamed(sig.Recv()); named != nil {
+						sym.WriteString("(")
+						if isPtr {
+							sym.WriteString("*")
+						}
+						sym.WriteString(named.Obj().Name())
+						sym.WriteString(").")
+					}
+				}
+				sym.WriteString(fn.Name())
+
+				if fn.Name() != "_" && // blank functions are not compiled
+					(fn.Name() != "init" || sig.Recv() != nil) && // init functions aren't linker functions
+					sig.TypeParams() == nil && sig.RecvTypeParams() == nil { // generic => no assembly
+					cmd, err := command.NewAssemblyCommand(
+						fmt.Sprintf("Show %s assembly for %s", view.GOARCH(), decl.Name),
+						view.ID(),
+						string(pkg.Metadata().ID),
+						sym.String())
+					if err != nil {
+						return nil, err
+					}
+					// For handler, see commandHandler.Assembly.
+					actions = append(actions, protocol.CodeAction{
+						Title:   cmd.Title,
+						Kind:    protocol.GoAssembly,
+						Command: &cmd,
+					})
+				}
+			}
+		}
+	}
+	return actions, nil
+}
diff --git a/gopls/internal/protocol/codeactionkind.go b/gopls/internal/protocol/codeactionkind.go
index f3d953fbcac..d493b452f86 100644
--- a/gopls/internal/protocol/codeactionkind.go
+++ b/gopls/internal/protocol/codeactionkind.go
@@ -74,9 +74,10 @@ package protocol
 // instead of == for CodeActionKinds throughout gopls.
 // See golang/go#40438 for related discussion.
 const (
-	GoTest        CodeActionKind = "goTest"
+	GoAssembly    CodeActionKind = "source.assembly"
 	GoDoc         CodeActionKind = "source.doc"
 	GoFreeSymbols CodeActionKind = "source.freesymbols"
+	GoTest        CodeActionKind = "goTest" // TODO(adonovan): rename "source.test"
 )
 
 // CodeActionUnknownTrigger indicates that the trigger for a
diff --git a/gopls/internal/protocol/command/command_gen.go b/gopls/internal/protocol/command/command_gen.go
index 5e267afdfe9..2ba73dbc90c 100644
--- a/gopls/internal/protocol/command/command_gen.go
+++ b/gopls/internal/protocol/command/command_gen.go
@@ -28,6 +28,7 @@ const (
 	AddImport               Command = "gopls.add_import"
 	AddTelemetryCounters    Command = "gopls.add_telemetry_counters"
 	ApplyFix                Command = "gopls.apply_fix"
+	Assembly                Command = "gopls.assembly"
 	ChangeSignature         Command = "gopls.change_signature"
 	CheckUpgrades           Command = "gopls.check_upgrades"
 	DiagnoseFiles           Command = "gopls.diagnose_files"
@@ -66,6 +67,7 @@ var Commands = []Command{
 	AddImport,
 	AddTelemetryCounters,
 	ApplyFix,
+	Assembly,
 	ChangeSignature,
 	CheckUpgrades,
 	DiagnoseFiles,
@@ -125,6 +127,14 @@ func Dispatch(ctx context.Context, params *protocol.ExecuteCommandParams, s Inte
 			return nil, err
 		}
 		return s.ApplyFix(ctx, a0)
+	case Assembly:
+		var a0 string
+		var a1 string
+		var a2 string
+		if err := UnmarshalArgs(params.Arguments, &a0, &a1, &a2); err != nil {
+			return nil, err
+		}
+		return nil, s.Assembly(ctx, a0, a1, a2)
 	case ChangeSignature:
 		var a0 ChangeSignatureArgs
 		if err := UnmarshalArgs(params.Arguments, &a0); err != nil {
@@ -350,6 +360,18 @@ func NewApplyFixCommand(title string, a0 ApplyFixArgs) (protocol.Command, error)
 	}, nil
 }
 
+func NewAssemblyCommand(title string, a0 string, a1 string, a2 string) (protocol.Command, error) {
+	args, err := MarshalArgs(a0, a1, a2)
+	if err != nil {
+		return protocol.Command{}, err
+	}
+	return protocol.Command{
+		Title:     title,
+		Command:   Assembly.String(),
+		Arguments: args,
+	}, nil
+}
+
 func NewChangeSignatureCommand(title string, a0 ChangeSignatureArgs) (protocol.Command, error) {
 	args, err := MarshalArgs(a0)
 	if err != nil {
diff --git a/gopls/internal/protocol/command/interface.go b/gopls/internal/protocol/command/interface.go
index e8d6d7dbac9..9f76add467d 100644
--- a/gopls/internal/protocol/command/interface.go
+++ b/gopls/internal/protocol/command/interface.go
@@ -252,6 +252,13 @@ type Interface interface {
 	// block of code depends on, perhaps as a precursor to
 	// extracting it into a separate function.
 	FreeSymbols(context.Context, protocol.DocumentURI, protocol.Range) error
+
+	// Assembly: Show disassembly of current function.
+	//
+	// 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.
+	Assembly(_ context.Context, viewID, packageID, symbol string) error
 }
 
 type RunTestsArgs struct {
diff --git a/gopls/internal/server/code_action.go b/gopls/internal/server/code_action.go
index 45f230e5061..7252aa98acf 100644
--- a/gopls/internal/server/code_action.go
+++ b/gopls/internal/server/code_action.go
@@ -139,7 +139,10 @@ func (s *server) CodeAction(ctx context.Context, params *protocol.CodeActionPara
 		if golang.IsGenerated(ctx, snapshot, uri) {
 			actions = slices.DeleteFunc(actions, func(a protocol.CodeAction) bool {
 				switch a.Kind {
-				case protocol.GoTest, protocol.GoDoc, protocol.GoFreeSymbols:
+				case protocol.GoTest,
+					protocol.GoDoc,
+					protocol.GoFreeSymbols,
+					protocol.GoAssembly:
 					return false // read-only query
 				}
 				return true // potential write operation
diff --git a/gopls/internal/server/command.go b/gopls/internal/server/command.go
index 0a64d1cd1be..04febab9a33 100644
--- a/gopls/internal/server/command.go
+++ b/gopls/internal/server/command.go
@@ -104,7 +104,7 @@ type commandConfig struct {
 // be populated, depending on which configuration is set. See comments in-line
 // for details.
 type commandDeps struct {
-	snapshot *cache.Snapshot    // present if cfg.forURI was set
+	snapshot *cache.Snapshot    // present if cfg.forURI or forView was set
 	fh       file.Handle        // present if cfg.forURI was set
 	work     *progress.WorkDone // present if cfg.progress was set
 }
@@ -1466,6 +1466,9 @@ func (c *commandHandler) Views(ctx context.Context) ([]command.View, error) {
 }
 
 func (c *commandHandler) FreeSymbols(ctx context.Context, uri protocol.DocumentURI, rng protocol.Range) error {
+	// TODO(adonovan): simplify, following Assembly, by putting the
+	// viewID in the command so that c.run isn't necessary.
+	// (freesymbolsURL needs only a viewID, not a view.)
 	return c.run(ctx, commandConfig{
 		forURI: uri,
 	}, func(ctx context.Context, deps commandDeps) error {
@@ -1481,3 +1484,13 @@ func (c *commandHandler) FreeSymbols(ctx context.Context, uri protocol.DocumentU
 		return nil
 	})
 }
+
+func (c *commandHandler) Assembly(ctx context.Context, viewID, packageID, symbol string) error {
+	web, err := c.s.getWeb()
+	if err != nil {
+		return err
+	}
+	url := web.assemblyURL(viewID, packageID, symbol)
+	openClientBrowser(ctx, c.s.client, url)
+	return nil
+}
diff --git a/gopls/internal/server/server.go b/gopls/internal/server/server.go
index 34182156fd8..2c0c34ac98e 100644
--- a/gopls/internal/server/server.go
+++ b/gopls/internal/server/server.go
@@ -205,6 +205,8 @@ func (s *server) WorkDoneProgressCancel(ctx context.Context, params *protocol.Wo
 //
 //	open?file=%s&line=%d&col=%d       - open a file
 //	pkg/PKGPATH?view=%s               - show doc for package in a given view
+//	assembly?pkg=%s&view=%s&symbol=%s - show assembly of specified func symbol
+//	freesymbols?file=%s&range=%d:%d:%d:%d:&view=%s - show report of free symbols
 type web struct {
 	server *http.Server
 	addr   url.URL // "http://127.0.0.1:PORT/gopls/SECRET"
@@ -315,7 +317,7 @@ func (s *server) initWeb() (*web, error) {
 		// Get snapshot of specified view.
 		view, err := s.session.View(req.Form.Get("view"))
 		if err != nil {
-			http.Error(w, err.Error(), http.StatusBadRequest)
+			http.Error(w, err.Error(), http.StatusNotFound)
 			return
 		}
 		snapshot, release, err := view.Snapshot()
@@ -410,6 +412,55 @@ func (s *server) initWeb() (*web, error) {
 		w.Write(html)
 	})
 
+	// The /assembly?pkg=...&view=...&symbol=... handler shows
+	// the assembly of the current function.
+	webMux.HandleFunc("/assembly", func(w http.ResponseWriter, req *http.Request) {
+		ctx := req.Context()
+		if err := req.ParseForm(); err != nil {
+			http.Error(w, err.Error(), http.StatusBadRequest)
+			return
+		}
+
+		// Get parameters.
+		var (
+			viewID = req.Form.Get("view")
+			pkgID  = metadata.PackageID(req.Form.Get("pkg"))
+			symbol = req.Form.Get("symbol")
+		)
+		if viewID == "" || pkgID == "" || symbol == "" {
+			http.Error(w, "/assembly requires view, pkg, symbol", http.StatusBadRequest)
+			return
+		}
+
+		// Get snapshot of specified view.
+		view, err := s.session.View(viewID)
+		if err != nil {
+			http.Error(w, err.Error(), http.StatusNotFound)
+			return
+		}
+		snapshot, release, err := view.Snapshot()
+		if err != nil {
+			http.Error(w, err.Error(), http.StatusInternalServerError)
+			return
+		}
+		defer release()
+
+		pkgs, err := snapshot.TypeCheck(ctx, pkgID)
+		if err != nil {
+			http.Error(w, err.Error(), http.StatusInternalServerError)
+			return
+		}
+		pkg := pkgs[0]
+
+		// Produce report.
+		html, err := golang.AssemblyHTML(ctx, snapshot, pkg, symbol, web.openURL)
+		if err != nil {
+			http.Error(w, err.Error(), http.StatusInternalServerError)
+			return
+		}
+		w.Write(html)
+	})
+
 	return web, nil
 }
 
@@ -456,6 +507,17 @@ func (w *web) freesymbolsURL(v *cache.View, loc protocol.Location) protocol.URI
 		"")
 }
 
+// assemblyURL returns the URL of an assembly listing of the specified function symbol.
+func (w *web) assemblyURL(viewID, packageID, symbol string) protocol.URI {
+	return w.url(
+		"assembly",
+		fmt.Sprintf("view=%s&pkg=%s&symbol=%s",
+			url.QueryEscape(viewID),
+			url.QueryEscape(packageID),
+			url.QueryEscape(symbol)),
+		"")
+}
+
 // url returns a URL by joining a relative path, an (encoded) query,
 // and an (unencoded) fragment onto the authenticated base URL of the
 // web server.
diff --git a/gopls/internal/settings/default.go b/gopls/internal/settings/default.go
index e280c756b32..fec64e39d2f 100644
--- a/gopls/internal/settings/default.go
+++ b/gopls/internal/settings/default.go
@@ -49,6 +49,7 @@ func DefaultOptions(overrides ...func(*Options)) *Options {
 						protocol.RefactorRewrite:       true,
 						protocol.RefactorInline:        true,
 						protocol.RefactorExtract:       true,
+						protocol.GoAssembly:            true,
 						protocol.GoDoc:                 true,
 						protocol.GoFreeSymbols:         true,
 					},
diff --git a/gopls/internal/test/integration/misc/codeactions_test.go b/gopls/internal/test/integration/misc/codeactions_test.go
index 1a169c6896f..09629b482d6 100644
--- a/gopls/internal/test/integration/misc/codeactions_test.go
+++ b/gopls/internal/test/integration/misc/codeactions_test.go
@@ -23,12 +23,12 @@ func TestCodeActionsInGeneratedFiles(t *testing.T) {
 module example.com
 go 1.19
 
--- src.go --
+-- src/a.go --
 package a
 
 func f() { g() }
 func g() {}
--- gen.go --
+-- gen/a.go --
 // Code generated by hand; DO NOT EDIT.
 package a
 
@@ -62,12 +62,14 @@ func g() {}
 			}
 		}
 
-		check("src.go",
+		check("src/a.go",
+			protocol.GoAssembly,
 			protocol.GoDoc,
 			protocol.GoFreeSymbols,
 			protocol.RefactorExtract,
 			protocol.RefactorInline)
-		check("gen.go",
+		check("gen/a.go",
+			protocol.GoAssembly,
 			protocol.GoDoc,
 			protocol.GoFreeSymbols)
 	})
diff --git a/gopls/internal/test/integration/misc/webserver_test.go b/gopls/internal/test/integration/misc/webserver_test.go
index 851cc68e4c6..22f65ecd86a 100644
--- a/gopls/internal/test/integration/misc/webserver_test.go
+++ b/gopls/internal/test/integration/misc/webserver_test.go
@@ -9,12 +9,14 @@ import (
 	"io"
 	"net/http"
 	"regexp"
+	"runtime"
 	"strings"
 	"testing"
 
 	"golang.org/x/tools/gopls/internal/protocol"
 	"golang.org/x/tools/gopls/internal/protocol/command"
 	. "golang.org/x/tools/gopls/internal/test/integration"
+	"golang.org/x/tools/internal/testenv"
 )
 
 // TestWebServer exercises the web server created on demand
@@ -295,6 +297,81 @@ func f(buf bytes.Buffer, greeting string) {
 	})
 }
 
+// TestAssembly is a basic test of the web-based assembly listing.
+func TestAssembly(t *testing.T) {
+	testenv.NeedsGo1Point(t, 22) // for up-to-date assembly listing
+
+	const files = `
+-- go.mod --
+module example.com
+
+-- a/a.go --
+package a
+
+func f() {
+	println("hello")
+	defer println("world")
+}
+
+func g() {
+	println("goodbye")
+}
+`
+	Run(t, files, func(t *testing.T, env *Env) {
+		env.OpenFile("a/a.go")
+
+		// Invoke the "Show assembly" code action to start the server.
+		loc := env.RegexpSearch("a/a.go", "println")
+		actions, err := env.Editor.CodeAction(env.Ctx, loc, nil)
+		if err != nil {
+			t.Fatalf("CodeAction: %v", err)
+		}
+		const wantTitle = "Show " + runtime.GOARCH + " assembly for f"
+		var action *protocol.CodeAction
+		for _, a := range actions {
+			if a.Title == wantTitle {
+				action = &a
+				break
+			}
+		}
+		if action == nil {
+			t.Fatalf("can't find action with Title %s, only %#v",
+				wantTitle, actions)
+		}
+
+		// Execute the command.
+		// Its side effect should be a single showDocument request.
+		params := &protocol.ExecuteCommandParams{
+			Command:   action.Command.Command,
+			Arguments: action.Command.Arguments,
+		}
+		var result command.DebuggingResult
+		env.ExecuteCommand(params, &result)
+		doc := shownDocument(t, env, "http:")
+		if doc == nil {
+			t.Fatalf("no showDocument call had 'file:' prefix")
+		}
+		t.Log("showDocument(package doc) URL:", doc.URI)
+
+		// Get the report and do some minimal checks for sensible results.
+		// Use only portable instructions below!
+		report := get(t, doc.URI)
+		checkMatch(t, true, report, `TEXT.*example.com/a.f`)
+		checkMatch(t, true, report, `CALL	runtime.printlock`)
+		checkMatch(t, true, report, `CALL	runtime.printstring`)
+		checkMatch(t, true, report, `CALL	runtime.printunlock`)
+		checkMatch(t, true, report, `CALL	example.com/a.f.deferwrap1`)
+		checkMatch(t, true, report, `RET`)
+		checkMatch(t, true, report, `CALL	runtime.morestack_noctxt`)
+
+		// Nested functions are also shown.
+		checkMatch(t, true, report, `TEXT.*example.com/a.f.deferwrap1`)
+
+		// But other functions are not.
+		checkMatch(t, false, report, `TEXT.*example.com/a.g`)
+	})
+}
+
 // shownDocument returns the first shown document matching the URI prefix.
 // It may be nil.
 //

From 8f4c6b0bfa2ef40516afa835b61e07b6fcf5ed64 Mon Sep 17 00:00:00 2001
From: Alan Donovan 
Date: Fri, 7 Jun 2024 12:01:29 -0400
Subject: [PATCH 12/84] gopls/internal/test/integration/misc: fix build

Change-Id: I841144ce85051e4441090ef684beb7865b7273fb
Reviewed-on: https://go-review.googlesource.com/c/tools/+/591316
Auto-Submit: Alan Donovan 
LUCI-TryBot-Result: Go LUCI 
Reviewed-by: Than McIntosh 
---
 gopls/internal/test/integration/misc/webserver_test.go | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/gopls/internal/test/integration/misc/webserver_test.go b/gopls/internal/test/integration/misc/webserver_test.go
index 22f65ecd86a..ae89745012f 100644
--- a/gopls/internal/test/integration/misc/webserver_test.go
+++ b/gopls/internal/test/integration/misc/webserver_test.go
@@ -322,7 +322,7 @@ func g() {
 
 		// Invoke the "Show assembly" code action to start the server.
 		loc := env.RegexpSearch("a/a.go", "println")
-		actions, err := env.Editor.CodeAction(env.Ctx, loc, nil)
+		actions, err := env.Editor.CodeAction(env.Ctx, loc, nil, protocol.CodeActionUnknownTrigger)
 		if err != nil {
 			t.Fatalf("CodeAction: %v", err)
 		}

From 03419175878954be2b792c470f49819e7e0f616e Mon Sep 17 00:00:00 2001
From: Rob Findley 
Date: Fri, 7 Jun 2024 15:25:11 +0000
Subject: [PATCH 13/84] gopls/internal/cache: stop module cache refresh on view
 shutdown

As described in golang/go#67865, CL 590377 exacerbated a problem that
module cache refreshes may outlive the lifecycle of the view.

Fixes golang/go#67865

Change-Id: Ieafdf6601fee00b6e8ce705502a80224da071578
Reviewed-on: https://go-review.googlesource.com/c/tools/+/591315
Auto-Submit: Robert Findley 
LUCI-TryBot-Result: Go LUCI 
Reviewed-by: Alan Donovan 
---
 gopls/internal/cache/imports.go | 31 +++++++++++++++++++++++++++----
 gopls/internal/cache/view.go    |  1 +
 2 files changed, 28 insertions(+), 4 deletions(-)

diff --git a/gopls/internal/cache/imports.go b/gopls/internal/cache/imports.go
index 76a5855fe87..c467a851f8f 100644
--- a/gopls/internal/cache/imports.go
+++ b/gopls/internal/cache/imports.go
@@ -35,6 +35,18 @@ func newRefreshTimer(refresh func()) *refreshTimer {
 	}
 }
 
+// stop stops any future scheduled refresh.
+func (t *refreshTimer) stop() {
+	t.mu.Lock()
+	defer t.mu.Unlock()
+
+	if t.timer != nil {
+		t.timer.Stop()
+		t.timer = nil
+		t.refreshFn = nil // release resources
+	}
+}
+
 // schedule schedules the refresh function to run at some point in the future,
 // if no existing refresh is already scheduled.
 //
@@ -54,11 +66,16 @@ func (t *refreshTimer) schedule() {
 		}
 		t.timer = time.AfterFunc(delay, func() {
 			start := time.Now()
-			t.refreshFn()
 			t.mu.Lock()
-			t.duration = time.Since(start)
-			t.timer = nil
+			refreshFn := t.refreshFn
 			t.mu.Unlock()
+			if refreshFn != nil { // timer may be stopped.
+				refreshFn()
+				t.mu.Lock()
+				t.duration = time.Since(start)
+				t.timer = nil
+				t.mu.Unlock()
+			}
 		})
 	}
 }
@@ -70,7 +87,8 @@ func (t *refreshTimer) schedule() {
 type sharedModCache struct {
 	mu     sync.Mutex
 	caches map[string]*imports.DirInfoCache // GOMODCACHE -> cache content; never invalidated
-	timers map[string]*refreshTimer         // GOMODCACHE -> timer
+	// TODO(rfindley): consider stopping these timers when the session shuts down.
+	timers map[string]*refreshTimer // GOMODCACHE -> timer
 }
 
 func (c *sharedModCache) dirCache(dir string) *imports.DirInfoCache {
@@ -131,6 +149,11 @@ func newImportsState(backgroundCtx context.Context, modCache *sharedModCache, en
 	return s
 }
 
+// stopTimer stops scheduled refreshes of this imports state.
+func (s *importsState) stopTimer() {
+	s.refreshTimer.stop()
+}
+
 // runProcessEnvFunc runs goimports.
 //
 // Any call to runProcessEnvFunc will schedule a refresh of the imports state
diff --git a/gopls/internal/cache/view.go b/gopls/internal/cache/view.go
index 6f76c55a435..ab77e6381cb 100644
--- a/gopls/internal/cache/view.go
+++ b/gopls/internal/cache/view.go
@@ -470,6 +470,7 @@ func (v *View) filterFunc() func(protocol.DocumentURI) bool {
 func (v *View) shutdown() {
 	// Cancel the initial workspace load if it is still running.
 	v.cancelInitialWorkspaceLoad()
+	v.importsState.stopTimer()
 
 	v.snapshotMu.Lock()
 	if v.snapshot != nil {

From f5a26d251e8e7d0d5f6b2531b2f0fbb3d9c6fa86 Mon Sep 17 00:00:00 2001
From: Alan Donovan 
Date: Fri, 7 Jun 2024 12:00:25 -0400
Subject: [PATCH 14/84] gopls/internal/server: simplify FreeSymbols plumbing

Since only a view ID, not a View, is needed, we can simplify
the argument plumbing following the pattern used by Assembly.

Also, rename RenderPackageDoc to PackageDocHTML.

Change-Id: Ib12c26ff0960a3ba96a6b8e6872740dd8767dfbe
Reviewed-on: https://go-review.googlesource.com/c/tools/+/591157
Reviewed-by: Robert Findley 
Auto-Submit: Alan Donovan 
LUCI-TryBot-Result: Go LUCI 
---
 gopls/doc/commands.md                         | 19 +++++++-------
 gopls/internal/doc/api.json                   |  2 +-
 gopls/internal/golang/codeaction.go           |  3 ++-
 gopls/internal/golang/pkgdoc.go               |  6 ++---
 .../internal/protocol/command/command_gen.go  |  6 ++---
 gopls/internal/protocol/command/interface.go  |  2 +-
 gopls/internal/server/command.go              | 26 ++++++-------------
 gopls/internal/server/server.go               |  6 ++---
 8 files changed, 30 insertions(+), 40 deletions(-)

diff --git a/gopls/doc/commands.md b/gopls/doc/commands.md
index 357e3497de2..66614ce4d23 100644
--- a/gopls/doc/commands.md
+++ b/gopls/doc/commands.md
@@ -325,15 +325,16 @@ 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,
+		},
 	},
 }
 ```
diff --git a/gopls/internal/doc/api.json b/gopls/internal/doc/api.json
index 555c56ac8a6..a977633df91 100644
--- a/gopls/internal/doc/api.json
+++ b/gopls/internal/doc/api.json
@@ -1013,7 +1013,7 @@
 			"Command": "gopls.free_symbols",
 			"Title": "report free symbols referenced by the selection.",
 			"Doc": "This command is a query over a selected range of Go source\ncode. It reports the set of \"free\" symbols of the\nselection: the set of symbols that are referenced within\nthe selection but are declared outside of it. This\ninformation is useful for understanding at a glance what a\nblock of code depends on, perhaps as a precursor to\nextracting it into a separate function.",
-			"ArgDoc": "string,\n{\n\t// The range's start position.\n\t\"start\": {\n\t\t\"line\": uint32,\n\t\t\"character\": uint32,\n\t},\n\t// The range's end position.\n\t\"end\": {\n\t\t\"line\": uint32,\n\t\t\"character\": uint32,\n\t},\n}",
+			"ArgDoc": "string,\n{\n\t\"uri\": string,\n\t\"range\": {\n\t\t\"start\": {\n\t\t\t\"line\": uint32,\n\t\t\t\"character\": uint32,\n\t\t},\n\t\t\"end\": {\n\t\t\t\"line\": uint32,\n\t\t\t\"character\": uint32,\n\t\t},\n\t},\n}",
 			"ResultDoc": ""
 		},
 		{
diff --git a/gopls/internal/golang/codeaction.go b/gopls/internal/golang/codeaction.go
index 07f777eefdc..1aeecddff03 100644
--- a/gopls/internal/golang/codeaction.go
+++ b/gopls/internal/golang/codeaction.go
@@ -117,7 +117,8 @@ func CodeActions(ctx context.Context, snapshot *cache.Snapshot, fh file.Handle,
 		}
 
 		if want[protocol.GoFreeSymbols] && rng.End != rng.Start {
-			cmd, err := command.NewFreeSymbolsCommand("Show free symbols", pgf.URI, rng)
+			loc := protocol.Location{URI: pgf.URI, Range: rng}
+			cmd, err := command.NewFreeSymbolsCommand("Show free symbols", snapshot.View().ID(), loc)
 			if err != nil {
 				return nil, err
 			}
diff --git a/gopls/internal/golang/pkgdoc.go b/gopls/internal/golang/pkgdoc.go
index ce0e5bc7ac4..c605d29afe4 100644
--- a/gopls/internal/golang/pkgdoc.go
+++ b/gopls/internal/golang/pkgdoc.go
@@ -62,7 +62,7 @@ type (
 	PosURLFunc = func(filename string, line, col8 int) protocol.URI
 )
 
-// RenderPackageDoc formats the package documentation page.
+// 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
@@ -70,9 +70,7 @@ type (
 //
 // 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(pkg *cache.Package, posURL PosURLFunc, pkgURL PkgURLFunc) ([]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
diff --git a/gopls/internal/protocol/command/command_gen.go b/gopls/internal/protocol/command/command_gen.go
index 2ba73dbc90c..3f0142d0238 100644
--- a/gopls/internal/protocol/command/command_gen.go
+++ b/gopls/internal/protocol/command/command_gen.go
@@ -172,8 +172,8 @@ func Dispatch(ctx context.Context, params *protocol.ExecuteCommandParams, s Inte
 		}
 		return s.FetchVulncheckResult(ctx, a0)
 	case FreeSymbols:
-		var a0 protocol.DocumentURI
-		var a1 protocol.Range
+		var a0 string
+		var a1 protocol.Location
 		if err := UnmarshalArgs(params.Arguments, &a0, &a1); err != nil {
 			return nil, err
 		}
@@ -444,7 +444,7 @@ func NewFetchVulncheckResultCommand(title string, a0 URIArg) (protocol.Command,
 	}, nil
 }
 
-func NewFreeSymbolsCommand(title string, a0 protocol.DocumentURI, a1 protocol.Range) (protocol.Command, error) {
+func NewFreeSymbolsCommand(title string, a0 string, a1 protocol.Location) (protocol.Command, error) {
 	args, err := MarshalArgs(a0, a1)
 	if err != nil {
 		return protocol.Command{}, err
diff --git a/gopls/internal/protocol/command/interface.go b/gopls/internal/protocol/command/interface.go
index 9f76add467d..6e0e2684a8a 100644
--- a/gopls/internal/protocol/command/interface.go
+++ b/gopls/internal/protocol/command/interface.go
@@ -251,7 +251,7 @@ type Interface interface {
 	// information is useful for understanding at a glance what a
 	// block of code depends on, perhaps as a precursor to
 	// extracting it into a separate function.
-	FreeSymbols(context.Context, protocol.DocumentURI, protocol.Range) error
+	FreeSymbols(ctx context.Context, viewID string, loc protocol.Location) error
 
 	// Assembly: Show disassembly of current function.
 	//
diff --git a/gopls/internal/server/command.go b/gopls/internal/server/command.go
index 04febab9a33..201a96d2d66 100644
--- a/gopls/internal/server/command.go
+++ b/gopls/internal/server/command.go
@@ -1465,24 +1465,14 @@ func (c *commandHandler) Views(ctx context.Context) ([]command.View, error) {
 	return summaries, nil
 }
 
-func (c *commandHandler) FreeSymbols(ctx context.Context, uri protocol.DocumentURI, rng protocol.Range) error {
-	// TODO(adonovan): simplify, following Assembly, by putting the
-	// viewID in the command so that c.run isn't necessary.
-	// (freesymbolsURL needs only a viewID, not a view.)
-	return c.run(ctx, commandConfig{
-		forURI: uri,
-	}, func(ctx context.Context, deps commandDeps) error {
-		web, err := c.s.getWeb()
-		if err != nil {
-			return err
-		}
-		url := web.freesymbolsURL(deps.snapshot.View(), protocol.Location{
-			URI:   deps.fh.URI(),
-			Range: rng,
-		})
-		openClientBrowser(ctx, c.s.client, url)
-		return nil
-	})
+func (c *commandHandler) FreeSymbols(ctx context.Context, viewID string, loc protocol.Location) error {
+	web, err := c.s.getWeb()
+	if err != nil {
+		return err
+	}
+	url := web.freesymbolsURL(viewID, loc)
+	openClientBrowser(ctx, c.s.client, url)
+	return nil
 }
 
 func (c *commandHandler) Assembly(ctx context.Context, viewID, packageID, symbol string) error {
diff --git a/gopls/internal/server/server.go b/gopls/internal/server/server.go
index 2c0c34ac98e..58747bbd6b9 100644
--- a/gopls/internal/server/server.go
+++ b/gopls/internal/server/server.go
@@ -350,7 +350,7 @@ func (s *server) initWeb() (*web, error) {
 		pkgURL := func(path golang.PackagePath, fragment string) protocol.URI {
 			return web.pkgURL(view, path, fragment)
 		}
-		content, err := golang.RenderPackageDoc(pkgs[0], web.openURL, pkgURL)
+		content, err := golang.PackageDocHTML(pkgs[0], web.openURL, pkgURL)
 		if err != nil {
 			http.Error(w, err.Error(), http.StatusInternalServerError)
 			return
@@ -494,7 +494,7 @@ func (w *web) pkgURL(v *cache.View, path golang.PackagePath, fragment string) pr
 
 // freesymbolsURL returns a /freesymbols URL for a report
 // on the free symbols referenced within the selection span (loc).
-func (w *web) freesymbolsURL(v *cache.View, loc protocol.Location) protocol.URI {
+func (w *web) freesymbolsURL(viewID string, loc protocol.Location) protocol.URI {
 	return w.url(
 		"freesymbols",
 		fmt.Sprintf("file=%s&range=%d:%d:%d:%d&view=%s",
@@ -503,7 +503,7 @@ func (w *web) freesymbolsURL(v *cache.View, loc protocol.Location) protocol.URI
 			loc.Range.Start.Character,
 			loc.Range.End.Line,
 			loc.Range.End.Character,
-			url.QueryEscape(v.ID())),
+			url.QueryEscape(viewID)),
 		"")
 }
 

From f41a407b04d4ad2114f43cef35ae284fcbf8ae89 Mon Sep 17 00:00:00 2001
From: Alan Donovan 
Date: Fri, 7 Jun 2024 12:44:16 -0400
Subject: [PATCH 15/84] gopls/internal/golang: Web, an abstraction of
 server.web

This change consolidates the two func types, PosURL and PkgURL,
into an interface, Web, that aligns with the server.web
implementation.

Also, strength-reduce PkgURL to require only a viewID,
not a view (as we did for freesymbolsURL in CL 591157).

Change-Id: Ic48e0d5808257934c56b31126fd4880ee88c7a33
Reviewed-on: https://go-review.googlesource.com/c/tools/+/591318
Commit-Queue: Alan Donovan 
Reviewed-by: Robert Findley 
LUCI-TryBot-Result: Go LUCI 
Auto-Submit: Alan Donovan 
---
 gopls/internal/golang/assembly.go    |  4 ++--
 gopls/internal/golang/freesymbols.go |  6 +++---
 gopls/internal/golang/pkgdoc.go      | 31 +++++++++++++---------------
 gopls/internal/server/command.go     |  2 +-
 gopls/internal/server/server.go      | 22 +++++++-------------
 5 files changed, 28 insertions(+), 37 deletions(-)

diff --git a/gopls/internal/golang/assembly.go b/gopls/internal/golang/assembly.go
index cbb951288a6..ca9f61cefb3 100644
--- a/gopls/internal/golang/assembly.go
+++ b/gopls/internal/golang/assembly.go
@@ -30,7 +30,7 @@ import (
 // TODO(adonovan):
 // - display a "Compiling..." message as a cold build can be slow.
 // - cross-link jumps and block labels, like github.com/aclements/objbrowse.
-func AssemblyHTML(ctx context.Context, snapshot *cache.Snapshot, pkg *cache.Package, symbol string, posURL PosURLFunc) ([]byte, error) {
+func AssemblyHTML(ctx context.Context, snapshot *cache.Snapshot, pkg *cache.Package, symbol string, web Web) ([]byte, error) {
 	// Compile the package with -S, and capture its stderr stream.
 	inv, cleanupInvocation, err := snapshot.GoCommandInvocation(false, &gocommand.Invocation{
 		Verb:       "build",
@@ -145,7 +145,7 @@ function httpGET(url) {
 			if file, linenum, ok := cutLast(parts[2], ":"); ok && !strings.HasPrefix(file, "<") {
 				if linenum, err := strconv.Atoi(linenum); err == nil {
 					text := fmt.Sprintf("L%04d", linenum)
-					link = sourceLink(text, posURL(file, linenum, 1))
+					link = sourceLink(text, web.OpenURL(file, linenum, 1))
 				}
 			}
 			fmt.Fprintf(&buf, "%s\t%s\t%s", escape(parts[1]), link, escape(parts[3]))
diff --git a/gopls/internal/golang/freesymbols.go b/gopls/internal/golang/freesymbols.go
index 4333a8505a5..0bf0d9c2e24 100644
--- a/gopls/internal/golang/freesymbols.go
+++ b/gopls/internal/golang/freesymbols.go
@@ -27,7 +27,7 @@ import (
 
 // FreeSymbolsHTML returns an HTML document containing the report of
 // free symbols referenced by the selection.
-func FreeSymbolsHTML(pkg *cache.Package, pgf *parsego.File, start, end token.Pos, posURL PosURLFunc, pkgURL PkgURLFunc) []byte {
+func FreeSymbolsHTML(viewID string, pkg *cache.Package, pgf *parsego.File, start, end token.Pos, web Web) []byte {
 
 	// Compute free references.
 	refs := freeRefs(pkg.Types(), pkg.TypesInfo(), pgf.File, start, end)
@@ -210,7 +210,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, ", ")) } @@ -236,7 +236,7 @@ function httpGET(url) { 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 sourceLink(text, web.OpenURL(posn.Filename, posn.Line, posn.Column)) } return text } diff --git a/gopls/internal/golang/pkgdoc.go b/gopls/internal/golang/pkgdoc.go index c605d29afe4..94e00befc23 100644 --- a/gopls/internal/golang/pkgdoc.go +++ b/gopls/internal/golang/pkgdoc.go @@ -53,24 +53,21 @@ import ( "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 +// 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 - // A PosURLFunc forms URLs that cause the editor to navigate to a position. - PosURLFunc = func(filename string, line, col8 int) protocol.URI -) + // OpenURL forms URLs that cause the editor to open a file at a specific position. + OpenURL(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. -func PackageDocHTML(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 @@ -143,7 +140,7 @@ func PackageDocHTML(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) { @@ -334,7 +331,7 @@ window.onload = () => { 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 sourceLink(text, web.OpenURL(posn.Filename, posn.Line, posn.Column)) } return text } @@ -350,7 +347,7 @@ window.onload = () => { // imported package name? if pkgname, ok := obj.(*types.PkgName); ok { // TODO(adonovan): do this for Defs of PkgName too. - return pkgURL(PackagePath(pkgname.Imported().Path()), "") + return web.PkgURL(viewID, PackagePath(pkgname.Imported().Path()), "") } // package-level symbol? @@ -358,7 +355,7 @@ window.onload = () => { if obj.Pkg() == pkg.Types() { return "#" + obj.Name() // intra-package ref } else { - return pkgURL(PackagePath(obj.Pkg().Path()), obj.Name()) + return web.PkgURL(viewID, PackagePath(obj.Pkg().Path()), obj.Name()) } } @@ -369,7 +366,7 @@ window.onload = () => { _, named := typesinternal.ReceiverNamed(sig.Recv()) if named != nil { fragment := named.Obj().Name() + "." + fn.Name() - return pkgURL(PackagePath(fn.Pkg().Path()), fragment) + return web.PkgURL(viewID, PackagePath(fn.Pkg().Path()), fragment) } } return "" @@ -671,7 +668,7 @@ window.onload = () => { fmt.Fprintf(&buf, "

    Source files

    \n") for _, filename := range docpkg.Filenames { fmt.Fprintf(&buf, "
    %s
    \n", - sourceLink(filepath.Base(filename), posURL(filename, 1, 1))) + sourceLink(filepath.Base(filename), web.OpenURL(filename, 1, 1))) } fmt.Fprintf(&buf, "\n") diff --git a/gopls/internal/server/command.go b/gopls/internal/server/command.go index 201a96d2d66..75f51347003 100644 --- a/gopls/internal/server/command.go +++ b/gopls/internal/server/command.go @@ -571,7 +571,7 @@ func (c *commandHandler) Doc(ctx context.Context, loc protocol.Location) error { } // Direct the client to open the /pkg page. - url := web.pkgURL(deps.snapshot.View(), pkgpath, fragment) + url := web.PkgURL(deps.snapshot.View().ID(), pkgpath, fragment) openClientBrowser(ctx, c.s.client, url) return nil diff --git a/gopls/internal/server/server.go b/gopls/internal/server/server.go index 58747bbd6b9..a4149422e2e 100644 --- a/gopls/internal/server/server.go +++ b/gopls/internal/server/server.go @@ -347,10 +347,7 @@ func (s *server) initWeb() (*web, error) { http.Error(w, err.Error(), http.StatusInternalServerError) return } - pkgURL := func(path golang.PackagePath, fragment string) protocol.URI { - return web.pkgURL(view, path, fragment) - } - content, err := golang.PackageDocHTML(pkgs[0], web.openURL, pkgURL) + content, err := golang.PackageDocHTML(view.ID(), pkgs[0], web) if err != nil { http.Error(w, err.Error(), http.StatusInternalServerError) return @@ -405,10 +402,7 @@ func (s *server) initWeb() (*web, error) { } // Produce report. - pkgURL := func(path golang.PackagePath, fragment string) protocol.URI { - return web.pkgURL(view, path, fragment) - } - html := golang.FreeSymbolsHTML(pkg, pgf, start, end, web.openURL, pkgURL) + html := golang.FreeSymbolsHTML(view.ID(), pkg, pgf, start, end, web) w.Write(html) }) @@ -453,7 +447,7 @@ func (s *server) initWeb() (*web, error) { pkg := pkgs[0] // Produce report. - html, err := golang.AssemblyHTML(ctx, snapshot, pkg, symbol, web.openURL) + html, err := golang.AssemblyHTML(ctx, snapshot, pkg, symbol, web) if err != nil { http.Error(w, err.Error(), http.StatusInternalServerError) return @@ -469,26 +463,26 @@ func (s *server) initWeb() (*web, error) { //go:embed assets/* var assets embed.FS -// openURL returns an /open URL that, when visited, causes the client +// OpenURL returns an /open URL that, when visited, causes the client // editor to open the specified file/line/column (in 1-based UTF-8 // coordinates). // // (Rendering may generate hundreds of positions across files of many // packages, so don't convert to LSP coordinates yet: wait until the // URL is opened.) -func (w *web) openURL(filename string, line, col8 int) protocol.URI { +func (w *web) OpenURL(filename string, line, col8 int) protocol.URI { return w.url( "open", fmt.Sprintf("file=%s&line=%d&col=%d", url.QueryEscape(filename), line, col8), "") } -// pkgURL returns a /pkg URL for the documentation of the specified package. +// PkgURL returns a /pkg URL for the documentation of the specified package. // The optional fragment must be of the form "Println" or "Buffer.WriteString". -func (w *web) pkgURL(v *cache.View, path golang.PackagePath, fragment string) protocol.URI { +func (w *web) PkgURL(viewID string, path golang.PackagePath, fragment string) protocol.URI { return w.url( "pkg/"+string(path), - "view="+url.QueryEscape(v.ID()), + "view="+url.QueryEscape(viewID), fragment) } From 413a4912710c4fa5586dcda78006b48c19093cc0 Mon Sep 17 00:00:00 2001 From: Alan Donovan Date: Fri, 7 Jun 2024 14:14:47 -0400 Subject: [PATCH 16/84] gopls/internal/golang: factor the 3 web reports This change factors the common elements of the three reports: - the common CSS and JS, previously constants, are now assets; only the ad hoc styles particular to each page remain in the HTML. - The disconnect banner element is now created on load, in common.js, so no
    HTML is required. - objHTML, sourceLink are factored out. Also: - use the same font-families as pkg.go.dev. - use addEventListener instead of clobbering window.onload. Change-Id: Ic21cc46fc8d92a94b78aa1faf5b2f3012f539e57 Reviewed-on: https://go-review.googlesource.com/c/tools/+/591355 Auto-Submit: Alan Donovan Reviewed-by: Robert Findley LUCI-TryBot-Result: Go LUCI --- gopls/internal/golang/assembly.go | 39 +--- gopls/internal/golang/freesymbols.go | 80 +++---- gopls/internal/golang/pkgdoc.go | 215 +++--------------- gopls/internal/server/assets/common.css | 116 ++++++++++ gopls/internal/server/assets/common.js | 28 +++ gopls/internal/server/server.go | 12 +- .../test/integration/misc/webserver_test.go | 6 +- 7 files changed, 214 insertions(+), 282 deletions(-) create mode 100644 gopls/internal/server/assets/common.css create mode 100644 gopls/internal/server/assets/common.js diff --git a/gopls/internal/golang/assembly.go b/gopls/internal/golang/assembly.go index ca9f61cefb3..dc5b58900b7 100644 --- a/gopls/internal/golang/assembly.go +++ b/gopls/internal/golang/assembly.go @@ -50,7 +50,6 @@ func AssemblyHTML(ctx context.Context, snapshot *cache.Snapshot, pkg *cache.Pack escape := html.EscapeString // Produce the report. - // TODO(adonovan): factor with RenderPkgDoc, FreeSymbolsHTML title := fmt.Sprintf("%s assembly for %s", escape(snapshot.View().GOARCH()), escape(symbol)) @@ -59,31 +58,11 @@ func AssemblyHTML(ctx context.Context, snapshot *cache.Snapshot, pkg *cache.Pack - ` + escape(title) + ` - + + -
    Gopls server has terminated. Page is inactive.

    ` + title + `

    A Quick Guide to Go's Assembler @@ -101,18 +80,6 @@ function httpGET(url) {

     `)
     
    -	// sourceLink returns HTML for a link to open a file in the client editor.
    -	// TODO(adonovan): factor with two other copies.
    -	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`,
    -			escape(url), text)
    -	}
    -
     	// insnRx matches an assembly instruction line.
     	// Submatch groups are: (offset-hex-dec, file-line-column, instruction).
     	insnRx := regexp.MustCompile(`^(\s+0x[0-9a-f ]+)\(([^)]*)\)\s+(.*)$`)
    @@ -145,7 +112,7 @@ function httpGET(url) {
     			if file, linenum, ok := cutLast(parts[2], ":"); ok && !strings.HasPrefix(file, "<") {
     				if linenum, err := strconv.Atoi(linenum); err == nil {
     					text := fmt.Sprintf("L%04d", linenum)
    -					link = sourceLink(text, web.OpenURL(file, linenum, 1))
    +					link = sourceLink(text, web.SrcURL(file, linenum, 1))
     				}
     			}
     			fmt.Fprintf(&buf, "%s\t%s\t%s", escape(parts[1]), link, escape(parts[3]))
    diff --git a/gopls/internal/golang/freesymbols.go b/gopls/internal/golang/freesymbols.go
    index 0bf0d9c2e24..6a71c20e8c5 100644
    --- a/gopls/internal/golang/freesymbols.go
    +++ b/gopls/internal/golang/freesymbols.go
    @@ -161,40 +161,11 @@ func FreeSymbolsHTML(viewID string, pkg *cache.Package, pgf *parsego.File, start
     .col-local { color: #0cb7c9 }
     li { font-family: monospace; }
     p { max-width: 6in; }
    -#disconnected {
    -  position: fixed;
    -  top: 1em;
    -  left: 1em;
    -  display: none; /* initially */
    -  background-color: white;
    -  border: thick solid red;
    -  padding: 2em;
    -}
     
    -
    -  
    +  
    +  
     
     
    -
    Gopls server has terminated. Page is inactive.

    Free symbols

    The selected code contains references to these free* symbols: @@ -219,28 +190,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, web.OpenURL(posn.Filename, posn.Line, posn.Column)) - } - return text - } - // -- package and local symbols -- showSymbols := func(scope, title string, symbols []Symbol) { @@ -253,7 +202,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)) } @@ -452,3 +401,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/pkgdoc.go b/gopls/internal/golang/pkgdoc.go index 94e00befc23..93761d69254 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 ( @@ -58,8 +57,8 @@ type Web interface { // PkgURL forms URLs of package or symbol documentation. PkgURL(viewID string, path PackagePath, fragment string) protocol.URI - // OpenURL forms URLs that cause the editor to open a file at a specific position. - OpenURL(filename string, line, col8 int) 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. @@ -199,39 +198,40 @@ func PackageDocHTML(viewID string, pkg *cache.Package, web Web) ([]byte, error) - ` + title + ` - + +
    -
    Gopls server has terminated. Page is inactive.