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.

Sign up for GitHub

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

Jump to bottom

Support records in JavaParsers #16762

Merged
merged 4 commits into from May 21, 2023
Merged

Support records in JavaParsers #16762

merged 4 commits into from May 21, 2023
+208 −20

Conversation

TheElectronWill
Copy link
Member

This is a port of scala/scala#9551.

Fixes #14846.

@TheElectronWill TheElectronWill force-pushed the parse-java-records branch 2 times, most recently from 6ec3616 to cb56936 Compare March 7, 2023 16:04
@TheElectronWill TheElectronWill changed the title [WIP] Support records in JavaParsers Support records in JavaParsers Mar 7, 2023
@TheElectronWill TheElectronWill force-pushed the parse-java-records branch 2 times, most recently from c97685d to a9e705a Compare March 7, 2023 18:21
@TheElectronWill TheElectronWill marked this pull request as ready for review March 7, 2023 18:22
@TheElectronWill
Copy link
Member Author

Finished during the issue Spree of 07 March 2023 😃

@mbovel
Copy link
Member

mbovel commented Mar 20, 2023

What is the state of this PR? Does it need to be reviewed?

@mbovel mbovel mentioned this pull request Mar 20, 2023
Closed
@TheElectronWill
Copy link
Member Author

TheElectronWill commented Mar 20, 2023 •
edited

What is the state of this PR? Does it need to be reviewed?

Yes, it's ready to be reviewed, please :)
It should solve the issue, as shown in the tests.

Sorry, I forgot to assign someone.
Speaking of that, who should I have chosen? Do we assign all the Spree PRs to you?

@mbovel
Copy link
Member

mbovel commented Mar 20, 2023

Speaking of that, who should I have chosen? Do we assign all the Spree PRs to you?

Nop, I think for this one @smarter is appropriate.

@ckipp01 ckipp01 marked this pull request as draft May 9, 2023 17:47
@TheElectronWill
Copy link
Member Author

Thanks for the bump @ckipp01! That was long overdue 😅
I've resolved the pending comments 😉

TheElectronWill and others added 3 commits May 9, 2023 21:17
@TheElectronWill
Support records in JavaParsers
68e720c 
This is a port of scala/scala#9551
@TheElectronWill @smarter
Fix record's constructor
ed126d9 
Co-authored-by: Guillaume Martres <smarter@users.noreply.github.com>
@TheElectronWill @smarter
Add more record tests and only run them on java >= 16
d1df3ee 
Co-authored-by: Guillaume Martres <smarter@users.noreply.github.com>
@ckipp01 ckipp01 assigned smarter and unassigned TheElectronWill May 9, 2023
@ckipp01 ckipp01 marked this pull request as ready for review May 9, 2023 19:22
smarter
smarter approved these changes May 18, 2023
View reviewed changes
Copy link
Member

@smarter smarter left a comment

Choose a reason for hiding this comment

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

Otherwise LGTM, thanks!

compiler/src/dotty/tools/dotc/parsing/JavaParsers.scala Outdated Show resolved Hide resolved
@smarter smarter assigned TheElectronWill and unassigned smarter May 18, 2023
@TheElectronWill @smarter
Remove synthetic record constructor if the user has written one manually
da4996a 
Co-authored-by: Guillaume Martres <smarter@users.noreply.github.com>
@TheElectronWill TheElectronWill merged commit 4f3632f into scala:main May 21, 2023
15 checks passed
@He-Pin
Copy link

He-Pin commented May 21, 2023 •
edited

So this will be included in Scala 3.3.1?

@smarter
Copy link
Member

smarter commented May 21, 2023

Yes, it will also be available for testing in tomorrow's nightly (see https://repo1.maven.org/maven2/org/scala-lang/scala3-compiler_3 for nightly version numbers)

@smarter smarter added the release-notes Should be mentioned in the release notes label May 21, 2023
@Kordyjan Kordyjan added this to the 3.3.1 milestone Aug 1, 2023
This was referenced Aug 21, 2023
Closed
Merged
@SethTisue
Copy link
Member

I have reported @misnupur to GitHub, but I lack sufficient permissions in this repo to delete their comments or block them from the lampepfl org.

@som-snytt
Copy link
Contributor

som-snytt commented Dec 5, 2023 •
edited

My blocking comment:

This is the first user I've blocked, and github doesn't offer enough tools to deal with bad actors.

Edit: by "bad actor", I don't mean Keanu in Much Ado About Nothing (1993) because compare The Matrix (1999) or John Wick (2014).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@som-snytt som-snytt som-snytt left review comments

@smarter smarter smarter approved these changes

Assignees

@TheElectronWill TheElectronWill

Labels
release-notes Should be mentioned in the release notes
Projects
None yet
Milestone
3.3.1
Development

Successfully merging this pull request may close these issues.

Support parsing record classes in Java source files
7 participants
@TheElectronWill @mbovel @He-Pin @smarter @SethTisue @som-snytt @Kordyjan