Merge pull request #9305 from som-snytt/issue/warn-no-interp-this

Notice this when linting for missing interpolator

src/compiler/scala/tools/nsc/backend/jvm/BCodeBodyBuilder.scala

@@ -266,7 +266,7 @@ abstract class BCodeBodyBuilder extends BCodeSkelBuilder {
         case lblDf : LabelDef => genLabelDef(lblDf, expectedType)
 
         case ValDef(_, nme.THIS, _, _) =>
-          debuglog("skipping trivial assign to _$this: " + tree)
+          debuglog(s"skipping trivial assign to ${nme.THIS}: $tree")
 
         case ValDef(_, _, _, rhs) =>
           val sym = tree.symbol

src/compiler/scala/tools/nsc/backend/jvm/analysis/NullnessAnalyzer.scala

@@ -93,7 +93,7 @@ final class NullnessInterpreter(knownNonNullInvocation: MethodInsnNode => Boolea
     val isThis = local == 0 && (isInstanceMethod || {
       method.parameters != null && !method.parameters.isEmpty && {
         val p = method.parameters.get(0)
-        (p.access & Opcodes.ACC_SYNTHETIC) != 0 && p.name == "$this"
+        (p.access & Opcodes.ACC_SYNTHETIC) != 0 && p.name == s"$$this"
       }
     })
     if (isThis) NotNullValue

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

@@ -5774,41 +5774,45 @@ trait Typers extends Adaptations with Tags with TypersTracking with PatternTyper
             case _                                          => false
           })
         }
-        // attempt to avoid warning about the special interpolated message string
-        // for implicitNotFound or any standard interpolation (with embedded $$).
-        def isRecognizablyNotForInterpolation = context.enclosingApply.tree match {
-          case Apply(Select(Apply(RefTree(_, nme.StringContextName), _), _), _) => true
-          case Apply(Select(New(RefTree(_, tpnme.implicitNotFound)), _), _) => true
-          case _                                                            => isMacroExpansion
+        // An interpolation desugars to `StringContext(parts).m(args)`, so obviously not missing.
+        // `implicitNotFound` annotations have strings with `${A}`, so never warn for that.
+        // Also don't warn for macro expansion.
+        def mightBeMissingInterpolation: Boolean = context.enclosingApply.tree match {
+          case Apply(Select(Apply(RefTree(_, nme.StringContextName), _), _), _) => false
+          case Apply(Select(New(RefTree(_, tpnme.implicitNotFound)), _), _)     => false
+          case _                                                                => !isMacroExpansion
         }
-        @tailrec
-        def requiresNoArgs(tp: Type): Boolean = tp match {
-          case PolyType(_, restpe)     => requiresNoArgs(restpe)
-          case MethodType(Nil, restpe) => requiresNoArgs(restpe)  // may be a curried method - can't tell yet
-          case MethodType(p :: _, _)   => p.isImplicit            // implicit method requires no args
-          case _                       => true                    // catches all others including NullaryMethodType
-        }
-        def isPlausible(m: Symbol) = !m.hasPackageFlag && m.alternatives.exists(x => requiresNoArgs(x.info))
-
         def maybeWarn(s: String): Unit = {
-          def warn(message: String)         = context.warning(lit.pos, s"possible missing interpolator: $message", WarningCategory.LintMissingInterpolator)
-          def suspiciousSym(name: TermName) = context.lookupSymbol(name, _ => true).symbol
-          val suspiciousExprs               = InterpolatorCodeRegex findAllMatchIn s
-          def suspiciousIdents              = InterpolatorIdentRegex findAllIn s map (s => suspiciousSym(TermName(s drop 1)))
-          def isCheapIdent(expr: String)    = Character.isJavaIdentifierStart(expr.charAt(0)) && expr.tail.forall(Character.isJavaIdentifierPart)
-          def warnableExpr(expr: String)    = !expr.isEmpty && (!isCheapIdent(expr) || isPlausible(suspiciousSym(TermName(expr))))
+          def warn(message: String) = context.warning(lit.pos, s"possible missing interpolator: $message", WarningCategory.LintMissingInterpolator)
+          def isPlausible(id: String): Boolean = {
+            def requiresNoArgs(tp: Type): Boolean = tp match {
+              case PolyType(_, restpe)     => requiresNoArgs(restpe)
+              case MethodType(Nil, restpe) => requiresNoArgs(restpe)  // may be a curried method - can't tell yet
+              case MethodType(p :: _, _)   => p.isImplicit            // implicit method requires no args
+              case _                       => true                    // catches all others including NullaryMethodType
+            }
+            def isNullaryTerm: Boolean = {
+              val maybe = context.lookupSymbol(TermName(id), _ => true).symbol
+              maybe != NoSymbol && !maybe.hasPackageFlag && maybe.alternatives.exists(x => requiresNoArgs(x.info))
+            }
+            id == "this" || isNullaryTerm
+          }
+          val suspiciousExprs  = InterpolatorCodeRegex.findAllMatchIn(s)
+          def suspiciousIdents = InterpolatorIdentRegex.findAllIn(s)
 
           if (suspiciousExprs.hasNext) {
-            val exprs = (suspiciousExprs map (_ group 1)).toList
+            def isCheapIdent(expr: String) = Character.isJavaIdentifierStart(expr.charAt(0)) && expr.tail.forall(Character.isJavaIdentifierPart)
+            def warnableExpr(expr: String) = !expr.isEmpty && (!isCheapIdent(expr) || isPlausible(expr))
+            val exprs = suspiciousExprs.map(_ group 1).toList
             // short-circuit on leading ${}
             if (!exprs.head.isEmpty && exprs.exists(warnableExpr))
               warn("detected an interpolated expression") // "${...}"
           } else
-            suspiciousIdents find isPlausible foreach (sym => warn(s"detected interpolated identifier `$$${sym.name}`")) // "$id"
+            suspiciousIdents.find(id => isPlausible(id.substring(1))).foreach(id => warn(s"detected interpolated identifier `$id`"))
         }
         lit match {
-          case Literal(Constant(s: String)) if !isRecognizablyNotForInterpolation => maybeWarn(s)
-          case _                                                                  =>
+          case Literal(Constant(s: String)) if mightBeMissingInterpolation => maybeWarn(s)
+          case _                                                           =>
         }
       }
 

