-
Notifications
You must be signed in to change notification settings - Fork 53
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
Issue with this in presence of self alias + & types #99
Merged
Merged
Changes from all commits
Commits
Show all changes
7 commits
Select commit
Hold shift + click to select a range
acb7683
Add test: Demonstrate issue with this in presence of self alias
OndrejSpanel 7ce707a
Add another self type test
KacperFKorban f6969ab
Fix self type modification
KacperFKorban 504a38f
Add tests for & types
KacperFKorban e875d50
Mostly working solution for & types
KacperFKorban ac964b8
Add an implementation limitation error for sealed hierarchies mixed w…
KacperFKorban fb15825
Fix wrong usage of copy$default$ methods
KacperFKorban File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
88 changes: 88 additions & 0 deletions
88
quicklens/src/test/scala-3/com/softwaremill/quicklens/test/ModifyAndTypeTest.scala
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,88 @@ | ||
package com.softwaremill.quicklens | ||
|
||
import org.scalatest.flatspec.AnyFlatSpec | ||
import org.scalatest.matchers.should.Matchers | ||
|
||
import ModifyAndTypeTest._ | ||
|
||
object ModifyAndTypeTest { | ||
case class A(a: Int) extends B | ||
trait B { | ||
def a: Int | ||
} | ||
|
||
case class A1(a: Int) | ||
|
||
sealed trait T | ||
case class C(a: Int) extends T with B | ||
|
||
sealed trait T1 | ||
case class C1(a: Int) extends T1 | ||
} | ||
|
||
class ModifyAndTypeTest extends AnyFlatSpec with Matchers { | ||
it should "modify an & type object" in { | ||
val ab: A & B = A(0) | ||
|
||
val modified = ab.modify(_.a).setTo(1) | ||
|
||
modified.a shouldBe 1 | ||
} | ||
|
||
it should "modify an & type object 1" in { | ||
val ab: B & A = A(0) | ||
|
||
val modified = ab.modify(_.a).setTo(1) | ||
|
||
modified.a shouldBe 1 | ||
} | ||
|
||
it should "modify an & type object 2" in { | ||
val ab: B & A1 = new A1(0) with B | ||
|
||
val modified = ab.modify(_.a).setTo(1) | ||
|
||
modified.a shouldBe 1 | ||
} | ||
|
||
it should "modify an & type object 3" in { | ||
val ab: A1 & B = new A1(0) with B | ||
|
||
val modified = ab.modify(_.a).setTo(1) | ||
|
||
modified.a shouldBe 1 | ||
} | ||
|
||
// TODO this is an implemenation limitation for now, since anonymous classes crash on runtime | ||
// it should "modify an & type object with a sealed trait" in { | ||
// val tb: T & B = C(0) | ||
|
||
// val modified = tb.modify(_.a).setTo(1) | ||
|
||
// modified.a shouldBe 1 | ||
// } | ||
|
||
// it should "modify an & type object with a sealed trait 1" in { | ||
// val tb: B & T = C(0) | ||
|
||
// val modified = tb.modify(_.a).setTo(1) | ||
|
||
// modified.a shouldBe 1 | ||
// } | ||
|
||
// it should "modify an & type object with a sealed trait 2" in { | ||
// val tb: B & T1 = new C1(0) with B | ||
|
||
// val modified = tb.modify(_.a).setTo(1) | ||
|
||
// modified.a shouldBe 1 | ||
// } | ||
|
||
// it should "modify an & type object with a sealed trait 3" in { | ||
// val tb: T1 & B = new C1(0) with B | ||
|
||
// val modified = tb.modify(_.a).setTo(1) | ||
|
||
// modified.a shouldBe 1 | ||
// } | ||
} |
41 changes: 41 additions & 0 deletions
41
quicklens/src/test/scala/com/softwaremill/quicklens/ModifySelfThisTest.scala
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,41 @@ | ||
package com.softwaremill.quicklens | ||
|
||
import org.scalatest.flatspec.AnyFlatSpec | ||
import org.scalatest.matchers.should.Matchers | ||
|
||
import ModifySelfThisTest._ | ||
|
||
object ModifySelfThisTest { | ||
|
||
case class State(x: Int) { self => | ||
|
||
def mod: State = this.modify(_.x).setTo(1) | ||
} | ||
|
||
trait A { | ||
def a: Unit | ||
} | ||
|
||
case class State1(x: Int) extends A { self: A => | ||
|
||
def mod: State1 = this.modify(_.x).setTo(1) | ||
|
||
def a: Unit = () | ||
} | ||
} | ||
|
||
class ModifySelfThisTest extends AnyFlatSpec with Matchers { | ||
it should "modify an object even in presence of self alias" in { | ||
val s = State(0) | ||
val modified = s.mod | ||
|
||
modified.x shouldBe 1 | ||
} | ||
|
||
it should "modify an object even in presence of self type" in { | ||
val s = State(0) | ||
val modified = s.mod | ||
|
||
modified.x shouldBe 1 | ||
} | ||
} |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
limitation of quicklens, Scala 3? If the latter, is there a dotty ticket maybe?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
quicklens
. I think.I'm not sure if there should be a dotty ticket for it since we're using
asInstanceOf
.Also, the problem occurs only for the last two tests - the ones with anonymous classes.
It's not hard to minimize, because we can just type the generated code by hand:
Scastie: https://scastie.scala-lang.org/KacperFKorban/s7A3euoTST2AoPYcTug8iA/5
Maybe there is some other way to write it so that it won't crash on runtime 🤔
The only weird thing is that it doesn't crash if we change the type ascription to the more specific one.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note: when I rewrite this using
with
instead of ¨&` and try it with Scala 2.13, it works: https://scastie.scala-lang.org/0vAFV0B8SDmZrjvbRj3jsAThe same code with
with
still does not work in Scala 3.To me this looks like a Scala 3 bug.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reported in scala/scala3#15952
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok so now that we know it's a quicklens not a dotty issue, can we do anything about it? :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't see an immediate solution. The problem is in the generated bytecode, so no matter what we do
ab.asInstanceOf[A].copy()
will have to be cast toT & B
. Which is impossible.Also if we want to have a discussion about it, maybe a new issue will be a better place for it 😃
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure :) #100