internal/lsp/cache: fix check for excluded paths in locateTemplateFiles

The pathExcludedByFilter helper function expects path to be relative to
the workspace folder. This is a confusing API, and led to us passing
absolute paths when checking for excluded template files.

Fix it, add a regtest, and add a TODO to clean up the API.

Fixes golang/go#50431

Change-Id: Iedcf0dd9710e541c9fb8c296d9856a13ef3fbcb6
Reviewed-on: https://go-review.googlesource.com/c/tools/+/378398
Trust: Robert Findley <rfindley@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
This commit is contained in:
Robert Findley 2022-01-13 15:38:31 -05:00
parent 4e31bdea78
commit 7424a4db47
2 changed files with 46 additions and 3 deletions

View File

@ -43,6 +43,33 @@ Hello {{}} <-- missing body
})
}
func TestTemplatesObserveDirectoryFilters(t *testing.T) {
const files = `
-- go.mod --
module mod.com
go 1.12
-- a/a.tmpl --
A {{}} <-- missing body
-- b/b.tmpl --
B {{}} <-- missing body
`
WithOptions(
EditorConfig{
Settings: map[string]interface{}{
"templateExtensions": []string{"tmpl"},
},
DirectoryFilters: []string{"-b"},
},
).Run(t, files, func(t *testing.T, env *Env) {
env.Await(
OnceMet(env.DiagnosticAtRegexp("a/a.tmpl", "()A")),
NoDiagnostics("b/b.tmpl"),
)
})
}
func TestTemplatesFromLangID(t *testing.T) {
const files = `
-- go.mod --

View File

@ -357,7 +357,7 @@ func (s *snapshot) RunProcessEnvFunc(ctx context.Context, fn func(*imports.Optio
return s.view.importsState.runProcessEnvFunc(ctx, s, fn)
}
// separated out from its sole use in locateTemplatFiles for testability
// separated out from its sole use in locateTemplateFiles for testability
func fileHasExtension(path string, suffixes []string) bool {
ext := filepath.Ext(path)
if ext != "" && ext[0] == '.' {
@ -376,15 +376,25 @@ func (s *snapshot) locateTemplateFiles(ctx context.Context) {
return
}
suffixes := s.view.Options().TemplateExtensions
// The workspace root may have been expanded to a module, but we should apply
// directory filters based on the configured workspace folder.
//
// TODO(rfindley): we should be more principled about paths outside of the
// workspace folder: do we even consider them? Do we support absolute
// exclusions? Relative exclusions starting with ..?
dir := s.workspace.root.Filename()
relativeTo := s.view.folder.Filename()
searched := 0
// Change to WalkDir when we move up to 1.16
err := filepath.Walk(dir, func(path string, fi os.FileInfo, err error) error {
if err != nil {
return err
}
if fileHasExtension(path, suffixes) && !pathExcludedByFilter(path, dir, s.view.gomodcache, s.view.options) &&
!fi.IsDir() {
relpath := strings.TrimPrefix(path, relativeTo)
excluded := pathExcludedByFilter(relpath, dir, s.view.gomodcache, s.view.options)
if fileHasExtension(path, suffixes) && !excluded && !fi.IsDir() {
k := span.URIFromPath(path)
fh, err := s.GetVersionedFile(ctx, k)
if err != nil {
@ -1114,6 +1124,12 @@ func pathExcludedByFilterFunc(root, gomodcache string, opts *source.Options) fun
}
}
// pathExcludedByFilter reports whether the path (relative to the workspace
// folder) should be excluded by the configured directory filters.
//
// TODO(rfindley): passing root and gomodcache here makes it confusing whether
// path should be absolute or relative, and has already caused at least one
// bug.
func pathExcludedByFilter(path, root, gomodcache string, opts *source.Options) bool {
path = strings.TrimPrefix(filepath.ToSlash(path), "/")
gomodcache = strings.TrimPrefix(filepath.ToSlash(strings.TrimPrefix(gomodcache, root)), "/")