Warn if implicit should have explicit type

This commit is contained in:
Som Snytt 2022-08-28 12:42:44 -07:00
parent bfedc2b966
commit 2aaf097229
11 changed files with 203 additions and 63 deletions

View File

@ -21,6 +21,7 @@ import scala.annotation.tailrec
import scala.reflect.runtime.ReflectionUtils
import scala.reflect.macros.runtime.AbortMacroException
import scala.util.control.{ControlThrowable, NonFatal}
import scala.tools.nsc.Reporting.WarningCategory
import scala.tools.nsc.util.stackTraceString
import scala.reflect.io.NoAbstractFile
import scala.reflect.internal.util.NoSourceFile
@ -153,7 +154,7 @@ trait ContextErrors extends splain.SplainErrors {
MacroIncompatibleEngineError("macro cannot be expanded, because it was compiled by an incompatible macro engine", internalMessage)
/** The implicit not found message from the annotation, and whether it's a supplement message or not. */
def NoImplicitFoundAnnotation(tree: Tree, param: Symbol): (Boolean, String) = {
def NoImplicitFoundAnnotation(tree: Tree, param: Symbol): (Boolean, String) =
param match {
case ImplicitNotFoundMsg(msg) => (false, msg.formatParameterMessage(tree))
case _ =>
@ -167,7 +168,6 @@ trait ContextErrors extends splain.SplainErrors {
true -> supplement
}
}
}
def NoImplicitFoundError(tree: Tree, param: Symbol)(implicit context: Context): Unit = {
val (isSupplement, annotationMsg) = NoImplicitFoundAnnotation(tree, param)
@ -186,6 +186,24 @@ trait ContextErrors extends splain.SplainErrors {
issueNormalTypeError(tree, if (errMsg.isEmpty) defaultErrMsg else errMsg)
}
private def InferredImplicitErrorImpl(tree: Tree, inferred: Type, cx: Context, isTyper: Boolean): Unit = {
def err(): Unit = {
val msg =
s"Implicit definition ${if (currentRun.isScala3) "must" else "should"} have explicit type${
if (!inferred.isErroneous) s" (inferred $inferred)" else ""
}"
if (currentRun.isScala3) ErrorUtils.issueNormalTypeError(tree, msg)(cx)
else cx.warning(tree.pos, msg, WarningCategory.Other)
}
val sym = tree.symbol
// Defer warning field of class until typing getter (which is marked implicit)
if (sym.isImplicit) {
if (!sym.isLocalToBlock) err()
}
else if (!isTyper && sym.isField && !sym.isLocalToBlock)
sym.updateAttachment(FieldTypeInferred)
}
trait TyperContextErrors {
self: Typer =>
@ -1049,6 +1067,8 @@ trait ContextErrors extends splain.SplainErrors {
def MacroAnnotationTopLevelModuleBadExpansion(ann: Tree) =
issueNormalTypeError(ann, "top-level object can only expand into an eponymous object")
def InferredImplicitError(tree: Tree, inferred: Type, cx: Context): Unit =
InferredImplicitErrorImpl(tree, inferred, cx, isTyper = true)
}
/** This file will be the death of me. */
@ -1077,7 +1097,7 @@ trait ContextErrors extends splain.SplainErrors {
object InferErrorGen {
implicit val contextInferErrorGen = getContext
implicit val contextInferErrorGen: Context = getContext
object PolyAlternativeErrorKind extends Enumeration {
type ErrorType = Value
@ -1282,7 +1302,7 @@ trait ContextErrors extends splain.SplainErrors {
object NamerErrorGen {
implicit val contextNamerErrorGen = context
implicit val contextNamerErrorGen: Context = context
object SymValidateErrors extends Enumeration {
val ImplicitConstr, ImplicitNotTermOrClass, ImplicitAtToplevel,
@ -1400,6 +1420,9 @@ trait ContextErrors extends splain.SplainErrors {
issueNormalTypeError(tree, name.decode + " " + msg)
}
}
def InferredImplicitError(tree: Tree, inferred: Type, cx: Context): Unit =
InferredImplicitErrorImpl(tree, inferred, cx, isTyper = false)
}
trait ImplicitsContextErrors {

View File

@ -18,6 +18,7 @@ import scala.collection.mutable
import symtab.Flags._
import scala.reflect.internal.util.ListOfNil
import scala.tools.nsc.Reporting.WarningCategory
import scala.util.chaining._
/** This trait declares methods to create symbols and to enter them into scopes.
*
@ -1149,6 +1150,7 @@ trait Namers extends MethodSynthesis {
}
if (inferOverridden) pt
else dropIllegalStarTypes(widenIfNecessary(tree.symbol, rhsTpe, pt))
.tap(InferredImplicitError(tree, _, context))
}.setPos(tree.pos.focus)
tree.tpt.tpe
}
@ -1180,9 +1182,7 @@ trait Namers extends MethodSynthesis {
private def templateSig(templ: Template): Type = {
val clazz = context.owner
val parentTrees = typer.typedParentTypes(templ)
val pending = mutable.ListBuffer[AbsTypeError]()
parentTrees foreach { tpt =>
val ptpe = tpt.tpe
@ -1400,7 +1400,6 @@ trait Namers extends MethodSynthesis {
*
* If the result type is missing, assign a MethodType to `meth` that's constructed using this return type.
* This allows omitting the result type for recursive methods.
*
*/
val resTpFromOverride =
if (!inferResTp || overridden == NoSymbol || overridden.isOverloaded) resTpGiven
@ -1459,7 +1458,7 @@ trait Namers extends MethodSynthesis {
val resTp = {
// When return type is inferred, we don't just use resTpFromOverride -- it must be packed and widened.
// Here, C.f has type String:
// Here, C.f has type String (unless -Xsource:3):
// trait T { def f: Object }; class C extends T { def f = "" }
// using resTpFromOverride as expected type allows for the following (C.f has type A):
// trait T { def f: A }; class C extends T { implicit def b2a(t: B): A = ???; def f = new B }
@ -1721,63 +1720,58 @@ trait Namers extends MethodSynthesis {
}
}
private def valDefSig(vdef: ValDef) = {
private def valDefSig(vdef: ValDef): Type = {
val ValDef(_, _, tpt, rhs) = vdef
val result =
if (tpt.isEmpty) {
if (rhs.isEmpty) {
MissingParameterOrValTypeError(tpt)
ErrorType
} else {
// enterGetterSetter assigns the getter's symbol to a ValDef when there's no underlying field
// (a deferred val or most vals defined in a trait -- see Field.noFieldFor)
val isGetter = vdef.symbol hasFlag ACCESSOR
def inferredValTpt: Type = {
// enterGetterSetter assigns the getter's symbol to a ValDef when there's no underlying field
// (a deferred val or most vals defined in a trait -- see Field.noFieldFor)
val isGetter = vdef.symbol hasFlag ACCESSOR
val pt = {
val valOwner = owner.owner
// there's no overriding outside of classes, and we didn't use to do this in 2.11, so provide opt-out
val pt: Type = {
val valOwner = owner.owner
if (!valOwner.isClass) WildcardType
else {
// normalize to getter so that we correctly consider a val overriding a def
// (a val's name ends in a " ", so can't compare to def)
val overridingSym = if (isGetter) vdef.symbol else vdef.symbol.getterIn(valOwner)
if (!valOwner.isClass) WildcardType
else {
// normalize to getter so that we correctly consider a val overriding a def
// (a val's name ends in a " ", so can't compare to def)
val overridingSym = if (isGetter) vdef.symbol else vdef.symbol.getterIn(valOwner)
// We're called from an accessorTypeCompleter, which is completing the info for the accessor's symbol,
// which may or may not be `vdef.symbol` (see isGetter above)
val overridden = safeNextOverriddenSymbol(overridingSym)
// We're called from an accessorTypeCompleter, which is completing the info for the accessor's symbol,
// which may or may not be `vdef.symbol` (see isGetter above)
val overridden = safeNextOverriddenSymbol(overridingSym)
if (overridden == NoSymbol || overridden.isOverloaded) WildcardType
else valOwner.thisType.memberType(overridden).resultType
}
}
def patchSymInfo(tp: Type): Unit =
if (pt ne WildcardType) // no patching up to do if we didn't infer a prototype
vdef.symbol setInfo (if (isGetter) NullaryMethodType(tp) else tp)
patchSymInfo(pt)
// derives the val's result type from type checking its rhs under the expected type `pt`
// vdef.tpt is mutated, and `vdef.tpt.tpe` is `assignTypeToTree`'s result
val tptFromRhsUnderPt = assignTypeToTree(vdef, typer, pt)
// need to re-align with assignTypeToTree, as the type we're returning from valDefSig (tptFromRhsUnderPt)
// may actually go to the accessor, not the valdef (and if assignTypeToTree returns a subtype of `pt`,
// we would be out of synch between field and its accessors), and thus the type completer won't
// fix the symbol's info for us -- we set it to tmpInfo above, which may need to be improved to tptFromRhsUnderPt
if (!isGetter) patchSymInfo(tptFromRhsUnderPt)
tptFromRhsUnderPt
if (overridden == NoSymbol || overridden.isOverloaded) WildcardType
else valOwner.thisType.memberType(overridden).resultType
}
}
def patchSymInfo(tp: Type): Unit =
if (pt ne WildcardType) // no patching up to do if we didn't infer a prototype
vdef.symbol.setInfo { if (isGetter) NullaryMethodType(tp) else tp }
patchSymInfo(pt)
// derives the val's result type from type checking its rhs under the expected type `pt`
// vdef.tpt is mutated, and `vdef.tpt.tpe` is `assignTypeToTree`'s result
val tptFromRhsUnderPt = assignTypeToTree(vdef, typer, pt)
// need to re-align with assignTypeToTree, as the type we're returning from valDefSig (tptFromRhsUnderPt)
// may actually go to the accessor, not the valdef (and if assignTypeToTree returns a subtype of `pt`,
// we would be out of synch between field and its accessors), and thus the type completer won't
// fix the symbol's info for us -- we set it to tmpInfo above, which may need to be improved to tptFromRhsUnderPt
if (!isGetter) patchSymInfo(tptFromRhsUnderPt)
tptFromRhsUnderPt
}
val result: Type =
if (tpt.isEmpty) {
if (rhs.isEmpty) { MissingParameterOrValTypeError(tpt); ErrorType }
else inferredValTpt
} else {
val tptTyped = typer.typedType(tpt)
context.unit.transformed(tpt) = tptTyped
tptTyped.tpe
}
// println(s"val: $result / ${vdef.tpt.tpe} / ")
pluginsTypeSig(result, typer, vdef, if (tpt.isEmpty) WildcardType else result)
}

View File

@ -2173,7 +2173,7 @@ trait Typers extends Adaptations with Tags with TypersTracking with PatternTyper
}
/** Analyze the super constructor call to record information used later to compute parameter aliases */
def analyzeSuperConsructor(meth: Symbol, vparamss: List[List[ValDef]], rhs: Tree): Unit = {
def analyzeSuperConstructor(meth: Symbol, vparamss: List[List[ValDef]], rhs: Tree): Unit = {
val clazz = meth.owner
debuglog(s"computing param aliases for $clazz:${clazz.primaryConstructor.tpe}:$rhs")
val pending = ListBuffer[AbsTypeError]()
@ -2355,7 +2355,7 @@ trait Typers extends Adaptations with Tags with TypersTracking with PatternTyper
}
}
val tparams1 = ddef.tparams mapConserve typedTypeDef
val tparams1 = ddef.tparams.mapConserve(typedTypeDef)
val vparamss1 = ddef.vparamss.mapConserve(_.mapConserve(typedValDef))
warnTypeParameterShadow(tparams1, meth)
@ -2395,9 +2395,9 @@ trait Typers extends Adaptations with Tags with TypersTracking with PatternTyper
if (meth.isClassConstructor && !isPastTyper && !meth.owner.isSubClass(AnyValClass) && !meth.isJava) {
// There are no supercalls for AnyVal or constructors from Java sources, which
// would blow up in analyzeSuperConsructor; there's nothing to be computed for them anyway.
// would blow up in analyzeSuperConstructor; there's nothing to be computed for them anyway.
if (meth.isPrimaryConstructor)
analyzeSuperConsructor(meth, vparamss1, rhs1)
analyzeSuperConstructor(meth, vparamss1, rhs1)
else
checkSelfConstructorArgs(ddef, meth.owner)
}
@ -2424,13 +2424,18 @@ trait Typers extends Adaptations with Tags with TypersTracking with PatternTyper
if (meth.isStructuralRefinementMember)
checkMethodStructuralCompatible(ddef)
if (meth.isImplicit && !meth.isSynthetic) meth.paramss match {
case List(param) :: _ if !param.isImplicit =>
checkFeature(ddef.pos, currentRun.runDefinitions.ImplicitConversionsFeature, meth.toString)
case _ =>
if (meth.isImplicit) {
if (!meth.isSynthetic) meth.paramss match {
case List(param) :: _ if !param.isImplicit =>
checkFeature(ddef.pos, currentRun.runDefinitions.ImplicitConversionsFeature, meth.toString)
case _ =>
}
if (meth.isGetter && !meth.isLocalToBlock && meth.accessed.hasAttachment[FieldTypeInferred.type]) {
meth.accessed.removeAttachment[FieldTypeInferred.type]
InferredImplicitError(ddef, meth.accessed.tpe.resultType, context)
}
}
}
treeCopy.DefDef(ddef, typedMods, ddef.name, tparams1, vparamss1, tpt1, rhs1) setType NoType
} finally {
currentRun.profiler.afterTypedImplDef(meth)

View File

@ -149,4 +149,7 @@ trait StdAttachments {
/** Marks a Typed tree with Unit tpt. */
case object TypedExpectingUnitAttachment
def explicitlyUnit(tree: Tree): Boolean = tree.hasAttachment[TypedExpectingUnitAttachment.type]
/** For `val i = 42`, marks field as inferred so accessor (getter) can warn if implicit. */
case object FieldTypeInferred extends PlainAttachment
}

View File

@ -78,6 +78,7 @@ trait JavaUniverseForce { self: runtime.JavaUniverse =>
this.InterpolatedString
this.RootSelection
this.TypedExpectingUnitAttachment
this.FieldTypeInferred
this.noPrint
this.typeDebug
// inaccessible: this.posAssigner

View File

@ -0,0 +1,18 @@
t5265a.scala:7: warning: Implicit definition should have explicit type (inferred T[String])
implicit val tsMissing = new T[String] {} // warn val in trait
^
t5265a.scala:20: warning: Implicit definition should have explicit type (inferred T[String])
implicit val tsChild = new T[String] {} // warn because inferred from RHS
^
t5265a.scala:22: warning: Implicit definition should have explicit type (inferred Int)
implicit private[this] val pChild = 42 // also warn
^
t5265a.scala:27: warning: Implicit definition should have explicit type (inferred Int)
implicit private[this] val y = 42 // also warn
^
t5265a.scala:25: warning: Implicit definition should have explicit type (inferred T[String])
implicit val tsD = new T[String] {} // warn val in class
^
error: No warnings can be incurred under -Werror.
5 warnings
1 error

View File

@ -0,0 +1,32 @@
// scalac: -Werror
trait T[A]
class C[A: T]
trait Missing {
implicit val tsMissing = new T[String] {} // warn val in trait
def f = new C[String]
}
trait Local {
def f = {
implicit val tsLocal = new T[String] {} // nowarn because local
new C[String]
}
}
trait Parent {
def t: T[String]
}
trait Child extends Parent {
implicit val tsChild = new T[String] {} // warn because inferred from RHS
def f = new C[String]
implicit private[this] val pChild = 42 // also warn
}
class D {
implicit val tsD = new T[String] {} // warn val in class
def f = new C[String]
implicit private[this] val y = 42 // also warn
}
class X extends Missing
trait Z {
val z = 42
}

View File

@ -0,0 +1,4 @@
t5265b.scala:7: error: Implicit definition must have explicit type (inferred T[String])
implicit val tsMissing = new T[String] {} // warn val in trait
^
1 error

View File

@ -0,0 +1,22 @@
// scalac: -Werror -Xlint -Xsource:3
trait T[A]
class C[A: T]
trait Missing {
implicit val tsMissing = new T[String] {} // warn val in trait
def f = new C[String]
}
trait Local {
def f = {
implicit val tsLocal = new T[String] {} // nowarn because local
new C[String]
}
}
trait Parent {
def tsChild: T[String]
}
trait Child extends Parent {
implicit val tsChild = new T[String] {} // nowarn because inferred from overridden
def f = new C[String]
}

View File

@ -0,0 +1,16 @@
t7212.scala:8: error: type mismatch;
found : Object
required: String
val s: String = k.f
^
t7212.scala:14: error: type mismatch;
found : Object
required: String
val s: String = f.f
^
t7212.scala:21: error: type mismatch;
found : Object
required: String
val s: String = w.f
^
3 errors

View File

@ -0,0 +1,22 @@
// scalac: -Xsource:3
trait T { def f: Object }
class K extends T { def f = "" }
object K {
val k = new K
val s: String = k.f
}
class F extends T { val f = "" }
object F {
val f = new F
val s: String = f.f
}
trait V extends T { var f = "" }
class W extends V
object W {
val w = new W
val s: String = w.f
}