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

Regression in typer for named arguments of infix methods in ghostdogpr/caliban #21767

Open
WojciechMazur opened this issue Oct 15, 2024 · 3 comments
Assignees
Labels
area:typer itype:bug regression This worked in a previous version but doesn't anymore

Comments

@WojciechMazur
Copy link
Contributor

WojciechMazur commented Oct 15, 2024

Based on OpenCB failure for ghostdogpr/caliban - build logs
Affects also:

The new behaviour might have become present by the stabilization of named tuples. Previously named tuples syntax was not tried without an explicit experimental flag enabled.

Compiler version

Last good release: 3.6.0-RC1-bin-20241003-a672e05-NIGHTLY
First bad release: 3.6.0-RC1-bin-20241005-6fa81cf-NIGHTLY

Bisect points to 87cdbc8

Minimized code

trait GraphQLRequest
sealed trait SelectionBuilder[+A]:
  def toGraphQL[A1 >: A](queryName: Option[String] = None): GraphQLRequest = ???

trait Character
trait Field[A] extends SelectionBuilder[A]

def query = 
  val field: Field[Character]  = ???
  val works = field.toGraphQL(queryName = Some("GetCharacter"))
  val fails = field `toGraphQL` (queryName = Some("GetCharacter"))

Output

-- Error: /Users/wmazur/projects/sandbox/test.scala:11:44 ----------------------
11 |  val fails = field `toGraphQL` (queryName = Some("GetCharacter"))
   |                                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
   |method toGraphQL in trait SelectionBuilder: (queryName: Option[String]): GraphQLRequest does not have a parameter equeryName

Expectation

The compiler should interpret infix apply as list of arguments instead of named tuple.

@Gedochao
Copy link
Contributor

cc @odersky

@odersky
Copy link
Contributor

odersky commented Oct 17, 2024

The compiler should interpret infix apply as list of arguments instead of named tuple.

No, it should not. Argument syntax should be tied to regular method calls, An infix operator takes two expressions; there is no argument list in the traditional sense. Another case where this matters is

  a *: (b, c)

This should really preprend a to the tuple (b, c). Right now it's a type error because the right operand is treated like an argument list. We want to get away from that. Named tuples are just one bit where this problem manifests itself.

Workaround:

  val fails = field.toGraphQL(queryName = Some("GetCharacter"))

It's a lot clearer anyway.

As with the other named tuple problem we should try to do a migration warning and offer a rewrite rule.

@ghostdogpr
Copy link
Contributor

I agree that this code was weird, I fixed it in Caliban: ghostdogpr/caliban#2440

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:typer itype:bug regression This worked in a previous version but doesn't anymore
Projects
None yet
Development

No branches or pull requests

5 participants