From a8edefcef8905ed3487c7293056f6d0946e79dd7 Mon Sep 17 00:00:00 2001 From: Eugene Burmako Date: Wed, 20 Mar 2013 16:09:04 +0100 Subject: SI-7271 fixes positions of string interpolation parts Positions of static parts are now set explicitly during parsing rather than filled in wholesale during subsequent atPos after parsing. I also had to change the offsets that scanner uses for initial static parts of string interpolations so that they no longer point to the opening double quote, but rather to the actual beginning of the part. --- src/compiler/scala/tools/nsc/ast/parser/Parsers.scala | 4 ++-- src/compiler/scala/tools/nsc/ast/parser/Scanners.scala | 2 ++ 2 files changed, 4 insertions(+), 2 deletions(-) (limited to 'src') diff --git a/src/compiler/scala/tools/nsc/ast/parser/Parsers.scala b/src/compiler/scala/tools/nsc/ast/parser/Parsers.scala index 6f79f639b9..9c936ea2c5 100644 --- a/src/compiler/scala/tools/nsc/ast/parser/Parsers.scala +++ b/src/compiler/scala/tools/nsc/ast/parser/Parsers.scala @@ -1152,7 +1152,7 @@ self => val exprBuf = new ListBuffer[Tree] in.nextToken() while (in.token == STRINGPART) { - partsBuf += literal() + partsBuf += atPos(in.offset)(literal()) exprBuf += { if (inPattern) dropAnyBraces(pattern()) else { @@ -1166,7 +1166,7 @@ self => } } } - if (in.token == STRINGLIT) partsBuf += literal() + if (in.token == STRINGLIT) partsBuf += atPos(in.offset)(literal()) val t1 = atPos(o2p(start)) { Ident(nme.StringContext) } val t2 = atPos(start) { Apply(t1, partsBuf.toList) } diff --git a/src/compiler/scala/tools/nsc/ast/parser/Scanners.scala b/src/compiler/scala/tools/nsc/ast/parser/Scanners.scala index c05906c740..1aa50be83a 100644 --- a/src/compiler/scala/tools/nsc/ast/parser/Scanners.scala +++ b/src/compiler/scala/tools/nsc/ast/parser/Scanners.scala @@ -425,6 +425,7 @@ trait Scanners extends ScannersCommon { if (ch == '\"') { nextRawChar() if (ch == '\"') { + offset += 3 nextRawChar() getStringPart(multiLine = true) sepRegions = STRINGPART :: sepRegions // indicate string part @@ -434,6 +435,7 @@ trait Scanners extends ScannersCommon { strVal = "" } } else { + offset += 1 getStringPart(multiLine = false) sepRegions = STRINGLIT :: sepRegions // indicate single line string part } -- cgit v1.2.3 From cb1a427d8ad4a92690610bfa7d6c79f48d440864 Mon Sep 17 00:00:00 2001 From: Eugene Burmako Date: Sat, 27 Apr 2013 15:26:34 +0200 Subject: 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. --- .../scala/tools/reflect/MacroImplementations.scala | 78 ++++++++++++++-------- .../scala/reflect/internal/util/Statistics.scala | 2 +- test/files/neg/stringinterpolation_macro-neg.check | 2 +- test/files/neg/t7325.check | 19 ++++++ test/files/neg/t7325.scala | 25 +++++++ test/files/run/interpolation.scala | 2 +- test/files/run/interpolationMultiline1.scala | 2 +- test/files/run/t7325.check | 19 ++++++ test/files/run/t7325.scala | 25 +++++++ 9 files changed, 141 insertions(+), 33 deletions(-) create mode 100644 test/files/neg/t7325.check create mode 100644 test/files/neg/t7325.scala create mode 100644 test/files/run/t7325.check create mode 100644 test/files/run/t7325.scala (limited to 'src') 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 -- cgit v1.2.3 From 25f49cb392846052025d745e2403b681793ce647 Mon Sep 17 00:00:00 2001 From: Eugene Burmako Date: Sat, 27 Apr 2013 21:45:23 +0200 Subject: literal() now assigns a position to the tree it produces At all its callsites except for a single one, the results of literal() were wrapped in atPos(in.offset), so the simplification was pretty much warranted. Thanks @paulp --- .../scala/tools/nsc/ast/parser/Parsers.scala | 63 +++++++++++----------- 1 file changed, 32 insertions(+), 31 deletions(-) (limited to 'src') diff --git a/src/compiler/scala/tools/nsc/ast/parser/Parsers.scala b/src/compiler/scala/tools/nsc/ast/parser/Parsers.scala index 9c936ea2c5..a70fb5369d 100644 --- a/src/compiler/scala/tools/nsc/ast/parser/Parsers.scala +++ b/src/compiler/scala/tools/nsc/ast/parser/Parsers.scala @@ -1107,32 +1107,33 @@ self => * | symbol * | null * }}} - * @note The returned tree does not yet have a position */ - def literal(isNegated: Boolean = false, inPattern: Boolean = false): Tree = { - def finish(value: Any): Tree = { - val t = Literal(Constant(value)) - in.nextToken() - t - } - if (in.token == SYMBOLLIT) - Apply(scalaDot(nme.Symbol), List(finish(in.strVal))) - else if (in.token == INTERPOLATIONID) - interpolatedString(inPattern = inPattern) - else finish(in.token match { - case CHARLIT => in.charVal - case INTLIT => in.intVal(isNegated).toInt - case LONGLIT => in.intVal(isNegated) - case FLOATLIT => in.floatVal(isNegated).toFloat - case DOUBLELIT => in.floatVal(isNegated) - case STRINGLIT | STRINGPART => in.strVal.intern() - case TRUE => true - case FALSE => false - case NULL => null - case _ => - syntaxErrorOrIncomplete("illegal literal", true) - null - }) + def literal(isNegated: Boolean = false, inPattern: Boolean = false, start: Int = in.offset): Tree = { + atPos(start) { + def finish(value: Any): Tree = { + val t = Literal(Constant(value)) + in.nextToken() + t + } + if (in.token == SYMBOLLIT) + Apply(scalaDot(nme.Symbol), List(finish(in.strVal))) + else if (in.token == INTERPOLATIONID) + interpolatedString(inPattern = inPattern) + else finish(in.token match { + case CHARLIT => in.charVal + case INTLIT => in.intVal(isNegated).toInt + case LONGLIT => in.intVal(isNegated) + case FLOATLIT => in.floatVal(isNegated).toFloat + case DOUBLELIT => in.floatVal(isNegated) + case STRINGLIT | STRINGPART => in.strVal.intern() + case TRUE => true + case FALSE => false + case NULL => null + case _ => + syntaxErrorOrIncomplete("illegal literal", true) + null + }) + } } private def stringOp(t: Tree, op: TermName) = { @@ -1152,7 +1153,7 @@ self => val exprBuf = new ListBuffer[Tree] in.nextToken() while (in.token == STRINGPART) { - partsBuf += atPos(in.offset)(literal()) + partsBuf += literal() exprBuf += { if (inPattern) dropAnyBraces(pattern()) else { @@ -1166,7 +1167,7 @@ self => } } } - if (in.token == STRINGLIT) partsBuf += atPos(in.offset)(literal()) + if (in.token == STRINGLIT) partsBuf += literal() val t1 = atPos(o2p(start)) { Ident(nme.StringContext) } val t2 = atPos(start) { Apply(t1, partsBuf.toList) } @@ -1510,7 +1511,7 @@ self => atPos(in.offset) { val name = nme.toUnaryName(rawIdent()) if (name == nme.UNARY_- && isNumericLit) - simpleExprRest(atPos(in.offset)(literal(isNegated = true)), canApply = true) + simpleExprRest(literal(isNegated = true), canApply = true) else Select(stripParens(simpleExpr()), name) } @@ -1535,7 +1536,7 @@ self => def simpleExpr(): Tree = { var canApply = true val t = - if (isLiteral) atPos(in.offset)(literal()) + if (isLiteral) literal() else in.token match { case XMLSTART => xmlLiteral() @@ -1924,7 +1925,7 @@ self => case INTLIT | LONGLIT | FLOATLIT | DOUBLELIT => t match { case Ident(nme.MINUS) => - return atPos(start) { literal(isNegated = true, inPattern = true) } + return literal(isNegated = true, inPattern = true, start = start) case _ => } case _ => @@ -1942,7 +1943,7 @@ self => atPos(start, start) { Ident(nme.WILDCARD) } case CHARLIT | INTLIT | LONGLIT | FLOATLIT | DOUBLELIT | STRINGLIT | INTERPOLATIONID | SYMBOLLIT | TRUE | FALSE | NULL => - atPos(start) { literal(inPattern = true) } + literal(inPattern = true) case LPAREN => atPos(start)(makeParens(noSeq.patterns())) case XMLSTART => -- cgit v1.2.3