Implement ignorelist for module indexing (#696)

* add new IgnoreDirectoryNames setting * add test for ignoreDirectoryNames * implement ignoreDirectoryNames * add comment about dirs in settings.md * validate IgnoreDirectoryNames input * implement isSkippableDir as function of the walker * make test platform agnostic * fix settings test for windows * Apply suggestions from code review Co-authored-by: Radek Simko <[email protected]> * fix test Co-authored-by: Radek Simko <[email protected]>
hashicorp · Nov 8, 2021 · fe7b087 · fe7b087
1 parent b29e7ec
commit fe7b087
Show file tree

Hide file tree

Showing 6 changed files with 142 additions and 22 deletions.
diff --git a/docs/SETTINGS.md b/docs/SETTINGS.md
@@ -68,6 +68,18 @@ Or if left empty
 This setting should be deprecated once the language server supports multiple workspaces,
 as this arises in VS code because a server instance is started per VS Code workspace.
 
+## `ignoreDirectoryNames` (`[]string`)
+
+This allows excluding directories from being indexed upon initialization by passing a list of directory names.
+
+The following list of directories will always be ignored:
+
+- `.git`
+- `.idea`
+- `.vscode`
+- `terraform.tfstate.d`
+- `.terragrunt-cache`
+
 ## `experimentalFeatures` (object)
 
 This object contains inner settings used to opt into experimental features not yet ready to be on by default.

diff --git a/internal/langserver/handlers/initialize.go b/internal/langserver/handlers/initialize.go
@@ -59,6 +59,7 @@ func (svc *service) Initialize(ctx context.Context, params lsp.InitializeParams)
 		"options.rootModulePaths":                         false,
 		"options.excludeModulePaths":                      false,
 		"options.commandPrefix":                           false,
+		"options.ignoreDirectoryNames":                    false,
 		"options.experimentalFeatures.validateOnSave":     false,
 		"options.terraformExecPath":                       false,
 		"options.terraformExecTimeout":                    "",
@@ -123,6 +124,7 @@ func (svc *service) Initialize(ctx context.Context, params lsp.InitializeParams)
 	properties["options.rootModulePaths"] = len(out.Options.ModulePaths) > 0
 	properties["options.excludeModulePaths"] = len(out.Options.ExcludeModulePaths) > 0
 	properties["options.commandPrefix"] = len(out.Options.CommandPrefix) > 0
+	properties["options.ignoreDirectoryNames"] = len(out.Options.IgnoreDirectoryNames) > 0
 	properties["options.experimentalFeatures.prefillRequiredFields"] = out.Options.ExperimentalFeatures.PrefillRequiredFields
 	properties["options.experimentalFeatures.validateOnSave"] = out.Options.ExperimentalFeatures.ValidateOnSave
 	properties["options.terraformExecPath"] = len(out.Options.TerraformExecPath) > 0
@@ -192,6 +194,7 @@ func (svc *service) Initialize(ctx context.Context, params lsp.InitializeParams)
 		excludeModulePaths = append(excludeModulePaths, modPath)
 	}
 
+	svc.walker.SetIgnoreDirectoryNames(cfgOpts.IgnoreDirectoryNames)
 	svc.walker.SetExcludeModulePaths(excludeModulePaths)
 	svc.walker.EnqueuePath(fh.Dir())
 

