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 all commits
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
12 changes: 8 additions & 4 deletions NOTICE.md
Original file line number Diff line number Diff line change
Expand Up @@ -89,15 +89,19 @@ major authors were omitted by oversight.
details.

* dotty.tools.dotc.coverage: Coverage instrumentation utilities have been
adapted from the scoverage plugin for scala 2 [5], which is under the
adapted from the scoverage plugin for scala 2 [4], which is under the
Apache 2.0 license.

* dooty.tools.pc: Presentation compiler implementation adapted from
scalameta/metals [5] mtags module, which is under the Apache 2.0 license.

* The Dotty codebase contains parts which are derived from
the ScalaPB protobuf library [4], which is under the Apache 2.0 license.
the ScalaPB protobuf library [6], which is under the Apache 2.0 license.


[1] https://github.com/scala/scala
[2] https://github.com/adriaanm/scala/tree/sbt-api-consolidate/src/compiler/scala/tools/sbt
[3] https://github.com/sbt/sbt/tree/0.13/compile/interface/src/main/scala/xsbt
[4] https://github.com/lampepfl/dotty/pull/5783/files
[5] https://github.com/scoverage/scalac-scoverage-plugin
[4] https://github.com/scoverage/scalac-scoverage-plugin
[5] https://github.com/scalameta/metals
[6] https://github.com/lampepfl/dotty/pull/5783/files
2 changes: 2 additions & 0 deletions build.sbt
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,8 @@ val `scala3-bench-run` = Build.`scala3-bench-run`
val dist = Build.dist
val `community-build` = Build.`community-build`
val `sbt-community-build` = Build.`sbt-community-build`
val `scala3-presentation-compiler` = Build.`scala3-presentation-compiler`
Copy link
Contributor

Choose a reason for hiding this comment

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

It should be ok to remove scala3-language-server now, no?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I am almost sure that the coverage of scala3-language-server is not a subset of mtags tests, which may result in lost test cases. We would either have to verify if each test has its substitute in presentation-compiler suite and migrate missing cases.

It should be done, but may require additional time to verify all cases.

Copy link
Contributor

Choose a reason for hiding this comment

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

Sure, but would be good to have an issue or a plan about it. This might be done even by someone less proficient in the compiler.

val `scala3-presentation-compiler-bootstrapped` = Build.`scala3-presentation-compiler-bootstrapped`

