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

Resolve multi-preview annotations in other modules #69

Merged
merged 7 commits into from
Sep 27, 2023
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
package com.emergetools.snapshots.sample.ui

import androidx.compose.ui.tooling.preview.Preview

@Preview(
name = "English",
locale = "en"
)
@Preview(
name = "German",
locale = "de"
)
annotation class LocalePreviews
trevor-e marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package com.emergetools.snapshots.sample.ui
import androidx.compose.foundation.layout.Column
import androidx.compose.material3.Text
import androidx.compose.runtime.Composable
import androidx.compose.ui.res.stringResource
import androidx.compose.ui.tooling.preview.Preview
import com.emergetools.snapshots.annotations.IgnoreEmergeSnapshot

Expand All @@ -25,15 +26,17 @@ fun TextRowWithIcon(

@Preview
@FontScalePreviews
@LocalePreviews
Copy link
Contributor

Choose a reason for hiding this comment

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

Ran locally and this outputted 5 previews, matching what I expected. Awesome job!

Might be good for us to add a test case in snapshots-processor-test for this, but inherently that's a bit tough as it currently relies on a single /input directory producing outputs. Maybe adding an input2 or submodule containing a multipreview with an import/usage in input would produce a valid test.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yea testing this has proven to be difficult, I'll keep trying some things.

@Composable
fun TextRowWithIconPreviewFromMain() {
TextRowWithIcon(
titleText = "Title",
subtitleText = "Subtitle"
titleText = stringResource(com.emergetools.snapshots.sample.R.string.sample_title),
trevor-e marked this conversation as resolved.
Show resolved Hide resolved
subtitleText = stringResource(com.emergetools.snapshots.sample.R.string.sample_subtitle)
)
}

@FontScalePreviews
@LocalePreviews
@Composable
fun TextRowWithIconPreviewFromMainJustMultiPreview() {
TextRowWithIcon(
Expand Down
5 changes: 5 additions & 0 deletions snapshots/sample/app/src/main/res/values-de/strings.xml
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
<resources>
<string name="app_name">Emerge snapshots sample</string>
<string name="sample_title">Testtitel</string>
<string name="sample_subtitle">Test-Untertitel</string>
</resources>
2 changes: 2 additions & 0 deletions snapshots/sample/app/src/main/res/values/strings.xml
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
<resources>
<string name="app_name">Emerge snapshots sample</string>
<string name="sample_title">Test title</string>
<string name="sample_subtitle">Test subtitle</string>
</resources>
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import com.emergetools.snapshots.processor.preview.ComposablePreviewSnapshotBuil
import com.emergetools.snapshots.processor.utils.COMPOSE_PREVIEW_ANNOTATION_NAME
import com.emergetools.snapshots.processor.utils.functionsWithMultiPreviewAnnotation
import com.emergetools.snapshots.processor.utils.functionsWithPreviewAnnotation
import com.emergetools.snapshots.processor.utils.getMultiPreviewAnnotations
import com.emergetools.snapshots.processor.utils.putOrAppend
import com.emergetools.snapshots.shared.ComposePreviewSnapshotConfig
import com.google.devtools.ksp.KspExperimental
Expand Down Expand Up @@ -46,6 +47,9 @@ class PreviewProcessor(

@OptIn(KspExperimental::class)
override fun process(resolver: Resolver): List<KSAnnotated> {
val multiPreviewAnnotations = resolver.getMultiPreviewAnnotations()
multiPreviewAnnotations.forEach { logger.info("telkins found custom annotation ${it}")}

val symbolsWithPreviewAnnotations = resolver
.getSymbolsWithAnnotation(COMPOSE_PREVIEW_ANNOTATION_NAME)
.toList()
Expand All @@ -54,10 +58,14 @@ class PreviewProcessor(
.functionsWithPreviewAnnotation()
val multiPreviewAnnotatedFunctions = symbolsWithPreviewAnnotations
trevor-e marked this conversation as resolved.
Show resolved Hide resolved
.functionsWithMultiPreviewAnnotation(resolver)
val customMultiPreviewAnnotatedFunctions = multiPreviewAnnotations
.functionsWithMultiPreviewAnnotation(resolver)
customMultiPreviewAnnotatedFunctions.forEach { logger.info("telkins found custom functions ${it}")}

val previewFunctionMap = buildMap {
putOrAppend(previewAnnotatedFunctions)
putOrAppend(multiPreviewAnnotatedFunctions)
putOrAppend(customMultiPreviewAnnotatedFunctions)
}

val codeGenerator = environment.codeGenerator
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package com.emergetools.snapshots.processor.utils
import com.emergetools.snapshots.processor.preview.ComposePreviewUtils.getUniqueSnapshotConfigsFromMultiPreviewAnnotation
import com.emergetools.snapshots.processor.preview.ComposePreviewUtils.getUniqueSnapshotConfigsFromPreviewAnnotations
import com.emergetools.snapshots.shared.ComposePreviewSnapshotConfig
import com.google.devtools.ksp.processing.KSPLogger
import com.google.devtools.ksp.processing.Resolver
import com.google.devtools.ksp.symbol.ClassKind
import com.google.devtools.ksp.symbol.KSAnnotated
Expand All @@ -21,10 +22,9 @@ fun List<KSAnnotated>.functionsWithPreviewAnnotation(): Map<KSFunctionDeclaratio
fun List<KSAnnotated>.functionsWithMultiPreviewAnnotation(
resolver: Resolver,
): Map<KSFunctionDeclaration, List<ComposePreviewSnapshotConfig>> {
return filterIsInstance<KSClassDeclaration>()
val uniqueSnapshotConfigs = filterIsInstance<KSClassDeclaration>()
.filter { it.classKind == ClassKind.ANNOTATION_CLASS }
.flatMap { annotation ->

val multiPreviewAnnotationPreviewAnnotations = annotation.annotations.filter {
it.annotationType.resolve().declaration.qualifiedName?.asString() == COMPOSE_PREVIEW_ANNOTATION_NAME
}.toList()
Expand All @@ -36,9 +36,43 @@ fun List<KSAnnotated>.functionsWithMultiPreviewAnnotation(
.map { function ->
function to getUniqueSnapshotConfigsFromMultiPreviewAnnotation(
annotations = multiPreviewAnnotationPreviewAnnotations,
previewFunction = function
previewFunction = function,
)
}
}
.toMap()

val mergedConfigs = mutableMapOf<KSFunctionDeclaration, List<ComposePreviewSnapshotConfig>>()
uniqueSnapshotConfigs.forEach {
mergedConfigs.putOrAppend(it.first, it.second)
}

return mergedConfigs
}

fun Resolver.getMultiPreviewAnnotations(): List<KSClassDeclaration> {
// Find all symbols with annotations and map to the annotation class declarations
val annotationClassDecls = getAllFiles()
trevor-e marked this conversation as resolved.
Show resolved Hide resolved
.flatMap { it.declarations }
.filter { it.annotations.count() > 0 }
.flatMap { symbol ->
symbol.annotations.mapNotNull { annotation ->
val annotationQN = annotation.annotationType.resolve().declaration.qualifiedName
annotationQN?.let { qualifiedName ->
getClassDeclarationByName(qualifiedName)
}
}
}
.filter { it.classKind == ClassKind.ANNOTATION_CLASS }
Copy link
Contributor

Choose a reason for hiding this comment

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

My only potential nit here would be to move this up above .filter { it.annotations.count() > 0 }, that way we're only working with annotation class declarations at that point, and don't risk running annotation.count() and getClassDeclarationByName on classes that are not annotation classes.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Don't think that's possible since it is a KSDeclaration for the sequence of symbols with annotations, we haven't mapped to the annotation class declarations yet.

I think the filter is actually redundant since we are flatmap'ing to the annotation class declaration, so it should only be a sequence of annotation classes at this point, but it does make it a little more readable so I'll leave it in.

.toSet()

// Of the annotation classes we found, take those that themselves have a preview annotation.
// We can assume these are multi-preview annotations.
return annotationClassDecls
.filter {
it.annotations.any { annotation ->
Copy link
Member

Choose a reason for hiding this comment

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

Will this get annotations that are transitively added to this annotation? Ex:

@Preview(...)
annotation class Class1

@Class1
annotation class Class2

Will it pick up Class2 as having a preview annotation? If not can we do this in a loop building up what annotations are eligible to make a preview?

Copy link
Contributor

Choose a reason for hiding this comment

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

We had discussed v1 of this only supporting a single level of Multipreview nesting (and checked with clients that that was ok), so that means we're only intending to cover your first example here.

But, if it's easy enough to add, we might as well add support for all potential cases here.

annotation.annotationType.resolve().declaration.qualifiedName?.asString() == COMPOSE_PREVIEW_ANNOTATION_NAME
}
}
.toList()
.sortedBy { it.simpleName.asString() }
trevor-e marked this conversation as resolved.
Show resolved Hide resolved
}
Loading