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

Experiment with dynamic call origin information #88

Draft
wants to merge 4 commits into
base: master
Choose a base branch
from
Draft

Conversation

bnorm
Copy link
Owner

@bnorm bnorm commented Apr 6, 2023

Experimenting with what an annotation driven form of kotlin-power-assert would look like, and what form the data structure would take for getting call source information.

@OptIn(ExperimentalContracts::class)
@Introspected
fun assertTrue(condition: Boolean, message: String? = null) {
  contract {
    returns() implies condition
  }

  if (!condition) {
    val diagram = CallOrigin.get()?.toSimpleDiagram()
    val errorMessage = when {
      message == null -> diagram
      diagram != null -> "$message:\n$diagram"
      else -> null
    }
    throw AssertionError(errorMessage)
  }
}

// Compiler plugin should generate the following function:

@JvmSynthetic
internal fun assertTrue__introspected(condition: Boolean, message: String? = null, callOrigin: CallOrigin) {
  if (!condition) {
    val diagram = callOrigin?.toSimpleDiagram() // CallOrigin.get() replaced with `callOrigin` parameter
    val errorMessage = when {
      message == null -> diagram
      diagram != null -> "$message:\n$diagram"
      else -> null
    }
    throw AssertionError(errorMessage)
  }
}

Goal is to handle as much boiler-plate as possible and not exposing the mechanics of how call-site information is passed. This also hopefully means that libraries which have @Introspected functions would only need to include the support library as a compileOnly dependency, since any calling code would also need to include the compiler plugin anyways.

@christophsturm
Copy link
Contributor

looks good. what do you think of a different name for @Introspected, which seems very generic. I like @Empowered

@TWiStErRob
Copy link
Contributor

Or keep it close to the lib... @PowerAssert?

@bnorm bnorm force-pushed the annotation branch 3 times, most recently from c9e8f1f to 7372943 Compare April 9, 2023 16:14
@bnorm
Copy link
Owner Author

bnorm commented Apr 9, 2023

I've been thinking about renaming this repository for a while since it is starting to handle so many use-cases outside of just asserts. Especially with this new annotation driven stuff, I see it as a chance to do some rebranding. So I'm hesitant to go with @PowerAssert.

I do like @Empowered, though. Fits with the power-assert name most people know this functionality under from various languages. It may be a good enough name to eventually rebrand the entire repository under as well.

But nothing is set in stone yet; naming ideas are very welcome for everything related to this PR!

@christophsturm
Copy link
Contributor

what about merging this to master and releasing it as an experimental feature for people to play with it?
as far as i can see it should not interfere with the current stable functionality.

@bnorm
Copy link
Owner Author

bnorm commented Apr 24, 2023

what about merging this to master and releasing it as an experimental feature for people to play with it? as far as i can see it should not interfere with the current stable functionality.

I plan to do that, but right now I've only created the constructs people will use and not implemented anything on the compiler plugin side of things yet. Still lots of work to do before this is useable.

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