Merge pull request #10375 from som-snytt/issue/12735-minimal-err-span

Trees of named defs get NamePos used for symbol

src/compiler/scala/tools/nsc/ast/parser/Parsers.scala

@@ -2929,7 +2929,7 @@ self =>
      *  FunSig ::= id [FunTypeParamClause] ParamClauses
      *  }}}
      */
-    def funDefOrDcl(start : Int, mods: Modifiers): Tree = {
+    def funDefOrDcl(start: Int, mods: Modifiers): Tree = {
       in.nextToken()
       if (in.token == THIS) {
         def missingEquals() = deprecationWarning(in.lastOffset, "procedure syntax is deprecated for constructors: add `=`, as in method definition", "2.13.2")
@@ -2955,7 +2955,9 @@ self =>
     }
 
     def funDefRest(start: Offset, nameOffset: Offset, mods: Modifiers, name: Name): Tree = {
-      val result = atPos(start, if (name.toTermName == nme.ERROR) start else nameOffset) {
+      def orStart(p: Offset) = if (name.toTermName == nme.ERROR) start else p
+      val namePos = NamePos(r2p(orStart(nameOffset), orStart(nameOffset)))
+      val result = atPos(start, orStart(nameOffset)) {
         var newmods = mods
         // contextBoundBuf is for context bounded type parameters of the form
         // [T : B] or [T : => B]; it contains the equivalent implicit parameter type,
@@ -3005,7 +3007,7 @@ self =>
             case _ => // ok
           }
         }
-        DefDef(newmods, name.toTermName, tparams, vparamss, restype, rhs)
+        DefDef(newmods, name.toTermName, tparams, vparamss, restype, rhs).updateAttachment(namePos)
       }
       signalParseProgress(result.pos)
       result
@@ -3128,7 +3130,9 @@ self =>
       val name = identForType()
       if (currentRun.isScala3 && in.token == LBRACKET && isAfterLineEnd)
         deprecationWarning(in.offset, "type parameters should not follow newline", "2.13.7")
-      atPos(start, if (name == tpnme.ERROR) start else nameOffset) {
+      def orStart(p: Offset) = if (name == tpnme.ERROR) start else p
+      val namePos = NamePos(r2p(orStart(nameOffset), orStart(nameOffset)))
+      atPos(start, orStart(nameOffset)) {
         savingClassContextBounds {
           val contextBoundBuf = new ListBuffer[Tree]
           val tparams = typeParamClauseOpt(name, contextBoundBuf, ParamOwner.Class)
@@ -3144,7 +3148,7 @@ self =>
             if (mods.isTrait) (Modifiers(Flags.TRAIT), List())
             else (accessModifierOpt(), paramClauses(name, classContextBounds, ofCaseClass = mods.isCase))
           val template = templateOpt(mods, name, constrMods withAnnotations constrAnnots, vparamss, tstart)
-          val result = gen.mkClassDef(mods, name, tparams, template)
+          val result = gen.mkClassDef(mods, name, tparams, template).updateAttachment(namePos)
           // Context bounds generate implicit parameters (part of the template) with types
           // from tparams: we need to ensure these don't overlap
           if (!classContextBounds.isEmpty)
@@ -3164,9 +3168,11 @@ self =>
       checkKeywordDefinition()
       val name = ident()
       val tstart = in.offset
-      atPos(start, if (name == nme.ERROR) start else nameOffset) {
+      def orStart(p: Offset) = if (name == tpnme.ERROR) start else p
+      val namePos = NamePos(r2p(orStart(nameOffset), orStart(nameOffset)))
+      atPos(start, orStart(nameOffset)) {
         val template = templateOpt(mods, if (isPackageObject) nme.PACKAGEkw else name, NoMods, Nil, tstart)
-        ModuleDef(mods, name.toTermName, template)
+        ModuleDef(mods, name.toTermName, template).updateAttachment(namePos)
       }
     }
 

src/compiler/scala/tools/nsc/typechecker/Namers.scala

@@ -342,7 +342,7 @@ trait Namers extends MethodSynthesis {
      *  the flags to keep.
      */
     def createMemberSymbol(tree: MemberDef, name: Name, mask: Long): Symbol = {
-      val pos         = tree.pos
+      val pos         = tree.namePos
       val isParameter = tree.mods.isParameter
       val flags       = tree.mods.flags & mask
 
@@ -461,7 +461,7 @@ trait Namers extends MethodSynthesis {
 
     /** Enter a module symbol.
      */
-    def enterModuleSymbol(tree : ModuleDef): Symbol = {
+    def enterModuleSymbol(tree: ModuleDef): Symbol = {
       val moduleFlags = tree.mods.flags | MODULE
 
       val existingModule = context.scope lookupModule tree.name

src/compiler/scala/tools/nsc/typechecker/RefChecks.scala

@@ -753,7 +753,7 @@ abstract class RefChecks extends Transform {
         if (abstractErrors.nonEmpty)
           reporter.error(clazz.pos, abstractErrorMessage)
       }
-      else if (clazz.isTrait && !(clazz isSubClass AnyValClass)) {
+      else if (clazz.isTrait && !clazz.isSubClass(AnyValClass)) {
         // For non-AnyVal classes, prevent abstract methods in interfaces that override
         // final members in Object; see #4431
         for (decl <- clazz.info.decls) {

src/compiler/scala/tools/nsc/typechecker/TypeDiagnostics.scala

@@ -696,14 +696,13 @@ trait TypeDiagnostics extends splain.SplainDiagnostics {
         def wcat(sym: Symbol) = if (sym.isPrivate) WarningCategory.UnusedPrivates else WarningCategory.UnusedLocals
         def termWarning(defn: SymTree): Unit = {
           val sym = defn.symbol
-          val pos = (
-            if (defn.pos.isDefined) defn.pos
-            else if (sym.pos.isDefined) sym.pos
-            else sym match {
-              case sym: TermSymbol => sym.referenced.pos
-              case _               => NoPosition
+          val pos =
+            sym match {
+              case sym if sym.pos.isDefined                        => sym.pos
+              case sym: TermSymbol if sym.referenced.pos.isDefined => sym.referenced.pos
+              case _ if defn.pos.isDefined                         => defn.pos
+              case _                                               => NoPosition
             }
-          )
           val why = if (sym.isPrivate) "private" else "local"
           var cond = "is never used"
           def long = if (settings.uniqid.value) s" (${sym.nameString})" else ""

src/partest/scala/tools/partest/nest/DirectCompiler.scala

@@ -13,26 +13,32 @@
 package scala.tools.partest
 package nest
 
-import java.io.{FileWriter, PrintWriter}
+import java.io.{BufferedReader, FileWriter, PrintWriter}
 
 import scala.collection.mutable.ListBuffer
-import scala.reflect.internal.util.NoPosition
+import scala.reflect.internal.util.{NoPosition, Position, ScalaClassLoader}
 import scala.reflect.io.AbstractFile
 import scala.tools.nsc.reporters.{ConsoleReporter, Reporter}
 import scala.tools.nsc.{CompilerCommand, Global, Settings}
 import scala.util.chaining._
 import scala.sys.process._
 
 object ExtConsoleReporter {
-  def apply(settings: Settings, writer: PrintWriter) = new ConsoleReporter(settings, Console.in, writer, writer).tap(_.shortname = true)
+  def apply(settings: Settings, writer: PrintWriter) = {
+    val loader = new ClassLoader(getClass.getClassLoader) with ScalaClassLoader
+    loader.create[ConsoleReporter](settings.reporter.value, settings.errorFn)(settings, Console.in, writer, writer).tap(_.shortname = true)
+  }
+}
+class PlainReporter(settings: Settings, reader: BufferedReader, writer: PrintWriter, echo: PrintWriter) extends ConsoleReporter(settings, reader, writer, echo) {
+  override def doReport(pos: Position, msg: String, severity: Severity): Unit = writer.println(s"[$severity] [$pos]: $msg")
 }
 
 class TestSettings(cp: String, error: String => Unit) extends Settings(error) {
   @deprecated("Use primary constructor", "1.0.12")
   def this(cp: String) = this(cp, _ => ())
-  nowarnings.value  = false
-  encoding.value    = "UTF-8"
-  classpath.value   = cp
+  nowarnings.value = false
+  encoding.value   = "UTF-8"
+  classpath.value  = cp
   //lint.add("_")
 }
 
@@ -50,10 +56,10 @@ class DirectCompiler(val runner: Runner) {
 
 
   /** Massage args to merge plugins and fix paths.
-    *  Plugin path can be relative to test root, or cwd is out.
-    *  While we're at it, mix in the baseline options, too.
-    *  That's how ant passes in the plugins dir.
-    */
+   *  Plugin path can be relative to test root, or cwd is out.
+   *  While we're at it, mix in the baseline options, too.
+   *  That's how ant passes in the plugins dir.
+   */
   private def updatePluginPath(args: List[String], out: AbstractFile, srcdir: AbstractFile): Seq[String] = {
     val dir = runner.suiteRunner.pathSettings.testRoot
     // The given path, or the output dir if ".", or a temp dir if output is virtual (since plugin loading doesn't like virtual)
@@ -104,13 +110,13 @@ class DirectCompiler(val runner: Runner) {
     val testSettings = new TestSettings(FileManager.joinPaths(classPath), s => parseArgErrors += s)
     val logWriter    = new FileWriter(logFile)
     val srcDir       = if (testFile.isDirectory) testFile else Path(testFile).parent.jfile
-    val opts         = updatePluginPath(opts0, AbstractFile getDirectory outDir, AbstractFile getDirectory srcDir)
+    val opts         = updatePluginPath(opts0, AbstractFile.getDirectory(outDir), AbstractFile.getDirectory(srcDir))
     val command      = new CompilerCommand(opts.toList, testSettings)
     val reporter     = ExtConsoleReporter(testSettings, new PrintWriter(logWriter, true))
     val global       = newGlobal(testSettings, reporter)
     def errorCount   = reporter.errorCount
 
-    testSettings.outputDirs setSingleOutput outDir.getPath
+    testSettings.outputDirs.setSingleOutput(outDir.getPath)
 
     def reportError(s: String): Unit = reporter.error(NoPosition, s)
 

src/reflect/scala/reflect/internal/StdAttachments.scala

@@ -158,4 +158,6 @@ trait StdAttachments {
   /** Java sealed classes may be qualified with a permits clause specifying allowed subclasses. */
   case class PermittedSubclasses(permits: List[Tree]) extends PlainAttachment
   case class PermittedSubclassSymbols(permits: List[Symbol]) extends PlainAttachment
+
+  case class NamePos(pos: Position) extends PlainAttachment
 }

src/reflect/scala/reflect/internal/TreeGen.scala

@@ -452,7 +452,7 @@ abstract class TreeGen {
     else parents
 
   def mkClassDef(mods: Modifiers, name: TypeName, tparams: List[TypeDef], templ: Template): ClassDef = {
-    val isInterface = mods.isTrait && (templ.body forall treeInfo.isInterfaceMember)
+    val isInterface = mods.isTrait && templ.body.forall(treeInfo.isInterfaceMember)
     val mods1 = if (isInterface) (mods | Flags.INTERFACE) else mods
     ClassDef(mods1, name, tparams, templ)
   }

src/reflect/scala/reflect/internal/Trees.scala

@@ -292,6 +292,7 @@ trait Trees extends api.Trees {
     def getterName: TermName = name.getterName
     def setterName: TermName = name.setterName
     def localName: TermName = name.localName
+    def namePos: Position = this.attachments.get[NamePos].map(_.pos).getOrElse(this.pos)
   }
 
   trait RefTree extends SymTree with NameTree with RefTreeApi {

src/reflect/scala/reflect/macros/Attachments.scala

@@ -138,6 +138,7 @@ private final class SingleAttachment[P >: Null](override val pos: P, val att: An
     else new NonemptyAttachments[P](pos, Set.empty[Any] + att + newAtt)
   override def remove[T](implicit tt: ClassTag[T]) =
     if (contains(tt)) pos.asInstanceOf[Attachments { type Pos = P }] else this
+  override def toString = s"SingleAttachment at $pos: $att"
 }
 
 // scala/bug#7018: This used to be an inner class of `Attachments`, but that led to a memory leak in the

src/reflect/scala/reflect/runtime/JavaUniverseForce.scala

@@ -82,6 +82,7 @@ trait JavaUniverseForce { self: runtime.JavaUniverse  =>
     this.LookupAmbiguityWarning
     this.PermittedSubclasses
     this.PermittedSubclassSymbols
+    this.NamePos
     this.noPrint
     this.typeDebug
     // inaccessible: this.posAssigner

test/files/neg/t12735a.check

@@ -0,0 +1,6 @@
+[ERROR] [RangePosition(t12735a.scala, 97, 97, 98)]: class B needs to be abstract.
+Missing implementation for member of trait A:
+  def x: String = ???
+
+[ERROR] [RangePosition(t12735a.scala, 123, 123, 127)]: covariant type T occurs in contravariant position in type T of value t
+2 errors

test/files/neg/t12735a.scala

@@ -0,0 +1,9 @@
+// scalac: -Xreporter:scala.tools.partest.nest.PlainReporter
+
+trait A {
+  def x: String
+}
+
+class B[+T] extends A {
+  def y(t: T): Unit = ()
+}

test/files/neg/t12735b.check

@@ -0,0 +1,8 @@
+[WARNING] [RangePosition(t12735b.scala, 112, 112, 113)]: private method m in class UnusedMethod is never used
+[WARNING] [RangePosition(t12735b.scala, 216, 216, 217)]: private object X in object UnusedObject is never used
+[WARNING] [RangePosition(t12735b.scala, 228, 228, 253)]: side-effecting nullary methods are discouraged: suggest defining as `def stuff to create a range()` instead
+[WARNING] [RangePosition(t12735b.scala, 128, 132, 135)]: match may not be exhaustive.
+It would fail on the following input: List(_)
+[ERROR] [NoPosition]: No warnings can be incurred under -Werror.
+4 warnings
+1 error

test/files/neg/t12735b.scala

@@ -0,0 +1,14 @@
+// scalac: -Werror -Xlint -Xreporter:scala.tools.partest.nest.PlainReporter
+
+class UnusedMethod {
+  private def m: String =
+    List(1) match {
+      case Nil => "nil"
+    }
+}
+
+object UnusedObject {
+  private object X {
+    def `stuff to create a range` = ()
+  }
+}

test/junit/scala/tools/nsc/typechecker/TypedTreeTest.scala

@@ -57,6 +57,7 @@ class TypedTreeTest extends BytecodeTesting {
 
   @Test
   def constantFoldedOriginalTreeAttachment(): Unit = {
+    import compiler.global._
     val code =
       """object O {
         |  final val x = 42
@@ -68,7 +69,9 @@ class TypedTreeTest extends BytecodeTesting {
     val run = compiler.newRun()
     run.compileSources(List(BytecodeTesting.makeSourceFile(code, "UnitTestSource.scala")))
     val tree = run.units.next().body
-    val List(t) = tree.filter(_.attachments.all.nonEmpty).toList
+    val attached = tree.filter(_.hasAttachment[analyzer.OriginalTreeAttachment]).toList
+    assertEquals(1, attached.length)
+    val List(t) = attached
     assertEquals("42:Set(OriginalTreeAttachment(O.x))", s"$t:${t.attachments.all}")
   }