Skip to content

Support records in JavaParsers#16762

Merged
TheElectronWill merged 4 commits intoscala:mainfrom
TheElectronWill:parse-java-records
May 21, 2023
Merged

Support records in JavaParsers#16762
TheElectronWill merged 4 commits intoscala:mainfrom
TheElectronWill:parse-java-records

Conversation

@TheElectronWill
Copy link
Copy Markdown
Contributor

@TheElectronWill TheElectronWill commented Jan 24, 2023

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
Copy Markdown
Contributor Author

TheElectronWill commented Mar 7, 2023

Finished during the issue Spree of 07 March 2023 😃

@mbovel
Copy link
Copy Markdown
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
smarter
smarter reviewed Mar 20, 2023
View reviewed changes
smarter
smarter reviewed Mar 20, 2023
View reviewed changes
@TheElectronWill
Copy link
Copy Markdown
Contributor Author

TheElectronWill commented Mar 20, 2023 •
edited
Loading

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
Copy Markdown
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.

@TheElectronWill TheElectronWill requested a review from smarter March 24, 2023 08:42
smarter
smarter requested changes Mar 25, 2023
View reviewed changes
@ckipp01 ckipp01 marked this pull request as draft May 9, 2023 17:47
@TheElectronWill
Copy link
Copy Markdown
Contributor Author

TheElectronWill commented May 9, 2023

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

@TheElectronWill TheElectronWill requested a review from smarter May 9, 2023 19:17
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
Copy Markdown
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!

@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 enabled auto-merge May 21, 2023 11:15
@TheElectronWill TheElectronWill merged commit 4f3632f into scala:main May 21, 2023
@He-Pin
Copy link
Copy Markdown
Contributor

He-Pin commented May 21, 2023 •
edited
Loading

So this will be included in Scala 3.3.1?

@smarter
Copy link
Copy Markdown
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
Copy Markdown
Member

SethTisue commented Dec 5, 2023

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
Copy Markdown
Contributor

som-snytt commented Dec 5, 2023 •
edited
Loading

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