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

Add Stable Presentation Compiler #17528

Merged
merged 25 commits into from
Jul 6, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
63d3a35
compiler util: collect comments during Scanner phase and store it in …
rochala May 12, 2023
fde037a
metals initial version: 41e96ee33f82 copied into dotty
rochala May 12, 2023
9f271fe
changes: make mtags compile in dotty, changes metals code and adds ne…
rochala May 12, 2023
38f58dc
additions: copy tests from metals version: 41e96ee33f82 , create diff…
rochala May 12, 2023
1a02dbf
refactor: change package from scala.meta to dotty.tools.pc
rochala May 12, 2023
fffe497
refactor: organize imports, unify formatting with mtags in metals
rochala May 12, 2023
3e857a2
update Build.scala and build.sbt to support nonbootstrapped compiler,…
rochala May 16, 2023
e643f9f
implement FIXME from previous commit
rochala May 16, 2023
3bf65da
add nightly mtags-shared as dependency instead of local snapshot
rochala May 17, 2023
34814ce
remove unnecessary changes in Comments.scala
rochala May 17, 2023
f2d8c64
remove metals wrappers around compiler implementation created to work…
rochala May 22, 2023
32c50e0
update NOTICE.md
rochala May 22, 2023
d823951
use java8 compatible api
rochala May 23, 2023
7074f0d
Mock Symbol Search documentation and definition
rochala May 26, 2023
122f36f
Revert incorrectly removed line
rochala May 26, 2023
421380c
fix build.sbt and windows tests
rochala May 26, 2023
f59457c
Filter tests to contain only completions that should be available on …
rochala May 27, 2023
6ad9dc7
Bump presentatation compiler to c8ef4e0
rochala Jun 15, 2023
5ee339d
use compiler printer instead of ShortenedNames with ShortType
rochala Jul 3, 2023
aa7542f
undo unnecessary changes, add -Wunsued:all, remove unused
rochala Jul 3, 2023
2d6aff2
Remove -Wunused from build.sbt, undo scalafmt
rochala Jul 3, 2023
9856331
apply review comments, inline toTextPrefix
rochala Jul 5, 2023
0869f93
update metals to latest commit - 7d0397b
rochala Jul 5, 2023
ae8ee2f
remove check for JavaStatic, as they are no longer necessary
rochala Jul 6, 2023
2b34077
remove flaky mock
rochala Jul 6, 2023
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
29 changes: 12 additions & 17 deletions presentation-compiler/src/main/dotty/tools/pc/AutoImports.scala
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ import dotty.tools.pc.utils.MtagsEnrichments.*

import org.eclipse.lsp4j as l

object AutoImports extends AutoImportsBackticks:
object AutoImports:

object AutoImport:
def renameConfigMap(config: PresentationCompilerConfig)(using
Expand Down Expand Up @@ -199,7 +199,7 @@ object AutoImports extends AutoImportsBackticks:
(
SymbolIdent.Select(
ownerImport.ident,
symbol.nameBacktickedImport
symbol.nameBackticked(false)
),
ownerImport.importSel,
)
Expand Down Expand Up @@ -233,7 +233,7 @@ object AutoImports extends AutoImportsBackticks:
symbol,
SymbolIdent.Select(
SymbolIdent.direct(rename),
symbol.nameBacktickedImport
symbol.nameBackticked(false)
),
importSel
)
Expand Down Expand Up @@ -272,7 +272,7 @@ object AutoImports extends AutoImportsBackticks:
.map {
case ImportSel.Direct(sym) => importName(sym)
case ImportSel.Rename(sym, rename) =>
s"${importName(sym.owner)}.{${sym.nameBacktickedImport} => $rename}"
s"${importName(sym.owner)}.{${sym.nameBackticked(false)} => $rename}"
}
.map(sel => s"${indent}import $sel")
.mkString(topPadding, "\n", "\n")
Expand All @@ -283,8 +283,8 @@ object AutoImports extends AutoImportsBackticks:

private def importName(sym: Symbol): String =
if indexedContext.importContext.toplevelClashes(sym) then
s"_root_.${sym.fullNameBacktickedImport}"
else sym.fullNameBacktickedImport
s"_root_.${sym.fullNameBackticked(false)}"
else sym.fullNameBackticked(false)
end AutoImportsGenerator

