Implement NonEmptyList using value class #2911

serras · 2023-01-30T15:41:42Z

After the discussion in #2899, I've tried to implement NonEmptyList using a value class wrapping over List. This means that most of List is implemented by delegation, instead of using AbstractList, and for those who need an overload returning NonEmptyList, the implementation now is just a call to the underlying List one.

Something we lose is good interoperability with Java. Method names in value classes as mangled, and this seems to happen even for extension methods. Furthermore, at runtime NonEmptyList is literally List, which can be noticed in the return type of methods like nonEmptyListOf.

Since the binary API changes, it may make sense to push this PR to arrow-2 instead of main.

nomisRev · 2023-01-30T15:48:28Z

Since the binary API changes, it may make sense to push this PR to arrow-2 instead of main.

Yes, I think that will be necessary indeed. Besides that I am in favour of this change ☺️ Thank you for looking into this 🙌

Method names in value classes as mangled, and this seems to happen even for extension methods.

I saw this happen in the api files before, but was never 100% sure where it came from but knew it was a stable binary name. Thanks for linking the relative Kotlin documentation!

github-actions · 2023-01-30T20:13:07Z

Kover Report

File	Coverage [50.94%]
arrow-libs/core/arrow-core/src/commonMain/kotlin/arrow/core/NonEmptyList.kt	50.94%

Total Project Coverage	43.16%

serras · 2023-01-31T10:06:28Z

I've rebased the PR against arrow-2, as agreed in the comments

nomisRev

Seems this PR brings back a couple constructors that were previously deprecated.

nomisRev · 2023-02-02T13:12:15Z

arrow-libs/core/arrow-core/src/commonMain/kotlin/arrow/core/NonEmptyList.kt

+    @JvmStatic @JvmName("of")
+    public fun <A> of(first: A, vararg tail: A): NonEmptyList<A> =
+      NonEmptyList(listOf(first) + tail)
+
+    @JvmStatic @JvmName("ofOrNull")
+    public fun <A> ofOrNull(list: Iterable<A>): NonEmptyList<A>? =
+      list.firstOrNull()?.let { NonEmptyList(list.toList()) }


Weren't these constructors deprecated in 1.x.x in favour of nonEmptyListOf to be in line with listOf etc from Kotlin Std?

These were added back so that they could be called from Java, because mangling make it otherwise impossible. We could also remove them.

🤔 I would be in favour to remove them. Optimising for Java seems strange, especially since on the Java side it just returns List anyway.

nomisRev · 2023-02-02T13:12:55Z

arrow-libs/core/arrow-core/src/commonMain/kotlin/arrow/core/NonEmptyList.kt

    public fun <A> fromList(l: List<A>): Option<NonEmptyList<A>> =
      if (l.isEmpty()) None else Some(NonEmptyList(l))


I thought these constructors were also deprecated in favour of toEmptyListOrNone() and arrow.core.toNonEmptyListOrNull.

nomisRev · 2023-02-02T13:13:22Z

arrow-libs/core/arrow-core/src/commonMain/kotlin/arrow/core/NonEmptyList.kt

+    public inline fun <A> fromListUnsafe(l: List<A>): NonEmptyList<A> =
      NonEmptyList(l)


Is there any good reason to keep this API? 🤔

I think it's important to give a escape hatch in case somebody "knows what there're doing". But they could always use the version which may return null and do !! afterwards.

But they could always use the version which may return null and do !! afterwards.

Yes, that is actually what I was thinking of instead of explicitly exposing an API for it. We don't expose any escape hatches anywhere else in Kotlin since you can always quite easily explicitly walk around it in the few cases you need to.

franciscodr · 2023-02-02T14:17:27Z

arrow-libs/core/arrow-core/src/jvmTest/java/arrow/core/NonEmptyListUsage.java

+        // List<Integer> flatten = flatten(nonEmptyListOf(
+        //         nonEmptyListOf(1, 2),
+        //         nonEmptyListOf(3, 4)
+        // ));


Should we remove these commented lines?

