From 15ef5549327088a0b15dcc08291fa4204333709e Mon Sep 17 00:00:00 2001 From: Martin Duhem Date: Wed, 3 Oct 2018 10:21:06 +0200 Subject: [PATCH 01/11] Add `dependencies` field to `ProjectConfig` This field will be used to restrict which projects to inspect when doing workspace-wide rename or finding all references. The code that extracts dependency information from sbt has been ported from scalacenter/bloop: https://github.com/scalacenter/bloop/blob/v1.0.0/integrations/sbt-bloop/src/main/scala/bloop/integrations/sbt/SbtBloop.scala --- .../languageserver/config/ProjectConfig.java | 7 +- .../util/server/TestServer.scala | 3 +- .../tools/sbtplugin/DottyIDEPlugin.scala | 135 +++++++++++++++++- 3 files changed, 139 insertions(+), 6 deletions(-) diff --git a/language-server/src/dotty/tools/languageserver/config/ProjectConfig.java b/language-server/src/dotty/tools/languageserver/config/ProjectConfig.java index 7d8499b2d2ea..8a60966ca2bb 100644 --- a/language-server/src/dotty/tools/languageserver/config/ProjectConfig.java +++ b/language-server/src/dotty/tools/languageserver/config/ProjectConfig.java @@ -11,6 +11,7 @@ public class ProjectConfig { public final File[] sourceDirectories; public final File[] dependencyClasspath; public final File classDirectory; + public final String[] dependencies; @JsonCreator public ProjectConfig( @@ -19,12 +20,14 @@ public ProjectConfig( @JsonProperty("compilerArguments") String[] compilerArguments, @JsonProperty("sourceDirectories") File[] sourceDirectories, @JsonProperty("dependencyClasspath") File[] dependencyClasspath, - @JsonProperty("classDirectory") File classDirectory) { + @JsonProperty("classDirectory") File classDirectory, + @JsonProperty("dependencies") String[] dependencies) { this.id = id; this.compilerVersion = compilerVersion; this.compilerArguments = compilerArguments; this.sourceDirectories = sourceDirectories; this.dependencyClasspath = dependencyClasspath; - this.classDirectory =classDirectory; + this.classDirectory = classDirectory; + this.dependencies = dependencies; } } diff --git a/language-server/test/dotty/tools/languageserver/util/server/TestServer.scala b/language-server/test/dotty/tools/languageserver/util/server/TestServer.scala index b8857a7c9241..fa95420020b6 100644 --- a/language-server/test/dotty/tools/languageserver/util/server/TestServer.scala +++ b/language-server/test/dotty/tools/languageserver/util/server/TestServer.scala @@ -59,7 +59,8 @@ class TestServer(testFolder: Path, projects: List[Project]) { | "compilerArguments" : ${showSeq(BuildInfo.ideTestsCompilerArguments)}, | "sourceDirectories" : ${showSeq(sourceDirectory(project, wipe = false) :: Nil)}, | "dependencyClasspath" : ${showSeq(dependencyClasspath(project))}, - | "classDirectory" : "${classDirectory(project, wipe = false).toString.replace('\\','/')}" + | "classDirectory" : "${classDirectory(project, wipe = false).toString.replace('\\','/')}", + | "dependencies": ${showSeq(project.dependsOn.map(_.name))} |} |""".stripMargin } diff --git a/sbt-dotty/src/dotty/tools/sbtplugin/DottyIDEPlugin.scala b/sbt-dotty/src/dotty/tools/sbtplugin/DottyIDEPlugin.scala index cfd029fb8ca2..567fad776f5e 100644 --- a/sbt-dotty/src/dotty/tools/sbtplugin/DottyIDEPlugin.scala +++ b/sbt-dotty/src/dotty/tools/sbtplugin/DottyIDEPlugin.scala @@ -269,14 +269,21 @@ object DottyIDEPlugin extends AutoPlugin { Command.process("runCode", state1) } + private def makeId(name: String, config: String): String = s"$name/$config" + private def projectConfigTask(config: Configuration): Initialize[Task[Option[ProjectConfig]]] = Def.taskDyn { val depClasspath = Attributed.data((dependencyClasspath in config).value) + val projectName = name.value // Try to detect if this is a real Scala project or not. This is pretty // fragile because sbt simply does not keep track of this information. We // could check if at least one source file ends with ".scala" but that // doesn't work for empty projects. - val isScalaProject = depClasspath.exists(_.getAbsolutePath.contains("dotty-library")) && depClasspath.exists(_.getAbsolutePath.contains("scala-library")) + val isScalaProject = ( + // Our `dotty-library` project is a Scala project + (projectName.startsWith("dotty-library") || depClasspath.exists(_.getAbsolutePath.contains("dotty-library"))) + && depClasspath.exists(_.getAbsolutePath.contains("scala-library")) + ) if (!isScalaProject) Def.task { None } else Def.task { @@ -285,11 +292,30 @@ object DottyIDEPlugin extends AutoPlugin { // step. val _ = (compile in config).value - val id = s"${thisProject.value.id}/${config.name}" + val project = thisProject.value + val id = makeId(project.id, config.name) val compilerVersion = (scalaVersion in config).value val compilerArguments = (scalacOptions in config).value val sourceDirectories = (unmanagedSourceDirectories in config).value ++ (managedSourceDirectories in config).value val classDir = (classDirectory in config).value + val extracted = Project.extract(state.value) + val settings = extracted.structure.data + + val dependencies = { + val logger = streams.value.log + // Project dependencies come from classpath deps and also inter-project config deps + // We filter out dependencies that do not compile using Dotty + val classpathProjectDependencies = + project.dependencies.filter { d => + val version = scalaVersion.in(d.project).get(settings).get + isDottyVersion(version) + }.map(d => projectDependencyName(d, config, project, logger)) + val configDependencies = + eligibleDepsFromConfig(config).value.map(c => makeId(project.id, c.name)) + + // The distinct here is important to make sure that there are no repeated project deps + (classpathProjectDependencies ++ configDependencies).distinct.toList + } Some(new ProjectConfig( id, @@ -297,7 +323,8 @@ object DottyIDEPlugin extends AutoPlugin { compilerArguments.toArray, sourceDirectories.toArray, depClasspath.toArray, - classDir + classDir, + dependencies.toArray )) } } @@ -338,4 +365,106 @@ object DottyIDEPlugin extends AutoPlugin { } ) ++ addCommandAlias("launchIDE", ";configureIDE;runCode") + + // Ported from Bloop + /** + * Detect the eligible configuration dependencies from a given configuration. + * + * A configuration is elibile if the project defines it and `bloopGenerate` + * exists for it. Otherwise, the configuration dependency is ignored. + * + * This is required to prevent transitive configurations like `Runtime` from + * generating useless bloop configuration files and possibly incorrect project + * dependencies. For example, if we didn't do this then the dependencies of + * `IntegrationTest` would be `projectName-runtime` and `projectName-compile`, + * whereas the following logic will return only the configuration `Compile` + * so that the use site of this function can create the project dep + * `projectName-compile`. + */ + private def eligibleDepsFromConfig(config: Configuration): Def.Initialize[Task[List[Configuration]]] = { + Def.task { + def depsFromConfig(configuration: Configuration): List[Configuration] = { + configuration.extendsConfigs.toList match { + case config :: Nil if config.extendsConfigs.isEmpty => config :: Nil + case config :: Nil => config :: depsFromConfig(config) + case Nil => Nil + } + } + + val configs = depsFromConfig(config) + val activeProjectConfigs = thisProject.value.configurations.toSet + + val data = settingsData.value + val thisProjectRef = Keys.thisProjectRef.value + + val eligibleConfigs = activeProjectConfigs.filter { c => + val configKey = ConfigKey.configurationToKey(c) + // Consider only configurations where the `compile` key is defined + val eligibleKey = compile in (thisProjectRef, configKey) + eligibleKey.get(data) match { + case Some(t) => + // Sbt seems to return tasks for the extended configurations (looks like a big bug) + t.info.get(taskDefinitionKey) match { + // So we now make sure that the returned config key matches the original one + case Some(taskDef) => taskDef.scope.config.toOption.toList.contains(configKey) + case None => true + } + case None => false + } + } + + configs.filter(c => eligibleConfigs.contains(c)) + } + } + + /** + * Creates a project name from a classpath dependency and its configuration. + * + * This function uses internal sbt utils (`sbt.Classpaths`) to parse configuration + * dependencies like sbt does and extract them. This parsing only supports compile + * and test, any kind of other dependency will be assumed to be test and will be + * reported to the user. + * + * Ref https://www.scala-sbt.org/1.x/docs/Library-Management.html#Configurations. + */ + private def projectDependencyName( + dep: ClasspathDep[ProjectRef], + configuration: Configuration, + project: ResolvedProject, + logger: Logger + ): String = { + val ref = dep.project + dep.configuration match { + case Some(_) => + val mapping = sbt.Classpaths.mapped( + dep.configuration, + List("compile", "test"), + List("compile", "test"), + "compile", + "*->compile" + ) + + mapping(configuration.name) match { + case Nil => + makeId(ref.project, configuration.name) + case List(conf) if Compile.name == conf => + makeId(ref.project, Compile.name) + case List(conf) if Test.name == conf => + makeId(ref.project, Test.name) + case List(conf1, conf2) if Test.name == conf1 && Compile.name == conf2 => + makeId(ref.project, Test.name) + case List(conf1, conf2) if Compile.name == conf1 && Test.name == conf2 => + makeId(ref.project, Test.name) + case unknown => + val msg = + s"Unsupported dependency '${project.id}' -> '${ref.project}:${unknown.mkString(", ")}' is understood as '${ref.project}:test'." + logger.warn(msg) + makeId(ref.project, Test.name) + } + case None => + // If no configuration, default is `Compile` dependency (see scripted tests `cross-compile-test-configuration`) + makeId(ref.project, Compile.name) + } + } + } From 5875250a7ee4fd0ff6676403ef4a947a304fb004 Mon Sep 17 00:00:00 2001 From: Martin Duhem Date: Thu, 4 Oct 2018 19:46:52 +0200 Subject: [PATCH 02/11] Support for multi-project `find all references` This commit adapts the `find all references` feature so that it is able to find references to given symbols across projects. For symbols that come from the classpath, all the projects in the build will be inspected. If we're able to find a source for the symbol, only the projects that depend on the projects where we found the definition need to be inspected. --- .../tools/dotc/interactive/Interactive.scala | 28 +++ .../dotc/interactive/InteractiveDriver.scala | 167 +++++++++++------- .../languageserver/DottyLanguageServer.scala | 84 ++++++--- .../tools/languageserver/ReferencesTest.scala | 158 +++++++++++++++++ .../util/actions/CodeReferences.scala | 8 +- .../util/server/TestServer.scala | 2 +- .../tools/sbtplugin/DottyIDEPlugin.scala | 4 + 7 files changed, 360 insertions(+), 91 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/interactive/Interactive.scala b/compiler/src/dotty/tools/dotc/interactive/Interactive.scala index 030875578aa0..8db1b263a9a2 100644 --- a/compiler/src/dotty/tools/dotc/interactive/Interactive.scala +++ b/compiler/src/dotty/tools/dotc/interactive/Interactive.scala @@ -490,4 +490,32 @@ object Interactive { } } + /** + * Given `sym`, originating from `sourceDriver`, find its representation in + * `targetDriver`. + * + * @param symbol The symbol to expression in the new driver. + * @param sourceDriver The driver from which `symbol` originates. + * @param targetDriver The driver in which we want to get a representation of `symbol`. + * @return A representation of `symbol` in `targetDriver`. + */ + def localize(symbol: Symbol, sourceDriver: InteractiveDriver, targetDriver: InteractiveDriver): Symbol = { + + def in[T](driver: InteractiveDriver)(fn: Context => T): T = + fn(driver.currentCtx) + + if (sourceDriver == targetDriver) symbol + else { + val owners = in(sourceDriver) { implicit ctx => + symbol.ownersIterator.toList.reverse.map(_.name) + } + in(targetDriver) { implicit ctx => + val base: Symbol = ctx.definitions.RootClass + owners.tail.foldLeft(base) { (prefix, symbolName) => + prefix.info.member(symbolName).symbol + } + } + } + } + } diff --git a/compiler/src/dotty/tools/dotc/interactive/InteractiveDriver.scala b/compiler/src/dotty/tools/dotc/interactive/InteractiveDriver.scala index 02fdc7acd9ec..4cf897f99dee 100644 --- a/compiler/src/dotty/tools/dotc/interactive/InteractiveDriver.scala +++ b/compiler/src/dotty/tools/dotc/interactive/InteractiveDriver.scala @@ -53,61 +53,85 @@ class InteractiveDriver(val settings: List[String]) extends Driver { def openedTrees: Map[URI, List[SourceTree]] = myOpenedTrees def compilationUnits: Map[URI, CompilationUnit] = myCompilationUnits + /** + * The trees for all the source files in this project. + * + * This includes the trees for the buffers that are presently open in the IDE, and the trees + * from the target directory. + */ + def sourceTrees(implicit ctx: Context): List[SourceTree] = sourceTreesContaining("") + + /** + * The trees for all the source files in this project that contain `id`. + * + * This includes the trees for the buffers that are presently open in the IDE, and the trees + * from the target directory. + */ + def sourceTreesContaining(id: String)(implicit ctx: Context): List[SourceTree] = { + val fromBuffers = openedTrees.values.flatten.toList + val fromCompilationOutput = { + val classNames = new mutable.ListBuffer[String] + val output = ctx.settings.outputDir.value + if (output.isDirectory) { + classesFromDir(output.jpath, classNames) + } else { + val zipFile = new ZipFile(output.file) + classesFromZip(zipFile, classNames) + } + classNames.flatMap { cls => + val className = cls.toTypeName + treesFromClassName(className, id = "") + } + } + (fromBuffers ++ fromCompilationOutput).distinct + } + + /** + * All the trees for this project. + * + * This includes the trees of the sources of this project, along with the trees that are found + * on this project's classpath. + */ def allTrees(implicit ctx: Context): List[SourceTree] = allTreesContaining("") + /** + * All the trees for this project that contain `id`. + * + * This includes the trees of the sources of this project, along with the trees that are found + * on this project's classpath. + */ def allTreesContaining(id: String)(implicit ctx: Context): List[SourceTree] = { val fromSource = openedTrees.values.flatten.toList val fromClassPath = (dirClassPathClasses ++ zipClassPathClasses).flatMap { cls => val className = cls.toTypeName - List(tree(className, id), tree(className.moduleClassName, id)).flatten + treesFromClassName(className, id) } (fromSource ++ fromClassPath).distinct } - private def tree(className: TypeName, id: String)(implicit ctx: Context): Option[SourceTree] = { - val clsd = ctx.base.staticRef(className) - clsd match { - case clsd: ClassDenotation => - clsd.ensureCompleted() - SourceTree.fromSymbol(clsd.symbol.asClass, id) - case _ => - None + /** + * The `SourceTree`s that define the class `className` and/or module `className`. + * + * @see SourceTree.fromSymbol + */ + private def treesFromClassName(className: TypeName, id: String)(implicit ctx: Context): List[SourceTree] = { + def tree(className: TypeName, id: String): Option[SourceTree] = { + val clsd = ctx.base.staticRef(className) + clsd match { + case clsd: ClassDenotation => + clsd.ensureCompleted() + SourceTree.fromSymbol(clsd.symbol.asClass, id) + case _ => + None + } } + List(tree(className, id), tree(className.moduleClassName, id)).flatten } // Presence of a file with one of these suffixes indicates that the // corresponding class has been pickled with TASTY. private val tastySuffixes = List(".hasTasty", ".tasty") - private def classNames(cp: ClassPath, packageName: String): List[String] = { - def className(classSegments: List[String]) = - classSegments.mkString(".").stripSuffix(".class") - - val ClassPathEntries(pkgs, classReps) = cp.list(packageName) - - classReps - .filter((classRep: ClassRepresentation) => classRep.binary match { - case None => - true - case Some(binFile) => - val prefix = - if (binFile.name.endsWith(".class")) - binFile.name.stripSuffix(".class") - else - null - prefix != null && { - binFile match { - case pf: PlainFile => - tastySuffixes.map(suffix => pf.givenPath.parent / (prefix + suffix)).exists(_.exists) - case _ => - sys.error(s"Unhandled file type: $binFile [getClass = ${binFile.getClass}]") - } - } - }) - .map(classRep => (packageName ++ (if (packageName != "") "." else "") ++ classRep.name)).toList ++ - pkgs.flatMap(pkg => classNames(cp, pkg.name)) - } - // FIXME: All the code doing classpath handling is very fragile and ugly, // improving this requires changing the dotty classpath APIs to handle our usecases. // We also need something like sbt server-mode to be informed of changes on @@ -128,17 +152,13 @@ class InteractiveDriver(val settings: List[String]) extends Driver { } // Like in `ZipArchiveFileLookup` we assume that zips are immutable - private val zipClassPathClasses: Seq[String] = zipClassPaths.flatMap { zipCp => - val zipFile = new ZipFile(zipCp.zipFile) - - try { - for { - entry <- zipFile.stream.toArray((size: Int) => new Array[ZipEntry](size)) - name = entry.getName - tastySuffix <- tastySuffixes.find(name.endsWith) - } yield name.replace("/", ".").stripSuffix(tastySuffix) + private val zipClassPathClasses: Seq[String] = { + val names = new mutable.ListBuffer[String] + zipClassPaths.foreach { zipCp => + val zipFile = new ZipFile(zipCp.zipFile) + classesFromZip(zipFile, names) } - finally zipFile.close() + names } // FIXME: classfiles in directories may change at any point, so we retraverse @@ -148,26 +168,43 @@ class InteractiveDriver(val settings: List[String]) extends Driver { val names = new mutable.ListBuffer[String] dirClassPaths.foreach { dirCp => val root = dirCp.dir.toPath - try - Files.walkFileTree(root, new SimpleFileVisitor[Path] { - override def visitFile(path: Path, attrs: BasicFileAttributes) = { - if (!attrs.isDirectory) { - val name = path.getFileName.toString - for { - tastySuffix <- tastySuffixes - if name.endsWith(tastySuffix) - } { - names += root.relativize(path).toString.replace("/", ".").stripSuffix(tastySuffix) - } + classesFromDir(root, names) + } + names + } + + /** Adds the names of the classes that are defined in `zipFile` to `buffer`. */ + private def classesFromZip(zipFile: ZipFile, buffer: mutable.ListBuffer[String]): Unit = { + try { + for { + entry <- zipFile.stream.toArray((size: Int) => new Array[ZipEntry](size)) + name = entry.getName + tastySuffix <- tastySuffixes.find(name.endsWith) + } buffer += name.replace("/", ".").stripSuffix(tastySuffix) + } + finally zipFile.close() + } + + /** Adds the names of the classes that are defined in `dir` to `buffer`. */ + private def classesFromDir(dir: Path, buffer: mutable.ListBuffer[String]): Unit = { + try + Files.walkFileTree(dir, new SimpleFileVisitor[Path] { + override def visitFile(path: Path, attrs: BasicFileAttributes) = { + if (!attrs.isDirectory) { + val name = path.getFileName.toString + for { + tastySuffix <- tastySuffixes + if name.endsWith(tastySuffix) + } { + buffer += dir.relativize(path).toString.replace("/", ".").stripSuffix(tastySuffix) } - FileVisitResult.CONTINUE } - }) - catch { - case _: NoSuchFileException => - } + FileVisitResult.CONTINUE + } + }) + catch { + case _: NoSuchFileException => } - names.toList } private def topLevelClassTrees(topTree: Tree, source: SourceFile): List[SourceTree] = { diff --git a/language-server/src/dotty/tools/languageserver/DottyLanguageServer.scala b/language-server/src/dotty/tools/languageserver/DottyLanguageServer.scala index 5972dcf5318d..9a6fce0d3f0b 100644 --- a/language-server/src/dotty/tools/languageserver/DottyLanguageServer.scala +++ b/language-server/src/dotty/tools/languageserver/DottyLanguageServer.scala @@ -80,6 +80,7 @@ class DottyLanguageServer extends LanguageServer val settings = defaultFlags ++ config.compilerArguments.toList + .update("-d", config.classDirectory.getAbsolutePath) .update("-classpath", (config.classDirectory +: config.dependencyClasspath).mkString(File.pathSeparator)) .update("-sourcepath", config.sourceDirectories.mkString(File.pathSeparator)) :+ "-scansource" @@ -106,21 +107,37 @@ class DottyLanguageServer extends LanguageServer private def checkMemory() = if (Memory.isCritical()) CompletableFutures.computeAsync { _ => restart() } - /** The driver instance responsible for compiling `uri` */ - def driverFor(uri: URI): InteractiveDriver = thisServer.synchronized { - val matchingConfig = + /** The configuration of the project that owns `uri`. */ + def configFor(uri: URI): ProjectConfig = thisServer.synchronized { + val config = drivers.keys.find(config => config.sourceDirectories.exists(sourceDir => new File(uri.getPath).getCanonicalPath.startsWith(sourceDir.getCanonicalPath))) - matchingConfig match { - case Some(config) => - drivers(config) - case None => - val config = drivers.keys.head - // println(s"No configuration contains $uri as a source file, arbitrarily choosing ${config.id}") - drivers(config) + + config.getOrElse { + val config = drivers.keys.head + // println(s"No configuration contains $uri as a source file, arbitrarily choosing ${config.id}") + config } } + /** The driver instance responsible for compiling `uri` */ + def driverFor(uri: URI): InteractiveDriver = { + drivers(configFor(uri)) + } + + /** The set of projects that transitively depend on `config` */ + def transitivelyDependentProjects(config: ProjectConfig): immutable.Set[ProjectConfig] = { + val allProjects = drivers.keySet.toSet + allProjects.filter(transitiveDependencies(_).contains(config)) + } + + /** The set of transitive dependencies of `config`. */ + def transitiveDependencies(config: ProjectConfig): immutable.Set[ProjectConfig] = { + val idToConfig = drivers.keys.map(k => k.id -> k).toMap + val dependencies = config.dependencies.map(idToConfig).toSet + dependencies ++ dependencies.flatMap(transitiveDependencies) + } + def connect(client: WorksheetClient): Unit = { myClient = client } @@ -281,22 +298,43 @@ class DottyLanguageServer extends LanguageServer val driver = driverFor(uri) implicit val ctx = driver.currentCtx + val includes = { + val includeDeclaration = params.getContext.isIncludeDeclaration + Include.references | Include.overriding | (if (includeDeclaration) Include.definitions else 0) + } + val pos = sourcePosition(driver, uri, params.getPosition) - val sym = Interactive.enclosingSourceSymbol(driver.openedTrees(uri), pos) + val path = Interactive.pathTo(driver.openedTrees(uri), pos) - if (sym == NoSymbol) Nil.asJava - else { - // FIXME: this will search for references in all trees on the classpath, but we really - // only need to look for trees in the target directory if the symbol is defined in the - // current project - val trees = driver.allTreesContaining(sym.name.sourceModuleName.toString) - val includeDeclaration = params.getContext.isIncludeDeclaration - val includes = - Include.references | Include.overriding | (if (includeDeclaration) Include.definitions else 0) - val refs = Interactive.findTreesMatching(trees, includes, sym) + // Find definitions of the symbol under the cursor, so that we can determine + // what projects are worth exploring + val definitions = Interactive.findDefinitions(path, driver) + val projectsToInspect = + if (definitions.isEmpty) { + drivers.keySet + } else { + for { + definition <- definitions + uri <- toUriOption(definition.pos.source).toSet + config = configFor(uri) + project <- transitivelyDependentProjects(config) + config + } yield project + } - refs.flatMap(ref => location(ref.namePos, positionMapperFor(ref.source))).asJava - } + val originalSymbol = Interactive.enclosingSourceSymbol(path) + val symbolName = originalSymbol.name.sourceModuleName.toString + val references = + for { config <- projectsToInspect.toList + remoteDriver = drivers(config) + ctx = remoteDriver.currentCtx + remoteDefinition = Interactive.localize(originalSymbol, driver, remoteDriver) + trees = remoteDriver.sourceTreesContaining(symbolName)(ctx) + reference <- Interactive.findTreesMatching(trees, includes, remoteDefinition)(ctx) + } yield { + reference + } + + references.flatMap(ref => location(ref.namePos, positionMapperFor(ref.source))).asJava } override def rename(params: RenameParams) = computeAsync { cancelToken => diff --git a/language-server/test/dotty/tools/languageserver/ReferencesTest.scala b/language-server/test/dotty/tools/languageserver/ReferencesTest.scala index 67adfc232f80..9b2425582157 100644 --- a/language-server/test/dotty/tools/languageserver/ReferencesTest.scala +++ b/language-server/test/dotty/tools/languageserver/ReferencesTest.scala @@ -59,4 +59,162 @@ class ReferencesTest { .references(m3 to m4, List(m3 to m4), withDecl = false) } + @Test def valReferencesInDifferentProject: Unit = { + val p0 = Project.withSources( + code"""object A { val ${m1}x${m2} = 1 }""" + ) + + val p1 = Project.dependingOn(p0).withSources( + code"""object B { A.${m3}x${m4} }""" + ) + + val p2 = Project.dependingOn(p0).withSources( + code"""object C { A.${m5}x${m6} }""" + ) + + withProjects(p0, p1, p2) + .references(m1 to m2, List(m1 to m2, m3 to m4, m5 to m6), withDecl = true) + .references(m1 to m2, List(m3 to m4, m5 to m6), withDecl = false) + .references(m3 to m4, List(m1 to m2, m3 to m4, m5 to m6), withDecl = true) + .references(m3 to m4, List(m3 to m4, m5 to m6), withDecl = false) + .references(m5 to m6, List(m1 to m2, m3 to m4, m5 to m6), withDecl = true) + .references(m5 to m6, List(m3 to m4, m5 to m6), withDecl = false) + } + + @Test def valReferencesInDifferentProjectNoDef: Unit = { + val p0 = Project.withSources( + code"""object A { new java.util.${m1}ArrayList${m2}[Int] }""" + ) + + val p1 = Project.withSources( + code"""object B { new java.util.${m3}ArrayList${m4}[Int] }""" + ) + + val p2 = Project.withSources( + code"""object C { new java.util.${m5}ArrayList${m6}[Int] }""" + ) + + withProjects(p0, p1, p2) + .references(m1 to m2, List(m1 to m2, m3 to m4, m5 to m6), withDecl = true) + .references(m1 to m2, List(m1 to m2, m3 to m4, m5 to m6), withDecl = false) + .references(m3 to m4, List(m1 to m2, m3 to m4, m5 to m6), withDecl = true) + .references(m3 to m4, List(m1 to m2, m3 to m4, m5 to m6), withDecl = false) + .references(m5 to m6, List(m1 to m2, m3 to m4, m5 to m6), withDecl = true) + .references(m5 to m6, List(m1 to m2, m3 to m4, m5 to m6), withDecl = false) + } + + @Test def moduleReferencesInDifferentProject: Unit = { + val p0 = Project.withSources( + code"""object ${m1}A${m2}""" + ) + + val p1 = Project.dependingOn(p0).withSources( + code"""class B { ${m3}A${m4} }""" + ) + + withProjects(p0, p1) + .references(m1 to m2, List(m1 to m2, m3 to m4), withDecl = true) + .references(m1 to m2, List(m3 to m4), withDecl = false) + .references(m3 to m4, List(m1 to m2, m3 to m4), withDecl = true) + .references(m3 to m4, List(m3 to m4), withDecl = false) + } + + @Test def classReferencesInDifferentProject: Unit = { + val p0 = Project.withSources( + code"""class ${m1}A${m2}""" + ) + + val p1 = Project.dependingOn(p0).withSources( + code"""class B extends ${m3}A${m4}""" + ) + + val p2 = Project.dependingOn(p0).withSources( + code"""class C { new ${m5}A${m6} }""" + ) + + withProjects(p0, p1, p2) + .references(m1 to m2, List(m1 to m2, m3 to m4, m5 to m6), withDecl = true) + .references(m1 to m2, List(m3 to m4, m5 to m6), withDecl = false) + .references(m3 to m4, List(m1 to m2, m3 to m4, m5 to m6), withDecl = true) + .references(m3 to m4, List(m3 to m4, m5 to m6), withDecl = false) + .references(m5 to m6, List(m1 to m2, m3 to m4, m5 to m6), withDecl = true) + .references(m5 to m6, List(m3 to m4, m5 to m6), withDecl = false) + } + + @Test def defReferencesInDifferentProject: Unit = { + val p0 = Project.withSources( + code"""object A { def ${m1}x${m2} = 1 }""" + ) + + val p1 = Project.dependingOn(p0).withSources( + code"""object B { A.${m3}x${m4} }""" + ) + + val p2 = Project.dependingOn(p0).withSources( + code"""object C { A.${m5}x${m6} }""" + ) + + withProjects(p0, p1, p2) + .references(m1 to m2, List(m1 to m2, m3 to m4, m5 to m6), withDecl = true) + .references(m1 to m2, List(m3 to m4, m5 to m6), withDecl = false) + .references(m3 to m4, List(m1 to m2, m3 to m4, m5 to m6), withDecl = true) + .references(m3 to m4, List(m3 to m4, m5 to m6), withDecl = false) + .references(m5 to m6, List(m1 to m2, m3 to m4, m5 to m6), withDecl = true) + .references(m5 to m6, List(m3 to m4, m5 to m6), withDecl = false) + } + + @Test def deeplyNestedValReferencesInDifferentProject: Unit = { + val p0 = Project.withSources( + code"""class A { class Z { class Y { class X { val ${m1}x${m2} = 1 } } } }""" + ) + + val p1 = Project.dependingOn(p0).withSources( + code"""class B { + val a = new A() + val z = new a.Z() + val y = new z.Y() + val x = new y.X() + x.${m3}x${m4} + }""" + ) + + withProjects(p0, p1) + .references(m1 to m2, List(m1 to m2, m3 to m4), withDecl = true) + .references(m1 to m2, List(m3 to m4), withDecl = false) + .references(m3 to m4, List(m1 to m2, m3 to m4), withDecl = true) + .references(m3 to m4, List(m3 to m4), withDecl = false) + } + + @Test def deeplyNestedStaticValReferencesInDifferentProject: Unit = { + val p0 = Project.withSources( + code"""object A { object Z { object Y { object X { val ${m1}x${m2} = 1 } } } }""" + ) + + val p1 = Project.dependingOn(p0).withSources( + code"""object B { A.Z.Y.X.${m3}x${m4} }""" + ) + + withProjects(p0, p1) + .references(m1 to m2, List(m1 to m2, m3 to m4), withDecl = true) + .references(m1 to m2, List(m3 to m4), withDecl = false) + .references(m3 to m4, List(m1 to m2, m3 to m4), withDecl = true) + .references(m3 to m4, List(m3 to m4), withDecl = false) + } + + @Test def findReferencesInUntouchedProject: Unit = { + val p0 = Project.withSources( + code"""package hello + object A { def ${m1}foo${m2} = 1 }""" + ) + + val p1 = Project.dependingOn(p0).withSources( + tasty"""package hello + object B { def bar = A.${m3}foo${m4} }""" + ) + + withProjects(p0, p1) + .references(m1 to m2, List(m1 to m2, m3 to m4), withDecl = true) + .references(m1 to m2, List(m3 to m4), withDecl = false) + } + } diff --git a/language-server/test/dotty/tools/languageserver/util/actions/CodeReferences.scala b/language-server/test/dotty/tools/languageserver/util/actions/CodeReferences.scala index 758a2b0e43fc..8bd213e24dd7 100644 --- a/language-server/test/dotty/tools/languageserver/util/actions/CodeReferences.scala +++ b/language-server/test/dotty/tools/languageserver/util/actions/CodeReferences.scala @@ -5,6 +5,8 @@ import dotty.tools.languageserver.util.{CodeRange, PositionContext} import org.junit.Assert.assertEquals +import org.eclipse.lsp4j.Location + import scala.collection.JavaConverters._ /** @@ -19,9 +21,11 @@ class CodeReferences(override val range: CodeRange, expected: List[CodeRange], withDecl: Boolean) extends ActionOnRange { + private implicit val LocationOrdering: Ordering[Location] = Ordering.by(_.toString) + override def onMarker(marker: CodeMarker): Exec[Unit] = { - val expectedLocations = expected.map(_.toLocation) - val results = server.references(marker.toReferenceParams(withDecl)).get().asScala + val expectedLocations = expected.map(_.toLocation).sorted + val results = server.references(marker.toReferenceParams(withDecl)).get().asScala.sorted assertEquals(expectedLocations, results) } diff --git a/language-server/test/dotty/tools/languageserver/util/server/TestServer.scala b/language-server/test/dotty/tools/languageserver/util/server/TestServer.scala index fa95420020b6..86ebcd4d170b 100644 --- a/language-server/test/dotty/tools/languageserver/util/server/TestServer.scala +++ b/language-server/test/dotty/tools/languageserver/util/server/TestServer.scala @@ -107,8 +107,8 @@ class TestServer(testFolder: Path, projects: List[Project]) { val path = testFolder.resolve(project.name).resolve("out") if (wipe) { Directory(path).deleteRecursively() - Files.createDirectories(path) } + Files.createDirectories(path) path.toAbsolutePath } diff --git a/sbt-dotty/src/dotty/tools/sbtplugin/DottyIDEPlugin.scala b/sbt-dotty/src/dotty/tools/sbtplugin/DottyIDEPlugin.scala index 567fad776f5e..e6dc2f51038f 100644 --- a/sbt-dotty/src/dotty/tools/sbtplugin/DottyIDEPlugin.scala +++ b/sbt-dotty/src/dotty/tools/sbtplugin/DottyIDEPlugin.scala @@ -317,6 +317,10 @@ object DottyIDEPlugin extends AutoPlugin { (classpathProjectDependencies ++ configDependencies).distinct.toList } + // For projects without sources, we need to create it. Otherwise `InteractiveDriver` + // complains that the target directory doesn't exist. + if (!classDir.exists) IO.createDirectory(classDir) + Some(new ProjectConfig( id, compilerVersion, From fa1260f77c8173b0637d1288f4280409f40fec56 Mon Sep 17 00:00:00 2001 From: Martin Duhem Date: Fri, 5 Oct 2018 11:42:05 +0200 Subject: [PATCH 03/11] Cache computation of dependent projects --- .../languageserver/DottyLanguageServer.scala | 33 ++++++++++++------- 1 file changed, 21 insertions(+), 12 deletions(-) diff --git a/language-server/src/dotty/tools/languageserver/DottyLanguageServer.scala b/language-server/src/dotty/tools/languageserver/DottyLanguageServer.scala index 9a6fce0d3f0b..10c5b20ef9e5 100644 --- a/language-server/src/dotty/tools/languageserver/DottyLanguageServer.scala +++ b/language-server/src/dotty/tools/languageserver/DottyLanguageServer.scala @@ -23,7 +23,7 @@ import Comments._, Contexts._, Flags._, Names._, NameOps._, Symbols._, SymDenota import classpath.ClassPathEntries import reporting._, reporting.diagnostic.{Message, MessageContainer, messages} import typer.Typer -import util._ +import util.{Set => _, _} import interactive._, interactive.InteractiveDriver._ import Interactive.Include import config.Printers.interactiv @@ -60,6 +60,8 @@ class DottyLanguageServer extends LanguageServer private[this] var myDrivers: mutable.Map[ProjectConfig, InteractiveDriver] = _ + private[this] var myDependentProjects: mutable.Map[ProjectConfig, mutable.Set[ProjectConfig]] = _ + def drivers: Map[ProjectConfig, InteractiveDriver] = thisServer.synchronized { if (myDrivers == null) { assert(rootUri != null, "`drivers` cannot be called before `initialize`") @@ -125,17 +127,24 @@ class DottyLanguageServer extends LanguageServer drivers(configFor(uri)) } - /** The set of projects that transitively depend on `config` */ - def transitivelyDependentProjects(config: ProjectConfig): immutable.Set[ProjectConfig] = { - val allProjects = drivers.keySet.toSet - allProjects.filter(transitiveDependencies(_).contains(config)) - } + /** A mapping from project `p` to the set of projects that transitively depend on `p`. */ + def dependentProjects: Map[ProjectConfig, Set[ProjectConfig]] = thisServer.synchronized { + if (myDependentProjects == null) { + val idToConfig = drivers.keys.map(k => k.id -> k).toMap + val allProjects = drivers.keySet + + def transitiveDependencies(config: ProjectConfig): Set[ProjectConfig] = { + val dependencies = config.dependencies.map(idToConfig).toSet + dependencies ++ dependencies.flatMap(transitiveDependencies) + } - /** The set of transitive dependencies of `config`. */ - def transitiveDependencies(config: ProjectConfig): immutable.Set[ProjectConfig] = { - val idToConfig = drivers.keys.map(k => k.id -> k).toMap - val dependencies = config.dependencies.map(idToConfig).toSet - dependencies ++ dependencies.flatMap(transitiveDependencies) + myDependentProjects = new mutable.HashMap().withDefaultValue(mutable.Set.empty) + for { project <- allProjects + dependency <- transitiveDependencies(project) } { + myDependentProjects(dependency) += project + } + } + myDependentProjects } def connect(client: WorksheetClient): Unit = { @@ -317,7 +326,7 @@ class DottyLanguageServer extends LanguageServer definition <- definitions uri <- toUriOption(definition.pos.source).toSet config = configFor(uri) - project <- transitivelyDependentProjects(config) + config + project <- dependentProjects(config) + config } yield project } From a49b25dd5b5ab4e4fe73d53548ad792c6be21270 Mon Sep 17 00:00:00 2001 From: Martin Duhem Date: Thu, 1 Nov 2018 16:55:30 +0100 Subject: [PATCH 04/11] Move code around in InteractiveDriver (This commit contains no functional change) --- .../dotc/interactive/InteractiveDriver.scala | 153 +++++++++--------- 1 file changed, 76 insertions(+), 77 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/interactive/InteractiveDriver.scala b/compiler/src/dotty/tools/dotc/interactive/InteractiveDriver.scala index 4cf897f99dee..ef6ea7638325 100644 --- a/compiler/src/dotty/tools/dotc/interactive/InteractiveDriver.scala +++ b/compiler/src/dotty/tools/dotc/interactive/InteractiveDriver.scala @@ -36,23 +36,56 @@ class InteractiveDriver(val settings: List[String]) extends Driver { } private[this] var myCtx: Context = myInitCtx - def currentCtx: Context = myCtx + private val compiler: Compiler = new InteractiveCompiler + private val myOpenedFiles = new mutable.LinkedHashMap[URI, SourceFile] { override def default(key: URI) = NoSource } + def openedFiles: Map[URI, SourceFile] = myOpenedFiles private val myOpenedTrees = new mutable.LinkedHashMap[URI, List[SourceTree]] { override def default(key: URI) = Nil } + def openedTrees: Map[URI, List[SourceTree]] = myOpenedTrees private val myCompilationUnits = new mutable.LinkedHashMap[URI, CompilationUnit] - - def openedFiles: Map[URI, SourceFile] = myOpenedFiles - def openedTrees: Map[URI, List[SourceTree]] = myOpenedTrees def compilationUnits: Map[URI, CompilationUnit] = myCompilationUnits + // Presence of a file with one of these suffixes indicates that the + // corresponding class has been pickled with TASTY. + private val tastySuffixes = List(".hasTasty", ".tasty") + + // FIXME: All the code doing classpath handling is very fragile and ugly, + // improving this requires changing the dotty classpath APIs to handle our usecases. + // We also need something like sbt server-mode to be informed of changes on + // the classpath. + + private val (zipClassPaths, dirClassPaths) = currentCtx.platform.classPath(currentCtx) match { + case AggregateClassPath(cps) => + // FIXME: We shouldn't assume that ClassPath doesn't have other + // subclasses. For now, the only other subclass is JrtClassPath on Java + // 9+, we can safely ignore it for now because it's only used for the + // standard Java library, but this will change once we start supporting + // adding entries to the modulepath. + val zipCps = cps.collect { case cp: ZipArchiveFileLookup[_] => cp } + val dirCps = cps.collect { case cp: JFileDirectoryLookup[_] => cp } + (zipCps, dirCps) + case _ => + (Seq(), Seq()) + } + + // Like in `ZipArchiveFileLookup` we assume that zips are immutable + private val zipClassPathClasses: Seq[String] = { + val names = new mutable.ListBuffer[String] + zipClassPaths.foreach { zipCp => + val zipFile = new ZipFile(zipCp.zipFile) + classesFromZip(zipFile, names) + } + names + } + /** * The trees for all the source files in this project. * @@ -109,6 +142,45 @@ class InteractiveDriver(val settings: List[String]) extends Driver { (fromSource ++ fromClassPath).distinct } + def run(uri: URI, sourceCode: String): List[MessageContainer] = run(uri, toSource(uri, sourceCode)) + + def run(uri: URI, source: SourceFile): List[MessageContainer] = { + val previousCtx = myCtx + try { + val reporter = + new StoreReporter(null) with UniqueMessagePositions with HideNonSensicalMessages + + val run = compiler.newRun(myInitCtx.fresh.setReporter(reporter)) + myCtx = run.runContext + + implicit val ctx = myCtx + + myOpenedFiles(uri) = source + + run.compileSources(List(source)) + run.printSummary() + val unit = ctx.run.units.head + val t = unit.tpdTree + cleanup(t) + myOpenedTrees(uri) = topLevelClassTrees(t, source) + myCompilationUnits(uri) = unit + + reporter.removeBufferedMessages + } + catch { + case ex: FatalError => + myCtx = previousCtx + close(uri) + Nil + } + } + + def close(uri: URI): Unit = { + myOpenedFiles.remove(uri) + myOpenedTrees.remove(uri) + myCompilationUnits.remove(uri) + } + /** * The `SourceTree`s that define the class `className` and/or module `className`. * @@ -128,39 +200,6 @@ class InteractiveDriver(val settings: List[String]) extends Driver { List(tree(className, id), tree(className.moduleClassName, id)).flatten } - // Presence of a file with one of these suffixes indicates that the - // corresponding class has been pickled with TASTY. - private val tastySuffixes = List(".hasTasty", ".tasty") - - // FIXME: All the code doing classpath handling is very fragile and ugly, - // improving this requires changing the dotty classpath APIs to handle our usecases. - // We also need something like sbt server-mode to be informed of changes on - // the classpath. - - private val (zipClassPaths, dirClassPaths) = currentCtx.platform.classPath(currentCtx) match { - case AggregateClassPath(cps) => - // FIXME: We shouldn't assume that ClassPath doesn't have other - // subclasses. For now, the only other subclass is JrtClassPath on Java - // 9+, we can safely ignore it for now because it's only used for the - // standard Java library, but this will change once we start supporting - // adding entries to the modulepath. - val zipCps = cps.collect { case cp: ZipArchiveFileLookup[_] => cp } - val dirCps = cps.collect { case cp: JFileDirectoryLookup[_] => cp } - (zipCps, dirCps) - case _ => - (Seq(), Seq()) - } - - // Like in `ZipArchiveFileLookup` we assume that zips are immutable - private val zipClassPathClasses: Seq[String] = { - val names = new mutable.ListBuffer[String] - zipClassPaths.foreach { zipCp => - val zipFile = new ZipFile(zipCp.zipFile) - classesFromZip(zipFile, names) - } - names - } - // FIXME: classfiles in directories may change at any point, so we retraverse // the directories each time, if we knew when classfiles changed (sbt // server-mode might help here), we could do cache invalidation instead. @@ -222,8 +261,6 @@ class InteractiveDriver(val settings: List[String]) extends Driver { trees.toList } - private val compiler: Compiler = new InteractiveCompiler - /** Remove attachments and error out completers. The goal is to avoid * having a completer hanging in a typed tree which can capture the context * of a previous run. Note that typed trees can have untyped or partially @@ -261,44 +298,6 @@ class InteractiveDriver(val settings: List[String]) extends Driver { new SourceFile(virtualFile, Codec.UTF8) } - def run(uri: URI, sourceCode: String): List[MessageContainer] = run(uri, toSource(uri, sourceCode)) - - def run(uri: URI, source: SourceFile): List[MessageContainer] = { - val previousCtx = myCtx - try { - val reporter = - new StoreReporter(null) with UniqueMessagePositions with HideNonSensicalMessages - - val run = compiler.newRun(myInitCtx.fresh.setReporter(reporter)) - myCtx = run.runContext - - implicit val ctx = myCtx - - myOpenedFiles(uri) = source - - run.compileSources(List(source)) - run.printSummary() - val unit = ctx.run.units.head - val t = unit.tpdTree - cleanup(t) - myOpenedTrees(uri) = topLevelClassTrees(t, source) - myCompilationUnits(uri) = unit - - reporter.removeBufferedMessages - } - catch { - case ex: FatalError => - myCtx = previousCtx - close(uri) - Nil - } - } - - def close(uri: URI): Unit = { - myOpenedFiles.remove(uri) - myOpenedTrees.remove(uri) - myCompilationUnits.remove(uri) - } } object InteractiveDriver { From 786b0bcdb51dac445b297038ecd5e81255c3aa4c Mon Sep 17 00:00:00 2001 From: Martin Duhem Date: Thu, 1 Nov 2018 17:06:17 +0100 Subject: [PATCH 05/11] Compile an empty source in InteractiveDriver ctor This is necessary because we may be using this InteractiveDriver before asking it to compile anything (which would have initialized it). This happens, for instance, if we use this Driver to resolve a symbol coming from a different Driver. Drivers are not meant to be used before being initialized and can crash. --- .../tools/dotc/interactive/InteractiveDriver.scala | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/compiler/src/dotty/tools/dotc/interactive/InteractiveDriver.scala b/compiler/src/dotty/tools/dotc/interactive/InteractiveDriver.scala index ef6ea7638325..d211a03b8145 100644 --- a/compiler/src/dotty/tools/dotc/interactive/InteractiveDriver.scala +++ b/compiler/src/dotty/tools/dotc/interactive/InteractiveDriver.scala @@ -86,6 +86,8 @@ class InteractiveDriver(val settings: List[String]) extends Driver { names } + initialize() + /** * The trees for all the source files in this project. * @@ -298,6 +300,18 @@ class InteractiveDriver(val settings: List[String]) extends Driver { new SourceFile(virtualFile, Codec.UTF8) } + /** + * Initialize this driver and compiler by "compiling" a fake, empty source file. + * + * This is necessary because an `InteractiveDriver` can be put to work without having + * compiled anything (for instance, resolving a symbol coming from a different compiler in + * this compiler). In those cases, an un-initialized compiler will crash. + */ + private[this] def initialize(): Unit = { + val fakeSource = new File("fake.scala") + run(fakeSource.toURI, "") + } + } object InteractiveDriver { From 5c6651f219217db6d4603322b6917efca527bf01 Mon Sep 17 00:00:00 2001 From: Martin Duhem Date: Thu, 1 Nov 2018 17:16:00 +0100 Subject: [PATCH 06/11] Find references in projects in parallel --- .../languageserver/DottyLanguageServer.scala | 25 +++++++++++-------- 1 file changed, 15 insertions(+), 10 deletions(-) diff --git a/language-server/src/dotty/tools/languageserver/DottyLanguageServer.scala b/language-server/src/dotty/tools/languageserver/DottyLanguageServer.scala index 10c5b20ef9e5..a7cbc7f30bfe 100644 --- a/language-server/src/dotty/tools/languageserver/DottyLanguageServer.scala +++ b/language-server/src/dotty/tools/languageserver/DottyLanguageServer.scala @@ -332,18 +332,23 @@ class DottyLanguageServer extends LanguageServer val originalSymbol = Interactive.enclosingSourceSymbol(path) val symbolName = originalSymbol.name.sourceModuleName.toString - val references = - for { config <- projectsToInspect.toList - remoteDriver = drivers(config) - ctx = remoteDriver.currentCtx - remoteDefinition = Interactive.localize(originalSymbol, driver, remoteDriver) - trees = remoteDriver.sourceTreesContaining(symbolName)(ctx) - reference <- Interactive.findTreesMatching(trees, includes, remoteDefinition)(ctx) - } yield { - reference + val references = { + // Collect the information necessary to look into each project separately: representation of + // `originalSymbol` in this project, the context and correct Driver. + val perProjectInfo = projectsToInspect.toList.map { config => + val remoteDriver = drivers(config) + val ctx = remoteDriver.currentCtx + val definition = Interactive.localize(originalSymbol, driver, remoteDriver) + (remoteDriver, ctx, definition) } - references.flatMap(ref => location(ref.namePos, positionMapperFor(ref.source))).asJava + perProjectInfo.par.flatMap { (remoteDriver, ctx, definition) => + val trees = remoteDriver.sourceTreesContaining(symbolName)(ctx) + Interactive.findTreesMatching(trees, includes, definition)(ctx) + } + }.toList + + references.flatMap(ref => location(ref.namePos, positionMapperFor(ref.source))).asJava } override def rename(params: RenameParams) = computeAsync { cancelToken => From 4d3a1047e257b483063f6d42122ea638c5d58444 Mon Sep 17 00:00:00 2001 From: Martin Duhem Date: Fri, 9 Nov 2018 16:19:19 +0100 Subject: [PATCH 07/11] Rename `dependencies` to `projectDependencies` --- .../dotty/tools/languageserver/DottyLanguageServer.scala | 2 +- .../dotty/tools/languageserver/config/ProjectConfig.java | 6 +++--- .../dotty/tools/languageserver/util/server/TestServer.scala | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/language-server/src/dotty/tools/languageserver/DottyLanguageServer.scala b/language-server/src/dotty/tools/languageserver/DottyLanguageServer.scala index a7cbc7f30bfe..d9960609d921 100644 --- a/language-server/src/dotty/tools/languageserver/DottyLanguageServer.scala +++ b/language-server/src/dotty/tools/languageserver/DottyLanguageServer.scala @@ -134,7 +134,7 @@ class DottyLanguageServer extends LanguageServer val allProjects = drivers.keySet def transitiveDependencies(config: ProjectConfig): Set[ProjectConfig] = { - val dependencies = config.dependencies.map(idToConfig).toSet + val dependencies = config.projectDependencies.map(idToConfig).toSet dependencies ++ dependencies.flatMap(transitiveDependencies) } diff --git a/language-server/src/dotty/tools/languageserver/config/ProjectConfig.java b/language-server/src/dotty/tools/languageserver/config/ProjectConfig.java index 8a60966ca2bb..e1f55554dc42 100644 --- a/language-server/src/dotty/tools/languageserver/config/ProjectConfig.java +++ b/language-server/src/dotty/tools/languageserver/config/ProjectConfig.java @@ -11,7 +11,7 @@ public class ProjectConfig { public final File[] sourceDirectories; public final File[] dependencyClasspath; public final File classDirectory; - public final String[] dependencies; + public final String[] projectDependencies; @JsonCreator public ProjectConfig( @@ -21,13 +21,13 @@ public ProjectConfig( @JsonProperty("sourceDirectories") File[] sourceDirectories, @JsonProperty("dependencyClasspath") File[] dependencyClasspath, @JsonProperty("classDirectory") File classDirectory, - @JsonProperty("dependencies") String[] dependencies) { + @JsonProperty("projectDependencies") String[] projectDependencies) { this.id = id; this.compilerVersion = compilerVersion; this.compilerArguments = compilerArguments; this.sourceDirectories = sourceDirectories; this.dependencyClasspath = dependencyClasspath; this.classDirectory = classDirectory; - this.dependencies = dependencies; + this.projectDependencies = projectDependencies; } } diff --git a/language-server/test/dotty/tools/languageserver/util/server/TestServer.scala b/language-server/test/dotty/tools/languageserver/util/server/TestServer.scala index 86ebcd4d170b..25a8ca417d4e 100644 --- a/language-server/test/dotty/tools/languageserver/util/server/TestServer.scala +++ b/language-server/test/dotty/tools/languageserver/util/server/TestServer.scala @@ -60,7 +60,7 @@ class TestServer(testFolder: Path, projects: List[Project]) { | "sourceDirectories" : ${showSeq(sourceDirectory(project, wipe = false) :: Nil)}, | "dependencyClasspath" : ${showSeq(dependencyClasspath(project))}, | "classDirectory" : "${classDirectory(project, wipe = false).toString.replace('\\','/')}", - | "dependencies": ${showSeq(project.dependsOn.map(_.name))} + | "projectDependencies": ${showSeq(project.dependsOn.map(_.name))} |} |""".stripMargin } From fb7d82c70975a87ee4d138df118f18715f43ce16 Mon Sep 17 00:00:00 2001 From: Martin Duhem Date: Fri, 9 Nov 2018 16:53:40 +0100 Subject: [PATCH 08/11] Address review comments --- .../tools/dotc/interactive/Interactive.scala | 3 ++- .../dotc/interactive/InteractiveDriver.scala | 24 +++++++++---------- .../tools/sbtplugin/DottyIDEPlugin.scala | 8 +++---- 3 files changed, 17 insertions(+), 18 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/interactive/Interactive.scala b/compiler/src/dotty/tools/dotc/interactive/Interactive.scala index 8db1b263a9a2..e30a0b587e37 100644 --- a/compiler/src/dotty/tools/dotc/interactive/Interactive.scala +++ b/compiler/src/dotty/tools/dotc/interactive/Interactive.scala @@ -512,7 +512,8 @@ object Interactive { in(targetDriver) { implicit ctx => val base: Symbol = ctx.definitions.RootClass owners.tail.foldLeft(base) { (prefix, symbolName) => - prefix.info.member(symbolName).symbol + if (prefix.exists) prefix.info.member(symbolName).symbol + else NoSymbol } } } diff --git a/compiler/src/dotty/tools/dotc/interactive/InteractiveDriver.scala b/compiler/src/dotty/tools/dotc/interactive/InteractiveDriver.scala index d211a03b8145..cccc04d69acc 100644 --- a/compiler/src/dotty/tools/dotc/interactive/InteractiveDriver.scala +++ b/compiler/src/dotty/tools/dotc/interactive/InteractiveDriver.scala @@ -77,8 +77,8 @@ class InteractiveDriver(val settings: List[String]) extends Driver { } // Like in `ZipArchiveFileLookup` we assume that zips are immutable - private val zipClassPathClasses: Seq[String] = { - val names = new mutable.ListBuffer[String] + private val zipClassPathClasses: Seq[TypeName] = { + val names = new mutable.ListBuffer[TypeName] zipClassPaths.foreach { zipCp => val zipFile = new ZipFile(zipCp.zipFile) classesFromZip(zipFile, names) @@ -105,7 +105,7 @@ class InteractiveDriver(val settings: List[String]) extends Driver { def sourceTreesContaining(id: String)(implicit ctx: Context): List[SourceTree] = { val fromBuffers = openedTrees.values.flatten.toList val fromCompilationOutput = { - val classNames = new mutable.ListBuffer[String] + val classNames = new mutable.ListBuffer[TypeName] val output = ctx.settings.outputDir.value if (output.isDirectory) { classesFromDir(output.jpath, classNames) @@ -114,8 +114,7 @@ class InteractiveDriver(val settings: List[String]) extends Driver { classesFromZip(zipFile, classNames) } classNames.flatMap { cls => - val className = cls.toTypeName - treesFromClassName(className, id = "") + treesFromClassName(cls, id) } } (fromBuffers ++ fromCompilationOutput).distinct @@ -138,8 +137,7 @@ class InteractiveDriver(val settings: List[String]) extends Driver { def allTreesContaining(id: String)(implicit ctx: Context): List[SourceTree] = { val fromSource = openedTrees.values.flatten.toList val fromClassPath = (dirClassPathClasses ++ zipClassPathClasses).flatMap { cls => - val className = cls.toTypeName - treesFromClassName(className, id) + treesFromClassName(cls, id) } (fromSource ++ fromClassPath).distinct } @@ -205,8 +203,8 @@ class InteractiveDriver(val settings: List[String]) extends Driver { // FIXME: classfiles in directories may change at any point, so we retraverse // the directories each time, if we knew when classfiles changed (sbt // server-mode might help here), we could do cache invalidation instead. - private def dirClassPathClasses: Seq[String] = { - val names = new mutable.ListBuffer[String] + private def dirClassPathClasses: Seq[TypeName] = { + val names = new mutable.ListBuffer[TypeName] dirClassPaths.foreach { dirCp => val root = dirCp.dir.toPath classesFromDir(root, names) @@ -215,19 +213,19 @@ class InteractiveDriver(val settings: List[String]) extends Driver { } /** Adds the names of the classes that are defined in `zipFile` to `buffer`. */ - private def classesFromZip(zipFile: ZipFile, buffer: mutable.ListBuffer[String]): Unit = { + private def classesFromZip(zipFile: ZipFile, buffer: mutable.ListBuffer[TypeName]): Unit = { try { for { entry <- zipFile.stream.toArray((size: Int) => new Array[ZipEntry](size)) name = entry.getName tastySuffix <- tastySuffixes.find(name.endsWith) - } buffer += name.replace("/", ".").stripSuffix(tastySuffix) + } buffer += name.replace("/", ".").stripSuffix(tastySuffix).toTypeName } finally zipFile.close() } /** Adds the names of the classes that are defined in `dir` to `buffer`. */ - private def classesFromDir(dir: Path, buffer: mutable.ListBuffer[String]): Unit = { + private def classesFromDir(dir: Path, buffer: mutable.ListBuffer[TypeName]): Unit = { try Files.walkFileTree(dir, new SimpleFileVisitor[Path] { override def visitFile(path: Path, attrs: BasicFileAttributes) = { @@ -237,7 +235,7 @@ class InteractiveDriver(val settings: List[String]) extends Driver { tastySuffix <- tastySuffixes if name.endsWith(tastySuffix) } { - buffer += dir.relativize(path).toString.replace("/", ".").stripSuffix(tastySuffix) + buffer += dir.relativize(path).toString.replace("/", ".").stripSuffix(tastySuffix).toTypeName } } FileVisitResult.CONTINUE diff --git a/sbt-dotty/src/dotty/tools/sbtplugin/DottyIDEPlugin.scala b/sbt-dotty/src/dotty/tools/sbtplugin/DottyIDEPlugin.scala index e6dc2f51038f..4f2610362a7e 100644 --- a/sbt-dotty/src/dotty/tools/sbtplugin/DottyIDEPlugin.scala +++ b/sbt-dotty/src/dotty/tools/sbtplugin/DottyIDEPlugin.scala @@ -374,16 +374,16 @@ object DottyIDEPlugin extends AutoPlugin { /** * Detect the eligible configuration dependencies from a given configuration. * - * A configuration is elibile if the project defines it and `bloopGenerate` + * A configuration is eligible if the project defines it and `compile` * exists for it. Otherwise, the configuration dependency is ignored. * * This is required to prevent transitive configurations like `Runtime` from - * generating useless bloop configuration files and possibly incorrect project + * generating useless IDE configuration files and possibly incorrect project * dependencies. For example, if we didn't do this then the dependencies of - * `IntegrationTest` would be `projectName-runtime` and `projectName-compile`, + * `IntegrationTest` would be `projectName/runtime` and `projectName/compile`, * whereas the following logic will return only the configuration `Compile` * so that the use site of this function can create the project dep - * `projectName-compile`. + * `projectName/compile`. */ private def eligibleDepsFromConfig(config: Configuration): Def.Initialize[Task[List[Configuration]]] = { Def.task { From 7b07ad94ba49bda0806c9ae1ccba680d1da7250b Mon Sep 17 00:00:00 2001 From: Martin Duhem Date: Mon, 12 Nov 2018 11:42:22 +0100 Subject: [PATCH 09/11] Cleaner initialization of InteractiveDriver --- .../tools/dotc/interactive/InteractiveDriver.scala | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/interactive/InteractiveDriver.scala b/compiler/src/dotty/tools/dotc/interactive/InteractiveDriver.scala index cccc04d69acc..0e4c38fddf30 100644 --- a/compiler/src/dotty/tools/dotc/interactive/InteractiveDriver.scala +++ b/compiler/src/dotty/tools/dotc/interactive/InteractiveDriver.scala @@ -299,15 +299,17 @@ class InteractiveDriver(val settings: List[String]) extends Driver { } /** - * Initialize this driver and compiler by "compiling" a fake, empty source file. + * Initialize this driver and compiler. * * This is necessary because an `InteractiveDriver` can be put to work without having * compiled anything (for instance, resolving a symbol coming from a different compiler in - * this compiler). In those cases, an un-initialized compiler will crash. + * this compiler). In those cases, an un-initialized compiler may crash (for instance if + * late-compilation is needed). */ private[this] def initialize(): Unit = { - val fakeSource = new File("fake.scala") - run(fakeSource.toURI, "") + val run = compiler.newRun(myInitCtx.fresh) + myCtx = run.runContext + run.compileUnits(Nil, myCtx) } } From c23a304d853320c76508038bf3c3452035239603 Mon Sep 17 00:00:00 2001 From: Martin Duhem Date: Mon, 12 Nov 2018 11:43:13 +0100 Subject: [PATCH 10/11] Don't find references in parallel It appears that the `TreeUnpickler` is not able to be run in parallel. --- .../src/dotty/tools/languageserver/DottyLanguageServer.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/language-server/src/dotty/tools/languageserver/DottyLanguageServer.scala b/language-server/src/dotty/tools/languageserver/DottyLanguageServer.scala index d9960609d921..492fe8a9b7b8 100644 --- a/language-server/src/dotty/tools/languageserver/DottyLanguageServer.scala +++ b/language-server/src/dotty/tools/languageserver/DottyLanguageServer.scala @@ -342,7 +342,7 @@ class DottyLanguageServer extends LanguageServer (remoteDriver, ctx, definition) } - perProjectInfo.par.flatMap { (remoteDriver, ctx, definition) => + perProjectInfo.flatMap { (remoteDriver, ctx, definition) => val trees = remoteDriver.sourceTreesContaining(symbolName)(ctx) Interactive.findTreesMatching(trees, includes, definition)(ctx) } From d5280a691d763709290192d0bcdc6e81a5325316 Mon Sep 17 00:00:00 2001 From: Martin Duhem Date: Mon, 12 Nov 2018 13:26:06 +0100 Subject: [PATCH 11/11] Narrow scope of `ctx` to avoid shadowing --- .../languageserver/DottyLanguageServer.scala | 48 +++++++++++-------- 1 file changed, 27 insertions(+), 21 deletions(-) diff --git a/language-server/src/dotty/tools/languageserver/DottyLanguageServer.scala b/language-server/src/dotty/tools/languageserver/DottyLanguageServer.scala index 492fe8a9b7b8..736d1f335d77 100644 --- a/language-server/src/dotty/tools/languageserver/DottyLanguageServer.scala +++ b/language-server/src/dotty/tools/languageserver/DottyLanguageServer.scala @@ -305,7 +305,6 @@ class DottyLanguageServer extends LanguageServer override def references(params: ReferenceParams) = computeAsync { cancelToken => val uri = new URI(params.getTextDocument.getUri) val driver = driverFor(uri) - implicit val ctx = driver.currentCtx val includes = { val includeDeclaration = params.getContext.isIncludeDeclaration @@ -313,25 +312,31 @@ class DottyLanguageServer extends LanguageServer } val pos = sourcePosition(driver, uri, params.getPosition) - val path = Interactive.pathTo(driver.openedTrees(uri), pos) - // Find definitions of the symbol under the cursor, so that we can determine - // what projects are worth exploring - val definitions = Interactive.findDefinitions(path, driver) - val projectsToInspect = - if (definitions.isEmpty) { - drivers.keySet - } else { - for { - definition <- definitions - uri <- toUriOption(definition.pos.source).toSet - config = configFor(uri) - project <- dependentProjects(config) + config - } yield project - } + val (definitions, projectsToInspect, originalSymbol, originalSymbolName) = { + implicit val ctx: Context = driver.currentCtx + val path = Interactive.pathTo(driver.openedTrees(uri), pos) + val originalSymbol = Interactive.enclosingSourceSymbol(path) + val originalSymbolName = originalSymbol.name.sourceModuleName.toString + + // Find definitions of the symbol under the cursor, so that we can determine + // what projects are worth exploring + val definitions = Interactive.findDefinitions(path, driver) + val projectsToInspect = + if (definitions.isEmpty) { + drivers.keySet + } else { + for { + definition <- definitions + uri <- toUriOption(definition.pos.source).toSet + config = configFor(uri) + project <- dependentProjects(config) + config + } yield project + } + + (definitions, projectsToInspect, originalSymbol, originalSymbolName) + } - val originalSymbol = Interactive.enclosingSourceSymbol(path) - val symbolName = originalSymbol.name.sourceModuleName.toString val references = { // Collect the information necessary to look into each project separately: representation of // `originalSymbol` in this project, the context and correct Driver. @@ -343,12 +348,13 @@ class DottyLanguageServer extends LanguageServer } perProjectInfo.flatMap { (remoteDriver, ctx, definition) => - val trees = remoteDriver.sourceTreesContaining(symbolName)(ctx) - Interactive.findTreesMatching(trees, includes, definition)(ctx) + val trees = remoteDriver.sourceTreesContaining(originalSymbolName)(ctx) + val matches = Interactive.findTreesMatching(trees, includes, definition)(ctx) + matches.map(tree => location(tree.namePos(ctx), positionMapperFor(tree.source))) } }.toList - references.flatMap(ref => location(ref.namePos, positionMapperFor(ref.source))).asJava + references.flatten.asJava } override def rename(params: RenameParams) = computeAsync { cancelToken =>