private def autoImportPosition(
Expand Down Expand Up @@ -361,9 +361,12 @@ object AutoImports extends AutoImportsBackticks:
else
ScriptFirstImportPosition.scalaCliScStartOffset(text, comments)

scriptOffset.getOrElse(
pos.source.lineToOffset(tmpl.self.srcPos.line)
)
scriptOffset.getOrElse {
val tmplPoint = tmpl.self.srcPos.span.point
if tmplPoint >= 0 && tmplPoint < pos.source.length
then pos.source.lineToOffset(tmpl.self.srcPos.line)
else 0
}
new AutoImportPosition(offset, text, false)
}
end forScript
Expand All @@ -388,11 +391,3 @@ object AutoImports extends AutoImportsBackticks:
end autoImportPosition

end AutoImports

trait AutoImportsBackticks:
// Avoids backticketing import parts that match soft keywords
extension (sym: Symbol)(using Context)
def fullNameBacktickedImport: String =
sym.fullNameBackticked(KeywordWrapper.Scala3SoftKeywords)
def nameBacktickedImport: String =
sym.nameBackticked(KeywordWrapper.Scala3SoftKeywords)
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package dotty.tools.pc

import java.nio.file.Paths

import scala.meta.internal.metals.ReportContext
import scala.meta.internal.pc.ExtractMethodUtils
import scala.meta.pc.OffsetParams
import scala.meta.pc.RangeParams
Expand Down Expand Up @@ -33,7 +34,8 @@ final class ExtractMethodProvider(
driver: InteractiveDriver,
search: SymbolSearch,
noIndent: Boolean
) extends ExtractMethodUtils:
)(using ReportContext)
extends ExtractMethodUtils:

def extractMethod(): List[TextEdit] =
val text = range.text()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -136,18 +136,21 @@ object IndexedContext:
)

private def extractNames(ctx: Context): Names =
def isAccessibleFromSafe(sym: Symbol, site: Type): Boolean =
try sym.isAccessibleFrom(site, superAccess = false)
catch
case NonFatal(e) =>
false

def accessibleSymbols(site: Type, tpe: Type)(using
Context
): List[Symbol] =
tpe.decls.toList.filter(sym =>
sym.isAccessibleFrom(site, superAccess = false)
)
tpe.decls.toList.filter(sym => isAccessibleFromSafe(sym, site))

def accesibleMembers(site: Type)(using Context): List[Symbol] =
site.allMembers
.filter(denot =>
try denot.symbol.isAccessibleFrom(site, superAccess = false)
try isAccessibleFromSafe(denot.symbol, site)
catch
case NonFatal(e) =>
false
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package dotty.tools.pc
import java.nio.file.Paths

import scala.annotation.tailrec
import scala.meta.internal.metals.ReportContext
import scala.meta.pc.OffsetParams
import scala.meta.pc.PresentationCompilerConfig
import scala.meta.pc.SymbolSearch
Expand Down Expand Up @@ -50,7 +51,7 @@ final class InferredTypeProvider(
driver: InteractiveDriver,
config: PresentationCompilerConfig,
symbolSearch: SymbolSearch
):
)(using ReportContext):

