Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

LSIF confusion #680

Closed
mpickering opened this issue Feb 14, 2019 · 15 comments
Closed

LSIF confusion #680

mpickering opened this issue Feb 14, 2019 · 15 comments

Comments

@mpickering
Copy link

mpickering commented Feb 14, 2019

I am trying to implement a program which generates the LSIF files as specified in #623 but am struggling to understand how cross-package references are supposed to work.

Question 1

In my mental model, what should happen is that each package generates its own LSIF file. Called lsif.json which contains the indicies for a specific package. Then in a local project, I generate a lsif.json file which includes these lsif.json files from whereever they are stored.

However, is it intended that all these lsif.json files are combined together so that there is one lsif.json file for a project which copies in the lsif.json files from dependencies?

Question 2: URIs

For a single file which exists on disk it is clear the URI should be something like:

file:///home/matt/hie-lsif/test/simple-tests/A.js

However, if an external package distributes an LSIF file what should it declare its URI as?
In the first case our decision is forced because vscode requests information about file://... from the language server. Is this a situation to set the URI to a URL or embed the module content in using data?

Question 3: Confusing example from imports section

In the following example, the observablemap.d.ts module is imported but the node 76 is not used
anywhere in the example so it's not clear what the point of it is. Can this please be clarified?

{ id: 76, type: "vertex", label: "document", uri: "file:///Users/dirkb/samples/node_modules/mobx/lib/types/observablemap.d.ts", languageId: "typescript" }
{ id: 77, type: "vertex", label: "externalImportResult" }
{ id: 61, type: "edge", label: "imports", outV: 59, inV: 60 }
{ id: 80, type: "vertex", label: "range", start: { line: 30, character: 21 }, end: { line: 30, character: 34 }, tag: { type: "definition", text: "ObservableMap", kind: 5, fullRange: {start: { line: 30, character:0 }, end: { line: 81, character:1 } } } }
{ id: 88, type: "vertex", label: "externalImportItem", moniker: "ObservableMap", rangeIds:[80] }
{ id: 89, type: "edge", label: "item", outV: 77, inV: 88 }

cc @dbaeumer

Could you also please provide complete .json dumps of the examples in the specification as it's hard to verify if the JSON produced is correct when only fragments are included.

@mpickering
Copy link
Author

I think my main source of confusion here is to do with the imports example.

The file is set to "file:///Users/dirkb/samples/node_modules/mobx/lib/types/observablemap.d.ts" but then how does the language server know where to find the lsif index for this file. I'm thinking of situations like cross-project goto-definition.

@jdneo
Copy link
Member

jdneo commented Apr 2, 2019

I'm confused about the cross-projet GTD too.

Different from TS, for the Java project, a lot of projects are managed by Maven/Gradle. The external dependencies are in the form of *.jar file, which is not a source file. Then what will the uris of the import files be?

@dbaeumer
Copy link
Member

dbaeumer commented Apr 2, 2019

Sorry for the long delay. I push a change today about how the monikers work. In principal they stay the same, but the do have some details which help to explain how this will work. The two big changes are:

  • a moniker is linked to a range directly. This will allow to link it to a reference or a declaration / definition.
  • a moniker has a scheme and an identifier.

In addition a moniker can be linked to a package information which can carry version information.

With the the Java problem can be solved as follows:

  • the moniker is directly linked to the reference of a symbol from a jar.
  • the tool generates a virtual document for the class file in a Jar (basicallly what you get when you open a class file in Eclipse without having sources attached). This file can then be treated as a declaration class file. The file doesn't need to exist on disk. So you can craft a URL like java-jar://..../ for it. You can even add the content of the generated file to the dump.

I would actually go with the second solution since it makes the dump self contained. Basically it will contain the Java jar declarations as virtual documents.

Let me know if this helps?

@jdneo
Copy link
Member

jdneo commented Apr 3, 2019

@dbaeumer Thank you for the reply. The idea of the virtual document looks great! A new question from my side is that: will there be a spec for the virtual document part? For example, how should it be stored with the dump file. Guessing the same problem also exists in other languages, like C# maybe.

@arjunattam
Copy link

Hey @jdneo, the spec for this is to add a property contents to the document vertex which stores the encoded content of the document in base64. Learn more in this section of the spec. Let us know if this answers your question.

@jdneo
Copy link
Member

jdneo commented Apr 3, 2019

@arjun27 Yes, that answers my questions. Thank you for your help. 👍

@jdneo
Copy link
Member

jdneo commented Jun 14, 2019

Hi @dbaeumer,

Sorry I was working on some other stuff the days before and just started to revisit the Java LSIF these days.

Regarding the moniker, it's clear that in the import scenario, since the packageInformation vertex has the version and URL information, as long as the information is correctly provided in the pom.xml we can navigate correctly crossing the repo.

One thing I'm still confusing is about the export scenario. How will the export moniker be used? Is it designed to be used when the packageInformation vertex could not provide enough information?

Thanks.

//cc @arjun27

@dbaeumer
Copy link
Member

@jdneo yes, a exported symbol should have a moniker as well. The moniker can of course only have a scheme that is tailored to Java since you don't have any package information. In TS export symbols do have a tsc scheme. For TS and npm we have a second tool that checks whether the project defines a npm package and then makes the moniker npm specific and adds a package information.

This being said we are working on a new version of the LSIF spec which will ease this since it will support moniker mapping. So the Java exporter can make all moniker java specific and then a second tool can map them for example to Maven specific monikers including package information.

@jdneo
Copy link
Member

jdneo commented Jun 17, 2019

Thank you @dbaeumer for the explanation. Does that mean:

Both the import and export moniker can point to the package information vertex?

By saying the new LSIF spec, is that already implemented in the newest LSIF-Node indexer?

@dbaeumer
Copy link
Member

@jdneo yes, two monikers can point to one package information.

A new version of ths LSIF Spec

I am in the process of implementing it right now. Will post something in the LSIF repository when available.

@jdneo
Copy link
Member

jdneo commented Jun 20, 2019

Thank you @dbaeumer for your information. May I ask when the new spec will be updated? Just an estimated approximate time could be helpful!

Thanks.

@dbaeumer
Copy link
Member

Beginning of July :-)

@Avi-D-coder
Copy link

@dbaeumer Perhaps I missed this, but I could not find any reference to when a client should use LSIF vs server.

  1. What happens when a file is changed? Is the entire LSIF file regenerated?

  2. Is LSIF only meant for static dependencies not the project being worked on?

  3. What if a build tool produced a LSIF file, not the server? The file will become partially out of date, should clients track changed ranges or files, to invalidate, or offset queries into these potentially slightly out of sync LSIF files?

  4. What if a two tools produce different supplemental LSIF files?

@dbaeumer
Copy link
Member

@Avi-D-coder the main use case for LSIF is right now read-only browsing. The experience shows that generating the file is as expensive as starting a LS for single projects. A win can only be seen of multi project workspaces where the user edits leaf projects in the dependency tree.

@dbaeumer
Copy link
Member

Closing the issue. I see that LSIF has potential aside rea-only scenarios but it is nothing I am actively working on right now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants