Skip to content

Fix #4498: Go to definition in lifted expressions #4516

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

Merged
merged 1 commit into from
May 11, 2018

Conversation

Duhemm
Copy link
Contributor

@Duhemm Duhemm commented May 11, 2018

Consider the following snippet:

val lst = new A :: Nil

This is rewritten as:

val lst = {
  val x$0 = new A
  Nil.::(x$0)
}

During lifting, we need to assign positions to the trees. We were
assigning the position of new A in the original code to x$0 in
Nil.::(x$0), and the original position of new A to val x$0 = new A
with a zero-extent.

Still considering this snippet, this means that when trying to go to the
definition on new A, the IDE would think that the user tried to go to
the definition of x$0, because the IDE would look for the matching
tree by position.

This commit changes this so that the lifted definition
(val x$0 = new A) gets the position of the original expression, and
the reference gets a synthetic, zero-extent position. With this change,
the IDE correctly finds the right tree when selecting new A in the
original code.

Fixes #4498

Consider the following snippet:

```
val lst = new A :: Nil
```

This is rewritten as:

```
val lst = {
  val x$0 = new A
  Nil.::(x$0)
}
```

During lifting, we need to assign positions to the trees. We were
assigning the position of `new A` in the original code to `x$0` in
`Nil.::(x$0)`, and the original position of `new A` to `val x$0 = new A`
with a zero-extent.

Still considering this snippet, this means that when trying to go to the
definition on `new A`, the IDE would think that the user tried to go to
the definition of `x$0`, because the IDE would look for the matching
tree by position.

This commit changes this so that the lifted definition
(`val x$0 = new A`) gets the position of the original expression, and
the reference gets a synthetic, zero-extent position. With this change,
the IDE correctly finds the right tree when selecting `new A` in the
original code.

Fixes scala#4498
@Duhemm Duhemm requested a review from smarter May 11, 2018 14:14
@Duhemm
Copy link
Contributor Author

Duhemm commented May 11, 2018

@smarter I think you're the positions expert, can you take a look at this patch, please?

@smarter
Copy link
Member

smarter commented May 11, 2018

Still considering this snippet, this means that when trying to go to the
definition on new A, the IDE would think that the user tried to go to
the definition of x$0

Interesting, I didn't think of this. We should check what happens when going to definition on accessors created by the Inliner because I used a similar pattern there: 87ffb88

@odersky odersky merged commit 2136fc1 into scala:master May 11, 2018
@allanrenucci allanrenucci deleted the fix/4498 branch May 11, 2018 20:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants