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

bugfix: Completions for named args in wrong order #18702

Merged
merged 1 commit into from
Nov 10, 2023
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -56,8 +56,8 @@ class Completions(

val coursierComplete = new CoursierComplete(BuildInfo.scalaVersion)

private lazy val adjustedPath = Completion.resolveTypedOrUntypedPath(path, pos)
private lazy val completionMode =
val adjustedPath = Completion.resolveTypedOrUntypedPath(path, pos)
val mode = Completion.completionMode(adjustedPath, pos)
path match
case Literal(Constant(_: String)) :: _ => Mode.Term // literal completions
Expand Down Expand Up @@ -451,6 +451,7 @@ class Completions(
val args = NamedArgCompletions.contribute(
pos,
path,
adjustedPath,
indexedContext,
config.isCompletionSnippetsEnabled()
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,10 @@ package dotty.tools.pc.completions

import scala.util.Try

import dotty.tools.dotc.ast.NavigateAST
import dotty.tools.dotc.ast.Trees.ValDef
import dotty.tools.dotc.ast.tpd.*
import dotty.tools.dotc.ast.untpd
import dotty.tools.dotc.core.Constants.Constant
import dotty.tools.dotc.core.ContextOps.localContext
import dotty.tools.dotc.core.Contexts.Context
Expand All @@ -12,7 +14,10 @@ import dotty.tools.dotc.core.Flags
import dotty.tools.dotc.core.Flags.Method
import dotty.tools.dotc.core.NameKinds.DefaultGetterName
import dotty.tools.dotc.core.Names.Name
import dotty.tools.dotc.core.StdNames.*
import dotty.tools.dotc.core.SymDenotations.NoDenotation
import dotty.tools.dotc.core.Symbols
import dotty.tools.dotc.core.Symbols.NoSymbol
import dotty.tools.dotc.core.Symbols.Symbol
import dotty.tools.dotc.core.Types.AndType
import dotty.tools.dotc.core.Types.AppliedType
Expand All @@ -33,8 +38,9 @@ object NamedArgCompletions:
def contribute(
pos: SourcePosition,
path: List[Tree],
untypedPath: => List[untpd.Tree],
indexedContext: IndexedContext,
clientSupportsSnippets: Boolean
clientSupportsSnippets: Boolean,
)(using ctx: Context): List[CompletionValue] =
path match
case (ident: Ident) :: ValDef(_, _, _) :: Block(_, app: Apply) :: _
Expand All @@ -43,7 +49,7 @@ object NamedArgCompletions:
Some(ident),
app,
indexedContext,
clientSupportsSnippets
clientSupportsSnippets,
)
case (ident: Ident) :: rest =>
def getApplyForContextFunctionParam(path: List[Tree]): Option[Apply] =
Expand All @@ -63,9 +69,29 @@ object NamedArgCompletions:
Some(ident),
app,
indexedContext,
clientSupportsSnippets
clientSupportsSnippets,
)
contribution.getOrElse(Nil)
case (app: Apply) :: _ =>
/**
* def foo(aaa: Int, bbb: Int, ccc: Int) = ???
* val x = foo(
* bbb = 123,
* ccc = 123,
* @@
* )
* In this case, typed path doesn't contain already provided arguments
*/
untypedPath match
case (ident: Ident) :: (app: Apply) :: _ =>
contribute(
Some(ident),
app,
indexedContext,
clientSupportsSnippets,
)
case _ =>
Nil
case _ =>
Nil
end match
Expand All @@ -87,7 +113,7 @@ object NamedArgCompletions:
ident: Option[Ident],
apply: Apply,
indexedContext: IndexedContext,
clientSupportsSnippets: Boolean
clientSupportsSnippets: Boolean,
)(using context: Context): List[CompletionValue] =
def isUselessLiteral(arg: Tree): Boolean =
arg match
Expand Down Expand Up @@ -117,6 +143,11 @@ object NamedArgCompletions:

val argss = collectArgss(apply)

