From 7de0056b0d478d26317affedce9e9f6244464a9d Mon Sep 17 00:00:00 2001 From: Johannes Rudolph Date: Wed, 25 Apr 2012 14:39:30 +0200 Subject: [PATCH 1/2] warn if a companion object of a case class is referenced in a stand-alone fashion This happens easily when writing actor based code where you often have a set of messages defined like this: case class Start(arg: Int) case object Stop If you now use `Start` without the argument in pattern matches (like in `receive`) or message sends (`self ! Start`) your code may fail silently because you are sending the companion object around which is rarely what you want to do. --- src/main/scala/LinterPlugin.scala | 40 +++++++++++++++++++++++++++ src/test/scala/LinterPluginTest.scala | 28 +++++++++++++++++++ 2 files changed, 68 insertions(+) diff --git a/src/main/scala/LinterPlugin.scala b/src/main/scala/LinterPlugin.scala index ec73b4c..5053200 100644 --- a/src/main/scala/LinterPlugin.scala +++ b/src/main/scala/LinterPlugin.scala @@ -66,6 +66,8 @@ class LinterPlugin(val global: Global) extends Plugin { selector.name == nme.WILDCARD && selector.renamePos == -1 } + var moduleStack: List[Symbol] = Nil + override def traverse(tree: Tree): Unit = tree match { case Apply(eqeq @ Select(lhs, nme.EQ), List(rhs)) if methodImplements(eqeq.symbol, Object_==) && !(isSubtype(lhs, rhs) || isSubtype(rhs, lhs)) => @@ -86,9 +88,47 @@ class LinterPlugin(val global: Global) extends Plugin { unit.warning(get.pos, "Calling .get on Option will throw an exception if the Option is None.") } + // keep track of the module definition we are inside of + case m: ModuleDef => + val oldStack = moduleStack + + moduleStack ::= m.symbol + m.children.foreach(super.traverse) + moduleStack = oldStack + + case _: Ident => + warnCompanionClassUsage(tree) + + case sel: Select => + if (!warnCompanionClassUsage(tree)) + super.traverse(sel.qualifier) + case _ => super.traverse(tree) } + + def warnCompanionClassUsage(tree: Tree): Boolean = { + val sym = tree.symbol + + val condition = + sym.isModule && + sym.companionClass.isCaseClass && + + // there's a spurious auto-generated reference inside of the + // companion object itself in its `readResolve`, so we + // don't report inside of the companion object itself + !moduleStack.contains(sym) + + if (condition) + unit.warning(tree.pos, Warnings.SuspiciousCompanionObjectUse) + + condition + } + } } + + object Warnings { + val SuspiciousCompanionObjectUse = "Using the companion object of a case class here is probably not what you intended. You probably meant some instance of the case class instead." + } } diff --git a/src/test/scala/LinterPluginTest.scala b/src/test/scala/LinterPluginTest.scala index 20b0a00..d1ce075 100644 --- a/src/test/scala/LinterPluginTest.scala +++ b/src/test/scala/LinterPluginTest.scala @@ -141,4 +141,32 @@ class LinterPluginTest extends SpecsMatchers { x == "bar" }""") } + + @Test + def testCompanionObjectUsages(): Unit = { + val msg = Some("Using the companion object of a case class here is probably not what you intended. You probably meant some instance of the case class instead.") + + // should warn + check("""(Some(5): Any) match { + case Some => 12 + case None => 25 + } + """, msg) + + check("Some", msg) + + check("""object Test { + case class Test2(i: Int) + Test2 + }""", msg) + + // should compile + check("""(Some(5): Any) match { + case Some(5) => 12 + case None => 25 + } + """) + + check("""object Test { case class Test3(i: Int) }""") + } } From c77dae6755d178290e3b20e300c2b651ef67633d Mon Sep 17 00:00:00 2001 From: Johannes Rudolph Date: Wed, 25 Apr 2012 14:42:34 +0200 Subject: [PATCH 2/2] fix stupid syntax error --- src/main/scala/LinterPlugin.scala | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/main/scala/LinterPlugin.scala b/src/main/scala/LinterPlugin.scala index 5053200..11c65f7 100644 --- a/src/main/scala/LinterPlugin.scala +++ b/src/main/scala/LinterPlugin.scala @@ -110,14 +110,14 @@ class LinterPlugin(val global: Global) extends Plugin { def warnCompanionClassUsage(tree: Tree): Boolean = { val sym = tree.symbol - val condition = + val condition = ( sym.isModule && sym.companionClass.isCaseClass && // there's a spurious auto-generated reference inside of the // companion object itself in its `readResolve`, so we // don't report inside of the companion object itself - !moduleStack.contains(sym) + !moduleStack.contains(sym)) if (condition) unit.warning(tree.pos, Warnings.SuspiciousCompanionObjectUse)