From 96ed055769483d661b09f346cd1641f956f3172a Mon Sep 17 00:00:00 2001 From: Jason Zaugg Date: Wed, 23 Jan 2013 01:10:05 +0100 Subject: [backport] SI-6567 Warning for Option(implicitView(foo)) commit 284bd754fa5dfc8bc626b0c5ebe85d872dd044cb Author: Jason Zaugg 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 0bcb9e9169146e3f589c6c9f65cc4a5523b78120) --- src/compiler/scala/tools/nsc/typechecker/RefChecks.scala | 11 ++++++++++- src/reflect/scala/reflect/internal/Definitions.scala | 10 ++++++---- test/files/neg/t6567.check | 7 +++++++ test/files/neg/t6567.flags | 1 + test/files/neg/t6567.scala | 11 +++++++++++ 5 files changed, 35 insertions(+), 5 deletions(-) create mode 100644 test/files/neg/t6567.check create mode 100644 test/files/neg/t6567.flags create mode 100644 test/files/neg/t6567.scala diff --git a/src/compiler/scala/tools/nsc/typechecker/RefChecks.scala b/src/compiler/scala/tools/nsc/typechecker/RefChecks.scala index 7118375b82..969bb8aceb 100644 --- a/src/compiler/scala/tools/nsc/typechecker/RefChecks.scala +++ b/src/compiler/scala/tools/nsc/typechecker/RefChecks.scala @@ -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 } diff --git a/src/reflect/scala/reflect/internal/Definitions.scala b/src/reflect/scala/reflect/internal/Definitions.scala index 2a7b55cb5a..4bd0aca9c4 100644 --- a/src/reflect/scala/reflect/internal/Definitions.scala +++ b/src/reflect/scala/reflect/internal/Definitions.scala @@ -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 diff --git a/test/files/neg/t6567.check b/test/files/neg/t6567.check new file mode 100644 index 0000000000..4c513e64cd --- /dev/null +++ b/test/files/neg/t6567.check @@ -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 diff --git a/test/files/neg/t6567.flags b/test/files/neg/t6567.flags new file mode 100644 index 0000000000..e93641e931 --- /dev/null +++ b/test/files/neg/t6567.flags @@ -0,0 +1 @@ +-Xlint -Xfatal-warnings \ No newline at end of file diff --git a/test/files/neg/t6567.scala b/test/files/neg/t6567.scala new file mode 100644 index 0000000000..650e5e39ae --- /dev/null +++ b/test/files/neg/t6567.scala @@ -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) +} -- cgit v1.2.3