diff --git a/src/compiler/scala/tools/nsc/typechecker/ContextErrors.scala b/src/compiler/scala/tools/nsc/typechecker/ContextErrors.scala index 2521ce0b4c..0b56ec5495 100644 --- a/src/compiler/scala/tools/nsc/typechecker/ContextErrors.scala +++ b/src/compiler/scala/tools/nsc/typechecker/ContextErrors.scala @@ -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 { diff --git a/src/compiler/scala/tools/nsc/typechecker/Namers.scala b/src/compiler/scala/tools/nsc/typechecker/Namers.scala index 3926a2265b..ad1aee3579 100644 --- a/src/compiler/scala/tools/nsc/typechecker/Namers.scala +++ b/src/compiler/scala/tools/nsc/typechecker/Namers.scala @@ -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) } diff --git a/src/compiler/scala/tools/nsc/typechecker/Typers.scala b/src/compiler/scala/tools/nsc/typechecker/Typers.scala index aaf4eabfc9..f76643fafc 100644 --- a/src/compiler/scala/tools/nsc/typechecker/Typers.scala +++ b/src/compiler/scala/tools/nsc/typechecker/Typers.scala @@ -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) diff --git a/src/reflect/scala/reflect/internal/StdAttachments.scala b/src/reflect/scala/reflect/internal/StdAttachments.scala index 6045c42ade..2aefc327cc 100644 --- a/src/reflect/scala/reflect/internal/StdAttachments.scala +++ b/src/reflect/scala/reflect/internal/StdAttachments.scala @@ -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 } diff --git a/src/reflect/scala/reflect/runtime/JavaUniverseForce.scala b/src/reflect/scala/reflect/runtime/JavaUniverseForce.scala index ead2054e90..b4d932c591 100644 --- a/src/reflect/scala/reflect/runtime/JavaUniverseForce.scala +++ b/src/reflect/scala/reflect/runtime/JavaUniverseForce.scala @@ -78,6 +78,7 @@ trait JavaUniverseForce { self: runtime.JavaUniverse => this.InterpolatedString this.RootSelection this.TypedExpectingUnitAttachment + this.FieldTypeInferred this.noPrint this.typeDebug // inaccessible: this.posAssigner diff --git a/test/files/neg/t5265a.check b/test/files/neg/t5265a.check new file mode 100644 index 0000000000..f788868ec4 --- /dev/null +++ b/test/files/neg/t5265a.check @@ -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 diff --git a/test/files/neg/t5265a.scala b/test/files/neg/t5265a.scala new file mode 100644 index 0000000000..80b127e1ad --- /dev/null +++ b/test/files/neg/t5265a.scala @@ -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 +} diff --git a/test/files/neg/t5265b.check b/test/files/neg/t5265b.check new file mode 100644 index 0000000000..97f96fd82b --- /dev/null +++ b/test/files/neg/t5265b.check @@ -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 diff --git a/test/files/neg/t5265b.scala b/test/files/neg/t5265b.scala new file mode 100644 index 0000000000..19512c3c91 --- /dev/null +++ b/test/files/neg/t5265b.scala @@ -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] +} diff --git a/test/files/neg/t7212.check b/test/files/neg/t7212.check new file mode 100644 index 0000000000..8f1fe20edc --- /dev/null +++ b/test/files/neg/t7212.check @@ -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 diff --git a/test/files/neg/t7212.scala b/test/files/neg/t7212.scala new file mode 100644 index 0000000000..5dd16a6c80 --- /dev/null +++ b/test/files/neg/t7212.scala @@ -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 +}