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

Tuple modification fails with Scala 3 #96

Merged
merged 3 commits into from
Aug 31, 2022

Conversation

OndrejSpanel
Copy link
Contributor

I have added unit tests which demonstrate tuple modification fails with Scala 3.

The test itself is very simple and works fine with Scala 2:

    val tuple4 = (0, 1, 2, 3)
    val modified = tuple4.modify(_._3).setTo(3)
    modified shouldBe(0, 1, 3, 3)

The error is:

Exception occurred while executing macro expansion.
java.util.NoSuchElementException: head of empty list
	at scala.collection.immutable.Nil$.head(List.scala:662)
	at scala.collection.immutable.Nil$.head(List.scala:661)
	at com.softwaremill.quicklens.QuicklensMacros$.termMethodByNameUnsafe$1(QuicklensMacros.scala:142)
	at com.softwaremill.quicklens.QuicklensMacros$.$anonfun$38$$anonfun$1(QuicklensMacros.scala:173)
	at scala.collection.immutable.Map$Map1.getOrElse(Map.scala:250)
	at com.softwaremill.quicklens.QuicklensMacros$.$anonfun$38(QuicklensMacros.scala:173)
	at com.softwaremill.quicklens.QuicklensMacros$.$anonfun$adapted$1(QuicklensMacros.scala:174)
	at scala.collection.immutable.Range.map(Range.scala:59)
	at com.softwaremill.quicklens.QuicklensMacros$.caseClassCopy$1(QuicklensMacros.scala:174)

I will try to learn more, or develop a fix, but I cannot promise when.

@OndrejSpanel
Copy link
Contributor Author

There is a difference between tuple and case class in caseFields. Case class with two items:

Fields of class Pair: List(val a, val b)

Tuple with two items:

Fields of class Tuple2: List(method _1, val _1 , method _2, val _2 )

Note: the name of the val field is not _1, but _1 (with a trailing space).

@KacperFKorban
Copy link
Collaborator

Maybe using primaryConstructor to find the index might be easier. I can push a possible fix here if you want.

@OndrejSpanel
Copy link
Contributor Author

I can push a possible fix here if you want.

Sure.

I was experimenting with caseClassCopy, but I was unable to find any working solution, and frankly, I had little idea what am I doing. I can help making the solution work / completing it if necessary (and if I am able to).

@KacperFKorban
Copy link
Collaborator

Couldn't push here, so created OndrejSpanel#6

@OndrejSpanel
Copy link
Contributor Author

Great, I have merged your commits here and all tests passed, therefore this looks promising. Passing for a review from higher powers.

@OndrejSpanel OndrejSpanel marked this pull request as ready for review August 31, 2022 14:22
@adamw adamw merged commit a546896 into softwaremill:master Aug 31, 2022
@adamw
Copy link
Member

adamw commented Aug 31, 2022

Thanks! :)

@OndrejSpanel OndrejSpanel deleted the pr-modify-tuple branch August 31, 2022 20:13
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.

3 participants