Merge pull request #5327 from lrytz/2.12.x

SI-7187 deprecate eta-expansion of zero-arg method values

8 files changed, 54 insertions, 42 deletions

diff --git a/src/compiler/scala/tools/nsc/typechecker/Typers.scala b/src/compiler/scala/tools/nsc/typechecker/Typers.scala
index ba104fb7a6..91cff54bc7 100644
--- a/src/compiler/scala/tools/nsc/typechecker/Typers.scala
+++ b/src/compiler/scala/tools/nsc/typechecker/Typers.scala
@@ -872,16 +872,32 @@ trait Typers extends Adaptations with Tags with TypersTracking with PatternTyper
           case Block(_, tree1) => tree1.symbol
           case _               => tree.symbol
         }
-        def shouldEtaExpandToSam: Boolean = {
-          // SI-9536 don't adapt parameterless method types to a to SAM's, fall through to empty application
-          // instead for backwards compatiblity with 2.11. See comments of that ticket and SI-7187
-          // for analogous trouble with non-SAM eta expansion. Suggestions there are: a) deprecate eta expansion to Function0,
-          // or b) switch the order of eta-expansion and empty application in this adaptation.
-          !mt.params.isEmpty && samOf(pt).exists
-        }
-        if (!meth.isConstructor && (isFunctionType(pt) || shouldEtaExpandToSam)) { // (4.2)
+
+        def cantAdapt =
+          if (context.implicitsEnabled) MissingArgsForMethodTpeError(tree, meth)
+          else setError(tree)
+
+        // constructors do not eta-expand
+        if (meth.isConstructor) cantAdapt
+        // (4.2) eta-expand method value when function or sam type is expected
+        else if (isFunctionType(pt) || (!mt.params.isEmpty && samOf(pt).exists)) {
+          // SI-9536 `!mt.params.isEmpty &&`: for backwards compatiblity with 2.11,
+          // we don't adapt a zero-arg method value to a SAM
+          // In 2.13, we won't do any eta-expansion for zero-arg method values, but we should deprecate first
+
           debuglog(s"eta-expanding $tree: ${tree.tpe} to $pt")
           checkParamsConvertible(tree, tree.tpe)
+
+          // SI-7187 eta-expansion of zero-arg method value is deprecated, switch order of (4.3) and (4.2) in 2.13
+          def isExplicitEtaExpansion = original match {
+            case Typed(_, Function(Nil, EmptyTree)) => true // tree shape for `f _`
+            case _ => false
+          }
+          if (mt.params.isEmpty && !isExplicitEtaExpansion) {
+            currentRun.reporting.deprecationWarning(tree.pos, NoSymbol,
+              s"Eta-expansion of zero-argument method values is deprecated. Did you intend to write ${Apply(tree, Nil)}?", "2.12.0")
+          }
+
           val tree0 = etaExpand(context.unit, tree, this)
 
           // #2624: need to infer type arguments for eta expansion of a polymorphic method
@@ -895,12 +911,9 @@ trait Typers extends Adaptations with Tags with TypersTracking with PatternTyper
           else
             typed(tree0, mode, pt)
         }
-        else if (!meth.isConstructor && mt.params.isEmpty) // (4.3)
-          adapt(typed(Apply(tree, Nil) setPos tree.pos), mode, pt, original)
-        else if (context.implicitsEnabled)
-          MissingArgsForMethodTpeError(tree, meth)
-        else
-          setError(tree)
+        // (4.3) apply to empty argument list -- TODO 2.13: move this one case up to avoid eta-expanding at arity 0
+        else if (mt.params.isEmpty) adapt(typed(Apply(tree, Nil) setPos tree.pos), mode, pt, original)
+        else cantAdapt
       }
 
       def adaptType(): Tree = {
@@ -4398,11 +4411,11 @@ trait Typers extends Adaptations with Tags with TypersTracking with PatternTyper
         *   (2) If $e$ is a parameterless method or call-by-name parameter of type `=>$T$`, `$e$ _` represents
         *       the function of type `() => $T$`, which evaluates $e$ when it is applied to the empty parameterlist `()`.
         */
-      def typedEta(methodValue: Tree): Tree = methodValue.tpe match {
+      def typedEta(methodValue: Tree, original: Tree): Tree = methodValue.tpe match {
         case tp@(MethodType(_, _) | PolyType(_, MethodType(_, _))) => // (1)
           val formals = tp.params
           if (isFunctionType(pt) || samMatchesFunctionBasedOnArity(samOf(pt), formals)) methodValue
-          else adapt(methodValue, mode, checkArity(methodValue)(functionTypeWildcard(formals.length)))
+          else adapt(methodValue, mode, checkArity(methodValue)(functionTypeWildcard(formals.length)), original)
 
         case TypeRef(_, ByNameParamClass, _) |  NullaryMethodType(_) => // (2)
           val pos = methodValue.pos
@@ -5106,7 +5119,7 @@ trait Typers extends Adaptations with Tags with TypersTracking with PatternTyper
           case Typed(expr, Function(Nil, EmptyTree)) =>
             typed1(suppressMacroExpansion(expr), mode, pt) match {
               case macroDef if treeInfo.isMacroApplication(macroDef) => MacroEtaError(macroDef)
-              case methodValue                                       => typedEta(checkDead(methodValue))
+              case methodValue                                       => typedEta(checkDead(methodValue), tree)
             }
           case Typed(expr, tpt) =>
             val tpt1  = typedType(tpt, mode)                           // type the ascribed type first
diff --git a/test/files/neg/t7187.check b/test/files/neg/t7187.check
new file mode 100644
index 0000000000..a30803c746
--- /dev/null
+++ b/test/files/neg/t7187.check
@@ -0,0 +1,6 @@
+t7187.scala:3: warning: Eta-expansion of zero-argument method values is deprecated. Did you intend to write EtaExpandZeroArg.this.foo()?
+  val f: () => Any = foo
+                     ^
+error: No warnings can be incurred under -Xfatal-warnings.
+one warning found
+one error found
diff --git a/test/files/neg/t7187.flags b/test/files/neg/t7187.flags
new file mode 100644
index 0000000000..c6bfaf1f64
--- /dev/null
+++ b/test/files/neg/t7187.flags
@@ -0,0 +1 @@
+-deprecation -Xfatal-warnings
diff --git a/test/files/neg/t7187.scala b/test/files/neg/t7187.scala
new file mode 100644
index 0000000000..45d33f06af
--- /dev/null
+++ b/test/files/neg/t7187.scala
@@ -0,0 +1,6 @@
+class EtaExpandZeroArg {
+  def foo(): () => String = () => ""
+  val f: () => Any = foo
+
+  // f() would evaluate to <function0> instead of ""
+}
diff --git a/test/files/pos/t9178.flags b/test/files/pos/t9178.flags
deleted file mode 100644
index 7de3c0f3ee..0000000000
--- a/test/files/pos/t9178.flags
+++ /dev/null
@@ -1 +0,0 @@
--Xfatal-warnings -deprecation
diff --git a/test/files/pos/t9178.scala b/test/files/pos/t9178.scala
deleted file mode 100644
index f2cf20a778..0000000000
--- a/test/files/pos/t9178.scala
+++ /dev/null
@@ -1,13 +0,0 @@
-// eta expansion to Function0 is problematic (as shown here).
-// Perhaps we should we deprecate it? See discussion in the comments of
-// on SI-9178.
-//
-// This test encodes the status quo: no deprecation.
-object Test {
-  def foo(): () => String = () => ""
-  val f: () => Any = foo
-
-  def main(args: Array[String]): Unit = {
-    println(f()) // <function0>
-  }
-}
diff --git a/test/files/run/Course-2002-08.scala b/test/files/run/Course-2002-08.scala
index 5e21edaba3..1d0e02262d 100644
--- a/test/files/run/Course-2002-08.scala
+++ b/test/files/run/Course-2002-08.scala
@@ -205,7 +205,7 @@ object M5 {
         val inputSig = input.getSignal;
         afterDelay(InverterDelay) {() => output.setSignal(!inputSig) };
       }
-      input addAction invertAction
+      input addAction invertAction _
     }
 
     def andGate(a1: Wire, a2: Wire, output: Wire): Unit = {
@@ -214,8 +214,8 @@ object M5 {
         val a2Sig = a2.getSignal;
         afterDelay(AndGateDelay) {() => output.setSignal(a1Sig & a2Sig) };
       }
-      a1 addAction andAction;
-      a2 addAction andAction;
+      a1 addAction andAction _
+      a2 addAction andAction _
     }
 
     def orGate(o1: Wire, o2: Wire, output: Wire): Unit = {
@@ -224,8 +224,8 @@ object M5 {
         val o2Sig = o2.getSignal;
         afterDelay(OrGateDelay) {() => output.setSignal(o1Sig | o2Sig) };
       }
-      o1 addAction orAction;
-      o2 addAction orAction;
+      o1 addAction orAction _
+      o2 addAction orAction _
     }
 
     def probe(name: String, wire: Wire): Unit = {
@@ -479,7 +479,7 @@ abstract class BasicCircuitSimulator() extends Simulator() {
       val inputSig = input.getSignal;
       afterDelay(InverterDelay) {() => output.setSignal(!inputSig) };
     }
-    input addAction invertAction
+    input addAction invertAction _
   }
 
   def andGate(a1: Wire, a2: Wire, output: Wire) = {
@@ -488,8 +488,8 @@ abstract class BasicCircuitSimulator() extends Simulator() {
       val a2Sig = a2.getSignal;
       afterDelay(AndGateDelay) {() => output.setSignal(a1Sig & a2Sig) };
     }
-    a1 addAction andAction;
-    a2 addAction andAction
+    a1 addAction andAction _
+    a2 addAction andAction _
   }
 
   def orGate(a1: Wire, a2: Wire, output: Wire) = {
@@ -498,8 +498,8 @@ abstract class BasicCircuitSimulator() extends Simulator() {
       val a2Sig = a2.getSignal;
       afterDelay(OrGateDelay) {() => output.setSignal(a1Sig | a2Sig) };
     }
-    a1 addAction orAction;
-    a2 addAction orAction
+    a1 addAction orAction _
+    a2 addAction orAction _
   }
 
   def orGate2(a1: Wire, a2: Wire, output: Wire) = {
diff --git a/test/files/run/runtime.scala b/test/files/run/runtime.scala
index 89348b294d..468a80fc0c 100644
--- a/test/files/run/runtime.scala
+++ b/test/files/run/runtime.scala
@@ -73,7 +73,7 @@ object Test1Test {
     // {System.out.print(22); test1.bar}.System.out.println();
     {Console.print(23); test1.bar.System}.out.println();
     {Console.print(24); test1.bar.System.out}.println();
-    {Console.print(25); test1.bar.System.out.println:(() => Unit)} apply ();
+    {Console.print(25); test1.bar.System.out.println _ : (() => Unit)} apply ();
     {Console.print(26); test1.bar.System.out.println()};
   }