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

surface: Fix #3454 - methods in generic classes - for Scala 3 #3455

Merged
merged 3 commits into from
Mar 27, 2024

Conversation

OndrejSpanel
Copy link
Contributor

This PR implements a test and Scala 3 fix of #3454. The major part of the fix is a simplification of typeMappingTable function. I am not 100% sure what the previous implementation using method.paramSymss was trying to do, I only hope whatever it was, it had tests in place for that.

@xerial The new test fails on Scala 2 - if you or anyone is interested in completing the fix, feel free to continue.

@github-actions github-actions bot added the bug label Mar 18, 2024
@OndrejSpanel
Copy link
Contributor Author

OndrejSpanel commented Mar 22, 2024

Note: like #3466 this avoid using .tree and provides a simpler implementation of typeMappingTable. I have cherrypicked the typeMappingTable simplification into #3466 from here.

The rest (resolving method types) stays only here.

OndrejSpanel added a commit to OndrejSpanel/airframe that referenced this pull request Mar 22, 2024
OndrejSpanel added a commit to OndrejSpanel/airframe that referenced this pull request Mar 22, 2024
@xerial xerial changed the title Fix #3454 - methods in generic classes - for Scala 3 surface: Fix #3454 - methods in generic classes - for Scala 3 Mar 22, 2024
@xerial
Copy link
Member

xerial commented Mar 25, 2024

As we don't need to support everything for Scala 2 as well, we can put the test case under src/test/scala-3 folder to pass the CI

@OndrejSpanel
Copy link
Contributor Author

Rebased, test moved to Scala 3. Should be ready to go.

@OndrejSpanel OndrejSpanel marked this pull request as ready for review March 26, 2024 07:57
@xerial xerial merged commit 8e6d378 into wvlet:main Mar 27, 2024
15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants