Use correct range for error in escape

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

@@ -1359,7 +1359,6 @@ self =>
     def selector(start: Offset, t0: Tree): Tree = {
       val t = stripParens(t0)
       val point = if (isIdent) in.offset else in.lastOffset //scala/bug#8459
-      //assert(t.pos.isDefined, t)
       if (t != EmptyTree)
         Select(t, ident(skipIt = false)) setPos r2p(start, point, in.lastOffset)
       else
@@ -2732,7 +2731,7 @@ self =>
         case Nil => Nil
         case t :: rest =>
           // The first import should start at the position of the keyword.
-          t.setPos(t.pos.withStart(offset))
+          if (t.pos.isRange) t.setPos(t.pos.withStart(offset))
           t :: rest
       }
     }

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

@@ -589,8 +589,10 @@ trait Namers extends MethodSynthesis {
           //       so don't warn for them. There is a corresponding special treatment
           //       in the shadowing rules in typedIdent to (scala/bug#7232). In any case,
           //       we shouldn't be emitting warnings for .java source files.
-          if (!context.unit.isJava)
-            checkNotRedundant(tree.pos withPoint fromPos, from, to)
+          if (!context.unit.isJava) {
+            val at = if (tree.pos.isRange) tree.pos.withPoint(fromPos) else tree.pos
+            checkNotRedundant(at, from, to)
+          }
         }
       }
       selectors.foreach(checkSelector)

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

@@ -188,19 +188,22 @@ trait NamesDefaults { self: Analyzer =>
 
       // never used for constructor calls, they always have a stable qualifier
       def blockWithQualifier(qual: Tree, selected: Name) = {
-        val sym = blockTyper.context.owner.newValue(freshTermName(nme.QUAL_PREFIX)(typer.fresh), newFlags = ARTIFACT) setInfo uncheckedBounds(qual.tpe) setPos (qual.pos.makeTransparent)
+        val sym = blockTyper.context.owner.newValue(freshTermName(nme.QUAL_PREFIX)(typer.fresh), newFlags = ARTIFACT)
+                  .setInfo(uncheckedBounds(qual.tpe))
+                  .setPos(qual.pos.makeTransparent)
         blockTyper.context.scope enter sym
         val vd = atPos(sym.pos)(ValDef(sym, qual) setType NoType)
-        // it stays in Vegas: scala/bug#5720, scala/bug#5727
-        qual.changeOwner(blockTyper.context.owner, sym)
+        qual.changeOwner(blockTyper.context.owner, sym) // scala/bug#5720, scala/bug#5727
 
         val newQual = atPos(qual.pos.focus)(blockTyper.typedQualifier(Ident(sym.name)))
         val baseFunTransformed = atPos(baseFun.pos.makeTransparent) {
           // setSymbol below is important because the 'selected' function might be overloaded. by
           // assigning the correct method symbol, typedSelect will just assign the type. the reason
           // to still call 'typed' is to correctly infer singleton types, scala/bug#5259.
           val selectPos =
-            if(qual.pos.isRange && baseFun1.pos.isRange) qual.pos.union(baseFun1.pos).withStart(Math.min(qual.pos.end, baseFun1.pos.end))
+            if (qual.pos.isRange && baseFun1.pos.isRange)
+              if (qual.pos == baseFun1.pos) qual.pos
+              else baseFun1.pos.union(qual.pos).withStart(Math.min(qual.pos.end, baseFun1.pos.end)) // use basefun point; why isn't start always qual.pos.end
             else baseFun1.pos
           val f = blockTyper.typedOperator(Select(newQual, selected).setSymbol(baseFun1.symbol).setPos(selectPos))
           if (funTargs.isEmpty) f

src/compiler/scala/tools/reflect/FastStringInterpolator.scala

@@ -13,6 +13,9 @@
 package scala.tools
 package reflect
 
+import scala.StringContext.{InvalidEscapeException, InvalidUnicodeEscapeException, processEscapes, processUnicode}
+import scala.collection.mutable.ListBuffer
+import scala.reflect.internal.util.Position
 import nsc.Reporting.WarningCategory.Scala3Migration
 
 trait FastStringInterpolator extends FormatInterpolator {
@@ -26,45 +29,46 @@ trait FastStringInterpolator extends FormatInterpolator {
   // rewrite a tree like `scala.StringContext.apply("hello \\n ", " ", "").s("world", Test.this.foo)`
   // to `"hello \n world ".+(Test.this.foo)`
   private def interpolated(macroApp: Tree, isRaw: Boolean): Tree = macroApp match {
-    case Apply(Select(Apply(stringCtx@Select(qualSC, _), parts), _interpol), args) if
-      stringCtx.symbol == currentRun.runDefinitions.StringContext_apply &&
-      treeInfo.isQualifierSafeToElide(qualSC) &&
-      parts.forall(treeInfo.isLiteralString) &&
-      parts.length == (args.length + 1) =>
+    case Apply(Select(Apply(stringCtx @ Select(qualSC, _), parts), _interpol), args)
+    if stringCtx.symbol == currentRun.runDefinitions.StringContext_apply
+    && treeInfo.isQualifierSafeToElide(qualSC)
+    && parts.forall(treeInfo.isLiteralString)
+    && parts.length == (args.length + 1) =>
 
-      val treated = 
-        try
-          parts.mapConserve {
-            case lit @ Literal(Constant(stringVal: String)) =>
-              def asRaw = if (currentRun.sourceFeatures.unicodeEscapesRaw) stringVal else {
-                val processed = StringContext.processUnicode(stringVal)
-                if (processed == stringVal) stringVal else {
-                  val pos = {
-                    val diffindex = processed.zip(stringVal).zipWithIndex.collectFirst {
-                      case ((p, o), i) if p != o => i
-                    }.getOrElse(processed.length - 1)
-                    lit.pos.withShift(diffindex)
-                  }
-                  def msg(fate: String) = s"Unicode escapes in raw interpolations are $fate; use literal characters instead"
-                  if (currentRun.isScala3)
-                    runReporting.warning(pos, msg("ignored in Scala 3 (or with -Xsource-features:unicode-escapes-raw)"), Scala3Migration, c.internal.enclosingOwner)
-                  else
-                    runReporting.deprecationWarning(pos, msg("deprecated"), "2.13.2", "", "")
-                  processed
-                }
+      def adjustedEscPos(p: Position, delta: Int) = {
+        val start = p.start + delta
+        Position.range(p.source, start = start, point = start, end = start + 2)
+      }
+      val treated = parts.mapConserve {
+        case lit @ Literal(Constant(stringVal: String)) =>
+          def asRaw = if (currentRun.sourceFeatures.unicodeEscapesRaw) stringVal else {
+            val processed = processUnicode(stringVal)
+            if (processed == stringVal) stringVal else {
+              val pos = {
+                val diffindex = processed.zip(stringVal).zipWithIndex.collectFirst {
+                  case ((p, o), i) if p != o => i
+                }.getOrElse(processed.length - 1)
+                lit.pos.withShift(diffindex)
               }
-              val value =
-                if (isRaw) asRaw
-                else StringContext.processEscapes(stringVal)
-              val k = Constant(value)
-              // To avoid the backlash of backslash, taken literally by Literal, escapes are processed strictly (scala/bug#11196)
-              treeCopy.Literal(lit, k).setType(ConstantType(k))
-            case x => throw new MatchError(x)
+              def msg(fate: String) = s"Unicode escapes in raw interpolations are $fate; use literal characters instead"
+              if (currentRun.isScala3)
+                runReporting.warning(pos, msg("ignored in Scala 3 (or with -Xsource-features:unicode-escapes-raw)"), Scala3Migration, c.internal.enclosingOwner)
+              else
+                runReporting.deprecationWarning(pos, msg("deprecated"), "2.13.2", "", "")
+              processed
+            }
           }
-        catch {
-          case ie: StringContext.InvalidEscapeException => c.abort(parts.head.pos.withShift(ie.index), ie.getMessage)
-          case iue: StringContext.InvalidUnicodeEscapeException => c.abort(parts.head.pos.withShift(iue.index), iue.getMessage)
-        }
+          val value =
+            try if (isRaw) asRaw else processEscapes(stringVal)
+            catch {
+              case ie: InvalidEscapeException => c.abort(adjustedEscPos(lit.pos, ie.index), ie.getMessage)
+              case iue: InvalidUnicodeEscapeException => c.abort(adjustedEscPos(lit.pos, iue.index), iue.getMessage)
+            }
+          val k = Constant(value)
+          // To avoid the backlash of backslash, taken literally by Literal, escapes are processed strictly (scala/bug#11196)
+          treeCopy.Literal(lit, k).setType(ConstantType(k))
+        case x => throw new MatchError(x)
+      }
 
       if (args.forall(treeInfo.isLiteralString)) {
         val it1 = treated.iterator
@@ -82,7 +86,7 @@ trait FastStringInterpolator extends FormatInterpolator {
       else concatenate(treated, args)
 
     // Fallback -- inline the original implementation of the `s` or `raw` interpolator.
-    case t@Apply(Select(someStringContext, _interpol), args) =>
+    case Apply(Select(someStringContext, _interpol), args) =>
       q"""{
         val sc = $someStringContext
         _root_.scala.StringContext.standardInterpolator(
@@ -95,28 +99,25 @@ trait FastStringInterpolator extends FormatInterpolator {
 
   def concatenate(parts: List[Tree], args: List[Tree]): Tree = {
     val argsIndexed = args.toVector
-    val concatArgs = collection.mutable.ListBuffer[Tree]()
+    val concatArgs = ListBuffer.empty[Tree]
     val numLits = parts.length
     foreachWithIndex(parts.tail) { (lit, i) =>
       val treatedContents = lit.asInstanceOf[Literal].value.stringValue
       val emptyLit = treatedContents.isEmpty
-      if (i < numLits - 1) {
+      if (i < numLits - 1)
         concatArgs += argsIndexed(i)
-        if (!emptyLit) concatArgs += lit
-      } else if (!emptyLit) {
-        concatArgs += lit
-      }
+      if (!emptyLit) concatArgs += lit
     }
     def mkConcat(pos: Position, lhs: Tree, rhs: Tree): Tree =
       atPos(pos)(gen.mkMethodCall(gen.mkAttributedSelect(lhs, definitions.String_+), rhs :: Nil)).setType(definitions.StringTpe)
 
     var result: Tree = parts.head
     val chunkSize = 32
-    if (concatArgs.lengthCompare(chunkSize) <= 0) {
+    if (concatArgs.lengthCompare(chunkSize) <= 0)
       concatArgs.foreach { t =>
         result = mkConcat(t.pos, result, t)
       }
-    } else {
+    else
       concatArgs.toList.grouped(chunkSize).foreach {
         case group =>
           var chunkResult: Tree = Literal(Constant("")).setType(definitions.StringTpe)
@@ -125,7 +126,6 @@ trait FastStringInterpolator extends FormatInterpolator {
           }
           result = mkConcat(chunkResult.pos, result, chunkResult)
       }
-    }
 
     result
   }

src/reflect/scala/reflect/internal/Positions.scala

@@ -46,23 +46,35 @@ trait Positions extends api.Positions { self: SymbolTable =>
    *  Otherwise returns default position that is either focused or not.
    */
   def wrappingPos(default: Position, trees: List[Tree]): Position = wrappingPos(default, trees, focus = true)
-  def wrappingPos(default: Position, trees: List[Tree], focus: Boolean): Position = {
-    if (useOffsetPositions) default else {
-      val accum = new WrappingPosAccumulator()
-      var rest = trees
-      while (rest ne Nil) {
-        val head = rest.head
-        rest = rest.tail
-        // TODO: a tree's range position should cover the positions of all trees it "includes"
-        // (inclusion mostly refers to subtrees, but also other attributes reached through the tree, such as its annotations/modifiers);
-        // concretely, a MemberDef's position should cover its annotations (scala/bug#11060)
-        // Workaround, which explicitly includes annotations of traversed trees, can be removed when TODO above is resolved:
-        head match { case md: MemberDef => rest = md.mods.annotations ::: rest  case _ => }
 
+  /** If the default point falls outside the calculated range, widen the result to include it.
+   */
+  def wideningPos(default: Position, trees: List[Tree]): Position = {
+    val res = wrappingPos(default, trees)
+    val pointed = default.isDefined && default.start <= default.point && default.point <= default.end // has a point
+    if (pointed && (default.point < res.start || default.point > res.end)) {
+      val start = Math.min(res.start, default.point)
+      val end   = Math.max(res.end, default.point)
+      Position.range(default.source, start = start, point = default.point, end = end)
+    }
+    else res
+  }
+  def wrappingPos(default: Position, trees: List[Tree], focus: Boolean): Position = if (useOffsetPositions) default else {
+    // TODO: a tree's range position should cover the positions of all trees it "includes"
+    // (inclusion mostly refers to subtrees, but also other attributes reached through the tree, such as its annotations/modifiers);
+    // concretely, a MemberDef's position should cover its annotations (scala/bug#11060)
+    // Workaround, which explicitly includes annotations of traversed trees, can be removed when TODO above is resolved:
+    val accum = new WrappingPosAccumulator()
+    def loop(trees: List[Tree]): Position = trees match {
+      case head :: rest =>
         accum(head)
-      }
-      accum.result(default, focus)
+        head match {
+          case md: MemberDef => loop(md.mods.annotations ::: rest)
+          case _ => loop(rest)
+        }
+      case _ => accum.result(default, focus)
     }
+    loop(trees)
   }
   private final class WrappingPosAccumulator extends (Tree => Boolean) {
     private[this] var min: Int = _
@@ -72,11 +84,16 @@ trait Positions extends api.Positions { self: SymbolTable =>
       max = Int.MinValue
     }
     reset()
-    def result(default: Position, focus: Boolean): Position = {
-      if (min > max)
-        if (focus) default.focus else default //there are no ranges
-      else Position.range(default.source, min, default.pointOrElse(min), max)
-    }
+    def result(default: Position, focus: Boolean): Position =
+      if (min > max) // there are no ranges
+        if (focus) default.focus else default
+      else { // ignore default.point not included by range min..max
+        val pointed = default.isDefined && default.start <= default.point && default.point <= default.end // has a point
+        val point =
+          if (pointed && min <= default.point && default.point <= max) default.point
+          else min
+        Position.range(default.source, start = min, point = point, end = max)
+      }
     override def apply(v1: Tree): Boolean = {
       val pos = v1.pos
       if (pos.isRange) {
@@ -103,9 +120,8 @@ trait Positions extends api.Positions { self: SymbolTable =>
    *  shortening the range, assigning TransparentPositions
    *  to some of the nodes in `tree` or focusing on the position.
    */
-  def ensureNonOverlapping(tree: Tree, others: List[Tree]): Unit ={ ensureNonOverlapping(tree, others, focus = true) }
-  def ensureNonOverlapping(tree: Tree, others: List[Tree], focus: Boolean): Unit = {
-    if (useOffsetPositions) return
+  def ensureNonOverlapping(tree: Tree, others: List[Tree]): Unit = ensureNonOverlapping(tree, others, focus = true)
+  def ensureNonOverlapping(tree: Tree, others: List[Tree], focus: Boolean): Unit = if (!useOffsetPositions) {
 
     def isOverlapping(pos: Position) =
       pos.isRange && (others exists (pos overlaps _.pos))
@@ -305,15 +321,15 @@ trait Positions extends api.Positions { self: SymbolTable =>
     def set(pos: Position, parent: Tree): Unit = {
       wrappingPosAccumulator.reset()
       this.pos = pos
-      try parent.foreachChild(this)
+      try parent.foreachChild(apply)
       finally {
         this.pos = null
       }
     }
     def apply(tree: Tree): Boolean = {
       wrappingPosAccumulator.reset()
       if (!tree.isEmpty && tree.canHaveAttrs && tree.pos == NoPosition) {
-        tree.foreachChild(this)
+        tree.foreachChild(apply)
         tree.foreachChild(wrappingPosAccumulator)
         val wrappingPos = wrappingPosAccumulator.result(pos, focus = true)
         tree setPos wrappingPos

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

@@ -423,13 +423,10 @@ abstract class TreeGen {
         if (vparamss1.isEmpty || !vparamss1.head.isEmpty && vparamss1.head.head.mods.isImplicit)
           vparamss1 = List() :: vparamss1
         val superCall = pendingSuperCall // we can't know in advance which of the parents will end up as a superclass
-                                         // this requires knowing which of the parents is a type macro and which is not
-                                         // and that's something that cannot be found out before typer
-                                         // (the type macros aren't in the trunk yet, but there is a plan for them to land there soon)
                                          // this means that we don't know what will be the arguments of the super call
-                                         // therefore here we emit a dummy which gets populated when the template is named and typechecked
+                                         // here we emit a dummy which gets populated when the template is named and typechecked
         Some(
-          atPos(wrappingPos(superPos, lvdefs ::: vparamss1.flatten).makeTransparent) (
+          atPos(wideningPos(superPos, lvdefs ::: vparamss1.flatten).makeTransparent) (
             DefDef(constrMods, nme.CONSTRUCTOR, List(), vparamss1, TypeTree(), Block(lvdefs ::: List(superCall), mkLiteralUnit))))
       }
     }
@@ -666,8 +663,14 @@ abstract class TreeGen {
      * the limits given by pat and body.
      */
     def makeClosure(pos: Position, pat: Tree, body: Tree): Tree = {
-      def wrapped  = wrappingPos(List(pat, body))
-      def splitpos = (if (pos != NoPosition) wrapped.withPoint(pos.point) else pos).makeTransparent
+      val splitpos = {
+        val wrapped = wrappingPos(List(pat, body))
+        // ignore proposed point if not in range
+        val res =
+          if (pos != NoPosition && wrapped.start <= pos.point && pos.point < wrapped.end) wrapped.withPoint(pos.point)
+          else pos
+        res.makeTransparent
+      }
       matchVarPattern(pat) match {
         case Some((name, tpt)) =>
           Function(

src/reflect/scala/reflect/internal/util/Position.scala

@@ -37,9 +37,12 @@ object Position {
   final val tabInc = 8
 
   private def validate[T <: Position](pos: T): T = {
-    if (pos.isRange)
-      assert(pos.start <= pos.end, s"bad position: ${pos.show}")
-
+    if (pos.isRange) {
+      import pos.{pos => _, _}
+      assert(start <= end, s"bad position: ${pos.show}")
+      //assert(start <= point && point <= end, s"bad position: point $point out of range $start..$end: ${pos.show}")
+      //assert(start <= point && point <= end, s"bad position: point $point out of range $start..$end: ${pos.show}\n${pos.lineContent}\n${pos.lineCaret}")
+    }
     pos
   }
 

test/files/neg/annots-constant-neg.check

@@ -3,7 +3,7 @@ Test.scala:12: error: class Ann1 cannot have auxiliary constructors because it e
       ^
 Test.scala:14: error: class Ann2 needs to have exactly one argument list because it extends ConstantAnnotation
 class Ann2(x: Int)(y: Int) extends ConstantAnnotation // err
-                                   ^
+          ^
 Test.scala:27: error: annotation argument needs to be a constant; found: Test.this.nonConst
   @JAnn(nonConst) def t3 = 0 // err
         ^
@@ -68,7 +68,7 @@ Test.scala:66: error: annotation argument needs to be a constant; found: java.la
 Test.scala:69: error: Java annotation SuppressWarnings is abstract; cannot be instantiated
 Error occurred in an application involving default arguments.
   @Ann(value = 0, c = Array(new SuppressWarnings(value = Array("")))) def u17 = 0 // err
-                            ^
+                                ^
 Test.scala:69: error: not found: value value
 Error occurred in an application involving default arguments.
   @Ann(value = 0, c = Array(new SuppressWarnings(value = Array("")))) def u17 = 0 // err

test/files/neg/java-annotation-bad.check

@@ -3,13 +3,13 @@ Test_1.scala:12: error: Java annotation Ann_0 is abstract; cannot be instantiate
                  ^
 Test_1.scala:13: error: Java annotation Ann_0 is abstract; cannot be instantiated
   val b: Ann_0 = new Ann_0(Array()) // nok
-                 ^
+                     ^
 Test_1.scala:14: error: Java annotation Ann_1 is abstract; cannot be instantiated
   val c: Ann_1 = new Ann_1          // nok
                  ^
 Test_1.scala:15: error: Java annotation Ann_1 is abstract; cannot be instantiated
   val d: Ann_1 = new Ann_1(Array()) // nok
-                 ^
+                     ^
 Test_1.scala:18: error: type mismatch;
  found   : ann.Ann_0
  required: scala.annotation.Annotation