Deprecate top-level wildcard type parameters

[som-snytt]

Deprecates using underscore (_) as a type parameter, except for "higher-order type parameters."

Under -Xsource:3, issue an error instead of merely warning.

class C[A]
class C[F[_]]
class C[_]       // no

Fixes scala/bug#5606

[SethTisue]

changes LGTM, but @som-snytt would you mind revising the PR description to be more self-contained and user-facing? current text seems more aimed at implementers and assumes familiarity with the ticket. (it's fine to include such reviewer-aimed commentary, but at the end, after the what-users-need-to-know part)

in particular, it wasn't obvious to me before I groveled through the diffs that this alters both default behaviors and under--Xsource:3 behaviors

[som-snytt]

@SethTisue let me check what they're doing at https://docs.scala-lang.org/scala3/reference/changed-features/wildcards.html

I can't get that to work, and I haven't tried their advice to use -Xsource:3 -P:kind-projector:underscore-placeholders together.

[som-snytt]

It's already a month old, enough for a conflict in Parsers.

[SethTisue]

neg/t5606.scala is failing

[SethTisue]

@som-snytt do you mind if we reopen and merge this?

[som-snytt]

If this is an obvious win to someone, it's OK. It may belong to the long tail of "smudges on the eyeglasses of Scala 2 which no one notices until you point it out". Maybe it helps align with Scala 3?

[SethTisue]

@dwijnand final review(/merge)?

[SethTisue]

The community build noticed that @lihaoyi's Fastparse library actually recommends top-level _ as an idiom. At https://com-lihaoyi.github.io/fastparse/ we see many examples such as:

def number[_: P]: P[Int] = P( CharIn("0-9").rep(1).!.map(_.toInt) )
def parens[_: P]: P[Int] = P( "(" ~/ addSub ~ ")" )
def factor[_: P]: P[Int] = P( number | parens )

But then what's peculiar is that the log doesn't merely show a deprecation warning being emitted, as one might have expected. Instead we see:

[fastparse] [error] /home/jenkins/workspace/scala-2.13.x-jdk11-integrate-community-build/target-0.9.17/project-builds/fastparse-ae79983f4e3b61f0f3b6b2440b38f7c1b6050be5/fastparse/test/src/fastparse/ExampleTests.scala:199:44: could not find implicit value for evidence parameter of type fastparse.P[_]
[fastparse] [error]         def xml[_: P] = P( leftTag.flatMap(rightTag) )
[fastparse] [error]                                            ^

So, that's a regression, perhaps? (If so, then once again we discover that no good deed goes unpunished...)

We'll need to do something about this before releasing 2.13.7, since fastparse is fairly widely used. @som-snytt are you interested in digging into this sometime this week? If not, I can dig into it.

Simply reverting the entire PR for 2.13.7 and then perhaps revisiting for 2.13.8 is of course a perfectly fine fallback plan.

[SethTisue]

It's peculiar that only a single test case in a big file full of tests is affected.

And it's interesting that if we give names to all of the _ type parameters in the relevant lines of that test:

        def leftTag[A: P] = P( "<" ~ (!">" ~ AnyChar).rep(1).! ~ ">")
        def rightTag[B: P](s: String) = P( "</" ~ s.! ~ ">" )
        def xml[C: P] = P( leftTag.flatMap(rightTag) )

then we still get the same "could not find implicit value" error, even on 2.13.6.

This has me wondering if something actually progressed here, rather than regressed.

[SethTisue]

changing leftTag.flatMap(rightTag) to leftTag.flatMap(rightTag(_)) makes the problem go away

[SethTisue]

I never got to the bottom of this, but I've decided that I've spent enough time on it and I'm comfortable letting this change stand for 2.13.7, because:

• Two easy user-level workarounds exist (using a function literal instead of relying on eta-expansion, or switching to for which is suggested in the fastparse documentation as an equivalent alternative)
• The experiment with giving names to the type parameters shows the test case was only compiling because of some kind of loophole that is apparently now closed.
• fastparse is heavily macro-based, so to really get to the bottom of this would require understanding and debugging those macros and verifying that it isn't the macros themselves that are at fault here.

If actual fastparse users run afoul of this, they can report the problem to the maintainers of fastparse, and if those maintainers conclude that this PR contains a regression in Scala, it's up to them to provided a detailed substantiation of that.

fyi @lrytz

[SethTisue]

As an aside, I feel like I distinctly remember that Martin saw the [_ : X] syntax being used heavily in fastparse and expressed surprise that it even compiled and surprise (or even dismay) that the apparent loophole had been so heavily exploited by a library author. However, I can't find a ticket or forum thread about this, so I can't prove that I didn't dream that :-)

fyi @lihaoyi

[lihaoyi]

Sounds to me like the underscores aren't the problem, and the real problem is the behavior of eta-expanding methods taking implicit parameters has changed?

[SethTisue]

@lihaoyi I haven't observed any behavior difference unless type parameters named _ are involved. If the type parameters aren't named _, then the code doesn't compile in either 2.13.6 or the 2.13.7 nightlies.

[lihaoyi]

Got it, adding an explocit (_) seems easy enough

[unkarjedy]

@som-snytt
Current warning message wording feels a little bit vague.

Top-level wildcard is not allowed and will error under -Xsource:3
private def foo1[_ : MyClass]: MyClass[Unit] = ???

_ : MyClass doesn't look like a top-level wildcard
What does it even mean?
I would expect something top-level at the top level of the file.

def foo = 1
_
val _ = 1
import x._

Maybe we could refine the wording to be more clear:

Top-level wildcard type parameter is not allowed and will error under -Xsource:3
private def foo1[_ : MyClass]: MyClass[Unit] = ???

?

[som-snytt]

The top of this page says: Deprecates using underscore (_) as a type parameter, except for "higher-order type parameters."

Is top-level type param ever called first-order?

I closed this PR, but someone else might be invested in editing the message.

[SethTisue]

Maybe we could refine the wording to be more clear

Yes, a PR on that would be welcome.

[limansky]

Hi,

Could anybody please explain, what is the proper way to use fastparse with 2.13.7? I get this kind of warning for each parser I have: Top-level wildcard is not allowed and will error under -Xsource:3. Does it mean that I have to define type parameter name for each parser? ([X : P] instead of [_ : P]). For now, I cannot build project with -Xfatal-warnings anymore.

[som-snytt]

@limansky I'm about to tweak the message (with a related fix) but compare

Welcome to Scala 3.1.2-RC1-bin-SNAPSHOT-git-ad5c714 (17, Java OpenJDK 64-Bit Server VM).
Type in expressions for evaluation. Or try :help.

scala> class C[_]
-- [E040] Syntax Error: -------------------------------------------------------------------------------------------------------------------------
1 |class C[_]
  |        ^
  |        an identifier expected, but '_' found

longer explanation available when compiling with `-explain`

scala>
➜  scala
Welcome to Scala 2.13.7 (OpenJDK 64-Bit Server VM, Java 17).
Type in expressions for evaluation. Or try :help.

scala> class C[_]
               ^
       warning: Top-level wildcard is not allowed and will error under -Xsource:3
class C

➜  scala -Xsource:3
Welcome to Scala 2.13.7 (OpenJDK 64-Bit Server VM, Java 17).
Type in expressions for evaluation. Or try :help.

scala> class C[_]
               ^
       error: identifier expected but '_' found.

➜  scala -Wconf:msg=Top-level:s
Welcome to Scala 2.13.7 (OpenJDK 64-Bit Server VM, Java 17).
Type in expressions for evaluation. Or try :help.

scala> class C[_]
class C

Also -Wconf:cat=deprecation:s etc. @annotation.nowarn doesn't work with syntax warnings.

[lrytz]

Also -Wconf:cat=deprecation:s etc. @annotation.nowarn doesn't work with syntax warnings.

They should work - do they not?

[limansky]

@som-snytt do you mean that only I can do is to suppress the warnings, but not to fix the root cause?

[som-snytt]

sorry @lrytz I tried nowarn in REPL but now it works so shruggie.

@limansky yes, I meant to suggest suppressing the warnings. I think I just saw someone mention fastparse in this regard, but I don't remember which forum. I'll update here if I learn anything else.