Migrate IconPackPsiParser to use KtFile, create intellij test-fixtures module#785
Migrate IconPackPsiParser to use KtFile, create intellij test-fixtures module#785
Conversation
WalkthroughThis pull request refactors the icon pack parsing and data model. It replaces the IconPackInfo structure from using String and List fields to using a proper IconPack domain object. The IconPackPsiParser is updated with a new public parse(KtFile) method and private buildIconPack helper for recursive assembly. A new test-fixtures module is introduced containing KotlinLightTestCase base class for IntelliJ platform tests. Test dependencies are reorganized with JUnit Platform configuration made conditional by project path. The test suite is expanded with new nested icon pack validation scenarios and resource files, while existing code using IconPackInfo is updated to reference the new IconPack structure fields. Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
sdk/intellij/test-fixtures/src/testFixtures/kotlin/io/github/composegears/valkyrie/sdk/intellij/testfixtures/KotlinLightTestCase.kt (1)
23-28: Consider safer cast with explicit error message.The direct cast
as KtFilewill throw aClassCastExceptionif the file is not a Kotlin file, which provides a less informative error than the null check above.protected fun loadKtFile(fileName: String): KtFile { val kotlinFile = LocalFileSystem.getInstance().findFileByPath("$testDataPath/$fileName") ?: error("File not found: $fileName") - return psiManager.findFile(kotlinFile) as KtFile + return psiManager.findFile(kotlinFile) as? KtFile + ?: error("File is not a Kotlin file: $fileName") }sdk/intellij/psi/iconpack/src/test/kotlin/io/github/composegears/valkyrie/sdk/intellij/psi/iconpack/IconPackPsiParserTest.kt (1)
127-136: Consider improving error diagnostics for test failures.Using
first { }will throw a genericNoSuchElementExceptionif the path segment is not found, making test failures harder to debug.private fun IconPack.navigate(path: String): IconPack { val parts = path.split('.') var current = this parts.forEach { part -> - current = current.nested.first { it.name == part } + current = current.nested.firstOrNull { it.name == part } + ?: error("Nested pack '$part' not found in '${current.name}'. Available: ${current.nested.map { it.name }}") } return current }
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
build.gradle.kts(1 hunks)gradle/libs.versions.toml(1 hunks)sdk/intellij/psi/iconpack/api/iconpack.api(2 hunks)sdk/intellij/psi/iconpack/build.gradle.kts(1 hunks)sdk/intellij/psi/iconpack/src/main/kotlin/io/github/composegears/valkyrie/sdk/intellij/psi/iconpack/IconPackPsiParser.kt(1 hunks)sdk/intellij/psi/iconpack/src/test/kotlin/io/github/composegears/valkyrie/sdk/intellij/psi/iconpack/IconPackPsiParserTest.kt(1 hunks)sdk/intellij/psi/iconpack/src/test/resources/DeepNestedIconPack.kt(1 hunks)sdk/intellij/test-fixtures/build.gradle.kts(1 hunks)sdk/intellij/test-fixtures/src/testFixtures/kotlin/io/github/composegears/valkyrie/sdk/intellij/testfixtures/KotlinLightTestCase.kt(1 hunks)settings.gradle.kts(1 hunks)tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/mode/iconpack/existingpack/ui/viewmodel/ExistingPackViewModel.kt(1 hunks)
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
Learnt from: egorikftp
Repo: ComposeGears/Valkyrie PR: 750
File: tools/gradle-plugin/src/main/kotlin/io/github/composegears/valkyrie/gradle/internal/task/GenerateImageVectorsTask.kt:71-85
Timestamp: 2025-12-07T20:07:49.753Z
Learning: In the Valkyrie Gradle plugin (Kotlin), the `useFlatPackage` flag in `IconPackExtension` is only applicable when nested packs are configured. For single icon packs (without nested packs), the flag is intentionally not propagated to `ImageVectorGeneratorConfig` as there is no package hierarchy to flatten.
📚 Learning: 2025-11-26T09:19:45.016Z
Learnt from: egorikftp
Repo: ComposeGears/Valkyrie PR: 730
File: tools/gradle-plugin/build.gradle.kts:17-18
Timestamp: 2025-11-26T09:19:45.016Z
Learning: In Gradle version catalogs, hyphenated keys in TOML files (e.g., `gradle-plugin-version`) are automatically normalized to dot-separated accessors (e.g., `gradle.plugin.version`). Hyphens, underscores, and dots are all treated as separators for generating nested accessor paths.
Applied to files:
gradle/libs.versions.toml
📚 Learning: 2025-10-21T20:55:27.073Z
Learnt from: egorikftp
Repo: ComposeGears/Valkyrie PR: 651
File: tools/idea-plugin/build.gradle.kts:147-175
Timestamp: 2025-10-21T20:55:27.073Z
Learning: In Gradle Kotlin DSL (.gradle.kts) scripts, the types `org.gradle.api.artifacts.ArtifactCollection` and `org.gradle.api.artifacts.component.ModuleComponentIdentifier` are implicitly available and do not require explicit import statements.
Applied to files:
settings.gradle.ktssdk/intellij/psi/iconpack/build.gradle.ktssdk/intellij/test-fixtures/build.gradle.kts
📚 Learning: 2025-12-07T20:07:49.753Z
Learnt from: egorikftp
Repo: ComposeGears/Valkyrie PR: 750
File: tools/gradle-plugin/src/main/kotlin/io/github/composegears/valkyrie/gradle/internal/task/GenerateImageVectorsTask.kt:71-85
Timestamp: 2025-12-07T20:07:49.753Z
Learning: In the Valkyrie Gradle plugin (Kotlin), the `useFlatPackage` flag in `IconPackExtension` is only applicable when nested packs are configured. For single icon packs (without nested packs), the flag is intentionally not propagated to `ImageVectorGeneratorConfig` as there is no package hierarchy to flatten.
Applied to files:
sdk/intellij/psi/iconpack/build.gradle.ktssdk/intellij/psi/iconpack/src/test/kotlin/io/github/composegears/valkyrie/sdk/intellij/psi/iconpack/IconPackPsiParserTest.ktsdk/intellij/psi/iconpack/api/iconpack.apisdk/intellij/psi/iconpack/src/test/resources/DeepNestedIconPack.ktsdk/intellij/psi/iconpack/src/main/kotlin/io/github/composegears/valkyrie/sdk/intellij/psi/iconpack/IconPackPsiParser.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/mode/iconpack/existingpack/ui/viewmodel/ExistingPackViewModel.kt
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build (ubuntu-latest)
🔇 Additional comments (15)
tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/mode/iconpack/existingpack/ui/viewmodel/ExistingPackViewModel.kt (1)
136-147: LGTM! Clean adaptation to the new IconPack data model.The changes correctly adapt the input field state construction to use the refactored IconPack domain object. The code properly accesses
iconPack.nameand iterates overiconPack.nested, which aligns with the IconPack structure used elsewhere in the file (lines 108-111).settings.gradle.kts (1)
86-86: LGTM!The new module inclusion is correctly placed within the
sdk:intellijsection and follows the project's naming conventions.build.gradle.kts (1)
86-92: LGTM!The conditional exclusion for the iconpack module is appropriate since it relies on IntelliJ Platform's test framework, which uses JUnit 4. The excluded module configures its own test framework via the IntelliJ Platform Gradle plugin.
sdk/intellij/test-fixtures/build.gradle.kts (1)
1-17: LGTM!The test-fixtures module is well configured with appropriate plugins and dependencies for IntelliJ platform testing. Using
testFixturesApicorrectly exposes JUnit 4 to consuming modules.sdk/intellij/psi/iconpack/src/test/resources/DeepNestedIconPack.kt (1)
1-43: LGTM!Comprehensive test resource covering important edge cases: deep linear chains, branching structures, wide trees, and single leaves. Good documentation with comments.
sdk/intellij/psi/iconpack/build.gradle.kts (1)
10-21: LGTM!Clean dependency restructuring. The test fixtures module now provides the JUnit 4 dependency via
testFixturesApi, and thegenerator.coredependency is correctly added for theIconPacktype usage.sdk/intellij/psi/iconpack/src/main/kotlin/io/github/composegears/valkyrie/sdk/intellij/psi/iconpack/IconPackPsiParser.kt (2)
22-35: LGTM!Clean implementation of the new
parse(KtFile)entry point. The logic correctly extracts the first top-level object declaration and delegates to the recursive builder.
37-49: Well-structured recursive builder.The
buildIconPackhelper elegantly handles nested object declarations with proper null safety. Using.orEmpty()ensures clean handling of objects without nested declarations.gradle/libs.versions.toml (1)
31-32: > Likely an incorrect or invalid review comment.sdk/intellij/psi/iconpack/src/test/kotlin/io/github/composegears/valkyrie/sdk/intellij/psi/iconpack/IconPackPsiParserTest.kt (4)
11-23: LGTM!The migration to
KotlinLightTestCaseand the newparse(KtFile)API is clean. The test correctly validates the package name, icon pack name, and empty nested list for a simple icon pack.
44-54: LGTM!Good addition testing
data objecticon pack support, ensuring the parser handles this Kotlin construct correctly.
56-125: LGTM! Comprehensive nested structure validation.The deep nested test is thorough—it validates linear chains, branching hierarchies, wide trees, and leaf nodes. The round-trip test (
toRawString→fromString→ equality check) is excellent for ensuring serialization consistency.
138-148: LGTM!The helper provides a clean DSL for structure assertions with optional nested name validation.
sdk/intellij/psi/iconpack/api/iconpack.api (2)
14-18: LGTM! Newparse(KtFile)method expands API capabilities.The addition of
parse(KtFile)alongside the existingextractIconPack(Path, Project)provides flexibility—direct PSI parsing for callers that already have aKtFile, and file-based extraction for path-based workflows.
1-12: Breaking API change:IconPackInfostructure refactored to useIconPackdomain object.Constructor now takes
(String, IconPack)instead of separate fields. Nested packs are accessed throughiconPack.nestedrather than a dedicated method. This consolidates the representation into a proper domain model but requires consumers to update their usage patterns.
1 participant
Now we able to parse deep iconpack structure