fix: handle backticked names in inffered-type #3791
Conversation
dos65
force-pushed
the
inferred_type_fix_backticks
branch
from
4b89643
to
9a39c5b
Compare
There was a problem hiding this comment.
LGTM!
Maybe we can backport this namePos strategy (in Scala3) to Interactive#namePos https://github.com/lampepfl/dotty/blob/73cda0c9ad9a23132dabc8f8a7a47663da3ee3fa/compiler/src/dotty/tools/dotc/interactive/SourceTree.scala#L34-L36 ?
| tree.source.atSpan(Span(start, end, start)) | ||
| case None => | ||
| val ch = text.charAt(idx) | ||
| if ch == realName.head then |
There was a problem hiding this comment.
Maybe it's too cautious, but can we use headOption to avoid NoSuchElementException here?
There was a problem hiding this comment.
Is it possible to get zero length name?
There was a problem hiding this comment.
I don't think it's possible, we could add a require with a proper message just in case
|
|
||
| def lookup( | ||
| idx: Int, | ||
| start: Option[(Int, List[Char])], |
There was a problem hiding this comment.
[note]
A pair of next position and trailing realName
There was a problem hiding this comment.
Maybe forge this note into code?
case class Lookup(position: Int, tralingName: List[Char])
| private def namePos(text: String, tree: untpd.NameTree)(using | ||
| Context | ||
| ): SourcePosition = | ||
| val realName = tree.name.stripModuleClassSuffix.lastPart.toString.toList |
There was a problem hiding this comment.
Could you provide some example how tree.name.stripModuleClassSuffix and tree.name.stripModuleClassSuffix.lastPart differs from each other?
There was a problem hiding this comment.
@kpodsiad I don't know. I just copied this line from from compiler impl for namePos
There was a problem hiding this comment.
Ok, adding info that this line from from compiler impl for namePos will also be helpful for future us.
There was a problem hiding this comment.
ok, just removed lastPart call. Tests pass without it.
| val end = if withBacktick then idx + 1 else idx | ||
| tree.source.atSpan(Span(start, end, start)) | ||
| case None => | ||
| val ch = text.charAt(idx) |
There was a problem hiding this comment.
Should we guard idx with, for example, tree.sourcePos.end?
There was a problem hiding this comment.
case None if idx < text.length should probably suffice
I thought about that. Honestly, I don't like the approach in this pr but there is no other ways to fix it. |
| tree.source.atSpan(Span(start, end, start)) | ||
| case None => | ||
| val ch = text.charAt(idx) | ||
| if ch == realName.head then |
There was a problem hiding this comment.
I don't think it's possible, we could add a require with a proper message just in case
| val end = if withBacktick then idx + 1 else idx | ||
| tree.source.atSpan(Span(start, end, start)) | ||
| case None => | ||
| val ch = text.charAt(idx) |
There was a problem hiding this comment.
case None if idx < text.length should probably suffice
| ): SourcePosition = | ||
| val realName = tree.name.stripModuleClassSuffix.lastPart.toString.toList | ||
|
|
||
| def lookup( |
dos65
force-pushed
the
inferred_type_fix_backticks
branch
from
9a39c5b
to
f03d20e
Compare
dos65
force-pushed
the
inferred_type_fix_backticks
branch
from
f03d20e
to
8427aeb
Compare
Fixes #3785