case class AdjustTypeOpts(
text: String,
Expand Down
12 changes: 8 additions & 4 deletions presentation-compiler/src/main/dotty/tools/pc/PcCollector.scala
Original file line number Diff line number Diff line change
Expand Up @@ -91,18 +91,22 @@ abstract class PcCollector[T](
else pos1

val pos =
if sourceText(pos0.`end` - 1) == ',' then pos0.withEnd(pos0.`end` - 1)
if pos0.end > 0 && sourceText(pos0.end - 1) == ',' then
pos0.withEnd(pos0.end - 1)
else pos0
val isBackticked =
sourceText(pos.start) == '`' && sourceText(pos.end - 1) == '`'
sourceText(pos.start) == '`' &&
pos.end > 0 &&
sourceText(pos.end - 1) == '`'
// when the old name contains backticks, the position is incorrect
val isOldNameBackticked = sourceText(pos.start) != '`' &&
pos.start > 0 &&
sourceText(pos.start - 1) == '`' &&
sourceText(pos.end) == '`'
if isBackticked && forRename then
(pos.withStart(pos.start + 1).withEnd(pos.`end` - 1), true)
(pos.withStart(pos.start + 1).withEnd(pos.end - 1), true)
else if isOldNameBackticked then
(pos.withStart(pos.start - 1).withEnd(pos.`end` + 1), false)
(pos.withStart(pos.start - 1).withEnd(pos.end + 1), false)
else (pos, false)
end adjust

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import dotty.tools.dotc.core.Contexts.*
import dotty.tools.dotc.core.Flags.*
import dotty.tools.dotc.core.Names.*
import dotty.tools.dotc.core.Symbols.*
import dotty.tools.dotc.semanticdb._
import dotty.tools.dotc.semanticdb.*

object SemanticdbSymbols:

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -218,14 +218,18 @@ class CompletionProvider(
case _ =>
false

lazy val backtickSoftKeyword = path match
case (_: Select) :: _ => false
case _ => true

def mkItemWithImports(
v: CompletionValue.Workspace | CompletionValue.Extension |
CompletionValue.Interpolator
) =
val sym = v.symbol
path match
case (_: Ident) :: (_: Import) :: _ =>
mkItem(sym.fullNameBackticked)
mkItem(sym.fullNameBackticked(backtickSoftKeyword = false))
case _ =>
autoImports.editsForSymbol(v.importSymbol) match
case Some(edits) =>
Expand All @@ -239,7 +243,9 @@ class CompletionProvider(
case _ =>
mkItem(
v.insertText.getOrElse(
ident.backticked + completionTextSuffix
ident.backticked(
backtickSoftKeyword
) + completionTextSuffix
),
edits.edits,
range = v.range
Expand All @@ -249,14 +255,18 @@ class CompletionProvider(
r match
case IndexedContext.Result.InScope =>
mkItem(
ident.backticked + completionTextSuffix
ident.backticked(backtickSoftKeyword) + completionTextSuffix
)
case _ if isInStringInterpolation =>
mkItem(
"{" + sym.fullNameBackticked + completionTextSuffix + "}"
)
case _ =>
mkItem(sym.fullNameBackticked + completionTextSuffix)
mkItem(
sym.fullNameBackticked(
backtickSoftKeyword
) + completionTextSuffix
)
end match
end match
end match
Expand All @@ -268,7 +278,8 @@ class CompletionProvider(
case v: CompletionValue.Interpolator if v.isWorkspace || v.isExtension =>
mkItemWithImports(v)
case _ =>
val insert = completion.insertText.getOrElse(ident.backticked)
val insert =
completion.insertText.getOrElse(ident.backticked(backtickSoftKeyword))
mkItem(
insert + completionTextSuffix,
range = completion.range
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -579,7 +579,7 @@ class Completions(
).map(visit).forall(_ == true),
)
Some(search.search(query, buildTargetIdentifier, visitor))
case CompletionKind.Members if query.nonEmpty =>
case CompletionKind.Members =>
val visitor = new CompilerSearchVisitor(sym =>
if sym.is(ExtensionMethod) &&
qualType.widenDealias <:< sym.extensionParam.info.widenDealias
Expand All @@ -592,8 +592,6 @@ class Completions(
else false,
)
Some(search.searchMethods(query, buildTargetIdentifier, visitor))
case CompletionKind.Members => // query.isEmpry
Some(SymbolSearch.Result.INCOMPLETE)
end match
end enrichWithSymbolSearch

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ package completions
import java.util as ju

import scala.jdk.CollectionConverters._
import scala.meta.internal.metals.ReportContext
import scala.meta.pc.OffsetParams
import scala.meta.pc.PresentationCompilerConfig
import scala.meta.pc.PresentationCompilerConfig.OverrideDefFormat
Expand Down Expand Up @@ -52,7 +53,7 @@ object OverrideCompletions:
config: PresentationCompilerConfig,
autoImportsGen: AutoImportsGenerator,
fallbackName: Option[String]
): List[CompletionValue] =
)(using ReportContext): List[CompletionValue] =
import indexedContext.ctx
val clazz = td.symbol.asClass
val syntheticCoreMethods: Set[Name] =
Expand Down Expand Up @@ -133,7 +134,7 @@ object OverrideCompletions:
driver: InteractiveDriver,
search: SymbolSearch,
config: PresentationCompilerConfig
): ju.List[l.TextEdit] =
)(using ReportContext): ju.List[l.TextEdit] =
object FindTypeDef:
def unapply(path: List[Tree])(using Context): Option[TypeDef] = path match
// class <<Foo>> extends ... {}
Expand Down Expand Up @@ -225,7 +226,7 @@ object OverrideCompletions:
config: PresentationCompilerConfig
)(
defn: TargetDef
)(using Context): List[l.TextEdit] =
)(using Context, ReportContext): List[l.TextEdit] =
def calcIndent(
defn: TargetDef,
decls: List[Symbol],
Expand Down Expand Up @@ -391,7 +392,7 @@ object OverrideCompletions:
config: PresentationCompilerConfig,
autoImportsGen: AutoImportsGenerator,
shouldAddOverrideKwd: Boolean
)(using Context): CompletionValue.Override =
)(using Context, ReportContext): CompletionValue.Override =
val renames = AutoImport.renameConfigMap(config)
val printer = MetalsPrinter.standard(
indexedContext,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,15 @@ object DotcPrinter:
def fullName(sym: Symbol): String =
fullNameString(sym)

override def toTextRef(tp: SingletonType): Text = controlled {
Copy link
Member

Choose a reason for hiding this comment

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

It looks like this file is also working around and duplicating things from Dotty and should be folded into the regular RefinedPrinter/PlainPrinter (it looks like ForInferredType below uses metals-specific stuff and could stay separate), but this can be done in a separate PR if you prefer.

tp match
case tp: TermRef if !printDebug && tp.symbol.is(Package) =>
toTextPrefix(tp.prefix) ~
tp.symbol.name.stripModuleClassSuffix.toString()
case _ =>
super.toTextRef(tp)
}

override def toText(tp: Type): Text =
// Override the behavior for `AppliedType` because
// `toText` in RefinedPrinter doesn't pretty print AppliedType
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,13 @@ package dotty.tools.pc.printer

import scala.collection.mutable
import scala.meta.internal.jdk.CollectionConverters.*
import scala.meta.internal.metals.Report
import scala.meta.internal.metals.ReportContext
import scala.meta.pc.SymbolDocumentation
import scala.meta.pc.SymbolSearch
import scala.util.Failure
import scala.util.Success
import scala.util.Try

import dotty.tools.dotc.core.Contexts.Context
import dotty.tools.dotc.core.Flags
Expand All @@ -28,7 +33,8 @@ class MetalsPrinter(
includeDefaultParam: MetalsPrinter.IncludeDefaultParam =
IncludeDefaultParam.ResolveLater,
)(using
Context
Context,
ReportContext
):

private val methodFlags =
Expand Down Expand Up @@ -63,8 +69,26 @@ class MetalsPrinter(
case _ => None

def tpe(tpe: Type): String =
val short = names.shortType(tpe)
val short = Try(names.shortType(tpe)) match
case Success(short) => short
case Failure(e) =>
val reportContext = summon[ReportContext]
val report = Report(
"short-name-error",
s"""|Error while printing type, could not create short name for type:
|
|$tpe
|
|Exception:
|${e.getMessage}
|${e.getStackTrace.mkString("\n")}
|""".stripMargin,
tpe.typeSymbol.name.show
)
reportContext.unsanitized.create(report, ifVerbose = false)
tpe
dotcPrinter.tpe(short)
end tpe

def hoverSymbol(sym: Symbol, info: Type)(using Context): String =
val typeSymbol = info.typeSymbol
Expand Down Expand Up @@ -322,7 +346,7 @@ class MetalsPrinter(
index: Int,
defaultValues: => Seq[SymbolDocumentation],
nameToInfo: Map[Name, Type]
): String =
)(using ReportContext): String =
val docInfo = defaultValues.lift(index)
val rawKeywordName = dotcPrinter.name(param)
val keywordName = docInfo match
Expand Down Expand Up @@ -409,7 +433,7 @@ object MetalsPrinter:
symbolSearch: SymbolSearch,
includeDefaultParam: IncludeDefaultParam,
renames: Map[Symbol, String] = Map.empty
): MetalsPrinter =
)(using ReportContext): MetalsPrinter =
import indexed.ctx
MetalsPrinter(
new ShortenedNames(indexed, renames),
Expand All @@ -423,7 +447,7 @@ object MetalsPrinter:
indexed: IndexedContext,
symbolSearch: SymbolSearch,
includeDefaultParam: IncludeDefaultParam
): MetalsPrinter =
)(using ReportContext): MetalsPrinter =
import shortenedNames.indexedContext.ctx
MetalsPrinter(
shortenedNames,
Expand Down
Loading