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 <[email protected]>
Run-TryBot: Robert Findley <[email protected]>
Reviewed-by: Ian Cottrell <[email protected]>
gopls-CI: kokoro <[email protected]>
TryBot-Result: Gopher Robot <[email protected]>

Author: findleyr

gopls/internal/regtest/template/template_test.go

@@ -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 --

internal/lsp/cache/view.go

@@ -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)), "/")