val sjsSandbox = Build.sjsSandbox
val sjsJUnitTests = Build.sjsJUnitTests
Expand Down
4 changes: 4 additions & 0 deletions compiler/src/dotty/tools/dotc/CompilationUnit.scala
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import core._
import Contexts._
import SymDenotations.ClassDenotation
import Symbols._
import Comments.Comment
import util.{FreshNameCreator, SourceFile, NoSource}
import util.Spans.Span
import ast.{tpd, untpd}
Expand Down Expand Up @@ -69,6 +70,9 @@ class CompilationUnit protected (val source: SourceFile) {
/** Can this compilation unit be suspended */
def isSuspendable: Boolean = true

/** List of all comments present in this compilation unit */
var comments: List[Comment] = Nil

/** Suspends the compilation unit by thowing a SuspendException
* and recording the suspended compilation unit
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,7 @@ class InteractiveDriver(val settings: List[String]) extends Driver {
(fromSource ++ fromClassPath).distinct
}

def run(uri: URI, sourceCode: String): List[Diagnostic] = run(uri, toSource(uri, sourceCode))
def run(uri: URI, sourceCode: String): List[Diagnostic] = run(uri, SourceFile.virtual(uri, sourceCode))

def run(uri: URI, source: SourceFile): List[Diagnostic] = {
import typer.ImportInfo._
Expand Down Expand Up @@ -297,9 +297,6 @@ class InteractiveDriver(val settings: List[String]) extends Driver {
cleanupTree(tree)
}

private def toSource(uri: URI, sourceCode: String): SourceFile =
SourceFile.virtual(Paths.get(uri).toString, sourceCode)

/**
* Initialize this driver and compiler.
*
Expand All @@ -323,7 +320,7 @@ object InteractiveDriver {
else
try
// We don't use file.file here since it'll be null
// for the VirtualFiles created by InteractiveDriver#toSource
// for the VirtualFiles created by SourceFile#virtual
// TODO: To avoid these round trip conversions, we could add an
// AbstractFile#toUri method and implement it by returning a constant
// passed as a parameter to a constructor of VirtualFile
Expand Down
1 change: 1 addition & 0 deletions compiler/src/dotty/tools/dotc/parsing/ParserPhase.scala
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ class Parser extends Phase {
val p = new Parsers.Parser(unit.source)
// p.in.debugTokenStream = true
val tree = p.parse()
ctx.compilationUnit.comments = p.in.comments
if (p.firstXmlPos.exists && !firstXmlPos.exists)
firstXmlPos = p.firstXmlPos
tree
Expand Down
22 changes: 11 additions & 11 deletions compiler/src/dotty/tools/dotc/parsing/Scanners.scala
Original file line number Diff line number Diff line change
Expand Up @@ -227,11 +227,11 @@ object Scanners {
*/
private var docstringMap: SortedMap[Int, Comment] = SortedMap.empty

/* A Buffer for comment positions */
private val commentPosBuf = new mutable.ListBuffer[Span]
/* A Buffer for comments */
private val commentBuf = new mutable.ListBuffer[Comment]

/** Return a list of all the comment positions */
def commentSpans: List[Span] = commentPosBuf.toList
/** Return a list of all the comments */
def comments: List[Comment] = commentBuf.toList

private def addComment(comment: Comment): Unit = {
val lookahead = lookaheadReader()
Expand All @@ -246,7 +246,7 @@ object Scanners {
def getDocComment(pos: Int): Option[Comment] = docstringMap.get(pos)

/** A buffer for comments */
private val commentBuf = CharBuffer(initialCharBufferSize)
private val currentCommentBuf = CharBuffer(initialCharBufferSize)

def toToken(identifier: SimpleName): Token =
def handleMigration(keyword: Token): Token =
Expand Down Expand Up @@ -523,7 +523,7 @@ object Scanners {
*
* The following tokens can start an indentation region:
*
* : = => <- if then else while do try catch
* : = => <- if then else while do try catch
* finally for yield match throw return with
*
* Inserting an INDENT starts a new indentation region with the indentation of the current
Expand Down Expand Up @@ -1019,7 +1019,7 @@ object Scanners {

private def skipComment(): Boolean = {
def appendToComment(ch: Char) =
if (keepComments) commentBuf.append(ch)
if (keepComments) currentCommentBuf.append(ch)
def nextChar() = {
appendToComment(ch)
Scanner.this.nextChar()
Expand Down Expand Up @@ -1047,9 +1047,9 @@ object Scanners {
def finishComment(): Boolean = {
if (keepComments) {
val pos = Span(start, charOffset - 1, start)
val comment = Comment(pos, commentBuf.toString)
commentBuf.clear()
commentPosBuf += pos
val comment = Comment(pos, currentCommentBuf.toString)
currentCommentBuf.clear()
commentBuf += comment

if (comment.isDocComment)
addComment(comment)
Expand All @@ -1065,7 +1065,7 @@ object Scanners {
else if (ch == '*') { nextChar(); skipComment(); finishComment() }
else {
// This was not a comment, remove the `/` from the buffer
commentBuf.clear()
currentCommentBuf.clear()
false
}
}
Expand Down
22 changes: 11 additions & 11 deletions compiler/src/dotty/tools/dotc/printing/PlainPrinter.scala
Original file line number Diff line number Diff line change
Expand Up @@ -179,7 +179,7 @@ class PlainPrinter(_ctx: Context) extends Printer {
if (printWithoutPrefix.contains(tp.symbol))
toText(tp.name)
else
toTextPrefix(tp.prefix) ~ selectionString(tp)
toTextPrefixOf(tp) ~ selectionString(tp)
case tp: TermParamRef =>
ParamRefNameString(tp) ~ lambdaHash(tp.binder) ~ ".type"
case tp: TypeParamRef =>
Expand Down Expand Up @@ -353,7 +353,7 @@ class PlainPrinter(_ctx: Context) extends Printer {
def toTextRef(tp: SingletonType): Text = controlled {
tp match {
case tp: TermRef =>
toTextPrefix(tp.prefix) ~ selectionString(tp)
toTextPrefixOf(tp) ~ selectionString(tp)
case tp: ThisType =>
nameString(tp.cls) + ".this"
case SuperType(thistpe: SingletonType, _) =>
Expand All @@ -375,15 +375,6 @@ class PlainPrinter(_ctx: Context) extends Printer {
}
}

/** The string representation of this type used as a prefix, including separator */
def toTextPrefix(tp: Type): Text = controlled {
homogenize(tp) match {
case NoPrefix => ""
case tp: SingletonType => toTextRef(tp) ~ "."
case tp => trimPrefix(toTextLocal(tp)) ~ "#"
}
}

def toTextCaptureRef(tp: Type): Text =
homogenize(tp) match
case tp: TermRef if tp.symbol == defn.captureRoot => Str("cap")
Expand All @@ -393,6 +384,15 @@ class PlainPrinter(_ctx: Context) extends Printer {
protected def isOmittablePrefix(sym: Symbol): Boolean =
defn.unqualifiedOwnerTypes.exists(_.symbol == sym) || isEmptyPrefix(sym)

/** The string representation of type prefix, including separator */
def toTextPrefixOf(tp: NamedType): Text = controlled {
homogenize(tp.prefix) match {
case NoPrefix => ""
case tp: SingletonType => toTextRef(tp) ~ "."
case tp => trimPrefix(toTextLocal(tp)) ~ "#"
}
}

protected def isEmptyPrefix(sym: Symbol): Boolean =
sym.isEffectiveRoot || sym.isAnonymousClass || sym.name.isReplWrapperName

Expand Down
4 changes: 2 additions & 2 deletions compiler/src/dotty/tools/dotc/printing/Printer.scala
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ package printing

import core._
import Texts._, ast.Trees._
import Types.{Type, SingletonType, LambdaParam},
import Types.{Type, SingletonType, LambdaParam, NamedType},
Symbols.Symbol, Scopes.Scope, Constants.Constant,
Names.Name, Denotations._, Annotations.Annotation, Contexts.Context
import typer.Implicits.SearchResult
Expand Down Expand Up @@ -101,7 +101,7 @@ abstract class Printer {
def toTextRef(tp: SingletonType): Text

/** Textual representation of a prefix of some reference, ending in `.` or `#` */
def toTextPrefix(tp: Type): Text
def toTextPrefixOf(tp: NamedType): Text

/** Textual representation of a reference in a capture set */
def toTextCaptureRef(tp: Type): Text
Expand Down
20 changes: 10 additions & 10 deletions compiler/src/dotty/tools/dotc/printing/RefinedPrinter.scala
Original file line number Diff line number Diff line change
Expand Up @@ -117,21 +117,22 @@ class RefinedPrinter(_ctx: Context) extends PlainPrinter(_ctx) {
}
}

override def toTextPrefix(tp: Type): Text = controlled {
override def toTextPrefixOf(tp: NamedType): Text = controlled {
def isOmittable(sym: Symbol) =
if printDebug then false
else if homogenizedView then isEmptyPrefix(sym) // drop <root> and anonymous classes, but not scala, Predef.
else if sym.isPackageObject then isOmittablePrefix(sym.owner)
else isOmittablePrefix(sym)
tp match {
case tp: ThisType if isOmittable(tp.cls) =>

tp.prefix match {
case thisType: ThisType if isOmittable(thisType.cls) =>
""
case tp @ TermRef(pre, _) =>
val sym = tp.symbol
if sym.isPackageObject && !homogenizedView && !printDebug then toTextPrefix(pre)
case termRef @ TermRef(pre, _) =>
val sym = termRef.symbol
if sym.isPackageObject && !homogenizedView && !printDebug then toTextPrefixOf(termRef)
else if (isOmittable(sym)) ""
else super.toTextPrefix(tp)
case _ => super.toTextPrefix(tp)
else super.toTextPrefixOf(tp)
case _ => super.toTextPrefixOf(tp)
}
}

Expand Down Expand Up @@ -427,8 +428,7 @@ class RefinedPrinter(_ctx: Context) extends PlainPrinter(_ctx) {
case id @ Ident(name) =>
val txt = tree.typeOpt match {
case tp: NamedType if name != nme.WILDCARD =>
val pre = if (tp.symbol.is(JavaStatic)) tp.prefix.widen else tp.prefix
toTextPrefix(pre) ~ withPos(selectionString(tp), tree.sourcePos)
toTextPrefixOf(tp) ~ withPos(selectionString(tp), tree.sourcePos)
case _ =>
toText(name)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -83,8 +83,8 @@ object SyntaxHighlighting {
}
}

for (span <- scanner.commentSpans)
highlightPosition(span, CommentColor)
for (comment <- scanner.comments)
highlightPosition(comment.span, CommentColor)

object TreeHighlighter extends untpd.UntypedTreeTraverser {
import untpd._
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -330,7 +330,7 @@ trait ImportSuggestions:
def importString(ref: TermRef): String =
val imported =
if ref.symbol.is(ExtensionMethod) then
s"${ctx.printer.toTextPrefix(ref.prefix).show}${ref.symbol.name}"
s"${ctx.printer.toTextPrefixOf(ref).show}${ref.symbol.name}"
else
ctx.printer.toTextRef(ref).show
s" import $imported"
Expand Down
68 changes: 67 additions & 1 deletion compiler/src/dotty/tools/dotc/util/DiffUtil.scala
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,9 @@ object DiffUtil {
* differences are highlighted.
*/
def mkColoredLineDiff(expected: Seq[String], actual: Seq[String]): String = {
val expectedSize = EOF.length max expected.maxBy(_.length).length
val longestExpected = expected.map(_.length).maxOption.getOrElse(0)
val longestActual = actual.map(_.length).maxOption.getOrElse(0)
val expectedSize = EOF.length max longestActual max longestExpected
actual.padTo(expected.length, "").zip(expected.padTo(actual.length, "")).map { case (act, exp) =>
mkColoredLineDiff(exp, act, expectedSize)
}.mkString(System.lineSeparator)
Expand Down Expand Up @@ -101,11 +103,75 @@ object DiffUtil {
case Deleted(str) => deleted(str)
}.mkString

(expectedDiff, actualDiff)
val pad = " " * 0.max(expectedSize - expected.length)

expectedDiff + pad + " | " + actualDiff
}

private def ensureLineSeparator(str: String): String =
if str.endsWith(System.lineSeparator) then
str
else
str + System.lineSeparator

/**
* Returns a colored diffs by comparison of lines instead of tokens.
* It will automatically group subsequential pairs of `Insert` and `Delete`
* in order to improve the readability
*
* @param expected The expected lines
* @param actual The actual lines
* @return A string with colored diffs between `expected` and `actual` grouped whenever possible
*/
def mkColoredHorizontalLineDiff(expected: String, actual: String): String = {
val indent = 2
val tab = " " * indent
val insertIndent = "+" ++ (" " * (indent - 1))
val deleteIndent = "-" ++ (" " * (indent - 1))

if actual.isEmpty then
(expected.linesIterator.map(line => added(insertIndent + line)).toList :+ deleted("--- EMPTY OUTPUT ---"))
.map(ensureLineSeparator).mkString
else if expected.isEmpty then
(added("--- NO VALUE EXPECTED ---") +: actual.linesIterator.map(line => deleted(deleteIndent + line)).toList)
.map(ensureLineSeparator).mkString
else
lazy val diff = {
val expectedTokens = expected.linesWithSeparators.toArray
val actualTokens = actual.linesWithSeparators.toArray
hirschberg(actualTokens, expectedTokens)
}.toList

val transformedDiff = diff.flatMap {
case Modified(original, str) => Seq(
Inserted(ensureLineSeparator(original)), Deleted(ensureLineSeparator(str))
)
case other => Seq(other)
}

val zipped = transformedDiff zip transformedDiff.drop(1)

val (acc, inserts, deletions) = zipped.foldLeft((Seq[Patch](), Seq[Inserted](), Seq[Deleted]())): (acc, patches) =>
val (currAcc, inserts, deletions) = acc
patches match
case (currentPatch: Inserted, nextPatch: Deleted) =>
(currAcc, inserts :+ currentPatch, deletions)
case (currentPatch: Deleted, nextPatch: Inserted) =>
(currAcc, inserts, deletions :+ currentPatch)
case (currentPatch, nextPatch) =>
(currAcc :++ inserts :++ deletions :+ currentPatch, Seq.empty, Seq.empty)

val stackedDiff = acc :++ inserts :++ deletions :+ diff.last

stackedDiff.collect {
case Unmodified(str) => tab + str
case Inserted(str) => added(insertIndent + str)
case Deleted(str) => deleted(deleteIndent + str)
}.map(ensureLineSeparator).mkString

}

def mkColoredCodeDiff(code: String, lastCode: String, printDiffDel: Boolean): String = {
val tokens = splitTokens(code, Nil).toArray
val lastTokens = splitTokens(lastCode, Nil).toArray
Expand Down
10 changes: 9 additions & 1 deletion compiler/src/dotty/tools/dotc/util/SourceFile.scala
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,9 @@ import scala.collection.mutable.ArrayBuffer
import scala.util.chaining.given

import java.io.File.separator
import java.net.URI
import java.nio.charset.StandardCharsets
import java.nio.file.{FileSystemException, NoSuchFileException}
import java.nio.file.{FileSystemException, NoSuchFileException, Paths}
import java.util.Optional
import java.util.concurrent.atomic.AtomicInteger
import java.util.regex.Pattern
Expand Down Expand Up @@ -222,6 +223,13 @@ object SourceFile {
SourceFile(new VirtualFile(name.replace(separator, "/"), content.getBytes(StandardCharsets.UTF_8)), content.toCharArray)
.tap(_._maybeInComplete = maybeIncomplete)

/** A helper method to create a virtual source file for given URI.
* It relies on SourceFile#virtual implementation to create the virtual file.
*/
def virtual(uri: URI, content: String): SourceFile =
val path = Paths.get(uri).toString
SourceFile.virtual(path, content)

/** Returns the relative path of `source` within the `reference` path
*
* It returns the absolute path of `source` if it is not contained in `reference`.
Expand Down
Loading
Loading