Skip to content

Unify completion pos usage, fix presentation compiler crash in interpolation #19614

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

Merged
merged 9 commits into from
Feb 29, 2024
Merged

Unify completion pos usage, fix presentation compiler crash in interpolation #19614

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
7d1d325
Unify completion pos usage, fix presentation compiler crash in interp…
rochala Feb 5, 2024
12e7d4a
Remove leftovers
rochala Feb 5, 2024
2b009c5
Remove leftovers
rochala Feb 5, 2024
a6be0cf
Apply review suggestions
rochala Feb 5, 2024
ebe397c
apply review comments, remove unnecessary param
rochala Feb 10, 2024
f899864
Remove cases which are impossible to match
rochala Feb 13, 2024
9f630b7
Remove unnecessary cases, fix CI, remove never used files
rochala Feb 15, 2024
d9c6efb
Revert changes to completion mode for renamed imports
rochala Feb 28, 2024
27acfd9
Add completions for given types
rochala Feb 28, 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
57 changes: 28 additions & 29 deletions compiler/src/dotty/tools/dotc/interactive/Completion.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,29 +90,22 @@ object Completion:

val completionSymbolKind: Mode =
path match
case untpd.Ident(_) :: untpd.Import(_, _) :: _ => Mode.ImportOrExport
case untpd.Ident(_) :: (_: untpd.ImportSelector) :: _ => Mode.ImportOrExport
case untpd.Literal(Constants.Constant(_: String)) :: _ => Mode.Term // literal completions
case GenericImportSelector(sel) =>
if sel.imported.span.contains(pos.span) then Mode.ImportOrExport // import scala.@@
else if sel.isGiven && sel.bound.span.contains(pos.span) then Mode.ImportOrExport
else Mode.None // import scala.{util => u@@}
case GenericImportOrExport(_) => Mode.ImportOrExport | Mode.Scope // import TrieMa@@
case untpd.Literal(Constants.Constant(_: String)) :: _ => Mode.Term | Mode.Scope // literal completions
case (ref: untpd.RefTree) :: _ =>
if (ref.name.isTermName) Mode.Term
else if (ref.name.isTypeName) Mode.Type
else Mode.None
val maybeSelectMembers = if ref.isInstanceOf[untpd.Select] then Mode.Member else Mode.Scope

case (sel: untpd.ImportSelector) :: _ =>
if sel.imported.span.contains(pos.span) then Mode.ImportOrExport
Copy link
Contributor

Choose a reason for hiding this comment

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

Why remove this check?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actually I have to revert it. I was tricked by our tests, as they don't enter completions in this position for some reason. We should probably also add check to not compute any completions when mode is None...

else Mode.None // Can't help completing the renaming
if (ref.name.isTermName) Mode.Term | maybeSelectMembers
else if (ref.name.isTypeName) Mode.Type | maybeSelectMembers
else Mode.None

case (_: untpd.ImportOrExport) :: _ => Mode.ImportOrExport
case _ => Mode.None

val completionKind: Mode =
path match
case Nil | (_: untpd.PackageDef) :: _ => Mode.None
case untpd.Ident(_) :: (_: untpd.ImportSelector) :: _ => Mode.Member
case (_: untpd.Select) :: _ => Mode.Member
case _ => Mode.Scope

completionSymbolKind | completionKind
completionSymbolKind

/** When dealing with <errors> in varios palces we check to see if they are
* due to incomplete backticks. If so, we ensure we get the full prefix
Expand Down Expand Up @@ -141,18 +134,11 @@ object Completion:
i + 1

path match
case (sel: untpd.ImportSelector) :: _ =>
completionPrefix(sel.imported :: Nil, pos)

case untpd.Ident(_) :: (sel: untpd.ImportSelector) :: _ if !sel.isGiven =>
if sel.isWildcard then pos.source.content()(pos.point - 1).toString
case GenericImportSelector(sel) =>
if sel.isGiven then completionPrefix(sel.bound :: Nil, pos)
else if sel.isWildcard then pos.source.content()(pos.point - 1).toString
else completionPrefix(sel.imported :: Nil, pos)

case (tree: untpd.ImportOrExport) :: _ =>
tree.selectors.find(_.span.contains(pos.span)).map: selector =>
completionPrefix(selector :: Nil, pos)
.getOrElse("")

// Foo.`se<TAB> will result in Select(Ident(Foo), <error>)
case (select: untpd.Select) :: _ if select.name == nme.ERROR =>
checkBacktickPrefix(select.source.content(), select.nameSpan.start, select.span.end)
Expand All @@ -169,6 +155,20 @@ object Completion:

end completionPrefix

private object GenericImportSelector:
def unapply(path: List[untpd.Tree]): Option[untpd.ImportSelector] =
path match
case untpd.Ident(_) :: (sel: untpd.ImportSelector) :: _ => Some(sel)
case (sel: untpd.ImportSelector) :: _ => Some(sel)
case _ => None

private object GenericImportOrExport:
def unapply(path: List[untpd.Tree]): Option[untpd.ImportOrExport] =
path match
case untpd.Ident(_) :: (importOrExport: untpd.ImportOrExport) :: _ => Some(importOrExport)
case (importOrExport: untpd.ImportOrExport) :: _ => Some(importOrExport)
case _ => None

/** Inspect `path` to determine the offset where the completion result should be inserted. */
def completionOffset(untpdPath: List[untpd.Tree]): Int =
untpdPath match
Expand Down Expand Up @@ -211,7 +211,6 @@ object Completion:
case tpd.Select(qual, _) :: _ if qual.typeOpt.hasSimpleKind => completer.selectionCompletions(qual)
case tpd.Select(qual, _) :: _ => Map.empty
case (tree: tpd.ImportOrExport) :: _ => completer.directMemberCompletions(tree.expr)
Copy link
Member

Choose a reason for hiding this comment

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

Is this case needed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, those are typed Trees, and this is the head of the path. We need it to find package members

case (_: untpd.ImportSelector) :: tpd.Import(expr, _) :: _ => completer.directMemberCompletions(expr)
case _ => completer.scopeCompletions

interactiv.println(i"""completion info with pos = $pos,
Expand Down
130 changes: 0 additions & 130 deletions compiler/test/dotty/tools/dotc/interactive/CustomCompletion.scala
View file
Open in desktop

This file was deleted.

171 changes: 0 additions & 171 deletions compiler/test/dotty/tools/dotc/interactive/CustomCompletionTests.scala
View file
Open in desktop

This file was deleted.

Loading