Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Issue "positional after named argument" errors #18363

Merged
merged 2 commits into from
Aug 21, 2023

Conversation

odersky
Copy link
Contributor

@odersky odersky commented Aug 8, 2023

Issue "positional after named argument" errors if a positional argument follows a named argument and one of the following is true:

  • There is a formal argument before the argument position that has not yet been instantiated with a previous actual argument, (either named or positional), or
  • The formal parameter at the argument position is also mentioned in a subsequent named parameter.

This brings the behavior largely in line with Scala 2.

Fixes #18122

Issue "positional after named argument" errors if a positional
argument follows a named argument and one of the following is true:

 - There is a formal argument before the argument position
   that has not yet been instantiated with a previous actual argument,
   (either named or positional), or
 - The formal parameter at the argument position is also mentioned
   in a subsequent named parameter.

This brings the behavior largely in line with Scala 2
bar2(x = 1, 2, ys = 3) // error: positional after named
bar1(ys = 1, 2, x = 3) // error: positional after named
bar2(ys = 1, 2, x = 3) // error: positional after named
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

bar2(x = 1, ys = 2, ys = 3)? probably parameter ys is already instantiated.

bar2(x = 1, 2, ys = 3) // error: positional after named on L45 is slightly confusing message.

I expect that if named args do not change ordering, then it's always OK.

I see the rule that's being enforced is that ys always names the start of the repeated param. Then the positional arg 2 is unknown detritus.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All true, but positional after named is technically correct, and it would be messy to change the logic to emit a different error message. Keeping logic clean in the compiler is an underestimated good. Lose it and you enter quickly a downward slide where in the end nobody understands the code anymore.

@prolativ
Copy link
Contributor

@som-snytt any further objections or can we get this merged?

@som-snytt
Copy link
Contributor

LGTM! I especially appreciate

Keeping logic clean in the compiler is an underestimated good.

It's easy to go overboard, and the voice of experience is also an underestimated good.

I haven't looked at Scala 2 messaging yet, which I will do in light of:

I see the rule that's being enforced is that ys always names the start of the repeated param.

@prolativ prolativ merged commit d0cc4c1 into scala:main Aug 21, 2023
17 checks passed
@prolativ prolativ deleted the fix-18122 branch August 21, 2023 13:27
@WojciechMazur
Copy link
Contributor

WojciechMazur commented Sep 4, 2023

There are 3 OpenCB projects which are now failing after this change, but I would not say it's an regression, but rather an improvement.

Project Version Build URL Notes
carlos-verdes/zio-arangodb 0.3.1 Open CB logs
fabiopinheiro/scala-did 0.1.0-M9 -> 0.1.0-M10 Open CB logs
greenfossil/data-mapping 1.0.5 -> 1.0.7 Open CB logs

My question is, @Kordyjan should we backport this change to Scala 3.3.x?
One example of weakly-defined behaviours possibly leading to bugs before this change was:

case class Foo(a: Int, b: Long, c: Int){
  def x = copy(b = 0L, 0) // is the second param targetting `a` or `c`?
}

@som-snytt
Copy link
Contributor

The previous (interesting) expectation was that a named arg "resets" the position, such that defaults to the left of that position are supplied. (Such as on zio-arangodb.)

Welcome to Scala 2.13.11 (OpenJDK 64-Bit Server VM, Java 20.0.1).
Type in expressions for evaluation. Or try :help.

scala> def f(x: Int, y: Int = 42, z: Int, a: Int) = List(x,y,z,a).sum
def f(x: Int, y: Int, z: Int, a: Int): Int

scala> f(1, z=3, 4)
                 ^
       error: positional after named argument.
        ^
       error: not enough arguments for method f: (x: Int, y: Int, z: Int, a: Int): Int.
       Unspecified value parameter a.

which on 3.3.0

Welcome to Scala 3.3.0 (20.0.1, Java OpenJDK 64-Bit Server VM).
Type in expressions for evaluation. Or try :help.

scala> def f(x: Int, y: Int = 42, z: Int, a: Int) = List(x,y,z,a).sum
def f(x: Int, y: Int, z: Int, a: Int): Int

scala> f(1, z=3, 4)
val res0: Int = 50

my dotty just finished compiling, so for completeness

scala> f(1, z=3, 4)
-- Error: --------------------------------------------------------------------------------------------------------------
1 |f(1, z=3, 4)
  |          ^
  |          positional after named argument
1 error found

The wrong expectation is not terrible, because visually you align the named arg with the signature. It feels like a feature, because otherwise default args are convenient only in trailing position.

The case class copy example is deceptive because there is no signature to look at besides the class parameters. (You don't see the defaults, in other words.)

The argument for backporting is that it's never too early to correct a mistaken expectation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Unnamed parameters after named ones
5 participants