mvdan.cc / cue
this repo has no description
0
fork

Configure Feed

Select the types of activity you want to include in your feed.

lsp/fscache: Force parsing mode to always be ParseComments

lsp/fscache is a bridge between the uri-world of LSP, and the fs/fs
world of modpkgload and modimports.

These caches and overlays support the reading, parsing and caching of
CUE files and their AST. When modimports is grouping files into
packages, it uses [parser.ImportsOnly] to minimise parsing time.
However, this can lead to a nasty situation where the cached AST within
fscache contains only the imports and not the rest of the file, and this
cache can not be updated.

I, and one other early adopter, have observed this can cause data loss
when the lsp formats a file, and the lsp's version of this file contains
only the imports.

For now, force the fscache code to always overwrite the parsing mode to
ParseComments. It's quite likely that we always want the full AST - we
will need it for all sorts of other analysis. The only place where this
will waste work is for files that turn out to belong to packages that we
never load - which is obviously tricky to predict. In the future, if it
does turn out that we're doing too much work we could always change the
logic so that we don't overwrite the provided parsing mode, but we also
only cache the AST if the mode is ParseComments. Or something more
complicated.

Change-Id: I93d58bb2dac980c8053eb84ff6232b4ca36edd0d
Signed-off-by: Matthew Sackman <matthew@cue.works>
Reviewed-on: https://review.gerrithub.io/c/cue-lang/cue/+/1219287
Reviewed-by: Daniel Martí <mvdan@mvdan.cc>
Unity-Result: CUE porcuepine <cue.porcuepine@gmail.com>
TryBot-Result: CUEcueckoo <cueckoo@cuelang.org>

Matthew Sackman 741955cc d8451da6

+26 -5
+6 -1
internal/lsp/fscache/fs_cache.go
··· 34 34 // only one config value is used for each file: if you change the 35 35 // config value and re-read the file, you will not receive back an 36 36 // updated [ast.File]. 37 + // 38 + // Additionally, in order to ensure the first parse of a file does 39 + // not cause the cache to contain a partial AST, ReadCUE 40 + // unconditionally sets config.Mode to [parser.ParseComments]. 37 41 ReadCUE(config parser.Config) (*ast.File, error) 38 42 // Version returns the file version, as defined by the LSP client. 39 43 Version() int32 ··· 75 79 return nil, nil 76 80 } 77 81 78 - ast, err := parser.ParseFile(bf.Filename, bf.Source, config, parser.ParseComments) 82 + config.Mode = parser.ParseComments 83 + ast, err := parser.ParseFile(bf.Filename, bf.Source, config) 79 84 if err != nil { 80 85 return nil, err 81 86 }
+2 -1
internal/lsp/fscache/fs_overlay.go
··· 59 59 return nil, nil 60 60 } 61 61 62 - ast, err := parser.ParseFile(bf.Filename, bf.Source, config, parser.ParseComments) 62 + config.Mode = parser.ParseComments 63 + ast, err := parser.ParseFile(bf.Filename, bf.Source, config) 63 64 if err != nil { 64 65 return nil, err 65 66 }
+18 -3
internal/lsp/fscache/fs_test.go
··· 10 10 "testing/fstest" 11 11 "time" 12 12 13 + "cuelang.org/go/cue/parser" 13 14 "cuelang.org/go/internal/golangorgx/gopls/protocol" 14 15 "cuelang.org/go/internal/lsp/fscache" 15 16 "github.com/go-quicktest/qt" 16 17 ) 18 + 19 + const fileContent = "package foo\n\nx: true" 17 20 18 21 func TestCUECacheFSURI(t *testing.T) { 19 22 _, _, onDiskFilesAbs := setup(t) ··· 23 26 uri := protocol.URIFromPath(f) 24 27 fh, err := fs.ReadFile(uri) 25 28 qt.Assert(t, qt.IsNil(err)) 26 - qt.Assert(t, qt.DeepEquals(fh.Content(), []byte(f))) 29 + qt.Assert(t, qt.DeepEquals(fh.Content(), []byte(fileContent))) 30 + ast, err := fh.ReadCUE(parser.NewConfig(parser.ImportsOnly)) 31 + qt.Assert(t, qt.IsNil(err)) 32 + // 2 decls: 1 for the package one for x, because the 33 + // [parser.ImportsOnly] mode is modified to 34 + // [parser.ParseComments]. 35 + qt.Assert(t, qt.Equals(len(ast.Decls), 2)) 27 36 } 28 37 } 29 38 ··· 73 82 if i == 0 { 74 83 qt.Assert(t, qt.DeepEquals(fh.Content(), content)) 75 84 } else { 76 - qt.Assert(t, qt.DeepEquals(fh.Content(), []byte(f))) 85 + qt.Assert(t, qt.DeepEquals(fh.Content(), []byte(fileContent))) 86 + ast, err := fh.ReadCUE(parser.NewConfig(parser.ImportsOnly)) 87 + qt.Assert(t, qt.IsNil(err)) 88 + // 2 decls: 1 for the package one for x, because the 89 + // [parser.ImportsOnly] mode is modified to 90 + // [parser.ParseComments]. 91 + qt.Assert(t, qt.Equals(len(ast.Decls), 2)) 77 92 } 78 93 } 79 94 } ··· 133 148 onDiskFilesAbs[i] = filepath.Join(dir, filepath.FromSlash(f)) 134 149 } 135 150 for _, f := range onDiskFilesAbs { 136 - writeFile(t, f, f) 151 + writeFile(t, f, fileContent) 137 152 } 138 153 forceMFTUpdateOnWindows(t, dir) 139 154 return dir, onDiskFiles, onDiskFilesAbs