src/reflect/scala/reflect/internal/StdNames.scala

@@ -371,8 +371,8 @@ trait StdNames {
     val MIRROR_SHORT: NameType             = nameType("$m")
     val MIRROR_UNTYPED: NameType           = nameType("$m$untyped")
     val REIFY_FREE_PREFIX: NameType        = nameType("free$")
-    val REIFY_FREE_THIS_SUFFIX: NameType   = nameType("$this")
-    val REIFY_FREE_VALUE_SUFFIX: NameType  = nameType("$" + "value") // looks like missing interpolator due to `value` in scope
+    val REIFY_FREE_THIS_SUFFIX: NameType   = nameType(s"$$this")
+    val REIFY_FREE_VALUE_SUFFIX: NameType  = nameType(s"$$value") // looks like missing interpolator due to `value` in scope
     val REIFY_SYMDEF_PREFIX: NameType      = nameType("symdef$")
     val QUASIQUOTE_CASE: NameType          = nameType("$quasiquote$case$")
     val QUASIQUOTE_EARLY_DEF: NameType     = nameType("$quasiquote$early$def$")
@@ -394,11 +394,11 @@ trait StdNames {
     val OUTER_SYNTH: NameType              = nameType("<outer>") // emitted by virtual pattern matcher, replaced by outer accessor in explicitouter
     val ROOTPKG: NameType                  = nameType("_root_")
     val SELECTOR_DUMMY: NameType           = nameType("<unapply-selector>")
-    val SELF: NameType                     = nameType("$this")
+    val SELF: NameType                     = nameType(s"$$this")
     val SETTER_SUFFIX: NameType            = nameType(NameTransformer.SETTER_SUFFIX_STRING)
     val SPECIALIZED_INSTANCE: NameType     = nameType("specInstance$")
     val STAR: NameType                     = nameType("*")
-    val THIS: NameType                     = nameType("_$this")
+    val THIS: NameType                     = nameType(s"_$$this")
 
 
     val annottees: NameType               = nameType("annottees")       // for macro annotations
@@ -542,7 +542,7 @@ trait StdNames {
     }
 
     def localDummyName(clazz: Symbol): TermName = newTermName(LOCALDUMMY_PREFIX + clazz.name + ">")
-    def superName(name: Name, mix: Name = EMPTY): TermName = newTermName(SUPER_PREFIX_STRING + name + (if (mix.isEmpty) "" else "$" + mix))
+    def superName(name: Name, mix: Name = EMPTY): TermName = newTermName(s"${SUPER_PREFIX_STRING}${name}${if (mix.isEmpty) "" else s"$$$mix"}")
 
     /** The name of an accessor for protected symbols. */
     def protName(name: Name): TermName = newTermName(PROTECTED_PREFIX + name)
@@ -602,7 +602,7 @@ trait StdNames {
       case 7  => nme.x_7
       case 8  => nme.x_8
       case 9  => nme.x_9
-      case _  => newTermName("x$" + i)
+      case _  => newTermName(s"x$$$i")
     }
 
     def productAccessorName(j: Int): TermName = (j: @switch) match {
@@ -718,7 +718,7 @@ trait StdNames {
     val asModule: NameType             = nameType("asModule")
     val asType: NameType               = nameType("asType")
     val asInstanceOf_ : NameType       = nameType("asInstanceOf")
-    val asInstanceOf_Ob : NameType     = nameType("$" + "asInstanceOf") // looks like missing interpolator due to Any member in scope
+    val asInstanceOf_Ob : NameType     = nameType(s"$$asInstanceOf") // looks like missing interpolator due to Any member in scope
     val async : NameType               = nameType("async")
     val await : NameType               = nameType("await")
     val box: NameType                  = nameType("box")
@@ -728,7 +728,7 @@ trait StdNames {
     val classOf: NameType              = nameType("classOf")
     val clone_ : NameType              = nameType("clone")
     val collection: NameType           = nameType("collection")
-    val conforms: NameType             = nameType("$" + "conforms") // $ prefix to avoid shadowing Predef.conforms
+    val conforms: NameType             = nameType(s"$$conforms") // $ prefix to avoid shadowing Predef.conforms
     val copy: NameType                 = nameType("copy")
     val create: NameType               = nameType("create")
     val currentMirror: NameType        = nameType("currentMirror")
@@ -775,7 +775,7 @@ trait StdNames {
     val isDefinedAt: NameType          = nameType("isDefinedAt")
     val isEmpty: NameType              = nameType("isEmpty")
     val isInstanceOf_ : NameType       = nameType("isInstanceOf")
-    val isInstanceOf_Ob : NameType     = nameType("$" + "isInstanceOf") // looks like missing interpolator due to Any member in scope
+    val isInstanceOf_Ob : NameType     = nameType(s"$$isInstanceOf") // looks like missing interpolator due to Any member in scope
     val java: NameType                 = nameType("java")
     val key: NameType                  = nameType("key")
     val lang: NameType                 = nameType("lang")
@@ -879,13 +879,13 @@ trait StdNames {
     val zero: NameType                 = nameType("zero")
 
     // async
-    val result           : NameType       = nameType("result$" + "async") // avoid missing interpolator warnings
-    val awaitable        : NameType       = nameType("awaitable$" + "async")
-    val completed        : NameType       = nameType("completed$" + "async")
-    val stateMachine     : NameType       = nameType("stateMachine$" + "async")
+    val result           : NameType       = nameType(s"result$$async") // avoid missing interpolator warnings
+    val awaitable        : NameType       = nameType(s"awaitable$$async")
+    val completed        : NameType       = nameType(s"completed$$async")
+    val stateMachine     : NameType       = nameType(s"stateMachine$$async")
     val state            : NameType       = nameType("state")
-    val tr               : NameType       = nameType("tr$" + "async")
-    val t                : NameType       = nameType("throwable$" + "async")
+    val tr               : NameType       = nameType(s"tr$$async")
+    val t                : NameType       = nameType(s"throwable$$async")
 
     // quasiquote interpolators:
     val q: NameType  = nameType("q")

src/reflect/scala/reflect/internal/Symbols.scala

@@ -1722,7 +1722,7 @@ trait Symbols extends api.Symbols { self: SymbolTable =>
     }
     def maybeInitialize = {
       try   { initialize ; true }
-      catch { case _: CyclicReference => debuglog("Hit cycle in maybeInitialize of $this") ; false }
+      catch { case _: CyclicReference => debuglog(s"Hit cycle in maybeInitialize of $this") ; false }
     }
 
     /** Was symbol's type updated during given phase? */

test/files/neg/t7848-interp-warn.check

@@ -22,6 +22,12 @@ t7848-interp-warn.scala:36: warning: possible missing interpolator: detected an
 t7848-interp-warn.scala:38: warning: possible missing interpolator: detected an interpolated expression
   def z = "${ baz * 3}"                                       // warn, no expr parsing
           ^
+t7848-interp-warn.scala:40: warning: possible missing interpolator: detected interpolated identifier `$this`
+  def thisly = "$this"
+               ^
+t7848-interp-warn.scala:41: warning: possible missing interpolator: detected an interpolated expression
+  def exprly = "${this}"
+               ^
 error: No warnings can be incurred under -Werror.
-8 warnings
+10 warnings
 1 error

test/files/neg/t7848-interp-warn.scala

@@ -36,4 +36,7 @@ object Test {
   def x = "${ bar }"                                          // warn, a cheap ident in scope
   def y = "${ baz }"                                          // no warn, cheap ident not in scope
   def z = "${ baz * 3}"                                       // warn, no expr parsing
+
+  def thisly = "$this"
+  def exprly = "${this}"
 }