[fidl][lsp] Use new fidl_project.json format
Changes the Analyzer to use to new fidl_project.json format, which,
instead of being a map of library name --> library info, is a list of
libraries. This supports non-unique FIDL library names.
This essentially involves replacing indexes into Analyzer.libs with
calls to either Analyzer.getLibrary (which just takes a library name,
and uses a "heuristic" to find the library -- assumes library names are
unique) or, better, Analyzer.getLibraryWithFile, which takes a library
name and a file, thereby supporting lookup to libraries with non-unique
names.
This CL also fixes a flaky test, references_test.
Test: go test -p 1 ./...
Change-Id: I66e9e6be010ca7cab02ec751ecc6410ee3df20f8
Reviewed-on: https://fuchsia-review.googlesource.com/c/fidl-misc/+/399954
Reviewed-by: Pascal Perez <pascallouis@google.com>
Reviewed-by: Benjamin Prosnitz <bprosnitz@google.com>
diff --git a/fidl-lsp/analysis/analyzer.go b/fidl-lsp/analysis/analyzer.go
index 45fa471..1d8729e 100644
--- a/fidl-lsp/analysis/analyzer.go
+++ b/fidl-lsp/analysis/analyzer.go
@@ -33,6 +33,7 @@
// exists (`json`), its deserialized JSON IR (`ir`), and any diagnostics on it.
// `deps` and `files` are treated as sets.
type Library struct {
+ name fidlcommon.LibraryName
deps map[fidlcommon.LibraryName]bool
// This library's constituent files in the form of absolute paths
files map[string]bool
@@ -56,7 +57,7 @@
// A map from library name (string), like "zx", to a set of data about that
// library: the FIDL files it includes, its JSON IR, its dependencies, its
// JSON IR, and diagnostics returned on it from fidlc and fidl-lint.
- libs map[fidlcommon.LibraryName]*Library
+ libs []*Library
// A map from open files to their corresponding tmp file used by the
// Analyzer as an input file for fidlc, fidl-lint, fidl-format, etc.
@@ -73,7 +74,7 @@
func NewAnalyzer(cfg Config, compiledLibraries CompiledLibraries) *Analyzer {
a := &Analyzer{
cfg: cfg,
- libs: make(map[fidlcommon.LibraryName]*Library),
+ libs: []*Library{},
inputFilesFIDL: make(map[state.FileID]string),
inputFilesJSON: make(map[fidlcommon.LibraryName]string),
}
@@ -84,19 +85,25 @@
// ImportCompiledLibraries adds the info for the compiled FIDL libraries in
// `compiledLibraries` to the Analyzer's set of libraries.
func (a *Analyzer) ImportCompiledLibraries(compiledLibraries CompiledLibraries) {
- for name, lib := range compiledLibraries {
- libName := fidlcommon.MustReadLibraryName(name)
- a.libs[libName] = &Library{
- deps: make(map[fidlcommon.LibraryName]bool, len(lib.Deps)),
- files: make(map[string]bool, len(lib.Files)),
- json: lib.JSON,
+ for _, compiledLib := range compiledLibraries {
+ name, err := fidlcommon.ReadLibraryName(compiledLib.Name)
+ if err != nil {
+ log.Printf("could not import compiled library `%s`\n", compiledLib.Name)
+ continue
}
- for _, dep := range lib.Deps {
- a.libs[libName].deps[fidlcommon.MustReadLibraryName(dep)] = true
+ lib := &Library{
+ name: name,
+ deps: make(map[fidlcommon.LibraryName]bool, len(compiledLib.Deps)),
+ files: make(map[string]bool, len(compiledLib.Files)),
+ json: compiledLib.JSON,
}
- for _, file := range lib.Files {
- a.libs[libName].files[file] = true
+ for _, dep := range compiledLib.Deps {
+ lib.deps[fidlcommon.MustReadLibraryName(dep)] = true
}
+ for _, file := range compiledLib.Files {
+ lib.files[file] = true
+ }
+ a.libs = append(a.libs, lib)
}
}
@@ -120,6 +127,60 @@
}
}
+// getLibrary looks up the library with `name` in a.libs. It does this with an
+// imperfect "heuristic" -- return the first library with a matching name --
+// imperfect because FIDL library names are not required to be unique. This
+// method essentially assumes that they are unique.
+//
+// Ideally, everywhere getLibrary is called should instead use the
+// (*Analyzer).getLibraryWithFile method, since this is guaranteed to be unique.
+// Calls to getLibrary should include a comment explaining why it's necessary.
+func (a *Analyzer) getLibrary(name fidlcommon.LibraryName) (*Library, bool) {
+ for _, lib := range a.libs {
+ if lib.name == name {
+ return lib, true
+ }
+ }
+ return nil, false
+}
+
+// getLibraryWithFile looks for a library in a.libs whose name matches `name`
+// and which contains a file with the path specified.
+//
+// This allows a more precise search than getLibrary, since library name + file
+// path together is guaranteed to be unique (a file only belongs to one FIDL
+// library).
+func (a *Analyzer) getLibraryWithFile(name fidlcommon.LibraryName, path state.FileID) (*Library, bool) {
+ // If there is a corresponding temporary input file used by the Analyzer,
+ // use that to lookup the library in a.libs.
+ inputFilePath, hadTmpFile := a.inputFilesFIDL[path]
+
+ // Also, try to convert the file to an absolute filepath.
+ // The Analyzer stores filepaths for the last analysis of the library, so if
+ // files have been changed and are now stored as temporary input file, there
+ // could be a mix of tmp files and absolute filepaths.
+ // We check for both.
+ absPath, err := state.FileIDToPath(path)
+
+ // If we can't find either a temporary file path to lookup, or the absolute
+ // file path, we fall back to using the getLibrary method, which doesn't
+ // parameterize on a file path.
+ if err != nil && !hadTmpFile {
+ return a.getLibrary(name)
+ }
+
+ for _, lib := range a.libs {
+ if lib.name == name {
+ for libFile := range lib.files {
+ if libFile == inputFilePath || libFile == absPath {
+ return lib, true
+ }
+ }
+ }
+ }
+ return nil, false
+}
+
// Analyze compiles and generates an analysis for the file at `path` in the
// FileSystem. This currently includes compiling the library the file belongs to
// and obtaining diagnostics on it.
@@ -137,15 +198,26 @@
return fmt.Errorf("could not find file `%s`", path)
}
if libName, err := state.LibraryOfFile(file); err == nil {
- if _, ok := a.libs[libName]; !ok {
- a.libs[libName] = &Library{
- deps: make(map[fidlcommon.LibraryName]bool),
- files: make(map[string]bool),
- diags: make(map[state.FileID][]Diagnostic),
+ // If this file is not already part of a library, either add it to its
+ // existing library or, if its library does not exist yet, create a new
+ // one.
+ var lib *Library
+ lib, ok := a.getLibraryWithFile(libName, path)
+ if !ok {
+ // We don't use getLibraryWithFile here because we have not yet added `file`
+ // to its library.
+ lib, ok = a.getLibrary(libName)
+ if !ok {
+ lib = &Library{
+ name: libName,
+ deps: make(map[fidlcommon.LibraryName]bool),
+ files: make(map[string]bool),
+ diags: make(map[state.FileID][]Diagnostic),
+ }
+ a.libs = append(a.libs, lib)
}
}
- lib := a.libs[libName]
// If the file currently being edited was already part of lib.files,
// delete that and replace it with the temporary/edited version.
if absPath, err := state.FileIDToPath(path); err != nil {
@@ -169,7 +241,7 @@
return fmt.Errorf("error on analysis: %s", err)
}
- a.libs[compileResult.lib].diags = compileResult.diags
+ compileResult.lib.diags = compileResult.diags
return nil
}
@@ -179,13 +251,13 @@
for _, path := range a.inputFilesFIDL {
err := os.Remove(path)
if err != nil {
- fmt.Fprintln(os.Stderr, err)
+ log.Print(err)
}
}
for _, path := range a.inputFilesJSON {
err := os.Remove(path)
if err != nil {
- fmt.Fprintln(os.Stderr, err)
+ log.Print(err)
}
}
}
diff --git a/fidl-lsp/analysis/compile.go b/fidl-lsp/analysis/compile.go
index 9336999..78d1744 100644
--- a/fidl-lsp/analysis/compile.go
+++ b/fidl-lsp/analysis/compile.go
@@ -9,6 +9,7 @@
"encoding/json"
"fmt"
"io/ioutil"
+ "log"
"os/exec"
"strconv"
"strings"
@@ -19,7 +20,7 @@
)
type compileResult struct {
- lib fidlcommon.LibraryName
+ lib *Library
diags map[state.FileID][]Diagnostic
}
@@ -52,7 +53,7 @@
cmd.Stderr = &stderr
if err := cmd.Run(); err == nil {
// If fidlc compiled successfully, update a.libs with the compiled JSON IR
- if err := a.importLibrary(jsonPath); err != nil {
+ if err := a.importLibraryWithFile(jsonPath, path); err != nil {
return compileResult{}, fmt.Errorf("error on adding compiled JSON: %s", err)
}
} else {
@@ -65,10 +66,15 @@
}
diags := make(map[state.FileID][]Diagnostic)
- for fileName := range a.libs[libraryName].files {
- fileID, err := a.inputFileToFileID(fileName)
- if err == nil {
- diags[fileID] = []Diagnostic{}
+ lib, ok := a.getLibraryWithFile(libraryName, path)
+ if !ok {
+ log.Printf("could not find library `%s` with file `%s`\n", libraryName.FullyQualifiedName(), path)
+ } else {
+ for fileName := range lib.files {
+ fileID, err := a.inputFileToFileID(fileName)
+ if err == nil {
+ diags[fileID] = []Diagnostic{}
+ }
}
}
if errorsAndWarnings, err := a.fidlcDiagsFromStderr(stderr.Bytes()); err == nil {
@@ -89,35 +95,61 @@
}
return compileResult{
- lib: libraryName,
+ lib: lib,
diags: diags,
}, nil
}
-// Read in the JSON IR at absolute filepath `jsonPath`.
-func (a *Analyzer) importLibrary(jsonPath string) error {
+// compileLibrary essentially does the same thing as importLibrary but for a
+// library that is already stored in a.libs, but the JSON IR for which has not
+// been imported.
+func (a *Analyzer) compileLibrary(lib *Library) error {
+ data, err := ioutil.ReadFile(lib.json)
+ if err != nil {
+ return err
+ }
+ var jsonIR FidlLibrary
+ if err := json.Unmarshal(data, &jsonIR); err != nil {
+ return err
+ }
+ lib.ir = &jsonIR
+
+ // Construct symbol map from JSON IR for the file's library
+ symbols, err := a.genSymbolMap(jsonIR)
+ if err != nil {
+ return fmt.Errorf("unable to generate symbol map from library %s: %s", jsonIR.Name, err)
+ }
+ lib.symbols = symbols
+
+ return nil
+}
+
+// importLibrary reads in the JSON IR at absolute filepath `jsonPath`.
+func (a *Analyzer) importLibraryWithFile(jsonPath string, path state.FileID) error {
data, err := ioutil.ReadFile(jsonPath)
if err != nil {
return err
}
- var lib FidlLibrary
- if err := json.Unmarshal(data, &lib); err != nil {
+ var jsonIR FidlLibrary
+ if err := json.Unmarshal(data, &jsonIR); err != nil {
return err
}
// TODO: import more things? files + deps?
- libName := fidlcommon.MustReadLibraryName(lib.Name)
- if _, ok := a.libs[libName]; !ok {
- a.libs[libName] = &Library{}
+ libName := fidlcommon.MustReadLibraryName(jsonIR.Name)
+ lib, ok := a.getLibraryWithFile(libName, path)
+ if !ok {
+ lib = &Library{}
+ a.libs = append(a.libs, lib)
}
- a.libs[libName].ir = &lib
- a.libs[libName].json = jsonPath
+ lib.ir = &jsonIR
+ lib.json = jsonPath
// Construct symbol map from JSON IR for the file's library
- symbols, err := a.genSymbolMap(lib)
+ symbols, err := a.genSymbolMap(jsonIR)
if err != nil {
- return fmt.Errorf("unable to generate symbol map from library %s: %s", lib.Name, err)
+ return fmt.Errorf("unable to generate symbol map from library %s: %s", jsonIR.Name, err)
}
- a.libs[libName].symbols = symbols
+ lib.symbols = symbols
return nil
}
diff --git a/fidl-lsp/analysis/definition.go b/fidl-lsp/analysis/definition.go
index 566b3be..e9ccc30 100644
--- a/fidl-lsp/analysis/definition.go
+++ b/fidl-lsp/analysis/definition.go
@@ -41,8 +41,12 @@
// the library (specifically, pointing at their `library` declarations).
libName, err := fidlcommon.ReadLibraryName(sym.Name)
if err == nil {
- if _, isLib := a.libs[libName]; isLib {
- return a.declarationsOfLibrary(fs, libName), nil
+ // TODO: We could use getLibraryWithFile here, but it would be more
+ // complicated: if this library name is a declaration, we should pass it
+ // the file the symbol is in; but if it's a library import, we should
+ // pass it a file of that imported library.
+ if lib, isLib := a.getLibrary(libName); isLib {
+ return a.declarationsOfLibrary(fs, lib), nil
}
}
@@ -56,17 +60,33 @@
)
}
- symInfo, err := a.lookupSymbolInfo(name)
- if err != nil {
- return nil, fmt.Errorf("could not find definition of symbol `%s`: %s", name.FullyQualifiedName(), err)
+ var symInfo *symbolInfo
+ if name.lib != nil {
+ symInfo, err = a.lookupSymbolInfoInLibrary(name.name, name.lib)
+ if err != nil {
+ return nil, fmt.Errorf(
+ "could not find definition of symbol `%s`: %s",
+ name.name.FullyQualifiedName(),
+ err,
+ )
+ }
+ } else {
+ symInfo, err = a.lookupSymbolInfo(name.name)
+ if err != nil {
+ return nil, fmt.Errorf(
+ "could not find definition of symbol `%s`: %s",
+ name.name.FullyQualifiedName(),
+ err,
+ )
+ }
}
return []state.Location{symInfo.definition}, nil
}
-func (a *Analyzer) declarationsOfLibrary(fs *state.FileSystem, lib fidlcommon.LibraryName) []state.Location {
+func (a *Analyzer) declarationsOfLibrary(fs *state.FileSystem, lib *Library) []state.Location {
files := []state.FileID{}
- for file := range a.libs[lib].files {
+ for file := range lib.files {
// These files are all absolute paths, but some are to temporary input
// files used by the Analyzer. For these, we want to convert the path
// to the document URI the editor knows about.
diff --git a/fidl-lsp/analysis/definition_test.go b/fidl-lsp/analysis/definition_test.go
index 3bd4c2e..b42a652 100644
--- a/fidl-lsp/analysis/definition_test.go
+++ b/fidl-lsp/analysis/definition_test.go
@@ -31,6 +31,7 @@
func TestDefinitionOfLibrary(t *testing.T) {
analyzer := analysis.NewAnalyzer(defaultConfig(), analysis.CompiledLibraries{})
+ defer analyzer.Cleanup()
fs := state.NewFileSystem()
fs.NewFile(
"file.fidl",
@@ -71,6 +72,7 @@
func TestDefinitionOfLibraryMultipleFiles(t *testing.T) {
analyzer := analysis.NewAnalyzer(defaultConfig(), analysis.CompiledLibraries{})
+ defer analyzer.Cleanup()
fs := state.NewFileSystem()
fs.NewFile(
"file1.fidl",
@@ -136,6 +138,7 @@
func TestDefinitionOfLibraryImport(t *testing.T) {
analyzer := analysis.NewAnalyzer(defaultConfig(), analysis.CompiledLibraries{})
+ defer analyzer.Cleanup()
fs := state.NewFileSystem()
fs.NewFile(
"import.fidl",
@@ -192,6 +195,7 @@
func TestDefinitionOfFullyQualifiedSymbol(t *testing.T) {
analyzer := analysis.NewAnalyzer(defaultConfig(), analysis.CompiledLibraries{})
+ defer analyzer.Cleanup()
fs := state.NewFileSystem()
fs.NewFile(
"import.fidl",
@@ -250,6 +254,7 @@
func TestDefinitionOfLocalSymbol(t *testing.T) {
analyzer := analysis.NewAnalyzer(defaultConfig(), analysis.CompiledLibraries{})
+ defer analyzer.Cleanup()
fs := state.NewFileSystem()
fs.NewFile(
"test.fidl",
@@ -298,6 +303,7 @@
func TestDefinitionOfSymbolInSameLibraryDifferentFile(t *testing.T) {
analyzer := analysis.NewAnalyzer(defaultConfig(), analysis.CompiledLibraries{})
+ defer analyzer.Cleanup()
fs := state.NewFileSystem()
fs.NewFile(
"test1.fidl",
@@ -353,8 +359,85 @@
}
}
+func TestTwoLibrariesWithSameName(t *testing.T) {
+ analyzer := analysis.NewAnalyzer(
+ defaultConfig(),
+ // Two distinct libraries with the same name
+ analysis.CompiledLibraries{
+ {
+ Name: "fuchsia.test",
+ Files: []string{"test1.fidl"},
+ },
+ {
+ Name: "fuchsia.test",
+ Files: []string{"test2.fidl"},
+ },
+ },
+ )
+ defer analyzer.Cleanup()
+
+ fs := state.NewFileSystem()
+ fs.NewFile(
+ "test1.fidl",
+ `
+library fuchsia.test;
+
+struct Foo {};
+union Bar {
+ 1: Foo foo;
+};
+`,
+ )
+ fs.NewFile(
+ "test2.fidl",
+ `
+library fuchsia.test;
+
+struct Foo {};
+union Bar {
+ 1: Foo foo;
+};
+`,
+ )
+ if err := analyzer.Analyze(fs, "test1.fidl"); err != nil {
+ t.Fatalf("failed to analyze file: %s", err)
+ }
+ if err := analyzer.Analyze(fs, "test2.fidl"); err != nil {
+ t.Fatalf("failed to analyze file: %s", err)
+ }
+
+ locs, err := analyzer.DefinitionOfSymbol(fs, state.Symbol{
+ Name: "Foo",
+ Location: state.Location{
+ FileID: state.FileID("test1.fidl"),
+ Range: state.Range{
+ Start: state.Position{Line: 5, Character: 7},
+ End: state.Position{Line: 5, Character: 10},
+ },
+ },
+ })
+ if err != nil {
+ t.Fatalf("error getting definition of symbol: %s", err)
+ }
+
+ expLoc := state.Location{
+ FileID: state.FileID("test1.fidl"),
+ Range: state.Range{
+ Start: state.Position{Line: 3, Character: 7},
+ End: state.Position{Line: 3, Character: 10},
+ },
+ }
+ if len(locs) != 1 {
+ t.Fatalf("expected 1 definition location")
+ }
+ if locs[0] != expLoc {
+ t.Errorf("unexpected definition of symbol: expected %v, got %v", expLoc, locs[0])
+ }
+}
+
func TestDefinitionOfReservedKeyword(t *testing.T) {
analyzer := analysis.NewAnalyzer(defaultConfig(), analysis.CompiledLibraries{})
+ defer analyzer.Cleanup()
fs := state.NewFileSystem()
fs.NewFile(
"test.fidl",
diff --git a/fidl-lsp/analysis/deps.go b/fidl-lsp/analysis/deps.go
index 506ecfa..24d6b00 100644
--- a/fidl-lsp/analysis/deps.go
+++ b/fidl-lsp/analysis/deps.go
@@ -19,12 +19,13 @@
// This is a file that declares all the FIDL libraries the language server
// should be aware of, and includes information on how to build them and find
// their compiled JSON IR.
-type CompiledLibraries map[string]CompiledLibrary
+type CompiledLibraries []CompiledLibrary
// CompiledLibrary represents the build information for a single FIDL library,
-// including its constituent files, its dependencies, and the absolute filepath
-// to its compiled JSON IR.
+// including its name, its constituent files, its dependencies, and the absolute
+// filepath to its compiled JSON IR.
type CompiledLibrary struct {
+ Name string
Files []string
Deps []string
JSON string
@@ -92,7 +93,11 @@
// Find the library's files and dependencies.
var info libraryInfo
- if lib, ok := a.libs[libName]; ok {
+ // We don't call getLibraryWithFile here because these libraries are
+ // found either by searching for regex patterns of library imports, or
+ // by looking in a library's dependencies, which are only stored as bare
+ // library names.
+ if lib, ok := a.getLibrary(libName); ok {
// Add the library's files to info.files
for file := range lib.files {
// Unless that file is a duplicate of one that is open
diff --git a/fidl-lsp/analysis/deps_test.go b/fidl-lsp/analysis/deps_test.go
index 04e7140..000e9b0 100644
--- a/fidl-lsp/analysis/deps_test.go
+++ b/fidl-lsp/analysis/deps_test.go
@@ -22,6 +22,7 @@
func TestFindDepsNoDeps(t *testing.T) {
analyzer := analysis.NewAnalyzer(analysis.Config{}, analysis.CompiledLibraries{})
+ defer analyzer.Cleanup()
fs := state.NewFileSystem()
fs.NewFile(
"file.fidl",
@@ -42,14 +43,17 @@
analyzer := analysis.NewAnalyzer(
analysis.Config{},
analysis.CompiledLibraries{
- "fuchsia.import1": analysis.CompiledLibrary{
+ {
+ Name: "fuchsia.import1",
Files: []string{"import1.fidl"},
},
- "fuchsia.import2": analysis.CompiledLibrary{
+ {
+ Name: "fuchsia.import2",
Files: []string{"import2.fidl"},
},
},
)
+ defer analyzer.Cleanup()
fs := state.NewFileSystem()
fs.NewFile(
@@ -88,22 +92,27 @@
analyzer := analysis.NewAnalyzer(
analysis.Config{},
analysis.CompiledLibraries{
- "fuchsia.import1": analysis.CompiledLibrary{
+ {
+ Name: "fuchsia.import1",
Files: []string{"import1.fidl"},
Deps: []string{"fuchsia.import2", "fuchsia.import3"},
},
- "fuchsia.import2": analysis.CompiledLibrary{
+ {
+ Name: "fuchsia.import2",
Files: []string{"import2.fidl"},
Deps: []string{"fuchsia.import4"},
},
- "fuchsia.import3": analysis.CompiledLibrary{
+ {
+ Name: "fuchsia.import3",
Files: []string{"import3.fidl"},
},
- "fuchsia.import4": analysis.CompiledLibrary{
+ {
+ Name: "fuchsia.import4",
Files: []string{"import4.fidl"},
},
},
)
+ defer analyzer.Cleanup()
fs := state.NewFileSystem()
fs.NewFile(
@@ -146,22 +155,27 @@
analyzer := analysis.NewAnalyzer(
analysis.Config{},
analysis.CompiledLibraries{
- "fuchsia.import1": analysis.CompiledLibrary{
+ {
+ Name: "fuchsia.import1",
Files: []string{"import1_1.fidl", "import1_2.fidl"},
Deps: []string{"fuchsia.import3", "fuchsia.import4"},
},
- "fuchsia.import2": analysis.CompiledLibrary{
+ {
+ Name: "fuchsia.import2",
Files: []string{"import2_1.fidl", "import2_2.fidl"},
Deps: []string{"fuchsia.import4"},
},
- "fuchsia.import3": analysis.CompiledLibrary{
+ {
+ Name: "fuchsia.import3",
Files: []string{"import3.fidl"},
},
- "fuchsia.import4": analysis.CompiledLibrary{
+ {
+ Name: "fuchsia.import4",
Files: []string{"import4_1.fidl", "import4_2.fidl", "import4_3.fidl"},
},
},
)
+ defer analyzer.Cleanup()
fs := state.NewFileSystem()
fs.NewFile(
@@ -206,6 +220,7 @@
func TestFindDepsNoLibraryInFile(t *testing.T) {
analyzer := analysis.NewAnalyzer(analysis.Config{}, analysis.CompiledLibraries{})
+ defer analyzer.Cleanup()
fs := state.NewFileSystem()
fs.NewFile(
"test1.fidl",
@@ -220,6 +235,7 @@
func TestFindDepsImportNotInFidlProject(t *testing.T) {
analyzer := analysis.NewAnalyzer(analysis.Config{}, analysis.CompiledLibraries{})
+ defer analyzer.Cleanup()
fs := state.NewFileSystem()
fs.NewFile(
"foo.fidl",
@@ -239,16 +255,19 @@
analyzer := analysis.NewAnalyzer(
analysis.Config{},
analysis.CompiledLibraries{
- "fuchsia.foo": analysis.CompiledLibrary{
+ {
+ Name: "fuchsia.foo",
Files: []string{"foo.fidl"},
Deps: []string{"fuchsia.bar"},
},
- "fuchsia.bar": analysis.CompiledLibrary{
+ {
+ Name: "fuchsia.bar",
Files: []string{"bar.fidl"},
Deps: []string{"fuchsia.foo"},
},
},
)
+ defer analyzer.Cleanup()
fs := state.NewFileSystem()
fs.NewFile(
diff --git a/fidl-lsp/analysis/diagnostics.go b/fidl-lsp/analysis/diagnostics.go
index 05a42b6..c8f7e4a 100644
--- a/fidl-lsp/analysis/diagnostics.go
+++ b/fidl-lsp/analysis/diagnostics.go
@@ -14,12 +14,14 @@
"fidl-lsp/state"
)
-// DiagnosticsOnLibrary retrieves the cached diagnostics for `lib`.
-func (a *Analyzer) DiagnosticsOnLibrary(lib fidlcommon.LibraryName) (map[state.FileID][]Diagnostic, error) {
- if _, ok := a.libs[lib]; !ok {
- return nil, fmt.Errorf("could not find library `%s`", lib)
+// DiagnosticsOnLibrary retrieves the cached diagnostics for the library with
+// name `libName` including the file `path`.
+func (a *Analyzer) DiagnosticsOnLibrary(libName fidlcommon.LibraryName, path state.FileID) (map[state.FileID][]Diagnostic, error) {
+ lib, ok := a.getLibraryWithFile(libName, path)
+ if !ok {
+ return nil, fmt.Errorf("could not find library `%s`", libName)
}
- return a.libs[lib].diags, nil
+ return lib.diags, nil
}
type fidlLintSuggestion struct {
diff --git a/fidl-lsp/analysis/references.go b/fidl-lsp/analysis/references.go
index 346f9c9..95727a1 100644
--- a/fidl-lsp/analysis/references.go
+++ b/fidl-lsp/analysis/references.go
@@ -27,8 +27,12 @@
// declarations).
libName, err := fidlcommon.ReadLibraryName(sym.Name)
if err == nil {
- if _, isLib := a.libs[libName]; isLib {
- return a.importsOfLibrary(fs, libName), nil
+ // TODO: We could use getLibraryWithFile here, but it would be more
+ // complicated: if this library name is a declaration, we should pass it
+ // the file the symbol is in; but if it's a library import, we should
+ // pass it a file of that imported library.
+ if lib, isLib := a.getLibrary(libName); isLib {
+ return a.importsOfLibrary(fs, lib.name), nil
}
}
@@ -44,23 +48,27 @@
// Search for references to `name` in all the libraries that import `name`'s
// library, as well as its own library.
- libs := a.librariesThatImport(name.LibraryName())
- libs = append(libs, name.LibraryName())
+ libs := a.librariesThatImport(name.name.LibraryName())
+ selfLibrary, ok := a.getLibraryWithFile(name.name.LibraryName(), sym.Location.FileID)
+ if !ok {
+ return nil, fmt.Errorf("could not find library of symbol `%s`", sym.Name)
+ }
+ libs = append(libs, selfLibrary)
refs := []state.Location{}
for _, lib := range libs {
- if a.libs[lib].ir == nil {
- if err := a.importLibrary(a.libs[lib].json); err != nil {
+ if lib.ir == nil {
+ if err := a.compileLibrary(lib); err != nil {
return nil, fmt.Errorf(
"error importing library `%s`: %s",
- name.LibraryName().FullyQualifiedName(),
+ name.name.LibraryName().FullyQualifiedName(),
err,
)
}
}
- for _, si := range a.libs[lib].symbols {
- if a.typeReferencesName(si.typeInfo, name) {
+ for _, si := range lib.symbols {
+ if a.typeReferencesName(si.typeInfo, name.name) {
refs = append(refs, si.definition)
}
}
@@ -78,12 +86,12 @@
return locs
}
-func (a *Analyzer) librariesThatImport(library fidlcommon.LibraryName) []fidlcommon.LibraryName {
- libs := []fidlcommon.LibraryName{}
- for name, lib := range a.libs {
+func (a *Analyzer) librariesThatImport(name fidlcommon.LibraryName) []*Library {
+ libs := []*Library{}
+ for _, lib := range a.libs {
for dep := range lib.deps {
- if dep == library {
- libs = append(libs, name)
+ if dep == name {
+ libs = append(libs, lib)
break
}
}
@@ -91,9 +99,9 @@
return libs
}
-func (a *Analyzer) importsOfLibraryInLibrary(fs *state.FileSystem, importedLib fidlcommon.LibraryName, library fidlcommon.LibraryName) []state.Location {
+func (a *Analyzer) importsOfLibraryInLibrary(fs *state.FileSystem, importedLib fidlcommon.LibraryName, library *Library) []state.Location {
files := []state.FileID{}
- for file := range a.libs[library].files {
+ for file := range library.files {
// These files are all absolute paths, but some are to temporary input
// files used by the Analyzer. For these, we want to convert the path
// to the document URI the editor knows about.
diff --git a/fidl-lsp/analysis/references_test.go b/fidl-lsp/analysis/references_test.go
index 608477a..5e021b4 100644
--- a/fidl-lsp/analysis/references_test.go
+++ b/fidl-lsp/analysis/references_test.go
@@ -11,20 +11,31 @@
"fidl-lsp/state"
)
+type TestFile struct {
+ name state.FileID
+ text string
+}
+
func TestReferencesToSymbol(t *testing.T) {
cases := []struct {
name string
- files map[state.FileID]string
+ files []TestFile
symbol state.Symbol
expRefs []state.Location
}{
{
name: "Reference to library declaration",
- files: map[state.FileID]string{
- "test.fidl": `library fuchsia.test;`,
- "other.fidl": `
+ files: []TestFile{
+ {
+ "test.fidl",
+ `library fuchsia.test;`,
+ },
+ {
+ "other.fidl",
+ `
library other;
using fuchsia.test;`,
+ },
},
symbol: state.Symbol{
Name: "fuchsia.test",
@@ -48,11 +59,17 @@
},
{
name: "Reference to library import",
- files: map[state.FileID]string{
- "test.fidl": `library fuchsia.test;`,
- "other.fidl": `
+ files: []TestFile{
+ {
+ "test.fidl",
+ `library fuchsia.test;`,
+ },
+ {
+ "other.fidl",
+ `
library other;
using fuchsia.test;`,
+ },
},
symbol: state.Symbol{
Name: "fuchsia.test",
@@ -76,8 +93,9 @@
},
{
name: "Reference to symbol in same file",
- files: map[state.FileID]string{
- "test.fidl": `
+ files: []TestFile{{
+ "test.fidl",
+ `
library test;
struct Foo {};
@@ -87,7 +105,7 @@
// ~~~
};
`,
- },
+ }},
symbol: state.Symbol{
Name: "Foo",
Location: state.Location{
@@ -110,13 +128,18 @@
},
{
name: "Reference to symbol in other file in same library",
- files: map[state.FileID]string{
- "test1.fidl": `
+ files: []TestFile{
+ {
+ "test1.fidl",
+ `
library test;
struct Foo {};
`,
- "test2.fidl": `
+ },
+ {
+ "test2.fidl",
+ `
library test;
protocol Bar {
@@ -124,6 +147,7 @@
// ~~~
};
`,
+ },
},
symbol: state.Symbol{
Name: "Foo",
@@ -147,13 +171,18 @@
},
{
name: "References to symbol in other library",
- files: map[state.FileID]string{
- "imported.fidl": `
+ files: []TestFile{
+ {
+ "imported.fidl",
+ `
library fuchsia.imported;
struct Foo {};
`,
- "test.fidl": `
+ },
+ {
+ "test.fidl",
+ `
library test;
using fuchsia.imported;
@@ -162,6 +191,7 @@
// ~~~
};
`,
+ },
},
symbol: state.Symbol{
Name: "Foo",
@@ -185,13 +215,14 @@
},
{
name: "No references to symbol",
- files: map[state.FileID]string{
- "test.fidl": `
+ files: []TestFile{{
+ "test.fidl",
+ `
library test;
struct Foo {};
`,
- },
+ }},
symbol: state.Symbol{
Name: "Foo",
Location: state.Location{
@@ -206,13 +237,19 @@
},
{
name: "References to type alias",
- files: map[state.FileID]string{
- "zx.fidl": `
+ files: []TestFile{
+ {
+ "zx.fidl",
+ `
library zx;
using status = int32;
`,
- "test.fidl": `
+ },
+ {
+
+ "test.fidl",
+ `
library test;
using zx;
@@ -221,6 +258,7 @@
// ~~~~~~
};
`,
+ },
},
symbol: state.Symbol{
Name: "status",
@@ -244,8 +282,9 @@
},
{
name: "References in type parameters",
- files: map[state.FileID]string{
- "test.fidl": `
+ files: []TestFile{{
+ "test.fidl",
+ `
library test;
table Foo {};
@@ -255,7 +294,7 @@
// ~~~~~
};
`,
- },
+ }},
symbol: state.Symbol{
Name: "Foo",
Location: state.Location{
@@ -280,12 +319,13 @@
for _, ex := range cases {
analyzer := analysis.NewAnalyzer(defaultConfig(), analysis.CompiledLibraries{})
+ defer analyzer.Cleanup()
fs := state.NewFileSystem()
- for file, text := range ex.files {
- fs.NewFile(file, text)
+ for _, file := range ex.files {
+ fs.NewFile(file.name, file.text)
}
- for file := range ex.files {
- if err := analyzer.Analyze(fs, file); err != nil {
+ for _, file := range ex.files {
+ if err := analyzer.Analyze(fs, file.name); err != nil {
t.Fatalf("case `%s`: failed to analyze file `%s`: %s", ex.name, file, err)
}
}
diff --git a/fidl-lsp/analysis/symbol.go b/fidl-lsp/analysis/symbol.go
index c6e1554..a76e227 100644
--- a/fidl-lsp/analysis/symbol.go
+++ b/fidl-lsp/analysis/symbol.go
@@ -186,7 +186,12 @@
Type Type
}
-func (a *Analyzer) symbolToFullyQualifiedName(fs *state.FileSystem, sym state.Symbol) (fidlcommon.Name, error) {
+type nameInLibrary struct {
+ lib *Library // Left nil if uncertain
+ name fidlcommon.Name
+}
+
+func (a *Analyzer) symbolToFullyQualifiedName(fs *state.FileSystem, sym state.Symbol) (nameInLibrary, error) {
// TODO: if we want to support hovering over members (e.g. struct fields,
// method parameters, bits members) or protocol methods, we need to check
// here whether the symbol is namespaced -- whether it is inside a
@@ -205,20 +210,25 @@
// If `sym` is a local name (not fully-qualified), we create a FQN by
// attaching its library name.
var fqn string
+ var lib *Library
if !strings.Contains(sym.Name, ".") {
file, err := fs.File(sym.Location.FileID)
if err != nil {
- return fidlcommon.Name{}, fmt.Errorf("could not open file `%s`", sym.Location.FileID)
+ return nameInLibrary{}, fmt.Errorf("could not open file `%s`", sym.Location.FileID)
}
libName, err := state.LibraryOfFile(file)
if err != nil {
- return fidlcommon.Name{}, fmt.Errorf(
+ return nameInLibrary{}, fmt.Errorf(
"could not find library of symbol `%s` in file `%s`",
sym.Name,
sym.Location.FileID,
)
}
fqn = libName.FullyQualifiedName() + "/" + sym.Name
+ // We know this symbol's library, so we can return it
+ if library, ok := a.getLibraryWithFile(libName, sym.Location.FileID); ok {
+ lib = library
+ }
} else {
// If the symbol contains '.', we assume it is a fully-qualified name.
i := strings.LastIndex(sym.Name, ".")
@@ -228,18 +238,27 @@
// Convert `fqn` to a fidlcommon.Name.
name, err := fidlcommon.ReadName(fqn)
if err != nil {
- return fidlcommon.Name{}, fmt.Errorf("could not read fully-qualified name `%s`: %s", fqn, err)
+ return nameInLibrary{}, fmt.Errorf("could not read fully-qualified name `%s`: %s", fqn, err)
}
- return name, nil
+ return nameInLibrary{lib: lib, name: name}, nil
}
func (a *Analyzer) lookupSymbolInfo(name fidlcommon.Name) (*symbolInfo, error) {
- lib, ok := a.libs[name.LibraryName()]
+ // We don't call getLibraryWithFile here because we don't know for sure
+ // which library this symbol is part of. Unless the library is declared as a
+ // dependency of the library that includes this symbol, it couldn't be
+ // resolved, and we want to be able to resolve the symbol even if a
+ // dependency isn't declared.
+ lib, ok := a.getLibrary(name.LibraryName())
if !ok {
return nil, fmt.Errorf("unknown library `%s`", name.LibraryName().FullyQualifiedName())
}
+ return a.lookupSymbolInfoInLibrary(name, lib)
+}
+
+func (a *Analyzer) lookupSymbolInfoInLibrary(name fidlcommon.Name, lib *Library) (*symbolInfo, error) {
if lib.ir == nil {
- if err := a.importLibrary(lib.json); err != nil {
+ if err := a.compileLibrary(lib); err != nil {
return nil, fmt.Errorf(
"error importing library `%s`: %s",
name.LibraryName().FullyQualifiedName(),
diff --git a/fidl-lsp/analysis/type.go b/fidl-lsp/analysis/type.go
index fcec389..041c4ec 100644
--- a/fidl-lsp/analysis/type.go
+++ b/fidl-lsp/analysis/type.go
@@ -20,7 +20,11 @@
// If `sym` is a library name, return a `library`-kinded TypeInfo.
libName, err := fidlcommon.ReadLibraryName(sym.Name)
if err == nil {
- if _, isLib := a.libs[libName]; isLib {
+ // TODO: We could use getLibraryWithFile here, but it would be more
+ // complicated: if this library name is a declaration, we should pass it
+ // the file the symbol is in; but if it's a library import, we should
+ // pass it a file of that imported library.
+ if _, isLib := a.getLibrary(libName); isLib {
return Type{
IsLib: true,
}, nil
@@ -37,9 +41,25 @@
)
}
- symInfo, err := a.lookupSymbolInfo(name)
- if err != nil {
- return Type{}, fmt.Errorf("could not find type of symbol `%s`: %s", name.FullyQualifiedName(), err)
+ var symInfo *symbolInfo
+ if name.lib != nil {
+ symInfo, err = a.lookupSymbolInfoInLibrary(name.name, name.lib)
+ if err != nil {
+ return Type{}, fmt.Errorf(
+ "could not find type of symbol `%s`: %s",
+ name.name.FullyQualifiedName(),
+ err,
+ )
+ }
+ } else {
+ symInfo, err = a.lookupSymbolInfo(name.name)
+ if err != nil {
+ return Type{}, fmt.Errorf(
+ "could not find type of symbol `%s`: %s",
+ name.name.FullyQualifiedName(),
+ err,
+ )
+ }
}
// Resolve identifier type, if necessary.
@@ -60,7 +80,7 @@
return Type{}, fmt.Errorf(
"could not find identifier type `%s` of symbol `%s`: %s",
typeName.FullyQualifiedName(),
- name.FullyQualifiedName(),
+ name.name.FullyQualifiedName(),
err,
)
}
@@ -91,7 +111,7 @@
return Type{}, fmt.Errorf(
"could not find identifier type `%s` of symbol `%s`: %s",
typeName.FullyQualifiedName(),
- name.FullyQualifiedName(),
+ name.name.FullyQualifiedName(),
err,
)
}
diff --git a/fidl-lsp/langserver/config.go b/fidl-lsp/langserver/config.go
index 1924550..56662fd 100644
--- a/fidl-lsp/langserver/config.go
+++ b/fidl-lsp/langserver/config.go
@@ -67,7 +67,7 @@
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)
+ 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)
diff --git a/fidl-lsp/langserver/diagnostics.go b/fidl-lsp/langserver/diagnostics.go
index 01d1212..f8aef73 100644
--- a/fidl-lsp/langserver/diagnostics.go
+++ b/fidl-lsp/langserver/diagnostics.go
@@ -42,7 +42,7 @@
return nil, fmt.Errorf("could not find library name of file `%s`: %s", uri, err)
}
- diags, err := h.analyzer.DiagnosticsOnLibrary(fidlcommon.LibraryName(libraryName))
+ diags, err := h.analyzer.DiagnosticsOnLibrary(fidlcommon.LibraryName(libraryName), state.FileID(uri))
if err != nil {
return nil, fmt.Errorf("error getting diagnostics: %s", err)
}
