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.

Sign up for GitHub

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

Resolve multi-preview annotations in other modules #69

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
c70c817
Progress
trevor-e Sep 27, 2023
07a3941
Add a locale multi-preview annotation
trevor-e Sep 27, 2023
9c57e26
Fixes
trevor-e Sep 27, 2023
4dc1cdb
Remove logging
trevor-e Sep 27, 2023
fa63263
Comment
trevor-e Sep 27, 2023
16ae684
Fix lint and duplicated behavior
trevor-e Sep 27, 2023
4cd623f
Cleanup
trevor-e Sep 27, 2023
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
13 changes: 13 additions & 0 deletions snapshots/sample/app/src/main/kotlin/com/emergetools/snapshots/sample/ui/LocalePreviews.kt
View file
Open in desktop
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
7 changes: 5 additions & 2 deletions snapshots/sample/app/src/main/kotlin/com/emergetools/snapshots/sample/ui/TextRowWithIcon.kt
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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>
0 .../snapshots/sample/ui/FontScalePreviews.kt → .../snapshots/sample/ui/FontScalePreviews.kt
View file
Open in desktop
File renamed without changes.
4 changes: 3 additions & 1 deletion ...apshots-processor/src/main/kotlin/com/emergetools/snapshots/processor/PreviewProcessor.kt
View file
Open in desktop
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 @@ -49,10 +50,11 @@ class PreviewProcessor(
val symbolsWithPreviewAnnotations = resolver
.getSymbolsWithAnnotation(COMPOSE_PREVIEW_ANNOTATION_NAME)
.toList()
val symbolsWithMultiPreviewAnnotations = resolver.getMultiPreviewAnnotations()

val previewAnnotatedFunctions = symbolsWithPreviewAnnotations
.functionsWithPreviewAnnotation()
val multiPreviewAnnotatedFunctions = symbolsWithPreviewAnnotations
val multiPreviewAnnotatedFunctions = symbolsWithMultiPreviewAnnotations
.functionsWithMultiPreviewAnnotation(resolver)

val previewFunctionMap = buildMap {
Expand Down
42 changes: 38 additions & 4 deletions ...processor/src/main/kotlin/com/emergetools/snapshots/processor/utils/PreviewFinderUtils.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,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 +35,44 @@ fun List<KSAnnotated>.functionsWithMultiPreviewAnnotation(
.map { function ->
function to getUniqueSnapshotConfigsFromMultiPreviewAnnotation(
annotations = multiPreviewAnnotationPreviewAnnotations,
previewFunction = function
previewFunction = function,
)
}
}
.toMap()

// The same function declaration can show up multiple times, so ensure the values are merged together
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