Permalink
Browse files

[backport] SI-6567 Warning for Option(implicitView(foo))

commit 284bd754fa5dfc8bc626b0c5ebe85d872dd044cb
Author: Jason Zaugg <jzaugg@gmail.com>
Date:   Sat Nov 3 16:19:46 2012 +0100

    SI-6567 Warning for Option(implicitView(foo))

    I've seen the reported problem before in the wild. It seems
    worthy of a special warning, so long as we advocate Option.apply
    as an alternative to `if (x == null) Some(x) else None`.

    It is behind -Xlint at the moment, an option that could do with
    some promotion.
    (cherry picked from commit 0bcb9e9)
  • Loading branch information...
1 parent 884737c commit 96ed055769483d661b09f346cd1641f956f3172a @retronym retronym committed Jan 23, 2013
@@ -1062,6 +1062,12 @@ abstract class RefChecks extends InfoTransform with scala.reflect.internal.trans
def apply(tp: Type) = mapOver(tp).normalize
}
+ def checkImplicitViewOptionApply(pos: Position, fn: Tree, args: List[Tree]): Unit = if (settings.lint.value) (fn, args) match {
+ case (tap@TypeApply(fun, targs), List(view: ApplyImplicitView)) if fun.symbol == Option_apply =>
+ unit.warning(pos, s"Suspicious application of an implicit view (${view.fun}) in the argument to Option.apply.") // SI-6567
+ case _ =>
+ }
+
def checkSensible(pos: Position, fn: Tree, args: List[Tree]) = fn match {
case Select(qual, name @ (nme.EQ | nme.NE | nme.eq | nme.ne)) if args.length == 1 =>
def isReferenceOp = name == nme.eq || name == nme.ne
@@ -1563,7 +1569,10 @@ abstract class RefChecks extends InfoTransform with scala.reflect.internal.trans
case Apply(fn, args) =>
// sensicality should be subsumed by the unreachability/exhaustivity/irrefutability analyses in the pattern matcher
- if (!inPattern) checkSensible(tree.pos, fn, args)
+ if (!inPattern) {
+ checkImplicitViewOptionApply(tree.pos, fn, args)
+ checkSensible(tree.pos, fn, args)
+ }
currentApplication = tree
tree
}
@@ -536,10 +536,12 @@ trait Definitions extends api.StandardDefinitions {
lazy val ScalaLongSignatureAnnotation = requiredClass[scala.reflect.ScalaLongSignature]
// Option classes
- lazy val OptionClass: ClassSymbol = requiredClass[Option[_]]
- lazy val SomeClass: ClassSymbol = requiredClass[Some[_]]
- lazy val NoneModule: ModuleSymbol = requiredModule[scala.None.type]
- lazy val SomeModule: ModuleSymbol = requiredModule[scala.Some.type]
+ lazy val OptionClass: ClassSymbol = requiredClass[Option[_]]
+ lazy val OptionModule: ModuleSymbol = requiredModule[scala.Option.type]
+ lazy val Option_apply = getMemberMethod(OptionModule, nme.apply)
+ lazy val SomeClass: ClassSymbol = requiredClass[Some[_]]
+ lazy val NoneModule: ModuleSymbol = requiredModule[scala.None.type]
+ lazy val SomeModule: ModuleSymbol = requiredModule[scala.Some.type]
def compilerTypeFromTag(tt: ApiUniverse # WeakTypeTag[_]): Type = tt.in(rootMirror).tpe
def compilerSymbolFromTag(tt: ApiUniverse # WeakTypeTag[_]): Symbol = tt.in(rootMirror).tpe.typeSymbol
@@ -0,0 +1,7 @@
+t6567.scala:8: error: Suspicious application of an implicit view (Test.this.a2b) in the argument to Option.apply.
+ Option[B](a)
+ ^
+t6567.scala:10: error: Suspicious application of an implicit view (Test.this.a2b) in the argument to Option.apply.
+ val b: Option[B] = Option(a)
+ ^
+two errors found
@@ -0,0 +1 @@
+-Xlint -Xfatal-warnings
View
@@ -0,0 +1,11 @@
+class A
+class B
+
+object Test {
+ val a: A = null
+ implicit def a2b(a: A) = new B
+
+ Option[B](a)
+
+ val b: Option[B] = Option(a)
+}

0 comments on commit 96ed055

Please sign in to comment.