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 generation can't be interpreted by 0.6.0 validators #63840

Closed
NTaylorMullen opened this issue Sep 7, 2022 · 9 comments · Fixed by #64256
Closed

LSIF generation can't be interpreted by 0.6.0 validators #63840

NTaylorMullen opened this issue Sep 7, 2022 · 9 comments · Fixed by #64256
Assignees
Labels
Area-Compilers untriaged Issues and PRs which have not yet been triaged by a lead
Milestone

Comments

@NTaylorMullen
Copy link
Contributor

NTaylorMullen commented Sep 7, 2022

Version Used: 4.4.0-1.22372.4

Description:

When LSIF moved to 0.6.0 they introduced a few breaking changes in order to ease the storability of the LSIF format. These breaking changes end up hindering validators from understanding C# output. Some of those breaking changes are:

  1. All moniker nodes should have a unique property that specifies its UniquenessLevel.
  2. All nodes where the document property was referencing an id of the document containing it got renamed to shard.
  3. All project nodes should have a name property on them to indicate what the project name is

Steps to Reproduce:

  1. Run LSIF generator on any C# project with content 😄
@dotnet-issue-labeler dotnet-issue-labeler bot added Area-Compilers untriaged Issues and PRs which have not yet been triaged by a lead labels Sep 7, 2022
@jasonmalinowski jasonmalinowski self-assigned this Sep 8, 2022
@jasonmalinowski
Copy link
Member

@NTaylorMullen Does the moniker uniqueness replace the older MonikerKind? Or do I still need both?

@jasonmalinowski
Copy link
Member

item nodes document property got replaced with shard. Instead of re-specifying what document the shard indicates the id of the document to associate with.

Was this just a rename (effectively) or is there some other newer concept that a 'shard' is invoking here?

@jasonmalinowski jasonmalinowski added this to the 17.4 milestone Sep 8, 2022
@NTaylorMullen
Copy link
Contributor Author

@NTaylorMullen Does the moniker uniqueness replace the older MonikerKind? Or do I still need both?

kind is now optional. Judging from the docs to me it does look like a replacement.

@dbaeumer am I wrong?

@NTaylorMullen
Copy link
Contributor Author

item nodes document property got replaced with shard. Instead of re-specifying what document the shard indicates the id of the document to associate with.

Was this just a rename (effectively) or is there some other newer concept that a 'shard' is invoking here?

Was a naming change! And after looking through the docs, it looks like everywhere where document was used as an id reference (references, definition, referenceResults, referenceLInks) it was renamed. I've updated this issue.

@NTaylorMullen
Copy link
Contributor Author

As an example on how it looks now:
image
....
image

@dbaeumer
Copy link

dbaeumer commented Sep 9, 2022

@NTaylorMullen Does the moniker uniqueness replace the older MonikerKind? Or do I still need both?

kind is now optional. Judging from the docs to me it does look like a replacement.

No, kind in used to denote if the symbol is import or exported
Uniqueness denote how unique a moniker is: for example a TSC moniker is only unique inside a project since two project can define the same fully qualified name for a symbol. A npm moniker in contrast is globally unique since there can only be one package with name X

@jasonmalinowski
Copy link
Member

As far as uniqueness then I think all the monitors we have been generating are at least unique under this:

	/**
	 * The moniker is unique inside the moniker scheme.
	 */
	scheme = 'scheme',

We create two kinds of monikers with two schemes for historical reasons:

  1. Ones that are prefixed with the assembly names and then the fully qualified name of the type/member.
  2. Ones that represent a namespace, which spreads across all assemblies.

But either of those are unique from the perspective of the .NET type system -- you can't have a project with a duplicate (or at least not in any meaningful way.)

A npm moniker in contrast is globally unique since there can only be one package with name X

@dbaeumer Would that be 'global' or 'scheme' unique? I'd assume there'd be a npm 'scheme' that its' unique under?

@dbaeumer
Copy link

@jasonmalinowski thanks for asking that since my last answer was indeed misleading. For npm package we have an npm scheme and these packages are scheme unique. 'global' unique makes IMO only sense if a language uses global unique URIs.

@jasonmalinowski
Copy link
Member

@dbaeumer Thanks for confirming, and I think after I made my comment I saw the other description in the LSIF spec that clarified.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-Compilers untriaged Issues and PRs which have not yet been triaged by a lead
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants