Skip to content

Commit 2d7ad61

Browse files
committed
Remove scope completions, add proper shadowing resolution, improve shortened-type-printer
1 parent 86420f1 commit 2d7ad61

File tree

14 files changed

+177
-130
lines changed

14 files changed

+177
-130
lines changed

compiler/src/dotty/tools/dotc/core/Flags.scala

+1-1
Original file line numberDiff line numberDiff line change
@@ -242,7 +242,7 @@ object Flags {
242242
val (AccessorOrSealed @ _, Accessor @ _, Sealed @ _) = newFlags(11, "<accessor>", "sealed")
243243

244244
/** A mutable var, an open class */
245-
val (MutableOrOpen @ __, Mutable @ _, Open @ _) = newFlags(12, "mutable", "open")
245+
val (MutableOrOpen @ _, Mutable @ _, Open @ _) = newFlags(12, "mutable", "open")
246246

247247
/** Symbol is local to current class (i.e. private[this] or protected[this]
248248
* pre: Private or Protected are also set

compiler/src/dotty/tools/dotc/interactive/Completion.scala

+68-62
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package dotty.tools.dotc.interactive
22

33
import dotty.tools.dotc.ast.untpd
4+
import dotty.tools.dotc.ast.tpd
45
import dotty.tools.dotc.ast.NavigateAST
56
import dotty.tools.dotc.config.Printers.interactiv
67
import dotty.tools.dotc.core.Contexts.*
@@ -45,8 +46,6 @@ case class Completion(label: String, description: String, symbols: List[Symbol])
4546

4647
object Completion:
4748

48-
import dotty.tools.dotc.ast.tpd.*
49-
5049
/** Get possible completions from tree at `pos`
5150
*
5251
* @return offset and list of symbols for possible completions
@@ -62,7 +61,6 @@ object Completion:
6261

6362
postProcessCompletions(untpdPath, completions, rawPrefix)
6463

65-
6664
/** Get possible completions from tree at `pos`
6765
* This method requires manually computing the mode, prefix and paths.
6866
*
@@ -72,7 +70,7 @@ object Completion:
7270
pos: SourcePosition,
7371
mode: Mode,
7472
rawPrefix: String,
75-
tpdPath: List[Tree],
73+
tpdPath: List[tpd.Tree],
7674
untpdPath: List[untpd.Tree]
7775
)(using Context): CompletionMap =
7876
val adjustedPath = typeCheckExtensionConstructPath(untpdPath, tpdPath, pos)
@@ -94,7 +92,7 @@ object Completion:
9492
path match
9593
case untpd.Ident(_) :: untpd.Import(_, _) :: _ => Mode.ImportOrExport
9694
case untpd.Ident(_) :: (_: untpd.ImportSelector) :: _ => Mode.ImportOrExport
97-
case Literal(Constants.Constant(_: String)) :: _ => Mode.Term // literal completions
95+
case untpd.Literal(Constants.Constant(_: String)) :: _ => Mode.Term // literal completions
9896
case (ref: untpd.RefTree) :: _ =>
9997
if (ref.name.isTermName) Mode.Term
10098
else if (ref.name.isTypeName) Mode.Type
@@ -109,9 +107,9 @@ object Completion:
109107

110108
val completionKind: Mode =
111109
path match
112-
case Nil | (_: PackageDef) :: _ => Mode.None
110+
case Nil | (_: untpd.PackageDef) :: _ => Mode.None
113111
case untpd.Ident(_) :: (_: untpd.ImportSelector) :: _ => Mode.Member
114-
case (_: Select) :: _ => Mode.Member
112+
case (_: untpd.Select) :: _ => Mode.Member
115113
case _ => Mode.Scope
116114

117115
completionSymbolKind | completionKind
@@ -188,8 +186,8 @@ object Completion:
188186
* @return Typed path to the parameter of the extension construct if found or tpdPath
189187
*/
190188
private def typeCheckExtensionConstructPath(
191-
untpdPath: List[untpd.Tree], tpdPath: List[Tree], pos: SourcePosition
192-
)(using Context): List[Tree] =
189+
untpdPath: List[untpd.Tree], tpdPath: List[tpd.Tree], pos: SourcePosition
190+
)(using Context): List[tpd.Tree] =
193191
untpdPath.collectFirst:
194192
case untpd.ExtMethods(paramss, _) =>
195193
val enclosingParam = paramss.flatten.find(_.span.contains(pos.span))
@@ -199,27 +197,29 @@ object Completion:
199197
Interactive.pathTo(typedEnclosingParam, pos.span)
200198
.flatten.getOrElse(tpdPath)
201199

202-
private def computeCompletions(pos: SourcePosition, mode: Mode, rawPrefix: String, adjustedPath: List[Tree])(using Context): CompletionMap =
200+
private def computeCompletions(
201+
pos: SourcePosition, mode: Mode, rawPrefix: String, adjustedPath: List[tpd.Tree]
202+
)(using Context): CompletionMap =
203203
val hasBackTick = rawPrefix.headOption.contains('`')
204204
val prefix = if hasBackTick then rawPrefix.drop(1) else rawPrefix
205205
val completer = new Completer(mode, prefix, pos)
206206

207207
val result = adjustedPath match
208208
// Ignore synthetic select from `This` because in code it was `Ident`
209209
// See example in dotty.tools.languageserver.CompletionTest.syntheticThis
210-
case Select(qual @ This(_), _) :: _ if qual.span.isSynthetic => completer.scopeCompletions
211-
case Select(qual, _) :: _ if qual.tpe.hasSimpleKind => completer.selectionCompletions(qual)
212-
case Select(qual, _) :: _ => Map.empty
213-
case (tree: ImportOrExport) :: _ => completer.directMemberCompletions(tree.expr)
214-
case (_: untpd.ImportSelector) :: Import(expr, _) :: _ => completer.directMemberCompletions(expr)
215-
case _ => completer.scopeCompletions
210+
case tpd.Select(qual @ tpd.This(_), _) :: _ if qual.span.isSynthetic => completer.scopeCompletions
211+
case tpd.Select(qual, _) :: _ if qual.tpe.hasSimpleKind => completer.selectionCompletions(qual)
212+
case tpd.Select(qual, _) :: _ => Map.empty
213+
case (tree: tpd.ImportOrExport) :: _ => completer.directMemberCompletions(tree.expr)
214+
case (_: untpd.ImportSelector) :: tpd.Import(expr, _) :: _ => completer.directMemberCompletions(expr)
215+
case _ => completer.scopeCompletions
216216

