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

Fix #1104 #1105

Merged
merged 3 commits into from
May 3, 2020
Merged

Fix #1104 #1105

merged 3 commits into from
May 3, 2020

Conversation

giabao
Copy link
Contributor

@giabao giabao commented Apr 24, 2020

Fix #1104

Copy link
Contributor

@olafurpg olafurpg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for this contribution! Is there a relationship between this change and scalameta/scalameta#2043 ?

@ohze
Copy link

ohze commented May 3, 2020

Given:

0 trait SymbolTest {
1   def shouldBe(right: Any)
2   def arg = 1
3   this shouldBe (arg)
4 }                  
                 16   21

We use textDocument.occurrences produced by semanticdb-scalac to get symbols (String, eg (arg)) from a Range/ Position (eg, [3:16..3.21))

To workaround for scalameta/scalameta#1083, when get Position for name (arg), instead of returning [3:16..3.21), we return [3:17..3.20) and use that Position to lookup symbols from occurrences.

But in textDocument.occurrences produced by semanticdb-scalac, there is only SymbolOccurrence(range = [3:16..3.21), symbol = "(arg)") (have no Occurrence with range == [3:17..3.20)). So the lookup will return None.

Code changes in this PR: Only re-try getting symbol from range [3:16..3.21) when workingaround scalameta/scalameta#1083.

@ohze
Copy link

ohze commented May 3, 2020

Is there a relationship between this change and scalameta/scalameta#2043 ?

No. The bug resolved in this PR have no relation to implicit conversion as in scalameta/scalameta#2043.

Given:

trait AnyShouldWrapper {
  def shouldBe(right: Any)
}
trait SymbolTest {
  implicit def convert(o: Any)(implicit foo: Int): AnyShouldWrapper
  implicit val foo: Int = 1
  def arg = 1
  this shouldBe (arg)
}

In the last statement this shouldBe (arg):

Copy link
Contributor

@olafurpg olafurpg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍 Thank you for explaining!

@olafurpg olafurpg merged commit 990380c into scalacenter:master May 3, 2020
@olafurpg
Copy link
Contributor

olafurpg commented May 3, 2020

Please let me know if you're blocked by this fixed and need a new release. I would prefer to wait a bit longer to cut a new release since it always takes a while to synchronize scalafix-core/sbt-scalafix

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.

Can't get symbol from TextDocument for arg of ApplyInfix which have parens around
3 participants