Skip to content

Conversation

stabbylambda
Copy link
Contributor

@stabbylambda stabbylambda commented Jun 18, 2022

What Changed

Over the last week, @phubbard and I spent some time converting things to Kotlin Poet. The result is infinitely better than the previous string concatenation code. We were able to reduce some of the duplication between the Sync and Async versions of various hooks, the code isn't as brittle, and it's all using real type references from KSP instead of just hoping the Strings work out.

Todo:

  • SyncHook
  • The rest of the hooks
  • Delete all the godawful string manipulation code

📦 Published PR as canary version: 0.12.1-canary.28.294-SNAPSHOT

@stabbylambda stabbylambda changed the base branch from ksp to main June 22, 2022 20:12
@stabbylambda stabbylambda changed the title Add some KotlinPoet Code generation Switch to KotlinPoet Code Generation Jun 22, 2022

import com.google.devtools.ksp.getVisibility
import com.google.devtools.ksp.symbol.*
import com.intuit.hooks.plugin.ksp.text
Copy link
Collaborator

Choose a reason for hiding this comment

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

When I was migrating from meta -> KSP in the first place, I tried to pull out all the intermediate data into pure codegen constructs, rather than a dependence on the modeling framework. This would ensure the codegen code is truly agnostic and you could potentially use the codegen module without needing the KSP part. i.e. if we want to use another processing framework (live javax.processing), or change the actual way we represent the DSL, or generate hooks via CLI.

I would like to try to continue following the strict separation of concerns. They can certainly be updated from simple String representations to whatever poet representation works best (TypeName, etc.).

Thoughts?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We discussed this earlier, but just putting this here for posterity. Today, we have multiple areas where we reach into the type resolver to get the information necessary to codegen the new class. When @sugarmanz looked at this, some of that was happening in the codegen, some in the validator, some in the processor. Instead what we discussed was making the whole pipeline a bit cleaner by changing it to be:

  1. process KSP -> Poet
  2. validate Poet constructs
  3. code generate from Poet constructs

That way any potential frontend (we're not switching away from KSP anytime soon) just has to convert to KotlinPoet classes.

@stabbylambda stabbylambda mentioned this pull request Jun 28, 2022
3 tasks
@sugarmanz sugarmanz merged commit 44e6650 into intuit:main Jul 1, 2022
@stabbylambda stabbylambda deleted the poet branch July 1, 2022 17:47
@sugarmanz
Copy link
Collaborator

🚀 PR was released in v0.12.1 🚀

@sugarmanz sugarmanz added the released This issue/pull request has been released. label Jul 9, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

released This issue/pull request has been released.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants