-
-
Notifications
You must be signed in to change notification settings - Fork 792
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
Make DoubleMutabilityForCollection configurable and set a DoubleMutability alias #4541
Make DoubleMutabilityForCollection configurable and set a DoubleMutability alias #4541
Conversation
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.
Great stuff! 🚀
@@ -489,6 +530,26 @@ class DoubleMutabilityForCollectionSpec : Spek({ | |||
assertThat(result).hasSize(1) | |||
assertThat(result).hasSourceLocation(2, 5) | |||
} | |||
|
|||
it("detects var declaration with MutableState, when configured") { |
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.
Could we also add a test where you declare and use those functions:
fun <T : Any?> mutableStateOf(value: T?): MutableState<T>
fun <T : Any?> remember(calculation: (() -> T)?): T
So to make sure everything works correctly in a Compose scenario
@cortinico ready for re-review. Note, I also tried to add a negative test to validate that property delegates are ignored:
but got an error:
Any ideas? Seems like the property delegate breaks it. |
Please note that we're in the process of migrating tests from Spek to JUnit. Potentially adapting those tests would fix your failure |
Codecov Report
@@ Coverage Diff @@
## main #4541 +/- ##
===========================================
+ Coverage 0 84.11% +84.11%
- Complexity 0 3314 +3314
===========================================
Files 0 477 +477
Lines 0 10924 +10924
Branches 0 2029 +2029
===========================================
+ Hits 0 9189 +9189
- Misses 0 700 +700
- Partials 0 1035 +1035
Continue to review full report at Codecov.
|
b56155a
to
59a907d
Compare
Thank goodness, running Spek tests from within IntelliJ is a pain. Actually couldn't figure out a way to do it, even with the plugin.
It does indeed fix the problem. I've rebased this PR on the Junit branch and added the additional tests. |
NOTE: GitHub is not letting me set the junit branch as the base of this PR, so the junit commits are here too. |
You should now be able to rebase 👍 |
59a907d
to
fb661f5
Compare
Done |
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.
Great contrib 🙏 LGTM
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 really like this one! Thanks :)
Resolves #4501