* The beginning of Arrow 2.0 * Resource Arrow 2.0 (#2786) * Flatten Resource ADT, maintain API * [Arrow 2.0] Effect without suspending shift (#2797) * Shift without suspend, inline all the rest * Add new error handlers signatures * Make ShiftCancellationException private * Rename Shift to Raise according to Slack Poll, and add some initial docs (#2827) * Remove all references to shift from new Arrow 2.0 code (#2834) * Remove all references to shift from new code * Update API files * Fixes merge conflict between main and arrow-2 (#2835) * Add Resource.allocated() to decompose Resource into it's allocate and… (#2820) * [2743] Migrate internal use of CircuitBreaker double to duration (#2748) * Fix typo (#2824) * Make the server disconnect test more general (#2822) * Update NonEmptyList.fromList deprecation to suggest `toOption()` instead (#2832) * Improve Either.getOrHandle() docs (#2833) * Improve allocated, and fix knit examples Co-authored-by: Jeff Martin <jeff@custommonkey.org> Co-authored-by: Martin Moore <martinmoorej@gmail.com> Co-authored-by: valery1707 <valery1707@gmail.com> Co-authored-by: Lukasz Kalnik <70261110+lukasz-kalnik-gcx@users.noreply.github.com> Co-authored-by: stylianosgakis <stylianos.gakis98@gmail.com> * Add Atomic module, and StateShift (#2817) * Two small deprecations * Add Atomic module, and StateShift. Implement ior through StateShift * Fix build * Fix atomic knit * Fix knit attempt #2 * Update API files * Remove references to shift * Change return type of raise to Nothing (#2839) * Change return type of raise to Nothing Implements #2805 * Update public api with ./gradlew apiDump * Suppress warnings for unreachable calls to fail in tests The test is verifying that no code will be executed after a call to raise, so it's actually testing the very behaviour that the compiler is warning us about. Co-authored-by: Alejandro Serrano <trupill@gmail.com> * Update API files * Increase timeout * Fix compiler bug with nested inline + while + return * Clean up ExitCase.fromError * Update API files@ * Feature/remove validated (#2795) Co-authored-by: Simon Vergauwen <nomisRev@users.noreply.github.com> Co-authored-by: Alejandro Serrano <trupill@gmail.com> * Remove CancellationExceptionNoTrace.kt after merge, and run apiDump * Add missing runnerJUnit5 for arrow-atomic JVM * Publish Arrow 2.0.0-SNAPSHOT (#2871) Co-authored-by: Javier Segovia Córdoba <7463564+JavierSegoviaCordoba@users.noreply.github.com> * Simplify optics to Traversal/Optional/Lens/Prism (#2873) * 'mapOrAccumulate' for Raise (#2872) Co-authored-by: Simon Vergauwen <nomisRev@users.noreply.github.com> * Fix problems with Atomic * Smaller timeouts * Remove Tuple10 to Tuple22, Const, Eval, computation blocks, and arrow-continuations (#2784) * Revert typo * Fix build * Fix ParMapJvmTest * Implement NonEmptyList using value class (#2911) * Fix merge w.r.t. Saga * apiDump * Test other return expression * change unalign signature (#2972) see discussion in #2960 (comment) #2960 (comment) --------- Co-authored-by: Alejandro Serrano <trupill@gmail.com> * Update after merge main * Fix :arrow-optics-ksp-plugin:compileKotlin * Fix Every instances * Move functions to arrow functions (#3014) * Bring back `Iso` (#3013) * Bring back Iso * API Dump --------- Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> * Update BOM (#3019) * Fix andThen duplication (#3020) * Fix Knit * Fix weird problem with value classes * Update API docs * Update publish workflow Following the instructions in #3090 (comment) * No closing repo on snapshot * knit * Fix optics tests * Fix after merge * Refactor ParMapTest from Kotest Plugin to Kotlin-test runtime #3191 (#3221) Co-authored-by: Simon Vergauwen <nomisRev@users.noreply.github.com> Co-authored-by: Alejandro Serrano <trupill@gmail.com> * Refactor: Use Kotlin-test runtime for arrow-fx-stm tests (#3224) Co-authored-by: Alejandro Serrano <trupill@gmail.com> * Update all Gradle files to mention kotlin.test * Refactor ParZip2Test from Kotest Plugin to Kotlin-test runtime #3192 (#3222) Co-authored-by: Alejandro Serrano <trupill@gmail.com> * Refactor ParZip3Test from Kotest Plugin to Kotlin-test runtime #3193 (#3223) Co-authored-by: Alejandro Serrano <trupill@gmail.com> * Refactor GuaranteeCaseTest to use kotlin test (#3226) Closes #3190 * refactor: migrate NotEmptySetTest to kotlin-test (#3230) Co-authored-by: Alejandro Serrano <trupill@gmail.com> * refactor: migrate EagerEffectSpec to kotlin-test (#3233) Co-authored-by: Alejandro Serrano <trupill@gmail.com> * Refactor NullableSpec from Kotest Plugin to Kotlin-test runtime (#3236) #3153 Co-authored-by: Alejandro Serrano <trupill@gmail.com> * Refactor BracketCaseTest to use kotlin test (#3237) Closes #3186 Co-authored-by: Alejandro Serrano <trupill@gmail.com> * Move arrow-functions tests to kotlin.test (#3243) * Inline `AtomicBoolean` (#3240) * Inline AtomicBoolean * Update API files --------- Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> * refactor: migrate MappersSpec to kotlin-test (#3248) * Refactor ResourceTestJvm from Kotest Plugin to Kotlin-test runtime (#3244) Closes #3213 * refactor: migrate FlowJvmTest to Kotlin-test (#3228) * Refactor ParZip9JvmTest from Kotest Plugin to Kotlin-test runtime (#3245) Closes #3211 * Refactor ParZip8JvmTest from Kotest Plugin to Kotlin-test runtime (#3246) Closes #3210 * refactor: migrate NumberInstancesTest to kotlin-test (#3232) * refactor: OptionTest to kotlin-test runtime (#3229) * Revert "Inline `AtomicBoolean` (#3240)" (#3279) This reverts commit a6f1e73. * Refactor ParZip6JvmTest from Kotest Plugin to Kotlin-test runtime (#3255) Closes #3208 * Refactor ParZip5JvmTest from Kotest Plugin to Kotlin-test runtime (#3256) Closes #3207 * Refactor ParZip3JvmTest from Kotest Plugin to Kotlin-test runtime (#3258) Closes #3205 * Refactor ParZip2JvmTest from Kotest Plugin to Kotlin-test runtime (#3259) Closes #3204 * Refactor ParMapJvmTest from Kotest Plugin to Kotlin-test runtime (#3260) Closes #3203 * Refactor ParZip4JvmTest from Kotest Plugin to Kotlin-test runtime (#3257) Closes #3206 * refactor: migrate RaiseAccumulateSpec to kotlin-test (#3250) * Refactor ParZip7JvmTest from Kotest Plugin to Kotlin-test runtime (#3247) Closes #3209 Co-authored-by: Alejandro Serrano <trupill@gmail.com> * Update ComparisonKtTest.kt (#3274) Co-authored-by: Simon Vergauwen <nomisRev@users.noreply.github.com> * Update OptionSpec.kt (#3271) Co-authored-by: Simon Vergauwen <nomisRev@users.noreply.github.com> * Update TraceJvmSpec.kt (#3276) * Update TraceJvmSpec.kt * Update TraceJvmSpec.kt --------- Co-authored-by: Simon Vergauwen <nomisRev@users.noreply.github.com> * Update ParZip9Test.kt (#3265) * Update ParZip9Test.kt * Update ParZip9Test.kt * Update ParZip8Test.kt (#3266) * Update ParZip7Test.kt (#3267) * Update ParZip6Test.kt (#3268) * Update ParZip5Test.kt (#3269) * Update ParZip4Test.kt (#3270) * Refactor CountDownLatchSpec and CyclicBarrierSpec to use kotlin test (#3227) * Refactor CountDownLatchSpec to use kotlin test Closes #3187 * Refactor CyclicBarrierSpec to use kotlin test Closes #3188 --------- Co-authored-by: Alejandro Serrano <trupill@gmail.com> * Refactor NonEmptyListTest to kotlin-test (#3231) * Add kotlin test dependency * Refactor NonEmptyList Test to use kotlin test --------- Co-authored-by: Alejandro Serrano <trupill@gmail.com> * refactor: migrate EffectSpec to kotlin-test (#3234) * Refactor FlowTest to use kotlin test (#3238) Closes #3189 Co-authored-by: Alejandro Serrano <trupill@gmail.com> * refactor: migrate IorSpec to kotlin-test (#3249) Co-authored-by: Simon Vergauwen <nomisRev@users.noreply.github.com> * refactor: migrate ResultSpec to kotlin-test (#3251) Co-authored-by: Simon Vergauwen <nomisRev@users.noreply.github.com> * refactor: migrate StructuredConcurrencySpec to kotlin-test (#3252) * refactor: migrate TraceSpec to kotlin-test (#3253) * refactor: migrate GeneratorsTest to kotlin-test (#3254) * Refactor RaceNJvmTest from Kotest Plugin to Kotlin-test runtime (#3261) Closes #3212 Co-authored-by: Alejandro Serrano <trupill@gmail.com> * Update ArrowResponseEAdapterTest.kt (#3264) * Update ArrowResponseEAdapterTest.kt * Update ArrowResponseEAdapterTest.kt --------- Co-authored-by: Simon Vergauwen <nomisRev@users.noreply.github.com> Co-authored-by: Alejandro Serrano <trupill@gmail.com> * Update CollectionsSyntaxTests.kt (#3273) * Update CollectionsSyntaxTests.kt * Update CollectionsSyntaxTests.kt --------- Co-authored-by: Simon Vergauwen <nomisRev@users.noreply.github.com> Co-authored-by: Alejandro Serrano <trupill@gmail.com> * Update NonFatalJvmTest.kt (#3277) * Update ArrowEitherCallAdapterTest.kt (#3278) * Update ArrowEitherCallAdapterTest.kt * Update ArrowEitherCallAdapterTest.kt * Move tests from `serialization` and `functions` completely to `kotlin.test` (#3289) Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> * Fix problems with tests * Remove a bunch of warnings in `arrow-2` (#3282) Co-authored-by: serras <serras@users.noreply.github.com> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> * Finish transition to `kotlin.test` of `retrofit` and `fx-coroutines` (#3291) * Fix problems with concurrency in tests, take 8 * Port rest of `arrow-core` to `kotlin.test` (#3292) Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> * Implement `fixedRate` using monotonic time source (#3294) Co-authored-by: serras <serras@users.noreply.github.com> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> * Port `optics` tests to `kotlin.test` (#3295) Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> * Add or fix contracts in Raise (#3293) * Add or fix contracts in Raise * Make contracts less strict * Get back contract on Either * ignoreErrors should also be AT_MOST_ONCE --------- Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> * Alternate `SingletonRaise` (#3328) Co-authored-by: Youssef Shoaib <canonballt@gmail.com> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: serras <serras@users.noreply.github.com> * Remove (unused) tests for high-arity module * Fix tests + Knit * Fix merge NullableSpec * Regression in Arb.list? * Fix test for nonEmptyList * Develocity warning * Fix merge problem with optics-ksp-plugin * Fix timeout in test --------- Co-authored-by: Simon Vergauwen <nomisRev@users.noreply.github.com> Co-authored-by: Sam Cooper <54266448+roomscape@users.noreply.github.com> Co-authored-by: Marc Moreno Ferrer <ignis.fatue@gmail.com> Co-authored-by: Javier Segovia Córdoba <7463564+JavierSegoviaCordoba@users.noreply.github.com> Co-authored-by: Alphonse Bendt <370821+abendt@users.noreply.github.com> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Andreas Storesund Madsen <andreas@asmadsen.no> Co-authored-by: molikuner <molikuner@gmail.com> Co-authored-by: Jonathan Lagneaux <lagneaux.j@gmail.com> Co-authored-by: Marcus Ilgner <mail@marcusilgner.com> Co-authored-by: Chris Black <2538545+chrsblck@users.noreply.github.com> Co-authored-by: Tejas Mate <hi.tejas@pm.me> Co-authored-by: HyunWoo Lee (Nunu Lee) <54518925+l2hyunwoo@users.noreply.github.com> Co-authored-by: serras <serras@users.noreply.github.com> Co-authored-by: Youssef Shoaib <canonballt@gmail.com>

serras requested a review from a team January 30, 2023 15:41

serras self-assigned this Jan 31, 2023

serras added the 2.0.0 Tickets / PRs belonging to Arrow 2.0 label Jan 31, 2023

serras and others added 3 commits January 31, 2023 11:01

Implement NonEmptyList using value class

b387e6e

Update API files

1f8ed01

Fix generator for NonEmptyList in optics

ece9c1b

serras force-pushed the as-nonemptylist-value-class branch from aa5c750 to ece9c1b Compare January 31, 2023 10:05

serras changed the base branch from main to arrow-2 January 31, 2023 10:05

serras mentioned this pull request Jan 31, 2023

Arrow 2.0 #2778

Merged

20 tasks

nomisRev reviewed Feb 2, 2023

View reviewed changes

franciscodr reviewed Feb 2, 2023

View reviewed changes

franciscodr approved these changes Feb 2, 2023

View reviewed changes

serras and others added 2 commits February 2, 2023 21:15

Apply suggestions

96b7f93

Update API files

a196e08

serras merged commit 1514b53 into arrow-2 Feb 3, 2023

serras deleted the as-nonemptylist-value-class branch February 3, 2023 14:32

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement NonEmptyList using value class #2911

Implement NonEmptyList using value class #2911

serras commented Jan 30, 2023 •

edited

Loading

nomisRev commented Jan 30, 2023

github-actions bot commented Jan 30, 2023 •

edited

Loading

serras commented Jan 31, 2023

nomisRev left a comment

nomisRev Feb 2, 2023

serras Feb 2, 2023

nomisRev Feb 2, 2023

nomisRev Feb 2, 2023

nomisRev Feb 2, 2023

serras Feb 2, 2023

nomisRev Feb 2, 2023

franciscodr Feb 2, 2023

		public fun <A> fromList(l: List<A>): Option<NonEmptyList<A>> =
		if (l.isEmpty()) None else Some(NonEmptyList(l))

		public inline fun <A> fromListUnsafe(l: List<A>): NonEmptyList<A> =
		NonEmptyList(l)

Implement NonEmptyList using value class #2911

Implement NonEmptyList using value class #2911

Conversation

serras commented Jan 30, 2023 • edited Loading

nomisRev commented Jan 30, 2023

github-actions bot commented Jan 30, 2023 • edited Loading

Kover Report

serras commented Jan 31, 2023

nomisRev left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

serras commented Jan 30, 2023 •

edited

Loading

github-actions bot commented Jan 30, 2023 •

edited

Loading