A new and better fix for ticket #1373.

6 files changed, 73 insertions, 35 deletions

diff --git a/src/compiler/scala/tools/nsc/ast/Trees.scala b/src/compiler/scala/tools/nsc/ast/Trees.scala
index 7da71eaed6..3337366269 100644
--- a/src/compiler/scala/tools/nsc/ast/Trees.scala
+++ b/src/compiler/scala/tools/nsc/ast/Trees.scala
@@ -64,6 +64,7 @@ trait Trees {
     def isAbstract  = hasFlag(ABSTRACT )
     def isDeferred  = hasFlag(DEFERRED )
     def isCase      = hasFlag(CASE     )
+    def isLazy      = hasFlag(LAZY     )
     def isSealed    = hasFlag(SEALED   )
     def isFinal     = hasFlag(FINAL    )
     def isTrait     = hasFlag(TRAIT | notDEFERRED) // (part of DEVIRTUALIZE)
diff --git a/src/compiler/scala/tools/nsc/symtab/Symbols.scala b/src/compiler/scala/tools/nsc/symtab/Symbols.scala
index 0003a50823..94d1764d8c 100644
--- a/src/compiler/scala/tools/nsc/symtab/Symbols.scala
+++ b/src/compiler/scala/tools/nsc/symtab/Symbols.scala
@@ -1078,10 +1078,38 @@ trait Symbols {
      */
     def thisType: Type = NoPrefix
 
-    /** Return every accessor of a primary constructor parameter in this case class
+    /** Return every accessor of a primary constructor parameter in this case class.
+     *  The scope declarations may be out of order because fields with less than private
+     *  access are first given a regular getter, then a new renamed getter which comes
+     *  later in the declaration list.  For this reason we have to pinpoint the
+     *  right accessors by starting with the original fields (which will be in the right
+     *  order) and looking for getters with applicable names.  The getters may have the
+     *  standard name "foo" or may have been renamed to "foo$\d+" in SyntheticMethods.
+     *  See ticket #1373.
      */
-    final def caseFieldAccessors: List[Symbol] =
-      info.decls.toList filter (sym => !(sym hasFlag PRIVATE) && sym.hasFlag(CASEACCESSOR))
+    final def caseFieldAccessors: List[Symbol] = {
+      val allWithFlag = info.decls.toList filter (_ hasFlag CASEACCESSOR)
+      val (accessors, fields) = allWithFlag partition (_.isMethod)
+
+      def findAccessor(field: Symbol): Symbol = {
+        // There is another renaming the field may have undergone, for instance as in
+        // ticket #2175: case class Property[T](private var t: T), t becomes Property$$t.
+        // So we use the original name everywhere.
+        val getterName    = nme.getterName(field.originalName)
+
+        // Note this is done in two passes intentionally, to ensure we pick up the original
+        // getter if present before looking for the renamed getter.
+        def origGetter    = accessors find (_.originalName == getterName)
+        def renamedGetter = accessors find (_.originalName startsWith (getterName + "$"))
+        val accessorName  = origGetter orElse renamedGetter
+
+        accessorName getOrElse {
+          throw new Error("Could not find case accessor for %s in %s".format(field, this))
+        }
+      }
+
+      fields map findAccessor
+    }
 
     final def constrParamAccessors: List[Symbol] =
       info.decls.toList filter (sym => !sym.isMethod && sym.hasFlag(PARAMACCESSOR))
@@ -1117,7 +1145,7 @@ trait Symbols {
     def defaultGetter_=(getter: Symbol): Unit =
       throw new Error("defaultGetter cannot be set for " + this)
 
-    /** For a lazy value, it's lazy accessor. NoSymbol for all others */
+    /** For a lazy value, its lazy accessor. NoSymbol for all others */
     def lazyAccessor: Symbol = NoSymbol
 
     /** For an outer accessor: The class from which the outer originates.
@@ -1586,15 +1614,17 @@ trait Symbols {
       clone
     }
 
+    private val validAliasFlags = SUPERACCESSOR | PARAMACCESSOR | MIXEDIN | SPECIALIZED
+
     override def alias: Symbol =
-      if (hasFlag(SUPERACCESSOR | PARAMACCESSOR | MIXEDIN | SPECIALIZED)) initialize.referenced
+      if (hasFlag(validAliasFlags)) initialize.referenced
       else NoSymbol
 
     def setAlias(alias: Symbol): TermSymbol = {
       assert(alias != NoSymbol, this)
       assert(!(alias hasFlag OVERLOADED), alias)
 
-      assert(hasFlag(SUPERACCESSOR | PARAMACCESSOR | MIXEDIN | SPECIALIZED), this)
+      assert(hasFlag(validAliasFlags), this)
       referenced = alias
       this
     }
diff --git a/src/compiler/scala/tools/nsc/transform/SpecializeTypes.scala b/src/compiler/scala/tools/nsc/transform/SpecializeTypes.scala
index 2ee768b592..aeb54ea02d 100644
--- a/src/compiler/scala/tools/nsc/transform/SpecializeTypes.scala
+++ b/src/compiler/scala/tools/nsc/transform/SpecializeTypes.scala
@@ -430,11 +430,9 @@ abstract class SpecializeTypes extends InfoTransform with TypingTransformers {
         log(" * looking at: " + m)
         if (!m.isDeferred) concreteSpecMethods += m
 
-        // specialized members have to be overridable. Fields should not
-        // have the field CASEACCESSOR (messes up patmatch)
-        if (m.hasFlag(PRIVATE)) {
-          m.resetFlag(PRIVATE | CASEACCESSOR).setFlag(PROTECTED)
-        }
+        // specialized members have to be overridable.
+        if (m.hasFlag(PRIVATE))
+          m.resetFlag(PRIVATE).setFlag(PROTECTED)
 
         if (m.isConstructor) {
           val specCtor = enterMember(m.cloneSymbol(cls).setFlag(SPECIALIZED))
diff --git a/src/compiler/scala/tools/nsc/typechecker/Namers.scala b/src/compiler/scala/tools/nsc/typechecker/Namers.scala
index f31394407e..91b0facd92 100644
--- a/src/compiler/scala/tools/nsc/typechecker/Namers.scala
+++ b/src/compiler/scala/tools/nsc/typechecker/Namers.scala
@@ -127,13 +127,13 @@ trait Namers { self: Analyzer =>
         unsafeTypeParams foreach(sym => paramContext.scope.enter(sym))
         newNamer(paramContext)
       }
-      if (sym.isTerm) {
-        if (sym.hasFlag(PARAM) && sym.owner.isPrimaryConstructor)
-          primaryConstructorParamNamer
-        else if (sym.hasFlag(PARAMACCESSOR))
-          primaryConstructorParamNamer
-        else innerNamer
-      } else innerNamer
+      def usePrimary = sym.isTerm && (
+        (sym hasFlag PARAMACCESSOR) ||
+        ((sym hasFlag PARAM) && sym.owner.isPrimaryConstructor)
+      )
+
+      if (usePrimary) primaryConstructorParamNamer
+      else innerNamer
     }
 
     protected def conflict(newS : Symbol, oldS : Symbol) : Boolean = {
@@ -339,7 +339,7 @@ trait Namers { self: Analyzer =>
           case tree @ ClassDef(mods, name, tparams, impl) =>
             tree.symbol = enterClassSymbol(tree)
             finishWith(tparams)
-            if ((mods.flags & CASE) != 0L) {
+            if (mods.isCase) {
               val m = ensureCompanionObject(tree, caseModuleDef(tree))
               caseClassOfModuleClass(m.moduleClass) = tree
             }
@@ -368,7 +368,7 @@ trait Namers { self: Analyzer =>
                  (mods.flags & (PRIVATE | LOCAL)) == (PRIVATE | LOCAL).toLong ||
                  name.endsWith(nme.OUTER, nme.OUTER.length) ||
                  context.unit.isJava) &&
-                (mods.flags & LAZY) == 0L) {
+                 !mods.isLazy) {
               tree.symbol = enterInScope(owner.newValue(tree.pos, name)
                 .setFlag(mods.flags))
               finish
@@ -389,22 +389,24 @@ trait Namers { self: Analyzer =>
                 setInfo(setter)(namerOf(setter).setterTypeCompleter(vd))
               }
               tree.symbol =
-                if (mods.hasFlag(DEFERRED)) {
+                if (mods.isDeferred) {
                   getter setPos tree.pos // unfocus getter position, because there won't be a separate value
                 } else {
                   var vsym =
                     if (!context.owner.isClass) {
-                      assert((mods.flags & LAZY) != 0L) // if not a field, it has to be a lazy val
+                      assert(mods.isLazy)   // if not a field, it has to be a lazy val
                       owner.newValue(tree.pos, name + "$lzy" ).setFlag(mods.flags | MUTABLE)
                     } else {
-                      owner.newValue(tree.pos, nme.getterToLocal(name))
-                        .setFlag(mods.flags & FieldFlags | PRIVATE | LOCAL |
-                                 (if (mods.hasFlag(LAZY)) MUTABLE else 0))
+                      val mflag = if (mods.isLazy) MUTABLE else 0
+                      val newflags = mods.flags & FieldFlags | PRIVATE | LOCAL | mflag
+
+                      owner.newValue(tree.pos, nme.getterToLocal(name)) setFlag newflags
                     }
                   enterInScope(vsym)
                   setInfo(vsym)(namerOf(vsym).typeCompleter(tree))
-                  if ((mods.flags & LAZY) != 0L)
+                  if (mods.isLazy)
                     vsym.setLazyAccessor(getter)
+
                   vsym
                 }
               addBeanGetterSetter(vd, getter)
@@ -472,7 +474,7 @@ trait Namers { self: Analyzer =>
         if (!name(0).isLetter)
           context.error(vd.pos, "`BeanProperty' annotation can be applied "+
                                 "only to fields that start with a letter")
-        else if (mods hasFlag PRIVATE)
+        else if (mods.isPrivate)
           // avoids name clashes with private fields in traits
           context.error(vd.pos, "`BeanProperty' annotation can only be applied "+
                                 "to non-private fields")
@@ -485,7 +487,7 @@ trait Namers { self: Analyzer =>
           val getterMods = Modifiers(flags, mods.privateWithin, Nil, mods.positions)
           val beanGetterDef = atPos(vd.pos.focus) {
             DefDef(getterMods, getterName, Nil, List(Nil), tpt.duplicate,
-                   if (mods hasFlag DEFERRED) EmptyTree
+                   if (mods.isDeferred) EmptyTree
                    else Select(This(getter.owner.name), name)) }
           enterSyntheticSym(beanGetterDef)
 
diff --git a/src/compiler/scala/tools/nsc/typechecker/SyntheticMethods.scala b/src/compiler/scala/tools/nsc/typechecker/SyntheticMethods.scala
index a1c1a7b661..4fa66a242b 100644
--- a/src/compiler/scala/tools/nsc/typechecker/SyntheticMethods.scala
+++ b/src/compiler/scala/tools/nsc/typechecker/SyntheticMethods.scala
@@ -204,8 +204,6 @@ trait SyntheticMethods extends ast.TreeDSL {
 
     def hasSerializableAnnotation(clazz: Symbol) =
       clazz hasAnnotation SerializableAttr
-    def isPublic(sym: Symbol) =
-      !sym.hasFlag(PRIVATE | PROTECTED) && sym.privateWithin == NoSymbol
 
     def readResolveMethod: Tree = {
       // !!! the synthetic method "readResolve" should be private, but then it is renamed !!!
@@ -233,11 +231,14 @@ trait SyntheticMethods extends ast.TreeDSL {
         clazz addAnnotation AnnotationInfo(SerializableAttr.tpe, Nil, Nil)
 
         if (isTop) {
-          for (stat <- templ.body) {
-            if (stat.isDef && stat.symbol.isMethod && stat.symbol.hasFlag(CASEACCESSOR) && !isPublic(stat.symbol)) {
-              ts += newAccessorMethod(stat)
-              stat.symbol resetFlag CASEACCESSOR
-            }
+          // If this case class has fields with less than public visibility, their getter at this
+          // point also has those permissions.  In that case we create a new, public accessor method
+          // with a new name and remove the CASEACCESSOR flag from the existing getter.  This complicates
+          // the retrieval of the case field accessors (see def caseFieldAccessors in Symbols.)
+          def needsService(s: Symbol) = s.isMethod && (s hasFlag CASEACCESSOR) && !s.isPublic
+          for (stat <- templ.body ; if stat.isDef && needsService(stat.symbol)) {
+            ts += newAccessorMethod(stat)
+            stat.symbol resetFlag CASEACCESSOR
           }
         }
 
diff --git a/test/files/run/bug1373.scala b/test/files/run/bug1373.scala
new file mode 100644
index 0000000000..537421c788
--- /dev/null
+++ b/test/files/run/bug1373.scala
@@ -0,0 +1,6 @@
+// Testing whether case class params come back in the right order.
+object Test extends Application {
+  case class Foo(private val a: String, b: String, private val c: String, d: String, private val e: String)
+  val x = Foo("a", "b", "c", "d", "e")
+  assert(x.toString == """Foo(a,b,c,d,e)""")
+}
\ No newline at end of file