Skip to content

Add explicit return types to public members #5203

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 7 commits into from
Oct 5, 2018

Conversation

OlivierBlanvillain
Copy link
Contributor

No description provided.

@@ -84,7 +76,7 @@ class DottyBackendInterface(outputDirectory: AbstractFile, val superCallsMap: Ma
type LabelDef = tpd.DefDef
type Closure = tpd.Closure

val NoSymbol = Symbols.NoSymbol
val NoSymbol: Symbols.NoSymbol.type = Symbols.NoSymbol
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would do val NoSymbol: Symbol to match the way it's done just below

Copy link
Contributor

@allanrenucci allanrenucci left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

First pass up to Definitions.scala

@@ -31,10 +32,10 @@ import dotty.tools.io._
class GenBCode extends Phase {
def phaseName: String = GenBCode.name
private val entryPoints = new mutable.HashSet[Symbol]()
def registerEntryPoint(sym: Symbol) = entryPoints += sym
def registerEntryPoint(sym: Symbol): mutable.HashSet[Symbol] = entryPoints += sym
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this be Unit?

@@ -71,7 +72,7 @@ class GenBCodePipeline(val entryPoints: List[Symbol], val int: DottyBackendInter

var tree: Tree = _

val sourceFile = ctx.compilationUnit.source
val sourceFile: SourceFile = ctx.compilationUnit.source
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can sourceFile and tree above be private[this]?

@@ -609,7 +609,7 @@ object desugar {
else cpy.ValDef(self)(tpt = selfType).withMods(self.mods | SelfName)
}

val cdef1 = addEnumFlags {
val cdef1: DefTree = addEnumFlags {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why?

}

/** Attachment containing the number of enum cases and the smallest kind that was seen so far. */
val EnumCaseCount = new Property.Key[(Int, CaseKind.Value)]
val EnumCaseCount: Property.Key[(Int, DesugarEnums.CaseKind.Value)] = new Property.Key[(Int, CaseKind.Value)]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rewrite RHS to new Property.Key?

if (enumClass.exists) TypeTree(enumClass.typeRef) else TypeTree()

/** Add implied flags to an enum class or an enum case */
def addEnumFlags(cdef: TypeDef)(implicit ctx: Context) =
def addEnumFlags(cdef: TypeDef)(implicit ctx: Context): cdef.ThisTree[Untyped] =
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TypeDef ?

@@ -81,75 +81,75 @@ object Contexts {
val base: ContextBase

/** All outer contexts, ending in `base.initialCtx` and then `NoContext` */
def outersIterator = new Iterator[Context] {
def outersIterator: AnyRef with Iterator[Context] { var current: Context; } = new Iterator[Context] {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Iterator[Context]?

if (exprOwner == this.owner) this
else if (untpd.isSuperConstrCall(stat) && this.owner.isClass) superCallContext
else ctx.fresh.setOwner(exprOwner)

/** A new context that summarizes an import statement */
def importContext(imp: Import[_], sym: Symbol) = {
def importContext(imp: Import[_], sym: Symbol): FreshContext = {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure it is meant to be a FreshContext. I would try Context and see if it still compile

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nope, has to be fresh:

[error] /home/olivier/workspace/dotty/compiler/src/dotty/tools/repl/ReplCompiler.scala:58:24: type mismatch;
[error]  found   : dotty.tools.dotc.core.Contexts.Context
[error]  required: dotty.tools.dotc.core.Contexts.FreshContext
[error]           importContext(imp)(ctx))
[error]                        ^
[error] one error found

@@ -523,7 +523,7 @@ object Contexts {
def setSetting[T](setting: Setting[T], value: T): this.type =
setSettings(setting.updateIn(settingsState, value))

def setDebug = setSetting(base.settings.Ydebug, true)
def setDebug: FreshContext = setSetting(base.settings.Ydebug, true)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this.type?


/** A map from ErrorType to associated message computation. We use this map
* instead of storing message computations directly in ErrorTypes in order
* to avoid space leaks - the message computation usually captures a context.
*/
private[core] val errorTypeMsg = mutable.Map[ErrorType, () => Message]()
private[core] val errorTypeMsg: mutable.Map[Types.ErrorType, () => Message] = mutable.Map[ErrorType, () => Message]()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove type annotation on RHS

}

@sharable object EmptyGADTMap extends GADTMap(SimpleIdentityMap.Empty) {
override def setBounds(sym: Symbol, b: TypeBounds) = unsupported("EmptyGADTMap.setBounds")
override def setBounds(sym: Symbol, b: TypeBounds): Nothing = unsupported("EmptyGADTMap.setBounds")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unit I beleive

Copy link
Contributor

@allanrenucci allanrenucci left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Second pass up to Types.scala

@@ -190,40 +191,40 @@ class Definitions {
lazy val RootPackage: TermSymbol = ctx.newSymbol(
NoSymbol, nme.ROOTPKG, PackageCreationFlags, TypeRef(NoPrefix, RootClass))

lazy val EmptyPackageVal = ctx.newPackageSymbol(
lazy val EmptyPackageVal: Symbol { type ThisName = Names.TermName } = ctx.newPackageSymbol(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TermSymbol?


/** A package in which we can place all methods that are interpreted specially by the compiler */
lazy val OpsPackageVal = ctx.newCompletePackageSymbol(RootClass, nme.OPS_PACKAGE).entered
lazy val OpsPackageClass = OpsPackageVal.moduleClass.asClass
lazy val OpsPackageVal: Symbol { type ThisName = Names.TermName } = ctx.newCompletePackageSymbol(RootClass, nme.OPS_PACKAGE).entered
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TermSymbol?

lazy val Object_waitL: TermSymbol = enterMethod(ObjectClass, nme.wait_, MethodType(LongType :: Nil, UnitType), Final)
lazy val Object_waitLI: TermSymbol = enterMethod(ObjectClass, nme.wait_, MethodType(LongType :: IntType :: Nil, UnitType), Final)

def ObjectMethods: List[Symbols.TermSymbol] = List(Object_eq, Object_ne, Object_synchronized, Object_clone,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove prefix Symbols.?


// The set of all wrap{X, Ref}Array methods, where X is a value type
val WrapArrayMethods = new PerRun[collection.Set[Symbol]]({ implicit ctx =>
val WrapArrayMethods: PerRun[collection.Set[Symbol]] = new PerRun[collection.Set[Symbol]]({ implicit ctx =>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove type annotation from RHS?

def staticsMethodRef(name: PreName) = ScalaStaticsModule.requiredMethodRef(name)
def staticsMethod(name: PreName) = ScalaStaticsModule.requiredMethod(name)
def NullType: TypeRef = NullClass.typeRef
lazy val RuntimeNullModuleRef: Types.TermRef = ctx.requiredModuleRef("scala.runtime.Null")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

import TermRef for all the definitions below?

newConstructor(cls, EmptyFlags, Nil, Nil)

/** Create a synthetic lazy implicit value */
def newLazyImplicit(info: Type) =
def newLazyImplicit(info: Type): Symbol { type ThisName = Names.TermName } =
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TermSymbol?

@@ -308,9 +308,9 @@ trait Symbols { this: Context =>
/** Create a new skolem symbol. This is not the same as SkolemType, even though the
* motivation (create a singleton referencing to a type) is similar.
*/
def newSkolem(tp: Type) = newSymbol(defn.RootClass, nme.SKOLEM, SyntheticArtifact | NonMember | Permanent, tp)
def newSkolem(tp: Type): Symbol { type ThisName = Names.TermName } = newSymbol(defn.RootClass, nme.SKOLEM, SyntheticArtifact | NonMember | Permanent, tp)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TermSymbol?


def newErrorSymbol(owner: Symbol, name: Name, msg: => Message) = {
def newErrorSymbol(owner: Symbol, name: Name, msg: => Message): Symbol { type ThisName = Names.Name } = {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Symbol?

@@ -404,7 +404,7 @@ object Symbols {
implicit def eqSymbol: Eq[Symbol, Symbol] = Eq

/** Tree attachment containing the identifiers in a tree as a sorted array */
val Ids = new Property.Key[Array[String]]
val Ids: Property.Key[Array[String]] = new Property.Key[Array[String]]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

RHS: new Property.Key?

}

final class CachedErasedValueType(tycon: TypeRef, erasedUnderlying: Type)
extends ErasedValueType(tycon, erasedUnderlying)

object ErasedValueType {
def apply(tycon: TypeRef, erasedUnderlying: Type)(implicit ctx: Context) = {
def apply(tycon: TypeRef, erasedUnderlying: Type)(implicit ctx: Context): CachedErasedValueType = {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ErasedValueType?

@@ -76,11 +72,11 @@ class TyperState(previous: TyperState /* | Null */) {
new TyperState(this).setReporter(new StoreReporter(reporter)).setCommittable(isCommittable)

/** The uninstantiated variables */
def uninstVars = constraint.uninstVars
def uninstVars: Seq[Types.TypeVar] = constraint.uninstVars
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seq[TypeVar]

def kindString = "Term"
def copyBoundType(bt: BT) = bt.paramRefs(paramNum)
def kindString: String = "Term"
def copyBoundType(bt: BT): bt.ParamRefType = bt.paramRefs(paramNum)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Type?

def kindString = "Type"
def copyBoundType(bt: BT) = bt.paramRefs(paramNum)
def kindString: String = "Type"
def copyBoundType(bt: BT): bt.ParamRefType = bt.paramRefs(paramNum)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Type?


val watchList = List[String](
val watchList: List[Names.TypeName] = List[String](
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove prefix Names.

@@ -68,7 +68,7 @@ class ClassfileParser(
protected var pool: ConstantPool = _ // the classfile's constant pool

protected var currentClassName: SimpleName = _ // JVM name of the current class
protected var classTParams = Map[Name,Symbol]()
protected var classTParams: Map[Name, Symbol] = Map[Name, Symbol]()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove RHS type annotation

val matchMarker = marker("Match")
val orMarker: xsbti.api.Annotation = marker("Or")
val byNameMarker: xsbti.api.Annotation = marker("ByName")
val matchMarker: xsbti.api.Annotation = marker("Match")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

privates

@@ -71,7 +71,7 @@ class CapturedVars extends MiniPhase with IdentityDenotTransformer { thisPhase =
}
}

override def prepareForUnit(tree: Tree)(implicit ctx: Context) = {
override def prepareForUnit(tree: Tree)(implicit ctx: Context): FreshContext = {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Context

@@ -90,14 +90,14 @@ class Erasure extends Phase with DenotTransformer {
ref.derivedSingleDenotation(ref.symbol, transformInfo(ref.symbol, ref.symbol.info))
}

val eraser = new Erasure.Typer(this)
val eraser: Erasure.Typer = new Erasure.Typer(this)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

private

val functionName = "JFunction".toTermName
val functionPackage = "scala.compat.java8.".toTermName
val functionName: TermName = "JFunction".toTermName
val functionPackage: TermName = "scala.compat.java8.".toTermName
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

privates

@@ -55,7 +55,7 @@ class HoistSuperArgs extends MiniPhase with IdentityDenotTransformer { thisPhase
* Hoisted superarg methods are collected in `superArgDefs`
*/
class Hoister(cls: Symbol)(implicit ctx: Context) {
val superArgDefs = new mutable.ListBuffer[DefDef]
val superArgDefs: mutable.ListBuffer[DefDef] = new mutable.ListBuffer[DefDef]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove RHS type annotation

@@ -29,7 +29,7 @@ object LambdaLift {
private type SymSet = TreeSet[Symbol]

/** A map storing free variables of functions and classes */
val free = new LinkedHashMap[Symbol, SymSet]
val free: mutable.LinkedHashMap[Symbol, SymSet] = new LinkedHashMap[Symbol, SymSet]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove RHS type annotation

@@ -56,7 +56,7 @@ object LambdaLift {
private val outerParam = new HashMap[Symbol, Symbol]

/** Buffers for lifted out classes and methods, indexed by owner */
val liftedDefs = new HashMap[Symbol, mutable.ListBuffer[Tree]]
val liftedDefs: mutable.HashMap[Symbol, mutable.ListBuffer[Tree]] = new HashMap[Symbol, mutable.ListBuffer[Tree]]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove RHS type annotation

@@ -27,22 +28,22 @@ class LazyVals extends MiniPhase with IdentityDenotTransformer {
/** this map contains mutable state of transformation: OffsetDefs to be appended to companion object definitions,
* and number of bits currently used */
class OffsetInfo(var defs: List[Tree], var ord:Int)
val appendOffsetDefs = mutable.Map.empty[Symbol, OffsetInfo]
val appendOffsetDefs: mutable.Map[Symbol, OffsetInfo] = mutable.Map.empty[Symbol, OffsetInfo]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

private?


val containerFlagsMask = Flags.Method | Flags.Lazy | Flags.Accessor | Flags.Module
val containerFlagsMask: Flags.FlagSet = Flags.Method | Flags.Lazy | Flags.Accessor | Flags.Module
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

import FlagSet

def forwarder(target: Symbol) = (targs: List[Type]) => (vrefss: List[List[Tree]]) =>
superRef(target).appliedToTypes(targs).appliedToArgss(vrefss)
def forwarder(target: Symbol): List[Type] => List[List[Tree]] => Tree =
(targs: List[Type]) => (vrefss: List[List[Tree]]) =>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

targs => vrefss =>


/** Does this abstract file denote an existing file? */
def create() = { unsupported() }
def create(): Nothing = { unsupported() }
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unit


/** Delete the underlying file or directory (recursively). */
def delete() = { unsupported() }
def delete(): Nothing = { unsupported() }
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unit

@@ -92,5 +92,5 @@ class VirtualFile(val name: String, override val path: String) extends AbstractF
/** Returns an abstract file with the given name. It does not
* check that it exists.
*/
def lookupNameUnchecked(name: String, directory: Boolean) = unsupported()
def lookupNameUnchecked(name: String, directory: Boolean): Nothing = unsupported()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AbstractFile

case x: VirtualFile => x.path == path
case _ => false
}

private[this] var content = Array.emptyByteArray

def absolute = this
def absolute: VirtualFile = this
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AbstractFile

@@ -21,22 +21,22 @@ extends AbstractFile {
case Some(parent) => parent.path+'/'+ name
}

def absolute = this
def absolute: VirtualDirectory = this
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AbstractFile

Copy link
Contributor

@allanrenucci allanrenucci left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done!

def output = unsupported()
def container = unsupported()
def absolute = unsupported()
override def underlyingSource: Some[ZipArchive] = Some(this)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Option[AbstractFile]

def absolute = unsupported()
override def underlyingSource: Some[ZipArchive] = Some(this)
def isDirectory: Boolean = true
def lookupName(name: String, directory: Boolean): Nothing = unsupported()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AbstractFile

override def underlyingSource: Some[ZipArchive] = Some(this)
def isDirectory: Boolean = true
def lookupName(name: String, directory: Boolean): Nothing = unsupported()
def lookupNameUnchecked(name: String, directory: Boolean): Nothing = unsupported()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AbstractFile

def isDirectory: Boolean = true
def lookupName(name: String, directory: Boolean): Nothing = unsupported()
def lookupNameUnchecked(name: String, directory: Boolean): Nothing = unsupported()
def create(): Nothing = unsupported()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unit

def lookupName(name: String, directory: Boolean): Nothing = unsupported()
def lookupNameUnchecked(name: String, directory: Boolean): Nothing = unsupported()
def create(): Nothing = unsupported()
def delete(): Nothing = unsupported()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unit


/** ''Note: This library is considered experimental and should not be used unless you know what you are doing.'' */
sealed abstract class Entry(path: String) extends VirtualFile(baseName(path), path) {
// have to keep this name for compat with sbt's compiler-interface
def getArchive: ZipFile = null
override def underlyingSource = Some(self)
override def toString = self.path + "(" + path + ")"
override def underlyingSource: Some[ZipArchive] = Some(self)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Option[AbstractFile]

}

/** ''Note: This library is considered experimental and should not be used unless you know what you are doing.'' */
class DirEntry(path: String) extends Entry(path) {
val entries = mutable.HashMap[String, Entry]()
val entries: mutable.HashMap[String, Entry] = mutable.HashMap[String, Entry]()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove RHS type annotation

@@ -147,7 +147,7 @@ final class FileZipArchive(jpath: JPath) extends ZipArchive(jpath) {
override def sizeOption: Option[Int] = Some(zipEntry.getSize.toInt)
}

@volatile lazy val (root, allDirs) = {
@volatile lazy val ((root: DirEntry), (allDirs: mutable.HashMap[String, DirEntry])) = {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lazy val (root , allDirs): (DirEntry, Map[String, DirEntry]))

override def canEqual(other: Any) = other.isInstanceOf[FileZipArchive]
override def hashCode() = jpath.hashCode
override def equals(that: Any) = that match {
override def sizeOption: Some[Int] = Some(Files.size(jpath).toInt)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Option[Int]

def lastModified = Files.getLastModifiedTime(jpath).toMillis
def name: String = jpath.getFileName.toString
def path: String = jpath.toString
def input: java.io.InputStream = Files.newInputStream(jpath)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove prefix java.io.

Using dotty-compiler/scalafix RemoveUnused
Using a published local version of RscCompat
This reverts commit 8861d18.
@OlivierBlanvillain
Copy link
Contributor Author

:shipit:

@OlivierBlanvillain OlivierBlanvillain merged commit 786e6da into scala:master Oct 5, 2018
@OlivierBlanvillain OlivierBlanvillain deleted the scalafix branch October 5, 2018 16:17
allanrenucci added a commit to dotty-staging/dotty that referenced this pull request Oct 5, 2018
allanrenucci added a commit to dotty-staging/dotty that referenced this pull request Oct 6, 2018
allanrenucci added a commit to dotty-staging/dotty that referenced this pull request Oct 7, 2018
nicolasstucki added a commit that referenced this pull request Oct 8, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants