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

Deprecate infix named args #21565

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

som-snytt
Copy link
Contributor

Addresses #19072
Ports scala/scala#10857

@som-snytt som-snytt force-pushed the discussion/19072-infix-named-arg branch from 2cfa491 to 4d25ea3 Compare September 7, 2024 17:04
@som-snytt
Copy link
Contributor Author

Probably better as an external lint.

@odersky
Copy link
Contributor

odersky commented Oct 1, 2024

With named tuples this becomes more urgent. I agree, we should deprecate that syntax.

@mbovel
Copy link
Member

mbovel commented Oct 22, 2024

Probably better as an external lint.

Looking at #21681, seems like this is common enough to require a compiler warning. Would you like to revive this PR? (Otherwise I am happy to do it.)

@som-snytt som-snytt reopened this Oct 22, 2024
@som-snytt
Copy link
Contributor Author

som-snytt commented Oct 22, 2024

The warning at parser goes unseen in October because the arg is now a unituple. (I don't even know the lingo yet.) So the warning info is only meaningful as an addendum to the error here. The warning should be backported to LTS. I'll try that after a second coffee.

@som-snytt som-snytt force-pushed the discussion/19072-infix-named-arg branch 2 times, most recently from 67ba634 to 1d8838c Compare October 22, 2024 21:01
@som-snytt
Copy link
Contributor Author

There is not much to addend, as it turns out. Mention the named arg, in case they didn't count the parens; handle the very contrived example where the param name and the tuple element name coincide.

@som-snytt som-snytt force-pushed the discussion/19072-infix-named-arg branch from 1d8838c to 4448c8b Compare October 22, 2024 22:25
@som-snytt
Copy link
Contributor Author

Should be a migration warning (not deprecation); should not warn on named tuple ops. Probably move the warning to typer (as I meant to do) (or I was thinking refchecks, but it needs to see infix ops).

@SethTisue SethTisue added the backport:nominated If we agree to backport this PR, replace this tag with "backport:accepted", otherwise delete it. label Nov 5, 2024
@SethTisue
Copy link
Member

SethTisue commented Nov 5, 2024

I've added the "backport:nominated" label, but note the discussion on #21826 about whether this change should even be considered eligible for backporting.

@som-snytt is this one ready for review? I guess it isn't, actually, because we need to decide on:

Should be a migration warning (not deprecation)

above, @odersky suggested deprecation, but I'm not sure if he had considered the "migration warning" option, or was just expressing a general "we should phase this out" opinion

the advantage of actually deprecating is that more users will see the warning

the disadvantage of actually deprecating is that some users will be annoyed

WojciechMazur added a commit that referenced this pull request Nov 18, 2024
…21949)

Best effort migration rewrites based on prior work in #21565  

At this point, it's too late to deprecate named infix arguments. Let's
produce warnings instead. We accept infix arguments that might be an
argument of a named tuple, eg. `zip`, `++` or `==` - each of these takes
a single argument with NamedTuple.

---------

Co-authored-by: Som Snytt <[email protected]>
Co-authored-by: Matt Bovel <[email protected]>
WojciechMazur added a commit that referenced this pull request Nov 18, 2024
…21949)

Best effort migration rewrites based on prior work in #21565  

At this point, it's too late to deprecate named infix arguments. Let's
produce warnings instead. We accept infix arguments that might be an
argument of a named tuple, eg. `zip`, `++` or `==` - each of these takes
a single argument with NamedTuple.

---------

Co-authored-by: Som Snytt <[email protected]>
Co-authored-by: Matt Bovel <[email protected]>
[Cherry-picked 5d5a9e6]
KacperFKorban pushed a commit to dotty-staging/dotty that referenced this pull request Nov 20, 2024
…cala#21949)

Best effort migration rewrites based on prior work in scala#21565  

At this point, it's too late to deprecate named infix arguments. Let's
produce warnings instead. We accept infix arguments that might be an
argument of a named tuple, eg. `zip`, `++` or `==` - each of these takes
a single argument with NamedTuple.

---------

Co-authored-by: Som Snytt <[email protected]>
Co-authored-by: Matt Bovel <[email protected]>
KacperFKorban pushed a commit to dotty-staging/dotty that referenced this pull request Nov 29, 2024
…cala#21949)

Best effort migration rewrites based on prior work in scala#21565  

At this point, it's too late to deprecate named infix arguments. Let's
produce warnings instead. We accept infix arguments that might be an
argument of a named tuple, eg. `zip`, `++` or `==` - each of these takes
a single argument with NamedTuple.

---------

Co-authored-by: Som Snytt <[email protected]>
Co-authored-by: Matt Bovel <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:nominated If we agree to backport this PR, replace this tag with "backport:accepted", otherwise delete it.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants