diff --git a/.golangci.yml b/.golangci.yml index a53b93cc05..ba5a81b1c1 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -115,6 +115,11 @@ issues: # G115 checks for use of truncating conversions. path: private/buf/buflsp/file.go text: "G115:" + - linters: + - gosec + # G115 checks for use of truncating conversions. + path: private/buf/buflsp/image.go + text: "G115:" - linters: - gosec # G115 checks for use of truncating conversions. diff --git a/CHANGELOG.md b/CHANGELOG.md index cc2a049253..ec4d834d4e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,7 +2,7 @@ ## [Unreleased] -- No changes yet. +- Breaking analysis support for `buf beta lsp`. ## [v1.47.2] - 2024-11-14 diff --git a/private/buf/buflsp/buflsp.go b/private/buf/buflsp/buflsp.go index 9d04d840e3..536cf71d43 100644 --- a/private/buf/buflsp/buflsp.go +++ b/private/buf/buflsp/buflsp.go @@ -65,6 +65,7 @@ func Serve( &connWrapper{Conn: conn, logger: container.Logger()}, zap.NewNop(), // The logging from protocol itself isn't very good, we've replaced it with connAdapter here. ), + container: container, logger: container.Logger(), controller: controller, checkClient: checkClient, @@ -89,8 +90,9 @@ func Serve( // Its handler methods are not defined in buflsp.go; they are defined in other files, grouped // according to the groupings in type lsp struct { - conn jsonrpc2.Conn - client protocol.Client + conn jsonrpc2.Conn + client protocol.Client + container appext.Container logger *slog.Logger controller bufctl.Controller @@ -130,19 +132,38 @@ func (l *lsp) init(_ context.Context, params *protocol.InitializeParams) error { // to inject debug logging, tracing, and timeouts to requests. func (l *lsp) newHandler() jsonrpc2.Handler { actual := protocol.ServerHandler(newServer(l), nil) - return func(ctx context.Context, reply jsonrpc2.Replier, req jsonrpc2.Request) (retErr error) { - defer slogext.DebugProfile(l.logger, slog.String("method", req.Method()), slog.Any("params", req.Params()))() + return jsonrpc2.AsyncHandler(func(ctx context.Context, reply jsonrpc2.Replier, req jsonrpc2.Request) error { + l.logger.Debug( + "handling request", + slog.String("method", req.Method()), + slog.Any("params", req.Params()), + ) + defer slogext.DebugProfile( + l.logger, + slog.String("method", req.Method()), + slog.Any("params", req.Params()), + )() replier := l.wrapReplier(reply, req) - // Verify that the server has been initialized if this isn't the initialization - // request. + var err error if req.Method() != protocol.MethodInitialize && l.initParams.Load() == nil { - return replier(ctx, nil, fmt.Errorf("the first call to the server must be the %q method", protocol.MethodInitialize)) + // Verify that the server has been initialized if this isn't the initialization + // request. + err = replier(ctx, nil, fmt.Errorf("the first call to the server must be the %q method", protocol.MethodInitialize)) + } else { + l.lock.Lock() + err = actual(ctx, replier, req) + l.lock.Unlock() } - l.lock.Lock() - defer l.lock.Unlock() - return actual(ctx, replier, req) - } + if err != nil { + l.logger.Error( + "error while replying to request", + slog.String("method", req.Method()), + slogext.ErrorAttr(err), + ) + } + return nil + }) } diff --git a/private/buf/buflsp/config.go b/private/buf/buflsp/config.go new file mode 100644 index 0000000000..53618ffb22 --- /dev/null +++ b/private/buf/buflsp/config.go @@ -0,0 +1,63 @@ +// Copyright 2020-2024 Buf Technologies, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package buflsp + +// Configuration keys for the LSP. +// +// These are part of the public API of the LSP server, in that any user of +// the LSP can configure it using these keys. The server requests them over +// the LSP protocol as-needed. +// +// Keep in sync with bufbuild/vscode-buf package.json. +const ( + // The strategy for how to calculate the --against input for a breaking + // check. This must be one of the following values: + // + // - "git". Use a particular Git revision to find the against file. + // + // - "disk". Use the last-saved value on disk as the against file. + ConfigBreakingStrategy = "buf.checks.breaking.againstStrategy" + // The Git revision to use for calculating the --against input for a + // breaking check when using the "git" strategy. + ConfigBreakingGitRef = "buf.checks.breaking.againstGitRef" +) + +const ( + // Compare against the configured git branch. + againstGit againstStrategy = iota + 1 + // Against the last saved file on disk (i.e. saved vs unsaved changes). + againstDisk +) + +// againstStrategy is a strategy for selecting which version of a file to use as +// --against for the purposes of breaking lints. +type againstStrategy int + +// parseAgainstStrategy parses an againstKind from a config setting sent by +// the client. +// +// Returns againstTrunk, false if the value is not recognized. +func parseAgainstStrategy(s string) (againstStrategy, bool) { + switch s { + // These values are the same as those present in the package.json for the + // VSCode client. + case "git": + return againstGit, true + case "disk": + return againstDisk, true + default: + return againstGit, false + } +} diff --git a/private/buf/buflsp/file.go b/private/buf/buflsp/file.go index bbd8d9188a..8bad05ec37 100644 --- a/private/buf/buflsp/file.go +++ b/private/buf/buflsp/file.go @@ -17,6 +17,7 @@ package buflsp import ( + "bytes" "context" "errors" "fmt" @@ -31,18 +32,15 @@ import ( "github.com/bufbuild/buf/private/bufpkg/bufcheck" "github.com/bufbuild/buf/private/bufpkg/bufimage" "github.com/bufbuild/buf/private/bufpkg/bufmodule" + "github.com/bufbuild/buf/private/pkg/git" "github.com/bufbuild/buf/private/pkg/ioext" + "github.com/bufbuild/buf/private/pkg/normalpath" "github.com/bufbuild/buf/private/pkg/slogext" "github.com/bufbuild/buf/private/pkg/storage" - "github.com/bufbuild/protocompile" "github.com/bufbuild/protocompile/ast" - "github.com/bufbuild/protocompile/linker" "github.com/bufbuild/protocompile/parser" - "github.com/bufbuild/protocompile/protoutil" "github.com/bufbuild/protocompile/reporter" - "github.com/google/uuid" "go.lsp.dev/protocol" - "google.golang.org/protobuf/reflect/protoreflect" ) const descriptorPath = "google/protobuf/descriptor.proto" @@ -60,20 +58,22 @@ type file struct { // diagnostics or symbols an operating refers to. version int32 hasText bool // Whether this file has ever had text read into it. - // Always set false->true. Once true, never becomes false again. workspace bufworkspace.Workspace module bufmodule.Module + againstStrategy againstStrategy + againstGitRef string + objectInfo storage.ObjectInfo importablePathToObject map[string]storage.ObjectInfo - fileNode *ast.FileNode - packageNode *ast.PackageNode - diagnostics []protocol.Diagnostic - importToFile map[string]*file - symbols []*symbol - image bufimage.Image + fileNode *ast.FileNode + packageNode *ast.PackageNode + diagnostics []protocol.Diagnostic + importToFile map[string]*file + symbols []*symbol + image, againstImage bufimage.Image } // IsWKT returns whether this file corresponds to a well-known type. @@ -170,6 +170,99 @@ func (f *file) Update(ctx context.Context, version int32, text string) { f.hasText = true } +// FetchSettings refreshes configuration settings for this file. +// +// This only needs to happen when the file is open or when the client signals +// that configuration settings have changed. +func (f *file) RefreshSettings(ctx context.Context) { + settings, err := f.lsp.client.Configuration(ctx, &protocol.ConfigurationParams{ + Items: []protocol.ConfigurationItem{ + {ScopeURI: f.uri, Section: ConfigBreakingStrategy}, + {ScopeURI: f.uri, Section: ConfigBreakingGitRef}, + }, + }) + if err != nil { + // We can throw the error away, since the handler logs it for us. + return + } + + // NOTE: indices here are those from the array in the call to Configuration above. + f.againstStrategy = getSetting(f, settings, ConfigBreakingStrategy, 0, parseAgainstStrategy) + f.againstGitRef = getSetting(f, settings, ConfigBreakingGitRef, 1, func(s string) (string, bool) { return s, true }) + + switch f.againstStrategy { + case againstDisk: + f.againstGitRef = "" + case againstGit: + // Check to see if the user setting is a valid Git ref. + err := git.IsValidRef( + ctx, + f.lsp.container, + normalpath.Dir(f.uri.Filename()), + f.againstGitRef, + ) + if err != nil { + f.lsp.logger.Warn( + "failed to validate buf.againstGit", + slog.String("uri", string(f.uri)), + slogext.ErrorAttr(err), + ) + f.againstGitRef = "" + } else { + f.lsp.logger.Debug( + "found remote branch", + slog.String("uri", string(f.uri)), + slog.String("ref", f.againstGitRef), + ) + } + } +} + +// getSetting is a helper that extracts a configuration setting from the return +// value of [protocol.Client.Configuration]. +// +// The parse function should convert the JSON value we get from the protocol +// (such as a string), potentially performing validation, and returning a default +// value on validation failure. +func getSetting[T, U any](f *file, settings []any, name string, index int, parse func(T) (U, bool)) (value U) { + if len(settings) <= index { + f.lsp.logger.Warn( + "missing config setting", + slog.String("setting", name), + slog.String("uri", string(f.uri)), + ) + } + + if raw, ok := settings[index].(T); ok { + // For invalid settings, this will default to againstTrunk for us! + value, ok = parse(raw) + if !ok { + f.lsp.logger.Warn( + "invalid config setting", + slog.String("setting", name), + slog.String("uri", string(f.uri)), + slog.Any("raw", raw), + ) + } + } else { + f.lsp.logger.Warn( + "invalid config setting", + slog.String("setting", name), + slog.String("uri", string(f.uri)), + slog.Any("raw", raw), + ) + } + + f.lsp.logger.Debug( + "parsed config setting", + slog.String("setting", name), + slog.String("uri", string(f.uri)), + slog.Any("value", value), + ) + + return value +} + // Refresh rebuilds all of a file's internal book-keeping. // // If deep is set, this will also load imports and refresh those, too. @@ -188,12 +281,13 @@ func (f *file) Refresh(ctx context.Context) { progress.Report(ctx, "Indexing Imports", 2.0/6) f.IndexImports(ctx) - progress.Report(ctx, "Detecting Module", 3.0/6) + progress.Report(ctx, "Detecting Buf Module", 3.0/6) f.FindModule(ctx) - progress.Report(ctx, "Linking Descriptors", 4.0/6) - f.BuildImage(ctx) + progress.Report(ctx, "Running Checks", 4.0/6) + f.BuildImages(ctx) f.RunLints(ctx) + f.RunBreaking(ctx) progress.Report(ctx, "Indexing Symbols", 5.0/6) f.IndexSymbols(ctx) @@ -210,10 +304,6 @@ func (f *file) Refresh(ctx context.Context) { // // Returns whether a reparse was necessary. func (f *file) RefreshAST(ctx context.Context) bool { - if f.fileNode != nil { - return false - } - // NOTE: We intentionally do not use var report report here, because we need // report to be non-nil when empty; this is because if it is nil, when calling // PublishDiagnostics() below it will be serialized as JSON null. @@ -429,132 +519,113 @@ func (f *file) IndexImports(ctx context.Context) { // Parse the imported file and find all symbols in it, but do not // index symbols in the import's imports, otherwise we will recursively // index the universe and that would be quite slow. - file.RefreshAST(ctx) + + if f.fileNode != nil { + file.RefreshAST(ctx) + } file.IndexSymbols(ctx) } } -// BuildImage builds a Buf Image for this file. This does not use the controller to build -// the image, because we need delicate control over the input files: namely, for the case -// when we depend on a file that has been opened and modified in the editor. +// newFileOpener returns a fileOpener for the context of this file. // -// This operation requires IndexImports(). -func (f *file) BuildImage(ctx context.Context) { - importable := f.importablePathToObject - fileInfo := f.objectInfo - - if importable == nil || fileInfo == nil { - return - } - - var report report - var symbols linker.Symbols - compiler := protocompile.Compiler{ - SourceInfoMode: protocompile.SourceInfoExtraOptionLocations, - Resolver: &protocompile.SourceResolver{ - Accessor: func(path string) (io.ReadCloser, error) { - var uri protocol.URI - fileInfo, ok := importable[path] - if ok { - uri = protocol.URI("file://" + fileInfo.LocalPath()) - } else { - uri = protocol.URI("file://" + path) - } - - if file := f.Manager().Get(uri); file != nil { - return ioext.CompositeReadCloser(strings.NewReader(file.text), ioext.NopCloser), nil - } else if !ok { - return nil, os.ErrNotExist - } - - return os.Open(fileInfo.LocalPath()) - }, - }, - Symbols: &symbols, - Reporter: &report, - } - - compiled, err := compiler.Compile(ctx, fileInfo.Path()) - if err != nil { - f.diagnostics = report.diagnostics - } - if compiled[0] == nil { - return +// May return nil, if insufficient information is present to open the file. +func (f *file) newFileOpener() fileOpener { + if f.importablePathToObject == nil { + return nil } - var imageFiles []bufimage.ImageFile - seen := map[string]bool{} - - queue := []protoreflect.FileDescriptor{compiled[0]} - for len(queue) > 0 { - descriptor := queue[len(queue)-1] - queue = queue[:len(queue)-1] - - if seen[descriptor.Path()] { - continue + return func(path string) (io.ReadCloser, error) { + var uri protocol.URI + fileInfo, ok := f.importablePathToObject[path] + if ok { + uri = protocol.URI("file://" + fileInfo.LocalPath()) + } else { + uri = protocol.URI("file://" + path) } - seen[descriptor.Path()] = true - unused, ok := report.pathToUnusedImports[descriptor.Path()] - var unusedIndices []int32 - if ok { - unusedIndices = make([]int32, 0, len(unused)) + if file := f.Manager().Get(uri); file != nil { + return ioext.CompositeReadCloser(strings.NewReader(file.text), ioext.NopCloser), nil + } else if !ok { + return nil, os.ErrNotExist } - imports := descriptor.Imports() - for i := 0; i < imports.Len(); i++ { - dep := imports.Get(i).FileDescriptor - if dep == nil { - f.lsp.logger.Warn(fmt.Sprintf("found nil FileDescriptor for import %s", imports.Get(i).Path())) - continue - } + return os.Open(fileInfo.LocalPath()) + } +} - queue = append(queue, dep) +// newAgainstFileOpener returns a fileOpener for building the --against file +// for this file. In other words, this pulls files out of the git index, if +// necessary. +// +// May return nil, if there is insufficient information to build an --against +// file. +func (f *file) newAgainstFileOpener(ctx context.Context) fileOpener { + if !f.IsLocal() || f.importablePathToObject == nil { + return nil + } - if unused != nil { - if _, ok := unused[dep.Path()]; ok { - unusedIndices = append(unusedIndices, int32(i)) - } - } - } + if f.againstStrategy == againstGit && f.againstGitRef == "" { + return nil + } - descriptorProto := protoutil.ProtoFromFileDescriptor(descriptor) - if descriptorProto == nil { - err = fmt.Errorf("protoutil.ProtoFromFileDescriptor() returned nil for %q", descriptor.Path()) - break + return func(path string) (io.ReadCloser, error) { + fileInfo, ok := f.importablePathToObject[path] + if !ok { + return nil, fmt.Errorf("failed to resolve import: %s", path) } - var imageFile bufimage.ImageFile - imageFile, err = bufimage.NewImageFile( - descriptorProto, - nil, - uuid.UUID{}, - "", - descriptor.Path(), - descriptor.Path() != fileInfo.Path(), - report.syntaxMissing[descriptor.Path()], - unusedIndices, + var ( + data []byte + err error ) - if err != nil { - break + if f.againstGitRef != "" { + data, err = git.ReadFileAtRef( + ctx, + f.lsp.container, + fileInfo.LocalPath(), + f.againstGitRef, + ) + } + + if data == nil || errors.Is(err, git.ErrInvalidGitCheckout) { + return os.Open(fileInfo.LocalPath()) } - imageFiles = append(imageFiles, imageFile) - f.lsp.logger.Debug(fmt.Sprintf("added image file for %s", descriptor.Path())) + return ioext.CompositeReadCloser(bytes.NewReader(data), ioext.NopCloser), err } +} - if err != nil { - f.lsp.logger.Warn("could not build image", slog.String("uri", string(f.uri)), slogext.ErrorAttr(err)) +// BuildImages builds Buf Images for this file, to be used with linting +// routines. +// +// This operation requires IndexImports(). +func (f *file) BuildImages(ctx context.Context) { + if f.objectInfo == nil { return } - image, err := bufimage.NewImage(imageFiles) - if err != nil { - f.lsp.logger.Warn("could not build image", slog.String("uri", string(f.uri)), slogext.ErrorAttr(err)) - return + if opener := f.newFileOpener(); opener != nil { + image, diagnostics := buildImage(ctx, f.objectInfo.Path(), f.lsp.logger, opener) + if len(diagnostics) > 0 { + f.diagnostics = diagnostics + } + f.image = image + } else { + f.lsp.logger.Warn("not building image", slog.String("uri", string(f.uri))) } - f.image = image + if opener := f.newAgainstFileOpener(ctx); opener != nil { + // We explicitly throw the diagnostics away. + image, diagnostics := buildImage(ctx, f.objectInfo.Path(), f.lsp.logger, opener) + + f.againstImage = image + if image == nil { + f.lsp.logger.Warn("failed to build --against image", slog.Any("diagnostics", diagnostics)) + } + } else { + f.lsp.logger.Warn("not building --against image", slog.String("uri", string(f.uri))) + } } // RunLints runs linting on this file. Returns whether any lints failed. @@ -566,41 +637,61 @@ func (f *file) RunLints(ctx context.Context) bool { return false } - workspace := f.workspace - module := f.module - image := f.image - - if module == nil || image == nil { + if f.module == nil || f.image == nil { f.lsp.logger.Warn(fmt.Sprintf("could not find image for %q", f.uri)) return false } - f.lsp.logger.Debug(fmt.Sprintf("running lint for %q in %v", f.uri, module.FullName())) + f.lsp.logger.Debug(fmt.Sprintf("running lint for %q in %v", f.uri, f.module.FullName())) + return f.appendLintErrors("buf lint", f.lsp.checkClient.Lint( + ctx, + f.workspace.GetLintConfigForOpaqueID(f.module.OpaqueID()), + f.image, + bufcheck.WithPluginConfigs(f.workspace.PluginConfigs()...), + )) +} - lintConfig := workspace.GetLintConfigForOpaqueID(module.OpaqueID()) - err := f.lsp.checkClient.Lint( +// RunBreaking runs breaking lints on this file. Returns whether any lints failed. +// +// This operation requires BuildImage(). +func (f *file) RunBreaking(ctx context.Context) bool { + if f.IsWKT() { + // Well-known types are not linted. + return false + } + + if f.module == nil || f.image == nil || f.againstImage == nil { + f.lsp.logger.Warn(fmt.Sprintf("could not find --against image for %q", f.uri)) + return false + } + + f.lsp.logger.Debug(fmt.Sprintf("running breaking for %q in %v", f.uri, f.module.FullName())) + return f.appendLintErrors("buf breaking", f.lsp.checkClient.Breaking( ctx, - lintConfig, - image, - bufcheck.WithPluginConfigs(workspace.PluginConfigs()...), - ) + f.workspace.GetBreakingConfigForOpaqueID(f.module.OpaqueID()), + f.image, + f.againstImage, + bufcheck.WithPluginConfigs(f.workspace.PluginConfigs()...), + )) +} +func (f *file) appendLintErrors(source string, err error) bool { if err == nil { - f.lsp.logger.Warn(fmt.Sprintf("lint generated no errors for %s", f.uri)) + f.lsp.logger.Debug(fmt.Sprintf("%s generated no errors for %s", source, f.uri)) return false } var annotations bufanalysis.FileAnnotationSet if !errors.As(err, &annotations) { - f.lsp.logger.Warn("error while linting", slog.String("uri", string(f.uri)), slogext.ErrorAttr(err)) + f.lsp.logger.Warn( + "error while linting", + slog.String("uri", string(f.uri)), + slogext.ErrorAttr(err), + ) return false } - f.lsp.logger.Warn(fmt.Sprintf("lint generated %d error(s) for %s", len(annotations.FileAnnotations()), f.uri)) - for _, annotation := range annotations.FileAnnotations() { - f.lsp.logger.Info(annotation.FileInfo().Path(), " ", annotation.FileInfo().ExternalPath()) - f.diagnostics = append(f.diagnostics, protocol.Diagnostic{ Range: protocol.Range{ Start: protocol.Position{ @@ -614,10 +705,11 @@ func (f *file) RunLints(ctx context.Context) bool { }, Code: annotation.Type(), Severity: protocol.DiagnosticSeverityError, - Source: serverName, + Source: source, Message: annotation.Message(), }) } + return true } diff --git a/private/buf/buflsp/image.go b/private/buf/buflsp/image.go new file mode 100644 index 0000000000..0e4bc7df5b --- /dev/null +++ b/private/buf/buflsp/image.go @@ -0,0 +1,141 @@ +// Copyright 2020-2024 Buf Technologies, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package buflsp + +import ( + "context" + "fmt" + "io" + "log/slog" + + "github.com/bufbuild/buf/private/bufpkg/bufimage" + "github.com/bufbuild/buf/private/pkg/slogext" + "github.com/bufbuild/protocompile" + "github.com/bufbuild/protocompile/linker" + "github.com/bufbuild/protocompile/protoutil" + "github.com/google/uuid" + "go.lsp.dev/protocol" + "google.golang.org/protobuf/reflect/protoreflect" +) + +// fileOpener is a function that opens files as they are named in the import +// statements of .proto files. +// +// This is the context given to [buildImage] to control what text to look up for +// specific files, so that we can e.g. use file contents that are still unsaved +// in the editor, or use files from a different commit for building an --against +// image. +type fileOpener func(string) (io.ReadCloser, error) + +// buildImage builds a Buf Image for the given path. This does not use the controller to build +// the image, because we need delicate control over the input files: namely, for the case +// when we depend on a file that has been opened and modified in the editor. +func buildImage( + ctx context.Context, + path string, + logger *slog.Logger, + opener fileOpener, +) (bufimage.Image, []protocol.Diagnostic) { + var report report + var symbols linker.Symbols + compiler := protocompile.Compiler{ + SourceInfoMode: protocompile.SourceInfoExtraOptionLocations, + Resolver: &protocompile.SourceResolver{Accessor: opener}, + Symbols: &symbols, + Reporter: &report, + } + + compiled, err := compiler.Compile(ctx, path) + if err != nil { + logger.Warn("error building image", slog.String("path", path), slogext.ErrorAttr(err)) + } + if compiled[0] == nil { + return nil, report.diagnostics + } + + var imageFiles []bufimage.ImageFile + seen := map[string]bool{} + + queue := []protoreflect.FileDescriptor{compiled[0]} + for len(queue) > 0 { + descriptor := queue[len(queue)-1] + queue = queue[:len(queue)-1] + + if seen[descriptor.Path()] { + continue + } + seen[descriptor.Path()] = true + + unused, ok := report.pathToUnusedImports[descriptor.Path()] + var unusedIndices []int32 + if ok { + unusedIndices = make([]int32, 0, len(unused)) + } + + imports := descriptor.Imports() + for i := 0; i < imports.Len(); i++ { + dep := imports.Get(i).FileDescriptor + if dep == nil { + logger.Warn(fmt.Sprintf("found nil FileDescriptor for import %s", imports.Get(i).Path())) + continue + } + + queue = append(queue, dep) + + if unused != nil { + if _, ok := unused[dep.Path()]; ok { + unusedIndices = append(unusedIndices, int32(i)) + } + } + } + + descriptorProto := protoutil.ProtoFromFileDescriptor(descriptor) + if descriptorProto == nil { + err = fmt.Errorf("protoutil.ProtoFromFileDescriptor() returned nil for %q", descriptor.Path()) + break + } + + var imageFile bufimage.ImageFile + imageFile, err = bufimage.NewImageFile( + descriptorProto, + nil, + uuid.UUID{}, + "", + descriptor.Path(), + descriptor.Path() != path, + report.syntaxMissing[descriptor.Path()], + unusedIndices, + ) + if err != nil { + break + } + + imageFiles = append(imageFiles, imageFile) + logger.Debug(fmt.Sprintf("added image file for %s", descriptor.Path())) + } + + if err != nil { + logger.Warn("could not build image", slog.String("path", path), slogext.ErrorAttr(err)) + return nil, report.diagnostics + } + + image, err := bufimage.NewImage(imageFiles) + if err != nil { + logger.Warn("could not build image", slog.String("path", path), slogext.ErrorAttr(err)) + return nil, report.diagnostics + } + + return image, report.diagnostics +} diff --git a/private/buf/buflsp/server.go b/private/buf/buflsp/server.go index c52d2f85ad..e6f79f1512 100644 --- a/private/buf/buflsp/server.go +++ b/private/buf/buflsp/server.go @@ -114,6 +114,9 @@ func (s *server) Initialize( // usually get especially huge, so this simplifies our logic without // necessarily making the LSP slow. Change: protocol.TextDocumentSyncKindFull, + Save: &protocol.SaveOptions{ + IncludeText: false, + }, }, DefinitionProvider: &protocol.DefinitionOptions{ WorkDoneProgressOptions: protocol.WorkDoneProgressOptions{WorkDoneProgress: true}, @@ -139,6 +142,15 @@ func (s *server) Initialized( ctx context.Context, params *protocol.InitializedParams, ) error { + if s.initParams.Load().Capabilities.Workspace.DidChangeConfiguration.DynamicRegistration { + // The error is logged for us by the client wrapper. + _ = s.client.RegisterCapability(ctx, &protocol.RegistrationParams{ + Registrations: []protocol.Registration{ + {Method: protocol.MethodWorkspaceDidChangeConfiguration}, + }, + }) + } + return nil } @@ -166,6 +178,24 @@ func (s *server) Exit(ctx context.Context) error { return s.lsp.conn.Close() } +// DidChangeConfiguration is sent whenever the client changes its config settings. +func (s *server) DidChangeConfiguration( + ctx context.Context, + params *protocol.DidChangeConfigurationParams, +) error { + // We need to refresh every open file's settings, and refresh the file + // itself. + s.fileManager.uriToFile.Range(func(_ protocol.URI, file *file) bool { + if file.IsOpenInEditor() { + file.RefreshSettings(ctx) + file.Refresh(ctx) + } + return true + }) + + return nil +} + // -- File synchronization methods. // DidOpen is called whenever the client opens a document. This is our signal to parse @@ -175,8 +205,9 @@ func (s *server) DidOpen( params *protocol.DidOpenTextDocumentParams, ) error { file := s.fileManager.Open(ctx, params.TextDocument.URI) + file.RefreshSettings(ctx) file.Update(ctx, params.TextDocument.Version, params.TextDocument.Text) - file.Refresh(context.WithoutCancel(ctx)) + file.Refresh(ctx) return nil } @@ -193,7 +224,23 @@ func (s *server) DidChange( } file.Update(ctx, params.TextDocument.Version, params.ContentChanges[0].Text) - file.Refresh(context.WithoutCancel(ctx)) + file.Refresh(ctx) + return nil +} + +// DidSave is called whenever the client saves a document. +func (s *server) DidSave( + ctx context.Context, + params *protocol.DidSaveTextDocumentParams, +) error { + // We use this as an opportunity to do a refresh; some lints, such as + // breaking-against-last-saved, rely on this. + file := s.fileManager.Get(params.TextDocument.URI) + if file == nil { + // Update for a file we don't know about? Seems bad! + return fmt.Errorf("received update for file that was not open: %q", params.TextDocument.URI) + } + file.Refresh(ctx) return nil } @@ -217,7 +264,7 @@ func (s *server) Formatting( } } if errorCount > 0 { - return nil, fmt.Errorf("cannot format file %q, %v error(s) found.", file.uri.Filename(), errorCount) + return nil, fmt.Errorf("cannot format file %q, %v error(s) found", file.uri.Filename(), errorCount) } // Currently we have no way to honor any of the parameters. diff --git a/private/pkg/git/git.go b/private/pkg/git/git.go index 339fad2bd4..ea2b4faa09 100644 --- a/private/pkg/git/git.go +++ b/private/pkg/git/git.go @@ -21,7 +21,9 @@ import ( "errors" "fmt" "log/slog" + "os" "os/exec" + "path/filepath" "regexp" "strings" @@ -46,6 +48,11 @@ var ( // ErrInvalidGitCheckout is returned from CheckDirectoryIsValidGitCheckout when the // specified directory is not a valid git checkout. ErrInvalidGitCheckout = errors.New("invalid git checkout") + + // ErrInvalidRef is returned from IsValidRef if the ref does not exist in the + // repository containing the given directory (or, if the directory is not + // a valid git checkout). + ErrInvalidRef = errors.New("invalid git ref") ) // Name is a name identifiable by git. @@ -346,6 +353,93 @@ func GetRefsForGitCommitAndRemote( return refs, nil } +// IsValidRef returns whether or not ref is a valid git ref for the git +// repository that contains dir. Returns nil if the ref is valid. +func IsValidRef( + ctx context.Context, + envContainer app.EnvContainer, + dir, ref string, +) error { + stdout := bytes.NewBuffer(nil) + stderr := bytes.NewBuffer(nil) + if err := execext.Run( + ctx, + gitCommand, + execext.WithArgs("rev-parse", "--verify", ref), + execext.WithStdout(stdout), + execext.WithStderr(stderr), + execext.WithDir(dir), + execext.WithEnv(app.Environ(envContainer)), + ); err != nil { + var exitErr *exec.ExitError + if errors.As(err, &exitErr) { + if exitErr.ExitCode() == 128 { + return fmt.Errorf("could not find ref %s in %s: %w", ref, dir, ErrInvalidRef) + } + } + return err + } + return nil +} + +// ReadFileAtRef will read the file at path rolled back to the given ref, if +// it exists at that ref. +// +// Specifically, this will take path, and find the associated .git directory +// (and thus, repository root) associated with that path, if any. It will then +// look up the commit corresponding to ref in that git directory, and within +// that commit, the blob corresponding to that path (relative to the repository +// root). +func ReadFileAtRef( + ctx context.Context, + envContainer app.EnvContainer, + path, ref string, +) ([]byte, error) { + orig := path + path, err := filepath.Abs(path) + if err != nil { + return nil, err + } + + // Find a directory with a .git directory. + dir := filepath.Dir(path) + for { + _, err := os.Stat(filepath.Join(dir, ".git")) + if err == nil { + break + } else if errors.Is(err, os.ErrNotExist) { + parent := filepath.Dir(dir) + if parent == dir { + return nil, fmt.Errorf("could not find .git directory for %s: %w", orig, ErrInvalidGitCheckout) + } + dir = parent + } else { + return nil, err + } + } + + // This gives us the name of the blob we're looking for. + rel, err := filepath.Rel(dir, path) + if err != nil { + return nil, err + } + + // Call git show to show us the file we want. + stdout := bytes.NewBuffer(nil) + stderr := bytes.NewBuffer(nil) + if err := execext.Run(ctx, gitCommand, + execext.WithArgs("--no-pager", "show", ref+":"+rel), + execext.WithStdout(stdout), + execext.WithStderr(stderr), + execext.WithDir(dir), + execext.WithEnv(app.Environ(envContainer)), + ); err != nil { + return nil, fmt.Errorf("failed to get text of file %s at ref %s: %w: %s", orig, ref, err, stderr.String()) + } + + return stdout.Bytes(), nil +} + func getAllTrimmedLinesFromBuffer(buffer *bytes.Buffer) []string { scanner := bufio.NewScanner(buffer) var lines []string diff --git a/private/pkg/refcount/refcount.go b/private/pkg/refcount/refcount.go index cdbdf3b63b..efed9b1971 100644 --- a/private/pkg/refcount/refcount.go +++ b/private/pkg/refcount/refcount.go @@ -83,6 +83,20 @@ func (m *Map[K, V]) Get(key K) *V { return &value.value } +// Range ranges over this map. Beware! While ranging, writes to the map will block. +// +// This implements [iter.Seq2] for Map[K, V]. +func (m *Map[K, V]) Range(yield func(k K, v *V) bool) { + m.lock.RLock() + defer m.lock.RUnlock() + + for k, v := range m.table { + if !yield(k, &v.value) { + break + } + } +} + // Delete deletes a key from the map. // // The key will only be evicted once [Map.Delete] has been called an equal number of times