Remove a bunch of warnings in `arrow-2` #3282

serras · 2023-11-04T11:22:25Z

This removes every warning in main source sets, and most in the Knit examples and tests.

github-actions · 2023-11-04T11:32:43Z

Kover Report

File	Coverage [63.69%]
arrow-libs/core/arrow-core-retrofit/src/main/kotlin/arrow/retrofit/adapter/either/EitherCallAdapterFactory.kt	60.61%
arrow-libs/core/arrow-core/src/commonMain/kotlin/arrow/core/Either.kt	58.42%
arrow-libs/core/arrow-core/src/commonMain/kotlin/arrow/core/NonEmptyList.kt	72.97%
arrow-libs/core/arrow-core/src/commonMain/kotlin/arrow/core/NonFatal.kt	100.00%
arrow-libs/core/arrow-core/src/commonMain/kotlin/arrow/core/Sequence.kt	64.29%
arrow-libs/core/arrow-core/src/commonMain/kotlin/arrow/core/map.kt	86.19%
arrow-libs/core/arrow-core/src/commonMain/kotlin/arrow/core/nonFatalOrThrow.kt	100.00%
arrow-libs/core/arrow-core/src/commonMain/kotlin/arrow/core/predef.kt	100.00%
arrow-libs/core/arrow-core/src/commonMain/kotlin/arrow/core/raise/Effect.kt	100.00%
arrow-libs/core/arrow-core/src/commonMain/kotlin/arrow/core/raise/ErrorHandlers.kt	100.00%
arrow-libs/core/arrow-core/src/commonMain/kotlin/arrow/core/raise/Raise.kt	96.30%
arrow-libs/fx/arrow-fx-coroutines/src/commonMain/kotlin/arrow/fx/coroutines/Bracket.kt	55.56%
arrow-libs/fx/arrow-fx-coroutines/src/commonMain/kotlin/arrow/fx/coroutines/ParZip.kt	88.24%
arrow-libs/fx/arrow-fx-coroutines/src/commonMain/kotlin/arrow/fx/coroutines/ParZipOrAccumulate.kt	0.00%
arrow-libs/fx/arrow-fx-coroutines/src/jvmMain/kotlin/arrow/fx/coroutines/ResourceExtensions.kt	55.56%
arrow-libs/fx/arrow-fx-stm/src/commonMain/kotlin/arrow/fx/stm/STM.kt	86.21%
arrow-libs/fx/arrow-fx-stm/src/commonMain/kotlin/arrow/fx/stm/TArray.kt	28.57%
arrow-libs/fx/arrow-fx-stm/src/commonMain/kotlin/arrow/fx/stm/TVar.kt	73.08%
arrow-libs/fx/arrow-fx-stm/src/commonMain/kotlin/arrow/fx/stm/internal/Hamt.kt	93.98%
arrow-libs/fx/arrow-fx-stm/src/commonMain/kotlin/arrow/fx/stm/internal/Impl.kt	91.03%
arrow-libs/optics/arrow-optics/src/commonMain/kotlin/arrow/optics/Optional.kt	29.79%
arrow-libs/optics/arrow-optics/src/commonMain/kotlin/arrow/optics/Traversal.kt	4.31%
arrow-libs/optics/arrow-optics/src/commonMain/kotlin/arrow/optics/predef.kt	0.00%
arrow-libs/resilience/arrow-resilience/src/commonMain/kotlin/arrow/resilience/Saga.kt	89.58%

Total Project Coverage	63.56%

JavierSegoviaCordoba · 2023-11-04T12:31:23Z

arrow-libs/core/arrow-atomic/build.gradle.kts

-    freeCompilerArgs = freeCompilerArgs + "-Xexpect-actual-classes"
-  }
+tasks.withType<KotlinCompile> {
+  kotlinOptions.freeCompilerArgs += "-Xexpect-actual-classes"
 }



I highly recommend to keep the configureEach

Suggested change

tasks.withType<KotlinCompile>.configureEach {

kotlinOptions.freeCompilerArgs += "-Xexpect-actual-classes"

}

Any particular reason for this? (Gradle is all magic to me)

Yep, configureEach is lazy, the other isn't. If I remember correctly, it is calling all under the hood.

I hope they mark all of those APIs as deprecated in Gradle 9.

JavierSegoviaCordoba · 2023-11-04T12:32:40Z

arrow-libs/core/arrow-core/build.gradle.kts

+tasks.withType<KotlinCompile> {
+  kotlinOptions.freeCompilerArgs += "-Xexpect-actual-classes"
+}
+


Same as above

Suggested change

tasks.withType<KotlinCompile> {

kotlinOptions.freeCompilerArgs += "-Xexpect-actual-classes"

}

tasks.withType<KotlinCompile>.configureEach {

kotlinOptions.freeCompilerArgs += "-Xexpect-actual-classes"

}

nomisRev · 2023-11-07T17:28:10Z

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

@@ -179,7 +179,7 @@ public value class NonEmptyList<out A> @PublishedApi internal constructor(
    else -> head
  }

-  @Suppress("OVERRIDE_BY_INLINE")
+  @Suppress("OVERRIDE_BY_INLINE", "NOTHING_TO_INLINE")


@serras, so is this is actually inlining?

I know Kotlin Std has @InlineOnly, and afaik that forces inlining and removes the method from the binary. At least that's my understanding. So I am wondering, if that exists and my understanding is correct does the compiler ignore this inline or does it still forces inline (but keep it in the binary?)?

