Skip to content

Commit a32508f

Browse files
authored
Merge pull request scala#11104 from som-snytt/issue/13116-missing-interpolator-macro
Nowarn missing interpolator if string has no position
2 parents 7d9fef7 + 11572b8 commit a32508f

File tree

7 files changed

+362
-20
lines changed

7 files changed

+362
-20
lines changed

src/compiler/scala/tools/nsc/typechecker/Typers.scala

Lines changed: 27 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -6093,29 +6093,34 @@ trait Typers extends Adaptations with Tags with TypersTracking with PatternTyper
60936093

60946094
// Warn about likely interpolated strings which are missing their interpolators
60956095
def warnMissingInterpolator(lit: Literal): Unit = if (!isPastTyper) {
6096-
// attempt to avoid warning about trees munged by macros
6097-
def isMacroExpansion = {
6098-
// context.tree is not the expandee; it is plain new SC(ps).m(args)
6099-
//context.tree.exists(t => t.pos.includes(lit.pos) && hasMacroExpansionAttachment(t))
6100-
// testing pos works and may suffice
6101-
//openMacros.exists(_.macroApplication.pos.includes(lit.pos))
6102-
// tests whether the lit belongs to the expandee of an open macro
6103-
openMacros.exists(_.macroApplication.attachments.get[MacroExpansionAttachment] match {
6104-
case Some(MacroExpansionAttachment(_, t: Tree)) => t.exists(_ eq lit)
6105-
case _ => false
6106-
})
6107-
}
6108-
val checkMacroExpansion = settings.warnMacros.value match {
6109-
case "both" | "after" => true
6110-
case _ => !isMacroExpansion
6111-
}
6112-
// An interpolation desugars to `StringContext(parts).m(args)`, so obviously not missing.
6096+
// Attempt to avoid warning about trees munged by macros, according to `-Wmacros`.
6097+
// By default, if it looks like a macro expansion, do not warn.
6098+
// A macro expansion is detected if the literal tree has no position
6099+
// (such as when a macro `c.typecheck(qq)` explicitly), or if there is an open macro
6100+
// whose expansion (expandee) contains the literal.
6101+
// Note context.tree is not the expandee but `new StringContext(parts).s(args)`.
6102+
def isMacroExpansion =
6103+
!lit.pos.isDefined ||
6104+
openMacros.exists { ctx =>
6105+
ctx.macroApplication.attachments.get[MacroExpansionAttachment] match {
6106+
case Some(MacroExpansionAttachment(_, t: Tree)) =>
6107+
ctx.macroApplication.pos.includes(lit.pos) && t.exists(_ eq lit)
6108+
case _ => false
6109+
}
6110+
}
6111+
// An interpolation desugars to `StringContext(parts).m(args)`, so obviously not missing in that case.
61136112
// `implicitNotFound` annotations have strings with `${A}`, so never warn for that.
61146113
// Also don't warn for macro expansion unless they ask for it.
61156114
def mightBeMissingInterpolation: Boolean = context.enclosingApply.tree match {
61166115
case Apply(Select(Apply(RefTree(_, nme.StringContextName), _), _), _) => false
61176116
case Apply(Select(New(RefTree(_, tpnme.implicitNotFound)), _), _) => false
6118-
case _ => checkMacroExpansion
6117+
case _ =>
6118+
settings.warnMacros.value match {
6119+
case "default" | "before" => !isMacroExpansion
6120+
case "both" => true
6121+
case "after" => isMacroExpansion
6122+
case _ => false
6123+
}
61196124
}
61206125
def maybeWarn(s: String): Unit = {
61216126
def warn(message: String) = context.warning(lit.pos, s"possible missing interpolator: $message", WarningCategory.LintMissingInterpolator)
@@ -6152,8 +6157,10 @@ trait Typers extends Adaptations with Tags with TypersTracking with PatternTyper
61526157
}
61536158
}
61546159
lit match {
6155-
case Literal(Constant(s: String)) if mightBeMissingInterpolation => maybeWarn(s)
6156-
case _ =>
6160+
case Literal(Constant(s: String)) if !s.isEmpty =>
6161+
if (mightBeMissingInterpolation)
6162+
maybeWarn(s)
6163+
case _ =>
61576164
}
61586165
}
61596166

test/files/run/t13116.check

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
Expectations(assertion failed
2+
3+
expect(s"$x" == "42")
4+
5+
Use the `clue` function to troubleshoot)
Lines changed: 253 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,253 @@
1+
//package weaver
2+
3+
import scala.annotation._
4+
import scala.collection.mutable.ListBuffer
5+
import scala.language.experimental.macros
6+
import scala.reflect.macros.blackbox, blackbox.Context
7+
8+
case class SourceLocation(loc: String)
9+
10+
case class Expectations(assertion: String)
11+
12+
trait ExpectMacro {
13+
14+
def apply(value: Boolean)(implicit loc: SourceLocation): Expectations =
15+
macro ExpectMacro.applyImpl
16+
}
17+
18+
object ExpectMacro {
19+
20+
/**
21+
* Constructs [[Expectations]] from a boolean value.
22+
*
23+
* A macro is needed to support clues. The value expression may contain calls
24+
* to [[ClueHelpers.clue]], which generate clues for values under test.
25+
*
26+
* This macro constructs a local collection of [[Clues]] and adds the
27+
* generated clues to it. Calls to [[ClueHelpers.clue]] are rewritten to calls
28+
* to [[Clues.addClue]].
29+
*
30+
* After the value is evaluated, the [[Clues]] collection is used to contruct
31+
* [[Expectations]].
32+
*/
33+
def applyImpl(c: blackbox.Context)(value: c.Tree)(loc: c.Tree): c.Tree = {
34+
35+
import c.universe._
36+
val sourcePos = c.enclosingPosition
37+
val sourceCode =
38+
new String(sourcePos.source.content.slice(sourcePos.start, sourcePos.end))
39+
40+
val (cluesName, cluesValDef) = makeClues(c)
41+
val clueMethodSymbol = getClueMethodSymbol(c)
42+
43+
val transformedValue =
44+
replaceClueMethodCalls(c)(clueMethodSymbol, cluesName, value)
45+
makeExpectations(c)(cluesName = cluesName,
46+
cluesValDef = cluesValDef,
47+
value = transformedValue,
48+
loc = loc,
49+
sourceCode = sourceCode,
50+
message = q"None")
51+
}
52+
53+
/** Constructs [[Expectations]] from the local [[Clues]] collection. */
54+
private def makeExpectations(c: blackbox.Context)(
55+
cluesName: c.TermName,
56+
cluesValDef: c.Tree,
57+
value: c.Tree,
58+
loc: c.Tree,
59+
sourceCode: String,
60+
message: c.Tree): c.Tree = {
61+
import c.universe._
62+
//val sanitizedSourceCode = SourceCode.sanitize(c)(sourceCode)
63+
val block =
64+
q"$cluesValDef; Clues.toExpectations($loc, Some($sourceCode), $message, $cluesName, $value)"
65+
//q"$cluesValDef; _root_.weaver.internals.Clues.toExpectations($loc, Some($sanitizedSourceCode), $message, $cluesName, $value)"
66+
val untyped = c.untypecheck(block)
67+
val retyped = c.typecheck(untyped, pt = c.typeOf[Expectations])
68+
retyped
69+
}
70+
71+
/** Get the [[ClueHelpers.clue]] symbol. */
72+
private def getClueMethodSymbol(c: blackbox.Context): c.Symbol = {
73+
import c.universe._
74+
symbolOf[ClueHelpers].info.member(TermName("clue"))
75+
}
76+
77+
/** Construct a [[Clues]] collection local to the `expect` call. */
78+
private def makeClues(c: blackbox.Context): (c.TermName, c.Tree) = {
79+
import c.universe._
80+
val cluesName = TermName(c.freshName("clues$"))
81+
val cluesValDef =
82+
q"val $cluesName: Clues = new Clues()"
83+
(cluesName, cluesValDef)
84+
}
85+
86+
/**
87+
* Replaces all calls to [[ClueHelpers.clue]] with calls to [[Clues.addClue]].
88+
*/
89+
private def replaceClueMethodCalls(c: blackbox.Context)(
90+
clueMethodSymbol: c.Symbol,
91+
cluesName: c.TermName,
92+
value: c.Tree): c.Tree = {
93+
94+
import c.universe._
95+
96+
// This transformation outputs code that adds clues to a local
97+
// clues collection `cluesName`. It recurses over the input code and replaces
98+
// all calls of `ClueHelpers.clue` with `cluesName.addClue`.
99+
object transformer extends Transformer {
100+
101+
override def transform(input: Tree): Tree = input match {
102+
case c.universe.Apply(fun, List(clueValue))
103+
if fun.symbol == clueMethodSymbol =>
104+
// The input tree corresponds to `ClueHelpers.clue(clueValue)` .
105+
// Transform it into `clueName.addClue(clueValue)`
106+
// Apply the transformation recursively to `clueValue` to support nested clues.
107+
val transformedClueValue = super.transform(clueValue)
108+
q"""${cluesName}.addClue($transformedClueValue)"""
109+
case o =>
110+
// Otherwise, recurse over the input.
111+
super.transform(o)
112+
}
113+
}
114+
115+
transformer.transform(value)
116+
}
117+
}
118+
119+
trait Show[T] {
120+
def show(t: T): String
121+
}
122+
object Show {
123+
implicit val showString: Show[String] = new Show[String] { def show(s: String) = s }
124+
implicit val showAny: Show[Any] = new Show[Any] { def show(x: Any) = x.toString }
125+
}
126+
127+
trait ClueHelpers {
128+
129+
// This function is removed as part of the `expect` macro expansion.
130+
@compileTimeOnly("This function can only be used within `expect`.")
131+
final def clue[A](@unused a: Clue[A]): A = ???
132+
}
133+
134+
class Clue[T](
135+
source: String,
136+
val value: T,
137+
valueType: String,
138+
show: Show[T]
139+
) {
140+
def prettyPrint: String =
141+
s"${source}: ${valueType} = ${show.show(value)}"
142+
}
143+
object Clue extends LowPriorityClueImplicits {
144+
145+
/**
146+
* Generates a clue for a given value using a [[Show]] instance to print the
147+
* value.
148+
*/
149+
implicit def generateClue[A](value: A)(implicit catsShow: Show[A]): Clue[A] =
150+
macro ClueMacro.impl
151+
}
152+
trait LowPriorityClueImplicits {
153+
154+
/**
155+
* Generates a clue for a given value using the [[toString]] function to print
156+
* the value.
157+
*/
158+
implicit def generateClueFromToString[A](value: A): Clue[A] =
159+
macro ClueMacro.showFromToStringImpl
160+
}
161+
object ClueMacro {
162+
def showFromToStringImpl(c: Context)(value: c.Tree): c.Tree = {
163+
import c.universe._
164+
impl(c)(value)(q"Show.showAny")
165+
}
166+
167+
/**
168+
* Constructs a clue by extracting the source code and type information of a
169+
* value.
170+
*/
171+
def impl(c: Context)(value: c.Tree)(catsShow: c.Tree): c.Tree = {
172+
import c.universe._
173+
val text: String =
174+
if (value.pos != null && value.pos.isRange) {
175+
val chars = value.pos.source.content
176+
val start = value.pos.start
177+
val end = value.pos.end
178+
if (end > start &&
179+
start >= 0 && start < chars.length &&
180+
end >= 0 && end < chars.length) {
181+
new String(chars, start, end - start)
182+
} else {
183+
""
184+
}
185+
} else {
186+
""
187+
}
188+
def simplifyType(tpe: Type): Type = tpe match {
189+
case TypeRef(ThisType(pre), sym, args) if pre == sym.owner =>
190+
simplifyType(c.internal.typeRef(NoPrefix, sym, args))
191+
case t =>
192+
t.widen
193+
}
194+
val source = Literal(Constant(text.trim))
195+
val valueType = Literal(Constant(simplifyType(value.tpe).toString()))
196+
val clueTpe = c.internal.typeRef(
197+
NoPrefix,
198+
c.mirror.staticClass(classOf[Clue[_]].getName()),
199+
List(value.tpe.widen)
200+
)
201+
q"new $clueTpe(..$source, $value, $valueType, $catsShow)"
202+
}
203+
}
204+
205+
final class Clues {
206+
private val clues: ListBuffer[Clue[?]] = ListBuffer.empty
207+
208+
/**
209+
* Adds a clue to the collection.
210+
*
211+
* This function is called as part of the expansion of the `expect` macro. It
212+
* should not be called explicitly.
213+
*/
214+
def addClue[A](clue: Clue[A]): A = {
215+
clues.addOne(clue)
216+
clue.value
217+
}
218+
219+
def getClues: List[Clue[?]] = clues.toList
220+
}
221+
222+
object Clues {
223+
224+
/**
225+
* Constructs [[Expectations]] from the collection of clues.
226+
*
227+
* If the result is successful, the clues are discarded. If the result has
228+
* failed, the clues are printed as part of the failure message.
229+
*
230+
* This function is called as part of the expansion of the `expect` macro. It
231+
* should not be called explicitly.
232+
*/
233+
def toExpectations(
234+
sourceLoc: SourceLocation,
235+
sourceCode: Option[String],
236+
message: Option[String],
237+
clues: Clues,
238+
success: Boolean): Expectations = {
239+
if (success) {
240+
Expectations("success")
241+
} else {
242+
val header = "assertion failed" + message.fold("")(msg => s": $msg")
243+
val sourceCodeMessage = sourceCode.fold("")(msg => s"\n\n$msg")
244+
val clueList = clues.getClues
245+
val cluesMessage = if (clueList.nonEmpty) {
246+
val lines = clueList.map(clue => s" ${clue.prettyPrint}")
247+
lines.mkString("Clues {\n", "\n", "\n}")
248+
} else "Use the `clue` function to troubleshoot"
249+
val fullMessage = header + sourceCodeMessage + "\n\n" + cluesMessage
250+
Expectations(fullMessage)
251+
}
252+
}
253+
}

test/files/run/t13116/test_2.scala

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
//> using options -Werror -Xlint
2+
3+
object Test extends App {
4+
val expect: ExpectMacro = null
5+
implicit val loc: SourceLocation = SourceLocation("testloc")
6+
7+
val x = 27
8+
println {
9+
expect(s"$x" == "42")
10+
}
11+
}

test/files/run/t8013.check

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
Perverting ["Hello, $foo"]
2+
"Hello, $foo"
3+
Perverting [s"Hello, $foo")]
4+
s"Hello, $foo")
5+
Hello, $foo
Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
//> using options -Werror -Xlint
2+
3+
import annotation._
4+
5+
package t8013 {
6+
7+
// unsuspecting user of perverse macro
8+
trait User {
9+
import Perverse._
10+
val foo = "bar"
11+
Console.println {
12+
p"Hello, $foo"
13+
}
14+
Console.println {
15+
p(s"Hello, $foo")
16+
}
17+
Console.println {
18+
"Hello, $foo"
19+
}: @nowarn
20+
}
21+
}
22+
23+
object Test extends App {
24+
new t8013.User {}
25+
}

0 commit comments

Comments
 (0)