-
Notifications
You must be signed in to change notification settings - Fork 337
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
bugfix: workaround for missing span in reference to an extension method defined in the companion object #5640
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it can also close #5522
The idea looks good, but there is an issue when the method is not simply on ident
case sel: Select | ||
if sel.span.isZeroExtent && sel.symbol.is(Flags.ExtensionMethod) => | ||
scala.util | ||
.Try(NavigateAST.untypedPath(sel.span)(using compilatonUnitContext)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This won't work for case mentioned in the comment, because sel.span
points to @@MyInt(1).uneven
,
so path is Ident :: Apply(MyInt, 1) :: Select :: _
.
We can maybe take span from parent and use it? Also, I'm not sure if untyped will be then even needed here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Which comment? I get it now 😂 .
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The one above this case
class MyIntOut(val value: Int)
object MyIntOut:
extension (i: MyIntOut) def <<uneven>> = i.value % 2 == 1
val a = MyIntOut(1).un@@even
case (untypedSel: untpd.Select) :: _ => | ||
Some( | ||
symbolAlternatives(sel.symbol), | ||
pos.withSpan(untypedSel.nameSpan), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think for most cases, the span would be the Apply span, from point to end, so we won't have to use untypedPath, but I have to check how it will work for infix methods
tests/cross/src/test/scala/tests/highlight/Scala3DocumentHighlightSuite.scala
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would add a test for hover, besides LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! Does it also fix #5478 ?
I think for the presentation compiler, we should actually fix the span, so no need to port this to the compiler most likely. (just tests after the span fix is done) |
|
Och, that's amazing! |
Workaround for missing span in:
to provide correct:
#5630