I'm not sure. The error I was getting was "performance of inline is not significant" (even though the error is called "nothing to inline"). I think we should keep those inline, it feels like each step is not very significant, but in a big code this all adds up.

nomisRev · 2023-11-07T17:33:28Z

arrow-libs/fx/arrow-fx-coroutines/src/commonMain/kotlin/arrow/fx/coroutines/Bracket.kt

-  fa: suspend () -> A,
+  fa: () -> A,


I think this is still needed.. Or what I've used to hit in the passed was a compiler bug ... 😅 Perhaps I should've suppressed with a comment at the time.

IIRC this is needed for method references, we can potentially also add a test so we can not accidentally break this in the future unless this was fixed in the compiler. For the old cases of:

onCancel(::awaitCancellation) { ... }

This doesn't compile without fa: suspend () -> A.

It definitely works now (at least in the examples and tests). The main choice for this removal was the compiler telling me that suspend was not super-great in an inline function. But we can bring it back if you feel this is breaking the way the library should be used.

nomisRev · 2023-11-07T17:37:31Z

...-libs/fx/arrow-fx-coroutines/src/commonMain/kotlin/arrow/fx/coroutines/ParZipOrAccumulate.kt

@@ -14,44 +14,44 @@ public suspend inline fun <E, A, B, C> Raise<E>.parZipOrAccumulate(
  crossinline combine: (E, E) -> E,
  crossinline fa: suspend ScopedRaiseAccumulate<E>.() -> A,
  crossinline fb: suspend ScopedRaiseAccumulate<E>.() -> B,
-  crossinline f: suspend CoroutineScope.(A, B) -> C
+  crossinline transform: suspend CoroutineScope.(A, B) -> C


nomisRev · 2023-11-07T17:40:10Z

arrow-libs/optics/arrow-optics/src/commonMain/kotlin/arrow/optics/Traversal.kt

@@ -195,11 +195,11 @@ public interface PTraversal<S, T, A, B> {
      set: (B, B, S) -> T
    ): PTraversal<S, T, A, B> =
      object : PTraversal<S, T, A, B> {
-        override fun <R> foldMap(initial: R, combine: (R, R) -> R, s: S, f: (focus: A) -> R): R =
-          combine(f(get1(s)), f(get2(s)))
+        override fun <R> foldMap(initial: R, combine: (R, R) -> R, source: S, map: (focus: A) -> R): R =


Sorry, this is a bit nitpicky but do we have a rule of thumb for map vs transform?

Not really... in this case the change was motivated by keeping the naming of all subclasses the same as the parent class.

Personally, I think that Kotlin has implicitly made a choice of calling the "function applied" argument transform. Should we change the name in optics?

nomisRev

Amazing work Ale!! 👏 👏 👏 🙌
I'd only like to know your opinion on the suspend for method references, and then I think we can merge ☺️

* 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>

Remove a bunch of warnings

ad08c57

serras requested review from nomisRev and a team November 4, 2023 11:22

serras changed the title ~~Remove a bunch of warnings~~ Remove a bunch of warnings in arrow-2 Nov 4, 2023

Update API files

5c81bd1

JavierSegoviaCordoba reviewed Nov 4, 2023

View reviewed changes

serras and others added 6 commits November 5, 2023 08:11

Update build.gradle.kts

e471811

Update build.gradle.kts

542d6b0

Update build.gradle.kts

09ac5b6

Update build.gradle.kts

6da6e17

Branch was auto-updated.

1bc5581

Branch was auto-updated.

aa35ab1

nomisRev reviewed Nov 7, 2023

View reviewed changes

github-actions bot and others added 9 commits November 8, 2023 17:11

Branch was auto-updated.

b9c7ab0

Branch was auto-updated.

a139f36

Branch was auto-updated.

54a980a

Branch was auto-updated.

2cac2d1

Merge branch 'arrow-2' into serras/fewer-warnings

002e742

Merge branch 'arrow-2' into serras/fewer-warnings

d4eeaa4

No warnings in Gradle files

cf4ea7a

Rollback changes in Bracket

dd4960d

API Dump

024ca03

serras merged commit 0c39851 into arrow-2 Nov 8, 2023
1 of 2 checks passed

serras deleted the serras/fewer-warnings branch November 8, 2023 19:35

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Remove a bunch of warnings in `arrow-2` #3282

Remove a bunch of warnings in `arrow-2` #3282

serras commented Nov 4, 2023

github-actions bot commented Nov 4, 2023

JavierSegoviaCordoba Nov 4, 2023 •

edited

Loading

serras Nov 4, 2023

JavierSegoviaCordoba Nov 4, 2023

JavierSegoviaCordoba Nov 4, 2023

serras Nov 5, 2023

nomisRev Nov 7, 2023

serras Nov 7, 2023

nomisRev Nov 7, 2023

serras Nov 7, 2023

nomisRev Nov 7, 2023

nomisRev Nov 7, 2023

serras Nov 7, 2023

nomisRev left a comment

Remove a bunch of warnings in arrow-2 #3282

Remove a bunch of warnings in arrow-2 #3282

Conversation

serras commented Nov 4, 2023

github-actions bot commented Nov 4, 2023

Kover Report

JavierSegoviaCordoba Nov 4, 2023 • edited Loading

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nomisRev left a comment

Choose a reason for hiding this comment

Remove a bunch of warnings in `arrow-2` #3282

Remove a bunch of warnings in `arrow-2` #3282

JavierSegoviaCordoba Nov 4, 2023 •

edited

Loading