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: handle backticked names in inffered-type #3791

Merged
merged 1 commit into from Apr 13, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Expand Up @@ -60,14 +60,20 @@ final class InferredTypeProvider(
def prettyType(tpe: Type) =
metalsToLongString(tpe.widen.finalResultType, history)

def findNameEnd(start: Int, name: Name): Int = {
// dropLocal will remove a space that might appear at the of a name in some places
val identLength = name.dropLocal.length
val backtickInc = if (params.text().charAt(start) == '`') 2 else 0
start + identLength + backtickInc
}

typedTree match {
/* `val a = 1` or `var b = 2`
* turns into
* `val a: Int = 1` or `var b: Int = 2`
*/
case vl @ ValDef(_, name, tpt, _) if !vl.symbol.isParameter =>
// dropLocal will remove a space that might appear at the of a name in some places
val nameEnd = tpt.pos.start + name.dropLocal.length()
val nameEnd = findNameEnd(tpt.pos.start, name)
val nameEndPos = tpt.pos.withEnd(nameEnd).withStart(nameEnd).toLSP
val typeNameEdit = new TextEdit(nameEndPos, ": " + prettyType(tpt.tpe))
typeNameEdit :: additionalImports
Expand All @@ -77,7 +83,7 @@ final class InferredTypeProvider(
* `.map((a: Int) => a + a)`
*/
case vl @ ValDef(_, name, tpt, _) if vl.symbol.isParameter =>
val nameEnd = vl.pos.start + name.length()
val nameEnd = findNameEnd(vl.pos.start, name)
val namePos = tpt.pos.withEnd(nameEnd).withStart(nameEnd).toLSP

def leftParenStart = vl.pos.withEnd(vl.pos.start).toLSP
Expand Down Expand Up @@ -120,7 +126,7 @@ final class InferredTypeProvider(
* `def a[T](param : Int): Int = param`
*/
case DefDef(_, name, _, _, tpt, rhs) =>
val nameEnd = tpt.pos.start + name.length()
val nameEnd = findNameEnd(tpt.pos.start, name)

// search for `)` or `]` or defaut to name's end to insert type
val searchString = params
Expand Down Expand Up @@ -148,7 +154,7 @@ final class InferredTypeProvider(
def openingParenPos = body.pos.withEnd(body.pos.start)
def openingParen = new TextEdit(openingParenPos.toLSP, "(")

val insertStart = bind.pos.start + name.length()
val insertStart = findNameEnd(bind.pos.start, name)
val insertPos = bind.pos.withEnd(insertStart).withStart(insertStart)

/* In case it's an infix pattern match
Expand Down
Expand Up @@ -17,13 +17,16 @@ import scala.meta.tokens.{Token as T}
import dotty.tools.dotc.ast.Trees.*
import dotty.tools.dotc.ast.untpd
import dotty.tools.dotc.core.Contexts.*
import dotty.tools.dotc.core.NameOps.*
import dotty.tools.dotc.core.Names.*
import dotty.tools.dotc.core.Symbols.*
import dotty.tools.dotc.core.Types.*
import dotty.tools.dotc.interactive.Interactive
import dotty.tools.dotc.interactive.InteractiveDriver
import dotty.tools.dotc.util.SourceFile
import dotty.tools.dotc.util.SourcePosition
import dotty.tools.dotc.util.Spans
import dotty.tools.dotc.util.Spans.Span
import org.eclipse.lsp4j.TextEdit

/**
Expand Down Expand Up @@ -121,9 +124,10 @@ final class InferredTypeProvider(
*/
case Some(vl @ ValDef(sym, tpt, _)) =>
val isParam = path.tail.headOption.exists(_.symbol.isAnonymousFunction)
def baseEdit(withParens: Boolean) =
def baseEdit(withParens: Boolean): TextEdit =
val endPos = findNamePos(params.text, vl).endPos
new TextEdit(
vl.namePos.endPos.toLSP,
endPos.toLSP,
": " + printType(tpt.tpe) + {
if withParens then ")" else ""
}
Expand All @@ -134,7 +138,7 @@ final class InferredTypeProvider(
toCheckFor: Char,
blockStartPos: SourcePosition
) =
val text = params.text.toString()
val text = params.text
val isParensFunction: Boolean = text(applyEndingPos) == toCheckFor

val alreadyHasParens =
Expand Down Expand Up @@ -251,4 +255,41 @@ final class InferredTypeProvider(
end match
end inferredTypeEdits

private def findNamePos(text: String, tree: untpd.NamedDefTree)(using
Context
): SourcePosition =
val realName = tree.name.stripModuleClassSuffix.toString.toList

// `NamedDefTree.namePos` is incorrect for bacticked names
@tailrec
def lookup(
Copy link
Contributor

Choose a reason for hiding this comment

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

Could it be @tailrec ?

idx: Int,
start: Option[(Int, List[Char])],
Copy link
Member

Choose a reason for hiding this comment

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

[note]
A pair of next position and trailing realName

Copy link
Member

Choose a reason for hiding this comment

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

Maybe forge this note into code?

case class Lookup(position: Int, tralingName: List[Char])

withBacktick: Boolean
): Option[SourcePosition] =
start match
case Some((start, nextMatch :: left)) =>
if text.charAt(idx) == nextMatch then
lookup(idx + 1, Some((start, left)), withBacktick)
else lookup(idx + 1, None, withBacktick = false)
case Some((start, Nil)) =>
val end = if withBacktick then idx + 1 else idx
val pos = tree.source.atSpan(Span(start, end, start))
Some(pos)
case None if idx < text.length =>
val ch = text.charAt(idx)
Copy link
Member

Choose a reason for hiding this comment

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

Should we guard idx with, for example, tree.sourcePos.end?

Copy link
Contributor

Choose a reason for hiding this comment

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

case None if idx < text.length should probably suffice

if ch == realName.head then
Copy link
Member

Choose a reason for hiding this comment

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

Maybe it's too cautious, but can we use headOption to avoid NoSuchElementException here?

Copy link
Member

Choose a reason for hiding this comment

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

Is it possible to get zero length name?

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think it's possible, we could add a require with a proper message just in case

Copy link
Member Author

Choose a reason for hiding this comment

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

added a check for that

lookup(idx + 1, Some((idx, realName.tail)), withBacktick)
else if ch == '`' then lookup(idx + 1, None, withBacktick = true)
else lookup(idx + 1, None, withBacktick = false)
case _ =>
None

val macthedByText =
if realName.nonEmpty then lookup(tree.sourcePos.start, None, false)
else None

macthedByText.getOrElse(tree.namePos)
end findNamePos

end InferredTypeProvider
33 changes: 33 additions & 0 deletions tests/cross/src/test/scala/tests/pc/InsertInferredTypeSuite.scala
Expand Up @@ -478,6 +478,39 @@ class InsertInferredTypeSuite extends BaseCodeActionSuite {
)
)

checkEdit(
"backticks-1",
"""|object O{
| val <<`bar`>> = 42
|}""".stripMargin,
"""|object O{
| val `bar`: Int = 42
|}
|""".stripMargin
)

checkEdit(
"backticks-2",
"""|object O{
| def <<`bar`>> = 42
|}""".stripMargin,
"""|object O{
| def `bar`: Int = 42
|}
|""".stripMargin
)

checkEdit(
"backticks-3",
"""|object O{
| List(1).map(<<`a`>> => a + 1)
|}""".stripMargin,
"""|object O{
| List(1).map((`a`: Int) => a + 1)
|}
|""".stripMargin
)

def checkEdit(
name: TestOptions,
original: String,
Expand Down