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

fix: cast args when applying mockFunctionValDef in scala 3 #529

Merged
merged 1 commit into from
Sep 16, 2024

Conversation

hughsimpson
Copy link
Contributor

@hughsimpson hughsimpson commented Aug 4, 2024

Pull Request Checklist

  • I agree to licence my contributions under the MIT licence
  • I have added copyright headers to new files
  • I have added tests for any changed functionality

Fixes

Whilst mocking a trait or class with a method that took a type parameter worked in scala 3 for many cases, it did not work when the type parameter was used as the input for a function arg to that method. The newly-added test case failed in those instances (i.e. def p[T]) with the following:

[error] 43 |    val mockedTrait = mock[Foo]
[error]    |                               ^
[error]    |                               Found:    (t : Seq[Toot] => Seq[String])
[error]    |                               Required: Seq[?] => Seq[String]
[error]    |
[error]    | longer explanation available when compiling with `-explain`

This is a regression from scala 2

Purpose

This pr casts the args when passing them to the mock method implementation, which avoids the above error.

Background Context

This seemed the least intrusive way of getting the new test case to work


trait Foo {
def p[T <: Bar](gen: Seq[T], t: Seq[T] => Seq[String]): Seq[String] = t(gen)
def q[T <: Bar](gen: Seq[T]): Seq[String] = gen.map(_.toString)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This method (q) was already absolutely fine, it was only the above method p that caused compilation failures

Copy link
Contributor

@goshacodes goshacodes Sep 14, 2024

Choose a reason for hiding this comment

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

Looked one more time.
Lets add some more simple methods in this trait and also I would change names of method and trait so they would correspond to what we are trying to achieve.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure, should have time tomorrow

Copy link
Contributor Author

@hughsimpson hughsimpson Sep 15, 2024

Choose a reason for hiding this comment

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

Yeah thinking about it would be good to have some

def overloaded(gen: Seq[C1], t: Seq[C1] => Seq[String]): Seq[String] = t(gen)
def overloaded(gen: Seq[C2], t: Seq[C2] => Seq[String]): Seq[String] = t(gen)

stuff wouldn't it...

@barkhorn barkhorn added this to the v6.1.0 milestone Aug 10, 2024
@hughsimpson
Copy link
Contributor Author

@barkhorn anything I can/should do on this pr to help it on its way?

@barkhorn
Copy link
Collaborator

barkhorn commented Sep 7, 2024

hey, I think @goshacodes should review this. Happy to do the admin bits but i'm not that active on the project anymor

@goshacodes
Copy link
Contributor

goshacodes commented Sep 12, 2024

Hi, looks good, lets merge

@barkhorn barkhorn merged commit c254ce1 into paulbutcher:master Sep 16, 2024
3 checks passed
@hughsimpson hughsimpson deleted the otherMock branch September 21, 2024 08:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants