Open
Conversation
Member
Author
|
@Goooler maybe you can take a look 🙂 |
WalkthroughThis PR updates the Valkyrie build and Gradle plugin to target AGP 9.x and Gradle 9.4.1: bumps AGP to 9.2.0, updates android-build-tools and the Gradle wrapper, adds AGENTS.md documentation, refactors the plugin to use AGP's CommonExtension (removing kotlin("android") usage), changes task/source-set registration and task wiring for configuration-cache safety, removes Possibly related issues
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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: 4
🧹 Nitpick comments (4)
tools/gradle-plugin/src/main/kotlin/io/github/composegears/valkyrie/gradle/dsl/TaskCollectionExtensions.kt (1)
6-7: Optional: simpler unchecked cast.Casting to
TaskCollection<in T>is a bit unusual — sinceT : Task, casting toTaskCollection<Task>reads more naturally and produces the same bytecode. Purely cosmetic.♻️ Optional refactor
-@Suppress("UNCHECKED_CAST") -internal inline fun <reified T : Task> TaskCollection<*>.withType(): TaskCollection<T> = (this as TaskCollection<in T>).withType(T::class.java) +@Suppress("UNCHECKED_CAST") +internal inline fun <reified T : Task> TaskCollection<*>.withType(): TaskCollection<T> = + (this as TaskCollection<Task>).withType(T::class.java)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tools/gradle-plugin/src/main/kotlin/io/github/composegears/valkyrie/gradle/dsl/TaskCollectionExtensions.kt` around lines 6 - 7, The cast in the extension function withType() is oddly written as TaskCollection<in T>; change it to cast to TaskCollection<Task> for readability: locate the inline reified function TaskCollection<*>.withType() and replace the (this as TaskCollection<in T>) cast with (this as TaskCollection<Task>) while keeping the call to .withType(T::class.java) and preserving the `@Suppress`("UNCHECKED_CAST") and function signature.tools/gradle-plugin/CHANGELOG.md (1)
5-9: Consider documenting the AGP minimum / 8.x drop.Current "Added" entry conveys the new capability, but since the plugin classpath is now compiled against AGP 9.2.0, users on AGP 8.x will most likely fail with
NoClassDefFoundError/NoSuchMethodError. Mentioning the new minimum AGP and (if applicable) the AGP 8 drop under a "Changed"/"Removed" section would prevent surprises for downstream consumers.📝 Suggested addition
### Added - Add support for AGP 9.0+ built-in Kotlin — source-set enumeration and generated source wiring now work without the external `kotlin("android")` plugin via the stable `CommonExtension` API + +### Changed + +- **Breaking:** Minimum required Android Gradle Plugin is now `9.0.0` (previously `8.x`).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tools/gradle-plugin/CHANGELOG.md` around lines 5 - 9, Update the CHANGELOG entry to explicitly state the new minimum Android Gradle Plugin (AGP) requirement and note the AGP 8.x drop: add a "Changed" (or "Removed") subsection clarifying that the plugin classpath is compiled against AGP 9.2.0 so AGP 8.x users may see runtime errors (NoClassDefFoundError/NoSuchMethodError) and must upgrade to AGP 9.x (reference AGP 9.2.0) to use the new built-in Kotlin source-set enumeration and generated source wiring behavior mentioned in the "Added" entry.tools/gradle-plugin/src/main/kotlin/io/github/composegears/valkyrie/gradle/dsl/ExtensionContainerExtensions.kt (1)
11-13: AlignfindByTypewithgetByTypeto preserve parameterized type information.
getByTypeusestypeOf<T>()which preserves full type information including parameterization (e.g.,NamedDomainObjectContainer<Foo>), whilefindByTyperesolves only by rawClass<T>, losing generic type info due to type erasure. Switching to theTypeOfoverload makes the two helpers symmetric and ensures proper matching for parameterized extensions.♻️ Proposed change
-internal inline fun <reified T : Any> ExtensionContainer.findByType(): T? = findByType(T::class.java) +internal inline fun <reified T : Any> ExtensionContainer.findByType(): T? = findByType(typeOf<T>())🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tools/gradle-plugin/src/main/kotlin/io/github/composegears/valkyrie/gradle/dsl/ExtensionContainerExtensions.kt` around lines 11 - 13, The findByType helper loses generic type info because it calls findByType(Class<T>); update ExtensionContainer.findByType to use the TypeOf overload like getByType does: change the body to call findByType(typeOf<T>()) so it preserves parameterized types (keep it inline and reified as declared), mirroring getByType's approach and ensuring proper matching for parameterized extensions.tools/gradle-plugin/src/main/kotlin/io/github/composegears/valkyrie/gradle/ValkyrieGradlePlugin.kt (1)
104-107: Drop the unnecessary@Suppress("UNCHECKED_CAST").The
registerAndroidTasksfunction contains no explicit unchecked cast operations. ThefindByType<T>()method calls use generic parameters but are not unchecked casts, and star projections inAndroidComponentsExtension<*, *, *>do not trigger this warning. The suppression is a leftover and can be safely removed.♻️ Proposed cleanup
- `@Suppress`("UNCHECKED_CAST") private fun Project.registerAndroidTasks(extension: ValkyrieExtension) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tools/gradle-plugin/src/main/kotlin/io/github/composegears/valkyrie/gradle/ValkyrieGradlePlugin.kt` around lines 104 - 107, Remove the unnecessary `@Suppress`("UNCHECKED_CAST") annotation on the private function registerAndroidTasks(Project.registerAndroidTasks) in ValkyrieGradlePlugin; the body only calls extensions.findByType<CommonExtension>() and extensions.findByType<AndroidComponentsExtension<*, *, *>>() and performs no unchecked casts, so delete the `@Suppress` line to clean up the code.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@AGENTS.md`:
- Around line 78-86: Update the documentation to include the missing convention
plugin `valkyrie.wasm.resources`: in the section listing convention plugins
(alongside `alias(libs.plugins.valkyrie.jvm)`,
`alias(libs.plugins.valkyrie.kmp)`, and `alias(libs.plugins.valkyrie.compose)`),
add `valkyrie.wasm.resources` (or place it under a separate WASM/resources
category or with the optional plugins `valkyrie.abi` and `valkyrie.kover`) so
the AGENTS.md list matches the plugins registered in the build-logic (e.g., the
plugin id `valkyrie.wasm.resources`).
In `@gradle/gradle.versions.toml`:
- Line 4: Update the Unreleased section of tools/gradle-plugin/CHANGELOG.md to
state that AGP 9.0.0 is the minimum supported version and AGP 8.x is dropped
(breaking change), change the catalog entry agp (in gradle/gradle.versions.toml
where agp = "9.2.0") to compile against the lowest supported patch of 9.0.x
(e.g., 9.0.<latest-patch>) to avoid accidental use of newer APIs, and add
CI/test matrix coverage that runs the plugin tests against AGP 9.0.0 (or the
chosen 9.0.x patch) to enforce the baseline; ensure references to
AndroidComponentsExtension, CommonExtension.sourceSets and androidLibrary
build-blocks still compile against the new agp version.
In
`@tools/gradle-plugin/src/main/kotlin/io/github/composegears/valkyrie/gradle/internal/TaskRegistry.kt`:
- Around line 73-85: When findIconFiles returns an empty FileCollection for a
given sourceSetName and resourceDirectoryName, emit a clear warning that
includes the sourceSetName and the concrete path checked (use
resourceDirectoryName.get()) so users know which
src/<sourceSetName>/<resourceDir> was searched; specifically update
Project.findIconFiles to check the resulting collection size and, if empty and
an iconPack is configured for this source set (detect via the project extension
or property used for iconPack), call project.logger.warn with a message like "No
icons found for sourceSet '<sourceSetName>' in
'src/<sourceSetName>/<resourceDirectoryName>' while iconPack configured —
generation will be skipped" to aid debugging.
In
`@tools/gradle-plugin/src/main/kotlin/io/github/composegears/valkyrie/gradle/ValkyrieGradlePlugin.kt`:
- Around line 109-117: Update the stale inline comment inside the
androidExt.sourceSets.configureEach block that currently says "handled via
addGeneratedSourceDirectory in phase 2" to correctly state that phase 2 uses
addStaticSourceDirectory (see
variant.sources.kotlin?.addStaticSourceDirectory(genDir)) and reference the
function KDoc rationale; specifically, change the placeholder on the
registerTask call where addGeneratedSrcDir is passed to indicate that the
generated source dir is handled via addStaticSourceDirectory in phase 2 to avoid
contradicting the KDoc and the variant.sources.kotlin usage.
---
Nitpick comments:
In `@tools/gradle-plugin/CHANGELOG.md`:
- Around line 5-9: Update the CHANGELOG entry to explicitly state the new
minimum Android Gradle Plugin (AGP) requirement and note the AGP 8.x drop: add a
"Changed" (or "Removed") subsection clarifying that the plugin classpath is
compiled against AGP 9.2.0 so AGP 8.x users may see runtime errors
(NoClassDefFoundError/NoSuchMethodError) and must upgrade to AGP 9.x (reference
AGP 9.2.0) to use the new built-in Kotlin source-set enumeration and generated
source wiring behavior mentioned in the "Added" entry.
In
`@tools/gradle-plugin/src/main/kotlin/io/github/composegears/valkyrie/gradle/dsl/ExtensionContainerExtensions.kt`:
- Around line 11-13: The findByType helper loses generic type info because it
calls findByType(Class<T>); update ExtensionContainer.findByType to use the
TypeOf overload like getByType does: change the body to call
findByType(typeOf<T>()) so it preserves parameterized types (keep it inline and
reified as declared), mirroring getByType's approach and ensuring proper
matching for parameterized extensions.
In
`@tools/gradle-plugin/src/main/kotlin/io/github/composegears/valkyrie/gradle/dsl/TaskCollectionExtensions.kt`:
- Around line 6-7: The cast in the extension function withType() is oddly
written as TaskCollection<in T>; change it to cast to TaskCollection<Task> for
readability: locate the inline reified function TaskCollection<*>.withType() and
replace the (this as TaskCollection<in T>) cast with (this as
TaskCollection<Task>) while keeping the call to .withType(T::class.java) and
preserving the `@Suppress`("UNCHECKED_CAST") and function signature.
In
`@tools/gradle-plugin/src/main/kotlin/io/github/composegears/valkyrie/gradle/ValkyrieGradlePlugin.kt`:
- Around line 104-107: Remove the unnecessary `@Suppress`("UNCHECKED_CAST")
annotation on the private function
registerAndroidTasks(Project.registerAndroidTasks) in ValkyrieGradlePlugin; the
body only calls extensions.findByType<CommonExtension>() and
extensions.findByType<AndroidComponentsExtension<*, *, *>>() and performs no
unchecked casts, so delete the `@Suppress` line to clean up the code.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: a3e8731c-3c6b-44fa-97de-8762426d692e
📒 Files selected for processing (14)
AGENTS.mdgradle/gradle.versions.tomlgradle/libs.versions.tomlgradle/wrapper/gradle-wrapper.propertiesgradlewtools/gradle-plugin/CHANGELOG.mdtools/gradle-plugin/src/main/kotlin/io/github/composegears/valkyrie/gradle/IconPackExtension.kttools/gradle-plugin/src/main/kotlin/io/github/composegears/valkyrie/gradle/ValkyrieExtension.kttools/gradle-plugin/src/main/kotlin/io/github/composegears/valkyrie/gradle/ValkyrieGradlePlugin.kttools/gradle-plugin/src/main/kotlin/io/github/composegears/valkyrie/gradle/dsl/ExtensionContainerExtensions.kttools/gradle-plugin/src/main/kotlin/io/github/composegears/valkyrie/gradle/dsl/TaskCollectionExtensions.kttools/gradle-plugin/src/main/kotlin/io/github/composegears/valkyrie/gradle/internal/TaskRegistry.kttools/gradle-plugin/src/test/kotlin/io/github/composegears/valkyrie/gradle/ValkyrieGradlePluginTest.kttools/gradle-plugin/src/test/kotlin/io/github/composegears/valkyrie/gradle/common/CommonGradleTest.kt
💤 Files with no reviewable changes (3)
- tools/gradle-plugin/src/main/kotlin/io/github/composegears/valkyrie/gradle/IconPackExtension.kt
- tools/gradle-plugin/src/test/kotlin/io/github/composegears/valkyrie/gradle/common/CommonGradleTest.kt
- tools/gradle-plugin/src/main/kotlin/io/github/composegears/valkyrie/gradle/ValkyrieExtension.kt
Member
|
I'll take a look later. |
egorikftp
force-pushed
the
feature/gradle/agp-9
branch
from
9a49e26 to
7ed477f
Compare
There was a problem hiding this comment.
♻️ Duplicate comments (1)
tools/gradle-plugin/src/main/kotlin/io/github/composegears/valkyrie/gradle/internal/TaskRegistry.kt (1)
33-36:⚠️ Potential issue | 🟡 MinorSilent skip when
iconPacksource set name does not match any registered source set.
onlyIfonly triggers icon generation for the source set whose name equalsextension.iconPack.get().targetSourceSet.get(). If the user misconfigurestargetSourceSet(typo, or referencing a source set that doesn't exist for the active platform — e.g.,androidMainin a JVM-only project), no task will run and there will be no diagnostic. Consider validating the configuredtargetSourceSetagainst the set of registered source sets at configuration time and failing with an actionable error.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tools/gradle-plugin/src/main/kotlin/io/github/composegears/valkyrie/gradle/internal/TaskRegistry.kt` around lines 33 - 36, The current onlyIf in TaskRegistry.kt lets tasks silently skip when extension.iconPack.targetSourceSet references a non-existent source set; add an explicit validation during configuration: when reading extension.iconPack.isPresent and extension.iconPack.get().targetSourceSet.get(), check that the named source set exists in the project’s registered source sets (the same collection you use when creating tasks) and if it does not, throw a GradleException with an actionable message (include the invalid targetSourceSet value and list of available source set names). This validation should run once at configuration time (before creating task.onlyIf predicates) so misconfiguration fails fast instead of silently skipping execution.
🧹 Nitpick comments (2)
tools/gradle-plugin/src/main/kotlin/io/github/composegears/valkyrie/gradle/dsl/TaskCollectionExtensions.kt (1)
6-7: Optional: simplify the unchecked cast direction.The widened receiver makes sense for AGP 9's stricter
TaskCollection<out …>return types. However, casting toTaskCollection<in T>is a bit unusual since the helper neither inserts elements nor exposes a contravariant view — it simply calls the read-onlywithType(Class)and returnsTaskCollection<T>. A direct cast toTaskCollection<T>reads more naturally and matches the return type.♻️ Proposed refactor
-@Suppress("UNCHECKED_CAST") -internal inline fun <reified T : Task> TaskCollection<*>.withType(): TaskCollection<T> = (this as TaskCollection<in T>).withType(T::class.java) +@Suppress("UNCHECKED_CAST") +internal inline fun <reified T : Task> TaskCollection<*>.withType(): TaskCollection<T> = + (this as TaskCollection<T>).withType(T::class.java)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tools/gradle-plugin/src/main/kotlin/io/github/composegears/valkyrie/gradle/dsl/TaskCollectionExtensions.kt` around lines 6 - 7, The unchecked cast in the extension function TaskCollection<*>.withType() uses TaskCollection<in T> which is semantically odd; change the cast to TaskCollection<T> so the receiver is cast directly to the expected return type before calling withType(T::class.java) and returning TaskCollection<T> (adjust the cast in the internal inline fun <reified T : Task> TaskCollection<*>.withType() implementation).tools/gradle-plugin/src/main/kotlin/io/github/composegears/valkyrie/gradle/internal/TaskRegistry.kt (1)
78-85: Consider lazy Provider chaining instead of eagerresourceDirectoryName.get().
findIconFilesis invoked inside thetasks.register { … }configuration action (line 31), which still resolves theProperty<String>at task realization rather than at execution. If a user setsvalkyrie.resourceDirectoryNameafter the task is realized (e.g., from another plugin orafterEvaluate), that change won't be reflected. UsingProvider.mapkeeps the directory lookup fully lazy and configuration-cache friendly.♻️ Proposed refactor
-private fun Project.findIconFiles(sourceSetName: String, resourceDirectoryName: Property<String>): FileCollection { - val resourceDir = layout.projectDirectory - .dir("src/$sourceSetName") - .dir(resourceDirectoryName.get()) - return resourceDir.asFileTree.filter { - it.extension == "svg" || it.extension == "xml" - } -} +private fun Project.findIconFiles(sourceSetName: String, resourceDirectoryName: Property<String>): FileCollection { + val sourceSetRoot = layout.projectDirectory.dir("src/$sourceSetName") + val resourceDir = resourceDirectoryName.map { sourceSetRoot.dir(it) } + return files(resourceDir).asFileTree.filter { + it.extension == "svg" || it.extension == "xml" + } +}Gradle Property get() vs map for lazy evaluation in task registration configuration cache🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tools/gradle-plugin/src/main/kotlin/io/github/composegears/valkyrie/gradle/internal/TaskRegistry.kt` around lines 78 - 85, The findIconFiles function eagerly calls resourceDirectoryName.get(), which resolves the Property too early; change it to chain Providers so the directory lookup and asFileTree/filter are lazy: use resourceDirectoryName.map { dirName -> layout.projectDirectory.dir("src/$sourceSetName").dir(dirName) } and then map/flatMap that Provider to produce the FileCollection (apply asFileTree and the svg/xml filter inside the Provider chain) so the returned value is resolved lazily and reflects late changes to valkyrie.resourceDirectoryName; update the function findIconFiles to use Provider.map/flatMap instead of calling resourceDirectoryName.get().
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In
`@tools/gradle-plugin/src/main/kotlin/io/github/composegears/valkyrie/gradle/internal/TaskRegistry.kt`:
- Around line 33-36: The current onlyIf in TaskRegistry.kt lets tasks silently
skip when extension.iconPack.targetSourceSet references a non-existent source
set; add an explicit validation during configuration: when reading
extension.iconPack.isPresent and extension.iconPack.get().targetSourceSet.get(),
check that the named source set exists in the project’s registered source sets
(the same collection you use when creating tasks) and if it does not, throw a
GradleException with an actionable message (include the invalid targetSourceSet
value and list of available source set names). This validation should run once
at configuration time (before creating task.onlyIf predicates) so
misconfiguration fails fast instead of silently skipping execution.
---
Nitpick comments:
In
`@tools/gradle-plugin/src/main/kotlin/io/github/composegears/valkyrie/gradle/dsl/TaskCollectionExtensions.kt`:
- Around line 6-7: The unchecked cast in the extension function
TaskCollection<*>.withType() uses TaskCollection<in T> which is semantically
odd; change the cast to TaskCollection<T> so the receiver is cast directly to
the expected return type before calling withType(T::class.java) and returning
TaskCollection<T> (adjust the cast in the internal inline fun <reified T : Task>
TaskCollection<*>.withType() implementation).
In
`@tools/gradle-plugin/src/main/kotlin/io/github/composegears/valkyrie/gradle/internal/TaskRegistry.kt`:
- Around line 78-85: The findIconFiles function eagerly calls
resourceDirectoryName.get(), which resolves the Property too early; change it to
chain Providers so the directory lookup and asFileTree/filter are lazy: use
resourceDirectoryName.map { dirName ->
layout.projectDirectory.dir("src/$sourceSetName").dir(dirName) } and then
map/flatMap that Provider to produce the FileCollection (apply asFileTree and
the svg/xml filter inside the Provider chain) so the returned value is resolved
lazily and reflects late changes to valkyrie.resourceDirectoryName; update the
function findIconFiles to use Provider.map/flatMap instead of calling
resourceDirectoryName.get().
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: daa87b15-7542-4133-b047-64926fa47eaa
📒 Files selected for processing (16)
AGENTS.mdREADME.mdgradle/gradle.versions.tomlgradle/libs.versions.tomlgradle/wrapper/gradle-wrapper.propertiesgradlewtools/gradle-plugin/CHANGELOG.mdtools/gradle-plugin/build.gradle.ktstools/gradle-plugin/src/main/kotlin/io/github/composegears/valkyrie/gradle/IconPackExtension.kttools/gradle-plugin/src/main/kotlin/io/github/composegears/valkyrie/gradle/ValkyrieExtension.kttools/gradle-plugin/src/main/kotlin/io/github/composegears/valkyrie/gradle/ValkyrieGradlePlugin.kttools/gradle-plugin/src/main/kotlin/io/github/composegears/valkyrie/gradle/dsl/ExtensionContainerExtensions.kttools/gradle-plugin/src/main/kotlin/io/github/composegears/valkyrie/gradle/dsl/TaskCollectionExtensions.kttools/gradle-plugin/src/main/kotlin/io/github/composegears/valkyrie/gradle/internal/TaskRegistry.kttools/gradle-plugin/src/test/kotlin/io/github/composegears/valkyrie/gradle/ValkyrieGradlePluginTest.kttools/gradle-plugin/src/test/kotlin/io/github/composegears/valkyrie/gradle/common/CommonGradleTest.kt
💤 Files with no reviewable changes (3)
- tools/gradle-plugin/src/main/kotlin/io/github/composegears/valkyrie/gradle/IconPackExtension.kt
- tools/gradle-plugin/src/test/kotlin/io/github/composegears/valkyrie/gradle/common/CommonGradleTest.kt
- tools/gradle-plugin/src/main/kotlin/io/github/composegears/valkyrie/gradle/ValkyrieExtension.kt
✅ Files skipped from review due to trivial changes (9)
- gradlew
- tools/gradle-plugin/src/main/kotlin/io/github/composegears/valkyrie/gradle/dsl/ExtensionContainerExtensions.kt
- gradle/wrapper/gradle-wrapper.properties
- gradle/libs.versions.toml
- tools/gradle-plugin/build.gradle.kts
- README.md
- gradle/gradle.versions.toml
- tools/gradle-plugin/CHANGELOG.md
- AGENTS.md
🚧 Files skipped from review as they are similar to previous changes (2)
- tools/gradle-plugin/src/test/kotlin/io/github/composegears/valkyrie/gradle/ValkyrieGradlePluginTest.kt
- tools/gradle-plugin/src/main/kotlin/io/github/composegears/valkyrie/gradle/ValkyrieGradlePlugin.kt
egorikftp
force-pushed
the
feature/gradle/agp-9
branch
from
aa6a4e5 to
9b6def4
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (2)
tools/gradle-plugin/src/main/kotlin/io/github/composegears/valkyrie/gradle/dsl/TaskCollectionExtensions.kt (1)
6-9: Optional: cast toTaskCollection<Task>instead ofTaskCollection<T>to make the unchecked cast actually safe.The receiver change to
TaskCollection<*>correctly accommodates bothtasks.withType<GenerateImageVectorsTask>()andtasks.withType<AbstractKotlinCompile<*>>()call sites without variance issues — that part is good.However,
this as TaskCollection<T>is a misleading cast: the underlying collection may contain tasks that are notT. It happens to be harmless here only because the very next call (withType(T::class.java)) filters by type and returns the correctly-typedTaskCollection<T>. Casting toTaskCollection<Task>first is genuinely safe (sinceTaskCollection<*>'s upper bound isTask) and conveys intent more accurately.♻️ Proposed refactor
-@Suppress("UNCHECKED_CAST") internal inline fun <reified T : Task> TaskCollection<*>.withType(): TaskCollection<T> { - return (this as TaskCollection<T>).withType(T::class.java) + `@Suppress`("UNCHECKED_CAST") + return (this as TaskCollection<Task>).withType(T::class.java) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tools/gradle-plugin/src/main/kotlin/io/github/composegears/valkyrie/gradle/dsl/TaskCollectionExtensions.kt` around lines 6 - 9, The unchecked cast in TaskCollectionExtensions.withType() is misleading: replace the cast (this as TaskCollection<T>) with a safe cast to TaskCollection<Task> before calling withType(T::class.java) so the receiver cast reflects the actual upper bound and the subsequent withType() call does the filtering and returns TaskCollection<T>; update the single expression in withType() to cast to TaskCollection<Task> and then call .withType(T::class.java) (keeping the reified T and `@Suppress` if desired).tools/gradle-plugin/src/main/kotlin/io/github/composegears/valkyrie/gradle/internal/TaskRegistry.kt (1)
20-71: Optional: tighten theaddGeneratedSrcDircallback type.Both call sites pass a
Provider<Directory>(line 41 producesoutputRoot.map { it.dir(...) }), so the looserProvider<*>is broader than necessary and forces consumers to deal with a star-projected provider. Narrowing toProvider<Directory>improves type safety and makes the contract self-documenting (the AGPaddStaticSourceDirectoryandKotlinSourceDirectorySet.srcDirpaths both acceptProvider<Directory>cleanly).♻️ Proposed refactor
+import org.gradle.api.file.Directory import org.gradle.api.file.FileCollection import org.gradle.api.provider.Property import org.gradle.api.provider.Provider @@ internal fun registerTask( project: Project, extension: ValkyrieExtension, sourceSetName: String, - addGeneratedSrcDir: (Provider<*>) -> Unit, + addGeneratedSrcDir: (Provider<Directory>) -> Unit, ) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tools/gradle-plugin/src/main/kotlin/io/github/composegears/valkyrie/gradle/internal/TaskRegistry.kt` around lines 20 - 71, The addGeneratedSrcDir parameter on the registerTask overload is too broad (Provider<*>)—tighten it to Provider<Directory> to reflect the actual provider type produced by perSourceSetDir and improve type safety; update the declaration of the first registerTask (the one taking sourceSetName: String and addGeneratedSrcDir) to accept addGeneratedSrcDir: (Provider<Directory>) -> Unit and adjust the Kotlin-source-set overload call site (the second registerTask) to pass the same Provider<Directory> (i.e., keep addGeneratedSrcDir = { dir -> sourceSet.kotlin.srcDir(dir) }) so callers such as the perSourceSetDir mapping and sourceSet.kotlin.srcDir align without star projections.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In
`@tools/gradle-plugin/src/main/kotlin/io/github/composegears/valkyrie/gradle/dsl/TaskCollectionExtensions.kt`:
- Around line 6-9: The unchecked cast in TaskCollectionExtensions.withType() is
misleading: replace the cast (this as TaskCollection<T>) with a safe cast to
TaskCollection<Task> before calling withType(T::class.java) so the receiver cast
reflects the actual upper bound and the subsequent withType() call does the
filtering and returns TaskCollection<T>; update the single expression in
withType() to cast to TaskCollection<Task> and then call
.withType(T::class.java) (keeping the reified T and `@Suppress` if desired).
In
`@tools/gradle-plugin/src/main/kotlin/io/github/composegears/valkyrie/gradle/internal/TaskRegistry.kt`:
- Around line 20-71: The addGeneratedSrcDir parameter on the registerTask
overload is too broad (Provider<*>)—tighten it to Provider<Directory> to reflect
the actual provider type produced by perSourceSetDir and improve type safety;
update the declaration of the first registerTask (the one taking sourceSetName:
String and addGeneratedSrcDir) to accept addGeneratedSrcDir:
(Provider<Directory>) -> Unit and adjust the Kotlin-source-set overload call
site (the second registerTask) to pass the same Provider<Directory> (i.e., keep
addGeneratedSrcDir = { dir -> sourceSet.kotlin.srcDir(dir) }) so callers such as
the perSourceSetDir mapping and sourceSet.kotlin.srcDir align without star
projections.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 5d8b31f8-af21-4e9d-8023-53cf1f3254e6
📒 Files selected for processing (16)
AGENTS.mdREADME.mdgradle/gradle.versions.tomlgradle/libs.versions.tomlgradle/wrapper/gradle-wrapper.propertiesgradlewtools/gradle-plugin/CHANGELOG.mdtools/gradle-plugin/build.gradle.ktstools/gradle-plugin/src/main/kotlin/io/github/composegears/valkyrie/gradle/IconPackExtension.kttools/gradle-plugin/src/main/kotlin/io/github/composegears/valkyrie/gradle/ValkyrieExtension.kttools/gradle-plugin/src/main/kotlin/io/github/composegears/valkyrie/gradle/ValkyrieGradlePlugin.kttools/gradle-plugin/src/main/kotlin/io/github/composegears/valkyrie/gradle/dsl/ExtensionContainerExtensions.kttools/gradle-plugin/src/main/kotlin/io/github/composegears/valkyrie/gradle/dsl/TaskCollectionExtensions.kttools/gradle-plugin/src/main/kotlin/io/github/composegears/valkyrie/gradle/internal/TaskRegistry.kttools/gradle-plugin/src/test/kotlin/io/github/composegears/valkyrie/gradle/ValkyrieGradlePluginTest.kttools/gradle-plugin/src/test/kotlin/io/github/composegears/valkyrie/gradle/common/CommonGradleTest.kt
💤 Files with no reviewable changes (3)
- tools/gradle-plugin/src/test/kotlin/io/github/composegears/valkyrie/gradle/common/CommonGradleTest.kt
- tools/gradle-plugin/src/main/kotlin/io/github/composegears/valkyrie/gradle/IconPackExtension.kt
- tools/gradle-plugin/src/main/kotlin/io/github/composegears/valkyrie/gradle/ValkyrieExtension.kt
✅ Files skipped from review due to trivial changes (9)
- gradle/gradle.versions.toml
- gradlew
- tools/gradle-plugin/build.gradle.kts
- gradle/wrapper/gradle-wrapper.properties
- gradle/libs.versions.toml
- README.md
- tools/gradle-plugin/CHANGELOG.md
- AGENTS.md
- tools/gradle-plugin/src/main/kotlin/io/github/composegears/valkyrie/gradle/ValkyrieGradlePlugin.kt
🚧 Files skipped from review as they are similar to previous changes (2)
- tools/gradle-plugin/src/main/kotlin/io/github/composegears/valkyrie/gradle/dsl/ExtensionContainerExtensions.kt
- tools/gradle-plugin/src/test/kotlin/io/github/composegears/valkyrie/gradle/ValkyrieGradlePluginTest.kt
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
📝 Changelog
If this PR introduces user-facing changes, please update the relevant Unreleased section in changelogs: