Skip to content

Better error for StaticFieldShouldPrecedeNonStatic rule #5616

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 1 commit into from
Dec 15, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,8 @@ public enum ErrorMessageID {
StaticOverridingNonStaticMembersID,
OverloadInRefinementID,
NoMatchingOverloadID,
StableIdentPatternID
StableIdentPatternID,
StaticFieldsShouldPrecedeNonStaticID
;

public int errorNumber() {
Expand Down
22 changes: 22 additions & 0 deletions compiler/src/dotty/tools/dotc/reporting/diagnostic/messages.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
2 changes: 1 addition & 1 deletion compiler/src/dotty/tools/dotc/transform/CheckStatic.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
15 changes: 15 additions & 0 deletions compiler/test/dotty/tools/dotc/reporting/ErrorMessagesTests.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down