Merge pull request #5616 from youroff/master

Better error for StaticFieldShouldPrecedeNonStatic rule

Author: nicolasstucki

compiler/src/dotty/tools/dotc/reporting/diagnostic/ErrorMessageID.java

@@ -142,7 +142,8 @@ public enum ErrorMessageID {
     StaticOverridingNonStaticMembersID,
     OverloadInRefinementID,
     NoMatchingOverloadID,
-    StableIdentPatternID
+    StableIdentPatternID,
+    StaticFieldsShouldPrecedeNonStaticID
     ;
 
     public int errorNumber() {

compiler/src/dotty/tools/dotc/reporting/diagnostic/messages.scala

@@ -1936,6 +1936,28 @@ object messages {
       hl"${"@static"} members are only allowed inside objects."
   }
 
+  case class StaticFieldsShouldPrecedeNonStatic(member: Symbol, defns: List[tpd.Tree])(implicit ctx: Context) extends Message(StaticFieldsShouldPrecedeNonStaticID) {
+    val msg: String = hl"${"@static"} $member in ${member.owner} must be defined before non-static fields."
+    val kind: String = "Syntax"
+
+    val explanation: String = {
+      val nonStatics = defns.takeWhile(_.symbol != member).take(3).filter(_.isInstanceOf[tpd.ValDef])
+      val codeExample = s"""object ${member.owner.name.firstPart} {
+                        |  @static ${member} = ...
+                        |  ${nonStatics.map(m => s"${m.symbol} = ...").mkString("\n  ")}
+                        |  ...
+                        |}"""
+      hl"""The fields annotated with @static should precede any non @static fields.
+        |This ensures that we do not introduce surprises for users in initialization order of this class.
+        |Static field are initialized when class loading the code of Foo.
+        |Non static fields are only initialized the first  time that Foo is accessed.
+        |
+        |The definition of ${member.name} should have been before the non ${"@static val"}s:
+        |$codeExample
+        |"""
+    }
+  }
+
   case class CyclicInheritance(symbol: Symbol, addendum: String)(implicit ctx: Context) extends Message(CyclicInheritanceID) {
     val kind: String = "Syntax"
     val msg: String = hl"Cyclic inheritance: $symbol extends itself$addendum"

compiler/src/dotty/tools/dotc/transform/CheckStatic.scala

@@ -37,7 +37,7 @@ class CheckStatic extends MiniPhase {
         }
 
         if (defn.isInstanceOf[ValDef] && hadNonStaticField) {
-          ctx.error("@static fields should precede non-static ones", defn.pos)
+          ctx.error(StaticFieldsShouldPrecedeNonStatic(defn.symbol, defns), defn.pos)
         }
 
         val companion = ctx.owner.companionClass

compiler/test/dotty/tools/dotc/reporting/ErrorMessagesTests.scala

@@ -1393,6 +1393,21 @@ class ErrorMessagesTests extends ErrorMessagesTest {
       assertEquals(field.show, "method bar")
     }
 
+  @Test def staticShouldPrecedeNonStatic =
+    checkMessagesAfter(CheckStatic.name) {
+      """
+        |class Foo
+        |object Foo {
+        |  val foo = 1
+        |  @annotation.static val bar = 2
+        |}
+      """.stripMargin
+    }.expect { (ictx, messages) =>
+      implicit val ctx: Context = ictx
+      val StaticFieldsShouldPrecedeNonStatic(field, _) = messages.head
+      assertEquals(field.show, "value bar")
+    }
+
   @Test def cyclicInheritance =
     checkMessagesAfter(FrontEnd.name) {
       "class A extends A"