diff --git a/internal/langserver/handlers/initialize_test.go b/internal/langserver/handlers/initialize_test.go
@@ -2,6 +2,7 @@ package handlers
 
 import (
 	"fmt"
+	"path/filepath"
 	"testing"
 
 	"github.com/creachadair/jrpc2/code"
@@ -119,3 +120,42 @@ func TestInitialize_multipleFolders(t *testing.T) {
 	    ]
 	}`, rootDir.URI(), rootDir.URI())})
 }
+
+func TestInitialize_ignoreDirectoryNames(t *testing.T) {
+	tmpDir := TempDir(t, "plugin", "ignore")
+	pluginDir := filepath.Join(tmpDir.Dir(), "plugin")
+	emptyDir := filepath.Join(tmpDir.Dir(), "ignore")
+
+	InitPluginCache(t, pluginDir)
+	InitPluginCache(t, emptyDir)
+
+	ls := langserver.NewLangServerMock(t, NewMockSession(&MockSessionInput{
+		TerraformCalls: &exec.TerraformMockCalls{
+			PerWorkDir: map[string][]*mock.Call{
+				pluginDir: validTfMockCalls(),
+				emptyDir: {
+					// TODO! improve mock and remove entry for `emptyDir` here afterwards
+					{
+						Method:        "GetExecPath",
+						Repeatability: 1,
+						ReturnArguments: []interface{}{
+							"",
+						},
+					},
+				},
+			},
+		}}))
+	stop := ls.Start(t)
+	defer stop()
+
+	ls.Call(t, &langserver.CallRequest{
+		Method: "initialize",
+		ReqParams: fmt.Sprintf(`{
+			"capabilities": {},
+			"rootUri": %q,
+			"processId": 12345,
+			"initializationOptions": {
+				"ignoreDirectoryNames": [%q]
+			}
+	}`, tmpDir.URI(), "ignore")})
+}
diff --git a/internal/settings/settings.go b/internal/settings/settings.go
@@ -4,7 +4,9 @@ import (
 	"fmt"
 	"os"
 	"path/filepath"
+	"strings"
 
+	"github.com/hashicorp/terraform-ls/internal/terraform/datadir"
 	"github.com/mitchellh/mapstructure"
 )
 
@@ -15,9 +17,10 @@ type ExperimentalFeatures struct {
 
 type Options struct {
 	// ModulePaths describes a list of absolute paths to modules to load
-	ModulePaths        []string `mapstructure:"rootModulePaths"`
-	ExcludeModulePaths []string `mapstructure:"excludeModulePaths"`
-	CommandPrefix      string   `mapstructure:"commandPrefix"`
+	ModulePaths          []string `mapstructure:"rootModulePaths"`
+	ExcludeModulePaths   []string `mapstructure:"excludeModulePaths"`
+	CommandPrefix        string   `mapstructure:"commandPrefix"`
+	IgnoreDirectoryNames []string `mapstructure:"ignoreDirectoryNames"`
 
 	// ExperimentalFeatures encapsulates experimental features users can opt into.
 	ExperimentalFeatures ExperimentalFeatures `mapstructure:"experimentalFeatures"`
@@ -46,6 +49,18 @@ func (o *Options) Validate() error {
 		}
 	}
 
+	if len(o.IgnoreDirectoryNames) > 0 {
+		for _, directory := range o.IgnoreDirectoryNames {
+			if directory == datadir.DataDirName {
+				return fmt.Errorf("cannot ignore directory %q", datadir.DataDirName)
+			}
+
+			if strings.Contains(directory, string(filepath.Separator)) {
+				return fmt.Errorf("expected directory name, got a path: %q", directory)
+			}
+		}
+	}
+
 	return nil
 }
 

diff --git a/internal/settings/settings_test.go b/internal/settings/settings_test.go
@@ -1,9 +1,12 @@
 package settings
 
 import (
+	"fmt"
+	"path/filepath"
 	"testing"
 
 	"github.com/google/go-cmp/cmp"
+	"github.com/hashicorp/terraform-ls/internal/terraform/datadir"
 )
 
 func TestDecodeOptions_nil(t *testing.T) {
@@ -40,3 +43,40 @@ func TestDecodeOptions_success(t *testing.T) {
 		t.Fatalf("options mismatch: %s", diff)
 	}
 }
+
+func TestValidate_IgnoreDirectoryNames_error(t *testing.T) {
+	tables := []struct {
+		input  string
+		result string
+	}{
+		{datadir.DataDirName, `cannot ignore directory ".terraform"`},
+		{filepath.Join("path", "path"), fmt.Sprintf(`expected directory name, got a path: %q`, filepath.Join("path", "path"))},
+	}
+
+	for _, table := range tables {
+		out, err := DecodeOptions(map[string]interface{}{
+			"ignoreDirectoryNames": []string{table.input},
+		})
+		if err != nil {
+			t.Fatal(err)
+		}
+
+		result := out.Options.Validate()
+		if result.Error() != table.result {
+			t.Fatalf("expected error: %s, got: %s", table.result, result)
+		}
+	}
+}
+func TestValidate_IgnoreDirectoryNames_success(t *testing.T) {
+	out, err := DecodeOptions(map[string]interface{}{
+		"ignoreDirectoryNames": []string{"directory"},
+	})
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	result := out.Options.Validate()
+	if result != nil {
+		t.Fatalf("did not expect error: %s", result)
+	}
+}
diff --git a/internal/terraform/module/walker.go b/internal/terraform/module/walker.go
@@ -21,6 +21,8 @@ var (
 
 	// skipDirNames represent directory names which would never contain
 	// plugin/module cache, so it's safe to skip them during the walk
+	//
+	// please keep the list in `SETTINGS.md` in sync
 	skipDirNames = map[string]bool{
 		".git":                true,
 		".idea":               true,
@@ -48,7 +50,8 @@ type Walker struct {
 	cancelFunc context.CancelFunc
 	doneCh     <-chan struct{}
 
-	excludeModulePaths map[string]bool
+	excludeModulePaths   map[string]bool
+	ignoreDirectoryNames map[string]bool
 }
 
 // queueCap represents channel buffer size
@@ -58,14 +61,15 @@ const queueCap = 50
 
 func NewWalker(fs filesystem.Filesystem, modMgr ModuleManager) *Walker {
 	return &Walker{
-		fs:        fs,
-		modMgr:    modMgr,
-		logger:    discardLogger,
-		walkingMu: &sync.RWMutex{},
-		queue:     newWalkerQueue(fs),
-		queueMu:   &sync.Mutex{},
-		pushChan:  make(chan struct{}, queueCap),
-		doneCh:    make(chan struct{}, 0),
+		fs:                   fs,
+		modMgr:               modMgr,
+		logger:               discardLogger,
+		walkingMu:            &sync.RWMutex{},
+		queue:                newWalkerQueue(fs),
+		queueMu:              &sync.Mutex{},
+		pushChan:             make(chan struct{}, queueCap),
+		doneCh:               make(chan struct{}, 0),
+		ignoreDirectoryNames: skipDirNames,
 	}
 }
 
@@ -84,6 +88,12 @@ func (w *Walker) SetExcludeModulePaths(excludeModulePaths []string) {
 	}
 }
 
+func (w *Walker) SetIgnoreDirectoryNames(ignoreDirectoryNames []string) {
+	for _, path := range ignoreDirectoryNames {
+		w.ignoreDirectoryNames[path] = true
+	}
+}
+
 func (w *Walker) Stop() {
 	if w.cancelFunc != nil {
 		w.cancelFunc()
@@ -199,6 +209,11 @@ func (w *Walker) IsWalking() bool {
 	return w.walking
 }
 
+func (w *Walker) isSkippableDir(dirName string) bool {
+	_, ok := w.ignoreDirectoryNames[dirName]
+	return ok
+}
+
 func (w *Walker) walk(ctx context.Context, rootPath string) error {
 	// We ignore the passed FS and instead read straight from OS FS
 	// because that would require reimplementing filepath.Walk and
@@ -221,6 +236,11 @@ func (w *Walker) walk(ctx context.Context, rootPath string) error {
 			return err
 		}
 
+		if w.isSkippableDir(info.Name()) {
+			w.logger.Printf("skipping %s", path)
+			return filepath.SkipDir
+		}
+
 		if _, ok := w.excludeModulePaths[dir]; ok {
 			return filepath.SkipDir
 		}
@@ -272,18 +292,8 @@ func (w *Walker) walk(ctx context.Context, rootPath string) error {
 			return nil
 		}
 
-		if isSkippableDir(info.Name()) {
-			w.logger.Printf("skipping %s", path)
-			return filepath.SkipDir
-		}
-
 		return nil
 	})
 	w.logger.Printf("walking of %s finished", rootPath)
 	return err
 }
-
-func isSkippableDir(dirName string) bool {
-	_, ok := skipDirNames[dirName]
-	return ok
-}