Skip to content
This repository has been archived by the owner on Mar 8, 2020. It is now read-only.

Operator Role missing position information #65

Closed
zurk opened this issue Mar 8, 2019 · 5 comments
Closed

Operator Role missing position information #65

zurk opened this issue Mar 8, 2019 · 5 comments
Assignees
Milestone

Comments

@zurk
Copy link

zurk commented Mar 8, 2019

Run:

echo "var a = 0 + 1;" > o.js
docker run -d --rm --name style_analyzer_bblfshd_new --privileged -p 9432:9432 bblfsh/bblfshd\:v2.11.0 --log-level DEBUG
docker exec style_analyzer_bblfshd_new bblfshctl driver install javascript docker://bblfsh/javascript-driver:v2.6.0
docker cp o.js style_analyzer_bblfshd_new:o.js
docker exec -it style_analyzer_bblfshd_new bblfshctl parse o.js

You get next UAST: https://gist.github.com/zurk/b594c7642a1bdfeca334ab27a2394426
And operator tokens are missing their positions:

.  .  .  .  .  .  .  .  .  .  0: Operator {
.  .  .  .  .  .  .  .  .  .  .  Roles: Arithmetic,Add,Expression,Operator,Binary
.  .  .  .  .  .  .  .  .  .  .  TOKEN "+"
.  .  .  .  .  .  .  .  .  .  .  Properties: {
.  .  .  .  .  .  .  .  .  .  .  .  internalRole: operator
.  .  .  .  .  .  .  .  .  .  .  }
.  .  .  .  .  .  .  .  .  .  }

And for driver v.1.2.0 I do not see this node. So I think it was added inside bblfsh, but without position information. It should be easy to include positions because you have positions for the right and left part of the expression.

@bzz
Copy link
Contributor

bzz commented Mar 11, 2019

Removing the assignment as it's used to communicated that something "has been take" and is kind-of-WIP but I'm not looking into this yet until the initial scope of https://github.com/bblfsh/javascript-driver/milestone/1 is finished.

Will get back to this ASAP right after though.

@bzz bzz changed the title [bug] Operator Role missing position information for v2.6.0 driver Operator Role missing position information Mar 11, 2019
@bzz bzz added the question label Mar 11, 2019
@bzz bzz added this to the v2.7.1 milestone Mar 12, 2019
@bzz bzz self-assigned this Mar 20, 2019
@bzz
Copy link
Contributor

bzz commented Mar 20, 2019

Indeed, some drivers do not have position at the operator node of the binary expression in annotated UAST: js, java. Others do: cpp.

In some language native ASTs, there is no position information for a node representing such operation e.g InfixExpression in java does not have it.

In others, like js and cpp, there is a position in native AST.

Thus I'll suggest to treat this issue like a bug in js driver (native AST has position, UAST does not).

But we also would need to have a broader discussion \w @creachadair and @dennwc on user expectations and a way of dealing with the same question e.g for a java-driver and alike, where native AST does not have this position, => UAST does not as well.

@bzz bzz added bug and removed question labels Mar 20, 2019
@bzz
Copy link
Contributor

bzz commented Mar 20, 2019

After digging deeper, I could not find a driver that have a position information for '@type': "uast:Operator"

So I would assume it's a wontfix for JS, but would ask @dennwc to verify that first.

@bzz bzz added question and removed bug labels Mar 20, 2019
@dennwc
Copy link
Member

dennwc commented Mar 20, 2019

Correct, I think it's "won't fix" for now. We need to change the tokenizer for this to work.

@zurk
Copy link
Author

zurk commented Mar 20, 2019

Ok, it is not critical, we have a workaround.

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

No branches or pull requests

3 participants