From 474552988e43ec1143493cc925106568b7404b0f Mon Sep 17 00:00:00 2001 From: Som Snytt Date: Fri, 13 Jun 2025 11:36:32 -0700 Subject: [PATCH] Check OrType in interpolated toString lint --- .../transform/localopt/FormatChecker.scala | 18 ++++++- .../localopt/StringInterpolatorOpt.scala | 22 ++++++-- tests/warn/tostring-interpolated.scala | 53 +++++++++++++++++-- 3 files changed, 82 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 42e5f0acb3f6..b628108d3a1d 100644 --- a/compiler/src/dotty/tools/dotc/transform/localopt/FormatChecker.scala +++ b/compiler/src/dotty/tools/dotc/transform/localopt/FormatChecker.scala @@ -228,8 +228,22 @@ class TypedFormatChecker(partsElems: List[Tree], parts: List[String], args: List case _ => true def lintToString(arg: Type): Unit = - if ctx.settings.Whas.toStringInterpolated && kind == StringXn && !(arg.widen =:= defn.StringType) && !arg.isPrimitiveValueType - then warningAt(CC)("interpolation uses toString") + def check(tp: Type): Boolean = tp.widen match + case OrType(tp1, tp2) => + check(tp1) || check(tp2) + case tp => + if tp =:= defn.StringType then + false + else if tp =:= defn.UnitType then + warningAt(CC)("interpolated Unit value") + true + else if !tp.isPrimitiveValueType then + warningAt(CC)("interpolation uses toString") + true + else + false + if ctx.settings.Whas.toStringInterpolated && kind == StringXn && check(arg) then + () // what arg type if any does the conversion accept def acceptableVariants: List[Type] = diff --git a/compiler/src/dotty/tools/dotc/transform/localopt/StringInterpolatorOpt.scala b/compiler/src/dotty/tools/dotc/transform/localopt/StringInterpolatorOpt.scala index 1afcfbac6206..37ae391325db 100644 --- a/compiler/src/dotty/tools/dotc/transform/localopt/StringInterpolatorOpt.scala +++ b/compiler/src/dotty/tools/dotc/transform/localopt/StringInterpolatorOpt.scala @@ -105,13 +105,27 @@ class StringInterpolatorOpt extends MiniPhase: lintToString(elem) concat(elem) val str = stri.next() - if !str.const.stringValue.isEmpty then concat(str) + if !str.const.stringValue.isEmpty then + concat(str) result end mkConcat def lintToString(t: Tree): Unit = - val arg: Type = t.tpe - if ctx.settings.Whas.toStringInterpolated && !(arg.widen =:= defn.StringType) && !arg.isPrimitiveValueType - then report.warning("interpolation uses toString", t.srcPos) + def check(tp: Type): Boolean = tp.widen match + case OrType(tp1, tp2) => + check(tp1) || check(tp2) + case tp => + if tp =:= defn.StringType then + false + else if tp =:= defn.UnitType then + report.warning("interpolated Unit value", t.srcPos) + true + else if !tp.isPrimitiveValueType then + report.warning("interpolation uses toString", t.srcPos) + true + else + false + if ctx.settings.Whas.toStringInterpolated && check(t.tpe) then + () val sym = tree.symbol // Test names first to avoid loading scala.StringContext if not used, and common names first val isInterpolatedMethod = diff --git a/tests/warn/tostring-interpolated.scala b/tests/warn/tostring-interpolated.scala index 165bc374b5ef..f81dab3a518b 100644 --- a/tests/warn/tostring-interpolated.scala +++ b/tests/warn/tostring-interpolated.scala @@ -11,14 +11,18 @@ trait T { 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 + + def literally = s"Hello, ${"world"}" // nowarn literal, widened to String + + def bool = f"$c%b" // warn just a null check (quirk of Java format) + + def oops = s"${null} slipped thru my fingers" // warn although conforms to String + + def exceptionally = s"Hello, ${???}" // warn although conforms to String } class Mitigations { @@ -29,8 +33,47 @@ class Mitigations { def ok = s"$s is ok" def jersey = s"number $i" - def unitized = s"unfortunately $shown" // maybe tell them about unintended ()? + def unitized = s"unfortunately $shown" // warn accidental unit value + def funitized = f"unfortunately $shown" // warn accidental unit value def nopct = f"$s is ok" def nofmt = f"number $i" } + +class Branches { + + class C { + val shouldCaps = true + val greeting = s"Hello ${if (shouldCaps) "WORLD" else "world"}" + } + + class D { + val shouldCaps = true + object world { override def toString = "world" } + val greeting = s"Hello ${if (shouldCaps) "WORLD" else world}" // warn + } + + class E { + def x = 42 + val greeting = s"Hello ${x match { case 42 => "WORLD" case 27 => "world" case _ => ??? }}" + } + + class F { + def x = 42 + object world { override def toString = "world" } + val greeting = s"Hello ${ + x match { // warn + case 17 => "Welt" + case 42 => "WORLD" + case 27 => world + case _ => ??? } + }" + } + + class Z { + val shouldCaps = true + val greeting = s"Hello ${if (shouldCaps) ??? else null}" // warn + val farewell = s"Bye-bye ${if (shouldCaps) "Bob" else null}" // warn + } + +}