Skip to content

Improve sourcepath handling #7678

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

Merged
merged 4 commits into from
Dec 4, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
51 changes: 35 additions & 16 deletions compiler/src/dotty/tools/dotc/ast/Desugar.scala
Original file line number Diff line number Diff line change
Expand Up @@ -1220,6 +1220,29 @@ object desugar {
else Apply(ref(tupleTypeRef.classSymbol.companionModule.termRef), ts)
}

private def isTopLevelDef(stat: Tree)(given Context): Boolean = stat match
case _: ValDef | _: PatDef | _: DefDef | _: Export => true
case stat: ModuleDef =>
stat.mods.isOneOf(GivenOrImplicit)
case stat: TypeDef =>
!stat.isClassDef || stat.mods.isOneOf(GivenOrImplicit)
case _ =>
false

/** Does this package contains at least one top-level definition
* that will require a wrapping object ?
*/
def hasTopLevelDef(pdef: PackageDef)(given Context): Boolean =
pdef.stats.exists(isTopLevelDef)

/** Assuming `src` contains top-level definition, returns the name that should
* be using for the package object that will wrap them.
*/
def packageObjectName(src: SourceFile): TermName =
val fileName = src.file.name
val sourceName = fileName.take(fileName.lastIndexOf('.'))
(sourceName ++ str.TOPLEVEL_SUFFIX).toTermName

