Merge pull request #12541 from dotty-staging/fix-12326

record end markers in trees and semanticdb, exclude top level def wrappers

Author: bishabosha

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

@@ -182,6 +182,7 @@ object desugar {
         tpt     = TypeTree(defn.UnitType),
         rhs     = setterRhs
       ).withMods((mods | Accessor) &~ (CaseAccessor | GivenOrImplicit | Lazy))
+       .dropEndMarker() // the end marker should only appear on the getter definition
       Thicket(vdef1, setter)
     }
     else vdef1
@@ -883,6 +884,7 @@ object desugar {
       val clsTmpl = cpy.Template(impl)(self = clsSelf, body = impl.body)
       val cls = TypeDef(clsName, clsTmpl)
         .withMods(mods.toTypeFlags & RetainedModuleClassFlags | ModuleClassCreationFlags)
+        .withEndMarker(copyFrom = mdef) // copy over the end marker position to the module class def
       Thicket(modul, classDef(cls).withSpan(mdef.span))
     }
   }
@@ -1299,7 +1301,9 @@ object desugar {
     if (nestedStats.isEmpty) pdef
     else {
       val name = packageObjectName(ctx.source)
-      val grouped = ModuleDef(name, Template(emptyConstructor, Nil, Nil, EmptyValDef, nestedStats))
+      val grouped =
+        ModuleDef(name, Template(emptyConstructor, Nil, Nil, EmptyValDef, nestedStats))
+          .withMods(Modifiers(Synthetic))
       cpy.PackageDef(pdef)(pdef.pid, topStats :+ grouped)
     }
   }

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

