Skip to content

Commit

Permalink
rootmodules: Surface whole RootModule candidates instead of paths
Browse files Browse the repository at this point in the history
  • Loading branch information
radeksimko committed Jul 7, 2020
1 parent aa04dcc commit 499d12d
Show file tree
Hide file tree
Showing 4 changed files with 35 additions and 76 deletions.
38 changes: 12 additions & 26 deletions internal/terraform/rootmodule/root_module_manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ func (rmm *rootModuleManager) AddRootModule(dir string) (RootModule, error) {

// TODO: Follow symlinks (requires proper test data)

if rmm.exists(dir) {
if _, ok := rmm.rootModuleByPath(dir); ok {
return nil, fmt.Errorf("root module %s was already added", dir)
}

Expand All @@ -90,46 +90,37 @@ func (rmm *rootModuleManager) AddRootModule(dir string) (RootModule, error) {
return rm, nil
}

func (rmm *rootModuleManager) exists(dir string) bool {
func (rmm *rootModuleManager) rootModuleByPath(dir string) (*rootModule, bool) {
for _, rm := range rmm.rms {
if pathEquals(rm.Path(), dir) {
return true
return rm, true
}
}
return false
return nil, false
}

func (rmm *rootModuleManager) rootModuleByPath(dir string) *rootModule {
for _, rm := range rmm.rms {
if pathEquals(rm.Path(), dir) {
return rm
}
}
return nil
}

func (rmm *rootModuleManager) RootModuleCandidatesByPath(path string) []string {
func (rmm *rootModuleManager) RootModuleCandidatesByPath(path string) RootModules {
path = filepath.Clean(path)

// TODO: Follow symlinks (requires proper test data)

if rmm.exists(path) {
if rm, ok := rmm.rootModuleByPath(path); ok {
rmm.logger.Printf("direct root module lookup succeeded: %s", path)
return []string{path}
return []RootModule{rm}
}

dir := rootModuleDirFromFilePath(path)
if rmm.exists(dir) {
if rm, ok := rmm.rootModuleByPath(dir); ok {
rmm.logger.Printf("dir-based root module lookup succeeded: %s", dir)
return []string{dir}
return []RootModule{rm}
}

candidates := make([]string, 0)
candidates := make([]RootModule, 0)
for _, rm := range rmm.rms {
rmm.logger.Printf("looking up %s in module references of %s", dir, rm.Path())
if rm.ReferencesModulePath(dir) {
rmm.logger.Printf("module-ref-based root module lookup succeeded: %s", dir)
candidates = append(candidates, rm.Path())
candidates = append(candidates, rm)
}
}

Expand All @@ -139,12 +130,7 @@ func (rmm *rootModuleManager) RootModuleCandidatesByPath(path string) []string {
func (rmm *rootModuleManager) RootModuleByPath(path string) (RootModule, error) {
candidates := rmm.RootModuleCandidatesByPath(path)
if len(candidates) > 0 {
firstMatch := candidates[0]
if !rmm.exists(firstMatch) {
return nil, fmt.Errorf("Discovered root module %s not available,"+
" this is most likely a bug, please report it", firstMatch)
}
return rmm.rootModuleByPath(firstMatch), nil
return candidates[0], nil
}

return nil, &RootModuleNotFoundErr{path}
Expand Down
42 changes: 1 addition & 41 deletions internal/terraform/rootmodule/root_module_manager_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,46 +14,6 @@ import (
"github.com/hashicorp/terraform-ls/internal/terraform/exec"
)

// func TestRootModuleManager_RootModuleByPath_basic(t *testing.T) {
// rmm := testRootModuleManager(t)

// tmpDir := tempDir(t)
// direct, unrelated, dirbased := newTestRootModule(t, "direct"), newTestRootModule(t, "unrelated"), newTestRootModule(t, "dirbased")
// rmm.rms = map[string]*rootModule{
// direct.Dir: direct.RootModule,
// unrelated.Dir: unrelated.RootModule,
// dirbased.Dir: dirbased.RootModule,
// }

// w1, err := rmm.RootModuleByPath(direct.Dir)
// if err != nil {
// t.Fatal(err)
// }
// if direct.RootModule != w1 {
// t.Fatalf("unexpected root module found: %p, expected: %p", w1, direct)
// }

// lockDirPath := filepath.Join(tmpDir, "dirbased", ".terraform", "plugins")
// lockFilePath := filepath.Join(lockDirPath, "selections.json")
// err = os.MkdirAll(lockDirPath, 0775)
// if err != nil {
// t.Fatal(err)
// }
// f, err := os.Create(lockFilePath)
// if err != nil {
// t.Fatal(err)
// }
// f.Close()

// w2, err := rmm.RootModuleByPath(lockFilePath)
// if err != nil {
// t.Fatal(err)
// }
// if dirbased.RootModule != w2 {
// t.Fatalf("unexpected root module found: %p, expected: %p", w2, dirbased)
// }
// }

func TestRootModuleManager_RootModuleCandidatesByPath(t *testing.T) {
testData, err := filepath.Abs("testdata")
if err != nil {
Expand Down Expand Up @@ -484,7 +444,7 @@ func TestRootModuleManager_RootModuleCandidatesByPath(t *testing.T) {
}

candidates := rmm.RootModuleCandidatesByPath(tc.lookupPath)
if diff := cmp.Diff(tc.expectedCandidates, candidates); diff != "" {
if diff := cmp.Diff(tc.expectedCandidates, candidates.Paths()); diff != "" {
t.Fatalf("candidates don't match: %s", diff)
}
})
Expand Down
13 changes: 12 additions & 1 deletion internal/terraform/rootmodule/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ type TerraformExecFinder interface {
}

type RootModuleCandidateFinder interface {
RootModuleCandidatesByPath(path string) []string
RootModuleCandidatesByPath(path string) RootModules
}

type RootModuleManager interface {
Expand All @@ -39,7 +39,18 @@ type RootModuleManager interface {
RootModuleByPath(path string) (RootModule, error)
}

type RootModules []RootModule

func (rms RootModules) Paths() []string {
paths := make([]string, len(rms))
for i, rm := range rms {
paths[i] = rm.Path()
}
return paths
}

type RootModule interface {
Path() string
IsKnownPluginLockFile(path string) bool
IsKnownModuleManifestFile(path string) bool
PathsToWatch() []string
Expand Down
18 changes: 10 additions & 8 deletions langserver/handlers/did_open.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"github.com/creachadair/jrpc2"
lsctx "github.com/hashicorp/terraform-ls/internal/context"
ilsp "github.com/hashicorp/terraform-ls/internal/lsp"
"github.com/hashicorp/terraform-ls/internal/terraform/rootmodule"
lsp "github.com/sourcegraph/go-lsp"
)

Expand Down Expand Up @@ -47,8 +48,8 @@ func TextDocumentDidOpen(ctx context.Context, params lsp.DidOpenTextDocumentPara
// TODO: Suggest specifying explicit root modules?

msg := fmt.Sprintf("Alternative root modules found for %s (%s), picked: %s",
f.Filename(), renderCandidates(rootDir, candidates[1:]),
renderCandidate(rootDir, candidates[0]))
f.Filename(), candidatePaths(rootDir, candidates[1:]),
renderCandidatePath(rootDir, candidates[0]))
return jrpc2.ServerPush(ctx, "window/showMessage", lsp.ShowMessageParams{
Type: lsp.MTWarning,
Message: msg,
Expand All @@ -58,17 +59,18 @@ func TextDocumentDidOpen(ctx context.Context, params lsp.DidOpenTextDocumentPara
return nil
}

func renderCandidates(rootDir string, candidatePaths []string) string {
for i, p := range candidatePaths {
func candidatePaths(rootDir string, candidates []rootmodule.RootModule) string {
paths := make([]string, len(candidates))
for i, rm := range candidates {
// This helps displaying shorter, but still relevant paths
candidatePaths[i] = renderCandidate(rootDir, p)
paths[i] = renderCandidatePath(rootDir, rm)
}
return strings.Join(candidatePaths, ", ")
return strings.Join(paths, ", ")
}

func renderCandidate(rootDir, path string) string {
func renderCandidatePath(rootDir string, candidate rootmodule.RootModule) string {
trimmed := strings.TrimPrefix(
strings.TrimPrefix(path, rootDir), string(os.PathSeparator))
strings.TrimPrefix(candidate.Path(), rootDir), string(os.PathSeparator))
if trimmed == "" {
return "."
}
Expand Down

0 comments on commit 499d12d

Please sign in to comment.