Skip to content
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

@DisallowLambdaCapture checker example #22

Open
wants to merge 11 commits into
base: main
Choose a base branch
from

Conversation

raulraja
Copy link
Member

@raulraja raulraja commented Jan 4, 2023

The @DisallowLambdaCapture macro is an annotation that can be applied to functions. It checks that functions annotated with @DisallowLambdaCapture are not called inside a non-inline anonymous function that captures variables. If it finds such a call, it will report an error with a message specified in the @DisallowLambdaCapture annotation or a default message if the msg parameter is not specified.

To implement this macro, the DisallowLambdaCapture companion object extends the Meta.Checker.Expression interface and overrides the check method. If present, this method first retrieves the msg argument of the DisallowLambdaCapture annotation. It then filters the declarations in the current scope to find any non-inline anonymous functions and checks if any of these functions capture variables. If it finds a function that captures variables and the function being checked is annotated with DisallowLambdaCapture, it reports an error with the specified message.

The DisallowLambdaCapture macro can be helpful in cases where you want to ensure that certain functions are only called in a context where variable capture is not allowed. This can be useful for avoiding unintended side effects or enforcing a certain code style. For example, you might use this macro to disallow calling functions that modify the global state inside anonymous functions to ensure that the state changes are properly isolated and not leaked through the lambda capture.

package foo.bar

import arrow.meta.samples.DisallowLambdaCapture

interface Raise<in E> {
  @DisallowLambdaCapture("It's unsafe to capture `raise` inside non-inline anonymous functions") fun raise(e: E): Nothing
}

context(Raise<String>)
fun shouldNotCapture(): () -> Unit {
  return { <!UnsafeCaptureDetected!>raise("boom")<!> }
}

context(Raise<String>)
fun inlineCaptureOk(): Unit {
  listOf(1, 2, 3).map { raise("boom") }
}

context(Raise<String>)
fun leakedNotOk(): () -> Unit = {
  listOf(1, 2, 3).map { <!UnsafeCaptureDetected!>raise("boom")<!> }
}

context(Raise<String>)
fun ok(): Unit {
  raise("boom")
}

# Conflicts:
#	arrow-reflect-annotations/src/main/kotlin/arrow/meta/Meta.kt
#	arrow-reflect-annotations/src/main/kotlin/arrow/meta/MetaContext.kt
#	arrow-reflect-annotations/src/main/kotlin/arrow/meta/TemplateCompiler.kt
#	arrow-reflect-compiler-plugin/src/main/kotlin/arrow/reflect/compiler/plugin/fir/checkers/FirMetaAdditionalCheckersExtension.kt
#	arrow-reflect-compiler-plugin/src/main/kotlin/arrow/reflect/compiler/plugin/fir/transformers/FirMetaTransformer.kt
# Conflicts:
#	arrow-reflect-annotations/src/main/kotlin/MetaModule.kt
#	arrow-reflect-compiler-plugin/src/main/kotlin/arrow/reflect/compiler/plugin/fir/checkers/FirMetaAdditionalCheckersExtension.kt
#	arrow-reflect-compiler-plugin/src/testGenerated/arrow/reflect/compiler/plugin/runners/DiagnosticTestGenerated.java
Copy link
Member

@nomisRev nomisRev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, but I am missing expertise to review all changes in this PR.


@Meta
@Target(AnnotationTarget.FUNCTION)
annotation class DisallowLambdaCapture(val msg: String = "") {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would this not be more precise as msg: String??

Comment on lines +30 to +32
contract {
callsInPlace(f, InvocationKind.EXACTLY_ONCE)
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure where this is checked in DisallowLambdaCapture.kt? 🤔

@JavierSegoviaCordoba
Copy link
Member

I will review it too, sorry I miss it!

@what-the-diff
Copy link

what-the-diff bot commented Jan 29, 2023

  • Added a new annotation DisallowLambdaCapture
  • Updated the meta checker to support checking for calls to annotated functions and not just declarations with annotations
  • Added tests that demonstrate how this works in practice (see capture_test)

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.

None yet

3 participants