Fix parsing material icon with multiple material paths#767
Conversation
egorikftp
commented
Dec 11, 2025

WalkthroughThe parser logic has been refactored to support multiple Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ 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 |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
components/psi/imagevector/src/main/kotlin/io/github/composegears/valkyrie/psi/imagevector/parser/MaterialImageVectorPsiParser.kt (1)
83-87: Consider narrowingmaterialPathCallssearch scope if getters ever contain multiple icons
childrenOfType<KtCallExpression>()walks the entireKtBlockExpressionsubtree, so anymaterialPathcall inside the getter (even if unrelated to the primarymaterialIcon/builder you parse above) will be included. If you later support more complex getter bodies (e.g., multiple icons or helper functions with their ownmaterialPathcalls), it may be safer to derive paths from the specificmaterialIcon/builder lambda body instead of the whole block.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
components/psi/imagevector/src/main/kotlin/io/github/composegears/valkyrie/psi/imagevector/parser/MaterialImageVectorPsiParser.kt(2 hunks)components/psi/imagevector/src/test/kotlin/io/github/composegears/valkyrie/psi/imagevector/KtFileToImageVectorParserTest.kt(2 hunks)components/psi/imagevector/src/test/kotlin/io/github/composegears/valkyrie/psi/imagevector/expected/MaterialIcon.kt(1 hunks)components/psi/imagevector/src/test/resources/backing/MaterialIcon.several.materialpath.kt(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
components/psi/imagevector/src/test/resources/backing/MaterialIcon.several.materialpath.kt (1)
components/psi/imagevector/src/test/kotlin/androidx/compose/material/icons/Icons.kt (2)
materialIcon(18-29)materialPath(31-47)
components/psi/imagevector/src/main/kotlin/io/github/composegears/valkyrie/psi/imagevector/parser/MaterialImageVectorPsiParser.kt (1)
components/psi/imagevector/src/main/kotlin/io/github/composegears/valkyrie/psi/extension/PsiElement.kt (1)
childrenOfType(6-8)
components/psi/imagevector/src/test/kotlin/io/github/composegears/valkyrie/psi/imagevector/expected/MaterialIcon.kt (1)
components/psi/imagevector/src/test/kotlin/androidx/compose/material/icons/Icons.kt (2)
materialIcon(18-29)materialPath(31-47)
⏰ 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 (4)
components/psi/imagevector/src/test/kotlin/io/github/composegears/valkyrie/psi/imagevector/expected/MaterialIcon.kt (1)
45-86: Expected Percent icon with multiplematerialPathblocks looks consistentThe new
ExpectedMaterialIconWithoutParam2mirrors the backingPercenticon (three paths, EvenOdd on the first two, default on the slash) and is well‑structured for asserting the multi‑materialPathparsing behavior.components/psi/imagevector/src/test/resources/backing/MaterialIcon.several.materialpath.kt (1)
1-62: BackingPercenticon correctly exercises multi‑materialPathparsingThe
_percentbacking property andIcons.Percentbuilder follow the existing material icon pattern and define threematerialPathblocks with coordinates andPathFillTypematching the new expected icon, which is exactly what you need to validate the multi‑path parser change.components/psi/imagevector/src/test/kotlin/io/github/composegears/valkyrie/psi/imagevector/KtFileToImageVectorParserTest.kt (1)
24-25: New multi‑materialPathtest is wired correctlyImporting
ExpectedMaterialIconWithoutParam2and theparse material icon with several materialPathtest that readsbacking/MaterialIcon.several.materialpath.ktand asserts against that expected icon are consistent with the existing test patterns and should reliably cover the multi‑path parsing behavior.Also applies to: 100-106
components/psi/imagevector/src/main/kotlin/io/github/composegears/valkyrie/psi/imagevector/parser/MaterialImageVectorPsiParser.kt (1)
63-79: Multi-path parsing logic looks correct and resilientThe new implementation correctly builds one
IrPathpermaterialPathcall, preserves order, and safely skips calls with missing/invalid lambda bodies viamapNotNull. This aligns with the PR goal of supporting multiple material paths and avoids failing the entire parse on a single malformed block.