Skip to content

Commit bcc9a45

Browse files
cuematthewmvdan
cuematthew
authored and
mvdan
committed
lsp/fscache: improve removal of phantom package decls
In CL 1226050 we added code to remove phantom package declarations when formatting standalone files. An oversight which wasn't spotted until after landing meant this code is both inefficient and overly complex. Here we simplify and correct this code. Change-Id: I430a55c567e552b92f773dc8ca3407a1d63c18de Signed-off-by: Matthew Sackman <[email protected]> Reviewed-on: https://cue.gerrithub.io/c/cue-lang/cue/+/1226051 Reviewed-by: Daniel Martí <[email protected]> TryBot-Result: CUEcueckoo <[email protected]> Unity-Result: CUE porcuepine <[email protected]> Reviewed-on: https://cue.gerrithub.io/c/cue-lang/cue/+/1226687
1 parent bf16055 commit bcc9a45

File tree

4 files changed

+21
-38
lines changed

4 files changed

+21
-38
lines changed

‎cmd/cue/cmd/orphans.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -160,7 +160,7 @@ func (b *buildPlan) placeOrphans(i *build.Instance, a []*decoderInfo) error {
160160
}
161161

162162
func setPackage(f *ast.File, name string, overwrite bool) {
163-
if pkg := internal.Package(f); pkg != nil {
163+
if pkg, _ := internal.Package(f); pkg != nil {
164164
if !overwrite || pkg.Name.Name == name {
165165
return
166166
}
@@ -299,7 +299,7 @@ func placeOrphans(b *buildPlan, d *encoding.Decoder, pkg string, objs ...*ast.Fi
299299
} else {
300300
field := &ast.Field{Label: labels[0]}
301301
f.Decls = append(f.Decls, field)
302-
if pkg := internal.Package(file); pkg != nil {
302+
if pkg, _ := internal.Package(file); pkg != nil {
303303
astutil.CopyComments(field, pkg)
304304
}
305305
for _, e := range labels[1:] {

‎cue/marshal.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -149,7 +149,7 @@ func (r *Runtime) Marshal(values ...InstanceOrValue) (b []byte, err error) {
149149
}
150150

151151
if inst.PkgName != "" {
152-
if pkg := internal.Package(file); pkg == nil {
152+
if pkg, _ := internal.Package(file); pkg == nil {
153153
pkg := &ast.Package{Name: ast.NewIdent(inst.PkgName)}
154154
file.Decls = append([]ast.Decl{pkg}, file.Decls...)
155155
} else if pkg.Name.Name != inst.PkgName {

‎internal/internal.go‎

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -117,22 +117,23 @@ const (
117117
DevVersion = EvalV3 // TODO(mvdan): rename to EvalExperiment for consistency with cuecontext
118118
)
119119

120-
// Package finds the package declaration from the preamble of a file.
121-
func Package(f *ast.File) *ast.Package {
122-
for _, d := range f.Decls {
120+
// Package finds the package declaration from the preamble of a file,
121+
// returning it, and its index within the file's Decls.
122+
func Package(f *ast.File) (*ast.Package, int) {
123+
for i, d := range f.Decls {
123124
switch d := d.(type) {
124125
case *ast.CommentGroup:
125126
case *ast.Attribute:
126127
case *ast.Package:
127128
if d.Name == nil { // malformed package declaration
128-
return nil
129+
return nil, -1
129130
}
130-
return d
131+
return d, i
131132
default:
132-
return nil
133+
return nil, -1
133134
}
134135
}
135-
return nil
136+
return nil, -1
136137
}
137138

138139
// NewComment creates a new CommentGroup from the given text.
@@ -178,7 +179,7 @@ func NewComment(isDoc bool, s string) *ast.CommentGroup {
178179

179180
func FileComments(f *ast.File) (docs, rest []*ast.CommentGroup) {
180181
hasPkg := false
181-
if pkg := Package(f); pkg != nil {
182+
if pkg, _ := Package(f); pkg != nil {
182183
hasPkg = true
183184
docs = pkg.Comments()
184185
}

‎internal/lsp/fscache/fs_cache.go‎

Lines changed: 9 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -13,10 +13,10 @@ import (
1313
"time"
1414

1515
"cuelang.org/go/cue/ast"
16-
"cuelang.org/go/cue/ast/astutil"
1716
"cuelang.org/go/cue/build"
1817
"cuelang.org/go/cue/parser"
1918
"cuelang.org/go/cue/token"
19+
"cuelang.org/go/internal"
2020
"cuelang.org/go/internal/filetypes"
2121
"cuelang.org/go/internal/golangorgx/gopls/protocol"
2222
"cuelang.org/go/internal/robustio"
@@ -170,34 +170,16 @@ func IsPhantomPackage(pkgDecl *ast.Package) bool {
170170
// RemovePhantomPackageDecl removes any phantom package declaration
171171
// from the provided AST.
172172
func RemovePhantomPackageDecl(file *ast.File) ast.Node {
173-
topLevelDecls := make(map[ast.Node]struct{})
174-
for _, decl := range file.Preamble() {
175-
topLevelDecls[decl] = struct{}{}
173+
pkgDecl, i := internal.Package(file)
174+
if pkgDecl == nil || !IsPhantomPackage(pkgDecl) {
175+
return file
176176
}
177177

178-
// TODO: this is still wasteful: although we ensure we do not go
179-
// "deep" down the tree, we nevertheless always look at every top
180-
// level decl in the File.
181-
182-
pkgDeclSeen := false
183-
return astutil.Apply(file, func(c astutil.Cursor) bool {
184-
if pkgDeclSeen {
185-
return false
186-
}
187-
node := c.Node()
188-
pkgDecl, ok := node.(*ast.Package)
189-
if ok {
190-
pkgDeclSeen = true
191-
if IsPhantomPackage(pkgDecl) {
192-
c.Delete()
193-
}
194-
return false
195-
}
196-
// package decls must appear at the top level, so do not go any
197-
// deeper.
198-
_, isTopLevel := topLevelDecls[node]
199-
return !isTopLevel
200-
}, nil)
178+
decls := slices.Clone(file.Decls)
179+
decls = slices.Delete(decls, i, i+1)
180+
fileCopy := *file
181+
fileCopy.Decls = decls
182+
return &fileCopy
201183
}
202184

203185
// Version implements [FileHandle]

0 commit comments

Comments
 (0)