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.

Sign up for GitHub

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

Jump to bottom

Weekly metals backport #20542

Merged
merged 6 commits into from
Jun 13, 2024
Merged

Weekly metals backport #20542

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
29dc892
fix: insert missing members in correct place for case classes
kasiaMarek Jun 10, 2024
d389999
fix: show implicit param when it is an apply
kasiaMarek Jun 10, 2024
284e7cc
fix: handle implicit params in extract method
kasiaMarek Jun 10, 2024
a4c7e4a
fix: correctly auto import when there is a renamed symbol with the sa…
kasiaMarek Jun 10, 2024
a782184
adjust test
kasiaMarek Jun 10, 2024
d31aab8
improvement: sort abstract members for auto implement
kasiaMarek Jun 13, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 16 additions & 6 deletions presentation-compiler/src/main/dotty/tools/pc/ExtractMethodProvider.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import dotty.tools.dotc.ast.Trees.*
import dotty.tools.dotc.ast.tpd
import dotty.tools.dotc.ast.tpd.DeepFolder
import dotty.tools.dotc.core.Contexts.*
import dotty.tools.dotc.core.Flags
import dotty.tools.dotc.core.Symbols.Symbol
import dotty.tools.dotc.core.Types.MethodType
import dotty.tools.dotc.core.Types.PolyType
Expand Down Expand Up @@ -116,9 +117,15 @@ final class ExtractMethodProvider(
typeParams.toList.sortBy(_.decodedName),
)
end localRefs
val optEnclosing =
path.dropWhile(src => !src.sourcePos.encloses(range)) match
case Nil => None
case _ :: (app @ Apply(fun, args)) :: _ if args.exists(ImplicitParameters.isSyntheticArg(_)) => Some(app)
case found :: _ => Some(found)

