Skip to content

Commit

Permalink
Warn if implicit should have explicit type
Browse files Browse the repository at this point in the history
  • Loading branch information
som-snytt committed Aug 28, 2022
1 parent 1d1b6d7 commit f9395e1
Show file tree
Hide file tree
Showing 104 changed files with 616 additions and 167 deletions.
2 changes: 1 addition & 1 deletion src/compiler/scala/tools/nsc/PipelineMain.scala
Expand Up @@ -77,7 +77,7 @@ class PipelineMainClass(argFiles: Seq[Path], pipelineSettings: PipelineMain.Pipe
}
}

implicit val executor = ExecutionContext.fromExecutor(new java.util.concurrent.ForkJoinPool(parallelism), t => handler.uncaughtException(Thread.currentThread(), t))
implicit val executor: ExecutionContext = ExecutionContext.fromExecutor(new java.util.concurrent.ForkJoinPool(parallelism), t => handler.uncaughtException(Thread.currentThread(), t))
def changeExtension(p: Path, newExtension: String): Path = {
val fileName = p.getFileName.toString
val changedFileName = fileName.lastIndexOf('.') match {
Expand Down
Expand Up @@ -54,7 +54,8 @@ trait AnalyzerPlugins { self: Analyzer with splain.SplainData =>
/**
* Let analyzer plugins change the types assigned to definitions. For definitions that have
* an annotated type, the assigned type is obtained by typing that type tree. Otherwise, the
* type is inferred by typing the definition's righthand side.
* type is inferred by typing the definition's righthand side, or from the overridden
* member under `-Xsource:3`.
*
* In order to know if the type was inferred, you can query the `wasEmpty` field in the `tpt`
* TypeTree of the definition (for DefDef and ValDef).
Expand Down
33 changes: 29 additions & 4 deletions src/compiler/scala/tools/nsc/typechecker/ContextErrors.scala
Expand Up @@ -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
Expand Down Expand Up @@ -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 _ =>
Expand All @@ -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)
Expand All @@ -186,6 +186,26 @@ 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 =>

Expand Down Expand Up @@ -1049,6 +1069,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. */
Expand Down Expand Up @@ -1077,7 +1099,7 @@ trait ContextErrors extends splain.SplainErrors {

object InferErrorGen {

implicit val contextInferErrorGen = getContext
implicit val contextInferErrorGen: Context = getContext

object PolyAlternativeErrorKind extends Enumeration {
type ErrorType = Value
Expand Down Expand Up @@ -1282,7 +1304,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,
Expand Down Expand Up @@ -1400,6 +1422,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 {
Expand Down
100 changes: 48 additions & 52 deletions src/compiler/scala/tools/nsc/typechecker/Namers.scala
Expand Up @@ -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.
*
Expand Down Expand Up @@ -1122,13 +1123,16 @@ trait Namers extends MethodSynthesis {
* assigns the type to the tpt's node. Returns the type.
*/
private def assignTypeToTree(tree: ValOrDefDef, defnTyper: Typer, pt: Type): Type = {
val rhsTpe = tree match {
case ddef: DefDef if tree.symbol.isTermMacro => defnTyper.computeMacroDefType(ddef, pt)
case _ => defnTyper.computeType(tree.rhs, pt)
}
tree.tpt.defineType {
if (currentRun.isScala3 && !pt.isWildcard && pt != NoType && !pt.isErroneous) pt
else dropIllegalStarTypes(widenIfNecessary(tree.symbol, rhsTpe, pt))
else {
val rhsTpe = tree match {
case ddef: DefDef if tree.symbol.isTermMacro => defnTyper.computeMacroDefType(ddef, pt)
case _ => defnTyper.computeType(tree.rhs, pt)
}
dropIllegalStarTypes(widenIfNecessary(tree.symbol, rhsTpe, pt))
.tap(InferredImplicitError(tree, _, context))
}
}.setPos(tree.pos.focus)
tree.tpt.tpe
}
Expand Down Expand Up @@ -1160,9 +1164,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
Expand Down Expand Up @@ -1383,7 +1385,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
Expand Down Expand Up @@ -1442,7 +1443,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 }
Expand Down Expand Up @@ -1704,63 +1705,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

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

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)
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: 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)

// 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
}
}
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)
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)
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)
// 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)
// 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
}
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)
}

Expand Down
23 changes: 14 additions & 9 deletions src/compiler/scala/tools/nsc/typechecker/Typers.scala
Expand Up @@ -2164,7 +2164,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]()
Expand Down Expand Up @@ -2346,7 +2346,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)
Expand Down Expand Up @@ -2386,9 +2386,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)
}
Expand All @@ -2415,13 +2415,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)
Expand Down
6 changes: 3 additions & 3 deletions src/manual/scala/tools/docutil/ManPage.scala
Expand Up @@ -24,7 +24,7 @@ object ManPage {
case class Emph(contents: AbstractText) extends AbstractText
case class Mono(contents: AbstractText) extends AbstractText
case class Quote(contents: AbstractText) extends AbstractText
implicit def str2text(str: String) = Text(str)
implicit def str2text(str: String): Text = Text(str)

case class Definition(term: AbstractText, description: AbstractText)
case class DefinitionList(definitions: Definition*) extends AbstractText
Expand All @@ -37,14 +37,14 @@ object ManPage {
case class CodeSample(text: String) extends Paragraph
case class BlockQuote(text: AbstractText) extends Paragraph
implicit def text2para(text: AbstractText): Paragraph = TextParagraph(text)
implicit def str2para(str: String) = text2para(str2text(str))
implicit def str2para(str: String): Paragraph = text2para(str2text(str))

case class BulletList(items: AbstractText*) extends Paragraph
case class NumberedList(items: AbstractText*) extends Paragraph
case class TitledPara(title: String, text: AbstractText) extends Paragraph

case class EmbeddedSection(section: Section) extends Paragraph
implicit def section2Para(section: Section) = EmbeddedSection(section)
implicit def section2Para(section: Section): EmbeddedSection = EmbeddedSection(section)

case class Section(title: String, paragraphs: Paragraph*)

Expand Down
3 changes: 2 additions & 1 deletion src/partest/scala/tools/partest/CompilerTest.scala
Expand Up @@ -12,6 +12,7 @@

package scala.tools.partest

import scala.language.implicitConversions
import scala.reflect.runtime.{universe => ru}
import scala.tools.nsc._

Expand Down Expand Up @@ -46,7 +47,7 @@ abstract class CompilerTest extends DirectTest {
if (sym eq NoSymbol) NoType
else appliedType(sym, compilerTypeFromTag(t))
}
implicit def mkMkType(sym: Symbol) = new MkType(sym)
implicit def mkMkType(sym: Symbol): MkType = new MkType(sym)

def allMembers(root: Symbol): List[Symbol] = {
def loop(seen: Set[Symbol], roots: List[Symbol]): List[Symbol] = {
Expand Down
2 changes: 1 addition & 1 deletion src/partest/scala/tools/partest/nest/Instance.scala
Expand Up @@ -28,5 +28,5 @@ trait Instance extends Spec {
def residualArgs = parsed.residualArgs // only args which were not options or args to options

type OptionMagic = Opt.Instance
protected implicit def optionMagicAdditions(name: String) = new Opt.Instance(programInfo, parsed, name)
protected implicit def optionMagicAdditions(name: String): Opt.Instance = new Opt.Instance(programInfo, parsed, name)
}
2 changes: 1 addition & 1 deletion src/partest/scala/tools/partest/nest/Reference.scala
Expand Up @@ -45,7 +45,7 @@ trait Reference extends Spec {
final def apply(args: String*): ThisCommandLine = creator(propertyArgs ++ args flatMap expandArg)

type OptionMagic = Opt.Reference
protected implicit def optionMagicAdditions(name: String) = new Opt.Reference(programInfo, options, name)
protected implicit def optionMagicAdditions(name: String): Opt.Reference = new Opt.Reference(programInfo, options, name)
}

object Reference {
Expand Down
3 changes: 1 addition & 2 deletions src/partest/scala/tools/partest/package.scala
Expand Up @@ -18,6 +18,7 @@ import java.util.concurrent.{Callable, ExecutorService}
import scala.concurrent.duration.Duration
import scala.io.Codec
import scala.jdk.CollectionConverters._
import scala.language.implicitConversions
import scala.tools.nsc.util.Exceptional

package object partest {
Expand Down Expand Up @@ -123,8 +124,6 @@ package object partest {
implicit def temporaryPath2File(x: Path): File = x.jfile
implicit def stringPathToJavaFile(path: String): File = new File(path)

implicit lazy val implicitConversions = scala.language.implicitConversions

def fileSeparator = java.io.File.separator
def pathSeparator = java.io.File.pathSeparator

Expand Down

0 comments on commit f9395e1

Please sign in to comment.