@@ -327,9 +327,58 @@ object Trees {
 
   extension (mdef: untpd.DefTree) def mods: untpd.Modifiers = mdef.rawMods
 
-  abstract class NamedDefTree[-T >: Untyped](implicit @constructorOnly src: SourceFile) extends NameTree[T] with DefTree[T] {
+  sealed trait WithEndMarker[-T >: Untyped]:
+    self: PackageDef[T] | NamedDefTree[T] =>
+
+    import WithEndMarker.*
+
+    final def endSpan(using Context): Span =
+      if hasEndMarker then
+        val realName = srcName.stripModuleClassSuffix.lastPart
+        span.withStart(span.end - realName.length)
+      else
+        NoSpan
+
+    /** The name in source code that represents this construct,
+     *  and is the name that the user must write to create a valid
+     *  end marker.
+     *  e.g. a constructor definition is terminated in the source
+     *  code by `end this`, so it's `srcName` should return `this`.
+     */
+    protected def srcName(using Context): Name
+
+    final def withEndMarker(): self.type =
+      self.withAttachment(HasEndMarker, ())
+
+    final def withEndMarker(copyFrom: WithEndMarker[?]): self.type =
+      if copyFrom.hasEndMarker then
+        this.withEndMarker()
+      else
+        this
+
+    final def dropEndMarker(): self.type =
+      self.removeAttachment(HasEndMarker)
+      this
+
+    protected def hasEndMarker: Boolean = self.hasAttachment(HasEndMarker)
+
+  object WithEndMarker:
+    /** Property key that signals the tree was terminated
+     *  with an `end` marker in the source code
+     */
+    private val HasEndMarker: Property.StickyKey[Unit] = Property.StickyKey()
+
+  end WithEndMarker
+
+  abstract class NamedDefTree[-T >: Untyped](implicit @constructorOnly src: SourceFile)
+  extends NameTree[T] with DefTree[T] with WithEndMarker[T] {
     type ThisTree[-T >: Untyped] <: NamedDefTree[T]
 
+    protected def srcName(using Context): Name =
+      if name == nme.CONSTRUCTOR then nme.this_
+      else if symbol.isPackageObject then symbol.owner.name
+      else name
+
     /** The position of the name defined by this definition.
      *  This is a point position if the definition is synthetic, or a range position
      *  if the definition comes from source.
@@ -342,7 +391,7 @@ object Trees {
         val point = span.point
         if (rawMods.is(Synthetic) || span.isSynthetic || name.toTermName == nme.ERROR) Span(point)
         else {
-          val realName = name.stripModuleClassSuffix.lastPart
+          val realName = srcName.stripModuleClassSuffix.lastPart
           Span(point, point + realName.length, point)
         }
       }
@@ -857,9 +906,10 @@ object Trees {
 
   /** package pid { stats } */
   case class PackageDef[-T >: Untyped] private[ast] (pid: RefTree[T], stats: List[Tree[T]])(implicit @constructorOnly src: SourceFile)
-    extends ProxyTree[T] {
+    extends ProxyTree[T] with WithEndMarker[T] {
     type ThisTree[-T >: Untyped] = PackageDef[T]
     def forwardTo: RefTree[T] = pid
+    protected def srcName(using Context): Name = pid.name
   }
 
   /** arg @annot */

compiler/src/dotty/tools/dotc/parsing/Parsers.scala

@@ -1275,16 +1275,20 @@ object Parsers {
 
     def checkEndMarker[T <: Tree](stats: ListBuffer[T]): Unit =
 
-      def matches(stat: Tree): Boolean = stat match
+      def updateSpanOfLast(last: T): Unit =
+        last match
+          case last: WithEndMarker[t] => last.withEndMarker()
+          case _ =>
+        last.span = last.span.withEnd(in.lastCharOffset)
+
+      def matches(stat: T): Boolean = stat match
         case stat: MemberDef if !stat.name.isEmpty =>
           if stat.name == nme.CONSTRUCTOR then in.token == THIS
           else in.isIdent && in.name == stat.name.toTermName
         case ExtMethods(_, _) =>
           in.token == IDENTIFIER && in.name == nme.extension
         case PackageDef(pid: RefTree, _) =>
           in.isIdent && in.name == pid.name
-        case PatDef(_, IdPattern(id, _) :: Nil, _, _) =>
-         in.isIdent && in.name == id.name
         case stat: MemberDef if stat.mods.is(Given) => in.token == GIVEN
         case _: PatDef => in.token == VAL
         case _: If => in.token == IF
@@ -1295,9 +1299,16 @@ object Parsers {
         case _: (ForYield | ForDo) => in.token == FOR
         case _ => false
 
+      def matchesAndSetEnd(last: T): Boolean = {
+        val didMatch = matches(last)
+        if didMatch then
+          updateSpanOfLast(last)
+        didMatch
+      }
+
       if in.token == END then
         val start = in.skipToken()
-        if stats.isEmpty || !matches(stats.last) then
+        if stats.isEmpty || !matchesAndSetEnd(stats.last) then
           syntaxError("misaligned end marker", Span(start, in.lastCharOffset))
         in.token = IDENTIFIER // Leaving it as the original token can confuse newline insertion
         in.nextToken()

compiler/src/dotty/tools/dotc/semanticdb/ExtractSemanticDB.scala

@@ -6,7 +6,7 @@ import core._
 import Phases._
 import ast.tpd._
 import ast.untpd.given
-import ast.Trees.mods
+import ast.Trees.{mods, WithEndMarker}
 import Contexts._
 import Symbols._
 import Flags._
@@ -139,47 +139,46 @@ class ExtractSemanticDB extends Phase:
             case tree => registerDefinition(tree.symbol, tree.span, Set.empty, tree.source)
           tree.stats.foreach(traverse)
         case tree: NamedDefTree =>
-          if tree.symbol.isAllOf(ModuleValCreationFlags) then
-            return
-          if !excludeDef(tree.symbol)
-          && tree.span.hasLength then
-            registerDefinition(tree.symbol, tree.nameSpan, symbolKinds(tree), tree.source)
-            val privateWithin = tree.symbol.privateWithin
-            if privateWithin.exists then
-              registerUseGuarded(None, privateWithin, spanOfSymbol(privateWithin, tree.span, tree.source), tree.source)
-          else if !excludeSymbol(tree.symbol) then
-            registerSymbol(tree.symbol, symbolName(tree.symbol), symbolKinds(tree))
-          tree match
-          case tree: ValDef
-          if tree.symbol.isAllOf(EnumValue) =>
-            tree.rhs match
-            case Block(TypeDef(_, template: Template) :: _, _) => // simple case with specialised extends clause
-              template.parents.filter(!_.span.isZeroExtent).foreach(traverse)
-            case _ => // calls $new
-          case tree: ValDef
-          if tree.symbol.isSelfSym =>
-            if tree.tpt.span.hasLength then
-              traverse(tree.tpt)
-          case tree: DefDef
-          if tree.symbol.isConstructor => // ignore typeparams for secondary ctors
-            tree.trailingParamss.foreach(_.foreach(traverse))
-            traverse(tree.rhs)
-          case tree: (DefDef | ValDef)
-          if tree.symbol.isSyntheticWithIdent =>
+          if !tree.symbol.isAllOf(ModuleValCreationFlags) then
+            if !excludeDef(tree.symbol)
+            && tree.span.hasLength then
+              registerDefinition(tree.symbol, tree.nameSpan, symbolKinds(tree), tree.source)
+              val privateWithin = tree.symbol.privateWithin
+              if privateWithin.exists then
+                registerUseGuarded(None, privateWithin, spanOfSymbol(privateWithin, tree.span, tree.source), tree.source)
+            else if !excludeSymbol(tree.symbol) then
+              registerSymbol(tree.symbol, symbolName(tree.symbol), symbolKinds(tree))
             tree match
-              case tree: DefDef =>
-                tree.paramss.foreach(_.foreach(param => registerSymbolSimple(param.symbol)))
-              case tree: ValDef if tree.symbol.is(Given) => traverse(tree.tpt)
-              case _ =>
-            if !tree.symbol.isGlobal then
-              localBodies(tree.symbol) = tree.rhs
-            // ignore rhs
-          case PatternValDef(pat, rhs) =>
-            traverse(rhs)
-            PatternValDef.collectPats(pat).foreach(traverse)
-          case tree =>
-            if !excludeChildren(tree.symbol) then
-              traverseChildren(tree)
+            case tree: ValDef
+            if tree.symbol.isAllOf(EnumValue) =>
+              tree.rhs match
+              case Block(TypeDef(_, template: Template) :: _, _) => // simple case with specialised extends clause
+                template.parents.filter(!_.span.isZeroExtent).foreach(traverse)
+              case _ => // calls $new
+            case tree: ValDef
+            if tree.symbol.isSelfSym =>
+              if tree.tpt.span.hasLength then
+                traverse(tree.tpt)
+            case tree: DefDef
+            if tree.symbol.isConstructor => // ignore typeparams for secondary ctors
+              tree.trailingParamss.foreach(_.foreach(traverse))
+              traverse(tree.rhs)
+            case tree: (DefDef | ValDef)
+            if tree.symbol.isSyntheticWithIdent =>
+              tree match
+                case tree: DefDef =>
+                  tree.paramss.foreach(_.foreach(param => registerSymbolSimple(param.symbol)))
+                case tree: ValDef if tree.symbol.is(Given) => traverse(tree.tpt)
+                case _ =>
+              if !tree.symbol.isGlobal then
+                localBodies(tree.symbol) = tree.rhs
+              // ignore rhs
+            case PatternValDef(pat, rhs) =>
+              traverse(rhs)
+              PatternValDef.collectPats(pat).foreach(traverse)
+            case tree =>
+              if !excludeChildren(tree.symbol) then
+                traverseChildren(tree)
         case tree: Template =>
           val ctorSym = tree.constr.symbol
           if !excludeDef(ctorSym) then
@@ -240,6 +239,13 @@ class ExtractSemanticDB extends Phase:
         case _ =>
           traverseChildren(tree)
 
+      tree match
+        case tree: WithEndMarker[t] =>
+          val endSpan = tree.endSpan
+          if endSpan.exists then
+            registerUseGuarded(None, tree.symbol, endSpan, tree.source)
+        case _ =>
+
     end traverse
 
     private def funParamSymbol(funSym: Symbol)(using Context): Name => String =

tests/semanticdb/expect/Annotations.expect.scala

@@ -21,7 +21,7 @@ class Annotations/*<-annot::Annotations#*/[@TypeParameterAnnotation/*->com::java
 
 class B/*<-annot::B#*/ @ConstructorAnnotation/*->com::javacp::annot::ConstructorAnnotation#*/()(x/*<-annot::B#x.*/: Int/*->scala::Int#*/) {
   @ConstructorAnnotation/*->com::javacp::annot::ConstructorAnnotation#*/
-  def this()/*<-annot::B#`<init>`(+1).*/ = this(42)
+  def this/*<-annot::B#`<init>`(+1).*/() = this(42)
 }
 
 @ObjectAnnotation/*->com::javacp::annot::ObjectAnnotation#*/

tests/semanticdb/expect/AnonymousGiven.expect.scala

@@ -2,4 +2,4 @@ package angiven
 
 trait Foo/*<-angiven::Foo#*/
 
-/*<-angiven::AnonymousGiven$package.*/def bar/*<-angiven::AnonymousGiven$package.bar().*/(using Foo/*->angiven::Foo#*/) = 42
+def bar/*<-angiven::AnonymousGiven$package.bar().*/(using Foo/*->angiven::Foo#*/) = 42

tests/semanticdb/expect/EndMarkers.expect.scala

@@ -0,0 +1,59 @@
+package endmarkers:
+
+  class MultiCtor/*<-endmarkers::MultiCtor#*/(val i/*<-endmarkers::MultiCtor#i.*/: Int/*->scala::Int#*/):
+    def this/*<-endmarkers::MultiCtor#`<init>`(+1).*/() =
+      this(23)
+    end this/*->endmarkers::MultiCtor#`<init>`(+1).*/
+  end MultiCtor/*->endmarkers::MultiCtor#*/
+
+  def topLevelMethod/*<-endmarkers::EndMarkers$package.topLevelMethod().*/: String/*->scala::Predef.String#*/ =
+    "hello"
+  end topLevelMethod/*->endmarkers::EndMarkers$package.topLevelMethod().*/
+
+  val topLevelVal/*<-endmarkers::EndMarkers$package.topLevelVal.*/: Int/*->scala::Int#*/ =
+    23
+  end topLevelVal/*->endmarkers::EndMarkers$package.topLevelVal.*/
+
+  var topLevelVar/*<-endmarkers::EndMarkers$package.topLevelVar().*/: String/*->scala::Predef.String#*/ =
+    ""
+  end topLevelVar/*->endmarkers::EndMarkers$package.topLevelVar().*/
+
+  class Container/*<-endmarkers::Container#*/:
+
+    def foo/*<-endmarkers::Container#foo().*/ =
+      (/*->scala::Tuple3.apply().*/1,2,3)
+    end foo/*->endmarkers::Container#foo().*/
+
+    val bar/*<-endmarkers::Container#bar.*/ =
+      (/*->scala::Tuple3.apply().*/4,5,6)
+    end bar/*->endmarkers::Container#bar.*/
+
+    var baz/*<-endmarkers::Container#baz().*/ =
+      15
+    end baz/*->endmarkers::Container#baz().*/
+
+  end Container/*->endmarkers::Container#*/
+
+  def topLevelWithLocals/*<-endmarkers::EndMarkers$package.topLevelWithLocals().*/: Unit/*->scala::Unit#*/ =
+
+    val localVal/*<-local0*/ =
+      37
+    end localVal/*->local0*/
+
+    var localVar/*<-local1*/ =
+      43
+    end localVar/*->local1*/
+
+    def localDef/*<-local2*/ =
+      97
+    end localDef/*->local2*/
+
+  end topLevelWithLocals/*->endmarkers::EndMarkers$package.topLevelWithLocals().*/
+
+  object TestObj/*<-endmarkers::TestObj.*/:
+
+    def foo/*<-endmarkers::TestObj.foo().*/ = 23
+
+  end TestObj/*->endmarkers::TestObj.*/
+
+end endmarkers

tests/semanticdb/expect/EndMarkers.scala

@@ -0,0 +1,59 @@
+package endmarkers:
+
+  class MultiCtor(val i: Int):
+    def this() =
+      this(23)
+    end this
+  end MultiCtor
+
+  def topLevelMethod: String =
+    "hello"
+  end topLevelMethod
+
+  val topLevelVal: Int =
+    23
+  end topLevelVal
+
+  var topLevelVar: String =
+    ""
+  end topLevelVar
+
+  class Container:
+
+    def foo =
+      (1,2,3)
+    end foo
+
+    val bar =
+      (4,5,6)
+    end bar
+
+    var baz =
+      15
+    end baz
+
+  end Container
+
+  def topLevelWithLocals: Unit =
+
+    val localVal =
+      37
+    end localVal
+
+    var localVar =
+      43
+    end localVar
+
+    def localDef =
+      97
+    end localDef
+
+  end topLevelWithLocals
+
+  object TestObj:
+
+    def foo = 23
+
+  end TestObj
+
+end endmarkers