val edits =
for
enclosing <- path.find(src => src.sourcePos.encloses(range))
enclosing <- optEnclosing
extracted = extractFromBlock(enclosing)
head <- extracted.headOption
expr <- extracted.lastOption
Expand All @@ -131,11 +138,14 @@ final class ExtractMethodProvider(
val exprType = prettyPrint(expr.typeOpt.widen)
val name =
genName(indexedCtx.scopeSymbols.map(_.decodedName).toSet, "newMethod")
val (methodParams, typeParams) =
val (allMethodParams, typeParams) =
localRefs(extracted, stat.sourcePos, extractedPos)
val methodParamsText = methodParams
.map(sym => s"${sym.decodedName}: ${prettyPrint(sym.info)}")
.mkString(", ")
val (methodParams, implicitParams) = allMethodParams.partition(!_.isOneOf(Flags.GivenOrImplicit))
def toParamText(params: List[Symbol]) =
params.map(sym => s"${sym.decodedName}: ${prettyPrint(sym.info)}")
.mkString(", ")
val methodParamsText = toParamText(methodParams)
val implicitParamsText = if implicitParams.nonEmpty then s"(given ${toParamText(implicitParams)})" else ""
val typeParamsText = typeParams
.map(_.decodedName) match
case Nil => ""
Expand All @@ -155,7 +165,7 @@ final class ExtractMethodProvider(
if noIndent && extracted.length > 1 then (" {", s"$newIndent}")
else ("", "")
val defText =
s"def $name$typeParamsText($methodParamsText): $exprType =$obracket\n${toExtract}\n$cbracket\n$newIndent"
s"def $name$typeParamsText($methodParamsText)$implicitParamsText: $exprType =$obracket\n${toExtract}\n$cbracket\n$newIndent"
val replacedText = s"$name($exprParamsText)"
List(
new l.TextEdit(
Expand Down
4 changes: 2 additions & 2 deletions presentation-compiler/src/main/dotty/tools/pc/IndexedContext.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,8 @@ sealed trait IndexedContext:
Result.InScope
// when all the conflicting symbols came from an old version of the file
case Some(symbols) if symbols.nonEmpty && symbols.forall(_.isStale) => Result.Missing
case Some(_) => Result.Conflict
case None => Result.Missing
case Some(symbols) if symbols.exists(rename(_).isEmpty) => Result.Conflict
case _ => Result.Missing
end lookupSym

/**
Expand Down
16 changes: 10 additions & 6 deletions presentation-compiler/src/main/dotty/tools/pc/PcInlayHintsProvider.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@ package dotty.tools.pc

import java.nio.file.Paths

import scala.annotation.tailrec

import scala.meta.internal.metals.ReportContext
import dotty.tools.pc.utils.InteractiveEnrichments.*
import dotty.tools.pc.printer.ShortenedTypePrinter
Expand Down Expand Up @@ -194,10 +196,10 @@ object ImplicitConversion:
def unapply(tree: Tree)(using params: InlayHintsParams, ctx: Context) =
if (params.implicitConversions()) {
tree match
case Apply(fun: Ident, args) if isSynthetic(fun) =>
case Apply(fun: Ident, args) if isSynthetic(fun) && args.exists(!_.span.isZeroExtent) =>
implicitConversion(fun, args)
case Apply(Select(fun, name), args)
if name == nme.apply && isSynthetic(fun) =>
if name == nme.apply && isSynthetic(fun) && args.exists(!_.span.isZeroExtent) =>
implicitConversion(fun, args)
case _ => None
} else None
Expand All @@ -218,7 +220,7 @@ object ImplicitParameters:
if (params.implicitParameters()) {
tree match
case Apply(fun, args)
if args.exists(isSyntheticArg) && !tree.sourcePos.span.isZeroExtent =>
if args.exists(isSyntheticArg) && !tree.sourcePos.span.isZeroExtent && !args.exists(isQuotes(_)) =>
val (implicitArgs, providedArgs) = args.partition(isSyntheticArg)
val allImplicit = providedArgs.isEmpty || providedArgs.forall {
case Ident(name) => name == nme.MISSING
Expand All @@ -229,10 +231,12 @@ object ImplicitParameters:
case _ => None
} else None

private def isSyntheticArg(tree: Tree)(using Context) = tree match
@tailrec
def isSyntheticArg(tree: Tree)(using Context): Boolean = tree match
case tree: Ident =>
tree.span.isSynthetic && tree.symbol.isOneOf(Flags.GivenOrImplicit) &&
!isQuotes(tree)
tree.span.isSynthetic && tree.symbol.isOneOf(Flags.GivenOrImplicit)
case Apply(fun, _ ) if tree.span.isZeroExtent => isSyntheticArg(fun)
case TypeApply(fun, _ ) if tree.span.isZeroExtent => isSyntheticArg(fun)
case _ => false

// Decorations for Quotes are rarely useful
Expand Down
11 changes: 10 additions & 1 deletion presentation-compiler/src/main/dotty/tools/pc/completions/OverrideCompletions.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -279,7 +279,14 @@ object OverrideCompletions:
else ""
(indent, indent, lastIndent)
end calcIndent
val abstractMembers = defn.typeOpt.abstractTermMembers.map(_.symbol)
val abstractMembers =
defn.tpe.abstractTermMembers.map(_.symbol).groupBy(_.owner).map {
case (owner, members) => (owner, members.sortWith{ (sym1, sym2) =>
if(sym1.sourcePos.exists && sym2.sourcePos.exists)
sym1.sourcePos.start <= sym2.sourcePos.start
else !sym2.sourcePos.exists
})
}.toSeq.sortBy(_._1.name.decoded).flatMap(_._2)

val caseClassOwners = Set("Product", "Equals")
val overridables =
Expand Down Expand Up @@ -506,6 +513,8 @@ object OverrideCompletions:
defn match
case td: TypeDef if text.charAt(td.rhs.span.end) == ':' =>
Some(td.rhs.span.end)
case TypeDef(_, temp : Template) =>
temp.parentsOrDerived.lastOption.map(_.span.end).filter(text.charAt(_) == ':')
case _ => None

private def fallbackFromParent(parent: Tree, name: String)(using Context) =
Expand Down
31 changes: 31 additions & 0 deletions presentation-compiler/test/dotty/tools/pc/tests/completion/CompletionSuite.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1952,3 +1952,34 @@ class CompletionSuite extends BaseCompletionSuite:
"""TestEnum test
|""".stripMargin,
)

@Test def `i6477-1` =
checkEdit(
"""|package a
|import a.b.SomeClass as SC
|
|package b {
| class SomeClass
|}
|package c {
| class SomeClass
|}
|
|val bar: SC = ???
|val foo: SomeClass@@
|""".stripMargin,
"""|package a
|import a.b.SomeClass as SC
|import a.c.SomeClass
|
|package b {
| class SomeClass
|}
|package c {
| class SomeClass
|}
|
|val bar: SC = ???
|val foo: SomeClass
|""".stripMargin,
)
37 changes: 31 additions & 6 deletions presentation-compiler/test/dotty/tools/pc/tests/edit/AutoImplementAbstractMembersSuite.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -345,10 +345,10 @@ class AutoImplementAbstractMembersSuite extends BaseCodeActionSuite:
|object Main {
| class Baz extends Bar {
|
| override def foo: Int = ???
|
| override def bar: Int = ???
|
| override def foo: Int = ???
|
| }
|}
|""".stripMargin
Expand Down Expand Up @@ -1243,7 +1243,6 @@ class AutoImplementAbstractMembersSuite extends BaseCodeActionSuite:
|
|object A {
| trait Base:
| def foo(x: Int): Int
| def bar(x: String): String
|
| class <<Concrete>>(x: Int, y: String) extends Base:
Expand All @@ -1256,13 +1255,10 @@ class AutoImplementAbstractMembersSuite extends BaseCodeActionSuite:
|
|object A {
| trait Base:
| def foo(x: Int): Int
| def bar(x: String): String
|
| class Concrete(x: Int, y: String) extends Base:
|
| override def foo(x: Int): Int = ???
|
| override def bar(x: String): String = ???
|
|
Expand All @@ -1272,6 +1268,35 @@ class AutoImplementAbstractMembersSuite extends BaseCodeActionSuite:
|""".stripMargin,
)

@Test def `braceless-case-class` =
checkEdit(
"""|package a
|
|trait Base:
| def foo(x: Int): Int
| def bar(x: String): String
|
|case class <<Concrete>>() extends Base:
| def aaa = "aaa"
|end Concrete
|""".stripMargin,
"""|package a
|
|trait Base:
| def foo(x: Int): Int
| def bar(x: String): String
|
|case class Concrete() extends Base:
|
| override def foo(x: Int): Int = ???
|
| override def bar(x: String): String = ???
|
| def aaa = "aaa"
|end Concrete
|""".stripMargin
)

def checkEdit(
original: String,
expected: String
Expand Down
31 changes: 31 additions & 0 deletions presentation-compiler/test/dotty/tools/pc/tests/edit/AutoImportsSuite.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -405,6 +405,37 @@ class AutoImportsSuite extends BaseAutoImportsSuite:
|""".stripMargin,
)

@Test def `i6477` =
checkEdit(
"""|package a
|import a.b.SomeClass as SC
|
|package b {
| class SomeClass
|}
|package c {
| class SomeClass
|}
|
|val bar: SC = ???
|val foo: <<SomeClass>> = ???
|""".stripMargin,
"""|package a
|import a.b.SomeClass as SC
|import a.c.SomeClass
|
|package b {
| class SomeClass
|}
|package c {
| class SomeClass
|}
|
|val bar: SC = ???
|val foo: SomeClass = ???
|""".stripMargin
)

private def ammoniteWrapper(code: String): String =
// Vaguely looks like a scala file that Ammonite generates
// from a sc file.
Expand Down
92 changes: 92 additions & 0 deletions presentation-compiler/test/dotty/tools/pc/tests/edit/ExtractMethodSuite.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -446,3 +446,95 @@ class ExtractMethodSuite extends BaseExtractMethodSuite:
| }
|}""".stripMargin
)

@Test def `i6476` =
checkEdit(
"""|object O {
| class C
| def foo(i: Int)(implicit o: C) = i
|
| @@val o = {
| implicit val c = new C
| <<foo(2)>>
| ???
| }
|}
|""".stripMargin,
"""|object O {
| class C
| def foo(i: Int)(implicit o: C) = i
|
| def newMethod()(given c: C): Int =
| foo(2)
|
| val o = {
| implicit val c = new C
| newMethod()
| ???
| }
|}
|""".stripMargin
)


@Test def `i6476-2` =
checkEdit(
"""|object O {
| class C
| def foo(i: Int)(implicit o: C) = i
|
| @@val o = {
| <<foo(2)(new C)>>
| ???
| }
|}
|""".stripMargin,
"""|object O {
| class C
| def foo(i: Int)(implicit o: C) = i
|
| def newMethod(): Int =
| foo(2)(new C)
|
| val o = {
| newMethod()
| ???
| }
|}
|""".stripMargin
)

@Test def `i6476-3` =
checkEdit(
"""|object O {
| class C
| class D
| def foo(i: Int)(using o: C)(x: Int)(using d: D) = i
|
| @@val o = {
| given C = new C
| given D = new D
| val w = 2
| <<foo(w)(w)>>
| ???
| }
|}
|""".stripMargin,
"""|object O {
| class C
| class D
| def foo(i: Int)(using o: C)(x: Int)(using d: D) = i
|
| def newMethod(w: Int)(given given_C: C, given_D: D): Int =
| foo(w)(w)
|
| val o = {
| given C = new C
| given D = new D
| val w = 2
| newMethod(w)
| ???
| }
|}
|""".stripMargin
)
Loading
Loading