-
Notifications
You must be signed in to change notification settings - Fork 237
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
LocationRange is missing for object fields #456
Comments
It should have the LocRange (all expressions should ideally have a locrange). In case of desugared expressions it's not always obvious to what part of the original code it maps, but it can always be "the smallest expression in the original code to which it can be traced". In this particular case, it's not a very big deal, but may be worth fixing while doing other cleanup (unless we know it's breaking something). |
@sbarzowski My understanding is that this AST is the representation of the original source code so I'm not sure why it's not obvious to what part of the original code the AST node maps. It would be great if you can give an example of such a case. For this particular case, I'm working on IDE integration so the location information is crucial for me to find out where the cursor is located in the AST so that I can implement features such as info on hover, auto-complete, etc. |
There are two kinds of ASTs:
The purpose of desugared AST is to minimize the number of cases to handle in the interpreter, static analysis etc. For example it doesn't distinguish between: Here the desugared name expression is created: https://github.com/google/go-jsonnet/blob/v0.16.0/internal/program/desugarer.go#L77. The
Let me know if it's more clear now. |
That makes sense, thanks! In that case, I should probably work with the output of the internal function |
In some cases, the
ast.Node
instances (especially the literals) returned bySnippetToAST
don't haveLocRange
as seen in the following picture:In this case, it's a
ast.LiteralString
inside theName
ofast.DesugaredObjectField
. Is this expected?The text was updated successfully, but these errors were encountered: