SI-7325 cleans up corner cases of percent handling in StringContext.f

See comments in code for the exhaustive list of the cases handled.
Also note that treatment of non-formatting percents has been changed.
Embedding literal %'s now requires escaping.

Moreover, this commit also features exact error positions for string
interpolation, something that has been impossible until the fix for
SI-7271, also included in this patch.

9 files changed, 141 insertions, 33 deletions

diff --git a/src/compiler/scala/tools/reflect/MacroImplementations.scala b/src/compiler/scala/tools/reflect/MacroImplementations.scala
index ae13cc561f..f4f385f8b3 100644
--- a/src/compiler/scala/tools/reflect/MacroImplementations.scala
+++ b/src/compiler/scala/tools/reflect/MacroImplementations.scala
@@ -4,6 +4,7 @@ import scala.reflect.macros.{ReificationException, UnexpectedReificationExceptio
 import scala.reflect.macros.runtime.Context
 import scala.collection.mutable.ListBuffer
 import scala.collection.mutable.Stack
+import scala.reflect.internal.util.OffsetPosition
 
 abstract class MacroImplementations {
   val c: Context
@@ -26,16 +27,12 @@ abstract class MacroImplementations {
         c.abort(args(parts.length-1).pos,
             "too many arguments for interpolated string")
     }
-    val stringParts = parts map {
-      case Literal(Constant(s: String)) => s;
-      case _ => throw new IllegalArgumentException("argument parts must be a list of string literals")
-    }
 
-    val pi = stringParts.iterator
+    val pi = parts.iterator
     val bldr = new java.lang.StringBuilder
     val evals = ListBuffer[ValDef]()
     val ids = ListBuffer[Ident]()
-    val argsStack = Stack(args : _*)
+    val argStack = Stack(args : _*)
 
     def defval(value: Tree, tpe: Type): Unit = {
       val freshName = newTermName(c.fresh("arg$"))
@@ -82,50 +79,73 @@ abstract class MacroImplementations {
     }
 
     def copyString(first: Boolean): Unit = {
-      val str = StringContext.treatEscapes(pi.next())
+      val strTree = pi.next()
+      val rawStr = strTree match {
+        case Literal(Constant(str: String)) => str
+        case _ => throw new IllegalArgumentException("internal error: argument parts must be a list of string literals")
+      }
+      val str = StringContext.treatEscapes(rawStr)
       val strLen = str.length
       val strIsEmpty = strLen == 0
-      var start = 0
+      def charAtIndexIs(idx: Int, ch: Char) = idx < strLen && str(idx) == ch
+      def isPercent(idx: Int) = charAtIndexIs(idx, '%')
+      def isConversion(idx: Int) = isPercent(idx) && !charAtIndexIs(idx + 1, 'n') && !charAtIndexIs(idx + 1, '%')
       var idx = 0
 
+      def errorAtIndex(idx: Int, msg: String) = c.error(new OffsetPosition(strTree.pos.source, strTree.pos.point + idx), msg)
+      def wrongConversionString(idx: Int) = errorAtIndex(idx, "wrong conversion string")
+      def illegalConversionCharacter(idx: Int) = errorAtIndex(idx, "illegal conversion character")
+      def nonEscapedPercent(idx: Int) = errorAtIndex(idx, "percent signs not directly following splicees must be escaped")
+
+      // STEP 1: handle argument conversion
+      // 1) "...${smth}" => okay, equivalent to "...${smth}%s"
+      // 2) "...${smth}blahblah" => okay, equivalent to "...${smth}%sblahblah"
+      // 3) "...${smth}%" => error
+      // 4) "...${smth}%n" => okay, equivalent to "...${smth}%s%n"
+      // 5) "...${smth}%%" => okay, equivalent to "...${smth}%s%%"
+      // 6) "...${smth}[%legalJavaConversion]" => okay, according to http://docs.oracle.com/javase/1.5.0/docs/api/java/util/Formatter.html
+      // 7) "...${smth}[%illegalJavaConversion]" => error
       if (!first) {
-        val arg = argsStack.pop
-        if (strIsEmpty || (str charAt 0) != '%') {
-          bldr append "%s"
-          defval(arg, AnyTpe)
-        } else {
+        val arg = argStack.pop
+        if (isConversion(0)) {
           // PRE str is not empty and str(0) == '%'
           // argument index parameter is not allowed, thus parse
           //    [flags][width][.precision]conversion
           var pos = 1
-          while(pos < strLen && isFlag(str charAt pos)) pos += 1
-          while(pos < strLen && Character.isDigit(str charAt pos)) pos += 1
-          if(pos < strLen && str.charAt(pos) == '.') { pos += 1
-            while(pos < strLen && Character.isDigit(str charAt pos)) pos += 1
+          while (pos < strLen && isFlag(str charAt pos)) pos += 1
+          while (pos < strLen && Character.isDigit(str charAt pos)) pos += 1
+          if (pos < strLen && str.charAt(pos) == '.') {
+            pos += 1
+            while (pos < strLen && Character.isDigit(str charAt pos)) pos += 1
           }
-          if(pos < strLen) {
+          if (pos < strLen) {
             conversionType(str charAt pos, arg) match {
               case Some(tpe) => defval(arg, tpe)
-              case None => c.error(arg.pos, "illegal conversion character")
+              case None => illegalConversionCharacter(pos)
             }
           } else {
-            // TODO: place error message on conversion string
-            c.error(arg.pos, "wrong conversion string")
+            wrongConversionString(pos - 1)
           }
+          idx = 1
+        } else {
+          bldr append "%s"
+          defval(arg, AnyTpe)
         }
-        idx = 1
       }
+
+      // STEP 2: handle the rest of the text
+      // 1) %n tokens are left as is
+      // 2) %% tokens are left as is
+      // 3) other usages of percents are reported as errors
       if (!strIsEmpty) {
-        val len = str.length
-        while (idx < len) {
-          def notPercentN = str(idx) != '%' || (idx + 1 < len && str(idx + 1) != 'n')
-          if (str(idx) == '%' && notPercentN) {
-            bldr append (str substring (start, idx)) append "%%"
-            start = idx + 1
+        while (idx < strLen) {
+          if (isPercent(idx)) {
+            if (isConversion(idx)) nonEscapedPercent(idx)
+            else idx += 1 // skip n and % in %n and %%
           }
           idx += 1
         }
-        bldr append (str substring (start, idx))
+        bldr append (str take idx)
       }
     }
 
diff --git a/src/reflect/scala/reflect/internal/util/Statistics.scala b/src/reflect/scala/reflect/internal/util/Statistics.scala
index af4a0263ec..cbd27b0d65 100644
--- a/src/reflect/scala/reflect/internal/util/Statistics.scala
+++ b/src/reflect/scala/reflect/internal/util/Statistics.scala
@@ -103,7 +103,7 @@ quant)
          r <- q :: q.children.toList if r.prefix.nonEmpty) yield r
 
   private def showPercent(x: Double, base: Double) =
-    if (base == 0) "" else f" (${x / base * 100}%2.1f%)"
+    if (base == 0) "" else f" (${x / base * 100}%2.1f%%)"
 
   /** The base trait for quantities.
    *  Quantities with non-empty prefix are printed in the statistics info.
diff --git a/test/files/neg/stringinterpolation_macro-neg.check b/test/files/neg/stringinterpolation_macro-neg.check
index 8986b899a3..457f497f2f 100644
--- a/test/files/neg/stringinterpolation_macro-neg.check
+++ b/test/files/neg/stringinterpolation_macro-neg.check
@@ -66,5 +66,5 @@ Note that implicit conversions are not applicable because they are ambiguous:
        ^
 stringinterpolation_macro-neg.scala:30: error: illegal conversion character
   f"$s%i"
-     ^
+       ^
 15 errors found
diff --git a/test/files/neg/t7325.check b/test/files/neg/t7325.check
new file mode 100644
index 0000000000..709ab6db3e
--- /dev/null
+++ b/test/files/neg/t7325.check
@@ -0,0 +1,19 @@
+t7325.scala:2: error: percent signs not directly following splicees must be escaped
+  println(f"%")
+            ^
+t7325.scala:4: error: percent signs not directly following splicees must be escaped
+  println(f"%%%")
+              ^
+t7325.scala:6: error: percent signs not directly following splicees must be escaped
+  println(f"%%%%%")
+                ^
+t7325.scala:16: error: wrong conversion string
+  println(f"${0}%")
+                ^
+t7325.scala:19: error: percent signs not directly following splicees must be escaped
+  println(f"${0}%%%d")
+                  ^
+t7325.scala:21: error: percent signs not directly following splicees must be escaped
+  println(f"${0}%%%%%d")
+                    ^
+6 errors found
diff --git a/test/files/neg/t7325.scala b/test/files/neg/t7325.scala
new file mode 100644
index 0000000000..adfd8dd47a
--- /dev/null
+++ b/test/files/neg/t7325.scala
@@ -0,0 +1,25 @@
+object Test extends App {
+  println(f"%")
+  println(f"%%")
+  println(f"%%%")
+  println(f"%%%%")
+  println(f"%%%%%")
+  println(f"%%%%%%")
+
+  println(f"%%n")
+  println(f"%%%n")
+  println(f"%%%%n")
+  println(f"%%%%%n")
+  println(f"%%%%%%n")
+  println(f"%%%%%%%n")
+
+  println(f"${0}%")
+  println(f"${0}%d")
+  println(f"${0}%%d")
+  println(f"${0}%%%d")
+  println(f"${0}%%%%d")
+  println(f"${0}%%%%%d")
+
+  println(f"${0}%n")
+  println(f"${0}%d%n")
+}
\ No newline at end of file
diff --git a/test/files/run/interpolation.scala b/test/files/run/interpolation.scala
index f443bd5feb..14d9819348 100644
--- a/test/files/run/interpolation.scala
+++ b/test/files/run/interpolation.scala
@@ -13,7 +13,7 @@ object Test extends App {
     println(s"Best price: $f")
     println(f"Best price: $f%.2f")
     println(s"$f% discount included")
-    println(f"$f%3.2f% discount included")
+    println(f"$f%3.2f%% discount included")
   }
 
   test1(1)
diff --git a/test/files/run/interpolationMultiline1.scala b/test/files/run/interpolationMultiline1.scala
index 437aed44b0..db634e7775 100644
--- a/test/files/run/interpolationMultiline1.scala
+++ b/test/files/run/interpolationMultiline1.scala
@@ -13,7 +13,7 @@ object Test extends App {
     println(s"""Best price: $f""")
     println(f"""Best price: $f%.2f""")
     println(s"""$f% discount included""")
-    println(f"""$f%3.2f% discount included""")
+    println(f"""$f%3.2f%% discount included""")
   }
 
   test1(1)
diff --git a/test/files/run/t7325.check b/test/files/run/t7325.check
new file mode 100644
index 0000000000..3c7652f42c
--- /dev/null
+++ b/test/files/run/t7325.check
@@ -0,0 +1,19 @@
+%
+%%
+%%%
+%n
+%
+
+%%n
+%%
+
+%%%n
+%%%
+
+0
+0%d
+0%%d
+0
+
+0
+
diff --git a/test/files/run/t7325.scala b/test/files/run/t7325.scala
new file mode 100644
index 0000000000..26f6bc6ef7
--- /dev/null
+++ b/test/files/run/t7325.scala
@@ -0,0 +1,25 @@
+object Test extends App {
+  // println(f"%")
+  println(f"%%")
+  // println(f"%%%")
+  println(f"%%%%")
+  // println(f"%%%%%")
+  println(f"%%%%%%")
+
+  println(f"%%n")
+  println(f"%%%n")
+  println(f"%%%%n")
+  println(f"%%%%%n")
+  println(f"%%%%%%n")
+  println(f"%%%%%%%n")
+
+  // println(f"${0}%")
+  println(f"${0}%d")
+  println(f"${0}%%d")
+  // println(f"${0}%%%d")
+  println(f"${0}%%%%d")
+  // println(f"${0}%%%%%d")
+
+  println(f"${0}%n")
+  println(f"${0}%d%n")
+}
\ No newline at end of file