Skip to content

Draft: additional completions for using clause #23647

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

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft
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,6 +56,7 @@ case class CompletionAffix(
private def loopPrefix(prefixes: List[PrefixKind]): String =
prefixes match
case PrefixKind.New :: tail => "new " + loopPrefix(tail)
case PrefixKind.Using :: tail => "using " + loopPrefix(tail)
case _ => ""

/**
Expand Down Expand Up @@ -87,7 +88,7 @@ enum SuffixKind:
case Brace, Bracket, Template, NoSuffix

enum PrefixKind:
case New
case New, Using

type Suffix = Affix[SuffixKind]
type Prefix = Affix[PrefixKind]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -193,13 +193,23 @@ class Completions(
)
end isAbstractType

private def findSuffix(symbol: Symbol): CompletionAffix =
private def findSuffix(symbol: Symbol, adjustedPath: List[untpd.Tree]): CompletionAffix =
CompletionAffix.empty
.chain { suffix => // for [] suffix
if shouldAddSuffix && symbol.info.typeParams.nonEmpty then
suffix.withNewSuffixSnippet(Affix(SuffixKind.Bracket))
else suffix
}
.chain{ suffix =>
adjustedPath match
case (ident: Ident) :: (app@Apply(_,args)) :: _ if args.size == 1 =>
app.symbol.info match
case mt@MethodType(termNames) if app.symbol.paramSymss.last.exists(_.is(Given)) =>
suffix.withNewPrefix(Affix(PrefixKind.Using))
case _ => suffix
case _ => suffix

}
.chain { suffix => // for () suffix
if shouldAddSuffix && symbol.is(Flags.Method) then
val paramss = getParams(symbol)
Expand Down Expand Up @@ -271,7 +281,7 @@ class Completions(
val existsApply = extraMethodDenots.exists(_.symbol.name == nme.apply)

extraMethodDenots.map { methodDenot =>
val suffix = findSuffix(methodDenot.symbol)
val suffix = findSuffix(methodDenot.symbol, adjustedPath)
val affix = if methodDenot.symbol.isConstructor && existsApply then
adjustedPath match
case (select @ Select(qual, _)) :: _ =>
Expand All @@ -293,7 +303,7 @@ class Completions(

if skipOriginalDenot then extraCompletionValues
else
val suffix = findSuffix(denot.symbol)
val suffix = findSuffix(denot.symbol, adjustedPath)
val name = undoBacktick(label)
val denotCompletionValue = toCompletionValue(name, denot, suffix)
denotCompletionValue :: extraCompletionValues
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -238,6 +238,65 @@ class CompletionArgSuite extends BaseCompletionSuite:
""
)

@Test def `using` =
checkEdit(
s"""|def hello(using String): Unit = ???
|@main def main1(): Unit =
| val str = "hello"
| hello(st@@)
|""".stripMargin,
s"""|def hello(using String): Unit = ???
|@main def main1(): Unit =
| val str = "hello"
| hello(using str)
|""".stripMargin,
assertSingleItem = false)

@Test def `using2` =
checkEdit(
s"""|def hello(using String): Unit = ???
|@main def main1(): Unit =
| val str = "hello"
| hello(using st@@)
|""".stripMargin,
s"""|def hello(using String): Unit = ???
|@main def main1(): Unit =
| val str = "hello"
| hello(using str)
Copy link
Contributor

Choose a reason for hiding this comment

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

Looks like this one is adding two usings still, do you need help figuring this one out?

Copy link
Author

Choose a reason for hiding this comment

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

yup some hints will be welcomed :). As far as I understand it the completion check is done from method invocation level, but I'm not sure how could I check the actual code or find out if there already i s'using' clause in any other way.

|""".stripMargin,
assertSingleItem = false)

@Test def `using3` =
checkEdit(
s"""|def hello(using String, Int): Unit = ???
|@main def main1(): Unit =
| val str = "hello"
| val int = 4
| hello(str, in@@)
|""".stripMargin,
s"""|def hello(using String, Int): Unit = ???
|@main def main1(): Unit =
| val str = "hello"
| val int = 4
| hello(using str, int)
Copy link
Contributor

Choose a reason for hiding this comment

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

Looks like this check is actually failing because we check if arguments.size == 1

I would leave this test out or just document the current behaviour.

Copy link
Author

Choose a reason for hiding this comment

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

yeah the test was written this way as this was one of the proposed solutions to do nothing when we have 2 arguments. When I remove that guard the using keyword will be before int arg anyway. I'm not sure should i leave it that way or try to fix it.

|""".stripMargin,
assertSingleItem = false)

@Test def `using4` =
checkEdit(
s"""|def hello(name: String)(using String): Unit = ???
|@main def main1(): Unit =
| val str = "hello"
| hello("name")(str@@)
|""".stripMargin,
s"""|def hello(name: String)(using String): Unit = ???
|@main def main1(): Unit =
| val str = "hello"
| hello("name")(using str)
|""".stripMargin,
assertSingleItem = false
)

@Test def `default-args` =
check(
s"""|object Main {
Expand Down
Loading