From fb9c0e091352836c0cb5f21c49dccfa24d657db8 Mon Sep 17 00:00:00 2001 From: Som Snytt Date: Sun, 16 Jun 2024 07:08:22 -0700 Subject: [PATCH 1/8] Boolean format is null test for non-Boolean --- .../transform/localopt/FormatChecker.scala | 2 +- tests/neg/f-interpolator-neg.check | 8 ++--- tests/neg/f-interpolator-neg.scala | 2 +- tests/warn/tostring-interpolated.scala | 36 +++++++++++++++++++ 4 files changed, 42 insertions(+), 6 deletions(-) create mode 100644 tests/warn/tostring-interpolated.scala diff --git a/compiler/src/dotty/tools/dotc/transform/localopt/FormatChecker.scala b/compiler/src/dotty/tools/dotc/transform/localopt/FormatChecker.scala index 4922024b6c35..0ae9c85b2240 100644 --- a/compiler/src/dotty/tools/dotc/transform/localopt/FormatChecker.scala +++ b/compiler/src/dotty/tools/dotc/transform/localopt/FormatChecker.scala @@ -219,7 +219,7 @@ class TypedFormatChecker(partsElems: List[Tree], parts: List[String], args: List def acceptableVariants: List[Type] = kind match case StringXn => if hasFlag('#') then FormattableType :: Nil else defn.AnyType :: Nil - case BooleanXn => defn.BooleanType :: defn.NullType :: Nil + case BooleanXn => defn.BooleanType :: defn.NullType :: defn.AnyType :: Nil // warn if not boolean case HashXn => defn.AnyType :: Nil case CharacterXn => defn.CharType :: defn.ByteType :: defn.ShortType :: defn.IntType :: Nil case IntegralXn => defn.IntType :: defn.LongType :: defn.ByteType :: defn.ShortType :: BigIntType :: Nil diff --git a/tests/neg/f-interpolator-neg.check b/tests/neg/f-interpolator-neg.check index ea8df052589e..1a931351d09c 100644 --- a/tests/neg/f-interpolator-neg.check +++ b/tests/neg/f-interpolator-neg.check @@ -14,10 +14,10 @@ 7 | new StringContext("", "").f() // error | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ | too few arguments for interpolated string --- Error: tests/neg/f-interpolator-neg.scala:11:7 ---------------------------------------------------------------------- -11 | f"$s%b" // error - | ^ - | Found: (s : String), Required: Boolean, Null +-- Warning: tests/neg/f-interpolator-neg.scala:11:9 -------------------------------------------------------------------- +11 | f"$s%b" // warn only + | ^ + | Boolean format is null test for non-Boolean -- Error: tests/neg/f-interpolator-neg.scala:12:7 ---------------------------------------------------------------------- 12 | f"$s%c" // error | ^ diff --git a/tests/neg/f-interpolator-neg.scala b/tests/neg/f-interpolator-neg.scala index d5c49d5a8111..a4cfe887bb95 100644 --- a/tests/neg/f-interpolator-neg.scala +++ b/tests/neg/f-interpolator-neg.scala @@ -8,7 +8,7 @@ object Test { } def interpolationMismatches(s : String, f : Double, b : Boolean) = { - f"$s%b" // error + f"$s%b" // warn only f"$s%c" // error f"$f%c" // error f"$s%x" // error diff --git a/tests/warn/tostring-interpolated.scala b/tests/warn/tostring-interpolated.scala new file mode 100644 index 000000000000..165bc374b5ef --- /dev/null +++ b/tests/warn/tostring-interpolated.scala @@ -0,0 +1,36 @@ +//> using options -Wtostring-interpolated +//> abusing options -Wconf:cat=w-flag-tostring-interpolated:e -Wtostring-interpolated + +case class C(x: Int) + +trait T { + def c = C(42) + def f = f"$c" // warn + def s = s"$c" // warn + def r = raw"$c" // warn + + def format = f"${c.x}%d in $c or $c%s" // warn using c.toString // warn + + def bool = f"$c%b" // warn just a null check + + def oops = s"${null} slipped thru my fingers" // warn + + def ok = s"${c.toString}" + + def sb = new StringBuilder().append("hello") + def greeting = s"$sb, world" // warn +} + +class Mitigations { + + val s = "hello, world" + val i = 42 + def shown = println("shown") + + def ok = s"$s is ok" + def jersey = s"number $i" + def unitized = s"unfortunately $shown" // maybe tell them about unintended ()? + + def nopct = f"$s is ok" + def nofmt = f"number $i" +} From e8522dda32ba5495f369a8000c3baca4a410ea74 Mon Sep 17 00:00:00 2001 From: Som Snytt Date: Sun, 16 Jun 2024 08:12:02 -0700 Subject: [PATCH 2/8] Respect conversion before conform to Any --- .../transform/localopt/FormatChecker.scala | 32 +++++++++++-------- 1 file changed, 18 insertions(+), 14 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/transform/localopt/FormatChecker.scala b/compiler/src/dotty/tools/dotc/transform/localopt/FormatChecker.scala index 0ae9c85b2240..14d45cb28417 100644 --- a/compiler/src/dotty/tools/dotc/transform/localopt/FormatChecker.scala +++ b/compiler/src/dotty/tools/dotc/transform/localopt/FormatChecker.scala @@ -29,8 +29,9 @@ class TypedFormatChecker(partsElems: List[Tree], parts: List[String], args: List def argType(argi: Int, types: Type*): Type = require(argi < argc, s"$argi out of range picking from $types") val tpe = argTypes(argi) - types.find(t => argConformsTo(argi, tpe, t)) - .orElse(types.find(t => argConvertsTo(argi, tpe, t))) + types.find(t => t != defn.AnyType && argConformsTo(argi, tpe, t)) + .orElse(types.find(t => t != defn.AnyType && argConvertsTo(argi, tpe, t))) + .orElse(types.find(t => t == defn.AnyType && argConformsTo(argi, tpe, t))) .getOrElse { report.argError(s"Found: ${tpe.show}, Required: ${types.map(_.show).mkString(", ")}", argi) actuals += args(argi) @@ -72,15 +73,16 @@ class TypedFormatChecker(partsElems: List[Tree], parts: List[String], args: List @tailrec def loop(remaining: List[String], n: Int): Unit = remaining match - case part0 :: more => + case part0 :: remaining => def badPart(t: Throwable): String = "".tap(_ => report.partError(t.getMessage.nn, index = n, offset = 0)) val part = try StringContext.processEscapes(part0) catch badPart val matches = formatPattern.findAllMatchIn(part) def insertStringConversion(): Unit = amended += "%s" + part - convert += Conversion(formatPattern.findAllMatchIn("%s").next(), n) // improve - argType(n-1, defn.AnyType) + val cv = Conversion(n) + cv.accepts(argType(n-1, defn.AnyType)) + convert += cv def errorLeading(op: Conversion) = op.errorAt(Spec)(s"conversions must follow a splice; ${Conversion.literalHelp}") def accept(op: Conversion): Unit = if !op.isLeading then errorLeading(op) @@ -104,8 +106,8 @@ class TypedFormatChecker(partsElems: List[Tree], parts: List[String], args: List if n == 0 && cv.hasFlag('<') then cv.badFlag('<', "No last arg") else if !cv.isLiteral && !cv.isIndexed then errorLeading(cv) - loop(more, n + 1) - case Nil => () + loop(remaining, n + 1) + case Nil => end loop loop(parts, n = 0) @@ -146,9 +148,10 @@ class TypedFormatChecker(partsElems: List[Tree], parts: List[String], args: List // the conversion char is the head of the op string (but see DateTimeXn) val cc: Char = kind match - case ErrorXn => if op.isEmpty then '?' else op(0) - case DateTimeXn => if op.length > 1 then op(1) else '?' - case _ => op(0) + case ErrorXn => if op.isEmpty then '?' else op(0) + case DateTimeXn => if op.length <= 1 then '?' else op(1) + case StringXn => if op.isEmpty then 's' else op(0) // accommodate the default %s + case _ => op(0) def isIndexed: Boolean = index.nonEmpty || hasFlag('<') def isError: Boolean = kind == ErrorXn @@ -209,10 +212,9 @@ class TypedFormatChecker(partsElems: List[Tree], parts: List[String], args: List def accepts(arg: Type): Boolean = kind match case BooleanXn => arg == defn.BooleanType orElse warningAt(CC)("Boolean format is null test for non-Boolean") - case IntegralXn => - arg == BigIntType || !cond(cc) { - case 'o' | 'x' | 'X' if hasAnyFlag("+ (") => "+ (".filter(hasFlag).foreach(bad => badFlag(bad, s"only use '$bad' for BigInt conversions to o, x, X")) ; true - } + case IntegralXn => arg == BigIntType || !cond(cc) { + case 'o' | 'x' | 'X' if hasAnyFlag("+ (") => "+ (".filter(hasFlag).foreach(bad => badFlag(bad, s"only use '$bad' for BigInt conversions to o, x, X")); true + } case _ => true // what arg type if any does the conversion accept @@ -267,6 +269,8 @@ class TypedFormatChecker(partsElems: List[Tree], parts: List[String], args: List case Some(cc) => new Conversion(m, i, kindOf(cc(0))).tap(_.verify) case None => new Conversion(m, i, ErrorXn).tap(_.errorAt(Spec)(s"Missing conversion operator in '${m.matched}'; $literalHelp")) end apply + // construct a default %s conversion + def apply(i: Int): Conversion = new Conversion(formatPattern.findAllMatchIn("%").next(), i, StringXn) val literalHelp = "use %% for literal %, %n for newline" end Conversion From 31402164bcbc0307167fab5418db6b5d823a1a7e Mon Sep 17 00:00:00 2001 From: Som Snytt Date: Sun, 16 Jun 2024 10:13:16 -0700 Subject: [PATCH 3/8] -Wtostring-interpolated for f --- compiler/src/dotty/tools/dotc/config/ScalaSettings.scala | 1 + .../tools/dotc/transform/localopt/FormatChecker.scala | 9 ++++++++- 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/compiler/src/dotty/tools/dotc/config/ScalaSettings.scala b/compiler/src/dotty/tools/dotc/config/ScalaSettings.scala index 99dd80a26cae..36590c2b68d9 100644 --- a/compiler/src/dotty/tools/dotc/config/ScalaSettings.scala +++ b/compiler/src/dotty/tools/dotc/config/ScalaSettings.scala @@ -167,6 +167,7 @@ private sealed trait WarningSettings: private val WenumCommentDiscard = BooleanSetting(WarningSetting, "Wenum-comment-discard", "Warn when a comment ambiguously assigned to multiple enum cases is discarded.") private val WimplausiblePatterns = BooleanSetting(WarningSetting, "Wimplausible-patterns", "Warn if comparison with a pattern value looks like it might always fail.") private val WunstableInlineAccessors = BooleanSetting(WarningSetting, "WunstableInlineAccessors", "Warn an inline methods has references to non-stable binary APIs.") + private val WtoStringInterpolated = BooleanSetting(WarningSetting, "Wtostring-interpolated", "Warn a standard interpolator used toString on a reference type.") private val Wunused: Setting[List[ChoiceWithHelp[String]]] = MultiChoiceHelpSetting( WarningSetting, name = "Wunused", diff --git a/compiler/src/dotty/tools/dotc/transform/localopt/FormatChecker.scala b/compiler/src/dotty/tools/dotc/transform/localopt/FormatChecker.scala index 14d45cb28417..53a9eeac8628 100644 --- a/compiler/src/dotty/tools/dotc/transform/localopt/FormatChecker.scala +++ b/compiler/src/dotty/tools/dotc/transform/localopt/FormatChecker.scala @@ -64,7 +64,7 @@ class TypedFormatChecker(partsElems: List[Tree], parts: List[String], args: List /** For N part strings and N-1 args to interpolate, normalize parts and check arg types. * - * Returns normalized part strings and args, where args correcpond to conversions in tail of parts. + * Returns normalized part strings and args, where args correspond to conversions in tail of parts. */ def checked: (List[String], List[Tree]) = val amended = ListBuffer.empty[String] @@ -83,12 +83,15 @@ class TypedFormatChecker(partsElems: List[Tree], parts: List[String], args: List val cv = Conversion(n) cv.accepts(argType(n-1, defn.AnyType)) convert += cv + cv.lintToString(argTypes(n-1)) + def errorLeading(op: Conversion) = op.errorAt(Spec)(s"conversions must follow a splice; ${Conversion.literalHelp}") def accept(op: Conversion): Unit = if !op.isLeading then errorLeading(op) op.accepts(argType(n-1, op.acceptableVariants*)) amended += part convert += op + op.lintToString(argTypes(n-1)) // after the first part, a leading specifier is required for the interpolated arg; %s is supplied if needed if n == 0 then amended += part @@ -217,6 +220,10 @@ class TypedFormatChecker(partsElems: List[Tree], parts: List[String], args: List } case _ => true + def lintToString(arg: Type): Unit = + if ctx.settings.WtoStringInterpolated.value && kind == StringXn && !(arg =:= defn.StringType) && !isPrimitiveValueType(arg) + then warningAt(CC)("interpolation uses toString") + // what arg type if any does the conversion accept def acceptableVariants: List[Type] = kind match From 764bc40dba93022c97ca33608c320d781bb470ac Mon Sep 17 00:00:00 2001 From: Som Snytt Date: Sun, 16 Jun 2024 17:39:36 -0700 Subject: [PATCH 4/8] Lint concatenation --- .../dotc/transform/localopt/FormatChecker.scala | 2 +- .../transform/localopt/StringInterpolatorOpt.scala | 12 +++++++++--- 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/transform/localopt/FormatChecker.scala b/compiler/src/dotty/tools/dotc/transform/localopt/FormatChecker.scala index 53a9eeac8628..e1720e7d5887 100644 --- a/compiler/src/dotty/tools/dotc/transform/localopt/FormatChecker.scala +++ b/compiler/src/dotty/tools/dotc/transform/localopt/FormatChecker.scala @@ -221,7 +221,7 @@ class TypedFormatChecker(partsElems: List[Tree], parts: List[String], args: List case _ => true def lintToString(arg: Type): Unit = - if ctx.settings.WtoStringInterpolated.value && kind == StringXn && !(arg =:= defn.StringType) && !isPrimitiveValueType(arg) + if ctx.settings.WtoStringInterpolated.value && kind == StringXn && !(arg.widen =:= defn.StringType) && !arg.isPrimitiveValueType then warningAt(CC)("interpolation uses toString") // what arg type if any does the conversion accept diff --git a/compiler/src/dotty/tools/dotc/transform/localopt/StringInterpolatorOpt.scala b/compiler/src/dotty/tools/dotc/transform/localopt/StringInterpolatorOpt.scala index 7743054f5487..96ca43a18c3a 100644 --- a/compiler/src/dotty/tools/dotc/transform/localopt/StringInterpolatorOpt.scala +++ b/compiler/src/dotty/tools/dotc/transform/localopt/StringInterpolatorOpt.scala @@ -96,16 +96,22 @@ class StringInterpolatorOpt extends MiniPhase: def mkConcat(strs: List[Literal], elems: List[Tree]): Tree = val stri = strs.iterator val elemi = elems.iterator - var result: Tree = stri.next + var result: Tree = stri.next() def concat(tree: Tree): Unit = result = result.select(defn.String_+).appliedTo(tree).withSpan(tree.span) while elemi.hasNext do - concat(elemi.next) - val str = stri.next + val elem = elemi.next() + lintToString(elem) + concat(elem) + val str = stri.next() if !str.const.stringValue.isEmpty then concat(str) result end mkConcat + def lintToString(t: Tree): Unit = + val arg: Type = t.tpe + if ctx.settings.WtoStringInterpolated.value && !(arg.widen =:= defn.StringType) && !arg.isPrimitiveValueType + then report.warning("interpolation uses toString", t.srcPos) val sym = tree.symbol // Test names first to avoid loading scala.StringContext if not used, and common names first val isInterpolatedMethod = From 4f06364ba1efc0004624802497a646e41ce427e2 Mon Sep 17 00:00:00 2001 From: Som Snytt Date: Thu, 22 Aug 2024 12:55:12 -0700 Subject: [PATCH 5/8] -Wall of shame --- compiler/src/dotty/tools/dotc/config/ScalaSettings.scala | 1 + .../src/dotty/tools/dotc/transform/localopt/FormatChecker.scala | 2 +- .../tools/dotc/transform/localopt/StringInterpolatorOpt.scala | 2 +- 3 files changed, 3 insertions(+), 2 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/config/ScalaSettings.scala b/compiler/src/dotty/tools/dotc/config/ScalaSettings.scala index 36590c2b68d9..0517d5b46b9e 100644 --- a/compiler/src/dotty/tools/dotc/config/ScalaSettings.scala +++ b/compiler/src/dotty/tools/dotc/config/ScalaSettings.scala @@ -309,6 +309,7 @@ private sealed trait WarningSettings: def enumCommentDiscard(using Context): Boolean = allOr(WenumCommentDiscard) def implausiblePatterns(using Context): Boolean = allOr(WimplausiblePatterns) def unstableInlineAccessors(using Context): Boolean = allOr(WunstableInlineAccessors) + def toStringInterpolated(using Context): Boolean = allOr(WtoStringInterpolated) def checkInit(using Context): Boolean = allOr(WcheckInit) /** -X "Extended" or "Advanced" settings */ diff --git a/compiler/src/dotty/tools/dotc/transform/localopt/FormatChecker.scala b/compiler/src/dotty/tools/dotc/transform/localopt/FormatChecker.scala index e1720e7d5887..5aec3810d877 100644 --- a/compiler/src/dotty/tools/dotc/transform/localopt/FormatChecker.scala +++ b/compiler/src/dotty/tools/dotc/transform/localopt/FormatChecker.scala @@ -221,7 +221,7 @@ class TypedFormatChecker(partsElems: List[Tree], parts: List[String], args: List case _ => true def lintToString(arg: Type): Unit = - if ctx.settings.WtoStringInterpolated.value && kind == StringXn && !(arg.widen =:= defn.StringType) && !arg.isPrimitiveValueType + if ctx.settings.Whas.toStringInterpolated && kind == StringXn && !(arg.widen =:= defn.StringType) && !arg.isPrimitiveValueType then warningAt(CC)("interpolation uses toString") // what arg type if any does the conversion accept diff --git a/compiler/src/dotty/tools/dotc/transform/localopt/StringInterpolatorOpt.scala b/compiler/src/dotty/tools/dotc/transform/localopt/StringInterpolatorOpt.scala index 96ca43a18c3a..1afcfbac6206 100644 --- a/compiler/src/dotty/tools/dotc/transform/localopt/StringInterpolatorOpt.scala +++ b/compiler/src/dotty/tools/dotc/transform/localopt/StringInterpolatorOpt.scala @@ -110,7 +110,7 @@ class StringInterpolatorOpt extends MiniPhase: end mkConcat def lintToString(t: Tree): Unit = val arg: Type = t.tpe - if ctx.settings.WtoStringInterpolated.value && !(arg.widen =:= defn.StringType) && !arg.isPrimitiveValueType + if ctx.settings.Whas.toStringInterpolated && !(arg.widen =:= defn.StringType) && !arg.isPrimitiveValueType then report.warning("interpolation uses toString", t.srcPos) val sym = tree.symbol // Test names first to avoid loading scala.StringContext if not used, and common names first From ee6babc5df4dd038a63cb9e1434cbf9072cf6b3c Mon Sep 17 00:00:00 2001 From: Som Snytt Date: Mon, 7 Apr 2025 09:19:52 -0700 Subject: [PATCH 6/8] Improve weird bool fmt message --- .../transform/localopt/FormatChecker.scala | 23 +++++++++++-------- tests/neg/f-interpolator-neg.check | 2 +- 2 files changed, 14 insertions(+), 11 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/transform/localopt/FormatChecker.scala b/compiler/src/dotty/tools/dotc/transform/localopt/FormatChecker.scala index 5aec3810d877..8a745732ae65 100644 --- a/compiler/src/dotty/tools/dotc/transform/localopt/FormatChecker.scala +++ b/compiler/src/dotty/tools/dotc/transform/localopt/FormatChecker.scala @@ -5,8 +5,6 @@ import scala.annotation.tailrec import scala.collection.mutable.ListBuffer import scala.util.matching.Regex.Match -import PartialFunction.cond - import dotty.tools.dotc.ast.tpd.{Match => _, *} import dotty.tools.dotc.core.Contexts.* import dotty.tools.dotc.core.Symbols.* @@ -129,10 +127,8 @@ class TypedFormatChecker(partsElems: List[Tree], parts: List[String], args: List def intOf(g: SpecGroup): Option[Int] = group(g).map(_.toInt) extension (inline value: Boolean) - inline def or(inline body: => Unit): Boolean = value || { body ; false } - inline def orElse(inline body: => Unit): Boolean = value || { body ; true } - inline def and(inline body: => Unit): Boolean = value && { body ; true } - inline def but(inline body: => Unit): Boolean = value && { body ; false } + inline infix def or(inline body: => Unit): Boolean = value || { body; false } + inline infix def and(inline body: => Unit): Boolean = value && { body; true } enum Kind: case StringXn, HashXn, BooleanXn, CharacterXn, IntegralXn, FloatingPointXn, DateTimeXn, LiteralXn, ErrorXn @@ -214,11 +210,18 @@ class TypedFormatChecker(partsElems: List[Tree], parts: List[String], args: List // is the specifier OK with the given arg def accepts(arg: Type): Boolean = kind match - case BooleanXn => arg == defn.BooleanType orElse warningAt(CC)("Boolean format is null test for non-Boolean") - case IntegralXn => arg == BigIntType || !cond(cc) { - case 'o' | 'x' | 'X' if hasAnyFlag("+ (") => "+ (".filter(hasFlag).foreach(bad => badFlag(bad, s"only use '$bad' for BigInt conversions to o, x, X")); true - } + case BooleanXn if arg != defn.BooleanType => + warningAt(CC): + """non-Boolean value formats as "true" for non-null references and boxed primitives, otherwise "false"""" + true + case IntegralXn if arg != BigIntType => + cc match + case 'o' | 'x' | 'X' if hasAnyFlag("+ (") => + "+ (".filter(hasFlag).foreach: bad => + badFlag(bad, s"only use '$bad' for BigInt conversions to o, x, X") + false case _ => true + case _ => true def lintToString(arg: Type): Unit = if ctx.settings.Whas.toStringInterpolated && kind == StringXn && !(arg.widen =:= defn.StringType) && !arg.isPrimitiveValueType diff --git a/tests/neg/f-interpolator-neg.check b/tests/neg/f-interpolator-neg.check index 1a931351d09c..eb5bcab230a8 100644 --- a/tests/neg/f-interpolator-neg.check +++ b/tests/neg/f-interpolator-neg.check @@ -17,7 +17,7 @@ -- Warning: tests/neg/f-interpolator-neg.scala:11:9 -------------------------------------------------------------------- 11 | f"$s%b" // warn only | ^ - | Boolean format is null test for non-Boolean + | non-Boolean value formats as "true" for non-null references and boxed primitives, otherwise "false" -- Error: tests/neg/f-interpolator-neg.scala:12:7 ---------------------------------------------------------------------- 12 | f"$s%c" // error | ^ From 71142d090b98fd8eb2817a9b652ffac06a9b7b64 Mon Sep 17 00:00:00 2001 From: Som Snytt Date: Mon, 7 Apr 2025 09:50:00 -0700 Subject: [PATCH 7/8] Improve name to stringXn, minor refactor for reading --- .../transform/localopt/FormatChecker.scala | 124 +++++++++--------- 1 file changed, 65 insertions(+), 59 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/transform/localopt/FormatChecker.scala b/compiler/src/dotty/tools/dotc/transform/localopt/FormatChecker.scala index 8a745732ae65..afb2e3d37f64 100644 --- a/compiler/src/dotty/tools/dotc/transform/localopt/FormatChecker.scala +++ b/compiler/src/dotty/tools/dotc/transform/localopt/FormatChecker.scala @@ -68,48 +68,51 @@ class TypedFormatChecker(partsElems: List[Tree], parts: List[String], args: List val amended = ListBuffer.empty[String] val convert = ListBuffer.empty[Conversion] + def checkPart(part: String, n: Int): Unit = + val matches = formatPattern.findAllMatchIn(part) + + def insertStringConversion(): Unit = + amended += "%s" + part + val cv = Conversion.stringXn(n) + cv.accepts(argType(n-1, defn.AnyType)) + convert += cv + cv.lintToString(argTypes(n-1)) + + def errorLeading(op: Conversion) = op.errorAt(Spec): + s"conversions must follow a splice; ${Conversion.literalHelp}" + + def accept(op: Conversion): Unit = + if !op.isLeading then errorLeading(op) + op.accepts(argType(n-1, op.acceptableVariants*)) + amended += part + convert += op + op.lintToString(argTypes(n-1)) + + // after the first part, a leading specifier is required for the interpolated arg; %s is supplied if needed + if n == 0 then amended += part + else if !matches.hasNext then insertStringConversion() + else + val cv = Conversion(matches.next(), n) + if cv.isLiteral then insertStringConversion() + else if cv.isIndexed then + if cv.index.getOrElse(-1) == n then accept(cv) else insertStringConversion() + else if !cv.isError then accept(cv) + + // any remaining conversions in this part must be either literals or indexed + while matches.hasNext do + val cv = Conversion(matches.next(), n) + if n == 0 && cv.hasFlag('<') then cv.badFlag('<', "No last arg") + else if !cv.isLiteral && !cv.isIndexed then errorLeading(cv) + end checkPart + @tailrec - def loop(remaining: List[String], n: Int): Unit = - remaining match - case part0 :: remaining => - def badPart(t: Throwable): String = "".tap(_ => report.partError(t.getMessage.nn, index = n, offset = 0)) - val part = try StringContext.processEscapes(part0) catch badPart - val matches = formatPattern.findAllMatchIn(part) - - def insertStringConversion(): Unit = - amended += "%s" + part - val cv = Conversion(n) - cv.accepts(argType(n-1, defn.AnyType)) - convert += cv - cv.lintToString(argTypes(n-1)) - - def errorLeading(op: Conversion) = op.errorAt(Spec)(s"conversions must follow a splice; ${Conversion.literalHelp}") - def accept(op: Conversion): Unit = - if !op.isLeading then errorLeading(op) - op.accepts(argType(n-1, op.acceptableVariants*)) - amended += part - convert += op - op.lintToString(argTypes(n-1)) - - // after the first part, a leading specifier is required for the interpolated arg; %s is supplied if needed - if n == 0 then amended += part - else if !matches.hasNext then insertStringConversion() - else - val cv = Conversion(matches.next(), n) - if cv.isLiteral then insertStringConversion() - else if cv.isIndexed then - if cv.index.getOrElse(-1) == n then accept(cv) else insertStringConversion() - else if !cv.isError then accept(cv) - - // any remaining conversions in this part must be either literals or indexed - while matches.hasNext do - val cv = Conversion(matches.next(), n) - if n == 0 && cv.hasFlag('<') then cv.badFlag('<', "No last arg") - else if !cv.isLiteral && !cv.isIndexed then errorLeading(cv) - - loop(remaining, n + 1) - case Nil => - end loop + def loop(remaining: List[String], n: Int): Unit = remaining match + case part0 :: remaining => + def badPart(t: Throwable): String = "".tap(_ => report.partError(t.getMessage.nn, index = n, offset = 0)) + val part = try StringContext.processEscapes(part0) catch badPart + checkPart(part, n) + loop(remaining, n + 1) + case Nil => loop(parts, n = 0) if reported then (Nil, Nil) @@ -260,27 +263,30 @@ class TypedFormatChecker(partsElems: List[Tree], parts: List[String], args: List object Conversion: def apply(m: Match, i: Int): Conversion = - def kindOf(cc: Char) = cc match - case 's' | 'S' => StringXn - case 'h' | 'H' => HashXn - case 'b' | 'B' => BooleanXn - case 'c' | 'C' => CharacterXn - case 'd' | 'o' | - 'x' | 'X' => IntegralXn - case 'e' | 'E' | - 'f' | - 'g' | 'G' | - 'a' | 'A' => FloatingPointXn - case 't' | 'T' => DateTimeXn - case '%' | 'n' => LiteralXn - case _ => ErrorXn - end kindOf m.group(CC) match - case Some(cc) => new Conversion(m, i, kindOf(cc(0))).tap(_.verify) - case None => new Conversion(m, i, ErrorXn).tap(_.errorAt(Spec)(s"Missing conversion operator in '${m.matched}'; $literalHelp")) + case Some(cc) => + val xn = cc(0) match + case 's' | 'S' => StringXn + case 'h' | 'H' => HashXn + case 'b' | 'B' => BooleanXn + case 'c' | 'C' => CharacterXn + case 'd' | 'o' | + 'x' | 'X' => IntegralXn + case 'e' | 'E' | + 'f' | + 'g' | 'G' | + 'a' | 'A' => FloatingPointXn + case 't' | 'T' => DateTimeXn + case '%' | 'n' => LiteralXn + case _ => ErrorXn + new Conversion(m, i, xn) + .tap(_.verify) + case None => + new Conversion(m, i, ErrorXn) + .tap(_.errorAt(Spec)(s"Missing conversion operator in '${m.matched}'; $literalHelp")) end apply // construct a default %s conversion - def apply(i: Int): Conversion = new Conversion(formatPattern.findAllMatchIn("%").next(), i, StringXn) + def stringXn(i: Int): Conversion = new Conversion(formatPattern.findAllMatchIn("%").next(), i, StringXn) val literalHelp = "use %% for literal %, %n for newline" end Conversion From ba4204ebe105a6ffef74b9de3f03ddb7d71e7bcf Mon Sep 17 00:00:00 2001 From: Som Snytt Date: Mon, 7 Apr 2025 10:45:12 -0700 Subject: [PATCH 8/8] Format interpolator messages get an ID --- .../tools/dotc/reporting/ErrorMessageID.scala | 1 + .../tools/dotc/reporting/MessageKind.scala | 1 + .../dotty/tools/dotc/reporting/messages.scala | 5 + .../transform/localopt/FormatChecker.scala | 15 ++- tests/neg/f-interpolator-neg.check | 92 +++++++++---------- tests/neg/f-interpolator-tests.check | 4 +- tests/neg/fEscapes.check | 2 +- 7 files changed, 67 insertions(+), 53 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/reporting/ErrorMessageID.scala b/compiler/src/dotty/tools/dotc/reporting/ErrorMessageID.scala index 8f8f4676f43b..150df4646d86 100644 --- a/compiler/src/dotty/tools/dotc/reporting/ErrorMessageID.scala +++ b/compiler/src/dotty/tools/dotc/reporting/ErrorMessageID.scala @@ -222,6 +222,7 @@ enum ErrorMessageID(val isActive: Boolean = true) extends java.lang.Enum[ErrorMe case EnumMayNotBeValueClassesID // errorNumber: 206 case IllegalUnrollPlacementID // errorNumber: 207 case ExtensionHasDefaultID // errorNumber: 208 + case FormatInterpolationErrorID // errorNumber: 209 def errorNumber = ordinal - 1 diff --git a/compiler/src/dotty/tools/dotc/reporting/MessageKind.scala b/compiler/src/dotty/tools/dotc/reporting/MessageKind.scala index bb02a08d2e46..e09dd1d6e69e 100644 --- a/compiler/src/dotty/tools/dotc/reporting/MessageKind.scala +++ b/compiler/src/dotty/tools/dotc/reporting/MessageKind.scala @@ -23,6 +23,7 @@ enum MessageKind: case PotentialIssue case UnusedSymbol case Staging + case Interpolation /** Human readable message that will end up being shown to the user. * NOTE: This is only used in the situation where you have multiple words diff --git a/compiler/src/dotty/tools/dotc/reporting/messages.scala b/compiler/src/dotty/tools/dotc/reporting/messages.scala index dcd7ed10987b..7ed65cedbb8b 100644 --- a/compiler/src/dotty/tools/dotc/reporting/messages.scala +++ b/compiler/src/dotty/tools/dotc/reporting/messages.scala @@ -3444,3 +3444,8 @@ extends DeclarationMsg(IllegalUnrollPlacementID): def explain(using Context) = "" end IllegalUnrollPlacement + +class BadFormatInterpolation(errorText: String)(using Context) extends Message(FormatInterpolationErrorID): + def kind = MessageKind.Interpolation + def msg(using Context) = errorText + def explain(using Context) = "" diff --git a/compiler/src/dotty/tools/dotc/transform/localopt/FormatChecker.scala b/compiler/src/dotty/tools/dotc/transform/localopt/FormatChecker.scala index afb2e3d37f64..b0ce1d6614fd 100644 --- a/compiler/src/dotty/tools/dotc/transform/localopt/FormatChecker.scala +++ b/compiler/src/dotty/tools/dotc/transform/localopt/FormatChecker.scala @@ -10,6 +10,7 @@ import dotty.tools.dotc.core.Contexts.* import dotty.tools.dotc.core.Symbols.* import dotty.tools.dotc.core.Types.* import dotty.tools.dotc.core.Phases.typerPhase +import dotty.tools.dotc.reporting.BadFormatInterpolation import dotty.tools.dotc.util.Spans.Span import dotty.tools.dotc.util.chaining.* @@ -296,10 +297,16 @@ class TypedFormatChecker(partsElems: List[Tree], parts: List[String], args: List val pos = partsElems(index).sourcePos val bgn = pos.span.start + offset val fin = if end < 0 then pos.span.end else pos.span.start + end - pos.withSpan(Span(bgn, fin, bgn)) + pos.withSpan(Span(start = bgn, end = fin, point = bgn)) extension (r: report.type) - def argError(message: String, index: Int): Unit = r.error(message, args(index).srcPos).tap(_ => reported = true) - def partError(message: String, index: Int, offset: Int, end: Int = -1): Unit = r.error(message, partPosAt(index, offset, end)).tap(_ => reported = true) - def partWarning(message: String, index: Int, offset: Int, end: Int = -1): Unit = r.warning(message, partPosAt(index, offset, end)).tap(_ => reported = true) + def argError(message: String, index: Int): Unit = + r.error(BadFormatInterpolation(message), args(index).srcPos) + .tap(_ => reported = true) + def partError(message: String, index: Int, offset: Int, end: Int = -1): Unit = + r.error(BadFormatInterpolation(message), partPosAt(index, offset, end)) + .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/neg/f-interpolator-neg.check b/tests/neg/f-interpolator-neg.check index eb5bcab230a8..fe1f41ab3871 100644 --- a/tests/neg/f-interpolator-neg.check +++ b/tests/neg/f-interpolator-neg.check @@ -14,187 +14,187 @@ 7 | new StringContext("", "").f() // error | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ | too few arguments for interpolated string --- Warning: tests/neg/f-interpolator-neg.scala:11:9 -------------------------------------------------------------------- +-- [E209] Interpolation Warning: tests/neg/f-interpolator-neg.scala:11:9 ----------------------------------------------- 11 | f"$s%b" // warn only | ^ | non-Boolean value formats as "true" for non-null references and boxed primitives, otherwise "false" --- Error: tests/neg/f-interpolator-neg.scala:12:7 ---------------------------------------------------------------------- +-- [E209] Interpolation Error: tests/neg/f-interpolator-neg.scala:12:7 ------------------------------------------------- 12 | f"$s%c" // error | ^ | Found: (s : String), Required: Char, Byte, Short, Int --- Error: tests/neg/f-interpolator-neg.scala:13:7 ---------------------------------------------------------------------- +-- [E209] Interpolation Error: tests/neg/f-interpolator-neg.scala:13:7 ------------------------------------------------- 13 | f"$f%c" // error | ^ | Found: (f : Double), Required: Char, Byte, Short, Int --- Error: tests/neg/f-interpolator-neg.scala:14:7 ---------------------------------------------------------------------- +-- [E209] Interpolation Error: tests/neg/f-interpolator-neg.scala:14:7 ------------------------------------------------- 14 | f"$s%x" // error | ^ | Found: (s : String), Required: Int, Long, Byte, Short, BigInt --- Error: tests/neg/f-interpolator-neg.scala:15:7 ---------------------------------------------------------------------- +-- [E209] Interpolation Error: tests/neg/f-interpolator-neg.scala:15:7 ------------------------------------------------- 15 | f"$b%d" // error | ^ | Found: (b : Boolean), Required: Int, Long, Byte, Short, BigInt --- Error: tests/neg/f-interpolator-neg.scala:16:7 ---------------------------------------------------------------------- +-- [E209] Interpolation Error: tests/neg/f-interpolator-neg.scala:16:7 ------------------------------------------------- 16 | f"$s%d" // error | ^ | Found: (s : String), Required: Int, Long, Byte, Short, BigInt --- Error: tests/neg/f-interpolator-neg.scala:17:7 ---------------------------------------------------------------------- +-- [E209] Interpolation Error: tests/neg/f-interpolator-neg.scala:17:7 ------------------------------------------------- 17 | f"$f%o" // error | ^ | Found: (f : Double), Required: Int, Long, Byte, Short, BigInt --- Error: tests/neg/f-interpolator-neg.scala:18:7 ---------------------------------------------------------------------- +-- [E209] Interpolation Error: tests/neg/f-interpolator-neg.scala:18:7 ------------------------------------------------- 18 | f"$s%e" // error | ^ | Found: (s : String), Required: Double, Float, BigDecimal --- Error: tests/neg/f-interpolator-neg.scala:19:7 ---------------------------------------------------------------------- +-- [E209] Interpolation Error: tests/neg/f-interpolator-neg.scala:19:7 ------------------------------------------------- 19 | f"$b%f" // error | ^ | Found: (b : Boolean), Required: Double, Float, BigDecimal --- Error: tests/neg/f-interpolator-neg.scala:20:9 ---------------------------------------------------------------------- +-- [E209] Interpolation Error: tests/neg/f-interpolator-neg.scala:20:9 ------------------------------------------------- 20 | f"$s%i" // error | ^ | illegal conversion character 'i' --- Error: tests/neg/f-interpolator-neg.scala:24:9 ---------------------------------------------------------------------- +-- [E209] Interpolation Error: tests/neg/f-interpolator-neg.scala:24:9 ------------------------------------------------- 24 | f"$s%+ 0,(s" // error | ^^^^^ | Illegal flag '+' --- Error: tests/neg/f-interpolator-neg.scala:25:9 ---------------------------------------------------------------------- +-- [E209] Interpolation Error: tests/neg/f-interpolator-neg.scala:25:9 ------------------------------------------------- 25 | f"$c%#+ 0,(c" // error | ^^^^^^ | Only '-' allowed for c conversion --- Error: tests/neg/f-interpolator-neg.scala:26:9 ---------------------------------------------------------------------- +-- [E209] Interpolation Error: tests/neg/f-interpolator-neg.scala:26:9 ------------------------------------------------- 26 | f"$d%#d" // error | ^ | # not allowed for d conversion --- Error: tests/neg/f-interpolator-neg.scala:27:9 ---------------------------------------------------------------------- +-- [E209] Interpolation Error: tests/neg/f-interpolator-neg.scala:27:9 ------------------------------------------------- 27 | f"$d%,x" // error | ^ | ',' only allowed for d conversion of integral types --- Error: tests/neg/f-interpolator-neg.scala:28:9 ---------------------------------------------------------------------- +-- [E209] Interpolation Error: tests/neg/f-interpolator-neg.scala:28:9 ------------------------------------------------- 28 | f"$d%+ (x" // error | ^^^ | only use '+' for BigInt conversions to o, x, X --- Error: tests/neg/f-interpolator-neg.scala:29:9 ---------------------------------------------------------------------- +-- [E209] Interpolation Error: tests/neg/f-interpolator-neg.scala:29:9 ------------------------------------------------- 29 | f"$f%,(a" // error | ^^ | ',' not allowed for a, A --- Error: tests/neg/f-interpolator-neg.scala:30:9 ---------------------------------------------------------------------- +-- [E209] Interpolation Error: tests/neg/f-interpolator-neg.scala:30:9 ------------------------------------------------- 30 | f"$t%#+ 0,(tT" // error | ^^^^^^ | Only '-' allowed for date/time conversions --- Error: tests/neg/f-interpolator-neg.scala:31:7 ---------------------------------------------------------------------- +-- [E209] Interpolation Error: tests/neg/f-interpolator-neg.scala:31:7 ------------------------------------------------- 31 | f"%-#+ 0,(n" // error | ^^^^^^^ | flags not allowed --- Error: tests/neg/f-interpolator-neg.scala:32:7 ---------------------------------------------------------------------- +-- [E209] Interpolation Error: tests/neg/f-interpolator-neg.scala:32:7 ------------------------------------------------- 32 | f"%#+ 0,(%" // error | ^^^^^^ | Illegal flag '#' --- Error: tests/neg/f-interpolator-neg.scala:36:9 ---------------------------------------------------------------------- +-- [E209] Interpolation Error: tests/neg/f-interpolator-neg.scala:36:9 ------------------------------------------------- 36 | f"$c%.2c" // error | ^^ | precision not allowed --- Error: tests/neg/f-interpolator-neg.scala:37:9 ---------------------------------------------------------------------- +-- [E209] Interpolation Error: tests/neg/f-interpolator-neg.scala:37:9 ------------------------------------------------- 37 | f"$d%.2d" // error | ^^ | precision not allowed --- Error: tests/neg/f-interpolator-neg.scala:38:7 ---------------------------------------------------------------------- +-- [E209] Interpolation Error: tests/neg/f-interpolator-neg.scala:38:7 ------------------------------------------------- 38 | f"%.2%" // error | ^^ | precision not allowed --- Error: tests/neg/f-interpolator-neg.scala:39:7 ---------------------------------------------------------------------- +-- [E209] Interpolation Error: tests/neg/f-interpolator-neg.scala:39:7 ------------------------------------------------- 39 | f"%.2n" // error | ^^ | precision not allowed --- Error: tests/neg/f-interpolator-neg.scala:40:9 ---------------------------------------------------------------------- +-- [E209] Interpolation Error: tests/neg/f-interpolator-neg.scala:40:9 ------------------------------------------------- 40 | f"$f%.2a" // error | ^^ | precision not allowed --- Error: tests/neg/f-interpolator-neg.scala:41:9 ---------------------------------------------------------------------- +-- [E209] Interpolation Error: tests/neg/f-interpolator-neg.scala:41:9 ------------------------------------------------- 41 | f"$t%.2tT" // error | ^^ | precision not allowed --- Error: tests/neg/f-interpolator-neg.scala:45:7 ---------------------------------------------------------------------- +-- [E209] Interpolation Error: tests/neg/f-interpolator-neg.scala:45:7 ------------------------------------------------- 45 | f"%