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

fix: colliding pattern and scope completions #7295

Merged
merged 1 commit into from
Mar 13, 2025
Merged

fix: colliding pattern and scope completions #7295

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
25 changes: 13 additions & 12 deletions mtags/src/main/scala-2/scala/meta/internal/pc/CompletionProvider.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -362,19 +362,20 @@ class CompletionProvider(
val isIgnored = mutable.Set.empty[Symbol]
val buf = List.newBuilder[Member]
def visit(head: Member): Boolean = {
val id =
if (head.sym.isClass || head.sym.isModule) {
head.sym.fullName
} else {
head match {
case o: OverrideDefMember =>
o.label
case named: NamedArgMember =>
s"named-${semanticdbSymbol(named.sym)}"
case _ =>
semanticdbSymbol(head.sym)
val id = head match {
case o: OverrideDefMember =>
o.label
case named: NamedArgMember =>
s"named-${semanticdbSymbol(named.sym)}"
case pattern: CasePatternMember =>
s"case-pattern-${semanticdbSymbol(pattern.sym)}"
case _ =>
if (head.sym.isClass || head.sym.isModule) {
head.sym.fullName
} else {
semanticdbSymbol(head.sym)
}
}
}

def isIgnoredWorkspace: Boolean =
head.isInstanceOf[WorkspaceMember] &&
Expand Down
10 changes: 6 additions & 4 deletions mtags/src/main/scala-2/scala/meta/internal/pc/MetalsGlobal.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -598,10 +598,12 @@ class MetalsGlobal(
case Descriptor.None =>
Nil
case Descriptor.Type(value) =>
val member = owner.info.decl(TypeName(value).encode) :: Nil
if (sym.isJava)
owner.info.decl(TermName(value).encode) :: member
else member
val members =
if (sym.isJava)
owner.info.decl(TermName(value).encode) :: Nil
else Nil
// Put the type ahead of the Java-induced term for `inverseSemanticdbSymbol`
owner.info.decl(TypeName(value).encode) :: members
case Descriptor.Term(value) =>
owner.info.decl(TermName(value).encode) :: Nil
case Descriptor.Package(value) =>
Expand Down
19 changes: 19 additions & 0 deletions mtags/src/main/scala-2/scala/meta/internal/pc/completions/Completions.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,25 @@ trait Completions { this: MetalsGlobal =>
val commitCharacter: Option[String] = None
) extends ScopeMember(sym, NoType, true, EmptyTree)

class CasePatternMember(
override val filterText: String,
override val edit: l.TextEdit,
sym: Symbol,
override val label: Option[String] = None,
override val detail: Option[String] = None,
override val command: Option[String] = None,
override val additionalTextEdits: List[l.TextEdit] = Nil
) extends TextEditMember(
filterText,
edit,
sym,
label,
detail,
command,
additionalTextEdits,
None
)

val packageSymbols: mutable.Map[String, Option[Symbol]] =
mutable.Map.empty[String, Option[Symbol]]
def packageSymbolFromString(symbol: String): Option[Symbol] =
Expand Down
44 changes: 28 additions & 16 deletions mtags/src/main/scala-2/scala/meta/internal/pc/completions/MatchCaseCompletions.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ trait MatchCaseCompletions { this: MetalsGlobal =>
}
override def isPrioritized(member: Member): Boolean =
member match {
case m: TextEditMember => !m.filterText.contains("(exhaustive)")
case m: CasePatternMember => !m.filterText.contains("(exhaustive)")
case _ => false
}

Expand All @@ -88,7 +88,7 @@ trait MatchCaseCompletions { this: MetalsGlobal =>
if (definitions.isTupleType(parents.selector)) {
if (patternOnly.isEmpty)
List(
new TextEditMember(
new CasePatternMember(
"case () =>",
new l.TextEdit(
editRange,
Expand Down Expand Up @@ -221,7 +221,7 @@ trait MatchCaseCompletions { this: MetalsGlobal =>

val detail =
s" ${metalsToLongString(selectorSym.tpe, new ShortenedNames())} (${sealedMembers.length} cases)"
val exhaustive = new TextEditMember(
val exhaustive = new CasePatternMember(
"case (exhaustive)",
newText,
selectorSym,
Expand Down Expand Up @@ -306,7 +306,7 @@ trait MatchCaseCompletions { this: MetalsGlobal =>
val sortedSubclasses = sortSubclasses(subclassesResult, tpe, source)

val members = sortedSubclasses.map(_._2)
val basicMatch = new TextEditMember(
val basicMatch = new CasePatternMember(
"match",
new l.TextEdit(
editRange,
Expand Down Expand Up @@ -339,7 +339,7 @@ trait MatchCaseCompletions { this: MetalsGlobal =>
)
val detail =
s" ${metalsToLongString(tpe, new ShortenedNames())} (${members.length} cases)"
val exhaustiveMatch = new TextEditMember(
val exhaustiveMatch = new CasePatternMember(
"match (exhaustive)",
newText,
tpe.typeSymbol,
Expand Down Expand Up @@ -454,26 +454,38 @@ trait MatchCaseCompletions { this: MetalsGlobal =>
else pattern
val cursorSuffix = (if (patternOnly.nonEmpty) "" else " ") +
(if (clientSupportsSnippets) "$0" else "")
new TextEditMember(
filterText = label,
edit = new l.TextEdit(
editRange,
label + cursorSuffix
),
sym = sym,
label = Some(label),
additionalTextEdits = autoImports

val edit = new l.TextEdit(
editRange,
label + cursorSuffix
)
if (patternOnly.nonEmpty && name == pattern) {
Copy link
Contributor Author

@harpocrates harpocrates Mar 12, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

This is a case to ensure that in something like

Option(123) match {
  case No@@
}

you only see None once instead of once for something from match completions then again from the general scope completions. Said differently, this is an exception where we want the match completions and general scope completions to collide so we see only one of them.

new TextEditMember(
filterText = label,
edit = edit,
sym = sym,
label = Some(label),
additionalTextEdits = autoImports
)
} else {
new CasePatternMember(
filterText = label,
edit = edit,
sym = sym,
label = Some(label),
additionalTextEdits = autoImports
)
}
}

def caseKeywordOnly: List[TextEditMember] =
def caseKeywordOnly: List[CasePatternMember] =
if (patternOnly.isEmpty) {
val label = "case"
val suffix =
if (clientSupportsSnippets) " $0 =>"
else " "
List(
new TextEditMember(
new CasePatternMember(
label,
new l.TextEdit(editRange, label + suffix),
NoSymbol.newErrorSymbol(TermName("case")).setInfo(NoType),
Expand Down
20 changes: 20 additions & 0 deletions tests/cross/src/test/scala/tests/pc/CompletionCaseSuite.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -910,4 +910,24 @@ class CompletionCaseSuite extends BaseCompletionSuite {
|}""".stripMargin
)

check(
"underscore-and-object",
"""package myPackage1 {
| class MyClass
| object MyClass {
| val TheValue = new MyClass
| }
|}
|object Test {
| val x = myPackage1.MyClass.TheValue
| x match {
| case My@@
| }
|}
|""".stripMargin,
"""_: MyClass myPackage1
|MyClass - myPackage1
|""".stripMargin
)

}
8 changes: 4 additions & 4 deletions tests/cross/src/test/scala/tests/pc/CompletionPatternSuite.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,8 +85,8 @@ class CompletionPatternSuite extends BaseCompletionSuite {
| case abc @ @@ =>
| }
|}""".stripMargin,
"""|None scala
|Some(value) scala
"""|Some(value) scala
|None scala
|""".stripMargin,
topLines = Some(2)
)
Expand All @@ -111,7 +111,7 @@ class CompletionPatternSuite extends BaseCompletionSuite {
| case _: @@ =>
| }
|}""".stripMargin,
"""|None scala
"""|Some[_] scala
|""".stripMargin,
compat = Map(
"3" ->
Expand Down Expand Up @@ -141,7 +141,7 @@ class CompletionPatternSuite extends BaseCompletionSuite {
| case ab: @@ =>
| }
|}""".stripMargin,
"""|None scala
"""|Some[_] scala
|""".stripMargin,
compat = Map(
"3" ->
Expand Down
10 changes: 7 additions & 3 deletions tests/cross/src/test/scala/tests/pc/CompletionSuite.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1028,9 +1028,13 @@ class CompletionSuite extends BaseCompletionSuite {
|}
|""".stripMargin,
"""|Some(value) scala
|Some scala
|""".stripMargin,
compat = Map(
"2.11" -> "Some(x) scala",
"2.11" ->
"""|Some(x) scala
|Some scala
|""".stripMargin,
"3" ->
"""|Some(value) scala
|Some scala
Expand Down Expand Up @@ -1060,11 +1064,11 @@ class CompletionSuite extends BaseCompletionSuite {
"adt",
s"""|object Main {
| Option(1) match {
| case No@@
| case Non@@
|}
|""".stripMargin,
"""|None scala
|NoManifest scala.reflect
|NonFatal - scala.util.control
|""".stripMargin,
topLines = Some(2)
)
Expand Down
Loading