Skip to content

Commit 819bbec

Browse files
charlieegan3charlieegan3
authored
lsp: Fix missing initial root (#1692)
lsp: Fix missing initial root Signed-off-by: Charlie Egan <charlie_egan@apple.com>
1 parent e149c6d commit 819bbec

3 files changed

Lines changed: 190 additions & 74 deletions

File tree

‎internal/lsp/server.go‎

Lines changed: 104 additions & 73 deletions
Original file line numberDiff line numberDiff line change
@@ -218,7 +218,7 @@ func (l *LanguageServer) Handle(ctx context.Context, _ *jsonrpc2.Conn, req *json
218218
case "textDocument/diagnostic":
219219
return l.handleTextDocumentDiagnostic()
220220
case "textDocument/didOpen":
221-
return handler.WithParams(req, l.handleTextDocumentDidOpen)
221+
return handler.WithContextAndParams(ctx, req, l.handleTextDocumentDidOpen)
222222
case "textDocument/didClose":
223223
return handler.WithParams(req, l.handleTextDocumentDidClose)
224224
case "textDocument/didSave":
@@ -875,19 +875,21 @@ func (l *LanguageServer) loadConfig(ctx context.Context, conf config.Config) {
875875
}
876876

877877
// Rego versions may have changed, so reload them.
878-
allRegoVersions, err := config.AllRegoVersions(l.workspacePath(), &conf)
879-
if err != nil {
880-
l.log.Debug("failed to reload rego versions: %s", err)
881-
} else {
882-
l.loadedConfigAllRegoVersions.Clear()
878+
if l.workspacePath() != "" {
879+
allRegoVersions, err := config.AllRegoVersions(l.workspacePath(), &conf)
880+
if err != nil {
881+
l.log.Debug("failed to reload rego versions: %s", err)
882+
} else {
883+
l.loadedConfigAllRegoVersions.Clear()
883884

884-
for k, v := range allRegoVersions {
885-
l.loadedConfigAllRegoVersions.Set(k, v)
885+
for k, v := range allRegoVersions {
886+
l.loadedConfigAllRegoVersions.Set(k, v)
887+
}
886888
}
887889
}
888890

889891
// Enabled rules might have changed with the new config, so reload.
890-
if err = l.loadEnabledRulesFromConfig(ctx, conf); err != nil {
892+
if err := l.loadEnabledRulesFromConfig(ctx, conf); err != nil {
891893
l.log.Message("failed to cache enabled rules: %s", err)
892894
}
893895

@@ -1528,7 +1530,21 @@ func (l *LanguageServer) handleTextDocumentDefinition(params types.DefinitionPar
15281530
}, nil
15291531
}
15301532

1531-
func (l *LanguageServer) handleTextDocumentDidOpen(params types.DidOpenTextDocumentParams) (any, error) {
1533+
func (l *LanguageServer) handleTextDocumentDidOpen(
1534+
ctx context.Context,
1535+
params types.DidOpenTextDocumentParams,
1536+
) (any, error) {
1537+
// then we have started the server, and not yet received a suitable root to use.
1538+
if l.workspaceRootURI == "" {
1539+
err := l.updateRootURI(ctx,
1540+
// get the URI of the file's immediate parent
1541+
l.fromPath(filepath.Dir(l.toPath(params.TextDocument.URI))),
1542+
)
1543+
if err != nil {
1544+
l.log.Message("failed to update server root URI: %w", err)
1545+
}
1546+
}
1547+
15321548
// if the opened file is ignored, we only store the contents for file level operations like formatting
15331549
if l.ignoreURI(params.TextDocument.URI) {
15341550
l.cache.SetIgnoredFileContents(params.TextDocument.URI, params.TextDocument.Text)
@@ -1905,40 +1921,6 @@ func (l *LanguageServer) handleInitialize(ctx context.Context, params types.Init
19051921
Capabilities: params.Capabilities,
19061922
}
19071923

1908-
// params.RootURI not expected to have a trailing slash, remove if present for consistency
1909-
rootURI := strings.TrimSuffix(params.RootURI, string(os.PathSeparator))
1910-
if rootURI == "" {
1911-
return nil, errors.New("rootURI was not set by the client but is required")
1912-
}
1913-
1914-
workspaceRootPath := uri.ToPath(l.client.Identifier, rootURI)
1915-
1916-
configRoots, err := lsconfig.FindConfigRoots(workspaceRootPath)
1917-
if err != nil {
1918-
return nil, fmt.Errorf("failed to find config roots: %w", err)
1919-
}
1920-
1921-
l.workspaceRootURI = rootURI
1922-
1923-
switch {
1924-
case len(configRoots) > 1:
1925-
l.log.Message("warning: multiple configuration root directories found in workspace:"+
1926-
"\n%s\nusing %q as workspace root directory",
1927-
strings.Join(configRoots, "\n"), configRoots[0],
1928-
)
1929-
1930-
l.workspaceRootURI = uri.FromPath(l.client.Identifier, configRoots[0])
1931-
case len(configRoots) == 1:
1932-
l.log.Message("using %q as workspace root directory", configRoots[0])
1933-
1934-
l.workspaceRootURI = uri.FromPath(l.client.Identifier, configRoots[0])
1935-
default:
1936-
l.log.Message(
1937-
"using workspace root directory: %q, custom config not found — may be inherited from parent directory",
1938-
workspaceRootPath,
1939-
)
1940-
}
1941-
19421924
if l.client.Identifier == clients.IdentifierGeneric {
19431925
l.log.Message(
19441926
"unable to match client identifier for initializing client, using generic functionality: %s",
@@ -2029,45 +2011,94 @@ func (l *LanguageServer) handleInitialize(ctx context.Context, params types.Init
20292011
l.log.Message("failed to cache enabled rules: %s", err)
20302012
}
20312013

2032-
if l.workspaceRootURI != "" {
2033-
workspaceRootPath := l.workspacePath()
2034-
2035-
l.bundleCache = bundles.NewCache(workspaceRootPath, l.log)
2036-
2037-
var configFilePath string
2038-
if configFile, err := config.FindConfig(workspaceRootPath); err == nil {
2039-
configFilePath = configFile.Name()
2040-
} else if globalConfigDir := config.GlobalConfigDir(false); globalConfigDir != "" {
2041-
// the file might not exist and we only want to log we're using the global file if it does.
2042-
if globalConfigFile := filepath.Join(globalConfigDir, "config.yaml"); rio.IsFile(globalConfigFile) {
2043-
configFilePath = globalConfigFile
2044-
}
2014+
if params.RootURI != "" {
2015+
err := l.updateRootURI(ctx, params.RootURI)
2016+
if err != nil {
2017+
l.log.Message("failed to set rootURI: %w", err)
20452018
}
2046-
2047-
if configFilePath != "" {
2048-
l.log.Message("using config file: %s", configFilePath)
2049-
l.configWatcher.Watch(configFilePath)
2050-
} else {
2051-
l.log.Message("no config file found for workspace")
2019+
} else if params.WorkspaceFolders != nil && len(*params.WorkspaceFolders) != 0 {
2020+
// note, using workspace folders is untested, and is based on the spec alone.
2021+
if len(*params.WorkspaceFolders) > 1 {
2022+
l.log.Message("cannot operate with more than one workspace folder, using: %s", (*params.WorkspaceFolders)[0].URI)
20522023
}
20532024

2054-
_, failed, err := l.loadWorkspaceContents(ctx, false)
2055-
for _, f := range failed {
2056-
l.log.Message("failed to load file %s: %s", f.URI, f.Error)
2025+
err := l.updateRootURI(ctx, (*params.WorkspaceFolders)[0].URI)
2026+
if err != nil {
2027+
l.log.Message("failed to set rootURI to workspace folder: %w", err)
20572028
}
2029+
}
20582030

2059-
if err != nil {
2060-
l.log.Message("failed to load workspace contents: %s", err)
2031+
return initializeResult, nil
2032+
}
2033+
2034+
func (l *LanguageServer) updateRootURI(ctx context.Context, rootURI string) error {
2035+
// rootURI not expected to have a trailing slash, remove if present for
2036+
// consistency
2037+
normalizedRootURI := strings.TrimSuffix(rootURI, string(os.PathSeparator))
2038+
2039+
configRoots, err := lsconfig.FindConfigRoots(l.toPath(normalizedRootURI))
2040+
if err != nil {
2041+
return fmt.Errorf("failed to find config roots: %w", err)
2042+
}
2043+
2044+
switch {
2045+
case len(configRoots) > 1:
2046+
l.log.Message("warning: multiple configuration root directories found in workspace:"+
2047+
"\n%s\nusing %q as workspace root directory",
2048+
strings.Join(configRoots, "\n"), configRoots[0],
2049+
)
2050+
2051+
l.workspaceRootURI = uri.FromPath(l.client.Identifier, configRoots[0])
2052+
case len(configRoots) == 1:
2053+
l.log.Message("using %q as workspace root directory", configRoots[0])
2054+
2055+
l.workspaceRootURI = uri.FromPath(l.client.Identifier, configRoots[0])
2056+
default:
2057+
l.workspaceRootURI = rootURI
2058+
2059+
l.log.Message(
2060+
"using workspace root directory: %q, custom config not found — may be inherited from parent directory",
2061+
rootURI,
2062+
)
2063+
}
2064+
2065+
workspaceRootPath := l.workspacePath()
2066+
2067+
l.bundleCache = bundles.NewCache(workspaceRootPath, l.log)
2068+
2069+
var configFilePath string
2070+
if configFile, err := config.FindConfig(workspaceRootPath); err == nil {
2071+
configFilePath = configFile.Name()
2072+
} else if globalConfigDir := config.GlobalConfigDir(false); globalConfigDir != "" {
2073+
// the file might not exist and we only want to log we're using the global file if it does.
2074+
if globalConfigFile := filepath.Join(globalConfigDir, "config.yaml"); rio.IsFile(globalConfigFile) {
2075+
configFilePath = globalConfigFile
20612076
}
2077+
}
20622078

2063-
l.webServer.SetWorkspaceURI(l.workspaceRootURI)
2079+
if configFilePath != "" {
2080+
l.log.Message("using config file: %s", configFilePath)
2081+
l.configWatcher.Watch(configFilePath)
2082+
} else {
2083+
l.log.Message("no config file found for workspace")
2084+
}
20642085

2065-
// 'OverwriteAggregates' is set to populate the cache's initial aggregate state.
2066-
// Subsequent runs of lintWorkspaceJobs will not set this and use the cached state.
2067-
l.lintWorkspaceJobs <- lintWorkspaceJob{Reason: "server initialize", OverwriteAggregates: true}
2086+
_, failed, err := l.loadWorkspaceContents(ctx, false)
2087+
for _, f := range failed {
2088+
l.log.Message("failed to load file %s: %s", f.URI, f.Error)
20682089
}
20692090

2070-
return initializeResult, nil
2091+
if err != nil {
2092+
l.log.Message("failed to load workspace contents: %s", err)
2093+
}
2094+
2095+
l.webServer.SetWorkspaceURI(l.workspaceRootURI)
2096+
2097+
// 'OverwriteAggregates' is set to populate the cache's initial aggregate state.
2098+
// Subsequent runs of lintWorkspaceJobs will not set this and use the cached state.
2099+
l.lintWorkspaceJobs <- lintWorkspaceJob{Reason: "server initialize", OverwriteAggregates: true}
2100+
2101+
return nil
20712102
}
20722103

20732104
type fileLoadFailure struct {

‎internal/lsp/server_no_workspace_test.go‎

Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,79 @@
1+
package lsp
2+
3+
import (
4+
"context"
5+
"path/filepath"
6+
"testing"
7+
"time"
8+
9+
"github.com/open-policy-agent/regal/internal/lsp/clients"
10+
"github.com/open-policy-agent/regal/internal/lsp/test"
11+
"github.com/open-policy-agent/regal/internal/lsp/types"
12+
"github.com/open-policy-agent/regal/internal/lsp/uri"
13+
"github.com/open-policy-agent/regal/internal/testutil"
14+
)
15+
16+
func TestLanguageServerNoWorkspace(t *testing.T) {
17+
t.Parallel()
18+
19+
mainRegoContents := `package main
20+
21+
import rego.v1
22+
allow = true
23+
`
24+
25+
files := map[string]string{
26+
"foo/main.rego": mainRegoContents,
27+
// here we are ignoring two issues with the file, so we expect not to
28+
// see these but to see another issue (opa-fmt)
29+
".regal/config.yaml": `
30+
rules:
31+
idiomatic:
32+
directory-package-mismatch:
33+
level: ignore
34+
style:
35+
use-assignment-operator:
36+
level: ignore
37+
`,
38+
}
39+
40+
// set up the workspace content with some example rego and regal config
41+
tempDir := testutil.TempDirectoryOf(t, files)
42+
mainRegoURI := uri.FromPath(clients.IdentifierGeneric, filepath.Join(tempDir, filepath.FromSlash(mainRegoFileName)))
43+
44+
receivedMessages := make(chan types.FileDiagnostics, defaultBufferedChannelSize)
45+
clientHandler := test.HandlerFor(methodTdPublishDiagnostics, test.SendsToChannel(receivedMessages))
46+
47+
// set up the server and client connections
48+
ctx, cancel := context.WithCancel(t.Context())
49+
defer cancel()
50+
51+
// note using a blank tempDir here so we can simulate the single file mode
52+
_, connClient := createAndInitServer(t, ctx, "", clientHandler)
53+
54+
// client sends textDocument/didOpen notification with contents for main.rego
55+
if err := connClient.Notify(ctx, "textDocument/didOpen", types.DidOpenTextDocumentParams{
56+
TextDocument: types.TextDocumentItem{
57+
URI: mainRegoURI,
58+
Text: mainRegoContents,
59+
},
60+
}, nil); err != nil {
61+
t.Fatalf("failed to send didOpen notification: %s", err)
62+
}
63+
64+
timeout := time.NewTimer(determineTimeout())
65+
defer timeout.Stop()
66+
67+
// validate that the client received a diagnostics notification for the file
68+
// with the correct items based on the settings.
69+
for success := false; !success; {
70+
select {
71+
case requestData := <-receivedMessages:
72+
success = testRequestDataCodes(t, requestData, mainRegoURI, []string{"opa-fmt"})
73+
case <-timeout.C:
74+
t.Fatalf("timed out waiting for file diagnostics to be sent")
75+
}
76+
}
77+
78+
timeout.Reset(determineTimeout())
79+
}

‎internal/lsp/server_test.go‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -94,8 +94,14 @@ func createAndInitServer(
9494

9595
ls.SetConn(connServer)
9696

97+
// a blank tempDir means no workspace root was required.
98+
rootURI := ""
99+
if tempDir != "" {
100+
rootURI = uri.FromPath(clients.IdentifierGeneric, tempDir)
101+
}
102+
97103
request := types.InitializeParams{
98-
RootURI: uri.FromPath(clients.IdentifierGeneric, tempDir),
104+
RootURI: rootURI,
99105
ClientInfo: types.ClientInfo{Name: "go test"},
100106
}
101107

0 commit comments

Comments
 (0)