Merge pull request #7678 from dotty-staging/sourcepath-toplevel

Improve sourcepath handling

Author: smarter

compiler/src/dotty/tools/dotc/ast/Desugar.scala

@@ -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:
@@ -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)
     }
   }

compiler/src/dotty/tools/dotc/config/ScalaSettings.scala

@@ -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.",

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

@@ -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._
@@ -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)
@@ -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

compiler/test/dotty/tools/dotc/CompilationTests.scala

@@ -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),
@@ -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"),

language-server/src/dotty/tools/languageserver/DottyLanguageServer.scala

@@ -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

library/src/scala/internal/WithBounds.scala renamed to library/src/scala/annotation/internal/WithBounds.scala

@@ -1,4 +1,4 @@
-package completeFromSource.nested;
+package outer.nested;
 
 public class D {
     public D(int i) {

tests/neg-custom-args/completeFromSource/nested/D.java renamed to tests/neg-custom-args/sourcepath/outer/nested/D.java

@@ -1,4 +1,4 @@
-package completeFromSource.nested
+package outer.nested
 
 class Test4 {
 

tests/neg-custom-args/completeFromSource/nested/Test1.scala renamed to tests/neg-custom-args/sourcepath/outer/nested/Test1.scala

@@ -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

tests/neg-custom-args/sourcepath2/hi/A.check

@@ -0,0 +1,5 @@
+// Missing `package hi`
+
+class Hello { // error
+  val x: Int = 1
+}

tests/neg-custom-args/sourcepath2/hi/A.scala

@@ -0,0 +1,2 @@
+package hi
+class B

tests/neg-custom-args/sourcepath2/hi/B.scala

@@ -1,4 +1,4 @@
-package completeFromSource
+package outer
 
 class Test extends nested.A(22) {
 

tests/pos-special/completeFromSource/Test.scala renamed to tests/pos-special/sourcepath/outer/Test.scala

@@ -1,4 +1,4 @@
-package completeFromSource
+package outer
 import nested._
 
 class Test2 extends A(22) {

tests/pos-special/completeFromSource/Test2.scala renamed to tests/pos-special/sourcepath/outer/Test2.scala

@@ -1,4 +1,4 @@
-package completeFromSource
+package outer
 import nested._
 
 class Test3 {

tests/pos-special/completeFromSource/Test3.scala renamed to tests/pos-special/sourcepath/outer/Test3.scala

@@ -1,4 +1,4 @@
-package completeFromSource.nested
+package outer.nested
 
 class A(y: Int) {
 

tests/pos-special/completeFromSource/nested/A.scala renamed to tests/pos-special/sourcepath/outer/nested/A.scala

@@ -1,4 +1,4 @@
-package completeFromSource.nested
+package outer.nested
 
 
 case class B(x: Int) extends A(x)

tests/pos-special/completeFromSource/nested/BC.scala renamed to tests/pos-special/sourcepath/outer/nested/BC.scala

@@ -1,4 +1,4 @@
-package completeFromSource.nested;
+package outer.nested;
 
 public class D {
     public D(int i) {

tests/pos-special/completeFromSource/nested/D.java renamed to tests/pos-special/sourcepath/outer/nested/D.java

@@ -1,4 +1,4 @@
-package completeFromSource.nested
+package outer.nested
 
 class Test4 {
 

tests/pos-special/completeFromSource/nested/Test4.scala renamed to tests/pos-special/sourcepath/outer/nested/Test4.scala

@@ -0,0 +1,9 @@
+package outer
+package nested
+
+val one: Int = 1
+
+type Hi = Int
+object Hi {
+  def hi: Hi = 2
+}

tests/pos-special/sourcepath/outer/nested/toplevel1.scala

@@ -0,0 +1,7 @@
+package outer
+import nested._
+
+object toplevel2 {
+  val a: Int = one
+  val b: Hi = Hi.hi
+}