217217
interactiv.println(i"""completion info with pos = $pos,
218-
| prefix = ${completer.prefix},
219-
| term = ${completer.mode.is(Mode.Term)},
220-
| type = ${completer.mode.is(Mode.Type)},
221-
| scope = ${completer.mode.is(Mode.Scope)},
222-
| member = ${completer.mode.is(Mode.Member)}""")
218+
| prefix = ${completer.prefix},
219+
| term = ${completer.mode.is(Mode.Term)},
220+
| type = ${completer.mode.is(Mode.Type)},
221+
| scope = ${completer.mode.is(Mode.Scope)},
222+
| member = ${completer.mode.is(Mode.Member)}""")
223223

224224
result
225225

@@ -279,6 +279,16 @@ object Completion:
279279
if denot.isType then denot.symbol.showFullName
280280
else denot.info.widenTermRefExpr.show
281281

282+
given ScopeOrdering(using Context): Ordering[Seq[SingleDenotation]] with
283+
val order =
284+
List(defn.ScalaPredefModuleClass, defn.ScalaPackageClass, defn.JavaLangPackageClass)
285+
286+
override def compare(x: Seq[SingleDenotation], y: Seq[SingleDenotation]): Int =
287+
val owner0 = x.headOption.map(_.symbol.effectiveOwner).getOrElse(NoSymbol)
288+
val owner1 = y.headOption.map(_.symbol.effectiveOwner).getOrElse(NoSymbol)
289+
290+
order.indexOf(owner0) - order.indexOf(owner1)
291+
282292
/** Computes code completions depending on the context in which completion is requested
283293
* @param mode Should complete names of terms, types or both
284294
* @param prefix The prefix that all suggested completions should start with
@@ -295,32 +305,41 @@ object Completion:
295305
* This mimics the logic for deciding what is ambiguous used by the compiler.
296306
* In general in case of a name clash symbols introduced in more deeply nested scopes
297307
* have higher priority and shadow previous definitions with the same name although:
298-
* - imports with the same level of nesting cause an ambiguity
308+
* - imports with the same level of nesting cause an ambiguity if they are in the same name space
299309
* - members and local definitions with the same level of nesting are allowed for overloading
300310
* - an import is ignored if there is a local definition or a member introduced in the same scope
301311
* (even if the import follows it syntactically)
302312
* - a more deeply nested import shadowing a member or a local definition causes an ambiguity
303313
*/
304314
def scopeCompletions(using context: Context): CompletionMap =
315+
316+
/** Temporary data structure representing denotations with the same name introduced in a given scope
317+
* as a member of a type, by a local definition or by an import clause
318+
*/
319+
case class ScopedDenotations private (denots: Seq[SingleDenotation], ctx: Context)
320+
object ScopedDenotations:
321+
def apply(denots: Seq[SingleDenotation], ctx: Context, includeFn: SingleDenotation => Boolean): ScopedDenotations =
322+
ScopedDenotations(denots.filter(includeFn), ctx)
323+
305324
val mappings = collection.mutable.Map.empty[Name, List[ScopedDenotations]].withDefaultValue(List.empty)
306325
def addMapping(name: Name, denots: ScopedDenotations) =
307326
mappings(name) = mappings(name) :+ denots
308327

309328
ctx.outersIterator.foreach { case ctx @ given Context =>
310329
if ctx.isImportContext then
311330
importedCompletions.foreach { (name, denots) =>
312-
addMapping(name, ScopedDenotations(denots, ctx))
331+
addMapping(name, ScopedDenotations(denots, ctx, include(_, name)))
313332
}
314333
else if ctx.owner.isClass then
315334
accessibleMembers(ctx.owner.thisType)
316335
.groupByName.foreach { (name, denots) =>
317-
addMapping(name, ScopedDenotations(denots, ctx))
336+
addMapping(name, ScopedDenotations(denots, ctx, include(_, name)))
318337
}
319338
else if ctx.scope ne EmptyScope then
320339
ctx.scope.toList.filter(symbol => include(symbol, symbol.name))
321340
.flatMap(_.alternatives)
322341
.groupByName.foreach { (name, denots) =>
323-
addMapping(name, ScopedDenotations(denots, ctx))
342+
addMapping(name, ScopedDenotations(denots, ctx, include(_, name)))
324343
}
325344
}
326345

@@ -333,26 +352,22 @@ object Completion:
333352
def isSingleImport = denotss.length < 2
334353
// import a.C
335354
// locally { import b.C }
336-
def isImportedInDifferentScope = first.ctx.scope ne denotss(1).ctx.scope
355+
def isImportedInDifferentScope = first.ctx.scope ne denotss(1).ctx.scope
337356
// import a.C
338357
// import a.C
339-
def isSameSymbolImportedDouble = denotss.forall(_.denots == first.denots)
340-
341-
def isScalaPackage(scopedDenots: ScopedDenotations) =
342-
scopedDenots.denots.exists(_.info.typeSymbol.owner == defn.ScalaPackageClass)
343-
344-
def isJavaLangPackage(scopedDenots: ScopedDenotations) =
345-
scopedDenots.denots.exists(_.info.typeSymbol.owner == defn.JavaLangPackageClass)
346-
347-
// For example
348-
// import java.lang.annotation
349-
// is shadowed by
350-
// import scala.annotation
351-
def isJavaLangAndScala =
352-
try
353-
denotss.forall(denots => isScalaPackage(denots) || isJavaLangPackage(denots))
354-
catch
355-
case NonFatal(_) => false
358+
def isSameSymbolImportedDouble = denotss.forall(_.denots == first.denots)
359+
360+
// https://scala-lang.org/files/archive/spec/3.4/02-identifiers-names-and-scopes.html
361+
// import java.lang.*
362+
// {
363+
// import scala.*
364+
// {
365+
// import Predef.*
366+
// { /* source */ }
367+
// }
368+
// }
369+
def notConflictingWithDefaults = // is imported symbol
370+
denotss.filterNot(_.denots.exists(denot => Interactive.isImportedByDefault(denot.symbol))).size <= 1
356371

357372
denotss.find(!_.ctx.isImportContext) match {
358373
// most deeply nested member or local definition if not shadowed by an import
@@ -361,13 +376,9 @@ object Completion:
361376

362377
case None if isSingleImport || isImportedInDifferentScope || isSameSymbolImportedDouble =>
363378
resultMappings += name -> first.denots
364-
case None if isJavaLangAndScala =>
365-
denotss.foreach{
366-
denots =>
367-
if isScalaPackage(denots) then
368-
resultMappings += name -> denots.denots
369-
}
370-
379+
case None if notConflictingWithDefaults =>
380+
val ordered = denotss.map(_.denots).sorted
381+
resultMappings += name -> ordered.head
371382
case _ =>
372383
}
373384
}
@@ -377,7 +388,7 @@ object Completion:
377388

378389
/** Widen only those types which are applied or are exactly nothing
379390
*/
380-
def widenQualifier(qual: Tree)(using Context): Tree =
391+
def widenQualifier(qual: tpd.Tree)(using Context): tpd.Tree =
381392
qual.tpe.widenDealias match
382393
case widenedType if widenedType.isExactlyNothing => qual.withType(widenedType)
383394
case appliedType: AppliedType => qual.withType(appliedType)
@@ -387,7 +398,7 @@ object Completion:
387398
* Direct members take priority over members from extensions
388399
* and so do members from extensions over members from implicit conversions
389400
*/
390-
def selectionCompletions(qual: Tree)(using Context): CompletionMap =
401+
def selectionCompletions(qual: tpd.Tree)(using Context): CompletionMap =
391402
val adjustedQual = widenQualifier(qual)
392403

393404
implicitConversionMemberCompletions(adjustedQual) ++
@@ -397,7 +408,7 @@ object Completion:
397408
/** Completions for members of `qual`'s type.
398409
* These include inherited definitions but not members added by extensions or implicit conversions
399410
*/
400-
def directMemberCompletions(qual: Tree)(using Context): CompletionMap =
411+
def directMemberCompletions(qual: tpd.Tree)(using Context): CompletionMap =
401412
if qual.tpe.isExactlyNothing then
402413
Map.empty
403414
else
@@ -444,13 +455,13 @@ object Completion:
444455
end importedCompletions
445456

446457
/** Completions from implicit conversions including old style extensions using implicit classes */
447-
private def implicitConversionMemberCompletions(qual: Tree)(using Context): CompletionMap =
458+
private def implicitConversionMemberCompletions(qual: tpd.Tree)(using Context): CompletionMap =
448459

449460
def tryToInstantiateTypeVars(conversionTarget: SearchSuccess): Type =
450461
try
451462
val typingCtx = ctx.fresh
452463
inContext(typingCtx):
453-
val methodRefTree = ref(conversionTarget.ref, needLoad = false)
464+
val methodRefTree = tpd.ref(conversionTarget.ref, needLoad = false)
454465
val convertedTree = ctx.typer.typedAheadExpr(untpd.Apply(untpd.TypedSplice(methodRefTree), untpd.TypedSplice(qual) :: Nil))
455466
Inferencing.fullyDefinedType(convertedTree.tpe, "", pos)
456467
catch
@@ -465,7 +476,7 @@ object Completion:
465476
.groupByName
466477

467478
/** Completions from extension methods */
468-
private def extensionCompletions(qual: Tree)(using Context): CompletionMap =
479+
private def extensionCompletions(qual: tpd.Tree)(using Context): CompletionMap =
469480
def asDefLikeType(tpe: Type): Type = tpe match
470481
case _: MethodOrPoly => tpe
471482
case _ => ExprType(tpe)
@@ -592,7 +603,7 @@ object Completion:
592603
* @param qual The argument to which the implicit conversion should be applied.
593604
* @return The set of types after `qual` implicit conversion.
594605
*/
595-
private def implicitConversionTargets(qual: Tree)(using Context): Set[SearchSuccess] = {
606+
private def implicitConversionTargets(qual: tpd.Tree)(using Context): Set[SearchSuccess] = {
596607
val typer = ctx.typer
597608
val conversions = new typer.ImplicitSearch(defn.AnyType, qual, pos.span).allImplicits
598609
val targets = conversions.map(_.tree.tpe)
@@ -617,11 +628,6 @@ object Completion:
617628

618629
private type CompletionMap = Map[Name, Seq[SingleDenotation]]
619630

620-
/** Temporary data structure representing denotations with the same name introduced in a given scope
621-
* as a member of a type, by a local definition or by an import clause
622-
*/
623-
private case class ScopedDenotations(denots: Seq[SingleDenotation], ctx: Context)
624-
625631
/**
626632
* The completion mode: defines what kinds of symbols should be included in the completion
627633
* results.

compiler/src/dotty/tools/dotc/interactive/Interactive.scala

+14
Original file line numberDiff line numberDiff line change
@@ -451,6 +451,20 @@ object Interactive {
451451
def sameName(n0: Name, n1: Name): Boolean =
452452
n0.stripModuleClassSuffix.toTermName eq n1.stripModuleClassSuffix.toTermName
453453

454+
/** https://scala-lang.org/files/archive/spec/3.4/02-identifiers-names-and-scopes.html
455+
* import java.lang.*
456+
* {
457+
* import scala.*
458+
* {
459+
* import Predef.*
460+
* { /* source */ }
461+
* }
462+
* }
463+
*/
464+
def isImportedByDefault(sym: Symbol)(using Context): Boolean =
465+
val owner = sym.effectiveOwner
466+
owner == defn.ScalaPredefModuleClass || owner == defn.ScalaPackageClass || owner == defn.JavaLangPackageClass
467+
454468
private[interactive] def safely[T](op: => List[T]): List[T] =
455469
try op catch { case ex: TypeError => Nil }
456470
}

language-server/test/dotty/tools/languageserver/CompletionTest.scala

+33
Original file line numberDiff line numberDiff line change
@@ -1646,4 +1646,37 @@ class CompletionTest {
16461646
|"""
16471647
.completion(m1, Set(("selectDynamic", Method, "(field: String): Foo")))
16481648
.completion(m2, Set(("banana", Method, "=> Int")))
1649+
1650+
@Test def shadowedImport: Unit =
1651+
code"""|
1652+
|import Matches.*
1653+
|object Matches {
1654+
| val Number = "".r
1655+
|}
1656+
|object Main {
1657+
| Num$m1
1658+
|}
1659+
|""".completion(m1, Set(
1660+
("Number", Field, "scala.util.matching.Regex"),
1661+
("NumberFormatException", Module, "NumberFormatException"),
1662+
("Numeric", Field, "scala.math.Numeric")
1663+
))
1664+
1665+
@Test def shadowedImportType: Unit =
1666+
code"""|
1667+
|import Matches.*
1668+
|object Matches {
1669+
| val Number = "".r
1670+
|}
1671+
|object Main {
1672+
| val x: Num$m1
1673+
|}
1674+
|""".completion(m1, Set(
1675+
("Number", Class, "Number"),
1676+
("Number", Field, "scala.util.matching.Regex"),
1677+
("NumberFormatException", Module, "NumberFormatException"),
1678+
("NumberFormatException", Field, "NumberFormatException"),
1679+
("Numeric", Field, "Numeric"),
1680+
("Numeric", Field, "scala.math.Numeric")
1681+
))
16491682
}

0 commit comments

Comments
 (0)