Skip to content

Conversation

hoc081098
Copy link
Contributor

Issue #104

This pull request introduces a new @UnsafeResultValueAccess annotation to enforce safer access patterns for Result.value and Result.error in the kotlin-result library. Additionally, it updates the Kotlin conventions to opt into this annotation. Below are the key changes:

Enhancements to Result class safety:

  • Added the @UnsafeResultValueAccess annotation to mark access to Result.value and Result.error as potentially unsafe unless the Result state is explicitly checked using Result.isOk or Result.isErr. This encourages safer usage patterns. (kotlin-result/src/commonMain/kotlin/com/github/michaelbull/result/Result.kt, [1] [2]

Kotlin conventions update:

  • Updated the Kotlin conventions in kotlin-conventions.gradle.kts to opt into the @UnsafeResultValueAccess annotation, ensuring that projects using this configuration can leverage the new safety mechanism. (buildSrc/src/main/kotlin/kotlin-conventions.gradle.kts, buildSrc/src/main/kotlin/kotlin-conventions.gradle.ktsR62)

@michaelbull
Copy link
Owner

This is a great idea! Can you show me what it looks like in IntelliJ when you try to use the .value and .error fields without explicitly adding the annotation? Does it cause a compilation error or is it a warning?

@hoc081098
Copy link
Contributor Author

This is a great idea! Can you show me what it looks like in IntelliJ when you try to use the .value and .error fields without explicitly adding the annotation? Does it cause a compilation error or is it a warning?

Yes 🙏

Screenshot 2025-04-28 at 22 07 47

And when running :example:compileKotlin gradle task:

value_or_error

@michaelbull
Copy link
Owner

That's a really nice solution. Is there a way that we can point users to a compiler flag that turns this annotation off, such that we don't break all their existing code if they aren't ready to adopt it?

@hoc081098
Copy link
Contributor Author

That's a really nice solution. Is there a way that we can point users to a compiler flag that turns this annotation off, such that we don't break all their existing code if they aren't ready to adopt it?

We absolutely want to avoid breaking existing code for users who aren't ready to adopt this yet. According to Kotlin's opt-in documentation, we can point users to explicitly opt in by annotating their usage with @OptIn(UnsafeResultValueAccess::class), or configure it globally via Gradle.

We'll include this in the changelog and provide guidance on both approaches so users can adopt the change at their own pace.

@michaelbull
Copy link
Owner

That's great - I really appreciate the detailed explanation and how much thought you've put into this. I have to investigate the build failures on GitHub Actions before I can approve the workflow to be built.

@michaelbull michaelbull force-pushed the master branch 2 times, most recently from 9133ba7 to 260d2c2 Compare August 2, 2025 23:25
@michaelbull
Copy link
Owner

Merged in db45c67.

I've separated the access of value and error into separate annotations, so that their error messages are more specific and detailed to the consumer.

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.

2 participants