Wrong rewrite of implicit arguments passed as { }

[OndrejSpanel]

Compiler version

3.7.0-RC1-bin-20250303-91985db-NIGHTLY

Minimized code

class Input(x: String)

trait Decoder[T]

object Decoder {
  inline def apply[T](implicit f: () => Unit): Decoder[T] = ???
}

object Input {
  given Decoder[Input] = Decoder { () =>
    Input("")
  }
}

Compile this with "-rewrite", "-source", "3.7-migration".

Output

object Input {
  given Decoder[Input] = Decoder using { () =>
    Input("")
  }
}

The code cannot be compiled, the using word is placed at a location where it makes no sense.

Note:

• this style is used quite a lot do define custom codecs when using Borer library.
• the code is rewritten correctly when using parentheses, like `given Decoder[Input] = Decoder(() => Input(""))

Expectation

The rewrite should not produce invalid code.

[mbovel]

This issue was picked for the Scala Issue Spree of Monday, April 7th. @mbovel, @mrdziuban and @ajafri2001 will be working on it. If you have any insight into the issue or guidance on how to fix it, please leave it here.

[mbovel]

Reproduced with scala -S 3.nightly -rewrite -source:3.7-migration 22731.scala, which rewrites:

class Input(x: String)

trait Decoder[T]

object Decoder {
  inline def apply[T](implicit f: () => Unit): Decoder[T] = ???
}

object Input {
  given Decoder[Input] = Decoder { () =>
    Input("")
  }
}

to (using inserted before { () =>):

class Input(x: String)

trait Decoder[T]

object Decoder {
  inline def apply[T](implicit f: () => Unit): Decoder[T] = ???
}

object Input {
  given Decoder[Input] = Decoder using { () =>
    Input("")
  }
}

while we'd probably want the following (curly braces changed to parentheses, and using added within the parentheses):

class Input(x: String)

trait Decoder[T]

object Decoder {
  inline def apply[T](implicit f: () => Unit): Decoder[T] = ???
}

object Input {
  given Decoder[Input] = Decoder(using () =>
    Input("")
  )
}

[OndrejSpanel]

while we'd probably want the following (curly braces changed to parentheses, and using added within the parentheses):

This may be valid sometimes, but often you may need to keep curly braces, and just add parentheses around the block, as the function body may contain constructs not available without braces, like variable declarations.

[mbovel]

Ah right, so should we aim to rewrite to the following?

object Input {
  given Decoder[Input] = Decoder (using {() => 
    Input("")
  })
}

[OndrejSpanel]

As a primary form, yes, this. Removing the inner braces may be sometimes possible, but I find this secondary - I do not expect rewrites to create a code which is always nice, but it should be compilable.

[mbovel]

[...] the function body may contain constructs not available without braces, like variable declarations.

Here is such an example:

trait Decoder[T]

object Decoder {
  inline def apply[T](implicit f: () => Unit): Decoder[T] = ???
}

object Input {
  given Decoder[Int] = Decoder(using {() => val x = 42; println("blip")})
}

@main def main = ()

We should add it as a test case.

[OndrejSpanel]

Does this compile? I would expect Decoder[Int] to return Int, this returns Unit.

[mbovel]

Does this compile? I would expect Decoder[Int] to return Int, this returns Unit.

Yes, the type parameter of Decoder is not used, and Decoder.apply requires a function () => Unit.

This can actually further be minimized to:

trait Decoder

def Decoder(implicit f: () => Unit): Decoder = ???

@main def main =
  Decoder { () => val x = 42; println("blip") }

or even

def foo(implicit bar: () => Unit): Unit = ???

@main def main =
  foo { () => val x = 42; println("blip") }

[mbovel]

The error message and patch are generated here:

scala3/compiler/src/dotty/tools/dotc/typer/Migrations.scala

Lines 130 to 142 in 0cd43fe

	def implicitParams(tree: Tree, tp: MethodOrPoly, pt: FunProto)(using Context): Unit =
	val mversion = mv.ImplicitParamsWithoutUsing
	if tp.companion == ImplicitMethodType && pt.applyKind != ApplyKind.Using && pt.args.nonEmpty then
	val rewriteMsg = Message.rewriteNotice("This code", mversion.patchFrom)
	report.errorOrMigrationWarning(
	em"""Implicit parameters should be provided with a `using` clause.$rewriteMsg
	|To disable the warning, please use the following option:
	| "-Wconf:msg=Implicit parameters should be provided with a `using` clause:s"
	|""",
	pt.args.head.srcPos, mversion)
	if mversion.needsPatch then
	patch(Span(pt.args.head.span.start), "using ")
	end implicitParams

[mbovel]

So, from what we observed, the parentheses should be added if and only if the function was applied with the trailing lambda syntax (curly braces):

def foo(implicit bar: () => Unit): Unit = ???

@main def main =
  foo { () => val x = 42; println("blip") } // should add parentheses
  foo({ () => val x = 42; println("blip") }) // should not add parentheses

However we are missing this information. Could we add it to the Apply node?

[som-snytt]

Probably I'm the only one who was hoping for "infix using" syntax.

[odersky]

I think adding an applyStyle to every apply node is overkill by a large margin. Why can't we simply not suggest a rewrite when the argument is enclosed in braces. The user has to do it manually then, but that's OK I think. Not worth spending a lot of effort on a corner case.

[mbovel]

Okay, I tried an alternative implementation based on @som-snytt's suggestion: #22937. The changeset is indeed much smaller.

[jpsacha]

#23023 is marked as a duplicate if this issue. The specific problem in #23023 is still there in 3.7.0-RC4. Code generated by "-rewrite -source 3.7-migration" is invalid and will not compile.

[tgodzik]

@jpsacha 3.7.0-RC4 only backported a single commit, the fix should be in 3.7.1

[som-snytt]

We almost got the infix syntax (last decade). Significantly, "we know how to parse this."

https://contributors.scala-lang.org/t/proposal-to-revise-implicit-parameters/3044/46?u=som-snytt