Skip to content

Check OrType in interpolated toString lint #23365

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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] =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 =
Expand Down
53 changes: 48 additions & 5 deletions tests/warn/tostring-interpolated.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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
}

}
Loading