/** Group all definitions that can't be at the toplevel in
* an object named `<source>$package` where `<source>` is the name of the source file.
* Definitions that can't be at the toplevel are:
Expand All @@ -1231,26 +1254,22 @@ object desugar {
* (i.e. objects having the same name as a wrapped type)
*/
def packageDef(pdef: PackageDef)(implicit ctx: Context): PackageDef = {
def isWrappedType(stat: TypeDef): Boolean =
!stat.isClassDef || stat.mods.isOneOf(GivenOrImplicit)
val wrappedTypeNames = pdef.stats.collect {
case stat: TypeDef if isWrappedType(stat) => stat.name
}
def needsObject(stat: Tree) = stat match {
case _: ValDef | _: PatDef | _: DefDef | _: Export => true
case stat: ModuleDef =>
stat.mods.isOneOf(GivenOrImplicit) ||
wrappedTypeNames.contains(stat.name.stripModuleClassSuffix.toTypeName)
case stat: TypeDef => isWrappedType(stat)
case _ => false
case stat: TypeDef if isTopLevelDef(stat) => stat.name
}
val (nestedStats, topStats) = pdef.stats.partition(needsObject)
def inPackageObject(stat: Tree) =
isTopLevelDef(stat) || {
stat match
case stat: ModuleDef =>
wrappedTypeNames.contains(stat.name.stripModuleClassSuffix.toTypeName)
case _ =>
false
}
val (nestedStats, topStats) = pdef.stats.partition(inPackageObject)
if (nestedStats.isEmpty) pdef
else {
var fileName = ctx.source.file.name
val sourceName = fileName.take(fileName.lastIndexOf('.'))
val groupName = (sourceName ++ str.TOPLEVEL_SUFFIX).toTermName.asSimpleName
val grouped = ModuleDef(groupName, Template(emptyConstructor, Nil, Nil, EmptyValDef, nestedStats))
val name = packageObjectName(ctx.source)
val grouped = ModuleDef(name, Template(emptyConstructor, Nil, Nil, EmptyValDef, nestedStats))
cpy.PackageDef(pdef)(pdef.pid, topStats :+ grouped)
}
}
Expand Down
1 change: 0 additions & 1 deletion compiler/src/dotty/tools/dotc/config/ScalaSettings.scala
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ class ScalaSettings extends Settings.SettingGroup {
val javabootclasspath: Setting[String] = PathSetting("-javabootclasspath", "Override java boot classpath.", Defaults.javaBootClassPath) withAbbreviation "--java-boot-class-path"
val javaextdirs: Setting[String] = PathSetting("-javaextdirs", "Override java extdirs classpath.", Defaults.javaExtDirs) withAbbreviation "--java-extension-directories"
val sourcepath: Setting[String] = PathSetting("-sourcepath", "Specify location(s) of source files.", Defaults.scalaSourcePath) withAbbreviation "--source-path"
val scansource: Setting[Boolean] = BooleanSetting("-scansource", "Scan source files to locate classes for which class-name != file-name") withAbbreviation "--scan-source"

val classpath: Setting[String] = PathSetting("-classpath", "Specify where to find user class files.", defaultClasspath) withAbbreviation "-cp" withAbbreviation "--class-path"
val outputDir: Setting[AbstractFile] = OutputSetting("-d", "directory|jar", "destination for generated classfiles.",
Expand Down
98 changes: 46 additions & 52 deletions compiler/src/dotty/tools/dotc/core/SymbolLoaders.scala
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import ast.Trees._
import parsing.JavaParsers.OutlineJavaParser
import parsing.Parsers.OutlineParser
import reporting.trace
import ast.desugar.{ packageObjectName, hasTopLevelDef }

object SymbolLoaders {
import ast.untpd._
Expand All @@ -31,7 +32,7 @@ object SymbolLoaders {
owner: Symbol, member: Symbol,
completer: SymbolLoader, scope: Scope = EmptyScope)(implicit ctx: Context): Symbol = {
val comesFromScan =
completer.isInstanceOf[SourcefileLoader] && ctx.settings.scansource.value
completer.isInstanceOf[SourcefileLoader]
assert(comesFromScan || scope.lookup(member.name) == NoSymbol,
s"${owner.fullName}.${member.name} already has a symbol")
owner.asClass.enter(member, scope)
Expand Down Expand Up @@ -103,69 +104,62 @@ object SymbolLoaders {
scope = scope)
}

/** If setting -scansource is set:
* Enter all toplevel classes and objects in file `src` into package `owner`, provided
* they are in the right package. Issue a warning if a class or object is in the wrong
* package, i.e. if the file path differs from the declared package clause.
* If -scansource is not set:
* Enter class and module with given `name` into scope of `owner`.
/** Enter all toplevel classes and objects in file `src` into package `owner`, provided
* they are in the right package. Issue a warning if a class or object is in the wrong
* package, i.e. if the file path differs from the declared package clause.
*
* All entered symbols are given a source completer of `src` as info.
*/
def enterToplevelsFromSource(
owner: Symbol, name: PreName, src: AbstractFile,
scope: Scope = EmptyScope)(implicit ctx: Context): Unit = {
scope: Scope = EmptyScope)(implicit ctx: Context): Unit =
if src.exists && !src.isDirectory
val completer = new SourcefileLoader(src)
val filePath = owner.ownersIterator.takeWhile(!_.isRoot).map(_.name.toTermName).toList

def addPrefix(pid: RefTree, path: List[TermName]): List[TermName] = pid match {
case Ident(name: TermName) => name :: path
case Select(qual: RefTree, name: TermName) => name :: addPrefix(qual, path)
case _ => path
}

val completer = new SourcefileLoader(src)
if (ctx.settings.scansource.value && ctx.run != null) {
if (src.exists && !src.isDirectory) {
val filePath = owner.ownersIterator.takeWhile(!_.isRoot).map(_.name.toTermName).toList
def enterScanned(unit: CompilationUnit)(implicit ctx: Context) = {

def addPrefix(pid: RefTree, path: List[TermName]): List[TermName] = pid match {
case Ident(name: TermName) => name :: path
case Select(qual: RefTree, name: TermName) => name :: addPrefix(qual, path)
case _ => path
def checkPathMatches(path: List[TermName], what: String, tree: NameTree): Boolean = {
val ok = filePath == path
if (!ok)
ctx.warning(i"""$what ${tree.name} is in the wrong directory.
|It was declared to be in package ${path.reverse.mkString(".")}
|But it is found in directory ${filePath.reverse.mkString(File.separator)}""",
tree.sourcePos.focus)
ok
}

def enterScanned(unit: CompilationUnit)(implicit ctx: Context) = {

def checkPathMatches(path: List[TermName], what: String, tree: MemberDef): Boolean = {
val ok = filePath == path
if (!ok)
ctx.warning(i"""$what ${tree.name} is in the wrong directory.
|It was declared to be in package ${path.reverse.mkString(".")}
|But it is found in directory ${filePath.reverse.mkString(File.separator)}""",
tree.sourcePos)
ok
}

def traverse(tree: Tree, path: List[TermName]): Unit = tree match {
case PackageDef(pid, body) =>
val path1 = addPrefix(pid, path)
for (stat <- body) traverse(stat, path1)
case tree: TypeDef if tree.isClassDef =>
if (checkPathMatches(path, "class", tree))
enterClassAndModule(owner, tree.name, completer, scope = scope)
// It might be a case class or implicit class,
// so enter class and module to be on the safe side
case tree: ModuleDef =>
if (checkPathMatches(path, "object", tree))
enterModule(owner, tree.name, completer, scope = scope)
case _ =>
}

traverse(
if (unit.isJava) new OutlineJavaParser(unit.source).parse()
else new OutlineParser(unit.source).parse(),
Nil)
def traverse(tree: Tree, path: List[TermName]): Unit = tree match {
case tree @ PackageDef(pid, body) =>
val path1 = addPrefix(pid, path)
if hasTopLevelDef(tree) && checkPathMatches(path1, "package", pid)
enterModule(owner, packageObjectName(unit.source), completer, scope = scope)
for (stat <- body) traverse(stat, path1)
case tree: TypeDef if tree.isClassDef =>
if (checkPathMatches(path, "class", tree))
// It might be a case class or implicit class,
// so enter class and module to be on the safe side
enterClassAndModule(owner, tree.name, completer, scope = scope)
case tree: ModuleDef =>
if (checkPathMatches(path, "object", tree))
enterModule(owner, tree.name, completer, scope = scope)
case _ =>
}

val unit = CompilationUnit(ctx.getSource(src.path))
enterScanned(unit)(ctx.run.runContext.fresh.setCompilationUnit(unit))
traverse(
if (unit.isJava) new OutlineJavaParser(unit.source).parse()
else new OutlineParser(unit.source).parse(),
Nil)
}
}
else enterClassAndModule(owner, name, completer, scope = scope)
}

val unit = CompilationUnit(ctx.getSource(src.path))
enterScanned(unit)(ctx.fresh.setCompilationUnit(unit))

/** The package objects of scala and scala.reflect should always
* be loaded in binary if classfiles are available, even if sourcefiles
Expand Down
11 changes: 6 additions & 5 deletions compiler/test/dotty/tools/dotc/CompilationTests.scala
Original file line number Diff line number Diff line change
Expand Up @@ -39,10 +39,10 @@ class CompilationTests extends ParallelTesting {
compileFile("tests/pos-scala2/rewrites.scala", scala2CompatMode.and("-rewrite")).copyToTarget(),
compileFile("tests/pos-special/utf8encoded.scala", explicitUTF8),
compileFile("tests/pos-special/utf16encoded.scala", explicitUTF16),
compileFile("tests/pos-special/completeFromSource/Test.scala", defaultOptions.and("-sourcepath", "tests/pos-special")),
compileFile("tests/pos-special/completeFromSource/Test2.scala", defaultOptions.and("-sourcepath", "tests/pos-special")),
compileFile("tests/pos-special/completeFromSource/Test3.scala", defaultOptions.and("-sourcepath", "tests/pos-special", "-scansource")),
compileFile("tests/pos-special/completeFromSource/nested/Test4.scala", defaultOptions.and("-sourcepath", "tests/pos-special", "-scansource")),
compileFile("tests/pos-special/sourcepath/outer/Test.scala", defaultOptions.and("-sourcepath", "tests/pos-special/sourcepath")),
compileFile("tests/pos-special/sourcepath/outer/Test2.scala", defaultOptions.and("-sourcepath", "tests/pos-special/sourcepath")),
compileFile("tests/pos-special/sourcepath/outer/Test3.scala", defaultOptions.and("-sourcepath", "tests/pos-special/sourcepath")),
compileFile("tests/pos-special/sourcepath/outer/nested/Test4.scala", defaultOptions.and("-sourcepath", "tests/pos-special/sourcepath")),
compileFilesInDir("tests/pos-special/fatal-warnings", defaultOptions.and("-Xfatal-warnings", "-feature")),
compileFilesInDir("tests/pos-special/spec-t5545", defaultOptions),
compileFilesInDir("tests/pos-special/strawman-collections", defaultOptions),
Expand Down Expand Up @@ -135,7 +135,8 @@ class CompilationTests extends ParallelTesting {
compileFilesInDir("tests/neg-custom-args/isInstanceOf", allowDeepSubtypes and "-Xfatal-warnings"),
compileFile("tests/neg-custom-args/i3627.scala", allowDeepSubtypes),
compileFile("tests/neg-custom-args/matchtype-loop.scala", allowDeepSubtypes),
compileFile("tests/neg-custom-args/completeFromSource/nested/Test1.scala", defaultOptions.and("-sourcepath", "tests/neg-custom-args", "-scansource")),
compileFile("tests/neg-custom-args/sourcepath/outer/nested/Test1.scala", defaultOptions.and("-sourcepath", "tests/neg-custom-args/sourcepath")),
compileDir("tests/neg-custom-args/sourcepath2/hi", defaultOptions.and("-sourcepath", "tests/neg-custom-args/sourcepath2", "-Xfatal-warnings")),
compileList("duplicate source", List(
"tests/neg-custom-args/toplevel-samesource/S.scala",
"tests/neg-custom-args/toplevel-samesource/nested/S.scala"),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -87,8 +87,7 @@ class DottyLanguageServer extends LanguageServer
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"
.update("-sourcepath", config.sourceDirectories.mkString(File.pathSeparator))
myDrivers(config) = new InteractiveDriver(settings)
}
myDrivers
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
package completeFromSource.nested;
package outer.nested;

public class D {
public D(int i) {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
package completeFromSource.nested
package outer.nested

class Test4 {

Expand Down
6 changes: 6 additions & 0 deletions tests/neg-custom-args/sourcepath2/hi/A.check
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
-- Error: tests/neg-custom-args/sourcepath2/hi/A.scala:3:6 -------------------------------------------------------------
3 |class Hello { // error
| ^
| class Hello is in the wrong directory.
| It was declared to be in package <empty>
| But it is found in directory hi
5 changes: 5 additions & 0 deletions tests/neg-custom-args/sourcepath2/hi/A.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
// Missing `package hi`

class Hello { // error
val x: Int = 1
}
2 changes: 2 additions & 0 deletions tests/neg-custom-args/sourcepath2/hi/B.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
package hi
class B
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
package completeFromSource
package outer

class Test extends nested.A(22) {

Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
package completeFromSource
package outer
import nested._

class Test2 extends A(22) {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
package completeFromSource
package outer
import nested._

class Test3 {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
package completeFromSource.nested
package outer.nested

class A(y: Int) {

Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
package completeFromSource.nested
package outer.nested


case class B(x: Int) extends A(x)
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
package completeFromSource.nested;
package outer.nested;

public class D {
public D(int i) {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
package completeFromSource.nested
package outer.nested

class Test4 {

Expand Down
9 changes: 9 additions & 0 deletions tests/pos-special/sourcepath/outer/nested/toplevel1.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
package outer
package nested

val one: Int = 1

type Hi = Int
object Hi {
def hi: Hi = 2
}
7 changes: 7 additions & 0 deletions tests/pos-special/sourcepath/outer/toplevel2.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
package outer
import nested._

object toplevel2 {
val a: Int = one
val b: Hi = Hi.hi
}