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

Add finer-grained validation for determining the symbols to defer during symbol processing #3559

Merged
merged 1 commit into from
Jan 10, 2025

Conversation

FWest98
Copy link
Contributor

@FWest98 FWest98 commented Jan 7, 2025

This addresses an issue introduced here: #3537 (comment)

Essentially, at the moment, processing for any candidate class for optics is "deferred" when "validation" fails - essentially when any type in the class definition is not resolved. Unfortunately, this causes issues when these types are dependent on the lenses-to-be-generated themselves, such as in the following example:

@optics
data class UsingLens(val field: String) {
  fun getLens() = UsingLens.field // Return type of the function is dependent on the lens
  companion object
}

While this class obviously has unresolved types, these do not impact code generation of optics. Ideally, we should limit the validation to only check the validity of the types involved in the generated lenses. The best place for this would probably be in the Focus class after generating the ADT, but I could not get this to work very neatly without changing code everywhere.

Instead, I implemented a simpler, "best effort" approach where I skip validation for irrelevant declarations/nodes, such as functions and annotations. This is not perfect, and still causes the validation to be "too strict", but might be a good initial step.

The approach suggested in the other thread - to defer classes once and actually process them on a second pass - does not work, as KSP will stop processing symbols when there is no other processor to defer symbols to.

@serras serras merged commit c98377b into arrow-kt:main Jan 10, 2025
@serras
Copy link
Member

serras commented Jan 10, 2025

Thanks for your help in fixing this!

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