From c43487a48c17b2baa7f6e295530fd355402e8fa7 Mon Sep 17 00:00:00 2001 From: Som Snytt Date: Sat, 7 Nov 2020 15:36:22 -0800 Subject: [PATCH] Notice this when linting for missing interpolator --- .../nsc/backend/jvm/BCodeBodyBuilder.scala | 2 +- .../jvm/analysis/NullnessAnalyzer.scala | 2 +- .../scala/tools/nsc/typechecker/Typers.scala | 54 ++++++++++--------- .../scala/reflect/internal/StdNames.scala | 30 +++++------ .../scala/reflect/internal/Symbols.scala | 2 +- test/files/neg/t7848-interp-warn.check | 8 ++- test/files/neg/t7848-interp-warn.scala | 3 ++ 7 files changed, 57 insertions(+), 44 deletions(-) diff --git a/src/compiler/scala/tools/nsc/backend/jvm/BCodeBodyBuilder.scala b/src/compiler/scala/tools/nsc/backend/jvm/BCodeBodyBuilder.scala index 619af5496048..f7d8648e8859 100644 --- a/src/compiler/scala/tools/nsc/backend/jvm/BCodeBodyBuilder.scala +++ b/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 diff --git a/src/compiler/scala/tools/nsc/backend/jvm/analysis/NullnessAnalyzer.scala b/src/compiler/scala/tools/nsc/backend/jvm/analysis/NullnessAnalyzer.scala index f4f54acf7c93..81923fe80461 100644 --- a/src/compiler/scala/tools/nsc/backend/jvm/analysis/NullnessAnalyzer.scala +++ b/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 diff --git a/src/compiler/scala/tools/nsc/typechecker/Typers.scala b/src/compiler/scala/tools/nsc/typechecker/Typers.scala index 961efdaf34f7..a3bfd7fd00da 100644 --- a/src/compiler/scala/tools/nsc/typechecker/Typers.scala +++ b/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 _ => } } diff --git a/src/reflect/scala/reflect/internal/StdNames.scala b/src/reflect/scala/reflect/internal/StdNames.scala index 35929b550a72..df8399d06adb 100644 --- a/src/reflect/scala/reflect/internal/StdNames.scala +++ b/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("") // emitted by virtual pattern matcher, replaced by outer accessor in explicitouter val ROOTPKG: NameType = nameType("_root_") val SELECTOR_DUMMY: NameType = nameType("") - 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") diff --git a/src/reflect/scala/reflect/internal/Symbols.scala b/src/reflect/scala/reflect/internal/Symbols.scala index 306b9ba781ab..ae234a13f29a 100644 --- a/src/reflect/scala/reflect/internal/Symbols.scala +++ b/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? */ diff --git a/test/files/neg/t7848-interp-warn.check b/test/files/neg/t7848-interp-warn.check index 3febc02687e7..c002144f2614 100644 --- a/test/files/neg/t7848-interp-warn.check +++ b/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 diff --git a/test/files/neg/t7848-interp-warn.scala b/test/files/neg/t7848-interp-warn.scala index 5b5c7126f791..af087ff1ac52 100644 --- a/test/files/neg/t7848-interp-warn.scala +++ b/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}" }