Leveraged -Xmigration to burn off some warts wh...

Leveraged -Xmigration to burn off some warts which arose in the new
collections. Warnings put in place for behavioral changes, allowing the
following. 1) Buffers: create new collections on

++ and -- like all the other collections.  2) Maps: eliminated
never-shipped redundant method valuesIterable and supplied these
return types:

  def keys: Iterable[A]
  def keysIterator: Iterator[A]
  def values: Iterable[B]
  def valuesIterator: Iterator[B]
  def keySet: Set[A]

I concluded that keys should return Iterable because keySet also exists
on Map, and is not solely in the province of Maps even if we wanted to
change it: it's defined on Sorted and also appears in some Sets. So it
seems sensible to have keySet return a Set and keys return the more
general type.

Closes #3089, #3145.  Review by odersky.

22 files changed, 47 insertions, 50 deletions

diff --git a/src/compiler/scala/tools/nsc/Global.scala b/src/compiler/scala/tools/nsc/Global.scala
index bb9fbed0e9..8219edd464 100644
--- a/src/compiler/scala/tools/nsc/Global.scala
+++ b/src/compiler/scala/tools/nsc/Global.scala
@@ -923,7 +923,7 @@ class Global(var settings: Settings, var reporter: Reporter) extends SymbolTable
 
   private def writeICode() {
     val printer = new icodes.TextPrinter(null, icodes.linearizer)
-    icodes.classes.valuesIterator.foreach((cls) => {
+    icodes.classes.values.foreach((cls) => {
       val suffix = if (cls.symbol hasFlag Flags.MODULE) "$.icode" else ".icode"
       var file = getFile(cls.symbol, suffix)
 //      if (file.exists())
diff --git a/src/compiler/scala/tools/nsc/Interpreter.scala b/src/compiler/scala/tools/nsc/Interpreter.scala
index 489a0161ba..cd9bbc6127 100644
--- a/src/compiler/scala/tools/nsc/Interpreter.scala
+++ b/src/compiler/scala/tools/nsc/Interpreter.scala
@@ -268,10 +268,10 @@ class Interpreter(val settings: Settings, out: PrintWriter) {
     // ))
   }
 
-  private def keyList[T](x: collection.Map[T, _]): List[T] = x.keysIterator.toList sortBy (_.toString)
+  private def keyList[T](x: collection.Map[T, _]): List[T] = x.keys.toList sortBy (_.toString)
   def allUsedNames = keyList(usedNameMap)
   def allBoundNames = keyList(boundNameMap)
-  def allSeenTypes = prevRequests.toList flatMap (_.typeOf.valuesIterator.toList) distinct
+  def allSeenTypes = prevRequests.toList flatMap (_.typeOf.values.toList) distinct
   def allValueGeneratingNames = allHandlers flatMap (_.generatesValue)
   def allImplicits = partialFlatMap(allHandlers) {
     case x: MemberHandler if x.definesImplicit => x.boundNames
diff --git a/src/compiler/scala/tools/nsc/PhaseAssembly.scala b/src/compiler/scala/tools/nsc/PhaseAssembly.scala
index 95ccee6b9d..dabf9547fe 100644
--- a/src/compiler/scala/tools/nsc/PhaseAssembly.scala
+++ b/src/compiler/scala/tools/nsc/PhaseAssembly.scala
@@ -96,21 +96,8 @@ trait PhaseAssembly { self: Global =>
     /* Given the entire graph, collect the phase objects at each level, where the phase
      * names are sorted alphabetical at each level, into the compiler phase list
      */
-    def compilerPhaseList(): List[SubComponent] = {
-      var chain: List[SubComponent] = Nil
-
-      var lvl = 1
-      var nds = nodes.valuesIterator.filter(_.level == lvl).toList
-      while(nds.size > 0) {
-        nds = nds sortBy (_.phasename)
-        for (n <- nds) {
-          chain = chain ::: n.phaseobj.get
-        }
-        lvl += 1
-        nds = nodes.valuesIterator.filter(_.level == lvl).toList
-      }
-      chain
-    }
+    def compilerPhaseList(): List[SubComponent] =
+      nodes.values.toList sortBy (x => (x.level, x.phasename)) flatMap (_.phaseobj) flatten
 
     /* Test if there are cycles in the graph, assign levels to the nodes
      * and collapse hard links into nodes
diff --git a/src/compiler/scala/tools/nsc/ast/parser/SymbolicXMLBuilder.scala b/src/compiler/scala/tools/nsc/ast/parser/SymbolicXMLBuilder.scala
index 31529f83e6..049c8aa2ff 100644
--- a/src/compiler/scala/tools/nsc/ast/parser/SymbolicXMLBuilder.scala
+++ b/src/compiler/scala/tools/nsc/ast/parser/SymbolicXMLBuilder.scala
@@ -191,7 +191,7 @@ abstract class SymbolicXMLBuilder(p: Parsers#Parser, preserveWS: Boolean)
 
     /** Extract all the namespaces from the attribute map. */
     val namespaces: List[Tree] =
-      for (z <- attrMap.keysIterator.toList ; if z startsWith xmlns) yield {
+      for (z <- attrMap.keys.toList ; if z startsWith xmlns) yield {
         val ns = splitPrefix(z) match {
           case (Some(_), rest)  => rest
           case _                => null
diff --git a/src/compiler/scala/tools/nsc/backend/icode/Checkers.scala b/src/compiler/scala/tools/nsc/backend/icode/Checkers.scala
index 63ccb12495..c3fadb1abf 100644
--- a/src/compiler/scala/tools/nsc/backend/icode/Checkers.scala
+++ b/src/compiler/scala/tools/nsc/backend/icode/Checkers.scala
@@ -74,7 +74,7 @@ abstract class Checkers {
     def checkICodes: Unit = {
       if (settings.verbose.value)
       println("[[consistency check at the beginning of phase " + globalPhase.name + "]]")
-      classes.valuesIterator foreach check
+      classes.values foreach check
     }
 
     def check(cls: IClass) {
diff --git a/src/compiler/scala/tools/nsc/backend/icode/ICodes.scala b/src/compiler/scala/tools/nsc/backend/icode/ICodes.scala
index b3285fa967..419edd8b83 100644
--- a/src/compiler/scala/tools/nsc/backend/icode/ICodes.scala
+++ b/src/compiler/scala/tools/nsc/backend/icode/ICodes.scala
@@ -59,7 +59,7 @@ abstract class ICodes extends AnyRef
     val printer = new TextPrinter(new PrintWriter(Console.out, true),
                                   new DumpLinearizer)
 
-    classes.valuesIterator foreach printer.printClass
+    classes.values foreach printer.printClass
   }
 
   object liveness extends Liveness {
diff --git a/src/compiler/scala/tools/nsc/backend/icode/Linearizers.scala b/src/compiler/scala/tools/nsc/backend/icode/Linearizers.scala
index 8ec4464dde..d090454129 100644
--- a/src/compiler/scala/tools/nsc/backend/icode/Linearizers.scala
+++ b/src/compiler/scala/tools/nsc/backend/icode/Linearizers.scala
@@ -238,7 +238,7 @@ trait Linearizers { self: ICodes =>
         covered.size + (hs :\ 0)((h, s) => h.blocks.length + s)
       }
 
-      val tryBlocks = handlersByCovered.keysIterator.toList sortBy size
+      val tryBlocks = handlersByCovered.keys.toList sortBy size
 
       var result = normalLinearizer.linearize(m)
 
diff --git a/src/compiler/scala/tools/nsc/backend/jvm/GenJVM.scala b/src/compiler/scala/tools/nsc/backend/jvm/GenJVM.scala
index 74100976fc..eccce2922e 100644
--- a/src/compiler/scala/tools/nsc/backend/jvm/GenJVM.scala
+++ b/src/compiler/scala/tools/nsc/backend/jvm/GenJVM.scala
@@ -49,7 +49,7 @@ abstract class GenJVM extends SubComponent {
         for ((sym, cls) <- icodes.classes ; if inliner.isClosureClass(sym) && !deadCode.liveClosures(sym))
           icodes.classes -= sym
 
-      classes.valuesIterator foreach apply
+      classes.values foreach apply
     }
 
     override def apply(cls: IClass) {
diff --git a/src/compiler/scala/tools/nsc/backend/msil/GenMSIL.scala b/src/compiler/scala/tools/nsc/backend/msil/GenMSIL.scala
index 2bf4f51806..f4cf5a7089 100644
--- a/src/compiler/scala/tools/nsc/backend/msil/GenMSIL.scala
+++ b/src/compiler/scala/tools/nsc/backend/msil/GenMSIL.scala
@@ -42,15 +42,15 @@ abstract class GenMSIL extends SubComponent {
       val codeGenerator = new BytecodeGenerator
 
       //classes is ICodes.classes, a HashMap[Symbol, IClass]
-      classes.valuesIterator foreach codeGenerator.findEntryPoint
+      classes.values foreach codeGenerator.findEntryPoint
 
       codeGenerator.initAssembly
 
-      classes.valuesIterator foreach codeGenerator.createTypeBuilder
-      classes.valuesIterator foreach codeGenerator.createClassMembers
+      classes.values foreach codeGenerator.createTypeBuilder
+      classes.values foreach codeGenerator.createClassMembers
 
       try {
-        classes.valuesIterator foreach codeGenerator.genClass
+        classes.values foreach codeGenerator.genClass
       } finally {
         codeGenerator.writeAssembly
       }
@@ -469,7 +469,7 @@ abstract class GenMSIL extends SubComponent {
     }
 
     private def createTypes() {
-      for (sym <- classes.keysIterator) {
+      for (sym <- classes.keys) {
         val iclass   = classes(sym)
         val tBuilder = types(sym).asInstanceOf[TypeBuilder]
 
@@ -755,7 +755,7 @@ abstract class GenMSIL extends SubComponent {
         val newHandlersBySize = newHandlers.groupBy(_.covered.size)
         // big handlers first, smaller ones are nested inside the try of the big one
         // (checked by the assertions below)
-        val sizes = newHandlersBySize.keysIterator.toList.sortWith(_ > _)
+        val sizes = newHandlersBySize.keys.toList.sortWith(_ > _)
 
         val beginHandlers = new ListBuffer[ExceptionHandler]
         for (s <- sizes) {
diff --git a/src/compiler/scala/tools/nsc/doc/model/comment/CommentFactory.scala b/src/compiler/scala/tools/nsc/doc/model/comment/CommentFactory.scala
index a04e98ddc5..e39942e029 100644
--- a/src/compiler/scala/tools/nsc/doc/model/comment/CommentFactory.scala
+++ b/src/compiler/scala/tools/nsc/doc/model/comment/CommentFactory.scala
@@ -143,7 +143,7 @@ final class CommentFactory(val reporter: Reporter) { parser =>
       case Nil =>
 
         val bodyTags: mutable.Map[TagKey, List[Body]] =
-          mutable.Map((tags map { case (key, values) => key -> (values map (parseWiki(_, pos))) }).toSeq:_*)
+          mutable.Map(tags mapValues (_ map (parseWiki(_, pos))) toSeq: _*)
 
         def oneTag(key: SimpleTagKey): Option[Body] =
           ((bodyTags remove key): @unchecked) match {
@@ -158,7 +158,7 @@ final class CommentFactory(val reporter: Reporter) { parser =>
 
         def allSymsOneTag(key: TagKey): Map[String, Body] = {
           val keys: Seq[SymbolTagKey] =
-            bodyTags.keysIterator.toSeq flatMap {
+            bodyTags.keys.toSeq flatMap {
               case stk: SymbolTagKey if (stk.name == key.name) => Some(stk)
               case stk: SimpleTagKey if (stk.name == key.name) =>
                 reporter.warning(pos, "Tag '@" + stk.name + "' must be followed by a symbol name")
diff --git a/src/compiler/scala/tools/nsc/interactive/Global.scala b/src/compiler/scala/tools/nsc/interactive/Global.scala
index c4fec12667..73b728d646 100644
--- a/src/compiler/scala/tools/nsc/interactive/Global.scala
+++ b/src/compiler/scala/tools/nsc/interactive/Global.scala
@@ -222,7 +222,7 @@ self =>
     reporter.reset
     firsts = firsts filter (s => unitOfFile contains (s.file))
     val prefix = firsts map unitOf
-    val units = prefix ::: (unitOfFile.valuesIterator.toList diff prefix) filter (!_.isUpToDate)
+    val units = prefix ::: (unitOfFile.values.toList diff prefix) filter (!_.isUpToDate)
     recompile(units)
     if (debugIDE) inform("Everything is now up to date")
   }
@@ -387,7 +387,7 @@ self =>
         addScopeMember(sym, pre, imp.qual)
       }
     }
-    val result = locals.valuesIterator.toList
+    val result = locals.values.toList
     if (debugIDE) for (m <- result) println(m)
     result
   }
@@ -455,7 +455,7 @@ self =>
         addTypeMember(sym, vpre, false, view.tree.symbol)
       }
     }
-    members.valuesIterator.toList
+    members.values.toList
   }
 
   // ---------------- Helper classes ---------------------------
diff --git a/src/compiler/scala/tools/nsc/interpreter/CompletionAware.scala b/src/compiler/scala/tools/nsc/interpreter/CompletionAware.scala
index bdb390c9fc..7e94b687bf 100644
--- a/src/compiler/scala/tools/nsc/interpreter/CompletionAware.scala
+++ b/src/compiler/scala/tools/nsc/interpreter/CompletionAware.scala
@@ -108,6 +108,6 @@ object CompletionAware {
    */
   def apply(terms: () => List[String]): CompletionAware = apply(terms, _ => None)
   def apply(map: collection.Map[String, CompletionAware]): CompletionAware =
-    apply(() => map.keysIterator.toList, map.get _)
+    apply(() => map.keys.toList, map.get _)
 }
 
diff --git a/src/compiler/scala/tools/nsc/io/VirtualDirectory.scala b/src/compiler/scala/tools/nsc/io/VirtualDirectory.scala
index f7e6832109..b4b1eca439 100644
--- a/src/compiler/scala/tools/nsc/io/VirtualDirectory.scala
+++ b/src/compiler/scala/tools/nsc/io/VirtualDirectory.scala
@@ -45,7 +45,7 @@ extends AbstractFile {
 
   // the toList is so that the directory may continue to be
   // modified while its elements are iterated
-  def iterator = files.valuesIterator.toList.iterator
+  def iterator = files.values.toList.iterator
 
   override def lookupName(name: String, directory: Boolean): AbstractFile =
     files get name filter (_.isDirectory == directory) orNull
diff --git a/src/compiler/scala/tools/nsc/symtab/Symbols.scala b/src/compiler/scala/tools/nsc/symtab/Symbols.scala
index dfdab4189f..e92fd28026 100644
--- a/src/compiler/scala/tools/nsc/symtab/Symbols.scala
+++ b/src/compiler/scala/tools/nsc/symtab/Symbols.scala
@@ -456,7 +456,16 @@ trait Symbols extends reflect.generic.Symbols { self: SymbolTable =>
 
     def isDeprecated        = hasAnnotation(DeprecatedAttr)
     def deprecationMessage  = getAnnotationArg(DeprecatedAttr, 0) partialMap { case Literal(const) => const.stringValue }
-    def migrationMessage    = getAnnotationArg(MigrationAnnotationClass, 2) partialMap { case Literal(const) => const.stringValue }
+    // !!! when annotation arguments are not literal strings, but any sort of
+    // assembly of strings, there is a fair chance they will turn up here not as
+    // Literal(const) but some arbitrary AST.  However nothing in the compiler
+    // prevents someone from writing a @migration annotation with a calculated
+    // string.  So this needs attention.  For now the fact that migration is
+    // private[scala] ought to provide enough protection.
+    def migrationMessage    = getAnnotationArg(MigrationAnnotationClass, 2) partialMap {
+      case Literal(const) => const.stringValue
+      case x              => x.toString           // should not be necessary, but better than silently ignoring an issue
+    }
     def elisionLevel        = getAnnotationArg(ElidableMethodClass, 0) partialMap { case Literal(Constant(x: Int)) => x }
 
     /** Does this symbol denote a wrapper object of the interpreter or its class? */
diff --git a/src/compiler/scala/tools/nsc/symtab/Types.scala b/src/compiler/scala/tools/nsc/symtab/Types.scala
index 3db5be2e4e..9ca344070b 100644
--- a/src/compiler/scala/tools/nsc/symtab/Types.scala
+++ b/src/compiler/scala/tools/nsc/symtab/Types.scala
@@ -3366,7 +3366,7 @@ A type's typeSymbol should never be inspected directly.
     override val dropNonConstraintAnnotations = true
 
     private var existSyms = immutable.Map.empty[Int, Symbol]
-    def existentialsNeeded: List[Symbol] = existSyms.valuesIterator.toList
+    def existentialsNeeded: List[Symbol] = existSyms.values.toList
 
     /* Return the type symbol for referencing a parameter index
      * inside the existential quantifier.  */
diff --git a/src/compiler/scala/tools/nsc/symtab/classfile/ClassfileParser.scala b/src/compiler/scala/tools/nsc/symtab/classfile/ClassfileParser.scala
index efc899743d..e8b2b84653 100644
--- a/src/compiler/scala/tools/nsc/symtab/classfile/ClassfileParser.scala
+++ b/src/compiler/scala/tools/nsc/symtab/classfile/ClassfileParser.scala
@@ -952,7 +952,7 @@ abstract class ClassfileParser {
       }
     }
 
-    for (entry <- innerClasses.valuesIterator) {
+    for (entry <- innerClasses.values) {
       // create a new class member for immediate inner classes
       if (entry.outerName == externalName) {
         val file = global.classPath.findSourceFile(entry.externalName.toString) getOrElse {
diff --git a/src/compiler/scala/tools/nsc/transform/LambdaLift.scala b/src/compiler/scala/tools/nsc/transform/LambdaLift.scala
index bd86ae42cc..179cf8211a 100644
--- a/src/compiler/scala/tools/nsc/transform/LambdaLift.scala
+++ b/src/compiler/scala/tools/nsc/transform/LambdaLift.scala
@@ -428,7 +428,7 @@ abstract class LambdaLift extends InfoTransform {
     override def transformUnit(unit: CompilationUnit) {
       computeFreeVars
       atPhase(phase.next)(super.transformUnit(unit))
-      assert(liftedDefs.size == 0, liftedDefs.keysIterator.toList)
+      assert(liftedDefs.size == 0, liftedDefs.keys.toList)
     }
   } // class LambdaLifter
 
diff --git a/src/compiler/scala/tools/nsc/transform/SpecializeTypes.scala b/src/compiler/scala/tools/nsc/transform/SpecializeTypes.scala
index 1ad045d367..dacc0cf506 100644
--- a/src/compiler/scala/tools/nsc/transform/SpecializeTypes.scala
+++ b/src/compiler/scala/tools/nsc/transform/SpecializeTypes.scala
@@ -278,7 +278,7 @@ abstract class SpecializeTypes extends InfoTransform with TypingTransformers {
             tpes
           case _ =>
             log(sym + " specialized on everything")
-            primitiveTypes.valuesIterator.toList
+            primitiveTypes.values.toList
         }
       case _ =>
         Nil
@@ -581,8 +581,8 @@ abstract class SpecializeTypes extends InfoTransform with TypingTransformers {
     if (sym.isMethod && !sym.info.typeParams.isEmpty) {
       val (stps, tps) = splitParams(sym.info.typeParams)
       val res = sym :: (for (env <- specializations(stps) if needsSpecialization(env, sym)) yield {
-        val keys = env.keysIterator.toList;
-        val vals = env.valuesIterator.toList
+        val keys = env.keys.toList;
+        val vals = env.values.toList
         val specMember =  sym.cloneSymbol(owner).setFlag(SPECIALIZED).resetFlag(DEFERRED)
         specMember.name = specializedName(sym, env)
 
@@ -778,8 +778,8 @@ abstract class SpecializeTypes extends InfoTransform with TypingTransformers {
     }
     // disabled because of bugs in std. collections
     //val (keys, values) = env.iterator.toList.unzip
-    val keys = env.keysIterator.toList
-    val values = env.valuesIterator.toList
+    val keys = env.keys.toList
+    val values = env.values.toList
     (new FullTypeMap(keys, values))(tpe)
 //    tpe.subst(keys, values)
   }
diff --git a/src/compiler/scala/tools/nsc/typechecker/Duplicators.scala b/src/compiler/scala/tools/nsc/typechecker/Duplicators.scala
index 7ca2ff81bb..3ab0b9bb94 100644
--- a/src/compiler/scala/tools/nsc/typechecker/Duplicators.scala
+++ b/src/compiler/scala/tools/nsc/typechecker/Duplicators.scala
@@ -27,7 +27,7 @@ abstract class Duplicators extends Analyzer {
       newClassOwner = newThis
     } else resetClassOwners
 
-    envSubstitution = new SubstSkolemsTypeMap(env.keysIterator.toList, env.valuesIterator.toList)
+    envSubstitution = new SubstSkolemsTypeMap(env.keys.toList, env.values.toList)
     log("retyped with env: " + env)
     (new BodyDuplicator(context)).typed(tree)
   }
diff --git a/src/compiler/scala/tools/nsc/typechecker/Implicits.scala b/src/compiler/scala/tools/nsc/typechecker/Implicits.scala
index a59792c4e1..ab933227fb 100644
--- a/src/compiler/scala/tools/nsc/typechecker/Implicits.scala
+++ b/src/compiler/scala/tools/nsc/typechecker/Implicits.scala
@@ -913,7 +913,7 @@ self: Analyzer =>
     def allImplicits: List[SearchResult] = {
       val invalidImplicits = new ListBuffer[Symbol]
       def search(iss: List[List[ImplicitInfo]], isLocal: Boolean) =
-        applicableInfos(iss, isLocal, invalidImplicits).valuesIterator.toList
+        applicableInfos(iss, isLocal, invalidImplicits).values.toList
       search(context.implicitss, true) ::: search(implicitsOfExpectedType, false)
     }
   }
diff --git a/src/compiler/scala/tools/nsc/typechecker/RefChecks.scala b/src/compiler/scala/tools/nsc/typechecker/RefChecks.scala
index 7e7adb12be..be2109c9f8 100644
--- a/src/compiler/scala/tools/nsc/typechecker/RefChecks.scala
+++ b/src/compiler/scala/tools/nsc/typechecker/RefChecks.scala
@@ -929,9 +929,10 @@ abstract class RefChecks extends InfoTransform {
     /** Similar to deprecation: check if the symbol is marked with @migration
      *  indicating it has changed semantics between versions.
      */
-    private def checkMigration(sym: Symbol, pos: Position) =
+    private def checkMigration(sym: Symbol, pos: Position) = {
       for (msg <- sym.migrationMessage)
-        unit.warning(pos, "%s%s has changed semantics:\n %s".format(sym, sym.locationString, msg))
+        unit.warning(pos, "%s%s has changed semantics:\n%s".format(sym, sym.locationString, msg))
+    }
 
     /** Check that a deprecated val or def does not override a
       * concrete, non-deprecated method.  If it does, then
diff --git a/src/compiler/scala/tools/nsc/util/Statistics.scala b/src/compiler/scala/tools/nsc/util/Statistics.scala
index 37f70ddbaf..5aee76d946 100644
--- a/src/compiler/scala/tools/nsc/util/Statistics.scala
+++ b/src/compiler/scala/tools/nsc/util/Statistics.scala
@@ -206,7 +206,7 @@ abstract class Statistics {
     if (phase.name != "parser") {
       val counts = new ClassCounts
       for (u <- currentRun.units; t <- u.body) counts(t.getClass) += 1
-      inform("#retained nodes          : " + counts.valuesIterable.sum)
+      inform("#retained nodes          : " + counts.values.sum)
       inform("#retained nodes by type  : " + showCounts(counts))
       inform("#typechecked identifiers : " + typedIdentCount)
       inform("#typechecked selections  : " + typedSelectCount)