From e6e549cffbdfba270d72fc17a9f68fac37f22c90 Mon Sep 17 00:00:00 2001 From: Som Snytt Date: Fri, 8 Aug 2025 09:00:47 -0700 Subject: [PATCH 1/4] Use interpolation message kind also in s --- .../localopt/StringInterpolatorOpt.scala | 15 ++++++++++---- tests/run/i23693.check | 4 ++++ tests/run/i23693.scala | 17 ++++++++++++++++ tests/warn/i23693.check | 20 +++++++++++++++++++ tests/warn/i23693.scala | 19 ++++++++++++++++++ 5 files changed, 71 insertions(+), 4 deletions(-) create mode 100644 tests/run/i23693.check create mode 100644 tests/run/i23693.scala create mode 100644 tests/warn/i23693.check create mode 100644 tests/warn/i23693.scala diff --git a/compiler/src/dotty/tools/dotc/transform/localopt/StringInterpolatorOpt.scala b/compiler/src/dotty/tools/dotc/transform/localopt/StringInterpolatorOpt.scala index 804150eafc4e..dc810d37a17d 100644 --- a/compiler/src/dotty/tools/dotc/transform/localopt/StringInterpolatorOpt.scala +++ b/compiler/src/dotty/tools/dotc/transform/localopt/StringInterpolatorOpt.scala @@ -10,6 +10,8 @@ import dotty.tools.dotc.core.Contexts.* import dotty.tools.dotc.core.StdNames.* import dotty.tools.dotc.core.Symbols.* import dotty.tools.dotc.core.Types.* +import dotty.tools.dotc.printing.Formatting.* +import dotty.tools.dotc.reporting.BadFormatInterpolation import dotty.tools.dotc.transform.MegaPhase.MiniPhase import dotty.tools.dotc.typer.ConstFold @@ -22,8 +24,9 @@ import dotty.tools.dotc.typer.ConstFold */ class StringInterpolatorOpt extends MiniPhase: import tpd.* + import StringInterpolatorOpt.* - override def phaseName: String = StringInterpolatorOpt.name + override def phaseName: String = name override def description: String = StringInterpolatorOpt.description @@ -31,7 +34,7 @@ class StringInterpolatorOpt extends MiniPhase: tree match case tree: RefTree => val sym = tree.symbol - assert(!StringInterpolatorOpt.isCompilerIntrinsic(sym), + assert(!isCompilerIntrinsic(sym), i"$tree in ${ctx.owner.showLocated} should have been rewritten by phase $phaseName") case _ => @@ -117,10 +120,10 @@ class StringInterpolatorOpt extends MiniPhase: !(tp =:= defn.StringType) && { tp =:= defn.UnitType - && { report.warning("interpolated Unit value", t.srcPos); true } + && { report.warning(bfi"interpolated Unit value", t.srcPos); true } || !tp.isPrimitiveValueType - && { report.warning("interpolation uses toString", t.srcPos); true } + && { report.warning(bfi"interpolation uses toString", t.srcPos); true } } if ctx.settings.Whas.toStringInterpolated then checkIsStringify(t.tpe): Unit @@ -186,3 +189,7 @@ object StringInterpolatorOpt: sym == defn.StringContext_s || sym == defn.StringContext_f || sym == defn.StringContext_raw + + extension (sc: StringContext) + def bfi(args: Shown*)(using Context): BadFormatInterpolation = + BadFormatInterpolation(i(sc)(args*)) diff --git a/tests/run/i23693.check b/tests/run/i23693.check new file mode 100644 index 000000000000..dc8fba77450a --- /dev/null +++ b/tests/run/i23693.check @@ -0,0 +1,4 @@ +k == K(42) +\k == \K(42) +k == +k == K(42) diff --git a/tests/run/i23693.scala b/tests/run/i23693.scala new file mode 100644 index 000000000000..4bd255543a47 --- /dev/null +++ b/tests/run/i23693.scala @@ -0,0 +1,17 @@ +//> using options -Wtostring-interpolated + +// verify warning messages and runtime result +// never mind, the test rig doesn't log diagnostics! unlike beloved partest. + +case class K(i: Int) + +@main def Test = + val k = K(42) + println: + s"k == $k" + println: + raw"\k == \$k" + println: + f"k == $k" + println: + f"k == $k%s" diff --git a/tests/warn/i23693.check b/tests/warn/i23693.check new file mode 100644 index 000000000000..66b238de13a2 --- /dev/null +++ b/tests/warn/i23693.check @@ -0,0 +1,20 @@ +-- [E209] Interpolation Warning: tests/warn/i23693.scala:11:12 --------------------------------------------------------- +11 | s"k == $k" // warn + | ^ + | interpolation uses toString +-- [E209] Interpolation Warning: tests/warn/i23693.scala:13:16 --------------------------------------------------------- +13 | raw"\k == \$k" // warn + | ^ + | interpolation uses toString +-- [E209] Interpolation Warning: tests/warn/i23693.scala:15:12 --------------------------------------------------------- +15 | f"k == $k" // warn + | ^ + | interpolation uses toString +-- [E209] Interpolation Warning: tests/warn/i23693.scala:17:14 --------------------------------------------------------- +17 | f"k == $k%s" // warn + | ^ + | interpolation uses toString +-- [E209] Interpolation Warning: tests/warn/i23693.scala:19:18 --------------------------------------------------------- +19 | s"show == ${k.show}" // warn + | ^^^^^^ + | interpolated Unit value diff --git a/tests/warn/i23693.scala b/tests/warn/i23693.scala new file mode 100644 index 000000000000..79fb7ab155ce --- /dev/null +++ b/tests/warn/i23693.scala @@ -0,0 +1,19 @@ +//> using options -Wtostring-interpolated + +// verify warning messages and runtime result + +case class K(i: Int): + def show: Unit = () + +@main def Test = + val k = K(42) + println: + s"k == $k" // warn + println: + raw"\k == \$k" // warn + println: + f"k == $k" // warn + println: + f"k == $k%s" // warn + println: + s"show == ${k.show}" // warn From 547a186b537fd6f21fae6eb601264305c48608da Mon Sep 17 00:00:00 2001 From: Som Snytt Date: Fri, 8 Aug 2025 09:59:32 -0700 Subject: [PATCH 2/4] Do not discard amended format on warning in f --- .../dotty/tools/dotc/transform/localopt/FormatChecker.scala | 3 +-- tests/run/i23693.check | 2 +- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/transform/localopt/FormatChecker.scala b/compiler/src/dotty/tools/dotc/transform/localopt/FormatChecker.scala index 00daefba3547..83ec7fb8399e 100644 --- a/compiler/src/dotty/tools/dotc/transform/localopt/FormatChecker.scala +++ b/compiler/src/dotty/tools/dotc/transform/localopt/FormatChecker.scala @@ -116,7 +116,7 @@ class TypedFormatChecker(partsElems: List[Tree], parts: List[String], args: List case Nil => loop(parts, n = 0) - if reported then (Nil, Nil) + if reported then (Nil, Nil) // on error, Transform.checked will revert to unamended inputs else assert(argc == actuals.size, s"Expected ${argc} args but got ${actuals.size} for [${parts.mkString(", ")}]") (amended.toList, actuals.toList) @@ -320,5 +320,4 @@ class TypedFormatChecker(partsElems: List[Tree], parts: List[String], args: List .tap(_ => reported = true) def partWarning(message: String, index: Int, offset: Int, end: Int): Unit = r.warning(BadFormatInterpolation(message), partPosAt(index, offset, end)) - .tap(_ => reported = true) end TypedFormatChecker diff --git a/tests/run/i23693.check b/tests/run/i23693.check index dc8fba77450a..32cd0bef7dee 100644 --- a/tests/run/i23693.check +++ b/tests/run/i23693.check @@ -1,4 +1,4 @@ k == K(42) \k == \K(42) -k == +k == K(42) k == K(42) From ed63fe8dfa9e7535ece8ee38ba147bec963a8b93 Mon Sep 17 00:00:00 2001 From: Som Snytt Date: Fri, 8 Aug 2025 10:45:44 -0700 Subject: [PATCH 3/4] Inline extra object to StringInterpOpt --- .../FormatInterpolatorTransform.scala | 39 ------------------- .../localopt/StringInterpolatorOpt.scala | 30 +++++++++++++- 2 files changed, 29 insertions(+), 40 deletions(-) delete mode 100644 compiler/src/dotty/tools/dotc/transform/localopt/FormatInterpolatorTransform.scala diff --git a/compiler/src/dotty/tools/dotc/transform/localopt/FormatInterpolatorTransform.scala b/compiler/src/dotty/tools/dotc/transform/localopt/FormatInterpolatorTransform.scala deleted file mode 100644 index 79d94c26c692..000000000000 --- a/compiler/src/dotty/tools/dotc/transform/localopt/FormatInterpolatorTransform.scala +++ /dev/null @@ -1,39 +0,0 @@ -package dotty.tools.dotc -package transform.localopt - -import dotty.tools.dotc.ast.tpd.* -import dotty.tools.dotc.core.Constants.Constant -import dotty.tools.dotc.core.Contexts.* - -object FormatInterpolatorTransform: - - /** For f"${arg}%xpart", check format conversions and return (format, args) - * suitable for String.format(format, args). - */ - def checked(fun: Tree, args0: Tree)(using Context): (Tree, Tree) = - val (partsExpr, parts) = fun match - case TypeApply(Select(Apply(_, (parts: SeqLiteral) :: Nil), _), _) => - (parts.elems, parts.elems.map { case Literal(Constant(s: String)) => s }) - case _ => - report.error("Expected statically known StringContext", fun.srcPos) - (Nil, Nil) - val (args, elemtpt) = args0 match - case seqlit: SeqLiteral => (seqlit.elems, seqlit.elemtpt) - case _ => - report.error("Expected statically known argument list", args0.srcPos) - (Nil, EmptyTree) - - def literally(s: String) = Literal(Constant(s)) - if parts.lengthIs != args.length + 1 then - val badParts = - if parts.isEmpty then "there are no parts" - else s"too ${if parts.lengthIs > args.length + 1 then "few" else "many"} arguments for interpolated string" - report.error(badParts, fun.srcPos) - (literally(""), args0) - else - val checker = TypedFormatChecker(partsExpr, parts, args) - val (format, formatArgs) = checker.checked - if format.isEmpty then (literally(parts.mkString), args0) - else (literally(format.mkString), SeqLiteral(formatArgs.toList, elemtpt)) - end checked -end FormatInterpolatorTransform diff --git a/compiler/src/dotty/tools/dotc/transform/localopt/StringInterpolatorOpt.scala b/compiler/src/dotty/tools/dotc/transform/localopt/StringInterpolatorOpt.scala index dc810d37a17d..db3a0c6c71f2 100644 --- a/compiler/src/dotty/tools/dotc/transform/localopt/StringInterpolatorOpt.scala +++ b/compiler/src/dotty/tools/dotc/transform/localopt/StringInterpolatorOpt.scala @@ -137,10 +137,38 @@ class StringInterpolatorOpt extends MiniPhase: case _ => false // Perform format checking and normalization, then make it StringOps(fmt).format(args1) with tweaked args def transformF(fun: Tree, args: Tree): Tree = - val (fmt, args1) = FormatInterpolatorTransform.checked(fun, args) + // For f"${arg}%xpart", check format conversions and return (format, args) for String.format(format, args). + def checked(args0: Tree)(using Context): (Tree, Tree) = + val (partsExpr, parts) = fun match + case TypeApply(Select(Apply(_, (parts: SeqLiteral) :: Nil), _), _) => + (parts.elems, parts.elems.map { case Literal(Constant(s: String)) => s }) + case _ => + report.error("Expected statically known StringContext", fun.srcPos) + (Nil, Nil) + val (args, elemtpt) = args0 match + case seqlit: SeqLiteral => (seqlit.elems, seqlit.elemtpt) + case _ => + report.error("Expected statically known argument list", args0.srcPos) + (Nil, EmptyTree) + + def literally(s: String) = Literal(Constant(s)) + if parts.lengthIs != args.length + 1 then + val badParts = + if parts.isEmpty then "there are no parts" + else s"too ${if parts.lengthIs > args.length + 1 then "few" else "many"} arguments for interpolated string" + report.error(badParts, fun.srcPos) + (literally(""), args0) + else + val checker = TypedFormatChecker(partsExpr, parts, args) + val (format, formatArgs) = checker.checked + if format.isEmpty then (literally(parts.mkString), args0) // on error just use unchecked inputs + else (literally(format.mkString), SeqLiteral(formatArgs.toList, elemtpt)) + end checked + val (fmt, args1) = checked(args) resolveConstructor(defn.StringOps.typeRef, List(fmt)) .select(nme.format) .appliedTo(args1) + end transformF // Starting with Scala 2.13, s and raw are macros in the standard // library, so we need to expand them manually. // sc.s(args) --> standardInterpolator(processEscapes, args, sc.parts) From f73185f8c61879ece6e809baa2321dc04a356d42 Mon Sep 17 00:00:00 2001 From: Som Snytt Date: Mon, 11 Aug 2025 12:24:30 -0700 Subject: [PATCH 4/4] Tweak test per review; prefer assert to print --- tests/run/i23693.check | 4 ---- tests/run/i23693.scala | 15 ++++++++++----- tests/warn/i23693.scala | 2 +- 3 files changed, 11 insertions(+), 10 deletions(-) delete mode 100644 tests/run/i23693.check diff --git a/tests/run/i23693.check b/tests/run/i23693.check deleted file mode 100644 index 32cd0bef7dee..000000000000 --- a/tests/run/i23693.check +++ /dev/null @@ -1,4 +0,0 @@ -k == K(42) -\k == \K(42) -k == K(42) -k == K(42) diff --git a/tests/run/i23693.scala b/tests/run/i23693.scala index 4bd255543a47..c77959d99263 100644 --- a/tests/run/i23693.scala +++ b/tests/run/i23693.scala @@ -1,17 +1,22 @@ //> using options -Wtostring-interpolated -// verify warning messages and runtime result +// verify ~warning messages and~ runtime result // never mind, the test rig doesn't log diagnostics! unlike beloved partest. +// Sadly, junit is not available. +//import org.junit.Assert.assertEquals as jassert + +def assertEquals(expected: String)(actual: String): Unit = assert(expected == actual) + case class K(i: Int) @main def Test = val k = K(42) - println: + assertEquals("k == K(42)"): s"k == $k" - println: + assertEquals("\\k == \\K(42)"): raw"\k == \$k" - println: + assertEquals("k == K(42)"): f"k == $k" - println: + assertEquals("k == K(42)"): f"k == $k%s" diff --git a/tests/warn/i23693.scala b/tests/warn/i23693.scala index 79fb7ab155ce..341977dc717c 100644 --- a/tests/warn/i23693.scala +++ b/tests/warn/i23693.scala @@ -1,6 +1,6 @@ //> using options -Wtostring-interpolated -// verify warning messages and runtime result +// verify warning messages; cf run test; we must verify runtime while warning. case class K(i: Int): def show: Unit = ()