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

Support LambdaType and MatchType #1801

Merged
merged 5 commits into from Feb 17, 2024

Conversation

mrdziuban
Copy link
Contributor

Fixes #1766

@bjaglin I still haven't been able to reproduce the error through the test inputs and I'm not sure how to test this otherwise. It compiles, but I have no idea if it prints the correct thing for each type.

@bjaglin
Copy link
Collaborator

bjaglin commented Jul 1, 2023

Thanks for putting this together!

Sorry for not following-up on #1766 (comment). Taking a deeper look at the stack trace, it looks like I was too fast suggesting a testkit test (apologies), as it's actually only a specific Scalafix API that is affected:

  at scalafix.internal.v0.LegacySemanticdbIndex.synthetics(LegacySemanticdbIndex.scala:52)

So we need an integration test, probably based on BaseSemanticSuite. The problem is that it's still coupled to Scala 2 at the moment (while these 2 new types are only expressable in Scala 3), so you won't be able to craft them.

So there is a bit of testing infrastructure refactoring to be done in order to test /see the effect of this properly. I don't really have an idea how to do it at the moment, I will most likely get to it in the coming weeks.

So I'd rather put this on hold for now. In the meantime, happy to review your proposal if you have some time to explore how the BaseSemanticSuite hierarchy can be refactored to allow scala 3 tests in a decent way.

@bjaglin bjaglin marked this pull request as draft July 1, 2023 00:19
@bjaglin bjaglin changed the title Add cases to LegacyCodePrinter for LambdaType and MatchType Add cases for LambdaType and MatchType Sep 10, 2023
@bjaglin bjaglin force-pushed the print-lambda-and-match-types branch 2 times, most recently from bee0baf to d4cd5f9 Compare September 11, 2023 00:35
@bjaglin bjaglin changed the title Add cases for LambdaType and MatchType Support LambdaType and MatchType Sep 11, 2023
@bjaglin
Copy link
Collaborator

bjaglin commented Sep 11, 2023

I pushed to this branch to patch other places missing support, hoping I could rush the whole thing for Scalafix 0.11.1, but I am not confident at all about them: the few BaseSemanticSuite tests I managed to get running locally still don't exercise the new code, so I will need to take a deeper look in the coming days.

@mrdziuban
Copy link
Contributor Author

@bjaglin have you had a chance to revisit this? Not being able to run scalafix on scala 3.3.1 is one of the last things holding me back from dropping scala 2 in my project so I'm eager to get it resolved 🙂 let me know if there's anything I can do to help!

@tanishiking
Copy link
Contributor

tanishiking commented Nov 17, 2023

I would take a look this when I have time hopefully within this year 🤞 since I introduced the upstream changes (LambdaType to semanticdb + Scala3).

@mrdziuban
Copy link
Contributor Author

Hi @bjaglin and @tanishiking, do you have any update on when this might be resolved? Let me know if I can help, it's pretty much the only thing stopping me from committing fully to Scala 3

@bjaglin
Copy link
Collaborator

bjaglin commented Feb 13, 2024

@mrdziuban sorry for the silence. I will tackle it this Friday, most likely in time for the upcoming release next week (pending Scala 2.x patch releases)

@bjaglin
Copy link
Collaborator

bjaglin commented Feb 16, 2024

@mrdziuban I have spent hours trying to create source code that results in a LambdaType referenced in synthetics (which seems to be your scenario looking at your stack trace), but I did not manage. The current (v1) API is now fixed/tested in this PR, but I can only guess that the deprecated (v0) one exercised by Scaluzzi is as well.

Any chance you could minimize your problem so we can be sure the fix is right?

@mrdziuban
Copy link
Contributor Author

@bjaglin thank you for looking! Definitely, I was able to minimize it here: https://github.com/mrdziuban/scalafix-issue-1801

@bjaglin
Copy link
Collaborator

bjaglin commented Feb 16, 2024

@mrdziuban Great! I'll integrate as a test tonight

@bjaglin bjaglin marked this pull request as ready for review February 17, 2024 00:15
@bjaglin bjaglin merged commit 0c3e52d into scalacenter:main Feb 17, 2024
8 checks passed
@bjaglin
Copy link
Collaborator

bjaglin commented Feb 17, 2024

Thanks again for the PR @mrdziuban! The test I added in the last commit confirmed the first commit of yours was enough for your use-case. My other commits handle other code paths that you would have most likely ran into sooner or later.

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.

MatchError on LambdaType in scala 3.3.1-RC1
3 participants