def fallbackFindApply(sym: Symbol) =
sym.info.member(nme.apply) match
case NoDenotation => Nil
case den => List(den.symbol)

// fallback for when multiple overloaded methods match the supplied args
def fallbackFindMatchingMethods() =
def maybeNameAndIndexedContext(
Expand Down Expand Up @@ -182,7 +213,9 @@ object NamedArgCompletions:
if foundPotential.contains(method.symbol) then foundPotential
else method.symbol :: foundPotential
else List(method.symbol)
else fallbackFindMatchingMethods()
else if method.symbol.is(Method) || method.symbol == NoSymbol then
fallbackFindMatchingMethods()
else fallbackFindApply(method.symbol)
end if
end matchingMethods

Expand Down Expand Up @@ -227,8 +260,13 @@ object NamedArgCompletions:
def refineParams(method: Tree, level: Int): List[ParamSymbol] =
method match
case Select(Apply(f, _), _) => refineParams(f, level + 1)
case Select(h, v) => getRefinedParams(h.symbol.info, level)
case _ => defaultBaseParams
case Select(h, name) =>
// for Select(foo, name = apply) we want `foo.symbol`
if name == nme.apply then getRefinedParams(h.symbol.info, level)
else getRefinedParams(method.symbol.info, level)
case Apply(f, _) =>
refineParams(f, level + 1)
case _ => getRefinedParams(method.symbol.info, level)
refineParams(method, 0)
end baseParams

Expand Down Expand Up @@ -330,15 +368,15 @@ object NamedArgCompletions:
param.nameBackticked + " = " + memberName + " "
CompletionValue.namedArg(
label = editText,
param
param,
)
}
}

params.map(p =>
CompletionValue.namedArg(
s"${p.nameBackticked} = ",
p
p,
)
) ::: findPossibleDefaults() ::: fillAllFields()
end contribute
Expand Down Expand Up @@ -411,4 +449,4 @@ case class JustSymbol(symbol: Symbol)(using Context) extends ParamSymbol:
def info: Type = symbol.info

case class RefinedSymbol(symbol: Symbol, name: Name, info: Type)
extends ParamSymbol
extends ParamSymbol
Original file line number Diff line number Diff line change
Expand Up @@ -1038,3 +1038,71 @@ class CompletionArgSuite extends BaseCompletionSuite:
|""".stripMargin,
topLines = Some(1),
)

@Test def `second-first` =
check(
"""|object Main {
| def foo(aaa: Int, bbb: Int, ccc: Int) = aaa + bbb + ccc
| val k = foo (
| bbb = 123,
| aa@@
| )
|}
|""".stripMargin,
"""|aaa = : Int
|""".stripMargin,
topLines = Some(1),
)

@Test def `second-first2` =
check(
"""|object Main {
| def foo(aaa: Int, bbb: Int, ccc: Int) = aaa + bbb + ccc
| val k = foo (
| bbb = 123,
| ccc = 123,
| aa@@
| )
|}
|""".stripMargin,
"""|aaa = : Int
|""".stripMargin,
topLines = Some(1),
)

@Test def `second-first3` =
check(
"""|object Main {
| def foo(ddd: Int)(aaa: Int, bbb: Int, ccc: Int) = aaa + bbb + ccc
| val k = foo(123)(
| bbb = 123,
| ccc = 123,
| aa@@
| )
|}
|""".stripMargin,
"""|aaa = : Int
|""".stripMargin,
topLines = Some(1),
)

@Test def `second-first4` =
check(
"""|object O:
| val hello: (x: Int, y: Int) => Unit = (x, _) => println(x)
|val k = O.hello(y = 1, @@)
|""".stripMargin,
"""|x = : Int
|""".stripMargin,
topLines = Some(1),
)

@Test def `second-first5` =
check(
"""|val hello: (x: Int) => Int => (str: String, ccc: String) => Unit = x => j => (str, _) => println(str)
|val k = hello(x = 1)(2)(ccc = "abc", @@)
|""".stripMargin,
"""|str = : String
| """.stripMargin,
topLines = Some(1),
)
Loading