[fidl][lsp] Notify user on failure to import fidl_project.json
Currently, when the server fails to find or parse fidl_project.json, it
crashes, which is not helpful for the user. This instead sends a dialog
message to the user suggesting to regenerate fidl_project.json or to
point the editor to their custom file.
This CL also prepares the extension for a new release, v0.3.1.
Test: go test -p 1 ./...
Change-Id: Ib0b8e5a93ca304a2780296d1aebf417a3d56d88e
Reviewed-on: https://fuchsia-review.googlesource.com/c/fidl-misc/+/401993
Reviewed-by: Benjamin Prosnitz <bprosnitz@google.com>
diff --git a/fidl-lsp/analysis/analyzer.go b/fidl-lsp/analysis/analyzer.go
index 1d8729e..6c0397d 100644
--- a/fidl-lsp/analysis/analyzer.go
+++ b/fidl-lsp/analysis/analyzer.go
@@ -71,20 +71,24 @@
// NewAnalyzer returns an Analyzer initialized with the set of CompiledLibraries
// passed in.
-func NewAnalyzer(cfg Config, compiledLibraries CompiledLibraries) *Analyzer {
+func NewAnalyzer(cfg Config) *Analyzer {
a := &Analyzer{
cfg: cfg,
libs: []*Library{},
inputFilesFIDL: make(map[state.FileID]string),
inputFilesJSON: make(map[fidlcommon.LibraryName]string),
}
- a.ImportCompiledLibraries(compiledLibraries)
return a
}
// ImportCompiledLibraries adds the info for the compiled FIDL libraries in
// `compiledLibraries` to the Analyzer's set of libraries.
func (a *Analyzer) ImportCompiledLibraries(compiledLibraries CompiledLibraries) {
+ // TODO(fxbug.dev/55060): currently this leaves the set of cached libraries
+ // in a.libs. There will more than likely be conflicts with the new set of
+ // compiled libraries, so maybe this should clear a.libs before importing a
+ // new set.
+
for _, compiledLib := range compiledLibraries {
name, err := fidlcommon.ReadLibraryName(compiledLib.Name)
if err != nil {
diff --git a/fidl-lsp/analysis/definition_test.go b/fidl-lsp/analysis/definition_test.go
index b42a652..054cde0 100644
--- a/fidl-lsp/analysis/definition_test.go
+++ b/fidl-lsp/analysis/definition_test.go
@@ -30,7 +30,7 @@
}
func TestDefinitionOfLibrary(t *testing.T) {
- analyzer := analysis.NewAnalyzer(defaultConfig(), analysis.CompiledLibraries{})
+ analyzer := analysis.NewAnalyzer(defaultConfig())
defer analyzer.Cleanup()
fs := state.NewFileSystem()
fs.NewFile(
@@ -71,7 +71,7 @@
}
func TestDefinitionOfLibraryMultipleFiles(t *testing.T) {
- analyzer := analysis.NewAnalyzer(defaultConfig(), analysis.CompiledLibraries{})
+ analyzer := analysis.NewAnalyzer(defaultConfig())
defer analyzer.Cleanup()
fs := state.NewFileSystem()
fs.NewFile(
@@ -137,7 +137,7 @@
}
func TestDefinitionOfLibraryImport(t *testing.T) {
- analyzer := analysis.NewAnalyzer(defaultConfig(), analysis.CompiledLibraries{})
+ analyzer := analysis.NewAnalyzer(defaultConfig())
defer analyzer.Cleanup()
fs := state.NewFileSystem()
fs.NewFile(
@@ -194,7 +194,7 @@
}
func TestDefinitionOfFullyQualifiedSymbol(t *testing.T) {
- analyzer := analysis.NewAnalyzer(defaultConfig(), analysis.CompiledLibraries{})
+ analyzer := analysis.NewAnalyzer(defaultConfig())
defer analyzer.Cleanup()
fs := state.NewFileSystem()
fs.NewFile(
@@ -253,7 +253,7 @@
}
func TestDefinitionOfLocalSymbol(t *testing.T) {
- analyzer := analysis.NewAnalyzer(defaultConfig(), analysis.CompiledLibraries{})
+ analyzer := analysis.NewAnalyzer(defaultConfig())
defer analyzer.Cleanup()
fs := state.NewFileSystem()
fs.NewFile(
@@ -302,7 +302,7 @@
}
func TestDefinitionOfSymbolInSameLibraryDifferentFile(t *testing.T) {
- analyzer := analysis.NewAnalyzer(defaultConfig(), analysis.CompiledLibraries{})
+ analyzer := analysis.NewAnalyzer(defaultConfig())
defer analyzer.Cleanup()
fs := state.NewFileSystem()
fs.NewFile(
@@ -360,8 +360,9 @@
}
func TestTwoLibrariesWithSameName(t *testing.T) {
- analyzer := analysis.NewAnalyzer(
- defaultConfig(),
+ analyzer := analysis.NewAnalyzer(defaultConfig())
+ defer analyzer.Cleanup()
+ analyzer.ImportCompiledLibraries(
// Two distinct libraries with the same name
analysis.CompiledLibraries{
{
@@ -374,7 +375,6 @@
},
},
)
- defer analyzer.Cleanup()
fs := state.NewFileSystem()
fs.NewFile(
@@ -436,7 +436,7 @@
}
func TestDefinitionOfReservedKeyword(t *testing.T) {
- analyzer := analysis.NewAnalyzer(defaultConfig(), analysis.CompiledLibraries{})
+ analyzer := analysis.NewAnalyzer(defaultConfig())
defer analyzer.Cleanup()
fs := state.NewFileSystem()
fs.NewFile(
diff --git a/fidl-lsp/analysis/deps_test.go b/fidl-lsp/analysis/deps_test.go
index 000e9b0..b27140b 100644
--- a/fidl-lsp/analysis/deps_test.go
+++ b/fidl-lsp/analysis/deps_test.go
@@ -21,7 +21,7 @@
}
func TestFindDepsNoDeps(t *testing.T) {
- analyzer := analysis.NewAnalyzer(analysis.Config{}, analysis.CompiledLibraries{})
+ analyzer := analysis.NewAnalyzer(analysis.Config{})
defer analyzer.Cleanup()
fs := state.NewFileSystem()
fs.NewFile(
@@ -40,8 +40,9 @@
}
func TestFindDepsDirectImports(t *testing.T) {
- analyzer := analysis.NewAnalyzer(
- analysis.Config{},
+ analyzer := analysis.NewAnalyzer(analysis.Config{})
+ defer analyzer.Cleanup()
+ analyzer.ImportCompiledLibraries(
analysis.CompiledLibraries{
{
Name: "fuchsia.import1",
@@ -53,7 +54,6 @@
},
},
)
- defer analyzer.Cleanup()
fs := state.NewFileSystem()
fs.NewFile(
@@ -89,8 +89,9 @@
}
func TestFindDepsIndirectImports(t *testing.T) {
- analyzer := analysis.NewAnalyzer(
- analysis.Config{},
+ analyzer := analysis.NewAnalyzer(analysis.Config{})
+ defer analyzer.Cleanup()
+ analyzer.ImportCompiledLibraries(
analysis.CompiledLibraries{
{
Name: "fuchsia.import1",
@@ -112,7 +113,6 @@
},
},
)
- defer analyzer.Cleanup()
fs := state.NewFileSystem()
fs.NewFile(
@@ -152,8 +152,9 @@
}
func TestFindDepsMultiFileLibraries(t *testing.T) {
- analyzer := analysis.NewAnalyzer(
- analysis.Config{},
+ analyzer := analysis.NewAnalyzer(analysis.Config{})
+ defer analyzer.Cleanup()
+ analyzer.ImportCompiledLibraries(
analysis.CompiledLibraries{
{
Name: "fuchsia.import1",
@@ -175,7 +176,6 @@
},
},
)
- defer analyzer.Cleanup()
fs := state.NewFileSystem()
fs.NewFile(
@@ -219,7 +219,7 @@
}
func TestFindDepsNoLibraryInFile(t *testing.T) {
- analyzer := analysis.NewAnalyzer(analysis.Config{}, analysis.CompiledLibraries{})
+ analyzer := analysis.NewAnalyzer(analysis.Config{})
defer analyzer.Cleanup()
fs := state.NewFileSystem()
fs.NewFile(
@@ -234,7 +234,7 @@
}
func TestFindDepsImportNotInFidlProject(t *testing.T) {
- analyzer := analysis.NewAnalyzer(analysis.Config{}, analysis.CompiledLibraries{})
+ analyzer := analysis.NewAnalyzer(analysis.Config{})
defer analyzer.Cleanup()
fs := state.NewFileSystem()
fs.NewFile(
@@ -252,8 +252,9 @@
}
func TestFindDepsImportCycle(t *testing.T) {
- analyzer := analysis.NewAnalyzer(
- analysis.Config{},
+ analyzer := analysis.NewAnalyzer(analysis.Config{})
+ defer analyzer.Cleanup()
+ analyzer.ImportCompiledLibraries(
analysis.CompiledLibraries{
{
Name: "fuchsia.foo",
@@ -267,7 +268,6 @@
},
},
)
- defer analyzer.Cleanup()
fs := state.NewFileSystem()
fs.NewFile(
diff --git a/fidl-lsp/analysis/references_test.go b/fidl-lsp/analysis/references_test.go
index 5e021b4..f9639d0 100644
--- a/fidl-lsp/analysis/references_test.go
+++ b/fidl-lsp/analysis/references_test.go
@@ -318,7 +318,7 @@
}
for _, ex := range cases {
- analyzer := analysis.NewAnalyzer(defaultConfig(), analysis.CompiledLibraries{})
+ analyzer := analysis.NewAnalyzer(defaultConfig())
defer analyzer.Cleanup()
fs := state.NewFileSystem()
for _, file := range ex.files {
diff --git a/fidl-lsp/langserver/config.go b/fidl-lsp/langserver/config.go
index 56662fd..9dec18c 100644
--- a/fidl-lsp/langserver/config.go
+++ b/fidl-lsp/langserver/config.go
@@ -6,6 +6,7 @@
import (
"context"
+ "fmt"
"path/filepath"
"github.com/sourcegraph/go-lsp"
@@ -19,7 +20,7 @@
// defaults.
type clientConfig struct {
FidlProject string `json:"fidlProject,omitempty"`
- BuildRoot string `json:"buildRoot,omitempty"`
+ BuildRoot string `json:"projectRoot,omitempty"`
Tools toolsConfig `json:"tools,omitempty"`
}
@@ -44,11 +45,11 @@
h.log.Printf("received invalid configuration settings. unable to update\n")
return
}
- h.updateConfig(cfg[0])
+ h.updateConfig(ctx, conn, cfg[0])
}()
}
-func (h *LangHandler) handleDidChangeConfiguration(params lsp.DidChangeConfigurationParams) {
+func (h *LangHandler) handleDidChangeConfiguration(ctx context.Context, conn *jsonrpc2.Conn, params lsp.DidChangeConfigurationParams) {
h.log.Printf("updating with new config settings: %#v", params.Settings)
cfg, ok := params.Settings.([]clientConfig)
@@ -60,17 +61,12 @@
h.log.Printf("received invalid configuration settings. unable to update\n")
return
}
- h.updateConfig(cfg[0])
+ h.updateConfig(ctx, conn, cfg[0])
}
-func (h *LangHandler) updateConfig(cfg clientConfig) {
+func (h *LangHandler) updateConfig(ctx context.Context, conn *jsonrpc2.Conn, cfg clientConfig) {
if cfg.FidlProject != "" {
- fidlProject, err := analysis.LoadFidlProject(cfg.FidlProject)
- if err != nil {
- h.log.Printf("failed to parse fidl_project.json at `%s`: %s\n", cfg.FidlProject, err)
- }
- // TODO: remove the currently compiled libraries?
- h.analyzer.ImportCompiledLibraries(fidlProject)
+ h.LoadFidlProject(ctx, conn, cfg.FidlProject)
// If the BuildRoot is unset, set it to the directory containing
// `FidlProject`
@@ -86,3 +82,24 @@
FidlFormatPath: cfg.Tools.Formatter,
})
}
+
+// LoadFidlProject attempts to read in the fidl_project.json file at
+// `fidlProjectPath`, and if it can't, it displays an error dialog in the client
+// so the user can try to regenerate the file.
+func (h *LangHandler) LoadFidlProject(ctx context.Context, conn *jsonrpc2.Conn, fidlProjectPath string) {
+ fidlProject, err := analysis.LoadFidlProject(fidlProjectPath)
+ if err != nil {
+ h.log.Printf("failed to parse fidl_project.json at `%s`: %s\n", fidlProjectPath, err)
+ h.Notify(ctx, conn, "window/showMessage", lsp.ShowMessageParams{
+ Type: lsp.MTError,
+ Message: fmt.Sprintf(
+ "Failed to load fidl_project.json at '%s'.\n"+
+ "To generate this file, run "+
+ "`fx gen && fx build build/fidl:validate_fidl_project_json`. "+
+ "Alternatively, custom path can be specified in settings.",
+ fidlProjectPath,
+ ),
+ })
+ }
+ h.analyzer.ImportCompiledLibraries(fidlProject)
+}
diff --git a/fidl-lsp/langserver/handler.go b/fidl-lsp/langserver/handler.go
index a3e5086..d71d58f 100644
--- a/fidl-lsp/langserver/handler.go
+++ b/fidl-lsp/langserver/handler.go
@@ -195,7 +195,7 @@
return nil, err
}
- h.handleDidChangeConfiguration(params)
+ h.handleDidChangeConfiguration(ctx, conn, params)
return nil, nil
case "textDocument/didOpen":
diff --git a/fidl-lsp/langserver/testutils.go b/fidl-lsp/langserver/testutils.go
index 43ce4c5..843746e 100644
--- a/fidl-lsp/langserver/testutils.go
+++ b/fidl-lsp/langserver/testutils.go
@@ -36,7 +36,6 @@
FidlLintPath: fidlLintPath,
FidlFormatPath: fidlFormatPath,
},
- analysis.CompiledLibraries{},
),
)
diff --git a/fidl-lsp/main.go b/fidl-lsp/main.go
index ef44688..393367e 100644
--- a/fidl-lsp/main.go
+++ b/fidl-lsp/main.go
@@ -49,16 +49,6 @@
trace := log.New(os.Stderr, "[LSP] ", log.Lshortfile)
logOpt := jsonrpc2.LogMessages(trace)
- fidlProject, err := analysis.LoadFidlProject(fidlProjectPath)
- if err != nil {
- trace.Printf(
- "Failed to parse fidl_project.json at `%s`: %s\n"+
- "To regenerate, run 'fx build build/fidl:validate_fidl_project_json'.\n",
- fidlProjectPath,
- err,
- )
- }
-
// Assume that `fidlc` and `fidl-format` are distributed in the same
// platform-specific directory as this server binary, so we can find them in
// this directory.
@@ -75,7 +65,6 @@
FidlFormatPath: fmt.Sprintf("%s/fidl-format", cwd),
FidlLintPath: fidlLintPath,
},
- fidlProject,
)
handler := langserver.NewLangHandler(langserver.NewDefaultConfig(), trace, analyzer)
@@ -89,6 +78,7 @@
server,
logOpt,
)
+ handler.LoadFidlProject(ctx, conn, fidlProjectPath)
defer conn.Close()
<-conn.DisconnectNotify()
diff --git a/vscode-language-fidl/CHANGELOG.md b/vscode-language-fidl/CHANGELOG.md
index dda7d92..1958f99 100644
--- a/vscode-language-fidl/CHANGELOG.md
+++ b/vscode-language-fidl/CHANGELOG.md
@@ -22,3 +22,12 @@
- Find all references
- Hover for type information
- Document links
+
+# v0.3.1
+
+- Bug fixes
+ - Don't crash when fail to read `fidl_project.json`
+ - Correctly lookup library on `textDocument/references` request
+- Improvements
+ - Notify the user when the server fails to read `fidl_project.json`
+ - Rename config setting for clarity
diff --git a/vscode-language-fidl/package-lock.json b/vscode-language-fidl/package-lock.json
index 5ee5360..223a7fb 100644
--- a/vscode-language-fidl/package-lock.json
+++ b/vscode-language-fidl/package-lock.json
@@ -1,6 +1,6 @@
{
"name": "language-fidl",
- "version": "0.3.0",
+ "version": "0.3.1",
"lockfileVersion": 1,
"requires": true,
"dependencies": {
diff --git a/vscode-language-fidl/package.json b/vscode-language-fidl/package.json
index 9f99f78..fc7d3f1 100644
--- a/vscode-language-fidl/package.json
+++ b/vscode-language-fidl/package.json
@@ -3,7 +3,7 @@
"displayName": "FIDL Language Support",
"description": "Support for FIDL files",
"license": "SEE LICENSE IN LICENSE",
- "version": "0.3.0",
+ "version": "0.3.1",
"publisher": "fuchsia-authors",
"engines": {
"vscode": "^1.45.1"
@@ -47,7 +47,7 @@
"description": "Specifies the absolute path to fidl_project.json, a declarative file of FIDL libraries and their build artifacts.",
"default": ""
},
- "fidl.buildRoot": {
+ "fidl.projectRoot": {
"type": "string",
"description": "Specifies the absolute path to the root directory of the FIDL project. If not set, will be assumed to be the path containing fidl_project.json.",
"default": ""
